Re: [Intel-gfx] [PATCH v6 3/5] drm/i915: Introduce guard pages to i915_vma

[Andi Shyti <andi.shyti@linux.intel.com>]

Hi Tvrtko,

On Fri, Dec 02, 2022 at 10:20:11AM +0000, Tvrtko Ursulin wrote:
> 
> On 01/12/2022 20:39, Andi Shyti wrote:
> > From: Chris Wilson <chris@chris-wilson.co.uk>
> > 
> > Introduce the concept of padding the i915_vma with guard pages before
> > and after. The major consequence is that all ordinary uses of i915_vma
> > must use i915_vma_offset/i915_vma_size and not i915_vma.node.start/size
> > directly, as the drm_mm_node will include the guard pages that surround
> > our object.
> > 
> > The biggest connundrum is how exactly to mix requesting a fixed address
> > with guard pages, particularly through the existing uABI. The user does
> > not know about guard pages, so such must be transparent to the user, and
> > so the execobj.offset must be that of the object itself excluding the
> > guard. So a PIN_OFFSET_FIXED must then be exclusive of the guard pages.
> > The caveat is that some placements will be impossible with guard pages,
> > as wrap arounds need to be avoided, and the vma itself will require a
> > larger node. We must not report EINVAL but ENOSPC as these are unavailable
> > locations within the GTT rather than conflicting user requirements.
> > 
> > In the next patch, we start using guard pages for scanout objects. While
> > these are limited to GGTT vma, on a few platforms these vma (or at least
> > an alias of the vma) is shared with userspace, so we may leak the
> > existence of such guards if we are not careful to ensure that the
> > execobj.offset is transparent and excludes the guards. (On such platforms
> > like ivb, without full-ppgtt, userspace has to use relocations so the
> > presence of more untouchable regions within its GTT such be of no further
> > issue.)
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Tejas Upadhyay <tejaskumarx.surendrakumar.upadhyay@intel.com>
> > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
> > ---
> > Hi Tvrtko,
> > 
> > I removed your r-b in this version because I restored the original value
> > of the guard being aligned with the vma size alignment. Turns out that
> > CI failed with the latest version because the guard was becoming too big
> > (we would have hit the GEM_BUG_ON)[*].
> > 
> > The reason why now the guard is aligned with the vma alignment is that
> > the area is already aligned and if we use as a starting address start +
> > guard, guard needs to be aligned, otherwise we screw up all the memory
> > alignment.
> > 
> > Let me know if it makes sense to you.
> 
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Conditional to promise of a prioritised follow up improvement, if it turns
> out GGTT wastage due a bit over zealous guard size comes to bite.

Sure! I'll be alert!

There are some unrelated failures from CI, just to be sure I sent
last night a trybot run.

Thanks, Tvrtko!

Andi