Re: [RFC v7 04/11] drm/i915/pmu: Expose a PMU interface for perf queries

Rogozhkin, Dmitry V
2017-09-15T00:00:34Z

On Wed, 2017-09-13 at 11:34 +0100, Tvrtko Ursulin wrote:
> +static int i915_pmu_event_init(struct perf_event *event)
> +{
> + struct drm_i915_private *i915 =
> + container_of(event->pmu, typeof(*i915), pmu.base);
> + int cpu, ret;
> +
> + if (event->attr.type != event->pmu->type)
> + return -ENOENT;
> +
> + /* unsupported modes and filters */
> + if (event->attr.sample_period) /* no sampling */
> + return -EINVAL;
> +
> + if (has_branch_stack(event))
> + return -EOPNOTSUPP;
> +
> + if (event->cpu < 0)
> + return -EINVAL;
> +
> + cpu = cpumask_any_and(&i915_pmu_cpumask,
> + topology_sibling_cpumask(event->cpu));
> + if (cpu >= nr_cpu_ids)
> + return -ENODEV;
> +
> + ret = 0;
> + if (is_engine_event(event)) {
> + ret = engine_event_init(event);
> + } else switch (event->attr.config) {
> + case I915_PMU_ACTUAL_FREQUENCY:
> + if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
> + ret = -ENODEV; /* requires a mutex for
> sampling! */
> + case I915_PMU_REQUESTED_FREQUENCY:
> + case I915_PMU_ENERGY:
> + case I915_PMU_RC6_RESIDENCY:
> + case I915_PMU_RC6p_RESIDENCY:
> + case I915_PMU_RC6pp_RESIDENCY:
> + if (INTEL_GEN(i915) < 6)
> + ret = -ENODEV;
> + break;
> + }
> + if (ret)
> + return ret;

The switch for non-engine events should error out by default:

diff --git a/drivers/gpu/drm/i915/i915_pmu.c
b/drivers/gpu/drm/i915/i915_pmu.c
index d734879..3145e9a 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -329,6 +329,9 @@ static int i915_pmu_event_init(struct perf_event
*event)
if (INTEL_GEN(i915) < 6)
ret = -ENODEV;
break;
+ default:
+ ret = -ENOENT;
+ break;
}
if (ret)
return ret;

Otherwise user may try to enable non-existing metric (> I915_PMU_LAST)
and eventually will be subject to kernel panic on
i915_pmu_enable/disable during refcount operations. And we need to have
an IGT test to check that.