[RFC 00/11] Device unplug and bus cleanup refactoring for NXP

[RFC 00/11] Device unplug and bus cleanup refactoring for NXP

[David Marchand]

Hello Hemant,

This is a followup to the refactoring started in 26.07.

I took some time with my best AI friend to cleanup DPAA and FSLMC bus
drivers.

Like the last time, only compilation has been checked.
I have no hardware to test runtime.

One thing that could be broken is either the order of devices
initialisation, or bugs in the device filtering that I tried to
simplify.

I think it is worth testing and fixing, as it will make the two NXP
bus drivers similar to other bus drivers (but keep the special IO devices
handling internal to the FSLMC bus for example).


-- 
David Marchand

David Marchand (11):
  drivers/bus: cleanup device freeing in NXP bus scan
  bus/dpaa: allocate interrupt during probing
  bus/dpaa: support unplug and use generic cleanup
  bus/fslmc: fix memory leaks in scan
  bus/fslmc: fix per type device count
  bus/fslmc: simplify device parsing in scan
  bus/fslmc: refactor device filtering for multiprocess
  bus/fslmc: move unplug for some device out of VFIO
  bus/fslmc: call VFIO setup for some device from bus layer
  bus/fslmc: allocate interrupt during probing
  bus/fslmc: use generic cleanup

 doc/guides/rel_notes/release_26_11.rst   |   4 +
 drivers/bus/dpaa/dpaa_bus.c              | 134 +++++-----
 drivers/bus/fslmc/fslmc_bus.c            | 295 +++++++++++++++--------
 drivers/bus/fslmc/fslmc_vfio.c           | 255 +++++++-------------
 drivers/bus/fslmc/fslmc_vfio.h           |   5 +
 drivers/bus/fslmc/portal/dpaa2_hw_dpio.c |   1 +
 drivers/bus/fslmc/private.h              |   2 +
 7 files changed, 368 insertions(+), 328 deletions(-)

-- 
2.54.0

[RFC 01/11] drivers/bus: cleanup device freeing in NXP bus scan

[David Marchand]

Following the scan/probe refactoring, in the DPAA bus driver scan method,
the device list is empty at the time the pthread key object is allocated.
This leaves only one location that wants to release the whole device
list, so remove the dpaa_clean_device_list() helper.

Remove the same helper in FSLMC bus for consistency.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/bus/dpaa/dpaa_bus.c   | 15 ++-------------
 drivers/bus/fslmc/fslmc_bus.c | 21 ++++++---------------
 2 files changed, 8 insertions(+), 28 deletions(-)

diff --git a/drivers/bus/dpaa/dpaa_bus.c b/drivers/bus/dpaa/dpaa_bus.c
index 368c8eeb98..06962a5b29 100644
--- a/drivers/bus/dpaa/dpaa_bus.c
+++ b/drivers/bus/dpaa/dpaa_bus.c
@@ -193,8 +193,6 @@ dpaa_sec_available(void)
 	return -1;
 }
 
-static void dpaa_clean_device_list(void);
-
 static int
 dpaa_create_device_list(void)
 {
@@ -336,21 +334,13 @@ dpaa_create_device_list(void)
 	return 0;
 
 cleanup:
-	dpaa_clean_device_list();
-	return ret;
-}
-
-static void
-dpaa_clean_device_list(void)
-{
-	struct rte_dpaa_device *dev = NULL;
-
 	RTE_BUS_FOREACH_DEV(dev, &rte_dpaa_bus) {
 		rte_bus_remove_device(&rte_dpaa_bus, &dev->device);
 		rte_intr_instance_free(dev->intr_handle);
 		free(dev);
-		dev = NULL;
 	}
+
+	return ret;
 }
 
 RTE_EXPORT_INTERNAL_SYMBOL(rte_dpaa_portal_init)
@@ -699,7 +689,6 @@ rte_dpaa_bus_scan(void)
 	ret = pthread_key_create(&dpaa_portal_key, dpaa_portal_finish);
 	if (ret) {
 		DPAA_BUS_LOG(DEBUG, "Unable to create pthread key. (%d)", ret);
-		dpaa_clean_device_list();
 		return ret;
 	}
 
diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
index 3626b12316..4518f038c6 100644
--- a/drivers/bus/fslmc/fslmc_bus.c
+++ b/drivers/bus/fslmc/fslmc_bus.c
@@ -43,19 +43,6 @@ rte_fslmc_get_device_count(enum rte_dpaa2_dev_type device_type)
 	return fslmc_bus_device_count[device_type];
 }
 
-static void
-cleanup_fslmc_device_list(void)
-{
-	struct rte_dpaa2_device *dev;
-
-	RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus) {
-		rte_bus_remove_device(&rte_fslmc_bus, &dev->device);
-		rte_intr_instance_free(dev->intr_handle);
-		free(dev);
-		dev = NULL;
-	}
-}
-
 static int
 compare_dpaa2_devname(struct rte_dpaa2_device *dev1,
 		      struct rte_dpaa2_device *dev2)
@@ -305,6 +292,7 @@ fslmc_dev_compare(const char *name1, const char *name2)
 static int
 rte_fslmc_scan(void)
 {
+	struct rte_dpaa2_device *dev;
 	int ret;
 	char fslmc_dirpath[PATH_MAX];
 	DIR *dir;
@@ -314,7 +302,6 @@ rte_fslmc_scan(void)
 	char *group_name;
 
 	if (process_once) {
-		struct rte_dpaa2_device *dev;
 
 		DPAA2_BUS_DEBUG("Fslmc bus already scanned. Not rescanning");
 		RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus) {
@@ -416,7 +403,11 @@ rte_fslmc_scan(void)
 	closedir(dir);
 
 	/* Remove all devices in the list */
-	cleanup_fslmc_device_list();
+	RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus) {
+		rte_bus_remove_device(&rte_fslmc_bus, &dev->device);
+		rte_intr_instance_free(dev->intr_handle);
+		free(dev);
+	}
 scan_fail:
 	DPAA2_BUS_DEBUG("FSLMC Bus Not Available. Skipping (%d)", ret);
 	/* Irrespective of failure, scan only return success */
-- 
2.54.0

[RFC 02/11] bus/dpaa: allocate interrupt during probing

[David Marchand]

Allocating the interrupt handle is a waste of memory if no device is
probed later (like for example, if a allowlist is passed).

Instead, allocate this handle, set eventfds and vfio at the time
probe_device is called.

Adjust the unplug_device path accordingly.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/bus/dpaa/dpaa_bus.c | 76 +++++++++++++++++++------------------
 1 file changed, 40 insertions(+), 36 deletions(-)

diff --git a/drivers/bus/dpaa/dpaa_bus.c b/drivers/bus/dpaa/dpaa_bus.c
index 06962a5b29..b9d7256c29 100644
--- a/drivers/bus/dpaa/dpaa_bus.c
+++ b/drivers/bus/dpaa/dpaa_bus.c
@@ -215,16 +215,6 @@ dpaa_create_device_list(void)
 
 		dev->device.numa_node = SOCKET_ID_ANY;
 
-		/* Allocate interrupt handle instance */
-		dev->intr_handle =
-			rte_intr_instance_alloc(RTE_INTR_INSTANCE_F_PRIVATE);
-		if (dev->intr_handle == NULL) {
-			DPAA_BUS_LOG(ERR, "Failed to allocate intr handle");
-			ret = -ENOMEM;
-			free(dev);
-			goto cleanup;
-		}
-
 		cfg = &dpaa_netcfg->port_cfg[i];
 		fman_intf = cfg->fman_if;
 
@@ -276,16 +266,6 @@ dpaa_create_device_list(void)
 			goto cleanup;
 		}
 
-		/* Allocate interrupt handle instance */
-		dev->intr_handle =
-			rte_intr_instance_alloc(RTE_INTR_INSTANCE_F_PRIVATE);
-		if (dev->intr_handle == NULL) {
-			DPAA_BUS_LOG(ERR, "Failed to allocate intr handle");
-			ret = -ENOMEM;
-			free(dev);
-			goto cleanup;
-		}
-
 		dev->device_type = FSL_DPAA_CRYPTO;
 		dev->id.dev_id = dpaa_bus.device_count + i;
 
@@ -336,7 +316,6 @@ dpaa_create_device_list(void)
 cleanup:
 	RTE_BUS_FOREACH_DEV(dev, &rte_dpaa_bus) {
 		rte_bus_remove_device(&rte_dpaa_bus, &dev->device);
-		rte_intr_instance_free(dev->intr_handle);
 		free(dev);
 	}
 
@@ -637,7 +616,8 @@ rte_dpaa_bus_dev_build(void)
 	return 0;
 }
 
-static int rte_dpaa_setup_intr(struct rte_intr_handle *intr_handle)
+static int
+dpaa_setup_intr(struct rte_intr_handle *intr_handle)
 {
 	int fd;
 
@@ -657,13 +637,21 @@ static int rte_dpaa_setup_intr(struct rte_intr_handle *intr_handle)
 	return 0;
 }
 
+static void
+dpaa_close_intr(struct rte_intr_handle *intr_handle)
+{
+	if (rte_intr_fd_get(intr_handle) >= 0) {
+		close(rte_intr_fd_get(intr_handle));
+		rte_intr_fd_set(intr_handle, -1);
+	}
+}
+
 #define DPAA_DEV_PATH1 "/sys/devices/platform/soc/soc:fsl,dpaa"
 #define DPAA_DEV_PATH2 "/sys/devices/platform/fsl,dpaa"
 
 static int
 rte_dpaa_bus_scan(void)
 {
-	struct rte_dpaa_device *dev;
 	FILE *svr_file = NULL;
 	uint32_t svr_ver;
 	static int process_once;
@@ -752,14 +740,6 @@ rte_dpaa_bus_scan(void)
 	 */
 	rte_mbuf_set_platform_mempool_ops(DPAA_MEMPOOL_OPS_NAME);
 
-	RTE_BUS_FOREACH_DEV(dev, &rte_dpaa_bus) {
-		if (dev->device_type == FSL_DPAA_ETH) {
-			ret = rte_dpaa_setup_intr(dev->intr_handle);
-			if (ret)
-				DPAA_BUS_ERR("Error setting up interrupt.");
-		}
-	}
-
 	/* And initialize the PA->VA translation table */
 	dpaax_iova_table_populate();
 
@@ -787,10 +767,30 @@ dpaa_bus_probe_device(struct rte_driver *drv, struct rte_device *dev)
 	struct rte_dpaa_driver *dpaa_drv = RTE_BUS_DRIVER(drv, *dpaa_drv);
 	int ret;
 
+	/* Allocate interrupt handle instance */
+	dpaa_dev->intr_handle = rte_intr_instance_alloc(RTE_INTR_INSTANCE_F_PRIVATE);
+	if (dpaa_dev->intr_handle == NULL) {
+		DPAA_BUS_LOG(ERR, "Failed to allocate intr handle");
+		return -ENOMEM;
+	}
+
+	if (dpaa_dev->device_type == FSL_DPAA_ETH) {
+		ret = dpaa_setup_intr(dpaa_dev->intr_handle);
+		if (ret != 0) {
+			DPAA_BUS_ERR("error setting up interrupt: %s", dpaa_dev->name);
+			ret = -ret;
+			goto release_intr;
+		}
+	}
+
 	ret = dpaa_drv->probe(dpaa_drv, dpaa_dev);
-	if (ret != 0)
+	if (ret != 0) {
 		DPAA_BUS_ERR("unable to probe: %s", dpaa_dev->name);
-
+		dpaa_close_intr(dpaa_dev->intr_handle);
+release_intr:
+		rte_intr_instance_free(dpaa_dev->intr_handle);
+		dpaa_dev->intr_handle = NULL;
+	}
 	return ret;
 }
 
@@ -805,16 +805,20 @@ dpaa_bus_cleanup(struct rte_bus *bus)
 		int ret = 0;
 
 		if (!rte_dev_is_probed(&dev->device))
-			continue;
+			goto next;
 		drv = RTE_BUS_DRIVER(dev->device.driver, *drv);
 		if (drv->remove == NULL)
-			continue;
+			goto next;
 		ret = drv->remove(dev);
 		if (ret < 0) {
 			rte_errno = errno;
-			return -1;
+			goto next;
 		}
 		dev->device.driver = NULL;
+next:
+		dpaa_close_intr(dev->intr_handle);
+		rte_intr_instance_free(dev->intr_handle);
+		dev->intr_handle = NULL;
 	}
 	dpaa_portal_finish((void *)DPAA_PER_LCORE_PORTAL);
 	dpaa_bus_global_init = 0;
-- 
2.54.0

[RFC 03/11] bus/dpaa: support unplug and use generic cleanup

[David Marchand]

Add .unplug callback to handle driver removal and interrupt cleanup.
This enables use of the generic bus cleanup helper while preserving
bus-specific cleanup (portal finish, global init reset).

The cleanup function was already performing these operations, so it
seems safe to expose them through the unplug operation.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 doc/guides/rel_notes/release_26_11.rst |  4 ++
 drivers/bus/dpaa/dpaa_bus.c            | 57 +++++++++++++++-----------
 2 files changed, 38 insertions(+), 23 deletions(-)

diff --git a/doc/guides/rel_notes/release_26_11.rst b/doc/guides/rel_notes/release_26_11.rst
index 938617ca75..ceb3bf0616 100644
--- a/doc/guides/rel_notes/release_26_11.rst
+++ b/doc/guides/rel_notes/release_26_11.rst
@@ -55,6 +55,10 @@ New Features
      Also, make sure to start the actual text at the margin.
      =======================================================
 
+* **Added unplug operation support to DPAA bus.**
+
+  Implemented device unplug operation to allow runtime removal of DPAA devices.
+
 
 Removed Items
 -------------
diff --git a/drivers/bus/dpaa/dpaa_bus.c b/drivers/bus/dpaa/dpaa_bus.c
index b9d7256c29..3dd372e600 100644
--- a/drivers/bus/dpaa/dpaa_bus.c
+++ b/drivers/bus/dpaa/dpaa_bus.c
@@ -795,36 +795,45 @@ dpaa_bus_probe_device(struct rte_driver *drv, struct rte_device *dev)
 }
 
 static int
-dpaa_bus_cleanup(struct rte_bus *bus)
+dpaa_bus_unplug_device(struct rte_device *rte_dev)
 {
-	struct rte_dpaa_device *dev;
+	const struct rte_dpaa_driver *drv = RTE_BUS_DRIVER(rte_dev->driver, *drv);
+	struct rte_dpaa_device *dev = RTE_BUS_DEVICE(rte_dev, *dev);
+	int ret = 0;
 
-	BUS_INIT_FUNC_TRACE();
-	RTE_BUS_FOREACH_DEV(dev, bus) {
-		const struct rte_dpaa_driver *drv;
-		int ret = 0;
-
-		if (!rte_dev_is_probed(&dev->device))
-			goto next;
-		drv = RTE_BUS_DRIVER(dev->device.driver, *drv);
-		if (drv->remove == NULL)
-			goto next;
+	if (drv->remove != NULL) {
 		ret = drv->remove(dev);
-		if (ret < 0) {
-			rte_errno = errno;
-			goto next;
-		}
-		dev->device.driver = NULL;
-next:
-		dpaa_close_intr(dev->intr_handle);
-		rte_intr_instance_free(dev->intr_handle);
-		dev->intr_handle = NULL;
+		if (ret < 0)
+			return ret;
 	}
+
+	dpaa_close_intr(dev->intr_handle);
+	rte_intr_instance_free(dev->intr_handle);
+	dev->intr_handle = NULL;
+
+	return 0;
+}
+
+static void
+dpaa_bus_free_device(struct rte_device *dev)
+{
+	free(RTE_BUS_DEVICE(dev, struct rte_dpaa_device));
+}
+
+static int
+dpaa_bus_cleanup(struct rte_bus *bus)
+{
+	int ret;
+
+	BUS_INIT_FUNC_TRACE();
+
+	ret = rte_bus_generic_cleanup(bus);
+
 	dpaa_portal_finish((void *)DPAA_PER_LCORE_PORTAL);
 	dpaa_bus_global_init = 0;
 	DPAA_BUS_DEBUG("Bus cleanup done");
 
-	return 0;
+	return ret;
 }
 
 /* Adding destructor for double check in case non-gracefully
@@ -850,14 +859,16 @@ RTE_FINI_PRIO(dpaa_cleanup, 102)
 static struct rte_bus rte_dpaa_bus = {
 	.scan = rte_dpaa_bus_scan,
 	.probe = rte_bus_generic_probe,
+	.free_device = dpaa_bus_free_device,
+	.cleanup = dpaa_bus_cleanup,
 	.parse = rte_dpaa_bus_parse,
 	.dev_compare = dpaa_bus_dev_compare,
 	.find_device = rte_bus_generic_find_device,
 	.get_iommu_class = rte_dpaa_get_iommu_class,
 	.match = dpaa_bus_match,
 	.probe_device = dpaa_bus_probe_device,
+	.unplug_device = dpaa_bus_unplug_device,
 	.dev_iterate = rte_bus_generic_dev_iterate,
-	.cleanup = dpaa_bus_cleanup,
 };
 
 static struct rte_dpaa_bus_private dpaa_bus = {
-- 
2.54.0

[RFC 04/11] bus/fslmc: fix memory leaks in scan

[David Marchand]

During the scan phase (more precisely, during device filtering),
device pointers were leaked.

Add a simple helper so a device pointer is fully freed.

As a consequence of the filtering during scan, all blocklisted devices
are freed from the bus list during scan: there is no need for
reevaluating if a device is blocklisted during cleanup.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/bus/fslmc/fslmc_bus.c  | 15 ++++++++++-----
 drivers/bus/fslmc/fslmc_vfio.c | 33 +++++++++------------------------
 drivers/bus/fslmc/private.h    |  2 ++
 3 files changed, 21 insertions(+), 29 deletions(-)

diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
index 4518f038c6..94685a0f5a 100644
--- a/drivers/bus/fslmc/fslmc_bus.c
+++ b/drivers/bus/fslmc/fslmc_bus.c
@@ -85,6 +85,14 @@ insert_in_device_list(struct rte_dpaa2_device *newdev)
 		rte_bus_add_device(&rte_fslmc_bus, &newdev->device);
 }
 
+void
+fslmc_bus_remove_device(struct rte_dpaa2_device *dev)
+{
+	rte_bus_remove_device(&rte_fslmc_bus, &dev->device);
+	rte_intr_instance_free(dev->intr_handle);
+	free(dev);
+}
+
 static void
 dump_device_list(void)
 {
@@ -403,11 +411,8 @@ rte_fslmc_scan(void)
 	closedir(dir);
 
 	/* Remove all devices in the list */
-	RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus) {
-		rte_bus_remove_device(&rte_fslmc_bus, &dev->device);
-		rte_intr_instance_free(dev->intr_handle);
-		free(dev);
-	}
+	RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus)
+		fslmc_bus_remove_device(dev);
 scan_fail:
 	DPAA2_BUS_DEBUG("FSLMC Bus Not Available. Skipping (%d)", ret);
 	/* Irrespective of failure, scan only return success */
diff --git a/drivers/bus/fslmc/fslmc_vfio.c b/drivers/bus/fslmc/fslmc_vfio.c
index 412b70e5ae..3e45a8ed5c 100644
--- a/drivers/bus/fslmc/fslmc_vfio.c
+++ b/drivers/bus/fslmc/fslmc_vfio.c
@@ -1555,13 +1555,6 @@ fslmc_vfio_close_group(void)
 	}
 
 	RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus) {
-		if (dev->device.devargs &&
-		    dev->device.devargs->policy == RTE_DEV_BLOCKED) {
-			DPAA2_BUS_LOG(DEBUG, "%s Blacklisted, skipping",
-				      dev->device.name);
-			rte_bus_remove_device(&rte_fslmc_bus, &dev->device);
-				continue;
-		}
 		switch (dev->dev_type) {
 		case DPAA2_ETH:
 		case DPAA2_CRYPTO:
@@ -1623,8 +1616,7 @@ fslmc_vfio_process_group(void)
 			    dev->device.devargs->policy == RTE_DEV_BLOCKED) {
 				DPAA2_BUS_LOG(DEBUG, "%s Blocked, skipping",
 					      dev->device.name);
-				rte_bus_remove_device(&rte_fslmc_bus,
-						&dev->device);
+				fslmc_bus_remove_device(dev);
 				continue;
 			}
 
@@ -1632,8 +1624,7 @@ fslmc_vfio_process_group(void)
 			    !is_dpmcp_in_blocklist) {
 				if (dpmcp_count == 1 ||
 				    current_device != dpmcp_count) {
-					rte_bus_remove_device(&rte_fslmc_bus,
-						     &dev->device);
+					fslmc_bus_remove_device(dev);
 					continue;
 				}
 			}
@@ -1647,9 +1638,7 @@ fslmc_vfio_process_group(void)
 				found_mportal = 1;
 			}
 
-			rte_bus_remove_device(&rte_fslmc_bus, &dev->device);
-			free(dev);
-			dev = NULL;
+			fslmc_bus_remove_device(dev);
 			/* Ideally there is only a single dpmcp, but in case
 			 * multiple exists, looping on remaining devices.
 			 */
@@ -1673,7 +1662,7 @@ fslmc_vfio_process_group(void)
 				DPAA2_BUS_ERR("Unable to process dprc");
 				return ret;
 			}
