[dpdk-dev,v8,10/14] eal/pci: Cleanup pci driver initialization code
Commit Message
- 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
> -----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>
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.
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.
@@ -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,
@@ -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
*
@@ -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.
*
@@ -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)