[v3,21/29] crypto/cnxk: add more info on command timeout

Message ID 1639732811-1440-22-git-send-email-anoobj@marvell.com (mailing list archive)
State Accepted, archived
Delegated to: akhil goyal
Headers
Series New features and improvements in cnxk crypto PMD |

Checks

Context Check Description
ci/checkpatch warning coding style issues

Commit Message

Anoob Joseph Dec. 17, 2021, 9:20 a.m. UTC
  Print more info when command timeout happens. Print software and
hardware queue information.

Signed-off-by: Anoob Joseph <anoobj@marvell.com>
Signed-off-by: Tejasree Kondoj <ktejasree@marvell.com>
---
 drivers/common/cnxk/hw/cpt.h              | 11 ++++++++
 drivers/crypto/cnxk/cn10k_cryptodev_ops.c |  1 +
 drivers/crypto/cnxk/cn9k_cryptodev_ops.c  |  1 +
 drivers/crypto/cnxk/cnxk_cryptodev_ops.c  | 43 +++++++++++++++++++++++++++++++
 drivers/crypto/cnxk/cnxk_cryptodev_ops.h  |  1 +
 5 files changed, 57 insertions(+)
  

Comments

Thomas Monjalon Jan. 11, 2022, 3:23 p.m. UTC | #1
17/12/2021 10:20, Anoob Joseph:
> Print more info when command timeout happens. Print software and
> hardware queue information.
> 
> Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> Signed-off-by: Tejasree Kondoj <ktejasree@marvell.com>
> ---
> +void
> +cnxk_cpt_dump_on_err(struct cnxk_cpt_qp *qp)
> +{
> +	struct pending_queue *pend_q = &qp->pend_q;
> +	uint64_t inflight, enq_ptr, deq_ptr, insts;
> +	union cpt_lf_q_inst_ptr inst_ptr;
> +	union cpt_lf_inprog lf_inprog;
> +
> +	plt_print("Lcore ID: %d, LF/QP ID: %d", rte_lcore_id(), qp->lf.lf_id);
> +	plt_print("");
> +	plt_print("S/w pending queue:");
> +	plt_print("\tHead: %ld", pend_q->head);
> +	plt_print("\tTail: %ld", pend_q->tail);
> +	plt_print("\tMask: 0x%lx", pend_q->pq_mask);
> +	plt_print("\tInflight count: %ld",
> +		  pending_queue_infl_cnt(pend_q->head, pend_q->tail,
> +					 pend_q->pq_mask));
> +
> +	plt_print("");
> +	plt_print("H/w pending queue:");
> +
> +	lf_inprog.u = plt_read64(qp->lf.rbase + CPT_LF_INPROG);
> +	inflight = lf_inprog.s.inflight;
> +	plt_print("\tInflight in engines: %ld", inflight);
> +
> +	inst_ptr.u = plt_read64(qp->lf.rbase + CPT_LF_Q_INST_PTR);
> +
> +	enq_ptr = inst_ptr.s.nq_ptr;
> +	deq_ptr = inst_ptr.s.dq_ptr;
> +
> +	if (enq_ptr >= deq_ptr)
> +		insts = enq_ptr - deq_ptr;
> +	else
> +		insts = (enq_ptr + pend_q->pq_mask + 1 + 320 + 40) - deq_ptr;
> +
> +	plt_print("\tNQ ptr: 0x%lx", enq_ptr);
> +	plt_print("\tDQ ptr: 0x%lx", deq_ptr);
> +	plt_print("Insts waiting in CPT: %ld", insts);
> +
> +	plt_print("");
> +	roc_cpt_afs_print(qp->lf.roc_cpt);
> +}

This functions is wrong. You cannot print 64-bit values with %l.
In 32-bit mode, compilation will fail.
Please use PRIx64.

Note: this mistake is warned by the script devtools/checkpatches.sh
  Warning in drivers/crypto/cnxk/cnxk_cryptodev_ops.c:
  Using %l format, prefer %PRI*64 if type is [u]int64_t

I will wait for the next-crypto tree to be fixed.
  
