fdo-mirrors/mesa
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/mesa/mesa.git synced 2026-05-05 03:08:05 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

anv: don't leave undefined values in exec->syncobj_values

Browse source 
In anv_execbuf_add_syncobj(), we try to not create or use
exec->syncobj_values if we don't need to. But when we figure we're
going to need it (i.e., when timeline_value is not zero), then we
create exec->syncobj_values with vk_zalloc, which means every previous
value is set to zero, as it should be. This is all correct.

The problem starts when we add a 16th element. In this case we double
exec->syncobj_array_length and realloc the buffer by using vk_alloc
and copying the old array to the new one. After that, we write the
timeline_value to the array only if it's not zero, and that's the
problem: since we just used vkalloc and memcpy, we don't have any
guarantees that the new array will be zero after the 16th element, and
if timeline_value is zero we write nothing to that position.

Once we start using exec->syncobj_values we have to commit to using
it, so the "if (timeline_value)" check near the end of the function
has to be changed to "if (exec->syncobj_values)", so we actually set
elements after the 16th to zero when they need to be zero.  Another
approach to fix this would be to memset the new elements once we
double syncobj_array_length.

In practice, I couldn't find any application or deqp test that used
more than 3 elements in exec->syncobj_array_length, and we need more
than 16 elements in order to be able to reproduce the bug, so I'm not
aware of any real-world bug that goes away with this patch. This issue
was found while reading code.

If we craft a little Vulkan program that submits a ton of timeline and
binary semaphores on vkQueueSubmit, then waits for them, we get the
following error without this patch:

MESA: error: ../../src/intel/vulkan/anv_batch_chain.c:1910: execbuf2 failed: Invalid argument (VK_ERROR_DEVICE_LOST)

v2: Rebase.

Cc: mesa-stable
Reviewed-by: Ivan Briano <ivan.briano@intel.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20703>
(cherry picked from commit ad6a036a68)

Conflicts:
	src/intel/vulkan/i915/anv_batch_chain.c

Stable:
    reapplied to src/intel/vulkan/anv_batch_chain.c, as this hasn't been
    moved in the staging branch.
This commit is contained in:
Paulo Zanoni 2023-01-12 10:23:15 -08:00 • committed by Dylan Baker
parent 34055e7822
commit 2d11fda237
2 changed files with 2 additions and 2 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
.pick_status.json
View file

@ -607,7 +607,7 @@
"description": "anv: don't leave undefined values in exec->syncobj_values",
"nominated": true,
"nomination_type": 0,
"resolution": 0,
"resolution": 1,
"main_sha": null,
"because_sha": null
},

2
src/intel/vulkan/anv_batch_chain.c
View file

@ -1350,7 +1350,7 @@ anv_execbuf_add_syncobj(struct anv_device *device,
.handle = syncobj,
.flags = flags,
};
if (timeline_value)
if (exec->syncobj_values)
exec->syncobj_values[exec->syncobj_count] = timeline_value;
exec->syncobj_count++;