Re: [PATCH 5/5] drm/i915: fix gtt space allocated for tiled objects

[Chris Wilson <chris@chris-wilson.co.uk>]

On Fri, 04 Jan 2013 19:23:42 +0200, Imre Deak <imre.deak@intel.com> wrote:
> On Fri, 2013-01-04 at 17:07 +0000, Chris Wilson wrote:
> > On Fri,  4 Jan 2013 18:42:00 +0200, Imre Deak <imre.deak@intel.com> wrote:
> > > The gtt space needed for tiled objects might be bigger than the linear
> > > size programmed into the correpsonding fence register. For example for
> > > the following buffer on a Gen5+ HW:
> > > 
> > > - allocation size: 4096 bytes
> > > - tiling mode: X tiled
> > > - stride: 1536
> > > 
> > > we need (1536 / 512) * 4096 bytes of gtt space to cover all the pixels
> > > in the buffer, but at the moment we allocate only 4096. This means that
> > > any buffer following this tiled buffer in the gtt space will be
> > > corrupted if pixels belonging to the 2nd and 3rd tiles are written.
> > > 
> > > Fix this by rounding up the size of the allocated gtt space to the next
> > > tile row address. The page frames beyond the allocation size will be
> > > backed by the single gtt scratch page used already elsewhere for similar
> > > padding.
> > > 
> > > Note that this is more of a security/robustness problem and not fixing any
> > > reported issue that I know of. This is because applications will normally
> > > access only the part of the buffer that is tile row size aligned.
> > 
> > There should not be any reported issues because all userspace already
> > allocates up to the end of tile-row and stride should be enforced to be
> > a multiple of tile-width. So the use of DIV_ROUND_UP implies a
> > programming error that should have been reported back to userspace
> > earlier. We can extend that by checking to make sure userspace has
> > allocated a valid buffer, that is, it has allocated sufficient pages for
> > the sampler access into the tiled buffer (or reject the set-tiling).
> > -Chris
> 
> Ok, I tested this with older UXA that still allocated non-aligned
> buffers. If that's not the case any more then rejecting set-tiling if
> it's called on a non tile-row size aligned buffer would work too.

Meep. A long time ago we got the calcuations wrong (slightly less for
gen2), but it was and still is a userspace bug with the potential of
handing the GPU.

A multiple of tile_height tall and a mutiple of tile_width across should
always be an exact number of pages (and an exact multiple of tile-row
pages). And we should have been obeying that since the introduction of
set-tiling (ignoring the aforementioned bugs) - so I'd really like to
see any evidence of userspace getting that wrong.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre