Need to support mapping in bar1

Need to support mapping in bar1

[Patrick Mahan]

Before I do this, I wanted to check with the list.

The UIO support code in DPDK only maps in bar 0 for PCI devices, and it does provide support for mapping any other bar that the device provides.

Has anyone fixed this yet?  Or maybe has a patch?

DPDK version: 1.3.1r2

Thanks,

Patrick

Re: Need to support mapping in bar1

[Stephen Hemminger]

On Thu, 8 Aug 2013 16:45:42 -0700
Patrick Mahan <mahan-5dHXHCkEAVbYtjvyW6yDsg@public.gmane.org> wrote:

> Before I do this, I wanted to check with the list.
> 
> The UIO support code in DPDK only maps in bar 0 for PCI devices, and it does provide support for mapping any other bar that the device provides.
> 
> Has anyone fixed this yet?  Or maybe has a patch?
> 
> DPDK version: 1.3.1r2
> 
> Thanks,
> 
> Patrick

Yes, I sent patches to fix this, look on list. If not will resend.

Re: Need to support mapping in bar1

[Patrick Mahan]

When?

I scanned the archives for this month and last month.  I don't see any patches regarding allowing other BARs to be mapped.

Thanks,

Patrick

Coming to you from deep inside Fortress Mahan

On Aug 8, 2013, at 4:53 PM, Stephen Hemminger <stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ@public.gmane.org> wrote:

> On Thu, 8 Aug 2013 16:45:42 -0700
> Patrick Mahan <mahan-5dHXHCkEAVbYtjvyW6yDsg@public.gmane.org> wrote:
> 
>> Before I do this, I wanted to check with the list.
>> 
>> The UIO support code in DPDK only maps in bar 0 for PCI devices, and it does provide support for mapping any other bar that the device provides.
>> 
>> Has anyone fixed this yet?  Or maybe has a patch?
>> 
>> DPDK version: 1.3.1r2
>> 
>> Thanks,
>> 
>> Patrick
> 
> Yes, I sent patches to fix this, look on list. If not will resend.

Re: Need to support mapping in bar1

[Stephen Hemminger]

Still needs a little cleanup (patch is messy).

Subject: pci: support multiple PCI regions per device
    
Need to change PCI code to support multiple I/O regions on a single device.
Some devices like VMXNET3 have multiple PCI memory regions, and some
have none.

Signed-off-by: Stephen Hemminger <shemminger-ZtmgI6mnKB3QT0dZR+AlfA@public.gmane.org>

---
 app/test-pmd/config.c                   |    2 
 app/test-pmd/testpmd.h                  |    8 -
 lib/librte_eal/common/eal_common_pci.c  |   11 +
 lib/librte_eal/common/include/rte_pci.h |    5 
 lib/librte_eal/linuxapp/eal/eal_pci.c   |  244 +++++++++++++++++++-------------
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c     |    4 
 6 files changed, 167 insertions(+), 107 deletions(-)

--- a/lib/librte_eal/common/eal_common_pci.c	2013-06-05 14:41:46.000000000 -0700
+++ b/lib/librte_eal/common/eal_common_pci.c	2013-08-08 17:47:56.541720272 -0700
@@ -121,12 +121,19 @@ rte_eal_pci_probe(void)
 static int
 pci_dump_one_device(struct rte_pci_device *dev)
 {
+	int i;
+
 	printf(PCI_PRI_FMT, dev->addr.domain, dev->addr.bus,
 	       dev->addr.devid, dev->addr.function);
 	printf(" - vendor:%x device:%x\n", dev->id.vendor_id,
 	       dev->id.device_id);
-	printf("   %16.16"PRIx64" %16.16"PRIx64"\n",
-	       dev->mem_resource.phys_addr, dev->mem_resource.len);
+
+	for (i = 0; i < PCI_MAX_RESOURCE; i++) {
+		printf("   %16.16"PRIx64" %16.16"PRIx64"\n",
+		       dev->mem_resource[i].phys_addr,
+		       dev->mem_resource[i].len);
+	}
+
 	return 0;
 }
 
