[PATCH igt] igt/gem_ctx_isolation: Check isolation of registers between contexts

* [PATCH igt] igt/gem_ctx_isolation: Check isolation of registers between contexts
@ 2017-10-24 11:07 Chris Wilson
  2017-10-24 11:16 ` Chris Wilson
                   ` (10 more replies)
  0 siblings, 11 replies; 13+ messages in thread
From: Chris Wilson @ 2017-10-24 11:07 UTC (permalink / raw)
  To: intel-gfx

A new context assumes that all of its registers are in the default state
when it is created. What may happen is that a register written by one
context may leak into the second, causing mass confusion.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/Makefile.sources    |   1 +
 tests/gem_ctx_isolation.c | 351 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 352 insertions(+)
 create mode 100644 tests/gem_ctx_isolation.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index ac9f90bc..d18b7461 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -57,6 +57,7 @@ TESTS_progs = \
 	gem_ctx_basic \
 	gem_ctx_create \
 	gem_ctx_exec \
+	gem_ctx_isolation \
 	gem_ctx_param \
 	gem_ctx_switch \
 	gem_ctx_thrash \
diff --git a/tests/gem_ctx_isolation.c b/tests/gem_ctx_isolation.c
new file mode 100644
index 00000000..1569f5a8
--- /dev/null
+++ b/tests/gem_ctx_isolation.c
@@ -0,0 +1,351 @@
+/*
+ * Copyright © 2017 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include "igt.h"
+#include "igt_dummyload.h"
+
+#define MAX_REG 0x40000
+#define NUM_REGS (MAX_REG / sizeof(uint32_t))
+
+#define PAGE_ALIGN(x) ALIGN(x, 4096)
+
+#define DIRTY 0x1
+#define UNSAFE 0x2
+
+enum {
+	RCS_MASK = 0x1,
+	BCS_MASK = 0x2,
+	VCS_MASK = 0x4,
+	VECS_MASK = 0x8,
+};
+
+#define ALL ~0u
+#define GEN_RANGE(x, y) ((ALL >> (32 - (y - x + 1))) << x)
+
+static const struct named_register {
+	const char *name;
+	unsigned int gen_mask;
+	unsigned int engine_mask;
+	uint32_t offset;
+} safe_registers[] = {
+	/* Keep in ascending offset order */
+	{ "CTX_PREEMPT", GEN_RANGE(9, 10), RCS_MASK, 0x2248 },
+	{ "CS_CHICKEN1", GEN_RANGE(9, 10), RCS_MASK, 0x2580 },
+	{ "HDC_CHICKEN1", GEN_RANGE(9, 10), RCS_MASK, 0x7304 },
+	{ "L3SQREG1", GEN_RANGE(8, 10), RCS_MASK, 0xb010 },
+	{}
+}, ignore_registers[] = {
+	{ "RCS timestamp", ALL, RCS_MASK, 0x2358 },
+	{ "VCS0 timestamp", ALL, VCS_MASK, 0x12358 },
+	{ "VCS1 timestamp", ALL, VCS_MASK, 0x1c358 },
+	{ "BCS timestamp", ALL, BCS_MASK, 0x22358 },
+	{ "VECS timestamp", ALL, VECS_MASK, 0x1a358 },
+	{}
+};
+
+static const char *register_name(uint32_t offset)
+{
+	/* XXX bsearch? */
+	for (const struct named_register *r = safe_registers; r->name; r++) {
+		if (r->offset == offset)
+			return r->name;
+	}
+
+	return "unknown";
+}
+
+static bool ignore_register(uint32_t offset)
+{
+	for (const struct named_register *r = ignore_registers; r->name; r++) {
+		if (r->offset == offset)
+			return true;
+	}
+
+	return false;
+}
+
+static uint32_t read_all_regs(int fd,
+			      uint32_t ctx, unsigned int engine,
+			      unsigned int flags)
+{
+	struct drm_i915_gem_exec_object2 obj[2];
+	struct drm_i915_gem_relocation_entry *reloc;
+	struct drm_i915_gem_execbuffer2 execbuf;
+	unsigned int regs_size, batch_size;
+	unsigned int engine_bit, gen_bit;
+	uint32_t *batch, *b;
+
+	switch (engine & 0x63) {
+	case I915_EXEC_DEFAULT:
+	case I915_EXEC_RENDER:
+		engine_bit = RCS_MASK;
+		break;
+	case I915_EXEC_BLT:
+		engine_bit = BCS_MASK;
+		break;
+	case I915_EXEC_BSD:
+		engine_bit = VCS_MASK;
+		break;
+	case I915_EXEC_VEBOX:
+		engine_bit = VECS_MASK;
+		break;
+	default:
+		igt_assert(0);
+	}
+	gen_bit = 1 << intel_gen(intel_get_drm_devid(fd));
+
+	reloc = calloc(NUM_REGS, sizeof(*reloc));
+	igt_assert(reloc);
+
+	regs_size = NUM_REGS * sizeof(uint32_t);
+	regs_size = PAGE_ALIGN(regs_size);
+
+	batch_size = NUM_REGS * 4 * sizeof(uint32_t) + 4;
+	batch_size = PAGE_ALIGN(batch_size);
+
+	memset(obj, 0, sizeof(obj));
+	obj[0].handle = gem_create(fd, regs_size);
+	obj[1].handle = gem_create(fd, batch_size);
+	obj[1].relocs_ptr = to_user_pointer(reloc);
+
+	b = batch = gem_mmap__cpu(fd, obj[1].handle, 0, batch_size, PROT_WRITE);
+	if (flags & UNSAFE) {
+		for (unsigned int n = 0; n < NUM_REGS; n++) {
+			*b++ = 0x24 << 23 | 2;
+			*b++ = n * sizeof(uint32_t);
+			reloc[n].target_handle = obj[0].handle;
+			reloc[n].presumed_offset = 0;
+			reloc[n].offset = (b - batch) * sizeof(*b);
+			reloc[n].delta = sizeof(uint32_t) * n;
+			reloc[n].read_domains = I915_GEM_DOMAIN_RENDER;
+			reloc[n].write_domain = I915_GEM_DOMAIN_RENDER;
+			*b++ = reloc[n].delta;
+			*b++ = 0;
+		}
+		obj[1].relocation_count = NUM_REGS;
+	} else {
+		unsigned int n = 0;
+
+		for (const struct named_register *r = safe_registers;
+		     r->name; r++) {
+			if (!(r->engine_mask & engine_bit))
+				continue;
+			if (!(r->gen_mask & gen_bit))
+				continue;
+
+			*b++ = 0x24 << 23 | 2; /* SRM */
+			*b++ = r->offset;
+			reloc[n].target_handle = obj[0].handle;
+			reloc[n].presumed_offset = 0;
+			reloc[n].offset = (b - batch) * sizeof(*b);
+			reloc[n].delta = r->offset;
+			reloc[n].read_domains = I915_GEM_DOMAIN_RENDER;
+			reloc[n].write_domain = I915_GEM_DOMAIN_RENDER;
+			*b++ = reloc[n].delta;
+			*b++ = 0;
+
+			n++;
+		}
+
+		obj[1].relocation_count = n;
+	}
+	*b++ = MI_BATCH_BUFFER_END;
+	munmap(batch, batch_size);
+
+	memset(&execbuf, 0, sizeof(execbuf));
+	execbuf.buffers_ptr = to_user_pointer(obj);
+	execbuf.buffer_count = 2;
+	execbuf.flags = engine;
+	gem_execbuf(fd, &execbuf);
+	gem_close(fd, obj[1].handle);
+	free(reloc);
+
+	return obj[0].handle;
+}
+
+static void write_all_regs(int fd,
+			   uint32_t ctx, unsigned int engine,
+			   unsigned int flags)
+{
+	struct drm_i915_gem_exec_object2 obj;
+	struct drm_i915_gem_execbuffer2 execbuf;
+	unsigned int engine_bit, gen_bit;
+	unsigned int batch_size;
+	uint32_t *batch, *b;
+
+	switch (engine & 0x63) {
+	case I915_EXEC_DEFAULT:
+	case I915_EXEC_RENDER:
+		engine_bit = RCS_MASK;
+		break;
+	case I915_EXEC_BLT:
+		engine_bit = BCS_MASK;
+		break;
+	case I915_EXEC_BSD:
+		engine_bit = VCS_MASK;
+		break;
+	case I915_EXEC_VEBOX:
+		engine_bit = VECS_MASK;
+		break;
+	default:
+		igt_assert(0);
+	}
+	gen_bit = 1 << intel_gen(intel_get_drm_devid(fd));
+
+	batch_size = NUM_REGS * 3 * sizeof(uint32_t) + 4;
+	batch_size = PAGE_ALIGN(batch_size);
+
+	memset(&obj, 0, sizeof(obj));
+	obj.handle = gem_create(fd, batch_size);
+	b = batch = gem_mmap__cpu(fd, obj.handle, 0, batch_size, PROT_WRITE);
+	if (flags & UNSAFE) {
+		for (unsigned int n = 0; n < NUM_REGS; n++) {
+			*b++ = 0x23 << 23 | 2; /* LRI */
+			*b++ = n * sizeof(uint32_t);
+			*b++ = 0xdeadbeef;
+		}
+	} else {
+		for (const struct named_register *r = safe_registers;
+		     r->name; r++) {
+			if (!(r->engine_mask & engine_bit))
+				continue;
+			if (!(r->gen_mask & gen_bit))
+				continue;
+			*b++ = 0x23 << 23 | 2; /* LRI */
+			*b++ = r->offset;
+			*b++ = 0xdeadbeef;
+		}
+	}
+	*b++ = MI_BATCH_BUFFER_END;
+	munmap(batch, batch_size);
+
+	memset(&execbuf, 0, sizeof(execbuf));
+	execbuf.buffers_ptr = to_user_pointer(&obj);
+	execbuf.buffer_count = 1;
+	execbuf.flags = engine;
+	gem_execbuf(fd, &execbuf);
+	gem_close(fd, obj.handle);
+}
+
+static void compare_regs(int fd, uint32_t regs[2])
+{
+	unsigned int num_errors;
+	unsigned int regs_size;
+	uint32_t *a, *b;
+
+	regs_size = NUM_REGS * sizeof(uint32_t);
+	regs_size = PAGE_ALIGN(regs_size);
+
+	a = gem_mmap__cpu(fd, regs[0], 0, regs_size, PROT_READ);
+	gem_set_domain(fd, regs[0], I915_GEM_DOMAIN_CPU, 0);
+
+	b = gem_mmap__cpu(fd, regs[1], 0, regs_size, PROT_READ);
+	gem_set_domain(fd, regs[1], I915_GEM_DOMAIN_CPU, 0);
+
+	num_errors = 0;
+	for (unsigned int n = 0; n < NUM_REGS; n++) {
+		if (a[n] != b[n] && !ignore_register(n*sizeof(uint32_t))) {
+			igt_warn("Register 0x%04x [%s]: A=%08x B=%08x\n",
+				 n, register_name(n*sizeof(uint32_t)),
+				 a[n], b[n]);
+			num_errors++;
+		}
+	}
+	munmap(b, regs_size);
+	munmap(a, regs_size);
+
+	igt_assert_f(num_errors == 0,
+		     "%d registers mistached between two virgin contexts\n",
+		     num_errors);
+}
+
+static void isolation(int fd, unsigned int engine, unsigned int flags)
+{
+	igt_spin_t *spin = NULL;
+	uint32_t ctx[2];
+	uint32_t regs[2];
+
+	ctx[0] = gem_context_create(fd);
+	regs[0] = read_all_regs(fd, ctx[0], engine, flags);
+
+	if (flags & DIRTY) {
+		spin = igt_spin_batch_new(fd, ctx[0], engine, 0);
+		write_all_regs(fd, ctx[0], engine, flags);
+	}
+
+	/*
+	 * We create and execute a new context, whilst the HW is occupied
+	 * with the previous context (we should switch from the old to the
+	 * new proto context without idling, which could then load the
+	 * powercontext). If all goes well, we only see the default values
+	 * from this context, but if goes badly we see the corruption from
+	 * the previous context instead!
+	 */
+	ctx[1] = gem_context_create(fd);
+	regs[1] = read_all_regs(fd, ctx[1], engine, flags);
+
+	igt_spin_batch_free(fd, spin);
+
+	compare_regs(fd, regs);
+
+	for (int n = 0; n < ARRAY_SIZE(ctx); n++) {
+		gem_close(fd, regs[n]);
+		gem_context_destroy(fd, ctx[n]);
+	}
+}
+
+igt_main
+{
+	const unsigned int platform_validation = 0;
+	int fd = -1;
+
+	igt_fixture {
+		fd = drm_open_driver(DRIVER_INTEL);
+		igt_require_gem(fd);
+
+		igt_require(gem_has_execlists(fd));
+
+		/* check that we can create contexts. */
+		gem_context_destroy(fd, gem_context_create(fd));
+	}
+
+	for (const struct intel_execution_engine *e =
+			intel_execution_engines; e->name; e++) {
+		igt_subtest_group {
+			unsigned int engine = e->exec_id | e->flags;
+			igt_fixture {
+				gem_require_ring(fd, engine);
+			}
+
+			igt_subtest_f("%s-clean", e->name)
+				isolation(fd, engine, 0);
+			igt_subtest_f("%s-dirty", e->name)
+				isolation(fd, engine, DIRTY);
+
+			igt_subtest_f("%s-unsafe", e->name) {
+				igt_require(platform_validation);
+				isolation(fd, engine, DIRTY | UNSAFE);
+			}
+		}
+	}
+}
-- 
2.15.0.rc1

_______________________________________________
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