[dpdk-dev] [PATCH] vfio: support iommu group zero

[dpdk-dev] [PATCH] vfio: support iommu group zero

[Stephen Hemminger]

The current implementation of VFIO will not with the new no-IOMMU mode
in 4.4 kernel. The original code assumed that IOMMU group zero would
never be used. Group numbers are assigned starting at zero, and up
until now the group numbers came from the hardware which is likely
to use group 0 for system devices that are not used with DPDK.

The fix is to allow 0 as a valid group and rearrange code
to split the return value from the group value.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
Why was this ignored? It was originally sent on 26 Oct 15 back
when IOMMU discussion was lively.

 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
index 0e6c48a..74f91ba 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
@@ -499,14 +499,15 @@ pci_vfio_get_group_fd(int iommu_group_no)
 }
 
 /* parse IOMMU group number for a PCI device
- * returns -1 for errors, 0 for non-existent group */
+ * returns 1 on success, -1 for errors, 0 for non-existent group
+ */
 static int
-pci_vfio_get_group_no(const char *pci_addr)
+pci_vfio_get_group_no(const char *pci_addr, int *iommu_group_no)
 {
 	char linkname[PATH_MAX];
 	char filename[PATH_MAX];
 	char *tok[16], *group_tok, *end;
-	int ret, iommu_group_no;
+	int ret;
 
 	memset(linkname, 0, sizeof(linkname));
 	memset(filename, 0, sizeof(filename));
@@ -533,13 +534,13 @@ pci_vfio_get_group_no(const char *pci_addr)
 	errno = 0;
 	group_tok = tok[ret - 1];
 	end = group_tok;
-	iommu_group_no = strtol(group_tok, &end, 10);
+	*iommu_group_no = strtol(group_tok, &end, 10);
 	if ((end != group_tok && *end != '\0') || errno != 0) {
 		RTE_LOG(ERR, EAL, "  %s error parsing IOMMU number!\n", pci_addr);
 		return -1;
 	}
 
-	return iommu_group_no;
+	return 1;
 }
 
 static void
@@ -581,16 +582,15 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
 			loc->domain, loc->bus, loc->devid, loc->function);
 
 	/* get group number */
-	iommu_group_no = pci_vfio_get_group_no(pci_addr);
-
-	/* if 0, group doesn't exist */
-	if (iommu_group_no == 0) {
+	ret = pci_vfio_get_group_no(pci_addr, &iommu_group_no);
+	if (ret == 0) {
 		RTE_LOG(WARNING, EAL, "  %s not managed by VFIO driver, skipping\n",
-				pci_addr);
+			pci_addr);
 		return 1;
 	}
+
 	/* if negative, something failed */
-	else if (iommu_group_no < 0)
+	if (ret < 0)
 		return -1;
 
 	/* get the actual group fd */
-- 
2.1.4

Re: [dpdk-dev] [PATCH] vfio: support iommu group zero

[Thomas Monjalon]

2015-12-09 09:55, Stephen Hemminger:
> The current implementation of VFIO will not with the new no-IOMMU mode
> in 4.4 kernel. The original code assumed that IOMMU group zero would
> never be used. Group numbers are assigned starting at zero, and up
> until now the group numbers came from the hardware which is likely
> to use group 0 for system devices that are not used with DPDK.
> 
> The fix is to allow 0 as a valid group and rearrange code
> to split the return value from the group value.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> Why was this ignored? It was originally sent on 26 Oct 15 back
> when IOMMU discussion was lively.

There was no review of this patch.
The patch has been marked as deferred recently when it was too late
to do such feature changes in DPDK code:
	http://dpdk.org/dev/patchwork/patch/8035/

So I guess you have tested the VFIO no-iommu patch.
Is it working well with DPDK with only this patch for group zero?
Have you talked to Alex Williamson?
He was requesting some feedbacks:
	http://dpdk.org/ml/archives/dev/2015-December/029291.html

Thanks

Re: [dpdk-dev] [PATCH] vfio: support iommu group zero

[Stephen Hemminger]

On Wed, 09 Dec 2015 22:12:33 +0100
Thomas Monjalon <thomas.monjalon@6wind.com> wrote:

> So I guess you have tested the VFIO no-iommu patch.
> Is it working well with DPDK with only this patch for group zero?
> Have you talked to Alex Williamson?

I did test it enough to bring up a link but I needed this patch.
The patch fixes an incorrect assumption in original DPDK use of
VFIO, it just isn't visible on normal IOMMU hardware.

The real goal was to be able to use per-rxq interrupts but didn't
get that far along (because of time and other application issues).

Re: [dpdk-dev] [PATCH] vfio: support iommu group zero

[Stephen Hemminger]

On Wed, 09 Dec 2015 22:12:33 +0100
Thomas Monjalon <thomas.monjalon@6wind.com> wrote:

> 2015-12-09 09:55, Stephen Hemminger:
> > The current implementation of VFIO will not with the new no-IOMMU mode
> > in 4.4 kernel. The original code assumed that IOMMU group zero would
> > never be used. Group numbers are assigned starting at zero, and up
> > until now the group numbers came from the hardware which is likely
> > to use group 0 for system devices that are not used with DPDK.
> > 
> > The fix is to allow 0 as a valid group and rearrange code
> > to split the return value from the group value.
> > 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> > Why was this ignored? It was originally sent on 26 Oct 15 back
> > when IOMMU discussion was lively.
> 
> There was no review of this patch.
> The patch has been marked as deferred recently when it was too late
> to do such feature changes in DPDK code:
> 	http://dpdk.org/dev/patchwork/patch/8035/

This is why as a fallback the MAINTAINER has to review the patch
or direct a sub-maintainer to do it. I think almost 2 months is
plenty of time for review. 

Another alternative policy is to have
a "default yes" policy such that if there are no objections or
discussion things that are submitted early just go in (that
is what ZeroMQ does). http://rfc.zeromq.org/spec:22

* Maintainers SHOULD NOT merge their own patches except in exceptional cases, such as non-responsiveness from other Maintainers for an extended period (more than 1-2 days).

* Maintainers SHALL NOT make value judgments on correct patches.
* Maintainers SHALL merge correct patches from other Contributors rapidly.

* Maintainers SHOULD ask for improvements to incorrect patches and SHOULD reject incorrect patches if the Contributor does not respond constructively.
* Any Contributor who has value judgments on a correct patch SHOULD express these via their own patches.
* Maintainers MAY commit changes to non-source documentation directly to the project.

Re: [dpdk-dev] [PATCH] vfio: support iommu group zero

[Thomas Monjalon]

2015-12-09 13:58, Stephen Hemminger:
> On Wed, 09 Dec 2015 22:12:33 +0100
> Thomas Monjalon <thomas.monjalon@6wind.com> wrote:
> 
> > 2015-12-09 09:55, Stephen Hemminger:
> > > The current implementation of VFIO will not with the new no-IOMMU mode
> > > in 4.4 kernel. The original code assumed that IOMMU group zero would
> > > never be used. Group numbers are assigned starting at zero, and up
> > > until now the group numbers came from the hardware which is likely
> > > to use group 0 for system devices that are not used with DPDK.
> > > 
> > > The fix is to allow 0 as a valid group and rearrange code
> > > to split the return value from the group value.
> > > 
> > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > > ---
> > > Why was this ignored? It was originally sent on 26 Oct 15 back
> > > when IOMMU discussion was lively.
> > 
> > There was no review of this patch.
> > The patch has been marked as deferred recently when it was too late
> > to do such feature changes in DPDK code:
> > 	http://dpdk.org/dev/patchwork/patch/8035/
> 
> This is why as a fallback the MAINTAINER has to review the patch
> or direct a sub-maintainer to do it. I think almost 2 months is
> plenty of time for review.

27 October was 3 days before the feature deadline.
And you have not pinged about it since then.
But that's true I have missed the importance of this patch.
Would it help to have it integrated today?
Are you sure it won't break something else?

Re: [dpdk-dev] [PATCH] vfio: support iommu group zero

[Stephen Hemminger]

On Wed, 09 Dec 2015 23:49:59 +0100
Thomas Monjalon <thomas.monjalon@6wind.com> wrote:

> 2015-12-09 13:58, Stephen Hemminger:
> > On Wed, 09 Dec 2015 22:12:33 +0100
> > Thomas Monjalon <thomas.monjalon@6wind.com> wrote:
> > 
> > > 2015-12-09 09:55, Stephen Hemminger:
> > > > The current implementation of VFIO will not with the new no-IOMMU mode
> > > > in 4.4 kernel. The original code assumed that IOMMU group zero would
> > > > never be used. Group numbers are assigned starting at zero, and up
> > > > until now the group numbers came from the hardware which is likely
> > > > to use group 0 for system devices that are not used with DPDK.
> > > > 
> > > > The fix is to allow 0 as a valid group and rearrange code
> > > > to split the return value from the group value.
> > > > 
> > > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > > > ---
> > > > Why was this ignored? It was originally sent on 26 Oct 15 back
> > > > when IOMMU discussion was lively.
> > > 
> > > There was no review of this patch.
> > > The patch has been marked as deferred recently when it was too late
> > > to do such feature changes in DPDK code:
> > > 	http://dpdk.org/dev/patchwork/patch/8035/
> > 
> > This is why as a fallback the MAINTAINER has to review the patch
> > or direct a sub-maintainer to do it. I think almost 2 months is
> > plenty of time for review.
> 
> 27 October was 3 days before the feature deadline.
> And you have not pinged about it since then.
> But that's true I have missed the importance of this patch.
> Would it help to have it integrated today?
> Are you sure it won't break something else?

Could the original VFIO submitter from Intel review it.

Re: [dpdk-dev] [PATCH] vfio: support iommu group zero

[Alex Williamson]

On Wed, 2015-12-09 at 15:12 -0800, Stephen Hemminger wrote:
> On Wed, 09 Dec 2015 23:49:59 +0100
> Thomas Monjalon <thomas.monjalon@6wind.com> wrote:
> 
> > 2015-12-09 13:58, Stephen Hemminger:
> > > On Wed, 09 Dec 2015 22:12:33 +0100
> > > Thomas Monjalon <thomas.monjalon@6wind.com> wrote:
> > > 
> > > > 2015-12-09 09:55, Stephen Hemminger:
> > > > > The current implementation of VFIO will not with the new no-IOMMU mode
> > > > > in 4.4 kernel. The original code assumed that IOMMU group zero would
> > > > > never be used. Group numbers are assigned starting at zero, and up
> > > > > until now the group numbers came from the hardware which is likely
> > > > > to use group 0 for system devices that are not used with DPDK.
> > > > > 
> > > > > The fix is to allow 0 as a valid group and rearrange code
> > > > > to split the return value from the group value.
> > > > > 
> > > > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > > > > ---
> > > > > Why was this ignored? It was originally sent on 26 Oct 15 back
> > > > > when IOMMU discussion was lively.
> > > > 
> > > > There was no review of this patch.
> > > > The patch has been marked as deferred recently when it was too late
> > > > to do such feature changes in DPDK code:
> > > > 	http://dpdk.org/dev/patchwork/patch/8035/
> > > 
> > > This is why as a fallback the MAINTAINER has to review the patch
> > > or direct a sub-maintainer to do it. I think almost 2 months is
> > > plenty of time for review.
> > 
> > 27 October was 3 days before the feature deadline.
> > And you have not pinged about it since then.
> > But that's true I have missed the importance of this patch.
> > Would it help to have it integrated today?
> > Are you sure it won't break something else?
> 
> Could the original VFIO submitter from Intel review it.

vfio group 0 has always been valid, but it's unlikely that you'd ever
hit it in regular usage since it will typically be the root bus device.
It's only with no-iommu mode in vfio that it's common, but that's
getting reverted for Linux v4.4, so that may change your priorities
about squeezing this in at the last minute.  Thanks,

Alex

Re: [dpdk-dev] [PATCH] vfio: support iommu group zero

[Stephen Hemminger]