--- a/lib/librte_eal/common/include/rte_pci.h	2013-08-08 17:41:37.866416535 -0700
+++ b/lib/librte_eal/common/include/rte_pci.h	2013-08-08 17:42:34.585716365 -0700
@@ -50,6 +50,7 @@ extern "C" {
 #include <sys/queue.h>
 #include <stdint.h>
 #include <inttypes.h>
+#include <limits.h>
 #include <rte_interrupts.h>
 
 TAILQ_HEAD(pci_device_list, rte_pci_device); /**< PCI devices in D-linked Q. */
@@ -110,10 +111,12 @@ struct rte_pci_device {
 	TAILQ_ENTRY(rte_pci_device) next;       /**< Next probed PCI device. */
 	struct rte_pci_addr addr;               /**< PCI location. */
 	struct rte_pci_id id;                   /**< PCI ID. */
-	struct rte_pci_resource mem_resource;   /**< PCI Memory Resource */
+	struct rte_pci_resource mem_resource[PCI_MAX_RESOURCE];
+						/**< PCI Memory Resource */
 	struct rte_intr_handle intr_handle;     /**< Interrupt handle */
 	const struct rte_pci_driver *driver;    /**< Associated driver */
 	unsigned int blacklisted:1;             /**< Device is blacklisted */
+	char uio_name[PATH_MAX];		/**< Associated UIO device name */
 };
 
 /** Any PCI device identifier (vendor, device, ...) */
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c	2013-08-08 17:40:52.606974358 -0700
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c	2013-08-08 17:47:28.942064203 -0700
@@ -99,7 +99,6 @@ struct uio_resource {
 
 	struct rte_pci_addr pci_addr;
 	void *addr;
-	char path[PATH_MAX];
 	unsigned long size;
 	unsigned long offset;
 };
@@ -212,64 +211,108 @@ pci_uio_bind_device(struct rte_pci_devic
 	return 0;
 }
 
-/* map a particular resource from a file */
-static void *
-pci_map_resource(struct rte_pci_device *dev, void *requested_addr, const char *devname,
-		unsigned long offset, unsigned long size)
+/*
+ * open devname: it can take some time to
+ * appear, so we wait some time before returning an error
+ */
+static int uio_open(const char *devname)
 {
-	unsigned n;
-	int fd;
-	void *mapaddr;
+	int n, fd;
 
-	/*
-	 * open devname, and mmap it: it can take some time to
-	 * appear, so we wait some time before returning an error
-	 */
-	for (n=0; n<UIO_DEV_WAIT_TIMEOUT*10; n++) {
+	for (n=0; n < UIO_DEV_WAIT_TIMEOUT*10; n++) {
 		fd = open(devname, O_RDWR);
 		if (fd >= 0)
-			break;
+			return fd;
+
 		if (errno != ENOENT)
 			break;
 		usleep(100000);
 	}
-	if (fd < 0) {
-		RTE_LOG(ERR, EAL, "Cannot open %s: %s\n", devname, strerror(errno));
-		goto fail;
-	}
+	return -1;
+}
+
+/* map a particular resource from a file */
+static void *
+pci_mmap(int fd, void *addr, off_t offset, size_t size)
+{
+	void *mapaddr;
 
 	/* Map the PCI memory resource of device */
-	mapaddr = mmap(requested_addr, size, PROT_READ | PROT_WRITE,
-			MAP_SHARED, fd, offset);
-	if (mapaddr == MAP_FAILED ||
-			(requested_addr != NULL && mapaddr != requested_addr)) {
-		RTE_LOG(ERR, EAL, "%s(): cannot mmap %s: %s\n", __func__,
-			devname, strerror(errno));
-		close(fd);
-		goto fail;
-	}
-	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
-		/* save fd if in primary process */
-		dev->intr_handle.fd = fd;
-		dev->intr_handle.type = RTE_INTR_HANDLE_UIO;
-	} else {
-		/* fd is not needed in slave process, close it */
-		dev->intr_handle.fd = -1;
-		dev->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN;
-		close(fd);
+	mapaddr = mmap(addr, size, PROT_READ | PROT_WRITE, MAP_SHARED,
+		       fd, offset);
+	if (mapaddr == MAP_FAILED || (addr != NULL && mapaddr != addr)) {
+		RTE_LOG(ERR, EAL, "%s(): cannot mmap %zd@0x%lx: %s\n",
+			__func__, size, offset, strerror(errno));
+		return NULL;
 	}
 
 	RTE_LOG(DEBUG, EAL, "PCI memory mapped at %p\n", mapaddr);
-
 	return mapaddr;
+}
+
+/* save the mapping details for secondary processes*/
+static int pci_uio_map_save(const struct rte_pci_device *dev, void *mapaddr,
+			    unsigned long offset, unsigned long size)
+{
+	struct uio_resource *uio_res;
+
+	uio_res = rte_malloc("UIO_RES", sizeof(*uio_res), 0);
+	if (uio_res == NULL) {
+		RTE_LOG(ERR, EAL, "%s(): cannot store uio mmap details\n",
+			__func__);
+		return -1;
+	}
+
+	uio_res->addr = mapaddr;
+	uio_res->offset = offset;
+	uio_res->size = size;
+	memcpy(&uio_res->pci_addr, &dev->addr, sizeof(uio_res->pci_addr));
+
+	TAILQ_INSERT_TAIL(uio_res_list, uio_res, next);
+	return 0;
+}
+
+static int pci_uio_map_restore(struct rte_pci_device *dev)
+{
+	struct uio_resource *uio_res;
+	int i, fd;
+	void *addr;
 
-fail:
 	dev->intr_handle.fd = -1;
 	dev->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN;
 
-	return NULL;
+	fd = uio_open(dev->uio_name);
+	if (fd < 0)
+		return -1;
+
+	TAILQ_FOREACH(uio_res, uio_res_list, next) {
+		/* skip this element if it doesn't match our PCI address */
+		if (memcmp(&uio_res->pci_addr, &dev->addr, sizeof(dev->addr)))
+			continue;
+
+		for (i = 0; i < PCI_MAX_RESOURCE; i++) {
+			if (dev->mem_resource[i].len == 0)
+				continue;
+
+			addr = pci_mmap(fd, uio_res->addr,
+					uio_res->offset, uio_res->size);
+			if (addr != uio_res->addr) {
+				RTE_LOG(ERR, EAL, "Cannot mmap device resource\n");
+				close(fd);
+				return -1;
+			}
+		}
+
+		close(fd);
+		return 0;
+	}
+
+	RTE_LOG(ERR, EAL, "Cannot find resource for device\n");
+	close(fd);
+	return -1;
 }
-/* map the PCI resource of a PCI device in virtual memory */
+
+/* map the PCI resources of a PCI device in virtual memory */
 static int
 pci_uio_map_resource(struct rte_pci_device *dev)
 {
@@ -278,35 +321,20 @@ pci_uio_map_resource(struct rte_pci_devi
 	char dirname[PATH_MAX];
 	char dirname2[PATH_MAX];
 	char filename[PATH_MAX];
-	char devname[PATH_MAX]; /* contains the /dev/uioX */
+	int i, fd;
 	void *mapaddr;
 	unsigned uio_num;
-	unsigned long size, offset;
+	unsigned long size, offset, page_size;
 	struct rte_pci_addr *loc = &dev->addr;
-	struct uio_resource *uio_res;
+
+	page_size = sysconf(_SC_PAGE_SIZE);
 
 	RTE_LOG(DEBUG, EAL, "map PCI resource for device "PCI_PRI_FMT"\n",
 	        loc->domain, loc->bus, loc->devid, loc->function);
 
 	/* secondary processes - use already recorded details */
-	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
-
-			TAILQ_FOREACH(uio_res, uio_res_list, next) {
-				/* skip this element if it doesn't match our PCI address */
-				if (memcmp(&uio_res->pci_addr, &dev->addr, sizeof(dev->addr)))
-					continue;
-
-				if (pci_map_resource(dev, uio_res->addr, uio_res->path, \
-						uio_res->offset, uio_res->size) == uio_res->addr)
-					return 0;
-				else {
-					RTE_LOG(ERR, EAL, "Cannot mmap device resource\n");
-					return -1;
-				}
-			}
-			RTE_LOG(ERR, EAL, "Cannot find resource for device\n");
-			return -1;
-	}
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return pci_uio_map_restore(dev);
 
 	/* depending on kernel version, uio can be located in uio/uioX
 	 * or uio:uioX */
@@ -362,44 +390,59 @@ pci_uio_map_resource(struct rte_pci_devi
 	if (e == NULL)
 		return 0;
 
-	/* get mapping offset */
-	rte_snprintf(filename, sizeof(filename),
-		 "%s/maps/map0/offset", dirname2);
-	if (pci_parse_sysfs_value(filename, &offset) < 0) {
-		RTE_LOG(ERR, EAL, "%s(): cannot parse offset\n",
-			__func__);
-		return -1;
-	}
+	/* open /dev/uioX */
+	rte_snprintf(dev->uio_name, sizeof(dev->uio_name),
+		     "/dev/uio%u", uio_num);
 
-	/* get mapping size */
-	rte_snprintf(filename, sizeof(filename),
-		 "%s/maps/map0/size", dirname2);
-	if (pci_parse_sysfs_value(filename, &size) < 0) {
-		RTE_LOG(ERR, EAL, "%s(): cannot parse size\n",
-			__func__);
+	fd = uio_open(dev->uio_name);
+	if (fd < 0) {
+		RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
+			dev->uio_name, strerror(errno));
 		return -1;
 	}
 
-	/* open and mmap /dev/uioX */
-	rte_snprintf(devname, sizeof(devname), "/dev/uio%u", uio_num);
-	mapaddr = pci_map_resource(dev, NULL, devname, offset, size);
-	if (mapaddr == NULL)
-		return -1;
-	dev->mem_resource.addr = mapaddr;
+	/* map associated memory resources. */
+	for (i = 0; i < PCI_MAX_RESOURCE; i++) {
+		if (dev->mem_resource[i].len == 0)
+			continue;
 
-	/* save the mapping details for secondary processes*/
-	uio_res = rte_malloc("UIO_RES", sizeof(*uio_res), 0);
-	if (uio_res == NULL){
-		RTE_LOG(ERR, EAL, "%s(): cannot store uio mmap details\n", __func__);
-		return -1;
+		rte_snprintf(filename, sizeof(filename),
+			     "%s/maps/map%d/offset", dirname2, i);
+
+		if (access(filename, F_OK) < 0)
+			continue; /* this resource is not mapped via uio */
+
+		/* get mapping offset */
+		if (pci_parse_sysfs_value(filename, &offset) < 0) {
+			RTE_LOG(ERR, EAL, "%s(): cannot parse offset\n",
+				__func__);
+			return -1;
+		}
+
+		/* page number indicates which resource */
+		offset += i * page_size;
+
+		/* get mapping size */
+		rte_snprintf(filename, sizeof(filename),
+			     "%s/maps/map%d/size", dirname2, i);
+		if (pci_parse_sysfs_value(filename, &size) < 0) {
+			RTE_LOG(ERR, EAL, "%s(): cannot parse size\n",
+				__func__);
+			return -1;
+		}
+
+		mapaddr = pci_mmap(fd, NULL, offset, size);
+		if (mapaddr == NULL)
+			return -1;
+
+		dev->mem_resource[i].addr = mapaddr;
+		if (pci_uio_map_save(dev, mapaddr, offset, size) < 0)
+			return -1;
 	}
-	uio_res->addr = mapaddr;
-	uio_res->offset = offset;
-	uio_res->size = size;
-	rte_snprintf(uio_res->path, sizeof(uio_res->path), "%s", devname);
-	memcpy(&uio_res->pci_addr, &dev->addr, sizeof(uio_res->pci_addr));
 
-	TAILQ_INSERT_TAIL(uio_res_list, uio_res, next);
+	/* save fd if in primary process */
+	dev->intr_handle.fd = fd;
+	dev->intr_handle.type = RTE_INTR_HANDLE_UIO;
 
 	return 0;
 }
@@ -420,7 +463,7 @@ pci_parse_sysfs_resource(const char *fil
 		};
 		char *ptrs[PCI_RESOURCE_FMT_NVAL];
 	} res_info;
-	int i;
+	int i, m;
 	uint64_t phys_addr, end_addr, flags;
 
 	f = fopen(filename, "r");
@@ -429,6 +472,7 @@ pci_parse_sysfs_resource(const char *fil
 		return -1;
 	}
 
+	m = 0;
 	for (i = 0; i<PCI_MAX_RESOURCE; i++) {
 
 		if (fgets(buf, sizeof(buf), f) == NULL) {
@@ -450,10 +494,16 @@ pci_parse_sysfs_resource(const char *fil
 		}
 
 		if (flags & IORESOURCE_MEM) {
-			dev->mem_resource.phys_addr = phys_addr;
-			dev->mem_resource.len = end_addr - phys_addr + 1;
-			dev->mem_resource.addr = NULL; /* not mapped for now */
-			break;
+			if (m == PCI_MAX_RESOURCE) {
+				RTE_LOG(ERR, EAL, "%s(): too many memory resources\n",
+					__func__);
+				goto error;
+			}
+
+			dev->mem_resource[m].phys_addr = phys_addr;
+			dev->mem_resource[m].len = end_addr - phys_addr + 1;
+			dev->mem_resource[m].addr = NULL; /* not mapped for now */
+			++m;
 		}
 	}
 	fclose(f);
--- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c	2013-08-08 17:40:52.606974358 -0700
+++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c	2013-08-08 17:41:50.266263568 -0700
@@ -369,7 +369,7 @@ eth_ixgbe_dev_init(__attribute__((unused
 	/* Vendor and Device ID need to be set before init of shared code */
 	hw->device_id = pci_dev->id.device_id;
 	hw->vendor_id = pci_dev->id.vendor_id;
-	hw->hw_addr = (void *)pci_dev->mem_resource.addr;
+	hw->hw_addr = pci_dev->mem_resource[0].addr;
 
 	/* Initialize the shared code */
 	diag = ixgbe_init_shared_code(hw);
@@ -490,7 +490,7 @@ eth_ixgbevf_dev_init(__attribute__((unus
 
 	hw->device_id = pci_dev->id.device_id;
 	hw->vendor_id = pci_dev->id.vendor_id;
-	hw->hw_addr = (void *)pci_dev->mem_resource.addr;
+	hw->hw_addr = pci_dev->mem_resource[0].addr;
 
 	/* Initialize the shared code */
 	diag = ixgbe_init_shared_code(hw);
--- a/app/test-pmd/config.c	2013-06-05 14:41:46.000000000 -0700
+++ b/app/test-pmd/config.c	2013-08-08 17:41:50.294263224 -0700
@@ -180,7 +180,7 @@ port_reg_off_is_invalid(portid_t port_id
 		       (unsigned)reg_off);
 		return 1;
 	}
-	pci_len = ports[port_id].dev_info.pci_dev->mem_resource.len;
+	pci_len = ports[port_id].dev_info.pci_dev->mem_resource[0].len;
 	if (reg_off >= pci_len) {
 		printf("Port %d: register offset %u (0x%X) out of port PCI "
 		       "resource (length=%"PRIu64")\n",
--- a/app/test-pmd/testpmd.h	2013-06-05 14:41:46.000000000 -0700
+++ b/app/test-pmd/testpmd.h	2013-08-08 17:41:50.294263224 -0700
@@ -304,8 +304,8 @@ port_pci_reg_read(struct rte_port *port,
 	void *reg_addr;
 	uint32_t reg_v;
 
-	reg_addr = (void *)((char *)port->dev_info.pci_dev->mem_resource.addr +
-			    reg_off);
+	reg_addr = (char *)port->dev_info.pci_dev->mem_resource[0].addr
+		+ reg_off;
 	reg_v = *((volatile uint32_t *)reg_addr);
 	return rte_le_to_cpu_32(reg_v);
 }
@@ -318,8 +318,8 @@ port_pci_reg_write(struct rte_port *port
 {
 	void *reg_addr;
 
-	reg_addr = (void *)((char *)port->dev_info.pci_dev->mem_resource.addr +
-			    reg_off);
+	reg_addr = (char *)port->dev_info.pci_dev->mem_resource[0].addr
+		    + reg_off;
 	*((volatile uint32_t *)reg_addr) = rte_cpu_to_le_32(reg_v);
 }
 

Re: Need to support mapping in bar1

[Patrick Mahan]

Stephen,

When did you generate this diff?  It failed to apply to 1.3.1r2 so I am hand
patching instead.  But you might want to update to the latest version.

Thanks,

Patrick

On 8/8/13 5:50 PM, Stephen Hemminger wrote:
> Still needs a little cleanup (patch is messy).
> 
> Subject: pci: support multiple PCI regions per device
>     
> Need to change PCI code to support multiple I/O regions on a single device.
> Some devices like VMXNET3 have multiple PCI memory regions, and some
> have none.
> 
> Signed-off-by: Stephen Hemminger <shemminger-ZtmgI6mnKB3QT0dZR+AlfA@public.gmane.org>
> 
> ---
>  app/test-pmd/config.c                   |    2 
>  app/test-pmd/testpmd.h                  |    8 -
>  lib/librte_eal/common/eal_common_pci.c  |   11 +
>  lib/librte_eal/common/include/rte_pci.h |    5 
>  lib/librte_eal/linuxapp/eal/eal_pci.c   |  244 +++++++++++++++++++-------------
>  lib/librte_pmd_ixgbe/ixgbe_ethdev.c     |    4 
>  6 files changed, 167 insertions(+), 107 deletions(-)
> 
> --- a/lib/librte_eal/common/eal_common_pci.c	2013-06-05 14:41:46.000000000 -0700
> +++ b/lib/librte_eal/common/eal_common_pci.c	2013-08-08 17:47:56.541720272 -0700
> @@ -121,12 +121,19 @@ rte_eal_pci_probe(void)
>  static int
>  pci_dump_one_device(struct rte_pci_device *dev)
>  {
> +	int i;
> +
>  	printf(PCI_PRI_FMT, dev->addr.domain, dev->addr.bus,
>  	       dev->addr.devid, dev->addr.function);
>  	printf(" - vendor:%x device:%x\n", dev->id.vendor_id,
>  	       dev->id.device_id);
> -	printf("   %16.16"PRIx64" %16.16"PRIx64"\n",
> -	       dev->mem_resource.phys_addr, dev->mem_resource.len);
> +
> +	for (i = 0; i < PCI_MAX_RESOURCE; i++) {
> +		printf("   %16.16"PRIx64" %16.16"PRIx64"\n",
> +		       dev->mem_resource[i].phys_addr,
> +		       dev->mem_resource[i].len);
> +	}
> +
>  	return 0;
>  }
>  
> --- a/lib/librte_eal/common/include/rte_pci.h	2013-08-08 17:41:37.866416535 -0700
> +++ b/lib/librte_eal/common/include/rte_pci.h	2013-08-08 17:42:34.585716365 -0700
> @@ -50,6 +50,7 @@ extern "C" {
>  #include <sys/queue.h>
>  #include <stdint.h>
>  #include <inttypes.h>
> +#include <limits.h>
>  #include <rte_interrupts.h>
>  
>  TAILQ_HEAD(pci_device_list, rte_pci_device); /**< PCI devices in D-linked Q. */
> @@ -110,10 +111,12 @@ struct rte_pci_device {
>  	TAILQ_ENTRY(rte_pci_device) next;       /**< Next probed PCI device. */
>  	struct rte_pci_addr addr;               /**< PCI location. */
>  	struct rte_pci_id id;                   /**< PCI ID. */
> -	struct rte_pci_resource mem_resource;   /**< PCI Memory Resource */
> +	struct rte_pci_resource mem_resource[PCI_MAX_RESOURCE];
> +						/**< PCI Memory Resource */
>  	struct rte_intr_handle intr_handle;     /**< Interrupt handle */
>  	const struct rte_pci_driver *driver;    /**< Associated driver */
>  	unsigned int blacklisted:1;             /**< Device is blacklisted */
> +	char uio_name[PATH_MAX];		/**< Associated UIO device name */
>  };
>  
>  /** Any PCI device identifier (vendor, device, ...) */
> --- a/lib/librte_eal/linuxapp/eal/eal_pci.c	2013-08-08 17:40:52.606974358 -0700
> +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c	2013-08-08 17:47:28.942064203 -0700
> @@ -99,7 +99,6 @@ struct uio_resource {
>  
>  	struct rte_pci_addr pci_addr;
>  	void *addr;
> -	char path[PATH_MAX];
>  	unsigned long size;
>  	unsigned long offset;
>  };
> @@ -212,64 +211,108 @@ pci_uio_bind_device(struct rte_pci_devic
>  	return 0;
>  }
>  
> -/* map a particular resource from a file */
> -static void *
> -pci_map_resource(struct rte_pci_device *dev, void *requested_addr, const char *devname,
> -		unsigned long offset, unsigned long size)
> +/*
> + * open devname: it can take some time to
> + * appear, so we wait some time before returning an error
> + */
> +static int uio_open(const char *devname)
>  {
> -	unsigned n;
> -	int fd;
> -	void *mapaddr;
> +	int n, fd;
>  
> -	/*
> -	 * open devname, and mmap it: it can take some time to
> -	 * appear, so we wait some time before returning an error
> -	 */
> -	for (n=0; n<UIO_DEV_WAIT_TIMEOUT*10; n++) {
> +	for (n=0; n < UIO_DEV_WAIT_TIMEOUT*10; n++) {
>  		fd = open(devname, O_RDWR);
>  		if (fd >= 0)
> -			break;
> +			return fd;
> +
>  		if (errno != ENOENT)
>  			break;
>  		usleep(100000);
>  	}
> -	if (fd < 0) {
> -		RTE_LOG(ERR, EAL, "Cannot open %s: %s\n", devname, strerror(errno));
> -		goto fail;
> -	}
> +	return -1;
> +}
> +
> +/* map a particular resource from a file */
> +static void *
> +pci_mmap(int fd, void *addr, off_t offset, size_t size)
> +{
> +	void *mapaddr;
>  
>  	/* Map the PCI memory resource of device */
> -	mapaddr = mmap(requested_addr, size, PROT_READ | PROT_WRITE,
> -			MAP_SHARED, fd, offset);
> -	if (mapaddr == MAP_FAILED ||
> -			(requested_addr != NULL && mapaddr != requested_addr)) {
> -		RTE_LOG(ERR, EAL, "%s(): cannot mmap %s: %s\n", __func__,
> -			devname, strerror(errno));
> -		close(fd);
> -		goto fail;
> -	}
> -	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> -		/* save fd if in primary process */
> -		dev->intr_handle.fd = fd;
> -		dev->intr_handle.type = RTE_INTR_HANDLE_UIO;
> -	} else {
> -		/* fd is not needed in slave process, close it */
> -		dev->intr_handle.fd = -1;
> -		dev->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN;
> -		close(fd);
> +	mapaddr = mmap(addr, size, PROT_READ | PROT_WRITE, MAP_SHARED,
> +		       fd, offset);
> +	if (mapaddr == MAP_FAILED || (addr != NULL && mapaddr != addr)) {
> +		RTE_LOG(ERR, EAL, "%s(): cannot mmap %zd@0x%lx: %s\n",
> +			__func__, size, offset, strerror(errno));
> +		return NULL;
>  	}
>  
>  	RTE_LOG(DEBUG, EAL, "PCI memory mapped at %p\n", mapaddr);
> -
>  	return mapaddr;
> +}
> +
> +/* save the mapping details for secondary processes*/
> +static int pci_uio_map_save(const struct rte_pci_device *dev, void *mapaddr,
> +			    unsigned long offset, unsigned long size)
> +{
> +	struct uio_resource *uio_res;
> +
> +	uio_res = rte_malloc("UIO_RES", sizeof(*uio_res), 0);
> +	if (uio_res == NULL) {
> +		RTE_LOG(ERR, EAL, "%s(): cannot store uio mmap details\n",
> +			__func__);
> +		return -1;
> +	}
> +
> +	uio_res->addr = mapaddr;
> +	uio_res->offset = offset;
> +	uio_res->size = size;
> +	memcpy(&uio_res->pci_addr, &dev->addr, sizeof(uio_res->pci_addr));
> +
> +	TAILQ_INSERT_TAIL(uio_res_list, uio_res, next);
> +	return 0;
> +}
> +
> +static int pci_uio_map_restore(struct rte_pci_device *dev)
> +{
> +	struct uio_resource *uio_res;
> +	int i, fd;
> +	void *addr;
>  
> -fail:
>  	dev->intr_handle.fd = -1;
>  	dev->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN;
>  
> -	return NULL;
> +	fd = uio_open(dev->uio_name);
> +	if (fd < 0)
> +		return -1;
> +
> +	TAILQ_FOREACH(uio_res, uio_res_list, next) {
> +		/* skip this element if it doesn't match our PCI address */
> +		if (memcmp(&uio_res->pci_addr, &dev->addr, sizeof(dev->addr)))
> +			continue;
> +
> +		for (i = 0; i < PCI_MAX_RESOURCE; i++) {
> +			if (dev->mem_resource[i].len == 0)
> +				continue;
> +
> +			addr = pci_mmap(fd, uio_res->addr,
> +					uio_res->offset, uio_res->size);
> +			if (addr != uio_res->addr) {
> +				RTE_LOG(ERR, EAL, "Cannot mmap device resource\n");
> +				close(fd);
> +				return -1;
> +			}
> +		}
> +
> +		close(fd);
> +		return 0;
> +	}
> +
> +	RTE_LOG(ERR, EAL, "Cannot find resource for device\n");
> +	close(fd);
> +	return -1;
>  }
> -/* map the PCI resource of a PCI device in virtual memory */
> +
> +/* map the PCI resources of a PCI device in virtual memory */
>  static int
>  pci_uio_map_resource(struct rte_pci_device *dev)
>  {
> @@ -278,35 +321,20 @@ pci_uio_map_resource(struct rte_pci_devi
>  	char dirname[PATH_MAX];
>  	char dirname2[PATH_MAX];
>  	char filename[PATH_MAX];
> -	char devname[PATH_MAX]; /* contains the /dev/uioX */
> +	int i, fd;
>  	void *mapaddr;
>  	unsigned uio_num;
> -	unsigned long size, offset;
> +	unsigned long size, offset, page_size;
>  	struct rte_pci_addr *loc = &dev->addr;
> -	struct uio_resource *uio_res;
> +
> +	page_size = sysconf(_SC_PAGE_SIZE);
>  
>  	RTE_LOG(DEBUG, EAL, "map PCI resource for device "PCI_PRI_FMT"\n",
>  	        loc->domain, loc->bus, loc->devid, loc->function);
>  
>  	/* secondary processes - use already recorded details */
> -	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
> -
> -			TAILQ_FOREACH(uio_res, uio_res_list, next) {
> -				/* skip this element if it doesn't match our PCI address */
> -				if (memcmp(&uio_res->pci_addr, &dev->addr, sizeof(dev->addr)))
> -					continue;
> -
> -				if (pci_map_resource(dev, uio_res->addr, uio_res->path, \
> -						uio_res->offset, uio_res->size) == uio_res->addr)
> -					return 0;
> -				else {
> -					RTE_LOG(ERR, EAL, "Cannot mmap device resource\n");
> -					return -1;
> -				}
> -			}
> -			RTE_LOG(ERR, EAL, "Cannot find resource for device\n");
> -			return -1;
> -	}
> +	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> +		return pci_uio_map_restore(dev);
>  
>  	/* depending on kernel version, uio can be located in uio/uioX
>  	 * or uio:uioX */
> @@ -362,44 +390,59 @@ pci_uio_map_resource(struct rte_pci_devi
>  	if (e == NULL)
>  		return 0;
>  
> -	/* get mapping offset */
> -	rte_snprintf(filename, sizeof(filename),
> -		 "%s/maps/map0/offset", dirname2);
> -	if (pci_parse_sysfs_value(filename, &offset) < 0) {
> -		RTE_LOG(ERR, EAL, "%s(): cannot parse offset\n",
> -			__func__);
> -		return -1;
> -	}
> +	/* open /dev/uioX */
> +	rte_snprintf(dev->uio_name, sizeof(dev->uio_name),
> +		     "/dev/uio%u", uio_num);
>  
> -	/* get mapping size */
> -	rte_snprintf(filename, sizeof(filename),
> -		 "%s/maps/map0/size", dirname2);
> -	if (pci_parse_sysfs_value(filename, &size) < 0) {
> -		RTE_LOG(ERR, EAL, "%s(): cannot parse size\n",
> -			__func__);
> +	fd = uio_open(dev->uio_name);
> +	if (fd < 0) {
> +		RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
> +			dev->uio_name, strerror(errno));
>  		return -1;
>  	}
>  
> -	/* open and mmap /dev/uioX */
> -	rte_snprintf(devname, sizeof(devname), "/dev/uio%u", uio_num);
> -	mapaddr = pci_map_resource(dev, NULL, devname, offset, size);
> -	if (mapaddr == NULL)
> -		return -1;
> -	dev->mem_resource.addr = mapaddr;
> +	/* map associated memory resources. */
> +	for (i = 0; i < PCI_MAX_RESOURCE; i++) {
> +		if (dev->mem_resource[i].len == 0)
> +			continue;
>  
> -	/* save the mapping details for secondary processes*/
> -	uio_res = rte_malloc("UIO_RES", sizeof(*uio_res), 0);
> -	if (uio_res == NULL){
> -		RTE_LOG(ERR, EAL, "%s(): cannot store uio mmap details\n", __func__);
> -		return -1;
> +		rte_snprintf(filename, sizeof(filename),
> +			     "%s/maps/map%d/offset", dirname2, i);
> +
> +		if (access(filename, F_OK) < 0)
> +			continue; /* this resource is not mapped via uio */
> +
> +		/* get mapping offset */
> +		if (pci_parse_sysfs_value(filename, &offset) < 0) {
> +			RTE_LOG(ERR, EAL, "%s(): cannot parse offset\n",
> +				__func__);
> +			return -1;
> +		}
> +
> +		/* page number indicates which resource */
> +		offset += i * page_size;
> +
> +		/* get mapping size */
> +		rte_snprintf(filename, sizeof(filename),
> +			     "%s/maps/map%d/size", dirname2, i);
> +		if (pci_parse_sysfs_value(filename, &size) < 0) {
> +			RTE_LOG(ERR, EAL, "%s(): cannot parse size\n",
> +				__func__);
> +			return -1;
> +		}
> +
> +		mapaddr = pci_mmap(fd, NULL, offset, size);
> +		if (mapaddr == NULL)
> +			return -1;
> +
> +		dev->mem_resource[i].addr = mapaddr;
> +		if (pci_uio_map_save(dev, mapaddr, offset, size) < 0)
> +			return -1;
>  	}
> -	uio_res->addr = mapaddr;
> -	uio_res->offset = offset;
> -	uio_res->size = size;
> -	rte_snprintf(uio_res->path, sizeof(uio_res->path), "%s", devname);
> -	memcpy(&uio_res->pci_addr, &dev->addr, sizeof(uio_res->pci_addr));
>  
> -	TAILQ_INSERT_TAIL(uio_res_list, uio_res, next);
> +	/* save fd if in primary process */
> +	dev->intr_handle.fd = fd;
> +	dev->intr_handle.type = RTE_INTR_HANDLE_UIO;
>  
>  	return 0;
>  }
> @@ -420,7 +463,7 @@ pci_parse_sysfs_resource(const char *fil
>  		};
>  		char *ptrs[PCI_RESOURCE_FMT_NVAL];
>  	} res_info;
> -	int i;
> +	int i, m;
>  	uint64_t phys_addr, end_addr, flags;
>  
>  	f = fopen(filename, "r");
> @@ -429,6 +472,7 @@ pci_parse_sysfs_resource(const char *fil
>  		return -1;
>  	}
>  
> +	m = 0;
>  	for (i = 0; i<PCI_MAX_RESOURCE; i++) {
>  
>  		if (fgets(buf, sizeof(buf), f) == NULL) {
> @@ -450,10 +494,16 @@ pci_parse_sysfs_resource(const char *fil
>  		}
>  
>  		if (flags & IORESOURCE_MEM) {
> -			dev->mem_resource.phys_addr = phys_addr;
> -			dev->mem_resource.len = end_addr - phys_addr + 1;
> -			dev->mem_resource.addr = NULL; /* not mapped for now */
> -			break;
> +			if (m == PCI_MAX_RESOURCE) {
> +				RTE_LOG(ERR, EAL, "%s(): too many memory resources\n",
> +					__func__);
> +				goto error;
> +			}
> +
> +			dev->mem_resource[m].phys_addr = phys_addr;
> +			dev->mem_resource[m].len = end_addr - phys_addr + 1;
> +			dev->mem_resource[m].addr = NULL; /* not mapped for now */
> +			++m;
>  		}
>  	}
>  	fclose(f);
> --- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c	2013-08-08 17:40:52.606974358 -0700
> +++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c	2013-08-08 17:41:50.266263568 -0700
> @@ -369,7 +369,7 @@ eth_ixgbe_dev_init(__attribute__((unused
>  	/* Vendor and Device ID need to be set before init of shared code */
>  	hw->device_id = pci_dev->id.device_id;
>  	hw->vendor_id = pci_dev->id.vendor_id;
> -	hw->hw_addr = (void *)pci_dev->mem_resource.addr;
> +	hw->hw_addr = pci_dev->mem_resource[0].addr;
>  
>  	/* Initialize the shared code */
>  	diag = ixgbe_init_shared_code(hw);
> @@ -490,7 +490,7 @@ eth_ixgbevf_dev_init(__attribute__((unus
>  
>  	hw->device_id = pci_dev->id.device_id;
>  	hw->vendor_id = pci_dev->id.vendor_id;
> -	hw->hw_addr = (void *)pci_dev->mem_resource.addr;
> +	hw->hw_addr = pci_dev->mem_resource[0].addr;
>  
>  	/* Initialize the shared code */
>  	diag = ixgbe_init_shared_code(hw);
> --- a/app/test-pmd/config.c	2013-06-05 14:41:46.000000000 -0700
> +++ b/app/test-pmd/config.c	2013-08-08 17:41:50.294263224 -0700
> @@ -180,7 +180,7 @@ port_reg_off_is_invalid(portid_t port_id
>  		       (unsigned)reg_off);
>  		return 1;
>  	}
> -	pci_len = ports[port_id].dev_info.pci_dev->mem_resource.len;
> +	pci_len = ports[port_id].dev_info.pci_dev->mem_resource[0].len;
>  	if (reg_off >= pci_len) {
>  		printf("Port %d: register offset %u (0x%X) out of port PCI "
>  		       "resource (length=%"PRIu64")\n",
> --- a/app/test-pmd/testpmd.h	2013-06-05 14:41:46.000000000 -0700
> +++ b/app/test-pmd/testpmd.h	2013-08-08 17:41:50.294263224 -0700
> @@ -304,8 +304,8 @@ port_pci_reg_read(struct rte_port *port,
>  	void *reg_addr;
>  	uint32_t reg_v;
>  
> -	reg_addr = (void *)((char *)port->dev_info.pci_dev->mem_resource.addr +
> -			    reg_off);
> +	reg_addr = (char *)port->dev_info.pci_dev->mem_resource[0].addr
> +		+ reg_off;
>  	reg_v = *((volatile uint32_t *)reg_addr);
>  	return rte_le_to_cpu_32(reg_v);
>  }
> @@ -318,8 +318,8 @@ port_pci_reg_write(struct rte_port *port
>  {
>  	void *reg_addr;
>  
> -	reg_addr = (void *)((char *)port->dev_info.pci_dev->mem_resource.addr +
> -			    reg_off);
> +	reg_addr = (char *)port->dev_info.pci_dev->mem_resource[0].addr
> +		    + reg_off;
>  	*((volatile uint32_t *)reg_addr) = rte_cpu_to_le_32(reg_v);
>  }
>  
> 

Re: Need to support mapping in bar1

[Stephen Hemminger]

On Fri, 09 Aug 2013 12:46:09 -0700
Patrick Mahan <mahan-5dHXHCkEAVbYtjvyW6yDsg@public.gmane.org> wrote:

> Stephen,
> 
> When did you generate this diff?  It failed to apply to 1.3.1r2 so I am hand
> patching instead.  But you might want to update to the latest version.
> 
> Thanks,
> 
> Patrick
> 

Diff was against current git HEAD

Re: Need to support mapping in bar1

[Stephen Hemminger]

It assumed another patch to allow different UIO driver devices.
It hasn't been accepted, I presume because 6Wind doesn't like the patch
because their drivers work differently.

Subject: eal: support different modules
    
In order to support new devices, the module name should not be a boolean
flag to EAL subsystem, instead use have a  module name in pci device structure.

An additional change is that failure to detect module should not be fatal,
but an error like other EAL setup problems, allowing the application
to recover.

Signed-off-by: Stephen Hemminger <shemminger-ZtmgI6mnKB3QT0dZR+AlfA@public.gmane.org>

---
 app/chkincs/test_pci.c                  |    3 ++-
 app/test/test_pci.c                     |    4 ++--
 lib/librte_eal/common/include/rte_pci.h |    5 ++---
 lib/librte_eal/linuxapp/eal/eal_pci.c   |    9 ++-------
 lib/librte_pmd_igb/e1000_ethdev.c       |    2 +-
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c     |    4 ++--
 6 files changed, 11 insertions(+), 16 deletions(-)

--- a/lib/librte_eal/common/include/rte_pci.h	2013-07-16 09:44:47.000000000 -0700
+++ b/lib/librte_eal/common/include/rte_pci.h	2013-08-08 17:41:37.866416535 -0700
@@ -151,10 +151,9 @@ struct rte_pci_driver {
 	pci_devinit_t *devinit;                 /**< Device init. function. */
 	struct rte_pci_id *id_table;            /**< ID table, NULL terminated. */
 	uint32_t drv_flags;                     /**< Flags contolling handling of device. */
+	const char *module_name;		/**< Associated kernel module */
 };
 
-/** Device needs igb_uio kernel module */
-#define RTE_PCI_DRV_NEED_IGB_UIO 0x0001
 /** Device driver must be registered several times until failure */
 #define RTE_PCI_DRV_MULTIPLE 0x0002
 /** Device needs to be unbound even if no module is provided */
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c	2013-07-16 09:44:47.255210583 -0700
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c	2013-08-08 17:40:52.606974358 -0700
@@ -76,7 +76,7 @@
  * This code is used to simulate a PCI probe by parsing information in
  * sysfs. Moreover, when a registered driver matches a device, the
  * kernel driver currently using it is unloaded and replaced by
- * igb_uio module, which is a very minimal userland driver for Intel
+ * a module, which is a very minimal userland driver for Intel
  * network card, only providing access to PCI BAR to applications, and
  * enabling bus master.
  */
@@ -84,8 +84,6 @@
 
 #define PROC_MODULES "/proc/modules"
 
-#define IGB_UIO_NAME "igb_uio"
-
 #define UIO_NEWID "/sys/bus/pci/drivers/%s/new_id"
 #define UIO_BIND  "/sys/bus/pci/drivers/%s/bind"
 
@@ -700,12 +698,9 @@ int
 rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *dev)
 {
 	struct rte_pci_id *id_table;
-	const char *module_name = NULL;
+	const char *module_name = dr->module_name;
 	int uio_status = -1;
 
-	if (dr->drv_flags & RTE_PCI_DRV_NEED_IGB_UIO)
-		module_name = IGB_UIO_NAME;
-
 	uio_status = pci_uio_check_module(module_name);
 
 	for (id_table = dr->id_table ; id_table->vendor_id != 0; id_table++) {
--- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c	2013-06-05 14:41:53.000000000 -0700
+++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c	2013-08-08 17:40:52.606974358 -0700
@@ -547,7 +547,7 @@ static struct eth_driver rte_ixgbe_pmd =
 	{
 		.name = "rte_ixgbe_pmd",
 		.id_table = pci_id_ixgbe_map,
-		.drv_flags = RTE_PCI_DRV_NEED_IGB_UIO,
+		.module_name = "igb_uio",
 	},
 	.eth_dev_init = eth_ixgbe_dev_init,
 	.dev_private_size = sizeof(struct ixgbe_adapter),
@@ -560,7 +560,7 @@ static struct eth_driver rte_ixgbevf_pmd
 	{
 		.name = "rte_ixgbevf_pmd",
 		.id_table = pci_id_ixgbevf_map,
-		.drv_flags = RTE_PCI_DRV_NEED_IGB_UIO,
+		.module_name = "igb_uio",
 	},
 	.eth_dev_init = eth_ixgbevf_dev_init,
 	.dev_private_size = sizeof(struct ixgbe_adapter),
--- a/app/test/test_pci.c	2013-06-05 14:41:53.000000000 -0700
+++ b/app/test/test_pci.c	2013-08-08 17:40:52.606974358 -0700
@@ -69,7 +69,7 @@ static int my_driver_init(struct rte_pci
 			  struct rte_pci_device *dev);
 
 /*
- * To test cases where RTE_PCI_DRV_NEED_IGB_UIO is set, and isn't set, two
+ * To test cases where module_name is set, and isn't set, two
  * drivers are created (one with IGB devices, the other with IXGBE devices).
  */
 
@@ -101,7 +101,7 @@ struct rte_pci_driver my_driver = {
 	.name = "test_driver",
 	.devinit = my_driver_init,
 	.id_table = my_driver_id,
-	.drv_flags = RTE_PCI_DRV_NEED_IGB_UIO,
+	.module_name = "igb_uio",
 };
 
 struct rte_pci_driver my_driver2 = {
--- a/app/chkincs/test_pci.c	2013-06-05 14:41:53.000000000 -0700
+++ b/app/chkincs/test_pci.c	2013-08-08 17:40:52.606974358 -0700
@@ -64,7 +64,8 @@ struct rte_pci_driver my_driver = {
 	"test_driver",
 	my_driver_init,
 	my_driver_id,
-	RTE_PCI_DRV_NEED_IGB_UIO,
+	0,
+	"igb_uio",
 };
 
 static int
--- a/lib/librte_pmd_igb/e1000_ethdev.c	2013-06-05 14:41:53.000000000 -0700
+++ b/lib/librte_pmd_igb/e1000_ethdev.c	2013-08-08 17:40:52.610974310 -0700
@@ -235,7 +235,7 @@ eth_igb_dev_init(__attribute__((unused))
 		return 0;
 	}
 
-	hw->hw_addr= (void *)pci_dev->mem_resource.addr;
+	hw->hw_addr = pci_dev->mem_resource[0].addr;
 
 	igb_identify_hardware(eth_dev);
 
@@ -336,7 +336,7 @@ static struct eth_driver rte_igb_pmd = {
 	{
 		.name = "rte_igb_pmd",
 		.id_table = pci_id_igb_map,
-		.drv_flags = RTE_PCI_DRV_NEED_IGB_UIO,
+		.module_name = "igb_uio",
 	},
 	.eth_dev_init = eth_igb_dev_init,
 	.dev_private_size = sizeof(struct e1000_adapter),

Re: Need to support mapping in bar1

[Thomas Monjalon]

09/08/2013 15:17, Stephen Hemminger :
> It assumed another patch to allow different UIO driver devices.
> It hasn't been accepted, I presume because 6Wind doesn't like the patch
> because their drivers work differently.

This patch was reviewed but not approved because it was waiting a refactoring.
The discussion was stopped shortly.
Probably someone should provide a new version which integrate a string list of 
modules. Otherwise you should give strong arguments to integrate your patch as 
is.
If it's clear that your patch is right, it will be integrated as any other 
one. Please avoid to presume wrongly ;)

Note that I'm also waiting reviews for my last patches about Linux compat.
Reviewers are welcome !

-- 
Thomas