-			rte_bus_remove_device(&rte_fslmc_bus, &dev->device);
+			fslmc_bus_remove_device(dev);
 		}
 	}
 
@@ -1685,7 +1674,7 @@ fslmc_vfio_process_group(void)
 		    dev->device.devargs->policy == RTE_DEV_BLOCKED) {
 			DPAA2_BUS_LOG(DEBUG, "%s Blocked, skipping",
 				      dev->device.name);
-			rte_bus_remove_device(&rte_fslmc_bus, &dev->device);
+			fslmc_bus_remove_device(dev);
 			continue;
 		}
 		if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
@@ -1693,7 +1682,7 @@ fslmc_vfio_process_group(void)
 		    dev->dev_type != DPAA2_CRYPTO &&
 		    dev->dev_type != DPAA2_QDMA &&
 		    dev->dev_type != DPAA2_IO) {
-			rte_bus_remove_device(&rte_fslmc_bus, &dev->device);
+			fslmc_bus_remove_device(dev);
 			continue;
 		}
 		switch (dev->dev_type) {
@@ -1735,14 +1724,12 @@ fslmc_vfio_process_group(void)
 			if (!is_dpio_in_blocklist && dpio_count > 1) {
 				if (rte_eal_process_type() == RTE_PROC_SECONDARY
 				    && current_device != dpio_count) {
-					rte_bus_remove_device(&rte_fslmc_bus,
-						     &dev->device);
+					fslmc_bus_remove_device(dev);
 					break;
 				}
 				if (rte_eal_process_type() == RTE_PROC_PRIMARY
 				    && current_device == dpio_count) {
-					rte_bus_remove_device(&rte_fslmc_bus,
-						     &dev->device);
+					fslmc_bus_remove_device(dev);
 					break;
 				}
 			}
@@ -1760,9 +1747,7 @@ fslmc_vfio_process_group(void)
 			/* Unknown - ignore */
 			DPAA2_BUS_DEBUG("Found unknown device (%s)",
 					dev->device.name);
-			rte_bus_remove_device(&rte_fslmc_bus, &dev->device);
-			free(dev);
-			dev = NULL;
+			fslmc_bus_remove_device(dev);
 		}
 	}
 
diff --git a/drivers/bus/fslmc/private.h b/drivers/bus/fslmc/private.h
index 20a454c3fc..825a364f1b 100644
--- a/drivers/bus/fslmc/private.h
+++ b/drivers/bus/fslmc/private.h
@@ -11,4 +11,6 @@
 
 extern struct rte_bus rte_fslmc_bus;
 
+void fslmc_bus_remove_device(struct rte_dpaa2_device *dev);
+
 #endif /* BUS_FSLMC_PRIVATE_H */
-- 
2.54.0

[RFC 05/11] bus/fslmc: fix per type device count

[David Marchand]

When blocklisting some device, the per type count was wrong.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/bus/fslmc/fslmc_bus.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
index 94685a0f5a..a7ac59173f 100644
--- a/drivers/bus/fslmc/fslmc_bus.c
+++ b/drivers/bus/fslmc/fslmc_bus.c
@@ -83,11 +83,17 @@ insert_in_device_list(struct rte_dpaa2_device *newdev)
 
 	if (!inserted)
 		rte_bus_add_device(&rte_fslmc_bus, &newdev->device);
+
+	if (newdev->dev_type < DPAA2_DEVTYPE_MAX)
+		fslmc_bus_device_count[newdev->dev_type]++;
 }
 
 void
 fslmc_bus_remove_device(struct rte_dpaa2_device *dev)
 {
+	if (dev->dev_type < DPAA2_DEVTYPE_MAX)
+		fslmc_bus_device_count[dev->dev_type]--;
+
 	rte_bus_remove_device(&rte_fslmc_bus, &dev->device);
 	rte_intr_instance_free(dev->intr_handle);
 	free(dev);
@@ -195,9 +201,6 @@ scan_one_fslmc_device(char *dev_name)
 	}
 	dev->device.devargs = rte_bus_find_devargs(&rte_fslmc_bus, dev_name);
 
-	/* Update the device found into the device_count table */
-	fslmc_bus_device_count[dev->dev_type]++;
-
 	/* Add device in the fslmc device list */
 	insert_in_device_list(dev);
 
-- 
2.54.0

[RFC 06/11] bus/fslmc: simplify device parsing in scan

[David Marchand]

Refactor string manipulations and avoid unneeded allocation.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/bus/fslmc/fslmc_bus.c | 78 +++++++++++++----------------------
 1 file changed, 28 insertions(+), 50 deletions(-)

diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
index a7ac59173f..4f6e0bf387 100644
--- a/drivers/bus/fslmc/fslmc_bus.c
+++ b/drivers/bus/fslmc/fslmc_bus.c
@@ -116,18 +116,33 @@ dump_device_list(void)
 static int
 scan_one_fslmc_device(char *dev_name)
 {
-	char *dup_dev_name, *t_ptr;
+	enum rte_dpaa2_dev_type dev_type = DPAA2_UNKNOWN;
 	struct rte_dpaa2_device *dev = NULL;
+	struct {
+		const char *prefix;
+		enum rte_dpaa2_dev_type type;
+	} dev_types[] = {
+		{ "dpni.", DPAA2_ETH },
+		{ "dpseci.", DPAA2_CRYPTO },
+		{ "dpcon.", DPAA2_CON },
+		{ "dpbp.", DPAA2_BPOOL },
+		{ "dpio.", DPAA2_IO },
+		{ "dpci.", DPAA2_CI },
+		{ "dpmcp.", DPAA2_MPORTAL },
+		{ "dpdmai.", DPAA2_QDMA },
+		{ "dpdmux.", DPAA2_MUX },
+		{ "dprtc.", DPAA2_DPRTC },
+		{ "dprc.", DPAA2_DPRC },
+	};
+	char *dev_id = NULL;
 	int ret = -1;
 
-	if (!dev_name)
-		return ret;
-
-	/* Creating a temporary copy to perform cut-parse over string */
-	dup_dev_name = strdup(dev_name);
-	if (!dup_dev_name) {
-		DPAA2_BUS_ERR("Unable to allocate device name memory");
-		return -ENOMEM;
+	for (unsigned int i = 0; i < RTE_DIM(dev_types); i++) {
+		if (strncmp(dev_types[i].prefix, dev_name, strlen(dev_types[i].prefix)) != 0)
+			continue;
+		dev_id = dev_name + strlen(dev_types[i].prefix);
+		dev_type = dev_types[i].type;
+		break;
 	}
 
 	/* For all other devices, we allocate rte_dpaa2_device.
@@ -138,11 +153,11 @@ scan_one_fslmc_device(char *dev_name)
 	dev = calloc(1, sizeof(struct rte_dpaa2_device));
 	if (!dev) {
 		DPAA2_BUS_ERR("Unable to allocate device object");
-		free(dup_dev_name);
 		return -ENOMEM;
 	}
 
 	dev->device.numa_node = SOCKET_ID_ANY;
+	dev->dev_type = dev_type;
 
 	/* Allocate interrupt instance */
 	dev->intr_handle =
@@ -153,46 +168,13 @@ scan_one_fslmc_device(char *dev_name)
 		goto cleanup;
 	}
 
-	/* Parse the device name and ID */
-	t_ptr = strtok(dup_dev_name, ".");
-	if (!t_ptr) {
-		DPAA2_BUS_ERR("Invalid device found: (%s)", dup_dev_name);
-		ret = -EINVAL;
-		goto cleanup;
-	}
-	if (!strncmp("dpni", t_ptr, 4))
-		dev->dev_type = DPAA2_ETH;
-	else if (!strncmp("dpseci", t_ptr, 6))
-		dev->dev_type = DPAA2_CRYPTO;
-	else if (!strncmp("dpcon", t_ptr, 5))
-		dev->dev_type = DPAA2_CON;
-	else if (!strncmp("dpbp", t_ptr, 4))
-		dev->dev_type = DPAA2_BPOOL;
-	else if (!strncmp("dpio", t_ptr, 4))
-		dev->dev_type = DPAA2_IO;
-	else if (!strncmp("dpci", t_ptr, 4))
-		dev->dev_type = DPAA2_CI;
-	else if (!strncmp("dpmcp", t_ptr, 5))
-		dev->dev_type = DPAA2_MPORTAL;
-	else if (!strncmp("dpdmai", t_ptr, 6))
-		dev->dev_type = DPAA2_QDMA;
-	else if (!strncmp("dpdmux", t_ptr, 6))
-		dev->dev_type = DPAA2_MUX;
-	else if (!strncmp("dprtc", t_ptr, 5))
-		dev->dev_type = DPAA2_DPRTC;
-	else if (!strncmp("dprc", t_ptr, 4))
-		dev->dev_type = DPAA2_DPRC;
-	else
-		dev->dev_type = DPAA2_UNKNOWN;
-
-	t_ptr = strtok(NULL, ".");
-	if (!t_ptr) {
-		DPAA2_BUS_ERR("Skipping invalid device (%s)", dup_dev_name);
+	if (dev_id == NULL) {
+		DPAA2_BUS_ERR("Skipping invalid device (%s)", dev_name);
 		ret = 0;
 		goto cleanup;
 	}
 
-	sscanf(t_ptr, "%hu", &dev->object_id);
+	sscanf(dev_id, "%hu", &dev->object_id);
 	dev->device.name = strdup(dev_name);
 	if (!dev->device.name) {
 		DPAA2_BUS_ERR("Unable to clone device name. Out of memory");
@@ -204,12 +186,8 @@ scan_one_fslmc_device(char *dev_name)
 	/* Add device in the fslmc device list */
 	insert_in_device_list(dev);
 
-	/* Don't need the duplicated device filesystem entry anymore */
-	free(dup_dev_name);
-
 	return 0;
 cleanup:
-	free(dup_dev_name);
 	if (dev) {
 		rte_intr_instance_free(dev->intr_handle);
 		free(dev);
-- 
2.54.0

[RFC 07/11] bus/fslmc: refactor device filtering for multiprocess

[David Marchand]

Refactor device filtering logic by splitting what was done in
fslmc_vfio_process_group().

During scan, do not add device to the bus list unless needed
(wrt secondary process, blocklist ...). Yet, keep a special
case for MPORTAL/DPIO objects and filter them in a new
fslmc_filter_control_devices() helper.

This helper is also responsible for selecting the right MPORTAL/DPIO
objects depending on primary/secondary considerations.

As a consequence, fslmc_vfio_process_group() only handles IO
device init without having to care about skipping some device.

Finally, remove now dead code in fslmc_vfio_close_group() since
only used devices are left in the bus device list.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/bus/fslmc/fslmc_bus.c  | 134 ++++++++++++++++++++++++++---
 drivers/bus/fslmc/fslmc_vfio.c | 150 +++++----------------------------
 2 files changed, 139 insertions(+), 145 deletions(-)

diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
index 4f6e0bf387..4b80948aee 100644
--- a/drivers/bus/fslmc/fslmc_bus.c
+++ b/drivers/bus/fslmc/fslmc_bus.c
@@ -135,7 +135,6 @@ scan_one_fslmc_device(char *dev_name)
 		{ "dprc.", DPAA2_DPRC },
 	};
 	char *dev_id = NULL;
-	int ret = -1;
 
 	for (unsigned int i = 0; i < RTE_DIM(dev_types); i++) {
 		if (strncmp(dev_types[i].prefix, dev_name, strlen(dev_types[i].prefix)) != 0)
@@ -145,11 +144,37 @@ scan_one_fslmc_device(char *dev_name)
 		break;
 	}
 
-	/* For all other devices, we allocate rte_dpaa2_device.
-	 * For those devices where there is no driver, probe would release
-	 * the memory associated with the rte_dpaa2_device after necessary
-	 * initialization.
+	if (dev_id == NULL) {
+		DPAA2_BUS_ERR("Skipping invalid device (%s)", dev_name);
+		return 0;
+	}
+
+	/*
+	 * DPAA2_MPORTAL and DPAA2_IO types are handled separately,
+	 * see fslmc_filter_control_devices()
 	 */
+	if (rte_bus_device_is_ignored(&rte_fslmc_bus, dev_name) &&
+			dev_type != DPAA2_MPORTAL && dev_type != DPAA2_IO) {
+		DPAA2_BUS_DEBUG("Skipping blocklisted device (%s)", dev_name);
+		return 0;
+	}
+
+	/* For secondary processes, control objects are not needed */
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
+		switch (dev_type) {
+		case DPAA2_ETH:
+		case DPAA2_CRYPTO:
+		case DPAA2_QDMA:
+		case DPAA2_IO:
+		case DPAA2_MPORTAL:
+		case DPAA2_DPRC:
+			break;
+		default:
+			DPAA2_BUS_DEBUG("Skipping device in secondary process (%s)", dev_name);
+			return 0;
+		}
+	}
+
 	dev = calloc(1, sizeof(struct rte_dpaa2_device));
 	if (!dev) {
 		DPAA2_BUS_ERR("Unable to allocate device object");
@@ -164,13 +189,6 @@ scan_one_fslmc_device(char *dev_name)
 		rte_intr_instance_alloc(RTE_INTR_INSTANCE_F_PRIVATE);
 	if (dev->intr_handle == NULL) {
 		DPAA2_BUS_ERR("Failed to allocate intr handle");
-		ret = -ENOMEM;
-		goto cleanup;
-	}
-
-	if (dev_id == NULL) {
-		DPAA2_BUS_ERR("Skipping invalid device (%s)", dev_name);
-		ret = 0;
 		goto cleanup;
 	}
 
@@ -178,7 +196,6 @@ scan_one_fslmc_device(char *dev_name)
 	dev->device.name = strdup(dev_name);
 	if (!dev->device.name) {
 		DPAA2_BUS_ERR("Unable to clone device name. Out of memory");
-		ret = -ENOMEM;
 		goto cleanup;
 	}
 	dev->device.devargs = rte_bus_find_devargs(&rte_fslmc_bus, dev_name);
@@ -192,7 +209,7 @@ scan_one_fslmc_device(char *dev_name)
 		rte_intr_instance_free(dev->intr_handle);
 		free(dev);
 	}
-	return ret;
+	return -ENOMEM;
 }
 
 static int
@@ -278,6 +295,91 @@ fslmc_dev_compare(const char *name1, const char *name2)
 	return strncmp(devname1, devname2, sizeof(devname1));
 }
 
+static int
+fslmc_filter_control_devices(void)
+{
+	bool is_dpmcp_in_blocklist = false, is_dpio_in_blocklist = false;
+	int dpmcp_count = 0, dpio_count = 0;
+	struct rte_dpaa2_device *dev;
+
+	/* Track MPORTAL/DPIO blocklists */
+	RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus) {
+		if (dev->dev_type != DPAA2_MPORTAL && dev->dev_type != DPAA2_IO)
+			continue;
+		if (rte_bus_device_is_ignored(&rte_fslmc_bus, rte_dev_name(&dev->device))) {
+			DPAA2_BUS_LOG(DEBUG, "%s Blocked, skipping", dev->device.name);
+			if (dev->dev_type == DPAA2_MPORTAL)
+				is_dpmcp_in_blocklist = true;
+			else if (dev->dev_type == DPAA2_IO)
+				is_dpio_in_blocklist = true;
+			fslmc_bus_remove_device(dev);
+			continue;
+		}
+		if (dev->dev_type == DPAA2_MPORTAL)
+			dpmcp_count++;
+		else if (dev->dev_type == DPAA2_IO)
+			dpio_count++;
+	}
+
+	if (dpmcp_count == 0) {
+		DPAA2_BUS_ERR("No MC Portal device found");
+		return -ENODEV;
+	}
+
+	/* Automatic MPORTAL split: primary keeps first, secondary keeps last */
+	if (!is_dpmcp_in_blocklist) {
+		int current_device = 0;
+		int keep_index;
+
+		/* Check MPORTAL availability for secondary */
+		if (rte_eal_process_type() == RTE_PROC_SECONDARY && dpmcp_count < 2) {
+			DPAA2_BUS_ERR("No MC Portal device found for secondary");
+			return -ENODEV;
+		}
+
+		if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+			keep_index = 0;
+		else
+			keep_index = dpmcp_count - 1;
+
+		RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus) {
+			if (dev->dev_type != DPAA2_MPORTAL)
+				continue;
+			if (current_device != keep_index)
+				fslmc_bus_remove_device(dev);
+
+			current_device++;
+			if (current_device == dpmcp_count)
+				break;
+		}
+	}
+
+	/* Automatic DPIO split: secondary keeps last only, primary removes last */
+	if (!is_dpio_in_blocklist) {
+		int last_index = dpio_count - 1;
+		int current_device = 0;
+
+		RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus) {
+			if (dev->dev_type != DPAA2_IO)
+				continue;
+
+			if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
+					current_device != last_index) {
+				fslmc_bus_remove_device(dev);
+			} else if (rte_eal_process_type() == RTE_PROC_PRIMARY &&
+					current_device == last_index) {
+				fslmc_bus_remove_device(dev);
+			}
+
+			current_device++;
+			if (current_device == dpio_count)
+				break;
+		}
+	}
+
+	return 0;
+}
+
 static int
 rte_fslmc_scan(void)
 {
@@ -377,6 +479,10 @@ rte_fslmc_scan(void)
 			}
 		}
 
+		ret = fslmc_filter_control_devices();
+		if (ret)
+			return 0;
+
 		ret = fslmc_vfio_process_group();
 		if (ret) {
 			DPAA2_BUS_ERR("Unable to setup devices %d", ret);
diff --git a/drivers/bus/fslmc/fslmc_vfio.c b/drivers/bus/fslmc/fslmc_vfio.c
index 3e45a8ed5c..d67232c31b 100644
--- a/drivers/bus/fslmc/fslmc_vfio.c
+++ b/drivers/bus/fslmc/fslmc_vfio.c
@@ -1560,15 +1560,10 @@ fslmc_vfio_close_group(void)
 		case DPAA2_CRYPTO:
 		case DPAA2_QDMA:
 		case DPAA2_IO:
-			fslmc_close_iodevices(dev, vfio_group_fd);
-			break;
 		case DPAA2_CON:
 		case DPAA2_CI:
 		case DPAA2_BPOOL:
 		case DPAA2_MUX:
-			if (rte_eal_process_type() == RTE_PROC_SECONDARY)
-				continue;
-
 			fslmc_close_iodevices(dev, vfio_group_fd);
 			break;
 		case DPAA2_DPRTC:
@@ -1586,154 +1581,48 @@ fslmc_vfio_close_group(void)
 int
 fslmc_vfio_process_group(void)
 {
-	int ret;
-	int found_mportal = 0;
 	struct rte_dpaa2_device *dev;
-	bool is_dpmcp_in_blocklist = false, is_dpio_in_blocklist = false;
-	int dpmcp_count = 0, dpio_count = 0, current_device;
-
-	RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus) {
-		if (dev->dev_type == DPAA2_MPORTAL) {
-			dpmcp_count++;
-			if (dev->device.devargs &&
-			    dev->device.devargs->policy == RTE_DEV_BLOCKED)
-				is_dpmcp_in_blocklist = true;
-		}
-		if (dev->dev_type == DPAA2_IO) {
-			dpio_count++;
-			if (dev->device.devargs &&
-			    dev->device.devargs->policy == RTE_DEV_BLOCKED)
-				is_dpio_in_blocklist = true;
-		}
-	}
+	int ret;
 
-	/* Search the MCP as that should be initialized first. */
-	current_device = 0;
+	/* Process MPORTAL - should be initialized first */
 	RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus) {
-		if (dev->dev_type == DPAA2_MPORTAL) {
-			current_device++;
-			if (dev->device.devargs &&
-			    dev->device.devargs->policy == RTE_DEV_BLOCKED) {
-				DPAA2_BUS_LOG(DEBUG, "%s Blocked, skipping",
-					      dev->device.name);
-				fslmc_bus_remove_device(dev);
-				continue;
-			}
-
-			if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
-			    !is_dpmcp_in_blocklist) {
-				if (dpmcp_count == 1 ||
-				    current_device != dpmcp_count) {
-					fslmc_bus_remove_device(dev);
-					continue;
-				}
-			}
-
-			if (!found_mportal) {
-				ret = fslmc_process_mcp(dev);
-				if (ret) {
-					DPAA2_BUS_ERR("Unable to map MC Portal");
-					return ret;
-				}
-				found_mportal = 1;
-			}
-
-			fslmc_bus_remove_device(dev);
-			/* Ideally there is only a single dpmcp, but in case
-			 * multiple exists, looping on remaining devices.
-			 */
+		if (dev->dev_type != DPAA2_MPORTAL)
+			continue;
+		ret = fslmc_process_mcp(dev);
+		if (ret) {
+			DPAA2_BUS_ERR("Unable to map MC Portal");
+			return ret;
 		}
+		fslmc_bus_remove_device(dev);
+		break;
 	}
 
-	/* Cannot continue if there is not even a single mportal */
-	if (!found_mportal) {
-		DPAA2_BUS_ERR("No MC Portal device found. Not continuing");
-		return -EIO;
-	}
-
-	/* Search for DPRC device next as it updates endpoint of
+	/* Process DPRC device next as it updates endpoint of
 	 * other devices.
 	 */
-	current_device = 0;
 	RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus) {
-		if (dev->dev_type == DPAA2_DPRC) {
-			ret = fslmc_process_iodevices(dev);
-			if (ret) {
-				DPAA2_BUS_ERR("Unable to process dprc");
-				return ret;
-			}
-			fslmc_bus_remove_device(dev);
+		if (dev->dev_type != DPAA2_DPRC)
+			continue;
+		ret = fslmc_process_iodevices(dev);
+		if (ret) {
+			DPAA2_BUS_ERR("Unable to process dprc");
+			return ret;
 		}
+		fslmc_bus_remove_device(dev);
 	}
 
-	current_device = 0;
+	/* Process remaining devices */
 	RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus) {
-		if (dev->dev_type == DPAA2_IO)
-			current_device++;
-		if (dev->device.devargs &&
-		    dev->device.devargs->policy == RTE_DEV_BLOCKED) {
-			DPAA2_BUS_LOG(DEBUG, "%s Blocked, skipping",
-				      dev->device.name);
-			fslmc_bus_remove_device(dev);
-			continue;
-		}
-		if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
-		    dev->dev_type != DPAA2_ETH &&
-		    dev->dev_type != DPAA2_CRYPTO &&
-		    dev->dev_type != DPAA2_QDMA &&
-		    dev->dev_type != DPAA2_IO) {
-			fslmc_bus_remove_device(dev);
-			continue;
-		}
 		switch (dev->dev_type) {
 		case DPAA2_ETH:
 		case DPAA2_CRYPTO:
 		case DPAA2_QDMA:
-			ret = fslmc_process_iodevices(dev);
-			if (ret) {
-				DPAA2_BUS_DEBUG("Dev (%s) init failed",
-						dev->device.name);
-				return ret;
-			}
-			break;
 		case DPAA2_CON:
 		case DPAA2_CI:
 		case DPAA2_BPOOL:
 		case DPAA2_DPRTC:
 		case DPAA2_MUX:
-			/* IN case of secondary processes, all control objects
-			 * like dpbp, dpcon, dpci are not initialized/required
-			 * - all of these are assumed to be initialized and made
-			 *   available by primary.
-			 */
-			if (rte_eal_process_type() == RTE_PROC_SECONDARY)
-				continue;
-
-			/* Call the object creation routine and remove the
-			 * device entry from device list
-			 */
-			ret = fslmc_process_iodevices(dev);
-			if (ret) {
-				DPAA2_BUS_DEBUG("Dev (%s) init failed",
-						dev->device.name);
-				return ret;
-			}
-
-			break;
 		case DPAA2_IO:
-			if (!is_dpio_in_blocklist && dpio_count > 1) {
-				if (rte_eal_process_type() == RTE_PROC_SECONDARY
-				    && current_device != dpio_count) {
-					fslmc_bus_remove_device(dev);
-					break;
-				}
-				if (rte_eal_process_type() == RTE_PROC_PRIMARY
-				    && current_device == dpio_count) {
-					fslmc_bus_remove_device(dev);
-					break;
-				}
-			}
-
 			ret = fslmc_process_iodevices(dev);
 			if (ret) {
 				DPAA2_BUS_DEBUG("Dev (%s) init failed",
@@ -1742,7 +1631,6 @@ fslmc_vfio_process_group(void)
 			}
 
 			break;
-		case DPAA2_UNKNOWN:
 		default:
 			/* Unknown - ignore */
 			DPAA2_BUS_DEBUG("Found unknown device (%s)",
-- 
2.54.0

[RFC 08/11] bus/fslmc: move unplug for some device out of VFIO

[David Marchand]

Move device unplug out of the code dealing with VFIO, interrupts and
IO devices.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/bus/fslmc/fslmc_bus.c  | 15 ++++++++++++++-
 drivers/bus/fslmc/fslmc_vfio.c | 10 ----------
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
index 4b80948aee..a03ef6051a 100644
--- a/drivers/bus/fslmc/fslmc_bus.c
+++ b/drivers/bus/fslmc/fslmc_bus.c
@@ -519,10 +519,23 @@ fslmc_bus_match(const struct rte_driver *drv, const struct rte_device *dev)
 }
 
 static int
-rte_fslmc_close(struct rte_bus *bus __rte_unused)
+fslmc_bus_unplug_device(struct rte_device *rte_dev);
+
+static int
+rte_fslmc_close(struct rte_bus *bus)
 {
+	struct rte_dpaa2_device *dev;
 	int ret = 0;
 
+	RTE_BUS_FOREACH_DEV(dev, bus) {
+		if (dev->dev_type != DPAA2_ETH &&
+		    dev->dev_type != DPAA2_CRYPTO &&
+		    dev->dev_type != DPAA2_QDMA)
+			continue;
+		if (rte_dev_is_probed(&dev->device) && fslmc_bus_unplug_device(&dev->device))
+			DPAA2_BUS_ERR("Unable to remove %s", dev->device.name);
+	}
+
 	ret = fslmc_vfio_close_group();
 	if (ret)
 		DPAA2_BUS_INFO("Unable to close devices %d", ret);
diff --git a/drivers/bus/fslmc/fslmc_vfio.c b/drivers/bus/fslmc/fslmc_vfio.c
index d67232c31b..5720f366db 100644
--- a/drivers/bus/fslmc/fslmc_vfio.c
+++ b/drivers/bus/fslmc/fslmc_vfio.c
@@ -1392,7 +1392,6 @@ fslmc_close_iodevices(struct rte_dpaa2_device *dev,
 	int vfio_fd)
 {
 	struct rte_dpaa2_object *object = NULL;
-	const struct rte_dpaa2_driver *drv;
 	int ret;
 
 	switch (dev->dev_type) {
@@ -1408,15 +1407,6 @@ fslmc_close_iodevices(struct rte_dpaa2_device *dev,
 				continue;
 		}
 		break;
-	case DPAA2_ETH:
-	case DPAA2_CRYPTO:
-	case DPAA2_QDMA:
-		if (dev->device.driver != NULL) {
-			drv = RTE_BUS_DRIVER(dev->device.driver, *drv);
-			if (drv->remove && drv->remove(dev))
-				DPAA2_BUS_ERR("Unable to remove");
-		}
-		break;
 	default:
 		break;
 	}
-- 
2.54.0

[RFC 09/11] bus/fslmc: call VFIO setup for some device from bus layer

[David Marchand]

Move VFIO device and interrupt setup for ETH, CRYPTO, and QDMA
devices from fslmc_vfio.c to fslmc_bus.c scan layer.

Add fslmc_vfio_dev_setup() (resp. fslmc_vfio_dev_close()) wrapper
that handles VFIO device fd acquisition and interrupt setup
(resp. device removal), hiding VFIO internals from the bus layer.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/bus/fslmc/fslmc_bus.c  | 13 ++++++
 drivers/bus/fslmc/fslmc_vfio.c | 80 ++++++++++++++++++++++++++++++----
 drivers/bus/fslmc/fslmc_vfio.h |  5 +++
 3 files changed, 90 insertions(+), 8 deletions(-)

diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
index a03ef6051a..a1bc7d216c 100644
--- a/drivers/bus/fslmc/fslmc_bus.c
+++ b/drivers/bus/fslmc/fslmc_bus.c
@@ -488,6 +488,18 @@ rte_fslmc_scan(void)
 			DPAA2_BUS_ERR("Unable to setup devices %d", ret);
 			return 0;
 		}
+
+		RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus) {
+			if (dev->dev_type != DPAA2_ETH &&
+			    dev->dev_type != DPAA2_CRYPTO &&
+			    dev->dev_type != DPAA2_QDMA)
+				continue;
+			ret = fslmc_vfio_dev_setup(dev);
+			if (ret) {
+				DPAA2_BUS_ERR("Dev (%s) VFIO setup failed", dev->device.name);
+				return 0;
+			}
+		}
 	}
 
 	process_once = 1;
@@ -534,6 +546,7 @@ rte_fslmc_close(struct rte_bus *bus)
 			continue;
 		if (rte_dev_is_probed(&dev->device) && fslmc_bus_unplug_device(&dev->device))
 			DPAA2_BUS_ERR("Unable to remove %s", dev->device.name);
+		fslmc_vfio_dev_close(dev);
 	}
 
 	ret = fslmc_vfio_close_group();
diff --git a/drivers/bus/fslmc/fslmc_vfio.c b/drivers/bus/fslmc/fslmc_vfio.c
index 5720f366db..9655c27ca8 100644
--- a/drivers/bus/fslmc/fslmc_vfio.c
+++ b/drivers/bus/fslmc/fslmc_vfio.c
@@ -1387,6 +1387,15 @@ rte_dpaa2_vfio_setup_intr(struct rte_intr_handle *intr_handle,
 	return -EIO;
 }
 
+static void
+dpaa2_close_intr(struct rte_intr_handle *intr_handle)
+{
+	if (rte_intr_fd_get(intr_handle) >= 0) {
+		close(rte_intr_fd_get(intr_handle));
+		rte_intr_fd_set(intr_handle, -1);
+	}
+}
+
 static void
 fslmc_close_iodevices(struct rte_dpaa2_device *dev,
 	int vfio_fd)
@@ -1394,6 +1403,10 @@ fslmc_close_iodevices(struct rte_dpaa2_device *dev,
 	struct rte_dpaa2_object *object = NULL;
 	int ret;
 
+	RTE_VERIFY(dev->dev_type != DPAA2_CRYPTO);
+	RTE_VERIFY(dev->dev_type != DPAA2_ETH);
+	RTE_VERIFY(dev->dev_type != DPAA2_QDMA);
+
 	switch (dev->dev_type) {
 	case DPAA2_IO:
 	case DPAA2_CON:
@@ -1421,8 +1434,7 @@ fslmc_close_iodevices(struct rte_dpaa2_device *dev,
 }
 
 /*
- * fslmc_process_iodevices for processing only IO (ETH, CRYPTO, and possibly
- * EVENT) devices.
+ * fslmc_process_iodevices for processing only IO devices.
  */
 static int
 fslmc_process_iodevices(struct rte_dpaa2_device *dev)
@@ -1437,12 +1449,6 @@ fslmc_process_iodevices(struct rte_dpaa2_device *dev)
 		return ret;
 
 	switch (dev->dev_type) {
-	case DPAA2_ETH:
-		ret = rte_dpaa2_vfio_setup_intr(dev->intr_handle, dev_fd,
-				device_info.num_irqs);
-		if (ret)
-			return ret;
-		break;
 	case DPAA2_CON:
 	case DPAA2_IO:
 	case DPAA2_CI:
@@ -1466,6 +1472,60 @@ fslmc_process_iodevices(struct rte_dpaa2_device *dev)
 	return 0;
 }
 
+int
+fslmc_vfio_dev_setup(struct rte_dpaa2_device *dev)
+{
+	int dev_fd = -1;
+	int ret;
+	struct vfio_device_info device_info = { .argsz = sizeof(device_info) };
+
+	ret = fslmc_vfio_setup_device(dev->device.name, &dev_fd, &device_info);
+	if (ret) {
+		DPAA2_BUS_ERR("VFIO setup failed for %s: %d",
+			      dev->device.name, ret);
+		return ret;
+	}
+
+	if (dev->dev_type == DPAA2_ETH) {
+		ret = rte_dpaa2_vfio_setup_intr(dev->intr_handle, dev_fd,
+						device_info.num_irqs);
+		if (ret) {
+			DPAA2_BUS_ERR("Interrupt setup failed for %s: %d",
+				      dev->device.name, ret);
+			return ret;
+		}
+	}
+
+	DPAA2_BUS_DEBUG("Device (%s) VFIO setup completed", dev->device.name);
+	return 0;
+}
+
+int
+fslmc_vfio_dev_close(struct rte_dpaa2_device *dev)
+{
+	int vfio_group_fd;
+	int ret;
+	const char *group_name = fslmc_vfio_get_group_name();
+
+	vfio_group_fd = fslmc_vfio_group_fd_by_name(group_name);
+	if (vfio_group_fd <= 0) {
+		DPAA2_BUS_ERR("Get fd by name(%s) failed(%d)",
+			      group_name, vfio_group_fd);
+		if (vfio_group_fd < 0)
+			return vfio_group_fd;
+		return -EIO;
+	}
+
+	dpaa2_close_intr(dev->intr_handle);
+
+	ret = fslmc_vfio_group_remove_dev(vfio_group_fd, dev->device.name);
+	if (ret)
+		DPAA2_BUS_ERR("Failed to remove %s from vfio", dev->device.name);
+
+	DPAA2_BUS_DEBUG("Device (%s) closed", dev->device.name);
+	return ret;
+}
+
 static int
 fslmc_process_mcp(struct rte_dpaa2_device *dev)
 {
@@ -1549,6 +1609,8 @@ fslmc_vfio_close_group(void)
 		case DPAA2_ETH:
 		case DPAA2_CRYPTO:
 		case DPAA2_QDMA:
+			/* ethdev, cryptodev, dmadev are handled at the bus level */
+			break;
 		case DPAA2_IO:
 		case DPAA2_CON:
 		case DPAA2_CI:
@@ -1607,6 +1669,8 @@ fslmc_vfio_process_group(void)
 		case DPAA2_ETH:
 		case DPAA2_CRYPTO:
 		case DPAA2_QDMA:
+			/* ethdev, cryptodev, dmadev are handled at the bus level */
+			break;
 		case DPAA2_CON:
 		case DPAA2_CI:
 		case DPAA2_BPOOL:
diff --git a/drivers/bus/fslmc/fslmc_vfio.h b/drivers/bus/fslmc/fslmc_vfio.h
index c995fd67b8..530aaa0a32 100644
--- a/drivers/bus/fslmc/fslmc_vfio.h
+++ b/drivers/bus/fslmc/fslmc_vfio.h
@@ -60,4 +60,9 @@ int fslmc_vfio_close_group(void);
 char *fslmc_get_container(void);
 int fslmc_get_container_group(const char *group_name, int *gropuid);
 int fslmc_vfio_dmamap(void);
+
+struct rte_dpaa2_device;
+int fslmc_vfio_dev_setup(struct rte_dpaa2_device *dev);
+int fslmc_vfio_dev_close(struct rte_dpaa2_device *dev);
+
 #endif /* _FSLMC_VFIO_H_ */
-- 
2.54.0

[RFC 10/11] bus/fslmc: allocate interrupt during probing

[David Marchand]

Allocating the interrupt handle is a waste of memory if no device is
probed later (like for example, if a allowlist is passed).

Instead, allocate this handle, set eventfds and vfio at the time
probe_device is called.

Adjust the unplug_device path accordingly.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/bus/fslmc/fslmc_bus.c            | 54 ++++++++++--------------
 drivers/bus/fslmc/portal/dpaa2_hw_dpio.c |  1 +
 2 files changed, 24 insertions(+), 31 deletions(-)

diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
index a1bc7d216c..39afdee9ee 100644
--- a/drivers/bus/fslmc/fslmc_bus.c
+++ b/drivers/bus/fslmc/fslmc_bus.c
@@ -95,7 +95,6 @@ fslmc_bus_remove_device(struct rte_dpaa2_device *dev)
 		fslmc_bus_device_count[dev->dev_type]--;
 
 	rte_bus_remove_device(&rte_fslmc_bus, &dev->device);
-	rte_intr_instance_free(dev->intr_handle);
 	free(dev);
 }
 
@@ -184,19 +183,12 @@ scan_one_fslmc_device(char *dev_name)
 	dev->device.numa_node = SOCKET_ID_ANY;
 	dev->dev_type = dev_type;
 
-	/* Allocate interrupt instance */
-	dev->intr_handle =
-		rte_intr_instance_alloc(RTE_INTR_INSTANCE_F_PRIVATE);
-	if (dev->intr_handle == NULL) {
-		DPAA2_BUS_ERR("Failed to allocate intr handle");
-		goto cleanup;
-	}
-
 	sscanf(dev_id, "%hu", &dev->object_id);
 	dev->device.name = strdup(dev_name);
 	if (!dev->device.name) {
 		DPAA2_BUS_ERR("Unable to clone device name. Out of memory");
-		goto cleanup;
+		free(dev);
+		return -ENOMEM;
 	}
 	dev->device.devargs = rte_bus_find_devargs(&rte_fslmc_bus, dev_name);
 
@@ -204,12 +196,6 @@ scan_one_fslmc_device(char *dev_name)
 	insert_in_device_list(dev);
 
 	return 0;
-cleanup:
-	if (dev) {
-		rte_intr_instance_free(dev->intr_handle);
-		free(dev);
-	}
-	return -ENOMEM;
 }
 
 static int
@@ -488,18 +474,6 @@ rte_fslmc_scan(void)
 			DPAA2_BUS_ERR("Unable to setup devices %d", ret);
 			return 0;
 		}
-
-		RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus) {
-			if (dev->dev_type != DPAA2_ETH &&
-			    dev->dev_type != DPAA2_CRYPTO &&
-			    dev->dev_type != DPAA2_QDMA)
-				continue;
-			ret = fslmc_vfio_dev_setup(dev);
-			if (ret) {
-				DPAA2_BUS_ERR("Dev (%s) VFIO setup failed", dev->device.name);
-				return 0;
-			}
-		}
 	}
 
 	process_once = 1;
@@ -546,7 +520,6 @@ rte_fslmc_close(struct rte_bus *bus)
 			continue;
 		if (rte_dev_is_probed(&dev->device) && fslmc_bus_unplug_device(&dev->device))
 			DPAA2_BUS_ERR("Unable to remove %s", dev->device.name);
-		fslmc_vfio_dev_close(dev);
 	}
 
 	ret = fslmc_vfio_close_group();
@@ -622,10 +595,27 @@ fslmc_bus_probe_device(struct rte_driver *driver, struct rte_device *rte_dev)
 	struct rte_dpaa2_driver *drv = RTE_BUS_DRIVER(driver, *drv);
 	int ret = 0;
 
-	/* FIXME: probe_device should allocate intr_handle */
+	/* Allocate interrupt instance */
+	dev->intr_handle =
+		rte_intr_instance_alloc(RTE_INTR_INSTANCE_F_PRIVATE);
+	if (dev->intr_handle == NULL) {
+		DPAA2_BUS_ERR("Failed to allocate intr handle");
+		return -ENOMEM;
+	}
+
+	ret = fslmc_vfio_dev_setup(dev);
+	if (ret) {
+		DPAA2_BUS_ERR("Dev (%s) VFIO setup failed", dev->device.name);
+		goto release_intr;
+	}
+
 	ret = drv->probe(drv, dev);
 	if (ret != 0) {
 		DPAA2_BUS_ERR("Unable to probe");
+		fslmc_vfio_dev_close(dev);
+release_intr:
+		rte_intr_instance_free(dev->intr_handle);
+		dev->intr_handle = NULL;
 	} else {
 		DPAA2_BUS_INFO("%s Plugged",  dev->device.name);
 	}
@@ -643,7 +633,9 @@ fslmc_bus_unplug_device(struct rte_device *rte_dev)
 		int ret = drv->remove(dev);
 		if (ret != 0)
 			return ret;
-		/* FIXME: unplug_device should free intr_handle */
+		fslmc_vfio_dev_close(dev);
+		rte_intr_instance_free(dev->intr_handle);
+		dev->intr_handle = NULL;
 		DPAA2_BUS_INFO("%s Un-Plugged",  dev->device.name);
 		return 0;
 	}
diff --git a/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c b/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
index e17050b625..71996b2b38 100644
--- a/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
+++ b/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
@@ -470,6 +470,7 @@ dpaa2_close_dpio_device(int object_id)
 			rte_free(dpio_dev->dpio);
 		}
 		TAILQ_REMOVE(&dpio_dev_list, dpio_dev, next);
+		rte_intr_instance_free(dpio_dev->intr_handle);
 		rte_free(dpio_dev);
 	}
 }
-- 
2.54.0

[RFC 11/11] bus/fslmc: use generic cleanup

[David Marchand]

Now that the probe and unplug callback behave like other buses, we can
call generic cleanup in addition to the special handling for IO devices.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/bus/fslmc/fslmc_bus.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
index 39afdee9ee..49f5d0290e 100644
--- a/drivers/bus/fslmc/fslmc_bus.c
+++ b/drivers/bus/fslmc/fslmc_bus.c
@@ -504,23 +504,18 @@ fslmc_bus_match(const struct rte_driver *drv, const struct rte_device *dev)
 	return false;
 }
 
-static int
-fslmc_bus_unplug_device(struct rte_device *rte_dev);
+static void
+fslmc_free_device(struct rte_device *rte_dev)
+{
+	free(RTE_BUS_DEVICE(rte_dev, struct rte_dpaa2_device));
+}
 
 static int
