diff mbox series

[v2] app/test-pmd: add IFPGA AFU register read/write access for testpmd

Message ID 1544750062-80485-1-git-send-email-rosen.xu@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series [v2] app/test-pmd: add IFPGA AFU register read/write access for testpmd |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/intel-Performance-Testing success Performance Testing PASS

Commit Message

Xu, Rosen Dec. 14, 2018, 1:14 a.m. UTC 
  Currently register read/write of testpmd is only for PCI device,
but more and more IFPGA based AFU devices need this feature to
access registers, this patch will add support for it.

Signed-off-by: Rosen Xu <rosen.xu@intel.com>
---
 app/test-pmd/config.c  | 113 ++++++++++++++++++++++++++++++++++++++++---------
 app/test-pmd/testpmd.h |  60 ++++++++++++++++++++++++++
 2 files changed, 153 insertions(+), 20 deletions(-)

Comments

Iremonger, Bernard Dec. 14, 2018, 10:18 a.m. UTC | #1 
Hi Rosen

> -----Original Message-----
> From: Xu, Rosen
> Sent: Friday, December 14, 2018 1:14 AM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Iremonger, Bernard
> <bernard.iremonger@intel.com>; Xu, Rosen <rosen.xu@intel.com>; Yigit,
> Ferruh <ferruh.yigit@intel.com>
> Subject: [PATCH v2] app/test-pmd: add IFPGA AFU register read/write
> access for testpmd
> 
> Currently register read/write of testpmd is only for PCI device, but more and
> more IFPGA based AFU devices need this feature to access registers, this
> patch will add support for it.
> 
> Signed-off-by: Rosen Xu <rosen.xu@intel.com>
> ---

./devtools/check-git-log.sh -1
Wrong headline label:
        app/test-pmd: add IFPGA AFU register read/write access for testpmd
Headline too long:
        app/test-pmd: add IFPGA AFU register read/write access for testpmd

Otherwise
Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>
Pattan, Reshma Dec. 14, 2018, 5:37 p.m. UTC | #2 
Hi,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Rosen Xu
> Sent: Friday, December 14, 2018 1:14 AM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Iremonger, Bernard <bernard.iremonger@intel.com>;
> Xu, Rosen <rosen.xu@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>
> Subject: [dpdk-dev] [PATCH v2] app/test-pmd: add IFPGA AFU register
> read/write access for testpmd
> 
> Currently register read/write of testpmd is only for PCI device, but more and
> more IFPGA based AFU devices need this feature to access registers, this patch
> will add support for it.
> 
> Signed-off-by: Rosen Xu <rosen.xu@intel.com>
> -	pci_len = pci_dev->mem_resource[0].len;
> -	if (reg_off >= pci_len) {
> +	if (reg_off >= len) {
>  		printf("Port %d: register offset %u (0x%X) out of port PCI "

Here log message mentions only PCI not ifpga device. Might need to edit the log.

> port_reg_bit_display(portid_t port_id, uint32_t reg_off, uint8_t bit_x)  {
>  	uint32_t reg_v;
> -
> +	const struct rte_bus *bus;
> 
>  	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
> @@ -935,7 +940,16 @@ void print_valid_ports(void)
>  		return;
>  	if (reg_bit_pos_is_invalid(bit_x))
>  		return;
> -	reg_v = port_id_pci_reg_read(port_id, reg_off);
> +
> +	bus = rte_bus_find_by_device(ports[port_id].dev_info.device);
> +	if (bus && !strcmp(bus->name, "pci")) {
> +		reg_v = port_id_pci_reg_read(port_id, reg_off);
> +	} else if (bus && !strcmp(bus->name, "ifpga")) {
> +		reg_v = port_id_afu_reg_read(port_id, reg_off);
> +	} else {
> +		printf("Not a PCI or AFU device\n");
> +		return;
> +	}

Here and in other places for reg_read , we have similar code  i.e. finding the device , checking its type, if ifpga call ifpga function else call pci functions.
Can this common code be moved to new function say pci_read_reg like port_reg_set() which we have already.
Also , again inside respective pci/ifpga reg read/write we are checking for pci type. So can all this be simplified, to remove redundant code.

Thanks,
Reshma
Xu, Rosen Dec. 17, 2018, 12:16 p.m. UTC | #3 
Hi,

> -----Original Message-----
> From: Pattan, Reshma
> Sent: Saturday, December 15, 2018 1:38
> To: Xu, Rosen <rosen.xu@intel.com>; dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Iremonger, Bernard
> <bernard.iremonger@intel.com>; Xu, Rosen <rosen.xu@intel.com>; Yigit,
> Ferruh <ferruh.yigit@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2] app/test-pmd: add IFPGA AFU register
> read/write access for testpmd
> 
> Hi,
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Rosen Xu
> > Sent: Friday, December 14, 2018 1:14 AM
> > To: dev@dpdk.org
> > Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> > <jingjing.wu@intel.com>; Iremonger, Bernard
> > <bernard.iremonger@intel.com>; Xu, Rosen <rosen.xu@intel.com>; Yigit,
> > Ferruh <ferruh.yigit@intel.com>
> > Subject: [dpdk-dev] [PATCH v2] app/test-pmd: add IFPGA AFU register
> > read/write access for testpmd
> >
> > Currently register read/write of testpmd is only for PCI device, but
> > more and more IFPGA based AFU devices need this feature to access
> > registers, this patch will add support for it.
> >
> > Signed-off-by: Rosen Xu <rosen.xu@intel.com>
> > -	pci_len = pci_dev->mem_resource[0].len;
> > -	if (reg_off >= pci_len) {
> > +	if (reg_off >= len) {
> >  		printf("Port %d: register offset %u (0x%X) out of port PCI "
> 
> Here log message mentions only PCI not ifpga device. Might need to edit the
> log.

I have fixed in revision V3.

> > port_reg_bit_display(portid_t port_id, uint32_t reg_off, uint8_t bit_x)  {
> >  	uint32_t reg_v;
> > -
> > +	const struct rte_bus *bus;
> >
> >  	if (port_id_is_invalid(port_id, ENABLED_WARN))
> >  		return;
> > @@ -935,7 +940,16 @@ void print_valid_ports(void)
> >  		return;
> >  	if (reg_bit_pos_is_invalid(bit_x))
> >  		return;
> > -	reg_v = port_id_pci_reg_read(port_id, reg_off);
> > +
> > +	bus = rte_bus_find_by_device(ports[port_id].dev_info.device);
> > +	if (bus && !strcmp(bus->name, "pci")) {
> > +		reg_v = port_id_pci_reg_read(port_id, reg_off);
> > +	} else if (bus && !strcmp(bus->name, "ifpga")) {
> > +		reg_v = port_id_afu_reg_read(port_id, reg_off);
> > +	} else {
> > +		printf("Not a PCI or AFU device\n");
> > +		return;
> > +	}
> 
> Here and in other places for reg_read , we have similar code  i.e. finding the
> device , checking its type, if ifpga call ifpga function else call pci functions.
> Can this common code be moved to new function say pci_read_reg like
> port_reg_set() which we have already.
> Also , again inside respective pci/ifpga reg read/write we are checking for pci
> type. So can all this be simplified, to remove redundant code.

PCI device and AFU device belongs to different bus, if we merge the register access
code to same function, it's not clarify.
 
> Thanks,
> Reshma
>
Xu, Rosen Dec. 17, 2018, 12:17 p.m. UTC | #4 
Hi Bernard,

> -----Original Message-----
> From: Iremonger, Bernard
> Sent: Friday, December 14, 2018 18:19
> To: Xu, Rosen <rosen.xu@intel.com>; dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>
> Subject: RE: [PATCH v2] app/test-pmd: add IFPGA AFU register read/write
> access for testpmd
> 
> Hi Rosen
> 
> > -----Original Message-----
> > From: Xu, Rosen
> > Sent: Friday, December 14, 2018 1:14 AM
> > To: dev@dpdk.org
> > Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> > <jingjing.wu@intel.com>; Iremonger, Bernard
> > <bernard.iremonger@intel.com>; Xu, Rosen <rosen.xu@intel.com>; Yigit,
> > Ferruh <ferruh.yigit@intel.com>
> > Subject: [PATCH v2] app/test-pmd: add IFPGA AFU register read/write
> > access for testpmd
> >
> > Currently register read/write of testpmd is only for PCI device, but
> > more and more IFPGA based AFU devices need this feature to access
> > registers, this patch will add support for it.
> >
> > Signed-off-by: Rosen Xu <rosen.xu@intel.com>
> > ---
> 
> ./devtools/check-git-log.sh -1
> Wrong headline label:
>         app/test-pmd: add IFPGA AFU register read/write access for testpmd
> Headline too long:
>         app/test-pmd: add IFPGA AFU register read/write access for testpmd

I will fix it in revision V3 patch, thanks a lot.

> Otherwise
> Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>
diff mbox series

Patch

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index b9e5dd9..b51daa9 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -872,7 +872,8 @@  void print_valid_ports(void)
 {
 	const struct rte_pci_device *pci_dev;
 	const struct rte_bus *bus;
-	uint64_t pci_len;
+	uint64_t len;
+	const struct rte_afu_device *afu_dev;
 
 	if (reg_off & 0x3) {
 		printf("Port register offset 0x%X not aligned on a 4-byte "
@@ -889,16 +890,20 @@  void print_valid_ports(void)
 	bus = rte_bus_find_by_device(ports[port_id].dev_info.device);
 	if (bus && !strcmp(bus->name, "pci")) {
 		pci_dev = RTE_DEV_TO_PCI(ports[port_id].dev_info.device);
+		len = pci_dev->mem_resource[0].len;
+	} else if (bus && !strcmp(bus->name, "ifpga")) {
+		afu_dev = RTE_DEV_TO_AFU(ports[port_id].dev_info.device);
+		len = afu_dev->mem_resource[0].len;
 	} else {
-		printf("Not a PCI device\n");
+		printf("Not a PCI or AFU device\n");
 		return 1;
 	}
 
-	pci_len = pci_dev->mem_resource[0].len;
-	if (reg_off >= pci_len) {
+	if (reg_off >= len) {
 		printf("Port %d: register offset %u (0x%X) out of port PCI "
 		       "resource (length=%"PRIu64")\n",
-		       port_id, (unsigned)reg_off, (unsigned)reg_off,  pci_len);
+		       port_id, (unsigned int)reg_off,
+			(unsigned int)reg_off, len);
 		return 1;
 	}
 	return 0;
@@ -927,7 +932,7 @@  void print_valid_ports(void)
 port_reg_bit_display(portid_t port_id, uint32_t reg_off, uint8_t bit_x)
 {
 	uint32_t reg_v;
-
+	const struct rte_bus *bus;
 
 	if (port_id_is_invalid(port_id, ENABLED_WARN))
 		return;
@@ -935,7 +940,16 @@  void print_valid_ports(void)
 		return;
 	if (reg_bit_pos_is_invalid(bit_x))
 		return;
-	reg_v = port_id_pci_reg_read(port_id, reg_off);
+
+	bus = rte_bus_find_by_device(ports[port_id].dev_info.device);
+	if (bus && !strcmp(bus->name, "pci")) {
+		reg_v = port_id_pci_reg_read(port_id, reg_off);
+	} else if (bus && !strcmp(bus->name, "ifpga")) {
+		reg_v = port_id_afu_reg_read(port_id, reg_off);
+	} else {
+		printf("Not a PCI or AFU device\n");
+		return;
+	}
 	display_port_and_reg_off(port_id, (unsigned)reg_off);
 	printf("bit %d=%d\n", bit_x, (int) ((reg_v & (1 << bit_x)) >> bit_x));
 }
@@ -947,6 +961,7 @@  void print_valid_ports(void)
 	uint32_t reg_v;
 	uint8_t  l_bit;
 	uint8_t  h_bit;
+	const struct rte_bus *bus;
 
 	if (port_id_is_invalid(port_id, ENABLED_WARN))
 		return;
@@ -961,7 +976,15 @@  void print_valid_ports(void)
 	else
 		l_bit = bit1_pos, h_bit = bit2_pos;
 
-	reg_v = port_id_pci_reg_read(port_id, reg_off);
+	bus = rte_bus_find_by_device(ports[port_id].dev_info.device);
+	if (bus && !strcmp(bus->name, "pci")) {
+		reg_v = port_id_pci_reg_read(port_id, reg_off);
+	} else if (bus && !strcmp(bus->name, "ifpga")) {
+		reg_v = port_id_afu_reg_read(port_id, reg_off);
+	} else {
+		printf("Not a PCI or AFU device\n");
+		return;
+	}
 	reg_v >>= l_bit;
 	if (h_bit < 31)
 		reg_v &= ((1 << (h_bit - l_bit + 1)) - 1);
@@ -974,12 +997,22 @@  void print_valid_ports(void)
 port_reg_display(portid_t port_id, uint32_t reg_off)
 {
 	uint32_t reg_v;
+	const struct rte_bus *bus;
 
 	if (port_id_is_invalid(port_id, ENABLED_WARN))
 		return;
 	if (port_reg_off_is_invalid(port_id, reg_off))
 		return;
-	reg_v = port_id_pci_reg_read(port_id, reg_off);
+
+	bus = rte_bus_find_by_device(ports[port_id].dev_info.device);
+	if (bus && !strcmp(bus->name, "pci")) {
+		reg_v = port_id_pci_reg_read(port_id, reg_off);
+	} else if (bus && !strcmp(bus->name, "ifpga")) {
+		reg_v = port_id_afu_reg_read(port_id, reg_off);
+	} else {
+		printf("Not a PCI or AFU device\n");
+		return;
+	}
 	display_port_reg_value(port_id, reg_off, reg_v);
 }
 
@@ -988,6 +1021,7 @@  void print_valid_ports(void)
 		 uint8_t bit_v)
 {
 	uint32_t reg_v;
+	const struct rte_bus *bus;
 
 	if (port_id_is_invalid(port_id, ENABLED_WARN))
 		return;
@@ -999,12 +1033,26 @@  void print_valid_ports(void)
 		printf("Invalid bit value %d (must be 0 or 1)\n", (int) bit_v);
 		return;
 	}
-	reg_v = port_id_pci_reg_read(port_id, reg_off);
-	if (bit_v == 0)
-		reg_v &= ~(1 << bit_pos);
-	else
-		reg_v |= (1 << bit_pos);
-	port_id_pci_reg_write(port_id, reg_off, reg_v);
+
+	bus = rte_bus_find_by_device(ports[port_id].dev_info.device);
+	if (bus && !strcmp(bus->name, "pci")) {
+		reg_v = port_id_pci_reg_read(port_id, reg_off);
+		if (bit_v == 0)
+			reg_v &= ~(1 << bit_pos);
+		else
+			reg_v |= (1 << bit_pos);
+		port_id_pci_reg_write(port_id, reg_off, reg_v);
+	} else if (bus && !strcmp(bus->name, "ifpga")) {
+		reg_v = port_id_afu_reg_read(port_id, reg_off);
+		if (bit_v == 0)
+			reg_v &= ~(1 << bit_pos);
+		else
+			reg_v |= (1 << bit_pos);
+		port_id_afu_reg_write(port_id, reg_off, reg_v);
+	} else {
+		printf("Not a PCI or AFU device\n");
+		return;
+	}
 	display_port_reg_value(port_id, reg_off, reg_v);
 }
 
@@ -1016,6 +1064,7 @@  void print_valid_ports(void)
 	uint32_t reg_v;
 	uint8_t  l_bit;
 	uint8_t  h_bit;
+	const struct rte_bus *bus;
 
 	if (port_id_is_invalid(port_id, ENABLED_WARN))
 		return;
@@ -1041,21 +1090,45 @@  void print_valid_ports(void)
 				(unsigned)max_v, (unsigned)max_v);
 		return;
 	}
-	reg_v = port_id_pci_reg_read(port_id, reg_off);
-	reg_v &= ~(max_v << l_bit); /* Keep unchanged bits */
-	reg_v |= (value << l_bit); /* Set changed bits */
-	port_id_pci_reg_write(port_id, reg_off, reg_v);
+
+	bus = rte_bus_find_by_device(ports[port_id].dev_info.device);
+	if (bus && !strcmp(bus->name, "pci")) {
+		reg_v = port_id_pci_reg_read(port_id, reg_off);
+		reg_v &= ~(max_v << l_bit); /* Keep unchanged bits */
+		reg_v |= (value << l_bit); /* Set changed bits */
+		port_id_pci_reg_write(port_id, reg_off, reg_v);
+	} else if (bus && !strcmp(bus->name, "ifpga")) {
+		reg_v = port_id_afu_reg_read(port_id, reg_off);
+		reg_v &= ~(max_v << l_bit); /* Keep unchanged bits */
+		reg_v |= (value << l_bit); /* Set changed bits */
+		port_id_afu_reg_write(port_id, reg_off, reg_v);
+	} else {
+		printf("Not a PCI or AFU device\n");
+		return;
+	}
 	display_port_reg_value(port_id, reg_off, reg_v);
 }
 
 void
 port_reg_set(portid_t port_id, uint32_t reg_off, uint32_t reg_v)
 {
+	const struct rte_bus *bus;
+
 	if (port_id_is_invalid(port_id, ENABLED_WARN))
 		return;
 	if (port_reg_off_is_invalid(port_id, reg_off))
 		return;
-	port_id_pci_reg_write(port_id, reg_off, reg_v);
+
+	bus = rte_bus_find_by_device(ports[port_id].dev_info.device);
+	if (bus && !strcmp(bus->name, "pci")) {
+		port_id_pci_reg_write(port_id, reg_off, reg_v);
+	} else if (bus && !strcmp(bus->name, "ifpga")) {
+		port_id_afu_reg_write(port_id, reg_off, reg_v);
+	} else {
+		printf("Not a PCI or AFU device\n");
+		return;
+	}
+
 	display_port_reg_value(port_id, reg_off, reg_v);
 }
 
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 3ff11e6..0f51df4 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -11,6 +11,7 @@ 
 #include <rte_bus_pci.h>
 #include <rte_gro.h>
 #include <rte_gso.h>
+#include <rte_bus_ifpga.h>
 
 #define RTE_PORT_ALL            (~(portid_t)0x0)
 
@@ -671,6 +672,65 @@  struct mplsoudp_decap_conf {
 #define port_id_pci_reg_write(pt_id, reg_off, reg_value) \
 	port_pci_reg_write(&ports[(pt_id)], (reg_off), (reg_value))
 
+/**
+ * Read/Write operations on an AFU register of a port.
+ */
+static inline uint32_t
+port_afu_reg_read(struct rte_port *port, uint32_t reg_off)
+{
+	const struct rte_afu_device *afu_dev;
+	const struct rte_bus *bus;
+	void *reg_addr;
+	uint32_t reg_v;
+
+	if (!port->dev_info.device) {
+		printf("Invalid device\n");
+		return 0;
+	}
+
+	bus = rte_bus_find_by_device(port->dev_info.device);
+	if (bus && !strcmp(bus->name, "ifpga")) {
+		afu_dev = RTE_DEV_TO_AFU(port->dev_info.device);
+	} else {
+		printf("Not an IFPGA AFU device\n");
+		return 0;
+	}
+
+	reg_addr = ((char *)afu_dev->mem_resource[0].addr + reg_off);
+	reg_v = *((volatile uint32_t *)reg_addr);
+	return rte_le_to_cpu_32(reg_v);
+}
+
+#define port_id_afu_reg_read(pt_id, reg_off) \
+	port_afu_reg_read(&ports[(pt_id)], (reg_off))
+
+static inline void
+port_afu_reg_write(struct rte_port *port, uint32_t reg_off, uint32_t reg_v)
+{
+	const struct rte_afu_device *afu_dev;
+	const struct rte_bus *bus;
+	void *reg_addr;
+
+	if (!port->dev_info.device) {
+		printf("Invalid device\n");
+		return;
+	}
+
+	bus = rte_bus_find_by_device(port->dev_info.device);
+	if (bus && !strcmp(bus->name, "ifpga")) {
+		afu_dev = RTE_DEV_TO_AFU(port->dev_info.device);
+	} else {
+		printf("Not an IFPGA AFU device\n");
+		return;
+	}
+
+	reg_addr = ((char *)afu_dev->mem_resource[0].addr + reg_off);
+	*((volatile uint32_t *)reg_addr) = rte_cpu_to_le_32(reg_v);
+}
+
+#define port_id_afu_reg_write(pt_id, reg_off, reg_value) \
+	port_afu_reg_write(&ports[(pt_id)], (reg_off), (reg_value))
+
 /* Prototypes */
 unsigned int parse_item_list(char* str, const char* item_name,
 			unsigned int max_items,