[dpdk-dev,v11,07/13] pci: split match and probe
Checks
Commit Message
From: Shreyansh Jain <shreyansh.jain@nxp.com>
Matching of PCI device address and driver ID table is being done at two
discreet locations duplicating the code. (rte_eal_pci_probe_one_driver
and rte_eal_pci_detach_dev).
Splitting the matching function into a public fn rte_pci_match.
Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
lib/librte_eal/bsdapp/eal/rte_eal_version.map | 1 +
lib/librte_eal/common/eal_common_pci.c | 189 +++++++++++++-----------
lib/librte_eal/common/include/rte_pci.h | 15 ++
lib/librte_eal/linuxapp/eal/rte_eal_version.map | 1 +
4 files changed, 121 insertions(+), 85 deletions(-)
Comments
On Thu, Jan 19, 2017 at 5:45 AM, Thomas Monjalon
<thomas.monjalon@6wind.com> wrote:
> From: Shreyansh Jain <shreyansh.jain@nxp.com>
>
> Matching of PCI device address and driver ID table is being done at two
> discreet locations duplicating the code. (rte_eal_pci_probe_one_driver
> and rte_eal_pci_detach_dev).
>
> Splitting the matching function into a public fn rte_pci_match.
>
> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> lib/librte_eal/bsdapp/eal/rte_eal_version.map | 1 +
> lib/librte_eal/common/eal_common_pci.c | 189 +++++++++++++-----------
> lib/librte_eal/common/include/rte_pci.h | 15 ++
> lib/librte_eal/linuxapp/eal/rte_eal_version.map | 1 +
> 4 files changed, 121 insertions(+), 85 deletions(-)
>
> diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> index a3d9fac..afa1643 100644
> --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> @@ -184,5 +184,6 @@ DPDK_17.02 {
> rte_bus_register;
> rte_bus_scan;
> rte_bus_unregister;
> + rte_pci_match;
>
> } DPDK_16.11;
> diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
> index 72547bd..4f155c6 100644
> --- a/lib/librte_eal/common/eal_common_pci.c
> +++ b/lib/librte_eal/common/eal_common_pci.c
> @@ -152,129 +152,148 @@ pci_unmap_resource(void *requested_addr, size_t size)
> requested_addr);
> }
>
> -/*
> - * If vendor/device ID match, call the probe() function of the
> - * driver.
> - */
> -static int
> -rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *dev)
> +int
> +rte_pci_match(const struct rte_pci_driver *pci_drv,
> + const struct rte_pci_device *pci_dev)
> {
> - int ret;
> + int match = 1;
> const struct rte_pci_id *id_table;
>
> - for (id_table = dr->id_table; id_table->vendor_id != 0; id_table++) {
> + if (!pci_drv || !pci_dev || !pci_drv->id_table) {
> + RTE_LOG(DEBUG, EAL, "Invalid PCI Driver object\n");
> + return -1;
> + }
>
> + for (id_table = pci_drv->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 &&
> + if (id_table->vendor_id != pci_dev->id.vendor_id &&
> id_table->vendor_id != PCI_ANY_ID)
> continue;
> - if (id_table->device_id != dev->id.device_id &&
> + if (id_table->device_id != pci_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)
> + if (id_table->subsystem_vendor_id !=
> + pci_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)
> + if (id_table->subsystem_device_id !=
> + pci_dev->id.subsystem_device_id &&
> + id_table->subsystem_device_id != PCI_ANY_ID)
> continue;
> - if (id_table->class_id != dev->id.class_id &&
> + if (id_table->class_id != pci_dev->id.class_id &&
> id_table->class_id != RTE_CLASS_ANY_ID)
> continue;
>
> - struct rte_pci_addr *loc = &dev->addr;
> -
> - RTE_LOG(INFO, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
> - loc->domain, loc->bus, loc->devid, loc->function,
> - dev->device.numa_node);
> -
> - /* no initialization when blacklisted, return without error */
> - if (dev->device.devargs != NULL &&
> - dev->device.devargs->type ==
> - RTE_DEVTYPE_BLACKLISTED_PCI) {
> - RTE_LOG(INFO, EAL, " Device is blacklisted, not initializing\n");
> - return 1;
> - }
> -
> - RTE_LOG(INFO, EAL, " probe driver: %x:%x %s\n", dev->id.vendor_id,
> - dev->id.device_id, dr->driver.name);
> -
> - if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
> - /* map resources for devices that use igb_uio */
> - ret = rte_eal_pci_map_device(dev);
> - if (ret != 0)
> - return ret;
> - }
> -
> - /* reference driver structure */
> - dev->driver = dr;
> -
> - /* call the driver probe() function */
> - ret = dr->probe(dr, dev);
> - if (ret) {
> - dev->driver = NULL;
> - if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
> - rte_eal_pci_unmap_device(dev);
> - }
> -
> - return ret;
> + match = 0;
> + break;
> }
> - /* return positive value if driver doesn't support this device */
> - return 1;
> +
> + return match;
> }
>
> /*
> - * If vendor/device ID match, call the remove() function of the
> + * If vendor/device ID match, call the probe() function of the
> * driver.
> */
> static int
> -rte_eal_pci_detach_dev(struct rte_pci_driver *dr,
> - struct rte_pci_device *dev)
> +rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr,
> + struct rte_pci_device *dev)
> {
> - const struct rte_pci_id *id_table;
> + int ret;
> + struct rte_pci_addr *loc;
>
> if ((dr == NULL) || (dev == NULL))
> return -EINVAL;
>
> - for (id_table = dr->id_table; id_table->vendor_id != 0; id_table++) {
> + loc = &dev->addr;
>
> - /* 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;
> + /* The device is not blacklisted; Check if driver supports it */
> + ret = rte_pci_match(dr, dev);
> + if (ret) {
> + /* Match of device and driver failed */
> + RTE_LOG(DEBUG, EAL, "Driver (%s) doesn't match the device\n",
> + dr->driver.name);
> + return 1;
> + }
>
> - struct rte_pci_addr *loc = &dev->addr;
> + RTE_LOG(INFO, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
> + loc->domain, loc->bus, loc->devid, loc->function,
> + dev->device.numa_node);
> +
> + /* no initialization when blacklisted, return without error */
> + if (dev->device.devargs != NULL &&
> + dev->device.devargs->type ==
> + RTE_DEVTYPE_BLACKLISTED_PCI) {
> + RTE_LOG(INFO, EAL, " Device is blacklisted, not"
> + " initializing\n");
> + return 1;
> + }
>
> - RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
> - loc->domain, loc->bus, loc->devid,
> - loc->function, dev->device.numa_node);
> + RTE_LOG(INFO, EAL, " probe driver: %x:%x %s\n", dev->id.vendor_id,
> + dev->id.device_id, dr->driver.name);
>
> - RTE_LOG(DEBUG, EAL, " remove driver: %x:%x %s\n", dev->id.vendor_id,
> - dev->id.device_id, dr->driver.name);
> + if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
> + /* map resources for devices that use igb_uio */
> + ret = rte_eal_pci_map_device(dev);
> + if (ret != 0)
> + return ret;
> + }
>
> - if (dr->remove && (dr->remove(dev) < 0))
> - return -1; /* negative value is an error */
> + /* reference driver structure */
> + dev->driver = dr;
>
> - /* clear driver structure */
> + /* call the driver probe() function */
> + ret = dr->probe(dr, dev);
> + if (ret) {
> dev->driver = NULL;
> -
> if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
> - /* unmap resources for devices that use igb_uio */
> rte_eal_pci_unmap_device(dev);
> + }
>
> - return 0;
> + return ret;
> +}
> +
> +/*
> + * If vendor/device ID match, call the remove() function of the
> + * driver.
> + */
> +static int
> +rte_eal_pci_detach_dev(struct rte_pci_driver *dr,
> + struct rte_pci_device *dev)
> +{
> + int ret;
> + struct rte_pci_addr *loc;
> +
> + if ((dr == NULL) || (dev == NULL))
> + return -EINVAL;
> +
> + ret = rte_pci_match(dr, dev);
> + if (ret) {
> + /* Device and driver don't match */
> + return 1;
> }
>
> - /* return positive value if driver doesn't support this device */
> - return 1;
> + 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->device.numa_node);
> +
> + RTE_LOG(DEBUG, EAL, " remove driver: %x:%x %s\n", dev->id.vendor_id,
> + dev->id.device_id, dr->driver.name);
> +
> + if (dr->remove && (dr->remove(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 */
> + rte_eal_pci_unmap_device(dev);
> +
> + return 0;
> }
>
> /*
> diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
> index 8557e47..adc20b9 100644
> --- a/lib/librte_eal/common/include/rte_pci.h
> +++ b/lib/librte_eal/common/include/rte_pci.h
> @@ -371,6 +371,21 @@ rte_eal_compare_pci_addr(const struct rte_pci_addr *addr,
> int rte_eal_pci_scan(void);
>
> /**
> + * Match the PCI Driver and Device using the ID Table
> + *
> + * @param pci_drv
> + * PCI driver from which ID table would be extracted
> + * @param pci_dev
> + * PCI device to match against the driver
> + * @return
> + * 0 for successful match
> + * !0 for unsuccessful match
> + */
> +int
> +rte_pci_match(const struct rte_pci_driver *pci_drv,
> + const struct rte_pci_device *pci_dev);
I don't think this symbol needs to be exported and visible outside of EAL.
> +
> +/**
> * Probe the PCI bus for registered drivers.
> *
> * Scan the content of the PCI bus, and call the probe() function for
> diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> index 69e0e7b..b557e7c 100644
> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> @@ -188,5 +188,6 @@ DPDK_17.02 {
> rte_bus_register;
> rte_bus_scan;
> rte_bus_unregister;
> + rte_pci_match;
>
> } DPDK_16.11;
> --
> 2.7.0
>
2017-02-15 11:45, Jan Blunck:
> > /**
> > + * Match the PCI Driver and Device using the ID Table
> > + *
> > + * @param pci_drv
> > + * PCI driver from which ID table would be extracted
> > + * @param pci_dev
> > + * PCI device to match against the driver
> > + * @return
> > + * 0 for successful match
> > + * !0 for unsuccessful match
> > + */
> > +int
> > +rte_pci_match(const struct rte_pci_driver *pci_drv,
> > + const struct rte_pci_device *pci_dev);
>
> I don't think this symbol needs to be exported and visible outside of EAL.
If I remember well, Shreyansh wants to re-use it for its own bus which
is closed to PCI.
I think it is weird and we could look for another way to re-use some code.
Shreyansh?
On Wednesday 15 February 2017 04:15 PM, Jan Blunck wrote:
> On Thu, Jan 19, 2017 at 5:45 AM, Thomas Monjalon
> <thomas.monjalon@6wind.com> wrote:
>> From: Shreyansh Jain <shreyansh.jain@nxp.com>
>>
>> Matching of PCI device address and driver ID table is being done at two
>> discreet locations duplicating the code. (rte_eal_pci_probe_one_driver
>> and rte_eal_pci_detach_dev).
>>
>> Splitting the matching function into a public fn rte_pci_match.
>>
>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> ---
>> lib/librte_eal/bsdapp/eal/rte_eal_version.map | 1 +
>> lib/librte_eal/common/eal_common_pci.c | 189 +++++++++++++-----------
>> lib/librte_eal/common/include/rte_pci.h | 15 ++
>> lib/librte_eal/linuxapp/eal/rte_eal_version.map | 1 +
>> 4 files changed, 121 insertions(+), 85 deletions(-)
>>
>> diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
>> index a3d9fac..afa1643 100644
>> --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
>> +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
>> @@ -184,5 +184,6 @@ DPDK_17.02 {
>> rte_bus_register;
>> rte_bus_scan;
>> rte_bus_unregister;
>> + rte_pci_match;
>>
>> } DPDK_16.11;
>> diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
>> index 72547bd..4f155c6 100644
>> --- a/lib/librte_eal/common/eal_common_pci.c
>> +++ b/lib/librte_eal/common/eal_common_pci.c
>> @@ -152,129 +152,148 @@ pci_unmap_resource(void *requested_addr, size_t size)
>> requested_addr);
>> }
>>
>> -/*
>> - * If vendor/device ID match, call the probe() function of the
>> - * driver.
>> - */
>> -static int
>> -rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *dev)
>> +int
>> +rte_pci_match(const struct rte_pci_driver *pci_drv,
>> + const struct rte_pci_device *pci_dev)
>> {
>> - int ret;
>> + int match = 1;
>> const struct rte_pci_id *id_table;
>>
>> - for (id_table = dr->id_table; id_table->vendor_id != 0; id_table++) {
>> + if (!pci_drv || !pci_dev || !pci_drv->id_table) {
>> + RTE_LOG(DEBUG, EAL, "Invalid PCI Driver object\n");
>> + return -1;
>> + }
>>
>> + for (id_table = pci_drv->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 &&
>> + if (id_table->vendor_id != pci_dev->id.vendor_id &&
>> id_table->vendor_id != PCI_ANY_ID)
>> continue;
>> - if (id_table->device_id != dev->id.device_id &&
>> + if (id_table->device_id != pci_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)
>> + if (id_table->subsystem_vendor_id !=
>> + pci_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)
>> + if (id_table->subsystem_device_id !=
>> + pci_dev->id.subsystem_device_id &&
>> + id_table->subsystem_device_id != PCI_ANY_ID)
>> continue;
>> - if (id_table->class_id != dev->id.class_id &&
>> + if (id_table->class_id != pci_dev->id.class_id &&
>> id_table->class_id != RTE_CLASS_ANY_ID)
>> continue;
>>
>> - struct rte_pci_addr *loc = &dev->addr;
>> -
>> - RTE_LOG(INFO, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
>> - loc->domain, loc->bus, loc->devid, loc->function,
>> - dev->device.numa_node);
>> -
>> - /* no initialization when blacklisted, return without error */
>> - if (dev->device.devargs != NULL &&
>> - dev->device.devargs->type ==
>> - RTE_DEVTYPE_BLACKLISTED_PCI) {
>> - RTE_LOG(INFO, EAL, " Device is blacklisted, not initializing\n");
>> - return 1;
>> - }
>> -
>> - RTE_LOG(INFO, EAL, " probe driver: %x:%x %s\n", dev->id.vendor_id,
>> - dev->id.device_id, dr->driver.name);
>> -
>> - if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
>> - /* map resources for devices that use igb_uio */
>> - ret = rte_eal_pci_map_device(dev);
>> - if (ret != 0)
>> - return ret;
>> - }
>> -
>> - /* reference driver structure */
>> - dev->driver = dr;
>> -
>> - /* call the driver probe() function */
>> - ret = dr->probe(dr, dev);
>> - if (ret) {
>> - dev->driver = NULL;
>> - if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
>> - rte_eal_pci_unmap_device(dev);
>> - }
>> -
>> - return ret;
>> + match = 0;
>> + break;
>> }
>> - /* return positive value if driver doesn't support this device */
>> - return 1;
>> +
>> + return match;
>> }
>>
>> /*
>> - * If vendor/device ID match, call the remove() function of the
>> + * If vendor/device ID match, call the probe() function of the
>> * driver.
>> */
>> static int
>> -rte_eal_pci_detach_dev(struct rte_pci_driver *dr,
>> - struct rte_pci_device *dev)
>> +rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr,
>> + struct rte_pci_device *dev)
>> {
>> - const struct rte_pci_id *id_table;
>> + int ret;
>> + struct rte_pci_addr *loc;
>>
>> if ((dr == NULL) || (dev == NULL))
>> return -EINVAL;
>>
>> - for (id_table = dr->id_table; id_table->vendor_id != 0; id_table++) {
>> + loc = &dev->addr;
>>
>> - /* 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;
>> + /* The device is not blacklisted; Check if driver supports it */
>> + ret = rte_pci_match(dr, dev);
>> + if (ret) {
>> + /* Match of device and driver failed */
>> + RTE_LOG(DEBUG, EAL, "Driver (%s) doesn't match the device\n",
>> + dr->driver.name);
>> + return 1;
>> + }
>>
>> - struct rte_pci_addr *loc = &dev->addr;
>> + RTE_LOG(INFO, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
>> + loc->domain, loc->bus, loc->devid, loc->function,
>> + dev->device.numa_node);
>> +
>> + /* no initialization when blacklisted, return without error */
>> + if (dev->device.devargs != NULL &&
>> + dev->device.devargs->type ==
>> + RTE_DEVTYPE_BLACKLISTED_PCI) {
>> + RTE_LOG(INFO, EAL, " Device is blacklisted, not"
>> + " initializing\n");
>> + return 1;
>> + }
>>
>> - RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
>> - loc->domain, loc->bus, loc->devid,
>> - loc->function, dev->device.numa_node);
>> + RTE_LOG(INFO, EAL, " probe driver: %x:%x %s\n", dev->id.vendor_id,
>> + dev->id.device_id, dr->driver.name);
>>
>> - RTE_LOG(DEBUG, EAL, " remove driver: %x:%x %s\n", dev->id.vendor_id,
>> - dev->id.device_id, dr->driver.name);
>> + if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
>> + /* map resources for devices that use igb_uio */
>> + ret = rte_eal_pci_map_device(dev);
>> + if (ret != 0)
>> + return ret;
>> + }
>>
>> - if (dr->remove && (dr->remove(dev) < 0))
>> - return -1; /* negative value is an error */
>> + /* reference driver structure */
>> + dev->driver = dr;
>>
>> - /* clear driver structure */
>> + /* call the driver probe() function */
>> + ret = dr->probe(dr, dev);
>> + if (ret) {
>> dev->driver = NULL;
>> -
>> if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
>> - /* unmap resources for devices that use igb_uio */
>> rte_eal_pci_unmap_device(dev);
>> + }
>>
>> - return 0;
>> + return ret;
>> +}
>> +
>> +/*
>> + * If vendor/device ID match, call the remove() function of the
>> + * driver.
>> + */
>> +static int
>> +rte_eal_pci_detach_dev(struct rte_pci_driver *dr,
>> + struct rte_pci_device *dev)
>> +{
>> + int ret;
>> + struct rte_pci_addr *loc;
>> +
>> + if ((dr == NULL) || (dev == NULL))
>> + return -EINVAL;
>> +
>> + ret = rte_pci_match(dr, dev);
>> + if (ret) {
>> + /* Device and driver don't match */
>> + return 1;
>> }
>>
>> - /* return positive value if driver doesn't support this device */
>> - return 1;
>> + 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->device.numa_node);
>> +
>> + RTE_LOG(DEBUG, EAL, " remove driver: %x:%x %s\n", dev->id.vendor_id,
>> + dev->id.device_id, dr->driver.name);
>> +
>> + if (dr->remove && (dr->remove(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 */
>> + rte_eal_pci_unmap_device(dev);
>> +
>> + return 0;
>> }
>>
>> /*
>> diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
>> index 8557e47..adc20b9 100644
>> --- a/lib/librte_eal/common/include/rte_pci.h
>> +++ b/lib/librte_eal/common/include/rte_pci.h
>> @@ -371,6 +371,21 @@ rte_eal_compare_pci_addr(const struct rte_pci_addr *addr,
>> int rte_eal_pci_scan(void);
>>
>> /**
>> + * Match the PCI Driver and Device using the ID Table
>> + *
>> + * @param pci_drv
>> + * PCI driver from which ID table would be extracted
>> + * @param pci_dev
>> + * PCI device to match against the driver
>> + * @return
>> + * 0 for successful match
>> + * !0 for unsuccessful match
>> + */
>> +int
>> +rte_pci_match(const struct rte_pci_driver *pci_drv,
>> + const struct rte_pci_device *pci_dev);
>
> I don't think this symbol needs to be exported and visible outside of EAL.
Agree.
2 reasons why this exists:
1. Initial RFC has match as a callback. That doesn't exist now.
2. Earlier the idea was that other buses/device-types might use this
function (buses which are PCI like), but that is not a good
assumption.
>
>> +
>> +/**
>> * Probe the PCI bus for registered drivers.
>> *
>> * Scan the content of the PCI bus, and call the probe() function for
>> diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>> index 69e0e7b..b557e7c 100644
>> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>> @@ -188,5 +188,6 @@ DPDK_17.02 {
>> rte_bus_register;
>> rte_bus_scan;
>> rte_bus_unregister;
>> + rte_pci_match;
>>
>> } DPDK_16.11;
>> --
>> 2.7.0
>>
>
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, February 15, 2017 4:53 PM
> To: Jan Blunck <jblunck@infradead.org>; Shreyansh Jain
> <shreyansh.jain@nxp.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v11 07/13] pci: split match and probe
>
> 2017-02-15 11:45, Jan Blunck:
> > > /**
> > > + * Match the PCI Driver and Device using the ID Table
> > > + *
> > > + * @param pci_drv
> > > + * PCI driver from which ID table would be extracted
> > > + * @param pci_dev
> > > + * PCI device to match against the driver
> > > + * @return
> > > + * 0 for successful match
> > > + * !0 for unsuccessful match
> > > + */
> > > +int
> > > +rte_pci_match(const struct rte_pci_driver *pci_drv,
> > > + const struct rte_pci_device *pci_dev);
> >
> > I don't think this symbol needs to be exported and visible outside of EAL.
>
> If I remember well, Shreyansh wants to re-use it for its own bus which
> is closed to PCI.
> I think it is weird and we could look for another way to re-use some code.
> Shreyansh?
From what I recall pci_match _was_ exported was because:
1. earlier match was a callback for bus. Which is not the case now
2. idea was that some other Buses might be able to re-use this. NXP DPAA2 implementation (in early stages).
I don't think this is needed anymore.
@@ -184,5 +184,6 @@ DPDK_17.02 {
rte_bus_register;
rte_bus_scan;
rte_bus_unregister;
+ rte_pci_match;
} DPDK_16.11;
@@ -152,129 +152,148 @@ pci_unmap_resource(void *requested_addr, size_t size)
requested_addr);
}
-/*
- * If vendor/device ID match, call the probe() function of the
- * driver.
- */
-static int
-rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *dev)
+int
+rte_pci_match(const struct rte_pci_driver *pci_drv,
+ const struct rte_pci_device *pci_dev)
{
- int ret;
+ int match = 1;
const struct rte_pci_id *id_table;
- for (id_table = dr->id_table; id_table->vendor_id != 0; id_table++) {
+ if (!pci_drv || !pci_dev || !pci_drv->id_table) {
+ RTE_LOG(DEBUG, EAL, "Invalid PCI Driver object\n");
+ return -1;
+ }
+ for (id_table = pci_drv->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 &&
+ if (id_table->vendor_id != pci_dev->id.vendor_id &&
id_table->vendor_id != PCI_ANY_ID)
continue;
- if (id_table->device_id != dev->id.device_id &&
+ if (id_table->device_id != pci_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)
+ if (id_table->subsystem_vendor_id !=
+ pci_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)
+ if (id_table->subsystem_device_id !=
+ pci_dev->id.subsystem_device_id &&
+ id_table->subsystem_device_id != PCI_ANY_ID)
continue;
- if (id_table->class_id != dev->id.class_id &&
+ if (id_table->class_id != pci_dev->id.class_id &&
id_table->class_id != RTE_CLASS_ANY_ID)
continue;
- struct rte_pci_addr *loc = &dev->addr;
-
- RTE_LOG(INFO, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
- loc->domain, loc->bus, loc->devid, loc->function,
- dev->device.numa_node);
-
- /* no initialization when blacklisted, return without error */
- if (dev->device.devargs != NULL &&
- dev->device.devargs->type ==
- RTE_DEVTYPE_BLACKLISTED_PCI) {
- RTE_LOG(INFO, EAL, " Device is blacklisted, not initializing\n");
- return 1;
- }
-
- RTE_LOG(INFO, EAL, " probe driver: %x:%x %s\n", dev->id.vendor_id,
- dev->id.device_id, dr->driver.name);
-
- if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
- /* map resources for devices that use igb_uio */
- ret = rte_eal_pci_map_device(dev);
- if (ret != 0)
- return ret;
- }
-
- /* reference driver structure */
- dev->driver = dr;
-
- /* call the driver probe() function */
- ret = dr->probe(dr, dev);
- if (ret) {
- dev->driver = NULL;
- if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
- rte_eal_pci_unmap_device(dev);
- }
-
- return ret;
+ match = 0;
+ break;
}
- /* return positive value if driver doesn't support this device */
- return 1;
+
+ return match;
}
/*
- * If vendor/device ID match, call the remove() function of the
+ * If vendor/device ID match, call the probe() function of the
* driver.
*/
static int
-rte_eal_pci_detach_dev(struct rte_pci_driver *dr,
- struct rte_pci_device *dev)
+rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr,
+ struct rte_pci_device *dev)
{
- const struct rte_pci_id *id_table;
+ int ret;
+ struct rte_pci_addr *loc;
if ((dr == NULL) || (dev == NULL))
return -EINVAL;
- for (id_table = dr->id_table; id_table->vendor_id != 0; id_table++) {
+ loc = &dev->addr;
- /* 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;
+ /* The device is not blacklisted; Check if driver supports it */
+ ret = rte_pci_match(dr, dev);
+ if (ret) {
+ /* Match of device and driver failed */
+ RTE_LOG(DEBUG, EAL, "Driver (%s) doesn't match the device\n",
+ dr->driver.name);
+ return 1;
+ }
- struct rte_pci_addr *loc = &dev->addr;
+ RTE_LOG(INFO, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
+ loc->domain, loc->bus, loc->devid, loc->function,
+ dev->device.numa_node);
+
+ /* no initialization when blacklisted, return without error */
+ if (dev->device.devargs != NULL &&
+ dev->device.devargs->type ==
+ RTE_DEVTYPE_BLACKLISTED_PCI) {
+ RTE_LOG(INFO, EAL, " Device is blacklisted, not"
+ " initializing\n");
+ return 1;
+ }
- RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
- loc->domain, loc->bus, loc->devid,
- loc->function, dev->device.numa_node);
+ RTE_LOG(INFO, EAL, " probe driver: %x:%x %s\n", dev->id.vendor_id,
+ dev->id.device_id, dr->driver.name);
- RTE_LOG(DEBUG, EAL, " remove driver: %x:%x %s\n", dev->id.vendor_id,
- dev->id.device_id, dr->driver.name);
+ if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
+ /* map resources for devices that use igb_uio */
+ ret = rte_eal_pci_map_device(dev);
+ if (ret != 0)
+ return ret;
+ }
- if (dr->remove && (dr->remove(dev) < 0))
- return -1; /* negative value is an error */
+ /* reference driver structure */
+ dev->driver = dr;
- /* clear driver structure */
+ /* call the driver probe() function */
+ ret = dr->probe(dr, dev);
+ if (ret) {
dev->driver = NULL;
-
if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
- /* unmap resources for devices that use igb_uio */
rte_eal_pci_unmap_device(dev);
+ }
- return 0;
+ return ret;
+}
+
+/*
+ * If vendor/device ID match, call the remove() function of the
+ * driver.
+ */
+static int
+rte_eal_pci_detach_dev(struct rte_pci_driver *dr,
+ struct rte_pci_device *dev)
+{
+ int ret;
+ struct rte_pci_addr *loc;
+
+ if ((dr == NULL) || (dev == NULL))
+ return -EINVAL;
+
+ ret = rte_pci_match(dr, dev);
+ if (ret) {
+ /* Device and driver don't match */
+ return 1;
}
- /* return positive value if driver doesn't support this device */
- return 1;
+ 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->device.numa_node);
+
+ RTE_LOG(DEBUG, EAL, " remove driver: %x:%x %s\n", dev->id.vendor_id,
+ dev->id.device_id, dr->driver.name);
+
+ if (dr->remove && (dr->remove(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 */
+ rte_eal_pci_unmap_device(dev);
+
+ return 0;
}
/*
@@ -371,6 +371,21 @@ rte_eal_compare_pci_addr(const struct rte_pci_addr *addr,
int rte_eal_pci_scan(void);
/**
+ * Match the PCI Driver and Device using the ID Table
+ *
+ * @param pci_drv
+ * PCI driver from which ID table would be extracted
+ * @param pci_dev
+ * PCI device to match against the driver
+ * @return
+ * 0 for successful match
+ * !0 for unsuccessful match
+ */
+int
+rte_pci_match(const struct rte_pci_driver *pci_drv,
+ const struct rte_pci_device *pci_dev);
+
+/**
* Probe the PCI bus for registered drivers.
*
* Scan the content of the PCI bus, and call the probe() function for
@@ -188,5 +188,6 @@ DPDK_17.02 {
rte_bus_register;
rte_bus_scan;
rte_bus_unregister;
+ rte_pci_match;
} DPDK_16.11;