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>

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;

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));
 }

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,

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. */

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;