From 6e66ea137ff5bd292ed734c53f569892068a7a70 Mon Sep 17 00:00:00 2001 From: Brad Volkin Date: Fri, 28 Mar 2014 10:21:50 -0700 Subject: [PATCH] drm/i915: Track OACONTROL register enable/disable during parsing There is some thought that the data from the performance counters enabled via OACONTROL should only be available to the process that enabled counting. To limit snooping, require that any batch buffer which sets OACONTROL to a non-zero value also sets it back to 0 before the end of the batch. This requires limiting OACONTROL writes to happen via MI_LOAD_REGISTER_IMM so that we can access the value being written. This should be in line with the expected use case for writing OACONTROL. v2: Drop an unnecessary '? true : false' Cc: Kenneth Graunke Signed-off-by: Brad Volkin Reviewed-by: Kenneth Graunke Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_cmd_parser.c | 35 ++++++++++++++++++++------ 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c index ef7242e14979..5b45c191c4e7 100644 --- a/drivers/gpu/drm/i915/i915_cmd_parser.c +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c @@ -407,12 +407,7 @@ static const u32 gen7_render_regs[] = { REG64(CL_PRIMITIVES_COUNT), REG64(PS_INVOCATION_COUNT), REG64(PS_DEPTH_COUNT), - /* - * FIXME: This is just to keep mesa working for now, we need to check - * that mesa resets this again and that it doesn't use any of the - * special modes which write into the gtt. - */ - OACONTROL, + OACONTROL, /* Only allowed for LRI and SRM. See below. */ REG64(GEN7_SO_NUM_PRIMS_WRITTEN(0)), REG64(GEN7_SO_NUM_PRIMS_WRITTEN(1)), REG64(GEN7_SO_NUM_PRIMS_WRITTEN(2)), @@ -761,7 +756,8 @@ bool i915_needs_cmd_parser(struct intel_ring_buffer *ring) static bool check_cmd(const struct intel_ring_buffer *ring, const struct drm_i915_cmd_descriptor *desc, const u32 *cmd, - const bool is_master) + const bool is_master, + bool *oacontrol_set) { if (desc->flags & CMD_DESC_REJECT) { DRM_DEBUG_DRIVER("CMD: Rejected command: 0x%08X\n", *cmd); @@ -777,6 +773,23 @@ static bool check_cmd(const struct intel_ring_buffer *ring, if (desc->flags & CMD_DESC_REGISTER) { u32 reg_addr = cmd[desc->reg.offset] & desc->reg.mask; + /* + * OACONTROL requires some special handling for writes. We + * want to make sure that any batch which enables OA also + * disables it before the end of the batch. The goal is to + * prevent one process from snooping on the perf data from + * another process. To do that, we need to check the value + * that will be written to the register. Hence, limit + * OACONTROL writes to only MI_LOAD_REGISTER_IMM commands. + */ + if (reg_addr == OACONTROL) { + if (desc->cmd.value == MI_LOAD_REGISTER_MEM) + return false; + + if (desc->cmd.value == MI_LOAD_REGISTER_IMM(1)) + *oacontrol_set = (cmd[2] != 0); + } + if (!valid_reg(ring->reg_table, ring->reg_count, reg_addr)) { if (!is_master || @@ -851,6 +864,7 @@ int i915_parse_cmds(struct intel_ring_buffer *ring, u32 *cmd, *batch_base, *batch_end; struct drm_i915_cmd_descriptor default_desc = { 0 }; int needs_clflush = 0; + bool oacontrol_set = false; /* OACONTROL tracking. See check_cmd() */ ret = i915_gem_obj_prepare_shmem_read(batch_obj, &needs_clflush); if (ret) { @@ -900,7 +914,7 @@ int i915_parse_cmds(struct intel_ring_buffer *ring, break; } - if (!check_cmd(ring, desc, cmd, is_master)) { + if (!check_cmd(ring, desc, cmd, is_master, &oacontrol_set)) { ret = -EINVAL; break; } @@ -908,6 +922,11 @@ int i915_parse_cmds(struct intel_ring_buffer *ring, cmd += length; } + if (oacontrol_set) { + DRM_DEBUG_DRIVER("CMD: batch set OACONTROL but did not clear it\n"); + ret = -EINVAL; + } + if (cmd >= batch_end) { DRM_DEBUG_DRIVER("CMD: Got to the end of the buffer w/o a BBE cmd!\n"); ret = -EINVAL; -- 2.39.5