On Wed, 09 Dec 2015 16:22:14 -0700
Alex Williamson <alex.williamson@redhat.com> wrote:

> On Wed, 2015-12-09 at 15:12 -0800, Stephen Hemminger wrote:
> > On Wed, 09 Dec 2015 23:49:59 +0100
> > Thomas Monjalon <thomas.monjalon@6wind.com> wrote:
> > 
> > > 2015-12-09 13:58, Stephen Hemminger:
> > > > On Wed, 09 Dec 2015 22:12:33 +0100
> > > > Thomas Monjalon <thomas.monjalon@6wind.com> wrote:
> > > > 
> > > > > 2015-12-09 09:55, Stephen Hemminger:
> > > > > > The current implementation of VFIO will not with the new no-IOMMU mode
> > > > > > in 4.4 kernel. The original code assumed that IOMMU group zero would
> > > > > > never be used. Group numbers are assigned starting at zero, and up
> > > > > > until now the group numbers came from the hardware which is likely
> > > > > > to use group 0 for system devices that are not used with DPDK.
> > > > > > 
> > > > > > The fix is to allow 0 as a valid group and rearrange code
> > > > > > to split the return value from the group value.
> > > > > > 
> > > > > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > > > > > ---
> > > > > > Why was this ignored? It was originally sent on 26 Oct 15 back
> > > > > > when IOMMU discussion was lively.
> > > > > 
> > > > > There was no review of this patch.
> > > > > The patch has been marked as deferred recently when it was too late
> > > > > to do such feature changes in DPDK code:
> > > > > 	http://dpdk.org/dev/patchwork/patch/8035/
> > > > 
> > > > This is why as a fallback the MAINTAINER has to review the patch
> > > > or direct a sub-maintainer to do it. I think almost 2 months is
> > > > plenty of time for review.
> > > 
> > > 27 October was 3 days before the feature deadline.
> > > And you have not pinged about it since then.
> > > But that's true I have missed the importance of this patch.
> > > Would it help to have it integrated today?
> > > Are you sure it won't break something else?
> > 
> > Could the original VFIO submitter from Intel review it.
> 
> vfio group 0 has always been valid, but it's unlikely that you'd ever
> hit it in regular usage since it will typically be the root bus device.
> It's only with no-iommu mode in vfio that it's common, but that's
> getting reverted for Linux v4.4, so that may change your priorities
> about squeezing this in at the last minute.  Thanks,

Why, who objected? It was useful and working fine as far as I tested.
Really wanted to get to the per-queue stuff, but that was harder to
got working (even on regular IOMMU).

Re: [dpdk-dev] [PATCH] vfio: support iommu group zero

[Alex Williamson]

