> From: Gal Pressman <galpress@xxxxxxxxxx>
> Sent: Wednesday, July 3, 2019 11:20 AM
>
> 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.
Can you give an example of a use-case where the user would want to override the kernel hint ?
Do you plan on changing this and exposing the flags to ABI?
