Re: [PATCH for-next 2/4] RDMA/bnxt_re: Get the WQE index from slot index while completing the WQEs

[Leon Romanovsky <leon@kernel.org>]

On Mon, Aug 12, 2024 at 04:48:44PM +0800, Zhu Yanjun wrote:
> 在 2024/8/11 3:19, Selvin Xavier 写道:
> > While reporting the completions, SQ Work Queue index is required to
> > identify the WQE that generated the completions. In variable WQE mode,
> > FW returns the slot index for Error completions. Driver need to walk
> > through the shadow queue between the consumer index  and producer index
> > and matches the slot index returned by FW. If a match is found, the next
> > index of the shadow queue is the WQE index to be considered for remaining
> > poll_cq loop.
> > 
> > Signed-off-by: Hongguang Gao <hong``guang.gao@broadcom.com>
> > Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
> > ---
> >   drivers/infiniband/hw/bnxt_re/qplib_fp.c | 40 ++++++++++++++++++++++++++++++++
> >   drivers/infiniband/hw/bnxt_re/qplib_fp.h | 10 ++++++++
> >   2 files changed, 50 insertions(+)
> > 
> > diff --git a/drivers/infiniband/hw/bnxt_re/qplib_fp.c b/drivers/infiniband/hw/bnxt_re/qplib_fp.c
> > index 0af09e7..b49f49c 100644
> > --- a/drivers/infiniband/hw/bnxt_re/qplib_fp.c
> > +++ b/drivers/infiniband/hw/bnxt_re/qplib_fp.c
> > @@ -2471,6 +2471,32 @@ static int do_wa9060(struct bnxt_qplib_qp *qp, struct bnxt_qplib_cq *cq,
> >   	return rc;
> >   }
> > +static int bnxt_qplib_get_cqe_sq_cons(struct bnxt_qplib_q *sq, u32 cqe_slot)
> > +{
> > +	struct bnxt_qplib_hwq *sq_hwq;
> > +	struct bnxt_qplib_swq *swq;
> > +	int cqe_sq_cons = -1;
> > +	u32 start, last;
> > +
> > +	sq_hwq = &sq->hwq;
> > +
> > +	start = sq->swq_start;
> > +	last = sq->swq_last;
> > +
> > +	while (last != start) {
> > +		swq = &sq->swq[last];
> > +		if (swq->slot_idx  == cqe_slot) {
> > +			cqe_sq_cons = swq->next_idx;
> > +			dev_err(&sq_hwq->pdev->dev, "%s: Found cons wqe = %d slot = %d\n",
> > +				__func__, cqe_sq_cons, cqe_slot);
> > +			break;
> > +		}
> > +
> > +		last = swq->next_idx;
> > +	}
> > +	return cqe_sq_cons;
> > +}
> > +
> >   static int bnxt_qplib_cq_process_req(struct bnxt_qplib_cq *cq,
> >   				     struct cq_req *hwcqe,
> >   				     struct bnxt_qplib_cqe **pcqe, int *budget,
> > @@ -2481,6 +2507,7 @@ static int bnxt_qplib_cq_process_req(struct bnxt_qplib_cq *cq,
> >   	struct bnxt_qplib_qp *qp;
> >   	struct bnxt_qplib_q *sq;
> >   	u32 cqe_sq_cons;
> > +	int cqe_cons;
> >   	int rc = 0;
> >   	qp = (struct bnxt_qplib_qp *)((unsigned long)
> > @@ -2498,6 +2525,19 @@ static int bnxt_qplib_cq_process_req(struct bnxt_qplib_cq *cq,
> >   			"%s: QP in Flush QP = %p\n", __func__, qp);
> >   		goto done;
> >   	}
> > +
> > +	if (__is_err_cqe_for_var_wqe(qp, hwcqe->status)) {
> > +		cqe_cons = bnxt_qplib_get_cqe_sq_cons(sq, hwcqe->sq_cons_idx);
> > +		if (cqe_cons < 0) {
> > +			dev_err(&cq->hwq.pdev->dev, "%s: Wrong SQ cons cqe_slot_indx = %d\n",
> > +				__func__, hwcqe->sq_cons_idx);
> > +			goto done;
> > +		}
> > +		cqe_sq_cons = cqe_cons;
> > +		dev_err(&cq->hwq.pdev->dev, "%s: cqe_sq_cons = %d swq_last = %d swq_start = %d\n",
> > +			__func__, cqe_sq_cons, sq->swq_last, sq->swq_start);
> > +	}
> > +
> >   	/* Require to walk the sq's swq to fabricate CQEs for all previously
> >   	 * signaled SWQEs due to CQE aggregation from the current sq cons
> >   	 * to the cqe_sq_cons
> > diff --git a/drivers/infiniband/hw/bnxt_re/qplib_fp.h b/drivers/infiniband/hw/bnxt_re/qplib_fp.h
> > index f54d7a0..2e7a4fd 100644
> > --- a/drivers/infiniband/hw/bnxt_re/qplib_fp.h
> > +++ b/drivers/infiniband/hw/bnxt_re/qplib_fp.h
> > @@ -649,4 +649,14 @@ static inline __le64 bnxt_re_update_msn_tbl(u32 st_idx, u32 npsn, u32 start_psn)
> >   		(((start_psn) << SQ_MSN_SEARCH_START_PSN_SFT) &
> >   		SQ_MSN_SEARCH_START_PSN_MASK));
> >   }
> > +
> > +static inline bool __is_var_wqe(struct bnxt_qplib_qp *qp)
> 
> IIRC, inline is not needed here. The compiler will determine if the function
> inline is needed or not.

Selvin added inline functions to *.h file and not to *.c file.

Thanks

> 
> It is a trivial problem.
> 
> Zhu Yanjun
> 
> > +{
> > +	return (qp->wqe_mode == BNXT_QPLIB_WQE_MODE_VARIABLE);
> > +}
> > +
> > +static inline bool __is_err_cqe_for_var_wqe(struct bnxt_qplib_qp *qp, u8 status)
> 
> ditto.
> 
> > +{
> > +	return (status != CQ_REQ_STATUS_OK) && __is_var_wqe(qp);
> > +}
> >   #endif /* __BNXT_QPLIB_FP_H__ */
>