[PATCH for-next 0/2] RDMA/bnxt_re: Share the CQ pages for GenP7 adapters

[PATCH for-next 0/2] RDMA/bnxt_re: Share the CQ pages for GenP7 adapters

[Selvin Xavier]

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

This is the follow up series that adds support for GenP7
Adapters. Implements mechanism to share the toggle bits
received from the HW to user space. Adds a new UAPI routine as
the toggle bit is required for both CQ and SRQ. Current
patch series is adding support for only CQ.

Please review and apply. Will be posting the user lib changes soon.

Thanks,
Selvin Xavier

Selvin Xavier (2):
  RDMA/bnxt_re: Add UAPI to share a page with user space
  RDMA/bnxt_re: Share a page to expose per CQ info with userspace

 drivers/infiniband/hw/bnxt_re/bnxt_re.h   |   3 +
 drivers/infiniband/hw/bnxt_re/ib_verbs.c  | 165 ++++++++++++++++++++++++++++--
 drivers/infiniband/hw/bnxt_re/ib_verbs.h  |   3 +
 drivers/infiniband/hw/bnxt_re/main.c      |  10 +-
 drivers/infiniband/hw/bnxt_re/qplib_res.h |   6 ++
 include/uapi/rdma/bnxt_re-abi.h           |  31 ++++++
 6 files changed, 210 insertions(+), 8 deletions(-)

-- 
2.5.5


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

[PATCH for-next 1/2] RDMA/bnxt_re: Add UAPI to share a page with user space

[Selvin Xavier]

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

Gen P7 adapters require to share a toggle value for CQ
and SRQ. This is received by the driver as part of
interrupt notifications and needs to be shared with the
user space. Add a new UAPI infrastructure to get the
shared page for CQ and SRQ.

Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/ib_verbs.c | 106 +++++++++++++++++++++++++++++++
 drivers/infiniband/hw/bnxt_re/ib_verbs.h |   1 +
 include/uapi/rdma/bnxt_re-abi.h          |  26 ++++++++
 3 files changed, 133 insertions(+)

diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
index e7ef099..76cea30 100644
--- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
+++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
@@ -567,6 +567,7 @@ bnxt_re_mmap_entry_insert(struct bnxt_re_ucontext *uctx, u64 mem_offset,
 	case BNXT_RE_MMAP_WC_DB:
 	case BNXT_RE_MMAP_DBR_BAR:
 	case BNXT_RE_MMAP_DBR_PAGE:
+	case BNXT_RE_MMAP_TOGGLE_PAGE:
 		ret = rdma_user_mmap_entry_insert(&uctx->ib_uctx,
 						  &entry->rdma_entry, PAGE_SIZE);
 		break;
@@ -4254,6 +4255,7 @@ int bnxt_re_mmap(struct ib_ucontext *ib_uctx, struct vm_area_struct *vma)
 					rdma_entry);
 		break;
 	case BNXT_RE_MMAP_DBR_PAGE:
+	case BNXT_RE_MMAP_TOGGLE_PAGE:
 		/* Driver doesn't expect write access for user space */
 		if (vma->vm_flags & VM_WRITE)
 			return -EFAULT;
@@ -4430,8 +4432,112 @@ DECLARE_UVERBS_NAMED_METHOD(BNXT_RE_METHOD_NOTIFY_DRV);
 DECLARE_UVERBS_GLOBAL_METHODS(BNXT_RE_OBJECT_NOTIFY_DRV,
 			      &UVERBS_METHOD(BNXT_RE_METHOD_NOTIFY_DRV));
 
