[dpdk-dev,v6,3/8] pci: split match and probe function
Checks
Commit Message
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>
---
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 Mon, 16 Jan 2017 21:08:22 +0530
Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
> -rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *dev)
> +int
> +rte_pci_match(struct rte_pci_driver *pci_drv,
> + struct rte_pci_device *pci_dev)
Use const?
On 1/16/2017 3:38 PM, Shreyansh Jain wrote:
> 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>
<...>
> /*
> - * 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;
> + 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);
This cause bunch of log printed during app startup, what about printing
this log when probed device found?
>
> - struct rte_pci_addr *loc = &dev->addr;
> + /* 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;
> + }
> +
> + /* 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);
Same for this one, this line cause printing all registered drivers for
each device during app initialization, only matched one can be logged.
>
> - 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;
> +}
<...>
> diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> index b553b13..5ed2589 100644
> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> @@ -186,5 +186,6 @@ DPDK_17.02 {
> rte_bus_dump;
> rte_bus_register;
> rte_bus_unregister;
> + rte_pci_match;
I think this is internal API, should library expose this API?
>
> } DPDK_16.11;
>
Hello Ferruh,
On Tuesday 17 January 2017 01:23 AM, Ferruh Yigit wrote:
> On 1/16/2017 3:38 PM, Shreyansh Jain wrote:
>> 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>
>
> <...>
>
>> /*
>> - * 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;
>> + 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);
>
> This cause bunch of log printed during app startup, what about printing
> this log when probed device found?
Only thing I did was move around this log message without adding
anything new. Maybe earlier it was in some conditional (match) and now
it isn't. I will check again and move to match-only case.
>
>>
>> - struct rte_pci_addr *loc = &dev->addr;
>> + /* 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;
>> + }
>> +
>> + /* 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);
>
> Same for this one, this line cause printing all registered drivers for
> each device during app initialization, only matched one can be logged.
Same. Will post v7 shortly with only match case printing.
What about DEBUG for all cases?
>
>>
>> - 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;
>> +}
>
> <...>
>
>> diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>> index b553b13..5ed2589 100644
>> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>> @@ -186,5 +186,6 @@ DPDK_17.02 {
>> rte_bus_dump;
>> rte_bus_register;
>> rte_bus_unregister;
>> + rte_pci_match;
>
> I think this is internal API, should library expose this API?
Idea is that pci_match be useable outside of PCI for any other PCI-like
bus (BDF compliant). For example, one of NXP's devices are very close to
PCI (but not exactly PCI) and they too rely on BDF for addressing/naming.
>
>>
>> } DPDK_16.11;
>>
>
>
-
Shreyansh
On 1/17/2017 4:54 AM, Shreyansh Jain wrote:
> Hello Ferruh,
>
> On Tuesday 17 January 2017 01:23 AM, Ferruh Yigit wrote:
>> On 1/16/2017 3:38 PM, Shreyansh Jain wrote:
>>> 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>
>>
>> <...>
>>
>>> /*
>>> - * 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;
>>> + 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);
>>
>> This cause bunch of log printed during app startup, what about printing
>> this log when probed device found?
>
> Only thing I did was move around this log message without adding
> anything new. Maybe earlier it was in some conditional (match) and now
> it isn't. I will check again and move to match-only case.
>
>>
>>>
>>> - struct rte_pci_addr *loc = &dev->addr;
>>> + /* 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;
>>> + }
>>> +
>>> + /* 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);
>>
>> Same for this one, this line cause printing all registered drivers for
>> each device during app initialization, only matched one can be logged.
>
> Same. Will post v7 shortly with only match case printing.
> What about DEBUG for all cases?
I would prefer existing behavior, INFO level for successfully probed
device and driver, but no strong opinion.
>
>>
>>>
>>> - 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;
>>> +}
>>
>> <...>
>>
>>> diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>>> index b553b13..5ed2589 100644
>>> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>>> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>>> @@ -186,5 +186,6 @@ DPDK_17.02 {
>>> rte_bus_dump;
>>> rte_bus_register;
>>> rte_bus_unregister;
>>> + rte_pci_match;
>>
>> I think this is internal API, should library expose this API?
>
> Idea is that pci_match be useable outside of PCI for any other PCI-like
> bus (BDF compliant). For example, one of NXP's devices are very close to
> PCI (but not exactly PCI) and they too rely on BDF for addressing/naming.
OK.
>
>>
>>>
>>> } DPDK_16.11;
>>>
>>
>>
>
> -
> Shreyansh
>
On Monday 16 January 2017 11:54 PM, Stephen Hemminger wrote:
> On Mon, 16 Jan 2017 21:08:22 +0530
> Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
>
>> -rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *dev)
>> +int
>> +rte_pci_match(struct rte_pci_driver *pci_drv,
>> + struct rte_pci_device *pci_dev)
>
> Use const?
>
Done in v7. Thanks
On Tuesday 17 January 2017 03:28 PM, Ferruh Yigit wrote:
> On 1/17/2017 4:54 AM, Shreyansh Jain wrote:
>> Hello Ferruh,
>>
>> On Tuesday 17 January 2017 01:23 AM, Ferruh Yigit wrote:
>>> On 1/16/2017 3:38 PM, Shreyansh Jain wrote:
>>>> 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>
>>>
>>> <...>
>>>
>>>> /*
>>>> - * 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;
>>>> + 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);
>>>
>>> This cause bunch of log printed during app startup, what about printing
>>> this log when probed device found?
>>
>> Only thing I did was move around this log message without adding
>> anything new. Maybe earlier it was in some conditional (match) and now
>> it isn't. I will check again and move to match-only case.
>>
>>>
>>>>
>>>> - struct rte_pci_addr *loc = &dev->addr;
>>>> + /* 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;
>>>> + }
>>>> +
>>>> + /* 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);
>>>
>>> Same for this one, this line cause printing all registered drivers for
>>> each device during app initialization, only matched one can be logged.
>>
>> Same. Will post v7 shortly with only match case printing.
>> What about DEBUG for all cases?
>
> I would prefer existing behavior, INFO level for successfully probed
> device and driver, but no strong opinion.
Reverted to existing behavior. It was a miss from my side. I had moved
this log message _before_ matching unlike the original code.
Pushed v7 to ML.
>
>>
>>>
>>>>
>>>> - 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;
>>>> +}
>>>
>>> <...>
>>>
>>>> diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>>>> index b553b13..5ed2589 100644
>>>> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>>>> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>>>> @@ -186,5 +186,6 @@ DPDK_17.02 {
>>>> rte_bus_dump;
>>>> rte_bus_register;
>>>> rte_bus_unregister;
>>>> + rte_pci_match;
>>>
>>> I think this is internal API, should library expose this API?
>>
>> Idea is that pci_match be useable outside of PCI for any other PCI-like
>> bus (BDF compliant). For example, one of NXP's devices are very close to
>> PCI (but not exactly PCI) and they too rely on BDF for addressing/naming.
>
> OK.
>
>>
>>>
>>>>
>>>> } DPDK_16.11;
>>>>
>>>
>>>
>>
>> -
>> Shreyansh
>>
>
>
@@ -182,5 +182,6 @@ DPDK_17.02 {
rte_bus_dump;
rte_bus_register;
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(struct rte_pci_driver *pci_drv,
+ 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;
+ 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);
- struct rte_pci_addr *loc = &dev->addr;
+ /* 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;
+ }
+
+ /* 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(struct rte_pci_driver *pci_drv,
+ 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
@@ -186,5 +186,6 @@ DPDK_17.02 {
rte_bus_dump;
rte_bus_register;
rte_bus_unregister;
+ rte_pci_match;
} DPDK_16.11;