Hi Tom,
> From: Tom Rix <t...@redhat.com>
> On 9/29/20 4:17 PM, Chautru, Nicolas wrote:
> > Hi Tom,
> >
> >> -----Original Message-----
> >> From: Tom Rix <t...@redhat.com>
> >> Sent: Tuesday, September 29, 2020 12:54 PM
> >> To: Chautru, Nicolas <nicolas.chau...@intel.com>; dev@dpdk.org;
> >> akhil.go...@nxp.com
> >> Cc: Richardson, Bruce <bruce.richard...@intel.com>; Xu, Rosen
> >> <rosen...@intel.com>; dave.bur...@accelercomm.com;
> >> aidan.godd...@accelercomm.com; Yigit, Ferruh
> >> <ferruh.yi...@intel.com>; Liu, Tianjiao <tianjiao....@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.chau...@intel.com>
> >>> Acked-by: Liu Tianjiao <tianjiao....@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_@0@'
> >>> driver_name_fmt = 'rte_pmd_bbdev_@0@'
