| Message ID | 20190927134616.21899-2-yuq825@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/lima: simplify driver by using more drm helpers | expand |
On 27/09/2019 14:46, 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. Nit: It would be good to mention in the commit message that this adds drm_gem_objects_lookup_user() as it helps grepping commits. Also if you're rewriting it - it would be good to point out that you are _changing_ the behvaviour of drm_gem_objects_lookpu() to not use user space handles. > > 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@kernel.org> > Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com> > Cc: Steven Price <steven.price@arm.com> > Cc: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com> > Suggested-by: Rob Herring <robh@kernel.org> > Signed-off-by: Qiang Yu <yuq825@gmail.com> Reviewed-by: Steven Price <steven.price@arm.com> > --- > 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) > { > 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); >
On Fri, Sep 27, 2019 at 9:57 PM Steven Price <steven.price@arm.com> wrote: > > On 27/09/2019 14:46, 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. > > Nit: It would be good to mention in the commit message that this adds > drm_gem_objects_lookup_user() as it helps grepping commits. Also if > you're rewriting it - it would be good to point out that you are > _changing_ the behvaviour of drm_gem_objects_lookpu() to not use user > space handles. > OK, I can improve the comments of two patches in v3. Thanks, Qiang > > > > 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@kernel.org> > > Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com> > > Cc: Steven Price <steven.price@arm.com> > > Cc: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com> > > Suggested-by: Rob Herring <robh@kernel.org> > > Signed-off-by: Qiang Yu <yuq825@gmail.com> > > Reviewed-by: Steven Price <steven.price@arm.com> > > > --- > > 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) > > { > > 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); > > >
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@kernel.org> > Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com> > Cc: Steven Price <steven.price@arm.com> > Cc: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com> > Suggested-by: Rob Herring <robh@kernel.org> > Signed-off-by: Qiang Yu <yuq825@gmail.com> > --- > 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 >
On Wed, Oct 9, 2019 at 10:07 AM Daniel Vetter <daniel@ffwll.ch> wrote: > > 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@kernel.org> > > Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com> > > Cc: Steven Price <steven.price@arm.com> > > Cc: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com> > > Suggested-by: Rob Herring <robh@kernel.org> > > Signed-off-by: Qiang Yu <yuq825@gmail.com> > > --- > > 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. The only driver currently is panfrost and it is updated in this patch. Rob
On Wed, Oct 09, 2019 at 11:05:31AM -0500, Rob Herring wrote: > On Wed, Oct 9, 2019 at 10:07 AM Daniel Vetter <daniel@ffwll.ch> wrote: > > > > 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@kernel.org> > > > Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com> > > > Cc: Steven Price <steven.price@arm.com> > > > Cc: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com> > > > Suggested-by: Rob Herring <robh@kernel.org> > > > Signed-off-by: Qiang Yu <yuq825@gmail.com> > > > --- > > > 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. > > The only driver currently is panfrost and it is updated in this patch. Oops sry missed that in the diffstat somehow. -Daniel
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) { 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);
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@kernel.org> Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com> Cc: Steven Price <steven.price@arm.com> Cc: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com> Suggested-by: Rob Herring <robh@kernel.org> Signed-off-by: Qiang Yu <yuq825@gmail.com> --- 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(-)