Commit d3f3e5e4 authored by Chris Wilson's avatar Chris Wilson
Browse files

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: default avatarChris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180911132206.23032-1-chris@chris-wilson.co.uk
parent 146cdf3f

drivers/gpu/drm/i915/i915_gem_context.c

+7 −13
Original line number Diff line number Diff line
@@ -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;

}

drivers/gpu/drm/i915/i915_gem_context.h

+19 −14
Original line number Diff line number Diff line
@@ -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)

drivers/gpu/drm/i915/i915_gem_execbuffer.c

+1 −1
Original line number Diff line number Diff line
@@ -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;

CRA Git | Maintained and supported by SUSTech CRA and CCSE