[dpdk-dev,v8,10/14] eal/pci: Cleanup pci driver initialization code

Message ID 1424060073-23484-11-git-send-email-mukawa@igel.co.jp (mailing list archive)
State Superseded, archived
Headers

Commit Message

Tetsuya Mukawa Feb. 16, 2015, 4:14 a.m. UTC
- Add rte_eal_pci_close_one_dirver()
  The function is used for closing the specified driver and device.
- Add pci_invoke_all_drivers()
  The function is based on pci_probe_all_drivers. But it can not only
  probe but also close drivers.
- Add pci_close_all_drivers()
  The function tries to find a driver for the specified device, and
  then close the driver.
- Add rte_eal_pci_probe_one() and rte_eal_pci_close_one()
  The functions are used for probe and close a device.
  First the function tries to find a device that has the specified
  PCI address. Then, probe or close the device.

v5:
- Remove RTE_EAL_INVOKE_TYPE_UNKNOWN, because it's unused.
v4:
- Fix parameter checking.
- Fix indent of 'if' statement.

Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
---
 lib/librte_eal/common/eal_common_pci.c  | 90 +++++++++++++++++++++++++++++----
 lib/librte_eal/common/eal_private.h     | 24 +++++++++
 lib/librte_eal/common/include/rte_pci.h | 33 ++++++++++++
 lib/librte_eal/linuxapp/eal/eal_pci.c   | 69 +++++++++++++++++++++++++
 4 files changed, 206 insertions(+), 10 deletions(-)
  

Comments

Iremonger, Bernard Feb. 16, 2015, 4:27 p.m. UTC | #1
> -----Original Message-----
> From: Tetsuya Mukawa [mailto:mukawa@igel.co.jp]
> Sent: Monday, February 16, 2015 4:14 AM
> To: dev@dpdk.org
> Cc: Qiu, Michael; Iremonger, Bernard; Tetsuya Mukawa
> Subject: [PATCH v8 10/14] eal/pci: Cleanup pci driver initialization code
> 
> - Add rte_eal_pci_close_one_dirver()
>   The function is used for closing the specified driver and device.
> - Add pci_invoke_all_drivers()
>   The function is based on pci_probe_all_drivers. But it can not only
>   probe but also close drivers.
> - Add pci_close_all_drivers()
>   The function tries to find a driver for the specified device, and
>   then close the driver.
> - Add rte_eal_pci_probe_one() and rte_eal_pci_close_one()
>   The functions are used for probe and close a device.
>   First the function tries to find a device that has the specified
>   PCI address. Then, probe or close the device.
> 
> v5:
> - Remove RTE_EAL_INVOKE_TYPE_UNKNOWN, because it's unused.
> v4:
> - Fix parameter checking.
> - Fix indent of 'if' statement.
> 
> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>

Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>
  
Thomas Monjalon Feb. 17, 2015, 1:18 a.m. UTC | #2
2015-02-16 13:14, Tetsuya Mukawa:
> - Add rte_eal_pci_close_one_dirver()
>   The function is used for closing the specified driver and device.
> - Add pci_invoke_all_drivers()
>   The function is based on pci_probe_all_drivers. But it can not only
>   probe but also close drivers.
> - Add pci_close_all_drivers()
>   The function tries to find a driver for the specified device, and
>   then close the driver.
> - Add rte_eal_pci_probe_one() and rte_eal_pci_close_one()
>   The functions are used for probe and close a device.
>   First the function tries to find a device that has the specified
>   PCI address. Then, probe or close the device.
> 
> v5:
> - Remove RTE_EAL_INVOKE_TYPE_UNKNOWN, because it's unused.
> v4:
> - Fix parameter checking.
> - Fix indent of 'if' statement.
> 
> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
> ---
>  lib/librte_eal/common/eal_common_pci.c  | 90 +++++++++++++++++++++++++++++----
>  lib/librte_eal/common/eal_private.h     | 24 +++++++++
>  lib/librte_eal/common/include/rte_pci.h | 33 ++++++++++++
>  lib/librte_eal/linuxapp/eal/eal_pci.c   | 69 +++++++++++++++++++++++++
>  4 files changed, 206 insertions(+), 10 deletions(-)

206 insertions and 10 deletions: it cannot really be a cleanup ;)
Maybe the title should be reworded.

[...]
> -		rc = rte_eal_pci_probe_one_driver(dr, dev);
> +		switch (type) {
> +		case RTE_EAL_INVOKE_TYPE_PROBE:
> +			rc = rte_eal_pci_probe_one_driver(dr, dev);
> +			break;
> +		case RTE_EAL_INVOKE_TYPE_CLOSE:
> +			rc = rte_eal_pci_close_one_driver(dr, dev);
> +			break;

Honestly, I don't like this kind of functions with a switch to toggle
different actions. It makes code unclear.
  
Tetsuya Mukawa Feb. 17, 2015, 6:15 a.m. UTC | #3
On 2015/02/17 10:18, Thomas Monjalon wrote:
> 2015-02-16 13:14, Tetsuya Mukawa:
>> - Add rte_eal_pci_close_one_dirver()
>>   The function is used for closing the specified driver and device.
>> - Add pci_invoke_all_drivers()
>>   The function is based on pci_probe_all_drivers. But it can not only
>>   probe but also close drivers.
>> - Add pci_close_all_drivers()
>>   The function tries to find a driver for the specified device, and
>>   then close the driver.
>> - Add rte_eal_pci_probe_one() and rte_eal_pci_close_one()
>>   The functions are used for probe and close a device.
>>   First the function tries to find a device that has the specified
>>   PCI address. Then, probe or close the device.
>>
>> v5:
>> - Remove RTE_EAL_INVOKE_TYPE_UNKNOWN, because it's unused.
>> v4:
>> - Fix parameter checking.
>> - Fix indent of 'if' statement.
>>
>> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
>> ---
>>  lib/librte_eal/common/eal_common_pci.c  | 90 +++++++++++++++++++++++++++++----
>>  lib/librte_eal/common/eal_private.h     | 24 +++++++++
>>  lib/librte_eal/common/include/rte_pci.h | 33 ++++++++++++
>>  lib/librte_eal/linuxapp/eal/eal_pci.c   | 69 +++++++++++++++++++++++++
>>  4 files changed, 206 insertions(+), 10 deletions(-)
> 206 insertions and 10 deletions: it cannot really be a cleanup ;)
> Maybe the title should be reworded.

I will reword the tile.

>
> [...]
>> -		rc = rte_eal_pci_probe_one_driver(dr, dev);
>> +		switch (type) {
>> +		case RTE_EAL_INVOKE_TYPE_PROBE:
>> +			rc = rte_eal_pci_probe_one_driver(dr, dev);
>> +			break;
>> +		case RTE_EAL_INVOKE_TYPE_CLOSE:
>> +			rc = rte_eal_pci_close_one_driver(dr, dev);
>> +			break;
> Honestly, I don't like this kind of functions with a switch to toggle
> different actions. It makes code unclear.
>

Sure, I will remove such a toggle. Probably I will separate the function.
  

Patch

diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index a89f5c3..7c9b8c5 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -99,19 +99,27 @@  static struct rte_devargs *pci_devargs_lookup(struct rte_pci_device *dev)
 	return NULL;
 }
 
-/*
- * If vendor/device ID match, call the devinit() function of all
- * registered driver for the given device. Return -1 if initialization
- * failed, return 1 if no driver is found for this device.
- */
 static int
-pci_probe_all_drivers(struct rte_pci_device *dev)
+pci_invoke_all_drivers(struct rte_pci_device *dev,
+		enum rte_eal_invoke_type type)
 {
 	struct rte_pci_driver *dr = NULL;
-	int rc;
+	int rc = 0;
+
+	if ((dev == NULL) || (type >= RTE_EAL_INVOKE_TYPE_MAX))
+		return -1;
 
 	TAILQ_FOREACH(dr, &pci_driver_list, next) {
-		rc = rte_eal_pci_probe_one_driver(dr, dev);
+		switch (type) {
+		case RTE_EAL_INVOKE_TYPE_PROBE:
+			rc = rte_eal_pci_probe_one_driver(dr, dev);
+			break;
+		case RTE_EAL_INVOKE_TYPE_CLOSE:
+			rc = rte_eal_pci_close_one_driver(dr, dev);
+			break;
+		default:
+			return -1;
+		}
 		if (rc < 0)
 			/* negative value is an error */
 			return -1;
@@ -123,6 +131,66 @@  pci_probe_all_drivers(struct rte_pci_device *dev)
 	return 1;
 }
 
+#ifdef ENABLE_HOTPLUG
+static int
+rte_eal_pci_invoke_one(struct rte_pci_addr *addr,
+		enum rte_eal_invoke_type type)
+{
+	struct rte_pci_device *dev = NULL;
+	int ret = 0;
+
+	if ((addr == NULL) || (type >= RTE_EAL_INVOKE_TYPE_MAX))
+		return -1;
+
+	TAILQ_FOREACH(dev, &pci_device_list, next) {
+		if (eal_compare_pci_addr(&dev->addr, addr))
+			continue;
+
+		ret = pci_invoke_all_drivers(dev, type);
+		if (ret < 0)
+			goto invoke_err_return;
+
+		if (type == RTE_EAL_INVOKE_TYPE_CLOSE)
+			goto remove_dev;
+
+		return 0;
+	}
+
+	return -1;
+
+invoke_err_return:
+	RTE_LOG(WARNING, EAL, "Requested device " PCI_PRI_FMT
+			" cannot be used\n", dev->addr.domain, dev->addr.bus,
+			dev->addr.devid, dev->addr.function);
+	return -1;
+
+remove_dev:
+	TAILQ_REMOVE(&pci_device_list, dev, next);
+	return 0;
+}
+
+
+/*
+ * Find the pci device specified by pci address, then invoke probe function of
+ * the driver of the devive.
+ */
+int
+rte_eal_pci_probe_one(struct rte_pci_addr *addr)
+{
+	return rte_eal_pci_invoke_one(addr, RTE_EAL_INVOKE_TYPE_PROBE);
+}
+
+/*
+ * Find the pci device specified by pci address, then invoke close function of
+ * the driver of the devive.
+ */
+int
+rte_eal_pci_close_one(struct rte_pci_addr *addr)
+{
+	return rte_eal_pci_invoke_one(addr, RTE_EAL_INVOKE_TYPE_CLOSE);
+}
+#endif /* ENABLE_HOTPLUG */
+
 /*
  * Scan the content of the PCI bus, and call the devinit() function for
  * all registered drivers that have a matching entry in its id_table
@@ -148,10 +216,12 @@  rte_eal_pci_probe(void)
 
 		/* probe all or only whitelisted devices */
 		if (probe_all)
-			ret = pci_probe_all_drivers(dev);
+			ret = pci_invoke_all_drivers(dev,
+					RTE_EAL_INVOKE_TYPE_PROBE);
 		else if (devargs != NULL &&
 			devargs->type == RTE_DEVTYPE_WHITELISTED_PCI)
-			ret = pci_probe_all_drivers(dev);
+			ret = pci_invoke_all_drivers(dev,
+					RTE_EAL_INVOKE_TYPE_PROBE);
 		if (ret < 0)
 			rte_exit(EXIT_FAILURE, "Requested device " PCI_PRI_FMT
 				 " cannot be used\n", dev->addr.domain, dev->addr.bus,
diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
index 159cd66..1a362ab 100644
--- a/lib/librte_eal/common/eal_private.h
+++ b/lib/librte_eal/common/eal_private.h
@@ -154,6 +154,15 @@  struct rte_pci_driver;
 struct rte_pci_device;
 
 /**
+ * The invoke type.
+ */
+enum rte_eal_invoke_type {
+	RTE_EAL_INVOKE_TYPE_PROBE,  /**< invoke probe function */
+	RTE_EAL_INVOKE_TYPE_CLOSE,  /**< invoke close function */
+	RTE_EAL_INVOKE_TYPE_MAX     /**< max value of this enum */
+};
+
+/**
  * Mmap memory for single PCI device
  *
  * This function is private to EAL.
@@ -165,6 +174,21 @@  int rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr,
 		struct rte_pci_device *dev);
 
 /**
+ * Munmap memory for single PCI device
+ *
+ * This function is private to EAL.
+ *
+ * @param	dr
+ *  The pointer to the pci driver structure
+ * @param	dev
+ *  The pointer to the pci device structure
+ * @return
+ *   0 on success, negative on error
+ */
+int rte_eal_pci_close_one_driver(struct rte_pci_driver *dr,
+		struct rte_pci_device *dev);
+
+/**
  * Init tail queues for non-EAL library structures. This is to allow
  * the rings, mempools, etc. lists to be shared among multiple processes
  *
diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 87ca4cf..a111066 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -82,6 +82,7 @@  extern "C" {
 #include <inttypes.h>
 
 #include <rte_interrupts.h>
+#include <rte_dev_hotplug.h>
 
 TAILQ_HEAD(pci_device_list, rte_pci_device); /**< PCI devices in D-linked Q. */
 TAILQ_HEAD(pci_driver_list, rte_pci_driver); /**< PCI drivers in D-linked Q. */
@@ -323,6 +324,38 @@  eal_compare_pci_addr(struct rte_pci_addr *addr, struct rte_pci_addr *addr2)
  */
 int rte_eal_pci_probe(void);
 
+#ifdef ENABLE_HOTPLUG
+/**
+ * Probe the single PCI device.
+ *
+ * Scan the content of the PCI bus, and find the pci device specified by pci
+ * address, then call the probe() function for registered driver that has a
+ * matching entry in its id_table for discovered device.
+ *
+ * @param addr
+ *	The PCI Bus-Device-Function address to probe or close.
+ * @return
+ *   - 0 on success.
+ *   - Negative on error.
+ */
+int rte_eal_pci_probe_one(struct rte_pci_addr *addr);
+
+/**
+ * Close the single PCI device.
+ *
+ * Scan the content of the PCI bus, and find the pci device specified by pci
+ * address, then call the close() function for registered driver that has a
+ * matching entry in its id_table for discovered device.
+ *
+ * @param addr
+ *	The PCI Bus-Device-Function address to probe or close.
+ * @return
+ *   - 0 on success.
+ *   - Negative on error.
+ */
+int rte_eal_pci_close_one(struct rte_pci_addr *addr);
+#endif /* ENABLE_HOTPLUG */
+
 /**
  * Dump the content of the PCI bus.
  *
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index 72a1362..0adb217 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -694,6 +694,75 @@  rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
 	return 1;
 }
 
+#ifdef ENABLE_HOTPLUG
+/*
+ * If vendor/device ID match, call the devuninit() function of the
+ * driver.
+ */
+int
+rte_eal_pci_close_one_driver(struct rte_pci_driver *dr,
+		struct rte_pci_device *dev)
+{
+	struct rte_pci_id *id_table;
+
+	if ((dr == NULL) || (dev == NULL))
+		return -EINVAL;
+
+	for (id_table = dr->id_table ; id_table->vendor_id != 0; id_table++) {
+
+		/* check if device's identifiers match the driver's ones */
+		if (id_table->vendor_id != dev->id.vendor_id &&
+		    id_table->vendor_id != PCI_ANY_ID)
+			continue;
+		if (id_table->device_id != dev->id.device_id &&
+		    id_table->device_id != PCI_ANY_ID)
+			continue;
+		if (id_table->subsystem_vendor_id !=
+		    dev->id.subsystem_vendor_id &&
+		    id_table->subsystem_vendor_id != PCI_ANY_ID)
+			continue;
+		if (id_table->subsystem_device_id !=
+		    dev->id.subsystem_device_id &&
+		    id_table->subsystem_device_id != PCI_ANY_ID)
+			continue;
+
+		struct rte_pci_addr *loc = &dev->addr;
+
+		RTE_LOG(DEBUG, EAL,
+				"PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
+				loc->domain, loc->bus, loc->devid,
+				loc->function, dev->numa_node);
+
+		RTE_LOG(DEBUG, EAL, "  remove driver: %x:%x %s\n",
+				dev->id.vendor_id, dev->id.device_id,
+				dr->name);
+
+		/* call the driver devuninit() function */
+		if (dr->devuninit && (dr->devuninit(dr, dev) < 0))
+			return -1;	/* negative value is an error */
+
+		/* clear driver structure */
+		dev->driver = NULL;
+
+		if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
+			/* unmap resources for devices that use igb_uio */
+			pci_unmap_device(dev);
+
+		return 0;
+	}
+	/* return positive value if driver is not found */
+	return 1;
+}
+#else /* ENABLE_HOTPLUG */
+int
+rte_eal_pci_close_one_driver(struct rte_pci_driver *dr __rte_unused,
+		struct rte_pci_device *dev __rte_unused)
+{
+	RTE_LOG(ERR, EAL, "Hotplug support isn't enabled\n");
+	return -1;
+}
+#endif /* ENABLE_HOTPLUG */
+
 /* Init the PCI EAL subsystem */
 int
 rte_eal_pci_init(void)