Skip to contentSkip to conversation

acc baseband PMD fix and updates for 24.11

PATCH Superseded Hernan Vargas 2 participants 13 messages Aug 30, 2024 – Sep 13, 2024

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

cover acc baseband PMD fix and updates for 24.11

This series targets 24.11.
It includes a memory access fix, refactoring of queue allocation and general improvements.

Hernan Vargas (10):
baseband/acc: fix access to deallocated mem
baseband/acc: queue allocation refactor
baseband/acc: configure max queues per device
baseband/acc: future proof structure comparison
baseband/acc: enhance SW ring alignment
baseband/acc: remove soft output bypass
baseband/acc: algorithm tuning for LDPC decoder
baseband/acc: remove check on HARQ memory
baseband/acc: reset ring data valid bit
baseband/acc: cosmetic changes

 drivers/baseband/acc/acc_common.h     |  16 +-
 drivers/baseband/acc/rte_acc100_pmd.c |  39 +--
 drivers/baseband/acc/rte_vrb_pmd.c    | 357 ++++++++++++++++----------
 3 files changed, 230 insertions(+), 182 deletions(-)
Signature · 1 line
2.37.1
#1 lore raw

PATCH 1/10 baseband/acc: fix access to deallocated mem

Prevent op_addr access during queue_stop operation, as this memory may have been deallocated.

Fixes e640f6cdfa84 ("baseband/acc200: add LDPC processing")
---
 drivers/baseband/acc/rte_acc100_pmd.c | 36 ----------------------
 drivers/baseband/acc/rte_vrb_pmd.c    | 44 +--------------------------
 2 files changed, 1 insertion(+), 79 deletions(-)
drivers/baseband/acc/rte_acc100_pmd.c +0 -36
index ccf2003c3da1..3501af2710fd 100644
@@ -837,51 +837,15 @@ acc100_queue_setup(struct rte_bbdev *dev, uint16_t queue_id,
 	return ret;
 }
 
-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;
drivers/baseband/acc/rte_vrb_pmd.c +1 -43

59 lines not on this page · raw message

Signature · 1 line
2.37.1
Reviewed-by on v1 1/10
#2 lore raw

Re: [PATCH v1 01/10] baseband/acc: fix access to deallocated mem

Quoted 12 lines · Hernan Vargas

Prevent op_addr access during queue_stop operation, as this memory may have been deallocated.

Fixes e640f6cdfa84 ("baseband/acc200: add LDPC processing")
---
  drivers/baseband/acc/rte_acc100_pmd.c | 36 ----------------------
  drivers/baseband/acc/rte_vrb_pmd.c    | 44 +--------------------------
  2 files changed, 1 insertion(+), 79 deletions(-)

Thanks,
Maxime

#3 lore raw

PATCH 2/10 baseband/acc: queue allocation refactor

In reply to Hernan Vargas

Refactor to manage queue memory per operation more flexibly for VRB devices.

---
 drivers/baseband/acc/acc_common.h  |   5 +
 drivers/baseband/acc/rte_vrb_pmd.c | 214 ++++++++++++++++++++---------
 2 files changed, 157 insertions(+), 62 deletions(-)
drivers/baseband/acc/acc_common.h +5 -0
index cd88ba4a6d8f..0d1357ace61f 100644
@@ -149,6 +149,8 @@
 #define VRB2_VF_ID_SHIFT     6
 
 #define ACC_MAX_FFT_WIN      16
+#define ACC_MAX_RING_BUFFER  64
+#define VRB2_MAX_Q_PER_OP 256
 #define ACC_MAX_LOGLEN    256
 #define ACC_MAX_BUFFERLEN 256
 
@@ -583,6 +585,9 @@ struct acc_device {
 	void *sw_rings_base;  /* Base addr of un-aligned memory for sw rings */
 	void *sw_rings;  /* 64MBs of 64MB aligned memory for sw rings */
 	rte_iova_t sw_rings_iova;  /* IOVA address of sw_rings */
+	void *sw_rings_array[ACC_MAX_RING_BUFFER];  /* Array of aligned memory for sw rings. */
+	rte_iova_t sw_rings_iova_array[ACC_MAX_RING_BUFFER];  /* Array of sw_rings IOVA. */
+	uint32_t queue_index[ACC_MAX_RING_BUFFER]; /* Tracking queue index per ring buffer. */
 	/* Virtual address of the info memory routed to the this function under
 	 * operation, whether it is PF or VF.
 	 * HW may DMA information data at this location asynchronously
drivers/baseband/acc/rte_vrb_pmd.c +152 -62

300 lines not on this page · raw message

Signature · 1 line
2.37.1
#4 lore raw

PATCH 3/10 baseband/acc: configure max queues per device

In reply to Hernan Vargas

Configure max_queues based on the number of queue groups and numbers of AQS per device variant.

---
 drivers/baseband/acc/rte_vrb_pmd.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
drivers/baseband/acc/rte_vrb_pmd.c +7 -2
index 637578f0ddd6..9beb05cd95cf 100644
@@ -544,7 +544,7 @@ vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
 	uint32_t phys_low, phys_high, value;
 	struct acc_device *d = dev->data->dev_private;
 	uint16_t queues_per_op, i;
-	int ret;
+	int ret, max_queues = 0;
 
 	if (d->pf_device && !d->acc_conf.pf_mode_en) {
 		rte_bbdev_log(NOTICE,
@@ -671,10 +671,15 @@ vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
 	value = log2_basic(d->sw_ring_size / ACC_RING_SIZE_GRANULARITY);
 	acc_reg_write(d, d->reg_addr->ring_size, value);
 
+	if (d->device_variant == VRB1_VARIANT)
+		max_queues = VRB1_NUM_QGRPS * VRB1_NUM_AQS;
+	else if (d->device_variant == VRB2_VARIANT)
+		max_queues = VRB2_NUM_QGRPS * VRB2_NUM_AQS;
+
 	/* Configure tail pointer for use when SDONE enabled. */
 	if (d->tail_ptrs == NULL)
 		d->tail_ptrs = rte_zmalloc_socket(dev->device->driver->name,
-				VRB_MAX_QGRPS * VRB_MAX_AQS * sizeof(uint32_t),
+				max_queues * sizeof(uint32_t),
 				RTE_CACHE_LINE_SIZE, socket_id);
 	if (d->tail_ptrs == NULL) {
 		rte_bbdev_log(ERR, "Failed to allocate tail ptr for %s:%u",
Signature · 1 line
2.37.1
#5 lore raw

PATCH 4/10 baseband/acc: future proof structure comparison

In reply to Hernan Vargas

Some implementation in the PMD is based on some size assumption from the bbdev structure, which should use sizeof instead to be more future proof in case these structures change.

---
 drivers/baseband/acc/acc_common.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/baseband/acc/acc_common.h +2 -2
index 0d1357ace61f..5c5cccf4c9c6 100644
@@ -95,8 +95,8 @@
 #define ACC_COMPANION_PTRS             8
 #define ACC_FCW_VER                    2
 #define ACC_MUX_5GDL_DESC              6
-#define ACC_CMP_ENC_SIZE               20
-#define ACC_CMP_DEC_SIZE               24
+#define ACC_CMP_ENC_SIZE               (sizeof(struct rte_bbdev_op_ldpc_enc) - ACC_ENC_OFFSET)
+#define ACC_CMP_DEC_SIZE               (sizeof(struct rte_bbdev_op_ldpc_dec) - ACC_DEC_OFFSET)
 #define ACC_ENC_OFFSET                (32)
 #define ACC_DEC_OFFSET                (80)
 #define ACC_LIMIT_DL_MUX_BITS          534
Signature · 1 line
2.37.1
#6 lore raw

PATCH 5/10 baseband/acc: enhance SW ring alignment

In reply to Hernan Vargas

Calculate the aligned total size required for queue rings, ensuring that the size is a power of two for proper memory allocation.

---
 drivers/baseband/acc/acc_common.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
drivers/baseband/acc/acc_common.h +4 -3
index 5c5cccf4c9c6..49fdf495613c 100644
@@ -804,19 +804,20 @@ alloc_sw_rings_min_mem(struct rte_bbdev *dev, struct acc_device *d,
 	int i = 0;
 	uint32_t q_sw_ring_size = ACC_MAX_QUEUE_DEPTH * get_desc_len();
 	uint32_t dev_sw_ring_size = q_sw_ring_size * num_queues;
-	/* Free first in case this is a reconfiguration */
+	uint32_t alignment = q_sw_ring_size * rte_align32pow2(num_queues);
+	/* Free first in case this is dev_sw_ring_size, q_sw_ring_size, socket); reconfiguration */
 	rte_free(d->sw_rings_base);
 
 	/* Find an aligned block of memory to store sw rings */
 	while (i < ACC_SW_RING_MEM_ALLOC_ATTEMPTS) {
 		/*
 		 * sw_ring allocated memory is guaranteed to be aligned to
-		 * q_sw_ring_size at the condition that the requested size is
+		 * alignment at the condition that the requested size is
 		 * less than the page size
 		 */
 		sw_rings_base = rte_zmalloc_socket(
 				dev->device->driver->name,
-				dev_sw_ring_size, q_sw_ring_size, socket);
+				dev_sw_ring_size, alignment, socket);
 
 		if (sw_rings_base == NULL) {
 			rte_acc_log(ERR,
Signature · 1 line
2.37.1
#7 lore raw

PATCH 6/10 baseband/acc: remove soft output bypass

In reply to Hernan Vargas

Removing soft output bypass capability due to device limitations.

---
 drivers/baseband/acc/rte_vrb_pmd.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
drivers/baseband/acc/rte_vrb_pmd.c +3 -4
index 9beb05cd95cf..654b4d902732 100644
@@ -1363,7 +1363,6 @@ vrb_dev_info_get(struct rte_bbdev *dev, struct rte_bbdev_driver_info *dev_info)
 				RTE_BBDEV_LDPC_HARQ_4BIT_COMPRESSION |
 				RTE_BBDEV_LDPC_LLR_COMPRESSION |
 				RTE_BBDEV_LDPC_SOFT_OUT_ENABLE |
-				RTE_BBDEV_LDPC_SOFT_OUT_RM_BYPASS |
 				RTE_BBDEV_LDPC_SOFT_OUT_DEINTERLEAVER_BYPASS |
 				RTE_BBDEV_LDPC_DEC_INTERRUPTS,
 			.llr_size = 8,
@@ -1752,18 +1751,18 @@ vrb_fcw_ld_fill(struct rte_bbdev_dec_op *op, struct acc_fcw_ld *fcw,
 		fcw->so_en = check_bit(op->ldpc_dec.op_flags, RTE_BBDEV_LDPC_SOFT_OUT_ENABLE);
 		fcw->so_bypass_intlv = check_bit(op->ldpc_dec.op_flags,
 				RTE_BBDEV_LDPC_SOFT_OUT_DEINTERLEAVER_BYPASS);
-		fcw->so_bypass_rm = check_bit(op->ldpc_dec.op_flags,
-				RTE_BBDEV_LDPC_SOFT_OUT_RM_BYPASS);
+		fcw->so_bypass_rm = 0;
 		fcw->minsum_offset = 1;
 		fcw->dec_llrclip   = 2;
 	}
 
 	/*
-	 * These are all implicitly set
+	 * These are all implicitly set:
 	 * fcw->synd_post = 0;
 	 * fcw->dec_convllr = 0;
 	 * fcw->hcout_convllr = 0;
 	 * fcw->hcout_size1 = 0;
+	 * fcw->so_it = 0;
 	 * fcw->hcout_offset = 0;
 	 * fcw->negstop_th = 0;
 	 * fcw->negstop_it = 0;
Signature · 1 line
2.37.1
#8 lore raw

PATCH 7/10 baseband/acc: algorithm tuning for LDPC decoder

In reply to Hernan Vargas

Reverting to MS1 version of the algorithm to improve MU1 fading conditions.

---
 drivers/baseband/acc/rte_vrb_pmd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/baseband/acc/rte_vrb_pmd.c +3 -3
index 654b4d902732..858de1eada6e 100644
@@ -1366,7 +1366,7 @@ vrb_dev_info_get(struct rte_bbdev *dev, struct rte_bbdev_driver_info *dev_info)
 				RTE_BBDEV_LDPC_SOFT_OUT_DEINTERLEAVER_BYPASS |
 				RTE_BBDEV_LDPC_DEC_INTERRUPTS,
 			.llr_size = 8,
-			.llr_decimals = 2,
+			.llr_decimals = 1,
 			.num_buffers_src =
 					RTE_BBDEV_LDPC_MAX_CODE_BLOCKS,
 			.num_buffers_hard_out =
@@ -1752,8 +1752,8 @@ vrb_fcw_ld_fill(struct rte_bbdev_dec_op *op, struct acc_fcw_ld *fcw,
 		fcw->so_bypass_intlv = check_bit(op->ldpc_dec.op_flags,
 				RTE_BBDEV_LDPC_SOFT_OUT_DEINTERLEAVER_BYPASS);
 		fcw->so_bypass_rm = 0;
-		fcw->minsum_offset = 1;
-		fcw->dec_llrclip   = 2;
+		fcw->minsum_offset = 0;
+		fcw->dec_llrclip   = 0;
 	}
 
 	/*
Signature · 1 line
2.37.1
#9 lore raw

PATCH 8/10 baseband/acc: remove check on HARQ memory

In reply to Hernan Vargas

Automatically reset HARQ memory to prevent errors and simplify usage. In a way we can assume that the HARQ output operation will always overwrite the buffer, so we can reset this from the driver to prevent an error being reported when application fails to do this explicitly.

---
 drivers/baseband/acc/rte_vrb_pmd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/baseband/acc/rte_vrb_pmd.c +2 -1
index 858de1eada6e..581779160569 100644
@@ -2612,8 +2612,9 @@ vrb_enqueue_ldpc_dec_one_op_cb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
 	/* Hard output. */
 	mbuf_append(h_output_head, h_output, h_out_length);
 	if (op->ldpc_dec.harq_combined_output.length > 0) {
-		/* Push the HARQ output into host memory. */
+		/* Push the HARQ output into host memory overwriting existing data. */
 		struct rte_mbuf *hq_output_head, *hq_output;
+		op->ldpc_dec.harq_combined_output.data->data_len = 0;
 		hq_output_head = op->ldpc_dec.harq_combined_output.data;
 		hq_output = op->ldpc_dec.harq_combined_output.data;
 		hq_len = op->ldpc_dec.harq_combined_output.length;
Signature · 1 line
2.37.1
#10 lore raw

PATCH 9/10 baseband/acc: reset ring data valid bit

In reply to Hernan Vargas

Reset only the valid bit to keep info ring data notably for dumping.

---
 drivers/baseband/acc/rte_vrb_pmd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/baseband/acc/rte_vrb_pmd.c +1 -1
index 581779160569..6353a27e69b2 100644
@@ -411,7 +411,7 @@ vrb_check_ir(struct acc_device *acc_dev)
 			rte_bbdev_log(WARNING, "InfoRing: ITR:%d Info:0x%x",
 					int_nb, ring_data->detailed_info);
 			/* Initialize Info Ring entry and move forward. */
-			ring_data->val = 0;
+			ring_data->valid = 0;
 		}
 		info_ring_head++;
 		ring_data = acc_dev->info_ring + (info_ring_head & ACC_INFO_RING_MASK);
Signature · 1 line
2.37.1
#11 lore raw

PATCH 10/10 baseband/acc: cosmetic changes

In reply to Hernan Vargas

Cosmetic code changes.
No functional impact.

---
 drivers/baseband/acc/rte_acc100_pmd.c |  3 +-
 drivers/baseband/acc/rte_vrb_pmd.c    | 72 ++++++++++++++++++---------
 2 files changed, 50 insertions(+), 25 deletions(-)
drivers/baseband/acc/rte_acc100_pmd.c +2 -1
index 3501af2710fd..c33e2758b100 100644
@@ -995,6 +995,7 @@ acc100_dev_info_get(struct rte_bbdev *dev,
 	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->num_queues[RTE_BBDEV_OP_MLDTS] = 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;
@@ -4198,7 +4199,7 @@ poweron_cleanup(struct rte_bbdev *bbdev, struct acc_device *d,
 		acc_reg_write(d, HWPfQmgrIngressAq + 0x100, enq_req.val);
 		usleep(ACC_LONG_WAIT * 100);
 		if (desc->req.word0 != 2)
-			rte_bbdev_log(WARNING, "DMA Response %#"PRIx32, desc->req.word0);
+			rte_bbdev_log(WARNING, "DMA Response %#"PRIx32"", desc->req.word0);
 	}
 
 	/* Reset LDPC Cores */
drivers/baseband/acc/rte_vrb_pmd.c +48 -24

181 lines not on this page · raw message

Signature · 1 line
2.37.1
#12 lore raw
In reply to Hernan Vargas

This series is dependent on:
https://patches.dpdk.org/project/dpdk/list/?series=32868
https://patches.dpdk.org/project/dpdk/list/?series=32855
https://patches.dpdk.org/project/dpdk/list/?series=32765
https://patches.dpdk.org/project/dpdk/list/?series=32344

Thanks,
Hernan
________________________________
From: Vargas, Hernan <[email protected]>
Sent: Friday, August 30, 2024 3:09 PM
To: [email protected] <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>
Cc: Chautru, Nicolas <[email protected]>; Zhang, Qi Z <[email protected]>; Vargas, Hernan <[email protected]>
Subject: [PATCH v1 00/10] acc baseband PMD fix and updates for 24.11

This series targets 24.11.
It includes a memory access fix, refactoring of queue allocation and general improvements.

Hernan Vargas (10):
baseband/acc: fix access to deallocated mem
baseband/acc: queue allocation refactor
baseband/acc: configure max queues per device
baseband/acc: future proof structure comparison
baseband/acc: enhance SW ring alignment
baseband/acc: remove soft output bypass
baseband/acc: algorithm tuning for LDPC decoder
baseband/acc: remove check on HARQ memory
baseband/acc: reset ring data valid bit
baseband/acc: cosmetic changes

 drivers/baseband/acc/acc_common.h     |  16 +-
 drivers/baseband/acc/rte_acc100_pmd.c |  39 +--
 drivers/baseband/acc/rte_vrb_pmd.c    | 357 ++++++++++++++++----------
 3 files changed, 230 insertions(+), 182 deletions(-)

--
2.37.1