[Intel-gfx] [PATCH i-g-t v2] i915/gem_ctx_exec: Cover all engines for nohangcheck

[Chris Wilson]

Quoting Chris Wilson (2020-02-05 11:52:11)
> Quoting Tvrtko Ursulin (2020-02-05 11:48:42)
> > 
> > On 04/02/2020 16:19, Chris Wilson wrote:
> > > No engine can be missed when verifying that a rogue user cannot cause a
> > > denial-of-service with nohangcheck.
> > > 
> > > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > > Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> > > ---
> > > __for_each_physical_engine, keep the leaks
> > > ---
> > >   tests/i915/gem_ctx_exec.c | 38 ++++++++++++++++++++++++++++++++------
> > >   1 file changed, 32 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/tests/i915/gem_ctx_exec.c b/tests/i915/gem_ctx_exec.c
> > > index b1ae65774..aeb8d2976 100644
> > > --- a/tests/i915/gem_ctx_exec.c
> > > +++ b/tests/i915/gem_ctx_exec.c
> > > @@ -42,6 +42,7 @@
> > >   
> > >   #include "igt_dummyload.h"
> > >   #include "igt_sysfs.h"
> > > +#include "sw_sync.h"
> > >   
> > >   IGT_TEST_DESCRIPTION("Test context batch buffer execution.");
> > >   
> > > @@ -203,9 +204,9 @@ static bool __enable_hangcheck(int dir, bool state)
> > >   
> > >   static void nohangcheck_hostile(int i915)
> > >   {
> > > -     int64_t timeout = NSEC_PER_SEC / 2;
> > > -     igt_spin_t *spin;
> > > +     const struct intel_execution_engine2 *e;
> > >       igt_hang_t hang;
> > > +     int fence = -1;
> > >       uint32_t ctx;
> > >       int err = 0;
> > >       int dir;
> > > @@ -215,6 +216,8 @@ static void nohangcheck_hostile(int i915)
> > >        * we forcibly terminate that context.
> > >        */
> > >   
> > > +     i915 = gem_reopen_driver(i915);
> > > +
> > >       dir = igt_sysfs_open_parameters(i915);
> > >       igt_require(dir != -1);
> > >   
> > > @@ -223,16 +226,35 @@ static void nohangcheck_hostile(int i915)
> > >   
> > >       igt_require(__enable_hangcheck(dir, false));
> > >   
> > > -     spin = igt_spin_new(i915, ctx, .flags = IGT_SPIN_NO_PREEMPTION);
> > > +     __for_each_physical_engine(i915, e) {
> > > +             igt_spin_t *spin;
> > > +
> > > +             spin = igt_spin_new(i915, ctx,
> > > +                                 .engine = e->flags,
> > 
> > Ouch, I missed a mismatch between ctx and e->flags here. Thanks to 
> > Sreedhar for reporting it.
> > 
> > We either need gem_context_set_all_engines back or to rethink a cleaner 
> > strategy.
> 
> Copy engines, or pass the ctx into __for_each_physical_engine.

Which [copy engines] reveals a bug... More missing tests in
gem_ctx_persistence.
-Chris