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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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 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

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)

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));

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

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",

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);

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);

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.

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

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);

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;

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

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);