On 03/07/2019 1:31, Jason Gunthorpe wrote:
>> Seems except Mellanox + hns the mmap flags aren't ABI.
>> Also, current Mellanox code seems like it won't benefit from
>> mmap cookie helper functions in any case as the mmap function is very specific and the flags used indicate
>> the address and not just how to map it.
>
> IMHO, mlx5 has a goofy implementaiton here as it codes all of the object
> type, handle and cachability flags in one thing.
Do we need object type flags as well in the generic mmap code?
>
>> For most drivers (efa, qedr, siw, cxgb3/4, ocrdma) mmap is called on
>> address received by kernel in some response. Meaning driver can
>> write anything in the response that will serve as the key / flag.
>> Other drivers ( i40iw ) have a simple mmap function that doesn't
>> require a mmap database at all.
>
> Are you sure? I thought the reason to have to flags at all was so that
> userspace could specify different cachability..
>
> Otherwise the offset should just be an opaque cookie and internal xa
> should specify the cachability mode..
That was my intention as well. The driver returns a "hint" flag to the
user, but the userspace library can override it with its own cachability flags.
However, I now notice EFA lost that functionality when it was merged
as the mmap function looks at 'entry->mmap_flag' (hint) instead of the
given offset flag:
static int __efa_mmap(struct efa_dev *dev, struct efa_ucontext *ucontext,
struct vm_area_struct *vma, u64 key, u64 length)
{
struct efa_mmap_entry *entry;
unsigned long va;
u64 pfn;
int err;
entry = mmap_entry_get(dev, ucontext, key, length);
if (!entry) {
ibdev_dbg(&dev->ibdev, "key[%#llx] does not have valid entry\n",
key);
return -EINVAL;
}
ibdev_dbg(&dev->ibdev,
"Mapping address[%#llx], length[%#llx], mmap_flag[%d]\n",
entry->address, length, entry->mmap_flag);
pfn = entry->address >> PAGE_SHIFT;
switch (entry->mmap_flag) {
^^^^^^^^^^^^^^^^^^^^^^^^^
case EFA_MMAP_IO_NC:
err = rdma_user_mmap_io(&ucontext->ibucontext, vma, pfn, length,
pgprot_noncached(vma->vm_page_prot));
break;
case EFA_MMAP_IO_WC:
err = rdma_user_mmap_io(&ucontext->ibucontext, vma, pfn, length,
pgprot_writecombine(vma->vm_page_prot));
break;
case EFA_MMAP_DMA_PAGE:
for (va = vma->vm_start; va < vma->vm_end;
va += PAGE_SIZE, pfn++) {
err = vm_insert_page(vma, va, pfn_to_page(pfn));
if (err)
break;
}
break;
default:
err = -EINVAL;
}
if (err) {
ibdev_dbg(
&dev->ibdev,
"Couldn't mmap address[%#llx] length[%#llx] mmap_flag[%d] err[%d]\n",
entry->address, length, entry->mmap_flag, err);
return err;
}
return 0;
}
Another issue is that these flags aren't exposed in an ABI file, so a userspace library can't
really make use of it in current state.
