[PATCH RFC 0/4] Add JSON metrics for arm CMN and Yitian710 DDR

[PATCH RFC 0/4] Add JSON metrics for arm CMN and Yitian710 DDR

[Jing Zhang]

Hi all,

I add an identifier sysfs file for the yitian710 SoC DDR and arm CMN to
allow userspace to identify the specific implementation of the device,
so that the perf tool can match the corresponding uncore events and
metrics through the identifier. Then added several general CMN700 metrics
and yitian710 soc DDR metrics.

Since the eventid of cmn700 events is different from other events, it
can't be specified by "EventCode" or "ConfigCode", so in the cmn.json
file of cmn700, no "EventCode" and "ConfigCode" are added for these
events. For example, the eventid of "arm_cmn_0/hnf_sf_hit is/":
cat /sys/bus/event_source/devices/arm_cmn_0/events/hnf_sf_hit
type=0x5,eventid=0x6

In addition, both cmn700 and ddr PMU can count the information in a die,
but the same SoC can also be configured with different numbers of dies,
so it is dificult to design a general expression to obtain metrics in
different dies. The current yitian710 ddr bandwidth metric describes the
sum of all dies bandwidth. I would like to ask you, is there any general
expression can obtain metrics for die? Add an option to specify die?

Thanks,
Jing

Jing Zhang (4):
  driver/perf: Add identifier sysfs file for CMN
  perf vendor events: Add JSON metrics for cmn700
  driver/perf: Add identifier sysfs file for Yitian 710 DDR
  perf vendor events: Add JSON metrics for Yitian 710 DDR

 drivers/perf/alibaba_uncore_drw_pmu.c              |  27 ++
 drivers/perf/arm-cmn.c                             |  43 +++
 .../pmu-events/arch/arm64/arm/cmn700/sys/cmn.json  | 188 +++++++++++
 .../arch/arm64/arm/cmn700/sys/metrics.json         |  74 ++++
 .../arm64/freescale/yitian710/sys/ali_drw.json     | 373 +++++++++++++++++++++
 .../arm64/freescale/yitian710/sys/metrics.json     |  20 ++
 tools/perf/pmu-events/jevents.py                   |   2 +
 7 files changed, 727 insertions(+)
 create mode 100644 tools/perf/pmu-events/arch/arm64/arm/cmn700/sys/cmn.json
 create mode 100644 tools/perf/pmu-events/arch/arm64/arm/cmn700/sys/metrics.json
 create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/ali_drw.json
 create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/metrics.json

-- 
1.8.3.1

[PATCH RFC 1/4] driver/perf: Add identifier sysfs file for CMN

[Jing Zhang]

To allow userspace to identify the specific implementation of the device,
add an "identifier" sysfs file.

The perf tool can match the arm CMN metric through the identifier.

Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com>
---
 drivers/perf/arm-cmn.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/drivers/perf/arm-cmn.c b/drivers/perf/arm-cmn.c
index c968986..0c138ad 100644
--- a/drivers/perf/arm-cmn.c
+++ b/drivers/perf/arm-cmn.c
@@ -1168,10 +1168,53 @@ static ssize_t arm_cmn_cpumask_show(struct device *dev,
 	.attrs = arm_cmn_cpumask_attrs,
 };
 
+static ssize_t arm_cmn_identifier_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct arm_cmn *cmn = to_cmn(dev_get_drvdata(dev));
+	if (cmn->model == CMN700) {
+		return sysfs_emit(buf, "%s\n", "CMN700");
+	}
+	else if (cmn->model == CMN650) {
+		return sysfs_emit(buf, "%s\n", "CMN650");
+	}
+	else if (cmn->model == CMN600) {
+		return sysfs_emit(buf, "%s\n", "CMN600");
+	}
+	else if (cmn->model == CI700) {
+		return sysfs_emit(buf, "%s\n", "CI700");
+	}
+	return sysfs_emit(buf, "%s\n", "UNKNOWN");
+}
+
+static umode_t arm_cmn_identifier_attr_visible(struct kobject *kobj,
+		struct attribute *attr, int n)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct arm_cmn *cmn = to_cmn(dev_get_drvdata(dev));
+	if (cmn->model <= 0)
+		return 0;
+	return attr->mode;
+};
+
+static struct device_attribute arm_cmn_identifier_attr =
+__ATTR(identifier, 0444, arm_cmn_identifier_show, NULL);
+
+static struct attribute *arm_cmn_identifier_attrs[] = {
+	&arm_cmn_identifier_attr.attr,
+	NULL,
+};
+
+static struct attribute_group arm_cmn_identifier_attr_group = {
+	.attrs = arm_cmn_identifier_attrs,
+	.is_visible = arm_cmn_identifier_attr_visible,
+};
+
 static const struct attribute_group *arm_cmn_attr_groups[] = {
 	&arm_cmn_event_attrs_group,
 	&arm_cmn_format_attrs_group,
 	&arm_cmn_cpumask_attr_group,
+	&arm_cmn_identifier_attr_group,
 	NULL
 };
 
-- 
1.8.3.1

[PATCH RFC 2/4] perf vendor events: Add JSON metrics for cmn700

[Jing Zhang]

Add JSON metrics for cmn700. Currently just add part of cmn700 PMU
events and metrics which are general and compatible for any SoC.

Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com>
---
 .../pmu-events/arch/arm64/arm/cmn700/sys/cmn.json  | 188 +++++++++++++++++++++
 .../arch/arm64/arm/cmn700/sys/metrics.json         |  74 ++++++++
 tools/perf/pmu-events/jevents.py                   |   1 +
 3 files changed, 263 insertions(+)
 create mode 100644 tools/perf/pmu-events/arch/arm64/arm/cmn700/sys/cmn.json
 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/cmn.json b/tools/perf/pmu-events/arch/arm64/arm/cmn700/sys/cmn.json
new file mode 100644
index 0000000..a060d1d
--- /dev/null
+++ b/tools/perf/pmu-events/arch/arm64/arm/cmn700/sys/cmn.json
@@ -0,0 +1,188 @@
+[
+	{
+		"EventName": "hnf_cache_miss",
+		"BriefDescription": "Counts total cache misses in first lookup result (high priority)",
+		"Unit": "cmn700",
+		"Compat": "CMN700"
+	},
+	{
+		"EventName": "hnf_slc_sf_cache_access",
+		"BriefDescription": "Counts number of cache accesses in first access (high priority)",
+		"Unit": "cmn700",
+		"Compat": "CMN700"
+	},
+	{
+		"EventName": "hnf_cache_fill",
+		"BriefDescription": "Counts total allocations in HN SLC (all cache line allocations to SLC)",
+		"Unit": "cmn700",
+		"Compat": "CMN700"
+	},
+	{
+		"EventName": "hnf_pocq_retry",
+		"BriefDescription": "Counts number of retried requests",
+		"Unit": "cmn700",
+		"Compat": "CMN700"
+	},
+	{
+		"EventName": "hnf_pocq_reqs_recvd",
+		"BriefDescription": "Counts number of requests that HN receives",
+		"Unit": "cmn700",
+		"Compat": "CMN700"
+	},
+	{
+		"EventName": "hnf_sf_hit",
+		"BriefDescription": "Counts number of SF hits",
+		"Unit": "cmn700",
+		"Compat": "CMN700"
+	},
+	{
+		"EventName": "hnf_sf_evictions",
+		"BriefDescription": "Counts number of SF eviction cache invalidations initiated",
+		"Unit": "cmn700",
+		"Compat": "CMN700"
+	},
+	{
+		"EventName": "hnf_dir_snoops_sent",
+		"BriefDescription": "Counts number of directed snoops sent (not including SF back invalidation)",
+		"Unit": "cmn700",
+		"Compat": "CMN700"
+	},
+	{
+		"EventName": "hnf_brd_snoops_sent",
+		"BriefDescription": "Counts number of multicast snoops sent (not including SF back invalidation)",
+		"Unit": "cmn700",
+		"Compat": "CMN700"
+	},
+	{
+		"EventName": "hnf_mc_retries",
+		"BriefDescription": "Counts number of retried transactions by the MC",
+		"Unit": "cmn700",
+		"Compat": "CMN700"
+	},
+	{
+		"EventName": "hnf_mc_reqs",
+		"BriefDescription": "Counts number of requests that are sent to MC",
+		"Unit": "cmn700",
+		"Compat": "CMN700"
+	},
+	{
+		"EventName": "hnf_qos_hh_retry",
+		"BriefDescription": "Counts number of times a HighHigh priority request is protocolretried at the HN‑F",
+		"Unit": "cmn700",
+		"Compat": "CMN700"
+	},
+	{
+		"EventName": "rnid_s0_rdata_beats",
+		"BriefDescription": "Number of RData beats (RVALID and RREADY) dispatched on port 0. This event measures the read bandwidth, including CMO responses.",
+		"Unit": "cmn700",
+		"Compat": "CMN700"
+	},
+	{
+		"EventName": "rnid_s1_rdata_beats",
+		"BriefDescription": "Number of RData beats (RVALID and RREADY) dispatched on port 1. This event measures the read bandwidth, including CMO responses.",
+		"Unit": "cmn700",
+		"Compat": "CMN700"
+	},
+	{
+		"EventName": "rnid_s2_rdata_beats",
+		"BriefDescription": "Number of RData beats (RVALID and RREADY) dispatched on port 2. This event measures the read bandwidth, including CMO responses.",
+		"Unit": "cmn700",
+		"Compat": "CMN700"
+	},
+	{
+		"EventName": "rnid_rxdat_flits",
+		"BriefDescription": "Number of RXDAT flits received. This event measures the true read data bandwidth, excluding CMOs.",
+		"Unit": "cmn700",
+		"Compat": "CMN700"
+	},
+	{
+		"EventName": "rnid_txdat_flits",
+		"BriefDescription": "Number of TXDAT flits dispatched. This event measures the write bandwidth.",
+		"Unit": "cmn700",
+		"Compat": "CMN700"
+	},
+	{
+		"EventName": "rnid_txreq_flits_total",
+		"BriefDescription": "Number of TXREQ flits dispatched. This event measures the total request bandwidth.",
+		"Unit": "cmn700",
+		"Compat": "CMN700"
+	},
+	{
+		"EventName": "rnid_txreq_flits_retried",
+		"BriefDescription": "Number of retried TXREQ flits dispatched. This event measures the retry rate.",
+		"Unit": "cmn700",
+		"Compat": "CMN700"
+	},
+	{
+		"EventName": "sbsx_txdat_flitv",
+		"BriefDescription": "Number of TXDAT flits dispatched from XP to SBSX. This event is a measure of the write bandwidth.",
+		"Unit": "cmn700",
+		"Compat": "CMN700"
+	},
+	{
+		"EventName": "sbsx_txrsp_retryack",
+		"BriefDescription": "Number of RXREQ flits dispatched. This event is a measure of the retry rate.",
+		"Unit": "cmn700",
+		"Compat": "CMN700"
+	},
+	{
+		"EventName": "sbsx_arvalid_no_arready",
+		"BriefDescription": "Number of cycles the SBSX bridge is stalled because of backpressure on AR channel.",
+		"Unit": "cmn700",
+		"Compat": "CMN700"
+	},
+	{
+		"EventName": "sbsx_awvalid_no_awready",
+		"BriefDescription": "Number of cycles the SBSX bridge is stalled because of backpressure on AW channel.",
+		"Unit": "cmn700",
+		"Compat": "CMN700"
+	},
+	{
+		"EventName": "sbsx_wvalid_no_wready",
+		"BriefDescription": "Number of cycles the SBSX bridge is stalled because of backpressure on W channel.",
+		"Unit": "cmn700",
+		"Compat": "CMN700"
+	},
+	{
+		"EventName": "hni_txdat_stall",
+		"BriefDescription": "TXDAT valid but no link credit available.",
+		"Unit": "cmn700",
+		"Compat": "CMN700"
+	},
+	{
+		"EventName": "hni_txrsp_retryack",
+		"BriefDescription": "Number of RXREQ flits dispatched. This event is a measure of the retry rate.",
+		"Unit": "cmn700",
+		"Compat": "CMN700"
+	},
+	{
+		"EventName": "hni_arvalid_no_arready",
+		"BriefDescription": "Number of cycles the HN-I bridge is stalled because of backpressure on AR channel.",
+		"Unit": "cmn700",
+		"Compat": "CMN700"
+	},
+	{
+		"EventName": "hni_awvalid_no_awready",
+		"BriefDescription": "Number of cycles the HN-I bridge is stalled because of backpressure on AW channel.",
+		"Unit": "cmn700",
+		"Compat": "CMN700"
+	},
+	{
+		"EventName": "hni_wvalid_no_wready",
+		"BriefDescription": "Number of cycles the HN-I bridge is stalled because of backpressure on W channel.",
+		"Unit": "cmn700",
+		"Compat": "CMN700"
+	},
+	{
+		"EventName": "hni_arready_no_arvalid",
+		"BriefDescription": "Number of cycles the AR channel is waiting for new requests from HN-I bridge.",
+		"Unit": "cmn700",
+		"Compat": "CMN700"
+	},
+	{
+		"EventName": "hni_awready_no_awvalid",
+		"BriefDescription": "Number of cycles the AW channel is waiting for new requests from HN-I bridge.",
+		"Unit": "cmn700",
+		"Compat": "CMN700"
+	}
+]
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..430567b
--- /dev/null
+++ b/tools/perf/pmu-events/arch/arm64/arm/cmn700/sys/metrics.json
@@ -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": "cmn700",
+		"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": "cmn700",
+		"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": "cmn700",
+		"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": "cmn700",
+		"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": "cmn700",
+		"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": "cmn700",
+		"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": "cmn700",
+		"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": "cmn700",
+		"Compat": "CMN700"
+	}
+]
diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
index 2bcd07c..20ed492 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',
+          'cmn700': 'cmn700',
       }
       return table[unit] if unit in table else f'uncore_{unit.lower()}'
 
-- 
1.8.3.1

[PATCH RFC 3/4] driver/perf: Add identifier sysfs file for Yitian 710 DDR

[Jing Zhang]

To allow userspace to identify the specific implementation of the device,
add an "identifier" sysfs file.

The perf tool can match the Yitian 710 DDR metric through the identifier.

Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com>
---
 drivers/perf/alibaba_uncore_drw_pmu.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/perf/alibaba_uncore_drw_pmu.c b/drivers/perf/alibaba_uncore_drw_pmu.c
index a7689fe..6639a57 100644
--- a/drivers/perf/alibaba_uncore_drw_pmu.c
+++ b/drivers/perf/alibaba_uncore_drw_pmu.c
@@ -236,10 +236,37 @@ static ssize_t ali_drw_pmu_cpumask_show(struct device *dev,
 	.attrs = ali_drw_pmu_cpumask_attrs,
 };
 
+static ssize_t ali_drw_pmu_identifier_show(struct device *dev,
+					struct device_attribute *attr,
+					char *page)
+{
+	return sysfs_emit(page, "%s\n", "ali_drw_yitian710");
+}
+
+static umode_t ali_drw_pmu_identifier_attr_visible(struct kobject *kobj,
+						struct attribute *attr, int n)
+{
+	return attr->mode;
+}
+
+static struct device_attribute ali_drw_pmu_identifier_attr =
+	__ATTR(identifier, 0444, ali_drw_pmu_identifier_show, NULL);
+
+static struct attribute *ali_drw_pmu_identifier_attrs[] = {
+	&ali_drw_pmu_identifier_attr.attr,
+	NULL,
+};
+
+static const struct attribute_group ali_drw_pmu_identifier_attr_group = {
+	.attrs = ali_drw_pmu_identifier_attrs,
+	.is_visible = ali_drw_pmu_identifier_attr_visible,
+};
+
 static const struct attribute_group *ali_drw_pmu_attr_groups[] = {
 	&ali_drw_pmu_events_attr_group,
 	&ali_drw_pmu_cpumask_attr_group,
 	&ali_drw_pmu_format_group,
+	&ali_drw_pmu_identifier_attr_group,
 	NULL,
 };
 
-- 
1.8.3.1

[PATCH RFC 4/4] perf vendor events: Add JSON metrics for Yitian 710 DDR

[Jing Zhang]

Add JSON metrics for T-HEAD Yitian 710 SoC DDR.

Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com>
---
 .../arm64/freescale/yitian710/sys/ali_drw.json     | 373 +++++++++++++++++++++
 .../arm64/freescale/yitian710/sys/metrics.json     |  20 ++
 tools/perf/pmu-events/jevents.py                   |   1 +
 3 files changed, 394 insertions(+)
 create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/ali_drw.json
 create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/metrics.json

diff --git a/tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/ali_drw.json b/tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/ali_drw.json
new file mode 100644
index 0000000..cb8694b
--- /dev/null
+++ b/tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/ali_drw.json
@@ -0,0 +1,373 @@
+[
+	{
+		"BriefDescription": "A Write or Read Op at HIF interface. 64B",
+		"ConfigCode": "0x0",
+		"EventName": "hif_rd_or_wr",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A Write Op at HIF interface. 64B",
+		"ConfigCode": "0x1",
+		"EventName": "hif_wr",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710" 
+	},
+	{
+		"BriefDescription": "A Read Op at HIF interface. 64B",
+		"ConfigCode": "0x2",
+		"EventName": "hif_rd",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A Read-Modify-Write Op at HIF interface. 64B",
+		"ConfigCode": "0x3",
+		"EventName": "hif_rmw",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A high priority Read at HIF interface. 64B",
+		"ConfigCode": "0x4",
+		"EventName": "hif_hi_pri_rd",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A write data cycle at DFI interface (to DRAM)",
+		"ConfigCode": "0x7",
+		"EventName": "dfi_wr_data_cycles",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A read data cycle at DFI interface (to DRAM).",
+		"ConfigCode": "0x8",
+		"EventName": "dfi_rd_data_cycles",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A high priority read becomes critical.",
+		"ConfigCode": "0x9",
+		"EventName": "hpr_xact_when_critical",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"  
+	},
+	{
+		"BriefDescription": "A low priority read becomes critical.",
+		"ConfigCode": "0xA",
+		"EventName": "lpr_xact_when_critical",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A write becomes critical.",
+		"ConfigCode": "0xB",
+		"EventName": "wr_xact_when_critical",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "An Activate (ACT) command to DRAM.",
+		"ConfigCode": "0xC",
+		"EventName": "op_is_activate",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A Read or Write CAS command to DRAM.",
+		"ConfigCode": "0xD",
+		"EventName": "op_is_rd_or_wr",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "An ACT command for read to DRAM.",
+		"ConfigCode": "0xE",
+		"EventName": "op_is_rd_activate",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A Read CAS command to DRAM.",
+		"ConfigCode": "0xF",
+		"EventName": "op_is_rd",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A Write CAS command to DRAM.",
+		"ConfigCode": "0x10",
+		"EventName": "op_is_wr",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A Masked Write command to DRAM.",
+		"ConfigCode": "0x11",
+		"EventName": "op_is_mwr",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A Precharge (PRE) command to DRAM.",
+		"ConfigCode": "0x12",
+		"EventName": "op_is_precharge",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A PRE required by read or write.",
+		"ConfigCode": "0x13",
+		"EventName": "precharge_for_rdwr",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A PRE required by other conditions.",
+		"ConfigCode": "0x14",
+		"EventName": "precharge_for_other",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A read-write turnaround.",
+		"ConfigCode": "0x15",
+		"EventName": "rdwr_transitions",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A write combine (merge) in write data buffer.",
+		"ConfigCode": "0x16",
+		"EventName": "write_combine",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A Write-After-Read hazard.",
+		"ConfigCode": "0x17",
+		"EventName": "war_hazard",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A Read-After-Write hazard.",
+		"ConfigCode": "0x18",
+		"EventName": "raw_hazard",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A Write-After-Write hazard.",
+		"ConfigCode": "0x19",
+		"EventName": "waw_hazard",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "Rank0 enters self-refresh (SRE).",
+		"ConfigCode": "0x1A",
+		"EventName": "op_is_enter_selfref_rk0",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "Rank1 enters self-refresh (SRE).",
+		"ConfigCode": "0x1B",
+		"EventName": "op_is_enter_selfref_rk1",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "Rank2 enters self-refresh (SRE).",
+		"ConfigCode": "0x1C",
+		"EventName": "op_is_enter_selfref_rk2",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "Rank3 enters self-refresh (SRE).",
+		"ConfigCode": "0x1D",
+		"EventName": "op_is_enter_selfref_rk3",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "Rank0 enters power-down (PDE).",
+		"ConfigCode": "0x1E",
+		"EventName": "op_is_enter_powerdown_rk0",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "Rank1 enters power-down (PDE).",
+		"ConfigCode": "0x1F",
+		"EventName": "op_is_enter_powerdown_rk1",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "Rank2 enters power-down (PDE).",
+		"ConfigCode": "0x20",
+		"EventName": "op_is_enter_powerdown_rk2",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "Rank3 enters power-down (PDE).",
+		"ConfigCode": "0x21",
+		"EventName": "op_is_enter_powerdown_rk3",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A cycle that Rank0 stays in self-refresh mode.",
+		"ConfigCode": "0x26",
+		"EventName": "selfref_mode_rk0",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A cycle that Rank1 stays in self-refresh mode.",
+		"ConfigCode": "0x27",
+		"EventName": "selfref_mode_rk1",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A cycle that Rank2 stays in self-refresh mode.",
+		"ConfigCode": "0x28",
+		"EventName": "selfref_mode_rk2",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A cycle that Rank3 stays in self-refresh mode.",
+		"ConfigCode": "0x29",
+		"EventName": "selfref_mode_rk3",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "An auto-refresh (REF) command to DRAM.",
+		"ConfigCode": "0x2A",
+		"EventName": "op_is_refresh",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A critical REF command to DRAM.",
+		"ConfigCode": "0x2B",
+		"EventName": "op_is_crit_ref",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "An MRR or MRW command to DRAM.",
+		"ConfigCode": "0x2D",
+		"EventName": "op_is_load_mode",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A ZQCal command to DRAM.",
+		"ConfigCode": "0x2E",
+		"EventName": "op_is_zqcl",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "At least one entry in read queue reaches the visible window limit.",
+		"ConfigCode": "0x30",
+		"EventName": "visible_window_limit_reached_rd",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "At least one entry in write queue reaches the visible window limit.",
+		"ConfigCode": "0x31",
+		"EventName": "visible_window_limit_reached_wr",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A DQS Oscillator MPC command to DRAM.",
+		"ConfigCode": "0x34",
+		"EventName": "op_is_dqsosc_mpc",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A DQS Oscillator MRR command to DRAM.",
+		"ConfigCode": "0x35",
+		"EventName": "op_is_dqsosc_mrr",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A TCR (Temperature Compensated Refresh) MRR command to DRAM.",
+		"ConfigCode": "0x36",
+		"EventName": "op_is_tcr_mrr",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A ZQCal Start command to DRAM.",
+		"ConfigCode": "0x37",
+		"EventName": "op_is_zqstart",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A ZQCal Latch command to DRAM.",
+		"ConfigCode": "0x38",
+		"EventName": "op_is_zqlatch",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A packet at CHI TXREQ interface (request).",
+		"ConfigCode": "0x39",
+		"EventName": "chi_txreq",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A packet at CHI TXDAT interface (read data).",
+		"ConfigCode": "0x3A",
+		"EventName": "chi_txdat",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A packet at CHI RXDAT interface (write data).",
+		"ConfigCode": "0x3B",
+		"EventName": "chi_rxdat",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A packet at CHI RXRSP interface.",
+		"ConfigCode": "0x3C",
+		"EventName": "chi_rxrsp",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "A violation detected in TZC.",
+		"ConfigCode": "0x3D",
+		"EventName": "tsz_vio",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"BriefDescription": "The ddr cycle.",
+		"ConfigCode": "0x80",
+		"EventName": "cycle",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	}
+]
diff --git a/tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/metrics.json b/tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/metrics.json
new file mode 100644
index 0000000..c14ecac
--- /dev/null
+++ b/tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/metrics.json
@@ -0,0 +1,20 @@
+[
+	{
+		"MetricName": "ddr_read_bandwidth.all",
+		"BriefDescription": "The ddr read bandwidth(MB/s).",
+		"MetricGroup": "ddr",
+		"MetricExpr": "hif_rd * 64 / 1e6 / duration_time",
+		"ScaleUnit": "1MB/s",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	},
+	{
+		"MetricName": "ddr_write_bandwidth.all",
+		"BriefDescription": "The ddr write bandwidth(MB/s).",
+		"MetricGroup": "ddr",
+		"MetricExpr": "(hif_wr + hif_rmw) * 64 / 1e6 / duration_time",
+		"ScaleUnit": "1MB/s",
+		"Unit": "yitian710_ddr",
+		"Compat": "ali_drw_yitian710"
+	}
+]
diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
index 20ed492..8cfb4b6 100755
--- a/tools/perf/pmu-events/jevents.py
+++ b/tools/perf/pmu-events/jevents.py
@@ -257,6 +257,7 @@ class JsonEvent:
           'cpu_core': 'cpu_core',
           'cpu_atom': 'cpu_atom',
           'cmn700': 'cmn700',
+          'yitian710_ddr': 'yitian710_ddr',
       }
       return table[unit] if unit in table else f'uncore_{unit.lower()}'
 
-- 
1.8.3.1

Re: [PATCH RFC 1/4] driver/perf: Add identifier sysfs file for CMN

[John Garry]

On 27/03/2023 03:46, Jing Zhang wrote:
> To allow userspace to identify the specific implementation of the device,
> add an "identifier" sysfs file.
> 
> The perf tool can match the arm CMN metric through the identifier.
> 
> Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com>
> ---
>   drivers/perf/arm-cmn.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 43 insertions(+)
> 
> diff --git a/drivers/perf/arm-cmn.c b/drivers/perf/arm-cmn.c
> index c968986..0c138ad 100644
> --- a/drivers/perf/arm-cmn.c
> +++ b/drivers/perf/arm-cmn.c
> @@ -1168,10 +1168,53 @@ static ssize_t arm_cmn_cpumask_show(struct device *dev,
>   	.attrs = arm_cmn_cpumask_attrs,
>   };
>   
> +static ssize_t arm_cmn_identifier_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct arm_cmn *cmn = to_cmn(dev_get_drvdata(dev));
> +	if (cmn->model == CMN700) {
> +		return sysfs_emit(buf, "%s\n", "CMN700");

Is it possible to have a pointer to this string in struct arm_cmn, such 
that we don't have to do this model to identifier lookup here? If-else 
chains like this are not scalable.

BTW, does this HW have some HW identifier register, like iidr? I think 
that using that may be preferable.

> +	}
> +	else if (cmn->model == CMN650) {
> +		return sysfs_emit(buf, "%s\n", "CMN650");

I'd use lowercase names

> +	}
> +	else if (cmn->model == CMN600) {
> +		return sysfs_emit(buf, "%s\n", "CMN600");
> +	}
> +	else if (cmn->model == CI700) {
> +		return sysfs_emit(buf, "%s\n", "CI700");
> +	}
> +	return sysfs_emit(buf, "%s\n", "UNKNOWN");

can we have a "is_visble" attr to just no show this when unknown?

> +}
> +
> +static umode_t arm_cmn_identifier_attr_visible(struct kobject *kobj,
> +		struct attribute *attr, int n)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct arm_cmn *cmn = to_cmn(dev_get_drvdata(dev));
> +	if (cmn->model <= 0)
> +		return 0;
> +	return attr->mode;
> +};
> +
> +static struct device_attribute arm_cmn_identifier_attr =
> +__ATTR(identifier, 0444, arm_cmn_identifier_show, NULL);
> +
> +static struct attribute *arm_cmn_identifier_attrs[] = {
> +	&arm_cmn_identifier_attr.attr,
> +	NULL,

nit: no need for trailing ',' on a sentinel

> +};
> +
> +static struct attribute_group arm_cmn_identifier_attr_group = {
> +	.attrs = arm_cmn_identifier_attrs,
> +	.is_visible = arm_cmn_identifier_attr_visible,
> +};
> +
>   static const struct attribute_group *arm_cmn_attr_groups[] = {
>   	&arm_cmn_event_attrs_group,
>   	&arm_cmn_format_attrs_group,
>   	&arm_cmn_cpumask_attr_group,
> +	&arm_cmn_identifier_attr_group,
>   	NULL
>   };
>   

Re: [PATCH RFC 0/4] Add JSON metrics for arm CMN and Yitian710 DDR

[Ian Rogers]

On Sun, Mar 26, 2023 at 7:46 PM Jing Zhang <renyu.zj@linux.alibaba.com> wrote:
>
> Hi all,
>
> I add an identifier sysfs file for the yitian710 SoC DDR and arm CMN to
> allow userspace to identify the specific implementation of the device,
> so that the perf tool can match the corresponding uncore events and
> metrics through the identifier. Then added several general CMN700 metrics
> and yitian710 soc DDR metrics.

Thanks!

> Since the eventid of cmn700 events is different from other events, it
> can't be specified by "EventCode" or "ConfigCode", so in the cmn.json
> file of cmn700, no "EventCode" and "ConfigCode" are added for these
> events. For example, the eventid of "arm_cmn_0/hnf_sf_hit is/":
> cat /sys/bus/event_source/devices/arm_cmn_0/events/hnf_sf_hit
> type=0x5,eventid=0x6

This is done to add descriptions to the events? We can add encodings
to jevents.py and the event parsing to handle the names eventid and
type.

> In addition, both cmn700 and ddr PMU can count the information in a die,
> but the same SoC can also be configured with different numbers of dies,
> so it is dificult to design a general expression to obtain metrics in
> different dies. The current yitian710 ddr bandwidth metric describes the
> sum of all dies bandwidth. I would like to ask you, is there any general
> expression can obtain metrics for die? Add an option to specify die?

So hopefully the logic for this is getting clearer in the
perf-tools-next branch. When perf stat runs it will aggregate in a
number of different ways, if you pass -A it will remove the
aggregation, but you can also use --per-socket, per-die, .. The
metrics take the individual counter values, say instructions and
cycles and produce a metric like IPC. By default all the instruction
counts are aggregated together, the cycles are aggregated together and
then the metric produced on the two aggregated values. When -A or
--per-die are passed, the appropriate amount of aggregation should be
done then the metric computed multiple times.

Are you asking for a way in a metric to take counts from one die and
use them in the other's metric? For example, reads on one die are
writes on the other? This is possible as we have all the counts in the
tool. I've thought about this in the context of some metrics we have
for AMD, but there is no support for this in the tool currently.

Thanks,
Ian

> Thanks,
> Jing
>
> Jing Zhang (4):
>   driver/perf: Add identifier sysfs file for CMN
>   perf vendor events: Add JSON metrics for cmn700
>   driver/perf: Add identifier sysfs file for Yitian 710 DDR
>   perf vendor events: Add JSON metrics for Yitian 710 DDR
>
>  drivers/perf/alibaba_uncore_drw_pmu.c              |  27 ++
>  drivers/perf/arm-cmn.c                             |  43 +++
>  .../pmu-events/arch/arm64/arm/cmn700/sys/cmn.json  | 188 +++++++++++
>  .../arch/arm64/arm/cmn700/sys/metrics.json         |  74 ++++
>  .../arm64/freescale/yitian710/sys/ali_drw.json     | 373 +++++++++++++++++++++
>  .../arm64/freescale/yitian710/sys/metrics.json     |  20 ++
>  tools/perf/pmu-events/jevents.py                   |   2 +
>  7 files changed, 727 insertions(+)
>  create mode 100644 tools/perf/pmu-events/arch/arm64/arm/cmn700/sys/cmn.json
>  create mode 100644 tools/perf/pmu-events/arch/arm64/arm/cmn700/sys/metrics.json
>  create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/ali_drw.json
>  create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/metrics.json
>
> --
> 1.8.3.1
>

Re: [PATCH RFC 3/4] driver/perf: Add identifier sysfs file for Yitian 710 DDR

[Shuai Xue]

On 2023/3/27 AM10:46, Jing Zhang wrote:
> To allow userspace to identify the specific implementation of the device,
> add an "identifier" sysfs file.
> 
> The perf tool can match the Yitian 710 DDR metric through the identifier.
> 
> Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com>
> ---
>  drivers/perf/alibaba_uncore_drw_pmu.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/perf/alibaba_uncore_drw_pmu.c b/drivers/perf/alibaba_uncore_drw_pmu.c
> index a7689fe..6639a57 100644
> --- a/drivers/perf/alibaba_uncore_drw_pmu.c
> +++ b/drivers/perf/alibaba_uncore_drw_pmu.c
> @@ -236,10 +236,37 @@ static ssize_t ali_drw_pmu_cpumask_show(struct device *dev,
>  	.attrs = ali_drw_pmu_cpumask_attrs,
>  };
>  
> +static ssize_t ali_drw_pmu_identifier_show(struct device *dev,
> +					struct device_attribute *attr,
> +					char *page)
> +{
> +	return sysfs_emit(page, "%s\n", "ali_drw_yitian710");

Is it possible to rename identifier as "ali_drw_pmu"? I don't think we need only
limit alibaba_uncore_drw_pmu to Yitian710 SoC here.

Thank you.
Shuai

> +}
> +
> +static umode_t ali_drw_pmu_identifier_attr_visible(struct kobject *kobj,
> +						struct attribute *attr, int n)
> +{
> +	return attr->mode;
> +}
> +
> +static struct device_attribute ali_drw_pmu_identifier_attr =
> +	__ATTR(identifier, 0444, ali_drw_pmu_identifier_show, NULL);
> +
> +static struct attribute *ali_drw_pmu_identifier_attrs[] = {
> +	&ali_drw_pmu_identifier_attr.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group ali_drw_pmu_identifier_attr_group = {
> +	.attrs = ali_drw_pmu_identifier_attrs,
> +	.is_visible = ali_drw_pmu_identifier_attr_visible,
> +};
> +
>  static const struct attribute_group *ali_drw_pmu_attr_groups[] = {
>  	&ali_drw_pmu_events_attr_group,
>  	&ali_drw_pmu_cpumask_attr_group,
>  	&ali_drw_pmu_format_group,
> +	&ali_drw_pmu_identifier_attr_group,
>  	NULL,
>  };
>  

Re: [PATCH RFC 1/4] driver/perf: Add identifier sysfs file for CMN

[Jing Zhang]

在 2023/3/27 下午3:55, John Garry 写道:
> On 27/03/2023 03:46, Jing Zhang wrote:
>> To allow userspace to identify the specific implementation of the device,
>> add an "identifier" sysfs file.
>>
>> The perf tool can match the arm CMN metric through the identifier.
>>
>> Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com>
>> ---
>>   drivers/perf/arm-cmn.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 43 insertions(+)
>>
>> diff --git a/drivers/perf/arm-cmn.c b/drivers/perf/arm-cmn.c
>> index c968986..0c138ad 100644
>> --- a/drivers/perf/arm-cmn.c
>> +++ b/drivers/perf/arm-cmn.c
>> @@ -1168,10 +1168,53 @@ static ssize_t arm_cmn_cpumask_show(struct device *dev,
>>       .attrs = arm_cmn_cpumask_attrs,
>>   };
>>   +static ssize_t arm_cmn_identifier_show(struct device *dev,
>> +        struct device_attribute *attr, char *buf)
>> +{
>> +    struct arm_cmn *cmn = to_cmn(dev_get_drvdata(dev));
>> +    if (cmn->model == CMN700) {
>> +        return sysfs_emit(buf, "%s\n", "CMN700");
> 
> Is it possible to have a pointer to this string in struct arm_cmn, such that we don't have to do this model to identifier lookup here? If-else chains like this are not scalable.
> 
Will do.

> BTW, does this HW have some HW identifier register, like iidr? I think that using that may be preferable.
> 

I didn't find the relevant identifier register.

Do Illka and Robin know that there is such a register that can identify different CMN versions? Looking forward to your suggestions.


>> +    }
>> +    else if (cmn->model == CMN650) {
>> +        return sysfs_emit(buf, "%s\n", "CMN650");
> 
> I'd use lowercase names
> 
Ok.

>> +    }
>> +    else if (cmn->model == CMN600) {
>> +        return sysfs_emit(buf, "%s\n", "CMN600");
>> +    }
>> +    else if (cmn->model == CI700) {
>> +        return sysfs_emit(buf, "%s\n", "CI700");
>> +    }
>> +    return sysfs_emit(buf, "%s\n", "UNKNOWN");
> 
> can we have a "is_visble" attr to just no show this when unknown?
> 

Ok.

>> +}
>> +
>> +static umode_t arm_cmn_identifier_attr_visible(struct kobject *kobj,
>> +        struct attribute *attr, int n)
>> +{
>> +    struct device *dev = kobj_to_dev(kobj);
>> +    struct arm_cmn *cmn = to_cmn(dev_get_drvdata(dev));
>> +    if (cmn->model <= 0)
>> +        return 0;
>> +    return attr->mode;
>> +};
>> +
>> +static struct device_attribute arm_cmn_identifier_attr =
>> +__ATTR(identifier, 0444, arm_cmn_identifier_show, NULL);
>> +
>> +static struct attribute *arm_cmn_identifier_attrs[] = {
>> +    &arm_cmn_identifier_attr.attr,
>> +    NULL,
> 
> nit: no need for trailing ',' on a sentinel
> 

Ok, Will do.

>> +};
>> +
>> +static struct attribute_group arm_cmn_identifier_attr_group = {
>> +    .attrs = arm_cmn_identifier_attrs,
>> +    .is_visible = arm_cmn_identifier_attr_visible,
>> +};
>> +
>>   static const struct attribute_group *arm_cmn_attr_groups[] = {
>>       &arm_cmn_event_attrs_group,
>>       &arm_cmn_format_attrs_group,
>>       &arm_cmn_cpumask_attr_group,
>> +    &arm_cmn_identifier_attr_group,
>>       NULL
>>   };
>>   

Re: [PATCH RFC 0/4] Add JSON metrics for arm CMN and Yitian710 DDR

[Jing Zhang]

在 2023/3/28 上午12:51, Ian Rogers 写道:
> On Sun, Mar 26, 2023 at 7:46 PM Jing Zhang <renyu.zj@linux.alibaba.com> wrote:
>>
>> Hi all,
>>
>> I add an identifier sysfs file for the yitian710 SoC DDR and arm CMN to
>> allow userspace to identify the specific implementation of the device,
>> so that the perf tool can match the corresponding uncore events and
>> metrics through the identifier. Then added several general CMN700 metrics
>> and yitian710 soc DDR metrics.
> 
> Thanks!
> 
>> Since the eventid of cmn700 events is different from other events, it
>> can't be specified by "EventCode" or "ConfigCode", so in the cmn.json
>> file of cmn700, no "EventCode" and "ConfigCode" are added for these
>> events. For example, the eventid of "arm_cmn_0/hnf_sf_hit is/":
>> cat /sys/bus/event_source/devices/arm_cmn_0/events/hnf_sf_hit
>> type=0x5,eventid=0x6
> 
> This is done to add descriptions to the events? We can add encodings
> to jevents.py and the event parsing to handle the names eventid and
> type.
> 

Ok, I will try it.

>> In addition, both cmn700 and ddr PMU can count the information in a die,
>> but the same SoC can also be configured with different numbers of dies,
>> so it is dificult to design a general expression to obtain metrics in
>> different dies. The current yitian710 ddr bandwidth metric describes the
>> sum of all dies bandwidth. I would like to ask you, is there any general
>> expression can obtain metrics for die? Add an option to specify die?
> 
> So hopefully the logic for this is getting clearer in the
> perf-tools-next branch. When perf stat runs it will aggregate in a
> number of different ways, if you pass -A it will remove the
> aggregation, but you can also use --per-socket, per-die, .. The
> metrics take the individual counter values, say instructions and
> cycles and produce a metric like IPC. By default all the instruction
> counts are aggregated together, the cycles are aggregated together and
> then the metric produced on the two aggregated values. When -A or
> --per-die are passed, the appropriate amount of aggregation should be
> done then the metric computed multiple times.
> 

Thanks! It is helpful. It solved my problem.

Thanks,
Jing

> Are you asking for a way in a metric to take counts from one die and
> use them in the other's metric? For example, reads on one die are
> writes on the other? This is possible as we have all the counts in the
> tool. I've thought about this in the context of some metrics we have
> for AMD, but there is no support for this in the tool currently.
> 
> Thanks,
> Ian
> 
>> Thanks,
>> Jing
>>
>> Jing Zhang (4):
>>   driver/perf: Add identifier sysfs file for CMN
>>   perf vendor events: Add JSON metrics for cmn700
>>   driver/perf: Add identifier sysfs file for Yitian 710 DDR
>>   perf vendor events: Add JSON metrics for Yitian 710 DDR
>>
>>  drivers/perf/alibaba_uncore_drw_pmu.c              |  27 ++
>>  drivers/perf/arm-cmn.c                             |  43 +++
>>  .../pmu-events/arch/arm64/arm/cmn700/sys/cmn.json  | 188 +++++++++++
>>  .../arch/arm64/arm/cmn700/sys/metrics.json         |  74 ++++
>>  .../arm64/freescale/yitian710/sys/ali_drw.json     | 373 +++++++++++++++++++++
>>  .../arm64/freescale/yitian710/sys/metrics.json     |  20 ++
>>  tools/perf/pmu-events/jevents.py                   |   2 +
>>  7 files changed, 727 insertions(+)
>>  create mode 100644 tools/perf/pmu-events/arch/arm64/arm/cmn700/sys/cmn.json
>>  create mode 100644 tools/perf/pmu-events/arch/arm64/arm/cmn700/sys/metrics.json
>>  create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/ali_drw.json
>>  create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/metrics.json
>>
>> --
>> 1.8.3.1
>>

Re: [PATCH RFC 3/4] driver/perf: Add identifier sysfs file for Yitian 710 DDR

[Jing Zhang]

在 2023/3/29 下午3:55, Shuai Xue 写道:
> 
> 
> On 2023/3/27 AM10:46, Jing Zhang wrote:
>> To allow userspace to identify the specific implementation of the device,
>> add an "identifier" sysfs file.
>>
>> The perf tool can match the Yitian 710 DDR metric through the identifier.
>>
>> Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com>
>> ---
>>  drivers/perf/alibaba_uncore_drw_pmu.c | 27 +++++++++++++++++++++++++++
>>  1 file changed, 27 insertions(+)
>>
>> diff --git a/drivers/perf/alibaba_uncore_drw_pmu.c b/drivers/perf/alibaba_uncore_drw_pmu.c
>> index a7689fe..6639a57 100644
>> --- a/drivers/perf/alibaba_uncore_drw_pmu.c
>> +++ b/drivers/perf/alibaba_uncore_drw_pmu.c
>> @@ -236,10 +236,37 @@ static ssize_t ali_drw_pmu_cpumask_show(struct device *dev,
>>  	.attrs = ali_drw_pmu_cpumask_attrs,
>>  };
>>  
>> +static ssize_t ali_drw_pmu_identifier_show(struct device *dev,
>> +					struct device_attribute *attr,
>> +					char *page)
>> +{
>> +	return sysfs_emit(page, "%s\n", "ali_drw_yitian710");
> 
> Is it possible to rename identifier as "ali_drw_pmu"? I don't think we need only
> limit alibaba_uncore_drw_pmu to Yitian710 SoC here.
> 

Ok, I will rename it as "ali_drw_pmu".

Thanks,
Jing

> Thank you.
> Shuai
> 
>> +}
>> +
>> +static umode_t ali_drw_pmu_identifier_attr_visible(struct kobject *kobj,
>> +						struct attribute *attr, int n)
>> +{
>> +	return attr->mode;
>> +}
>> +
>> +static struct device_attribute ali_drw_pmu_identifier_attr =
>> +	__ATTR(identifier, 0444, ali_drw_pmu_identifier_show, NULL);
>> +
>> +static struct attribute *ali_drw_pmu_identifier_attrs[] = {
>> +	&ali_drw_pmu_identifier_attr.attr,
>> +	NULL,
>> +};
>> +
>> +static const struct attribute_group ali_drw_pmu_identifier_attr_group = {
>> +	.attrs = ali_drw_pmu_identifier_attrs,
>> +	.is_visible = ali_drw_pmu_identifier_attr_visible,
>> +};
>> +
>>  static const struct attribute_group *ali_drw_pmu_attr_groups[] = {
>>  	&ali_drw_pmu_events_attr_group,
>>  	&ali_drw_pmu_cpumask_attr_group,
>>  	&ali_drw_pmu_format_group,
>> +	&ali_drw_pmu_identifier_attr_group,
>>  	NULL,
>>  };
>>  

Re: [PATCH RFC 1/4] driver/perf: Add identifier sysfs file for CMN

[Robin Murphy]

On 2023-03-29 12:53, Jing Zhang wrote:
> 
> 
> 在 2023/3/27 下午3:55, John Garry 写道:
>> On 27/03/2023 03:46, Jing Zhang wrote:
>>> To allow userspace to identify the specific implementation of the device,
>>> add an "identifier" sysfs file.
>>>
>>> The perf tool can match the arm CMN metric through the identifier.
>>>
>>> Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com>
>>> ---
>>>    drivers/perf/arm-cmn.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>>>    1 file changed, 43 insertions(+)
>>>
>>> diff --git a/drivers/perf/arm-cmn.c b/drivers/perf/arm-cmn.c
>>> index c968986..0c138ad 100644
>>> --- a/drivers/perf/arm-cmn.c
>>> +++ b/drivers/perf/arm-cmn.c
>>> @@ -1168,10 +1168,53 @@ static ssize_t arm_cmn_cpumask_show(struct device *dev,
>>>        .attrs = arm_cmn_cpumask_attrs,
>>>    };
>>>    +static ssize_t arm_cmn_identifier_show(struct device *dev,
>>> +        struct device_attribute *attr, char *buf)
>>> +{
>>> +    struct arm_cmn *cmn = to_cmn(dev_get_drvdata(dev));
>>> +    if (cmn->model == CMN700) {
>>> +        return sysfs_emit(buf, "%s\n", "CMN700");
>>
>> Is it possible to have a pointer to this string in struct arm_cmn, such that we don't have to do this model to identifier lookup here? If-else chains like this are not scalable.
>>
> Will do.
> 
>> BTW, does this HW have some HW identifier register, like iidr? I think that using that may be preferable.
>>
> 
> I didn't find the relevant identifier register.
> 
> Do Illka and Robin know that there is such a register that can identify different CMN versions? Looking forward to your suggestions.

In principle the "part number" fields from CFGM_PERIPH_ID_0/1 are 
supposed to identify the model, but for various reasons I'm suspicious 
of that being unreliable (not least that no actual values are 
documented, only "configuration-dependent"). That's why I went down the 
route of making sure we have explicit ACPI/DT identifiers for every model.

However, the model alone seems either too specific or not specific 
enough for a jevents identifier. The defined metrics are pretty trivial 
and should have no real reason not to be common to *any* CMN PMU where 
the underlying events are present. On the other hand, if we want to get 
down to the level of specific events in JSON then we'd need to consider 
the revision as well, since there are several events which only exist on 
certain revisions of a given model (but often are also common to later 
models).

This actually foreshadows a question I was planning to bring up in the 
context of another driver I'm working on - for this one I would rather 
like to try using jevents rather than have to maintain another sprawl of 
event tables in a driver, but it's still going to have the same thing of 
wanting model/revision matching along the lines of what 
arm_cmn_event_attr_is_visible() is doing for CMN events. AFAICS this 
would need jevents to grow a rather more flexible way of encoding and 
matching identifiers, since having dozens of almost-identical copies of 
event definitions for every exact identifier value is clearly 
unworkable. Does anyone happen to have any thoughts or preferences 
around how that might be approached?

>>> +    }
>>> +    else if (cmn->model == CMN650) {
>>> +        return sysfs_emit(buf, "%s\n", "CMN650");
>>
>> I'd use lowercase names
>>
> Ok.
> 
>>> +    }
>>> +    else if (cmn->model == CMN600) {
>>> +        return sysfs_emit(buf, "%s\n", "CMN600");
>>> +    }
>>> +    else if (cmn->model == CI700) {
>>> +        return sysfs_emit(buf, "%s\n", "CI700");
>>> +    }
>>> +    return sysfs_emit(buf, "%s\n", "UNKNOWN");
>>
>> can we have a "is_visble" attr to just no show this when unknown?

No need - it will never be unknown unless someone goes out of their way 
to break the probing code and/or match_data.

>>
> 
> Ok.
> 
>>> +}
>>> +
>>> +static umode_t arm_cmn_identifier_attr_visible(struct kobject *kobj,
>>> +        struct attribute *attr, int n)
>>> +{
>>> +    struct device *dev = kobj_to_dev(kobj);
>>> +    struct arm_cmn *cmn = to_cmn(dev_get_drvdata(dev));
>>> +    if (cmn->model <= 0)
>>> +        return 0;
>>> +    return attr->mode;
>>> +};

As above, "cmn->model <= 0" can never be true.

Thanks,
Robin.

>>> +
>>> +static struct device_attribute arm_cmn_identifier_attr =
>>> +__ATTR(identifier, 0444, arm_cmn_identifier_show, NULL);
>>> +
>>> +static struct attribute *arm_cmn_identifier_attrs[] = {
>>> +    &arm_cmn_identifier_attr.attr,
>>> +    NULL,
>>
>> nit: no need for trailing ',' on a sentinel
>>
> 
> Ok, Will do.
> 
>>> +};
>>> +
>>> +static struct attribute_group arm_cmn_identifier_attr_group = {
>>> +    .attrs = arm_cmn_identifier_attrs,
>>> +    .is_visible = arm_cmn_identifier_attr_visible,
>>> +};
>>> +
>>>    static const struct attribute_group *arm_cmn_attr_groups[] = {
>>>        &arm_cmn_event_attrs_group,
>>>        &arm_cmn_format_attrs_group,
>>>        &arm_cmn_cpumask_attr_group,
>>> +    &arm_cmn_identifier_attr_group,
>>>        NULL
>>>    };
>>>    

Re: [PATCH RFC 1/4] driver/perf: Add identifier sysfs file for CMN

[John Garry]

On 29/03/2023 18:47, Robin Murphy wrote:
>> Do Illka and Robin know that there is such a register that can 
>> identify different CMN versions? Looking forward to your suggestions.
> 
> In principle the "part number" fields from CFGM_PERIPH_ID_0/1 are 
> supposed to identify the model, but for various reasons I'm suspicious 
> of that being unreliable (not least that no actual values are 
> documented, only "configuration-dependent"). That's why I went down the 
> route of making sure we have explicit ACPI/DT identifiers for every model.
> 
> However, the model alone seems either too specific or not specific 
> enough for a jevents identifier. The defined metrics are pretty trivial 
> and should have no real reason not to be common to *any* CMN PMU where 
> the underlying events are present. On the other hand, if we want to get 
> down to the level of specific events in JSON then we'd need to consider 
> the revision as well, since there are several events which only exist on 
> certain revisions of a given model (but often are also common to later 
> models).
> 
> This actually foreshadows a question I was planning to bring up in the 
> context of another driver I'm working on - for this one I would rather 
> like to try using jevents rather than have to maintain another sprawl of 
> event tables in a driver, but it's still going to have the same thing of 
> wanting model/revision matching along the lines of what 
> arm_cmn_event_attr_is_visible() is doing for CMN events. AFAICS this 
> would need jevents to grow a rather more flexible way of encoding and 
> matching identifiers, since having dozens of almost-identical copies of 
> event definitions for every exact identifier value is clearly 
> unworkable.

This sort of problem has not occurred yet as perf tool only supports 
"system" events for a handful of SoCs so far :)

> Does anyone happen to have any thoughts or preferences 
> around how that might be approached?
> 

Currently the perf tool will just match system events based on the exact 
HW identifier and PMU name.

However, if you consider PMCG PMU support as an example of possible area 
of improvement, it has a number of fixed events and a number of IMPDEF 
events. There should be no reason to need to describe in a separate JSON 
those fixed events for every instance of that PMU.

So a simple change would be to teach perf tool that for certain fixed 
events we only need to match based on the PMU name. For others we need 
to match based on some identifier.

If matching based on an identifier still leads to unwieldy amounts of 
tables, then maybe HW identifier wildcard matching may suit, like what 
is done for CPU events.

Thanks,
John