DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Xu, Rosen" <rosen.xu@intel.com>
To: "Iremonger, Bernard" <bernard.iremonger@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>
Cc: "Lu, Wenzhuo" <wenzhuo.lu@intel.com>,
	"Wu, Jingjing" <jingjing.wu@intel.com>,
	"Yigit, Ferruh" <ferruh.yigit@intel.com>
Subject: Re: [dpdk-dev] [PATCH v4] app/test-pmd: add IFPGA AFU register access function for testpmd
Date: Tue, 18 Dec 2018 11:32:00 +0000	[thread overview]
Message-ID: <0E78D399C70DA940A335608C6ED296D73A44B972@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <8CEF83825BEC744B83065625E567D7C260D3ECCB@IRSMSX108.ger.corp.intel.com>

Hi Bernard,


> -----Original Message-----
> From: Iremonger, Bernard
> Sent: Tuesday, December 18, 2018 18:21
> 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 v4] app/test-pmd: add IFPGA AFU register access
> function for testpmd
> 
> Hi Rosen,
> 
> > -----Original Message-----
> > From: Xu, Rosen
> > Sent: Monday, December 17, 2018 12:56 PM
> > 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 v4] app/test-pmd: add IFPGA AFU register access
> > function for testpmd
> 
> ./devtools/check-git-log.sh -1
> Wrong headline label:
>         app/test-pmd: add IFPGA AFU register access function for testpmd
> Headline too long:
>         app/test-pmd: add IFPGA AFU register access function for testpmd
> 
> Headline label should be "app/testpmd"
> The text needs to be shortened, maybe drop "for testpmd"

Fixed in v5 patch, thanks a lot.
 
> >
> > 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  | 116
> > ++++++++++++++++++++++++++++++++++++++++---------
> >  app/test-pmd/testpmd.h |  60 +++++++++++++++++++++++++
> >  2 files changed, 155 insertions(+), 21 deletions(-)
> >
> > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
> > b9e5dd9..f39a8eb 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,21 @@ 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) {
> > -		printf("Port %d: register offset %u (0x%X) out of port PCI "
> > +	if (reg_off >= len) {
> > +		printf("Port %d: register offset %u (0x%X) out of port "
> > +		       "PCI or AFU device "
> >  		       "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 +933,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 +941,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 +962,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 +977,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 +998,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 +1022,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 +1034,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 +1065,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 +1091,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,
> > --
> > 1.8.3.1
> 
> Other wise
> 
> Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>
> 

  reply	other threads:[~2018-12-18 11:32 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-06 12:16 [dpdk-dev] [PATCH] app/test-pmd: add IFPGA AFU register read/write access " Rosen Xu
2018-12-07 10:24 ` Iremonger, Bernard
2018-12-14  1:16   ` Xu, Rosen
2018-12-14  1:14 ` [dpdk-dev] [PATCH v2] " Rosen Xu
2018-12-14 10:18   ` Iremonger, Bernard
2018-12-17 12:17     ` Xu, Rosen
2018-12-14 17:37   ` Pattan, Reshma
2018-12-17 12:16     ` Xu, Rosen
2018-12-17 12:29 ` [dpdk-dev] [PATCH v3] app/test-pmd: add IFPGA AFU register access fuction " Rosen Xu
2018-12-17 12:56 ` [dpdk-dev] [PATCH v4] app/test-pmd: add IFPGA AFU register access function " Rosen Xu
2018-12-18 10:21   ` Iremonger, Bernard
2018-12-18 11:32     ` Xu, Rosen [this message]
2018-12-18 11:30 ` [dpdk-dev] [PATCH v5] app/testpmd: add IFPGA AFU register access function Rosen Xu
2018-12-18 18:12   ` Iremonger, Bernard
2018-12-20  8:58     ` Ferruh Yigit
2018-12-20 10:48       ` Xu, Rosen
2018-12-20 14:22         ` Ferruh Yigit
2019-01-02  6:20           ` Xu, Rosen
2018-12-20 12:57   ` Ferruh Yigit
2019-01-02  6:19 ` [dpdk-dev] [PATCH v6] " Rosen Xu
2019-01-08 15:28   ` Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0E78D399C70DA940A335608C6ED296D73A44B972@SHSMSX104.ccr.corp.intel.com \
    --to=rosen.xu@intel.com \
    --cc=bernard.iremonger@intel.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=jingjing.wu@intel.com \
    --cc=wenzhuo.lu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).