From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <rosen.xu@intel.com>
Received: from mga09.intel.com (mga09.intel.com [134.134.136.24])
 by dpdk.org (Postfix) with ESMTP id 111B11BB54
 for <dev@dpdk.org>; Wed,  4 Apr 2018 06:01:58 +0200 (CEST)
X-Amp-Result: SKIPPED(no attachment in message)
X-Amp-File-Uploaded: False
Received: from orsmga008.jf.intel.com ([10.7.209.65])
 by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
 03 Apr 2018 21:01:58 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.48,404,1517904000"; d="scan'208";a="30855685"
Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202])
 by orsmga008.jf.intel.com with ESMTP; 03 Apr 2018 21:01:57 -0700
Received: from fmsmsx111.amr.corp.intel.com (10.18.116.5) by
 fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS)
 id 14.3.319.2; Tue, 3 Apr 2018 21:01:57 -0700
Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by
 fmsmsx111.amr.corp.intel.com (10.18.116.5) with Microsoft SMTP Server (TLS)
 id 14.3.319.2; Tue, 3 Apr 2018 21:01:56 -0700
Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.43]) by
 shsmsx102.ccr.corp.intel.com ([169.254.2.61]) with mapi id 14.03.0319.002;
 Wed, 4 Apr 2018 12:01:54 +0800
From: "Xu, Rosen" <rosen.xu@intel.com>
To: "gaetan.rivet@6wind.com" <gaetan.rivet@6wind.com>
CC: "dev@dpdk.org" <dev@dpdk.org>, "Doherty, Declan"
 <declan.doherty@intel.com>, "Richardson, Bruce" <bruce.richardson@intel.com>, 
 "shreyansh.jain@nxp.com" <shreyansh.jain@nxp.com>, "Zhang, Tianfei"
 <tianfei.zhang@intel.com>, "Wu, Hao" <hao.wu@intel.com>
Thread-Topic: [PATCH v3 4/6] drivers/bus: Add Intel FPGA Bus Lib Code
Thread-Index: AQHTxpv88O8hV7S99EGCM0w/em2RZaPv5OKQ
Date: Wed, 4 Apr 2018 04:01:54 +0000
Message-ID: <0E78D399C70DA940A335608C6ED296D739F34FA4@SHSMSX104.ccr.corp.intel.com>
References: <1521553556-62982-1-git-send-email-rosen.xu@intel.com>
 <1522229396-17898-1-git-send-email-rosen.xu@intel.com>
 <1522229396-17898-5-git-send-email-rosen.xu@intel.com>
 <20180328135218.b3umvqqmfjnto5nl@bidouze.vm.6wind.com>
In-Reply-To: <20180328135218.b3umvqqmfjnto5nl@bidouze.vm.6wind.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNzQ3NWUwMmItZmFlOC00YzA1LWFiNWQtNjRmZmRiZmE4YjI1IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6IkFpMmpkVjZFUnRDRzV5MlVkM1BFWTA5TVZBaklVelR6bmVzUU84SU0xR3c9In0=
x-ctpclassification: CTP_NT
dlp-product: dlpe-windows
dlp-version: 11.0.0.116
dlp-reaction: no-action
x-originating-ip: [10.239.127.40]
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [dpdk-dev] [PATCH v3 4/6] drivers/bus: Add Intel FPGA Bus Lib
	Code
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Wed, 04 Apr 2018 04:01:59 -0000

Hi Gaetan,

> -----Original Message-----
> From: Ga=EBtan Rivet [mailto:gaetan.rivet@6wind.com]
> Sent: Wednesday, March 28, 2018 21:52
> To: Xu, Rosen <rosen.xu@intel.com>
> Cc: dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>;
> Richardson, Bruce <bruce.richardson@intel.com>; shreyansh.jain@nxp.com;
> Zhang, Tianfei <tianfei.zhang@intel.com>; Wu, Hao <hao.wu@intel.com>
> Subject: Re: [PATCH v3 4/6] drivers/bus: Add Intel FPGA Bus Lib Code
>=20
> On Wed, Mar 28, 2018 at 05:29:54PM +0800, Rosen Xu wrote:
> > Signed-off-by: Rosen Xu <rosen.xu@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) +=3D dpaa
> >  DIRS-$(CONFIG_RTE_LIBRTE_FSLMC_BUS) +=3D fslmc
> >  DIRS-$(CONFIG_RTE_LIBRTE_PCI_BUS) +=3D pci
> >  DIRS-$(CONFIG_RTE_LIBRTE_VDEV_BUS) +=3D vdev
> > +DIRS-$(CONFIG_RTE_LIBRTE_IFPGA_BUS) +=3D 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
>=20
> Copyrigth 2018 (same comment for most SPDX tags, I think I saw 2017 or a
> variation of it everywhere I think).

For PATCH v4 and later patches, I have change all the Copyright 2017 to Cop=
yright 2018.
=20
> > +
> > +include $(RTE_SDK)/mk/rte.vars.mk
> > +
> > +#
> > +# library name
> > +#
> > +LIB =3D librte_bus_ifpga.a
> > +
> > +CFLAGS +=3D -O3
> > +CFLAGS +=3D $(WERROR_FLAGS)
> > +
> > +# versioning export map
> > +EXPORT_MAP :=3D rte_bus_ifpga_version.map
> > +
> > +# library version
> > +LIBABIVER :=3D 1
> > +
> > +VPATH +=3D $(SRCDIR)/base
> > +
> > +SRCS-y +=3D \
> > +        ifpga_bus.c \
> > +        ifpga_common.c
> > +
> > +LDLIBS +=3D -lrte_eal
>=20
> 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.

Yes, I have fixed it and test with SHARED build in PATCH v5.
=20
> On that note, you haven't acted on my previous remarks to use the common
> PCI utilities provided.

To be honest, FPGA BUS only need IFPGA_Rawdev name not pci_addr, so I have =
removed
all pci_addr and pci common function in PATCH v5.

> > +
> > +#
> > +# Export include files
> > +#
> > +SYMLINK-y-include +=3D 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 =3D 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 =3D 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[] =3D {
> > +#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
> > +};
>=20
> So these parameters are parsed during scan afterward, taken from the PCI
> device that was probed.
>=20
> Given the kerfuffle of the VIRTUAL devtype in the EAL option, I'm not sur=
e
> how the PCI bus would react with your IFPGA PCI device being probed by it=
s
> blacklist operation: the devargs would be null. Do you segfault during yo=
ur
> scan? Have you tested this?
>=20
> Why are those parameters parsed again here on this note? Shouldn't they b=
e
> 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?
>=20
> Is there a reason that you did all this in this order? (specific initiali=
zation step
> for example?)

For PATCH v4 and later patches, I have remove all the modifications from ea=
l
library, is it ok?

> > +
> > +/*
> > + * 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 =3D NULL;
> > +	struct rte_bus *pci_bus =3D NULL;
> > +	struct rte_device *dev =3D NULL;
> > +	struct rte_rawdev *rawdev =3D NULL;
> > +	struct rte_afu_device *afu_dev =3D NULL;
> > +	struct rte_afu_pr_conf afu_pr_conf;
> > +	int ret =3D 0;
> > +	char *path =3D NULL;
> > +	int pr_enable =3D 1;
> > +	int debug =3D 0;
> > +
> > +	memset((char *)(&afu_pr_conf), 0, sizeof(struct rte_afu_pr_conf));
> > +
> > +	kvlist =3D 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) =3D=3D 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) =3D=3D 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) =3D=3D 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) =3D=3D 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) =3D=3D 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) =3D=3D 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) =3D=3D 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 =3D rte_bus_find_by_name("pci");
> > +		if (pci_bus =3D=3D NULL) {
> > +			IFPGA_BUS_ERR("unable to find PCI bus\n");
> > +			goto end;
> > +		}
> > +
> > +		dev =3D pci_bus->find_device(NULL,
> > +				ifpga_pci_addr_cmp,
> > +				&afu_pr_conf.afu_id.pci_addr);
> > +		if (dev =3D=3D 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 =3D ifpga_dev->rdev;
> > +	if (ifpga_find_afu_dev(ifpga_dev, &afu_pr_conf.afu_id))
> > +		goto end;
> > +
> > +	afu_dev =3D calloc(1, sizeof(*afu_dev));
> > +	if (!afu_dev)
> > +		goto end;
> > +
> > +	afu_dev->device.devargs =3D devargs;
> > +	afu_dev->device.numa_node =3D SOCKET_ID_ANY;
> > +	afu_dev->device.name =3D devargs->name;
> > +	afu_dev->rawdev =3D rawdev;
> > +	afu_dev->id.pci_addr.domain =3D afu_pr_conf.afu_id.pci_addr.domain;
> > +	afu_dev->id.pci_addr.bus =3D afu_pr_conf.afu_id.pci_addr.bus;
> > +	afu_dev->id.pci_addr.devid =3D afu_pr_conf.afu_id.pci_addr.devid;
> > +	afu_dev->id.pci_addr.function =3D
> > +afu_pr_conf.afu_id.pci_addr.function;
>=20
> 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.

To be honest, FPGA BUS only need IFPGA_Rawdev name not pci_addr, so I have =
remove
all pci_addr and pci common function in PATCH v5.
So these copy may no need.

> > +	afu_dev->id.uuid_low  =3D afu_pr_conf.afu_id.uuid_low;
> > +	afu_dev->id.uuid_high =3D afu_pr_conf.afu_id.uuid_high;
> > +	afu_dev->id.port      =3D afu_pr_conf.afu_id.port;
> > +	afu_dev->ifpga_dev    =3D 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));
>=20
> 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.)

Fixed in PATCH v5.

> > +		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 =3D NULL;
> > +	struct rte_pci_addr pci_addr;
> > +	struct rte_rawdev *rawdev =3D NULL;
> > +	char name[RTE_RAWDEV_NAME_MAX_LEN];
> > +	struct rte_afu_device *afu_dev =3D NULL;
> > +
> > +	/* for FPGA devices we scan the devargs_list populated via cmdline
> */
> > +	TAILQ_FOREACH(devargs, &devargs_list, next) {
> > +		if (devargs->bus !=3D &rte_ifpga_bus.bus)
> > +			continue;
> > +
> > +		kvlist =3D 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) =3D=3D 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 =3D rte_rawdev_pmd_get_named_dev(name);
> > +		if (!rawdev)
> > +			goto end;
> > +
> > +		if (ifpga_find_ifpga_dev(&pci_addr))
> > +			goto end;
> > +
> > +		ifpga_dev =3D calloc(1, sizeof(*ifpga_dev));
> > +		if (!ifpga_dev)
> > +			goto end;
> > +
> > +		ifpga_dev->pci_addr.domain   =3D pci_addr.domain;
> > +		ifpga_dev->pci_addr.bus      =3D pci_addr.bus;
> > +		ifpga_dev->pci_addr.devid    =3D pci_addr.devid;
> > +		ifpga_dev->pci_addr.function =3D pci_addr.function;
>=20
> Again, PCI copy would be nice instead of doing it by hand.
>=20
> > +		ifpga_dev->rdev =3D rawdev;
> > +		TAILQ_INIT(&ifpga_dev->afu_list);
> > +
> > +		TAILQ_INSERT_TAIL(&rte_ifpga_bus.ifpga_list, ifpga_dev,
> next);
> > +		afu_dev =3D rte_ifpga_scan_one(devargs, ifpga_dev);
> > +		if (afu_dev !=3D 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 =3D=3D afu_dev->id.pci_addr.bus) &&
> > +		(drv->id.pci_addr.devid =3D=3D afu_dev->id.pci_addr.devid) &&
> > +		(drv->id.pci_addr.function =3D=3D afu_dev->id.pci_addr.function)
> &&
>=20
> 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 domai=
n
> irrelevant? If so, it should be documented. If not, please use the releva=
nt
> function instead.

