DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v6 0/2] support both PIO and MMIO BAR for legacy device in virtio PMD
@ 2021-01-29  3:18 谢华伟(此时此刻)
  2021-01-29  3:18 ` [dpdk-dev] [PATCH v6 1/2] bus/pci: use PCI standard sysfs entry to get PIO address 谢华伟(此时此刻)
                   ` (3 more replies)
  0 siblings, 4 replies; 28+ messages in thread
From: 谢华伟(此时此刻) @ 2021-01-29  3:18 UTC (permalink / raw)
  To: ferruh.yigit, maxime.coquelin
  Cc: dev, david.marchand, anatoly.burakov, xuemingl, grive,
	chenbo.xia,
	谢华伟(此时此刻)

From: "huawei.xhw" <huawei.xhw@alibaba-inc.com>


virtio PMD assumes legacy device only supports PIO BAR resource. This is wrong.
As we need to create lots of devices, as PIO resource on x86 is very limited, 
we expose MMIO(memory IO) BAR.

Kernel supports both PIO and MMIO BAR for legacy virtio-pci device, and for all
other pci devices. This patchset handles different type of BAR in the similar way.

In previous implementation, under igb_uio driver we get PIO address from igb_uio
sysfs entry; with uio_pci_generic, we get PIO address from /proc/ioports for x86,
and for other ARCHs, we get PIO address from standard PCI sysfs entry.
For PIO/MMIO RW, there is different path for different drivers and arch.


All of the above is too much twisted.
This patchset unifies the way to get both PIO and MMIO address for different driver
and ARCHs, all from standard resource attr under pci sysfs. This is most generic.

We distinguish PIO and MMIO by their address like how kernel does. It is ugly but works.

v2 changes:
    - add more explanation in the commit message

v3 changes:
    - fix patch format issues

v4 changes:
    - fixes for RTE_KDRV_UIO_GENERIC -> RTE_PCI_KDRV_UIO_GENERIC

v5 changes:
    - split into three seperate patches

v6 changes:
    - change to DEBUG level for IO bar detection in pci_uio_ioport_map
    - rework the code in iobar branch
    - fixes commit message format issue
    - temporarily remove the 3rd patch for vfio path, leave it for future discusssion
    - rework against virtio_pmd_rework_v2


huawei.xhw (2):
  bus/pci: use PCI standard sysfs entry to get PIO address
  bus/pci: support MMIO in PCI ioport accessors

 drivers/bus/pci/linux/pci.c     |  81 -------------------
 drivers/bus/pci/linux/pci_uio.c | 170 ++++++++++++++++++++++++++++------------
 2 files changed, 118 insertions(+), 133 deletions(-)

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [dpdk-dev] [PATCH v6 1/2] bus/pci: use PCI standard sysfs entry to get PIO address
  2021-01-29  3:18 [dpdk-dev] [PATCH v6 0/2] support both PIO and MMIO BAR for legacy device in virtio PMD 谢华伟(此时此刻)
@ 2021-01-29  3:18 ` 谢华伟(此时此刻)
  2021-02-03  9:37   ` Maxime Coquelin
  2021-02-18  9:33   ` David Marchand
  2021-01-29  3:18 ` [dpdk-dev] [PATCH v6 2/2] bus/pci: support MMIO in PCI ioport accessors 谢华伟(此时此刻)
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 28+ messages in thread
From: 谢华伟(此时此刻) @ 2021-01-29  3:18 UTC (permalink / raw)
  To: ferruh.yigit, maxime.coquelin
  Cc: dev, david.marchand, anatoly.burakov, xuemingl, grive,
	chenbo.xia,
	谢华伟(此时此刻)

From: "huawei.xhw" <huawei.xhw@alibaba-inc.com>

Currently virtio PMD asssumes legacy device uses PIO bar.
There are three ways to get PIO(PortIO) address for virtio legacy device.
    under igb_uio, get pio address from uio/uio# sysfs attribute
    under uio_pci_generic:
        for X86, get PIO address from /proc/ioport
        for other ARCH, get PIO address from standard PCI sysfs attribute

Actually, igb_uio sysfs attribute exports exactly the same thing as standard PCI sysfs, i.e,
    pci_dev->resource[]

This patch fixes these messy things, and uses standard PCI sysfs attribute.

Signed-off-by: huawei xie <huawei.xhw@alibaba-inc.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/bus/pci/linux/pci.c     | 77 -----------------------------------------
 drivers/bus/pci/linux/pci_uio.c | 64 ++++++++++++++++++++++++----------
 2 files changed, 46 insertions(+), 95 deletions(-)

diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index 2e1808b..0f38abf 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -677,71 +677,6 @@ int rte_pci_write_config(const struct rte_pci_device *device,
 	}
 }
 
-#if defined(RTE_ARCH_X86)
-static int
-pci_ioport_map(struct rte_pci_device *dev, int bar __rte_unused,
-		struct rte_pci_ioport *p)
-{
-	uint16_t start, end;
-	FILE *fp;
-	char *line = NULL;
-	char pci_id[16];
-	int found = 0;
-	size_t linesz;
-
-	if (rte_eal_iopl_init() != 0) {
-		RTE_LOG(ERR, EAL, "%s(): insufficient ioport permissions for PCI device %s\n",
-			__func__, dev->name);
-		return -1;
-	}
-
-	snprintf(pci_id, sizeof(pci_id), PCI_PRI_FMT,
-		 dev->addr.domain, dev->addr.bus,
-		 dev->addr.devid, dev->addr.function);
-
-	fp = fopen("/proc/ioports", "r");
-	if (fp == NULL) {
-		RTE_LOG(ERR, EAL, "%s(): can't open ioports\n", __func__);
-		return -1;
-	}
-
-	while (getdelim(&line, &linesz, '\n', fp) > 0) {
-		char *ptr = line;
-		char *left;
-		int n;
-
-		n = strcspn(ptr, ":");
-		ptr[n] = 0;
-		left = &ptr[n + 1];
-
-		while (*left && isspace(*left))
-			left++;
-
-		if (!strncmp(left, pci_id, strlen(pci_id))) {
-			found = 1;
-
-			while (*ptr && isspace(*ptr))
-				ptr++;
-
-			sscanf(ptr, "%04hx-%04hx", &start, &end);
-
-			break;
-		}
-	}
-
-	free(line);
-	fclose(fp);
-
-	if (!found)
-		return -1;
-
-	p->base = start;
-	RTE_LOG(DEBUG, EAL, "PCI Port IO found start=0x%x\n", start);
-
-	return 0;
-}
-#endif
-
 int
 rte_pci_ioport_map(struct rte_pci_device *dev, int bar,
 		struct rte_pci_ioport *p)
@@ -756,14 +691,8 @@ int rte_pci_write_config(const struct rte_pci_device *device,
 		break;
 #endif
 	case RTE_PCI_KDRV_IGB_UIO:
-		ret = pci_uio_ioport_map(dev, bar, p);
-		break;
 	case RTE_PCI_KDRV_UIO_GENERIC:
-#if defined(RTE_ARCH_X86)
-		ret = pci_ioport_map(dev, bar, p);
-#else
 		ret = pci_uio_ioport_map(dev, bar, p);
-#endif
 		break;
 	default:
 		break;
@@ -830,14 +759,8 @@ int rte_pci_write_config(const struct rte_pci_device *device,
 		break;
 #endif
 	case RTE_PCI_KDRV_IGB_UIO:
-		ret = pci_uio_ioport_unmap(p);
-		break;
 	case RTE_PCI_KDRV_UIO_GENERIC:
-#if defined(RTE_ARCH_X86)
-		ret = 0;
-#else
 		ret = pci_uio_ioport_unmap(p);
-#endif
 		break;
 	default:
 		break;
diff --git a/drivers/bus/pci/linux/pci_uio.c b/drivers/bus/pci/linux/pci_uio.c
index f3305a2..01f2a40 100644
--- a/drivers/bus/pci/linux/pci_uio.c
+++ b/drivers/bus/pci/linux/pci_uio.c
@@ -373,10 +373,13 @@
 pci_uio_ioport_map(struct rte_pci_device *dev, int bar,
 		   struct rte_pci_ioport *p)
 {
+	FILE *f = NULL;
 	char dirname[PATH_MAX];
 	char filename[PATH_MAX];
-	int uio_num;
-	unsigned long start;
+	char buf[BUFSIZ];
+	uint64_t phys_addr, end_addr, flags;
+	unsigned long base;
+	int i;
 
 	if (rte_eal_iopl_init() != 0) {
 		RTE_LOG(ERR, EAL, "%s(): insufficient ioport permissions for PCI device %s\n",
@@ -384,41 +387,66 @@
 		return -1;
 	}
 
-	uio_num = pci_get_uio_dev(dev, dirname, sizeof(dirname), 0);
-	if (uio_num < 0)
+	/* open and read addresses of the corresponding resource in sysfs */
+	snprintf(filename, sizeof(filename), "%s/" PCI_PRI_FMT "/resource",
+		rte_pci_get_sysfs_path(), dev->addr.domain, dev->addr.bus,
+		dev->addr.devid, dev->addr.function);
+	f = fopen(filename, "r");
+	if (f == NULL) {
+		RTE_LOG(ERR, EAL, "%s(): Cannot open sysfs resource: %s\n",
+			__func__, strerror(errno));
 		return -1;
+	}
 
-	/* get portio start */
-	snprintf(filename, sizeof(filename),
-		 "%s/portio/port%d/start", dirname, bar);
-	if (eal_parse_sysfs_value(filename, &start) < 0) {
-		RTE_LOG(ERR, EAL, "%s(): cannot parse portio start\n",
-			__func__);
-		return -1;
+	for (i = 0; i < bar + 1; i++) {
+		if (fgets(buf, sizeof(buf), f) == NULL) {
+			RTE_LOG(ERR, EAL, "%s(): Cannot read sysfs resource\n", __func__);
+			goto error;
+		}
 	}
-	/* ensure we don't get anything funny here, read/write will cast to
-	 * uin16_t */
-	if (start > UINT16_MAX)
-		return -1;
+	if (pci_parse_one_sysfs_resource(buf, sizeof(buf), &phys_addr,
+		&end_addr, &flags) < 0)
+		goto error;
+
+	if (!(flags & IORESOURCE_IO)) {
+		RTE_LOG(ERR, EAL, "%s(): bar resource other than IO is not supported\n", __func__);
+		goto error;
+	}
+	base = (unsigned long)phys_addr;
+	RTE_LOG(INFO, EAL, "%s(): PIO BAR %08lx detected\n", __func__, base);
+
+	if (base > UINT16_MAX)
+		goto error;
 
 	/* FIXME only for primary process ? */
 	if (dev->intr_handle.type == RTE_INTR_HANDLE_UNKNOWN) {
+		int uio_num = pci_get_uio_dev(dev, dirname, sizeof(dirname), 0);
+		if (uio_num < 0) {
+			RTE_LOG(ERR, EAL, "cannot open %s: %s\n",
+				dirname, strerror(errno));
+			goto error;
+		}
 
 		snprintf(filename, sizeof(filename), "/dev/uio%u", uio_num);
 		dev->intr_handle.fd = open(filename, O_RDWR);
 		if (dev->intr_handle.fd < 0) {
 			RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
 				filename, strerror(errno));
-			return -1;
+			goto error;
 		}
 		dev->intr_handle.type = RTE_INTR_HANDLE_UIO;
 	}
 
-	RTE_LOG(DEBUG, EAL, "PCI Port IO found start=0x%lx\n", start);
+	RTE_LOG(DEBUG, EAL, "PCI Port IO found start=0x%lx\n", base);
 
-	p->base = start;
+	p->base = base;
 	p->len = 0;
+	fclose(f);
 	return 0;
+error:
+	if (f)
+		fclose(f);
+	return -1;
 }
 #else
 int
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [dpdk-dev] [PATCH v6 2/2] bus/pci: support MMIO in PCI ioport accessors
  2021-01-29  3:18 [dpdk-dev] [PATCH v6 0/2] support both PIO and MMIO BAR for legacy device in virtio PMD 谢华伟(此时此刻)
  2021-01-29  3:18 ` [dpdk-dev] [PATCH v6 1/2] bus/pci: use PCI standard sysfs entry to get PIO address 谢华伟(此时此刻)
@ 2021-01-29  3:18 ` 谢华伟(此时此刻)
  2021-02-03  9:37   ` Maxime Coquelin
                     ` (2 more replies)
  2021-01-29  3:25 ` [dpdk-dev] [PATCH v6 0/2] support both PIO and MMIO BAR for legacy device in virtio PMD 谢华伟(此时此刻)
  2021-02-22 17:15 ` [dpdk-dev] [PATCH v7 " 谢华伟(此时此刻)
  3 siblings, 3 replies; 28+ messages in thread
From: 谢华伟(此时此刻) @ 2021-01-29  3:18 UTC (permalink / raw)
  To: ferruh.yigit, maxime.coquelin
  Cc: dev, david.marchand, anatoly.burakov, xuemingl, grive,
	chenbo.xia,
	谢华伟(此时此刻)

From: "huawei.xhw" <huawei.xhw@alibaba-inc.com>

With IO BAR, we get PIO(programmed IO) address.
With MMIO BAR, we get mapped virtual address.
We distinguish PIO(Programmed IO) and MMIO(memory mapped IO) by their address like how kernel does.
ioread/write8/16/32 is provided to access PIO/MMIO.
By the way, for virtio on arch other than x86, BAR flag indicates PIO but is mapped.

Signed-off-by: huawei xie <huawei.xhw@alibaba-inc.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/bus/pci/linux/pci.c     |   4 --
 drivers/bus/pci/linux/pci_uio.c | 124 ++++++++++++++++++++++++++--------------
 2 files changed, 81 insertions(+), 47 deletions(-)

diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index 0f38abf..0dc99e9 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -715,8 +715,6 @@ int rte_pci_write_config(const struct rte_pci_device *device,
 		break;
 #endif
 	case RTE_PCI_KDRV_IGB_UIO:
-		pci_uio_ioport_read(p, data, len, offset);
-		break;
 	case RTE_PCI_KDRV_UIO_GENERIC:
 		pci_uio_ioport_read(p, data, len, offset);
 		break;
@@ -736,8 +734,6 @@ int rte_pci_write_config(const struct rte_pci_device *device,
 		break;
 #endif
 	case RTE_PCI_KDRV_IGB_UIO:
-		pci_uio_ioport_write(p, data, len, offset);
-		break;
 	case RTE_PCI_KDRV_UIO_GENERIC:
 		pci_uio_ioport_write(p, data, len, offset);
 		break;
diff --git a/drivers/bus/pci/linux/pci_uio.c b/drivers/bus/pci/linux/pci_uio.c
index 01f2a40..78b2952 100644
--- a/drivers/bus/pci/linux/pci_uio.c
+++ b/drivers/bus/pci/linux/pci_uio.c
@@ -368,6 +368,8 @@
 	return -1;
 }
 
+#define PIO_MAX 0x10000
+
 #if defined(RTE_ARCH_X86)
 int
 pci_uio_ioport_map(struct rte_pci_device *dev, int bar,
@@ -381,12 +383,6 @@
 	unsigned long base;
 	int i;
 
-	if (rte_eal_iopl_init() != 0) {
-		RTE_LOG(ERR, EAL, "%s(): insufficient ioport permissions for PCI device %s\n",
-			__func__, dev->name);
-		return -1;
-	}
-
 	/* open and read addresses of the corresponding resource in sysfs */
 	snprintf(filename, sizeof(filename), "%s/" PCI_PRI_FMT "/resource",
 		rte_pci_get_sysfs_path(), dev->addr.domain, dev->addr.bus,
@@ -408,15 +404,27 @@
 		&end_addr, &flags) < 0)
 		goto error;
 
-	if (!(flags & IORESOURCE_IO)) {
-		RTE_LOG(ERR, EAL, "%s(): bar resource other than IO is not supported\n", __func__);
-		goto error;
-	}
-	base = (unsigned long)phys_addr;
-	RTE_LOG(INFO, EAL, "%s(): PIO BAR %08lx detected\n", __func__, base);
+	if (flags & IORESOURCE_IO) {
+		if (rte_eal_iopl_init()) {
+			RTE_LOG(ERR, EAL, "%s(): insufficient ioport permissions for PCI device %s\n",
+				__func__, dev->name);
+			goto error;
+		}
+
+		base = (unsigned long)phys_addr;
+		if (base > PIO_MAX) {
+			RTE_LOG(ERR, EAL, "%s(): %08lx too large PIO resource\n", __func__, base);
+			goto error;
+		}
 
-	if (base > UINT16_MAX)
+		RTE_LOG(DEBUG, EAL, "%s(): PIO BAR %08lx detected\n", __func__, base);
+	} else if (flags & IORESOURCE_MEM) {
+		base = (unsigned long)dev->mem_resource[bar].addr;
+		RTE_LOG(DEBUG, EAL, "%s(): MMIO BAR %08lx detected\n", __func__, base);
+	} else {
+		RTE_LOG(ERR, EAL, "%s(): unknown BAR type\n", __func__);
 		goto error;
+	}
 
 	/* FIXME only for primary process ? */
 	if (dev->intr_handle.type == RTE_INTR_HANDLE_UNKNOWN) {
@@ -517,6 +525,60 @@
 }
 #endif
 
+static inline uint8_t ioread8(void *addr)
+{
+	uint8_t val;
+
+	val = (uint64_t)(uintptr_t)addr >= PIO_MAX ?
+		*(volatile uint8_t *)addr :
+		inb((unsigned long)addr);
+
+	return val;
+}
+
+static inline uint16_t ioread16(void *addr)
+{
+	uint16_t val;
+
+	val = (uint64_t)(uintptr_t)addr >= PIO_MAX ?
+		*(volatile uint16_t *)addr :
+		inw((unsigned long)addr);
+
+	return val;
+}
+
+static inline uint32_t ioread32(void *addr)
+{
+	uint32_t val;
+
+	val = (uint64_t)(uintptr_t)addr >= PIO_MAX ?
+		*(volatile uint32_t *)addr :
+		inl((unsigned long)addr);
+
+	return val;
+}
+
+static inline void iowrite8(uint8_t val, void *addr)
+{
+	(uint64_t)(uintptr_t)addr >= PIO_MAX ?
+		*(volatile uint8_t *)addr = val :
+		outb(val, (unsigned long)addr);
+}
+
+static inline void iowrite16(uint16_t val, void *addr)
+{
+	(uint64_t)(uintptr_t)addr >= PIO_MAX ?
+		*(volatile uint16_t *)addr = val :
+		outw(val, (unsigned long)addr);
+}
+
+static inline void iowrite32(uint32_t val, void *addr)
+{
+	(uint64_t)(uintptr_t)addr >= PIO_MAX ?
+		*(volatile uint32_t *)addr = val :
+		outl(val, (unsigned long)addr);
+}
+
 void
 pci_uio_ioport_read(struct rte_pci_ioport *p,
 		    void *data, size_t len, off_t offset)
@@ -528,25 +590,13 @@
 	for (d = data; len > 0; d += size, reg += size, len -= size) {
 		if (len >= 4) {
 			size = 4;
-#if defined(RTE_ARCH_X86)
-			*(uint32_t *)d = inl(reg);
-#else
-			*(uint32_t *)d = *(volatile uint32_t *)reg;
-#endif
+			*(uint32_t *)d = ioread32((void *)reg);
 		} else if (len >= 2) {
 			size = 2;
-#if defined(RTE_ARCH_X86)
-			*(uint16_t *)d = inw(reg);
-#else
-			*(uint16_t *)d = *(volatile uint16_t *)reg;
-#endif
+			*(uint16_t *)d = ioread16((void *)reg);
 		} else {
 			size = 1;
-#if defined(RTE_ARCH_X86)
-			*d = inb(reg);
-#else
-			*d = *(volatile uint8_t *)reg;
-#endif
+			*d = ioread8((void *)reg);
 		}
 	}
 }
@@ -562,25 +612,13 @@
 	for (s = data; len > 0; s += size, reg += size, len -= size) {
 		if (len >= 4) {
 			size = 4;
-#if defined(RTE_ARCH_X86)
-			outl_p(*(const uint32_t *)s, reg);
-#else
-			*(volatile uint32_t *)reg = *(const uint32_t *)s;
-#endif
+			iowrite32(*(const uint32_t *)s, (void *)reg);
 		} else if (len >= 2) {
 			size = 2;
-#if defined(RTE_ARCH_X86)
-			outw_p(*(const uint16_t *)s, reg);
-#else
-			*(volatile uint16_t *)reg = *(const uint16_t *)s;
-#endif
+			iowrite16(*(const uint16_t *)s, (void *)reg);
 		} else {
 			size = 1;
-#if defined(RTE_ARCH_X86)
-			outb_p(*s, reg);
-#else
-			*(volatile uint8_t *)reg = *s;
-#endif
+			iowrite8(*s, (void *)reg);
 		}
 	}
 }
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-dev] [PATCH v6 0/2] support both PIO and MMIO BAR for legacy device in virtio PMD
  2021-01-29  3:18 [dpdk-dev] [PATCH v6 0/2] support both PIO and MMIO BAR for legacy device in virtio PMD 谢华伟(此时此刻)
  2021-01-29  3:18 ` [dpdk-dev] [PATCH v6 1/2] bus/pci: use PCI standard sysfs entry to get PIO address 谢华伟(此时此刻)
  2021-01-29  3:18 ` [dpdk-dev] [PATCH v6 2/2] bus/pci: support MMIO in PCI ioport accessors 谢华伟(此时此刻)
@ 2021-01-29  3:25 ` 谢华伟(此时此刻)
  2021-02-01  7:43   ` 谢华伟(此时此刻)
  2021-02-22 17:15 ` [dpdk-dev] [PATCH v7 " 谢华伟(此时此刻)
  3 siblings, 1 reply; 28+ messages in thread
From: 谢华伟(此时此刻) @ 2021-01-29  3:25 UTC (permalink / raw)
  To: ferruh.yigit, maxime.coquelin
  Cc: dev, david.marchand, anatoly.burakov, xuemingl, grive, chenbo.xia

Hi ferruh and maxime:

v6 changes:

send v6. Let us discuss if merge in this or early next release.

Sorry that forget to reply to previous message id.

>      - change to DEBUG level for IO bar detection in pci_uio_ioport_map
>      - rework the code in iobar branch
>      - fixes commit message format issue
>      - temporarily remove the 3rd patch for vfio path, leave it for future discusssion
>      - rework against virtio_pmd_rework_v2
>
>
> huawei.xhw (2):
>    bus/pci: use PCI standard sysfs entry to get PIO address
>    bus/pci: support MMIO in PCI ioport accessors
>
>   drivers/bus/pci/linux/pci.c     |  81 -------------------
>   drivers/bus/pci/linux/pci_uio.c | 170 ++++++++++++++++++++++++++++------------
>   2 files changed, 118 insertions(+), 133 deletions(-)
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-dev] [PATCH v6 0/2] support both PIO and MMIO BAR for legacy device in virtio PMD
  2021-01-29  3:25 ` [dpdk-dev] [PATCH v6 0/2] support both PIO and MMIO BAR for legacy device in virtio PMD 谢华伟(此时此刻)
@ 2021-02-01  7:43   ` 谢华伟(此时此刻)
  2021-02-03  9:37     ` Maxime Coquelin
  0 siblings, 1 reply; 28+ messages in thread
From: 谢华伟(此时此刻) @ 2021-02-01  7:43 UTC (permalink / raw)
  To: ferruh.yigit, maxime.coquelin
  Cc: dev, david.marchand, anatoly.burakov, xuemingl, grive, chenbo.xia


On 2021/1/29 11:25, chris wrote:
> Hi ferruh and maxime:
>
> v6 changes:
>
> send v6. Let us discuss if merge in this or early next release.

Ping.


>
> Sorry that forget to reply to previous message id.
>
>>      - change to DEBUG level for IO bar detection in pci_uio_ioport_map
>>      - rework the code in iobar branch
>>      - fixes commit message format issue
>>      - temporarily remove the 3rd patch for vfio path, leave it for 
>> future discusssion
>>      - rework against virtio_pmd_rework_v2
>>
>>
>> huawei.xhw (2):
>>    bus/pci: use PCI standard sysfs entry to get PIO address
>>    bus/pci: support MMIO in PCI ioport accessors
>>
>>   drivers/bus/pci/linux/pci.c     |  81 -------------------
>>   drivers/bus/pci/linux/pci_uio.c | 170 
>> ++++++++++++++++++++++++++++------------
>>   2 files changed, 118 insertions(+), 133 deletions(-)
>>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-dev] [PATCH v6 0/2] support both PIO and MMIO BAR for legacy device in virtio PMD
  2021-02-01  7:43   ` 谢华伟(此时此刻)
@ 2021-02-03  9:37     ` Maxime Coquelin
  2021-02-04  2:50       ` 谢华伟(此时此刻)
  0 siblings, 1 reply; 28+ messages in thread
From: Maxime Coquelin @ 2021-02-03  9:37 UTC (permalink / raw)
  To: 谢华伟(此时此刻),
	ferruh.yigit
  Cc: dev, david.marchand, anatoly.burakov, xuemingl, grive, chenbo.xia

Hi Huawei,

On 2/1/21 8:43 AM, 谢华伟(此时此刻) wrote:
> 
> On 2021/1/29 11:25, chris wrote:
>> Hi ferruh and maxime:
>>
>> v6 changes:
>>
>> send v6. Let us discuss if merge in this or early next release.
> 
> Ping.

The -rc2 was released on the 29th, so I think it is too late for this
release.

As Ferruh suggested, he can pick it early for next release, so doing
that we'll be sure to have a wider testing.

Apart from that, I am fine with the series.

Thanks,
Maxime

> 
>>
>> Sorry that forget to reply to previous message id.
>>
>>>      - change to DEBUG level for IO bar detection in pci_uio_ioport_map
>>>      - rework the code in iobar branch
>>>      - fixes commit message format issue
>>>      - temporarily remove the 3rd patch for vfio path, leave it for
>>> future discusssion
>>>      - rework against virtio_pmd_rework_v2
>>>
>>>
>>> huawei.xhw (2):
>>>    bus/pci: use PCI standard sysfs entry to get PIO address
>>>    bus/pci: support MMIO in PCI ioport accessors
>>>
>>>   drivers/bus/pci/linux/pci.c     |  81 -------------------
>>>   drivers/bus/pci/linux/pci_uio.c | 170
>>> ++++++++++++++++++++++++++++------------
>>>   2 files changed, 118 insertions(+), 133 deletions(-)
>>>
> 


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-dev] [PATCH v6 1/2] bus/pci: use PCI standard sysfs entry to get PIO address
  2021-01-29  3:18 ` [dpdk-dev] [PATCH v6 1/2] bus/pci: use PCI standard sysfs entry to get PIO address 谢华伟(此时此刻)
@ 2021-02-03  9:37   ` Maxime Coquelin
  2021-02-18  9:33   ` David Marchand
  1 sibling, 0 replies; 28+ messages in thread
From: Maxime Coquelin @ 2021-02-03  9:37 UTC (permalink / raw)
  To: 谢华伟(此时此刻),
	ferruh.yigit
  Cc: dev, david.marchand, anatoly.burakov, xuemingl, grive, chenbo.xia



On 1/29/21 4:18 AM, 谢华伟(此时此刻) wrote:
> From: "huawei.xhw" <huawei.xhw@alibaba-inc.com>
> 
> Currently virtio PMD asssumes legacy device uses PIO bar.
> There are three ways to get PIO(PortIO) address for virtio legacy device.
>     under igb_uio, get pio address from uio/uio# sysfs attribute
>     under uio_pci_generic:
>         for X86, get PIO address from /proc/ioport
>         for other ARCH, get PIO address from standard PCI sysfs attribute
> 
> Actually, igb_uio sysfs attribute exports exactly the same thing as standard PCI sysfs, i.e,
>     pci_dev->resource[]
> 
> This patch fixes these messy things, and uses standard PCI sysfs attribute.
> 
> Signed-off-by: huawei xie <huawei.xhw@alibaba-inc.com>
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  drivers/bus/pci/linux/pci.c     | 77 -----------------------------------------
>  drivers/bus/pci/linux/pci_uio.c | 64 ++++++++++++++++++++++++----------
>  2 files changed, 46 insertions(+), 95 deletions(-)
> 

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-dev] [PATCH v6 2/2] bus/pci: support MMIO in PCI ioport accessors
  2021-01-29  3:18 ` [dpdk-dev] [PATCH v6 2/2] bus/pci: support MMIO in PCI ioport accessors 谢华伟(此时此刻)
@ 2021-02-03  9:37   ` Maxime Coquelin
  2021-02-09 14:51   ` Ferruh Yigit
  2021-02-17  9:06   ` David Marchand
  2 siblings, 0 replies; 28+ messages in thread
From: Maxime Coquelin @ 2021-02-03  9:37 UTC (permalink / raw)
  To: 谢华伟(此时此刻),
	ferruh.yigit
  Cc: dev, david.marchand, anatoly.burakov, xuemingl, grive, chenbo.xia



On 1/29/21 4:18 AM, 谢华伟(此时此刻) wrote:
> |From: "huawei.xhw" <huawei.xhw@alibaba-inc.com> With IO BAR, we get
> PIO(programmed IO) address. With MMIO BAR, we get mapped virtual
> address. We distinguish PIO(Programmed IO) and MMIO(memory mapped IO) by
> their address like how kernel does. ioread/write8/16/32 is provided to
> access PIO/MMIO. By the way, for virtio on arch other than x86, BAR flag
> indicates PIO but is mapped. Signed-off-by: huawei xie
> <huawei.xhw@alibaba-inc.com> Reviewed-by: Maxime Coquelin
> <maxime.coquelin@redhat.com> --- drivers/bus/pci/linux/pci.c | 4 --
> drivers/bus/pci/linux/pci_uio.c | 124
> ++++++++++++++++++++++++++-------------- 2 files changed, 81
> insertions(+), 47 deletions(-)|

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-dev] [PATCH v6 0/2] support both PIO and MMIO BAR for legacy device in virtio PMD
  2021-02-03  9:37     ` Maxime Coquelin
@ 2021-02-04  2:50       ` 谢华伟(此时此刻)
  0 siblings, 0 replies; 28+ messages in thread
From: 谢华伟(此时此刻) @ 2021-02-04  2:50 UTC (permalink / raw)
  To: Maxime Coquelin, ferruh.yigit
  Cc: dev, david.marchand, anatoly.burakov, xuemingl, grive, chenbo.xia


On 2021/2/3 17:37, Maxime Coquelin wrote:
> Hi Huawei,
>
> On 2/1/21 8:43 AM, 谢华伟(此时此刻) wrote:
>> On 2021/1/29 11:25, chris wrote:
>>> Hi ferruh and maxime:
>>>
>>> v6 changes:
>>>
>>> send v6. Let us discuss if merge in this or early next release.
>> Ping.
> The -rc2 was released on the 29th, so I think it is too late for this
> release.
>
> As Ferruh suggested, he can pick it early for next release, so doing
> that we'll be sure to have a wider testing.
>
> Apart from that, I am fine with the series.
OK. Thanks Maxime.
>
> Thanks,
> Maxime
>
>>> Sorry that forget to reply to previous message id.
>>>
>>>>       - change to DEBUG level for IO bar detection in pci_uio_ioport_map
>>>>       - rework the code in iobar branch
>>>>       - fixes commit message format issue
>>>>       - temporarily remove the 3rd patch for vfio path, leave it for
>>>> future discusssion
>>>>       - rework against virtio_pmd_rework_v2
>>>>
>>>>
>>>> huawei.xhw (2):
>>>>     bus/pci: use PCI standard sysfs entry to get PIO address
>>>>     bus/pci: support MMIO in PCI ioport accessors
>>>>
>>>>    drivers/bus/pci/linux/pci.c     |  81 -------------------
>>>>    drivers/bus/pci/linux/pci_uio.c | 170
>>>> ++++++++++++++++++++++++++++------------
>>>>    2 files changed, 118 insertions(+), 133 deletions(-)
>>>>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-dev] [PATCH v6 2/2] bus/pci: support MMIO in PCI ioport accessors
  2021-01-29  3:18 ` [dpdk-dev] [PATCH v6 2/2] bus/pci: support MMIO in PCI ioport accessors 谢华伟(此时此刻)
  2021-02-03  9:37   ` Maxime Coquelin
