[PATCH v3 00/30] baseband/acc100: changes for 22.11

[PATCH v3 00/30] baseband/acc100: changes for 22.11

[Hernan Vargas]

Upstreaming ACC100 changes for 22.11.
This patch series is dependant on series:
https://patches.dpdk.org/project/dpdk/list/?series=25041

Hernan Vargas (30):
  baseband/acc100: fix ring availability calculation
  baseband/acc100: add function to check AQ availability
  baseband/acc100: memory leak fix
  baseband/acc100: add LDPC encoder padding function
  baseband/acc100: check turbo dec/enc input
  baseband/acc100: check for unlikely operation vals
  baseband/acc100: enforce additional check on FCW
  baseband/acc100: allocate ring/queue mem when NULL
  baseband/acc100: reduce input length for CRC24B
  baseband/acc100: fix clearing PF IR outside handler
  baseband/acc100: set device min alignment to 1
  baseband/acc100: add protection for NULL HARQ input
  baseband/acc100: reset pointer after rte_free
  baseband/acc100: fix debug print for LDPC FCW
  baseband/acc100: add enqueue status
  baseband/acc100: add scatter-gather support
  baseband/acc100: add HARQ index helper function
  baseband/acc100: enable input validation by default
  baseband/acc100: added LDPC transport block support
  baseband/acc100: update validate LDPC enc/dec
  baseband/acc100: implement configurable queue depth
  baseband/acc100: add queue stop operation
  baseband/acc100: update uplink CB input length
  baseband/acc100: rename ldpc encode function arg
  baseband/acc100: update log messages
  baseband/acc100: store FCW from first CB descriptor
  baseband/acc100: update device info
  baseband/acc100: add ring companion address
  baseband/acc100: add workaround for deRM corner cases
  baseband/acc100: configure PMON control registers

 drivers/baseband/acc/acc100_pmd.h     |    5 +
 drivers/baseband/acc/acc_common.h     |   10 +
 drivers/baseband/acc/meson.build      |   21 +
 drivers/baseband/acc/rte_acc100_pmd.c | 1197 ++++++++++++++++++++-----
 4 files changed, 1010 insertions(+), 223 deletions(-)

-- 
2.37.1

[PATCH v3 01/30] baseband/acc100: fix ring availability calculation

[Hernan Vargas]

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: stable@dpdk.org

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
---
 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);
 	uint16_t i;
 	union acc_dma_desc *desc;
 	int ret;
@@ -2915,7 +2915,7 @@ acc100_enqueue_ldpc_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);
 	uint16_t i = 0;
 	union acc_dma_desc *desc;
 	int ret, desc_idx = 0;
@@ -2966,7 +2966,7 @@ acc100_enqueue_enc_tb(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);
 	uint16_t i, enqueued_cbs = 0;
 	uint8_t cbs_in_tb;
 	int ret;
@@ -3028,7 +3028,7 @@ acc100_enqueue_dec_cb(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 avail = q->sw_ring_depth + q->sw_ring_tail - q->sw_ring_head;
+	int32_t avail = acc_ring_avail_enq(q);
 	uint16_t i;
 	union acc_dma_desc *desc;
 	int ret;
@@ -3068,7 +3068,7 @@ acc100_enqueue_ldpc_dec_tb(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 avail = q->sw_ring_depth + q->sw_ring_tail - q->sw_ring_head;
+	int32_t avail = acc_ring_avail_enq(q);
 	uint16_t i, enqueued_cbs = 0;
 	uint8_t cbs_in_tb;
 	int ret;
@@ -3101,7 +3101,7 @@ acc100_enqueue_ldpc_dec_cb(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 avail = q->sw_ring_depth + q->sw_ring_tail - q->sw_ring_head;
+	int32_t avail = acc_ring_avail_enq(q);
 	uint16_t i;
 	union acc_dma_desc *desc;
 	int ret;
@@ -3151,7 +3151,7 @@ acc100_enqueue_dec_tb(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 avail = q->sw_ring_depth + q->sw_ring_tail - q->sw_ring_head;
+	int32_t avail = acc_ring_avail_enq(q);
 	uint16_t i, enqueued_cbs = 0;
 	uint8_t cbs_in_tb;
 	int ret;
@@ -3526,12 +3526,13 @@ acc100_dequeue_enc(struct rte_bbdev_queue_data *q_data,
 {
 	struct acc_queue *q = q_data->queue_private;
 	uint16_t dequeue_num;
-	uint32_t avail = q->sw_ring_head - q->sw_ring_tail;
+	uint32_t avail = acc_ring_avail_deq(q);
 	uint32_t aq_dequeued = 0;
 	uint16_t i, dequeued_cbs = 0;
 	struct rte_bbdev_enc_op *op;
 	int ret;
-
+	if (avail == 0)
+		return 0;
 #ifdef RTE_LIBRTE_BBDEV_DEBUG
 	if (unlikely(ops == NULL || q == NULL)) {
 		rte_bbdev_log_debug("Unexpected undefined pointer");
@@ -3571,7 +3572,7 @@ acc100_dequeue_ldpc_enc(struct rte_bbdev_queue_data *q_data,
 		struct rte_bbdev_enc_op **ops, uint16_t num)
 {
 	struct acc_queue *q = q_data->queue_private;
-	uint32_t avail = q->sw_ring_head - q->sw_ring_tail;
+	uint32_t avail = acc_ring_avail_deq(q);
 	uint32_t aq_dequeued = 0;
 	uint16_t dequeue_num, i, dequeued_cbs = 0, dequeued_descs = 0;
 	int ret;
@@ -3611,7 +3612,7 @@ acc100_dequeue_dec(struct rte_bbdev_queue_data *q_data,
 {
 	struct acc_queue *q = q_data->queue_private;
 	uint16_t dequeue_num;
-	uint32_t avail = q->sw_ring_head - q->sw_ring_tail;
+	uint32_t avail = acc_ring_avail_deq(q);
 	uint32_t aq_dequeued = 0;
 	uint16_t i;
 	uint16_t dequeued_cbs = 0;
@@ -3656,7 +3657,7 @@ acc100_dequeue_ldpc_dec(struct rte_bbdev_queue_data *q_data,
 {
 	struct acc_queue *q = q_data->queue_private;
 	uint16_t dequeue_num;
-	uint32_t avail = q->sw_ring_head - q->sw_ring_tail;
+	uint32_t avail = acc_ring_avail_deq(q);
 	uint32_t aq_dequeued = 0;
 	uint16_t i;
 	uint16_t dequeued_cbs = 0;
-- 
2.37.1

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: stable@dpdk.org
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   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 <maxime.coquelin@redhat.com>

Thanks,
Maxime

[PATCH v3 02/30] baseband/acc100: add function to check AQ availability

[Hernan Vargas]

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: stable@dpdk.org

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
---
 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)
+		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);
+	if (unlikely((aq_avail <= 0) || (num == 0)))
 		return 0;
 	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);
+	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);
+	if (unlikely((aq_avail <= 0) || (num == 0)))
 		return 0;
 	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);
+	if (unlikely((aq_avail <= 0) || (num == 0)))
 		return 0;
 
 	if (ops[0]->ldpc_dec.code_block_mode == RTE_BBDEV_TRANSPORT_BLOCK)
-- 
2.37.1

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: stable@dpdk.org
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   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)

[PATCH v3 03/30] baseband/acc100: memory leak fix

[Hernan Vargas]

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: stable@dpdk.org

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 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));
-- 
2.37.1

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: stable@dpdk.org
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>

No new line here.

> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>   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));

[PATCH v3 04/30] baseband/acc100: add LDPC encoder padding function

[Hernan Vargas]

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: stable@dpdk.org

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
---
 drivers/baseband/acc/rte_acc100_pmd.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
index 436af977e4..f636d4fa0f 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -1016,7 +1016,6 @@ acc100_fcw_td_fill(const struct rte_bbdev_dec_op *op, struct acc_fcw_td *fcw)
 			RTE_BBDEV_TURBO_HALF_ITERATION_EVEN);
 }
 
-#ifdef RTE_LIBRTE_BBDEV_DEBUG
 
 static inline bool
 is_acc100(struct acc_queue *q)
@@ -1029,7 +1028,6 @@ validate_op_required(struct acc_queue *q)
 {
 	return is_acc100(q);
 }
-#endif
 
 /* Fill in a frame control word for LDPC decoding. */
 static inline void
@@ -1354,12 +1352,28 @@ acc100_dma_fill_blk_type_in(struct acc_dma_req_desc *desc,
 	return next_triplet;
 }
 
+/* May need to pad LDPC Encoder input to avoid small beat for ACC100. */
+static inline uint16_t
+pad_le_in(uint16_t blen, struct acc_queue *q)
+{
+	uint16_t last_beat;
+
+	if (!is_acc100(q))
+		return blen;
+
+	last_beat = blen % 64;
+	if ((last_beat > 0) && (last_beat <= 8))
+		blen += 8;
+
+	return blen;
+}
+
 static inline int
 acc100_dma_desc_le_fill(struct rte_bbdev_enc_op *op,
 		struct acc_dma_req_desc *desc, struct rte_mbuf **input,
 		struct rte_mbuf *output, uint32_t *in_offset,
 		uint32_t *out_offset, uint32_t *out_length,
-		uint32_t *mbuf_total_left, uint32_t *seg_total_left)
+		uint32_t *mbuf_total_left, uint32_t *seg_total_left, struct acc_queue *q)
 {
 	int next_triplet = 1; /* FCW already done */
 	uint16_t K, in_length_in_bits, in_length_in_bytes;
@@ -1383,8 +1397,7 @@ acc100_dma_desc_le_fill(struct rte_bbdev_enc_op *op,
 	}
 
 	next_triplet = acc100_dma_fill_blk_type_in(desc, input, in_offset,
-			in_length_in_bytes,
-			seg_total_left, next_triplet);
+			pad_le_in(in_length_in_bytes, q), seg_total_left, next_triplet);
 	if (unlikely(next_triplet < 0)) {
 		rte_bbdev_log(ERR,
 				"Mismatch between data to process and mbuf data length in bbdev_op: %p",
@@ -2038,7 +2051,7 @@ enqueue_ldpc_enc_n_op_cb(struct acc_queue *q, struct rte_bbdev_enc_op **ops,
 	acc_header_init(&desc->req);
 	desc->req.numCBs = num;
 
-	in_length_in_bytes = ops[0]->ldpc_enc.input.data->data_len;
+	in_length_in_bytes = pad_le_in(ops[0]->ldpc_enc.input.data->data_len, q);
 	out_length = (enc->cb_params.e + 7) >> 3;
 	desc->req.m2dlen = 1 + num;
 	desc->req.d2mlen = num;
@@ -2107,7 +2120,7 @@ enqueue_ldpc_enc_one_op_cb(struct acc_queue *q, struct rte_bbdev_enc_op *op,
 
 	ret = acc100_dma_desc_le_fill(op, &desc->req, &input, output,
 			&in_offset, &out_offset, &out_length, &mbuf_total_left,
-			&seg_total_left);
+			&seg_total_left, q);
 
 	if (unlikely(ret < 0))
 		return ret;
-- 
2.37.1

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: stable@dpdk.org
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   drivers/baseband/acc/rte_acc100_pmd.c | 27 ++++++++++++++++++++-------
>   1 file changed, 20 insertions(+), 7 deletions(-)
> 

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime

[PATCH v3 05/30] baseband/acc100: check turbo dec/enc input

[Hernan Vargas]

Add NULL check for the turbo decoder and encoder input length.

Fixes: 3bfc5f60403 ("baseband/acc100: add debug function to validate
input")
Cc: stable@dpdk.org

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 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) {
+		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) {
 			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) {
+		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)) {
 			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",
-- 
2.37.1

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: stable@dpdk.org
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> 
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>   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",

[PATCH v3 06/30] baseband/acc100: check for unlikely operation vals

[Hernan Vargas]

Add unlikely checks for NULL operation values.

Fixes: f404dfe35cc ("baseband/acc100: support 4G processing")
Cc: stable@dpdk.org

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 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)) {
+			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);
-- 
2.37.1

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: stable@dpdk.org
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> 
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>   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);

[PATCH v3 07/30] baseband/acc100: enforce additional check on FCW

[Hernan Vargas]

Enforce additional check on Frame Control Word validity and add stronger
alignment for decompression mode.

Fixes: 5ad5060f8f7 ("baseband/acc100: add LDPC processing functions")
Cc: stable@dpdk.org

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
---
 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_SPA                0
 #define ACC_ALGO_MSA                1
+#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) {
 			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 +1111,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 when in compression mode */
+		if (fcw->hcout_comp_mode > 0)
+			harq_out_length = RTE_ALIGN_CEIL(harq_out_length, ACC100_HARQ_ALIGN_COMP);
+		/* Cannot exceed the pruned Ncb circular buffer */
+		harq_out_length = RTE_MIN(harq_out_length, ncb_p);
+		/* Alignment on next 64B */
+		harq_out_length = RTE_ALIGN_CEIL(harq_out_length, ACC_HARQ_ALIGN_64B);
+		/* Stronger alignment when in compression mode enforced again */
+		if (fcw->hcout_comp_mode > 0)
+			harq_out_length = RTE_ALIGN_FLOOR(harq_out_length, ACC100_HARQ_ALIGN_COMP);
 		if ((k0_p > fcw->hcin_size0 + ACC_HARQ_OFFSET_THRESHOLD) &&
 				harq_prun) {
 			fcw->hcout_size0 = (uint16_t) fcw->hcin_size0;
@@ -1146,6 +1176,13 @@ acc100_fcw_ld_fill(struct rte_bbdev_dec_op *op, struct acc_fcw_ld *fcw,
 			fcw->hcout_size1 = 0;
 			fcw->hcout_offset = 0;
 		}
+		if (fcw->hcout_size0 == 0) {
+			rte_bbdev_log(ERR, " Invalid FCW : HCout %d",
+				fcw->hcout_size0);
+			op->ldpc_dec.op_flags ^= RTE_BBDEV_LDPC_HQ_COMBINE_OUT_ENABLE;
+			fcw->hcout_en = 0;
+		}
+
 		harq_layout[harq_index].offset = fcw->hcout_offset;
 		harq_layout[harq_index].size0 = fcw->hcout_size0;
 	} else {
@@ -1186,6 +1223,10 @@ acc101_fcw_ld_fill(struct rte_bbdev_dec_op *op, struct acc_fcw_ld *fcw,
 		/* Disable HARQ input in that case to carry forward */
 		op->ldpc_dec.op_flags ^= RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE;
 	}
+	if (unlikely(fcw->rm_e == 0)) {
+		rte_bbdev_log(WARNING, "Null E input provided");
+		fcw->rm_e = 2;
+	}
 
 	fcw->hcin_en = check_bit(op->ldpc_dec.op_flags,
 			RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE);
-- 
2.37.1

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: stable@dpdk.org
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   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_SPA                0
>   #define ACC_ALGO_MSA                1
> +#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 +1111,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 when in compression mode */
> +		if (fcw->hcout_comp_mode > 0)
> +			harq_out_length = RTE_ALIGN_CEIL(harq_out_length, ACC100_HARQ_ALIGN_COMP);
> +		/* Cannot exceed the pruned Ncb circular buffer */
> +		harq_out_length = RTE_MIN(harq_out_length, ncb_p);
> +		/* Alignment on next 64B */
> +		harq_out_length = RTE_ALIGN_CEIL(harq_out_length, ACC_HARQ_ALIGN_64B);
> +		/* Stronger alignment when in compression mode enforced again */
> +		if (fcw->hcout_comp_mode > 0)
> +			harq_out_length = RTE_ALIGN_FLOOR(harq_out_length, ACC100_HARQ_ALIGN_COMP);

Same here, this is very packed.

>   		if ((k0_p > fcw->hcin_size0 + ACC_HARQ_OFFSET_THRESHOLD) &&
>   				harq_prun) {
>   			fcw->hcout_size0 = (uint16_t) fcw->hcin_size0;
> @@ -1146,6 +1176,13 @@ acc100_fcw_ld_fill(struct rte_bbdev_dec_op *op, struct acc_fcw_ld *fcw,
>   			fcw->hcout_size1 = 0;
>   			fcw->hcout_offset = 0;
>   		}
> +		if (fcw->hcout_size0 == 0) {
> +			rte_bbdev_log(ERR, " Invalid FCW : HCout %d",
> +				fcw->hcout_size0);
> +			op->ldpc_dec.op_flags ^= RTE_BBDEV_LDPC_HQ_COMBINE_OUT_ENABLE;
> +			fcw->hcout_en = 0;
> +		}
> +
>   		harq_layout[harq_index].offset = fcw->hcout_offset;
>   		harq_layout[harq_index].size0 = fcw->hcout_size0;
>   	} else {
> @@ -1186,6 +1223,10 @@ acc101_fcw_ld_fill(struct rte_bbdev_dec_op *op, struct acc_fcw_ld *fcw,
>   		/* Disable HARQ input in that case to carry forward */
>   		op->ldpc_dec.op_flags ^= RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE;
>   	}
> +	if (unlikely(fcw->rm_e == 0)) {
> +		rte_bbdev_log(WARNING, "Null E input provided");
> +		fcw->rm_e = 2;
> +	}
>   
>   	fcw->hcin_en = check_bit(op->ldpc_dec.op_flags,
>   			RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE);

[PATCH v3 08/30] baseband/acc100: allocate ring/queue mem when NULL

[Hernan Vargas]

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: stable@dpdk.org

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
---
 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)
+		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);
@@ -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) {
-- 
2.37.1

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: stable@dpdk.org
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   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.

[PATCH v3 09/30] baseband/acc100: reduce input length for CRC24B

[Hernan Vargas]

Input length should be reduced only for CRC24B.

Fixes: 5ad5060f8f7 ("baseband/acc100: add LDPC processing functions")
Cc: stable@dpdk.org

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
---
 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;
 
-- 
2.37.1

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: stable@dpdk.org
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   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 <maxime.coquelin@redhat.com>

Thanks,
Maxime

[PATCH v3 10/30] baseband/acc100: fix clearing PF IR outside handler

[Hernan Vargas]

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: stable@dpdk.org

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 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);
-- 
2.37.1

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: stable@dpdk.org
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>

No new line here.

> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>   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);

[PATCH v3 11/30] baseband/acc100: set device min alignment to 1

[Hernan Vargas]

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: stable@dpdk.org

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 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;
-- 
2.37.1

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: stable@dpdk.org
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>

No new line

> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>   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;

[PATCH v3 12/30] baseband/acc100: add protection for NULL HARQ input

[Hernan Vargas]

It is possible to cause an invalid HW operation in case the user
provides the BBDEV API and HARQ operation with input enabled and zero
input. Adding protection for that case.

Fixes: 5ad5060f8f7 ("baseband/acc100: add LDPC processing functions")
Cc: stable@dpdk.org

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
---
 drivers/baseband/acc/rte_acc100_pmd.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
index 8e5cd76693..c8dffda8a4 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -1059,6 +1059,14 @@ acc100_fcw_ld_fill(struct rte_bbdev_dec_op *op, struct acc_fcw_ld *fcw,
 						op->ldpc_dec.tb_params.ea :
 						op->ldpc_dec.tb_params.eb;
 
+	if (unlikely(check_bit(op->ldpc_dec.op_flags,
+			RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE) &&
+			(op->ldpc_dec.harq_combined_input.length == 0))) {
+		rte_bbdev_log(WARNING, "Null HARQ input size provided");
+		/* Disable HARQ input in that case to carry forward. */
+		op->ldpc_dec.op_flags ^= RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE;
+	}
+
 	fcw->hcin_en = check_bit(op->ldpc_dec.op_flags,
 			RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE);
 	fcw->hcout_en = check_bit(op->ldpc_dec.op_flags,
-- 
2.37.1

Re: [PATCH v3 12/30] baseband/acc100: add protection for NULL HARQ input

[Maxime Coquelin]

On 10/12/22 04:53, Hernan Vargas wrote:
> It is possible to cause an invalid HW operation in case the user
> provides the BBDEV API and HARQ operation with input enabled and zero
> input. Adding protection for that case.
> 
> Fixes: 5ad5060f8f7 ("baseband/acc100: add LDPC processing functions")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   drivers/baseband/acc/rte_acc100_pmd.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
> index 8e5cd76693..c8dffda8a4 100644
> --- a/drivers/baseband/acc/rte_acc100_pmd.c
> +++ b/drivers/baseband/acc/rte_acc100_pmd.c
> @@ -1059,6 +1059,14 @@ acc100_fcw_ld_fill(struct rte_bbdev_dec_op *op, struct acc_fcw_ld *fcw,
>   						op->ldpc_dec.tb_params.ea :
>   						op->ldpc_dec.tb_params.eb;
>   
> +	if (unlikely(check_bit(op->ldpc_dec.op_flags,
> +			RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE) &&
> +			(op->ldpc_dec.harq_combined_input.length == 0))) {
> +		rte_bbdev_log(WARNING, "Null HARQ input size provided");
> +		/* Disable HARQ input in that case to carry forward. */
> +		op->ldpc_dec.op_flags ^= RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE;
> +	}
> +
>   	fcw->hcin_en = check_bit(op->ldpc_dec.op_flags,
>   			RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE);
>   	fcw->hcout_en = check_bit(op->ldpc_dec.op_flags,

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime

[PATCH v3 13/30] baseband/acc100: reset pointer after rte_free

[Hernan Vargas]

Set local pointer to NULL after rte_free.
This needs to be set explicitly since logic may check for null pointers.

Fixes: 060e7672930 ("baseband/acc100: add queue configuration")
Cc: stable@dpdk.org

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/baseband/acc/rte_acc100_pmd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
index c8dffda8a4..3914aa7814 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -610,6 +610,9 @@ acc100_dev_close(struct rte_bbdev *dev)
 		rte_free(d->info_ring);
 		rte_free(d->sw_rings_base);
 		d->sw_rings_base = NULL;
+		d->tail_ptrs = NULL;
+		d->info_ring = NULL;
+		d->harq_layout = NULL;
 	}
 	/* Ensure all in flight HW transactions are completed */
 	usleep(ACC_LONG_WAIT);
-- 
2.37.1

Re: [PATCH v3 13/30] baseband/acc100: reset pointer after rte_free

[Maxime Coquelin]

On 10/12/22 04:53, Hernan Vargas wrote:
> Set local pointer to NULL after rte_free.
> This needs to be set explicitly since logic may check for null pointers.
> 
> Fixes: 060e7672930 ("baseband/acc100: add queue configuration")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>

No new line

> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>   drivers/baseband/acc/rte_acc100_pmd.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
> index c8dffda8a4..3914aa7814 100644
> --- a/drivers/baseband/acc/rte_acc100_pmd.c
> +++ b/drivers/baseband/acc/rte_acc100_pmd.c
> @@ -610,6 +610,9 @@ acc100_dev_close(struct rte_bbdev *dev)
>   		rte_free(d->info_ring);
>   		rte_free(d->sw_rings_base);
>   		d->sw_rings_base = NULL;
> +		d->tail_ptrs = NULL;
> +		d->info_ring = NULL;
> +		d->harq_layout = NULL;
>   	}
>   	/* Ensure all in flight HW transactions are completed */
>   	usleep(ACC_LONG_WAIT);

[PATCH v3 14/30] baseband/acc100: fix debug print for LDPC FCW

[Hernan Vargas]

Print full size of FCW LDPC structure on debug messages.
This is just a cosmetic fix, no need to fix on previous code base.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 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 3914aa7814..0ceea477e1 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -2755,7 +2755,7 @@ enqueue_ldpc_dec_one_op_cb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
 
 #ifdef RTE_LIBRTE_BBDEV_DEBUG
 	rte_memdump(stderr, "FCW", &desc->req.fcw_ld,
-			sizeof(desc->req.fcw_ld) - 8);
+			sizeof(desc->req.fcw_ld));
 	rte_memdump(stderr, "Req Desc.", desc, sizeof(*desc));
 #endif
 
-- 
2.37.1

Re: [PATCH v3 14/30] baseband/acc100: fix debug print for LDPC FCW

[Maxime Coquelin]

On 10/12/22 04:53, Hernan Vargas wrote:
> Print full size of FCW LDPC structure on debug messages.
> This is just a cosmetic fix, no need to fix on previous code base.
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>

No new line

> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>   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 3914aa7814..0ceea477e1 100644
> --- a/drivers/baseband/acc/rte_acc100_pmd.c
> +++ b/drivers/baseband/acc/rte_acc100_pmd.c
> @@ -2755,7 +2755,7 @@ enqueue_ldpc_dec_one_op_cb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
>   
>   #ifdef RTE_LIBRTE_BBDEV_DEBUG
>   	rte_memdump(stderr, "FCW", &desc->req.fcw_ld,
> -			sizeof(desc->req.fcw_ld) - 8);
> +			sizeof(desc->req.fcw_ld));
>   	rte_memdump(stderr, "Req Desc.", desc, sizeof(*desc));
>   #endif
>   

[PATCH v3 15/30] baseband/acc100: add enqueue status

[Hernan Vargas]

