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. _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx