On Thu, Aug 08, 2019 at 03:44:05PM +0200, Gerd Hoffmann wrote:
> drm_gem_object_funcs->vm_ops alone can't handle
> everything mmap() needs. Add a new callback for it.
>
> Signed-off-by: Gerd Hoffmann <[email protected]>
> ---
> include/drm/drm_gem.h | 9 +++++++++
> drivers/gpu/drm/drm_gem.c | 6 ++++++
> 2 files changed, 15 insertions(+)
>
> diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> index ae693c0666cd..ee3c4ad742c6 100644
> --- a/include/drm/drm_gem.h
> +++ b/include/drm/drm_gem.h
> @@ -150,6 +150,15 @@ struct drm_gem_object_funcs {
> */
> void (*vunmap)(struct drm_gem_object *obj, void *vaddr);
>
> + /**
> + * @mmap:
> + *
> + * Called by drm_gem_mmap() for additional checks/setup.
> + *
> + * This callback is optional.
> + */
> + int (*mmap)(struct drm_gem_object *obj, struct vm_area_struct *vma);
I think if we do an mmap callback, it should replace all the mmap handling
(except the drm_gem_object_get) that drm_gem_mmap_obj does. So maybe
something like the below:
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 6854f5867d51..e8b7779633dd 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1104,17 +1104,22 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj,
unsigned long obj_size,
if (obj_size < vma->vm_end - vma->vm_start)
return -EINVAL;
- if (obj->funcs && obj->funcs->vm_ops)
- vma->vm_ops = obj->funcs->vm_ops;
- else if (dev->driver->gem_vm_ops)
- vma->vm_ops = dev->driver->gem_vm_ops;
- else
- return -EINVAL;
-
- vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP;
vma->vm_private_data = obj;
- vma->vm_page_prot =
pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
- vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
+
+ if (obj->funcs && obj->funcs->mmap)
+ obj->funcs->mmap(obj, vma);
+ else {
+ if (obj->funcs && obj->funcs->vm_ops)
+ vma->vm_ops = obj->funcs->vm_ops;
+ else if (dev->driver->gem_vm_ops)
+ vma->vm_ops = dev->driver->gem_vm_ops;
+ else
+ return -EINVAL;
+
+ vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND |
VM_DONTDUMP;
+ vma->vm_page_prot =
pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
+ vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
+ }
/* Take a ref for this mapping of the object, so that the fault
* handler can dereference the mmap offset's pointer to the object.
Since I remember quite a few discussions where the default vma flag
wrangling we're doing is seriously getting in the way of things too.
I think even better would be if this new ->mmap hook could also be used
directly by the dma-buf mmap code, without having to jump through hoops
creating a fake file and fake vma offset and everything. I think with that
we'd have a really solid case to add this ->mmap hook.
-Daniel
> +
> /**
> * @vm_ops:
> *
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index afc38cece3f5..84db8de217e1 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -1105,6 +1105,8 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj,
> unsigned long obj_size,
> vma->vm_ops = obj->funcs->vm_ops;
> else if (dev->driver->gem_vm_ops)
> vma->vm_ops = dev->driver->gem_vm_ops;
> + else if (obj->funcs && obj->funcs->mmap)
> + /* obj->funcs->mmap must set vma->vm_ops */;
> else
> return -EINVAL;
>
> @@ -1192,6 +1194,10 @@ int drm_gem_mmap(struct file *filp, struct
> vm_area_struct *vma)
> ret = drm_gem_mmap_obj(obj, drm_vma_node_size(node) << PAGE_SHIFT,
> vma);
>
> + if (ret == 0)
> + if (obj->funcs->mmap)
> + ret = obj->funcs->mmap(obj, vma);
> +
> drm_gem_object_put_unlocked(obj);
>
> return ret;
> --
> 2.18.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
