DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] vfio: open VFIO container at startup rather than during init
@ 2014-06-17  9:12 Anatoly Burakov
  2014-06-17 16:50 ` Richardson, Bruce
                   ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Anatoly Burakov @ 2014-06-17  9:12 UTC (permalink / raw)
  To: dev


Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-dev] [PATCH] vfio: open VFIO container at startup rather than during init
  2014-06-17  9:12 [dpdk-dev] [PATCH] vfio: open VFIO container at startup rather than during init Anatoly Burakov
@ 2014-06-17 16:50 ` Richardson, Bruce
  2014-06-17 16:55 ` Thomas Monjalon
  2014-06-18 13:07 ` [dpdk-dev] [PATCH v2 0/2] Fix issues with VFIO Anatoly Burakov
  2 siblings, 0 replies; 19+ messages in thread
From: Richardson, Bruce @ 2014-06-17 16:50 UTC (permalink / raw)
  To: Burakov, Anatoly, dev



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

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

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-dev] [PATCH] vfio: open VFIO container at startup rather than during init
  2014-06-17  9:12 [dpdk-dev] [PATCH] vfio: open VFIO container at startup rather than during init Anatoly Burakov
  2014-06-17 16:50 ` Richardson, Bruce
@ 2014-06-17 16:55 ` Thomas Monjalon
  2014-06-18  8:57   ` Burakov, Anatoly
  2014-06-18 13:07 ` [dpdk-dev] [PATCH v2 0/2] Fix issues with VFIO Anatoly Burakov
  2 siblings, 1 reply; 19+ messages in thread
From: Thomas Monjalon @ 2014-06-17 16:55 UTC (permalink / raw)
  To: Anatoly Burakov; +Cc: dev

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

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

Thanks
-- 
Thomas

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-dev] [PATCH] vfio: open VFIO container at startup rather than during init
  2014-06-17 16:55 ` Thomas Monjalon
@ 2014-06-18  8:57   ` Burakov, Anatoly
  2014-06-18 10:08     ` Dumitrescu, Cristian
  0 siblings, 1 reply; 19+ messages in thread
From: Burakov, Anatoly @ 2014-06-18  8:57 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

Hi Thomas,

> Subject: Re: [dpdk-dev] [PATCH] vfio: open VFIO container at startup rather
> than during init
> 
> > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> 
> 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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-dev] [PATCH] vfio: open VFIO container at startup rather than during init
  2014-06-18  8:57   ` Burakov, Anatoly
@ 2014-06-18 10:08     ` Dumitrescu, Cristian
  2014-06-18 10:26       ` Burakov, Anatoly
  0 siblings, 1 reply; 19+ messages in thread
From: Dumitrescu, Cristian @ 2014-06-18 10:08 UTC (permalink / raw)
  To: Burakov, Anatoly, Thomas Monjalon; +Cc: dev

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@dpdk.org] On Behalf Of Burakov, Anatoly
Sent: Wednesday, June 18, 2014 9:57 AM
To: Thomas Monjalon
Cc: dev@dpdk.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@intel.com>
> 
> 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.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-dev] [PATCH] vfio: open VFIO container at startup rather than during init
  2014-06-18 10:08     ` Dumitrescu, Cristian
@ 2014-06-18 10:26       ` Burakov, Anatoly
  2014-06-18 11:00         ` Neil Horman
  0 siblings, 1 reply; 19+ messages in thread
From: Burakov, Anatoly @ 2014-06-18 10:26 UTC (permalink / raw)
  To: Dumitrescu, Cristian, Thomas Monjalon; +Cc: dev

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-dev] [PATCH] vfio: open VFIO container at startup rather than during init
  2014-06-18 10:26       ` Burakov, Anatoly
@ 2014-06-18 11:00         ` Neil Horman
  2014-06-18 11:02           ` Burakov, Anatoly
  0 siblings, 1 reply; 19+ messages in thread
From: Neil Horman @ 2014-06-18 11:00 UTC (permalink / raw)
  To: Burakov, Anatoly; +Cc: dev

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-dev] [PATCH] vfio: open VFIO container at startup rather than during init
  2014-06-18 11:00         ` Neil Horman
@ 2014-06-18 11:02           ` Burakov, Anatoly
  2014-06-18 11:07             ` Neil Horman
  0 siblings, 1 reply; 19+ messages in thread
From: Burakov, Anatoly @ 2014-06-18 11:02 UTC (permalink / raw)
  To: Neil Horman; +Cc: dev

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-dev] [PATCH] vfio: open VFIO container at startup rather than during init
  2014-06-18 11:02           ` Burakov, Anatoly
@ 2014-06-18 11:07             ` Neil Horman
  0 siblings, 0 replies; 19+ messages in thread
From: Neil Horman @ 2014-06-18 11:07 UTC (permalink / raw)
  To: Burakov, Anatoly; +Cc: dev

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
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [dpdk-dev] [PATCH v2 0/2] Fix issues with VFIO
  2014-06-17  9:12 [dpdk-dev] [PATCH] vfio: open VFIO container at startup rather than during init Anatoly Burakov
  2014-06-17 16:50 ` Richardson, Bruce
  2014-06-17 16:55 ` Thomas Monjalon
@ 2014-06-18 13:07 ` Anatoly Burakov
  2014-06-18 13:07   ` [dpdk-dev] [PATCH v2 1/2] vfio: open VFIO container at startup rather than during init Anatoly Burakov
                     ` (3 more replies)
  2 siblings, 4 replies; 19+ messages in thread
From: Anatoly Burakov @ 2014-06-18 13:07 UTC (permalink / raw)
  To: dev

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [dpdk-dev] [PATCH v2 1/2] vfio: open VFIO container at startup rather than during init
  2014-06-18 13:07 ` [dpdk-dev] [PATCH v2 0/2] Fix issues with VFIO Anatoly Burakov
@ 2014-06-18 13:07   ` Anatoly Burakov
  2014-06-18 13:07   ` [dpdk-dev] [PATCH v2 2/2] vfio: more verbose error messages Anatoly Burakov
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 19+ messages in thread
From: Anatoly Burakov @ 2014-06-18 13:07 UTC (permalink / raw)
  To: dev

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@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
 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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [dpdk-dev] [PATCH v2 2/2] vfio: more verbose error messages
  2014-06-18 13:07 ` [dpdk-dev] [PATCH v2 0/2] Fix issues with VFIO Anatoly Burakov
  2014-06-18 13:07   ` [dpdk-dev] [PATCH v2 1/2] vfio: open VFIO container at startup rather than during init Anatoly Burakov
@ 2014-06-18 13:07   ` Anatoly Burakov
  2014-06-18 13:44   ` [dpdk-dev] [PATCH v2 0/2] Fix issues with VFIO Neil Horman
  2014-06-18 15:07   ` [dpdk-dev] [PATCH v3 " Anatoly Burakov
  3 siblings, 0 replies; 19+ messages in thread
From: Anatoly Burakov @ 2014-06-18 13:07 UTC (permalink / raw)
  To: dev


Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
CC: Neil Horman <nhorman@tuxdriver.com>
---
 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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-dev] [PATCH v2 0/2] Fix issues with VFIO
  2014-06-18 13:07 ` [dpdk-dev] [PATCH v2 0/2] Fix issues with VFIO Anatoly Burakov
  2014-06-18 13:07   ` [dpdk-dev] [PATCH v2 1/2] vfio: open VFIO container at startup rather than during init Anatoly Burakov
  2014-06-18 13:07   ` [dpdk-dev] [PATCH v2 2/2] vfio: more verbose error messages Anatoly Burakov
@ 2014-06-18 13:44   ` Neil Horman
  2014-06-18 15:07   ` [dpdk-dev] [PATCH v3 " Anatoly Burakov
  3 siblings, 0 replies; 19+ messages in thread
From: Neil Horman @ 2014-06-18 13:44 UTC (permalink / raw)
  To: Anatoly Burakov; +Cc: dev

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [dpdk-dev] [PATCH v3 0/2] Fix issues with VFIO
  2014-06-18 13:07 ` [dpdk-dev] [PATCH v2 0/2] Fix issues with VFIO Anatoly Burakov
                     ` (2 preceding siblings ...)
  2014-06-18 13:44   ` [dpdk-dev] [PATCH v2 0/2] Fix issues with VFIO Neil Horman
@ 2014-06-18 15:07   ` Anatoly Burakov
  2014-06-18 15:07     ` [dpdk-dev] [PATCH v3 1/2] vfio: open VFIO container at startup rather than during init Anatoly Burakov
                       ` (3 more replies)
  3 siblings, 4 replies; 19+ messages in thread
From: Anatoly Burakov @ 2014-06-18 15:07 UTC (permalink / raw)
  To: dev

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [dpdk-dev] [PATCH v3 1/2] vfio: open VFIO container at startup rather than during init
  2014-06-18 15:07   ` [dpdk-dev] [PATCH v3 " Anatoly Burakov
@ 2014-06-18 15:07     ` Anatoly Burakov
  2014-06-18 15:07     ` [dpdk-dev] [PATCH v3 2/2] vfio: more verbose error messages Anatoly Burakov
                       ` (2 subsequent siblings)
  3 siblings, 0 replies; 19+ messages in thread
From: Anatoly Burakov @ 2014-06-18 15:07 UTC (permalink / raw)
  To: dev

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@intel.com>
---
 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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [dpdk-dev] [PATCH v3 2/2] vfio: more verbose error messages
  2014-06-18 15:07   ` [dpdk-dev] [PATCH v3 " Anatoly Burakov
  2014-06-18 15:07     ` [dpdk-dev] [PATCH v3 1/2] vfio: open VFIO container at startup rather than during init Anatoly Burakov
@ 2014-06-18 15:07     ` Anatoly Burakov
  2014-06-18 17:55     ` [dpdk-dev] [PATCH v3 0/2] Fix issues with VFIO Richardson, Bruce
  2014-06-18 18:51     ` Neil Horman
  3 siblings, 0 replies; 19+ messages in thread
From: Anatoly Burakov @ 2014-06-18 15:07 UTC (permalink / raw)
  To: dev

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

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-dev] [PATCH v3 0/2] Fix issues with VFIO
  2014-06-18 15:07   ` [dpdk-dev] [PATCH v3 " Anatoly Burakov
  2014-06-18 15:07     ` [dpdk-dev] [PATCH v3 1/2] vfio: open VFIO container at startup rather than during init Anatoly Burakov
  2014-06-18 15:07     ` [dpdk-dev] [PATCH v3 2/2] vfio: more verbose error messages Anatoly Burakov
@ 2014-06-18 17:55     ` Richardson, Bruce
  2014-06-18 23:09       ` Thomas Monjalon
  2014-06-18 18:51     ` Neil Horman
  3 siblings, 1 reply; 19+ messages in thread
From: Richardson, Bruce @ 2014-06-18 17:55 UTC (permalink / raw)
  To: Burakov, Anatoly, dev

> -----Original Message-----
> From: Burakov, Anatoly
> Sent: Wednesday, June 18, 2014 8:07 AM
> To: dev@dpdk.org
> Cc: Richardson, Bruce; nhorman@tuxdriver.com
> 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@intel.com>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-dev] [PATCH v3 0/2] Fix issues with VFIO
  2014-06-18 15:07   ` [dpdk-dev] [PATCH v3 " Anatoly Burakov
                       ` (2 preceding siblings ...)
  2014-06-18 17:55     ` [dpdk-dev] [PATCH v3 0/2] Fix issues with VFIO Richardson, Bruce
@ 2014-06-18 18:51     ` Neil Horman
  3 siblings, 0 replies; 19+ messages in thread
From: Neil Horman @ 2014-06-18 18:51 UTC (permalink / raw)
  To: Anatoly Burakov; +Cc: dev

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@tuxdriver.com>
Thanks Anatoly!
Neil

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-dev] [PATCH v3 0/2] Fix issues with VFIO
  2014-06-18 17:55     ` [dpdk-dev] [PATCH v3 0/2] Fix issues with VFIO Richardson, Bruce
@ 2014-06-18 23:09       ` Thomas Monjalon
  0 siblings, 0 replies; 19+ messages in thread
From: Thomas Monjalon @ 2014-06-18 23:09 UTC (permalink / raw)
  To: Burakov, Anatoly; +Cc: dev

> > 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@intel.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>

Applied for version 1.7.0.

Thanks
-- 
Thomas

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2014-06-18 23:09 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-17  9:12 [dpdk-dev] [PATCH] vfio: open VFIO container at startup rather than during init Anatoly Burakov
2014-06-17 16:50 ` Richardson, Bruce
2014-06-17 16:55 ` Thomas Monjalon
2014-06-18  8:57   ` Burakov, Anatoly
2014-06-18 10:08     ` Dumitrescu, Cristian
2014-06-18 10:26       ` Burakov, Anatoly
2014-06-18 11:00         ` Neil Horman
2014-06-18 11:02           ` Burakov, Anatoly
2014-06-18 11:07             ` Neil Horman
2014-06-18 13:07 ` [dpdk-dev] [PATCH v2 0/2] Fix issues with VFIO Anatoly Burakov
2014-06-18 13:07   ` [dpdk-dev] [PATCH v2 1/2] vfio: open VFIO container at startup rather than during init Anatoly Burakov
2014-06-18 13:07   ` [dpdk-dev] [PATCH v2 2/2] vfio: more verbose error messages Anatoly Burakov
2014-06-18 13:44   ` [dpdk-dev] [PATCH v2 0/2] Fix issues with VFIO Neil Horman
2014-06-18 15:07   ` [dpdk-dev] [PATCH v3 " Anatoly Burakov
2014-06-18 15:07     ` [dpdk-dev] [PATCH v3 1/2] vfio: open VFIO container at startup rather than during init Anatoly Burakov
2014-06-18 15:07     ` [dpdk-dev] [PATCH v3 2/2] vfio: more verbose error messages Anatoly Burakov
2014-06-18 17:55     ` [dpdk-dev] [PATCH v3 0/2] Fix issues with VFIO Richardson, Bruce
2014-06-18 23:09       ` Thomas Monjalon
2014-06-18 18:51     ` Neil Horman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).