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 <hoegsberg@chromium.org>
Signed-off-by: Rob Clark <robdclark@chromium.org>

diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h
index 0617c44d99b02dc73a4f6d270a3acfbe7f00ac4b..c5ac781dffeea366402979acebe099fff24528f6 100644 (file)
--- 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 12b983fc0b567601fa796840b65f7135209b933b..648e0c1b92e6df73ff7886eef3591d047f44a014 100644 (file)
--- 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: