[PATCH 0/9] pci cleanup and blacklist rework

[PATCH 0/9] pci cleanup and blacklist rework

[David Marchand]

Before 2.2.0 release, while preparing for more changes in eal (and fixing
a problem reported by Roger M. [1]), I came up with this patchset that
tries to make the pci code more compact and easier to read.

I did limited testing, but this has been in my tree for quite some time
now, so sending this to get feedback for 2.3 / 16.04.


The 4th patch introduces a change in linux eal.
Before, if a pci device was bound to no kernel driver, eal would set kdrv
to "unknown". With this change, kdrv is set to "none".
This might make it possible to avoid the old issue of virtio devices being
used by dpdk while still bound to kernel driver reported by Franck B. [2].
I'll let virtio guys look at this.
At the very least, it makes more sense to me.


The 5th and 6th patches could be squashed.
I just find it easier to review this way.


In the 8th patch, I noticed that a driver lookup is done when detaching a
pci device while this makes no sense and looks buggy to me.


The last patch moves pci blacklist evaluation to rte_eal_pci_probe().
With this, it is now possible to attach a pci device blacklisted
initially.
I am not entirely happy with this last patch, I will try to send a
different solution next week (hooks in pci layer).


[1] http://dpdk.org/ml/archives/dev/2015-November/028140.html
[2] http://dpdk.org/ml/archives/dev/2015-September/023389.html

-- 
David Marchand

David Marchand (9):
  pci: no need for dynamic tailq init
  pci: add internal device list helpers
  pci: minor cleanup
  pci: rework sysfs parsing for driver
  pci: factorize probe/detach code
  pci: cosmetic change
  pci: factorize driver search
  pci: remove driver lookup from detach
  pci: blacklist only in global probe function

 lib/librte_eal/bsdapp/eal/eal_pci.c     |  37 +---
 lib/librte_eal/common/eal_common_pci.c  | 320 +++++++++++++++-----------------
 lib/librte_eal/common/include/rte_pci.h |  24 +++
 lib/librte_eal/linuxapp/eal/eal_pci.c   |  94 ++++------
 4 files changed, 218 insertions(+), 257 deletions(-)

-- 
1.9.1

[PATCH 1/9] pci: no need for dynamic tailq init

[David Marchand]

These lists can be initialized once and for all at build time.
With this, those lists are only manipulated in a common place
(and we could even make them private).

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_eal/bsdapp/eal/eal_pci.c    | 3 ---
 lib/librte_eal/common/eal_common_pci.c | 6 ++++--
 lib/librte_eal/linuxapp/eal/eal_pci.c  | 3 ---
 3 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
index 6c21fbd..4584522 100644
--- a/lib/librte_eal/bsdapp/eal/eal_pci.c
+++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
@@ -483,9 +483,6 @@ int rte_eal_pci_write_config(const struct rte_pci_device *dev,
 int
 rte_eal_pci_init(void)
 {
-	TAILQ_INIT(&pci_driver_list);
-	TAILQ_INIT(&pci_device_list);
-
 	/* for debug purposes, PCI can be disabled */
 	if (internal_config.no_pci)
 		return 0;
diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index dcfe947..1e12776 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -82,8 +82,10 @@
 
 #include "eal_private.h"
 
-struct pci_driver_list pci_driver_list;
-struct pci_device_list pci_device_list;
+struct pci_driver_list pci_driver_list =
+	TAILQ_HEAD_INITIALIZER(pci_driver_list);
+struct pci_device_list pci_device_list =
+	TAILQ_HEAD_INITIALIZER(pci_device_list);
 
 static struct rte_devargs *pci_devargs_lookup(struct rte_pci_device *dev)
 {
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index bc5b5be..a354f76 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -625,9 +625,6 @@ int rte_eal_pci_write_config(const struct rte_pci_device *device,
 int
 rte_eal_pci_init(void)
 {
-	TAILQ_INIT(&pci_driver_list);
-	TAILQ_INIT(&pci_device_list);
-
 	/* for debug purposes, PCI can be disabled */
 	if (internal_config.no_pci)
 		return 0;
-- 
1.9.1

[PATCH 2/9] pci: add internal device list helpers

[David Marchand]

Remove duplicate code by moving this to helpers in common.

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_eal/bsdapp/eal/eal_pci.c     | 34 +++++-----------
 lib/librte_eal/common/eal_common_pci.c  | 70 ++++++++++++++++++++++++---------
 lib/librte_eal/common/include/rte_pci.h | 24 +++++++++++
 lib/librte_eal/linuxapp/eal/eal_pci.c   | 34 +++++-----------
 4 files changed, 94 insertions(+), 68 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
index 4584522..401cda3 100644
--- a/lib/librte_eal/bsdapp/eal/eal_pci.c
+++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
@@ -247,6 +247,7 @@ static int
 pci_scan_one(int dev_pci_fd, struct pci_conf *conf)
 {
 	struct rte_pci_device *dev;
+	struct rte_pci_device *dev2;
 	struct pci_bar_io bar;
 	unsigned i, max;
 
@@ -311,32 +312,15 @@ pci_scan_one(int dev_pci_fd, struct pci_conf *conf)
 		dev->mem_resource[i].phys_addr = bar.pbi_base & ~((uint64_t)0xf);
 	}
 
-	/* device is valid, add in list (sorted) */
-	if (TAILQ_EMPTY(&pci_device_list)) {
-		TAILQ_INSERT_TAIL(&pci_device_list, dev, next);
-	}
+	dev2 = pci_find_device(&dev->addr);
+	if (!dev2)
+		pci_add_device(dev);
 	else {
-		struct rte_pci_device *dev2 = NULL;
-		int ret;
-
-		TAILQ_FOREACH(dev2, &pci_device_list, next) {
-			ret = rte_eal_compare_pci_addr(&dev->addr, &dev2->addr);
-			if (ret > 0)
-				continue;
-			else if (ret < 0) {
-				TAILQ_INSERT_BEFORE(dev2, dev, next);
-				return 0;
-			} else { /* already registered */
-				dev2->kdrv = dev->kdrv;
-				dev2->max_vfs = dev->max_vfs;
-				memmove(dev2->mem_resource,
-					dev->mem_resource,
-					sizeof(dev->mem_resource));
-				free(dev);
-				return 0;
-			}
-		}
-		TAILQ_INSERT_TAIL(&pci_device_list, dev, next);
+		dev2->kdrv = dev->kdrv;
+		dev2->max_vfs = dev->max_vfs;
+		memmove(dev2->mem_resource, dev->mem_resource,
+			sizeof(dev->mem_resource));
+		free(dev);
 	}
 
 	return 0;
diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index 1e12776..2528775 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -139,6 +139,41 @@ pci_unmap_resource(void *requested_addr, size_t size)
 				requested_addr);
 }
 
+struct rte_pci_device *
+pci_find_device(const struct rte_pci_addr *addr)
+{
+	struct rte_pci_device *dev;
+
+	TAILQ_FOREACH(dev, &pci_device_list, next) {
+		if (!rte_eal_compare_pci_addr(&dev->addr, addr))
+			break;
+	}
+
+	return dev;
+}
+
+int
+pci_add_device(struct rte_pci_device *dev)
+{
+	struct rte_pci_device *dev2;
+	int ret;
+
+	TAILQ_FOREACH(dev2, &pci_device_list, next) {
+		ret = rte_eal_compare_pci_addr(&dev->addr, &dev2->addr);
+		if (ret > 0)
+			continue;
+
+		if (ret == 0)
+			return -1;
+
+		TAILQ_INSERT_BEFORE(dev2, dev, next);
+		return 0;
+	}
+
+	TAILQ_INSERT_TAIL(&pci_device_list, dev, next);
+	return 0;
+}
+
 /*
  * If vendor/device ID match, call the devinit() function of the
  * driver.
@@ -332,16 +367,15 @@ rte_eal_pci_probe_one(const struct rte_pci_addr *addr)
 	if (addr == NULL)
 		return -1;
 
-	TAILQ_FOREACH(dev, &pci_device_list, next) {
-		if (rte_eal_compare_pci_addr(&dev->addr, addr))
-			continue;
+	dev = pci_find_device(addr);
+	if (!dev)
+		goto err_return;
 
-		ret = pci_probe_all_drivers(dev);
-		if (ret < 0)
-			goto err_return;
-		return 0;
-	}
-	return -1;
+	ret = pci_probe_all_drivers(dev);
+	if (ret < 0)
+		goto err_return;
+
+	return 0;
 
 err_return:
 	RTE_LOG(WARNING, EAL, "Requested device " PCI_PRI_FMT
@@ -362,18 +396,16 @@ rte_eal_pci_detach(const struct rte_pci_addr *addr)
 	if (addr == NULL)
 		return -1;
 
-	TAILQ_FOREACH(dev, &pci_device_list, next) {
-		if (rte_eal_compare_pci_addr(&dev->addr, addr))
-			continue;
+	dev = pci_find_device(addr);
+	if (!dev)
+		goto err_return;
 
-		ret = pci_detach_all_drivers(dev);
-		if (ret < 0)
-			goto err_return;
+	ret = pci_detach_all_drivers(dev);
+	if (ret < 0)
+		goto err_return;
 
-		TAILQ_REMOVE(&pci_device_list, dev, next);
-		return 0;
-	}
-	return -1;
+	TAILQ_REMOVE(&pci_device_list, dev, next);
+	return 0;
 
 err_return:
 	RTE_LOG(WARNING, EAL, "Requested device " PCI_PRI_FMT
diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 334c12e..4ba7192 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -399,6 +399,30 @@ void *pci_map_resource(void *requested_addr, int fd, off_t offset,
 void pci_unmap_resource(void *requested_addr, size_t size);
 
 /**
+ * @internal
+ * Add a pci device to sorted pci device list
+ *
+ * @param dev
+ *	The PCI device object.
+ * @return
+ *   - 0 on success.
+ *   - negative on error.
+ */
+int pci_add_device(struct rte_pci_device *dev);
+
+/**
+ * @internal
+ * Find a pci device object based on a PCI Bus-Device-Function address.
+ *
+ * @param addr
+ *	The PCI Bus-Device-Function address to look for
+ * @return
+ *   - pointer to the object,
+ *   - NULL when not found.
+ */
+struct rte_pci_device *pci_find_device(const struct rte_pci_addr *addr);
+
+/**
  * Probe the single PCI device.
  *
  * Scan the content of the PCI bus, and find the pci device specified by pci
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index a354f76..83eec5d 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -259,6 +259,7 @@ pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
 	char filename[PATH_MAX];
 	unsigned long tmp;
 	struct rte_pci_device *dev;
+	struct rte_pci_device *dev2;
 	char driver[PATH_MAX];
 	int ret;
 
@@ -364,30 +365,15 @@ pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
 	} else
 		dev->kdrv = RTE_KDRV_UNKNOWN;
 
-	/* device is valid, add in list (sorted) */
-	if (TAILQ_EMPTY(&pci_device_list)) {
-		TAILQ_INSERT_TAIL(&pci_device_list, dev, next);
-	} else {
-		struct rte_pci_device *dev2;
-		int ret;
-
-		TAILQ_FOREACH(dev2, &pci_device_list, next) {
-			ret = rte_eal_compare_pci_addr(&dev->addr, &dev2->addr);
-			if (ret > 0)
-				continue;
-
-			if (ret < 0) {
-				TAILQ_INSERT_BEFORE(dev2, dev, next);
-			} else { /* already registered */
-				dev2->kdrv = dev->kdrv;
-				dev2->max_vfs = dev->max_vfs;
-				memmove(dev2->mem_resource, dev->mem_resource,
-					sizeof(dev->mem_resource));
-				free(dev);
-			}
-			return 0;
-		}
-		TAILQ_INSERT_TAIL(&pci_device_list, dev, next);
+	dev2 = pci_find_device(&dev->addr);
+	if (!dev2)
+		pci_add_device(dev);
+	else {
+		dev2->kdrv = dev->kdrv;
+		dev2->max_vfs = dev->max_vfs;
+		memmove(dev2->mem_resource, dev->mem_resource,
+			sizeof(dev->mem_resource));
+		free(dev);
 	}
 
 	return 0;
-- 
1.9.1

[PATCH 3/9] pci: minor cleanup

[David Marchand]

Those are the only fields that are explicitly set to 0 while the global
dev pointer is set to 0 a few lines before.

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_eal/linuxapp/eal/eal_pci.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index 83eec5d..7bec30f 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -308,7 +308,6 @@ pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
 	dev->id.subsystem_device_id = (uint16_t)tmp;
 
 	/* get max_vfs */
-	dev->max_vfs = 0;
 	snprintf(filename, sizeof(filename), "%s/max_vfs", dirname);
 	if (!access(filename, F_OK) &&
 	    eal_parse_sysfs_value(filename, &tmp) == 0)
@@ -323,12 +322,8 @@ pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
 	}
 
 	/* get numa node */
-	snprintf(filename, sizeof(filename), "%s/numa_node",
-		 dirname);
-	if (access(filename, R_OK) != 0) {
-		/* if no NUMA support, set default to 0 */
-		dev->numa_node = 0;
-	} else {
+	snprintf(filename, sizeof(filename), "%s/numa_node", dirname);
+	if (!access(filename, R_OK)) {
 		if (eal_parse_sysfs_value(filename, &tmp) < 0) {
 			free(dev);
 			return -1;
-- 
1.9.1

[PATCH 4/9] pci: rework sysfs parsing for driver

[David Marchand]

There is no use for pci_get_kernel_driver_by_path() apart recognising
kernel driver and fill kdrv field.
If driver parsing fails, report "none" driver rather than "unknown".

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_eal/linuxapp/eal/eal_pci.c | 48 ++++++++++++++---------------------
 1 file changed, 19 insertions(+), 29 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index 7bec30f..00fdee7 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -94,32 +94,37 @@ error:
 }
 
 static int
-pci_get_kernel_driver_by_path(const char *filename, char *dri_name)
+pci_parse_sysfs_driver(const char *filename, struct rte_pci_device *dev)
 {
 	int count;
 	char path[PATH_MAX];
 	char *name;
 
-	if (!filename || !dri_name)
-		return -1;
-
 	count = readlink(filename, path, PATH_MAX);
 	if (count >= PATH_MAX)
 		return -1;
 
-	/* For device does not have a driver */
-	if (count < 0)
-		return 1;
+	dev->kdrv = RTE_KDRV_NONE;
 
-	path[count] = '\0';
+	if (count > 0) {
+		dev->kdrv = RTE_KDRV_UNKNOWN;
 
-	name = strrchr(path, '/');
-	if (name) {
-		strncpy(dri_name, name + 1, strlen(name + 1) + 1);
-		return 0;
+		path[count] = '\0';
+		name = strrchr(path, '/');
+		if (name) {
+			name[0] = '\0';
+			name++;
+		}
+
+		if (!strcmp(name, "vfio-pci"))
+			dev->kdrv = RTE_KDRV_VFIO;
+		else if (!strcmp(name, "igb_uio"))
+			dev->kdrv = RTE_KDRV_IGB_UIO;
+		else if (!strcmp(name, "uio_pci_generic"))
+			dev->kdrv = RTE_KDRV_UIO_GENERIC;
 	}
 
-	return -1;
+	return 0;
 }
 
 /* Map pci device */
@@ -260,8 +265,6 @@ pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
 	unsigned long tmp;
 	struct rte_pci_device *dev;
 	struct rte_pci_device *dev2;
-	char driver[PATH_MAX];
-	int ret;
 
 	dev = malloc(sizeof(*dev));
 	if (dev == NULL)
@@ -341,25 +344,12 @@ pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
 
 	/* parse driver */
 	snprintf(filename, sizeof(filename), "%s/driver", dirname);
-	ret = pci_get_kernel_driver_by_path(filename, driver);
-	if (ret < 0) {
+	if (pci_parse_sysfs_driver(filename, dev) < 0) {
 		RTE_LOG(ERR, EAL, "Fail to get kernel driver\n");
 		free(dev);
 		return -1;
 	}
 
-	if (!ret) {
-		if (!strcmp(driver, "vfio-pci"))
-			dev->kdrv = RTE_KDRV_VFIO;
-		else if (!strcmp(driver, "igb_uio"))
-			dev->kdrv = RTE_KDRV_IGB_UIO;
-		else if (!strcmp(driver, "uio_pci_generic"))
-			dev->kdrv = RTE_KDRV_UIO_GENERIC;
-		else
-			dev->kdrv = RTE_KDRV_UNKNOWN;
-	} else
-		dev->kdrv = RTE_KDRV_UNKNOWN;
-
 	dev2 = pci_find_device(&dev->addr);
 	if (!dev2)
 		pci_add_device(dev);
-- 
1.9.1

[PATCH 5/9] pci: factorize probe/detach code

[David Marchand]

Move pci id matching to a helper and reuse it in probe and detach
functions.

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_eal/common/eal_common_pci.c | 67 ++++++++++++----------------------
 1 file changed, 23 insertions(+), 44 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index 2528775..44549f7 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -174,14 +174,10 @@ pci_add_device(struct rte_pci_device *dev)
 	return 0;
 }
 
-/*
- * If vendor/device ID match, call the devinit() function of the
- * driver.
- */
 static int
-rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *dev)
+pci_driver_supports_device(const struct rte_pci_driver *dr,
+			   const struct rte_pci_device *dev)
 {
-	int ret;
 	const struct rte_pci_id *id_table;
 
 	for (id_table = dr->id_table; id_table->vendor_id != 0; id_table++) {
@@ -200,6 +196,20 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
 				id_table->subsystem_device_id != PCI_ANY_ID)
 			continue;
 
+		return 1;
+	}
+	return 0;
+}
+
+/*
+ * If vendor/device ID match, call the devinit() function of the
+ * driver.
+ */
+static int
+rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr,
+			     struct rte_pci_device *dev)
+{
+	int ret;
 		struct rte_pci_addr *loc = &dev->addr;
 
 		RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
@@ -240,9 +250,6 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
 
 		/* call the driver devinit() function */
 		return dr->devinit(dr, dev);
-	}
-	/* return positive value if driver is not found */
-	return 1;
 }
 
 /*
@@ -253,27 +260,6 @@ static int
 rte_eal_pci_detach_dev(struct rte_pci_driver *dr,
 		struct rte_pci_device *dev)
 {
-	const 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",
@@ -294,10 +280,6 @@ rte_eal_pci_detach_dev(struct rte_pci_driver *dr,
 			pci_unmap_device(dev);
 
 		return 0;
-	}
-
-	/* return positive value if driver is not found */
-	return 1;
 }
 
 /*
@@ -311,16 +293,16 @@ pci_probe_all_drivers(struct rte_pci_device *dev)
 	struct rte_pci_driver *dr = NULL;
 	int rc = 0;
 
-	if (dev == NULL)
-		return -1;
-
 	TAILQ_FOREACH(dr, &pci_driver_list, next) {
+		if (!pci_driver_supports_device(dr, dev))
+			continue;
+
 		rc = rte_eal_pci_probe_one_driver(dr, dev);
 		if (rc < 0)
 			/* negative value is an error */
 			return -1;
 		if (rc > 0)
-			/* positive value means driver not found */
+			/* positive value means device is blacklisted */
 			continue;
 		return 0;
 	}
