Changes since v5:
- Split the first patch out into 1st pmu.c change and 2nd metricgroup.c
change.
- Change order to check alias and then identifier.
- Fix matching_pmu.
- Add event documentation links.
Jing Zhang (7):
perf pmu: "Compat" supports matching multiple identifiers
perf metric: "Compat" supports matching multiple identifiers
perf jevents: Support more event fields
perf test: Fix matching_pmu
perf test: Add pmu-event test for "Compat" and new event_field.
perf jevents: Add support for Arm CMN PMU aliasing
perf vendor events: Add JSON metrics for Arm CMN
.../pmu-events/arch/arm64/arm/cmn/sys/cmn.json | 266 +++++++++++++++++++++
.../pmu-events/arch/arm64/arm/cmn/sys/metric.json | 74 ++++++
.../pmu-events/arch/test/test_soc/sys/uncore.json | 8 +
tools/perf/pmu-events/jevents.py | 22 +-
tools/perf/tests/pmu-events.c | 64 ++++-
tools/perf/util/metricgroup.c | 2 +-
tools/perf/util/pmu.c | 33 ++-
tools/perf/util/pmu.h | 1 +
8 files changed, 457 insertions(+), 13 deletions(-)
create mode 100644 tools/perf/pmu-events/arch/arm64/arm/cmn/sys/cmn.json
create mode 100644 tools/perf/pmu-events/arch/arm64/arm/cmn/sys/metric.json
--
1.8.3.1
The usual event descriptions are "event=xxx" or "config=xxx", while the
event descriptions of CMN are "type=xxx, eventid=xxx" or more complex.
$cat /sys/bus/event_source/devices/arm_cmn_0/events/hnf_cache_fill
type=0x5,eventid=0x3
When adding aliases for events described as "event=xxx" or "config=xxx",
EventCode or ConfigCode can be used in the JSON files to describe the
events. But "eventid=xxx, type=xxx" cannot be supported at present.
If EventCode and ConfigCode is not added in the alias JSON file, the
event description will add "event=0" by default. So, even if the event
field is added to supplement "eventid=xxx" and "type=xxx", the final
parsing result will be "event=0, eventid=xxx, type=xxx".
Therefore, when EventCode and ConfigCode are missing in JSON, "event=0" is
no longer added by default. EventIdCode and Type are added to the event
field, and ConfigCode is moved into the event_field array which can also
guarantee its original function.
Signed-off-by: Jing Zhang <[email protected]>
---
tools/perf/pmu-events/jevents.py | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
index f57a8f2..9c0f63a 100755
--- a/tools/perf/pmu-events/jevents.py
+++ b/tools/perf/pmu-events/jevents.py
@@ -275,12 +275,6 @@ class JsonEvent:
}
return table[unit] if unit in table else f'uncore_{unit.lower()}'
- eventcode = 0
- if 'EventCode' in jd:
- eventcode = int(jd['EventCode'].split(',', 1)[0], 0)
- if 'ExtSel' in jd:
- eventcode |= int(jd['ExtSel']) << 8
- configcode = int(jd['ConfigCode'], 0) if 'ConfigCode' in jd else None
self.name = jd['EventName'].lower() if 'EventName' in jd else None
self.topic = ''
self.compat = jd.get('Compat')
@@ -317,7 +311,15 @@ class JsonEvent:
if precise and self.desc and '(Precise Event)' not in self.desc:
extra_desc += ' (Must be precise)' if precise == '2' else (' (Precise '
'event)')
- event = f'config={llx(configcode)}' if configcode is not None else f'event={llx(eventcode)}'
+ eventcode = None
+ if 'EventCode' in jd:
+ eventcode = int(jd['EventCode'].split(',', 1)[0], 0)
+ if 'ExtSel' in jd:
+ if eventcode is None:
+ eventcode = int(jd['ExtSel']) << 8
+ else:
+ eventcode |= int(jd['ExtSel']) << 8
+ event = f'event={llx(eventcode)}' if eventcode is not None else None
event_fields = [
('AnyThread', 'any='),
('PortMask', 'ch_mask='),
@@ -327,10 +329,13 @@ class JsonEvent:
('Invert', 'inv='),
('SampleAfterValue', 'period='),
('UMask', 'umask='),
+ ('ConfigCode', 'config='),
+ ('Type', 'type='),
+ ('EventIdCode', 'eventid='),
]
for key, value in event_fields:
if key in jd and jd[key] != '0':
- event += ',' + value + jd[key]
+ event = event + ',' + value + jd[key] if event is not None else value + jd[key]
if filter:
event += f',{filter}'
if msr:
--
1.8.3.1
On 07/08/2023 08:51, Jing Zhang wrote:
> The usual event descriptions are "event=xxx" or "config=xxx", while the
> event descriptions of CMN are "type=xxx, eventid=xxx" or more complex.
>
> $cat /sys/bus/event_source/devices/arm_cmn_0/events/hnf_cache_fill
> type=0x5,eventid=0x3
>
> When adding aliases for events described as "event=xxx" or "config=xxx",
> EventCode or ConfigCode can be used in the JSON files to describe the
> events. But "eventid=xxx, type=xxx" cannot be supported at present.
>
> If EventCode and ConfigCode is not added in the alias JSON file, the
> event description will add "event=0" by default. So, even if the event
> field is added to supplement "eventid=xxx" and "type=xxx", the final
> parsing result will be "event=0, eventid=xxx, type=xxx".
>
> Therefore, when EventCode and ConfigCode are missing in JSON, "event=0" is
> no longer added by default. EventIdCode and Type are added to the event
> field, and ConfigCode is moved into the event_field array which can also
> guarantee its original function.
>
> Signed-off-by: Jing Zhang <[email protected]>
I'll let Ian check this change as he is more familiar with this code.
Thanks
在 2023/8/7 下午5:24, John Garry 写道:
> On 07/08/2023 08:51, Jing Zhang wrote:
>> The usual event descriptions are "event=xxx" or "config=xxx", while the
>> event descriptions of CMN are "type=xxx, eventid=xxx" or more complex.
>>
>> $cat /sys/bus/event_source/devices/arm_cmn_0/events/hnf_cache_fill
>> type=0x5,eventid=0x3
>>
>> When adding aliases for events described as "event=xxx" or "config=xxx",
>> EventCode or ConfigCode can be used in the JSON files to describe the
>> events. But "eventid=xxx, type=xxx" cannot be supported at present.
>>
>> If EventCode and ConfigCode is not added in the alias JSON file, the
>> event description will add "event=0" by default. So, even if the event
>> field is added to supplement "eventid=xxx" and "type=xxx", the final
>> parsing result will be "event=0, eventid=xxx, type=xxx".
>>
>> Therefore, when EventCode and ConfigCode are missing in JSON, "event=0" is
>> no longer added by default. EventIdCode and Type are added to the event
>> field, and ConfigCode is moved into the event_field array which can also
>> guarantee its original function.
>>
>> Signed-off-by: Jing Zhang <[email protected]>
>
> I'll let Ian check this change as he is more familiar with this code.
>
Ok, thanks!
在 2023/8/7 下午5:24, John Garry 写道:
> On 07/08/2023 08:51, Jing Zhang wrote:
>> The usual event descriptions are "event=xxx" or "config=xxx", while the
>> event descriptions of CMN are "type=xxx, eventid=xxx" or more complex.
>>
>> $cat /sys/bus/event_source/devices/arm_cmn_0/events/hnf_cache_fill
>> type=0x5,eventid=0x3
>>
>> When adding aliases for events described as "event=xxx" or "config=xxx",
>> EventCode or ConfigCode can be used in the JSON files to describe the
>> events. But "eventid=xxx, type=xxx" cannot be supported at present.
>>
>> If EventCode and ConfigCode is not added in the alias JSON file, the
>> event description will add "event=0" by default. So, even if the event
>> field is added to supplement "eventid=xxx" and "type=xxx", the final
>> parsing result will be "event=0, eventid=xxx, type=xxx".
>>
>> Therefore, when EventCode and ConfigCode are missing in JSON, "event=0" is
>> no longer added by default. EventIdCode and Type are added to the event
>> field, and ConfigCode is moved into the event_field array which can also
>> guarantee its original function.
>>
>> Signed-off-by: Jing Zhang <[email protected]>
>
> I'll let Ian check this change as he is more familiar with this code.
>
Hi Ian,
Could you please help to review this change？:)
Thanks,
Jing
在 2023/8/15 上午6:31, Ian Rogers 写道:
> On Mon, Aug 7, 2023 at 12:51 AM Jing Zhang <[email protected]> wrote:
>>
>> The usual event descriptions are "event=xxx" or "config=xxx", while the
>> event descriptions of CMN are "type=xxx, eventid=xxx" or more complex.
>
> I found this difficult to read in relation to the code. Perhaps:
>
> The previous code assumes an event has either an "event=" or "config"
> field at the beginning. For CMN neither of these may be present, as an
> event is typically "type=xx,eventid=xxx".
>
Thank you for providing a more accurate and readable description.
I was indeed struggling with how to describe the problem more accurately before.
> I think the use of the name "type" here is unfortunate. It conflicts
> with the PMU's type as defined in perf_event_attr.
>
I agree, but it would require modifying the driver, which is not currently being
considered. In the meantime, I can describe the event_field as ('NodeType', 'type=').
> In general I think the jevents.py code needs improving, the
> event_fields dictionary is convoluted, we shouldn't be afraid to
> change the event json for example to get rid of things like ExtSel, we
> should really ensure the formats in the events are valid for the PMU
> they are for.
>
>> $cat /sys/bus/event_source/devices/arm_cmn_0/events/hnf_cache_fill
>> type=0x5,eventid=0x3
>>
>> When adding aliases for events described as "event=xxx" or "config=xxx",
>> EventCode or ConfigCode can be used in the JSON files to describe the
>> events. But "eventid=xxx, type=xxx" cannot be supported at present.
>>
>> If EventCode and ConfigCode is not added in the alias JSON file, the
>> event description will add "event=0" by default. So, even if the event
>> field is added to supplement "eventid=xxx" and "type=xxx", the final
>> parsing result will be "event=0, eventid=xxx, type=xxx".
>>
>> Therefore, when EventCode and ConfigCode are missing in JSON, "event=0" is
>> no longer added by default. EventIdCode and Type are added to the event
>> field, and ConfigCode is moved into the event_field array which can also
>> guarantee its original function.
>
> A useful test can be to build with JEVENTS_ARCH=all and confirm the
> before and after change generated pmu-events.c is the same.
>
Okay, I will test it right away.
>> Signed-off-by: Jing Zhang <[email protected]>
>> ---
>> tools/perf/pmu-events/jevents.py | 21 +++++++++++++--------
>> 1 file changed, 13 insertions(+), 8 deletions(-)
>>
>> diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
>> index f57a8f2..9c0f63a 100755
>> --- a/tools/perf/pmu-events/jevents.py
>> +++ b/tools/perf/pmu-events/jevents.py
>> @@ -275,12 +275,6 @@ class JsonEvent:
>> }
>> return table[unit] if unit in table else f'uncore_{unit.lower()}'
>>
>> - eventcode = 0
>> - if 'EventCode' in jd:
>> - eventcode = int(jd['EventCode'].split(',', 1)[0], 0)
>> - if 'ExtSel' in jd:
>> - eventcode |= int(jd['ExtSel']) << 8
>> - configcode = int(jd['ConfigCode'], 0) if 'ConfigCode' in jd else None
>> self.name = jd['EventName'].lower() if 'EventName' in jd else None
>> self.topic = ''
>> self.compat = jd.get('Compat')
>> @@ -317,7 +311,15 @@ class JsonEvent:
>> if precise and self.desc and '(Precise Event)' not in self.desc:
>> extra_desc += ' (Must be precise)' if precise == '2' else (' (Precise '
>> 'event)')
>> - event = f'config={llx(configcode)}' if configcode is not None else f'event={llx(eventcode)}'
>> + eventcode = None
>> + if 'EventCode' in jd:
>> + eventcode = int(jd['EventCode'].split(',', 1)[0], 0)
>> + if 'ExtSel' in jd:
>> + if eventcode is None:
>> + eventcode = int(jd['ExtSel']) << 8
>> + else:
>> + eventcode |= int(jd['ExtSel']) << 8
>> + event = f'event={llx(eventcode)}' if eventcode is not None else None
>> event_fields = [
>> ('AnyThread', 'any='),
>> ('PortMask', 'ch_mask='),
>> @@ -327,10 +329,13 @@ class JsonEvent:
>> ('Invert', 'inv='),
>> ('SampleAfterValue', 'period='),
>> ('UMask', 'umask='),
>> + ('ConfigCode', 'config='),
>
> This loses the int and potential base conversion of ConfigCode.
> Clearly the code was taking care to maintain this behavior so I
> suspect this change has broken something. JEVENTS_ARCH=all should
> reveal the answer.
>
You are correct, I compared the generated pmu-events.c files before and after,
and they are indeed different, with before:config=0x5 vs after:config=0x05. I will keep the
original way of handling ConfigCode in the next version.
>> + ('Type', 'type='),
>> + ('EventIdCode', 'eventid='),
>> ]
>> for key, value in event_fields:
>> if key in jd and jd[key] != '0':
>> - event += ',' + value + jd[key]
>> + event = event + ',' + value + jd[key] if event is not None else value + jd[key]
>
> Perhaps initialize event above to the empty string then:
>
> if key in jd and jd[key] != '0':
> if event:
> event += ','
> event += value + jd[key]
>
If the event is None, the statement event += value + jd[key] would result in an error.
So, maybe I can use the following way:
if event:
event += ',' + value + jd[key]
else:
event = value + jd[key]
Thanks,
Jing