Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
> Quoting Mika Kuoppala (2019-07-10 15:18:32)
>> Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
>> > - if (i915_vm_is_4lvl(vm)) {
>> > - if (unlikely(setup_page_dma(vm, &vm->scratch_pdp))) {
>> > - ret = -ENOMEM;
>> > - goto free_pd;
>> > - }
>> > - fill_page_dma(&vm->scratch_pdp,
>> > - gen8_pde_encode(vm->scratch_pdp.daddr,
>> > - I915_CACHE_LLC));
>> > + fill_px(&vm->scratch[i], vm->scratch[i - 1].encode);
>> > + vm->scratch[i].encode =
>> > + gen8_pde_encode(px_dma(&vm->scratch[i]),
>> > + I915_CACHE_LLC);
>>
>> Ok. The new code makes perfect sense here.
>>
>> And with it confusion arises: how did we manage
>> to get the old code work with pdp encoding pointing to itself?
>
> What the.... You're right. That must have caused some funky GPU hangs if
> people tried to access something far outside of their set.
>
> Hmm, you know that's exactly what live_contexts/vm_isolation tries. Well
> it tries to write into random invalid addresses and see if the writes
> affect scratch of another context. Did I choose randomly carefully
> enough? Hmm. offset &= -sizeof(u32); Is there a danger there that's only
> u32 and not u64. Maybe. But otherwise it looks like it should be picking
> a prng over the whole vm->total and so should be tripping over the
> recursion :|
Worth investigation surely.
But this patch looks good and makes it sane,
Reviewed-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
