Bug 90734

Created attachment 116223 [details]
test case

Comment on attachment 116223 [details]
test case


Added a test case.
Also tested on a gen4 chip, and the error appears.

From oibaf ppa:
xserver-xorg-video-intel
2:2.99.917+git1506011932.189ad4~gd~t

diff --git a/src/mesa/drivers/dri/i965/intel_blit.c b/src/mesa/drivers/dri/i965/intel_blit.c
index 4bfa0ef..a73da35 100644
--- a/src/mesa/drivers/dri/i965/intel_blit.c
+++ b/src/mesa/drivers/dri/i965/intel_blit.c
@@ -539,23 +539,26 @@ intel_emit_linear_blit(struct brw_context *brw,
     */
    pitch = ROUND_DOWN_TO(MIN2(size, (1 << 15) - 1), 4);
    height = (pitch == 0) ? 1 : size / pitch;
+   if (height) {
+      src_x = src_offset % 64;
+      dst_x = dst_offset % 64;
+      ok = intelEmitCopyBlit(brw, 4,
+                             pitch, src_bo, src_offset - src_x, I915_TILING_NONE,
+                             pitch, dst_bo, dst_offset - dst_x, I915_TILING_NONE,
+                             src_x/4, 0, /* src x/y */
+                             dst_x/4, 0, /* dst x/y */
+                             pitch/4, height, /* w, h */
+                             GL_COPY);
+      if (!ok)
+         _mesa_problem(ctx, "Failed to linear blit %dx%d\n", pitch, height);
+
+      src_offset += pitch * height;
+      dst_offset += pitch * height;
+      size -= pitch * height;
+   }
+
    src_x = src_offset % 64;
    dst_x = dst_offset % 64;
-   ok = intelEmitCopyBlit(brw, 1,
-                         pitch, src_bo, src_offset - src_x, I915_TILING_NONE,
-                         pitch, dst_bo, dst_offset - dst_x, I915_TILING_NONE,
-                         src_x, 0, /* src x/y */
-                         dst_x, 0, /* dst x/y */
-                         pitch, height, /* w, h */
-                         GL_COPY);
-   if (!ok)
-      _mesa_problem(ctx, "Failed to linear blit %dx%d\n", pitch, height);
-
-   src_offset += pitch * height;
-   dst_offset += pitch * height;
-   src_x = src_offset % 64;
-   dst_x = dst_offset % 64;
-   size -= pitch * height;
    assert (size < (1 << 15));
    pitch = ALIGN(size, 4);

No space in margin for explanation! (It's the coordinates after the alignment adjustment exceeding 32k, the fix here is to translate them into 32bpp copy instead for the well aligned large transfer.)

Well, I built mesa with the patch and the test is ok but my application is not. I use various buffer size and update is quasi random so I modified the test case to reflect this and the first errors were at the 64k boundary.

Changing from :
//#define ELEMENT_CNT (4095 * 2) // NO ERROR
#define ELEMENT_CNT (4096 * 2) // ERROR, >= 4096

to :

//#define ELEMENT_CNT (8191 * 2) // NO ERROR
#define ELEMENT_CNT (8192 * 2) //  ERROR, >= 8192

catches more errors.

Tested on sandy bridge hd 2000, in bytes as it seems to be the relevant factor :

buffer size 65536 updating [44-65536] size 65492 bytes
buffer size 65536 updating [64-65536] size 65472 bytes
buffer size 65536 updating [56-65536] size 65480 bytes

And increasing the buffer size:
buffer size 400000 updating [50820-149076] size 98256 bytes
buffer size 400000 updating [11312-142364] size 131052 bytes

buffer size 4000000 updating [3255368-3484676] size 229308 bytes
buffer size 4000000 updating [2179900-2802388] size 622488 bytes
...

Sorry for the late reply, I had rewritten the patch to try and make the logic more robust, could you please test http://patchwork.freedesktop.org/patch/56678/ ?

(In reply to Chris Wilson from comment #6)
> Sorry for the late reply, I had rewritten the patch to try and make the
> logic more robust, could you please test
> http://patchwork.freedesktop.org/patch/56678/ ?

I have exhumed the old version of my application with the test case included, compiled current git mesa (Mesa 11.1.0-devel (git-000e225)) and applied the patch manually.

Testing without the patch applied give me the errors described in my initial bug report.

Testing with the patch applied and running with a buffer of max size 125000 * 3 vertices : millions of changes later, no errors logged and the output is visually as expected. I've only tested on sandy bridge hd2000, but this seems ok to me.

commit d38a5601068ae1d923efece8f28757777f4474e4
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Sat Jun 6 09:33:33 2015 +0100

    i965: Prevent coordinate overflow in intel_emit_linear_blit