From ba2d22e95f3313450fc4ce80fb900f8e0dd462f3 Mon Sep 17 00:00:00 2001 From: Samuel Pitoiset Date: Wed, 19 Jan 2022 16:19:11 +0100 Subject: [PATCH] Revert "radv: re-apply "Do not access set layout during vkCmdBindDescriptorSets."" The most famous RADV revert over the past months. This was an issue in RADV and not an use-after-free (descriptor set layouts can be destroyed almost at any time). This reverts commit b775aaff1ec86f4ebd50867a045695da1fbeb2e1. Signed-off-by: Samuel Pitoiset Reviewed-by: Bas Nieuwenhuizen Part-of: (cherry picked from commit 9ea4029f9fea4ed6131c72a65981b39eb0e2353d) --- .pick_status.json | 2 +- src/amd/vulkan/radv_cmd_buffer.c | 5 +++-- src/amd/vulkan/radv_descriptor_set.c | 9 ++------- src/amd/vulkan/radv_descriptor_set.h | 4 +--- 4 files changed, 7 insertions(+), 13 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index b46b6c5ad18..d89300e1d7e 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -193,7 +193,7 @@ "description": "Revert \"radv: re-apply \"Do not access set layout during vkCmdBindDescriptorSets.\"\"", "nominated": true, "nomination_type": 2, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": "b775aaff1ec86f4ebd50867a045695da1fbeb2e1" }, diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c index 7c87e78fd5e..405c98bd17e 100644 --- a/src/amd/vulkan/radv_cmd_buffer.c +++ b/src/amd/vulkan/radv_cmd_buffer.c @@ -4727,6 +4727,7 @@ radv_bind_descriptor_set(struct radv_cmd_buffer *cmd_buffer, VkPipelineBindPoint radv_set_descriptor_set(cmd_buffer, bind_point, set, idx); assert(set); + assert(!(set->header.layout->flags & VK_DESCRIPTOR_SET_LAYOUT_CREATE_PUSH_DESCRIPTOR_BIT_KHR)); if (!cmd_buffer->device->use_global_bo_list) { for (unsigned j = 0; j < set->header.buffer_count; ++j) @@ -4764,7 +4765,7 @@ radv_CmdBindDescriptorSets(VkCommandBuffer commandBuffer, VkPipelineBindPoint pi radv_bind_descriptor_set(cmd_buffer, pipelineBindPoint, set, set_idx); } - for (unsigned j = 0; j < layout->set[set_idx].dynamic_offset_count; ++j, ++dyn_idx) { + for (unsigned j = 0; j < set->header.layout->dynamic_offset_count; ++j, ++dyn_idx) { unsigned idx = j + layout->set[i + firstSet].dynamic_offset_start; uint32_t *dst = descriptors_state->dynamic_buffers + idx * 4; assert(dyn_idx < dynamicOffsetCount); @@ -4790,7 +4791,7 @@ radv_CmdBindDescriptorSets(VkCommandBuffer commandBuffer, VkPipelineBindPoint pi } } - cmd_buffer->push_constant_stages |= layout->set[set_idx].dynamic_offset_stages; + cmd_buffer->push_constant_stages |= set->header.layout->dynamic_shader_stages; } } } diff --git a/src/amd/vulkan/radv_descriptor_set.c b/src/amd/vulkan/radv_descriptor_set.c index 56baa8ae73a..1365dde1b3d 100644 --- a/src/amd/vulkan/radv_descriptor_set.c +++ b/src/amd/vulkan/radv_descriptor_set.c @@ -496,16 +496,11 @@ radv_CreatePipelineLayout(VkDevice _device, const VkPipelineLayoutCreateInfo *pC layout->set[set].layout = set_layout; layout->set[set].dynamic_offset_start = dynamic_offset_count; - layout->set[set].dynamic_offset_count = 0; - layout->set[set].dynamic_offset_stages = 0; for (uint32_t b = 0; b < set_layout->binding_count; b++) { - layout->set[set].dynamic_offset_count += - set_layout->binding[b].array_size * set_layout->binding[b].dynamic_offset_count; - layout->set[set].dynamic_offset_stages |= set_layout->dynamic_shader_stages; + dynamic_offset_count += set_layout->binding[b].array_size * set_layout->binding[b].dynamic_offset_count; + dynamic_shader_stages |= set_layout->dynamic_shader_stages; } - dynamic_offset_count += layout->set[set].dynamic_offset_count; - dynamic_shader_stages |= layout->set[set].dynamic_offset_stages; /* Hash the entire set layout except for the vk_object_base. The * rest of the set layout is carefully constructed to not have diff --git a/src/amd/vulkan/radv_descriptor_set.h b/src/amd/vulkan/radv_descriptor_set.h index 1038bc52b16..298c4c62fcb 100644 --- a/src/amd/vulkan/radv_descriptor_set.h +++ b/src/amd/vulkan/radv_descriptor_set.h @@ -89,9 +89,7 @@ struct radv_pipeline_layout { struct { struct radv_descriptor_set_layout *layout; uint32_t size; - uint16_t dynamic_offset_start; - uint16_t dynamic_offset_count; - VkShaderStageFlags dynamic_offset_stages; + uint32_t dynamic_offset_start; } set[MAX_SETS]; uint32_t num_sets;