| Message ID | 20150710113826.GA10608@bricha3-MOBL3 (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 71AE3ADA7; Fri, 10 Jul 2015 13:38:32 +0200 (CEST) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 33F5E9A81 for <dev@dpdk.org>; Fri, 10 Jul 2015 13:38:31 +0200 (CEST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga102.jf.intel.com with ESMTP; 10 Jul 2015 04:38:31 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.15,446,1432623600"; d="scan'208";a="759767724" Received: from bricha3-mobl3.ger.corp.intel.com ([10.237.208.63]) by fmsmga002.fm.intel.com with SMTP; 10 Jul 2015 04:38:28 -0700 Received: by (sSMTP sendmail emulation); Fri, 10 Jul 2015 12:38:27 +0025 Date: Fri, 10 Jul 2015 12:38:26 +0100 From: Bruce Richardson <bruce.richardson@intel.com> To: David Marchand <david.marchand@6wind.com> Message-ID: <20150710113826.GA10608@bricha3-MOBL3> References: <1436514439-4893-1-git-send-email-michael.qiu@intel.com> <559F83AE.4050409@igel.co.jp> <CALwxeUsTYhjXbsR8XeFuaYeA+nzMHj1esqb_D4nb+49xCD2K7A@mail.gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <CALwxeUsTYhjXbsR8XeFuaYeA+nzMHj1esqb_D4nb+49xCD2K7A@mail.gmail.com> Organization: Intel Shannon Ltd. User-Agent: Mutt/1.5.23 (2014-03-12) Cc: "dev@dpdk.org" <dev@dpdk.org> Subject: Re: [dpdk-dev] [PATCH] BugFix: VFIO never works X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Bruce Richardson
July 10, 2015, 11:38 a.m. UTC
On Fri, Jul 10, 2015 at 01:25:49PM +0200, David Marchand wrote: > On Fri, Jul 10, 2015 at 10:34 AM, Tetsuya Mukawa <mukawa@igel.co.jp> wrote: > > > On 2015/07/10 16:47, Michael Qiu wrote: > > > Commit 35b3313e322b ("pci: merge mapping functions for linux and bsd") > > > > > > introduced a bug that all vfio will be > > > blocked. > > > > > > Root cause is that VFIO_PRESENT is unaccessable in eal > > > common level. > > > > > > This patch is to fix this. > > > > > > Signed-off-by: Michael Qiu <michael.qiu@intel.com> > > > --- > > > lib/librte_eal/common/eal_common_pci.c | 2 -- > > > lib/librte_eal/common/eal_private.h | 3 +++ > > > lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 10 ++++++++++ > > > 3 files changed, 13 insertions(+), 2 deletions(-) > > > > > > diff --git a/lib/librte_eal/common/eal_common_pci.c > > b/lib/librte_eal/common/eal_common_pci.c > > > index 3805aed..f3dc697 100644 > > > --- a/lib/librte_eal/common/eal_common_pci.c > > > +++ b/lib/librte_eal/common/eal_common_pci.c > > > @@ -146,10 +146,8 @@ pci_map_device(struct rte_pci_device *dev) > > > /* try mapping the NIC resources using VFIO if it exists */ > > > switch (dev->kdrv) { > > > case RTE_KDRV_VFIO: > > > -#ifdef VFIO_PRESENT > > > if (pci_vfio_is_enabled()) > > > ret = pci_vfio_map_resource(dev); > > > -#endif > > > > This is a common file, vfio is not available on BSD. > I missed that during review. > > Did you test build on BSD ? > Just tried it. BSD build fails with this patch applied. Rather than using ifdefs, we could also look at providing dummy functions for vfio in BSD. They would never be called by this code as drivers in BSD should never have dev->kdrv == RTE_KDRV_VFIO. /Bruce
Comments
2015-07-10 20:38 GMT+09:00 Bruce Richardson <bruce.richardson@intel.com>: > On Fri, Jul 10, 2015 at 01:25:49PM +0200, David Marchand wrote: >> On Fri, Jul 10, 2015 at 10:34 AM, Tetsuya Mukawa <mukawa@igel.co.jp> wrote: >> >> > On 2015/07/10 16:47, Michael Qiu wrote: >> > > Commit 35b3313e322b ("pci: merge mapping functions for linux and bsd") >> > > >> > > introduced a bug that all vfio will be >> > > blocked. >> > > >> > > Root cause is that VFIO_PRESENT is unaccessable in eal >> > > common level. >> > > >> > > This patch is to fix this. >> > > >> > > Signed-off-by: Michael Qiu <michael.qiu@intel.com> >> > > --- >> > > lib/librte_eal/common/eal_common_pci.c | 2 -- >> > > lib/librte_eal/common/eal_private.h | 3 +++ >> > > lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 10 ++++++++++ >> > > 3 files changed, 13 insertions(+), 2 deletions(-) >> > > >> > > diff --git a/lib/librte_eal/common/eal_common_pci.c >> > b/lib/librte_eal/common/eal_common_pci.c >> > > index 3805aed..f3dc697 100644 >> > > --- a/lib/librte_eal/common/eal_common_pci.c >> > > +++ b/lib/librte_eal/common/eal_common_pci.c >> > > @@ -146,10 +146,8 @@ pci_map_device(struct rte_pci_device *dev) >> > > /* try mapping the NIC resources using VFIO if it exists */ >> > > switch (dev->kdrv) { >> > > case RTE_KDRV_VFIO: >> > > -#ifdef VFIO_PRESENT >> > > if (pci_vfio_is_enabled()) >> > > ret = pci_vfio_map_resource(dev); >> > > -#endif >> > >> >> This is a common file, vfio is not available on BSD. >> I missed that during review. >> >> Did you test build on BSD ? >> > Just tried it. BSD build fails with this patch applied. > > Rather than using ifdefs, we could also look at providing dummy functions for > vfio in BSD. They would never be called by this code as drivers in BSD should > never have dev->kdrv == RTE_KDRV_VFIO. > > /Bruce I've got it. Right, we need to fix the patch. I don't have BSD machine within reach now. So I will write and test a new patch tomorrow. Tetsuya > > diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c > index 12f39d9..8c1422c 100644 > --- a/lib/librte_eal/bsdapp/eal/eal_pci.c > +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c > @@ -358,6 +358,16 @@ error: > return -1; > } > > +/* > + * no vfio on BSD, but provide empty functions to avoid compile errors in > + * common code. > + */ > +int > +pci_vfio_is_enabled(void) { return 0; } > + > +int > +pci_vfio_map_resource(struct rte_pci_device *dev __rte_unused) { return -1; } > + > /* Init the PCI EAL subsystem */ > int > rte_eal_pci_init(void) >
2015-07-10 12:38, Bruce Richardson: > On Fri, Jul 10, 2015 at 01:25:49PM +0200, David Marchand wrote: > > On Fri, Jul 10, 2015 at 10:34 AM, Tetsuya Mukawa <mukawa@igel.co.jp> wrote: > > > On 2015/07/10 16:47, Michael Qiu wrote: > > > > Commit 35b3313e322b ("pci: merge mapping functions for linux and bsd") > > > > > > > > introduced a bug that all vfio will be > > > > blocked. > > > > > > > > Root cause is that VFIO_PRESENT is unaccessable in eal > > > > common level. > > > > > > > > This patch is to fix this. > > > > > > > > Signed-off-by: Michael Qiu <michael.qiu@intel.com> [...] > > > > --- a/lib/librte_eal/common/eal_common_pci.c > > > > +++ b/lib/librte_eal/common/eal_common_pci.c > > > > @@ -146,10 +146,8 @@ pci_map_device(struct rte_pci_device *dev) > > > > /* try mapping the NIC resources using VFIO if it exists */ > > > > switch (dev->kdrv) { > > > > case RTE_KDRV_VFIO: > > > > -#ifdef VFIO_PRESENT > > > > if (pci_vfio_is_enabled()) > > > > ret = pci_vfio_map_resource(dev); > > > > -#endif > > > > > > > This is a common file, vfio is not available on BSD. > > I missed that during review. > > > > Did you test build on BSD ? > > > Just tried it. BSD build fails with this patch applied. > > Rather than using ifdefs, we could also look at providing dummy functions for > vfio in BSD. They would never be called by this code as drivers in BSD should > never have dev->kdrv == RTE_KDRV_VFIO. Why not implementing different versions of pci_map_device() and pci_unmap_device() in linuxapp and bsdapp EAL? They are only wrappers so no code would be duplicated.
On 2015/07/10 21:16, Thomas Monjalon wrote: > 2015-07-10 12:38, Bruce Richardson: >> On Fri, Jul 10, 2015 at 01:25:49PM +0200, David Marchand wrote: >>> On Fri, Jul 10, 2015 at 10:34 AM, Tetsuya Mukawa <mukawa@igel.co.jp> wrote: >>>> On 2015/07/10 16:47, Michael Qiu wrote: >>>>> Commit 35b3313e322b ("pci: merge mapping functions for linux and bsd") >>>>> >>>>> introduced a bug that all vfio will be >>>>> blocked. >>>>> >>>>> Root cause is that VFIO_PRESENT is unaccessable in eal >>>>> common level. >>>>> >>>>> This patch is to fix this. >>>>> >>>>> Signed-off-by: Michael Qiu <michael.qiu@intel.com> > [...] >>>>> --- a/lib/librte_eal/common/eal_common_pci.c >>>>> +++ b/lib/librte_eal/common/eal_common_pci.c >>>>> @@ -146,10 +146,8 @@ pci_map_device(struct rte_pci_device *dev) >>>>> /* try mapping the NIC resources using VFIO if it exists */ >>>>> switch (dev->kdrv) { >>>>> case RTE_KDRV_VFIO: >>>>> -#ifdef VFIO_PRESENT >>>>> if (pci_vfio_is_enabled()) >>>>> ret = pci_vfio_map_resource(dev); >>>>> -#endif >>> This is a common file, vfio is not available on BSD. >>> I missed that during review. >>> >>> Did you test build on BSD ? >>> >> Just tried it. BSD build fails with this patch applied. >> >> Rather than using ifdefs, we could also look at providing dummy functions for >> vfio in BSD. They would never be called by this code as drivers in BSD should >> never have dev->kdrv == RTE_KDRV_VFIO. > Why not implementing different versions of pci_map_device() and pci_unmap_device() > in linuxapp and bsdapp EAL? They are only wrappers so no code would be duplicated. Thanks for comment. Right, it should be. I fixed like above. Tetsuya
diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c index 12f39d9..8c1422c 100644 --- a/lib/librte_eal/bsdapp/eal/eal_pci.c +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c @@ -358,6 +358,16 @@ error: return -1; } +/* + * no vfio on BSD, but provide empty functions to avoid compile errors in + * common code. + */ +int +pci_vfio_is_enabled(void) { return 0; } + +int +pci_vfio_map_resource(struct rte_pci_device *dev __rte_unused) { return -1; } + /* Init the PCI EAL subsystem */ int rte_eal_pci_init(void)