[dpdk-dev] [PATCH v11 07/13] pci: split match and probe

Shreyansh Jain shreyansh.jain at nxp.com
Wed Feb 15 12:30:19 CET 2017


On Wednesday 15 February 2017 04:15 PM, Jan Blunck wrote:
> On Thu, Jan 19, 2017 at 5:45 AM, Thomas Monjalon
> <thomas.monjalon at 6wind.com> wrote:
>> From: Shreyansh Jain <shreyansh.jain at 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 at nxp.com>
>> Reviewed-by: Ferruh Yigit <ferruh.yigit at 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
>>
>



More information about the dev mailing list