[igt-dev] [PATCH i-g-t 07/16] i915: Add gem_ctx_clone
Chris Wilson
chris at chris-wilson.co.uk
Wed May 15 19:14:21 UTC 2019
Quoting Tvrtko Ursulin (2019-05-14 13:41:13)
>
> On 08/05/2019 11:09, Chris Wilson wrote:
> > Exercise cloning contexts, an extension of merely creating one.
> >
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> > tests/Makefile.sources | 1 +
> > tests/i915/gem_ctx_clone.c | 460 +++++++++++++++++++++++++++++++++++++
> > tests/meson.build | 1 +
> > 3 files changed, 462 insertions(+)
> > create mode 100644 tests/i915/gem_ctx_clone.c
> >
> > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > index 1a541d206..e1b7feeb2 100644
> > --- a/tests/Makefile.sources
> > +++ b/tests/Makefile.sources
> > @@ -21,6 +21,7 @@ TESTS_progs = \
> > drm_import_export \
> > drm_mm \
> > drm_read \
> > + i915/gem_ctx_clone \
> > i915/gem_vm_create \
> > kms_3d \
> > kms_addfb_basic \
> > diff --git a/tests/i915/gem_ctx_clone.c b/tests/i915/gem_ctx_clone.c
> > new file mode 100644
> > index 000000000..cdc5bf413
> > --- /dev/null
> > +++ b/tests/i915/gem_ctx_clone.c
> > @@ -0,0 +1,460 @@
> > +/*
> > + * Copyright © 2019 Intel Corporation
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a
> > + * copy of this software and associated documentation files (the "Software"),
> > + * to deal in the Software without restriction, including without limitation
> > + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> > + * and/or sell copies of the Software, and to permit persons to whom the
> > + * Software is furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including the next
> > + * paragraph) shall be included in all copies or substantial portions of the
> > + * Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> > + * IN THE SOFTWARE.
> > + */
> > +
> > +#include "igt.h"
> > +#include "igt_gt.h"
> > +#include "i915/gem_vm.h"
> > +#include "i915_drm.h"
> > +
> > +static int ctx_create_ioctl(int i915, struct drm_i915_gem_context_create_ext *arg)
> > +{
> > + int err;
> > +
> > + err = 0;
> > + if (igt_ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_CREATE_EXT, arg)) {
> > + err = -errno;
> > + igt_assume(err);
> > + }
> > +
> > + errno = 0;
> > + return err;
> > +}
> > +
> > +static bool has_ctx_clone(int i915)
> > +{
> > + struct drm_i915_gem_context_create_ext_clone ext = {
> > + { .name = I915_CONTEXT_CREATE_EXT_CLONE },
> > + .clone_id = -1,
> > + };
> > + struct drm_i915_gem_context_create_ext create = {
> > + .flags = I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS,
> > + .extensions = to_user_pointer(&ext),
> > + };
> > + return ctx_create_ioctl(i915, &create) == -ENOENT;
> > +}
> > +
> > +static void invalid_clone(int i915)
> > +{
> > + struct drm_i915_gem_context_create_ext_clone ext = {
> > + { .name = I915_CONTEXT_CREATE_EXT_CLONE },
> > + };
> > + struct drm_i915_gem_context_create_ext create = {
> > + .flags = I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS,
> > + .extensions = to_user_pointer(&ext),
> > + };
> > +
> > + igt_assert_eq(ctx_create_ioctl(i915, &create), 0);
> > + gem_context_destroy(i915, create.ctx_id);
> > +
> > + ext.flags = -1; /* Hopefully we won't run out of flags */
> > + igt_assert_eq(ctx_create_ioctl(i915, &create), -EINVAL);
> > + ext.flags = 0;
> > +
> > + ext.base.next_extension = -1;
> > + igt_assert_eq(ctx_create_ioctl(i915, &create), -EFAULT);
> > + ext.base.next_extension = to_user_pointer(&ext);
> > + igt_assert_eq(ctx_create_ioctl(i915, &create), -E2BIG);
> > + ext.base.next_extension = 0;
> > +
> > + ext.clone_id = -1;
> > + igt_assert_eq(ctx_create_ioctl(i915, &create), -ENOENT);
> > + ext.clone_id = 0;
> > +}
> > +
> > +static void clone_flags(int i915)
> > +{
> > + struct drm_i915_gem_context_create_ext_setparam set = {
> > + { .name = I915_CONTEXT_CREATE_EXT_SETPARAM },
> > + { .param = I915_CONTEXT_PARAM_RECOVERABLE },
> > + };
> > + struct drm_i915_gem_context_create_ext_clone ext = {
> > + { .name = I915_CONTEXT_CREATE_EXT_CLONE },
> > + .flags = I915_CONTEXT_CLONE_FLAGS,
> > + };
> > + struct drm_i915_gem_context_create_ext create = {
> > + .flags = I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS,
> > + .extensions = to_user_pointer(&ext),
> > + };
> > + int expected;
> > +
> > + set.param.value = 1; /* default is recoverable */
> > + igt_require(__gem_context_set_param(i915, &set.param) == 0);
> > +
> > + for (int pass = 0; pass < 2; pass++) { /* cloning default, then child */
> > + igt_debug("Cloning %d\n", ext.clone_id);
> > + igt_assert_eq(ctx_create_ioctl(i915, &create), 0);
> > +
> > + set.param.ctx_id = ext.clone_id;
> > + gem_context_get_param(i915, &set.param); > + expected = set.param.value;
> > +
> > + set.param.ctx_id = create.ctx_id;
> > + gem_context_get_param(i915, &set.param);
> > +
> > + igt_assert_eq_u64(set.param.param,
> > + I915_CONTEXT_PARAM_RECOVERABLE);
> > + igt_assert_eq((int)set.param.value, expected);
> > +
> > + gem_context_destroy(i915, create.ctx_id);
> > +
> > + expected = set.param.value = 0;
> > + set.param.ctx_id = ext.clone_id;
> > + gem_context_set_param(i915, &set.param);
> > +
> > + igt_assert_eq(ctx_create_ioctl(i915, &create), 0);
> > +
> > + set.param.ctx_id = create.ctx_id;
> > + gem_context_get_param(i915, &set.param);
> > +
> > + igt_assert_eq_u64(set.param.param,
> > + I915_CONTEXT_PARAM_RECOVERABLE);
> > + igt_assert_eq((int)set.param.value, expected);
> > +
> > + gem_context_destroy(i915, create.ctx_id);
> > +
> > + /* clone but then reset priority to default... */
>
> Just correct priority/prio here and below.
>
> > + set.param.ctx_id = 0;
> > + set.param.value = 1;
> > + ext.base.next_extension = to_user_pointer(&set);
> > + igt_assert_eq(ctx_create_ioctl(i915, &create), 0);
> > + ext.base.next_extension = 0;
> > +
> > + /* new context should have updated prio... */
> > + set.param.ctx_id = create.ctx_id;
> > + gem_context_get_param(i915, &set.param);
> > + igt_assert_eq_u64(set.param.value, 1);
> > +
> > + /* but original context should have default prio */
> > + set.param.ctx_id = ext.clone_id;
> > + gem_context_get_param(i915, &set.param);
> > + igt_assert_eq_u64(set.param.value, 0);
> > +
> > + gem_context_destroy(i915, create.ctx_id);
> > + ext.clone_id = gem_context_create(i915);
> > + }
> > +
> > + gem_context_destroy(i915, ext.clone_id);
> > +}
> > +
> > +static void clone_engines(int i915)
> > +{
> > + struct drm_i915_gem_context_create_ext_setparam set = {
> > + { .name = I915_CONTEXT_CREATE_EXT_SETPARAM },
> > + { .param = I915_CONTEXT_PARAM_ENGINES },
> > + };
> > + struct drm_i915_gem_context_create_ext_clone ext = {
> > + { .name = I915_CONTEXT_CREATE_EXT_CLONE },
> > + .flags = I915_CONTEXT_CLONE_ENGINES,
> > + };
> > + struct drm_i915_gem_context_create_ext create = {
> > + .flags = I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS,
> > + .extensions = to_user_pointer(&ext),
> > + };
> > + I915_DEFINE_CONTEXT_PARAM_ENGINES(expected, 64);
> > + I915_DEFINE_CONTEXT_PARAM_ENGINES(engines, 64);
> > + uint64_t ex_size;
> > +
> > + memset(&expected, 0, sizeof(expected));
> > + memset(&engines, 0, sizeof(engines));
> > +
> > + igt_require(__gem_context_set_param(i915, &set.param) == 0);
> > +
> > + for (int pass = 0; pass < 2; pass++) { /* cloning default, then child */
> > + igt_debug("Cloning %d\n", ext.clone_id);
> > + igt_assert_eq(ctx_create_ioctl(i915, &create), 0);
> > +
> > + set.param.ctx_id = ext.clone_id;
> > + set.param.size = sizeof(expected);
> > + set.param.value = to_user_pointer(&expected);
> > + gem_context_get_param(i915, &set.param);
> > + ex_size = set.param.size;
> > +
> > + set.param.ctx_id = create.ctx_id;
> > + set.param.size = sizeof(engines);
> > + set.param.value = to_user_pointer(&engines);
> > + gem_context_get_param(i915, &set.param);
> > +
> > + igt_assert_eq_u64(set.param.param, I915_CONTEXT_PARAM_ENGINES);
> > + igt_assert_eq_u64(set.param.size, ex_size);
> > + igt_assert(!memcmp(&engines, &expected, ex_size));
> > +
> > + gem_context_destroy(i915, create.ctx_id);
> > +
> > + expected.engines[0].engine_class =
> > + I915_ENGINE_CLASS_INVALID;
> > + expected.engines[0].engine_instance =
> > + I915_ENGINE_CLASS_INVALID_NONE;
> > + ex_size = (sizeof(struct i915_context_param_engines) +
> > + sizeof(expected.engines[0]));
> > +
> > + set.param.ctx_id = ext.clone_id;
> > + set.param.size = ex_size;
> > + set.param.value = to_user_pointer(&expected);
> > + gem_context_set_param(i915, &set.param);
> > +
> > + igt_assert_eq(ctx_create_ioctl(i915, &create), 0);
> > +
> > + set.param.ctx_id = create.ctx_id;
> > + set.param.size = sizeof(engines);
> > + set.param.value = to_user_pointer(&engines);
> > + gem_context_get_param(i915, &set.param);
> > +
> > + igt_assert_eq_u64(set.param.size, ex_size);
> > + igt_assert(!memcmp(&engines, &expected, ex_size));
> > +
> > + gem_context_destroy(i915, create.ctx_id);
> > +
> > + /* clone but then reset engines to default */
> > + set.param.ctx_id = 0;
> > + set.param.size = 0;
> > + set.param.value = 0;
> > + ext.base.next_extension = to_user_pointer(&set);
> > +
> > + igt_assert_eq(ctx_create_ioctl(i915, &create), 0);
> > + ext.base.next_extension = 0;
> > +
> > + set.param.ctx_id = create.ctx_id;
> > + set.param.size = sizeof(engines);
> > + set.param.value = to_user_pointer(&engines);
> > + gem_context_get_param(i915, &set.param);
> > + igt_assert_eq_u64(set.param.size, 0);
> > +
> > + gem_context_destroy(i915, create.ctx_id);
> > +
> > + /* And check we ignore the flag */
> > + ext.flags = 0;
> > + igt_assert_eq(ctx_create_ioctl(i915, &create), 0);
> > + ext.flags = I915_CONTEXT_CLONE_ENGINES;
> > +
> > + set.param.ctx_id = create.ctx_id;
> > + set.param.size = sizeof(engines);
> > + set.param.value = to_user_pointer(&engines);
> > + gem_context_get_param(i915, &set.param);
> > + igt_assert_eq_u64(set.param.size, 0);
>
> It is quite hard to review/follow all these tests (and so gauge the
> coverage). It is a very stateful flow and for each step one has to
> remember/back-reference what is the currently active chain of
> extensions, and what is the active state of contexts and used context ids.
>
> Annoyingly I don't have any good ideas on how to easily and reasonably
> express this. Perhaps less reuse of the same stack objects in favour of
> dedicated helpers for querying would reduce the mess? Hard to say
> without trying it out.
>
> But I think something needs to be done since people will struggle to
> follow this if there is a bug one day.
[snip]
> It looks fine in principle so I leave to your conscience if you'll try
> to improve the readability. :) With the priority renamed to recoverable:
You know I care very little for negative testing that is better left to
fuzzing. Coverage, utility and versatility of these handwritten tests
barely justifies the effort. A lot of work to scratch the surface, that
fails to probe anything interesting.
-Chris
More information about the igt-dev
mailing list