DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] vfio: revert retry logic for MSI-X BAR mapping
@ 2018-07-30 10:59 Anatoly Burakov
  2018-07-31  3:47 ` Jerin Jacob
  0 siblings, 1 reply; 3+ messages in thread
From: Anatoly Burakov @ 2018-07-30 10:59 UTC (permalink / raw)
  To: dev; +Cc: thomas, jerin.jacob, t.yoshimura8869

This reverts commit d4774a568ba0a5923229974a002972c83eb04570.

The patch is incomplete because kernel 4.16+, while being capable
of mapping MSI-X BARs, will also report if such a capability is
available. Without checking this capability, gratuitous errors
are displayed on kernels <4.16 while VFIO is attempting to mmap
MSI-X BAR and fails, which can be confusing to the user.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/bus/pci/linux/pci_vfio.c | 93 ++++++++++++++------------------
 1 file changed, 41 insertions(+), 52 deletions(-)

diff --git a/drivers/bus/pci/linux/pci_vfio.c b/drivers/bus/pci/linux/pci_vfio.c
index bb6ef7b67..686386d6a 100644
--- a/drivers/bus/pci/linux/pci_vfio.c
+++ b/drivers/bus/pci/linux/pci_vfio.c
@@ -332,59 +332,50 @@ pci_vfio_mmap_bar(int vfio_dev_fd, struct mapped_pci_resource *vfio_res,
 	void *bar_addr;
 	struct pci_msix_table *msix_table = &vfio_res->msix_table;
 	struct pci_map *bar = &vfio_res->maps[bar_index];
-	bool again = false;
 
 	if (bar->size == 0)
 		/* Skip this BAR */
 		return 0;
 
+	if (msix_table->bar_index == bar_index) {
+		/*
+		 * VFIO will not let us map the MSI-X table,
+		 * but we can map around it.
+		 */
+		uint32_t table_start = msix_table->offset;
+		uint32_t table_end = table_start + msix_table->size;
+		table_end = (table_end + ~PAGE_MASK) & PAGE_MASK;
+		table_start &= PAGE_MASK;
+
+		if (table_start == 0 && table_end >= bar->size) {
+			/* Cannot map this BAR */
+			RTE_LOG(DEBUG, EAL, "Skipping BAR%d\n", bar_index);
+			bar->size = 0;
+			bar->addr = 0;
+			return 0;
+		}
+
+		memreg[0].offset = bar->offset;
+		memreg[0].size = table_start;
+		memreg[1].offset = bar->offset + table_end;
+		memreg[1].size = bar->size - table_end;
+
+		RTE_LOG(DEBUG, EAL,
+			"Trying to map BAR%d that contains the MSI-X "
+			"table. Trying offsets: "
+			"0x%04lx:0x%04lx, 0x%04lx:0x%04lx\n", bar_index,
+			memreg[0].offset, memreg[0].size,
+			memreg[1].offset, memreg[1].size);
+	} else {
+		memreg[0].offset = bar->offset;
+		memreg[0].size = bar->size;
+	}
+
 	/* reserve the address using an inaccessible mapping */
 	bar_addr = mmap(bar->addr, bar->size, 0, MAP_PRIVATE |
 			MAP_ANONYMOUS | additional_flags, -1, 0);
-	if (bar_addr == MAP_FAILED) {
-		RTE_LOG(ERR, EAL,
-			"Failed to create inaccessible mapping for BAR%d\n",
-			bar_index);
-		return -1;
-	}
-
-	memreg[0].offset = bar->offset;
-	memreg[0].size = bar->size;
-	do {
+	if (bar_addr != MAP_FAILED) {
 		void *map_addr = NULL;
-		if (again) {
-			/*
-			 * VFIO did not let us map the MSI-X table,
-			 * but we can map around it.
-			 */
-			uint32_t table_start = msix_table->offset;
-			uint32_t table_end = table_start + msix_table->size;
-			table_end = (table_end + ~PAGE_MASK) & PAGE_MASK;
-			table_start &= PAGE_MASK;
-
-			if (table_start == 0 && table_end >= bar->size) {
-				/* Cannot map this BAR */
-				RTE_LOG(DEBUG, EAL, "Skipping BAR%d\n",
-						bar_index);
-				munmap(bar_addr, bar->size);
-				bar->size = 0;
-				bar->addr = 0;
-				return 0;
-			}
-
-			memreg[0].offset = bar->offset;
-			memreg[0].size = table_start;
-			memreg[1].offset = bar->offset + table_end;
-			memreg[1].size = bar->size - table_end;
-
-			RTE_LOG(DEBUG, EAL,
-				"Trying to map BAR%d that contains the MSI-X "
-				"table. Trying offsets: "
-				"0x%04lx:0x%04lx, 0x%04lx:0x%04lx\n", bar_index,
-				memreg[0].offset, memreg[0].size,
-				memreg[1].offset, memreg[1].size);
-		}
-
 		if (memreg[0].size) {
 			/* actual map of first part */
 			map_addr = pci_map_resource(bar_addr, vfio_dev_fd,
@@ -393,12 +384,6 @@ pci_vfio_mmap_bar(int vfio_dev_fd, struct mapped_pci_resource *vfio_res,
 							MAP_FIXED);
 		}
 
-		if (map_addr == MAP_FAILED &&
-			msix_table->bar_index == bar_index && !again) {
-			again = true;
-			continue;
-		}
-
 		/* if there's a second part, try to map it */
 		if (map_addr != MAP_FAILED
 			&& memreg[1].offset && memreg[1].size) {
@@ -419,8 +404,12 @@ pci_vfio_mmap_bar(int vfio_dev_fd, struct mapped_pci_resource *vfio_res,
 					bar_index);
 			return -1;
 		}
-		break;
-	} while (again);
+	} else {
+		RTE_LOG(ERR, EAL,
+				"Failed to create inaccessible mapping for BAR%d\n",
+				bar_index);
+		return -1;
+	}
 
 	bar->addr = bar_addr;
 	return 0;
-- 
2.17.1

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

* Re: [dpdk-dev] [PATCH] vfio: revert retry logic for MSI-X BAR mapping
  2018-07-30 10:59 [dpdk-dev] [PATCH] vfio: revert retry logic for MSI-X BAR mapping Anatoly Burakov
@ 2018-07-31  3:47 ` Jerin Jacob
  2018-08-01 15:52   ` Thomas Monjalon
  0 siblings, 1 reply; 3+ messages in thread
From: Jerin Jacob @ 2018-07-31  3:47 UTC (permalink / raw)
  To: Anatoly Burakov; +Cc: dev, thomas, t.yoshimura8869

-----Original Message-----
> Date: Mon, 30 Jul 2018 11:59:06 +0100
> From: Anatoly Burakov <anatoly.burakov@intel.com>
> To: dev@dpdk.org
> CC: thomas@monjalon.net, jerin.jacob@caviumnetworks.com,
>  t.yoshimura8869@gmail.com
> Subject: [PATCH] vfio: revert retry logic for MSI-X BAR mapping
> X-Mailer: git-send-email 1.7.0.7
> 
> External Email
> 
> This reverts commit d4774a568ba0a5923229974a002972c83eb04570.
> 
> The patch is incomplete because kernel 4.16+, while being capable
> of mapping MSI-X BARs, will also report if such a capability is
> available. Without checking this capability, gratuitous errors
> are displayed on kernels <4.16 while VFIO is attempting to mmap
> MSI-X BAR and fails, which can be confusing to the user.
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>

Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>

> ---
>  drivers/bus/pci/linux/pci_vfio.c | 93 ++++++++++++++------------------
>  1 file changed, 41 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/bus/pci/linux/pci_vfio.c b/drivers/bus/pci/linux/pci_vfio.c
> index bb6ef7b67..686386d6a 100644
> --- a/drivers/bus/pci/linux/pci_vfio.c
> +++ b/drivers/bus/pci/linux/pci_vfio.c
> @@ -332,59 +332,50 @@ pci_vfio_mmap_bar(int vfio_dev_fd, struct mapped_pci_resource *vfio_res,
>         void *bar_addr;
>         struct pci_msix_table *msix_table = &vfio_res->msix_table;
>         struct pci_map *bar = &vfio_res->maps[bar_index];
> -       bool again = false;
> 
>         if (bar->size == 0)
>                 /* Skip this BAR */
>                 return 0;
> 
> +       if (msix_table->bar_index == bar_index) {
> +               /*
> +                * VFIO will not let us map the MSI-X table,
> +                * but we can map around it.
> +                */
> +               uint32_t table_start = msix_table->offset;
> +               uint32_t table_end = table_start + msix_table->size;
> +               table_end = (table_end + ~PAGE_MASK) & PAGE_MASK;
> +               table_start &= PAGE_MASK;
> +
> +               if (table_start == 0 && table_end >= bar->size) {
> +                       /* Cannot map this BAR */
> +                       RTE_LOG(DEBUG, EAL, "Skipping BAR%d\n", bar_index);
> +                       bar->size = 0;
> +                       bar->addr = 0;
> +                       return 0;
> +               }
> +
> +               memreg[0].offset = bar->offset;
> +               memreg[0].size = table_start;
> +               memreg[1].offset = bar->offset + table_end;
> +               memreg[1].size = bar->size - table_end;
> +
> +               RTE_LOG(DEBUG, EAL,
> +                       "Trying to map BAR%d that contains the MSI-X "
> +                       "table. Trying offsets: "
> +                       "0x%04lx:0x%04lx, 0x%04lx:0x%04lx\n", bar_index,
> +                       memreg[0].offset, memreg[0].size,
> +                       memreg[1].offset, memreg[1].size);
> +       } else {
> +               memreg[0].offset = bar->offset;
> +               memreg[0].size = bar->size;
> +       }
> +
>         /* reserve the address using an inaccessible mapping */
>         bar_addr = mmap(bar->addr, bar->size, 0, MAP_PRIVATE |
>                         MAP_ANONYMOUS | additional_flags, -1, 0);
> -       if (bar_addr == MAP_FAILED) {
> -               RTE_LOG(ERR, EAL,
> -                       "Failed to create inaccessible mapping for BAR%d\n",
> -                       bar_index);
> -               return -1;
> -       }
> -
> -       memreg[0].offset = bar->offset;
> -       memreg[0].size = bar->size;
> -       do {
> +       if (bar_addr != MAP_FAILED) {
>                 void *map_addr = NULL;
> -               if (again) {
> -                       /*
> -                        * VFIO did not let us map the MSI-X table,
> -                        * but we can map around it.
> -                        */
> -                       uint32_t table_start = msix_table->offset;
> -                       uint32_t table_end = table_start + msix_table->size;
> -                       table_end = (table_end + ~PAGE_MASK) & PAGE_MASK;
> -                       table_start &= PAGE_MASK;
> -
> -                       if (table_start == 0 && table_end >= bar->size) {
> -                               /* Cannot map this BAR */
> -                               RTE_LOG(DEBUG, EAL, "Skipping BAR%d\n",
> -                                               bar_index);
> -                               munmap(bar_addr, bar->size);
> -                               bar->size = 0;
> -                               bar->addr = 0;
> -                               return 0;
> -                       }
> -
> -                       memreg[0].offset = bar->offset;
> -                       memreg[0].size = table_start;
> -                       memreg[1].offset = bar->offset + table_end;
> -                       memreg[1].size = bar->size - table_end;
> -
> -                       RTE_LOG(DEBUG, EAL,
> -                               "Trying to map BAR%d that contains the MSI-X "
> -                               "table. Trying offsets: "
> -                               "0x%04lx:0x%04lx, 0x%04lx:0x%04lx\n", bar_index,
> -                               memreg[0].offset, memreg[0].size,
> -                               memreg[1].offset, memreg[1].size);
> -               }
> -
>                 if (memreg[0].size) {
>                         /* actual map of first part */
>                         map_addr = pci_map_resource(bar_addr, vfio_dev_fd,
> @@ -393,12 +384,6 @@ pci_vfio_mmap_bar(int vfio_dev_fd, struct mapped_pci_resource *vfio_res,
>                                                         MAP_FIXED);
>                 }
> 
> -               if (map_addr == MAP_FAILED &&
> -                       msix_table->bar_index == bar_index && !again) {
> -                       again = true;
> -                       continue;
> -               }
> -
>                 /* if there's a second part, try to map it */
>                 if (map_addr != MAP_FAILED
>                         && memreg[1].offset && memreg[1].size) {
> @@ -419,8 +404,12 @@ pci_vfio_mmap_bar(int vfio_dev_fd, struct mapped_pci_resource *vfio_res,
>                                         bar_index);
>                         return -1;
>                 }
> -               break;
> -       } while (again);
> +       } else {
> +               RTE_LOG(ERR, EAL,
> +                               "Failed to create inaccessible mapping for BAR%d\n",
> +                               bar_index);
> +               return -1;
> +       }
> 
>         bar->addr = bar_addr;
>         return 0;
> --
> 2.17.1

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

* Re: [dpdk-dev] [PATCH] vfio: revert retry logic for MSI-X BAR mapping
  2018-07-31  3:47 ` Jerin Jacob
@ 2018-08-01 15:52   ` Thomas Monjalon
  0 siblings, 0 replies; 3+ messages in thread
From: Thomas Monjalon @ 2018-08-01 15:52 UTC (permalink / raw)
  To: Anatoly Burakov; +Cc: dev, Jerin Jacob, t.yoshimura8869

> > This reverts commit d4774a568ba0a5923229974a002972c83eb04570.
> > 
> > The patch is incomplete because kernel 4.16+, while being capable
> > of mapping MSI-X BARs, will also report if such a capability is
> > available. Without checking this capability, gratuitous errors
> > are displayed on kernels <4.16 while VFIO is attempting to mmap
> > MSI-X BAR and fails, which can be confusing to the user.
> > 
> > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> 
> Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>

Applied, thanks

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

end of thread, other threads:[~2018-08-01 15:52 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-30 10:59 [dpdk-dev] [PATCH] vfio: revert retry logic for MSI-X BAR mapping Anatoly Burakov
2018-07-31  3:47 ` Jerin Jacob
2018-08-01 15:52   ` Thomas Monjalon

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