From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up. Date: Fri, 13 Dec 2013 09:39:30 -0500 Message-ID: <20131213143930.GJ2923@phenom.dumpdata.com> References: <1386154783.17466.14.camel@kazak.uk.xensource.com> <1386155291.17466.19.camel@kazak.uk.xensource.com> <1386156220.17466.29.camel@kazak.uk.xensource.com> <20131211202657.GA5153@phenom.dumpdata.com> <1386845001.12610.57.camel@kazak.uk.xensource.com> <20131213012925.GB2697@pegasus.dumpdata.com> <52AADA33.7070702@m2r.biz> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1VrTv0-0007fV-4U for xen-devel@lists.xenproject.org; Fri, 13 Dec 2013 14:40:42 +0000 Content-Disposition: inline In-Reply-To: <52AADA33.7070702@m2r.biz> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Fabio Fantoni Cc: Ian Campbell , Stefano Stabellini , dmitry.torokhov@gmail.com, david.vrabel@citrix.com, xen-devel@lists.xenproject.org, boris.ostrovsky@oracle.com List-Id: xen-devel@lists.xenproject.org On Fri, Dec 13, 2013 at 10:58:11AM +0100, Fabio Fantoni wrote: > 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? Lets wait until I get an Reviewed-by from either Stefano or Ian. Wouldn't want to spend extra cycles to retest the similar patch over and over. > > Thanks for any reply. Thank you for the question! > > >>>+ 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@lists.xen.org > >http://lists.xen.org/xen-devel >