[PATCH] vfio: open VFIO container at startup rather than during init

[PATCH] vfio: open VFIO container at startup rather than during init

[Anatoly Burakov]

Signed-off-by: Anatoly Burakov <anatoly.burakov-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
index 4de6061..9eb5dcd 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
@@ -523,17 +523,6 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
 	rte_snprintf(pci_addr, sizeof(pci_addr), PCI_PRI_FMT,
 			loc->domain, loc->bus, loc->devid, loc->function);
 
-	/* get container fd (needs to be done only once per initialization) */
-	if (vfio_cfg.vfio_container_fd == -1) {
-		int vfio_container_fd = pci_vfio_get_container_fd();
-		if (vfio_container_fd < 0) {
-			RTE_LOG(ERR, EAL, "  %s cannot open VFIO container!\n", pci_addr);
-			return -1;
-		}
-
-		vfio_cfg.vfio_container_fd = vfio_container_fd;
-	}
-
 	/* get group number */
 	iommu_group_no = pci_vfio_get_group_no(pci_addr);
 
@@ -770,10 +759,10 @@ pci_vfio_enable(void)
 		vfio_cfg.vfio_groups[i].fd = -1;
 		vfio_cfg.vfio_groups[i].group_no = -1;
 	}
-	vfio_cfg.vfio_container_fd = -1;
+	vfio_cfg.vfio_container_fd = pci_vfio_get_container_fd();
 
 	/* check if we have VFIO driver enabled */
-	if (access(VFIO_DIR, F_OK) == 0)
+	if (vfio_cfg.vfio_container_fd != -1)
 		vfio_cfg.vfio_enabled = 1;
 	else
 		RTE_LOG(INFO, EAL, "VFIO driver not loaded or wrong permissions\n");
-- 
1.8.1.4

Re: [PATCH] vfio: open VFIO container at startup rather than during init

[Richardson, Bruce]

> -----Original Message-----
> From: Burakov, Anatoly
> Sent: Tuesday, June 17, 2014 2:12 AM
> To: dev-VfR2kkLFssw@public.gmane.org
> Cc: Richardson, Bruce
> Subject: [PATCH] vfio: open VFIO container at startup rather than during init
> 
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---

This seems to fix the issue I was having. \o/

Acked-by: Bruce Richardson <bruce.richardson-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Re: [PATCH] vfio: open VFIO container at startup rather than during init

[Thomas Monjalon]

> Signed-off-by: Anatoly Burakov <anatoly.burakov-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Please Anatoly, could you provide a text explaining what was broken
and why you fixed it this way?

Thanks
-- 
Thomas

Re: [PATCH] vfio: open VFIO container at startup rather than during init

[Burakov, Anatoly]

Hi Thomas,

> Subject: Re: [dpdk-dev] [PATCH] vfio: open VFIO container at startup rather
> than during init
> 
> > Signed-off-by: Anatoly Burakov <anatoly.burakov-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> Please Anatoly, could you provide a text explaining what was broken and
> why you fixed it this way?

What was broken was if, for some reason, VFIO is loaded but the user can't initialize it (an example would be wrong permissions, or unsupported IOMMU type, which is what Bruce seems to be having... which shouldn't happen as far as I know, but there's nothing I can do on DPDK's side to fix this as this is the kernel reporting wrong kind of IOMMU type), DPDK would fail to load. The fix makes DPDK simply not try VFIO support at all if the container cannot be opened for some reason.

Best regards,
Anatoly Burakov
DPDK SW Engineer

Re: [PATCH] vfio: open VFIO container at startup rather than during init

[Dumitrescu, Cristian]

Hi Anatoly,

I would suggest we add a log message explaining which mechanism is loaded (igb_uio/vfio) and why (e.g. tried vfio first but container could not be opened, so falling back to igb_uio, etc).

Regards,
Cristian

-----Original Message-----
From: dev [mailto:dev-bounces-VfR2kkLFssw@public.gmane.org] On Behalf Of Burakov, Anatoly
Sent: Wednesday, June 18, 2014 9:57 AM
To: Thomas Monjalon
Cc: dev-VfR2kkLFssw@public.gmane.org
Subject: Re: [dpdk-dev] [PATCH] vfio: open VFIO container at startup rather than during init

Hi Thomas,

> Subject: Re: [dpdk-dev] [PATCH] vfio: open VFIO container at startup rather
> than during init
> 
> > Signed-off-by: Anatoly Burakov <anatoly.burakov-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> Please Anatoly, could you provide a text explaining what was broken and
> why you fixed it this way?

What was broken was if, for some reason, VFIO is loaded but the user can't initialize it (an example would be wrong permissions, or unsupported IOMMU type, which is what Bruce seems to be having... which shouldn't happen as far as I know, but there's nothing I can do on DPDK's side to fix this as this is the kernel reporting wrong kind of IOMMU type), DPDK would fail to load. The fix makes DPDK simply not try VFIO support at all if the container cannot be opened for some reason.

Best regards,
Anatoly Burakov
DPDK SW Engineer



--------------------------------------------------------------
Intel Shannon Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263
Business address: Dromore House, East Park, Shannon, Co. Clare

This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.

Re: [PATCH] vfio: open VFIO container at startup rather than during init

[Burakov, Anatoly]

Hi Cristian,

> I would suggest we add a log message explaining which mechanism is loaded
> (igb_uio/vfio) and why (e.g. tried vfio first but container could not be
> opened, so falling back to igb_uio, etc).

This already happens.

If the container could not be loaded for whatever reason, the log message is displayed (it did before, but the previous code didn't account for situations such as Bruce was having, hence the patch). If VFIO is loaded and enabled, all drivers will then report either being bound or skipped (e.g. "not bound to VFIO, skipping").

Best regards,
Anatoly Burakov
DPDK SW Engineer

Re: [PATCH] vfio: open VFIO container at startup rather than during init

[Neil Horman]

On Wed, Jun 18, 2014 at 10:26:08AM +0000, Burakov, Anatoly wrote:
> Hi Cristian,
> 
> > I would suggest we add a log message explaining which mechanism is loaded
> > (igb_uio/vfio) and why (e.g. tried vfio first but container could not be
> > opened, so falling back to igb_uio, etc).
> 
> This already happens.
> 
> If the container could not be loaded for whatever reason, the log message is displayed (it did before, but the previous code didn't account for situations such as Bruce was having, hence the patch). If VFIO is loaded and enabled, all drivers will then report either being bound or skipped (e.g. "not bound to VFIO, skipping").

I think what Thomas wants is for you to resend the patch with a proper changelog
entry added to it, so the commit has an explination of what was changed and, for
posterity.
Neil

> 
> Best regards,
> Anatoly Burakov
> DPDK SW Engineer
> 
> 
> 
> 

Re: [PATCH] vfio: open VFIO container at startup rather than during init

[Burakov, Anatoly]

Hi Neil

> I think what Thomas wants is for you to resend the patch with a proper
> changelog entry added to it, so the commit has an explination of what was
> changed and, for posterity.

Got it. Can I also incorporate your changes to error codes as well?

Best regards,
Anatoly Burakov
DPDK SW Engineer

Re: [PATCH] vfio: open VFIO container at startup rather than during init

[Neil Horman]

On Wed, Jun 18, 2014 at 11:02:23AM +0000, Burakov, Anatoly wrote:
> Hi Neil
> 
> > I think what Thomas wants is for you to resend the patch with a proper
> > changelog entry added to it, so the commit has an explination of what was
> > changed and, for posterity.
> 
> Got it. Can I also incorporate your changes to error codes as well?
> 
If you want to take Bruce's suggestions and incoproate them as well from my
thread below, sure, as long as Thomas is ok with it.
Neil

> Best regards,
> Anatoly Burakov
> DPDK SW Engineer
> 

[PATCH v2 0/2] Fix issues with VFIO

[Anatoly Burakov]

This patchset fixes an issue with VFIO where DPDK initialization could
fail even if the user didn't want to use VFIO in the first place. Also,
more verbose and descriptive error messages were added to VFIO code, for
example distinguishing between a failed ioctl() call and an unsupported
VFIO API version.

Anatoly Burakov (2):
  vfio: open VFIO container at startup rather than during init
  vfio: more verbose error messages

 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 63 ++++++++++++++++--------------
 1 file changed, 34 insertions(+), 29 deletions(-)

-- 
1.8.1.4

Re: [PATCH v3 0/2] Fix issues with VFIO

[Richardson, Bruce]

> -----Original Message-----
> From: Burakov, Anatoly
> Sent: Wednesday, June 18, 2014 8:07 AM
> To: dev-VfR2kkLFssw@public.gmane.org
> Cc: Richardson, Bruce; nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org
> Subject: [PATCH v3 0/2] Fix issues with VFIO
> 
> This patchset fixes an issue with VFIO where DPDK initialization could
> fail even if the user didn't want to use VFIO in the first place. Also,
> more verbose and descriptive error messages were added to VFIO code, for
> example distinguishing between a failed ioctl() call and an unsupported
> VFIO API version.
> 
> Anatoly Burakov (2):
>   vfio: open VFIO container at startup rather than during init
>   vfio: more verbose error messages
> 
>  lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 63 ++++++++++++++++--------------
>  1 file changed, 34 insertions(+), 29 deletions(-)
> 
> --

Both patches work fine for me. Thanks Anatoly and Neil!

Acked-by: Bruce Richardson <bruce.richardson-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Re: [PATCH v3 0/2] Fix issues with VFIO

[Thomas Monjalon]

> > This patchset fixes an issue with VFIO where DPDK initialization could
> > fail even if the user didn't want to use VFIO in the first place. Also,
> > more verbose and descriptive error messages were added to VFIO code, for
> > example distinguishing between a failed ioctl() call and an unsupported
> > VFIO API version.
> > 
> > Anatoly Burakov (2):
> >   vfio: open VFIO container at startup rather than during init
> >   vfio: more verbose error messages
> 
> Both patches work fine for me. Thanks Anatoly and Neil!
> 
> Acked-by: Bruce Richardson <bruce.richardson-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Acked-by: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>

Applied for version 1.7.0.

Thanks
-- 
Thomas

Re: [PATCH v3 0/2] Fix issues with VFIO

[Neil Horman]

On Wed, Jun 18, 2014 at 04:07:15PM +0100, Anatoly Burakov wrote:
> This patchset fixes an issue with VFIO where DPDK initialization could
> fail even if the user didn't want to use VFIO in the first place. Also,
> more verbose and descriptive error messages were added to VFIO code, for
> example distinguishing between a failed ioctl() call and an unsupported
> VFIO API version.
> 
> Anatoly Burakov (2):
>   vfio: open VFIO container at startup rather than during init
>   vfio: more verbose error messages
> 
>  lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 63 ++++++++++++++++--------------
>  1 file changed, 34 insertions(+), 29 deletions(-)
> 
> -- 
> 1.8.1.4
> 
> 

Series
Acked-by: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
Thanks Anatoly!
Neil

[PATCH v2 1/2] vfio: open VFIO container at startup rather than during init

[Anatoly Burakov]

Currently, VFIO only checks for being able to access the /dev/vfio
directory when initializing VFIO, deferring actual VFIO container
initialization to VFIO binding code. This doesn't bode well for when
VFIO container cannot be initialized for whatever reason, because
it results in unrecoverable error even if the user didn't set up
VFIO and didn't even want to use it in the first place.

This patch fixes this by moving container initialization into the
code that checks if VFIO is available at runtime. Therefore, any
issues with the container will be known at initialization stage and
VFIO will simply be turned off if container could not be set up.

Signed-off-by: Anatoly Burakov <anatoly.burakov-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Acked-by: Bruce Richardson <bruce.richardson-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
index 4de6061..9eb5dcd 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
@@ -523,17 +523,6 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
 	rte_snprintf(pci_addr, sizeof(pci_addr), PCI_PRI_FMT,
 			loc->domain, loc->bus, loc->devid, loc->function);
 
-	/* get container fd (needs to be done only once per initialization) */
-	if (vfio_cfg.vfio_container_fd == -1) {
-		int vfio_container_fd = pci_vfio_get_container_fd();
-		if (vfio_container_fd < 0) {
-			RTE_LOG(ERR, EAL, "  %s cannot open VFIO container!\n", pci_addr);
-			return -1;
-		}
-
-		vfio_cfg.vfio_container_fd = vfio_container_fd;
-	}
-
 	/* get group number */
 	iommu_group_no = pci_vfio_get_group_no(pci_addr);
 
@@ -770,10 +759,10 @@ pci_vfio_enable(void)
 		vfio_cfg.vfio_groups[i].fd = -1;
 		vfio_cfg.vfio_groups[i].group_no = -1;
 	}
-	vfio_cfg.vfio_container_fd = -1;
+	vfio_cfg.vfio_container_fd = pci_vfio_get_container_fd();
 
 	/* check if we have VFIO driver enabled */
-	if (access(VFIO_DIR, F_OK) == 0)
+	if (vfio_cfg.vfio_container_fd != -1)
 		vfio_cfg.vfio_enabled = 1;
 	else
 		RTE_LOG(INFO, EAL, "VFIO driver not loaded or wrong permissions\n");
-- 
1.8.1.4

[PATCH v2 2/2] vfio: more verbose error messages

[Anatoly Burakov]

Signed-off-by: Anatoly Burakov <anatoly.burakov-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
CC: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
---
 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 48 ++++++++++++++++++++----------
 1 file changed, 32 insertions(+), 16 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
index 9eb5dcd..bf765b5 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
@@ -180,7 +180,8 @@ pci_vfio_setup_dma_maps(int vfio_container_fd)
 	ret = ioctl(vfio_container_fd, VFIO_SET_IOMMU,
 			VFIO_TYPE1_IOMMU);
 	if (ret) {
-		RTE_LOG(ERR, EAL, "  cannot set IOMMU type!\n");
+		RTE_LOG(ERR, EAL, "  cannot set IOMMU type, "
+				"error %i (%s)\n", errno, strerror(errno));
 		return -1;
 	}
 
@@ -201,7 +202,8 @@ pci_vfio_setup_dma_maps(int vfio_container_fd)
 		ret = ioctl(vfio_container_fd, VFIO_IOMMU_MAP_DMA, &dma_map);
 
 		if (ret) {
-			RTE_LOG(ERR, EAL, "  cannot set up DMA remapping!\n");
+			RTE_LOG(ERR, EAL, "  cannot set up DMA remapping, "
+					"error %i (%s)\n", errno, strerror(errno));
 			return -1;
 		}
 	}