Akhil Goyal Jan. 21, 2022, 9:16 a.m. UTC | #2
> 17/12/2021 10:20, Anoob Joseph:
> > Print more info when command timeout happens. Print software and
> > hardware queue information.
> >
> > Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> > Signed-off-by: Tejasree Kondoj <ktejasree@marvell.com>
> > ---
> > +void
> > +cnxk_cpt_dump_on_err(struct cnxk_cpt_qp *qp)
> > +{
> > +	struct pending_queue *pend_q = &qp->pend_q;
> > +	uint64_t inflight, enq_ptr, deq_ptr, insts;
> > +	union cpt_lf_q_inst_ptr inst_ptr;
> > +	union cpt_lf_inprog lf_inprog;
> > +
> > +	plt_print("Lcore ID: %d, LF/QP ID: %d", rte_lcore_id(), qp->lf.lf_id);
> > +	plt_print("");
> > +	plt_print("S/w pending queue:");
> > +	plt_print("\tHead: %ld", pend_q->head);
> > +	plt_print("\tTail: %ld", pend_q->tail);
> > +	plt_print("\tMask: 0x%lx", pend_q->pq_mask);
> > +	plt_print("\tInflight count: %ld",
> > +		  pending_queue_infl_cnt(pend_q->head, pend_q->tail,
> > +					 pend_q->pq_mask));
> > +
> > +	plt_print("");
> > +	plt_print("H/w pending queue:");
> > +
> > +	lf_inprog.u = plt_read64(qp->lf.rbase + CPT_LF_INPROG);
> > +	inflight = lf_inprog.s.inflight;
> > +	plt_print("\tInflight in engines: %ld", inflight);
> > +
> > +	inst_ptr.u = plt_read64(qp->lf.rbase + CPT_LF_Q_INST_PTR);
> > +
> > +	enq_ptr = inst_ptr.s.nq_ptr;
> > +	deq_ptr = inst_ptr.s.dq_ptr;
> > +
> > +	if (enq_ptr >= deq_ptr)
> > +		insts = enq_ptr - deq_ptr;
> > +	else
> > +		insts = (enq_ptr + pend_q->pq_mask + 1 + 320 + 40) - deq_ptr;
> > +
> > +	plt_print("\tNQ ptr: 0x%lx", enq_ptr);
> > +	plt_print("\tDQ ptr: 0x%lx", deq_ptr);
> > +	plt_print("Insts waiting in CPT: %ld", insts);
> > +
> > +	plt_print("");
> > +	roc_cpt_afs_print(qp->lf.roc_cpt);
> > +}
> 
> This functions is wrong. You cannot print 64-bit values with %l.
> In 32-bit mode, compilation will fail.
> Please use PRIx64.
> 
> Note: this mistake is warned by the script devtools/checkpatches.sh
>   Warning in drivers/crypto/cnxk/cnxk_cryptodev_ops.c:
>   Using %l format, prefer %PRI*64 if type is [u]int64_t
I believe there is something wrong in the reporting;
it said 1 warning which is for spell check of head
and in the end a line is added for another warning.
I skipped this issue as it was a false positive for spelling. Did not see the last line.

WARNING:TYPO_SPELLING: 'tHead' may be misspelled - perhaps 'thread'?
#157: FILE: drivers/crypto/cnxk/cnxk_cryptodev_ops.c:718:
+	plt_print("	Head: %ld", pend_q->head);

total: 0 errors, 1 warnings, 84 lines checked
	            ^^^^^^^^^^	
Warning in drivers/crypto/cnxk/cnxk_cryptodev_ops.c:
Using %l format, prefer %PRI*64 if type is [u]int64_t

> 
> I will wait for the next-crypto tree to be fixed.
> 
Following changes are mage in this patch on crypto tree.
diff --git a/drivers/crypto/cnxk/cnxk_cryptodev_ops.c b/drivers/crypto/cnxk/cnxk_cryptodev_ops.c
index 0ce54d7bf0..67a2d9b08e 100644
--- a/drivers/crypto/cnxk/cnxk_cryptodev_ops.c
+++ b/drivers/crypto/cnxk/cnxk_cryptodev_ops.c
@@ -715,10 +715,10 @@ cnxk_cpt_dump_on_err(struct cnxk_cpt_qp *qp)
        plt_print("Lcore ID: %d, LF/QP ID: %d", rte_lcore_id(), qp->lf.lf_id);
        plt_print("");
        plt_print("S/w pending queue:");
