From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <dev-bounces@dpdk.org> Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 40A12438CE; Mon, 15 Jan 2024 12:44:03 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B426A402C6; Mon, 15 Jan 2024 12:44:02 +0100 (CET) Received: from szxga04-in.huawei.com (szxga04-in.huawei.com [45.249.212.190]) by mails.dpdk.org (Postfix) with ESMTP id 13C7A402C0 for <dev@dpdk.org>; Mon, 15 Jan 2024 12:43:59 +0100 (CET) Received: from mail.maildlp.com (unknown [172.19.88.214]) by szxga04-in.huawei.com (SkyGuard) with ESMTP id 4TD9H11w0Lz29kKj; Mon, 15 Jan 2024 19:42:21 +0800 (CST) Received: from dggpeml500024.china.huawei.com (unknown [7.185.36.10]) by mail.maildlp.com (Postfix) with ESMTPS id A9C921A016B; Mon, 15 Jan 2024 19:43:57 +0800 (CST) Received: from [10.67.121.161] (10.67.121.161) by dggpeml500024.china.huawei.com (7.185.36.10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35; Mon, 15 Jan 2024 19:43:57 +0800 Subject: Re: [PATCH] bus/uacce: introduce UACCE bus To: "lihuisong (C)" <lihuisong@huawei.com>, <dev@dpdk.org> CC: "tangkunshan@huawei.com >> Tangkunshan" <tangkunshan@huawei.com>, "Wangzhou (B)" <wangzhou1@hisilicon.com>, Thomas Monjalon <thomas@monjalon.net> References: <20231208061836.31693-1-fengchengwen@huawei.com> <17ed03df-4dab-c7ff-97c2-12088de42a31@huawei.com> From: fengchengwen <fengchengwen@huawei.com> Message-ID: <91c020f3-5522-fecc-403f-a07f1302d94e@huawei.com> Date: Mon, 15 Jan 2024 19:43:57 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 MIME-Version: 1.0 In-Reply-To: <17ed03df-4dab-c7ff-97c2-12088de42a31@huawei.com> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.121.161] X-ClientProxiedBy: dggems705-chm.china.huawei.com (10.3.19.182) To dggpeml500024.china.huawei.com (7.185.36.10) X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions <dev.dpdk.org> List-Unsubscribe: <https://mails.dpdk.org/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://mails.dpdk.org/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://mails.dpdk.org/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Hi Huisong, On 2024/1/15 16:14, lihuisong (C) wrote: > Hi chengwen, > > lgtm, > with below to changes, > Acked-by: Huisong Li <lihuisong@huawei.com> > > > 在 2023/12/8 14:18, Chengwen Feng 写道: >> UACCE (Unified/User-space-access-intended Accelerator Framework) was >> upstream to Linux kernel version 5.7, and it targets to provide Shared >> Virtual Addressing (SVA) between accelerators and processes. So >> accelerator can access any data structure of the main cpu. [1] for more >> information. >> >> This commit introduces UACCE bus, so that the accelerator devices could >> seen at DPDK and could be further registered as a compress, crypto, dma >> and ethdev device. >> >> [1] https://docs.kernel.org/misc-devices/uacce.html >> >> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com> >> --- >> MAINTAINERS | 4 + >> drivers/bus/meson.build | 1 + >> drivers/bus/uacce/bus_uacce_driver.h | 254 ++++++++++ >> drivers/bus/uacce/meson.build | 12 + >> drivers/bus/uacce/uacce.c | 702 +++++++++++++++++++++++++++ >> drivers/bus/uacce/version.map | 15 + >> 6 files changed, 988 insertions(+) >> create mode 100644 drivers/bus/uacce/bus_uacce_driver.h >> create mode 100644 drivers/bus/uacce/meson.build >> create mode 100644 drivers/bus/uacce/uacce.c >> create mode 100644 drivers/bus/uacce/version.map >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 0d1c8126e3..89711029d5 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -604,6 +604,10 @@ Platform bus driver >> M: Tomasz Duszynski <tduszynski@marvell.com> >> F: drivers/bus/platform/ >> +UACCE bus driver >> +M: Chengwen Feng <fengchengwen@huawei.com> >> +F: drivers/bus/uacce/ >> + >> VDEV bus driver >> F: drivers/bus/vdev/ >> F: app/test/test_vdev.c >> diff --git a/drivers/bus/meson.build b/drivers/bus/meson.build >> index a78b4283bf..d67db8576b 100644 >> --- a/drivers/bus/meson.build >> +++ b/drivers/bus/meson.build >> @@ -9,6 +9,7 @@ drivers = [ >> 'ifpga', >> 'pci', >> 'platform', >> + 'uacce', >> 'vdev', >> 'vmbus', >> ] >> diff --git a/drivers/bus/uacce/bus_uacce_driver.h b/drivers/bus/uacce/bus_uacce_driver.h >> new file mode 100644 >> index 0000000000..0276154658 >> --- /dev/null >> +++ b/drivers/bus/uacce/bus_uacce_driver.h >> @@ -0,0 +1,254 @@ >> +/* SPDX-License-Identifier: BSD-3-Clause >> + * Copyright(c) 2023 HiSilicon Limited >> + */ >> + >> +#ifndef BUS_UACCE_DRIVER_H >> +#define BUS_UACCE_DRIVER_H >> + >> +/** >> + * @file >> + * >> + * HiSilicon UACCE bus interface. >> + */ >> + >> +#include <inttypes.h> >> +#include <stdlib.h> >> +#include <linux/types.h> >> + >> +#include <rte_compat.h> >> +#include <rte_devargs.h> >> +#include <dev_driver.h> >> + >> +#ifdef __cplusplus >> +extern "C" { >> +#endif /* __cplusplus */ >> + >> +#define RTE_UACCE_DEV_PATH_SIZE 256 >> +#define RTE_UACCE_API_NAME_SIZE 64 >> +#define RTE_UACCE_ALGS_NAME_SIZE 384 >> +#define RTE_UACCE_ATTR_MAX_SIZE 384 >> + >> +/* >> + * Definition for queue file region type. >> + */ >> +enum rte_uacce_qfrt { >> + RTE_UACCE_QFRT_MMIO = 0, /**< Device mmio region. */ >> + RTE_UACCE_QFRT_DUS, /**< Device user share region. */ >> + RTE_UACCE_QFRT_BUTT >> +}; >> + >> +struct rte_uacce_driver; >> + >> +/** >> + * A structure describing a UACCE device. >> + */ >> +struct rte_uacce_device { >> + RTE_TAILQ_ENTRY(rte_uacce_device) next; /**< Next in device list. */ >> + struct rte_device device; /**< Inherit core device. */ >> + struct rte_uacce_driver *driver; /**< Driver used in probing. */ >> + char name[RTE_DEV_NAME_MAX_LEN]; /**< Device name. */ >> + char dev_root[RTE_UACCE_DEV_PATH_SIZE]; /**< Sysfs path with device name. */ >> + char cdev_path[RTE_UACCE_DEV_PATH_SIZE]; /**< Device path in devfs. */ >> + char api[RTE_UACCE_API_NAME_SIZE]; /**< Device context type. */ >> + char algs[RTE_UACCE_ALGS_NAME_SIZE]; /**< Device supported algorithms. */ >> + uint32_t flags; /**< Device flags. */ >> + int numa_node; /**< NUMA node connection, -1 if unknown. */ >> + uint32_t qfrt_sz[RTE_UACCE_QFRT_BUTT]; /**< Queue file region type's size. */ >> +}; >> + >> +/** >> + * @internal >> + * Helper macro for drivers that need to convert to struct rte_uacce_device. >> + */ >> +#define RTE_DEV_TO_UACCE_DEV(ptr) \ >> + container_of(ptr, struct rte_uacce_device, device) >> + >> +#define RTE_DEV_TO_UACCE_DEV_CONST(ptr) \ >> + container_of(ptr, const struct rte_uacce_device, device) >> + >> +/** >> + * A structure describing an ID for a UACCE driver. Each driver provides a >> + * table of these IDs for each device that it supports. >> + */ >> +struct rte_uacce_id { >> + const char *dev_api; /**< Device context type. */ >> + /** Device algorithm. >> + * If this field is NULL, only dev_api is matched. Otherwise, in >> + * addition to match dev_api, dev_alg must be a subset of device's >> + * algs. >> + */ >> + const char *dev_alg; >> +}; >> + >> +/** >> + * Initialization function for the driver called during probing. >> + */ >> +typedef int (rte_uacce_probe_t)(struct rte_uacce_driver *, struct rte_uacce_device *); >> + >> +/** >> + * Uninitialization function for the driver called during hotplugging. >> + */ >> +typedef int (rte_uacce_remove_t)(struct rte_uacce_device *); >> + >> +/** >> + * A structure describing a UACCE driver. >> + */ >> +struct rte_uacce_driver { >> + RTE_TAILQ_ENTRY(rte_uacce_driver) next; /**< Next in list. */ >> + struct rte_driver driver; /**< Inherit core driver. */ >> + struct rte_uacce_bus *bus; /**< UACCE bus reference. */ >> + rte_uacce_probe_t *probe; /**< Device probe function. */ >> + rte_uacce_remove_t *remove; /**< Device remove function. */ >> + const struct rte_uacce_id *id_table; /**< ID table, NULL terminated. */ >> +}; >> + >> +/** >> + * Get available queue number. >> + * >> + * @param dev >> + * A pointer to a rte_uacce_device structure describing the device >> + * to use. >> + * >> + * @note The available queues on the device may changes dynamically, >> + * for examples, other process may alloc or free queues. >> + * >> + * @return >> + * 0 on success. Otherwise negative value is returned. >> + */ >> +__rte_internal >> +int rte_uacce_avail_queues(struct rte_uacce_device *dev); >> + >> +/* >> + * The queue context for a UACCE queue. >> + */ >> +struct rte_uacce_qcontex { >> + int fd; /**< The file descriptor associated to the queue. */ >> + struct rte_uacce_device *dev; /**< The device associated to the queue. */ >> + void *qfrt_base[RTE_UACCE_QFRT_BUTT]; /**< The qfrt mmap's memory base. */ >> +}; >> + >> +/** >> + * Alloc one queue. >> + * >> + * @param dev >> + * A pointer to a rte_uacce_device structure describing the device to use. >> + * @param qctx >> + * Pointer to queue context, which is used to store the queue information >> + * that is successfully applied for. >> + * >> + * @return >> + * 0 on success. Otherwise negative value is returned. >> + */ >> +__rte_internal >> +int rte_uacce_queue_alloc(struct rte_uacce_device *dev, struct rte_uacce_qcontex *qctx); >> + >> +/** >> + * Free one queue. >> + * >> + * @param qctx >> + * Pointer to queue context, which allocated by @see rte_uacce_alloc_queue. >> + * >> + * @note Once the queue is freed, any operations on the queue (including >> + * control-plane and data-plane, and also read & write mmap region) are not >> + * allowed. >> + */ >> +__rte_internal >> +void rte_uacce_queue_free(struct rte_uacce_qcontex *qctx); >> + >> +/** >> + * Start one queue. >> + * >> + * @param qctx >> + * Pointer to queue context, which allocated by @see rte_uacce_alloc_queue. >> + * >> + * @return >> + * 0 on success. Otherwise negative value is returned. >> + */ >> +__rte_internal >> +int rte_uacce_queue_start(struct rte_uacce_qcontex *qctx); >> + >> +/** >> + * Send ioctl command to one queue. >> + * >> + * @param qctx >> + * Pointer to queue context, which allocated by @see rte_uacce_alloc_queue. >> + * @param cmd >> + * ioctl command. >> + * @note The nr must not conflict with the definition in Linux kerel: >> + * include/uapi/misc/uacce/uacce.h. It is recommended that the driver >> + * custom nr start from 64. >> + * @param arg >> + * Command input & output buffer. >> + * >> + * @return >> + * 0 on success. Otherwise negative value is returned. >> + */ >> +__rte_internal >> +int rte_uacce_queue_ioctl(struct rte_uacce_qcontex *qctx, unsigned long cmd, void *arg); >> + >> +/** >> + * Mmap queue file region. >> + * >> + * @param qctx >> + * Pointer to queue context, which allocated by @see rte_uacce_alloc_queue. >> + * @param qfrt >> + * The queue file region type. Must be RTE_UACCE_QFRT_MMIO or >> + * RTE_UACCE_QFRT_DUS. >> + * >> + * @return >> + * Non-NULL on success. Otherwise NULL is returned. >> + */ >> +__rte_internal >> +void *rte_uacce_queue_mmap(struct rte_uacce_qcontex *qctx, enum rte_uacce_qfrt qfrt); >> + >> +/** >> + * Unmap queue file region. >> + * >> + * @param qctx >> + * Pointer to queue context, which allocated by @see rte_uacce_alloc_queue. >> + * @param qfrt >> + * The queue file region type. Must be RTE_UACCE_QFRT_MMIO or >> + * RTE_UACCE_QFRT_DUS. >> + * >> + * @return >> + * Non-NULL on success. Otherwise NULL is returned. >> + */ >> +__rte_internal >> +void rte_uacce_queue_unmap(struct rte_uacce_qcontex *qctx, enum rte_uacce_qfrt qfrt); >> + >> +/** >> + * Register a UACCE driver. >> + * >> + * @param driver >> + * A pointer to a rte_uacce_driver structure describing the driver to be >> + * registered. >> + */ >> +__rte_internal >> +void rte_uacce_register(struct rte_uacce_driver *driver); >> + >> +/** >> + * Unregister a UACCE driver. >> + * >> + * @param driver >> + * A pointer to a rte_uacce_driver structure describing the driver to be >> + * unregistered. >> + */ >> +__rte_internal >> +void rte_uacce_unregister(struct rte_uacce_driver *driver); >> + >> +/** >> + * Helper for UACCE device registration from driver instance. >> + */ >> +#define RTE_PMD_REGISTER_UACCE(nm, uacce_drv) \ >> + RTE_INIT(uacceinitfn_ ##nm) \ >> + {\ >> + (uacce_drv).driver.name = RTE_STR(nm);\ >> + rte_uacce_register(&uacce_drv); \ >> + } \ >> + RTE_PMD_EXPORT_NAME(nm, __COUNTER__) >> + >> +#ifdef __cplusplus >> +} >> +#endif /* __cplusplus */ >> + >> +#endif /* BUS_UACCE_DRIVER_H */ >> diff --git a/drivers/bus/uacce/meson.build b/drivers/bus/uacce/meson.build >> new file mode 100644 >> index 0000000000..b48d6db11a >> --- /dev/null >> +++ b/drivers/bus/uacce/meson.build >> @@ -0,0 +1,12 @@ >> +# SPDX-License-Identifier: BSD-3-Clause >> +# Copyright(c) 2023 HiSilicon Limited. >> + >> +if not is_linux >> + build = false >> + reason = 'only supported on Linux' >> +endif >> + >> +sources = files('uacce.c') >> +driver_sdk_headers += files('bus_uacce_driver.h') >> + >> +deps += ['kvargs'] >> diff --git a/drivers/bus/uacce/uacce.c b/drivers/bus/uacce/uacce.c >> new file mode 100644 >> index 0000000000..8e824c44df >> --- /dev/null >> +++ b/drivers/bus/uacce/uacce.c >> @@ -0,0 +1,702 @@ >> +/* SPDX-License-Identifier: BSD-3-Clause >> + * Copyright(c) 2023 HiSilicon Limited >> + */ >> + >> +#include <dirent.h> >> +#include <errno.h> >> +#include <fcntl.h> >> +#include <unistd.h> >> +#include <stdlib.h> >> +#include <string.h> >> +#include <sys/ioctl.h> >> +#include <sys/mman.h> >> +#include <sys/stat.h> >> +#include <sys/types.h> >> + >> +#include <rte_bitops.h> >> +#include <rte_common.h> >> +#include <rte_devargs.h> >> +#include <rte_errno.h> >> +#include <rte_log.h> >> +#include <rte_kvargs.h> >> +#include <bus_driver.h> >> + >> +#include "bus_uacce_driver.h" >> + >> +#define UACCE_BUS_CLASS_PATH "/sys/class/uacce" >> + >> +/* UACCE device flag of SVA. */ >> +#define UACCE_DEV_FLGA_SVA RTE_BIT32(0) >> + >> +/* Support -a uacce:device-name when start DPDK application. */ >> +#define UACCE_DEV_PREFIX "uacce:" >> + >> +/* >> + * Structure describing the UACCE bus. >> + */ >> +struct rte_uacce_bus { >> + struct rte_bus bus; /* Inherit the generic class. */ >> + TAILQ_HEAD(, rte_uacce_device) device_list; /* List of devices. */ >> + TAILQ_HEAD(, rte_uacce_driver) driver_list; /* List of drivers. */ >> +}; >> + >> +/* Forward declaration of UACCE bus. */ >> +static struct rte_uacce_bus uacce_bus; >> + >> +enum uacce_params { >> + RTE_UACCE_PARAM_NAME, >> +}; >> + >> +static const char *const uacce_params_keys[] = { >> + [RTE_UACCE_PARAM_NAME] = "name", >> + NULL, >> +}; >> + >> +#define FOREACH_DEVICE_ON_UACCEBUS(p) \ >> + RTE_TAILQ_FOREACH(p, &uacce_bus.device_list, next) >> +#define FOREACH_DRIVER_ON_UACCEBUS(p) \ >> + RTE_TAILQ_FOREACH(p, &uacce_bus.driver_list, next) >> + >> +extern int uacce_bus_logtype; >> +#define UACCE_BUS_LOG(level, fmt, args...) \ >> + rte_log(RTE_LOG_ ## level, uacce_bus_logtype, "uacce: " fmt "\n", \ >> + ##args) >> +#define UACCE_BUS_ERR(fmt, args...) UACCE_BUS_LOG(ERR, fmt, ##args) >> +#define UACCE_BUS_WARN(fmt, args...) UACCE_BUS_LOG(WARNING, fmt, ##args) >> +#define UACCE_BUS_INFO(fmt, args...) UACCE_BUS_LOG(INFO, fmt, ##args) >> +#define UACCE_BUS_DEBUG(fmt, args...) UACCE_BUS_LOG(DEBUG, fmt, ##args) >> + >> + >> +static struct rte_devargs * >> +uacce_devargs_lookup(const char *dev_name) >> +{ >> + char name[RTE_UACCE_DEV_PATH_SIZE] = {0}; >> + struct rte_devargs *devargs; >> + >> + snprintf(name, sizeof(name), "%s%s", UACCE_DEV_PREFIX, dev_name); >> + RTE_EAL_DEVARGS_FOREACH("uacce", devargs) { >> + if (strcmp(devargs->name, name) == 0) >> + return devargs; >> + } >> + >> + return NULL; >> +} >> + >> +static bool >> +uacce_ignore_device(const char *dev_name) >> +{ >> + struct rte_devargs *devargs = uacce_devargs_lookup(dev_name); >> + >> + switch (uacce_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; >> +} >> + >> +/* >> + * Returns the number of bytes read (removed last newline) on success. >> + * Otherwise negative value is returned. >> + */ >> +static int >> +uacce_read_attr(const char *dev_root, const char *attr, char *buf, uint32_t sz) >> +{ >> + char filename[PATH_MAX] = {0}; >> + int ret; >> + int fd; >> + int i; >> + >> + snprintf(filename, sizeof(filename), "%s/%s", dev_root, attr); >> + fd = open(filename, O_RDONLY, 0); >> + if (fd < 0) { >> + UACCE_BUS_ERR("failed to open %s", filename); >> + return -EIO; >> + } >> + >> + ret = read(fd, buf, sz); >> + if (ret > 0) { >> + /* Remove the last new line character. */ >> + for (i = ret - 1; i >= 0; i--) { >> + if (buf[i] == '\n') { >> + buf[i] = '\0'; >> + ret--; >> + break; >> + } >> + } >> + } >> + if (ret <= 0) { > close(fd) is also needed here. ack >> + UACCE_BUS_ERR("failed to read %s", filename); >> + return -EIO; >> + } >> + >> + close(fd); >> + >> + return ret; >> +} >> + >> +/* 0 on success. Otherwise negative value is returned. */ >> +static int >> +uacce_read_attr_int(const char *dev_root, const char *attr, int *val) >> +{ >> + char buf[RTE_UACCE_ATTR_MAX_SIZE] = {0}; >> + char *s = NULL; >> + int ret; >> + >> + ret = uacce_read_attr(dev_root, attr, buf, sizeof(buf) - 1); >> + if (ret < 0) >> + return ret; >> + >> + *val = strtol(buf, &s, 0); >> + if (s[0] != '\0') { >> + UACCE_BUS_ERR("read attr %s/%s expect an integer value", dev_root, attr); >> + return -EINVAL; >> + } >> + >> + return 0; >> +} >> + >> +/* 0 on success. Otherwise negative value is returned. */ >> +static int >> +uacce_read_attr_u32(const char *dev_root, const char *attr, uint32_t *val) >> +{ >> + char buf[RTE_UACCE_ATTR_MAX_SIZE] = {0}; >> + char *s = NULL; >> + int ret; >> + >> + ret = uacce_read_attr(dev_root, attr, buf, sizeof(buf) - 1); >> + if (ret < 0) >> + return ret; >> + >> + *val = strtoul(buf, &s, 0); >> + if (s[0] != '\0') { >> + UACCE_BUS_ERR("read attr %s/%s expect an uint32 value", dev_root, attr); >> + return -EINVAL; >> + } >> + >> + return 0; >> +} >> + >> +static int >> +uacce_read_api(struct rte_uacce_device *dev) >> +{ >> + int ret = uacce_read_attr(dev->dev_root, "api", dev->api, sizeof(dev->api) - 1); >> + if (ret < 0) >> + return ret; >> + return 0; > Could you use return uacce_read_attr()? it is more concise. This function should return zero when success. and uacce_read_attr will return readed bytes after read. So it need to convert like above. >> +} >> + >> +static int >> +uacce_read_algs(struct rte_uacce_device *dev) >> +{ >> + int ret = uacce_read_attr(dev->dev_root, "algorithms", dev->algs, sizeof(dev->algs) - 1); >> + if (ret < 0) >> + return ret; >> + return 0; > return uacce_read_attr() same as above reason. >> +} >> + >> +static int >> +uacce_read_flags(struct rte_uacce_device *dev) >> +{ >> + return uacce_read_attr_u32(dev->dev_root, "flags", &dev->flags); >> +} >> + >> +static void >> +uacce_read_numa_node(struct rte_uacce_device *dev) >> +{ >> + int ret = uacce_read_attr_int(dev->dev_root, "device/numa_node", &dev->numa_node); >> + if (ret != 0) { >> + UACCE_BUS_WARN("read attr numa_node failed! set to default"); >> + dev->numa_node = -1; >> + } >> +} >> + >> +static int >> +uacce_read_qfrt_sz(struct rte_uacce_device *dev) >> +{ >> + int ret = uacce_read_attr_u32(dev->dev_root, "region_mmio_size", >> + &dev->qfrt_sz[RTE_UACCE_QFRT_MMIO]); >> + if (ret != 0) >> + return ret; >> + return uacce_read_attr_u32(dev->dev_root, "region_dus_size", >> + &dev->qfrt_sz[RTE_UACCE_QFRT_DUS]); >> +} >> + >> +static int >> +uacce_verify(struct rte_uacce_device *dev) >> +{ >> + if (!(dev->flags & UACCE_DEV_FLGA_SVA)) { >> + UACCE_BUS_WARN("device %s don't support SVA, skip it!", dev->name); >> + return 1; /* >0 will skip this device. */ >> + } >> + >> + return 0; >> +} >> + >> +/* >> + * Scan one UACCE sysfs entry, and fill the devices list from it. >> + * It reads api/algs/flags/numa_node/region-size (please refer Linux kernel: >> + * Documentation/ABI/testing/sysfs-driver-uacce) and stores them for later >> + * device-driver matching, driver init... >> + */ >> +static int >> +uacce_scan_one(const char *dev_name) >> +{ >> + struct rte_uacce_device *dev = NULL; > This assignment for dev is redundant. ack >> + int ret; >> + >> + dev = calloc(1, sizeof(*dev)); >> + if (!dev) >> + return -ENOMEM; >> + >> + dev->device.bus = &uacce_bus.bus; >> + dev->device.name = dev->name; >> + dev->device.devargs = uacce_devargs_lookup(dev_name); >> + snprintf(dev->name, sizeof(dev->name), "%s", dev_name); >> + snprintf(dev->dev_root, sizeof(dev->dev_root), "%s/%s", >> + UACCE_BUS_CLASS_PATH, dev_name); >> + snprintf(dev->cdev_path, sizeof(dev->cdev_path), "/dev/%s", dev_name); >> + >> + ret = uacce_read_api(dev); >> + if (ret != 0) >> + goto err; >> + ret = uacce_read_algs(dev); >> + if (ret != 0) >> + goto err; >> + ret = uacce_read_flags(dev); >> + if (ret != 0) >> + goto err; >> + uacce_read_numa_node(dev); >> + ret = uacce_read_qfrt_sz(dev); >> + if (ret != 0) >> + goto err; >> + >> + ret = uacce_verify(dev); >> + if (ret != 0) >> + goto err; >> + >> + TAILQ_INSERT_TAIL(&uacce_bus.device_list, dev, next); >> + return 0; >> + >> +err: >> + free(dev); >> + return ret; >> +} >> + >> +static int >> +uacce_scan(void) >> +{ >> + struct dirent *e; >> + DIR *dir; >> + >> + dir = opendir(UACCE_BUS_CLASS_PATH); >> + if (dir == NULL) { >> + UACCE_BUS_LOG(INFO, "open %s failed!", UACCE_BUS_CLASS_PATH); >> + return 0; > Why return 0 here? This sitation should not treated as error. If return non-zero, the rte_bus_scan() will output a error trace "Scan for uacce bus failed". But the reason maybe the havn't loaded uacce.ko. So I think it's okay to return zero here. >> + } >> + >> + while ((e = readdir(dir)) != NULL) { >> + if (e->d_name[0] == '.') >> + continue; >> + >> + if (strlen(e->d_name) >= RTE_DEV_NAME_MAX_LEN) { >> + UACCE_BUS_LOG(WARNING, "uacce device name %s too long, skip it!", >> + e->d_name); >> + continue; >> + } >> + >> + if (uacce_ignore_device(e->d_name)) >> + continue; >> + >> + if (uacce_scan_one(e->d_name) < 0) >> + goto error; >> + } >> + closedir(dir); >> + return 0; >> + >> +error: >> + closedir(dir); >> + return -1; >> +} >> + >> +static bool >> +uacce_match(const struct rte_uacce_driver *dr, const struct rte_uacce_device *dev) >> +{ >> + const struct rte_uacce_id *id_table; >> + uint32_t len; >> + char *map; >> + >> + for (id_table = dr->id_table; id_table->dev_api != NULL; id_table++) { >> + if (strcmp(id_table->dev_api, dev->api) != 0) >> + continue; >> + >> + if (id_table->dev_alg == NULL) >> + return 1; > This func() should use "ture" or "false" as return value. ack > Should here return "false"? it means only match dev_api if dev_alg is NULL, so it's okay to return 1(true) here. >> + >> + /* The dev->algs's algrothims is separated by new line, for >> + * example: dev->algs could be: aaa\nbbbb\ncc, which has three >> + * algorithms: aaa, bbbb and cc. >> + * The id_table->dev_alg should be a single algrithm, e.g. bbbb. >> + */ >> + map = strstr(dev->algs, id_table->dev_alg); >> + if (map == NULL) >> + continue; >> + if (map != dev->algs && map[-1] != '\n') >> + continue; >> + len = strlen(id_table->dev_alg); >> + if (map[len] != '\0' && map[len] != '\n') >> + continue; >> + >> + return 1; >> + } >> + >> + return 0; >> +} >> + >> +static int >> +uacce_probe_one_driver(struct rte_uacce_driver *dr, struct rte_uacce_device *dev) >> +{ >> + const char *dev_name = dev->name; >> + bool already_probed; >> + int ret; >> + >> + if (!uacce_match(dr, dev)) >> + /* Match of device and driver failed */ >> + return 1; >> + >> + already_probed = rte_dev_is_probed(&dev->device); >> + if (already_probed) { >> + UACCE_BUS_INFO("device %s is already probed", dev_name); >> + return -EEXIST; >> + } >> + >> + UACCE_BUS_DEBUG("probe device %s using driver %s", dev_name, dr->driver.name); >> + >> + ret = dr->probe(dr, dev); >> + if (ret != 0) { >> + UACCE_BUS_ERR("probe device %s with driver %s failed %d", >> + dev_name, dr->driver.name, ret); >> + } else { >> + dev->device.driver = &dr->driver; >> + dev->driver = dr; >> + UACCE_BUS_DEBUG("probe device %s with driver %s success", >> + dev_name, dr->driver.name); >> + } >> + >> + return ret; >> +} >> + >> +static int >> +uacce_probe_all_drivers(struct rte_uacce_device *dev) >> +{ >> + struct rte_uacce_driver *dr = NULL; >> + int rc = 0; > reduntant assignment. ack >> + >> + FOREACH_DRIVER_ON_UACCEBUS(dr) { >> + rc = uacce_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; >> +} >> + >> +static int >> +uacce_probe(void) >> +{ >> + struct rte_uacce_device *dev = NULL; >> + size_t probed = 0, failed = 0; >> + int ret = 0; > This ret is reduntant assignment. ack Will send v2 to fix the place which acks. Thanks.