Module: Mesa Branch: main Commit: ad6a036a6815f973355c2500023ddaf780593394 URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=ad6a036a6815f973355c2500023ddaf780593394
Author: Paulo Zanoni <[email protected]> Date: Thu Jan 12 10:23:15 2023 -0800 anv: don't leave undefined values in exec->syncobj_values 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 <[email protected]> Reviewed-by: Lionel Landwerlin <[email protected]> Signed-off-by: Paulo Zanoni <[email protected]> Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20703> --- src/intel/vulkan/i915/anv_batch_chain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/intel/vulkan/i915/anv_batch_chain.c b/src/intel/vulkan/i915/anv_batch_chain.c index 56754725165..4e71985a832 100644 --- a/src/intel/vulkan/i915/anv_batch_chain.c +++ b/src/intel/vulkan/i915/anv_batch_chain.c @@ -241,7 +241,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++;
