[dpdk-dev] [PATCH 0/3] Enable uio_pci_generic support

[dpdk-dev] [PATCH 0/3] Enable uio_pci_generic support

[Danny Zhou]

Linux kernel provides UIO as well as VFIO mechanism to support writing user
space device driver. Comparing to UIO which is available since 2.6.32 kernel,
the VFIO is introduced into kernel since version 3.6.0 with better interrupt
and memory protection (build on top of Intel VT-d technology) supports.
Basically, UIO and VFIO do two common things below:
1) Map PCIe device's I/O memory space to user space driver
2) Support device interrupt notification mechanism that notifies user space
   driver/application when a device interrupt triggers.

To run an DPDK application and make use of VFIO, two in_kernel modules
vfio and vfio-pci module must be loaded. To use UIO, a DPDK kernel
module igb_uio, which was there since DPDK is invented, must be loaded to
attach to in_kernel uio module. As an solution to deprecate igb_uio, 
this patch serials leverage the uio_pci_generic in_kernel module to support
DPDK user space PMD in a generic fashion (similar to how VFIO works), to
remove user space DPDK dependency on GPL code igb_uio in kernel. 

Example to bind Network Ports to uio_pci_generic:
	modprobe uio
	modprobe uio_pci_generic
	/* to bind device 08:00.0, to the uio_pci_generic driver */
	./tools/dpdk_nic_bind.py -b uio_pci_generic 08:00.0

Note: this patch set does not remove igb_uio support due to igb_uio supports
creating maximum number of SR-IOV VFs (Virtual Functions) by using max_vfs 
kernel parameter on older kernels (kernel 3.7.x and below).
Specifically, igb_uio explicitly calls pci_enable_sriov() to create VFs, while
it is not invoked in either uio or uio_pci_generic kernel modules. On kernel 3.8.x
and above, user can use the standard sysfs to enable VFs. For examples:

#echo $num_vf_enabled > /sys/class/net/$dev/device/sriov_numvfs   //enable VFs
#echo 0 > /sys/class/net/$dev/device/sriov_numvfs                 //disable VFs

Danny Zhou (3):
  eal: add interrupt enable/disable routines for uio_pci_generic
  eal: enable uio_pci_generic support
  tools: enable binding NIC device to uio_pci_generic

 lib/librte_eal/common/include/rte_pci.h            |   1 +
 lib/librte_eal/linuxapp/eal/eal_interrupts.c       |  68 +++++--
 lib/librte_eal/linuxapp/eal/eal_pci.c              |   2 +-
 lib/librte_eal/linuxapp/eal/eal_pci_uio.c          | 202 +++++++++++----------
 .../linuxapp/eal/include/exec-env/rte_interrupts.h |  10 +-
 tools/dpdk_nic_bind.py                             |   2 +-
 6 files changed, 172 insertions(+), 113 deletions(-)

-- 
1.8.1.4

[dpdk-dev] [PATCH v1 1/3] eal: enable uio_pci_generic support

[Danny Zhou]

1) Unify procedure to retrieve BAR resource mapping information.  
2) Setup bus master bit in NIC's PCIe configuration space for uio_pci_generic.

Signed-off-by: Danny Zhou <danny.zhou@intel.com>
Tested-by: Qun Wan <qun.wan@intel.com>
---
 lib/librte_eal/common/include/rte_pci.h            |   1 +
 lib/librte_eal/linuxapp/eal/eal_pci.c              |   2 +-
 lib/librte_eal/linuxapp/eal/eal_pci_uio.c          | 202 +++++++++++----------
 .../linuxapp/eal/include/exec-env/rte_interrupts.h |  10 +-
 4 files changed, 119 insertions(+), 96 deletions(-)

diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 66ed793..71ca882 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -148,6 +148,7 @@ struct rte_pci_device {
 	struct rte_pci_id id;                   /**< PCI ID. */
 	struct rte_pci_resource mem_resource[PCI_MAX_RESOURCE];   /**< PCI Memory Resource */
 	struct rte_intr_handle intr_handle;     /**< Interrupt handle */
+	char driver_name[BUFSIZ];               /**< driver name */
 	const struct rte_pci_driver *driver;    /**< Associated driver */
 	uint16_t max_vfs;                       /**< sriov enable if not zero */
 	int numa_node;                          /**< NUMA node connection */
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index b5f5410..8c307e9 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -512,7 +512,7 @@ pci_map_device(struct rte_pci_device *dev)
 			return ret;
 	}
 #endif