-rte_fslmc_close(struct rte_bus *bus)
+fslmc_cleanup(struct rte_bus *bus)
 {
-	struct rte_dpaa2_device *dev;
-	int ret = 0;
+	int ret;
 
-	RTE_BUS_FOREACH_DEV(dev, bus) {
-		if (dev->dev_type != DPAA2_ETH &&
-		    dev->dev_type != DPAA2_CRYPTO &&
-		    dev->dev_type != DPAA2_QDMA)
-			continue;
-		if (rte_dev_is_probed(&dev->device) && fslmc_bus_unplug_device(&dev->device))
-			DPAA2_BUS_ERR("Unable to remove %s", dev->device.name);
-	}
+	rte_bus_generic_cleanup(bus);
 
 	ret = fslmc_vfio_close_group();
 	if (ret)
@@ -646,7 +641,8 @@ fslmc_bus_unplug_device(struct rte_device *rte_dev)
 struct rte_bus rte_fslmc_bus = {
 	.scan = rte_fslmc_scan,
 	.probe = rte_bus_generic_probe,
-	.cleanup = rte_fslmc_close,
+	.free_device = fslmc_free_device,
+	.cleanup = fslmc_cleanup,
 	.parse = rte_fslmc_parse,
 	.dev_compare = fslmc_dev_compare,
 	.find_device = rte_bus_generic_find_device,
-- 
2.54.0

RE: [RFC 00/11] Device unplug and bus cleanup refactoring for NXP

[Hemant Agrawal]

HI David,
	Thanks for the work. We will test it soon.

Regards
Hemant

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: 23 July 2026 19:24
> To: Hemant Agrawal <hemant.agrawal@nxp.com>; dev@dpdk.org
> Subject: [RFC 00/11] Device unplug and bus cleanup refactoring for NXP
> Importance: High
> 
> Hello Hemant,
> 
> This is a followup to the refactoring started in 26.07.
> 
> I took some time with my best AI friend to cleanup DPAA and FSLMC bus drivers.
> 
> Like the last time, only compilation has been checked.
> I have no hardware to test runtime.
> 
> One thing that could be broken is either the order of devices initialisation, or bugs
> in the device filtering that I tried to simplify.
> 
> I think it is worth testing and fixing, as it will make the two NXP bus drivers similar
> to other bus drivers (but keep the special IO devices handling internal to the
> FSLMC bus for example).
> 
> 
> --
> David Marchand
> 
> David Marchand (11):
>   drivers/bus: cleanup device freeing in NXP bus scan
>   bus/dpaa: allocate interrupt during probing
>   bus/dpaa: support unplug and use generic cleanup
>   bus/fslmc: fix memory leaks in scan
>   bus/fslmc: fix per type device count
>   bus/fslmc: simplify device parsing in scan
>   bus/fslmc: refactor device filtering for multiprocess
>   bus/fslmc: move unplug for some device out of VFIO
>   bus/fslmc: call VFIO setup for some device from bus layer
>   bus/fslmc: allocate interrupt during probing
>   bus/fslmc: use generic cleanup
> 
>  doc/guides/rel_notes/release_26_11.rst   |   4 +
>  drivers/bus/dpaa/dpaa_bus.c              | 134 +++++-----
>  drivers/bus/fslmc/fslmc_bus.c            | 295 +++++++++++++++--------
>  drivers/bus/fslmc/fslmc_vfio.c           | 255 +++++++-------------
>  drivers/bus/fslmc/fslmc_vfio.h           |   5 +
>  drivers/bus/fslmc/portal/dpaa2_hw_dpio.c |   1 +
>  drivers/bus/fslmc/private.h              |   2 +
>  7 files changed, 368 insertions(+), 328 deletions(-)
> 
> --
> 2.54.0

Re: [RFC 00/11] Device unplug and bus cleanup refactoring for NXP

[Stephen Hemminger]

On Thu, 23 Jul 2026 15:53:48 +0200
David Marchand <david.marchand@redhat.com> wrote:

> Hello Hemant,
> 
> This is a followup to the refactoring started in 26.07.
> 
> I took some time with my best AI friend to cleanup DPAA and FSLMC bus
> drivers.
> 
> Like the last time, only compilation has been checked.
> I have no hardware to test runtime.
> 
> One thing that could be broken is either the order of devices
> initialisation, or bugs in the device filtering that I tried to
> simplify.
> 
> I think it is worth testing and fixing, as it will make the two NXP
> bus drivers similar to other bus drivers (but keep the special IO devices
> handling internal to the FSLMC bus for example).
> 

Detailed AI review spotted some things.

Review of [RFC 00/11] bus refactoring for NXP buses

Reviewed against DPDK main (c1a46b9), applied with git am, cross-built
with config/arm/arm64_dpaa_linux_gcc -Dwerror=true.

Patch 02/11: bus/dpaa: allocate interrupt during probing

Error: fd leak on the dpaa_setup_intr() failure path.

  ret = dpaa_setup_intr(dpaa_dev->intr_handle);
  if (ret != 0) {
          ...
          goto release_intr;
  }
  ...
          dpaa_close_intr(dpaa_dev->intr_handle);
  release_intr:
          rte_intr_instance_free(dpaa_dev->intr_handle);

dpaa_setup_intr() opens an eventfd and stores it with rte_intr_fd_set()
before calling rte_intr_type_set().  If rte_intr_type_set() fails, the fd
is already installed in the handle, but the goto lands past
dpaa_close_intr(), so the instance is freed with the fd still open.
Move the label above dpaa_close_intr(), or close the fd inside
dpaa_setup_intr() on its own error paths.

Warning: error propagation lost in dpaa_bus_cleanup().  The old code
returned -1 when drv->remove() failed; the rewritten loop does
"rte_errno = errno; goto next;" and the function unconditionally returns
0.  The local "ret" is also assigned and never read afterwards.  This is
transient (patch 03 replaces the body with rte_bus_generic_cleanup())
but each commit should stand on its own; track the failure in a variable
and return it.

Info: jumping into the middle of an if-block ("release_intr:" inside the
"if (ret != 0)" body) is legal but hard to follow.  A separate error
block would read better.

Patch 04/11: bus/fslmc: fix memory leaks in scan

Warning: fslmc_bus_remove_device() is described as fully freeing a
device, but dev->device.name is strdup()'d in scan_one_fslmc_device()
and never freed here or anywhere else.  Same gap in fslmc_free_device()
added by patch 11.  Since this patch is specifically about scan-time
leaks, freeing the name belongs here.

Patch 07/11: bus/fslmc: refactor device filtering for multiprocess

Error: allowlist mode is broken by moving the ignore test into scan.
scan_one_fslmc_device() now calls rte_bus_device_is_ignored(), which in
RTE_BUS_SCAN_ALLOWLIST mode returns true for every device lacking an
explicit RTE_DEV_ALLOWED devargs.  A single "-a fslmc:dpni.1" sets
rte_fslmc_bus.conf.scan_mode to ALLOWLIST (eal_common_devargs.c:349),
after which dpbp/dpcon/dpci/dprc/dpdmux/dprtc are dropped at scan time.
Those control objects are not probed by a driver; they are initialised
by fslmc_vfio_process_group(), so dropping them at scan leaves the bus
non-functional.  fslmc_filter_control_devices() applies the same test to
DPMCP and DPIO, so allowlist mode also fails "No MC Portal device found"
(-ENODEV).  The old code tested only devargs->policy == RTE_DEV_BLOCKED,
and the generic probe loop already skips ignored devices at probe time,
which is why allowlists worked before.

Error: the DPIO split drops the only DPIO on a primary process.  The old
code guarded the split with "dpio_count > 1":

  if (!is_dpio_in_blocklist && dpio_count > 1) {

The new helper has no such guard, so with exactly one DPIO,
last_index == 0, current_device == 0, and the primary branch removes it:

  } else if (rte_eal_process_type() == RTE_PROC_PRIMARY &&
                  current_device == last_index) {
          fslmc_bus_remove_device(dev);

Restore the dpio_count > 1 condition.

Warning: when a DPMCP is blocklisted, surplus MPORTAL devices are left
in the bus list.  fslmc_vfio_process_group() now breaks out of the
MPORTAL loop after the first device.  When is_dpmcp_in_blocklist is set,
fslmc_filter_control_devices() skips the split, so more than one MPORTAL
can remain and the extras are never removed.  The previous loop had no
break and removed all of them.

Patch 11/11: bus/fslmc: use generic cleanup

Error: cleanup ordering makes fslmc_vfio_close_group() a no-op.

  rte_bus_generic_cleanup(bus);

  ret = fslmc_vfio_close_group();

rte_bus_generic_cleanup() unplugs every device, calls
rte_bus_remove_device() and then bus->free_device(), emptying
rte_fslmc_bus.device_list.  fslmc_vfio_close_group() then iterates that
now-empty list, so fslmc_close_iodevices() is never called for DPIO,
DPCON, DPCI, DPBP or DPDMUX; only fslmc_vfio_clear_group() still runs.
Call fslmc_vfio_close_group() before rte_bus_generic_cleanup().

Warning: the return value of rte_bus_generic_cleanup() is discarded.  It
reports unplug failures via -1/rte_errno; fslmc_cleanup() overwrites
"ret" with the fslmc_vfio_close_group() result and loses it.

Warning: fslmc_free_device() only calls free().  It does not release
dev->intr_handle and does not decrement fslmc_bus_device_count[], both
of which fslmc_bus_remove_device() handles.  Devices torn down through
the generic path therefore leave the per-type counters permanently
stale.

Info (01/11): removing the "struct rte_dpaa2_device *dev" declaration
from the process_once block leaves a stray blank line at the top of the
block in rte_fslmc_scan().

Info (06/11): dev_types[] is a non-static, non-const local array, so it
is rebuilt on the stack for every device.  Make it "static const".

Info (06/11): the sscanf() return value is unchecked, and the new
prefix-based match is weaker than the old strtok() guard.  A name such
as "dpni." matches the prefix, leaves dev_id pointing at "", sscanf()
fails, and the device is registered with object_id 0 -- colliding with a
real dpni.0.  The old code rejected it because strtok(NULL, ".")
returned NULL.  Check that sscanf() returns 1 and skip the device
otherwise.

Re: [RFC 00/11] Device unplug and bus cleanup refactoring for NXP

[David Marchand]

On Thu, 23 Jul 2026 at 16:11, Hemant Agrawal <hemant.agrawal@nxp.com> wrote:
>
> HI David,
>         Thanks for the work. We will test it soon.

Any update?


-- 
David Marchand

Re: [RFC 00/11] Device unplug and bus cleanup refactoring for NXP

[Hemant Agrawal]

[-- Attachment #1: Type: text/plain, Size: 595 bytes --]




________________________________
From: David Marchand <david.marchand@redhat.com>
Sent: Thursday, September 3, 2026 7:00 PM
To: Hemant Agrawal <hemant.agrawal@nxp.com>
Cc: dev@dpdk.org <dev@dpdk.org>
Subject: Re: [RFC 00/11] Device unplug and bus cleanup refactoring for NXP

On Thu, 23 Jul 2026 at 16:11, Hemant Agrawal <hemant.agrawal@nxp.com> wrote:
>
> HI David,
>         Thanks for the work. We will test it soon.

Any update?

We just completed sanity and a few quick tests. This patch set is not causing any regression.


--
David Marchand


NXP Confidential

[-- Attachment #2: Type: text/html, Size: 2000 bytes --]

Re: [RFC 00/11] Device unplug and bus cleanup refactoring for NXP

[David Marchand]

Hello Hemant,

On Tue, 8 Sept 2026 at 16:20, Hemant Agrawal <hemant.agrawal@nxp.com> wrote:
> On Thu, 23 Jul 2026 at 16:11, Hemant Agrawal <hemant.agrawal@nxp.com> wrote:
> > >
> > > HI David,
> > >         Thanks for the work. We will test it soon.
> >
> > Any update?
>
> We just completed sanity and a few quick tests. This patch set is not causing any regression.

Thanks a lot for testing.
Stephen comments have spotted a number of issues, I'll post a v2 soon.


-- 
David Marchand

[RFC v2 00/16] Device unplug and bus cleanup refactoring for NXP

[David Marchand]

Hello Hemant,

This is a followup to the refactoring started in 26.07.

I took some time with my best AI friend to cleanup DPAA and FSLMC bus
drivers.

Like the last time, only compilation has been checked.
I have no hardware to test runtime.

One thing that could be broken is either the order of devices
initialisation, or bugs in the device filtering that I tried to
simplify.

I went with the approach of splitting the device list between
internal bus object (that I called "control" devices, maybe a better
name can be found), and normal bus device object for which DPDK has a
driver.

I think it is worth testing and fixing, as it will make the two NXP
bus drivers similar to other bus drivers (but keep the special IO devices
handling internal to the FSLMC bus for example).


-- 
David Marchand

Changes since RFC v1:
- fixed FD leak in bus/dpaa pre existing code,
- fixed device name leak in bus/fslmc pre existing code,
- fixed other leaks found by AI review,
- split DPAA2 devices in two list:
  - one list in the bus object for devices for which a driver exists,
  - one list local to the fslmc bus code for internal resources that
    are initialised out of the EAL / bus framework,

David Marchand (16):
  drivers/bus: cleanup device freeing in NXP bus scan
  drivers/bus: fix FD leak during scan in NXP drivers
  bus/dpaa: allocate interrupt during probing
  bus/dpaa: support unplug and use generic cleanup
  bus/fslmc: fix device name leak
  bus/fslmc: fix memory leaks in scan
  bus/fslmc: fix per type device count
  bus/fslmc: fix some VFIO device FD and memory leaks
  bus/fslmc: fix interrupt leak in DPIO cleanup
  bus/fslmc: simplify device parsing in scan
  bus/fslmc: release resources on scan failure
  bus/fslmc: refactor device filtering for multiprocess
  bus/fslmc: move unplug for some device out of VFIO
  bus/fslmc: call VFIO setup for some device from bus layer
  bus/fslmc: allocate interrupt during probing
  bus/fslmc: use generic cleanup

 doc/guides/rel_notes/release_26_11.rst   |   4 +
 drivers/bus/dpaa/dpaa_bus.c              | 146 ++++----
 drivers/bus/fslmc/bus_fslmc_driver.h     |   4 +-
 drivers/bus/fslmc/fslmc_bus.c            | 431 +++++++++++++++++------
 drivers/bus/fslmc/fslmc_vfio.c           | 407 ++++++++++-----------
 drivers/bus/fslmc/fslmc_vfio.h           |   6 +
 drivers/bus/fslmc/portal/dpaa2_hw_dpio.c |   1 +
 drivers/bus/fslmc/private.h              |   6 +
 8 files changed, 601 insertions(+), 404 deletions(-)

-- 
2.54.0

[RFC v2 01/16] drivers/bus: cleanup device freeing in NXP bus scan

[David Marchand]

Following the scan/probe refactoring, in the DPAA bus driver scan method,
the device list is empty at the time the pthread key object is allocated.
This leaves only one location that wants to release the whole device
list, so remove the dpaa_clean_device_list() helper.

Remove the same helper in FSLMC bus for consistency.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/bus/dpaa/dpaa_bus.c   | 15 ++-------------
 drivers/bus/fslmc/fslmc_bus.c | 22 ++++++----------------
 2 files changed, 8 insertions(+), 29 deletions(-)

diff --git a/drivers/bus/dpaa/dpaa_bus.c b/drivers/bus/dpaa/dpaa_bus.c
index 368c8eeb98..06962a5b29 100644
--- a/drivers/bus/dpaa/dpaa_bus.c
+++ b/drivers/bus/dpaa/dpaa_bus.c
@@ -193,8 +193,6 @@ dpaa_sec_available(void)
 	return -1;
 }
 
-static void dpaa_clean_device_list(void);
-
 static int
 dpaa_create_device_list(void)
 {
@@ -336,21 +334,13 @@ dpaa_create_device_list(void)
 	return 0;
 
 cleanup:
-	dpaa_clean_device_list();
-	return ret;
-}
-
-static void
-dpaa_clean_device_list(void)
-{
-	struct rte_dpaa_device *dev = NULL;
-
 	RTE_BUS_FOREACH_DEV(dev, &rte_dpaa_bus) {
 		rte_bus_remove_device(&rte_dpaa_bus, &dev->device);
 		rte_intr_instance_free(dev->intr_handle);
 		free(dev);
-		dev = NULL;
 	}
+
+	return ret;
 }
 
 RTE_EXPORT_INTERNAL_SYMBOL(rte_dpaa_portal_init)
@@ -699,7 +689,6 @@ rte_dpaa_bus_scan(void)
 	ret = pthread_key_create(&dpaa_portal_key, dpaa_portal_finish);
 	if (ret) {
 		DPAA_BUS_LOG(DEBUG, "Unable to create pthread key. (%d)", ret);
-		dpaa_clean_device_list();
 		return ret;
 	}
 
diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
index 3626b12316..94ae96f942 100644
--- a/drivers/bus/fslmc/fslmc_bus.c
+++ b/drivers/bus/fslmc/fslmc_bus.c
@@ -43,19 +43,6 @@ rte_fslmc_get_device_count(enum rte_dpaa2_dev_type device_type)
 	return fslmc_bus_device_count[device_type];
 }
 
-static void
-cleanup_fslmc_device_list(void)
-{
-	struct rte_dpaa2_device *dev;
-
-	RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus) {
-		rte_bus_remove_device(&rte_fslmc_bus, &dev->device);
-		rte_intr_instance_free(dev->intr_handle);
-		free(dev);
-		dev = NULL;
-	}
-}
-
 static int
 compare_dpaa2_devname(struct rte_dpaa2_device *dev1,
 		      struct rte_dpaa2_device *dev2)
@@ -305,6 +292,7 @@ fslmc_dev_compare(const char *name1, const char *name2)
 static int
 rte_fslmc_scan(void)
 {
+	struct rte_dpaa2_device *dev;
 	int ret;
 	char fslmc_dirpath[PATH_MAX];
 	DIR *dir;
@@ -314,8 +302,6 @@ rte_fslmc_scan(void)
 	char *group_name;
 
 	if (process_once) {
-		struct rte_dpaa2_device *dev;
-
 		DPAA2_BUS_DEBUG("Fslmc bus already scanned. Not rescanning");
 		RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus) {
 			dev->device.devargs = rte_bus_find_devargs(&rte_fslmc_bus,
@@ -416,7 +402,11 @@ rte_fslmc_scan(void)
 	closedir(dir);
 
 	/* Remove all devices in the list */
-	cleanup_fslmc_device_list();
+	RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus) {
+		rte_bus_remove_device(&rte_fslmc_bus, &dev->device);
+		rte_intr_instance_free(dev->intr_handle);
+		free(dev);
+	}
 scan_fail:
 	DPAA2_BUS_DEBUG("FSLMC Bus Not Available. Skipping (%d)", ret);
 	/* Irrespective of failure, scan only return success */
-- 
2.54.0

[RFC v2 02/16] drivers/bus: fix FD leak during scan in NXP drivers

[David Marchand]

In theory, if the interrupt handle setup fails, an eventfd gets leaked.

In practice, as long as the interrupt handle is valid, those calls can
not fail, but this could get broken in the future so it is better to
handle this error branch correctly.

Fixes: d61138d4f0e2 ("drivers: remove direct access to interrupt handle")

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/bus/dpaa/dpaa_bus.c    | 11 +++++++----
 drivers/bus/fslmc/fslmc_vfio.c | 18 ++++++++++--------
 2 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/bus/dpaa/dpaa_bus.c b/drivers/bus/dpaa/dpaa_bus.c
index 06962a5b29..5df2dff8f2 100644
--- a/drivers/bus/dpaa/dpaa_bus.c
+++ b/drivers/bus/dpaa/dpaa_bus.c
@@ -648,11 +648,14 @@ static int rte_dpaa_setup_intr(struct rte_intr_handle *intr_handle)
 		return errno;
 	}
 
-	if (rte_intr_fd_set(intr_handle, fd))
-		return rte_errno;
+	if (rte_intr_fd_set(intr_handle, fd) ||
+			rte_intr_type_set(intr_handle, RTE_INTR_HANDLE_EXT)) {
+		int err = rte_errno;
 
-	if (rte_intr_type_set(intr_handle, RTE_INTR_HANDLE_EXT))
-		return rte_errno;
+		close(fd);
+		rte_intr_fd_set(intr_handle, -1);
+		return err;
+	}
 
 	return 0;
 }
diff --git a/drivers/bus/fslmc/fslmc_vfio.c b/drivers/bus/fslmc/fslmc_vfio.c
index 412b70e5ae..165e9444f5 100644
--- a/drivers/bus/fslmc/fslmc_vfio.c
+++ b/drivers/bus/fslmc/fslmc_vfio.c
@@ -1371,14 +1371,16 @@ rte_dpaa2_vfio_setup_intr(struct rte_intr_handle *intr_handle,
 			return fd;
 		}
 
-		if (rte_intr_fd_set(intr_handle, fd))
-			return -rte_errno;
-
-		if (rte_intr_type_set(intr_handle, RTE_INTR_HANDLE_VFIO_MSI))
-			return -rte_errno;
-
-		if (rte_intr_dev_fd_set(intr_handle, vfio_dev_fd))
-			return -rte_errno;
+		if (rte_intr_fd_set(intr_handle, fd) ||
+				rte_intr_type_set(intr_handle, RTE_INTR_HANDLE_VFIO_MSI) ||
+				rte_intr_dev_fd_set(intr_handle, vfio_dev_fd)) {
+			int error = -rte_errno;
+
+			close(fd);
+			rte_intr_fd_set(intr_handle, -1);
+			rte_intr_dev_fd_set(intr_handle, -1);
+			return error;
+		}
 
 		return 0;
 	}
-- 
2.54.0

[RFC v2 03/16] bus/dpaa: allocate interrupt during probing

[David Marchand]

Allocating the interrupt handle is a waste of memory if no device is
probed later (like for example, if a allowlist is passed).

Instead, allocate this handle, set eventfds and vfio at the time
probe_device is called.

Adjust the unplug_device path accordingly.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changes since RFC v1:
- restored return code on error in cleanup,

---
 drivers/bus/dpaa/dpaa_bus.c | 82 ++++++++++++++++++++-----------------
 1 file changed, 44 insertions(+), 38 deletions(-)

diff --git a/drivers/bus/dpaa/dpaa_bus.c b/drivers/bus/dpaa/dpaa_bus.c
index 5df2dff8f2..ef904a7192 100644
--- a/drivers/bus/dpaa/dpaa_bus.c
+++ b/drivers/bus/dpaa/dpaa_bus.c
@@ -215,16 +215,6 @@ dpaa_create_device_list(void)
 
 		dev->device.numa_node = SOCKET_ID_ANY;
 
-		/* Allocate interrupt handle instance */
-		dev->intr_handle =
-			rte_intr_instance_alloc(RTE_INTR_INSTANCE_F_PRIVATE);
-		if (dev->intr_handle == NULL) {
-			DPAA_BUS_LOG(ERR, "Failed to allocate intr handle");
-			ret = -ENOMEM;
-			free(dev);
-			goto cleanup;
-		}
-
 		cfg = &dpaa_netcfg->port_cfg[i];
 		fman_intf = cfg->fman_if;
 
@@ -276,16 +266,6 @@ dpaa_create_device_list(void)
 			goto cleanup;
 		}
 
