[PATCH 04/23] drivers/bus: remove device and driver checks in unplug

[David Marchand <david.marchand@redhat.com>]

rte_dev_remove() checks if a device is probed before calling the bus
unplug operation. Individual bus detach/remove functions checking that
dev->driver is non-NULL are therefore redundant.

However, when the unplug operation is called at bus cleanup, care must
be taken that devices are in probed state, so some check on
rte_dev_is_probed() must be added.

The device parameter passed to bus unplug operations cannot be NULL as
the caller already dereferenced the bus structure to invoke these
operations.
The driver reference in the bus-specific device cannot be NULL since
calling the .unplug is done after dereferencing this pointer.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/bus/auxiliary/auxiliary_common.c | 14 ++++--------
 drivers/bus/cdx/cdx.c                    | 12 +++-------
 drivers/bus/dpaa/dpaa_bus.c              |  2 +-
 drivers/bus/fslmc/fslmc_bus.c            |  2 +-
 drivers/bus/ifpga/ifpga_bus.c            | 29 ++++--------------------
 drivers/bus/pci/pci_common.c             | 19 +++++-----------
 drivers/bus/platform/platform.c          | 13 ++++-------
 drivers/bus/uacce/uacce.c                | 13 +++++------
 drivers/bus/vdev/vdev.c                  |  4 ++--
 drivers/bus/vmbus/vmbus_common.c         |  4 +++-
 10 files changed, 36 insertions(+), 76 deletions(-)

diff --git a/drivers/bus/auxiliary/auxiliary_common.c b/drivers/bus/auxiliary/auxiliary_common.c
index 9690687600..1fe0cb4d78 100644
--- a/drivers/bus/auxiliary/auxiliary_common.c
+++ b/drivers/bus/auxiliary/auxiliary_common.c
@@ -144,16 +144,9 @@ rte_auxiliary_probe_one_driver(struct rte_auxiliary_driver *drv,
 static int
 rte_auxiliary_driver_remove_dev(struct rte_auxiliary_device *dev)
 {
-	struct rte_auxiliary_driver *drv;
+	struct rte_auxiliary_driver *drv = dev->driver;
 	int ret = 0;
 
-	if (dev == NULL)
-		return -EINVAL;
-
-	drv = dev->driver;
-	if (drv == NULL)
-		return 0;
-
 	AUXILIARY_LOG(DEBUG, "Driver %s remove auxiliary device %s on NUMA node %i",
 		      drv->driver.name, dev->name, dev->device.numa_node);
 
@@ -318,10 +311,9 @@ auxiliary_plug(struct rte_device *dev)
 static int
 auxiliary_unplug(struct rte_device *dev)
 {
-	struct rte_auxiliary_device *adev;
+	struct rte_auxiliary_device *adev = RTE_DEV_TO_AUXILIARY(dev);
 	int ret;
 
-	adev = RTE_DEV_TO_AUXILIARY(dev);
 	ret = rte_auxiliary_driver_remove_dev(adev);
 	if (ret == 0) {
 		rte_auxiliary_remove_device(adev);
@@ -341,6 +333,8 @@ auxiliary_cleanup(void)
 	RTE_TAILQ_FOREACH_SAFE(dev, &auxiliary_bus.device_list, next, tmp_dev) {
 		int ret;
 
+		if (!rte_dev_is_probed(&dev->device))
+			continue;
 		ret = auxiliary_unplug(&dev->device);
 		if (ret < 0) {
 			rte_errno = errno;
diff --git a/drivers/bus/cdx/cdx.c b/drivers/bus/cdx/cdx.c
index 9bc41d9980..f498b747e2 100644
--- a/drivers/bus/cdx/cdx.c
+++ b/drivers/bus/cdx/cdx.c
@@ -501,18 +501,13 @@ cdx_remove_device(struct rte_cdx_device *cdx_dev)
 static int
 cdx_detach_dev(struct rte_cdx_device *dev)
 {
-	struct rte_cdx_driver *dr;
+	struct rte_cdx_driver *dr = dev->driver;
 	int ret = 0;
 
-	if (dev == NULL)
-		return -EINVAL;
-
-	dr = dev->driver;
-
 	CDX_BUS_DEBUG("detach device %s using driver: %s",
 		dev->device.name, dr->driver.name);
 
-	if (dr->remove) {
+	if (dr->remove != NULL) {
 		ret = dr->remove(dev);
 		if (ret < 0)
 			return ret;
@@ -539,10 +534,9 @@ cdx_plug(struct rte_device *dev)
 static int
 cdx_unplug(struct rte_device *dev)
 {
-	struct rte_cdx_device *cdx_dev;
+	struct rte_cdx_device *cdx_dev = RTE_DEV_TO_CDX_DEV(dev);
 	int ret;
 
-	cdx_dev = RTE_DEV_TO_CDX_DEV(dev);
 	ret = cdx_detach_dev(cdx_dev);
 	if (ret == 0) {
 		cdx_remove_device(cdx_dev);
diff --git a/drivers/bus/dpaa/dpaa_bus.c b/drivers/bus/dpaa/dpaa_bus.c
index 1d12f2dceb..1bfc44155d 100644
--- a/drivers/bus/dpaa/dpaa_bus.c
+++ b/drivers/bus/dpaa/dpaa_bus.c
@@ -938,7 +938,7 @@ dpaa_bus_cleanup(void)
 
 		if (!rte_dev_is_probed(&dev->device))
 			continue;
-		if (!drv || !drv->remove)
+		if (drv->remove == NULL)
 			continue;
 		ret = drv->remove(dev);
 		if (ret < 0) {
diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
index cf881b3eec..7e5a3e947e 100644
--- a/drivers/bus/fslmc/fslmc_bus.c
+++ b/drivers/bus/fslmc/fslmc_bus.c
@@ -619,7 +619,7 @@ fslmc_bus_unplug(struct rte_device *rte_dev)
 			struct rte_dpaa2_device, device);
 	struct rte_dpaa2_driver *drv = dev->driver;
 
-	if (drv && drv->remove) {
+	if (drv->remove != NULL) {
 		drv->remove(dev);
 		dev->driver = NULL;
 		dev->device.driver = NULL;
diff --git a/drivers/bus/ifpga/ifpga_bus.c b/drivers/bus/ifpga/ifpga_bus.c
index 5cc1207c46..fc5308b6f4 100644
--- a/drivers/bus/ifpga/ifpga_bus.c
+++ b/drivers/bus/ifpga/ifpga_bus.c
@@ -365,7 +365,9 @@ ifpga_cleanup(void)
 		struct rte_afu_driver *drv = afu_dev->driver;
 		int ret = 0;
 
-		if (drv == NULL || drv->remove == NULL)
+		if (!rte_dev_is_probed(&afu_dev->device))
+			goto free;
+		if (drv->remove == NULL)
 			goto free;
 
 		ret = drv->remove(afu_dev);
@@ -392,34 +394,13 @@ ifpga_plug(struct rte_device *dev)
 	return ifpga_probe_all_drivers(RTE_DEV_TO_AFU(dev));
 }
 
-static int
-ifpga_remove_driver(struct rte_afu_device *afu_dev)
-{
-	const char *name;
-
-	name = rte_ifpga_device_name(afu_dev);
-	if (afu_dev->driver == NULL) {
-		IFPGA_BUS_DEBUG("no driver attach to device %s", name);
-		return 1;
-	}
-
-	return afu_dev->driver->remove(afu_dev);
-}
-
 static int
 ifpga_unplug(struct rte_device *dev)
 {
-	struct rte_afu_device *afu_dev = NULL;
+	struct rte_afu_device *afu_dev = RTE_DEV_TO_AFU(dev);
 	int ret;
 
-	if (dev == NULL)
-		return -EINVAL;
-
-	afu_dev = RTE_DEV_TO_AFU(dev);
-	if (!afu_dev)
-		return -ENOENT;
-
-	ret = ifpga_remove_driver(afu_dev);
+	ret = afu_dev->driver->remove(afu_dev);
 	if (ret)
 		return ret;
 
diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
index d7f028e365..1385b0c959 100644
--- a/drivers/bus/pci/pci_common.c
+++ b/drivers/bus/pci/pci_common.c
@@ -310,13 +310,9 @@ static int
 rte_pci_detach_dev(struct rte_pci_device *dev)
 {
 	struct rte_pci_addr *loc;
-	struct rte_pci_driver *dr;
+	struct rte_pci_driver *dr = dev->driver;
 	int ret = 0;
 
-	if (dev == NULL)
-		return -EINVAL;
-
-	dr = dev->driver;
 	loc = &dev->addr;
 
 	PCI_LOG(DEBUG, "PCI device "PCI_PRI_FMT" on NUMA socket %i",
@@ -416,7 +412,9 @@ pci_cleanup(void)
 		struct rte_pci_driver *drv = dev->driver;
 		int ret = 0;
 
-		if (drv == NULL || drv->remove == NULL)
+		if (!rte_dev_is_probed(&dev->device))
+			goto free;
+		if (drv->remove == NULL)
 			goto free;
 
 		ret = drv->remove(dev);
@@ -590,13 +588,9 @@ pci_find_device_by_addr(const void *failure_addr)
 static int
 pci_hot_unplug_handler(struct rte_device *dev)
 {
-	struct rte_pci_device *pdev = NULL;
+	struct rte_pci_device *pdev = RTE_DEV_TO_PCI(dev);
 	int ret = 0;
 
-	pdev = RTE_DEV_TO_PCI(dev);
-	if (!pdev)
-		return -1;
-
 	switch (pdev->kdrv) {
 	case RTE_PCI_KDRV_VFIO:
 		/*
@@ -654,10 +648,9 @@ pci_plug(struct rte_device *dev)
 static int
 pci_unplug(struct rte_device *dev)
 {
-	struct rte_pci_device *pdev;
+	struct rte_pci_device *pdev = RTE_DEV_TO_PCI(dev);
 	int ret;
 
-	pdev = RTE_DEV_TO_PCI(dev);
 	ret = rte_pci_detach_dev(pdev);
 	if (ret == 0) {
 		rte_pci_remove_device(pdev);
diff --git a/drivers/bus/platform/platform.c b/drivers/bus/platform/platform.c
index 8a89a3cad8..0345f1daf7 100644
--- a/drivers/bus/platform/platform.c
+++ b/drivers/bus/platform/platform.c
@@ -476,11 +476,10 @@ platform_bus_plug(struct rte_device *dev)
 static void
 device_release_driver(struct rte_platform_device *pdev)
 {
-	struct rte_platform_driver *pdrv;
+	struct rte_platform_driver *pdrv = pdev->driver;
 	int ret;
 
-	pdrv = pdev->driver;
-	if (pdrv != NULL && pdrv->remove != NULL) {
+	if (pdrv->remove != NULL) {
 		ret = pdrv->remove(pdev);
 		if (ret)
 			PLATFORM_LOG_LINE(WARNING, "failed to remove %s", pdev->name);
@@ -493,11 +492,7 @@ device_release_driver(struct rte_platform_device *pdev)
 static int
 platform_bus_unplug(struct rte_device *dev)
 {
-	struct rte_platform_device *pdev;
-
-	pdev = RTE_DEV_TO_PLATFORM_DEV(dev);
-	if (pdev == NULL)
-		return -EINVAL;
+	struct rte_platform_device *pdev = RTE_DEV_TO_PLATFORM_DEV(dev);
 
 	device_release_driver(pdev);
 	device_cleanup(pdev);
@@ -572,6 +567,8 @@ platform_bus_cleanup(void)
 
 	RTE_TAILQ_FOREACH_SAFE(pdev, &platform_bus.device_list, next, tmp) {
 		TAILQ_REMOVE(&platform_bus.device_list, pdev, next);
+		if (!rte_dev_is_probed(&pdev->device))
+			continue;
 		platform_bus_unplug(&pdev->device);
 	}
 
diff --git a/drivers/bus/uacce/uacce.c b/drivers/bus/uacce/uacce.c
index ade2452ad5..d0ea454911 100644
--- a/drivers/bus/uacce/uacce.c
+++ b/drivers/bus/uacce/uacce.c
@@ -453,7 +453,9 @@ uacce_cleanup(void)
 		struct rte_uacce_driver *dr = dev->driver;
 		int ret = 0;
 
-		if (dr == NULL || dr->remove == NULL)
+		if (!rte_dev_is_probed(&dev->device))
+			goto free;
+		if (dr->remove == NULL)
 			goto free;
 
 		ret = dr->remove(dev);
@@ -481,14 +483,12 @@ uacce_plug(struct rte_device *dev)
 static int
 uacce_detach_dev(struct rte_uacce_device *dev)
 {
-	struct rte_uacce_driver *dr;
+	struct rte_uacce_driver *dr = dev->driver;
 	int ret = 0;
 
-	dr = dev->driver;
-
 	UACCE_BUS_DEBUG("detach device %s using driver: %s", dev->device.name, dr->driver.name);
 
-	if (dr->remove) {
+	if (dr->remove != NULL) {
 		ret = dr->remove(dev);
 		if (ret < 0)
 			return ret;
@@ -503,10 +503,9 @@ uacce_detach_dev(struct rte_uacce_device *dev)
 static int
 uacce_unplug(struct rte_device *dev)
 {
-	struct rte_uacce_device *uacce_dev;
+	struct rte_uacce_device *uacce_dev = RTE_DEV_TO_UACCE_DEV(dev);
 	int ret;
 
-	uacce_dev = RTE_DEV_TO_UACCE_DEV(dev);
 	ret = uacce_detach_dev(uacce_dev);
 	if (ret == 0) {
 		TAILQ_REMOVE(&uacce_bus.device_list, uacce_dev, next);
diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index a200a67847..906e9dbe08 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -567,9 +567,9 @@ vdev_cleanup(void)
 
 	RTE_TAILQ_FOREACH_SAFE(dev, &vdev_device_list, next, tmp_dev) {
 		const struct rte_vdev_driver *drv;
-		int ret = 0;
+		int ret;
 
-		if (dev->device.driver == NULL)
+		if (!rte_dev_is_probed(&dev->device))
 			goto free;
 
 		drv = container_of(dev->device.driver, const struct rte_vdev_driver, driver);
diff --git a/drivers/bus/vmbus/vmbus_common.c b/drivers/bus/vmbus/vmbus_common.c
index bdc0fbb62d..d38c75d597 100644
--- a/drivers/bus/vmbus/vmbus_common.c
+++ b/drivers/bus/vmbus/vmbus_common.c
@@ -210,7 +210,9 @@ rte_vmbus_cleanup(void)
 		const struct rte_vmbus_driver *drv = dev->driver;
 		int ret;
 
-		if (drv == NULL || drv->remove == NULL)
+		if (!rte_dev_is_probed(&dev->device))
+			continue;
+		if (drv->remove == NULL)
 			continue;
 
 		ret = drv->remove(dev);
-- 
2.53.0