On 8/26/25 15:43, Mark Rutland wrote:
> On Wed, Aug 13, 2025 at 06:01:10PM +0100, Robin Murphy wrote:
>> Only a handful of CPU PMUs accept PERF_TYPE_{RAW,HARDWARE,HW_CACHE}
>> events without registering themselves as PERF_TYPE_RAW in the first
>> place. Add an explicit opt-in for these special cases, so that we can
>> make life easier for every other driver (and probably also speed up the
>> slow-path search) by having perf_try_init_event() do the basic type
>> checking to cover the majority of cases.
>>
>> Signed-off-by: Robin Murphy <robin.mur...@arm.com>
>
>
> To bikeshed a little here, I'm not keen on the PERF_PMU_CAP_RAW_EVENTS
> name, because it's not clear what "RAW" really means, and people will
> definitely read that to mean something else.
>
> Could we go with something like PERF_PMU_CAP_COMMON_CPU_EVENTS, to make
> it clear that this is about opting into CPU-PMU specific event types (of
> which PERF_TYPE_RAW is one of)?
>
> Likewise, s/is_raw_pmu()/pmu_supports_common_cpu_events()/.
>
>> ---
>>
>> A further possibility is to automatically add the cap to PERF_TYPE_RAW
>> PMUs in perf_pmu_register() to have a single point-of-use condition; I'm
>> undecided...
>
> I reckon we don't need to automagically do that, but I reckon that
> is_raw_pmu()/pmu_supports_common_cpu_events() should only check the cap,
> and we don't read anything special into any of
> PERF_TYPE_{RAW,HARDWARE,HW_CACHE}.
>
>> ---
>> arch/s390/kernel/perf_cpum_cf.c | 1 +
>> arch/s390/kernel/perf_pai_crypto.c | 2 +-
>> arch/s390/kernel/perf_pai_ext.c | 2 +-
>> arch/x86/events/core.c | 2 +-
>> drivers/perf/arm_pmu.c | 1 +
>> include/linux/perf_event.h | 1 +
>> kernel/events/core.c | 15 +++++++++++++++
>> 7 files changed, 21 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/s390/kernel/perf_cpum_cf.c
>> b/arch/s390/kernel/perf_cpum_cf.c
>> index 1a94e0944bc5..782ab755ddd4 100644
>> --- a/arch/s390/kernel/perf_cpum_cf.c
>> +++ b/arch/s390/kernel/perf_cpum_cf.c
>> @@ -1054,6 +1054,7 @@ static void cpumf_pmu_del(struct perf_event *event,
>> int flags)
>> /* Performance monitoring unit for s390x */
>> static struct pmu cpumf_pmu = {
>> .task_ctx_nr = perf_sw_context,
>> + .capabilities = PERF_PMU_CAP_RAW_EVENTS,
>> .pmu_enable = cpumf_pmu_enable,
>> .pmu_disable = cpumf_pmu_disable,
>> .event_init = cpumf_pmu_event_init,
>
> Tangential, but use of perf_sw_context here looks bogus.
>
It might look strange, but it was done on purpose. For details see
commit 9254e70c4ef1 ("s390/cpum_cf: use perf software context for hardware
counters")
Background was a WARN_ON() statement which fired, because several PMU device
drivers
existed in parallel on s390x platform.
Not sure if this condition is still true after all these years...
--
Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
--
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Wolfgang Wendt
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB
243294
