From 5a85c4432c0073e0f458e5e3c8a74ee641b5edd0 Mon Sep 17 00:00:00 2001 From: Tatsuyuki Ishi Date: Wed, 1 Feb 2023 17:40:02 +0900 Subject: [PATCH] radv: Guard against misplaced RGP barrier markers. RGP will crash if we emit a layout transition marker outside a barrier. If this happens, trigger an assertion if its enabled or silently discard the marker otherwise to avoid traces that cannot be opened. Also, guard against attempts to start barrier markers recursively, since this will corrupt the internal start/end matching. Closes: Reviewed-by: Samuel Pitoiset Part-of: --- src/amd/vulkan/layers/radv_sqtt_layer.c | 12 ++++++++++++ src/amd/vulkan/meta/radv_meta_copy.c | 4 ---- src/amd/vulkan/radv_cmd_buffer.c | 8 +++++--- src/amd/vulkan/radv_private.h | 1 + 4 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/amd/vulkan/layers/radv_sqtt_layer.c b/src/amd/vulkan/layers/radv_sqtt_layer.c index 2895c8b..acad0f4 100644 --- a/src/amd/vulkan/layers/radv_sqtt_layer.c +++ b/src/amd/vulkan/layers/radv_sqtt_layer.c @@ -463,8 +463,14 @@ radv_describe_barrier_start(struct radv_cmd_buffer *cmd_buffer, enum rgp_barrier if (likely(!cmd_buffer->device->sqtt.bo)) return; + if (cmd_buffer->state.in_barrier) { + assert(!"attempted to start a barrier while already in a barrier"); + return; + } + radv_describe_barrier_end_delayed(cmd_buffer); cmd_buffer->state.sqtt_flush_bits = 0; + cmd_buffer->state.in_barrier = true; marker.identifier = RGP_SQTT_MARKER_IDENTIFIER_BARRIER_START; marker.cb_id = cmd_buffer->sqtt_cb_id; @@ -476,6 +482,7 @@ radv_describe_barrier_start(struct radv_cmd_buffer *cmd_buffer, enum rgp_barrier void radv_describe_barrier_end(struct radv_cmd_buffer *cmd_buffer) { + cmd_buffer->state.in_barrier = false; cmd_buffer->state.pending_sqtt_barrier_end = true; } @@ -488,6 +495,11 @@ radv_describe_layout_transition(struct radv_cmd_buffer *cmd_buffer, if (likely(!cmd_buffer->device->sqtt.bo)) return; + if (!cmd_buffer->state.in_barrier) { + assert(!"layout transition marker should be only emitted inside a barrier marker"); + return; + } + marker.identifier = RGP_SQTT_MARKER_IDENTIFIER_LAYOUT_TRANSITION; marker.depth_stencil_expand = barrier->layout_transitions.depth_stencil_expand; marker.htile_hiz_range_expand = barrier->layout_transitions.htile_hiz_range_expand; diff --git a/src/amd/vulkan/meta/radv_meta_copy.c b/src/amd/vulkan/meta/radv_meta_copy.c index b29aeb4..caad9b4 100644 --- a/src/amd/vulkan/meta/radv_meta_copy.c +++ b/src/amd/vulkan/meta/radv_meta_copy.c @@ -555,8 +555,6 @@ copy_image(struct radv_cmd_buffer *cmd_buffer, struct radv_image *src_image, if (radv_layout_is_htile_compressed(cmd_buffer->device, dst_image, dst_image_layout, queue_mask)) { - radv_describe_barrier_start(cmd_buffer, RGP_BARRIER_UNKNOWN_REASON); - cmd_buffer->state.flush_bits |= RADV_CMD_FLAG_CS_PARTIAL_FLUSH | RADV_CMD_FLAG_INV_VCACHE; VkImageSubresourceRange range = { @@ -570,8 +568,6 @@ copy_image(struct radv_cmd_buffer *cmd_buffer, struct radv_image *src_image, uint32_t htile_value = radv_get_htile_initial_value(cmd_buffer->device, dst_image); cmd_buffer->state.flush_bits |= radv_clear_htile(cmd_buffer, dst_image, &range, htile_value); - - radv_describe_barrier_end(cmd_buffer); } } diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c index d2644a4..0e51ef6 100644 --- a/src/amd/vulkan/radv_cmd_buffer.c +++ b/src/amd/vulkan/radv_cmd_buffer.c @@ -7856,9 +7856,11 @@ radv_CmdBeginRendering(VkCommandBuffer commandBuffer, const VkRenderingInfo *pRe sample_locs_info->sampleLocationsCount); } - /* Implicit transitions only happens if traditional render passes are used. - * Some meta shaders invoked during barrier handling could call CmdBeginRendering, and we need to - * take care to not call radv_describe_barrier_start recursively. + /* Dynamic rendering does not have implicit transitions, so limit the marker to + * when a render pass is used. + * Additionally, some internal meta operations called inside a barrier may issue + * render calls (with dynamic rendering), so this makes sure those case don't + * create a nested barrier scope. */ if (cmd_buffer->vk.render_pass) radv_describe_barrier_start(cmd_buffer, RGP_BARRIER_EXTERNAL_RENDER_PASS_SYNC); diff --git a/src/amd/vulkan/radv_private.h b/src/amd/vulkan/radv_private.h index 17232f3..b2f3758 100644 --- a/src/amd/vulkan/radv_private.h +++ b/src/amd/vulkan/radv_private.h @@ -1677,6 +1677,7 @@ struct radv_cmd_state { uint32_t current_event_type; uint32_t num_events; uint32_t num_layout_transitions; + bool in_barrier; bool pending_sqtt_barrier_end; enum rgp_flush_bits sqtt_flush_bits; -- 2.7.4