On Fri, Sep 27, 2019 at 09:46:11PM +0800, Qiang Yu wrote:
> drm_gem_objects_lookup does not use user space bo handles
> directly and left the user to kernel copy work to a new
> interface drm_gem_objects_lookup_user.
>
> This is for driver like lima which does not pass gem bo
> handles continously in an array in ioctl argument.
>
> v2:
> 1. add drm_gem_objects_lookup_user
> 2. remove none-zero check as all caller will check before
> calling this function
>
> Cc: Rob Herring <robh@xxxxxxxxxx>
> Cc: Tomeu Vizoso <tomeu.vizoso@xxxxxxxxxxxxx>
> Cc: Steven Price <steven.price@xxxxxxx>
> Cc: Alyssa Rosenzweig <alyssa.rosenzweig@xxxxxxxxxxxxx>
> Suggested-by: Rob Herring <robh@xxxxxxxxxx>
> Signed-off-by: Qiang Yu <yuq825@xxxxxxxxx>
> ---
> drivers/gpu/drm/drm_gem.c | 57 +++++++++++++++++++------
> drivers/gpu/drm/panfrost/panfrost_drv.c | 6 +--
> include/drm/drm_gem.h | 4 +-
> 3 files changed, 49 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 6854f5867d51..a5e88c3e6d25 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -679,11 +679,11 @@ static int objects_lookup(struct drm_file *filp, u32 *handle, int count,
> /**
> * drm_gem_objects_lookup - look up GEM objects from an array of handles
> * @filp: DRM file private date
> - * @bo_handles: user pointer to array of userspace handle
> + * @bo_handles: array of GEM object handles
> * @count: size of handle array
> * @objs_out: returned pointer to array of drm_gem_object pointers
> *
> - * Takes an array of userspace handles and returns a newly allocated array of
> + * Takes an array of GEM object handles and returns a newly allocated array of
> * GEM objects.
> *
> * For a single handle lookup, use drm_gem_object_lookup().
> @@ -695,26 +695,56 @@ static int objects_lookup(struct drm_file *filp, u32 *handle, int count,
> * failure. 0 is returned on success.
> *
> */
> -int drm_gem_objects_lookup(struct drm_file *filp, void __user *bo_handles,
> +int drm_gem_objects_lookup(struct drm_file *filp, u32 *bo_handles,
> int count, struct drm_gem_object ***objs_out)
You can't do this change without updating all the drivers. Simpler to keep
this one as-is, and create a new function with an _internal suffix.
-Daniel
> {
> int ret;
> - u32 *handles;
> struct drm_gem_object **objs;
>
> - if (!count)
> - return 0;
> -
> objs = kvmalloc_array(count, sizeof(struct drm_gem_object *),
> GFP_KERNEL | __GFP_ZERO);
> if (!objs)
> return -ENOMEM;
>
> + ret = objects_lookup(filp, bo_handles, count, objs);
> + if (ret)
> + kvfree(objs);
> + else
> + *objs_out = objs;
> +
> + return ret;
> +
> +}
> +EXPORT_SYMBOL(drm_gem_objects_lookup);
> +
> +/**
> + * drm_gem_objects_lookup_user - look up GEM objects from an array of handles
> + * @filp: DRM file private date
> + * @bo_handles: user pointer to array of userspace handle
> + * @count: size of handle array
> + * @objs_out: returned pointer to array of drm_gem_object pointers
> + *
> + * Takes an array of userspace handles and returns a newly allocated array of
> + * GEM objects.
> + *
> + * For a single handle lookup, use drm_gem_object_lookup().
> + *
> + * Returns:
> + *
> + * @objs filled in with GEM object pointers. Returned GEM objects need to be
> + * released with drm_gem_object_put(). -ENOENT is returned on a lookup
> + * failure. 0 is returned on success.
> + *
> + */
> +int drm_gem_objects_lookup_user(struct drm_file *filp, void __user *bo_handles,
> + int count, struct drm_gem_object ***objs_out)
> +{
> + int ret;
> + u32 *handles;
> +
> handles = kvmalloc_array(count, sizeof(u32), GFP_KERNEL);
> - if (!handles) {
> - ret = -ENOMEM;
> - goto out;
> - }
> + if (!handles)
> + return -ENOMEM;
>
> if (copy_from_user(handles, bo_handles, count * sizeof(u32))) {
> ret = -EFAULT;
> @@ -722,15 +752,14 @@ int drm_gem_objects_lookup(struct drm_file *filp, void __user *bo_handles,
> goto out;
> }
>
> - ret = objects_lookup(filp, handles, count, objs);
> - *objs_out = objs;
> + ret = drm_gem_objects_lookup(filp, handles, count, objs_out);
>
> out:
> kvfree(handles);
> return ret;
>
> }
> -EXPORT_SYMBOL(drm_gem_objects_lookup);
> +EXPORT_SYMBOL(drm_gem_objects_lookup_user);
>
> /**
> * drm_gem_object_lookup - look up a GEM object from its handle
> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
> index d74442d71048..486ca51d5662 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> @@ -130,9 +130,9 @@ panfrost_lookup_bos(struct drm_device *dev,
> if (!job->implicit_fences)
> return -ENOMEM;
>
> - return drm_gem_objects_lookup(file_priv,
> - (void __user *)(uintptr_t)args->bo_handles,
> - job->bo_count, &job->bos);
> + return drm_gem_objects_lookup_user(file_priv,
> + (void __user *)(uintptr_t)args->bo_handles,
> + job->bo_count, &job->bos);
> }
>
> /**
> diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> index 6aaba14f5972..354fc8d240e4 100644
> --- a/include/drm/drm_gem.h
> +++ b/include/drm/drm_gem.h
> @@ -387,8 +387,10 @@ struct page **drm_gem_get_pages(struct drm_gem_object *obj);
> void drm_gem_put_pages(struct drm_gem_object *obj, struct page **pages,
> bool dirty, bool accessed);
>
> -int drm_gem_objects_lookup(struct drm_file *filp, void __user *bo_handles,
> +int drm_gem_objects_lookup(struct drm_file *filp, u32 *bo_handles,
> int count, struct drm_gem_object ***objs_out);
> +int drm_gem_objects_lookup_user(struct drm_file *filp, void __user *bo_handles,
> + int count, struct drm_gem_object ***objs_out);
> struct drm_gem_object *drm_gem_object_lookup(struct drm_file *filp, u32 handle);
> long drm_gem_dma_resv_wait(struct drm_file *filep, u32 handle,
> bool wait_all, unsigned long timeout);
> --
> 2.17.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
