From 0afe83078d10e0d376f7c3e2515ab2682fec0eb1 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Sat, 5 Dec 2015 12:25:25 -0800 Subject: [PATCH] vc4: Bring over cleanups from submitting to the kernel. --- .../drivers/vc4/kernel/vc4_render_cl.c | 5 +- src/gallium/drivers/vc4/kernel/vc4_validate.c | 87 +++++++++---------- .../drivers/vc4/kernel/vc4_validate_shaders.c | 71 +++++++-------- .../drivers/vc4/vc4_simulator_validate.h | 2 +- 4 files changed, 78 insertions(+), 87 deletions(-) diff --git a/src/gallium/drivers/vc4/kernel/vc4_render_cl.c b/src/gallium/drivers/vc4/kernel/vc4_render_cl.c index b827eb7e9e1..31784b79771 100644 --- a/src/gallium/drivers/vc4/kernel/vc4_render_cl.c +++ b/src/gallium/drivers/vc4/kernel/vc4_render_cl.c @@ -62,7 +62,6 @@ static inline void rcl_u32(struct vc4_rcl_setup *setup, u32 val) setup->next_offset += 4; } - /* * Emits a no-op STORE_TILE_BUFFER_GENERAL. * @@ -255,6 +254,7 @@ static int vc4_create_rcl_bo(struct drm_device *dev, struct vc4_exec_info *exec, for (x = min_x_tile; x <= max_x_tile; x++) { bool first = (x == min_x_tile && y == min_y_tile); bool last = (x == max_x_tile && y == max_y_tile); + emit_tile(exec, setup, x, y, first, last); } } @@ -414,7 +414,8 @@ int vc4_get_rcl(struct drm_device *dev, struct vc4_exec_info *exec) if (has_bin && (args->max_x_tile > exec->bin_tiles_x || args->max_y_tile > exec->bin_tiles_y)) { - DRM_ERROR("Render tiles (%d,%d) outside of bin config (%d,%d)\n", + DRM_ERROR("Render tiles (%d,%d) outside of bin config " + "(%d,%d)\n", args->max_x_tile, args->max_y_tile, exec->bin_tiles_x, exec->bin_tiles_y); return -EINVAL; diff --git a/src/gallium/drivers/vc4/kernel/vc4_validate.c b/src/gallium/drivers/vc4/kernel/vc4_validate.c index b248831113c..a50e9c38261 100644 --- a/src/gallium/drivers/vc4/kernel/vc4_validate.c +++ b/src/gallium/drivers/vc4/kernel/vc4_validate.c @@ -47,7 +47,6 @@ void *validated, \ void *untrusted - /** Return the width in pixels of a 64-byte microtile. */ static uint32_t utile_width(int cpp) @@ -191,7 +190,7 @@ vc4_check_tex_size(struct vc4_exec_info *exec, struct drm_gem_cma_object *fbo, if (size + offset < size || size + offset > fbo->base.size) { - DRM_ERROR("Overflow in %dx%d (%dx%d) fbo size (%d + %d > %d)\n", + DRM_ERROR("Overflow in %dx%d (%dx%d) fbo size (%d + %d > %zd)\n", width, height, aligned_width, aligned_height, size, offset, fbo->base.size); @@ -201,7 +200,6 @@ vc4_check_tex_size(struct vc4_exec_info *exec, struct drm_gem_cma_object *fbo, return true; } - static int validate_flush(VALIDATE_ARGS) { @@ -270,7 +268,7 @@ validate_indexed_prim_list(VALIDATE_ARGS) if (offset > ib->base.size || (ib->base.size - offset) / index_size < length) { - DRM_ERROR("IB access overflow (%d + %d*%d > %d)\n", + DRM_ERROR("IB access overflow (%d + %d*%d > %zd)\n", offset, length, index_size, ib->base.size); return -EINVAL; } @@ -424,8 +422,8 @@ validate_gem_handles(VALIDATE_ARGS) return 0; } -#define VC4_DEFINE_PACKET(packet, name, func) \ - [packet] = { packet ## _SIZE, name, func } +#define VC4_DEFINE_PACKET(packet, func) \ + [packet] = { packet ## _SIZE, #packet, func } static const struct cmd_info { uint16_t len; @@ -433,42 +431,42 @@ static const struct cmd_info { int (*func)(struct vc4_exec_info *exec, void *validated, void *untrusted); } cmd_info[] = { - VC4_DEFINE_PACKET(VC4_PACKET_HALT, "halt", NULL), - VC4_DEFINE_PACKET(VC4_PACKET_NOP, "nop", NULL), - VC4_DEFINE_PACKET(VC4_PACKET_FLUSH, "flush", validate_flush), - VC4_DEFINE_PACKET(VC4_PACKET_FLUSH_ALL, "flush all state", NULL), - VC4_DEFINE_PACKET(VC4_PACKET_START_TILE_BINNING, "start tile binning", validate_start_tile_binning), - VC4_DEFINE_PACKET(VC4_PACKET_INCREMENT_SEMAPHORE, "increment semaphore", validate_increment_semaphore), - - VC4_DEFINE_PACKET(VC4_PACKET_GL_INDEXED_PRIMITIVE, "Indexed Primitive List", validate_indexed_prim_list), - - VC4_DEFINE_PACKET(VC4_PACKET_GL_ARRAY_PRIMITIVE, "Vertex Array Primitives", validate_gl_array_primitive), - - /* This is only used by clipped primitives (packets 48 and 49), which - * we don't support parsing yet. - */ - VC4_DEFINE_PACKET(VC4_PACKET_PRIMITIVE_LIST_FORMAT, "primitive list format", NULL), - - VC4_DEFINE_PACKET(VC4_PACKET_GL_SHADER_STATE, "GL Shader State", validate_gl_shader_state), - /* We don't support validating NV shader states. */ - - VC4_DEFINE_PACKET(VC4_PACKET_CONFIGURATION_BITS, "configuration bits", NULL), - VC4_DEFINE_PACKET(VC4_PACKET_FLAT_SHADE_FLAGS, "flat shade flags", NULL), - VC4_DEFINE_PACKET(VC4_PACKET_POINT_SIZE, "point size", NULL), - VC4_DEFINE_PACKET(VC4_PACKET_LINE_WIDTH, "line width", NULL), - VC4_DEFINE_PACKET(VC4_PACKET_RHT_X_BOUNDARY, "RHT X boundary", NULL), - VC4_DEFINE_PACKET(VC4_PACKET_DEPTH_OFFSET, "Depth Offset", NULL), - VC4_DEFINE_PACKET(VC4_PACKET_CLIP_WINDOW, "Clip Window", NULL), - VC4_DEFINE_PACKET(VC4_PACKET_VIEWPORT_OFFSET, "Viewport Offset", NULL), - VC4_DEFINE_PACKET(VC4_PACKET_CLIPPER_XY_SCALING, "Clipper XY Scaling", NULL), + VC4_DEFINE_PACKET(VC4_PACKET_HALT, NULL), + VC4_DEFINE_PACKET(VC4_PACKET_NOP, NULL), + VC4_DEFINE_PACKET(VC4_PACKET_FLUSH, validate_flush), + VC4_DEFINE_PACKET(VC4_PACKET_FLUSH_ALL, NULL), + VC4_DEFINE_PACKET(VC4_PACKET_START_TILE_BINNING, + validate_start_tile_binning), + VC4_DEFINE_PACKET(VC4_PACKET_INCREMENT_SEMAPHORE, + validate_increment_semaphore), + + VC4_DEFINE_PACKET(VC4_PACKET_GL_INDEXED_PRIMITIVE, + validate_indexed_prim_list), + VC4_DEFINE_PACKET(VC4_PACKET_GL_ARRAY_PRIMITIVE, + validate_gl_array_primitive), + + VC4_DEFINE_PACKET(VC4_PACKET_PRIMITIVE_LIST_FORMAT, NULL), + + VC4_DEFINE_PACKET(VC4_PACKET_GL_SHADER_STATE, validate_gl_shader_state), + + VC4_DEFINE_PACKET(VC4_PACKET_CONFIGURATION_BITS, NULL), + VC4_DEFINE_PACKET(VC4_PACKET_FLAT_SHADE_FLAGS, NULL), + VC4_DEFINE_PACKET(VC4_PACKET_POINT_SIZE, NULL), + VC4_DEFINE_PACKET(VC4_PACKET_LINE_WIDTH, NULL), + VC4_DEFINE_PACKET(VC4_PACKET_RHT_X_BOUNDARY, NULL), + VC4_DEFINE_PACKET(VC4_PACKET_DEPTH_OFFSET, NULL), + VC4_DEFINE_PACKET(VC4_PACKET_CLIP_WINDOW, NULL), + VC4_DEFINE_PACKET(VC4_PACKET_VIEWPORT_OFFSET, NULL), + VC4_DEFINE_PACKET(VC4_PACKET_CLIPPER_XY_SCALING, NULL), /* Note: The docs say this was also 105, but it was 106 in the * initial userland code drop. */ - VC4_DEFINE_PACKET(VC4_PACKET_CLIPPER_Z_SCALING, "Clipper Z Scale and Offset", NULL), + VC4_DEFINE_PACKET(VC4_PACKET_CLIPPER_Z_SCALING, NULL), - VC4_DEFINE_PACKET(VC4_PACKET_TILE_BINNING_MODE_CONFIG, "tile binning configuration", validate_tile_binning_config), + VC4_DEFINE_PACKET(VC4_PACKET_TILE_BINNING_MODE_CONFIG, + validate_tile_binning_config), - VC4_DEFINE_PACKET(VC4_PACKET_GEM_HANDLES, "GEM handles", validate_gem_handles), + VC4_DEFINE_PACKET(VC4_PACKET_GEM_HANDLES, validate_gem_handles), }; int @@ -500,11 +498,6 @@ vc4_validate_bin_cl(struct drm_device *dev, return -EINVAL; } -#if 0 - DRM_INFO("0x%08x: packet %d (%s) size %d processing...\n", - src_offset, cmd, info->name, info->len); -#endif - if (src_offset + info->len > len) { DRM_ERROR("0x%08x: packet %d (%s) length 0x%08x " "exceeds bounds (0x%08x)\n", @@ -519,8 +512,7 @@ vc4_validate_bin_cl(struct drm_device *dev, if (info->func && info->func(exec, dst_pkt + 1, src_pkt + 1)) { - DRM_ERROR("0x%08x: packet %d (%s) failed to " - "validate\n", + DRM_ERROR("0x%08x: packet %d (%s) failed to validate\n", src_offset, cmd, info->name); return -EINVAL; } @@ -588,12 +580,14 @@ reloc_tex(struct vc4_exec_info *exec, if (sample->is_direct) { uint32_t remaining_size = tex->base.size - p0; + if (p0 > tex->base.size - 4) { DRM_ERROR("UBO offset greater than UBO size\n"); goto fail; } if (p1 > remaining_size - 4) { - DRM_ERROR("UBO clamp would allow reads outside of UBO\n"); + DRM_ERROR("UBO clamp would allow reads " + "outside of UBO\n"); goto fail; } *validated_p0 = tex->paddr + p0; @@ -875,7 +869,8 @@ validate_gl_shader_rec(struct drm_device *dev, max_index = ((vbo->base.size - offset - attr_size) / stride); if (state->max_index > max_index) { - DRM_ERROR("primitives use index %d out of supplied %d\n", + DRM_ERROR("primitives use index %d out of " + "supplied %d\n", state->max_index, max_index); return -EINVAL; } diff --git a/src/gallium/drivers/vc4/kernel/vc4_validate_shaders.c b/src/gallium/drivers/vc4/kernel/vc4_validate_shaders.c index ab9a6512e82..868a0ad1a3a 100644 --- a/src/gallium/drivers/vc4/kernel/vc4_validate_shaders.c +++ b/src/gallium/drivers/vc4/kernel/vc4_validate_shaders.c @@ -24,24 +24,16 @@ /** * DOC: Shader validator for VC4. * - * The VC4 has no IOMMU between it and system memory. So, a user with access - * to execute shaders could escalate privilege by overwriting system memory - * (using the VPM write address register in the general-purpose DMA mode) or - * reading system memory it shouldn't (reading it as a texture, or uniform - * data, or vertex data). + * The VC4 has no IOMMU between it and system memory, so a user with + * access to execute shaders could escalate privilege by overwriting + * system memory (using the VPM write address register in the + * general-purpose DMA mode) or reading system memory it shouldn't + * (reading it as a texture, or uniform data, or vertex data). * - * This walks over a shader starting from some offset within a BO, ensuring - * that its accesses are appropriately bounded, and recording how many texture - * accesses are made and where so that we can do relocations for them in the + * This walks over a shader BO, ensuring that its accesses are + * appropriately bounded, and recording how many texture accesses are + * made and where so that we can do relocations for them in the * uniform stream. - * - * The kernel API has shaders stored in user-mapped BOs. The BOs will be - * forcibly unmapped from the process before validation, and any cache of - * validated state will be flushed if the mapping is faulted back in. - * - * Storing the shaders in BOs means that the validation process will be slow - * due to uncached reads, but since shaders are long-lived and shader BOs are - * never actually modified, this shouldn't be a problem. */ #include "vc4_drv.h" @@ -71,7 +63,6 @@ waddr_to_live_reg_index(uint32_t waddr, bool is_b) else return waddr; } else if (waddr <= QPU_W_ACC3) { - return 64 + waddr - QPU_W_ACC0; } else { return ~0; @@ -86,15 +77,14 @@ raddr_add_a_to_live_reg_index(uint64_t inst) uint32_t raddr_a = QPU_GET_FIELD(inst, QPU_RADDR_A); uint32_t raddr_b = QPU_GET_FIELD(inst, QPU_RADDR_B); - if (add_a == QPU_MUX_A) { + if (add_a == QPU_MUX_A) return raddr_a; - } else if (add_a == QPU_MUX_B && sig != QPU_SIG_SMALL_IMM) { + else if (add_a == QPU_MUX_B && sig != QPU_SIG_SMALL_IMM) return 32 + raddr_b; - } else if (add_a <= QPU_MUX_R3) { + else if (add_a <= QPU_MUX_R3) return 64 + add_a; - } else { + else return ~0; - } } static bool @@ -112,9 +102,9 @@ is_tmu_write(uint32_t waddr) } static bool -record_validated_texture_sample(struct vc4_validated_shader_info *validated_shader, - struct vc4_shader_validation_state *validation_state, - int tmu) +record_texture_sample(struct vc4_validated_shader_info *validated_shader, + struct vc4_shader_validation_state *validation_state, + int tmu) { uint32_t s = validated_shader->num_texture_samples; int i; @@ -227,8 +217,8 @@ check_tmu_write(uint64_t inst, validated_shader->uniforms_size += 4; if (submit) { - if (!record_validated_texture_sample(validated_shader, - validation_state, tmu)) { + if (!record_texture_sample(validated_shader, + validation_state, tmu)) { return false; } @@ -239,10 +229,10 @@ check_tmu_write(uint64_t inst, } static bool -check_register_write(uint64_t inst, - struct vc4_validated_shader_info *validated_shader, - struct vc4_shader_validation_state *validation_state, - bool is_mul) +check_reg_write(uint64_t inst, + struct vc4_validated_shader_info *validated_shader, + struct vc4_shader_validation_state *validation_state, + bool is_mul) { uint32_t waddr = (is_mul ? QPU_GET_FIELD(inst, QPU_WADDR_MUL) : @@ -298,7 +288,7 @@ check_register_write(uint64_t inst, return true; case QPU_W_TLB_STENCIL_SETUP: - return true; + return true; } return true; @@ -361,7 +351,7 @@ track_live_clamps(uint64_t inst, } validation_state->live_max_clamp_regs[lri_add] = true; - } if (op_add == QPU_A_MIN) { + } else if (op_add == QPU_A_MIN) { /* Track live clamps of a value clamped to a minimum of 0 and * a maximum of some uniform's offset. */ @@ -393,8 +383,10 @@ check_instruction_writes(uint64_t inst, return false; } - ok = (check_register_write(inst, validated_shader, validation_state, false) && - check_register_write(inst, validated_shader, validation_state, true)); + ok = (check_reg_write(inst, validated_shader, validation_state, + false) && + check_reg_write(inst, validated_shader, validation_state, + true)); track_live_clamps(inst, validated_shader, validation_state); @@ -442,7 +434,7 @@ vc4_validate_shader(struct drm_gem_cma_object *shader_obj) shader = shader_obj->vaddr; max_ip = shader_obj->base.size / sizeof(uint64_t); - validated_shader = kcalloc(sizeof(*validated_shader), 1, GFP_KERNEL); + validated_shader = kcalloc(1, sizeof(*validated_shader), GFP_KERNEL); if (!validated_shader) return NULL; @@ -498,7 +490,7 @@ vc4_validate_shader(struct drm_gem_cma_object *shader_obj) if (ip == max_ip) { DRM_ERROR("shader failed to terminate before " - "shader BO end at %d\n", + "shader BO end at %zd\n", shader_obj->base.size); goto fail; } @@ -514,6 +506,9 @@ vc4_validate_shader(struct drm_gem_cma_object *shader_obj) return validated_shader; fail: - kfree(validated_shader); + if (validated_shader) { + kfree(validated_shader->texture_samples); + kfree(validated_shader); + } return NULL; } diff --git a/src/gallium/drivers/vc4/vc4_simulator_validate.h b/src/gallium/drivers/vc4/vc4_simulator_validate.h index 68ace0216aa..40d3ada6ca2 100644 --- a/src/gallium/drivers/vc4/vc4_simulator_validate.h +++ b/src/gallium/drivers/vc4/vc4_simulator_validate.h @@ -65,7 +65,7 @@ struct drm_device { }; struct drm_gem_object { - uint32_t size; + size_t size; struct drm_device *dev; }; -- 2.30.2