[dpdk-dev] [PATCH] BugFix: VFIO never works

[dpdk-dev] [PATCH] BugFix: VFIO never works

[Michael Qiu]

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
 		break;
 	case RTE_KDRV_IGB_UIO:
 	case RTE_KDRV_UIO_GENERIC:
diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
index e16bb68..10995c3 100644
--- a/lib/librte_eal/common/eal_private.h
+++ b/lib/librte_eal/common/eal_private.h
@@ -174,6 +174,9 @@ int pci_unbind_kernel_driver(struct rte_pci_device *dev);
  */
 int pci_uio_map_resource(struct rte_pci_device *dev);
 
+int pci_vfio_is_enabled(void);
+
+int pci_vfio_map_resource(struct rte_pci_device *dev);
 /**
  * Unmap the PCI resource of a PCI device
  *
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
index 426953a..3b5da34 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
@@ -909,4 +909,14 @@ pci_vfio_is_enabled(void)
 {
 	return vfio_cfg.vfio_enabled;
 }
+#else
+int pci_vfio_is_enabled(void)
+{
+	return 0;
+}
+
+int pci_vfio_map_resource(__rte_unused struct rte_pci_device *dev)
+{
+	return -1;
+}
 #endif
-- 
2.1.0

Re: [dpdk-dev] [PATCH] BugFix: VFIO never works

[Tetsuya Mukawa]

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
>  		break;
>  	case RTE_KDRV_IGB_UIO:
>  	case RTE_KDRV_UIO_GENERIC:
> diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
> index e16bb68..10995c3 100644
> --- a/lib/librte_eal/common/eal_private.h
> +++ b/lib/librte_eal/common/eal_private.h
> @@ -174,6 +174,9 @@ int pci_unbind_kernel_driver(struct rte_pci_device *dev);
>   */
>  int pci_uio_map_resource(struct rte_pci_device *dev);
>  
> +int pci_vfio_is_enabled(void);
> +
> +int pci_vfio_map_resource(struct rte_pci_device *dev);
>  /**
>   * Unmap the PCI resource of a PCI device
>   *
> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
> index 426953a..3b5da34 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
> @@ -909,4 +909,14 @@ pci_vfio_is_enabled(void)
>  {
>  	return vfio_cfg.vfio_enabled;
>  }
> +#else
> +int pci_vfio_is_enabled(void)
> +{
> +	return 0;
> +}
> +
> +int pci_vfio_map_resource(__rte_unused struct rte_pci_device *dev)
> +{
> +	return -1;
> +}
>  #endif

Tested-by: Tetsuya Mukawa <mukawa@igel.co.jp>
Acked-by: Tetsuya Mukawa <mukawa@igel.co.jp>


Hi Michael,


Thanks, I haven't checked it with vfio.
I've make sure the patch works fine with vfio and pci_uio_generic.

Regards,
Tetsuya

Re: [dpdk-dev] [PATCH] BugFix: VFIO never works

[David Marchand]

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 ?


-- 
David Marchand

Re: [dpdk-dev] [PATCH] BugFix: VFIO never works

[Tetsuya Mukawa]

2015-07-10 20:25 GMT+09:00 David Marchand <david.marchand@6wind.com>:
> 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 ?
>
>
> --
> David Marchand

Hi David,

Yes I did.
Before submitting the patch series, I've tested it with igb_uio on
Linux and with nic_uio on BSD.
Both were OK. But I didn't test it with vfio and pci_uio generic on Linux.
After receiving the patch, I tested both. So now all cases should be ok.

Tetsuya

Re: [dpdk-dev] [PATCH] BugFix: VFIO never works

[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>
> > > ---
> > >  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

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)

Re: [dpdk-dev] [PATCH] BugFix: VFIO never works

[Tetsuya Mukawa]

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)
>

Re: [dpdk-dev] [PATCH] BugFix: VFIO never works

[Thomas Monjalon]

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.

[dpdk-dev] [PATCH] eal: fix vfio device never works.

[Tetsuya Mukawa]

The patch fixes vfio initialization issue introduced by below patch.
 - Commit 35b3313e322b ("pci: merge mapping functions for linux and bsd")

Root cause is that VFIO_PRESENT is inaccessible in eal common level.
To fix it, remove pci_map/unmap_device from common code, then implement
in linux and bsd code.

Reported-by: Michael Qiu <michael.qiu@intel.com>
Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
---
 lib/librte_eal/bsdapp/eal/eal_pci.c    | 39 ++++++++++++++++++++++++
 lib/librte_eal/common/eal_common_pci.c | 55 ----------------------------------
 lib/librte_eal/common/eal_private.h    | 18 +++++++++++
 lib/librte_eal/linuxapp/eal/eal_pci.c  | 50 +++++++++++++++++++++++++++++++
 4 files changed, 107 insertions(+), 55 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
index 12f39d9..ed31222 100644
--- a/lib/librte_eal/bsdapp/eal/eal_pci.c
+++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
@@ -92,6 +92,45 @@ pci_unbind_kernel_driver(struct rte_pci_device *dev __rte_unused)
 	return -ENOTSUP;
 }
 
+/* Map pci device */
+int
+pci_map_device(struct rte_pci_device *dev)
+{
+	int ret = -1;
+
+	/* try mapping the NIC resources */
+	switch (dev->kdrv) {
+	case RTE_KDRV_NIC_UIO:
+		/* map resources for devices that use uio */
+		ret = pci_uio_map_resource(dev);
+		break;
+	default:
+		RTE_LOG(DEBUG, EAL,
+			"  Not managed by a supported kernel driver, skipped\n");
+		ret = 1;
+		break;
+	}
+
+	return ret;
+}
+
+/* Unmap pci device */
+void
+pci_unmap_device(struct rte_pci_device *dev)
+{
+	/* try unmapping the NIC resources */
+	switch (dev->kdrv) {
+	case RTE_KDRV_NIC_UIO:
+		/* unmap resources for devices that use uio */
+		pci_uio_unmap_resource(dev);
+		break;
+	default:
+		RTE_LOG(DEBUG, EAL,
+			"  Not managed by a supported kernel driver, skipped\n");
+		break;
+	}
+}
+
 void
 pci_uio_free_resource(struct rte_pci_device *dev,
 		struct mapped_pci_resource *uio_res)
diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index 3805aed..60e40e0 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -137,61 +137,6 @@ pci_unmap_resource(void *requested_addr, size_t size)
 				requested_addr);
 }
 
-/* Map pci device */
-static int
-pci_map_device(struct rte_pci_device *dev)
-{
-	int ret = -1;
-
-	/* 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
-		break;
-	case RTE_KDRV_IGB_UIO:
-	case RTE_KDRV_UIO_GENERIC:
-	case RTE_KDRV_NIC_UIO:
-		/* map resources for devices that use uio */
-		ret = pci_uio_map_resource(dev);
-		break;
-	default:
-		RTE_LOG(DEBUG, EAL, "  Not managed by a supported kernel driver,"
-			" skipped\n");
-		ret = 1;
-		break;
-	}
-
-	return ret;
-}
-
-/* Unmap pci device */
-static void
-pci_unmap_device(struct rte_pci_device *dev)
-{
-	if (dev == NULL)
-		return;
-
-	/* try unmapping the NIC resources using VFIO if it exists */
-	switch (dev->kdrv) {
-	case RTE_KDRV_VFIO:
-		RTE_LOG(ERR, EAL, "Hotplug doesn't support vfio yet\n");
-		break;
-	case RTE_KDRV_IGB_UIO:
-	case RTE_KDRV_UIO_GENERIC:
-	case RTE_KDRV_NIC_UIO:
-		/* unmap resources for devices that use uio */
-		pci_uio_unmap_resource(dev);
-		break;
-	default:
-		RTE_LOG(DEBUG, EAL, "  Not managed by a supported kernel driver,"
-			" skipped\n");
-		break;
-	}
-}
-
 /*
  * If vendor/device ID match, call the devinit() function of the
  * driver.
diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
index e16bb68..73363f6 100644
--- a/lib/librte_eal/common/eal_private.h
+++ b/lib/librte_eal/common/eal_private.h
@@ -165,6 +165,24 @@ struct rte_pci_device;
 int pci_unbind_kernel_driver(struct rte_pci_device *dev);
 
 /**
+ * Map this device
+ *
+ * This function is private to EAL.
+ *
+ * @return
+ *   0 on success, negative on error and positive if no driver
+ *   is found for the device.
+ */
+int pci_map_device(struct rte_pci_device *dev);
+
+/**
+ * Unmap this device
+ *
+ * This function is private to EAL.
+ */
+void pci_unmap_device(struct rte_pci_device *dev);
+
+/**
  * Map the PCI resource of a PCI device in virtual memory
  *
  * This function is private to EAL.
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index 1d5a13b..9a28ede 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -123,6 +123,56 @@ pci_get_kernel_driver_by_path(const char *filename, char *dri_name)
 	return -1;
 }
 
+/* Map pci device */
+int
+pci_map_device(struct rte_pci_device *dev)
+{
+	int ret = -1;
+
+	/* 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
+		break;
+	case RTE_KDRV_IGB_UIO:
+	case RTE_KDRV_UIO_GENERIC:
+		/* map resources for devices that use uio */
+		ret = pci_uio_map_resource(dev);
+		break;
+	default:
+		RTE_LOG(DEBUG, EAL,
+			"  Not managed by a supported kernel driver, skipped\n");
+		ret = 1;
+		break;
+	}
+
+	return ret;
+}
+
+/* Unmap pci device */
+void
+pci_unmap_device(struct rte_pci_device *dev)
+{
+	/* try unmapping the NIC resources using VFIO if it exists */
+	switch (dev->kdrv) {
+	case RTE_KDRV_VFIO:
+		RTE_LOG(ERR, EAL, "Hotplug doesn't support vfio yet\n");
+		break;
+	case RTE_KDRV_IGB_UIO:
+	case RTE_KDRV_UIO_GENERIC:
+		/* unmap resources for devices that use uio */
+		pci_uio_unmap_resource(dev);
+		break;
+	default:
+		RTE_LOG(DEBUG, EAL,
+			"  Not managed by a supported kernel driver, skipped\n");
+		break;
+	}
+}
+
 void *
 pci_find_max_end_va(void)
 {
-- 
2.1.4

Re: [dpdk-dev] [PATCH] BugFix: VFIO never works

[Tetsuya Mukawa]

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

Re: [dpdk-dev] [PATCH] eal: fix vfio device never works.

[Qiu, Michael]

Hi, Tetsuya

So this patch  make linux app back to original?

Thanks,
Michael

On 7/11/2015 2:30 PM, Tetsuya Mukawa wrote:
> The patch fixes vfio initialization issue introduced by below patch.
>  - Commit 35b3313e322b ("pci: merge mapping functions for linux and bsd")
>
> Root cause is that VFIO_PRESENT is inaccessible in eal common level.
> To fix it, remove pci_map/unmap_device from common code, then implement
> in linux and bsd code.
>
> Reported-by: Michael Qiu <michael.qiu@intel.com>
> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
> ---
>  lib/librte_eal/bsdapp/eal/eal_pci.c    | 39 ++++++++++++++++++++++++
>  lib/librte_eal/common/eal_common_pci.c | 55 ----------------------------------
>  lib/librte_eal/common/eal_private.h    | 18 +++++++++++
>  lib/librte_eal/linuxapp/eal/eal_pci.c  | 50 +++++++++++++++++++++++++++++++
>  4 files changed, 107 insertions(+), 55 deletions(-)
>
> diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
> index 12f39d9..ed31222 100644
> --- a/lib/librte_eal/bsdapp/eal/eal_pci.c
> +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
> @@ -92,6 +92,45 @@ pci_unbind_kernel_driver(struct rte_pci_device *dev __rte_unused)
>  	return -ENOTSUP;
>  }
>  
> +/* Map pci device */
> +int
> +pci_map_device(struct rte_pci_device *dev)
> +{
> +	int ret = -1;
> +
> +	/* try mapping the NIC resources */
> +	switch (dev->kdrv) {
> +	case RTE_KDRV_NIC_UIO:
> +		/* map resources for devices that use uio */
> +		ret = pci_uio_map_resource(dev);
> +		break;
> +	default:
> +		RTE_LOG(DEBUG, EAL,
> +			"  Not managed by a supported kernel driver, skipped\n");
> +		ret = 1;
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +/* Unmap pci device */
> +void
> +pci_unmap_device(struct rte_pci_device *dev)
> +{
> +	/* try unmapping the NIC resources */
> +	switch (dev->kdrv) {
> +	case RTE_KDRV_NIC_UIO:
> +		/* unmap resources for devices that use uio */
> +		pci_uio_unmap_resource(dev);
> +		break;
> +	default:
> +		RTE_LOG(DEBUG, EAL,
> +			"  Not managed by a supported kernel driver, skipped\n");
> +		break;
> +	}
> +}
> +
>  void
>  pci_uio_free_resource(struct rte_pci_device *dev,
>  		struct mapped_pci_resource *uio_res)
> diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
> index 3805aed..60e40e0 100644
> --- a/lib/librte_eal/common/eal_common_pci.c
> +++ b/lib/librte_eal/common/eal_common_pci.c
> @@ -137,61 +137,6 @@ pci_unmap_resource(void *requested_addr, size_t size)
>  				requested_addr);
>  }
>  
> -/* Map pci device */
> -static int
> -pci_map_device(struct rte_pci_device *dev)
> -{
> -	int ret = -1;
> -
> -	/* 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
> -		break;
> -	case RTE_KDRV_IGB_UIO:
> -	case RTE_KDRV_UIO_GENERIC:
> -	case RTE_KDRV_NIC_UIO:
> -		/* map resources for devices that use uio */
> -		ret = pci_uio_map_resource(dev);
> -		break;
> -	default:
> -		RTE_LOG(DEBUG, EAL, "  Not managed by a supported kernel driver,"
> -			" skipped\n");
> -		ret = 1;
> -		break;
> -	}
> -
> -	return ret;
> -}
> -
> -/* Unmap pci device */
> -static void
> -pci_unmap_device(struct rte_pci_device *dev)
> -{
> -	if (dev == NULL)
> -		return;
> -
> -	/* try unmapping the NIC resources using VFIO if it exists */
> -	switch (dev->kdrv) {
> -	case RTE_KDRV_VFIO:
> -		RTE_LOG(ERR, EAL, "Hotplug doesn't support vfio yet\n");
> -		break;
> -	case RTE_KDRV_IGB_UIO:
> -	case RTE_KDRV_UIO_GENERIC:
> -	case RTE_KDRV_NIC_UIO:
> -		/* unmap resources for devices that use uio */
> -		pci_uio_unmap_resource(dev);
> -		break;
> -	default:
> -		RTE_LOG(DEBUG, EAL, "  Not managed by a supported kernel driver,"
> -			" skipped\n");
> -		break;
> -	}
> -}
> -
>  /*
>   * If vendor/device ID match, call the devinit() function of the
>   * driver.
> diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
> index e16bb68..73363f6 100644
> --- a/lib/librte_eal/common/eal_private.h
> +++ b/lib/librte_eal/common/eal_private.h
> @@ -165,6 +165,24 @@ struct rte_pci_device;
>  int pci_unbind_kernel_driver(struct rte_pci_device *dev);
>  
>  /**
> + * Map this device
> + *
> + * This function is private to EAL.
> + *
> + * @return
> + *   0 on success, negative on error and positive if no driver
> + *   is found for the device.
> + */
> +int pci_map_device(struct rte_pci_device *dev);
> +
> +/**
> + * Unmap this device
> + *
> + * This function is private to EAL.
> + */
> +void pci_unmap_device(struct rte_pci_device *dev);
> +
> +/**
>   * Map the PCI resource of a PCI device in virtual memory
>   *
>   * This function is private to EAL.
> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
> index 1d5a13b..9a28ede 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_pci.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
> @@ -123,6 +123,56 @@ pci_get_kernel_driver_by_path(const char *filename, char *dri_name)
>  	return -1;
>  }
>  
> +/* Map pci device */
> +int
> +pci_map_device(struct rte_pci_device *dev)
> +{
> +	int ret = -1;
> +
> +	/* 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
> +		break;
> +	case RTE_KDRV_IGB_UIO:
> +	case RTE_KDRV_UIO_GENERIC:
> +		/* map resources for devices that use uio */
> +		ret = pci_uio_map_resource(dev);
> +		break;
> +	default:
> +		RTE_LOG(DEBUG, EAL,
> +			"  Not managed by a supported kernel driver, skipped\n");
> +		ret = 1;
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +/* Unmap pci device */
> +void
> +pci_unmap_device(struct rte_pci_device *dev)
> +{
> +	/* try unmapping the NIC resources using VFIO if it exists */
> +	switch (dev->kdrv) {
> +	case RTE_KDRV_VFIO:
> +		RTE_LOG(ERR, EAL, "Hotplug doesn't support vfio yet\n");
> +		break;
> +	case RTE_KDRV_IGB_UIO:
> +	case RTE_KDRV_UIO_GENERIC:
> +		/* unmap resources for devices that use uio */
> +		pci_uio_unmap_resource(dev);
> +		break;
> +	default:
> +		RTE_LOG(DEBUG, EAL,
> +			"  Not managed by a supported kernel driver, skipped\n");
> +		break;
> +	}
> +}
> +
>  void *
>  pci_find_max_end_va(void)
>  {

Re: [dpdk-dev] [PATCH] eal: fix vfio device never works.

[Tetsuya Mukawa]

On 2015/07/14 14:03, Qiu, Michael wrote:
> Hi, Tetsuya
>
> So this patch  make linux app back to original?

For linux code, it is yes.
For bsd code, it's slight different, because original bsd code doesn't
have pci_map/unmap_device().
When I implement "Commit 35b3313e322b", I should have just implemented
pci_map/unmap_device() in bsd code like linux.

Thanks,
Tetsuya

> Thanks,
> Michael
>
>

Re: [dpdk-dev] [PATCH] eal: fix vfio device never works.

[Thomas Monjalon]

2015-07-11 15:29, Tetsuya Mukawa:
> The patch fixes vfio initialization issue introduced by below patch.
>  - Commit 35b3313e322b ("pci: merge mapping functions for linux and bsd")
> 
> Root cause is that VFIO_PRESENT is inaccessible in eal common level.
> To fix it, remove pci_map/unmap_device from common code, then implement
> in linux and bsd code.
> 
> Reported-by: Michael Qiu <michael.qiu@intel.com>
> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>

Applied, thanks