Re: [Xen-devel] [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.

[Fabio Fantoni <fabio.fantoni@xxxxxxx>]

Il 13/12/2013 02:29, Konrad Rzeszutek Wilk ha scritto:
> On Thu, Dec 12, 2013 at 10:43:21AM +0000, Ian Campbell wrote:
> > On Wed, 2013-12-11 at 15:26 -0500, Konrad Rzeszutek Wilk wrote:
> >
> > > diff --git a/arch/x86/xen/platform-pci-unplug.c 
> > > b/arch/x86/xen/platform-pci-unplug.c
> > > index 0a78524..2fb9088 100644
> > > --- a/arch/x86/xen/platform-pci-unplug.c
> > > +++ b/arch/x86/xen/platform-pci-unplug.c
> > > @@ -69,6 +69,75 @@ static int check_platform_magic(void)
> > >         return 0;
> > >   }
> > >   
> > > +bool xen_has_pv_devices()
> > > +{
> > > +       if (!xen_domain())
> > > +               return false;
> > > +
> > > +       /* PV domains always have them. */
> > > +       if (xen_pv_domain())
> > > +               return true;
> > > +
> > > +       /* And user has xen_platform_pci=0 set in guest config as
> > > +        * driver did not modify the value. */
> > > +       if (xen_platform_pci_unplug == 0)
> > > +               return false;
> > > +
> > > +       if (xen_platform_pci_unplug & XEN_UNPLUG_NEVER)
> > > +               return false;
> > > +
> > > +       if (xen_platform_pci_unplug & XEN_UNPLUG_ALL)
> > > +               return true;
> > > +
> > > +       /* And the calleer has to follow with xen_pv_{disk,nic}_devices
> > "caller" (or "callee"? probably not)
> >
> > > +        * to be certain which driver can load. */
> > In the XEN_UNPLUG_UNNECESSARY case won't we end up here and return
> > false, when in fact this means we have PV devices which we want to use?
> Yes. Thanks for spotting that bug.
Thanks for your patch.

I tested the first version of this patch and was working, I should wait 
a third version before do another test or I should test also the second 
version?
Thanks for any reply.

> > > +       return false;
> > > +}
> > > +EXPORT_SYMBOL_GPL(xen_has_pv_devices);
> > > +
> > > +bool __xen_has_pv_device(int state)
> >    ^static?
> Yes :-)
> > > [...]diff --git a/include/xen/platform_pci.h b/include/xen/platform_pci.h
> > > index 438c256..b49eeab 100644
> > > --- a/include/xen/platform_pci.h
> > > +++ b/include/xen/platform_pci.h
> > > @@ -48,4 +48,27 @@ static inline int xen_must_unplug_disks(void) {
> > >   
> > >   extern int xen_platform_pci_unplug;
> > I think with all this stuff you could now make xen_platform_pci_unplug
> > private to the .c file?
> Yes, I have another patch that does that.
> > > +#if defined(CONFIG_XEN_PVHVM)
> > > +extern bool xen_has_pv_devices(void);
> > > +extern bool xen_has_pv_disk_devices(void);
> > > +extern bool xen_has_pv_nic_devices(void);
> > > +extern bool xen_has_pv_and_legacy_disk_devices(void);
> > The logic and usage for all these looked right so far as my cold fuddled
> > brain could determine.
> Yeeey!
> > Ian.
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel