WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH 6/6] xen/hybrid: Enable grant table and xenbus

To: Ian Campbell <Ian.Campbell@xxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH 6/6] xen/hybrid: Enable grant table and xenbus
From: Sheng Yang <sheng@xxxxxxxxxxxxxxx>
Date: Tue, 2 Feb 2010 21:24:40 +0800
Cc: xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxx>, Jeremy Fitzhardinge <Jeremy.Fitzhardinge@xxxxxxxxxx>, Keir Fraser <Keir.Fraser@xxxxxxxxxxxxx>, "linux-kernel@xxxxxxxxxxxxxxx" <linux-kernel@xxxxxxxxxxxxxxx>
Delivery-date: Tue, 02 Feb 2010 05:26:33 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <1265110390.2965.22456.camel@xxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Organization: Intel Opensource Technology Center
References: <1265098747-10117-1-git-send-email-sheng@xxxxxxxxxxxxxxx> <1265098747-10117-7-git-send-email-sheng@xxxxxxxxxxxxxxx> <1265110390.2965.22456.camel@xxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: KMail/1.12.2 (Linux/2.6.31-17-generic; KDE/4.3.2; x86_64; ; )
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