Re: [PATCH v2] perf vendor events arm64: Add FUJITSU-MONAKA pmu event

[James Clark]

On 17/12/2024 6:35 am, Yoshihiro Furudera (Fujitsu) wrote:
> Hi, James
>
> > > Also in the common files, we're using the description strings from the
> > > Arm ARM. I noticed that the ones from your spec are slightly different.
> > > This is ok for now, but if we add any new Arm cores that use the same
> > > events this description may get overwritten. For example
> > > ASE_SVE_INST_SPEC in the Arm ARM is currently "Operation speculatively
> > executed, Advanced SIMD or SVE".
> > > If you have any actual relevant details that are different from the
> > > common string, you should put them in .../arch/arm64/fujitsu/monaka.
> > > But if you are ok with the potential overwrite (which looks like it
> > > should be ok) then you can leave them as is.
> >
> > I think it's okay to overwrite it, but I'll check the specs.
>
> Regarding this matter, the descriptions of the events added to the common files
> (common-and-microarch.json, recommended.json) will be modified to match Arm ARM.
> Then, we will write current descriptions as "BriefDescription" in .../arch/arm64/fujitsu/monaka.
> We will post this as a v3 patch later.
>
> > > [...]
> > >
> > > > diff --git
> > > > a/tools/perf/pmu-events/arch/arm64/fujitsu/monaka/general.json
> > > > b/tools/perf/pmu-events/arch/arm64/fujitsu/monaka/general.json
> > > > new file mode 100644
> > > > index 000000000000..80bf17fb8f4c
> > > > --- /dev/null
> > > > +++ b/tools/perf/pmu-events/arch/arm64/fujitsu/monaka/general.json
> > > > @@ -0,0 +1,8 @@
> > > > +[
> > > > +    {
> > > > +        "ArchStdEvent": "CPU_CYCLES"
> > > > +    },
> > >
> > > Other cores put CPU_CYCLES in bus.json. For user friendlyness I would
> > > put the common ones into the same existing groups. The same issue
> > > applies for some other common events.
> >
> > I will update as suggested.
>
> MONAKA's event grouping is based on the grouping of "neoverse n1/n2".
> We found that the groups in "neoverse n1/n2" may differ from those in other cores.
> For example, "CPU_CYCLES" is classified in general.json in "neoverse n1/n2",
> but in bus.json in many other cores.
>
> As a result, it is not possible to completely match the event groups of other cores.
> We will be checking the current grouping of events and modifying them as much as possible
> to match the other core, and will post this later as a v3 patch.
>
> Best Regards,
> Yoshihiro Furudera

Hmmm ok, I didn't realize it was already inconsistent. Still it's better 
to try to not introduce a third variant though.