drm/i915: Nuke struct_mutex from context_setparam

Userspace should be free to race against itself and shoot itself in
the foot if it so desires to adjust a parameter at the same time as
submitting a batch to that context. As such, the struct_mutex in context
setparam is only being used to serialise userspace against itself and
not for any protection of internal structs and so is superfluous.

v2: Separate user_flags from internal flags to reduce chance of
interference; and use locked bit ops for user updates.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180911132206.23032-1-chris@chris-wilson.co.uk

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 747b8170a15a0249d5bc25c965ccf840786910d8..f772593b99ab484563de0e1cad543e95254e0553 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -862,7 +862,7 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
                ret = -EINVAL;
                break;
        case I915_CONTEXT_PARAM_NO_ZEROMAP:
-               args->value = ctx->flags & CONTEXT_NO_ZEROMAP;
+               args->value = test_bit(UCONTEXT_NO_ZEROMAP, &ctx->user_flags);
                break;
        case I915_CONTEXT_PARAM_GTT_SIZE:
                if (ctx->ppgtt)
@@ -896,27 +896,23 @@ int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
        struct drm_i915_file_private *file_priv = file->driver_priv;
        struct drm_i915_gem_context_param *args = data;
        struct i915_gem_context *ctx;
-       int ret;
+       int ret = 0;
 
        ctx = i915_gem_context_lookup(file_priv, args->ctx_id);
        if (!ctx)
                return -ENOENT;
 
-       ret = i915_mutex_lock_interruptible(dev);
-       if (ret)
-               goto out;
-
        switch (args->param) {
        case I915_CONTEXT_PARAM_BAN_PERIOD:
                ret = -EINVAL;
                break;
        case I915_CONTEXT_PARAM_NO_ZEROMAP:
-               if (args->size) {
+               if (args->size)
                        ret = -EINVAL;
-               } else {
-                       ctx->flags &= ~CONTEXT_NO_ZEROMAP;
-                       ctx->flags |= args->value ? CONTEXT_NO_ZEROMAP : 0;
-               }
+               else if (args->value)
+                       set_bit(UCONTEXT_NO_ZEROMAP, &ctx->user_flags);
+               else
+                       clear_bit(UCONTEXT_NO_ZEROMAP, &ctx->user_flags);
                break;
        case I915_CONTEXT_PARAM_NO_ERROR_CAPTURE:
                if (args->size)
@@ -960,9 +956,7 @@ int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
                ret = -EINVAL;
                break;
        }
-       mutex_unlock(&dev->struct_mutex);
 
-out:
        i915_gem_context_put(ctx);
        return ret;
 }

diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h
index e09673ca731d37c185c6d655d95d5df372827df1..08165f6a0a842482cd1eec32f5bd131e39e44500 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/i915_gem_context.h
@@ -116,16 +116,21 @@ struct i915_gem_context {
         */
        struct rcu_head rcu;
 
+       /**
+        * @user_flags: small set of booleans controlled by the user
+        */
+       unsigned long user_flags;
+#define UCONTEXT_NO_ZEROMAP            0
+#define UCONTEXT_NO_ERROR_CAPTURE      1
+#define UCONTEXT_BANNABLE              2
+
        /**
         * @flags: small set of booleans
         */
        unsigned long flags;
-#define CONTEXT_NO_ZEROMAP             BIT(0)
-#define CONTEXT_NO_ERROR_CAPTURE       1
-#define CONTEXT_CLOSED                 2
-#define CONTEXT_BANNABLE               3
-#define CONTEXT_BANNED                 4
-#define CONTEXT_FORCE_SINGLE_SUBMISSION        5
+#define CONTEXT_BANNED                 0
+#define CONTEXT_CLOSED                 1
+#define CONTEXT_FORCE_SINGLE_SUBMISSION        2
 
        /**
         * @hw_id: - unique identifier for the context
@@ -209,37 +214,37 @@ static inline bool i915_gem_context_is_closed(const struct i915_gem_context *ctx
 static inline void i915_gem_context_set_closed(struct i915_gem_context *ctx)
 {
        GEM_BUG_ON(i915_gem_context_is_closed(ctx));
-       __set_bit(CONTEXT_CLOSED, &ctx->flags);
+       set_bit(CONTEXT_CLOSED, &ctx->flags);
 }
 
 static inline bool i915_gem_context_no_error_capture(const struct i915_gem_context *ctx)
 {
-       return test_bit(CONTEXT_NO_ERROR_CAPTURE, &ctx->flags);
+       return test_bit(UCONTEXT_NO_ERROR_CAPTURE, &ctx->user_flags);
 }
 
 static inline void i915_gem_context_set_no_error_capture(struct i915_gem_context *ctx)
 {
-       __set_bit(CONTEXT_NO_ERROR_CAPTURE, &ctx->flags);
+       set_bit(UCONTEXT_NO_ERROR_CAPTURE, &ctx->user_flags);
 }
 
 static inline void i915_gem_context_clear_no_error_capture(struct i915_gem_context *ctx)
 {
-       __clear_bit(CONTEXT_NO_ERROR_CAPTURE, &ctx->flags);
+       clear_bit(UCONTEXT_NO_ERROR_CAPTURE, &ctx->user_flags);
 }
 
 static inline bool i915_gem_context_is_bannable(const struct i915_gem_context *ctx)
 {
-       return test_bit(CONTEXT_BANNABLE, &ctx->flags);
+       return test_bit(UCONTEXT_BANNABLE, &ctx->user_flags);
 }
 
 static inline void i915_gem_context_set_bannable(struct i915_gem_context *ctx)
 {
-       __set_bit(CONTEXT_BANNABLE, &ctx->flags);
+       set_bit(UCONTEXT_BANNABLE, &ctx->user_flags);
 }
 
 static inline void i915_gem_context_clear_bannable(struct i915_gem_context *ctx)
 {
-       __clear_bit(CONTEXT_BANNABLE, &ctx->flags);
+       clear_bit(UCONTEXT_BANNABLE, &ctx->user_flags);
 }
 
 static inline bool i915_gem_context_is_banned(const struct i915_gem_context *ctx)
@@ -249,7 +254,7 @@ static inline bool i915_gem_context_is_banned(const struct i915_gem_context *ctx
 
 static inline void i915_gem_context_set_banned(struct i915_gem_context *ctx)
 {
-       __set_bit(CONTEXT_BANNED, &ctx->flags);
+       set_bit(CONTEXT_BANNED, &ctx->flags);
 }
 
 static inline bool i915_gem_context_force_single_submission(const struct i915_gem_context *ctx)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 7d0b3a2c30e2e86bff42b2fe8492e245964e429c..094c3b1cf2027041bf940ee47eec40cc0e63fe76 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -743,7 +743,7 @@ static int eb_select_context(struct i915_execbuffer *eb)
        }
 
        eb->context_flags = 0;
-       if (ctx->flags & CONTEXT_NO_ZEROMAP)
+       if (test_bit(UCONTEXT_NO_ZEROMAP, &ctx->user_flags))
                eb->context_flags |= __EXEC_OBJECT_NEEDS_BIAS;
 
        return 0;