@@ -338,17 +320,14 @@ pci_detach_all_drivers(struct rte_pci_device *dev)
 	struct rte_pci_driver *dr = NULL;
 	int rc = 0;
 
-	if (dev == NULL)
-		return -1;
-
 	TAILQ_FOREACH(dr, &pci_driver_list, next) {
+		if (!pci_driver_supports_device(dr, dev))
+			continue;
+
 		rc = rte_eal_pci_detach_dev(dr, dev);
 		if (rc < 0)
 			/* negative value is an error */
 			return -1;
-		if (rc > 0)
-			/* positive value means driver not found */
-			continue;
 		return 0;
 	}
 	return 1;
-- 
1.9.1

[PATCH 6/9] pci: cosmetic change

[David Marchand]

Indent previous commit, rename functions (internal and local functions do
not need rte_eal_ prefix).

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_eal/common/eal_common_pci.c | 106 ++++++++++++++++-----------------
 1 file changed, 52 insertions(+), 54 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index 44549f7..769c42e 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -206,50 +206,49 @@ pci_driver_supports_device(const struct rte_pci_driver *dr,
  * driver.
  */
 static int
-rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr,
-			     struct rte_pci_device *dev)
+pci_probe_device(struct rte_pci_driver *dr, struct rte_pci_device *dev)
 {
 	int ret;
-		struct rte_pci_addr *loc = &dev->addr;
+	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, "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, "  probe driver: %x:%x %s\n", dev->id.vendor_id,
-				dev->id.device_id, dr->name);
+	RTE_LOG(DEBUG, EAL, "  probe driver: %x:%x %s\n", dev->id.vendor_id,
+		dev->id.device_id, dr->name);
 
-		/* no initialization when blacklisted, return without error */
-		if (dev->devargs != NULL &&
-			dev->devargs->type == RTE_DEVTYPE_BLACKLISTED_PCI) {
-			RTE_LOG(DEBUG, EAL, "  Device is blacklisted, not initializing\n");
-			return 1;
-		}
+	/* no initialization when blacklisted, return without error */
+	if (dev->devargs != NULL &&
+		dev->devargs->type == RTE_DEVTYPE_BLACKLISTED_PCI) {
+		RTE_LOG(DEBUG, EAL, "  Device is blacklisted, not initializing\n");
+		return 1;
+	}
 
-		if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
+	if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
 #ifdef RTE_PCI_CONFIG
-			/*
-			 * Set PCIe config space for high performance.
-			 * Return value can be ignored.
-			 */
-			pci_config_space_set(dev);
+		/*
+		 * Set PCIe config space for high performance.
+		 * Return value can be ignored.
+		 */
+		pci_config_space_set(dev);
 #endif
-			/* map resources for devices that use igb_uio */
-			ret = pci_map_device(dev);
-			if (ret != 0)
-				return ret;
-		} else if (dr->drv_flags & RTE_PCI_DRV_FORCE_UNBIND &&
-				rte_eal_process_type() == RTE_PROC_PRIMARY) {
-			/* unbind current driver */
-			if (pci_unbind_kernel_driver(dev) < 0)
-				return -1;
-		}
-
-		/* reference driver structure */
-		dev->driver = dr;
-
-		/* call the driver devinit() function */
-		return dr->devinit(dr, dev);
+		/* map resources for devices that use igb_uio */
+		ret = pci_map_device(dev);
+		if (ret != 0)
+			return ret;
+	} else if (dr->drv_flags & RTE_PCI_DRV_FORCE_UNBIND &&
+		   rte_eal_process_type() == RTE_PROC_PRIMARY) {
+		/* unbind current driver */
+		if (pci_unbind_kernel_driver(dev) < 0)
+			return -1;
+	}
+
+	/* reference driver structure */
+	dev->driver = dr;
+
+	/* call the driver devinit() function */
+	return dr->devinit(dr, dev);
 }
 
 /*
@@ -257,29 +256,28 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr,
  * driver.
  */
 static int
-rte_eal_pci_detach_dev(struct rte_pci_driver *dr,
-		struct rte_pci_device *dev)
+pci_detach_device(struct rte_pci_driver *dr, struct rte_pci_device *dev)
 {
-		struct rte_pci_addr *loc = &dev->addr;
+	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, "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);
+	RTE_LOG(DEBUG, EAL, "  remove driver: %x:%x %s\n", dev->id.vendor_id,
+		dev->id.device_id, dr->name);
 
-		if (dr->devuninit && (dr->devuninit(dev) < 0))
-			return -1;	/* negative value is an error */
+	if (dr->devuninit && (dr->devuninit(dev) < 0))
+		return -1;	/* negative value is an error */
 
-		/* clear driver structure */
-		dev->driver = NULL;
+	/* 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);
+	if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
+		/* unmap resources for devices that use igb_uio */
+		pci_unmap_device(dev);
 
-		return 0;
+	return 0;
 }
 
 /*
@@ -297,7 +295,7 @@ pci_probe_all_drivers(struct rte_pci_device *dev)
 		if (!pci_driver_supports_device(dr, dev))
 			continue;
 
-		rc = rte_eal_pci_probe_one_driver(dr, dev);
+		rc = pci_probe_device(dr, dev);
 		if (rc < 0)
 			/* negative value is an error */
 			return -1;
@@ -324,7 +322,7 @@ pci_detach_all_drivers(struct rte_pci_device *dev)
 		if (!pci_driver_supports_device(dr, dev))
 			continue;
 
-		rc = rte_eal_pci_detach_dev(dr, dev);
+		rc = pci_detach_device(dr, dev);
 		if (rc < 0)
 			/* negative value is an error */
 			return -1;
-- 
1.9.1

[PATCH 7/9] pci: factorize driver search

[David Marchand]

Same idea as a few commits before, no need to implement the same logic in
probe and detach functions.
Here, the driver matching is done by a helper, then probe / detach
functions are called respectively.

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_eal/common/eal_common_pci.c | 96 ++++++++++++----------------------
 1 file changed, 34 insertions(+), 62 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index 769c42e..a1efd57 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -280,55 +280,16 @@ pci_detach_device(struct rte_pci_driver *dr, struct rte_pci_device *dev)
 	return 0;
 }
 
-/*
- * 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)
-{
-	struct rte_pci_driver *dr = NULL;
-	int rc = 0;
-
-	TAILQ_FOREACH(dr, &pci_driver_list, next) {
-		if (!pci_driver_supports_device(dr, dev))
-			continue;
-
-		rc = pci_probe_device(dr, dev);
-		if (rc < 0)
-			/* negative value is an error */
-			return -1;
-		if (rc > 0)
-			/* positive value means device is blacklisted */
-			continue;
-		return 0;
-	}
-	return 1;
-}
-
-/*
- * If vendor/device ID match, call the devuninit() 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_detach_all_drivers(struct rte_pci_device *dev)
+static struct rte_pci_driver *
+pci_find_driver(struct rte_pci_device *dev)
 {
-	struct rte_pci_driver *dr = NULL;
-	int rc = 0;
+	struct rte_pci_driver *dr;
 
 	TAILQ_FOREACH(dr, &pci_driver_list, next) {
-		if (!pci_driver_supports_device(dr, dev))
-			continue;
-
-		rc = pci_detach_device(dr, dev);
-		if (rc < 0)
-			/* negative value is an error */
-			return -1;
-		return 0;
+		if (pci_driver_supports_device(dr, dev))
+			break;
 	}
-	return 1;
+	return dr;
 }
 
 /*
@@ -338,8 +299,8 @@ pci_detach_all_drivers(struct rte_pci_device *dev)
 int
 rte_eal_pci_probe_one(const struct rte_pci_addr *addr)
 {
-	struct rte_pci_device *dev = NULL;
-	int ret = 0;
+	struct rte_pci_device *dev;
+	struct rte_pci_driver *dr;
 
 	if (addr == NULL)
 		return -1;
@@ -348,8 +309,11 @@ rte_eal_pci_probe_one(const struct rte_pci_addr *addr)
 	if (!dev)
 		goto err_return;
 
-	ret = pci_probe_all_drivers(dev);
-	if (ret < 0)
+	dr = pci_find_driver(dev);
+	if (!dr)
+		goto err_return;
+
+	if (pci_probe_device(dr, dev) < 0)
 		goto err_return;
 
 	return 0;
@@ -367,8 +331,8 @@ err_return:
 int
 rte_eal_pci_detach(const struct rte_pci_addr *addr)
 {
-	struct rte_pci_device *dev = NULL;
-	int ret = 0;
+	struct rte_pci_device *dev;
+	struct rte_pci_driver *dr;
 
 	if (addr == NULL)
 		return -1;
@@ -377,8 +341,11 @@ rte_eal_pci_detach(const struct rte_pci_addr *addr)
 	if (!dev)
 		goto err_return;
 
-	ret = pci_detach_all_drivers(dev);
-	if (ret < 0)
+	dr = pci_find_driver(dev);
+	if (!dr)
+		goto err_return;
+
+	if (pci_detach_device(dr, dev) < 0)
 		goto err_return;
 
 	TAILQ_REMOVE(&pci_device_list, dev, next);
@@ -399,28 +366,33 @@ err_return:
 int
 rte_eal_pci_probe(void)
 {
-	struct rte_pci_device *dev = NULL;
+	struct rte_pci_device *dev;
+	struct rte_pci_driver *dr;
 	struct rte_devargs *devargs;
 	int probe_all = 0;
-	int ret = 0;
 
 	if (rte_eal_devargs_type_count(RTE_DEVTYPE_WHITELISTED_PCI) == 0)
 		probe_all = 1;
 
 	TAILQ_FOREACH(dev, &pci_device_list, next) {
 
+		/* no driver available */
+		dr = pci_find_driver(dev);
+		if (!dr)
+			continue;
+
 		/* set devargs in PCI structure */
 		devargs = pci_devargs_lookup(dev);
 		if (devargs != NULL)
 			dev->devargs = devargs;
 
-		/* probe all or only whitelisted devices */
-		if (probe_all)
-			ret = pci_probe_all_drivers(dev);
-		else if (devargs != NULL &&
-			devargs->type == RTE_DEVTYPE_WHITELISTED_PCI)
-			ret = pci_probe_all_drivers(dev);
-		if (ret < 0)
+		/* skip if not probing all and device is not whitelisted */
+		if (!probe_all &&
+		    (devargs == NULL ||
+		     devargs->type != RTE_DEVTYPE_WHITELISTED_PCI))
+			continue;
+
+		if (pci_probe_device(dr, dev) < 0)
 			rte_exit(EXIT_FAILURE, "Requested device " PCI_PRI_FMT
 				 " cannot be used\n", dev->addr.domain, dev->addr.bus,
 				 dev->addr.devid, dev->addr.function);
-- 
1.9.1

[PATCH 8/9] pci: remove driver lookup from detach

[David Marchand]

A device is linked to a driver at probe time.
When detaching, we should call this same driver detach() function and no
need for a driver lookup.

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_eal/common/eal_common_pci.c | 25 ++++++++++---------------
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index a1efd57..71222a5 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -256,16 +256,9 @@ pci_probe_device(struct rte_pci_driver *dr, struct rte_pci_device *dev)
  * driver.
  */
 static int
-pci_detach_device(struct rte_pci_driver *dr, struct rte_pci_device *dev)
+pci_detach_device(struct rte_pci_device *dev)
 {
-	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);
+	struct rte_pci_driver *dr = dev->driver;
 
 	if (dr->devuninit && (dr->devuninit(dev) < 0))
 		return -1;	/* negative value is an error */
@@ -332,20 +325,22 @@ int
 rte_eal_pci_detach(const struct rte_pci_addr *addr)
 {
 	struct rte_pci_device *dev;
-	struct rte_pci_driver *dr;
 
 	if (addr == NULL)
 		return -1;
 
 	dev = pci_find_device(addr);
-	if (!dev)
+	if (!dev || !dev->driver)
 		goto err_return;
 
-	dr = pci_find_driver(dev);
-	if (!dr)
-		goto err_return;
+	RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
+		dev->addr.domain, dev->addr.bus, dev->addr.devid,
+		dev->addr.function, dev->numa_node);
+
+	RTE_LOG(DEBUG, EAL, "  remove driver: %x:%x %s\n", dev->id.vendor_id,
+		dev->id.device_id, dev->driver->name);
 
-	if (pci_detach_device(dr, dev) < 0)
+	if (pci_detach_device(dev) < 0)
 		goto err_return;
 
 	TAILQ_REMOVE(&pci_device_list, dev, next);
-- 
1.9.1

[PATCH 9/9] pci: blacklist only in global probe function

[David Marchand]

Move blacklist evaluation to rte_eal_pci_probe().
This way, rte_eal_pci_probe_one() (used by hotplug when attaching) now
accepts to probe devices that were blacklisted initially.

A new debug trace is added when skipping a device not part of a given
whitelist.

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_eal/common/eal_common_pci.c | 56 ++++++++++++++++++++--------------
 1 file changed, 33 insertions(+), 23 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index 71222a5..52d4594 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -209,21 +209,6 @@ static int
 pci_probe_device(struct rte_pci_driver *dr, struct rte_pci_device *dev)
 {
 	int ret;
-	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, "  probe driver: %x:%x %s\n", dev->id.vendor_id,
-		dev->id.device_id, dr->name);
-
-	/* no initialization when blacklisted, return without error */
-	if (dev->devargs != NULL &&
-		dev->devargs->type == RTE_DEVTYPE_BLACKLISTED_PCI) {
-		RTE_LOG(DEBUG, EAL, "  Device is blacklisted, not initializing\n");
-		return 1;
-	}
 
 	if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
 #ifdef RTE_PCI_CONFIG
@@ -306,6 +291,14 @@ rte_eal_pci_probe_one(const struct rte_pci_addr *addr)
 	if (!dr)
 		goto err_return;
 
+	RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
+		dev->addr.domain, dev->addr.bus,
+		dev->addr.devid, dev->addr.function,
+		dev->numa_node);
+
+	RTE_LOG(DEBUG, EAL, "  probe driver: %x:%x %s\n", dev->id.vendor_id,
+		dev->id.device_id, dr->name);
+
 	if (pci_probe_device(dr, dev) < 0)
 		goto err_return;
 
@@ -364,10 +357,8 @@ rte_eal_pci_probe(void)
 	struct rte_pci_device *dev;
 	struct rte_pci_driver *dr;
 	struct rte_devargs *devargs;
