Quoting Matthew Auld (2019-01-10 10:52:48)
> On Mon, 7 Jan 2019 at 11:55, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxx>
> > ---
> > + 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
