[Intel-gfx] [PATCH v2 i-g-t 0/8] Fixes for gem_exec_capture

[Intel-gfx] [PATCH v2 i-g-t 0/8] Fixes for gem_exec_capture

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

Fix a bunch of issues with gem_exec_capture with the ultimate aim of
making it pass on GuC enabled platforms.

v2: Abstract the 'find first available engine' block into a helper
(review feedback from Matthew B). Note that for unknown reasons, this
helper does not work as a function. After wasting far too long trying
to debug out why the engine mask in the execbuf was *sometimes*
invalid, just making it a macro instead worked fine. Seems like maybe
the for_each_ctx_engine macro creates a local stack variable that is
silently required to remain in scope for the resulting ctx to be valid?

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>


John Harrison (8):
  tests/i915/gem_exec_capture: Remove pointless assert
  tests/i915/gem_exec_capture: Cope with larger page sizes
  tests/i915/gem_exec_capture: Make the error decode a common helper
  tests/i915/gem_exec_capture: Use contexts and engines properly
  tests/i915/gem_exec_capture: Check for memory allocation failure
  lib/igt_sysfs: Support large files
  lib/igt_gt: Allow per engine reset testing
  tests/i915/gem_exec_capture: Update to support GuC based resets

 lib/igt_gt.c                  |  44 ++--
 lib/igt_gt.h                  |   1 +
 lib/igt_sysfs.c               |  17 +-
 tests/i915/gem_exec_capture.c | 469 ++++++++++++++++++++--------------
 4 files changed, 315 insertions(+), 216 deletions(-)

-- 
2.25.1

[Intel-gfx] [PATCH v2 i-g-t 1/8] tests/i915/gem_exec_capture: Remove pointless assert

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

The 'many' test ended with an 'assert(count)', presumably meaning to
ensure that some objects were actually captured. However, 'count' is
the number of objects created not how many were captured. Plus, there
is already a 'require(count > 1)' at the start and count is invarient
so the final assert is basically pointless.

General concensus appears to be that the test should not fail
irrespective of how many blobs are captured as low memory situations
could cause the capture to be abbreviated. So just remove the
pointless assert completely.

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
---
 tests/i915/gem_exec_capture.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/i915/gem_exec_capture.c b/tests/i915/gem_exec_capture.c
index 7e0a8b8ad..53649cdb2 100644
--- a/tests/i915/gem_exec_capture.c
+++ b/tests/i915/gem_exec_capture.c
@@ -524,7 +524,6 @@ static void many(int fd, int dir, uint64_t size, unsigned int flags)
 	}
 	igt_info("Captured %lu %"PRId64"-blobs out of a total of %lu\n",
 		 blobs, size >> 12, count);
-	igt_assert(count);
 
 	free(error);
 	free(offsets);
-- 
2.25.1

[Intel-gfx] [PATCH v2 i-g-t 2/8] tests/i915/gem_exec_capture: Cope with larger page sizes

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

At some point, larger than 4KB page sizes were added to the i915
driver. This included adding an informational line to the buffer
entries in error capture logs. However, the error capture test was not
updated to skip this string, thus it would silently abort processing.

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
---
 tests/i915/gem_exec_capture.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tests/i915/gem_exec_capture.c b/tests/i915/gem_exec_capture.c
index 53649cdb2..47ca64dd6 100644
--- a/tests/i915/gem_exec_capture.c
+++ b/tests/i915/gem_exec_capture.c
@@ -484,6 +484,12 @@ static void many(int fd, int dir, uint64_t size, unsigned int flags)
 		addr |= strtoul(str + 1, &str, 16);
 		igt_assert(*str++ == '\n');
 
+		/* gtt_page_sizes = 0x00010000 */
+		if (strncmp(str, "gtt_page_sizes = 0x", 19) == 0) {
+			str += 19 + 8;
+			igt_assert(*str++ == '\n');
+		}
+
 		if (!(*str == ':' || *str == '~'))
 			continue;
 
-- 
2.25.1

[Intel-gfx] [PATCH v2 i-g-t 3/8] tests/i915/gem_exec_capture: Make the error decode a common helper

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

The decode of the error capture contents was happening in two
different sub-tests with two very different pieces of code. One being
much more extensive than the other (actually decodes and verifies the
contents of the captured buffers rather than just the address). So,
move the code into a common helper function and use that in both
places.

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
---
 tests/i915/gem_exec_capture.c | 344 +++++++++++++++++-----------------
 1 file changed, 170 insertions(+), 174 deletions(-)

diff --git a/tests/i915/gem_exec_capture.c b/tests/i915/gem_exec_capture.c
index 47ca64dd6..c85c198f7 100644
--- a/tests/i915/gem_exec_capture.c
+++ b/tests/i915/gem_exec_capture.c
@@ -33,32 +33,175 @@
 
 IGT_TEST_DESCRIPTION("Check that we capture the user specified objects on a hang");
 
-static void check_error_state(int dir, struct drm_i915_gem_exec_object2 *obj)
+struct offset {
+	uint64_t addr;
+	unsigned long idx;
+	bool found;
+};
+
+static unsigned long zlib_inflate(uint32_t **ptr, unsigned long len)
+{
+	struct z_stream_s zstream;
+	void *out;
+
+	memset(&zstream, 0, sizeof(zstream));
+
+	zstream.next_in = (unsigned char *)*ptr;
+	zstream.avail_in = 4*len;
+
+	if (inflateInit(&zstream) != Z_OK)
+		return 0;
+
+	out = malloc(128*4096); /* approximate obj size */
+	zstream.next_out = out;
+	zstream.avail_out = 128*4096;
+
+	do {
+		switch (inflate(&zstream, Z_SYNC_FLUSH)) {
+		case Z_STREAM_END:
+			goto end;
+		case Z_OK:
+			break;
+		default:
+			inflateEnd(&zstream);
+			return 0;
+		}
+
+		if (zstream.avail_out)
+			break;
+
+		out = realloc(out, 2*zstream.total_out);
+		if (out == NULL) {
+			inflateEnd(&zstream);
+			return 0;
+		}
+
+		zstream.next_out = (unsigned char *)out + zstream.total_out;
+		zstream.avail_out = zstream.total_out;
+	} while (1);
+end:
+	inflateEnd(&zstream);
+	free(*ptr);
+	*ptr = out;
+	return zstream.total_out / 4;
+}
+
+static unsigned long
+ascii85_decode(char *in, uint32_t **out, bool inflate, char **end)
+{
+	unsigned long len = 0, size = 1024;
+
+	*out = realloc(*out, sizeof(uint32_t)*size);
+	if (*out == NULL)
+		return 0;
+
+	while (*in >= '!' && *in <= 'z') {
+		uint32_t v = 0;
+
+		if (len == size) {
+			size *= 2;
+			*out = realloc(*out, sizeof(uint32_t)*size);
+			if (*out == NULL)
+				return 0;
+		}
+
+		if (*in == 'z') {
+			in++;
+		} else {
+			v += in[0] - 33; v *= 85;
+			v += in[1] - 33; v *= 85;
+			v += in[2] - 33; v *= 85;
+			v += in[3] - 33; v *= 85;
+			v += in[4] - 33;
+			in += 5;
+		}
+		(*out)[len++] = v;
+	}
+	*end = in;
+
+	if (!inflate)
+		return len;
+
+	return zlib_inflate(out, len);
+}
+
+static int check_error_state(int dir, struct offset *obj_offsets, int obj_count,
+			     uint64_t obj_size, bool incremental)
 {
 	char *error, *str;
-	bool found = false;
+	int blobs = 0;
 
 	error = igt_sysfs_get(dir, "error");
 	igt_sysfs_set(dir, "error", "Begone!");
-
 	igt_assert(error);
 	igt_debug("%s\n", error);
 
 	/* render ring --- user = 0x00000000 ffffd000 */
-	for (str = error; (str = strstr(str, "--- user = ")); str++) {
+	for (str = error; (str = strstr(str, "--- user = ")); ) {
+		uint32_t *data = NULL;
 		uint64_t addr;
-		uint32_t hi, lo;
+		unsigned long i, sz;
+		unsigned long start;
+		unsigned long end;
 
-		igt_assert(sscanf(str, "--- user = 0x%x %x", &hi, &lo) == 2);
-		addr = hi;
+		if (strncmp(str, "--- user = 0x", 13))
+			break;
+		str += 13;
+		addr = strtoul(str, &str, 16);
 		addr <<= 32;
-		addr |= lo;
-		igt_assert_eq_u64(addr, obj->offset);
-		found = true;
+		addr |= strtoul(str + 1, &str, 16);
+		igt_assert(*str++ == '\n');
+
+		start = 0;
+		end = obj_count;
+		while (end > start) {
+			i = (end - start) / 2 + start;
+			if (obj_offsets[i].addr < addr)
+				start = i + 1;
+			else if (obj_offsets[i].addr > addr)
+				end = i;
+			else
+				break;
+		}
+		igt_assert(obj_offsets[i].addr == addr);
+		igt_assert(!obj_offsets[i].found);
+		obj_offsets[i].found = true;
+		igt_debug("offset:%"PRIx64", index:%ld\n",
+			  addr, obj_offsets[i].idx);
+
+		/* gtt_page_sizes = 0x00010000 */
+		if (strncmp(str, "gtt_page_sizes = 0x", 19) == 0) {
+			str += 19 + 8;
+			igt_assert(*str++ == '\n');
+		}
+
+		if (!(*str == ':' || *str == '~'))
+			continue;
+
+		igt_debug("blob:%.64s\n", str);
+		sz = ascii85_decode(str + 1, &data, *str == ':', &str);
+
+		igt_assert_eq(4 * sz, obj_size);
+		igt_assert(*str++ == '\n');
+		str = strchr(str, '-');
+
+		if (incremental) {
+			uint32_t expect;
+
+			expect = obj_offsets[i].idx * obj_size;
+			for (i = 0; i < sz; i++)
+				igt_assert_eq(data[i], expect++);
+		} else {
+			for (i = 0; i < sz; i++)
+				igt_assert_eq(data[i], 0);
+		}
+
+		blobs++;
+		free(data);
 	}
 
 	free(error);
-	igt_assert(found);
+	return blobs;
 }
 
 static void __capture1(int fd, int dir, uint64_t ahnd, const intel_ctx_t *ctx,
@@ -73,6 +216,7 @@ static void __capture1(int fd, int dir, uint64_t ahnd, const intel_ctx_t *ctx,
 	struct drm_i915_gem_relocation_entry reloc[2];
 	struct drm_i915_gem_execbuffer2 execbuf;
 	uint32_t *batch, *seqno;
+	struct offset offset;
 	int i;
 
 	memset(obj, 0, sizeof(obj));
@@ -168,7 +312,10 @@ static void __capture1(int fd, int dir, uint64_t ahnd, const intel_ctx_t *ctx,
 
 	/* Check that only the buffer we marked is reported in the error */
 	igt_force_gpu_reset(fd);
-	check_error_state(dir, &obj[CAPTURE]);
+	memset(&offset, 0, sizeof(offset));
+	offset.addr = obj[CAPTURE].offset;
+	igt_assert_eq(check_error_state(dir, &offset, 1, target_size, false), 1);
+	igt_assert(offset.found);
 
 	gem_sync(fd, obj[BATCH].handle);
 
@@ -183,11 +330,12 @@ static void capture(int fd, int dir, const intel_ctx_t *ctx, unsigned ring)
 {
 	uint32_t handle;
 	uint64_t ahnd;
+	int obj_size = 4096;
 
-	handle = gem_create(fd, 4096);
+	handle = gem_create(fd, obj_size);
 	ahnd = get_reloc_ahnd(fd, ctx->id);
 
-	__capture1(fd, dir, ahnd, ctx, ring, handle, 4096);
+	__capture1(fd, dir, ahnd, ctx, ring, handle, obj_size);
 
 	gem_close(fd, handle);
 	put_ahnd(ahnd);
@@ -206,10 +354,8 @@ static int cmp(const void *A, const void *B)
 	return 0;
 }
 
-static struct offset {
-	uint64_t addr;
-	unsigned long idx;
-} *__captureN(int fd, int dir, uint64_t ahnd, unsigned ring,
+static struct offset *
+__captureN(int fd, int dir, uint64_t ahnd, unsigned ring,
 	      unsigned int size, int count,
 	      unsigned int flags)
 #define INCREMENTAL 0x1
@@ -357,98 +503,11 @@ static struct offset {
 	return offsets;
 }
 
-static unsigned long zlib_inflate(uint32_t **ptr, unsigned long len)
-{
-	struct z_stream_s zstream;
-	void *out;
-
-	memset(&zstream, 0, sizeof(zstream));
-
-	zstream.next_in = (unsigned char *)*ptr;
-	zstream.avail_in = 4*len;
-
-	if (inflateInit(&zstream) != Z_OK)
-		return 0;
-
-	out = malloc(128*4096); /* approximate obj size */
-	zstream.next_out = out;
-	zstream.avail_out = 128*4096;
-
-	do {
-		switch (inflate(&zstream, Z_SYNC_FLUSH)) {
-		case Z_STREAM_END:
-			goto end;
-		case Z_OK:
-			break;
-		default:
-			inflateEnd(&zstream);
-			return 0;
-		}
-
-		if (zstream.avail_out)
-			break;
-
-		out = realloc(out, 2*zstream.total_out);
-		if (out == NULL) {
-			inflateEnd(&zstream);
-			return 0;
-		}
-
-		zstream.next_out = (unsigned char *)out + zstream.total_out;
-		zstream.avail_out = zstream.total_out;
-	} while (1);
-end:
-	inflateEnd(&zstream);
-	free(*ptr);
-	*ptr = out;
-	return zstream.total_out / 4;
-}
-
-static unsigned long
-ascii85_decode(char *in, uint32_t **out, bool inflate, char **end)
-{
-	unsigned long len = 0, size = 1024;
-
-	*out = realloc(*out, sizeof(uint32_t)*size);
-	if (*out == NULL)
-		return 0;
-
-	while (*in >= '!' && *in <= 'z') {
-		uint32_t v = 0;
-
-		if (len == size) {
-			size *= 2;
-			*out = realloc(*out, sizeof(uint32_t)*size);
-			if (*out == NULL)
-				return 0;
-		}
-
-		if (*in == 'z') {
-			in++;
-		} else {
-			v += in[0] - 33; v *= 85;
-			v += in[1] - 33; v *= 85;
-			v += in[2] - 33; v *= 85;
-			v += in[3] - 33; v *= 85;
-			v += in[4] - 33;
-			in += 5;
-		}
-		(*out)[len++] = v;
-	}
-	*end = in;
-
-	if (!inflate)
-		return len;
-
-	return zlib_inflate(out, len);
-}
-
 static void many(int fd, int dir, uint64_t size, unsigned int flags)
 {
 	uint64_t ram, gtt, ahnd;
 	unsigned long count, blobs;
 	struct offset *offsets;
-	char *error, *str;
 
 	gtt = gem_aperture_size(fd) / size;
 	ram = (intel_get_avail_ram_mb() << 20) / size;
@@ -463,75 +522,10 @@ static void many(int fd, int dir, uint64_t size, unsigned int flags)
 
 	offsets = __captureN(fd, dir, ahnd, 0, size, count, flags);
 
-	error = igt_sysfs_get(dir, "error");
-	igt_sysfs_set(dir, "error", "Begone!");
-	igt_assert(error);
-
-	blobs = 0;
-	/* render ring --- user = 0x00000000 ffffd000 */
-	str = strstr(error, "--- user = ");
-	while (str) {
-		uint32_t *data = NULL;
-		unsigned long i, sz;
-		uint64_t addr;
-
-		if (strncmp(str, "--- user = 0x", 13))
-			break;
-
-		str += 13;
-		addr = strtoul(str, &str, 16);
-		addr <<= 32;
-		addr |= strtoul(str + 1, &str, 16);
-		igt_assert(*str++ == '\n');
-
-		/* gtt_page_sizes = 0x00010000 */
-		if (strncmp(str, "gtt_page_sizes = 0x", 19) == 0) {
-			str += 19 + 8;
-			igt_assert(*str++ == '\n');
-		}
-
-		if (!(*str == ':' || *str == '~'))
-			continue;
-
-		igt_debug("blob:%.64s\n", str);
-		sz = ascii85_decode(str + 1, &data, *str == ':', &str);
-		igt_assert_eq(4 * sz, size);
-		igt_assert(*str++ == '\n');
-		str = strchr(str, '-');
-
-		if (flags & INCREMENTAL) {
-			unsigned long start = 0;
-			unsigned long end = count;
-			uint32_t expect;
-
-			while (end > start) {
-				i = (end - start) / 2 + start;
-				if (offsets[i].addr < addr)
-					start = i + 1;
-				else if (offsets[i].addr > addr)
-					end = i;
-				else
-					break;
-			}
-			igt_assert(offsets[i].addr == addr);
-			igt_debug("offset:%"PRIx64", index:%ld\n",
-				  addr, offsets[i].idx);
-
-			expect = offsets[i].idx * size;
-			for (i = 0; i < sz; i++)
-				igt_assert_eq(data[i], expect++);
-		} else {
-			for (i = 0; i < sz; i++)
-				igt_assert_eq(data[i], 0);
-		}
-
-		blobs++;
-		free(data);
-	}
+	blobs = check_error_state(dir, offsets, count, size, !!(flags & INCREMENTAL));
 	igt_info("Captured %lu %"PRId64"-blobs out of a total of %lu\n",
 		 blobs, size >> 12, count);
 
-	free(error);
 	free(offsets);
 	put_ahnd(ahnd);
 }
@@ -625,12 +619,14 @@ static void userptr(int fd, int dir)
 	uint32_t handle;
 	uint64_t ahnd;
 	void *ptr;
+	int obj_size = 4096;
 
-	igt_assert(posix_memalign(&ptr, 4096, 4096) == 0);
-	igt_require(__gem_userptr(fd, ptr, 4096, 0, 0, &handle) == 0);
+	igt_assert(posix_memalign(&ptr, obj_size, obj_size) == 0);
+	memset(ptr, 0, obj_size);
+	igt_require(__gem_userptr(fd, ptr, obj_size, 0, 0, &handle) == 0);
 	ahnd = get_reloc_ahnd(fd, ctx->id);
 
-	__capture1(fd, dir, ahnd, intel_ctx_0(fd), 0, handle, 4096);
+	__capture1(fd, dir, ahnd, intel_ctx_0(fd), 0, handle, obj_size);
 
 	gem_close(fd, handle);
 	put_ahnd(ahnd);
-- 
2.25.1

[Intel-gfx] [PATCH v2 i-g-t 4/8] tests/i915/gem_exec_capture: Use contexts and engines properly

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

Some of the capture tests were using explicit contexts, some not. Some
were poking the per engine pre-emption timeout, some not. This would
lead to sporadic failures due to random timeouts, contexts being
banned depending upon how many subtests were run and/or how many
engines a given platform has, and other such failures.

So, update all tests to be conistent.

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 tests/i915/gem_exec_capture.c | 79 +++++++++++++++++++++++++----------
 1 file changed, 57 insertions(+), 22 deletions(-)

diff --git a/tests/i915/gem_exec_capture.c b/tests/i915/gem_exec_capture.c
index c85c198f7..11c348d3b 100644
--- a/tests/i915/gem_exec_capture.c
+++ b/tests/i915/gem_exec_capture.c
@@ -204,8 +204,19 @@ static int check_error_state(int dir, struct offset *obj_offsets, int obj_count,
 	return blobs;
 }
 
+static void configure_hangs(int fd, const struct intel_execution_engine2 *e, int ctxt_id)
+{
+	/* Ensure fast hang detection */
+	gem_engine_property_printf(fd, e->name, "preempt_timeout_ms", "%d", 250);
+	gem_engine_property_printf(fd, e->name, "heartbeat_interval_ms", "%d", 500);
+
+	/* Allow engine based resets and disable banning */
+	igt_allow_hang(fd, ctxt_id, HANG_ALLOW_CAPTURE);
+}
+
 static void __capture1(int fd, int dir, uint64_t ahnd, const intel_ctx_t *ctx,
-		       unsigned ring, uint32_t target, uint64_t target_size)
+		       const struct intel_execution_engine2 *e,
+		       uint32_t target, uint64_t target_size)
 {
 	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
 	struct drm_i915_gem_exec_object2 obj[4];
@@ -219,6 +230,8 @@ static void __capture1(int fd, int dir, uint64_t ahnd, const intel_ctx_t *ctx,
 	struct offset offset;
 	int i;
 
+	configure_hangs(fd, e, ctx->id);
+
 	memset(obj, 0, sizeof(obj));
 	obj[SCRATCH].handle = gem_create(fd, 4096);
 	obj[SCRATCH].flags = EXEC_OBJECT_WRITE;
@@ -297,7 +310,7 @@ static void __capture1(int fd, int dir, uint64_t ahnd, const intel_ctx_t *ctx,
 	memset(&execbuf, 0, sizeof(execbuf));
 	execbuf.buffers_ptr = (uintptr_t)obj;
 	execbuf.buffer_count = ARRAY_SIZE(obj);
-	execbuf.flags = ring;
+	execbuf.flags = e->flags;
 	if (gen > 3 && gen < 6)
 		execbuf.flags |= I915_EXEC_SECURE;
 	execbuf.rsvd1 = ctx->id;
@@ -326,7 +339,8 @@ static void __capture1(int fd, int dir, uint64_t ahnd, const intel_ctx_t *ctx,
 	gem_close(fd, obj[SCRATCH].handle);
 }
 
-static void capture(int fd, int dir, const intel_ctx_t *ctx, unsigned ring)
+static void capture(int fd, int dir, const intel_ctx_t *ctx,
+		    const struct intel_execution_engine2 *e)
 {
 	uint32_t handle;
 	uint64_t ahnd;
@@ -335,7 +349,7 @@ static void capture(int fd, int dir, const intel_ctx_t *ctx, unsigned ring)
 	handle = gem_create(fd, obj_size);
 	ahnd = get_reloc_ahnd(fd, ctx->id);
 
-	__capture1(fd, dir, ahnd, ctx, ring, handle, obj_size);
+	__capture1(fd, dir, ahnd, ctx, e, handle, obj_size);
 
 	gem_close(fd, handle);
 	put_ahnd(ahnd);
@@ -355,9 +369,9 @@ static int cmp(const void *A, const void *B)
 }
 
 static struct offset *
-__captureN(int fd, int dir, uint64_t ahnd, unsigned ring,
-	      unsigned int size, int count,
-	      unsigned int flags)
+__captureN(int fd, int dir, uint64_t ahnd, const intel_ctx_t *ctx,
+	   const struct intel_execution_engine2 *e,
+	   unsigned int size, int count, unsigned int flags)
 #define INCREMENTAL 0x1
 #define ASYNC 0x2
 {
@@ -369,6 +383,8 @@ __captureN(int fd, int dir, uint64_t ahnd, unsigned ring,
 	struct offset *offsets;
 	int i;
 
+	configure_hangs(fd, e, ctx->id);
+
 	offsets = calloc(count, sizeof(*offsets));
 	igt_assert(offsets);
 
@@ -470,9 +486,10 @@ __captureN(int fd, int dir, uint64_t ahnd, unsigned ring,
 	memset(&execbuf, 0, sizeof(execbuf));
 	execbuf.buffers_ptr = (uintptr_t)obj;
 	execbuf.buffer_count = count + 2;
-	execbuf.flags = ring;
+	execbuf.flags = e->flags;
 	if (gen > 3 && gen < 6)
 		execbuf.flags |= I915_EXEC_SECURE;
+	execbuf.rsvd1 = ctx->id;
 
 	igt_assert(!READ_ONCE(*seqno));
 	gem_execbuf(fd, &execbuf);
@@ -503,12 +520,27 @@ __captureN(int fd, int dir, uint64_t ahnd, unsigned ring,
 	return offsets;
 }
 
+#define find_first_available_engine(fd, ctx, e) \
+	do { \
+		ctx = intel_ctx_create_all_physical(fd); \
+		igt_assert(ctx); \
+		for_each_ctx_engine(fd, ctx, e) \
+			for_each_if(gem_class_can_store_dword(fd, e->class)) \
+				break; \
+		igt_assert(e); \
+		configure_hangs(fd, e, ctx->id); \
+	} while(0)
+
 static void many(int fd, int dir, uint64_t size, unsigned int flags)
 {
+	const struct intel_execution_engine2 *e;
+	const intel_ctx_t *ctx;
 	uint64_t ram, gtt, ahnd;
 	unsigned long count, blobs;
 	struct offset *offsets;
 
+	find_first_available_engine(fd, ctx, e);
+
 	gtt = gem_aperture_size(fd) / size;
 	ram = (intel_get_avail_ram_mb() << 20) / size;
 	igt_debug("Available objects in GTT:%"PRIu64", RAM:%"PRIu64"\n",
@@ -518,9 +550,9 @@ static void many(int fd, int dir, uint64_t size, unsigned int flags)
 	igt_require(count > 1);
 
 	intel_require_memory(count, size, CHECK_RAM);
-	ahnd = get_reloc_ahnd(fd, 0);
+	ahnd = get_reloc_ahnd(fd, ctx->id);
 
-	offsets = __captureN(fd, dir, ahnd, 0, size, count, flags);
+	offsets = __captureN(fd, dir, ahnd, ctx, e, size, count, flags);
 
 	blobs = check_error_state(dir, offsets, count, size, !!(flags & INCREMENTAL));
 	igt_info("Captured %lu %"PRId64"-blobs out of a total of %lu\n",
@@ -531,7 +563,7 @@ static void many(int fd, int dir, uint64_t size, unsigned int flags)
 }
 
 static void prioinv(int fd, int dir, const intel_ctx_t *ctx,
-		    unsigned ring, const char *name)
+		    const struct intel_execution_engine2 *e)
 {
 	const uint32_t bbe = MI_BATCH_BUFFER_END;
 	struct drm_i915_gem_exec_object2 obj = {
@@ -540,7 +572,7 @@ static void prioinv(int fd, int dir, const intel_ctx_t *ctx,
 	struct drm_i915_gem_execbuffer2 execbuf = {
 		.buffers_ptr = to_user_pointer(&obj),
 		.buffer_count = 1,
-		.flags = ring,
+		.flags = e->flags,
 		.rsvd1 = ctx->id,
 	};
 	int64_t timeout = NSEC_PER_SEC; /* 1s, feeling generous, blame debug */
@@ -555,10 +587,6 @@ static void prioinv(int fd, int dir, const intel_ctx_t *ctx,
 	igt_require(igt_params_set(fd, "reset", "%u", -1)); /* engine resets! */
 	igt_require(gem_gpu_reset_type(fd) > 1);
 
-	/* Needs to be fast enough for the hangcheck to return within 1s */
-	igt_require(gem_engine_property_printf(fd, name, "preempt_timeout_ms", "%d", 0) > 0);
-	gem_engine_property_printf(fd, name, "preempt_timeout_ms", "%d", 500);
-
 	gtt = gem_aperture_size(fd) / size;
 	ram = (intel_get_avail_ram_mb() << 20) / size;
 	igt_debug("Available objects in GTT:%"PRIu64", RAM:%"PRIu64"\n",
@@ -576,15 +604,19 @@ static void prioinv(int fd, int dir, const intel_ctx_t *ctx,
 
 	igt_assert(pipe(link) == 0);
 	igt_fork(child, 1) {
+		const intel_ctx_t *ctx2;
 		fd = gem_reopen_driver(fd);
 		igt_debug("Submitting large capture [%ld x %dMiB objects]\n",
 			  count, (int)(size >> 20));
 
+		ctx2 = intel_ctx_create_all_physical(fd);
+		igt_assert(ctx2);
+
 		intel_allocator_init();
 		/* Reopen the allocator in the new process. */
-		ahnd = get_reloc_ahnd(fd, 0);
+		ahnd = get_reloc_ahnd(fd, ctx2->id);
 
-		free(__captureN(fd, dir, ahnd, ring, size, count, ASYNC));
+		free(__captureN(fd, dir, ahnd, ctx2, e, size, count, ASYNC));
 		put_ahnd(ahnd);
 
 		write(link[1], &fd, sizeof(fd)); /* wake the parent up */
@@ -615,18 +647,21 @@ static void prioinv(int fd, int dir, const intel_ctx_t *ctx,
 
 static void userptr(int fd, int dir)
 {
-	const intel_ctx_t *ctx = intel_ctx_0(fd);
+	const struct intel_execution_engine2 *e;
+	const intel_ctx_t *ctx;
 	uint32_t handle;
 	uint64_t ahnd;
 	void *ptr;
 	int obj_size = 4096;
 
+	find_first_available_engine(fd, ctx, e);
+
 	igt_assert(posix_memalign(&ptr, obj_size, obj_size) == 0);
 	memset(ptr, 0, obj_size);
 	igt_require(__gem_userptr(fd, ptr, obj_size, 0, 0, &handle) == 0);
 	ahnd = get_reloc_ahnd(fd, ctx->id);
 
-	__capture1(fd, dir, ahnd, intel_ctx_0(fd), 0, handle, obj_size);
+	__capture1(fd, dir, ahnd, ctx, e, handle, obj_size);
 
 	gem_close(fd, handle);
 	put_ahnd(ahnd);
@@ -684,7 +719,7 @@ igt_main
 	}
 
 	test_each_engine("capture", fd, ctx, e)
-		capture(fd, dir, ctx, e->flags);
+		capture(fd, dir, ctx, e);
 
 	igt_subtest_f("many-4K-zero") {
 		igt_require(gem_can_store_dword(fd, 0));
@@ -719,7 +754,7 @@ igt_main
 	}
 
 	test_each_engine("pi", fd, ctx, e)
-		prioinv(fd, dir, ctx, e->flags, e->name);
+		prioinv(fd, dir, ctx, e);
 
 	igt_fixture {
 		close(dir);
-- 
2.25.1

Re: [Intel-gfx] [PATCH v2 i-g-t 4/8] tests/i915/gem_exec_capture: Use contexts and engines properly

[Matthew Brost]

On Wed, Nov 03, 2021 at 10:04:45AM -0700, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
> 
> Some of the capture tests were using explicit contexts, some not. Some
> were poking the per engine pre-emption timeout, some not. This would
> lead to sporadic failures due to random timeouts, contexts being
> banned depending upon how many subtests were run and/or how many
> engines a given platform has, and other such failures.
> 
> So, update all tests to be conistent.
> 
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>

Reviewed-by: Matthew Brost <matthew.brost@intel.com>

> ---
>  tests/i915/gem_exec_capture.c | 79 +++++++++++++++++++++++++----------
>  1 file changed, 57 insertions(+), 22 deletions(-)
> 
> diff --git a/tests/i915/gem_exec_capture.c b/tests/i915/gem_exec_capture.c
> index c85c198f7..11c348d3b 100644
> --- a/tests/i915/gem_exec_capture.c
> +++ b/tests/i915/gem_exec_capture.c
> @@ -204,8 +204,19 @@ static int check_error_state(int dir, struct offset *obj_offsets, int obj_count,
>  	return blobs;
>  }
>  
> +static void configure_hangs(int fd, const struct intel_execution_engine2 *e, int ctxt_id)
> +{
> +	/* Ensure fast hang detection */
> +	gem_engine_property_printf(fd, e->name, "preempt_timeout_ms", "%d", 250);
> +	gem_engine_property_printf(fd, e->name, "heartbeat_interval_ms", "%d", 500);
> +
> +	/* Allow engine based resets and disable banning */
> +	igt_allow_hang(fd, ctxt_id, HANG_ALLOW_CAPTURE);
> +}
> +
>  static void __capture1(int fd, int dir, uint64_t ahnd, const intel_ctx_t *ctx,
> -		       unsigned ring, uint32_t target, uint64_t target_size)
> +		       const struct intel_execution_engine2 *e,
> +		       uint32_t target, uint64_t target_size)
>  {
>  	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
>  	struct drm_i915_gem_exec_object2 obj[4];
> @@ -219,6 +230,8 @@ static void __capture1(int fd, int dir, uint64_t ahnd, const intel_ctx_t *ctx,
>  	struct offset offset;
>  	int i;
>  
> +	configure_hangs(fd, e, ctx->id);
> +
>  	memset(obj, 0, sizeof(obj));
>  	obj[SCRATCH].handle = gem_create(fd, 4096);
>  	obj[SCRATCH].flags = EXEC_OBJECT_WRITE;
> @@ -297,7 +310,7 @@ static void __capture1(int fd, int dir, uint64_t ahnd, const intel_ctx_t *ctx,
>  	memset(&execbuf, 0, sizeof(execbuf));
>  	execbuf.buffers_ptr = (uintptr_t)obj;
>  	execbuf.buffer_count = ARRAY_SIZE(obj);
> -	execbuf.flags = ring;
> +	execbuf.flags = e->flags;
>  	if (gen > 3 && gen < 6)
>  		execbuf.flags |= I915_EXEC_SECURE;
>  	execbuf.rsvd1 = ctx->id;
> @@ -326,7 +339,8 @@ static void __capture1(int fd, int dir, uint64_t ahnd, const intel_ctx_t *ctx,
>  	gem_close(fd, obj[SCRATCH].handle);
>  }
>  
> -static void capture(int fd, int dir, const intel_ctx_t *ctx, unsigned ring)
> +static void capture(int fd, int dir, const intel_ctx_t *ctx,
> +		    const struct intel_execution_engine2 *e)
>  {
>  	uint32_t handle;
>  	uint64_t ahnd;
> @@ -335,7 +349,7 @@ static void capture(int fd, int dir, const intel_ctx_t *ctx, unsigned ring)
>  	handle = gem_create(fd, obj_size);
>  	ahnd = get_reloc_ahnd(fd, ctx->id);
>  
> -	__capture1(fd, dir, ahnd, ctx, ring, handle, obj_size);
> +	__capture1(fd, dir, ahnd, ctx, e, handle, obj_size);
>  
>  	gem_close(fd, handle);
>  	put_ahnd(ahnd);
> @@ -355,9 +369,9 @@ static int cmp(const void *A, const void *B)
>  }
>  
>  static struct offset *
> -__captureN(int fd, int dir, uint64_t ahnd, unsigned ring,
> -	      unsigned int size, int count,
> -	      unsigned int flags)
> +__captureN(int fd, int dir, uint64_t ahnd, const intel_ctx_t *ctx,
> +	   const struct intel_execution_engine2 *e,
> +	   unsigned int size, int count, unsigned int flags)
>  #define INCREMENTAL 0x1
>  #define ASYNC 0x2
>  {
> @@ -369,6 +383,8 @@ __captureN(int fd, int dir, uint64_t ahnd, unsigned ring,
>  	struct offset *offsets;
>  	int i;
>  
> +	configure_hangs(fd, e, ctx->id);
> +
>  	offsets = calloc(count, sizeof(*offsets));
>  	igt_assert(offsets);
>  
> @@ -470,9 +486,10 @@ __captureN(int fd, int dir, uint64_t ahnd, unsigned ring,
>  	memset(&execbuf, 0, sizeof(execbuf));
>  	execbuf.buffers_ptr = (uintptr_t)obj;
>  	execbuf.buffer_count = count + 2;
> -	execbuf.flags = ring;
> +	execbuf.flags = e->flags;
>  	if (gen > 3 && gen < 6)
>  		execbuf.flags |= I915_EXEC_SECURE;
> +	execbuf.rsvd1 = ctx->id;
>  
>  	igt_assert(!READ_ONCE(*seqno));
>  	gem_execbuf(fd, &execbuf);
> @@ -503,12 +520,27 @@ __captureN(int fd, int dir, uint64_t ahnd, unsigned ring,
>  	return offsets;
>  }
>  
> +#define find_first_available_engine(fd, ctx, e) \
> +	do { \
> +		ctx = intel_ctx_create_all_physical(fd); \
> +		igt_assert(ctx); \
> +		for_each_ctx_engine(fd, ctx, e) \
> +			for_each_if(gem_class_can_store_dword(fd, e->class)) \
> +				break; \
> +		igt_assert(e); \
> +		configure_hangs(fd, e, ctx->id); \
> +	} while(0)
> +
>  static void many(int fd, int dir, uint64_t size, unsigned int flags)
>  {
> +	const struct intel_execution_engine2 *e;
> +	const intel_ctx_t *ctx;
>  	uint64_t ram, gtt, ahnd;
>  	unsigned long count, blobs;
>  	struct offset *offsets;
>  
> +	find_first_available_engine(fd, ctx, e);
> +
>  	gtt = gem_aperture_size(fd) / size;
>  	ram = (intel_get_avail_ram_mb() << 20) / size;
>  	igt_debug("Available objects in GTT:%"PRIu64", RAM:%"PRIu64"\n",
> @@ -518,9 +550,9 @@ static void many(int fd, int dir, uint64_t size, unsigned int flags)
>  	igt_require(count > 1);
>  
>  	intel_require_memory(count, size, CHECK_RAM);
> -	ahnd = get_reloc_ahnd(fd, 0);
> +	ahnd = get_reloc_ahnd(fd, ctx->id);
>  
> -	offsets = __captureN(fd, dir, ahnd, 0, size, count, flags);
> +	offsets = __captureN(fd, dir, ahnd, ctx, e, size, count, flags);
>  
>  	blobs = check_error_state(dir, offsets, count, size, !!(flags & INCREMENTAL));
>  	igt_info("Captured %lu %"PRId64"-blobs out of a total of %lu\n",
> @@ -531,7 +563,7 @@ static void many(int fd, int dir, uint64_t size, unsigned int flags)
>  }
>  
>  static void prioinv(int fd, int dir, const intel_ctx_t *ctx,
> -		    unsigned ring, const char *name)
> +		    const struct intel_execution_engine2 *e)
>  {
>  	const uint32_t bbe = MI_BATCH_BUFFER_END;
>  	struct drm_i915_gem_exec_object2 obj = {
> @@ -540,7 +572,7 @@ static void prioinv(int fd, int dir, const intel_ctx_t *ctx,
>  	struct drm_i915_gem_execbuffer2 execbuf = {
>  		.buffers_ptr = to_user_pointer(&obj),
>  		.buffer_count = 1,
> -		.flags = ring,
> +		.flags = e->flags,
>  		.rsvd1 = ctx->id,
>  	};
>  	int64_t timeout = NSEC_PER_SEC; /* 1s, feeling generous, blame debug */
> @@ -555,10 +587,6 @@ static void prioinv(int fd, int dir, const intel_ctx_t *ctx,
>  	igt_require(igt_params_set(fd, "reset", "%u", -1)); /* engine resets! */
>  	igt_require(gem_gpu_reset_type(fd) > 1);
>  
> -	/* Needs to be fast enough for the hangcheck to return within 1s */
> -	igt_require(gem_engine_property_printf(fd, name, "preempt_timeout_ms", "%d", 0) > 0);
> -	gem_engine_property_printf(fd, name, "preempt_timeout_ms", "%d", 500);
> -
>  	gtt = gem_aperture_size(fd) / size;
>  	ram = (intel_get_avail_ram_mb() << 20) / size;
>  	igt_debug("Available objects in GTT:%"PRIu64", RAM:%"PRIu64"\n",
> @@ -576,15 +604,19 @@ static void prioinv(int fd, int dir, const intel_ctx_t *ctx,
>  
>  	igt_assert(pipe(link) == 0);
>  	igt_fork(child, 1) {
> +		const intel_ctx_t *ctx2;
>  		fd = gem_reopen_driver(fd);
>  		igt_debug("Submitting large capture [%ld x %dMiB objects]\n",
>  			  count, (int)(size >> 20));
>  
> +		ctx2 = intel_ctx_create_all_physical(fd);
> +		igt_assert(ctx2);
> +
>  		intel_allocator_init();
>  		/* Reopen the allocator in the new process. */
> -		ahnd = get_reloc_ahnd(fd, 0);
> +		ahnd = get_reloc_ahnd(fd, ctx2->id);
>  
> -		free(__captureN(fd, dir, ahnd, ring, size, count, ASYNC));
> +		free(__captureN(fd, dir, ahnd, ctx2, e, size, count, ASYNC));
>  		put_ahnd(ahnd);
>  
>  		write(link[1], &fd, sizeof(fd)); /* wake the parent up */
> @@ -615,18 +647,21 @@ static void prioinv(int fd, int dir, const intel_ctx_t *ctx,
>  
>  static void userptr(int fd, int dir)
>  {
> -	const intel_ctx_t *ctx = intel_ctx_0(fd);
> +	const struct intel_execution_engine2 *e;
> +	const intel_ctx_t *ctx;
>  	uint32_t handle;
>  	uint64_t ahnd;
>  	void *ptr;
>  	int obj_size = 4096;
>  
> +	find_first_available_engine(fd, ctx, e);
> +
>  	igt_assert(posix_memalign(&ptr, obj_size, obj_size) == 0);
>  	memset(ptr, 0, obj_size);
>  	igt_require(__gem_userptr(fd, ptr, obj_size, 0, 0, &handle) == 0);
>  	ahnd = get_reloc_ahnd(fd, ctx->id);
>  
> -	__capture1(fd, dir, ahnd, intel_ctx_0(fd), 0, handle, obj_size);
> +	__capture1(fd, dir, ahnd, ctx, e, handle, obj_size);
>  
>  	gem_close(fd, handle);
>  	put_ahnd(ahnd);
> @@ -684,7 +719,7 @@ igt_main
>  	}
>  
>  	test_each_engine("capture", fd, ctx, e)
> -		capture(fd, dir, ctx, e->flags);
> +		capture(fd, dir, ctx, e);
>  
>  	igt_subtest_f("many-4K-zero") {
>  		igt_require(gem_can_store_dword(fd, 0));
> @@ -719,7 +754,7 @@ igt_main
>  	}
>  
>  	test_each_engine("pi", fd, ctx, e)
> -		prioinv(fd, dir, ctx, e->flags, e->name);
> +		prioinv(fd, dir, ctx, e);
>  
>  	igt_fixture {
>  		close(dir);
> -- 
> 2.25.1
> 

[Intel-gfx] [PATCH v2 i-g-t 5/8] tests/i915/gem_exec_capture: Check for memory allocation failure

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

The sysfs file read helper does not actually report any errors if a
realloc fails. It just silently returns a 'valid' but truncated
buffer. This then leads to the decode of the buffer failing in random
ways. So, add a check for ENOMEM being generated during the read.

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
---
 tests/i915/gem_exec_capture.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/i915/gem_exec_capture.c b/tests/i915/gem_exec_capture.c
index 11c348d3b..e924d0a30 100644
--- a/tests/i915/gem_exec_capture.c
+++ b/tests/i915/gem_exec_capture.c
@@ -131,9 +131,11 @@ static int check_error_state(int dir, struct offset *obj_offsets, int obj_count,
 	char *error, *str;
 	int blobs = 0;
 
+	errno = 0;
 	error = igt_sysfs_get(dir, "error");
 	igt_sysfs_set(dir, "error", "Begone!");
 	igt_assert(error);
+	igt_assert(errno != ENOMEM);
 	igt_debug("%s\n", error);
 
 	/* render ring --- user = 0x00000000 ffffd000 */
-- 
2.25.1

[Intel-gfx] [PATCH v2 i-g-t 6/8] lib/igt_sysfs: Support large files

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

The syfs helper functions were all using basic 'int' data types for
sizs, offsets, etc. when reading from sysfs. This works fine for
little files, but not for large error capture logs (which can be
gigabytes in sizes).

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
---
 lib/igt_sysfs.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
index 6919ac361..ee75e3ef1 100644
--- a/lib/igt_sysfs.c
+++ b/lib/igt_sysfs.c
@@ -53,9 +53,11 @@
  * provides basic support for like igt_sysfs_open().
  */
 
-static int readN(int fd, char *buf, int len)
+static ssize_t readN(int fd, char *buf, size_t len)
 {
-	int ret, total = 0;
+	ssize_t ret;
+	size_t total = 0;
+
 	do {
 		ret = read(fd, buf + total, len - total);
 		if (ret < 0)
@@ -69,9 +71,11 @@ static int readN(int fd, char *buf, int len)
 	return total ?: ret;
 }
 
-static int writeN(int fd, const char *buf, int len)
+static ssize_t writeN(int fd, const char *buf, size_t len)
 {
-	int ret, total = 0;
+	ssize_t ret;
+	size_t total = 0;
+
 	do {
 		ret = write(fd, buf + total, len - total);
 		if (ret < 0)
@@ -218,8 +222,9 @@ bool igt_sysfs_set(int dir, const char *attr, const char *value)
 char *igt_sysfs_get(int dir, const char *attr)
 {
 	char *buf;
-	int len, offset, rem;
-	int ret, fd;
+	size_t len, offset, rem;
+	ssize_t ret;
+	int fd;
 
 	fd = openat(dir, attr, O_RDONLY);
 	if (igt_debug_on(fd < 0))
-- 
2.25.1

[Intel-gfx] [PATCH v2 i-g-t 7/8] lib/igt_gt: Allow per engine reset testing

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

With GuC submission, engine resets are handled entirely within GuC
rather than within i915. Traditionally, IGT has disallowed engine
based resets becuase they don't send the uevent which IGT uses to
check for unexpected resets. However, it is important to be able to
test all reset mechanisms that can be used, so allow engine based
resets to be enabled.

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
---
 lib/igt_gt.c | 44 +++++++++++++++++++++++++++++---------------
 lib/igt_gt.h |  1 +
 2 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/lib/igt_gt.c b/lib/igt_gt.c
index a0ba04cc1..7c7df95ee 100644
--- a/lib/igt_gt.c
+++ b/lib/igt_gt.c
@@ -56,23 +56,28 @@
  * engines.
  */
 
+static int reset_query_once = -1;
+
 static bool has_gpu_reset(int fd)
 {
-	static int once = -1;
-	if (once < 0) {
-		struct drm_i915_getparam gp;
-		int val = 0;
-
-		memset(&gp, 0, sizeof(gp));
-		gp.param = 35; /* HAS_GPU_RESET */
-		gp.value = &val;
-
-		if (ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp))
-			once = intel_gen(intel_get_drm_devid(fd)) >= 5;
-		else
-			once = val > 0;
+	if (reset_query_once < 0) {
+		reset_query_once = gem_gpu_reset_type(fd);
+
+		/* Very old kernels did not support the query */
+		if (reset_query_once == -1)
+			reset_query_once =
+			      (intel_gen(intel_get_drm_devid(fd)) >= 5) ? 1 : 0;
 	}
-	return once;
+
+	return reset_query_once > 0;
+}
+
+static bool has_engine_reset(int fd)
+{
+	if (reset_query_once < 0)
+		has_gpu_reset(fd);
+
+	return reset_query_once > 1;
 }
 
 static void eat_error_state(int dev)
@@ -176,7 +181,11 @@ igt_hang_t igt_allow_hang(int fd, unsigned ctx, unsigned flags)
 		igt_skip("hang injection disabled by user [IGT_HANG=0]\n");
 	gem_context_require_bannable(fd);
 
-	allow_reset = 1;
+	if (flags & HANG_WANT_ENGINE_RESET)
+		allow_reset = 2;
+	else
+		allow_reset = 1;
+
 	if ((flags & HANG_ALLOW_CAPTURE) == 0) {
 		param.param = I915_CONTEXT_PARAM_NO_ERROR_CAPTURE;
 		param.value = 1;
@@ -187,11 +196,16 @@ igt_hang_t igt_allow_hang(int fd, unsigned ctx, unsigned flags)
 		__gem_context_set_param(fd, &param);
 		allow_reset = INT_MAX; /* any reset method */
 	}
+
 	igt_require(igt_params_set(fd, "reset", "%d", allow_reset));
+	reset_query_once = -1;  /* Re-query after changing param */
 
 	if (!igt_check_boolean_env_var("IGT_HANG_WITHOUT_RESET", false))
 		igt_require(has_gpu_reset(fd));
 
+	if (flags & HANG_WANT_ENGINE_RESET)
+		igt_require(has_engine_reset(fd));
+
 	ban = context_get_ban(fd, ctx);
 	if ((flags & HANG_ALLOW_BAN) == 0)
 		context_set_ban(fd, ctx, 0);
diff --git a/lib/igt_gt.h b/lib/igt_gt.h
index ceb044b86..c5059817b 100644
--- a/lib/igt_gt.h
+++ b/lib/igt_gt.h
@@ -51,6 +51,7 @@ igt_hang_t igt_hang_ctx_with_ahnd(int fd, uint64_t ahnd, uint32_t ctx, int ring,
 
 #define HANG_ALLOW_BAN 1
 #define HANG_ALLOW_CAPTURE 2
+#define HANG_WANT_ENGINE_RESET 4
 
 igt_hang_t igt_hang_ring(int fd, int ring);
 igt_hang_t igt_hang_ring_with_ahnd(int fd, int ring, uint64_t ahnd);
-- 
2.25.1

[Intel-gfx] [PATCH v2 i-g-t 8/8] tests/i915/gem_exec_capture: Update to support GuC based resets

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

When GuC submission is enabled, GuC itself manages hang detection and
recovery. Therefore, any test that relies on being able to trigger an
engine reset in the driver will fail. Full GT resets can still be
triggered by the driver. However, in that situation detecting the
specific context that caused a hang is not possible as the driver has
no information about what is actually running on the hardware at any
given time. Plus of course, there was no context that caused the hang
because the hang was triggered manually, so it's basically a bogus
mechanism in the first place!

Update the capture test to cause a reset via a the hangcheck mechanism
by submitting a hanging batch and waiting. That way it is guaranteed to
be testing the correct reset code paths for the current platform,
whether that is GuC enabled or not.

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
---
 tests/i915/gem_exec_capture.c | 65 ++++++++++++++++++++++++++++-------
 1 file changed, 53 insertions(+), 12 deletions(-)

diff --git a/tests/i915/gem_exec_capture.c b/tests/i915/gem_exec_capture.c
index e924d0a30..143d97ad4 100644
--- a/tests/i915/gem_exec_capture.c
+++ b/tests/i915/gem_exec_capture.c
@@ -23,6 +23,7 @@
 
 #include <sys/poll.h>
 #include <zlib.h>
+#include <sched.h>
 
 #include "i915/gem.h"
 #include "i915/gem_create.h"
@@ -31,6 +32,8 @@
 #include "igt_rand.h"
 #include "igt_sysfs.h"
 
+#define MAX_RESET_TIME	600
+
 IGT_TEST_DESCRIPTION("Check that we capture the user specified objects on a hang");
 
 struct offset {
@@ -213,7 +216,29 @@ static void configure_hangs(int fd, const struct intel_execution_engine2 *e, int
 	gem_engine_property_printf(fd, e->name, "heartbeat_interval_ms", "%d", 500);
 
 	/* Allow engine based resets and disable banning */
-	igt_allow_hang(fd, ctxt_id, HANG_ALLOW_CAPTURE);
+	igt_allow_hang(fd, ctxt_id, HANG_ALLOW_CAPTURE | HANG_WANT_ENGINE_RESET);
+}
+
+static bool fence_busy(int fence)
+{
+	return poll(&(struct pollfd){fence, POLLIN}, 1, 0) == 0;
+}
+
+static void wait_to_die(int fence_out)
+{
+	struct timeval before, after, delta;
+
+	/* Wait for a reset to occur */
+	gettimeofday(&before, NULL);
+	while (fence_busy(fence_out)) {
+		gettimeofday(&after, NULL);
+		timersub(&after, &before, &delta);
+		igt_assert(delta.tv_sec < MAX_RESET_TIME);
+		sched_yield();
+	}
+	gettimeofday(&after, NULL);
+	timersub(&after, &before, &delta);
+	igt_info("Target died after %ld.%06lds\n", delta.tv_sec, delta.tv_usec);
 }
 
 static void __capture1(int fd, int dir, uint64_t ahnd, const intel_ctx_t *ctx,
@@ -230,7 +255,7 @@ static void __capture1(int fd, int dir, uint64_t ahnd, const intel_ctx_t *ctx,
 	struct drm_i915_gem_execbuffer2 execbuf;
 	uint32_t *batch, *seqno;
 	struct offset offset;
-	int i;
+	int i, fence_out;
 
 	configure_hangs(fd, e, ctx->id);
 
@@ -315,18 +340,25 @@ static void __capture1(int fd, int dir, uint64_t ahnd, const intel_ctx_t *ctx,
 	execbuf.flags = e->flags;
 	if (gen > 3 && gen < 6)
 		execbuf.flags |= I915_EXEC_SECURE;
+	execbuf.flags |= I915_EXEC_FENCE_OUT;
 	execbuf.rsvd1 = ctx->id;
+	execbuf.rsvd2 = ~0UL;
 
 	igt_assert(!READ_ONCE(*seqno));
-	gem_execbuf(fd, &execbuf);
+	gem_execbuf_wr(fd, &execbuf);
+
+	fence_out = execbuf.rsvd2 >> 32;
+	igt_assert(fence_out >= 0);
 
 	/* Wait for the request to start */
 	while (READ_ONCE(*seqno) != 0xc0ffee)
 		igt_assert(gem_bo_busy(fd, obj[SCRATCH].handle));
 	munmap(seqno, 4096);
 
+	/* Wait for a reset to occur */
+	wait_to_die(fence_out);
+
 	/* Check that only the buffer we marked is reported in the error */
-	igt_force_gpu_reset(fd);
 	memset(&offset, 0, sizeof(offset));
 	offset.addr = obj[CAPTURE].offset;
 	igt_assert_eq(check_error_state(dir, &offset, 1, target_size, false), 1);
@@ -373,7 +405,8 @@ static int cmp(const void *A, const void *B)
 static struct offset *
 __captureN(int fd, int dir, uint64_t ahnd, const intel_ctx_t *ctx,
 	   const struct intel_execution_engine2 *e,
-	   unsigned int size, int count, unsigned int flags)
+	   unsigned int size, int count,
+	   unsigned int flags, int *_fence_out)
 #define INCREMENTAL 0x1
 #define ASYNC 0x2
 {
@@ -383,7 +416,7 @@ __captureN(int fd, int dir, uint64_t ahnd, const intel_ctx_t *ctx,
 	struct drm_i915_gem_execbuffer2 execbuf;
 	uint32_t *batch, *seqno;
 	struct offset *offsets;
-	int i;
+	int i, fence_out;
 
 	configure_hangs(fd, e, ctx->id);
 
@@ -491,10 +524,17 @@ __captureN(int fd, int dir, uint64_t ahnd, const intel_ctx_t *ctx,
 	execbuf.flags = e->flags;
 	if (gen > 3 && gen < 6)
 		execbuf.flags |= I915_EXEC_SECURE;
+	execbuf.flags |= I915_EXEC_FENCE_OUT;
 	execbuf.rsvd1 = ctx->id;
+	execbuf.rsvd2 = ~0UL;
 
 	igt_assert(!READ_ONCE(*seqno));
-	gem_execbuf(fd, &execbuf);
+	gem_execbuf_wr(fd, &execbuf);
+
+	fence_out = execbuf.rsvd2 >> 32;
+	igt_assert(fence_out >= 0);
+	if (_fence_out)
+		*_fence_out = fence_out;
 
 	/* Wait for the request to start */
 	while (READ_ONCE(*seqno) != 0xc0ffee)
@@ -502,7 +542,7 @@ __captureN(int fd, int dir, uint64_t ahnd, const intel_ctx_t *ctx,
 	munmap(seqno, 4096);
 
 	if (!(flags & ASYNC)) {
-		igt_force_gpu_reset(fd);
+		wait_to_die(fence_out);
 		gem_sync(fd, obj[count + 1].handle);
 	}
 
@@ -554,7 +594,7 @@ static void many(int fd, int dir, uint64_t size, unsigned int flags)
 	intel_require_memory(count, size, CHECK_RAM);
 	ahnd = get_reloc_ahnd(fd, ctx->id);
 
-	offsets = __captureN(fd, dir, ahnd, ctx, e, size, count, flags);
+	offsets = __captureN(fd, dir, ahnd, ctx, e, size, count, flags, NULL);
 
 	blobs = check_error_state(dir, offsets, count, size, !!(flags & INCREMENTAL));
 	igt_info("Captured %lu %"PRId64"-blobs out of a total of %lu\n",
@@ -607,6 +647,7 @@ static void prioinv(int fd, int dir, const intel_ctx_t *ctx,
 	igt_assert(pipe(link) == 0);
 	igt_fork(child, 1) {
 		const intel_ctx_t *ctx2;
+		int fence_out;
 		fd = gem_reopen_driver(fd);
 		igt_debug("Submitting large capture [%ld x %dMiB objects]\n",
 			  count, (int)(size >> 20));
@@ -618,11 +659,11 @@ static void prioinv(int fd, int dir, const intel_ctx_t *ctx,
 		/* Reopen the allocator in the new process. */
 		ahnd = get_reloc_ahnd(fd, ctx2->id);
 
-		free(__captureN(fd, dir, ahnd, ctx2, e, size, count, ASYNC));
+		free(__captureN(fd, dir, ahnd, ctx2, e, size, count, ASYNC, &fence_out));
 		put_ahnd(ahnd);
 
 		write(link[1], &fd, sizeof(fd)); /* wake the parent up */
-		igt_force_gpu_reset(fd);
+		wait_to_die(fence_out);
 		write(link[1], &fd, sizeof(fd)); /* wake the parent up */
 	}
 	read(link[0], &dummy, sizeof(dummy));
@@ -713,7 +754,7 @@ igt_main
 		gem_require_mmap_wc(fd);
 		igt_require(has_capture(fd));
 		ctx = intel_ctx_create_all_physical(fd);
-		igt_allow_hang(fd, ctx->id, HANG_ALLOW_CAPTURE);
+		igt_allow_hang(fd, ctx->id, HANG_ALLOW_CAPTURE | HANG_WANT_ENGINE_RESET);
 
 		dir = igt_sysfs_open(fd);
 		igt_require(igt_sysfs_set(dir, "error", "Begone!"));
-- 
2.25.1