From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by dpdk.org (Postfix) with ESMTP id 3211A2BA3 for ; Thu, 3 May 2018 10:35:11 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 03 May 2018 01:35:10 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.49,357,1520924400"; d="scan'208";a="225442295" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by fmsmga006.fm.intel.com with ESMTP; 03 May 2018 01:35:10 -0700 Received: from fmsmsx115.amr.corp.intel.com (10.18.116.19) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 3 May 2018 01:35:10 -0700 Received: from cdsmsx101.ccr.corp.intel.com (172.17.3.36) by fmsmsx115.amr.corp.intel.com (10.18.116.19) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 3 May 2018 01:35:09 -0700 Received: from cdsmsx104.ccr.corp.intel.com ([169.254.4.183]) by CDSMSX101.ccr.corp.intel.com ([169.254.1.224]) with mapi id 14.03.0319.002; Thu, 3 May 2018 16:35:06 +0800 From: "Zhang, Tianfei" To: "Tan, Jianfeng" , "Xu, Rosen" , "dev@dpdk.org" CC: "Doherty, Declan" , "Richardson, Bruce" , "shreyansh.jain@nxp.com" , "Yigit, Ferruh" , "Ananyev, Konstantin" , "Wu, Hao" , "gaetan.rivet@6wind.com" Thread-Topic: [dpdk-dev] [PATCH v6 1/5] iFPGA: Add Intel FPGA BUS Library Thread-Index: AQHT3ULHmbTdgzh0hkW85Tk21biWq6Qc5T6AgAC3ozA= Date: Thu, 3 May 2018 08:35:06 +0000 Message-ID: References: <1521553556-62982-1-git-send-email-rosen.xu@intel.com> <1524735793-37302-1-git-send-email-rosen.xu@intel.com> <1524735793-37302-2-git-send-email-rosen.xu@intel.com> <807826d0-bf13-41d4-1612-5cbdaaf82098@intel.com> In-Reply-To: <807826d0-bf13-41d4-1612-5cbdaaf82098@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNDhiZjMzYjEtMjliZi00M2Y4LWE1N2EtNTE1NjNlOWYzNjNmIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjIuNS4xOCIsIlRydXN0ZWRMYWJlbEhhc2giOiIrUnRXZVQwaTludXNPWFUwTm5lZUlpeGdcL3ZRR0ZZUVYraVRPWUFUXC9tWTRLdlVDYkk1MzZXSnoxRk1mUzljUXUifQ== x-ctpclassification: CTP_NT x-originating-ip: [172.17.6.105] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v6 1/5] iFPGA: Add Intel FPGA BUS Library X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 03 May 2018 08:35:13 -0000 > -----Original Message----- > From: Tan, Jianfeng > Sent: Thursday, May 3, 2018 11:58 AM > To: Xu, Rosen ; dev@dpdk.org > Cc: Doherty, Declan ; Richardson, Bruce > ; shreyansh.jain@nxp.com; Yigit, Ferruh > ; Ananyev, Konstantin > ; Zhang, Tianfei ; > Wu, Hao ; gaetan.rivet@6wind.com > Subject: Re: [dpdk-dev] [PATCH v6 1/5] iFPGA: Add Intel FPGA BUS Library >=20 > For title prefix, use "bus/ifpga" instead. Will fix in next version. >=20 >=20 > On 4/26/2018 5:43 PM, Xu, Rosen wrote: > > From: Rosen Xu > > > > Defined FPGA-BUS for Acceleration Drivers of AFUs 1. FPGA PCI Scan > > (1st Scan) follows DPDK UIO/VFIO PCI Scan Process, probe Intel FPGA > > Rawdev Driver. >=20 > As I understand, this part is not covered in this patch. You might want t= o add > a note that "it will be covered in following patches". Yes, this scan in Intel FPGA raw device driver. Will fix this comment in ne= xt version. >=20 > > 2. AFU Scan(2nd Scan) bind DPDK driver to FPGA Partial-Bitstream. > > This scan is trigged by hotplug of IFPGA Rawdev probe, in this scan > > the AFUs will be created and their drivers are also probed. >=20 > This patch is not only responsible for scan, but also other bus ops. So i= n your > commit log, you might want to clarify this, like we introduce rte_afu_dev= ice. >=20 > I have a question, hope you can clarify it a little bit. As I understand,= this > FPGA bus are used for AFU device enumeration, and each device on this bus > needs a AFU driver to drive. But now we register AFU drivers, but enumera= te > rte_ifpga_device. Why? The reason I can think of, we need to maintain the > relationship of fpga devices and afu devices. > But I think similar problem would exist in dpaa and fslmc bus too. It is > interesting to know the best practice on this. Suppose we will have multiple FPGA chips in one system, and each FPGA chip = will has multiple AFUs, So the rte_ifpga_device will describe the FPGA chip.=20 >=20 > Besides, I see you are using a devargs for scanning, would you like to pr= ovide > an example here? This is an example run ethernet functionality on AFU 0 of FPGA (pci addr = =3D 5e:00.0)=20 #./x86_64-native-linuxapp-gcc/app/testpmd -c 0x3 -n 4 --socket-mem 1024,0 -= -huge-dir=3D/mnt/huge --vdev=20 'net_ifpga_cfg0,ifpga=3D5e:00.0,port=3D0,afu_bts=3D./ether.gbs' -- -i --no-= numa >=20 >=20 > > > > Signed-off-by: Rosen Xu > > Signed-off-by: Figo zhang > > --- > > config/common_base | 5 + > > drivers/bus/Makefile | 1 + > > drivers/bus/ifpga/Makefile | 32 ++ > > drivers/bus/ifpga/ifpga_bus.c | 503 > ++++++++++++++++++++++++++++ > > drivers/bus/ifpga/ifpga_common.c | 88 +++++ > > drivers/bus/ifpga/ifpga_common.h | 18 + > > drivers/bus/ifpga/ifpga_logs.h | 31 ++ > > drivers/bus/ifpga/meson.build | 6 + > > drivers/bus/ifpga/rte_bus_ifpga.h | 168 ++++++++++ > > drivers/bus/ifpga/rte_bus_ifpga_version.map | 10 + > > drivers/bus/meson.build | 2 +- > > mk/rte.app.mk | 2 + > > 12 files changed, 865 insertions(+), 1 deletion(-) > > 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/meson.build > > create mode 100644 drivers/bus/ifpga/rte_bus_ifpga.h > > create mode 100644 drivers/bus/ifpga/rte_bus_ifpga_version.map > > > > diff --git a/config/common_base b/config/common_base > > index 7e45412..b59f1de 100644 > > --- a/config/common_base > > +++ b/config/common_base > > @@ -148,6 +148,11 @@ CONFIG_RTE_LIBRTE_PCI_BUS=3Dy > > CONFIG_RTE_LIBRTE_VDEV_BUS=3Dy > > > > # > > +# Compile the Intel FPGA bus > > +# > > +CONFIG_RTE_LIBRTE_IFPGA_BUS=3Dy > > + > > +# > > # Compile ARK PMD > > # > > CONFIG_RTE_LIBRTE_ARK_PMD=3Dy > > diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile > > index c251b65..cff3567 100644 > > --- a/drivers/bus/Makefile > > +++ b/drivers/bus/Makefile > > @@ -9,5 +9,6 @@ DIRS-$(CONFIG_RTE_LIBRTE_FSLMC_BUS) +=3D fslmc > > endif > > 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..3ff3bdb > > --- /dev/null > > +++ b/drivers/bus/ifpga/Makefile > > @@ -0,0 +1,32 @@ > > +# SPDX-License-Identifier: BSD-3-Clause > > +# Copyright(c) 2018 Intel Corporation > > + > > +include $(RTE_SDK)/mk/rte.vars.mk > > + > > +# > > +# library name > > +# > > +LIB =3D librte_bus_ifpga.a > > + > > +CFLAGS +=3D -DALLOW_EXPERIMENTAL_API > > +CFLAGS +=3D -O3 > > +CFLAGS +=3D $(WERROR_FLAGS) > > +LDLIBS +=3D -lrte_eal > > +LDLIBS +=3D -lrte_rawdev > > +LDLIBS +=3D -lrte_kvargs > > + > > +# versioning export map > > +EXPORT_MAP :=3D rte_bus_ifpga_version.map > > + > > +# library version > > +LIBABIVER :=3D 1 > > + > > +SRCS-$(CONFIG_RTE_LIBRTE_IFPGA_BUS) +=3D ifpga_bus.c > > +SRCS-$(CONFIG_RTE_LIBRTE_IFPGA_BUS) +=3D ifpga_common.c > > + > > +# > > +# Export include files > > +# > > +SYMLINK-$(CONFIG_RTE_LIBRTE_IFPGA_BUS)-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_bu= s.c > > new file mode 100644 > > index 0000000..67c24ae > > --- /dev/null > > +++ b/drivers/bus/ifpga/ifpga_bus.c > > @@ -0,0 +1,503 @@ > > +/* SPDX-License-Identifier: BSD-3-Clause > > + * Copyright(c) 2010-2018 Intel Corporation > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#include > > +#include > > +#include > > + > > +#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; > > + > > +/* Forward declare to access Intel FPGA bus name */ >=20 > s/declare/declaration, actually, does "Intel FPGA bus on which iFPGA > devices are connected" sound better? Yes, sounds good. >=20 > > +static struct rte_bus rte_ifpga_bus; > > + > > +/** Double linked list of IFPGA device. */ > > +TAILQ_HEAD(ifpga_device_list, rte_ifpga_device); > > + > > +static struct ifpga_device_list ifpga_device_list =3D > > + TAILQ_HEAD_INITIALIZER(ifpga_device_list); > > +struct afu_driver_list afu_driver_list =3D >=20 > Why not keep this list as static? Will fix it in next version. >=20 > > + TAILQ_HEAD_INITIALIZER(afu_driver_list); > > + > > + > > +/* register a ifpga bus based driver */ > > +void rte_ifpga_driver_register(struct rte_afu_driver *driver) > > +{ > > + RTE_VERIFY(driver); > > + > > + TAILQ_INSERT_TAIL(&afu_driver_list, driver, next); > > +} > > + > > +/* un-register a fpga bus based driver */ > > +void rte_ifpga_driver_unregister(struct rte_afu_driver *driver) > > +{ > > + TAILQ_REMOVE(&afu_driver_list, driver, next); > > +} > > + > > +static struct rte_ifpga_device * > > +ifpga_find_ifpga_dev(const struct rte_rawdev *rdev) > > +{ > > + struct rte_ifpga_device *ifpga_dev =3D NULL; > > + > > + TAILQ_FOREACH(ifpga_dev, &ifpga_device_list, next) { > > + if (rdev && > > + ifpga_dev->rdev && > > + ifpga_dev->rdev =3D=3D rdev) > > + 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_NAME "ifpga" > > + IFPGA_ARG_NAME, > > +#define IFPGA_ARG_PORT "port" > > + IFPGA_ARG_PORT, > > +#define IFPGA_AFU_BTS "afu_bts" > > + IFPGA_AFU_BTS, > > + NULL > > +}; > > + > > +/* > > + * 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, >=20 > For internal functions, we'd better not use the prefix "rte". Will fix in next version. >=20 > > + struct rte_ifpga_device *ifpga_dev) > > +{ > > + struct rte_kvargs *kvlist =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; > > + > > + memset((char *)(&afu_pr_conf), 0, sizeof(struct rte_afu_pr_conf)); >=20 > For "void *", no need to add explicit type cast. Will fix in next version. >=20 > > + > > + 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_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_PORT); > > + goto end; > > + } > > + > > + if (rte_kvargs_count(kvlist, IFPGA_AFU_BTS) =3D=3D 1) { > > + if (rte_kvargs_process(kvlist, IFPGA_AFU_BTS, > > + &ifpga_get_string_arg, &path) < 0) { > > + IFPGA_BUS_ERR("error to parse %s", >=20 > s/error/failed I think error and failed is ok. >=20 > > + IFPGA_AFU_BTS); > > + goto end; > > + } > > + } else { > > + IFPGA_BUS_ERR("arg %s is mandatory for ifpga bus", > > + IFPGA_AFU_BTS); > > + goto end; > > + } > > + > > + afu_pr_conf.afu_id.uuid_low =3D 0; > > + afu_pr_conf.afu_id.uuid_high =3D 0; > > + afu_pr_conf.pr_enable =3D path?1:0; > > + > > + 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.uuid_low =3D 0; > > + afu_dev->id.uuid_high =3D 0; > > + 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; > > + > > + strncpy(afu_pr_conf.bs_path, path, sizeof(afu_pr_conf.bs_path)); > > + if (rawdev->dev_ops->firmware_load && > > + rawdev->dev_ops->firmware_load(rawdev, > > + &afu_pr_conf)){ > > + IFPGA_BUS_ERR("firmware load error %d\n", ret); > > + goto free_dev; > > + } > > + 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; > > + > > + > > + return afu_dev; > > + > > +free_dev: > > + free(afu_dev); > > +end: > > + if (kvlist) >=20 > You can remove above if statement for simplicity. >=20 > > + rte_kvargs_free(kvlist); > > + if (path) >=20 > Ditto. >=20 > > + 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_rawdev *rawdev =3D NULL; > > + char *name =3D NULL; > > + char name1[RTE_RAWDEV_NAME_MAX_LEN]; > > + struct rte_afu_device *afu_dev =3D NULL; > > + > > + /* for FPGA devices we scan the devargs_list populated via cmdline */ > > + RTE_EAL_DEVARGS_FOREACH("ifpga", devargs) { > > + if (devargs->bus !=3D &rte_ifpga_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_NAME) =3D=3D 1) { > > + if (rte_kvargs_process(kvlist, IFPGA_ARG_NAME, > > + &ifpga_get_string_arg, &name) < 0) { > > + IFPGA_BUS_ERR("error to parse %s", > > + IFPGA_ARG_NAME); > > + goto end; > > + } > > + } else { > > + IFPGA_BUS_ERR("arg %s is mandatory for ifpga bus", > > + IFPGA_ARG_NAME); > > + goto end; > > + } > > + > > + memset(name1, 0, sizeof(name1)); > > + snprintf(name1, RTE_RAWDEV_NAME_MAX_LEN, "IFPGA:%s", > name); >=20 > Change to use strlcpy instead. Got it, I agree, will fix in next version. >=20 > > + > > + rawdev =3D rte_rawdev_pmd_get_named_dev(name1); > > + if (!rawdev) > > + goto end; > > + > > + if (ifpga_find_ifpga_dev(rawdev)) > > + continue; > > + > > + ifpga_dev =3D calloc(1, sizeof(*ifpga_dev)); > > + if (!ifpga_dev) > > + goto end; > > + > > + ifpga_dev->rdev =3D rawdev; > > + TAILQ_INIT(&ifpga_dev->afu_list); > > + > > + TAILQ_INSERT_TAIL(&ifpga_device_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) >=20 > Ditto. >=20 > > + rte_kvargs_free(kvlist); > > + if (name) >=20 > Ditto. >=20 > > + free(name); > > + > > + return 0; > > +} > > + > > +/* > > + * Match the AFU Driver and AFU Device using the ID Table > > + */ > > +static int > > +rte_afu_match(const struct rte_afu_driver *afu_drv, > > + const struct rte_afu_device *afu_dev) > > +{ > > + const struct rte_afu_uuid *id_table; > > + > > + for (id_table =3D afu_drv->id_table; > > + ((id_table->uuid_low !=3D 0) && (id_table->uuid_high !=3D 0)); > > + id_table++) { > > + /* check if device's identifiers match the driver's ones */ > > + if ((id_table->uuid_low !=3D afu_dev->id.uuid_low) || > > + (id_table->uuid_high !=3D afu_dev->id.uuid_high)) > > + continue; > > + > > + return 1; > > + } > > + > > + return 0; > > +} > > + > > +static int > > +ifpga_probe_one_driver(struct rte_afu_driver *drv, > > + struct rte_afu_device *afu_dev) > > +{ > > + int ret; > > + > > + if (!rte_afu_match(drv, afu_dev)) > > + /* Match of device and driver failed */ > > + return 1; > > + > > + /* reference driver structure */ > > + afu_dev->driver =3D drv; > > + afu_dev->device.driver =3D &drv->driver; > > + > > + /* call the driver probe() function */ > > + ret =3D drv->probe(afu_dev); > > + if (ret) { > > + afu_dev->driver =3D NULL; > > + afu_dev->device.driver =3D NULL; > > + } > > + > > + /* return positive value if driver doesn't support this device */ >=20 > This comment seems wrong; you already exclude the case that this driver > does not support this device (>0); here, we only have the cases that, > probe succeeds (=3D0), and probe fails (<0). >=20 > > + return 0; >=20 > return ret instead of 0? I agree, this return ret is better. >=20 > > +} > > + > > +static int > > +ifpga_probe_all_drivers(struct rte_afu_device *afu_dev) > > +{ > > + 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; > > + > > + TAILQ_FOREACH(drv, &afu_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 Intel FPGA bus, and call the probe() functi= on for > > + * 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, &ifpga_device_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 RTE_DRV_TO_AFU_CONST(afu_dev->device.driver); > > + 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); > > + > > + rte_devargs_remove(devargs->bus->name, devargs->name); > > + 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, &ifpga_device_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) > > +{ > > + int *out =3D addr; > > + struct rte_rawdev *rawdev =3D NULL; > > + char rawdev_name[RTE_RAWDEV_NAME_MAX_LEN]; > > + char *c1 =3D NULL, *c2 =3D NULL; > > + int port =3D IFPGA_BUS_DEV_PORT_MAX; >=20 > Duplicated spaces after "int". Will fix it in next version. >=20 > > + char str_port[8]; > > + int str_port_len =3D 0; > > + int ret; > > + > > + memset(str_port, 0, 8); > > + c1 =3D strchr(name, '|'); > > + if (c1 !=3D NULL) { > > + str_port_len =3D c1-name; > > + c2 =3D c1+1; > > + } > > + > > + if (str_port_len < 8 && > > + str_port_len > 0) { > > + memcpy(str_port, name, str_port_len); > > + ret =3D sscanf(str_port, "%d", &port); > > + if (ret =3D=3D -1) > > + return 0; > > + } > > + > > + memset(rawdev_name, 0, sizeof(rawdev_name)); > > + snprintf(rawdev_name, RTE_RAWDEV_NAME_MAX_LEN, "IFPGA:%s", > c2); > > + rawdev =3D rte_rawdev_pmd_get_named_dev(rawdev_name); > > + > > + if ((port < IFPGA_BUS_DEV_PORT_MAX) && > > + rawdev && > > + (addr !=3D NULL)) > > + *out =3D port; > > + > > + if ((port < IFPGA_BUS_DEV_PORT_MAX) && > > + rawdev) > > + return 0; > > + else > > + return 1; > > +} > > + > > +static struct rte_bus rte_ifpga_bus =3D { > > + .scan =3D rte_ifpga_scan, > > + .probe =3D rte_ifpga_probe, > > + .find_device =3D rte_ifpga_find_device, > > + .plug =3D rte_ifpga_plug, > > + .unplug =3D rte_ifpga_unplug, > > + .parse =3D rte_ifpga_parse, > > +}; > > + > > +RTE_REGISTER_BUS(IFPGA_BUS_NAME, rte_ifpga_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..26fee27 > > --- /dev/null > > +++ b/drivers/bus/ifpga/ifpga_common.c > > @@ -0,0 +1,88 @@ > > +/* SPDX-License-Identifier: BSD-3-Clause > > + * Copyright(c) 2010-2018 Intel Corporation > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#include > > +#include > > +#include > > + > > +#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; > > +} > > + > > +int ifpga_afu_id_cmp(const struct rte_afu_id *afu_id0, > > + const struct rte_afu_id *afu_id1) > > +{ > > + if ((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; > > +} > > diff --git a/drivers/bus/ifpga/ifpga_common.h > b/drivers/bus/ifpga/ifpga_common.h > > new file mode 100644 > > index 0000000..b6662c8 > > --- /dev/null > > +++ b/drivers/bus/ifpga/ifpga_common.h > > @@ -0,0 +1,18 @@ > > +/* SPDX-License-Identifier: BSD-3-Clause > > + * Copyright(c) 2010-2018 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_afu_id_cmp(const struct rte_afu_id *afu_id0, > > + const struct rte_afu_id *afu_id1); > > + > > +#endif /* _IFPGA_COMMON_H_ */ > > diff --git a/drivers/bus/ifpga/ifpga_logs.h b/drivers/bus/ifpga/ifpga_l= ogs.h > > new file mode 100644 > > index 0000000..873e0a4 > > --- /dev/null > > +++ b/drivers/bus/ifpga/ifpga_logs.h > > @@ -0,0 +1,31 @@ > > +/* SPDX-License-Identifier: BSD-3-Clause > > + * Copyright(c) 2010-2018 Intel Corporation > > + */ > > + > > +#ifndef _IFPGA_LOGS_H_ > > +#define _IFPGA_LOGS_H_ > > + > > +#include > > + > > +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/meson.build b/drivers/bus/ifpga/meson.bu= ild > > new file mode 100644 > > index 0000000..4ea31f1 > > --- /dev/null > > +++ b/drivers/bus/ifpga/meson.build > > @@ -0,0 +1,6 @@ > > +# SPDX-License-Identifier: BSD-3-Clause > > +# Copyright(c) 2010-2018 Intel Corporation > > + > > +deps +=3D ['pci', 'kvargs', 'rawdev'] > > +install_headers('rte_bus_ifpga.h') > > +sources =3D files('ifpga_common.c', 'ifpga_bus.c') > > diff --git a/drivers/bus/ifpga/rte_bus_ifpga.h > b/drivers/bus/ifpga/rte_bus_ifpga.h > > new file mode 100644 > > index 0000000..53e7183 > > --- /dev/null > > +++ b/drivers/bus/ifpga/rte_bus_ifpga.h > > @@ -0,0 +1,168 @@ > > +/* SPDX-License-Identifier: BSD-3-Clause > > + * Copyright(c) 2010-2018 Intel Corporation > > + */ > > + > > +#ifndef _RTE_BUS_IFPGA_H_ > > +#define _RTE_BUS_IFPGA_H_ > > + > > +/** > > + * @file > > + * > > + * RTE Intel FPGA Bus Interface > > + */ > > + > > +#ifdef __cplusplus > > +extern "C" { > > +#endif > > + > > +#include > > +#include > > + > > +/** Name of Intel FPGA Bus */ > > +#define IFPGA_BUS_NAME ifpga > > + > > +/* Forward declarations */ > > +struct rte_afu_device; > > + > > +/** List of Intel AFU devices */ > > +TAILQ_HEAD(afu_device_list, rte_afu_device); > > +/** Double linked list of AFU device drivers. */ > > +TAILQ_HEAD(afu_driver_list, rte_afu_driver); > > + > > +#define IFPGA_BUS_BITSTREAM_PATH_MAX_LEN 256 > > + > > +struct rte_afu_uuid { > > + uint64_t uuid_low; > > + uint64_t uuid_high; > > +} __attribute__ ((packed)); > > + > > +#define IFPGA_BUS_DEV_PORT_MAX 4 > > + > > +/** > > + * A structure describing an ID for a AFU driver. Each driver provides= a >=20 > It seems that this struct is for describing the AFU device, instead of > AFU driver, no? This struct is describing the AFU device's attribute like UUID which is mat= ch AFU driver for AFU device, and port number. >=20 > > + * table of these IDs for each device that it supports. > > + */ > > +struct rte_afu_id { > > + uint64_t uuid_low; > > + uint64_t uuid_high; >=20 > Why not reuse struct rte_afu_uuid for above two fields? Yes, we can reuse the struct rte_afu_uuid. >=20 > > + int port; >=20 > Can you add a note on what *port* means? This is port number, suppose that one FPGA chip may has multiple AFUs, one AFU has one dedicate port. It is like bridge function. >=20 > > +} __attribute__ ((packed)); > > + > > +/** > > + * A structure pr configuration AFU driver. >=20 > What does pr mean? Mind to add a full name for it? PR means partial reconfiguration. >=20 > > + */ > > + > > +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_rawdev *rdev; > > + struct 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]; > > + /**< AFU 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) > > + > > +#define RTE_DRV_TO_AFU_CONST(ptr) \ > > + container_of(ptr, const struct rte_afu_driver, driver) > > + > > +/** > > + * Initialisation function for the driver called during FPGA BUS probi= ng. > > + */ > > +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 AFU 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. */ > > + const struct rte_afu_uuid *id_table; /**< AFU uuid 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 */ > > +}; >=20 > Where is this struct used? This struct no need, will remove in next version. >=20 > > + > > +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; > > +} > > + > > +/** > > + * Register a ifpga afu device driver. > > + * > > + * @param driver > > + * A pointer to a rte_afu_driver structure describing the driver > > + * to be registered. > > + */ > > +void rte_ifpga_driver_register(struct rte_afu_driver *driver); > > + > > +/** > > + * Unregister a ifpga afu device driver. > > + * > > + * @param driver > > + * A pointer to a rte_afu_driver structure describing the driver > > + * to be unregistered. > > + */ > > +void rte_ifpga_driver_unregister(struct rte_afu_driver *driver); > > + > > +#define RTE_PMD_REGISTER_AFU(nm, afudrv)\ > > +RTE_INIT(afudrvinitfn_ ##afudrv);\ > > +static const char *afudrvinit_ ## nm ## _alias;\ > > +static void afudrvinitfn_ ##afudrv(void)\ > > +{\ > > + (afudrv).driver.name =3D RTE_STR(nm);\ > > + (afudrv).driver.alias =3D afudrvinit_ ## nm ## _alias;\ > > + rte_ifpga_driver_register(&afudrv);\ > > +} \ > > +RTE_PMD_EXPORT_NAME(nm, __COUNTER__) > > + > > +#define RTE_PMD_REGISTER_AFU_ALIAS(nm, alias)\ > > +static const char *afudrvinit_ ## nm ## _alias =3D RTE_STR(alias) >=20 > Do we really need alias for new bus? Originally, alias is a way for > compatibility, but it seems that it's not necessary for new bus. >=20 > > + > > +#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..1304caf > > --- /dev/null > > +++ b/drivers/bus/ifpga/rte_bus_ifpga_version.map > > @@ -0,0 +1,10 @@ > > +DPDK_18.05 { > > + global: > > + > > + ifpga_get_integer32_arg; > > + ifpga_get_string_arg; >=20 > Above two functions shall be not exposed as APIs. Those APIs used in ifpga raw device driver. >=20 > > + rte_ifpga_driver_register; > > + rte_ifpga_driver_unregister; > > + > > + local: *; > > +}; > > diff --git a/drivers/bus/meson.build b/drivers/bus/meson.build > > index 58dfbe2..170df25 100644 > > --- a/drivers/bus/meson.build > > +++ b/drivers/bus/meson.build > > @@ -1,7 +1,7 @@ > > # SPDX-License-Identifier: BSD-3-Clause > > # Copyright(c) 2017 Intel Corporation > > > > -drivers =3D ['dpaa', 'fslmc', 'pci', 'vdev'] > > +drivers =3D ['dpaa', 'fslmc', 'pci', 'vdev', 'ifpga'] > > std_deps =3D ['eal'] > > config_flag_fmt =3D 'RTE_LIBRTE_@0@_BUS' > > driver_name_fmt =3D 'rte_bus_@0@' > > diff --git a/mk/rte.app.mk b/mk/rte.app.mk > > index a145791..9de2edb 100644 > > --- a/mk/rte.app.mk > > +++ b/mk/rte.app.mk > > @@ -107,6 +107,8 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_CMDLINE) > +=3D -lrte_cmdline > > _LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER) +=3D -lrte_reorder > > _LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) +=3D -lrte_sched > > > > +_LDLIBS-$(CONFIG_RTE_LIBRTE_IFPGA_BUS) +=3D -lrte_bus_ifpga > > + > > ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) > > _LDLIBS-$(CONFIG_RTE_LIBRTE_KNI) +=3D -lrte_kni > > endif