-		/* Allocate interrupt handle instance */
-		dev->intr_handle =
-			rte_intr_instance_alloc(RTE_INTR_INSTANCE_F_PRIVATE);
-		if (dev->intr_handle == NULL) {
-			DPAA_BUS_LOG(ERR, "Failed to allocate intr handle");
-			ret = -ENOMEM;
-			free(dev);
-			goto cleanup;
-		}
-
 		dev->device_type = FSL_DPAA_CRYPTO;
 		dev->id.dev_id = dpaa_bus.device_count + i;
 
@@ -336,7 +316,6 @@ dpaa_create_device_list(void)
 cleanup:
 	RTE_BUS_FOREACH_DEV(dev, &rte_dpaa_bus) {
 		rte_bus_remove_device(&rte_dpaa_bus, &dev->device);
-		rte_intr_instance_free(dev->intr_handle);
 		free(dev);
 	}
 
@@ -637,7 +616,8 @@ rte_dpaa_bus_dev_build(void)
 	return 0;
 }
 
-static int rte_dpaa_setup_intr(struct rte_intr_handle *intr_handle)
+static int
+dpaa_setup_intr(struct rte_intr_handle *intr_handle)
 {
 	int fd;
 
@@ -660,13 +640,21 @@ static int rte_dpaa_setup_intr(struct rte_intr_handle *intr_handle)
 	return 0;
 }
 
+static void
+dpaa_close_intr(struct rte_intr_handle *intr_handle)
+{
+	if (rte_intr_fd_get(intr_handle) >= 0) {
+		close(rte_intr_fd_get(intr_handle));
+		rte_intr_fd_set(intr_handle, -1);
+	}
+}
+
 #define DPAA_DEV_PATH1 "/sys/devices/platform/soc/soc:fsl,dpaa"
 #define DPAA_DEV_PATH2 "/sys/devices/platform/fsl,dpaa"
 
 static int
 rte_dpaa_bus_scan(void)
 {
-	struct rte_dpaa_device *dev;
 	FILE *svr_file = NULL;
 	uint32_t svr_ver;
 	static int process_once;
@@ -755,14 +743,6 @@ rte_dpaa_bus_scan(void)
 	 */
 	rte_mbuf_set_platform_mempool_ops(DPAA_MEMPOOL_OPS_NAME);
 
-	RTE_BUS_FOREACH_DEV(dev, &rte_dpaa_bus) {
-		if (dev->device_type == FSL_DPAA_ETH) {
-			ret = rte_dpaa_setup_intr(dev->intr_handle);
-			if (ret)
-				DPAA_BUS_ERR("Error setting up interrupt.");
-		}
-	}
-
 	/* And initialize the PA->VA translation table */
 	dpaax_iova_table_populate();
 
@@ -790,10 +770,30 @@ dpaa_bus_probe_device(struct rte_driver *drv, struct rte_device *dev)
 	struct rte_dpaa_driver *dpaa_drv = RTE_BUS_DRIVER(drv, *dpaa_drv);
 	int ret;
 
+	/* Allocate interrupt handle instance */
+	dpaa_dev->intr_handle = rte_intr_instance_alloc(RTE_INTR_INSTANCE_F_PRIVATE);
+	if (dpaa_dev->intr_handle == NULL) {
+		DPAA_BUS_LOG(ERR, "Failed to allocate intr handle");
+		return -ENOMEM;
+	}
+
+	if (dpaa_dev->device_type == FSL_DPAA_ETH) {
+		ret = dpaa_setup_intr(dpaa_dev->intr_handle);
+		if (ret != 0) {
+			DPAA_BUS_ERR("error setting up interrupt: %s", dpaa_dev->name);
+			ret = -ret;
+			goto release_intr;
+		}
+	}
+
 	ret = dpaa_drv->probe(dpaa_drv, dpaa_dev);
-	if (ret != 0)
+	if (ret != 0) {
 		DPAA_BUS_ERR("unable to probe: %s", dpaa_dev->name);
-
+		dpaa_close_intr(dpaa_dev->intr_handle);
+release_intr:
+		rte_intr_instance_free(dpaa_dev->intr_handle);
+		dpaa_dev->intr_handle = NULL;
+	}
 	return ret;
 }
 
@@ -801,29 +801,35 @@ static int
 dpaa_bus_cleanup(struct rte_bus *bus)
 {
 	struct rte_dpaa_device *dev;
+	int error = 0;
 
 	BUS_INIT_FUNC_TRACE();
 	RTE_BUS_FOREACH_DEV(dev, bus) {
 		const struct rte_dpaa_driver *drv;
-		int ret = 0;
+		int ret;
 
 		if (!rte_dev_is_probed(&dev->device))
-			continue;
+			goto next;
 		drv = RTE_BUS_DRIVER(dev->device.driver, *drv);
 		if (drv->remove == NULL)
-			continue;
+			goto next;
 		ret = drv->remove(dev);
 		if (ret < 0) {
 			rte_errno = errno;
-			return -1;
+			error = -1;
+			goto next;
 		}
 		dev->device.driver = NULL;
+next:
+		dpaa_close_intr(dev->intr_handle);
+		rte_intr_instance_free(dev->intr_handle);
+		dev->intr_handle = NULL;
 	}
 	dpaa_portal_finish((void *)DPAA_PER_LCORE_PORTAL);
 	dpaa_bus_global_init = 0;
 	DPAA_BUS_DEBUG("Bus cleanup done");
 
-	return 0;
+	return error;
 }
 
 /* Adding destructor for double check in case non-gracefully
-- 
2.54.0

[RFC v2 04/16] bus/dpaa: support unplug and use generic cleanup

[David Marchand]

Add .unplug callback to handle driver removal and interrupt cleanup.
This enables use of the generic bus cleanup helper while preserving
bus-specific cleanup (portal finish, global init reset).

The cleanup function was already performing these operations, so it
seems safe to expose them through the unplug operation.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 doc/guides/rel_notes/release_26_11.rst |  4 ++
 drivers/bus/dpaa/dpaa_bus.c            | 56 +++++++++++++++-----------
 2 files changed, 37 insertions(+), 23 deletions(-)

diff --git a/doc/guides/rel_notes/release_26_11.rst b/doc/guides/rel_notes/release_26_11.rst
index 4b3e5d995c..7a5a0c55cb 100644
--- a/doc/guides/rel_notes/release_26_11.rst
+++ b/doc/guides/rel_notes/release_26_11.rst
@@ -55,6 +55,10 @@ New Features
      Also, make sure to start the actual text at the margin.
      =======================================================
 
+* **Added unplug operation support to DPAA bus.**
+
+  Implemented device unplug operation to allow runtime removal of DPAA devices.
+
 
 Removed Items
 -------------
diff --git a/drivers/bus/dpaa/dpaa_bus.c b/drivers/bus/dpaa/dpaa_bus.c
index ef904a7192..bb1c379dfd 100644
--- a/drivers/bus/dpaa/dpaa_bus.c
+++ b/drivers/bus/dpaa/dpaa_bus.c
@@ -798,38 +798,46 @@ dpaa_bus_probe_device(struct rte_driver *drv, struct rte_device *dev)
 }
 
 static int
-dpaa_bus_cleanup(struct rte_bus *bus)
+dpaa_bus_unplug_device(struct rte_device *rte_dev)
 {
-	struct rte_dpaa_device *dev;
-	int error = 0;
+	const struct rte_dpaa_driver *drv = RTE_BUS_DRIVER(rte_dev->driver, *drv);
+	struct rte_dpaa_device *dev = RTE_BUS_DEVICE(rte_dev, *dev);
 
-	BUS_INIT_FUNC_TRACE();
-	RTE_BUS_FOREACH_DEV(dev, bus) {
-		const struct rte_dpaa_driver *drv;
+	if (drv->remove != NULL) {
 		int ret;
 
-		if (!rte_dev_is_probed(&dev->device))
-			goto next;
-		drv = RTE_BUS_DRIVER(dev->device.driver, *drv);
-		if (drv->remove == NULL)
-			goto next;
 		ret = drv->remove(dev);
-		if (ret < 0) {
-			rte_errno = errno;
-			error = -1;
-			goto next;
-		}
-		dev->device.driver = NULL;
-next:
-		dpaa_close_intr(dev->intr_handle);
-		rte_intr_instance_free(dev->intr_handle);
-		dev->intr_handle = NULL;
+		if (ret < 0)
+			return ret;
 	}
+
+	dpaa_close_intr(dev->intr_handle);
+	rte_intr_instance_free(dev->intr_handle);
+	dev->intr_handle = NULL;
+
+	return 0;
+}
+
+static void
+dpaa_bus_free_device(struct rte_device *dev)
+{
+	free(RTE_BUS_DEVICE(dev, struct rte_dpaa_device));
+}
+
+static int
+dpaa_bus_cleanup(struct rte_bus *bus)
+{
+	int ret;
+
+	BUS_INIT_FUNC_TRACE();
+
+	ret = rte_bus_generic_cleanup(bus);
+
 	dpaa_portal_finish((void *)DPAA_PER_LCORE_PORTAL);
 	dpaa_bus_global_init = 0;
 	DPAA_BUS_DEBUG("Bus cleanup done");
 
-	return error;
+	return ret;
 }
 
 /* Adding destructor for double check in case non-gracefully
@@ -855,14 +863,16 @@ RTE_FINI_PRIO(dpaa_cleanup, 102)
 static struct rte_bus rte_dpaa_bus = {
 	.scan = rte_dpaa_bus_scan,
 	.probe = rte_bus_generic_probe,
+	.free_device = dpaa_bus_free_device,
+	.cleanup = dpaa_bus_cleanup,
 	.parse = rte_dpaa_bus_parse,
 	.dev_compare = dpaa_bus_dev_compare,
 	.find_device = rte_bus_generic_find_device,
 	.get_iommu_class = rte_dpaa_get_iommu_class,
 	.match = dpaa_bus_match,
 	.probe_device = dpaa_bus_probe_device,
+	.unplug_device = dpaa_bus_unplug_device,
 	.dev_iterate = rte_bus_generic_dev_iterate,
-	.cleanup = dpaa_bus_cleanup,
 };
 
 static struct rte_dpaa_bus_private dpaa_bus = {
-- 
2.54.0

[RFC v2 05/16] bus/fslmc: fix device name leak

[David Marchand]

The device name allocated during scan was never freed.
Store it to the (unused) name field in the rte_dpaa2_device.

This simplifies the device freeing to a simple free() call later.

Fixes: 828d51d8fc3e ("bus/fslmc: refactor scan and probe functions")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/bus/fslmc/bus_fslmc_driver.h | 4 +---
 drivers/bus/fslmc/fslmc_bus.c        | 8 ++++----
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/bus/fslmc/bus_fslmc_driver.h b/drivers/bus/fslmc/bus_fslmc_driver.h
index 44f81cf662..ead9d6613c 100644
--- a/drivers/bus/fslmc/bus_fslmc_driver.h
+++ b/drivers/bus/fslmc/bus_fslmc_driver.h
@@ -39,8 +39,6 @@ extern "C" {
 
 struct vfio_device_info;
 
-#define FSLMC_OBJECT_MAX_LEN 32   /**< Length of each device on bus */
-
 #define DPAA2_INVALID_MBUF_SEQN        0
 
 typedef uint32_t dpaa2_seqn_t;
@@ -101,7 +99,7 @@ struct rte_dpaa2_device {
 	uint16_t ep_object_id;                 /**< Endpoint DPAA2 Object ID */
 	char ep_name[RTE_DEV_NAME_MAX_LEN];
 	struct rte_intr_handle *intr_handle; /**< Interrupt handle */
-	char name[FSLMC_OBJECT_MAX_LEN];    /**< DPAA2 Object name*/
+	char name[RTE_DEV_NAME_MAX_LEN];
 };
 
 typedef int (*rte_dpaa2_obj_create_t)(int vdev_fd,
diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
index 94ae96f942..4b0896e42e 100644
--- a/drivers/bus/fslmc/fslmc_bus.c
+++ b/drivers/bus/fslmc/fslmc_bus.c
@@ -179,12 +179,12 @@ scan_one_fslmc_device(char *dev_name)
 	}
 
 	sscanf(t_ptr, "%hu", &dev->object_id);
-	dev->device.name = strdup(dev_name);
-	if (!dev->device.name) {
-		DPAA2_BUS_ERR("Unable to clone device name. Out of memory");
-		ret = -ENOMEM;
+	if (rte_strscpy(dev->name, dev_name, sizeof(dev->name)) < 0) {
+		DPAA2_BUS_ERR("Unable to copy device name: %s", rte_strerror(rte_errno));
+		ret = -rte_errno;
 		goto cleanup;
 	}
+	dev->device.name = dev->name;
 	dev->device.devargs = rte_bus_find_devargs(&rte_fslmc_bus, dev_name);
 
 	/* Update the device found into the device_count table */
-- 
2.54.0

[RFC v2 06/16] bus/fslmc: fix memory leaks in scan

[David Marchand]

During the scan phase (more precisely, during device filtering),
device pointers were leaked.

Add a simple helper so a device pointer is fully freed.

As a consequence of the filtering during scan, all blocklisted devices
are freed from the bus list during scan: there is no need for
reevaluating if a device is blocklisted during cleanup.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changes since RFC v1:
- fixed interrupt handle leak by adding fslmc_free_device helper
  sooner in the series,

---
 drivers/bus/fslmc/fslmc_bus.c  | 23 ++++++++++++++++++-----
 drivers/bus/fslmc/fslmc_vfio.c | 33 +++++++++------------------------
 drivers/bus/fslmc/private.h    |  2 ++
 3 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
index 4b0896e42e..322b023652 100644
--- a/drivers/bus/fslmc/fslmc_bus.c
+++ b/drivers/bus/fslmc/fslmc_bus.c
@@ -85,6 +85,22 @@ insert_in_device_list(struct rte_dpaa2_device *newdev)
 		rte_bus_add_device(&rte_fslmc_bus, &newdev->device);
 }
 
+static void
+fslmc_free_device(struct rte_device *rte_dev)
+{
+	struct rte_dpaa2_device *dev = RTE_BUS_DEVICE(rte_dev, *dev);
+
+	rte_intr_instance_free(dev->intr_handle);
+	free(dev);
+}
+
+void
+fslmc_bus_remove_device(struct rte_dpaa2_device *dev)
+{
+	rte_bus_remove_device(&rte_fslmc_bus, &dev->device);
+	fslmc_free_device(&dev->device);
+}
+
 static void
 dump_device_list(void)
 {
@@ -402,11 +418,8 @@ rte_fslmc_scan(void)
 	closedir(dir);
 
 	/* Remove all devices in the list */
-	RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus) {
-		rte_bus_remove_device(&rte_fslmc_bus, &dev->device);
-		rte_intr_instance_free(dev->intr_handle);
-		free(dev);
-	}
+	RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus)
+		fslmc_bus_remove_device(dev);
 scan_fail:
 	DPAA2_BUS_DEBUG("FSLMC Bus Not Available. Skipping (%d)", ret);
 	/* Irrespective of failure, scan only return success */
diff --git a/drivers/bus/fslmc/fslmc_vfio.c b/drivers/bus/fslmc/fslmc_vfio.c
index 165e9444f5..6e0b35f391 100644
--- a/drivers/bus/fslmc/fslmc_vfio.c
+++ b/drivers/bus/fslmc/fslmc_vfio.c
@@ -1557,13 +1557,6 @@ fslmc_vfio_close_group(void)
 	}
 
 	RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus) {
-		if (dev->device.devargs &&
-		    dev->device.devargs->policy == RTE_DEV_BLOCKED) {
-			DPAA2_BUS_LOG(DEBUG, "%s Blacklisted, skipping",
-				      dev->device.name);
-			rte_bus_remove_device(&rte_fslmc_bus, &dev->device);
-				continue;
-		}
 		switch (dev->dev_type) {
 		case DPAA2_ETH:
 		case DPAA2_CRYPTO:
@@ -1625,8 +1618,7 @@ fslmc_vfio_process_group(void)
 			    dev->device.devargs->policy == RTE_DEV_BLOCKED) {
 				DPAA2_BUS_LOG(DEBUG, "%s Blocked, skipping",
 					      dev->device.name);
-				rte_bus_remove_device(&rte_fslmc_bus,
-						&dev->device);
+				fslmc_bus_remove_device(dev);
 				continue;
 			}
 
@@ -1634,8 +1626,7 @@ fslmc_vfio_process_group(void)
 			    !is_dpmcp_in_blocklist) {
 				if (dpmcp_count == 1 ||
 				    current_device != dpmcp_count) {
-					rte_bus_remove_device(&rte_fslmc_bus,
-						     &dev->device);
+					fslmc_bus_remove_device(dev);
 					continue;
 				}
 			}
@@ -1649,9 +1640,7 @@ fslmc_vfio_process_group(void)
 				found_mportal = 1;
 			}
 
-			rte_bus_remove_device(&rte_fslmc_bus, &dev->device);
-			free(dev);
-			dev = NULL;
+			fslmc_bus_remove_device(dev);
 			/* Ideally there is only a single dpmcp, but in case
 			 * multiple exists, looping on remaining devices.
 			 */
@@ -1675,7 +1664,7 @@ fslmc_vfio_process_group(void)
 				DPAA2_BUS_ERR("Unable to process dprc");
 				return ret;
 			}
-			rte_bus_remove_device(&rte_fslmc_bus, &dev->device);
+			fslmc_bus_remove_device(dev);
 		}
 	}
 
@@ -1687,7 +1676,7 @@ fslmc_vfio_process_group(void)
 		    dev->device.devargs->policy == RTE_DEV_BLOCKED) {
 			DPAA2_BUS_LOG(DEBUG, "%s Blocked, skipping",
 				      dev->device.name);
-			rte_bus_remove_device(&rte_fslmc_bus, &dev->device);
+			fslmc_bus_remove_device(dev);
 			continue;
 		}
 		if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
@@ -1695,7 +1684,7 @@ fslmc_vfio_process_group(void)
 		    dev->dev_type != DPAA2_CRYPTO &&
 		    dev->dev_type != DPAA2_QDMA &&
 		    dev->dev_type != DPAA2_IO) {
-			rte_bus_remove_device(&rte_fslmc_bus, &dev->device);
+			fslmc_bus_remove_device(dev);
 			continue;
 		}
 		switch (dev->dev_type) {
@@ -1737,14 +1726,12 @@ fslmc_vfio_process_group(void)
 			if (!is_dpio_in_blocklist && dpio_count > 1) {
 				if (rte_eal_process_type() == RTE_PROC_SECONDARY
 				    && current_device != dpio_count) {
-					rte_bus_remove_device(&rte_fslmc_bus,
-						     &dev->device);
+					fslmc_bus_remove_device(dev);
 					break;
 				}
 				if (rte_eal_process_type() == RTE_PROC_PRIMARY
 				    && current_device == dpio_count) {
-					rte_bus_remove_device(&rte_fslmc_bus,
-						     &dev->device);
+					fslmc_bus_remove_device(dev);
 					break;
 				}
 			}
@@ -1762,9 +1749,7 @@ fslmc_vfio_process_group(void)
 			/* Unknown - ignore */
 			DPAA2_BUS_DEBUG("Found unknown device (%s)",
 					dev->device.name);
-			rte_bus_remove_device(&rte_fslmc_bus, &dev->device);
-			free(dev);
-			dev = NULL;
+			fslmc_bus_remove_device(dev);
 		}
 	}
 
diff --git a/drivers/bus/fslmc/private.h b/drivers/bus/fslmc/private.h
index 20a454c3fc..825a364f1b 100644
--- a/drivers/bus/fslmc/private.h
+++ b/drivers/bus/fslmc/private.h
@@ -11,4 +11,6 @@
 
 extern struct rte_bus rte_fslmc_bus;
 
+void fslmc_bus_remove_device(struct rte_dpaa2_device *dev);
+
 #endif /* BUS_FSLMC_PRIVATE_H */
-- 
2.54.0

[RFC v2 07/16] bus/fslmc: fix per type device count

[David Marchand]

When blocklisting some device, the per type count was wrong.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changes since RFC v1:
- moved counter decrement to the free helper,

---
 drivers/bus/fslmc/fslmc_bus.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
