[Intel-gfx] [PATCH 39/46] drm/i915: Always allocate an object/vma for the HWSP

[Chris Wilson]

Quoting Matthew Auld (2019-01-10 10:52:48)
> On Mon, 7 Jan 2019 at 11:55, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> >
> > Currently we only allocate an object and vma if we are using a GGTT
> > virtual HWSP, and a plain struct page for a physical HWSP. For
> > convenience later on with global timelines, it will be useful to always
> > have the status page being tracked by a struct i915_vma. Make it so.
> >
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> > +       engine->status_page.addr = memset(vaddr, 0, PAGE_SIZE);
> >         engine->status_page.vma = vma;
> > -       engine->status_page.ggtt_offset = i915_ggtt_offset(vma);
> > -       engine->status_page.page_addr = memset(vaddr, 0, PAGE_SIZE);
> > +
> > +       if (!HWS_NEEDS_PHYSICAL(engine->i915)) {
> > +               ret = pin_ggtt_status_page(engine, vma);
> > +               if (ret)
> > +                       goto err_unpin;
> > +       }
> 
> Don't we now need special casing in gem_record_rings, since the error
> capture will now try to iterate over vma->pages for the status page?

The weird part is that I spent several hours debugging hangs on
Crestline with this patch enabled, and we even trigger GPU capture in
CI, neither died.

No idea as that for_each_sgt_dma(vma->pages) ought to be a NULL deref.
Whatever,
-       if (!vma)
+       if (!vma || !vma->pages)
                return NULL;
suffices.
-Chris