DPDK patches and discussions
 help / color / mirror / Atom feed
From: Nicolas Chautru <nicolas.chautru@intel.com>
To: dev@dpdk.org, maxime.coquelin@redhat.com
Cc: hemant.agrawal@nxp.com, hernan.vargas@intel.com,
	Nicolas Chautru <nicolas.chautru@intel.com>
Subject: [PATCH v1 2/3] baseband/acc: add trace point
Date: Thu, 23 Jan 2025 12:28:06 -0800	[thread overview]
Message-ID: <20250123202807.2089618-3-nicolas.chautru@intel.com> (raw)
In-Reply-To: <20250123202807.2089618-1-nicolas.chautru@intel.com>

Improvement of logging to notably use trace point
for driver specific error logging and tracepoint.

Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
---
 drivers/baseband/acc/acc_common.c  |  8 ++++
 drivers/baseband/acc/acc_common.h  | 55 ++++++++++++++++++++++++++
 drivers/baseband/acc/rte_vrb_pmd.c | 63 +++++++++++++++++-------------
 drivers/baseband/acc/vrb_trace.h   | 35 +++++++++++++++++
 4 files changed, 133 insertions(+), 28 deletions(-)
 create mode 100644 drivers/baseband/acc/vrb_trace.h

diff --git a/drivers/baseband/acc/acc_common.c b/drivers/baseband/acc/acc_common.c
index f8d2b19570..25ddef8a6c 100644
--- a/drivers/baseband/acc/acc_common.c
+++ b/drivers/baseband/acc/acc_common.c
@@ -3,5 +3,13 @@
  */
 
 #include <rte_log.h>
+#include <rte_trace_point_register.h>
+#include "vrb_trace.h"
 
 RTE_LOG_REGISTER_SUFFIX(acc_common_logtype, common, INFO);
+
+RTE_TRACE_POINT_REGISTER(rte_bbdev_vrb_trace_error,
+	bbdev.vrb.device.error);
+
+RTE_TRACE_POINT_REGISTER(rte_bbdev_vrb_trace_queue_error,
+	bbdev.vrb.queue.error);
diff --git a/drivers/baseband/acc/acc_common.h b/drivers/baseband/acc/acc_common.h
index a49b154a0c..4880444450 100644
--- a/drivers/baseband/acc/acc_common.h
+++ b/drivers/baseband/acc/acc_common.h
@@ -7,6 +7,7 @@
 
 #include <bus_pci_driver.h>
 #include "rte_acc_common_cfg.h"
+#include "vrb_trace.h"
 
 /* Values used in filling in descriptors */
 #define ACC_DMA_DESC_TYPE           2
@@ -653,6 +654,56 @@ struct __rte_cache_aligned acc_queue {
 	struct acc_device *d;
 };
 
+/* These strings for rte_trace must be limited to RTE_TRACE_EMIT_STRING_LEN_MAX. */
+static const char * const acc_error_string[] = {
+	"Warn: HARQ offset unexpected.",
+	"HARQ in/output is not defined.",
+	"Mismatch related to Mbuf data.",
+	"Soft output is not defined.",
+	"Device incompatible cap.",
+	"HARQ cannot be appended.",
+	"Undefined error message.",
+};
+
+/* Matching indexes for acc_error_string. */
+enum acc_error_enum {
+	ACC_ERR_HARQ_UNEXPECTED,
+	ACC_ERR_REJ_HARQ,
+	ACC_ERR_REJ_MBUF,
+	ACC_ERR_REJ_SOFT,
+	ACC_ERR_REJ_CAP,
+	ACC_ERR_REJ_HARQ_OUT,
+	ACC_ERR_MAX
+};
+
+/**
+ * @brief Report error both through RTE logging and into trace point.
+ *
+ * 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(4, 5)
+static inline void
+acc_error_log(struct acc_queue *q, void *op, uint8_t acc_error_idx, const char *fmt, ...)
+{
+	va_list args;
+	RTE_SET_USED(op);
+	va_start(args, fmt);
+	rte_vlog(RTE_LOG_ERR, acc_common_logtype, fmt, args);
+
+	if (acc_error_idx > ACC_ERR_MAX)
+		acc_error_idx = ACC_ERR_MAX;
+
+	rte_bbdev_vrb_trace_error(0, rte_bbdev_op_type_str(q->op_type),
+			acc_error_string[acc_error_idx]);
+
+	va_end(args);
+}
+
 /* Write to MMIO register address */
 static inline void
 mmio_write(void *addr, uint32_t value)
@@ -1511,6 +1562,10 @@ acc_enqueue_status(struct rte_bbdev_queue_data *q_data,
 {
 	q_data->enqueue_status = status;
 	q_data->queue_stats.enqueue_status_count[status]++;
+	struct acc_queue *q = q_data->queue_private;
+
+	rte_bbdev_vrb_trace_queue_error(q->qgrp_id, q->aq_id,
+			rte_bbdev_enqueue_status_str(status));
 
 	rte_acc_log(WARNING, "Enqueue Status: %s %#"PRIx64"",
 			rte_bbdev_enqueue_status_str(status),
diff --git a/drivers/baseband/acc/rte_vrb_pmd.c b/drivers/baseband/acc/rte_vrb_pmd.c
index eb9892ff31..27620ccc10 100644
--- a/drivers/baseband/acc/rte_vrb_pmd.c
+++ b/drivers/baseband/acc/rte_vrb_pmd.c
@@ -1816,7 +1816,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;
@@ -1860,8 +1860,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, ACC_ERR_REJ_MBUF,
+				"Mismatch between mbuf length and included CB sizes: mbuf len %u, cb len %u\n",
 				*mbuf_total_left, kw);
 		return -1;
 	}
@@ -1871,8 +1871,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, ACC_ERR_REJ_MBUF,
+				"Mismatch between data to process and mbuf data length in bbdev_op: %p\n",
 				op);
 		return -1;
 	}
@@ -1884,8 +1884,8 @@ 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,
-				"Mismatch between data to process and mbuf data length in bbdev_op: %p",
+		acc_error_log(q, (void *)op, ACC_ERR_REJ_MBUF,
+				"Mismatch between data to process and mbuf data length in bbdev_op: %p\n",
 				op);
 		return -1;
 	}
@@ -1896,7 +1896,8 @@ 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, ACC_ERR_REJ_SOFT,
+					"Soft output is not defined\n");
 			return -1;
 		}
 		if (check_bit(op->turbo_dec.op_flags,
@@ -1909,8 +1910,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, ACC_ERR_REJ_MBUF,
+					"Mismatch between data to process and mbuf data length in bbdev_op: %p\n",
 					op);
 			return -1;
 		}
@@ -1933,7 +1934,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. */
@@ -1944,8 +1946,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, ACC_ERR_REJ_CAP,
+					"VRB1 does not support the requested capabilities %x\n",
 					op->ldpc_dec.op_flags);
 			return -1;
 		}
@@ -1965,8 +1967,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, ACC_ERR_REJ_MBUF,
+				"Mismatch between mbuf length and included CB sizes: mbuf len %u, cb len %u\n",
 				*mbuf_total_left, input_length);
 		return -1;
 	}
@@ -1978,15 +1980,16 @@ 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, ACC_ERR_REJ_MBUF,
+				"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,  ACC_ERR_REJ_HARQ,
+					"HARQ input is not defined\n");
 			return -1;
 		}
 		h_p_size = fcw->hcin_size0 + fcw->hcin_size1;
@@ -1995,7 +1998,8 @@ 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, ACC_ERR_REJ_HARQ,
+					"HARQ input is not defined\n");
 			return -1;
 		}
 		acc_dma_fill_blk_type(
@@ -2018,7 +2022,8 @@ 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, ACC_ERR_REJ_SOFT,
+					"Soft output is not defined\n");
 			return -1;
 		}
 		dec->soft_output.length = fcw->rm_e;
@@ -2029,7 +2034,8 @@ 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, ACC_ERR_REJ_HARQ,
+					"HARQ output is not defined\n");
 			return -1;
 		}
 
@@ -2534,7 +2540,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;
@@ -2612,7 +2618,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;
 	}
@@ -2627,9 +2633,10 @@ vrb_enqueue_ldpc_dec_one_op_cb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
 		hq_output = op->ldpc_dec.harq_combined_output.data;
 		hq_len = op->ldpc_dec.harq_combined_output.length;
 		if (unlikely(!mbuf_append(hq_output_head, hq_output, hq_len))) {
-			rte_bbdev_log(ERR, "HARQ output mbuf issue %d %d",
-					hq_output->buf_len,
-					hq_len);
+			acc_error_log(q, (void *)op, ACC_ERR_REJ_HARQ_OUT,
+					"HARQ output mbuf cannot be appended Buffer %d Current data %d New data %d\n",
+					hq_output->buf_len, hq_output->data_len, hq_len);
+
 			return -1;
 		}
 	}
@@ -2706,7 +2713,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;
@@ -2792,7 +2799,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;
diff --git a/drivers/baseband/acc/vrb_trace.h b/drivers/baseband/acc/vrb_trace.h
new file mode 100644
index 0000000000..0bbfdc47d1
--- /dev/null
+++ b/drivers/baseband/acc/vrb_trace.h
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2025 Intel Corporation
+ */
+
+#ifndef VRB_TRACE_H_
+#define VRB_TRACE_H_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <rte_trace_point.h>
+
+RTE_TRACE_POINT_FP(
+	rte_bbdev_vrb_trace_error,
+	RTE_TRACE_POINT_ARGS(uint8_t dev_id, const char *op_string, const char *err_string),
+	rte_trace_point_emit_u8(dev_id);
+	rte_trace_point_emit_string(op_string);
+	rte_trace_point_emit_string(err_string);
+)
+
+RTE_TRACE_POINT_FP(
+	rte_bbdev_vrb_trace_queue_error,
+	RTE_TRACE_POINT_ARGS(uint8_t qg_id, uint8_t aq_id, const char *str),
+	rte_trace_point_emit_u8(qg_id);
+	rte_trace_point_emit_u8(aq_id);
+	rte_trace_point_emit_string(str);
+)
+
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* VRB_TRACE_H_ */
-- 
2.34.1


  parent reply	other threads:[~2025-01-23 20:40 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-23 20:28 [PATCH v1 0/3] bbdev: trace point and logging Nicolas Chautru
2025-01-23 20:28 ` [PATCH v1 1/3] bbdev: add trace point Nicolas Chautru
2025-01-23 20:28 ` Nicolas Chautru [this message]
2025-01-23 20:28 ` [PATCH v1 3/3] baseband/acc: add internal logging Nicolas Chautru

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250123202807.2089618-3-nicolas.chautru@intel.com \
    --to=nicolas.chautru@intel.com \
    --cc=dev@dpdk.org \
    --cc=hemant.agrawal@nxp.com \
    --cc=hernan.vargas@intel.com \
    --cc=maxime.coquelin@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).