-	/* map resources for devices that use igb_uio */
+	/* map resources for devices that use uio_pci_generic or igb_uio */
 	if (!mapped) {
 		ret = pci_uio_map_resource(dev);
 		if (ret != 0)
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
index e53f06b..0f0d4e0 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
@@ -36,6 +36,7 @@
 #include <dirent.h>
 #include <sys/stat.h>
 #include <sys/mman.h>
+#include <linux/pci_regs.h>
 
 #include <rte_log.h>
 #include <rte_pci.h>
@@ -47,71 +48,71 @@
 #include "eal_filesystem.h"
 #include "eal_pci_init.h"
 
-static int pci_parse_sysfs_value(const char *filename, uint64_t *val);
+#define IORESOURCE_MEM        0x00000200
+#define UIO_PCI_GENERIC_NAME  "uio_pci_generic"
 
 void *pci_map_addr = NULL;
 
 
 #define OFF_MAX              ((uint64_t)(off_t)-1)
 static int
-pci_uio_get_mappings(const char *devname, struct pci_map maps[], int nb_maps)
+pci_uio_get_mappings(struct rte_pci_device *dev, struct pci_map maps[], int nb_maps)
 {
-	int i;
-	char dirname[PATH_MAX];
+	struct rte_pci_addr *loc = &dev->addr;
+        int i = 0;
 	char filename[PATH_MAX];
-	uint64_t offset, size;
-
-	for (i = 0; i != nb_maps; i++) {
+	unsigned long long start_addr, end_addr, flags;
+	FILE *f;
 
-		/* check if map directory exists */
-		snprintf(dirname, sizeof(dirname),
-			"%s/maps/map%u", devname, i);
+	snprintf(filename, sizeof(filename),
+		SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/resource",
+		loc->domain, loc->bus, loc->devid, loc->function);
 
-		if (access(dirname, F_OK) != 0)
-			break;
+	f = fopen(filename, "r");
+	if (f == NULL) {
+		RTE_LOG(ERR, EAL,
+		"%s(): cannot open sysfs %s\n",
+		__func__, filename);
+		return -1;
+	}
 
-		/* get mapping offset */
-		snprintf(filename, sizeof(filename),
-			"%s/offset", dirname);
-		if (pci_parse_sysfs_value(filename, &offset) < 0) {
-			RTE_LOG(ERR, EAL,
-				"%s(): cannot parse offset of %s\n",
-				__func__, dirname);
-			return -1;
+	while (fscanf(f, "%llx %llx %llx", &start_addr,
+			&end_addr, &flags) == 3 && i < nb_maps) {
+		if (flags & IORESOURCE_MEM) {
+			maps[i].offset = 0x0;
+			maps[i].size = end_addr - start_addr + 1;
+			maps[i].phaddr = start_addr;
+			i++;
 		}
+	}
+	fclose(f);
 
-		/* get mapping size */
-		snprintf(filename, sizeof(filename),
-			"%s/size", dirname);
-		if (pci_parse_sysfs_value(filename, &size) < 0) {
-			RTE_LOG(ERR, EAL,
-				"%s(): cannot parse size of %s\n",
-				__func__, dirname);
-			return -1;
-		}
+	return i;
+}
 
-		/* get mapping physical address */
-		snprintf(filename, sizeof(filename),
-			"%s/addr", dirname);
-		if (pci_parse_sysfs_value(filename, &maps[i].phaddr) < 0) {
-			RTE_LOG(ERR, EAL,
-				"%s(): cannot parse addr of %s\n",
-				__func__, dirname);
-			return -1;
-		}
+static int
+pci_uio_set_bus_master(int dev_fd)
+{
+	uint16_t reg;
+	int ret;
 
-		if ((offset > OFF_MAX) || (size > SIZE_MAX)) {
-			RTE_LOG(ERR, EAL,
-				"%s(): offset/size exceed system max value\n",
-				__func__);
-			return -1;
-		}
+	ret = pread(dev_fd, &reg, sizeof(reg), PCI_COMMAND);
+	if (ret != sizeof(reg)) {
+		RTE_LOG(ERR, EAL,
+			"Cannot read command from PCI config space!\n");
+		return -1;
+	}
+
+	reg |= PCI_COMMAND_MASTER;
 
-		maps[i].offset = offset;
-		maps[i].size = size;
+	ret = pwrite(dev_fd, &reg, sizeof(reg), PCI_COMMAND);
+	if (ret != sizeof(reg)) {
+		RTE_LOG(ERR, EAL,
+			"Cannot write command to PCI config space!\n");
+		return -1;
 	}
 
-	return i;
+	return 0;
 }
 
 static int
@@ -213,6 +214,10 @@ pci_get_uio_dev(struct rte_pci_device *dev, char *dstbuf,
 	struct dirent *e;
 	DIR *dir;
 	char dirname[PATH_MAX];
+	FILE *f;
+	char fullpath[PATH_MAX];
+	char buf[BUFSIZ];
+	char *s;
 
 	/* depending on kernel version, uio can be located in uio/uioX
 	 * or uio:uioX */
@@ -268,6 +273,30 @@ pci_get_uio_dev(struct rte_pci_device *dev, char *dstbuf,
 	if (e == NULL)
 		return -1;
 
+	/* remember driver name of uio device */
+	snprintf(fullpath, sizeof(fullpath),
+			SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/uio/%s/name" ,
+			loc->domain, loc->bus, loc->devid,
+			loc->function, e->d_name);
+
+	f = fopen(fullpath, "r");
+	if (f == NULL) {
+		RTE_LOG(ERR, EAL,
+			"%s(): cannot open sysfs to get driver name\n",
+			__func__);
+		return -1;
+	}
+	s = fgets(buf, sizeof(buf), f);
+	if (s == NULL) {
+		RTE_LOG(ERR, EAL,
+			"%s(): cannot read sysfs to get driver name\n",
+			__func__);
+		fclose(f);
+		return -1;
+	}
+	strncpy(dev->driver_name, buf, sizeof(buf));
+	fclose(f);
+
 	/* create uio device if we've been asked to */
 	if (internal_config.create_uio_dev &&
 			pci_mknod_uio_dev(dstbuf, uio_num) < 0)
@@ -282,6 +311,7 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 {
 	int i, j;
 	char dirname[PATH_MAX];
+	char cfgname[PATH_MAX];
 	char devname[PATH_MAX]; /* contains the /dev/uioX */
 	void *mapaddr;
 	int uio_num;
@@ -294,6 +324,7 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 	struct pci_map *maps;
 
 	dev->intr_handle.fd = -1;
+	dev->intr_handle.uio_cfg_fd = -1;
 	dev->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN;
 
 	/* secondary processes - use already recorded details */
@@ -318,6 +349,33 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 	}
 	dev->intr_handle.type = RTE_INTR_HANDLE_UIO;
 
+	snprintf(cfgname, sizeof(cfgname),
+			"/sys/class/uio/uio%u/device/config", uio_num);
+	dev->intr_handle.uio_cfg_fd = open(cfgname, O_RDWR);
+	if (dev->intr_handle.uio_cfg_fd < 0) {
+		RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
+			cfgname, strerror(errno));
+		return -1;
+	}
+
+	/* handle Misc. stuff for uio_pci_generic  */
+	if (strncmp(dev->driver_name, UIO_PCI_GENERIC_NAME,
+			strlen(UIO_PCI_GENERIC_NAME)) == 0) {
+		/* update devname for uio_pci_generic  */
+		snprintf(devname, sizeof(devname),
+			SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/resource%d",
+			loc->domain, loc->bus, loc->devid, loc->function, 0);
+
+		/* set bus master that is not done by uio_pci_generic */
+		if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+			if (pci_uio_set_bus_master(dev->intr_handle.uio_cfg_fd)) {
+				RTE_LOG(ERR, EAL,
+					"Cannot set up bus mastering!\n");
+				return -1;
+			}
+		}
+	}
+
 	/* allocate the mapping details for secondary processes*/
 	uio_res = rte_zmalloc("UIO_RES", sizeof(*uio_res), 0);
 	if (uio_res == NULL) {
@@ -330,13 +388,12 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 	memcpy(&uio_res->pci_addr, &dev->addr, sizeof(uio_res->pci_addr));
 
 	/* collect info about device mappings */
-	nb_maps = pci_uio_get_mappings(dirname, uio_res->maps,
-				       RTE_DIM(uio_res->maps));
+	nb_maps = pci_uio_get_mappings(dev, uio_res->maps,
+					RTE_DIM(uio_res->maps));
 	if (nb_maps < 0) {
 		rte_free(uio_res);
 		return nb_maps;
 	}
-
 	uio_res->nb_maps = nb_maps;
 
 	/* Map all BARs */
@@ -374,16 +431,10 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 			if (maps[j].addr != NULL)
 				fail = 1;
 			else {
-				/* try mapping somewhere close to the end of hugepages */
-				if (pci_map_addr == NULL)
-					pci_map_addr = pci_find_max_end_va();
-
-				mapaddr = pci_map_resource(pci_map_addr, fd, (off_t)offset,
+				mapaddr = pci_map_resource(NULL, fd, (off_t)offset,
 						(size_t)maps[j].size);
-				if (mapaddr == MAP_FAILED)
+				if (mapaddr == NULL)
 					fail = 1;
-
-				pci_map_addr = RTE_PTR_ADD(mapaddr, (size_t) maps[j].size);
 			}
 
 			if (fail) {
@@ -403,38 +454,3 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 
 	return 0;
 }
-
-/*
- * parse a sysfs file containing one integer value
- * different to the eal version, as it needs to work with 64-bit values
- */
-static int
-pci_parse_sysfs_value(const char *filename, uint64_t *val)
-{
-	FILE *f;
-	char buf[BUFSIZ];
-	char *end = NULL;
-
-	f = fopen(filename, "r");
-	if (f == NULL) {
-		RTE_LOG(ERR, EAL, "%s(): cannot open sysfs value %s\n",
-				__func__, filename);
-		return -1;
-	}
-
-	if (fgets(buf, sizeof(buf), f) == NULL) {
-		RTE_LOG(ERR, EAL, "%s(): cannot read sysfs value %s\n",
-				__func__, filename);
-		fclose(f);
-		return -1;
-	}
-	*val = strtoull(buf, &end, 0);
-	if ((buf[0] == '\0') || (end == NULL) || (*end != '\n')) {
-		RTE_LOG(ERR, EAL, "%s(): cannot parse sysfs value %s\n",
-				__func__, filename);
-		fclose(f);
-		return -1;
-	}
-	fclose(f);
-	return 0;
-}
diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
index 23eafd9..adc1c8d 100644
--- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
+++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
@@ -50,8 +50,14 @@ enum rte_intr_handle_type {
 
 /** Handle for interrupts. */
 struct rte_intr_handle {
-	int vfio_dev_fd;                 /**< VFIO device file descriptor */
-	int fd;                          /**< file descriptor */
+	union {
+		int vfio_dev_fd;  /**< VFIO device file descriptor */
+	};
+        union {
+		int uio_cfg_fd;  /**< UIO config file descriptor
+					for uio_pci_generic */
+	};
+	int fd;	 /**< interrupt event file descriptor */
 	enum rte_intr_handle_type type;  /**< handle type */
 };
 
-- 
1.8.1.4

[dpdk-dev] [PATCH v1 2/3] eal: add interrupt enable/disable routines for uio_pci_generic

[Danny Zhou]

enable/disable interrupt by manipulating enable/disable bit of command register
on NIC's PCIe configuration space.

Signed-off-by: Danny Zhou <danny.zhou@intel.com>
Tested-by: Qun Wan <qun.wan@intel.com>
---
 lib/librte_eal/linuxapp/eal/eal_interrupts.c | 68 +++++++++++++++++++++-------
 1 file changed, 52 insertions(+), 16 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
index dc2668a..8c5b834 100644
--- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
+++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
@@ -361,6 +361,54 @@ vfio_disable_msix(struct rte_intr_handle *intr_handle) {
 }
 #endif
 
+static int
+uio_intr_disable(struct rte_intr_handle *intr_handle)
+{
+	unsigned char command_high;
+
+	/* use UIO config file descriptor for uio_pci_generic */
+	if (pread(intr_handle->uio_cfg_fd, &command_high, 1, 5) != 1) {
+		RTE_LOG(ERR, EAL,
+			"Error reading interrupts status for fd %d\n",
+			intr_handle->uio_cfg_fd);
+		return -1;
+	}
+	/* disable interrupts */
+	command_high |= 0x4;
+	if (pwrite(intr_handle->uio_cfg_fd, &command_high, 1, 5) != 1) {
+		RTE_LOG(ERR, EAL,
+			"Error disabling interrupts for fd %d\n",
+			intr_handle->uio_cfg_fd);
+		return -1;
+	}
+
+	return 0;
+}
+
+static int
+uio_intr_enable(struct rte_intr_handle *intr_handle)
+{
+	unsigned char command_high;
+
+	/* use UIO config file descriptor for uio_pci_generic */
+	if (pread(intr_handle->uio_cfg_fd, &command_high, 1, 5) != 1) {
+		RTE_LOG(ERR, EAL,
+			"Error reading interrupts status for fd %d\n",
+			intr_handle->uio_cfg_fd);
+		return -1;
+	}
+	/* enable interrupts */
+	command_high &= ~0x4;
+	if (pwrite(intr_handle->uio_cfg_fd, &command_high, 1, 5) != 1) {
+		RTE_LOG(ERR, EAL,
+			"Error enabling interrupts for fd %d\n",
+			intr_handle->uio_cfg_fd);
+		return -1;
+	}
+
+	return 0;
+}
+
 int
 rte_intr_callback_register(struct rte_intr_handle *intr_handle,
 			rte_intr_callback_fn cb, void *cb_arg)
@@ -500,20 +548,14 @@ rte_intr_callback_unregister(struct rte_intr_handle *intr_handle,
 int
 rte_intr_enable(struct rte_intr_handle *intr_handle)
 {
-	const int value = 1;
-
-	if (!intr_handle || intr_handle->fd < 0)
+	if (!intr_handle || intr_handle->fd < 0 || intr_handle->uio_cfg_fd < 0)
 		return -1;
 
 	switch (intr_handle->type){
 	/* write to the uio fd to enable the interrupt */
 	case RTE_INTR_HANDLE_UIO:
-		if (write(intr_handle->fd, &value, sizeof(value)) < 0) {
-			RTE_LOG(ERR, EAL,
-				"Error enabling interrupts for fd %d\n",
-							intr_handle->fd);
+		if (uio_intr_enable(intr_handle))
 			return -1;
-		}
 		break;
 	/* not used at this moment */
 	case RTE_INTR_HANDLE_ALARM:
@@ -546,20 +588,14 @@ rte_intr_enable(struct rte_intr_handle *intr_handle)
 int
 rte_intr_disable(struct rte_intr_handle *intr_handle)
 {
-	const int value = 0;
-
-	if (!intr_handle || intr_handle->fd < 0)
+	if (!intr_handle || intr_handle->fd < 0 || intr_handle->uio_cfg_fd < 0)
 		return -1;
 
 	switch (intr_handle->type){
 	/* write to the uio fd to disable the interrupt */
 	case RTE_INTR_HANDLE_UIO:
-		if (write(intr_handle->fd, &value, sizeof(value)) < 0){
-			RTE_LOG(ERR, EAL,
-				"Error disabling interrupts for fd %d\n",
-							intr_handle->fd);
+		if (uio_intr_disable(intr_handle))
 			return -1;
-		}
 		break;
 	/* not used at this moment */
 	case RTE_INTR_HANDLE_ALARM:
-- 
1.8.1.4

[dpdk-dev] [PATCH v1 3/3] tools: enable binding NIC device to uio_pci_generic

[Danny Zhou]

Add uio_pci_generic to the list of supported DPDK driver.

Signed-off-by: Danny Zhou <danny.zhou@intel.com>
Tested-by: Qun Wan <qun.wan@intel.com>
---
 tools/dpdk_nic_bind.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/dpdk_nic_bind.py b/tools/dpdk_nic_bind.py
index 812b6a1..2483056 100755
--- a/tools/dpdk_nic_bind.py
+++ b/tools/dpdk_nic_bind.py
@@ -43,7 +43,7 @@ ETHERNET_CLASS = "0200"
 # Each device within this is itself a dictionary of device properties
 devices = {}
 # list of supported DPDK drivers
-dpdk_drivers = [ "igb_uio", "vfio-pci" ]
+dpdk_drivers = [ "igb_uio", "vfio-pci", "uio_pci_generic" ]
 
 # command-line arg flags
 b_flag = None
-- 
1.8.1.4

Re: [dpdk-dev] [PATCH 0/3] Enable uio_pci_generic support

[Qiu, Michael]

On 1/29/2015 5:28 PM, Danny Zhou wrote:
> Linux kernel provides UIO as well as VFIO mechanism to support writing user
> space device driver. Comparing to UIO which is available since 2.6.32 kernel,
> the VFIO is introduced into kernel since version 3.6.0 with better interrupt
> and memory protection (build on top of Intel VT-d technology) supports.

I would like to say that, VFIO is not only for X86.

Thanks,
Michael
> Basically, UIO and VFIO do two common things below:
> 1) Map PCIe device's I/O memory space to user space driver
> 2) Support device interrupt notification mechanism that notifies user space
>    driver/application when a device interrupt triggers.
>
> To run an DPDK application and make use of VFIO, two in_kernel modules
> vfio and vfio-pci module must be loaded. To use UIO, a DPDK kernel
> module igb_uio, which was there since DPDK is invented, must be loaded to
> attach to in_kernel uio module. As an solution to deprecate igb_uio, 
> this patch serials leverage the uio_pci_generic in_kernel module to support
> DPDK user space PMD in a generic fashion (similar to how VFIO works), to
> remove user space DPDK dependency on GPL code igb_uio in kernel. 
>
> Example to bind Network Ports to uio_pci_generic:
> 	modprobe uio
> 	modprobe uio_pci_generic
> 	/* to bind device 08:00.0, to the uio_pci_generic driver */
> 	./tools/dpdk_nic_bind.py -b uio_pci_generic 08:00.0
>
> Note: this patch set does not remove igb_uio support due to igb_uio supports
> creating maximum number of SR-IOV VFs (Virtual Functions) by using max_vfs 
> kernel parameter on older kernels (kernel 3.7.x and below).
> Specifically, igb_uio explicitly calls pci_enable_sriov() to create VFs, while
> it is not invoked in either uio or uio_pci_generic kernel modules. On kernel 3.8.x
> and above, user can use the standard sysfs to enable VFs. For examples:
>
> #echo $num_vf_enabled > /sys/class/net/$dev/device/sriov_numvfs   //enable VFs
> #echo 0 > /sys/class/net/$dev/device/sriov_numvfs                 //disable VFs
>
> Danny Zhou (3):
>   eal: add interrupt enable/disable routines for uio_pci_generic
>   eal: enable uio_pci_generic support
>   tools: enable binding NIC device to uio_pci_generic
>
>  lib/librte_eal/common/include/rte_pci.h            |   1 +
>  lib/librte_eal/linuxapp/eal/eal_interrupts.c       |  68 +++++--
>  lib/librte_eal/linuxapp/eal/eal_pci.c              |   2 +-
>  lib/librte_eal/linuxapp/eal/eal_pci_uio.c          | 202 +++++++++++----------
>  .../linuxapp/eal/include/exec-env/rte_interrupts.h |  10 +-
>  tools/dpdk_nic_bind.py                             |   2 +-
>  6 files changed, 172 insertions(+), 113 deletions(-)
>

Re: [dpdk-dev] [PATCH 0/3] Enable uio_pci_generic support

[Bruce Richardson]

On Thu, Jan 29, 2015 at 05:28:16PM +0800, Danny Zhou wrote:
> Linux kernel provides UIO as well as VFIO mechanism to support writing user
> space device driver. Comparing to UIO which is available since 2.6.32 kernel,
> the VFIO is introduced into kernel since version 3.6.0 with better interrupt
> and memory protection (build on top of Intel VT-d technology) supports.
> Basically, UIO and VFIO do two common things below:
> 1) Map PCIe device's I/O memory space to user space driver
> 2) Support device interrupt notification mechanism that notifies user space
>    driver/application when a device interrupt triggers.
> 
> To run an DPDK application and make use of VFIO, two in_kernel modules
> vfio and vfio-pci module must be loaded. To use UIO, a DPDK kernel
> module igb_uio, which was there since DPDK is invented, must be loaded to
> attach to in_kernel uio module. As an solution to deprecate igb_uio, 
> this patch serials leverage the uio_pci_generic in_kernel module to support
> DPDK user space PMD in a generic fashion (similar to how VFIO works), to
> remove user space DPDK dependency on GPL code igb_uio in kernel. 
> 
> Example to bind Network Ports to uio_pci_generic:
> 	modprobe uio
> 	modprobe uio_pci_generic
> 	/* to bind device 08:00.0, to the uio_pci_generic driver */
> 	./tools/dpdk_nic_bind.py -b uio_pci_generic 08:00.0
> 
> Note: this patch set does not remove igb_uio support due to igb_uio supports
> creating maximum number of SR-IOV VFs (Virtual Functions) by using max_vfs 
> kernel parameter on older kernels (kernel 3.7.x and below).
> Specifically, igb_uio explicitly calls pci_enable_sriov() to create VFs, while
> it is not invoked in either uio or uio_pci_generic kernel modules. On kernel 3.8.x
> and above, user can use the standard sysfs to enable VFs. For examples:
> 
> #echo $num_vf_enabled > /sys/class/net/$dev/device/sriov_numvfs   //enable VFs
> #echo 0 > /sys/class/net/$dev/device/sriov_numvfs                 //disable VFs
> 
> Danny Zhou (3):
>   eal: add interrupt enable/disable routines for uio_pci_generic
>   eal: enable uio_pci_generic support
>   tools: enable binding NIC device to uio_pci_generic
> 
>  lib/librte_eal/common/include/rte_pci.h            |   1 +
>  lib/librte_eal/linuxapp/eal/eal_interrupts.c       |  68 +++++--
>  lib/librte_eal/linuxapp/eal/eal_pci.c              |   2 +-
>  lib/librte_eal/linuxapp/eal/eal_pci_uio.c          | 202 +++++++++++----------
>  .../linuxapp/eal/include/exec-env/rte_interrupts.h |  10 +-
>  tools/dpdk_nic_bind.py                             |   2 +-
>  6 files changed, 172 insertions(+), 113 deletions(-)
> 
> -- 
> 1.8.1.4
> 

Series
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [dpdk-dev] [PATCH v1 1/3] eal: enable uio_pci_generic support

[Thomas Monjalon]

Hi Danny,

I wanted to apply this patchset which was reviewed. But when having a quick
overview, I've seen some strange additions.

2015-01-29 17:28, Danny Zhou:
> 1) Unify procedure to retrieve BAR resource mapping information.  
> 2) Setup bus master bit in NIC's PCIe configuration space for uio_pci_generic.
> 
> Signed-off-by: Danny Zhou <danny.zhou@intel.com>
> Tested-by: Qun Wan <qun.wan@intel.com>
[...]
> --- a/lib/librte_eal/common/include/rte_pci.h
> +++ b/lib/librte_eal/common/include/rte_pci.h
> @@ -148,6 +148,7 @@ struct rte_pci_device {
>  	struct rte_pci_id id;                   /**< PCI ID. */
>  	struct rte_pci_resource mem_resource[PCI_MAX_RESOURCE];   /**< PCI Memory Resource */
>  	struct rte_intr_handle intr_handle;     /**< Interrupt handle */
> +	char driver_name[BUFSIZ];               /**< driver name */

Why not embedding this field in driver struct?
The name and comment should be more precise.
There is also driver->name and hotplug patchset is adding a kernel driver name.
Please bring the light in all these driver names :)

>  	const struct rte_pci_driver *driver;    /**< Associated driver */
[...]
> --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> +#define IORESOURCE_MEM        0x00000200

Please comment this value.

> --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
> +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
> @@ -50,8 +50,14 @@ enum rte_intr_handle_type {
>  
>  /** Handle for interrupts. */
>  struct rte_intr_handle {
> -	int vfio_dev_fd;                 /**< VFIO device file descriptor */
> -	int fd;                          /**< file descriptor */
> +	union {
> +		int vfio_dev_fd;  /**< VFIO device file descriptor */
> +	};
> +        union {
> +		int uio_cfg_fd;  /**< UIO config file descriptor
> +					for uio_pci_generic */
> +	};

Apart the indent, it seems there is a mistake here.
Why 2 unions with 1 field each?

> +	int fd;	 /**< interrupt event file descriptor */
>  	enum rte_intr_handle_type type;  /**< handle type */
>  };

Re: [dpdk-dev] [PATCH v1 1/3] eal: enable uio_pci_generic support

[Zhou, Danny]

Thomas, thanks for review and I added comments inline.

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, February 18, 2015 9:40 PM
> To: Zhou, Danny
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v1 1/3] eal: enable uio_pci_generic support
> 
> Hi Danny,
> 
> I wanted to apply this patchset which was reviewed. But when having a quick
> overview, I've seen some strange additions.
> 
> 2015-01-29 17:28, Danny Zhou:
> > 1) Unify procedure to retrieve BAR resource mapping information.
> > 2) Setup bus master bit in NIC's PCIe configuration space for uio_pci_generic.
> >
> > Signed-off-by: Danny Zhou <danny.zhou@intel.com>
> > Tested-by: Qun Wan <qun.wan@intel.com>
> [...]
> > --- a/lib/librte_eal/common/include/rte_pci.h
> > +++ b/lib/librte_eal/common/include/rte_pci.h
> > @@ -148,6 +148,7 @@ struct rte_pci_device {
> >  	struct rte_pci_id id;                   /**< PCI ID. */
> >  	struct rte_pci_resource mem_resource[PCI_MAX_RESOURCE];   /**< PCI Memory Resource */
> >  	struct rte_intr_handle intr_handle;     /**< Interrupt handle */
> > +	char driver_name[BUFSIZ];               /**< driver name */
> 
> Why not embedding this field in driver struct?
> The name and comment should be more precise.
> There is also driver->name and hotplug patchset is adding a kernel driver name.
> Please bring the light in all these driver names :)
> 

This driver_name is the name of kernel driver(e.g. vfio_pci, igb_uio, uio_pci_generic) while the driver->name is
a user-defined name for user space driver. I am going to change it to kernel_driver_name with precise comment in V2
patch, and when the V2 patch is applied, I think the function pci_get_kernel_driver_by_path() in hotplug patchset is not 
necessary then as it could directly retrieve the kernel driver name from this variable.

> >  	const struct rte_pci_driver *driver;    /**< Associated driver */
> [...]
> > --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> > +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> > +#define IORESOURCE_MEM        0x00000200
> 
> Please comment this value.

Will do.

> 
> > --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
> > +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
> > @@ -50,8 +50,14 @@ enum rte_intr_handle_type {
> >
> >  /** Handle for interrupts. */
> >  struct rte_intr_handle {
> > -	int vfio_dev_fd;                 /**< VFIO device file descriptor */
> > -	int fd;                          /**< file descriptor */
> > +	union {
> > +		int vfio_dev_fd;  /**< VFIO device file descriptor */
> > +	};
> > +        union {
> > +		int uio_cfg_fd;  /**< UIO config file descriptor
> > +					for uio_pci_generic */
> > +	};
> 
> Apart the indent, it seems there is a mistake here.
> Why 2 unions with 1 field each?

It is a mistake I made during code merge, will fix it in V2.

> 
> > +	int fd;	 /**< interrupt event file descriptor */
> >  	enum rte_intr_handle_type type;  /**< handle type */
> >  };