RE: [PATCH v7 1/5] raw/ifpga: introduce AFU driver framework

["Huang, Wei" <wei.huang@intel.com>]

> -----Original Message-----
> From: Xu, Rosen <rosen.xu@intel.com>
> Sent: Wednesday, June 15, 2022 14:55
> To: Huang, Wei <wei.huang@intel.com>; dev@dpdk.org;
> thomas@monjalon.net; nipun.gupta@nxp.com; hemant.agrawal@nxp.com
> Cc: stable@dpdk.org; Zhang, Tianfei <tianfei.zhang@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>
> Subject: RE: [PATCH v7 1/5] raw/ifpga: introduce AFU driver framework
> 
> Hi Wei,
> 
> Some comments.
> 
> Thanks,
> Rosen
> 
> > -----Original Message-----
> > From: Huang, Wei <wei.huang@intel.com>
> > Sent: Thursday, June 09, 2022 15:37
> > To: dev@dpdk.org; thomas@monjalon.net; nipun.gupta@nxp.com;
> > hemant.agrawal@nxp.com
> > Cc: stable@dpdk.org; Xu, Rosen <rosen.xu@intel.com>; Zhang, Tianfei
> > <tianfei.zhang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Huang,
> > Wei <wei.huang@intel.com>
> > Subject: [PATCH v7 1/5] raw/ifpga: introduce AFU driver framework
> >
> > AFU (Acceleration Function Unit) is part of FPGA and enumerated by
> > ifpga driver.
> > This driver implements common AFU device interfaces and exposes them
> > to application as standard raw device APIs.
> > Normally application can operate specified AFU as below, 1. call
> > rte_rawdev_pmd_get_named_dev() to find AFU device.
> > 2. call rte_rawdev_configure() to initialize AFU device.
> > 3. call rte_rawdev_selftest() to test AFU device.
> >
> > Signed-off-by: Wei Huang <wei.huang@intel.com>
> > ---
> > v2: fix typo
> > ---
> > v3: fix build error in FreeBSD13-64, UB2004-32 and UB2204-32
> > ---
> > v4: fix coding style issue and build error in FreeBSD13-64
> > ---
> > v5: split patch into several patches
> > ---
> > v6: move source files to ifpga and rename, use spinlock
> > ---
> >  drivers/raw/ifpga/afu_pmd_core.c | 453
> > +++++++++++++++++++++++++++++++++++++++
> >  drivers/raw/ifpga/afu_pmd_core.h |  76 +++++++
> >  drivers/raw/ifpga/meson.build    |   2 +-
> >  3 files changed, 530 insertions(+), 1 deletion(-)  create mode 100644
> > drivers/raw/ifpga/afu_pmd_core.c  create mode 100644
> > drivers/raw/ifpga/afu_pmd_core.h
> >
> > diff --git a/drivers/raw/ifpga/afu_pmd_core.c
> > b/drivers/raw/ifpga/afu_pmd_core.c
> > new file mode 100644
> > index 0000000..a24b517
> > --- /dev/null
> > +++ b/drivers/raw/ifpga/afu_pmd_core.c
> > @@ -0,0 +1,453 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright 2022 Intel Corporation
> > + *
> > + * AFU is Acceleration Function Unit in FPGA, it is enumerated by ifpga
> pmd.
> > + * Suppose AFU is found in FPGA at PCI address 31:00.0, then you can
> > +create
> > + * and test a AFU device by following steps in application.
> > + * 1. rte_vdev_init("ifpga_rawdev_cfg0", "ifpga=31:00.0,port=0")
> > + * 2. rawdev = rte_rawdev_pmd_get_named_dev("afu_0|31:00.0")
> > + * 3. rte_rawdev_configure(rawdev->dev_id, &cfg, sizeof(cfg))
> > + * 4. rte_rawdev_selftest(rawdev->dev_id)
> > + * 5. rte_vdev_uninit("ifpga_rawdev_cfg0")
> > + *
> > + * AFU device name format used in rte_rawdev_pmd_get_named_dev is
> > + * "afu_[port]|[BDF]". Please refer to OPAE documentation for the
> > +meaning of
> > + * port. Each AFU device has specific configuration data, they are
> > +defined
> > + * in rte_pmd_afu.h.
> > + *
> > + */
> > +
> Could you split these notes? General speaking we just announce license in
> head of source file.
> 
Agree
> > +#include <errno.h>
> > +#include <stdio.h>
> > +#include <stdint.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <unistd.h>
> > +#include <fcntl.h>
> > +#include <poll.h>
> > +#include <sys/eventfd.h>
> > +
> > +#include <rte_eal.h>
> > +#include <rte_malloc.h>
> > +#include <rte_memzone.h>
> > +#include <rte_rawdev_pmd.h>
> > +
> > +#include "afu_pmd_core.h"
> > +
> > +static struct rte_afu_uuid
> > afu_pmd_uuid_map[AFU_RAWDEV_MAX_DRVS+1];
> > +TAILQ_HEAD(afu_drv_list, afu_rawdev_drv); static struct afu_drv_list
> > +afu_pmd_list = TAILQ_HEAD_INITIALIZER(afu_pmd_list);
> > +
> > +static inline int afu_rawdev_trylock(struct afu_rawdev *dev) {
> > +	if (!dev || !dev->sd)
> > +		return 0;
> > +
> > +	return rte_spinlock_trylock(&dev->sd->lock);
> > +}
> > +
> > +static inline void afu_rawdev_unlock(struct afu_rawdev *dev) {
> > +	if (!dev || !dev->sd)
> > +		return;
> > +
> > +	rte_spinlock_unlock(&dev->sd->lock);
> > +}
> > +
> > +static int afu_rawdev_configure(const struct rte_rawdev *rawdev,
> > +	rte_rawdev_obj_t config, size_t config_size) {
> > +	struct afu_rawdev *dev = NULL;
> > +	int ret = 0;
> > +
> > +	IFPGA_RAWDEV_PMD_FUNC_TRACE();
> > +
> > +	dev = afu_rawdev_get_priv(rawdev);
> > +	if (!dev)
> > +		return -ENODEV;
> > +
> > +	if (dev->ops && dev->ops->config)
> > +		ret = (*dev->ops->config)(dev, config, config_size);
> > +
> > +	return ret;
> > +}
> > +
> > +static int afu_rawdev_start(struct rte_rawdev *rawdev) {
> > +	struct afu_rawdev *dev = NULL;
> > +	int ret = 0;
> > +
> > +	IFPGA_RAWDEV_PMD_FUNC_TRACE();
> > +
> > +	dev = afu_rawdev_get_priv(rawdev);
> > +	if (!dev)
> > +		return -ENODEV;
> > +
> > +	ret = afu_rawdev_trylock(dev);
> > +	if (!ret) {
> > +		IFPGA_RAWDEV_PMD_WARN("AFU is busy, please start it
> > later");
> > +		return ret;
> > +	}
> > +
> > +	if (dev->ops && dev->ops->start)
> > +		ret = (*dev->ops->start)(dev);
> > +
> > +	afu_rawdev_unlock(dev);
> > +
> > +	return ret;
> > +}
> > +
> > +static void afu_rawdev_stop(struct rte_rawdev *rawdev) {
> > +	struct afu_rawdev *dev = NULL;
> > +	int ret = 0;
> > +
> > +	IFPGA_RAWDEV_PMD_FUNC_TRACE();
> > +
> > +	dev = afu_rawdev_get_priv(rawdev);
> > +	if (!dev)
> > +		return;
> > +
> > +	ret = afu_rawdev_trylock(dev);
> > +	if (!ret) {
> > +		IFPGA_RAWDEV_PMD_WARN("AFU is busy, please stop it
> > later");
> > +		return;
> > +	}
> > +
> > +	if (dev->ops && dev->ops->stop)
> > +		ret = (*dev->ops->stop)(dev);
> > +
> > +	afu_rawdev_unlock(dev);
> > +}
> > +
> > +static int afu_rawdev_close(struct rte_rawdev *rawdev) {
> > +	struct afu_rawdev *dev = NULL;
> > +	int ret = 0;
> > +
> > +	IFPGA_RAWDEV_PMD_FUNC_TRACE();
> > +
> > +	dev = afu_rawdev_get_priv(rawdev);
> > +	if (!dev)
> > +		return -ENODEV;
> > +
> > +	if (dev->ops && dev->ops->close)
> > +		ret = (*dev->ops->close)(dev);
> > +
> > +	return ret;
> > +}
> > +
> > +static int afu_rawdev_reset(struct rte_rawdev *rawdev) {
> > +	struct afu_rawdev *dev = NULL;
> > +	int ret = 0;
> > +
> > +	IFPGA_RAWDEV_PMD_FUNC_TRACE();
> > +
> > +	dev = afu_rawdev_get_priv(rawdev);
> > +	if (!dev)
> > +		return -ENODEV;
> > +
> > +	ret = afu_rawdev_trylock(dev);
> > +	if (!ret) {
> > +		IFPGA_RAWDEV_PMD_WARN("AFU is busy, please reset it
> > later");
> > +		return ret;
> > +	}
> > +
> > +	if (dev->ops && dev->ops->reset)
> > +		ret = (*dev->ops->reset)(dev);
> > +
> > +	afu_rawdev_unlock(dev);
> > +
> > +	return ret;
> > +}
> > +
> > +static int afu_rawdev_selftest(uint16_t dev_id) {
> > +	struct afu_rawdev *dev = NULL;
> > +	int ret = 0;
> > +
> > +	IFPGA_RAWDEV_PMD_FUNC_TRACE();
> > +
> > +	if (!rte_rawdev_pmd_is_valid_dev(dev_id))
> > +		return -ENODEV;
> > +
> > +	dev = afu_rawdev_get_priv(&rte_rawdevs[dev_id]);
> > +	if (!dev)
> > +		return -ENOENT;
> > +
> > +	ret = afu_rawdev_trylock(dev);
> > +	if (!ret) {
> > +		IFPGA_RAWDEV_PMD_WARN("AFU is busy, please test it
> > later");
> > +		return ret;
> > +	}
> > +
> > +	if (dev->ops && dev->ops->test)
> > +		ret = (*dev->ops->test)(dev);
> > +
> > +	afu_rawdev_unlock(dev);
> > +
> > +	return ret;
> > +}
> > +
> > +static int afu_rawdev_dump(struct rte_rawdev *rawdev, FILE *f) {
> > +	struct afu_rawdev *dev = NULL;
> > +	int ret = 0;
> > +
> > +	IFPGA_RAWDEV_PMD_FUNC_TRACE();
> > +
> > +	dev = afu_rawdev_get_priv(rawdev);
> > +	if (!dev)
> > +		return -ENODEV;
> > +
> > +	if (dev->ops && dev->ops->dump)
> > +		ret = (*dev->ops->dump)(dev, f);
> > +
> > +	return ret;
> > +}
> > +
> > +static const struct rte_rawdev_ops afu_rawdev_ops = {
> > +	.dev_info_get = NULL,
> > +	.dev_configure = afu_rawdev_configure,
> > +	.dev_start = afu_rawdev_start,
> > +	.dev_stop = afu_rawdev_stop,
> > +	.dev_close = afu_rawdev_close,
> > +	.dev_reset = afu_rawdev_reset,
> > +
> > +	.queue_def_conf = NULL,
> > +	.queue_setup = NULL,
> > +	.queue_release = NULL,
> > +	.queue_count = NULL,
> > +
> > +	.attr_get = NULL,
> > +	.attr_set = NULL,
> > +
> > +	.enqueue_bufs = NULL,
> > +	.dequeue_bufs = NULL,
> > +
> > +	.dump = afu_rawdev_dump,
> > +
> > +	.xstats_get = NULL,
> > +	.xstats_get_names = NULL,
> > +	.xstats_get_by_name = NULL,
> > +	.xstats_reset = NULL,
> > +
> > +	.firmware_status_get = NULL,
> > +	.firmware_version_get = NULL,
> > +	.firmware_load = NULL,
> > +	.firmware_unload = NULL,
> > +
> > +	.dev_selftest = afu_rawdev_selftest, };
> > +
> > +static int afu_shared_data_alloc(const char *name,
> > +	struct afu_shared_data **data, int socket_id) {
> > +	const struct rte_memzone *mz;
> > +	char mz_name[RTE_MEMZONE_NAMESIZE];
> > +	struct afu_shared_data *sd = NULL;
> > +	int init_mz = 0;
> > +
> > +	if (!name || !data)
> > +		return -EINVAL;
> > +
> > +	/* name format is afu_?|??:??.? which is unique */
> > +	snprintf(mz_name, sizeof(mz_name), "%s", name);
> > +
> > +	mz = rte_memzone_lookup(mz_name);
> > +	if (!mz) {
> > +		mz = rte_memzone_reserve(mz_name, sizeof(struct
> > afu_shared_data),
> > +				socket_id, 0);
> > +		init_mz = 1;
> > +	}
> > +
> > +	if (!mz) {
> > +		IFPGA_RAWDEV_PMD_ERR("Allocate memory zone %s
> > failed!",
> > +			mz_name);
> > +		return -ENOMEM;
> > +	}
> > +
> > +	sd = (struct afu_shared_data *)mz->addr;
> > +
> > +	if (init_mz)  /* initialize memory zone on the first time */
> > +		rte_spinlock_init(&sd->lock);
> > +
> > +	*data = sd;
> > +
> > +	return 0;
> > +}
> > +
> > +static int afu_rawdev_name_get(struct rte_afu_device *afu_dev, char
> > *name,
> > +	size_t size)
> > +{
> > +	int n = 0;
> > +
> > +	if (!afu_dev || !name || !size)
> > +		return -EINVAL;
> > +
> > +	n = snprintf(name, size, "afu_%s", afu_dev->device.name);
> > +	if (n >= (int)size) {
> > +		IFPGA_RAWDEV_PMD_ERR("Name of AFU device is too
> > long!");
> > +		return -ENAMETOOLONG;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static struct afu_ops *afu_ops_get(struct rte_afu_uuid *afu_id) {
> > +	struct afu_rawdev_drv *drv = NULL;
> > +
> > +	if (!afu_id)
> > +		return NULL;
> > +
> > +	TAILQ_FOREACH(drv, &afu_pmd_list, next) {
> > +		if ((drv->uuid.uuid_low == afu_id->uuid_low) &&
> > +			(drv->uuid.uuid_high == afu_id->uuid_high))
> > +			break;
> > +	}
> > +
> > +	return drv ? drv->ops : NULL;
> > +}
> > +
> > +static int afu_rawdev_create(struct rte_afu_device *afu_dev, int
> > +socket_id) {
> > +	struct rte_rawdev *rawdev = NULL;
> > +	struct afu_rawdev *dev = NULL;
> > +	char name[RTE_RAWDEV_NAME_MAX_LEN] = {0};
> > +	int ret = 0;
> > +
> > +	if (!afu_dev)
> > +		return -EINVAL;
> > +
> > +	ret = afu_rawdev_name_get(afu_dev, name, sizeof(name));
> > +	if (ret)
> > +		return ret;
> > +
> > +	IFPGA_RAWDEV_PMD_INFO("Create raw device %s on NUMA
> > node %d",
> > +		name, socket_id);
> > +
> > +	/* Allocate device structure */
> > +	rawdev = rte_rawdev_pmd_allocate(name, sizeof(struct
> > afu_rawdev),
> > +				socket_id);
> > +	if (!rawdev) {
> > +		IFPGA_RAWDEV_PMD_ERR("Unable to allocate raw device");
> > +		return -ENOMEM;
> > +	}
> > +
> > +	rawdev->dev_ops = &afu_rawdev_ops;
> > +	rawdev->device = &afu_dev->device;
> > +	rawdev->driver_name = afu_dev->driver->driver.name;
> > +
> > +	dev = afu_rawdev_get_priv(rawdev);
> > +	if (!dev)
> > +		goto cleanup;
> > +
> > +	dev->rawdev = rawdev;
> > +	dev->port = afu_dev->id.port;
> > +	dev->addr = afu_dev->mem_resource[0].addr;
> > +	dev->ops = afu_ops_get(&afu_dev->id.uuid);
> > +	if (dev->ops == NULL) {
> > +		IFPGA_RAWDEV_PMD_ERR("Unsupported AFU device");
> > +		goto cleanup;
> > +	}
> > +
> > +	if (dev->ops->init) {
> > +		ret = (*dev->ops->init)(dev);
> > +		if (ret) {
> > +			IFPGA_RAWDEV_PMD_ERR("Failed to init %s",
> > name);
> > +			goto cleanup;
> > +		}
> > +	}
> > +
> > +	ret = afu_shared_data_alloc(name, &dev->sd, socket_id);
> > +	if (ret)
> > +		goto cleanup;
> > +
> > +	return ret;
> > +
> > +cleanup:
> > +	rte_rawdev_pmd_release(rawdev);
> > +	return ret;
> > +}
> > +
> > +static int afu_rawdev_destroy(struct rte_afu_device *afu_dev) {
> > +	struct rte_rawdev *rawdev = NULL;
> > +	char name[RTE_RAWDEV_NAME_MAX_LEN] = {0};
> > +	int ret = 0;
> > +
> > +	if (!afu_dev)
> > +		return -EINVAL;
> > +
> > +	ret = afu_rawdev_name_get(afu_dev, name, sizeof(name));
> > +	if (ret)
> > +		return ret;
> > +
> > +	IFPGA_RAWDEV_PMD_INFO("Destroy raw device %s", name);
> > +
> > +	rawdev = rte_rawdev_pmd_get_named_dev(name);
> > +	if (!rawdev) {
> > +		IFPGA_RAWDEV_PMD_ERR("Raw device %s not found",
> > name);
> > +		return -EINVAL;
> > +	}
> > +
> > +	/* rte_rawdev_close is called by pmd_release */
> > +	ret = rte_rawdev_pmd_release(rawdev);
> > +	if (ret)
> > +		IFPGA_RAWDEV_PMD_DEBUG("Device cleanup failed");
> > +
> > +	return 0;
> > +}
> > +
> > +static int afu_rawdev_probe(struct rte_afu_device *afu_dev) {
> > +	IFPGA_RAWDEV_PMD_FUNC_TRACE();
> > +	return afu_rawdev_create(afu_dev, rte_socket_id()); }
> > +
> Function format issue.
> 
> > +static int afu_rawdev_remove(struct rte_afu_device *afu_dev) {
> > +	IFPGA_RAWDEV_PMD_FUNC_TRACE();
> > +	return afu_rawdev_destroy(afu_dev);
> > +}
> > +
> > +static struct rte_afu_driver afu_pmd = {
> > +	.id_table = afu_pmd_uuid_map,
> > +	.probe = afu_rawdev_probe,
> > +	.remove = afu_rawdev_remove
> > +};
> > +
> > +RTE_PMD_REGISTER_AFU(afu_rawdev_driver, afu_pmd);
> > +
> > +static void update_uuid_map(void)
> > +{
> > +	int i = 0;
> > +	struct rte_afu_uuid *afu_id = afu_pmd_uuid_map;
> > +	struct afu_rawdev_drv *drv;
> > +
> > +	TAILQ_FOREACH(drv, &afu_pmd_list, next) {
> > +		if (i++ < AFU_RAWDEV_MAX_DRVS) {
> > +			afu_id->uuid_low = drv->uuid.uuid_low;
> > +			afu_id->uuid_high = drv->uuid.uuid_high;
> > +			afu_id++;
> > +		}
> > +	}
> > +	if (i <= AFU_RAWDEV_MAX_DRVS) {
> > +		afu_id->uuid_low = 0;
> > +		afu_id->uuid_high = 0;
> > +	}
> > +}
> > +
> > +void afu_pmd_register(struct afu_rawdev_drv *driver) {
> > +	TAILQ_INSERT_TAIL(&afu_pmd_list, driver, next);
> > +	update_uuid_map();
> > +}
> > +
> > +void afu_pmd_unregister(struct afu_rawdev_drv *driver) {
> > +	TAILQ_REMOVE(&afu_pmd_list, driver, next);
> > +	update_uuid_map();
> > +}
> > diff --git a/drivers/raw/ifpga/afu_pmd_core.h
> > b/drivers/raw/ifpga/afu_pmd_core.h
> > new file mode 100644
> > index 0000000..4fad2c7
> > --- /dev/null
> > +++ b/drivers/raw/ifpga/afu_pmd_core.h
> > @@ -0,0 +1,76 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright 2022 Intel Corporation
> > + */
> > +
> > +#ifndef __AFU_PMD_CORE_H__
> > +#define __AFU_PMD_CORE_H__
> > +
> > +#ifdef __cplusplus
> > +extern "C" {
> > +#endif
> > +
> > +#include <stdint.h>
> > +#include <stdio.h>
> > +#include <unistd.h>
> > +
> > +#include <rte_spinlock.h>
> > +#include <rte_bus_ifpga.h>
> > +#include <rte_rawdev.h>
> > +
> > +#include "ifpga_rawdev.h"
> > +
> > +#define AFU_RAWDEV_MAX_DRVS  32
> > +
> > +struct afu_rawdev;
> > +
> > +struct afu_ops {
> > +	int (*init)(struct afu_rawdev *dev);
> > +	int (*config)(struct afu_rawdev *dev, void *config,
> > +		size_t config_size);
> > +	int (*start)(struct afu_rawdev *dev);
> > +	int (*stop)(struct afu_rawdev *dev);
> > +	int (*test)(struct afu_rawdev *dev);
> > +	int (*close)(struct afu_rawdev *dev);
> > +	int (*reset)(struct afu_rawdev *dev);
> > +	int (*dump)(struct afu_rawdev *dev, FILE *f); };
> > +
> > +struct afu_shared_data {
> > +	rte_spinlock_t lock;  /* lock for multi-process access */ };
> > +
> > +struct afu_rawdev_drv {
> > +	TAILQ_ENTRY(afu_rawdev_drv) next;
> > +	struct rte_afu_uuid uuid;
> > +	struct afu_ops *ops;
> > +};
> > +
> > +struct afu_rawdev {
> > +	struct rte_rawdev *rawdev;  /* point to parent raw device */
> > +	struct afu_shared_data *sd;  /* shared data for multi-process */
> > +	struct afu_ops *ops;  /* device operation functions */
> > +	int port;  /* index of port the AFU attached */
> > +	void *addr;  /* base address of AFU registers */
> > +	void *priv;  /* private driver data */ };
> > +
> > +static inline struct afu_rawdev *
> > +afu_rawdev_get_priv(const struct rte_rawdev *rawdev) {
> > +	return rawdev ? (struct afu_rawdev *)rawdev->dev_private : NULL; }
> > +
> > +void afu_pmd_register(struct afu_rawdev_drv *driver); void
> > +afu_pmd_unregister(struct afu_rawdev_drv *driver);
> > +
> > +#define AFU_PMD_REGISTER(drv)\
> > +RTE_INIT(afupmdinitfunc_ ##drv)\
> > +{\
> > +	afu_pmd_register(&drv);\
> > +}
> > +
> > +#ifdef __cplusplus
> > +}
> > +#endif
> > +
> > +#endif /* __AFU_PMD_CORE_H__ */
> > diff --git a/drivers/raw/ifpga/meson.build
> > b/drivers/raw/ifpga/meson.build index aeccc17..d9a6f29 100644
> > --- a/drivers/raw/ifpga/meson.build
> > +++ b/drivers/raw/ifpga/meson.build
> > @@ -13,7 +13,7 @@ objs = [base_objs]
> >  deps += ['ethdev', 'rawdev', 'pci', 'bus_pci', 'kvargs',
> >      'bus_vdev', 'bus_ifpga', 'net', 'net_i40e', 'net_ipn3ke']
> >
> > -sources = files('ifpga_rawdev.c', 'rte_pmd_ifpga.c')
> > +sources = files('ifpga_rawdev.c', 'rte_pmd_ifpga.c',
> > +'afu_pmd_core.c')
> >
> >  includes += include_directories('base')  includes +=
> > include_directories('../../net/ipn3ke')
> > --
> > 1.8.3.1
> 
> Reviewed-by: Rosen Xu <rosen.xu@intel.com>