Add enqueue status as part of rte_bbdev_queue_data.
This is a new feature to update queue status and indicate the reason why
a previous enqueue may or may not have consumed all requested operations.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/baseband/acc/rte_acc100_pmd.c | 56 ++++++++++++++++++++-------
 1 file changed, 42 insertions(+), 14 deletions(-)

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
index 0ceea477e1..d2c69866a5 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -2968,13 +2968,17 @@ acc100_enqueue_enc_cb(struct rte_bbdev_queue_data *q_data,
 
 	for (i = 0; i < num; ++i) {
 		/* Check if there are available space for further processing */
-		if (unlikely(avail - 1 < 0))
+		if (unlikely(avail - 1 < 0)) {
+			acc_enqueue_ring_full(q_data);
 			break;
+		}
 		avail -= 1;
 
 		ret = enqueue_enc_one_op_cb(q, ops[i], i);
-		if (ret < 0)
+		if (ret < 0) {
+			acc_enqueue_invalid(q_data);
 			break;
+		}
 	}
 
 	if (unlikely(i == 0))
@@ -3007,20 +3011,26 @@ acc100_enqueue_ldpc_enc_cb(struct rte_bbdev_queue_data *q_data,
 	int16_t enq, left = num;
 
 	while (left > 0) {
-		if (unlikely(avail < 1))
+		if (unlikely(avail < 1)) {
+			acc_enqueue_ring_full(q_data);
 			break;
+		}
 		avail--;
 		enq = RTE_MIN(left, ACC_MUX_5GDL_DESC);
 		if (check_mux(&ops[i], enq)) {
 			ret = enqueue_ldpc_enc_n_op_cb(q, &ops[i],
 					desc_idx, enq);
-			if (ret < 0)
+			if (ret < 0) {
+				acc_enqueue_invalid(q_data);
 				break;
+			}
 			i += enq;
 		} else {
 			ret = enqueue_ldpc_enc_one_op_cb(q, ops[i], desc_idx);
-			if (ret < 0)
+			if (ret < 0) {
+				acc_enqueue_invalid(q_data);
 				break;
+			}
 			i++;
 		}
 		desc_idx++;
@@ -3059,13 +3069,17 @@ acc100_enqueue_enc_tb(struct rte_bbdev_queue_data *q_data,
 	for (i = 0; i < num; ++i) {
 		cbs_in_tb = get_num_cbs_in_tb_enc(&ops[i]->turbo_enc);
 		/* Check if there are available space for further processing */
-		if (unlikely(avail - cbs_in_tb < 0))
+		if (unlikely(avail - cbs_in_tb < 0)) {
+			acc_enqueue_ring_full(q_data);
 			break;
+		}
 		avail -= cbs_in_tb;
 
 		ret = enqueue_enc_one_op_tb(q, ops[i], enqueued_cbs, cbs_in_tb);
-		if (ret < 0)
+		if (ret < 0) {
+			acc_enqueue_invalid(q_data);
 			break;
+		}
 		enqueued_cbs += ret;
 	}
 	if (unlikely(enqueued_cbs == 0))
@@ -3136,13 +3150,17 @@ acc100_enqueue_dec_cb(struct rte_bbdev_queue_data *q_data,
 
 	for (i = 0; i < num; ++i) {
 		/* Check if there are available space for further processing */
-		if (unlikely(avail - 1 < 0))
+		if (unlikely(avail - 1 < 0)) {
+			acc_enqueue_ring_full(q_data);
 			break;
+		}
 		avail -= 1;
 
 		ret = enqueue_dec_one_op_cb(q, ops[i], i);
-		if (ret < 0)
+		if (ret < 0) {
+			acc_enqueue_invalid(q_data);
 			break;
+		}
 	}
 
 	if (unlikely(i == 0))
@@ -3183,8 +3201,10 @@ acc100_enqueue_ldpc_dec_tb(struct rte_bbdev_queue_data *q_data,
 
 		ret = enqueue_ldpc_dec_one_op_tb(q, ops[i],
 				enqueued_cbs, cbs_in_tb);
-		if (ret < 0)
+		if (ret < 0) {
+			acc_enqueue_invalid(q_data);
 			break;
+		}
 		enqueued_cbs += ret;
 	}
 	if (unlikely(enqueued_cbs == 0))
@@ -3211,8 +3231,10 @@ acc100_enqueue_ldpc_dec_cb(struct rte_bbdev_queue_data *q_data,
 	bool same_op = false;
 	for (i = 0; i < num; ++i) {
 		/* Check if there are available space for further processing */
-		if (unlikely(avail < 1))
+		if (unlikely(avail < 1)) {
+			acc_enqueue_ring_full(q_data);
 			break;
+		}
 		avail -= 1;
 
 		if (i > 0)
@@ -3225,8 +3247,10 @@ acc100_enqueue_ldpc_dec_cb(struct rte_bbdev_queue_data *q_data,
 			ops[i]->ldpc_dec.n_filler, ops[i]->ldpc_dec.cb_params.e,
 			same_op);
 		ret = enqueue_ldpc_dec_one_op_cb(q, ops[i], i, same_op);
-		if (ret < 0)
+		if (ret < 0) {
+			acc_enqueue_invalid(q_data);
 			break;
+		}
 	}
 
 	if (unlikely(i == 0))
@@ -3262,13 +3286,17 @@ acc100_enqueue_dec_tb(struct rte_bbdev_queue_data *q_data,
 	for (i = 0; i < num; ++i) {
 		cbs_in_tb = get_num_cbs_in_tb_dec(&ops[i]->turbo_dec);
 		/* Check if there are available space for further processing */
-		if (unlikely(avail - cbs_in_tb < 0))
+		if (unlikely(avail - cbs_in_tb < 0)) {
+			acc_enqueue_ring_full(q_data);
 			break;
+		}
 		avail -= cbs_in_tb;
 
 		ret = enqueue_dec_one_op_tb(q, ops[i], enqueued_cbs, cbs_in_tb);
-		if (ret < 0)
+		if (ret < 0) {
+			acc_enqueue_invalid(q_data);
 			break;
+		}
 		enqueued_cbs += ret;
 	}
 
-- 
2.37.1

Re: [PATCH v3 15/30] baseband/acc100: add enqueue status

[Maxime Coquelin]

On 10/12/22 04:53, Hernan Vargas wrote:
> Add enqueue status as part of rte_bbdev_queue_data.
> This is a new feature to update queue status and indicate the reason why
> a previous enqueue may or may not have consumed all requested operations.
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>

No new line

> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>   drivers/baseband/acc/rte_acc100_pmd.c | 56 ++++++++++++++++++++-------
>   1 file changed, 42 insertions(+), 14 deletions(-)
> 

[PATCH v3 16/30] baseband/acc100: add scatter-gather support

[Hernan Vargas]

Add flag to support scatter-gather for the mbuf

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/baseband/acc/rte_acc100_pmd.c | 37 ++++++++++++++++++---------
 1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
index d2c69866a5..9c1218e1f2 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -1347,6 +1347,8 @@ acc101_fcw_ld_fill(struct rte_bbdev_dec_op *op, struct acc_fcw_ld *fcw,
  *   Store information about device capabilities
  * @param next_triplet
  *   Index for ACC100 DMA Descriptor triplet
+ * @param scattergather
+ *   Flag to support scatter-gather for the mbuf
  *
  * @return
  *   Returns index of next triplet on success, other value if lengths of
@@ -1356,12 +1358,16 @@ acc101_fcw_ld_fill(struct rte_bbdev_dec_op *op, struct acc_fcw_ld *fcw,
 static inline int
 acc100_dma_fill_blk_type_in(struct acc_dma_req_desc *desc,
 		struct rte_mbuf **input, uint32_t *offset, uint32_t cb_len,
-		uint32_t *seg_total_left, int next_triplet)
+		uint32_t *seg_total_left, int next_triplet,
+		bool scattergather)
 {
 	uint32_t part_len;
 	struct rte_mbuf *m = *input;
 
-	part_len = (*seg_total_left < cb_len) ? *seg_total_left : cb_len;
+	if (scattergather)
+		part_len = (*seg_total_left < cb_len) ? *seg_total_left : cb_len;
+	else
+		part_len = cb_len;
 	cb_len -= part_len;
 	*seg_total_left -= part_len;
 
@@ -1452,7 +1458,7 @@ acc100_dma_desc_le_fill(struct rte_bbdev_enc_op *op,
 	}
 
 	next_triplet = acc100_dma_fill_blk_type_in(desc, input, in_offset,
-			pad_le_in(in_length_in_bytes, q), seg_total_left, next_triplet);
+			pad_le_in(in_length_in_bytes, q), seg_total_left, next_triplet, false);
 	if (unlikely(next_triplet < 0)) {
 		rte_bbdev_log(ERR,
 				"Mismatch between data to process and mbuf data length in bbdev_op: %p",
@@ -1540,7 +1546,9 @@ acc100_dma_desc_td_fill(struct rte_bbdev_dec_op *op,
 	}
 
 	next_triplet = acc100_dma_fill_blk_type_in(desc, input, in_offset, kw,
-			seg_total_left, next_triplet);
+			seg_total_left, next_triplet,
+			check_bit(op->turbo_dec.op_flags,
+			RTE_BBDEV_TURBO_DEC_SCATTER_GATHER));
 	if (unlikely(next_triplet < 0)) {
 		rte_bbdev_log(ERR,
 				"Mismatch between data to process and mbuf data length in bbdev_op: %p",
@@ -1642,7 +1650,9 @@ acc100_dma_desc_ld_fill(struct rte_bbdev_dec_op *op,
 
 	next_triplet = acc100_dma_fill_blk_type_in(desc, input,
 			in_offset, input_length,
-			seg_total_left, next_triplet);
+			seg_total_left, next_triplet,
+			check_bit(op->ldpc_dec.op_flags,
+			RTE_BBDEV_LDPC_DEC_SCATTER_GATHER));
 
 	if (unlikely(next_triplet < 0)) {
 		rte_bbdev_log(ERR,
@@ -2725,8 +2735,9 @@ enqueue_ldpc_dec_one_op_cb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
 		fcw = &desc->req.fcw_ld;
 		q->d->fcw_ld_fill(op, fcw, harq_layout);
 
-		/* Special handling when overusing mbuf */
-		if (fcw->rm_e < ACC_MAX_E_MBUF)
+		/* Special handling when using mbuf or not */
+		if (check_bit(op->ldpc_dec.op_flags,
+			RTE_BBDEV_LDPC_DEC_SCATTER_GATHER))
 			seg_total_left = rte_pktmbuf_data_len(input)
 					- in_offset;
 		else
@@ -2802,9 +2813,10 @@ enqueue_ldpc_dec_one_op_tb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
 	r = op->ldpc_dec.tb_params.r;
 
 	while (mbuf_total_left > 0 && r < c) {
-
-		seg_total_left = rte_pktmbuf_data_len(input) - in_offset;
-
+		if (check_bit(op->ldpc_dec.op_flags, RTE_BBDEV_LDPC_DEC_SCATTER_GATHER))
+			seg_total_left = rte_pktmbuf_data_len(input) - in_offset;
+		else
+			seg_total_left = op->ldpc_dec.input.length;
 		/* Set up DMA descriptor */
 		desc = q->ring_addr + ((q->sw_ring_head + total_enqueued_cbs)
 				& q->sw_ring_wrap_mask);
@@ -2829,8 +2841,9 @@ enqueue_ldpc_dec_one_op_tb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
 				sizeof(desc->req.fcw_td) - 8);
 		rte_memdump(stderr, "Req Desc.", desc, sizeof(*desc));
 #endif
-
-		if (seg_total_left == 0) {
+		if (check_bit(op->ldpc_dec.op_flags,
+				RTE_BBDEV_LDPC_DEC_SCATTER_GATHER)
+				&& (seg_total_left == 0)) {
 			/* Go to the next mbuf */
 			input = input->next;
 			in_offset = 0;
-- 
2.37.1

Re: [PATCH v3 16/30] baseband/acc100: add scatter-gather support

[Maxime Coquelin]

On 10/12/22 04:53, Hernan Vargas wrote:
> Add flag to support scatter-gather for the mbuf
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>

No new line

> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>   drivers/baseband/acc/rte_acc100_pmd.c | 37 ++++++++++++++++++---------
>   1 file changed, 25 insertions(+), 12 deletions(-)

[PATCH v3 17/30] baseband/acc100: add HARQ index helper function

[Hernan Vargas]

Refactor code to use the HARQ index helper function and make harq_idx
uint32.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/baseband/acc/rte_acc100_pmd.c | 29 +++++++++++----------------
 1 file changed, 12 insertions(+), 17 deletions(-)

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
index 9c1218e1f2..76667d1293 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -1042,7 +1042,7 @@ acc100_fcw_ld_fill(struct rte_bbdev_dec_op *op, struct acc_fcw_ld *fcw,
 		union acc_harq_layout_data *harq_layout)
 {
 	uint16_t harq_out_length, harq_in_length, ncb_p, k0_p, parity_offset;
-	uint16_t harq_index;
+	uint32_t harq_index;
 	uint32_t l;
 	bool harq_prun = false;
 	uint32_t max_hc_in;
@@ -1090,8 +1090,7 @@ acc100_fcw_ld_fill(struct rte_bbdev_dec_op *op, struct acc_fcw_ld *fcw,
 			RTE_BBDEV_LDPC_HARQ_6BIT_COMPRESSION);
 	fcw->llr_pack_mode = check_bit(op->ldpc_dec.op_flags,
 			RTE_BBDEV_LDPC_LLR_COMPRESSION);
-	harq_index = op->ldpc_dec.harq_combined_output.offset /
-			ACC_HARQ_OFFSET;
+	harq_index = hq_index(op->ldpc_dec.harq_combined_output.offset);
 #ifdef ACC100_EXT_MEM
 	/* Limit cases when HARQ pruning is valid */
 	harq_prun = ((op->ldpc_dec.harq_combined_output.offset %
@@ -1761,20 +1760,17 @@ acc100_dma_desc_ld_update(struct rte_bbdev_dec_op *op,
 	*h_out_length = desc->data_ptrs[next_triplet].blen;
 	next_triplet++;
 
-	if (check_bit(op->ldpc_dec.op_flags,
-				RTE_BBDEV_LDPC_HQ_COMBINE_OUT_ENABLE)) {
-		desc->data_ptrs[next_triplet].address =
-				op->ldpc_dec.harq_combined_output.offset;
+	if (check_bit(op->ldpc_dec.op_flags, RTE_BBDEV_LDPC_HQ_COMBINE_OUT_ENABLE)) {
+		struct rte_bbdev_dec_op *prev_op;
+		uint32_t harq_idx, prev_harq_idx;
+		desc->data_ptrs[next_triplet].address = op->ldpc_dec.harq_combined_output.offset;
 		/* Adjust based on previous operation */
-		struct rte_bbdev_dec_op *prev_op = desc->op_addr;
+		prev_op = desc->op_addr;
 		op->ldpc_dec.harq_combined_output.length =
 				prev_op->ldpc_dec.harq_combined_output.length;
-		int16_t hq_idx = op->ldpc_dec.harq_combined_output.offset /
-				ACC_HARQ_OFFSET;
-		int16_t prev_hq_idx =
-				prev_op->ldpc_dec.harq_combined_output.offset
-				/ ACC_HARQ_OFFSET;
-		harq_layout[hq_idx].val = harq_layout[prev_hq_idx].val;
+		harq_idx = hq_index(op->ldpc_dec.harq_combined_output.offset);
+		prev_harq_idx = hq_index(prev_op->ldpc_dec.harq_combined_output.offset);
+		harq_layout[harq_idx].val = harq_layout[prev_harq_idx].val;
 #ifndef ACC100_EXT_MEM
 		struct rte_bbdev_op_data ho =
 				op->ldpc_dec.harq_combined_output;
@@ -2549,10 +2545,9 @@ harq_loopback(struct acc_queue *q, struct rte_bbdev_dec_op *op,
 	bool ddr_mem_in = check_bit(op->ldpc_dec.op_flags,
 			RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_IN_ENABLE);
 	union acc_harq_layout_data *harq_layout = q->d->harq_layout;
-	uint16_t harq_index = (ddr_mem_in ?
+	uint32_t harq_index = hq_index(ddr_mem_in ?
 			op->ldpc_dec.harq_combined_input.offset :
-			op->ldpc_dec.harq_combined_output.offset)
-			/ ACC_HARQ_OFFSET;
+			op->ldpc_dec.harq_combined_output.offset);
 
 	uint16_t desc_idx = ((q->sw_ring_head + total_enqueued_cbs)
 			& q->sw_ring_wrap_mask);
-- 
2.37.1

Re: [PATCH v3 17/30] baseband/acc100: add HARQ index helper function

[Maxime Coquelin]

On 10/12/22 04:53, Hernan Vargas wrote:
> Refactor code to use the HARQ index helper function and make harq_idx
> uint32.
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>

No new line

> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>   drivers/baseband/acc/rte_acc100_pmd.c | 29 +++++++++++----------------
>   1 file changed, 12 insertions(+), 17 deletions(-)
> 

[PATCH v3 18/30] baseband/acc100: enable input validation by default

[Hernan Vargas]

Enable validation functions by default and provide a new flag
RTE_LIBRTE_SKIP_VALIDATE if the user wants to run without
validating input to save cycles.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/baseband/acc/rte_acc100_pmd.c | 36 +++++++++++++--------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
index 76667d1293..1ccbe4b8b7 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -1784,7 +1784,7 @@ acc100_dma_desc_ld_update(struct rte_bbdev_dec_op *op,
 	desc->op_addr = op;
 }
 
-#ifdef RTE_LIBRTE_BBDEV_DEBUG
+#ifndef RTE_LIBRTE_BBDEV_SKIP_VALIDATE
 /* Validates turbo encoder parameters */
 static inline int
 validate_enc_op(struct rte_bbdev_enc_op *op, struct acc_queue *q)
@@ -2047,10 +2047,10 @@ enqueue_enc_one_op_cb(struct acc_queue *q, struct rte_bbdev_enc_op *op,
 		seg_total_left;
 	struct rte_mbuf *input, *output_head, *output;
 
-#ifdef RTE_LIBRTE_BBDEV_DEBUG
+#ifndef RTE_LIBRTE_BBDEV_SKIP_VALIDATE
 	/* Validate op structure */
 	if (validate_enc_op(op, q) == -1) {
-		rte_bbdev_log(ERR, "Turbo encoder validation failed");
+		rte_bbdev_log(ERR, "Turbo encoder validation rejected");
 		return -EINVAL;
 	}
 #endif
@@ -2101,10 +2101,10 @@ enqueue_ldpc_enc_n_op_cb(struct acc_queue *q, struct rte_bbdev_enc_op **ops,
 	uint16_t  in_length_in_bytes;
 	struct rte_bbdev_op_ldpc_enc *enc = &ops[0]->ldpc_enc;
 
-#ifdef RTE_LIBRTE_BBDEV_DEBUG
+#ifndef RTE_LIBRTE_BBDEV_SKIP_VALIDATE
 	/* Validate op structure */
 	if (validate_ldpc_enc_op(ops[0], q) == -1) {
-		rte_bbdev_log(ERR, "LDPC encoder validation failed");
+		rte_bbdev_log(ERR, "LDPC encoder validation rejected");
 		return -EINVAL;
 	}
 #endif
@@ -2163,10 +2163,10 @@ enqueue_ldpc_enc_one_op_cb(struct acc_queue *q, struct rte_bbdev_enc_op *op,
 		seg_total_left;
 	struct rte_mbuf *input, *output_head, *output;
 
-#ifdef RTE_LIBRTE_BBDEV_DEBUG
 	/* Validate op structure */
+#ifndef RTE_LIBRTE_BBDEV_SKIP_VALIDATE
 	if (validate_ldpc_enc_op(op, q) == -1) {
-		rte_bbdev_log(ERR, "LDPC encoder validation failed");
+		rte_bbdev_log(ERR, "LDPC encoder validation rejected");
 		return -EINVAL;
 	}
 #endif
@@ -2220,10 +2220,10 @@ enqueue_enc_one_op_tb(struct acc_queue *q, struct rte_bbdev_enc_op *op,
 	struct rte_mbuf *input, *output_head, *output;
 	uint16_t current_enqueued_cbs = 0;
 
-#ifdef RTE_LIBRTE_BBDEV_DEBUG
+#ifndef RTE_LIBRTE_BBDEV_SKIP_VALIDATE
 	/* Validate op structure */
 	if (validate_enc_op(op, q) == -1) {
-		rte_bbdev_log(ERR, "Turbo encoder validation failed");
+		rte_bbdev_log(ERR, "Turbo encoder validation rejected");
 		return -EINVAL;
 	}
 #endif
@@ -2296,7 +2296,7 @@ enqueue_enc_one_op_tb(struct acc_queue *q, struct rte_bbdev_enc_op *op,
 	return current_enqueued_cbs;
 }
 
-#ifdef RTE_LIBRTE_BBDEV_DEBUG
+#ifndef RTE_LIBRTE_BBDEV_SKIP_VALIDATE
 /* Validates turbo decoder parameters */
 static inline int
 validate_dec_op(struct rte_bbdev_dec_op *op, struct acc_queue *q)
@@ -2454,10 +2454,10 @@ 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;
 
-#ifdef RTE_LIBRTE_BBDEV_DEBUG
+#ifndef RTE_LIBRTE_BBDEV_SKIP_VALIDATE
 	/* Validate op structure */
 	if (validate_dec_op(op, q) == -1) {
-		rte_bbdev_log(ERR, "Turbo decoder validation failed");
+		rte_bbdev_log(ERR, "Turbo decoder validation rejected");
 		return -EINVAL;
 	}
 #endif
@@ -2678,10 +2678,10 @@ enqueue_ldpc_dec_one_op_cb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
 		return ret;
 	}
 
-#ifdef RTE_LIBRTE_BBDEV_DEBUG
+#ifndef RTE_LIBRTE_BBDEV_SKIP_VALIDATE
 	/* Validate op structure */
 	if (validate_ldpc_dec_op(op, q) == -1) {
-		rte_bbdev_log(ERR, "LDPC decoder validation failed");
+		rte_bbdev_log(ERR, "LDPC decoder validation rejected");
 		return -EINVAL;
 	}
 #endif
@@ -2783,10 +2783,10 @@ enqueue_ldpc_dec_one_op_tb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
 	struct rte_mbuf *input, *h_output_head, *h_output;
 	uint16_t current_enqueued_cbs = 0;
 
-#ifdef RTE_LIBRTE_BBDEV_DEBUG
+#ifndef RTE_LIBRTE_BBDEV_SKIP_VALIDATE
 	/* Validate op structure */
 	if (validate_ldpc_dec_op(op, q) == -1) {
-		rte_bbdev_log(ERR, "LDPC decoder validation failed");
+		rte_bbdev_log(ERR, "LDPC decoder validation rejected");
 		return -EINVAL;
 	}
 #endif
@@ -2875,10 +2875,10 @@ enqueue_dec_one_op_tb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
 		*s_output_head, *s_output;
 	uint16_t current_enqueued_cbs = 0;
 
-#ifdef RTE_LIBRTE_BBDEV_DEBUG
+#ifndef RTE_LIBRTE_BBDEV_SKIP_VALIDATE
 	/* Validate op structure */
 	if (validate_dec_op(op, q) == -1) {
-		rte_bbdev_log(ERR, "Turbo decoder validation failed");
+		rte_bbdev_log(ERR, "Turbo decoder validation rejected");
 		return -EINVAL;
 	}
 #endif
-- 
2.37.1

RE: [EXT] [PATCH v3 18/30] baseband/acc100: enable input validation by default

[Akhil Goyal]

> Enable validation functions by default and provide a new flag
> RTE_LIBRTE_SKIP_VALIDATE if the user wants to run without
> validating input to save cycles.
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> 
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  drivers/baseband/acc/rte_acc100_pmd.c | 36 +++++++++++++--------------
>  1 file changed, 18 insertions(+), 18 deletions(-)
How will the user know about this new flag?
Can you update the driver documentation for this?

Re: [PATCH v3 18/30] baseband/acc100: enable input validation by default

[Maxime Coquelin]

On 10/12/22 04:53, Hernan Vargas wrote:
> Enable validation functions by default and provide a new flag
> RTE_LIBRTE_SKIP_VALIDATE if the user wants to run without
> validating input to save cycles.

I would prefer a devarg, so that it can be enabled/disabled at runtime,
instead of build time. The extra if condition would minimal.

> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> 
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>   drivers/baseband/acc/rte_acc100_pmd.c | 36 +++++++++++++--------------
>   1 file changed, 18 insertions(+), 18 deletions(-)
> 

RE: [PATCH v3 18/30] baseband/acc100: enable input validation by default

[Chautru, Nicolas]

Hi Maxime, 

> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> On 10/12/22 04:53, Hernan Vargas wrote:
> > Enable validation functions by default and provide a new flag
> > RTE_LIBRTE_SKIP_VALIDATE if the user wants to run without validating
> > input to save cycles.
> 
> I would prefer a devarg, so that it can be enabled/disabled at runtime, instead
> of build time. The extra if condition would minimal.

Could you please review the previous discussion on https://patches.dpdk.org/project/dpdk/cover/20221012025346.204394-1-hernan.vargas@intel.com/
Basically we would keep this build time flag and update documentation for this release. Then for next release consider devarg. 
Let us know what you think?

Re: [PATCH v3 18/30] baseband/acc100: enable input validation by default

[Maxime Coquelin]

Hi Nicolas,

On 10/20/22 00:12, Chautru, Nicolas wrote:
> Hi Maxime,
> 
>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>> On 10/12/22 04:53, Hernan Vargas wrote:
>>> Enable validation functions by default and provide a new flag
>>> RTE_LIBRTE_SKIP_VALIDATE if the user wants to run without validating
>>> input to save cycles.
>>
>> I would prefer a devarg, so that it can be enabled/disabled at runtime, instead
>> of build time. The extra if condition would minimal.
> 
> Could you please review the previous discussion on https://patches.dpdk.org/project/dpdk/cover/20221012025346.204394-1-hernan.vargas@intel.com/
> Basically we would keep this build time flag and update documentation for this release. Then for next release consider devarg.
> Let us know what you think?
> 
> 

That's not ideal, but if you commit to do it in the next release this is
OK for me.

Thanks,
Maxime

[PATCH v3 19/30] baseband/acc100: added LDPC transport block support

[Hernan Vargas]

Added LDPC enqueue functions to handle transport blocks.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/baseband/acc/rte_acc100_pmd.c | 167 +++++++++++++++++++++++++-
 1 file changed, 164 insertions(+), 3 deletions(-)

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
index 1ccbe4b8b7..1f671ac8f2 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -2152,6 +2152,56 @@ enqueue_ldpc_enc_n_op_cb(struct acc_queue *q, struct rte_bbdev_enc_op **ops,
 	return num;
 }
 
+/* Enqueue one encode operations for ACC100 device for a partial TB
+ * all codes blocks have same configuration multiplexed on the same descriptor.
+ */
+static inline void
+enqueue_ldpc_enc_part_tb(struct acc_queue *q, struct rte_bbdev_enc_op *op,
+		uint16_t total_enqueued_descs, int16_t num_cbs, uint32_t e,
+		uint16_t in_len_bytes, uint32_t out_len_bytes, uint32_t *in_offset,
+		uint32_t *out_offset)
+{
+	union acc_dma_desc *desc = NULL;
+	struct rte_mbuf *output_head, *output;
+	int i, next_triplet;
+	struct rte_bbdev_op_ldpc_enc *enc = &op->ldpc_enc;
+	uint16_t desc_idx = ((q->sw_ring_head + total_enqueued_descs) & q->sw_ring_wrap_mask);
+
+	desc = q->ring_addr + desc_idx;
+	acc_fcw_le_fill(op, &desc->req.fcw_le, num_cbs, e);
+
+	/* This could be done at polling. */
+	acc_header_init(&desc->req);
+	desc->req.numCBs = num_cbs;
+
+	desc->req.m2dlen = 1 + num_cbs;
+	desc->req.d2mlen = num_cbs;
+	next_triplet = 1;
+
+	for (i = 0; i < num_cbs; i++) {
+		desc->req.data_ptrs[next_triplet].address =
+			rte_pktmbuf_iova_offset(enc->input.data, *in_offset);
+		*in_offset += in_len_bytes;
+		desc->req.data_ptrs[next_triplet].blen = in_len_bytes;
+		next_triplet++;
+		desc->req.data_ptrs[next_triplet].address =
+			rte_pktmbuf_iova_offset(enc->output.data, *out_offset);
+		*out_offset += out_len_bytes;
+		desc->req.data_ptrs[next_triplet].blen = out_len_bytes;
+		next_triplet++;
+		enc->output.length += out_len_bytes;
+		output_head = output = enc->output.data;
+		mbuf_append(output_head, output, out_len_bytes);
+	}
+
+#ifdef RTE_LIBRTE_BBDEV_DEBUG
+	rte_memdump(stderr, "FCW", &desc->req.fcw_le,
+			sizeof(desc->req.fcw_le) - 8);
+	rte_memdump(stderr, "Req Desc.", desc, sizeof(*desc));
+#endif
+
+}
+
 /* Enqueue one encode operations for ACC100 device in CB mode */
 static inline int
 enqueue_ldpc_enc_one_op_cb(struct acc_queue *q, struct rte_bbdev_enc_op *op,
@@ -2296,6 +2346,76 @@ enqueue_enc_one_op_tb(struct acc_queue *q, struct rte_bbdev_enc_op *op,
 	return current_enqueued_cbs;
 }
 
+/* Enqueue one encode operations for ACC100 device in TB mode.
+ * returns the number of descs used.
+ */
+static inline int
+enqueue_ldpc_enc_one_op_tb(struct acc_queue *q, struct rte_bbdev_enc_op *op,
+		uint16_t enq_descs, uint8_t cbs_in_tb)
+{
+#ifndef RTE_LIBRTE_BBDEV_SKIP_VALIDATE
+	if (validate_ldpc_enc_op(op, q) == -1) {
+		rte_bbdev_log(ERR, "LDPC encoder validation failed");
+		return -EINVAL;
+	}
+#endif
+	uint8_t num_a, num_b;
+	uint16_t desc_idx;
+	uint8_t r = op->ldpc_enc.tb_params.r;
+	uint8_t cab =  op->ldpc_enc.tb_params.cab;
+	union acc_dma_desc *desc;
+	uint16_t init_enq_descs = enq_descs;
+	uint16_t input_len_B = ((op->ldpc_enc.basegraph == 1 ? 22 : 10) *
+			op->ldpc_enc.z_c - op->ldpc_enc.n_filler) >> 3;
+	uint32_t in_offset = 0, out_offset = 0;
+	uint16_t return_descs;
+
+	if (check_bit(op->ldpc_enc.op_flags, RTE_BBDEV_LDPC_CRC_24B_ATTACH))
+		input_len_B -= 3;
+
+	if (r < cab) {
+		num_a = cab - r;
+		num_b = cbs_in_tb - cab;
+	} else {
+		num_a = 0;
+		num_b = cbs_in_tb - r;
+	}
+
+	while (num_a > 0) {
+		uint32_t e = op->ldpc_enc.tb_params.ea;
+		uint32_t out_len_bytes = (e + 7) >> 3;
+		uint8_t enq = RTE_MIN(num_a, ACC_MUX_5GDL_DESC);
+		num_a -= enq;
+		enqueue_ldpc_enc_part_tb(q, op, enq_descs, enq, e, input_len_B,
+				out_len_bytes, &in_offset, &out_offset);
+		enq_descs++;
+	}
+	while (num_b > 0) {
+		uint32_t e = op->ldpc_enc.tb_params.eb;
+		uint32_t out_len_bytes = (e + 7) >> 3;
+		uint8_t enq = RTE_MIN(num_b, ACC_MUX_5GDL_DESC);
+		num_b -= enq;
+		enqueue_ldpc_enc_part_tb(q, op, enq_descs, enq, e, input_len_B,
+				out_len_bytes, &in_offset, &out_offset);
+		enq_descs++;
+	}
+
+	return_descs = enq_descs - init_enq_descs;
+	/* Keep total number of CBs in first TB. */
+	desc_idx = ((q->sw_ring_head + init_enq_descs) & q->sw_ring_wrap_mask);
+	desc = q->ring_addr + desc_idx;
+	desc->req.cbs_in_tb = return_descs; /** Actual number of descriptors. */
+	desc->req.op_addr = op;
+
+	/* Set SDone on last CB descriptor for TB mode. */
+	desc_idx = ((q->sw_ring_head + enq_descs - 1) & q->sw_ring_wrap_mask);
+	desc = q->ring_addr + desc_idx;
+	desc->req.sdone_enable = 1;
+	desc->req.irq_enable = q->irq_enable;
+	desc->req.op_addr = op;
+	return return_descs;
+}
+
 #ifndef RTE_LIBRTE_BBDEV_SKIP_VALIDATE
 /* Validates turbo decoder parameters */
 static inline int
@@ -2876,7 +2996,10 @@ enqueue_dec_one_op_tb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
 	uint16_t current_enqueued_cbs = 0;
 
 #ifndef RTE_LIBRTE_BBDEV_SKIP_VALIDATE
-	/* Validate op structure */
+	if (cbs_in_tb == 0) {
+		rte_bbdev_log(ERR, "Turbo decoder invalid number of CBs");
+		return -EINVAL;
+	}
 	if (validate_dec_op(op, q) == -1) {
 		rte_bbdev_log(ERR, "Turbo decoder validation rejected");
 		return -EINVAL;
@@ -3102,7 +3225,45 @@ acc100_enqueue_enc_tb(struct rte_bbdev_queue_data *q_data,
 	return i;
 }
 
-/* Check room in AQ for the enqueues batches into Qmgr */
+/* Enqueue LDPC encode operations for ACC100 device in TB mode. */
+static uint16_t
+acc100_enqueue_ldpc_enc_tb(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 = acc_ring_avail_enq(q);
+	uint16_t i, enqueued_descs = 0;
+	uint8_t cbs_in_tb;
+	int descs_used;
+
+	for (i = 0; i < num; ++i) {
+		cbs_in_tb = get_num_cbs_in_tb_ldpc_enc(&ops[i]->ldpc_enc);
+		/* Check if there are available space for further processing. */
+		if (unlikely(avail - cbs_in_tb < 0)) {
+			acc_enqueue_ring_full(q_data);
+			break;
+		}
+		descs_used = enqueue_ldpc_enc_one_op_tb(q, ops[i], enqueued_descs, cbs_in_tb);
+		if (descs_used < 0) {
+			acc_enqueue_invalid(q_data);
+			break;
+		}
+		enqueued_descs += descs_used;
+		avail -= descs_used;
+	}
+	if (unlikely(enqueued_descs == 0))
+		return 0; /* Nothing to enqueue. */
+
+	acc_dma_enqueue(q, enqueued_descs, &q_data->queue_stats);
+
+	/* Update stats. */
+	q_data->queue_stats.enqueued_count += i;
+	q_data->queue_stats.enqueue_err_count += num - i;
+
+	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)
 {
@@ -3139,7 +3300,7 @@ acc100_enqueue_ldpc_enc(struct rte_bbdev_queue_data *q_data,
 	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);
+		return acc100_enqueue_ldpc_enc_tb(q_data, ops, num);
 	else
 		return acc100_enqueue_ldpc_enc_cb(q_data, ops, num);
 }
-- 
2.37.1

[PATCH v3 20/30] baseband/acc100: update validate LDPC enc/dec

[Hernan Vargas]

Update validate functions to check for valid LDPC parameters to avoid
any HW issues.
Adding protection for null corner case and for HARQ inbound size out
of range.
HARQ input size from application may be invalid and causing HW issue.
Add checks to ensure that if HARQ is invalid, set to some valid size to
ensure HW issues do not occur.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
---
 drivers/baseband/acc/acc_common.h     |   1 +
 drivers/baseband/acc/rte_acc100_pmd.c | 285 +++++++++++++++++++++++---
 2 files changed, 255 insertions(+), 31 deletions(-)

diff --git a/drivers/baseband/acc/acc_common.h b/drivers/baseband/acc/acc_common.h
index b18319c06d..5a9929c336 100644
--- a/drivers/baseband/acc/acc_common.h
+++ b/drivers/baseband/acc/acc_common.h
@@ -120,6 +120,7 @@
 #define ACC_ALGO_SPA                0
 #define ACC_ALGO_MSA                1
 #define ACC_HARQ_ALIGN_64B          64
+#define ACC_MAX_ZC                  384
 
 /* 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 1f671ac8f2..38faa49189 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -1937,14 +1937,11 @@ static inline int
 validate_ldpc_enc_op(struct rte_bbdev_enc_op *op, struct acc_queue *q)
 {
 	struct rte_bbdev_op_ldpc_enc *ldpc_enc = &op->ldpc_enc;
+	int K, N, q_m, crc24;
 
 	if (!validate_op_required(q))
 		return 0;
 
-	if (op->mempool == NULL) {
-		rte_bbdev_log(ERR, "Invalid mempool pointer");
-		return -1;
-	}
 	if (ldpc_enc->input.data == NULL) {
 		rte_bbdev_log(ERR, "Invalid input pointer");
 		return -1;
@@ -1953,17 +1950,12 @@ validate_ldpc_enc_op(struct rte_bbdev_enc_op *op, struct acc_queue *q)
 		rte_bbdev_log(ERR, "Invalid output pointer");
 		return -1;
 	}
-	if (ldpc_enc->input.length >
-			RTE_BBDEV_LDPC_MAX_CB_SIZE >> 3) {
-		rte_bbdev_log(ERR, "CB size (%u) is too big, max: %d",
-				ldpc_enc->input.length,
-				RTE_BBDEV_LDPC_MAX_CB_SIZE);
+	if (ldpc_enc->input.length == 0) {
+		rte_bbdev_log(ERR, "CB size (%u) is null", ldpc_enc->input.length);
 		return -1;
 	}
 	if ((ldpc_enc->basegraph > 2) || (ldpc_enc->basegraph == 0)) {
-		rte_bbdev_log(ERR,
-				"BG (%u) is out of range 1 <= value <= 2",
-				ldpc_enc->basegraph);
+		rte_bbdev_log(ERR, "BG (%u) is out of range 1 <= value <= 2", ldpc_enc->basegraph);
 		return -1;
 	}
 	if (ldpc_enc->rv_index > 3) {
@@ -1978,13 +1970,89 @@ validate_ldpc_enc_op(struct rte_bbdev_enc_op *op, struct acc_queue *q)
 				ldpc_enc->code_block_mode);
 		return -1;
 	}
-	int K = (ldpc_enc->basegraph == 1 ? 22 : 10) * ldpc_enc->z_c;
-	if (ldpc_enc->n_filler >= K) {
-		rte_bbdev_log(ERR,
-				"K and F are not compatible %u %u",
-				K, ldpc_enc->n_filler);
+	if (ldpc_enc->z_c > ACC_MAX_ZC) {
+		rte_bbdev_log(ERR, "Zc (%u) is out of range", ldpc_enc->z_c);
+		return -1;
+	}
+
+	K = (ldpc_enc->basegraph == 1 ? 22 : 10) * ldpc_enc->z_c;
+	N = (ldpc_enc->basegraph == 1 ? ACC_N_ZC_1 : ACC_N_ZC_2) * ldpc_enc->z_c;
+	q_m = ldpc_enc->q_m;
+	crc24 = 0;
+
+	if (check_bit(op->ldpc_enc.op_flags,
+			RTE_BBDEV_LDPC_CRC_24A_ATTACH) ||
+			check_bit(op->ldpc_enc.op_flags,
+			RTE_BBDEV_LDPC_CRC_24B_ATTACH))
+		crc24 = 24;
+	if ((K - ldpc_enc->n_filler) % 8 > 0) {
+		rte_bbdev_log(ERR, "K - F not byte aligned %u", K - ldpc_enc->n_filler);
+		return -1;
+	}
+	if (ldpc_enc->n_filler > (K - 2 * ldpc_enc->z_c)) {
+		rte_bbdev_log(ERR, "K - F invalid %u %u", K, ldpc_enc->n_filler);
+		return -1;
+	}
+	if ((ldpc_enc->n_cb > N) || (ldpc_enc->n_cb <= K)) {
+		rte_bbdev_log(ERR, "Ncb (%u) is out of range K  %d N %d", ldpc_enc->n_cb, K, N);
 		return -1;
 	}
+	if (!check_bit(op->ldpc_enc.op_flags,
+			RTE_BBDEV_LDPC_INTERLEAVER_BYPASS) &&
+			((q_m == 0) || ((q_m > 2) && ((q_m % 2) == 1))
+			|| (q_m > 8))) {
+		rte_bbdev_log(ERR, "Qm (%u) is out of range", ldpc_enc->q_m);
+		return -1;
+	}
+	if (ldpc_enc->code_block_mode == RTE_BBDEV_CODE_BLOCK) {
+		if (ldpc_enc->cb_params.e == 0) {
+			rte_bbdev_log(ERR, "E is null");
+			return -1;
+		}
+		if (q_m > 0) {
+			if (ldpc_enc->cb_params.e % q_m > 0) {
+				rte_bbdev_log(ERR, "E not multiple of qm %d", q_m);
+				return -1;
+			}
+		}
+		if ((ldpc_enc->z_c <= 11) && (ldpc_enc->cb_params.e > 3456)) {
+			rte_bbdev_log(ERR, "E too large for small block");
+			return -1;
+		}
+		if (ldpc_enc->input.length >
+			RTE_BBDEV_LDPC_MAX_CB_SIZE >> 3) {
+			rte_bbdev_log(ERR, "CB size (%u) is too big, max: %d",
+					ldpc_enc->input.length,
+					RTE_BBDEV_LDPC_MAX_CB_SIZE);
+		return -1;
+		}
+		if (K < (int) (ldpc_enc->input.length * 8 + ldpc_enc->n_filler) + crc24) {
+			rte_bbdev_log(ERR,
+					"K and F not matching input size %u %u %u",
+					K, ldpc_enc->n_filler,
+					ldpc_enc->input.length);
+			return -1;
+		}
+	} else {
+		if ((ldpc_enc->tb_params.c == 0) ||
+				(ldpc_enc->tb_params.ea == 0) ||
+				(ldpc_enc->tb_params.eb == 0)) {
+			rte_bbdev_log(ERR, "TB parameter is null");
+			return -1;
+		}
+		if (q_m > 0) {
+			if ((ldpc_enc->tb_params.ea % q_m > 0) ||
+				(ldpc_enc->tb_params.eb % q_m > 0)) {
+				rte_bbdev_log(ERR, "E not multiple of qm %d", q_m);
+				return -1;
+			}
+		}
+		if ((ldpc_enc->z_c <= 11) && (RTE_MAX(ldpc_enc->tb_params.ea,
+				ldpc_enc->tb_params.eb) > 3456)) {
+			rte_bbdev_log(ERR, "E too large for small block");
+			return -1;
+		}
+	}
 	return 0;
 }
 
@@ -1993,24 +2061,30 @@ static inline int
 validate_ldpc_dec_op(struct rte_bbdev_dec_op *op, struct acc_queue *q)
 {
 	struct rte_bbdev_op_ldpc_dec *ldpc_dec = &op->ldpc_dec;
+	int K, N, q_m;
+	uint32_t min_harq_input;
 
 	if (!validate_op_required(q))
 		return 0;
 
-	if (op->mempool == NULL) {
-		rte_bbdev_log(ERR, "Invalid mempool pointer");
+	if (ldpc_dec->input.data == NULL) {
+		rte_bbdev_log(ERR, "Invalid input pointer");
+		return -1;
+	}
+	if (ldpc_dec->hard_output.data == NULL) {
+		rte_bbdev_log(ERR, "Invalid output pointer");
+		return -1;
+	}
+	if (ldpc_dec->input.length == 0) {
+		rte_bbdev_log(ERR, "input is null");
 		return -1;
 	}
 	if ((ldpc_dec->basegraph > 2) || (ldpc_dec->basegraph == 0)) {
-		rte_bbdev_log(ERR,
-				"BG (%u) is out of range 1 <= value <= 2",
-				ldpc_dec->basegraph);
+		rte_bbdev_log(ERR, "BG (%u) is out of range 1 <= value <= 2", ldpc_dec->basegraph);
 		return -1;
 	}
 	if (ldpc_dec->iter_max == 0) {
-		rte_bbdev_log(ERR,
-				"iter_max (%u) is equal to 0",
-				ldpc_dec->iter_max);
+		rte_bbdev_log(ERR, "iter_max (%u) is equal to 0", ldpc_dec->iter_max);
 		return -1;
 	}
 	if (ldpc_dec->rv_index > 3) {
@@ -2025,13 +2099,162 @@ validate_ldpc_dec_op(struct rte_bbdev_dec_op *op, struct acc_queue *q)
 				ldpc_dec->code_block_mode);
 		return -1;
 	}
-	int K = (ldpc_dec->basegraph == 1 ? 22 : 10) * ldpc_dec->z_c;
-	if (ldpc_dec->n_filler >= K) {
-		rte_bbdev_log(ERR,
-				"K and F are not compatible %u %u",
-				K, ldpc_dec->n_filler);
+	/* Check Zc is valid value. */
+	if ((ldpc_dec->z_c > ACC_MAX_ZC) || (ldpc_dec->z_c < 2)) {
+		rte_bbdev_log(ERR, "Zc (%u) is out of range", ldpc_dec->z_c);
+		return -1;
+	}
+	if (ldpc_dec->z_c > 256) {
+		if ((ldpc_dec->z_c % 32) != 0) {
+			rte_bbdev_log(ERR, "Invalid Zc %d", ldpc_dec->z_c);
+			return -1;
+		}
+	} else if (ldpc_dec->z_c > 128) {
+		if ((ldpc_dec->z_c % 16) != 0) {
+			rte_bbdev_log(ERR, "Invalid Zc %d", ldpc_dec->z_c);
+			return -1;
+		}
+	} else if (ldpc_dec->z_c > 64) {
+		if ((ldpc_dec->z_c % 8) != 0) {
+			rte_bbdev_log(ERR, "Invalid Zc %d", ldpc_dec->z_c);
+			return -1;
+		}
+	} else if (ldpc_dec->z_c > 32) {
+		if ((ldpc_dec->z_c % 4) != 0) {
+			rte_bbdev_log(ERR, "Invalid Zc %d", ldpc_dec->z_c);
+			return -1;
+		}
+	} else if (ldpc_dec->z_c > 16) {
+		if ((ldpc_dec->z_c % 2) != 0) {
+			rte_bbdev_log(ERR, "Invalid Zc %d", ldpc_dec->z_c);
+			return -1;
+		}
+	}
+
+	K = (ldpc_dec->basegraph == 1 ? 22 : 10) * ldpc_dec->z_c;
+	N = (ldpc_dec->basegraph == 1 ? ACC_N_ZC_1 : ACC_N_ZC_2) * ldpc_dec->z_c;
+	q_m = ldpc_dec->q_m;
+
+	if (ldpc_dec->n_filler >= K - 2 * ldpc_dec->z_c) {
+		rte_bbdev_log(ERR, "K and F are not compatible %u %u", K, ldpc_dec->n_filler);
+		return -1;
+	}
+	if ((ldpc_dec->n_cb > N) || (ldpc_dec->n_cb <= K)) {
+		rte_bbdev_log(ERR, "Ncb (%u) is out of range K  %d N %d", ldpc_dec->n_cb, K, N);
+		return -1;
+	}
+	if (((q_m == 0) || ((q_m > 2) && ((q_m % 2) == 1))
+			|| (q_m > 8))) {
+		rte_bbdev_log(ERR, "Qm (%u) is out of range", ldpc_dec->q_m);
+		return -1;
+	}
+	if (ldpc_dec->code_block_mode == RTE_BBDEV_CODE_BLOCK) {
+		if (ldpc_dec->cb_params.e == 0) {
+			rte_bbdev_log(ERR, "E is null");
+			return -1;
+		}
+		if (ldpc_dec->cb_params.e % q_m > 0) {
+			rte_bbdev_log(ERR, "E not multiple of qm %d", q_m);
+			return -1;
+		}
+		if (ldpc_dec->cb_params.e > 512 * ldpc_dec->z_c) {
+			rte_bbdev_log(ERR, "E too high");
+			return -1;
+		}
+	} else {
+		if ((ldpc_dec->tb_params.c == 0) ||
+				(ldpc_dec->tb_params.ea == 0) ||
+				(ldpc_dec->tb_params.eb == 0)) {
+			rte_bbdev_log(ERR, "TB parameter is null");
+			return -1;
+		}
+		if ((ldpc_dec->tb_params.ea % q_m > 0) ||
+				(ldpc_dec->tb_params.eb % q_m > 0)) {
+			rte_bbdev_log(ERR, "E not multiple of qm %d", q_m);
+			return -1;
+		}
+		if ((ldpc_dec->tb_params.ea > 512 * ldpc_dec->z_c) ||
+				(ldpc_dec->tb_params.eb > 512 * ldpc_dec->z_c)) {
+			rte_bbdev_log(ERR, "E too high");
+			return -1;
+		}
+	}
+	if (check_bit(op->ldpc_dec.op_flags, RTE_BBDEV_LDPC_DECODE_BYPASS)) {
+		rte_bbdev_log(ERR, "Avoid LDPC Decode bypass");
+		return -1;
+	}
+
+	/* Avoid HARQ compression for small block size */
+	if ((check_bit(op->ldpc_dec.op_flags, RTE_BBDEV_LDPC_HARQ_6BIT_COMPRESSION)) && (K < 2048))
+		op->ldpc_dec.op_flags ^= RTE_BBDEV_LDPC_HARQ_6BIT_COMPRESSION;
+
+	min_harq_input = check_bit(op->ldpc_dec.op_flags,
+			RTE_BBDEV_LDPC_HARQ_6BIT_COMPRESSION) ? 256 : 64;
+	if (check_bit(op->ldpc_dec.op_flags,
+			RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE) &&
+			ldpc_dec->harq_combined_input.length <
+			min_harq_input) {
+		rte_bbdev_log(ERR, "HARQ input size is too small %d < %d",
+			ldpc_dec->harq_combined_input.length,
+			min_harq_input);
 		return -1;
 	}
+
+	/* Enforce in-range HARQ input size */
+	if (check_bit(op->ldpc_dec.op_flags, RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE)) {
+		uint32_t max_harq_input = RTE_ALIGN_CEIL(ldpc_dec->n_cb - ldpc_dec->n_filler, 64);
+
+		if (check_bit(op->ldpc_dec.op_flags, RTE_BBDEV_LDPC_HARQ_6BIT_COMPRESSION))
+			max_harq_input = max_harq_input * 3 / 4;
+
+		if (ldpc_dec->harq_combined_input.length > max_harq_input) {
+			rte_bbdev_log(ERR,
+					"HARQ input size out of range %d > %d, Ncb %d F %d K %d N %d",
+					ldpc_dec->harq_combined_input.length,
+					max_harq_input, ldpc_dec->n_cb,
+					ldpc_dec->n_filler, K, N);
+			/* Fallback to flush HARQ combine */
+			ldpc_dec->harq_combined_input.length = 0;
+
+			if (check_bit(op->ldpc_dec.op_flags, RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE))
+				op->ldpc_dec.op_flags ^= RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE;
+		}
+	}
+
+#ifdef ACC100_EXT_MEM
+	/* Enforce in-range HARQ offset */
+	if (check_bit(op->ldpc_dec.op_flags, RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE)) {
+		if ((op->ldpc_dec.harq_combined_input.offset >> 10) >= q->d->ddr_size) {
+			rte_bbdev_log(ERR,
+				"HARQin offset out of range %d > %d",
+				op->ldpc_dec.harq_combined_input.offset,
+				q->d->ddr_size);
+			return -1;
+		}
+		if ((op->ldpc_dec.harq_combined_input.offset & 0x3FF) > 0) {
+			rte_bbdev_log(ERR,
+				"HARQin offset not aligned on 1kB %d",
+				op->ldpc_dec.harq_combined_input.offset);
+			return -1;
+		}
+	}
+	if (check_bit(op->ldpc_dec.op_flags, RTE_BBDEV_LDPC_HQ_COMBINE_OUT_ENABLE)) {
+		if ((op->ldpc_dec.harq_combined_output.offset >> 10) >= q->d->ddr_size) {
+			rte_bbdev_log(ERR,
+				"HARQout offset out of range %d > %d",
+				op->ldpc_dec.harq_combined_output.offset,
+				q->d->ddr_size);
+			return -1;
+		}
+		if ((op->ldpc_dec.harq_combined_output.offset & 0x3FF) > 0) {
+			rte_bbdev_log(ERR,
+				"HARQout offset not aligned on 1kB %d",
+				op->ldpc_dec.harq_combined_output.offset);
+			return -1;
+		}
+	}
+#endif
+
 	return 0;
 }
 #endif
@@ -2677,7 +2900,7 @@ harq_loopback(struct acc_queue *q, struct rte_bbdev_dec_op *op,
 	memset(fcw, 0, sizeof(struct acc_fcw_ld));
 	fcw->FCWversion = ACC_FCW_VER;
 	fcw->qm = 2;
-	fcw->Zc = 384;
+	fcw->Zc = ACC_MAX_ZC;
 	if (harq_in_length < 16 * ACC_N_ZC_1)
 		fcw->Zc = 16;
 	fcw->ncb = fcw->Zc * ACC_N_ZC_1;
-- 
2.37.1

[PATCH v3 21/30] baseband/acc100: implement configurable queue depth

[Hernan Vargas]

Implement new feature to make queue depth configurable based on decode
or encode mode.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/baseband/acc/rte_acc100_pmd.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
index 38faa49189..936a417c3d 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -768,9 +768,15 @@ acc100_queue_setup(struct rte_bbdev *dev, uint16_t queue_id,
 	q->qgrp_id = (q_idx >> ACC100_GRP_ID_SHIFT) & 0xF;
 	q->vf_id = (q_idx >> ACC100_VF_ID_SHIFT)  & 0x3F;
 	q->aq_id = q_idx & 0xF;
-	q->aq_depth = (conf->op_type ==  RTE_BBDEV_OP_TURBO_DEC) ?
-			(1 << d->acc_conf.q_ul_4g.aq_depth_log2) :
-			(1 << d->acc_conf.q_dl_4g.aq_depth_log2);
+	q->aq_depth = 0;
+	if (conf->op_type ==  RTE_BBDEV_OP_TURBO_DEC)
+		q->aq_depth = (1 << d->acc_conf.q_ul_4g.aq_depth_log2);
+	else if (conf->op_type ==  RTE_BBDEV_OP_TURBO_ENC)
+		q->aq_depth = (1 << d->acc_conf.q_dl_4g.aq_depth_log2);
+	else if (conf->op_type ==  RTE_BBDEV_OP_LDPC_DEC)
+		q->aq_depth = (1 << d->acc_conf.q_ul_5g.aq_depth_log2);
+	else if (conf->op_type ==  RTE_BBDEV_OP_LDPC_ENC)
+		q->aq_depth = (1 << d->acc_conf.q_dl_5g.aq_depth_log2);
 
 	q->mmio_reg_enqueue = RTE_PTR_ADD(d->mmio_base,
 			queue_offset(d->pf_device,
-- 
2.37.1

[PATCH v3 22/30] baseband/acc100: add queue stop operation

[Hernan Vargas]

Implement new feature to stop queue operation.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/baseband/acc/rte_acc100_pmd.c | 60 +++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
index 936a417c3d..269d8295c2 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -791,6 +791,65 @@ acc100_queue_setup(struct rte_bbdev *dev, uint16_t queue_id,
 	return 0;
 }
 
+static inline void
+acc100_print_op(struct rte_bbdev_dec_op *op, enum rte_bbdev_op_type op_type,
+		uint16_t index)
+{
+	if (op == NULL)
+		return;
+	if (op_type == RTE_BBDEV_OP_LDPC_DEC)
+		rte_bbdev_log(DEBUG,
+			"  Op 5GUL %d %d %d %d %d %d %d %d %d %d %d %d",
+			index,
+			op->ldpc_dec.basegraph, op->ldpc_dec.z_c,
+			op->ldpc_dec.n_cb, op->ldpc_dec.q_m,
+			op->ldpc_dec.n_filler, op->ldpc_dec.cb_params.e,
+			op->ldpc_dec.op_flags, op->ldpc_dec.rv_index,
+			op->ldpc_dec.iter_max, op->ldpc_dec.iter_count,
+			op->ldpc_dec.harq_combined_input.length
+			);
+	else if (op_type == RTE_BBDEV_OP_LDPC_ENC) {
+		struct rte_bbdev_enc_op *op_dl = (struct rte_bbdev_enc_op *) op;
+		rte_bbdev_log(DEBUG,
+			"  Op 5GDL %d %d %d %d %d %d %d %d %d",
+			index,
+			op_dl->ldpc_enc.basegraph, op_dl->ldpc_enc.z_c,
+			op_dl->ldpc_enc.n_cb, op_dl->ldpc_enc.q_m,
+			op_dl->ldpc_enc.n_filler, op_dl->ldpc_enc.cb_params.e,
+			op_dl->ldpc_enc.op_flags, op_dl->ldpc_enc.rv_index
+			);
+	}
+}
+
+static int
+acc100_queue_stop(struct rte_bbdev *dev, uint16_t queue_id)
+{
+	struct acc_queue *q;
+	struct rte_bbdev_dec_op *op;
+	uint16_t i;
+
+	q = dev->data->queues[queue_id].queue_private;
+	rte_bbdev_log(INFO, "Queue Stop %d H/T/D %d %d %x OpType %d",
+			queue_id, q->sw_ring_head, q->sw_ring_tail,
+			q->sw_ring_depth, q->op_type);
+	for (i = 0; i < q->sw_ring_depth; ++i) {
+		op = (q->ring_addr + i)->req.op_addr;
+		acc100_print_op(op, q->op_type, i);
+	}
+	/* ignore all operations in flight and clear counters */
+	q->sw_ring_tail = q->sw_ring_head;
+	q->aq_enqueued = 0;
+	q->aq_dequeued = 0;
+	dev->data->queues[queue_id].queue_stats.enqueued_count = 0;
+	dev->data->queues[queue_id].queue_stats.dequeued_count = 0;
+	dev->data->queues[queue_id].queue_stats.enqueue_err_count = 0;
+	dev->data->queues[queue_id].queue_stats.dequeue_err_count = 0;
+	dev->data->queues[queue_id].queue_stats.enqueue_warn_count = 0;
+	dev->data->queues[queue_id].queue_stats.dequeue_warn_count = 0;
+
+	return 0;
+}
+
 /* Release ACC100 queue */
 static int
 acc100_queue_release(struct rte_bbdev *dev, uint16_t q_id)
@@ -983,6 +1042,7 @@ static const struct rte_bbdev_ops acc100_bbdev_ops = {
 	.info_get = acc100_dev_info_get,
 	.queue_setup = acc100_queue_setup,
 	.queue_release = acc100_queue_release,
+	.queue_stop = acc100_queue_stop,
 	.queue_intr_enable = acc100_queue_intr_enable,
 	.queue_intr_disable = acc100_queue_intr_disable
 };
-- 
2.37.1

[PATCH v3 23/30] baseband/acc100: update uplink CB input length

[Hernan Vargas]

Use the FCW E parameter for rate matching as the code block input
length.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 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 269d8295c2..e0d22525e9 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -1697,7 +1697,7 @@ acc100_dma_desc_ld_fill(struct rte_bbdev_dec_op *op,
 		crc24_overlap = 24;
 
 	/* Compute some LDPC BG lengths */
-	input_length = dec->cb_params.e;
+	input_length = fcw->rm_e;
 	if (check_bit(op->ldpc_dec.op_flags,
 			RTE_BBDEV_LDPC_LLR_COMPRESSION))
 		input_length = (input_length * 3 + 3) / 4;
-- 
2.37.1

[PATCH v3 24/30] baseband/acc100: rename ldpc encode function arg

[Hernan Vargas]

Rename total_enqueued_cbs to total_enqueued_descs in the
enqueue_ldpc_enc_n_op_cb function. No functional impact.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/baseband/acc/rte_acc100_pmd.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
index e0d22525e9..c74b1c61d2 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -2378,28 +2378,29 @@ enqueue_enc_one_op_cb(struct acc_queue *q, struct rte_bbdev_enc_op *op,
 	return 1;
 }
 
-/* Enqueue one encode operations for ACC100 device in CB mode */
+
+/* Enqueue one encode operations for ACC100 device in CB mode
+ * multiplexed on the same descriptor
+ */
 static inline int
 enqueue_ldpc_enc_n_op_cb(struct acc_queue *q, struct rte_bbdev_enc_op **ops,
-		uint16_t total_enqueued_cbs, int16_t num)
+		uint16_t total_enqueued_descs, int16_t num)
 {
 	union acc_dma_desc *desc = NULL;
 	uint32_t out_length;
 	struct rte_mbuf *output_head, *output;
 	int i, next_triplet;
-	uint16_t  in_length_in_bytes;
+	uint16_t  in_length_in_bytes, desc_idx;
 	struct rte_bbdev_op_ldpc_enc *enc = &ops[0]->ldpc_enc;
 
 #ifndef RTE_LIBRTE_BBDEV_SKIP_VALIDATE
-	/* Validate op structure */
 	if (validate_ldpc_enc_op(ops[0], q) == -1) {
 		rte_bbdev_log(ERR, "LDPC encoder validation rejected");
 		return -EINVAL;
 	}
 #endif
 
-	uint16_t desc_idx = ((q->sw_ring_head + total_enqueued_cbs)
-			& q->sw_ring_wrap_mask);
+	desc_idx = ((q->sw_ring_head + total_enqueued_descs) & q->sw_ring_wrap_mask);
 	desc = q->ring_addr + desc_idx;
 	acc_fcw_le_fill(ops[0], &desc->req.fcw_le, num, 0);
 
-- 
2.37.1

RE: [EXT] [PATCH v3 24/30] baseband/acc100: rename ldpc encode function arg

[Akhil Goyal]

> Rename total_enqueued_cbs to total_enqueued_descs in the
> enqueue_ldpc_enc_n_op_cb function. No functional impact.
> 

Can you specify the reason for rename in description?

> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> 
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

[PATCH v3 25/30] baseband/acc100: update log messages

[Hernan Vargas]

Add extra values for some log messages. No functional impact.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 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 c74b1c61d2..5c22dc2c45 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -971,6 +971,7 @@ acc100_dev_info_get(struct rte_bbdev *dev,
 
 	/* Read and save the populated config from ACC100 registers */
 	fetch_acc100_config(dev);
+	/* Check the status of device */
 	dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
 
 	/* Expose number of queues */
@@ -2645,7 +2646,7 @@ enqueue_ldpc_enc_one_op_tb(struct acc_queue *q, struct rte_bbdev_enc_op *op,
 {
 #ifndef RTE_LIBRTE_BBDEV_SKIP_VALIDATE
 	if (validate_ldpc_enc_op(op, q) == -1) {
-		rte_bbdev_log(ERR, "LDPC encoder validation failed");
+		rte_bbdev_log(ERR, "LDPC encoder validation rejected");
 		return -EINVAL;
 	}
 #endif
@@ -3994,6 +3995,7 @@ dequeue_ldpc_dec_one_op_cb(struct rte_bbdev_queue_data *q_data,
 		return -1;
 
 	rsp.val = atom_desc.rsp.val;
+	rte_bbdev_log_debug("Resp. desc %p: %x\n", desc, rsp.val);
 
 	/* Dequeue */
 	op = desc->req.op_addr;
@@ -4076,8 +4078,9 @@ dequeue_dec_one_op_tb(struct acc_queue *q, struct rte_bbdev_dec_op **ref_op,
 		atom_desc.atom_hdr = __atomic_load_n((uint64_t *)desc,
 				__ATOMIC_RELAXED);
 		rsp.val = atom_desc.rsp.val;
-		rte_bbdev_log_debug("Resp. desc %p: %x", desc,
-				rsp.val);
+		rte_bbdev_log_debug("Resp. desc %p: %x r %d c %d\n",
+				desc, rsp.val,
+				cb_idx, cbs_in_tb);
 
 		op->status |= ((rsp.input_err)
 				? (1 << RTE_BBDEV_DATA_ERROR) : 0);
-- 
2.37.1

[PATCH v3 26/30] baseband/acc100: store FCW from first CB descriptor

[Hernan Vargas]

Store the descriptor from the first code block from a transport block.
Copy the LDPC FCW from the first descriptor into the rest of the CBs in
that TB.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/baseband/acc/rte_acc100_pmd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
index 5c22dc2c45..ca809aa792 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -3187,6 +3187,7 @@ enqueue_ldpc_dec_one_op_tb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
 		uint16_t total_enqueued_cbs, uint8_t cbs_in_tb)
 {
 	union acc_dma_desc *desc = NULL;
+	union acc_dma_desc *desc_first = NULL;
 	int ret;
 	uint8_t r, c;
 	uint32_t in_offset, h_out_offset,
@@ -3205,6 +3206,7 @@ enqueue_ldpc_dec_one_op_tb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
 	uint16_t desc_idx = ((q->sw_ring_head + total_enqueued_cbs)
 			& q->sw_ring_wrap_mask);
 	desc = q->ring_addr + desc_idx;
+	desc_first = desc;
 	uint64_t fcw_offset = (desc_idx << 8) + ACC_DESC_FCW_OFFSET;
 	union acc_harq_layout_data *harq_layout = q->d->harq_layout;
 	q->d->fcw_ld_fill(op, &desc->req.fcw_ld, harq_layout);
@@ -3228,6 +3230,7 @@ enqueue_ldpc_dec_one_op_tb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
 				& q->sw_ring_wrap_mask);
 		desc->req.data_ptrs[0].address = q->ring_addr_iova + fcw_offset;
 		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);
 		ret = acc100_dma_desc_ld_fill(op, &desc->req, &input,
 				h_output, &in_offset, &h_out_offset,
 				&h_out_length,
-- 
2.37.1

[PATCH v3 27/30] baseband/acc100: update device info

[Hernan Vargas]

Remove unused capabilities, use dummy operation as start count for
number of queues.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/baseband/acc/rte_acc100_pmd.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
index ca809aa792..23c8e47335 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -877,7 +877,6 @@ acc100_dev_info_get(struct rte_bbdev *dev,
 {
 	struct acc_device *d = dev->data->dev_private;
 	int i;
-
 	static const struct rte_bbdev_op_cap bbdev_capabilities[] = {
 		{
 			.type = RTE_BBDEV_OP_TURBO_DEC,
@@ -889,7 +888,6 @@ acc100_dev_info_get(struct rte_bbdev *dev,
 					RTE_BBDEV_TURBO_EARLY_TERMINATION |
 					RTE_BBDEV_TURBO_DEC_INTERRUPTS |
 					RTE_BBDEV_TURBO_NEG_LLR_1_BIT_IN |
-					RTE_BBDEV_TURBO_MAP_DEC |
 					RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP |
 					RTE_BBDEV_TURBO_DEC_CRC_24B_DROP |
 					RTE_BBDEV_TURBO_DEC_SCATTER_GATHER,
@@ -984,12 +982,13 @@ acc100_dev_info_get(struct rte_bbdev *dev,
 			d->acc_conf.q_ul_5g.num_qgroups;
 	dev_info->num_queues[RTE_BBDEV_OP_LDPC_ENC] = d->acc_conf.q_dl_5g.num_aqs_per_groups *
 			d->acc_conf.q_dl_5g.num_qgroups;
+	dev_info->num_queues[RTE_BBDEV_OP_FFT] = 0;
 	dev_info->queue_priority[RTE_BBDEV_OP_TURBO_DEC] = d->acc_conf.q_ul_4g.num_qgroups;
 	dev_info->queue_priority[RTE_BBDEV_OP_TURBO_ENC] = d->acc_conf.q_dl_4g.num_qgroups;
 	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_DEC] = d->acc_conf.q_ul_5g.num_qgroups;
 	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_ENC] = d->acc_conf.q_dl_5g.num_qgroups;
 	dev_info->max_num_queues = 0;
-	for (i = RTE_BBDEV_OP_TURBO_DEC; i <= RTE_BBDEV_OP_LDPC_ENC; i++)
+	for (i = RTE_BBDEV_OP_NONE; i <= RTE_BBDEV_OP_LDPC_ENC; i++)
 		dev_info->max_num_queues += dev_info->num_queues[i];
 	dev_info->queue_size_lim = ACC_MAX_QUEUE_DEPTH;
 	dev_info->hardware_accelerated = true;
-- 
2.37.1

[PATCH v3 28/30] baseband/acc100: add ring companion address

[Hernan Vargas]

Store the virtual address of companion ring as part of queue
information. Use this address to calculate the op address.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
---
 drivers/baseband/acc/rte_acc100_pmd.c | 175 +++++++++++++++++---------
 1 file changed, 113 insertions(+), 62 deletions(-)

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
index 23c8e47335..f93fd885a3 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -668,6 +668,7 @@ acc100_queue_setup(struct rte_bbdev *dev, uint16_t queue_id,
 	struct acc_device *d = dev->data->dev_private;
 	struct acc_queue *q;
 	int16_t q_idx;
+	int ret;
 
 	if (d == NULL) {
 		rte_bbdev_log(ERR, "Undefined device");
@@ -726,8 +727,8 @@ acc100_queue_setup(struct rte_bbdev *dev, uint16_t queue_id,
 			RTE_CACHE_LINE_SIZE, conf->socket);
 	if (q->lb_in == NULL) {
 		rte_bbdev_log(ERR, "Failed to allocate lb_in memory");
-		rte_free(q);
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto free_q;
 	}
 	q->lb_in_addr_iova = rte_malloc_virt2iova(q->lb_in);
 	q->lb_out = rte_zmalloc_socket(dev->device->driver->name,
@@ -735,11 +736,18 @@ acc100_queue_setup(struct rte_bbdev *dev, uint16_t queue_id,
 			RTE_CACHE_LINE_SIZE, conf->socket);
 	if (q->lb_out == NULL) {
 		rte_bbdev_log(ERR, "Failed to allocate lb_out memory");
-		rte_free(q->lb_in);
-		rte_free(q);
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto free_lb_in;
 	}
 	q->lb_out_addr_iova = rte_malloc_virt2iova(q->lb_out);
+	q->companion_ring_addr = rte_zmalloc_socket(dev->device->driver->name,
+			d->sw_ring_max_depth * sizeof(*q->companion_ring_addr),
+			RTE_CACHE_LINE_SIZE, conf->socket);
+	if (q->companion_ring_addr == NULL) {
+		rte_bbdev_log(ERR, "Failed to allocate companion_ring memory");
+		ret = -ENOMEM;
+		goto free_lb_out;
+	}
 
 	/*
 	 * Software queue ring wraps synchronously with the HW when it reaches
@@ -759,10 +767,8 @@ acc100_queue_setup(struct rte_bbdev *dev, uint16_t queue_id,
 
 	q_idx = acc100_find_free_queue_idx(dev, conf);
 	if (q_idx == -1) {
-		rte_free(q->lb_in);
-		rte_free(q->lb_out);
-		rte_free(q);
-		return -1;
+		ret = -EINVAL;
+		goto free_companion_ring_addr;
 	}
 
 	q->qgrp_id = (q_idx >> ACC100_GRP_ID_SHIFT) & 0xF;
@@ -789,6 +795,21 @@ acc100_queue_setup(struct rte_bbdev *dev, uint16_t queue_id,
 
 	dev->data->queues[queue_id].queue_private = q;
 	return 0;
+
+free_companion_ring_addr:
+	rte_free(q->companion_ring_addr);
+	q->companion_ring_addr = NULL;
+free_lb_out:
+	rte_free(q->lb_out);
+	q->lb_out = NULL;
+free_lb_in:
+	rte_free(q->lb_in);
+	q->lb_in = NULL;
+free_q:
+	rte_free(q);
+	q = NULL;
+
+	return ret;
 }
 
 static inline void
@@ -861,6 +882,7 @@ acc100_queue_release(struct rte_bbdev *dev, uint16_t q_id)
 		/* Mark the Queue as un-assigned */
 		d->q_assigned_bit_map[q->qgrp_id] &= (0xFFFFFFFFFFFFFFFF -
 				(uint64_t) (1 << q->aq_id));
+		rte_free(q->companion_ring_addr);
 		rte_free(q->lb_in);
 		rte_free(q->lb_out);
 		rte_free(q);
@@ -2431,6 +2453,10 @@ enqueue_ldpc_enc_n_op_cb(struct acc_queue *q, struct rte_bbdev_enc_op **ops,
 	}
 
 	desc->req.op_addr = ops[0];
+	/* Keep track of pointers even when multiplexed in single descriptor */
+	struct acc_ptrs *context_ptrs = q->companion_ring_addr + desc_idx;
+	for (i = 0; i < num; i++)
+		context_ptrs->ptr[i].op_addr = ops[i];
 
 #ifdef RTE_LIBRTE_BBDEV_DEBUG
 	rte_memdump(stderr, "FCW", &desc->req.fcw_le,
@@ -3804,15 +3830,17 @@ acc100_enqueue_ldpc_dec(struct rte_bbdev_queue_data *q_data,
 /* Dequeue one encode operations from ACC100 device in CB mode */
 static inline int
 dequeue_enc_one_op_cb(struct acc_queue *q, struct rte_bbdev_enc_op **ref_op,
-		uint16_t total_dequeued_cbs, uint32_t *aq_dequeued)
+		uint16_t *dequeued_ops, uint32_t *aq_dequeued,
+		uint16_t *dequeued_descs)
 {
 	union acc_dma_desc *desc, atom_desc;
 	union acc_dma_rsp_desc rsp;
 	struct rte_bbdev_enc_op *op;
 	int i;
-
-	desc = q->ring_addr + ((q->sw_ring_tail + total_dequeued_cbs)
+	int desc_idx = ((q->sw_ring_tail + *dequeued_descs)
 			& q->sw_ring_wrap_mask);
+
+	desc = q->ring_addr + desc_idx;
 	atom_desc.atom_hdr = __atomic_load_n((uint64_t *)desc,
 			__ATOMIC_RELAXED);
 
@@ -3821,7 +3849,8 @@ dequeue_enc_one_op_cb(struct acc_queue *q, struct rte_bbdev_enc_op **ref_op,
 		return -1;
 
 	rsp.val = atom_desc.rsp.val;
-	rte_bbdev_log_debug("Resp. desc %p: %x", desc, rsp.val);
+	rte_bbdev_log_debug("Resp. desc %p: %x num %d\n",
+			desc, rsp.val, desc->req.numCBs);
 
 	/* Dequeue */
 	op = desc->req.op_addr;
@@ -3842,27 +3871,32 @@ dequeue_enc_one_op_cb(struct acc_queue *q, struct rte_bbdev_enc_op **ref_op,
 	desc->rsp.add_info_0 = 0; /*Reserved bits */
 	desc->rsp.add_info_1 = 0; /*Reserved bits */
 
-	/* Flag that the muxing cause loss of opaque data */
-	op->opaque_data = (void *)-1;
-	for (i = 0 ; i < desc->req.numCBs; i++)
-		ref_op[i] = op;
+	ref_op[0] = op;
+	struct acc_ptrs *context_ptrs = q->companion_ring_addr + desc_idx;
+	for (i = 1 ; i < desc->req.numCBs; i++)
+		ref_op[i] = context_ptrs->ptr[i].op_addr;
 
-	/* One CB (op) was successfully dequeued */
+	/* One op was successfully dequeued */
+	(*dequeued_descs)++;
+	*dequeued_ops += desc->req.numCBs;
 	return desc->req.numCBs;
 }
 
-/* Dequeue one encode operations from ACC100 device in TB mode */
+/* Dequeue one LDPC encode operations from ACC100 device in TB mode
+ * That operation may cover multiple descriptors
+ */
 static inline int
 dequeue_enc_one_op_tb(struct acc_queue *q, struct rte_bbdev_enc_op **ref_op,
-		uint16_t total_dequeued_cbs, uint32_t *aq_dequeued)
+		uint16_t *dequeued_ops, uint32_t *aq_dequeued,
+		uint16_t *dequeued_descs)
 {
 	union acc_dma_desc *desc, *last_desc, atom_desc;
 	union acc_dma_rsp_desc rsp;
 	struct rte_bbdev_enc_op *op;
 	uint8_t i = 0;
-	uint16_t current_dequeued_cbs = 0, cbs_in_tb;
+	uint16_t current_dequeued_descs = 0, descs_in_tb;
 
-	desc = q->ring_addr + ((q->sw_ring_tail + total_dequeued_cbs)
+	desc = q->ring_addr + ((q->sw_ring_tail + *dequeued_descs)
 			& q->sw_ring_wrap_mask);
 	atom_desc.atom_hdr = __atomic_load_n((uint64_t *)desc,
 			__ATOMIC_RELAXED);
@@ -3872,10 +3906,10 @@ dequeue_enc_one_op_tb(struct acc_queue *q, struct rte_bbdev_enc_op **ref_op,
 		return -1;
 
 	/* Get number of CBs in dequeued TB */
-	cbs_in_tb = desc->req.cbs_in_tb;
+	descs_in_tb = desc->req.cbs_in_tb;
 	/* Get last CB */
 	last_desc = q->ring_addr + ((q->sw_ring_tail
-			+ total_dequeued_cbs + cbs_in_tb - 1)
+			+ *dequeued_descs + descs_in_tb - 1)
 			& q->sw_ring_wrap_mask);
 	/* Check if last CB in TB is ready to dequeue (and thus
 	 * the whole TB) - checking sdone bit. If not return.
@@ -3891,15 +3925,17 @@ dequeue_enc_one_op_tb(struct acc_queue *q, struct rte_bbdev_enc_op **ref_op,
 	/* Clearing status, it will be set based on response */
 	op->status = 0;
 
-	while (i < cbs_in_tb) {
+	while (i < descs_in_tb) {
 		desc = q->ring_addr + ((q->sw_ring_tail
-				+ total_dequeued_cbs)
+				+ *dequeued_descs)
 				& q->sw_ring_wrap_mask);
 		atom_desc.atom_hdr = __atomic_load_n((uint64_t *)desc,
 				__ATOMIC_RELAXED);
 		rsp.val = atom_desc.rsp.val;
-		rte_bbdev_log_debug("Resp. desc %p: %x", desc,
-				rsp.val);
+		rte_bbdev_log_debug("Resp. desc %p: %x descs %d cbs %d\n",
+				desc,
+				rsp.val, descs_in_tb,
+				desc->req.numCBs);
 
 		op->status |= ((rsp.input_err)
 				? (1 << RTE_BBDEV_DATA_ERROR) : 0);
@@ -3913,14 +3949,15 @@ dequeue_enc_one_op_tb(struct acc_queue *q, struct rte_bbdev_enc_op **ref_op,
 		desc->rsp.val = ACC_DMA_DESC_TYPE;
 		desc->rsp.add_info_0 = 0;
 		desc->rsp.add_info_1 = 0;
-		total_dequeued_cbs++;
-		current_dequeued_cbs++;
+		(*dequeued_descs)++;
+		current_dequeued_descs++;
 		i++;
 	}
 
 	*ref_op = op;
 
-	return current_dequeued_cbs;
+	(*dequeued_ops)++;
+	return current_dequeued_descs;
 }
 
 /* Dequeue one decode operation from ACC100 device in CB mode */
@@ -4118,12 +4155,11 @@ acc100_dequeue_enc(struct rte_bbdev_queue_data *q_data,
 		struct rte_bbdev_enc_op **ops, uint16_t num)
 {
 	struct acc_queue *q = q_data->queue_private;
-	uint16_t dequeue_num;
 	uint32_t avail = acc_ring_avail_deq(q);
 	uint32_t aq_dequeued = 0;
-	uint16_t i, dequeued_cbs = 0;
-	struct rte_bbdev_enc_op *op;
+	uint16_t i, dequeued_ops = 0, dequeued_descs = 0;
 	int ret;
+	struct rte_bbdev_enc_op *op;
 	if (avail == 0)
 		return 0;
 #ifdef RTE_LIBRTE_BBDEV_DEBUG
@@ -4132,31 +4168,35 @@ acc100_dequeue_enc(struct rte_bbdev_queue_data *q_data,
 		return 0;
 	}
 #endif
+	op = (q->ring_addr + (q->sw_ring_tail &
+			q->sw_ring_wrap_mask))->req.op_addr;
+	if (unlikely(ops == NULL || op == NULL))
+		return 0;
 
-	dequeue_num = (avail < num) ? avail : num;
+	int cbm = op->turbo_enc.code_block_mode;
 
-	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 (op->turbo_enc.code_block_mode == RTE_BBDEV_TRANSPORT_BLOCK)
-			ret = dequeue_enc_one_op_tb(q, &ops[i], dequeued_cbs,
-					&aq_dequeued);
+	for (i = 0; i < num; i++) {
+		if (cbm == RTE_BBDEV_TRANSPORT_BLOCK)
+			ret = dequeue_enc_one_op_tb(q, &ops[dequeued_ops],
+					&dequeued_ops, &aq_dequeued,
+					&dequeued_descs);
 		else
-			ret = dequeue_enc_one_op_cb(q, &ops[i], dequeued_cbs,
-					&aq_dequeued);
-
+			ret = dequeue_enc_one_op_cb(q, &ops[dequeued_ops],
+					&dequeued_ops, &aq_dequeued,
+					&dequeued_descs);
 		if (ret < 0)
 			break;
-		dequeued_cbs += ret;
+		if (dequeued_ops >= num)
+			break;
 	}
 
 	q->aq_dequeued += aq_dequeued;
-	q->sw_ring_tail += dequeued_cbs;
+	q->sw_ring_tail += dequeued_descs;
 
 	/* Update enqueue stats */
-	q_data->queue_stats.dequeued_count += i;
+	q_data->queue_stats.dequeued_count += dequeued_ops;
 
-	return i;
+	return dequeued_ops;
 }
 
 /* Dequeue LDPC encode operations from ACC100 device. */
@@ -4167,24 +4207,37 @@ acc100_dequeue_ldpc_enc(struct rte_bbdev_queue_data *q_data,
 	struct acc_queue *q = q_data->queue_private;
 	uint32_t avail = acc_ring_avail_deq(q);
 	uint32_t aq_dequeued = 0;
-	uint16_t dequeue_num, i, dequeued_cbs = 0, dequeued_descs = 0;
+	uint16_t i, dequeued_ops = 0, dequeued_descs = 0;
 	int ret;
+	struct rte_bbdev_enc_op *op;
+	union acc_dma_desc *desc;
 
+	if (q == NULL)
+		return 0;
 #ifdef RTE_LIBRTE_BBDEV_DEBUG
-	if (unlikely(ops == 0 && q == NULL))
+	if (unlikely(ops == 0))
 		return 0;
 #endif
+	desc = q->ring_addr + (q->sw_ring_tail & q->sw_ring_wrap_mask);
+	if (unlikely(desc == NULL))
+		return 0;
+	op = desc->req.op_addr;
+	if (unlikely(ops == NULL || op == NULL))
+		return 0;
+	int cbm = op->ldpc_enc.code_block_mode;
 
-	dequeue_num = RTE_MIN(avail, num);
-
-	for (i = 0; i < dequeue_num; i++) {
-		ret = dequeue_enc_one_op_cb(q, &ops[dequeued_cbs],
-				dequeued_descs, &aq_dequeued);
+	for (i = 0; i < avail; i++) {
+		if (cbm == RTE_BBDEV_TRANSPORT_BLOCK)
+			ret = dequeue_enc_one_op_tb(q, &ops[dequeued_ops],
+					&dequeued_ops, &aq_dequeued,
+					&dequeued_descs);
+		else
+			ret = dequeue_enc_one_op_cb(q, &ops[dequeued_ops],
+					&dequeued_ops, &aq_dequeued,
+					&dequeued_descs);
 		if (ret < 0)
 			break;
-		dequeued_cbs += ret;
-		dequeued_descs++;
-		if (dequeued_cbs >= num)
+		if (dequeued_ops >= num)
 			break;
 	}
 
@@ -4192,12 +4245,10 @@ acc100_dequeue_ldpc_enc(struct rte_bbdev_queue_data *q_data,
 	q->sw_ring_tail += dequeued_descs;
 
 	/* Update enqueue stats */
-	q_data->queue_stats.dequeued_count += dequeued_cbs;
-
-	return dequeued_cbs;
+	q_data->queue_stats.dequeued_count += dequeued_ops;
+	return dequeued_ops;
 }
 
-
 /* Dequeue decode operations from ACC100 device. */
 static uint16_t
 acc100_dequeue_dec(struct rte_bbdev_queue_data *q_data,
-- 
2.37.1

[PATCH v3 29/30] baseband/acc100: add workaround for deRM corner cases

[Hernan Vargas]

Add function to support de-ratematch pre-processing for SW corner cases.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
---
 drivers/baseband/acc/acc_common.h     |   8 ++
 drivers/baseband/acc/meson.build      |  21 +++++
 drivers/baseband/acc/rte_acc100_pmd.c | 108 +++++++++++++++++++++++++-
 3 files changed, 134 insertions(+), 3 deletions(-)

diff --git a/drivers/baseband/acc/acc_common.h b/drivers/baseband/acc/acc_common.h
index 5a9929c336..4b947280c9 100644
--- a/drivers/baseband/acc/acc_common.h
+++ b/drivers/baseband/acc/acc_common.h
@@ -122,6 +122,14 @@
 #define ACC_HARQ_ALIGN_64B          64
 #define ACC_MAX_ZC                  384
 
+/* De-ratematch code rate limitation when padding is required */
+#define ACC_LIM_03 2  /* 0.03 */
+#define ACC_LIM_09 6  /* 0.09 */
+#define ACC_LIM_14 9  /* 0.14 */
+#define ACC_LIM_21 14 /* 0.21 */
+#define ACC_LIM_31 20 /* 0.31 */
+#define ACC_MAX_E (128 * 1024 - 2)
+
 /* Helper macro for logging */
 #define rte_acc_log(level, fmt, ...) \
 	rte_log(RTE_LOG_ ## level, RTE_LOG_NOTICE, fmt "\n", \
diff --git a/drivers/baseband/acc/meson.build b/drivers/baseband/acc/meson.build
index bece3a6e48..b147569d7e 100644
--- a/drivers/baseband/acc/meson.build
+++ b/drivers/baseband/acc/meson.build
@@ -1,6 +1,27 @@
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2020 Intel Corporation
 
+# Check for FlexRAN SDK libraries
+dep_dec5g = dependency('flexran_sdk_ldpc_decoder_5gnr', required: false)
+
+if dep_dec5g.found()
+    ext_deps += cc.find_library('libstdc++', required: true)
+    ext_deps += cc.find_library('libirc', required: true)
+    ext_deps += cc.find_library('libimf', required: true)
+    ext_deps += cc.find_library('libipps', required: true)
+    ext_deps += cc.find_library('libsvml', required: true)
+    ext_deps += dep_dec5g
+    ext_deps += dependency('flexran_sdk_ldpc_encoder_5gnr', required: true)
+    ext_deps += dependency('flexran_sdk_LDPC_ratematch_5gnr', required: true)
+    ext_deps += dependency('flexran_sdk_rate_dematching_5gnr', required: true)
+    ext_deps += dependency('flexran_sdk_turbo', required: true)
+    ext_deps += dependency('flexran_sdk_crc', required: true)
+    ext_deps += dependency('flexran_sdk_rate_matching', required: true)
+    ext_deps += dependency('flexran_sdk_common', required: true)
+    cflags += ['-DRTE_BBDEV_SDK_AVX2']
+    cflags += ['-DRTE_BBDEV_SDK_AVX512']
+endif
+
 deps += ['bbdev', 'bus_pci']
 
 sources = files('rte_acc100_pmd.c', 'rte_acc200_pmd.c')
diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
index f93fd885a3..44fd0e9ad7 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -24,6 +24,10 @@
 #include "acc100_pmd.h"
 #include "acc101_pmd.h"
 
+#ifdef RTE_BBDEV_SDK_AVX512
+#include <phy_rate_dematching_5gnr.h>
+#endif
+
 #ifdef RTE_LIBRTE_BBDEV_DEBUG
 RTE_LOG_REGISTER_DEFAULT(acc100_logtype, DEBUG);
 #else
@@ -748,6 +752,14 @@ acc100_queue_setup(struct rte_bbdev *dev, uint16_t queue_id,
 		ret = -ENOMEM;
 		goto free_lb_out;
 	}
+	q->derm_buffer = rte_zmalloc_socket(dev->device->driver->name,
+			RTE_BBDEV_TURBO_MAX_CB_SIZE * 10,
+			RTE_CACHE_LINE_SIZE, conf->socket);
+	if (q->derm_buffer == NULL) {
+		rte_bbdev_log(ERR, "Failed to allocate derm_buffer memory");
+		ret = -ENOMEM;
+		goto free_companion_ring_addr;
+	}
 
 	/*
 	 * Software queue ring wraps synchronously with the HW when it reaches
@@ -768,7 +780,7 @@ acc100_queue_setup(struct rte_bbdev *dev, uint16_t queue_id,
 	q_idx = acc100_find_free_queue_idx(dev, conf);
 	if (q_idx == -1) {
 		ret = -EINVAL;
-		goto free_companion_ring_addr;
+		goto free_derm_buffer;
 	}
 
 	q->qgrp_id = (q_idx >> ACC100_GRP_ID_SHIFT) & 0xF;
@@ -796,6 +808,9 @@ acc100_queue_setup(struct rte_bbdev *dev, uint16_t queue_id,
 	dev->data->queues[queue_id].queue_private = q;
 	return 0;
 
+free_derm_buffer:
+	rte_free(q->derm_buffer);
+	q->derm_buffer = NULL;
 free_companion_ring_addr:
 	rte_free(q->companion_ring_addr);
 	q->companion_ring_addr = NULL;
@@ -882,6 +897,7 @@ acc100_queue_release(struct rte_bbdev *dev, uint16_t q_id)
 		/* Mark the Queue as un-assigned */
 		d->q_assigned_bit_map[q->qgrp_id] &= (0xFFFFFFFFFFFFFFFF -
 				(uint64_t) (1 << q->aq_id));
+		rte_free(q->derm_buffer);
 		rte_free(q->companion_ring_addr);
 		rte_free(q->lb_in);
 		rte_free(q->lb_out);
@@ -3102,10 +3118,44 @@ harq_loopback(struct acc_queue *q, struct rte_bbdev_dec_op *op,
 	return 1;
 }
 
+/** Assess whether a work around is required for the deRM corner cases */
+static inline bool
+derm_workaround_required(struct rte_bbdev_op_ldpc_dec *ldpc_dec, struct acc_queue *q)
+{
+	if (!is_acc100(q))
+		return false;
+	int32_t e = ldpc_dec->cb_params.e;
+	int q_m = ldpc_dec->q_m;
+	int z_c = ldpc_dec->z_c;
+	int K = (ldpc_dec->basegraph == 1 ? ACC_K_ZC_1 : ACC_K_ZC_2)
+			* z_c;
+
+	bool required = false;
+	if (ldpc_dec->basegraph == 1) {
+		if ((q_m == 4) && (z_c >= 320) && (e * ACC_LIM_31 > K * 64))
+			required = true;
+		else if ((e * ACC_LIM_21 > K * 64))
+			required = true;
+	} else {
+		if (q_m <= 2) {
+			if ((z_c >= 208) && (e * ACC_LIM_09 > K * 64))
+				required = true;
+			else if ((z_c < 208) && (e * ACC_LIM_03 > K * 64))
+				required = true;
+		} else if (e * ACC_LIM_14 > K * 64)
+			required = true;
+	}
+	if (required)
+		rte_bbdev_log(INFO, "Running deRM pre-processing in SW");
+
+	return required;
+}
+
 /** Enqueue one decode operations for ACC100 device in CB mode */
 static inline int
 enqueue_ldpc_dec_one_op_cb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
-		uint16_t total_enqueued_cbs, bool same_op)
+		uint16_t total_enqueued_cbs, bool same_op,
+		struct rte_bbdev_queue_data *q_data)
 {
 	int ret;
 	if (unlikely(check_bit(op->ldpc_dec.op_flags,
@@ -3163,6 +3213,58 @@ enqueue_ldpc_dec_one_op_cb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
 	} else {
 		struct acc_fcw_ld *fcw;
 		uint32_t seg_total_left;
+
+		if (derm_workaround_required(&op->ldpc_dec, q)) {
+			#ifdef RTE_BBDEV_SDK_AVX512
+			struct rte_bbdev_op_ldpc_dec *dec = &op->ldpc_dec;
+			struct bblib_rate_dematching_5gnr_request derm_req;
+			struct bblib_rate_dematching_5gnr_response derm_resp;
+			uint8_t *in;
+
+			/* Checking input size is matching with E */
+			if (dec->input.data->data_len < dec->cb_params.e) {
+				rte_bbdev_log(ERR, "deRM: Input size mismatch");
+				return -EFAULT;
+			}
+			/* Run first deRM processing in SW */
+			in = rte_pktmbuf_mtod_offset(dec->input.data, uint8_t *, in_offset);
+			derm_req.p_in = (int8_t *) in;
+			derm_req.p_harq = (int8_t *) q->derm_buffer;
+			derm_req.base_graph = dec->basegraph;
+			derm_req.zc = dec->z_c;
+			derm_req.ncb = dec->n_cb;
+			derm_req.e = dec->cb_params.e;
+			if (derm_req.e > ACC_MAX_E) {
+				rte_bbdev_log(WARNING,
+						"deRM: E %d > %d max",
+						derm_req.e, ACC_MAX_E);
+				derm_req.e = ACC_MAX_E;
+			}
+			derm_req.k0 = 0; /* Actual output from SDK */
+			derm_req.isretx = false;
+			derm_req.rvid = dec->rv_index;
+			derm_req.modulation_order = dec->q_m;
+			derm_req.start_null_index =
+					(dec->basegraph == 1 ? 22 : 10)
+					* dec->z_c - 2 * dec->z_c
+					- dec->n_filler;
+			derm_req.num_of_null = dec->n_filler;
+			bblib_rate_dematching_5gnr(&derm_req, &derm_resp);
+			/* Force back the HW DeRM */
+			dec->q_m = 1;
+			dec->cb_params.e = dec->n_cb - dec->n_filler;
+			dec->rv_index = 0;
+			rte_memcpy(in, q->derm_buffer, dec->cb_params.e);
+			/* Capture counter when pre-processing is used */
+			q_data->queue_stats.enqueue_warn_count++;
+			#else
+			RTE_SET_USED(q_data);
+			rte_bbdev_log(WARNING,
+				"Corner case may require deRM pre-processing in SDK"
+				);
+			#endif
+		}
+
 		fcw = &desc->req.fcw_ld;
 		q->d->fcw_ld_fill(op, fcw, harq_layout);
 
@@ -3734,7 +3836,7 @@ acc100_enqueue_ldpc_dec_cb(struct rte_bbdev_queue_data *q_data,
 			ops[i]->ldpc_dec.n_cb, ops[i]->ldpc_dec.q_m,
 			ops[i]->ldpc_dec.n_filler, ops[i]->ldpc_dec.cb_params.e,
 			same_op);
-		ret = enqueue_ldpc_dec_one_op_cb(q, ops[i], i, same_op);
+		ret = enqueue_ldpc_dec_one_op_cb(q, ops[i], i, same_op, q_data);
 		if (ret < 0) {
 			acc_enqueue_invalid(q_data);
 			break;
-- 
2.37.1

RE: [EXT] [PATCH v3 29/30] baseband/acc100: add workaround for deRM corner cases

[Akhil Goyal]

> Add function to support de-ratematch pre-processing for SW corner cases.
> 
Please add more meaningful description.
Title says adding workaround. You should explain the issue and then what is done in this patch.

> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>  drivers/baseband/acc/acc_common.h     |   8 ++
>  drivers/baseband/acc/meson.build      |  21 +++++
>  drivers/baseband/acc/rte_acc100_pmd.c | 108 +++++++++++++++++++++++++-
>  3 files changed, 134 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/baseband/acc/acc_common.h
> b/drivers/baseband/acc/acc_common.h
> index 5a9929c336..4b947280c9 100644
> --- a/drivers/baseband/acc/acc_common.h
> +++ b/drivers/baseband/acc/acc_common.h
> @@ -122,6 +122,14 @@
>  #define ACC_HARQ_ALIGN_64B          64
>  #define ACC_MAX_ZC                  384
> 
> +/* De-ratematch code rate limitation when padding is required */
> +#define ACC_LIM_03 2  /* 0.03 */
> +#define ACC_LIM_09 6  /* 0.09 */
> +#define ACC_LIM_14 9  /* 0.14 */
> +#define ACC_LIM_21 14 /* 0.21 */
> +#define ACC_LIM_31 20 /* 0.31 */
> +#define ACC_MAX_E (128 * 1024 - 2)
> +
>  /* Helper macro for logging */
>  #define rte_acc_log(level, fmt, ...) \
>  	rte_log(RTE_LOG_ ## level, RTE_LOG_NOTICE, fmt "\n", \
> diff --git a/drivers/baseband/acc/meson.build
> b/drivers/baseband/acc/meson.build
> index bece3a6e48..b147569d7e 100644
> --- a/drivers/baseband/acc/meson.build
> +++ b/drivers/baseband/acc/meson.build
> @@ -1,6 +1,27 @@
>  # SPDX-License-Identifier: BSD-3-Clause
>  # Copyright(c) 2020 Intel Corporation
> 
> +# Check for FlexRAN SDK libraries
> +dep_dec5g = dependency('flexran_sdk_ldpc_decoder_5gnr', required: false)
> +
> +if dep_dec5g.found()
> +    ext_deps += cc.find_library('libstdc++', required: true)
> +    ext_deps += cc.find_library('libirc', required: true)
> +    ext_deps += cc.find_library('libimf', required: true)
> +    ext_deps += cc.find_library('libipps', required: true)
> +    ext_deps += cc.find_library('libsvml', required: true)
> +    ext_deps += dep_dec5g
> +    ext_deps += dependency('flexran_sdk_ldpc_encoder_5gnr', required: true)
> +    ext_deps += dependency('flexran_sdk_LDPC_ratematch_5gnr', required:
> true)
> +    ext_deps += dependency('flexran_sdk_rate_dematching_5gnr', required:
> true)
> +    ext_deps += dependency('flexran_sdk_turbo', required: true)
> +    ext_deps += dependency('flexran_sdk_crc', required: true)
> +    ext_deps += dependency('flexran_sdk_rate_matching', required: true)
> +    ext_deps += dependency('flexran_sdk_common', required: true)
> +    cflags += ['-DRTE_BBDEV_SDK_AVX2']
> +    cflags += ['-DRTE_BBDEV_SDK_AVX512']
> +endif
> +
>  deps += ['bbdev', 'bus_pci']
> 
>  sources = files('rte_acc100_pmd.c', 'rte_acc200_pmd.c')
> diff --git a/drivers/baseband/acc/rte_acc100_pmd.c
> b/drivers/baseband/acc/rte_acc100_pmd.c
> index f93fd885a3..44fd0e9ad7 100644
> --- a/drivers/baseband/acc/rte_acc100_pmd.c
> +++ b/drivers/baseband/acc/rte_acc100_pmd.c
> @@ -24,6 +24,10 @@
>  #include "acc100_pmd.h"
>  #include "acc101_pmd.h"
> 
> +#ifdef RTE_BBDEV_SDK_AVX512
> +#include <phy_rate_dematching_5gnr.h>
> +#endif
> +
>  #ifdef RTE_LIBRTE_BBDEV_DEBUG
>  RTE_LOG_REGISTER_DEFAULT(acc100_logtype, DEBUG);
>  #else
> @@ -748,6 +752,14 @@ acc100_queue_setup(struct rte_bbdev *dev, uint16_t
> queue_id,
>  		ret = -ENOMEM;
>  		goto free_lb_out;
>  	}
> +	q->derm_buffer = rte_zmalloc_socket(dev->device->driver->name,
> +			RTE_BBDEV_TURBO_MAX_CB_SIZE * 10,
> +			RTE_CACHE_LINE_SIZE, conf->socket);
> +	if (q->derm_buffer == NULL) {
> +		rte_bbdev_log(ERR, "Failed to allocate derm_buffer memory");
> +		ret = -ENOMEM;
> +		goto free_companion_ring_addr;
> +	}
> 
>  	/*
>  	 * Software queue ring wraps synchronously with the HW when it
> reaches
> @@ -768,7 +780,7 @@ acc100_queue_setup(struct rte_bbdev *dev, uint16_t
> queue_id,
>  	q_idx = acc100_find_free_queue_idx(dev, conf);
>  	if (q_idx == -1) {
>  		ret = -EINVAL;
> -		goto free_companion_ring_addr;
> +		goto free_derm_buffer;
>  	}
> 
>  	q->qgrp_id = (q_idx >> ACC100_GRP_ID_SHIFT) & 0xF;
> @@ -796,6 +808,9 @@ acc100_queue_setup(struct rte_bbdev *dev, uint16_t
> queue_id,
>  	dev->data->queues[queue_id].queue_private = q;
>  	return 0;
> 
> +free_derm_buffer:
> +	rte_free(q->derm_buffer);
> +	q->derm_buffer = NULL;
>  free_companion_ring_addr:
>  	rte_free(q->companion_ring_addr);
>  	q->companion_ring_addr = NULL;
> @@ -882,6 +897,7 @@ acc100_queue_release(struct rte_bbdev *dev, uint16_t
> q_id)
>  		/* Mark the Queue as un-assigned */
>  		d->q_assigned_bit_map[q->qgrp_id] &= (0xFFFFFFFFFFFFFFFF -
>  				(uint64_t) (1 << q->aq_id));
> +		rte_free(q->derm_buffer);
>  		rte_free(q->companion_ring_addr);
>  		rte_free(q->lb_in);
>  		rte_free(q->lb_out);
> @@ -3102,10 +3118,44 @@ harq_loopback(struct acc_queue *q, struct
> rte_bbdev_dec_op *op,
>  	return 1;
>  }
> 
> +/** Assess whether a work around is required for the deRM corner cases */
> +static inline bool
> +derm_workaround_required(struct rte_bbdev_op_ldpc_dec *ldpc_dec, struct
> acc_queue *q)
> +{
> +	if (!is_acc100(q))
> +		return false;
> +	int32_t e = ldpc_dec->cb_params.e;
> +	int q_m = ldpc_dec->q_m;
> +	int z_c = ldpc_dec->z_c;
> +	int K = (ldpc_dec->basegraph == 1 ? ACC_K_ZC_1 : ACC_K_ZC_2)
> +			* z_c;
> +
> +	bool required = false;
> +	if (ldpc_dec->basegraph == 1) {
> +		if ((q_m == 4) && (z_c >= 320) && (e * ACC_LIM_31 > K * 64))
> +			required = true;
> +		else if ((e * ACC_LIM_21 > K * 64))
> +			required = true;
> +	} else {
> +		if (q_m <= 2) {
> +			if ((z_c >= 208) && (e * ACC_LIM_09 > K * 64))
> +				required = true;
> +			else if ((z_c < 208) && (e * ACC_LIM_03 > K * 64))
> +				required = true;
> +		} else if (e * ACC_LIM_14 > K * 64)
> +			required = true;
> +	}
> +	if (required)
> +		rte_bbdev_log(INFO, "Running deRM pre-processing in SW");
> +
> +	return required;
> +}
> +
>  /** Enqueue one decode operations for ACC100 device in CB mode */
>  static inline int
>  enqueue_ldpc_dec_one_op_cb(struct acc_queue *q, struct rte_bbdev_dec_op
> *op,
> -		uint16_t total_enqueued_cbs, bool same_op)
> +		uint16_t total_enqueued_cbs, bool same_op,
> +		struct rte_bbdev_queue_data *q_data)
>  {
>  	int ret;
>  	if (unlikely(check_bit(op->ldpc_dec.op_flags,
> @@ -3163,6 +3213,58 @@ enqueue_ldpc_dec_one_op_cb(struct acc_queue
> *q, struct rte_bbdev_dec_op *op,
>  	} else {
>  		struct acc_fcw_ld *fcw;
>  		uint32_t seg_total_left;
> +
> +		if (derm_workaround_required(&op->ldpc_dec, q)) {
> +			#ifdef RTE_BBDEV_SDK_AVX512
> +			struct rte_bbdev_op_ldpc_dec *dec = &op->ldpc_dec;
> +			struct bblib_rate_dematching_5gnr_request derm_req;
> +			struct bblib_rate_dematching_5gnr_response
> derm_resp;
> +			uint8_t *in;
> +
> +			/* Checking input size is matching with E */
> +			if (dec->input.data->data_len < dec->cb_params.e) {
> +				rte_bbdev_log(ERR, "deRM: Input size
> mismatch");
> +				return -EFAULT;
> +			}
> +			/* Run first deRM processing in SW */
> +			in = rte_pktmbuf_mtod_offset(dec->input.data, uint8_t
> *, in_offset);
> +			derm_req.p_in = (int8_t *) in;
> +			derm_req.p_harq = (int8_t *) q->derm_buffer;
> +			derm_req.base_graph = dec->basegraph;
> +			derm_req.zc = dec->z_c;
> +			derm_req.ncb = dec->n_cb;
> +			derm_req.e = dec->cb_params.e;
> +			if (derm_req.e > ACC_MAX_E) {
> +				rte_bbdev_log(WARNING,
> +						"deRM: E %d > %d max",
> +						derm_req.e, ACC_MAX_E);
> +				derm_req.e = ACC_MAX_E;
> +			}
> +			derm_req.k0 = 0; /* Actual output from SDK */
> +			derm_req.isretx = false;
> +			derm_req.rvid = dec->rv_index;
> +			derm_req.modulation_order = dec->q_m;
> +			derm_req.start_null_index =
> +					(dec->basegraph == 1 ? 22 : 10)
> +					* dec->z_c - 2 * dec->z_c
> +					- dec->n_filler;
> +			derm_req.num_of_null = dec->n_filler;
> +			bblib_rate_dematching_5gnr(&derm_req, &derm_resp);
> +			/* Force back the HW DeRM */
> +			dec->q_m = 1;
> +			dec->cb_params.e = dec->n_cb - dec->n_filler;
> +			dec->rv_index = 0;
> +			rte_memcpy(in, q->derm_buffer, dec->cb_params.e);
> +			/* Capture counter when pre-processing is used */
> +			q_data->queue_stats.enqueue_warn_count++;
> +			#else
> +			RTE_SET_USED(q_data);
> +			rte_bbdev_log(WARNING,
> +				"Corner case may require deRM pre-processing
> in SDK"
> +				);
> +			#endif
> +		}
> +
>  		fcw = &desc->req.fcw_ld;
>  		q->d->fcw_ld_fill(op, fcw, harq_layout);
> 
> @@ -3734,7 +3836,7 @@ acc100_enqueue_ldpc_dec_cb(struct
> rte_bbdev_queue_data *q_data,
>  			ops[i]->ldpc_dec.n_cb, ops[i]->ldpc_dec.q_m,
>  			ops[i]->ldpc_dec.n_filler, ops[i]-
> >ldpc_dec.cb_params.e,
>  			same_op);
> -		ret = enqueue_ldpc_dec_one_op_cb(q, ops[i], i, same_op);
> +		ret = enqueue_ldpc_dec_one_op_cb(q, ops[i], i, same_op,
> q_data);
>  		if (ret < 0) {
>  			acc_enqueue_invalid(q_data);
>  			break;
> --
> 2.37.1

[PATCH v3 30/30] baseband/acc100: configure PMON control registers

[Hernan Vargas]

Enable performance monitor control registers.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/baseband/acc/acc100_pmd.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/baseband/acc/acc100_pmd.h b/drivers/baseband/acc/acc100_pmd.h
index 28063b3db0..ca84ee792a 100644
--- a/drivers/baseband/acc/acc100_pmd.h
+++ b/drivers/baseband/acc/acc100_pmd.h
@@ -115,6 +115,8 @@ struct acc100_registry_addr {
 	unsigned int depth_log1_offset;
 	unsigned int qman_group_func;
 	unsigned int ddr_range;
+	unsigned int pmon_ctrl_a;
+	unsigned int pmon_ctrl_b;
 };
 
 /* Structure holding registry addresses for PF */
@@ -144,6 +146,8 @@ static const struct acc100_registry_addr pf_reg_addr = {
 	.depth_log1_offset = HWPfQmgrGrpDepthLog21Vf,
 	.qman_group_func = HWPfQmgrGrpFunction0,
 	.ddr_range = HWPfDmaVfDdrBaseRw,
+	.pmon_ctrl_a = HWVfPmACntrlRegVf,
+	.pmon_ctrl_b = HWVfPmBCntrlRegVf,
 };
 
 /* Structure holding registry addresses for VF */
-- 
2.37.1

RE: [EXT] [PATCH v3 00/30] baseband/acc100: changes for 22.11

[Akhil Goyal]

> Hernan Vargas (30):
>   baseband/acc100: fix ring availability calculation
>   baseband/acc100: add function to check AQ availability
>   baseband/acc100: memory leak fix
>   baseband/acc100: add LDPC encoder padding function
>   baseband/acc100: check turbo dec/enc input
>   baseband/acc100: check for unlikely operation vals
>   baseband/acc100: enforce additional check on FCW
>   baseband/acc100: allocate ring/queue mem when NULL
>   baseband/acc100: reduce input length for CRC24B
>   baseband/acc100: fix clearing PF IR outside handler
>   baseband/acc100: set device min alignment to 1
>   baseband/acc100: add protection for NULL HARQ input
>   baseband/acc100: reset pointer after rte_free
>   baseband/acc100: fix debug print for LDPC FCW
>   baseband/acc100: add enqueue status
>   baseband/acc100: add scatter-gather support
>   baseband/acc100: add HARQ index helper function
>   baseband/acc100: enable input validation by default
>   baseband/acc100: added LDPC transport block support
>   baseband/acc100: update validate LDPC enc/dec
>   baseband/acc100: implement configurable queue depth
>   baseband/acc100: add queue stop operation
>   baseband/acc100: update uplink CB input length
>   baseband/acc100: rename ldpc encode function arg
>   baseband/acc100: update log messages
>   baseband/acc100: store FCW from first CB descriptor
>   baseband/acc100: update device info
>   baseband/acc100: add ring companion address
>   baseband/acc100: add workaround for deRM corner cases
>   baseband/acc100: configure PMON control registers
> 
>  drivers/baseband/acc/acc100_pmd.h     |    5 +
>  drivers/baseband/acc/acc_common.h     |   10 +
>  drivers/baseband/acc/meson.build      |   21 +
>  drivers/baseband/acc/rte_acc100_pmd.c | 1197 ++++++++++++++++++++-----
>  4 files changed, 1010 insertions(+), 223 deletions(-)
> 
Please rebase over dpdk-next-crypto TOT

RE: [EXT] [PATCH v3 00/30] baseband/acc100: changes for 22.11

[Akhil Goyal]

> Hernan Vargas (30):
>   baseband/acc100: fix ring availability calculation
>   baseband/acc100: add function to check AQ availability
>   baseband/acc100: memory leak fix
>   baseband/acc100: add LDPC encoder padding function
>   baseband/acc100: check turbo dec/enc input
>   baseband/acc100: check for unlikely operation vals
>   baseband/acc100: enforce additional check on FCW
>   baseband/acc100: allocate ring/queue mem when NULL
>   baseband/acc100: reduce input length for CRC24B
>   baseband/acc100: fix clearing PF IR outside handler
>   baseband/acc100: set device min alignment to 1
>   baseband/acc100: add protection for NULL HARQ input
>   baseband/acc100: reset pointer after rte_free
>   baseband/acc100: fix debug print for LDPC FCW
>   baseband/acc100: add enqueue status
>   baseband/acc100: add scatter-gather support
>   baseband/acc100: add HARQ index helper function
>   baseband/acc100: enable input validation by default
>   baseband/acc100: added LDPC transport block support
>   baseband/acc100: update validate LDPC enc/dec
>   baseband/acc100: implement configurable queue depth
>   baseband/acc100: add queue stop operation
>   baseband/acc100: update uplink CB input length
>   baseband/acc100: rename ldpc encode function arg
>   baseband/acc100: update log messages
>   baseband/acc100: store FCW from first CB descriptor
>   baseband/acc100: update device info
>   baseband/acc100: add ring companion address
>   baseband/acc100: add workaround for deRM corner cases
>   baseband/acc100: configure PMON control registers
> 
>  drivers/baseband/acc/acc100_pmd.h     |    5 +
>  drivers/baseband/acc/acc_common.h     |   10 +
>  drivers/baseband/acc/meson.build      |   21 +
>  drivers/baseband/acc/rte_acc100_pmd.c | 1197 ++++++++++++++++++++-----
>  4 files changed, 1010 insertions(+), 223 deletions(-)
> 
Hi Hernan/Nicolas,

I see some ifdefs being used in the code and there is no documentation for them
On when and how to enable/disable them.
It would be much like a dead code which is not compiled at all,
if any of the build target does not enable them.

Is it possible to replace them with runtime devargs instead of compile time ifdefs?

RE: [EXT] [PATCH v3 00/30] baseband/acc100: changes for 22.11

[Chautru, Nicolas]

Hi Akhil, 

> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Thursday, October 13, 2022 6:02 AM
> To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org;
> trix@redhat.com; maxime.coquelin@redhat.com
> Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>
> Subject: RE: [EXT] [PATCH v3 00/30] baseband/acc100: changes for 22.11
> 
> > Hernan Vargas (30):
> >   baseband/acc100: fix ring availability calculation
> >   baseband/acc100: add function to check AQ availability
> >   baseband/acc100: memory leak fix
> >   baseband/acc100: add LDPC encoder padding function
> >   baseband/acc100: check turbo dec/enc input
> >   baseband/acc100: check for unlikely operation vals
> >   baseband/acc100: enforce additional check on FCW
> >   baseband/acc100: allocate ring/queue mem when NULL
> >   baseband/acc100: reduce input length for CRC24B
> >   baseband/acc100: fix clearing PF IR outside handler
> >   baseband/acc100: set device min alignment to 1
> >   baseband/acc100: add protection for NULL HARQ input
> >   baseband/acc100: reset pointer after rte_free
> >   baseband/acc100: fix debug print for LDPC FCW
> >   baseband/acc100: add enqueue status
> >   baseband/acc100: add scatter-gather support
> >   baseband/acc100: add HARQ index helper function
> >   baseband/acc100: enable input validation by default
> >   baseband/acc100: added LDPC transport block support
> >   baseband/acc100: update validate LDPC enc/dec
> >   baseband/acc100: implement configurable queue depth
> >   baseband/acc100: add queue stop operation
> >   baseband/acc100: update uplink CB input length
> >   baseband/acc100: rename ldpc encode function arg
> >   baseband/acc100: update log messages
> >   baseband/acc100: store FCW from first CB descriptor
> >   baseband/acc100: update device info
> >   baseband/acc100: add ring companion address
> >   baseband/acc100: add workaround for deRM corner cases
> >   baseband/acc100: configure PMON control registers
> >
> >  drivers/baseband/acc/acc100_pmd.h     |    5 +
> >  drivers/baseband/acc/acc_common.h     |   10 +
> >  drivers/baseband/acc/meson.build      |   21 +
> >  drivers/baseband/acc/rte_acc100_pmd.c | 1197
> > ++++++++++++++++++++-----
> >  4 files changed, 1010 insertions(+), 223 deletions(-)
> >
> Hi Hernan/Nicolas,
> 
> I see some ifdefs being used in the code and there is no documentation for
> them On when and how to enable/disable them.
> It would be much like a dead code which is not compiled at all, if any of the
> build target does not enable them.
> 
> Is it possible to replace them with runtime devargs instead of compile time
> ifdefs?

In term of documentation gap, that is a fair point: 
- I believe the build time variable ACC100_EXT_MEM can be valuable for very specific troubleshoot (not for production) to shortcut the DDR on the card, but good point to document in the acc100.rst. 
- The RTE_BBDEV_OFFLOAD_COST is used across PMDs for a while. That could also be added to each PMD .rst. 
- The #ifndef RTE_LIBRTE_BBDEV_SKIP_VALIDATE is to skip validation of user API which protect from negative scenario and hence save a few cycles by not recommended by default. Can be added to acc100.rst. 

In term of runtime devargs usage, can we decorrelate this from the series? If we introduce such dev args and expose them more explicitly we would need more updated validation in place, are you okay if we defer such change to next year?
If really this is a big concern for you for 22.11 (this is indeed not ideal), we could limit/strip the upstream of the series to the default build configuration only (hence no new #ifdef) but still these options can be genuinely valuable to users (until moved to devargs) so I would prefer to keep as is but with updated documentation in acc100.rst. 

What do you think?

Thanks
Nic

RE: [EXT] [PATCH v3 00/30] baseband/acc100: changes for 22.11

[Akhil Goyal]

> > > Hernan Vargas (30):
> > >   baseband/acc100: fix ring availability calculation
> > >   baseband/acc100: add function to check AQ availability
> > >   baseband/acc100: memory leak fix
> > >   baseband/acc100: add LDPC encoder padding function
> > >   baseband/acc100: check turbo dec/enc input
> > >   baseband/acc100: check for unlikely operation vals
> > >   baseband/acc100: enforce additional check on FCW
> > >   baseband/acc100: allocate ring/queue mem when NULL
> > >   baseband/acc100: reduce input length for CRC24B
> > >   baseband/acc100: fix clearing PF IR outside handler
> > >   baseband/acc100: set device min alignment to 1
> > >   baseband/acc100: add protection for NULL HARQ input
> > >   baseband/acc100: reset pointer after rte_free
> > >   baseband/acc100: fix debug print for LDPC FCW
> > >   baseband/acc100: add enqueue status
> > >   baseband/acc100: add scatter-gather support
> > >   baseband/acc100: add HARQ index helper function
> > >   baseband/acc100: enable input validation by default
> > >   baseband/acc100: added LDPC transport block support
> > >   baseband/acc100: update validate LDPC enc/dec
> > >   baseband/acc100: implement configurable queue depth
> > >   baseband/acc100: add queue stop operation
> > >   baseband/acc100: update uplink CB input length
> > >   baseband/acc100: rename ldpc encode function arg
> > >   baseband/acc100: update log messages
> > >   baseband/acc100: store FCW from first CB descriptor
> > >   baseband/acc100: update device info
> > >   baseband/acc100: add ring companion address
> > >   baseband/acc100: add workaround for deRM corner cases
> > >   baseband/acc100: configure PMON control registers
> > >
> > >  drivers/baseband/acc/acc100_pmd.h     |    5 +
> > >  drivers/baseband/acc/acc_common.h     |   10 +
> > >  drivers/baseband/acc/meson.build      |   21 +
> > >  drivers/baseband/acc/rte_acc100_pmd.c | 1197
> > > ++++++++++++++++++++-----
> > >  4 files changed, 1010 insertions(+), 223 deletions(-)
> > >
> > Hi Hernan/Nicolas,
> >
> > I see some ifdefs being used in the code and there is no documentation for
> > them On when and how to enable/disable them.
> > It would be much like a dead code which is not compiled at all, if any of the
> > build target does not enable them.
> >
> > Is it possible to replace them with runtime devargs instead of compile time
> > ifdefs?
> 
> In term of documentation gap, that is a fair point:
> - I believe the build time variable ACC100_EXT_MEM can be valuable for very
> specific troubleshoot (not for production) to shortcut the DDR on the card, but
> good point to document in the acc100.rst.
> - The RTE_BBDEV_OFFLOAD_COST is used across PMDs for a while. That could
> also be added to each PMD .rst.
> - The #ifndef RTE_LIBRTE_BBDEV_SKIP_VALIDATE is to skip validation of user
> API which protect from negative scenario and hence save a few cycles by not
> recommended by default. Can be added to acc100.rst.
> 
> In term of runtime devargs usage, can we decorrelate this from the series? If we
> introduce such dev args and expose them more explicitly we would need more
> updated validation in place, are you okay if we defer such change to next year?
> If really this is a big concern for you for 22.11 (this is indeed not ideal), we could
> limit/strip the upstream of the series to the default build configuration only
> (hence no new #ifdef) but still these options can be genuinely valuable to users
> (until moved to devargs) so I would prefer to keep as is but with updated
> documentation in acc100.rst.
> 
> What do you think?
I am ok to skip devargs for now. Can be added in next cycle.
Please update the documentation as needed for compile time flags.