[dpdk-stable] [PATCH 1/2] eal/bsd: fix ioport write operation

[dpdk-stable] [PATCH 1/2] eal/bsd: fix ioport write operation

[Tiwei Bie]

The first param of out*() on FreeBSD is port, and the second one
is data. But they are reversed in DPDK. This patch fixes it.

Fixes: 756ce64b1ecd ("eal: introduce PCI ioport API")
Cc: stable@dpdk.org

Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
---
 lib/librte_eal/bsdapp/eal/eal_pci.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
index 6294b7eab..59ceb7665 100644
--- a/lib/librte_eal/bsdapp/eal/eal_pci.c
+++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
@@ -618,13 +618,13 @@ pci_uio_ioport_write(struct rte_pci_ioport *p,
 	for (s = data; len > 0; s += size, reg += size, len -= size) {
 		if (len >= 4) {
 			size = 4;
-			outl(*(const uint32_t *)s, reg);
+			outl(reg, *(const uint32_t *)s);
 		} else if (len >= 2) {
 			size = 2;
-			outw(*(const uint16_t *)s, reg);
+			outw(reg, *(const uint16_t *)s);
 		} else {
 			size = 1;
-			outb(*s, reg);
+			outb(reg, *s);
 		}
 	}
 #else
-- 
2.12.1

[dpdk-stable] [PATCH 2/2] eal/bsd: fix the read operation on PCI configuration space

[Tiwei Bie]

Some drivers (such as virtio) may need to read more than 4 bytes
data from PCI configuration space via rte_eal_pci_read_config().
But it will return with an error on FreeBSD when the expected
data length is bigger than the size of pi.pi_data whose type is
u_int32_t. This patch removes this limitation.

Fixes: 632b2d1deeed ("eal: provide functions to access PCI config")
Cc: stable@dpdk.org

Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
---
 lib/librte_eal/bsdapp/eal/eal_pci.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
index 59ceb7665..e321461d8 100644
--- a/lib/librte_eal/bsdapp/eal/eal_pci.c
+++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
@@ -459,6 +459,7 @@ int rte_pci_read_config(const struct rte_pci_device *dev,
 		void *buf, size_t len, off_t offset)
 {
 	int fd = -1;
+	int size;
 	struct pci_io pi = {
 		.pi_sel = {
 			.pc_domain = dev->addr.domain,
@@ -467,25 +468,28 @@ int rte_pci_read_config(const struct rte_pci_device *dev,
 			.pc_func = dev->addr.function,
 		},
 		.pi_reg = offset,
-		.pi_width = len,
 	};
 
-	if (len == 3 || len > sizeof(pi.pi_data)) {
-		RTE_LOG(ERR, EAL, "%s(): invalid pci read length\n", __func__);
-		goto error;
-	}
-
 	fd = open("/dev/pci", O_RDWR);
 	if (fd < 0) {
 		RTE_LOG(ERR, EAL, "%s(): error opening /dev/pci\n", __func__);
 		goto error;
 	}
 
-	if (ioctl(fd, PCIOCREAD, &pi) < 0)
-		goto error;
+	while (len > 0) {
+		size = (len >= 4) ? 4 : ((len >= 2) ? 2 : 1);
+		pi.pi_width = size;
+
+		if (ioctl(fd, PCIOCREAD, &pi) < 0)
+			goto error;
+		memcpy(buf, &pi.pi_data, size);
+
+		buf = (char *)buf + size;
+		pi.pi_reg += size;
+		len -= size;
+	}
 	close(fd);
 
-	memcpy(buf, &pi.pi_data, len);
 	return 0;
 
  error:
-- 
2.12.1

Re: [dpdk-stable] [PATCH 1/2] eal/bsd: fix ioport write operation

[Bruce Richardson]

On Sun, May 07, 2017 at 01:33:33PM +0000, Tiwei Bie wrote:
> The first param of out*() on FreeBSD is port, and the second one is
> data. But they are reversed in DPDK. This patch fixes it.
> 
> Fixes: 756ce64b1ecd ("eal: introduce PCI ioport API") Cc:
> stable@dpdk.org
> 
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com> ---
How was this bug discovered so that we can verify that it is fixed?.  Is
this in use by virtio or was it just discovered via code inspection?

/Bruce

Re: [dpdk-stable] [PATCH 1/2] eal/bsd: fix ioport write operation

[Tiwei Bie]

On Mon, May 08, 2017 at 09:55:01AM +0100, Bruce Richardson wrote:
> On Sun, May 07, 2017 at 01:33:33PM +0000, Tiwei Bie wrote:
> > The first param of out*() on FreeBSD is port, and the second one is
> > data. But they are reversed in DPDK. This patch fixes it.
> > 
> > Fixes: 756ce64b1ecd ("eal: introduce PCI ioport API") Cc:
> > stable@dpdk.org
> > 
> > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com> ---
> How was this bug discovered so that we can verify that it is fixed?.  Is
> this in use by virtio or was it just discovered via code inspection?
> 

The virtio PMD in legacy mode doesn't work FreeBSD, and I tried to
fix this issue. And then I found this bug.

Best regards,
Tiwei Bie

Re: [dpdk-stable] [PATCH 1/2] eal/bsd: fix ioport write operation

[Bruce Richardson]

On Mon, May 08, 2017 at 05:07:36PM +0800, Tiwei Bie wrote:
> On Mon, May 08, 2017 at 09:55:01AM +0100, Bruce Richardson wrote:
> > On Sun, May 07, 2017 at 01:33:33PM +0000, Tiwei Bie wrote:
> > > The first param of out*() on FreeBSD is port, and the second one is
> > > data. But they are reversed in DPDK. This patch fixes it.
> > > 
> > > Fixes: 756ce64b1ecd ("eal: introduce PCI ioport API") Cc:
> > > stable@dpdk.org
> > > 
> > > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com> ---
> > How was this bug discovered so that we can verify that it is fixed?.  Is
> > this in use by virtio or was it just discovered via code inspection?
> > 
> 
> The virtio PMD in legacy mode doesn't work FreeBSD, and I tried to
> fix this issue. And then I found this bug.
> 
> Best regards,
> Tiwei Bie

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

Re: [dpdk-stable] [PATCH 2/2] eal/bsd: fix the read operation on PCI configuration space

[Bruce Richardson]

On Sun, May 07, 2017 at 01:33:34PM +0000, Tiwei Bie wrote:
> Some drivers (such as virtio) may need to read more than 4 bytes
> data from PCI configuration space via rte_eal_pci_read_config().
> But it will return with an error on FreeBSD when the expected
> data length is bigger than the size of pi.pi_data whose type is
> u_int32_t. This patch removes this limitation.
> 
> Fixes: 632b2d1deeed ("eal: provide functions to access PCI config")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> ---
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

[dpdk-stable] [PATCH 1/2] eal/bsd: fix ioport write operation

[Tiwei Bie]

The first param of out*() on FreeBSD is port, and the second one
is data. But they are reversed in DPDK. This patch fixes it.

Fixes: 756ce64b1ecd ("eal: introduce PCI ioport API")
Cc: stable@dpdk.org

Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
---
 lib/librte_eal/bsdapp/eal/eal_pci.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
index 6294b7eab..59ceb7665 100644
--- a/lib/librte_eal/bsdapp/eal/eal_pci.c
+++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
@@ -618,13 +618,13 @@ pci_uio_ioport_write(struct rte_pci_ioport *p,
 	for (s = data; len > 0; s += size, reg += size, len -= size) {
 		if (len >= 4) {
 			size = 4;
-			outl(*(const uint32_t *)s, reg);
+			outl(reg, *(const uint32_t *)s);
 		} else if (len >= 2) {
 			size = 2;
-			outw(*(const uint16_t *)s, reg);
+			outw(reg, *(const uint16_t *)s);
 		} else {
 			size = 1;
-			outb(*s, reg);
+			outb(reg, *s);
 		}
 	}
 #else
-- 
2.12.1