Re: [External] : [RFC PATCH v2 1/6] perf vendor events arm64: Add topdown L1 metrics for neoverse-n2

[John Garry <john.g.garry@oracle.com>]

On 15/11/2022 08:43, Jing Zhang wrote:
> I didn't find out how to put the metric as an arch std event, it would be best if you could provide me with an example in the upstream code,
> thank you very much.

As things stand, I don't think it's supported. We only support regular 
events for std arch events (and not metrics).

However we could expand support for metrics.

For the example of hip08 and FRONTEND_BOUND, we would have:

--->8---

diff --git 
a/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json 
b/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json
index 6443a061e22a..5b1ca45224de 100644
--- a/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json
+++ b/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json
@@ -1,10 +1,6 @@
  [
      {
-        "MetricExpr": "FETCH_BUBBLE / (4 * CPU_CYCLES)",
-        "PublicDescription": "Frontend bound L1 topdown metric",
-        "BriefDescription": "Frontend bound L1 topdown metric",
-        "MetricGroup": "TopDownL1",
-        "MetricName": "frontend_bound"
+        "ArchStdEvent": "FRONTEND_BOUND"
      },
      {
          "MetricExpr": "(INST_SPEC - INST_RETIRED) / (4 * CPU_CYCLES)",
diff --git a/tools/perf/pmu-events/arch/arm64/sbsa.json 
b/tools/perf/pmu-events/arch/arm64/sbsa.json
new file mode 100644
index 000000000000..10b9c0cccc40
--- /dev/null
+++ b/tools/perf/pmu-events/arch/arm64/sbsa.json
@@ -0,0 +1,9 @@
+[
+    {
+        "MetricExpr": "FETCH_BUBBLE / (4 * CPU_CYCLES)",
+        "PublicDescription": "Frontend bound L1 topdown metric",
+        "BriefDescription": "Frontend bound L1 topdown metric",
+        "MetricGroup": "TopDownL1",
+        "MetricName": "FRONTEND_BOUND"
+    }
+]
diff --git a/tools/perf/pmu-events/jevents.py 
b/tools/perf/pmu-events/jevents.py
index 0daa3e007528..77049853c0bf 100755
--- a/tools/perf/pmu-events/jevents.py
+++ b/tools/perf/pmu-events/jevents.py
@@ -352,6 +352,8 @@ def preprocess_arch_std_files(archpath: str) -> None:
        for event in read_json_events(item.path, topic=''):
          if event.name:
            _arch_std_events[event.name.lower()] = event
+        if event.metric_name:
+          _arch_std_events[event.metric_name.lower()] = event


  def print_events_table_prefix(tblname: str) -> None:
-- 
2.35.3

Note that this is for illustration only. The frontend bound metric for 
hip08 does not really belong in sbsa.json as it does not adhere to that 
spec. But for platforms which do adhere to the spec, we could pick up 
the metrics values from sbsa.json (or whatever we want to call it).

> 
>>> However, due to the wrong count of stall_slot and stall_slot_frontend
>>> in neoverse-n2, the real stall_slot and real stall_slot_frontend need
>>> to subtract cpu_cycles, so when calculating the topdownL1 metrics,
>>> stall_slot and stall_slot_frontend are corrected.
>> Is there a reference to this? It would be indeed useful to pass a link to the n2 doc as these metrics are not part of the arm64 arm. At least I assume that they are not there.
>>
> You are right, I need to add a doc link. ARM has released the n2 ERRATA document about the incorrect count of stall_slot and stall_slot_frontend,
> and provides a workaround to get the correct value.
> Link:https://urldefense.com/v3/__https://developer.arm.com/documentation/SDEN1982442/1200/?lang=en__;!!ACWV5N9M2RV99hQ!I3rCI-RcuDmpfAiWhA_SAxRrq1hCyhA9am8YmrwizljPz9X_G4H_odm_4aRgRo8VswDeC3TATbylxf_vhAJhWbJrlw$  
> 

Note that std arch events support is such that we can still overwrite 
individual std values in the platform-specific json (or at least we used 
to be able to - I have not checked recently). So for n2 case of 
stall_slot, we could use std arch events in the n2 json but overwrite 
the metric expression, like:

+++ b/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json
@@ -1,10 +1,6 @@
  [
      {
          "ArchStdEvent": "FRONTEND_BOUND"
	 "MetricExpr": <insert n2 specific expression>",
      },

Thanks,
John