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 6A6542BE2 for ; Thu, 3 May 2018 05:58:10 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 02 May 2018 20:58:09 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.49,356,1520924400"; d="scan'208";a="42658318" Received: from tanjianf-mobl.ccr.corp.intel.com (HELO [10.67.64.68]) ([10.67.64.68]) by fmsmga002.fm.intel.com with ESMTP; 02 May 2018 20:58:06 -0700 To: "Xu, Rosen" , dev@dpdk.org 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> Cc: declan.doherty@intel.com, bruce.richardson@intel.com, shreyansh.jain@nxp.com, ferruh.yigit@intel.com, konstantin.ananyev@intel.com, tianfei.zhang@intel.com, hao.wu@intel.com, gaetan.rivet@6wind.com From: "Tan, Jianfeng" Message-ID: <807826d0-bf13-41d4-1612-5cbdaaf82098@intel.com> Date: Thu, 3 May 2018 11:58:05 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <1524735793-37302-2-git-send-email-rosen.xu@intel.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit 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 03:58:11 -0000 For title prefix, use "bus/ifpga" instead. 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. As I understand, this part is not covered in this patch. You might want to add a note that "it will be covered in following patches". > 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. This patch is not only responsible for scan, but also other bus ops. So in your commit log, you might want to clarify this, like we introduce rte_afu_device. 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 enumerate 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. Besides, I see you are using a devargs for scanning, would you like to provide an example here? > > 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=y > CONFIG_RTE_LIBRTE_VDEV_BUS=y > > # > +# Compile the Intel FPGA bus > +# > +CONFIG_RTE_LIBRTE_IFPGA_BUS=y > + > +# > # Compile ARK PMD > # > CONFIG_RTE_LIBRTE_ARK_PMD=y > 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) += fslmc > endif > 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..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 = librte_bus_ifpga.a > + > +CFLAGS += -DALLOW_EXPERIMENTAL_API > +CFLAGS += -O3 > +CFLAGS += $(WERROR_FLAGS) > +LDLIBS += -lrte_eal > +LDLIBS += -lrte_rawdev > +LDLIBS += -lrte_kvargs > + > +# versioning export map > +EXPORT_MAP := rte_bus_ifpga_version.map > + > +# library version > +LIBABIVER := 1 > + > +SRCS-$(CONFIG_RTE_LIBRTE_IFPGA_BUS) += ifpga_bus.c > +SRCS-$(CONFIG_RTE_LIBRTE_IFPGA_BUS) += ifpga_common.c > + > +# > +# Export include files > +# > +SYMLINK-$(CONFIG_RTE_LIBRTE_IFPGA_BUS)-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..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 */ s/declare/declaration, actually, does "Intel FPGA bus on which iFPGA devices are connected" sound better? > +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 = > + TAILQ_HEAD_INITIALIZER(ifpga_device_list); > +struct afu_driver_list afu_driver_list = Why not keep this list as static? > + 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 = NULL; > + > + TAILQ_FOREACH(ifpga_dev, &ifpga_device_list, next) { > + if (rdev && > + ifpga_dev->rdev && > + ifpga_dev->rdev == 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 = 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_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, For internal functions, we'd better not use the prefix "rte". > + struct rte_ifpga_device *ifpga_dev) > +{ > + struct rte_kvargs *kvlist = 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; > + > + memset((char *)(&afu_pr_conf), 0, sizeof(struct rte_afu_pr_conf)); For "void *", no need to add explicit type cast. > + > + 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_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_PORT); > + goto end; > + } > + > + if (rte_kvargs_count(kvlist, IFPGA_AFU_BTS) == 1) { > + if (rte_kvargs_process(kvlist, IFPGA_AFU_BTS, > + &ifpga_get_string_arg, &path) < 0) { > + IFPGA_BUS_ERR("error to parse %s", s/error/failed > + 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 = 0; > + afu_pr_conf.afu_id.uuid_high = 0; > + afu_pr_conf.pr_enable = path?1:0; > + > + 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.uuid_low = 0; > + afu_dev->id.uuid_high = 0; > + 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; > + > + 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 = afu_pr_conf.afu_id.uuid_low; > + afu_dev->id.uuid_high = afu_pr_conf.afu_id.uuid_high; > + > + > + return afu_dev; > + > +free_dev: > + free(afu_dev); > +end: > + if (kvlist) You can remove above if statement for simplicity. > + rte_kvargs_free(kvlist); > + if (path) Ditto. > + 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_rawdev *rawdev = NULL; > + char *name = NULL; > + char name1[RTE_RAWDEV_NAME_MAX_LEN]; > + struct rte_afu_device *afu_dev = NULL; > + > + /* for FPGA devices we scan the devargs_list populated via cmdline */ > + RTE_EAL_DEVARGS_FOREACH("ifpga", devargs) { > + if (devargs->bus != &rte_ifpga_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_NAME) == 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); Change to use strlcpy instead. > + > + rawdev = rte_rawdev_pmd_get_named_dev(name1); > + if (!rawdev) > + goto end; > + > + if (ifpga_find_ifpga_dev(rawdev)) > + continue; > + > + ifpga_dev = calloc(1, sizeof(*ifpga_dev)); > + if (!ifpga_dev) > + goto end; > + > + ifpga_dev->rdev = rawdev; > + TAILQ_INIT(&ifpga_dev->afu_list); > + > + TAILQ_INSERT_TAIL(&ifpga_device_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) Ditto. > + rte_kvargs_free(kvlist); > + if (name) Ditto. > + 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 = afu_drv->id_table; > + ((id_table->uuid_low != 0) && (id_table->uuid_high != 0)); > + id_table++) { > + /* check if device's identifiers match the driver's ones */ > + if ((id_table->uuid_low != afu_dev->id.uuid_low) || > + (id_table->uuid_high != 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 = drv; > + afu_dev->device.driver = &drv->driver; > + > + /* call the driver probe() function */ > + ret = drv->probe(afu_dev); > + if (ret) { > + afu_dev->driver = NULL; > + afu_dev->device.driver = NULL; > + } > + > + /* return positive value if driver doesn't support this device */ 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 (=0), and probe fails (<0). > + return 0; return ret instead of 0? > +} > + > +static int > +ifpga_probe_all_drivers(struct rte_afu_device *afu_dev) > +{ > + 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; > + > + TAILQ_FOREACH(drv, &afu_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 Intel FPGA bus, and call the probe() function 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 = NULL; > + int ret = 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 = 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 = 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 = 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); > + > + 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 == 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) > +{ > + int *out = addr; > + struct rte_rawdev *rawdev = NULL; > + char rawdev_name[RTE_RAWDEV_NAME_MAX_LEN]; > + char *c1 = NULL, *c2 = NULL; > + int port = IFPGA_BUS_DEV_PORT_MAX; Duplicated spaces after "int". > + char str_port[8]; > + int str_port_len = 0; > + int ret; > + > + memset(str_port, 0, 8); > + c1 = strchr(name, '|'); > + if (c1 != NULL) { > + str_port_len = c1-name; > + c2 = c1+1; > + } > + > + if (str_port_len < 8 && > + str_port_len > 0) { > + memcpy(str_port, name, str_port_len); > + ret = sscanf(str_port, "%d", &port); > + if (ret == -1) > + return 0; > + } > + > + memset(rawdev_name, 0, sizeof(rawdev_name)); > + snprintf(rawdev_name, RTE_RAWDEV_NAME_MAX_LEN, "IFPGA:%s", c2); > + rawdev = rte_rawdev_pmd_get_named_dev(rawdev_name); > + > + if ((port < IFPGA_BUS_DEV_PORT_MAX) && > + rawdev && > + (addr != NULL)) > + *out = port; > + > + if ((port < IFPGA_BUS_DEV_PORT_MAX) && > + rawdev) > + return 0; > + else > + return 1; > +} > + > +static struct rte_bus rte_ifpga_bus = { > + .scan = rte_ifpga_scan, > + .probe = rte_ifpga_probe, > + .find_device = rte_ifpga_find_device, > + .plug = rte_ifpga_plug, > + .unplug = rte_ifpga_unplug, > + .parse = rte_ifpga_parse, > +}; > + > +RTE_REGISTER_BUS(IFPGA_BUS_NAME, rte_ifpga_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..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 = 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; > +} > + > +int ifpga_afu_id_cmp(const struct rte_afu_id *afu_id0, > + const struct rte_afu_id *afu_id1) > +{ > + if ((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; > +} > 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_logs.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.build > 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 += ['pci', 'kvargs', 'rawdev'] > +install_headers('rte_bus_ifpga.h') > +sources = 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 It seems that this struct is for describing the AFU device, instead of AFU driver, no? > + * table of these IDs for each device that it supports. > + */ > +struct rte_afu_id { > + uint64_t uuid_low; > + uint64_t uuid_high; Why not reuse struct rte_afu_uuid for above two fields? > + int port; Can you add a note on what *port* means? > +} __attribute__ ((packed)); > + > +/** > + * A structure pr configuration AFU driver. What does pr mean? Mind to add a full name for it? > + */ > + > +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_device. > + */ > +#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 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 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 */ > +}; Where is this struct used? > + > +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 = RTE_STR(nm);\ > + (afudrv).driver.alias = 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 = RTE_STR(alias) 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. > + > +#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; Above two functions shall be not exposed as APIs. > + 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 = ['dpaa', 'fslmc', 'pci', 'vdev'] > +drivers = ['dpaa', 'fslmc', 'pci', 'vdev', 'ifpga'] > std_deps = ['eal'] > config_flag_fmt = 'RTE_LIBRTE_@0@_BUS' > driver_name_fmt = '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) += -lrte_cmdline > _LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER) += -lrte_reorder > _LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lrte_sched > > +_LDLIBS-$(CONFIG_RTE_LIBRTE_IFPGA_BUS) += -lrte_bus_ifpga > + > ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) > _LDLIBS-$(CONFIG_RTE_LIBRTE_KNI) += -lrte_kni > endif