On Wed, 2015-12-09 at 16:52 -0800, Stephen Hemminger wrote:
> On Wed, 09 Dec 2015 16:22:14 -0700
> Alex Williamson <alex.williamson@redhat.com> wrote:
> 
> > On Wed, 2015-12-09 at 15:12 -0800, Stephen Hemminger wrote:
> > > On Wed, 09 Dec 2015 23:49:59 +0100
> > > Thomas Monjalon <thomas.monjalon@6wind.com> wrote:
> > > 
> > > > 2015-12-09 13:58, Stephen Hemminger:
> > > > > On Wed, 09 Dec 2015 22:12:33 +0100
> > > > > Thomas Monjalon <thomas.monjalon@6wind.com> wrote:
> > > > > 
> > > > > > 2015-12-09 09:55, Stephen Hemminger:
> > > > > > > The current implementation of VFIO will not with the new no-IOMMU mode
> > > > > > > in 4.4 kernel. The original code assumed that IOMMU group zero would
> > > > > > > never be used. Group numbers are assigned starting at zero, and up
> > > > > > > until now the group numbers came from the hardware which is likely
> > > > > > > to use group 0 for system devices that are not used with DPDK.
> > > > > > > 
> > > > > > > The fix is to allow 0 as a valid group and rearrange code
> > > > > > > to split the return value from the group value.
> > > > > > > 
> > > > > > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > > > > > > ---
> > > > > > > Why was this ignored? It was originally sent on 26 Oct 15 back
> > > > > > > when IOMMU discussion was lively.
> > > > > > 
> > > > > > There was no review of this patch.
> > > > > > The patch has been marked as deferred recently when it was too late
> > > > > > to do such feature changes in DPDK code:
> > > > > > 	http://dpdk.org/dev/patchwork/patch/8035/
> > > > > 
> > > > > This is why as a fallback the MAINTAINER has to review the patch
> > > > > or direct a sub-maintainer to do it. I think almost 2 months is
> > > > > plenty of time for review.
> > > > 
> > > > 27 October was 3 days before the feature deadline.
> > > > And you have not pinged about it since then.
> > > > But that's true I have missed the importance of this patch.
> > > > Would it help to have it integrated today?
> > > > Are you sure it won't break something else?
> > > 
> > > Could the original VFIO submitter from Intel review it.
> > 
> > vfio group 0 has always been valid, but it's unlikely that you'd ever
> > hit it in regular usage since it will typically be the root bus device.
> > It's only with no-iommu mode in vfio that it's common, but that's
> > getting reverted for Linux v4.4, so that may change your priorities
> > about squeezing this in at the last minute.  Thanks,
> 
> Why, who objected? It was useful and working fine as far as I tested.
> Really wanted to get to the per-queue stuff, but that was harder to
> got working (even on regular IOMMU).

I objected because nobody has spoken up for the month that I've been
asking to be notified if a user has been tested.  You can ask for it
again in v4.5 with evidence showing that it works and meets your needs.

Re: [dpdk-dev] [PATCH] vfio: support iommu group zero

[Burakov, Anatoly]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen
> Hemminger
> Sent: Wednesday, December 9, 2015 5:56 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] vfio: support iommu group zero
> 
> The current implementation of VFIO will not with the new no-IOMMU mode
> in 4.4 kernel. The original code assumed that IOMMU group zero would
> never be used. Group numbers are assigned starting at zero, and up until
> now the group numbers came from the hardware which is likely to use group
> 0 for system devices that are not used with DPDK.
> 
> The fix is to allow 0 as a valid group and rearrange code to split the return
> value from the group value.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> Why was this ignored? It was originally sent on 26 Oct 15 back when IOMMU
> discussion was lively.
> 
>  lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
> b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
> index 0e6c48a..74f91ba 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
> @@ -499,14 +499,15 @@ pci_vfio_get_group_fd(int iommu_group_no)  }
> 
>  /* parse IOMMU group number for a PCI device
> - * returns -1 for errors, 0 for non-existent group */
> + * returns 1 on success, -1 for errors, 0 for non-existent group */
>  static int
> -pci_vfio_get_group_no(const char *pci_addr)
> +pci_vfio_get_group_no(const char *pci_addr, int *iommu_group_no)
>  {
>  	char linkname[PATH_MAX];
>  	char filename[PATH_MAX];
>  	char *tok[16], *group_tok, *end;
> -	int ret, iommu_group_no;
> +	int ret;
> 
>  	memset(linkname, 0, sizeof(linkname));
>  	memset(filename, 0, sizeof(filename)); @@ -533,13 +534,13 @@
> pci_vfio_get_group_no(const char *pci_addr)
>  	errno = 0;
>  	group_tok = tok[ret - 1];
>  	end = group_tok;
> -	iommu_group_no = strtol(group_tok, &end, 10);
> +	*iommu_group_no = strtol(group_tok, &end, 10);
>  	if ((end != group_tok && *end != '\0') || errno != 0) {
>  		RTE_LOG(ERR, EAL, "  %s error parsing IOMMU number!\n",
> pci_addr);
>  		return -1;
>  	}
> 
> -	return iommu_group_no;
> +	return 1;
>  }
> 
>  static void
> @@ -581,16 +582,15 @@ pci_vfio_map_resource(struct rte_pci_device
> *dev)
>  			loc->domain, loc->bus, loc->devid, loc->function);
> 
>  	/* get group number */
> -	iommu_group_no = pci_vfio_get_group_no(pci_addr);
> -
> -	/* if 0, group doesn't exist */
> -	if (iommu_group_no == 0) {
> +	ret = pci_vfio_get_group_no(pci_addr, &iommu_group_no);
> +	if (ret == 0) {
>  		RTE_LOG(WARNING, EAL, "  %s not managed by VFIO driver,
> skipping\n",
> -				pci_addr);
> +			pci_addr);
>  		return 1;
>  	}
> +
>  	/* if negative, something failed */
> -	else if (iommu_group_no < 0)
> +	if (ret < 0)
>  		return -1;
> 
>  	/* get the actual group fd */
> --
> 2.1.4

