All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH i-g-t 1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec
@ 2018-09-17 15:46 ` Tvrtko Ursulin
  0 siblings, 0 replies; 35+ messages in thread
From: Tvrtko Ursulin @ 2018-09-17 15:46 UTC (permalink / raw)
  To: igt-dev; +Cc: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Move a really small test that invalid context is rejected under the
gem_ctx_exec umbrella.

v2:
 * And actually fix the test so it does what it claims. And add more
   variety in the invalid context id's it tests with. (Chris Wilson)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 tests/Makefile.sources   |  1 -
 tests/gem_ctx_bad_exec.c | 60 ----------------------------------------
 tests/gem_ctx_exec.c     | 34 +++++++++++++++++++++++
 tests/meson.build        |  1 -
 4 files changed, 34 insertions(+), 62 deletions(-)
 delete mode 100644 tests/gem_ctx_bad_exec.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index c84933f1d971..269336ad3150 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -51,7 +51,6 @@ TESTS_progs = \
 	gem_cs_prefetch \
 	gem_cs_tlb \
 	gem_ctx_bad_destroy \
-	gem_ctx_bad_exec \
 	gem_ctx_create \
 	gem_ctx_exec \
 	gem_ctx_isolation \
diff --git a/tests/gem_ctx_bad_exec.c b/tests/gem_ctx_bad_exec.c
deleted file mode 100644
index e3ccc5be46a0..000000000000
--- a/tests/gem_ctx_bad_exec.c
+++ /dev/null
@@ -1,60 +0,0 @@
-/*
- * Copyright © 2012 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"
-
-IGT_TEST_DESCRIPTION("Test that context cannot be submitted to any ring");
-
-static int exec(int fd, unsigned ring)
-{
-	struct drm_i915_gem_execbuffer2 execbuf;
-	struct drm_i915_gem_exec_object2 obj;
-
-	memset(&obj, 0, sizeof(obj));
-	memset(&execbuf, 0, sizeof(execbuf));
-
-	execbuf.buffers_ptr = to_user_pointer(&obj);
-	execbuf.buffer_count = 1;
-	i915_execbuffer2_set_context_id(execbuf, 1);
-
-	return __gem_execbuf(fd, &execbuf);
-}
-
-igt_main
-{
-	const struct intel_execution_engine *e;
-	int fd = -1;
-
-	igt_skip_on_simulation();
-
-	igt_fixture
-		fd = drm_open_driver_render(DRIVER_INTEL);
-
-	for (e = intel_execution_engines; e->name; e++) {
-		igt_subtest_f("%s", e->name) {
-			gem_require_ring(fd, e->exec_id | e->flags);
-			igt_assert_eq(exec(fd, e->exec_id | e->flags), -ENOENT);
-		}
-	}
-}
diff --git a/tests/gem_ctx_exec.c b/tests/gem_ctx_exec.c
index 1f8ed64d4bd3..4ed6febe12b8 100644
--- a/tests/gem_ctx_exec.c
+++ b/tests/gem_ctx_exec.c
@@ -30,6 +30,7 @@
  */
 
 #include "igt.h"
+#include <limits.h>
 #include <unistd.h>
 #include <stdlib.h>
 #include <stdint.h>
@@ -142,6 +143,30 @@ static void big_exec(int fd, uint32_t handle, int ring)
 	gem_sync(fd, handle);
 }
 
+static void invalid_context(int fd, unsigned ring, uint32_t handle)
+{
+	struct drm_i915_gem_exec_object2 obj = {
+		.handle = handle,
+	};
+	struct drm_i915_gem_execbuffer2 execbuf = {
+		.buffers_ptr = to_user_pointer(&obj),
+		.buffer_count = 1,
+		.flags = ring,
+	};
+	const uint64_t invalid[] = { 1, INT_MAX, UINT_MAX, LONG_MAX, ULONG_MAX,
+				     LLONG_MAX, ULLONG_MAX };
+	int i;
+
+	/* Verify all works. */
+	gem_execbuf(fd, &execbuf);
+
+	/* Go through some non-existent context id's. */
+	for (i = 0; i < ARRAY_SIZE(invalid); i++) {
+		i915_execbuffer2_set_context_id(execbuf, invalid[i]);
+		igt_assert_eq(__gem_execbuf(fd, &execbuf), -ENOENT);
+	}
+}
+
 uint32_t handle;
 uint32_t batch[2] = {0, MI_BATCH_BUFFER_END};
 uint32_t ctx_id, ctx_id2;
@@ -149,6 +174,8 @@ int fd;
 
 igt_main
 {
+	const struct intel_execution_engine *e;
+
 	igt_fixture {
 		fd = drm_open_driver_render(DRIVER_INTEL);
 		igt_require_gem(fd);
@@ -174,6 +201,13 @@ igt_main
 		gem_sync(fd, handle);
 	}
 
+	for (e = intel_execution_engines; e->name; e++) {
+		igt_subtest_f("invalid-context-%s", e->name) {
+			gem_require_ring(fd, e->exec_id | e->flags);
+			invalid_context(fd, e->exec_id | e->flags, handle);
+		}
+	}
+
 	igt_subtest("eviction")
 		big_exec(fd, handle, I915_EXEC_RENDER);
 
diff --git a/tests/meson.build b/tests/meson.build
index 17deb945ec95..d22d59e0837d 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -28,7 +28,6 @@ test_progs = [
 	'gem_cs_prefetch',
 	'gem_cs_tlb',
 	'gem_ctx_bad_destroy',
-	'gem_ctx_bad_exec',
 	'gem_ctx_create',
 	'gem_ctx_exec',
 	'gem_ctx_isolation',
-- 
2.17.1

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

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

* [igt-dev] [PATCH i-g-t 1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec
@ 2018-09-17 15:46 ` Tvrtko Ursulin
  0 siblings, 0 replies; 35+ messages in thread
From: Tvrtko Ursulin @ 2018-09-17 15:46 UTC (permalink / raw)
  To: igt-dev; +Cc: Intel-gfx, Tvrtko Ursulin

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Move a really small test that invalid context is rejected under the
gem_ctx_exec umbrella.

v2:
 * And actually fix the test so it does what it claims. And add more
   variety in the invalid context id's it tests with. (Chris Wilson)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 tests/Makefile.sources   |  1 -
 tests/gem_ctx_bad_exec.c | 60 ----------------------------------------
 tests/gem_ctx_exec.c     | 34 +++++++++++++++++++++++
 tests/meson.build        |  1 -
 4 files changed, 34 insertions(+), 62 deletions(-)
 delete mode 100644 tests/gem_ctx_bad_exec.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index c84933f1d971..269336ad3150 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -51,7 +51,6 @@ TESTS_progs = \
 	gem_cs_prefetch \
 	gem_cs_tlb \
 	gem_ctx_bad_destroy \
-	gem_ctx_bad_exec \
 	gem_ctx_create \
 	gem_ctx_exec \
 	gem_ctx_isolation \
diff --git a/tests/gem_ctx_bad_exec.c b/tests/gem_ctx_bad_exec.c
deleted file mode 100644
index e3ccc5be46a0..000000000000
--- a/tests/gem_ctx_bad_exec.c
+++ /dev/null
@@ -1,60 +0,0 @@
-/*
- * Copyright © 2012 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"
-
-IGT_TEST_DESCRIPTION("Test that context cannot be submitted to any ring");
-
-static int exec(int fd, unsigned ring)
-{
-	struct drm_i915_gem_execbuffer2 execbuf;
-	struct drm_i915_gem_exec_object2 obj;
-
-	memset(&obj, 0, sizeof(obj));
-	memset(&execbuf, 0, sizeof(execbuf));
-
-	execbuf.buffers_ptr = to_user_pointer(&obj);
-	execbuf.buffer_count = 1;
-	i915_execbuffer2_set_context_id(execbuf, 1);
-
-	return __gem_execbuf(fd, &execbuf);
-}
-
-igt_main
-{
-	const struct intel_execution_engine *e;
-	int fd = -1;
-
-	igt_skip_on_simulation();
-
-	igt_fixture
-		fd = drm_open_driver_render(DRIVER_INTEL);
-
-	for (e = intel_execution_engines; e->name; e++) {
-		igt_subtest_f("%s", e->name) {
-			gem_require_ring(fd, e->exec_id | e->flags);
-			igt_assert_eq(exec(fd, e->exec_id | e->flags), -ENOENT);
-		}
-	}
-}
diff --git a/tests/gem_ctx_exec.c b/tests/gem_ctx_exec.c
index 1f8ed64d4bd3..4ed6febe12b8 100644
--- a/tests/gem_ctx_exec.c
+++ b/tests/gem_ctx_exec.c
@@ -30,6 +30,7 @@
  */
 
 #include "igt.h"
+#include <limits.h>
 #include <unistd.h>
 #include <stdlib.h>
 #include <stdint.h>
@@ -142,6 +143,30 @@ static void big_exec(int fd, uint32_t handle, int ring)
 	gem_sync(fd, handle);
 }
 
+static void invalid_context(int fd, unsigned ring, uint32_t handle)
+{
+	struct drm_i915_gem_exec_object2 obj = {
+		.handle = handle,
+	};
+	struct drm_i915_gem_execbuffer2 execbuf = {
+		.buffers_ptr = to_user_pointer(&obj),
+		.buffer_count = 1,
+		.flags = ring,
+	};
+	const uint64_t invalid[] = { 1, INT_MAX, UINT_MAX, LONG_MAX, ULONG_MAX,
+				     LLONG_MAX, ULLONG_MAX };
+	int i;
+
+	/* Verify all works. */
+	gem_execbuf(fd, &execbuf);
+
+	/* Go through some non-existent context id's. */
+	for (i = 0; i < ARRAY_SIZE(invalid); i++) {
+		i915_execbuffer2_set_context_id(execbuf, invalid[i]);
+		igt_assert_eq(__gem_execbuf(fd, &execbuf), -ENOENT);
+	}
+}
+
 uint32_t handle;
 uint32_t batch[2] = {0, MI_BATCH_BUFFER_END};
 uint32_t ctx_id, ctx_id2;
@@ -149,6 +174,8 @@ int fd;
 
 igt_main
 {
+	const struct intel_execution_engine *e;
+
 	igt_fixture {
 		fd = drm_open_driver_render(DRIVER_INTEL);
 		igt_require_gem(fd);
@@ -174,6 +201,13 @@ igt_main
 		gem_sync(fd, handle);
 	}
 
+	for (e = intel_execution_engines; e->name; e++) {
+		igt_subtest_f("invalid-context-%s", e->name) {
+			gem_require_ring(fd, e->exec_id | e->flags);
+			invalid_context(fd, e->exec_id | e->flags, handle);
+		}
+	}
+
 	igt_subtest("eviction")
 		big_exec(fd, handle, I915_EXEC_RENDER);
 
diff --git a/tests/meson.build b/tests/meson.build
index 17deb945ec95..d22d59e0837d 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -28,7 +28,6 @@ test_progs = [
 	'gem_cs_prefetch',
 	'gem_cs_tlb',
 	'gem_ctx_bad_destroy',
-	'gem_ctx_bad_exec',
 	'gem_ctx_create',
 	'gem_ctx_exec',
 	'gem_ctx_isolation',
-- 
2.17.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* [PATCH i-g-t 2/2] gem_ctx_exec: Remove lrc-lite-restore
  2018-09-17 15:46 ` [igt-dev] " Tvrtko Ursulin
@ 2018-09-17 15:46   ` Tvrtko Ursulin
  -1 siblings, 0 replies; 35+ messages in thread
From: Tvrtko Ursulin @ 2018-09-17 15:46 UTC (permalink / raw)
  To: igt-dev; +Cc: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Lite restore is sufficiently covered in gem_exec_nop.

At the same time tidy code in the test a bit to bring it closer to today's
coding style.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/gem_ctx_exec.c | 116 +++++++++++--------------------------------
 1 file changed, 28 insertions(+), 88 deletions(-)

diff --git a/tests/gem_ctx_exec.c b/tests/gem_ctx_exec.c
index 4ed6febe12b8..b0f7b61e90be 100644
--- a/tests/gem_ctx_exec.c
+++ b/tests/gem_ctx_exec.c
@@ -24,11 +24,6 @@
  *    Ben Widawsky <ben@bwidawsk.net>
  *
  */
-
-/*
- * This test covers basic context switch functionality
- */
-
 #include "igt.h"
 #include <limits.h>
 #include <unistd.h>
@@ -46,44 +41,32 @@
 #include <drm.h>
 
 
-IGT_TEST_DESCRIPTION("Test basic context switch functionality.");
+IGT_TEST_DESCRIPTION("Test context batch buffer execution.");
 
 /* Copied from gem_exec_nop.c */
 static int exec(int fd, uint32_t handle, int ring, int ctx_id)
 {
-	struct drm_i915_gem_execbuffer2 execbuf;
-	struct drm_i915_gem_exec_object2 gem_exec;
-
-	gem_exec.handle = handle;
-	gem_exec.relocation_count = 0;
-	gem_exec.relocs_ptr = 0;
-	gem_exec.alignment = 0;
-	gem_exec.offset = 0;
-	gem_exec.flags = 0;
-	gem_exec.rsvd1 = 0;
-	gem_exec.rsvd2 = 0;
-
-	execbuf.buffers_ptr = to_user_pointer(&gem_exec);
-	execbuf.buffer_count = 1;
-	execbuf.batch_start_offset = 0;
-	execbuf.batch_len = 8;
-	execbuf.cliprects_ptr = 0;
-	execbuf.num_cliprects = 0;
-	execbuf.DR1 = 0;
-	execbuf.DR4 = 0;
-	execbuf.flags = ring;
+	struct drm_i915_gem_exec_object2 obj = { .handle = handle };
+	struct drm_i915_gem_execbuffer2 execbuf = {
+		.buffers_ptr = to_user_pointer(&obj),
+		.buffer_count = 1,
+		.flags = ring,
+	};
+
 	i915_execbuffer2_set_context_id(execbuf, ctx_id);
-	execbuf.rsvd2 = 0;
 
 	return __gem_execbuf(fd, &execbuf);
 }
 
 static void big_exec(int fd, uint32_t handle, int ring)
 {
-	struct drm_i915_gem_execbuffer2 execbuf;
+	int num_buffers = gem_global_aperture_size(fd) / 4096;
+	struct drm_i915_gem_execbuffer2 execbuf = {
+		.buffer_count = num_buffers,
+		.flags = ring,
+	};
 	struct drm_i915_gem_exec_object2 *gem_exec;
 	uint32_t ctx_id1, ctx_id2;
-	int num_buffers = gem_global_aperture_size(fd) / 4096;
 	int i;
 
 	/* Make sure we only fill half of RAM with gem objects. */
@@ -93,33 +76,19 @@ static void big_exec(int fd, uint32_t handle, int ring)
 	igt_assert(gem_exec);
 	memset(gem_exec, 0, (num_buffers + 1) * sizeof(*gem_exec));
 
-
 	ctx_id1 = gem_context_create(fd);
 	ctx_id2 = gem_context_create(fd);
 
 	gem_exec[0].handle = handle;
 
-
 	execbuf.buffers_ptr = to_user_pointer(gem_exec);
-	execbuf.buffer_count = num_buffers + 1;
-	execbuf.batch_start_offset = 0;
-	execbuf.batch_len = 8;
-	execbuf.cliprects_ptr = 0;
-	execbuf.num_cliprects = 0;
-	execbuf.DR1 = 0;
-	execbuf.DR4 = 0;
-	execbuf.flags = ring;
-	execbuf.rsvd2 = 0;
 
 	execbuf.buffer_count = 1;
 	i915_execbuffer2_set_context_id(execbuf, ctx_id1);
 	gem_execbuf(fd, &execbuf);
 
-	for (i = 0; i < num_buffers; i++) {
-		uint32_t tmp_handle = gem_create(fd, 4096);
-
-		gem_exec[i].handle = tmp_handle;
-	}
+	for (i = 0; i < num_buffers; i++)
+		gem_exec[i].handle = gem_create(fd, 4096);
 	gem_exec[i].handle = handle;
 	execbuf.buffer_count = i + 1;
 
@@ -132,8 +101,7 @@ static void big_exec(int fd, uint32_t handle, int ring)
 		igt_info("trying buffer count %i\n", i - 1);
 	}
 
-	igt_info("reduced buffer count to %i from %i\n",
-	       i - 1, num_buffers);
+	igt_info("reduced buffer count to %i from %i\n", i - 1, num_buffers);
 
 	/* double check that it works */
 	gem_execbuf(fd, &execbuf);
@@ -167,14 +135,13 @@ static void invalid_context(int fd, unsigned ring, uint32_t handle)
 	}
 }
 
-uint32_t handle;
-uint32_t batch[2] = {0, MI_BATCH_BUFFER_END};
-uint32_t ctx_id, ctx_id2;
-int fd;
-
 igt_main
 {
+	const uint32_t batch[2] = { 0, MI_BATCH_BUFFER_END };
 	const struct intel_execution_engine *e;
+	uint32_t handle;
+	uint32_t ctx_id;
+	int fd;
 
 	igt_fixture {
 		fd = drm_open_driver_render(DRIVER_INTEL);
@@ -188,16 +155,16 @@ igt_main
 
 	igt_subtest("basic") {
 		ctx_id = gem_context_create(fd);
-		igt_assert(exec(fd, handle, I915_EXEC_RENDER, ctx_id) == 0);
+		igt_assert(exec(fd, handle, 0, ctx_id) == 0);
 		gem_sync(fd, handle);
 		gem_context_destroy(fd, ctx_id);
 
 		ctx_id = gem_context_create(fd);
-		igt_assert(exec(fd, handle, I915_EXEC_RENDER, ctx_id) == 0);
+		igt_assert(exec(fd, handle, 0, ctx_id) == 0);
 		gem_sync(fd, handle);
 		gem_context_destroy(fd, ctx_id);
 
-		igt_assert(exec(fd, handle, I915_EXEC_RENDER, ctx_id) < 0);
+		igt_assert(exec(fd, handle, 0, ctx_id) < 0);
 		gem_sync(fd, handle);
 	}
 
@@ -209,7 +176,7 @@ igt_main
 	}
 
 	igt_subtest("eviction")
-		big_exec(fd, handle, I915_EXEC_RENDER);
+		big_exec(fd, handle, 0);
 
 	igt_subtest("reset-pin-leak") {
 		int i;
@@ -229,40 +196,13 @@ igt_main
 		 * the last context is leaked at every reset.
 		 */
 		for (i = 0; i < 20; i++) {
-			igt_hang_t hang = igt_hang_ring(fd, I915_EXEC_RENDER);
-			igt_assert(exec(fd, handle, I915_EXEC_RENDER, 0) == 0);
-			igt_assert(exec(fd, handle, I915_EXEC_RENDER, ctx_id) == 0);
-			igt_post_hang_ring(fd, hang);
-		}
-
-		gem_context_destroy(fd, ctx_id);
-	}
-
-	igt_subtest("lrc-lite-restore") {
-		int i, j;
-
-		/*
-		 * Need 2 contexts to be able to replicate a lite restore,
-		 * i.e. a running context is resubmitted.
-		 */
-		ctx_id = gem_context_create(fd);
-		ctx_id2 = gem_context_create(fd);
-
-		/*
-		 * Queue several small batchbuffers to be sure we'll send execlists
-		 * with 2 valid context, and likely cause a lite restore when ctxB
-		 * is resubmitted at the top of the new execlist.
-		 */
-		for (i = 0; i < 20; i++) {
-			for (j = 0; j < 200; j++) {
-				igt_assert(exec(fd, handle, I915_EXEC_RENDER, ctx_id) == 0);
-				igt_assert(exec(fd, handle, I915_EXEC_RENDER, ctx_id2) == 0);
-			}
+			igt_hang_t hang = igt_hang_ring(fd, 0);
 
-			gem_sync(fd, handle);
+			igt_assert_eq(exec(fd, handle, 0, 0), 0);
+			igt_assert_eq(exec(fd, handle, 0, ctx_id), 0);
+			igt_post_hang_ring(fd, hang);
 		}
 
 		gem_context_destroy(fd, ctx_id);
-		gem_context_destroy(fd, ctx_id2);
 	}
 }
-- 
2.17.1

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

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

* [igt-dev] [PATCH i-g-t 2/2] gem_ctx_exec: Remove lrc-lite-restore
@ 2018-09-17 15:46   ` Tvrtko Ursulin
  0 siblings, 0 replies; 35+ messages in thread
From: Tvrtko Ursulin @ 2018-09-17 15:46 UTC (permalink / raw)
  To: igt-dev; +Cc: Intel-gfx, Tvrtko Ursulin

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Lite restore is sufficiently covered in gem_exec_nop.

At the same time tidy code in the test a bit to bring it closer to today's
coding style.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/gem_ctx_exec.c | 116 +++++++++++--------------------------------
 1 file changed, 28 insertions(+), 88 deletions(-)

diff --git a/tests/gem_ctx_exec.c b/tests/gem_ctx_exec.c
index 4ed6febe12b8..b0f7b61e90be 100644
--- a/tests/gem_ctx_exec.c
+++ b/tests/gem_ctx_exec.c
@@ -24,11 +24,6 @@
  *    Ben Widawsky <ben@bwidawsk.net>
  *
  */
-
-/*
- * This test covers basic context switch functionality
- */
-
 #include "igt.h"
 #include <limits.h>
 #include <unistd.h>
@@ -46,44 +41,32 @@
 #include <drm.h>
 
 
-IGT_TEST_DESCRIPTION("Test basic context switch functionality.");
+IGT_TEST_DESCRIPTION("Test context batch buffer execution.");
 
 /* Copied from gem_exec_nop.c */
 static int exec(int fd, uint32_t handle, int ring, int ctx_id)
 {
-	struct drm_i915_gem_execbuffer2 execbuf;
-	struct drm_i915_gem_exec_object2 gem_exec;
-
-	gem_exec.handle = handle;
-	gem_exec.relocation_count = 0;
-	gem_exec.relocs_ptr = 0;
-	gem_exec.alignment = 0;
-	gem_exec.offset = 0;
-	gem_exec.flags = 0;
-	gem_exec.rsvd1 = 0;
-	gem_exec.rsvd2 = 0;
-
-	execbuf.buffers_ptr = to_user_pointer(&gem_exec);
-	execbuf.buffer_count = 1;
-	execbuf.batch_start_offset = 0;
-	execbuf.batch_len = 8;
-	execbuf.cliprects_ptr = 0;
-	execbuf.num_cliprects = 0;
-	execbuf.DR1 = 0;
-	execbuf.DR4 = 0;
-	execbuf.flags = ring;
+	struct drm_i915_gem_exec_object2 obj = { .handle = handle };
+	struct drm_i915_gem_execbuffer2 execbuf = {
+		.buffers_ptr = to_user_pointer(&obj),
+		.buffer_count = 1,
+		.flags = ring,
+	};
+
 	i915_execbuffer2_set_context_id(execbuf, ctx_id);
-	execbuf.rsvd2 = 0;
 
 	return __gem_execbuf(fd, &execbuf);
 }
 
 static void big_exec(int fd, uint32_t handle, int ring)
 {
-	struct drm_i915_gem_execbuffer2 execbuf;
+	int num_buffers = gem_global_aperture_size(fd) / 4096;
+	struct drm_i915_gem_execbuffer2 execbuf = {
+		.buffer_count = num_buffers,
+		.flags = ring,
+	};
 	struct drm_i915_gem_exec_object2 *gem_exec;
 	uint32_t ctx_id1, ctx_id2;
-	int num_buffers = gem_global_aperture_size(fd) / 4096;
 	int i;
 
 	/* Make sure we only fill half of RAM with gem objects. */
@@ -93,33 +76,19 @@ static void big_exec(int fd, uint32_t handle, int ring)
 	igt_assert(gem_exec);
 	memset(gem_exec, 0, (num_buffers + 1) * sizeof(*gem_exec));
 
-
 	ctx_id1 = gem_context_create(fd);
 	ctx_id2 = gem_context_create(fd);
 
 	gem_exec[0].handle = handle;
 
-
 	execbuf.buffers_ptr = to_user_pointer(gem_exec);
-	execbuf.buffer_count = num_buffers + 1;
-	execbuf.batch_start_offset = 0;
-	execbuf.batch_len = 8;
-	execbuf.cliprects_ptr = 0;
-	execbuf.num_cliprects = 0;
-	execbuf.DR1 = 0;
-	execbuf.DR4 = 0;
-	execbuf.flags = ring;
-	execbuf.rsvd2 = 0;
 
 	execbuf.buffer_count = 1;
 	i915_execbuffer2_set_context_id(execbuf, ctx_id1);
 	gem_execbuf(fd, &execbuf);
 
-	for (i = 0; i < num_buffers; i++) {
-		uint32_t tmp_handle = gem_create(fd, 4096);
-
-		gem_exec[i].handle = tmp_handle;
-	}
+	for (i = 0; i < num_buffers; i++)
+		gem_exec[i].handle = gem_create(fd, 4096);
 	gem_exec[i].handle = handle;
 	execbuf.buffer_count = i + 1;
 
@@ -132,8 +101,7 @@ static void big_exec(int fd, uint32_t handle, int ring)
 		igt_info("trying buffer count %i\n", i - 1);
 	}
 
-	igt_info("reduced buffer count to %i from %i\n",
-	       i - 1, num_buffers);
+	igt_info("reduced buffer count to %i from %i\n", i - 1, num_buffers);
 
 	/* double check that it works */
 	gem_execbuf(fd, &execbuf);
@@ -167,14 +135,13 @@ static void invalid_context(int fd, unsigned ring, uint32_t handle)
 	}
 }
 
-uint32_t handle;
-uint32_t batch[2] = {0, MI_BATCH_BUFFER_END};
-uint32_t ctx_id, ctx_id2;
-int fd;
-
 igt_main
 {
+	const uint32_t batch[2] = { 0, MI_BATCH_BUFFER_END };
 	const struct intel_execution_engine *e;
+	uint32_t handle;
+	uint32_t ctx_id;
+	int fd;
 
 	igt_fixture {
 		fd = drm_open_driver_render(DRIVER_INTEL);
@@ -188,16 +155,16 @@ igt_main
 
 	igt_subtest("basic") {
 		ctx_id = gem_context_create(fd);
-		igt_assert(exec(fd, handle, I915_EXEC_RENDER, ctx_id) == 0);
+		igt_assert(exec(fd, handle, 0, ctx_id) == 0);
 		gem_sync(fd, handle);
 		gem_context_destroy(fd, ctx_id);
 
 		ctx_id = gem_context_create(fd);
-		igt_assert(exec(fd, handle, I915_EXEC_RENDER, ctx_id) == 0);
+		igt_assert(exec(fd, handle, 0, ctx_id) == 0);
 		gem_sync(fd, handle);
 		gem_context_destroy(fd, ctx_id);
 
-		igt_assert(exec(fd, handle, I915_EXEC_RENDER, ctx_id) < 0);
+		igt_assert(exec(fd, handle, 0, ctx_id) < 0);
 		gem_sync(fd, handle);
 	}
 
@@ -209,7 +176,7 @@ igt_main
 	}
 
 	igt_subtest("eviction")
-		big_exec(fd, handle, I915_EXEC_RENDER);
+		big_exec(fd, handle, 0);
 
 	igt_subtest("reset-pin-leak") {
 		int i;
@@ -229,40 +196,13 @@ igt_main
 		 * the last context is leaked at every reset.
 		 */
 		for (i = 0; i < 20; i++) {
-			igt_hang_t hang = igt_hang_ring(fd, I915_EXEC_RENDER);
-			igt_assert(exec(fd, handle, I915_EXEC_RENDER, 0) == 0);
-			igt_assert(exec(fd, handle, I915_EXEC_RENDER, ctx_id) == 0);
-			igt_post_hang_ring(fd, hang);
-		}
-
-		gem_context_destroy(fd, ctx_id);
-	}
-
-	igt_subtest("lrc-lite-restore") {
-		int i, j;
-
-		/*
-		 * Need 2 contexts to be able to replicate a lite restore,
-		 * i.e. a running context is resubmitted.
-		 */
-		ctx_id = gem_context_create(fd);
-		ctx_id2 = gem_context_create(fd);
-
-		/*
-		 * Queue several small batchbuffers to be sure we'll send execlists
-		 * with 2 valid context, and likely cause a lite restore when ctxB
-		 * is resubmitted at the top of the new execlist.
-		 */
-		for (i = 0; i < 20; i++) {
-			for (j = 0; j < 200; j++) {
-				igt_assert(exec(fd, handle, I915_EXEC_RENDER, ctx_id) == 0);
-				igt_assert(exec(fd, handle, I915_EXEC_RENDER, ctx_id2) == 0);
-			}
+			igt_hang_t hang = igt_hang_ring(fd, 0);
 
-			gem_sync(fd, handle);
+			igt_assert_eq(exec(fd, handle, 0, 0), 0);
+			igt_assert_eq(exec(fd, handle, 0, ctx_id), 0);
+			igt_post_hang_ring(fd, hang);
 		}
 
 		gem_context_destroy(fd, ctx_id);
-		gem_context_destroy(fd, ctx_id2);
 	}
 }
-- 
2.17.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [igt-dev] [PATCH i-g-t 1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec
  2018-09-17 15:46 ` [igt-dev] " Tvrtko Ursulin
@ 2018-09-17 15:52   ` Chris Wilson
  -1 siblings, 0 replies; 35+ messages in thread
From: Chris Wilson @ 2018-09-17 15:52 UTC (permalink / raw)
  To: Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx

Quoting Tvrtko Ursulin (2018-09-17 16:46:18)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Move a really small test that invalid context is rejected under the
> gem_ctx_exec umbrella.
> 
> v2:
>  * And actually fix the test so it does what it claims. And add more
>    variety in the invalid context id's it tests with. (Chris Wilson)
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  tests/Makefile.sources   |  1 -
>  tests/gem_ctx_bad_exec.c | 60 ----------------------------------------
>  tests/gem_ctx_exec.c     | 34 +++++++++++++++++++++++
>  tests/meson.build        |  1 -
>  4 files changed, 34 insertions(+), 62 deletions(-)
>  delete mode 100644 tests/gem_ctx_bad_exec.c
> 
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index c84933f1d971..269336ad3150 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -51,7 +51,6 @@ TESTS_progs = \
>         gem_cs_prefetch \
>         gem_cs_tlb \
>         gem_ctx_bad_destroy \
> -       gem_ctx_bad_exec \
>         gem_ctx_create \
>         gem_ctx_exec \
>         gem_ctx_isolation \
> diff --git a/tests/gem_ctx_bad_exec.c b/tests/gem_ctx_bad_exec.c
> deleted file mode 100644
> index e3ccc5be46a0..000000000000
> --- a/tests/gem_ctx_bad_exec.c
> +++ /dev/null
> @@ -1,60 +0,0 @@
> -/*
> - * Copyright © 2012 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"
> -
> -IGT_TEST_DESCRIPTION("Test that context cannot be submitted to any ring");
> -
> -static int exec(int fd, unsigned ring)
> -{
> -       struct drm_i915_gem_execbuffer2 execbuf;
> -       struct drm_i915_gem_exec_object2 obj;
> -
> -       memset(&obj, 0, sizeof(obj));
> -       memset(&execbuf, 0, sizeof(execbuf));
> -
> -       execbuf.buffers_ptr = to_user_pointer(&obj);
> -       execbuf.buffer_count = 1;
> -       i915_execbuffer2_set_context_id(execbuf, 1);
> -
> -       return __gem_execbuf(fd, &execbuf);
> -}
> -
> -igt_main
> -{
> -       const struct intel_execution_engine *e;
> -       int fd = -1;
> -
> -       igt_skip_on_simulation();
> -
> -       igt_fixture
> -               fd = drm_open_driver_render(DRIVER_INTEL);
> -
> -       for (e = intel_execution_engines; e->name; e++) {
> -               igt_subtest_f("%s", e->name) {
> -                       gem_require_ring(fd, e->exec_id | e->flags);
> -                       igt_assert_eq(exec(fd, e->exec_id | e->flags), -ENOENT);
> -               }
> -       }
> -}
> diff --git a/tests/gem_ctx_exec.c b/tests/gem_ctx_exec.c
> index 1f8ed64d4bd3..4ed6febe12b8 100644
> --- a/tests/gem_ctx_exec.c
> +++ b/tests/gem_ctx_exec.c
> @@ -30,6 +30,7 @@
>   */
>  
>  #include "igt.h"
> +#include <limits.h>
>  #include <unistd.h>
>  #include <stdlib.h>
>  #include <stdint.h>
> @@ -142,6 +143,30 @@ static void big_exec(int fd, uint32_t handle, int ring)
>         gem_sync(fd, handle);
>  }
>  
> +static void invalid_context(int fd, unsigned ring, uint32_t handle)
> +{
> +       struct drm_i915_gem_exec_object2 obj = {
> +               .handle = handle,
> +       };
> +       struct drm_i915_gem_execbuffer2 execbuf = {
> +               .buffers_ptr = to_user_pointer(&obj),
> +               .buffer_count = 1,
> +               .flags = ring,
> +       };
> +       const uint64_t invalid[] = { 1, INT_MAX, UINT_MAX, LONG_MAX, ULONG_MAX,
> +                                    LLONG_MAX, ULLONG_MAX };

The field is only a u32. Strange you didn't notice that amidst the
absence of documentation ;)

Trim this array to suite, and
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [igt-dev] [PATCH i-g-t 1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec
@ 2018-09-17 15:52   ` Chris Wilson
  0 siblings, 0 replies; 35+ messages in thread
From: Chris Wilson @ 2018-09-17 15:52 UTC (permalink / raw)
  To: Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx, Tvrtko Ursulin

Quoting Tvrtko Ursulin (2018-09-17 16:46:18)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Move a really small test that invalid context is rejected under the
> gem_ctx_exec umbrella.
> 
> v2:
>  * And actually fix the test so it does what it claims. And add more
>    variety in the invalid context id's it tests with. (Chris Wilson)
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  tests/Makefile.sources   |  1 -
>  tests/gem_ctx_bad_exec.c | 60 ----------------------------------------
>  tests/gem_ctx_exec.c     | 34 +++++++++++++++++++++++
>  tests/meson.build        |  1 -
>  4 files changed, 34 insertions(+), 62 deletions(-)
>  delete mode 100644 tests/gem_ctx_bad_exec.c
> 
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index c84933f1d971..269336ad3150 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -51,7 +51,6 @@ TESTS_progs = \
>         gem_cs_prefetch \
>         gem_cs_tlb \
>         gem_ctx_bad_destroy \
> -       gem_ctx_bad_exec \
>         gem_ctx_create \
>         gem_ctx_exec \
>         gem_ctx_isolation \
> diff --git a/tests/gem_ctx_bad_exec.c b/tests/gem_ctx_bad_exec.c
> deleted file mode 100644
> index e3ccc5be46a0..000000000000
> --- a/tests/gem_ctx_bad_exec.c
> +++ /dev/null
> @@ -1,60 +0,0 @@
> -/*
> - * Copyright © 2012 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"
> -
> -IGT_TEST_DESCRIPTION("Test that context cannot be submitted to any ring");
> -
> -static int exec(int fd, unsigned ring)
> -{
> -       struct drm_i915_gem_execbuffer2 execbuf;
> -       struct drm_i915_gem_exec_object2 obj;
> -
> -       memset(&obj, 0, sizeof(obj));
> -       memset(&execbuf, 0, sizeof(execbuf));
> -
> -       execbuf.buffers_ptr = to_user_pointer(&obj);
> -       execbuf.buffer_count = 1;
> -       i915_execbuffer2_set_context_id(execbuf, 1);
> -
> -       return __gem_execbuf(fd, &execbuf);
> -}
> -
> -igt_main
> -{
> -       const struct intel_execution_engine *e;
> -       int fd = -1;
> -
> -       igt_skip_on_simulation();
> -
> -       igt_fixture
> -               fd = drm_open_driver_render(DRIVER_INTEL);
> -
> -       for (e = intel_execution_engines; e->name; e++) {
> -               igt_subtest_f("%s", e->name) {
> -                       gem_require_ring(fd, e->exec_id | e->flags);
> -                       igt_assert_eq(exec(fd, e->exec_id | e->flags), -ENOENT);
> -               }
> -       }
> -}
> diff --git a/tests/gem_ctx_exec.c b/tests/gem_ctx_exec.c
> index 1f8ed64d4bd3..4ed6febe12b8 100644
> --- a/tests/gem_ctx_exec.c
> +++ b/tests/gem_ctx_exec.c
> @@ -30,6 +30,7 @@
>   */
>  
>  #include "igt.h"
> +#include <limits.h>
>  #include <unistd.h>
>  #include <stdlib.h>
>  #include <stdint.h>
> @@ -142,6 +143,30 @@ static void big_exec(int fd, uint32_t handle, int ring)
>         gem_sync(fd, handle);
>  }
>  
> +static void invalid_context(int fd, unsigned ring, uint32_t handle)
> +{
> +       struct drm_i915_gem_exec_object2 obj = {
> +               .handle = handle,
> +       };
> +       struct drm_i915_gem_execbuffer2 execbuf = {
> +               .buffers_ptr = to_user_pointer(&obj),
> +               .buffer_count = 1,
> +               .flags = ring,
> +       };
> +       const uint64_t invalid[] = { 1, INT_MAX, UINT_MAX, LONG_MAX, ULONG_MAX,
> +                                    LLONG_MAX, ULLONG_MAX };

The field is only a u32. Strange you didn't notice that amidst the
absence of documentation ;)

Trim this array to suite, and
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [PATCH i-g-t 2/2] gem_ctx_exec: Remove lrc-lite-restore
  2018-09-17 15:46   ` [igt-dev] " Tvrtko Ursulin
@ 2018-09-17 15:53     ` Chris Wilson
  -1 siblings, 0 replies; 35+ messages in thread
From: Chris Wilson @ 2018-09-17 15:53 UTC (permalink / raw)
  To: Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx

Quoting Tvrtko Ursulin (2018-09-17 16:46:19)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Lite restore is sufficiently covered in gem_exec_nop.
> 
> At the same time tidy code in the test a bit to bring it closer to today's
> coding style.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t 2/2] gem_ctx_exec: Remove lrc-lite-restore
@ 2018-09-17 15:53     ` Chris Wilson
  0 siblings, 0 replies; 35+ messages in thread
From: Chris Wilson @ 2018-09-17 15:53 UTC (permalink / raw)
  To: Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx

Quoting Tvrtko Ursulin (2018-09-17 16:46:19)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Lite restore is sufficiently covered in gem_exec_nop.
> 
> At the same time tidy code in the test a bit to bring it closer to today's
> coding style.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* [igt-dev] ✗ Fi.CI.BAT: failure for series starting with [i-g-t,1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec
  2018-09-17 15:46 ` [igt-dev] " Tvrtko Ursulin
                   ` (2 preceding siblings ...)
  (?)
@ 2018-09-17 16:27 ` Patchwork
  -1 siblings, 0 replies; 35+ messages in thread
From: Patchwork @ 2018-09-17 16:27 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: igt-dev

== Series Details ==

Series: series starting with [i-g-t,1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec
URL   : https://patchwork.freedesktop.org/series/49797/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4833 -> IGTPW_1849 =

== Summary - FAILURE ==

  Serious unknown changes coming with IGTPW_1849 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_1849, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/49797/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in IGTPW_1849:

  === IGT changes ===

    ==== Possible regressions ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-icl-u:           NOTRUN -> INCOMPLETE

    
== Known issues ==

  Here are the changes found in IGTPW_1849 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_getparams_basic@basic-subslice-total:
      fi-snb-2520m:       PASS -> DMESG-WARN (fdo#103713) +10

    igt@gem_exec_suspend@basic-s3:
      fi-skl-caroline:    NOTRUN -> INCOMPLETE (fdo#104108, fdo#107556)

    igt@kms_frontbuffer_tracking@basic:
      fi-byt-clapper:     PASS -> FAIL (fdo#103167)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-byt-clapper:     PASS -> FAIL (fdo#103191, fdo#107362)

    igt@kms_psr@primary_mmap_gtt:
      {fi-cnl-u}:         NOTRUN -> FAIL (fdo#107383) +3

    
    ==== Possible fixes ====

    igt@drv_module_reload@basic-reload:
      fi-blb-e6850:       INCOMPLETE (fdo#107718) -> PASS

    igt@drv_selftest@live_hangcheck:
      fi-glk-j4005:       INCOMPLETE (fdo#103359, k.org#198133) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      fi-icl-u:           INCOMPLETE (fdo#107713) -> PASS

    igt@kms_psr@primary_page_flip:
      fi-kbl-r:           FAIL (fdo#107336) -> PASS

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
  fdo#107336 https://bugs.freedesktop.org/show_bug.cgi?id=107336
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107383 https://bugs.freedesktop.org/show_bug.cgi?id=107383
  fdo#107556 https://bugs.freedesktop.org/show_bug.cgi?id=107556
  fdo#107713 https://bugs.freedesktop.org/show_bug.cgi?id=107713
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (46 -> 42) ==

  Additional (2): fi-cnl-u fi-skl-caroline 
  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-bdw-gvtdvm fi-byt-squawks fi-bsw-cyan fi-bsw-kefka 


== Build changes ==

    * IGT: IGT_4644 -> IGTPW_1849

  CI_DRM_4833: 75bb460b367a614d10b0fba220143bee42657d7e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1849: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1849/
  IGT_4644: 0b59bb3231ab481959528c5c7b3a98762772e1b0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+igt@gem_ctx_exec@invalid-context-blt
+igt@gem_ctx_exec@invalid-context-bsd
+igt@gem_ctx_exec@invalid-context-bsd1
+igt@gem_ctx_exec@invalid-context-bsd2
+igt@gem_ctx_exec@invalid-context-default
+igt@gem_ctx_exec@invalid-context-render
+igt@gem_ctx_exec@invalid-context-vebox
-igt@gem_ctx_bad_exec@blt
-igt@gem_ctx_bad_exec@bsd
-igt@gem_ctx_bad_exec@bsd1
-igt@gem_ctx_bad_exec@bsd2
-igt@gem_ctx_bad_exec@default
-igt@gem_ctx_bad_exec@render
-igt@gem_ctx_bad_exec@vebox
-igt@gem_ctx_exec@lrc-lite-restore

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1849/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec
  2018-09-17 15:46 ` [igt-dev] " Tvrtko Ursulin
                   ` (3 preceding siblings ...)
  (?)
@ 2018-09-17 18:04 ` Patchwork
  -1 siblings, 0 replies; 35+ messages in thread
From: Patchwork @ 2018-09-17 18:04 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: igt-dev

== Series Details ==

Series: series starting with [i-g-t,1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec
URL   : https://patchwork.freedesktop.org/series/49797/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4833 -> IGTPW_1850 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/49797/revisions/1/mbox/

== Known issues ==

  Here are the changes found in IGTPW_1850 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@amdgpu/amd_cs_nop@sync-fork-compute0:
      fi-kbl-8809g:       PASS -> DMESG-WARN (fdo#107762)

    igt@gem_exec_suspend@basic-s3:
      fi-skl-caroline:    NOTRUN -> INCOMPLETE (fdo#107556, fdo#104108)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-blb-e6850:       PASS -> INCOMPLETE (fdo#107718)

    igt@kms_psr@primary_mmap_gtt:
      {fi-cnl-u}:         NOTRUN -> FAIL (fdo#107383) +3

    igt@kms_psr@primary_page_flip:
      fi-icl-u:           NOTRUN -> FAIL (fdo#107336)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_hangcheck:
      fi-glk-j4005:       INCOMPLETE (fdo#103359, k.org#198133) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      fi-icl-u:           INCOMPLETE (fdo#107713) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-snb-2520m:       DMESG-FAIL (fdo#103713) -> PASS

    igt@kms_psr@primary_page_flip:
      fi-kbl-r:           FAIL (fdo#107336) -> PASS

    igt@kms_setmode@basic-clone-single-crtc:
      fi-snb-2520m:       DMESG-WARN (fdo#103713) -> PASS

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
  fdo#107336 https://bugs.freedesktop.org/show_bug.cgi?id=107336
  fdo#107383 https://bugs.freedesktop.org/show_bug.cgi?id=107383
  fdo#107556 https://bugs.freedesktop.org/show_bug.cgi?id=107556
  fdo#107713 https://bugs.freedesktop.org/show_bug.cgi?id=107713
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
  fdo#107762 https://bugs.freedesktop.org/show_bug.cgi?id=107762
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (46 -> 43) ==

  Additional (2): fi-cnl-u fi-skl-caroline 
  Missing    (5): fi-bsw-kefka fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * IGT: IGT_4644 -> IGTPW_1850

  CI_DRM_4833: 75bb460b367a614d10b0fba220143bee42657d7e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1850: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1850/
  IGT_4644: 0b59bb3231ab481959528c5c7b3a98762772e1b0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+igt@gem_ctx_exec@invalid-context-blt
+igt@gem_ctx_exec@invalid-context-bsd
+igt@gem_ctx_exec@invalid-context-bsd1
+igt@gem_ctx_exec@invalid-context-bsd2
+igt@gem_ctx_exec@invalid-context-default
+igt@gem_ctx_exec@invalid-context-render
+igt@gem_ctx_exec@invalid-context-vebox
-igt@gem_ctx_bad_exec@blt
-igt@gem_ctx_bad_exec@bsd
-igt@gem_ctx_bad_exec@bsd1
-igt@gem_ctx_bad_exec@bsd2
-igt@gem_ctx_bad_exec@default
-igt@gem_ctx_bad_exec@render
-igt@gem_ctx_bad_exec@vebox
-igt@gem_ctx_exec@lrc-lite-restore

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1850/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* [igt-dev] ✓ Fi.CI.IGT: success for series starting with [i-g-t,1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec
  2018-09-17 15:46 ` [igt-dev] " Tvrtko Ursulin
                   ` (4 preceding siblings ...)
  (?)
@ 2018-09-17 20:02 ` Patchwork
  -1 siblings, 0 replies; 35+ messages in thread
From: Patchwork @ 2018-09-17 20:02 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: igt-dev

== Series Details ==

Series: series starting with [i-g-t,1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec
URL   : https://patchwork.freedesktop.org/series/49797/
State : success

== Summary ==

= CI Bug Log - changes from IGT_4644_full -> IGTPW_1850_full =

== Summary - WARNING ==

  Minor unknown changes coming with IGTPW_1850_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_1850_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/49797/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in IGTPW_1850_full:

  === IGT changes ===

    ==== Warnings ====

    igt@perf_pmu@rc6:
      shard-kbl:          SKIP -> PASS

    
== Known issues ==

  Here are the changes found in IGTPW_1850_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_suspend@shrink:
      shard-snb:          PASS -> INCOMPLETE (fdo#106886, fdo#105411)

    igt@gem_ctx_isolation@vcs1-s3:
      shard-kbl:          NOTRUN -> INCOMPLETE (fdo#103665)

    igt@kms_available_modes_crc@available_mode_test_crc:
      shard-snb:          PASS -> FAIL (fdo#106641)

    igt@kms_cursor_crc@cursor-128x128-suspend:
      shard-kbl:          PASS -> INCOMPLETE (fdo#103665)

    igt@kms_cursor_legacy@cursorb-vs-flipb-toggle:
      shard-glk:          PASS -> DMESG-WARN (fdo#105763, fdo#106538)

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-pri-shrfb-draw-mmap-wc:
      shard-glk:          PASS -> FAIL (fdo#103167)

    igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-cur-indfb-onoff:
      shard-glk:          PASS -> DMESG-FAIL (fdo#106538)

    igt@kms_properties@get_properties-sanity-atomic:
      shard-snb:          PASS -> INCOMPLETE (fdo#105411)

    igt@kms_setmode@basic:
      shard-kbl:          PASS -> FAIL (fdo#99912)

    
    ==== Possible fixes ====

    igt@gem_exec_big:
      shard-hsw:          TIMEOUT (fdo#107937) -> PASS

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-glk:          FAIL (fdo#105363) -> PASS

    igt@kms_frontbuffer_tracking@fbc-stridechange:
      shard-glk:          FAIL (fdo#103167) -> PASS

    igt@kms_rotation_crc@sprite-rotation-180:
      shard-snb:          FAIL (fdo#103925) -> PASS +1

    
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
  fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
  fdo#106641 https://bugs.freedesktop.org/show_bug.cgi?id=106641
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
  fdo#107937 https://bugs.freedesktop.org/show_bug.cgi?id=107937
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * IGT: IGT_4644 -> IGTPW_1850
    * Linux: CI_DRM_4827 -> CI_DRM_4833

  CI_DRM_4827: 8b1968f143e8bf65acf0ea6f7ce9120259043521 @ git://anongit.freedesktop.org/gfx-ci/linux
  CI_DRM_4833: 75bb460b367a614d10b0fba220143bee42657d7e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1850: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1850/
  IGT_4644: 0b59bb3231ab481959528c5c7b3a98762772e1b0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

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

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

* Re: [igt-dev] [PATCH i-g-t 1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec
  2018-09-17 15:52   ` Chris Wilson
@ 2018-09-18  9:38     ` Tvrtko Ursulin
  -1 siblings, 0 replies; 35+ messages in thread
From: Tvrtko Ursulin @ 2018-09-18  9:38 UTC (permalink / raw)
  To: Chris Wilson, Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx


On 17/09/2018 16:52, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-09-17 16:46:18)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Move a really small test that invalid context is rejected under the
>> gem_ctx_exec umbrella.
>>
>> v2:
>>   * And actually fix the test so it does what it claims. And add more
>>     variety in the invalid context id's it tests with. (Chris Wilson)
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   tests/Makefile.sources   |  1 -
>>   tests/gem_ctx_bad_exec.c | 60 ----------------------------------------
>>   tests/gem_ctx_exec.c     | 34 +++++++++++++++++++++++
>>   tests/meson.build        |  1 -
>>   4 files changed, 34 insertions(+), 62 deletions(-)
>>   delete mode 100644 tests/gem_ctx_bad_exec.c
>>
>> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
>> index c84933f1d971..269336ad3150 100644
>> --- a/tests/Makefile.sources
>> +++ b/tests/Makefile.sources
>> @@ -51,7 +51,6 @@ TESTS_progs = \
>>          gem_cs_prefetch \
>>          gem_cs_tlb \
>>          gem_ctx_bad_destroy \
>> -       gem_ctx_bad_exec \
>>          gem_ctx_create \
>>          gem_ctx_exec \
>>          gem_ctx_isolation \
>> diff --git a/tests/gem_ctx_bad_exec.c b/tests/gem_ctx_bad_exec.c
>> deleted file mode 100644
>> index e3ccc5be46a0..000000000000
>> --- a/tests/gem_ctx_bad_exec.c
>> +++ /dev/null
>> @@ -1,60 +0,0 @@
>> -/*
>> - * Copyright © 2012 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"
>> -
>> -IGT_TEST_DESCRIPTION("Test that context cannot be submitted to any ring");
>> -
>> -static int exec(int fd, unsigned ring)
>> -{
>> -       struct drm_i915_gem_execbuffer2 execbuf;
>> -       struct drm_i915_gem_exec_object2 obj;
>> -
>> -       memset(&obj, 0, sizeof(obj));
>> -       memset(&execbuf, 0, sizeof(execbuf));
>> -
>> -       execbuf.buffers_ptr = to_user_pointer(&obj);
>> -       execbuf.buffer_count = 1;
>> -       i915_execbuffer2_set_context_id(execbuf, 1);
>> -
>> -       return __gem_execbuf(fd, &execbuf);
>> -}
>> -
>> -igt_main
>> -{
>> -       const struct intel_execution_engine *e;
>> -       int fd = -1;
>> -
>> -       igt_skip_on_simulation();
>> -
>> -       igt_fixture
>> -               fd = drm_open_driver_render(DRIVER_INTEL);
>> -
>> -       for (e = intel_execution_engines; e->name; e++) {
>> -               igt_subtest_f("%s", e->name) {
>> -                       gem_require_ring(fd, e->exec_id | e->flags);
>> -                       igt_assert_eq(exec(fd, e->exec_id | e->flags), -ENOENT);
>> -               }
>> -       }
>> -}
>> diff --git a/tests/gem_ctx_exec.c b/tests/gem_ctx_exec.c
>> index 1f8ed64d4bd3..4ed6febe12b8 100644
>> --- a/tests/gem_ctx_exec.c
>> +++ b/tests/gem_ctx_exec.c
>> @@ -30,6 +30,7 @@
>>    */
>>   
>>   #include "igt.h"
>> +#include <limits.h>
>>   #include <unistd.h>
>>   #include <stdlib.h>
>>   #include <stdint.h>
>> @@ -142,6 +143,30 @@ static void big_exec(int fd, uint32_t handle, int ring)
>>          gem_sync(fd, handle);
>>   }
>>   
>> +static void invalid_context(int fd, unsigned ring, uint32_t handle)
>> +{
>> +       struct drm_i915_gem_exec_object2 obj = {
>> +               .handle = handle,
>> +       };
>> +       struct drm_i915_gem_execbuffer2 execbuf = {
>> +               .buffers_ptr = to_user_pointer(&obj),
>> +               .buffer_count = 1,
>> +               .flags = ring,
>> +       };
>> +       const uint64_t invalid[] = { 1, INT_MAX, UINT_MAX, LONG_MAX, ULONG_MAX,
>> +                                    LLONG_MAX, ULLONG_MAX };
> 
> The field is only a u32. Strange you didn't notice that amidst the
> absence of documentation ;)
> 
> Trim this array to suite, and
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Can I say it is testing the i915_execbuffer2_set_context_id as well by 
knowing underlying ABI field is 64-bit wide and keep the r-b? (No trash 
in unused part of rsvd1.)

Regards,

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

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

* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec
@ 2018-09-18  9:38     ` Tvrtko Ursulin
  0 siblings, 0 replies; 35+ messages in thread
From: Tvrtko Ursulin @ 2018-09-18  9:38 UTC (permalink / raw)
  To: Chris Wilson, Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx


On 17/09/2018 16:52, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-09-17 16:46:18)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Move a really small test that invalid context is rejected under the
>> gem_ctx_exec umbrella.
>>
>> v2:
>>   * And actually fix the test so it does what it claims. And add more
>>     variety in the invalid context id's it tests with. (Chris Wilson)
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   tests/Makefile.sources   |  1 -
>>   tests/gem_ctx_bad_exec.c | 60 ----------------------------------------
>>   tests/gem_ctx_exec.c     | 34 +++++++++++++++++++++++
>>   tests/meson.build        |  1 -
>>   4 files changed, 34 insertions(+), 62 deletions(-)
>>   delete mode 100644 tests/gem_ctx_bad_exec.c
>>
>> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
>> index c84933f1d971..269336ad3150 100644
>> --- a/tests/Makefile.sources
>> +++ b/tests/Makefile.sources
>> @@ -51,7 +51,6 @@ TESTS_progs = \
>>          gem_cs_prefetch \
>>          gem_cs_tlb \
>>          gem_ctx_bad_destroy \
>> -       gem_ctx_bad_exec \
>>          gem_ctx_create \
>>          gem_ctx_exec \
>>          gem_ctx_isolation \
>> diff --git a/tests/gem_ctx_bad_exec.c b/tests/gem_ctx_bad_exec.c
>> deleted file mode 100644
>> index e3ccc5be46a0..000000000000
>> --- a/tests/gem_ctx_bad_exec.c
>> +++ /dev/null
>> @@ -1,60 +0,0 @@
>> -/*
>> - * Copyright © 2012 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"
>> -
>> -IGT_TEST_DESCRIPTION("Test that context cannot be submitted to any ring");
>> -
>> -static int exec(int fd, unsigned ring)
>> -{
>> -       struct drm_i915_gem_execbuffer2 execbuf;
>> -       struct drm_i915_gem_exec_object2 obj;
>> -
>> -       memset(&obj, 0, sizeof(obj));
>> -       memset(&execbuf, 0, sizeof(execbuf));
>> -
>> -       execbuf.buffers_ptr = to_user_pointer(&obj);
>> -       execbuf.buffer_count = 1;
>> -       i915_execbuffer2_set_context_id(execbuf, 1);
>> -
>> -       return __gem_execbuf(fd, &execbuf);
>> -}
>> -
>> -igt_main
>> -{
>> -       const struct intel_execution_engine *e;
>> -       int fd = -1;
>> -
>> -       igt_skip_on_simulation();
>> -
>> -       igt_fixture
>> -               fd = drm_open_driver_render(DRIVER_INTEL);
>> -
>> -       for (e = intel_execution_engines; e->name; e++) {
>> -               igt_subtest_f("%s", e->name) {
>> -                       gem_require_ring(fd, e->exec_id | e->flags);
>> -                       igt_assert_eq(exec(fd, e->exec_id | e->flags), -ENOENT);
>> -               }
>> -       }
>> -}
>> diff --git a/tests/gem_ctx_exec.c b/tests/gem_ctx_exec.c
>> index 1f8ed64d4bd3..4ed6febe12b8 100644
>> --- a/tests/gem_ctx_exec.c
>> +++ b/tests/gem_ctx_exec.c
>> @@ -30,6 +30,7 @@
>>    */
>>   
>>   #include "igt.h"
>> +#include <limits.h>
>>   #include <unistd.h>
>>   #include <stdlib.h>
>>   #include <stdint.h>
>> @@ -142,6 +143,30 @@ static void big_exec(int fd, uint32_t handle, int ring)
>>          gem_sync(fd, handle);
>>   }
>>   
>> +static void invalid_context(int fd, unsigned ring, uint32_t handle)
>> +{
>> +       struct drm_i915_gem_exec_object2 obj = {
>> +               .handle = handle,
>> +       };
>> +       struct drm_i915_gem_execbuffer2 execbuf = {
>> +               .buffers_ptr = to_user_pointer(&obj),
>> +               .buffer_count = 1,
>> +               .flags = ring,
>> +       };
>> +       const uint64_t invalid[] = { 1, INT_MAX, UINT_MAX, LONG_MAX, ULONG_MAX,
>> +                                    LLONG_MAX, ULLONG_MAX };
> 
> The field is only a u32. Strange you didn't notice that amidst the
> absence of documentation ;)
> 
> Trim this array to suite, and
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Can I say it is testing the i915_execbuffer2_set_context_id as well by 
knowing underlying ABI field is 64-bit wide and keep the r-b? (No trash 
in unused part of rsvd1.)

Regards,

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

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

* Re: [igt-dev] [PATCH i-g-t 1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec
  2018-09-18  9:38     ` [Intel-gfx] " Tvrtko Ursulin
@ 2018-09-18  9:44       ` Chris Wilson
  -1 siblings, 0 replies; 35+ messages in thread
From: Chris Wilson @ 2018-09-18  9:44 UTC (permalink / raw)
  To: Tvrtko Ursulin, Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx

Quoting Tvrtko Ursulin (2018-09-18 10:38:44)
> 
> On 17/09/2018 16:52, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2018-09-17 16:46:18)
> >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>
> >> Move a really small test that invalid context is rejected under the
> >> gem_ctx_exec umbrella.
> >>
> >> v2:
> >>   * And actually fix the test so it does what it claims. And add more
> >>     variety in the invalid context id's it tests with. (Chris Wilson)
> >>
> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >> ---
> >>   tests/Makefile.sources   |  1 -
> >>   tests/gem_ctx_bad_exec.c | 60 ----------------------------------------
> >>   tests/gem_ctx_exec.c     | 34 +++++++++++++++++++++++
> >>   tests/meson.build        |  1 -
> >>   4 files changed, 34 insertions(+), 62 deletions(-)
> >>   delete mode 100644 tests/gem_ctx_bad_exec.c
> >>
> >> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> >> index c84933f1d971..269336ad3150 100644
> >> --- a/tests/Makefile.sources
> >> +++ b/tests/Makefile.sources
> >> @@ -51,7 +51,6 @@ TESTS_progs = \
> >>          gem_cs_prefetch \
> >>          gem_cs_tlb \
> >>          gem_ctx_bad_destroy \
> >> -       gem_ctx_bad_exec \
> >>          gem_ctx_create \
> >>          gem_ctx_exec \
> >>          gem_ctx_isolation \
> >> diff --git a/tests/gem_ctx_bad_exec.c b/tests/gem_ctx_bad_exec.c
> >> deleted file mode 100644
> >> index e3ccc5be46a0..000000000000
> >> --- a/tests/gem_ctx_bad_exec.c
> >> +++ /dev/null
> >> @@ -1,60 +0,0 @@
> >> -/*
> >> - * Copyright © 2012 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"
> >> -
> >> -IGT_TEST_DESCRIPTION("Test that context cannot be submitted to any ring");
> >> -
> >> -static int exec(int fd, unsigned ring)
> >> -{
> >> -       struct drm_i915_gem_execbuffer2 execbuf;
> >> -       struct drm_i915_gem_exec_object2 obj;
> >> -
> >> -       memset(&obj, 0, sizeof(obj));
> >> -       memset(&execbuf, 0, sizeof(execbuf));
> >> -
> >> -       execbuf.buffers_ptr = to_user_pointer(&obj);
> >> -       execbuf.buffer_count = 1;
> >> -       i915_execbuffer2_set_context_id(execbuf, 1);
> >> -
> >> -       return __gem_execbuf(fd, &execbuf);
> >> -}
> >> -
> >> -igt_main
> >> -{
> >> -       const struct intel_execution_engine *e;
> >> -       int fd = -1;
> >> -
> >> -       igt_skip_on_simulation();
> >> -
> >> -       igt_fixture
> >> -               fd = drm_open_driver_render(DRIVER_INTEL);
> >> -
> >> -       for (e = intel_execution_engines; e->name; e++) {
> >> -               igt_subtest_f("%s", e->name) {
> >> -                       gem_require_ring(fd, e->exec_id | e->flags);
> >> -                       igt_assert_eq(exec(fd, e->exec_id | e->flags), -ENOENT);
> >> -               }
> >> -       }
> >> -}
> >> diff --git a/tests/gem_ctx_exec.c b/tests/gem_ctx_exec.c
> >> index 1f8ed64d4bd3..4ed6febe12b8 100644
> >> --- a/tests/gem_ctx_exec.c
> >> +++ b/tests/gem_ctx_exec.c
> >> @@ -30,6 +30,7 @@
> >>    */
> >>   
> >>   #include "igt.h"
> >> +#include <limits.h>
> >>   #include <unistd.h>
> >>   #include <stdlib.h>
> >>   #include <stdint.h>
> >> @@ -142,6 +143,30 @@ static void big_exec(int fd, uint32_t handle, int ring)
> >>          gem_sync(fd, handle);
> >>   }
> >>   
> >> +static void invalid_context(int fd, unsigned ring, uint32_t handle)
> >> +{
> >> +       struct drm_i915_gem_exec_object2 obj = {
> >> +               .handle = handle,
> >> +       };
> >> +       struct drm_i915_gem_execbuffer2 execbuf = {
> >> +               .buffers_ptr = to_user_pointer(&obj),
> >> +               .buffer_count = 1,
> >> +               .flags = ring,
> >> +       };
> >> +       const uint64_t invalid[] = { 1, INT_MAX, UINT_MAX, LONG_MAX, ULONG_MAX,
> >> +                                    LLONG_MAX, ULLONG_MAX };
> > 
> > The field is only a u32. Strange you didn't notice that amidst the
> > absence of documentation ;)
> > 
> > Trim this array to suite, and
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Can I say it is testing the i915_execbuffer2_set_context_id as well by 
> knowing underlying ABI field is 64-bit wide and keep the r-b? (No trash 
> in unused part of rsvd1.)

The field isn't 64-bit wide, so the test would be misleading unfortunately.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec
@ 2018-09-18  9:44       ` Chris Wilson
  0 siblings, 0 replies; 35+ messages in thread
From: Chris Wilson @ 2018-09-18  9:44 UTC (permalink / raw)
  To: Tvrtko Ursulin, Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx

Quoting Tvrtko Ursulin (2018-09-18 10:38:44)
> 
> On 17/09/2018 16:52, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2018-09-17 16:46:18)
> >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>
> >> Move a really small test that invalid context is rejected under the
> >> gem_ctx_exec umbrella.
> >>
> >> v2:
> >>   * And actually fix the test so it does what it claims. And add more
> >>     variety in the invalid context id's it tests with. (Chris Wilson)
> >>
> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >> ---
> >>   tests/Makefile.sources   |  1 -
> >>   tests/gem_ctx_bad_exec.c | 60 ----------------------------------------
> >>   tests/gem_ctx_exec.c     | 34 +++++++++++++++++++++++
> >>   tests/meson.build        |  1 -
> >>   4 files changed, 34 insertions(+), 62 deletions(-)
> >>   delete mode 100644 tests/gem_ctx_bad_exec.c
> >>
> >> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> >> index c84933f1d971..269336ad3150 100644
> >> --- a/tests/Makefile.sources
> >> +++ b/tests/Makefile.sources
> >> @@ -51,7 +51,6 @@ TESTS_progs = \
> >>          gem_cs_prefetch \
> >>          gem_cs_tlb \
> >>          gem_ctx_bad_destroy \
> >> -       gem_ctx_bad_exec \
> >>          gem_ctx_create \
> >>          gem_ctx_exec \
> >>          gem_ctx_isolation \
> >> diff --git a/tests/gem_ctx_bad_exec.c b/tests/gem_ctx_bad_exec.c
> >> deleted file mode 100644
> >> index e3ccc5be46a0..000000000000
> >> --- a/tests/gem_ctx_bad_exec.c
> >> +++ /dev/null
> >> @@ -1,60 +0,0 @@
> >> -/*
> >> - * Copyright © 2012 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"
> >> -
> >> -IGT_TEST_DESCRIPTION("Test that context cannot be submitted to any ring");
> >> -
> >> -static int exec(int fd, unsigned ring)
> >> -{
> >> -       struct drm_i915_gem_execbuffer2 execbuf;
> >> -       struct drm_i915_gem_exec_object2 obj;
> >> -
> >> -       memset(&obj, 0, sizeof(obj));
> >> -       memset(&execbuf, 0, sizeof(execbuf));
> >> -
> >> -       execbuf.buffers_ptr = to_user_pointer(&obj);
> >> -       execbuf.buffer_count = 1;
> >> -       i915_execbuffer2_set_context_id(execbuf, 1);
> >> -
> >> -       return __gem_execbuf(fd, &execbuf);
> >> -}
> >> -
> >> -igt_main
> >> -{
> >> -       const struct intel_execution_engine *e;
> >> -       int fd = -1;
> >> -
> >> -       igt_skip_on_simulation();
> >> -
> >> -       igt_fixture
> >> -               fd = drm_open_driver_render(DRIVER_INTEL);
> >> -
> >> -       for (e = intel_execution_engines; e->name; e++) {
> >> -               igt_subtest_f("%s", e->name) {
> >> -                       gem_require_ring(fd, e->exec_id | e->flags);
> >> -                       igt_assert_eq(exec(fd, e->exec_id | e->flags), -ENOENT);
> >> -               }
> >> -       }
> >> -}
> >> diff --git a/tests/gem_ctx_exec.c b/tests/gem_ctx_exec.c
> >> index 1f8ed64d4bd3..4ed6febe12b8 100644
> >> --- a/tests/gem_ctx_exec.c
> >> +++ b/tests/gem_ctx_exec.c
> >> @@ -30,6 +30,7 @@
> >>    */
> >>   
> >>   #include "igt.h"
> >> +#include <limits.h>
> >>   #include <unistd.h>
> >>   #include <stdlib.h>
> >>   #include <stdint.h>
> >> @@ -142,6 +143,30 @@ static void big_exec(int fd, uint32_t handle, int ring)
> >>          gem_sync(fd, handle);
> >>   }
> >>   
> >> +static void invalid_context(int fd, unsigned ring, uint32_t handle)
> >> +{
> >> +       struct drm_i915_gem_exec_object2 obj = {
> >> +               .handle = handle,
> >> +       };
> >> +       struct drm_i915_gem_execbuffer2 execbuf = {
> >> +               .buffers_ptr = to_user_pointer(&obj),
> >> +               .buffer_count = 1,
> >> +               .flags = ring,
> >> +       };
> >> +       const uint64_t invalid[] = { 1, INT_MAX, UINT_MAX, LONG_MAX, ULONG_MAX,
> >> +                                    LLONG_MAX, ULLONG_MAX };
> > 
> > The field is only a u32. Strange you didn't notice that amidst the
> > absence of documentation ;)
> > 
> > Trim this array to suite, and
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Can I say it is testing the i915_execbuffer2_set_context_id as well by 
> knowing underlying ABI field is 64-bit wide and keep the r-b? (No trash 
> in unused part of rsvd1.)

The field isn't 64-bit wide, so the test would be misleading unfortunately.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [igt-dev] [PATCH i-g-t 1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec
  2018-09-18  9:44       ` [Intel-gfx] " Chris Wilson
@ 2018-09-18  9:59         ` Tvrtko Ursulin
  -1 siblings, 0 replies; 35+ messages in thread
From: Tvrtko Ursulin @ 2018-09-18  9:59 UTC (permalink / raw)
  To: Chris Wilson, Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx


On 18/09/2018 10:44, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-09-18 10:38:44)
>>
>> On 17/09/2018 16:52, Chris Wilson wrote:
>>> Quoting Tvrtko Ursulin (2018-09-17 16:46:18)
>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>
>>>> Move a really small test that invalid context is rejected under the
>>>> gem_ctx_exec umbrella.
>>>>
>>>> v2:
>>>>    * And actually fix the test so it does what it claims. And add more
>>>>      variety in the invalid context id's it tests with. (Chris Wilson)
>>>>
>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>> ---
>>>>    tests/Makefile.sources   |  1 -
>>>>    tests/gem_ctx_bad_exec.c | 60 ----------------------------------------
>>>>    tests/gem_ctx_exec.c     | 34 +++++++++++++++++++++++
>>>>    tests/meson.build        |  1 -
>>>>    4 files changed, 34 insertions(+), 62 deletions(-)
>>>>    delete mode 100644 tests/gem_ctx_bad_exec.c
>>>>
>>>> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
>>>> index c84933f1d971..269336ad3150 100644
>>>> --- a/tests/Makefile.sources
>>>> +++ b/tests/Makefile.sources
>>>> @@ -51,7 +51,6 @@ TESTS_progs = \
>>>>           gem_cs_prefetch \
>>>>           gem_cs_tlb \
>>>>           gem_ctx_bad_destroy \
>>>> -       gem_ctx_bad_exec \
>>>>           gem_ctx_create \
>>>>           gem_ctx_exec \
>>>>           gem_ctx_isolation \
>>>> diff --git a/tests/gem_ctx_bad_exec.c b/tests/gem_ctx_bad_exec.c
>>>> deleted file mode 100644
>>>> index e3ccc5be46a0..000000000000
>>>> --- a/tests/gem_ctx_bad_exec.c
>>>> +++ /dev/null
>>>> @@ -1,60 +0,0 @@
>>>> -/*
>>>> - * Copyright © 2012 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"
>>>> -
>>>> -IGT_TEST_DESCRIPTION("Test that context cannot be submitted to any ring");
>>>> -
>>>> -static int exec(int fd, unsigned ring)
>>>> -{
>>>> -       struct drm_i915_gem_execbuffer2 execbuf;
>>>> -       struct drm_i915_gem_exec_object2 obj;
>>>> -
>>>> -       memset(&obj, 0, sizeof(obj));
>>>> -       memset(&execbuf, 0, sizeof(execbuf));
>>>> -
>>>> -       execbuf.buffers_ptr = to_user_pointer(&obj);
>>>> -       execbuf.buffer_count = 1;
>>>> -       i915_execbuffer2_set_context_id(execbuf, 1);
>>>> -
>>>> -       return __gem_execbuf(fd, &execbuf);
>>>> -}
>>>> -
>>>> -igt_main
>>>> -{
>>>> -       const struct intel_execution_engine *e;
>>>> -       int fd = -1;
>>>> -
>>>> -       igt_skip_on_simulation();
>>>> -
>>>> -       igt_fixture
>>>> -               fd = drm_open_driver_render(DRIVER_INTEL);
>>>> -
>>>> -       for (e = intel_execution_engines; e->name; e++) {
>>>> -               igt_subtest_f("%s", e->name) {
>>>> -                       gem_require_ring(fd, e->exec_id | e->flags);
>>>> -                       igt_assert_eq(exec(fd, e->exec_id | e->flags), -ENOENT);
>>>> -               }
>>>> -       }
>>>> -}
>>>> diff --git a/tests/gem_ctx_exec.c b/tests/gem_ctx_exec.c
>>>> index 1f8ed64d4bd3..4ed6febe12b8 100644
>>>> --- a/tests/gem_ctx_exec.c
>>>> +++ b/tests/gem_ctx_exec.c
>>>> @@ -30,6 +30,7 @@
>>>>     */
>>>>    
>>>>    #include "igt.h"
>>>> +#include <limits.h>
>>>>    #include <unistd.h>
>>>>    #include <stdlib.h>
>>>>    #include <stdint.h>
>>>> @@ -142,6 +143,30 @@ static void big_exec(int fd, uint32_t handle, int ring)
>>>>           gem_sync(fd, handle);
>>>>    }
>>>>    
>>>> +static void invalid_context(int fd, unsigned ring, uint32_t handle)
>>>> +{
>>>> +       struct drm_i915_gem_exec_object2 obj = {
>>>> +               .handle = handle,
>>>> +       };
>>>> +       struct drm_i915_gem_execbuffer2 execbuf = {
>>>> +               .buffers_ptr = to_user_pointer(&obj),
>>>> +               .buffer_count = 1,
>>>> +               .flags = ring,
>>>> +       };
>>>> +       const uint64_t invalid[] = { 1, INT_MAX, UINT_MAX, LONG_MAX, ULONG_MAX,
>>>> +                                    LLONG_MAX, ULLONG_MAX };
>>>
>>> The field is only a u32. Strange you didn't notice that amidst the
>>> absence of documentation ;)
>>>
>>> Trim this array to suite, and
>>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>>
>> Can I say it is testing the i915_execbuffer2_set_context_id as well by
>> knowing underlying ABI field is 64-bit wide and keep the r-b? (No trash
>> in unused part of rsvd1.)
> 
> The field isn't 64-bit wide, so the test would be misleading unfortunately.

rsvd1? It is 64-bit in i915_drm.h I am looking at. And ABI is free to 
set it directly.

But true, it seems we are not checking for trash in upper 32-bits in 
execbuf paths.. :(

So this means regardless of what helper does the ABI is actually 64-bit. 
So test should actually bypass the helper I think and set rsvd1 directly 
I think. And keep the 64-bit invalid values.

Regards,

Tvrtko



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

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

* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t 1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec
@ 2018-09-18  9:59         ` Tvrtko Ursulin
  0 siblings, 0 replies; 35+ messages in thread
From: Tvrtko Ursulin @ 2018-09-18  9:59 UTC (permalink / raw)
  To: Chris Wilson, Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx


On 18/09/2018 10:44, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-09-18 10:38:44)
>>
>> On 17/09/2018 16:52, Chris Wilson wrote:
>>> Quoting Tvrtko Ursulin (2018-09-17 16:46:18)
>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>
>>>> Move a really small test that invalid context is rejected under the
>>>> gem_ctx_exec umbrella.
>>>>
>>>> v2:
>>>>    * And actually fix the test so it does what it claims. And add more
>>>>      variety in the invalid context id's it tests with. (Chris Wilson)
>>>>
>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>> ---
>>>>    tests/Makefile.sources   |  1 -
>>>>    tests/gem_ctx_bad_exec.c | 60 ----------------------------------------
>>>>    tests/gem_ctx_exec.c     | 34 +++++++++++++++++++++++
>>>>    tests/meson.build        |  1 -
>>>>    4 files changed, 34 insertions(+), 62 deletions(-)
>>>>    delete mode 100644 tests/gem_ctx_bad_exec.c
>>>>
>>>> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
>>>> index c84933f1d971..269336ad3150 100644
>>>> --- a/tests/Makefile.sources
>>>> +++ b/tests/Makefile.sources
>>>> @@ -51,7 +51,6 @@ TESTS_progs = \
>>>>           gem_cs_prefetch \
>>>>           gem_cs_tlb \
>>>>           gem_ctx_bad_destroy \
>>>> -       gem_ctx_bad_exec \
>>>>           gem_ctx_create \
>>>>           gem_ctx_exec \
>>>>           gem_ctx_isolation \
>>>> diff --git a/tests/gem_ctx_bad_exec.c b/tests/gem_ctx_bad_exec.c
>>>> deleted file mode 100644
>>>> index e3ccc5be46a0..000000000000
>>>> --- a/tests/gem_ctx_bad_exec.c
>>>> +++ /dev/null
>>>> @@ -1,60 +0,0 @@
>>>> -/*
>>>> - * Copyright © 2012 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"
>>>> -
>>>> -IGT_TEST_DESCRIPTION("Test that context cannot be submitted to any ring");
>>>> -
>>>> -static int exec(int fd, unsigned ring)
>>>> -{
>>>> -       struct drm_i915_gem_execbuffer2 execbuf;
>>>> -       struct drm_i915_gem_exec_object2 obj;
>>>> -
>>>> -       memset(&obj, 0, sizeof(obj));
>>>> -       memset(&execbuf, 0, sizeof(execbuf));
>>>> -
>>>> -       execbuf.buffers_ptr = to_user_pointer(&obj);
>>>> -       execbuf.buffer_count = 1;
>>>> -       i915_execbuffer2_set_context_id(execbuf, 1);
>>>> -
>>>> -       return __gem_execbuf(fd, &execbuf);
>>>> -}
>>>> -
>>>> -igt_main
>>>> -{
>>>> -       const struct intel_execution_engine *e;
>>>> -       int fd = -1;
>>>> -
>>>> -       igt_skip_on_simulation();
>>>> -
>>>> -       igt_fixture
>>>> -               fd = drm_open_driver_render(DRIVER_INTEL);
>>>> -
>>>> -       for (e = intel_execution_engines; e->name; e++) {
>>>> -               igt_subtest_f("%s", e->name) {
>>>> -                       gem_require_ring(fd, e->exec_id | e->flags);
>>>> -                       igt_assert_eq(exec(fd, e->exec_id | e->flags), -ENOENT);
>>>> -               }
>>>> -       }
>>>> -}
>>>> diff --git a/tests/gem_ctx_exec.c b/tests/gem_ctx_exec.c
>>>> index 1f8ed64d4bd3..4ed6febe12b8 100644
>>>> --- a/tests/gem_ctx_exec.c
>>>> +++ b/tests/gem_ctx_exec.c
>>>> @@ -30,6 +30,7 @@
>>>>     */
>>>>    
>>>>    #include "igt.h"
>>>> +#include <limits.h>
>>>>    #include <unistd.h>
>>>>    #include <stdlib.h>
>>>>    #include <stdint.h>
>>>> @@ -142,6 +143,30 @@ static void big_exec(int fd, uint32_t handle, int ring)
>>>>           gem_sync(fd, handle);
>>>>    }
>>>>    
>>>> +static void invalid_context(int fd, unsigned ring, uint32_t handle)
>>>> +{
>>>> +       struct drm_i915_gem_exec_object2 obj = {
>>>> +               .handle = handle,
>>>> +       };
>>>> +       struct drm_i915_gem_execbuffer2 execbuf = {
>>>> +               .buffers_ptr = to_user_pointer(&obj),
>>>> +               .buffer_count = 1,
>>>> +               .flags = ring,
>>>> +       };
>>>> +       const uint64_t invalid[] = { 1, INT_MAX, UINT_MAX, LONG_MAX, ULONG_MAX,
>>>> +                                    LLONG_MAX, ULLONG_MAX };
>>>
>>> The field is only a u32. Strange you didn't notice that amidst the
>>> absence of documentation ;)
>>>
>>> Trim this array to suite, and
>>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>>
>> Can I say it is testing the i915_execbuffer2_set_context_id as well by
>> knowing underlying ABI field is 64-bit wide and keep the r-b? (No trash
>> in unused part of rsvd1.)
> 
> The field isn't 64-bit wide, so the test would be misleading unfortunately.

rsvd1? It is 64-bit in i915_drm.h I am looking at. And ABI is free to 
set it directly.

But true, it seems we are not checking for trash in upper 32-bits in 
execbuf paths.. :(

So this means regardless of what helper does the ABI is actually 64-bit. 
So test should actually bypass the helper I think and set rsvd1 directly 
I think. And keep the 64-bit invalid values.

Regards,

Tvrtko



_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [igt-dev] [PATCH i-g-t 1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec
  2018-09-18  9:59         ` [igt-dev] [Intel-gfx] " Tvrtko Ursulin
@ 2018-09-18 10:02           ` Chris Wilson
  -1 siblings, 0 replies; 35+ messages in thread
From: Chris Wilson @ 2018-09-18 10:02 UTC (permalink / raw)
  To: Tvrtko Ursulin, Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx

Quoting Tvrtko Ursulin (2018-09-18 10:59:11)
> 
> On 18/09/2018 10:44, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2018-09-18 10:38:44)
> >>
> >> On 17/09/2018 16:52, Chris Wilson wrote:
> >>> Quoting Tvrtko Ursulin (2018-09-17 16:46:18)
> >>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>>
> >>>> Move a really small test that invalid context is rejected under the
> >>>> gem_ctx_exec umbrella.
> >>>>
> >>>> v2:
> >>>>    * And actually fix the test so it does what it claims. And add more
> >>>>      variety in the invalid context id's it tests with. (Chris Wilson)
> >>>>
> >>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>> ---
> >>>>    tests/Makefile.sources   |  1 -
> >>>>    tests/gem_ctx_bad_exec.c | 60 ----------------------------------------
> >>>>    tests/gem_ctx_exec.c     | 34 +++++++++++++++++++++++
> >>>>    tests/meson.build        |  1 -
> >>>>    4 files changed, 34 insertions(+), 62 deletions(-)
> >>>>    delete mode 100644 tests/gem_ctx_bad_exec.c
> >>>>
> >>>> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> >>>> index c84933f1d971..269336ad3150 100644
> >>>> --- a/tests/Makefile.sources
> >>>> +++ b/tests/Makefile.sources
> >>>> @@ -51,7 +51,6 @@ TESTS_progs = \
> >>>>           gem_cs_prefetch \
> >>>>           gem_cs_tlb \
> >>>>           gem_ctx_bad_destroy \
> >>>> -       gem_ctx_bad_exec \
> >>>>           gem_ctx_create \
> >>>>           gem_ctx_exec \
> >>>>           gem_ctx_isolation \
> >>>> diff --git a/tests/gem_ctx_bad_exec.c b/tests/gem_ctx_bad_exec.c
> >>>> deleted file mode 100644
> >>>> index e3ccc5be46a0..000000000000
> >>>> --- a/tests/gem_ctx_bad_exec.c
> >>>> +++ /dev/null
> >>>> @@ -1,60 +0,0 @@
> >>>> -/*
> >>>> - * Copyright © 2012 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"
> >>>> -
> >>>> -IGT_TEST_DESCRIPTION("Test that context cannot be submitted to any ring");
> >>>> -
> >>>> -static int exec(int fd, unsigned ring)
> >>>> -{
> >>>> -       struct drm_i915_gem_execbuffer2 execbuf;
> >>>> -       struct drm_i915_gem_exec_object2 obj;
> >>>> -
> >>>> -       memset(&obj, 0, sizeof(obj));
> >>>> -       memset(&execbuf, 0, sizeof(execbuf));
> >>>> -
> >>>> -       execbuf.buffers_ptr = to_user_pointer(&obj);
> >>>> -       execbuf.buffer_count = 1;
> >>>> -       i915_execbuffer2_set_context_id(execbuf, 1);
> >>>> -
> >>>> -       return __gem_execbuf(fd, &execbuf);
> >>>> -}
> >>>> -
> >>>> -igt_main
> >>>> -{
> >>>> -       const struct intel_execution_engine *e;
> >>>> -       int fd = -1;
> >>>> -
> >>>> -       igt_skip_on_simulation();
> >>>> -
> >>>> -       igt_fixture
> >>>> -               fd = drm_open_driver_render(DRIVER_INTEL);
> >>>> -
> >>>> -       for (e = intel_execution_engines; e->name; e++) {
> >>>> -               igt_subtest_f("%s", e->name) {
> >>>> -                       gem_require_ring(fd, e->exec_id | e->flags);
> >>>> -                       igt_assert_eq(exec(fd, e->exec_id | e->flags), -ENOENT);
> >>>> -               }
> >>>> -       }
> >>>> -}
> >>>> diff --git a/tests/gem_ctx_exec.c b/tests/gem_ctx_exec.c
> >>>> index 1f8ed64d4bd3..4ed6febe12b8 100644
> >>>> --- a/tests/gem_ctx_exec.c
> >>>> +++ b/tests/gem_ctx_exec.c
> >>>> @@ -30,6 +30,7 @@
> >>>>     */
> >>>>    
> >>>>    #include "igt.h"
> >>>> +#include <limits.h>
> >>>>    #include <unistd.h>
> >>>>    #include <stdlib.h>
> >>>>    #include <stdint.h>
> >>>> @@ -142,6 +143,30 @@ static void big_exec(int fd, uint32_t handle, int ring)
> >>>>           gem_sync(fd, handle);
> >>>>    }
> >>>>    
> >>>> +static void invalid_context(int fd, unsigned ring, uint32_t handle)
> >>>> +{
> >>>> +       struct drm_i915_gem_exec_object2 obj = {
> >>>> +               .handle = handle,
> >>>> +       };
> >>>> +       struct drm_i915_gem_execbuffer2 execbuf = {
> >>>> +               .buffers_ptr = to_user_pointer(&obj),
> >>>> +               .buffer_count = 1,
> >>>> +               .flags = ring,
> >>>> +       };
> >>>> +       const uint64_t invalid[] = { 1, INT_MAX, UINT_MAX, LONG_MAX, ULONG_MAX,
> >>>> +                                    LLONG_MAX, ULLONG_MAX };
> >>>
> >>> The field is only a u32. Strange you didn't notice that amidst the
> >>> absence of documentation ;)
> >>>
> >>> Trim this array to suite, and
> >>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> >>
> >> Can I say it is testing the i915_execbuffer2_set_context_id as well by
> >> knowing underlying ABI field is 64-bit wide and keep the r-b? (No trash
> >> in unused part of rsvd1.)
> > 
> > The field isn't 64-bit wide, so the test would be misleading unfortunately.
> 
> rsvd1? It is 64-bit in i915_drm.h I am looking at. And ABI is free to 
> set it directly.
> 
> But true, it seems we are not checking for trash in upper 32-bits in 
> execbuf paths.. :(
> 
> So this means regardless of what helper does the ABI is actually 64-bit. 
> So test should actually bypass the helper I think and set rsvd1 directly 
> I think. And keep the 64-bit invalid values.

We are not checking 64b. Think of one value that invalidates the test.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t 1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec
@ 2018-09-18 10:02           ` Chris Wilson
  0 siblings, 0 replies; 35+ messages in thread
From: Chris Wilson @ 2018-09-18 10:02 UTC (permalink / raw)
  To: Tvrtko Ursulin, Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx

Quoting Tvrtko Ursulin (2018-09-18 10:59:11)
> 
> On 18/09/2018 10:44, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2018-09-18 10:38:44)
> >>
> >> On 17/09/2018 16:52, Chris Wilson wrote:
> >>> Quoting Tvrtko Ursulin (2018-09-17 16:46:18)
> >>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>>
> >>>> Move a really small test that invalid context is rejected under the
> >>>> gem_ctx_exec umbrella.
> >>>>
> >>>> v2:
> >>>>    * And actually fix the test so it does what it claims. And add more
> >>>>      variety in the invalid context id's it tests with. (Chris Wilson)
> >>>>
> >>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>> ---
> >>>>    tests/Makefile.sources   |  1 -
> >>>>    tests/gem_ctx_bad_exec.c | 60 ----------------------------------------
> >>>>    tests/gem_ctx_exec.c     | 34 +++++++++++++++++++++++
> >>>>    tests/meson.build        |  1 -
> >>>>    4 files changed, 34 insertions(+), 62 deletions(-)
> >>>>    delete mode 100644 tests/gem_ctx_bad_exec.c
> >>>>
> >>>> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> >>>> index c84933f1d971..269336ad3150 100644
> >>>> --- a/tests/Makefile.sources
> >>>> +++ b/tests/Makefile.sources
> >>>> @@ -51,7 +51,6 @@ TESTS_progs = \
> >>>>           gem_cs_prefetch \
> >>>>           gem_cs_tlb \
> >>>>           gem_ctx_bad_destroy \
> >>>> -       gem_ctx_bad_exec \
> >>>>           gem_ctx_create \
> >>>>           gem_ctx_exec \
> >>>>           gem_ctx_isolation \
> >>>> diff --git a/tests/gem_ctx_bad_exec.c b/tests/gem_ctx_bad_exec.c
> >>>> deleted file mode 100644
> >>>> index e3ccc5be46a0..000000000000
> >>>> --- a/tests/gem_ctx_bad_exec.c
> >>>> +++ /dev/null
> >>>> @@ -1,60 +0,0 @@
> >>>> -/*
> >>>> - * Copyright © 2012 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"
> >>>> -
> >>>> -IGT_TEST_DESCRIPTION("Test that context cannot be submitted to any ring");
> >>>> -
> >>>> -static int exec(int fd, unsigned ring)
> >>>> -{
> >>>> -       struct drm_i915_gem_execbuffer2 execbuf;
> >>>> -       struct drm_i915_gem_exec_object2 obj;
> >>>> -
> >>>> -       memset(&obj, 0, sizeof(obj));
> >>>> -       memset(&execbuf, 0, sizeof(execbuf));
> >>>> -
> >>>> -       execbuf.buffers_ptr = to_user_pointer(&obj);
> >>>> -       execbuf.buffer_count = 1;
> >>>> -       i915_execbuffer2_set_context_id(execbuf, 1);
> >>>> -
> >>>> -       return __gem_execbuf(fd, &execbuf);
> >>>> -}
> >>>> -
> >>>> -igt_main
> >>>> -{
> >>>> -       const struct intel_execution_engine *e;
> >>>> -       int fd = -1;
> >>>> -
> >>>> -       igt_skip_on_simulation();
> >>>> -
> >>>> -       igt_fixture
> >>>> -               fd = drm_open_driver_render(DRIVER_INTEL);
> >>>> -
> >>>> -       for (e = intel_execution_engines; e->name; e++) {
> >>>> -               igt_subtest_f("%s", e->name) {
> >>>> -                       gem_require_ring(fd, e->exec_id | e->flags);
> >>>> -                       igt_assert_eq(exec(fd, e->exec_id | e->flags), -ENOENT);
> >>>> -               }
> >>>> -       }
> >>>> -}
> >>>> diff --git a/tests/gem_ctx_exec.c b/tests/gem_ctx_exec.c
> >>>> index 1f8ed64d4bd3..4ed6febe12b8 100644
> >>>> --- a/tests/gem_ctx_exec.c
> >>>> +++ b/tests/gem_ctx_exec.c
> >>>> @@ -30,6 +30,7 @@
> >>>>     */
> >>>>    
> >>>>    #include "igt.h"
> >>>> +#include <limits.h>
> >>>>    #include <unistd.h>
> >>>>    #include <stdlib.h>
> >>>>    #include <stdint.h>
> >>>> @@ -142,6 +143,30 @@ static void big_exec(int fd, uint32_t handle, int ring)
> >>>>           gem_sync(fd, handle);
> >>>>    }
> >>>>    
> >>>> +static void invalid_context(int fd, unsigned ring, uint32_t handle)
> >>>> +{
> >>>> +       struct drm_i915_gem_exec_object2 obj = {
> >>>> +               .handle = handle,
> >>>> +       };
> >>>> +       struct drm_i915_gem_execbuffer2 execbuf = {
> >>>> +               .buffers_ptr = to_user_pointer(&obj),
> >>>> +               .buffer_count = 1,
> >>>> +               .flags = ring,
> >>>> +       };
> >>>> +       const uint64_t invalid[] = { 1, INT_MAX, UINT_MAX, LONG_MAX, ULONG_MAX,
> >>>> +                                    LLONG_MAX, ULLONG_MAX };
> >>>
> >>> The field is only a u32. Strange you didn't notice that amidst the
> >>> absence of documentation ;)
> >>>
> >>> Trim this array to suite, and
> >>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> >>
> >> Can I say it is testing the i915_execbuffer2_set_context_id as well by
> >> knowing underlying ABI field is 64-bit wide and keep the r-b? (No trash
> >> in unused part of rsvd1.)
> > 
> > The field isn't 64-bit wide, so the test would be misleading unfortunately.
> 
> rsvd1? It is 64-bit in i915_drm.h I am looking at. And ABI is free to 
> set it directly.
> 
> But true, it seems we are not checking for trash in upper 32-bits in 
> execbuf paths.. :(
> 
> So this means regardless of what helper does the ABI is actually 64-bit. 
> So test should actually bypass the helper I think and set rsvd1 directly 
> I think. And keep the 64-bit invalid values.

We are not checking 64b. Think of one value that invalidates the test.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [igt-dev] [PATCH i-g-t 1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec
  2018-09-18 10:02           ` [igt-dev] [Intel-gfx] " Chris Wilson
@ 2018-09-18 10:03             ` Chris Wilson
  -1 siblings, 0 replies; 35+ messages in thread
From: Chris Wilson @ 2018-09-18 10:03 UTC (permalink / raw)
  To: Tvrtko Ursulin, Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx

Quoting Chris Wilson (2018-09-18 11:02:09)
> Quoting Tvrtko Ursulin (2018-09-18 10:59:11)
> > 
> > On 18/09/2018 10:44, Chris Wilson wrote:
> > > Quoting Tvrtko Ursulin (2018-09-18 10:38:44)
> > >> Can I say it is testing the i915_execbuffer2_set_context_id as well by
> > >> knowing underlying ABI field is 64-bit wide and keep the r-b? (No trash
> > >> in unused part of rsvd1.)
> > > 
> > > The field isn't 64-bit wide, so the test would be misleading unfortunately.
> > 
> > rsvd1? It is 64-bit in i915_drm.h I am looking at. And ABI is free to 
> > set it directly.
> > 
> > But true, it seems we are not checking for trash in upper 32-bits in 
> > execbuf paths.. :(
> > 
> > So this means regardless of what helper does the ABI is actually 64-bit. 
> > So test should actually bypass the helper I think and set rsvd1 directly 
> > I think. And keep the 64-bit invalid values.
> 
> We are not checking 64b. Think of one value that invalidates the test.

We can construct more values that would fail if we create a few contexts
and destroy them testing that that are invalid after use.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t 1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec
@ 2018-09-18 10:03             ` Chris Wilson
  0 siblings, 0 replies; 35+ messages in thread
From: Chris Wilson @ 2018-09-18 10:03 UTC (permalink / raw)
  To: Tvrtko Ursulin, Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx

Quoting Chris Wilson (2018-09-18 11:02:09)
> Quoting Tvrtko Ursulin (2018-09-18 10:59:11)
> > 
> > On 18/09/2018 10:44, Chris Wilson wrote:
> > > Quoting Tvrtko Ursulin (2018-09-18 10:38:44)
> > >> Can I say it is testing the i915_execbuffer2_set_context_id as well by
> > >> knowing underlying ABI field is 64-bit wide and keep the r-b? (No trash
> > >> in unused part of rsvd1.)
> > > 
> > > The field isn't 64-bit wide, so the test would be misleading unfortunately.
> > 
> > rsvd1? It is 64-bit in i915_drm.h I am looking at. And ABI is free to 
> > set it directly.
> > 
> > But true, it seems we are not checking for trash in upper 32-bits in 
> > execbuf paths.. :(
> > 
> > So this means regardless of what helper does the ABI is actually 64-bit. 
> > So test should actually bypass the helper I think and set rsvd1 directly 
> > I think. And keep the 64-bit invalid values.
> 
> We are not checking 64b. Think of one value that invalidates the test.

We can construct more values that would fail if we create a few contexts
and destroy them testing that that are invalid after use.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [igt-dev] [PATCH i-g-t 1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec
  2018-09-18 10:03             ` [igt-dev] [Intel-gfx] " Chris Wilson
@ 2018-09-18 10:33               ` Tvrtko Ursulin
  -1 siblings, 0 replies; 35+ messages in thread
From: Tvrtko Ursulin @ 2018-09-18 10:33 UTC (permalink / raw)
  To: Chris Wilson, Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx


On 18/09/2018 11:03, Chris Wilson wrote:
> Quoting Chris Wilson (2018-09-18 11:02:09)
>> Quoting Tvrtko Ursulin (2018-09-18 10:59:11)
>>>
>>> On 18/09/2018 10:44, Chris Wilson wrote:
>>>> Quoting Tvrtko Ursulin (2018-09-18 10:38:44)
>>>>> Can I say it is testing the i915_execbuffer2_set_context_id as well by
>>>>> knowing underlying ABI field is 64-bit wide and keep the r-b? (No trash
>>>>> in unused part of rsvd1.)
>>>>
>>>> The field isn't 64-bit wide, so the test would be misleading unfortunately.
>>>
>>> rsvd1? It is 64-bit in i915_drm.h I am looking at. And ABI is free to
>>> set it directly.
>>>
>>> But true, it seems we are not checking for trash in upper 32-bits in
>>> execbuf paths.. :(
>>>
>>> So this means regardless of what helper does the ABI is actually 64-bit.
>>> So test should actually bypass the helper I think and set rsvd1 directly
>>> I think. And keep the 64-bit invalid values.
>>
>> We are not checking 64b. Think of one value that invalidates the test.

eb.rsdvd = ctx << 32; always gives you the default context?

But is ABI the macro helper or the comment in i915_drm.h against rsvd1?

Probably the main problem is that we forgot to check for trash in upper 
bits..

Or in other words, what do you want to see here in terms of invalid 
values and helper or not? Stick to 32-bits just because that's where we 
are now with the historical implementation?

> We can construct more values that would fail if we create a few contexts
> and destroy them testing that that are invalid after use.

Hello scope creep, okay, I'll add it.

Regards,

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

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

* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec
@ 2018-09-18 10:33               ` Tvrtko Ursulin
  0 siblings, 0 replies; 35+ messages in thread
From: Tvrtko Ursulin @ 2018-09-18 10:33 UTC (permalink / raw)
  To: Chris Wilson, Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx


On 18/09/2018 11:03, Chris Wilson wrote:
> Quoting Chris Wilson (2018-09-18 11:02:09)
>> Quoting Tvrtko Ursulin (2018-09-18 10:59:11)
>>>
>>> On 18/09/2018 10:44, Chris Wilson wrote:
>>>> Quoting Tvrtko Ursulin (2018-09-18 10:38:44)
>>>>> Can I say it is testing the i915_execbuffer2_set_context_id as well by
>>>>> knowing underlying ABI field is 64-bit wide and keep the r-b? (No trash
>>>>> in unused part of rsvd1.)
>>>>
>>>> The field isn't 64-bit wide, so the test would be misleading unfortunately.
>>>
>>> rsvd1? It is 64-bit in i915_drm.h I am looking at. And ABI is free to
>>> set it directly.
>>>
>>> But true, it seems we are not checking for trash in upper 32-bits in
>>> execbuf paths.. :(
>>>
>>> So this means regardless of what helper does the ABI is actually 64-bit.
>>> So test should actually bypass the helper I think and set rsvd1 directly
>>> I think. And keep the 64-bit invalid values.
>>
>> We are not checking 64b. Think of one value that invalidates the test.

eb.rsdvd = ctx << 32; always gives you the default context?

But is ABI the macro helper or the comment in i915_drm.h against rsvd1?

Probably the main problem is that we forgot to check for trash in upper 
bits..

Or in other words, what do you want to see here in terms of invalid 
values and helper or not? Stick to 32-bits just because that's where we 
are now with the historical implementation?

> We can construct more values that would fail if we create a few contexts
> and destroy them testing that that are invalid after use.

Hello scope creep, okay, I'll add it.

Regards,

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

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

* Re: [igt-dev] [PATCH i-g-t 1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec
  2018-09-18 10:33               ` [Intel-gfx] " Tvrtko Ursulin
@ 2018-09-18 10:45                 ` Chris Wilson
  -1 siblings, 0 replies; 35+ messages in thread
From: Chris Wilson @ 2018-09-18 10:45 UTC (permalink / raw)
  To: Tvrtko Ursulin, Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx

Quoting Tvrtko Ursulin (2018-09-18 11:33:14)
> 
> On 18/09/2018 11:03, Chris Wilson wrote:
> > Quoting Chris Wilson (2018-09-18 11:02:09)
> >> Quoting Tvrtko Ursulin (2018-09-18 10:59:11)
> >>>
> >>> On 18/09/2018 10:44, Chris Wilson wrote:
> >>>> Quoting Tvrtko Ursulin (2018-09-18 10:38:44)
> >>>>> Can I say it is testing the i915_execbuffer2_set_context_id as well by
> >>>>> knowing underlying ABI field is 64-bit wide and keep the r-b? (No trash
> >>>>> in unused part of rsvd1.)
> >>>>
> >>>> The field isn't 64-bit wide, so the test would be misleading unfortunately.
> >>>
> >>> rsvd1? It is 64-bit in i915_drm.h I am looking at. And ABI is free to
> >>> set it directly.
> >>>
> >>> But true, it seems we are not checking for trash in upper 32-bits in
> >>> execbuf paths.. :(
> >>>
> >>> So this means regardless of what helper does the ABI is actually 64-bit.
> >>> So test should actually bypass the helper I think and set rsvd1 directly
> >>> I think. And keep the 64-bit invalid values.
> >>
> >> We are not checking 64b. Think of one value that invalidates the test.
> 
> eb.rsdvd = ctx << 32; always gives you the default context?
> 
> But is ABI the macro helper or the comment in i915_drm.h against rsvd1?
> 
> Probably the main problem is that we forgot to check for trash in upper 
> bits..
> 
> Or in other words, what do you want to see here in terms of invalid 
> values and helper or not? Stick to 32-bits just because that's where we 
> are now with the historical implementation?

The context id ABI has always been u32, and that's the field under test.

> > We can construct more values that would fail if we create a few contexts
> > and destroy them testing that that are invalid after use.
> 
> Hello scope creep, okay, I'll add it.

The test is "invalid context id" :-p Feature creep would be identifying
that we have several tests that poke at IDRs through the ABI that could
be refactored into a framework for light fuzzing.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t 1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec
@ 2018-09-18 10:45                 ` Chris Wilson
  0 siblings, 0 replies; 35+ messages in thread
From: Chris Wilson @ 2018-09-18 10:45 UTC (permalink / raw)
  To: Tvrtko Ursulin, Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx

Quoting Tvrtko Ursulin (2018-09-18 11:33:14)
> 
> On 18/09/2018 11:03, Chris Wilson wrote:
> > Quoting Chris Wilson (2018-09-18 11:02:09)
> >> Quoting Tvrtko Ursulin (2018-09-18 10:59:11)
> >>>
> >>> On 18/09/2018 10:44, Chris Wilson wrote:
> >>>> Quoting Tvrtko Ursulin (2018-09-18 10:38:44)
> >>>>> Can I say it is testing the i915_execbuffer2_set_context_id as well by
> >>>>> knowing underlying ABI field is 64-bit wide and keep the r-b? (No trash
> >>>>> in unused part of rsvd1.)
> >>>>
> >>>> The field isn't 64-bit wide, so the test would be misleading unfortunately.
> >>>
> >>> rsvd1? It is 64-bit in i915_drm.h I am looking at. And ABI is free to
> >>> set it directly.
> >>>
> >>> But true, it seems we are not checking for trash in upper 32-bits in
> >>> execbuf paths.. :(
> >>>
> >>> So this means regardless of what helper does the ABI is actually 64-bit.
> >>> So test should actually bypass the helper I think and set rsvd1 directly
> >>> I think. And keep the 64-bit invalid values.
> >>
> >> We are not checking 64b. Think of one value that invalidates the test.
> 
> eb.rsdvd = ctx << 32; always gives you the default context?
> 
> But is ABI the macro helper or the comment in i915_drm.h against rsvd1?
> 
> Probably the main problem is that we forgot to check for trash in upper 
> bits..
> 
> Or in other words, what do you want to see here in terms of invalid 
> values and helper or not? Stick to 32-bits just because that's where we 
> are now with the historical implementation?

The context id ABI has always been u32, and that's the field under test.

> > We can construct more values that would fail if we create a few contexts
> > and destroy them testing that that are invalid after use.
> 
> Hello scope creep, okay, I'll add it.

The test is "invalid context id" :-p Feature creep would be identifying
that we have several tests that poke at IDRs through the ABI that could
be refactored into a framework for light fuzzing.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* [PATCH i-g-t v3 1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec
  2018-09-18 10:45                 ` [igt-dev] [Intel-gfx] " Chris Wilson
@ 2018-09-18 10:59                   ` Tvrtko Ursulin
  -1 siblings, 0 replies; 35+ messages in thread
From: Tvrtko Ursulin @ 2018-09-18 10:59 UTC (permalink / raw)
  To: igt-dev; +Cc: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Move a really small test that invalid context is rejected under the
gem_ctx_exec umbrella.

v2:
 * And actually fix the test so it does what it claims. And add more
   variety in the invalid context id's it tests with. (Chris Wilson)

v3:
 * Rename the test as basic.
 * Limit to 32-bit. (Chris Wilson)
 * Add previously valid but closed context id to the test. (Chris Wilson)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 tests/Makefile.sources   |  1 -
 tests/gem_ctx_bad_exec.c | 60 ----------------------------------------
 tests/gem_ctx_exec.c     | 38 +++++++++++++++++++++++++
 tests/meson.build        |  1 -
 4 files changed, 38 insertions(+), 62 deletions(-)
 delete mode 100644 tests/gem_ctx_bad_exec.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index c84933f1d971..269336ad3150 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -51,7 +51,6 @@ TESTS_progs = \
 	gem_cs_prefetch \
 	gem_cs_tlb \
 	gem_ctx_bad_destroy \
-	gem_ctx_bad_exec \
 	gem_ctx_create \
 	gem_ctx_exec \
 	gem_ctx_isolation \
diff --git a/tests/gem_ctx_bad_exec.c b/tests/gem_ctx_bad_exec.c
deleted file mode 100644
index e3ccc5be46a0..000000000000
--- a/tests/gem_ctx_bad_exec.c
+++ /dev/null
@@ -1,60 +0,0 @@
-/*
- * Copyright © 2012 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"
-
-IGT_TEST_DESCRIPTION("Test that context cannot be submitted to any ring");
-
-static int exec(int fd, unsigned ring)
-{
-	struct drm_i915_gem_execbuffer2 execbuf;
-	struct drm_i915_gem_exec_object2 obj;
-
-	memset(&obj, 0, sizeof(obj));
-	memset(&execbuf, 0, sizeof(execbuf));
-
-	execbuf.buffers_ptr = to_user_pointer(&obj);
-	execbuf.buffer_count = 1;
-	i915_execbuffer2_set_context_id(execbuf, 1);
-
-	return __gem_execbuf(fd, &execbuf);
-}
-
-igt_main
-{
-	const struct intel_execution_engine *e;
-	int fd = -1;
-
-	igt_skip_on_simulation();
-
-	igt_fixture
-		fd = drm_open_driver_render(DRIVER_INTEL);
-
-	for (e = intel_execution_engines; e->name; e++) {
-		igt_subtest_f("%s", e->name) {
-			gem_require_ring(fd, e->exec_id | e->flags);
-			igt_assert_eq(exec(fd, e->exec_id | e->flags), -ENOENT);
-		}
-	}
-}
diff --git a/tests/gem_ctx_exec.c b/tests/gem_ctx_exec.c
index 1f8ed64d4bd3..1c6c18eddc20 100644
--- a/tests/gem_ctx_exec.c
+++ b/tests/gem_ctx_exec.c
@@ -30,6 +30,7 @@
  */
 
 #include "igt.h"
+#include <limits.h>
 #include <unistd.h>
 #include <stdlib.h>
 #include <stdint.h>
@@ -142,6 +143,34 @@ static void big_exec(int fd, uint32_t handle, int ring)
 	gem_sync(fd, handle);
 }
 
+static void invalid_context(int fd, unsigned ring, uint32_t handle)
+{
+	struct drm_i915_gem_exec_object2 obj = {
+		.handle = handle,
+	};
+	struct drm_i915_gem_execbuffer2 execbuf = {
+		.buffers_ptr = to_user_pointer(&obj),
+		.buffer_count = 1,
+		.flags = ring,
+	};
+	uint32_t invalid[] = { 0, 1, INT_MAX, UINT_MAX };
+	int i;
+
+	invalid[0] = gem_context_create(fd);
+
+	/* Verify everything works. */
+	i915_execbuffer2_set_context_id(execbuf, invalid[0]);
+	gem_execbuf(fd, &execbuf);
+
+	gem_context_destroy(fd, invalid[0]);
+
+	/* Go through the non-existent context id's. */
+	for (i = 0; i < ARRAY_SIZE(invalid); i++) {
+		i915_execbuffer2_set_context_id(execbuf, invalid[i]);
+		igt_assert_eq(__gem_execbuf(fd, &execbuf), -ENOENT);
+	}
+}
+
 uint32_t handle;
 uint32_t batch[2] = {0, MI_BATCH_BUFFER_END};
 uint32_t ctx_id, ctx_id2;
@@ -149,6 +178,8 @@ int fd;
 
 igt_main
 {
+	const struct intel_execution_engine *e;
+
 	igt_fixture {
 		fd = drm_open_driver_render(DRIVER_INTEL);
 		igt_require_gem(fd);
@@ -174,6 +205,13 @@ igt_main
 		gem_sync(fd, handle);
 	}
 
+	for (e = intel_execution_engines; e->name; e++) {
+		igt_subtest_f("basic-invalid-context-%s", e->name) {
+			gem_require_ring(fd, e->exec_id | e->flags);
+			invalid_context(fd, e->exec_id | e->flags, handle);
+		}
+	}
+
 	igt_subtest("eviction")
 		big_exec(fd, handle, I915_EXEC_RENDER);
 
diff --git a/tests/meson.build b/tests/meson.build
index 17deb945ec95..d22d59e0837d 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -28,7 +28,6 @@ test_progs = [
 	'gem_cs_prefetch',
 	'gem_cs_tlb',
 	'gem_ctx_bad_destroy',
-	'gem_ctx_bad_exec',
 	'gem_ctx_create',
 	'gem_ctx_exec',
 	'gem_ctx_isolation',
-- 
2.17.1

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

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

* [igt-dev] [PATCH i-g-t v3 1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec
@ 2018-09-18 10:59                   ` Tvrtko Ursulin
  0 siblings, 0 replies; 35+ messages in thread
From: Tvrtko Ursulin @ 2018-09-18 10:59 UTC (permalink / raw)
  To: igt-dev; +Cc: Intel-gfx, Tvrtko Ursulin

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Move a really small test that invalid context is rejected under the
gem_ctx_exec umbrella.

v2:
 * And actually fix the test so it does what it claims. And add more
   variety in the invalid context id's it tests with. (Chris Wilson)

v3:
 * Rename the test as basic.
 * Limit to 32-bit. (Chris Wilson)
 * Add previously valid but closed context id to the test. (Chris Wilson)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 tests/Makefile.sources   |  1 -
 tests/gem_ctx_bad_exec.c | 60 ----------------------------------------
 tests/gem_ctx_exec.c     | 38 +++++++++++++++++++++++++
 tests/meson.build        |  1 -
 4 files changed, 38 insertions(+), 62 deletions(-)
 delete mode 100644 tests/gem_ctx_bad_exec.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index c84933f1d971..269336ad3150 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -51,7 +51,6 @@ TESTS_progs = \
 	gem_cs_prefetch \
 	gem_cs_tlb \
 	gem_ctx_bad_destroy \
-	gem_ctx_bad_exec \
 	gem_ctx_create \
 	gem_ctx_exec \
 	gem_ctx_isolation \
diff --git a/tests/gem_ctx_bad_exec.c b/tests/gem_ctx_bad_exec.c
deleted file mode 100644
index e3ccc5be46a0..000000000000
--- a/tests/gem_ctx_bad_exec.c
+++ /dev/null
@@ -1,60 +0,0 @@
-/*
- * Copyright © 2012 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"
-
-IGT_TEST_DESCRIPTION("Test that context cannot be submitted to any ring");
-
-static int exec(int fd, unsigned ring)
-{
-	struct drm_i915_gem_execbuffer2 execbuf;
-	struct drm_i915_gem_exec_object2 obj;
-
-	memset(&obj, 0, sizeof(obj));
-	memset(&execbuf, 0, sizeof(execbuf));
-
-	execbuf.buffers_ptr = to_user_pointer(&obj);
-	execbuf.buffer_count = 1;
-	i915_execbuffer2_set_context_id(execbuf, 1);
-
-	return __gem_execbuf(fd, &execbuf);
-}
-
-igt_main
-{
-	const struct intel_execution_engine *e;
-	int fd = -1;
-
-	igt_skip_on_simulation();
-
-	igt_fixture
-		fd = drm_open_driver_render(DRIVER_INTEL);
-
-	for (e = intel_execution_engines; e->name; e++) {
-		igt_subtest_f("%s", e->name) {
-			gem_require_ring(fd, e->exec_id | e->flags);
-			igt_assert_eq(exec(fd, e->exec_id | e->flags), -ENOENT);
-		}
-	}
-}
diff --git a/tests/gem_ctx_exec.c b/tests/gem_ctx_exec.c
index 1f8ed64d4bd3..1c6c18eddc20 100644
--- a/tests/gem_ctx_exec.c
+++ b/tests/gem_ctx_exec.c
@@ -30,6 +30,7 @@
  */
 
 #include "igt.h"
+#include <limits.h>
 #include <unistd.h>
 #include <stdlib.h>
 #include <stdint.h>
@@ -142,6 +143,34 @@ static void big_exec(int fd, uint32_t handle, int ring)
 	gem_sync(fd, handle);
 }
 
+static void invalid_context(int fd, unsigned ring, uint32_t handle)
+{
+	struct drm_i915_gem_exec_object2 obj = {
+		.handle = handle,
+	};
+	struct drm_i915_gem_execbuffer2 execbuf = {
+		.buffers_ptr = to_user_pointer(&obj),
+		.buffer_count = 1,
+		.flags = ring,
+	};
+	uint32_t invalid[] = { 0, 1, INT_MAX, UINT_MAX };
+	int i;
+
+	invalid[0] = gem_context_create(fd);
+
+	/* Verify everything works. */
+	i915_execbuffer2_set_context_id(execbuf, invalid[0]);
+	gem_execbuf(fd, &execbuf);
+
+	gem_context_destroy(fd, invalid[0]);
+
+	/* Go through the non-existent context id's. */
+	for (i = 0; i < ARRAY_SIZE(invalid); i++) {
+		i915_execbuffer2_set_context_id(execbuf, invalid[i]);
+		igt_assert_eq(__gem_execbuf(fd, &execbuf), -ENOENT);
+	}
+}
+
 uint32_t handle;
 uint32_t batch[2] = {0, MI_BATCH_BUFFER_END};
 uint32_t ctx_id, ctx_id2;
@@ -149,6 +178,8 @@ int fd;
 
 igt_main
 {
+	const struct intel_execution_engine *e;
+
 	igt_fixture {
 		fd = drm_open_driver_render(DRIVER_INTEL);
 		igt_require_gem(fd);
@@ -174,6 +205,13 @@ igt_main
 		gem_sync(fd, handle);
 	}
 
+	for (e = intel_execution_engines; e->name; e++) {
+		igt_subtest_f("basic-invalid-context-%s", e->name) {
+			gem_require_ring(fd, e->exec_id | e->flags);
+			invalid_context(fd, e->exec_id | e->flags, handle);
+		}
+	}
+
 	igt_subtest("eviction")
 		big_exec(fd, handle, I915_EXEC_RENDER);
 
diff --git a/tests/meson.build b/tests/meson.build
index 17deb945ec95..d22d59e0837d 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -28,7 +28,6 @@ test_progs = [
 	'gem_cs_prefetch',
 	'gem_cs_tlb',
 	'gem_ctx_bad_destroy',
-	'gem_ctx_bad_exec',
 	'gem_ctx_create',
 	'gem_ctx_exec',
 	'gem_ctx_isolation',
-- 
2.17.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [PATCH i-g-t v3 1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec
  2018-09-18 10:59                   ` [igt-dev] " Tvrtko Ursulin
@ 2018-09-18 11:06                     ` Chris Wilson
  -1 siblings, 0 replies; 35+ messages in thread
From: Chris Wilson @ 2018-09-18 11:06 UTC (permalink / raw)
  To: Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx

Quoting Tvrtko Ursulin (2018-09-18 11:59:25)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Move a really small test that invalid context is rejected under the
> gem_ctx_exec umbrella.
> 
> v2:
>  * And actually fix the test so it does what it claims. And add more
>    variety in the invalid context id's it tests with. (Chris Wilson)
> 
> v3:
>  * Rename the test as basic.
>  * Limit to 32-bit. (Chris Wilson)
>  * Add previously valid but closed context id to the test. (Chris Wilson)
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> diff --git a/tests/gem_ctx_exec.c b/tests/gem_ctx_exec.c
> index 1f8ed64d4bd3..1c6c18eddc20 100644
> --- a/tests/gem_ctx_exec.c
> +++ b/tests/gem_ctx_exec.c
> @@ -30,6 +30,7 @@
>   */
>  
>  #include "igt.h"
> +#include <limits.h>
>  #include <unistd.h>
>  #include <stdlib.h>
>  #include <stdint.h>
> @@ -142,6 +143,34 @@ static void big_exec(int fd, uint32_t handle, int ring)
>         gem_sync(fd, handle);
>  }
>  
> +static void invalid_context(int fd, unsigned ring, uint32_t handle)
> +{
> +       struct drm_i915_gem_exec_object2 obj = {
> +               .handle = handle,
> +       };
> +       struct drm_i915_gem_execbuffer2 execbuf = {
> +               .buffers_ptr = to_user_pointer(&obj),
> +               .buffer_count = 1,
> +               .flags = ring,
> +       };
> +       uint32_t invalid[] = { 0, 1, INT_MAX, UINT_MAX };

I think I would also do
i915_execbuffer2_set_context_id(execbuf, 0);
igt_assert_eq(__gem_execbuf(fd, &execbuf), 0);
for (int bit = 0; bit < 31; bit++) {
	i915_execbuffer2_set_context_id(execbuf, 1ul << bit);
	igt_assert_eq(__gem_execbuf(fd, &execbuf), -ENOENT);
}
as that would catch any aliasing <32bits with pot mask. (I leave
detection of N % M to fuzzers!)

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t v3 1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec
@ 2018-09-18 11:06                     ` Chris Wilson
  0 siblings, 0 replies; 35+ messages in thread
From: Chris Wilson @ 2018-09-18 11:06 UTC (permalink / raw)
  To: Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx

Quoting Tvrtko Ursulin (2018-09-18 11:59:25)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Move a really small test that invalid context is rejected under the
> gem_ctx_exec umbrella.
> 
> v2:
>  * And actually fix the test so it does what it claims. And add more
>    variety in the invalid context id's it tests with. (Chris Wilson)
> 
> v3:
>  * Rename the test as basic.
>  * Limit to 32-bit. (Chris Wilson)
>  * Add previously valid but closed context id to the test. (Chris Wilson)
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> diff --git a/tests/gem_ctx_exec.c b/tests/gem_ctx_exec.c
> index 1f8ed64d4bd3..1c6c18eddc20 100644
> --- a/tests/gem_ctx_exec.c
> +++ b/tests/gem_ctx_exec.c
> @@ -30,6 +30,7 @@
>   */
>  
>  #include "igt.h"
> +#include <limits.h>
>  #include <unistd.h>
>  #include <stdlib.h>
>  #include <stdint.h>
> @@ -142,6 +143,34 @@ static void big_exec(int fd, uint32_t handle, int ring)
>         gem_sync(fd, handle);
>  }
>  
> +static void invalid_context(int fd, unsigned ring, uint32_t handle)
> +{
> +       struct drm_i915_gem_exec_object2 obj = {
> +               .handle = handle,
> +       };
> +       struct drm_i915_gem_execbuffer2 execbuf = {
> +               .buffers_ptr = to_user_pointer(&obj),
> +               .buffer_count = 1,
> +               .flags = ring,
> +       };
> +       uint32_t invalid[] = { 0, 1, INT_MAX, UINT_MAX };

I think I would also do
i915_execbuffer2_set_context_id(execbuf, 0);
igt_assert_eq(__gem_execbuf(fd, &execbuf), 0);
for (int bit = 0; bit < 31; bit++) {
	i915_execbuffer2_set_context_id(execbuf, 1ul << bit);
	igt_assert_eq(__gem_execbuf(fd, &execbuf), -ENOENT);
}
as that would catch any aliasing <32bits with pot mask. (I leave
detection of N % M to fuzzers!)

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* [PATCH i-g-t v4 1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec
  2018-09-18 11:06                     ` [igt-dev] [Intel-gfx] " Chris Wilson
@ 2018-09-18 11:21                       ` Tvrtko Ursulin
  -1 siblings, 0 replies; 35+ messages in thread
From: Tvrtko Ursulin @ 2018-09-18 11:21 UTC (permalink / raw)
  To: igt-dev; +Cc: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Move a really small test that invalid context is rejected under the
gem_ctx_exec umbrella.

v2:
 * And actually fix the test so it does what it claims. And add more
   variety in the invalid context id's it tests with. (Chris Wilson)

v3:
 * Rename the test as basic.
 * Limit to 32-bit. (Chris Wilson)
 * Add previously valid but closed context id to the test. (Chris Wilson)

v4:
 * Add more invalid values. (Chris Wilson)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/Makefile.sources   |  1 -
 tests/gem_ctx_bad_exec.c | 60 ----------------------------------------
 tests/gem_ctx_exec.c     | 46 ++++++++++++++++++++++++++++++
 tests/meson.build        |  1 -
 4 files changed, 46 insertions(+), 62 deletions(-)
 delete mode 100644 tests/gem_ctx_bad_exec.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index c84933f1d971..269336ad3150 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -51,7 +51,6 @@ TESTS_progs = \
 	gem_cs_prefetch \
 	gem_cs_tlb \
 	gem_ctx_bad_destroy \
-	gem_ctx_bad_exec \
 	gem_ctx_create \
 	gem_ctx_exec \
 	gem_ctx_isolation \
diff --git a/tests/gem_ctx_bad_exec.c b/tests/gem_ctx_bad_exec.c
deleted file mode 100644
index e3ccc5be46a0..000000000000
--- a/tests/gem_ctx_bad_exec.c
+++ /dev/null
@@ -1,60 +0,0 @@
-/*
- * Copyright © 2012 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"
-
-IGT_TEST_DESCRIPTION("Test that context cannot be submitted to any ring");
-
-static int exec(int fd, unsigned ring)
-{
-	struct drm_i915_gem_execbuffer2 execbuf;
-	struct drm_i915_gem_exec_object2 obj;
-
-	memset(&obj, 0, sizeof(obj));
-	memset(&execbuf, 0, sizeof(execbuf));
-
-	execbuf.buffers_ptr = to_user_pointer(&obj);
-	execbuf.buffer_count = 1;
-	i915_execbuffer2_set_context_id(execbuf, 1);
-
-	return __gem_execbuf(fd, &execbuf);
-}
-
-igt_main
-{
-	const struct intel_execution_engine *e;
-	int fd = -1;
-
-	igt_skip_on_simulation();
-
-	igt_fixture
-		fd = drm_open_driver_render(DRIVER_INTEL);
-
-	for (e = intel_execution_engines; e->name; e++) {
-		igt_subtest_f("%s", e->name) {
-			gem_require_ring(fd, e->exec_id | e->flags);
-			igt_assert_eq(exec(fd, e->exec_id | e->flags), -ENOENT);
-		}
-	}
-}
diff --git a/tests/gem_ctx_exec.c b/tests/gem_ctx_exec.c
index 1f8ed64d4bd3..f02a88b1acfc 100644
--- a/tests/gem_ctx_exec.c
+++ b/tests/gem_ctx_exec.c
@@ -30,6 +30,7 @@
  */
 
 #include "igt.h"
+#include <limits.h>
 #include <unistd.h>
 #include <stdlib.h>
 #include <stdint.h>
@@ -142,6 +143,42 @@ static void big_exec(int fd, uint32_t handle, int ring)
 	gem_sync(fd, handle);
 }
 
+static void invalid_context(int fd, unsigned ring, uint32_t handle)
+{
+	struct drm_i915_gem_exec_object2 obj = {
+		.handle = handle,
+	};
+	struct drm_i915_gem_execbuffer2 execbuf = {
+		.buffers_ptr = to_user_pointer(&obj),
+		.buffer_count = 1,
+		.flags = ring,
+	};
+	unsigned int i;
+	uint32_t ctx;
+
+	/* Verify everything works. */
+	i915_execbuffer2_set_context_id(execbuf, 0);
+	gem_execbuf(fd, &execbuf);
+
+	ctx = gem_context_create(fd);
+	i915_execbuffer2_set_context_id(execbuf, ctx);
+	gem_execbuf(fd, &execbuf);
+
+	gem_context_destroy(fd, ctx);
+
+	/* Go through the non-existent context id's. */
+	for (i = 0; i < 32; i++) {
+		i915_execbuffer2_set_context_id(execbuf, 1UL << i);
+		igt_assert_eq(__gem_execbuf(fd, &execbuf), -ENOENT);
+	}
+
+	i915_execbuffer2_set_context_id(execbuf, INT_MAX);
+	igt_assert_eq(__gem_execbuf(fd, &execbuf), -ENOENT);
+
+	i915_execbuffer2_set_context_id(execbuf, UINT_MAX);
+	igt_assert_eq(__gem_execbuf(fd, &execbuf), -ENOENT);
+}
+
 uint32_t handle;
 uint32_t batch[2] = {0, MI_BATCH_BUFFER_END};
 uint32_t ctx_id, ctx_id2;
@@ -149,6 +186,8 @@ int fd;
 
 igt_main
 {
+	const struct intel_execution_engine *e;
+
 	igt_fixture {
 		fd = drm_open_driver_render(DRIVER_INTEL);
 		igt_require_gem(fd);
@@ -174,6 +213,13 @@ igt_main
 		gem_sync(fd, handle);
 	}
 
+	for (e = intel_execution_engines; e->name; e++) {
+		igt_subtest_f("basic-invalid-context-%s", e->name) {
+			gem_require_ring(fd, e->exec_id | e->flags);
+			invalid_context(fd, e->exec_id | e->flags, handle);
+		}
+	}
+
 	igt_subtest("eviction")
 		big_exec(fd, handle, I915_EXEC_RENDER);
 
diff --git a/tests/meson.build b/tests/meson.build
index 17deb945ec95..d22d59e0837d 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -28,7 +28,6 @@ test_progs = [
 	'gem_cs_prefetch',
 	'gem_cs_tlb',
 	'gem_ctx_bad_destroy',
-	'gem_ctx_bad_exec',
 	'gem_ctx_create',
 	'gem_ctx_exec',
 	'gem_ctx_isolation',
-- 
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [igt-dev] [PATCH i-g-t v4 1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec
@ 2018-09-18 11:21                       ` Tvrtko Ursulin
  0 siblings, 0 replies; 35+ messages in thread
From: Tvrtko Ursulin @ 2018-09-18 11:21 UTC (permalink / raw)
  To: igt-dev; +Cc: Intel-gfx, Tvrtko Ursulin

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Move a really small test that invalid context is rejected under the
gem_ctx_exec umbrella.

v2:
 * And actually fix the test so it does what it claims. And add more
   variety in the invalid context id's it tests with. (Chris Wilson)

v3:
 * Rename the test as basic.
 * Limit to 32-bit. (Chris Wilson)
 * Add previously valid but closed context id to the test. (Chris Wilson)

v4:
 * Add more invalid values. (Chris Wilson)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/Makefile.sources   |  1 -
 tests/gem_ctx_bad_exec.c | 60 ----------------------------------------
 tests/gem_ctx_exec.c     | 46 ++++++++++++++++++++++++++++++
 tests/meson.build        |  1 -
 4 files changed, 46 insertions(+), 62 deletions(-)
 delete mode 100644 tests/gem_ctx_bad_exec.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index c84933f1d971..269336ad3150 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -51,7 +51,6 @@ TESTS_progs = \
 	gem_cs_prefetch \
 	gem_cs_tlb \
 	gem_ctx_bad_destroy \
-	gem_ctx_bad_exec \
 	gem_ctx_create \
 	gem_ctx_exec \
 	gem_ctx_isolation \
diff --git a/tests/gem_ctx_bad_exec.c b/tests/gem_ctx_bad_exec.c
deleted file mode 100644
index e3ccc5be46a0..000000000000
--- a/tests/gem_ctx_bad_exec.c
+++ /dev/null
@@ -1,60 +0,0 @@
-/*
- * Copyright © 2012 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"
-
-IGT_TEST_DESCRIPTION("Test that context cannot be submitted to any ring");
-
-static int exec(int fd, unsigned ring)
-{
-	struct drm_i915_gem_execbuffer2 execbuf;
-	struct drm_i915_gem_exec_object2 obj;
-
-	memset(&obj, 0, sizeof(obj));
-	memset(&execbuf, 0, sizeof(execbuf));
-
-	execbuf.buffers_ptr = to_user_pointer(&obj);
-	execbuf.buffer_count = 1;
-	i915_execbuffer2_set_context_id(execbuf, 1);
-
-	return __gem_execbuf(fd, &execbuf);
-}
-
-igt_main
-{
-	const struct intel_execution_engine *e;
-	int fd = -1;
-
-	igt_skip_on_simulation();
-
-	igt_fixture
-		fd = drm_open_driver_render(DRIVER_INTEL);
-
-	for (e = intel_execution_engines; e->name; e++) {
-		igt_subtest_f("%s", e->name) {
-			gem_require_ring(fd, e->exec_id | e->flags);
-			igt_assert_eq(exec(fd, e->exec_id | e->flags), -ENOENT);
-		}
-	}
-}
diff --git a/tests/gem_ctx_exec.c b/tests/gem_ctx_exec.c
index 1f8ed64d4bd3..f02a88b1acfc 100644
--- a/tests/gem_ctx_exec.c
+++ b/tests/gem_ctx_exec.c
@@ -30,6 +30,7 @@
  */
 
 #include "igt.h"
+#include <limits.h>
 #include <unistd.h>
 #include <stdlib.h>
 #include <stdint.h>
@@ -142,6 +143,42 @@ static void big_exec(int fd, uint32_t handle, int ring)
 	gem_sync(fd, handle);
 }
 
+static void invalid_context(int fd, unsigned ring, uint32_t handle)
+{
+	struct drm_i915_gem_exec_object2 obj = {
+		.handle = handle,
+	};
+	struct drm_i915_gem_execbuffer2 execbuf = {
+		.buffers_ptr = to_user_pointer(&obj),
+		.buffer_count = 1,
+		.flags = ring,
+	};
+	unsigned int i;
+	uint32_t ctx;
+
+	/* Verify everything works. */
+	i915_execbuffer2_set_context_id(execbuf, 0);
+	gem_execbuf(fd, &execbuf);
+
+	ctx = gem_context_create(fd);
+	i915_execbuffer2_set_context_id(execbuf, ctx);
+	gem_execbuf(fd, &execbuf);
+
+	gem_context_destroy(fd, ctx);
+
+	/* Go through the non-existent context id's. */
+	for (i = 0; i < 32; i++) {
+		i915_execbuffer2_set_context_id(execbuf, 1UL << i);
+		igt_assert_eq(__gem_execbuf(fd, &execbuf), -ENOENT);
+	}
+
+	i915_execbuffer2_set_context_id(execbuf, INT_MAX);
+	igt_assert_eq(__gem_execbuf(fd, &execbuf), -ENOENT);
+
+	i915_execbuffer2_set_context_id(execbuf, UINT_MAX);
+	igt_assert_eq(__gem_execbuf(fd, &execbuf), -ENOENT);
+}
+
 uint32_t handle;
 uint32_t batch[2] = {0, MI_BATCH_BUFFER_END};
 uint32_t ctx_id, ctx_id2;
@@ -149,6 +186,8 @@ int fd;
 
 igt_main
 {
+	const struct intel_execution_engine *e;
+
 	igt_fixture {
 		fd = drm_open_driver_render(DRIVER_INTEL);
 		igt_require_gem(fd);
@@ -174,6 +213,13 @@ igt_main
 		gem_sync(fd, handle);
 	}
 
+	for (e = intel_execution_engines; e->name; e++) {
+		igt_subtest_f("basic-invalid-context-%s", e->name) {
+			gem_require_ring(fd, e->exec_id | e->flags);
+			invalid_context(fd, e->exec_id | e->flags, handle);
+		}
+	}
+
 	igt_subtest("eviction")
 		big_exec(fd, handle, I915_EXEC_RENDER);
 
diff --git a/tests/meson.build b/tests/meson.build
index 17deb945ec95..d22d59e0837d 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -28,7 +28,6 @@ test_progs = [
 	'gem_cs_prefetch',
 	'gem_cs_tlb',
 	'gem_ctx_bad_destroy',
-	'gem_ctx_bad_exec',
 	'gem_ctx_create',
 	'gem_ctx_exec',
 	'gem_ctx_isolation',
-- 
2.17.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* [igt-dev] ✗ Fi.CI.BAT: failure for series starting with [i-g-t,v3,1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec (rev2)
  2018-09-17 15:46 ` [igt-dev] " Tvrtko Ursulin
                   ` (5 preceding siblings ...)
  (?)
@ 2018-09-18 11:28 ` Patchwork
  -1 siblings, 0 replies; 35+ messages in thread
From: Patchwork @ 2018-09-18 11:28 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: igt-dev

== Series Details ==

Series: series starting with [i-g-t,v3,1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec (rev2)
URL   : https://patchwork.freedesktop.org/series/49797/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4833 -> IGTPW_1852 =

== Summary - FAILURE ==

  Serious unknown changes coming with IGTPW_1852 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_1852, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/49797/revisions/2/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in IGTPW_1852:

  === IGT changes ===

    ==== Possible regressions ====

    igt@drv_selftest@live_contexts:
      fi-bsw-n3050:       PASS -> DMESG-WARN

    
== Known issues ==

  Here are the changes found in IGTPW_1852 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@mock_hugepages:
      fi-bwr-2160:        PASS -> DMESG-FAIL (fdo#107930)

    igt@gem_exec_suspend@basic-s3:
      fi-skl-caroline:    NOTRUN -> INCOMPLETE (fdo#107556, fdo#104108)
      fi-blb-e6850:       PASS -> INCOMPLETE (fdo#107718)

    igt@gem_exec_suspend@basic-s4-devices:
      fi-skl-6600u:       PASS -> INCOMPLETE (fdo#104108)

    igt@kms_frontbuffer_tracking@basic:
      fi-byt-clapper:     PASS -> FAIL (fdo#103167)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      fi-bdw-samus:       NOTRUN -> INCOMPLETE (fdo#107773)

    igt@kms_psr@primary_mmap_gtt:
      fi-cnl-u:           NOTRUN -> FAIL (fdo#107383) +3

    igt@kms_psr@primary_page_flip:
      fi-cfl-s3:          PASS -> FAIL (fdo#107336)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_hangcheck:
      fi-glk-j4005:       INCOMPLETE (fdo#103359, k.org#198133) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      fi-icl-u:           INCOMPLETE (fdo#107713) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-snb-2520m:       DMESG-FAIL (fdo#103713) -> PASS

    igt@kms_setmode@basic-clone-single-crtc:
      fi-snb-2520m:       DMESG-WARN (fdo#103713) -> PASS

    
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
  fdo#107336 https://bugs.freedesktop.org/show_bug.cgi?id=107336
  fdo#107383 https://bugs.freedesktop.org/show_bug.cgi?id=107383
  fdo#107556 https://bugs.freedesktop.org/show_bug.cgi?id=107556
  fdo#107713 https://bugs.freedesktop.org/show_bug.cgi?id=107713
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
  fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773
  fdo#107930 https://bugs.freedesktop.org/show_bug.cgi?id=107930
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (46 -> 45) ==

  Additional (3): fi-cnl-u fi-skl-caroline fi-bdw-samus 
  Missing    (4): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * IGT: IGT_4644 -> IGTPW_1852

  CI_DRM_4833: 75bb460b367a614d10b0fba220143bee42657d7e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1852: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1852/
  IGT_4644: 0b59bb3231ab481959528c5c7b3a98762772e1b0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+igt@gem_ctx_exec@basic-invalid-context-blt
+igt@gem_ctx_exec@basic-invalid-context-bsd
+igt@gem_ctx_exec@basic-invalid-context-bsd1
+igt@gem_ctx_exec@basic-invalid-context-bsd2
+igt@gem_ctx_exec@basic-invalid-context-default
+igt@gem_ctx_exec@basic-invalid-context-render
+igt@gem_ctx_exec@basic-invalid-context-vebox
-igt@gem_ctx_bad_exec@blt
-igt@gem_ctx_bad_exec@bsd
-igt@gem_ctx_bad_exec@bsd1
-igt@gem_ctx_bad_exec@bsd2
-igt@gem_ctx_bad_exec@default
-igt@gem_ctx_bad_exec@render
-igt@gem_ctx_bad_exec@vebox
-igt@gem_ctx_exec@lrc-lite-restore

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1852/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,v4,1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec (rev3)
  2018-09-17 15:46 ` [igt-dev] " Tvrtko Ursulin
                   ` (6 preceding siblings ...)
  (?)
@ 2018-09-18 11:52 ` Patchwork
  -1 siblings, 0 replies; 35+ messages in thread
From: Patchwork @ 2018-09-18 11:52 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: igt-dev

== Series Details ==

Series: series starting with [i-g-t,v4,1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec (rev3)
URL   : https://patchwork.freedesktop.org/series/49797/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4833 -> IGTPW_1853 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/49797/revisions/3/mbox/

== Known issues ==

  Here are the changes found in IGTPW_1853 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@mock_hugepages:
      fi-bwr-2160:        PASS -> DMESG-FAIL (fdo#107930)

    igt@gem_exec_suspend@basic-s3:
      fi-bdw-samus:       NOTRUN -> INCOMPLETE (fdo#107773)
      fi-skl-caroline:    NOTRUN -> INCOMPLETE (fdo#107556, fdo#104108)

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-peppy:       PASS -> DMESG-WARN (fdo#102614)
      fi-byt-clapper:     PASS -> FAIL (fdo#103167)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      fi-byt-clapper:     PASS -> FAIL (fdo#107362, fdo#103191) +1

    igt@kms_psr@primary_mmap_gtt:
      fi-cnl-u:           NOTRUN -> FAIL (fdo#107383) +3

    igt@pm_backlight@basic-brightness:
      fi-snb-2520m:       NOTRUN -> INCOMPLETE (fdo#103713)

    
    ==== Possible fixes ====

    igt@drv_module_reload@basic-reload:
      fi-blb-e6850:       INCOMPLETE (fdo#107718) -> PASS

    igt@drv_selftest@live_hangcheck:
      fi-glk-j4005:       INCOMPLETE (k.org#198133, fdo#103359) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      fi-icl-u:           INCOMPLETE (fdo#107713) -> PASS

    igt@kms_psr@primary_page_flip:
      fi-kbl-r:           FAIL (fdo#107336) -> PASS

    
  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
  fdo#107336 https://bugs.freedesktop.org/show_bug.cgi?id=107336
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107383 https://bugs.freedesktop.org/show_bug.cgi?id=107383
  fdo#107556 https://bugs.freedesktop.org/show_bug.cgi?id=107556
  fdo#107713 https://bugs.freedesktop.org/show_bug.cgi?id=107713
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
  fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773
  fdo#107930 https://bugs.freedesktop.org/show_bug.cgi?id=107930
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (46 -> 45) ==

  Additional (3): fi-cnl-u fi-skl-caroline fi-bdw-samus 
  Missing    (4): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * IGT: IGT_4644 -> IGTPW_1853

  CI_DRM_4833: 75bb460b367a614d10b0fba220143bee42657d7e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1853: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1853/
  IGT_4644: 0b59bb3231ab481959528c5c7b3a98762772e1b0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+igt@gem_ctx_exec@basic-invalid-context-blt
+igt@gem_ctx_exec@basic-invalid-context-bsd
+igt@gem_ctx_exec@basic-invalid-context-bsd1
+igt@gem_ctx_exec@basic-invalid-context-bsd2
+igt@gem_ctx_exec@basic-invalid-context-default
+igt@gem_ctx_exec@basic-invalid-context-render
+igt@gem_ctx_exec@basic-invalid-context-vebox
-igt@gem_ctx_bad_exec@blt
-igt@gem_ctx_bad_exec@bsd
-igt@gem_ctx_bad_exec@bsd1
-igt@gem_ctx_bad_exec@bsd2
-igt@gem_ctx_bad_exec@default
-igt@gem_ctx_bad_exec@render
-igt@gem_ctx_bad_exec@vebox
-igt@gem_ctx_exec@lrc-lite-restore

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1853/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* [igt-dev] ✓ Fi.CI.IGT: success for series starting with [i-g-t,v3,1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec (rev2)
  2018-09-17 15:46 ` [igt-dev] " Tvrtko Ursulin
                   ` (7 preceding siblings ...)
  (?)
@ 2018-09-18 12:21 ` Patchwork
  -1 siblings, 0 replies; 35+ messages in thread
From: Patchwork @ 2018-09-18 12:21 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: igt-dev

== Series Details ==

Series: series starting with [i-g-t,v3,1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec (rev2)
URL   : https://patchwork.freedesktop.org/series/49797/
State : success

== Summary ==

= CI Bug Log - changes from IGT_4644_full -> IGTPW_1852_full =

== Summary - WARNING ==

  Minor unknown changes coming with IGTPW_1852_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_1852_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/49797/revisions/2/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in IGTPW_1852_full:

  === IGT changes ===

    ==== Warnings ====

    igt@perf_pmu@rc6:
      shard-kbl:          SKIP -> PASS

    igt@pm_rc6_residency@rc6-accuracy:
      shard-snb:          SKIP -> PASS

    
== Known issues ==

  Here are the changes found in IGTPW_1852_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_await@wide-contexts:
      shard-kbl:          PASS -> FAIL (fdo#106680)

    igt@gem_exec_suspend@basic-s3:
      shard-kbl:          PASS -> DMESG-WARN (fdo#103313)

    igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
      shard-glk:          PASS -> FAIL (fdo#105363)

    igt@kms_flip@flip-vs-expired-vblank:
      shard-glk:          PASS -> FAIL (fdo#105363, fdo#102887)

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-pri-shrfb-draw-mmap-cpu:
      shard-glk:          PASS -> FAIL (fdo#103167) +1

    igt@kms_plane@pixel-format-pipe-a-planes:
      shard-snb:          PASS -> FAIL (fdo#103166, fdo#107749)

    igt@kms_setmode@basic:
      shard-kbl:          PASS -> FAIL (fdo#99912)

    igt@perf@blocking:
      shard-hsw:          PASS -> FAIL (fdo#102252)

    
    ==== Possible fixes ====

    igt@gem_exec_big:
      shard-hsw:          TIMEOUT (fdo#107937) -> PASS

    igt@gem_ppgtt@blt-vs-render-ctxn:
      shard-kbl:          INCOMPLETE (fdo#106023, fdo#103665) -> PASS

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-glk:          FAIL (fdo#105363) -> PASS

    igt@kms_frontbuffer_tracking@fbc-stridechange:
      shard-glk:          FAIL (fdo#103167) -> PASS

    igt@kms_rotation_crc@sprite-rotation-180:
      shard-snb:          FAIL (fdo#103925) -> PASS

    igt@kms_setmode@basic:
      shard-apl:          FAIL (fdo#99912) -> PASS

    
  fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
  fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103313 https://bugs.freedesktop.org/show_bug.cgi?id=103313
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
  fdo#106680 https://bugs.freedesktop.org/show_bug.cgi?id=106680
  fdo#107749 https://bugs.freedesktop.org/show_bug.cgi?id=107749
  fdo#107937 https://bugs.freedesktop.org/show_bug.cgi?id=107937
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * IGT: IGT_4644 -> IGTPW_1852
    * Linux: CI_DRM_4827 -> CI_DRM_4833

  CI_DRM_4827: 8b1968f143e8bf65acf0ea6f7ce9120259043521 @ git://anongit.freedesktop.org/gfx-ci/linux
  CI_DRM_4833: 75bb460b367a614d10b0fba220143bee42657d7e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1852: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1852/
  IGT_4644: 0b59bb3231ab481959528c5c7b3a98762772e1b0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

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

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

* [igt-dev] ✓ Fi.CI.IGT: success for series starting with [i-g-t,v4,1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec (rev3)
  2018-09-17 15:46 ` [igt-dev] " Tvrtko Ursulin
                   ` (8 preceding siblings ...)
  (?)
@ 2018-09-18 13:04 ` Patchwork
  -1 siblings, 0 replies; 35+ messages in thread
From: Patchwork @ 2018-09-18 13:04 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: igt-dev

== Series Details ==

Series: series starting with [i-g-t,v4,1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec (rev3)
URL   : https://patchwork.freedesktop.org/series/49797/
State : success

== Summary ==

= CI Bug Log - changes from IGT_4644_full -> IGTPW_1853_full =

== Summary - WARNING ==

  Minor unknown changes coming with IGTPW_1853_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_1853_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/49797/revisions/3/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in IGTPW_1853_full:

  === IGT changes ===

    ==== Warnings ====

    igt@kms_draw_crc@draw-method-xrgb8888-pwrite-untiled:
      shard-snb:          PASS -> SKIP +2

    
== Known issues ==

  Here are the changes found in IGTPW_1853_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_atomic_transition@plane-primary-toggle-with-vblank-wait:
      shard-apl:          PASS -> DMESG-WARN (fdo#105602, fdo#103558) +14

    igt@kms_available_modes_crc@available_mode_test_crc:
      shard-snb:          PASS -> FAIL (fdo#106641)

    igt@kms_cursor_legacy@cursora-vs-flipa-toggle:
      shard-glk:          PASS -> DMESG-WARN (fdo#105763, fdo#106538) +1

    igt@kms_draw_crc@fill-fb:
      shard-glk:          PASS -> FAIL (fdo#103184)

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-shrfb-pgflip-blt:
      shard-glk:          PASS -> FAIL (fdo#103167)

    igt@kms_setmode@basic:
      shard-kbl:          PASS -> FAIL (fdo#99912)

    
    ==== Possible fixes ====

    igt@gem_exec_big:
      shard-hsw:          TIMEOUT (fdo#107937) -> PASS

    igt@gem_ppgtt@blt-vs-render-ctxn:
      shard-kbl:          INCOMPLETE (fdo#103665, fdo#106023) -> PASS

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-glk:          FAIL (fdo#105363) -> PASS

    igt@kms_frontbuffer_tracking@fbc-stridechange:
      shard-glk:          FAIL (fdo#103167) -> PASS

    igt@kms_rotation_crc@primary-rotation-180:
      shard-snb:          FAIL (fdo#103925) -> PASS

    
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103184 https://bugs.freedesktop.org/show_bug.cgi?id=103184
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
  fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
  fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
  fdo#106641 https://bugs.freedesktop.org/show_bug.cgi?id=106641
  fdo#107937 https://bugs.freedesktop.org/show_bug.cgi?id=107937
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * IGT: IGT_4644 -> IGTPW_1853
    * Linux: CI_DRM_4827 -> CI_DRM_4833

  CI_DRM_4827: 8b1968f143e8bf65acf0ea6f7ce9120259043521 @ git://anongit.freedesktop.org/gfx-ci/linux
  CI_DRM_4833: 75bb460b367a614d10b0fba220143bee42657d7e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1853: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1853/
  IGT_4644: 0b59bb3231ab481959528c5c7b3a98762772e1b0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

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

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

end of thread, other threads:[~2018-09-18 13:04 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-17 15:46 [PATCH i-g-t 1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec Tvrtko Ursulin
2018-09-17 15:46 ` [igt-dev] " Tvrtko Ursulin
2018-09-17 15:46 ` [PATCH i-g-t 2/2] gem_ctx_exec: Remove lrc-lite-restore Tvrtko Ursulin
2018-09-17 15:46   ` [igt-dev] " Tvrtko Ursulin
2018-09-17 15:53   ` Chris Wilson
2018-09-17 15:53     ` [igt-dev] [Intel-gfx] " Chris Wilson
2018-09-17 15:52 ` [igt-dev] [PATCH i-g-t 1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec Chris Wilson
2018-09-17 15:52   ` Chris Wilson
2018-09-18  9:38   ` Tvrtko Ursulin
2018-09-18  9:38     ` [Intel-gfx] " Tvrtko Ursulin
2018-09-18  9:44     ` Chris Wilson
2018-09-18  9:44       ` [Intel-gfx] " Chris Wilson
2018-09-18  9:59       ` Tvrtko Ursulin
2018-09-18  9:59         ` [igt-dev] [Intel-gfx] " Tvrtko Ursulin
2018-09-18 10:02         ` [igt-dev] " Chris Wilson
2018-09-18 10:02           ` [igt-dev] [Intel-gfx] " Chris Wilson
2018-09-18 10:03           ` [igt-dev] " Chris Wilson
2018-09-18 10:03             ` [igt-dev] [Intel-gfx] " Chris Wilson
2018-09-18 10:33             ` [igt-dev] " Tvrtko Ursulin
2018-09-18 10:33               ` [Intel-gfx] " Tvrtko Ursulin
2018-09-18 10:45               ` Chris Wilson
2018-09-18 10:45                 ` [igt-dev] [Intel-gfx] " Chris Wilson
2018-09-18 10:59                 ` [PATCH i-g-t v3 " Tvrtko Ursulin
2018-09-18 10:59                   ` [igt-dev] " Tvrtko Ursulin
2018-09-18 11:06                   ` Chris Wilson
2018-09-18 11:06                     ` [igt-dev] [Intel-gfx] " Chris Wilson
2018-09-18 11:21                     ` [PATCH i-g-t v4 " Tvrtko Ursulin
2018-09-18 11:21                       ` [igt-dev] " Tvrtko Ursulin
2018-09-17 16:27 ` [igt-dev] ✗ Fi.CI.BAT: failure for series starting with [i-g-t,1/2] " Patchwork
2018-09-17 18:04 ` [igt-dev] ✓ Fi.CI.BAT: success " Patchwork
2018-09-17 20:02 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
2018-09-18 11:28 ` [igt-dev] ✗ Fi.CI.BAT: failure for series starting with [i-g-t,v3,1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec (rev2) Patchwork
2018-09-18 11:52 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,v4,1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec (rev3) Patchwork
2018-09-18 12:21 ` [igt-dev] ✓ Fi.CI.IGT: success for series starting with [i-g-t,v3,1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec (rev2) Patchwork
2018-09-18 13:04 ` [igt-dev] ✓ Fi.CI.IGT: success for series starting with [i-g-t,v4,1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec (rev3) Patchwork

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.