[igt-dev] [Intel-gfx] [PATCH igt] lib: Fix MI_BATCH_BUFFER_START for hang injection

[Chris Wilson]

Quoting Ville Syrjälä (2018-03-02 17:09:29)
> On Fri, Mar 02, 2018 at 04:13:46PM +0000, Chris Wilson wrote:
> > A couple of bugs inside the hang injector, the worst being that the
> > presumed_offset of the reloc didn't match the batch; so if the reloc was
> > skipped (as the presumed_offset matched the reloc offset), the batch
> > wasn't updated and so we may not have generated a hanging batch at all!
> > Secondly, the MI_BATCH_BUFFER_START was not correct for all gen.
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >  lib/igt_gt.c | 28 +++++++++++++++++++++-------
> >  1 file changed, 21 insertions(+), 7 deletions(-)
> > 
> > diff --git a/lib/igt_gt.c b/lib/igt_gt.c
> > index e630550b..799ca1ae 100644
> > --- a/lib/igt_gt.c
> > +++ b/lib/igt_gt.c
> > @@ -276,6 +276,7 @@ igt_hang_t igt_hang_ctx(int fd,
> >       uint32_t b[16];
> >       unsigned ban;
> >       unsigned len;
> > +     int gen;
> >  
> >       igt_require_hang_ring(fd, ring);
> >  
> > @@ -310,12 +311,26 @@ igt_hang_t igt_hang_ctx(int fd,
> >  
> >       memset(b, 0xc5, sizeof(b));
> >  
> > -     len = 2;
> > -     if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> > +     len = 0;
> > +     gen = intel_gen(intel_get_drm_devid(fd));
> > +     if (gen >= 8) {
> > +             b[len++] = MI_BATCH_BUFFER_START | 1 << 8 | 1;
> > +             b[len++] = 0;
> > +             b[len++] = 0;
> > +     } else if (gen >= 6) {
> > +             b[len++] = MI_BATCH_BUFFER_START | 1 << 8;
> > +             b[len++] = 0;
> 
> ppgtt for gen6+
> 
> > +     } else {
> > +             b[len++] = MI_BATCH_BUFFER_START | 2 << 6;
> > +             b[len] = 0;
> > +             if (gen < 4) {
> > +                     b[len] |= 1;
> > +                     reloc.delta = 1;
> > +             }
> >               len++;
> 
> gtt secure for gen4/5
> gtt non-secure for gen2/3
> 
> How does the security thing work on ctg/ilk for chained batches? The spec
> is a wee bit unclear. It says the security bit for chained batches is
> ignored, but then it also says non-secure batches can't access the gtt.
> That was the case for MI_STORE_DWORD if I recall your earlier patch
> correctly. So if we don't execute the first batch as secure the chained
> MI_BB_START gets nopped out maybe?

Yes, as soon as we lose the privilege bit (usually when the kernel does
the first MI_BB to us), we can regain privilege in this batch chain. How
this works with the ppgtt is a mystery, but it also functions
differently if it's not enabled.
 
> Hmm. Now I wonder how the earlier MI_STORE_DWORD thing worked on pre-ctg
> with a non-secure batch? Wasn't the hardware supposed to nop those out
> entirely? /me confused

Aiui, before ctg/ilk they remained unprivileged ops.
-Chris