From 1e65762031071b0cd0fd740d5a0c808a237d3896 Mon Sep 17 00:00:00 2001 From: Iago Toral Quiroga Date: Mon, 6 Feb 2023 13:16:12 +0100 Subject: [PATCH] v3dv: ensure at least V3D_CL_MAX_INSTR_SIZE bytes in last CL instruction MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The CLE parser in the sim will read this many bytes for each instruction in a CL, so we should ensure we have at least that many bytes available in the BO when reading the last instruction, otherwise we can trigger a GMP violation. It is not clear whether this behavior applies to real hardware too. cc: mesa-stable Reviewed-by: Alejandro Piñeiro Part-of: (cherry picked from commit c2601f06908434aaddd6b1db5e995bbda22d2a72) --- .pick_status.json | 2 +- src/broadcom/common/v3d_limits.h | 2 ++ src/broadcom/vulkan/v3dv_cl.c | 18 +++++++++++------- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index 52c85b8..283380c 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -832,7 +832,7 @@ "description": "v3dv: ensure at least V3D_CL_MAX_INSTR_SIZE bytes in last CL instruction", "nominated": true, "nomination_type": 0, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": null }, diff --git a/src/broadcom/common/v3d_limits.h b/src/broadcom/common/v3d_limits.h index 755aedd..492740c 100644 --- a/src/broadcom/common/v3d_limits.h +++ b/src/broadcom/common/v3d_limits.h @@ -24,6 +24,8 @@ #ifndef V3D_LIMITS_H #define V3D_LIMITS_H +#define V3D_CL_MAX_INSTR_SIZE 25 + /* Number of channels a QPU thread executes in parallel. Also known as * gl_SubGroupSizeARB. */ diff --git a/src/broadcom/vulkan/v3dv_cl.c b/src/broadcom/vulkan/v3dv_cl.c index 55cf122..d1181ba 100644 --- a/src/broadcom/vulkan/v3dv_cl.c +++ b/src/broadcom/vulkan/v3dv_cl.c @@ -114,14 +114,18 @@ v3dv_cl_ensure_space_with_branch(struct v3dv_cl *cl, uint32_t space) * end with a 'return from sub list' command. */ bool needs_return_from_sub_list = false; - if (cl->job->type == V3DV_JOB_TYPE_GPU_CL_SECONDARY) { - if (cl->size > 0) { + if (cl->job->type == V3DV_JOB_TYPE_GPU_CL_SECONDARY && cl->size > 0) needs_return_from_sub_list = true; - space += cl_packet_length(RETURN_FROM_SUB_LIST); - } - } else { - space += cl_packet_length(BRANCH); - } + + /* + * The CLE processor in the simulator tries to read V3D_CL_MAX_INSTR_SIZE + * bytes form the CL for each new instruction. If the last instruction in our + * CL is smaller than that, and there are not at least V3D_CL_MAX_INSTR_SIZE + * bytes until the end of the BO, it will read out of bounds and possibly + * cause a GMP violation interrupt to trigger. Ensure we always have at + * least that many bytes available to read with the last instruction. + */ + space += V3D_CL_MAX_INSTR_SIZE; if (v3dv_cl_offset(cl) + space <= cl->size) return; -- 2.7.4