From 19907a24eca75f4deff536ae5cd27c2cfe3dd146 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Ol=C5=A1=C3=A1k?= Date: Wed, 15 Jan 2025 22:16:26 -0500 Subject: [PATCH] radeonsi: validate BITSET_TEST_RANGE_INSIDE_WORD assertion at compile time This will prevent accidental crashes and hangs because of how we define tracked enums. The reg_enum parameter must be a compile-time constant. Reviewed-by: Pierre-Eric Pelloux-Prayer Part-of: --- src/gallium/drivers/radeonsi/si_build_pm4.h | 14 ++++++++++++++ src/gallium/drivers/radeonsi/si_state_draw.cpp | 12 ++++-------- src/gallium/drivers/radeonsi/si_state_shaders.cpp | 11 ++++++----- 3 files changed, 24 insertions(+), 13 deletions(-) diff --git a/src/gallium/drivers/radeonsi/si_build_pm4.h b/src/gallium/drivers/radeonsi/si_build_pm4.h index c792d0fd2f8..6d2c9e5f87f 100644 --- a/src/gallium/drivers/radeonsi/si_build_pm4.h +++ b/src/gallium/drivers/radeonsi/si_build_pm4.h @@ -80,6 +80,8 @@ /* Set consecutive registers if any value is different. */ #define radeon_opt_set_reg2(reg, reg_enum, v1, v2, prefix_name, packet) do { \ + static_assert(BITSET_BITWORD(reg_enum) == BITSET_BITWORD(reg_enum + 1), \ + "bit range crosses dword boundary"); \ unsigned __v1 = (v1), __v2 = (v2); \ if (!BITSET_TEST_RANGE_INSIDE_WORD(sctx->tracked_regs.reg_saved_mask, \ (reg_enum), (reg_enum) + 1, 0x3) || \ @@ -96,6 +98,8 @@ } while (0) #define radeon_opt_set_reg3(reg, reg_enum, v1, v2, v3, prefix_name, packet) do { \ + static_assert(BITSET_BITWORD(reg_enum) == BITSET_BITWORD(reg_enum + 2), \ + "bit range crosses dword boundary"); \ unsigned __v1 = (v1), __v2 = (v2), __v3 = (v3); \ if (!BITSET_TEST_RANGE_INSIDE_WORD(sctx->tracked_regs.reg_saved_mask, \ (reg_enum), (reg_enum) + 2, 0x7) || \ @@ -115,6 +119,8 @@ } while (0) #define radeon_opt_set_reg4(reg, reg_enum, v1, v2, v3, v4, prefix_name, packet) do { \ + static_assert(BITSET_BITWORD((reg_enum)) == BITSET_BITWORD((reg_enum) + 3), \ + "bit range crosses dword boundary"); \ unsigned __v1 = (v1), __v2 = (v2), __v3 = (v3), __v4 = (v4); \ if (!BITSET_TEST_RANGE_INSIDE_WORD(sctx->tracked_regs.reg_saved_mask, \ (reg_enum), (reg_enum) + 3, 0xf) || \ @@ -137,6 +143,8 @@ } while (0) #define radeon_opt_set_reg5(reg, reg_enum, v1, v2, v3, v4, v5, prefix_name, packet) do { \ + static_assert(BITSET_BITWORD((reg_enum)) == BITSET_BITWORD((reg_enum) + 4), \ + "bit range crosses dword boundary"); \ unsigned __v1 = (v1), __v2 = (v2), __v3 = (v3), __v4 = (v4), __v5 = (v5); \ if (!BITSET_TEST_RANGE_INSIDE_WORD(sctx->tracked_regs.reg_saved_mask, \ (reg_enum), (reg_enum) + 4, 0x1f) || \ @@ -162,6 +170,8 @@ } while (0) #define radeon_opt_set_reg6(reg, reg_enum, v1, v2, v3, v4, v5, v6, prefix_name, packet) do { \ + static_assert(BITSET_BITWORD((reg_enum)) == BITSET_BITWORD((reg_enum) + 5), \ + "bit range crosses dword boundary"); \ unsigned __v1 = (v1), __v2 = (v2), __v3 = (v3), __v4 = (v4), __v5 = (v5), __v6 = (v6); \ if (!BITSET_TEST_RANGE_INSIDE_WORD(sctx->tracked_regs.reg_saved_mask, \ (reg_enum), (reg_enum) + 5, 0x3f) || \ @@ -319,6 +329,8 @@ } while (0) #define gfx11_opt_push_reg4(reg, reg_enum, v1, v2, v3, v4, prefix_name, buffer, reg_count) do { \ + static_assert(BITSET_BITWORD((reg_enum)) == BITSET_BITWORD((reg_enum) + 3), \ + "bit range crosses dword boundary"); \ unsigned __v1 = (v1); \ unsigned __v2 = (v2); \ unsigned __v3 = (v3); \ @@ -415,6 +427,8 @@ } while (0) #define gfx12_opt_set_reg4(reg, reg_enum, v1, v2, v3, v4, base_offset) do { \ + static_assert(BITSET_BITWORD((reg_enum)) == BITSET_BITWORD((reg_enum) + 3), \ + "bit range crosses dword boundary"); \ unsigned __v1 = (v1), __v2 = (v2), __v3 = (v3), __v4 = (v4); \ if (!BITSET_TEST_RANGE_INSIDE_WORD(sctx->tracked_regs.reg_saved_mask, \ (reg_enum), (reg_enum) + 3, 0xf) || \ diff --git a/src/gallium/drivers/radeonsi/si_state_draw.cpp b/src/gallium/drivers/radeonsi/si_state_draw.cpp index 9c65302a864..356f604094f 100644 --- a/src/gallium/drivers/radeonsi/si_state_draw.cpp +++ b/src/gallium/drivers/radeonsi/si_state_draw.cpp @@ -1367,14 +1367,10 @@ static void si_emit_draw_packets(struct si_context *sctx, const struct pipe_draw unsigned sh_base_reg = si_get_user_data_base(GFX_VERSION, HAS_TESS, HAS_GS, NGG, PIPE_SHADER_VERTEX); bool render_cond_bit = sctx->render_cond_enabled; - unsigned tracked_base_vertex_reg; - - if (HAS_TESS) - tracked_base_vertex_reg = SI_TRACKED_SPI_SHADER_USER_DATA_LS__BASE_VERTEX; - else if (HAS_GS || NGG) - tracked_base_vertex_reg = SI_TRACKED_SPI_SHADER_USER_DATA_ES__BASE_VERTEX; - else - tracked_base_vertex_reg = SI_TRACKED_SPI_SHADER_USER_DATA_VS__BASE_VERTEX; + const unsigned tracked_base_vertex_reg = + HAS_TESS ? SI_TRACKED_SPI_SHADER_USER_DATA_LS__BASE_VERTEX : + HAS_GS || NGG ? SI_TRACKED_SPI_SHADER_USER_DATA_ES__BASE_VERTEX : + SI_TRACKED_SPI_SHADER_USER_DATA_VS__BASE_VERTEX; if (!IS_DRAW_VERTEX_STATE && indirect) { assert(num_draws == 1); diff --git a/src/gallium/drivers/radeonsi/si_state_shaders.cpp b/src/gallium/drivers/radeonsi/si_state_shaders.cpp index ab97ad26a4c..4b96ce375c6 100644 --- a/src/gallium/drivers/radeonsi/si_state_shaders.cpp +++ b/src/gallium/drivers/radeonsi/si_state_shaders.cpp @@ -4947,12 +4947,13 @@ static void gfx6_emit_tess_io_layout_state(struct si_context *sctx, unsigned ind gfx11_opt_push_gfx_sh_reg(tes_sh_base + SI_SGPR_TES_OFFCHIP_ADDR * 4, SI_TRACKED_SPI_SHADER_USER_DATA_ES__DRAWID, sctx->tes_offchip_ring_va_sgpr); - } else { - bool has_gs = sctx->ngg || sctx->shader.gs.cso; - + } else if (sctx->ngg || sctx->shader.gs.cso) { radeon_opt_set_sh_reg2(tes_sh_base + SI_SGPR_TES_OFFCHIP_LAYOUT * 4, - has_gs ? SI_TRACKED_SPI_SHADER_USER_DATA_ES__BASE_VERTEX - : SI_TRACKED_SPI_SHADER_USER_DATA_VS__BASE_VERTEX, + SI_TRACKED_SPI_SHADER_USER_DATA_ES__BASE_VERTEX, + sctx->tcs_offchip_layout, sctx->tes_offchip_ring_va_sgpr); + } else { + radeon_opt_set_sh_reg2(tes_sh_base + SI_SGPR_TES_OFFCHIP_LAYOUT * 4, + SI_TRACKED_SPI_SHADER_USER_DATA_VS__BASE_VERTEX, sctx->tcs_offchip_layout, sctx->tes_offchip_ring_va_sgpr); } radeon_end();