Re: [dpdk-dev] [PATCH v6] app/testpmd: add IFPGA AFU register access function

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 1/2/2019 6:19 AM, Rosen Xu wrote:
> 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>
> Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>
> 
> v5 updates:
> ===========
>  - Added Macro to fix compile dependency of ifpga for testpmd
> ---
>  app/test-pmd/config.c  | 253 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  app/test-pmd/testpmd.h |  64 +++++++++++++
>  2 files changed, 315 insertions(+), 2 deletions(-)
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index b9e5dd9..5600ef5 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -866,7 +866,50 @@ void print_valid_ports(void)
>  	printf("Invalid vlan_id %d (must be < 4096)\n", vlan_id);
>  	return 1;
>  }
> +#ifdef RTE_LIBRTE_IFPGA_BUS
> +static int
> +port_reg_off_is_invalid(portid_t port_id, uint32_t reg_off)
> +{
> +	const struct rte_pci_device *pci_dev;
> +	const struct rte_bus *bus;
> +	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 "
> +		       "boundary\n",
> +		       (unsigned int)reg_off);
> +		return 1;
> +	}
> +
> +	if (!ports[port_id].dev_info.device) {
> +		printf("Invalid device\n");
> +		return 0;
> +	}
> +
> +	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 or AFU device\n");
> +		return 1;
> +	}
> +
> +	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 int)reg_off,
> +			(unsigned int)reg_off, len);
> +		return 1;
> +	}
> +	return 0;
> +}

Do we need to duplicate all function? I think wrapping only following part with
'#ifdef RTE_LIBRTE_IFPGA_BUS' can work:

  	} else if (bus && !strcmp(bus->name, "ifpga")) {
  		afu_dev = RTE_DEV_TO_AFU(ports[port_id].dev_info.device);


<...>

> +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;
> +
> +	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);

Same thing for all above functions, instead of duplication all function with
#ifdef, only wrapping "ifpga" related lines should work.

<...>

> @@ -11,6 +11,9 @@
>  #include <rte_bus_pci.h>
>  #include <rte_gro.h>
>  #include <rte_gso.h>
> +#ifdef RTE_LIBRTE_IFPGA_BUS
> +#include <rte_bus_ifpga.h>
> +#endif

This is causing build error with meson, because 'bus_ifpga' is not added as a
dependency to testpmd. Can you please fix it?