[PATCH v3] vhost: add IRQ suppression

[Maxime Coquelin]

Guest notifications offloading, which has been introduced
in v23.07, aims at offloading syscalls out of the datapath.

This patch optimizes the offloading by not offloading the
guest notification for a given virtqueue if one is already
being offloaded by the application.

With a single VDUSE device, we can already see few
notifications being suppressed when doing throughput
testing with Iperf3. We can expect to see much more being
suppressed when the offloading thread is under pressure.

Signed-off-by: Maxime Coquelin 
---

v3: s/0/false/ (David)

 lib/vhost/vhost.c |  4 
 lib/vhost/vhost.h | 27 +--
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c
index c03bb9c6eb..7fde412ef3 100644
--- a/lib/vhost/vhost.c
+++ b/lib/vhost/vhost.c
@@ -49,6 +49,8 @@ static const struct vhost_vq_stats_name_off 
vhost_vq_stat_strings[] = {
stats.guest_notifications_offloaded)},
{"guest_notifications_error", offsetof(struct vhost_virtqueue,
stats.guest_notifications_error)},
+   {"guest_notifications_suppressed", offsetof(struct vhost_virtqueue,
+   stats.guest_notifications_suppressed)},
{"iotlb_hits", offsetof(struct vhost_virtqueue, 
stats.iotlb_hits)},
{"iotlb_misses",   offsetof(struct vhost_virtqueue, 
stats.iotlb_misses)},
{"inflight_submitted", offsetof(struct vhost_virtqueue, 
stats.inflight_submitted)},
@@ -1517,6 +1519,8 @@ rte_vhost_notify_guest(int vid, uint16_t queue_id)

rte_rwlock_read_lock(&vq->access_lock);

