On Fri, 28 Mar 2014, [email protected] wrote: > From: Brad Volkin <[email protected]> > > 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 <[email protected]> > Signed-off-by: Brad Volkin <[email protected]> > --- > 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 2eb2aca..34e2d45 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;
Given the comment "Only allowed for LRI and SRM" above, would it be better to just whitelist the allowed commands here? *shrug* Reviewed-by: Jani Nikula <[email protected]> > + > + 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; > -- > 1.8.3.2 > > _______________________________________________ > Intel-gfx mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