-	int probe_all = 0;
-
-	if (rte_eal_devargs_type_count(RTE_DEVTYPE_WHITELISTED_PCI) == 0)
-		probe_all = 1;
+	int whitelist = rte_eal_devargs_type_count(RTE_DEVTYPE_WHITELISTED_PCI);
+	int blacklist = rte_eal_devargs_type_count(RTE_DEVTYPE_BLACKLISTED_PCI);
 
 	TAILQ_FOREACH(dev, &pci_device_list, next) {
 
@@ -381,11 +372,30 @@ rte_eal_pci_probe(void)
 		if (devargs != NULL)
 			dev->devargs = devargs;
 
-		/* skip if not probing all and device is not whitelisted */
-		if (!probe_all &&
-		    (devargs == NULL ||
-		     devargs->type != RTE_DEVTYPE_WHITELISTED_PCI))
+		RTE_LOG(DEBUG, EAL,
+			"PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
+			dev->addr.domain, dev->addr.bus,
+			dev->addr.devid, dev->addr.function,
+			dev->numa_node);
+
+		RTE_LOG(DEBUG, EAL, "  probe driver: %x:%x %s\n",
+			dev->id.vendor_id, dev->id.device_id, dr->name);
+
+		/*
+		 * We want to probe this device when:
+		 * - there is no whitelist/blacklist,
+		 * - there is a whitelist, and device is part of it,
+		 * - there is a blacklist, and device is not part of it.
+		 */
+		if (whitelist && !devargs) {
+			RTE_LOG(DEBUG, EAL, "  Device is not whitelisted, not initializing\n");
+			continue;
+		}
+
+		if (blacklist && devargs) {
+			RTE_LOG(DEBUG, EAL, "  Device is blacklisted, not initializing\n");
 			continue;
+		}
 
 		if (pci_probe_device(dr, dev) < 0)
 			rte_exit(EXIT_FAILURE, "Requested device " PCI_PRI_FMT
-- 
1.9.1

Re: [PATCH 0/9] pci cleanup and blacklist rework

[David Marchand]

On Fri, Jan 22, 2016 at 4:27 PM, David Marchand
<david.marchand@6wind.com> wrote:
> The 4th patch introduces a change in linux eal.
> Before, if a pci device was bound to no kernel driver, eal would set kdrv
> to "unknown". With this change, kdrv is set to "none".
> This might make it possible to avoid the old issue of virtio devices being
> used by dpdk while still bound to kernel driver reported by Franck B..
> I'll let virtio guys look at this.
> At the very least, it makes more sense to me.

Ok, actually, I had forgotten that Huawei had already sent a similar change [1].
So I suppose this patch commitlog is wrong, but the patch itself is
still worth for the cleanup.

Thomas, I suppose you will integrate Huawei patches first.
Then I will rebase and fix the commitlog.

[1] http://dpdk.org/dev/patchwork/patch/9718/


-- 
David Marchand

[PATCH v2 0/9] pci cleanup and blacklist rework

[David Marchand]

Before 2.2.0 release, while preparing for more changes in eal (and fixing
a problem reported by Roger M. [1]), I came up with this (part of) patchset
that tries to make the pci code more compact and easier to read.

I ended up introducing some hooks in the pci layer to customize pci
blacklist / whitelist handling and make it possible to automatically
bind / unbind pci devices to igb_uio (or equivalent) when attaching
a device.

I am still not really happy:
- the pci blacklist / whitelist makes me think we should let the
  application tell eal which resources to use and get rid of the
  unconditional pci scan code, which means removing rte_eal_pci_probe()
  from rte_eal_init(), and remove rte_eal_dev_init() for vdevs,
- the more I look at this, the more I think automatic bind / unbind for
  pci devices should be called from the pmd context. The drivers know best
  what they require and what they want to do with the resources passed by
  the eal (see the drv_flags / RTE_KDRV_NONE / rte_eal_pci_map_device stuff
  for virtio pmd).
  This behaviour would still be optional, on a per-device basis.

So, I think that these hooks are not that good of an idea and I kept
them private for now, but anyway, sending this for comments.


Changes since v1:
- split the initial patchset. This current patchset now depends on
  [2] sent separately which should be applied first,
- introduced hooks in pci common code,
- implemented automatic bind / unbind for "uio" pci devices


[1] http://dpdk.org/ml/archives/dev/2015-November/028140.html
[2] http://dpdk.org/ml/archives/dev/2016-January/032387.html

-- 
David Marchand

David Marchand (9):
  pci: add internal device list helpers
  pci/linux: minor cleanup
  pci/linux: rework sysfs parsing for driver
  pci: factorize probe/detach code
  pci: cosmetic change
  pci: factorize driver search
  pci: remove driver lookup from detach
  pci: implement blacklist using a hook
  pci: implement automatic bind/unbind

 lib/librte_eal/bsdapp/eal/eal_pci.c        |  59 ++--
 lib/librte_eal/common/eal_common_options.c |   8 +
 lib/librte_eal/common/eal_common_pci.c     | 438 ++++++++++++++++++-----------
 lib/librte_eal/common/eal_options.h        |   2 +
 lib/librte_eal/common/eal_private.h        |  64 +++++
 lib/librte_eal/common/include/rte_pci.h    |   7 +-
 lib/librte_eal/linuxapp/eal/eal_pci.c      | 301 ++++++++++++++++----
 7 files changed, 627 insertions(+), 252 deletions(-)

-- 
1.9.1

[PATCH v2 1/9] pci: add internal device list helpers

[David Marchand]

Remove duplicate code by moving this to helpers in common.

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
Changes since v1:
- moved helpers to eal_private.h

---
 lib/librte_eal/bsdapp/eal/eal_pci.c    | 34 ++++-----------
 lib/librte_eal/common/eal_common_pci.c | 76 ++++++++++++++++++++++++----------
 lib/librte_eal/common/eal_private.h    | 26 ++++++++++++
 lib/librte_eal/linuxapp/eal/eal_pci.c  | 34 +++++----------
 4 files changed, 99 insertions(+), 71 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
index 5dd89e3..e95249b 100644
--- a/lib/librte_eal/bsdapp/eal/eal_pci.c
+++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
@@ -247,6 +247,7 @@ static int
 pci_scan_one(int dev_pci_fd, struct pci_conf *conf)
 {
 	struct rte_pci_device *dev;
+	struct rte_pci_device *dev2;
 	struct pci_bar_io bar;
 	unsigned i, max;
 
@@ -311,32 +312,15 @@ pci_scan_one(int dev_pci_fd, struct pci_conf *conf)
 		dev->mem_resource[i].phys_addr = bar.pbi_base & ~((uint64_t)0xf);
 	}
 
-	/* device is valid, add in list (sorted) */
-	if (TAILQ_EMPTY(&pci_device_list)) {
-		TAILQ_INSERT_TAIL(&pci_device_list, dev, next);
-	}
+	dev2 = pci_find_device(&dev->addr);
+	if (!dev2)
+		pci_add_device(dev);
 	else {
-		struct rte_pci_device *dev2 = NULL;
-		int ret;
-
-		TAILQ_FOREACH(dev2, &pci_device_list, next) {
-			ret = rte_eal_compare_pci_addr(&dev->addr, &dev2->addr);
-			if (ret > 0)
-				continue;
-			else if (ret < 0) {
-				TAILQ_INSERT_BEFORE(dev2, dev, next);
-				return 0;
-			} else { /* already registered */
-				dev2->kdrv = dev->kdrv;
-				dev2->max_vfs = dev->max_vfs;
-				memmove(dev2->mem_resource,
-					dev->mem_resource,
-					sizeof(dev->mem_resource));
-				free(dev);
-				return 0;
-			}
-		}
-		TAILQ_INSERT_TAIL(&pci_device_list, dev, next);
+		dev2->kdrv = dev->kdrv;
+		dev2->max_vfs = dev->max_vfs;
+		memmove(dev2->mem_resource, dev->mem_resource,
+			sizeof(dev->mem_resource));
+		free(dev);
 	}
 
 	return 0;
diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index d9aa66b..c3a33c6 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -139,6 +139,41 @@ pci_unmap_resource(void *requested_addr, size_t size)
 				requested_addr);
 }
 
+struct rte_pci_device *
+pci_find_device(const struct rte_pci_addr *addr)
+{
+	struct rte_pci_device *dev;
+
+	TAILQ_FOREACH(dev, &pci_device_list, next) {
+		if (!rte_eal_compare_pci_addr(&dev->addr, addr))
+			break;
+	}
+
+	return dev;
+}
+
+int
+pci_add_device(struct rte_pci_device *dev)
+{
+	struct rte_pci_device *dev2;
+	int ret;
+
+	TAILQ_FOREACH(dev2, &pci_device_list, next) {
+		ret = rte_eal_compare_pci_addr(&dev->addr, &dev2->addr);
+		if (ret > 0)
+			continue;
+
+		if (ret == 0)
+			return -1;
+
+		TAILQ_INSERT_BEFORE(dev2, dev, next);
+		return 0;
+	}
+
+	TAILQ_INSERT_TAIL(&pci_device_list, dev, next);
+	return 0;
+}
+
 /*
  * If vendor/device ID match, call the devinit() function of the
  * driver.
@@ -337,16 +372,15 @@ rte_eal_pci_probe_one(const struct rte_pci_addr *addr)
 	if (pci_refresh_device(addr) < 0)
 		goto err_return;
 
-	TAILQ_FOREACH(dev, &pci_device_list, next) {
-		if (rte_eal_compare_pci_addr(&dev->addr, addr))
-			continue;
+	dev = pci_find_device(addr);
+	if (!dev)
+		goto err_return;
 
-		ret = pci_probe_all_drivers(dev);
-		if (ret < 0)
-			goto err_return;
-		return 0;
-	}
-	return -1;
+	ret = pci_probe_all_drivers(dev);
+	if (ret < 0)
+		goto err_return;
+
+	return 0;
 
 err_return:
 	RTE_LOG(WARNING, EAL,
@@ -367,23 +401,21 @@ rte_eal_pci_detach(const struct rte_pci_addr *addr)
 	if (addr == NULL)
 		return -1;
 
-	TAILQ_FOREACH(dev, &pci_device_list, next) {
-		if (rte_eal_compare_pci_addr(&dev->addr, addr))
-			continue;
+	dev = pci_find_device(addr);
+	if (!dev)
+		goto err_return;
 
-		ret = pci_detach_all_drivers(dev);
-		if (ret < 0)
-			goto err_return;
+	ret = pci_detach_all_drivers(dev);
+	if (ret < 0)
+		goto err_return;
 
-		TAILQ_REMOVE(&pci_device_list, dev, next);
-		return 0;
-	}
-	return -1;
+	TAILQ_REMOVE(&pci_device_list, dev, next);
+	return 0;
 
 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);
+	RTE_LOG(WARNING, EAL,
+		"Requested device " PCI_PRI_FMT " cannot be detached\n",
+		addr->domain, addr->bus, addr->devid, addr->function);
 	return -1;
 }
 
diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
index ed1903f..e44a448 100644
--- a/lib/librte_eal/common/eal_private.h
+++ b/lib/librte_eal/common/eal_private.h
@@ -155,6 +155,32 @@ struct rte_pci_driver;
 struct rte_pci_device;
 
 /**
+ * Add a pci device to sorted pci device list
+ *
+ * This function is private to EAL.
+ *
+ * @param dev
+ *	The PCI device object.
+ * @return
+ *   - 0 on success.
+ *   - negative on error.
+ */
+int pci_add_device(struct rte_pci_device *dev);
+
+/**
+ * Find a pci device object based on a PCI Bus-Device-Function address.
+ *
+ * This function is private to EAL.
+ *
+ * @param addr
+ *	The PCI Bus-Device-Function address to look for
+ * @return
+ *   - pointer to the object,
+ *   - NULL when not found.
+ */
+struct rte_pci_device *pci_find_device(const struct rte_pci_addr *addr);
+
+/**
  * Refresh a pci device object by asking the kernel for the latest information.
  *
  * This function is private to EAL.
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index 4fe8b60..d9d6de7 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -259,6 +259,7 @@ pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
 	char filename[PATH_MAX];
 	unsigned long tmp;
 	struct rte_pci_device *dev;
+	struct rte_pci_device *dev2;
 	char driver[PATH_MAX];
 	int ret;
 
@@ -364,30 +365,15 @@ pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
 	} else
 		dev->kdrv = RTE_KDRV_UNKNOWN;
 
-	/* device is valid, add in list (sorted) */
-	if (TAILQ_EMPTY(&pci_device_list)) {
-		TAILQ_INSERT_TAIL(&pci_device_list, dev, next);
-	} else {
-		struct rte_pci_device *dev2;
-		int ret;
-
-		TAILQ_FOREACH(dev2, &pci_device_list, next) {
-			ret = rte_eal_compare_pci_addr(&dev->addr, &dev2->addr);
-			if (ret > 0)
-				continue;
-
-			if (ret < 0) {
-				TAILQ_INSERT_BEFORE(dev2, dev, next);
-			} else { /* already registered */
-				dev2->kdrv = dev->kdrv;
-				dev2->max_vfs = dev->max_vfs;
-				memmove(dev2->mem_resource, dev->mem_resource,
-					sizeof(dev->mem_resource));
-				free(dev);
-			}
-			return 0;
-		}
-		TAILQ_INSERT_TAIL(&pci_device_list, dev, next);
+	dev2 = pci_find_device(&dev->addr);
+	if (!dev2)
+		pci_add_device(dev);
+	else {
+		dev2->kdrv = dev->kdrv;
+		dev2->max_vfs = dev->max_vfs;
+		memmove(dev2->mem_resource, dev->mem_resource,
+			sizeof(dev->mem_resource));
+		free(dev);
 	}
 
 	return 0;
-- 
1.9.1

[PATCH v2 2/9] pci/linux: minor cleanup

[David Marchand]

Those are the only fields that are explicitly set to 0 while the global
dev pointer is set to 0 a few lines before.

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_eal/linuxapp/eal/eal_pci.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index d9d6de7..6751b48 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -308,7 +308,6 @@ pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
 	dev->id.subsystem_device_id = (uint16_t)tmp;
 
 	/* get max_vfs */
-	dev->max_vfs = 0;
 	snprintf(filename, sizeof(filename), "%s/max_vfs", dirname);
 	if (!access(filename, F_OK) &&
 	    eal_parse_sysfs_value(filename, &tmp) == 0)
@@ -323,12 +322,8 @@ pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
 	}
 
 	/* get numa node */
