From 0ecc6eb2f5a006c12fcee797914c5c612a4298df Mon Sep 17 00:00:00 2001 From: Pierre-Eric Pelloux-Prayer Date: Wed, 6 May 2026 10:59:30 +0200 Subject: [PATCH] ac/sqtt: add ac_sqtt_update_bo_size buffer_size is uint32_t so we must be careful to not overflow it. radeonsi had code for this but radv doesn't, which means it will hang if RADV_THREAD_TRACE_BUFFER_SIZE is too big or if buffer_size is being doubled up to the point it overflows. Reviewed-by: Samuel Pitoiset Part-of: --- src/amd/common/ac_sqtt.c | 44 ++++++++++++++++++++++ src/amd/common/ac_sqtt.h | 2 + src/amd/vulkan/radv_sqtt.c | 14 +++---- src/gallium/drivers/radeonsi/gfx/si_sqtt.c | 22 ++--------- 4 files changed, 55 insertions(+), 27 deletions(-) diff --git a/src/amd/common/ac_sqtt.c b/src/amd/common/ac_sqtt.c index 6a9b55d05d1..9f4d2414a8d 100644 --- a/src/amd/common/ac_sqtt.c +++ b/src/amd/common/ac_sqtt.c @@ -700,3 +700,47 @@ ac_sqtt_emit_wait(const struct radeon_info *info, struct ac_pm4_state *pm4, ac_pm4_set_reg(pm4, R_030800_GRBM_GFX_INDEX, S_030800_SE_BROADCAST_WRITES(1) | S_030800_SH_BROADCAST_WRITES(1) | S_030800_INSTANCE_BROADCAST_WRITES(1)); } + +bool ac_sqtt_update_bo_size(struct ac_sqtt *sqtt, const char *env_var_prefix) +{ + if (strlen(env_var_prefix) > 4) + return false; + + if (sqtt->buffer_size == 0) { + char envvar[sizeof("xxxx_THREAD_TRACE_BUFFER_SIZE")]; + + sprintf(envvar, "%s_THREAD_TRACE_BUFFER_SIZE", env_var_prefix); + + /* Default buffer size set to 32MB per SE. */ + uint64_t s = debug_get_num_option(envvar, 32 * 1024 * 1024); + sqtt->buffer_size = s; + if ((uint64_t)sqtt->buffer_size != s) { + fprintf(stderr, + "Invalid %s value (must be <= %u).\n", + envvar, UINT32_MAX); + return false; + } + + return true; + } + if (sqtt->buffer_size < UINT32_MAX / 2) { + /* Double the size of the thread trace buffer per SE. */ + sqtt->buffer_size *= 2; + fprintf(stderr, + "Failed to get the thread trace because the buffer " + "was too small, resizing to %u kB per se\n", + sqtt->buffer_size / 1024); + } else { + fprintf(stderr, + "Failed to get the thread trace because the buffer " + "was too small (%u kB per se). Cancelling trace capture.\n", + sqtt->buffer_size / 1024); + if (sqtt->instruction_timing_enabled) + fprintf(stderr, + "Try again with %s_THREAD_TRACE_INSTRUCTION_TIMING=false" + " to reduce the size of the captured data.\n", + env_var_prefix); + return false; + } + return true; +} \ No newline at end of file diff --git a/src/amd/common/ac_sqtt.h b/src/amd/common/ac_sqtt.h index 7148ef13ab2..dffcf39460e 100644 --- a/src/amd/common/ac_sqtt.h +++ b/src/amd/common/ac_sqtt.h @@ -577,4 +577,6 @@ void ac_sqtt_emit_stop(const struct radeon_info *info, struct ac_pm4_state *pm4, void ac_sqtt_emit_wait(const struct radeon_info *info, struct ac_pm4_state *pm4, const struct ac_sqtt *sqtt, bool is_compute_queue); +bool ac_sqtt_update_bo_size(struct ac_sqtt *sqtt, const char *env_var_prefix); + #endif diff --git a/src/amd/vulkan/radv_sqtt.c b/src/amd/vulkan/radv_sqtt.c index 21be42726c4..16961ced7cd 100644 --- a/src/amd/vulkan/radv_sqtt.c +++ b/src/amd/vulkan/radv_sqtt.c @@ -506,8 +506,9 @@ radv_sqtt_init(struct radv_device *device) const struct radv_physical_device *pdev = radv_device_physical(device); struct ac_sqtt *sqtt = &device->sqtt; - /* Default buffer size set to 32MB per SE. */ - device->sqtt.buffer_size = (uint32_t)debug_get_num_option("RADV_THREAD_TRACE_BUFFER_SIZE", 32 * 1024 * 1024); + if (!ac_sqtt_update_bo_size(&device->sqtt, "RADV")) + return false; + device->sqtt.instruction_timing_enabled = radv_is_instruction_timing_enabled(); /* Whether to use a staging buffer for faster reads on dGPUs. */ @@ -556,13 +557,8 @@ radv_sqtt_resize_bo(struct radv_device *device) /* Destroy the previous thread trace BO. */ radv_sqtt_finish_bo(device); - /* Double the size of the thread trace buffer per SE. */ - device->sqtt.buffer_size *= 2; - - fprintf(stderr, - "Failed to get the thread trace because the buffer " - "was too small, resizing to %d KB\n", - device->sqtt.buffer_size / 1024); + if (!ac_sqtt_update_bo_size(&device->sqtt, "RADV")) + return false; /* Re-create the thread trace BO. */ return radv_sqtt_init_bo(device); diff --git a/src/gallium/drivers/radeonsi/gfx/si_sqtt.c b/src/gallium/drivers/radeonsi/gfx/si_sqtt.c index a1de2151d71..a6b20fe3d1e 100644 --- a/src/gallium/drivers/radeonsi/gfx/si_sqtt.c +++ b/src/gallium/drivers/radeonsi/gfx/si_sqtt.c @@ -260,21 +260,8 @@ si_sqtt_resize_bo(struct si_context *sctx) pipe_resource_reference(&bo, NULL); sctx->sqtt->bo = NULL; - if (sctx->sqtt->buffer_size < UINT32_MAX / 2) { - /* Double the size of the thread trace buffer per SE. */ - sctx->sqtt->buffer_size *= 2; - mesa_loge("Failed to get the thread trace because the buffer " - "was too small, resizing to %d kB per se", - sctx->sqtt->buffer_size / 1024); - } else { - mesa_loge("Failed to get the thread trace because the buffer " - "was too small (%d kB per se). Cancelling trace capture.", - sctx->sqtt->buffer_size / 1024); - if (sctx->sqtt->instruction_timing_enabled) - mesa_loge("Try again with AMD_THREAD_TRACE_INSTRUCTION_TIMING=false" - " to reduce the size of the captured data."); + if (!ac_sqtt_update_bo_size(sctx->sqtt, "AMD")) return false; - } /* Re-create the thread trace BO. */ return si_sqtt_init_bo(sctx); @@ -335,10 +322,9 @@ bool si_init_sqtt(struct si_context *sctx) return false; } - /* Default buffer size set to 32MB per SE. */ - sctx->sqtt->buffer_size = - debug_get_num_option("AMD_THREAD_TRACE_BUFFER_SIZE", 32 * 1024) * 1024; - assert(sctx->sqtt->buffer_size); + if (!ac_sqtt_update_bo_size(sctx->sqtt, "AMD")) + return false; + sctx->sqtt->instruction_timing_enabled = debug_get_bool_option("AMD_THREAD_TRACE_INSTRUCTION_TIMING", true); sctx->sqtt->start_frame = 10;