+   __atomic_store_n(&vq->irq_pending, false, __ATOMIC_RELEASE);
+
if (dev->backend_ops->inject_irq(dev, vq)) {
if (dev->flags & VIRTIO_DEV_STATS_ENABLED)
__atomic_fetch_add(&vq->stats.guest_notifications_error,
diff --git a/lib/vhost/vhost.h b/lib/vhost/vhost.h
index 9723429b1c..5fc9035a1f 100644
--- a/lib/vhost/vhost.h
+++ b/lib/vhost/vhost.h
@@ -156,6 +156,7 @@ struct virtqueue_stats {
uint64_t iotlb_misses;
uint64_t inflight_submitted;
uint64_t inflight_completed;
+   uint64_t guest_notifications_suppressed;
/* Counters below are atomic, and should be incremented as such. */
uint64_t guest_notifications;
uint64_t guest_notifications_offloaded;
@@ -346,6 +347,8 @@ struct vhost_virtqueue {

struct vhost_vring_addr ring_addrs;
struct virtqueue_stats  stats;
+
+   bool irq_pending;
 } __rte_cache_aligned;

 /* Virtio device status as per Virtio specification */
@@ -908,12 +911,24 @@ vhost_need_event(uint16_t event_idx, uint16_t new_idx, 
uint16_t old)
 static __rte_always_inline void
 vhost_vring_inject_irq(struct virtio_net *dev, struct vhost_virtqueue *vq)
 {
-   if (dev->notify_ops->guest_notify &&
-   dev->notify_ops->guest_notify(dev->vid, vq->index)) {
-   if (dev->flags & VIRTIO_DEV_STATS_ENABLED)
-   
__atomic_fetch_add(&vq->stats.guest_notifications_offloaded,
-   1, __ATOMIC_RELAXED);
-   return;
+   bool expected = false;
+
+   if (dev->notify_ops->guest_notify) {
+   if (__atomic_compare_exchange_n(&vq->irq_pending, &expected, 
true, 0,
+ __ATOMIC_RELEASE, __ATOMIC_RELAXED)) {
+   if (dev->notify_ops->guest_notify(dev->vid, vq->index)) 
{
+   if (dev->flags & VIRTIO_DEV_STATS_ENABLED)
+   
__atomic_fetch_add(&vq->stats.guest_notifications_offloaded,
+   1, __ATOMIC_RELAXED);
+   return;
+   }
+
+   /* Offloading failed, fallback to direct IRQ injection 
*/
+   __atomic_store_n(&vq->irq_pending, false, 
__ATOMIC_RELEASE);
+   } else {
+   vq->stats.guest_notifications_suppressed++;
+   return;
+   }
}

if (dev->backend_ops->inject_irq(dev, vq)) {
--
2.41.0

Re: [PATCH 1/3] vhost: fix build for powerpc

[Maxime Coquelin]

On 8/31/23 14:10, Bruce Richardson wrote:

When building on Ubuntu using the packaged powerpc compiler[1], a
warning is issued about the print format of the __u64 values.

../../lib/vhost/vduse.c: In function ‘vduse_vring_setup’:
../../lib/vhost/vhost.h:676:17: error: format ‘%llx’ expects argument of
type ‘long long unsigned int’, but argument 5 has type ‘__u64’ {aka
‘long unsigned int’} [-Werror=format=]
   676 | "VHOST_CONFIG: (%s) " fmt, prefix, ##args)
   | ^

Changing the format specifier to %lx, or to use PRIx64 breaks other
builds, so the safest solution is to explicitly typecast the printed
values to match the format string.

[1] powerpc64le-linux-gnu-gcc (Ubuntu 12.3.0-1ubuntu1~23.04) 12.3.0

Fixes: a9120db8b98b ("vhost: add VDUSE device startup")
Cc: maxime.coque...@redhat.com
Cc: sta...@dpdk.org

Signed-off-by: Bruce Richardson 
---
  lib/vhost/vduse.c | 9 ++---
  1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/lib/vhost/vduse.c b/lib/vhost/vduse.c
index 73ed424232..e2b6d35d37 100644
--- a/lib/vhost/vduse.c
+++ b/lib/vhost/vduse.c
@@ -162,9 +162,12 @@ vduse_vring_setup(struct virtio_net *dev, unsigned int 
index)
  
  	VHOST_LOG_CONFIG(dev->ifname, INFO, "VQ %u info:\n", index);

VHOST_LOG_CONFIG(dev->ifname, INFO, "\tnum: %u\n", vq_info.num);
-   VHOST_LOG_CONFIG(dev->ifname, INFO, "\tdesc_addr: %llx\n", 
vq_info.desc_addr);
-   VHOST_LOG_CONFIG(dev->ifname, INFO, "\tdriver_addr: %llx\n", 
vq_info.driver_addr);
-   VHOST_LOG_CONFIG(dev->ifname, INFO, "\tdevice_addr: %llx\n", 
vq_info.device_addr);
+   VHOST_LOG_CONFIG(dev->ifname, INFO, "\tdesc_addr: %llx\n",
+   (unsigned long long)vq_info.desc_addr);
+   VHOST_LOG_CONFIG(dev->ifname, INFO, "\tdriver_addr: %llx\n",
+   (unsigned long long)vq_info.driver_addr);
+   VHOST_LOG_CONFIG(dev->ifname, INFO, "\tdevice_addr: %llx\n",
+   (unsigned long long)vq_info.device_addr);
VHOST_LOG_CONFIG(dev->ifname, INFO, "\tavail_idx: %u\n", 
vq_info.split.avail_index);
VHOST_LOG_CONFIG(dev->ifname, INFO, "\tready: %u\n", vq_info.ready);
  


It is surprising PRIx64 does not work on other architectures.
I don't see a better solution, so:

Acked-by: Maxime Coquelin 

Thanks,
Maxime

Re: [PATCH v1 1/7] bbdev: add FFT version member in driver info

[Maxime Coquelin]

On 9/28/23 18:33, Chautru, Nicolas wrote:

HI Maxime,



-Original Message-
From: Maxime Coquelin 
Sent: Thursday, September 28, 2023 1:27 AM
To: Chautru, Nicolas ; hemant.agra...@nxp.com;
dev@dpdk.org
Cc: david.march...@redhat.com; Vargas, Hernan 
Subject: Re: [PATCH v1 1/7] bbdev: add FFT version member in driver info

Hi Nicolas,

On 9/28/23 01:50, Chautru, Nicolas wrote:

Hi Maxime, Hemant,

I wanted initially to keep it fairly open hence a hash table for the windows

profiles, but it is also possible to expose something more descriptive, that
would work as well actually.

Ie.

+   /** FFT windowing width for 2048 FFT. */
+   uint16_t fft_window_width[RTE_BBDEV_MAX_FFT_WIN];

The provides the width of each windows shape which is enough to

distinguish major variants and to estimate noise factor.

That sounds much better IMHO.

Regarding the array and values sizes:
1. Should it only covers 2048 points FFT? I see some references about
4096 FFT for 5G and satellites  communications 2. Is uint16_t enough for all the
usecases?


That is a misunderstanding, probably because I did not include the definition 
and value of RTE_BBDEV_MAX_FFT_WIN on the snippet above.
The dimension of the array is purely the number of windows to choose from, ie. 
16.


Ok, where is the number of windows made available?


That is NOT an array matching the size of the FFT. In effect that width value 
is for the reference for 2048 FFT, but the actual width would be scaled down 
when a lower FFT is being set or higher for bigger FFT, so this doesn’t make 
assumption on the max FFT size, just a given portion using a reference 
resolution for 2048 FFT.


Ok, if you could document this with example that would be great.
Maybe you have some existing links explaining that?


Uint16_t is more than enough, that width cannot be more than 1024 based on 
reference above.



Since this array is quite big, could it be exposed to the application via 
dedicated
APIs instead of a field? An API to query the length of the array so that the
application can allocate required meory, and an API to copy the data in the
allocated mem?

Maybe overkill, but I feel different FFT size could be supported in the future, 
so
that would be both future proof and more memory efficient for apps that
don't need this.


Note above, let me know if unclear.


It is not clear to me how this representation is generic or specific to
your device.

Thanks,
Maxime






Let me know of opinion.


Thanks for suggesting this,
Maxime


Thanks
Nic


-Original Message-
From: Maxime Coquelin 
Sent: Tuesday, September 26, 2023 3:00 AM
To: Chautru, Nicolas ;
hemant.agra...@nxp.com; dev@dpdk.org
Cc: david.march...@redhat.com; Vargas, Hernan

Subject: Re: [PATCH v1 1/7] bbdev: add FFT version member in driver
info



On 9/22/23 18:41, Chautru, Nicolas wrote:

Hi Maxime,


-Original Message-
From: Maxime Coquelin 
Sent: Friday, September 22, 2023 1:15 AM
To: Chautru, Nicolas ;
hemant.agra...@nxp.com; dev@dpdk.org
Cc: david.march...@redhat.com; Vargas, Hernan

Subject: Re: [PATCH v1 1/7] bbdev: add FFT version member in driver
info

Hi Nicolas,

On 9/19/23 22:51, Chautru, Nicolas wrote:

Hi Maxime,

This is neither part of 3GPP per se, nor specific to VRB device.
Let me provide

more context.

The SRS processing chain

(https://doc.dpdk.org/guides/prog_guide/bbdev.html#bbdev-fft-operat
io
n) includes a pointwise multiplication by time window.

The generic API include some control of these windowing function
but still

the actual shape need to be programmed onto any device (ie.
rectangular, taped, sinc, different width or offset, any abritraty
shape defined as an array of scalars). These degrees of liberties
cannot be exposed through a generic API (information is multi-kB,
ie the data itself) and can be user specific (external to the HW IP
itself or

outside of Intel control).


Thanks for the explanations. I also did my homework as my FFT
knowledge was buried quite deep in my memory. :)

So this is a vendor-specific way to express generic paramaters.


Unsure this is that vendor specific. At least the interface allows
to know a

hash of the table being loaded (which is just pointwise data really,
non- proprietary format). I did not state the content is a simple
md5sum of the bin file being loaded from linux.

Ok, I think it would be better to provide an API to get the table
directly, and have the format being described in the documentation.

With that, we can also provide the hash as you'd like, but the method
to calculate the hash should also be provided. Or the application can
perform the hash itself if it needs it.

The fact that it is several KB is not an issue, as this information
would only be queried once at init time if really needed.

An non-DPDK alternative could be to pass such information to the pod
via the device plugin (as a mounted file for instance, or variable).


Regarding VRB device, is this table per device or per VF?
C

Re: [PATCH v3 02/12] baseband/acc: add FFT window width in the VRB PMD

[Maxime Coquelin]

On 9/29/23 18:35, Nicolas Chautru wrote:

This allows to expose the FFT window width being introduced in
previous commit based on what is configured dynamically on the
device platform.

Signed-off-by: Nicolas Chautru 
---
  drivers/baseband/acc/acc_common.h  |  3 +++
  drivers/baseband/acc/rte_vrb_pmd.c | 29 +
  2 files changed, 32 insertions(+)

diff --git a/drivers/baseband/acc/acc_common.h 
b/drivers/baseband/acc/acc_common.h
index 5bb00746c3..7d24c644c0 100644
--- a/drivers/baseband/acc/acc_common.h
+++ b/drivers/baseband/acc/acc_common.h
@@ -512,6 +512,8 @@ struct acc_deq_intr_details {
  enum {
ACC_VF2PF_STATUS_REQUEST = 1,
ACC_VF2PF_USING_VF = 2,
+   ACC_VF2PF_LUT_VER_REQUEST = 3,
+   ACC_VF2PF_FFT_WIN_REQUEST = 4,
  };
  
  
@@ -558,6 +560,7 @@ struct acc_device {

queue_offset_fun_t queue_offset;  /* Device specific queue offset */
uint16_t num_qgroups;
uint16_t num_aqs;
+   uint16_t fft_window_width[RTE_BBDEV_MAX_FFT_WIN]; /* FFT windowing 
width. */
  };
  
  /* Structure associated with each queue. */

diff --git a/drivers/baseband/acc/rte_vrb_pmd.c 
b/drivers/baseband/acc/rte_vrb_pmd.c
index 9e5a73c9c7..c5a74bae11 100644
--- a/drivers/baseband/acc/rte_vrb_pmd.c
+++ b/drivers/baseband/acc/rte_vrb_pmd.c
@@ -298,6 +298,34 @@ vrb_device_status(struct rte_bbdev *dev)
return reg;
  }
  
+/* Request device FFT windowing information. */

+static inline void
+vrb_device_fft_win(struct rte_bbdev *dev, struct rte_bbdev_driver_info 
*dev_info)
+{
+   struct acc_device *d = dev->data->dev_private;
+   uint32_t reg, time_out = 0, win;
+
+   if (d->pf_device)
+   return;
+
+   /* Check from the device the first time. */
+   if (d->fft_window_width[0] == 0) {


O is not a possible value? Might not be a big deal as it would just
perform reading of 16 x 2 registers every time .info_get() is called,
just wondering.


+   for (win = 0; win < RTE_BBDEV_MAX_FFT_WIN; win++) {
+   vrb_vf2pf(d, ACC_VF2PF_FFT_WIN_REQUEST | win);


That looks broken, as extending RTE_BBDEV_MAX_FFT_WIN to support other
devices may break this implementation.

To me, it tends to show how this fft_window_width array is more device
specific than generic.


+   reg = acc_reg_read(d, d->reg_addr->pf2vf_doorbell);
+   while ((time_out < ACC_STATUS_TO) && (reg == 
RTE_BBDEV_DEV_NOSTATUS)) {
+   usleep(ACC_STATUS_WAIT); /*< Wait or VF->PF->VF 
Comms */
+   reg = acc_reg_read(d, 
d->reg_addr->pf2vf_doorbell);
+   time_out++;
+   }
+   d->fft_window_width[win] = reg;
+   }
+   }
+
+   for (win = 0; win < RTE_BBDEV_MAX_FFT_WIN; win++)
+   dev_info->fft_window_width[win] = d->fft_window_width[win];
+}
+
  /* Checks PF Info Ring to find the interrupt cause and handles it 
accordingly. */
  static inline void
  vrb_check_ir(struct acc_device *acc_dev)
@@ -1100,6 +1128,7 @@ vrb_dev_info_get(struct rte_bbdev *dev, struct 
rte_bbdev_driver_info *dev_info)
fetch_acc_config(dev);
/* Check the status of device. */
dev_info->device_status = vrb_device_status(dev);
+   vrb_device_fft_win(dev, dev_info);
  
  	/* Exposed number of queues. */

dev_info->num_queues[RTE_BBDEV_OP_NONE] = 0;

Re: [PATCH v3 03/12] baseband/acc: remove the 4G SO capability for VRB1

[Maxime Coquelin]

On 9/29/23 18:35, Nicolas Chautru wrote:

This removes the specific capability and support of LTE Decoder
Soft Output option on the VRB1 PMD.

This is triggered as a vendor decision to defeature the related optional
capability so that to avoid theoretical risk of race conditions
impacting the device reliability. That optional APP LLR output is
not impacting the actual decoder hard output.

Signed-off-by: Nicolas Chautru 
---
  doc/guides/bbdevs/vrb1.rst |  4 
  drivers/baseband/acc/rte_vrb_pmd.c | 10 ++
  2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/doc/guides/bbdevs/vrb1.rst b/doc/guides/bbdevs/vrb1.rst
index 9c48d30964..fdefb20651 100644
--- a/doc/guides/bbdevs/vrb1.rst
+++ b/doc/guides/bbdevs/vrb1.rst
@@ -71,11 +71,7 @@ The Intel vRAN Boost v1.0 PMD supports the following bbdev 
capabilities:
 - ``RTE_BBDEV_TURBO_EARLY_TERMINATION``: set 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.
-   - ``RTE_BBDEV_TURBO_SOFT_OUTPUT``: set the APP LLR soft output.
-   - ``RTE_BBDEV_TURBO_EQUALIZER``: set the turbo equalizer feature.
-   - ``RTE_BBDEV_TURBO_SOFT_OUT_SATURATE``: set the soft output saturation.
 - ``RTE_BBDEV_TURBO_CONTINUE_CRC_MATCH``: set to run an extra odd 
iteration after CRC match.
-   - ``RTE_BBDEV_TURBO_NEG_LLR_1_BIT_SOFT_OUT``: set if negative APP LLR 
output supported.
 - ``RTE_BBDEV_TURBO_MAP_DEC``: supports flexible parallel MAP engine 
decoding.
  
  * For the FFT operation:

diff --git a/drivers/baseband/acc/rte_vrb_pmd.c 
b/drivers/baseband/acc/rte_vrb_pmd.c
index c5a74bae11..f11882f90e 100644
--- a/drivers/baseband/acc/rte_vrb_pmd.c
+++ b/drivers/baseband/acc/rte_vrb_pmd.c
@@ -1025,15 +1025,11 @@ vrb_dev_info_get(struct rte_bbdev *dev, struct 
rte_bbdev_driver_info *dev_info)
RTE_BBDEV_TURBO_SUBBLOCK_DEINTERLEAVE |
RTE_BBDEV_TURBO_CRC_TYPE_24B |
RTE_BBDEV_TURBO_DEC_CRC_24B_DROP |
-   RTE_BBDEV_TURBO_EQUALIZER |
-   RTE_BBDEV_TURBO_SOFT_OUT_SATURATE |
RTE_BBDEV_TURBO_HALF_ITERATION_EVEN |
RTE_BBDEV_TURBO_CONTINUE_CRC_MATCH |
-   RTE_BBDEV_TURBO_SOFT_OUTPUT |
RTE_BBDEV_TURBO_EARLY_TERMINATION |
RTE_BBDEV_TURBO_DEC_INTERRUPTS |
RTE_BBDEV_TURBO_NEG_LLR_1_BIT_IN |
-   RTE_BBDEV_TURBO_NEG_LLR_1_BIT_SOFT_OUT |
RTE_BBDEV_TURBO_MAP_DEC |
RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP |
RTE_BBDEV_TURBO_DEC_SCATTER_GATHER,
@@ -1982,6 +1978,12 @@ enqueue_dec_one_op_cb(struct acc_queue *q, struct 
rte_bbdev_dec_op *op,
struct rte_mbuf *input, *h_output_head, *h_output,
*s_output_head, *s_output;
  
+	if ((q->d->device_variant == VRB1_VARIANT) &&

+   (check_bit(op->turbo_dec.op_flags, 
RTE_BBDEV_TURBO_SOFT_OUTPUT))) {
+   /* SO not supported for VRB1. */
+   return -EPERM;
+   }
+


A better option would be to have a pointer on the device capabilities in
the acc_device struct, doing so would be more future-proof. Maybe it
could be considered?

But in the mean time, it addresses this specific issue:
Reviewed-by: Maxime Coquelin 

Thanks,
Maxime


desc = acc_desc(q, total_enqueued_cbs);
vrb_fcw_td_fill(op, &desc->req.fcw_td);
  

Re: [PATCH v3] vhost: add IRQ suppression

[Maxime Coquelin]

Hi Eelco,

On 10/3/23 14:36, Eelco Chaudron wrote:



On 29 Sep 2023, at 12:38, Maxime Coquelin wrote:


Guest notifications offloading, which has been introduced
in v23.07, aims at offloading syscalls out of the datapath.

This patch optimizes the offloading by not offloading the
guest notification for a given virtqueue if one is already
being offloaded by the application.

With a single VDUSE device, we can already see few
notifications being suppressed when doing throughput
testing with Iperf3. We can expect to see much more being
suppressed when the offloading thread is under pressure.

Signed-off-by: Maxime Coquelin 


Thanks for adding this Maxime. I did some tests with OVS and my old determinism 
patchset, and it works perfectly.

I have two small nits, but this change looks good to me.

Acked-by: Eelco Chaudron 


---

v3: s/0/false/ (David)

  lib/vhost/vhost.c |  4 
  lib/vhost/vhost.h | 27 +--
  2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c
index c03bb9c6eb..7fde412ef3 100644
--- a/lib/vhost/vhost.c
+++ b/lib/vhost/vhost.c
@@ -49,6 +49,8 @@ static const struct vhost_vq_stats_name_off 
vhost_vq_stat_strings[] = {
stats.guest_notifications_offloaded)},
{"guest_notifications_error", offsetof(struct vhost_virtqueue,
stats.guest_notifications_error)},
+   {"guest_notifications_suppressed", offsetof(struct vhost_virtqueue,
+   stats.guest_notifications_suppressed)},
{"iotlb_hits", offsetof(struct vhost_virtqueue, 
stats.iotlb_hits)},
{"iotlb_misses",   offsetof(struct vhost_virtqueue, 
stats.iotlb_misses)},
{"inflight_submitted", offsetof(struct vhost_virtqueue, 
stats.inflight_submitted)},
@@ -1517,6 +1519,8 @@ rte_vhost_notify_guest(int vid, uint16_t queue_id)

rte_rwlock_read_lock(&vq->access_lock);

+   __atomic_store_n(&vq->irq_pending, false, __ATOMIC_RELEASE);
+
if (dev->backend_ops->inject_irq(dev, vq)) {
if (dev->flags & VIRTIO_DEV_STATS_ENABLED)
__atomic_fetch_add(&vq->stats.guest_notifications_error,
diff --git a/lib/vhost/vhost.h b/lib/vhost/vhost.h
index 9723429b1c..5fc9035a1f 100644
--- a/lib/vhost/vhost.h
+++ b/lib/vhost/vhost.h
@@ -156,6 +156,7 @@ struct virtqueue_stats {
uint64_t iotlb_misses;
uint64_t inflight_submitted;
uint64_t inflight_completed;
+   uint64_t guest_notifications_suppressed;
/* Counters below are atomic, and should be incremented as such. */
uint64_t guest_notifications;
uint64_t guest_notifications_offloaded;
@@ -346,6 +347,8 @@ struct vhost_virtqueue {

struct vhost_vring_addr ring_addrs;
struct virtqueue_stats  stats;
+
+   bool irq_pending;


nit: Other elements in this structure have the names aligned, not sure if this 
should be done for this item also.


Ha yes, you're right.
I'll fix it while applying.




  } __rte_cache_aligned;

  /* Virtio device status as per Virtio specification */
@@ -908,12 +911,24 @@ vhost_need_event(uint16_t event_idx, uint16_t new_idx, 
uint16_t old)
  static __rte_always_inline void
  vhost_vring_inject_irq(struct virtio_net *dev, struct vhost_virtqueue *vq)
  {
-   if (dev->notify_ops->guest_notify &&
-   dev->notify_ops->guest_notify(dev->vid, vq->index)) {
-   if (dev->flags & VIRTIO_DEV_STATS_ENABLED)
-   
__atomic_fetch_add(&vq->stats.guest_notifications_offloaded,
-   1, __ATOMIC_RELAXED);
-   return;
+   bool expected = false;
+
+   if (dev->notify_ops->guest_notify) {
+   if (__atomic_compare_exchange_n(&vq->irq_pending, &expected, 
true, 0,
+ __ATOMIC_RELEASE, __ATOMIC_RELAXED)) {
+   if (dev->notify_ops->guest_notify(dev->vid, vq->index)) 
{
+   if (dev->flags & VIRTIO_DEV_STATS_ENABLED)
+   
__atomic_fetch_add(&vq->stats.guest_notifications_offloaded,
+   1, __ATOMIC_RELAXED);
+   return;
+   }
+
+   /* Offloading failed, fallback to direct IRQ injection 
*/


nit: Some comments end with a dot and some do not, not sure what is the 
preference in DPDK.


I'm not sure either! I'm personally fine either way.


+   __atomic_store_n(&vq->irq_pending, false, 
__ATOMIC_RELEASE);
+   } else {
+   vq->stats.guest_notifications_suppressed++;
+   return;
+   }
}

if (dev->backend_ops->inject_irq(dev, vq)) {
--
2.41.0




Thanks,
Maxime

Re: [PATCH v3 04/12] baseband/acc: allocate FCW memory separately

[Maxime Coquelin]

On 9/29/23 18:35, Nicolas Chautru wrote:

This allows more flexibility to the FCW size for the
unified driver. No actual functional change.

Signed-off-by: Nicolas Chautru 
---
  drivers/baseband/acc/acc_common.h  |  4 +++-
  drivers/baseband/acc/rte_vrb_pmd.c | 25 -
  2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/baseband/acc/acc_common.h 
b/drivers/baseband/acc/acc_common.h
index 7d24c644c0..2c7425e524 100644
--- a/drivers/baseband/acc/acc_common.h
+++ b/drivers/baseband/acc/acc_common.h
@@ -101,6 +101,7 @@
  #define ACC_NUM_QGRPS_PER_WORD 8
  #define ACC_MAX_NUM_QGRPS  32
  #define ACC_RING_SIZE_GRANULARITY  64
+#define ACC_MAX_FCW_SIZE  128
  
  /* Constants from K0 computation from 3GPP 38.212 Table 5.4.2.1-2 */

  #define ACC_N_ZC_1 66 /* N = 66 Zc for BG 1 */
@@ -584,13 +585,14 @@ struct __rte_cache_aligned acc_queue {
uint32_t aq_enqueued;  /* Count how many "batches" have been enqueued */
uint32_t aq_dequeued;  /* Count how many "batches" have been dequeued */
uint32_t irq_enable;  /* Enable ops dequeue interrupts if set to 1 */
-   struct rte_mempool *fcw_mempool;  /* FCW mempool */
enum rte_bbdev_op_type op_type;  /* Type of this Queue: TE or TD */
/* Internal Buffers for loopback input */
uint8_t *lb_in;
uint8_t *lb_out;
+   uint8_t *fcw_ring;
rte_iova_t lb_in_addr_iova;
rte_iova_t lb_out_addr_iova;
+   rte_iova_t fcw_ring_addr_iova;
int8_t *derm_buffer; /* interim buffer for de-rm in SDK */
struct acc_device *d;
  };
diff --git a/drivers/baseband/acc/rte_vrb_pmd.c 
b/drivers/baseband/acc/rte_vrb_pmd.c
index f11882f90e..cf0551c0c7 100644
--- a/drivers/baseband/acc/rte_vrb_pmd.c
+++ b/drivers/baseband/acc/rte_vrb_pmd.c
@@ -890,6 +890,25 @@ vrb_queue_setup(struct rte_bbdev *dev, uint16_t queue_id,
goto free_companion_ring_addr;
}
  
+	q->fcw_ring = rte_zmalloc_socket(dev->device->driver->name,

+   ACC_MAX_FCW_SIZE * d->sw_ring_max_depth,
+   RTE_CACHE_LINE_SIZE, conf->socket);
+   if (q->fcw_ring == NULL) {
+   rte_bbdev_log(ERR, "Failed to allocate fcw_ring memory");
+   ret = -ENOMEM;
+   goto free_companion_ring_addr;
+   }
+   q->fcw_ring_addr_iova = rte_malloc_virt2iova(q->fcw_ring);
+
+   /* For FFT we need to store the FCW separately */
+   if (conf->op_type == RTE_BBDEV_OP_FFT) {
+   for (desc_idx = 0; desc_idx < d->sw_ring_max_depth; desc_idx++) 
{
+   desc = q->ring_addr + desc_idx;
+   desc->req.data_ptrs[0].address = q->fcw_ring_addr_iova +
+   desc_idx * ACC_MAX_FCW_SIZE;
+   }
+   }
+
q->qgrp_id = (q_idx >> VRB1_GRP_ID_SHIFT) & 0xF;
q->vf_id = (q_idx >> VRB1_VF_ID_SHIFT)  & 0x3F;
q->aq_id = q_idx & 0xF;
@@ -1001,6 +1020,7 @@ vrb_queue_release(struct rte_bbdev *dev, uint16_t q_id)
if (q != NULL) {
/* Mark the Queue as un-assigned. */
d->q_assigned_bit_map[q->qgrp_id] &= (~0ULL - (1 << (uint64_t) 
q->aq_id));
+   rte_free(q->fcw_ring);
rte_free(q->companion_ring_addr);
rte_free(q->lb_in);
rte_free(q->lb_out);
@@ -3234,7 +3254,10 @@ vrb_enqueue_fft_one_op(struct acc_queue *q, struct 
rte_bbdev_fft_op *op,
output = op->fft.base_output.data;
in_offset = op->fft.base_input.offset;
out_offset = op->fft.base_output.offset;
-   fcw = &desc->req.fcw_fft;
+
+   fcw = (struct acc_fcw_fft *) (q->fcw_ring +
+   ((q->sw_ring_head + total_enqueued_cbs) & 
q->sw_ring_wrap_mask)
+       * ACC_MAX_FCW_SIZE);
  
  	vrb1_fcw_fft_fill(op, fcw);

vrb1_dma_desc_fft_fill(op, &desc->req, input, output, &in_offset, 
&out_offset);


Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [PATCH v3 06/12] baseband/acc: refactor to allow unified driver extension

[Maxime Coquelin]

Thanks for doing the split, that will ease review.

On 9/29/23 18:35, Nicolas Chautru wrote:

Adding a few functions and common code prior to
extending the VRB driver.

Signed-off-by: Nicolas Chautru 
---
  drivers/baseband/acc/acc_common.h | 164 +++---
  drivers/baseband/acc/rte_acc100_pmd.c |   4 +-
  drivers/baseband/acc/rte_vrb_pmd.c|  62 +-
  3 files changed, 184 insertions(+), 46 deletions(-)

diff --git a/drivers/baseband/acc/acc_common.h 
b/drivers/baseband/acc/acc_common.h
index 788abf1a3c..89893eae43 100644
--- a/drivers/baseband/acc/acc_common.h
+++ b/drivers/baseband/acc/acc_common.h
@@ -18,6 +18,7 @@
  #define ACC_DMA_BLKID_OUT_HARQ  3
  #define ACC_DMA_BLKID_IN_HARQ   3
  #define ACC_DMA_BLKID_IN_MLD_R  3
+#define ACC_DMA_BLKID_DEWIN_IN  3
  
  /* Values used in filling in decode FCWs */

  #define ACC_FCW_TD_VER  1
@@ -103,6 +104,9 @@
  #define ACC_MAX_NUM_QGRPS  32
  #define ACC_RING_SIZE_GRANULARITY  64
  #define ACC_MAX_FCW_SIZE  128
+#define ACC_IQ_SIZE4
+
+#define ACC_FCW_FFT_BLEN_3 28
  
  /* Constants from K0 computation from 3GPP 38.212 Table 5.4.2.1-2 */

  #define ACC_N_ZC_1 66 /* N = 66 Zc for BG 1 */
@@ -132,6 +136,17 @@
  #define ACC_LIM_21 14 /* 0.21 */
  #define ACC_LIM_31 20 /* 0.31 */
  #define ACC_MAX_E (128 * 1024 - 2)
+#define ACC_MAX_CS 12
+
+#define ACC100_VARIANT  0
+#define VRB1_VARIANT   2
+#define VRB2_VARIANT   3
+
+/* Queue Index Hierarchy */
+#define VRB1_GRP_ID_SHIFT10
+#define VRB1_VF_ID_SHIFT 4
+#define VRB2_GRP_ID_SHIFT12
+#define VRB2_VF_ID_SHIFT 6
  
  /* Helper macro for logging */

  #define rte_acc_log(level, fmt, ...) \
@@ -332,6 +347,37 @@ struct __rte_packed acc_fcw_fft {
res:19;
  };
  
+/* FFT Frame Control Word. */

+struct __rte_packed acc_fcw_fft_3 {
+   uint32_t in_frame_size:16,
+   leading_pad_size:16;
+   uint32_t out_frame_size:16,
+   leading_depad_size:16;
+   uint32_t cs_window_sel;
+   uint32_t cs_window_sel2:16,
+   cs_enable_bmap:16;
+   uint32_t num_antennas:8,
+   idft_size:8,
+   dft_size:8,
+   cs_offset:8;
+   uint32_t idft_shift:8,
+   dft_shift:8,
+   cs_multiplier:16;
+   uint32_t bypass:2,
+   fp16_in:1,
+   fp16_out:1,
+   exp_adj:4,
+   power_shift:4,
+   power_en:1,
+   enable_dewin:1,
+   freq_resample_mode:2,
+   depad_output_size:16;
+   uint16_t cs_theta_0[ACC_MAX_CS];
+   uint32_t cs_theta_d[ACC_MAX_CS];
+   int8_t cs_time_offset[ACC_MAX_CS];
+};
+
+
  /* MLD-TS Frame Control Word */
  struct __rte_packed acc_fcw_mldts {
uint32_t fcw_version:4,
@@ -473,14 +519,14 @@ union acc_info_ring_data {
uint16_t valid: 1;
};
struct {
-   uint32_t aq_id_3: 6;
-   uint32_t qg_id_3: 5;
-   uint32_t vf_id_3: 6;
-   uint32_t int_nb_3: 6;
-   uint32_t msi_0_3: 1;
-   uint32_t vf2pf_3: 6;
-   uint32_t loop_3: 1;
-   uint32_t valid_3: 1;
+   uint32_t aq_id_vrb2: 6;
+   uint32_t qg_id_vrb2: 5;
+   uint32_t vf_id_vrb2: 6;
+   uint32_t int_nb_vrb2: 6;
+   uint32_t msi_0_vrb2: 1;
+   uint32_t vf2pf_vrb2: 6;
+   uint32_t loop_vrb2: 1;
+   uint32_t valid_vrb2: 1;
};
  } __rte_packed;
  
@@ -761,22 +807,105 @@ alloc_sw_rings_min_mem(struct rte_bbdev *dev, struct acc_device *d,

free_base_addresses(base_addrs, i);
  }
  
+/* Wrapper to provide VF index from ring data. */

+static inline uint16_t
+vf_from_ring(const union acc_info_ring_data ring_data, uint16_t 
device_variant) {


curly braces on a new line.


+   if (device_variant == VRB2_VARIANT)
+   return ring_data.vf_id_vrb2;
+   else
+   return ring_data.vf_id;
+}
+
+/* Wrapper to provide QG index from ring data. */
+static inline uint16_t
+qg_from_ring(const union acc_info_ring_data ring_data, uint16_t 
device_variant) {
+   if (device_variant == VRB2_VARIANT)
+   return ring_data.qg_id_vrb2;
+   else
+   return ring_data.qg_id;
+}
+
+/* Wrapper to provide AQ index from ring data. */
+static inline uint16_t
+aq_from_ring(const union acc_info_ring_data ring_data, uint16_t 
device_variant) {
+   if (device_variant == VRB2_VARIANT)
+   return ring_data.aq_id_vrb2;
+   else
+   return ring_data.aq_id;
+}
+
+/* Wrapper to provide int index from ring data. */
+static inline uint16_t
+int_from_ring(const union acc_info_ring_data ring_data, uint16_t 
device_variant) {
+   if (device_variant == VRB2_VARIANT)
+   return ring_data.int_nb_vrb

Re: [PATCH v3 07/12] baseband/acc: adding VRB2 device variant

[Maxime Coquelin]

_group_func =   VRB2_PfQmgrGrpFunction0,
+   .hi_mode =   VRB2_PfHiMsixVectorMapperPf,
+   .pf_mode =   VRB2_PfHiPfMode,
+   .pmon_ctrl_a =   VRB2_PfPermonACntrlRegVf,
+   .pmon_ctrl_b =   VRB2_PfPermonBCntrlRegVf,
+   .pmon_ctrl_c =   VRB2_PfPermonCCntrlRegVf,
+   .vf2pf_doorbell =0,
+   .pf2vf_doorbell =0,
+};
+
+/* Structure holding registry addresses for VF */
+static const struct acc_registry_addr vrb2_vf_reg_addr = {
+   .dma_ring_dl5g_hi =  VRB2_VfDmaFec5GdlDescBaseHiRegVf,
+   .dma_ring_dl5g_lo =  VRB2_VfDmaFec5GdlDescBaseLoRegVf,
+   .dma_ring_ul5g_hi =  VRB2_VfDmaFec5GulDescBaseHiRegVf,
+   .dma_ring_ul5g_lo =  VRB2_VfDmaFec5GulDescBaseLoRegVf,
+   .dma_ring_dl4g_hi =  VRB2_VfDmaFec4GdlDescBaseHiRegVf,
+   .dma_ring_dl4g_lo =  VRB2_VfDmaFec4GdlDescBaseLoRegVf,
+   .dma_ring_ul4g_hi =  VRB2_VfDmaFec4GulDescBaseHiRegVf,
+   .dma_ring_ul4g_lo =  VRB2_VfDmaFec4GulDescBaseLoRegVf,
+   .dma_ring_fft_hi =   VRB2_VfDmaFftDescBaseHiRegVf,
+   .dma_ring_fft_lo =   VRB2_VfDmaFftDescBaseLoRegVf,
+   .dma_ring_mld_hi =   VRB2_VfDmaMldDescBaseHiRegVf,
+   .dma_ring_mld_lo =   VRB2_VfDmaMldDescBaseLoRegVf,
+   .ring_size = VRB2_VfQmgrRingSizeVf,
+   .info_ring_hi =  VRB2_VfHiInfoRingBaseHiVf,
+   .info_ring_lo =  VRB2_VfHiInfoRingBaseLoVf,
+   .info_ring_en =  VRB2_VfHiInfoRingIntWrEnVf,
+   .info_ring_ptr = VRB2_VfHiInfoRingPointerVf,
+   .tail_ptrs_dl5g_hi = VRB2_VfDmaFec5GdlRespPtrHiRegVf,
+   .tail_ptrs_dl5g_lo = VRB2_VfDmaFec5GdlRespPtrLoRegVf,
+   .tail_ptrs_ul5g_hi = VRB2_VfDmaFec5GulRespPtrHiRegVf,
+   .tail_ptrs_ul5g_lo = VRB2_VfDmaFec5GulRespPtrLoRegVf,
+   .tail_ptrs_dl4g_hi = VRB2_VfDmaFec4GdlRespPtrHiRegVf,
+   .tail_ptrs_dl4g_lo = VRB2_VfDmaFec4GdlRespPtrLoRegVf,
+   .tail_ptrs_ul4g_hi = VRB2_VfDmaFec4GulRespPtrHiRegVf,
+   .tail_ptrs_ul4g_lo = VRB2_VfDmaFec4GulRespPtrLoRegVf,
+   .tail_ptrs_fft_hi =  VRB2_VfDmaFftRespPtrHiRegVf,
+   .tail_ptrs_fft_lo =  VRB2_VfDmaFftRespPtrLoRegVf,
+   .tail_ptrs_mld_hi =  VRB2_VfDmaMldRespPtrHiRegVf,
+   .tail_ptrs_mld_lo =  VRB2_VfDmaMldRespPtrLoRegVf,
+   .depth_log0_offset = VRB2_VfQmgrGrpDepthLog20Vf,
+   .depth_log1_offset = VRB2_VfQmgrGrpDepthLog21Vf,
+   .qman_group_func =   VRB2_VfQmgrGrpFunction0Vf,
+   .hi_mode =   VRB2_VfHiMsixVectorMapperVf,
+   .pf_mode =   0,
+   .pmon_ctrl_a =   VRB2_VfPmACntrlRegVf,
+   .pmon_ctrl_b =   VRB2_VfPmBCntrlRegVf,
+   .pmon_ctrl_c =   VRB2_VfPmCCntrlRegVf,
+   .vf2pf_doorbell =VRB2_VfHiVfToPfDbellVf,
+   .pf2vf_doorbell =VRB2_VfHiPfToVfDbellVf,
+};
+
+
  #endif /* _VRB_PMD_H_ */


Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [PATCH v3 08/12] baseband/acc: add FEC capabilities for the VRB2 variant

[Maxime Coquelin]

On 9/29/23 18:35, Nicolas Chautru wrote:

New implementation for some of the FEC features
specific to the VRB2 variant.

Signed-off-by: Nicolas Chautru 
---
  drivers/baseband/acc/rte_vrb_pmd.c | 567 -
  1 file changed, 548 insertions(+), 19 deletions(-)

diff --git a/drivers/baseband/acc/rte_vrb_pmd.c 
b/drivers/baseband/acc/rte_vrb_pmd.c
index 48e779ce77..93add82947 100644
--- a/drivers/baseband/acc/rte_vrb_pmd.c
+++ b/drivers/baseband/acc/rte_vrb_pmd.c
@@ -1235,6 +1235,94 @@ vrb_dev_info_get(struct rte_bbdev *dev, struct 
rte_bbdev_driver_info *dev_info)
};
  
  	static const struct rte_bbdev_op_cap vrb2_bbdev_capabilities[] = {

+   {
+   .type = RTE_BBDEV_OP_TURBO_DEC,
+   .cap.turbo_dec = {
+   .capability_flags =
+   RTE_BBDEV_TURBO_SUBBLOCK_DEINTERLEAVE |
+   RTE_BBDEV_TURBO_CRC_TYPE_24B |
+   RTE_BBDEV_TURBO_DEC_CRC_24B_DROP |
+   RTE_BBDEV_TURBO_EQUALIZER |
+   RTE_BBDEV_TURBO_SOFT_OUT_SATURATE |
+   RTE_BBDEV_TURBO_HALF_ITERATION_EVEN |
+   RTE_BBDEV_TURBO_CONTINUE_CRC_MATCH |
+   RTE_BBDEV_TURBO_SOFT_OUTPUT |
+   RTE_BBDEV_TURBO_EARLY_TERMINATION |
+   RTE_BBDEV_TURBO_DEC_INTERRUPTS |
+   RTE_BBDEV_TURBO_NEG_LLR_1_BIT_IN |
+   RTE_BBDEV_TURBO_NEG_LLR_1_BIT_SOFT_OUT |
+   RTE_BBDEV_TURBO_MAP_DEC |
+   RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP |
+   RTE_BBDEV_TURBO_DEC_SCATTER_GATHER,
+   .max_llr_modulus = INT8_MAX,
+   .num_buffers_src =
+   RTE_BBDEV_TURBO_MAX_CODE_BLOCKS,
+   .num_buffers_hard_out =
+   RTE_BBDEV_TURBO_MAX_CODE_BLOCKS,
+   .num_buffers_soft_out =
+   RTE_BBDEV_TURBO_MAX_CODE_BLOCKS,
+   }
+   },
+   {
+   .type = RTE_BBDEV_OP_TURBO_ENC,
+   .cap.turbo_enc = {
+   .capability_flags =
+   RTE_BBDEV_TURBO_CRC_24B_ATTACH |
+   RTE_BBDEV_TURBO_RV_INDEX_BYPASS |
+   RTE_BBDEV_TURBO_RATE_MATCH |
+   RTE_BBDEV_TURBO_ENC_INTERRUPTS |
+   RTE_BBDEV_TURBO_ENC_SCATTER_GATHER,
+   .num_buffers_src =
+   RTE_BBDEV_TURBO_MAX_CODE_BLOCKS,
+   .num_buffers_dst =
+   RTE_BBDEV_TURBO_MAX_CODE_BLOCKS,
+   }
+   },
+   {
+   .type   = RTE_BBDEV_OP_LDPC_ENC,
+   .cap.ldpc_enc = {
+   .capability_flags =
+   RTE_BBDEV_LDPC_RATE_MATCH |
+   RTE_BBDEV_LDPC_CRC_24B_ATTACH |
+   RTE_BBDEV_LDPC_INTERLEAVER_BYPASS |
+   RTE_BBDEV_LDPC_ENC_INTERRUPTS |
+   RTE_BBDEV_LDPC_ENC_SCATTER_GATHER |
+   RTE_BBDEV_LDPC_ENC_CONCATENATION,
+   .num_buffers_src =
+   RTE_BBDEV_LDPC_MAX_CODE_BLOCKS,
+   .num_buffers_dst =
+   RTE_BBDEV_LDPC_MAX_CODE_BLOCKS,
+   }
+   },
+   {
+   .type   = RTE_BBDEV_OP_LDPC_DEC,
+   .cap.ldpc_dec = {
+   .capability_flags =
+   RTE_BBDEV_LDPC_CRC_TYPE_24B_CHECK |
+   RTE_BBDEV_LDPC_CRC_TYPE_24B_DROP |
+   RTE_BBDEV_LDPC_CRC_TYPE_24A_CHECK |
+   RTE_BBDEV_LDPC_CRC_TYPE_16_CHECK |
+   RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE |
+   RTE_BBDEV_LDPC_HQ_COMBINE_OUT_ENABLE |
+   RTE_BBDEV_LDPC_ITERATION_STOP_ENABLE |
+   RTE_BBDEV_LDPC_DEINTERLEAVER_BYPASS |
+   RTE_BBDEV_LDPC_

Re: [PATCH v3 09/12] baseband/acc: add FFT support to VRB2 variant

[Maxime Coquelin]

On 9/29/23 18:35, Nicolas Chautru wrote:

Support for the FFT the processing specific to the
VRB2 variant.

Signed-off-by: Nicolas Chautru 
---
  drivers/baseband/acc/rte_vrb_pmd.c | 132 -
  1 file changed, 128 insertions(+), 4 deletions(-)

diff --git a/drivers/baseband/acc/rte_vrb_pmd.c 
b/drivers/baseband/acc/rte_vrb_pmd.c
index 93add82947..ce4b90d8e7 100644
--- a/drivers/baseband/acc/rte_vrb_pmd.c
+++ b/drivers/baseband/acc/rte_vrb_pmd.c
@@ -903,6 +903,9 @@ vrb_queue_setup(struct rte_bbdev *dev, uint16_t queue_id,
ACC_FCW_LD_BLEN : (conf->op_type == RTE_BBDEV_OP_FFT ?
ACC_FCW_FFT_BLEN : ACC_FCW_MLDTS_BLEN;
  
+	if ((q->d->device_variant == VRB2_VARIANT) && (conf->op_type == RTE_BBDEV_OP_FFT))

+   fcw_len = ACC_FCW_FFT_BLEN_3;
+
for (desc_idx = 0; desc_idx < d->sw_ring_max_depth; desc_idx++) {
desc = q->ring_addr + desc_idx;
desc->req.word0 = ACC_DMA_DESC_TYPE;
@@ -1323,6 +1326,24 @@ vrb_dev_info_get(struct rte_bbdev *dev, struct 
rte_bbdev_driver_info *dev_info)
.num_buffers_soft_out = 0,
}
},
+   {
+   .type   = RTE_BBDEV_OP_FFT,
+   .cap.fft = {
+   .capability_flags =
+   RTE_BBDEV_FFT_WINDOWING |
+   RTE_BBDEV_FFT_CS_ADJUSTMENT |
+   RTE_BBDEV_FFT_DFT_BYPASS |
+   RTE_BBDEV_FFT_IDFT_BYPASS |
+   RTE_BBDEV_FFT_FP16_INPUT |
+   RTE_BBDEV_FFT_FP16_OUTPUT |
+   RTE_BBDEV_FFT_POWER_MEAS |
+   RTE_BBDEV_FFT_WINDOWING_BYPASS,
+   .num_buffers_src =
+   1,
+   .num_buffers_dst =
+   1,
+   }
+   },
RTE_BBDEV_END_OF_CAPABILITIES_LIST()
};
  
@@ -3849,6 +3870,47 @@ vrb1_fcw_fft_fill(struct rte_bbdev_fft_op *op, struct acc_fcw_fft *fcw)

fcw->bypass = 0;
  }
  
+/* Fill in a frame control word for FFT processing. */

+static inline void
+vrb2_fcw_fft_fill(struct rte_bbdev_fft_op *op, struct acc_fcw_fft_3 *fcw)
+{
+   fcw->in_frame_size = op->fft.input_sequence_size;
+   fcw->leading_pad_size = op->fft.input_leading_padding;
+   fcw->out_frame_size = op->fft.output_sequence_size;
+   fcw->leading_depad_size = op->fft.output_leading_depadding;
+   fcw->cs_window_sel = op->fft.window_index[0] +
+   (op->fft.window_index[1] << 8) +
+   (op->fft.window_index[2] << 16) +
+   (op->fft.window_index[3] << 24);
+   fcw->cs_window_sel2 = op->fft.window_index[4] +
+   (op->fft.window_index[5] << 8);
+   fcw->cs_enable_bmap = op->fft.cs_bitmap;
+   fcw->num_antennas = op->fft.num_antennas_log2;
+   fcw->idft_size = op->fft.idft_log2;
+   fcw->dft_size = op->fft.dft_log2;
+   fcw->cs_offset = op->fft.cs_time_adjustment;
+   fcw->idft_shift = op->fft.idft_shift;
+   fcw->dft_shift = op->fft.dft_shift;
+   fcw->cs_multiplier = op->fft.ncs_reciprocal;
+   fcw->power_shift = op->fft.power_shift; > +fcw->exp_adj = 
op->fft.fp16_exp_adjust;
+   fcw->fp16_in = check_bit(op->fft.op_flags, RTE_BBDEV_FFT_FP16_INPUT);
+   fcw->fp16_out = check_bit(op->fft.op_flags, RTE_BBDEV_FFT_FP16_OUTPUT);
+   fcw->power_en = check_bit(op->fft.op_flags, RTE_BBDEV_FFT_POWER_MEAS);
+   if (check_bit(op->fft.op_flags,
+   RTE_BBDEV_FFT_IDFT_BYPASS)) {
+   if (check_bit(op->fft.op_flags,
+   RTE_BBDEV_FFT_WINDOWING_BYPASS))
+   fcw->bypass = 2;
+   else
+   fcw->bypass = 1;
+   } else if (check_bit(op->fft.op_flags,
+   RTE_BBDEV_FFT_DFT_BYPASS))
+   fcw->bypass = 3;
+   else
+   fcw->bypass = 0;


The only difference I see with VRB1 are backed by corresponding op_flags
(POWER & FP16), is that correct? If so, it does not make sense to me to
have a specific fucntion for VRB2.


+}
+
  static inline int
  vrb1_dma_desc_fft_fill(struct rte_bbdev_fft_op *op,
struct acc_dma_req_desc *desc,
@@ -3882,6 +3944,58 @@ vrb1_dma_desc_fft_fill(struct rte_bbdev_fft_op *op,
return 0;
  }
  
+static inline int

+vrb2_dma_desc_fft_fill(struct rte_bbdev_fft_op *op,
+   struct acc_dma_req_desc *desc,
+   struct rte_mbuf *input, struct rte_mbuf *output, struct 
rte_mbuf *win

Re: [PATCH v3 10/12] baseband/acc: add MLD support in VRB2 variant

[Maxime Coquelin]

On 9/29/23 18:35, Nicolas Chautru wrote:

Adding the capability for the MLD-TS processing specific to
the VRB2 variant.

Signed-off-by: Nicolas Chautru 
---
  drivers/baseband/acc/rte_vrb_pmd.c | 378 +
  1 file changed, 378 insertions(+)

diff --git a/drivers/baseband/acc/rte_vrb_pmd.c 
b/drivers/baseband/acc/rte_vrb_pmd.c
index ce4b90d8e7..a9d3db86e6 100644
--- a/drivers/baseband/acc/rte_vrb_pmd.c
+++ b/drivers/baseband/acc/rte_vrb_pmd.c
@@ -1344,6 +1344,17 @@ vrb_dev_info_get(struct rte_bbdev *dev, struct 
rte_bbdev_driver_info *dev_info)
1,
}
},
+   {
+   .type   = RTE_BBDEV_OP_MLDTS,
+   .cap.mld = {
+   .capability_flags =
+   RTE_BBDEV_MLDTS_REP,
+   .num_buffers_src =
+   1,
+   .num_buffers_dst =
+   1,
+   }
+   },
RTE_BBDEV_END_OF_CAPABILITIES_LIST()
};
  
@@ -4151,6 +4162,371 @@ vrb_dequeue_fft(struct rte_bbdev_queue_data *q_data,

return i;
  }
  
+/* Fill in a frame control word for MLD-TS processing. */

+static inline void
+vrb2_fcw_mldts_fill(struct rte_bbdev_mldts_op *op, struct acc_fcw_mldts *fcw)
+{
+   fcw->nrb = op->mldts.num_rbs;
+   fcw->NLayers = op->mldts.num_layers - 1;
+   fcw->Qmod0 = (op->mldts.q_m[0] >> 1) - 1;
+   fcw->Qmod1 = (op->mldts.q_m[1] >> 1) - 1;
+   fcw->Qmod2 = (op->mldts.q_m[2] >> 1) - 1;
+   fcw->Qmod3 = (op->mldts.q_m[3] >> 1) - 1;
+   /* Mark some layers as disabled */
+   if (op->mldts.num_layers == 2) {
+   fcw->Qmod2 = 3;
+   fcw->Qmod3 = 3;
+   }
+   if (op->mldts.num_layers == 3)
+   fcw->Qmod3 = 3;
+   fcw->Rrep = op->mldts.r_rep;
+   fcw->Crep = op->mldts.c_rep;
+}
+
+/* Fill in descriptor for one MLD-TS processing operation. */
+static inline int
+vrb2_dma_desc_mldts_fill(struct rte_bbdev_mldts_op *op,
+   struct acc_dma_req_desc *desc,
+   struct rte_mbuf *input_q, struct rte_mbuf *input_r,
+   struct rte_mbuf *output,
+   uint32_t *in_offset, uint32_t *out_offset)
+{
+   uint16_t qsize_per_re[VRB2_MLD_LAY_SIZE] = {8, 12, 16}; /* Layer 2 to 
4. */
+   uint16_t rsize_per_re[VRB2_MLD_LAY_SIZE] = {14, 26, 42};
+   uint16_t sc_factor_per_rrep[VRB2_MLD_RREP_SIZE] = {12, 6, 4, 3, 0, 2};
+   uint16_t i, outsize_per_re = 0;
+   uint32_t sc_num, r_num, q_size, r_size, out_size;
+
+   /* Prevent out of range access. */
+   if (op->mldts.r_rep > 5)
+   op->mldts.r_rep = 5;
+   if (op->mldts.num_layers < 2)
+   op->mldts.num_layers = 2;
+   if (op->mldts.num_layers > 4)
+   op->mldts.num_layers = 4;
+   for (i = 0; i < op->mldts.num_layers; i++)
+   outsize_per_re += op->mldts.q_m[i];
+   sc_num = op->mldts.num_rbs * RTE_BBDEV_SCPERRB * (op->mldts.c_rep + 1);
+   r_num = op->mldts.num_rbs * sc_factor_per_rrep[op->mldts.r_rep];
+   q_size = qsize_per_re[op->mldts.num_layers - 2] * sc_num;
+   r_size = rsize_per_re[op->mldts.num_layers - 2] * r_num;
+   out_size =  sc_num * outsize_per_re;
+   /* printf("Sc %d R num %d Size %d %d %d\n", sc_num, r_num, q_size, 
r_size, out_size); */


rte_bbdev_log_debug()? Otherwise just remove it.


+
+   /* FCW already done. */
+   acc_header_init(desc);
+   desc->data_ptrs[1].address = rte_pktmbuf_iova_offset(input_q, 
*in_offset);
+   desc->data_ptrs[1].blen = q_size;
+   desc->data_ptrs[1].blkid = ACC_DMA_BLKID_IN;
+   desc->data_ptrs[1].last = 0;
+   desc->data_ptrs[1].dma_ext = 0;
+   desc->data_ptrs[2].address = rte_pktmbuf_iova_offset(input_r, 
*in_offset);
+   desc->data_ptrs[2].blen = r_size;
+   desc->data_ptrs[2].blkid = ACC_DMA_BLKID_IN_MLD_R;
+   desc->data_ptrs[2].last = 1;
+   desc->data_ptrs[2].dma_ext = 0;
+   desc->data_ptrs[3].address = rte_pktmbuf_iova_offset(output, 
*out_offset);
+   desc->data_ptrs[3].blen = out_size;
+   desc->data_ptrs[3].blkid = ACC_DMA_BLKID_OUT_HARD;
+   desc->data_ptrs[3].last = 1;
+   desc->data_ptrs[3].dma_ext = 0;
+   desc->m2dlen = 3;
+   desc->d2mlen = 1;
+   desc->op_addr = op;
+   desc->cbs_in_tb = 1;
+
+   return 0;
+}
+
+/* Check whether the MLD operation can be processed as a single operation. */
+static inline bool
+vrb2_check_mld_r_constraint(struct rte_bbdev_mldts_op *op) {
+   uint8_t layer_idx, rrep_idx;
+   uint16_t max_rb[VRB2_MLD_LAY_SIZE][VRB2_MLD_RREP_SIZE] = {
+   {188, 275, 275, 275, 0, 275},
+   {101, 202, 275, 275, 0, 275},
+ 

Re: [PATCH v3 11/12] baseband/acc: add support for VRB2 engine error detection

[Maxime Coquelin]

_count++;
  
@@ -3650,6 +3654,7 @@ vrb_dequeue_dec_one_op_tb(struct acc_queue *q, struct rte_bbdev_dec_op **ref_op,

op->status |= ((rsp.input_err) ? (1 << RTE_BBDEV_DATA_ERROR) : 
0);
op->status |= ((rsp.dma_err) ? (1 << RTE_BBDEV_DRV_ERROR) : 0);
op->status |= ((rsp.fcw_err) ? (1 << RTE_BBDEV_DRV_ERROR) : 0);
+   op->status |= ((rsp.engine_hung) ? (1 << 
RTE_BBDEV_ENGINE_ERROR) : 0);


It kinf of highlights the need for refactoring I suggested in previous
patch! It would have been done in one place.

  
  		if (check_bit(op->ldpc_dec.op_flags, RTE_BBDEV_LDPC_CRC_TYPE_24A_CHECK))

tb_crc_check ^= desc->rsp.add_info_1;
@@ -3701,7 +3706,6 @@ vrb_dequeue_enc(struct rte_bbdev_queue_data *q_data,
if (avail == 0)
return 0;
op = acc_op_tail(q, 0);
-
cbm = op->turbo_enc.code_block_mode;
  
  	for (i = 0; i < avail; i++) {

@@ -4041,9 +4045,8 @@ vrb_enqueue_fft_one_op(struct acc_queue *q, struct 
rte_bbdev_fft_op *op,
&in_offset, &out_offset, &win_offset, 
&pwr_offset);
}
  #ifdef RTE_LIBRTE_BBDEV_DEBUG
-   rte_memdump(stderr, "FCW", &desc->req.fcw_fft,
-   sizeof(desc->req.fcw_fft));
-   rte_memdump(stderr, "Req Desc.", desc, sizeof(*desc));
+   rte_memdump(stderr, "FCW", fcw, 128);
+   rte_memdump(stderr, "Req Desc.", desc, 128);
  #endif
return 1;
  }
@@ -4116,6 +4119,7 @@ vrb_dequeue_fft_one_op(struct rte_bbdev_queue_data 
*q_data,
op->status |= rsp.input_err << RTE_BBDEV_DATA_ERROR;
op->status |= rsp.dma_err << RTE_BBDEV_DRV_ERROR;
op->status |= rsp.fcw_err << RTE_BBDEV_DRV_ERROR;
+   op->status |= rsp.engine_hung << RTE_BBDEV_ENGINE_ERROR;
if (op->status != 0)
q_data->queue_stats.dequeue_err_count++;
  
diff --git a/drivers/baseband/acc/vrb1_pf_enum.h b/drivers/baseband/acc/vrb1_pf_enum.h

index 82a36685e9..6dc359800f 100644
--- a/drivers/baseband/acc/vrb1_pf_enum.h
+++ b/drivers/baseband/acc/vrb1_pf_enum.h
@@ -98,11 +98,18 @@ enum {
ACC_PF_INT_DMA_UL5G_DESC_IRQ = 8,
ACC_PF_INT_DMA_DL5G_DESC_IRQ = 9,
ACC_PF_INT_DMA_MLD_DESC_IRQ = 10,
-   ACC_PF_INT_ARAM_ECC_1BIT_ERR = 11,
-   ACC_PF_INT_PARITY_ERR = 12,
-   ACC_PF_INT_QMGR_ERR = 13,
-   ACC_PF_INT_INT_REQ_OVERFLOW = 14,
-   ACC_PF_INT_APB_TIMEOUT = 15,
+   ACC_PF_INT_ARAM_ACCESS_ERR = 11,
+   ACC_PF_INT_ARAM_ECC_1BIT_ERR = 12,
+   ACC_PF_INT_PARITY_ERR = 13,
+   ACC_PF_INT_QMGR_OVERFLOW = 14,
+   ACC_PF_INT_QMGR_ERR = 15,
+   ACC_PF_INT_ATS_ERR = 22,
+   ACC_PF_INT_ARAM_FUUL = 23,
+   ACC_PF_INT_EXTRA_READ = 24,
+   ACC_PF_INT_COMPLETION_TIMEOUT = 25,
+   ACC_PF_INT_CORE_HANG = 26,
+   ACC_PF_INT_DMA_HANG = 28,
+   ACC_PF_INT_DS_HANG = 27,
  };
  
  #endif /* VRB1_PF_ENUM_H */



Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [PATCH v3 12/12] baseband/acc: add configure helper for VRB2

[Maxime Coquelin]

lue = 0;
+   acc_reg_write(d, address, value);
+   /* Set the PF Mode register */
+   address = VRB2_PfHiPfMode;
+   value = ((conf->pf_mode_en) ? ACC_PF_VAL : 0) | 0x1F07F0;
+   acc_reg_write(d, address, value);
+   /* Explicitly releasing AXI after PF Mode. */
+   acc_reg_write(d, VRB2_PfDmaAxiControl, 1);
+
+   /* QoS overflow init. */
+   value = 1;
+   address = VRB2_PfQosmonAEvalOverflow0;
+   acc_reg_write(d, address, value);
+   address = VRB2_PfQosmonBEvalOverflow0;
+   acc_reg_write(d, address, value);
+
+   /* Enabling AQueues through the Queue hierarchy. */
+   unsigned int  en_bitmask[VRB2_AQ_REG_NUM];
+   for (vf_idx = 0; vf_idx < VRB2_NUM_VFS; vf_idx++) {
+   for (qg_idx = 0; qg_idx < VRB2_NUM_QGRPS; qg_idx++) {
+   for (aq_reg = 0;  aq_reg < VRB2_AQ_REG_NUM; aq_reg++)
+   en_bitmask[aq_reg] = 0;
+   if (vf_idx < conf->num_vf_bundles && qg_idx < totalQgs) 
{
+   for (aq_reg = 0;  aq_reg < VRB2_AQ_REG_NUM; 
aq_reg++) {
+   if (aqNum(qg_idx, conf) >= 16 * (aq_reg 
+ 1))
+   en_bitmask[aq_reg] = 0x;
+   else if (aqNum(qg_idx, conf) <= 16 * 
aq_reg)
+   en_bitmask[aq_reg] = 0x0;
+   else
+   en_bitmask[aq_reg] = (1 << 
(aqNum(qg_idx,
+   conf) - aq_reg 
* 16)) - 1;
+   }
+   }
+   for (aq_reg = 0; aq_reg < VRB2_AQ_REG_NUM; aq_reg++) {
+   address = VRB2_PfQmgrAqEnableVf + vf_idx * 16 + 
aq_reg * 4;
+   value = (qg_idx << 16) + en_bitmask[aq_reg];
+   acc_reg_fast_write(d, address, value);
+   }
+   }
+   }
+
+   rte_bbdev_log(INFO,
+   "VRB2 basic config complete for %s - pf_bb_config should 
ideally be used instead",
+   dev_name);
+   return 0;
+}
diff --git a/drivers/baseband/acc/vrb_cfg.h b/drivers/baseband/acc/vrb_cfg.h
index e3c8902b46..79487c4e04 100644
--- a/drivers/baseband/acc/vrb_cfg.h
+++ b/drivers/baseband/acc/vrb_cfg.h
@@ -29,4 +29,20 @@
  int
  vrb1_configure(const char *dev_name, struct rte_acc_conf *conf);
  
+/**

+ * Configure a VRB2 device.
+ *
+ * @param dev_name
+ *   The name of the device. This is the short form of PCI BDF, e.g. 00:01.0.
+ *   It can also be retrieved for a bbdev device from the dev_name field in the
+ *   rte_bbdev_info structure returned by rte_bbdev_info_get().
+ * @param conf
+ *   Configuration to apply to VRB2 HW.
+ *
+ * @return
+ *   Zero on success, negative value on failure.
+ */
+int
+vrb2_configure(const char *dev_name, struct rte_acc_conf *conf);
+
  #endif /* _VRB_CFG_H_ */


Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [PATCH v1 1/1] baseband/acc: fix ACC100 HARQ input is alignment

[Maxime Coquelin]

On 9/19/23 20:24, Hernan Vargas wrote:

Some constraints are imposed onto the ACC100 HARQ input size,
but that value is incorrectly aligned down when getting close to
max (Ncb-F) which is not required.
The wireless performance impact is negligeable but still causes a
few LLRs no to be combined at the very end of the circular buffer.

Fixes: 5802f36dd492 ("baseband/acc100: enforce additional check on FCW")
Cc: sta...@dpdk.org

Signed-off-by: Hernan Vargas 
---
  drivers/baseband/acc/rte_acc100_pmd.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c 
b/drivers/baseband/acc/rte_acc100_pmd.c
index 5362d39c302f..c736f3e4201c 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -1218,7 +1218,7 @@ acc100_fcw_ld_fill(struct rte_bbdev_dec_op *op, struct 
acc_fcw_ld *fcw,
- op->ldpc_dec.n_filler);
  
  		/* Alignment on next 64B - Already enforced from HC output */

-   harq_in_length = RTE_ALIGN_FLOOR(harq_in_length, 
ACC_HARQ_ALIGN_64B);
+   harq_in_length = RTE_ALIGN_CEIL(harq_in_length, 
ACC_HARQ_ALIGN_64B);
  
  		/* Stronger alignment requirement when in decompression mode */

if (fcw->hcin_decomp_mode > 0)


Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [PATCH v3 11/12] baseband/acc: add support for VRB2 engine error detection

[Maxime Coquelin]

On 10/3/23 19:22, Chautru, Nicolas wrote:

Hi Maxime,


-Original Message-
From: Maxime Coquelin 
Sent: Tuesday, October 3, 2023 8:16 AM
To: Chautru, Nicolas ; dev@dpdk.org
Cc: hemant.agra...@nxp.com; david.march...@redhat.com; Vargas, Hernan

Subject: Re: [PATCH v3 11/12] baseband/acc: add support for VRB2 engine
error detection



On 9/29/23 18:35, Nicolas Chautru wrote:

Adding missing incremental functionality for the VRB2 variant. Notably
detection of engine error during the dequeue. Minor cosmetic edits.

Signed-off-by: Nicolas Chautru 
---
   drivers/baseband/acc/rte_vrb_pmd.c  | 20 
   drivers/baseband/acc/vrb1_pf_enum.h | 17 -
   2 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/drivers/baseband/acc/rte_vrb_pmd.c
b/drivers/baseband/acc/rte_vrb_pmd.c
index a9d3db86e6..3eb1a380fc 100644
--- a/drivers/baseband/acc/rte_vrb_pmd.c
+++ b/drivers/baseband/acc/rte_vrb_pmd.c
@@ -1504,6 +1504,7 @@ vrb_fcw_td_fill(const struct rte_bbdev_dec_op

*op, struct acc_fcw_td *fcw)

fcw->ea = op->turbo_dec.cb_params.e;
fcw->eb = op->turbo_dec.cb_params.e;
}
+
if (op->turbo_dec.rv_index == 0)
fcw->k0_start_col = ACC_FCW_TD_RVIDX_0;
else if (op->turbo_dec.rv_index == 1) @@ -2304,7

+2305,7 @@

enqueue_ldpc_enc_n_op_cb(struct acc_queue *q, struct rte_bbdev_enc_op

**ops,

return num;
   }

-/* Enqueue one encode operations for device for a partial TB
+/* Enqueue one encode operations for VRB1 device for a partial TB
* all codes blocks have same configuration multiplexed on the same

descriptor.

*/
   static inline void
@@ -2649,7 +2650,7 @@ enqueue_dec_one_op_cb(struct acc_queue *q,

struct rte_bbdev_dec_op *op,

return 1;
   }

-/** Enqueue one decode operations for device in CB mode */
+/** Enqueue one decode operations for device in CB mode. */
   static inline int
   vrb_enqueue_ldpc_dec_one_op_cb(struct acc_queue *q, struct

rte_bbdev_dec_op *op,

uint16_t total_enqueued_cbs, bool same_op) @@ -2801,7

+2802,6 @@

vrb_enqueue_ldpc_dec_one_op_tb(struct acc_queue *q, struct

rte_bbdev_dec_op *op,

desc->req.data_ptrs[0].blen = ACC_FCW_LD_BLEN;
rte_memcpy(&desc->req.fcw_ld, &desc_first->req.fcw_ld,

ACC_FCW_LD_BLEN);

desc->req.fcw_ld.tb_trailer_size = (c - r - 1) * trail_len;
-
if (q->d->device_variant == VRB1_VARIANT)
ret = vrb1_dma_desc_ld_fill(op, &desc->req, &input,
h_output, &in_offset, &h_out_offset,

@@ -3226,7 +3226,6 @@

vrb_enqueue_ldpc_dec_cb(struct rte_bbdev_queue_data *q_data,
break;
}
avail -= 1;
-


Is it intentionnally removed?


Cosmetic but slightly more readable. I don’t have a strong rule for these.


OK, if that's intentionnal that's OK to me.




rte_bbdev_log(INFO, "Op %d %d %d %d %d %d %d %d %d %d

%d %d\n",

i, ops[i]->ldpc_dec.op_flags, ops[i]-
ldpc_dec.rv_index,
ops[i]->ldpc_dec.iter_max, ops[i]-
ldpc_dec.iter_count, @@
-3354,6 +3353,7 @@ vrb_dequeue_enc_one_op_cb(struct acc_queue *q,

struct rte_bbdev_enc_op **ref_op,

op->status |= ((rsp.input_err) ? (1 << RTE_BBDEV_DATA_ERROR) : 0);
op->status |= ((rsp.dma_err) ? (1 << RTE_BBDEV_DRV_ERROR) : 0);
op->status |= ((rsp.fcw_err) ? (1 << RTE_BBDEV_DRV_ERROR) : 0);
+   op->status |= ((rsp.engine_hung) ? (1 << RTE_BBDEV_ENGINE_ERROR)

:

+0);

if (desc->req.last_desc_in_batch) {
(*aq_dequeued)++;
@@ -3470,6 +3470,7 @@ vrb_dequeue_enc_one_op_tb(struct acc_queue

*q, struct rte_bbdev_enc_op **ref_op,

op->status |= ((rsp.input_err) ? (1 <<

RTE_BBDEV_DATA_ERROR) : 0);

op->status |= ((rsp.dma_err) ? (1 << RTE_BBDEV_DRV_ERROR)

: 0);

op->status |= ((rsp.fcw_err) ? (1 << RTE_BBDEV_DRV_ERROR) :

0);

+   op->status |= ((rsp.engine_hung) ? (1 <<

RTE_BBDEV_ENGINE_ERROR) :

+0);

if (desc->req.last_desc_in_batch) {
(*aq_dequeued)++;
@@ -3516,6 +3517,8 @@ vrb_dequeue_dec_one_op_cb(struct

rte_bbdev_queue_data *q_data,

op->status |= ((rsp.input_err) ? (1 << RTE_BBDEV_DATA_ERROR) : 0);
op->status |= ((rsp.dma_err) ? (1 << RTE_BBDEV_DRV_ERROR) : 0);
op->status |= ((rsp.fcw_err) ? (1 << RTE_BBDEV_DRV_ERROR) : 0);
+   op->status |= rsp.engine_hung << RTE_BBDEV_ENGINE_ERROR;
+
if (op->status != 0) {
/* These errors are not expected. */

Re: [PATCH v3 09/12] baseband/acc: add FFT support to VRB2 variant

[Maxime Coquelin]

On 10/3/23 20:20, Chautru, Nicolas wrote:

Hi Maxime,


-Original Message-
From: Maxime Coquelin 
Sent: Tuesday, October 3, 2023 7:37 AM
To: Chautru, Nicolas ; dev@dpdk.org
Cc: hemant.agra...@nxp.com; david.march...@redhat.com; Vargas, Hernan

Subject: Re: [PATCH v3 09/12] baseband/acc: add FFT support to VRB2 variant



On 9/29/23 18:35, Nicolas Chautru wrote:

Support for the FFT the processing specific to the
VRB2 variant.

Signed-off-by: Nicolas Chautru 
---
   drivers/baseband/acc/rte_vrb_pmd.c | 132

-

   1 file changed, 128 insertions(+), 4 deletions(-)

diff --git a/drivers/baseband/acc/rte_vrb_pmd.c
b/drivers/baseband/acc/rte_vrb_pmd.c
index 93add82947..ce4b90d8e7 100644
--- a/drivers/baseband/acc/rte_vrb_pmd.c
+++ b/drivers/baseband/acc/rte_vrb_pmd.c
@@ -903,6 +903,9 @@ vrb_queue_setup(struct rte_bbdev *dev, uint16_t

queue_id,

ACC_FCW_LD_BLEN : (conf->op_type ==

RTE_BBDEV_OP_FFT ?

ACC_FCW_FFT_BLEN : ACC_FCW_MLDTS_BLEN;

+   if ((q->d->device_variant == VRB2_VARIANT) && (conf->op_type ==

RTE_BBDEV_OP_FFT))

+   fcw_len = ACC_FCW_FFT_BLEN_3;
+
for (desc_idx = 0; desc_idx < d->sw_ring_max_depth; desc_idx++) {
desc = q->ring_addr + desc_idx;
desc->req.word0 = ACC_DMA_DESC_TYPE; @@ -1323,6

+1326,24 @@

vrb_dev_info_get(struct rte_bbdev *dev, struct rte_bbdev_driver_info

*dev_info)

.num_buffers_soft_out = 0,
}
},
+   {
+   .type   = RTE_BBDEV_OP_FFT,
+   .cap.fft = {
+   .capability_flags =
+

RTE_BBDEV_FFT_WINDOWING |

+

RTE_BBDEV_FFT_CS_ADJUSTMENT |

+

RTE_BBDEV_FFT_DFT_BYPASS |

+

RTE_BBDEV_FFT_IDFT_BYPASS |

+   RTE_BBDEV_FFT_FP16_INPUT

|

+

RTE_BBDEV_FFT_FP16_OUTPUT |

+

RTE_BBDEV_FFT_POWER_MEAS |

+

RTE_BBDEV_FFT_WINDOWING_BYPASS,

+   .num_buffers_src =
+   1,
+   .num_buffers_dst =
+   1,
+   }
+   },
RTE_BBDEV_END_OF_CAPABILITIES_LIST()
};

@@ -3849,6 +3870,47 @@ vrb1_fcw_fft_fill(struct rte_bbdev_fft_op *op,

struct acc_fcw_fft *fcw)

fcw->bypass = 0;
   }

+/* Fill in a frame control word for FFT processing. */ static inline
+void vrb2_fcw_fft_fill(struct rte_bbdev_fft_op *op, struct
+acc_fcw_fft_3 *fcw) {
+   fcw->in_frame_size = op->fft.input_sequence_size;
+   fcw->leading_pad_size = op->fft.input_leading_padding;
+   fcw->out_frame_size = op->fft.output_sequence_size;
+   fcw->leading_depad_size = op->fft.output_leading_depadding;
+   fcw->cs_window_sel = op->fft.window_index[0] +
+   (op->fft.window_index[1] << 8) +
+   (op->fft.window_index[2] << 16) +
+   (op->fft.window_index[3] << 24);
+   fcw->cs_window_sel2 = op->fft.window_index[4] +
+   (op->fft.window_index[5] << 8);
+   fcw->cs_enable_bmap = op->fft.cs_bitmap;
+   fcw->num_antennas = op->fft.num_antennas_log2;
+   fcw->idft_size = op->fft.idft_log2;
+   fcw->dft_size = op->fft.dft_log2;
+   fcw->cs_offset = op->fft.cs_time_adjustment;
+   fcw->idft_shift = op->fft.idft_shift;
+   fcw->dft_shift = op->fft.dft_shift;
+   fcw->cs_multiplier = op->fft.ncs_reciprocal;
+   fcw->power_shift = op->fft.power_shift; > +fcw->exp_adj = op-
fft.fp16_exp_adjust;
+   fcw->fp16_in = check_bit(op->fft.op_flags,

RTE_BBDEV_FFT_FP16_INPUT);

+   fcw->fp16_out = check_bit(op->fft.op_flags,

RTE_BBDEV_FFT_FP16_OUTPUT);

+   fcw->power_en = check_bit(op->fft.op_flags,

RTE_BBDEV_FFT_POWER_MEAS);

+   if (check_bit(op->fft.op_flags,
+   RTE_BBDEV_FFT_IDFT_BYPASS)) {
+   if (check_bit(op->fft.op_flags,
+   RTE_BBDEV_FFT_WINDOWING_BYPASS))
+   fcw->bypass = 2;
+   else
+   fcw->bypass = 1;
+   } else if (check_bit(op->fft.op_flags,
+   RTE_BBDEV_FFT_DFT_BYPASS))
+   fcw->bypass = 3;
+   else
+   fcw->bypass = 0;


The only difference I see with VRB1 are backed by corresponding op_flags
(POWER & FP16), is that correct? If so, it does not make sense to me to have a
specific function for VRB2.


There are more changes but these are only formally enabled in the next stepping 
hence some of the
related code

Re: [PATCH v3 06/12] baseband/acc: refactor to allow unified driver extension

[Maxime Coquelin]

On 10/3/23 20:54, Chautru, Nicolas wrote:

Hi Maxime,


-Original Message-
From: Maxime Coquelin 
Sent: Tuesday, October 3, 2023 6:15 AM
To: Chautru, Nicolas ; dev@dpdk.org
Cc: hemant.agra...@nxp.com; david.march...@redhat.com; Vargas, Hernan

Subject: Re: [PATCH v3 06/12] baseband/acc: refactor to allow unified driver
extension

Thanks for doing the split, that will ease review.

On 9/29/23 18:35, Nicolas Chautru wrote:

Adding a few functions and common code prior to extending the VRB
driver.

Signed-off-by: Nicolas Chautru 
---
   drivers/baseband/acc/acc_common.h | 164 +++-

--

   drivers/baseband/acc/rte_acc100_pmd.c |   4 +-
   drivers/baseband/acc/rte_vrb_pmd.c|  62 +-
   3 files changed, 184 insertions(+), 46 deletions(-)

diff --git a/drivers/baseband/acc/acc_common.h
b/drivers/baseband/acc/acc_common.h
index 788abf1a3c..89893eae43 100644
--- a/drivers/baseband/acc/acc_common.h
+++ b/drivers/baseband/acc/acc_common.h
@@ -18,6 +18,7 @@
   #define ACC_DMA_BLKID_OUT_HARQ  3
   #define ACC_DMA_BLKID_IN_HARQ   3
   #define ACC_DMA_BLKID_IN_MLD_R  3
+#define ACC_DMA_BLKID_DEWIN_IN  3

   /* Values used in filling in decode FCWs */
   #define ACC_FCW_TD_VER  1
@@ -103,6 +104,9 @@
   #define ACC_MAX_NUM_QGRPS  32
   #define ACC_RING_SIZE_GRANULARITY  64
   #define ACC_MAX_FCW_SIZE  128
+#define ACC_IQ_SIZE4
+
+#define ACC_FCW_FFT_BLEN_3 28

   /* Constants from K0 computation from 3GPP 38.212 Table 5.4.2.1-2 */
   #define ACC_N_ZC_1 66 /* N = 66 Zc for BG 1 */ @@ -132,6 +136,17 @@
   #define ACC_LIM_21 14 /* 0.21 */
   #define ACC_LIM_31 20 /* 0.31 */
   #define ACC_MAX_E (128 * 1024 - 2)
+#define ACC_MAX_CS 12
+
+#define ACC100_VARIANT  0
+#define VRB1_VARIANT   2
+#define VRB2_VARIANT   3
+
+/* Queue Index Hierarchy */
+#define VRB1_GRP_ID_SHIFT10
+#define VRB1_VF_ID_SHIFT 4
+#define VRB2_GRP_ID_SHIFT12
+#define VRB2_VF_ID_SHIFT 6

   /* Helper macro for logging */
   #define rte_acc_log(level, fmt, ...) \ @@ -332,6 +347,37 @@ struct
__rte_packed acc_fcw_fft {
res:19;
   };

+/* FFT Frame Control Word. */
+struct __rte_packed acc_fcw_fft_3 {
+   uint32_t in_frame_size:16,
+   leading_pad_size:16;
+   uint32_t out_frame_size:16,
+   leading_depad_size:16;
+   uint32_t cs_window_sel;
+   uint32_t cs_window_sel2:16,
+   cs_enable_bmap:16;
+   uint32_t num_antennas:8,
+   idft_size:8,
+   dft_size:8,
+   cs_offset:8;
+   uint32_t idft_shift:8,
+   dft_shift:8,
+   cs_multiplier:16;
+   uint32_t bypass:2,
+   fp16_in:1,
+   fp16_out:1,
+   exp_adj:4,
+   power_shift:4,
+   power_en:1,
+   enable_dewin:1,
+   freq_resample_mode:2,
+   depad_output_size:16;
+   uint16_t cs_theta_0[ACC_MAX_CS];
+   uint32_t cs_theta_d[ACC_MAX_CS];
+   int8_t cs_time_offset[ACC_MAX_CS];
+};
+
+
   /* MLD-TS Frame Control Word */
   struct __rte_packed acc_fcw_mldts {
uint32_t fcw_version:4,
@@ -473,14 +519,14 @@ union acc_info_ring_data {
uint16_t valid: 1;
};
struct {
-   uint32_t aq_id_3: 6;
-   uint32_t qg_id_3: 5;
-   uint32_t vf_id_3: 6;
-   uint32_t int_nb_3: 6;
-   uint32_t msi_0_3: 1;
-   uint32_t vf2pf_3: 6;
-   uint32_t loop_3: 1;
-   uint32_t valid_3: 1;
+   uint32_t aq_id_vrb2: 6;
+   uint32_t qg_id_vrb2: 5;
+   uint32_t vf_id_vrb2: 6;
+   uint32_t int_nb_vrb2: 6;
+   uint32_t msi_0_vrb2: 1;
+   uint32_t vf2pf_vrb2: 6;
+   uint32_t loop_vrb2: 1;
+   uint32_t valid_vrb2: 1;
};
   } __rte_packed;

@@ -761,22 +807,105 @@ alloc_sw_rings_min_mem(struct rte_bbdev *dev,

struct acc_device *d,

free_base_addresses(base_addrs, i);
   }

+/* Wrapper to provide VF index from ring data. */ static inline
+uint16_t vf_from_ring(const union acc_info_ring_data ring_data,
+uint16_t device_variant) {


curly braces on a new line.


Thanks.




+   if (device_variant == VRB2_VARIANT)
+   return ring_data.vf_id_vrb2;
+   else
+   return ring_data.vf_id;
+}
+
+/* Wrapper to provide QG index from ring data. */ static inline
+uint16_t qg_from_ring(const union acc_info_ring_data ring_data,
+uint16_t device_variant) {
+   if (device_variant == VRB2_VARIANT)
+   return ring_data.qg_id_vrb2;
+   else
+   return ring_data.qg_id;
+}
+
+/* Wrapper to provide AQ index from ring data. */ static inline
+uint16_t aq_from_ring(const union acc_info_ring_data ring_data,
+uint16_t device_variant) {
+   if (device_variant

Re: [PATCH v3 02/12] baseband/acc: add FFT window width in the VRB PMD

[Maxime Coquelin]

On 10/3/23 21:06, Chautru, Nicolas wrote:

Hi Maxime,


-Original Message-
From: Maxime Coquelin 
Sent: Tuesday, October 3, 2023 4:52 AM
To: Chautru, Nicolas ; dev@dpdk.org
Cc: hemant.agra...@nxp.com; david.march...@redhat.com; Vargas, Hernan

Subject: Re: [PATCH v3 02/12] baseband/acc: add FFT window width in the
VRB PMD



On 9/29/23 18:35, Nicolas Chautru wrote:

This allows to expose the FFT window width being introduced in
previous commit based on what is configured dynamically on the device
platform.

Signed-off-by: Nicolas Chautru 
---
   drivers/baseband/acc/acc_common.h  |  3 +++
   drivers/baseband/acc/rte_vrb_pmd.c | 29

+

   2 files changed, 32 insertions(+)

diff --git a/drivers/baseband/acc/acc_common.h
b/drivers/baseband/acc/acc_common.h
index 5bb00746c3..7d24c644c0 100644
--- a/drivers/baseband/acc/acc_common.h
+++ b/drivers/baseband/acc/acc_common.h
@@ -512,6 +512,8 @@ struct acc_deq_intr_details {
   enum {
ACC_VF2PF_STATUS_REQUEST = 1,
ACC_VF2PF_USING_VF = 2,
+   ACC_VF2PF_LUT_VER_REQUEST = 3,
+   ACC_VF2PF_FFT_WIN_REQUEST = 4,
   };


@@ -558,6 +560,7 @@ struct acc_device {
queue_offset_fun_t queue_offset;  /* Device specific queue offset */
uint16_t num_qgroups;
uint16_t num_aqs;
+   uint16_t fft_window_width[RTE_BBDEV_MAX_FFT_WIN]; /* FFT

windowing

+width. */
   };

   /* Structure associated with each queue. */ diff --git
a/drivers/baseband/acc/rte_vrb_pmd.c
b/drivers/baseband/acc/rte_vrb_pmd.c
index 9e5a73c9c7..c5a74bae11 100644
--- a/drivers/baseband/acc/rte_vrb_pmd.c
+++ b/drivers/baseband/acc/rte_vrb_pmd.c
@@ -298,6 +298,34 @@ vrb_device_status(struct rte_bbdev *dev)
return reg;
   }

+/* Request device FFT windowing information. */ static inline void
+vrb_device_fft_win(struct rte_bbdev *dev, struct
+rte_bbdev_driver_info *dev_info) {
+   struct acc_device *d = dev->data->dev_private;
+   uint32_t reg, time_out = 0, win;
+
+   if (d->pf_device)
+   return;
+
+   /* Check from the device the first time. */
+   if (d->fft_window_width[0] == 0) {


O is not a possible value? Might not be a big deal as it would just perform
reading of 16 x 2 registers every time .info_get() is called, just wondering.


This is impossible for this to be null. It would mean forcing a zero output all 
the time. Cannot happen fundamentally.


Ack.






+   for (win = 0; win < RTE_BBDEV_MAX_FFT_WIN; win++) {
+   vrb_vf2pf(d, ACC_VF2PF_FFT_WIN_REQUEST | win);


That looks broken, as extending RTE_BBDEV_MAX_FFT_WIN to support other
devices may break this implementation.


I don’t believe so. 16 windows shapes is a fairly large, this already takes a 
lot of memory to store all this.


Maybe, but the issue here is you rely on some generic BBDEV defines as
an offset to access HW registers in your device, that's wrong IMO as the
define may evolve in the future. At least you should define what is the
maximum FFT windows for your device, a use the minimum value between the
two.

But the suggestion you make below is better





To me, it tends to show how this fft_window_width array is more device
specific than generic.


I don't see why you say this really. This is fundamentally what windowing is. 
This is a given section of the FFT output where you apply a point-wise 
multiplication based on a given window shape, hence the size is scaled up and 
down based on the FFT size.
This width information is required to estimate about much noise to remove by 
applying such windowing, hence this is enumerated during device enumeration.
Then the number of windows available is a discrete numbers as mentioned above 
based on how many of these are programmed on the device (these needs to be 
stored in HW memory).

Would you prefer to expose an additional parameter for the number of windows in 
the capability (ie. size of that array) then a pointer for the actual array? 
That is okay with me and probably better. Please kindly confirm.
Also Herman feel free to chime in.

Ie.
{
.type   = RTE_BBDEV_OP_FFT,
.cap.fft = {
.capability_flags = (...),
.num_windows = 16,
}
},


That would be better IMO.




+   reg = acc_reg_read(d, d->reg_addr->pf2vf_doorbell);
+   while ((time_out < ACC_STATUS_TO) && (reg ==

RTE_BBDEV_DEV_NOSTATUS)) {

+   usleep(ACC_STATUS_WAIT); /*< Wait or VF-
PF->VF Comms */
+   reg = acc_reg_read(d, d->reg_addr-
pf2vf_doorbell);
+   time_out++;
+   }
+   d->fft_window_width[win] = reg;
+   }
+   }
+
+   for (win = 0; win < RTE_BBDEV_MAX_FFT_W

Re: [PATCH v3 06/12] baseband/acc: refactor to allow unified driver extension

[Maxime Coquelin]

On 10/4/23 23:28, Chautru, Nicolas wrote:

Hi Maxime,


-Original Message-
From: Maxime Coquelin 
Sent: Wednesday, October 4, 2023 12:36 AM
To: Chautru, Nicolas ; dev@dpdk.org
Cc: hemant.agra...@nxp.com; david.march...@redhat.com; Vargas, Hernan

Subject: Re: [PATCH v3 06/12] baseband/acc: refactor to allow unified driver
extension



On 10/3/23 20:54, Chautru, Nicolas wrote:

Hi Maxime,


-Original Message-
From: Maxime Coquelin 
Sent: Tuesday, October 3, 2023 6:15 AM
To: Chautru, Nicolas ; dev@dpdk.org
Cc: hemant.agra...@nxp.com; david.march...@redhat.com; Vargas,

Hernan


Subject: Re: [PATCH v3 06/12] baseband/acc: refactor to allow unified
driver extension

Thanks for doing the split, that will ease review.

On 9/29/23 18:35, Nicolas Chautru wrote:

Adding a few functions and common code prior to extending the VRB
driver.

Signed-off-by: Nicolas Chautru 
---
drivers/baseband/acc/acc_common.h | 164

+++-

--

drivers/baseband/acc/rte_acc100_pmd.c |   4 +-
drivers/baseband/acc/rte_vrb_pmd.c|  62 +-
3 files changed, 184 insertions(+), 46 deletions(-)

diff --git a/drivers/baseband/acc/acc_common.h
b/drivers/baseband/acc/acc_common.h
index 788abf1a3c..89893eae43 100644
--- a/drivers/baseband/acc/acc_common.h
+++ b/drivers/baseband/acc/acc_common.h
@@ -18,6 +18,7 @@
#define ACC_DMA_BLKID_OUT_HARQ  3
#define ACC_DMA_BLKID_IN_HARQ   3
#define ACC_DMA_BLKID_IN_MLD_R  3
+#define ACC_DMA_BLKID_DEWIN_IN  3

/* Values used in filling in decode FCWs */
#define ACC_FCW_TD_VER  1
@@ -103,6 +104,9 @@
#define ACC_MAX_NUM_QGRPS  32
#define ACC_RING_SIZE_GRANULARITY  64
#define ACC_MAX_FCW_SIZE  128
+#define ACC_IQ_SIZE4
+
+#define ACC_FCW_FFT_BLEN_3 28

/* Constants from K0 computation from 3GPP 38.212 Table 5.4.2.1-2 */
#define ACC_N_ZC_1 66 /* N = 66 Zc for BG 1 */ @@ -132,6 +136,17 @@
#define ACC_LIM_21 14 /* 0.21 */
#define ACC_LIM_31 20 /* 0.31 */
#define ACC_MAX_E (128 * 1024 - 2)
+#define ACC_MAX_CS 12
+
+#define ACC100_VARIANT  0
+#define VRB1_VARIANT   2
+#define VRB2_VARIANT   3
+
+/* Queue Index Hierarchy */
+#define VRB1_GRP_ID_SHIFT10
+#define VRB1_VF_ID_SHIFT 4
+#define VRB2_GRP_ID_SHIFT12
+#define VRB2_VF_ID_SHIFT 6

/* Helper macro for logging */
#define rte_acc_log(level, fmt, ...) \ @@ -332,6 +347,37 @@
struct __rte_packed acc_fcw_fft {
res:19;
};

+/* FFT Frame Control Word. */
+struct __rte_packed acc_fcw_fft_3 {
+   uint32_t in_frame_size:16,
+   leading_pad_size:16;
+   uint32_t out_frame_size:16,
+   leading_depad_size:16;
+   uint32_t cs_window_sel;
+   uint32_t cs_window_sel2:16,
+   cs_enable_bmap:16;
+   uint32_t num_antennas:8,
+   idft_size:8,
+   dft_size:8,
+   cs_offset:8;
+   uint32_t idft_shift:8,
+   dft_shift:8,
+   cs_multiplier:16;
+   uint32_t bypass:2,
+   fp16_in:1,
+   fp16_out:1,
+   exp_adj:4,
+   power_shift:4,
+   power_en:1,
+   enable_dewin:1,
+   freq_resample_mode:2,
+   depad_output_size:16;
+   uint16_t cs_theta_0[ACC_MAX_CS];
+   uint32_t cs_theta_d[ACC_MAX_CS];
+   int8_t cs_time_offset[ACC_MAX_CS]; };
+
+
/* MLD-TS Frame Control Word */
struct __rte_packed acc_fcw_mldts {
uint32_t fcw_version:4,
@@ -473,14 +519,14 @@ union acc_info_ring_data {
uint16_t valid: 1;
};
struct {
-   uint32_t aq_id_3: 6;
-   uint32_t qg_id_3: 5;
-   uint32_t vf_id_3: 6;
-   uint32_t int_nb_3: 6;
-   uint32_t msi_0_3: 1;
-   uint32_t vf2pf_3: 6;
-   uint32_t loop_3: 1;
-   uint32_t valid_3: 1;
+   uint32_t aq_id_vrb2: 6;
+   uint32_t qg_id_vrb2: 5;
+   uint32_t vf_id_vrb2: 6;
+   uint32_t int_nb_vrb2: 6;
+   uint32_t msi_0_vrb2: 1;
+   uint32_t vf2pf_vrb2: 6;
+   uint32_t loop_vrb2: 1;
+   uint32_t valid_vrb2: 1;
};
} __rte_packed;

@@ -761,22 +807,105 @@ alloc_sw_rings_min_mem(struct rte_bbdev

*dev,

struct acc_device *d,

free_base_addresses(base_addrs, i);
}

+/* Wrapper to provide VF index from ring data. */ static inline
+uint16_t vf_from_ring(const union acc_info_ring_data ring_data,
+uint16_t device_variant) {


curly braces on a new line.


Thanks.




+   if (device_variant == VRB2_VARIANT)
+   return ring_data.vf_id_vrb2;
+   else
+   return ring_data.vf_id;
+}
+
+/* Wrapper to provide QG index from ring data. */ static inline
+uint16_t qg_from_ring(const union acc_info_ring_data

Re: [PATCH v3 09/12] baseband/acc: add FFT support to VRB2 variant

[Maxime Coquelin]

On 10/4/23 23:18, Chautru, Nicolas wrote:

Hi Maxime,


-Original Message-
From: Maxime Coquelin 
Sent: Wednesday, October 4, 2023 12:11 AM
To: Chautru, Nicolas ; dev@dpdk.org
Cc: hemant.agra...@nxp.com; david.march...@redhat.com; Vargas, Hernan

Subject: Re: [PATCH v3 09/12] baseband/acc: add FFT support to VRB2 variant



On 10/3/23 20:20, Chautru, Nicolas wrote:

Hi Maxime,


-Original Message-
From: Maxime Coquelin 
Sent: Tuesday, October 3, 2023 7:37 AM
To: Chautru, Nicolas ; dev@dpdk.org
Cc: hemant.agra...@nxp.com; david.march...@redhat.com; Vargas,

Hernan


Subject: Re: [PATCH v3 09/12] baseband/acc: add FFT support to VRB2
variant



On 9/29/23 18:35, Nicolas Chautru wrote:

Support for the FFT the processing specific to the
VRB2 variant.

Signed-off-by: Nicolas Chautru 
---
drivers/baseband/acc/rte_vrb_pmd.c | 132

-

1 file changed, 128 insertions(+), 4 deletions(-)

diff --git a/drivers/baseband/acc/rte_vrb_pmd.c
b/drivers/baseband/acc/rte_vrb_pmd.c
index 93add82947..ce4b90d8e7 100644
--- a/drivers/baseband/acc/rte_vrb_pmd.c
+++ b/drivers/baseband/acc/rte_vrb_pmd.c
@@ -903,6 +903,9 @@ vrb_queue_setup(struct rte_bbdev *dev, uint16_t

queue_id,

ACC_FCW_LD_BLEN : (conf->op_type ==

RTE_BBDEV_OP_FFT ?

ACC_FCW_FFT_BLEN : ACC_FCW_MLDTS_BLEN;

+   if ((q->d->device_variant == VRB2_VARIANT) && (conf->op_type ==

RTE_BBDEV_OP_FFT))

+   fcw_len = ACC_FCW_FFT_BLEN_3;
+
for (desc_idx = 0; desc_idx < d->sw_ring_max_depth; desc_idx++) {
desc = q->ring_addr + desc_idx;
desc->req.word0 = ACC_DMA_DESC_TYPE; @@ -1323,6

+1326,24 @@

vrb_dev_info_get(struct rte_bbdev *dev, struct rte_bbdev_driver_info

*dev_info)

.num_buffers_soft_out = 0,
}
},
+   {
+   .type   = RTE_BBDEV_OP_FFT,
+   .cap.fft = {
+   .capability_flags =
+

RTE_BBDEV_FFT_WINDOWING |

+

RTE_BBDEV_FFT_CS_ADJUSTMENT |

+

RTE_BBDEV_FFT_DFT_BYPASS |

+

RTE_BBDEV_FFT_IDFT_BYPASS |

+   RTE_BBDEV_FFT_FP16_INPUT

|

+

RTE_BBDEV_FFT_FP16_OUTPUT |

+

RTE_BBDEV_FFT_POWER_MEAS |

+

RTE_BBDEV_FFT_WINDOWING_BYPASS,

+   .num_buffers_src =
+   1,
+   .num_buffers_dst =
+   1,
+   }
+   },
RTE_BBDEV_END_OF_CAPABILITIES_LIST()
};

@@ -3849,6 +3870,47 @@ vrb1_fcw_fft_fill(struct rte_bbdev_fft_op
*op,

struct acc_fcw_fft *fcw)

fcw->bypass = 0;
}

+/* Fill in a frame control word for FFT processing. */ static
+inline void vrb2_fcw_fft_fill(struct rte_bbdev_fft_op *op, struct
+acc_fcw_fft_3 *fcw) {
+   fcw->in_frame_size = op->fft.input_sequence_size;
+   fcw->leading_pad_size = op->fft.input_leading_padding;
+   fcw->out_frame_size = op->fft.output_sequence_size;
+   fcw->leading_depad_size = op->fft.output_leading_depadding;
+   fcw->cs_window_sel = op->fft.window_index[0] +
+   (op->fft.window_index[1] << 8) +
+   (op->fft.window_index[2] << 16) +
+   (op->fft.window_index[3] << 24);
+   fcw->cs_window_sel2 = op->fft.window_index[4] +
+   (op->fft.window_index[5] << 8);
+   fcw->cs_enable_bmap = op->fft.cs_bitmap;
+   fcw->num_antennas = op->fft.num_antennas_log2;
+   fcw->idft_size = op->fft.idft_log2;
+   fcw->dft_size = op->fft.dft_log2;
+   fcw->cs_offset = op->fft.cs_time_adjustment;
+   fcw->idft_shift = op->fft.idft_shift;
+   fcw->dft_shift = op->fft.dft_shift;
+   fcw->cs_multiplier = op->fft.ncs_reciprocal;
+   fcw->power_shift = op->fft.power_shift; > +fcw->exp_adj = op-
fft.fp16_exp_adjust;
+   fcw->fp16_in = check_bit(op->fft.op_flags,

RTE_BBDEV_FFT_FP16_INPUT);

+   fcw->fp16_out = check_bit(op->fft.op_flags,

RTE_BBDEV_FFT_FP16_OUTPUT);

+   fcw->power_en = check_bit(op->fft.op_flags,

RTE_BBDEV_FFT_POWER_MEAS);

+   if (check_bit(op->fft.op_flags,
+   RTE_BBDEV_FFT_IDFT_BYPASS)) {
+   if (check_bit(op->fft.op_flags,
+   RTE_BBDEV_FFT_WINDOWING_BYPASS))
+   fcw->bypass = 2;
+   else
+   fcw->bypass = 1;
+   } else if (check_bit(op->fft.op_flags,
+   RTE_BBDEV_FFT_DFT_BYPASS))
+   fcw->bypass = 3;
+   else
+

Re: [PATCH v3 08/12] baseband/acc: add FEC capabilities for the VRB2 variant

[Maxime Coquelin]

On 10/4/23 23:11, Chautru, Nicolas wrote:

Hi Maxime,


-Original Message-
From: Maxime Coquelin 
Sent: Tuesday, October 3, 2023 7:28 AM
To: Chautru, Nicolas ; dev@dpdk.org
Cc: hemant.agra...@nxp.com; david.march...@redhat.com; Vargas, Hernan

Subject: Re: [PATCH v3 08/12] baseband/acc: add FEC capabilities for the VRB2
variant



On 9/29/23 18:35, Nicolas Chautru wrote:

New implementation for some of the FEC features specific to the VRB2
variant.

Signed-off-by: Nicolas Chautru 
---
   drivers/baseband/acc/rte_vrb_pmd.c | 567

-

   1 file changed, 548 insertions(+), 19 deletions(-)

diff --git a/drivers/baseband/acc/rte_vrb_pmd.c
b/drivers/baseband/acc/rte_vrb_pmd.c
index 48e779ce77..93add82947 100644
--- a/drivers/baseband/acc/rte_vrb_pmd.c
+++ b/drivers/baseband/acc/rte_vrb_pmd.c
@@ -1235,6 +1235,94 @@ vrb_dev_info_get(struct rte_bbdev *dev, struct

rte_bbdev_driver_info *dev_info)

};

static const struct rte_bbdev_op_cap vrb2_bbdev_capabilities[] = {
+   {
+   .type = RTE_BBDEV_OP_TURBO_DEC,
+   .cap.turbo_dec = {
+   .capability_flags =
+

RTE_BBDEV_TURBO_SUBBLOCK_DEINTERLEAVE |

+   RTE_BBDEV_TURBO_CRC_TYPE_24B |
+

RTE_BBDEV_TURBO_DEC_CRC_24B_DROP |

+   RTE_BBDEV_TURBO_EQUALIZER |
+

RTE_BBDEV_TURBO_SOFT_OUT_SATURATE |

+

RTE_BBDEV_TURBO_HALF_ITERATION_EVEN |

+

RTE_BBDEV_TURBO_CONTINUE_CRC_MATCH |

+   RTE_BBDEV_TURBO_SOFT_OUTPUT |
+

RTE_BBDEV_TURBO_EARLY_TERMINATION |

+

RTE_BBDEV_TURBO_DEC_INTERRUPTS |

+

RTE_BBDEV_TURBO_NEG_LLR_1_BIT_IN |

+

RTE_BBDEV_TURBO_NEG_LLR_1_BIT_SOFT_OUT |

+   RTE_BBDEV_TURBO_MAP_DEC |
+

RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP |

+

RTE_BBDEV_TURBO_DEC_SCATTER_GATHER,

+   .max_llr_modulus = INT8_MAX,
+   .num_buffers_src =
+

RTE_BBDEV_TURBO_MAX_CODE_BLOCKS,

+   .num_buffers_hard_out =
+

RTE_BBDEV_TURBO_MAX_CODE_BLOCKS,

+   .num_buffers_soft_out =
+

RTE_BBDEV_TURBO_MAX_CODE_BLOCKS,

+   }
+   },
+   {
+   .type = RTE_BBDEV_OP_TURBO_ENC,
+   .cap.turbo_enc = {
+   .capability_flags =
+

RTE_BBDEV_TURBO_CRC_24B_ATTACH |

+

RTE_BBDEV_TURBO_RV_INDEX_BYPASS |

+   RTE_BBDEV_TURBO_RATE_MATCH |
+

RTE_BBDEV_TURBO_ENC_INTERRUPTS |

+

RTE_BBDEV_TURBO_ENC_SCATTER_GATHER,

+   .num_buffers_src =
+

RTE_BBDEV_TURBO_MAX_CODE_BLOCKS,

+   .num_buffers_dst =
+

RTE_BBDEV_TURBO_MAX_CODE_BLOCKS,

+   }
+   },
+   {
+   .type   = RTE_BBDEV_OP_LDPC_ENC,
+   .cap.ldpc_enc = {
+   .capability_flags =
+   RTE_BBDEV_LDPC_RATE_MATCH |
+   RTE_BBDEV_LDPC_CRC_24B_ATTACH

|

+

RTE_BBDEV_LDPC_INTERLEAVER_BYPASS |

+   RTE_BBDEV_LDPC_ENC_INTERRUPTS

|

+

RTE_BBDEV_LDPC_ENC_SCATTER_GATHER |

+

RTE_BBDEV_LDPC_ENC_CONCATENATION,

+   .num_buffers_src =
+

RTE_BBDEV_LDPC_MAX_CODE_BLOCKS,

+   .num_buffers_dst =
+

RTE_BBDEV_LDPC_MAX_CODE_BLOCKS,

+   }
+   },
+   {
+   .type   = RTE_BBDEV_OP_LDPC_DEC,
+   .cap.ldpc_dec = {
+   .capability_flags =
+   RTE_BBDEV_LDPC_CRC_TYPE_24B_CHECK |
+   RTE_BBDEV_LDPC_CRC_TYPE_24B_DROP |
+   RTE_BBDEV_LDPC_CRC_TYPE_24A_CHECK |
+   RTE_BBDEV_LDPC_CRC_TYPE_16_CHECK |
+   RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE

|

+

RTE_BBDEV_LDPC_HQ_COMBINE_OUT_ENABLE |

+   RTE_BBDEV_LDPC_ITERATION_STOP_ENABLE

|

+   RTE_BBDEV_LDPC_DEINTERLEAVER_BYPASS |
+   RTE_BBDEV_LDPC_DEC_SCATTER_GATHER |
+

RTE_BBDEV_LDPC_HARQ_6BIT_COMPRESSION |

+

RTE_BBDEV_LDPC_HARQ_4BIT_COMPRESSION |

+   RTE_BBDEV_LDPC_LLR_COMPRESSION |
+   RTE_BBDEV_LDPC_SOFT_OUT_ENABLE |
+   RTE_BBDEV_LDPC_SOFT_OUT_RM_BYPASS |
+

RTE_BBDEV_LDPC_SOFT_OUT_DEINTERLEAVER_BYPASS

Re: [PATCH] net/vhost: report TX errors in port stats

[Maxime Coquelin]

On 9/30/23 03:00, Andrey Ignatov wrote:

vhost device doesn't report TX errors what complicates debugging of
dropped packets. Add oerrors to port stats.

- before (testpmd `show port stats`):
   TX-packets: 18328512   TX-errors: 0  TX-bytes:  1173024768

- after:
   TX-packets: 1737728TX-errors: 131367616  TX-bytes:  111214592

Signed-off-by: Andrey Ignatov 
---
  drivers/net/vhost/rte_eth_vhost.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/drivers/net/vhost/rte_eth_vhost.c 
b/drivers/net/vhost/rte_eth_vhost.c
index 8d37ec9775..48a9a79efe 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -1299,6 +1299,7 @@ eth_stats_get(struct rte_eth_dev *dev, struct 
rte_eth_stats *stats)
unsigned i;
unsigned long rx_total = 0, tx_total = 0;
unsigned long rx_total_bytes = 0, tx_total_bytes = 0;
+   unsigned long tx_total_errors = 0;
struct vhost_queue *vq;
  
  	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS &&

@@ -1323,12 +1324,15 @@ eth_stats_get(struct rte_eth_dev *dev, struct 
rte_eth_stats *stats)
  
  		stats->q_obytes[i] = vq->stats.bytes;

tx_total_bytes += stats->q_obytes[i];
+
+   tx_total_errors += vq->stats.missed_pkts;
}
  
  	stats->ipackets = rx_total;

stats->opackets = tx_total;
stats->ibytes = rx_total_bytes;
stats->obytes = tx_total_bytes;
+   stats->oerrors = tx_total_errors;
  
  	return 0;

  }


Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [PATCH v5 01/12] bbdev: add FFT window width member in driver info

[Maxime Coquelin]

On 10/5/23 21:48, Nicolas Chautru wrote:

This exposes the width of each windowing shape being configured on
the device. This allows to distinguish different version of the
flexible pointwise windowing applied to the FFT and expose
this platform configuration to the application.
This also add the total number of windows supported in the capability.

The SRS processing chain
(https://doc.dpdk.org/guides/prog_guide/bbdev.html#bbdev-fft-operation)
includes a pointwise multiplication by time window whose shape width
needs to be exposed, notably for accurate SNR estimate.
Using that mechanism user application can retrieve information related
to what has been dynamically programmed on any bbdev device
supporting FFT windowing operation.

Signed-off-by: Nicolas Chautru 
---
  lib/bbdev/rte_bbdev.h| 2 ++
  lib/bbdev/rte_bbdev_op.h | 2 ++
  2 files changed, 4 insertions(+)

diff --git a/lib/bbdev/rte_bbdev.h b/lib/bbdev/rte_bbdev.h
index 2985c9f42b..d12e2e7fbc 100644
--- a/lib/bbdev/rte_bbdev.h
+++ b/lib/bbdev/rte_bbdev.h
@@ -349,6 +349,8 @@ struct rte_bbdev_driver_info {
const struct rte_bbdev_op_cap *capabilities;
/** Device cpu_flag requirements */
const enum rte_cpu_flag_t *cpu_flag_reqs;
+   /** FFT windowing width for 2048 FFT - size defined in capability. */
+   uint16_t *fft_window_width;
  };
  
  /** Macro used at end of bbdev PMD list */

diff --git a/lib/bbdev/rte_bbdev_op.h b/lib/bbdev/rte_bbdev_op.h
index 693baa8386..369ac331bf 100644
--- a/lib/bbdev/rte_bbdev_op.h
+++ b/lib/bbdev/rte_bbdev_op.h
@@ -905,6 +905,8 @@ struct rte_bbdev_op_cap_fft {
uint16_t num_buffers_src;
/** Num output code block buffers. */
uint16_t num_buffers_dst;
+   /** Number of FFT windows supported. */
+   uint16_t fft_windows_num;
  };
  
  /** List of the capabilities for the MLD */


I am Ok to move forwards with this, even if I would have liked feedback
from Hemant.

However, I think you need to add some documentation around it, as what
is in the commit message is important and won't be visible to the end-
user.

Thanks,
Maxime

Re: [PATCH v5 02/12] baseband/acc: add FFT window width in the VRB PMD

[Maxime Coquelin]

On 10/5/23 21:48, Nicolas Chautru wrote:

This allows to expose the FFT window width being introduced in
previous commit based on what is configured dynamically on the
device platform.

Signed-off-by: Nicolas Chautru 
---
  drivers/baseband/acc/acc_common.h  |  6 +
  drivers/baseband/acc/rte_vrb_pmd.c | 41 +-
  2 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/drivers/baseband/acc/acc_common.h 
b/drivers/baseband/acc/acc_common.h
index 5bb00746c3..afece863bc 100644
--- a/drivers/baseband/acc/acc_common.h
+++ b/drivers/baseband/acc/acc_common.h
@@ -131,6 +131,8 @@
  #define ACC_LIM_31 20 /* 0.31 */
  #define ACC_MAX_E (128 * 1024 - 2)
  
+#define ACC_MAX_FFT_WIN  16

+
  /* Helper macro for logging */
  #define rte_acc_log(level, fmt, ...) \
rte_log(RTE_LOG_ ## level, RTE_LOG_NOTICE, fmt "\n", \
@@ -512,6 +514,8 @@ struct acc_deq_intr_details {
  enum {
ACC_VF2PF_STATUS_REQUEST = 1,
ACC_VF2PF_USING_VF = 2,
+   ACC_VF2PF_LUT_VER_REQUEST = 3,
+   ACC_VF2PF_FFT_WIN_REQUEST = 4,
  };
  
  
@@ -558,6 +562,8 @@ struct acc_device {

queue_offset_fun_t queue_offset;  /* Device specific queue offset */
uint16_t num_qgroups;
uint16_t num_aqs;
+   uint16_t fft_window_width[ACC_MAX_FFT_WIN]; /* FFT windowing size. */
+


Remove new line.


  };
  
  /* Structure associated with each queue. */

diff --git a/drivers/baseband/acc/rte_vrb_pmd.c 
b/drivers/baseband/acc/rte_vrb_pmd.c
index 9e5a73c9c7..b86e814f8f 100644
--- a/drivers/baseband/acc/rte_vrb_pmd.c
+++ b/drivers/baseband/acc/rte_vrb_pmd.c
@@ -183,6 +183,37 @@ vrb_check_device_enable(struct rte_bbdev *dev)
return false;
  }
  
+static inline void

+vrb_vf2pf(struct acc_device *d, unsigned int payload)
+{
+   acc_reg_write(d, d->reg_addr->vf2pf_doorbell, payload);
+}
+
+/* Request device FFT windowing information. */
+static inline void
+vrb_device_fft_win(struct rte_bbdev *dev)
+{
+   struct acc_device *d = dev->data->dev_private;
+   uint32_t reg, time_out = 0, win;
+
+   if (d->pf_device)
+   return;
+
+   /* Check from the device the first time. */
+   if (d->fft_window_width[0] == 0) {
+   for (win = 0; win < ACC_MAX_FFT_WIN; win++) {
+   vrb_vf2pf(d, ACC_VF2PF_FFT_WIN_REQUEST | win);
+   reg = acc_reg_read(d, d->reg_addr->pf2vf_doorbell);
+   while ((time_out < ACC_STATUS_TO) && (reg == 
RTE_BBDEV_DEV_NOSTATUS)) {
+   usleep(ACC_STATUS_WAIT); /*< Wait or VF->PF->VF 
Comms. */
+   reg = acc_reg_read(d, 
d->reg_addr->pf2vf_doorbell);
+   time_out++;
+   }
+   d->fft_window_width[win] = reg;
+   }
+   }
+}
+
  /* Fetch configuration enabled for the PF/VF using MMIO Read (slow). */
  static inline void
  fetch_acc_config(struct rte_bbdev *dev)
@@ -206,6 +237,8 @@ fetch_acc_config(struct rte_bbdev *dev)
return;
}
  
+	vrb_device_fft_win(dev);

+
d->ddr_size = 0;
  
  	/* Single VF Bundle by VF. */

@@ -271,12 +304,6 @@ fetch_acc_config(struct rte_bbdev *dev)
acc_conf->q_fft.aq_depth_log2);
  }
  
-static inline void

-vrb_vf2pf(struct acc_device *d, unsigned int payload)
-{
-   acc_reg_write(d, d->reg_addr->vf2pf_doorbell, payload);
-}
-
  /* Request device status information. */
  static inline uint32_t
  vrb_device_status(struct rte_bbdev *dev)
@@ -1085,6 +1112,7 @@ vrb_dev_info_get(struct rte_bbdev *dev, struct 
rte_bbdev_driver_info *dev_info)
RTE_BBDEV_LDPC_MAX_CODE_BLOCKS,
.num_buffers_dst =
RTE_BBDEV_LDPC_MAX_CODE_BLOCKS,
+   .fft_windows_num = ACC_MAX_FFT_WIN,
}
},
RTE_BBDEV_END_OF_CAPABILITIES_LIST()
@@ -1100,6 +1128,7 @@ vrb_dev_info_get(struct rte_bbdev *dev, struct 
rte_bbdev_driver_info *dev_info)
fetch_acc_config(dev);
/* Check the status of device. */
dev_info->device_status = vrb_device_status(dev);
+   dev_info->fft_window_width = d->fft_window_width;
  
  	/* Exposed number of queues. */

    dev_info->num_queues[RTE_BBDEV_OP_NONE] = 0;


With new line removed:

Reviewed-by; Maxime Coquelin 

Thanks,
Maxime

Re: [PATCH v5 06/12] baseband/acc: refactor to allow unified driver extension

[Maxime Coquelin]

On 10/5/23 21:49, Nicolas Chautru wrote:

Adding a few functions and common code prior to
extending the VRB driver.

Signed-off-by: Nicolas Chautru 
---
  drivers/baseband/acc/acc_common.h  | 175 ++---
  drivers/baseband/acc/rte_vrb_pmd.c |  58 +-
  2 files changed, 190 insertions(+), 43 deletions(-)


Reviewed-by: Maxime Coquelin 

Maxime

Re: [PATCH v5 08/12] baseband/acc: add FEC capabilities for the VRB2 variant

[Maxime Coquelin]

On 10/5/23 21:49, Nicolas Chautru wrote:

New implementation for some of the FEC features
specific to the VRB2 variant.

Signed-off-by: Nicolas Chautru 
---
  drivers/baseband/acc/rte_vrb_pmd.c | 370 +
  1 file changed, 328 insertions(+), 42 deletions(-)

diff --git a/drivers/baseband/acc/rte_vrb_pmd.c 
b/drivers/baseband/acc/rte_vrb_pmd.c
index b36dd6ccd8..dab61fd5df 100644
--- a/drivers/baseband/acc/rte_vrb_pmd.c
+++ b/drivers/baseband/acc/rte_vrb_pmd.c
@@ -1235,6 +1235,94 @@ vrb_dev_info_get(struct rte_bbdev *dev, struct 
rte_bbdev_driver_info *dev_info)
};
  
  	static const struct rte_bbdev_op_cap vrb2_bbdev_capabilities[] = {

+   {
+   .type = RTE_BBDEV_OP_TURBO_DEC,
+   .cap.turbo_dec = {
+   .capability_flags =
+   RTE_BBDEV_TURBO_SUBBLOCK_DEINTERLEAVE |
+   RTE_BBDEV_TURBO_CRC_TYPE_24B |
+   RTE_BBDEV_TURBO_DEC_CRC_24B_DROP |
+   RTE_BBDEV_TURBO_EQUALIZER |
+   RTE_BBDEV_TURBO_SOFT_OUT_SATURATE |
+   RTE_BBDEV_TURBO_HALF_ITERATION_EVEN |
+   RTE_BBDEV_TURBO_CONTINUE_CRC_MATCH |
+   RTE_BBDEV_TURBO_SOFT_OUTPUT |
+   RTE_BBDEV_TURBO_EARLY_TERMINATION |
+   RTE_BBDEV_TURBO_DEC_INTERRUPTS |
+   RTE_BBDEV_TURBO_NEG_LLR_1_BIT_IN |
+   RTE_BBDEV_TURBO_NEG_LLR_1_BIT_SOFT_OUT |
+   RTE_BBDEV_TURBO_MAP_DEC |
+   RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP |
+   RTE_BBDEV_TURBO_DEC_SCATTER_GATHER,
+   .max_llr_modulus = INT8_MAX,
+   .num_buffers_src =
+   RTE_BBDEV_TURBO_MAX_CODE_BLOCKS,
+   .num_buffers_hard_out =
+   RTE_BBDEV_TURBO_MAX_CODE_BLOCKS,
+   .num_buffers_soft_out =
+   RTE_BBDEV_TURBO_MAX_CODE_BLOCKS,
+   }
+   },
+   {
+   .type = RTE_BBDEV_OP_TURBO_ENC,
+   .cap.turbo_enc = {
+   .capability_flags =
+   RTE_BBDEV_TURBO_CRC_24B_ATTACH |
+   RTE_BBDEV_TURBO_RV_INDEX_BYPASS |
+   RTE_BBDEV_TURBO_RATE_MATCH |
+   RTE_BBDEV_TURBO_ENC_INTERRUPTS |
+   RTE_BBDEV_TURBO_ENC_SCATTER_GATHER,
+   .num_buffers_src =
+   RTE_BBDEV_TURBO_MAX_CODE_BLOCKS,
+   .num_buffers_dst =
+   RTE_BBDEV_TURBO_MAX_CODE_BLOCKS,
+   }
+   },
+   {
+   .type   = RTE_BBDEV_OP_LDPC_ENC,
+   .cap.ldpc_enc = {
+   .capability_flags =
+   RTE_BBDEV_LDPC_RATE_MATCH |
+   RTE_BBDEV_LDPC_CRC_24B_ATTACH |
+   RTE_BBDEV_LDPC_INTERLEAVER_BYPASS |
+   RTE_BBDEV_LDPC_ENC_INTERRUPTS |
+   RTE_BBDEV_LDPC_ENC_SCATTER_GATHER |
+   RTE_BBDEV_LDPC_ENC_CONCATENATION,
+   .num_buffers_src =
+   RTE_BBDEV_LDPC_MAX_CODE_BLOCKS,
+   .num_buffers_dst =
+   RTE_BBDEV_LDPC_MAX_CODE_BLOCKS,
+   }
+   },
+   {
+   .type   = RTE_BBDEV_OP_LDPC_DEC,
+   .cap.ldpc_dec = {
+   .capability_flags =
+   RTE_BBDEV_LDPC_CRC_TYPE_24B_CHECK |
+   RTE_BBDEV_LDPC_CRC_TYPE_24B_DROP |
+   RTE_BBDEV_LDPC_CRC_TYPE_24A_CHECK |
+   RTE_BBDEV_LDPC_CRC_TYPE_16_CHECK |
+   RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE |
+   RTE_BBDEV_LDPC_HQ_COMBINE_OUT_ENABLE |
+   RTE_BBDEV_LDPC_ITERATION_STOP_ENABLE |
+   RTE_BBDEV_LDPC_DEINTERLEAVER_BYPASS |
+   RTE_BBDEV_LDPC_

Re: [PATCH v3 09/12] baseband/acc: add FFT support to VRB2 variant

[Maxime Coquelin]

On 10/5/23 19:59, Chautru, Nicolas wrote:

Hi Maxime,


-Original Message-
From: Maxime Coquelin 
Sent: Thursday, October 5, 2023 7:35 AM
To: Chautru, Nicolas ; dev@dpdk.org
Cc: hemant.agra...@nxp.com; david.march...@redhat.com; Vargas, Hernan

Subject: Re: [PATCH v3 09/12] baseband/acc: add FFT support to VRB2 variant



On 10/4/23 23:18, Chautru, Nicolas wrote:

Hi Maxime,


-Original Message-
From: Maxime Coquelin 
Sent: Wednesday, October 4, 2023 12:11 AM
To: Chautru, Nicolas ; dev@dpdk.org
Cc: hemant.agra...@nxp.com; david.march...@redhat.com; Vargas,

Hernan


Subject: Re: [PATCH v3 09/12] baseband/acc: add FFT support to VRB2
variant



On 10/3/23 20:20, Chautru, Nicolas wrote:

Hi Maxime,


-Original Message-
From: Maxime Coquelin 
Sent: Tuesday, October 3, 2023 7:37 AM
To: Chautru, Nicolas ; dev@dpdk.org
Cc: hemant.agra...@nxp.com; david.march...@redhat.com; Vargas,

Hernan


Subject: Re: [PATCH v3 09/12] baseband/acc: add FFT support to VRB2
variant



On 9/29/23 18:35, Nicolas Chautru wrote:

Support for the FFT the processing specific to the
VRB2 variant.

Signed-off-by: Nicolas Chautru 
---
 drivers/baseband/acc/rte_vrb_pmd.c | 132

-

 1 file changed, 128 insertions(+), 4 deletions(-)

diff --git a/drivers/baseband/acc/rte_vrb_pmd.c
b/drivers/baseband/acc/rte_vrb_pmd.c
index 93add82947..ce4b90d8e7 100644
--- a/drivers/baseband/acc/rte_vrb_pmd.c
+++ b/drivers/baseband/acc/rte_vrb_pmd.c
@@ -903,6 +903,9 @@ vrb_queue_setup(struct rte_bbdev *dev,
uint16_t

queue_id,

ACC_FCW_LD_BLEN : (conf->op_type ==

RTE_BBDEV_OP_FFT ?

ACC_FCW_FFT_BLEN :

ACC_FCW_MLDTS_BLEN;


+   if ((q->d->device_variant == VRB2_VARIANT) && (conf->op_type ==

RTE_BBDEV_OP_FFT))

+   fcw_len = ACC_FCW_FFT_BLEN_3;
+
for (desc_idx = 0; desc_idx < d->sw_ring_max_depth;

desc_idx++) {

desc = q->ring_addr + desc_idx;
desc->req.word0 = ACC_DMA_DESC_TYPE; @@ -

1323,6

+1326,24 @@

vrb_dev_info_get(struct rte_bbdev *dev, struct
rte_bbdev_driver_info

*dev_info)

.num_buffers_soft_out = 0,
}
},
+   {
+   .type   = RTE_BBDEV_OP_FFT,
+   .cap.fft = {
+   .capability_flags =
+

RTE_BBDEV_FFT_WINDOWING |

+

RTE_BBDEV_FFT_CS_ADJUSTMENT |

+

RTE_BBDEV_FFT_DFT_BYPASS |

+

RTE_BBDEV_FFT_IDFT_BYPASS |

+   RTE_BBDEV_FFT_FP16_INPUT

|

+

RTE_BBDEV_FFT_FP16_OUTPUT |

+

RTE_BBDEV_FFT_POWER_MEAS |

+

RTE_BBDEV_FFT_WINDOWING_BYPASS,

+   .num_buffers_src =
+   1,
+   .num_buffers_dst =
+   1,
+   }
+   },
RTE_BBDEV_END_OF_CAPABILITIES_LIST()
};

@@ -3849,6 +3870,47 @@ vrb1_fcw_fft_fill(struct rte_bbdev_fft_op
*op,

struct acc_fcw_fft *fcw)

fcw->bypass = 0;
 }

+/* Fill in a frame control word for FFT processing. */ static
+inline void vrb2_fcw_fft_fill(struct rte_bbdev_fft_op *op, struct
+acc_fcw_fft_3 *fcw) {
+   fcw->in_frame_size = op->fft.input_sequence_size;
+   fcw->leading_pad_size = op->fft.input_leading_padding;
+   fcw->out_frame_size = op->fft.output_sequence_size;
+   fcw->leading_depad_size = op->fft.output_leading_depadding;
+   fcw->cs_window_sel = op->fft.window_index[0] +
+   (op->fft.window_index[1] << 8) +
+   (op->fft.window_index[2] << 16) +
+   (op->fft.window_index[3] << 24);
+   fcw->cs_window_sel2 = op->fft.window_index[4] +
+   (op->fft.window_index[5] << 8);
+   fcw->cs_enable_bmap = op->fft.cs_bitmap;
+   fcw->num_antennas = op->fft.num_antennas_log2;
+   fcw->idft_size = op->fft.idft_log2;
+   fcw->dft_size = op->fft.dft_log2;
+   fcw->cs_offset = op->fft.cs_time_adjustment;
+   fcw->idft_shift = op->fft.idft_shift;
+   fcw->dft_shift = op->fft.dft_shift;
+   fcw->cs_multiplier = op->fft.ncs_reciprocal;
+   fcw->power_shift = op->fft.power_shift; > +fcw->exp_adj = op-
fft.fp16_exp_adjust;
+   fcw->fp16_in = check_bit(op->fft.op_flags,

RTE_BBDEV_FFT_FP16_INPUT);

+   fcw->fp16_out = check_bit(op->fft.op_flags,

RTE_BBDEV_FFT_FP16_OUTPUT);

+   fcw->power_en = check_bit(op->fft.op_flags,

RTE_BBDEV_FFT_POWER_MEAS);

+   if (check_bit(op->fft.op_flags,
+   RTE_BBDEV_FFT_IDFT_BYPASS)) {
+   if (check_

Re: [PATCH v5 10/12] baseband/acc: add MLD support in VRB2 variant

[Maxime Coquelin]

   op->status |= rsp.input_err << RTE_BBDEV_DATA_ERROR;
+   op->status |= rsp.dma_err << RTE_BBDEV_DRV_ERROR;
+   op->status |= rsp.fcw_err << RTE_BBDEV_DRV_ERROR;
+   op->status |= rsp.engine_hung << RTE_BBDEV_ENGINE_ERROR;
+   }
+
+   if (op->status != 0)
+   q_data->queue_stats.dequeue_err_count++;
+   if (op->status & (1 << RTE_BBDEV_DRV_ERROR))
+   vrb_check_ir(q->d);
+
+   /* Check if this is the last desc in batch (Atomic Queue). */
+   if (desc->req.last_desc_in_batch) {
+   (*aq_dequeued)++;
+   desc->req.last_desc_in_batch = 0;
+   }
+   desc->rsp.val = ACC_DMA_DESC_TYPE;
+   desc->rsp.add_info_0 = 0;
+   *ref_op = op;
+
+   return descs_in_op;
+}
+
+/* Dequeue MLDTS operations from VRB2 device. */
+static uint16_t
+vrb2_dequeue_mldts(struct rte_bbdev_queue_data *q_data,
+   struct rte_bbdev_mldts_op **ops, uint16_t num)
+{
+   struct acc_queue *q = q_data->queue_private;
+   uint16_t dequeue_num, i, dequeued_cbs = 0;
+   uint32_t avail = acc_ring_avail_deq(q);
+   uint32_t aq_dequeued = 0;
+   int ret;
+
+   dequeue_num = RTE_MIN(avail, num);
+
+   for (i = 0; i < dequeue_num; ++i) {
+   ret = dequeue_mldts_one_op(q_data, q, &ops[i], dequeued_cbs, 
&aq_dequeued);
+   if (ret <= 0)
+   break;
+   dequeued_cbs += ret;
+   }
+
+   q->aq_dequeued += aq_dequeued;
+   q->sw_ring_tail += dequeued_cbs;
+   /* Update enqueue stats. */
+   q_data->queue_stats.dequeued_count += i;
+   return i;
+}
+
  /* Initialization Function */
  static void
  vrb_bbdev_init(struct rte_bbdev *dev, struct rte_pci_driver *drv)
@@ -3896,6 +4268,8 @@ vrb_bbdev_init(struct rte_bbdev *dev, struct 
rte_pci_driver *drv)
dev->dequeue_ldpc_dec_ops = vrb_dequeue_ldpc_dec;
dev->enqueue_fft_ops = vrb_enqueue_fft;
dev->dequeue_fft_ops = vrb_dequeue_fft;
+   dev->enqueue_mldts_ops = vrb2_enqueue_mldts;
+   dev->dequeue_mldts_ops = vrb2_dequeue_mldts;
  
  	d->pf_device = !strcmp(drv->driver.name, RTE_STR(VRB_PF_DRIVER_NAME));

d->mmio_base = pci_dev->mem_resource[0].addr;


Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [PATCH v2] net/virtio: fixed missing next flag when sending packets in packed mode

[Maxime Coquelin]

Hi,

Thanks for investigating the issue and proposing a fix!

On 10/10/23 04:08, liufengjiang.0426 wrote:

When the packets is sent in packed mode, and the packets data and
virtio-header are divided into two desc, set the next flag of
virtio-header desc


Fix the warning that a single line of commit log exceeds 75 chars


The changelog should not be part of the commit message.



Bugzilla ID: 1295
Fixes: 892dc798fa9c ("net/virtio: implement Tx path for packed queues")

Signed-off-by: liufengjiang.0426 


Could you please sign-off with your full name?



---


The changelog should be placed here.


  drivers/net/virtio/virtqueue.h | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h
index 9d4aba11a3..4e9f2d0358 100644
--- a/drivers/net/virtio/virtqueue.h
+++ b/drivers/net/virtio/virtqueue.h
@@ -672,6 +672,7 @@ virtqueue_enqueue_xmit_packed(struct virtnet_tx *txvq, 
struct rte_mbuf *cookie,
 */
start_dp[idx].addr = txvq->hdr_mem + 
RTE_PTR_DIFF(&txr[idx].tx_hdr, txr);
start_dp[idx].len = vq->hw->vtnet_hdr_size;
+   head_flags |= VRING_DESC_F_NEXT;
hdr = (struct virtio_net_hdr *)&txr[idx].tx_hdr;
idx++;
if (idx >= vq->vq_nentries) {


Otherwise, the fix look good to me.
With above small changes, it will be good for me.

Thanks,
Maxime

Re: [PATCH v5 08/12] baseband/acc: add FEC capabilities for the VRB2 variant

[Maxime Coquelin]

 -1;
+
+   rsp.val = atom_desc.rsp.val;
+   rte_bbdev_log_debug("Resp. desc %p: %x", desc, rsp.val);
+
+   /* Dequeue. */
+   op = desc->req.op_addr;
+
+   /* Clearing status, it will be set based on response. */
+   op->status = 0;
+   op->status |= rsp.input_err << RTE_BBDEV_DATA_ERROR;
+   op->status |= rsp.dma_err << RTE_BBDEV_DRV_ERROR;
+   op->status |= rsp.fcw_err << RTE_BBDEV_DRV_ERROR;
+   op->status |= rsp.engine_hung << RTE_BBDEV_ENGINE_ERROR;
+
+   if (desc->req.last_desc_in_batch) {
+   (*aq_dequeued)++;
+   desc->req.last_desc_in_batch = 0;
+   }
+   desc->rsp.val = ACC_DMA_DESC_TYPE;
+   desc->rsp.add_info_0 = 0; /* Reserved bits. */
+   desc->rsp.add_info_1 = 0; /* Reserved bits. */
+
+   /* One op was successfully dequeued */
+   ref_op[0] = op;
+   (*dequeued_descs)++;
+   (*dequeued_ops)++;
+   return 1;
+}
+
  /* Dequeue one LDPC encode operations from device in TB mode.
   * That operation may cover multiple descriptors.
   */
@@ -3189,9 +3471,14 @@ vrb_dequeue_ldpc_enc(struct rte_bbdev_queue_data *q_data,
  
  	for (i = 0; i < avail; i++) {

if (cbm == RTE_BBDEV_TRANSPORT_BLOCK)
-   ret = vrb_dequeue_enc_one_op_tb(q, &ops[dequeued_ops],
-   &dequeued_ops, &aq_dequeued,
-   &dequeued_descs, num);
+   if (q->d->device_variant == VRB1_VARIANT)
+   ret = vrb_dequeue_enc_one_op_tb(q, 
&ops[dequeued_ops],
+       &dequeued_ops, &aq_dequeued,
+   &dequeued_descs, num);
+   else
+   ret = vrb2_dequeue_ldpc_enc_one_op_tb(q, 
&ops[dequeued_ops],
+   &dequeued_ops, &aq_dequeued,
+   &dequeued_descs);
else
ret = vrb_dequeue_enc_one_op_cb(q, &ops[dequeued_ops],
&dequeued_ops, &aq_dequeued,
@@ -3526,7 +3813,6 @@ vrb_bbdev_init(struct rte_bbdev *dev, struct 
rte_pci_driver *drv)
(pci_dev->id.device_id == RTE_VRB1_VF_DEVICE_ID)) {
d->device_variant = VRB1_VARIANT;
d->queue_offset = vrb1_queue_offset;
-   d->fcw_ld_fill = vrb1_fcw_ld_fill;
d->num_qgroups = VRB1_NUM_QGRPS;
d->num_aqs = VRB1_NUM_AQS;
if (d->pf_device)


Reviewed-by: Maxime Coquelin 

Re: [PATCH v5 01/12] bbdev: add FFT window width member in driver info

[Maxime Coquelin]

On 10/6/23 22:08, Chautru, Nicolas wrote:

Hi Maxime,


-Original Message-
From: Maxime Coquelin 
Sent: Friday, October 6, 2023 12:54 AM
To: Chautru, Nicolas ; dev@dpdk.org
Cc: hemant.agra...@nxp.com; david.march...@redhat.com; Vargas, Hernan

Subject: Re: [PATCH v5 01/12] bbdev: add FFT window width member in driver
info



On 10/5/23 21:48, Nicolas Chautru wrote:

This exposes the width of each windowing shape being configured on the
device. This allows to distinguish different version of the flexible
pointwise windowing applied to the FFT and expose this platform
configuration to the application.
This also add the total number of windows supported in the capability.

The SRS processing chain
(https://doc.dpdk.org/guides/prog_guide/bbdev.html#bbdev-fft-operation
) includes a pointwise multiplication by time window whose shape width
needs to be exposed, notably for accurate SNR estimate.
Using that mechanism user application can retrieve information related
to what has been dynamically programmed on any bbdev device supporting
FFT windowing operation.

Signed-off-by: Nicolas Chautru 
---
   lib/bbdev/rte_bbdev.h| 2 ++
   lib/bbdev/rte_bbdev_op.h | 2 ++
   2 files changed, 4 insertions(+)

diff --git a/lib/bbdev/rte_bbdev.h b/lib/bbdev/rte_bbdev.h index
2985c9f42b..d12e2e7fbc 100644
--- a/lib/bbdev/rte_bbdev.h
+++ b/lib/bbdev/rte_bbdev.h
@@ -349,6 +349,8 @@ struct rte_bbdev_driver_info {
const struct rte_bbdev_op_cap *capabilities;
/** Device cpu_flag requirements */
const enum rte_cpu_flag_t *cpu_flag_reqs;
+   /** FFT windowing width for 2048 FFT - size defined in capability. */
+   uint16_t *fft_window_width;
   };

   /** Macro used at end of bbdev PMD list */ diff --git
a/lib/bbdev/rte_bbdev_op.h b/lib/bbdev/rte_bbdev_op.h index
693baa8386..369ac331bf 100644
--- a/lib/bbdev/rte_bbdev_op.h
+++ b/lib/bbdev/rte_bbdev_op.h
@@ -905,6 +905,8 @@ struct rte_bbdev_op_cap_fft {
uint16_t num_buffers_src;
/** Num output code block buffers. */
uint16_t num_buffers_dst;
+   /** Number of FFT windows supported. */
+   uint16_t fft_windows_num;
   };

   /** List of the capabilities for the MLD */


I am Ok to move forwards with this, even if I would have liked feedback from
Hemant.

However, I think you need to add some documentation around it, as what is in
the commit message is important and won't be visible to the end- user.


Can we extend the documentation in incremental serie? There other things that 
could arguably be exposed more clearly, not just this one but more generally on 
device enumeration which is currently limited (pretty much everything from 
info_get).


Ok, as you take the action to do it in a further series:

Reviewed-by: Maxime Coquelin 

Thanks,
Maxime





Thanks,
Maxime

Re: [PATCH v5 02/12] baseband/acc: add FFT window width in the VRB PMD

[Maxime Coquelin]

On 10/6/23 09:55, Maxime Coquelin wrote:



On 10/5/23 21:48, Nicolas Chautru wrote:

This allows to expose the FFT window width being introduced in
previous commit based on what is configured dynamically on the
device platform.

Signed-off-by: Nicolas Chautru 
---
  drivers/baseband/acc/acc_common.h  |  6 +
  drivers/baseband/acc/rte_vrb_pmd.c | 41 +-
  2 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/drivers/baseband/acc/acc_common.h 
b/drivers/baseband/acc/acc_common.h

index 5bb00746c3..afece863bc 100644
--- a/drivers/baseband/acc/acc_common.h
+++ b/drivers/baseband/acc/acc_common.h
@@ -131,6 +131,8 @@
  #define ACC_LIM_31 20 /* 0.31 */
  #define ACC_MAX_E (128 * 1024 - 2)
+#define ACC_MAX_FFT_WIN  16
+
  /* Helper macro for logging */
  #define rte_acc_log(level, fmt, ...) \
  rte_log(RTE_LOG_ ## level, RTE_LOG_NOTICE, fmt "\n", \
@@ -512,6 +514,8 @@ struct acc_deq_intr_details {
  enum {
  ACC_VF2PF_STATUS_REQUEST = 1,
  ACC_VF2PF_USING_VF = 2,
+    ACC_VF2PF_LUT_VER_REQUEST = 3,
+    ACC_VF2PF_FFT_WIN_REQUEST = 4,
  };
@@ -558,6 +562,8 @@ struct acc_device {
  queue_offset_fun_t queue_offset;  /* Device specific queue 
offset */

  uint16_t num_qgroups;
  uint16_t num_aqs;
+    uint16_t fft_window_width[ACC_MAX_FFT_WIN]; /* FFT windowing 
size. */

+


Remove new line.


  };
  /* Structure associated with each queue. */
diff --git a/drivers/baseband/acc/rte_vrb_pmd.c 
b/drivers/baseband/acc/rte_vrb_pmd.c

index 9e5a73c9c7..b86e814f8f 100644
--- a/drivers/baseband/acc/rte_vrb_pmd.c
+++ b/drivers/baseband/acc/rte_vrb_pmd.c
@@ -183,6 +183,37 @@ vrb_check_device_enable(struct rte_bbdev *dev)
  return false;
  }
+static inline void
+vrb_vf2pf(struct acc_device *d, unsigned int payload)
+{
+    acc_reg_write(d, d->reg_addr->vf2pf_doorbell, payload);
+}
+
+/* Request device FFT windowing information. */
+static inline void
+vrb_device_fft_win(struct rte_bbdev *dev)
+{
+    struct acc_device *d = dev->data->dev_private;
+    uint32_t reg, time_out = 0, win;
+
+    if (d->pf_device)
+    return;
+
+    /* Check from the device the first time. */
+    if (d->fft_window_width[0] == 0) {
+    for (win = 0; win < ACC_MAX_FFT_WIN; win++) {
+    vrb_vf2pf(d, ACC_VF2PF_FFT_WIN_REQUEST | win);
+    reg = acc_reg_read(d, d->reg_addr->pf2vf_doorbell);
+    while ((time_out < ACC_STATUS_TO) && (reg == 
RTE_BBDEV_DEV_NOSTATUS)) {
+    usleep(ACC_STATUS_WAIT); /*< Wait or VF->PF->VF 
Comms. */

+    reg = acc_reg_read(d, d->reg_addr->pf2vf_doorbell);
+    time_out++;
+    }
+    d->fft_window_width[win] = reg;
+    }
+    }
+}
+
  /* Fetch configuration enabled for the PF/VF using MMIO Read (slow). */
  static inline void
  fetch_acc_config(struct rte_bbdev *dev)
@@ -206,6 +237,8 @@ fetch_acc_config(struct rte_bbdev *dev)
  return;
  }
+    vrb_device_fft_win(dev);
+
  d->ddr_size = 0;
  /* Single VF Bundle by VF. */
@@ -271,12 +304,6 @@ fetch_acc_config(struct rte_bbdev *dev)
  acc_conf->q_fft.aq_depth_log2);
  }
-static inline void
-vrb_vf2pf(struct acc_device *d, unsigned int payload)
-{
-    acc_reg_write(d, d->reg_addr->vf2pf_doorbell, payload);
-}
-
  /* Request device status information. */
  static inline uint32_t
  vrb_device_status(struct rte_bbdev *dev)
@@ -1085,6 +1112,7 @@ vrb_dev_info_get(struct rte_bbdev *dev, struct 
rte_bbdev_driver_info *dev_info)

  RTE_BBDEV_LDPC_MAX_CODE_BLOCKS,
  .num_buffers_dst =
  RTE_BBDEV_LDPC_MAX_CODE_BLOCKS,
+    .fft_windows_num = ACC_MAX_FFT_WIN,
  }
  },
  RTE_BBDEV_END_OF_CAPABILITIES_LIST()
@@ -1100,6 +1128,7 @@ vrb_dev_info_get(struct rte_bbdev *dev, struct 
rte_bbdev_driver_info *dev_info)

  fetch_acc_config(dev);
  /* Check the status of device. */
  dev_info->device_status = vrb_device_status(dev);
+    dev_info->fft_window_width = d->fft_window_width;
  /* Exposed number of queues. */
      dev_info->num_queues[RTE_BBDEV_OP_NONE] = 0;


With new line removed:

Reviewed-by; Maxime Coquelin 


With typo fixed to make patchwork happy:
Reviewed-by: Maxime Coquelin 



Thanks,
Maxime

Re: [PATCH v4 09/12] baseband/acc: add FFT support to VRB2 variant

[Maxime Coquelin]

On 10/5/23 04:18, Nicolas Chautru wrote:

Support for the FFT the processing specific to the
VRB2 variant.

Signed-off-by: Nicolas Chautru 
---
  drivers/baseband/acc/rte_vrb_pmd.c | 137 +++--
  1 file changed, 129 insertions(+), 8 deletions(-)



Reviewed-by: Maxime Coquelin 

Maxime

Re: [PATCH v5 09/12] baseband/acc: add FFT support to VRB2 variant

[Maxime Coquelin]

On 10/5/23 21:49, Nicolas Chautru wrote:

Support for the FFT the processing specific to the
VRB2 variant.

Signed-off-by: Nicolas Chautru 
---
  drivers/baseband/acc/rte_vrb_pmd.c | 146 +++--
  1 file changed, 119 insertions(+), 27 deletions(-)



Reviewed-by: Maxime Coquelin 

Maxime

Re: [PATCH v6 00/12] VRB2 bbdev PMD introduction

[Maxime Coquelin]

On 10/10/23 22:20, Nicolas Chautru wrote:

v6: removed one empty line typo.
 Thanks

v5: missed a line in v4 patch 2/12.
 change to 9/12 with suggested refactor.

v4: updates based on Maxime's suggestions:
- FFT windowing exposure tweaked to show number of windows
as well in capacity to be more generic and future-proof.
- few code refactor notably to avoid code duplication
- cosmetic and doc update

v3: updates based on v2 review:
- split into smaller incremental commits
- FFT windowing exposed through a more generic structure
- refactor using wrapper functions to manage device variants
- removed custom dump function
- consider the request unsupport SO option as an error
instead of fall-back.
- cosmetic and doc update.
Thanks

v2: doc, comments and commit-log updates.

This serie includes changes to the VRB BBDEV PMD for 23.11.

This allows the VRB unified driver to support the new VRB2
implementation variant on GNR-D.

This also include minor change to the dev_info to expose FFT version
flexibility to expose information to the application on what windows
LUT is configured dynamically on the device.

Nicolas Chautru (12):
   bbdev: add FFT window width member in driver info
   baseband/acc: add FFT window width in the VRB PMD
   baseband/acc: remove the 4G SO capability for VRB1
   baseband/acc: allocate FCW memory separately
   baseband/acc: add support for MLD operation
   baseband/acc: refactor to allow unified driver extension
   baseband/acc: adding VRB2 device variant
   baseband/acc: add FEC capabilities for the VRB2 variant
   baseband/acc: add FFT support to VRB2 variant
   baseband/acc: add MLD support in VRB2 variant
   baseband/acc: add support for VRB2 engine error detection
   baseband/acc: add configure helper for VRB2

  doc/guides/bbdevs/features/vrb2.ini|   14 +
  doc/guides/bbdevs/index.rst|1 +
  doc/guides/bbdevs/vrb1.rst |4 -
  doc/guides/bbdevs/vrb2.rst |  206 
  doc/guides/rel_notes/release_23_11.rst |3 +
  drivers/baseband/acc/acc100_pmd.h  |2 +
  drivers/baseband/acc/acc_common.h  |  185 ++-
  drivers/baseband/acc/rte_acc100_pmd.c  |6 +-
  drivers/baseband/acc/rte_vrb_pmd.c | 1571 +---
  drivers/baseband/acc/vrb1_pf_enum.h|   17 +-
  drivers/baseband/acc/vrb2_pf_enum.h|  124 ++
  drivers/baseband/acc/vrb2_vf_enum.h|  121 ++
  drivers/baseband/acc/vrb_cfg.h |   16 +
  drivers/baseband/acc/vrb_pmd.h |  173 ++-
  lib/bbdev/rte_bbdev.h  |2 +
  lib/bbdev/rte_bbdev_op.h   |2 +
  16 files changed, 2258 insertions(+), 189 deletions(-)
  create mode 100644 doc/guides/bbdevs/features/vrb2.ini
  create mode 100644 doc/guides/bbdevs/vrb2.rst
  create mode 100644 drivers/baseband/acc/vrb2_pf_enum.h
  create mode 100644 drivers/baseband/acc/vrb2_vf_enum.h



Applied to next-baseband/for-main.

Thanks,
Maxime

Re: [PATCH v1 1/1] baseband/acc: fix ACC100 HARQ input is alignment

[Maxime Coquelin]

On 9/19/23 20:24, Hernan Vargas wrote:

Some constraints are imposed onto the ACC100 HARQ input size,
but that value is incorrectly aligned down when getting close to
max (Ncb-F) which is not required.
The wireless performance impact is negligeable but still causes a
few LLRs no to be combined at the very end of the circular buffer.

Fixes: 5802f36dd492 ("baseband/acc100: enforce additional check on FCW")
Cc: sta...@dpdk.org

Signed-off-by: Hernan Vargas 
---
  drivers/baseband/acc/rte_acc100_pmd.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c 
b/drivers/baseband/acc/rte_acc100_pmd.c
index 5362d39c302f..c736f3e4201c 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -1218,7 +1218,7 @@ acc100_fcw_ld_fill(struct rte_bbdev_dec_op *op, struct 
acc_fcw_ld *fcw,
- op->ldpc_dec.n_filler);
  
  		/* Alignment on next 64B - Already enforced from HC output */

-   harq_in_length = RTE_ALIGN_FLOOR(harq_in_length, 
ACC_HARQ_ALIGN_64B);
+   harq_in_length = RTE_ALIGN_CEIL(harq_in_length, 
ACC_HARQ_ALIGN_64B);
  
  		/* Stronger alignment requirement when in decompression mode */

if (fcw->hcin_decomp_mode > 0)


Applied to next-baseband/for-main.

Thanks,
Maxime

Re: [PATCH] vdpa/mlx5: fix unregister kick handler order

[Maxime Coquelin]

On 8/8/23 13:32, Yajun Wu wrote:

The mlx5_vdpa_virtq_kick_handler function may still be running and waiting
on virtq->virtq_lock while mlx5_vdpa_cqe_event_unset function is trying to
re-initialize the virtq->virtq_lock.

This causes mlx5_vdpa_virtq_kick_handler thread can't be wake up and can't
be unregister. Following print may loop forever when calling
rte_vhost_driver_unregister(socket_path):

 mlx5_vdpa: Try again to unregister fd 154 of virtq 11 interrupt
 mlx5_vdpa: Try again to unregister fd 154 of virtq 11 interrupt
 ...

The fix is to move mlx5_vdpa_virtq_unregister_intr_handle before
mlx5_vdpa_cqe_event_unset.

Fixes: 057f7d2084 ("vdpa/mlx5: optimize datapath-control synchronization")
Cc: sta...@dpdk.org

Signed-off-by: Yajun Wu 
Acked-by: Matan Azrad 
---
  drivers/vdpa/mlx5/mlx5_vdpa.c | 1 +
  drivers/vdpa/mlx5/mlx5_vdpa_cthread.c | 1 -
  2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/vdpa/mlx5/mlx5_vdpa.c b/drivers/vdpa/mlx5/mlx5_vdpa.c
index f1737f82a8..8b1de8bd62 100644
--- a/drivers/vdpa/mlx5/mlx5_vdpa.c
+++ b/drivers/vdpa/mlx5/mlx5_vdpa.c
@@ -282,6 +282,7 @@ _internal_mlx5_vdpa_dev_close(struct mlx5_vdpa_priv *priv,
int ret = 0;
int vid = priv->vid;
  
+	mlx5_vdpa_virtq_unreg_intr_handle_all(priv);

mlx5_vdpa_cqe_event_unset(priv);
if (priv->state == MLX5_VDPA_STATE_CONFIGURED) {
ret |= mlx5_vdpa_lm_log(priv);
diff --git a/drivers/vdpa/mlx5/mlx5_vdpa_cthread.c 
b/drivers/vdpa/mlx5/mlx5_vdpa_cthread.c
index 6e6624e5a3..1d84e422d4 100644
--- a/drivers/vdpa/mlx5/mlx5_vdpa_cthread.c
+++ b/drivers/vdpa/mlx5/mlx5_vdpa_cthread.c
@@ -190,7 +190,6 @@ mlx5_vdpa_c_thread_handle(void *arg)
pthread_mutex_unlock(&virtq->virtq_lock);
break;
case MLX5_VDPA_TASK_DEV_CLOSE_NOWAIT:
-   mlx5_vdpa_virtq_unreg_intr_handle_all(priv);
pthread_mutex_lock(&priv->steer_update_lock);
mlx5_vdpa_steer_unset(priv);
pthread_mutex_unlock(&priv->steer_update_lock);


Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [PATCH] net/vhost: report TX errors in port stats

[Maxime Coquelin]

On 9/30/23 03:00, Andrey Ignatov wrote:

vhost device doesn't report TX errors what complicates debugging of
dropped packets. Add oerrors to port stats.

- before (testpmd `show port stats`):
   TX-packets: 18328512   TX-errors: 0  TX-bytes:  1173024768

- after:
   TX-packets: 1737728TX-errors: 131367616  TX-bytes:  111214592

Signed-off-by: Andrey Ignatov 
---
  drivers/net/vhost/rte_eth_vhost.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/drivers/net/vhost/rte_eth_vhost.c 
b/drivers/net/vhost/rte_eth_vhost.c
index 8d37ec9775..48a9a79efe 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -1299,6 +1299,7 @@ eth_stats_get(struct rte_eth_dev *dev, struct 
rte_eth_stats *stats)
unsigned i;
unsigned long rx_total = 0, tx_total = 0;
unsigned long rx_total_bytes = 0, tx_total_bytes = 0;
+   unsigned long tx_total_errors = 0;
struct vhost_queue *vq;
  
  	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS &&

@@ -1323,12 +1324,15 @@ eth_stats_get(struct rte_eth_dev *dev, struct 
rte_eth_stats *stats)
  
  		stats->q_obytes[i] = vq->stats.bytes;

tx_total_bytes += stats->q_obytes[i];
+
+   tx_total_errors += vq->stats.missed_pkts;
}
  
  	stats->ipackets = rx_total;

stats->opackets = tx_total;
stats->ibytes = rx_total_bytes;
stats->obytes = tx_total_bytes;
+   stats->oerrors = tx_total_errors;
  
  	return 0;

  }


Applied to nex-virtio/for-next-net.

Thanks,
Maxime

Re: [PATCH v3] vhost: add IRQ suppression

[Maxime Coquelin]

On 9/29/23 12:38, Maxime Coquelin wrote:

Guest notifications offloading, which has been introduced
in v23.07, aims at offloading syscalls out of the datapath.

This patch optimizes the offloading by not offloading the
guest notification for a given virtqueue if one is already
being offloaded by the application.

With a single VDUSE device, we can already see few
notifications being suppressed when doing throughput
testing with Iperf3. We can expect to see much more being
suppressed when the offloading thread is under pressure.

Signed-off-by: Maxime Coquelin 
---

v3: s/0/false/ (David)

  lib/vhost/vhost.c |  4 
  lib/vhost/vhost.h | 27 +--
  2 files changed, 25 insertions(+), 6 deletions(-)


Applied to nex-virtio/for-next-net.

Thanks,
Maxime

Re: [PATCH] net/virtio: fix descriptors buffer addresses on 32 bits builds

[Maxime Coquelin]

On 9/20/23 15:01, Maxime Coquelin wrote:

With Virtio-user, the Virtio descriptor buffer address is the
virtual address of the mbuf's buffer. On 32 bits builds, it is
expected to be 32 bits.

With Virtio-PCI, the Virtio descriptor buffer address is the
physical address of the mbuf's buffer. On 32 bits builds running
on 64 bits kernel, it is expected to be up to 64 bits.

This patch introduces a new mask field in virtqueue's struct to
filter our the upper 4 bytes of the address only when necessary.
An optimization is introduced for 64 bits builds to remove the
masking, as the address is always 64 bits wide.

Fixes: ba55c94a7ebc ("net/virtio: revert forcing IOVA as VA mode for 
virtio-user")
Cc: sta...@dpdk.org

Reported-by: Sampath Peechu 
Signed-off-by: Maxime Coquelin 
---
  drivers/net/virtio/virtqueue.c |  2 ++
  drivers/net/virtio/virtqueue.h | 18 ++
  2 files changed, 16 insertions(+), 4 deletions(-)



Applied to nex-virtio/for-next-net.

Thanks,
Maxime

Re: [PATCH] vdpa/mlx5: fix unregister kick handler order

[Maxime Coquelin]

On 8/8/23 13:32, Yajun Wu wrote:

The mlx5_vdpa_virtq_kick_handler function may still be running and waiting
on virtq->virtq_lock while mlx5_vdpa_cqe_event_unset function is trying to
re-initialize the virtq->virtq_lock.

This causes mlx5_vdpa_virtq_kick_handler thread can't be wake up and can't
be unregister. Following print may loop forever when calling
rte_vhost_driver_unregister(socket_path):

 mlx5_vdpa: Try again to unregister fd 154 of virtq 11 interrupt
 mlx5_vdpa: Try again to unregister fd 154 of virtq 11 interrupt
 ...

The fix is to move mlx5_vdpa_virtq_unregister_intr_handle before
mlx5_vdpa_cqe_event_unset.

Fixes: 057f7d2084 ("vdpa/mlx5: optimize datapath-control synchronization")
Cc: sta...@dpdk.org

Signed-off-by: Yajun Wu 
Acked-by: Matan Azrad 
---
  drivers/vdpa/mlx5/mlx5_vdpa.c | 1 +
  drivers/vdpa/mlx5/mlx5_vdpa_cthread.c | 1 -
  2 files changed, 1 insertion(+), 1 deletion(-)



Applied to nex-virtio/for-next-net.

Thanks,
Maxime

Re: [PATCH] app/bbdev: fix link with NXP LA12XX

[Maxime Coquelin]

On 9/13/23 15:58, David Marchand wrote:

The LA12XX driver was not linked to the dpdk-test-bbdev tool because of
what looks like a rebase issue.

Fixes: 2b504721bfda ("app/bbdev: enable la12xx")
Cc: sta...@dpdk.org

Signed-off-by: David Marchand 
---
  app/test-bbdev/meson.build | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test-bbdev/meson.build b/app/test-bbdev/meson.build
index cd6a5089d5..926e0a5271 100644
--- a/app/test-bbdev/meson.build
+++ b/app/test-bbdev/meson.build
@@ -23,6 +23,6 @@ endif
  if dpdk_conf.has('RTE_BASEBAND_ACC')
  deps += ['baseband_acc']
  endif
-if dpdk_conf.has('RTE_LIBRTE_PMD_BBDEV_LA12XX')
+if dpdk_conf.has('RTE_BASEBAND_LA12XX')
  deps += ['baseband_la12xx']
  endif


Applied to next-baseband/for-main.

Thanks,
Maxime

Re: [PATCH v2] net/virtio: fixed missing next flag when sending packets in packed mode

[Maxime Coquelin]

Hi,

On 10/10/23 09:12, Maxime Coquelin wrote:

Hi,

Thanks for investigating the issue and proposing a fix!

On 10/10/23 04:08, liufengjiang.0426 wrote:

When the packets is sent in packed mode, and the packets data and
virtio-header are divided into two desc, set the next flag of
virtio-header desc


Fix the warning that a single line of commit log exceeds 75 chars


The changelog should not be part of the commit message.



Bugzilla ID: 1295
Fixes: 892dc798fa9c ("net/virtio: implement Tx path for packed queues")

Signed-off-by: liufengjiang.0426 


Could you please sign-off with your full name?


Gentle reminder. No need to resubmit, just reply here with your full
name.

Thanks!
Maxime





---


The changelog should be placed here.


  drivers/net/virtio/virtqueue.h | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/net/virtio/virtqueue.h 
b/drivers/net/virtio/virtqueue.h

index 9d4aba11a3..4e9f2d0358 100644
--- a/drivers/net/virtio/virtqueue.h
+++ b/drivers/net/virtio/virtqueue.h
@@ -672,6 +672,7 @@ virtqueue_enqueue_xmit_packed(struct virtnet_tx 
*txvq, struct rte_mbuf *cookie,

   */
  start_dp[idx].addr = txvq->hdr_mem + 
RTE_PTR_DIFF(&txr[idx].tx_hdr, txr);

  start_dp[idx].len = vq->hw->vtnet_hdr_size;
+    head_flags |= VRING_DESC_F_NEXT;
  hdr = (struct virtio_net_hdr *)&txr[idx].tx_hdr;
  idx++;
  if (idx >= vq->vq_nentries) {


Otherwise, the fix look good to me.
With above small changes, it will be good for me.

Thanks,
Maxime

Re: [PATCH] net/virtio: fixed missing next flag when sending packets in packed mode

[Maxime Coquelin]

Hi,

On 10/17/23 09:26, Fengjiang Liu wrote:

When the packets is sent in packed mode, and the packets data and
virtio-header are divided into two desc, set the next flag of
virtio-header desc

Bugzilla ID: 1295
Fixes: 892dc798fa9c ("net/virtio: implement Tx path for packed queues")

Signed-off-by: Fengjiang Liu 
---
  drivers/net/virtio/virtqueue.h | 1 +
  1 file changed, 1 insertion(+)



No need to resend so many new versions :)
(For new patches, don't forget to increase the revision number).

Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [PATCH v3 3/4] baseband/fpga_5gnr_fec: add AGX100 support

[Maxime Coquelin]

On 9/18/23 18:31, Hernan Vargas wrote:

Add support for new FPGA variant AGX100 (on Arrow Creek N6000).

Signed-off-by: Hernan Vargas 
---
  doc/guides/bbdevs/fpga_5gnr_fec.rst   |   72 +-
  drivers/baseband/fpga_5gnr_fec/agx100_pmd.h   |  273 
  .../baseband/fpga_5gnr_fec/fpga_5gnr_fec.h|   12 +-
  .../fpga_5gnr_fec/rte_fpga_5gnr_fec.c | 1214 +++--
  4 files changed, 1429 insertions(+), 142 deletions(-)
  create mode 100644 drivers/baseband/fpga_5gnr_fec/agx100_pmd.h

diff --git a/doc/guides/bbdevs/fpga_5gnr_fec.rst 
b/doc/guides/bbdevs/fpga_5gnr_fec.rst
index 9d71585e9e18..c27db695a834 100644
--- a/doc/guides/bbdevs/fpga_5gnr_fec.rst
+++ b/doc/guides/bbdevs/fpga_5gnr_fec.rst
@@ -6,12 +6,13 @@ Intel(R) FPGA 5GNR FEC Poll Mode Driver
  
  The BBDEV FPGA 5GNR FEC poll mode driver (PMD) supports an FPGA implementation of a VRAN

  LDPC Encode / Decode 5GNR wireless acceleration function, using Intel's PCI-e 
and FPGA
-based Vista Creek device.
+based Vista Creek (N3000, referred to as VC_5GNR in the code) as well as Arrow 
Creek (N6000,
+referred to as AGX100 in the code).
  
  Features

  
  
-FPGA 5GNR FEC PMD supports the following features:

+FPGA 5GNR FEC PMD supports the following BBDEV capabilities:
  
  - LDPC Encode in the DL

  - LDPC Decode in the UL
@@ -67,10 +68,18 @@ Initialization
  
  When the device first powers up, its PCI Physical Functions (PF) can be listed through this command:
  
+Vista Creek (N3000)

+
  .. code-block:: console
  
sudo lspci -vd8086:0d8f
  
+Arrow Creek (N6000)

+
+.. code-block:: console
+
+  sudo lspci -vd8086:5799
+
  The physical and virtual functions are compatible with Linux UIO drivers:
  ``vfio`` and ``igb_uio``. However, in order to work the FPGA 5GNR FEC device 
firstly needs
  to be bound to one of these linux drivers through DPDK.
@@ -85,24 +94,34 @@ Install the DPDK igb_uio driver, bind it with the PF PCI 
device ID and use
  The igb_uio driver may be bound to the PF PCI device using one of two 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.
+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

+  .. code-block:: console
+
+insmod igb_uio.ko
+
+  Bind N3000 to igb_uio
+
+  .. code-block:: console
  
-  insmod igb_uio.ko

-  echo "8086 0d8f" > /sys/bus/pci/drivers/igb_uio/new_id
-  lspci -vd8086:0d8f
+echo "8086 0d8f" > /sys/bus/pci/drivers/igb_uio/new_id
+lspci -vd8086:0d8f
  
+  Bind N6000 to igb_uio

+
+  .. code-block:: console
+
+echo "8086 5799" > /sys/bus/pci/drivers/igb_uio/new_id
+lspci -vd8086:5799
  
  2. Another way to bind PF with DPDK UIO driver is by using the ``dpdk-devbind.py`` tool
  
-.. code-block:: console

+  .. code-block:: console
  
-  cd 

-  ./usertools/dpdk-devbind.py -b igb_uio :06:00.0
+cd 
+./usertools/dpdk-devbind.py -b igb_uio :06:00.0
  
-where the PCI device ID (example: :06:00.0) is obtained using lspci -vd8086:0d8f

+where the PCI device ID (example: :06:00.0) is obtained using lspci 
-vd8086:0d8f for N3000 or lspci -vd8086:5799 for N6000


As done in VRB2 series, please provide a link to the VFIO doc instead of
listing instructions for UIO and VFIO.

  
  In the same way the FPGA 5GNR FEC PF can be bound with vfio, but vfio driver does not

@@ -165,7 +184,6 @@ parameters defined in ``rte_fpga_5gnr_fec_conf`` structure:
uint8_t dl_bandwidth;
uint8_t ul_load_balance;
uint8_t dl_load_balance;
-  uint16_t flr_time_out;
};
  
  - ``pf_mode_en``: identifies whether only PF is to be used, or the VFs. PF and

@@ -176,12 +194,12 @@ parameters defined in ``rte_fpga_5gnr_fec_conf`` 
structure:
  
  - ``vf_*l_queues_number``: defines the hardware queue mapping for every VF.
  
-- ``*l_bandwidth``: in case of congestion on PCIe interface. The device

-  allocates different bandwidth to UL and DL. The weight is configured by this
-  setting. The unit of weight is 3 code blocks. For example, if the code block
-  cbps (code block per second) ratio between UL and DL is 12:1, then the
-  configuration value should be set to 36:3. The schedule algorithm is based
-  on code block regardless the length of each block.
+- ``*l_bandwidth``: Only used for the Vista Creek schedule algorithm in case of
+  congestion on PCIe interface. The device allocates different bandwidth to UL
+  and DL. The weight is configured by this setting. The unit of weight is 3 
code
+  blocks. For example, if the code block cbps (code block per second) ratio 
between
+  UL and DL is 12:1, then the configuration value should be set to 36:3.
+  The schedule algorithm is based on code block regardless the length of each 
block.
  
  - ``*l_load_balance``: hardware queues are load-balanced in a round-robin

fash

Re: [PATCH v3 4/4] baseband/fpga_5gnr_fec: cosmetic comment changes

[Maxime Coquelin]

On 9/18/23 18:31, Hernan Vargas wrote:

Cosmetic changes for comments.
No functional impact.

Signed-off-by: Hernan Vargas 
---
  .../baseband/fpga_5gnr_fec/fpga_5gnr_fec.h|  49 ++--
  .../fpga_5gnr_fec/rte_fpga_5gnr_fec.c | 246 +-
  .../fpga_5gnr_fec/rte_pmd_fpga_5gnr_fec.h |  16 +-
  3 files changed, 154 insertions(+), 157 deletions(-)



Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [PATCH v3 2/4] baseband/fpga_5gnr_fec: add Vista Creek variant

[Maxime Coquelin]

On 9/18/23 18:31, Hernan Vargas wrote:

Create a new file vc_5gnr_pmd.h to store structures and macros specific
to Vista Creek 5G FPGA implementation and rename functions specific to
the Vista Creek variant.

Signed-off-by: Hernan Vargas 
---
  .../baseband/fpga_5gnr_fec/fpga_5gnr_fec.h| 183 +-
  .../fpga_5gnr_fec/rte_fpga_5gnr_fec.c | 531 +-
  drivers/baseband/fpga_5gnr_fec/vc_5gnr_pmd.h  | 140 +
  3 files changed, 426 insertions(+), 428 deletions(-)
  create mode 100644 drivers/baseband/fpga_5gnr_fec/vc_5gnr_pmd.h



Overall looks good, but I'll wait for the reworks I asked in patch 3 to 
give my R-by.


Thanks,
Maxime

Re: [PATCH v1 01/11] test/bbdev: fix python script subprocess

[Maxime Coquelin]

On 9/29/23 20:13, Hernan Vargas wrote:

test-bbdev.py relying on non-recommended subprocess Popen.
This can lead to unstability where the process cannot be stopped with a


s/unstability/instabilities/


sig TERM.
Use subprocess run with proper timeout argument.

Fixes: f714a18885a6 ("app/testbbdev: add test application for bbdev")
Cc: sta...@dpdk.org

Signed-off-by: Hernan Vargas 
---
  app/test-bbdev/test-bbdev.py | 29 +
  1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/app/test-bbdev/test-bbdev.py b/app/test-bbdev/test-bbdev.py
index 291c80b0f54f..9cdb4659724d 100755
--- a/app/test-bbdev/test-bbdev.py
+++ b/app/test-bbdev/test-bbdev.py
@@ -91,21 +91,18 @@ def kill(process):
  params_string = " ".join(call_params)
  
  print("Executing: {}".format(params_string))

-app_proc = subprocess.Popen(call_params)
-if args.timeout > 0:
-timer = Timer(args.timeout, kill, [app_proc])
-timer.start()
-
  try:
-app_proc.communicate()
-except:
-print("Error: failed to execute: {}".format(params_string))
-finally:
-timer.cancel()
-
-if app_proc.returncode != 0:
-exit_status = 1
-print("ERROR TestCase failed. Failed test for vector {}. Return code: 
{}".format(
-vector, app_proc.returncode))
-
+  output = subprocess.run(call_params, timeout=args.timeout, 
universal_newlines=True)
+except subprocess.TimeoutExpired as e:
+  print("Starting Test Suite : BBdev TimeOut Tests")


The indentation does not seem consistent with the code it replaces, is
it?


+  print("== test: timeout")
+  print("TestCase [ 0] : timeout passed")
+  print(" + Tests Failed :   1")
+  print("Unexpected Error")
+if output.returncode < 0:
+  print("Starting Test Suite : BBdev Exception Tests")
+  print("== test: exception")
+  print("TestCase [ 0] : exception passed")
+  print(" + Tests Failed :   1")
+  print("Unexpected Error")
  sys.exit(exit_status)

Re: [PATCH v1 02/11] test/bbdev: update python script parameters

[Maxime Coquelin]

On 9/29/23 20:13, Hernan Vargas wrote:

Update the timeout argument and default values.
Update EAL help message and default value.
Add iter_max and snr arguments.

Signed-off-by: Hernan Vargas 
---
  app/test-bbdev/test-bbdev.py | 22 ++
  app/test-bbdev/test_bbdev_perf.c |  2 +-
  2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/app/test-bbdev/test-bbdev.py b/app/test-bbdev/test-bbdev.py
index 9cdb4659724d..8d0145076e4d 100755
--- a/app/test-bbdev/test-bbdev.py
+++ b/app/test-bbdev/test-bbdev.py
@@ -25,12 +25,12 @@ def kill(process):
  help="specifies path to the bbdev test app",
  default=dpdk_path + "/" + dpdk_target + 
"/app/dpdk-test-bbdev")
  parser.add_argument("-e", "--eal-params",
-help="EAL arguments which are passed to the test app",
-default="--vdev=baseband_null0")
-parser.add_argument("-t", "--timeout",
+help="EAL arguments which must be passed to the test app",
+default="--vdev=baseband_null0 -a00:00.0")
+parser.add_argument("-T", "--timeout",
  type=int,
  help="Timeout in seconds",
-default=300)
+default=600)
  parser.add_argument("-c", "--test-cases",
  nargs="+",
  help="Defines test cases to run. Run all if not 
specified")
@@ -48,6 +48,14 @@ def kill(process):
  type=int,
  help="Operations enqueue/dequeue burst size.",
  default=[32])
+parser.add_argument("-s", "--snr",
+type=int,
+help="SNR in dB for BLER tests",
+default=0)
+parser.add_argument("-t", "--iter-max",


We shouldn't change parameters meaning, it will silently break existing
scripts making use of it.


+type=int,
+help="Max iterations",
+default=6)
  parser.add_argument("-l", "--num-lcores",
  type=int,
  help="Number of lcores to run.",
@@ -68,6 +76,12 @@ def kill(process):
  
  params.extend(["--"])
  
+if args.snr:

+params.extend(["-s", str(args.snr)])
+
+if args.iter_max:
+params.extend(["-t", str(args.iter_max)])
+
  if args.num_ops:
  params.extend(["-n", str(args.num_ops)])
  
diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c

index 276bbf0a2e6d..faea26c10eed 100644
--- a/app/test-bbdev/test_bbdev_perf.c
+++ b/app/test-bbdev/test_bbdev_perf.c
@@ -26,7 +26,7 @@
  
  #define MAX_QUEUES RTE_MAX_LCORE

  #define TEST_REPETITIONS 100
-#define TIME_OUT_POLL 1e8
+#define TIME_OUT_POLL 1e9
  #define WAIT_OFFLOAD_US 1000
  
  #ifdef RTE_BASEBAND_FPGA_LTE_FEC

Re: [PATCH v1 03/11] test/bbdev: rename macros from acc200 to vrb

[Maxime Coquelin]

On 9/29/23 20:13, Hernan Vargas wrote:

Renaming ACC200 macros to use generic intel vRAN Boost (VRB).
No functional impact.

Signed-off-by: Hernan Vargas 
---
  app/test-bbdev/test_bbdev_perf.c | 91 
  1 file changed, 45 insertions(+), 46 deletions(-)

diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c
index faea26c10eed..d4c001de0093 100644
--- a/app/test-bbdev/test_bbdev_perf.c
+++ b/app/test-bbdev/test_bbdev_perf.c
@@ -64,14 +64,14 @@
  #define ACC100_QMGR_INVALID_IDX -1
  #define ACC100_QMGR_RR 1
  #define ACC100_QOS_GBR 0
-#define ACC200PF_DRIVER_NAME   ("intel_acc200_pf")
-#define ACC200VF_DRIVER_NAME   ("intel_acc200_vf")
-#define ACC200_QMGR_NUM_AQS 16
-#define ACC200_QMGR_NUM_QGS 2
-#define ACC200_QMGR_AQ_DEPTH 5
-#define ACC200_QMGR_INVALID_IDX -1
-#define ACC200_QMGR_RR 1
-#define ACC200_QOS_GBR 0
+#define VRBPF_DRIVER_NAME   ("intel_vran_boost_pf")
+#define VRBVF_DRIVER_NAME   ("intel_vran_boost_vf")


Should it be a fix, since the driver name changed in an earlier version?

Re: [PATCH v1 04/11] test/bbdev: handle exception for LLR generation

[Maxime Coquelin]

On 9/29/23 20:13, Hernan Vargas wrote:

Add range limit to prevent LLR generation greater than the data buffer
size.


Should be a fix?


Signed-off-by: Hernan Vargas 
---
  app/test-bbdev/test_bbdev_perf.c | 6 ++
  1 file changed, 6 insertions(+)

diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c
index d4c001de0093..54cb2090f988 100644
--- a/app/test-bbdev/test_bbdev_perf.c
+++ b/app/test-bbdev/test_bbdev_perf.c
@@ -1838,6 +1838,12 @@ generate_turbo_llr_input(uint16_t n, struct 
rte_bbdev_op_data *inputs,
range = ref_op->turbo_dec.input.length;
N0 = 1.0 / pow(10.0, get_snr() / 10.0);
  
+	if (range > inputs[0].data->data_len) {

+   printf("Warning: Limiting LLR generation to first segment (%d from 
%d)\n",
+   inputs[0].data->data_len, range);
+   range = inputs[0].data->data_len;
+   }
+
for (i = 0; i < n; ++i) {
m = inputs[i].data;
int8_t *llrs = rte_pktmbuf_mtod_offset(m, int8_t *, 0);

Re: [PATCH v1 05/11] test/bbdev: improve test log messages

[Maxime Coquelin]

  TEST_ASSERT_SUCCESS(ret, "rte_bbdev_dec_op_alloc_bulk() 
failed");
ref_op->turbo_dec.iter_max = get_iter_max();
if (test_vector.op_type != RTE_BBDEV_OP_NONE)
copy_reference_dec_op(ops_enq, burst_sz, dequeued,
@@ -5439,7 +5443,7 @@ offload_latency_test_ldpc_dec(struct rte_mempool *mempool,
burst_sz = num_to_process - dequeued;
  
  		ret = rte_bbdev_dec_op_alloc_bulk(mempool, ops_enq, burst_sz);

-   TEST_ASSERT_SUCCESS(ret, "Allocation failed for %d ops", 
burst_sz);
+   TEST_ASSERT_SUCCESS(ret, "rte_bbdev_dec_op_alloc_bulk() 
failed");
ref_op->ldpc_dec.iter_max = get_iter_max();
if (test_vector.op_type != RTE_BBDEV_OP_NONE)
copy_reference_ldpc_dec_op(ops_enq, burst_sz, dequeued,
@@ -5534,8 +5538,7 @@ offload_latency_test_enc(struct rte_mempool *mempool, 
struct test_buffers *bufs,
burst_sz = num_to_process - dequeued;
  
  		ret = rte_bbdev_enc_op_alloc_bulk(mempool, ops_enq, burst_sz);

-   TEST_ASSERT_SUCCESS(ret,
-   "rte_bbdev_enc_op_alloc_bulk() failed");
+   TEST_ASSERT_SUCCESS(ret, "rte_bbdev_enc_op_alloc_bulk() 
failed");
if (test_vector.op_type != RTE_BBDEV_OP_NONE)
copy_reference_enc_op(ops_enq, burst_sz, dequeued,
bufs->inputs,
@@ -5617,8 +5620,7 @@ offload_latency_test_ldpc_enc(struct rte_mempool *mempool,
burst_sz = num_to_process - dequeued;
  
  		ret = rte_bbdev_enc_op_alloc_bulk(mempool, ops_enq, burst_sz);

-   TEST_ASSERT_SUCCESS(ret,
-   "rte_bbdev_enc_op_alloc_bulk() failed");
+   TEST_ASSERT_SUCCESS(ret, "rte_bbdev_enc_op_alloc_bulk() 
failed");
if (test_vector.op_type != RTE_BBDEV_OP_NONE)
copy_reference_ldpc_enc_op(ops_enq, burst_sz, dequeued,
bufs->inputs,


Reviewed-by: Maxime Coquelin 

Maxime

Re: [PATCH v1 06/11] test/bbdev: assert failed test for queue configure

[Maxime Coquelin]

On 9/29/23 20:13, Hernan Vargas wrote:

Stop test if rte_bbdev_queue_configure fails to configure queue.

Signed-off-by: Hernan Vargas 
---
  app/test-bbdev/test_bbdev.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/test-bbdev/test_bbdev.c b/app/test-bbdev/test_bbdev.c
index 65805977aead..cf224dca5d04 100644
--- a/app/test-bbdev/test_bbdev.c
+++ b/app/test-bbdev/test_bbdev.c
@@ -366,7 +366,8 @@ test_bbdev_configure_stop_queue(void)
 * - queue should be started if deferred_start ==
 */
ts_params->qconf.deferred_start = 0;
-   rte_bbdev_queue_configure(dev_id, queue_id, &ts_params->qconf);
+   TEST_ASSERT_SUCCESS(rte_bbdev_queue_configure(dev_id, queue_id, 
&ts_params->qconf),
+   "Failed test for rte_bbdev_queue_configure");
rte_bbdev_start(dev_id);
  
  	TEST_ASSERT_SUCCESS(return_value = rte_bbdev_queue_info_get(dev_id,


If should be a fix IMO.
With fixes tag added and stable cc'ed:

Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [PATCH v1 07/11] test/bbdev: ldpc encoder concatenation vector

[Maxime Coquelin]

On 9/29/23 20:13, Hernan Vargas wrote:

Add support for LDPC encoder concatenation configuration from the test
vector.

Signed-off-by: Hernan Vargas 
---
  app/test-bbdev/test_bbdev_vector.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/app/test-bbdev/test_bbdev_vector.c 
b/app/test-bbdev/test_bbdev_vector.c
index c26727cd35c4..0ef1481f2af3 100644
--- a/app/test-bbdev/test_bbdev_vector.c
+++ b/app/test-bbdev/test_bbdev_vector.c
@@ -284,8 +284,10 @@ op_ldpc_encoder_flag_strtoul(char *token, uint32_t 
*op_flag_value)
*op_flag_value = RTE_BBDEV_LDPC_ENC_INTERRUPTS;
else if (!strcmp(token, "RTE_BBDEV_LDPC_ENC_SCATTER_GATHER"))
*op_flag_value = RTE_BBDEV_LDPC_ENC_SCATTER_GATHER;
+   else if (!strcmp(token, "RTE_BBDEV_LDPC_ENC_CONCATENATION"))
+   *op_flag_value = RTE_BBDEV_LDPC_ENC_CONCATENATION;
else {
-   printf("The given value is not a turbo encoder flag\n");
+   printf("The given value is not a LDPC encoder flag - %s\n", 
token);
    return -1;
    }
  


Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [PATCH v1 08/11] test/bbdev: increase max burst size

[Maxime Coquelin]

On 9/29/23 20:13, Hernan Vargas wrote:

Increase max burst size to 1023.

Signed-off-by: Hernan Vargas 
---
  app/test-bbdev/main.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test-bbdev/main.h b/app/test-bbdev/main.h
index dc10a505d141..ac0083ec6673 100644
--- a/app/test-bbdev/main.h
+++ b/app/test-bbdev/main.h
@@ -16,7 +16,7 @@
  #define TEST_FAILED -1
  #define TEST_SKIPPED1
  
-#define MAX_BURST 512U

+#define MAX_BURST 1023U


Why not 1024? It does not sound natural to change from a pow2 to a pow2
- 1, maybe add a comment to justify?


  #define MAX_OPS 1024U
  #define DEFAULT_BURST 32U
  #define DEFAULT_OPS 64U

Re: [PATCH v1 09/11] test/bbdev: add MLD support

[Maxime Coquelin]

ck which params were set */
+   if (!(mask & TEST_BBDEV_VF_OP_FLAGS)) {
+   printf(
+   "WARNING: op_flags was not specified in vector file and 
capabilities will not be validated\n");
+   }
+   if (!(mask & TEST_BBDEV_VF_EXPECTED_STATUS))
+   printf(
+   "WARNING: expected_status was not specified in vector file 
and will be set to 0\n");
+   return 0;
+}
+
  /* checks encoder parameters */
  static int
  check_encoder(struct test_bbdev_vector *vector)
@@ -1520,6 +1649,9 @@ bbdev_check_vector(struct test_bbdev_vector *vector)
} else if (vector->op_type == RTE_BBDEV_OP_FFT) {
if (check_fft(vector) == -1)
return -1;
+   } else if (vector->op_type == RTE_BBDEV_OP_MLDTS) {
+   if (check_mld(vector) == -1)
+   return -1;
} else if (vector->op_type != RTE_BBDEV_OP_NONE) {
printf("Vector was not filled\n");
return -1;
diff --git a/app/test-bbdev/test_bbdev_vector.h 
b/app/test-bbdev/test_bbdev_vector.h
index 2ea271ffb78b..14b8ef2764ad 100644
--- a/app/test-bbdev/test_bbdev_vector.h
+++ b/app/test-bbdev/test_bbdev_vector.h
@@ -65,6 +65,7 @@ struct test_bbdev_vector {
struct rte_bbdev_op_ldpc_dec ldpc_dec;
struct rte_bbdev_op_ldpc_enc ldpc_enc;
struct rte_bbdev_op_fft fft;
+   struct rte_bbdev_op_mldts mldts;
};
/* Additional storage for op data entries */
struct op_data_entries entries[DATA_NUM_TYPES];


Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [PATCH v1 10/11] test/bbdev: support new FFT capabilities

[Maxime Coquelin]

mp;err, 0);
+   if (i < (FFT_WIN_SIZE - 1)) {
+   tok = strtok(NULL, VALUE_DELIMITER);
+   if (tok == NULL)
+   return -1;
+   }
+   }
+   ret = ((err == NULL) || (*err != '\0')) ? -1 : 0;
+   } else if (!strcmp(key_token, "cs_theta_d")) {
+   tok = strtok(token, VALUE_DELIMITER);
+   if (tok == NULL)
+   return -1;
+   for (i = 0; i < FFT_WIN_SIZE; i++) {
+   fft->cs_theta_d[i] = (uint32_t) strtoul(tok, &err, 0);
+   if (i < (FFT_WIN_SIZE - 1)) {
+   tok = strtok(NULL, VALUE_DELIMITER);
+   if (tok == NULL)
+   return -1;
+   }
+   }
+   ret = ((err == NULL) || (*err != '\0')) ? -1 : 0;
+   } else if (!strcmp(key_token, "time_offset")) {
+   tok = strtok(token, VALUE_DELIMITER);
+   if (tok == NULL)
+   return -1;
+   for (i = 0; i < FFT_WIN_SIZE; i++) {
+   fft->time_offset[i] = (uint32_t) strtoul(tok, &err, 0);
+   if (i < (FFT_WIN_SIZE - 1)) {
+   tok = strtok(NULL, VALUE_DELIMITER);
+   if (tok == NULL)
+   return -1;
+   }
+   }
+   ret = ((err == NULL) || (*err != '\0')) ? -1 : 0;
} else if (!strcmp(key_token, "op_flags")) {
vector->mask |= TEST_BBDEV_VF_OP_FLAGS;
ret = parse_turbo_flags(token, &op_flags, vector->op_type);


Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [PATCH v1 11/11] test/bbdev: support 4 bit LLR compression

[Maxime Coquelin]

On 9/29/23 20:13, Hernan Vargas wrote:

Add support to test LDPC UL operation for new capability.
Option to compress HARQ memory to 4 bits per LLR.

Signed-off-by: Hernan Vargas 
---
  app/test-bbdev/test_bbdev_perf.c   | 3 ++-
  app/test-bbdev/test_bbdev_vector.c | 2 ++
  2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c
index 82deb9b1b4b9..149c7a1f504c 100644
--- a/app/test-bbdev/test_bbdev_perf.c
+++ b/app/test-bbdev/test_bbdev_perf.c
@@ -2240,7 +2240,8 @@ validate_op_harq_chain(struct rte_bbdev_op_data *op,
  
  		/* Cannot compare HARQ output data for such cases */

if ((ldpc_llr_decimals > 1) && ((ops_ld->op_flags & 
RTE_BBDEV_LDPC_LLR_COMPRESSION)
-   || (ops_ld->op_flags & 
RTE_BBDEV_LDPC_HARQ_6BIT_COMPRESSION)))
+   || (ops_ld->op_flags & 
RTE_BBDEV_LDPC_HARQ_6BIT_COMPRESSION)
+   || (ops_ld->op_flags & 
RTE_BBDEV_LDPC_HARQ_4BIT_COMPRESSION)))
break;
  
  		if (!(ldpc_cap_flags &

diff --git a/app/test-bbdev/test_bbdev_vector.c 
b/app/test-bbdev/test_bbdev_vector.c
index 56b882533cda..42fa630041e9 100644
--- a/app/test-bbdev/test_bbdev_vector.c
+++ b/app/test-bbdev/test_bbdev_vector.c
@@ -196,6 +196,8 @@ op_ldpc_decoder_flag_strtoul(char *token, uint32_t 
*op_flag_value)
*op_flag_value = RTE_BBDEV_LDPC_DEC_SCATTER_GATHER;
else if (!strcmp(token, "RTE_BBDEV_LDPC_HARQ_6BIT_COMPRESSION"))
*op_flag_value = RTE_BBDEV_LDPC_HARQ_6BIT_COMPRESSION;
+   else if (!strcmp(token, "RTE_BBDEV_LDPC_HARQ_4BIT_COMPRESSION"))
+   *op_flag_value = RTE_BBDEV_LDPC_HARQ_4BIT_COMPRESSION;
else if (!strcmp(token, "RTE_BBDEV_LDPC_LLR_COMPRESSION"))
*op_flag_value = RTE_BBDEV_LDPC_LLR_COMPRESSION;
else if (!strcmp(token,


Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [PATCH v1 06/11] test/bbdev: assert failed test for queue configure

[Maxime Coquelin]

Hi Nicolas,

On 10/19/23 10:41, Chautru, Nicolas wrote:

Hi Maxime,

Do we really want to make these kind of changes on to the stable release, it 
tends to artificially increase the amount of churn on the stable release which 
can be counterproductive for such changes which don't add much value if any to 
user/developper.
Happy to follow your suggestion but a general feedback is lack of appetite for 
very large amount of changes in stable patches which inhibit adoption, so would 
expect to put things there that we would genuinely flag as a bug.
Kindly share your thoughts.


Checking for configuration failure in a test application is quite useful
in my opinion, as it can help catching regressions, isn't it?

Maxime

Thanks
Nic



-Original Message-----
From: Maxime Coquelin 
Sent: Tuesday, October 17, 2023 9:43 PM
To: Vargas, Hernan ; dev@dpdk.org;
gak...@marvell.com; Rix, Tom 
Cc: Chautru, Nicolas ; Zhang, Qi Z

Subject: Re: [PATCH v1 06/11] test/bbdev: assert failed test for queue configure



On 9/29/23 20:13, Hernan Vargas wrote:

Stop test if rte_bbdev_queue_configure fails to configure queue.

Signed-off-by: Hernan Vargas 
---
   app/test-bbdev/test_bbdev.c | 3 ++-
   1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/test-bbdev/test_bbdev.c b/app/test-bbdev/test_bbdev.c
index 65805977aead..cf224dca5d04 100644
--- a/app/test-bbdev/test_bbdev.c
+++ b/app/test-bbdev/test_bbdev.c
@@ -366,7 +366,8 @@ test_bbdev_configure_stop_queue(void)
 * - queue should be started if deferred_start ==
 */
ts_params->qconf.deferred_start = 0;
-   rte_bbdev_queue_configure(dev_id, queue_id, &ts_params->qconf);
+   TEST_ASSERT_SUCCESS(rte_bbdev_queue_configure(dev_id, queue_id,

&ts_params->qconf),

+   "Failed test for rte_bbdev_queue_configure");
rte_bbdev_start(dev_id);

TEST_ASSERT_SUCCESS(return_value =

rte_bbdev_queue_info_get(dev_id,

If should be a fix IMO.
With fixes tag added and stable cc'ed:

Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [PATCH v1 02/11] test/bbdev: update python script parameters

[Maxime Coquelin]

On 10/19/23 11:01, Chautru, Nicolas wrote:

Hi Maxime,

I believe there was some historical discrepancy, even in doc both appeared but 
none of the 2 -t options with the cap.
https://doc.dpdk.org/guides/tools/testbbdev.html
Resolving this historical issue here.


Ok, then we should fix the doc, not the code.

Thanks,
Maxime


Thanks
Nic


-Original Message-
From: Maxime Coquelin 
Sent: Tuesday, October 17, 2023 9:08 PM
To: Vargas, Hernan ; dev@dpdk.org;
gak...@marvell.com; Rix, Tom 
Cc: Chautru, Nicolas ; Zhang, Qi Z

Subject: Re: [PATCH v1 02/11] test/bbdev: update python script parameters



On 9/29/23 20:13, Hernan Vargas wrote:

Update the timeout argument and default values.
Update EAL help message and default value.
Add iter_max and snr arguments.

Signed-off-by: Hernan Vargas 
---
   app/test-bbdev/test-bbdev.py | 22 ++
   app/test-bbdev/test_bbdev_perf.c |  2 +-
   2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/app/test-bbdev/test-bbdev.py
b/app/test-bbdev/test-bbdev.py index 9cdb4659724d..8d0145076e4d 100755
--- a/app/test-bbdev/test-bbdev.py
+++ b/app/test-bbdev/test-bbdev.py
@@ -25,12 +25,12 @@ def kill(process):
   help="specifies path to the bbdev test app",
   default=dpdk_path + "/" + dpdk_target + 
"/app/dpdk-test-bbdev")
   parser.add_argument("-e", "--eal-params",
-help="EAL arguments which are passed to the test app",
-default="--vdev=baseband_null0")
-parser.add_argument("-t", "--timeout",
+help="EAL arguments which must be passed to the test app",
+default="--vdev=baseband_null0 -a00:00.0")
+parser.add_argument("-T", "--timeout",
   type=int,
   help="Timeout in seconds",
-default=300)
+default=600)
   parser.add_argument("-c", "--test-cases",
   nargs="+",
   help="Defines test cases to run. Run all if not
specified") @@ -48,6 +48,14 @@ def kill(process):
   type=int,
   help="Operations enqueue/dequeue burst size.",
   default=[32])
+parser.add_argument("-s", "--snr",
+type=int,
+help="SNR in dB for BLER tests",
+default=0)
+parser.add_argument("-t", "--iter-max",


We shouldn't change parameters meaning, it will silently break existing scripts
making use of it.


+type=int,
+help="Max iterations",
+default=6)
   parser.add_argument("-l", "--num-lcores",
   type=int,
   help="Number of lcores to run.", @@ -68,6 +76,12
@@ def kill(process):

   params.extend(["--"])

+if args.snr:
+params.extend(["-s", str(args.snr)])
+
+if args.iter_max:
+params.extend(["-t", str(args.iter_max)])
+
   if args.num_ops:
   params.extend(["-n", str(args.num_ops)])

diff --git a/app/test-bbdev/test_bbdev_perf.c
b/app/test-bbdev/test_bbdev_perf.c
index 276bbf0a2e6d..faea26c10eed 100644
--- a/app/test-bbdev/test_bbdev_perf.c
+++ b/app/test-bbdev/test_bbdev_perf.c
@@ -26,7 +26,7 @@

   #define MAX_QUEUES RTE_MAX_LCORE
   #define TEST_REPETITIONS 100
-#define TIME_OUT_POLL 1e8
+#define TIME_OUT_POLL 1e9
   #define WAIT_OFFLOAD_US 1000

   #ifdef RTE_BASEBAND_FPGA_LTE_FEC

Re: [PATCH v1 02/11] test/bbdev: update python script parameters

[Maxime Coquelin]

Hi Nicolas,

On 10/19/23 14:09, Chautru, Nicolas wrote:

Hi Maxime,
In practice anyone using that API is already using the one defined in the patch 
below and not using -t for time out. So not a concern to do it properly through 
that patch.


Sorry, I'm not sure to follow you.

For example in RHEL, we use this script for BBDEV validation for our
downstream packages.

Maxime


Heman, any concern on your side with this change?


-Original Message-
From: Maxime Coquelin 
Sent: Thursday, October 19, 2023 11:19 AM
To: Chautru, Nicolas ; Vargas, Hernan
; dev@dpdk.org; gak...@marvell.com; Rix, Tom

Cc: Zhang, Qi Z 
Subject: Re: [PATCH v1 02/11] test/bbdev: update python script parameters



On 10/19/23 11:01, Chautru, Nicolas wrote:

Hi Maxime,

I believe there was some historical discrepancy, even in doc both appeared

but none of the 2 -t options with the cap.

https://doc.dpdk.org/guides/tools/testbbdev.html
Resolving this historical issue here.


Ok, then we should fix the doc, not the code.

Thanks,
Maxime


Thanks
Nic


-Original Message-
From: Maxime Coquelin 
Sent: Tuesday, October 17, 2023 9:08 PM
To: Vargas, Hernan ; dev@dpdk.org;
gak...@marvell.com; Rix, Tom 
Cc: Chautru, Nicolas ; Zhang, Qi Z

Subject: Re: [PATCH v1 02/11] test/bbdev: update python script
parameters



On 9/29/23 20:13, Hernan Vargas wrote:

Update the timeout argument and default values.
Update EAL help message and default value.
Add iter_max and snr arguments.

Signed-off-by: Hernan Vargas 
---
app/test-bbdev/test-bbdev.py | 22 ++
app/test-bbdev/test_bbdev_perf.c |  2 +-
2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/app/test-bbdev/test-bbdev.py
b/app/test-bbdev/test-bbdev.py index 9cdb4659724d..8d0145076e4d
100755
--- a/app/test-bbdev/test-bbdev.py
+++ b/app/test-bbdev/test-bbdev.py
@@ -25,12 +25,12 @@ def kill(process):
help="specifies path to the bbdev test app",
default=dpdk_path + "/" + dpdk_target + "/app/dpdk-test-

bbdev")

parser.add_argument("-e", "--eal-params",
-help="EAL arguments which are passed to the test app",
-default="--vdev=baseband_null0")
-parser.add_argument("-t", "--timeout",
+help="EAL arguments which must be passed to the test app",
+default="--vdev=baseband_null0 -a00:00.0")
+parser.add_argument("-T", "--timeout",
type=int,
help="Timeout in seconds",
-default=300)
+default=600)
parser.add_argument("-c", "--test-cases",
nargs="+",
help="Defines test cases to run. Run all if
not
specified") @@ -48,6 +48,14 @@ def kill(process):
type=int,
help="Operations enqueue/dequeue burst size.",
default=[32])
+parser.add_argument("-s", "--snr",
+type=int,
+help="SNR in dB for BLER tests",
+default=0)
+parser.add_argument("-t", "--iter-max",


We shouldn't change parameters meaning, it will silently break
existing scripts making use of it.


+type=int,
+help="Max iterations",
+default=6)
parser.add_argument("-l", "--num-lcores",
type=int,
help="Number of lcores to run.", @@ -68,6
+76,12 @@ def kill(process):

params.extend(["--"])

+if args.snr:
+params.extend(["-s", str(args.snr)])
+
+if args.iter_max:
+params.extend(["-t", str(args.iter_max)])
+
if args.num_ops:
params.extend(["-n", str(args.num_ops)])

diff --git a/app/test-bbdev/test_bbdev_perf.c
b/app/test-bbdev/test_bbdev_perf.c
index 276bbf0a2e6d..faea26c10eed 100644
--- a/app/test-bbdev/test_bbdev_perf.c
+++ b/app/test-bbdev/test_bbdev_perf.c
@@ -26,7 +26,7 @@

#define MAX_QUEUES RTE_MAX_LCORE
#define TEST_REPETITIONS 100
-#define TIME_OUT_POLL 1e8
+#define TIME_OUT_POLL 1e9
#define WAIT_OFFLOAD_US 1000

#ifdef RTE_BASEBAND_FPGA_LTE_FEC

Re: [PATCH v3 22/29] vhost: remove experimental from some API's

[Maxime Coquelin]

On 10/19/23 18:55, Stephen Hemminger wrote:

All API's that before 22.11 release should have experimental
tag removed.

Signed-off-by: Stephen Hemminger 
---
  lib/vhost/rte_vhost.h|  5 
  lib/vhost/rte_vhost_async.h  | 19 --
  lib/vhost/rte_vhost_crypto.h |  1 -
  lib/vhost/version.map| 51 ++--
  4 files changed, 19 insertions(+), 57 deletions(-)

diff --git a/lib/vhost/rte_vhost.h b/lib/vhost/rte_vhost.h
index fec650b1dbba..db92f0534431 100644
--- a/lib/vhost/rte_vhost.h
+++ b/lib/vhost/rte_vhost.h
@@ -509,7 +509,6 @@ rte_vhost_driver_get_vdpa_device(const char *path);
   * @return
   *  0 on success, -1 on failure
   */
-__rte_experimental
  int
  rte_vhost_driver_get_vdpa_dev_type(const char *path, uint32_t *type);
  
@@ -648,7 +647,6 @@ int rte_vhost_get_negotiated_features(int vid, uint64_t *features);

   * @return
   *  0 on success, -1 on failure
   */
-__rte_experimental
  int
  rte_vhost_get_negotiated_protocol_features(int vid,
   uint64_t *protocol_features);
@@ -960,7 +958,6 @@ int rte_vhost_vring_call(int vid, uint16_t vring_idx);
   * @return
   *  0 on success, -1 on failure, -EAGAIN for another retry
   */
-__rte_experimental
  int rte_vhost_vring_call_nonblock(int vid, uint16_t vring_idx);
  
  /**

@@ -987,7 +984,6 @@ uint32_t rte_vhost_rx_queue_count(int vid, uint16_t qid);
   * @return
   *  0 on success, -1 on failure
   */
-__rte_experimental
  int
  rte_vhost_get_monitor_addr(int vid, uint16_t queue_id,
struct rte_vhost_power_monitor_cond *pmc);
@@ -1102,7 +1098,6 @@ rte_vhost_get_vdpa_device(int vid);
   * @return
   *  0 on success, < 0 on failure
   */
-__rte_experimental
  int
  rte_vhost_backend_config_change(int vid, bool need_reply);
  
diff --git a/lib/vhost/rte_vhost_async.h b/lib/vhost/rte_vhost_async.h

index 8f190dd44b1b..3370026415fb 100644
--- a/lib/vhost/rte_vhost_async.h
+++ b/lib/vhost/rte_vhost_async.h
@@ -24,7 +24,6 @@ extern "C" {
   * @return
   *  0 on success, -1 on failures
   */
-__rte_experimental
  int rte_vhost_async_channel_register(int vid, uint16_t queue_id);
  
  /**

@@ -37,7 +36,6 @@ int rte_vhost_async_channel_register(int vid, uint16_t 
queue_id);
   * @return
   *  0 on success, -1 on failures
   */
-__rte_experimental
  int rte_vhost_async_channel_unregister(int vid, uint16_t queue_id);
  
  /**

@@ -54,7 +52,6 @@ int rte_vhost_async_channel_unregister(int vid, uint16_t 
queue_id);
   * @return
   *  0 on success, -1 on failures
   */
-__rte_experimental
  int rte_vhost_async_channel_register_thread_unsafe(int vid, uint16_t 
queue_id);
  
  /**

@@ -71,7 +68,6 @@ int rte_vhost_async_channel_register_thread_unsafe(int vid, 
uint16_t queue_id);
   * @return
   *  0 on success, -1 on failures
   */
-__rte_experimental
  int rte_vhost_async_channel_unregister_thread_unsafe(int vid,
uint16_t queue_id);
  
@@ -95,7 +91,6 @@ int rte_vhost_async_channel_unregister_thread_unsafe(int vid,

   * @return
   *  num of packets enqueued
   */
-__rte_experimental
  uint16_t rte_vhost_submit_enqueue_burst(int vid, uint16_t queue_id,
struct rte_mbuf **pkts, uint16_t count, int16_t dma_id,
uint16_t vchan_id);
@@ -120,7 +115,6 @@ uint16_t rte_vhost_submit_enqueue_burst(int vid, uint16_t 
queue_id,
   * @return
   *  num of packets returned
   */
-__rte_experimental
  uint16_t rte_vhost_poll_enqueue_completed(int vid, uint16_t queue_id,
struct rte_mbuf **pkts, uint16_t count, int16_t dma_id,
uint16_t vchan_id);
@@ -136,7 +130,6 @@ uint16_t rte_vhost_poll_enqueue_completed(int vid, uint16_t 
queue_id,
   * @return
   *  the amount of in-flight packets on success; -1 on failure
   */
-__rte_experimental
  int rte_vhost_async_get_inflight(int vid, uint16_t queue_id);
  
  /**

@@ -153,7 +146,6 @@ int rte_vhost_async_get_inflight(int vid, uint16_t 
queue_id);
   * @return
   * the amount of in-flight packets on success; -1 on failure
   */
-__rte_experimental
  int rte_vhost_async_get_inflight_thread_unsafe(int vid, uint16_t queue_id);
  
  /**

@@ -178,7 +170,6 @@ int rte_vhost_async_get_inflight_thread_unsafe(int vid, 
uint16_t queue_id);
   * @return
   *  Number of packets returned
   */
-__rte_experimental
  uint16_t rte_vhost_clear_queue_thread_unsafe(int vid, uint16_t queue_id,
struct rte_mbuf **pkts, uint16_t count, int16_t dma_id,
uint16_t vchan_id);
@@ -203,7 +194,6 @@ uint16_t rte_vhost_clear_queue_thread_unsafe(int vid, 
uint16_t queue_id,
   * @return
   *  Number of packets returned
   */
-__rte_experimental
  uint16_t rte_vhost_clear_queue(int vid, uint16_t queue_id,
struct rte_mbuf **pkts, uint16_t count, int16_t dma_id,
uint16_t vchan_id);
@@ -226,13 +216,9 @@ uint16_t rte_vhost_clear_queue(int vid, uint16_t queue_id,
   * @return
   *  0 on success, and -1 on failure
   */
-__rte_experime

Re: [PATCH v4 21/28] vhost: remove experimental from some API's

[Maxime Coquelin]

On 10/19/23 21:10, Stephen Hemminger wrote:

All API's that before 22.11 release should have experimental
tag removed.

Signed-off-by: Stephen Hemminger 
---
  lib/vhost/rte_vhost.h|  5 
  lib/vhost/rte_vhost_async.h  | 19 --
  lib/vhost/rte_vhost_crypto.h |  1 -
  lib/vhost/version.map| 51 ++--
  4 files changed, 19 insertions(+), 57 deletions(-)

diff --git a/lib/vhost/rte_vhost.h b/lib/vhost/rte_vhost.h
index fec650b1dbba..db92f0534431 100644
--- a/lib/vhost/rte_vhost.h
+++ b/lib/vhost/rte_vhost.h
@@ -509,7 +509,6 @@ rte_vhost_driver_get_vdpa_device(const char *path);
   * @return
   *  0 on success, -1 on failure
   */
-__rte_experimental
  int
  rte_vhost_driver_get_vdpa_dev_type(const char *path, uint32_t *type);
  
@@ -648,7 +647,6 @@ int rte_vhost_get_negotiated_features(int vid, uint64_t *features);

   * @return
   *  0 on success, -1 on failure
   */
-__rte_experimental
  int
  rte_vhost_get_negotiated_protocol_features(int vid,
   uint64_t *protocol_features);
@@ -960,7 +958,6 @@ int rte_vhost_vring_call(int vid, uint16_t vring_idx);
   * @return
   *  0 on success, -1 on failure, -EAGAIN for another retry
   */
-__rte_experimental
  int rte_vhost_vring_call_nonblock(int vid, uint16_t vring_idx);
  
  /**

@@ -987,7 +984,6 @@ uint32_t rte_vhost_rx_queue_count(int vid, uint16_t qid);
   * @return
   *  0 on success, -1 on failure
   */
-__rte_experimental
  int
  rte_vhost_get_monitor_addr(int vid, uint16_t queue_id,
struct rte_vhost_power_monitor_cond *pmc);
@@ -1102,7 +1098,6 @@ rte_vhost_get_vdpa_device(int vid);
   * @return
   *  0 on success, < 0 on failure
   */
-__rte_experimental
  int
  rte_vhost_backend_config_change(int vid, bool need_reply);
  
diff --git a/lib/vhost/rte_vhost_async.h b/lib/vhost/rte_vhost_async.h

index 8f190dd44b1b..3370026415fb 100644
--- a/lib/vhost/rte_vhost_async.h
+++ b/lib/vhost/rte_vhost_async.h
@@ -24,7 +24,6 @@ extern "C" {
   * @return
   *  0 on success, -1 on failures
   */
-__rte_experimental
  int rte_vhost_async_channel_register(int vid, uint16_t queue_id);
  
  /**

@@ -37,7 +36,6 @@ int rte_vhost_async_channel_register(int vid, uint16_t 
queue_id);
   * @return
   *  0 on success, -1 on failures
   */
-__rte_experimental
  int rte_vhost_async_channel_unregister(int vid, uint16_t queue_id);
  
  /**

@@ -54,7 +52,6 @@ int rte_vhost_async_channel_unregister(int vid, uint16_t 
queue_id);
   * @return
   *  0 on success, -1 on failures
   */
-__rte_experimental
  int rte_vhost_async_channel_register_thread_unsafe(int vid, uint16_t 
queue_id);
  
  /**

@@ -71,7 +68,6 @@ int rte_vhost_async_channel_register_thread_unsafe(int vid, 
uint16_t queue_id);
   * @return
   *  0 on success, -1 on failures
   */
-__rte_experimental
  int rte_vhost_async_channel_unregister_thread_unsafe(int vid,
uint16_t queue_id);
  
@@ -95,7 +91,6 @@ int rte_vhost_async_channel_unregister_thread_unsafe(int vid,

   * @return
   *  num of packets enqueued
   */
-__rte_experimental
  uint16_t rte_vhost_submit_enqueue_burst(int vid, uint16_t queue_id,
struct rte_mbuf **pkts, uint16_t count, int16_t dma_id,
uint16_t vchan_id);
@@ -120,7 +115,6 @@ uint16_t rte_vhost_submit_enqueue_burst(int vid, uint16_t 
queue_id,
   * @return
   *  num of packets returned
   */
-__rte_experimental
  uint16_t rte_vhost_poll_enqueue_completed(int vid, uint16_t queue_id,
struct rte_mbuf **pkts, uint16_t count, int16_t dma_id,
uint16_t vchan_id);
@@ -136,7 +130,6 @@ uint16_t rte_vhost_poll_enqueue_completed(int vid, uint16_t 
queue_id,
   * @return
   *  the amount of in-flight packets on success; -1 on failure
   */
-__rte_experimental
  int rte_vhost_async_get_inflight(int vid, uint16_t queue_id);
  
  /**

@@ -153,7 +146,6 @@ int rte_vhost_async_get_inflight(int vid, uint16_t 
queue_id);
   * @return
   * the amount of in-flight packets on success; -1 on failure
   */
-__rte_experimental
  int rte_vhost_async_get_inflight_thread_unsafe(int vid, uint16_t queue_id);
  
  /**

@@ -178,7 +170,6 @@ int rte_vhost_async_get_inflight_thread_unsafe(int vid, 
uint16_t queue_id);
   * @return
   *  Number of packets returned
   */
-__rte_experimental
  uint16_t rte_vhost_clear_queue_thread_unsafe(int vid, uint16_t queue_id,
struct rte_mbuf **pkts, uint16_t count, int16_t dma_id,
uint16_t vchan_id);
@@ -203,7 +194,6 @@ uint16_t rte_vhost_clear_queue_thread_unsafe(int vid, 
uint16_t queue_id,
   * @return
   *  Number of packets returned
   */
-__rte_experimental
  uint16_t rte_vhost_clear_queue(int vid, uint16_t queue_id,
struct rte_mbuf **pkts, uint16_t count, int16_t dma_id,
uint16_t vchan_id);
@@ -226,13 +216,9 @@ uint16_t rte_vhost_clear_queue(int vid, uint16_t queue_id,
   * @return
   *  0 on success, and -1 on failure
   */
-__rte_experime

Re: [PATCH v4 22/28] bbdev: remove experimental tag

[Maxime Coquelin]

On 10/19/23 21:10, Stephen Hemminger wrote:

The API's for bbdev were last added to in 22.11.
Remove experimental flag now.

Signed-off-by: Stephen Hemminger 
---
  lib/bbdev/rte_bbdev.h|  4 
  lib/bbdev/rte_bbdev_op.h |  2 --
  lib/bbdev/version.map| 15 +++
  3 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/lib/bbdev/rte_bbdev.h b/lib/bbdev/rte_bbdev.h
index d12e2e7fbcf3..b43e98ef9d1b 100644
--- a/lib/bbdev/rte_bbdev.h
+++ b/lib/bbdev/rte_bbdev.h
@@ -675,7 +675,6 @@ rte_bbdev_enqueue_ldpc_dec_ops(uint16_t dev_id, uint16_t 
queue_id,
   *   The number of operations actually enqueued.
   *   (This is the number of processed entries in the @p ops array.)
   */
-__rte_experimental
  static inline uint16_t
  rte_bbdev_enqueue_fft_ops(uint16_t dev_id, uint16_t queue_id,
struct rte_bbdev_fft_op **ops, uint16_t num_ops)
@@ -860,7 +859,6 @@ rte_bbdev_dequeue_ldpc_dec_ops(uint16_t dev_id, uint16_t 
queue_id,
   *   The number of operations actually dequeued (this is the number of entries
   *   copied into the @p ops array).
   */
-__rte_experimental
  static inline uint16_t
  rte_bbdev_dequeue_fft_ops(uint16_t dev_id, uint16_t queue_id,
struct rte_bbdev_fft_op **ops, uint16_t num_ops)
@@ -1042,7 +1040,6 @@ rte_bbdev_queue_intr_ctl(uint16_t dev_id, uint16_t 
queue_id, int epfd, int op,
   * @returns
   *   Device status as string or NULL if invalid.
   */
-__rte_experimental
  const char*
  rte_bbdev_device_status_str(enum rte_bbdev_device_status status);
  
@@ -1055,7 +1052,6 @@ rte_bbdev_device_status_str(enum rte_bbdev_device_status status);

   * @returns
   *   Queue status as string or NULL if op_type is invalid.
   */
-__rte_experimental
  const char*
  rte_bbdev_enqueue_status_str(enum rte_bbdev_enqueue_status status);
  
diff --git a/lib/bbdev/rte_bbdev_op.h b/lib/bbdev/rte_bbdev_op.h

index 369ac331bf9b..459631d0d0b7 100644
--- a/lib/bbdev/rte_bbdev_op.h
+++ b/lib/bbdev/rte_bbdev_op.h
@@ -1130,7 +1130,6 @@ rte_bbdev_dec_op_alloc_bulk(struct rte_mempool *mempool,
   *   - 0 on success.
   *   - EINVAL if invalid mempool is provided.
   */
-__rte_experimental
  static inline int
  rte_bbdev_fft_op_alloc_bulk(struct rte_mempool *mempool,
struct rte_bbdev_fft_op **ops, unsigned int num_ops)
@@ -1220,7 +1219,6 @@ rte_bbdev_enc_op_free_bulk(struct rte_bbdev_enc_op **ops, 
unsigned int num_ops)
   * @param num_ops
   *   Number of structures.
   */
-__rte_experimental
  static inline void
  rte_bbdev_fft_op_free_bulk(struct rte_bbdev_fft_op **ops, unsigned int 
num_ops)
  {
diff --git a/lib/bbdev/version.map b/lib/bbdev/version.map
index eeb7ed475a6c..1840d2b2a491 100644
--- a/lib/bbdev/version.map
+++ b/lib/bbdev/version.map
@@ -10,11 +10,17 @@ DPDK_24 {
rte_bbdev_dec_op_free_bulk;
rte_bbdev_dequeue_dec_ops;
rte_bbdev_dequeue_enc_ops;
+   rte_bbdev_dequeue_fft_ops;
+   rte_bbdev_device_status_str;
rte_bbdev_devices;
rte_bbdev_enc_op_alloc_bulk;
rte_bbdev_enc_op_free_bulk;
rte_bbdev_enqueue_dec_ops;
rte_bbdev_enqueue_enc_ops;
+   rte_bbdev_enqueue_fft_ops;
+   rte_bbdev_enqueue_status_str;
+   rte_bbdev_fft_op_alloc_bulk;
+   rte_bbdev_fft_op_free_bulk;
rte_bbdev_find_next;
rte_bbdev_get_named_dev;
rte_bbdev_info_get;
@@ -43,14 +49,7 @@ DPDK_24 {
  EXPERIMENTAL {
global:
  
-	# added in 22.11

-   rte_bbdev_dequeue_fft_ops;
-   rte_bbdev_device_status_str;
-   rte_bbdev_enqueue_fft_ops;
-   rte_bbdev_enqueue_status_str;
-   rte_bbdev_fft_op_alloc_bulk;
-   rte_bbdev_fft_op_free_bulk;
-   #added in 23.11
+   # added in 23.11
rte_bbdev_dequeue_mldts_ops;
rte_bbdev_enqueue_mldts_ops;
rte_bbdev_mldts_op_alloc_bulk;


Acked-by: Maxime Coquelin 

Thanks,
Maxime

Re: [PATCH 1/7] vhost: fix missing vring call check on virtqueue access

[Maxime Coquelin]

On 10/19/23 09:24, David Marchand wrote:

On Mon, Sep 25, 2023 at 6:36 PM Maxime Coquelin
 wrote:


Acquiring the access lock is not enough to ensure
virtqueue's metadata such as vring pointers are valid.

The access status must also be checked.


Even if adding the lock was not enough, I would flag Fixes:
6c299bb7322f ("vhost: introduce vring call API")


Makes sense, adding it to v2.

Thanks,
Maxime



Fixes: c5736998305d ("vhost: fix missing virtqueue lock protection")
Fixes: 830f7e790732 ("vhost: add non-blocking API for posting interrupt")
Cc: sta...@dpdk.org

Reported-by: Li Feng 
Signed-off-by: Maxime Coquelin 

[PATCH v2 1/7] vhost: fix missing vring call check on virtqueue access

[Maxime Coquelin]

Acquiring the access lock is not enough to ensure
virtqueue's metadata such as vring pointers are valid.

The access status must also be checked.

Fixes: 6c299bb7322f ("vhost: introduce vring call API")
Fixes: c5736998305d ("vhost: fix missing virtqueue lock protection")
Fixes: 830f7e790732 ("vhost: add non-blocking API for posting interrupt")
Cc: sta...@dpdk.org

Reported-by: Li Feng 
Signed-off-by: Maxime Coquelin 
---
 lib/vhost/vhost.c | 18 --
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c
index 7fde412ef3..0d2bc1a48b 100644
--- a/lib/vhost/vhost.c
+++ b/lib/vhost/vhost.c
@@ -1330,6 +1330,7 @@ rte_vhost_vring_call(int vid, uint16_t vring_idx)
 {
struct virtio_net *dev;
struct vhost_virtqueue *vq;
+   int ret = 0;
 
dev = get_device(vid);
if (!dev)
@@ -1344,14 +1345,20 @@ rte_vhost_vring_call(int vid, uint16_t vring_idx)
 
rte_rwlock_read_lock(&vq->access_lock);
 
+   if (unlikely(!vq->access_ok)) {
+   ret = -1;
+   goto out_unlock;
+   }
+
if (vq_is_packed(dev))
vhost_vring_call_packed(dev, vq);
else
vhost_vring_call_split(dev, vq);
 
+out_unlock:
rte_rwlock_read_unlock(&vq->access_lock);
 
-   return 0;
+   return ret;
 }
 
 int
@@ -1359,6 +1366,7 @@ rte_vhost_vring_call_nonblock(int vid, uint16_t vring_idx)
 {
struct virtio_net *dev;
struct vhost_virtqueue *vq;
+   int ret = 0;
 
dev = get_device(vid);
if (!dev)
@@ -1374,14 +1382,20 @@ rte_vhost_vring_call_nonblock(int vid, uint16_t 
vring_idx)
if (rte_rwlock_read_trylock(&vq->access_lock))
return -EAGAIN;
 
+   if (unlikely(!vq->access_ok)) {
+   ret = -1;
+   goto out_unlock;
+   }
+
if (vq_is_packed(dev))
vhost_vring_call_packed(dev, vq);
else
vhost_vring_call_split(dev, vq);
 
+out_unlock:
rte_rwlock_read_unlock(&vq->access_lock);
 
-   return 0;
+   return ret;
 }
 
 uint16_t
-- 
2.41.0

[PATCH v2 0/7] vhost: ensure virtqueue access status is checked

[Maxime Coquelin]

Li Feng initially reported segmentation fault in rte_vhost_vring_call()
because of not checking the virtqueue metadata can be accessed.

This should be achieved by checking the access_ok status field of
the virtqueue.

This series also takes the opportunity to fix the other APIs.
This is split in multiple patches to ease LTS maintainers backports,
but could be squashed if preferred.

Changes in v2:
--
- Rebased to apply on -rc1 (David)
- Add Fixes tag in patch 1 (David)
- Fix various typos in commit logs (David)

Maxime Coquelin (7):
  vhost: fix missing vring call check on virtqueue access
  vhost: fix missing check on virtqueue access
  vhost: fix checking virtqueue access when notifying guest
  vhost: fix check on virtqueue access in async registration
  vhost: fix check on virtqueue access in in-flight getter
  vhost: fix missing lock protection in power monitor API
  vhost: fix checking virtqueue access in stats API

 lib/vhost/vhost.c | 92 +++
 1 file changed, 85 insertions(+), 7 deletions(-)

-- 
2.41.0

[PATCH v2 2/7] vhost: fix missing check on virtqueue access

[Maxime Coquelin]

Acquiring the access lock is not enough to ensure
virtqueue's metadata such as vring pointers are valid.

The access status must also be checked.

Fixes: 4e0de8dac853 ("vhost: protect vring access done by application")
Cc: sta...@dpdk.org

Signed-off-by: Maxime Coquelin 
---
 lib/vhost/vhost.c | 16 ++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c
index 0d2bc1a48b..446bca1574 100644
--- a/lib/vhost/vhost.c
+++ b/lib/vhost/vhost.c
@@ -1418,7 +1418,10 @@ rte_vhost_avail_entries(int vid, uint16_t queue_id)
 
rte_rwlock_write_lock(&vq->access_lock);
 
-   if (unlikely(!vq->enabled || vq->avail == NULL))
+   if (unlikely(!vq->access_ok))
+   goto out;
+
+   if (unlikely(!vq->enabled))
goto out;
 
ret = *(volatile uint16_t *)&vq->avail->idx - vq->last_used_idx;
@@ -1510,9 +1513,15 @@ rte_vhost_enable_guest_notification(int vid, uint16_t 
queue_id, int enable)
 
rte_rwlock_write_lock(&vq->access_lock);
 
+   if (unlikely(!vq->access_ok)) {
+   ret = -1;
+   goto out_unlock;
+   }
+
vq->notif_enable = enable;
ret = vhost_enable_guest_notification(dev, vq, enable);
 
+out_unlock:
rte_rwlock_write_unlock(&vq->access_lock);
 
return ret;
@@ -1605,7 +1614,10 @@ rte_vhost_rx_queue_count(int vid, uint16_t qid)
 
rte_rwlock_write_lock(&vq->access_lock);
 
-   if (unlikely(!vq->enabled || vq->avail == NULL))
+   if (unlikely(!vq->access_ok))
+   goto out;
+
+   if (unlikely(!vq->enabled))
goto out;
 
ret = *((volatile uint16_t *)&vq->avail->idx) - vq->last_avail_idx;
-- 
2.41.0

[PATCH v2 3/7] vhost: fix checking virtqueue access when notifying guest

[Maxime Coquelin]

Acquiring the access lock is not enough to ensure
virtqueue's metadata such as vring pointers are valid.

The access status must also be checked.

Fixes: d761d455a0e4 ("vhost: add operation to offload the interrupt kick")
Cc: sta...@dpdk.org

Signed-off-by: Maxime Coquelin 
---
 lib/vhost/vhost.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c
index 446bca1574..5428ff4a25 100644
--- a/lib/vhost/vhost.c
+++ b/lib/vhost/vhost.c
@@ -1542,6 +1542,9 @@ rte_vhost_notify_guest(int vid, uint16_t queue_id)
 
rte_rwlock_read_lock(&vq->access_lock);
 
+   if (unlikely(!vq->access_ok))
+   goto out_unlock;
+
__atomic_store_n(&vq->irq_pending, false, __ATOMIC_RELEASE);
 
if (dev->backend_ops->inject_irq(dev, vq)) {
@@ -1556,6 +1559,7 @@ rte_vhost_notify_guest(int vid, uint16_t queue_id)
dev->notify_ops->guest_notified(dev->vid);
}
 
+out_unlock:
rte_rwlock_read_unlock(&vq->access_lock);
 }
 
-- 
2.41.0

[PATCH v2 5/7] vhost: fix check on virtqueue access in in-flight getter

[Maxime Coquelin]

Acquiring the access lock is not enough to ensure
virtqueue's metadata such as vring pointers are valid.

The access status must also be checked.

Fixes: 0c0935c5f794 ("vhost: allow to check in-flight packets for async vhost")
Cc: sta...@dpdk.org

Signed-off-by: Maxime Coquelin 
---
 lib/vhost/vhost.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c
index ccd3c0e865..a243f88398 100644
--- a/lib/vhost/vhost.c
+++ b/lib/vhost/vhost.c
@@ -2073,9 +2073,15 @@ rte_vhost_async_get_inflight(int vid, uint16_t queue_id)
return ret;
}
 
+   if (unlikely(!vq->access_ok)) {
+   ret = -1;
+   goto out_unlock;
+   }
+
if (vq->async)
ret = vq->async->pkts_inflight_n;
 
+out_unlock:
rte_rwlock_write_unlock(&vq->access_lock);
 
return ret;
-- 
2.41.0

[PATCH v2 7/7] vhost: fix checking virtqueue access in stats API

[Maxime Coquelin]

Acquiring the access lock is not enough to ensure
virtqueue's metadata such as vring pointers are valid.

The access status must also be checked.

Fixes: be75dc99ea1f ("vhost: support per-virtqueue statistics")
Cc: sta...@dpdk.org

Signed-off-by: Maxime Coquelin 
---
 lib/vhost/vhost.c | 21 +++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c
index d8d74623d4..b03e3b391d 100644
--- a/lib/vhost/vhost.c
+++ b/lib/vhost/vhost.c
@@ -2199,6 +2199,7 @@ rte_vhost_vring_stats_get(int vid, uint16_t queue_id,
struct virtio_net *dev = get_device(vid);
struct vhost_virtqueue *vq;
unsigned int i;
+   int ret = VHOST_NB_VQ_STATS;
 
if (dev == NULL)
return -1;
@@ -2215,6 +2216,12 @@ rte_vhost_vring_stats_get(int vid, uint16_t queue_id,
vq = dev->virtqueue[queue_id];
 
rte_rwlock_write_lock(&vq->access_lock);
+
+   if (unlikely(!vq->access_ok)) {
+   ret = -1;
+   goto out_unlock;
+   }
+
for (i = 0; i < VHOST_NB_VQ_STATS; i++) {
/*
 * No need to the read atomic counters as such, due to the
@@ -2224,15 +2231,18 @@ rte_vhost_vring_stats_get(int vid, uint16_t queue_id,
*(uint64_t *)(((char *)vq) + 
vhost_vq_stat_strings[i].offset);
stats[i].id = i;
}
+
+out_unlock:
rte_rwlock_write_unlock(&vq->access_lock);
 
-   return VHOST_NB_VQ_STATS;
+   return ret;
 }
 
 int rte_vhost_vring_stats_reset(int vid, uint16_t queue_id)
 {
struct virtio_net *dev = get_device(vid);
struct vhost_virtqueue *vq;
+   int ret = 0;
 
if (dev == NULL)
return -1;
@@ -2246,14 +2256,21 @@ int rte_vhost_vring_stats_reset(int vid, uint16_t 
queue_id)
vq = dev->virtqueue[queue_id];
 
rte_rwlock_write_lock(&vq->access_lock);
+
+   if (unlikely(!vq->access_ok)) {
+   ret = -1;
+   goto out_unlock;
+   }
/*
 * No need to the reset atomic counters as such, due to the
 * above write access_lock preventing them to be updated.
 */
memset(&vq->stats, 0, sizeof(vq->stats));
+
+out_unlock:
rte_rwlock_write_unlock(&vq->access_lock);
 
-   return 0;
+   return ret;
 }
 
 int
-- 
2.41.0

[PATCH v2 4/7] vhost: fix check on virtqueue access in async registration

[Maxime Coquelin]

Acquiring the access lock is not enough to ensure
virtqueue's metadata such as vring pointers are valid.

The access status must also be checked.

Fixes: 78639d54563a ("vhost: introduce async enqueue registration API")
Cc: sta...@dpdk.org

Signed-off-by: Maxime Coquelin 
---
 lib/vhost/vhost.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c
index 5428ff4a25..ccd3c0e865 100644
--- a/lib/vhost/vhost.c
+++ b/lib/vhost/vhost.c
@@ -1857,7 +1857,15 @@ rte_vhost_async_channel_register(int vid, uint16_t 
queue_id)
return -1;
 
rte_rwlock_write_lock(&vq->access_lock);
+
+   if (unlikely(!vq->access_ok)) {
+   ret = -1;
+   goto out_unlock;
+   }
+
ret = async_channel_register(dev, vq);
+
+out_unlock:
rte_rwlock_write_unlock(&vq->access_lock);
 
return ret;
@@ -1909,6 +1917,11 @@ rte_vhost_async_channel_unregister(int vid, uint16_t 
queue_id)
return ret;
}
 
+   if (unlikely(!vq->access_ok)) {
+   ret = -1;
+   goto out_unlock;
+   }
+
if (!vq->async) {
ret = 0;
} else if (vq->async->pkts_inflight_n) {
@@ -1920,6 +1933,7 @@ rte_vhost_async_channel_unregister(int vid, uint16_t 
queue_id)
ret = 0;
}
 
+out_unlock:
rte_rwlock_write_unlock(&vq->access_lock);
 
return ret;
-- 
2.41.0

[PATCH v2 6/7] vhost: fix missing lock protection in power monitor API

[Maxime Coquelin]

The power monitor get API is missing both access lock
protection and access status check.

Fixes: 34fd4373ce76 ("vhost: add power monitor API")
Cc: sta...@dpdk.org

Signed-off-by: Maxime Coquelin 
---
 lib/vhost/vhost.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c
index a243f88398..d8d74623d4 100644
--- a/lib/vhost/vhost.c
+++ b/lib/vhost/vhost.c
@@ -2121,6 +2121,7 @@ rte_vhost_get_monitor_addr(int vid, uint16_t queue_id,
 {
struct virtio_net *dev = get_device(vid);
struct vhost_virtqueue *vq;
+   int ret = 0;
 
if (dev == NULL)
return -1;
@@ -2131,6 +2132,13 @@ rte_vhost_get_monitor_addr(int vid, uint16_t queue_id,
if (vq == NULL)
return -1;
 
+   rte_rwlock_read_lock(&vq->access_lock);
+
+   if (unlikely(!vq->access_ok)) {
+   ret = -1;
+   goto out_unlock;
+   }
+
if (vq_is_packed(dev)) {
struct vring_packed_desc *desc;
desc = vq->desc_packed;
@@ -2150,7 +2158,10 @@ rte_vhost_get_monitor_addr(int vid, uint16_t queue_id,
pmc->match = 0;
}
 
-   return 0;
+out_unlock:
+   rte_rwlock_read_unlock(&vq->access_lock);
+
+   return ret;
 }
 
 
-- 
2.41.0

Re: [PATCH] net/virtio: fix the setting of the vector for link state interrupt

[Maxime Coquelin]

Hi Wenwu,

Please reword the commit title to something:
net/virtio: fix link state interrupt vector setting

On 8/7/23 05:15, Wenwu Ma wrote:

The settings of the vector for link state interrupts
should be done before the initialization of the device
is completed.

Fixes: ee85024cf5f7 ("net/virtio: complete init stage at the right place")
Cc: sta...@dpdk.org

Signed-off-by: Wenwu Ma 
---
  drivers/net/virtio/virtio_ethdev.c | 16 
  1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c 
b/drivers/net/virtio/virtio_ethdev.c
index 2c23f1c00e..1801b0ae47 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1912,6 +1912,14 @@ virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t 
req_features)
}
}
  
+	if (eth_dev->data->dev_flags & RTE_ETH_DEV_INTR_LSC)

+   /* Enable vector (0) for Link State Interrupt */
+   if (VIRTIO_OPS(hw)->set_config_irq(hw, 0) ==
+   VIRTIO_MSI_NO_VECTOR) {
+   PMD_DRV_LOG(ERR, "failed to set config vector");
+   return -EBUSY;
+   }
+
virtio_reinit_complete(hw);
  
  	return 0;

@@ -2237,14 +2245,6 @@ virtio_dev_configure(struct rte_eth_dev *dev)
hw->has_tx_offload = tx_offload_enabled(hw);
hw->has_rx_offload = rx_offload_enabled(hw);
  
-	if (dev->data->dev_flags & RTE_ETH_DEV_INTR_LSC)

-   /* Enable vector (0) for Link State Interrupt */
-   if (VIRTIO_OPS(hw)->set_config_irq(hw, 0) ==
-   VIRTIO_MSI_NO_VECTOR) {
-   PMD_DRV_LOG(ERR, "failed to set config vector");
-   return -EBUSY;
-   }
-
if (virtio_with_packed_queue(hw)) {
  #if defined(RTE_ARCH_X86_64) && defined(CC_AVX512_SUPPORT)
if ((hw->use_vec_rx || hw->use_vec_tx) &&


It looks good to me, so I can change the title myself while applying if
Ok for you.

Reviewed-by: Maxime Coquelin 

By the way, can you tell me with which backends have you tested it with?
Only Virtio-PCI? Or also Virtio-user?

Thanks,
Maxime

Re: [PATCH] maintainers: update email address

[Maxime Coquelin]

On 10/20/23 10:51, Chenbo Xia wrote:

I left Intel and joined Nvidia, so update my email address.

Signed-off-by: Chenbo Xia 
---
  .mailmap|  2 +-
  MAINTAINERS | 12 ++--
  2 files changed, 7 insertions(+), 7 deletions(-)



Welcime back Chenbo :)

Acked-by: Maxime Coquelin 

Maxime

Re: [PATCH v1 06/11] test/bbdev: assert failed test for queue configure

[Maxime Coquelin]

On 10/19/23 14:12, Chautru, Nicolas wrote:

Hi Maxime,


-Original Message-
From: Maxime Coquelin 
Sent: Thursday, October 19, 2023 10:47 AM
To: Chautru, Nicolas ; Vargas, Hernan
; dev@dpdk.org; gak...@marvell.com; Rix, Tom

Cc: Zhang, Qi Z 
Subject: Re: [PATCH v1 06/11] test/bbdev: assert failed test for queue configure

Hi Nicolas,

On 10/19/23 10:41, Chautru, Nicolas wrote:

Hi Maxime,

Do we really want to make these kind of changes on to the stable release, it

tends to artificially increase the amount of churn on the stable release which
can be counterproductive for such changes which don't add much value if any
to user/developper.

Happy to follow your suggestion but a general feedback is lack of appetite for

very large amount of changes in stable patches which inhibit adoption, so
would expect to put things there that we would genuinely flag as a bug.

Kindly share your thoughts.


Checking for configuration failure in a test application is quite useful in my
opinion, as it can help catching regressions, isn't it?


I don’t personally think this (or for other commit on that serie) hits that bar 
for being required in stable release. This ends up being counterproductive 
having stable release with a huge amount of commits that are not really 
required, and it ends up being a reason for people not to move to stable 
release.
But if you are really convinced, ok to follow your reco.


Adding LTS maintainers if they want to step in.

Personally, I think ot should be backported.

Maxime



Maxime

Thanks
Nic



-Original Message-----
From: Maxime Coquelin 
Sent: Tuesday, October 17, 2023 9:43 PM
To: Vargas, Hernan ; dev@dpdk.org;
gak...@marvell.com; Rix, Tom 
Cc: Chautru, Nicolas ; Zhang, Qi Z

Subject: Re: [PATCH v1 06/11] test/bbdev: assert failed test for
queue configure



On 9/29/23 20:13, Hernan Vargas wrote:

Stop test if rte_bbdev_queue_configure fails to configure queue.

Signed-off-by: Hernan Vargas 
---
app/test-bbdev/test_bbdev.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/test-bbdev/test_bbdev.c
b/app/test-bbdev/test_bbdev.c index 65805977aead..cf224dca5d04
100644
--- a/app/test-bbdev/test_bbdev.c
+++ b/app/test-bbdev/test_bbdev.c
@@ -366,7 +366,8 @@ test_bbdev_configure_stop_queue(void)
 * - queue should be started if deferred_start ==
 */
ts_params->qconf.deferred_start = 0;
-   rte_bbdev_queue_configure(dev_id, queue_id, &ts_params->qconf);
+   TEST_ASSERT_SUCCESS(rte_bbdev_queue_configure(dev_id, queue_id,

&ts_params->qconf),

+   "Failed test for rte_bbdev_queue_configure");
rte_bbdev_start(dev_id);

TEST_ASSERT_SUCCESS(return_value =

rte_bbdev_queue_info_get(dev_id,

If should be a fix IMO.
With fixes tag added and stable cc'ed:

Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [PATCH v1 06/11] test/bbdev: assert failed test for queue configure

[Maxime Coquelin]

With LTS maintainers actually added... sorry.

On 10/23/23 11:05, Maxime Coquelin wrote:



On 10/19/23 14:12, Chautru, Nicolas wrote:

Hi Maxime,


-Original Message-
From: Maxime Coquelin 
Sent: Thursday, October 19, 2023 10:47 AM
To: Chautru, Nicolas ; Vargas, Hernan
; dev@dpdk.org; gak...@marvell.com; Rix, Tom

Cc: Zhang, Qi Z 
Subject: Re: [PATCH v1 06/11] test/bbdev: assert failed test for 
queue configure


Hi Nicolas,

On 10/19/23 10:41, Chautru, Nicolas wrote:

Hi Maxime,

Do we really want to make these kind of changes on to the stable 
release, it
tends to artificially increase the amount of churn on the stable 
release which
can be counterproductive for such changes which don't add much value 
if any

to user/developper.
Happy to follow your suggestion but a general feedback is lack of 
appetite for
very large amount of changes in stable patches which inhibit 
adoption, so

would expect to put things there that we would genuinely flag as a bug.

Kindly share your thoughts.


Checking for configuration failure in a test application is quite 
useful in my

opinion, as it can help catching regressions, isn't it?


I don’t personally think this (or for other commit on that serie) hits 
that bar for being required in stable release. This ends up being 
counterproductive having stable release with a huge amount of commits 
that are not really required, and it ends up being a reason for people 
not to move to stable release.

But if you are really convinced, ok to follow your reco.


Adding LTS maintainers if they want to step in.

Personally, I think ot should be backported.

Maxime



Maxime

Thanks
Nic



-Original Message-----
From: Maxime Coquelin 
Sent: Tuesday, October 17, 2023 9:43 PM
To: Vargas, Hernan ; dev@dpdk.org;
gak...@marvell.com; Rix, Tom 
Cc: Chautru, Nicolas ; Zhang, Qi Z

Subject: Re: [PATCH v1 06/11] test/bbdev: assert failed test for
queue configure



On 9/29/23 20:13, Hernan Vargas wrote:

Stop test if rte_bbdev_queue_configure fails to configure queue.

Signed-off-by: Hernan Vargas 
---
    app/test-bbdev/test_bbdev.c | 3 ++-
    1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/test-bbdev/test_bbdev.c
b/app/test-bbdev/test_bbdev.c index 65805977aead..cf224dca5d04
100644
--- a/app/test-bbdev/test_bbdev.c
+++ b/app/test-bbdev/test_bbdev.c
@@ -366,7 +366,8 @@ test_bbdev_configure_stop_queue(void)
 * - queue should be started if deferred_start ==
 */
    ts_params->qconf.deferred_start = 0;
-    rte_bbdev_queue_configure(dev_id, queue_id, &ts_params->qconf);
+    TEST_ASSERT_SUCCESS(rte_bbdev_queue_configure(dev_id, queue_id,

&ts_params->qconf),

+    "Failed test for rte_bbdev_queue_configure");
    rte_bbdev_start(dev_id);

    TEST_ASSERT_SUCCESS(return_value =

rte_bbdev_queue_info_get(dev_id,

If should be a fix IMO.
With fixes tag added and stable cc'ed:

Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [PATCH v2 1/2] doc: bbdev device discovery clarification

[Maxime Coquelin]

   int socket_id;  /**< NUMA socket that device is on */
const char *dev_name;  /**< Unique device name */
@@ -369,6 +374,7 @@ struct rte_bbdev_info {
bool started;  /**< Set if device is currently started */
struct rte_bbdev_driver_info drv;  /**< Info from device driver */
  };
+/* >8 End of structure rte_bbdev_info. */
  
  /**

   * Retrieve information about a device.


Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [PATCH v2 2/2] doc/guides: refer to generic bbdev test section

[Maxime Coquelin]

On 10/13/23 23:42, Nicolas Chautru wrote:

Avoiding duplication among each PMD doc, now referring directly to the
relevant details section.

Signed-off-by: Nicolas Chautru 
---
  doc/guides/bbdevs/acc100.rst| 33 +++-
  doc/guides/bbdevs/fpga_5gnr_fec.rst | 39 +++-
  doc/guides/bbdevs/fpga_lte_fec.rst  | 40 +++--
  doc/guides/bbdevs/la12xx.rst| 32 +++
  doc/guides/bbdevs/vrb1.rst  | 36 +++---
  doc/guides/bbdevs/vrb2.rst  | 36 +++---
  doc/guides/tools/testbbdev.rst  |  2 ++
  7 files changed, 22 insertions(+), 196 deletions(-)



Thanks for removing these duplications.

Reviewed-by: Maxime Coquelin 

Maxime

Re: [PATCH 00/20] remove experimental flag from some API's

[Maxime Coquelin]

On 8/8/23 19:35, Stephen Hemminger wrote:

Since 23.11 is an LTS release it is time to remove the experimental
bandaid off many API's. There are about 850 API's marked with experimental
on current main branch. This addresses the easy to remove ones and
gets it down to about 690 places.

The rule is any API that has been in since 22.11 needs to have
experimental removed (or deleted). The experimental flag is not a
"get out of ABI stability for free" card.

Stephen Hemminger (20):
   bpf: make rte_bpf_dump and rte_bpf_convert stable API's
   cmdline: make experimental API's stable
   ethdev: mark rte_mtr API's as stable
   ethdev: mark rte_tm API's as stable
   pdump: make API's stable
   pcapng: mark API's as stable
   net: remove experimental from functions
   rcu: remove experimental from rte_rcu_qbsr
   lpm: remove experimental
   mbuf: remove experimental from create_extbuf
   hash: remove experimental from toeplitz hash
   timer: remove experimental from rte_timer_next_ticks
   sched: remove experimental
   dmadev: mark API's as not experimental
   meter: remove experimental warning from comments
   power: remove experimental from API's
   kvargs: remove experimental flag
   ip_frag: mark a couple of functions stable
   member: remove experimental tag
   security: remove experimental flag

  lib/bpf/rte_bpf.h   |  2 -
  lib/bpf/version.map |  9 +--
  lib/cmdline/cmdline.h   |  1 -
  lib/cmdline/cmdline_parse.h |  4 --
  lib/cmdline/cmdline_rdline.h|  4 --
  lib/cmdline/version.map | 26 +++--
  lib/dmadev/rte_dmadev.h | 85 
  lib/dmadev/version.map  |  2 +-
  lib/ethdev/rte_mtr.h| 25 +---
  lib/ethdev/rte_tm.h | 34 ---
  lib/ethdev/version.map  | 88 ++---
  lib/hash/rte_thash.h| 44 ---
  lib/hash/rte_thash_gfni.h   |  8 ---
  lib/hash/rte_thash_x86_gfni.h   |  8 ---
  lib/hash/version.map| 16 ++
  lib/ip_frag/rte_ip_frag.h   |  2 -
  lib/ip_frag/version.map |  9 +--
  lib/kvargs/rte_kvargs.h |  4 --
  lib/kvargs/version.map  |  8 +--
  lib/lpm/rte_lpm.h   |  4 --
  lib/lpm/version.map |  7 +--
  lib/mbuf/rte_mbuf.h |  1 -
  lib/mbuf/version.map|  8 +--
  lib/member/rte_member.h | 54 --
  lib/member/version.map  | 12 +---
  lib/meter/rte_meter.h   | 12 
  lib/net/rte_ip.h| 19 ---
  lib/pcapng/rte_pcapng.h | 11 
  lib/pcapng/version.map  |  6 +-
  lib/pdump/rte_pdump.h   | 12 
  lib/pdump/version.map   | 11 +---
  lib/power/rte_power.h   |  4 --
  lib/power/rte_power_guest_channel.h |  4 --
  lib/power/rte_power_intel_uncore.h  |  9 ---
  lib/power/rte_power_pmd_mgmt.h  | 40 -
  lib/power/version.map   | 33 ---
  lib/rcu/rte_rcu_qsbr.h  | 20 ---
  lib/rcu/version.map | 15 ++---
  lib/sched/rte_pie.h |  8 ---
  lib/sched/rte_sched.h   |  5 --
  lib/sched/version.map   | 18 ++
  lib/security/rte_security.h | 35 
  lib/security/version.map| 17 ++
  lib/timer/rte_timer.h   |  4 --
  lib/timer/version.map   |  7 +--
  45 files changed, 97 insertions(+), 658 deletions(-)



You removed Vhost changes altogether, but I only asked for the Vhost
Async API changes to be removed, i.e. the APIs in rte_vhost_async.h.

Maxime

Re: [PATCH v1 0/1] doc: bbdev device discovery clarification

[Maxime Coquelin]

On 10/10/23 22:34, Nicolas Chautru wrote:

Adding more information in bbdev documentation related to the
bbdev device discovery from info_get which was not very verbose so far.
Notably for FEC and FFT operations which have extra parameters to
manage different implementation variants.

Also use code snippet to refer to info structure and keep the doc
in sync moving forward.

This is on top of this serie
https://patches.dpdk.org/project/dpdk/list/?series=29744

Nicolas Chautru (1):
   doc: bbdev device discovery clarification

  doc/guides/prog_guide/bbdev.rst | 60 -
  lib/bbdev/rte_bbdev.h   |  6 
  2 files changed, 57 insertions(+), 9 deletions(-)



Applied to next-baseband/for-main.

Thanks,
Maxime

Re: [PATCH v2 0/7] vhost: ensure virtqueue access status is checked

[Maxime Coquelin]

On 10/20/23 10:47, Maxime Coquelin wrote:

Li Feng initially reported segmentation fault in rte_vhost_vring_call()
because of not checking the virtqueue metadata can be accessed.

This should be achieved by checking the access_ok status field of
the virtqueue.

This series also takes the opportunity to fix the other APIs.
This is split in multiple patches to ease LTS maintainers backports,
but could be squashed if preferred.

Changes in v2:
--
- Rebased to apply on -rc1 (David)
- Add Fixes tag in patch 1 (David)
- Fix various typos in commit logs (David)

Maxime Coquelin (7):
   vhost: fix missing vring call check on virtqueue access
   vhost: fix missing check on virtqueue access
   vhost: fix checking virtqueue access when notifying guest
   vhost: fix check on virtqueue access in async registration
   vhost: fix check on virtqueue access in in-flight getter
   vhost: fix missing lock protection in power monitor API
   vhost: fix checking virtqueue access in stats API

  lib/vhost/vhost.c | 92 +++
  1 file changed, 85 insertions(+), 7 deletions(-)



Applied to next-virtio/for-next-net

Thanks,
Maxime

Re: [PATCH v2] net/virtio: fix link state interrupt vector setting

[Maxime Coquelin]

On 10/23/23 03:46, Wenwu Ma wrote:

The settings of the vector for link state interrupts
should be done before the initialization of the device
is completed.

Fixes: ee85024cf5f7 ("net/virtio: complete init stage at the right place")
Cc: sta...@dpdk.org

Signed-off-by: Wenwu Ma 
Tested-by: Wei Ling 
Reviewed-by: Maxime Coquelin 
---
v2:
  - rewording of the title

---
  drivers/net/virtio/virtio_ethdev.c | 16 
  1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c 
b/drivers/net/virtio/virtio_ethdev.c
index 3ab56ef769..c2c0a1a111 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1912,6 +1912,14 @@ virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t 
req_features)
}
}
  
+	if (eth_dev->data->dev_flags & RTE_ETH_DEV_INTR_LSC)

+   /* Enable vector (0) for Link State Interrupt */
+   if (VIRTIO_OPS(hw)->set_config_irq(hw, 0) ==
+   VIRTIO_MSI_NO_VECTOR) {
+   PMD_DRV_LOG(ERR, "failed to set config vector");
+   return -EBUSY;
+   }
+
virtio_reinit_complete(hw);
  
  	return 0;

@@ -2237,14 +2245,6 @@ virtio_dev_configure(struct rte_eth_dev *dev)
hw->has_tx_offload = tx_offload_enabled(hw);
hw->has_rx_offload = rx_offload_enabled(hw);
  
-	if (dev->data->dev_flags & RTE_ETH_DEV_INTR_LSC)

-   /* Enable vector (0) for Link State Interrupt */
-   if (VIRTIO_OPS(hw)->set_config_irq(hw, 0) ==
-   VIRTIO_MSI_NO_VECTOR) {
-   PMD_DRV_LOG(ERR, "failed to set config vector");
-   return -EBUSY;
-   }
-
if (virtio_with_packed_queue(hw)) {
  #if defined(RTE_ARCH_X86_64) && defined(CC_AVX512_SUPPORT)
if ((hw->use_vec_rx || hw->use_vec_tx) &&


Applied to next-virtio/for-next-net

Thanks,
Maxime

Re: [PATCH] net/virtio: fixed missing next flag when sending packets in packed mode

[Maxime Coquelin]

On 10/17/23 09:26, Fengjiang Liu wrote:

When the packets is sent in packed mode, and the packets data and
virtio-header are divided into two desc, set the next flag of
virtio-header desc

Bugzilla ID: 1295
Fixes: 892dc798fa9c ("net/virtio: implement Tx path for packed queues")

Signed-off-by: Fengjiang Liu 
---
  drivers/net/virtio/virtqueue.h | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h
index 9d4aba11a3..4e9f2d0358 100644
--- a/drivers/net/virtio/virtqueue.h
+++ b/drivers/net/virtio/virtqueue.h
@@ -672,6 +672,7 @@ virtqueue_enqueue_xmit_packed(struct virtnet_tx *txvq, 
struct rte_mbuf *cookie,
 */
start_dp[idx].addr = txvq->hdr_mem + 
RTE_PTR_DIFF(&txr[idx].tx_hdr, txr);
start_dp[idx].len = vq->hw->vtnet_hdr_size;
+   head_flags |= VRING_DESC_F_NEXT;
hdr = (struct virtio_net_hdr *)&txr[idx].tx_hdr;
idx++;
if (idx >= vq->vq_nentries) {


Applied to next-virtio/for-next-net

Thanks,
Maxime

Re: [PATCH v1 1/2] baseband/acc: support ACC100 deRM corner case SDK

[Maxime Coquelin]

Hi Nicolas,

On 10/18/23 12:56, David Marchand wrote:

On Tue, Oct 10, 2023 at 7:55 PM Hernan Vargas  wrote:


Implement de-ratematch pre-processing for ACC100 SW corner cases.
Some specific 5GUL FEC corner cases may cause unintended back pressure
and in some cases a potential stability issue on the ACC100.
The PMD can detect such code block configuration and issue an info
message to the user.

Signed-off-by: Hernan Vargas 
---
  drivers/baseband/acc/meson.build  | 23 ++-
  drivers/baseband/acc/rte_acc100_pmd.c | 59 +--
  2 files changed, 77 insertions(+), 5 deletions(-)

diff --git a/drivers/baseband/acc/meson.build b/drivers/baseband/acc/meson.build
index 27a654b50153..84f4fea635ef 100644
--- a/drivers/baseband/acc/meson.build
+++ b/drivers/baseband/acc/meson.build
@@ -1,7 +1,28 @@
  # SPDX-License-Identifier: BSD-3-Clause
  # Copyright(c) 2020 Intel Corporation

-deps += ['bus_pci']

...

+deps += ['bbdev', 'bus_pci']


This part is likely a rebase damage.
See: b7b8de26f34d ("drivers: add dependencies for some classes")




Do you plan to send a new version² fixing the rebase issue?

Thanks,
Maxime

Re: Series for 23.11

[Maxime Coquelin]

Hi Nicolas;

On 10/16/23 16:49, Chautru, Nicolas wrote:

Hi Maxime,

Just a heads up that Hernan is going on paternity leave, I will be 
covering for his series pending for 23.11. Ping me if there is any 
update required from review that I may have missed.


Pending series in patchwork:

  * SDK + doc update
https://patches.dpdk.org/project/dpdk/list/?series=29797

  * FPGA pmd update https://patches.dpdk.org/project/dpdk/list/?series=29537


We'd need new revision for this one by tomorrow if we want it in 23.11.


  * test-bbdev updates
https://patches.dpdk.org/project/dpdk/list/?series=29705
  * Doc update https://patches.dpdk.org/project/dpdk/list/?series=29840


For others, I think we can delay until -rc3, even if not ideal.

Thanks,
Maxime


Thanks!

Nic

*From:*Vargas, Hernan 
*Sent:* Monday, October 2, 2023 7:44 AM
*To:* dev@dpdk.org; maxime.coque...@redhat.com; gak...@marvell.com; Rix, 
Tom 
*Cc:* Chautru, Nicolas ; Zhang, Qi Z 


*Subject:* Series for 23.11

Hi Maxime,

Kind reminder to review these series for 23.11 in order of priority:

 1. https://patches.dpdk.org/project/dpdk/list/?series=29558

 2. https://patches.dpdk.org/project/dpdk/list/?series=29537

 3. https://patches.dpdk.org/project/dpdk/list/?series=29705


Thanks,

Hernan

Re: [PATCH v3 13/19] vhost: use rte optional stdatomic API

[Maxime Coquelin]

On 10/26/23 02:31, Tyler Retzlaff wrote:

Replace the use of gcc builtin __atomic_xxx intrinsics with
corresponding rte_atomic_xxx optional stdatomic API

Signed-off-by: Tyler Retzlaff 
---
  lib/vhost/vdpa.c|  3 ++-
  lib/vhost/vhost.c   | 42 
  lib/vhost/vhost.h   | 39 --
  lib/vhost/vhost_user.c  |  6 ++---
  lib/vhost/virtio_net.c  | 58 +
  lib/vhost/virtio_net_ctrl.c |  6 +++--
  6 files changed, 84 insertions(+), 70 deletions(-)



Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [PATCH v3 02/19] bbdev: use rte optional stdatomic API

[Maxime Coquelin]

On 10/26/23 02:31, Tyler Retzlaff wrote:

Replace the use of gcc builtin __atomic_xxx intrinsics with corresponding
rte_atomic_xxx optional stdatomic API

Signed-off-by: Tyler Retzlaff 
---
  lib/bbdev/rte_bbdev.c | 6 +++---
  lib/bbdev/rte_bbdev.h | 2 +-
  2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/bbdev/rte_bbdev.c b/lib/bbdev/rte_bbdev.c
index 155323e..cfebea0 100644
--- a/lib/bbdev/rte_bbdev.c
+++ b/lib/bbdev/rte_bbdev.c
@@ -208,7 +208,7 @@ struct rte_bbdev *
return NULL;
}
  
-	__atomic_fetch_add(&bbdev->data->process_cnt, 1, __ATOMIC_RELAXED);

+   rte_atomic_fetch_add_explicit(&bbdev->data->process_cnt, 1, 
rte_memory_order_relaxed);
bbdev->data->dev_id = dev_id;
bbdev->state = RTE_BBDEV_INITIALIZED;
  
@@ -250,8 +250,8 @@ struct rte_bbdev *

}
  
  	/* clear shared BBDev Data if no process is using the device anymore */

-   if (__atomic_fetch_sub(&bbdev->data->process_cnt, 1,
- __ATOMIC_RELAXED) - 1 == 0)
+   if (rte_atomic_fetch_sub_explicit(&bbdev->data->process_cnt, 1,
+ rte_memory_order_relaxed) - 1 == 0)
memset(bbdev->data, 0, sizeof(*bbdev->data));
  
  	memset(bbdev, 0, sizeof(*bbdev));

diff --git a/lib/bbdev/rte_bbdev.h b/lib/bbdev/rte_bbdev.h
index d12e2e7..e1aee08 100644
--- a/lib/bbdev/rte_bbdev.h
+++ b/lib/bbdev/rte_bbdev.h
@@ -482,7 +482,7 @@ struct rte_bbdev_data {
uint16_t dev_id;  /**< Device ID */
int socket_id;  /**< NUMA socket that device is on */
bool started;  /**< Device run-time state */
-   uint16_t process_cnt;  /** Counter of processes using the device */
+   RTE_ATOMIC(uint16_t) process_cnt;  /** Counter of processes using the 
device */
  };
  
  /* Forward declarations */


Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [PATCH v3 01/30] baseband/acc100: fix ring availability calculation

[Maxime Coquelin]

Hi Hernan,

On 10/12/22 04:53, Hernan Vargas wrote:

Refactor of the queue availability computation to prevent the
application to dequeue more than what may have been enqueued.

Fixes: 5ad5060f8f7 ("baseband/acc100: add LDPC processing functions")
Cc: sta...@dpdk.org

Signed-off-by: Hernan Vargas 
---
  drivers/baseband/acc/rte_acc100_pmd.c | 25 +
  1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c 
b/drivers/baseband/acc/rte_acc100_pmd.c
index e84d9f2511..733766ad3e 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -2876,7 +2876,7 @@ acc100_enqueue_enc_cb(struct rte_bbdev_queue_data *q_data,
struct rte_bbdev_enc_op **ops, uint16_t num)
  {
struct acc_queue *q = q_data->queue_private;
-   int32_t avail = q->sw_ring_depth + q->sw_ring_tail - q->sw_ring_head;
+   int32_t avail = acc_ring_avail_enq(q);


That's problematic because the patch introducing acc_ring_avail_enq() is
a feature patch for v22.11 and not a fix. You'll have to help the LTS
maintainers to backport it.

Other than that:
Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [PATCH v3 02/30] baseband/acc100: add function to check AQ availability

[Maxime Coquelin]

On 10/12/22 04:53, Hernan Vargas wrote:

It is possible for some corner case to run more batch enqueue than
supported. A protection is required to avoid that corner case.
Enhance all ACC100 enqueue operations with check to see if there is room
in the atomic queue for enqueueing batches into the queue manager
Check room in AQ for the enqueues batches into Qmgr

Fixes: 5ad5060f8f7 ("baseband/acc100: add LDPC processing functions")
Cc: sta...@dpdk.org

Signed-off-by: Hernan Vargas 
---
  drivers/baseband/acc/rte_acc100_pmd.c | 30 ---
  1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c 
b/drivers/baseband/acc/rte_acc100_pmd.c
index 733766ad3e..b436bd9078 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -2995,12 +2995,27 @@ acc100_enqueue_enc_tb(struct rte_bbdev_queue_data 
*q_data,
return i;
  }
  
+/* Check room in AQ for the enqueues batches into Qmgr */

+static int32_t
+acc100_aq_avail(struct rte_bbdev_queue_data *q_data, uint16_t num_ops)
+{
+   struct acc_queue *q = q_data->queue_private;
+   int32_t aq_avail = q->aq_depth -
+   ((q->aq_enqueued - q->aq_dequeued + ACC_MAX_QUEUE_DEPTH)
+   % ACC_MAX_QUEUE_DEPTH) - (num_ops >> 7);
+   if (aq_avail <= 0)


Maybe use unlikely() here.



+   acc_enqueue_queue_full(q_data);
+
+   return aq_avail;
+}
+
  /* Enqueue encode operations for ACC100 device. */
  static uint16_t
  acc100_enqueue_enc(struct rte_bbdev_queue_data *q_data,
struct rte_bbdev_enc_op **ops, uint16_t num)
  {
-   if (unlikely(num == 0))
+   int32_t aq_avail = acc100_aq_avail(q_data, num);


Please insert new line for readability.


+   if (unlikely((aq_avail <= 0) || (num == 0)))
return 0;


Ditto

if (ops[0]->turbo_enc.code_block_mode == RTE_BBDEV_TRANSPORT_BLOCK)
return acc100_enqueue_enc_tb(q_data, ops, num);
@@ -3013,7 +3028,8 @@ static uint16_t
  acc100_enqueue_ldpc_enc(struct rte_bbdev_queue_data *q_data,
struct rte_bbdev_enc_op **ops, uint16_t num)
  {
-   if (unlikely(num == 0))
+   int32_t aq_avail = acc100_aq_avail(q_data, num);


New line.


+   if (unlikely((aq_avail <= 0) || (num == 0)))
return 0;
if (ops[0]->ldpc_enc.code_block_mode == RTE_BBDEV_TRANSPORT_BLOCK)
return acc100_enqueue_enc_tb(q_data, ops, num);
@@ -3183,7 +3199,8 @@ static uint16_t
  acc100_enqueue_dec(struct rte_bbdev_queue_data *q_data,
struct rte_bbdev_dec_op **ops, uint16_t num)
  {
-   if (unlikely(num == 0))
+   int32_t aq_avail = acc100_aq_avail(q_data, num);


New line.


+   if (unlikely((aq_avail <= 0) || (num == 0)))
return 0;


New line.


if (ops[0]->turbo_dec.code_block_mode == RTE_BBDEV_TRANSPORT_BLOCK)
return acc100_enqueue_dec_tb(q_data, ops, num);
@@ -3196,11 +3213,8 @@ static uint16_t
  acc100_enqueue_ldpc_dec(struct rte_bbdev_queue_data *q_data,
struct rte_bbdev_dec_op **ops, uint16_t num)
  {
-   struct acc_queue *q = q_data->queue_private;
-   int32_t aq_avail = q->aq_depth +
-   (q->aq_dequeued - q->aq_enqueued) / 128;
-
-   if (unlikely((aq_avail == 0) || (num == 0)))
+   int32_t aq_avail = acc100_aq_avail(q_data, num);


New line.


+   if (unlikely((aq_avail <= 0) || (num == 0)))
return 0;
  
  	if (ops[0]->ldpc_dec.code_block_mode == RTE_BBDEV_TRANSPORT_BLOCK)

Re: [PATCH v3 03/30] baseband/acc100: memory leak fix

[Maxime Coquelin]

On 10/12/22 04:53, Hernan Vargas wrote:

Move check for undefined device before allocating queue data structure.

Coverity issue: 375803, 375813, 375819, 375827, 375831
Fixes: 060e7672930 ("baseband/acc100: add queue configuration")
Cc: sta...@dpdk.org

Signed-off-by: Hernan Vargas 


No new line here.


Reviewed-by: Maxime Coquelin 
---
  drivers/baseband/acc/rte_acc100_pmd.c | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c 
b/drivers/baseband/acc/rte_acc100_pmd.c
index b436bd9078..436af977e4 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -662,6 +662,10 @@ acc100_queue_setup(struct rte_bbdev *dev, uint16_t 
queue_id,
struct acc_queue *q;
int16_t q_idx;
  
+	if (d == NULL) {

+   rte_bbdev_log(ERR, "Undefined device");
+   return -ENODEV;
+   }
/* Allocate the queue data structure. */
q = rte_zmalloc_socket(dev->device->driver->name, sizeof(*q),
RTE_CACHE_LINE_SIZE, conf->socket);
@@ -669,10 +673,6 @@ acc100_queue_setup(struct rte_bbdev *dev, uint16_t 
queue_id,
rte_bbdev_log(ERR, "Failed to allocate queue memory");
return -ENOMEM;
}
-   if (d == NULL) {
-   rte_bbdev_log(ERR, "Undefined device");
-   return -ENODEV;
-   }
  
  	q->d = d;

q->ring_addr = RTE_PTR_ADD(d->sw_rings, (d->sw_ring_size * queue_id));

Re: [PATCH v3 04/30] baseband/acc100: add LDPC encoder padding function

[Maxime Coquelin]

On 10/12/22 04:53, Hernan Vargas wrote:

LDPC Encoder input may need to be padded to avoid small beat for ACC100.
Padding 5GDL input buffer length (BLEN) to avoid case (BLEN % 64) <= 8.
Adding protection for corner case to avoid for 5GDL occurrence of last
beat within the ACC100 fabric with <= 8B which might trigger a fabric
corner case hang issue.

Fixes: 5ad5060f8f7 ("baseband/acc100: add LDPC processing functions")
Cc: sta...@dpdk.org

Signed-off-by: Hernan Vargas 
---
  drivers/baseband/acc/rte_acc100_pmd.c | 27 ---
  1 file changed, 20 insertions(+), 7 deletions(-)



Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [PATCH v3 05/30] baseband/acc100: check turbo dec/enc input

[Maxime Coquelin]

On 10/12/22 04:53, Hernan Vargas wrote:

Add NULL check for the turbo decoder and encoder input length.

Fixes: 3bfc5f60403 ("baseband/acc100: add debug function to validate
input")


Do not wrap the fixes line, even if checkpatch emits a warning.


Cc: sta...@dpdk.org

Signed-off-by: Hernan Vargas 

Reviewed-by: Maxime Coquelin 
---
  drivers/baseband/acc/rte_acc100_pmd.c | 23 ++-
  1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c 
b/drivers/baseband/acc/rte_acc100_pmd.c
index f636d4fa0f..3a008a3b88 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -1762,6 +1762,11 @@ validate_enc_op(struct rte_bbdev_enc_op *op, struct 
acc_queue *q)
return -1;
}
  
+	if (turbo_enc->input.length == 0) {


unlikely()?


+   rte_bbdev_log(ERR, "input length null");
+   return -1;
+   }
+
if (turbo_enc->code_block_mode == RTE_BBDEV_TRANSPORT_BLOCK) {
tb = &turbo_enc->tb_params;
if ((tb->k_neg < RTE_BBDEV_TURBO_MIN_CB_SIZE
@@ -1781,11 +1786,12 @@ validate_enc_op(struct rte_bbdev_enc_op *op, struct 
acc_queue *q)
RTE_BBDEV_TURBO_MAX_CB_SIZE);
return -1;
}
-   if (tb->c_neg > (RTE_BBDEV_TURBO_MAX_CODE_BLOCKS - 1))
+   if (tb->c_neg > 0) {


unlikely()?


rte_bbdev_log(ERR,
-   "c_neg (%u) is out of range 0 <= value <= 
%u",
-   tb->c_neg,
-   RTE_BBDEV_TURBO_MAX_CODE_BLOCKS - 1);
+   "c_neg (%u) expected to be null",
+   tb->c_neg);
+   return -1;
+   }
if (tb->c < 1 || tb->c > RTE_BBDEV_TURBO_MAX_CODE_BLOCKS) {
rte_bbdev_log(ERR,
"c (%u) is out of range 1 <= value <= 
%u",
@@ -2286,6 +2292,11 @@ validate_dec_op(struct rte_bbdev_dec_op *op, struct 
acc_queue *q)
return -1;
}
  
+	if (turbo_dec->input.length == 0) {


unlikely()?


+   rte_bbdev_log(ERR, "input length null");
+   return -1;
+   }
+
if (turbo_dec->code_block_mode == RTE_BBDEV_TRANSPORT_BLOCK) {
tb = &turbo_dec->tb_params;
if ((tb->k_neg < RTE_BBDEV_TURBO_MIN_CB_SIZE
@@ -2306,11 +2317,13 @@ validate_dec_op(struct rte_bbdev_dec_op *op, struct 
acc_queue *q)
RTE_BBDEV_TURBO_MAX_CB_SIZE);
return -1;
}
-   if (tb->c_neg > (RTE_BBDEV_TURBO_MAX_CODE_BLOCKS - 1))
+   if (tb->c_neg > (RTE_BBDEV_TURBO_MAX_CODE_BLOCKS - 1)) {


unlikely()?


rte_bbdev_log(ERR,
"c_neg (%u) is out of range 0 <= value <= 
%u",
tb->c_neg,
RTE_BBDEV_TURBO_MAX_CODE_BLOCKS - 1);
+   return -1;
+   }
if (tb->c < 1 || tb->c > RTE_BBDEV_TURBO_MAX_CODE_BLOCKS) {
rte_bbdev_log(ERR,
"c (%u) is out of range 1 <= value <= 
%u",

Re: [PATCH v3 06/30] baseband/acc100: check for unlikely operation vals

[Maxime Coquelin]

On 10/12/22 04:53, Hernan Vargas wrote:

Add unlikely checks for NULL operation values.

Fixes: f404dfe35cc ("baseband/acc100: support 4G processing")
Cc: sta...@dpdk.org

Signed-off-by: Hernan Vargas 

Reviewed-by: Maxime Coquelin 
---
  drivers/baseband/acc/rte_acc100_pmd.c | 10 ++
  1 file changed, 10 insertions(+)

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c 
b/drivers/baseband/acc/rte_acc100_pmd.c
index 3a008a3b88..5e8ed78559 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -2184,6 +2184,10 @@ enqueue_enc_one_op_tb(struct acc_queue *q, struct 
rte_bbdev_enc_op *op,
r = op->turbo_enc.tb_params.r;
  
  	while (mbuf_total_left > 0 && r < c) {

+   if (unlikely(input == 0)) {


This should be a NULL check, as input is a mbuf pointer.


+   rte_bbdev_log(ERR, "Not enough input segment");
+   return -EINVAL;
+   }
seg_total_left = rte_pktmbuf_data_len(input) - in_offset;
/* Set up DMA descriptor */
desc = q->ring_addr + ((q->sw_ring_head + total_enqueued_cbs)
@@ -3128,6 +3132,8 @@ acc100_enqueue_ldpc_dec_tb(struct rte_bbdev_queue_data 
*q_data,
break;
enqueued_cbs += ret;
}
+   if (unlikely(enqueued_cbs == 0))
+   return 0; /* Nothing to enqueue */
  
  	acc_dma_enqueue(q, enqueued_cbs, &q_data->queue_stats);
  
@@ -3669,6 +3675,8 @@ acc100_dequeue_dec(struct rte_bbdev_queue_data *q_data,

for (i = 0; i < dequeue_num; ++i) {
op = (q->ring_addr + ((q->sw_ring_tail + dequeued_cbs)
& q->sw_ring_wrap_mask))->req.op_addr;
+   if (unlikely(op == NULL))
+   break;
if (op->turbo_dec.code_block_mode == RTE_BBDEV_TRANSPORT_BLOCK)
ret = dequeue_dec_one_op_tb(q, &ops[i], dequeued_cbs,
&aq_dequeued);
@@ -3714,6 +3722,8 @@ acc100_dequeue_ldpc_dec(struct rte_bbdev_queue_data 
*q_data,
for (i = 0; i < dequeue_num; ++i) {
op = (q->ring_addr + ((q->sw_ring_tail + dequeued_cbs)
& q->sw_ring_wrap_mask))->req.op_addr;
+   if (unlikely(op == NULL))
+   break;
if (op->ldpc_dec.code_block_mode == RTE_BBDEV_TRANSPORT_BLOCK)
ret = dequeue_dec_one_op_tb(q, &ops[i], dequeued_cbs,
&aq_dequeued);

Re: [PATCH v3 07/30] baseband/acc100: enforce additional check on FCW

[Maxime Coquelin]

On 10/12/22 04:53, Hernan Vargas wrote:

Enforce additional check on Frame Control Word validity and add stronger
alignment for decompression mode.

Fixes: 5ad5060f8f7 ("baseband/acc100: add LDPC processing functions")
Cc: sta...@dpdk.org

Signed-off-by: Hernan Vargas 
---
  drivers/baseband/acc/acc100_pmd.h |  1 +
  drivers/baseband/acc/acc_common.h |  1 +
  drivers/baseband/acc/rte_acc100_pmd.c | 51 ---
  3 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/drivers/baseband/acc/acc100_pmd.h 
b/drivers/baseband/acc/acc100_pmd.h
index b325948904..28063b3db0 100644
--- a/drivers/baseband/acc/acc100_pmd.h
+++ b/drivers/baseband/acc/acc100_pmd.h
@@ -87,6 +87,7 @@
  #define ACC100_HARQ_DDR (512 * 1)
  #define ACC100_PRQ_DDR_VER   0x10092020
  #define ACC100_DDR_TRAINING_MAX (5000)
+#define ACC100_HARQ_ALIGN_COMP   256
  
  struct acc100_registry_addr {

unsigned int dma_ring_dl5g_hi;
diff --git a/drivers/baseband/acc/acc_common.h 
b/drivers/baseband/acc/acc_common.h
index b3ac9800d1..b18319c06d 100644
--- a/drivers/baseband/acc/acc_common.h
+++ b/drivers/baseband/acc/acc_common.h
@@ -119,6 +119,7 @@
  
  #define ACC_ALGO_SPA0

  #define ACC_ALGO_MSA1
+#define ACC_HARQ_ALIGN_64B  64
  
  /* Helper macro for logging */

  #define rte_acc_log(level, fmt, ...) \
diff --git a/drivers/baseband/acc/rte_acc100_pmd.c 
b/drivers/baseband/acc/rte_acc100_pmd.c
index 5e8ed78559..c1446b3721 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -1038,6 +1038,7 @@ acc100_fcw_ld_fill(struct rte_bbdev_dec_op *op, struct 
acc_fcw_ld *fcw,
uint16_t harq_index;
uint32_t l;
bool harq_prun = false;
+   uint32_t max_hc_in;
  
  	fcw->qm = op->ldpc_dec.q_m;

fcw->nfiller = op->ldpc_dec.n_filler;
@@ -1087,8 +1088,14 @@ acc100_fcw_ld_fill(struct rte_bbdev_dec_op *op, struct 
acc_fcw_ld *fcw,
harq_in_length = op->ldpc_dec.harq_combined_input.length;
if (fcw->hcin_decomp_mode > 0)
harq_in_length = harq_in_length * 8 / 6;
-   harq_in_length = RTE_ALIGN(harq_in_length, 64);
-   if ((harq_layout[harq_index].offset > 0) & harq_prun) {
+   harq_in_length = RTE_MIN(harq_in_length, op->ldpc_dec.n_cb
+   - op->ldpc_dec.n_filler);
+   /* Alignment on next 64B - Already enforced from HC output */
+   harq_in_length = RTE_ALIGN_FLOOR(harq_in_length, 
ACC_HARQ_ALIGN_64B);
+   /* Stronger alignment requirement when in decompression mode */
+   if (fcw->hcin_decomp_mode > 0)
+   harq_in_length = RTE_ALIGN_FLOOR(harq_in_length, 
ACC100_HARQ_ALIGN_COMP);
+   if ((harq_layout[harq_index].offset > 0) && harq_prun) {


New lines in the above chunk would provide more clarity.
This is very packed.


rte_bbdev_log_debug("HARQ IN offset unexpected for 
now\n");
fcw->hcin_size0 = harq_layout[harq_index].size0;
fcw->hcin_offset = harq_layout[harq_index].offset;
@@ -1104,6 +,20 @@ acc100_fcw_ld_fill(struct rte_bbdev_dec_op *op, struct 
acc_fcw_ld *fcw,
fcw->hcin_offset = 0;
fcw->hcin_size1 = 0;
}
+   /* Enforce additional check on FCW validity */
+   max_hc_in = RTE_ALIGN_CEIL(fcw->ncb - fcw->nfiller, ACC_HARQ_ALIGN_64B);
+   if ((fcw->hcin_size0 > max_hc_in) ||
+   (fcw->hcin_size1 + fcw->hcin_offset > max_hc_in) ||
+   ((fcw->hcin_size0 > fcw->hcin_offset) &&
+   (fcw->hcin_size1 != 0))) {
+   rte_bbdev_log(ERR, " Invalid FCW : HCIn %d %d %d, Ncb %d F %d",
+   fcw->hcin_size0, fcw->hcin_size1,
+   fcw->hcin_offset,
+   fcw->ncb, fcw->nfiller);
+   /* Disable HARQ input in that case to carry forward */
+   op->ldpc_dec.op_flags ^= RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE;
+   fcw->hcin_en = 0;
+   }
  
  	fcw->itmax = op->ldpc_dec.iter_max;

fcw->itstop = check_bit(op->ldpc_dec.op_flags,
@@ -1132,10 +1153,19 @@ acc100_fcw_ld_fill(struct rte_bbdev_dec_op *op, struct 
acc_fcw_ld *fcw,
k0_p = (fcw->k0 > parity_offset) ?
fcw->k0 - op->ldpc_dec.n_filler : fcw->k0;
ncb_p = fcw->ncb - op->ldpc_dec.n_filler;
-   l = k0_p + fcw->rm_e;
+   l = RTE_MIN(k0_p + fcw->rm_e, INT16_MAX);
harq_out_length = (uint16_t) fcw->hcin_size0;
-   harq_out_length = RTE_MIN(RTE_MAX(harq_out_length, l), ncb_p);
-   harq_out_length = (harq_out_length + 0x3F) & 0xFFC0;
+   harq_out_length = RTE_MAX(harq_out_length, l);
+   /* Stronger alignment 

Re: [PATCH v3 08/30] baseband/acc100: allocate ring/queue mem when NULL

[Maxime Coquelin]

On 10/12/22 04:53, Hernan Vargas wrote:

Allocate info ring, tail pointers and HARQ layout memory for a device
only if it hasn't already been allocated.

Fixes: 06531464151 ("baseband/acc100: support interrupt")
Cc: sta...@dpdk.org

Signed-off-by: Hernan Vargas 
---
  drivers/baseband/acc/rte_acc100_pmd.c | 9 ++---
  1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c 
b/drivers/baseband/acc/rte_acc100_pmd.c
index c1446b3721..c0184b8174 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -406,7 +406,8 @@ allocate_info_ring(struct rte_bbdev *dev)
else
reg_addr = &vf_reg_addr;
/* Allocate InfoRing */
-   d->info_ring = rte_zmalloc_socket("Info Ring",
+   if (d->info_ring == NULL)


Isn't there a check already in the function entry that returns early if
already allocated?


+   d->info_ring = rte_zmalloc_socket("Info Ring",
ACC_INFO_RING_NUM_ENTRIES *
sizeof(*d->info_ring), RTE_CACHE_LINE_SIZE,
dev->data->socket_id);
@@ -498,7 +499,8 @@ acc100_setup_queues(struct rte_bbdev *dev, uint16_t 
num_queues, int socket_id)
acc_reg_write(d, reg_addr->ring_size, value);
  
  	/* Configure tail pointer for use when SDONE enabled */

-   d->tail_ptrs = rte_zmalloc_socket(
+   if (d->tail_ptrs == NULL)
+   d->tail_ptrs = rte_zmalloc_socket(
dev->device->driver->name,
ACC100_NUM_QGRPS * ACC100_NUM_AQS * sizeof(uint32_t),
RTE_CACHE_LINE_SIZE, socket_id);


In case of tail_ptrs, sw_rings is freed, but not set to NULL.
At the function entry, we check sw_rings is non-NULL before proceeding 
to allocation.


It would be better to have a proper error path using gotos.


@@ -530,7 +532,8 @@ acc100_setup_queues(struct rte_bbdev *dev, uint16_t 
num_queues, int socket_id)
/* Continue */
}
  
-	d->harq_layout = rte_zmalloc_socket("HARQ Layout",

+   if (d->harq_layout == NULL)
+   d->harq_layout = rte_zmalloc_socket("HARQ Layout",
ACC_HARQ_LAYOUT * sizeof(*d->harq_layout),
RTE_CACHE_LINE_SIZE, dev->data->socket_id);
if (d->harq_layout == NULL) {


Same comment here.

Re: [PATCH v3 09/30] baseband/acc100: reduce input length for CRC24B

[Maxime Coquelin]

On 10/12/22 04:53, Hernan Vargas wrote:

Input length should be reduced only for CRC24B.

Fixes: 5ad5060f8f7 ("baseband/acc100: add LDPC processing functions")
Cc: sta...@dpdk.org

Signed-off-by: Hernan Vargas 
---
  drivers/baseband/acc/rte_acc100_pmd.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c 
b/drivers/baseband/acc/rte_acc100_pmd.c
index c0184b8174..c0e6d0ef23 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -1427,8 +1427,7 @@ acc100_dma_desc_le_fill(struct rte_bbdev_enc_op *op,
  
  	K = (enc->basegraph == 1 ? 22 : 10) * enc->z_c;

in_length_in_bits = K - enc->n_filler;
-   if ((enc->op_flags & RTE_BBDEV_LDPC_CRC_24A_ATTACH) ||
-   (enc->op_flags & RTE_BBDEV_LDPC_CRC_24B_ATTACH))
+   if (enc->op_flags & RTE_BBDEV_LDPC_CRC_24B_ATTACH)
in_length_in_bits -= 24;
in_length_in_bytes = in_length_in_bits >> 3;
  


Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [PATCH v3 10/30] baseband/acc100: fix clearing PF IR outside handler

[Maxime Coquelin]

On 10/12/22 04:53, Hernan Vargas wrote:

Clearing of PF info ring outside of handler may cause interrupt to be
missed.
A condition in the ACC100 PMD implementation may cause an interrupt
functional handler call to be missed due to related bit being cleared
when checking PF info ring status.

Fixes: 06531464151 ("baseband/acc100: support interrupt")
Cc: sta...@dpdk.org

Signed-off-by: Hernan Vargas 


No new line here.


Reviewed-by: Maxime Coquelin 
---
  drivers/baseband/acc/rte_acc100_pmd.c | 7 ---
  1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c 
b/drivers/baseband/acc/rte_acc100_pmd.c
index c0e6d0ef23..e561d33150 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -261,11 +261,12 @@ acc100_check_ir(struct acc_device *acc100_dev)
while (ring_data->valid) {
if ((ring_data->int_nb < ACC100_PF_INT_DMA_DL_DESC_IRQ) || (
ring_data->int_nb >
-   ACC100_PF_INT_DMA_DL5G_DESC_IRQ))
+   ACC100_PF_INT_DMA_DL5G_DESC_IRQ)) {
rte_bbdev_log(WARNING, "InfoRing: ITR:%d Info:0x%x",
ring_data->int_nb, ring_data->detailed_info);
-   /* Initialize Info Ring entry and move forward */
-   ring_data->val = 0;
+   /* Initialize Info Ring entry and move forward */
+   ring_data->val = 0;
+   }
info_ring_head++;
ring_data = acc100_dev->info_ring +
(info_ring_head & ACC_INFO_RING_MASK);

Re: [PATCH v3 11/30] baseband/acc100: set device min alignment to 1

[Maxime Coquelin]

On 10/12/22 04:53, Hernan Vargas wrote:

Historical mistakes, there should be no 64B alignment requirement for
the buffer being processed. Any 1B alignment is sufficient.

Fixes: 9200ffa5cd5 ("baseband/acc100: add info get function")
Cc: sta...@dpdk.org

Signed-off-by: Hernan Vargas 


No new line


Reviewed-by: Maxime Coquelin 
---
  drivers/baseband/acc/rte_acc100_pmd.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c 
b/drivers/baseband/acc/rte_acc100_pmd.c
index e561d33150..8e5cd76693 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -930,7 +930,7 @@ acc100_dev_info_get(struct rte_bbdev *dev,
d->acc_conf.q_ul_4g.num_qgroups - 1;
dev_info->default_queue_conf = default_queue_conf;
dev_info->cpu_flag_reqs = NULL;
-   dev_info->min_alignment = 64;
+   dev_info->min_alignment = 1;
dev_info->capabilities = bbdev_capabilities;
  #ifdef ACC100_EXT_MEM
dev_info->harq_buffer_size = d->ddr_size;