Re: [PATCH v2 for-next] RDMA/hns: Add a new mmap implementation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Sorry, the reply is incomplete, please ignore this email.

On 2021/10/22 16:56, Wenpeng Liang wrote:
> On 2021/10/21 7:15, Jason Gunthorpe wrote:
>> On Tue, Oct 12, 2021 at 08:41:55PM +0800, Wenpeng Liang wrote:
>>> From: Chengchang Tang <tangchengchang@xxxxxxxxxx>
>>>
>>> Add a new implementation for mmap by using the new mmap entry API.
>>>
>>> The new implementation prepares for subsequent features and is compatible
>>> with the old implementation. And the old implementation using hard-coded
>>> offset will not be extended in the future.
>>>
>>> Signed-off-by: Chengchang Tang <tangchengchang@xxxxxxxxxx>
>>> Signed-off-by: Wenpeng Liang <liangwenpeng@xxxxxxxxxx>
>>>  drivers/infiniband/hw/hns/hns_roce_device.h |  23 +++
>>>  drivers/infiniband/hw/hns/hns_roce_main.c   | 208 +++++++++++++++++---
>>>  include/uapi/rdma/hns-abi.h                 |  21 +-
>>>  3 files changed, 225 insertions(+), 27 deletions(-)
>>>
>>> diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
>>> index 9467c39e3d28..1d4cf3f083c2 100644
>>> +++ b/drivers/infiniband/hw/hns/hns_roce_device.h
>>> @@ -225,11 +225,25 @@ struct hns_roce_uar {
>>>  	unsigned long	logic_idx;
>>>  };
>>>  
>>> +struct hns_user_mmap_entry {
>>> +	struct rdma_user_mmap_entry rdma_entry;
>>> +	u64 address;
>>> +	u8 mmap_flag;
>>
>> Call this mmap_type and use the enum:
>>
>>  enum hns_roce_mmap_type mmap_type
>>
> 
> I will fix it in v3, thanks!
> 
>>> +struct hns_user_mmap_entry *hns_roce_user_mmap_entry_insert(
>>> +				struct ib_ucontext *ucontext, u64 address,
>>> +				size_t length, u8 mmap_flag)
>>> +{
>>> +#define HNS_ROCE_PGOFFSET_TPTR 1
>>> +#define HNS_ROCE_PGOFFSET_DB 0
>>> +	struct hns_roce_ucontext *context = to_hr_ucontext(ucontext);
>>> +	struct hns_user_mmap_entry *entry;
>>> +	int ret;
>>> +
>>> +	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
>>> +	if (!entry)
>>> +		return NULL;
>>> +
>>> +	entry->address = address;
>>> +	entry->mmap_flag = mmap_flag;
>>> +
>>> +	if (context->mmap_key_support) {
>>> +		ret = rdma_user_mmap_entry_insert(ucontext, &entry->rdma_entry,
>>> +						  length);
>>> +	} else {
>>> +		switch (mmap_flag) {
>>> +		case HNS_ROCE_MMAP_TYPE_DB:
>>> +			ret = rdma_user_mmap_entry_insert_range(ucontext,
>>> +						&entry->rdma_entry, length,
>>> +						HNS_ROCE_PGOFFSET_DB,
>>> +						HNS_ROCE_PGOFFSET_DB);
>>
>> Please add this to avoid the odd #defines:
>>
>> static inline int
>> rdma_user_mmap_entry_insert_exact(struct ib_ucontext *ucontext,
>>                                   struct rdma_user_mmap_entry *entry,
>>                                   size_t length, u32 pgoff)
>> {
>>         return rdma_user_mmap_entry_insert_range(ucontext, entry, length, pgoff,
>>                                                  pgoff);
>> }
>>
> 
> ditto.
> 
>>> -static int hns_roce_mmap(struct ib_ucontext *context,
>>> -			 struct vm_area_struct *vma)
>>> +static int hns_roce_mmap(struct ib_ucontext *uctx, struct vm_area_struct *vma)
>>>  {
>>> -	struct hns_roce_dev *hr_dev = to_hr_dev(context->device);
>>> -
>>> -	switch (vma->vm_pgoff) {
>>> -	case 0:
>>> -		return rdma_user_mmap_io(context, vma,
>>> -					 to_hr_ucontext(context)->uar.pfn,
>>> -					 PAGE_SIZE,
>>> -					 pgprot_noncached(vma->vm_page_prot),
>>> -					 NULL);
>>> -
>>> -	/* vm_pgoff: 1 -- TPTR */
>>> -	case 1:
>>> -		if (!hr_dev->tptr_dma_addr || !hr_dev->tptr_size)
>>> -			return -EINVAL;
>>> +	struct hns_roce_dev *hr_dev = to_hr_dev(uctx->device);
>>> +	struct ib_device *ibdev = &hr_dev->ib_dev;
>>> +	struct rdma_user_mmap_entry *rdma_entry;
>>> +	struct hns_user_mmap_entry *entry;
>>> +	phys_addr_t pfn;
>>> +	pgprot_t prot;
>>> +	int ret;
>>> +
>>> +	rdma_entry = rdma_user_mmap_entry_get_pgoff(uctx, vma->vm_pgoff);
>>> +	if (!rdma_entry) {
>>> +		ibdev_err(ibdev, "Invalid entry vm_pgoff %lu.\n",
>>> +			  vma->vm_pgoff);
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	entry = to_hns_mmap(rdma_entry);
>>> +	pfn = entry->address >> PAGE_SHIFT;
>>> +	prot = vma->vm_page_prot;
>>
>> Just write
>>
>>  if (entry->mmap_type != HNS_ROCE_MMAP_TYPE_TPTR)
>>     prot = pgprot_noncached(prot);
>>  ret = rdma_user_mmap_io(uctx, vma, pfn,
>> 			rdma_entry->npages * PAGE_SIZE,
>> 			pgprot_noncached(prot), rdma_entry);
>>
>> No need for the big case statement
>>
> 
> There will be new features added to this switch branch in a later patch.
> 
>>> diff --git a/include/uapi/rdma/hns-abi.h b/include/uapi/rdma/hns-abi.h
>>> index 42b177655560..ce1e39f21d73 100644
>>> +++ b/include/uapi/rdma/hns-abi.h
>>> @@ -83,11 +83,30 @@ struct hns_roce_ib_create_qp_resp {
>>>  	__aligned_u64 cap_flags;
>>>  };
>>>  
>>> +enum hns_roce_alloc_uctx_comp_flag {
>>> +	HNS_ROCE_ALLOC_UCTX_COMP_CONFIG = 1 << 0,
>>> +};
>>> +
>>> +enum hns_roce_alloc_uctx_resp_config {
>>> +	HNS_ROCE_UCTX_RESP_MMAP_KEY_EN = 1 << 0,
>>> +};
>>> +
>>> +enum hns_roce_alloc_uctx_req_config {
>>> +	HNS_ROCE_UCTX_REQ_MMAP_KEY_EN = 1 << 0,
>>> +};
>>> +
>>> +struct hns_roce_ib_alloc_ucontext {
>>> +	__u32 comp;
>>> +	__u32 config;
>>> +};
>>> +
>>>  struct hns_roce_ib_alloc_ucontext_resp {
>>>  	__u32	qp_tab_size;
>>>  	__u32	cqe_size;
>>>  	__u32	srq_tab_size;
>>> -	__u32	reserved;
>>> +	__u8    config;
>>> +	__u8    rsv[3];
>>> +	__aligned_u64 db_mmap_key;
>>
>> I'm confused, this doesn't change the uAPI, so why add this stuff?
>> This should go in a later patch?
>>
>> Jason
>> .
>>
> 
> The related userspace series is named "libhns: Add a new mmap implementation".
> 
> Thanks,
> Wenpeng
> .
> 



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux