Skip to content

Commit

Permalink
drm/msm: Split submit_lookup_objects() into two loops
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Kristian H. Kristensen authored and Rob Clark committed Apr 19, 2019
1 parent 8ea274a commit b673499
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 28 deletions.
5 changes: 4 additions & 1 deletion drivers/gpu/drm/msm/msm_gem.h
Original file line number Diff line number Diff line change
Expand Up @@ -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];
};
Expand Down
44 changes: 17 additions & 27 deletions drivers/gpu/drm/msm/msm_gem_submit.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)));

Expand All @@ -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: */
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -155,7 +146,6 @@ static int submit_lookup_objects(struct msm_gem_submit *submit,
}

out_unlock:
pagefault_enable();
spin_unlock(&file->table_lock);

out:
Expand Down

0 comments on commit b673499

Please sign in to comment.