Sorry, slipped through the cracks somehow. I see no obvious issues with the patch, however I've never encountered a group 0 while testing VFIO. As per Alex, that would be more used in the no-iommu scenario, and this break doesn't appear to affect anything else even without no-iommu functionality, so...

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

Re: [dpdk-dev] [PATCH] vfio: support iommu group zero

[Thomas Monjalon]

> > The current implementation of VFIO will not with the new no-IOMMU mode
> > in 4.4 kernel. The original code assumed that IOMMU group zero would
> > never be used. Group numbers are assigned starting at zero, and up until
> > now the group numbers came from the hardware which is likely to use group
> > 0 for system devices that are not used with DPDK.
> > 
> > The fix is to allow 0 as a valid group and rearrange code to split the return
> > value from the group value.
> > 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> 
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

Applied, thanks

Stephen, please keep us updated about VFIO no-iommu.

[dpdk-dev] [PATCH] vfio: support iommu group zero

[Stephen Hemminger]

From: Stephen Hemminger <stephen@networkplumber.org>

The implementation of VFIO is broken on some platforms and if using
the proposed VFIO without IOMMU patch.  IOMMU group zero is a valid value.

Change code to split the return value from the group value.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
index 0e6c48a..74f91ba 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
@@ -499,14 +499,15 @@ pci_vfio_get_group_fd(int iommu_group_no)
 }
 
 /* parse IOMMU group number for a PCI device
- * returns -1 for errors, 0 for non-existent group */
+ * returns 1 on success, -1 for errors, 0 for non-existent group
+ */
 static int
-pci_vfio_get_group_no(const char *pci_addr)
+pci_vfio_get_group_no(const char *pci_addr, int *iommu_group_no)
 {
 	char linkname[PATH_MAX];
 	char filename[PATH_MAX];
 	char *tok[16], *group_tok, *end;
-	int ret, iommu_group_no;
+	int ret;
 
 	memset(linkname, 0, sizeof(linkname));
 	memset(filename, 0, sizeof(filename));
@@ -533,13 +534,13 @@ pci_vfio_get_group_no(const char *pci_addr)
 	errno = 0;
 	group_tok = tok[ret - 1];
 	end = group_tok;
-	iommu_group_no = strtol(group_tok, &end, 10);
+	*iommu_group_no = strtol(group_tok, &end, 10);
 	if ((end != group_tok && *end != '\0') || errno != 0) {
 		RTE_LOG(ERR, EAL, "  %s error parsing IOMMU number!\n", pci_addr);
 		return -1;
 	}
 
-	return iommu_group_no;
+	return 1;
 }
 
 static void
@@ -581,16 +582,15 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
 			loc->domain, loc->bus, loc->devid, loc->function);
 
 	/* get group number */
-	iommu_group_no = pci_vfio_get_group_no(pci_addr);
-
-	/* if 0, group doesn't exist */
-	if (iommu_group_no == 0) {
+	ret = pci_vfio_get_group_no(pci_addr, &iommu_group_no);
+	if (ret == 0) {
 		RTE_LOG(WARNING, EAL, "  %s not managed by VFIO driver, skipping\n",
-				pci_addr);
+			pci_addr);
 		return 1;
 	}
+
 	/* if negative, something failed */
-	else if (iommu_group_no < 0)
+	if (ret < 0)
 		return -1;
 
 	/* get the actual group fd */
-- 
2.1.4