All of lore.kernel.org
 help / color / mirror / Atom feed
From: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
To: intel-gfx@lists.freedesktop.org
Cc: Chris Wilson <chris.p.wilson@intel.com>
Subject: [Intel-gfx] [PATCH 4/6] drm/i915/perf: Whitelist OA report trigger registers
Date: Wed, 29 Jul 2020 17:48:24 -0700	[thread overview]
Message-ID: <20200730004826.8415-5-umesh.nerlige.ramappa@intel.com> (raw)
In-Reply-To: <20200730004826.8415-1-umesh.nerlige.ramappa@intel.com>

From: Piotr Maciejewski <piotr.maciejewski@intel.com>

OA reports can be triggered into the OA buffer by writing into the
OAREPORTTRIG registers. Whitelist the registers to allow non-privileged
user to trigger reports.

Whitelist registers only if perf_stream_paranoid is set to 0. In
i915_perf_open_ioctl, this setting is checked and the whitelist is
enabled accordingly. On closing the perf fd, the whitelist is removed.

This ensures that the access to the whitelist is gated by
perf_stream_paranoid.

v2:
- Move related change to this patch (Lionel)
- Bump up perf revision (Lionel)

v3: Pardon whitelisted registers for selftest (Umesh)
v4: Document supported gens for the feature (Lionel)
v5: Whitelist registers only if perf_stream_paranoid is set to 0 (Jon)

Signed-off-by: Piotr Maciejewski <piotr.maciejewski@intel.com>
Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 67 +++++++++++++++++++++
 drivers/gpu/drm/i915/gt/intel_workarounds.h |  3 +
 drivers/gpu/drm/i915/i915_perf.c            | 33 +++++++++-
 drivers/gpu/drm/i915/i915_perf_types.h      |  5 ++
 4 files changed, 105 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 98927f5d63ab..e096282ad547 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -1671,6 +1671,73 @@ void intel_engine_apply_whitelist(struct intel_engine_cs *engine)
 				   i915_mmio_reg_offset(RING_NOPID(base)));
 }
 
+struct i915_oa_wl {
+	i915_reg_t reg;
+	u32 flags;
+} gen9_oa_regs[] = {
+	{ OAREPORTTRIG2, RING_FORCE_TO_NONPRIV_ACCESS_RW },
+	{ OAREPORTTRIG6, RING_FORCE_TO_NONPRIV_ACCESS_RW },
+}, gen12_oa_regs[] = {
+	{ GEN12_OAG_OAREPORTTRIG2, RING_FORCE_TO_NONPRIV_ACCESS_RW },
+	{ GEN12_OAG_OAREPORTTRIG6, RING_FORCE_TO_NONPRIV_ACCESS_RW },
+};
+
+static void whitelist_delete_perf_counters(struct i915_wa_list *wal,
+					   struct i915_oa_wl *regs,
+					   int count)
+{
+	while (count--) {
+		_wa_remove(wal, regs->reg);
+		regs++;
+	}
+
+	wa_init_finish(wal);
+}
+
+static void whitelist_build_perf_counters(struct i915_wa_list *wal,
+					  struct i915_oa_wl *regs,
+					  int count)
+{
+	while (count--) {
+		whitelist_reg_ext(wal, regs->reg, regs->flags);
+		regs++;
+	}
+}
+
+void intel_engine_apply_oa_whitelist(struct intel_engine_cs *engine)
+{
+	struct i915_wa_list *w = &engine->whitelist;
+	struct drm_i915_private *i915 = engine->i915;
+
+	if (IS_GEN(i915, 12))
+		whitelist_build_perf_counters(w, gen12_oa_regs,
+					      ARRAY_SIZE(gen12_oa_regs));
+	else if (INTEL_GEN(i915) > 8)
+		whitelist_build_perf_counters(w, gen9_oa_regs,
+					      ARRAY_SIZE(gen9_oa_regs));
+	else
+		return;
+
+	intel_engine_apply_whitelist(engine);
+}
+
+void intel_engine_remove_oa_whitelist(struct intel_engine_cs *engine)
+{
+	struct i915_wa_list *w = &engine->whitelist;
+	struct drm_i915_private *i915 = engine->i915;
+
+	if (IS_GEN(i915, 12))
+		whitelist_delete_perf_counters(w, gen12_oa_regs,
+					       ARRAY_SIZE(gen12_oa_regs));
+	else if (INTEL_GEN(i915) > 8)
+		whitelist_delete_perf_counters(w, gen9_oa_regs,
+					       ARRAY_SIZE(gen9_oa_regs));
+	else
+		return;
+
+	intel_engine_apply_whitelist(engine);
+}
+
 static void
 rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
 {
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.h b/drivers/gpu/drm/i915/gt/intel_workarounds.h
index 8c9c769c2204..5ad3f44f615d 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.h
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.h
@@ -32,6 +32,9 @@ bool intel_gt_verify_workarounds(struct intel_gt *gt, const char *from);
 void intel_engine_init_whitelist(struct intel_engine_cs *engine);
 void intel_engine_apply_whitelist(struct intel_engine_cs *engine);
 
+void intel_engine_apply_oa_whitelist(struct intel_engine_cs *engine);
+void intel_engine_remove_oa_whitelist(struct intel_engine_cs *engine);
+
 void intel_engine_init_workarounds(struct intel_engine_cs *engine);
 void intel_engine_apply_workarounds(struct intel_engine_cs *engine);
 int intel_engine_verify_workarounds(struct intel_engine_cs *engine,
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index fe408c327d3c..0e2313d50c91 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -201,6 +201,7 @@
 #include "gt/intel_gt.h"
 #include "gt/intel_lrc_reg.h"
 #include "gt/intel_ring.h"
+#include "gt/intel_workarounds.h"
 
 #include "i915_drv.h"
 #include "i915_perf.h"
@@ -1353,6 +1354,9 @@ static void i915_oa_stream_destroy(struct i915_perf_stream *stream)
 
 	BUG_ON(stream != perf->exclusive_stream);
 
+	if (stream->oa_whitelisted)
+		intel_engine_remove_oa_whitelist(stream->engine);
+
 	/*
 	 * Unset exclusive_stream first, it will be checked while disabling
 	 * the metric set on gen8+.
@@ -1448,7 +1452,8 @@ static void gen8_init_oa_buffer(struct i915_perf_stream *stream)
 	 *  bit."
 	 */
 	intel_uncore_write(uncore, GEN8_OABUFFER, gtt_offset |
-		   OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT);
+			   OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT |
+			   GEN7_OABUFFER_EDGE_TRIGGER);
 	intel_uncore_write(uncore, GEN8_OATAILPTR, gtt_offset & GEN8_OATAILPTR_MASK);
 
 	/* Mark that we need updated tail pointers to read from... */
@@ -1501,7 +1506,8 @@ static void gen12_init_oa_buffer(struct i915_perf_stream *stream)
 	 *  bit."
 	 */
 	intel_uncore_write(uncore, GEN12_OAG_OABUFFER, gtt_offset |
-			   OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT);
+			   OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT |
+			   GEN7_OABUFFER_EDGE_TRIGGER);
 	intel_uncore_write(uncore, GEN12_OAG_OATAILPTR,
 			   gtt_offset & GEN12_OAG_OATAILPTR_MASK);
 
