On Tuesday 02 February 2010 19:33:10 Ian Campbell wrote:
> On Tue, 2010-02-02 at 08:19 +0000, Sheng Yang wrote:
> > Notice one memory region(0xfbfe0000ul - 0xfc000000ul) would be reserved
> > in the bios E820 table. This memory region would be used as grant table.
>
> I queried this requirement in a reply to the hypervisor patch.
>
> > diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> > index 05a31e5..d7dfba9 100644
> > --- a/drivers/block/xen-blkfront.c
> > +++ b/drivers/block/xen-blkfront.c
> > @@ -1071,6 +1071,9 @@ static int __init xlblk_init(void)
> > if (!xen_domain())
> > return -ENODEV;
> >
> > + if (xen_hybrid_domain() && !xen_hybrid_evtchn_enabled())
> > + return -ENODEV;
> > +
>
> This seems ugly, at the very least the check should be something like
> xen_evtchn_enabled()
Yeah, seems indeed ugly...
> but preferable would be to hook up evtchn's by
> demuxing the PCI device IRQ (the exiting PVonHVM drivers mechanism) in
> the case where hybrid evtchn's are not available and encapsulating the
> differences inside the evtchn code, there should be no need to scatter
> these sorts of checks throughout every driver.
>
> If you don't want to demux the PCI device IRQ for the non-hybrid case
> another option might be simply return failure from the evtchn operations
> if hybrid evtchns are not available and to ensure that the drivers
> handle that sort of error gracefully (which they should in any case). At
> least the difference in mode would be encapsulated that way.
>
> (same for netfront).
I am not sure if I understand you right, but I think the issue is, there is no
PVonHVM drivers in Linux upstream. The drivers are currently maintained by
OSVs, and the one in Xen upstream code only support 2.6.18. So I didn't take
them into consideration at the time.
I think the "xen_evtchn_enable()" looks much better. Would replace these ugly
lines in the next version.
>
> > if (register_blkdev(XENVBD_MAJOR, DEV_NAME)) {
> > printk(KERN_WARNING "xen_blk: can't get major %d with name
> > %s\n",
> > XENVBD_MAJOR, DEV_NAME);
> > diff --git a/drivers/input/xen-kbdfront.c b/drivers/input/xen-kbdfront.c
> > index c721c0a..74cbb25 100644
> > --- a/drivers/input/xen-kbdfront.c
> > +++ b/drivers/input/xen-kbdfront.c
> > @@ -341,6 +341,10 @@ static int __init xenkbd_init(void)
> > if (!xen_domain())
> > return -ENODEV;
> >
> > + /* Xen Hybrid domain don't need vkbd */
> > + if (xen_hybrid_domain())
> > + return -ENODEV;
> > +
>
> Why disallow it if the platform has specified it (same for fbfront)?
Well.. The direct reason is I didn't test them and don't know what would
happen... I would give it a try later.
>
> > diff --git a/include/xen/xen.h b/include/xen/xen.h
> > index aace9cc..632e76f 100644
> > --- a/include/xen/xen.h
> > +++ b/include/xen/xen.h
> > @@ -5,6 +5,7 @@ enum xen_domain_type {
> > XEN_NATIVE, /* running on bare hardware */
> > XEN_PV_DOMAIN, /* running in a PV domain */
> > XEN_HVM_DOMAIN, /* running in a Xen hvm domain */
> > + XEN_HYBRID_DOMAIN, /* running in a Xen hybrid hvm domain */
> > };
>
> I don't think you should need to distinguish HYBRID from HVM mode...
The only purpose is for event channel of hybrid... Yes, I think I can find
other way to indicate the availability of event channel. :)
--
regards
Yang, Sheng
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home