* [RFC 01/11] drivers/bus: cleanup device freeing in NXP bus scan
2026-07-23 13:53 [RFC 00/11] Device unplug and bus cleanup refactoring for NXP David Marchand
@ 2026-07-23 13:53 ` David Marchand
2026-07-23 13:53 ` [RFC 02/11] bus/dpaa: allocate interrupt during probing David Marchand
` (11 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: David Marchand @ 2026-07-23 13:53 UTC (permalink / raw)
To: hemant.agrawal, dev; +Cc: Sachin Saxena
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [RFC 02/11] bus/dpaa: allocate interrupt during probing
2026-07-23 13:53 [RFC 00/11] Device unplug and bus cleanup refactoring for NXP David Marchand
2026-07-23 13:53 ` [RFC 01/11] drivers/bus: cleanup device freeing in NXP bus scan David Marchand
@ 2026-07-23 13:53 ` David Marchand
2026-07-23 13:53 ` [RFC 03/11] bus/dpaa: support unplug and use generic cleanup David Marchand
` (10 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: David Marchand @ 2026-07-23 13:53 UTC (permalink / raw)
To: hemant.agrawal, dev; +Cc: Sachin Saxena
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [RFC 03/11] bus/dpaa: support unplug and use generic cleanup
2026-07-23 13:53 [RFC 00/11] Device unplug and bus cleanup refactoring for NXP David Marchand
2026-07-23 13:53 ` [RFC 01/11] drivers/bus: cleanup device freeing in NXP bus scan David Marchand
2026-07-23 13:53 ` [RFC 02/11] bus/dpaa: allocate interrupt during probing David Marchand
@ 2026-07-23 13:53 ` David Marchand
2026-07-23 13:53 ` [RFC 04/11] bus/fslmc: fix memory leaks in scan David Marchand
` (9 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: David Marchand @ 2026-07-23 13:53 UTC (permalink / raw)
To: hemant.agrawal, dev; +Cc: Sachin Saxena
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [RFC 04/11] bus/fslmc: fix memory leaks in scan
2026-07-23 13:53 [RFC 00/11] Device unplug and bus cleanup refactoring for NXP David Marchand
` (2 preceding siblings ...)
2026-07-23 13:53 ` [RFC 03/11] bus/dpaa: support unplug and use generic cleanup David Marchand
@ 2026-07-23 13:53 ` David Marchand
2026-07-23 13:53 ` [RFC 05/11] bus/fslmc: fix per type device count David Marchand
` (8 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: David Marchand @ 2026-07-23 13:53 UTC (permalink / raw)
To: hemant.agrawal, dev; +Cc: Sachin Saxena
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [RFC 05/11] bus/fslmc: fix per type device count
2026-07-23 13:53 [RFC 00/11] Device unplug and bus cleanup refactoring for NXP David Marchand
` (3 preceding siblings ...)
2026-07-23 13:53 ` [RFC 04/11] bus/fslmc: fix memory leaks in scan David Marchand
@ 2026-07-23 13:53 ` David Marchand
2026-07-23 13:53 ` [RFC 06/11] bus/fslmc: simplify device parsing in scan David Marchand
` (7 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: David Marchand @ 2026-07-23 13:53 UTC (permalink / raw)
To: hemant.agrawal, dev; +Cc: Sachin Saxena
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [RFC 06/11] bus/fslmc: simplify device parsing in scan
2026-07-23 13:53 [RFC 00/11] Device unplug and bus cleanup refactoring for NXP David Marchand
` (4 preceding siblings ...)
2026-07-23 13:53 ` [RFC 05/11] bus/fslmc: fix per type device count David Marchand
@ 2026-07-23 13:53 ` David Marchand
2026-07-23 13:53 ` [RFC 07/11] bus/fslmc: refactor device filtering for multiprocess David Marchand
` (6 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: David Marchand @ 2026-07-23 13:53 UTC (permalink / raw)
To: hemant.agrawal, dev; +Cc: Sachin Saxena
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [RFC 07/11] bus/fslmc: refactor device filtering for multiprocess
2026-07-23 13:53 [RFC 00/11] Device unplug and bus cleanup refactoring for NXP David Marchand
` (5 preceding siblings ...)
2026-07-23 13:53 ` [RFC 06/11] bus/fslmc: simplify device parsing in scan David Marchand
@ 2026-07-23 13:53 ` David Marchand
2026-07-23 13:53 ` [RFC 08/11] bus/fslmc: move unplug for some device out of VFIO David Marchand
` (5 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: David Marchand @ 2026-07-23 13:53 UTC (permalink / raw)
To: hemant.agrawal, dev; +Cc: Sachin Saxena, Anatoly Burakov
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [RFC 08/11] bus/fslmc: move unplug for some device out of VFIO
2026-07-23 13:53 [RFC 00/11] Device unplug and bus cleanup refactoring for NXP David Marchand
` (6 preceding siblings ...)
2026-07-23 13:53 ` [RFC 07/11] bus/fslmc: refactor device filtering for multiprocess David Marchand
@ 2026-07-23 13:53 ` David Marchand
2026-07-23 13:53 ` [RFC 09/11] bus/fslmc: call VFIO setup for some device from bus layer David Marchand
` (4 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: David Marchand @ 2026-07-23 13:53 UTC (permalink / raw)
To: hemant.agrawal, dev; +Cc: Sachin Saxena
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [RFC 09/11] bus/fslmc: call VFIO setup for some device from bus layer
2026-07-23 13:53 [RFC 00/11] Device unplug and bus cleanup refactoring for NXP David Marchand
` (7 preceding siblings ...)
2026-07-23 13:53 ` [RFC 08/11] bus/fslmc: move unplug for some device out of VFIO David Marchand
@ 2026-07-23 13:53 ` David Marchand
2026-07-23 13:53 ` [RFC 10/11] bus/fslmc: allocate interrupt during probing David Marchand
` (3 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: David Marchand @ 2026-07-23 13:53 UTC (permalink / raw)
To: hemant.agrawal, dev; +Cc: Sachin Saxena
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [RFC 10/11] bus/fslmc: allocate interrupt during probing
2026-07-23 13:53 [RFC 00/11] Device unplug and bus cleanup refactoring for NXP David Marchand
` (8 preceding siblings ...)
2026-07-23 13:53 ` [RFC 09/11] bus/fslmc: call VFIO setup for some device from bus layer David Marchand
@ 2026-07-23 13:53 ` David Marchand
2026-07-23 13:53 ` [RFC 11/11] bus/fslmc: use generic cleanup David Marchand
` (2 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: David Marchand @ 2026-07-23 13:53 UTC (permalink / raw)
To: hemant.agrawal, dev; +Cc: Sachin Saxena
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [RFC 11/11] bus/fslmc: use generic cleanup
2026-07-23 13:53 [RFC 00/11] Device unplug and bus cleanup refactoring for NXP David Marchand
` (9 preceding siblings ...)
2026-07-23 13:53 ` [RFC 10/11] bus/fslmc: allocate interrupt during probing David Marchand
@ 2026-07-23 13:53 ` David Marchand
2026-07-23 14:11 ` [RFC 00/11] Device unplug and bus cleanup refactoring for NXP Hemant Agrawal
2026-08-11 20:09 ` Stephen Hemminger
12 siblings, 0 replies; 14+ messages in thread
From: David Marchand @ 2026-07-23 13:53 UTC (permalink / raw)
To: hemant.agrawal, dev; +Cc: Sachin Saxena
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* RE: [RFC 00/11] Device unplug and bus cleanup refactoring for NXP
2026-07-23 13:53 [RFC 00/11] Device unplug and bus cleanup refactoring for NXP David Marchand
` (10 preceding siblings ...)
2026-07-23 13:53 ` [RFC 11/11] bus/fslmc: use generic cleanup David Marchand
@ 2026-07-23 14:11 ` Hemant Agrawal
2026-08-11 20:09 ` Stephen Hemminger
12 siblings, 0 replies; 14+ messages in thread
From: Hemant Agrawal @ 2026-07-23 14:11 UTC (permalink / raw)
To: David Marchand, dev@dpdk.org
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
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [RFC 00/11] Device unplug and bus cleanup refactoring for NXP
2026-07-23 13:53 [RFC 00/11] Device unplug and bus cleanup refactoring for NXP David Marchand
` (11 preceding siblings ...)
2026-07-23 14:11 ` [RFC 00/11] Device unplug and bus cleanup refactoring for NXP Hemant Agrawal
@ 2026-08-11 20:09 ` Stephen Hemminger
12 siblings, 0 replies; 14+ messages in thread
From: Stephen Hemminger @ 2026-08-11 20:09 UTC (permalink / raw)
To: David Marchand; +Cc: hemant.agrawal, dev
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.
^ permalink raw reply [flat|nested] 14+ messages in thread