[igt-dev] [PATCH i-g-t 1/1] tests/i915/gem_ctx_persistence: Set context with supported engines

[Chris Wilson]

Quoting Bommu Krishnaiah (2020-01-09 08:50:23)
> Update the context with supported engines on the platform with set_property
> I915_CONTEXT_PARAM_ENGINES to make sure the work load is submitted to
> the available engines only.
> 
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Signed-off-by: Bommu Krishnaiah <krishnaiah.bommu at intel.com>
> ---
>  tests/i915/gem_ctx_persistence.c | 34 ++++++++++++++++++--------------
>  1 file changed, 19 insertions(+), 15 deletions(-)
> 
> diff --git a/tests/i915/gem_ctx_persistence.c b/tests/i915/gem_ctx_persistence.c
> index d68431ae..e410e85f 100644
> --- a/tests/i915/gem_ctx_persistence.c
> +++ b/tests/i915/gem_ctx_persistence.c
> @@ -145,6 +145,15 @@ static void test_clone(int i915)
>         gem_context_destroy(i915, clone);
>  }
>  
> +static uint32_t create_context(int i915, bool persistent)
> +{
> +       uint32_t ctx;
> +
> +       ctx = gem_context_create(i915);
> +       gem_context_set_persistence(i915, ctx, persistent);
> +       gem_context_set_all_engines(i915, ctx);

Really? We only need a single engine, could you not be a little more
precise.

> +       return ctx;
> +}
>  static void test_persistence(int i915, unsigned int engine)
>  {
>         igt_spin_t *spin;
> @@ -156,8 +165,7 @@ static void test_persistence(int i915, unsigned int engine)
>          * request is retired -- no early termination.
>          */
>  
> -       ctx = gem_context_create(i915);
> -       gem_context_set_persistence(i915, ctx, true);
> +       ctx = create_context(i915, true);
>  
>         spin = igt_spin_new(i915, ctx,
>                             .engine = engine,
> @@ -188,8 +196,7 @@ static void test_nonpersistent_cleanup(int i915, unsigned int engine)
>          * any inflight request is cancelled.
>          */
>  
> -       ctx = gem_context_create(i915);
> -       gem_context_set_persistence(i915, ctx, false);
> +       ctx = create_context(i915, false);
>  
>         spin = igt_spin_new(i915, ctx,
>                             .engine = engine,
> @@ -217,8 +224,7 @@ static void test_nonpersistent_mixed(int i915, unsigned int engine)
>                 igt_spin_t *spin;
>                 uint32_t ctx;
>  
> -               ctx = gem_context_create(i915);
> -               gem_context_set_persistence(i915, ctx, i & 1);
> +               ctx = create_context(i915, i & 1);
>  
>                 spin = igt_spin_new(i915, ctx,
>                                     .engine = engine,
> @@ -253,8 +259,7 @@ static void test_nonpersistent_hostile(int i915, unsigned int engine)
>          * the requests and cleanup the context.
>          */
>  
> -       ctx = gem_context_create(i915);
> -       gem_context_set_persistence(i915, ctx, false);
> +       ctx = create_context(i915, false);
>  
>         spin = igt_spin_new(i915, ctx,
>                             .engine = engine,
> @@ -284,8 +289,8 @@ static void test_nonpersistent_hostile_preempt(int i915, unsigned int engine)
>  
>         igt_require(gem_scheduler_has_preemption(i915));
>  
> -       ctx = gem_context_create(i915);
> -       gem_context_set_persistence(i915, ctx, true);
> +       ctx = create_context(i915, true);
> +
>         gem_context_set_priority(i915, ctx, 0);
>         spin[0] = igt_spin_new(i915, ctx,
>                                .engine = engine,
> @@ -385,8 +390,8 @@ static void test_nonpersistent_queued(int i915, unsigned int engine)
>  
>         gem_quiescent_gpu(i915);
>  
> -       ctx = gem_context_create(i915);
> -       gem_context_set_persistence(i915, ctx, false);
> +       ctx = create_context(i915, false);
> +
>         spin = igt_spin_new(i915, ctx,
>                             .engine = engine,
>                             .flags = IGT_SPIN_FENCE_OUT);
> @@ -512,8 +517,7 @@ static void test_process_mixed(int i915, unsigned int engine)
>                         igt_spin_t *spin;
>                         uint32_t ctx;
>  
> -                       ctx = gem_context_create(i915);
> -                       gem_context_set_persistence(i915, ctx, persists);
> +                       ctx = create_context(i915, persists);
>  
>                         spin = igt_spin_new(i915, ctx,
>                                             .engine = engine,
> @@ -727,7 +731,7 @@ igt_main
>         igt_subtest("hangcheck")
>                 test_nohangcheck_hostile(i915);
>  
> -       __for_each_static_engine(e) {
> +       __for_each_physical_engine(i915, e) {

Please note this has to check ABI. We have to confirm that no matter how
the user tries to fool us, they cannot. So the gem_context_set_engines()
should be considered a separate step, or a very, very persuasive
argument made as to why we consider the multiple ABI paths equivalent,
bearing in mind the security implications of getting it wrong.
-Chris