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

Leon Romanovsky
2025-11-09T09:49:10Z

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