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



Reply via email to