@@ -3474,6 +3480,22 @@ i915_perf_open_ioctl_locked(struct i915_perf *perf,
 	if (!(param->flags & I915_PERF_FLAG_DISABLED))
 		i915_perf_enable_locked(stream);
 
+	/* 
+	 * OA whitelist allows non-privileged access to some OA counters for
+	 * triggering reports into the OA buffer. This is only allowed if
+	 * perf_stream_paranoid is set to 0 by the sysadmin.
+	 *
+	 * We want to make sure this is almost the last thing we do before
+	 * returning the stream fd. If we do end up checking for errors in code
+	 * that follows this, we MUST call intel_engine_remove_oa_whitelist in
+	 * the error handling path to remove the whitelisted registers.
+	 */
+	if (!i915_perf_stream_paranoid && 
+	    props->sample_flags & SAMPLE_OA_REPORT) {
+		intel_engine_apply_oa_whitelist(stream->engine);
+		stream->oa_whitelisted = true;
+	}
+
 	/* Take a reference on the driver that will be kept with stream_fd
 	 * until its release.
 	 */
@@ -4445,8 +4467,13 @@ int i915_perf_ioctl_version(void)
 	 *
 	 * 5: Add DRM_I915_PERF_PROP_POLL_OA_PERIOD parameter that controls the
 	 *    interval for the hrtimer used to check for OA data.
+	 *
+	 * 6: Whitelist OATRIGGER registers to allow user to trigger reports
+	 *    into the OA buffer. This applies only to gen8+. The feature can
+	 *    only be accessed if perf_stream_paranoid is set to 0 by privileged
+	 *    user.
 	 */
-	return 5;
+	return 6;
 }
 
 #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
diff --git a/drivers/gpu/drm/i915/i915_perf_types.h b/drivers/gpu/drm/i915/i915_perf_types.h
index a36a455ae336..35f8240fd6ce 100644
--- a/drivers/gpu/drm/i915/i915_perf_types.h
+++ b/drivers/gpu/drm/i915/i915_perf_types.h
@@ -311,6 +311,11 @@ struct i915_perf_stream {
 	 * buffer should be checked for available data.
 	 */
 	u64 poll_oa_period;
+
+	/**
+	 * @oa_whitelisted: Indicates that the oa registers are whitelisted.
+	 */
+	bool oa_whitelisted;
 };
 
 /**
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  parent reply	other threads:[~2020-07-30  0:48 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-30  0:48 [Intel-gfx] [PATCH 0/6] Allow privileged user to map the OA buffer Umesh Nerlige Ramappa
2020-07-30  0:48 ` [Intel-gfx] [PATCH 1/6] drm/i915: Allow removal of whitelist register and refactor Umesh Nerlige Ramappa
2020-07-30  0:48 ` [Intel-gfx] [PATCH 2/6] drm/i915/selftests: Clear flags when using wa->reg for comparison Umesh Nerlige Ramappa
2020-07-30  0:48 ` [Intel-gfx] [PATCH 3/6] drm/i915/perf: Ensure observation logic is not clock gated Umesh Nerlige Ramappa
2020-07-30  0:48 ` Umesh Nerlige Ramappa [this message]
2020-07-30  8:14   ` [Intel-gfx] [PATCH 4/6] drm/i915/perf: Whitelist OA report trigger registers Chris Wilson
2020-07-30  0:48 ` [Intel-gfx] [PATCH 5/6] drm/i915/perf: Whitelist OA counter and buffer registers Umesh Nerlige Ramappa
2020-07-30  0:48 ` [Intel-gfx] [PATCH 6/6] drm/i915/perf: Map OA buffer to user space for gen12 performance query Umesh Nerlige Ramappa
2020-07-30  9:00   ` Chris Wilson
2020-07-30  9:08   ` Chris Wilson
2020-07-30  1:11 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Allow privileged user to map the OA buffer (rev6) Patchwork
2020-07-30  1:12 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2020-07-30  1:30 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-07-30  2:27 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200730004826.8415-5-umesh.nerlige.ramappa@intel.com \
    --to=umesh.nerlige.ramappa@intel.com \
    --cc=chris.p.wilson@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.