[dpdk-dev] [PATCH v5 7/7] eal: allow probing a device again

[Thomas Monjalon <thomas@monjalon.net>]

In the devargs syntax for device representors, it is possible to add
several devices at once: -w dbdf,representor=[0-3]
It will become a more frequent case when introducing wildcards
and ranges in the new devargs syntax.

If a devargs string is provided for probing, and updated with a bigger
range for a new probing, then we do not want it to fail because
part of this range was already probed previously.
There can be new ports to create from an existing rte_device.

That's why the check for an already probed device
is moved as bus responsibility.
In the case of vdev, a global check is kept in insert_vdev(),
assuming that a vdev will always have only one port.
In the case of ifpga and vmbus, already probed devices are checked.
In the case of NXP buses, the probing is done only once (no hotplug),
though a check is added at bus level for consistency.
In the case of PCI, a driver flag is added to allow PMD probing again.
Only the PMD knows the ports attached to one rte_device.

As another consequence of being able to probe in several steps,
the field rte_device.devargs must not be considered as a full
representation of the rte_device, but only the latest probing args.
Anyway, the field rte_device.devargs is used only for probing.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
Tested-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 drivers/bus/dpaa/dpaa_bus.c             |  3 +++
 drivers/bus/fslmc/fslmc_bus.c           |  3 +++
 drivers/bus/ifpga/ifpga_bus.c           | 14 ++++++-----
 drivers/bus/pci/pci_common.c            | 33 ++++++++++++++++---------
 drivers/bus/pci/rte_bus_pci.h           |  4 ++-
 drivers/bus/vdev/vdev.c                 |  5 ++++
 lib/librte_eal/common/eal_common_dev.c  |  7 ++----
 lib/librte_eal/common/include/rte_dev.h |  2 +-
 8 files changed, 47 insertions(+), 24 deletions(-)

diff --git a/drivers/bus/dpaa/dpaa_bus.c b/drivers/bus/dpaa/dpaa_bus.c
index 138e0f98d..89d1e415d 100644
--- a/drivers/bus/dpaa/dpaa_bus.c
+++ b/drivers/bus/dpaa/dpaa_bus.c
@@ -555,6 +555,9 @@ rte_dpaa_bus_probe(void)
 			if (ret)
 				continue;
 
+			if (rte_dev_is_probed(&dev->device))
+				continue;
+
 			if (!drv->probe ||
 			    (dev->device.devargs &&
 			    dev->device.devargs->policy == RTE_DEV_BLACKLISTED))
diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
index 960f55071..7ebd980aa 100644
--- a/drivers/bus/fslmc/fslmc_bus.c
+++ b/drivers/bus/fslmc/fslmc_bus.c
@@ -386,6 +386,9 @@ rte_fslmc_probe(void)
 			if (!drv->probe)
 				continue;
 
+			if (rte_dev_is_probed(&dev->device))
+				continue;
+
 			if (dev->device.devargs &&
 			  dev->device.devargs->policy == RTE_DEV_BLACKLISTED) {
 				DPAA2_BUS_LOG(DEBUG, "%s Blacklisted, skipping",
diff --git a/drivers/bus/ifpga/ifpga_bus.c b/drivers/bus/ifpga/ifpga_bus.c
index 2ca1efa72..5f23ed8b4 100644
--- a/drivers/bus/ifpga/ifpga_bus.c
+++ b/drivers/bus/ifpga/ifpga_bus.c
@@ -301,8 +301,11 @@ ifpga_probe_all_drivers(struct rte_afu_device *afu_dev)
 		return -1;
 
 	/* Check if a driver is already loaded */
-	if (rte_dev_is_probed(&afu_dev->device))
-		return 0;
+	if (rte_dev_is_probed(&afu_dev->device)) {
+		IFPGA_BUS_DEBUG("Device %s is already probed\n",
+				rte_ifpga_device_name(afu_dev));
+		return -EEXIST;
+	}
 
 	TAILQ_FOREACH(drv, &ifpga_afu_drv_list, next) {
 		if (ifpga_probe_one_driver(drv, afu_dev)) {
@@ -325,14 +328,13 @@ ifpga_probe(void)
 	int ret = 0;
 
 	TAILQ_FOREACH(afu_dev, &ifpga_afu_dev_list, next) {
-		if (rte_dev_is_probed(&afu_dev->device))
-			continue;
-
 		ret = ifpga_probe_all_drivers(afu_dev);
+		if (ret == -EEXIST)
+			continue;
 		if (ret < 0)
 			IFPGA_BUS_ERR("failed to initialize %s device\n",
 				rte_ifpga_device_name(afu_dev));
-		}
+	}
 
 	return ret;
 }
diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
index 62b17fba9..11c5da587 100644
--- a/drivers/bus/pci/pci_common.c
+++ b/drivers/bus/pci/pci_common.c
@@ -6,6 +6,7 @@
 #include <string.h>
 #include <inttypes.h>
 #include <stdint.h>
+#include <stdbool.h>
 #include <stdlib.h>
 #include <stdio.h>
 #include <sys/queue.h>
@@ -121,6 +122,7 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr,
 			 struct rte_pci_device *dev)
 {
 	int ret;
+	bool already_probed;
 	struct rte_pci_addr *loc;
 
 	if ((dr == NULL) || (dev == NULL))
@@ -151,6 +153,13 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr,
 		dev->device.numa_node = 0;
 	}
 
+	already_probed = rte_dev_is_probed(&dev->device);
+	if (already_probed && !(dr->drv_flags & RTE_PCI_DRV_PROBE_AGAIN)) {
+		RTE_LOG(DEBUG, EAL, "Device %s is already probed\n",
+				dev->device.name);
+		return -EEXIST;
+	}
+
 	RTE_LOG(INFO, EAL, "  probe driver: %x:%x %s\n", dev->id.vendor_id,
 		dev->id.device_id, dr->driver.name);
 
@@ -159,9 +168,10 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr,
 	 * This needs to be before rte_pci_map_device(), as it enables to use
 	 * driver flags for adjusting configuration.
 	 */
-	dev->driver = dr;
+	if (!already_probed)
+		dev->driver = dr;
 
-	if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
+	if (!already_probed && (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)) {
 		/* map resources for devices that use igb_uio */
 		ret = rte_pci_map_device(dev);
 		if (ret != 0) {
@@ -172,6 +182,8 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr,
 
 	/* call the driver probe() function */
 	ret = dr->probe(dr, dev);
+	if (already_probed)
+		return ret; /* no rollback if already succeeded earlier */
 	if (ret) {
 		dev->driver = NULL;
 		if ((dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) &&
@@ -242,10 +254,6 @@ pci_probe_all_drivers(struct rte_pci_device *dev)
 	if (dev == NULL)
 		return -1;
 
-	/* Check if a driver is already loaded */
-	if (rte_dev_is_probed(&dev->device))
-		return 0;
-
 	FOREACH_DRIVER_ON_PCIBUS(dr) {
 		rc = rte_pci_probe_one_driver(dr, dev);
 		if (rc < 0)
@@ -287,11 +295,14 @@ rte_pci_probe(void)
 			devargs->policy == RTE_DEV_WHITELISTED)
 			ret = pci_probe_all_drivers(dev);
 		if (ret < 0) {
-			RTE_LOG(ERR, EAL, "Requested device " PCI_PRI_FMT
-				 " cannot be used\n", dev->addr.domain, dev->addr.bus,
-				 dev->addr.devid, dev->addr.function);
-			rte_errno = errno;
-			failed++;
+			if (ret != -EEXIST) {
+				RTE_LOG(ERR, EAL, "Requested device "
+					PCI_PRI_FMT " cannot be used\n",
+					dev->addr.domain, dev->addr.bus,
+					dev->addr.devid, dev->addr.function);
+				rte_errno = errno;
+				failed++;
+			}
 			ret = 0;
 		}
 	}
diff --git a/drivers/bus/pci/rte_bus_pci.h b/drivers/bus/pci/rte_bus_pci.h
index 984df2b37..a3baa2895 100644
--- a/drivers/bus/pci/rte_bus_pci.h
+++ b/drivers/bus/pci/rte_bus_pci.h
@@ -121,7 +121,7 @@ struct rte_pci_driver {
 	pci_probe_t *probe;                /**< Device Probe function. */
 	pci_remove_t *remove;              /**< Device Remove function. */
 	const struct rte_pci_id *id_table; /**< ID table, NULL terminated. */
-	uint32_t drv_flags;                /**< Flags contolling handling of device. */
+	uint32_t drv_flags;                /**< Flags RTE_PCI_DRV_*. */
 };
 
 /**
@@ -137,6 +137,8 @@ struct rte_pci_bus {
 #define RTE_PCI_DRV_NEED_MAPPING 0x0001
 /** Device needs PCI BAR mapping with enabled write combining (wc) */
 #define RTE_PCI_DRV_WC_ACTIVATE 0x0002
+/** Device already probed can be probed again to check for new ports. */
+#define RTE_PCI_DRV_PROBE_AGAIN 0x0004
 /** Device driver supports link state interrupt */
 #define RTE_PCI_DRV_INTR_LSC	0x0008
 /** Device driver supports device removal interrupt */
diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index f666099e6..06f314843 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -226,6 +226,11 @@ insert_vdev(const char *name, const char *args, struct rte_vdev_device **p_dev)
 	dev->device.name = devargs->name;
 
 	if (find_vdev(name)) {
+		/*
+		 * A vdev is expected to have only one port.
+		 * So there is no reason to try probing again,
+		 * even with new arguments.
+		 */
 		ret = -EEXIST;
 		goto fail;
 	}
diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index 7e8a9b260..e733eb779 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -196,13 +196,10 @@ rte_dev_probe(const char *devargs)
 		goto err_devarg;
 	}
 
-	if (rte_dev_is_probed(dev)) {
-		RTE_LOG(ERR, EAL, "Device is already plugged\n");
-		return -EEXIST;
-	}
-
 	ret = dev->bus->plug(dev);
 	if (ret) {
+		if (rte_dev_is_probed(dev)) /* if already succeeded earlier */
+			return ret; /* no rollback */
 		RTE_LOG(ERR, EAL, "Driver cannot attach the device (%s)\n",
 			dev->name);
 		goto err_devarg;
diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index 9f169e3b3..a7ec8ec25 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -159,7 +159,7 @@ struct rte_device {
 	const struct rte_driver *driver; /**< Driver assigned after probing */
 	const struct rte_bus *bus;    /**< Bus handle assigned on scan */
 	int numa_node;                /**< NUMA node connection */
-	struct rte_devargs *devargs;  /**< Device user arguments */
+	struct rte_devargs *devargs;  /**< Arguments for latest probing */
 };
 
 /**
-- 
2.19.0