@@ -253,7 +255,8 @@ pci_vfio_setup_interrupts(struct rte_pci_device *dev, int vfio_dev_fd)
 
 		ret = ioctl(vfio_dev_fd, VFIO_DEVICE_GET_IRQ_INFO, &irq);
 		if (ret < 0) {
-			RTE_LOG(ERR, EAL, "  cannot get IRQ info!\n");
+			RTE_LOG(ERR, EAL, "  cannot get IRQ info, "
+					"error %i (%s)\n", errno, strerror(errno));
 			return -1;
 		}
 
@@ -271,7 +274,8 @@ pci_vfio_setup_interrupts(struct rte_pci_device *dev, int vfio_dev_fd)
 		/* set up an eventfd for interrupts */
 		fd = eventfd(0, 0);
 		if (fd < 0) {
-			RTE_LOG(ERR, EAL, "  cannot set up eventfd!\n");
+			RTE_LOG(ERR, EAL, "  cannot set up eventfd, "
+					"error %i (%s)\n", errno, strerror(errno));
 			return -1;
 		}
 
@@ -313,22 +317,31 @@ pci_vfio_get_container_fd(void)
 	if (internal_config.process_type == RTE_PROC_PRIMARY) {
 		vfio_container_fd = open(VFIO_CONTAINER_PATH, O_RDWR);
 		if (vfio_container_fd < 0) {
-			RTE_LOG(ERR, EAL, "  cannot open VFIO container!\n");
+			RTE_LOG(ERR, EAL, "  cannot open VFIO container, "
+					"error %i (%s)\n", errno, strerror(errno));
 			return -1;
 		}
 
 		/* check VFIO API version */
 		ret = ioctl(vfio_container_fd, VFIO_GET_API_VERSION);
 		if (ret != VFIO_API_VERSION) {
-			RTE_LOG(ERR, EAL, "  unknown VFIO API version!\n");
+			if (ret < 0)
+				RTE_LOG(ERR, EAL, "  could not get VFIO API version, "
+						"error %i (%s)\n", errno, strerror(errno));
+			else
+				RTE_LOG(ERR, EAL, "  unsupported VFIO API version!\n");
 			close(vfio_container_fd);
 			return -1;
 		}
 
 		/* check if we support IOMMU type 1 */
 		ret = ioctl(vfio_container_fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1_IOMMU);
-		if (!ret) {
-			RTE_LOG(ERR, EAL, "  unknown IOMMU driver!\n");
+		if (ret != 1) {
+			if (ret < 0)
+				RTE_LOG(ERR, EAL, "  could not get IOMMU type, "
+						"error %i (%s)\n", errno, strerror(errno));
+			else
+				RTE_LOG(ERR, EAL, "  unsupported IOMMU type!\n");
 			close(vfio_container_fd);
 			return -1;
 		}
@@ -564,7 +577,8 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
 	/* check if the group is viable */
 	ret = ioctl(vfio_group_fd, VFIO_GROUP_GET_STATUS, &group_status);
 	if (ret) {
-		RTE_LOG(ERR, EAL, "  %s cannot get group status!\n", pci_addr);
+		RTE_LOG(ERR, EAL, "  %s cannot get group status, "
+				"error %i (%s)\n", pci_addr, errno, strerror(errno));
 		close(vfio_group_fd);
 		clear_current_group();
 		return -1;
@@ -587,8 +601,8 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
 		ret = ioctl(vfio_group_fd, VFIO_GROUP_SET_CONTAINER,
 				&vfio_cfg.vfio_container_fd);
 		if (ret) {
-			RTE_LOG(ERR, EAL, "  %s cannot add VFIO group to container!\n",
-					pci_addr);
+			RTE_LOG(ERR, EAL, "  %s cannot add VFIO group to container, "
+					"error %i (%s)\n", pci_addr, errno, strerror(errno));
 			close(vfio_group_fd);
 			clear_current_group();
 			return -1;
@@ -611,7 +625,8 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
 			vfio_cfg.vfio_container_has_dma == 0) {
 		ret = pci_vfio_setup_dma_maps(vfio_cfg.vfio_container_fd);
 		if (ret) {
-			RTE_LOG(ERR, EAL, "  %s DMA remapping failed!\n", pci_addr);
+			RTE_LOG(ERR, EAL, "  %s DMA remapping failed, "
+					"error %i (%s)\n", pci_addr, errno, strerror(errno));
 			return -1;
 		}
 		vfio_cfg.vfio_container_has_dma = 1;
@@ -631,7 +646,8 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
 	/* test and setup the device */
 	ret = ioctl(vfio_dev_fd, VFIO_DEVICE_GET_INFO, &device_info);
 	if (ret) {
-		RTE_LOG(ERR, EAL, "  %s cannot get device info!\n", pci_addr);
+		RTE_LOG(ERR, EAL, "  %s cannot get device info, "
+				"error %i (%s)\n", pci_addr, errno, strerror(errno));
 		close(vfio_dev_fd);
 		return -1;
 	}
@@ -688,8 +704,8 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
 		ret = ioctl(vfio_dev_fd, VFIO_DEVICE_GET_REGION_INFO, &reg);
 
 		if (ret) {
-			RTE_LOG(ERR, EAL, "  %s cannot get device region info!\n",
-					pci_addr);
+			RTE_LOG(ERR, EAL, "  %s cannot get device region info "
+					"error %i (%s)\n", pci_addr, errno, strerror(errno));
 			close(vfio_dev_fd);
 			if (internal_config.process_type == RTE_PROC_PRIMARY)
 				rte_free(vfio_res);
@@ -765,7 +781,7 @@ pci_vfio_enable(void)
 	if (vfio_cfg.vfio_container_fd != -1)
 		vfio_cfg.vfio_enabled = 1;
 	else
-		RTE_LOG(INFO, EAL, "VFIO driver not loaded or wrong permissions\n");
+		RTE_LOG(INFO, EAL, "VFIO support could not be initialized\n");
 
 	return 0;
 }
-- 
1.8.1.4

Re: [PATCH v2 0/2] Fix issues with VFIO

[Neil Horman]

On Wed, Jun 18, 2014 at 02:07:17PM +0100, Anatoly Burakov wrote:
> This patchset fixes an issue with VFIO where DPDK initialization could
> fail even if the user didn't want to use VFIO in the first place. Also,
> more verbose and descriptive error messages were added to VFIO code, for
> example distinguishing between a failed ioctl() call and an unsupported
> VFIO API version.
> 
> Anatoly Burakov (2):
>   vfio: open VFIO container at startup rather than during init
>   vfio: more verbose error messages
> 

You still need a changelog entry with each patch (the cover letter doesn't get
included with the patches in git).  You also probably want to mention that the
second patch also fixes a case in which syscall errors are erroneously treated
as a success case

Neil

>  lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 63 ++++++++++++++++--------------
>  1 file changed, 34 insertions(+), 29 deletions(-)
> 
> -- 
> 1.8.1.4
> 
> 

[PATCH v3 0/2] Fix issues with VFIO

[Anatoly Burakov]

This patchset fixes an issue with VFIO where DPDK initialization could
fail even if the user didn't want to use VFIO in the first place. Also,
more verbose and descriptive error messages were added to VFIO code, for
example distinguishing between a failed ioctl() call and an unsupported
VFIO API version.

Anatoly Burakov (2):
  vfio: open VFIO container at startup rather than during init
  vfio: more verbose error messages

 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 63 ++++++++++++++++--------------
 1 file changed, 34 insertions(+), 29 deletions(-)

-- 
1.8.1.4

[PATCH v3 1/2] vfio: open VFIO container at startup rather than during init

[Anatoly Burakov]

Currently, VFIO only checks for being able to access the /dev/vfio
directory when initializing VFIO, deferring actual VFIO container
initialization to VFIO binding code. This doesn't bode well for when
VFIO container cannot be initialized for whatever reason, because
it results in unrecoverable error even if the user didn't set up
VFIO and didn't even want to use it in the first place.

This patch fixes this by moving container initialization into the
code that checks if VFIO is available at runtime. Therefore, any
issues with the container will be known at initialization stage and
VFIO will simply be turned off if container could not be set up.

Signed-off-by: Anatoly Burakov <anatoly.burakov-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
index 4de6061..9eb5dcd 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
@@ -523,17 +523,6 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
 	rte_snprintf(pci_addr, sizeof(pci_addr), PCI_PRI_FMT,
 			loc->domain, loc->bus, loc->devid, loc->function);
 
-	/* get container fd (needs to be done only once per initialization) */
-	if (vfio_cfg.vfio_container_fd == -1) {
-		int vfio_container_fd = pci_vfio_get_container_fd();
-		if (vfio_container_fd < 0) {
-			RTE_LOG(ERR, EAL, "  %s cannot open VFIO container!\n", pci_addr);
-			return -1;
-		}
-
-		vfio_cfg.vfio_container_fd = vfio_container_fd;
-	}
-
 	/* get group number */
 	iommu_group_no = pci_vfio_get_group_no(pci_addr);
 
@@ -770,10 +759,10 @@ pci_vfio_enable(void)
 		vfio_cfg.vfio_groups[i].fd = -1;
 		vfio_cfg.vfio_groups[i].group_no = -1;
 	}
-	vfio_cfg.vfio_container_fd = -1;
+	vfio_cfg.vfio_container_fd = pci_vfio_get_container_fd();
 
 	/* check if we have VFIO driver enabled */
-	if (access(VFIO_DIR, F_OK) == 0)
+	if (vfio_cfg.vfio_container_fd != -1)
 		vfio_cfg.vfio_enabled = 1;
 	else
 		RTE_LOG(INFO, EAL, "VFIO driver not loaded or wrong permissions\n");
-- 
1.8.1.4

[PATCH v3 2/2] vfio: more verbose error messages

[Anatoly Burakov]

also, making VFIO code distinguish between actual unexpected values
and ioctl() failures, providing appropriate error messages.

Signed-off-by: Anatoly Burakov <anatoly.burakov-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 48 ++++++++++++++++++++----------
 1 file changed, 32 insertions(+), 16 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
index 9eb5dcd..bf765b5 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
@@ -180,7 +180,8 @@ pci_vfio_setup_dma_maps(int vfio_container_fd)
 	ret = ioctl(vfio_container_fd, VFIO_SET_IOMMU,
 			VFIO_TYPE1_IOMMU);
 	if (ret) {
-		RTE_LOG(ERR, EAL, "  cannot set IOMMU type!\n");
+		RTE_LOG(ERR, EAL, "  cannot set IOMMU type, "
+				"error %i (%s)\n", errno, strerror(errno));
 		return -1;
 	}
 
@@ -201,7 +202,8 @@ pci_vfio_setup_dma_maps(int vfio_container_fd)
 		ret = ioctl(vfio_container_fd, VFIO_IOMMU_MAP_DMA, &dma_map);
 
 		if (ret) {
-			RTE_LOG(ERR, EAL, "  cannot set up DMA remapping!\n");
+			RTE_LOG(ERR, EAL, "  cannot set up DMA remapping, "
+					"error %i (%s)\n", errno, strerror(errno));
 			return -1;
 		}
 	}
@@ -253,7 +255,8 @@ pci_vfio_setup_interrupts(struct rte_pci_device *dev, int vfio_dev_fd)
 
 		ret = ioctl(vfio_dev_fd, VFIO_DEVICE_GET_IRQ_INFO, &irq);
 		if (ret < 0) {
-			RTE_LOG(ERR, EAL, "  cannot get IRQ info!\n");
+			RTE_LOG(ERR, EAL, "  cannot get IRQ info, "
+					"error %i (%s)\n", errno, strerror(errno));
 			return -1;
 		}
 
@@ -271,7 +274,8 @@ pci_vfio_setup_interrupts(struct rte_pci_device *dev, int vfio_dev_fd)
 		/* set up an eventfd for interrupts */
 		fd = eventfd(0, 0);
 		if (fd < 0) {
-			RTE_LOG(ERR, EAL, "  cannot set up eventfd!\n");
+			RTE_LOG(ERR, EAL, "  cannot set up eventfd, "
+					"error %i (%s)\n", errno, strerror(errno));
 			return -1;
 		}
 
@@ -313,22 +317,31 @@ pci_vfio_get_container_fd(void)
 	if (internal_config.process_type == RTE_PROC_PRIMARY) {
 		vfio_container_fd = open(VFIO_CONTAINER_PATH, O_RDWR);
 		if (vfio_container_fd < 0) {
-			RTE_LOG(ERR, EAL, "  cannot open VFIO container!\n");
+			RTE_LOG(ERR, EAL, "  cannot open VFIO container, "
+					"error %i (%s)\n", errno, strerror(errno));
 			return -1;
 		}
 
 		/* check VFIO API version */
 		ret = ioctl(vfio_container_fd, VFIO_GET_API_VERSION);
 		if (ret != VFIO_API_VERSION) {
-			RTE_LOG(ERR, EAL, "  unknown VFIO API version!\n");
+			if (ret < 0)
+				RTE_LOG(ERR, EAL, "  could not get VFIO API version, "
+						"error %i (%s)\n", errno, strerror(errno));
+			else
+				RTE_LOG(ERR, EAL, "  unsupported VFIO API version!\n");
 			close(vfio_container_fd);
 			return -1;
 		}
 
 		/* check if we support IOMMU type 1 */
 		ret = ioctl(vfio_container_fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1_IOMMU);
-		if (!ret) {
-			RTE_LOG(ERR, EAL, "  unknown IOMMU driver!\n");
+		if (ret != 1) {
+			if (ret < 0)
+				RTE_LOG(ERR, EAL, "  could not get IOMMU type, "
+						"error %i (%s)\n", errno, strerror(errno));
+			else
+				RTE_LOG(ERR, EAL, "  unsupported IOMMU type!\n");
 			close(vfio_container_fd);
 			return -1;
 		}
@@ -564,7 +577,8 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
 	/* check if the group is viable */
 	ret = ioctl(vfio_group_fd, VFIO_GROUP_GET_STATUS, &group_status);
 	if (ret) {
-		RTE_LOG(ERR, EAL, "  %s cannot get group status!\n", pci_addr);
+		RTE_LOG(ERR, EAL, "  %s cannot get group status, "
+				"error %i (%s)\n", pci_addr, errno, strerror(errno));
 		close(vfio_group_fd);
 		clear_current_group();
 		return -1;
@@ -587,8 +601,8 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
 		ret = ioctl(vfio_group_fd, VFIO_GROUP_SET_CONTAINER,
 				&vfio_cfg.vfio_container_fd);
 		if (ret) {
-			RTE_LOG(ERR, EAL, "  %s cannot add VFIO group to container!\n",
-					pci_addr);
+			RTE_LOG(ERR, EAL, "  %s cannot add VFIO group to container, "
+					"error %i (%s)\n", pci_addr, errno, strerror(errno));
 			close(vfio_group_fd);
 			clear_current_group();
 			return -1;
@@ -611,7 +625,8 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
 			vfio_cfg.vfio_container_has_dma == 0) {
 		ret = pci_vfio_setup_dma_maps(vfio_cfg.vfio_container_fd);
 		if (ret) {
-			RTE_LOG(ERR, EAL, "  %s DMA remapping failed!\n", pci_addr);
+			RTE_LOG(ERR, EAL, "  %s DMA remapping failed, "
+					"error %i (%s)\n", pci_addr, errno, strerror(errno));
 			return -1;
 		}
 		vfio_cfg.vfio_container_has_dma = 1;
@@ -631,7 +646,8 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
 	/* test and setup the device */
 	ret = ioctl(vfio_dev_fd, VFIO_DEVICE_GET_INFO, &device_info);
 	if (ret) {
-		RTE_LOG(ERR, EAL, "  %s cannot get device info!\n", pci_addr);
+		RTE_LOG(ERR, EAL, "  %s cannot get device info, "
+				"error %i (%s)\n", pci_addr, errno, strerror(errno));
 		close(vfio_dev_fd);
 		return -1;
 	}
@@ -688,8 +704,8 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
 		ret = ioctl(vfio_dev_fd, VFIO_DEVICE_GET_REGION_INFO, &reg);
 
 		if (ret) {
-			RTE_LOG(ERR, EAL, "  %s cannot get device region info!\n",
-					pci_addr);
+			RTE_LOG(ERR, EAL, "  %s cannot get device region info "
+					"error %i (%s)\n", pci_addr, errno, strerror(errno));
 			close(vfio_dev_fd);
 			if (internal_config.process_type == RTE_PROC_PRIMARY)
 				rte_free(vfio_res);
@@ -765,7 +781,7 @@ pci_vfio_enable(void)
 	if (vfio_cfg.vfio_container_fd != -1)
 		vfio_cfg.vfio_enabled = 1;
 	else
-		RTE_LOG(INFO, EAL, "VFIO driver not loaded or wrong permissions\n");
+		RTE_LOG(INFO, EAL, "VFIO support could not be initialized\n");
 
 	return 0;
 }
-- 
1.8.1.4