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

[Chris Wilson]

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.
-Chris