-       plt_print("\tHead: %ld", pend_q->head);
-       plt_print("\tTail: %ld", pend_q->tail);
-       plt_print("\tMask: 0x%lx", pend_q->pq_mask);
-       plt_print("\tInflight count: %ld",
+       plt_print("\tHead: %"PRIu64"", pend_q->head);
+       plt_print("\tTail: %"PRIu64"", pend_q->tail);
+       plt_print("\tMask: 0x%"PRIx64"", pend_q->pq_mask);
+       plt_print("\tInflight count: %"PRIu64"",
                  pending_queue_infl_cnt(pend_q->head, pend_q->tail,
                                         pend_q->pq_mask));

@@ -727,7 +727,7 @@ cnxk_cpt_dump_on_err(struct cnxk_cpt_qp *qp)

        lf_inprog.u = plt_read64(qp->lf.rbase + CPT_LF_INPROG);
        inflight = lf_inprog.s.inflight;
-       plt_print("\tInflight in engines: %ld", inflight);
+       plt_print("\tInflight in engines: %"PRIu64"", inflight);

        inst_ptr.u = plt_read64(qp->lf.rbase + CPT_LF_Q_INST_PTR);

@@ -739,9 +739,9 @@ cnxk_cpt_dump_on_err(struct cnxk_cpt_qp *qp)
        else
                insts = (enq_ptr + pend_q->pq_mask + 1 + 320 + 40) - deq_ptr;

-       plt_print("\tNQ ptr: 0x%lx", enq_ptr);
-       plt_print("\tDQ ptr: 0x%lx", deq_ptr);
-       plt_print("Insts waiting in CPT: %ld", insts);
+       plt_print("\tNQ ptr: 0x%"PRIx64"", enq_ptr);
+       plt_print("\tDQ ptr: 0x%"PRIx64"", deq_ptr);
+       plt_print("Insts waiting in CPT: %"PRIu64"", insts);

        plt_print("");
        roc_cpt_afs_print(qp->lf.roc_cpt);
  
Thomas Monjalon Jan. 21, 2022, 10:41 a.m. UTC | #3
21/01/2022 10:16, Akhil Goyal:
> > Note: this mistake is warned by the script devtools/checkpatches.sh
> >   Warning in drivers/crypto/cnxk/cnxk_cryptodev_ops.c:
> >   Using %l format, prefer %PRI*64 if type is [u]int64_t
> I believe there is something wrong in the reporting;
> it said 1 warning which is for spell check of head
> and in the end a line is added for another warning.
> I skipped this issue as it was a false positive for spelling. Did not see the last line.
> 
> WARNING:TYPO_SPELLING: 'tHead' may be misspelled - perhaps 'thread'?
> #157: FILE: drivers/crypto/cnxk/cnxk_cryptodev_ops.c:718:
> +	plt_print("	Head: %ld", pend_q->head);
> 
> total: 0 errors, 1 warnings, 84 lines checked
> 	            ^^^^^^^^^^	
> Warning in drivers/crypto/cnxk/cnxk_cryptodev_ops.c:
> Using %l format, prefer %PRI*64 if type is [u]int64_t

That's because they are 2 different checks.
The first total is from checkpatch.pl.
  

Patch

diff --git a/drivers/common/cnxk/hw/cpt.h b/drivers/common/cnxk/hw/cpt.h
index 412dd76..4d9c896 100644
--- a/drivers/common/cnxk/hw/cpt.h
+++ b/drivers/common/cnxk/hw/cpt.h
@@ -91,6 +91,17 @@  union cpt_lf_inprog {
 	} s;
 };
 
+union cpt_lf_q_inst_ptr {
+	uint64_t u;
+	struct cpt_lf_q_inst_ptr_s {
+		uint64_t dq_ptr : 20;
+		uint64_t reserved_20_31 : 12;
+		uint64_t nq_ptr : 20;
+		uint64_t reserved_52_62 : 11;
+		uint64_t xq_xor : 1;
+	} s;
+};
+
 union cpt_lf_q_base {
 	uint64_t u;
 	struct cpt_lf_q_base_s {
diff --git a/drivers/crypto/cnxk/cn10k_cryptodev_ops.c b/drivers/crypto/cnxk/cn10k_cryptodev_ops.c
index f8240e1..1905ea3 100644
--- a/drivers/crypto/cnxk/cn10k_cryptodev_ops.c
+++ b/drivers/crypto/cnxk/cn10k_cryptodev_ops.c
@@ -548,6 +548,7 @@  cn10k_cpt_dequeue_burst(void *qptr, struct rte_crypto_op **ops, uint16_t nb_ops)
 			if (unlikely(rte_get_timer_cycles() >
 				     pend_q->time_out)) {
 				plt_err("Request timed out");
+				cnxk_cpt_dump_on_err(qp);
 				pend_q->time_out = rte_get_timer_cycles() +
 						   DEFAULT_COMMAND_TIMEOUT *
 							   rte_get_timer_hz();
diff --git a/drivers/crypto/cnxk/cn9k_cryptodev_ops.c b/drivers/crypto/cnxk/cn9k_cryptodev_ops.c
index cf80d47..ac1953b 100644
--- a/drivers/crypto/cnxk/cn9k_cryptodev_ops.c
+++ b/drivers/crypto/cnxk/cn9k_cryptodev_ops.c
@@ -547,6 +547,7 @@  cn9k_cpt_dequeue_burst(void *qptr, struct rte_crypto_op **ops, uint16_t nb_ops)
 			if (unlikely(rte_get_timer_cycles() >
 				     pend_q->time_out)) {
 				plt_err("Request timed out");
+				cnxk_cpt_dump_on_err(qp);
 				pend_q->time_out = rte_get_timer_cycles() +
 						   DEFAULT_COMMAND_TIMEOUT *
 							   rte_get_timer_hz();
diff --git a/drivers/crypto/cnxk/cnxk_cryptodev_ops.c b/drivers/crypto/cnxk/cnxk_cryptodev_ops.c
index 7953a08..0ce54d7 100644
--- a/drivers/crypto/cnxk/cnxk_cryptodev_ops.c
+++ b/drivers/crypto/cnxk/cnxk_cryptodev_ops.c
@@ -703,3 +703,46 @@  cnxk_ae_session_cfg(struct rte_cryptodev *dev,
 
 	return 0;
 }
+
+void
+cnxk_cpt_dump_on_err(struct cnxk_cpt_qp *qp)
+{
+	struct pending_queue *pend_q = &qp->pend_q;
+	uint64_t inflight, enq_ptr, deq_ptr, insts;
+	union cpt_lf_q_inst_ptr inst_ptr;
+	union cpt_lf_inprog lf_inprog;
+
+	plt_print("Lcore ID: %d, LF/QP ID: %d", rte_lcore_id(), qp->lf.lf_id);
+	plt_print("");
+	plt_print("S/w pending queue:");
+	plt_print("\tHead: %ld", pend_q->head);
+	plt_print("\tTail: %ld", pend_q->tail);
+	plt_print("\tMask: 0x%lx", pend_q->pq_mask);
+	plt_print("\tInflight count: %ld",
+		  pending_queue_infl_cnt(pend_q->head, pend_q->tail,
+					 pend_q->pq_mask));
+
+	plt_print("");
+	plt_print("H/w pending queue:");
+
+	lf_inprog.u = plt_read64(qp->lf.rbase + CPT_LF_INPROG);
+	inflight = lf_inprog.s.inflight;
+	plt_print("\tInflight in engines: %ld", inflight);
+
+	inst_ptr.u = plt_read64(qp->lf.rbase + CPT_LF_Q_INST_PTR);
+
+	enq_ptr = inst_ptr.s.nq_ptr;
+	deq_ptr = inst_ptr.s.dq_ptr;
+
+	if (enq_ptr >= deq_ptr)
+		insts = enq_ptr - deq_ptr;
+	else
+		insts = (enq_ptr + pend_q->pq_mask + 1 + 320 + 40) - deq_ptr;
+
+	plt_print("\tNQ ptr: 0x%lx", enq_ptr);
+	plt_print("\tDQ ptr: 0x%lx", deq_ptr);
+	plt_print("Insts waiting in CPT: %ld", insts);
+
+	plt_print("");
+	roc_cpt_afs_print(qp->lf.roc_cpt);
+}
diff --git a/drivers/crypto/cnxk/cnxk_cryptodev_ops.h b/drivers/crypto/cnxk/cnxk_cryptodev_ops.h
index 0336ae1..e521f07 100644
--- a/drivers/crypto/cnxk/cnxk_cryptodev_ops.h
+++ b/drivers/crypto/cnxk/cnxk_cryptodev_ops.h
@@ -122,6 +122,7 @@  int cnxk_ae_session_cfg(struct rte_cryptodev *dev,
 			struct rte_crypto_asym_xform *xform,
 			struct rte_cryptodev_asym_session *sess,
 			struct rte_mempool *pool);
+void cnxk_cpt_dump_on_err(struct cnxk_cpt_qp *qp);
 
 static inline union rte_event_crypto_metadata *
 cnxk_event_crypto_mdata_get(struct rte_crypto_op *op)