Re: [PATCH] arm64: perf: Do not write event type for cycle counter

[Julien Thierry <julien.thierry@arm.com>]

On 01/04/2019 09:02, Mark Rutland wrote:
> On Thu, Mar 28, 2019 at 09:49:05AM +0000, Julien Thierry wrote:
>> Perf events using the dedicated cycle counter do not need to program the
>> event type, the counter only ever counts that kind of events.
> 
> Good catch!
> 
>> Even worse, trying to program an event type without excluding the
>> cycle counter index might end up in the modification of the type of
>> event counted by another perf event.
> 
> IIUC, we shouldn't affect an unrelated event. The ARM ARM says:
> 
>   When PMSELR_EL0.SEL == 31, this register accesses PMCCFILTR_EL0.
> 

Ah yes, I missed that fact.

> ... and AFAICT we program PMSELR_EL0 appropriately to select the cycle counter.
> 
> I think we're erroneously programming the RES0 bits of PMCCFILTR_EL0, but we're
> also relying on this write to clear all the filter controls in the high bits of
> PMCCFILTR_EL0, which otherwise reset to UNKNOWN values.
> 
> Given that, I think we also need to explicitly reset PMCCFILTR_EL0 in
> armv8pmu_reset().
> 

I just realized that for the PMCCFILTR_EL0 there is also the exclude
control bits for the event. Do those not need to be set when we enable
the event rather than on reset?

Now, I'm thinking what we need is to use a different mask than
EVTYPE_MASK (one that excludes the event bits) when setting
PMCCFILTR_EL0, but still do that when enabling an event using the cycle
counter.

Does that approach sound correct? Or am I missing something that would
allow us to only set that upon reset?

Thanks,

> Thanks,
> Mark.
> 
>>
>> Reported-by: Wei Li <liwei391@huawei.com>
>> Signed-off-by: Julien Thierry <julien.thierry@arm.com>
>> Cc: Will Deacon <will.deacon@arm.com>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
>> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>> Cc: Jiri Olsa <jolsa@redhat.com>
>> Cc: Namhyung Kim <namhyung@kernel.org>
>> ---
>>  arch/arm64/kernel/perf_event.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
>> index 4addb38..3f898bc 100644
>> --- a/arch/arm64/kernel/perf_event.c
>> +++ b/arch/arm64/kernel/perf_event.c
>> @@ -514,7 +514,7 @@ static inline void armv8pmu_write_event_type(struct perf_event *event)
>>
>>  		armv8pmu_write_evtype(idx - 1, hwc->config_base);
>>  		armv8pmu_write_evtype(idx, chain_evt);
>> -	} else {
>> +	} else if (idx != ARMV8_IDX_CYCLE_COUNTER) {
>>  		armv8pmu_write_evtype(idx, hwc->config_base);
>>  	}
>>  }
>> --
>> 1.9.1

-- 
Julien Thierry

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel