[Intel-gfx] [PATCH] drm/i915: don't do allocate_va_range again on pin update

[Matthew Auld]

On 12 May 2017 at 10:31, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> On Fri, May 12, 2017 at 10:14:23AM +0100, Chris Wilson wrote:
>> From: Matthew Auld <matthew.auld at intel.com>
>>
>> If a vma is already bound to a ppgtt, we incorrectly call
>> allocate_va_range again when doing a PIN_UPDATE, which will result in
>> over accounting within our paging structures, such that when we do
>> unbind something we don't actually destroy the structures and end up
>> inadvertently recycling them. In reality this probably isn't too bad,
>> but once we start touching PDEs and PDPEs for 64K/2M/1G pages this
>> apparent recycling will manifest into lots of really, really subtle
>> bugs.
>>
>> v2: Fix the testing of vma->flags for aliasing_ppgtt_bind_vma
>>
>> Fixes: ff685975d97f ("drm/i915: Move allocate_va_range to GTT")
>> Signed-off-by: Matthew Auld <matthew.auld at intel.com>
>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
>> Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
>
> So, we are missing coverage of PIN_UPDATE and set-cache-level from the
> kselftests. Ideas?
>
> Matthew, do you have any clue how to reproduce those subtle errors?
The errors I encountered were only visible with huge-pages, for
example hitting the 4K PTE  insertion path while the PDE still points
to a stale 2M page and not a page-table.

I wanted to add a GEM_BUG_ON(pt->used_ptes > GEN8_PTES) as part of
this patch but the appgtt would make that a pain iiuc. Although I
don't really understand why we bother with
allocate_va_range/clear_range for the appgtt bind/unbind given that we
preallocate it anyway...

> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx