All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH igt] igt/gem_ctx_isolation: Check isolation of registers between contexts
@ 2017-12-01 11:13 Chris Wilson
  2017-12-01 12:22 ` ✓ Fi.CI.BAT: success for igt/gem_ctx_isolation: Check isolation of registers between contexts (rev9) Patchwork
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Chris Wilson @ 2017-12-01 11:13 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.

v2: Extend back to Sandybridge (etc)
v3: Check context preserves registers across suspend/hibernate and resets.
v4: Complete the remapping onto the new class:instance
v5: Not like that, like this, try again to use class:instance
v6: Prepare for retrospective gen4 contexts!
v7: Repaint register set name to nonpriv, as this is what bspec calls the
registers that are writable by userspace.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 tests/Makefile.sources    |   1 +
 tests/gem_ctx_isolation.c | 677 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/gem_exec_fence.c    |   2 +-
 3 files changed, 679 insertions(+), 1 deletion(-)
 create mode 100644 tests/gem_ctx_isolation.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 34ca71a0..e16e586a 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -57,6 +57,7 @@ TESTS_progs = \
 	gem_ctx_bad_exec \
 	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..8b69cd33
--- /dev/null
+++ b/tests/gem_ctx_isolation.c
@@ -0,0 +1,677 @@
+/*
+ * 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 DIRTY1 0x1
+#define DIRTY2 0x2
+#define RESET 0x4
+
+#define BIT(x) (1ul << (x))
+#define ENGINE(x, y) BIT(4*(x) + (y))
+
+enum {
+	RCS0 = ENGINE(I915_ENGINE_CLASS_RENDER, 0),
+	BCS0 = ENGINE(I915_ENGINE_CLASS_COPY, 0),
+	VCS0 = ENGINE(I915_ENGINE_CLASS_VIDEO, 0),
+	VCS1 = ENGINE(I915_ENGINE_CLASS_VIDEO, 1),
+	VECS0 = ENGINE(I915_ENGINE_CLASS_VIDEO_ENHANCE, 0),
+};
+
+#define ALL ~0u
+#define GEN_RANGE(x, y) ((ALL >> (32 - (y - x + 1))) << x)
+#define GEN4 (ALL << 4)
+#define GEN5 (ALL << 5)
+#define GEN6 (ALL << 6)
+#define GEN7 (ALL << 7)
+#define GEN8 (ALL << 8)
+#define GEN9 (ALL << 9)
+
+#define NOCTX 0
+
+#define LAST_KNOWN_GEN 10
+
+static const struct named_register {
+	const char *name;
+	unsigned int gen_mask;
+	unsigned int engine_mask;
+	uint32_t offset;
+	uint32_t count;
+} nonpriv_registers[] = {
+	{ "NOPID", NOCTX, RCS0, 0x2094 },
+	{ "MI_PREDICATE_RESULT_2", NOCTX, RCS0, 0x23bc },
+	{ "INSTPM", GEN9, RCS0, 0x20c0 },
+	{ "IA_VERTICES_COUNT", GEN4, RCS0, 0x2310, 2 },
+	{ "IA_PRIMITIVES_COUNT", GEN4, RCS0, 0x2318, 2 },
+	{ "VS_INVOCATION_COUNT", GEN4, RCS0, 0x2320, 2 },
+	{ "HS_INVOCATION_COUNT", GEN4, RCS0, 0x2300, 2 },
+	{ "DS_INVOCATION_COUNT", GEN4, RCS0, 0x2308, 2 },
+	{ "GS_INVOCATION_COUNT", GEN4, RCS0, 0x2328, 2 },
+	{ "GS_PRIMITIVES_COUNT", GEN4, RCS0, 0x2330, 2 },
+	{ "CL_INVOCATION_COUNT", GEN4, RCS0, 0x2338, 2 },
+	{ "CL_PRIMITIVES_COUNT", GEN4, RCS0, 0x2340, 2 },
+	{ "PS_INVOCATION_COUNT_0", GEN4, RCS0, 0x22c8, 2 },
+	{ "PS_DEPTH_COUNT_0", GEN4, RCS0, 0x22d8, 2 },
+	{ "GPUGPU_DISPATCHDIMX", GEN8, RCS0, 0x2500 },
+	{ "GPUGPU_DISPATCHDIMY", GEN8, RCS0, 0x2504 },
+	{ "GPUGPU_DISPATCHDIMZ", GEN8, RCS0, 0x2508 },
+	{ "MI_PREDICATE_SRC0", GEN8, RCS0, 0x2400, 2 },
+	{ "MI_PREDICATE_SRC1", GEN8, RCS0, 0x2408, 2 },
+	{ "MI_PREDICATE_DATA", GEN8, RCS0, 0x2410, 2 },
+	{ "MI_PRED_RESULT", GEN8, RCS0, 0x2418 },
+	{ "3DPRIM_END_OFFSET", GEN6, RCS0, 0x2420 },
+	{ "3DPRIM_START_VERTEX", GEN6, RCS0, 0x2430 },
+	{ "3DPRIM_VERTEX_COUNT", GEN6, RCS0, 0x2434 },
+	{ "3DPRIM_INSTANCE_COUNT", GEN6, RCS0, 0x2438 },
+	{ "3DPRIM_START_INSTANCE", GEN6, RCS0, 0x243c },
+	{ "3DPRIM_BASE_VERTEX", GEN6, RCS0, 0x2440 },
+	{ "GPGPU_THREADS_DISPATCHED", GEN8, RCS0, 0x2290, 2 },
+	{ "PS_INVOCATION_COUNT_1", GEN8, RCS0, 0x22f0, 2 },
+	{ "PS_DEPTH_COUNT_1", GEN8, RCS0, 0x22f8, 2 },
+	{ "BB_OFFSET", GEN8, RCS0, 0x2158 },
+	{ "MI_PREDICATE_RESULT_1", GEN8, RCS0, 0x241c },
+	{ "CS_GPR", GEN8, RCS0, 0x2600, 32 },
+	{ "OA_CTX_CONTROL", GEN8, RCS0, 0x2360 },
+	{ "OACTXID", GEN8, RCS0, 0x2364 },
+	{ "PS_INVOCATION_COUNT_2", GEN8, RCS0, 0x2448, 2 },
+	{ "PS_DEPTH_COUNT_2", GEN8, RCS0, 0x2450, 2 },
+	{ "Cache_Mode_0", GEN7, RCS0, 0x7000 },
+	{ "Cache_Mode_1", GEN7, RCS0, 0x7004 },
+	{ "GT_MODE", GEN8, RCS0, 0x7008 },
+	{ "L3_Config", GEN7, RCS0, 0x7034 },
+	{ "TD_CTL", GEN8, RCS0, 0xe400 },
+	{ "TD_CTL2", GEN8, RCS0, 0xe404 },
+	{ "SO_NUM_PRIMS_WRITEN0", GEN6, RCS0, 0x5200, 2 },
+	{ "SO_NUM_PRIMS_WRITEN1", GEN6, RCS0, 0x5208, 2 },
+	{ "SO_NUM_PRIMS_WRITEN2", GEN6, RCS0, 0x5210, 2 },
+	{ "SO_NUM_PRIMS_WRITEN3", GEN6, RCS0, 0x5218, 2 },
+	{ "SO_PRIM_STORAGE_NEEDED0", GEN6, RCS0, 0x5240, 2 },
+	{ "SO_PRIM_STORAGE_NEEDED1", GEN6, RCS0, 0x5248, 2 },
+	{ "SO_PRIM_STORAGE_NEEDED2", GEN6, RCS0, 0x5250, 2 },
+	{ "SO_PRIM_STORAGE_NEEDED3", GEN6, RCS0, 0x5258, 2 },
+	{ "SO_WRITE_OFFSET0", GEN7, RCS0, 0x5280 },
+	{ "SO_WRITE_OFFSET1", GEN7, RCS0, 0x5284 },
+	{ "SO_WRITE_OFFSET2", GEN7, RCS0, 0x5288 },
+	{ "SO_WRITE_OFFSET3", GEN7, RCS0, 0x528c },
+	{ "OA_CONTROL", NOCTX, RCS0, 0x2b00 },
+	{ "PERF_CNT_1", NOCTX, RCS0, 0x91b8, 2 },
+	{ "PERF_CNT_2", NOCTX, RCS0, 0x91c0, 2 },
+
+	/* Privileged (enabled by w/a + FORCE_TO_NONPRIV) */
+	{ "CTX_PREEMPT", NOCTX /* GEN_RANGE(9, 10) */, RCS0, 0x2248 },
+	{ "CS_CHICKEN1", GEN_RANGE(9, 10), RCS0, 0x2580 },
+	{ "HDC_CHICKEN1", GEN_RANGE(9, 10), RCS0, 0x7304 },
+	{ "L3SQREG1", GEN8, RCS0, 0xb010 },
+
+	{ "BCS_GPR", GEN9, BCS0, 0x22600, 32 },
+	{ "BCS_SWCTRL", GEN8, BCS0, 0x22200 },
+
+	{ "VCS0_GPR", GEN9, VCS0, 0x12600, 32 },
+	{ "MFC_VDBOX1", NOCTX, VCS0, 0x12800, 64 },
+
+	{ "VCS1_GPR", GEN9, VCS1, 0x1c600, 32 },
+	{ "MFC_VDBOX2", NOCTX, VCS1, 0x1c800, 64 },
+
+	{ "VECS_GPR", GEN9, VECS0, 0x1a600, 32 },
+
+	{}
+}, ignore_registers[] = {
+	{ "RCS timestamp", GEN6, ~0u, 0x2358 },
+	{ "VCS0 timestamp", GEN7, ~0u, 0x12358 },
+	{ "VCS1 timestamp", GEN7, ~0u, 0x1c358 },
+	{ "BCS timestamp", GEN7, ~0u, 0x22358 },
+	{ "VECS timestamp", GEN8, ~0u, 0x1a358 },
+	{}
+};
+
+static const char *register_name(uint32_t offset, char *buf, size_t len)
+{
+	for (const struct named_register *r = nonpriv_registers; r->name; r++) {
+		unsigned int width = r->count ? 4*r->count : 4;
+		if (offset >= r->offset && offset < r->offset + width) {
+			if (r->count <= 1)
+				return r->name;
+
+			snprintf(buf, len, "%s[%d]",
+				 r->name, (offset - r->offset)/4);
+			return buf;
+		}
+	}
+
+	return "unknown";
+}
+
+static bool ignore_register(uint32_t offset)
+{
+	for (const struct named_register *r = ignore_registers; r->name; r++) {
+		unsigned int width = r->count ? 4*r->count : 4;
+		if (offset >= r->offset && offset < r->offset + width)
+			return true;
+	}
+
+	return false;
+}
+
+static uint32_t read_regs(int fd,
+			  uint32_t ctx,
+			  const struct intel_execution_engine2 *e,
+			  unsigned int flags)
+{
+	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
+	const unsigned int gen_bit = 1 << gen;
+	const unsigned int engine_bit = ENGINE(e->class, e->instance);
+	const bool r64b = gen >= 8;
+	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, n;
+	uint32_t *batch, *b;
+
+	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);
+	gem_set_domain(fd, obj[1].handle,
+		       I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
+
+	n = 0;
+	for (const struct named_register *r = nonpriv_registers; r->name; r++) {
+		if (!(r->engine_mask & engine_bit))
+			continue;
+		if (!(r->gen_mask & gen_bit))
+			continue;
+
+		for (unsigned count = r->count ?: 1, offset = r->offset;
+		     count--; offset += 4) {
+			*b++ = 0x24 << 23 | (1 + r64b); /* SRM */
+			*b++ = offset;
+			reloc[n].target_handle = obj[0].handle;
+			reloc[n].presumed_offset = 0;
+			reloc[n].offset = (b - batch) * sizeof(*b);
+			reloc[n].delta = offset;
+			reloc[n].read_domains = I915_GEM_DOMAIN_RENDER;
+			reloc[n].write_domain = I915_GEM_DOMAIN_RENDER;
+			*b++ = offset;
+			if (r64b)
+				*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 =
+		gem_class_instance_to_eb_flags(fd, e->class, e->instance);
+	execbuf.rsvd1 = ctx;
+	gem_execbuf(fd, &execbuf);
+	gem_close(fd, obj[1].handle);
+	free(reloc);
+
+	return obj[0].handle;
+}
+
+static void write_regs(int fd,
+		       uint32_t ctx,
+		       const struct intel_execution_engine2 *e,
+		       unsigned int flags,
+		       uint32_t value)
+{
+	const unsigned int gen_bit = 1 << intel_gen(intel_get_drm_devid(fd));
+	const unsigned int engine_bit = ENGINE(e->class, e->instance);
+	struct drm_i915_gem_exec_object2 obj;
+	struct drm_i915_gem_execbuffer2 execbuf;
+	unsigned int batch_size;
+	uint32_t *batch, *b;
+
+	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);
+	gem_set_domain(fd, obj.handle,
+		       I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
+	for (const struct named_register *r = nonpriv_registers; r->name; r++) {
+		if (!(r->engine_mask & engine_bit))
+			continue;
+		if (!(r->gen_mask & gen_bit))
+			continue;
+		for (unsigned count = r->count ?: 1, offset = r->offset;
+		     count--; offset += 4) {
+			*b++ = 0x22 << 23 | 1; /* LRI */
+			*b++ = offset;
+			*b++ = value;
+		}
+	}
+	*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 =
+		gem_class_instance_to_eb_flags(fd, e->class, e->instance);
+	execbuf.rsvd1 = ctx;
+	gem_execbuf(fd, &execbuf);
+	gem_close(fd, obj.handle);
+}
+
+static void restore_regs(int fd,
+			 uint32_t ctx,
+			 const struct intel_execution_engine2 *e,
+			 unsigned int flags,
+			 uint32_t regs)
+{
+	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
+	const unsigned int gen_bit = 1 << gen;
+	const unsigned int engine_bit = ENGINE(e->class, e->instance);
+	const bool r64b = gen >= 8;
+	struct drm_i915_gem_exec_object2 obj[2];
+	struct drm_i915_gem_execbuffer2 execbuf;
+	struct drm_i915_gem_relocation_entry *reloc;
+	unsigned int batch_size, n;
+	uint32_t *batch, *b;
+
+	if (gen < 7) /* no LRM */
+		return;
+
+	reloc = calloc(NUM_REGS, sizeof(*reloc));
+	igt_assert(reloc);
+
+	batch_size = NUM_REGS * 3 * sizeof(uint32_t) + 4;
+	batch_size = PAGE_ALIGN(batch_size);
+
+	memset(obj, 0, sizeof(obj));
+	obj[0].handle = regs;
+	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);
+	gem_set_domain(fd, obj[1].handle,
+		       I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
+
+	n = 0;
+	for (const struct named_register *r = nonpriv_registers; r->name; r++) {
+		if (!(r->engine_mask & engine_bit))
+			continue;
+		if (!(r->gen_mask & gen_bit))
+			continue;
+
+		for (unsigned count = r->count ?: 1, offset = r->offset;
+		     count--; offset += 4) {
+			*b++ = 0x29 << 23 | (1 + r64b); /* LRM */
+			*b++ = offset;
+			reloc[n].target_handle = obj[0].handle;
+			reloc[n].presumed_offset = 0;
+			reloc[n].offset = (b - batch) * sizeof(*b);
+			reloc[n].delta = offset;
+			reloc[n].read_domains = I915_GEM_DOMAIN_RENDER;
+			reloc[n].write_domain = 0;
+			*b++ = offset;
+			if (gen >= 9)
+				*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 =
+		gem_class_instance_to_eb_flags(fd, e->class, e->instance);
+	execbuf.rsvd1 = ctx;
+	gem_execbuf(fd, &execbuf);
+	gem_close(fd, obj[1].handle);
+}
+
+__attribute__((unused))
+static void dump_regs(int fd,
+		      const struct intel_execution_engine2 *e,
+		      unsigned int regs)
+{
+	const int gen = intel_gen(intel_get_drm_devid(fd));
+	const unsigned int gen_bit = 1 << gen;
+	const unsigned int engine_bit = ENGINE(e->class, e->instance);
+	unsigned int regs_size;
+	uint32_t *out;
+
+	regs_size = NUM_REGS * sizeof(uint32_t);
+	regs_size = PAGE_ALIGN(regs_size);
+
+	out = gem_mmap__cpu(fd, regs, 0, regs_size, PROT_READ);
+	gem_set_domain(fd, regs, I915_GEM_DOMAIN_CPU, 0);
+
+	for (const struct named_register *r = nonpriv_registers; r->name; r++) {
+		if (!(r->engine_mask & engine_bit))
+			continue;
+		if (!(r->gen_mask & gen_bit))
+			continue;
+
+		if (r->count <= 1) {
+			igt_debug("0x%04x (%s): 0x%08x\n",
+				  r->offset, r->name, out[r->offset/4]);
+		} else {
+			for (unsigned x = 0; x < r->count; x++)
+				igt_debug("0x%04x (%s[%d]): 0x%08x\n",
+					  r->offset+4*x, r->name, x,
+					  out[r->offset/4 + x]);
+		}
+	}
+	munmap(out, regs_size);
+}
+
+static void compare_regs(int fd, uint32_t A, uint32_t B, const char *who)
+{
+	unsigned int num_errors;
+	unsigned int regs_size;
+	uint32_t *a, *b;
+	char buf[80];
+
+	regs_size = NUM_REGS * sizeof(uint32_t);
+	regs_size = PAGE_ALIGN(regs_size);
+
+	a = gem_mmap__cpu(fd, A, 0, regs_size, PROT_READ);
+	gem_set_domain(fd, A, I915_GEM_DOMAIN_CPU, 0);
+
+	b = gem_mmap__cpu(fd, B, 0, regs_size, PROT_READ);
+	gem_set_domain(fd, B, I915_GEM_DOMAIN_CPU, 0);
+
+	num_errors = 0;
+	for (unsigned int n = 0; n < NUM_REGS; n++) {
+		uint32_t offset = n * sizeof(uint32_t);
+		if (a[n] != b[n] && !ignore_register(offset)) {
+			igt_warn("Register 0x%04x (%s): A=%08x B=%08x\n",
+				 offset,
+				 register_name(offset, buf, sizeof(buf)),
+				 a[n], b[n]);
+			num_errors++;
+		}
+	}
+	munmap(b, regs_size);
+	munmap(a, regs_size);
+
+	igt_assert_f(num_errors == 0,
+		     "%d registers mistached between %s.\n",
+		     num_errors, who);
+}
+
+static void isolation(int fd,
+		      const struct intel_execution_engine2 *e,
+		      unsigned int flags)
+{
+	static const uint32_t values[] = {
+		0x0,
+		0xffffffff,
+		0xcccccccc,
+		0x33333333,
+		0x55555555,
+		0xaaaaaaaa,
+		0xdeadbeef
+	};
+	unsigned int engine = gem_class_instance_to_eb_flags(fd,
+							     e->class,
+							     e->instance);
+	unsigned int num_values =
+		flags & (DIRTY1 | DIRTY2) ? ARRAY_SIZE(values) : 1;
+
+	gem_quiescent_gpu(fd);
+
+	for (int v = 0; v < num_values; v++) {
+		igt_spin_t *spin = NULL;
+		uint32_t ctx[2], regs[2], tmp;
+
+		ctx[0] = gem_context_create(fd);
+		regs[0] = read_regs(fd, ctx[0], e, flags);
+
+		spin = igt_spin_batch_new(fd, ctx[0], engine, 0);
+
+		if (flags & DIRTY1) {
+			igt_debug("%s[%d]: Setting all registers of ctx 0 to 0x%08x\n",
+				  __func__, v, values[v]);
+			write_regs(fd, ctx[0], e, flags, values[v]);
+		}
+
+		/*
+		 * 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_regs(fd, ctx[1], e, flags);
+
+		if (flags & DIRTY2) {
+			igt_debug("%s[%d]: Setting all registers of ctx 1 to 0x%08x\n",
+				  __func__, v, ~values[v]);
+			write_regs(fd, ctx[1], e, flags, ~values[v]);
+		}
+
+		/*
+		 * Restore the original register values before the HW idles.
+		 * Or else it may never restart!
+		 */
+		tmp = read_regs(fd, ctx[0], e, flags);
+		restore_regs(fd, ctx[0], e, flags, regs[0]);
+
+		igt_spin_batch_free(fd, spin);
+
+		if (!(flags & DIRTY1))
+			compare_regs(fd, regs[0], tmp, "two reads of the same ctx");
+		compare_regs(fd, regs[0], regs[1], "two virgin contexts");
+
+		for (int n = 0; n < ARRAY_SIZE(ctx); n++) {
+			gem_close(fd, regs[n]);
+			gem_context_destroy(fd, ctx[n]);
+		}
+		gem_close(fd, tmp);
+	}
+}
+
+#define NOSLEEP (0 << 8)
+#define S3_DEVICES (1 << 8)
+#define S3 (2 << 8)
+#define S4_DEVICES (3 << 8)
+#define S4 (4 << 8)
+#define SLEEP_MASK (0xf << 8)
+
+static void preservation(int fd,
+			 const struct intel_execution_engine2 *e,
+			 unsigned int flags)
+{
+	static const uint32_t values[] = {
+		0x0,
+		0xffffffff,
+		0xcccccccc,
+		0x33333333,
+		0x55555555,
+		0xaaaaaaaa,
+		0xdeadbeef
+	};
+	const unsigned int num_values = ARRAY_SIZE(values);
+	unsigned int engine =
+		gem_class_instance_to_eb_flags(fd, e->class, e->instance);
+	uint32_t ctx[num_values +1 ];
+	uint32_t regs[num_values + 1][2];
+	igt_spin_t *spin;
+
+	gem_quiescent_gpu(fd);
+
+	ctx[num_values] = gem_context_create(fd);
+	spin = igt_spin_batch_new(fd, ctx[num_values], engine, 0);
+	regs[num_values][0] = read_regs(fd, ctx[num_values], e, flags);
+	for (int v = 0; v < num_values; v++) {
+		ctx[v] = gem_context_create(fd);
+		write_regs(fd, ctx[v], e, flags, values[v]);
+
+		regs[v][0] = read_regs(fd, ctx[v], e, flags);
+
+	}
+	gem_close(fd, read_regs(fd, ctx[num_values], e, flags));
+	igt_spin_batch_free(fd, spin);
+
+	if (flags & RESET)
+		igt_force_gpu_reset(fd);
+
+	switch (flags & SLEEP_MASK) {
+	case NOSLEEP:
+		break;
+
+	case S3_DEVICES:
+		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
+					      SUSPEND_TEST_DEVICES);
+		break;
+
+	case S3:
+		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
+					      SUSPEND_TEST_NONE);
+		break;
+
+	case S4_DEVICES:
+		igt_system_suspend_autoresume(SUSPEND_STATE_DISK,
+					      SUSPEND_TEST_DEVICES);
+		break;
+
+	case S4:
+		igt_system_suspend_autoresume(SUSPEND_STATE_DISK,
+					      SUSPEND_TEST_NONE);
+		break;
+	}
+
+	spin = igt_spin_batch_new(fd, ctx[num_values], engine, 0);
+	for (int v = 0; v < num_values; v++)
+		regs[v][1] = read_regs(fd, ctx[v], e, flags);
+	regs[num_values][1] = read_regs(fd, ctx[num_values], e, flags);
+	igt_spin_batch_free(fd, spin);
+
+	for (int v = 0; v < num_values; v++) {
+		char buf[80];
+
+		snprintf(buf, sizeof(buf), "dirty %x context\n", values[v]);
+		compare_regs(fd, regs[v][0], regs[v][1], buf);
+
+		gem_close(fd, regs[v][0]);
+		gem_close(fd, regs[v][1]);
+		gem_context_destroy(fd, ctx[v]);
+	}
+	compare_regs(fd, regs[num_values][0], regs[num_values][1], "clean");
+	gem_context_destroy(fd, ctx[num_values]);
+}
+
+static unsigned int __has_context_isolation(int fd)
+{
+	struct drm_i915_getparam gp;
+	int value = 0;
+
+	memset(&gp, 0, sizeof(gp));
+	gp.param = 50; /* I915_PARAM_HAS_CONTEXT_ISOLATION */
+	gp.value = &value;
+
+	igt_ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
+	errno = 0;
+
+	return value;
+}
+
+igt_main
+{
+	unsigned int has_context_isolation = 0;
+	int fd = -1;
+
+	igt_fixture {
+		int gen;
+
+		fd = drm_open_driver(DRIVER_INTEL);
+		igt_require_gem(fd);
+
+		gem_context_destroy(fd, gem_context_create(fd));
+		has_context_isolation = __has_context_isolation(fd);
+		igt_require(has_context_isolation);
+
+		gen = intel_gen(intel_get_drm_devid(fd));
+		//igt_ci_fail_on(gen > LAST_KNOWN_GEN);
+		igt_skip_on(gen > LAST_KNOWN_GEN);
+	}
+
+	for (const struct intel_execution_engine2 *e = intel_execution_engines2;
+	     e->name; e++) {
+		igt_subtest_group {
+			igt_fixture {
+				igt_require(has_context_isolation & (1 << e->class));
+				gem_require_engine(fd, e->class, e->instance);
+				igt_fork_hang_detector(fd);
+			}
+
+			igt_subtest_f("%s-clean", e->name)
+				isolation(fd, e, 0);
+			igt_subtest_f("%s-dirty-create", e->name)
+				isolation(fd, e, DIRTY1);
+			igt_subtest_f("%s-dirty-switch", e->name)
+				isolation(fd, e, DIRTY2);
+
+			igt_subtest_f("%s-none", e->name)
+				preservation(fd, e, 0);
+			igt_subtest_f("%s-S3", e->name)
+				preservation(fd, e, S3);
+			igt_subtest_f("%s-S4", e->name)
+				preservation(fd, e, S4);
+
+			igt_fixture {
+				igt_stop_hang_detector();
+			}
+
+			igt_subtest_f("%s-reset", e->name) {
+				igt_hang_t hang = igt_allow_hang(fd, 0, 0);
+				preservation(fd, e, RESET);
+				igt_disallow_hang(fd, hang);
+			}
+		}
+	}
+}
diff --git a/tests/gem_exec_fence.c b/tests/gem_exec_fence.c
index 6ee944ff..0094ca7f 100644
--- a/tests/gem_exec_fence.c
+++ b/tests/gem_exec_fence.c
@@ -698,7 +698,7 @@ static bool has_submit_fence(int fd)
 	int value = 0;
 
 	memset(&gp, 0, sizeof(gp));
-	gp.param = 50; /* I915_PARAM_HAS_EXEC_SUBMIT_FENCE */
+	gp.param = 0xdeadbeef ^ 51; /* I915_PARAM_HAS_EXEC_SUBMIT_FENCE */
 	gp.value = &value;
 
 	ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp, sizeof(gp));
-- 
2.15.1

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* ✓ Fi.CI.BAT: success for igt/gem_ctx_isolation: Check isolation of registers between contexts (rev9)
  2017-12-01 11:13 [PATCH igt] igt/gem_ctx_isolation: Check isolation of registers between contexts Chris Wilson
@ 2017-12-01 12:22 ` Patchwork
  2017-12-01 13:37 ` ✗ Fi.CI.IGT: failure " Patchwork
  2017-12-04 10:23 ` [PATCH igt] igt/gem_ctx_isolation: Check isolation of registers between contexts Joonas Lahtinen
  2 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2017-12-01 12:22 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: igt/gem_ctx_isolation: Check isolation of registers between contexts (rev9)
URL   : https://patchwork.freedesktop.org/series/32531/
State : success

== Summary ==

IGT patchset tested on top of latest successful build
476c4b462e0453c70ee81664c0227fdddc26cbd0 igt/gem_eio: Increase wakeup delay for in-flight-suspend

with latest DRM-Tip kernel build CI_DRM_3428
807db75b1a12 drm-tip: 2017y-12m-01d-11h-04m-28s UTC integration manifest

Testlist changes:
+igt@gem_ctx_isolation@bcs0-clean
+igt@gem_ctx_isolation@bcs0-dirty-create
+igt@gem_ctx_isolation@bcs0-dirty-switch
+igt@gem_ctx_isolation@bcs0-none
+igt@gem_ctx_isolation@bcs0-reset
+igt@gem_ctx_isolation@bcs0-s3
+igt@gem_ctx_isolation@bcs0-s4
+igt@gem_ctx_isolation@rcs0-clean
+igt@gem_ctx_isolation@rcs0-dirty-create
+igt@gem_ctx_isolation@rcs0-dirty-switch
+igt@gem_ctx_isolation@rcs0-none
+igt@gem_ctx_isolation@rcs0-reset
+igt@gem_ctx_isolation@rcs0-s3
+igt@gem_ctx_isolation@rcs0-s4
+igt@gem_ctx_isolation@vcs0-clean
+igt@gem_ctx_isolation@vcs0-dirty-create
+igt@gem_ctx_isolation@vcs0-dirty-switch
+igt@gem_ctx_isolation@vcs0-none
+igt@gem_ctx_isolation@vcs0-reset
+igt@gem_ctx_isolation@vcs0-s3
+igt@gem_ctx_isolation@vcs0-s4
+igt@gem_ctx_isolation@vcs1-clean
+igt@gem_ctx_isolation@vcs1-dirty-create
+igt@gem_ctx_isolation@vcs1-dirty-switch
+igt@gem_ctx_isolation@vcs1-none
+igt@gem_ctx_isolation@vcs1-reset
+igt@gem_ctx_isolation@vcs1-s3
+igt@gem_ctx_isolation@vcs1-s4
+igt@gem_ctx_isolation@vecs0-clean
+igt@gem_ctx_isolation@vecs0-dirty-create
+igt@gem_ctx_isolation@vecs0-dirty-switch
+igt@gem_ctx_isolation@vecs0-none
+igt@gem_ctx_isolation@vecs0-reset
+igt@gem_ctx_isolation@vecs0-s3
+igt@gem_ctx_isolation@vecs0-s4

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                pass       -> INCOMPLETE (fi-snb-2520m) fdo#103713

fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:443s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:386s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:526s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:284s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:498s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:507s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:493s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:480s
fi-elk-e7500     total:224  pass:162  dwarn:16  dfail:0   fail:0   skip:45 
fi-gdg-551       total:288  pass:178  dwarn:1   dfail:0   fail:1   skip:108 time:273s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:543s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:374s
fi-hsw-4770r     total:288  pass:224  dwarn:0   dfail:0   fail:0   skip:64  time:264s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:395s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:478s
fi-ivb-3770      total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:446s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:490s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:531s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:481s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:534s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:589s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:461s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:537s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:565s
fi-skl-6700k     total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:517s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:491s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:456s
fi-snb-2520m     total:245  pass:211  dwarn:0   dfail:0   fail:0   skip:33 
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:425s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:607s
fi-glk-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:492s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_576/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* ✗ Fi.CI.IGT: failure for igt/gem_ctx_isolation: Check isolation of registers between contexts (rev9)
  2017-12-01 11:13 [PATCH igt] igt/gem_ctx_isolation: Check isolation of registers between contexts Chris Wilson
  2017-12-01 12:22 ` ✓ Fi.CI.BAT: success for igt/gem_ctx_isolation: Check isolation of registers between contexts (rev9) Patchwork
@ 2017-12-01 13:37 ` Patchwork
  2017-12-04 10:23 ` [PATCH igt] igt/gem_ctx_isolation: Check isolation of registers between contexts Joonas Lahtinen
  2 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2017-12-01 13:37 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: igt/gem_ctx_isolation: Check isolation of registers between contexts (rev9)
URL   : https://patchwork.freedesktop.org/series/32531/
State : failure

== Summary ==

Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-primscrn-indfb-pgflip-blt:
                skip       -> PASS       (shard-snb) fdo#101623 +1
        Subgroup fbc-1p-offscren-pri-shrfb-draw-mmap-wc:
                skip       -> PASS       (shard-snb) fdo#103167
Test drv_selftest:
        Subgroup mock_sanitycheck:
                pass       -> DMESG-WARN (shard-snb) fdo#102707 +1
Test kms_cursor_legacy:
        Subgroup cursora-vs-flipa-legacy:
                skip       -> PASS       (shard-snb)
Test kms_flip:
        Subgroup blocking-wf_vblank:
                fail       -> PASS       (shard-snb)
Test kms_fbcon_fbt:
        Subgroup fbc-suspend:
                pass       -> SKIP       (shard-hsw)
Test kms_chv_cursor_fail:
        Subgroup pipe-b-128x128-top-edge:
                pass       -> INCOMPLETE (shard-hsw)
Test kms_plane:
        Subgroup plane-panning-top-left-pipe-a-planes:
                skip       -> PASS       (shard-snb)

fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#102707 https://bugs.freedesktop.org/show_bug.cgi?id=102707

shard-hsw        total:2608 pass:1484 dwarn:1   dfail:0   fail:9   skip:1112 time:8900s
shard-snb        total:2674 pass:1300 dwarn:3   dfail:0   fail:13  skip:1357 time:7701s
Blacklisted hosts:
shard-apl        total:2698 pass:1715 dwarn:1   dfail:0   fail:26  skip:956 time:14014s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_576/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH igt] igt/gem_ctx_isolation: Check isolation of registers between contexts
  2017-12-01 11:13 [PATCH igt] igt/gem_ctx_isolation: Check isolation of registers between contexts Chris Wilson
  2017-12-01 12:22 ` ✓ Fi.CI.BAT: success for igt/gem_ctx_isolation: Check isolation of registers between contexts (rev9) Patchwork
  2017-12-01 13:37 ` ✗ Fi.CI.IGT: failure " Patchwork
@ 2017-12-04 10:23 ` Joonas Lahtinen
  2017-12-04 10:30   ` Chris Wilson
  2 siblings, 1 reply; 8+ messages in thread
From: Joonas Lahtinen @ 2017-12-04 10:23 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

On Fri, 2017-12-01 at 11:13 +0000, Chris Wilson wrote:
> 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.
> 
> v2: Extend back to Sandybridge (etc)
> v3: Check context preserves registers across suspend/hibernate and resets.
> v4: Complete the remapping onto the new class:instance
> v5: Not like that, like this, try again to use class:instance
> v6: Prepare for retrospective gen4 contexts!
> v7: Repaint register set name to nonpriv, as this is what bspec calls the
> registers that are writable by userspace.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

<SNIP>

> +static uint32_t read_regs(int fd,
> +			  uint32_t ctx,
> +			  const struct intel_execution_engine2 *e,
> +			  unsigned int flags)
> +{
> +	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
> +	const unsigned int gen_bit = 1 << gen;

You did define BIT() in the beginning...

<SNIP>

> +	n = 0;
> +	for (const struct named_register *r = nonpriv_registers; r->name; r++) {
> +		if (!(r->engine_mask & engine_bit))
> +			continue;
> +		if (!(r->gen_mask & gen_bit))
> +			continue;
> +
> +		for (unsigned count = r->count ?: 1, offset = r->offset;
> +		     count--; offset += 4) {
> +			*b++ = 0x24 << 23 | (1 + r64b); /* SRM */
> +			*b++ = offset;
> +			reloc[n].target_handle = obj[0].handle;

I still kind of loathe the obj[0] vs obj[1] when it's not going to be
about comparing the two. obj_regs and obj_batch is just requires so
much fewer scroll-ups to see what was 0 and what was 1...

> +			reloc[n].presumed_offset = 0;
> +			reloc[n].offset = (b - batch) * sizeof(*b);
> +			reloc[n].delta = offset;

This might confuse somebody. At least call the variable r_offset (or
reg_offset).

> +			reloc[n].read_domains = I915_GEM_DOMAIN_RENDER;
> +			reloc[n].write_domain = I915_GEM_DOMAIN_RENDER;
> +			*b++ = offset;
> +			if (r64b)
> +				*b++ = 0;
> +			n++;

I can't see why this is not either inside the for () or bound closer to
filling the relocation, here it's bit of a "what?".

<SNIP>

> +static void restore_regs(int fd,
> +			 uint32_t ctx,
> +			 const struct intel_execution_engine2 *e,
> +			 unsigned int flags,
> +			 uint32_t regs)
> +{
> +	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
> +	const unsigned int gen_bit = 1 << gen;
> +	const unsigned int engine_bit = ENGINE(e->class, e->instance);
> +	const bool r64b = gen >= 8;
> +	struct drm_i915_gem_exec_object2 obj[2];
> +	struct drm_i915_gem_execbuffer2 execbuf;
> +	struct drm_i915_gem_relocation_entry *reloc;
> +	unsigned int batch_size, n;
> +	uint32_t *batch, *b;
> +
> +	if (gen < 7) /* no LRM */
> +		return;

Considering this restriction, would not it be possible to just CPU
mmap and have this as a bunch of LRIs?

<SNIP>

> +static void preservation(int fd,
> +			 const struct intel_execution_engine2 *e,
> +			 unsigned int flags)
> +{
> +	static const uint32_t values[] = {
> +		0x0,
> +		0xffffffff,
> +		0xcccccccc,
> +		0x33333333,
> +		0x55555555,
> +		0xaaaaaaaa,
> +		0xdeadbeef
> +	};
> +	const unsigned int num_values = ARRAY_SIZE(values);
> +	unsigned int engine =
> +		gem_class_instance_to_eb_flags(fd, e->class, e->instance);
> +	uint32_t ctx[num_values +1 ];

"+ 1"

<SNIP>

> +static unsigned int __has_context_isolation(int fd)
> +{
> +	struct drm_i915_getparam gp;
> +	int value = 0;
> +
> +	memset(&gp, 0, sizeof(gp));
> +	gp.param = 50; /* I915_PARAM_HAS_CONTEXT_ISOLATION */
> +	gp.value = &value;
> +
> +	igt_ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
> +	errno = 0;
> +
> +	return value;
> +}

<SNIP>

> +++ b/tests/gem_exec_fence.c
> @@ -698,7 +698,7 @@ static bool has_submit_fence(int fd)
>  	int value = 0;
>  
>  	memset(&gp, 0, sizeof(gp));
> -	gp.param = 50; /* I915_PARAM_HAS_EXEC_SUBMIT_FENCE */
> +	gp.param = 0xdeadbeef ^ 51; /* I915_PARAM_HAS_EXEC_SUBMIT_FENCE */
>  	gp.value = &value;
>  
>  	ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp, sizeof(gp));

Probably wan't to sort the param stuff out :)

With the params corrected, this is;

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH igt] igt/gem_ctx_isolation: Check isolation of registers between contexts
  2017-12-04 10:23 ` [PATCH igt] igt/gem_ctx_isolation: Check isolation of registers between contexts Joonas Lahtinen
@ 2017-12-04 10:30   ` Chris Wilson
  0 siblings, 0 replies; 8+ messages in thread
From: Chris Wilson @ 2017-12-04 10:30 UTC (permalink / raw)
  To: Joonas Lahtinen, intel-gfx

Quoting Joonas Lahtinen (2017-12-04 10:23:11)
> On Fri, 2017-12-01 at 11:13 +0000, Chris Wilson wrote:
> > +++ b/tests/gem_exec_fence.c
> > @@ -698,7 +698,7 @@ static bool has_submit_fence(int fd)
> >       int value = 0;
> >  
> >       memset(&gp, 0, sizeof(gp));
> > -     gp.param = 50; /* I915_PARAM_HAS_EXEC_SUBMIT_FENCE */
> > +     gp.param = 0xdeadbeef ^ 51; /* I915_PARAM_HAS_EXEC_SUBMIT_FENCE */
> >       gp.value = &value;
> >  
> >       ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp, sizeof(gp));
> 
> Probably wan't to sort the param stuff out :)
> 
> With the params corrected, this is;

That is the correction! Waiting for the feature to be requested...
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH igt] igt/gem_ctx_isolation: Check isolation of registers between contexts
@ 2017-11-27 13:03 Chris Wilson
  0 siblings, 0 replies; 8+ messages in thread
From: Chris Wilson @ 2017-11-27 13:03 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.

v2: Extend back to Sandybridge (etc)
v3: Check context preserves registers across suspend/hibernate and resets.
v4: Complete the remapping onto the new class:instance
v5: Not like that, like this, try again to use class:instance
v6: Prepare for retrospective gen4 contexts!

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

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index b4d4831e..57ecce6d 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -58,6 +58,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..0a1f09f8
--- /dev/null
+++ b/tests/gem_ctx_isolation.c
@@ -0,0 +1,677 @@
+/*
+ * 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 DIRTY1 0x1
+#define DIRTY2 0x2
+#define RESET 0x4
+
+#define BIT(x) (1ul << (x))
+#define ENGINE(x, y) BIT(4*(x) + (y))
+
+enum {
+	RCS0 = ENGINE(I915_ENGINE_CLASS_RENDER, 0),
+	BCS0 = ENGINE(I915_ENGINE_CLASS_COPY, 0),
+	VCS0 = ENGINE(I915_ENGINE_CLASS_VIDEO, 0),
+	VCS1 = ENGINE(I915_ENGINE_CLASS_VIDEO, 1),
+	VECS0 = ENGINE(I915_ENGINE_CLASS_VIDEO_ENHANCE, 0),
+};
+
+#define ALL ~0u
+#define GEN_RANGE(x, y) ((ALL >> (32 - (y - x + 1))) << x)
+#define GEN4 (ALL << 4)
+#define GEN5 (ALL << 5)
+#define GEN6 (ALL << 6)
+#define GEN7 (ALL << 7)
+#define GEN8 (ALL << 8)
+#define GEN9 (ALL << 9)
+
+#define NOCTX 0
+
+#define LAST_KNOWN_GEN 10
+
+static const struct named_register {
+	const char *name;
+	unsigned int gen_mask;
+	unsigned int engine_mask;
+	uint32_t offset;
+	uint32_t count;
+} safe_registers[] = {
+	{ "NOPID", NOCTX, RCS0, 0x2094 },
+	{ "MI_PREDICATE_RESULT_2", NOCTX, RCS0, 0x23bc },
+	{ "INSTPM", GEN9, RCS0, 0x20c0 },
+	{ "IA_VERTICES_COUNT", GEN4, RCS0, 0x2310, 2 },
+	{ "IA_PRIMITIVES_COUNT", GEN4, RCS0, 0x2318, 2 },
+	{ "VS_INVOCATION_COUNT", GEN4, RCS0, 0x2320, 2 },
+	{ "HS_INVOCATION_COUNT", GEN4, RCS0, 0x2300, 2 },
+	{ "DS_INVOCATION_COUNT", GEN4, RCS0, 0x2308, 2 },
+	{ "GS_INVOCATION_COUNT", GEN4, RCS0, 0x2328, 2 },
+	{ "GS_PRIMITIVES_COUNT", GEN4, RCS0, 0x2330, 2 },
+	{ "CL_INVOCATION_COUNT", GEN4, RCS0, 0x2338, 2 },
+	{ "CL_PRIMITIVES_COUNT", GEN4, RCS0, 0x2340, 2 },
+	{ "PS_INVOCATION_COUNT_0", GEN4, RCS0, 0x22c8, 2 },
+	{ "PS_DEPTH_COUNT_0", GEN4, RCS0, 0x22d8, 2 },
+	{ "GPUGPU_DISPATCHDIMX", GEN8, RCS0, 0x2500 },
+	{ "GPUGPU_DISPATCHDIMY", GEN8, RCS0, 0x2504 },
+	{ "GPUGPU_DISPATCHDIMZ", GEN8, RCS0, 0x2508 },
+	{ "MI_PREDICATE_SRC0", GEN8, RCS0, 0x2400, 2 },
+	{ "MI_PREDICATE_SRC1", GEN8, RCS0, 0x2408, 2 },
+	{ "MI_PREDICATE_DATA", GEN8, RCS0, 0x2410, 2 },
+	{ "MI_PRED_RESULT", GEN8, RCS0, 0x2418 },
+	{ "3DPRIM_END_OFFSET", GEN6, RCS0, 0x2420 },
+	{ "3DPRIM_START_VERTEX", GEN6, RCS0, 0x2430 },
+	{ "3DPRIM_VERTEX_COUNT", GEN6, RCS0, 0x2434 },
+	{ "3DPRIM_INSTANCE_COUNT", GEN6, RCS0, 0x2438 },
+	{ "3DPRIM_START_INSTANCE", GEN6, RCS0, 0x243c },
+	{ "3DPRIM_BASE_VERTEX", GEN6, RCS0, 0x2440 },
+	{ "GPGPU_THREADS_DISPATCHED", GEN8, RCS0, 0x2290, 2 },
+	{ "PS_INVOCATION_COUNT_1", GEN8, RCS0, 0x22f0, 2 },
+	{ "PS_DEPTH_COUNT_1", GEN8, RCS0, 0x22f8, 2 },
+	{ "BB_OFFSET", GEN8, RCS0, 0x2158 },
+	{ "MI_PREDICATE_RESULT_1", GEN8, RCS0, 0x241c },
+	{ "CS_GPR", GEN8, RCS0, 0x2600, 32 },
+	{ "OA_CTX_CONTROL", GEN8, RCS0, 0x2360 },
+	{ "OACTXID", GEN8, RCS0, 0x2364 },
+	{ "PS_INVOCATION_COUNT_2", GEN8, RCS0, 0x2448, 2 },
+	{ "PS_DEPTH_COUNT_2", GEN8, RCS0, 0x2450, 2 },
+	{ "Cache_Mode_0", GEN7, RCS0, 0x7000 },
+	{ "Cache_Mode_1", GEN7, RCS0, 0x7004 },
+	{ "GT_MODE", GEN8, RCS0, 0x7008 },
+	{ "L3_Config", GEN7, RCS0, 0x7034 },
+	{ "TD_CTL", GEN8, RCS0, 0xe400 },
+	{ "TD_CTL2", GEN8, RCS0, 0xe404 },
+	{ "SO_NUM_PRIMS_WRITEN0", GEN6, RCS0, 0x5200, 2 },
+	{ "SO_NUM_PRIMS_WRITEN1", GEN6, RCS0, 0x5208, 2 },
+	{ "SO_NUM_PRIMS_WRITEN2", GEN6, RCS0, 0x5210, 2 },
+	{ "SO_NUM_PRIMS_WRITEN3", GEN6, RCS0, 0x5218, 2 },
+	{ "SO_PRIM_STORAGE_NEEDED0", GEN6, RCS0, 0x5240, 2 },
+	{ "SO_PRIM_STORAGE_NEEDED1", GEN6, RCS0, 0x5248, 2 },
+	{ "SO_PRIM_STORAGE_NEEDED2", GEN6, RCS0, 0x5250, 2 },
+	{ "SO_PRIM_STORAGE_NEEDED3", GEN6, RCS0, 0x5258, 2 },
+	{ "SO_WRITE_OFFSET0", GEN7, RCS0, 0x5280 },
+	{ "SO_WRITE_OFFSET1", GEN7, RCS0, 0x5284 },
+	{ "SO_WRITE_OFFSET2", GEN7, RCS0, 0x5288 },
+	{ "SO_WRITE_OFFSET3", GEN7, RCS0, 0x528c },
+	{ "OA_CONTROL", NOCTX, RCS0, 0x2b00 },
+	{ "PERF_CNT_1", NOCTX, RCS0, 0x91b8, 2 },
+	{ "PERF_CNT_2", NOCTX, RCS0, 0x91c0, 2 },
+
+	/* Privileged (enabled by w/a + FORCE_TO_NONPRIV) */
+	{ "CTX_PREEMPT", NOCTX /* GEN_RANGE(9, 10) */, RCS0, 0x2248 },
+	{ "CS_CHICKEN1", GEN_RANGE(9, 10), RCS0, 0x2580 },
+	{ "HDC_CHICKEN1", GEN_RANGE(9, 10), RCS0, 0x7304 },
+	{ "L3SQREG1", GEN8, RCS0, 0xb010 },
+
+	{ "BCS_GPR", GEN9, BCS0, 0x22600, 32 },
+	{ "BCS_SWCTRL", GEN8, BCS0, 0x22200 },
+
+	{ "VCS0_GPR", GEN9, VCS0, 0x12600, 32 },
+	{ "MFC_VDBOX1", NOCTX, VCS0, 0x12800, 64 },
+
+	{ "VCS1_GPR", GEN9, VCS1, 0x1c600, 32 },
+	{ "MFC_VDBOX2", NOCTX, VCS1, 0x1c800, 64 },
+
+	{ "VECS_GPR", GEN9, VECS0, 0x1a600, 32 },
+
+	{}
+}, ignore_registers[] = {
+	{ "RCS timestamp", GEN6, ~0u, 0x2358 },
+	{ "VCS0 timestamp", GEN7, ~0u, 0x12358 },
+	{ "VCS1 timestamp", GEN7, ~0u, 0x1c358 },
+	{ "BCS timestamp", GEN7, ~0u, 0x22358 },
+	{ "VECS timestamp", GEN8, ~0u, 0x1a358 },
+	{}
+};
+
+static const char *register_name(uint32_t offset, char *buf, size_t len)
+{
+	for (const struct named_register *r = safe_registers; r->name; r++) {
+		unsigned int width = r->count ? 4*r->count : 4;
+		if (offset >= r->offset && offset < r->offset + width) {
+			if (r->count <= 1)
+				return r->name;
+
+			snprintf(buf, len, "%s[%d]",
+				 r->name, (offset - r->offset)/4);
+			return buf;
+		}
+	}
+
+	return "unknown";
+}
+
+static bool ignore_register(uint32_t offset)
+{
+	for (const struct named_register *r = ignore_registers; r->name; r++) {
+		unsigned int width = r->count ? 4*r->count : 4;
+		if (offset >= r->offset && offset < r->offset + width)
+			return true;
+	}
+
+	return false;
+}
+
+static uint32_t read_regs(int fd,
+			  uint32_t ctx,
+			  const struct intel_execution_engine2 *e,
+			  unsigned int flags)
+{
+	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
+	const unsigned int gen_bit = 1 << gen;
+	const unsigned int engine_bit = ENGINE(e->class, e->instance);
+	const bool r64b = gen >= 8;
+	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, n;
+	uint32_t *batch, *b;
+
+	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);
+	gem_set_domain(fd, obj[1].handle,
+		       I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
+
+	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;
+
+		for (unsigned count = r->count ?: 1, offset = r->offset;
+		     count--; offset += 4) {
+			*b++ = 0x24 << 23 | (1 + r64b); /* SRM */
+			*b++ = offset;
+			reloc[n].target_handle = obj[0].handle;
+			reloc[n].presumed_offset = 0;
+			reloc[n].offset = (b - batch) * sizeof(*b);
+			reloc[n].delta = offset;
+			reloc[n].read_domains = I915_GEM_DOMAIN_RENDER;
+			reloc[n].write_domain = I915_GEM_DOMAIN_RENDER;
+			*b++ = offset;
+			if (r64b)
+				*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 =
+		gem_class_instance_to_eb_flags(fd, e->class, e->instance);
+	execbuf.rsvd1 = ctx;
+	gem_execbuf(fd, &execbuf);
+	gem_close(fd, obj[1].handle);
+	free(reloc);
+
+	return obj[0].handle;
+}
+
+static void write_regs(int fd,
+		       uint32_t ctx,
+		       const struct intel_execution_engine2 *e,
+		       unsigned int flags,
+		       uint32_t value)
+{
+	const unsigned int gen_bit = 1 << intel_gen(intel_get_drm_devid(fd));
+	const unsigned int engine_bit = ENGINE(e->class, e->instance);
+	struct drm_i915_gem_exec_object2 obj;
+	struct drm_i915_gem_execbuffer2 execbuf;
+	unsigned int batch_size;
+	uint32_t *batch, *b;
+
+	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);
+	gem_set_domain(fd, obj.handle,
+		       I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
+	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;
+		for (unsigned count = r->count ?: 1, offset = r->offset;
+		     count--; offset += 4) {
+			*b++ = 0x22 << 23 | 1; /* LRI */
+			*b++ = offset;
+			*b++ = value;
+		}
+	}
+	*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 =
+		gem_class_instance_to_eb_flags(fd, e->class, e->instance);
+	execbuf.rsvd1 = ctx;
+	gem_execbuf(fd, &execbuf);
+	gem_close(fd, obj.handle);
+}
+
+static void restore_regs(int fd,
+			 uint32_t ctx,
+			 const struct intel_execution_engine2 *e,
+			 unsigned int flags,
+			 uint32_t regs)
+{
+	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
+	const unsigned int gen_bit = 1 << gen;
+	const unsigned int engine_bit = ENGINE(e->class, e->instance);
+	const bool r64b = gen >= 8;
+	struct drm_i915_gem_exec_object2 obj[2];
+	struct drm_i915_gem_execbuffer2 execbuf;
+	struct drm_i915_gem_relocation_entry *reloc;
+	unsigned int batch_size, n;
+	uint32_t *batch, *b;
+
+	if (gen < 7) /* no LRM */
+		return;
+
+	reloc = calloc(NUM_REGS, sizeof(*reloc));
+	igt_assert(reloc);
+
+	batch_size = NUM_REGS * 3 * sizeof(uint32_t) + 4;
+	batch_size = PAGE_ALIGN(batch_size);
+
+	memset(obj, 0, sizeof(obj));
+	obj[0].handle = regs;
+	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);
+	gem_set_domain(fd, obj[1].handle,
+		       I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
+
+	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;
+
+		for (unsigned count = r->count ?: 1, offset = r->offset;
+		     count--; offset += 4) {
+			*b++ = 0x29 << 23 | (1 + r64b); /* LRM */
+			*b++ = offset;
+			reloc[n].target_handle = obj[0].handle;
+			reloc[n].presumed_offset = 0;
+			reloc[n].offset = (b - batch) * sizeof(*b);
+			reloc[n].delta = offset;
+			reloc[n].read_domains = I915_GEM_DOMAIN_RENDER;
+			reloc[n].write_domain = 0;
+			*b++ = offset;
+			if (gen >= 9)
+				*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 =
+		gem_class_instance_to_eb_flags(fd, e->class, e->instance);
+	execbuf.rsvd1 = ctx;
+	gem_execbuf(fd, &execbuf);
+	gem_close(fd, obj[1].handle);
+}
+
+__attribute__((unused))
+static void dump_regs(int fd,
+		      const struct intel_execution_engine2 *e,
+		      unsigned int regs)
+{
+	const int gen = intel_gen(intel_get_drm_devid(fd));
+	const unsigned int gen_bit = 1 << gen;
+	const unsigned int engine_bit = ENGINE(e->class, e->instance);
+	unsigned int regs_size;
+	uint32_t *out;
+
+	regs_size = NUM_REGS * sizeof(uint32_t);
+	regs_size = PAGE_ALIGN(regs_size);
+
+	out = gem_mmap__cpu(fd, regs, 0, regs_size, PROT_READ);
+	gem_set_domain(fd, regs, I915_GEM_DOMAIN_CPU, 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;
+
+		if (r->count <= 1) {
+			igt_debug("0x%04x (%s): 0x%08x\n",
+				  r->offset, r->name, out[r->offset/4]);
+		} else {
+			for (unsigned x = 0; x < r->count; x++)
+				igt_debug("0x%04x (%s[%d]): 0x%08x\n",
+					  r->offset+4*x, r->name, x,
+					  out[r->offset/4 + x]);
+		}
+	}
+	munmap(out, regs_size);
+}
+
+static void compare_regs(int fd, uint32_t A, uint32_t B, const char *who)
+{
+	unsigned int num_errors;
+	unsigned int regs_size;
+	uint32_t *a, *b;
+	char buf[80];
+
+	regs_size = NUM_REGS * sizeof(uint32_t);
+	regs_size = PAGE_ALIGN(regs_size);
+
+	a = gem_mmap__cpu(fd, A, 0, regs_size, PROT_READ);
+	gem_set_domain(fd, A, I915_GEM_DOMAIN_CPU, 0);
+
+	b = gem_mmap__cpu(fd, B, 0, regs_size, PROT_READ);
+	gem_set_domain(fd, B, I915_GEM_DOMAIN_CPU, 0);
+
+	num_errors = 0;
+	for (unsigned int n = 0; n < NUM_REGS; n++) {
+		uint32_t offset = n * sizeof(uint32_t);
+		if (a[n] != b[n] && !ignore_register(offset)) {
+			igt_warn("Register 0x%04x (%s): A=%08x B=%08x\n",
+				 offset,
+				 register_name(offset, buf, sizeof(buf)),
+				 a[n], b[n]);
+			num_errors++;
+		}
+	}
+	munmap(b, regs_size);
+	munmap(a, regs_size);
+
+	igt_assert_f(num_errors == 0,
+		     "%d registers mistached between %s.\n",
+		     num_errors, who);
+}
+
+static void isolation(int fd,
+		      const struct intel_execution_engine2 *e,
+		      unsigned int flags)
+{
+	static const uint32_t values[] = {
+		0x0,
+		0xffffffff,
+		0xcccccccc,
+		0x33333333,
+		0x55555555,
+		0xaaaaaaaa,
+		0xdeadbeef
+	};
+	unsigned int engine = gem_class_instance_to_eb_flags(fd,
+							     e->class,
+							     e->instance);
+	unsigned int num_values =
+		flags & (DIRTY1 | DIRTY2) ? ARRAY_SIZE(values) : 1;
+
+	gem_quiescent_gpu(fd);
+
+	for (int v = 0; v < num_values; v++) {
+		igt_spin_t *spin = NULL;
+		uint32_t ctx[2], regs[2], tmp;
+
+		ctx[0] = gem_context_create(fd);
+		regs[0] = read_regs(fd, ctx[0], e, flags);
+
+		spin = igt_spin_batch_new(fd, ctx[0], engine, 0);
+
+		if (flags & DIRTY1) {
+			igt_debug("%s[%d]: Setting all registers of ctx 0 to 0x%08x\n",
+				  __func__, v, values[v]);
+			write_regs(fd, ctx[0], e, flags, values[v]);
+		}
+
+		/*
+		 * 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_regs(fd, ctx[1], e, flags);
+
+		if (flags & DIRTY2) {
+			igt_debug("%s[%d]: Setting all registers of ctx 1 to 0x%08x\n",
+				  __func__, v, ~values[v]);
+			write_regs(fd, ctx[1], e, flags, ~values[v]);
+		}
+
+		/*
+		 * Restore the original register values before the HW idles.
+		 * Or else it may never restart!
+		 */
+		tmp = read_regs(fd, ctx[0], e, flags);
+		restore_regs(fd, ctx[0], e, flags, regs[0]);
+
+		igt_spin_batch_free(fd, spin);
+
+		if (!(flags & DIRTY1))
+			compare_regs(fd, regs[0], tmp, "two reads of the same ctx");
+		compare_regs(fd, regs[0], regs[1], "two virgin contexts");
+
+		for (int n = 0; n < ARRAY_SIZE(ctx); n++) {
+			gem_close(fd, regs[n]);
+			gem_context_destroy(fd, ctx[n]);
+		}
+		gem_close(fd, tmp);
+	}
+}
+
+#define NOSLEEP (0 << 8)
+#define S3_DEVICES (1 << 8)
+#define S3 (2 << 8)
+#define S4_DEVICES (3 << 8)
+#define S4 (4 << 8)
+#define SLEEP_MASK (0xf << 8)
+
+static void preservation(int fd,
+			 const struct intel_execution_engine2 *e,
+			 unsigned int flags)
+{
+	static const uint32_t values[] = {
+		0x0,
+		0xffffffff,
+		0xcccccccc,
+		0x33333333,
+		0x55555555,
+		0xaaaaaaaa,
+		0xdeadbeef
+	};
+	const unsigned int num_values = ARRAY_SIZE(values);
+	unsigned int engine =
+		gem_class_instance_to_eb_flags(fd, e->class, e->instance);
+	uint32_t ctx[num_values +1 ];
+	uint32_t regs[num_values + 1][2];
+	igt_spin_t *spin;
+
+	gem_quiescent_gpu(fd);
+
+	ctx[num_values] = gem_context_create(fd);
+	spin = igt_spin_batch_new(fd, ctx[num_values], engine, 0);
+	regs[num_values][0] = read_regs(fd, ctx[num_values], e, flags);
+	for (int v = 0; v < num_values; v++) {
+		ctx[v] = gem_context_create(fd);
+		write_regs(fd, ctx[v], e, flags, values[v]);
+
+		regs[v][0] = read_regs(fd, ctx[v], e, flags);
+
+	}
+	gem_close(fd, read_regs(fd, ctx[num_values], e, flags));
+	igt_spin_batch_free(fd, spin);
+
+	if (flags & RESET)
+		igt_force_gpu_reset(fd);
+
+	switch (flags & SLEEP_MASK) {
+	case NOSLEEP:
+		break;
+
+	case S3_DEVICES:
+		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
+					      SUSPEND_TEST_DEVICES);
+		break;
+
+	case S3:
+		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
+					      SUSPEND_TEST_NONE);
+		break;
+
+	case S4_DEVICES:
+		igt_system_suspend_autoresume(SUSPEND_STATE_DISK,
+					      SUSPEND_TEST_DEVICES);
+		break;
+
+	case S4:
+		igt_system_suspend_autoresume(SUSPEND_STATE_DISK,
+					      SUSPEND_TEST_NONE);
+		break;
+	}
+
+	spin = igt_spin_batch_new(fd, ctx[num_values], engine, 0);
+	for (int v = 0; v < num_values; v++)
+		regs[v][1] = read_regs(fd, ctx[v], e, flags);
+	regs[num_values][1] = read_regs(fd, ctx[num_values], e, flags);
+	igt_spin_batch_free(fd, spin);
+
+	for (int v = 0; v < num_values; v++) {
+		char buf[80];
+
+		snprintf(buf, sizeof(buf), "dirty %x context\n", values[v]);
+		compare_regs(fd, regs[v][0], regs[v][1], buf);
+
+		gem_close(fd, regs[v][0]);
+		gem_close(fd, regs[v][1]);
+		gem_context_destroy(fd, ctx[v]);
+	}
+	compare_regs(fd, regs[num_values][0], regs[num_values][1], "clean");
+	gem_context_destroy(fd, ctx[num_values]);
+}
+
+static unsigned int __has_context_isolation(int fd)
+{
+	struct drm_i915_getparam gp;
+	int value = 0;
+
+	memset(&gp, 0, sizeof(gp));
+	gp.param = 50; /* I915_PARAM_HAS_CONTEXT_ISOLATION */
+	gp.value = &value;
+
+	igt_ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
+	errno = 0;
+
+	return value;
+}
+
+igt_main
+{
+	unsigned int has_context_isolation = 0;
+	int fd = -1;
+
+	igt_fixture {
+		int gen;
+
+		fd = drm_open_driver(DRIVER_INTEL);
+		igt_require_gem(fd);
+
+		gem_context_destroy(fd, gem_context_create(fd));
+		has_context_isolation = __has_context_isolation(fd);
+		igt_require(has_context_isolation);
+
+		gen = intel_gen(intel_get_drm_devid(fd));
+		//igt_ci_fail_on(gen > LAST_KNOWN_GEN);
+		igt_skip_on(gen > LAST_KNOWN_GEN);
+	}
+
+	for (const struct intel_execution_engine2 *e = intel_execution_engines2;
+	     e->name; e++) {
+		igt_subtest_group {
+			igt_fixture {
+				igt_require(has_context_isolation & (1 << e->class));
+				gem_require_engine(fd, e->class, e->instance);
+				igt_fork_hang_detector(fd);
+			}
+
+			igt_subtest_f("%s-clean", e->name)
+				isolation(fd, e, 0);
+			igt_subtest_f("%s-dirty-create", e->name)
+				isolation(fd, e, DIRTY1);
+			igt_subtest_f("%s-dirty-switch", e->name)
+				isolation(fd, e, DIRTY2);
+
+			igt_subtest_f("%s-none", e->name)
+				preservation(fd, e, 0);
+			igt_subtest_f("%s-S3", e->name)
+				preservation(fd, e, S3);
+			igt_subtest_f("%s-S4", e->name)
+				preservation(fd, e, S4);
+
+			igt_fixture {
+				igt_stop_hang_detector();
+			}
+
+			igt_subtest_f("%s-reset", e->name) {
+				igt_hang_t hang = igt_allow_hang(fd, 0, 0);
+				preservation(fd, e, RESET);
+				igt_disallow_hang(fd, hang);
+			}
+		}
+	}
+}
diff --git a/tests/gem_exec_fence.c b/tests/gem_exec_fence.c
index 6ee944ff..0094ca7f 100644
--- a/tests/gem_exec_fence.c
+++ b/tests/gem_exec_fence.c
@@ -698,7 +698,7 @@ static bool has_submit_fence(int fd)
 	int value = 0;
 
 	memset(&gp, 0, sizeof(gp));
-	gp.param = 50; /* I915_PARAM_HAS_EXEC_SUBMIT_FENCE */
+	gp.param = 0xdeadbeef ^ 51; /* I915_PARAM_HAS_EXEC_SUBMIT_FENCE */
 	gp.value = &value;
 
 	ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp, sizeof(gp));
-- 
2.15.0

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [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
  0 siblings, 0 replies; 8+ messages in thread
From: Chris Wilson @ 2017-10-24 11:16 UTC (permalink / raw)
  To: intel-gfx

Quoting Chris Wilson (2017-10-24 12:07:58)
> 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,
> +};

We be nice to include these in the future intel_engine interface!

> +#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[] = {

This list is incomplete, we need the list of nonpriv 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 },
> +       {}
> +};

> +igt_main
> +{
> +       const unsigned int platform_validation = 0;

So UNSAFE is decidedly unsafe; is there anyway we can run tests only
during PV?

> +       int fd = -1;
> +
> +       igt_fixture {
> +               fd = drm_open_driver(DRIVER_INTEL);
> +               igt_require_gem(fd);
> +
> +               igt_require(gem_has_execlists(fd));

And we need an igt_ci_fail_on(gen >= KNOWN_GEN);

> +               /* check that we can create contexts. */
> +               gem_context_destroy(fd, gem_context_create(fd));
> +       }
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [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
  0 siblings, 1 reply; 8+ 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] 8+ messages in thread

end of thread, other threads:[~2017-12-04 10:30 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-01 11:13 [PATCH igt] igt/gem_ctx_isolation: Check isolation of registers between contexts Chris Wilson
2017-12-01 12:22 ` ✓ Fi.CI.BAT: success for igt/gem_ctx_isolation: Check isolation of registers between contexts (rev9) Patchwork
2017-12-01 13:37 ` ✗ Fi.CI.IGT: failure " Patchwork
2017-12-04 10:23 ` [PATCH igt] igt/gem_ctx_isolation: Check isolation of registers between contexts Joonas Lahtinen
2017-12-04 10:30   ` Chris Wilson
  -- strict thread matches above, loose matches on Subject: below --
2017-11-27 13:03 Chris Wilson
2017-10-24 11:07 Chris Wilson
2017-10-24 11:16 ` Chris Wilson

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.