util/prim_convert: Don't set index_bounds_valid
authorAlyssa Rosenzweig <alyssa@collabora.com>
Tue, 14 Mar 2023 03:16:43 +0000 (23:16 -0400)
committerMarge Bot <emma+marge@anholt.net>
Tue, 14 Mar 2023 23:10:00 +0000 (23:10 +0000)
draw->index_bounds_valid tells drivers that the values of min_index/max_index
are set correctly and can be used e.g. to allocate memory for varyings. If set
incorrectly, the GL promises badness.

But, with primconvert, we go mucking with index buffers and then never update
the bounds. So it doesn't matter if the original index bounds were valid, we
can't promise the original bounds are *still* valid. If we were trying to
optimize CPU overhead, we could try to preserve the new min/max index but seeing
as only older Mali cares about this flag, and if you're using primconvert you're
already screwed, I'm not too inclined to go rework primconvert.

Fixes* page faults in primitive-restart-draw-mode on Mali-G52 for GL_QUAD_STRIPS
and GL_POLYGON, which hit the primconvert path. The full dmesg splat looks like:

[ 5438.811727] panfrost ffe40000.gpu: Unhandled Page fault in AS0 at VA 0x000000100A16BAC0
             Reason: TODO
             raw fault status: 0x25002C1
             decoded fault status: SLAVE FAULT
             exception type 0xC1: TRANSLATION_FAULT_1
             access type 0x2: READ
             source id 0x250

Notice that a high bit is randomly set in the address, this is trying to read
a varying from the actual varying buffer in the vicinity of 0xa16bac0. What's
actually happening is that we're trying to read index #0 despite promising the
driver a minimum index of 2, causing an integer underflow as we try to read
index -2, or as the hardware sees, 4294967294.

As long as we stop lying to panfrost about the bounds being correct, panfrost is
able to calculate the real (post-primconverted) bounds on its own, fixing the
test.

* Alternatively, maybe Panfrost should just ignore this bit, in which I don't
  know why we have it in Gallium, since it's probably not conformant to fault on
  out-of-range glDrawRangeElements.

Fixes: 72ff53098c6 ("gallium: add pipe_draw_info::index_bounds_valid")
Signed-off-by: Alyssa Rosenzweig <alyssa@collabora.com>
Reviewed-by: Mike Blumenkrantz <michael.blumenkrantz@gmail.com>
Reviewed-by: Emma Anholt <emma@anholt.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/21891>

src/gallium/auxiliary/indices/u_primconvert.c
src/panfrost/ci/panfrost-g52-fails.txt

index dcbc905..526e7b6 100644 (file)
@@ -130,9 +130,15 @@ primconvert_init_draw(struct primconvert_context *pc,
       return false;
 
    util_draw_init_info(new_info);
-   new_info->index_bounds_valid = info->index_bounds_valid;
-   new_info->min_index = info->min_index;
-   new_info->max_index = info->max_index;
+
+   /* Because we've changed the index buffer, the original min_index/max_index
+    * for the draw are no longer valid. That's ok, but we need to tell drivers
+    * so they don't optimize incorrectly.
+    */
+   new_info->index_bounds_valid = false;
+   new_info->min_index = 0;
+   new_info->max_index = ~0;
+
    new_info->start_instance = info->start_instance;
    new_info->instance_count = info->instance_count;
    new_info->primitive_restart = info->primitive_restart;
index 9bf8ccc..c4144a2 100644 (file)
@@ -495,8 +495,6 @@ spec@nv_copy_image@nv_copy_image-formats --samples=4@Source: GL_RGBA32I/Destinat
 spec@nv_copy_image@nv_copy_image-formats --samples=4@Source: GL_RGBA32UI/Destination: GL_RGBA32UI,Fail
 spec@nv_copy_image@nv_copy_image-formats --samples=4@Source: GL_RGBA8/Destination: GL_RGBA8,Fail
 spec@nv_copy_image@nv_copy_image-formats --samples=4@Source: GL_RGBA8_SNORM/Destination: GL_RGBA8_SNORM,Fail
-spec@nv_primitive_restart@primitive-restart-draw-mode-polygon,Crash
-spec@nv_primitive_restart@primitive-restart-draw-mode-quad_strip,Crash
 spec@oes_texture_view@rendering-formats,Crash
 spec@oes_texture_view@sampling-2d-array-as-cubemap,Crash
 spec@!opengl 1.0@gl-1.0-edgeflag-const,Fail