From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 2E76C42C04; Thu, 1 Jun 2023 17:01:08 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 1A75240DDC; Thu, 1 Jun 2023 17:01:08 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mails.dpdk.org (Postfix) with ESMTP id 349EC406BA for ; Thu, 1 Jun 2023 17:01:06 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1685631665; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=usJ+M5fUxIxBtNfideBxm/Jjns4vBcsa0D5UN0XzdYM=; b=B3YlQJQACY/aTKXAzJemld0lu2myWMuI0m4frJ/Zi+hNc6ODnPUhC4sZiAx4y0Kn2hPkKj pyOXaS9V3EVmzEsJwQWHo4DMNecDklSPVFi/H68x5RNjAKUMHApLYYqC2R0aCc/XoFcK0B 48tIqMl2f8mpLpkktipFV3foAUSLOIY= Received: from mail-pl1-f198.google.com (mail-pl1-f198.google.com [209.85.214.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-13-QDezksV0NOKdG1gTFHoo9g-1; Thu, 01 Jun 2023 11:01:02 -0400 X-MC-Unique: QDezksV0NOKdG1gTFHoo9g-1 Received: by mail-pl1-f198.google.com with SMTP id d9443c01a7336-1b02cd4b829so7245345ad.1 for ; Thu, 01 Jun 2023 08:01:01 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1685631658; x=1688223658; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=usJ+M5fUxIxBtNfideBxm/Jjns4vBcsa0D5UN0XzdYM=; b=VS6PqzTOhciz+bYl/IkFHFl69HvobIUD5MXrXCUeoRg0nqMIfDsOY+SPouXjCFAM7m v3zS0dEc80KfN/cqL6RDepwlmSnUlYyZRDLkE0+l4MvAz7P4CIlS2rtDs+tvx9LqR/tp 4O5WEwB9fa/HSrVzSMAT7mhNmywS/5uV0uyEcGGwMYi8Tk/rFeNzn1TGSupau1fV/rah 6KfC2xdA5O8hGHRIfn8k1qOcwLWjqwGO/jEKI0UnN2FhhwGRQRbrb6TCqA8cO+8Epl/I yJNqwP73t/0xvwDutCCe+6aDgwojyG8U8mSaHsLFGy91KYGXhJOhqKEn2z/CoL4+z9uY RhDA== X-Gm-Message-State: AC+VfDzkBnRqBS1UxnswMOKAKwAtD3N1+gojWCTYkT9SwXYiwO/ybOWp SL1+puuN+fpB942IrVKfOPW32qsRkJaWKxNgAHwTlWnXy31yVdQhrs5AnW6105WyV9KNaZe6o3W Dvh8ETNEv4CcQbh4+3lk= X-Received: by 2002:a17:902:f54c:b0:1a9:6a10:70e9 with SMTP id h12-20020a170902f54c00b001a96a1070e9mr2570732plf.33.1685631657937; Thu, 01 Jun 2023 08:00:57 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ4nCgB/wHxFMc1DBfeHFfHxLrLZsc78aTt0xKDGtWFWXhp60umLSOq3bKgAFtM07MO3MYwoxdQIs2vmJgZlnjw= X-Received: by 2002:a17:902:f54c:b0:1a9:6a10:70e9 with SMTP id h12-20020a170902f54c00b001a96a1070e9mr2570690plf.33.1685631657308; Thu, 01 Jun 2023 08:00:57 -0700 (PDT) MIME-Version: 1.0 References: <20230124140746.594066-1-nipun.gupta@amd.com> <20230525100821.12148-1-nipun.gupta@amd.com> <20230525100821.12148-2-nipun.gupta@amd.com> In-Reply-To: <20230525100821.12148-2-nipun.gupta@amd.com> From: David Marchand Date: Thu, 1 Jun 2023 17:00:46 +0200 Message-ID: Subject: Re: [PATCH v5 1/5] bus/cdx: introduce AMD CDX bus To: Nipun Gupta 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 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Hello, On Thu, May 25, 2023 at 12:08=E2=80=AFPM Nipun Gupta = 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 > Acked-by: Ferruh Yigit > --- > 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 > M: Xueming Li > F: drivers/bus/auxiliary/ > > +AMD CDX bus driver > +M: Nipun Gupta > +M: Nikhil Agarwal > +F: drivers/bus/cdx/ > + > Intel FPGA bus > M: Rosen Xu > F: drivers/bus/ifpga/ > diff --git a/doc/guides/rel_notes/release_23_07.rst b/doc/guides/rel_note= s/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. > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D > > +* **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_d= river.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 > +#include > + > +#include > +#include > +#include > +#include dev_driver.h is the "driver" header embedding rte_dev.h, no need to include rte_dev.h again. > +#include 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 =3D \ > +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 devi= ce. */ > + 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_devic= e. > + */ > +#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_d= ev)->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 =3D (vend), \ > + .device_id =3D (dev) > +#endif > + > +/** > + * Initialisation function for the driver called during CDX probing. > + */ > +typedef int (rte_cdx_probe_t)(struct rte_cdx_driver *, struct rte_cdx_de= vice *); > + > +/** > + * 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 functio= n. */ > + rte_cdx_remove_t *remove; /**< Device remove functi= on. */ > + const struct rte_cdx_id *id_table; /**< ID table, NULL termi= nated. */ > + 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 sym= bol? > + > +/** > + * 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) ins= tance > + */ > +#define RTE_PMD_REGISTER_CDX(nm, cdx_drv) \ > + RTE_INIT(cdxinitfn_ ##nm) \ > + {\ > + (cdx_drv).driver.name =3D 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 > + * =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > + * 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 inter= rupts > + * 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 > +#include > + > +#include > +#include > +#include > +#include > +#include > + > +#include > + > +#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 =3D readlink(filename, path, PATH_MAX); > + if (count >=3D PATH_MAX) > + return -1; > + > + /* For device does not have a driver */ > + if (count < 0) > + return 1; > + > + path[count] =3D '\0'; > + > + name =3D 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) =3D=3D 0) > + return devargs; > + } > + return NULL; > +} > + > +static bool > +cdx_ignore_device(const char *dev_name) > +{ > + struct rte_devargs *devargs =3D cdx_devargs_lookup(dev_name); > + > + switch (rte_cdx_bus.bus.conf.scan_mode) { > + case RTE_BUS_SCAN_ALLOWLIST: > + if (devargs && devargs->policy =3D=3D RTE_DEV_ALLOWED) > + return false; > + break; > + case RTE_BUS_SCAN_UNDEFINED: > + case RTE_BUS_SCAN_BLOCKLIST: > + if (devargs =3D=3D NULL || devargs->policy !=3D RTE_DEV_B= LOCKED) > + 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 =3D NULL; > + char driver[PATH_MAX]; > + unsigned long tmp; > + char *name =3D NULL; > + int ret; > + > + dev =3D calloc(1, sizeof(*dev)); > + if (!dev) > + return -ENOMEM; > + > + name =3D calloc(1, RTE_DEV_NAME_MAX_LEN); > + if (!name) { > + ret =3D -ENOMEM; > + goto err; > + } > + > + dev->device.bus =3D &rte_cdx_bus.bus; > + memcpy(name, dev_name, RTE_DEV_NAME_MAX_LEN); > + dev->device.name =3D 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 =3D cdx_get_kernel_driver_by_path(filename, driver, sizeof(dr= iver)); > + if (ret < 0) { > + CDX_BUS_ERR("Fail to get kernel driver"); > + ret =3D -1; > + goto err; > + } > + > + /* > + * Check if device is bound to 'vfio-cdx' driver, so that user-sp= ace > + * can gracefully access the device. > + */ > + if (ret || strcmp(driver, "vfio-cdx")) { > + ret =3D 0; > + goto err; > + } > + > + /* get vendor id */ > + snprintf(filename, sizeof(filename), "%s/vendor", dirname); > + if (eal_parse_sysfs_value(filename, &tmp) < 0) { > + ret =3D -1; > + goto err; > + } > + dev->id.vendor_id =3D (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 =3D (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 =3D opendir(rte_cdx_get_sysfs_path()); > + if (dir =3D=3D NULL) { > + CDX_BUS_ERR("%s(): opendir failed: %s", __func__, > + strerror(errno)); > + return -1; > + } > + > + while ((e =3D readdir(dir)) !=3D NULL) { > + if (e->d_name[0] =3D=3D '.') > + 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 s= ize, > + int additional_flags) > +{ > + void *mapaddr; > + > + /* Map the cdx MMIO memory resource of device */ > + mapaddr =3D rte_mem_map(requested_addr, size, > + RTE_PROT_READ | RTE_PROT_WRITE, > + RTE_MAP_SHARED | additional_flags, fd, offset); > + if (mapaddr =3D=3D 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 =3D=3D 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", __fun= c__, > + 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 =3D cdx_drv->id_table; id_table->vendor_id !=3D 0; > + id_table++) { > + /* check if device's identifiers match the driver's ones = */ > + if (id_table->vendor_id !=3D cdx_dev->id.vendor_id && > + id_table->vendor_id !=3D RTE_CDX_ANY_ID) > + continue; > + if (id_table->device_id !=3D cdx_dev->id.device_id && > + id_table->device_id !=3D 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 =3D dev->device.name; > + bool already_probed; > + int ret; > + > + if ((dr =3D=3D NULL) || (dev =3D=3D NULL)) > + return -EINVAL; This symbol can't be called with dr or dev =3D=3D 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 =3D rte_dev_is_probed(&dev->device); > + if (already_probed) { > + CDX_BUS_INFO("Device %s is already probed", dev->device.n= ame); > + return -EEXIST; > + } > + > + CDX_BUS_DEBUG(" probe device %s using driver: %s", dev_name, > + dr->driver.name); > + > + ret =3D cdx_vfio_map_resource(dev); > + if (ret !=3D 0) { > + CDX_BUS_ERR("CDX map device failed: %d", ret); > + goto error_map_device; > + } > + > + /* call the driver probe() function */ > + ret =3D 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 =3D &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 =3D NULL; > + int rc =3D 0; > + > + if (dev =3D=3D NULL) > + return -EINVAL; This symbol can't be called with dev =3D=3D NULL. > + > + FOREACH_DRIVER_ON_CDXBUS(dr) { > + rc =3D 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 =3D NULL; > + size_t probed =3D 0, failed =3D 0; > + int ret =3D 0; > + > + FOREACH_DEVICE_ON_CDXBUS(dev) { > + probed++; > + > + ret =3D cdx_probe_all_drivers(dev); > + if (ret < 0) { > + CDX_BUS_ERR("Requested device %s cannot be used", > + dev->device.name); > + rte_errno =3D errno; > + failed++; > + ret =3D 0; > + } > + } > + > + return (probed && probed =3D=3D failed) ? -1 : 0; > +} > + > +static int > +cdx_parse(const char *name, void *addr) > +{ > + const char **out =3D addr; > + int ret; > + > + ret =3D strncmp(name, CDX_DEV_PREFIX, strlen(CDX_DEV_PREFIX)); > + > + if (ret =3D=3D 0 && addr) > + *out =3D 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 =3D &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 =3D 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 !=3D NULL) { > + cdx_start =3D RTE_DEV_TO_CDX_DEV_CONST(start); > + cdx_dev =3D TAILQ_NEXT(cdx_start, next); > + } else { > + cdx_dev =3D TAILQ_FIRST(&rte_cdx_bus.device_list); > + } > + while (cdx_dev !=3D NULL) { > + if (cmp(&cdx_dev->device, data) =3D=3D 0) > + return &cdx_dev->device; > + cdx_dev =3D TAILQ_NEXT(cdx_dev, next); > + } > + return NULL; > +} > + > +struct rte_cdx_bus rte_cdx_bus =3D { > + .bus =3D { > + .scan =3D cdx_scan, > + .probe =3D cdx_probe, > + .find_device =3D cdx_find_device, > + .parse =3D cdx_parse, I see neither unplug, nor cleanup op which is strange because this API provides a way to unregister drivers. > + }, > + .device_list =3D TAILQ_HEAD_INITIALIZER(rte_cdx_bus.device_list), > + .driver_list =3D 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 =3D false > + reason =3D 'only supported on Linux' > +endif > + > +driver_sdk_headers =3D files('bus_cdx_driver.h') > +sources =3D 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 */ --=20 David Marchand