[dpdk-dev] [PATCH v9 01/10] drivers/baseband: add PMD for ACC100

Chautru, Nicolas nicolas.chautru at intel.com
Thu Oct 1 01:30:47 CEST 2020


Hi Tom, 

> From: Tom Rix <trix at redhat.com>
> On 9/29/20 4:17 PM, Chautru, Nicolas wrote:
> > Hi Tom,
> >
> >> -----Original Message-----
> >> From: Tom Rix <trix at redhat.com>
> >> Sent: Tuesday, September 29, 2020 12:54 PM
> >> To: Chautru, Nicolas <nicolas.chautru at intel.com>; dev at dpdk.org;
> >> akhil.goyal at nxp.com
> >> Cc: Richardson, Bruce <bruce.richardson at intel.com>; Xu, Rosen
> >> <rosen.xu at intel.com>; dave.burley at accelercomm.com;
> >> aidan.goddard at accelercomm.com; Yigit, Ferruh
> >> <ferruh.yigit at intel.com>; Liu, Tianjiao <tianjiao.liu at intel.com>
> >> Subject: Re: [dpdk-dev] [PATCH v9 01/10] drivers/baseband: add PMD
> >> for
> >> ACC100
> >>
> >>
> >> On 9/28/20 5:29 PM, Nicolas Chautru wrote:
> >>> Add stubs for the ACC100 PMD
> >>>
> >>> Signed-off-by: Nicolas Chautru <nicolas.chautru at intel.com>
> >>> Acked-by: Liu Tianjiao <Tianjiao.liu at intel.com>
> >>> ---
> >>>  doc/guides/bbdevs/acc100.rst                       | 233
> +++++++++++++++++++++
> >>>  doc/guides/bbdevs/features/acc100.ini              |  14 ++
> >>>  doc/guides/bbdevs/index.rst                        |   1 +
> >>>  drivers/baseband/acc100/meson.build                |   6 +
> >>>  drivers/baseband/acc100/rte_acc100_pmd.c           | 175
> >> ++++++++++++++++
> >>>  drivers/baseband/acc100/rte_acc100_pmd.h           |  37 ++++
> >>>  .../acc100/rte_pmd_bbdev_acc100_version.map        |   3 +
> >>>  drivers/baseband/meson.build                       |   2 +-
> >>>  8 files changed, 470 insertions(+), 1 deletion(-)  create mode
> >>> 100644 doc/guides/bbdevs/acc100.rst  create mode 100644
> >>> doc/guides/bbdevs/features/acc100.ini
> >>>  create mode 100644 drivers/baseband/acc100/meson.build
> >>>  create mode 100644 drivers/baseband/acc100/rte_acc100_pmd.c
> >>>  create mode 100644 drivers/baseband/acc100/rte_acc100_pmd.h
> >>>  create mode 100644
> >>> drivers/baseband/acc100/rte_pmd_bbdev_acc100_version.map
> >>>
> >>> diff --git a/doc/guides/bbdevs/acc100.rst
> >>> b/doc/guides/bbdevs/acc100.rst new file mode 100644 index
> >>> 0000000..f87ee09
> >>> --- /dev/null
> >>> +++ b/doc/guides/bbdevs/acc100.rst
> >>> @@ -0,0 +1,233 @@
> >>> +..  SPDX-License-Identifier: BSD-3-Clause
> >>> +    Copyright(c) 2020 Intel Corporation
> >>> +
> >>> +Intel(R) ACC100 5G/4G FEC Poll Mode Driver
> >>> +==========================================
> >>> +
> >>> +The BBDEV ACC100 5G/4G FEC poll mode driver (PMD) supports an
> >>> +implementation of a VRAN FEC wireless acceleration function.
> >>> +This device is also known as Mount Bryce.
> >> If this is code name or general chip name it should be removed.
> > We have used general chip name for other PMDs (ie. Vista Creek), I can
> > remove but why should this be removed for my benefit? This tends to be
> > the most user friendly name so arguablygood to name drop in
> documentation .
> 
> VistaCreek is the code name, the chip would be aria10.
> 
> Since mt bryce is the chip name, after more than 1 eASIC this becomes
> confusing.
> 

Actually MtBryce is the 5G personality on top of a given eASIC DM5, eASIC can be seen as process/fab technology.
Other eASIC chips != MtBryce. 
ACC100 == MtBryce literally, only usage is 4G/5G FEC as exposed by bbdev
Vista Creek is user friendly name for N3000 (Card + Arria10), these names tend to stick long after early deployments. 
I think it helps to include it in that doc as a one liner, even if through the rest of the doc and code the device is referred to as ACC100. 

> Generally public product names should be used because only the early
> developers will know the development code names.
> 
> >
> >
> >>> +
> >>> +Features
> >>> +--------
> >>> +
> >>> +ACC100 5G/4G FEC PMD supports the following features:
> >>> +
> >>> +- LDPC Encode in the DL (5GNR)
> >>> +- LDPC Decode in the UL (5GNR)
> >>> +- Turbo Encode in the DL (4G)
> >>> +- Turbo Decode in the UL (4G)
> >>> +- 16 VFs per PF (physical device)
> >>> +- Maximum of 128 queues per VF
> >>> +- PCIe Gen-3 x16 Interface
> >>> +- MSI
> >>> +- SR-IOV
> >>> +
> >>> +ACC100 5G/4G FEC PMD supports the following BBDEV capabilities:
> >>> +
> >>> +* For the LDPC encode operation:
> >>> +   - ``RTE_BBDEV_LDPC_CRC_24B_ATTACH`` :  set to attach CRC24B to
> >> CB(s)
> >>> +   - ``RTE_BBDEV_LDPC_RATE_MATCH`` :  if set then do not do Rate
> >>> + Match
> >> bypass
> >>> +   - ``RTE_BBDEV_LDPC_INTERLEAVER_BYPASS`` : if set then bypass
> >>> +interleaver
> >>> +
> >>> +* For the LDPC decode operation:
> >>> +   - ``RTE_BBDEV_LDPC_CRC_TYPE_24B_CHECK`` :  check CRC24B from
> >> CB(s)
> >>> +   - ``RTE_BBDEV_LDPC_ITERATION_STOP_ENABLE`` :  disable early
> >> termination
> >>> +   - ``RTE_BBDEV_LDPC_CRC_TYPE_24B_DROP`` :  drops CRC24B bits
> >> appended while decoding
> >>> +   - ``RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE`` :  provides an
> input
> >> for HARQ combining
> >>> +   - ``RTE_BBDEV_LDPC_HQ_COMBINE_OUT_ENABLE`` :  provides an
> input
> >> for HARQ combining
> >>> +   - ``RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_IN_ENABLE`` :
> HARQ
> >> memory input is internal
> >>> +   - ``RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_OUT_ENABLE`` :
> >> HARQ memory output is internal
> >>> +   - ``RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_LOOPBACK`` :
> >> loopback data to/from HARQ memory
> >>> +   - ``RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_FILLERS`` :  HARQ
> >> memory includes the fillers bits
> >>> +   - ``RTE_BBDEV_LDPC_DEC_SCATTER_GATHER`` :  supports scatter-
> >> gather for input/output data
> >>> +   - ``RTE_BBDEV_LDPC_HARQ_6BIT_COMPRESSION`` :  supports
> >> compression of the HARQ input/output
> >>> +   - ``RTE_BBDEV_LDPC_LLR_COMPRESSION`` :  supports LLR input
> >>> +compression
> >>> +
> >>> +* For the turbo encode operation:
> >>> +   - ``RTE_BBDEV_TURBO_CRC_24B_ATTACH`` :  set to attach CRC24B to
> >> CB(s)
> >>> +   - ``RTE_BBDEV_TURBO_RATE_MATCH`` :  if set then do not do Rate
> >> Match bypass
> >>> +   - ``RTE_BBDEV_TURBO_ENC_INTERRUPTS`` :  set for encoder dequeue
> >> interrupts
> >>> +   - ``RTE_BBDEV_TURBO_RV_INDEX_BYPASS`` :  set to bypass RV index
> >>> +   - ``RTE_BBDEV_TURBO_ENC_SCATTER_GATHER`` :  supports
> >>> +scatter-gather for input/output data
> >>> +
> >>> +* For the turbo decode operation:
> >>> +   - ``RTE_BBDEV_TURBO_CRC_TYPE_24B`` :  check CRC24B from CB(s)
> >>> +   - ``RTE_BBDEV_TURBO_SUBBLOCK_DEINTERLEAVE`` :  perform
> subblock
> >> de-interleave
> >>> +   - ``RTE_BBDEV_TURBO_DEC_INTERRUPTS`` :  set for decoder dequeue
> >> interrupts
> >>> +   - ``RTE_BBDEV_TURBO_NEG_LLR_1_BIT_IN`` :  set if negative LLR
> >> encoder i/p is supported
> >>> +   - ``RTE_BBDEV_TURBO_POS_LLR_1_BIT_IN`` :  set if positive LLR
> >>> + encoder
> >> i/p is supported
> >>> +   - ``RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP`` :  keep CRC24B bits
> >> appended while decoding
> >>> +   - ``RTE_BBDEV_TURBO_EARLY_TERMINATION`` :  set early early
> >> termination feature
> >>> +   - ``RTE_BBDEV_TURBO_DEC_SCATTER_GATHER`` :  supports scatter-
> >> gather for input/output data
> >>> +   - ``RTE_BBDEV_TURBO_HALF_ITERATION_EVEN`` :  set half iteration
> >>> +granularity
> >>> +
> >>> +Installation
> >>> +------------
> >>> +
> >>> +Section 3 of the DPDK manual provides instuctions on installing and
> >>> +compiling DPDK. The default set of bbdev compile flags may be found
> >>> +in config/common_base, where for example the flag to build the
> >>> +ACC100 5G/4G FEC device,
> ``CONFIG_RTE_LIBRTE_PMD_BBDEV_ACC100``,
> >>> +is already set.
> >>> +
> >>> +DPDK requires hugepages to be configured as detailed in section 2
> >>> +of the
> >> DPDK manual.
> >>> +The bbdev test application has been tested with a configuration 40
> >>> +x 1GB hugepages. The hugepage configuration of a server may be
> >>> +examined
> >> using:
> >>> +
> >>> +.. code-block:: console
> >>> +
> >>> +   grep Huge* /proc/meminfo
> >>> +
> >>> +
> >>> +Initialization
> >>> +--------------
> >>> +
> >>> +When the device first powers up, its PCI Physical Functions (PF)
> >>> +can be
> >> listed through this command:
> >>> +
> >>> +.. code-block:: console
> >>> +
> >>> +  sudo lspci -vd8086:0d5c
> >>> +
> >>> +The physical and virtual functions are compatible with Linux UIO
> drivers:
> >>> +``vfio`` and ``igb_uio``. However, in order to work the ACC100
> >>> +5G/4G FEC device firstly needs to be bound to one of these linux
> >>> +drivers through
> >> DPDK.
> >> FEC device first
> > ok
> >
> >>> +
> >>> +
> >>> +Bind PF UIO driver(s)
> >>> +~~~~~~~~~~~~~~~~~~~~~
> >>> +
> >>> +Install the DPDK igb_uio driver, bind it with the PF PCI device ID
> >>> +and use ``lspci`` to confirm the PF device is under use by
> >>> +``igb_uio`` DPDK
> >> UIO driver.
> >>> +
> >>> +The igb_uio driver may be bound to the PF PCI device using one of
> >>> +three
> >> methods:
> >>> +
> >>> +
> >>> +1. PCI functions (physical or virtual, depending on the use case)
> >>> +can be bound to the UIO driver by repeating this command for every
> function.
> >>> +
> >>> +.. code-block:: console
> >>> +
> >>> +  cd <dpdk-top-level-directory>
> >>> +  insmod ./build/kmod/igb_uio.ko
> >>> +  echo "8086 0d5c" > /sys/bus/pci/drivers/igb_uio/new_id
> >>> +  lspci -vd8086:0d5c
> >>> +
> >>> +
> >>> +2. Another way to bind PF with DPDK UIO driver is by using the
> >>> +``dpdk-devbind.py`` tool
> >>> +
> >>> +.. code-block:: console
> >>> +
> >>> +  cd <dpdk-top-level-directory>
> >>> +  ./usertools/dpdk-devbind.py -b igb_uio 0000:06:00.0
> >>> +
> >>> +where the PCI device ID (example: 0000:06:00.0) is obtained using
> >>> +lspci -vd8086:0d5c
> >>> +
> >>> +
> >>> +3. A third way to bind is to use ``dpdk-setup.sh`` tool
> >>> +
> >>> +.. code-block:: console
> >>> +
> >>> +  cd <dpdk-top-level-directory>
> >>> +  ./usertools/dpdk-setup.sh
> >>> +
> >>> +  select 'Bind Ethernet/Crypto/Baseband device to IGB UIO module'
> >>> +  or
> >>> +  select 'Bind Ethernet/Crypto/Baseband device to VFIO module'
> >>> + depending on driver required
> >> This is the igb_uio section, should defer vfio select to its section.
> > Ok
> >
> >>> +  enter PCI device ID
> >>> +  select 'Display current Ethernet/Crypto/Baseband device settings'
> >>> + to confirm binding
> >>> +
> >>> +
> >>> +In the same way the ACC100 5G/4G FEC PF can be bound with vfio, but
> >>> +vfio driver does not support SR-IOV configuration right out of the
> >>> +box, so
> >> it will need to be patched.
> >> Other documentation says works with 5.7
> > Yes this is a bit historical now. I can remove this bit which is not very
> informative and non specific to that PMD.
> >
> >>> +
> >>> +
> >>> +Enable Virtual Functions
> >>> +~~~~~~~~~~~~~~~~~~~~~~~~
> >>> +
> >>> +Now, it should be visible in the printouts that PCI PF is under
> >>> +igb_uio control "``Kernel driver in use: igb_uio``"
> >>> +
> >>> +To show the number of available VFs on the device, read
> >>> +``sriov_totalvfs``
> >> file..
> >>> +
> >>> +.. code-block:: console
> >>> +
> >>> +  cat /sys/bus/pci/devices/0000\:<b>\:<d>.<f>/sriov_totalvfs
> >>> +
> >>> +  where 0000\:<b>\:<d>.<f> is the PCI device ID
> >>> +
> >>> +
> >>> +To enable VFs via igb_uio, echo the number of virtual functions
> >>> +intended to enable to ``max_vfs`` file..
> >>> +
> >>> +.. code-block:: console
> >>> +
> >>> +  echo <num-of-vfs> >
> >>> + /sys/bus/pci/devices/0000\:<b>\:<d>.<f>/max_vfs
> >>> +
> >>> +
> >>> +Afterwards, all VFs must be bound to appropriate UIO drivers as
> >>> +required, same way it was done with the physical function previously.
> >>> +
> >>> +Enabling SR-IOV via vfio driver is pretty much the same, except
> >>> +that the file name is different:
> >>> +
> >>> +.. code-block:: console
> >>> +
> >>> +  echo <num-of-vfs> >
> >>> + /sys/bus/pci/devices/0000\:<b>\:<d>.<f>/sriov_numvfs
> >>> +
> >>> +
> >>> +Configure the VFs through PF
> >>> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >>> +
> >>> +The PCI virtual functions must be configured before working or
> >>> +getting assigned to VMs/Containers. The configuration involves
> >>> +allocating the number of hardware queues, priorities, load balance,
> >>> +bandwidth and other settings necessary for the device to perform
> >>> +FEC
> >> functions.
> >>> +
> >>> +This configuration needs to be executed at least once after reboot
> >>> +or PCI FLR and can be achieved by using the function
> >>> +``acc100_configure()``, which sets up the parameters defined in
> >> ``acc100_conf`` structure.
> >>> +
> >>> +Test Application
> >>> +----------------
> >>> +
> >>> +BBDEV provides a test application, ``test-bbdev.py`` and range of
> >>> +test data for testing the functionality of ACC100 5G/4G FEC encode
> >>> +and decode, depending on the device's capabilities. The test
> >>> +application is located under app->test-bbdev folder and has the
> >>> +following
> >> options:
> >>> +
> >>> +.. code-block:: console
> >>> +
> >>> +  "-p", "--testapp-path": specifies path to the bbdev test app.
> >>> +  "-e", "--eal-params"	: EAL arguments which are passed to the test
> >> app.
> >>> +  "-t", "--timeout"	: Timeout in seconds (default=300).
> >>> +  "-c", "--test-cases"	: Defines test cases to run. Run all if not
> specified.
> >>> +  "-v", "--test-vector"	: Test vector path
> (default=dpdk_path+/app/test-
> >> bbdev/test_vectors/bbdev_null.data).
> >>> +  "-n", "--num-ops"	: Number of operations to process on device
> >> (default=32).
> >>> +  "-b", "--burst-size"	: Operations enqueue/dequeue burst size
> >> (default=32).
> >>> +  "-s", "--snr"		: SNR in dB used when generating LLRs for
> bler tests.
> >>> +  "-s", "--iter_max"	: Number of iterations for LDPC decoder.
> >>> +  "-l", "--num-lcores"	: Number of lcores to run (default=16).
> >>> +  "-i", "--init-device" : Initialise PF device with default values.
> >>> +
> >>> +
> >>> +To execute the test application tool using simple decode or encode
> >>> +data, type one of the following:
> >>> +
> >>> +.. code-block:: console
> >>> +
> >>> +  ./test-bbdev.py -c validation -n 64 -b 1 -v
> >>> + ./ldpc_dec_default.data ./test-bbdev.py -c validation -n 64 -b 1
> >>> + -v ./ldpc_enc_default.data
> >>> +
> >>> +
> >>> +The test application ``test-bbdev.py``, supports the ability to
> >>> +configure the PF device with a default set of values, if the "-i"
> >>> +or
> >>> +"- -init-device" option is included. The default values are defined
> >>> +in
> >> test_bbdev_perf.c.
> >>> +
> >>> +
> >>> +Test Vectors
> >>> +~~~~~~~~~~~~
> >>> +
> >>> +In addition to the simple LDPC decoder and LDPC encoder tests,
> >>> +bbdev also provides a range of additional tests under the
> >>> +test_vectors folder, which may be useful. The results of these
> >>> +tests will depend on the ACC100 5G/4G FEC capabilities which may
> >>> +cause some testcases to
> >> be skipped, but no failure should be reported.
> >>
> >> Just
> >>
> >> to be skipped.
> >>
> >> should be able to assume skipped test do not get reported as failures.
> > Not necessaraly that obvious from feedback. It doesn't hurt to be
> > explicit and this statement is common to all PMDs.
> >
> ok
> 
> >>> diff --git a/doc/guides/bbdevs/features/acc100.ini
> >>> b/doc/guides/bbdevs/features/acc100.ini
> >>> new file mode 100644
> >>> index 0000000..c89a4d7
> >>> --- /dev/null
> >>> +++ b/doc/guides/bbdevs/features/acc100.ini
> >>> @@ -0,0 +1,14 @@
> >>> +;
> >>> +; Supported features of the 'acc100' bbdev driver.
> >>> +;
> >>> +; Refer to default.ini for the full list of available PMD features.
> >>> +;
> >>> +[Features]
> >>> +Turbo Decoder (4G)     = N
> >>> +Turbo Encoder (4G)     = N
> >>> +LDPC Decoder (5G)      = N
> >>> +LDPC Encoder (5G)      = N
> >>> +LLR/HARQ Compression   = N
> >>> +External DDR Access    = N
> >>> +HW Accelerated         = Y
> >>> +BBDEV API              = Y
> >>> diff --git a/doc/guides/bbdevs/index.rst
> >>> b/doc/guides/bbdevs/index.rst index a8092dd..4445cbd 100644
> >>> --- a/doc/guides/bbdevs/index.rst
> >>> +++ b/doc/guides/bbdevs/index.rst
> >>> @@ -13,3 +13,4 @@ Baseband Device Drivers
> >>>      turbo_sw
> >>>      fpga_lte_fec
> >>>      fpga_5gnr_fec
> >>> +    acc100
> >>> diff --git a/drivers/baseband/acc100/meson.build
> >>> b/drivers/baseband/acc100/meson.build
> >>> new file mode 100644
> >>> index 0000000..8afafc2
> >>> --- /dev/null
> >>> +++ b/drivers/baseband/acc100/meson.build
> >>> @@ -0,0 +1,6 @@
> >>> +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2020 Intel
> >>> +Corporation
> >>> +
> >>> +deps += ['bbdev', 'bus_vdev', 'ring', 'pci', 'bus_pci']
> >>> +
> >>> +sources = files('rte_acc100_pmd.c')
> >>> diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c
> >>> b/drivers/baseband/acc100/rte_acc100_pmd.c
> >>> new file mode 100644
> >>> index 0000000..1b4cd13
> >>> --- /dev/null
> >>> +++ b/drivers/baseband/acc100/rte_acc100_pmd.c
> >>> @@ -0,0 +1,175 @@
> >>> +/* SPDX-License-Identifier: BSD-3-Clause
> >>> + * Copyright(c) 2020 Intel Corporation  */
> >>> +
> >>> +#include <unistd.h>
> >>> +
> >>> +#include <rte_common.h>
> >>> +#include <rte_log.h>
> >>> +#include <rte_dev.h>
> >>> +#include <rte_malloc.h>
> >>> +#include <rte_mempool.h>
> >>> +#include <rte_byteorder.h>
> >>> +#include <rte_errno.h>
> >>> +#include <rte_branch_prediction.h>
> >>> +#include <rte_hexdump.h>
> >>> +#include <rte_pci.h>
> >>> +#include <rte_bus_pci.h>
> >>> +
> >>> +#include <rte_bbdev.h>
> >>> +#include <rte_bbdev_pmd.h>
> >> Should these #includes' be in alpha order ?
> > Interesting comment. Is this a coding guide line for DPDK or others?
> > I have never heard of this personnally, what is the rational?
> 
> Not sure if this is dpdk style, i know some other project do this.
> 
> This works for self consistent headers, no idea if dpdk are.
> 
> don't bother with this.
> 
> >>> +#include "rte_acc100_pmd.h"
> >>> +
> >>> +#ifdef RTE_LIBRTE_BBDEV_DEBUG
> >>> +RTE_LOG_REGISTER(acc100_logtype, pmd.bb.acc100, DEBUG); #else
> >>> +RTE_LOG_REGISTER(acc100_logtype, pmd.bb.acc100, NOTICE); #endif
> >>> +
> >>> +/* Free 64MB memory used for software rings */ static int
> >>> +acc100_dev_close(struct rte_bbdev *dev  __rte_unused) {
> >>> +	return 0;
> >>> +}
> >>> +
> >>> +static const struct rte_bbdev_ops acc100_bbdev_ops = {
> >>> +	.close = acc100_dev_close,
> >>> +};
> >>> +
> >>> +/* ACC100 PCI PF address map */
> >>> +static struct rte_pci_id pci_id_acc100_pf_map[] = {
> >>> +	{
> >>> +		RTE_PCI_DEVICE(RTE_ACC100_VENDOR_ID,
> >> RTE_ACC100_PF_DEVICE_ID)
> >>> +	},
> >>> +	{.device_id = 0},
> >>> +};
> >>> +
> >>> +/* ACC100 PCI VF address map */
> >>> +static struct rte_pci_id pci_id_acc100_vf_map[] = {
> >>> +	{
> >>> +		RTE_PCI_DEVICE(RTE_ACC100_VENDOR_ID,
> >> RTE_ACC100_VF_DEVICE_ID)
> >>> +	},
> >>> +	{.device_id = 0},
> >>> +};
> >>> +
> >>> +/* Initialization Function */
> >>> +static void
> >>> +acc100_bbdev_init(struct rte_bbdev *dev, struct rte_pci_driver
> >>> +*drv) {
> >>> +	struct rte_pci_device *pci_dev = RTE_DEV_TO_PCI(dev->device);
> >>> +
> >>> +	dev->dev_ops = &acc100_bbdev_ops;
> >>> +
> >>> +	((struct acc100_device *) dev->data->dev_private)->pf_device =
> >>> +			!strcmp(drv->driver.name,
> >>> +
> >> 	RTE_STR(ACC100PF_DRIVER_NAME));
> >>> +	((struct acc100_device *) dev->data->dev_private)->mmio_base =
> >>> +			pci_dev->mem_resource[0].addr;
> >>> +
> >>> +	rte_bbdev_log_debug("Init device %s [%s] @ vaddr %p paddr
> >> %#"PRIx64"",
> >>> +			drv->driver.name, dev->data->name,
> >>> +			(void *)pci_dev->mem_resource[0].addr,
> >>> +			pci_dev->mem_resource[0].phys_addr);
> >>> +}
> >>> +
> >>> +static int acc100_pci_probe(struct rte_pci_driver *pci_drv,
> >>> +	struct rte_pci_device *pci_dev)
> >>> +{
> >>> +	struct rte_bbdev *bbdev = NULL;
> >>> +	char dev_name[RTE_BBDEV_NAME_MAX_LEN];
> >>> +
> >>> +	if (pci_dev == NULL) {
> >>> +		rte_bbdev_log(ERR, "NULL PCI device");
> >>> +		return -EINVAL;
> >>> +	}
> >>> +
> >>> +	rte_pci_device_name(&pci_dev->addr, dev_name,
> >> sizeof(dev_name));
> >>> +
> >>> +	/* Allocate memory to be used privately by drivers */
> >>> +	bbdev = rte_bbdev_allocate(pci_dev->device.name);
> >>> +	if (bbdev == NULL)
> >>> +		return -ENODEV;
> >>> +
> >>> +	/* allocate device private memory */
> >>> +	bbdev->data->dev_private = rte_zmalloc_socket(dev_name,
> >>> +			sizeof(struct acc100_device), RTE_CACHE_LINE_SIZE,
> >>> +			pci_dev->device.numa_node);
> >>> +
> >>> +	if (bbdev->data->dev_private == NULL) {
> >>> +		rte_bbdev_log(CRIT,
> >>> +				"Allocate of %zu bytes for device \"%s\"
> >> failed",
> >>> +				sizeof(struct acc100_device), dev_name);
> >>> +				rte_bbdev_release(bbdev);
> >>> +			return -ENOMEM;
> >>> +	}
> >>> +
> >>> +	/* Fill HW specific part of device structure */
> >>> +	bbdev->device = &pci_dev->device;
> >>> +	bbdev->intr_handle = &pci_dev->intr_handle;
> >>> +	bbdev->data->socket_id = pci_dev->device.numa_node;
> >>> +
> >>> +	/* Invoke ACC100 device initialization function */
> >>> +	acc100_bbdev_init(bbdev, pci_drv);
> >>> +
> >>> +	rte_bbdev_log_debug("Initialised bbdev %s (id = %u)",
> >>> +			dev_name, bbdev->data->dev_id);
> >>> +	return 0;
> >>> +}
> >>> +
> >>> +static int acc100_pci_remove(struct rte_pci_device *pci_dev) {
> >>> +	struct rte_bbdev *bbdev;
> >>> +	int ret;
> >>> +	uint8_t dev_id;
> >>> +
> >>> +	if (pci_dev == NULL)
> >>> +		return -EINVAL;
> >>> +
> >>> +	/* Find device */
> >>> +	bbdev = rte_bbdev_get_named_dev(pci_dev->device.name);
> >>> +	if (bbdev == NULL) {
> >>> +		rte_bbdev_log(CRIT,
> >>> +				"Couldn't find HW dev \"%s\" to uninitialise
> >> it",
> >>> +				pci_dev->device.name);
> >>> +		return -ENODEV;
> >>> +	}
> >>> +	dev_id = bbdev->data->dev_id;
> >>> +
> >>> +	/* free device private memory before close */
> >>> +	rte_free(bbdev->data->dev_private);
> >>> +
> >>> +	/* Close device */
> >>> +	ret = rte_bbdev_close(dev_id);
> >> Do you want to reorder this close before the rte_free so you could
> >> recover from the failure ?
> > Given this is done the same way for other PMDs I would not change it as it
> would create a discrepency.
> > It could be done in principle as another patch for multiple PMDs to
> support this, but really I don't see a usecase for try to fall back in case there
> was such a speculative aerror.
> >
> fair enough
> 
> Tom
> 
> >> Tom
> >>
> > Thanks
> > Nic
> >
> >
> >>> +	if (ret < 0)
> >>> +		rte_bbdev_log(ERR,
> >>> +				"Device %i failed to close during uninit: %i",
> >>> +				dev_id, ret);
> >>> +
> >>> +	/* release bbdev from library */
> >>> +	rte_bbdev_release(bbdev);
> >>> +
> >>> +	rte_bbdev_log_debug("Destroyed bbdev = %u", dev_id);
> >>> +
> >>> +	return 0;
> >>> +}
> >>> +
> >>> +static struct rte_pci_driver acc100_pci_pf_driver = {
> >>> +		.probe = acc100_pci_probe,
> >>> +		.remove = acc100_pci_remove,
> >>> +		.id_table = pci_id_acc100_pf_map,
> >>> +		.drv_flags = RTE_PCI_DRV_NEED_MAPPING };
> >>> +
> >>> +static struct rte_pci_driver acc100_pci_vf_driver = {
> >>> +		.probe = acc100_pci_probe,
> >>> +		.remove = acc100_pci_remove,
> >>> +		.id_table = pci_id_acc100_vf_map,
> >>> +		.drv_flags = RTE_PCI_DRV_NEED_MAPPING };
> >>> +
> >>> +RTE_PMD_REGISTER_PCI(ACC100PF_DRIVER_NAME,
> >> acc100_pci_pf_driver);
> >>> +RTE_PMD_REGISTER_PCI_TABLE(ACC100PF_DRIVER_NAME,
> >>> +pci_id_acc100_pf_map);
> >> RTE_PMD_REGISTER_PCI(ACC100VF_DRIVER_NAME,
> >>> +acc100_pci_vf_driver);
> >>> +RTE_PMD_REGISTER_PCI_TABLE(ACC100VF_DRIVER_NAME,
> >>> +pci_id_acc100_vf_map);
> >>> +
> >>> diff --git a/drivers/baseband/acc100/rte_acc100_pmd.h
> >>> b/drivers/baseband/acc100/rte_acc100_pmd.h
> >>> new file mode 100644
> >>> index 0000000..6f46df0
> >>> --- /dev/null
> >>> +++ b/drivers/baseband/acc100/rte_acc100_pmd.h
> >>> @@ -0,0 +1,37 @@
> >>> +/* SPDX-License-Identifier: BSD-3-Clause
> >>> + * Copyright(c) 2020 Intel Corporation  */
> >>> +
> >>> +#ifndef _RTE_ACC100_PMD_H_
> >>> +#define _RTE_ACC100_PMD_H_
> >>> +
> >>> +/* Helper macro for logging */
> >>> +#define rte_bbdev_log(level, fmt, ...) \
> >>> +	rte_log(RTE_LOG_ ## level, acc100_logtype, fmt "\n", \
> >>> +		##__VA_ARGS__)
> >>> +
> >>> +#ifdef RTE_LIBRTE_BBDEV_DEBUG
> >>> +#define rte_bbdev_log_debug(fmt, ...) \
> >>> +		rte_bbdev_log(DEBUG, "acc100_pmd: " fmt, \
> >>> +		##__VA_ARGS__)
> >>> +#else
> >>> +#define rte_bbdev_log_debug(fmt, ...) #endif
> >>> +
> >>> +/* ACC100 PF and VF driver names */
> >>> +#define ACC100PF_DRIVER_NAME           intel_acc100_pf
> >>> +#define ACC100VF_DRIVER_NAME           intel_acc100_vf
> >>> +
> >>> +/* ACC100 PCI vendor & device IDs */
> >>> +#define RTE_ACC100_VENDOR_ID           (0x8086)
> >>> +#define RTE_ACC100_PF_DEVICE_ID        (0x0d5c)
> >>> +#define RTE_ACC100_VF_DEVICE_ID        (0x0d5d)
> >>> +
> >>> +/* Private data structure for each ACC100 device */ struct
> >>> +acc100_device {
> >>> +	void *mmio_base;  /**< Base address of MMIO registers (BAR0) */
> >>> +	bool pf_device; /**< True if this is a PF ACC100 device */
> >>> +	bool configured; /**< True if this ACC100 device is configured */
> >>> +};
> >>> +
> >>> +#endif /* _RTE_ACC100_PMD_H_ */
> >>> diff --git
> >>> a/drivers/baseband/acc100/rte_pmd_bbdev_acc100_version.map
> >>> b/drivers/baseband/acc100/rte_pmd_bbdev_acc100_version.map
> >>> new file mode 100644
> >>> index 0000000..4a76d1d
> >>> --- /dev/null
> >>> +++ b/drivers/baseband/acc100/rte_pmd_bbdev_acc100_version.map
> >>> @@ -0,0 +1,3 @@
> >>> +DPDK_21 {
> >>> +	local: *;
> >>> +};
> >>> diff --git a/drivers/baseband/meson.build
> >>> b/drivers/baseband/meson.build index 415b672..72301ce 100644
> >>> --- a/drivers/baseband/meson.build
> >>> +++ b/drivers/baseband/meson.build
> >>> @@ -5,7 +5,7 @@ if is_windows
> >>>  	subdir_done()
> >>>  endif
> >>>
> >>> -drivers = ['null', 'turbo_sw', 'fpga_lte_fec', 'fpga_5gnr_fec']
> >>> +drivers = ['null', 'turbo_sw', 'fpga_lte_fec', 'fpga_5gnr_fec',
> >>> +'acc100']
> >>>
> >>>  config_flag_fmt = 'RTE_LIBRTE_PMD_BBDEV_ at 0@'
> >>>  driver_name_fmt = 'rte_pmd_bbdev_ at 0@'



More information about the dev mailing list