> -----Original Message-----
> From: Xu, Rosen
> Sent: Monday, April 02, 2018 12:26
> To: 'gaetan.ri...@6wind.com' <gaetan.ri...@6wind.com>
> Cc: 'dev@dpdk.org' <dev@dpdk.org>; Doherty, Declan
> <declan.dohe...@intel.com>; Richardson, Bruce
> <bruce.richard...@intel.com>; 'shreyansh.j...@nxp.com'
> <shreyansh.j...@nxp.com>; Zhang, Tianfei <tianfei.zh...@intel.com>; Wu,
> Hao <hao...@intel.com>
> Subject: RE: [PATCH v3 4/6] drivers/bus: Add Intel FPGA Bus Lib Code
> 
> 
> 
> > -----Original Message-----
> > From: Xu, Rosen
> > Sent: Sunday, April 01, 2018 0:31
> > To: gaetan.ri...@6wind.com
> > Cc: dev@dpdk.org; Doherty, Declan <declan.dohe...@intel.com>;
> > Richardson, Bruce <bruce.richard...@intel.com>;
> > shreyansh.j...@nxp.com; Zhang, Tianfei <tianfei.zh...@intel.com>; Wu,
> > Hao <hao...@intel.com>
> > Subject: RE: [PATCH v3 4/6] drivers/bus: Add Intel FPGA Bus Lib Code
> >
> >
> >
> > > -----Original Message-----
> > > From: Gaëtan Rivet [mailto:gaetan.ri...@6wind.com]
> > > Sent: Wednesday, March 28, 2018 21:52
> > > To: Xu, Rosen <rosen...@intel.com>
> > > Cc: dev@dpdk.org; Doherty, Declan <declan.dohe...@intel.com>;
> > > Richardson, Bruce <bruce.richard...@intel.com>;
> > > shreyansh.j...@nxp.com; Zhang, Tianfei <tianfei.zh...@intel.com>;
> > > Wu, Hao <hao...@intel.com>
> > > Subject: Re: [PATCH v3 4/6] drivers/bus: Add Intel FPGA Bus Lib Code
> > >
> > > On Wed, Mar 28, 2018 at 05:29:54PM +0800, Rosen Xu wrote:
> > > > Signed-off-by: Rosen Xu <rosen...@intel.com>
> > > > ---
> > > >  drivers/bus/Makefile                        |   1 +
> > > >  drivers/bus/ifpga/Makefile                  |  33 ++
> > > >  drivers/bus/ifpga/ifpga_bus.c               | 562
> > > ++++++++++++++++++++++++++++
> > > >  drivers/bus/ifpga/ifpga_common.c            | 141 +++++++
> > > >  drivers/bus/ifpga/ifpga_common.h            |  22 ++
> > > >  drivers/bus/ifpga/ifpga_logs.h              |  31 ++
> > > >  drivers/bus/ifpga/rte_bus_ifpga.h           | 140 +++++++
> > > >  drivers/bus/ifpga/rte_bus_ifpga_version.map |   8 +
> > > >  8 files changed, 938 insertions(+)  create mode 100644
> > > > drivers/bus/ifpga/Makefile  create mode 100644
> > > > drivers/bus/ifpga/ifpga_bus.c  create mode 100644
> > > > drivers/bus/ifpga/ifpga_common.c  create mode 100644
> > > > drivers/bus/ifpga/ifpga_common.h  create mode 100644
> > > > drivers/bus/ifpga/ifpga_logs.h  create mode 100644
> > > > drivers/bus/ifpga/rte_bus_ifpga.h  create mode 100644
> > > > drivers/bus/ifpga/rte_bus_ifpga_version.map
> > > >
> > > > diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile index
> > > > 7ef2593..55d2dfe 100644
> > > > --- a/drivers/bus/Makefile
> > > > +++ b/drivers/bus/Makefile
> > > > @@ -7,5 +7,6 @@ DIRS-$(CONFIG_RTE_LIBRTE_DPAA_BUS) += dpaa
> > > >  DIRS-$(CONFIG_RTE_LIBRTE_FSLMC_BUS) += fslmc
> > > >  DIRS-$(CONFIG_RTE_LIBRTE_PCI_BUS) += pci
> > > >  DIRS-$(CONFIG_RTE_LIBRTE_VDEV_BUS) += vdev
> > > > +DIRS-$(CONFIG_RTE_LIBRTE_IFPGA_BUS) += ifpga
> > > >
> > > >  include $(RTE_SDK)/mk/rte.subdir.mk diff --git
> > > > a/drivers/bus/ifpga/Makefile b/drivers/bus/ifpga/Makefile new file
> > > > mode 100644 index 0000000..e611950
> > > > --- /dev/null
> > > > +++ b/drivers/bus/ifpga/Makefile
> > > > @@ -0,0 +1,33 @@
> > > > +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2017 Intel
> > > > +Corporation
> > >
> > > Copyrigth 2018 (same comment for most SPDX tags, I think I saw 2017
> > > or a variation of it everywhere I think).
> >
> > It's done by PATCH v4.
> >
> > > > +
> > > > +include $(RTE_SDK)/mk/rte.vars.mk
> > > > +
> > > > +#
> > > > +# library name
> > > > +#
> > > > +LIB = librte_bus_ifpga.a
> > > > +
> > > > +CFLAGS += -O3
> > > > +CFLAGS += $(WERROR_FLAGS)
> > > > +
> > > > +# versioning export map
> > > > +EXPORT_MAP := rte_bus_ifpga_version.map
> > > > +
> > > > +# library version
> > > > +LIBABIVER := 1
> > > > +
> > > > +VPATH += $(SRCDIR)/base
> > > > +
> > > > +SRCS-y += \
> > > > +        ifpga_bus.c \
> > > > +        ifpga_common.c
> > > > +
> > > > +LDLIBS += -lrte_eal
> > >
> > > you should probably load librte_pci.
> > > You use afterward functions (at least address comparison).
> > > I haven't tested the SHARED build, but my guess is that it would break.
> >
> > The FPGA BUS is based PCI(IFPGA is probed by PCI), do you think so
> > users to add it?
> >
> > > On that note, you haven't acted on my previous remarks to use the
> > > common PCI utilities provided.
> >
> > It's done in PATCH v4.
> >
> > > > +
> > > > +#
> > > > +# Export include files
> > > > +#
> > > > +SYMLINK-y-include += rte_bus_ifpga.h
> > > > +
> > > > +include $(RTE_SDK)/mk/rte.lib.mk
> > > > diff --git a/drivers/bus/ifpga/ifpga_bus.c
> > > > b/drivers/bus/ifpga/ifpga_bus.c new file mode 100644 index
> > > > 0000000..092be65
> > > > --- /dev/null
> > > > +++ b/drivers/bus/ifpga/ifpga_bus.c
> > > > @@ -0,0 +1,562 @@
> > > > +/* SPDX-License-Identifier: BSD-3-Clause
> > > > + * Copyright(c) 2010-2017 Intel Corporation  */
> > > > +
> > > > +#include <string.h>
> > > > +#include <inttypes.h>
> > > > +#include <stdint.h>
> > > > +#include <stdlib.h>
> > > > +#include <stdio.h>
> > > > +#include <sys/queue.h>
> > > > +#include <sys/mman.h>
> > > > +#include <sys/types.h>
> > > > +#include <unistd.h>
> > > > +#include <fcntl.h>
> > > > +
> > > > +#include <rte_errno.h>
> > > > +#include <rte_bus.h>
> > > > +#include <rte_per_lcore.h>
> > > > +#include <rte_memory.h>
> > > > +#include <rte_memzone.h>
> > > > +#include <rte_eal.h>
> > > > +#include <rte_common.h>
> > > > +
> > > > +#include <rte_devargs.h>
> > > > +#include <rte_pci.h>
> > > > +#include <rte_bus_pci.h>
> > > > +#include <rte_kvargs.h>
> > > > +#include <rte_alarm.h>
> > > > +
> > > > +#include "rte_rawdev.h"
> > > > +#include "rte_rawdev_pmd.h"
> > > > +#include "rte_bus_ifpga.h"
> > > > +#include "ifpga_logs.h"
> > > > +#include "ifpga_common.h"
> > > > +
> > > > +int ifpga_bus_logtype;
> > > > +
> > > > +/* register a ifpga bus based driver */ void
> > > > +rte_ifpga_driver_register(struct rte_afu_driver *driver) {
> > > > +       RTE_VERIFY(driver);
> > > > +
> > > > +       TAILQ_INSERT_TAIL(&rte_ifpga_bus.driver_list, driver, next); }
> > > > +
> > > > +/* un-register a fpga bus based driver */ void
> > > > +rte_ifpga_driver_unregister(struct rte_afu_driver *driver) {
> > > > +       TAILQ_REMOVE(&rte_ifpga_bus.driver_list, driver, next); }
> > > > +
> > > > +static struct rte_ifpga_device *
> > > > +ifpga_find_ifpga_dev(const struct rte_pci_addr *pci_addr) {
> > > > +       struct rte_ifpga_device *ifpga_dev = NULL;
> > > > +
> > > > +       TAILQ_FOREACH(ifpga_dev, &rte_ifpga_bus.ifpga_list, next) {
> > > > +               if (!rte_pci_addr_cmp(&ifpga_dev->pci_addr, pci_addr))
> > > > +                       return ifpga_dev;
> > > > +       }
> > > > +       return NULL;
> > > > +}
> > > > +
> > > > +static struct rte_afu_device *
> > > > +ifpga_find_afu_dev(const struct rte_ifpga_device *ifpga_dev,
> > > > +       const struct rte_afu_id *afu_id) {
> > > > +       struct rte_afu_device *afu_dev = NULL;
> > > > +
> > > > +       TAILQ_FOREACH(afu_dev, &ifpga_dev->afu_list, next) {
> > > > +               if (!ifpga_afu_id_cmp(&afu_dev->id, afu_id))
> > > > +                       return afu_dev;
> > > > +       }
> > > > +       return NULL;
> > > > +}
> > > > +
> > > > +static const char * const valid_args[] = {
> > > > +#define IFPGA_ARG_BDF          "bdf"
> > > > +       IFPGA_ARG_BDF,
> > > > +#define IFPGA_ARG_PORT         "port"
> > > > +       IFPGA_ARG_PORT,
> > > > +#define IFPGA_ARG_PATH         "path"
> > > > +       IFPGA_ARG_PATH,
> > > > +#define IFPGA_ARG_UUID_HIGH    "uuid_high"
> > > > +       IFPGA_ARG_UUID_HIGH,
> > > > +#define IFPGA_ARG_UUID_LOW     "uuid_low"
> > > > +       IFPGA_ARG_UUID_LOW,
> > > > +#define IFPGA_ARG_PR_ENABLE     "pr_enable"
> > > > +       IFPGA_ARG_PR_ENABLE,
> > > > +#define IFPGA_ARG_DEBUG         "debug"
> > > > +       IFPGA_ARG_DEBUG,
> > > > +       NULL
> > > > +};
> > >
> > > So these parameters are parsed during scan afterward, taken from the
> > > PCI device that was probed.
> > >
> > > Given the kerfuffle of the VIRTUAL devtype in the EAL option, I'm
> > > not sure how the PCI bus would react with your IFPGA PCI device
> > > being probed by its blacklist operation: the devargs would be null.
> > > Do you segfault during your scan? Have you tested this?
> > >
> > > Why are those parameters parsed again here on this note? Shouldn't
> > > they be parsed by the PCI device being probed with a custome PCI
> > > driver that you would have added (and I think you provide it
> > > afterward), that would spawn the rawdev with the relevant metadata?
> > > Then you could scan this rawdev here and have all these parameters
> > already parsed?
> > >
> > > Is there a reason that you did all this in this order? (specific
> > > initialization step for example?)
> >
>  Pls see PATCH v4 and my reply for your email of PATCH v3 1/6.
>  The command line is in that email.
> 
> For PATCH v4 I have removed all modification from eal library.
> I use vdev driver to take parameter configuration, and this vdev driver call
> rte_eal_hotplug_add() to add afu device and trigger scan/probe.

This is the run command example(take 2 afu device for example):
testpmd -c 0x3 -n 4 --socket-mem 1024,0 --huge-dir=/mnt/huge \
 --vdev 'net_ifpga_cfg0,bdf=5e:00.0,port=0,afu_bts=./afu_1.gbs' \
 --vdev 'net_ifpga_cfg1,bdf=be:00.0,port=0,afu_bts=./afu_2.gbs' -- -i --no-numa

> > > > +
> > > > +/*
> > > > + * Scan the content of the FPGA bus, and the devices in the
> > > > +devices
> > > > + * list
> > > > + */
> > > > +static struct rte_afu_device *
> > > > +rte_ifpga_scan_one(struct rte_devargs *devargs,
> > > > +       struct rte_ifpga_device *ifpga_dev) {
> > > > +       struct rte_kvargs *kvlist = NULL;
> > > > +       struct rte_bus *pci_bus = NULL;
> > > > +       struct rte_device *dev = NULL;
> > > > +       struct rte_rawdev *rawdev = NULL;
> > > > +       struct rte_afu_device *afu_dev = NULL;
> > > > +       struct rte_afu_pr_conf afu_pr_conf;
> > > > +       int ret = 0;
> > > > +       char *path = NULL;
> > > > +       int pr_enable = 1;
> > > > +       int debug = 0;
> > > > +
> > > > +       memset((char *)(&afu_pr_conf), 0, sizeof(struct
> > > > +rte_afu_pr_conf));
> > > > +
> > > > +       kvlist = rte_kvargs_parse(devargs->args, valid_args);
> > > > +       if (!kvlist) {
> > > > +               IFPGA_BUS_ERR("error when parsing param");
> > > > +               goto end;
> > > > +       }
> > > > +
> > > > +       if (rte_kvargs_count(kvlist, IFPGA_ARG_BDF) == 1) {
> > > > +               if (rte_kvargs_process(kvlist, IFPGA_ARG_BDF,
> > > > +                       &ifpga_get_bdf_arg, 
> > > > &afu_pr_conf.afu_id.pci_addr)
> > > < 0) {
> > > > +                       IFPGA_BUS_ERR("error to parse %s",
> > > IFPGA_ARG_BDF);
> > > > +                       goto end;
> > > > +               }
> > > > +       } else {
> > > > +               IFPGA_BUS_ERR("arg %s is mandatory for ifpga bus",
> > > > +                       IFPGA_ARG_PATH);
> > > > +               goto end;
> > > > +       }
> > > > +
> > > > +       if (rte_kvargs_count(kvlist, IFPGA_ARG_PORT) == 1) {
> > > > +               if (rte_kvargs_process(kvlist, IFPGA_ARG_PORT,
> > > > +               &ifpga_get_integer32_arg, &afu_pr_conf.afu_id.port) < 
> > > > 0) {
> > > > +                       IFPGA_BUS_ERR("error to parse %s",
> > > > +                                    IFPGA_ARG_PORT);
> > > > +                       goto end;
> > > > +               }
> > > > +       } else {
> > > > +               IFPGA_BUS_ERR("arg %s is mandatory for ifpga bus",
> > > > +                         IFPGA_ARG_PATH);
> > > > +               goto end;
> > > > +       }
> > > > +
> > > > +       if (rte_kvargs_count(kvlist, IFPGA_ARG_PATH) == 1) {
> > > > +               if (rte_kvargs_process(kvlist, IFPGA_ARG_PATH,
> > > > +                                      &ifpga_get_string_arg, &path) < 
> > > > 0) {
> > > > +                       IFPGA_BUS_ERR("error to parse %s",
> > > > +                                    IFPGA_ARG_PATH);
> > > > +                       goto end;
> > > > +               }
> > > > +       } else {
> > > > +               IFPGA_BUS_ERR("arg %s is mandatory for ifpga bus",
> > > > +                         IFPGA_ARG_PATH);
> > > > +               goto end;
> > > > +       }
> > > > +
> > > > +       if (rte_kvargs_count(kvlist, IFPGA_ARG_UUID_HIGH) == 1) {
> > > > +               if (rte_kvargs_process(kvlist, IFPGA_ARG_UUID_HIGH,
> > > > +               &ifpga_get_integer64_arg, &afu_pr_conf.afu_id.uuid_high)
> > > < 0) {
> > > > +                       IFPGA_BUS_ERR("error to parse %s",
> > > > +                                    IFPGA_ARG_UUID_HIGH);
> > > > +                       goto end;
> > > > +               }
> > > > +       } else {
> > > > +               IFPGA_BUS_ERR("arg %s is mandatory for ifpga bus",
> > > > +                         IFPGA_ARG_PATH);
> > > > +               goto end;
> > > > +       }
> > > > +
> > > > +       if (rte_kvargs_count(kvlist, IFPGA_ARG_UUID_LOW) == 1) {
> > > > +               if (rte_kvargs_process(kvlist, IFPGA_ARG_UUID_LOW,
> > > > +               &ifpga_get_integer64_arg, &afu_pr_conf.afu_id.uuid_low) 
> > > > <
> > > 0) {
> > > > +                       IFPGA_BUS_ERR("error to parse %s",
> > > > +                                    IFPGA_ARG_UUID_LOW);
> > > > +                       goto end;
> > > > +               }
> > > > +       } else {
> > > > +               IFPGA_BUS_ERR("arg %s is mandatory for ifpga bus",
> > > > +                         IFPGA_ARG_PATH);
> > > > +               goto end;
> > > > +       }
> > > > +
> > > > +       if (rte_kvargs_count(kvlist, IFPGA_ARG_PR_ENABLE) == 1) {
> > > > +               if (rte_kvargs_process(kvlist, IFPGA_ARG_PR_ENABLE,
> > > > +                       &ifpga_get_integer32_arg, &pr_enable) < 0) {
> > > > +                       IFPGA_BUS_ERR("error to parse %s",
> > > > +                                    IFPGA_ARG_UUID_HIGH);
> > > > +                       goto end;
> > > > +               }
> > > > +       }
> > > > +
> > > > +       if (rte_kvargs_count(kvlist, IFPGA_ARG_DEBUG) == 1) {
> > > > +               if (rte_kvargs_process(kvlist, IFPGA_ARG_DEBUG,
> > > > +                                      &ifpga_get_integer32_arg, 
> > > > &debug) < 0) {
> > > > +                       IFPGA_BUS_ERR("error to parse %s",
> > > > +                                    IFPGA_ARG_UUID_HIGH);
> > > > +                       goto end;
> > > > +               }
> > > > +       }
> > > > +
> > > > +       if (!debug) {
> > > > +               pci_bus = rte_bus_find_by_name("pci");
> > > > +               if (pci_bus == NULL) {
> > > > +                       IFPGA_BUS_ERR("unable to find PCI bus\n");
> > > > +                       goto end;
> > > > +               }
> > > > +
> > > > +               dev = pci_bus->find_device(NULL,
> > > > +                               ifpga_pci_addr_cmp,
> > > > +                               &afu_pr_conf.afu_id.pci_addr);
> > > > +               if (dev == NULL) {
> > > > +                       IFPGA_BUS_ERR("unable to find PCI device\n");
> > > > +                       goto end;
> > > > +               }
> > > > +       } else {
> > > > +               IFPGA_BUS_DEBUG("pci_addr domain   : %x\n",
> > > > +                               afu_pr_conf.afu_id.pci_addr.domain);
> > > > +               IFPGA_BUS_DEBUG("pci_addr bus      : %x\n",
> > > > +                               afu_pr_conf.afu_id.pci_addr.bus);
> > > > +               IFPGA_BUS_DEBUG("pci_addr devid    : %x\n",
> > > > +                               afu_pr_conf.afu_id.pci_addr.devid);
> > > > +               IFPGA_BUS_DEBUG("pci_addr function : %x\n",
> > > > +                               afu_pr_conf.afu_id.pci_addr.function);
> > > > +               IFPGA_BUS_DEBUG("uuid_low          : %lx\n",
> > > > +                               afu_pr_conf.afu_id.uuid_low);
> > > > +               IFPGA_BUS_DEBUG("uuid_high         : %lx\n",
> > > > +                               afu_pr_conf.afu_id.uuid_high);
> > > > +               IFPGA_BUS_DEBUG("afu port          : %x\n",
> > > > +                               afu_pr_conf.afu_id.port);
> > > > +       }
> > > > +
> > > > +       rawdev = ifpga_dev->rdev;
> > > > +       if (ifpga_find_afu_dev(ifpga_dev, &afu_pr_conf.afu_id))
> > > > +               goto end;
> > > > +
> > > > +       afu_dev = calloc(1, sizeof(*afu_dev));
> > > > +       if (!afu_dev)
> > > > +               goto end;
> > > > +
> > > > +       afu_dev->device.devargs = devargs;
> > > > +       afu_dev->device.numa_node = SOCKET_ID_ANY;
> > > > +       afu_dev->device.name = devargs->name;
> > > > +       afu_dev->rawdev = rawdev;
> > > > +       afu_dev->id.pci_addr.domain = 
> > > > afu_pr_conf.afu_id.pci_addr.domain;
> > > > +       afu_dev->id.pci_addr.bus = afu_pr_conf.afu_id.pci_addr.bus;
> > > > +       afu_dev->id.pci_addr.devid = afu_pr_conf.afu_id.pci_addr.devid;
> > > > +       afu_dev->id.pci_addr.function =
> > > > +afu_pr_conf.afu_id.pci_addr.function;
> > >
> > > Can you add a function called rte_pci_addr_cpy in librte_pci and use
> > > it instead of doing this by hand here? Others would benefit from this.
> > >
> > > > +       afu_dev->id.uuid_low  = afu_pr_conf.afu_id.uuid_low;
> > > > +       afu_dev->id.uuid_high = afu_pr_conf.afu_id.uuid_high;
> > > > +       afu_dev->id.port      = afu_pr_conf.afu_id.port;
> > > > +       afu_dev->ifpga_dev    = ifpga_dev;
> > > > +
> > > > +       if (rawdev->dev_ops && rawdev->dev_ops->dev_info_get)
> > > > +               rawdev->dev_ops->dev_info_get(rawdev, afu_dev);
> > > > +
> > > > +       if (rawdev->dev_ops &&
> > > > +               rawdev->dev_ops->dev_start &&
> > > > +               rawdev->dev_ops->dev_start(rawdev))
> > > > +               goto free_dev;
> > > > +       if (pr_enable) {
> > > > +               strncpy(afu_pr_conf.bs_path, path, strlen(path));
> > >
> > > strncpy is dangerous here, please use strlcpy or snprintf.
> > > (And your use of strncpy is dangerous, you are limitting the write
> > > to the length of the source, not to the size of the destination.)
> > >
> > > > +               if (rawdev->dev_ops->firmware_load &&
> > > > +                       rawdev->dev_ops->firmware_load(rawdev,
> > > > +                                       &afu_pr_conf)){
> > > > +                       printf("firmware load error %d\n", ret);
> > > > +                       goto free_dev;
> > > > +               }
> > > > +       }
> > > > +
> > > > +       return afu_dev;
> > > > +
> > > > +free_dev:
> > > > +       free(afu_dev);
> > > > +end:
> > > > +       if (kvlist)
> > > > +               rte_kvargs_free(kvlist);
> > > > +       if (path)
> > > > +               free(path);
> > > > +
> > > > +       return NULL;
> > > > +}
> > > > +
> > > > +/*
> > > > + * Scan the content of the FPGA bus, and the devices in the
> > > > +devices
> > > > + * list
> > > > + */
> > > > +static int
> > > > +rte_ifpga_scan(void)
> > > > +{
> > > > +       struct rte_ifpga_device *ifpga_dev;
> > > > +       struct rte_devargs *devargs;
> > > > +       struct rte_kvargs *kvlist = NULL;
> > > > +       struct rte_pci_addr pci_addr;
> > > > +       struct rte_rawdev *rawdev = NULL;
> > > > +       char name[RTE_RAWDEV_NAME_MAX_LEN];
> > > > +       struct rte_afu_device *afu_dev = NULL;
> > > > +
> > > > +       /* for FPGA devices we scan the devargs_list populated via
> > > > +cmdline
> > > */
> > > > +       TAILQ_FOREACH(devargs, &devargs_list, next) {
> > > > +               if (devargs->bus != &rte_ifpga_bus.bus)
> > > > +                       continue;
> > > > +
> > > > +               kvlist = rte_kvargs_parse(devargs->args, valid_args);
> > > > +               if (!kvlist) {
> > > > +                       IFPGA_BUS_ERR("error when parsing param");
> > > > +                       goto end;
> > > > +               }
> > > > +
> > > > +               if (rte_kvargs_count(kvlist, IFPGA_ARG_BDF) == 1) {
> > > > +                       if (rte_kvargs_process(kvlist, IFPGA_ARG_BDF,
> > > > +                               &ifpga_get_bdf_arg, &pci_addr) < 0) {
> > > > +                               IFPGA_BUS_ERR("error to parse %s",
> > > > +                                       IFPGA_ARG_BDF);
> > > > +                               goto end;
> > > > +                       }
> > > > +               } else {
> > > > +                       IFPGA_BUS_ERR("arg %s is mandatory for ifpga 
> > > > bus",
> > > > +                               IFPGA_ARG_PATH);
> > > > +                       goto end;
> > > > +               }
> > > > +
> > > > +               memset(name, 0, sizeof(name));
> > > > +               snprintf(name, RTE_RAWDEV_NAME_MAX_LEN,
> > > "IFPGA:%x:%x:%x",
> > > > +               pci_addr.bus, pci_addr.devid, pci_addr.function);
> > > > +
> > > > +               rawdev = rte_rawdev_pmd_get_named_dev(name);
> > > > +               if (!rawdev)
> > > > +                       goto end;
> > > > +
> > > > +               if (ifpga_find_ifpga_dev(&pci_addr))
> > > > +                       goto end;
> > > > +
> > > > +               ifpga_dev = calloc(1, sizeof(*ifpga_dev));
> > > > +               if (!ifpga_dev)
> > > > +                       goto end;
> > > > +
> > > > +               ifpga_dev->pci_addr.domain   = pci_addr.domain;
> > > > +               ifpga_dev->pci_addr.bus      = pci_addr.bus;
> > > > +               ifpga_dev->pci_addr.devid    = pci_addr.devid;
> > > > +               ifpga_dev->pci_addr.function = pci_addr.function;
> > >
> > > Again, PCI copy would be nice instead of doing it by hand.
> > >
> > > > +               ifpga_dev->rdev = rawdev;
> > > > +               TAILQ_INIT(&ifpga_dev->afu_list);
> > > > +
> > > > +               TAILQ_INSERT_TAIL(&rte_ifpga_bus.ifpga_list, ifpga_dev,
> > > next);
> > > > +               afu_dev = rte_ifpga_scan_one(devargs, ifpga_dev);
> > > > +               if (afu_dev != NULL)
> > > > +                       TAILQ_INSERT_TAIL(&ifpga_dev->afu_list, afu_dev,
> > > next);
> > > > +       }
> > > > +
> > > > +end:
> > > > +       if (kvlist)
> > > > +               rte_kvargs_free(kvlist);
> > > > +
> > > > +       return 0;
> > > > +}
> > > > +
> > > > +static int
> > > > +ifpga_probe_one_driver(struct rte_afu_driver *drv,
> > > > +                       struct rte_afu_device *afu_dev) {
> > > > +       int ret;
> > > > +
> > > > +       if ((drv->id.pci_addr.bus == afu_dev->id.pci_addr.bus) &&
> > > > +               (drv->id.pci_addr.devid == afu_dev->id.pci_addr.devid) 
> > > > &&
> > > > +               (drv->id.pci_addr.function == 
> > > > afu_dev->id.pci_addr.function)
> > > &&
> > >
> > > There is the rte_pci_addr_cmp function that should be used here instead.
> > > Additionally, you haven't responded to my comment about ignoring the
> > > Domain part of the address. Why? Is there a reason for this? Is the
> > > domain irrelevant? If so, it should be documented. If not, please
> > > use the relevant function instead.
> > >
> > > > +               (drv->id.uuid_low == afu_dev->id.uuid_low) &&
> > > > +               (drv->id.uuid_high == afu_dev->id.uuid_high) &&
> > > > +               (drv->id.port == afu_dev->id.port)) {
> > > > +
> > > > +               afu_dev->driver = drv;
> > > > +
> > > > +               /* call the driver probe() function */
> > > > +               ret = drv->probe(afu_dev);
> > > > +               if (ret)
> > > > +                       afu_dev->driver = NULL;
> > > > +               return ret;
> > > > +       }
> > > > +
> > > > +       /* return positive value if driver doesn't support this device 
> > > > */
> > > > +       return 1;
> > > > +}
> > > > +
> > > > +static int
> > > > +ifpga_probe_all_drivers(struct rte_afu_device *afu_dev) {
> > > > +       const char *name;
> > > > +       struct rte_afu_driver *drv = NULL;
> > > > +       int rc;
> > > > +
> > > > +       if (afu_dev == NULL)
> > > > +               return -1;
> > > > +
> > > > +       /* Check if a driver is already loaded */
> > > > +       if (afu_dev->driver != NULL)
> > > > +               return 0;
> > > > +
> > > > +       name = rte_ifpga_device_name(afu_dev);
> > > > +       IFPGA_BUS_DEBUG("Search driver %s to probe device %s\n", name,
> > > > +               rte_ifpga_device_name(afu_dev));
> > > > +
> > > > +       TAILQ_FOREACH(drv, &rte_ifpga_bus.driver_list, next) {
> > > > +               rc = ifpga_probe_one_driver(drv, afu_dev);
> > > > +               if (rc < 0)
> > > > +                       /* negative value is an error */
> > > > +                       return -1;
> > > > +               if (rc > 0)
> > > > +                       /* positive value means driver doesn't support 
> > > > it */
> > > > +                       continue;
> > > > +               return 0;
> > > > +       }
> > > > +       return 1;
> > > > +}
> > > > +
> > > > +/*
> > > > + * Scan the content of the PCI bus, and call the probe() function
> > > > +for
> > >
> > > One should read IFPGA bus here I guess.
> > >
> > > > + * all registered drivers that have a matching entry in its
> > > > +id_table
> > > > + * for discovered devices.
> > > > + */
> > > > +static int
> > > > +rte_ifpga_probe(void)
> > > > +{
> > > > +       struct rte_ifpga_device *ifpga_dev;
> > > > +       struct rte_afu_device *afu_dev = NULL;
> > > > +       int ret = 0;
> > > > +
> > > > +       TAILQ_FOREACH(ifpga_dev, &rte_ifpga_bus.ifpga_list, next) {
> > > > +               TAILQ_FOREACH(afu_dev, &ifpga_dev->afu_list, next) {
> > > > +
> > > > +                       if (afu_dev->device.driver)
> > > > +                               continue;
> > > > +
> > > > +                       ret = ifpga_probe_all_drivers(afu_dev);
> > > > +                       if (ret < 0)
> > > > +                               IFPGA_BUS_ERR("failed to initialize %s
> > > device\n",
> > > > +                                       rte_ifpga_device_name(afu_dev));
> > > > +               }
> > > > +       }
> > > > +
> > > > +       return 0;
> > > > +}
> > > > +
> > > > +static int
> > > > +rte_ifpga_plug(struct rte_device *dev) {
> > > > +       return ifpga_probe_all_drivers(RTE_DEV_TO_AFU(dev));
> > > > +}
> > > > +
> > > > +static int ifpga_remove_driver(struct rte_afu_device *afu_dev) {
> > > > +       const char *name;
> > > > +       const struct rte_afu_driver *driver;
> > > > +
> > > > +       name = rte_ifpga_device_name(afu_dev);
> > > > +       if (!afu_dev->device.driver) {
> > > > +               IFPGA_BUS_DEBUG("no driver attach to device %s\n",
> > > name);
> > > > +               return 1;
> > > > +       }
> > > > +
> > > > +       driver = container_of(afu_dev->device.driver,
> > > > +               const struct rte_afu_driver,
> > > > +               driver);
> > >
> > > No RTE_DRV_TO_AFU_CONST? It would be easier to read.
> > >
> > > > +       return driver->remove(afu_dev);
> > > > +}
> > > > +
> > > > +static int
> > > > +rte_ifpga_unplug(struct rte_device *dev) {
> > > > +       struct rte_ifpga_device *ifpga_dev = NULL;
> > > > +       struct rte_afu_device *afu_dev = NULL;
> > > > +       struct rte_devargs *devargs = NULL;
> > > > +       int ret;
> > > > +
> > > > +       if (dev == NULL)
> > > > +               return -EINVAL;
> > > > +
> > > > +       afu_dev = RTE_DEV_TO_AFU(dev);
> > > > +       if (!dev)
> > > > +               return -ENOENT;
> > > > +
> > > > +       ifpga_dev = afu_dev->ifpga_dev;
> > > > +       devargs = dev->devargs;
> > > > +
> > > > +       ret = ifpga_remove_driver(afu_dev);
> > > > +       if (ret)
> > > > +               return ret;
> > > > +
> > > > +       TAILQ_REMOVE(&ifpga_dev->afu_list, afu_dev, next);
> > > > +
> > > > +       TAILQ_REMOVE(&devargs_list, devargs, next);
> > > > +
> > > > +       free(devargs->args);
> > > > +       free(devargs);
> > > > +       free(afu_dev);
> > > > +       return 0;
> > > > +
> > > > +}
> > > > +
> > > > +static struct rte_device *
> > > > +rte_ifpga_find_device(const struct rte_device *start,
> > > > +       rte_dev_cmp_t cmp, const void *data) {
> > > > +       struct rte_ifpga_device *ifpga_dev;
> > > > +       struct rte_afu_device *afu_dev;
> > > > +
> > > > +       TAILQ_FOREACH(ifpga_dev, &rte_ifpga_bus.ifpga_list, next) {
> > > > +               TAILQ_FOREACH(afu_dev, &ifpga_dev->afu_list, next) {
> > > > +                       if (start && &afu_dev->device == start) {
> > > > +                               start = NULL;
> > > > +                               continue;
> > > > +                       }
> > > > +                       if (cmp(&afu_dev->device, data) == 0)
> > > > +                               return &afu_dev->device;
> > > > +               }
> > > > +       }
> > > > +       return NULL;
> > > > +}
> > > > +static int
> > > > +rte_ifpga_parse(const char *name, void *addr) {
> > > > +       struct rte_afu_driver **out = addr;
> > > > +       struct rte_afu_driver *driver = NULL;
> > > > +
> > > > +       TAILQ_FOREACH(driver, &rte_ifpga_bus.driver_list, next) {
> > > > +               if (strncmp(driver->driver.name, name,
> > > > +                           strlen(driver->driver.name)) == 0)
> > > > +                       break;
> > > > +               if (driver->driver.alias &&
> > > > +                   strncmp(driver->driver.alias, name,
> > > > +                           strlen(driver->driver.alias)) == 0)
> > > > +                       break;
> > > > +       }
> > > > +       if (driver != NULL &&
> > > > +           addr != NULL)
> > > > +               *out = driver;
> > > > +       return driver == NULL;
> > > > +}
> > > > +
> > > > +struct rte_ifpga_bus rte_ifpga_bus = {
> > > > +        .bus = {
> > > > +                .scan        = rte_ifpga_scan,
> > > > +                .probe       = rte_ifpga_probe,
> > > > +                .find_device = rte_ifpga_find_device,
> > >
> > > Wrong indentation (2 tabs instead of one).
> > >
> > > > +            .plug        = rte_ifpga_plug,
> > > > +            .unplug      = rte_ifpga_unplug,
> > > > +            .parse       = rte_ifpga_parse,
> > > > +        },
> > > > +       .ifpga_list  = TAILQ_HEAD_INITIALIZER(rte_ifpga_bus.ifpga_list),
> > > > +       .driver_list =
> > > > +TAILQ_HEAD_INITIALIZER(rte_ifpga_bus.driver_list),
> > >
> > > Why are those lists part of the bus definition? Can you do the same
> > > as the vdev bus and have them statically declared in your source file 
> > > only?
> > >
> > > Is there a reason you want them part of the bus, do you need to
> > > access it somewhere?
> > >
> > > > +};
> > > > +
> > > > +RTE_REGISTER_BUS(IFPGA_BUS_NAME, rte_ifpga_bus.bus);
> > > > +
> > > > +RTE_INIT(ifpga_init_log)
> > > > +{
> > > > +       ifpga_bus_logtype = rte_log_register("bus.ifpga");
> > > > +       if (ifpga_bus_logtype >= 0)
> > > > +               rte_log_set_level(ifpga_bus_logtype, RTE_LOG_NOTICE); }
> > > > +
> > > > diff --git a/drivers/bus/ifpga/ifpga_common.c
> > > > b/drivers/bus/ifpga/ifpga_common.c
> > > > new file mode 100644
> > > > index 0000000..03a8d48
> > > > --- /dev/null
> > > > +++ b/drivers/bus/ifpga/ifpga_common.c
> > > > @@ -0,0 +1,141 @@
> > > > +/* SPDX-License-Identifier: BSD-3-Clause
> > > > + * Copyright(c) 2010-2017 Intel Corporation  */
> > > > +
> > > > +#include <string.h>
> > > > +#include <inttypes.h>
> > > > +#include <stdint.h>
> > > > +#include <stdlib.h>
> > > > +#include <stdio.h>
> > > > +#include <sys/queue.h>
> > > > +#include <sys/mman.h>
> > > > +#include <sys/types.h>
> > > > +#include <unistd.h>
> > > > +#include <fcntl.h>
> > > > +
> > > > +#include <rte_errno.h>
> > > > +#include <rte_bus.h>
> > > > +#include <rte_per_lcore.h>
> > > > +#include <rte_memory.h>
> > > > +#include <rte_memzone.h>
> > > > +#include <rte_eal.h>
> > > > +#include <rte_common.h>
> > > > +
> > > > +#include <rte_devargs.h>
> > > > +#include <rte_pci.h>
> > > > +#include <rte_bus_pci.h>
> > > > +#include <rte_kvargs.h>
> > > > +#include <rte_alarm.h>
> > > > +
> > > > +#include "rte_bus_ifpga.h"
> > > > +#include "ifpga_logs.h"
> > > > +#include "ifpga_common.h"
> > > > +
> > > > +int ifpga_get_string_arg(const char *key __rte_unused,
> > > > +       const char *value, void *extra_args) {
> > > > +       if (!value || !extra_args)
> > > > +               return -EINVAL;
> > > > +
> > > > +       *(char **)extra_args = strdup(value);
> > > > +
> > > > +       if (!*(char **)extra_args)
> > > > +               return -ENOMEM;
> > > > +
> > > > +       return 0;
> > > > +}
> > > > +int ifpga_get_integer32_arg(const char *key __rte_unused,
> > > > +       const char *value, void *extra_args) {
> > > > +       if (!value || !extra_args)
> > > > +               return -EINVAL;
> > > > +
> > > > +       *(int *)extra_args = strtoull(value, NULL, 0);
> > > > +
> > > > +       return 0;
> > > > +}
> > > > +int ifpga_get_integer64_arg(const char *key __rte_unused,
> > > > +       const char *value, void *extra_args) {
> > > > +       if (!value || !extra_args)
> > > > +               return -EINVAL;
> > > > +
> > > > +       *(uint64_t *)extra_args = strtoull(value, NULL, 0);
> > > > +
> > > > +       return 0;
> > > > +}
> > > > +int ifpga_get_unsigned_long(const char *str, int base) {
> > > > +       unsigned long num;
> > > > +       char *end = NULL;
> > > > +
> > > > +       errno = 0;
> > > > +
> > > > +       num = strtoul(str, &end, base);
> > > > +       if ((str[0] == '\0') || (end == NULL) || (*end != '\0') || 
> > > > (errno != 0))
> > > > +               return -1;
> > > > +
> > > > +       return num;
> > > > +
> > > > +}
> > >
> > > Missing newline here.
> > >
> > > > +int ifpga_get_bdf_arg(const char *key __rte_unused,
> > > > +       const char *value, void *extra_args) { #define MAX_PATH_LEN 1024
> > > > +       struct rte_pci_addr *addr;
> > > > +       int num[4];
> > > > +       char str[MAX_PATH_LEN];
> > > > +       int i, j;
> > > > +
> > > > +       if (!value || !extra_args)
> > > > +               return -EINVAL;
> > > > +
> > > > +       addr = (struct rte_pci_addr *)extra_args;
> > > > +       strcpy(str, value);
> > > > +       memset(num, 0, 4 * sizeof(num[0]));
> > > > +       i = strlen(str) - 1;
> > > > +       j = 3;
> > > > +       while (i > 0 && j >= 0) {
> > > > +               while ((str[i - 1] != ':' && str[i - 1] != '.') && i > 
> > > > 0)
> > > > +                       i--;
> > > > +               num[j--] = ifpga_get_unsigned_long(&str[i], 16);
> > > > +               i--;
> > > > +               if (i >= 0)
> > > > +                       str[i] = '\0';
> > > > +       }
> > > > +       addr->domain = num[0];
> > > > +       addr->bus = num[1];
> > > > +       addr->devid = num[2];
> > > > +       addr->function = num[3];
> > > > +       printf("[%s]: bdf %04d:%02d:%02d.%02d\n",
> > > > +                       __func__,
> > > > +                       addr->domain,
> > > > +                       addr->bus,
> > > > +                       addr->devid,
> > > > +                       addr->function);
> > > > +
> > > > +       return 0;
> > > > +}
> > >
> > > Use rte_pci_addr_parse instead of rewriting it please.
> > >
> > > > +
> > > > +int ifpga_afu_id_cmp(const struct rte_afu_id *afu_id0,
> > > > +       const struct rte_afu_id *afu_id1) {
> > > > +       if ((afu_id0->pci_addr.bus       == afu_id1->pci_addr.bus) &&
> > > > +               (afu_id0->pci_addr.devid     == 
> > > > afu_id1->pci_addr.devid) &&
> > > > +               (afu_id0->pci_addr.function  == 
> > > > afu_id1->pci_addr.function)
> > > &&
> > > > +               (afu_id0->uuid_low           == afu_id1->uuid_low) &&
> > > > +               (afu_id0->uuid_high          == afu_id1->uuid_high) &&
> > > > +               (afu_id0->port               == afu_id1->port)) {
> > > > +               return 0;
> > > > +       } else
> > > > +               return 1;
> > > > +}
> > >
> > > You used the exact same check in the ifpga_bus.c file, why not call
> > > this function instead?
> > >
> > > > +int ifpga_pci_addr_cmp(const struct rte_device *dev,
> > > > +       const void *_pci_addr)
> > > > +{
> > > > +       struct rte_pci_device *pdev;
> > > > +       const struct rte_pci_addr *paddr = _pci_addr;
> > > > +
> > > > +       pdev = RTE_DEV_TO_PCI(*(struct rte_device **)(void *)&dev);
> > > > +       return rte_eal_compare_pci_addr(&pdev->addr, paddr);
> > >
> > > This function is deprecated (I should mark is as so to forbid its use).
> > > Use rte_pci_addr_cmp() instead.
> > >
> > > > +}
> > > > diff --git a/drivers/bus/ifpga/ifpga_common.h
> > > > b/drivers/bus/ifpga/ifpga_common.h
> > > > new file mode 100644
> > > > index 0000000..48a5012
> > > > --- /dev/null
> > > > +++ b/drivers/bus/ifpga/ifpga_common.h
> > > > @@ -0,0 +1,22 @@
> > > > +/* SPDX-License-Identifier: BSD-3-Clause
> > > > + * Copyright(c) 2010-2017 Intel Corporation  */
> > > > +
> > > > +#ifndef _IFPGA_COMMON_H_
> > > > +#define _IFPGA_COMMON_H_
> > > > +
> > > > +int ifpga_get_string_arg(const char *key __rte_unused,
> > > > +       const char *value, void *extra_args); int
> > > > +ifpga_get_integer32_arg(const char *key __rte_unused,
> > > > +       const char *value, void *extra_args); int
> > > > +ifpga_get_integer64_arg(const char *key __rte_unused,
> > > > +       const char *value, void *extra_args); int
> > > > +ifpga_get_unsigned_long(const char *str, int base); int
> > > > +ifpga_get_bdf_arg(const char *key __rte_unused,
> > > > +       const char *value, void *extra_args); int ifpga_afu_id_cmp(const
> > > > +struct rte_afu_id *afu_id0,
> > > > +       const struct rte_afu_id *afu_id1); int ifpga_pci_addr_cmp(const
> > > > +struct rte_device *dev,
> > > > +       const void *_pci_addr);
> > > > +
> > > > +#endif /* _IFPGA_COMMON_H_ */
> > > > diff --git a/drivers/bus/ifpga/ifpga_logs.h
> > > > b/drivers/bus/ifpga/ifpga_logs.h new file mode 100644 index
> > > > 0000000..e3f4849
> > > > --- /dev/null
> > > > +++ b/drivers/bus/ifpga/ifpga_logs.h
> > > > @@ -0,0 +1,31 @@
> > > > +/* SPDX-License-Identifier: BSD-3-Clause
> > > > + * Copyright(c) 2010-2017 Intel Corporation  */
> > > > +
> > > > +#ifndef _IFPGA_LOGS_H_
> > > > +#define _IFPGA_LOGS_H_
> > > > +
> > > > +#include <rte_log.h>
> > > > +
> > > > +extern int ifpga_bus_logtype;
> > > > +
> > > > +#define IFPGA_LOG(level, fmt, args...) \
> > > > +       rte_log(RTE_LOG_ ## level, ifpga_bus_logtype, "%s(): " fmt 
> > > > "\n", \
> > > > +               __func__, ##args)
> > > > +
> > > > +#define IFPGA_BUS_LOG(level, fmt, args...) \
> > > > +       rte_log(RTE_LOG_ ## level, ifpga_bus_logtype, "%s(): " fmt 
> > > > "\n", \
> > > > +               __func__, ##args)
> > > > +
> > > > +#define IFPGA_BUS_FUNC_TRACE() IFPGA_BUS_LOG(DEBUG, ">>")
> > > > +
> > > > +#define IFPGA_BUS_DEBUG(fmt, args...) \
> > > > +       IFPGA_BUS_LOG(DEBUG, fmt, ## args) #define
> > IFPGA_BUS_INFO(fmt,
> > > > +args...) \
> > > > +       IFPGA_BUS_LOG(INFO, fmt, ## args) #define IFPGA_BUS_ERR(fmt,
> > > > +args...) \
> > > > +       IFPGA_BUS_LOG(ERR, fmt, ## args) #define IFPGA_BUS_WARN(fmt,
> > > > +args...) \
> > > > +       IFPGA_BUS_LOG(WARNING, fmt, ## args)
> > > > +
> > > > +#endif /* _IFPGA_BUS_LOGS_H_ */
> > > > diff --git a/drivers/bus/ifpga/rte_bus_ifpga.h
> > > > b/drivers/bus/ifpga/rte_bus_ifpga.h
> > > > new file mode 100644
> > > > index 0000000..7290149
> > > > --- /dev/null
> > > > +++ b/drivers/bus/ifpga/rte_bus_ifpga.h
> > > > @@ -0,0 +1,140 @@
> > > > +/* SPDX-License-Identifier: BSD-3-Clause
> > > > + * Copyright(c) 2010-2017 Intel Corporation  */
> > > > +
> > > > +#ifndef _RTE_BUS_IFPGA_H_
> > > > +#define _RTE_BUS_IFPGA_H_
> > > > +
> > > > +/**
> > > > + * @file
> > > > + *
> > > > + * RTE PCI Bus Interface
> > > > + */
> > > > +
> > > > +#ifdef __cplusplus
> > > > +extern "C" {
> > > > +#endif
> > > > +
> > > > +#include <rte_bus.h>
> > > > +#include <rte_pci.h>
> > > > +
> > > > +/** Name of Intel FPGA Bus */
> > > > +#define IFPGA_BUS_NAME ifpga
> > > > +
> > > > +/* Forward declarations */
> > > > +struct rte_ifpga_device;
> > > > +struct rte_afu_device;
> > > > +struct rte_afu_driver;
> > > > +
> > > > +/** List of Intel FPGA devices */
> > > > +TAILQ_HEAD(rte_ifpga_device_list, rte_ifpga_device);
> > > > +/** List of Intel AFU devices */
> > > > +TAILQ_HEAD(rte_afu_device_list, rte_afu_device);
> > > > +/** List of AFU drivers */
> > > > +TAILQ_HEAD(rte_afu_driver_list, rte_afu_driver);
> > >
> > > Is there a reason for you to expose your lists publicly?
> > > These symbols should be made private to your bus unless necessary.
> > >
> > > > +
> > > > +#define IFPGA_BUS_BITSTREAM_PATH_MAX_LEN 256
> > > > +
> > > > +/**
> > > > + * A structure describing an ID for a AFU driver. Each driver
> > > > +provides a
> > > > + * table of these IDs for each device that it supports.
> > > > + */
> > > > +struct rte_afu_id {
> > > > +       struct rte_pci_addr pci_addr;
> > >
> > > You use a symbol from librte_pci, without linking it.
> > >
> > > > +       uint64_t uuid_low;
> > > > +       uint64_t uuid_high;
> > > > +       int      port;
> > > > +} __attribute__ ((packed));
> > > > +
> > > > +/**
> > > > + * A structure pr configuration AFU driver.
> > > > + */
> > > > +
> > > > +struct rte_afu_pr_conf {
> > > > +       struct rte_afu_id afu_id;
> > > > +       int pr_enable;
> > > > +       char            bs_path[IFPGA_BUS_BITSTREAM_PATH_MAX_LEN];
> > > > +};
> > > > +
> > > > +#define AFU_PRI_STR_SIZE (PCI_PRI_STR_SIZE + 8)
> > > > +
> > > > +/**
> > > > + * A structure describing a fpga device.
> > > > + */
> > > > +struct rte_ifpga_device {
> > > > +       TAILQ_ENTRY(rte_ifpga_device) next;       /**< Next in device 
> > > > list. */
> > > > +       struct rte_pci_addr pci_addr;
> > > > +       struct rte_rawdev *rdev;
> > > > +       struct rte_afu_device_list afu_list;  /**< List of AFU devices
> > > > +*/ };
> > > > +
> > > > +/**
> > > > + * A structure describing a AFU device.
> > > > + */
> > > > +struct rte_afu_device {
> > > > +       TAILQ_ENTRY(rte_afu_device) next;       /**< Next in device 
> > > > list. */
> > > > +       struct rte_device device;               /**< Inherit core 
> > > > device */
> > > > +       struct rte_rawdev *rawdev;    /**< Point Rawdev */
> > > > +       struct rte_ifpga_device *ifpga_dev;    /**< Point ifpga device 
> > > > */
> > > > +       struct rte_afu_id id;                   /**< AFU id within 
> > > > FPGA. */
> > > > +       uint32_t num_region;   /**< number of regions found */
> > > > +       struct rte_mem_resource mem_resource[PCI_MAX_RESOURCE];
> > > > +                                               /**< PCI Memory Resource
> > > */
> > > > +       struct rte_intr_handle intr_handle;     /**< Interrupt handle */
> > > > +       struct rte_afu_driver *driver;          /**< Associated driver 
> > > > */
> > > > +       char path[IFPGA_BUS_BITSTREAM_PATH_MAX_LEN];
> > > > +} __attribute__ ((packed));
> > > > +
> > > > +/**
> > > > + * @internal
> > > > + * Helper macro for drivers that need to convert to struct
> > rte_afu_device.
> > > > + */
> > > > +#define RTE_DEV_TO_AFU(ptr) \
> > > > +       container_of(ptr, struct rte_afu_device, device)
> > > > +
> > > > +/**
> > > > + * Initialisation function for the driver called during PCI probing.
> > > > + */
> > > > +typedef int (afu_probe_t)(struct rte_afu_device *);
> > > > +
> > > > +/**
> > > > + * Uninitialisation function for the driver called during hotplugging.
> > > > + */
> > > > +typedef int (afu_remove_t)(struct rte_afu_device *);
> > > > +
> > > > +/**
> > > > + * A structure describing a PCI device.
> > > > + */
> > > > +struct rte_afu_driver {
> > > > +       TAILQ_ENTRY(rte_afu_driver) next;       /**< Next afu driver. */
> > > > +       struct rte_driver driver;               /**< Inherit core 
> > > > driver. */
> > > > +       afu_probe_t *probe;                     /**< Device Probe 
> > > > function. */
> > > > +       afu_remove_t *remove;                   /**< Device Remove 
> > > > function. */
> > > > +       struct rte_afu_id id;                   /**< AFU id within 
> > > > FPGA. */
> > > > +       uint32_t drv_flags;         /**< Flags contolling handling of 
> > > > device. */
> > > > +};
> > > > +
> > > > +/**
> > > > + * Structure describing the Intel FPGA bus  */ struct rte_ifpga_bus {
> > > > +       struct rte_bus bus;               /**< Inherit the generic 
> > > > class */
> > > > +       struct rte_ifpga_device_list ifpga_list;  /**< List of FPGA 
> > > > devices */
> > > > +       struct rte_afu_driver_list driver_list;  /**< List of FPGA
> > > > +drivers */ };
> > > > +
> > > > +static inline const char *
> > > > +rte_ifpga_device_name(const struct rte_afu_device *afu) {
> > > > +       if (afu && afu->device.name)
> > > > +               return afu->device.name;
> > > > +       return NULL;
> > > > +}
> > > > +
> > > > +extern struct rte_ifpga_bus rte_ifpga_bus;
> > > > +
> > > > +void rte_ifpga_driver_register(struct rte_afu_driver *driver);
> > > > +void rte_ifpga_driver_unregister(struct rte_afu_driver *driver);
> > > > +
> > > > +
> > > > +#endif /* _RTE_BUS_IFPGA_H_ */
> > > > diff --git a/drivers/bus/ifpga/rte_bus_ifpga_version.map
> > > > b/drivers/bus/ifpga/rte_bus_ifpga_version.map
> > > > new file mode 100644
> > > > index 0000000..4edc9c0
> > > > --- /dev/null
> > > > +++ b/drivers/bus/ifpga/rte_bus_ifpga_version.map
> > > > @@ -0,0 +1,8 @@
> > > > +DPDK_18.05 {
> > > > +       global:
> > > > +
> > > > +    rte_ifpga_driver_register;
> > > > +    rte_ifpga_driver_unregister;
> > >
> > > Wrong indentation.
> > > It should be one tabulation, not 4 spaces.
> > >
> > > > +
> > > > +       local: *;
> > > > +};
> > > > --
> > > > 1.8.3.1
> > > >
> > >
> > > --
> > > Gaëtan Rivet
> > > 6WIND

Reply via email to