From b673499ae738e7790b1b9f65e784999e5804abd3 Mon Sep 17 00:00:00 2001 From: "Kristian H. Kristensen" Date: Wed, 20 Mar 2019 10:09:10 -0700 Subject: [PATCH] drm/msm: Split submit_lookup_objects() into two loops First loop does copy_from_user() without the table lock held and just stores the handle. Second loop looks up buffer objects with the table_lock held without potentially blocking or faulting. This lets us clean up a bunch of custom, non-faulting copy_from_user() code. Signed-off-by: Kristian H. Kristensen Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem.h | 5 +++- drivers/gpu/drm/msm/msm_gem_submit.c | 44 +++++++++++----------------- 2 files changed, 21 insertions(+), 28 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h index 0617c44d99b0..c5ac781dffee 100644 --- a/drivers/gpu/drm/msm/msm_gem.h +++ b/drivers/gpu/drm/msm/msm_gem.h @@ -166,7 +166,10 @@ struct msm_gem_submit { } *cmd; /* array of size nr_cmds */ struct { uint32_t flags; - struct msm_gem_object *obj; + union { + struct msm_gem_object *obj; + uint32_t handle; + }; uint64_t iova; } bos[0]; }; diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c index 12b983fc0b56..648e0c1b92e6 100644 --- a/drivers/gpu/drm/msm/msm_gem_submit.c +++ b/drivers/gpu/drm/msm/msm_gem_submit.c @@ -74,27 +74,14 @@ void msm_gem_submit_free(struct msm_gem_submit *submit) kfree(submit); } -static inline unsigned long __must_check -copy_from_user_inatomic(void *to, const void __user *from, unsigned long n) -{ - if (access_ok(from, n)) - return __copy_from_user_inatomic(to, from, n); - return -EFAULT; -} - static int submit_lookup_objects(struct msm_gem_submit *submit, struct drm_msm_gem_submit *args, struct drm_file *file) { unsigned i; int ret = 0; - spin_lock(&file->table_lock); - pagefault_disable(); - for (i = 0; i < args->nr_bos; i++) { struct drm_msm_gem_submit_bo submit_bo; - struct drm_gem_object *obj; - struct msm_gem_object *msm_obj; void __user *userptr = u64_to_user_ptr(args->bos + (i * sizeof(submit_bo))); @@ -103,15 +90,10 @@ static int submit_lookup_objects(struct msm_gem_submit *submit, */ submit->bos[i].flags = 0; - if (copy_from_user_inatomic(&submit_bo, userptr, sizeof(submit_bo))) { - pagefault_enable(); - spin_unlock(&file->table_lock); - if (copy_from_user(&submit_bo, userptr, sizeof(submit_bo))) { - ret = -EFAULT; - goto out; - } - spin_lock(&file->table_lock); - pagefault_disable(); + if (copy_from_user(&submit_bo, userptr, sizeof(submit_bo))) { + ret = -EFAULT; + i = 0; + goto out; } /* at least one of READ and/or WRITE flags should be set: */ @@ -121,19 +103,28 @@ static int submit_lookup_objects(struct msm_gem_submit *submit, !(submit_bo.flags & MANDATORY_FLAGS)) { DRM_ERROR("invalid flags: %x\n", submit_bo.flags); ret = -EINVAL; - goto out_unlock; + i = 0; + goto out; } + submit->bos[i].handle = submit_bo.handle; submit->bos[i].flags = submit_bo.flags; /* in validate_objects() we figure out if this is true: */ submit->bos[i].iova = submit_bo.presumed; + } + + spin_lock(&file->table_lock); + + for (i = 0; i < args->nr_bos; i++) { + struct drm_gem_object *obj; + struct msm_gem_object *msm_obj; /* normally use drm_gem_object_lookup(), but for bulk lookup * all under single table_lock just hit object_idr directly: */ - obj = idr_find(&file->object_idr, submit_bo.handle); + obj = idr_find(&file->object_idr, submit->bos[i].handle); if (!obj) { - DRM_ERROR("invalid handle %u at index %u\n", submit_bo.handle, i); + DRM_ERROR("invalid handle %u at index %u\n", submit->bos[i].handle, i); ret = -EINVAL; goto out_unlock; } @@ -142,7 +133,7 @@ static int submit_lookup_objects(struct msm_gem_submit *submit, if (!list_empty(&msm_obj->submit_entry)) { DRM_ERROR("handle %u at index %u already on submit list\n", - submit_bo.handle, i); + submit->bos[i].handle, i); ret = -EINVAL; goto out_unlock; } @@ -155,7 +146,6 @@ static int submit_lookup_objects(struct msm_gem_submit *submit, } out_unlock: - pagefault_enable(); spin_unlock(&file->table_lock); out: