[Intel-gfx] [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 Intel-gfx mailing list