index 322b023652..e4aed1bb1a 100644
--- a/drivers/bus/fslmc/fslmc_bus.c
+++ b/drivers/bus/fslmc/fslmc_bus.c
@@ -83,6 +83,9 @@ insert_in_device_list(struct rte_dpaa2_device *newdev)
 
 	if (!inserted)
 		rte_bus_add_device(&rte_fslmc_bus, &newdev->device);
+
+	if (newdev->dev_type < DPAA2_DEVTYPE_MAX)
+		fslmc_bus_device_count[newdev->dev_type]++;
 }
 
 static void
@@ -90,6 +93,8 @@ fslmc_free_device(struct rte_device *rte_dev)
 {
 	struct rte_dpaa2_device *dev = RTE_BUS_DEVICE(rte_dev, *dev);
 
+	if (dev->dev_type < DPAA2_DEVTYPE_MAX)
+		fslmc_bus_device_count[dev->dev_type]--;
 	rte_intr_instance_free(dev->intr_handle);
 	free(dev);
 }
@@ -203,9 +208,6 @@ scan_one_fslmc_device(char *dev_name)
 	dev->device.name = dev->name;
 	dev->device.devargs = rte_bus_find_devargs(&rte_fslmc_bus, dev_name);
 
-	/* Update the device found into the device_count table */
-	fslmc_bus_device_count[dev->dev_type]++;
-
 	/* Add device in the fslmc device list */
 	insert_in_device_list(dev);
 
-- 
2.54.0

[RFC v2 08/16] bus/fslmc: fix some VFIO device FD and memory leaks

[David Marchand]

In case VFIO_DEVICE_GET_INFO fails, the vfio_dev_fd was leaked.
In a similar way, failing to add this device FD in the group list would
result in leaking the FD.

When uninitialising a device or doing a bus cleanup, the per device VFIO
FD and the associated memory used for tracking it were leaked.

Fixes: 57cb02edf122 ("bus/fslmc: enhance MC VFIO multi-process support")

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/bus/fslmc/fslmc_vfio.c | 45 ++++++++++++++++++++++------------
 1 file changed, 30 insertions(+), 15 deletions(-)

diff --git a/drivers/bus/fslmc/fslmc_vfio.c b/drivers/bus/fslmc/fslmc_vfio.c
index 6e0b35f391..705f5aeffc 100644
--- a/drivers/bus/fslmc/fslmc_vfio.c
+++ b/drivers/bus/fslmc/fslmc_vfio.c
@@ -205,13 +205,17 @@ static int
 fslmc_vfio_clear_group(int vfio_group_fd)
 {
 	struct fslmc_vfio_group *group;
-	struct fslmc_vfio_device *dev;
 	int clear = 0;
 
 	LIST_FOREACH(group, &s_vfio_container.groups, next) {
 		if (group->fd == vfio_group_fd) {
-			LIST_FOREACH(dev, &group->vfio_devices, next)
+			while (!LIST_EMPTY(&group->vfio_devices)) {
+				struct fslmc_vfio_device *dev = LIST_FIRST(&group->vfio_devices);
+
+				close(dev->fd);
 				LIST_REMOVE(dev, next);
+				rte_free(dev);
+			}
 
 			close(vfio_group_fd);
 			LIST_REMOVE(group, next);
@@ -311,6 +315,8 @@ fslmc_vfio_group_add_dev(int vfio_group_fd,
 		if (group->fd == vfio_group_fd) {
 			dev = rte_zmalloc(NULL,
 				sizeof(struct fslmc_vfio_device), 0);
+			if (dev == NULL)
+				return -ENOMEM;
 			dev->fd = dev_fd;
 			rte_strscpy(dev->dev_name, name, sizeof(dev->dev_name));
 			LIST_INSERT_HEAD(&group->vfio_devices, dev, next);
@@ -326,26 +332,25 @@ fslmc_vfio_group_remove_dev(int vfio_group_fd,
 {
 	struct fslmc_vfio_group *group = NULL;
 	struct fslmc_vfio_device *dev;
-	int removed = 0;
 
 	LIST_FOREACH(group, &s_vfio_container.groups, next) {
 		if (group->fd == vfio_group_fd)
 			break;
 	}
 
-	if (group) {
-		LIST_FOREACH(dev, &group->vfio_devices, next) {
-			if (!strcmp(dev->dev_name, name)) {
-				LIST_REMOVE(dev, next);
-				removed = 1;
-				break;
-			}
+	if (group == NULL)
+		goto err;
+
+	LIST_FOREACH(dev, &group->vfio_devices, next) {
+		if (strcmp(dev->dev_name, name) == 0) {
+			close(dev->fd);
+			LIST_REMOVE(dev, next);
+			rte_free(dev);
+			return 0;
 		}
 	}
 
-	if (removed)
-		return 0;
-
+err:
 	return -ENODEV;
 }
 
@@ -1240,11 +1245,21 @@ fslmc_vfio_setup_device(const char *dev_addr,
 	if (ret) {
 		DPAA2_BUS_ERR("%s cannot get device info err(%d)(%s)",
 			dev_addr, errno, strerror(errno));
+		close(*vfio_dev_fd);
+		*vfio_dev_fd = -1;
 		return ret;
 	}
 
-	return fslmc_vfio_group_add_dev(vfio_group_fd, *vfio_dev_fd,
-			dev_addr);
+	ret = fslmc_vfio_group_add_dev(vfio_group_fd, *vfio_dev_fd, dev_addr);
+	if (ret) {
+		DPAA2_BUS_ERR("%s cannot add device in group err(%d)(%s)",
+			dev_addr, ret, strerror(-ret));
+		close(*vfio_dev_fd);
+		*vfio_dev_fd = -1;
+		return ret;
+	}
+
+	return 0;
 }
 
 static intptr_t vfio_map_mcp_obj(const char *mcp_obj)
-- 
2.54.0

[RFC v2 09/16] bus/fslmc: fix interrupt leak in DPIO cleanup

[David Marchand]

The interrupt handle in the DPIO device is separate from the bus device
object handle. The cleanup code was leaking this handle.

Fixes: 274fd921ff7f ("bus/fslmc: support close operation")

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/bus/fslmc/portal/dpaa2_hw_dpio.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c b/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
index e17050b625..71996b2b38 100644
--- a/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
+++ b/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
@@ -470,6 +470,7 @@ dpaa2_close_dpio_device(int object_id)
 			rte_free(dpio_dev->dpio);
 		}
 		TAILQ_REMOVE(&dpio_dev_list, dpio_dev, next);
+		rte_intr_instance_free(dpio_dev->intr_handle);
 		rte_free(dpio_dev);
 	}
 }
-- 
2.54.0

[RFC v2 10/16] bus/fslmc: simplify device parsing in scan

[David Marchand]

Refactor string manipulations and avoid unneeded allocation.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changes since RFC v1:
- declared dev_types as static const,
- restored dev_id parsing for "unknown" devices,

---
 drivers/bus/fslmc/fslmc_bus.c | 87 ++++++++++++++++-------------------
 1 file changed, 39 insertions(+), 48 deletions(-)

diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
index e4aed1bb1a..2f66484ad2 100644
--- a/drivers/bus/fslmc/fslmc_bus.c
+++ b/drivers/bus/fslmc/fslmc_bus.c
@@ -123,18 +123,47 @@ dump_device_list(void)
 static int
 scan_one_fslmc_device(char *dev_name)
 {
-	char *dup_dev_name, *t_ptr;
+	enum rte_dpaa2_dev_type dev_type = DPAA2_UNKNOWN;
 	struct rte_dpaa2_device *dev = NULL;
+	static const struct {
+		const char *prefix;
+		enum rte_dpaa2_dev_type type;
+	} dev_types[] = {
+		{ "dpni.", DPAA2_ETH },
+		{ "dpseci.", DPAA2_CRYPTO },
+		{ "dpcon.", DPAA2_CON },
+		{ "dpbp.", DPAA2_BPOOL },
+		{ "dpio.", DPAA2_IO },
+		{ "dpci.", DPAA2_CI },
+		{ "dpmcp.", DPAA2_MPORTAL },
+		{ "dpdmai.", DPAA2_QDMA },
+		{ "dpdmux.", DPAA2_MUX },
+		{ "dprtc.", DPAA2_DPRTC },
+		{ "dprc.", DPAA2_DPRC },
+	};
+	char *dev_id;
 	int ret = -1;
 
-	if (!dev_name)
+	if (dev_name == NULL)
 		return ret;
 
-	/* Creating a temporary copy to perform cut-parse over string */
-	dup_dev_name = strdup(dev_name);
-	if (!dup_dev_name) {
-		DPAA2_BUS_ERR("Unable to allocate device name memory");
-		return -ENOMEM;
+	for (unsigned int i = 0; i < RTE_DIM(dev_types); i++) {
+		if (strncmp(dev_types[i].prefix, dev_name, strlen(dev_types[i].prefix)) != 0)
+			continue;
+		dev_id = dev_name + strlen(dev_types[i].prefix);
+		dev_type = dev_types[i].type;
+		break;
+	}
+
+	if (dev_type == DPAA2_UNKNOWN) {
+		char *ptr = strchr(dev_name, '.');
+
+		if (ptr == NULL) {
+			DPAA2_BUS_ERR("Invalid device found: (%s)", dev_name);
+			ret = 0;
+			goto cleanup;
+		}
+		dev_id = ptr + 1;
 	}
 
 	/* For all other devices, we allocate rte_dpaa2_device.
@@ -145,11 +174,11 @@ scan_one_fslmc_device(char *dev_name)
 	dev = calloc(1, sizeof(struct rte_dpaa2_device));
 	if (!dev) {
 		DPAA2_BUS_ERR("Unable to allocate device object");
-		free(dup_dev_name);
 		return -ENOMEM;
 	}
 
 	dev->device.numa_node = SOCKET_ID_ANY;
+	dev->dev_type = dev_type;
 
 	/* Allocate interrupt instance */
 	dev->intr_handle =
@@ -160,46 +189,12 @@ scan_one_fslmc_device(char *dev_name)
 		goto cleanup;
 	}
 
-	/* Parse the device name and ID */
-	t_ptr = strtok(dup_dev_name, ".");
-	if (!t_ptr) {
-		DPAA2_BUS_ERR("Invalid device found: (%s)", dup_dev_name);
+	if (sscanf(dev_id, "%hu", &dev->object_id) != 1) {
+		DPAA2_BUS_ERR("Failed to parse object ID");
 		ret = -EINVAL;
 		goto cleanup;
 	}
-	if (!strncmp("dpni", t_ptr, 4))
-		dev->dev_type = DPAA2_ETH;
-	else if (!strncmp("dpseci", t_ptr, 6))
-		dev->dev_type = DPAA2_CRYPTO;
-	else if (!strncmp("dpcon", t_ptr, 5))
-		dev->dev_type = DPAA2_CON;
-	else if (!strncmp("dpbp", t_ptr, 4))
-		dev->dev_type = DPAA2_BPOOL;
-	else if (!strncmp("dpio", t_ptr, 4))
-		dev->dev_type = DPAA2_IO;
-	else if (!strncmp("dpci", t_ptr, 4))
-		dev->dev_type = DPAA2_CI;
-	else if (!strncmp("dpmcp", t_ptr, 5))
-		dev->dev_type = DPAA2_MPORTAL;
-	else if (!strncmp("dpdmai", t_ptr, 6))
-		dev->dev_type = DPAA2_QDMA;
-	else if (!strncmp("dpdmux", t_ptr, 6))
-		dev->dev_type = DPAA2_MUX;
-	else if (!strncmp("dprtc", t_ptr, 5))
-		dev->dev_type = DPAA2_DPRTC;
-	else if (!strncmp("dprc", t_ptr, 4))
-		dev->dev_type = DPAA2_DPRC;
-	else
-		dev->dev_type = DPAA2_UNKNOWN;
-
-	t_ptr = strtok(NULL, ".");
-	if (!t_ptr) {
-		DPAA2_BUS_ERR("Skipping invalid device (%s)", dup_dev_name);
-		ret = 0;
-		goto cleanup;
-	}
 
-	sscanf(t_ptr, "%hu", &dev->object_id);
 	if (rte_strscpy(dev->name, dev_name, sizeof(dev->name)) < 0) {
 		DPAA2_BUS_ERR("Unable to copy device name: %s", rte_strerror(rte_errno));
 		ret = -rte_errno;
@@ -211,12 +206,8 @@ scan_one_fslmc_device(char *dev_name)
 	/* Add device in the fslmc device list */
 	insert_in_device_list(dev);
 
-	/* Don't need the duplicated device filesystem entry anymore */
-	free(dup_dev_name);
-
 	return 0;
 cleanup:
-	free(dup_dev_name);
 	if (dev) {
 		rte_intr_instance_free(dev->intr_handle);
 		free(dev);
-- 
2.54.0

[RFC v2 11/16] bus/fslmc: release resources on scan failure

[David Marchand]

In case of a single failure, all mapped memory, VFIO resources and
device pointers were left untouched.
When reaching one init failure during scan, unwind what had been done so
far.

Note: the mapping helper was ignoring the memory callback registration
failure, so it is kept as is.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/bus/fslmc/fslmc_bus.c  | 38 +++++++++++------------
 drivers/bus/fslmc/fslmc_vfio.c | 56 +++++++++++++++++++++++++++++-----
 drivers/bus/fslmc/fslmc_vfio.h |  1 +
 3 files changed, 68 insertions(+), 27 deletions(-)

diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
index 2f66484ad2..802617c5fe 100644
--- a/drivers/bus/fslmc/fslmc_bus.c
+++ b/drivers/bus/fslmc/fslmc_bus.c
@@ -341,23 +341,20 @@ rte_fslmc_scan(void)
 
 	/* Scan the DPRC container object */
 	ret = scan_one_fslmc_device(group_name);
-	if (ret != 0) {
-		/* Error in parsing directory - exit gracefully */
-		goto scan_fail_cleanup;
-	}
-
-	while ((entry = readdir(dir)) != NULL) {
-		if (entry->d_name[0] == '.' || entry->d_type != DT_DIR)
-			continue;
+	if (ret == 0) {
+		while ((entry = readdir(dir)) != NULL) {
+			if (entry->d_name[0] == '.' || entry->d_type != DT_DIR)
+				continue;
 
-		ret = scan_one_fslmc_device(entry->d_name);
-		if (ret != 0) {
-			/* Error in parsing directory - exit gracefully */
-			goto scan_fail_cleanup;
+			ret = scan_one_fslmc_device(entry->d_name);
+			if (ret != 0)
+				break;
 		}
 	}
 
 	closedir(dir);
+	if (ret != 0)
+		goto scan_fail;
 
 	DPAA2_BUS_INFO("FSLMC Bus scan completed");
 	/* If debugging is enabled, device list is dumped to log output */
@@ -375,13 +372,13 @@ rte_fslmc_scan(void)
 			rte_mbuf_dynfield_register(&dpaa2_seqn_dynfield_desc);
 		if (dpaa2_seqn_dynfield_offset < 0) {
 			DPAA2_BUS_ERR("Failed to register mbuf field for dpaa sequence number");
-			return 0;
+			goto scan_fail;
 		}
 
 		ret = fslmc_vfio_setup_group();
 		if (ret) {
 			DPAA2_BUS_ERR("Unable to setup VFIO %d", ret);
-			return 0;
+			goto scan_fail;
 		}
 
 		/* Map existing segments as well as, in case of hotpluggable memory,
@@ -392,14 +389,14 @@ rte_fslmc_scan(void)
 			if (ret) {
 				DPAA2_BUS_ERR("Unable to DMA map existing VAs: (%d)", ret);
 				DPAA2_BUS_ERR("FSLMC VFIO Mapping failed");
-				return 0;
+				goto vfio_close_group;
 			}
 		}
 
 		ret = fslmc_vfio_process_group();
 		if (ret) {
 			DPAA2_BUS_ERR("Unable to setup devices %d", ret);
-			return 0;
+			goto vfio_dma_unmap;
 		}
 	}
 
@@ -407,13 +404,16 @@ rte_fslmc_scan(void)
 
 	return 0;
 
-scan_fail_cleanup:
-	closedir(dir);
+vfio_dma_unmap:
+	fslmc_vfio_dmaunmap();
+vfio_close_group:
+	fslmc_vfio_close_group();
 
+scan_fail:
 	/* Remove all devices in the list */
 	RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus)
 		fslmc_bus_remove_device(dev);
-scan_fail:
+
 	DPAA2_BUS_DEBUG("FSLMC Bus Not Available. Skipping (%d)", ret);
 	/* Irrespective of failure, scan only return success */
 	return 0;
diff --git a/drivers/bus/fslmc/fslmc_vfio.c b/drivers/bus/fslmc/fslmc_vfio.c
index 705f5aeffc..03cac79c50 100644
--- a/drivers/bus/fslmc/fslmc_vfio.c
+++ b/drivers/bus/fslmc/fslmc_vfio.c
@@ -1157,6 +1157,27 @@ fslmc_dmamap_seg(const struct rte_memseg_list *msl __rte_unused,
 	return ret;
 }
 
+static int
+fslmc_dmaunmap_seg(const struct rte_memseg_list *msl __rte_unused,
+		const struct rte_memseg *ms, void *arg)
+{
+	int *n_segs = arg;
+	int ret;
+
+	/* if IOVA address is invalid, skip */
+	if (ms->iova == RTE_BAD_IOVA)
+		return 0;
+
+	ret = fslmc_unmap_dma(ms->addr_64, ms->iova, ms->len);
+	if (ret)
+		DPAA2_BUS_ERR("Unable to VFIO unmap (addr=%p, len=%zu)",
+				ms->addr, ms->len);
+	else
+		(*n_segs)++;
+
+	return ret;
+}
+
 RTE_EXPORT_SYMBOL(rte_fslmc_vfio_mem_dmamap)
 int
 rte_fslmc_vfio_mem_dmamap(uint64_t vaddr, uint64_t iova, uint64_t size)
@@ -1180,10 +1201,8 @@ fslmc_vfio_dmamap(void)
 	rte_mcfg_mem_read_lock();
 
 	ret = rte_memseg_walk(fslmc_dmamap_seg, &i);
-	if (ret) {
-		rte_mcfg_mem_read_unlock();
-		return ret;
-	}
+	if (ret != 0)
+		goto unmap;
 
 	ret = rte_mem_event_callback_register("fslmc_memevent_clb",
 			fslmc_memevent_cb, NULL);
@@ -1196,12 +1215,33 @@ fslmc_vfio_dmamap(void)
 
 	DPAA2_BUS_DEBUG("Total %d segments found.", i);
 
-	/* Existing segments have been mapped and memory callback for hotplug
-	 * has been installed.
-	 */
+	/* Ignore callback handler registration failure */
+	ret = 0;
+
+unmap:
+	if (ret != 0) {
+		i = 0;
+		rte_memseg_walk(fslmc_dmaunmap_seg, &i);
+	}
+
 	rte_mcfg_mem_read_unlock();
 
-	return 0;
+	return ret;
+}
+
+int
+fslmc_vfio_dmaunmap(void)
+{
+	int i = 0, ret;
+
+	rte_mcfg_mem_read_lock();
+
+	rte_mem_event_callback_unregister("fslmc_memevent_clb", NULL);
+	ret = rte_memseg_walk(fslmc_dmaunmap_seg, &i);
+
+	rte_mcfg_mem_read_unlock();
+
+	return ret;
 }
 
 static int
diff --git a/drivers/bus/fslmc/fslmc_vfio.h b/drivers/bus/fslmc/fslmc_vfio.h
index c995fd67b8..57fe7038de 100644
--- a/drivers/bus/fslmc/fslmc_vfio.h
+++ b/drivers/bus/fslmc/fslmc_vfio.h
@@ -60,4 +60,5 @@ int fslmc_vfio_close_group(void);
 char *fslmc_get_container(void);
 int fslmc_get_container_group(const char *group_name, int *gropuid);
 int fslmc_vfio_dmamap(void);
+int fslmc_vfio_dmaunmap(void);
 #endif /* _FSLMC_VFIO_H_ */
-- 
2.54.0

[RFC v2 12/16] bus/fslmc: refactor device filtering for multiprocess

[David Marchand]

Refactor device filtering logic by splitting what was done in
fslmc_vfio_process_group().

Separate control devices from other bus devices.

During scan, do not add device to the control list unless needed
(wrt secondary process, blocklist ...). Yet, keep a special
case for MPORTAL/DPIO objects and filter them in a new
fslmc_filter_control_devices() helper.

This helper is also responsible for selecting the right MPORTAL/DPIO
objects depending on primary/secondary considerations.

As a consequence, fslmc_vfio_process_group() only handles IO
device init without having to care about skipping some device.

Finally, remove now dead code in fslmc_vfio_close_group() since
only used devices are left in the control and bus device lists.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changes since RFC v1:
- split DPAA2 devices in two lists, one for EAL and one for the internal
  devices initialisation. This new list is kept ordered,
- fixed regressions raised by Stephen AI review:
  - allowlist issue: internal devices must be exempted from allowlist
    in some cases, so I simply reproduced the existing custom logic,
  - device pruning with only one DPIO device,
  - (unused) MPORTAL objects kept in device list,

---
 drivers/bus/fslmc/fslmc_bus.c  | 202 ++++++++++++++++++++++++++++++--
 drivers/bus/fslmc/fslmc_vfio.c | 205 ++++++++-------------------------
 drivers/bus/fslmc/private.h    |   4 +
 3 files changed, 250 insertions(+), 161 deletions(-)

diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
index 802617c5fe..4789ab0cf3 100644
--- a/drivers/bus/fslmc/fslmc_bus.c
+++ b/drivers/bus/fslmc/fslmc_bus.c
@@ -30,6 +30,15 @@
 struct rte_bus rte_fslmc_bus;
 static int fslmc_bus_device_count[DPAA2_DEVTYPE_MAX];
 
+/*
+ * Control devices (MPORTAL, IO, CON, CI, BPOOL, MUX, DPRC) are bus infrastructure,
+ * not regular devices. They are initialized during scan/setup and don't participate
+ * in normal probe/unplug. Keep them in a separate list to avoid interference with
+ * generic bus operations.
+ */
+struct fslmc_control_device_list fslmc_control_devices =
+	TAILQ_HEAD_INITIALIZER(fslmc_control_devices);
+
 #define DPAA2_SEQN_DYNFIELD_NAME "dpaa2_seqn_dynfield"
 RTE_EXPORT_INTERNAL_SYMBOL(dpaa2_seqn_dynfield_offset)
 int dpaa2_seqn_dynfield_offset = -1;
@@ -106,10 +115,44 @@ fslmc_bus_remove_device(struct rte_dpaa2_device *dev)
 	fslmc_free_device(&dev->device);
 }
 
+static void
+insert_in_control_device_list(struct rte_dpaa2_device *dev)
+{
+	struct rte_device *rte_dev;
+	struct rte_device *prev;
+
+	prev = NULL;
+	TAILQ_FOREACH(rte_dev, &fslmc_control_devices, next) {
+		if (compare_dpaa2_devname(dev, RTE_BUS_DEVICE(rte_dev, *dev)) < 0)
+			break;
+		prev = rte_dev;
+	}
+
+	if (prev != NULL)
+		TAILQ_INSERT_AFTER(&fslmc_control_devices, prev, &dev->device, next);
+	else
+		TAILQ_INSERT_HEAD(&fslmc_control_devices, &dev->device, next);
+
+	if (dev->dev_type < DPAA2_DEVTYPE_MAX)
+		fslmc_bus_device_count[dev->dev_type]++;
+}
+
+void
+fslmc_remove_control_device(struct rte_dpaa2_device *dev)
+{
+	if (dev->dev_type < DPAA2_DEVTYPE_MAX)
+		fslmc_bus_device_count[dev->dev_type]--;
+
+	TAILQ_REMOVE(&fslmc_control_devices, &dev->device, next);
+	rte_intr_instance_free(dev->intr_handle);
+	free(dev);
+}
+
 static void
 dump_device_list(void)
 {
 	struct rte_dpaa2_device *dev;
+	struct rte_device *rte_dev;
 
 	/* Only if the log level has been set to Debugging, print list */
 	if (rte_log_can_log(dpaa2_logtype_bus, RTE_LOG_DEBUG)) {
@@ -117,6 +160,11 @@ dump_device_list(void)
 		RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus) {
 			DPAA2_BUS_LOG(DEBUG, "\t\t%s", dev->device.name);
 		}
+		DPAA2_BUS_LOG(DEBUG, "List of control devices:");
+		TAILQ_FOREACH(rte_dev, &fslmc_control_devices, next) {
+			dev = RTE_BUS_DEVICE(rte_dev, struct rte_dpaa2_device);
+			DPAA2_BUS_LOG(DEBUG, "\t\t%s", dev->device.name);
+		}
 	}
 }
 
@@ -166,11 +214,39 @@ scan_one_fslmc_device(char *dev_name)
 		dev_id = ptr + 1;
 	}
 
-	/* For all other devices, we allocate rte_dpaa2_device.
-	 * For those devices where there is no driver, probe would release
-	 * the memory associated with the rte_dpaa2_device after necessary
-	 * initialization.
+	/*
+	 * DPAA2_MPORTAL and DPAA2_IO types are handled separately,
+	 * see fslmc_filter_control_devices()
+	 *
+	 * Note: Only check for explicit blocklist (RTE_DEV_BLOCKED).
+	 * Control objects (dpbp, dpcon, etc.) are required even in allowlist
+	 * mode as they are initialized by fslmc_vfio_process_group(), not probed.
 	 */
+	if (dev_type != DPAA2_MPORTAL && dev_type != DPAA2_IO) {
+		struct rte_devargs *devargs = rte_bus_find_devargs(&rte_fslmc_bus, dev_name);
+
+		if (devargs && devargs->policy == RTE_DEV_BLOCKED) {
+			DPAA2_BUS_DEBUG("Skipping blocklisted device (%s)", dev_name);
+			return 0;
+		}
+	}
+
+	/* For secondary processes, control objects are not needed */
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
+		switch (dev_type) {
+		case DPAA2_ETH:
+		case DPAA2_CRYPTO:
+		case DPAA2_QDMA:
+		case DPAA2_IO:
+		case DPAA2_MPORTAL:
+		case DPAA2_DPRC:
+			break;
+		default:
+			DPAA2_BUS_DEBUG("Skipping device in secondary process (%s)", dev_name);
+			return 0;
+		}
+	}
+
 	dev = calloc(1, sizeof(struct rte_dpaa2_device));
 	if (!dev) {
 		DPAA2_BUS_ERR("Unable to allocate device object");
@@ -203,8 +279,16 @@ scan_one_fslmc_device(char *dev_name)
 	dev->device.name = dev->name;
 	dev->device.devargs = rte_bus_find_devargs(&rte_fslmc_bus, dev_name);
 
-	/* Add device in the fslmc device list */
-	insert_in_device_list(dev);
+	switch (dev_type) {
+	case DPAA2_ETH:
+	case DPAA2_CRYPTO:
+	case DPAA2_QDMA:
+		insert_in_device_list(dev);
+		break;
+	default:
+		insert_in_control_device_list(dev);
+		break;
+	}
 
 	return 0;
 cleanup:
@@ -298,6 +382,97 @@ fslmc_dev_compare(const char *name1, const char *name2)
 	return strncmp(devname1, devname2, sizeof(devname1));
 }
 
+static int
+fslmc_filter_control_devices(void)
+{
+	bool is_dpmcp_in_blocklist = false, is_dpio_in_blocklist = false;
+	int dpmcp_count = 0, dpio_count = 0;
+	struct rte_dpaa2_device *dev;
+	struct rte_device *rte_dev;
+	struct rte_device *tmp_dev;
+
+	/* Track MPORTAL/DPIO blocklists */
+	RTE_TAILQ_FOREACH_SAFE(rte_dev, &fslmc_control_devices, next, tmp_dev) {
+		dev = RTE_BUS_DEVICE(rte_dev, *dev);
+		if (dev->dev_type != DPAA2_MPORTAL && dev->dev_type != DPAA2_IO)
+			continue;
+		/* Only check for explicit blocklist, not allowlist */
+		if (dev->device.devargs && dev->device.devargs->policy == RTE_DEV_BLOCKED) {
+			DPAA2_BUS_LOG(DEBUG, "%s Blocked, skipping", dev->device.name);
+			if (dev->dev_type == DPAA2_MPORTAL)
+				is_dpmcp_in_blocklist = true;
+			else if (dev->dev_type == DPAA2_IO)
+				is_dpio_in_blocklist = true;
+			fslmc_remove_control_device(dev);
+			continue;
+		}
+		if (dev->dev_type == DPAA2_MPORTAL)
+			dpmcp_count++;
+		else if (dev->dev_type == DPAA2_IO)
+			dpio_count++;
+	}
+
+	if (dpmcp_count == 0) {
+		DPAA2_BUS_ERR("No MC Portal device found");
+		return -ENODEV;
+	}
+
+	/* Automatic MPORTAL split: primary keeps first, secondary keeps last */
+	if (!is_dpmcp_in_blocklist) {
+		int current_device = 0;
+		int keep_index;
+
+		/* Check MPORTAL availability for secondary */
+		if (rte_eal_process_type() == RTE_PROC_SECONDARY && dpmcp_count < 2) {
+			DPAA2_BUS_ERR("No MC Portal device found for secondary");
+			return -ENODEV;
+		}
+
+		if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+			keep_index = 0;
+		else
+			keep_index = dpmcp_count - 1;
+
+		RTE_TAILQ_FOREACH_SAFE(rte_dev, &fslmc_control_devices, next, tmp_dev) {
+			dev = RTE_BUS_DEVICE(rte_dev, *dev);
+			if (dev->dev_type != DPAA2_MPORTAL)
+				continue;
+			if (current_device != keep_index)
+				fslmc_remove_control_device(dev);
+
+			current_device++;
+			if (current_device == dpmcp_count)
+				break;
+		}
+	}
+
+	/* Automatic DPIO split: secondary keeps last only, primary removes last */
+	if (!is_dpio_in_blocklist && dpio_count > 1) {
+		int last_index = dpio_count - 1;
+		int current_device = 0;
+
+		RTE_TAILQ_FOREACH_SAFE(rte_dev, &fslmc_control_devices, next, tmp_dev) {
+			dev = RTE_BUS_DEVICE(rte_dev, *dev);
+			if (dev->dev_type != DPAA2_IO)
+				continue;
+
+			if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
+					current_device != last_index) {
+				fslmc_remove_control_device(dev);
+			} else if (rte_eal_process_type() == RTE_PROC_PRIMARY &&
+					current_device == last_index) {
+				fslmc_remove_control_device(dev);
+			}
+
+			current_device++;
+			if (current_device == dpio_count)
+				break;
+		}
+	}
+
+	return 0;
+}
+
 static int
 rte_fslmc_scan(void)
 {
@@ -361,7 +536,7 @@ rte_fslmc_scan(void)
 	dump_device_list();
 
 	/* Bus initialization - only if devices were found */
-	if (!TAILQ_EMPTY(&rte_fslmc_bus.device_list)) {
+	if (!TAILQ_EMPTY(&rte_fslmc_bus.device_list) || !TAILQ_EMPTY(&fslmc_control_devices)) {
 		static const struct rte_mbuf_dynfield dpaa2_seqn_dynfield_desc = {
 			.name = DPAA2_SEQN_DYNFIELD_NAME,
 			.size = sizeof(dpaa2_seqn_t),
@@ -393,6 +568,12 @@ rte_fslmc_scan(void)
 			}
 		}
 
+		ret = fslmc_filter_control_devices();
+		if (ret) {
+			DPAA2_BUS_ERR("Unable to filter control devices %d", ret);
+			goto vfio_dma_unmap;
+		}
+
 		ret = fslmc_vfio_process_group();
 		if (ret) {
 			DPAA2_BUS_ERR("Unable to setup devices %d", ret);
@@ -410,6 +591,13 @@ rte_fslmc_scan(void)
 	fslmc_vfio_close_group();
 
 scan_fail:
+	while (!TAILQ_EMPTY(&fslmc_control_devices)) {
+		struct rte_device *rte_dev = TAILQ_FIRST(&fslmc_control_devices);
+
+		dev = RTE_BUS_DEVICE(rte_dev, *dev);
+		fslmc_remove_control_device(dev);
+	}
+
 	/* Remove all devices in the list */
 	RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus)
 		fslmc_bus_remove_device(dev);
diff --git a/drivers/bus/fslmc/fslmc_vfio.c b/drivers/bus/fslmc/fslmc_vfio.c
index 03cac79c50..954167f452 100644
--- a/drivers/bus/fslmc/fslmc_vfio.c
+++ b/drivers/bus/fslmc/fslmc_vfio.c
@@ -1599,6 +1599,7 @@ int
 fslmc_vfio_close_group(void)
 {
 	struct rte_dpaa2_device *dev;
+	struct rte_device *rte_dev;
 	int vfio_group_fd;
 	const char *group_name = fslmc_vfio_get_group_name();
 
@@ -1611,30 +1612,14 @@ fslmc_vfio_close_group(void)
 		return -EIO;
 	}
 
-	RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus) {
-		switch (dev->dev_type) {
-		case DPAA2_ETH:
-		case DPAA2_CRYPTO:
-		case DPAA2_QDMA:
-		case DPAA2_IO:
-			fslmc_close_iodevices(dev, vfio_group_fd);
-			break;
-		case DPAA2_CON:
-		case DPAA2_CI:
-		case DPAA2_BPOOL:
-		case DPAA2_MUX:
-			if (rte_eal_process_type() == RTE_PROC_SECONDARY)
-				continue;
-
-			fslmc_close_iodevices(dev, vfio_group_fd);
-			break;
-		case DPAA2_DPRTC:
-		default:
-			DPAA2_BUS_DEBUG("Device cannot be closed: Not supported (%s)",
-					dev->device.name);
-		}
+	TAILQ_FOREACH(rte_dev, &fslmc_control_devices, next) {
+		dev = RTE_BUS_DEVICE(rte_dev, *dev);
+		fslmc_close_iodevices(dev, vfio_group_fd);
 	}
 
+	RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus)
+		fslmc_close_iodevices(dev, vfio_group_fd);
+
 	fslmc_vfio_clear_group(vfio_group_fd);
 
 	return 0;
@@ -1643,168 +1628,80 @@ fslmc_vfio_close_group(void)
 int
 fslmc_vfio_process_group(void)
 {
-	int ret;
-	int found_mportal = 0;
 	struct rte_dpaa2_device *dev;
-	bool is_dpmcp_in_blocklist = false, is_dpio_in_blocklist = false;
-	int dpmcp_count = 0, dpio_count = 0, current_device;
-
-	RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus) {
-		if (dev->dev_type == DPAA2_MPORTAL) {
-			dpmcp_count++;
-			if (dev->device.devargs &&
-			    dev->device.devargs->policy == RTE_DEV_BLOCKED)
-				is_dpmcp_in_blocklist = true;
-		}
-		if (dev->dev_type == DPAA2_IO) {
-			dpio_count++;
-			if (dev->device.devargs &&
-			    dev->device.devargs->policy == RTE_DEV_BLOCKED)
-				is_dpio_in_blocklist = true;
-		}
-	}
-
-	/* Search the MCP as that should be initialized first. */
-	current_device = 0;
-	RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus) {
-		if (dev->dev_type == DPAA2_MPORTAL) {
-			current_device++;
-			if (dev->device.devargs &&
-			    dev->device.devargs->policy == RTE_DEV_BLOCKED) {
-				DPAA2_BUS_LOG(DEBUG, "%s Blocked, skipping",
-					      dev->device.name);
-				fslmc_bus_remove_device(dev);
-				continue;
-			}
-
-			if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
-			    !is_dpmcp_in_blocklist) {
-				if (dpmcp_count == 1 ||
-				    current_device != dpmcp_count) {
-					fslmc_bus_remove_device(dev);
-					continue;
-				}
-			}
-
-			if (!found_mportal) {
-				ret = fslmc_process_mcp(dev);
-				if (ret) {
-					DPAA2_BUS_ERR("Unable to map MC Portal");
-					return ret;
-				}
-				found_mportal = 1;
-			}
+	struct rte_device *rte_dev;
+	struct rte_device *tmp_dev;
+	int ret;
 
-			fslmc_bus_remove_device(dev);
-			/* Ideally there is only a single dpmcp, but in case
-			 * multiple exists, looping on remaining devices.
-			 */
+	/* Process MPORTAL - should be initialized first */
+	RTE_TAILQ_FOREACH_SAFE(rte_dev, &fslmc_control_devices, next, tmp_dev) {
+		dev = RTE_BUS_DEVICE(rte_dev, *dev);
+		if (dev->dev_type != DPAA2_MPORTAL)
+			continue;
+		ret = fslmc_process_mcp(dev);
+		if (ret) {
+			DPAA2_BUS_ERR("Unable to map MC Portal");
+			return ret;
 		}
+		fslmc_remove_control_device(dev);
+		break;
 	}
 
-	/* Cannot continue if there is not even a single mportal */
-	if (!found_mportal) {
-		DPAA2_BUS_ERR("No MC Portal device found. Not continuing");
-		return -EIO;
+	/* Remove any remaining MPORTAL devices */
+	RTE_TAILQ_FOREACH_SAFE(rte_dev, &fslmc_control_devices, next, tmp_dev) {
+		dev = RTE_BUS_DEVICE(rte_dev, *dev);
+		if (dev->dev_type == DPAA2_MPORTAL)
+			fslmc_remove_control_device(dev);
 	}
 
-	/* Search for DPRC device next as it updates endpoint of
+	/* Process DPRC device next as it updates endpoint of
 	 * other devices.
 	 */
-	current_device = 0;
-	RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus) {
-		if (dev->dev_type == DPAA2_DPRC) {
-			ret = fslmc_process_iodevices(dev);
-			if (ret) {
-				DPAA2_BUS_ERR("Unable to process dprc");
-				return ret;
-			}
-			fslmc_bus_remove_device(dev);
+	RTE_TAILQ_FOREACH_SAFE(rte_dev, &fslmc_control_devices, next, tmp_dev) {
+		dev = RTE_BUS_DEVICE(rte_dev, *dev);
+		if (dev->dev_type != DPAA2_DPRC)
+			continue;
+		ret = fslmc_process_iodevices(dev);
+		if (ret) {
+			DPAA2_BUS_ERR("Unable to process dprc");
+			return ret;
 		}
+		fslmc_remove_control_device(dev);
 	}
 
-	current_device = 0;
-	RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus) {
-		if (dev->dev_type == DPAA2_IO)
-			current_device++;
-		if (dev->device.devargs &&
-		    dev->device.devargs->policy == RTE_DEV_BLOCKED) {
-			DPAA2_BUS_LOG(DEBUG, "%s Blocked, skipping",
-				      dev->device.name);
-			fslmc_bus_remove_device(dev);
-			continue;
-		}
-		if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
-		    dev->dev_type != DPAA2_ETH &&
-		    dev->dev_type != DPAA2_CRYPTO &&
-		    dev->dev_type != DPAA2_QDMA &&
-		    dev->dev_type != DPAA2_IO) {
-			fslmc_bus_remove_device(dev);
-			continue;
-		}
+	/* Process remaining control devices */
+	RTE_TAILQ_FOREACH_SAFE(rte_dev, &fslmc_control_devices, next, tmp_dev) {
+		dev = RTE_BUS_DEVICE(rte_dev, *dev);
 		switch (dev->dev_type) {
-		case DPAA2_ETH:
-		case DPAA2_CRYPTO:
-		case DPAA2_QDMA:
-			ret = fslmc_process_iodevices(dev);
-			if (ret) {
-				DPAA2_BUS_DEBUG("Dev (%s) init failed",
-						dev->device.name);
-				return ret;
-			}
-			break;
 		case DPAA2_CON:
 		case DPAA2_CI:
 		case DPAA2_BPOOL:
 		case DPAA2_DPRTC:
 		case DPAA2_MUX:
-			/* IN case of secondary processes, all control objects
-			 * like dpbp, dpcon, dpci are not initialized/required
-			 * - all of these are assumed to be initialized and made
-			 *   available by primary.
-			 */
-			if (rte_eal_process_type() == RTE_PROC_SECONDARY)
-				continue;
-
-			/* Call the object creation routine and remove the
-			 * device entry from device list
-			 */
-			ret = fslmc_process_iodevices(dev);
-			if (ret) {
-				DPAA2_BUS_DEBUG("Dev (%s) init failed",
-						dev->device.name);
-				return ret;
-			}
-
-			break;
 		case DPAA2_IO:
-			if (!is_dpio_in_blocklist && dpio_count > 1) {
-				if (rte_eal_process_type() == RTE_PROC_SECONDARY
-				    && current_device != dpio_count) {
-					fslmc_bus_remove_device(dev);
-					break;
-				}
-				if (rte_eal_process_type() == RTE_PROC_PRIMARY
-				    && current_device == dpio_count) {
-					fslmc_bus_remove_device(dev);
-					break;
-				}
-			}
-
 			ret = fslmc_process_iodevices(dev);
 			if (ret) {
 				DPAA2_BUS_DEBUG("Dev (%s) init failed",
 						dev->device.name);
 				return ret;
 			}
-
 			break;
-		case DPAA2_UNKNOWN:
 		default:
 			/* Unknown - ignore */
 			DPAA2_BUS_DEBUG("Found unknown device (%s)",
 					dev->device.name);
-			fslmc_bus_remove_device(dev);
+			fslmc_remove_control_device(dev);
+			break;
+		}
+	}
+
+	/* Process regular devices */
+	RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus) {
+		ret = fslmc_process_iodevices(dev);
+		if (ret) {
+			DPAA2_BUS_DEBUG("Dev (%s) init failed",
+					dev->device.name);
+			return ret;
 		}
 	}
 
diff --git a/drivers/bus/fslmc/private.h b/drivers/bus/fslmc/private.h
index 825a364f1b..064ffa9536 100644
--- a/drivers/bus/fslmc/private.h
+++ b/drivers/bus/fslmc/private.h
@@ -11,6 +11,10 @@
 
 extern struct rte_bus rte_fslmc_bus;
 
+RTE_TAILQ_HEAD(fslmc_control_device_list, rte_device);
+extern struct fslmc_control_device_list fslmc_control_devices;
+
 void fslmc_bus_remove_device(struct rte_dpaa2_device *dev);
+void fslmc_remove_control_device(struct rte_dpaa2_device *dev);
 
 #endif /* BUS_FSLMC_PRIVATE_H */
-- 
2.54.0

[RFC v2 13/16] bus/fslmc: move unplug for some device out of VFIO

[David Marchand]

Move device unplug out of the code dealing with VFIO, interrupts and
IO devices.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changes since RFC v1:
- simplified the loop over devices since only devices for ethdev, dma or
  crypto are in the bus device list,

---
 drivers/bus/fslmc/fslmc_bus.c  | 11 ++++++++++-
 drivers/bus/fslmc/fslmc_vfio.c | 13 -------------
 2 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
index 4789ab0cf3..3b22d3367d 100644
--- a/drivers/bus/fslmc/fslmc_bus.c
+++ b/drivers/bus/fslmc/fslmc_bus.c
@@ -620,10 +620,19 @@ fslmc_bus_match(const struct rte_driver *drv, const struct rte_device *dev)
 }
 
 static int
-rte_fslmc_close(struct rte_bus *bus __rte_unused)
+fslmc_bus_unplug_device(struct rte_device *rte_dev);
+
+static int
+rte_fslmc_close(struct rte_bus *bus)
 {
+	struct rte_dpaa2_device *dev;
 	int ret = 0;
 
+	RTE_BUS_FOREACH_DEV(dev, bus) {
+		if (rte_dev_is_probed(&dev->device) && fslmc_bus_unplug_device(&dev->device))
+			DPAA2_BUS_ERR("Unable to remove %s", dev->device.name);
+	}
+
 	ret = fslmc_vfio_close_group();
 	if (ret)
 		DPAA2_BUS_INFO("Unable to close devices %d", ret);
diff --git a/drivers/bus/fslmc/fslmc_vfio.c b/drivers/bus/fslmc/fslmc_vfio.c
index 954167f452..857f22f204 100644
--- a/drivers/bus/fslmc/fslmc_vfio.c
+++ b/drivers/bus/fslmc/fslmc_vfio.c
@@ -1449,7 +1449,6 @@ fslmc_close_iodevices(struct rte_dpaa2_device *dev,
 	int vfio_fd)
 {
 	struct rte_dpaa2_object *object = NULL;
-	const struct rte_dpaa2_driver *drv;
 	int ret;
 
 	switch (dev->dev_type) {
@@ -1465,15 +1464,6 @@ fslmc_close_iodevices(struct rte_dpaa2_device *dev,
 				continue;
 		}
 		break;
-	case DPAA2_ETH:
-	case DPAA2_CRYPTO:
-	case DPAA2_QDMA:
-		if (dev->device.driver != NULL) {
-			drv = RTE_BUS_DRIVER(dev->device.driver, *drv);
-			if (drv->remove && drv->remove(dev))
-				DPAA2_BUS_ERR("Unable to remove");
-		}
-		break;
 	default:
 		break;
 	}
@@ -1617,9 +1607,6 @@ fslmc_vfio_close_group(void)
 		fslmc_close_iodevices(dev, vfio_group_fd);
 	}
 
-	RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus)
-		fslmc_close_iodevices(dev, vfio_group_fd);
-
 	fslmc_vfio_clear_group(vfio_group_fd);
 
 	return 0;
-- 
2.54.0

[RFC v2 14/16] bus/fslmc: call VFIO setup for some device from bus layer

[David Marchand]

Move VFIO setup for ETH, CRYPTO and QDMA devices from fslmc_vfio.c
to fslmc_bus.c scan layer.
Also move interrupt setup for ETH devices.

Add fslmc_vfio_dev_setup() (resp. fslmc_vfio_dev_close()) wrapper
that handles VFIO device fd acquisition and interrupt setup
(resp. device removal), hiding VFIO internals from the bus layer.

Note: before this change, CRYPTO and QDMA devices were getting a
VFIO device FD even though nothing seems to be done with it.
This is kept as is, the device FD is closed on call to
fslmc_vfio_dev_close().

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changes since RFC v1:
- added cleanup on failure,

---
 drivers/bus/fslmc/fslmc_bus.c  | 12 +++++
 drivers/bus/fslmc/fslmc_vfio.c | 83 ++++++++++++++++++++++++++--------
 drivers/bus/fslmc/fslmc_vfio.h |  5 ++
 3 files changed, 82 insertions(+), 18 deletions(-)

diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
index 3b22d3367d..4f4abd19ce 100644
--- a/drivers/bus/fslmc/fslmc_bus.c
+++ b/drivers/bus/fslmc/fslmc_bus.c
@@ -579,12 +579,23 @@ rte_fslmc_scan(void)
 			DPAA2_BUS_ERR("Unable to setup devices %d", ret);
 			goto vfio_dma_unmap;
 		}
+
+		RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus) {
+			ret = fslmc_vfio_dev_setup(dev);
+			if (ret) {
+				DPAA2_BUS_ERR("Dev (%s) VFIO setup failed", dev->device.name);
+				goto vfio_dev_close;
+			}
+		}
 	}
 
 	process_once = 1;
 
 	return 0;
 
+vfio_dev_close:
+	RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus)
+		fslmc_vfio_dev_close(dev);
 vfio_dma_unmap:
 	fslmc_vfio_dmaunmap();
 vfio_close_group:
