diff options
author | Karmjit Mahil <Karmjit.Mahil@imgtec.com> | 2023-05-02 17:23:34 +0100 |
---|---|---|
committer | Marge Bot <emma+marge@anholt.net> | 2023-05-16 18:09:03 +0000 |
commit | b59eb30e8860a8bd40ba8747194ff91e27d1812c (patch) | |
tree | d4ac73dd582c11edf84c0717b7d999131c5ae201 | |
parent | 27d55436173802948eaffb0b53ce12aafd922492 (diff) | |
download | mesa-b59eb30e8860a8bd40ba8747194ff91e27d1812c.tar.gz |
pvr: Fix cs corruption in pvr_pack_clear_vdm_state()
VDMCTRL_INDEX_LIST3 is packed conditionally which can cause the
generation of a corrupted control stream as the function mandated
the provided buffer to be of a fixed size always including the
possibly unpacked word. This would leave a gap in the control
stream when the caller ends up copying the buffer into the control
stream.
Reported-by: James Glanville <james.glanville@imgtec.com>
Signed-off-by: Karmjit Mahil <Karmjit.Mahil@imgtec.com>
Reviewed-by: Frank Binns <frank.binns@imgtec.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/22918>
-rw-r--r-- | src/imagination/vulkan/pvr_blit.c | 9 | ||||
-rw-r--r-- | src/imagination/vulkan/pvr_clear.c | 57 | ||||
-rw-r--r-- | src/imagination/vulkan/pvr_clear.h | 46 | ||||
-rw-r--r-- | src/imagination/vulkan/pvr_cmd_buffer.c | 31 | ||||
-rw-r--r-- | src/imagination/vulkan/pvr_private.h | 4 |
5 files changed, 93 insertions, 54 deletions
diff --git a/src/imagination/vulkan/pvr_blit.c b/src/imagination/vulkan/pvr_blit.c index d0b4b9db215..5943045130e 100644 --- a/src/imagination/vulkan/pvr_blit.c +++ b/src/imagination/vulkan/pvr_blit.c @@ -1947,6 +1947,7 @@ static void pvr_clear_attachments(struct pvr_cmd_buffer *cmd_buffer, struct pvr_pds_upload pds_program_data_upload; const VkClearRect *clear_rect = &rects[j]; struct pvr_suballoc_bo *vertices_bo; + uint32_t vdm_cs_size_in_dw; uint32_t *vdm_cs_buffer; VkResult result; @@ -2025,8 +2026,12 @@ static void pvr_clear_attachments(struct pvr_cmd_buffer *cmd_buffer, pds_program_upload.data_offset = pds_program_data_upload.data_offset; pds_program_upload.data_size = pds_program_data_upload.data_size; - vdm_cs_buffer = pvr_csb_alloc_dwords(&sub_cmd->control_stream, - PVR_CLEAR_VDM_STATE_DWORD_COUNT); + vdm_cs_size_in_dw = + pvr_clear_vdm_state_get_size_in_dw(dev_info, + clear_rect->layerCount); + + vdm_cs_buffer = + pvr_csb_alloc_dwords(&sub_cmd->control_stream, vdm_cs_size_in_dw); if (!vdm_cs_buffer) { result = vk_error(cmd_buffer, VK_ERROR_OUT_OF_HOST_MEMORY); cmd_buffer->state.status = result; diff --git a/src/imagination/vulkan/pvr_clear.c b/src/imagination/vulkan/pvr_clear.c index cd4647457cc..cf2362a5fe8 100644 --- a/src/imagination/vulkan/pvr_clear.c +++ b/src/imagination/vulkan/pvr_clear.c @@ -485,10 +485,13 @@ VkResult pvr_device_init_graphics_static_clear_state(struct pvr_device *device) .height = rogue_get_param_vf_max_y(dev_info) } }; + const uint32_t vdm_state_size_in_dw = + pvr_clear_vdm_state_get_size_in_dw(dev_info, 1); struct pvr_device_static_clear_state *state = &device->static_clear_state; const uint32_t cache_line_size = rogue_get_slc_cache_line_size(dev_info); - struct util_dynarray passthrough_vert_shader; struct pvr_pds_vertex_shader_program pds_program; + struct util_dynarray passthrough_vert_shader; + uint32_t *state_buffer; VkResult result; if (PVR_HAS_FEATURE(dev_info, gs_rta_support)) { @@ -548,6 +551,15 @@ VkResult pvr_device_init_graphics_static_clear_state(struct pvr_device *device) assert(pds_program.code_size <= state->pds.code_size); + state_buffer = vk_alloc(&device->vk.alloc, + PVR_DW_TO_BYTES(vdm_state_size_in_dw * 2), + 8, + VK_SYSTEM_ALLOCATION_SCOPE_DEVICE); + if (state_buffer == NULL) { + result = vk_error(device, VK_ERROR_OUT_OF_HOST_MEMORY); + goto err_free_pds_program; + } + /* TODO: The difference between the large and normal words is only the last * word. The value is 3 or 4 depending on the amount of indices. Should we * dedup this? @@ -568,7 +580,9 @@ VkResult pvr_device_init_graphics_static_clear_state(struct pvr_device *device) 3, 4 * sizeof(uint32_t), 1, - state->vdm_words); + state_buffer); + state->vdm_words = state_buffer; + state_buffer += vdm_state_size_in_dw; pvr_pack_clear_vdm_state(&device->pdevice->dev_info, &state->pds, @@ -576,14 +590,19 @@ VkResult pvr_device_init_graphics_static_clear_state(struct pvr_device *device) 4, 4 * sizeof(uint32_t), 1, - state->large_clear_vdm_words); + state_buffer); + state->large_clear_vdm_words = state_buffer; result = pvr_device_init_clear_attachment_programs(device); if (result != VK_SUCCESS) - goto err_free_pds_program; + goto err_free_vdm_state; return VK_SUCCESS; +err_free_vdm_state: + /* Cast away the const :( */ + vk_free(&device->vk.alloc, (void *)state->vdm_words); + err_free_pds_program: pvr_bo_suballoc_free(state->pds.pvr_bo); @@ -605,6 +624,12 @@ void pvr_device_finish_graphics_static_clear_state(struct pvr_device *device) pvr_device_finish_clear_attachment_programs(device); + /* Don't free `large_clear_vdm_words` since it was allocated together with + * `vdm_words`. + */ + /* Cast away the const :( */ + vk_free(&device->vk.alloc, (void *)state->vdm_words); + pvr_bo_suballoc_free(state->pds.pvr_bo); pvr_bo_suballoc_free(state->vertices_bo); pvr_bo_suballoc_free(state->usc_vertex_shader_bo); @@ -834,14 +859,19 @@ VkResult pvr_pds_clear_rta_vertex_shader_program_create_and_upload_code( return VK_SUCCESS; } -void pvr_pack_clear_vdm_state( - const struct pvr_device_info *const dev_info, - const struct pvr_pds_upload *const program, - uint32_t temps, - uint32_t index_count, - uint32_t vs_output_size_in_bytes, - uint32_t layer_count, - uint32_t state_buffer[const static PVR_CLEAR_VDM_STATE_DWORD_COUNT]) +/** + * Pack VDM control stream words for clear. + * + * The size of the `state_buffer` provided is expected to point to a buffer of + * size equal to what is returned by `pvr_clear_vdm_state_get_size_in_dw()`. + */ +void pvr_pack_clear_vdm_state(const struct pvr_device_info *const dev_info, + const struct pvr_pds_upload *const program, + uint32_t temps, + uint32_t index_count, + uint32_t vs_output_size_in_bytes, + uint32_t layer_count, + uint32_t *const state_buffer) { const uint32_t vs_output_size = DIV_ROUND_UP(vs_output_size_in_bytes, @@ -925,5 +955,6 @@ void pvr_pack_clear_vdm_state( stream += pvr_cmd_length(VDMCTRL_INDEX_LIST3); } - assert((uint64_t)(stream - state_buffer) <= PVR_CLEAR_VDM_STATE_DWORD_COUNT); + assert((uint64_t)(stream - state_buffer) == + pvr_clear_vdm_state_get_size_in_dw(dev_info, layer_count)); } diff --git a/src/imagination/vulkan/pvr_clear.h b/src/imagination/vulkan/pvr_clear.h index dfcb9b5b397..9846569b312 100644 --- a/src/imagination/vulkan/pvr_clear.h +++ b/src/imagination/vulkan/pvr_clear.h @@ -29,21 +29,12 @@ #include <vulkan/vulkan_core.h> #include "pvr_csb.h" +#include "pvr_device_info.h" #include "util/macros.h" #define PVR_CLEAR_VERTEX_COUNT 4 #define PVR_CLEAR_VERTEX_COORDINATES 3 -/* We don't always need ROGUE_VDMCTRL_INDEX_LIST3 so maybe change the code to - * not have it in here but use an alternative definition when needed if we want - * to really squeeze out a some bytes of memory. - */ -#define PVR_CLEAR_VDM_STATE_DWORD_COUNT \ - (pvr_cmd_length(VDMCTRL_VDM_STATE0) + pvr_cmd_length(VDMCTRL_VDM_STATE2) + \ - pvr_cmd_length(VDMCTRL_VDM_STATE3) + pvr_cmd_length(VDMCTRL_VDM_STATE4) + \ - pvr_cmd_length(VDMCTRL_VDM_STATE5) + pvr_cmd_length(VDMCTRL_INDEX_LIST0) + \ - pvr_cmd_length(VDMCTRL_INDEX_LIST2) + pvr_cmd_length(VDMCTRL_INDEX_LIST3)) - #define PVR_STATIC_CLEAR_PDS_STATE_COUNT \ (pvr_cmd_length(TA_STATE_PDS_SHADERBASE) + \ pvr_cmd_length(TA_STATE_PDS_TEXUNICODEBASE) + \ @@ -80,7 +71,6 @@ static_assert(PVR_STATIC_CLEAR_PPP_PDS_TYPE_TEXTUREDATABASE + 1 == struct pvr_bo; struct pvr_cmd_buffer; struct pvr_device; -struct pvr_device_info; struct pvr_pds_upload; struct pvr_pds_vertex_shader_program; @@ -170,14 +160,32 @@ pvr_pds_clear_rta_vertex_shader_program_create_and_upload_data( pds_upload_out); } -void pvr_pack_clear_vdm_state( - const struct pvr_device_info *const dev_info, - const struct pvr_pds_upload *const program, - uint32_t temps, - uint32_t index_count, - uint32_t vs_output_size_in_bytes, - uint32_t layer_count, - uint32_t state_buffer[const static PVR_CLEAR_VDM_STATE_DWORD_COUNT]); +static inline uint32_t +pvr_clear_vdm_state_get_size_in_dw(const struct pvr_device_info *const dev_info, + uint32_t layer_count) +{ + uint32_t size_in_dw = + pvr_cmd_length(VDMCTRL_VDM_STATE0) + pvr_cmd_length(VDMCTRL_VDM_STATE2) + + pvr_cmd_length(VDMCTRL_VDM_STATE3) + pvr_cmd_length(VDMCTRL_VDM_STATE4) + + pvr_cmd_length(VDMCTRL_VDM_STATE5) + pvr_cmd_length(VDMCTRL_INDEX_LIST0) + + pvr_cmd_length(VDMCTRL_INDEX_LIST2); + + const bool needs_instance_count = + !PVR_HAS_FEATURE(dev_info, gs_rta_support) && layer_count > 1; + + if (needs_instance_count) + size_in_dw += pvr_cmd_length(VDMCTRL_INDEX_LIST3); + + return size_in_dw; +} + +void pvr_pack_clear_vdm_state(const struct pvr_device_info *const dev_info, + const struct pvr_pds_upload *const program, + uint32_t temps, + uint32_t index_count, + uint32_t vs_output_size_in_bytes, + uint32_t layer_count, + uint32_t *const state_buffer); VkResult pvr_clear_vertices_upload(struct pvr_device *device, const VkRect2D *rect, diff --git a/src/imagination/vulkan/pvr_cmd_buffer.c b/src/imagination/vulkan/pvr_cmd_buffer.c index 7823678d58e..36e3c710c26 100644 --- a/src/imagination/vulkan/pvr_cmd_buffer.c +++ b/src/imagination/vulkan/pvr_cmd_buffer.c @@ -2750,25 +2750,27 @@ pvr_is_large_clear_required(const struct pvr_cmd_buffer *const cmd_buffer) static void pvr_emit_clear_words(struct pvr_cmd_buffer *const cmd_buffer, struct pvr_sub_cmd_gfx *const sub_cmd) { - struct pvr_csb *csb = &sub_cmd->control_stream; struct pvr_device *device = cmd_buffer->device; + struct pvr_csb *csb = &sub_cmd->control_stream; + uint32_t vdm_state_size_in_dw; + const uint32_t *vdm_state; uint32_t *stream; - stream = pvr_csb_alloc_dwords(csb, PVR_CLEAR_VDM_STATE_DWORD_COUNT); + vdm_state_size_in_dw = + pvr_clear_vdm_state_get_size_in_dw(&device->pdevice->dev_info, 1); + + stream = pvr_csb_alloc_dwords(csb, vdm_state_size_in_dw); if (!stream) { cmd_buffer->state.status = VK_ERROR_OUT_OF_HOST_MEMORY; return; } - if (pvr_is_large_clear_required(cmd_buffer)) { - memcpy(stream, - device->static_clear_state.large_clear_vdm_words, - sizeof(device->static_clear_state.large_clear_vdm_words)); - } else { - memcpy(stream, - device->static_clear_state.vdm_words, - sizeof(device->static_clear_state.vdm_words)); - } + if (pvr_is_large_clear_required(cmd_buffer)) + vdm_state = device->static_clear_state.large_clear_vdm_words; + else + vdm_state = device->static_clear_state.vdm_words; + + memcpy(stream, vdm_state, PVR_DW_TO_BYTES(vdm_state_size_in_dw)); } static VkResult pvr_cs_write_load_op(struct pvr_cmd_buffer *cmd_buffer, @@ -7087,13 +7089,6 @@ static void pvr_insert_transparent_obj(struct pvr_cmd_buffer *const cmd_buffer, /* Emit VDM state. */ - static_assert(sizeof(device->static_clear_state.large_clear_vdm_words) >= - PVR_DW_TO_BYTES(PVR_CLEAR_VDM_STATE_DWORD_COUNT), - "Large clear VDM control stream word length mismatch"); - static_assert(sizeof(device->static_clear_state.vdm_words) == - PVR_DW_TO_BYTES(PVR_CLEAR_VDM_STATE_DWORD_COUNT), - "Clear VDM control stream word length mismatch"); - pvr_emit_clear_words(cmd_buffer, sub_cmd); /* Reset graphics state. */ diff --git a/src/imagination/vulkan/pvr_private.h b/src/imagination/vulkan/pvr_private.h index 1d89ee4b81e..07aadaf6b57 100644 --- a/src/imagination/vulkan/pvr_private.h +++ b/src/imagination/vulkan/pvr_private.h @@ -237,8 +237,8 @@ struct pvr_device { struct pvr_static_clear_ppp_template ppp_templates[PVR_STATIC_CLEAR_VARIANT_COUNT]; - uint32_t vdm_words[PVR_CLEAR_VDM_STATE_DWORD_COUNT]; - uint32_t large_clear_vdm_words[PVR_CLEAR_VDM_STATE_DWORD_COUNT]; + const uint32_t *vdm_words; + const uint32_t *large_clear_vdm_words; struct pvr_suballoc_bo *usc_clear_attachment_programs; struct pvr_suballoc_bo *pds_clear_attachment_programs; |