linux-kernel - Re: [PATCH 0/2] perf vendor events arm64: Add A720/A520 events/metrics

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2834c9d7-f401-4b29-8f92-22193eb9ce31@linaro.org>
Date: Wed, 19 Feb 2025 15:25:39 +0000
From: James Clark <james.clark@...aro.org>
To: Ian Rogers <irogers@...gle.com>, Namhyung Kim <namhyung@...nel.org>
Cc: Yangyu Chen <cyy@...self.name>, linux-perf-users@...r.kernel.org,
 John Garry <john.g.garry@...cle.com>, Will Deacon <will@...nel.org>,
 Mike Leach <mike.leach@...aro.org>, Leo Yan <leo.yan@...ux.dev>,
 Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
 Arnaldo Carvalho de Melo <acme@...nel.org>,
 Mark Rutland <mark.rutland@....com>,
 Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
 Jiri Olsa <jolsa@...nel.org>, Adrian Hunter <adrian.hunter@...el.com>,
 Liang Kan <kan.liang@...ux.intel.com>,
 Yoshihiro Furudera <fj5100bi@...itsu.com>,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/2] perf vendor events arm64: Add A720/A520
 events/metrics



On 18/02/2025 10:33 pm, Ian Rogers wrote:
> On Tue, Feb 18, 2025 at 2:19 PM Namhyung Kim <namhyung@...nel.org> wrote:
>>
>> On Tue, Feb 18, 2025 at 09:30:23AM +0000, James Clark wrote:
>>>
>>>
>>> On 18/02/2025 12:41 am, Ian Rogers wrote:
>>>> On Fri, Feb 14, 2025 at 2:02 AM James Clark <james.clark@...aro.org> wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 14/02/2025 5:49 am, Yangyu Chen wrote:
>>>>>>
>>>>>>
>>>>>>> On 14 Feb 2025, at 09:12, Namhyung Kim <namhyung@...nel.org> wrote:
>>>>>>>
>>>>>>> Hello,
>>>>>>>
>>>>>>> On Thu, Feb 13, 2025 at 11:11:01PM +0800, Yangyu Chen wrote:
>>>>>>>> This patchset adds the perf JSON files for the Cortex-A720 and Cortex-A520
>>>>>>>> processors. Some events have been tested on Raxda Orion 6 with Cix P1 SoC
>>>>>>>> (8xA720 + 4xA520) running mainline Kernel with ACPI mode.
>>>>>>>
>>>>>>> I'm curious how the name of PMUs look like.  It is cortex_a720 (or a520)?
>>>>>>
>>>>>> The name of PMUs comes from Arm's documentation. I have included these
>>>>>> links in each patch.
>>>>>>
>>>>>>> I remember there's a logic to check the length of hex digits at the end.
>>>>>>>
>>>>>>
>>>>>> Could you provide more details about this?
>>>>>>
>>>>>>> Ian, are you ok with this?
>>>>>>>
>>>>>
>>>>> I think they wouldn't be merged because they're core PMUs, so should be
>>>>> fine? Even though they would otherwise be merged because they're more
>>>>> than 3 hex digits.
>>>>
>>>> Do we know the PMU names? If they are cortex_a520 and cortex_a720 then
>>>
>>> It will be "armv9_cortex_a720" from this line:
>>>
>>>    PMUV3_INIT_SIMPLE(armv9_cortex_a720)
>>
>> I see, thanks!
>>
>>>
>>>> this comment at least reads a little stale:
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/pmus.c?h=perf-tools-next#n76
>>>> ```
>>>> /*
>>>> * There is a '_{num}' suffix. For decimal suffixes any length
>>>> * will do, for hexadecimal ensure more than 2 hex digits so
>>>> * that S390's cpum_cf PMU doesn't match.
>>>> */
>>>> ```
>>>> James is right that core PMUs aren't put on the same list as uncore/other PMUs.
>>
>> Ok, then I guess we're good.
> 
> I think you may be able to do things that look odd, like today the
> "i915" PMU can be called just "i", I think the a520/a720 naming will
> allow "armv9_cortex/cycles/" as an event name, then open it on two
> PMUs if they are present. 

I assumed that was the intended behavior. It seems fairly useful to be 
able to open on ones with common prefixes.

> We may only show one PMU in perf list as
> that code I think assumes they're the same PMU as they only differ by
> suffix:
> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/pmus.c?h=perf-tools-next#n384

Yeah that is the case. I didn't realise it when looking at the previous 
fixes to keep the suffixes in perf stat output.

> I can imagine aggregation possibly being broken, but I think that
> works off the number of PMUs not the names of the PMUs, so it should
> be okay. Probably the only thing broken that matter is perf list when
> you have a BIG.little system with a520 and a720, this may be broken
> with say a a53 and a72 today as both of those suffix lengths are >2,
> but maybe they use the "armv8._pmuv3_0", "armv8._pmuv3_1", etc. naming
> convention. I suspect the >2 here:

Also the case for a53 and a72 right now. Even "perf list --unit 
armv8_cortex_a57" doesn't work because we deduplicate before filtering. 
Adding -v fixes it though because that disables deduplication. Perhaps 
we can change it to disable it with the --unit argument?

> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/pmus.c?h=perf-tools-next#n80
> would still work and be correct if it were >4. If that changes then
> this will also need to change:
> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/Documentation/ABI/testing/sysfs-bus-event_source-devices?h=perf-tools-next#n12

That could be an easy fix. If >4 is enough to still get rid of all the 
uncore duplicates I can make the change?

> 
> Thanks,
> Ian
> 
>>
>> Thanks,
>> Namhyung
>>