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

Sriharsha Basavapatna
2025-11-10T14:58:59Z

[-- Attachment #1: Type: text/plain, Size: 6312 bytes --]

On Sun, Nov 9, 2025 at 3:19 PM Leon Romanovsky <leon@kernel.org> wrote:
>
> 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.
Thanks, will change them to ibdev_XXX().
>
> > + 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.
I'll remove the above function, which was needed during the early dev
cycle. Also, will reduce both errors and debug prints, and will keep
only a small set of dbg msgs that we have found to be useful.
Thanks,
-Harsha
>
> Thanks

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5505 bytes --]