-	snprintf(filename, sizeof(filename), "%s/numa_node",
-		 dirname);
-	if (access(filename, R_OK) != 0) {
-		/* if no NUMA support, set default to 0 */
-		dev->numa_node = 0;
-	} else {
+	snprintf(filename, sizeof(filename), "%s/numa_node", dirname);
+	if (!access(filename, R_OK)) {
 		if (eal_parse_sysfs_value(filename, &tmp) < 0) {
 			free(dev);
 			return -1;
-- 
1.9.1

[PATCH v2 3/9] pci/linux: rework sysfs parsing for driver

[David Marchand]

There is no use for pci_get_kernel_driver_by_path() apart recognising
kernel driver and fill kdrv field.

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
Changes since v1:
- updated the commitlog, Huawei already did the "unknown" -> "none"
  change, so this patch ends up just refactoring code

---
 lib/librte_eal/linuxapp/eal/eal_pci.c | 48 ++++++++++++++---------------------
 1 file changed, 19 insertions(+), 29 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index 6751b48..c3118fc 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -94,32 +94,37 @@ error:
 }
 
 static int
-pci_get_kernel_driver_by_path(const char *filename, char *dri_name)
+pci_parse_sysfs_driver(const char *filename, struct rte_pci_device *dev)
 {
 	int count;
 	char path[PATH_MAX];
 	char *name;
 
-	if (!filename || !dri_name)
-		return -1;
-
 	count = readlink(filename, path, PATH_MAX);
 	if (count >= PATH_MAX)
 		return -1;
 
-	/* For device does not have a driver */
-	if (count < 0)
-		return 1;
+	dev->kdrv = RTE_KDRV_NONE;
 
-	path[count] = '\0';
+	if (count > 0) {
+		dev->kdrv = RTE_KDRV_UNKNOWN;
 
-	name = strrchr(path, '/');
-	if (name) {
-		strncpy(dri_name, name + 1, strlen(name + 1) + 1);
-		return 0;
+		path[count] = '\0';
+		name = strrchr(path, '/');
+		if (name) {
+			name[0] = '\0';
+			name++;
+		}
+
+		if (!strcmp(name, "vfio-pci"))
+			dev->kdrv = RTE_KDRV_VFIO;
+		else if (!strcmp(name, "igb_uio"))
+			dev->kdrv = RTE_KDRV_IGB_UIO;
+		else if (!strcmp(name, "uio_pci_generic"))
+			dev->kdrv = RTE_KDRV_UIO_GENERIC;
 	}
 
-	return -1;
+	return 0;
 }
 
 /* Map pci device */
@@ -260,8 +265,6 @@ pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
 	unsigned long tmp;
 	struct rte_pci_device *dev;
 	struct rte_pci_device *dev2;
-	char driver[PATH_MAX];
-	int ret;
 
 	dev = malloc(sizeof(*dev));
 	if (dev == NULL)
@@ -341,25 +344,12 @@ pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
 
 	/* parse driver */
 	snprintf(filename, sizeof(filename), "%s/driver", dirname);
-	ret = pci_get_kernel_driver_by_path(filename, driver);
-	if (ret < 0) {
+	if (pci_parse_sysfs_driver(filename, dev) < 0) {
 		RTE_LOG(ERR, EAL, "Fail to get kernel driver\n");
 		free(dev);
 		return -1;
 	}
 
-	if (!ret) {
-		if (!strcmp(driver, "vfio-pci"))
-			dev->kdrv = RTE_KDRV_VFIO;
-		else if (!strcmp(driver, "igb_uio"))
-			dev->kdrv = RTE_KDRV_IGB_UIO;
-		else if (!strcmp(driver, "uio_pci_generic"))
-			dev->kdrv = RTE_KDRV_UIO_GENERIC;
-		else
-			dev->kdrv = RTE_KDRV_UNKNOWN;
-	} else
-		dev->kdrv = RTE_KDRV_UNKNOWN;
-
 	dev2 = pci_find_device(&dev->addr);
 	if (!dev2)
 		pci_add_device(dev);
-- 
1.9.1

[PATCH v2 4/9] pci: factorize probe/detach code

[David Marchand]

Move pci id matching to a helper and reuse it in probe and detach
functions.

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_eal/common/eal_common_pci.c | 67 ++++++++++++----------------------
 1 file changed, 23 insertions(+), 44 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index c3a33c6..a6791c1 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -174,14 +174,10 @@ pci_add_device(struct rte_pci_device *dev)
 	return 0;
 }
 
-/*
- * If vendor/device ID match, call the devinit() function of the
- * driver.
- */
 static int
-rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *dev)
+pci_driver_supports_device(const struct rte_pci_driver *dr,
+			   const struct rte_pci_device *dev)
 {
-	int ret;
 	const struct rte_pci_id *id_table;
 
 	for (id_table = dr->id_table; id_table->vendor_id != 0; id_table++) {
@@ -200,6 +196,20 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
 				id_table->subsystem_device_id != PCI_ANY_ID)
 			continue;
 
+		return 1;
+	}
+	return 0;
+}
+
+/*
+ * If vendor/device ID match, call the devinit() function of the
+ * driver.
+ */
+static int
+rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr,
+			     struct rte_pci_device *dev)
+{
+	int ret;
 		struct rte_pci_addr *loc = &dev->addr;
 
 		RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
@@ -240,9 +250,6 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
 
 		/* call the driver devinit() function */
 		return dr->devinit(dr, dev);
-	}
-	/* return positive value if driver is not found */
-	return 1;
 }
 
 /*
@@ -253,27 +260,6 @@ static int
 rte_eal_pci_detach_dev(struct rte_pci_driver *dr,
 		struct rte_pci_device *dev)
 {
-	const 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",
@@ -294,10 +280,6 @@ rte_eal_pci_detach_dev(struct rte_pci_driver *dr,
 			pci_unmap_device(dev);
 
 		return 0;
-	}
-
-	/* return positive value if driver is not found */
-	return 1;
 }
 
 /*
@@ -311,16 +293,16 @@ pci_probe_all_drivers(struct rte_pci_device *dev)
 	struct rte_pci_driver *dr = NULL;
 	int rc = 0;
 
-	if (dev == NULL)
-		return -1;
-
 	TAILQ_FOREACH(dr, &pci_driver_list, next) {
+		if (!pci_driver_supports_device(dr, dev))
+			continue;
+
 		rc = rte_eal_pci_probe_one_driver(dr, dev);
 		if (rc < 0)
 			/* negative value is an error */
 			return -1;
 		if (rc > 0)
-			/* positive value means driver not found */
+			/* positive value means device is blacklisted */
 			continue;
 		return 0;
 	}
@@ -338,17 +320,14 @@ pci_detach_all_drivers(struct rte_pci_device *dev)
 	struct rte_pci_driver *dr = NULL;
 	int rc = 0;
 
-	if (dev == NULL)
-		return -1;
-
 	TAILQ_FOREACH(dr, &pci_driver_list, next) {
+		if (!pci_driver_supports_device(dr, dev))
+			continue;
+
 		rc = rte_eal_pci_detach_dev(dr, dev);
 		if (rc < 0)
 			/* negative value is an error */
 			return -1;
-		if (rc > 0)
-			/* positive value means driver not found */
-			continue;
 		return 0;
 	}
 	return 1;
-- 
1.9.1

[PATCH v2 5/9] pci: cosmetic change

[David Marchand]

Indent previous commit, rename functions (internal and local functions do
not need rte_eal_ prefix).

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_eal/common/eal_common_pci.c | 106 ++++++++++++++++-----------------
 1 file changed, 52 insertions(+), 54 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index a6791c1..fd88305 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -206,50 +206,49 @@ pci_driver_supports_device(const struct rte_pci_driver *dr,
  * driver.
  */
 static int
-rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr,
-			     struct rte_pci_device *dev)
+pci_probe_device(struct rte_pci_driver *dr, struct rte_pci_device *dev)
 {
 	int ret;
-		struct rte_pci_addr *loc = &dev->addr;
+	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, "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, "  probe driver: %x:%x %s\n", dev->id.vendor_id,
-				dev->id.device_id, dr->name);
+	RTE_LOG(DEBUG, EAL, "  probe driver: %x:%x %s\n", dev->id.vendor_id,
+		dev->id.device_id, dr->name);
 
-		/* no initialization when blacklisted, return without error */
-		if (dev->devargs != NULL &&
-			dev->devargs->type == RTE_DEVTYPE_BLACKLISTED_PCI) {
-			RTE_LOG(DEBUG, EAL, "  Device is blacklisted, not initializing\n");
-			return 1;
-		}
+	/* no initialization when blacklisted, return without error */
+	if (dev->devargs != NULL &&
+		dev->devargs->type == RTE_DEVTYPE_BLACKLISTED_PCI) {
+		RTE_LOG(DEBUG, EAL, "  Device is blacklisted, not initializing\n");
+		return 1;
+	}
 
-		if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
+	if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
 #ifdef RTE_PCI_CONFIG
-			/*
-			 * Set PCIe config space for high performance.
-			 * Return value can be ignored.
-			 */
-			pci_config_space_set(dev);
+		/*
+		 * Set PCIe config space for high performance.
+		 * Return value can be ignored.
+		 */
+		pci_config_space_set(dev);
 #endif
-			/* map resources for devices that use igb_uio */
-			ret = pci_map_device(dev);
-			if (ret != 0)
-				return ret;
-		} else if (dr->drv_flags & RTE_PCI_DRV_FORCE_UNBIND &&
-				rte_eal_process_type() == RTE_PROC_PRIMARY) {
-			/* unbind current driver */
-			if (pci_unbind_kernel_driver(dev) < 0)
-				return -1;
-		}
-
-		/* reference driver structure */
-		dev->driver = dr;
-
-		/* call the driver devinit() function */
-		return dr->devinit(dr, dev);
+		/* map resources for devices that use igb_uio */
+		ret = pci_map_device(dev);
+		if (ret != 0)
+			return ret;
+	} else if (dr->drv_flags & RTE_PCI_DRV_FORCE_UNBIND &&
+		   rte_eal_process_type() == RTE_PROC_PRIMARY) {
+		/* unbind current driver */
+		if (pci_unbind_kernel_driver(dev) < 0)
+			return -1;
+	}
+
+	/* reference driver structure */
+	dev->driver = dr;
+
+	/* call the driver devinit() function */
+	return dr->devinit(dr, dev);
 }
 
 /*
@@ -257,29 +256,28 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr,
  * driver.
  */
 static int
-rte_eal_pci_detach_dev(struct rte_pci_driver *dr,
-		struct rte_pci_device *dev)
+pci_detach_device(struct rte_pci_driver *dr, struct rte_pci_device *dev)
 {
-		struct rte_pci_addr *loc = &dev->addr;
+	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, "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);
+	RTE_LOG(DEBUG, EAL, "  remove driver: %x:%x %s\n", dev->id.vendor_id,
+		dev->id.device_id, dr->name);
 
-		if (dr->devuninit && (dr->devuninit(dev) < 0))
-			return -1;	/* negative value is an error */
+	if (dr->devuninit && (dr->devuninit(dev) < 0))
+		return -1;	/* negative value is an error */
 
-		/* clear driver structure */
-		dev->driver = NULL;
+	/* 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);
+	if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
+		/* unmap resources for devices that use igb_uio */
+		pci_unmap_device(dev);
 
-		return 0;
+	return 0;
 }
 
 /*
@@ -297,7 +295,7 @@ pci_probe_all_drivers(struct rte_pci_device *dev)
 		if (!pci_driver_supports_device(dr, dev))
 			continue;
 
-		rc = rte_eal_pci_probe_one_driver(dr, dev);
+		rc = pci_probe_device(dr, dev);
 		if (rc < 0)
 			/* negative value is an error */
 			return -1;
@@ -324,7 +322,7 @@ pci_detach_all_drivers(struct rte_pci_device *dev)
 		if (!pci_driver_supports_device(dr, dev))
 			continue;
 
-		rc = rte_eal_pci_detach_dev(dr, dev);
+		rc = pci_detach_device(dr, dev);
 		if (rc < 0)
 			/* negative value is an error */
 			return -1;
-- 
1.9.1

[PATCH v2 6/9] pci: factorize driver search

[David Marchand]

Same idea as a few commits before, no need to implement the same logic in
probe and detach functions.
Here, the driver matching is done by a helper, then probe / detach
functions are called respectively.

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_eal/common/eal_common_pci.c | 96 ++++++++++++----------------------
 1 file changed, 34 insertions(+), 62 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index fd88305..768421a 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -280,55 +280,16 @@ pci_detach_device(struct rte_pci_driver *dr, struct rte_pci_device *dev)
 	return 0;
 }
 
-/*
- * 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)
-{
-	struct rte_pci_driver *dr = NULL;
-	int rc = 0;
-
-	TAILQ_FOREACH(dr, &pci_driver_list, next) {
-		if (!pci_driver_supports_device(dr, dev))
-			continue;
-
-		rc = pci_probe_device(dr, dev);
-		if (rc < 0)
-			/* negative value is an error */
-			return -1;
-		if (rc > 0)
-			/* positive value means device is blacklisted */
-			continue;
-		return 0;
-	}
-	return 1;
-}
-
-/*
- * If vendor/device ID match, call the devuninit() 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_detach_all_drivers(struct rte_pci_device *dev)
+static struct rte_pci_driver *
+pci_find_driver(struct rte_pci_device *dev)
 {
-	struct rte_pci_driver *dr = NULL;
-	int rc = 0;
+	struct rte_pci_driver *dr;
 
 	TAILQ_FOREACH(dr, &pci_driver_list, next) {
-		if (!pci_driver_supports_device(dr, dev))
-			continue;
-
-		rc = pci_detach_device(dr, dev);
-		if (rc < 0)
-			/* negative value is an error */
-			return -1;
-		return 0;
+		if (pci_driver_supports_device(dr, dev))
+			break;
 	}
