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

[Zhu Yanjun <yanjun.zhu@linux.dev>]

在 2024/8/12 18:08, Leon Romanovsky 写道:
> 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.

Yeah. I mistook that these 2 functions were in *.c file.

Thanks.

Zhu Yanjun

>
> 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__ */

-- 
Best Regards,
Yanjun.Zhu