@@ -631,6 +642,7 @@ rte_fslmc_close(struct rte_bus *bus)
 	RTE_BUS_FOREACH_DEV(dev, bus) {
 		if (rte_dev_is_probed(&dev->device) && fslmc_bus_unplug_device(&dev->device))
 			DPAA2_BUS_ERR("Unable to remove %s", dev->device.name);
+		fslmc_vfio_dev_close(dev);
 	}
 
 	ret = fslmc_vfio_close_group();
diff --git a/drivers/bus/fslmc/fslmc_vfio.c b/drivers/bus/fslmc/fslmc_vfio.c
index 857f22f204..04582e38c6 100644
--- a/drivers/bus/fslmc/fslmc_vfio.c
+++ b/drivers/bus/fslmc/fslmc_vfio.c
@@ -1444,6 +1444,15 @@ rte_dpaa2_vfio_setup_intr(struct rte_intr_handle *intr_handle,
 	return -EIO;
 }
 
+static void
+dpaa2_close_intr(struct rte_intr_handle *intr_handle)
+{
+	if (rte_intr_fd_get(intr_handle) >= 0) {
+		close(rte_intr_fd_get(intr_handle));
+		rte_intr_fd_set(intr_handle, -1);
+	}
+}
+
 static void
 fslmc_close_iodevices(struct rte_dpaa2_device *dev,
 	int vfio_fd)
@@ -1478,8 +1487,7 @@ fslmc_close_iodevices(struct rte_dpaa2_device *dev,
 }
 
 /*
- * fslmc_process_iodevices for processing only IO (ETH, CRYPTO, and possibly
- * EVENT) devices.
+ * fslmc_process_iodevices for processing only IO devices.
  */
 static int
 fslmc_process_iodevices(struct rte_dpaa2_device *dev)
@@ -1494,12 +1502,6 @@ fslmc_process_iodevices(struct rte_dpaa2_device *dev)
 		return ret;
 
 	switch (dev->dev_type) {
-	case DPAA2_ETH:
-		ret = rte_dpaa2_vfio_setup_intr(dev->intr_handle, dev_fd,
-				device_info.num_irqs);
-		if (ret)
-			return ret;
-		break;
 	case DPAA2_CON:
 	case DPAA2_IO:
 	case DPAA2_CI:
@@ -1523,6 +1525,61 @@ fslmc_process_iodevices(struct rte_dpaa2_device *dev)
 	return 0;
 }
 
+int
+fslmc_vfio_dev_setup(struct rte_dpaa2_device *dev)
+{
+	struct vfio_device_info device_info = { .argsz = sizeof(device_info) };
+	int dev_fd;
+	int ret;
+
+	ret = fslmc_vfio_setup_device(dev->device.name, &dev_fd, &device_info);
+	if (ret) {
+		DPAA2_BUS_ERR("VFIO setup failed for %s: %d",
+			      dev->device.name, ret);
+		return ret;
+	}
+
+	if (dev->dev_type == DPAA2_ETH) {
+		ret = rte_dpaa2_vfio_setup_intr(dev->intr_handle, dev_fd,
+						device_info.num_irqs);
+		if (ret) {
+			DPAA2_BUS_ERR("Interrupt setup failed for %s: %d",
+				      dev->device.name, ret);
+			fslmc_vfio_dev_close(dev);
+			return ret;
+		}
+	}
+
+	DPAA2_BUS_DEBUG("Device (%s) VFIO setup completed", dev->device.name);
+	return 0;
+}
+
+int
+fslmc_vfio_dev_close(struct rte_dpaa2_device *dev)
+{
+	int vfio_group_fd;
+	int ret;
+	const char *group_name = fslmc_vfio_get_group_name();
+
+	vfio_group_fd = fslmc_vfio_group_fd_by_name(group_name);
+	if (vfio_group_fd <= 0) {
+		DPAA2_BUS_ERR("Get fd by name(%s) failed(%d)",
+			      group_name, vfio_group_fd);
+		if (vfio_group_fd < 0)
+			return vfio_group_fd;
+		return -EIO;
+	}
+
+	dpaa2_close_intr(dev->intr_handle);
+
+	ret = fslmc_vfio_group_remove_dev(vfio_group_fd, dev->device.name);
+	if (ret)
+		DPAA2_BUS_ERR("Failed to remove %s from vfio", dev->device.name);
+
+	DPAA2_BUS_DEBUG("Device (%s) closed", dev->device.name);
+	return ret;
+}
+
 static int
 fslmc_process_mcp(struct rte_dpaa2_device *dev)
 {
@@ -1682,16 +1739,6 @@ fslmc_vfio_process_group(void)
 		}
 	}
 
-	/* Process regular devices */
-	RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus) {
-		ret = fslmc_process_iodevices(dev);
-		if (ret) {
-			DPAA2_BUS_DEBUG("Dev (%s) init failed",
-					dev->device.name);
-			return ret;
-		}
-	}
-
 	return 0;
 }
 
diff --git a/drivers/bus/fslmc/fslmc_vfio.h b/drivers/bus/fslmc/fslmc_vfio.h
index 57fe7038de..0973ddbe37 100644
--- a/drivers/bus/fslmc/fslmc_vfio.h
+++ b/drivers/bus/fslmc/fslmc_vfio.h
@@ -61,4 +61,9 @@ char *fslmc_get_container(void);
 int fslmc_get_container_group(const char *group_name, int *gropuid);
 int fslmc_vfio_dmamap(void);
 int fslmc_vfio_dmaunmap(void);
+
+struct rte_dpaa2_device;
+int fslmc_vfio_dev_setup(struct rte_dpaa2_device *dev);
+int fslmc_vfio_dev_close(struct rte_dpaa2_device *dev);
+
 #endif /* _FSLMC_VFIO_H_ */
-- 
2.54.0

[RFC v2 15/16] bus/fslmc: allocate interrupt during probing

[David Marchand]

Allocating the interrupt handle is a waste of memory if no device is
probed later (like for example, if a allowlist is passed).

Instead, allocate this handle, set eventfds and vfio at the time
probe_device is called.

Adjust the unplug_device path accordingly.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/bus/fslmc/fslmc_bus.c | 51 +++++++++++++++--------------------
 1 file changed, 22 insertions(+), 29 deletions(-)

diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
index 4f4abd19ce..909edb9124 100644
--- a/drivers/bus/fslmc/fslmc_bus.c
+++ b/drivers/bus/fslmc/fslmc_bus.c
@@ -104,7 +104,6 @@ fslmc_free_device(struct rte_device *rte_dev)
 
 	if (dev->dev_type < DPAA2_DEVTYPE_MAX)
 		fslmc_bus_device_count[dev->dev_type]--;
-	rte_intr_instance_free(dev->intr_handle);
 	free(dev);
 }
 
@@ -144,7 +143,6 @@ fslmc_remove_control_device(struct rte_dpaa2_device *dev)
 		fslmc_bus_device_count[dev->dev_type]--;
 
 	TAILQ_REMOVE(&fslmc_control_devices, &dev->device, next);
-	rte_intr_instance_free(dev->intr_handle);
 	free(dev);
 }
 
@@ -256,15 +254,6 @@ scan_one_fslmc_device(char *dev_name)
 	dev->device.numa_node = SOCKET_ID_ANY;
 	dev->dev_type = dev_type;
 
-	/* Allocate interrupt instance */
-	dev->intr_handle =
-		rte_intr_instance_alloc(RTE_INTR_INSTANCE_F_PRIVATE);
-	if (dev->intr_handle == NULL) {
-		DPAA2_BUS_ERR("Failed to allocate intr handle");
-		ret = -ENOMEM;
-		goto cleanup;
-	}
-
 	if (sscanf(dev_id, "%hu", &dev->object_id) != 1) {
 		DPAA2_BUS_ERR("Failed to parse object ID");
 		ret = -EINVAL;
@@ -292,10 +281,7 @@ scan_one_fslmc_device(char *dev_name)
 
 	return 0;
 cleanup:
-	if (dev) {
-		rte_intr_instance_free(dev->intr_handle);
-		free(dev);
-	}
+	free(dev);
 	return ret;
 }
 
@@ -579,23 +565,12 @@ rte_fslmc_scan(void)
 			DPAA2_BUS_ERR("Unable to setup devices %d", ret);
 			goto vfio_dma_unmap;
 		}
-
-		RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus) {
-			ret = fslmc_vfio_dev_setup(dev);
-			if (ret) {
-				DPAA2_BUS_ERR("Dev (%s) VFIO setup failed", dev->device.name);
-				goto vfio_dev_close;
-			}
-		}
 	}
 
 	process_once = 1;
 
 	return 0;
 
-vfio_dev_close:
-	RTE_BUS_FOREACH_DEV(dev, &rte_fslmc_bus)
-		fslmc_vfio_dev_close(dev);
 vfio_dma_unmap:
 	fslmc_vfio_dmaunmap();
 vfio_close_group:
@@ -642,7 +617,6 @@ rte_fslmc_close(struct rte_bus *bus)
 	RTE_BUS_FOREACH_DEV(dev, bus) {
 		if (rte_dev_is_probed(&dev->device) && fslmc_bus_unplug_device(&dev->device))
 			DPAA2_BUS_ERR("Unable to remove %s", dev->device.name);
-		fslmc_vfio_dev_close(dev);
 	}
 
 	ret = fslmc_vfio_close_group();
@@ -718,10 +692,27 @@ fslmc_bus_probe_device(struct rte_driver *driver, struct rte_device *rte_dev)
 	struct rte_dpaa2_driver *drv = RTE_BUS_DRIVER(driver, *drv);
 	int ret = 0;
 
-	/* FIXME: probe_device should allocate intr_handle */
+	/* Allocate interrupt instance */
+	dev->intr_handle =
+		rte_intr_instance_alloc(RTE_INTR_INSTANCE_F_PRIVATE);
+	if (dev->intr_handle == NULL) {
+		DPAA2_BUS_ERR("Failed to allocate intr handle");
+		return -ENOMEM;
+	}
+
+	ret = fslmc_vfio_dev_setup(dev);
+	if (ret) {
+		DPAA2_BUS_ERR("Dev (%s) VFIO setup failed", dev->device.name);
+		goto release_intr;
+	}
+
 	ret = drv->probe(drv, dev);
 	if (ret != 0) {
 		DPAA2_BUS_ERR("Unable to probe");
+		fslmc_vfio_dev_close(dev);
+release_intr:
+		rte_intr_instance_free(dev->intr_handle);
+		dev->intr_handle = NULL;
 	} else {
 		DPAA2_BUS_INFO("%s Plugged",  dev->device.name);
 	}
@@ -739,7 +730,9 @@ fslmc_bus_unplug_device(struct rte_device *rte_dev)
 		int ret = drv->remove(dev);
 		if (ret != 0)
 			return ret;
-		/* FIXME: unplug_device should free intr_handle */
+		fslmc_vfio_dev_close(dev);
+		rte_intr_instance_free(dev->intr_handle);
+		dev->intr_handle = NULL;
 		DPAA2_BUS_INFO("%s Un-Plugged",  dev->device.name);
 		return 0;
 	}
-- 
2.54.0

[RFC v2 16/16] bus/fslmc: use generic cleanup

[David Marchand]

Now that the probe and unplug callback behave like other buses, we can
call generic cleanup in addition to the special handling for IO devices.

Also cleanup any object left in the control devices list.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changes since RFC v1:
- added a debug log on cleanup failure,

---
 drivers/bus/fslmc/fslmc_bus.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
index 909edb9124..361a123802 100644
--- a/drivers/bus/fslmc/fslmc_bus.c
+++ b/drivers/bus/fslmc/fslmc_bus.c
@@ -606,23 +606,25 @@ fslmc_bus_match(const struct rte_driver *drv, const struct rte_device *dev)
 }
 
 static int
-fslmc_bus_unplug_device(struct rte_device *rte_dev);
-
-static int
-rte_fslmc_close(struct rte_bus *bus)
+fslmc_cleanup(struct rte_bus *bus)
 {
-	struct rte_dpaa2_device *dev;
-	int ret = 0;
+	int ret;
 
-	RTE_BUS_FOREACH_DEV(dev, bus) {
-		if (rte_dev_is_probed(&dev->device) && fslmc_bus_unplug_device(&dev->device))
-			DPAA2_BUS_ERR("Unable to remove %s", dev->device.name);
-	}
+	ret = rte_bus_generic_cleanup(bus);
+	if (ret)
+		DPAA2_BUS_INFO("Unable to cleanup devices %d", ret);
 
 	ret = fslmc_vfio_close_group();
 	if (ret)
 		DPAA2_BUS_INFO("Unable to close devices %d", ret);
 
+	while (!TAILQ_EMPTY(&fslmc_control_devices)) {
+		struct rte_device *rte_dev = TAILQ_FIRST(&fslmc_control_devices);
+		struct rte_dpaa2_device *dev = RTE_BUS_DEVICE(rte_dev, *dev);
+
+		fslmc_remove_control_device(dev);
+	}
+
 	return 0;
 }
 
@@ -743,7 +745,8 @@ fslmc_bus_unplug_device(struct rte_device *rte_dev)
 struct rte_bus rte_fslmc_bus = {
 	.scan = rte_fslmc_scan,
 	.probe = rte_bus_generic_probe,
-	.cleanup = rte_fslmc_close,
+	.free_device = fslmc_free_device,
+	.cleanup = fslmc_cleanup,
 	.parse = rte_fslmc_parse,
 	.dev_compare = fslmc_dev_compare,
 	.find_device = rte_bus_generic_find_device,
-- 
2.54.0

Re: [RFC v2 00/16] Device unplug and bus cleanup refactoring for NXP

[David Marchand]

Hello Hemant,

On Thu, 17 Sept 2026 at 09:49, David Marchand <david.marchand@redhat.com> wrote:
> This is a followup to the refactoring started in 26.07.
>
> I took some time with my best AI friend to cleanup DPAA and FSLMC bus
> drivers.
>
> Like the last time, only compilation has been checked.
> I have no hardware to test runtime.
>
> One thing that could be broken is either the order of devices
> initialisation, or bugs in the device filtering that I tried to
> simplify.
>
> I went with the approach of splitting the device list between
> internal bus object (that I called "control" devices, maybe a better
> name can be found), and normal bus device object for which DPDK has a
> driver.
>
> I think it is worth testing and fixing, as it will make the two NXP
> bus drivers similar to other bus drivers (but keep the special IO devices
> handling internal to the FSLMC bus for example).

Could I get a round of tests? Or should I just sent a non RFC series?

I see potential conflicts with other series coming from NXP.
I think it is worth getting this cleanup in, to limit the number of
special cases for those two bus drivers when compared to other buses.


-- 
David Marchand

Re: [RFC v2 00/16] Device unplug and bus cleanup refactoring for NXP

[Hemant Agrawal]

[-- Attachment #1: Type: text/plain, Size: 1677 bytes --]




________________________________
From: David Marchand <david.marchand@redhat.com>
Sent: Thursday, September 24, 2026 9:46 AM
To: Hemant Agrawal <hemant.agrawal@nxp.com>
Cc: dev <dev@dpdk.org>; Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [RFC v2 00/16] Device unplug and bus cleanup refactoring for NXP

Hello Hemant,

On Thu, 17 Sept 2026 at 09:49, David Marchand <david.marchand@redhat.com> wrote:
> This is a followup to the refactoring started in 26.07.
>
> I took some time with my best AI friend to cleanup DPAA and FSLMC bus
> drivers.
>
> Like the last time, only compilation has been checked.
> I have no hardware to test runtime.
>
> One thing that could be broken is either the order of devices
> initialisation, or bugs in the device filtering that I tried to
> simplify.
>
> I went with the approach of splitting the device list between
> internal bus object (that I called "control" devices, maybe a better
> name can be found), and normal bus device object for which DPDK has a
> driver.
>
> I think it is worth testing and fixing, as it will make the two NXP
> bus drivers similar to other bus drivers (but keep the special IO devices
> handling internal to the FSLMC bus for example).

Could I get a round of tests? Or should I just sent a non RFC series?

I see potential conflicts with other series coming from NXP.
I think it is worth getting this cleanup in, to limit the number of
special cases for those two bus drivers when compared to other buses.

++ @Prashant Gupta<mailto:prashant.gupta_3@nxp.com>
Please confirm your test report on this updated series.

--
David Marchand


NXP Confidential

[-- Attachment #2: Type: text/html, Size: 3404 bytes --]