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

[Tvrtko Ursulin]

On 15/09/2017 01:00, Rogozhkin, Dmitry V wrote:
> 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.

Well spotted, I'll add the test for this. I am working on tests at the 
moment anyway. It is taking a bit longer than expected due finding 
issues, not limited to this one.

Regards,

Tvrtko