[Intel-gfx] [PATCH v2 3/3] drm/i915: Track OACONTROL register enable/disable during parsing

[bradley.d.volkin at intel.com]

From: Brad Volkin <bradley.d.volkin at intel.com>

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 <kenneth at whitecape.org>
Signed-off-by: Brad Volkin <bradley.d.volkin at intel.com>
---
 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;
+
+			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