Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <942fdca3-2cd4-88d2-033c-6bd3f7bb258a@fujitsu.com>
Date:   Thu, 29 Sep 2022 14:21:24 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [for-next PATCH v5 03/11] RDMA: Extend RDMA kernel verbs ABI to
 support flush
Content-Language: en-US
To:     Bob Pearson <rpearsonhpe@gmail.com>,
        Leon Romanovsky <leon@kernel.org>,
        Jason Gunthorpe <jgg@ziepe.ca>, linux-rdma@vger.kernel.org
Cc:     Zhu Yanjun <zyjzyj2000@gmail.com>, yangx.jy@fujitsu.com,
        y-goto@fujitsu.com, mbloch@nvidia.com, liangwenpeng@huawei.com,
        tom@talpey.com, tomasz.gromadzki@intel.com,
        dan.j.williams@intel.com, linux-kernel@vger.kernel.org
References: <20220927055337.22630-1-lizhijian@fujitsu.com>
 <20220927055337.22630-4-lizhijian@fujitsu.com>
From:   Li Zhijian <lizhijian@fujitsu.com>
In-Reply-To: <20220927055337.22630-4-lizhijian@fujitsu.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

Leon, Jason


On 27/09/2022 13:53, Li Zhijian wrote:
>   /*
> @@ -4321,6 +4330,8 @@ int ib_dealloc_xrcd_user(struct ib_xrcd *xrcd, struct ib_udata *udata);
>   static inline int ib_check_mr_access(struct ib_device *ib_dev,
>   				     unsigned int flags)
>   {
> +	u64 device_cap = ib_dev->attrs.device_cap_flags;
> +
>   	/*
>   	 * Local write permission is required if remote write or
>   	 * remote atomic permission is also requested.
> @@ -4335,6 +4346,13 @@ static inline int ib_check_mr_access(struct ib_device *ib_dev,
>   	if (flags & IB_ACCESS_ON_DEMAND &&
>   	    !(ib_dev->attrs.kernel_cap_flags & IBK_ON_DEMAND_PAGING))
>   		return -EINVAL;
> +
> +	if ((flags & IB_ACCESS_FLUSH_GLOBAL &&
> +	    !(device_cap & IB_DEVICE_FLUSH_GLOBAL)) ||
> +	    (flags & IB_ACCESS_FLUSH_PERSISTENT &&
> +	    !(device_cap & IB_DEVICE_FLUSH_PERSISTENT)))
> +		return -EINVAL;
> +
Regarding of the return value of ib_check_mr_access. While updating the man page of ibv_reg_mr(3) of rdma-core,
```
        IBV_ACCESS_REMOTE_READ Enable Remote Read Access
        IBV_ACCESS_REMOTE_ATOMIC Enable Remote Atomic Operation Access (if supported)
        IBV_ACCESS_MW_BIND Enable Memory Window Binding
        IBV_ACCESS_ZERO_BASED  Use  byte offset from beginning of MR to access this MR, instead of a pointer address
        IBV_ACCESS_ON_DEMAND Create an on-demand paging MR (if supported)
...
RETURN VALUE
        ibv_reg_mr() / ibv_reg_mr_iova() / ibv_reg_dmabuf_mr() returns a pointer to the registered MR, or NULL if the request fails.  The local key (L_Key) field lkey is used as the lkey field of struct  ibv_sge  when  posting
        buffers  with  ibv_post_* verbs, and the the remote key (R_Key) field rkey is used by remote processes to perform Atomic and RDMA operations.  The remote process places this rkey as the rkey field of struct ibv_send_wr
        passed to the ibv_post_send function.
```
we can see, IBV_ACCESS_REMOTE_ATOMIC and IBV_ACCESS_ON_DEMAND are tagged "if supported" . but currently kernel
just returns EINVAL when user registers a MR with IB_ACCESS_ON_DEMAND to RXE.

I wonder we should return -EOPNOTSUPP if the device doesn't support requested capabilities

Thanks
Li


>   	return 0;
>   }
>