diff mbox series

[i-g-t,v3,1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec

Message ID 20180918105925.2997-1-tvrtko.ursulin@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [i-g-t,v3,1/2] tests/gem_ctx_bad_exec: Consolidate to gem_ctx_exec | expand

Commit Message

Tvrtko Ursulin Sept. 18, 2018, 10:59 a.m. UTC
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

Comments

Chris Wilson Sept. 18, 2018, 11:06 a.m. UTC | #1
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
diff mbox series

Patch

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',