Re: [PATCH rdma-next v2 4/4] RDMA/bnxt_re: Direct Verbs: Support CQ and QP verbs

[Leon Romanovsky <leon@kernel.org>]

On Tue, Nov 04, 2025 at 12:53:20PM +0530, Sriharsha Basavapatna wrote:
> The following Direct Verb (DV) methods have been implemented in
> this patch.
> 
> CQ Direct Verbs:
> ----------------
> - BNXT_RE_METHOD_DV_CREATE_CQ:
>   Create a CQ of requested size (cqe). The application must have
>   already registered this memory with the driver using DV_UMEM_REG.
>   The CQ umem-handle and umem-offset are passed to the driver. The
>   driver now maps/pins the CQ user memory and registers it with the
>   hardware. The driver returns a CQ-handle to the application.
> 
> - BNXT_RE_METHOD_DV_DESTROY_CQ:
>   Destroy the DV_CQ specified by the CQ-handle; unmap the user memory.
> 
> QP Direct Verbs:
> ----------------
> - BNXT_RE_METHOD_DV_CREATE_QP:
>   Create a QP using specified params (struct bnxt_re_dv_create_qp_req).
>   The application must have already registered SQ/RQ memory with the
>   driver using DV_UMEM_REG. The SQ/RQ umem-handle and umem-offset are
>   passed to the driver. The driver now maps/pins the SQ/RQ user memory
>   and registers it with the hardware. The driver returns a QP-handle to
>   the application.
> 
> - BNXT_RE_METHOD_DV_DESTROY_QP:
>   Destroy the DV_QP specified by the QP-handle; unmap SQ/RQ user memory.
> 
> - BNXT_RE_METHOD_DV_MODIFY_QP:
>   Modify QP attributes for the DV_QP specified by the QP-handle;
>   wrapper functions have been implemented to resolve dmac/smac using
>   rdma_resolve_ip().
> 
> - BNXT_RE_METHOD_DV_QUERY_QP:
>   Return QP attributes for the DV_QP specified by the QP-handle.
> 
> Note:
> -----
> Some applications might want to allocate memory for all resources of a
> given type (CQ/QP) in one big chunk and then register that entire memory
> once using DV_UMEM_REG. At the time of creating each individual
> resource, the application would pass a specific offset/length in the
> umem registered memory.
> 
> - The DV_UMEM_REG handler (previous patch) only creates a dv_umem object
>   and saves user memory parameters, but doesn't really map/pin this
>   memory.
> - The mapping would be done at the time of creating individual objects.
> - This actual mapping of specific umem offsets is implemented by the
>   function bnxt_re_dv_umem_get(). This function validates the
>   umem-offset and size parameters passed during CQ/QP creation. If the
>   request is valid, it maps the specified offset/length within the umem
>   registered memory.
> - The CQ and QP creation DV handlers call bnxt_re_dv_umem_get() to map
>   offsets/sizes specific to each individual object. This means each
>   object gets its own mapped dv_umem object that is distinct from the
>   main dv_umem object created during DV_UMEM_REG.
> - The object specific dv_umem is unmapped when the object is destroyed.
> 
> Signed-off-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com>
> Co-developed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> Co-developed-by: Selvin Xavier <selvin.xavier@broadcom.com>
> Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
> ---
>  drivers/infiniband/hw/bnxt_re/bnxt_re.h  |   12 +-
>  drivers/infiniband/hw/bnxt_re/dv.c       | 1208 ++++++++++++++++++++++
>  drivers/infiniband/hw/bnxt_re/ib_verbs.c |   55 +-
>  drivers/infiniband/hw/bnxt_re/ib_verbs.h |   12 +
>  include/uapi/rdma/bnxt_re-abi.h          |   93 ++
>  5 files changed, 1364 insertions(+), 16 deletions(-)

<...>

> +		if (IS_ERR(umem_dmabuf)) {
> +			rc = PTR_ERR(umem_dmabuf);
> +			dev_err(rdev_to_dev(rdev),
> +				"%s: failed to get umem dmabuf : %d\n",
> +				__func__, rc);

All these dev_XXX() lines should go. They can be used before IB device
is created, after that you are invited to use ibdev_XXX() helpers.

> +			goto free_umem;

<...>

> +static void
> +bnxt_re_print_dv_qp_attr(struct bnxt_re_dev *rdev,
> +			 struct bnxt_re_cq *send_cq,
> +			 struct bnxt_re_cq *recv_cq,
> +			 struct  bnxt_re_dv_create_qp_req *req)
> +{
> +	dev_dbg(rdev_to_dev(rdev), "DV_QP_ATTR:\n");
> +	dev_dbg(rdev_to_dev(rdev),
> +		"\t qp_type: 0x%x pdid: 0x%x qp_handle: 0x%llx\n",
> +		req->qp_type, req->pd_id, req->qp_handle);
> +
> +	dev_dbg(rdev_to_dev(rdev), "\t SQ ATTR:\n");
> +	dev_dbg(rdev_to_dev(rdev),
> +		"\t\t max_send_wr: 0x%x max_send_sge: 0x%x\n",
> +		req->max_send_wr, req->max_send_sge);
> +	dev_dbg(rdev_to_dev(rdev),
> +		"\t\t va: 0x%llx len: 0x%x slots: 0x%x wqe_sz: 0x%x\n",
> +		req->sq_va, req->sq_len, req->sq_slots, req->sq_wqe_sz);
> +	dev_dbg(rdev_to_dev(rdev), "\t\t psn_sz: 0x%x npsn: 0x%x\n",
> +		req->sq_psn_sz, req->sq_npsn);
> +	dev_dbg(rdev_to_dev(rdev),
> +		"\t\t send_cq_id: 0x%x\n", send_cq->qplib_cq.id);
> +
> +	dev_dbg(rdev_to_dev(rdev), "\t RQ ATTR:\n");
> +	dev_dbg(rdev_to_dev(rdev),
> +		"\t\t max_recv_wr: 0x%x max_recv_sge: 0x%x\n",
> +		req->max_recv_wr, req->max_recv_sge);
> +	dev_dbg(rdev_to_dev(rdev),
> +		"\t\t va: 0x%llx len: 0x%x slots: 0x%x wqe_sz: 0x%x\n",
> +		req->rq_va, req->rq_len, req->rq_slots, req->rq_wqe_sz);
> +	dev_dbg(rdev_to_dev(rdev),
> +		"\t\t recv_cq_id: 0x%x\n", recv_cq->qplib_cq.id);
> +}

And I afraid that you went too far with debug prints in this patch.
Please remove ALL of them and leave only minimal number of error prints.

Thanks