Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752027AbcDTTDH (ORCPT ); Wed, 20 Apr 2016 15:03:07 -0400 Received: from bhuna.collabora.co.uk ([46.235.227.227]:35764 "EHLO bhuna.collabora.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751219AbcDTTDD (ORCPT ); Wed, 20 Apr 2016 15:03:03 -0400 Date: Wed, 20 Apr 2016 16:02:54 -0300 From: Gustavo Padovan To: Maarten Lankhorst Cc: Gustavo Padovan , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, devel@driverdev.osuosl.org, dri-devel@lists.freedesktop.org, Daniel Stone , Arve =?iso-8859-1?B?SGr4bm5lduVn?= , Riley Andrews , Daniel Vetter , Rob Clark , Greg Hackmann , John Harrison , Joe Perches , Andrew Morton , David Airlie , Daniel Vetter Subject: Re: [PATCH v10 RESEND 2/3] kernel.h: add u64_to_user_ptr() Message-ID: <20160420190254.GB2398@joana> References: <1461098522-1508-1-git-send-email-gustavo@padovan.org> <1461098522-1508-2-git-send-email-gustavo@padovan.org> <57172AB7.9080705@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <57172AB7.9080705@linux.intel.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 10820 Lines: 270 2016-04-20 Maarten Lankhorst : > Op 19-04-16 om 22:42 schreef Gustavo Padovan: > > From: Gustavo Padovan > > > > This function had copies in 3 different files. Unify them in kernel.h. > > > > Cc: Joe Perches > > Cc: Andrew Morton > > Cc: David Airlie > > Cc: Daniel Vetter > > Cc: Rob Clark > > Signed-off-by: Gustavo Padovan > > --- > > drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 11 +++-------- > > drivers/gpu/drm/i915/i915_drv.h | 5 ----- > > drivers/gpu/drm/i915/i915_gem.c | 14 +++++++------- > > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 14 +++++++------- > > drivers/gpu/drm/msm/msm_gem_submit.c | 11 +++-------- > > include/linux/kernel.h | 5 +++++ > > 6 files changed, 25 insertions(+), 35 deletions(-) > > > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c > > index 236ada9..afdd55d 100644 > > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c > > @@ -28,11 +28,6 @@ > > #define BO_LOCKED 0x4000 > > #define BO_PINNED 0x2000 > > > > -static inline void __user *to_user_ptr(u64 address) > > -{ > > - return (void __user *)(uintptr_t)address; > > -} > > - > > static struct etnaviv_gem_submit *submit_create(struct drm_device *dev, > > struct etnaviv_gpu *gpu, size_t nr) > > { > > @@ -347,21 +342,21 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, > > cmdbuf->exec_state = args->exec_state; > > cmdbuf->ctx = file->driver_priv; > > > > - ret = copy_from_user(bos, to_user_ptr(args->bos), > > + ret = copy_from_user(bos, u64_to_user_ptr(args->bos), > > args->nr_bos * sizeof(*bos)); > > if (ret) { > > ret = -EFAULT; > > goto err_submit_cmds; > > } > > > > - ret = copy_from_user(relocs, to_user_ptr(args->relocs), > > + ret = copy_from_user(relocs, u64_to_user_ptr(args->relocs), > > args->nr_relocs * sizeof(*relocs)); > > if (ret) { > > ret = -EFAULT; > > goto err_submit_cmds; > > } > > > > - ret = copy_from_user(stream, to_user_ptr(args->stream), > > + ret = copy_from_user(stream, u64_to_user_ptr(args->stream), > > args->stream_size); > > if (ret) { > > ret = -EFAULT; > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > index 1048093..bb624cc 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -3576,11 +3576,6 @@ static inline i915_reg_t i915_vgacntrl_reg(struct drm_device *dev) > > return VGACNTRL; > > } > > > > -static inline void __user *to_user_ptr(u64 address) > > -{ > > - return (void __user *)(uintptr_t)address; > > -} > > - > > static inline unsigned long msecs_to_jiffies_timeout(const unsigned int m) > > { > > unsigned long j = msecs_to_jiffies(m); > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > > index dabc089..2889716 100644 > > --- a/drivers/gpu/drm/i915/i915_gem.c > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > @@ -324,7 +324,7 @@ i915_gem_phys_pwrite(struct drm_i915_gem_object *obj, > > { > > struct drm_device *dev = obj->base.dev; > > void *vaddr = obj->phys_handle->vaddr + args->offset; > > - char __user *user_data = to_user_ptr(args->data_ptr); > > + char __user *user_data = u64_to_user_ptr(args->data_ptr); > > int ret = 0; > > > > /* We manually control the domain here and pretend that it > > @@ -605,7 +605,7 @@ i915_gem_shmem_pread(struct drm_device *dev, > > int needs_clflush = 0; > > struct sg_page_iter sg_iter; > > > > - user_data = to_user_ptr(args->data_ptr); > > + user_data = u64_to_user_ptr(args->data_ptr); > > remain = args->size; > > > > obj_do_bit17_swizzling = i915_gem_object_needs_bit17_swizzle(obj); > > @@ -692,7 +692,7 @@ i915_gem_pread_ioctl(struct drm_device *dev, void *data, > > return 0; > > > > if (!access_ok(VERIFY_WRITE, > > - to_user_ptr(args->data_ptr), > > + u64_to_user_ptr(args->data_ptr), > > args->size)) > > return -EFAULT; > > > > @@ -783,7 +783,7 @@ i915_gem_gtt_pwrite_fast(struct drm_device *dev, > > if (ret) > > goto out_unpin; > > > > - user_data = to_user_ptr(args->data_ptr); > > + user_data = u64_to_user_ptr(args->data_ptr); > > remain = args->size; > > > > offset = i915_gem_obj_ggtt_offset(obj) + args->offset; > > @@ -907,7 +907,7 @@ i915_gem_shmem_pwrite(struct drm_device *dev, > > int needs_clflush_before = 0; > > struct sg_page_iter sg_iter; > > > > - user_data = to_user_ptr(args->data_ptr); > > + user_data = u64_to_user_ptr(args->data_ptr); > > remain = args->size; > > > > obj_do_bit17_swizzling = i915_gem_object_needs_bit17_swizzle(obj); > > @@ -1036,12 +1036,12 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data, > > return 0; > > > > if (!access_ok(VERIFY_READ, > > - to_user_ptr(args->data_ptr), > > + u64_to_user_ptr(args->data_ptr), > > args->size)) > > return -EFAULT; > > > > if (likely(!i915.prefault_disable)) { > > - ret = fault_in_multipages_readable(to_user_ptr(args->data_ptr), > > + ret = fault_in_multipages_readable(u64_to_user_ptr(args->data_ptr), > > args->size); > > if (ret) > > return -EFAULT; > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > index 1328bc5..e60b4e7 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > @@ -514,7 +514,7 @@ i915_gem_execbuffer_relocate_vma(struct i915_vma *vma, > > struct drm_i915_gem_exec_object2 *entry = vma->exec_entry; > > int remain, ret; > > > > - user_relocs = to_user_ptr(entry->relocs_ptr); > > + user_relocs = u64_to_user_ptr(entry->relocs_ptr); > > > > remain = entry->relocation_count; > > while (remain) { > > @@ -865,7 +865,7 @@ i915_gem_execbuffer_relocate_slow(struct drm_device *dev, > > u64 invalid_offset = (u64)-1; > > int j; > > > > - user_relocs = to_user_ptr(exec[i].relocs_ptr); > > + user_relocs = u64_to_user_ptr(exec[i].relocs_ptr); > > > > if (copy_from_user(reloc+total, user_relocs, > > exec[i].relocation_count * sizeof(*reloc))) { > > @@ -1009,7 +1009,7 @@ validate_exec_list(struct drm_device *dev, > > invalid_flags |= EXEC_OBJECT_NEEDS_GTT; > > > > for (i = 0; i < count; i++) { > > - char __user *ptr = to_user_ptr(exec[i].relocs_ptr); > > + char __user *ptr = u64_to_user_ptr(exec[i].relocs_ptr); > > int length; /* limited by fault_in_pages_readable() */ > > > > if (exec[i].flags & invalid_flags) > > @@ -1696,7 +1696,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, > > return -ENOMEM; > > } > > ret = copy_from_user(exec_list, > > - to_user_ptr(args->buffers_ptr), > > + u64_to_user_ptr(args->buffers_ptr), > > sizeof(*exec_list) * args->buffer_count); > > if (ret != 0) { > > DRM_DEBUG("copy %d exec entries failed %d\n", > > @@ -1732,7 +1732,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, > > ret = i915_gem_do_execbuffer(dev, data, file, &exec2, exec2_list); > > if (!ret) { > > struct drm_i915_gem_exec_object __user *user_exec_list = > > - to_user_ptr(args->buffers_ptr); > > + u64_to_user_ptr(args->buffers_ptr); > > > > /* Copy the new buffer offsets back to the user's exec list. */ > > for (i = 0; i < args->buffer_count; i++) { > > @@ -1786,7 +1786,7 @@ i915_gem_execbuffer2(struct drm_device *dev, void *data, > > return -ENOMEM; > > } > > ret = copy_from_user(exec2_list, > > - to_user_ptr(args->buffers_ptr), > > + u64_to_user_ptr(args->buffers_ptr), > > sizeof(*exec2_list) * args->buffer_count); > > if (ret != 0) { > > DRM_DEBUG("copy %d exec entries failed %d\n", > > @@ -1799,7 +1799,7 @@ i915_gem_execbuffer2(struct drm_device *dev, void *data, > > if (!ret) { > > /* Copy the new buffer offsets back to the user's exec list. */ > > struct drm_i915_gem_exec_object2 __user *user_exec_list = > > - to_user_ptr(args->buffers_ptr); > > + u64_to_user_ptr(args->buffers_ptr); > > int i; > > > > for (i = 0; i < args->buffer_count; i++) { > > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c > > index 43d2181..23d2528 100644 > > --- a/drivers/gpu/drm/msm/msm_gem_submit.c > > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c > > @@ -28,11 +28,6 @@ > > #define BO_LOCKED 0x4000 > > #define BO_PINNED 0x2000 > > > > -static inline void __user *to_user_ptr(u64 address) > > -{ > > - return (void __user *)(uintptr_t)address; > > -} > > - > > static struct msm_gem_submit *submit_create(struct drm_device *dev, > > struct msm_gpu *gpu, int nr) > > { > > @@ -68,7 +63,7 @@ static int submit_lookup_objects(struct msm_gem_submit *submit, > > struct drm_gem_object *obj; > > struct msm_gem_object *msm_obj; > > void __user *userptr = > > - to_user_ptr(args->bos + (i * sizeof(submit_bo))); > > + u64_to_user_ptr(args->bos + (i * sizeof(submit_bo))); > > > > ret = copy_from_user(&submit_bo, userptr, sizeof(submit_bo)); > > if (ret) { > > @@ -257,7 +252,7 @@ static int submit_reloc(struct msm_gem_submit *submit, struct msm_gem_object *ob > > for (i = 0; i < nr_relocs; i++) { > > struct drm_msm_gem_submit_reloc submit_reloc; > > void __user *userptr = > > - to_user_ptr(relocs + (i * sizeof(submit_reloc))); > > + u64_to_user_ptr(relocs + (i * sizeof(submit_reloc))); > > uint32_t iova, off; > > bool valid; > > > > @@ -356,7 +351,7 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data, > > for (i = 0; i < args->nr_cmds; i++) { > > struct drm_msm_gem_submit_cmd submit_cmd; > > void __user *userptr = > > - to_user_ptr(args->cmds + (i * sizeof(submit_cmd))); > > + u64_to_user_ptr(args->cmds + (i * sizeof(submit_cmd))); > > struct msm_gem_object *msm_obj; > > uint32_t iova; > > > > diff --git a/include/linux/kernel.h b/include/linux/kernel.h > > index 2f7775e..0c843ec 100644 > > --- a/include/linux/kernel.h > > +++ b/include/linux/kernel.h > > @@ -53,6 +53,11 @@ > > > > #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr)) > > > > +static inline void __user *u64_to_user_ptr(u64 address) > > +{ > > + return (void __user *)(uintptr_t)address; > > +} > This could cast any integer to a user ptr, maybe add some preprocessor magic to only allow u64? Right, that makes sense. I'll add typecheck() and send an updated v11 version. Gustavo