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@xxxxxxxxx>
> >>
> >> 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@xxxxxxxxx>
> >> ---
> >> 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@xxxxxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
