diff mbox series

[v6,1/2] bus/pci: fix secondary process PCI uio resource map problem

Message ID 20240702074007.1547-2-chaoyong.he@corigine.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series fix secondary process PCI UIO resource problem |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Chaoyong He July 2, 2024, 7:40 a.m. UTC 
From: Zerun Fu <zerun.fu@corigine.com>

For the primary process, the logic loops all BARs and will skip
the map of BAR with an invalid physical address (0), also will
assign 'uio_res->nb_maps' with the real mapped BARs number. But
for the secondary process, instead of loops all BARs, the logic
using the 'uio_res->nb_map' as index. If the device uses continuous
BARs there will be no problem, whereas if it uses discrete BARs,
it will lead to mapping errors.

Fix this problem by also loops all BARs and skip the map of BAR
with an invalid physical address in secondary process.

Fixes: 9b957f378abf ("pci: merge uio functions for linux and bsd")
Cc: mukawa@igel.co.jp
Cc: stable@dpdk.org

Signed-off-by: Zerun Fu <zerun.fu@corigine.com>
Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/bus/pci/pci_common_uio.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

Comments

Chenbo Xia July 4, 2024, 9 a.m. UTC | #1 
> On Jul 2, 2024, at 15:40, Chaoyong He <chaoyong.he@corigine.com> wrote:
> 
> External email: Use caution opening links or attachments
> 
> 
> From: Zerun Fu <zerun.fu@corigine.com>
> 
> For the primary process, the logic loops all BARs and will skip
> the map of BAR with an invalid physical address (0), also will
> assign 'uio_res->nb_maps' with the real mapped BARs number. But
> for the secondary process, instead of loops all BARs, the logic
> using the 'uio_res->nb_map' as index. If the device uses continuous
> BARs there will be no problem, whereas if it uses discrete BARs,
> it will lead to mapping errors.
> 
> Fix this problem by also loops all BARs and skip the map of BAR
> with an invalid physical address in secondary process.
> 
> Fixes: 9b957f378abf ("pci: merge uio functions for linux and bsd")
> Cc: mukawa@igel.co.jp
> Cc: stable@dpdk.org
> 
> Signed-off-by: Zerun Fu <zerun.fu@corigine.com>
> Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
> Reviewed-by: Long Wu <long.wu@corigine.com>
> Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
> drivers/bus/pci/pci_common_uio.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/bus/pci/pci_common_uio.c b/drivers/bus/pci/pci_common_uio.c
> index 76c661f054..f44ccdf27c 100644
> --- a/drivers/bus/pci/pci_common_uio.c
> +++ b/drivers/bus/pci/pci_common_uio.c
> @@ -26,7 +26,7 @@ EAL_REGISTER_TAILQ(rte_uio_tailq)
> static int
> pci_uio_map_secondary(struct rte_pci_device *dev)
> {
> -       int fd, i, j;
> +       int fd, i = 0, j, res_idx;
>        struct mapped_pci_resource *uio_res;
>        struct mapped_pci_res_list *uio_res_list =
>                        RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
> @@ -37,7 +37,15 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
>                if (rte_pci_addr_cmp(&uio_res->pci_addr, &dev->addr))
>                        continue;
> 
> -               for (i = 0; i != uio_res->nb_maps; i++) {
> +               /* Map all BARs */
> +               for (res_idx = 0; res_idx != PCI_MAX_RESOURCE; res_idx++) {
> +                       /* skip empty BAR */
> +                       if (dev->mem_resource[res_idx].phys_addr == 0)
> +                               continue;
> +
> +                       if (i >= uio_res->nb_maps)
> +                               return -1;
> +
>                        /*
>                         * open devname, to mmap it
>                         */
> @@ -71,7 +79,9 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
>                                }
>                                return -1;
>                        }
> -                       dev->mem_resource[i].addr = mapaddr;
> +                       dev->mem_resource[res_idx].addr = mapaddr;
> +
> +                       i++;
>                }
>                return 0;
>        }
> —
> 2.39.1
> 

Reviewed-by: Chenbo Xia <chenbox@nvidia.com>
Dengdui Huang July 15, 2024, 5:55 a.m. UTC | #2 
On 2024/7/2 15:40, Chaoyong He wrote:
> From: Zerun Fu <zerun.fu@corigine.com>
> 
> For the primary process, the logic loops all BARs and will skip
> the map of BAR with an invalid physical address (0), also will
> assign 'uio_res->nb_maps' with the real mapped BARs number. But
> for the secondary process, instead of loops all BARs, the logic
> using the 'uio_res->nb_map' as index. If the device uses continuous
> BARs there will be no problem, whereas if it uses discrete BARs,
> it will lead to mapping errors.
> 
> Fix this problem by also loops all BARs and skip the map of BAR
> with an invalid physical address in secondary process.
> 
> Fixes: 9b957f378abf ("pci: merge uio functions for linux and bsd")

Fixes: e6cf7bee1c77 ("bus/pci: fix UIO resource access from secondary process")

> Cc: mukawa@igel.co.jp
> Cc: stable@dpdk.org
> 
> Signed-off-by: Zerun Fu <zerun.fu@corigine.com>
> Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
> Reviewed-by: Long Wu <long.wu@corigine.com>
> Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

Tested-by: Dengdui Huang <huangdengdui@huawei.com>
diff mbox series

Patch

diff --git a/drivers/bus/pci/pci_common_uio.c b/drivers/bus/pci/pci_common_uio.c
index 76c661f054..f44ccdf27c 100644
--- a/drivers/bus/pci/pci_common_uio.c
+++ b/drivers/bus/pci/pci_common_uio.c
@@ -26,7 +26,7 @@  EAL_REGISTER_TAILQ(rte_uio_tailq)
 static int
 pci_uio_map_secondary(struct rte_pci_device *dev)
 {
-	int fd, i, j;
+	int fd, i = 0, j, res_idx;
 	struct mapped_pci_resource *uio_res;
 	struct mapped_pci_res_list *uio_res_list =
 			RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
@@ -37,7 +37,15 @@  pci_uio_map_secondary(struct rte_pci_device *dev)
 		if (rte_pci_addr_cmp(&uio_res->pci_addr, &dev->addr))
 			continue;
 
-		for (i = 0; i != uio_res->nb_maps; i++) {
+		/* Map all BARs */
+		for (res_idx = 0; res_idx != PCI_MAX_RESOURCE; res_idx++) {
+			/* skip empty BAR */
+			if (dev->mem_resource[res_idx].phys_addr == 0)
+				continue;
+
+			if (i >= uio_res->nb_maps)
+				return -1;
+
 			/*
 			 * open devname, to mmap it
 			 */
@@ -71,7 +79,9 @@  pci_uio_map_secondary(struct rte_pci_device *dev)
 				}
 				return -1;
 			}
-			dev->mem_resource[i].addr = mapaddr;
+			dev->mem_resource[res_idx].addr = mapaddr;
+
+			i++;
 		}
 		return 0;
 	}