-	return 1;
+	return dr;
 }
 
 /*
@@ -338,8 +299,8 @@ pci_detach_all_drivers(struct rte_pci_device *dev)
 int
 rte_eal_pci_probe_one(const struct rte_pci_addr *addr)
 {
-	struct rte_pci_device *dev = NULL;
-	int ret = 0;
+	struct rte_pci_device *dev;
+	struct rte_pci_driver *dr;
 
 	if (addr == NULL)
 		return -1;
@@ -353,8 +314,11 @@ rte_eal_pci_probe_one(const struct rte_pci_addr *addr)
 	if (!dev)
 		goto err_return;
 
-	ret = pci_probe_all_drivers(dev);
-	if (ret < 0)
+	dr = pci_find_driver(dev);
+	if (!dr)
+		goto err_return;
+
+	if (pci_probe_device(dr, dev) < 0)
 		goto err_return;
 
 	return 0;
@@ -372,8 +336,8 @@ err_return:
 int
 rte_eal_pci_detach(const struct rte_pci_addr *addr)
 {
-	struct rte_pci_device *dev = NULL;
-	int ret = 0;
+	struct rte_pci_device *dev;
+	struct rte_pci_driver *dr;
 
 	if (addr == NULL)
 		return -1;
@@ -382,8 +346,11 @@ rte_eal_pci_detach(const struct rte_pci_addr *addr)
 	if (!dev)
 		goto err_return;
 
-	ret = pci_detach_all_drivers(dev);
-	if (ret < 0)
+	dr = pci_find_driver(dev);
+	if (!dr)
+		goto err_return;
+
+	if (pci_detach_device(dr, dev) < 0)
 		goto err_return;
 
 	TAILQ_REMOVE(&pci_device_list, dev, next);
@@ -404,28 +371,33 @@ err_return:
 int
 rte_eal_pci_probe(void)
 {
-	struct rte_pci_device *dev = NULL;
+	struct rte_pci_device *dev;
+	struct rte_pci_driver *dr;
 	struct rte_devargs *devargs;
 	int probe_all = 0;
-	int ret = 0;
 
 	if (rte_eal_devargs_type_count(RTE_DEVTYPE_WHITELISTED_PCI) == 0)
 		probe_all = 1;
 
 	TAILQ_FOREACH(dev, &pci_device_list, next) {
 
+		/* no driver available */
+		dr = pci_find_driver(dev);
+		if (!dr)
+			continue;
+
 		/* set devargs in PCI structure */
 		devargs = pci_devargs_lookup(dev);
 		if (devargs != NULL)
 			dev->devargs = devargs;
 
-		/* probe all or only whitelisted devices */
-		if (probe_all)
-			ret = pci_probe_all_drivers(dev);
-		else if (devargs != NULL &&
-			devargs->type == RTE_DEVTYPE_WHITELISTED_PCI)
-			ret = pci_probe_all_drivers(dev);
-		if (ret < 0)
+		/* skip if not probing all and device is not whitelisted */
+		if (!probe_all &&
+		    (devargs == NULL ||
+		     devargs->type != RTE_DEVTYPE_WHITELISTED_PCI))
+			continue;
+
+		if (pci_probe_device(dr, dev) < 0)
 			rte_exit(EXIT_FAILURE, "Requested device " PCI_PRI_FMT
 				 " cannot be used\n", dev->addr.domain, dev->addr.bus,
 				 dev->addr.devid, dev->addr.function);
-- 
1.9.1

[PATCH v2 7/9] pci: remove driver lookup from detach

[David Marchand]

A device is linked to a driver at probe time.
When detaching, we should call this same driver detach() function and no
need for a driver lookup.

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
Changes since v1:
- moved some logs for consistency

---
 lib/librte_eal/common/eal_common_pci.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index 768421a..082eab8 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -256,13 +256,9 @@ pci_probe_device(struct rte_pci_driver *dr, struct rte_pci_device *dev)
  * driver.
  */
 static int
-pci_detach_device(struct rte_pci_driver *dr, struct rte_pci_device *dev)
+pci_detach_device(struct rte_pci_device *dev)
 {
-	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);
+	struct rte_pci_driver *dr = dev->driver;
 
 	RTE_LOG(DEBUG, EAL, "  remove driver: %x:%x %s\n", dev->id.vendor_id,
 		dev->id.device_id, dr->name);
@@ -337,20 +333,19 @@ int
 rte_eal_pci_detach(const struct rte_pci_addr *addr)
 {
 	struct rte_pci_device *dev;
-	struct rte_pci_driver *dr;
 
 	if (addr == NULL)
 		return -1;
 
 	dev = pci_find_device(addr);
-	if (!dev)
+	if (!dev || !dev->driver)
 		goto err_return;
 
-	dr = pci_find_driver(dev);
-	if (!dr)
-		goto err_return;
+	RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
+		dev->addr.domain, dev->addr.bus, dev->addr.devid,
+		dev->addr.function, dev->numa_node);
 
-	if (pci_detach_device(dr, dev) < 0)
+	if (pci_detach_device(dev) < 0)
 		goto err_return;
 
 	TAILQ_REMOVE(&pci_device_list, dev, next);
-- 
1.9.1

[PATCH v2 8/9] pci: implement blacklist using a hook

[David Marchand]

The idea is to prepare a generic hook system for all bus, but I am still
unsure if this approach will be the right one, hence, keeping this as
private for now.

Here, blacklisting policy is removed from pci scan code, making it possible
to override it later by the application (once the api is judged generic and
good enough).

With this, blacklist evaluation moves to rte_eal_pci_probe() only.
This way, rte_eal_pci_probe_one() (used by hotplug when attaching) now
accepts to probe devices that were blacklisted initially.

A new debug trace is added when skipping a device not part of a given
whitelist.

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_eal/common/eal_common_pci.c | 101 +++++++++++++++++++++++++--------
 1 file changed, 77 insertions(+), 24 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index 082eab8..4a0ec73 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -87,6 +87,62 @@ struct pci_driver_list pci_driver_list =
 struct pci_device_list pci_device_list =
 	TAILQ_HEAD_INITIALIZER(pci_device_list);
 
+enum rte_eal_pci_hook {
+	RTE_EAL_PCI_SCAN,
+};
+
+enum rte_eal_pci_hook_return {
+	RTE_EAL_PCI_HOOK_OK,
+	RTE_EAL_PCI_HOOK_ERROR,
+	RTE_EAL_PCI_HOOK_SKIP,
+};
+
+typedef int (rte_eal_pci_hook_t)(enum rte_eal_pci_hook,
+				 struct rte_pci_driver *,
+				 struct rte_pci_device *);
+
+static int
+blacklist_pci_hook(enum rte_eal_pci_hook h,
+		   struct rte_pci_driver *dr __rte_unused,
+		   struct rte_pci_device *dev)
+{
+	int ret;
+
+	switch (h) {
+	case RTE_EAL_PCI_SCAN:
+	{
+		int whitelist =
+			rte_eal_devargs_type_count(RTE_DEVTYPE_WHITELISTED_PCI);
+		int blacklist =
+			rte_eal_devargs_type_count(RTE_DEVTYPE_BLACKLISTED_PCI);
+
+		/*
+		 * We want to probe this device when:
+		 * - there is no whitelist/blacklist,
+		 * - there is a whitelist, and device is part of it,
+		 * - there is a blacklist, and device is not part of it.
+		 */
+		if (whitelist && !dev->devargs) {
+			RTE_LOG(DEBUG, EAL, "  Device is not whitelisted, not initializing\n");
+			ret = RTE_EAL_PCI_HOOK_SKIP;
+		} else if (blacklist && dev->devargs) {
+			RTE_LOG(DEBUG, EAL, "  Device is blacklisted, not initializing\n");
+			ret = RTE_EAL_PCI_HOOK_SKIP;
+		} else
+			ret = RTE_EAL_PCI_HOOK_OK;
+		break;
+	}
+	default:
+		/* nothing to do here, just say ok */
+		ret = RTE_EAL_PCI_HOOK_OK;
+		break;
+	}
+
+	return ret;
+}
+
+static rte_eal_pci_hook_t *pci_hook = &blacklist_pci_hook;
+
 static struct rte_devargs *pci_devargs_lookup(struct rte_pci_device *dev)
 {
 	struct rte_devargs *devargs;
@@ -209,22 +265,10 @@ static int
 pci_probe_device(struct rte_pci_driver *dr, struct rte_pci_device *dev)
 {
 	int ret;
-	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, "  probe driver: %x:%x %s\n", dev->id.vendor_id,
 		dev->id.device_id, dr->name);
 
-	/* no initialization when blacklisted, return without error */
-	if (dev->devargs != NULL &&
-		dev->devargs->type == RTE_DEVTYPE_BLACKLISTED_PCI) {
-		RTE_LOG(DEBUG, EAL, "  Device is blacklisted, not initializing\n");
-		return 1;
-	}
-
 	if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
 #ifdef RTE_PCI_CONFIG
 		/*
@@ -310,6 +354,11 @@ rte_eal_pci_probe_one(const struct rte_pci_addr *addr)
 	if (!dev)
 		goto err_return;
 
+	RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
+		dev->addr.domain, dev->addr.bus,
+		dev->addr.devid, dev->addr.function,
+		dev->numa_node);
+
 	dr = pci_find_driver(dev);
 	if (!dr)
 		goto err_return;
@@ -369,27 +418,31 @@ rte_eal_pci_probe(void)
 	struct rte_pci_device *dev;
 	struct rte_pci_driver *dr;
 	struct rte_devargs *devargs;
-	int probe_all = 0;
-
-	if (rte_eal_devargs_type_count(RTE_DEVTYPE_WHITELISTED_PCI) == 0)
-		probe_all = 1;
+	int ret;
 
 	TAILQ_FOREACH(dev, &pci_device_list, next) {
 
-		/* no driver available */
-		dr = pci_find_driver(dev);
-		if (!dr)
-			continue;
+		RTE_LOG(DEBUG, EAL,
+			"PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
+			dev->addr.domain, dev->addr.bus,
+			dev->addr.devid, dev->addr.function,
+			dev->numa_node);
 
 		/* set devargs in PCI structure */
 		devargs = pci_devargs_lookup(dev);
 		if (devargs != NULL)
 			dev->devargs = devargs;
 
-		/* skip if not probing all and device is not whitelisted */
-		if (!probe_all &&
-		    (devargs == NULL ||
-		     devargs->type != RTE_DEVTYPE_WHITELISTED_PCI))
+		ret = pci_hook(RTE_EAL_PCI_SCAN, NULL, dev);
+		if (ret != RTE_EAL_PCI_HOOK_OK) {
+			RTE_LOG(DEBUG, EAL, "  scan hook refused dev, err=%d\n",
+				ret);
+			continue;
+		}
+
+		/* no driver available */
+		dr = pci_find_driver(dev);
+		if (!dr)
 			continue;
 
 		if (pci_probe_device(dr, dev) < 0)
-- 
1.9.1

[PATCH v2 9/9] pci: implement automatic bind/unbind

[David Marchand]

Reuse pci hook to implement automatic bind / unbind.
The more I look at this, the more I think this should go to the PMDs
themselves (with options per devices to control this), with EAL offering
helpers to achieve this.

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_eal/bsdapp/eal/eal_pci.c        |  25 ++++
 lib/librte_eal/common/eal_common_options.c |   8 ++
 lib/librte_eal/common/eal_common_pci.c     |  79 +++++++++++
 lib/librte_eal/common/eal_options.h        |   2 +
 lib/librte_eal/common/eal_private.h        |  38 ++++++
 lib/librte_eal/common/include/rte_pci.h    |   7 +-
 lib/librte_eal/linuxapp/eal/eal_pci.c      | 210 +++++++++++++++++++++++++++++
 7 files changed, 367 insertions(+), 2 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
index e95249b..130f7e9 100644
--- a/lib/librte_eal/bsdapp/eal/eal_pci.c
+++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
@@ -91,6 +91,31 @@ pci_unbind_kernel_driver(struct rte_pci_device *dev __rte_unused)
 	return -ENOTSUP;
 }
 
