From: David Marchand <david.marchand@redhat.com>
To: Nipun Gupta <nipun.gupta@amd.com>
Cc: dev@dpdk.org, thomas@monjalon.net, hkalra@marvell.com,
anatoly.burakov@intel.com, stephen@networkplumber.org,
ferruh.yigit@amd.com, harpreet.anand@amd.com,
nikhil.agarwal@amd.com
Subject: Re: [PATCH v5 1/5] bus/cdx: introduce AMD CDX bus
Date: Thu, 1 Jun 2023 17:00:46 +0200 [thread overview]
Message-ID: <CAJFAV8wZVjZ8QXGVGU2+Eue-CEaExnHDfHVw7pay12b32EM8tg@mail.gmail.com> (raw)
In-Reply-To: <20230525100821.12148-2-nipun.gupta@amd.com>
Hello,
On Thu, May 25, 2023 at 12:08 PM Nipun Gupta <nipun.gupta@amd.com> wrote:
>
> AMD CDX bus supports multiple type of devices, which can be
> exposed to user-space via vfio-cdx.
>
> vfio-cdx provides the MMIO IO_MEMORY regions as well as the
> DMA interface for the device (IOMMU).
>
> This support aims to enable the DPDK to support the cdx
> devices in user-space using VFIO interface.
>
> Signed-off-by: Nipun Gupta <nipun.gupta@amd.com>
> Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>
> ---
> MAINTAINERS | 5 +
> doc/guides/rel_notes/release_23_07.rst | 6 +
> drivers/bus/cdx/bus_cdx_driver.h | 196 ++++++++++
> drivers/bus/cdx/cdx.c | 517 +++++++++++++++++++++++++
> drivers/bus/cdx/cdx_logs.h | 37 ++
> drivers/bus/cdx/cdx_vfio.c | 427 ++++++++++++++++++++
> drivers/bus/cdx/meson.build | 13 +
> drivers/bus/cdx/private.h | 46 +++
> drivers/bus/cdx/version.map | 11 +
> drivers/bus/meson.build | 1 +
> 10 files changed, 1259 insertions(+)
> create mode 100644 drivers/bus/cdx/bus_cdx_driver.h
> create mode 100644 drivers/bus/cdx/cdx.c
> create mode 100644 drivers/bus/cdx/cdx_logs.h
> create mode 100644 drivers/bus/cdx/cdx_vfio.c
> create mode 100644 drivers/bus/cdx/meson.build
> create mode 100644 drivers/bus/cdx/private.h
> create mode 100644 drivers/bus/cdx/version.map
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a5219926ab..c4b2b3565b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -569,6 +569,11 @@ M: Parav Pandit <parav@nvidia.com>
> M: Xueming Li <xuemingl@nvidia.com>
> F: drivers/bus/auxiliary/
>
> +AMD CDX bus driver
> +M: Nipun Gupta <nipun.gupta@amd.com>
> +M: Nikhil Agarwal <nikhil.agarwal@amd.com>
> +F: drivers/bus/cdx/
> +
> Intel FPGA bus
> M: Rosen Xu <rosen.xu@intel.com>
> F: drivers/bus/ifpga/
> diff --git a/doc/guides/rel_notes/release_23_07.rst b/doc/guides/rel_notes/release_23_07.rst
> index a9b1293689..7c6bb2b894 100644
> --- a/doc/guides/rel_notes/release_23_07.rst
> +++ b/doc/guides/rel_notes/release_23_07.rst
> @@ -55,6 +55,12 @@ New Features
> Also, make sure to start the actual text at the margin.
> =======================================================
>
> +* **Added AMD CDX bus support.**
> +
> + CDX bus driver has been added to support AMD CDX bus, which operates
> + on FPGA based CDX devices. The CDX devices are memory mapped on system
> + bus for embedded CPUs.
> +
>
> Removed Items
> -------------
> diff --git a/drivers/bus/cdx/bus_cdx_driver.h b/drivers/bus/cdx/bus_cdx_driver.h
> new file mode 100644
> index 0000000000..f1dce06a16
> --- /dev/null
> +++ b/drivers/bus/cdx/bus_cdx_driver.h
> @@ -0,0 +1,196 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright (C) 2022-2023, Advanced Micro Devices, Inc.
> + */
> +
> +#ifndef BUS_CDX_DRIVER_H
> +#define BUS_CDX_DRIVER_H
> +
> +/**
> + * @file
> + *
No need for empty line here.
> + * AMD CDX bus interface
> + */
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
Do you expect some out of tree drivers written in C++?
Otherwise, this is unneeded.
> +
> +#include <stdlib.h>
> +#include <inttypes.h>
> +
> +#include <bus_driver.h>
> +#include <dev_driver.h>
> +#include <rte_interrupts.h>
> +#include <rte_dev.h>
dev_driver.h is the "driver" header embedding rte_dev.h, no need to
include rte_dev.h again.
> +#include <rte_bus.h>
Idem, including bus_driver.h is enough.
> +
> +/* Forward declarations */
> +struct rte_cdx_device;
> +struct rte_cdx_driver;
> +
> +#define CDX_MAX_RESOURCE 4
> +
> +/** List of CDX devices */
> +RTE_TAILQ_HEAD(rte_cdx_device_list, rte_cdx_device);
> +/** List of CDX drivers */
> +RTE_TAILQ_HEAD(rte_cdx_driver_list, rte_cdx_driver);
You don't need to name/expose those types, this is only used in the
singleton object for the bus (see below with comment on rte_cdx_bus
struct).
> +
> +/* CDX Bus iterators */
> +#define FOREACH_DEVICE_ON_CDXBUS(p) \
> + RTE_TAILQ_FOREACH(p, &rte_cdx_bus.device_list, next)
> +
> +#define FOREACH_DRIVER_ON_CDXBUS(p) \
> + RTE_TAILQ_FOREACH(p, &rte_cdx_bus.driver_list, next)
These iterators can probably be hidden in the bus code.
I see no need to expose them to drivers.
So either move this to cdx.c, or if multiple code units need them,
move to private.h.
> +
> +/** Any CDX device identifier (vendor, device) */
> +#define RTE_CDX_ANY_ID (0xffff)
> +
> +#define RTE_PMD_REGISTER_CDX_TABLE(name, table) \
> +static const char DRV_EXP_TAG(name, cdx_tbl_export)[] __rte_used = \
> +RTE_STR(table)
> +
> +/**
> + * A structure describing an ID for a CDX driver. Each driver provides a
> + * table of these IDs for each device that it supports.
> + */
> +struct rte_cdx_id {
> + uint16_t vendor_id; /**< Vendor ID. */
> + uint16_t device_id; /**< Device ID. */
> +};
> +
> +/**
> + * A structure describing a CDX device.
> + */
> +struct rte_cdx_device {
> + RTE_TAILQ_ENTRY(rte_cdx_device) next; /**< Next probed CDX device. */
> + struct rte_device device; /**< Inherit core device */
> + struct rte_cdx_id id; /**< CDX ID. */
> + struct rte_mem_resource mem_resource[CDX_MAX_RESOURCE];
> + /**< CDX Memory Resource */
> +};
> +
> +/**
> + * @internal
> + * Helper macro for drivers that need to convert to struct rte_cdx_device.
> + */
> +#define RTE_DEV_TO_CDX_DEV(ptr) \
> + container_of(ptr, struct rte_cdx_device, device)
> +
> +#define RTE_DEV_TO_CDX_DEV_CONST(ptr) \
> + container_of(ptr, const struct rte_cdx_device, device)
> +
> +#define RTE_ETH_DEV_TO_CDX_DEV(eth_dev) RTE_DEV_TO_CDX_DEV((eth_dev)->device)
> +
> +#ifdef __cplusplus
> +/** C++ macro used to help building up tables of device IDs */
> +#define RTE_CDX_DEVICE(vend, dev) \
> + (vend), \
> + (dev)
> +#else
> +/** Macro used to help building up tables of device IDs */
> +#define RTE_CDX_DEVICE(vend, dev) \
> + .vendor_id = (vend), \
> + .device_id = (dev)
> +#endif
> +
> +/**
> + * Initialisation function for the driver called during CDX probing.
> + */
> +typedef int (rte_cdx_probe_t)(struct rte_cdx_driver *, struct rte_cdx_device *);
> +
> +/**
> + * Uninitialisation function for the driver called during hotplugging.
> + */
> +typedef int (rte_cdx_remove_t)(struct rte_cdx_device *);
> +
> +/**
> + * A structure describing a CDX driver.
> + */
> +struct rte_cdx_driver {
> + RTE_TAILQ_ENTRY(rte_cdx_driver) next; /**< Next in list. */
> + struct rte_driver driver; /**< Inherit core driver. */
> + struct rte_cdx_bus *bus; /**< CDX bus reference. */
> + rte_cdx_probe_t *probe; /**< Device probe function. */
> + rte_cdx_remove_t *remove; /**< Device remove function. */
> + const struct rte_cdx_id *id_table; /**< ID table, NULL terminated. */
> + uint32_t drv_flags; /**< Flags RTE_CDX_DRV_*. */
> +};
> +
> +/**
> + * Structure describing the CDX bus
> + */
> +struct rte_cdx_bus {
> + struct rte_bus bus; /**< Inherit the generic class */
> + struct rte_cdx_device_list device_list; /**< List of CDX devices */
> + struct rte_cdx_driver_list driver_list; /**< List of CDX drivers */
As I mentionned above, we can go with a simple:
RTE_TAILQ_HEAD(, rte_cdx_device) device_list;
RTE_TAILQ_HEAD(, rte_cdx_driver) driver_list;
> +};
And I don't see a need to expose the object rte_cdx_bus object in the
drivers API, so the rte_cdx_bus structure can probably move to
private.h.
> +
> +/**
> + * Get Pathname of CDX devices directory.
> + *
> + * @return
> + * sysfs path for CDX devices.
> + */
> +__rte_internal
> +const char *rte_cdx_get_sysfs_path(void);
Hard to tell without a first CDX driver.. will there be a user for this symbol?
> +
> +/**
> + * Map the CDX device resources in user space virtual memory address
> + *
> + * @param dev
> + * A pointer to a rte_cdx_device structure describing the device
> + * to use
> + *
> + * @return
> + * 0 on success, negative on error and positive if no driver
> + * is found for the device.
positive if no driver is found?
This sounds like a copy/paste.
> + */
> +__rte_internal
> +int rte_cdx_map_device(struct rte_cdx_device *dev);
> +
> +/**
> + * Unmap this device
> + *
> + * @param dev
> + * A pointer to a rte_cdx_device structure describing the device
> + * to use
> + */
> +__rte_internal
> +void rte_cdx_unmap_device(struct rte_cdx_device *dev);
> +
> +/**
> + * Register a CDX driver.
> + *
> + * @param driver
> + * A pointer to a rte_cdx_driver structure describing the driver
> + * to be registered.
> + */
> +__rte_internal
> +void rte_cdx_register(struct rte_cdx_driver *driver);
> +
> +/**
> + * Helper for CDX device registration from driver (eth, crypto, raw) instance
> + */
> +#define RTE_PMD_REGISTER_CDX(nm, cdx_drv) \
> + RTE_INIT(cdxinitfn_ ##nm) \
> + {\
> + (cdx_drv).driver.name = RTE_STR(nm);\
> + rte_cdx_register(&cdx_drv); \
> + } \
> + RTE_PMD_EXPORT_NAME(nm, __COUNTER__)
> +
> +/**
> + * Unregister a CDX driver.
> + *
> + * @param driver
> + * A pointer to a rte_cdx_driver structure describing the driver
> + * to be unregistered.
> + */
> +__rte_internal
> +void rte_cdx_unregister(struct rte_cdx_driver *driver);
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* BUS_CDX_DRIVER_H */
> diff --git a/drivers/bus/cdx/cdx.c b/drivers/bus/cdx/cdx.c
> new file mode 100644
> index 0000000000..1ddb5a92f7
> --- /dev/null
> +++ b/drivers/bus/cdx/cdx.c
> @@ -0,0 +1,517 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright (C) 2022-2023, Advanced Micro Devices, Inc.
> + */
> +
> +/*
> + * Architecture Overview
> + * =====================
> + * CDX is a Hardware Architecture designed for AMD FPGA devices. It
> + * consists of sophisticated mechanism for interaction between FPGA,
> + * Firmware and the APUs (Application CPUs).
> + *
> + * Firmware resides on RPU (Realtime CPUs) which interacts with
> + * the FPGA program manager and the APUs. The RPU provides memory-mapped
> + * interface (RPU if) which is used to communicate with APUs.
> + *
> + * The diagram below shows an overview of the AMD CDX architecture:
> + *
> + * +--------------------------------------+
> + * | DPDK |
> + * | DPDK CDX drivers |
> + * | | |
> + * | DPDK AMD CDX bus |
> + * | | |
> + * +-----------------------------|--------+
> + * |
> + * +-----------------------------|--------+
> + * | Application CPUs (APU) | |
> + * | | |
> + * | VFIO CDX driver |
> + * | Linux OS | |
> + * | Linux AMD CDX bus |
> + * | | |
> + * +-----------------------------|--------+
> + * |
> + * |
> + * +------------------------| RPU if |----+
> + * | | |
> + * | V |
> + * | Realtime CPUs (RPU) |
> + * | |
> + * +--------------------------------------+
> + * |
> + * +---------------------|----------------+
> + * | FPGA | |
> + * | +-----------------------+ |
> + * | | | | |
> + * | +-------+ +-------+ +-------+ |
> + * | | dev 1 | | dev 2 | | dev 3 | |
> + * | +-------+ +-------+ +-------+ |
> + * +--------------------------------------+
> + *
> + * The RPU firmware extracts the device information from the loaded FPGA
> + * image and implements a mechanism that allows the APU drivers to
> + * enumerate such devices (device personality and resource details) via
> + * a dedicated communication channel.
> + *
> + * VFIO CDX driver provides the CDX device resources like MMIO and interrupts
> + * to map to user-space. DPDK CDX bus uses sysfs interface and the vfio-cdx
> + * driver to discover and initialize the CDX devices for user-space
> + * applications.
> + */
> +
> +#include <string.h>
> +#include <dirent.h>
> +
> +#include <rte_eal_paging.h>
> +#include <rte_errno.h>
> +#include <rte_devargs.h>
> +#include <rte_malloc.h>
> +#include <rte_vfio.h>
> +
> +#include <eal_filesystem.h>
> +
> +#include "bus_cdx_driver.h"
> +#include "cdx_logs.h"
> +#include "private.h"
> +
> +#define SYSFS_CDX_DEVICES "/sys/bus/cdx/devices"
> +#define CDX_BUS_NAME cdx
> +#define CDX_DEV_PREFIX "cdx-"
> +
> +/**
> + * @file
> + * CDX probing using Linux sysfs.
> + */
> +
> +/* Add a device to CDX bus */
> +static void
> +cdx_add_device(struct rte_cdx_device *cdx_dev)
> +{
> + TAILQ_INSERT_TAIL(&rte_cdx_bus.device_list, cdx_dev, next);
> +}
> +
> +static int
> +cdx_get_kernel_driver_by_path(const char *filename, char *driver_name,
> + size_t len)
> +{
> + int count;
> + char path[PATH_MAX];
> + char *name;
> +
> + if (!filename || !driver_name)
> + return -1;
> +
> + count = readlink(filename, path, PATH_MAX);
> + if (count >= PATH_MAX)
> + return -1;
> +
> + /* For device does not have a driver */
> + if (count < 0)
> + return 1;
> +
> + path[count] = '\0';
> +
> + name = strrchr(path, '/');
> + if (name) {
> + strlcpy(driver_name, name + 1, len);
> + return 0;
> + }
> +
> + return -1;
> +}
> +
> +int rte_cdx_map_device(struct rte_cdx_device *dev)
> +{
> + return cdx_vfio_map_resource(dev);
> +}
> +
> +void rte_cdx_unmap_device(struct rte_cdx_device *dev)
> +{
> + cdx_vfio_unmap_resource(dev);
> +}
> +
> +static struct rte_devargs *
> +cdx_devargs_lookup(const char *dev_name)
> +{
> + struct rte_devargs *devargs;
> +
> + RTE_EAL_DEVARGS_FOREACH("cdx", devargs) {
> + if (strcmp(devargs->name, dev_name) == 0)
> + return devargs;
> + }
> + return NULL;
> +}
> +
> +static bool
> +cdx_ignore_device(const char *dev_name)
> +{
> + struct rte_devargs *devargs = cdx_devargs_lookup(dev_name);
> +
> + switch (rte_cdx_bus.bus.conf.scan_mode) {
> + case RTE_BUS_SCAN_ALLOWLIST:
> + if (devargs && devargs->policy == RTE_DEV_ALLOWED)
> + return false;
> + break;
> + case RTE_BUS_SCAN_UNDEFINED:
> + case RTE_BUS_SCAN_BLOCKLIST:
> + if (devargs == NULL || devargs->policy != RTE_DEV_BLOCKED)
> + return false;
> + break;
> + }
> + return true;
> +}
> +
> +/*
> + * Scan one cdx sysfs entry, and fill the devices list from it.
> + * It checks if the CDX device is bound to vfio-cdx driver. In case
> + * the device is vfio bound, it reads the vendor and device id and
> + * stores it for device-driver matching.
> + */
> +static int
> +cdx_scan_one(const char *dirname, const char *dev_name)
> +{
> + char filename[PATH_MAX];
> + struct rte_cdx_device *dev = NULL;
> + char driver[PATH_MAX];
> + unsigned long tmp;
> + char *name = NULL;
> + int ret;
> +
> + dev = calloc(1, sizeof(*dev));
> + if (!dev)
> + return -ENOMEM;
> +
> + name = calloc(1, RTE_DEV_NAME_MAX_LEN);
> + if (!name) {
> + ret = -ENOMEM;
> + goto err;
> + }
> +
> + dev->device.bus = &rte_cdx_bus.bus;
> + memcpy(name, dev_name, RTE_DEV_NAME_MAX_LEN);
> + dev->device.name = name;
If you dynamically allocate the name, you need to keep a reference to
it in the rte_cdx_device object for freeing in a cleanup() later.
The reason is that the rte_device object "name" field is const.
So rather than dynamically allocate to a fixed size, why not have a
name[RTE_DEV_NAME_MAX_LEN] in rte_cdx_device?
> +
> + /* parse driver */
> + snprintf(filename, sizeof(filename), "%s/driver", dirname);
> + ret = cdx_get_kernel_driver_by_path(filename, driver, sizeof(driver));
> + if (ret < 0) {
> + CDX_BUS_ERR("Fail to get kernel driver");
> + ret = -1;
> + goto err;
> + }
> +
> + /*
> + * Check if device is bound to 'vfio-cdx' driver, so that user-space
> + * can gracefully access the device.
> + */
> + if (ret || strcmp(driver, "vfio-cdx")) {
> + ret = 0;
> + goto err;
> + }
> +
> + /* get vendor id */
> + snprintf(filename, sizeof(filename), "%s/vendor", dirname);
> + if (eal_parse_sysfs_value(filename, &tmp) < 0) {
> + ret = -1;
> + goto err;
> + }
> + dev->id.vendor_id = (uint16_t)tmp;
> +
> + /* get device id */
> + snprintf(filename, sizeof(filename), "%s/device", dirname);
> + if (eal_parse_sysfs_value(filename, &tmp) < 0) {
> + free(dev);
> + return -1;
> + }
> + dev->id.device_id = (uint16_t)tmp;
> +
> + cdx_add_device(dev);
> +
> + return 0;
> +
> +err:
> + if (name)
> + free(name);
> + if (dev)
> + free(dev);
free() handles NULL fine, no need to check.
> + return ret;
> +}
> +
> +/*
> + * Scan the content of the CDX bus, and the devices in the devices
> + * list.
> + */
> +static int
> +cdx_scan(void)
> +{
> + struct dirent *e;
> + DIR *dir;
> + char dirname[PATH_MAX];
> +
> + dir = opendir(rte_cdx_get_sysfs_path());
> + if (dir == NULL) {
> + CDX_BUS_ERR("%s(): opendir failed: %s", __func__,
> + strerror(errno));
> + return -1;
> + }
> +
> + while ((e = readdir(dir)) != NULL) {
> + if (e->d_name[0] == '.')
> + continue;
> +
> + if (cdx_ignore_device(e->d_name))
> + continue;
> +
> + snprintf(dirname, sizeof(dirname), "%s/%s",
> + rte_cdx_get_sysfs_path(), e->d_name);
> +
> + if (cdx_scan_one(dirname, e->d_name) < 0)
> + goto error;
> + }
> + closedir(dir);
> + return 0;
> +
> +error:
> + closedir(dir);
> + return -1;
> +}
> +
> +const char *
> +rte_cdx_get_sysfs_path(void)
> +{
> + return SYSFS_CDX_DEVICES;
> +}
> +
> +/* map a particular resource from a file */
> +void *
> +cdx_map_resource(void *requested_addr, int fd, uint64_t offset, size_t size,
> + int additional_flags)
> +{
> + void *mapaddr;
> +
> + /* Map the cdx MMIO memory resource of device */
> + mapaddr = rte_mem_map(requested_addr, size,
> + RTE_PROT_READ | RTE_PROT_WRITE,
> + RTE_MAP_SHARED | additional_flags, fd, offset);
> + if (mapaddr == NULL) {
> + CDX_BUS_ERR("%s(): cannot map resource(%d, %p, 0x%zx, 0x%"PRIx64"): %s (%p)",
> + __func__, fd, requested_addr, size, offset,
> + rte_strerror(rte_errno), mapaddr);
> + }
> + CDX_BUS_DEBUG("CDX MMIO memory mapped at %p", mapaddr);
> +
> + return mapaddr;
> +}
> +
> +/* unmap a particular resource */
> +void
> +cdx_unmap_resource(void *requested_addr, size_t size)
> +{
> + if (requested_addr == NULL)
> + return;
> +
> + /* Unmap the CDX memory resource of device */
> + if (rte_mem_unmap(requested_addr, size)) {
> + CDX_BUS_ERR("%s(): cannot mem unmap(%p, %#zx): %s", __func__,
> + requested_addr, size, rte_strerror(rte_errno));
> + }
> + CDX_BUS_DEBUG("CDX memory unmapped at %p", requested_addr);
> +}
> +/*
> + * Match the CDX Driver and Device using device id and vendor id.
> + */
> +static int
bool
> +cdx_match(const struct rte_cdx_driver *cdx_drv,
> + const struct rte_cdx_device *cdx_dev)
> +{
> + const struct rte_cdx_id *id_table;
> +
> + for (id_table = cdx_drv->id_table; id_table->vendor_id != 0;
> + id_table++) {
> + /* check if device's identifiers match the driver's ones */
> + if (id_table->vendor_id != cdx_dev->id.vendor_id &&
> + id_table->vendor_id != RTE_CDX_ANY_ID)
> + continue;
> + if (id_table->device_id != cdx_dev->id.device_id &&
> + id_table->device_id != RTE_CDX_ANY_ID)
> + continue;
> +
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * If vendor id and device id match, call the probe() function of the
> + * driver.
> + */
> +static int
> +cdx_probe_one_driver(struct rte_cdx_driver *dr,
> + struct rte_cdx_device *dev)
> +{
> + const char *dev_name = dev->device.name;
> + bool already_probed;
> + int ret;
> +
> + if ((dr == NULL) || (dev == NULL))
> + return -EINVAL;
This symbol can't be called with dr or dev == NULL.
> +
> + /* The device is not blocked; Check if driver supports it */
> + if (!cdx_match(dr, dev))
> + /* Match of device and driver failed */
> + return 1;
> +
> + already_probed = rte_dev_is_probed(&dev->device);
> + if (already_probed) {
> + CDX_BUS_INFO("Device %s is already probed", dev->device.name);
> + return -EEXIST;
> + }
> +
> + CDX_BUS_DEBUG(" probe device %s using driver: %s", dev_name,
> + dr->driver.name);
> +
> + ret = cdx_vfio_map_resource(dev);
> + if (ret != 0) {
> + CDX_BUS_ERR("CDX map device failed: %d", ret);
> + goto error_map_device;
> + }
> +
> + /* call the driver probe() function */
> + ret = dr->probe(dr, dev);
> + if (ret) {
> + CDX_BUS_ERR("Probe CDX driver: %s device: %s failed: %d",
> + dr->driver.name, dev_name, ret);
> + goto error_probe;
> + } else {
> + dev->device.driver = &dr->driver;
> + }
> +
> + return ret;
> +
> +error_probe:
> + cdx_vfio_unmap_resource(dev);
> +error_map_device:
> + return ret;
> +}
> +
> +/*
> + * If vendor/device ID match, call the probe() function of all
> + * registered driver for the given device. Return < 0 if initialization
> + * failed, return 1 if no driver is found for this device.
> + */
> +static int
> +cdx_probe_all_drivers(struct rte_cdx_device *dev)
> +{
> + struct rte_cdx_driver *dr = NULL;
> + int rc = 0;
> +
> + if (dev == NULL)
> + return -EINVAL;
This symbol can't be called with dev == NULL.
> +
> + FOREACH_DRIVER_ON_CDXBUS(dr) {
> + rc = cdx_probe_one_driver(dr, dev);
> + if (rc < 0)
> + /* negative value is an error */
> + return rc;
> + if (rc > 0)
> + /* positive value means driver doesn't support it */
> + continue;
> + return 0;
> + }
> + return 1;
> +}
> +
> +/*
> + * Scan the content of the CDX bus, and call the probe() function for
> + * all registered drivers that have a matching entry in its id_table
> + * for discovered devices.
> + */
> +static int
> +cdx_probe(void)
> +{
> + struct rte_cdx_device *dev = NULL;
> + size_t probed = 0, failed = 0;
> + int ret = 0;
> +
> + FOREACH_DEVICE_ON_CDXBUS(dev) {
> + probed++;
> +
> + ret = cdx_probe_all_drivers(dev);
> + if (ret < 0) {
> + CDX_BUS_ERR("Requested device %s cannot be used",
> + dev->device.name);
> + rte_errno = errno;
> + failed++;
> + ret = 0;
> + }
> + }
> +
> + return (probed && probed == failed) ? -1 : 0;
> +}
> +
> +static int
> +cdx_parse(const char *name, void *addr)
> +{
> + const char **out = addr;
> + int ret;
> +
> + ret = strncmp(name, CDX_DEV_PREFIX, strlen(CDX_DEV_PREFIX));
> +
> + if (ret == 0 && addr)
> + *out = name;
> +
> + return ret;
> +}
> +
> +/* register a driver */
> +void
> +rte_cdx_register(struct rte_cdx_driver *driver)
> +{
> + TAILQ_INSERT_TAIL(&rte_cdx_bus.driver_list, driver, next);
> + driver->bus = &rte_cdx_bus;
> +}
> +
> +/* unregister a driver */
> +void
> +rte_cdx_unregister(struct rte_cdx_driver *driver)
> +{
> + TAILQ_REMOVE(&rte_cdx_bus.driver_list, driver, next);
> + driver->bus = NULL;
> +}
> +
> +static struct rte_device *
> +cdx_find_device(const struct rte_device *start, rte_dev_cmp_t cmp,
> + const void *data)
> +{
> + const struct rte_cdx_device *cdx_start;
> + struct rte_cdx_device *cdx_dev;
> +
> + if (start != NULL) {
> + cdx_start = RTE_DEV_TO_CDX_DEV_CONST(start);
> + cdx_dev = TAILQ_NEXT(cdx_start, next);
> + } else {
> + cdx_dev = TAILQ_FIRST(&rte_cdx_bus.device_list);
> + }
> + while (cdx_dev != NULL) {
> + if (cmp(&cdx_dev->device, data) == 0)
> + return &cdx_dev->device;
> + cdx_dev = TAILQ_NEXT(cdx_dev, next);
> + }
> + return NULL;
> +}
> +
> +struct rte_cdx_bus rte_cdx_bus = {
> + .bus = {
> + .scan = cdx_scan,
> + .probe = cdx_probe,
> + .find_device = cdx_find_device,
> + .parse = cdx_parse,
I see neither unplug, nor cleanup op which is strange because this API
provides a way to unregister drivers.
> + },
> + .device_list = TAILQ_HEAD_INITIALIZER(rte_cdx_bus.device_list),
> + .driver_list = TAILQ_HEAD_INITIALIZER(rte_cdx_bus.driver_list),
> +};
> +
> +RTE_REGISTER_BUS(cdx, rte_cdx_bus.bus);
> +RTE_LOG_REGISTER_DEFAULT(cdx_logtype_bus, NOTICE);
[snip]
> diff --git a/drivers/bus/cdx/meson.build b/drivers/bus/cdx/meson.build
> new file mode 100644
> index 0000000000..f2ca104d34
> --- /dev/null
> +++ b/drivers/bus/cdx/meson.build
> @@ -0,0 +1,13 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright (C) 2022-2023, Advanced Micro Devices, Inc.
> +
> +if not is_linux
> + build = false
> + reason = 'only supported on Linux'
> +endif
> +
> +driver_sdk_headers = files('bus_cdx_driver.h')
> +sources = files(
> + 'cdx.c',
> + 'cdx_vfio.c',
> +)
> diff --git a/drivers/bus/cdx/private.h b/drivers/bus/cdx/private.h
> new file mode 100644
> index 0000000000..c5ce5a46b8
> --- /dev/null
> +++ b/drivers/bus/cdx/private.h
> @@ -0,0 +1,46 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright (C) 2022-2023, Advanced Micro Devices, Inc.
> + */
> +
> +#ifndef CDX_PRIVATE_H
> +#define CDX_PRIVATE_H
> +
> +#include "bus_cdx_driver.h"
> +
> +extern struct rte_cdx_bus rte_cdx_bus;
This could move to cdx.c.
> +
> +/**
> + * Map a particular resource from a file.
> + *
> + * @param requested_addr
> + * The starting address for the new mapping range.
> + * @param fd
> + * The file descriptor.
> + * @param offset
> + * The offset for the mapping range.
> + * @param size
> + * The size for the mapping range.
> + * @param additional_flags
> + * The additional rte_mem_map() flags for the mapping range.
> + * @return
> + * - On success, the function returns a pointer to the mapped area.
> + * - On error, NULL is returned.
> + */
> +void *cdx_map_resource(void *requested_addr, int fd, uint64_t offset,
> + size_t size, int additional_flags);
> +
> +/**
> + * Unmap a particular resource.
> + *
> + * @param requested_addr
> + * The address for the unmapping range.
> + * @param size
> + * The size for the unmapping range.
> + */
> +void cdx_unmap_resource(void *requested_addr, size_t size);
> +
> +/* map/unmap VFIO resource */
> +int cdx_vfio_map_resource(struct rte_cdx_device *dev);
> +int cdx_vfio_unmap_resource(struct rte_cdx_device *dev);
> +
> +#endif /* CDX_PRIVATE_H */
--
David Marchand
next prev parent reply other threads:[~2023-06-01 15:01 UTC|newest]
Thread overview: 102+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-24 14:07 [RFC PATCH 0/6] add support for " Nipun Gupta
2023-01-24 14:07 ` [RFC PATCH 1/6] bus/cdx: introduce cdx bus Nipun Gupta
2023-01-24 14:07 ` [RFC PATCH 2/6] bus/cdx: add dma map and unmap support Nipun Gupta
2023-01-24 14:07 ` [RFC PATCH 3/6] bus/cdx: add support for MSI Nipun Gupta
2023-01-24 14:07 ` [RFC PATCH 4/6] bus/cdx: support plug unplug and dev iterator Nipun Gupta
2023-01-24 14:07 ` [RFC PATCH 5/6] bus: enable cdx bus Nipun Gupta
2023-01-24 14:07 ` [RFC PATCH 6/6] config/arm: add AMD CDX Nipun Gupta
2023-04-07 6:01 ` [PATCH 0/6] add support for CDX bus Nipun Gupta
2023-04-07 6:01 ` [PATCH 1/6] bus/cdx: introduce cdx bus Nipun Gupta
2023-04-07 6:01 ` [PATCH 2/6] bus/cdx: add dma map and unmap support Nipun Gupta
2023-04-07 6:01 ` [PATCH 3/6] bus/cdx: add support for MSI Nipun Gupta
2023-04-07 6:01 ` [PATCH 4/6] bus/cdx: support plug unplug and dev iterator Nipun Gupta
2023-04-07 6:01 ` [PATCH 5/6] bus: enable cdx bus Nipun Gupta
2023-04-07 6:01 ` [PATCH 6/6] config/arm: add AMD CDX Nipun Gupta
2023-04-07 7:18 ` [PATCH 0/6] add support for CDX bus David Marchand
2023-04-07 7:29 ` Nipun Gupta
2023-04-13 13:25 ` Gupta, Nipun
2023-04-13 13:26 ` [PATCH v2 " Nipun Gupta
2023-04-13 13:26 ` [PATCH v2 1/6] bus/cdx: introduce cdx bus Nipun Gupta
2023-04-14 16:45 ` Ferruh Yigit
2023-04-16 9:20 ` Gupta, Nipun
2023-04-13 13:27 ` [PATCH v2 2/6] bus/cdx: add dma map and unmap support Nipun Gupta
2023-04-13 13:27 ` [PATCH v2 3/6] bus/cdx: add support for MSI Nipun Gupta
2023-04-14 16:45 ` Ferruh Yigit
2023-04-16 9:20 ` Gupta, Nipun
2023-04-13 13:27 ` [PATCH v2 4/6] bus/cdx: support plug unplug and dev iterator Nipun Gupta
2023-04-13 13:27 ` [PATCH v2 5/6] bus: enable cdx bus Nipun Gupta
2023-04-14 16:45 ` Ferruh Yigit
2023-04-16 9:21 ` Gupta, Nipun
2023-04-13 13:27 ` [PATCH v2 6/6] config/arm: add AMD CDX Nipun Gupta
2023-04-14 16:45 ` [PATCH v2 0/6] add support for CDX bus Ferruh Yigit
2023-04-21 14:54 ` [PATCH v3 0/5] Support AMD CDX bus, for FPGA based CDX devices. The CDX Nipun Gupta
2023-04-21 14:54 ` [PATCH v3 1/5] bus/cdx: introduce cdx bus Nipun Gupta
2023-04-21 14:54 ` [PATCH v3 2/5] bus/cdx: add DMA map and unmap support Nipun Gupta
2023-04-21 14:54 ` [PATCH v3 3/5] bus/cdx: add support for MSI Nipun Gupta
2023-04-21 14:54 ` [PATCH v3 4/5] bus/cdx: support plug unplug and dev iterator Nipun Gupta
2023-04-21 14:54 ` [PATCH v3 5/5] config/arm: add AMD CDX Nipun Gupta
2023-05-04 15:28 ` Ferruh Yigit
2023-05-08 10:24 ` Gupta, Nipun
2023-05-08 10:44 ` Thomas Monjalon
2023-05-08 10:48 ` Gupta, Nipun
2023-05-08 11:26 ` Ferruh Yigit
2023-05-08 17:16 ` Honnappa Nagarahalli
2023-05-08 17:47 ` Ferruh Yigit
2023-05-09 4:35 ` Gupta, Nipun
2023-05-09 5:55 ` Ruifeng Wang
2023-06-14 10:30 ` Ferruh Yigit
2023-06-15 7:00 ` Ruifeng Wang
2023-05-08 11:18 ` [PATCH v4 0/4] Support AMD CDX bus, for FPGA based CDX devices. The CDX Nipun Gupta
2023-05-08 11:18 ` [PATCH v4 1/4] bus/cdx: introduce cdx bus Nipun Gupta
2023-05-09 6:54 ` Xia, Chenbo
2023-05-09 11:09 ` Gupta, Nipun
2023-05-09 11:25 ` Ferruh Yigit
2023-05-10 1:29 ` Xia, Chenbo
2023-05-24 11:14 ` Thomas Monjalon
2023-05-24 17:04 ` Gupta, Nipun
2023-05-08 11:18 ` [PATCH v4 2/4] bus/cdx: add DMA map and unmap support Nipun Gupta
2023-05-08 11:18 ` [PATCH v4 3/4] bus/cdx: add support for MSI Nipun Gupta
2023-05-24 11:06 ` Thomas Monjalon
2023-05-24 17:06 ` Gupta, Nipun
2023-05-08 11:18 ` [PATCH v4 4/4] bus/cdx: support plug unplug and dev iterator Nipun Gupta
2023-05-12 10:25 ` [PATCH v4 0/4] Support AMD CDX bus, for FPGA based CDX devices. The CDX Ferruh Yigit
2023-05-25 10:08 ` [PATCH v5 0/5] Support AMD CDX bus Nipun Gupta
2023-05-25 10:08 ` [PATCH v5 1/5] bus/cdx: introduce " Nipun Gupta
2023-06-01 15:00 ` David Marchand [this message]
2023-06-01 18:10 ` Nipun Gupta
2023-06-05 8:04 ` Nipun Gupta
2023-05-25 10:08 ` [PATCH v5 2/5] bus/cdx: add DMA map and unmap support Nipun Gupta
2023-06-01 15:07 ` David Marchand
2023-05-25 10:08 ` [PATCH v5 3/5] eal/interrupts: add IRQ count in interrupt handle Nipun Gupta
2023-06-01 15:25 ` David Marchand
2023-06-01 18:04 ` Nipun Gupta
2023-06-01 18:18 ` Gupta, Nipun
2023-06-06 7:18 ` [EXT] " Harman Kalra
2023-06-06 7:27 ` Nipun Gupta
2023-06-07 5:45 ` Harman Kalra
2023-05-25 10:08 ` [PATCH v5 4/5] bus/cdx: add support for MSI Nipun Gupta
2023-06-01 15:09 ` David Marchand
2023-06-05 8:05 ` Nipun Gupta
2023-05-25 10:08 ` [PATCH v5 5/5] bus/cdx: support plug unplug and dev iterator Nipun Gupta
2023-06-05 13:26 ` [PATCH v6 0/4] Support AMD CDX bus Nipun Gupta
2023-06-05 13:26 ` [PATCH v6 1/4] bus/cdx: introduce " Nipun Gupta
2023-06-06 8:55 ` Thomas Monjalon
2023-06-05 13:26 ` [PATCH v6 2/4] bus/cdx: add DMA map and unmap support Nipun Gupta
2023-06-05 13:26 ` [PATCH v6 3/4] bus/cdx: add support for MSI Nipun Gupta
2023-06-06 9:30 ` David Marchand
2023-06-06 9:42 ` Nipun Gupta
2023-06-05 13:26 ` [PATCH v6 4/4] bus/cdx: support plug unplug and dev iterator Nipun Gupta
2023-06-06 10:02 ` [PATCH v7 0/4] Support AMD CDX bus Nipun Gupta
2023-06-06 10:02 ` [PATCH v7 1/4] bus/cdx: introduce " Nipun Gupta
2023-06-06 13:00 ` Thomas Monjalon
2023-06-06 13:38 ` Nipun Gupta
2023-06-06 13:43 ` Thomas Monjalon
2023-06-06 10:02 ` [PATCH v7 2/4] bus/cdx: add DMA map and unmap support Nipun Gupta
2023-06-06 10:02 ` [PATCH v7 3/4] bus/cdx: add support for MSI Nipun Gupta
2023-06-06 10:02 ` [PATCH v7 4/4] bus/cdx: support plug unplug and dev iterator Nipun Gupta
2023-06-07 4:24 ` [PATCH v8 0/4] Support AMD CDX bus Nipun Gupta
2023-06-07 4:24 ` [PATCH v8 1/4] bus/cdx: introduce " Nipun Gupta
2023-06-07 4:24 ` [PATCH v8 2/4] bus/cdx: add DMA map and unmap support Nipun Gupta
2023-06-07 4:24 ` [PATCH v8 3/4] bus/cdx: add support for MSI Nipun Gupta
2023-06-07 4:24 ` [PATCH v8 4/4] bus/cdx: support plug unplug and dev iterator Nipun Gupta
2023-06-07 13:36 ` [PATCH v8 0/4] Support AMD CDX bus Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAJFAV8wZVjZ8QXGVGU2+Eue-CEaExnHDfHVw7pay12b32EM8tg@mail.gmail.com \
--to=david.marchand@redhat.com \
--cc=anatoly.burakov@intel.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@amd.com \
--cc=harpreet.anand@amd.com \
--cc=hkalra@marvell.com \
--cc=nikhil.agarwal@amd.com \
--cc=nipun.gupta@amd.com \
--cc=stephen@networkplumber.org \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).