@ 2021-02-09 14:51   ` Ferruh Yigit
  2021-02-19  8:52     ` Ferruh Yigit
  2021-02-17  9:06   ` David Marchand
  2 siblings, 1 reply; 28+ messages in thread
From: Ferruh Yigit @ 2021-02-09 14:51 UTC (permalink / raw)
  To: 谢华伟(此时此刻),
	maxime.coquelin
  Cc: dev, david.marchand, anatoly.burakov, xuemingl, grive, chenbo.xia

On 1/29/2021 3:18 AM, 谢华伟(此时此刻) wrote:
> From: "huawei.xhw" <huawei.xhw@alibaba-inc.com>
> 
> With IO BAR, we get PIO(programmed IO) address.
> With MMIO BAR, we get mapped virtual address.
> We distinguish PIO(Programmed IO) and MMIO(memory mapped IO) by their address like how kernel does.
> ioread/write8/16/32 is provided to access PIO/MMIO.
> By the way, for virtio on arch other than x86, BAR flag indicates PIO but is mapped.
> 
> Signed-off-by: huawei xie <huawei.xhw@alibaba-inc.com>
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

<...>

> +static inline void iowrite8(uint8_t val, void *addr)
> +{
> +	(uint64_t)(uintptr_t)addr >= PIO_MAX ?
> +		*(volatile uint8_t *)addr = val :
> +		outb(val, (unsigned long)addr);

Is the 'outb_p' to 'outb' conversion intentional? And if so why?

Same of the all 'outb_p', 'outw_p', 'outl_p'.

<...>

>   			size = 1;
> -#if defined(RTE_ARCH_X86)
> -			outb_p(*s, reg);
> -#else
> -			*(volatile uint8_t *)reg = *s;
> -#endif
> +			iowrite8(*s, (void *)reg);
>   		}
>   	}
>   }
> 


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-dev] [PATCH v6 2/2] bus/pci: support MMIO in PCI ioport accessors
  2021-01-29  3:18 ` [dpdk-dev] [PATCH v6 2/2] bus/pci: support MMIO in PCI ioport accessors 谢华伟(此时此刻)
  2021-02-03  9:37   ` Maxime Coquelin
  2021-02-09 14:51   ` Ferruh Yigit
@ 2021-02-17  9:06   ` David Marchand
  2021-02-17 14:15     ` 谢华伟(此时此刻)
  2 siblings, 1 reply; 28+ messages in thread
From: David Marchand @ 2021-02-17  9:06 UTC (permalink / raw)
  To: 谢华伟(此时此刻)
  Cc: Yigit, Ferruh, Maxime Coquelin, dev, Burakov, Anatoly, xuemingl,
	Gaetan Rivet, Xia, Chenbo

On Fri, Jan 29, 2021 at 4:19 AM 谢华伟(此时此刻) <huawei.xhw@alibaba-inc.com> wrote:
> @@ -517,6 +525,60 @@
>  }
>  #endif
>
> +static inline uint8_t ioread8(void *addr)
> +{
> +       uint8_t val;
> +
> +       val = (uint64_t)(uintptr_t)addr >= PIO_MAX ?
> +               *(volatile uint8_t *)addr :
> +               inb((unsigned long)addr);

inb/outb and others are architecture (x86?) specific.

The CI caught this issue, see build failures on ARM64.
https://lab.dpdk.org/results/dashboard/results/results-uploads/test_runs/82432e287bc94831b7a65d7cd6f05783/log_upload_file/2021/2/dpdk_b49c677a0d24_15433_2021-02-01_00-15-59_NA.zip

I can see the same issue with ppc64le.


--
David Marchand


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-dev] [PATCH v6 2/2] bus/pci: support MMIO in PCI ioport accessors
  2021-02-17  9:06   ` David Marchand
@ 2021-02-17 14:15     ` 谢华伟(此时此刻)
  2021-02-18  9:33       ` David Marchand
  0 siblings, 1 reply; 28+ messages in thread
From: 谢华伟(此时此刻) @ 2021-02-17 14:15 UTC (permalink / raw)
  To: David Marchand
  Cc: Yigit, Ferruh, Maxime Coquelin, dev, Burakov, Anatoly, xuemingl,
	Gaetan Rivet, Xia, Chenbo


On 2021/2/17 17:06, David Marchand wrote:
> On Fri, Jan 29, 2021 at 4:19 AM 谢华伟(此时此刻) <huawei.xhw@alibaba-inc.com> wrote:
>> @@ -517,6 +525,60 @@
>>   }
>>   #endif
>>
>> +static inline uint8_t ioread8(void *addr)
>> +{
>> +       uint8_t val;
>> +
>> +       val = (uint64_t)(uintptr_t)addr >= PIO_MAX ?
>> +               *(volatile uint8_t *)addr :
>> +               inb((unsigned long)addr);
> inb/outb and others are architecture (x86?) specific.
Yes, only X86 has PIO.
>
> The CI caught this issue, see build failures on ARM64.
> https://lab.dpdk.org/results/dashboard/results/results-uploads/test_runs/82432e287bc94831b7a65d7cd6f05783/log_upload_file/2021/2/dpdk_b49c677a0d24_15433_2021-02-01_00-15-59_NA.zip
>
> I can see the same issue with ppc64le.
>
>
> --
> David Marchand

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-dev] [PATCH v6 2/2] bus/pci: support MMIO in PCI ioport accessors
  2021-02-17 14:15     ` 谢华伟(此时此刻)
@ 2021-02-18  9:33       ` David Marchand
  0 siblings, 0 replies; 28+ messages in thread
From: David Marchand @ 2021-02-18  9:33 UTC (permalink / raw)
  To: 谢华伟(此时此刻)
  Cc: Yigit, Ferruh, Maxime Coquelin, dev, Burakov, Anatoly, xuemingl,
	Gaetan Rivet, Xia, Chenbo

On Wed, Feb 17, 2021 at 3:15 PM 谢华伟(此时此刻) <huawei.xhw@alibaba-inc.com> wrote:
>
>
> On 2021/2/17 17:06, David Marchand wrote:
> > On Fri, Jan 29, 2021 at 4:19 AM 谢华伟(此时此刻) <huawei.xhw@alibaba-inc.com> wrote:
> >> @@ -517,6 +525,60 @@
> >>   }
> >>   #endif
> >>
> >> +static inline uint8_t ioread8(void *addr)
> >> +{
> >> +       uint8_t val;
> >> +
> >> +       val = (uint64_t)(uintptr_t)addr >= PIO_MAX ?
> >> +               *(volatile uint8_t *)addr :
> >> +               inb((unsigned long)addr);
> > inb/outb and others are architecture (x86?) specific.
> Yes, only X86 has PIO.

Ok, thanks for confirming.

> >
> > The CI caught this issue, see build failures on ARM64.
> > https://lab.dpdk.org/results/dashboard/results/results-uploads/test_runs/82432e287bc94831b7a65d7cd6f05783/log_upload_file/2021/2/dpdk_b49c677a0d24_15433_2021-02-01_00-15-59_NA.zip
> >
> > I can see the same issue with ppc64le.

Just to be clear.
This series can't be merged as it breaks non-x86 architectures.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-dev] [PATCH v6 1/2] bus/pci: use PCI standard sysfs entry to get PIO address
  2021-01-29  3:18 ` [dpdk-dev] [PATCH v6 1/2] bus/pci: use PCI standard sysfs entry to get PIO address 谢华伟(此时此刻)
  2021-02-03  9:37   ` Maxime Coquelin
@ 2021-02-18  9:33   ` David Marchand
  2021-02-21 15:58     ` 谢华伟(此时此刻)
  1 sibling, 1 reply; 28+ messages in thread
From: David Marchand @ 2021-02-18  9:33 UTC (permalink / raw)
  To: 谢华伟(此时此刻)
  Cc: Yigit, Ferruh, Maxime Coquelin, dev, Burakov, Anatoly, xuemingl,
	Gaetan Rivet, Xia, Chenbo

On Fri, Jan 29, 2021 at 4:19 AM 谢华伟(此时此刻) <huawei.xhw@alibaba-inc.com> wrote:
>
> From: "huawei.xhw" <huawei.xhw@alibaba-inc.com>
>
> Currently virtio PMD asssumes legacy device uses PIO bar.
> There are three ways to get PIO(PortIO) address for virtio legacy device.
>     under igb_uio, get pio address from uio/uio# sysfs attribute
>     under uio_pci_generic:
>         for X86, get PIO address from /proc/ioport
>         for other ARCH, get PIO address from standard PCI sysfs attribute
>
> Actually, igb_uio sysfs attribute exports exactly the same thing as standard PCI sysfs, i.e,
>     pci_dev->resource[]
>
> This patch fixes these messy things, and uses standard PCI sysfs attribute.

I can not find what is being fixed.
Could you elaborate?


-- 
David Marchand


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-dev] [PATCH v6 2/2] bus/pci: support MMIO in PCI ioport accessors
  2021-02-09 14:51   ` Ferruh Yigit
@ 2021-02-19  8:52     ` Ferruh Yigit
  2021-02-21 15:45       ` 谢华伟(此时此刻)
  0 siblings, 1 reply; 28+ messages in thread
From: Ferruh Yigit @ 2021-02-19  8:52 UTC (permalink / raw)
  To: 谢华伟(此时此刻),
	maxime.coquelin
  Cc: dev, david.marchand, anatoly.burakov, xuemingl, grive, chenbo.xia

On 2/9/2021 2:51 PM, Ferruh Yigit wrote:
> On 1/29/2021 3:18 AM, 谢华伟(此时此刻) wrote:
>> From: "huawei.xhw" <huawei.xhw@alibaba-inc.com>
>>
>> With IO BAR, we get PIO(programmed IO) address.
>> With MMIO BAR, we get mapped virtual address.
>> We distinguish PIO(Programmed IO) and MMIO(memory mapped IO) by their address 
>> like how kernel does.
>> ioread/write8/16/32 is provided to access PIO/MMIO.
>> By the way, for virtio on arch other than x86, BAR flag indicates PIO but is 
>> mapped.
>>
>> Signed-off-by: huawei xie <huawei.xhw@alibaba-inc.com>
>> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> 
> <...>
> 
>> +static inline void iowrite8(uint8_t val, void *addr)
>> +{
>> +    (uint64_t)(uintptr_t)addr >= PIO_MAX ?
>> +        *(volatile uint8_t *)addr = val :
>> +        outb(val, (unsigned long)addr);
> 
> Is the 'outb_p' to 'outb' conversion intentional? And if so why?
> 
> Same of the all 'outb_p', 'outw_p', 'outl_p'.
> 

Reminder of above question.

Let's try to close this patch before release pressure hit again.
And as far as I understand already a new version is required for build errors on 
non x86 architectures.

> <...>
> 
>>               size = 1;
>> -#if defined(RTE_ARCH_X86)
>> -            outb_p(*s, reg);
>> -#else
>> -            *(volatile uint8_t *)reg = *s;
>> -#endif
>> +            iowrite8(*s, (void *)reg);
>>           }
>>       }
>>   }
>>
> 


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-dev] [PATCH v6 2/2] bus/pci: support MMIO in PCI ioport accessors
  2021-02-19  8:52     ` Ferruh Yigit
@ 2021-02-21 15:45       ` 谢华伟(此时此刻)
  0 siblings, 0 replies; 28+ messages in thread
From: 谢华伟(此时此刻) @ 2021-02-21 15:45 UTC (permalink / raw)
  To: Ferruh Yigit, maxime.coquelin, David Marchand
  Cc: dev, david.marchand, anatoly.burakov, xuemingl, grive, chenbo.xia


On 2021/2/19 16:52, Ferruh Yigit wrote:
> On 2/9/2021 2:51 PM, Ferruh Yigit wrote:
>> On 1/29/2021 3:18 AM, 谢华伟(此时此刻) wrote:
>>> From: "huawei.xhw" <huawei.xhw@alibaba-inc.com>
>>>
>>> With IO BAR, we get PIO(programmed IO) address.
>>> With MMIO BAR, we get mapped virtual address.
>>> We distinguish PIO(Programmed IO) and MMIO(memory mapped IO) by 
>>> their address like how kernel does.
>>> ioread/write8/16/32 is provided to access PIO/MMIO.
>>> By the way, for virtio on arch other than x86, BAR flag indicates 
>>> PIO but is mapped.
>>>
>>> Signed-off-by: huawei xie <huawei.xhw@alibaba-inc.com>
>>> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>>
>> <...>
>>
>>> +static inline void iowrite8(uint8_t val, void *addr)
>>> +{
>>> +    (uint64_t)(uintptr_t)addr >= PIO_MAX ?
>>> +        *(volatile uint8_t *)addr = val :
>>> +        outb(val, (unsigned long)addr);
>>
>> Is the 'outb_p' to 'outb' conversion intentional? And if so why?
>>
>> Same of the all 'outb_p', 'outw_p', 'outl_p'.
>>
>
> Reminder of above question.
>
> Let's try to close this patch before release pressure hit again.
> And as far as I understand already a new version is required for build 
> errors on non x86 architectures.

I will check how to fix the arch issue.

>
>> <...>
>>
>>>               size = 1;
>>> -#if defined(RTE_ARCH_X86)
>>> -            outb_p(*s, reg);
>>> -#else
>>> -            *(volatile uint8_t *)reg = *s;
>>> -#endif
>>> +            iowrite8(*s, (void *)reg);
>>>           }
>>>       }
>>>   }
>>>
>>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-dev] [PATCH v6 1/2] bus/pci: use PCI standard sysfs entry to get PIO address
  2021-02-18  9:33   ` David Marchand
@ 2021-02-21 15:58     ` 谢华伟(此时此刻)
  2021-02-24 12:49       ` David Marchand
  0 siblings, 1 reply; 28+ messages in thread
From: 谢华伟(此时此刻) @ 2021-02-21 15:58 UTC (permalink / raw)
  To: David Marchand
  Cc: Yigit, Ferruh, Maxime Coquelin, dev, Burakov, Anatoly, xuemingl,
	Gaetan Rivet, Xia, Chenbo


On 2021/2/18 17:33, David Marchand wrote:
> On Fri, Jan 29, 2021 at 4:19 AM 谢华伟(此时此刻) <huawei.xhw@alibaba-inc.com> wrote:
>> From: "huawei.xhw" <huawei.xhw@alibaba-inc.com>
>>
>> Currently virtio PMD asssumes legacy device uses PIO bar.
>> There are three ways to get PIO(PortIO) address for virtio legacy device.
>>      under igb_uio, get pio address from uio/uio# sysfs attribute
>>      under uio_pci_generic:
>>          for X86, get PIO address from /proc/ioport
>>          for other ARCH, get PIO address from standard PCI sysfs attribute
>>
>> Actually, igb_uio sysfs attribute exports exactly the same thing as standard PCI sysfs, i.e,
>>      pci_dev->resource[]
>>
>> This patch fixes these messy things, and uses standard PCI sysfs attribute.
> I can not find what is being fixed.
> Could you elaborate?

Maybe i wrongly use the word fix?  With this patch, different drivers 
use the same way to get IO address for virtio device.

Previously different driver get IO address with different method.

>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [dpdk-dev] [PATCH v7 0/2] support both PIO and MMIO BAR for legacy device in virtio PMD
  2021-01-29  3:18 [dpdk-dev] [PATCH v6 0/2] support both PIO and MMIO BAR for legacy device in virtio PMD 谢华伟(此时此刻)
                   ` (2 preceding siblings ...)
  2021-01-29  3:25 ` [dpdk-dev] [PATCH v6 0/2] support both PIO and MMIO BAR for legacy device in virtio PMD 谢华伟(此时此刻)
@ 2021-02-22 17:15 ` 谢华伟(此时此刻)
  2021-02-22 17:15   ` [dpdk-dev] [PATCH v7 1/2] bus/pci: use PCI standard sysfs entry to get PIO address 谢华伟(此时此刻)
  2021-02-22 17:15   ` [dpdk-dev] [PATCH v7 2/2] bus/pci: support MMIO in PCI ioport accessors 谢华伟(此时此刻)
  3 siblings, 2 replies; 28+ messages in thread
From: 谢华伟(此时此刻) @ 2021-02-22 17:15 UTC (permalink / raw)
  To: ferruh.yigit, maxime.coquelin, david.marchand
  Cc: dev, anatoly.burakov, xuemingl, grive, chenbo.xia,
	谢华伟(此时此刻)

From: "huawei.xhw" <huawei.xhw@alibaba-inc.com>

virtio PMD assumes legacy device only supports PIO BAR resource. This is wrong.
As we need to create lots of devices, as PIO resource on x86 is very limited, 
we expose MMIO(memory IO) BAR.

Kernel supports both PIO and MMIO BAR for legacy virtio-pci device, and for all
other pci devices. This patchset handles different type of BAR in the similar way.

In previous implementation, under igb_uio driver we get PIO address from igb_uio
sysfs entry; with uio_pci_generic, we get PIO address from /proc/ioports for x86,
and for other ARCHs, we get PIO address from standard PCI sysfs entry.
For PIO/MMIO RW, there is different path for different drivers and arch.


All of the above is too much twisted.
This patchset unifies the way to get both PIO and MMIO address for different driver
and ARCHs, all from standard resource attr under pci sysfs. This is most generic.

We distinguish PIO and MMIO by their address like how kernel does. It is ugly but works.

v2 changes:
    - add more explanation in the commit message

v3 changes:
    - fix patch format issues

v4 changes:
    - fixes for RTE_KDRV_UIO_GENERIC -> RTE_PCI_KDRV_UIO_GENERIC

v5 changes:
    - split into three seperate patches

v6 changes:
    - change to DEBUG level for IO bar detection in pci_uio_ioport_map
    - rework the code in iobar branch
    - fixes commit message format issue
    - temporarily remove the 3rd patch for vfio path, leave it for future discusssion
    - rework against virtio_pmd_rework_v2

v7 changes:
    - fix compilation issues of in/out instruction on non X86 archs

huawei.xhw (2):
  bus/pci: use PCI standard sysfs entry to get PIO address
  bus/pci: support MMIO in PCI ioport accessors

 drivers/bus/pci/linux/pci.c     |  81 ----------------
 drivers/bus/pci/linux/pci_uio.c | 202 +++++++++++++++++++++++++++++-----------
 2 files changed, 150 insertions(+), 133 deletions(-)

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [dpdk-dev] [PATCH v7 1/2] bus/pci: use PCI standard sysfs entry to get PIO address
  2021-02-22 17:15 ` [dpdk-dev] [PATCH v7 " 谢华伟(此时此刻)
@ 2021-02-22 17:15   ` 谢华伟(此时此刻)
  2021-02-22 17:15   ` [dpdk-dev] [PATCH v7 2/2] bus/pci: support MMIO in PCI ioport accessors 谢华伟(此时此刻)
  1 sibling, 0 replies; 28+ messages in thread
From: 谢华伟(此时此刻) @ 2021-02-22 17:15 UTC (permalink / raw)
  To: ferruh.yigit, maxime.coquelin, david.marchand
  Cc: dev, anatoly.burakov, xuemingl, grive, chenbo.xia,
	谢华伟(此时此刻)

From: "huawei.xhw" <huawei.xhw@alibaba-inc.com>

Currently virtio PMD asssumes legacy device uses PIO bar.
There are three ways to get PIO(PortIO) address for virtio legacy device.
    under igb_uio, get pio address from uio/uio# sysfs attribute
    under uio_pci_generic:
        for X86, get PIO address from /proc/ioport
        for other ARCH, get PIO address from standard PCI sysfs attribute

Actually, igb_uio sysfs attribute exports exactly the same thing as standard PCI sysfs, i.e,
    pci_dev->resource[]

This patch fixes these messy things, and uses standard PCI sysfs attribute.

Signed-off-by: huawei xie <huawei.xhw@alibaba-inc.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/bus/pci/linux/pci.c     | 77 -----------------------------------------
 drivers/bus/pci/linux/pci_uio.c | 64 ++++++++++++++++++++++++----------
 2 files changed, 46 insertions(+), 95 deletions(-)

diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index 2e1808b..0f38abf 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -677,71 +677,6 @@ int rte_pci_write_config(const struct rte_pci_device *device,
 	}
 }
 
-#if defined(RTE_ARCH_X86)
-static int
-pci_ioport_map(struct rte_pci_device *dev, int bar __rte_unused,
-		struct rte_pci_ioport *p)
-{
-	uint16_t start, end;
-	FILE *fp;
-	char *line = NULL;
-	char pci_id[16];
-	int found = 0;
-	size_t linesz;
-
-	if (rte_eal_iopl_init() != 0) {
-		RTE_LOG(ERR, EAL, "%s(): insufficient ioport permissions for PCI device %s\n",
-			__func__, dev->name);
-		return -1;
-	}
-
-	snprintf(pci_id, sizeof(pci_id), PCI_PRI_FMT,
-		 dev->addr.domain, dev->addr.bus,
-		 dev->addr.devid, dev->addr.function);
-
-	fp = fopen("/proc/ioports", "r");
-	if (fp == NULL) {
-		RTE_LOG(ERR, EAL, "%s(): can't open ioports\n", __func__);
-		return -1;
-	}
-
-	while (getdelim(&line, &linesz, '\n', fp) > 0) {
-		char *ptr = line;
-		char *left;
-		int n;
-
-		n = strcspn(ptr, ":");
-		ptr[n] = 0;
-		left = &ptr[n + 1];
-
-		while (*left && isspace(*left))
-			left++;
-
-		if (!strncmp(left, pci_id, strlen(pci_id))) {
-			found = 1;
-
-			while (*ptr && isspace(*ptr))
-				ptr++;
-
-			sscanf(ptr, "%04hx-%04hx", &start, &end);
-
-			break;
-		}
-	}
-
-	free(line);
-	fclose(fp);
-
-	if (!found)
-		return -1;
-
-	p->base = start;
-	RTE_LOG(DEBUG, EAL, "PCI Port IO found start=0x%x\n", start);
-
-	return 0;
-}
-#endif
-
 int
 rte_pci_ioport_map(struct rte_pci_device *dev, int bar,
 		struct rte_pci_ioport *p)
@@ -756,14 +691,8 @@ int rte_pci_write_config(const struct rte_pci_device *device,
 		break;
 #endif
 	case RTE_PCI_KDRV_IGB_UIO:
-		ret = pci_uio_ioport_map(dev, bar, p);
-		break;
 	case RTE_PCI_KDRV_UIO_GENERIC:
-#if defined(RTE_ARCH_X86)
-		ret = pci_ioport_map(dev, bar, p);
-#else
 		ret = pci_uio_ioport_map(dev, bar, p);
-#endif
 		break;
 	default:
 		break;
@@ -830,14 +759,8 @@ int rte_pci_write_config(const struct rte_pci_device *device,
 		break;
 #endif
 	case RTE_PCI_KDRV_IGB_UIO:
-		ret = pci_uio_ioport_unmap(p);
-		break;
 	case RTE_PCI_KDRV_UIO_GENERIC:
-#if defined(RTE_ARCH_X86)
-		ret = 0;
-#else
 		ret = pci_uio_ioport_unmap(p);
-#endif
 		break;
 	default:
 		break;
diff --git a/drivers/bus/pci/linux/pci_uio.c b/drivers/bus/pci/linux/pci_uio.c
index f3305a2..01f2a40 100644
--- a/drivers/bus/pci/linux/pci_uio.c
+++ b/drivers/bus/pci/linux/pci_uio.c
@@ -373,10 +373,13 @@
 pci_uio_ioport_map(struct rte_pci_device *dev, int bar,
 		   struct rte_pci_ioport *p)
 {
+	FILE *f = NULL;
 	char dirname[PATH_MAX];
 	char filename[PATH_MAX];
-	int uio_num;
-	unsigned long start;
+	char buf[BUFSIZ];
+	uint64_t phys_addr, end_addr, flags;
+	unsigned long base;
+	int i;
 
 	if (rte_eal_iopl_init() != 0) {
 		RTE_LOG(ERR, EAL, "%s(): insufficient ioport permissions for PCI device %s\n",
@@ -384,41 +387,66 @@
 		return -1;
 	}
 
-	uio_num = pci_get_uio_dev(dev, dirname, sizeof(dirname), 0);
-	if (uio_num < 0)
+	/* open and read addresses of the corresponding resource in sysfs */
+	snprintf(filename, sizeof(filename), "%s/" PCI_PRI_FMT "/resource",
+		rte_pci_get_sysfs_path(), dev->addr.domain, dev->addr.bus,
+		dev->addr.devid, dev->addr.function);
+	f = fopen(filename, "r");
+	if (f == NULL) {
+		RTE_LOG(ERR, EAL, "%s(): Cannot open sysfs resource: %s\n",
+			__func__, strerror(errno));
 		return -1;
+	}
 
-	/* get portio start */
-	snprintf(filename, sizeof(filename),
-		 "%s/portio/port%d/start", dirname, bar);
-	if (eal_parse_sysfs_value(filename, &start) < 0) {
-		RTE_LOG(ERR, EAL, "%s(): cannot parse portio start\n",
-			__func__);
-		return -1;
+	for (i = 0; i < bar + 1; i++) {
+		if (fgets(buf, sizeof(buf), f) == NULL) {
+			RTE_LOG(ERR, EAL, "%s(): Cannot read sysfs resource\n", __func__);
+			goto error;
+		}
 	}
-	/* ensure we don't get anything funny here, read/write will cast to
-	 * uin16_t */
-	if (start > UINT16_MAX)
-		return -1;
+	if (pci_parse_one_sysfs_resource(buf, sizeof(buf), &phys_addr,
+		&end_addr, &flags) < 0)
+		goto error;
+
+	if (!(flags & IORESOURCE_IO)) {
+		RTE_LOG(ERR, EAL, "%s(): bar resource other than IO is not supported\n", __func__);
+		goto error;
+	}
+	base = (unsigned long)phys_addr;
+	RTE_LOG(INFO, EAL, "%s(): PIO BAR %08lx detected\n", __func__, base);
+
+	if (base > UINT16_MAX)
+		goto error;
 
 	/* FIXME only for primary process ? */
 	if (dev->intr_handle.type == RTE_INTR_HANDLE_UNKNOWN) {
+		int uio_num = pci_get_uio_dev(dev, dirname, sizeof(dirname), 0);
+		if (uio_num < 0) {
+			RTE_LOG(ERR, EAL, "cannot open %s: %s\n",
+				dirname, strerror(errno));
+			goto error;
+		}
 
 		snprintf(filename, sizeof(filename), "/dev/uio%u", uio_num);
 		dev->intr_handle.fd = open(filename, O_RDWR);
 		if (dev->intr_handle.fd < 0) {
 			RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
 				filename, strerror(errno));
-			return -1;
+			goto error;
 		}
 		dev->intr_handle.type = RTE_INTR_HANDLE_UIO;
 	}
 
-	RTE_LOG(DEBUG, EAL, "PCI Port IO found start=0x%lx\n", start);
+	RTE_LOG(DEBUG, EAL, "PCI Port IO found start=0x%lx\n", base);
 
-	p->base = start;
+	p->base = base;
 	p->len = 0;
+	fclose(f);
 	return 0;
+error:
+	if (f)
+		fclose(f);
+	return -1;
 }
 #else
 int
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [dpdk-dev] [PATCH v7 2/2] bus/pci: support MMIO in PCI ioport accessors
  2021-02-22 17:15 ` [dpdk-dev] [PATCH v7 " 谢华伟(此时此刻)
  2021-02-22 17:15   ` [dpdk-dev] [PATCH v7 1/2] bus/pci: use PCI standard sysfs entry to get PIO address 谢华伟(此时此刻)
@ 2021-02-22 17:15   ` 谢华伟(此时此刻)
  2021-02-22 17:25     ` Ferruh Yigit
  1 sibling, 1 reply; 28+ messages in thread
From: 谢华伟(此时此刻) @ 2021-02-22 17:15 UTC (permalink / raw)
  To: ferruh.yigit, maxime.coquelin, david.marchand
  Cc: dev, anatoly.burakov, xuemingl, grive, chenbo.xia,
	谢华伟(此时此刻)

From: "huawei.xhw" <huawei.xhw@alibaba-inc.com>

With IO BAR, we get PIO(programmed IO) address.
With MMIO BAR, we get mapped virtual address.
We distinguish PIO(Programmed IO) and MMIO(memory mapped IO) by their address like how kernel does.
ioread/write8/16/32 is provided to access PIO/MMIO.
By the way, for virtio on arch other than x86, BAR flag indicates PIO but is mapped.

Signed-off-by: huawei xie <huawei.xhw@alibaba-inc.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/bus/pci/linux/pci.c     |   4 --
 drivers/bus/pci/linux/pci_uio.c | 156 +++++++++++++++++++++++++++++-----------
 2 files changed, 113 insertions(+), 47 deletions(-)

diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index 0f38abf..0dc99e9 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -715,8 +715,6 @@ int rte_pci_write_config(const struct rte_pci_device *device,
 		break;
 #endif
 	case RTE_PCI_KDRV_IGB_UIO:
-		pci_uio_ioport_read(p, data, len, offset);
-		break;
 	case RTE_PCI_KDRV_UIO_GENERIC:
 		pci_uio_ioport_read(p, data, len, offset);
 		break;
@@ -736,8 +734,6 @@ int rte_pci_write_config(const struct rte_pci_device *device,
 		break;
 #endif
 	case RTE_PCI_KDRV_IGB_UIO:
-		pci_uio_ioport_write(p, data, len, offset);
-		break;
 	case RTE_PCI_KDRV_UIO_GENERIC:
 		pci_uio_ioport_write(p, data, len, offset);
 		break;
diff --git a/drivers/bus/pci/linux/pci_uio.c b/drivers/bus/pci/linux/pci_uio.c
index 01f2a40..f566953 100644
--- a/drivers/bus/pci/linux/pci_uio.c
+++ b/drivers/bus/pci/linux/pci_uio.c
@@ -368,6 +368,8 @@
 	return -1;
 }
 
+#define PIO_MAX 0x10000
+
 #if defined(RTE_ARCH_X86)
 int
 pci_uio_ioport_map(struct rte_pci_device *dev, int bar,
@@ -381,12 +383,6 @@
 	unsigned long base;
 	int i;
 
-	if (rte_eal_iopl_init() != 0) {
-		RTE_LOG(ERR, EAL, "%s(): insufficient ioport permissions for PCI device %s\n",
-			__func__, dev->name);
-		return -1;
-	}
-
 	/* open and read addresses of the corresponding resource in sysfs */
 	snprintf(filename, sizeof(filename), "%s/" PCI_PRI_FMT "/resource",
 		rte_pci_get_sysfs_path(), dev->addr.domain, dev->addr.bus,
@@ -408,15 +404,27 @@
 		&end_addr, &flags) < 0)
 		goto error;
 
-	if (!(flags & IORESOURCE_IO)) {
-		RTE_LOG(ERR, EAL, "%s(): bar resource other than IO is not supported\n", __func__);
-		goto error;
-	}
-	base = (unsigned long)phys_addr;
-	RTE_LOG(INFO, EAL, "%s(): PIO BAR %08lx detected\n", __func__, base);
+	if (flags & IORESOURCE_IO) {
+		if (rte_eal_iopl_init()) {
+			RTE_LOG(ERR, EAL, "%s(): insufficient ioport permissions for PCI device %s\n",
+				__func__, dev->name);
+			goto error;
+		}
 
-	if (base > UINT16_MAX)
+		base = (unsigned long)phys_addr;
+		if (base > PIO_MAX) {
+			RTE_LOG(ERR, EAL, "%s(): %08lx too large PIO resource\n", __func__, base);
+			goto error;
+		}
+
+		RTE_LOG(DEBUG, EAL, "%s(): PIO BAR %08lx detected\n", __func__, base);
+	} else if (flags & IORESOURCE_MEM) {
+		base = (unsigned long)dev->mem_resource[bar].addr;
+		RTE_LOG(DEBUG, EAL, "%s(): MMIO BAR %08lx detected\n", __func__, base);
+	} else {
+		RTE_LOG(ERR, EAL, "%s(): unknown BAR type\n", __func__);
 		goto error;
+	}
 
 	/* FIXME only for primary process ? */
 	if (dev->intr_handle.type == RTE_INTR_HANDLE_UNKNOWN) {
@@ -517,6 +525,92 @@
 }
 #endif
 
+#if defined(RTE_ARCH_X86)
+static inline uint8_t ioread8(void *addr)
+{
+	uint8_t val;
+
+	val = (uint64_t)(uintptr_t)addr >= PIO_MAX ?
+		*(volatile uint8_t *)addr :
+		inb((unsigned long)addr);
+
+	return val;
+}
+
+static inline uint16_t ioread16(void *addr)
+{
+	uint16_t val;
+
+	val = (uint64_t)(uintptr_t)addr >= PIO_MAX ?
+		*(volatile uint16_t *)addr :
+		inw((unsigned long)addr);
+
+	return val;
+}
+
+static inline uint32_t ioread32(void *addr)
+{
+	uint32_t val;
+
+	val = (uint64_t)(uintptr_t)addr >= PIO_MAX ?
+		*(volatile uint32_t *)addr :
+		inl((unsigned long)addr);
+
+	return val;
+}
+
+static inline void iowrite8(uint8_t val, void *addr)
+{
+	(uint64_t)(uintptr_t)addr >= PIO_MAX ?
+		*(volatile uint8_t *)addr = val :
+		outb(val, (unsigned long)addr);
+}
+
+static inline void iowrite16(uint16_t val, void *addr)
+{
+	(uint64_t)(uintptr_t)addr >= PIO_MAX ?
+		*(volatile uint16_t *)addr = val :
+		outw(val, (unsigned long)addr);
+}
+
+static inline void iowrite32(uint32_t val, void *addr)
+{
+	(uint64_t)(uintptr_t)addr >= PIO_MAX ?
+		*(volatile uint32_t *)addr = val :
+		outl(val, (unsigned long)addr);
+}
+#else
+static inline uint8_t ioread8(void *addr)
+{
+	return *(volatile uint8_t *)addr;
+}
+
+static inline uint16_t ioread16(void *addr)
+{
+	return *(volatile uint16_t *)addr;
+}
+
+static inline uint32_t ioread32(void *addr)
+{
+	return *(volatile uint32_t *)addr;
+}
+
+static inline void iowrite8(uint8_t val, void *addr)
+{
+	*(volatile uint8_t *)addr = val;
+}
+
+static inline void iowrite16(uint16_t val, void *addr)
+{
+	*(volatile uint16_t *)addr = val;
+}
+
+static inline void iowrite32(uint32_t val, void *addr)
+{
+	*(volatile uint32_t *)addr = val;
+}
+#endif
+
 void
 pci_uio_ioport_read(struct rte_pci_ioport *p,
 		    void *data, size_t len, off_t offset)
@@ -528,25 +622,13 @@
 	for (d = data; len > 0; d += size, reg += size, len -= size) {
 		if (len >= 4) {
 			size = 4;
-#if defined(RTE_ARCH_X86)
-			*(uint32_t *)d = inl(reg);
-#else
-			*(uint32_t *)d = *(volatile uint32_t *)reg;
-#endif
+			*(uint32_t *)d = ioread32((void *)reg);
 		} else if (len >= 2) {
 			size = 2;
-#if defined(RTE_ARCH_X86)
-			*(uint16_t *)d = inw(reg);
-#else
-			*(uint16_t *)d = *(volatile uint16_t *)reg;
-#endif
+			*(uint16_t *)d = ioread16((void *)reg);
 		} else {
 			size = 1;
-#if defined(RTE_ARCH_X86)
-			*d = inb(reg);
-#else
-			*d = *(volatile uint8_t *)reg;
-#endif
+			*d = ioread8((void *)reg);
 		}
 	}
 }
@@ -562,25 +644,13 @@
 	for (s = data; len > 0; s += size, reg += size, len -= size) {
 		if (len >= 4) {
 			size = 4;
-#if defined(RTE_ARCH_X86)
-			outl_p(*(const uint32_t *)s, reg);
-#else
-			*(volatile uint32_t *)reg = *(const uint32_t *)s;
-#endif
+			iowrite32(*(const uint32_t *)s, (void *)reg);
 		} else if (len >= 2) {
 			size = 2;
-#if defined(RTE_ARCH_X86)
-			outw_p(*(const uint16_t *)s, reg);
-#else
-			*(volatile uint16_t *)reg = *(const uint16_t *)s;
-#endif
+			iowrite16(*(const uint16_t *)s, (void *)reg);
 		} else {
 			size = 1;
-#if defined(RTE_ARCH_X86)
-			outb_p(*s, reg);
-#else
-			*(volatile uint8_t *)reg = *s;
-#endif
+			iowrite8(*s, (void *)reg);
 		}
 	}
 }
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-dev] [PATCH v7 2/2] bus/pci: support MMIO in PCI ioport accessors
  2021-02-22 17:15   ` [dpdk-dev] [PATCH v7 2/2] bus/pci: support MMIO in PCI ioport accessors 谢华伟(此时此刻)
@ 2021-02-22 17:25     ` Ferruh Yigit
  2021-02-23 14:20       ` 谢华伟(此时此刻)
  0 siblings, 1 reply; 28+ messages in thread
From: Ferruh Yigit @ 2021-02-22 17:25 UTC (permalink / raw)
  To: 谢华伟(此时此刻),
	maxime.coquelin, david.marchand
  Cc: dev, anatoly.burakov, xuemingl, grive, chenbo.xia

On 2/22/2021 5:15 PM, 谢华伟(此时此刻) wrote:
> From: "huawei.xhw" <huawei.xhw@alibaba-inc.com>
> 
> With IO BAR, we get PIO(programmed IO) address.
> With MMIO BAR, we get mapped virtual address.
> We distinguish PIO(Programmed IO) and MMIO(memory mapped IO) by their address like how kernel does.
> ioread/write8/16/32 is provided to access PIO/MMIO.
> By the way, for virtio on arch other than x86, BAR flag indicates PIO but is mapped.
> 
> Signed-off-by: huawei xie <huawei.xhw@alibaba-inc.com>
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

<...>

> +
> +static inline void iowrite8(uint8_t val, void *addr)
> +{
> +	(uint64_t)(uintptr_t)addr >= PIO_MAX ?
> +		*(volatile uint8_t *)addr = val :
> +		outb(val, (unsigned long)addr);

//copying question from previous version:

Is the 'outb_p' to 'outb' conversion intentional? And if so why?

Same of the all 'outb_p', 'outw_p', 'outl_p'.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-dev] [PATCH v7 2/2] bus/pci: support MMIO in PCI ioport accessors
  2021-02-22 17:25     ` Ferruh Yigit
@ 2021-02-23 14:20       ` 谢华伟(此时此刻)
  2021-02-24 15:45         ` Ferruh Yigit
  0 siblings, 1 reply; 28+ messages in thread
From: 谢华伟(此时此刻) @ 2021-02-23 14:20 UTC (permalink / raw)
  To: Ferruh Yigit, maxime.coquelin, david.marchand
  Cc: dev, anatoly.burakov, xuemingl, grive, chenbo.xia


On 2021/2/23 1:25, Ferruh Yigit wrote:
> On 2/22/2021 5:15 PM, 谢华伟(此时此刻) wrote:
>> From: "huawei.xhw" <huawei.xhw@alibaba-inc.com>
>>
>> With IO BAR, we get PIO(programmed IO) address.
>> With MMIO BAR, we get mapped virtual address.
>> We distinguish PIO(Programmed IO) and MMIO(memory mapped IO) by their 
>> address like how kernel does.
>> ioread/write8/16/32 is provided to access PIO/MMIO.
>> By the way, for virtio on arch other than x86, BAR flag indicates PIO 
>> but is mapped.
>>
>> Signed-off-by: huawei xie <huawei.xhw@alibaba-inc.com>
>> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>
> <...>
>
>> +
>> +static inline void iowrite8(uint8_t val, void *addr)
>> +{
>> +    (uint64_t)(uintptr_t)addr >= PIO_MAX ?
>> +        *(volatile uint8_t *)addr = val :
>> +        outb(val, (unsigned long)addr);
>
> //copying question from previous version:
>
> Is the 'outb_p' to 'outb' conversion intentional? And if so why?
>
> Same of the all 'outb_p', 'outw_p', 'outl_p'.

There is no need to delay for virtio device, as we can see in virtio 
legacy driver.

IMO, the delay is for ugly old device. The device itself should assure 
the previous IO completes when the subsequent IO instruction arrives.



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-dev] [PATCH v6 1/2] bus/pci: use PCI standard sysfs entry to get PIO address
  2021-02-21 15:58     ` 谢华伟(此时此刻)
@ 2021-02-24 12:49       ` David Marchand
  2021-02-24 15:29         ` 谢华伟(此时此刻)
  0 siblings, 1 reply; 28+ messages in thread
From: David Marchand @ 2021-02-24 12:49 UTC (permalink / raw)
  To: 谢华伟(此时此刻)
  Cc: Yigit, Ferruh, Maxime Coquelin, dev, Burakov, Anatoly, xuemingl,
	Gaetan Rivet, Xia, Chenbo

On Sun, Feb 21, 2021 at 4:58 PM 谢华伟(此时此刻) <huawei.xhw@alibaba-inc.com> wrote:
>
>
> On 2021/2/18 17:33, David Marchand wrote:
> > On Fri, Jan 29, 2021 at 4:19 AM 谢华伟(此时此刻) <huawei.xhw@alibaba-inc.com> wrote:
> >> From: "huawei.xhw" <huawei.xhw@alibaba-inc.com>
> >>
> >> Currently virtio PMD asssumes legacy device uses PIO bar.
> >> There are three ways to get PIO(PortIO) address for virtio legacy device.
> >>      under igb_uio, get pio address from uio/uio# sysfs attribute
> >>      under uio_pci_generic:
> >>          for X86, get PIO address from /proc/ioport
> >>          for other ARCH, get PIO address from standard PCI sysfs attribute
> >>
> >> Actually, igb_uio sysfs attribute exports exactly the same thing as standard PCI sysfs, i.e,
> >>      pci_dev->resource[]
> >>
> >> This patch fixes these messy things, and uses standard PCI sysfs attribute.
> > I can not find what is being fixed.
> > Could you elaborate?
>
> Maybe i wrongly use the word fix?  With this patch, different drivers
> use the same way to get IO address for virtio device.
> Previously different driver get IO address with different method.

The commitlog is confusing, mentioning pci_dev->resource[] (I guess
from the kernel sources?) and talking about a fix while it works fine
so far.
So this is a refactoring.

Did you check that virtio devices bound to uio_pci_generic still works
with legacy mode + PIO?


-- 
David Marchand


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-dev] [PATCH v6 1/2] bus/pci: use PCI standard sysfs entry to get PIO address
  2021-02-24 12:49       ` David Marchand
@ 2021-02-24 15:29         ` 谢华伟(此时此刻)
  2021-02-24 17:52           ` David Marchand
  0 siblings, 1 reply; 28+ messages in thread
From: 谢华伟(此时此刻) @ 2021-02-24 15:29 UTC (permalink / raw)
  To: David Marchand
  Cc: Yigit, Ferruh, Maxime Coquelin, dev, Burakov, Anatoly, xuemingl,
	Gaetan Rivet, Xia, Chenbo


On 2021/2/24 20:49, David Marchand wrote:
> On Sun, Feb 21, 2021 at 4:58 PM 谢华伟(此时此刻) <huawei.xhw@alibaba-inc.com> wrote:
>>
>> On 2021/2/18 17:33, David Marchand wrote:
>>> On Fri, Jan 29, 2021 at 4:19 AM 谢华伟(此时此刻) <huawei.xhw@alibaba-inc.com> wrote:
>>>> From: "huawei.xhw" <huawei.xhw@alibaba-inc.com>
>>>>
>>>> Currently virtio PMD asssumes legacy device uses PIO bar.
>>>> There are three ways to get PIO(PortIO) address for virtio legacy device.
>>>>       under igb_uio, get pio address from uio/uio# sysfs attribute
>>>>       under uio_pci_generic:
>>>>           for X86, get PIO address from /proc/ioport
>>>>           for other ARCH, get PIO address from standard PCI sysfs attribute
>>>>
>>>> Actually, igb_uio sysfs attribute exports exactly the same thing as standard PCI sysfs, i.e,
>>>>       pci_dev->resource[]
>>>>
>>>> This patch fixes these messy things, and uses standard PCI sysfs attribute.
>>> I can not find what is being fixed.
>>> Could you elaborate?
>> Maybe i wrongly use the word fix?  With this patch, different drivers
>> use the same way to get IO address for virtio device.
>> Previously different driver get IO address with different method.
> The commitlog is confusing, mentioning pci_dev->resource[] (I guess
> from the kernel sources?) and talking about a fix while it works fine
Yes.
> so far.
> So this is a refactoring.
Ok, will change the word fix.
>
> Did you check that virtio devices bound to uio_pci_generic still works
> with legacy mode + PIO?

I had verified PIO, might under igb_uio driver.

Theoretically it works with uio_pci_generic driver. I could do the test 
if needed. Give me some time.  Have to create a legacy VM. Now bars in 
virtio device in the cloud are basically mmio.

>
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-dev] [PATCH v7 2/2] bus/pci: support MMIO in PCI ioport accessors
  2021-02-23 14:20       ` 谢华伟(此时此刻)
@ 2021-02-24 15:45         ` Ferruh Yigit
  2021-02-25  3:59           ` 谢华伟(此时此刻)
  0 siblings, 1 reply; 28+ messages in thread
From: Ferruh Yigit @ 2021-02-24 15:45 UTC (permalink / raw)
  To: 谢华伟(此时此刻),
	maxime.coquelin, david.marchand
  Cc: dev, anatoly.burakov, xuemingl, grive, chenbo.xia

On 2/23/2021 2:20 PM, 谢华伟(此时此刻) wrote:
> 
> On 2021/2/23 1:25, Ferruh Yigit wrote:
>> On 2/22/2021 5:15 PM, 谢华伟(此时此刻) wrote:
>>> From: "huawei.xhw" <huawei.xhw@alibaba-inc.com>
>>>
>>> With IO BAR, we get PIO(programmed IO) address.
>>> With MMIO BAR, we get mapped virtual address.
>>> We distinguish PIO(Programmed IO) and MMIO(memory mapped IO) by their address 
>>> like how kernel does.
>>> ioread/write8/16/32 is provided to access PIO/MMIO.
>>> By the way, for virtio on arch other than x86, BAR flag indicates PIO but is 
>>> mapped.
>>>
>>> Signed-off-by: huawei xie <huawei.xhw@alibaba-inc.com>
>>> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>>
>> <...>
>>
>>> +
>>> +static inline void iowrite8(uint8_t val, void *addr)
>>> +{
>>> +    (uint64_t)(uintptr_t)addr >= PIO_MAX ?
>>> +        *(volatile uint8_t *)addr = val :
>>> +        outb(val, (unsigned long)addr);
>>
>> //copying question from previous version:
>>
>> Is the 'outb_p' to 'outb' conversion intentional? And if so why?
>>
>> Same of the all 'outb_p', 'outw_p', 'outl_p'.
> 
> There is no need to delay for virtio device, as we can see in virtio legacy driver.
> 
> IMO, the delay is for ugly old device. The device itself should assure the 
> previous IO completes when the subsequent IO instruction arrives.
> 

Can there be any virtio legacy device needing this?

What is the downside of using "pause until the I/O completes" versions?

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-dev] [PATCH v6 1/2] bus/pci: use PCI standard sysfs entry to get PIO address
  2021-02-24 15:29         ` 谢华伟(此时此刻)
@ 2021-02-24 17:52           ` David Marchand
  0 siblings, 0 replies; 28+ messages in thread
