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

[Leon Romanovsky <leon@kernel.org>]

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