All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] drm/i915: Verify workarounds immediately after application
@ 2019-04-15 16:00 Chris Wilson
  2019-04-15 16:00 ` [PATCH 2/4] drm/i915: Verify the engine workarounds stick on application Chris Wilson
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Chris Wilson @ 2019-04-15 16:00 UTC (permalink / raw)
  To: intel-gfx

Immediately after writing the workaround, verify that it stuck in the
register.

References: https://bugs.freedesktop.org/show_bug.cgi?id=108954
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_workarounds.c | 32 +++++++++++++-----------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
index ccaf63679435..1c54b5030807 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/intel_workarounds.c
@@ -913,6 +913,20 @@ wal_get_fw_for_rmw(struct intel_uncore *uncore, const struct i915_wa_list *wal)
 	return fw;
 }
 
+static bool
+wa_verify(const struct i915_wa *wa, u32 cur, const char *name, const char *from)
+{
+	if ((cur ^ wa->val) & wa->mask) {
+		DRM_ERROR("%s workaround lost on %s! (%x=%x/%x, expected %x, mask=%x)\n",
+			  name, from, i915_mmio_reg_offset(wa->reg), cur,
+			  cur & wa->mask, wa->val, wa->mask);
+
+		return false;
+	}
+
+	return true;
+}
+
 static void
 wa_list_apply(struct intel_uncore *uncore, const struct i915_wa_list *wal)
 {
@@ -931,6 +945,10 @@ wa_list_apply(struct intel_uncore *uncore, const struct i915_wa_list *wal)
 
 	for (i = 0, wa = wal->list; i < wal->count; i++, wa++) {
 		intel_uncore_rmw_fw(uncore, wa->reg, wa->mask, wa->val);
+		if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM))
+			wa_verify(wa,
+				  intel_uncore_read_fw(uncore, wa->reg),
+				  wal->name, "applictation");
 	}
 
 	intel_uncore_forcewake_put__locked(uncore, fw);
@@ -942,20 +960,6 @@ void intel_gt_apply_workarounds(struct drm_i915_private *i915)
 	wa_list_apply(&i915->uncore, &i915->gt_wa_list);
 }
 
-static bool
-wa_verify(const struct i915_wa *wa, u32 cur, const char *name, const char *from)
-{
-	if ((cur ^ wa->val) & wa->mask) {
-		DRM_ERROR("%s workaround lost on %s! (%x=%x/%x, expected %x, mask=%x)\n",
-			  name, from, i915_mmio_reg_offset(wa->reg), cur,
-			  cur & wa->mask, wa->val, wa->mask);
-
-		return false;
-	}
-
-	return true;
-}
-
 static bool wa_list_verify(struct intel_uncore *uncore,
 			   const struct i915_wa_list *wal,
 			   const char *from)
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH v2 4/4] drm/i915/selftests: Verify whitelist of context registers
@ 2019-04-16  8:10 Chris Wilson
  2019-04-16  9:12 ` [PATCH] " Chris Wilson
  0 siblings, 1 reply; 13+ messages in thread
From: Chris Wilson @ 2019-04-16  8:10 UTC (permalink / raw)
  To: intel-gfx

The RING_NONPRIV allows us to add registers to a whitelist that allows
userspace to modify them. Ideally such registers should be safe and
saved within the context such that they do not impact system behaviour
for other users. This selftest verifies that those registers we do add
are (a) then writable by userspace and (b) only affect a single client.

Opens:
- Is GEN9_SLICE_COMMON_ECO_CHICKEN1 really write-only?

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 .../drm/i915/selftests/intel_workarounds.c    | 320 ++++++++++++++++++
 1 file changed, 320 insertions(+)

diff --git a/drivers/gpu/drm/i915/selftests/intel_workarounds.c b/drivers/gpu/drm/i915/selftests/intel_workarounds.c
index a363748a7a4f..56799f3538f9 100644
--- a/drivers/gpu/drm/i915/selftests/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/selftests/intel_workarounds.c
@@ -700,6 +700,325 @@ static int live_reset_whitelist(void *arg)
 	return err;
 }
 
+static int read_whitelisted_registers(struct i915_gem_context *ctx,
+				      struct intel_engine_cs *engine,
+				      struct i915_vma *results)
+{
+	intel_wakeref_t wakeref;
+	struct i915_request *rq;
+	u32 srm, *cs;
+	int err, i;
+
+	rq = ERR_PTR(-ENODEV);
+	with_intel_runtime_pm(engine->i915, wakeref)
+		rq = i915_request_alloc(engine, ctx);
+	if (IS_ERR(rq))
+		return PTR_ERR(rq);
+
+	err = i915_vma_move_to_active(results, rq, EXEC_OBJECT_WRITE);
+	if (err)
+		goto err_req;
+
+	srm = MI_STORE_REGISTER_MEM;
+	if (INTEL_GEN(ctx->i915) >= 8)
+		srm++;
+
+	cs = intel_ring_begin(rq, 4 * engine->whitelist.count);
+	if (IS_ERR(cs)) {
+		err = PTR_ERR(cs);
+		goto err_req;
+	}
+
+	for (i = 0; i < engine->whitelist.count; i++) {
+		u64 offset = results->node.start + sizeof(u32) * i;
+
+		*cs++ = srm;
+		*cs++ = i915_mmio_reg_offset(engine->whitelist.list[i].reg);
+		*cs++ = lower_32_bits(offset);
+		*cs++ = upper_32_bits(offset);
+	}
+	intel_ring_advance(rq, cs);
+
+err_req:
+	i915_request_add(rq);
+
+	if (i915_request_wait(rq, I915_WAIT_LOCKED, HZ / 5) < 0)
+		err = -EIO;
+
+	return err;
+}
+
+static int scrub_whitelisted_registers(struct i915_gem_context *ctx,
+				       struct intel_engine_cs *engine)
+{
+	intel_wakeref_t wakeref;
+	struct i915_request *rq;
+	struct i915_vma *batch;
+	int i, err;
+	u32 *cs;
+
+	batch = create_batch(ctx);
+	if (IS_ERR(batch))
+		return PTR_ERR(batch);
+
+	cs = i915_gem_object_pin_map(batch->obj, I915_MAP_WC);
+	if (IS_ERR(cs)) {
+		err = PTR_ERR(cs);
+		goto err_batch;
+	}
+
+	*cs++ = MI_LOAD_REGISTER_IMM(engine->whitelist.count);
+	for (i = 0; i < engine->whitelist.count; i++) {
+		*cs++ = i915_mmio_reg_offset(engine->whitelist.list[i].reg);
+		*cs++ = 0xffffffff;
+	}
+	*cs++ = MI_BATCH_BUFFER_END;
+
+	i915_gem_object_flush_map(batch->obj);
+	i915_gem_chipset_flush(ctx->i915);
+
+	rq = ERR_PTR(-ENODEV);
+	with_intel_runtime_pm(engine->i915, wakeref)
+		rq = i915_request_alloc(engine, ctx);
+	if (IS_ERR(rq))
+		goto err_unpin;
+
+	if (engine->emit_init_breadcrumb) { /* Be nice if we hang */
+		err = engine->emit_init_breadcrumb(rq);
+		if (err)
+			goto err_request;
+	}
+
+	err = engine->emit_bb_start(rq, batch->node.start, 0, 0);
+
+err_request:
+	i915_request_add(rq);
+	if (i915_request_wait(rq, I915_WAIT_LOCKED, HZ / 5) < 0)
+		err = -EIO;
+
+err_unpin:
+	i915_gem_object_unpin_map(batch->obj);
+err_batch:
+	i915_vma_unpin_and_release(&batch, 0);
+	return err;
+}
+
+static bool pardon_reg(struct drm_i915_private *i915, i915_reg_t reg)
+{
+	/* Alas, we must pardon some whitelists */
+	static const struct {
+		i915_reg_t reg;
+		unsigned long gen_mask;
+	} pardon[] = {
+		{ GEN9_CTX_PREEMPT_REG, INTEL_GEN_MASK(9, 9) },
+		{ GEN8_L3SQCREG4, INTEL_GEN_MASK(9, 9) },
+	};
+	u32 offset = i915_mmio_reg_offset(reg);
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(pardon); i++) {
+		if (INTEL_INFO(i915)->gen_mask & pardon[i].gen_mask &&
+		    i915_mmio_reg_offset(pardon[i].reg) == offset)
+			return true;
+	}
+
+	return false;
+}
+
+static int eq_whitelisted_registers(struct i915_vma *A,
+				    struct i915_vma *B,
+				    struct intel_engine_cs *engine)
+{
+	u32 *a, *b;
+	int i, err;
+
+	a = i915_gem_object_pin_map(A->obj, I915_MAP_WB);
+	if (IS_ERR(a))
+		return PTR_ERR(a);
+
+	b = i915_gem_object_pin_map(B->obj, I915_MAP_WB);
+	if (IS_ERR(b)) {
+		err = PTR_ERR(b);
+		goto err_a;
+	}
+
+	err = 0;
+	for (i = 0; i < engine->whitelist.count; i++) {
+		if (a[i] != b[i] &&
+		    !pardon_reg(engine->i915, engine->whitelist.list[i].reg)) {
+			pr_err("[%d] Whitelisted register 0x%4x not context saved: A=%08x, B=%08x\n",
+			       i, i915_mmio_reg_offset(engine->whitelist.list[i].reg),
+			       a[i], b[i]);
+			err = -EINVAL;
+		}
+	}
+
+	i915_gem_object_unpin_map(B->obj);
+err_a:
+	i915_gem_object_unpin_map(A->obj);
+	return err;
+}
+
+static bool writeonly_reg(struct drm_i915_private *i915, i915_reg_t reg)
+{
+	static const struct {
+		i915_reg_t reg;
+		unsigned long gen_mask;
+	} wo[] = {
+		{ GEN9_SLICE_COMMON_ECO_CHICKEN1, INTEL_GEN_MASK(9, 9) },
+	};
+	u32 offset = i915_mmio_reg_offset(reg);
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(wo); i++) {
+		if (INTEL_INFO(i915)->gen_mask & wo[i].gen_mask &&
+		    i915_mmio_reg_offset(wo[i].reg) == offset)
+			return true;
+	}
+
+	return false;
+}
+
+static int neq_whitelisted_registers(struct i915_vma *A,
+				     struct i915_vma *B,
+				     struct intel_engine_cs *engine)
+{
+	u32 *a, *b;
+	int i, err;
+
+	a = i915_gem_object_pin_map(A->obj, I915_MAP_WB);
+	if (IS_ERR(a))
+		return PTR_ERR(a);
+
+	b = i915_gem_object_pin_map(B->obj, I915_MAP_WB);
+	if (IS_ERR(b)) {
+		err = PTR_ERR(b);
+		goto err_a;
+	}
+
+	err = 0;
+	for (i = 0; i < engine->whitelist.count; i++) {
+		if (a[i] == b[i] &&
+		    !writeonly_reg(engine->i915, engine->whitelist.list[i])) {
+			pr_err("[%d] Whitelist register 0x%4x:%08x was unwritable\n",
+			       i, i915_mmio_reg_offset(engine->whitelist.list[i].reg), a[i]);
+			err = -EINVAL;
+		}
+	}
+
+	i915_gem_object_unpin_map(B->obj);
+err_a:
+	i915_gem_object_unpin_map(A->obj);
+	return err;
+}
+
+static int live_isolated_whitelist(void *arg)
+{
+	struct drm_i915_private *i915 = arg;
+	struct {
+		struct i915_gem_context *ctx;
+		struct i915_vma *scratch[2];
+	} client[2] = {};
+	struct intel_engine_cs *engine;
+	enum intel_engine_id id;
+	int i, err = 0;
+
+	/*
+	 * Check that a write into a whitelist register works, but
+	 * invisible to a second context.
+	 */
+
+	if (!intel_engines_has_context_isolation(i915))
+		return 0;
+
+	if (!i915->kernel_context->ppgtt)
+		return 0;
+
+	for (i = 0; i < ARRAY_SIZE(client); i++) {
+		struct i915_gem_context *c;
+
+		c = kernel_context(i915);
+		if (IS_ERR(c)) {
+			err = PTR_ERR(c);
+			goto err;
+		}
+
+		client[i].scratch[0] = create_scratch(&c->ppgtt->vm, 1024);
+		if (IS_ERR(client[i].scratch[0])) {
+			err = PTR_ERR(client[i].scratch[0]);
+			kernel_context_close(c);
+			goto err;
+		}
+
+		client[i].scratch[1] = create_scratch(&c->ppgtt->vm, 1024);
+		if (IS_ERR(client[i].scratch[1])) {
+			err = PTR_ERR(client[i].scratch[1]);
+			i915_vma_unpin_and_release(&client[i].scratch[0], 0);
+			kernel_context_close(c);
+			goto err;
+		}
+
+		client[i].ctx = c;
+	}
+
+	for_each_engine(engine, i915, id) {
+		if (!engine->whitelist.count)
+			continue;
+
+		/* Read default values */
+		err = read_whitelisted_registers(client[0].ctx, engine,
+						 client[0].scratch[0]);
+		if (err)
+			goto err;
+
+		/* Try to overwrite registers (should only affect ctx0) */
+		err = scrub_whitelisted_registers(client[0].ctx, engine);
+		if (err)
+			goto err;
+
+		/* Read values from ctx1, we expect these to be defaults */
+		err = read_whitelisted_registers(client[1].ctx, engine,
+						 client[1].scratch[0]);
+		if (err)
+			goto err;
+
+		/* Verify that both reads return the same default values */
+		err = eq_whitelisted_registers(client[0].scratch[0],
+					       client[1].scratch[0],
+					       engine);
+		if (err)
+			goto err;
+
+		/* Read back the updated values in ctx0 */
+		err = read_whitelisted_registers(client[0].ctx, engine,
+						 client[0].scratch[1]);
+		if (err)
+			goto err;
+
+		/* User should be granted privilege to overwhite regs */
+		err = neq_whitelisted_registers(client[0].scratch[0],
+						client[0].scratch[1],
+						engine);
+		if (err)
+			goto err;
+	}
+
+err:
+	for (i = 0; i < ARRAY_SIZE(client); i++) {
+		if (!client[i].ctx)
+			break;
+
+		i915_vma_unpin_and_release(&client[i].scratch[1], 0);
+		i915_vma_unpin_and_release(&client[i].scratch[0], 0);
+		kernel_context_close(client[i].ctx);
+	}
+
+	if (igt_flush_test(i915, I915_WAIT_LOCKED))
+		err = -EIO;
+
+	return err;
+}
+
 static bool verify_gt_engine_wa(struct drm_i915_private *i915,
 				struct wa_lists *lists, const char *str)
 {
@@ -844,6 +1163,7 @@ int intel_workarounds_live_selftests(struct drm_i915_private *i915)
 	static const struct i915_subtest tests[] = {
 		SUBTEST(live_dirty_whitelist),
 		SUBTEST(live_reset_whitelist),
+		SUBTEST(live_isolated_whitelist),
 		SUBTEST(live_gpu_reset_gt_engine_workarounds),
 		SUBTEST(live_engine_reset_gt_engine_workarounds),
 	};
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-04-24 11:10 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-15 16:00 [PATCH 1/4] drm/i915: Verify workarounds immediately after application Chris Wilson
2019-04-15 16:00 ` [PATCH 2/4] drm/i915: Verify the engine workarounds stick on application Chris Wilson
2019-04-15 16:00 ` [PATCH 3/4] drm/i915: Make workaround verification *optional* Chris Wilson
2019-04-15 16:00 ` [PATCH 4/4] drm/i915/selftests: Verify whitelist of context registers Chris Wilson
2019-04-15 17:41   ` [PATCH] " Chris Wilson
2019-04-16 14:49     ` [Intel-gfx] " Dan Carpenter
2019-04-15 16:59 ` ✗ Fi.CI.BAT: failure for series starting with [1/4] drm/i915: Verify workarounds immediately after application Patchwork
2019-04-15 18:33 ` ✗ Fi.CI.BAT: failure for series starting with [1/4] drm/i915: Verify workarounds immediately after application (rev2) Patchwork
2019-04-16  8:10 [PATCH v2 4/4] drm/i915/selftests: Verify whitelist of context registers Chris Wilson
2019-04-16  9:12 ` [PATCH] " Chris Wilson
2019-04-16 14:50   ` Tvrtko Ursulin
2019-04-24 10:00     ` Chris Wilson
2019-04-24 11:03       ` Chris Wilson
2019-04-24 11:10         ` Tvrtko Ursulin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.