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

[Imre Deak <imre.deak@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 3646 bytes --]

On Fri, 2013-01-04 at 17:47 +0000, Chris Wilson wrote:
> 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.

On Ubuntu 12.10, with xf86-video-intel 2.20.9 I get the attached log
with the following patch applied:

diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c
b/drivers/gpu/drm/i915/i915_gem_tiling.c
index 7b0a3b3..846e96f 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -236,9 +236,17 @@ i915_tiling_ok(struct drm_device *dev, int stride,
int size, int tiling_mode)
 	if (INTEL_INFO(dev)->gen >= 4) {
 		if (stride & (tile_width - 1))
 			return false;
-		return true;
 	}
 
+	if (size % (stride / tile_width * PAGE_SIZE))
+		printk("unaligned tiling: comm %.*s size %u mode %c stride %d
size-mod-tilerow-size %zd\n",
+			(int)sizeof(current->comm), current->comm,
+			size, tiling_mode == I915_TILING_X ? 'X' : 'Y',
+			stride, size % (stride / tile_width * PAGE_SIZE));
+
+	if (INTEL_INFO(dev)->gen >= 4)
+		return true;
+
 	/* Pre-965 needs power of two tile widths */
 	if (stride < tile_width)
 		return false;



[-- Attachment #2: log --]
[-- Type: text/plain, Size: 4271 bytes --]

dmesg | grep unaligned | cut -c 16- | sort | uniq -c
      2 unaligned tiling: comm compiz size 10485760 mode X stride 6656 size-mod-tilerow-size 49152
      2 unaligned tiling: comm compiz size 10485760 mode Y stride 6400 size-mod-tilerow-size 40960
      1 unaligned tiling: comm compiz size 1572864 mode Y stride 2944 size-mod-tilerow-size 65536
      1 unaligned tiling: comm compiz size 163840 mode X stride 6144 size-mod-tilerow-size 16384
      1 unaligned tiling: comm compiz size 163840 mode Y stride 1536 size-mod-tilerow-size 16384
      2 unaligned tiling: comm compiz size 229376 mode Y stride 1152 size-mod-tilerow-size 8192
      1 unaligned tiling: comm compiz size 2621440 mode Y stride 4736 size-mod-tilerow-size 45056
      1 unaligned tiling: comm compiz size 262144 mode X stride 5120 size-mod-tilerow-size 16384
      2 unaligned tiling: comm compiz size 327680 mode X stride 6144 size-mod-tilerow-size 32768
      6 unaligned tiling: comm compiz size 327680 mode X stride 6656 size-mod-tilerow-size 8192
      1 unaligned tiling: comm compiz size 327680 mode Y stride 3072 size-mod-tilerow-size 32768
      1 unaligned tiling: comm compiz size 327680 mode Y stride 4736 size-mod-tilerow-size 24576
      1 unaligned tiling: comm compiz size 3670016 mode X stride 3072 size-mod-tilerow-size 8192
      1 unaligned tiling: comm compiz size 458752 mode Y stride 6400 size-mod-tilerow-size 49152
      1 unaligned tiling: comm compiz size 524288 mode Y stride 384 size-mod-tilerow-size 8192
      1 unaligned tiling: comm compiz size 6291456 mode X stride 6656 size-mod-tilerow-size 8192
      2 unaligned tiling: comm compiz size 6291456 mode Y stride 6400 size-mod-tilerow-size 147456
      6 unaligned tiling: comm compiz size 65536 mode Y stride 640 size-mod-tilerow-size 4096
      1 unaligned tiling: comm unity_support_t size 6291456 mode Y stride 6400 size-mod-tilerow-size 147456
     69 unaligned tiling: comm Xorg size 114688 mode X stride 2560 size-mod-tilerow-size 12288
    152 unaligned tiling: comm Xorg size 1310720 mode X stride 1536 size-mod-tilerow-size 8192
      3 unaligned tiling: comm Xorg size 131072 mode X stride 2560 size-mod-tilerow-size 8192
    317 unaligned tiling: comm Xorg size 131072 mode X stride 3072 size-mod-tilerow-size 8192
      4 unaligned tiling: comm Xorg size 163840 mode X stride 1536 size-mod-tilerow-size 4096
     87 unaligned tiling: comm Xorg size 163840 mode X stride 3072 size-mod-tilerow-size 16384
   1104 unaligned tiling: comm Xorg size 163840 mode X stride 3584 size-mod-tilerow-size 20480
      1 unaligned tiling: comm Xorg size 163840 mode X stride 4608 size-mod-tilerow-size 16384
      1 unaligned tiling: comm Xorg size 163840 mode X stride 6656 size-mod-tilerow-size 4096
      2 unaligned tiling: comm Xorg size 196608 mode X stride 2560 size-mod-tilerow-size 12288
     12 unaligned tiling: comm Xorg size 196608 mode X stride 3584 size-mod-tilerow-size 24576
     20 unaligned tiling: comm Xorg size 196608 mode X stride 4608 size-mod-tilerow-size 12288
      2 unaligned tiling: comm Xorg size 2097152 mode X stride 2560 size-mod-tilerow-size 8192
     10 unaligned tiling: comm Xorg size 229376 mode X stride 3072 size-mod-tilerow-size 8192
    193 unaligned tiling: comm Xorg size 2621440 mode X stride 4608 size-mod-tilerow-size 4096
      4 unaligned tiling: comm Xorg size 262144 mode X stride 1536 size-mod-tilerow-size 4096
     16 unaligned tiling: comm Xorg size 262144 mode X stride 4608 size-mod-tilerow-size 4096
      1 unaligned tiling: comm Xorg size 3145728 mode X stride 5632 size-mod-tilerow-size 36864
      3 unaligned tiling: comm Xorg size 327680 mode X stride 1536 size-mod-tilerow-size 8192
      2 unaligned tiling: comm Xorg size 393216 mode X stride 4608 size-mod-tilerow-size 24576
      3 unaligned tiling: comm Xorg size 40960 mode X stride 1536 size-mod-tilerow-size 4096
      1 unaligned tiling: comm Xorg size 5242880 mode X stride 5632 size-mod-tilerow-size 16384
    116 unaligned tiling: comm Xorg size 6291456 mode X stride 6656 size-mod-tilerow-size 8192
    280 unaligned tiling: comm Xorg size 65536 mode X stride 1536 size-mod-tilerow-size 4096
      6 unaligned tiling: comm Xorg size 65536 mode X stride 2560 size-mod-tilerow-size 4096

[-- Attachment #3: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx