Re: [PATCH v1 1/2] perf vendor events arm64: Add N3 events/metrics

[James Clark <james.clark@linaro.org>]

On 18/01/2025 6:22 pm, Namhyung Kim wrote:
> Hello,
> 
> On Mon, Jan 13, 2025 at 04:48:06PM +0000, James Clark wrote:
>>
>>
>> On 13/01/2025 3:51 pm, Ian Rogers wrote:
>>> On Mon, Jan 13, 2025 at 4:35 AM James Clark <james.clark@linaro.org> wrote:
>>>>
>>>>
>>>>
>>>> On 10/01/2025 5:20 pm, Ian Rogers wrote:
>>>>> Using the scripts at:
>>>>> https://gitlab.arm.com/telemetry-solution/telemetry-solution/
>>>>>
>>>>> Generate perf json for neoverse-n3 using the following command:
>>>>> ```
>>>>> $ telemetry-solution/tools/perf_json_generator/generate.py \
>>>>>      tools/perf/ --telemetry-files \
>>>>>      telemetry-solution/data/pmu/cpu/neoverse/neoverse-n3.json:neoverse-n3
>>>>> ```
>>>>>
>>>>> Signed-off-by: Ian Rogers <irogers@google.com>
>>>>> ---
>>>>>     .../arm64/arm/neoverse-n3/core-imp-def.json   | 930 ++++++++++++++++++
>>>>
>>>> I assume you got warnings about missing groups, if one is missing that
>>>> results in everything being dumped in core-imp-def.json. I have an
>>>> internal ticket open about it which is why I didn't post N3 and V3 jsons
>>>> yet, I'll check on it again.
>>>
>>> I was testing with JEVENTS_ARCH=all. I'm not sure what the group
>>> warning is, do you have a link?
>>>
>>
>>  From the perf_json_generator:
>>
>>   $ telemetry-solution/tools/perf_json_generator/generate.py \
>>     tools/perf/ --telemetry-files \
>>     telemetry-solution/data/pmu/cpu/neoverse/neoverse-n3.json:neoverse-n3
>>
>>   Warning: Not writing events which do not have a group (note that the
>>   mnemonic printed may differ to the one in the source json if it differs
>>   in the common Perf files):
>>
>>   IMP_CT_FLUSH: Counts flushes including architectural,
>>   microarchitectural, and branch redirects.
>>   ...
>>
>>>> Up to you whether you want to continue with it like this and I can send
>>>> a fix up later but there's also a conflict with the recent Fujitsu
>>>> Monaka events.
>>>
>>> I'm happy to let you push the updates. An alternative, that may work
>>> around the group issue, is just to push the event updates. Any chance
>>> on an ETA?
>>>
>>> Thanks,
>>> Ian
>>
>> Can't say on an eta. There was a fix already done but not made live yet, but
>> now it turns out maybe we don't want to put these particular events into
>> Perf because they're not in the TRM. Could be several weeks before anything
>> goes live.
>>
>> It's a bit of a hack but I can regenerate them without these particular
>> events in a V2, and then whatever the outcome is from above will be an
>> incremental change later.
>   
> So what's the final decision?
> 
> James, do you want to publish the metrics later at once or are you ok
> with this patch for now (and do the incremental change later)?
> 
> Thanks,
> Namhyung
> 

I'm just finishing off some changes to the generator script and the 
change will end up a bit different. Turns out it wasn't updating the 
common events properly either.

I think it's best to leave this one out and I'll send a v2 this week.