Re: [PATCH v8 09/12] drm/i915: Add dev.i915.perf_stream_paranoid sysctl option

On Thu, 2016-10-27 at 19:14 -0700, Robert Bragg wrote:

Consistent with the kernel.perf_event_paranoid sysctl option that can allow non-root users to access system wide cpu metrics, this can optionally allow non-root users to access system wide OA counter metrics from Gen graphics hardware.

Signed-off-by: Robert Bragg robert@sixbynine.org Reviewed-by: Matthew Auld matthew.auld@intel.com

---

drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_perf.c | 50 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 01438fb..a138f86 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2171,6 +2171,7 @@ struct drm_i915_private { bool initialized;

struct kobject *metrics_kobj;

• struct ctl_table_header *sysctl_header;
  
  

  struct mutex lock; struct list_head streams;

diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index 8d07c41..4e42073 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -64,6 +64,11 @@ #define POLL_FREQUENCY 200 #define POLL_PERIOD (NSEC_PER_SEC / POLL_FREQUENCY)

+/* for sysctl proc_dointvec_minmax of dev.i915.perf_stream_paranoid */ +static int zero; +static int one = 1; +static u32 i915_perf_stream_paranoid = true;

/* The maximum exponent the hardware accepts is 63 (essentially it selects one

• of the 64bit timestamp bits to trigger reports from) but there's currently
• no known use case for sampling as infrequently as once per 47 thousand years.

@@ -1207,7 +1212,13 @@ i915_perf_open_ioctl_locked(struct drm_i915_private *dev_priv, } }

• if (!specific_ctx && !capable(CAP_SYS_ADMIN)) {

• /* Similar to perf's kernel.perf_paranoid_cpu sysctl option

• * we check a dev.i915.perf_stream_paranoid sysctl option
  

• * to determine if it's ok to access system wide OA counters
  

• * without CAP_SYS_ADMIN privileges.
  

• */
  

• if (!specific_ctx &&

•    i915_perf_stream_paranoid && !capable(CAP_SYS_ADMIN)) {
  

  DRM_ERROR("Insufficient privileges to open system-wide i915 perf stream\n"); ret = -EACCES; goto err_ctx;

@@ -1454,6 +1465,39 @@ void i915_perf_unregister(struct drm_i915_private *dev_priv) dev_priv->perf.metrics_kobj = NULL; }

+static struct ctl_table oa_table[] = {

• {

• .procname = "perf_stream_paranoid",
  

• .data = &i915_perf_stream_paranoid,
  

• .maxlen = sizeof(i915_perf_stream_paranoid),
  

• .mode = 0644,
  

• .proc_handler = proc_dointvec_minmax,
  

• .extra1 = &zero,
  

• .extra2 = &one,
  

• },
  

• {}

+};

+static struct ctl_table i915_root[] = {

• {

• .procname = "i915",
  

• .maxlen = 0,
  

• .mode = 0555,
  

• .child = oa_table,
  

• },
  

• {}

+};

+static struct ctl_table dev_root[] = {

• {

• .procname = "dev",
  

• .maxlen = 0,
  

• .mode = 0555,
  

• .child = i915_root,
  

• },
  

• {}

+};

void i915_perf_init(struct drm_i915_private *dev_priv) { if (!IS_HASWELL(dev_priv)) @@ -1484,6 +1528,8 @@ void i915_perf_init(struct drm_i915_private *dev_priv) dev_priv->perf.oa.n_builtin_sets = i915_oa_n_builtin_metric_sets_hsw;

• dev_priv->perf.sysctl_header = register_sysctl_table(dev_root);
• dev_priv->perf.initialized = true;

}

@@ -1492,6 +1538,8 @@ void i915_perf_fini(struct drm_i915_private *dev_priv) if (!dev_priv->perf.initialized) return;

• unregister_sysctl_table(dev_priv->perf.sysctl_header);
• memset(&dev_priv->perf.oa.ops, 0, sizeof(dev_priv->perf.oa.ops)); dev_priv->perf.initialized = false;

}

Looks fine. Reviewed-by: Sourab Gupta sourab.gupta@intel.com