+int
+pci_rebind_device(const struct rte_pci_device *dev __rte_unused,
+		  const char *driver __rte_unused)
+{
+	RTE_LOG(ERR, EAL, "Rebinding device to pci kernel drivers is not implemented for BSD\n");
+	return -ENOTSUP;
+}
+
+int
+pci_mapping_driver_bound(const struct rte_pci_device *dev)
+{
+	int ret;
+
+	switch (dev->kdrv) {
+	case RTE_KDRV_NIC_UIO:
+		ret = 1;
+		break;
+	default:
+		ret = 0;
+		break;
+	}
+
+	return ret;
+}
+
 /* Map pci device */
 int
 pci_map_device(struct rte_pci_device *dev)
diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 29942ea..b646abd 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -54,6 +54,7 @@
 #include "eal_internal_cfg.h"
 #include "eal_options.h"
 #include "eal_filesystem.h"
+#include "eal_private.h"
 
 #define BITS_PER_HEX 4
 
@@ -95,6 +96,7 @@ eal_long_options[] = {
 	{OPT_VFIO_INTR,         1, NULL, OPT_VFIO_INTR_NUM        },
 	{OPT_VMWARE_TSC_MAP,    0, NULL, OPT_VMWARE_TSC_MAP_NUM   },
 	{OPT_XEN_DOM0,          0, NULL, OPT_XEN_DOM0_NUM         },
+	{OPT_PCI_UIO_AUTOBIND,  1, NULL, OPT_PCI_UIO_AUTOBIND_NUM },
 	{0,                     0, NULL, 0                        }
 };
 
@@ -897,6 +899,10 @@ eal_parse_common_option(int opt, const char *optarg,
 		}
 		break;
 
+	case OPT_PCI_UIO_AUTOBIND_NUM:
+		pci_init_autobind(optarg);
+		break;
+
 	/* don't know what to do, leave this to caller */
 	default:
 		return 1;
@@ -1019,5 +1025,7 @@ eal_common_usage(void)
 	       "  --"OPT_NO_PCI"            Disable PCI\n"
 	       "  --"OPT_NO_HPET"           Disable HPET\n"
 	       "  --"OPT_NO_SHCONF"         No shared config (mmap'd files)\n"
+	       "  --"OPT_PCI_UIO_AUTOBIND"  Set default kernel driver to bind pci devices,\n"
+	       "                            when their associated pmd requires uio\n"
 	       "\n", RTE_MAX_LCORE);
 }
diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index 4a0ec73..04a2490 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -89,6 +89,8 @@ struct pci_device_list pci_device_list =
 
 enum rte_eal_pci_hook {
 	RTE_EAL_PCI_SCAN,
+	RTE_EAL_PCI_ATTACH,
+	RTE_EAL_PCI_DETACH,
 };
 
 enum rte_eal_pci_hook_return {
@@ -132,6 +134,8 @@ blacklist_pci_hook(enum rte_eal_pci_hook h,
 			ret = RTE_EAL_PCI_HOOK_OK;
 		break;
 	}
+	case RTE_EAL_PCI_ATTACH:
+	case RTE_EAL_PCI_DETACH:
 	default:
 		/* nothing to do here, just say ok */
 		ret = RTE_EAL_PCI_HOOK_OK;
@@ -141,6 +145,61 @@ blacklist_pci_hook(enum rte_eal_pci_hook h,
 	return ret;
 }
 
+static char *uio_default_driver;
+
+static int
+autobind_uio_pci_hook(enum rte_eal_pci_hook h,
+		      struct rte_pci_driver *dr,
+		      struct rte_pci_device *dev)
+{
+	int ret;
+
+	/* stack with blacklist_pci_hook */
+	ret = blacklist_pci_hook(h, dr, dev);
+	if (ret != RTE_EAL_PCI_HOOK_OK)
+		goto exit;
+
+	switch (h) {
+	case RTE_EAL_PCI_ATTACH:
+	{
+		/* either nothing needed, or already bound */
+		if (!(dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) ||
+		    pci_mapping_driver_bound(dev)) {
+			ret = RTE_EAL_PCI_HOOK_OK;
+			goto exit;
+		}
+
+		if (pci_rebind_device(dev, uio_default_driver) < 0 ||
+		    pci_refresh_device(&dev->addr) < 0)
+			ret = RTE_EAL_PCI_HOOK_ERROR;
+		else
+			ret = RTE_EAL_PCI_HOOK_OK;
+
+		break;
+	}
+	case RTE_EAL_PCI_DETACH:
+	{
+		if (!(dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)) {
+			ret = RTE_EAL_PCI_HOOK_OK;
+			goto exit;
+		}
+
+		pci_rebind_device(dev, "");
+		ret = RTE_EAL_PCI_HOOK_OK;
+
+		break;
+	}
+	case RTE_EAL_PCI_SCAN:
+	default:
+		/* nothing to do here, just say ok */
+		ret = RTE_EAL_PCI_HOOK_OK;
+		break;
+	}
+
+exit:
+	return ret;
+}
+
 static rte_eal_pci_hook_t *pci_hook = &blacklist_pci_hook;
 
 static struct rte_devargs *pci_devargs_lookup(struct rte_pci_device *dev)
@@ -269,6 +328,12 @@ pci_probe_device(struct rte_pci_driver *dr, struct rte_pci_device *dev)
 	RTE_LOG(DEBUG, EAL, "  probe driver: %x:%x %s\n", dev->id.vendor_id,
 		dev->id.device_id, dr->name);
 
+	ret = pci_hook(RTE_EAL_PCI_ATTACH, dr, dev);
+	if (ret != RTE_EAL_PCI_HOOK_OK) {
+		RTE_LOG(DEBUG, EAL, "  attach hook refused dev, err=%d\n", ret);
+		return -1;
+	}
+
 	if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
 #ifdef RTE_PCI_CONFIG
 		/*
@@ -332,6 +397,15 @@ pci_find_driver(struct rte_pci_device *dev)
 	return dr;
 }
 
+void pci_init_autobind(const char *name)
+{
+	if (uio_default_driver)
+		free(uio_default_driver);
+	uio_default_driver = strdup(name);
+	if (uio_default_driver)
+		pci_hook = &autobind_uio_pci_hook;
+}
+
 /*
  * Find the pci device specified by pci address, then invoke probe function of
  * the driver of the devive.
@@ -382,6 +456,7 @@ int
 rte_eal_pci_detach(const struct rte_pci_addr *addr)
 {
 	struct rte_pci_device *dev;
+	struct rte_pci_driver *dr;
 
 	if (addr == NULL)
 		return -1;
@@ -394,9 +469,13 @@ rte_eal_pci_detach(const struct rte_pci_addr *addr)
 		dev->addr.domain, dev->addr.bus, dev->addr.devid,
 		dev->addr.function, dev->numa_node);
 
+	dr = dev->driver;
+
 	if (pci_detach_device(dev) < 0)
 		goto err_return;
 
+	pci_hook(RTE_EAL_PCI_DETACH, dr, dev);
+
 	TAILQ_REMOVE(&pci_device_list, dev, next);
 	return 0;
 
diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
index a881c62..182bc3d 100644
--- a/lib/librte_eal/common/eal_options.h
+++ b/lib/librte_eal/common/eal_options.h
@@ -83,6 +83,8 @@ enum {
 	OPT_VMWARE_TSC_MAP_NUM,
 #define OPT_XEN_DOM0          "xen-dom0"
 	OPT_XEN_DOM0_NUM,
+#define OPT_PCI_UIO_AUTOBIND  "pci-uio-autobind"
+	OPT_PCI_UIO_AUTOBIND_NUM,
 	OPT_LONG_MAX_NUM
 };
 
diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
index e44a448..9019144 100644
--- a/lib/librte_eal/common/eal_private.h
+++ b/lib/librte_eal/common/eal_private.h
@@ -215,6 +215,44 @@ int pci_unbind_kernel_driver(struct rte_pci_device *dev);
 int pci_map_device(struct rte_pci_device *dev);
 
 /**
+ * Tell to the autobind system which driver to bind to.
+ *
+ * This function is private to EAL.
+ *
+ * @param name
+ *    The kernel driver to bind to.
+ */
+void pci_init_autobind(const char *name);
+
+/**
+ * Helper used by autobind system to check if device is already bound
+ * to an adequate driver.
+ *
+ * This function is private to EAL.
+ *
+ * @param dev
+ *	The PCI device object.
+ * @return
+ *   0 on success, positive if device is bound.
+ */
+int pci_mapping_driver_bound(const struct rte_pci_device *dev);
+
+/**
+ * Rebind a pci device to a kernel driver.
+ *
+ * This function is private to EAL.
+ *
+ * @param dev
+ *	The PCI device object.
+ * @param name
+ *    The kernel driver to bind to, passing an empty name tries to rebind
+ *    the device to original kernel driver.
+ * @return
+ *   0 on success, negative on error
+ */
+int pci_rebind_device(const struct rte_pci_device *dev, const char *name);
+
+/**
  * Unmap this device
  *
  * This function is private to EAL.
diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 9edd5f5..75cab50 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -90,8 +90,11 @@ TAILQ_HEAD(pci_driver_list, rte_pci_driver); /**< PCI drivers in D-linked Q. */
 extern struct pci_driver_list pci_driver_list; /**< Global list of PCI drivers. */
 extern struct pci_device_list pci_device_list; /**< Global list of PCI devices. */
 
-/** Pathname of PCI devices directory. */
-#define SYSFS_PCI_DEVICES "/sys/bus/pci/devices"
+/* FIXME: this is linux stuff, should not be in common/ */
+/** Pathname of PCI directories. */
+#define SYSFS_PCI "/sys/bus/pci"
+#define SYSFS_PCI_DEVICES SYSFS_PCI"/devices"
+#define SYSFS_PCI_DRIVERS SYSFS_PCI"/drivers"
 
 /** Formatting string for PCI device identifier: Ex: 0000:00:01.0 */
 #define PCI_PRI_FMT "%.4" PRIx16 ":%.2" PRIx8 ":%.2" PRIx8 ".%" PRIx8
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index c3118fc..e711eea 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -94,6 +94,197 @@ error:
 }
 
 static int