To be honest, FPGA BUS only need IFPGA_Rawdev name not pci_addr, so I have =
removed
all pci_addr and pci common function in PATCH v5.
=20
> > +		(drv->id.uuid_low =3D=3D afu_dev->id.uuid_low) &&
> > +		(drv->id.uuid_high =3D=3D afu_dev->id.uuid_high) &&
> > +		(drv->id.port =3D=3D afu_dev->id.port)) {
> > +
> > +		afu_dev->driver =3D drv;
> > +
> > +		/* call the driver probe() function */
> > +		ret =3D drv->probe(afu_dev);
> > +		if (ret)
> > +			afu_dev->driver =3D 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 =3D NULL;
> > +	int rc;
> > +
> > +	if (afu_dev =3D=3D NULL)
> > +		return -1;
> > +
> > +	/* Check if a driver is already loaded */
> > +	if (afu_dev->driver !=3D NULL)
> > +		return 0;
> > +
> > +	name =3D 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 =3D 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
>=20
> One should read IFPGA bus here I guess.

I have fixed it in PATCH v5.

> > + * 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 =3D NULL;
> > +	int ret =3D 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 =3D 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 =3D 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 =3D container_of(afu_dev->device.driver,
> > +		const struct rte_afu_driver,
> > +		driver);
>=20
> No RTE_DRV_TO_AFU_CONST? It would be easier to read.

I have add RTE_DRV_TO_AFU_CONST macro in PATCH v5.

> > +	return driver->remove(afu_dev);
> > +}
> > +
> > +static int
> > +rte_ifpga_unplug(struct rte_device *dev) {
> > +	struct rte_ifpga_device *ifpga_dev =3D NULL;
> > +	struct rte_afu_device *afu_dev =3D NULL;
> > +	struct rte_devargs *devargs =3D NULL;
> > +	int ret;
> > +
> > +	if (dev =3D=3D NULL)
> > +		return -EINVAL;
> > +
> > +	afu_dev =3D RTE_DEV_TO_AFU(dev);
> > +	if (!dev)
> > +		return -ENOENT;
> > +
> > +	ifpga_dev =3D afu_dev->ifpga_dev;
> > +	devargs =3D dev->devargs;
> > +
> > +	ret =3D 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 =3D=3D start) {
> > +				start =3D NULL;
> > +				continue;
> > +			}
> > +			if (cmp(&afu_dev->device, data) =3D=3D 0)
> > +				return &afu_dev->device;
> > +		}
> > +	}
> > +	return NULL;
> > +}
> > +static int
> > +rte_ifpga_parse(const char *name, void *addr) {
> > +	struct rte_afu_driver **out =3D addr;
> > +	struct rte_afu_driver *driver =3D NULL;
> > +
> > +	TAILQ_FOREACH(driver, &rte_ifpga_bus.driver_list, next) {
> > +		if (strncmp(driver->driver.name, name,
> > +			    strlen(driver->driver.name)) =3D=3D 0)
> > +			break;
> > +		if (driver->driver.alias &&
> > +		    strncmp(driver->driver.alias, name,
> > +			    strlen(driver->driver.alias)) =3D=3D 0)
> > +			break;
> > +	}
> > +	if (driver !=3D NULL &&
> > +	    addr !=3D NULL)
> > +		*out =3D driver;
> > +	return driver =3D=3D NULL;
> > +}
> > +
> > +struct rte_ifpga_bus rte_ifpga_bus =3D {
> > +	 .bus =3D {
> > +		 .scan        =3D rte_ifpga_scan,
> > +		 .probe       =3D rte_ifpga_probe,
> > +		 .find_device =3D rte_ifpga_find_device,
>=20
> Wrong indentation (2 tabs instead of one).

Yes, I have fixed it in PATCH v5.
=20
> > +	     .plug        =3D rte_ifpga_plug,
> > +	     .unplug      =3D rte_ifpga_unplug,
> > +	     .parse       =3D rte_ifpga_parse,
> > +	 },
> > +	.ifpga_list  =3D TAILQ_HEAD_INITIALIZER(rte_ifpga_bus.ifpga_list),
> > +	.driver_list =3D TAILQ_HEAD_INITIALIZER(rte_ifpga_bus.driver_list),
>=20
> Why are those lists part of the bus definition? Can you do the same as th=
e
> vdev bus and have them statically declared in your source file only?
>=20
> Is there a reason you want them part of the bus, do you need to access it
> somewhere?

I have fixed it and do the same as the vdev bus in PATCH v5.

> > +};
> > +
> > +RTE_REGISTER_BUS(IFPGA_BUS_NAME, rte_ifpga_bus.bus);
> > +
> > +RTE_INIT(ifpga_init_log)
> > +{
> > +	ifpga_bus_logtype =3D rte_log_register("bus.ifpga");
> > +	if (ifpga_bus_logtype >=3D 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 =3D 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 =3D 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 =3D strtoull(value, NULL, 0);
> > +
> > +	return 0;
> > +}
> > +int ifpga_get_unsigned_long(const char *str, int base) {
> > +	unsigned long num;
> > +	char *end =3D NULL;
> > +
> > +	errno =3D 0;
> > +
> > +	num =3D strtoul(str, &end, base);
> > +	if ((str[0] =3D=3D '\0') || (end =3D=3D NULL) || (*end !=3D '\0') || =
(errno !=3D 0))
> > +		return -1;
> > +
> > +	return num;
> > +
> > +}
>=20
> Missing newline here.

Fixe in PATCH v5.

> > +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 =3D (struct rte_pci_addr *)extra_args;
> > +	strcpy(str, value);
> > +	memset(num, 0, 4 * sizeof(num[0]));
> > +	i =3D strlen(str) - 1;
> > +	j =3D 3;
> > +	while (i > 0 && j >=3D 0) {
> > +		while ((str[i - 1] !=3D ':' && str[i - 1] !=3D '.') && i > 0)
> > +			i--;
> > +		num[j--] =3D ifpga_get_unsigned_long(&str[i], 16);
> > +		i--;
> > +		if (i >=3D 0)
> > +			str[i] =3D '\0';
> > +	}
> > +	addr->domain =3D num[0];
> > +	addr->bus =3D num[1];
> > +	addr->devid =3D num[2];
> > +	addr->function =3D num[3];
> > +	printf("[%s]: bdf %04d:%02d:%02d.%02d\n",
> > +			__func__,
> > +			addr->domain,
> > +			addr->bus,
> > +			addr->devid,
> > +			addr->function);
> > +
> > +	return 0;
> > +}
>=20
> Use rte_pci_addr_parse instead of rewriting it please.

To be honest, FPGA BUS only need IFPGA_Rawdev name not pci_addr, so I have =
remove
All pci_addr and pci common function such as ifpga_get_bdf_arg in PATCH v5.

> > +
> > +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       =3D=3D afu_id1->pci_addr.bus) &&
> > +		(afu_id0->pci_addr.devid     =3D=3D afu_id1->pci_addr.devid) &&
> > +		(afu_id0->pci_addr.function  =3D=3D afu_id1->pci_addr.function)
> &&
> > +		(afu_id0->uuid_low           =3D=3D afu_id1->uuid_low) &&
> > +		(afu_id0->uuid_high          =3D=3D afu_id1->uuid_high) &&
> > +		(afu_id0->port               =3D=3D afu_id1->port)) {
> > +		return 0;
> > +	} else
> > +		return 1;
> > +}
>=20
> You used the exact same check in the ifpga_bus.c file, why not call this
> function instead?

To be honest, FPGA BUS only need IFPGA_Rawdev name not pci_addr, so I have =
remove
All pci_addr and pci common function such ifpga_pci_addr_cmp in PATCH v5.
=20
> > +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 =3D _pci_addr;
> > +
> > +	pdev =3D RTE_DEV_TO_PCI(*(struct rte_device **)(void *)&dev);
> > +	return rte_eal_compare_pci_addr(&pdev->addr, paddr);
>=20
> This function is deprecated (I should mark is as so to forbid its use).
> Use rte_pci_addr_cmp() instead.

To be honest, FPGA BUS only need IFPGA_Rawdev name not pci_addr, so I have =
remove
All pci_addr and pci common function such ifpga_pci_addr_cmp in PATCH v5.

> > +}
> > 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);
>=20
> Is there a reason for you to expose your lists publicly?
> These symbols should be made private to your bus unless necessary.

Yes, it should be private, and I have fix3e it in PATCH v5.
=20
> > +
> > +#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;
>=20
> You use a symbol from librte_pci, without linking it.

To be honest, FPGA BUS only need IFPGA_Rawdev name not pci_addr, so I have =
remove
All pci_addr and pci common function in PATCH v5.

> > +	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_dev=
ice.
> > + */
> > +#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;
>=20
> Wrong indentation.
> It should be one tabulation, not 4 spaces.

Fixed in PATCH v5.

> > +
> > +	local: *;
> > +};
> > --
> > 1.8.3.1
> >
>=20
> --
> Ga=EBtan Rivet
> 6WIND