Mesa (main): pvr: Add stricter type checking in pvr_csb_pack().
GitLab Mirror
gitlab-mirror at kemper.freedesktop.org
Mon Apr 4 03:47:21 UTC 2022
Module: Mesa
Branch: main
Commit: e5439bf4aa6ce69d323a0eb5108404d8e2a6ee18
URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=e5439bf4aa6ce69d323a0eb5108404d8e2a6ee18
Author: Karmjit Mahil <Karmjit.Mahil at imgtec.com>
Date: Mon Mar 14 11:46:54 2022 +0000
pvr: Add stricter type checking in pvr_csb_pack().
Since the packing functions generated by csbgen use a void pointer
for the buffer in which to pack, it's possible to easily write out
of bounds. This commits attempts to reduce the chances by
having the pack macro check that the pointer passed points to an
element sized equally to the state word being packed. Catching
these errors earlier.
As can be seen in this commit, there already was a case of this:
"pds_ctrl". The word size is meant to be 64 bits but the pointer
was pointing to a 32 bit field.
Although it's fine for the word size to be smaller than the
storage pointed to by the pointer, this is not allowed just to
be extra careful.
Signed-off-by: Karmjit Mahil <Karmjit.Mahil at imgtec.com>
Reviewed-by: Frank Binns <frank.binns at imgtec.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/15687>
---
src/imagination/vulkan/pvr_csb.h | 16 +++++-----
src/imagination/vulkan/pvr_job_context.c | 35 +++++++++++-----------
src/imagination/vulkan/winsys/pvr_winsys.h | 10 +++----
.../vulkan/winsys/pvrsrvkm/pvr_srv_job_render.c | 4 ++-
4 files changed, 35 insertions(+), 30 deletions(-)
diff --git a/src/imagination/vulkan/pvr_csb.h b/src/imagination/vulkan/pvr_csb.h
index 581595969a9..372c41ebd1d 100644
--- a/src/imagination/vulkan/pvr_csb.h
+++ b/src/imagination/vulkan/pvr_csb.h
@@ -38,6 +38,7 @@
#include "pvr_bo.h"
#include "pvr_winsys.h"
#include "util/list.h"
+#include "util/macros.h"
#define __pvr_address_type pvr_dev_addr_t
#define __pvr_get_address(pvr_dev_addr) (pvr_dev_addr).addr
@@ -137,13 +138,14 @@ VkResult pvr_csb_emit_terminate(struct pvr_csb *csb);
* This can be used by the caller to modify the command or
* state information before it's packed.
*/
-#define pvr_csb_pack(_dst, cmd, name) \
- for (struct pvr_cmd_struct(cmd) name = { pvr_cmd_header(cmd) }, \
- *_loop_terminate = &name; \
- __builtin_expect(_loop_terminate != NULL, 1); \
- ({ \
- pvr_cmd_pack(cmd)((_dst), &name); \
- _loop_terminate = NULL; \
+#define pvr_csb_pack(_dst, cmd, name) \
+ for (struct pvr_cmd_struct(cmd) name = { pvr_cmd_header(cmd) }, \
+ *_loop_terminate = &name; \
+ __builtin_expect(_loop_terminate != NULL, 1); \
+ ({ \
+ STATIC_ASSERT(sizeof(*(_dst)) == pvr_cmd_length(cmd) * 4); \
+ pvr_cmd_pack(cmd)((_dst), &name); \
+ _loop_terminate = NULL; \
}))
/**
diff --git a/src/imagination/vulkan/pvr_job_context.c b/src/imagination/vulkan/pvr_job_context.c
index 871a75b61fa..9fe128596c3 100644
--- a/src/imagination/vulkan/pvr_job_context.c
+++ b/src/imagination/vulkan/pvr_job_context.c
@@ -759,15 +759,16 @@ static void pvr_render_ctx_ws_static_state_init(
struct pvr_render_ctx *ctx,
struct pvr_winsys_render_ctx_static_state *static_state)
{
- void *dst;
+ uint64_t *q_dst;
+ uint32_t *d_dst;
- dst = &static_state->vdm_ctx_state_base_addr;
- pvr_csb_pack (dst, CR_VDM_CONTEXT_STATE_BASE, base) {
+ q_dst = &static_state->vdm_ctx_state_base_addr;
+ pvr_csb_pack (q_dst, CR_VDM_CONTEXT_STATE_BASE, base) {
base.addr = ctx->ctx_switch.vdm_state_bo->vma->dev_addr;
}
- dst = &static_state->geom_ctx_state_base_addr;
- pvr_csb_pack (dst, CR_TA_CONTEXT_STATE_BASE, base) {
+ q_dst = &static_state->geom_ctx_state_base_addr;
+ pvr_csb_pack (q_dst, CR_TA_CONTEXT_STATE_BASE, base) {
base.addr = ctx->ctx_switch.geom_state_bo->vma->dev_addr;
}
@@ -776,8 +777,8 @@ static void pvr_render_ctx_ws_static_state_init(
struct rogue_sr_programs *sr_prog = &ctx->ctx_switch.programs[i].sr;
/* Context store state. */
- dst = &static_state->geom_state[i].vdm_ctx_store_task0;
- pvr_csb_pack (dst, CR_VDM_CONTEXT_STORE_TASK0, task0) {
+ q_dst = &static_state->geom_state[i].vdm_ctx_store_task0;
+ pvr_csb_pack (q_dst, CR_VDM_CONTEXT_STORE_TASK0, task0) {
pvr_rogue_get_vdmctrl_pds_state_words(&sr_prog->pds.store_program,
PVRX(VDMCTRL_USC_TARGET_ANY),
sr_prog->usc.unified_size,
@@ -785,23 +786,23 @@ static void pvr_render_ctx_ws_static_state_init(
&task0.pds_state1);
}
- dst = &static_state->geom_state[i].vdm_ctx_store_task1;
- pvr_csb_pack (dst, CR_VDM_CONTEXT_STORE_TASK1, task1) {
+ d_dst = &static_state->geom_state[i].vdm_ctx_store_task1;
+ pvr_csb_pack (d_dst, CR_VDM_CONTEXT_STORE_TASK1, task1) {
pvr_csb_pack (&task1.pds_state2, VDMCTRL_PDS_STATE2, state) {
state.pds_code_addr.addr = sr_prog->pds.store_program.code_offset;
}
}
- dst = &static_state->geom_state[i].vdm_ctx_store_task2;
- pvr_csb_pack (dst, CR_VDM_CONTEXT_STORE_TASK2, task2) {
+ q_dst = &static_state->geom_state[i].vdm_ctx_store_task2;
+ pvr_csb_pack (q_dst, CR_VDM_CONTEXT_STORE_TASK2, task2) {
pvr_rogue_get_geom_state_stream_out_words(&pt_prog->pds_store_program,
&task2.stream_out1,
&task2.stream_out2);
}
/* Context resume state. */
- dst = &static_state->geom_state[i].vdm_ctx_resume_task0;
- pvr_csb_pack (dst, CR_VDM_CONTEXT_RESUME_TASK0, task0) {
+ q_dst = &static_state->geom_state[i].vdm_ctx_resume_task0;
+ pvr_csb_pack (q_dst, CR_VDM_CONTEXT_RESUME_TASK0, task0) {
pvr_rogue_get_vdmctrl_pds_state_words(&sr_prog->pds.load_program,
PVRX(VDMCTRL_USC_TARGET_ALL),
sr_prog->usc.unified_size,
@@ -809,15 +810,15 @@ static void pvr_render_ctx_ws_static_state_init(
&task0.pds_state1);
}
- dst = &static_state->geom_state[i].vdm_ctx_resume_task1;
- pvr_csb_pack (dst, CR_VDM_CONTEXT_RESUME_TASK1, task1) {
+ d_dst = &static_state->geom_state[i].vdm_ctx_resume_task1;
+ pvr_csb_pack (d_dst, CR_VDM_CONTEXT_RESUME_TASK1, task1) {
pvr_csb_pack (&task1.pds_state2, VDMCTRL_PDS_STATE2, state) {
state.pds_code_addr.addr = sr_prog->pds.load_program.code_offset;
}
}
- dst = &static_state->geom_state[i].vdm_ctx_resume_task2;
- pvr_csb_pack (dst, CR_VDM_CONTEXT_RESUME_TASK2, task2) {
+ q_dst = &static_state->geom_state[i].vdm_ctx_resume_task2;
+ pvr_csb_pack (q_dst, CR_VDM_CONTEXT_RESUME_TASK2, task2) {
pvr_rogue_get_geom_state_stream_out_words(&pt_prog->pds_resume_program,
&task2.stream_out1,
&task2.stream_out2);
diff --git a/src/imagination/vulkan/winsys/pvr_winsys.h b/src/imagination/vulkan/winsys/pvr_winsys.h
index a73ede5f897..a7d724f76c3 100644
--- a/src/imagination/vulkan/winsys/pvr_winsys.h
+++ b/src/imagination/vulkan/winsys/pvr_winsys.h
@@ -233,10 +233,10 @@ struct pvr_winsys_compute_ctx_create_info {
uint64_t cdm_ctx_store_pds0;
uint64_t cdm_ctx_store_pds0_b;
- uint64_t cdm_ctx_store_pds1;
+ uint32_t cdm_ctx_store_pds1;
uint64_t cdm_ctx_terminate_pds;
- uint64_t cdm_ctx_terminate_pds1;
+ uint32_t cdm_ctx_terminate_pds1;
uint64_t cdm_ctx_resume_pds0;
uint64_t cdm_ctx_resume_pds0_b;
@@ -262,7 +262,7 @@ struct pvr_winsys_compute_submit_info {
struct {
uint64_t tpu_border_colour_table;
uint64_t cdm_item;
- uint64_t compute_cluster;
+ uint32_t compute_cluster;
uint64_t cdm_ctrl_stream_base;
uint32_t tpu;
uint32_t cdm_resume_pds1;
@@ -309,7 +309,7 @@ struct pvr_winsys_render_submit_info {
struct pvr_winsys_geometry_state {
struct {
- uint32_t pds_ctrl;
+ uint64_t pds_ctrl;
uint32_t ppp_ctrl;
uint32_t te_psg;
uint32_t tpu;
@@ -336,7 +336,7 @@ struct pvr_winsys_render_submit_info {
uint64_t isp_stencil_load_store_base;
uint64_t isp_zload_store_base;
uint64_t isp_zlsctl;
- uint64_t isp_zls_pixels;
+ uint32_t isp_zls_pixels;
uint64_t pbe_word[PVR_MAX_COLOR_ATTACHMENTS]
[ROGUE_NUM_PBESTATE_REG_WORDS];
uint32_t pixel_phantom;
diff --git a/src/imagination/vulkan/winsys/pvrsrvkm/pvr_srv_job_render.c b/src/imagination/vulkan/winsys/pvrsrvkm/pvr_srv_job_render.c
index 5084cfb415c..2a40b62aef7 100644
--- a/src/imagination/vulkan/winsys/pvrsrvkm/pvr_srv_job_render.c
+++ b/src/imagination/vulkan/winsys/pvrsrvkm/pvr_srv_job_render.c
@@ -395,7 +395,9 @@ static void pvr_srv_geometry_cmd_init(
fw_regs->tpu = state->regs.tpu;
fw_regs->vdm_context_resume_task0_size =
state->regs.vdm_ctx_resume_task0_size;
- fw_regs->pds_ctrl = state->regs.pds_ctrl;
+
+ assert(state->regs.pds_ctrl >> 32U == 0U);
+ fw_regs->pds_ctrl = (uint32_t)state->regs.pds_ctrl;
if (state->flags & PVR_WINSYS_GEOM_FLAG_FIRST_GEOMETRY)
cmd->flags |= ROGUE_FWIF_TAFLAGS_FIRSTKICK;
More information about the mesa-commit
mailing list