+pci_rebind_device_override(const struct rte_pci_device *dev, const char *driver)
+{
+	int n;
+	FILE *f = NULL;
+	char filename[PATH_MAX];
+	char buf[BUFSIZ];
+	const struct rte_pci_addr *loc = &dev->addr;
+
+	snprintf(filename, sizeof(filename),
+		 SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/driver_override",
+		 loc->domain, loc->bus, loc->devid, loc->function);
+
+	n = snprintf(buf, sizeof(buf), "%s\n", driver);
+	if (n < 0 || n >= (int)sizeof(buf)) {
+		RTE_LOG(ERR, EAL, "%s(): snprintf failed\n", __func__);
+		goto error;
+	}
+
+	f = fopen(filename, "w");
+	if (!f || fwrite(buf, n, 1, f) == 0) {
+		RTE_LOG(ERR, EAL, "%s(): could not write to %s\n",
+			__func__, filename);
+		goto error;
+	}
+
+	fclose(f);
+
+	snprintf(filename, sizeof(filename),
+		 SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/driver/unbind",
+		 loc->domain, loc->bus, loc->devid, loc->function);
+
+	n = snprintf(buf, sizeof(buf), PCI_PRI_FMT "\n",
+		     loc->domain, loc->bus, loc->devid, loc->function);
+	if (n < 0 || n >= (int)sizeof(buf)) {
+		RTE_LOG(ERR, EAL, "%s(): snprintf failed\n", __func__);
+		goto error;
+	}
+
+	f = fopen(filename, "w");
+	/* device might be bound to nothing ? */
+	if (f) {
+		if (fwrite(buf, n, 1, f) == 0) {
+			RTE_LOG(ERR, EAL, "%s(): could not write to %s\n",
+				__func__, filename);
+			goto error;
+		}
+		fclose(f);
+	}
+
+	snprintf(filename, sizeof(filename), SYSFS_PCI "/drivers_probe");
+
+	f = fopen(filename, "w");
+	if (!f || fwrite(buf, n, 1, f) == 0) {
+		RTE_LOG(ERR, EAL, "%s(): could not write to %s\n", __func__,
+			filename);
+		goto error;
+	}
+
+	fclose(f);
+
+	return 0;
+error:
+	if (f)
+		fclose(f);
+	return -1;
+}
+
+static int
+pci_rebind_device_legacy(const struct rte_pci_device *dev, const char *driver)
+{
+	int n;
+	FILE *f = NULL;
+	char filename[PATH_MAX];
+	char buf[BUFSIZ];
+	const struct rte_pci_addr *loc = &dev->addr;
+
+	if (driver[0] != '\0') {
+		snprintf(filename, sizeof(filename),
+			 SYSFS_PCI_DRIVERS "/%s/new_id", driver);
+
+		n = snprintf(buf, sizeof(buf), "%4.4x %4.4x\n",
+			     dev->id.vendor_id, dev->id.device_id);
+		if (n < 0 || n >= (int)sizeof(buf)) {
+			RTE_LOG(ERR, EAL, "%s(): snprintf failed\n", __func__);
+			goto error;
+		}
+
+		f = fopen(filename, "w");
+		if (!f || fwrite(buf, n, 1, f) == 0) {
+			RTE_LOG(ERR, EAL, "%s(): could not write to %s\n",
+				__func__, filename);
+			goto error;
+		}
+
+		fclose(f);
+	}
+
+	snprintf(filename, sizeof(filename),
+		 SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/driver/unbind",
+		 loc->domain, loc->bus, loc->devid, loc->function);
+
+	n = snprintf(buf, sizeof(buf), PCI_PRI_FMT "\n",
+		     loc->domain, loc->bus, loc->devid, loc->function);
+	if (n < 0 || n >= (int)sizeof(buf)) {
+		RTE_LOG(ERR, EAL, "%s(): snprintf failed\n", __func__);
+		goto error;
+	}
+
+	f = fopen(filename, "w");
+	/* device might be bound to nothing ? */
+	if (f) {
+		if (fwrite(buf, n, 1, f) == 0) {
+			RTE_LOG(ERR, EAL, "%s(): could not write to %s\n",
+				__func__, filename);
+			goto error;
+		}
+		fclose(f);
+	}
+
+	if (driver[0] != '\0') {
+		snprintf(filename, sizeof(filename),
+			 SYSFS_PCI_DRIVERS "/%s/bind", driver);
+
+		f = fopen(filename, "w");
+		if (!f || fwrite(buf, n, 1, f) == 0) {
+			RTE_LOG(ERR, EAL, "%s(): could not write to %s\n",
+				__func__, filename);
+			goto error;
+		}
+
+		fclose(f);
+
+		snprintf(filename, sizeof(filename),
+			 SYSFS_PCI_DRIVERS "/%s/remove_id", driver);
+
+		n = snprintf(buf, sizeof(buf), "%4.4x %4.4x\n",
+			     dev->id.vendor_id, dev->id.device_id);
+		if (n < 0 || n >= (int)sizeof(buf)) {
+			RTE_LOG(ERR, EAL, "%s(): snprintf failed\n", __func__);
+			goto error;
+		}
+
+		f = fopen(filename, "w");
+		if (!f || fwrite(buf, n, 1, f) == 0) {
+			RTE_LOG(ERR, EAL, "%s(): could not write to %s\n",
+				__func__, filename);
+			goto error;
+		}
+
+		fclose(f);
+	} else {
+		snprintf(filename, sizeof(filename),
+			 SYSFS_PCI "/drivers_probe");
+
+		f = fopen(filename, "w");
+		if (!f || fwrite(buf, n, 1, f) == 0) {
+			RTE_LOG(ERR, EAL, "%s(): could not write to %s\n",
+				__func__, filename);
+			goto error;
+		}
+
+		fclose(f);
+	}
+
+	return 0;
+error:
+	if (f)
+		fclose(f);
+	return -1;
+}
+
+int pci_rebind_device(const struct rte_pci_device *dev, const char *driver)
+{
+	FILE *f;
+	char filename[PATH_MAX];
+	const struct rte_pci_addr *loc = &dev->addr;
+
+	snprintf(filename, sizeof(filename),
+		 SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/driver_override",
+		 loc->domain, loc->bus, loc->devid, loc->function);
+
+	f = fopen(filename, "w");
+	if (f) {
+		fclose(f);
+		return pci_rebind_device_override(dev, driver);
+	} else {
+		return pci_rebind_device_legacy(dev, driver);
+	}
+}
+
+static int
 pci_parse_sysfs_driver(const char *filename, struct rte_pci_device *dev)
 {
 	int count;
@@ -127,6 +318,25 @@ pci_parse_sysfs_driver(const char *filename, struct rte_pci_device *dev)
 	return 0;
 }
 
+int
+pci_mapping_driver_bound(const struct rte_pci_device *dev)
+{
+	int ret;
+
+	switch (dev->kdrv) {
+	case RTE_KDRV_VFIO:
+	case RTE_KDRV_IGB_UIO:
+	case RTE_KDRV_UIO_GENERIC:
+		ret = 1;
+		break;
+	default:
+		ret = 0;
+		break;
+	}
+
+	return ret;
+}
+
 /* Map pci device */
 int
 pci_map_device(struct rte_pci_device *dev)
-- 
1.9.1

Re: [PATCH v2 9/9] pci: implement automatic bind/unbind

[Ivan Boule]

On 01/29/2016 03:49 PM, David Marchand wrote:
> Reuse pci hook to implement automatic bind / unbind.
> The more I look at this, the more I think this should go to the PMDs
> themselves (with options per devices to control this), with EAL offering
> helpers to achieve this.

This sounds good to me.
As a basic software design rule, the decisions of using a given resource 
and/or performing a given operation (the intelligent knowledge) should 
systematically be taken at the maximum possible software level.
Conversely, such high-level decisions should be offered the most 
appropriate low-level services to facilitate their implementation.
In the case we are considering here - the need to bind a PCI device - 
the Poll Mode Driver of that device is the only component which actually 
knows.
Conversely, the EAL layer is the best place where to implement the
OS-dependent and/or architecture-dependent device binding services to be
invoked by PMDs at PCI device probing time.

My 2 cents,
Ivan

-- 
Ivan Boule
6WIND Development Engineer

Re: [PATCH v2 0/9] pci cleanup and blacklist rework

[Jan Viktorin]

Hello David,

I am confused a bit. I started to review the "[PATCH 0/9] prepare for rte_device
/ rte_driver" series and then I've noticed there are 2 patch series having "pci:
no need for dynamic tailq init" patch there. But then, there is this v2 that does
not have this patch. What is the right one? What should I look at. Is related?

Regards
Jan

On Fri, 29 Jan 2016 15:49:04 +0100
David Marchand <david.marchand@6wind.com> wrote:

> Before 2.2.0 release, while preparing for more changes in eal (and fixing
> a problem reported by Roger M. [1]), I came up with this (part of) patchset
> that tries to make the pci code more compact and easier to read.
> 
> I ended up introducing some hooks in the pci layer to customize pci
> blacklist / whitelist handling and make it possible to automatically
> bind / unbind pci devices to igb_uio (or equivalent) when attaching
> a device.
> 
> I am still not really happy:
> - the pci blacklist / whitelist makes me think we should let the
>   application tell eal which resources to use and get rid of the
>   unconditional pci scan code, which means removing rte_eal_pci_probe()
>   from rte_eal_init(), and remove rte_eal_dev_init() for vdevs,
> - the more I look at this, the more I think automatic bind / unbind for
>   pci devices should be called from the pmd context. The drivers know best
>   what they require and what they want to do with the resources passed by
>   the eal (see the drv_flags / RTE_KDRV_NONE / rte_eal_pci_map_device stuff
>   for virtio pmd).
>   This behaviour would still be optional, on a per-device basis.
> 
> So, I think that these hooks are not that good of an idea and I kept
> them private for now, but anyway, sending this for comments.
> 
> 
> Changes since v1:
> - split the initial patchset. This current patchset now depends on
>   [2] sent separately which should be applied first,
> - introduced hooks in pci common code,
> - implemented automatic bind / unbind for "uio" pci devices
> 
> 
> [1] http://dpdk.org/ml/archives/dev/2015-November/028140.html
> [2] http://dpdk.org/ml/archives/dev/2016-January/032387.html
> 



-- 
   Jan Viktorin                  E-mail: Viktorin@RehiveTech.com
   System Architect              Web:    www.RehiveTech.com
   RehiveTech
   Brno, Czech Republic

Re: [PATCH v2 0/9] pci cleanup and blacklist rework

[David Marchand]

Hello Jan,

On Mon, Feb 8, 2016 at 2:31 PM, Jan Viktorin <viktorin@rehivetech.com> wrote:
> I am confused a bit. I started to review the "[PATCH 0/9] prepare for rte_device
> / rte_driver" series and then I've noticed there are 2 patch series having "pci:
> no need for dynamic tailq init" patch there. But then, there is this v2 that does
> not have this patch. What is the right one? What should I look at. Is related?

I splitted this initial series, because I want to dissociate the work
trying to get to rte_driver/rte_device from other work that modifies
pci code (blacklist rework and "autobind").

I think you want to look at [1], but, if you want the global picture,
first apply [1], then [2] ([2] is the v2 patchset which supersedes the
initial patchset that had this "pci:
no need for dynamic tailq init" patch in it).

[1] http://dpdk.org/ml/archives/dev/2016-January/032387.html
[2] http://dpdk.org/ml/archives/dev/2016-January/032398.html


Regards,
-- 
David Marchand

Re: [PATCH v2 0/9] pci cleanup and blacklist rework

[Jan Viktorin]

On Fri, 29 Jan 2016 15:49:04 +0100
David Marchand <david.marchand@6wind.com> wrote:

> Before 2.2.0 release, while preparing for more changes in eal (and fixing
> a problem reported by Roger M. [1]), I came up with this (part of) patchset
> that tries to make the pci code more compact and easier to read.

Hello David,

what is the state of this series at the moment? Do you expect some more
reviews? I remember that I've sent some reviews but I don't think they
are integrated in the v2. By the way, there two patch series joined
into one or something like that, am I right?

Regards
Jan

> 
> I ended up introducing some hooks in the pci layer to customize pci
> blacklist / whitelist handling and make it possible to automatically
> bind / unbind pci devices to igb_uio (or equivalent) when attaching
> a device.
> 
> I am still not really happy:
> - the pci blacklist / whitelist makes me think we should let the
>   application tell eal which resources to use and get rid of the
>   unconditional pci scan code, which means removing rte_eal_pci_probe()
>   from rte_eal_init(), and remove rte_eal_dev_init() for vdevs,
> - the more I look at this, the more I think automatic bind / unbind for
>   pci devices should be called from the pmd context. The drivers know best
>   what they require and what they want to do with the resources passed by
>   the eal (see the drv_flags / RTE_KDRV_NONE / rte_eal_pci_map_device stuff
>   for virtio pmd).
>   This behaviour would still be optional, on a per-device basis.
> 
> So, I think that these hooks are not that good of an idea and I kept
> them private for now, but anyway, sending this for comments.
> 
> 
> Changes since v1:
> - split the initial patchset. This current patchset now depends on
>   [2] sent separately which should be applied first,
> - introduced hooks in pci common code,
> - implemented automatic bind / unbind for "uio" pci devices
> 
> 
> [1] http://dpdk.org/ml/archives/dev/2015-November/028140.html
> [2] http://dpdk.org/ml/archives/dev/2016-January/032387.html
> 



-- 
   Jan Viktorin                  E-mail: Viktorin@RehiveTech.com
   System Architect              Web:    www.RehiveTech.com
   RehiveTech
   Brno, Czech Republic

Re: [PATCH v2 0/9] pci cleanup and blacklist rework

[David Marchand]

Hello Jan,

On Wed, Mar 16, 2016 at 5:07 PM, Jan Viktorin <viktorin@rehivetech.com> wrote:
> On Fri, 29 Jan 2016 15:49:04 +0100
> David Marchand <david.marchand@6wind.com> wrote:
>
>> Before 2.2.0 release, while preparing for more changes in eal (and fixing
>> a problem reported by Roger M. [1]), I came up with this (part of) patchset
>> that tries to make the pci code more compact and easier to read.
>
> what is the state of this series at the moment? Do you expect some more
> reviews? I remember that I've sent some reviews but I don't think they
> are integrated in the v2. By the way, there two patch series joined
> into one or something like that, am I right?

Yes, I do remember your comments.
I have them in my tree, but this series still needs more work.
Trying to come up with something this week.


Regards,
-- 
David Marchand