From: David Marchand @ 2021-02-24 17:52 UTC (permalink / raw)
  To: 谢华伟(此时此刻)
  Cc: Yigit, Ferruh, Maxime Coquelin, dev, Burakov, Anatoly, xuemingl,
	Gaetan Rivet, Xia, Chenbo

On Wed, Feb 24, 2021 at 4:29 PM 谢华伟(此时此刻) <huawei.xhw@alibaba-inc.com> wrote:
> > Did you check that virtio devices bound to uio_pci_generic still works
> > with legacy mode + PIO?
>
> I had verified PIO, might under igb_uio driver.

Well, if you are unsure, please retest both cases, igb_uio and uio_pci_generic.


>
> Theoretically it works with uio_pci_generic driver. I could do the test
> if needed. Give me some time.  Have to create a legacy VM. Now bars in
> virtio device in the cloud are basically mmio.

Sure, take the time to check.
I'll wait for your confirmation, thanks.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-dev] [PATCH v7 2/2] bus/pci: support MMIO in PCI ioport accessors
  2021-02-24 15:45         ` Ferruh Yigit
@ 2021-02-25  3:59           ` 谢华伟(此时此刻)
  2021-02-25  9:52             ` David Marchand
  0 siblings, 1 reply; 28+ messages in thread
From: 谢华伟(此时此刻) @ 2021-02-25  3:59 UTC (permalink / raw)
  To: Ferruh Yigit, maxime.coquelin, david.marchand
  Cc: dev, anatoly.burakov, xuemingl, grive, chenbo.xia


On 2021/2/24 23:45, Ferruh Yigit wrote:
> On 2/23/2021 2:20 PM, 谢华伟(此时此刻) wrote:
>>
>> On 2021/2/23 1:25, Ferruh Yigit wrote:
>>> On 2/22/2021 5:15 PM, 谢华伟(此时此刻) wrote:
>>>> From: "huawei.xhw" <huawei.xhw@alibaba-inc.com>
>>>>
>>>> With IO BAR, we get PIO(programmed IO) address.
>>>> With MMIO BAR, we get mapped virtual address.
>>>> We distinguish PIO(Programmed IO) and MMIO(memory mapped IO) by 
>>>> their address like how kernel does.
>>>> ioread/write8/16/32 is provided to access PIO/MMIO.
>>>> By the way, for virtio on arch other than x86, BAR flag indicates 
>>>> PIO but is mapped.
>>>>
>>>> Signed-off-by: huawei xie <huawei.xhw@alibaba-inc.com>
>>>> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>>>
>>> <...>
>>>
>>>> +
>>>> +static inline void iowrite8(uint8_t val, void *addr)
>>>> +{
>>>> +    (uint64_t)(uintptr_t)addr >= PIO_MAX ?
>>>> +        *(volatile uint8_t *)addr = val :
>>>> +        outb(val, (unsigned long)addr);
>>>
>>> //copying question from previous version:
>>>
>>> Is the 'outb_p' to 'outb' conversion intentional? And if so why?
>>>
>>> Same of the all 'outb_p', 'outw_p', 'outl_p'.
>>
>> There is no need to delay for virtio device, as we can see in virtio 
>> legacy driver.
>>
>> IMO, the delay is for ugly old device. The device itself should 
>> assure the previous IO completes when the subsequent IO instruction 
>> arrives.
>>
>
> Can there be any virtio legacy device needing this?

The pause version delays sometime by writing to 0x80 debug port. virtio 
doesn't need this. virtio legacy PMD driver doens't use this.

Any device relying on this i think is buggy. How could the device rely 
on some uncertain cpu cycles to behave correct?

>
> What is the downside of using "pause until the I/O completes" versions?

The downside in virtio PMD is a small performance penalty when we use it 
to notify backend. CPU executes unnecessary serializing IO instruction.

I check kernel code, io wrapper for in/out doesn't use p version.


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-dev] [PATCH v7 2/2] bus/pci: support MMIO in PCI ioport accessors
  2021-02-25  3:59           ` 谢华伟(此时此刻)
@ 2021-02-25  9:52             ` David Marchand
  0 siblings, 0 replies; 28+ messages in thread
From: David Marchand @ 2021-02-25  9:52 UTC (permalink / raw)
  To: 谢华伟(此时此刻)
  Cc: Ferruh Yigit, Maxime Coquelin, dev, Burakov, Anatoly, xuemingl,
	Gaetan Rivet, Xia, Chenbo

On Thu, Feb 25, 2021 at 5:00 AM 谢华伟(此时此刻) <huawei.xhw@alibaba-inc.com> wrote:
> >>> Is the 'outb_p' to 'outb' conversion intentional? And if so why?
> >>>
> >>> Same of the all 'outb_p', 'outw_p', 'outl_p'.
> >>
> >> There is no need to delay for virtio device, as we can see in virtio
> >> legacy driver.
> >>
> >> IMO, the delay is for ugly old device. The device itself should
> >> assure the previous IO completes when the subsequent IO instruction
> >> arrives.
> >>
> >
> > Can there be any virtio legacy device needing this?
>
> The pause version delays sometime by writing to 0x80 debug port. virtio
> doesn't need this. virtio legacy PMD driver doens't use this.
>
> Any device relying on this i think is buggy. How could the device rely
> on some uncertain cpu cycles to behave correct?
>
> >
> > What is the downside of using "pause until the I/O completes" versions?
>
> The downside in virtio PMD is a small performance penalty when we use it
> to notify backend. CPU executes unnecessary serializing IO instruction.
>
> I check kernel code, io wrapper for in/out doesn't use p version.

This change is a fix/optimisation.
This is a separate topic from adding MMIO support with x86 ioport.
I would split as a separate patch.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2021-02-25  9:52 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-29  3:18 [dpdk-dev] [PATCH v6 0/2] support both PIO and MMIO BAR for legacy device in virtio PMD 谢华伟(此时此刻)
2021-01-29  3:18 ` [dpdk-dev] [PATCH v6 1/2] bus/pci: use PCI standard sysfs entry to get PIO address 谢华伟(此时此刻)
2021-02-03  9:37   ` Maxime Coquelin
2021-02-18  9:33   ` David Marchand
2021-02-21 15:58     ` 谢华伟(此时此刻)
2021-02-24 12:49       ` David Marchand
2021-02-24 15:29         ` 谢华伟(此时此刻)
2021-02-24 17:52           ` David Marchand
2021-01-29  3:18 ` [dpdk-dev] [PATCH v6 2/2] bus/pci: support MMIO in PCI ioport accessors 谢华伟(此时此刻)
2021-02-03  9:37   ` Maxime Coquelin
2021-02-09 14:51   ` Ferruh Yigit
2021-02-19  8:52     ` Ferruh Yigit
2021-02-21 15:45       ` 谢华伟(此时此刻)
2021-02-17  9:06   ` David Marchand
2021-02-17 14:15     ` 谢华伟(此时此刻)
2021-02-18  9:33       ` David Marchand
2021-01-29  3:25 ` [dpdk-dev] [PATCH v6 0/2] support both PIO and MMIO BAR for legacy device in virtio PMD 谢华伟(此时此刻)
2021-02-01  7:43   ` 谢华伟(此时此刻)
2021-02-03  9:37     ` Maxime Coquelin
2021-02-04  2:50       ` 谢华伟(此时此刻)
2021-02-22 17:15 ` [dpdk-dev] [PATCH v7 " 谢华伟(此时此刻)
2021-02-22 17:15   ` [dpdk-dev] [PATCH v7 1/2] bus/pci: use PCI standard sysfs entry to get PIO address 谢华伟(此时此刻)
2021-02-22 17:15   ` [dpdk-dev] [PATCH v7 2/2] bus/pci: support MMIO in PCI ioport accessors 谢华伟(此时此刻)
2021-02-22 17:25     ` Ferruh Yigit
2021-02-23 14:20       ` 谢华伟(此时此刻)
2021-02-24 15:45         ` Ferruh Yigit
2021-02-25  3:59           ` 谢华伟(此时此刻)
2021-02-25  9:52             ` David Marchand

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git