Re: [Intel-gfx] [PATCH 0/3] add guard padding around i915_vma

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

Hi,

On 09/11/2022 18:03, Thomas Hellström wrote:
> Hi, Andi,
> 
> This has been on the list before (three times I think) and at that
> point it (the guard pages) was NAK'd by Daniel as yet another
> complication, and a VT-d
> scanout workaround was implemented and pushed using a different
> approach, initially outlined by Daniel.

I can't find this discussion and NAKs on the list - do you have a link?

> Patch is 2ef6efa79fecd. Those suspend/resumes should now be fast.

So the initiator to re-start this series was actually the boot time is 
failing KPIs by quite a margin. Which means we may need a way forward 
after all. Especially if the most churny patch 1 was deemed okay, then I 
don't see why the concept of guard pages should be a problem. But again, 
I couldn't find the discussion you mention to read what were the 
objections..

For 2ef6efa79fecd specifically. I only looked at it today - do you think 
that the heuristic of checking one PTE and deciding all content was 
preserved is safe? What if someone scribbled at random locations? On a 
first thought it is making me a bit uncomfortable.

Regards,

Tvrtko

> I then also discussed patch 1 separately with Dave Airlie and Daniel
> and since both me and Dave liked it, Daniel OK'd it, but it never made
> it upstream.
> 
> Just a short heads up on the history.
> 
> /Thomas
> 
> 
> On Wed, 2022-11-09 at 18:40 +0100, Andi Shyti wrote:
>> Hi,
>>
>> This series adds guards around vma's but setting a pages at the
>> beginning and at the end that work as padding.
>>
>> The first user of the vma guard are scanout objects which don't
>> need anymore to add scratch to all the unused ggtt's and speeding
>> up up considerably the boot and resume by several hundreds of
>> milliseconds up to over a full second in slower machines.
>>
>> Andi
>>
>> Chris Wilson (3):
>>    drm/i915: Wrap all access to i915_vma.node.start|size
>>    drm/i915: Introduce guard pages to i915_vma
>>    drm/i915: Refine VT-d scanout workaround
>>
>>   drivers/gpu/drm/i915/display/intel_fbdev.c    |  2 +-
>>   drivers/gpu/drm/i915/gem/i915_gem_domain.c    | 13 ++++
>>   .../gpu/drm/i915/gem/i915_gem_execbuffer.c    | 33 ++++++-----
>>   drivers/gpu/drm/i915/gem/i915_gem_mman.c      |  2 +-
>>   drivers/gpu/drm/i915/gem/i915_gem_shrinker.c  |  2 +-
>>   drivers/gpu/drm/i915/gem/i915_gem_tiling.c    |  4 +-
>>   .../gpu/drm/i915/gem/selftests/huge_pages.c   |  2 +-
>>   .../i915/gem/selftests/i915_gem_client_blt.c  | 23 ++++----
>>   .../drm/i915/gem/selftests/i915_gem_context.c | 15 +++--
>>   .../drm/i915/gem/selftests/i915_gem_mman.c    |  2 +-
>>   .../drm/i915/gem/selftests/igt_gem_utils.c    |  7 ++-
>>   drivers/gpu/drm/i915/gt/gen7_renderclear.c    |  2 +-
>>   drivers/gpu/drm/i915/gt/intel_ggtt.c          | 39 ++++--------
>>   drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c  |  3 +-
>>   drivers/gpu/drm/i915/gt/intel_renderstate.c   |  2 +-
>>   .../gpu/drm/i915/gt/intel_ring_submission.c   |  2 +-
>>   drivers/gpu/drm/i915/gt/selftest_engine_cs.c  |  8 +--
>>   drivers/gpu/drm/i915/gt/selftest_execlists.c  | 18 +++---
>>   drivers/gpu/drm/i915/gt/selftest_hangcheck.c  | 15 ++---
>>   drivers/gpu/drm/i915/gt/selftest_lrc.c        | 16 ++---
>>   .../drm/i915/gt/selftest_ring_submission.c    |  2 +-
>>   drivers/gpu/drm/i915/gt/selftest_rps.c        | 12 ++--
>>   .../gpu/drm/i915/gt/selftest_workarounds.c    |  8 +--
>>   drivers/gpu/drm/i915/i915_cmd_parser.c        |  4 +-
>>   drivers/gpu/drm/i915/i915_debugfs.c           |  2 +-
>>   drivers/gpu/drm/i915/i915_gem_gtt.h           |  3 +-
>>   drivers/gpu/drm/i915/i915_perf.c              |  2 +-
>>   drivers/gpu/drm/i915/i915_vma.c               | 59 +++++++++++++----
>> --
>>   drivers/gpu/drm/i915/i915_vma.h               | 52 +++++++++++++++-
>>   drivers/gpu/drm/i915/i915_vma_resource.c      |  4 +-
>>   drivers/gpu/drm/i915/i915_vma_resource.h      | 17 ++++--
>>   drivers/gpu/drm/i915/i915_vma_types.h         |  3 +-
>>   drivers/gpu/drm/i915/selftests/i915_request.c | 20 +++----
>>   drivers/gpu/drm/i915/selftests/igt_spinner.c  |  8 +--
>>   34 files changed, 246 insertions(+), 160 deletions(-)
>>
>