lkml.org 
[lkml]   [2010]   [Feb]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [Xen-devel] [PATCH 6/6] xen/hybrid: Enable grant table and xenbus
    Date
    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


    \
     
     \ /
      Last update: 2010-02-02 14:29    [W:0.027 / U:0.536 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site