[PATCH v2 0/2] bbdev: dump debug information

[PATCH v2 0/2] bbdev: dump debug information

[Nicolas Chautru]

v2: updated with comments from Hemant and rebased.


v1: Hi Maxime. 

Adding new support to support troubleshooting. This provides
to the application an API to dump into file information
to help troubleshoot issue on a queue. Some of it is tracked
at bbdev level and some extra information can be tracked
as an option at PMD level. This is for 24.11. 
In practive logging is not enabled at run time, but information
can be dump into file when an issue happens and requires extra
information. This collates now multiple source of errors detected
notably at driver level made more explicit.

Thanks,
Nic


Nicolas Chautru (2):
  bbdev: add new function to dump debug information
  baseband/acc: improvement to logging mechanism

 drivers/baseband/acc/acc_common.h  |  37 +++++
 drivers/baseband/acc/rte_vrb_pmd.c | 128 +++++++++++++----
 lib/bbdev/rte_bbdev.c              | 214 +++++++++++++++++++++++++++++
 lib/bbdev/rte_bbdev.h              |  41 ++++++
 lib/bbdev/rte_bbdev_pmd.h          |   9 ++
 lib/bbdev/version.map              |   4 +
 6 files changed, 406 insertions(+), 27 deletions(-)

-- 
2.34.1

[PATCH v2 1/2] bbdev: add new function to dump debug information

[Nicolas Chautru]

This provides a new API to dump more debug information
related to the status on a given bbdev queue.
Some of this information is visible at bbdev level.
This also provides a new option dev op, to print more
information at the lower PMD level.
This helps user to troubleshoot issues related to
previous operations provided into a queue causing
possible hard-to-debug negative scenarios.

Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
---
 lib/bbdev/rte_bbdev.c     | 214 ++++++++++++++++++++++++++++++++++++++
 lib/bbdev/rte_bbdev.h     |  41 ++++++++
 lib/bbdev/rte_bbdev_pmd.h |   9 ++
 lib/bbdev/version.map     |   4 +
 4 files changed, 268 insertions(+)

diff --git a/lib/bbdev/rte_bbdev.c b/lib/bbdev/rte_bbdev.c
index 13bde3c25b..9087862c76 100644
--- a/lib/bbdev/rte_bbdev.c
+++ b/lib/bbdev/rte_bbdev.c
@@ -1190,3 +1190,217 @@ rte_bbdev_enqueue_status_str(enum rte_bbdev_enqueue_status status)
 	rte_bbdev_log(ERR, "Invalid enqueue status");
 	return NULL;
 }
+
+
+int
+rte_bbdev_queue_ops_dump(uint16_t dev_id, uint16_t queue_id, FILE *f)
+{
+	struct rte_bbdev_queue_data *q_data;
+	struct rte_bbdev_stats *stats;
+	uint16_t i;
+	struct rte_bbdev *dev = get_dev(dev_id);
+
+	VALID_DEV_OR_RET_ERR(dev, dev_id);
+	VALID_QUEUE_OR_RET_ERR(queue_id, dev);
+	VALID_DEV_OPS_OR_RET_ERR(dev, dev_id);
+	VALID_FUNC_OR_RET_ERR(dev->dev_ops->queue_ops_dump, dev_id);
+
+	q_data = &dev->data->queues[queue_id];
+
+	if (f == NULL)
+		return -EINVAL;
+
+	fprintf(f, "Dump of operations on %s queue %d\n",
+			dev->data->name, queue_id);
+	fprintf(f, "  Last Enqueue Status %s\n",
+			rte_bbdev_enqueue_status_str(q_data->enqueue_status));
+	for (i = 0; i < RTE_BBDEV_ENQ_STATUS_SIZE_MAX; i++)
+		if (q_data->queue_stats.enqueue_status_count[i] > 0)
+			fprintf(f, "  Enqueue Status Counters %s %" PRIu64 "\n",
+					rte_bbdev_enqueue_status_str(i),
+					q_data->queue_stats.enqueue_status_count[i]);
+	stats = &dev->data->queues[queue_id].queue_stats;
+
+	fprintf(f, "  Enqueue Count %" PRIu64 " Warning %" PRIu64 " Error %" PRIu64 "\n",
+			stats->enqueued_count, stats->enqueue_warn_count,
+			stats->enqueue_err_count);
+	fprintf(f, "  Dequeue Count %" PRIu64 " Warning %" PRIu64 " Error %" PRIu64 "\n",
+			stats->dequeued_count, stats->dequeue_warn_count,
+			stats->dequeue_err_count);
+
+	return dev->dev_ops->queue_ops_dump(dev, queue_id, f);
+}
+
+char *
+rte_bbdev_ops_param_string(void *op, enum rte_bbdev_op_type op_type)
+{
+	static char str[1024];
+	static char partial[1024];
+	struct rte_bbdev_dec_op *op_dec;
+	struct rte_bbdev_enc_op *op_enc;
+	struct rte_bbdev_fft_op *op_fft;
+	struct rte_bbdev_mldts_op *op_mldts;
+
+	rte_iova_t add0 = 0, add1 = 0, add2 = 0, add3 = 0, add4 = 0;
+
+	if (op == NULL) {
+		snprintf(str, sizeof(str), "Invalid Operation pointer\n");
+		return str;
+	}
+
+	if (op_type == RTE_BBDEV_OP_LDPC_DEC) {
+		op_dec = op;
+		if (op_dec->ldpc_dec.code_block_mode == RTE_BBDEV_TRANSPORT_BLOCK)
+			snprintf(partial, sizeof(partial), "C %d Cab %d Ea %d Eb %d r %d",
+					op_dec->ldpc_dec.tb_params.c,
+					op_dec->ldpc_dec.tb_params.cab,
+					op_dec->ldpc_dec.tb_params.ea,
+					op_dec->ldpc_dec.tb_params.eb,
+					op_dec->ldpc_dec.tb_params.r);
+		else
+			snprintf(partial, sizeof(partial), "E %d", op_dec->ldpc_dec.cb_params.e);
+		if (op_dec->ldpc_dec.input.data != NULL)
+			add0 = rte_pktmbuf_iova_offset(op_dec->ldpc_dec.input.data, 0);
+		if (op_dec->ldpc_dec.hard_output.data != NULL)
+			add1 = rte_pktmbuf_iova_offset(op_dec->ldpc_dec.hard_output.data, 0);
+		if (op_dec->ldpc_dec.soft_output.data != NULL)
+			add2 = rte_pktmbuf_iova_offset(op_dec->ldpc_dec.soft_output.data, 0);
+		if (op_dec->ldpc_dec.harq_combined_input.data != NULL)
+			add3 = rte_pktmbuf_iova_offset(op_dec->ldpc_dec.harq_combined_input.data,
+					0);
+		if (op_dec->ldpc_dec.harq_combined_output.data != NULL)
+			add4 = rte_pktmbuf_iova_offset(op_dec->ldpc_dec.harq_combined_output.data,
+					0);
+		snprintf(str, sizeof(str), "op %x st %x BG %d Zc %d Ncb %d qm %d F %d Rv %d It %d It %d "
+			"HARQin %d in %" PRIx64 " ho %" PRIx64 " so %" PRIx64 " hi %" PRIx64 " "
+			"ho %" PRIx64 " %s\n",
+			op_dec->ldpc_dec.op_flags, op_dec->status,
+			op_dec->ldpc_dec.basegraph, op_dec->ldpc_dec.z_c,
+			op_dec->ldpc_dec.n_cb, op_dec->ldpc_dec.q_m,
+			op_dec->ldpc_dec.n_filler, op_dec->ldpc_dec.rv_index,
+			op_dec->ldpc_dec.iter_max, op_dec->ldpc_dec.iter_count,
+			op_dec->ldpc_dec.harq_combined_input.length,
+			add0, add1, add2, add3, add4, partial);
+	} else if (op_type == RTE_BBDEV_OP_TURBO_DEC) {
+		op_dec = op;
+		if (op_dec->turbo_dec.code_block_mode == RTE_BBDEV_TRANSPORT_BLOCK)
+			snprintf(partial, sizeof(partial), "C %d Cab %d Ea %d Eb %d r %d K %d",
+					op_dec->turbo_dec.tb_params.c,
+					op_dec->turbo_dec.tb_params.cab,
+					op_dec->turbo_dec.tb_params.ea,
+					op_dec->turbo_dec.tb_params.eb,
+					op_dec->turbo_dec.tb_params.r,
+					op_dec->turbo_dec.tb_params.k_neg);
+		else
+			snprintf(partial, sizeof(partial), "E %d K %d",
+					op_dec->turbo_dec.cb_params.e,
+					op_dec->turbo_dec.cb_params.k);
+		if (op_dec->turbo_dec.input.data != NULL)
+			add0 = rte_pktmbuf_iova_offset(op_dec->turbo_dec.input.data, 0);
+		if (op_dec->turbo_dec.hard_output.data != NULL)
+			add1 = rte_pktmbuf_iova_offset(op_dec->turbo_dec.hard_output.data, 0);
+		if (op_dec->turbo_dec.soft_output.data != NULL)
+			add2 = rte_pktmbuf_iova_offset(op_dec->turbo_dec.soft_output.data, 0);
+		snprintf(str, sizeof(str), "op %x st %x CBM %d Iter %d map %d Rv %d ext %d "
+				"in %" PRIx64 " ho %" PRIx64 " so %" PRIx64 " %s\n",
+				op_dec->turbo_dec.op_flags, op_dec->status,
+				op_dec->turbo_dec.code_block_mode,
+				op_dec->turbo_dec.iter_max, op_dec->turbo_dec.num_maps,
+				op_dec->turbo_dec.rv_index, op_dec->turbo_dec.ext_scale,
+				add0, add1, add2, partial);
+	} else if (op_type == RTE_BBDEV_OP_LDPC_ENC) {
+		op_enc = op;
+		if (op_enc->ldpc_enc.code_block_mode == RTE_BBDEV_TRANSPORT_BLOCK)
+			snprintf(partial, sizeof(partial), "C %d Cab %d Ea %d Eb %d r %d",
+					op_enc->ldpc_enc.tb_params.c,
+					op_enc->ldpc_enc.tb_params.cab,
+					op_enc->ldpc_enc.tb_params.ea,
+					op_enc->ldpc_enc.tb_params.eb,
+					op_enc->ldpc_enc.tb_params.r);
+		else
+			snprintf(partial, sizeof(partial), "E %d",
+					op_enc->ldpc_enc.cb_params.e);
+		if (op_enc->ldpc_enc.input.data != NULL)
+			add0 = rte_pktmbuf_iova_offset(op_enc->ldpc_enc.input.data, 0);
+		if (op_enc->ldpc_enc.output.data != NULL)
+			add1 = rte_pktmbuf_iova_offset(op_enc->ldpc_enc.output.data, 0);
+		snprintf(str, sizeof(str), "op %x st %x BG %d Zc %d Ncb %d q_m %d F %d Rv %d "
+				"in %" PRIx64 " out %" PRIx64 " %s\n",
+				op_enc->ldpc_enc.op_flags, op_enc->status,
+				op_enc->ldpc_enc.basegraph, op_enc->ldpc_enc.z_c,
+				op_enc->ldpc_enc.n_cb, op_enc->ldpc_enc.q_m,
+				op_enc->ldpc_enc.n_filler, op_enc->ldpc_enc.rv_index,
+				add0, add1, partial);
+	} else if (op_type == RTE_BBDEV_OP_TURBO_ENC) {
+		op_enc = op;
+		if (op_enc->turbo_enc.code_block_mode == RTE_BBDEV_TRANSPORT_BLOCK)
+			snprintf(partial, sizeof(partial),
+					"C %d Cab %d Ea %d Eb %d r %d K %d Ncb %d",
+					op_enc->turbo_enc.tb_params.c,
+					op_enc->turbo_enc.tb_params.cab,
+					op_enc->turbo_enc.tb_params.ea,
+					op_enc->turbo_enc.tb_params.eb,
+					op_enc->turbo_enc.tb_params.r,
+					op_enc->turbo_enc.tb_params.k_neg,
+					op_enc->turbo_enc.tb_params.ncb_neg);
+		else
+			snprintf(partial, sizeof(partial), "E %d K %d",
+					op_enc->turbo_enc.cb_params.e,
+					op_enc->turbo_enc.cb_params.k);
+		if (op_enc->turbo_enc.input.data != NULL)
+			add0 = rte_pktmbuf_iova_offset(op_enc->turbo_enc.input.data, 0);
+		if (op_enc->turbo_enc.output.data != NULL)
+			add1 = rte_pktmbuf_iova_offset(op_enc->turbo_enc.output.data, 0);
+		snprintf(str, sizeof(str), "op %x st %x CBM %d Rv %d In %" PRIx64 " Out %" PRIx64 " %s\n",
+				op_enc->turbo_enc.op_flags, op_enc->status,
+				op_enc->turbo_enc.code_block_mode, op_enc->turbo_enc.rv_index,
+				add0, add1, partial);
+	} else if (op_type == RTE_BBDEV_OP_FFT) {
+		op_fft = op;
+		if (op_fft->fft.base_input.data != NULL)
+			add0 = rte_pktmbuf_iova_offset(op_fft->fft.base_input.data, 0);
+		if (op_fft->fft.base_output.data != NULL)
+			add1 = rte_pktmbuf_iova_offset(op_fft->fft.base_output.data, 0);
+		if (op_fft->fft.dewindowing_input.data != NULL)
+			add2 = rte_pktmbuf_iova_offset(op_fft->fft.dewindowing_input.data, 0);
+		if (op_fft->fft.power_meas_output.data != NULL)
+			add3 = rte_pktmbuf_iova_offset(op_fft->fft.power_meas_output.data, 0);
+		snprintf(str, sizeof(str), "op %x st %x in %d inl %d out %d outl %d cs %x ants %d "
+				"idft %d dft %d cst %d ish %d dsh %d ncs %d pwsh %d fp16 %d fr %d "
+				"outde %d in %" PRIx64 " out %" PRIx64 " dw %" PRIx64 " "
+				"pm %" PRIx64 "\n",
+				op_fft->fft.op_flags, op_fft->status,
+				op_fft->fft.input_sequence_size, op_fft->fft.input_leading_padding,
+				op_fft->fft.output_sequence_size,
+				op_fft->fft.output_leading_depadding,
+				op_fft->fft.cs_bitmap, op_fft->fft.num_antennas_log2,
+				op_fft->fft.idft_log2, op_fft->fft.dft_log2,
+				op_fft->fft.cs_time_adjustment,
+				op_fft->fft.idft_shift, op_fft->fft.dft_shift,
+				op_fft->fft.ncs_reciprocal, op_fft->fft.power_shift,
+				op_fft->fft.fp16_exp_adjust, op_fft->fft.freq_resample_mode,
+				op_fft->fft.output_depadded_size, add0, add1, add2, add3);
+	} else if (op_type == RTE_BBDEV_OP_MLDTS) {
+		op_mldts = op;
+		if (op_mldts->mldts.qhy_input.data != NULL)
+			add0 = rte_pktmbuf_iova_offset(op_mldts->mldts.qhy_input.data, 0);
+		if (op_mldts->mldts.r_input.data != NULL)
+			add1 = rte_pktmbuf_iova_offset(op_mldts->mldts.r_input.data, 0);
+		if (op_mldts->mldts.output.data != NULL)
+			add2 = rte_pktmbuf_iova_offset(op_mldts->mldts.output.data, 0);
+		snprintf(str, sizeof(str),
+				"op %x st %x rbs %d lay %d rrep %d crep%d qm %d %d %d %d "
+				"qhy %" PRIx64 " r %" PRIx64 " out %" PRIx64 "\n",
+				op_mldts->mldts.op_flags, op_mldts->status,
+				op_mldts->mldts.num_rbs, op_mldts->mldts.num_layers,
+				op_mldts->mldts.r_rep, op_mldts->mldts.c_rep,
+				op_mldts->mldts.q_m[0], op_mldts->mldts.q_m[1],
+				op_mldts->mldts.q_m[2], op_mldts->mldts.q_m[3],
+				add0, add1, add2);
+
+	} else {
+		snprintf(str, sizeof(str), "Invalid Operation type %d\n", op_type);
+	}
+
+	return str;
+}
diff --git a/lib/bbdev/rte_bbdev.h b/lib/bbdev/rte_bbdev.h
index 0cbfdd1c95..a70d841379 100644
--- a/lib/bbdev/rte_bbdev.h
+++ b/lib/bbdev/rte_bbdev.h
@@ -1061,6 +1061,47 @@ rte_bbdev_device_status_str(enum rte_bbdev_device_status status);
 const char*
 rte_bbdev_enqueue_status_str(enum rte_bbdev_enqueue_status status);
 
+/**
+ * Dump operations info from device to a file.
+ * This API is used for debugging provided input operations, not a dataplane API.
+ *
+ *  @param dev_id
+ *    The device identifier.
+ *
+ *  @param queue_index
+ *    Index of queue.
+ *
+ *  @param file
+ *    A pointer to a file for output.
+ *
+ * @returns
+ *   - 0 on success
+ *   - ENOTSUP if interrupts are not supported by the identified device
+ *   - negative value on failure - as returned from PMD
+ *
+ */
+__rte_experimental
+int
+rte_bbdev_queue_ops_dump(uint16_t dev_id, uint16_t queue_index, FILE *file);
+
+
+/**
+ * String of parameters related to the parameters of an operation of a given type.
+ *
+ *  @param op
+ *    Pointer to an operation.
+ *
+ *  @param op_type
+ *    Operation type enum.
+ *
+ * @returns
+ *   String describing the provided operation.
+ *
+ */
+__rte_experimental
+char *
+rte_bbdev_ops_param_string(void *op, enum rte_bbdev_op_type op_type);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/bbdev/rte_bbdev_pmd.h b/lib/bbdev/rte_bbdev_pmd.h
index 442b23943d..c21a7f0c1e 100644
--- a/lib/bbdev/rte_bbdev_pmd.h
+++ b/lib/bbdev/rte_bbdev_pmd.h
@@ -133,6 +133,13 @@ typedef int (*rte_bbdev_queue_intr_enable_t)(struct rte_bbdev *dev,
 typedef int (*rte_bbdev_queue_intr_disable_t)(struct rte_bbdev *dev,
 				    uint16_t queue_id);
 
+/*
+ * @internal
+ * Function to dump previous operations on a queue of a device.
+ */
+typedef int (*rte_bbdev_queue_ops_dump_t)(struct rte_bbdev *dev,
+		uint16_t queue_id, FILE *file);
+
 /**
  * Operations implemented by drivers. Fields marked as "Required" must be
  * provided by a driver for a device to have basic functionality. "Optional"
@@ -170,6 +177,8 @@ struct rte_bbdev_ops {
 	rte_bbdev_queue_intr_enable_t queue_intr_enable;
 	/** Disable queue interrupt. Optional */
 	rte_bbdev_queue_intr_disable_t queue_intr_disable;
+	/** Dump operations on the queue. Optional */
+	rte_bbdev_queue_ops_dump_t queue_ops_dump;
 };
 
 /**
diff --git a/lib/bbdev/version.map b/lib/bbdev/version.map
index e0d82ff752..db6dfdf3c6 100644
--- a/lib/bbdev/version.map
+++ b/lib/bbdev/version.map
@@ -54,4 +54,8 @@ EXPERIMENTAL {
 	rte_bbdev_enqueue_mldts_ops;
 	rte_bbdev_mldts_op_alloc_bulk;
 	rte_bbdev_mldts_op_free_bulk;
+
+	# added in 24.11
+	rte_bbdev_queue_ops_dump;
+	rte_bbdev_ops_param_string;
 };
-- 
2.34.1

[PATCH v2 2/2] baseband/acc: improvement to logging mechanism

[Nicolas Chautru]

Support the new dev op to dump operations information
related to a given queue and information on previous errors
detected by the driver and tracked internally in PMD.

Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
---
 drivers/baseband/acc/acc_common.h  |  37 +++++++++
 drivers/baseband/acc/rte_vrb_pmd.c | 128 +++++++++++++++++++++++------
 2 files changed, 138 insertions(+), 27 deletions(-)

diff --git a/drivers/baseband/acc/acc_common.h b/drivers/baseband/acc/acc_common.h
index e249f37e38..c8914b23e0 100644
--- a/drivers/baseband/acc/acc_common.h
+++ b/drivers/baseband/acc/acc_common.h
@@ -149,6 +149,8 @@
 #define VRB2_VF_ID_SHIFT     6
 
 #define ACC_MAX_FFT_WIN      16
+#define ACC_MAX_LOGLEN    256
+#define ACC_MAX_BUFFERLEN 256
 
 extern int acc_common_logtype;
 
@@ -646,8 +648,43 @@ struct __rte_cache_aligned acc_queue {
 	rte_iova_t fcw_ring_addr_iova;
 	int8_t *derm_buffer; /* interim buffer for de-rm in SDK */
 	struct acc_device *d;
+	char error_bufs[ACC_MAX_BUFFERLEN][ACC_MAX_LOGLEN]; /**< Buffer for error log. */
+	uint16_t error_head;  /**< Head - Buffer for error log. */
+	uint16_t  error_wrap; /**< Wrap Counter - Buffer for error log. */
 };
 
+/**
+ * @brief Report error both through RTE logging and into internal driver memory.
+ *
+ * This function is used to log an error for a specific ACC queue and operation.
+ *
+ * @param q   Pointer to the ACC queue.
+ * @param op  Pointer to the operation.
+ * @param fmt Format string for the error message.
+ * @param ... Additional arguments for the format string.
+ */
+__rte_format_printf(3, 4)
+static inline void
+acc_error_log(struct acc_queue *q, void *op, const char *fmt, ...)
+{
+	va_list args, args2;
+
+	va_start(args, fmt);
+	va_copy(args2, args);
+	rte_vlog(RTE_LOG_ERR, acc_common_logtype, fmt, args);
+	vsnprintf(q->error_bufs[q->error_head], ACC_MAX_LOGLEN, fmt, args2);
+	q->error_head++;
+	snprintf(q->error_bufs[q->error_head], ACC_MAX_LOGLEN,
+			"%s", rte_bbdev_ops_param_string(op, q->op_type));
+	q->error_head++;
+	if (q->error_head == ACC_MAX_LOGLEN) {
+		q->error_head = 0;
+		q->error_wrap++;
+	}
+	va_end(args);
+	va_end(args2);
+}
+
 /* Write to MMIO register address */
 static inline void
 mmio_write(void *addr, uint32_t value)
diff --git a/drivers/baseband/acc/rte_vrb_pmd.c b/drivers/baseband/acc/rte_vrb_pmd.c
index 585dc49bd6..9d0145d09b 100644
--- a/drivers/baseband/acc/rte_vrb_pmd.c
+++ b/drivers/baseband/acc/rte_vrb_pmd.c
@@ -1022,6 +1022,10 @@ vrb_queue_setup(struct rte_bbdev *dev, uint16_t queue_id,
 	q->mmio_reg_enqueue = RTE_PTR_ADD(d->mmio_base,
 			d->queue_offset(d->pf_device, q->vf_id, q->qgrp_id, q->aq_id));
 
+	/** initialize the error buffer. */
+	q->error_head = 0;
+	q->error_wrap = 0;
+
 	rte_bbdev_log_debug(
 			"Setup dev%u q%u: qgrp_id=%u, vf_id=%u, aq_id=%u, aq_depth=%u, mmio_reg_enqueue=%p base %p\n",
 			dev->data->dev_id, queue_id, q->qgrp_id, q->vf_id,
@@ -1434,6 +1438,74 @@ vrb_queue_intr_disable(struct rte_bbdev *dev, uint16_t queue_id)
 	return 0;
 }
 
+
+static int
+vrb_queue_ops_dump(struct rte_bbdev *dev, uint16_t queue_id, FILE *f)
+{
+	struct acc_queue *q = dev->data->queues[queue_id].queue_private;
+	struct rte_bbdev_dec_op *op;
+	uint16_t start_err, end_err, i, int_nb;
+	volatile union acc_info_ring_data *ring_data;
+	uint16_t info_ring_head = q->d->info_ring_head;
+
+	if (f == NULL) {
+		rte_bbdev_log(ERR, "Invalid File input");
+		return -EINVAL;
+	}
+
+	/** Print generic information on queue status. */
+	fprintf(f, "Dump of operations %s on Queue %d by %s\n",
+			rte_bbdev_op_type_str(q->op_type), queue_id, dev->device->driver->name);
+	fprintf(f, "    AQ Enqueued %d Dequeued %d Depth %d - Available Enq %d Deq %d\n",
+			q->aq_enqueued, q->aq_dequeued, q->aq_depth,
+			acc_ring_avail_enq(q), acc_ring_avail_deq(q));
+
+	/** Print information captured in the error buffer. */
+	if (q->error_wrap == 0) {
+		start_err = 0;
+		end_err = q->error_head;
+	} else {
+		start_err = q->error_head;
+		end_err = q->error_head + ACC_MAX_BUFFERLEN;
+	}
+	fprintf(f, "Error Buffer - Head %d Wrap %d\n", q->error_head, q->error_wrap);
+	for (i = start_err; i < end_err; ++i)
+		fprintf(f, "  %d\t%s", i, q->error_bufs[i % ACC_MAX_BUFFERLEN]);
+
+	/** Print information captured in the info ring. */
+	if (q->d->info_ring != NULL) {
+		fprintf(f, "Info Ring Buffer - Head %d\n", q->d->info_ring_head);
+		ring_data = q->d->info_ring + (q->d->info_ring_head & ACC_INFO_RING_MASK);
+		while (ring_data->valid) {
+			int_nb = int_from_ring(*ring_data, q->d->device_variant);
+			if ((int_nb < ACC_PF_INT_DMA_DL_DESC_IRQ) || (
+					int_nb > ACC_PF_INT_DMA_MLD_DESC_IRQ)) {
+				fprintf(f, "  InfoRing: ITR:%d Info:0x%x",
+						int_nb, ring_data->detailed_info);
+				/* Initialize Info Ring entry and move forward. */
+				ring_data->valid = 0;
+			}
+			info_ring_head++;
+			ring_data = q->d->info_ring + (info_ring_head & ACC_INFO_RING_MASK);
+		}
+	}
+
+	fprintf(f, "Ring Content - Head %d Tail %d Depth %d\n",
+			q->sw_ring_head, q->sw_ring_tail, q->sw_ring_depth);
+	/** Print information about each operation in the software ring. */
+	for (i = 0; i < q->sw_ring_depth; ++i) {
+		op = (q->ring_addr + i)->req.op_addr;
+		if (op != NULL)
+			fprintf(f, "  %d\tn %d %s",
+					i, (q->ring_addr + i)->req.numCBs,
+					rte_bbdev_ops_param_string(op, q->op_type));
+	}
+
+	fprintf(f, "== End of File ==\n");
+
+	return 0;
+}
+
 static const struct rte_bbdev_ops vrb_bbdev_ops = {
 	.setup_queues = vrb_setup_queues,
 	.intr_enable = vrb_intr_enable,
@@ -1443,7 +1515,8 @@ static const struct rte_bbdev_ops vrb_bbdev_ops = {
 	.queue_release = vrb_queue_release,
 	.queue_stop = vrb_queue_stop,
 	.queue_intr_enable = vrb_queue_intr_enable,
-	.queue_intr_disable = vrb_queue_intr_disable
+	.queue_intr_disable = vrb_queue_intr_disable,
+	.queue_ops_dump = vrb_queue_ops_dump
 };
 
 /* PCI PF address map. */
@@ -1680,7 +1753,7 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op *op,
 		uint32_t *in_offset, uint32_t *h_out_offset,
 		uint32_t *s_out_offset, uint32_t *h_out_length,
 		uint32_t *s_out_length, uint32_t *mbuf_total_left,
-		uint32_t *seg_total_left, uint8_t r)
+		uint32_t *seg_total_left, uint8_t r, struct acc_queue *q)
 {
 	int next_triplet = 1; /* FCW already done. */
 	uint16_t k;
@@ -1724,8 +1797,8 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op *op,
 	kw = RTE_ALIGN_CEIL(k + 4, 32) * 3;
 
 	if (unlikely((*mbuf_total_left == 0) || (*mbuf_total_left < kw))) {
-		rte_bbdev_log(ERR,
-				"Mismatch between mbuf length and included CB sizes: mbuf len %u, cb len %u",
+		acc_error_log(q, (void *)op,
+				"Mismatch between mbuf length and included CB sizes: mbuf len %u, cb len %u\n",
 				*mbuf_total_left, kw);
 		return -1;
 	}
@@ -1735,8 +1808,8 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op *op,
 			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",
+		acc_error_log(q, (void *)op,
+				"Mismatch between data to process and mbuf data length in bbdev_op: %p\n",
 				op);
 		return -1;
 	}
@@ -1748,7 +1821,7 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op *op,
 			desc, h_output, *h_out_offset,
 			*h_out_length, next_triplet, ACC_DMA_BLKID_OUT_HARD);
 	if (unlikely(next_triplet < 0)) {
-		rte_bbdev_log(ERR,
+		acc_error_log(q, (void *)op,
 				"Mismatch between data to process and mbuf data length in bbdev_op: %p",
 				op);
 		return -1;
@@ -1760,7 +1833,7 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op *op,
 	/* Soft output. */
 	if (check_bit(op->turbo_dec.op_flags, RTE_BBDEV_TURBO_SOFT_OUTPUT)) {
 		if (op->turbo_dec.soft_output.data == 0) {
-			rte_bbdev_log(ERR, "Soft output is not defined");
+			acc_error_log(q, (void *)op, "Soft output is not defined\n");
 			return -1;
 		}
 		if (check_bit(op->turbo_dec.op_flags,
@@ -1773,8 +1846,8 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op *op,
 				*s_out_offset, *s_out_length, next_triplet,
 				ACC_DMA_BLKID_OUT_SOFT);
 		if (unlikely(next_triplet < 0)) {
-			rte_bbdev_log(ERR,
-					"Mismatch between data to process and mbuf data length in bbdev_op: %p",
+			acc_error_log(q, (void *)op,
+					"Mismatch between data to process and mbuf data length in bbdev_op: %p\n",
 					op);
 			return -1;
 		}
@@ -1797,7 +1870,8 @@ vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op *op,
 		struct rte_mbuf **input, struct rte_mbuf *h_output,
 		uint32_t *in_offset, uint32_t *h_out_offset,
 		uint32_t *h_out_length, uint32_t *mbuf_total_left,
-		uint32_t *seg_total_left, struct acc_fcw_ld *fcw, uint16_t device_variant)
+		uint32_t *seg_total_left, struct acc_fcw_ld *fcw, uint16_t device_variant,
+		struct acc_queue *q)
 {
 	struct rte_bbdev_op_ldpc_dec *dec = &op->ldpc_dec;
 	int next_triplet = 1; /* FCW already done. */
@@ -1808,8 +1882,8 @@ vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op *op,
 	if (device_variant == VRB1_VARIANT) {
 		if (check_bit(op->ldpc_dec.op_flags, RTE_BBDEV_LDPC_HARQ_4BIT_COMPRESSION) ||
 				check_bit(op->ldpc_dec.op_flags, RTE_BBDEV_LDPC_SOFT_OUT_ENABLE)) {
-			rte_bbdev_log(ERR,
-					"VRB1 does not support the requested capabilities %x",
+			acc_error_log(q, (void *)op,
+					"VRB1 does not support the requested capabilities %x\n",
 					op->ldpc_dec.op_flags);
 			return -1;
 		}
@@ -1829,8 +1903,8 @@ vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op *op,
 	output_length = K - dec->n_filler - crc24_overlap;
 
 	if (unlikely((*mbuf_total_left == 0) || (*mbuf_total_left < input_length))) {
-		rte_bbdev_log(ERR,
-				"Mismatch between mbuf length and included CB sizes: mbuf len %u, cb len %u",
+		acc_error_log(q, (void *)op,
+				"Mismatch between mbuf length and included CB sizes: mbuf len %u, cb len %u\n",
 				*mbuf_total_left, input_length);
 		return -1;
 	}
@@ -1842,15 +1916,15 @@ vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op *op,
 			RTE_BBDEV_LDPC_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",
+		acc_error_log(q, (void *)op,
+				"Mismatch between data to process and mbuf data length in bbdev_op: %p\n",
 				op);
 		return -1;
 	}
 
 	if (check_bit(op->ldpc_dec.op_flags, RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE)) {
 		if (op->ldpc_dec.harq_combined_input.data == 0) {
-			rte_bbdev_log(ERR, "HARQ input is not defined");
+			acc_error_log(q, (void *)op, "HARQ input is not defined\n");
 			return -1;
 		}
 		h_p_size = fcw->hcin_size0 + fcw->hcin_size1;
@@ -1859,7 +1933,7 @@ vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op *op,
 		else if (fcw->hcin_decomp_mode == 4)
 			h_p_size = h_p_size / 2;
 		if (op->ldpc_dec.harq_combined_input.data == 0) {
-			rte_bbdev_log(ERR, "HARQ input is not defined");
+			acc_error_log(q, (void *)op, "HARQ input is not defined\n");
 			return -1;
 		}
 		acc_dma_fill_blk_type(
@@ -1882,7 +1956,7 @@ vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op *op,
 
 	if (check_bit(op->ldpc_dec.op_flags, RTE_BBDEV_LDPC_SOFT_OUT_ENABLE)) {
 		if (op->ldpc_dec.soft_output.data == 0) {
-			rte_bbdev_log(ERR, "Soft output is not defined");
+			acc_error_log(q, (void *)op, "Soft output is not defined\n");
 			return -1;
 		}
 		dec->soft_output.length = fcw->rm_e;
@@ -1894,7 +1968,7 @@ vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op *op,
 	if (check_bit(op->ldpc_dec.op_flags,
 				RTE_BBDEV_LDPC_HQ_COMBINE_OUT_ENABLE)) {
 		if (op->ldpc_dec.harq_combined_output.data == 0) {
-			rte_bbdev_log(ERR, "HARQ output is not defined");
+			acc_error_log(q, (void *)op, "HARQ output is not defined\n");
 			return -1;
 		}
 
@@ -2326,8 +2400,8 @@ vrb2_enqueue_ldpc_enc_one_op_tb(struct acc_queue *q, struct rte_bbdev_enc_op *op
 			in_length_in_bytes, &seg_total_left, next_triplet,
 			check_bit(enc->op_flags, RTE_BBDEV_LDPC_ENC_SCATTER_GATHER));
 	if (unlikely(next_triplet < 0)) {
-		rte_bbdev_log(ERR,
-				"Mismatch between data to process and mbuf data length in bbdev_op: %p",
+		acc_error_log(q, (void *)op,
+				"Mismatch between data to process and mbuf data length in bbdev_op: %p\n",
 				op);
 		return -1;
 	}
@@ -2399,7 +2473,7 @@ enqueue_dec_one_op_cb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
 	ret = vrb_dma_desc_td_fill(op, &desc->req, &input, h_output,
 			s_output, &in_offset, &h_out_offset, &s_out_offset,
 			&h_out_length, &s_out_length, &mbuf_total_left,
-			&seg_total_left, 0);
+			&seg_total_left, 0, q);
 
 	if (unlikely(ret < 0))
 		return ret;
@@ -2478,7 +2552,7 @@ vrb_enqueue_ldpc_dec_one_op_cb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
 		ret = vrb_dma_desc_ld_fill(op, &desc->req, &input, h_output,
 				&in_offset, &h_out_offset,
 				&h_out_length, &mbuf_total_left,
-				&seg_total_left, fcw, q->d->device_variant);
+				&seg_total_left, fcw, q->d->device_variant, q);
 		if (unlikely(ret < 0))
 			return ret;
 	}
@@ -2572,7 +2646,7 @@ vrb_enqueue_ldpc_dec_one_op_tb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
 				h_output, &in_offset, &h_out_offset,
 				&h_out_length,
 				&mbuf_total_left, &seg_total_left,
-				&desc->req.fcw_ld, q->d->device_variant);
+				&desc->req.fcw_ld, q->d->device_variant, q);
 
 		if (unlikely(ret < 0))
 			return ret;
@@ -2658,7 +2732,7 @@ enqueue_dec_one_op_tb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
 		ret = vrb_dma_desc_td_fill(op, &desc->req, &input,
 				h_output, s_output, &in_offset, &h_out_offset,
 				&s_out_offset, &h_out_length, &s_out_length,
-				&mbuf_total_left, &seg_total_left, r);
+				&mbuf_total_left, &seg_total_left, r, q);
 
 		if (unlikely(ret < 0))
 			return ret;
-- 
2.34.1

Re: [PATCH v2 1/2] bbdev: add new function to dump debug information

[Maxime Coquelin]

On 8/29/24 22:06, Nicolas Chautru wrote:
> This provides a new API to dump more debug information
> related to the status on a given bbdev queue.
> Some of this information is visible at bbdev level.
> This also provides a new option dev op, to print more
> information at the lower PMD level.
> This helps user to troubleshoot issues related to
> previous operations provided into a queue causing
> possible hard-to-debug negative scenarios.
> 
> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> ---
>   lib/bbdev/rte_bbdev.c     | 214 ++++++++++++++++++++++++++++++++++++++
>   lib/bbdev/rte_bbdev.h     |  41 ++++++++
>   lib/bbdev/rte_bbdev_pmd.h |   9 ++
>   lib/bbdev/version.map     |   4 +
>   4 files changed, 268 insertions(+)
> 
> diff --git a/lib/bbdev/rte_bbdev.c b/lib/bbdev/rte_bbdev.c
> index 13bde3c25b..9087862c76 100644
> --- a/lib/bbdev/rte_bbdev.c
> +++ b/lib/bbdev/rte_bbdev.c
> @@ -1190,3 +1190,217 @@ rte_bbdev_enqueue_status_str(enum rte_bbdev_enqueue_status status)
>   	rte_bbdev_log(ERR, "Invalid enqueue status");
>   	return NULL;
>   }
> +
> +
> +int
> +rte_bbdev_queue_ops_dump(uint16_t dev_id, uint16_t queue_id, FILE *f)
> +{
> +	struct rte_bbdev_queue_data *q_data;
> +	struct rte_bbdev_stats *stats;
> +	uint16_t i;
> +	struct rte_bbdev *dev = get_dev(dev_id);
> +
> +	VALID_DEV_OR_RET_ERR(dev, dev_id);
> +	VALID_QUEUE_OR_RET_ERR(queue_id, dev);
> +	VALID_DEV_OPS_OR_RET_ERR(dev, dev_id);
> +	VALID_FUNC_OR_RET_ERR(dev->dev_ops->queue_ops_dump, dev_id);
> +
> +	q_data = &dev->data->queues[queue_id];
> +
> +	if (f == NULL)
> +		return -EINVAL;
> +
> +	fprintf(f, "Dump of operations on %s queue %d\n",
> +			dev->data->name, queue_id);
> +	fprintf(f, "  Last Enqueue Status %s\n",
> +			rte_bbdev_enqueue_status_str(q_data->enqueue_status));
> +	for (i = 0; i < RTE_BBDEV_ENQ_STATUS_SIZE_MAX; i++)
> +		if (q_data->queue_stats.enqueue_status_count[i] > 0)
> +			fprintf(f, "  Enqueue Status Counters %s %" PRIu64 "\n",
> +					rte_bbdev_enqueue_status_str(i),
> +					q_data->queue_stats.enqueue_status_count[i]);
> +	stats = &dev->data->queues[queue_id].queue_stats;
> +
> +	fprintf(f, "  Enqueue Count %" PRIu64 " Warning %" PRIu64 " Error %" PRIu64 "\n",
> +			stats->enqueued_count, stats->enqueue_warn_count,
> +			stats->enqueue_err_count);
> +	fprintf(f, "  Dequeue Count %" PRIu64 " Warning %" PRIu64 " Error %" PRIu64 "\n",
> +			stats->dequeued_count, stats->dequeue_warn_count,
> +			stats->dequeue_err_count);
> +
> +	return dev->dev_ops->queue_ops_dump(dev, queue_id, f);
> +}
> +
> +char *
> +rte_bbdev_ops_param_string(void *op, enum rte_bbdev_op_type op_type)
> +{
> +	static char str[1024];
> +	static char partial[1024];

It is better to have the string allocated by the application, it would
be passed as a pointer and length of the allocated buffer would be
passed also as an argument.

> +	struct rte_bbdev_dec_op *op_dec;
> +	struct rte_bbdev_enc_op *op_enc;
> +	struct rte_bbdev_fft_op *op_fft;
> +	struct rte_bbdev_mldts_op *op_mldts;
> +
> +	rte_iova_t add0 = 0, add1 = 0, add2 = 0, add3 = 0, add4 = 0;
> +
> +	if (op == NULL) {
> +		snprintf(str, sizeof(str), "Invalid Operation pointer\n");
> +		return str;
> +	}
> +
> +	if (op_type == RTE_BBDEV_OP_LDPC_DEC) {
> +		op_dec = op;
> +		if (op_dec->ldpc_dec.code_block_mode == RTE_BBDEV_TRANSPORT_BLOCK)
> +			snprintf(partial, sizeof(partial), "C %d Cab %d Ea %d Eb %d r %d",
> +					op_dec->ldpc_dec.tb_params.c,
> +					op_dec->ldpc_dec.tb_params.cab,
> +					op_dec->ldpc_dec.tb_params.ea,
> +					op_dec->ldpc_dec.tb_params.eb,
> +					op_dec->ldpc_dec.tb_params.r);
> +		else
> +			snprintf(partial, sizeof(partial), "E %d", op_dec->ldpc_dec.cb_params.e);
> +		if (op_dec->ldpc_dec.input.data != NULL)
> +			add0 = rte_pktmbuf_iova_offset(op_dec->ldpc_dec.input.data, 0);
> +		if (op_dec->ldpc_dec.hard_output.data != NULL)
> +			add1 = rte_pktmbuf_iova_offset(op_dec->ldpc_dec.hard_output.data, 0);
> +		if (op_dec->ldpc_dec.soft_output.data != NULL)
> +			add2 = rte_pktmbuf_iova_offset(op_dec->ldpc_dec.soft_output.data, 0);
> +		if (op_dec->ldpc_dec.harq_combined_input.data != NULL)
> +			add3 = rte_pktmbuf_iova_offset(op_dec->ldpc_dec.harq_combined_input.data,
> +					0);
> +		if (op_dec->ldpc_dec.harq_combined_output.data != NULL)
> +			add4 = rte_pktmbuf_iova_offset(op_dec->ldpc_dec.harq_combined_output.data,
> +					0);
> +		snprintf(str, sizeof(str), "op %x st %x BG %d Zc %d Ncb %d qm %d F %d Rv %d It %d It %d "
> +			"HARQin %d in %" PRIx64 " ho %" PRIx64 " so %" PRIx64 " hi %" PRIx64 " "
> +			"ho %" PRIx64 " %s\n",
> +			op_dec->ldpc_dec.op_flags, op_dec->status,
> +			op_dec->ldpc_dec.basegraph, op_dec->ldpc_dec.z_c,
> +			op_dec->ldpc_dec.n_cb, op_dec->ldpc_dec.q_m,
> +			op_dec->ldpc_dec.n_filler, op_dec->ldpc_dec.rv_index,
> +			op_dec->ldpc_dec.iter_max, op_dec->ldpc_dec.iter_count,
> +			op_dec->ldpc_dec.harq_combined_input.length,
> +			add0, add1, add2, add3, add4, partial);
> +	} else if (op_type == RTE_BBDEV_OP_TURBO_DEC) {
> +		op_dec = op;
> +		if (op_dec->turbo_dec.code_block_mode == RTE_BBDEV_TRANSPORT_BLOCK)
> +			snprintf(partial, sizeof(partial), "C %d Cab %d Ea %d Eb %d r %d K %d",
> +					op_dec->turbo_dec.tb_params.c,
> +					op_dec->turbo_dec.tb_params.cab,
> +					op_dec->turbo_dec.tb_params.ea,
> +					op_dec->turbo_dec.tb_params.eb,
> +					op_dec->turbo_dec.tb_params.r,
> +					op_dec->turbo_dec.tb_params.k_neg);
> +		else
> +			snprintf(partial, sizeof(partial), "E %d K %d",
> +					op_dec->turbo_dec.cb_params.e,
> +					op_dec->turbo_dec.cb_params.k);
> +		if (op_dec->turbo_dec.input.data != NULL)
> +			add0 = rte_pktmbuf_iova_offset(op_dec->turbo_dec.input.data, 0);
> +		if (op_dec->turbo_dec.hard_output.data != NULL)
> +			add1 = rte_pktmbuf_iova_offset(op_dec->turbo_dec.hard_output.data, 0);
> +		if (op_dec->turbo_dec.soft_output.data != NULL)
> +			add2 = rte_pktmbuf_iova_offset(op_dec->turbo_dec.soft_output.data, 0);
> +		snprintf(str, sizeof(str), "op %x st %x CBM %d Iter %d map %d Rv %d ext %d "
> +				"in %" PRIx64 " ho %" PRIx64 " so %" PRIx64 " %s\n",
> +				op_dec->turbo_dec.op_flags, op_dec->status,
> +				op_dec->turbo_dec.code_block_mode,
> +				op_dec->turbo_dec.iter_max, op_dec->turbo_dec.num_maps,
> +				op_dec->turbo_dec.rv_index, op_dec->turbo_dec.ext_scale,
> +				add0, add1, add2, partial);
> +	} else if (op_type == RTE_BBDEV_OP_LDPC_ENC) {
> +		op_enc = op;
> +		if (op_enc->ldpc_enc.code_block_mode == RTE_BBDEV_TRANSPORT_BLOCK)
> +			snprintf(partial, sizeof(partial), "C %d Cab %d Ea %d Eb %d r %d",
> +					op_enc->ldpc_enc.tb_params.c,
> +					op_enc->ldpc_enc.tb_params.cab,
> +					op_enc->ldpc_enc.tb_params.ea,
> +					op_enc->ldpc_enc.tb_params.eb,
> +					op_enc->ldpc_enc.tb_params.r);
> +		else
> +			snprintf(partial, sizeof(partial), "E %d",
> +					op_enc->ldpc_enc.cb_params.e);
> +		if (op_enc->ldpc_enc.input.data != NULL)
> +			add0 = rte_pktmbuf_iova_offset(op_enc->ldpc_enc.input.data, 0);
> +		if (op_enc->ldpc_enc.output.data != NULL)
> +			add1 = rte_pktmbuf_iova_offset(op_enc->ldpc_enc.output.data, 0);
> +		snprintf(str, sizeof(str), "op %x st %x BG %d Zc %d Ncb %d q_m %d F %d Rv %d "
> +				"in %" PRIx64 " out %" PRIx64 " %s\n",
> +				op_enc->ldpc_enc.op_flags, op_enc->status,
> +				op_enc->ldpc_enc.basegraph, op_enc->ldpc_enc.z_c,
> +				op_enc->ldpc_enc.n_cb, op_enc->ldpc_enc.q_m,
> +				op_enc->ldpc_enc.n_filler, op_enc->ldpc_enc.rv_index,
> +				add0, add1, partial);
> +	} else if (op_type == RTE_BBDEV_OP_TURBO_ENC) {
> +		op_enc = op;
> +		if (op_enc->turbo_enc.code_block_mode == RTE_BBDEV_TRANSPORT_BLOCK)
> +			snprintf(partial, sizeof(partial),
> +					"C %d Cab %d Ea %d Eb %d r %d K %d Ncb %d",
> +					op_enc->turbo_enc.tb_params.c,
> +					op_enc->turbo_enc.tb_params.cab,
> +					op_enc->turbo_enc.tb_params.ea,
> +					op_enc->turbo_enc.tb_params.eb,
> +					op_enc->turbo_enc.tb_params.r,
> +					op_enc->turbo_enc.tb_params.k_neg,
> +					op_enc->turbo_enc.tb_params.ncb_neg);
> +		else
> +			snprintf(partial, sizeof(partial), "E %d K %d",
> +					op_enc->turbo_enc.cb_params.e,
> +					op_enc->turbo_enc.cb_params.k);
> +		if (op_enc->turbo_enc.input.data != NULL)
> +			add0 = rte_pktmbuf_iova_offset(op_enc->turbo_enc.input.data, 0);
> +		if (op_enc->turbo_enc.output.data != NULL)
> +			add1 = rte_pktmbuf_iova_offset(op_enc->turbo_enc.output.data, 0);
> +		snprintf(str, sizeof(str), "op %x st %x CBM %d Rv %d In %" PRIx64 " Out %" PRIx64 " %s\n",
> +				op_enc->turbo_enc.op_flags, op_enc->status,
> +				op_enc->turbo_enc.code_block_mode, op_enc->turbo_enc.rv_index,
> +				add0, add1, partial);
> +	} else if (op_type == RTE_BBDEV_OP_FFT) {
> +		op_fft = op;
> +		if (op_fft->fft.base_input.data != NULL)
> +			add0 = rte_pktmbuf_iova_offset(op_fft->fft.base_input.data, 0);
> +		if (op_fft->fft.base_output.data != NULL)
> +			add1 = rte_pktmbuf_iova_offset(op_fft->fft.base_output.data, 0);
> +		if (op_fft->fft.dewindowing_input.data != NULL)
> +			add2 = rte_pktmbuf_iova_offset(op_fft->fft.dewindowing_input.data, 0);
> +		if (op_fft->fft.power_meas_output.data != NULL)
> +			add3 = rte_pktmbuf_iova_offset(op_fft->fft.power_meas_output.data, 0);
> +		snprintf(str, sizeof(str), "op %x st %x in %d inl %d out %d outl %d cs %x ants %d "
> +				"idft %d dft %d cst %d ish %d dsh %d ncs %d pwsh %d fp16 %d fr %d "
> +				"outde %d in %" PRIx64 " out %" PRIx64 " dw %" PRIx64 " "
> +				"pm %" PRIx64 "\n",
> +				op_fft->fft.op_flags, op_fft->status,
> +				op_fft->fft.input_sequence_size, op_fft->fft.input_leading_padding,
> +				op_fft->fft.output_sequence_size,
> +				op_fft->fft.output_leading_depadding,
> +				op_fft->fft.cs_bitmap, op_fft->fft.num_antennas_log2,
> +				op_fft->fft.idft_log2, op_fft->fft.dft_log2,
> +				op_fft->fft.cs_time_adjustment,
> +				op_fft->fft.idft_shift, op_fft->fft.dft_shift,
> +				op_fft->fft.ncs_reciprocal, op_fft->fft.power_shift,
> +				op_fft->fft.fp16_exp_adjust, op_fft->fft.freq_resample_mode,
> +				op_fft->fft.output_depadded_size, add0, add1, add2, add3);
> +	} else if (op_type == RTE_BBDEV_OP_MLDTS) {
> +		op_mldts = op;
> +		if (op_mldts->mldts.qhy_input.data != NULL)
> +			add0 = rte_pktmbuf_iova_offset(op_mldts->mldts.qhy_input.data, 0);
> +		if (op_mldts->mldts.r_input.data != NULL)
> +			add1 = rte_pktmbuf_iova_offset(op_mldts->mldts.r_input.data, 0);
> +		if (op_mldts->mldts.output.data != NULL)
> +			add2 = rte_pktmbuf_iova_offset(op_mldts->mldts.output.data, 0);
> +		snprintf(str, sizeof(str),
> +				"op %x st %x rbs %d lay %d rrep %d crep%d qm %d %d %d %d "
> +				"qhy %" PRIx64 " r %" PRIx64 " out %" PRIx64 "\n",
> +				op_mldts->mldts.op_flags, op_mldts->status,
> +				op_mldts->mldts.num_rbs, op_mldts->mldts.num_layers,
> +				op_mldts->mldts.r_rep, op_mldts->mldts.c_rep,
> +				op_mldts->mldts.q_m[0], op_mldts->mldts.q_m[1],
> +				op_mldts->mldts.q_m[2], op_mldts->mldts.q_m[3],
> +				add0, add1, add2);
> +
> +	} else {
> +		snprintf(str, sizeof(str), "Invalid Operation type %d\n", op_type);
> +	}
> +
> +	return str;
> +}
> diff --git a/lib/bbdev/rte_bbdev.h b/lib/bbdev/rte_bbdev.h
> index 0cbfdd1c95..a70d841379 100644
> --- a/lib/bbdev/rte_bbdev.h
> +++ b/lib/bbdev/rte_bbdev.h
> @@ -1061,6 +1061,47 @@ rte_bbdev_device_status_str(enum rte_bbdev_device_status status);
>   const char*
>   rte_bbdev_enqueue_status_str(enum rte_bbdev_enqueue_status status);
>   
> +/**
> + * Dump operations info from device to a file.
> + * This API is used for debugging provided input operations, not a dataplane API.
> + *
> + *  @param dev_id
> + *    The device identifier.
> + *
> + *  @param queue_index
> + *    Index of queue.
> + *
> + *  @param file
> + *    A pointer to a file for output.
> + *
> + * @returns
> + *   - 0 on success
> + *   - ENOTSUP if interrupts are not supported by the identified device
> + *   - negative value on failure - as returned from PMD
> + *
> + */
> +__rte_experimental
> +int
> +rte_bbdev_queue_ops_dump(uint16_t dev_id, uint16_t queue_index, FILE *file);
> +
> +
> +/**
> + * String of parameters related to the parameters of an operation of a given type.
> + *
> + *  @param op
> + *    Pointer to an operation.
> + *
> + *  @param op_type
> + *    Operation type enum.
> + *
> + * @returns
> + *   String describing the provided operation.
> + *
> + */
> +__rte_experimental
> +char *
> +rte_bbdev_ops_param_string(void *op, enum rte_bbdev_op_type op_type);
> +
>   #ifdef __cplusplus
>   }
>   #endif
> diff --git a/lib/bbdev/rte_bbdev_pmd.h b/lib/bbdev/rte_bbdev_pmd.h
> index 442b23943d..c21a7f0c1e 100644
> --- a/lib/bbdev/rte_bbdev_pmd.h
> +++ b/lib/bbdev/rte_bbdev_pmd.h
> @@ -133,6 +133,13 @@ typedef int (*rte_bbdev_queue_intr_enable_t)(struct rte_bbdev *dev,
>   typedef int (*rte_bbdev_queue_intr_disable_t)(struct rte_bbdev *dev,
>   				    uint16_t queue_id);
>   
> +/*
> + * @internal
> + * Function to dump previous operations on a queue of a device.
> + */
> +typedef int (*rte_bbdev_queue_ops_dump_t)(struct rte_bbdev *dev,
> +		uint16_t queue_id, FILE *file);
> +
>   /**
>    * Operations implemented by drivers. Fields marked as "Required" must be
>    * provided by a driver for a device to have basic functionality. "Optional"
> @@ -170,6 +177,8 @@ struct rte_bbdev_ops {
>   	rte_bbdev_queue_intr_enable_t queue_intr_enable;
>   	/** Disable queue interrupt. Optional */
>   	rte_bbdev_queue_intr_disable_t queue_intr_disable;
> +	/** Dump operations on the queue. Optional */
> +	rte_bbdev_queue_ops_dump_t queue_ops_dump;
>   };
>   
>   /**
> diff --git a/lib/bbdev/version.map b/lib/bbdev/version.map
> index e0d82ff752..db6dfdf3c6 100644
> --- a/lib/bbdev/version.map
> +++ b/lib/bbdev/version.map
> @@ -54,4 +54,8 @@ EXPERIMENTAL {
>   	rte_bbdev_enqueue_mldts_ops;
>   	rte_bbdev_mldts_op_alloc_bulk;
>   	rte_bbdev_mldts_op_free_bulk;
> +
> +	# added in 24.11
> +	rte_bbdev_queue_ops_dump;
> +	rte_bbdev_ops_param_string;
>   };

Re: [PATCH v2 2/2] baseband/acc: improvement to logging mechanism

[Maxime Coquelin]

On 8/29/24 22:06, Nicolas Chautru wrote:
> Support the new dev op to dump operations information
> related to a given queue and information on previous errors
> detected by the driver and tracked internally in PMD.
> 
> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> ---
>   drivers/baseband/acc/acc_common.h  |  37 +++++++++
>   drivers/baseband/acc/rte_vrb_pmd.c | 128 +++++++++++++++++++++++------
>   2 files changed, 138 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/baseband/acc/acc_common.h b/drivers/baseband/acc/acc_common.h
> index e249f37e38..c8914b23e0 100644
> --- a/drivers/baseband/acc/acc_common.h
> +++ b/drivers/baseband/acc/acc_common.h
> @@ -149,6 +149,8 @@
>   #define VRB2_VF_ID_SHIFT     6
>   
>   #define ACC_MAX_FFT_WIN      16
> +#define ACC_MAX_LOGLEN    256
> +#define ACC_MAX_BUFFERLEN 256
>   
>   extern int acc_common_logtype;
>   
> @@ -646,8 +648,43 @@ struct __rte_cache_aligned acc_queue {
>   	rte_iova_t fcw_ring_addr_iova;
>   	int8_t *derm_buffer; /* interim buffer for de-rm in SDK */
>   	struct acc_device *d;
> +	char error_bufs[ACC_MAX_BUFFERLEN][ACC_MAX_LOGLEN]; /**< Buffer for error log. */

65KB is quite big for each queues

I don't see the point of all this, as errors are already logged using 
rte_bbdev_log. Only thing you miss is the queue/device info in curretn
solution, no?

> +	uint16_t error_head;  /**< Head - Buffer for error log. */
> +	uint16_t  error_wrap; /**< Wrap Counter - Buffer for error log. */
>   };
>   
> +/**
> + * @brief Report error both through RTE logging and into internal driver memory.
> + *
> + * This function is used to log an error for a specific ACC queue and operation.
> + *
> + * @param q   Pointer to the ACC queue.
> + * @param op  Pointer to the operation.
> + * @param fmt Format string for the error message.
> + * @param ... Additional arguments for the format string.
> + */
> +__rte_format_printf(3, 4)
> +static inline void
> +acc_error_log(struct acc_queue *q, void *op, const char *fmt, ...)
> +{
> +	va_list args, args2;
> +
> +	va_start(args, fmt);
> +	va_copy(args2, args);
> +	rte_vlog(RTE_LOG_ERR, acc_common_logtype, fmt, args);
> +	vsnprintf(q->error_bufs[q->error_head], ACC_MAX_LOGLEN, fmt, args2);
> +	q->error_head++;
> +	snprintf(q->error_bufs[q->error_head], ACC_MAX_LOGLEN,
> +			"%s", rte_bbdev_ops_param_string(op, q->op_type));
> +	q->error_head++;
> +	if (q->error_head == ACC_MAX_LOGLEN) {
> +		q->error_head = 0;
> +		q->error_wrap++;
> +	}
> +	va_end(args);
> +	va_end(args2);
> +}
> +
>   /* Write to MMIO register address */
>   static inline void
>   mmio_write(void *addr, uint32_t value)
> diff --git a/drivers/baseband/acc/rte_vrb_pmd.c b/drivers/baseband/acc/rte_vrb_pmd.c
> index 585dc49bd6..9d0145d09b 100644
> --- a/drivers/baseband/acc/rte_vrb_pmd.c
> +++ b/drivers/baseband/acc/rte_vrb_pmd.c
> @@ -1022,6 +1022,10 @@ vrb_queue_setup(struct rte_bbdev *dev, uint16_t queue_id,
>   	q->mmio_reg_enqueue = RTE_PTR_ADD(d->mmio_base,
>   			d->queue_offset(d->pf_device, q->vf_id, q->qgrp_id, q->aq_id));
>   
> +	/** initialize the error buffer. */
> +	q->error_head = 0;
> +	q->error_wrap = 0;
> +
>   	rte_bbdev_log_debug(
>   			"Setup dev%u q%u: qgrp_id=%u, vf_id=%u, aq_id=%u, aq_depth=%u, mmio_reg_enqueue=%p base %p\n",
>   			dev->data->dev_id, queue_id, q->qgrp_id, q->vf_id,
> @@ -1434,6 +1438,74 @@ vrb_queue_intr_disable(struct rte_bbdev *dev, uint16_t queue_id)
>   	return 0;
>   }
>   
> +
> +static int
> +vrb_queue_ops_dump(struct rte_bbdev *dev, uint16_t queue_id, FILE *f)
> +{
> +	struct acc_queue *q = dev->data->queues[queue_id].queue_private;
> +	struct rte_bbdev_dec_op *op;
> +	uint16_t start_err, end_err, i, int_nb;
> +	volatile union acc_info_ring_data *ring_data;
> +	uint16_t info_ring_head = q->d->info_ring_head;
> +
> +	if (f == NULL) {
> +		rte_bbdev_log(ERR, "Invalid File input");
> +		return -EINVAL;
> +	}
> +
> +	/** Print generic information on queue status. */
> +	fprintf(f, "Dump of operations %s on Queue %d by %s\n",
> +			rte_bbdev_op_type_str(q->op_type), queue_id, dev->device->driver->name);
> +	fprintf(f, "    AQ Enqueued %d Dequeued %d Depth %d - Available Enq %d Deq %d\n",
> +			q->aq_enqueued, q->aq_dequeued, q->aq_depth,
> +			acc_ring_avail_enq(q), acc_ring_avail_deq(q));
> +
> +	/** Print information captured in the error buffer. */
> +	if (q->error_wrap == 0) {
> +		start_err = 0;
> +		end_err = q->error_head;
> +	} else {
> +		start_err = q->error_head;
> +		end_err = q->error_head + ACC_MAX_BUFFERLEN;
> +	}
> +	fprintf(f, "Error Buffer - Head %d Wrap %d\n", q->error_head, q->error_wrap);
> +	for (i = start_err; i < end_err; ++i)
> +		fprintf(f, "  %d\t%s", i, q->error_bufs[i % ACC_MAX_BUFFERLEN]);
> +
> +	/** Print information captured in the info ring. */
> +	if (q->d->info_ring != NULL) {
> +		fprintf(f, "Info Ring Buffer - Head %d\n", q->d->info_ring_head);
> +		ring_data = q->d->info_ring + (q->d->info_ring_head & ACC_INFO_RING_MASK);
> +		while (ring_data->valid) {
> +			int_nb = int_from_ring(*ring_data, q->d->device_variant);
> +			if ((int_nb < ACC_PF_INT_DMA_DL_DESC_IRQ) || (
> +					int_nb > ACC_PF_INT_DMA_MLD_DESC_IRQ)) {
> +				fprintf(f, "  InfoRing: ITR:%d Info:0x%x",
> +						int_nb, ring_data->detailed_info);
> +				/* Initialize Info Ring entry and move forward. */
> +				ring_data->valid = 0;
> +			}
> +			info_ring_head++;
> +			ring_data = q->d->info_ring + (info_ring_head & ACC_INFO_RING_MASK);
> +		}
> +	}
> +
> +	fprintf(f, "Ring Content - Head %d Tail %d Depth %d\n",
> +			q->sw_ring_head, q->sw_ring_tail, q->sw_ring_depth);
> +	/** Print information about each operation in the software ring. */
> +	for (i = 0; i < q->sw_ring_depth; ++i) {
> +		op = (q->ring_addr + i)->req.op_addr;
> +		if (op != NULL)
> +			fprintf(f, "  %d\tn %d %s",
> +					i, (q->ring_addr + i)->req.numCBs,
> +					rte_bbdev_ops_param_string(op, q->op_type));
> +	}
> +
> +	fprintf(f, "== End of File ==\n");
> +
> +	return 0;
> +}
> +
>   static const struct rte_bbdev_ops vrb_bbdev_ops = {
>   	.setup_queues = vrb_setup_queues,
>   	.intr_enable = vrb_intr_enable,
> @@ -1443,7 +1515,8 @@ static const struct rte_bbdev_ops vrb_bbdev_ops = {
>   	.queue_release = vrb_queue_release,
>   	.queue_stop = vrb_queue_stop,
>   	.queue_intr_enable = vrb_queue_intr_enable,
> -	.queue_intr_disable = vrb_queue_intr_disable
> +	.queue_intr_disable = vrb_queue_intr_disable,
> +	.queue_ops_dump = vrb_queue_ops_dump
>   };
>   
>   /* PCI PF address map. */
> @@ -1680,7 +1753,7 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op *op,
>   		uint32_t *in_offset, uint32_t *h_out_offset,
>   		uint32_t *s_out_offset, uint32_t *h_out_length,
>   		uint32_t *s_out_length, uint32_t *mbuf_total_left,
> -		uint32_t *seg_total_left, uint8_t r)
> +		uint32_t *seg_total_left, uint8_t r, struct acc_queue *q)
>   {
>   	int next_triplet = 1; /* FCW already done. */
>   	uint16_t k;
> @@ -1724,8 +1797,8 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op *op,
>   	kw = RTE_ALIGN_CEIL(k + 4, 32) * 3;
>   
>   	if (unlikely((*mbuf_total_left == 0) || (*mbuf_total_left < kw))) {
> -		rte_bbdev_log(ERR,
> -				"Mismatch between mbuf length and included CB sizes: mbuf len %u, cb len %u",
> +		acc_error_log(q, (void *)op,
> +				"Mismatch between mbuf length and included CB sizes: mbuf len %u, cb len %u\n",
>   				*mbuf_total_left, kw);
>   		return -1;
>   	}
> @@ -1735,8 +1808,8 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op *op,
>   			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",
> +		acc_error_log(q, (void *)op,
> +				"Mismatch between data to process and mbuf data length in bbdev_op: %p\n",
>   				op);
>   		return -1;
>   	}
> @@ -1748,7 +1821,7 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op *op,
>   			desc, h_output, *h_out_offset,
>   			*h_out_length, next_triplet, ACC_DMA_BLKID_OUT_HARD);
>   	if (unlikely(next_triplet < 0)) {
> -		rte_bbdev_log(ERR,
> +		acc_error_log(q, (void *)op,
>   				"Mismatch between data to process and mbuf data length in bbdev_op: %p",
>   				op);
>   		return -1;
> @@ -1760,7 +1833,7 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op *op,
>   	/* Soft output. */
>   	if (check_bit(op->turbo_dec.op_flags, RTE_BBDEV_TURBO_SOFT_OUTPUT)) {
>   		if (op->turbo_dec.soft_output.data == 0) {
> -			rte_bbdev_log(ERR, "Soft output is not defined");
> +			acc_error_log(q, (void *)op, "Soft output is not defined\n");
>   			return -1;
>   		}
>   		if (check_bit(op->turbo_dec.op_flags,
> @@ -1773,8 +1846,8 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op *op,
>   				*s_out_offset, *s_out_length, next_triplet,
>   				ACC_DMA_BLKID_OUT_SOFT);
>   		if (unlikely(next_triplet < 0)) {
> -			rte_bbdev_log(ERR,
> -					"Mismatch between data to process and mbuf data length in bbdev_op: %p",
> +			acc_error_log(q, (void *)op,
> +					"Mismatch between data to process and mbuf data length in bbdev_op: %p\n",
>   					op);
>   			return -1;
>   		}
> @@ -1797,7 +1870,8 @@ vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op *op,
>   		struct rte_mbuf **input, struct rte_mbuf *h_output,
>   		uint32_t *in_offset, uint32_t *h_out_offset,
>   		uint32_t *h_out_length, uint32_t *mbuf_total_left,
> -		uint32_t *seg_total_left, struct acc_fcw_ld *fcw, uint16_t device_variant)
> +		uint32_t *seg_total_left, struct acc_fcw_ld *fcw, uint16_t device_variant,
> +		struct acc_queue *q)
>   {
>   	struct rte_bbdev_op_ldpc_dec *dec = &op->ldpc_dec;
>   	int next_triplet = 1; /* FCW already done. */
> @@ -1808,8 +1882,8 @@ vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op *op,
>   	if (device_variant == VRB1_VARIANT) {
>   		if (check_bit(op->ldpc_dec.op_flags, RTE_BBDEV_LDPC_HARQ_4BIT_COMPRESSION) ||
>   				check_bit(op->ldpc_dec.op_flags, RTE_BBDEV_LDPC_SOFT_OUT_ENABLE)) {
> -			rte_bbdev_log(ERR,
> -					"VRB1 does not support the requested capabilities %x",
> +			acc_error_log(q, (void *)op,
> +					"VRB1 does not support the requested capabilities %x\n",
>   					op->ldpc_dec.op_flags);
>   			return -1;
>   		}
> @@ -1829,8 +1903,8 @@ vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op *op,
>   	output_length = K - dec->n_filler - crc24_overlap;
>   
>   	if (unlikely((*mbuf_total_left == 0) || (*mbuf_total_left < input_length))) {
> -		rte_bbdev_log(ERR,
> -				"Mismatch between mbuf length and included CB sizes: mbuf len %u, cb len %u",
> +		acc_error_log(q, (void *)op,
> +				"Mismatch between mbuf length and included CB sizes: mbuf len %u, cb len %u\n",
>   				*mbuf_total_left, input_length);
>   		return -1;
>   	}
> @@ -1842,15 +1916,15 @@ vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op *op,
>   			RTE_BBDEV_LDPC_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",
> +		acc_error_log(q, (void *)op,
> +				"Mismatch between data to process and mbuf data length in bbdev_op: %p\n",
>   				op);
>   		return -1;
>   	}
>   
>   	if (check_bit(op->ldpc_dec.op_flags, RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE)) {
>   		if (op->ldpc_dec.harq_combined_input.data == 0) {
> -			rte_bbdev_log(ERR, "HARQ input is not defined");
> +			acc_error_log(q, (void *)op, "HARQ input is not defined\n");
>   			return -1;
>   		}
>   		h_p_size = fcw->hcin_size0 + fcw->hcin_size1;
> @@ -1859,7 +1933,7 @@ vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op *op,
>   		else if (fcw->hcin_decomp_mode == 4)
>   			h_p_size = h_p_size / 2;
>   		if (op->ldpc_dec.harq_combined_input.data == 0) {
> -			rte_bbdev_log(ERR, "HARQ input is not defined");
> +			acc_error_log(q, (void *)op, "HARQ input is not defined\n");
>   			return -1;
>   		}
>   		acc_dma_fill_blk_type(
> @@ -1882,7 +1956,7 @@ vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op *op,
>   
>   	if (check_bit(op->ldpc_dec.op_flags, RTE_BBDEV_LDPC_SOFT_OUT_ENABLE)) {
>   		if (op->ldpc_dec.soft_output.data == 0) {
> -			rte_bbdev_log(ERR, "Soft output is not defined");
> +			acc_error_log(q, (void *)op, "Soft output is not defined\n");
>   			return -1;
>   		}
>   		dec->soft_output.length = fcw->rm_e;
> @@ -1894,7 +1968,7 @@ vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op *op,
>   	if (check_bit(op->ldpc_dec.op_flags,
>   				RTE_BBDEV_LDPC_HQ_COMBINE_OUT_ENABLE)) {
>   		if (op->ldpc_dec.harq_combined_output.data == 0) {
> -			rte_bbdev_log(ERR, "HARQ output is not defined");
> +			acc_error_log(q, (void *)op, "HARQ output is not defined\n");
>   			return -1;
>   		}
>   
> @@ -2326,8 +2400,8 @@ vrb2_enqueue_ldpc_enc_one_op_tb(struct acc_queue *q, struct rte_bbdev_enc_op *op
>   			in_length_in_bytes, &seg_total_left, next_triplet,
>   			check_bit(enc->op_flags, RTE_BBDEV_LDPC_ENC_SCATTER_GATHER));
>   	if (unlikely(next_triplet < 0)) {
> -		rte_bbdev_log(ERR,
> -				"Mismatch between data to process and mbuf data length in bbdev_op: %p",
> +		acc_error_log(q, (void *)op,
> +				"Mismatch between data to process and mbuf data length in bbdev_op: %p\n",
>   				op);
>   		return -1;
>   	}
> @@ -2399,7 +2473,7 @@ enqueue_dec_one_op_cb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
>   	ret = vrb_dma_desc_td_fill(op, &desc->req, &input, h_output,
>   			s_output, &in_offset, &h_out_offset, &s_out_offset,
>   			&h_out_length, &s_out_length, &mbuf_total_left,
> -			&seg_total_left, 0);
> +			&seg_total_left, 0, q);
>   
>   	if (unlikely(ret < 0))
>   		return ret;
> @@ -2478,7 +2552,7 @@ vrb_enqueue_ldpc_dec_one_op_cb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
>   		ret = vrb_dma_desc_ld_fill(op, &desc->req, &input, h_output,
>   				&in_offset, &h_out_offset,
>   				&h_out_length, &mbuf_total_left,
> -				&seg_total_left, fcw, q->d->device_variant);
> +				&seg_total_left, fcw, q->d->device_variant, q);
>   		if (unlikely(ret < 0))
>   			return ret;
>   	}
> @@ -2572,7 +2646,7 @@ vrb_enqueue_ldpc_dec_one_op_tb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
>   				h_output, &in_offset, &h_out_offset,
>   				&h_out_length,
>   				&mbuf_total_left, &seg_total_left,
> -				&desc->req.fcw_ld, q->d->device_variant);
> +				&desc->req.fcw_ld, q->d->device_variant, q);
>   
>   		if (unlikely(ret < 0))
>   			return ret;
> @@ -2658,7 +2732,7 @@ enqueue_dec_one_op_tb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
>   		ret = vrb_dma_desc_td_fill(op, &desc->req, &input,
>   				h_output, s_output, &in_offset, &h_out_offset,
>   				&s_out_offset, &h_out_length, &s_out_length,
> -				&mbuf_total_left, &seg_total_left, r);
> +				&mbuf_total_left, &seg_total_left, r, q);
>   
>   		if (unlikely(ret < 0))
>   			return ret;

RE: [PATCH v2 2/2] baseband/acc: improvement to logging mechanism

[Chautru, Nicolas]

Hi Maxime, 

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Friday, September 13, 2024 7:18 AM
> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org
> Cc: hemant.agrawal@nxp.com; Marchand, David
> <david.marchand@redhat.com>; Vargas, Hernan <hernan.vargas@intel.com>
> Subject: Re: [PATCH v2 2/2] baseband/acc: improvement to logging
> mechanism
> 
> 
> 
> On 8/29/24 22:06, Nicolas Chautru wrote:
> > Support the new dev op to dump operations information related to a
> > given queue and information on previous errors detected by the driver
> > and tracked internally in PMD.
> >
> > Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> > ---
> >   drivers/baseband/acc/acc_common.h  |  37 +++++++++
> >   drivers/baseband/acc/rte_vrb_pmd.c | 128 +++++++++++++++++++++++-----
> -
> >   2 files changed, 138 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/baseband/acc/acc_common.h
> > b/drivers/baseband/acc/acc_common.h
> > index e249f37e38..c8914b23e0 100644
> > --- a/drivers/baseband/acc/acc_common.h
> > +++ b/drivers/baseband/acc/acc_common.h
> > @@ -149,6 +149,8 @@
> >   #define VRB2_VF_ID_SHIFT     6
> >
> >   #define ACC_MAX_FFT_WIN      16
> > +#define ACC_MAX_LOGLEN    256
> > +#define ACC_MAX_BUFFERLEN 256
> >
> >   extern int acc_common_logtype;
> >
> > @@ -646,8 +648,43 @@ struct __rte_cache_aligned acc_queue {
> >   	rte_iova_t fcw_ring_addr_iova;
> >   	int8_t *derm_buffer; /* interim buffer for de-rm in SDK */
> >   	struct acc_device *d;
> > +	char error_bufs[ACC_MAX_BUFFERLEN][ACC_MAX_LOGLEN]; /**<
> Buffer for
> > +error log. */
> 
> 65KB is quite big for each queues
> 
> I don't see the point of all this, as errors are already logged using
> rte_bbdev_log. Only thing you miss is the queue/device info in curretn
> solution, no?

The rte_bbdev_log is only for std_output which would be disabled by default for real time application so very limited usability.
Here we store the related information for the application to be able to retrieve themselves as required after the fact (not impacting real time). 

> 
> > +	uint16_t error_head;  /**< Head - Buffer for error log. */
> > +	uint16_t  error_wrap; /**< Wrap Counter - Buffer for error log. */
> >   };
> >
> > +/**
> > + * @brief Report error both through RTE logging and into internal driver
> memory.
> > + *
> > + * This function is used to log an error for a specific ACC queue and
> operation.
> > + *
> > + * @param q   Pointer to the ACC queue.
> > + * @param op  Pointer to the operation.
> > + * @param fmt Format string for the error message.
> > + * @param ... Additional arguments for the format string.
> > + */
> > +__rte_format_printf(3, 4)
> > +static inline void
> > +acc_error_log(struct acc_queue *q, void *op, const char *fmt, ...) {
> > +	va_list args, args2;
> > +
> > +	va_start(args, fmt);
> > +	va_copy(args2, args);
> > +	rte_vlog(RTE_LOG_ERR, acc_common_logtype, fmt, args);
> > +	vsnprintf(q->error_bufs[q->error_head], ACC_MAX_LOGLEN, fmt,
> args2);
> > +	q->error_head++;
> > +	snprintf(q->error_bufs[q->error_head], ACC_MAX_LOGLEN,
> > +			"%s", rte_bbdev_ops_param_string(op, q->op_type));
> > +	q->error_head++;
> > +	if (q->error_head == ACC_MAX_LOGLEN) {
> > +		q->error_head = 0;
> > +		q->error_wrap++;
> > +	}
> > +	va_end(args);
> > +	va_end(args2);
> > +}
> > +
> >   /* Write to MMIO register address */
> >   static inline void
> >   mmio_write(void *addr, uint32_t value) diff --git
> > a/drivers/baseband/acc/rte_vrb_pmd.c
> > b/drivers/baseband/acc/rte_vrb_pmd.c
> > index 585dc49bd6..9d0145d09b 100644
> > --- a/drivers/baseband/acc/rte_vrb_pmd.c
> > +++ b/drivers/baseband/acc/rte_vrb_pmd.c
> > @@ -1022,6 +1022,10 @@ vrb_queue_setup(struct rte_bbdev *dev,
> uint16_t queue_id,
> >   	q->mmio_reg_enqueue = RTE_PTR_ADD(d->mmio_base,
> >   			d->queue_offset(d->pf_device, q->vf_id, q->qgrp_id, q-
> >aq_id));
> >
> > +	/** initialize the error buffer. */
> > +	q->error_head = 0;
> > +	q->error_wrap = 0;
> > +
> >   	rte_bbdev_log_debug(
> >   			"Setup dev%u q%u: qgrp_id=%u, vf_id=%u, aq_id=%u,
> aq_depth=%u, mmio_reg_enqueue=%p base %p\n",
> >   			dev->data->dev_id, queue_id, q->qgrp_id, q->vf_id,
> @@ -1434,6
> > +1438,74 @@ vrb_queue_intr_disable(struct rte_bbdev *dev, uint16_t
> queue_id)
> >   	return 0;
> >   }
> >
> > +
> > +static int
> > +vrb_queue_ops_dump(struct rte_bbdev *dev, uint16_t queue_id, FILE *f)
> > +{
> > +	struct acc_queue *q = dev->data->queues[queue_id].queue_private;
> > +	struct rte_bbdev_dec_op *op;
> > +	uint16_t start_err, end_err, i, int_nb;
> > +	volatile union acc_info_ring_data *ring_data;
> > +	uint16_t info_ring_head = q->d->info_ring_head;
> > +
> > +	if (f == NULL) {
> > +		rte_bbdev_log(ERR, "Invalid File input");
> > +		return -EINVAL;
> > +	}
> > +
> > +	/** Print generic information on queue status. */
> > +	fprintf(f, "Dump of operations %s on Queue %d by %s\n",
> > +			rte_bbdev_op_type_str(q->op_type), queue_id, dev-
> >device->driver->name);
> > +	fprintf(f, "    AQ Enqueued %d Dequeued %d Depth %d - Available Enq
> %d Deq %d\n",
> > +			q->aq_enqueued, q->aq_dequeued, q->aq_depth,
> > +			acc_ring_avail_enq(q), acc_ring_avail_deq(q));
> > +
> > +	/** Print information captured in the error buffer. */
> > +	if (q->error_wrap == 0) {
> > +		start_err = 0;
> > +		end_err = q->error_head;
> > +	} else {
> > +		start_err = q->error_head;
> > +		end_err = q->error_head + ACC_MAX_BUFFERLEN;
> > +	}
> > +	fprintf(f, "Error Buffer - Head %d Wrap %d\n", q->error_head, q-
> >error_wrap);
> > +	for (i = start_err; i < end_err; ++i)
> > +		fprintf(f, "  %d\t%s", i, q->error_bufs[i %
> ACC_MAX_BUFFERLEN]);
> > +
> > +	/** Print information captured in the info ring. */
> > +	if (q->d->info_ring != NULL) {
> > +		fprintf(f, "Info Ring Buffer - Head %d\n", q->d-
> >info_ring_head);
> > +		ring_data = q->d->info_ring + (q->d->info_ring_head &
> ACC_INFO_RING_MASK);
> > +		while (ring_data->valid) {
> > +			int_nb = int_from_ring(*ring_data, q->d-
> >device_variant);
> > +			if ((int_nb < ACC_PF_INT_DMA_DL_DESC_IRQ) || (
> > +					int_nb >
> ACC_PF_INT_DMA_MLD_DESC_IRQ)) {
> > +				fprintf(f, "  InfoRing: ITR:%d Info:0x%x",
> > +						int_nb, ring_data-
> >detailed_info);
> > +				/* Initialize Info Ring entry and move forward.
> */
> > +				ring_data->valid = 0;
> > +			}
> > +			info_ring_head++;
> > +			ring_data = q->d->info_ring + (info_ring_head &
> ACC_INFO_RING_MASK);
> > +		}
> > +	}
> > +
> > +	fprintf(f, "Ring Content - Head %d Tail %d Depth %d\n",
> > +			q->sw_ring_head, q->sw_ring_tail, q->sw_ring_depth);
> > +	/** Print information about each operation in the software ring. */
> > +	for (i = 0; i < q->sw_ring_depth; ++i) {
> > +		op = (q->ring_addr + i)->req.op_addr;
> > +		if (op != NULL)
> > +			fprintf(f, "  %d\tn %d %s",
> > +					i, (q->ring_addr + i)->req.numCBs,
> > +					rte_bbdev_ops_param_string(op, q-
> >op_type));
> > +	}
> > +
> > +	fprintf(f, "== End of File ==\n");
> > +
> > +	return 0;
> > +}
> > +
> >   static const struct rte_bbdev_ops vrb_bbdev_ops = {
> >   	.setup_queues = vrb_setup_queues,
> >   	.intr_enable = vrb_intr_enable,
> > @@ -1443,7 +1515,8 @@ static const struct rte_bbdev_ops vrb_bbdev_ops
> = {
> >   	.queue_release = vrb_queue_release,
> >   	.queue_stop = vrb_queue_stop,
> >   	.queue_intr_enable = vrb_queue_intr_enable,
> > -	.queue_intr_disable = vrb_queue_intr_disable
> > +	.queue_intr_disable = vrb_queue_intr_disable,
> > +	.queue_ops_dump = vrb_queue_ops_dump
> >   };
> >
> >   /* PCI PF address map. */
> > @@ -1680,7 +1753,7 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op
> *op,
> >   		uint32_t *in_offset, uint32_t *h_out_offset,
> >   		uint32_t *s_out_offset, uint32_t *h_out_length,
> >   		uint32_t *s_out_length, uint32_t *mbuf_total_left,
> > -		uint32_t *seg_total_left, uint8_t r)
> > +		uint32_t *seg_total_left, uint8_t r, struct acc_queue *q)
> >   {
> >   	int next_triplet = 1; /* FCW already done. */
> >   	uint16_t k;
> > @@ -1724,8 +1797,8 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op
> *op,
> >   	kw = RTE_ALIGN_CEIL(k + 4, 32) * 3;
> >
> >   	if (unlikely((*mbuf_total_left == 0) || (*mbuf_total_left < kw))) {
> > -		rte_bbdev_log(ERR,
> > -				"Mismatch between mbuf length and included
> CB sizes: mbuf len %u, cb len %u",
> > +		acc_error_log(q, (void *)op,
> > +				"Mismatch between mbuf length and included
> CB sizes: mbuf len %u,
> > +cb len %u\n",
> >   				*mbuf_total_left, kw);
> >   		return -1;
> >   	}
> > @@ -1735,8 +1808,8 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op
> *op,
> >   			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",
> > +		acc_error_log(q, (void *)op,
> > +				"Mismatch between data to process and mbuf
> data length in
> > +bbdev_op: %p\n",
> >   				op);
> >   		return -1;
> >   	}
> > @@ -1748,7 +1821,7 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op
> *op,
> >   			desc, h_output, *h_out_offset,
> >   			*h_out_length, next_triplet,
> ACC_DMA_BLKID_OUT_HARD);
> >   	if (unlikely(next_triplet < 0)) {
> > -		rte_bbdev_log(ERR,
> > +		acc_error_log(q, (void *)op,
> >   				"Mismatch between data to process and mbuf
> data length in bbdev_op: %p",
> >   				op);
> >   		return -1;
> > @@ -1760,7 +1833,7 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op
> *op,
> >   	/* Soft output. */
> >   	if (check_bit(op->turbo_dec.op_flags,
> RTE_BBDEV_TURBO_SOFT_OUTPUT)) {
> >   		if (op->turbo_dec.soft_output.data == 0) {
> > -			rte_bbdev_log(ERR, "Soft output is not defined");
> > +			acc_error_log(q, (void *)op, "Soft output is not
> defined\n");
> >   			return -1;
> >   		}
> >   		if (check_bit(op->turbo_dec.op_flags,
> > @@ -1773,8 +1846,8 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op
> *op,
> >   				*s_out_offset, *s_out_length, next_triplet,
> >   				ACC_DMA_BLKID_OUT_SOFT);
> >   		if (unlikely(next_triplet < 0)) {
> > -			rte_bbdev_log(ERR,
> > -					"Mismatch between data to process
> and mbuf data length in bbdev_op: %p",
> > +			acc_error_log(q, (void *)op,
> > +					"Mismatch between data to process
> and mbuf data length in
> > +bbdev_op: %p\n",
> >   					op);
> >   			return -1;
> >   		}
> > @@ -1797,7 +1870,8 @@ vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op
> *op,
> >   		struct rte_mbuf **input, struct rte_mbuf *h_output,
> >   		uint32_t *in_offset, uint32_t *h_out_offset,
> >   		uint32_t *h_out_length, uint32_t *mbuf_total_left,
> > -		uint32_t *seg_total_left, struct acc_fcw_ld *fcw, uint16_t
> device_variant)
> > +		uint32_t *seg_total_left, struct acc_fcw_ld *fcw, uint16_t
> device_variant,
> > +		struct acc_queue *q)
> >   {
> >   	struct rte_bbdev_op_ldpc_dec *dec = &op->ldpc_dec;
> >   	int next_triplet = 1; /* FCW already done. */ @@ -1808,8 +1882,8
> @@
> > vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op *op,
> >   	if (device_variant == VRB1_VARIANT) {
> >   		if (check_bit(op->ldpc_dec.op_flags,
> RTE_BBDEV_LDPC_HARQ_4BIT_COMPRESSION) ||
> >   				check_bit(op->ldpc_dec.op_flags,
> RTE_BBDEV_LDPC_SOFT_OUT_ENABLE)) {
> > -			rte_bbdev_log(ERR,
> > -					"VRB1 does not support the requested
> capabilities %x",
> > +			acc_error_log(q, (void *)op,
> > +					"VRB1 does not support the requested
> capabilities %x\n",
> >   					op->ldpc_dec.op_flags);
> >   			return -1;
> >   		}
> > @@ -1829,8 +1903,8 @@ vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op
> *op,
> >   	output_length = K - dec->n_filler - crc24_overlap;
> >
> >   	if (unlikely((*mbuf_total_left == 0) || (*mbuf_total_left <
> input_length))) {
> > -		rte_bbdev_log(ERR,
> > -				"Mismatch between mbuf length and included
> CB sizes: mbuf len %u, cb len %u",
> > +		acc_error_log(q, (void *)op,
> > +				"Mismatch between mbuf length and included
> CB sizes: mbuf len %u,
> > +cb len %u\n",
> >   				*mbuf_total_left, input_length);
> >   		return -1;
> >   	}
> > @@ -1842,15 +1916,15 @@ vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op
> *op,
> >   			RTE_BBDEV_LDPC_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",
> > +		acc_error_log(q, (void *)op,
> > +				"Mismatch between data to process and mbuf
> data length in
> > +bbdev_op: %p\n",
> >   				op);
> >   		return -1;
> >   	}
> >
> >   	if (check_bit(op->ldpc_dec.op_flags,
> RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE)) {
> >   		if (op->ldpc_dec.harq_combined_input.data == 0) {
> > -			rte_bbdev_log(ERR, "HARQ input is not defined");
> > +			acc_error_log(q, (void *)op, "HARQ input is not
> defined\n");
> >   			return -1;
> >   		}
> >   		h_p_size = fcw->hcin_size0 + fcw->hcin_size1; @@ -1859,7
> +1933,7
> > @@ vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op *op,
> >   		else if (fcw->hcin_decomp_mode == 4)
> >   			h_p_size = h_p_size / 2;
> >   		if (op->ldpc_dec.harq_combined_input.data == 0) {
> > -			rte_bbdev_log(ERR, "HARQ input is not defined");
> > +			acc_error_log(q, (void *)op, "HARQ input is not
> defined\n");
> >   			return -1;
> >   		}
> >   		acc_dma_fill_blk_type(
> > @@ -1882,7 +1956,7 @@ vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op
> > *op,
> >
> >   	if (check_bit(op->ldpc_dec.op_flags,
> RTE_BBDEV_LDPC_SOFT_OUT_ENABLE)) {
> >   		if (op->ldpc_dec.soft_output.data == 0) {
> > -			rte_bbdev_log(ERR, "Soft output is not defined");
> > +			acc_error_log(q, (void *)op, "Soft output is not
> defined\n");
> >   			return -1;
> >   		}
> >   		dec->soft_output.length = fcw->rm_e; @@ -1894,7 +1968,7
> @@
> > vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op *op,
> >   	if (check_bit(op->ldpc_dec.op_flags,
> >
> 	RTE_BBDEV_LDPC_HQ_COMBINE_OUT_ENABLE)) {
> >   		if (op->ldpc_dec.harq_combined_output.data == 0) {
> > -			rte_bbdev_log(ERR, "HARQ output is not defined");
> > +			acc_error_log(q, (void *)op, "HARQ output is not
> defined\n");
> >   			return -1;
> >   		}
> >
> > @@ -2326,8 +2400,8 @@ vrb2_enqueue_ldpc_enc_one_op_tb(struct
> acc_queue *q, struct rte_bbdev_enc_op *op
> >   			in_length_in_bytes, &seg_total_left, next_triplet,
> >   			check_bit(enc->op_flags,
> RTE_BBDEV_LDPC_ENC_SCATTER_GATHER));
> >   	if (unlikely(next_triplet < 0)) {
> > -		rte_bbdev_log(ERR,
> > -				"Mismatch between data to process and mbuf
> data length in bbdev_op: %p",
> > +		acc_error_log(q, (void *)op,
> > +				"Mismatch between data to process and mbuf
> data length in
> > +bbdev_op: %p\n",
> >   				op);
> >   		return -1;
> >   	}
> > @@ -2399,7 +2473,7 @@ enqueue_dec_one_op_cb(struct acc_queue *q,
> struct rte_bbdev_dec_op *op,
> >   	ret = vrb_dma_desc_td_fill(op, &desc->req, &input, h_output,
> >   			s_output, &in_offset, &h_out_offset, &s_out_offset,
> >   			&h_out_length, &s_out_length, &mbuf_total_left,
> > -			&seg_total_left, 0);
> > +			&seg_total_left, 0, q);
> >
> >   	if (unlikely(ret < 0))
> >   		return ret;
> > @@ -2478,7 +2552,7 @@ vrb_enqueue_ldpc_dec_one_op_cb(struct
> acc_queue *q, struct rte_bbdev_dec_op *op,
> >   		ret = vrb_dma_desc_ld_fill(op, &desc->req, &input, h_output,
> >   				&in_offset, &h_out_offset,
> >   				&h_out_length, &mbuf_total_left,
> > -				&seg_total_left, fcw, q->d->device_variant);
> > +				&seg_total_left, fcw, q->d->device_variant, q);
> >   		if (unlikely(ret < 0))
> >   			return ret;
> >   	}
> > @@ -2572,7 +2646,7 @@ vrb_enqueue_ldpc_dec_one_op_tb(struct
> acc_queue *q, struct rte_bbdev_dec_op *op,
> >   				h_output, &in_offset, &h_out_offset,
> >   				&h_out_length,
> >   				&mbuf_total_left, &seg_total_left,
> > -				&desc->req.fcw_ld, q->d->device_variant);
> > +				&desc->req.fcw_ld, q->d->device_variant, q);
> >
> >   		if (unlikely(ret < 0))
> >   			return ret;
> > @@ -2658,7 +2732,7 @@ enqueue_dec_one_op_tb(struct acc_queue *q,
> struct rte_bbdev_dec_op *op,
> >   		ret = vrb_dma_desc_td_fill(op, &desc->req, &input,
> >   				h_output, s_output, &in_offset,
> &h_out_offset,
> >   				&s_out_offset, &h_out_length,
> &s_out_length,
> > -				&mbuf_total_left, &seg_total_left, r);
> > +				&mbuf_total_left, &seg_total_left, r, q);
> >
> >   		if (unlikely(ret < 0))
> >   			return ret;

RE: [PATCH v2 1/2] bbdev: add new function to dump debug information

[Chautru, Nicolas]

Hi Maxime, 

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Friday, September 13, 2024 7:10 AM
> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org
> Cc: hemant.agrawal@nxp.com; Marchand, David
> <david.marchand@redhat.com>; Vargas, Hernan <hernan.vargas@intel.com>
> Subject: Re: [PATCH v2 1/2] bbdev: add new function to dump debug
> information
> 
> 
> 
> On 8/29/24 22:06, Nicolas Chautru wrote:
> > This provides a new API to dump more debug information related to the
> > status on a given bbdev queue.
> > Some of this information is visible at bbdev level.
> > This also provides a new option dev op, to print more information at
> > the lower PMD level.
> > This helps user to troubleshoot issues related to previous operations
> > provided into a queue causing possible hard-to-debug negative
> > scenarios.
> >
> > Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> > ---
> >   lib/bbdev/rte_bbdev.c     | 214
> ++++++++++++++++++++++++++++++++++++++
> >   lib/bbdev/rte_bbdev.h     |  41 ++++++++
> >   lib/bbdev/rte_bbdev_pmd.h |   9 ++
> >   lib/bbdev/version.map     |   4 +
> >   4 files changed, 268 insertions(+)
> >
> > diff --git a/lib/bbdev/rte_bbdev.c b/lib/bbdev/rte_bbdev.c index
> > 13bde3c25b..9087862c76 100644
> > --- a/lib/bbdev/rte_bbdev.c
> > +++ b/lib/bbdev/rte_bbdev.c
> > @@ -1190,3 +1190,217 @@ rte_bbdev_enqueue_status_str(enum
> rte_bbdev_enqueue_status status)
> >   	rte_bbdev_log(ERR, "Invalid enqueue status");
> >   	return NULL;
> >   }
> > +
> > +
> > +int
> > +rte_bbdev_queue_ops_dump(uint16_t dev_id, uint16_t queue_id, FILE *f)
> > +{
> > +	struct rte_bbdev_queue_data *q_data;
> > +	struct rte_bbdev_stats *stats;
> > +	uint16_t i;
> > +	struct rte_bbdev *dev = get_dev(dev_id);
> > +
> > +	VALID_DEV_OR_RET_ERR(dev, dev_id);
> > +	VALID_QUEUE_OR_RET_ERR(queue_id, dev);
> > +	VALID_DEV_OPS_OR_RET_ERR(dev, dev_id);
> > +	VALID_FUNC_OR_RET_ERR(dev->dev_ops->queue_ops_dump,
> dev_id);
> > +
> > +	q_data = &dev->data->queues[queue_id];
> > +
> > +	if (f == NULL)
> > +		return -EINVAL;
> > +
> > +	fprintf(f, "Dump of operations on %s queue %d\n",
> > +			dev->data->name, queue_id);
> > +	fprintf(f, "  Last Enqueue Status %s\n",
> > +			rte_bbdev_enqueue_status_str(q_data-
> >enqueue_status));
> > +	for (i = 0; i < RTE_BBDEV_ENQ_STATUS_SIZE_MAX; i++)
> > +		if (q_data->queue_stats.enqueue_status_count[i] > 0)
> > +			fprintf(f, "  Enqueue Status Counters %s %" PRIu64
> "\n",
> > +					rte_bbdev_enqueue_status_str(i),
> > +					q_data-
> >queue_stats.enqueue_status_count[i]);
> > +	stats = &dev->data->queues[queue_id].queue_stats;
> > +
> > +	fprintf(f, "  Enqueue Count %" PRIu64 " Warning %" PRIu64 " Error %"
> PRIu64 "\n",
> > +			stats->enqueued_count, stats->enqueue_warn_count,
> > +			stats->enqueue_err_count);
> > +	fprintf(f, "  Dequeue Count %" PRIu64 " Warning %" PRIu64 " Error %"
> PRIu64 "\n",
> > +			stats->dequeued_count, stats->dequeue_warn_count,
> > +			stats->dequeue_err_count);
> > +
> > +	return dev->dev_ops->queue_ops_dump(dev, queue_id, f); }
> > +
> > +char *
> > +rte_bbdev_ops_param_string(void *op, enum rte_bbdev_op_type op_type)
> > +{
> > +	static char str[1024];
> > +	static char partial[1024];
> 
> It is better to have the string allocated by the application, it would be passed as
> a pointer and length of the allocated buffer would be passed also as an
> argument.

Ok. Updated in v3. Thanks

> 
> > +	struct rte_bbdev_dec_op *op_dec;
> > +	struct rte_bbdev_enc_op *op_enc;
> > +	struct rte_bbdev_fft_op *op_fft;
> > +	struct rte_bbdev_mldts_op *op_mldts;
> > +
> > +	rte_iova_t add0 = 0, add1 = 0, add2 = 0, add3 = 0, add4 = 0;
> > +
> > +	if (op == NULL) {
> > +		snprintf(str, sizeof(str), "Invalid Operation pointer\n");
> > +		return str;
> > +	}
> > +
> > +	if (op_type == RTE_BBDEV_OP_LDPC_DEC) {
> > +		op_dec = op;
> > +		if (op_dec->ldpc_dec.code_block_mode ==
> RTE_BBDEV_TRANSPORT_BLOCK)
> > +			snprintf(partial, sizeof(partial), "C %d Cab %d Ea %d Eb
> %d r %d",
> > +					op_dec->ldpc_dec.tb_params.c,
> > +					op_dec->ldpc_dec.tb_params.cab,
> > +					op_dec->ldpc_dec.tb_params.ea,
> > +					op_dec->ldpc_dec.tb_params.eb,
> > +					op_dec->ldpc_dec.tb_params.r);
> > +		else
> > +			snprintf(partial, sizeof(partial), "E %d", op_dec-
> >ldpc_dec.cb_params.e);
> > +		if (op_dec->ldpc_dec.input.data != NULL)
> > +			add0 = rte_pktmbuf_iova_offset(op_dec-
> >ldpc_dec.input.data, 0);
> > +		if (op_dec->ldpc_dec.hard_output.data != NULL)
> > +			add1 = rte_pktmbuf_iova_offset(op_dec-
> >ldpc_dec.hard_output.data, 0);
> > +		if (op_dec->ldpc_dec.soft_output.data != NULL)
> > +			add2 = rte_pktmbuf_iova_offset(op_dec-
> >ldpc_dec.soft_output.data, 0);
> > +		if (op_dec->ldpc_dec.harq_combined_input.data != NULL)
> > +			add3 = rte_pktmbuf_iova_offset(op_dec-
> >ldpc_dec.harq_combined_input.data,
> > +					0);
> > +		if (op_dec->ldpc_dec.harq_combined_output.data != NULL)
> > +			add4 = rte_pktmbuf_iova_offset(op_dec-
> >ldpc_dec.harq_combined_output.data,
> > +					0);
> > +		snprintf(str, sizeof(str), "op %x st %x BG %d Zc %d Ncb %d qm
> %d F %d Rv %d It %d It %d "
> > +			"HARQin %d in %" PRIx64 " ho %" PRIx64 " so %"
> PRIx64 " hi %" PRIx64 " "
> > +			"ho %" PRIx64 " %s\n",
> > +			op_dec->ldpc_dec.op_flags, op_dec->status,
> > +			op_dec->ldpc_dec.basegraph, op_dec->ldpc_dec.z_c,
> > +			op_dec->ldpc_dec.n_cb, op_dec->ldpc_dec.q_m,
> > +			op_dec->ldpc_dec.n_filler, op_dec-
> >ldpc_dec.rv_index,
> > +			op_dec->ldpc_dec.iter_max, op_dec-
> >ldpc_dec.iter_count,
> > +			op_dec->ldpc_dec.harq_combined_input.length,
> > +			add0, add1, add2, add3, add4, partial);
> > +	} else if (op_type == RTE_BBDEV_OP_TURBO_DEC) {
> > +		op_dec = op;
> > +		if (op_dec->turbo_dec.code_block_mode ==
> RTE_BBDEV_TRANSPORT_BLOCK)
> > +			snprintf(partial, sizeof(partial), "C %d Cab %d Ea %d Eb
> %d r %d K %d",
> > +					op_dec->turbo_dec.tb_params.c,
> > +					op_dec->turbo_dec.tb_params.cab,
> > +					op_dec->turbo_dec.tb_params.ea,
> > +					op_dec->turbo_dec.tb_params.eb,
> > +					op_dec->turbo_dec.tb_params.r,
> > +					op_dec-
> >turbo_dec.tb_params.k_neg);
> > +		else
> > +			snprintf(partial, sizeof(partial), "E %d K %d",
> > +					op_dec->turbo_dec.cb_params.e,
> > +					op_dec->turbo_dec.cb_params.k);
> > +		if (op_dec->turbo_dec.input.data != NULL)
> > +			add0 = rte_pktmbuf_iova_offset(op_dec-
> >turbo_dec.input.data, 0);
> > +		if (op_dec->turbo_dec.hard_output.data != NULL)
> > +			add1 = rte_pktmbuf_iova_offset(op_dec-
> >turbo_dec.hard_output.data, 0);
> > +		if (op_dec->turbo_dec.soft_output.data != NULL)
> > +			add2 = rte_pktmbuf_iova_offset(op_dec-
> >turbo_dec.soft_output.data, 0);
> > +		snprintf(str, sizeof(str), "op %x st %x CBM %d Iter %d map %d
> Rv %d ext %d "
> > +				"in %" PRIx64 " ho %" PRIx64 " so %" PRIx64 "
> %s\n",
> > +				op_dec->turbo_dec.op_flags, op_dec->status,
> > +				op_dec->turbo_dec.code_block_mode,
> > +				op_dec->turbo_dec.iter_max, op_dec-
> >turbo_dec.num_maps,
> > +				op_dec->turbo_dec.rv_index, op_dec-
> >turbo_dec.ext_scale,
> > +				add0, add1, add2, partial);
> > +	} else if (op_type == RTE_BBDEV_OP_LDPC_ENC) {
> > +		op_enc = op;
> > +		if (op_enc->ldpc_enc.code_block_mode ==
> RTE_BBDEV_TRANSPORT_BLOCK)
> > +			snprintf(partial, sizeof(partial), "C %d Cab %d Ea %d Eb
> %d r %d",
> > +					op_enc->ldpc_enc.tb_params.c,
> > +					op_enc->ldpc_enc.tb_params.cab,
> > +					op_enc->ldpc_enc.tb_params.ea,
> > +					op_enc->ldpc_enc.tb_params.eb,
> > +					op_enc->ldpc_enc.tb_params.r);
> > +		else
> > +			snprintf(partial, sizeof(partial), "E %d",
> > +					op_enc->ldpc_enc.cb_params.e);
> > +		if (op_enc->ldpc_enc.input.data != NULL)
> > +			add0 = rte_pktmbuf_iova_offset(op_enc-
> >ldpc_enc.input.data, 0);
> > +		if (op_enc->ldpc_enc.output.data != NULL)
> > +			add1 = rte_pktmbuf_iova_offset(op_enc-
> >ldpc_enc.output.data, 0);
> > +		snprintf(str, sizeof(str), "op %x st %x BG %d Zc %d Ncb %d q_m
> %d F %d Rv %d "
> > +				"in %" PRIx64 " out %" PRIx64 " %s\n",
> > +				op_enc->ldpc_enc.op_flags, op_enc->status,
> > +				op_enc->ldpc_enc.basegraph, op_enc-
> >ldpc_enc.z_c,
> > +				op_enc->ldpc_enc.n_cb, op_enc-
> >ldpc_enc.q_m,
> > +				op_enc->ldpc_enc.n_filler, op_enc-
> >ldpc_enc.rv_index,
> > +				add0, add1, partial);
> > +	} else if (op_type == RTE_BBDEV_OP_TURBO_ENC) {
> > +		op_enc = op;
> > +		if (op_enc->turbo_enc.code_block_mode ==
> RTE_BBDEV_TRANSPORT_BLOCK)
> > +			snprintf(partial, sizeof(partial),
> > +					"C %d Cab %d Ea %d Eb %d r %d K %d
> Ncb %d",
> > +					op_enc->turbo_enc.tb_params.c,
> > +					op_enc->turbo_enc.tb_params.cab,
> > +					op_enc->turbo_enc.tb_params.ea,
> > +					op_enc->turbo_enc.tb_params.eb,
> > +					op_enc->turbo_enc.tb_params.r,
> > +					op_enc->turbo_enc.tb_params.k_neg,
> > +					op_enc-
> >turbo_enc.tb_params.ncb_neg);
> > +		else
> > +			snprintf(partial, sizeof(partial), "E %d K %d",
> > +					op_enc->turbo_enc.cb_params.e,
> > +					op_enc->turbo_enc.cb_params.k);
> > +		if (op_enc->turbo_enc.input.data != NULL)
> > +			add0 = rte_pktmbuf_iova_offset(op_enc-
> >turbo_enc.input.data, 0);
> > +		if (op_enc->turbo_enc.output.data != NULL)
> > +			add1 = rte_pktmbuf_iova_offset(op_enc-
> >turbo_enc.output.data, 0);
> > +		snprintf(str, sizeof(str), "op %x st %x CBM %d Rv %d In %"
> PRIx64 " Out %" PRIx64 " %s\n",
> > +				op_enc->turbo_enc.op_flags, op_enc->status,
> > +				op_enc->turbo_enc.code_block_mode,
> op_enc->turbo_enc.rv_index,
> > +				add0, add1, partial);
> > +	} else if (op_type == RTE_BBDEV_OP_FFT) {
> > +		op_fft = op;
> > +		if (op_fft->fft.base_input.data != NULL)
> > +			add0 = rte_pktmbuf_iova_offset(op_fft-
> >fft.base_input.data, 0);
> > +		if (op_fft->fft.base_output.data != NULL)
> > +			add1 = rte_pktmbuf_iova_offset(op_fft-
> >fft.base_output.data, 0);
> > +		if (op_fft->fft.dewindowing_input.data != NULL)
> > +			add2 = rte_pktmbuf_iova_offset(op_fft-
> >fft.dewindowing_input.data, 0);
> > +		if (op_fft->fft.power_meas_output.data != NULL)
> > +			add3 = rte_pktmbuf_iova_offset(op_fft-
> >fft.power_meas_output.data, 0);
> > +		snprintf(str, sizeof(str), "op %x st %x in %d inl %d out %d outl
> %d cs %x ants %d "
> > +				"idft %d dft %d cst %d ish %d dsh %d ncs %d
> pwsh %d fp16 %d fr %d "
> > +				"outde %d in %" PRIx64 " out %" PRIx64 " dw
> %" PRIx64 " "
> > +				"pm %" PRIx64 "\n",
> > +				op_fft->fft.op_flags, op_fft->status,
> > +				op_fft->fft.input_sequence_size, op_fft-
> >fft.input_leading_padding,
> > +				op_fft->fft.output_sequence_size,
> > +				op_fft->fft.output_leading_depadding,
> > +				op_fft->fft.cs_bitmap, op_fft-
> >fft.num_antennas_log2,
> > +				op_fft->fft.idft_log2, op_fft->fft.dft_log2,
> > +				op_fft->fft.cs_time_adjustment,
> > +				op_fft->fft.idft_shift, op_fft->fft.dft_shift,
> > +				op_fft->fft.ncs_reciprocal, op_fft-
> >fft.power_shift,
> > +				op_fft->fft.fp16_exp_adjust, op_fft-
> >fft.freq_resample_mode,
> > +				op_fft->fft.output_depadded_size, add0, add1,
> add2, add3);
> > +	} else if (op_type == RTE_BBDEV_OP_MLDTS) {
> > +		op_mldts = op;
> > +		if (op_mldts->mldts.qhy_input.data != NULL)
> > +			add0 = rte_pktmbuf_iova_offset(op_mldts-
> >mldts.qhy_input.data, 0);
> > +		if (op_mldts->mldts.r_input.data != NULL)
> > +			add1 = rte_pktmbuf_iova_offset(op_mldts-
> >mldts.r_input.data, 0);
> > +		if (op_mldts->mldts.output.data != NULL)
> > +			add2 = rte_pktmbuf_iova_offset(op_mldts-
> >mldts.output.data, 0);
> > +		snprintf(str, sizeof(str),
> > +				"op %x st %x rbs %d lay %d rrep %d crep%d
> qm %d %d %d %d "
> > +				"qhy %" PRIx64 " r %" PRIx64 " out %" PRIx64
> "\n",
> > +				op_mldts->mldts.op_flags, op_mldts->status,
> > +				op_mldts->mldts.num_rbs, op_mldts-
> >mldts.num_layers,
> > +				op_mldts->mldts.r_rep, op_mldts-
> >mldts.c_rep,
> > +				op_mldts->mldts.q_m[0], op_mldts-
> >mldts.q_m[1],
> > +				op_mldts->mldts.q_m[2], op_mldts-
> >mldts.q_m[3],
> > +				add0, add1, add2);
> > +
> > +	} else {
> > +		snprintf(str, sizeof(str), "Invalid Operation type %d\n",
> op_type);
> > +	}
> > +
> > +	return str;
> > +}
> > diff --git a/lib/bbdev/rte_bbdev.h b/lib/bbdev/rte_bbdev.h index
> > 0cbfdd1c95..a70d841379 100644
> > --- a/lib/bbdev/rte_bbdev.h
> > +++ b/lib/bbdev/rte_bbdev.h
> > @@ -1061,6 +1061,47 @@ rte_bbdev_device_status_str(enum
> rte_bbdev_device_status status);
> >   const char*
> >   rte_bbdev_enqueue_status_str(enum rte_bbdev_enqueue_status status);
> >
> > +/**
> > + * Dump operations info from device to a file.
> > + * This API is used for debugging provided input operations, not a dataplane
> API.
> > + *
> > + *  @param dev_id
> > + *    The device identifier.
> > + *
> > + *  @param queue_index
> > + *    Index of queue.
> > + *
> > + *  @param file
> > + *    A pointer to a file for output.
> > + *
> > + * @returns
> > + *   - 0 on success
> > + *   - ENOTSUP if interrupts are not supported by the identified device
> > + *   - negative value on failure - as returned from PMD
> > + *
> > + */
> > +__rte_experimental
> > +int
> > +rte_bbdev_queue_ops_dump(uint16_t dev_id, uint16_t queue_index, FILE
> > +*file);
> > +
> > +
> > +/**
> > + * String of parameters related to the parameters of an operation of a given
> type.
> > + *
> > + *  @param op
> > + *    Pointer to an operation.
> > + *
> > + *  @param op_type
> > + *    Operation type enum.
> > + *
> > + * @returns
> > + *   String describing the provided operation.
> > + *
> > + */
> > +__rte_experimental
> > +char *
> > +rte_bbdev_ops_param_string(void *op, enum rte_bbdev_op_type op_type);
> > +
> >   #ifdef __cplusplus
> >   }
> >   #endif
> > diff --git a/lib/bbdev/rte_bbdev_pmd.h b/lib/bbdev/rte_bbdev_pmd.h
> > index 442b23943d..c21a7f0c1e 100644
> > --- a/lib/bbdev/rte_bbdev_pmd.h
> > +++ b/lib/bbdev/rte_bbdev_pmd.h
> > @@ -133,6 +133,13 @@ typedef int
> (*rte_bbdev_queue_intr_enable_t)(struct rte_bbdev *dev,
> >   typedef int (*rte_bbdev_queue_intr_disable_t)(struct rte_bbdev *dev,
> >   				    uint16_t queue_id);
> >
> > +/*
> > + * @internal
> > + * Function to dump previous operations on a queue of a device.
> > + */
> > +typedef int (*rte_bbdev_queue_ops_dump_t)(struct rte_bbdev *dev,
> > +		uint16_t queue_id, FILE *file);
> > +
> >   /**
> >    * Operations implemented by drivers. Fields marked as "Required" must be
> >    * provided by a driver for a device to have basic functionality. "Optional"
> > @@ -170,6 +177,8 @@ struct rte_bbdev_ops {
> >   	rte_bbdev_queue_intr_enable_t queue_intr_enable;
> >   	/** Disable queue interrupt. Optional */
> >   	rte_bbdev_queue_intr_disable_t queue_intr_disable;
> > +	/** Dump operations on the queue. Optional */
> > +	rte_bbdev_queue_ops_dump_t queue_ops_dump;
> >   };
> >
> >   /**
> > diff --git a/lib/bbdev/version.map b/lib/bbdev/version.map index
> > e0d82ff752..db6dfdf3c6 100644
> > --- a/lib/bbdev/version.map
> > +++ b/lib/bbdev/version.map
> > @@ -54,4 +54,8 @@ EXPERIMENTAL {
> >   	rte_bbdev_enqueue_mldts_ops;
> >   	rte_bbdev_mldts_op_alloc_bulk;
> >   	rte_bbdev_mldts_op_free_bulk;
> > +
> > +	# added in 24.11
> > +	rte_bbdev_queue_ops_dump;
> > +	rte_bbdev_ops_param_string;
> >   };

Re: [PATCH v2 2/2] baseband/acc: improvement to logging mechanism

[Maxime Coquelin]

On 9/13/24 21:00, Chautru, Nicolas wrote:
> Hi Maxime,
> 
>> -----Original Message-----
>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Sent: Friday, September 13, 2024 7:18 AM
>> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org
>> Cc: hemant.agrawal@nxp.com; Marchand, David
>> <david.marchand@redhat.com>; Vargas, Hernan <hernan.vargas@intel.com>
>> Subject: Re: [PATCH v2 2/2] baseband/acc: improvement to logging
>> mechanism
>>
>>
>>
>> On 8/29/24 22:06, Nicolas Chautru wrote:
>>> Support the new dev op to dump operations information related to a
>>> given queue and information on previous errors detected by the driver
>>> and tracked internally in PMD.
>>>
>>> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
>>> ---
>>>    drivers/baseband/acc/acc_common.h  |  37 +++++++++
>>>    drivers/baseband/acc/rte_vrb_pmd.c | 128 +++++++++++++++++++++++-----
>> -
>>>    2 files changed, 138 insertions(+), 27 deletions(-)
>>>
>>> diff --git a/drivers/baseband/acc/acc_common.h
>>> b/drivers/baseband/acc/acc_common.h
>>> index e249f37e38..c8914b23e0 100644
>>> --- a/drivers/baseband/acc/acc_common.h
>>> +++ b/drivers/baseband/acc/acc_common.h
>>> @@ -149,6 +149,8 @@
>>>    #define VRB2_VF_ID_SHIFT     6
>>>
>>>    #define ACC_MAX_FFT_WIN      16
>>> +#define ACC_MAX_LOGLEN    256
>>> +#define ACC_MAX_BUFFERLEN 256
>>>
>>>    extern int acc_common_logtype;
>>>
>>> @@ -646,8 +648,43 @@ struct __rte_cache_aligned acc_queue {
>>>    	rte_iova_t fcw_ring_addr_iova;
>>>    	int8_t *derm_buffer; /* interim buffer for de-rm in SDK */
>>>    	struct acc_device *d;
>>> +	char error_bufs[ACC_MAX_BUFFERLEN][ACC_MAX_LOGLEN]; /**<
>> Buffer for
>>> +error log. */
>>
>> 65KB is quite big for each queues
>>
>> I don't see the point of all this, as errors are already logged using
>> rte_bbdev_log. Only thing you miss is the queue/device info in curretn
>> solution, no?
> 
> The rte_bbdev_log is only for std_output which would be disabled by default for real time application so very limited usability.
> Here we store the related information for the application to be able to retrieve themselves as required after the fact (not impacting real time).

If you really care about realtime, then the objects should be saved in
binary form, and post-processed into strings in the dump function.

Doing this, no wasted CPU cycles in the datapath performing strings 
processing, and less memory wasted.

> 
>>
>>> +	uint16_t error_head;  /**< Head - Buffer for error log. */
>>> +	uint16_t  error_wrap; /**< Wrap Counter - Buffer for error log. */
>>>    };
>>>
>>> +/**
>>> + * @brief Report error both through RTE logging and into internal driver
>> memory.
>>> + *
>>> + * This function is used to log an error for a specific ACC queue and
>> operation.
>>> + *
>>> + * @param q   Pointer to the ACC queue.
>>> + * @param op  Pointer to the operation.
>>> + * @param fmt Format string for the error message.
>>> + * @param ... Additional arguments for the format string.
>>> + */
>>> +__rte_format_printf(3, 4)
>>> +static inline void
>>> +acc_error_log(struct acc_queue *q, void *op, const char *fmt, ...) {
>>> +	va_list args, args2;
>>> +
>>> +	va_start(args, fmt);
>>> +	va_copy(args2, args);
>>> +	rte_vlog(RTE_LOG_ERR, acc_common_logtype, fmt, args);
>>> +	vsnprintf(q->error_bufs[q->error_head], ACC_MAX_LOGLEN, fmt,
>> args2);
>>> +	q->error_head++;
>>> +	snprintf(q->error_bufs[q->error_head], ACC_MAX_LOGLEN,
>>> +			"%s", rte_bbdev_ops_param_string(op, q->op_type));
>>> +	q->error_head++;
>>> +	if (q->error_head == ACC_MAX_LOGLEN) {
>>> +		q->error_head = 0;
>>> +		q->error_wrap++;
>>> +	}
>>> +	va_end(args);
>>> +	va_end(args2);
>>> +}
>>> +
>>>    /* Write to MMIO register address */
>>>    static inline void
>>>    mmio_write(void *addr, uint32_t value) diff --git
>>> a/drivers/baseband/acc/rte_vrb_pmd.c
>>> b/drivers/baseband/acc/rte_vrb_pmd.c
>>> index 585dc49bd6..9d0145d09b 100644
>>> --- a/drivers/baseband/acc/rte_vrb_pmd.c
>>> +++ b/drivers/baseband/acc/rte_vrb_pmd.c
>>> @@ -1022,6 +1022,10 @@ vrb_queue_setup(struct rte_bbdev *dev,
>> uint16_t queue_id,
>>>    	q->mmio_reg_enqueue = RTE_PTR_ADD(d->mmio_base,
>>>    			d->queue_offset(d->pf_device, q->vf_id, q->qgrp_id, q-
>>> aq_id));
>>>
>>> +	/** initialize the error buffer. */
>>> +	q->error_head = 0;
>>> +	q->error_wrap = 0;
>>> +
>>>    	rte_bbdev_log_debug(
>>>    			"Setup dev%u q%u: qgrp_id=%u, vf_id=%u, aq_id=%u,
>> aq_depth=%u, mmio_reg_enqueue=%p base %p\n",
>>>    			dev->data->dev_id, queue_id, q->qgrp_id, q->vf_id,
>> @@ -1434,6
>>> +1438,74 @@ vrb_queue_intr_disable(struct rte_bbdev *dev, uint16_t
>> queue_id)
>>>    	return 0;
>>>    }
>>>
>>> +
>>> +static int
>>> +vrb_queue_ops_dump(struct rte_bbdev *dev, uint16_t queue_id, FILE *f)
>>> +{
>>> +	struct acc_queue *q = dev->data->queues[queue_id].queue_private;
>>> +	struct rte_bbdev_dec_op *op;
>>> +	uint16_t start_err, end_err, i, int_nb;
>>> +	volatile union acc_info_ring_data *ring_data;
>>> +	uint16_t info_ring_head = q->d->info_ring_head;
>>> +
>>> +	if (f == NULL) {
>>> +		rte_bbdev_log(ERR, "Invalid File input");
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	/** Print generic information on queue status. */
>>> +	fprintf(f, "Dump of operations %s on Queue %d by %s\n",
>>> +			rte_bbdev_op_type_str(q->op_type), queue_id, dev-
>>> device->driver->name);
>>> +	fprintf(f, "    AQ Enqueued %d Dequeued %d Depth %d - Available Enq
>> %d Deq %d\n",
>>> +			q->aq_enqueued, q->aq_dequeued, q->aq_depth,
>>> +			acc_ring_avail_enq(q), acc_ring_avail_deq(q));
>>> +
>>> +	/** Print information captured in the error buffer. */
>>> +	if (q->error_wrap == 0) {
>>> +		start_err = 0;
>>> +		end_err = q->error_head;
>>> +	} else {
>>> +		start_err = q->error_head;
>>> +		end_err = q->error_head + ACC_MAX_BUFFERLEN;
>>> +	}
>>> +	fprintf(f, "Error Buffer - Head %d Wrap %d\n", q->error_head, q-
>>> error_wrap);
>>> +	for (i = start_err; i < end_err; ++i)
>>> +		fprintf(f, "  %d\t%s", i, q->error_bufs[i %
>> ACC_MAX_BUFFERLEN]);
>>> +
>>> +	/** Print information captured in the info ring. */
>>> +	if (q->d->info_ring != NULL) {
>>> +		fprintf(f, "Info Ring Buffer - Head %d\n", q->d-
>>> info_ring_head);
>>> +		ring_data = q->d->info_ring + (q->d->info_ring_head &
>> ACC_INFO_RING_MASK);
>>> +		while (ring_data->valid) {
>>> +			int_nb = int_from_ring(*ring_data, q->d-
>>> device_variant);
>>> +			if ((int_nb < ACC_PF_INT_DMA_DL_DESC_IRQ) || (
>>> +					int_nb >
>> ACC_PF_INT_DMA_MLD_DESC_IRQ)) {
>>> +				fprintf(f, "  InfoRing: ITR:%d Info:0x%x",
>>> +						int_nb, ring_data-
>>> detailed_info);
>>> +				/* Initialize Info Ring entry and move forward.
>> */
>>> +				ring_data->valid = 0;
>>> +			}
>>> +			info_ring_head++;
>>> +			ring_data = q->d->info_ring + (info_ring_head &
>> ACC_INFO_RING_MASK);
>>> +		}
>>> +	}
>>> +
>>> +	fprintf(f, "Ring Content - Head %d Tail %d Depth %d\n",
>>> +			q->sw_ring_head, q->sw_ring_tail, q->sw_ring_depth);
>>> +	/** Print information about each operation in the software ring. */
>>> +	for (i = 0; i < q->sw_ring_depth; ++i) {
>>> +		op = (q->ring_addr + i)->req.op_addr;
>>> +		if (op != NULL)
>>> +			fprintf(f, "  %d\tn %d %s",
>>> +					i, (q->ring_addr + i)->req.numCBs,
>>> +					rte_bbdev_ops_param_string(op, q-
>>> op_type));
>>> +	}
>>> +
>>> +	fprintf(f, "== End of File ==\n");
>>> +
>>> +	return 0;
>>> +}
>>> +
>>>    static const struct rte_bbdev_ops vrb_bbdev_ops = {
>>>    	.setup_queues = vrb_setup_queues,
>>>    	.intr_enable = vrb_intr_enable,
>>> @@ -1443,7 +1515,8 @@ static const struct rte_bbdev_ops vrb_bbdev_ops
>> = {
>>>    	.queue_release = vrb_queue_release,
>>>    	.queue_stop = vrb_queue_stop,
>>>    	.queue_intr_enable = vrb_queue_intr_enable,
>>> -	.queue_intr_disable = vrb_queue_intr_disable
>>> +	.queue_intr_disable = vrb_queue_intr_disable,
>>> +	.queue_ops_dump = vrb_queue_ops_dump
>>>    };
>>>
>>>    /* PCI PF address map. */
>>> @@ -1680,7 +1753,7 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op
>> *op,
>>>    		uint32_t *in_offset, uint32_t *h_out_offset,
>>>    		uint32_t *s_out_offset, uint32_t *h_out_length,
>>>    		uint32_t *s_out_length, uint32_t *mbuf_total_left,
>>> -		uint32_t *seg_total_left, uint8_t r)
>>> +		uint32_t *seg_total_left, uint8_t r, struct acc_queue *q)
>>>    {
>>>    	int next_triplet = 1; /* FCW already done. */
>>>    	uint16_t k;
>>> @@ -1724,8 +1797,8 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op
>> *op,
>>>    	kw = RTE_ALIGN_CEIL(k + 4, 32) * 3;
>>>
>>>    	if (unlikely((*mbuf_total_left == 0) || (*mbuf_total_left < kw))) {
>>> -		rte_bbdev_log(ERR,
>>> -				"Mismatch between mbuf length and included
>> CB sizes: mbuf len %u, cb len %u",
>>> +		acc_error_log(q, (void *)op,
>>> +				"Mismatch between mbuf length and included
>> CB sizes: mbuf len %u,
>>> +cb len %u\n",
>>>    				*mbuf_total_left, kw);
>>>    		return -1;
>>>    	}
>>> @@ -1735,8 +1808,8 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op
>> *op,
>>>    			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",
>>> +		acc_error_log(q, (void *)op,
>>> +				"Mismatch between data to process and mbuf
>> data length in
>>> +bbdev_op: %p\n",
>>>    				op);
>>>    		return -1;
>>>    	}
>>> @@ -1748,7 +1821,7 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op
>> *op,
>>>    			desc, h_output, *h_out_offset,
>>>    			*h_out_length, next_triplet,
>> ACC_DMA_BLKID_OUT_HARD);
>>>    	if (unlikely(next_triplet < 0)) {
>>> -		rte_bbdev_log(ERR,
>>> +		acc_error_log(q, (void *)op,
>>>    				"Mismatch between data to process and mbuf
>> data length in bbdev_op: %p",
>>>    				op);
>>>    		return -1;
>>> @@ -1760,7 +1833,7 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op
>> *op,
>>>    	/* Soft output. */
>>>    	if (check_bit(op->turbo_dec.op_flags,
>> RTE_BBDEV_TURBO_SOFT_OUTPUT)) {
>>>    		if (op->turbo_dec.soft_output.data == 0) {
>>> -			rte_bbdev_log(ERR, "Soft output is not defined");
>>> +			acc_error_log(q, (void *)op, "Soft output is not
>> defined\n");
>>>    			return -1;
>>>    		}
>>>    		if (check_bit(op->turbo_dec.op_flags,
>>> @@ -1773,8 +1846,8 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op
>> *op,
>>>    				*s_out_offset, *s_out_length, next_triplet,
>>>    				ACC_DMA_BLKID_OUT_SOFT);
>>>    		if (unlikely(next_triplet < 0)) {
>>> -			rte_bbdev_log(ERR,
>>> -					"Mismatch between data to process
>> and mbuf data length in bbdev_op: %p",
>>> +			acc_error_log(q, (void *)op,
>>> +					"Mismatch between data to process
>> and mbuf data length in
>>> +bbdev_op: %p\n",
>>>    					op);
>>>    			return -1;
>>>    		}
>>> @@ -1797,7 +1870,8 @@ vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op
>> *op,
>>>    		struct rte_mbuf **input, struct rte_mbuf *h_output,
>>>    		uint32_t *in_offset, uint32_t *h_out_offset,
>>>    		uint32_t *h_out_length, uint32_t *mbuf_total_left,
>>> -		uint32_t *seg_total_left, struct acc_fcw_ld *fcw, uint16_t
>> device_variant)
>>> +		uint32_t *seg_total_left, struct acc_fcw_ld *fcw, uint16_t
>> device_variant,
>>> +		struct acc_queue *q)
>>>    {
>>>    	struct rte_bbdev_op_ldpc_dec *dec = &op->ldpc_dec;
>>>    	int next_triplet = 1; /* FCW already done. */ @@ -1808,8 +1882,8
>> @@
>>> vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op *op,
>>>    	if (device_variant == VRB1_VARIANT) {
>>>    		if (check_bit(op->ldpc_dec.op_flags,
>> RTE_BBDEV_LDPC_HARQ_4BIT_COMPRESSION) ||
>>>    				check_bit(op->ldpc_dec.op_flags,
>> RTE_BBDEV_LDPC_SOFT_OUT_ENABLE)) {
>>> -			rte_bbdev_log(ERR,
>>> -					"VRB1 does not support the requested
>> capabilities %x",
>>> +			acc_error_log(q, (void *)op,
>>> +					"VRB1 does not support the requested
>> capabilities %x\n",

You should not re-introduce "\n", David is currently working on getting
rid off them all.

>>>    					op->ldpc_dec.op_flags);
>>>    			return -1;
>>>    		}
>>> @@ -1829,8 +1903,8 @@ vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op
>> *op,
>>>    	output_length = K - dec->n_filler - crc24_overlap;
>>>
>>>    	if (unlikely((*mbuf_total_left == 0) || (*mbuf_total_left <
>> input_length))) {
>>> -		rte_bbdev_log(ERR,
>>> -				"Mismatch between mbuf length and included
>> CB sizes: mbuf len %u, cb len %u",
>>> +		acc_error_log(q, (void *)op,
>>> +				"Mismatch between mbuf length and included
>> CB sizes: mbuf len %u,
>>> +cb len %u\n",
>>>    				*mbuf_total_left, input_length);
>>>    		return -1;
>>>    	}
>>> @@ -1842,15 +1916,15 @@ vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op
>> *op,
>>>    			RTE_BBDEV_LDPC_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",
>>> +		acc_error_log(q, (void *)op,
>>> +				"Mismatch between data to process and mbuf
>> data length in
>>> +bbdev_op: %p\n",
>>>    				op);
>>>    		return -1;
>>>    	}
>>>
>>>    	if (check_bit(op->ldpc_dec.op_flags,
>> RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE)) {
>>>    		if (op->ldpc_dec.harq_combined_input.data == 0) {
>>> -			rte_bbdev_log(ERR, "HARQ input is not defined");
>>> +			acc_error_log(q, (void *)op, "HARQ input is not
>> defined\n");
>>>    			return -1;
>>>    		}
>>>    		h_p_size = fcw->hcin_size0 + fcw->hcin_size1; @@ -1859,7
>> +1933,7
>>> @@ vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op *op,
>>>    		else if (fcw->hcin_decomp_mode == 4)
>>>    			h_p_size = h_p_size / 2;
>>>    		if (op->ldpc_dec.harq_combined_input.data == 0) {
>>> -			rte_bbdev_log(ERR, "HARQ input is not defined");
>>> +			acc_error_log(q, (void *)op, "HARQ input is not
>> defined\n");
>>>    			return -1;
>>>    		}
>>>    		acc_dma_fill_blk_type(
>>> @@ -1882,7 +1956,7 @@ vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op
>>> *op,
>>>
>>>    	if (check_bit(op->ldpc_dec.op_flags,
>> RTE_BBDEV_LDPC_SOFT_OUT_ENABLE)) {
>>>    		if (op->ldpc_dec.soft_output.data == 0) {
>>> -			rte_bbdev_log(ERR, "Soft output is not defined");
>>> +			acc_error_log(q, (void *)op, "Soft output is not
>> defined\n");
>>>    			return -1;
>>>    		}
>>>    		dec->soft_output.length = fcw->rm_e; @@ -1894,7 +1968,7
>> @@
>>> vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op *op,
>>>    	if (check_bit(op->ldpc_dec.op_flags,
>>>
>> 	RTE_BBDEV_LDPC_HQ_COMBINE_OUT_ENABLE)) {
>>>    		if (op->ldpc_dec.harq_combined_output.data == 0) {
>>> -			rte_bbdev_log(ERR, "HARQ output is not defined");
>>> +			acc_error_log(q, (void *)op, "HARQ output is not
>> defined\n");
>>>    			return -1;
>>>    		}
>>>
>>> @@ -2326,8 +2400,8 @@ vrb2_enqueue_ldpc_enc_one_op_tb(struct
>> acc_queue *q, struct rte_bbdev_enc_op *op
>>>    			in_length_in_bytes, &seg_total_left, next_triplet,
>>>    			check_bit(enc->op_flags,
>> RTE_BBDEV_LDPC_ENC_SCATTER_GATHER));
>>>    	if (unlikely(next_triplet < 0)) {
>>> -		rte_bbdev_log(ERR,
>>> -				"Mismatch between data to process and mbuf
>> data length in bbdev_op: %p",
>>> +		acc_error_log(q, (void *)op,
>>> +				"Mismatch between data to process and mbuf
>> data length in
>>> +bbdev_op: %p\n",
>>>    				op);
>>>    		return -1;
>>>    	}
>>> @@ -2399,7 +2473,7 @@ enqueue_dec_one_op_cb(struct acc_queue *q,
>> struct rte_bbdev_dec_op *op,
>>>    	ret = vrb_dma_desc_td_fill(op, &desc->req, &input, h_output,
>>>    			s_output, &in_offset, &h_out_offset, &s_out_offset,
>>>    			&h_out_length, &s_out_length, &mbuf_total_left,
>>> -			&seg_total_left, 0);
>>> +			&seg_total_left, 0, q);
>>>
>>>    	if (unlikely(ret < 0))
>>>    		return ret;
>>> @@ -2478,7 +2552,7 @@ vrb_enqueue_ldpc_dec_one_op_cb(struct
>> acc_queue *q, struct rte_bbdev_dec_op *op,
>>>    		ret = vrb_dma_desc_ld_fill(op, &desc->req, &input, h_output,
>>>    				&in_offset, &h_out_offset,
>>>    				&h_out_length, &mbuf_total_left,
>>> -				&seg_total_left, fcw, q->d->device_variant);
>>> +				&seg_total_left, fcw, q->d->device_variant, q);
>>>    		if (unlikely(ret < 0))
>>>    			return ret;
>>>    	}
>>> @@ -2572,7 +2646,7 @@ vrb_enqueue_ldpc_dec_one_op_tb(struct
>> acc_queue *q, struct rte_bbdev_dec_op *op,
>>>    				h_output, &in_offset, &h_out_offset,
>>>    				&h_out_length,
>>>    				&mbuf_total_left, &seg_total_left,
>>> -				&desc->req.fcw_ld, q->d->device_variant);
>>> +				&desc->req.fcw_ld, q->d->device_variant, q);
>>>
>>>    		if (unlikely(ret < 0))
>>>    			return ret;
>>> @@ -2658,7 +2732,7 @@ enqueue_dec_one_op_tb(struct acc_queue *q,
>> struct rte_bbdev_dec_op *op,
>>>    		ret = vrb_dma_desc_td_fill(op, &desc->req, &input,
>>>    				h_output, s_output, &in_offset,
>> &h_out_offset,
>>>    				&s_out_offset, &h_out_length,
>> &s_out_length,
>>> -				&mbuf_total_left, &seg_total_left, r);
>>> +				&mbuf_total_left, &seg_total_left, r, q);
>>>
>>>    		if (unlikely(ret < 0))
>>>    			return ret;
> 

RE: [PATCH v2 2/2] baseband/acc: improvement to logging mechanism

[Chautru, Nicolas]

Hi Maxime, 

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Monday, September 16, 2024 12:57 AM
> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org
> Cc: hemant.agrawal@nxp.com; Marchand, David
> <david.marchand@redhat.com>; Vargas, Hernan <hernan.vargas@intel.com>
> Subject: Re: [PATCH v2 2/2] baseband/acc: improvement to logging mechanism
> 
> 
> 
> On 9/13/24 21:00, Chautru, Nicolas wrote:
> > Hi Maxime,
> >
> >> -----Original Message-----
> >> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> >> Sent: Friday, September 13, 2024 7:18 AM
> >> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org
> >> Cc: hemant.agrawal@nxp.com; Marchand, David
> >> <david.marchand@redhat.com>; Vargas, Hernan
> <hernan.vargas@intel.com>
> >> Subject: Re: [PATCH v2 2/2] baseband/acc: improvement to logging
> >> mechanism
> >>
> >>
> >>
> >> On 8/29/24 22:06, Nicolas Chautru wrote:
> >>> Support the new dev op to dump operations information related to a
> >>> given queue and information on previous errors detected by the
> >>> driver and tracked internally in PMD.
> >>>
> >>> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> >>> ---
> >>>    drivers/baseband/acc/acc_common.h  |  37 +++++++++
> >>>    drivers/baseband/acc/rte_vrb_pmd.c | 128
> >>> +++++++++++++++++++++++-----
> >> -
> >>>    2 files changed, 138 insertions(+), 27 deletions(-)
> >>>
> >>> diff --git a/drivers/baseband/acc/acc_common.h
> >>> b/drivers/baseband/acc/acc_common.h
> >>> index e249f37e38..c8914b23e0 100644
> >>> --- a/drivers/baseband/acc/acc_common.h
> >>> +++ b/drivers/baseband/acc/acc_common.h
> >>> @@ -149,6 +149,8 @@
> >>>    #define VRB2_VF_ID_SHIFT     6
> >>>
> >>>    #define ACC_MAX_FFT_WIN      16
> >>> +#define ACC_MAX_LOGLEN    256
> >>> +#define ACC_MAX_BUFFERLEN 256
> >>>
> >>>    extern int acc_common_logtype;
> >>>
> >>> @@ -646,8 +648,43 @@ struct __rte_cache_aligned acc_queue {
> >>>    	rte_iova_t fcw_ring_addr_iova;
> >>>    	int8_t *derm_buffer; /* interim buffer for de-rm in SDK */
> >>>    	struct acc_device *d;
> >>> +	char error_bufs[ACC_MAX_BUFFERLEN][ACC_MAX_LOGLEN]; /**<
> >> Buffer for
> >>> +error log. */
> >>
> >> 65KB is quite big for each queues
> >>
> >> I don't see the point of all this, as errors are already logged using
> >> rte_bbdev_log. Only thing you miss is the queue/device info in
> >> curretn solution, no?
> >
> > The rte_bbdev_log is only for std_output which would be disabled by default
> for real time application so very limited usability.
> > Here we store the related information for the application to be able to
> retrieve themselves as required after the fact (not impacting real time).
> 
> If you really care about realtime, then the objects should be saved in binary
> form, and post-processed into strings in the dump function.
> 
> Doing this, no wasted CPU cycles in the datapath performing strings
> processing, and less memory wasted.

This is the BKM we use for this real time workload across ingredients, 
best compromise of performance and convenience,
and definitely cannot rely on standard output
Let me know if you would like to discuss more.
Thanks
Nic

> 
> >
> >>
> >>> +	uint16_t error_head;  /**< Head - Buffer for error log. */
> >>> +	uint16_t  error_wrap; /**< Wrap Counter - Buffer for error log. */
> >>>    };
> >>>
> >>> +/**
> >>> + * @brief Report error both through RTE logging and into internal
> >>> +driver
> >> memory.
> >>> + *
> >>> + * This function is used to log an error for a specific ACC queue
> >>> + and
> >> operation.
> >>> + *
> >>> + * @param q   Pointer to the ACC queue.
> >>> + * @param op  Pointer to the operation.
> >>> + * @param fmt Format string for the error message.
> >>> + * @param ... Additional arguments for the format string.
> >>> + */
> >>> +__rte_format_printf(3, 4)
> >>> +static inline void
> >>> +acc_error_log(struct acc_queue *q, void *op, const char *fmt, ...) {
> >>> +	va_list args, args2;
> >>> +
> >>> +	va_start(args, fmt);
> >>> +	va_copy(args2, args);
> >>> +	rte_vlog(RTE_LOG_ERR, acc_common_logtype, fmt, args);
> >>> +	vsnprintf(q->error_bufs[q->error_head], ACC_MAX_LOGLEN, fmt,
> >> args2);
> >>> +	q->error_head++;
> >>> +	snprintf(q->error_bufs[q->error_head], ACC_MAX_LOGLEN,
> >>> +			"%s", rte_bbdev_ops_param_string(op, q->op_type));
> >>> +	q->error_head++;
> >>> +	if (q->error_head == ACC_MAX_LOGLEN) {
> >>> +		q->error_head = 0;
> >>> +		q->error_wrap++;
> >>> +	}
> >>> +	va_end(args);
> >>> +	va_end(args2);
> >>> +}
> >>> +
> >>>    /* Write to MMIO register address */
> >>>    static inline void
> >>>    mmio_write(void *addr, uint32_t value) diff --git
> >>> a/drivers/baseband/acc/rte_vrb_pmd.c
> >>> b/drivers/baseband/acc/rte_vrb_pmd.c
> >>> index 585dc49bd6..9d0145d09b 100644
> >>> --- a/drivers/baseband/acc/rte_vrb_pmd.c
> >>> +++ b/drivers/baseband/acc/rte_vrb_pmd.c
> >>> @@ -1022,6 +1022,10 @@ vrb_queue_setup(struct rte_bbdev *dev,
> >> uint16_t queue_id,
> >>>    	q->mmio_reg_enqueue = RTE_PTR_ADD(d->mmio_base,
> >>>    			d->queue_offset(d->pf_device, q->vf_id, q->qgrp_id,
> q-
> >>> aq_id));
> >>>
> >>> +	/** initialize the error buffer. */
> >>> +	q->error_head = 0;
> >>> +	q->error_wrap = 0;
> >>> +
> >>>    	rte_bbdev_log_debug(
> >>>    			"Setup dev%u q%u: qgrp_id=%u, vf_id=%u, aq_id=%u,
> >> aq_depth=%u, mmio_reg_enqueue=%p base %p\n",
> >>>    			dev->data->dev_id, queue_id, q->qgrp_id, q->vf_id,
> >> @@ -1434,6
> >>> +1438,74 @@ vrb_queue_intr_disable(struct rte_bbdev *dev, uint16_t
> >> queue_id)
> >>>    	return 0;
> >>>    }
> >>>
> >>> +
> >>> +static int
> >>> +vrb_queue_ops_dump(struct rte_bbdev *dev, uint16_t queue_id, FILE
> >>> +*f) {
> >>> +	struct acc_queue *q = dev->data->queues[queue_id].queue_private;
> >>> +	struct rte_bbdev_dec_op *op;
> >>> +	uint16_t start_err, end_err, i, int_nb;
> >>> +	volatile union acc_info_ring_data *ring_data;
> >>> +	uint16_t info_ring_head = q->d->info_ring_head;
> >>> +
> >>> +	if (f == NULL) {
> >>> +		rte_bbdev_log(ERR, "Invalid File input");
> >>> +		return -EINVAL;
> >>> +	}
> >>> +
> >>> +	/** Print generic information on queue status. */
> >>> +	fprintf(f, "Dump of operations %s on Queue %d by %s\n",
> >>> +			rte_bbdev_op_type_str(q->op_type), queue_id, dev-
> >>> device->driver->name);
> >>> +	fprintf(f, "    AQ Enqueued %d Dequeued %d Depth %d - Available Enq
> >> %d Deq %d\n",
> >>> +			q->aq_enqueued, q->aq_dequeued, q->aq_depth,
> >>> +			acc_ring_avail_enq(q), acc_ring_avail_deq(q));
> >>> +
> >>> +	/** Print information captured in the error buffer. */
> >>> +	if (q->error_wrap == 0) {
> >>> +		start_err = 0;
> >>> +		end_err = q->error_head;
> >>> +	} else {
> >>> +		start_err = q->error_head;
> >>> +		end_err = q->error_head + ACC_MAX_BUFFERLEN;
> >>> +	}
> >>> +	fprintf(f, "Error Buffer - Head %d Wrap %d\n", q->error_head, q-
> >>> error_wrap);
> >>> +	for (i = start_err; i < end_err; ++i)
> >>> +		fprintf(f, "  %d\t%s", i, q->error_bufs[i %
> >> ACC_MAX_BUFFERLEN]);
> >>> +
> >>> +	/** Print information captured in the info ring. */
> >>> +	if (q->d->info_ring != NULL) {
> >>> +		fprintf(f, "Info Ring Buffer - Head %d\n", q->d-
> >>> info_ring_head);
> >>> +		ring_data = q->d->info_ring + (q->d->info_ring_head &
> >> ACC_INFO_RING_MASK);
> >>> +		while (ring_data->valid) {
> >>> +			int_nb = int_from_ring(*ring_data, q->d-
> >>> device_variant);
> >>> +			if ((int_nb < ACC_PF_INT_DMA_DL_DESC_IRQ) || (
> >>> +					int_nb >
> >> ACC_PF_INT_DMA_MLD_DESC_IRQ)) {
> >>> +				fprintf(f, "  InfoRing: ITR:%d Info:0x%x",
> >>> +						int_nb, ring_data-
> >>> detailed_info);
> >>> +				/* Initialize Info Ring entry and move forward.
> >> */
> >>> +				ring_data->valid = 0;
> >>> +			}
> >>> +			info_ring_head++;
> >>> +			ring_data = q->d->info_ring + (info_ring_head &
> >> ACC_INFO_RING_MASK);
> >>> +		}
> >>> +	}
> >>> +
> >>> +	fprintf(f, "Ring Content - Head %d Tail %d Depth %d\n",
> >>> +			q->sw_ring_head, q->sw_ring_tail, q->sw_ring_depth);
> >>> +	/** Print information about each operation in the software ring. */
> >>> +	for (i = 0; i < q->sw_ring_depth; ++i) {
> >>> +		op = (q->ring_addr + i)->req.op_addr;
> >>> +		if (op != NULL)
> >>> +			fprintf(f, "  %d\tn %d %s",
> >>> +					i, (q->ring_addr + i)->req.numCBs,
> >>> +					rte_bbdev_ops_param_string(op, q-
> >>> op_type));
> >>> +	}
> >>> +
> >>> +	fprintf(f, "== End of File ==\n");
> >>> +
> >>> +	return 0;
> >>> +}
> >>> +
> >>>    static const struct rte_bbdev_ops vrb_bbdev_ops = {
> >>>    	.setup_queues = vrb_setup_queues,
> >>>    	.intr_enable = vrb_intr_enable,
> >>> @@ -1443,7 +1515,8 @@ static const struct rte_bbdev_ops
> >>> vrb_bbdev_ops
> >> = {
> >>>    	.queue_release = vrb_queue_release,
> >>>    	.queue_stop = vrb_queue_stop,
> >>>    	.queue_intr_enable = vrb_queue_intr_enable,
> >>> -	.queue_intr_disable = vrb_queue_intr_disable
> >>> +	.queue_intr_disable = vrb_queue_intr_disable,
> >>> +	.queue_ops_dump = vrb_queue_ops_dump
> >>>    };
> >>>
> >>>    /* PCI PF address map. */
> >>> @@ -1680,7 +1753,7 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op
> >> *op,
> >>>    		uint32_t *in_offset, uint32_t *h_out_offset,
> >>>    		uint32_t *s_out_offset, uint32_t *h_out_length,
> >>>    		uint32_t *s_out_length, uint32_t *mbuf_total_left,
> >>> -		uint32_t *seg_total_left, uint8_t r)
> >>> +		uint32_t *seg_total_left, uint8_t r, struct acc_queue *q)
> >>>    {
> >>>    	int next_triplet = 1; /* FCW already done. */
> >>>    	uint16_t k;
> >>> @@ -1724,8 +1797,8 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op
> >> *op,
> >>>    	kw = RTE_ALIGN_CEIL(k + 4, 32) * 3;
> >>>
> >>>    	if (unlikely((*mbuf_total_left == 0) || (*mbuf_total_left < kw))) {
> >>> -		rte_bbdev_log(ERR,
> >>> -				"Mismatch between mbuf length and included
> >> CB sizes: mbuf len %u, cb len %u",
> >>> +		acc_error_log(q, (void *)op,
> >>> +				"Mismatch between mbuf length and included
> >> CB sizes: mbuf len %u,
> >>> +cb len %u\n",
> >>>    				*mbuf_total_left, kw);
> >>>    		return -1;
> >>>    	}
> >>> @@ -1735,8 +1808,8 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op
> >> *op,
> >>>    			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",
> >>> +		acc_error_log(q, (void *)op,
> >>> +				"Mismatch between data to process and mbuf
> >> data length in
> >>> +bbdev_op: %p\n",
> >>>    				op);
> >>>    		return -1;
> >>>    	}
> >>> @@ -1748,7 +1821,7 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op
> >> *op,
> >>>    			desc, h_output, *h_out_offset,
> >>>    			*h_out_length, next_triplet,
> >> ACC_DMA_BLKID_OUT_HARD);
> >>>    	if (unlikely(next_triplet < 0)) {
> >>> -		rte_bbdev_log(ERR,
> >>> +		acc_error_log(q, (void *)op,
> >>>    				"Mismatch between data to process and mbuf
> >> data length in bbdev_op: %p",
> >>>    				op);
> >>>    		return -1;
> >>> @@ -1760,7 +1833,7 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op
> >> *op,
> >>>    	/* Soft output. */
> >>>    	if (check_bit(op->turbo_dec.op_flags,
> >> RTE_BBDEV_TURBO_SOFT_OUTPUT)) {
> >>>    		if (op->turbo_dec.soft_output.data == 0) {
> >>> -			rte_bbdev_log(ERR, "Soft output is not defined");
> >>> +			acc_error_log(q, (void *)op, "Soft output is not
> >> defined\n");
> >>>    			return -1;
> >>>    		}
> >>>    		if (check_bit(op->turbo_dec.op_flags,
> >>> @@ -1773,8 +1846,8 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op
> >> *op,
> >>>    				*s_out_offset, *s_out_length, next_triplet,
> >>>    				ACC_DMA_BLKID_OUT_SOFT);
> >>>    		if (unlikely(next_triplet < 0)) {
> >>> -			rte_bbdev_log(ERR,
> >>> -					"Mismatch between data to process
> >> and mbuf data length in bbdev_op: %p",
> >>> +			acc_error_log(q, (void *)op,
> >>> +					"Mismatch between data to process
> >> and mbuf data length in
> >>> +bbdev_op: %p\n",
> >>>    					op);
> >>>    			return -1;
> >>>    		}
> >>> @@ -1797,7 +1870,8 @@ vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op
> >> *op,
> >>>    		struct rte_mbuf **input, struct rte_mbuf *h_output,
> >>>    		uint32_t *in_offset, uint32_t *h_out_offset,
> >>>    		uint32_t *h_out_length, uint32_t *mbuf_total_left,
> >>> -		uint32_t *seg_total_left, struct acc_fcw_ld *fcw, uint16_t
> >> device_variant)
> >>> +		uint32_t *seg_total_left, struct acc_fcw_ld *fcw, uint16_t
> >> device_variant,
> >>> +		struct acc_queue *q)
> >>>    {
> >>>    	struct rte_bbdev_op_ldpc_dec *dec = &op->ldpc_dec;
> >>>    	int next_triplet = 1; /* FCW already done. */ @@ -1808,8 +1882,8
> >> @@
> >>> vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op *op,
> >>>    	if (device_variant == VRB1_VARIANT) {
> >>>    		if (check_bit(op->ldpc_dec.op_flags,
> >> RTE_BBDEV_LDPC_HARQ_4BIT_COMPRESSION) ||
> >>>    				check_bit(op->ldpc_dec.op_flags,
> >> RTE_BBDEV_LDPC_SOFT_OUT_ENABLE)) {
> >>> -			rte_bbdev_log(ERR,
> >>> -					"VRB1 does not support the requested
> >> capabilities %x",
> >>> +			acc_error_log(q, (void *)op,
> >>> +					"VRB1 does not support the requested
> >> capabilities %x\n",
> 
> You should not re-introduce "\n", David is currently working on getting rid off
> them all.
> 
> >>>    					op->ldpc_dec.op_flags);
> >>>    			return -1;
> >>>    		}
> >>> @@ -1829,8 +1903,8 @@ vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op
> >> *op,
> >>>    	output_length = K - dec->n_filler - crc24_overlap;
> >>>
> >>>    	if (unlikely((*mbuf_total_left == 0) || (*mbuf_total_left <
> >> input_length))) {
> >>> -		rte_bbdev_log(ERR,
> >>> -				"Mismatch between mbuf length and included
> >> CB sizes: mbuf len %u, cb len %u",
> >>> +		acc_error_log(q, (void *)op,
> >>> +				"Mismatch between mbuf length and included
> >> CB sizes: mbuf len %u,
> >>> +cb len %u\n",
> >>>    				*mbuf_total_left, input_length);
> >>>    		return -1;
> >>>    	}
> >>> @@ -1842,15 +1916,15 @@ vrb_dma_desc_ld_fill(struct
> rte_bbdev_dec_op
> >> *op,
> >>>    			RTE_BBDEV_LDPC_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",
> >>> +		acc_error_log(q, (void *)op,
> >>> +				"Mismatch between data to process and mbuf
> >> data length in
> >>> +bbdev_op: %p\n",
> >>>    				op);
> >>>    		return -1;
> >>>    	}
> >>>
> >>>    	if (check_bit(op->ldpc_dec.op_flags,
> >> RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE)) {
> >>>    		if (op->ldpc_dec.harq_combined_input.data == 0) {
> >>> -			rte_bbdev_log(ERR, "HARQ input is not defined");
> >>> +			acc_error_log(q, (void *)op, "HARQ input is not
> >> defined\n");
> >>>    			return -1;
> >>>    		}
> >>>    		h_p_size = fcw->hcin_size0 + fcw->hcin_size1; @@ -1859,7
> >> +1933,7
> >>> @@ vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op *op,
> >>>    		else if (fcw->hcin_decomp_mode == 4)
> >>>    			h_p_size = h_p_size / 2;
> >>>    		if (op->ldpc_dec.harq_combined_input.data == 0) {
> >>> -			rte_bbdev_log(ERR, "HARQ input is not defined");
> >>> +			acc_error_log(q, (void *)op, "HARQ input is not
> >> defined\n");
> >>>    			return -1;
> >>>    		}
> >>>    		acc_dma_fill_blk_type(
> >>> @@ -1882,7 +1956,7 @@ vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op
> >>> *op,
> >>>
> >>>    	if (check_bit(op->ldpc_dec.op_flags,
> >> RTE_BBDEV_LDPC_SOFT_OUT_ENABLE)) {
> >>>    		if (op->ldpc_dec.soft_output.data == 0) {
> >>> -			rte_bbdev_log(ERR, "Soft output is not defined");
> >>> +			acc_error_log(q, (void *)op, "Soft output is not
> >> defined\n");
> >>>    			return -1;
> >>>    		}
> >>>    		dec->soft_output.length = fcw->rm_e; @@ -1894,7 +1968,7
> >> @@
> >>> vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op *op,
> >>>    	if (check_bit(op->ldpc_dec.op_flags,
> >>>
> >> 	RTE_BBDEV_LDPC_HQ_COMBINE_OUT_ENABLE)) {
> >>>    		if (op->ldpc_dec.harq_combined_output.data == 0) {
> >>> -			rte_bbdev_log(ERR, "HARQ output is not defined");
> >>> +			acc_error_log(q, (void *)op, "HARQ output is not
> >> defined\n");
> >>>    			return -1;
> >>>    		}
> >>>
> >>> @@ -2326,8 +2400,8 @@ vrb2_enqueue_ldpc_enc_one_op_tb(struct
> >> acc_queue *q, struct rte_bbdev_enc_op *op
> >>>    			in_length_in_bytes, &seg_total_left, next_triplet,
> >>>    			check_bit(enc->op_flags,
> >> RTE_BBDEV_LDPC_ENC_SCATTER_GATHER));
> >>>    	if (unlikely(next_triplet < 0)) {
> >>> -		rte_bbdev_log(ERR,
> >>> -				"Mismatch between data to process and mbuf
> >> data length in bbdev_op: %p",
> >>> +		acc_error_log(q, (void *)op,
> >>> +				"Mismatch between data to process and mbuf
> >> data length in
> >>> +bbdev_op: %p\n",
> >>>    				op);
> >>>    		return -1;
> >>>    	}
> >>> @@ -2399,7 +2473,7 @@ enqueue_dec_one_op_cb(struct acc_queue *q,
> >> struct rte_bbdev_dec_op *op,
> >>>    	ret = vrb_dma_desc_td_fill(op, &desc->req, &input, h_output,
> >>>    			s_output, &in_offset, &h_out_offset, &s_out_offset,
> >>>    			&h_out_length, &s_out_length, &mbuf_total_left,
> >>> -			&seg_total_left, 0);
> >>> +			&seg_total_left, 0, q);
> >>>
> >>>    	if (unlikely(ret < 0))
> >>>    		return ret;
> >>> @@ -2478,7 +2552,7 @@ vrb_enqueue_ldpc_dec_one_op_cb(struct
> >> acc_queue *q, struct rte_bbdev_dec_op *op,
> >>>    		ret = vrb_dma_desc_ld_fill(op, &desc->req, &input, h_output,
> >>>    				&in_offset, &h_out_offset,
> >>>    				&h_out_length, &mbuf_total_left,
> >>> -				&seg_total_left, fcw, q->d->device_variant);
> >>> +				&seg_total_left, fcw, q->d->device_variant, q);
> >>>    		if (unlikely(ret < 0))
> >>>    			return ret;
> >>>    	}
> >>> @@ -2572,7 +2646,7 @@ vrb_enqueue_ldpc_dec_one_op_tb(struct
> >> acc_queue *q, struct rte_bbdev_dec_op *op,
> >>>    				h_output, &in_offset, &h_out_offset,
> >>>    				&h_out_length,
> >>>    				&mbuf_total_left, &seg_total_left,
> >>> -				&desc->req.fcw_ld, q->d->device_variant);
> >>> +				&desc->req.fcw_ld, q->d->device_variant, q);
> >>>
> >>>    		if (unlikely(ret < 0))
> >>>    			return ret;
> >>> @@ -2658,7 +2732,7 @@ enqueue_dec_one_op_tb(struct acc_queue *q,
> >> struct rte_bbdev_dec_op *op,
> >>>    		ret = vrb_dma_desc_td_fill(op, &desc->req, &input,
> >>>    				h_output, s_output, &in_offset,
> >> &h_out_offset,
> >>>    				&s_out_offset, &h_out_length,
> >> &s_out_length,
> >>> -				&mbuf_total_left, &seg_total_left, r);
> >>> +				&mbuf_total_left, &seg_total_left, r, q);
> >>>
> >>>    		if (unlikely(ret < 0))
> >>>    			return ret;
> >

Re: [PATCH v2 2/2] baseband/acc: improvement to logging mechanism

[Maxime Coquelin]

Hi Nicolas,

On 9/16/24 18:17, Chautru, Nicolas wrote:
> Hi Maxime,
> 
>> -----Original Message-----
>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Sent: Monday, September 16, 2024 12:57 AM
>> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org
>> Cc: hemant.agrawal@nxp.com; Marchand, David
>> <david.marchand@redhat.com>; Vargas, Hernan <hernan.vargas@intel.com>
>> Subject: Re: [PATCH v2 2/2] baseband/acc: improvement to logging mechanism
>>
>>
>>
>> On 9/13/24 21:00, Chautru, Nicolas wrote:
>>> Hi Maxime,
>>>
>>>> -----Original Message-----
>>>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>>>> Sent: Friday, September 13, 2024 7:18 AM
>>>> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org
>>>> Cc: hemant.agrawal@nxp.com; Marchand, David
>>>> <david.marchand@redhat.com>; Vargas, Hernan
>> <hernan.vargas@intel.com>
>>>> Subject: Re: [PATCH v2 2/2] baseband/acc: improvement to logging
>>>> mechanism
>>>>
>>>>
>>>>
>>>> On 8/29/24 22:06, Nicolas Chautru wrote:
>>>>> Support the new dev op to dump operations information related to a
>>>>> given queue and information on previous errors detected by the
>>>>> driver and tracked internally in PMD.
>>>>>
>>>>> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
>>>>> ---
>>>>>     drivers/baseband/acc/acc_common.h  |  37 +++++++++
>>>>>     drivers/baseband/acc/rte_vrb_pmd.c | 128
>>>>> +++++++++++++++++++++++-----
>>>> -
>>>>>     2 files changed, 138 insertions(+), 27 deletions(-)
>>>>>
>>>>> diff --git a/drivers/baseband/acc/acc_common.h
>>>>> b/drivers/baseband/acc/acc_common.h
>>>>> index e249f37e38..c8914b23e0 100644
>>>>> --- a/drivers/baseband/acc/acc_common.h
>>>>> +++ b/drivers/baseband/acc/acc_common.h
>>>>> @@ -149,6 +149,8 @@
>>>>>     #define VRB2_VF_ID_SHIFT     6
>>>>>
>>>>>     #define ACC_MAX_FFT_WIN      16
>>>>> +#define ACC_MAX_LOGLEN    256
>>>>> +#define ACC_MAX_BUFFERLEN 256
>>>>>
>>>>>     extern int acc_common_logtype;
>>>>>
>>>>> @@ -646,8 +648,43 @@ struct __rte_cache_aligned acc_queue {
>>>>>     	rte_iova_t fcw_ring_addr_iova;
>>>>>     	int8_t *derm_buffer; /* interim buffer for de-rm in SDK */
>>>>>     	struct acc_device *d;
>>>>> +	char error_bufs[ACC_MAX_BUFFERLEN][ACC_MAX_LOGLEN]; /**<
>>>> Buffer for
>>>>> +error log. */
>>>>
>>>> 65KB is quite big for each queues
>>>>
>>>> I don't see the point of all this, as errors are already logged using
>>>> rte_bbdev_log. Only thing you miss is the queue/device info in
>>>> curretn solution, no?
>>>
>>> The rte_bbdev_log is only for std_output which would be disabled by default
>> for real time application so very limited usability.
>>> Here we store the related information for the application to be able to
>> retrieve themselves as required after the fact (not impacting real time).
>>
>> If you really care about realtime, then the objects should be saved in binary
>> form, and post-processed into strings in the dump function.
>>
>> Doing this, no wasted CPU cycles in the datapath performing strings
>> processing, and less memory wasted.
> 
> This is the BKM we use for this real time workload across ingredients,
> best compromise of performance and convenience,
> and definitely cannot rely on standard output
> Let me know if you would like to discuss more.

I can understand you don't want to use the standard output, but in this
case save the objects as binary form, and let the dump function generate
the strings.

Thanks,
Maxime

> Thanks
> Nic
> 
>>
>>>
>>>>
>>>>> +	uint16_t error_head;  /**< Head - Buffer for error log. */
>>>>> +	uint16_t  error_wrap; /**< Wrap Counter - Buffer for error log. */
>>>>>     };
>>>>>
>>>>> +/**
>>>>> + * @brief Report error both through RTE logging and into internal
>>>>> +driver
>>>> memory.
>>>>> + *
>>>>> + * This function is used to log an error for a specific ACC queue
>>>>> + and
>>>> operation.
>>>>> + *
>>>>> + * @param q   Pointer to the ACC queue.
>>>>> + * @param op  Pointer to the operation.
>>>>> + * @param fmt Format string for the error message.
>>>>> + * @param ... Additional arguments for the format string.
>>>>> + */
>>>>> +__rte_format_printf(3, 4)
>>>>> +static inline void
>>>>> +acc_error_log(struct acc_queue *q, void *op, const char *fmt, ...) {
>>>>> +	va_list args, args2;
>>>>> +
>>>>> +	va_start(args, fmt);
>>>>> +	va_copy(args2, args);
>>>>> +	rte_vlog(RTE_LOG_ERR, acc_common_logtype, fmt, args);
>>>>> +	vsnprintf(q->error_bufs[q->error_head], ACC_MAX_LOGLEN, fmt,
>>>> args2);
>>>>> +	q->error_head++;
>>>>> +	snprintf(q->error_bufs[q->error_head], ACC_MAX_LOGLEN,
>>>>> +			"%s", rte_bbdev_ops_param_string(op, q->op_type));
>>>>> +	q->error_head++;
>>>>> +	if (q->error_head == ACC_MAX_LOGLEN) {
>>>>> +		q->error_head = 0;
>>>>> +		q->error_wrap++;
>>>>> +	}
>>>>> +	va_end(args);
>>>>> +	va_end(args2);
>>>>> +}
>>>>> +
>>>>>     /* Write to MMIO register address */
>>>>>     static inline void
>>>>>     mmio_write(void *addr, uint32_t value) diff --git
>>>>> a/drivers/baseband/acc/rte_vrb_pmd.c
>>>>> b/drivers/baseband/acc/rte_vrb_pmd.c
>>>>> index 585dc49bd6..9d0145d09b 100644
>>>>> --- a/drivers/baseband/acc/rte_vrb_pmd.c
>>>>> +++ b/drivers/baseband/acc/rte_vrb_pmd.c
>>>>> @@ -1022,6 +1022,10 @@ vrb_queue_setup(struct rte_bbdev *dev,
>>>> uint16_t queue_id,
>>>>>     	q->mmio_reg_enqueue = RTE_PTR_ADD(d->mmio_base,
>>>>>     			d->queue_offset(d->pf_device, q->vf_id, q->qgrp_id,
>> q-
>>>>> aq_id));
>>>>>
>>>>> +	/** initialize the error buffer. */
>>>>> +	q->error_head = 0;
>>>>> +	q->error_wrap = 0;
>>>>> +
>>>>>     	rte_bbdev_log_debug(
>>>>>     			"Setup dev%u q%u: qgrp_id=%u, vf_id=%u, aq_id=%u,
>>>> aq_depth=%u, mmio_reg_enqueue=%p base %p\n",
>>>>>     			dev->data->dev_id, queue_id, q->qgrp_id, q->vf_id,
>>>> @@ -1434,6
>>>>> +1438,74 @@ vrb_queue_intr_disable(struct rte_bbdev *dev, uint16_t
>>>> queue_id)
>>>>>     	return 0;
>>>>>     }
>>>>>
>>>>> +
>>>>> +static int
>>>>> +vrb_queue_ops_dump(struct rte_bbdev *dev, uint16_t queue_id, FILE
>>>>> +*f) {
>>>>> +	struct acc_queue *q = dev->data->queues[queue_id].queue_private;
>>>>> +	struct rte_bbdev_dec_op *op;
>>>>> +	uint16_t start_err, end_err, i, int_nb;
>>>>> +	volatile union acc_info_ring_data *ring_data;
>>>>> +	uint16_t info_ring_head = q->d->info_ring_head;
>>>>> +
>>>>> +	if (f == NULL) {
>>>>> +		rte_bbdev_log(ERR, "Invalid File input");
>>>>> +		return -EINVAL;
>>>>> +	}
>>>>> +
>>>>> +	/** Print generic information on queue status. */
>>>>> +	fprintf(f, "Dump of operations %s on Queue %d by %s\n",
>>>>> +			rte_bbdev_op_type_str(q->op_type), queue_id, dev-
>>>>> device->driver->name);
>>>>> +	fprintf(f, "    AQ Enqueued %d Dequeued %d Depth %d - Available Enq
>>>> %d Deq %d\n",
>>>>> +			q->aq_enqueued, q->aq_dequeued, q->aq_depth,
>>>>> +			acc_ring_avail_enq(q), acc_ring_avail_deq(q));
>>>>> +
>>>>> +	/** Print information captured in the error buffer. */
>>>>> +	if (q->error_wrap == 0) {
>>>>> +		start_err = 0;
>>>>> +		end_err = q->error_head;
>>>>> +	} else {
>>>>> +		start_err = q->error_head;
>>>>> +		end_err = q->error_head + ACC_MAX_BUFFERLEN;
>>>>> +	}
>>>>> +	fprintf(f, "Error Buffer - Head %d Wrap %d\n", q->error_head, q-
>>>>> error_wrap);
>>>>> +	for (i = start_err; i < end_err; ++i)
>>>>> +		fprintf(f, "  %d\t%s", i, q->error_bufs[i %
>>>> ACC_MAX_BUFFERLEN]);
>>>>> +
>>>>> +	/** Print information captured in the info ring. */
>>>>> +	if (q->d->info_ring != NULL) {
>>>>> +		fprintf(f, "Info Ring Buffer - Head %d\n", q->d-
>>>>> info_ring_head);
>>>>> +		ring_data = q->d->info_ring + (q->d->info_ring_head &
>>>> ACC_INFO_RING_MASK);
>>>>> +		while (ring_data->valid) {
>>>>> +			int_nb = int_from_ring(*ring_data, q->d-
>>>>> device_variant);
>>>>> +			if ((int_nb < ACC_PF_INT_DMA_DL_DESC_IRQ) || (
>>>>> +					int_nb >
>>>> ACC_PF_INT_DMA_MLD_DESC_IRQ)) {
>>>>> +				fprintf(f, "  InfoRing: ITR:%d Info:0x%x",
>>>>> +						int_nb, ring_data-
>>>>> detailed_info);
>>>>> +				/* Initialize Info Ring entry and move forward.
>>>> */
>>>>> +				ring_data->valid = 0;
>>>>> +			}
>>>>> +			info_ring_head++;
>>>>> +			ring_data = q->d->info_ring + (info_ring_head &
>>>> ACC_INFO_RING_MASK);
>>>>> +		}
>>>>> +	}
>>>>> +
>>>>> +	fprintf(f, "Ring Content - Head %d Tail %d Depth %d\n",
>>>>> +			q->sw_ring_head, q->sw_ring_tail, q->sw_ring_depth);
>>>>> +	/** Print information about each operation in the software ring. */
>>>>> +	for (i = 0; i < q->sw_ring_depth; ++i) {
>>>>> +		op = (q->ring_addr + i)->req.op_addr;
>>>>> +		if (op != NULL)
>>>>> +			fprintf(f, "  %d\tn %d %s",
>>>>> +					i, (q->ring_addr + i)->req.numCBs,
>>>>> +					rte_bbdev_ops_param_string(op, q-
>>>>> op_type));
>>>>> +	}
>>>>> +
>>>>> +	fprintf(f, "== End of File ==\n");
>>>>> +
>>>>> +	return 0;
>>>>> +}
>>>>> +
>>>>>     static const struct rte_bbdev_ops vrb_bbdev_ops = {
>>>>>     	.setup_queues = vrb_setup_queues,
>>>>>     	.intr_enable = vrb_intr_enable,
>>>>> @@ -1443,7 +1515,8 @@ static const struct rte_bbdev_ops
>>>>> vrb_bbdev_ops
>>>> = {
>>>>>     	.queue_release = vrb_queue_release,
>>>>>     	.queue_stop = vrb_queue_stop,
>>>>>     	.queue_intr_enable = vrb_queue_intr_enable,
>>>>> -	.queue_intr_disable = vrb_queue_intr_disable
>>>>> +	.queue_intr_disable = vrb_queue_intr_disable,
>>>>> +	.queue_ops_dump = vrb_queue_ops_dump
>>>>>     };
>>>>>
>>>>>     /* PCI PF address map. */
>>>>> @@ -1680,7 +1753,7 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op
>>>> *op,
>>>>>     		uint32_t *in_offset, uint32_t *h_out_offset,
>>>>>     		uint32_t *s_out_offset, uint32_t *h_out_length,
>>>>>     		uint32_t *s_out_length, uint32_t *mbuf_total_left,
>>>>> -		uint32_t *seg_total_left, uint8_t r)
>>>>> +		uint32_t *seg_total_left, uint8_t r, struct acc_queue *q)
>>>>>     {
>>>>>     	int next_triplet = 1; /* FCW already done. */
>>>>>     	uint16_t k;
>>>>> @@ -1724,8 +1797,8 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op
>>>> *op,
>>>>>     	kw = RTE_ALIGN_CEIL(k + 4, 32) * 3;
>>>>>
>>>>>     	if (unlikely((*mbuf_total_left == 0) || (*mbuf_total_left < kw))) {
>>>>> -		rte_bbdev_log(ERR,
>>>>> -				"Mismatch between mbuf length and included
>>>> CB sizes: mbuf len %u, cb len %u",
>>>>> +		acc_error_log(q, (void *)op,
>>>>> +				"Mismatch between mbuf length and included
>>>> CB sizes: mbuf len %u,
>>>>> +cb len %u\n",
>>>>>     				*mbuf_total_left, kw);
>>>>>     		return -1;
>>>>>     	}
>>>>> @@ -1735,8 +1808,8 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op
>>>> *op,
>>>>>     			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",
>>>>> +		acc_error_log(q, (void *)op,
>>>>> +				"Mismatch between data to process and mbuf
>>>> data length in
>>>>> +bbdev_op: %p\n",
>>>>>     				op);
>>>>>     		return -1;
>>>>>     	}
>>>>> @@ -1748,7 +1821,7 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op
>>>> *op,
>>>>>     			desc, h_output, *h_out_offset,
>>>>>     			*h_out_length, next_triplet,
>>>> ACC_DMA_BLKID_OUT_HARD);
>>>>>     	if (unlikely(next_triplet < 0)) {
>>>>> -		rte_bbdev_log(ERR,
>>>>> +		acc_error_log(q, (void *)op,
>>>>>     				"Mismatch between data to process and mbuf
>>>> data length in bbdev_op: %p",
>>>>>     				op);
>>>>>     		return -1;
>>>>> @@ -1760,7 +1833,7 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op
>>>> *op,
>>>>>     	/* Soft output. */
>>>>>     	if (check_bit(op->turbo_dec.op_flags,
>>>> RTE_BBDEV_TURBO_SOFT_OUTPUT)) {
>>>>>     		if (op->turbo_dec.soft_output.data == 0) {
>>>>> -			rte_bbdev_log(ERR, "Soft output is not defined");
>>>>> +			acc_error_log(q, (void *)op, "Soft output is not
>>>> defined\n");
>>>>>     			return -1;
>>>>>     		}
>>>>>     		if (check_bit(op->turbo_dec.op_flags,
>>>>> @@ -1773,8 +1846,8 @@ vrb_dma_desc_td_fill(struct rte_bbdev_dec_op
>>>> *op,
>>>>>     				*s_out_offset, *s_out_length, next_triplet,
>>>>>     				ACC_DMA_BLKID_OUT_SOFT);
>>>>>     		if (unlikely(next_triplet < 0)) {
>>>>> -			rte_bbdev_log(ERR,
>>>>> -					"Mismatch between data to process
>>>> and mbuf data length in bbdev_op: %p",
>>>>> +			acc_error_log(q, (void *)op,
>>>>> +					"Mismatch between data to process
>>>> and mbuf data length in
>>>>> +bbdev_op: %p\n",
>>>>>     					op);
>>>>>     			return -1;
>>>>>     		}
>>>>> @@ -1797,7 +1870,8 @@ vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op
>>>> *op,
>>>>>     		struct rte_mbuf **input, struct rte_mbuf *h_output,
>>>>>     		uint32_t *in_offset, uint32_t *h_out_offset,
>>>>>     		uint32_t *h_out_length, uint32_t *mbuf_total_left,
>>>>> -		uint32_t *seg_total_left, struct acc_fcw_ld *fcw, uint16_t
>>>> device_variant)
>>>>> +		uint32_t *seg_total_left, struct acc_fcw_ld *fcw, uint16_t
>>>> device_variant,
>>>>> +		struct acc_queue *q)
>>>>>     {
>>>>>     	struct rte_bbdev_op_ldpc_dec *dec = &op->ldpc_dec;
>>>>>     	int next_triplet = 1; /* FCW already done. */ @@ -1808,8 +1882,8
>>>> @@
>>>>> vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op *op,
>>>>>     	if (device_variant == VRB1_VARIANT) {
>>>>>     		if (check_bit(op->ldpc_dec.op_flags,
>>>> RTE_BBDEV_LDPC_HARQ_4BIT_COMPRESSION) ||
>>>>>     				check_bit(op->ldpc_dec.op_flags,
>>>> RTE_BBDEV_LDPC_SOFT_OUT_ENABLE)) {
>>>>> -			rte_bbdev_log(ERR,
>>>>> -					"VRB1 does not support the requested
>>>> capabilities %x",
>>>>> +			acc_error_log(q, (void *)op,
>>>>> +					"VRB1 does not support the requested
>>>> capabilities %x\n",
>>
>> You should not re-introduce "\n", David is currently working on getting rid off
>> them all.
>>
>>>>>     					op->ldpc_dec.op_flags);
>>>>>     			return -1;
>>>>>     		}
>>>>> @@ -1829,8 +1903,8 @@ vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op
>>>> *op,
>>>>>     	output_length = K - dec->n_filler - crc24_overlap;
>>>>>
>>>>>     	if (unlikely((*mbuf_total_left == 0) || (*mbuf_total_left <
>>>> input_length))) {
>>>>> -		rte_bbdev_log(ERR,
>>>>> -				"Mismatch between mbuf length and included
>>>> CB sizes: mbuf len %u, cb len %u",
>>>>> +		acc_error_log(q, (void *)op,
>>>>> +				"Mismatch between mbuf length and included
>>>> CB sizes: mbuf len %u,
>>>>> +cb len %u\n",
>>>>>     				*mbuf_total_left, input_length);
>>>>>     		return -1;
>>>>>     	}
>>>>> @@ -1842,15 +1916,15 @@ vrb_dma_desc_ld_fill(struct
>> rte_bbdev_dec_op
>>>> *op,
>>>>>     			RTE_BBDEV_LDPC_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",
>>>>> +		acc_error_log(q, (void *)op,
>>>>> +				"Mismatch between data to process and mbuf
>>>> data length in
>>>>> +bbdev_op: %p\n",
>>>>>     				op);
>>>>>     		return -1;
>>>>>     	}
>>>>>
>>>>>     	if (check_bit(op->ldpc_dec.op_flags,
>>>> RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE)) {
>>>>>     		if (op->ldpc_dec.harq_combined_input.data == 0) {
>>>>> -			rte_bbdev_log(ERR, "HARQ input is not defined");
>>>>> +			acc_error_log(q, (void *)op, "HARQ input is not
>>>> defined\n");
>>>>>     			return -1;
>>>>>     		}
>>>>>     		h_p_size = fcw->hcin_size0 + fcw->hcin_size1; @@ -1859,7
>>>> +1933,7
>>>>> @@ vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op *op,
>>>>>     		else if (fcw->hcin_decomp_mode == 4)
>>>>>     			h_p_size = h_p_size / 2;
>>>>>     		if (op->ldpc_dec.harq_combined_input.data == 0) {
>>>>> -			rte_bbdev_log(ERR, "HARQ input is not defined");
>>>>> +			acc_error_log(q, (void *)op, "HARQ input is not
>>>> defined\n");
>>>>>     			return -1;
>>>>>     		}
>>>>>     		acc_dma_fill_blk_type(
>>>>> @@ -1882,7 +1956,7 @@ vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op
>>>>> *op,
>>>>>
>>>>>     	if (check_bit(op->ldpc_dec.op_flags,
>>>> RTE_BBDEV_LDPC_SOFT_OUT_ENABLE)) {
>>>>>     		if (op->ldpc_dec.soft_output.data == 0) {
>>>>> -			rte_bbdev_log(ERR, "Soft output is not defined");
>>>>> +			acc_error_log(q, (void *)op, "Soft output is not
>>>> defined\n");
>>>>>     			return -1;
>>>>>     		}
>>>>>     		dec->soft_output.length = fcw->rm_e; @@ -1894,7 +1968,7
>>>> @@
>>>>> vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op *op,
>>>>>     	if (check_bit(op->ldpc_dec.op_flags,
>>>>>
>>>> 	RTE_BBDEV_LDPC_HQ_COMBINE_OUT_ENABLE)) {
>>>>>     		if (op->ldpc_dec.harq_combined_output.data == 0) {
>>>>> -			rte_bbdev_log(ERR, "HARQ output is not defined");
>>>>> +			acc_error_log(q, (void *)op, "HARQ output is not
>>>> defined\n");
>>>>>     			return -1;
>>>>>     		}
>>>>>
>>>>> @@ -2326,8 +2400,8 @@ vrb2_enqueue_ldpc_enc_one_op_tb(struct
>>>> acc_queue *q, struct rte_bbdev_enc_op *op
>>>>>     			in_length_in_bytes, &seg_total_left, next_triplet,
>>>>>     			check_bit(enc->op_flags,
>>>> RTE_BBDEV_LDPC_ENC_SCATTER_GATHER));
>>>>>     	if (unlikely(next_triplet < 0)) {
>>>>> -		rte_bbdev_log(ERR,
>>>>> -				"Mismatch between data to process and mbuf
>>>> data length in bbdev_op: %p",
>>>>> +		acc_error_log(q, (void *)op,
>>>>> +				"Mismatch between data to process and mbuf
>>>> data length in
>>>>> +bbdev_op: %p\n",
>>>>>     				op);
>>>>>     		return -1;
>>>>>     	}
>>>>> @@ -2399,7 +2473,7 @@ enqueue_dec_one_op_cb(struct acc_queue *q,
>>>> struct rte_bbdev_dec_op *op,
>>>>>     	ret = vrb_dma_desc_td_fill(op, &desc->req, &input, h_output,
>>>>>     			s_output, &in_offset, &h_out_offset, &s_out_offset,
>>>>>     			&h_out_length, &s_out_length, &mbuf_total_left,
>>>>> -			&seg_total_left, 0);
>>>>> +			&seg_total_left, 0, q);
>>>>>
>>>>>     	if (unlikely(ret < 0))
>>>>>     		return ret;
>>>>> @@ -2478,7 +2552,7 @@ vrb_enqueue_ldpc_dec_one_op_cb(struct
>>>> acc_queue *q, struct rte_bbdev_dec_op *op,
>>>>>     		ret = vrb_dma_desc_ld_fill(op, &desc->req, &input, h_output,
>>>>>     				&in_offset, &h_out_offset,
>>>>>     				&h_out_length, &mbuf_total_left,
>>>>> -				&seg_total_left, fcw, q->d->device_variant);
>>>>> +				&seg_total_left, fcw, q->d->device_variant, q);
>>>>>     		if (unlikely(ret < 0))
>>>>>     			return ret;
>>>>>     	}
>>>>> @@ -2572,7 +2646,7 @@ vrb_enqueue_ldpc_dec_one_op_tb(struct
>>>> acc_queue *q, struct rte_bbdev_dec_op *op,
>>>>>     				h_output, &in_offset, &h_out_offset,
>>>>>     				&h_out_length,
>>>>>     				&mbuf_total_left, &seg_total_left,
>>>>> -				&desc->req.fcw_ld, q->d->device_variant);
>>>>> +				&desc->req.fcw_ld, q->d->device_variant, q);
>>>>>
>>>>>     		if (unlikely(ret < 0))
>>>>>     			return ret;
>>>>> @@ -2658,7 +2732,7 @@ enqueue_dec_one_op_tb(struct acc_queue *q,
>>>> struct rte_bbdev_dec_op *op,
>>>>>     		ret = vrb_dma_desc_td_fill(op, &desc->req, &input,
>>>>>     				h_output, s_output, &in_offset,
>>>> &h_out_offset,
>>>>>     				&s_out_offset, &h_out_length,
>>>> &s_out_length,
>>>>> -				&mbuf_total_left, &seg_total_left, r);
>>>>> +				&mbuf_total_left, &seg_total_left, r, q);
>>>>>
>>>>>     		if (unlikely(ret < 0))
>>>>>     			return ret;
>>>
>