+/* Toggle MEM */
+static int UVERBS_HANDLER(BNXT_RE_METHOD_GET_TOGGLE_MEM)(struct uverbs_attr_bundle *attrs)
+{
+	struct ib_uobject *uobj = uverbs_attr_get_uobject(attrs, BNXT_RE_TOGGLE_MEM_HANDLE);
+	enum bnxt_re_get_toggle_mem_type res_type;
+	struct bnxt_re_user_mmap_entry *entry;
+	enum bnxt_re_mmap_flag mmap_flag;
+	struct bnxt_qplib_chip_ctx *cctx;
+	struct bnxt_re_ucontext *uctx;
+	struct bnxt_re_dev *rdev;
+	u64 mem_offset;
+	u32 length;
+	u32 offset;
+	u64 addr;
+	int err;
+
+	uctx = container_of(ib_uverbs_get_ucontext(attrs), struct bnxt_re_ucontext, ib_uctx);
+	if (IS_ERR(uctx))
+		return PTR_ERR(uctx);
+
+	err = uverbs_get_const(&res_type, attrs, BNXT_RE_TOGGLE_MEM_TYPE);
+	if (err)
+		return err;
+
+	rdev = uctx->rdev;
+	cctx = rdev->chip_ctx;
+
+	switch (res_type) {
+	case BNXT_RE_CQ_TOGGLE_MEM:
+		break;
+	case BNXT_RE_SRQ_TOGGLE_MEM:
+		break;
+
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	entry = bnxt_re_mmap_entry_insert(uctx, addr, mmap_flag, &mem_offset);
+	if (!entry)
+		return -ENOMEM;
+
+	uobj->object = entry;
+	uverbs_finalize_uobj_create(attrs, BNXT_RE_TOGGLE_MEM_HANDLE);
+	err = uverbs_copy_to(attrs, BNXT_RE_TOGGLE_MEM_MMAP_PAGE,
+			     &mem_offset, sizeof(mem_offset));
+	if (err)
+		return err;
+
+	err = uverbs_copy_to(attrs, BNXT_RE_TOGGLE_MEM_MMAP_LENGTH,
+			     &length, sizeof(length));
+	if (err)
+		return err;
+
+	err = uverbs_copy_to(attrs, BNXT_RE_TOGGLE_MEM_MMAP_OFFSET,
+			     &offset, sizeof(length));
+	if (err)
+		return err;
+
+	return 0;
+}
+
+static int get_toggle_mem_obj_cleanup(struct ib_uobject *uobject,
+				      enum rdma_remove_reason why,
+				      struct uverbs_attr_bundle *attrs)
+{
+	struct  bnxt_re_user_mmap_entry *entry = uobject->object;
+
+	rdma_user_mmap_entry_remove(&entry->rdma_entry);
+	return 0;
+}
+
+DECLARE_UVERBS_NAMED_METHOD(BNXT_RE_METHOD_GET_TOGGLE_MEM,
+			    UVERBS_ATTR_IDR(BNXT_RE_TOGGLE_MEM_HANDLE,
+					    BNXT_RE_OBJECT_GET_TOGGLE_MEM,
+					    UVERBS_ACCESS_NEW,
+					    UA_MANDATORY),
+			    UVERBS_ATTR_CONST_IN(BNXT_RE_TOGGLE_MEM_TYPE,
+						 enum bnxt_re_get_toggle_mem_type,
+						 UA_MANDATORY),
+			    UVERBS_ATTR_PTR_IN(BNXT_RE_TOGGLE_MEM_RES_ID,
+					       UVERBS_ATTR_TYPE(u32),
+					       UA_MANDATORY),
+			    UVERBS_ATTR_PTR_OUT(BNXT_RE_TOGGLE_MEM_MMAP_PAGE,
+						UVERBS_ATTR_TYPE(u64),
+						UA_MANDATORY),
+			    UVERBS_ATTR_PTR_OUT(BNXT_RE_TOGGLE_MEM_MMAP_OFFSET,
+						UVERBS_ATTR_TYPE(u32),
+						UA_MANDATORY),
+			    UVERBS_ATTR_PTR_OUT(BNXT_RE_TOGGLE_MEM_MMAP_LENGTH,
+						UVERBS_ATTR_TYPE(u32),
+						UA_MANDATORY));
+
+DECLARE_UVERBS_NAMED_METHOD_DESTROY(BNXT_RE_METHOD_RELEASE_TOGGLE_MEM,
+				    UVERBS_ATTR_IDR(BNXT_RE_RELEASE_TOGGLE_MEM_HANDLE,
+						    BNXT_RE_OBJECT_GET_TOGGLE_MEM,
+						    UVERBS_ACCESS_DESTROY,
+						    UA_MANDATORY));
+
+DECLARE_UVERBS_NAMED_OBJECT(BNXT_RE_OBJECT_GET_TOGGLE_MEM,
+			    UVERBS_TYPE_ALLOC_IDR(get_toggle_mem_obj_cleanup),
+			    &UVERBS_METHOD(BNXT_RE_METHOD_GET_TOGGLE_MEM),
+			    &UVERBS_METHOD(BNXT_RE_METHOD_RELEASE_TOGGLE_MEM));
+
 const struct uapi_definition bnxt_re_uapi_defs[] = {
 	UAPI_DEF_CHAIN_OBJ_TREE_NAMED(BNXT_RE_OBJECT_ALLOC_PAGE),
 	UAPI_DEF_CHAIN_OBJ_TREE_NAMED(BNXT_RE_OBJECT_NOTIFY_DRV),
+	UAPI_DEF_CHAIN_OBJ_TREE_NAMED(BNXT_RE_OBJECT_GET_TOGGLE_MEM),
 	{}
 };
diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.h b/drivers/infiniband/hw/bnxt_re/ib_verbs.h
index 98baea9..da3fe01 100644
--- a/drivers/infiniband/hw/bnxt_re/ib_verbs.h
+++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.h
@@ -149,6 +149,7 @@ enum bnxt_re_mmap_flag {
 	BNXT_RE_MMAP_WC_DB,
 	BNXT_RE_MMAP_DBR_PAGE,
 	BNXT_RE_MMAP_DBR_BAR,
+	BNXT_RE_MMAP_TOGGLE_PAGE,
 };
 
 struct bnxt_re_user_mmap_entry {
diff --git a/include/uapi/rdma/bnxt_re-abi.h b/include/uapi/rdma/bnxt_re-abi.h
index 3342276..9b9eb10 100644
--- a/include/uapi/rdma/bnxt_re-abi.h
+++ b/include/uapi/rdma/bnxt_re-abi.h
@@ -143,6 +143,7 @@ enum bnxt_re_shpg_offt {
 enum bnxt_re_objects {
 	BNXT_RE_OBJECT_ALLOC_PAGE = (1U << UVERBS_ID_NS_SHIFT),
 	BNXT_RE_OBJECT_NOTIFY_DRV,
+	BNXT_RE_OBJECT_GET_TOGGLE_MEM,
 };
 
 enum bnxt_re_alloc_page_type {
@@ -171,4 +172,29 @@ enum bnxt_re_alloc_page_methods {
 enum bnxt_re_notify_drv_methods {
 	BNXT_RE_METHOD_NOTIFY_DRV = (1U << UVERBS_ID_NS_SHIFT),
 };
+
+/* Toggle mem */
+
+enum bnxt_re_get_toggle_mem_type {
+	BNXT_RE_CQ_TOGGLE_MEM = 0,
+	BNXT_RE_SRQ_TOGGLE_MEM,
+};
+
+enum bnxt_re_var_toggle_mem_attrs {
+	BNXT_RE_TOGGLE_MEM_HANDLE = (1U << UVERBS_ID_NS_SHIFT),
+	BNXT_RE_TOGGLE_MEM_TYPE,
+	BNXT_RE_TOGGLE_MEM_RES_ID,
+	BNXT_RE_TOGGLE_MEM_MMAP_PAGE,
+	BNXT_RE_TOGGLE_MEM_MMAP_OFFSET,
+	BNXT_RE_TOGGLE_MEM_MMAP_LENGTH,
+};
+
+enum bnxt_re_toggle_mem_attrs {
+	BNXT_RE_RELEASE_TOGGLE_MEM_HANDLE = (1U << UVERBS_ID_NS_SHIFT),
+};
+
+enum bnxt_re_toggle_mem_methods {
+	BNXT_RE_METHOD_GET_TOGGLE_MEM = (1U << UVERBS_ID_NS_SHIFT),
+	BNXT_RE_METHOD_RELEASE_TOGGLE_MEM,
+};
 #endif /* __BNXT_RE_UVERBS_ABI_H__*/
-- 
2.5.5


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

Re: [PATCH for-next 1/2] RDMA/bnxt_re: Add UAPI to share a page with user space

[Leon Romanovsky]

On Tue, Dec 12, 2023 at 07:33:30PM -0800, Selvin Xavier wrote:
> Gen P7 adapters require to share a toggle value for CQ
> and SRQ. This is received by the driver as part of
> interrupt notifications and needs to be shared with the
> user space. Add a new UAPI infrastructure to get the
> shared page for CQ and SRQ.
> 
> Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
> ---
>  drivers/infiniband/hw/bnxt_re/ib_verbs.c | 106 +++++++++++++++++++++++++++++++
>  drivers/infiniband/hw/bnxt_re/ib_verbs.h |   1 +
>  include/uapi/rdma/bnxt_re-abi.h          |  26 ++++++++
>  3 files changed, 133 insertions(+)
> 
> diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> index e7ef099..76cea30 100644
> --- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> +++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> @@ -567,6 +567,7 @@ bnxt_re_mmap_entry_insert(struct bnxt_re_ucontext *uctx, u64 mem_offset,
>  	case BNXT_RE_MMAP_WC_DB:
>  	case BNXT_RE_MMAP_DBR_BAR:
>  	case BNXT_RE_MMAP_DBR_PAGE:
> +	case BNXT_RE_MMAP_TOGGLE_PAGE:
>  		ret = rdma_user_mmap_entry_insert(&uctx->ib_uctx,
>  						  &entry->rdma_entry, PAGE_SIZE);
>  		break;
> @@ -4254,6 +4255,7 @@ int bnxt_re_mmap(struct ib_ucontext *ib_uctx, struct vm_area_struct *vma)
>  					rdma_entry);
>  		break;
>  	case BNXT_RE_MMAP_DBR_PAGE:
> +	case BNXT_RE_MMAP_TOGGLE_PAGE:
>  		/* Driver doesn't expect write access for user space */
>  		if (vma->vm_flags & VM_WRITE)
>  			return -EFAULT;
> @@ -4430,8 +4432,112 @@ DECLARE_UVERBS_NAMED_METHOD(BNXT_RE_METHOD_NOTIFY_DRV);
>  DECLARE_UVERBS_GLOBAL_METHODS(BNXT_RE_OBJECT_NOTIFY_DRV,
>  			      &UVERBS_METHOD(BNXT_RE_METHOD_NOTIFY_DRV));
>  
> +/* Toggle MEM */
> +static int UVERBS_HANDLER(BNXT_RE_METHOD_GET_TOGGLE_MEM)(struct uverbs_attr_bundle *attrs)
> +{
> +	struct ib_uobject *uobj = uverbs_attr_get_uobject(attrs, BNXT_RE_TOGGLE_MEM_HANDLE);
> +	enum bnxt_re_get_toggle_mem_type res_type;
> +	struct bnxt_re_user_mmap_entry *entry;
> +	enum bnxt_re_mmap_flag mmap_flag;
> +	struct bnxt_qplib_chip_ctx *cctx;
> +	struct bnxt_re_ucontext *uctx;
> +	struct bnxt_re_dev *rdev;
> +	u64 mem_offset;
> +	u32 length;
> +	u32 offset;
> +	u64 addr;
> +	int err;
> +
> +	uctx = container_of(ib_uverbs_get_ucontext(attrs), struct bnxt_re_ucontext, ib_uctx);
> +	if (IS_ERR(uctx))

How is it possible? You should check return value from ib_uverbs_get_ucontext() and not container_of.

> +		return PTR_ERR(uctx);
> +
> +	err = uverbs_get_const(&res_type, attrs, BNXT_RE_TOGGLE_MEM_TYPE);
> +	if (err)
> +		return err;
> +
> +	rdev = uctx->rdev;
> +	cctx = rdev->chip_ctx;
> +
> +	switch (res_type) {
> +	case BNXT_RE_CQ_TOGGLE_MEM:
> +		break;

No need in this break here.

> +	case BNXT_RE_SRQ_TOGGLE_MEM:
> +		break;
> +
> +	default:
> +		return -EOPNOTSUPP;
> +	}

Thanks

Re: [PATCH for-next 1/2] RDMA/bnxt_re: Add UAPI to share a page with user space

[Selvin Xavier]

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

On Wed, Dec 13, 2023 at 1:40 PM Leon Romanovsky <leon@kernel.org> wrote:
>
> On Tue, Dec 12, 2023 at 07:33:30PM -0800, Selvin Xavier wrote:
> > Gen P7 adapters require to share a toggle value for CQ
> > and SRQ. This is received by the driver as part of
> > interrupt notifications and needs to be shared with the
> > user space. Add a new UAPI infrastructure to get the
> > shared page for CQ and SRQ.
> >
> > Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
> > ---
> >  drivers/infiniband/hw/bnxt_re/ib_verbs.c | 106 +++++++++++++++++++++++++++++++
> >  drivers/infiniband/hw/bnxt_re/ib_verbs.h |   1 +
> >  include/uapi/rdma/bnxt_re-abi.h          |  26 ++++++++
> >  3 files changed, 133 insertions(+)
> >
> > diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > index e7ef099..76cea30 100644
> > --- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > +++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > @@ -567,6 +567,7 @@ bnxt_re_mmap_entry_insert(struct bnxt_re_ucontext *uctx, u64 mem_offset,
> >       case BNXT_RE_MMAP_WC_DB:
> >       case BNXT_RE_MMAP_DBR_BAR:
> >       case BNXT_RE_MMAP_DBR_PAGE:
> > +     case BNXT_RE_MMAP_TOGGLE_PAGE:
> >               ret = rdma_user_mmap_entry_insert(&uctx->ib_uctx,
> >                                                 &entry->rdma_entry, PAGE_SIZE);
> >               break;
> > @@ -4254,6 +4255,7 @@ int bnxt_re_mmap(struct ib_ucontext *ib_uctx, struct vm_area_struct *vma)
> >                                       rdma_entry);
> >               break;
> >       case BNXT_RE_MMAP_DBR_PAGE:
> > +     case BNXT_RE_MMAP_TOGGLE_PAGE:
> >               /* Driver doesn't expect write access for user space */
> >               if (vma->vm_flags & VM_WRITE)
> >                       return -EFAULT;
> > @@ -4430,8 +4432,112 @@ DECLARE_UVERBS_NAMED_METHOD(BNXT_RE_METHOD_NOTIFY_DRV);
> >  DECLARE_UVERBS_GLOBAL_METHODS(BNXT_RE_OBJECT_NOTIFY_DRV,
> >                             &UVERBS_METHOD(BNXT_RE_METHOD_NOTIFY_DRV));
> >
> > +/* Toggle MEM */
> > +static int UVERBS_HANDLER(BNXT_RE_METHOD_GET_TOGGLE_MEM)(struct uverbs_attr_bundle *attrs)
> > +{
> > +     struct ib_uobject *uobj = uverbs_attr_get_uobject(attrs, BNXT_RE_TOGGLE_MEM_HANDLE);
> > +     enum bnxt_re_get_toggle_mem_type res_type;
> > +     struct bnxt_re_user_mmap_entry *entry;
> > +     enum bnxt_re_mmap_flag mmap_flag;
> > +     struct bnxt_qplib_chip_ctx *cctx;
> > +     struct bnxt_re_ucontext *uctx;
> > +     struct bnxt_re_dev *rdev;
> > +     u64 mem_offset;
> > +     u32 length;
> > +     u32 offset;
> > +     u64 addr;
> > +     int err;
> > +
> > +     uctx = container_of(ib_uverbs_get_ucontext(attrs), struct bnxt_re_ucontext, ib_uctx);
> > +     if (IS_ERR(uctx))
>
> How is it possible? You should check return value from ib_uverbs_get_ucontext() and not container_of.
ok. will include in the next revision.
>
> > +             return PTR_ERR(uctx);
> > +
> > +     err = uverbs_get_const(&res_type, attrs, BNXT_RE_TOGGLE_MEM_TYPE);
> > +     if (err)
> > +             return err;
> > +
> > +     rdev = uctx->rdev;
> > +     cctx = rdev->chip_ctx;
> > +
> > +     switch (res_type) {
> > +     case BNXT_RE_CQ_TOGGLE_MEM:
> > +             break;
>
> No need in this break here.
ok. thanks
>
> > +     case BNXT_RE_SRQ_TOGGLE_MEM:
> > +             break;
> > +
> > +     default:
> > +             return -EOPNOTSUPP;
> > +     }
>
> Thanks

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

Re: [PATCH for-next 1/2] RDMA/bnxt_re: Add UAPI to share a page with user space

[kernel test robot]

Hi Selvin,

kernel test robot noticed the following build warnings:

[auto build test WARNING on rdma/for-next]
[also build test WARNING on linus/master v6.7-rc5 next-20231213]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Selvin-Xavier/RDMA-bnxt_re-Add-UAPI-to-share-a-page-with-user-space/20231213-115222
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git for-next
patch link:    https://lore.kernel.org/r/1702438411-23530-2-git-send-email-selvin.xavier%40broadcom.com
patch subject: [PATCH for-next 1/2] RDMA/bnxt_re: Add UAPI to share a page with user space
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20231213/202312132318.8DO4UiPT-lkp@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231213/202312132318.8DO4UiPT-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202312132318.8DO4UiPT-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/infiniband/hw/bnxt_re/ib_verbs.c:4442:30: warning: variable 'cctx' set but not used [-Wunused-but-set-variable]
           struct bnxt_qplib_chip_ctx *cctx;
                                       ^
   drivers/infiniband/hw/bnxt_re/ib_verbs.c:4472:42: warning: variable 'addr' is uninitialized when used here [-Wuninitialized]
           entry = bnxt_re_mmap_entry_insert(uctx, addr, mmap_flag, &mem_offset);
                                                   ^~~~
   drivers/infiniband/hw/bnxt_re/ib_verbs.c:4448:10: note: initialize the variable 'addr' to silence this warning
           u64 addr;
                   ^
                    = 0
   drivers/infiniband/hw/bnxt_re/ib_verbs.c:4472:48: warning: variable 'mmap_flag' is uninitialized when used here [-Wuninitialized]
           entry = bnxt_re_mmap_entry_insert(uctx, addr, mmap_flag, &mem_offset);
                                                         ^~~~~~~~~
   drivers/infiniband/hw/bnxt_re/ib_verbs.c:4441:2: note: variable 'mmap_flag' is declared here
           enum bnxt_re_mmap_flag mmap_flag;
           ^
   3 warnings generated.


vim +/cctx +4442 drivers/infiniband/hw/bnxt_re/ib_verbs.c

  4431	
  4432	DECLARE_UVERBS_GLOBAL_METHODS(BNXT_RE_OBJECT_NOTIFY_DRV,
  4433				      &UVERBS_METHOD(BNXT_RE_METHOD_NOTIFY_DRV));
  4434	
  4435	/* Toggle MEM */
  4436	static int UVERBS_HANDLER(BNXT_RE_METHOD_GET_TOGGLE_MEM)(struct uverbs_attr_bundle *attrs)
  4437	{
  4438		struct ib_uobject *uobj = uverbs_attr_get_uobject(attrs, BNXT_RE_TOGGLE_MEM_HANDLE);
  4439		enum bnxt_re_get_toggle_mem_type res_type;
  4440		struct bnxt_re_user_mmap_entry *entry;
  4441		enum bnxt_re_mmap_flag mmap_flag;
> 4442		struct bnxt_qplib_chip_ctx *cctx;
  4443		struct bnxt_re_ucontext *uctx;
  4444		struct bnxt_re_dev *rdev;
  4445		u64 mem_offset;
  4446		u32 length;
  4447		u32 offset;
  4448		u64 addr;
  4449		int err;
  4450	
  4451		uctx = container_of(ib_uverbs_get_ucontext(attrs), struct bnxt_re_ucontext, ib_uctx);
  4452		if (IS_ERR(uctx))
  4453			return PTR_ERR(uctx);
  4454	
  4455		err = uverbs_get_const(&res_type, attrs, BNXT_RE_TOGGLE_MEM_TYPE);
  4456		if (err)
  4457			return err;
  4458	
  4459		rdev = uctx->rdev;
  4460		cctx = rdev->chip_ctx;
  4461	
  4462		switch (res_type) {
  4463		case BNXT_RE_CQ_TOGGLE_MEM:
  4464			break;
  4465		case BNXT_RE_SRQ_TOGGLE_MEM:
  4466			break;
  4467	
  4468		default:
  4469			return -EOPNOTSUPP;
  4470		}
  4471	
  4472		entry = bnxt_re_mmap_entry_insert(uctx, addr, mmap_flag, &mem_offset);
  4473		if (!entry)
  4474			return -ENOMEM;
  4475	
  4476		uobj->object = entry;
  4477		uverbs_finalize_uobj_create(attrs, BNXT_RE_TOGGLE_MEM_HANDLE);
  4478		err = uverbs_copy_to(attrs, BNXT_RE_TOGGLE_MEM_MMAP_PAGE,
  4479				     &mem_offset, sizeof(mem_offset));
  4480		if (err)
  4481			return err;
  4482	
  4483		err = uverbs_copy_to(attrs, BNXT_RE_TOGGLE_MEM_MMAP_LENGTH,
  4484				     &length, sizeof(length));
  4485		if (err)
  4486			return err;
  4487	
  4488		err = uverbs_copy_to(attrs, BNXT_RE_TOGGLE_MEM_MMAP_OFFSET,
  4489				     &offset, sizeof(length));
  4490		if (err)
  4491			return err;
  4492	
  4493		return 0;
  4494	}
  4495	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH for-next 2/2] RDMA/bnxt_re: Share a page to expose per CQ info with userspace

[Selvin Xavier]

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

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@broadcom.com>
---
 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;
+	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) {
+			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;
+		}
 		resp.cqid = cq->qplib_cq.id;
 		resp.tail = cq->qplib_cq.hwq.cons;
 		resp.phase = cq->qplib_cq.period;
@@ -3022,12 +3039,14 @@ int bnxt_re_create_cq(struct ib_cq *ibcq, const struct ib_cq_init_attr *attr,
 		if (rc) {
 			ibdev_err(&rdev->ibdev, "Failed to copy CQ udata");
 			bnxt_qplib_destroy_cq(&rdev->qplib_res, &cq->qplib_cq);
-			goto c2fail;
+			goto free_mem;
 		}
 	}
 
 	return 0;
 
+free_mem:
+	free_page((unsigned long)cq->uctx_cq_page);
 c2fail:
 	ib_umem_release(cq->umem);
 fail:
@@ -4214,6 +4233,19 @@ void bnxt_re_dealloc_ucontext(struct ib_ucontext *ib_uctx)
 	}
 }
 
+struct bnxt_re_cq *bnxt_re_search_for_cq(struct bnxt_re_dev *rdev, u32 cq_id)
+{
+	struct bnxt_re_cq *cq = NULL, *tmp_cq;
+
+	hash_for_each_possible(rdev->cq_hash, tmp_cq, hash_entry, cq_id) {
+		if (tmp_cq->qplib_cq.id == cq_id) {
+			cq = tmp_cq;
+			break;
+		}
+	}
+	return cq;
+}
+
 /* Helper function to mmap the virtual memory from user app */
 int bnxt_re_mmap(struct ib_ucontext *ib_uctx, struct vm_area_struct *vma)
 {
@@ -4342,7 +4374,6 @@ static int UVERBS_HANDLER(BNXT_RE_METHOD_ALLOC_PAGE)(struct uverbs_attr_bundle *
 		addr = (u64)rdev->pacing.dbr_page;
 		mmap_flag = BNXT_RE_MMAP_DBR_PAGE;
 		break;
-
 	default:
 		return -EOPNOTSUPP;
 	}
@@ -4442,9 +4473,11 @@ static int UVERBS_HANDLER(BNXT_RE_METHOD_GET_TOGGLE_MEM)(struct uverbs_attr_bund
 	struct bnxt_qplib_chip_ctx *cctx;
 	struct bnxt_re_ucontext *uctx;
 	struct bnxt_re_dev *rdev;
+	struct bnxt_re_cq *cq;
 	u64 mem_offset;
 	u32 length;
 	u32 offset;
+	u32 cq_id;
 	u64 addr;
 	int err;
 
@@ -4461,6 +4494,18 @@ static int UVERBS_HANDLER(BNXT_RE_METHOD_GET_TOGGLE_MEM)(struct uverbs_attr_bund
 
 	switch (res_type) {
 	case BNXT_RE_CQ_TOGGLE_MEM:
+		err = uverbs_copy_from(&cq_id, attrs, BNXT_RE_TOGGLE_MEM_RES_ID);
+		if (err)
+			return err;
+
+		cq = bnxt_re_search_for_cq(rdev, cq_id);
+		if (!cq)
+			return -EINVAL;
+
+		length = PAGE_SIZE;
+		addr = (u64)cq->uctx_cq_page;
+		mmap_flag = BNXT_RE_MMAP_TOGGLE_PAGE;
+		offset = 0;
 		break;
 	case BNXT_RE_SRQ_TOGGLE_MEM:
 		break;
diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.h b/drivers/infiniband/hw/bnxt_re/ib_verbs.h
index da3fe01..b267d6d 100644
--- a/drivers/infiniband/hw/bnxt_re/ib_verbs.h
+++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.h
@@ -108,6 +108,8 @@ struct bnxt_re_cq {
 	struct ib_umem		*umem;
 	struct ib_umem		*resize_umem;
 	int			resize_cqe;
+	void			*uctx_cq_page;
+	struct hlist_node	hash_entry;
 };
 
 struct bnxt_re_mr {
diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
index 7f4f6db..eb03eba 100644
--- a/drivers/infiniband/hw/bnxt_re/main.c
+++ b/drivers/infiniband/hw/bnxt_re/main.c
@@ -54,6 +54,7 @@
 #include <rdma/ib_user_verbs.h>
 #include <rdma/ib_umem.h>
 #include <rdma/ib_addr.h>
+#include <linux/hashtable.h>
 
 #include "bnxt_ulp.h"
 #include "roce_hsi.h"
@@ -136,6 +137,8 @@ static void bnxt_re_set_drv_mode(struct bnxt_re_dev *rdev, u8 mode)
 	if (bnxt_re_hwrm_qcaps(rdev))
 		dev_err(rdev_to_dev(rdev),
 			"Failed to query hwrm qcaps\n");
+	if (bnxt_qplib_is_chip_gen_p7(rdev->chip_ctx))
+		cctx->modes.toggle_bits |= BNXT_QPLIB_CQ_TOGGLE_BIT;
 }
 
 static void bnxt_re_destroy_chip_ctx(struct bnxt_re_dev *rdev)
@@ -1206,9 +1209,13 @@ static int bnxt_re_cqn_handler(struct bnxt_qplib_nq *nq,
 {
 	struct bnxt_re_cq *cq = container_of(handle, struct bnxt_re_cq,
 					     qplib_cq);
+	u32 *cq_ptr;
 
 	if (cq->ib_cq.comp_handler) {
-		/* Lock comp_handler? */
+		if (cq->uctx_cq_page) {
+			cq_ptr = (u32 *)cq->uctx_cq_page;
+			*cq_ptr = cq->qplib_cq.toggle;
+		}
 		(*cq->ib_cq.comp_handler)(&cq->ib_cq, cq->ib_cq.cq_context);
 	}
 
@@ -1730,6 +1737,7 @@ static int bnxt_re_dev_init(struct bnxt_re_dev *rdev, u8 wqe_mode)
 		 */
 		bnxt_re_vf_res_config(rdev);
 	}
+	hash_init(rdev->cq_hash);
 
 	return 0;
 free_sctx:
diff --git a/drivers/infiniband/hw/bnxt_re/qplib_res.h b/drivers/infiniband/hw/bnxt_re/qplib_res.h
index 382d89f..61628f7 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_res.h
+++ b/drivers/infiniband/hw/bnxt_re/qplib_res.h
@@ -55,6 +55,12 @@ struct bnxt_qplib_drv_modes {
 	u8	wqe_mode;
 	bool db_push;
 	bool dbr_pacing;
+	u32 toggle_bits;
+};
+
+enum bnxt_re_toggle_modes {
+	BNXT_QPLIB_CQ_TOGGLE_BIT = 0x1,
+	BNXT_QPLIB_SRQ_TOGGLE_BIT = 0x2,
 };
 
 struct bnxt_qplib_chip_ctx {
diff --git a/include/uapi/rdma/bnxt_re-abi.h b/include/uapi/rdma/bnxt_re-abi.h
index 9b9eb10..c0c34ac 100644
--- a/include/uapi/rdma/bnxt_re-abi.h
+++ b/include/uapi/rdma/bnxt_re-abi.h
@@ -102,11 +102,16 @@ struct bnxt_re_cq_req {
 	__aligned_u64 cq_handle;
 };
 
+enum bnxt_re_cq_mask {
+	BNXT_RE_CQ_TOGGLE_PAGE_SUPPORT = 0x1,
+};
+
 struct bnxt_re_cq_resp {
 	__u32 cqid;
 	__u32 tail;
 	__u32 phase;
 	__u32 rsvd;
+	__aligned_u64 comp_mask;
 };
 
 struct bnxt_re_resize_cq_req {
-- 
2.5.5


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

Re: [PATCH for-next 2/2] RDMA/bnxt_re: Share a page to expose per CQ info with userspace

[Leon Romanovsky]

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@broadcom.com>
> ---
>  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?

> +	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?

> +			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

Re: [PATCH for-next 2/2] RDMA/bnxt_re: Share a page to expose per CQ info with userspace

[Selvin Xavier]

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

On Wed, Dec 13, 2023 at 1:50 PM Leon Romanovsky <leon@kernel.org> 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@broadcom.com>
> > ---
> >  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 #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4224 bytes --]

Re: [PATCH for-next 2/2] RDMA/bnxt_re: Share a page to expose per CQ info with userspace

[kernel test robot]

Hi Selvin,

kernel test robot noticed the following build warnings:

[auto build test WARNING on rdma/for-next]
[also build test WARNING on next-20231213]
[cannot apply to linus/master v6.7-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Selvin-Xavier/RDMA-bnxt_re-Add-UAPI-to-share-a-page-with-user-space/20231213-115222
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git for-next
patch link:    https://lore.kernel.org/r/1702438411-23530-3-git-send-email-selvin.xavier%40broadcom.com
patch subject: [PATCH for-next 2/2] RDMA/bnxt_re: Share a page to expose per CQ info with userspace
config: loongarch-allmodconfig (https://download.01.org/0day-ci/archive/20231214/202312140044.BB9N8UAJ-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231214/202312140044.BB9N8UAJ-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202312140044.BB9N8UAJ-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/infiniband/hw/bnxt_re/ib_verbs.c:4236:20: warning: no previous prototype for 'bnxt_re_search_for_cq' [-Wmissing-prototypes]
    4236 | struct bnxt_re_cq *bnxt_re_search_for_cq(struct bnxt_re_dev *rdev, u32 cq_id)
         |                    ^~~~~~~~~~~~~~~~~~~~~
   drivers/infiniband/hw/bnxt_re/ib_verbs.c: In function 'bnxt_re_handler_BNXT_RE_METHOD_GET_TOGGLE_MEM':
   drivers/infiniband/hw/bnxt_re/ib_verbs.c:4473:37: warning: variable 'cctx' set but not used [-Wunused-but-set-variable]
    4473 |         struct bnxt_qplib_chip_ctx *cctx;
         |                                     ^~~~


vim +/bnxt_re_search_for_cq +4236 drivers/infiniband/hw/bnxt_re/ib_verbs.c

  4235	
> 4236	struct bnxt_re_cq *bnxt_re_search_for_cq(struct bnxt_re_dev *rdev, u32 cq_id)
  4237	{
  4238		struct bnxt_re_cq *cq = NULL, *tmp_cq;
  4239	
  4240		hash_for_each_possible(rdev->cq_hash, tmp_cq, hash_entry, cq_id) {
  4241			if (tmp_cq->qplib_cq.id == cq_id) {
  4242				cq = tmp_cq;
  4243				break;
  4244			}
  4245		}
  4246		return cq;
  4247	}
  4248	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH for-next 2/2] RDMA/bnxt_re: Share a page to expose per CQ info with userspace

[kernel test robot]

Hi Selvin,

kernel test robot noticed the following build warnings:

[auto build test WARNING on rdma/for-next]
[also build test WARNING on next-20231213]
[cannot apply to linus/master v6.7-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Selvin-Xavier/RDMA-bnxt_re-Add-UAPI-to-share-a-page-with-user-space/20231213-115222
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git for-next
patch link:    https://lore.kernel.org/r/1702438411-23530-3-git-send-email-selvin.xavier%40broadcom.com
patch subject: [PATCH for-next 2/2] RDMA/bnxt_re: Share a page to expose per CQ info with userspace
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20231214/202312140331.djKEJ9zR-lkp@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231214/202312140331.djKEJ9zR-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202312140331.djKEJ9zR-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/infiniband/hw/bnxt_re/ib_verbs.c:4236:20: warning: no previous prototype for function 'bnxt_re_search_for_cq' [-Wmissing-prototypes]
   struct bnxt_re_cq *bnxt_re_search_for_cq(struct bnxt_re_dev *rdev, u32 cq_id)
                      ^
   drivers/infiniband/hw/bnxt_re/ib_verbs.c:4236:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   struct bnxt_re_cq *bnxt_re_search_for_cq(struct bnxt_re_dev *rdev, u32 cq_id)
   ^
   static 
   drivers/infiniband/hw/bnxt_re/ib_verbs.c:4473:30: warning: variable 'cctx' set but not used [-Wunused-but-set-variable]
           struct bnxt_qplib_chip_ctx *cctx;
                                       ^
>> drivers/infiniband/hw/bnxt_re/ib_verbs.c:4510:7: warning: variable 'addr' is used uninitialized whenever switch case is taken [-Wsometimes-uninitialized]
           case BNXT_RE_SRQ_TOGGLE_MEM:
                ^~~~~~~~~~~~~~~~~~~~~~
   drivers/infiniband/hw/bnxt_re/ib_verbs.c:4517:42: note: uninitialized use occurs here
           entry = bnxt_re_mmap_entry_insert(uctx, addr, mmap_flag, &mem_offset);
                                                   ^~~~
   drivers/infiniband/hw/bnxt_re/ib_verbs.c:4481:10: note: initialize the variable 'addr' to silence this warning
           u64 addr;
                   ^
                    = 0
>> drivers/infiniband/hw/bnxt_re/ib_verbs.c:4510:7: warning: variable 'mmap_flag' is used uninitialized whenever switch case is taken [-Wsometimes-uninitialized]
           case BNXT_RE_SRQ_TOGGLE_MEM:
                ^~~~~~~~~~~~~~~~~~~~~~
   drivers/infiniband/hw/bnxt_re/ib_verbs.c:4517:48: note: uninitialized use occurs here
           entry = bnxt_re_mmap_entry_insert(uctx, addr, mmap_flag, &mem_offset);
                                                         ^~~~~~~~~
   drivers/infiniband/hw/bnxt_re/ib_verbs.c:4472:2: note: variable 'mmap_flag' is declared here
           enum bnxt_re_mmap_flag mmap_flag;
           ^
   4 warnings generated.


vim +/bnxt_re_search_for_cq +4236 drivers/infiniband/hw/bnxt_re/ib_verbs.c

  4235	
> 4236	struct bnxt_re_cq *bnxt_re_search_for_cq(struct bnxt_re_dev *rdev, u32 cq_id)
  4237	{
  4238		struct bnxt_re_cq *cq = NULL, *tmp_cq;
  4239	
  4240		hash_for_each_possible(rdev->cq_hash, tmp_cq, hash_entry, cq_id) {
  4241			if (tmp_cq->qplib_cq.id == cq_id) {
  4242				cq = tmp_cq;
  4243				break;
  4244			}
  4245		}
  4246		return cq;
  4247	}
  4248	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki