Re: [PATCH v2 2/5] perf vendor events: Add JSON metrics for CMN-700

[Jing Zhang <renyu.zj@linux.alibaba.com>]

在 2023/5/1 下午8:27, John Garry 写道:
> On 24/04/2023 10:44, Jing Zhang wrote:
>> Add JSON metrics for CMN-700. Currently just add part of CMN-700 PMU
>> metrics which are general and compatible for any SoC.
>>
>> Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com>
>> ---
>>   .../arch/arm64/arm/cmn700/sys/metrics.json         | 74 ++++++++++++++++++++++
>>   tools/perf/pmu-events/jevents.py                   |  1 +
>>   2 files changed, 75 insertions(+)
>>   create mode 100644 tools/perf/pmu-events/arch/arm64/arm/cmn700/sys/metrics.json
>>
>> diff --git a/tools/perf/pmu-events/arch/arm64/arm/cmn700/sys/metrics.json b/tools/perf/pmu-events/arch/arm64/arm/cmn700/sys/metrics.json
>> new file mode 100644
>> index 0000000..1577d86
>> --- /dev/null
>> +++ b/tools/perf/pmu-events/arch/arm64/arm/cmn700/sys/metrics.json
> 
> I suppose putting these here is ok. Are all these events a must for a CMN-700 implementation? Is there potential for impdef event in some implementations?
> 

Yes, according it's driver code, all these events are a must for a CMN-700 implementation even in different revision. But if the revision is added
as an identifier, then these metrics need to be defined for each revision. This doesn't seem lean enough. So I also need to consider how to
match the common metrics of multiple rivisions. Could you provide some suggestions?

> Previously we considered ARM PMCG events would be put in a per-SoC sys folder only (and not in perf/pmu-events/arch/arm64/arm)
> 
>> @@ -0,0 +1,74 @@
>> +[
>> +    {
>> +        "MetricName": "slc_miss_rate",
>> +        "BriefDescription": "The system level cache miss rate include.",
>> +        "MetricGroup": "cmn700",
>> +        "MetricExpr": "hnf_cache_miss / hnf_slc_sf_cache_access",
>> +        "ScaleUnit": "100%",
>> +        "Unit": "arm_cmn",
>> +        "Compat": "cmn700"
>> +    },
>> +    {
>> +        "MetricName": "hnf_message_retry_rate",
>> +        "BriefDescription": "HN-F message retry rate indicates whether a lack of credits is causing the bottlenecks.",
>> +        "MetricGroup": "cmn700",
>> +        "MetricExpr": "hnf_pocq_retry / hnf_pocq_reqs_recvd",
>> +        "ScaleUnit": "100%",
>> +        "Unit": "arm_cmn",
>> +        "Compat": "cmn700"
>> +    },
>> +    {
>> +        "MetricName": "sf_hit_rate",
>> +        "BriefDescription": "Snoop filter hit rate can be used to measure the Snoop Filter efficiency.",
>> +        "MetricGroup": "cmn700",
>> +        "MetricExpr": "hnf_sf_hit / hnf_slc_sf_cache_access",
>> +        "ScaleUnit": "100%",
>> +        "Unit": "arm_cmn",
>> +        "Compat": "cmn700"
>> +    },
>> +    {
>> +        "MetricName": "mc_message_retry_rate",
>> +        "BriefDescription": "The memory controller request retries rate indicates whether the memory controller is the bottleneck.",
>> +        "MetricGroup": "cmn700",
>> +        "MetricExpr": "hnf_mc_retries / hnf_mc_reqs",
>> +        "ScaleUnit": "100%",
>> +        "Unit": "arm_cmn",
>> +        "Compat": "cmn700"
>> +    },
>> +    {
>> +        "MetricName": "rni_actual_read_bandwidth.all",
>> +        "BriefDescription": "This event measure the actual bandwidth(MB/sec) that RN-I bridge sends to the interconnect.",
>> +        "MetricGroup": "cmn700",
>> +        "MetricExpr": "rnid_rxdat_flits * 32 / 1e6 / duration_time",
>> +        "ScaleUnit": "1MB/s",
>> +        "Unit": "arm_cmn",
>> +        "Compat": "cmn700"
>> +    },
>> +    {
>> +        "MetricName": "rni_actual_write_bandwidth.all",
>> +        "BriefDescription": "This event measures the actual write bandwidth(MB/sec) at RN-I bridges.",
>> +        "MetricGroup": "cmn700",
>> +        "MetricExpr": "rnid_txdat_flits * 32 / 1e6 / duration_time",
>> +        "ScaleUnit": "1MB/s",
>> +        "Unit": "arm_cmn",
>> +        "Compat": "cmn700"
>> +    },
>> +    {
>> +        "MetricName": "rni_retry_rate",
>> +        "BriefDescription": "RN-I bridge retry rate indicates whether the memory controller is the bottleneck.",
>> +        "MetricGroup": "cmn700",
>> +        "MetricExpr": "rnid_txreq_flits_retried / rnid_txreq_flits_total",
>> +        "ScaleUnit": "100%",
>> +        "Unit": "arm_cmn",
>> +        "Compat": "cmn700"
>> +    },
>> +    {
>> +        "MetricName": "sbsx_actual_write_bandwidth.all",
>> +        "BriefDescription": "sbsx actual write bandwidth(MB/sec).",
>> +        "MetricGroup": "cmn700",
>> +        "MetricExpr": "sbsx_txdat_flitv * 32 / 1e6 / duration_time",
>> +        "ScaleUnit": "1MB/s",
>> +        "Unit": "arm_cmn",
>> +        "Compat": "cmn700"
>> +    }
>> +]
>> diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
>> index 2bcd07c..7cff2c6 100755
>> --- a/tools/perf/pmu-events/jevents.py
>> +++ b/tools/perf/pmu-events/jevents.py
>> @@ -256,6 +256,7 @@ class JsonEvent:
>>             'DFPMC': 'amd_df',
>>             'cpu_core': 'cpu_core',
>>             'cpu_atom': 'cpu_atom',
>> +          'arm_cmn': 'arm_cmn',
>>         }
>>         return table[unit] if unit in table else f'uncore_{unit.lower()}'
>>