On Wed, Dec 13, 2023 at 1:50 PM Leon Romanovsky <leon@xxxxxxxxxx> wrote:
>
> On Tue, Dec 12, 2023 at 07:33:31PM -0800, Selvin Xavier wrote:
> > Gen P7 adapters needs to share a toggle bits information received
> > in kernel driver with the user space. User space needs this
> > info during the request notify call back to arm the CQ.
> >
> > User space application can get this page using the
> > UAPI routines. Library will mmap this page and get the
> > toggle bits to be used in the next ARM Doorbell.
> >
> > Uses a hash list to map the CQ structure from the CQ ID.
> > CQ structure is retrieved from the hash list while the
> > library calls the UAPI routine to get the toggle page
> > mapping. Currently the full page is mapped per CQ. This
> > can be optimized to enable multiple CQs from the same
> > application share the same page and different offsets
> > in the page.
> >
> > Signed-off-by: Selvin Xavier <selvin.xavier@xxxxxxxxxxxx>
> > ---
> > drivers/infiniband/hw/bnxt_re/bnxt_re.h | 3 ++
> > drivers/infiniband/hw/bnxt_re/ib_verbs.c | 59 +++++++++++++++++++++++++++----
> > drivers/infiniband/hw/bnxt_re/ib_verbs.h | 2 ++
> > drivers/infiniband/hw/bnxt_re/main.c | 10 +++++-
> > drivers/infiniband/hw/bnxt_re/qplib_res.h | 6 ++++
> > include/uapi/rdma/bnxt_re-abi.h | 5 +++
> > 6 files changed, 77 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/infiniband/hw/bnxt_re/bnxt_re.h b/drivers/infiniband/hw/bnxt_re/bnxt_re.h
> > index 9fd9849..9dca451 100644
> > --- a/drivers/infiniband/hw/bnxt_re/bnxt_re.h
> > +++ b/drivers/infiniband/hw/bnxt_re/bnxt_re.h
> > @@ -41,6 +41,7 @@
> > #define __BNXT_RE_H__
> > #include <rdma/uverbs_ioctl.h>
> > #include "hw_counters.h"
> > +#include <linux/hashtable.h>
> > #define ROCE_DRV_MODULE_NAME "bnxt_re"
> >
> > #define BNXT_RE_DESC "Broadcom NetXtreme-C/E RoCE Driver"
> > @@ -135,6 +136,7 @@ struct bnxt_re_pacing {
> > #define BNXT_RE_DB_FIFO_ROOM_SHIFT 15
> > #define BNXT_RE_GRC_FIFO_REG_BASE 0x2000
> >
> > +#define MAX_CQ_HASH_BITS (16)
> > struct bnxt_re_dev {
> > struct ib_device ibdev;
> > struct list_head list;
> > @@ -189,6 +191,7 @@ struct bnxt_re_dev {
> > struct bnxt_re_pacing pacing;
> > struct work_struct dbq_fifo_check_work;
> > struct delayed_work dbq_pacing_work;
> > + DECLARE_HASHTABLE(cq_hash, MAX_CQ_HASH_BITS);
> > };
> >
> > #define to_bnxt_re_dev(ptr, member) \
> > diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > index 76cea30..de3d404 100644
> > --- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > +++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > @@ -50,6 +50,7 @@
> > #include <rdma/ib_mad.h>
> > #include <rdma/ib_cache.h>
> > #include <rdma/uverbs_ioctl.h>
> > +#include <linux/hashtable.h>
> >
> > #include "bnxt_ulp.h"
> >
> > @@ -2910,14 +2911,20 @@ int bnxt_re_post_recv(struct ib_qp *ib_qp, const struct ib_recv_wr *wr,
> > /* Completion Queues */
> > int bnxt_re_destroy_cq(struct ib_cq *ib_cq, struct ib_udata *udata)
> > {
> > - struct bnxt_re_cq *cq;
> > + struct bnxt_qplib_chip_ctx *cctx;
> > struct bnxt_qplib_nq *nq;
> > struct bnxt_re_dev *rdev;
> > + struct bnxt_re_cq *cq;
> >
> > cq = container_of(ib_cq, struct bnxt_re_cq, ib_cq);
> > rdev = cq->rdev;
> > nq = cq->qplib_cq.nq;
> > + cctx = rdev->chip_ctx;
> >
> > + if (cctx->modes.toggle_bits & BNXT_QPLIB_CQ_TOGGLE_BIT) {
> > + free_page((unsigned long)cq->uctx_cq_page);
> > + hash_del(&cq->hash_entry);
> > + }
> > bnxt_qplib_destroy_cq(&rdev->qplib_res, &cq->qplib_cq);
> > ib_umem_release(cq->umem);
> >
> > @@ -2935,10 +2942,11 @@ int bnxt_re_create_cq(struct ib_cq *ibcq, const struct ib_cq_init_attr *attr,
> > struct bnxt_re_ucontext *uctx =
> > rdma_udata_to_drv_context(udata, struct bnxt_re_ucontext, ib_uctx);
> > struct bnxt_qplib_dev_attr *dev_attr = &rdev->dev_attr;
> > - int rc, entries;
> > - int cqe = attr->cqe;
> > + struct bnxt_qplib_chip_ctx *cctx;
> > struct bnxt_qplib_nq *nq = NULL;
> > unsigned int nq_alloc_cnt;
> > + int rc = -1, entries;
>
> Why -1 and not some -EXXX value?
Sure. I will update it to an error value.
>
> > + int cqe = attr->cqe;
> > u32 active_cqs;
> >
> > if (attr->flags)
> > @@ -2951,6 +2959,7 @@ int bnxt_re_create_cq(struct ib_cq *ibcq, const struct ib_cq_init_attr *attr,
> > }
> >
> > cq->rdev = rdev;
> > + cctx = rdev->chip_ctx;
> > cq->qplib_cq.cq_handle = (u64)(unsigned long)(&cq->qplib_cq);
> >
> > entries = bnxt_re_init_depth(cqe + 1, uctx);
> > @@ -3012,8 +3021,16 @@ int bnxt_re_create_cq(struct ib_cq *ibcq, const struct ib_cq_init_attr *attr,
> > spin_lock_init(&cq->cq_lock);
> >
> > if (udata) {
> > - struct bnxt_re_cq_resp resp;
> > -
> > + struct bnxt_re_cq_resp resp = {};
> > +
> > + if (cctx->modes.toggle_bits & BNXT_QPLIB_CQ_TOGGLE_BIT) {
>
> This bit is set for all gen_p7 cards, but bnxt_re_cq_resp doesn't have
> comp_mask field in old rdma-core and it will cause to ib_copy_to_udata()
> fail, isn't it?
Yes. I should have used min of (resp_size and outlen) like below. Will
change it.
rc = ib_copy_to_udata(udata, &resp, min(sizeof(resp), udata->outlen));
>
> > + hash_add(rdev->cq_hash, &cq->hash_entry, cq->qplib_cq.id);
> > + /* Allocate a page */
> > + cq->uctx_cq_page = (void *)get_zeroed_page(GFP_KERNEL);
> > + if (!cq->uctx_cq_page)
> > + goto c2fail;
> > + resp.comp_mask |= BNXT_RE_CQ_TOGGLE_PAGE_SUPPORT;
> > + }
>
> Thanks
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature
