[RFC PATCH v2 00/13] perf pmu-events: Support event aliasing for system PMUs

[RFC PATCH v2 00/13] perf pmu-events: Support event aliasing for system PMUs

[John Garry]

Currently event aliasing for only CPU and uncore PMUs is supported. In
fact, only uncore PMUs aliasing is supported for when the uncore PMUs are
fixed for a CPU, which may not always be the case for certain
architectures.

This series adds support for PMU event aliasing for system and other
uncore PMUs which are not tied to a specific CPU. Or, more specifically,
CPUs which not tied to those PMUs.

For this, we introduce system event tables in generated pmu-events.c,
which contain a per-SoC table of events of all its system PMUs. Each
per-PMU event is matched by a "COMPAT" property.

When creating aliases for PMUs, we treat core/uncore* and system PMUs
differently:

- For CPU PMU, we always match for the event mapfile based on the CPUID.
   This has not changed.

- For an uncore or system PMU, we iterate through all the events in all
   the system PMU tables.

   Matches are based on the "COMPAT" property matching the PMU sysfs
   identifier contents, in /sys/bus/event_source/devices/<PMU>/identifier

* uncore PMUs may also be matched by system PMUs event support.

Initial reference support is also added for ARM SMMUv3 PMCG (Performance
Monitor Event Group) PMU for HiSilicon hip08 platform with only a single
event so far - see driver in drivers/perf/arm_smmuv3_pmu.c for that driver.

Here is a sample output with this series on Huawei D06CS board:

root@ubuntu:/# ./perf list
   [...]

smmu v3 pmcg:
   smmuv3_pmcg.config_cache_miss
        [Configuration cache miss caused by transaction or(ATS or
        non-ATS)translation request. Unit: smmuv3_pmcg]
   smmuv3_pmcg.config_struct_access
        [Configuration structure access. Unit: smmuv3_pmcg]
   smmuv3_pmcg.cycles
        [Clock cycles. Unit: smmuv3_pmcg]
   smmuv3_pmcg.l1_tlb
        [SMMUv3 PMCG L1 TABLE transation. Unit: smmuv3_pmcg]
   smmuv3_pmcg.pcie_ats_trans_passed
        [PCIe ATS Translated Transaction passed through SMMU. Unit: 
smmuv3_pmcg]
   smmuv3_pmcg.pcie_ats_trans_rq
        [PCIe ATS Translation Request received. Unit: smmuv3_pmcg]
   smmuv3_pmcg.tlb_miss
        [TLB miss caused by incomingtransaction or (ATS or non-ATS) 
translation
         request. Unit: smmuv3_pmcg]
   smmuv3_pmcg.trans_table_walk_access
        [Translation table walk access. Unit: smmuv3_pmcg]
   smmuv3_pmcg.transaction
        [Transaction. Unit: smmuv3_pmcg]


root@ubuntu:/# ./perf stat -v -e smmuv3_pmcg.l1_tlb sleep 1
Using CPUID 0x00000000480fd010
Using SYSID HIP08
-> smmuv3_pmcg_200100020/event=0x8a/
-> smmuv3_pmcg_200140020/event=0x8a/
-> smmuv3_pmcg_100020/event=0x8a/
-> smmuv3_pmcg_140020/event=0x8a/
-> smmuv3_pmcg_200148020/event=0x8a/
-> smmuv3_pmcg_148020/event=0x8a/
smmuv3_pmcg.l1_tlb: 0 1001221690 1001221690
smmuv3_pmcg.l1_tlb: 0 1001220090 1001220090
smmuv3_pmcg.l1_tlb: 101 1001219660 1001219660
smmuv3_pmcg.l1_tlb: 0 1001219010 1001219010
smmuv3_pmcg.l1_tlb: 0 1001218360 1001218360
smmuv3_pmcg.l1_tlb: 134 1001217850 1001217850

Performance counter stats for 'system wide':

                235      smmuv3_pmcg.l1_tlb 


        1.001263128 seconds time elapsed

root@ubuntu:/#

Support is also added for imx8mm DDR PMU.

Series is here:
https://github.com/hisilicon/kernel-dev/commits/private-topic-perf-5.6-sys-pmu-events-v2-upstream

Differences to v1:
- Stop using SoC id and use a per-PMU identifier instead
- Add metric group sys events support
   - This is a bit hacky
- Add imx8mm DDR Perf support
- Add fix for parse events sel
	- without it, I get this spewed for metric event:

	assertion failed at util/parse-events.c:1637

Patches still need to be sent to support per-PMU identifer sysfs file
in the kernel.

Thanks,
John

Joakim Zhang (1):
  perf vendor events: Add JSON metrics for imx8mm DDR Perf

John Garry (12):
  perf parse-events: Fix comparison of evsel and leader pmu name
  perf jevents: Add support for an extra directory level
  perf jevents: Add support for system events tables
  perf vendor events arm64: Relocate hip08 events
  perf vendor events arm64: Add Architected events smmuv3-pmcg.json
  perf vendor events arm64: Add hip08 SMMUv3 PMCG events
  perf pmu: Add pmu_id()
  perf pmu: Add pmu_add_sys_aliases()
  perf metricgroup: Split up metricgroup__add_metric()
  perf metricgroup: Split up metricgroup__print()
  perf metricgroup: Support printing metric groups for system PMUs
  perf metricgroup: Support adding metrics for system PMUs

 .../arch/arm64/freescale/imx8mm/sys/ddrc.json      |  39 +++
 .../arch/arm64/freescale/imx8mm/sys/metrics.json   |  18 ++
 .../hisilicon/hip08/{ => cpu}/core-imp-def.json    |   0
 .../hisilicon/hip08/{ => cpu}/uncore-ddrc.json     |   0
 .../hisilicon/hip08/{ => cpu}/uncore-hha.json      |   0
 .../hisilicon/hip08/{ => cpu}/uncore-l3c.json      |   0
 .../arm64/hisilicon/hip08/sys/smmu-v3-pmcg.json    |  42 +++
 tools/perf/pmu-events/arch/arm64/mapfile.csv       |   2 +-
 tools/perf/pmu-events/arch/arm64/smmuv3-pmcg.json  |  58 ++++
 tools/perf/pmu-events/jevents.c                    | 152 ++++++++---
 tools/perf/pmu-events/jevents.h                    |  11 +-
 tools/perf/pmu-events/pmu-events.h                 |   6 +
 tools/perf/util/metricgroup.c                      | 291 ++++++++++++++-------
 tools/perf/util/parse-events.c                     |   2 +-
 tools/perf/util/pmu.c                              | 115 ++++++++
 tools/perf/util/pmu.h                              |   6 +
 16 files changed, 612 insertions(+), 130 deletions(-)
 create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/ddrc.json
 create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json
 rename tools/perf/pmu-events/arch/arm64/hisilicon/hip08/{ => cpu}/core-imp-def.json (100%)
 rename tools/perf/pmu-events/arch/arm64/hisilicon/hip08/{ => cpu}/uncore-ddrc.json (100%)
 rename tools/perf/pmu-events/arch/arm64/hisilicon/hip08/{ => cpu}/uncore-hha.json (100%)
 rename tools/perf/pmu-events/arch/arm64/hisilicon/hip08/{ => cpu}/uncore-l3c.json (100%)
 create mode 100644 tools/perf/pmu-events/arch/arm64/hisilicon/hip08/sys/smmu-v3-pmcg.json
 create mode 100644 tools/perf/pmu-events/arch/arm64/smmuv3-pmcg.json

-- 
2.16.4

[RFC PATCH v2 01/13] perf parse-events: Fix comparison of evsel and leader pmu name

[John Garry]

Since we now strdup() the pmu name for the event selector, use strcmp()
instead of pointer equality for comparison.

Fixes: d4953f7ef1a2 ("perf parse-events: Fix 3 use after frees found with clang ASANutil/parse-events.c")
Signed-off-by: John Garry <john.garry@huawei.com>
---

I am not 100% sure that this is the right fix....

 tools/perf/util/parse-events.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 10107747b361..90ddade1ba23 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1629,7 +1629,7 @@ parse_events__set_leader_for_uncore_aliase(char *name, struct list_head *list,
 		 * event. That can be used to distinguish the leader from
 		 * other members, even they have the same event name.
 		 */
-		if ((leader != evsel) && (leader->pmu_name == evsel->pmu_name)) {
+		if ((leader != evsel) && !strcmp(leader->pmu_name, evsel->pmu_name)) {
 			is_leader = false;
 			continue;
 		}
-- 
2.16.4

[RFC PATCH v2 02/13] perf jevents: Add support for an extra directory level

[John Garry]

Currently we support upto a level 2 directory, and level 2 would be in the
form vendor/platform.

Add support for a further level, to hold specific categories of events for
when we want to segregate them for matching purposes.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 tools/perf/pmu-events/jevents.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
index fa86c5f997cc..c7868d0a7a6b 100644
--- a/tools/perf/pmu-events/jevents.c
+++ b/tools/perf/pmu-events/jevents.c
@@ -981,15 +981,20 @@ static int process_one_file(const char *fpath, const struct stat *sb,
 	int level   = ftwbuf->level;
 	int err = 0;
 
-	if (level == 2 && is_dir) {
+	if (level >= 2 && is_dir) {
+		int count = 0;
 		/*
 		 * For level 2 directory, bname will include parent name,
 		 * like vendor/platform. So search back from platform dir
 		 * to find this.
+		 * Something similar for level 3 directory, but we're a PMU
+		 * category folder, like vendor/platform/cpu.
 		 */
 		bname = (char *) fpath + ftwbuf->base - 2;
 		for (;;) {
 			if (*bname == '/')
+				count++;
+			if (count == level - 1)
 				break;
 			bname--;
 		}
@@ -1002,13 +1007,13 @@ static int process_one_file(const char *fpath, const struct stat *sb,
 		 level, sb->st_size, bname, fpath);
 
 	/* base dir or too deep */
-	if (level == 0 || level > 3)
+	if (level == 0 || level > 4)
 		return 0;
 
 
 	/* model directory, reset topic */
 	if ((level == 1 && is_dir && is_leaf_dir(fpath)) ||
-	    (level == 2 && is_dir)) {
+	    (level >= 2 && is_dir && is_leaf_dir(fpath))) {
 		if (close_table)
 			print_events_table_suffix(eventsfp);
 
-- 
2.16.4

[RFC PATCH v2 03/13] perf jevents: Add support for system events tables

[John Garry]

Process the JSONs to find support for "system" events, which are not tied
to a specific CPUID.

We now use a "COMPAT" property to match against the namespace ID from the
kernel PMU driver.

The generated pmu-events.c will now have 2 tables: a. As before, we will
have the event table for CPU events. b. New pmu_sys_event_tables[] table,
which will have events matched to SoCs.

It will look like this:

struct pmu_event pme_hisilicon_hip08_sys[] = {
{
	.name = "cycles",
	.compat = "HIP08",
	.event = "event=0",
	.desc = "Clock cycles",
	.topic = "smmu v3 pmcg",
	.long_desc = "Clock cycles",
},
{
	.name = "smmuv3_pmcg.l1_tlb",
	.compat = "HIP08",
	.event = "event=0x8a",
	.desc = "SMMUv3 PMCG l1_tlb. Unit: smmuv3_pmcg ",
	.topic = "smmu v3 pmcg",
	.long_desc = "SMMUv3 PMCG l1_tlb",
	.pmu = "smmuv3_pmcg",
},
...
};

struct pmu_event pme_arm_cortex_a53[] = {
{
	.name = "ext_mem_req",
	.event = "event=0xc0",
	.desc = "External memory request",
	.topic = "memory",
},
{
	.name = "ext_mem_req_nc",
	.event = "event=0xc1",
	.desc = "Non-cacheable external memory request",
	.topic = "memory",
},
...
};

struct pmu_event pme_hisilicon_hip08_cpu[] = {
{
	.name = "l2d_cache_refill_wr",
	.event = "event=0x53",
	.desc = "L2D cache refill, write",
	.topic = "core imp def",
	.long_desc = "Attributable Level 2 data cache refill, write",
},
...
};

struct pmu_events_map pmu_events_map[] = {
{
	.cpuid = "0x00000000410fd030",
	.version = "v1",
	.type = "core",
	.table = pme_arm_cortex_a53
},
{
	.cpuid = "0x00000000480fd010",
	.version = "v1",
	.type = "core",
	.table = pme_hisilicon_hip08_cpu
},
	{
		.table = 0
	},
};

struct pmu_event pme_hisilicon_hip08_cpu[] = {
{
	.name = "uncore_hisi_l3c.rd_cpipe",
	.event = "event=0",
	.desc = "Total read accesses. Unit: hisi_sccl,l3c ",
	.topic = "uncore l3c",
	.long_desc = "Total read accesses",
	.pmu = "hisi_sccl,l3c",
},
{
	.name = "uncore_hisi_l3c.wr_cpipe",
	.event = "event=0x1",
	.desc = "Total write accesses. Unit: hisi_sccl,l3c ",
	.topic = "uncore l3c",
	.long_desc = "Total write accesses",
	.pmu = "hisi_sccl,l3c",
},
...
};


struct pmu_sys_events pmu_sys_event_tables[] = {
{
	.table = pme_hisilicon_hip08_sys,
},
...
};

Signed-off-by: John Garry <john.garry@huawei.com>
---
 tools/perf/pmu-events/jevents.c    | 138 ++++++++++++++++++++++++++++---------
 tools/perf/pmu-events/jevents.h    |  11 ++-
 tools/perf/pmu-events/pmu-events.h |   6 ++
 3 files changed, 118 insertions(+), 37 deletions(-)

diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
index c7868d0a7a6b..acb6b77bddc0 100644
--- a/tools/perf/pmu-events/jevents.c
+++ b/tools/perf/pmu-events/jevents.c
@@ -53,6 +53,23 @@
 int verbose;
 char *prog;
 
+static LIST_HEAD(sys_event_tables);
+
+struct sys_event_table {
+	struct list_head list;
+	char *name;
+};
+
+static void free_sys_event_tables(void)
+{
+	struct sys_event_table *et, *next;
+
+	list_for_each_entry_safe(et, next, &sys_event_tables, list) {
+		free(et->name);
+		free(et);
+	}
+}
+
 int eprintf(int level, int var, const char *fmt, ...)
 {
 
@@ -318,12 +335,12 @@ static void print_events_table_prefix(FILE *fp, const char *tblname)
 	close_table = 1;
 }
 
-static int print_events_table_entry(void *data, char *name, char *event,
-				    char *desc, char *long_desc,
+static int print_events_table_entry(void *data, char *name, char *compat,
+				    char *event, char *desc, char *long_desc,
 				    char *pmu, char *unit, char *perpkg,
-				    char *metric_expr,
-				    char *metric_name, char *metric_group,
-				    char *deprecated, char *metric_constraint)
+				    char *metric_expr, char *metric_name,
+				    char *metric_group, char *deprecated,
+				    char *metric_constraint)
 {
 	struct perf_entry_data *pd = data;
 	FILE *outfp = pd->outfp;
@@ -337,6 +354,9 @@ static int print_events_table_entry(void *data, char *name, char *event,
 
 	if (name)
 		fprintf(outfp, "\t.name = \"%s\",\n", name);
+	if (compat)
+		fprintf(outfp, "\t.compat = \"%s\",\n", compat);
+
 	if (event)
 		fprintf(outfp, "\t.event = \"%s\",\n", event);
 	fprintf(outfp, "\t.desc = \"%s\",\n", desc);
@@ -367,6 +387,7 @@ static int print_events_table_entry(void *data, char *name, char *event,
 struct event_struct {
 	struct list_head list;
 	char *name;
+	char *compat;
 	char *event;
 	char *desc;
 	char *long_desc;
@@ -421,11 +442,12 @@ static void free_arch_std_events(void)
 	}
 }
 
-static int save_arch_std_events(void *data, char *name, char *event,
-				char *desc, char *long_desc, char *pmu,
-				char *unit, char *perpkg, char *metric_expr,
-				char *metric_name, char *metric_group,
-				char *deprecated, char *metric_constraint)
+static int save_arch_std_events(void *data, char *name, char *compat,
+				char *event, char *desc, char *long_desc,
+				char *pmu, char *unit, char *perpkg,
+				char *metric_expr, char *metric_name,
+				char *metric_group, char *deprecated,
+				char *metric_constraint)
 {
 	struct event_struct *es;
 
@@ -513,12 +535,11 @@ try_fixup(const char *fn, char *arch_std, char **event, char **desc,
 
 /* Call func with each event in the json file */
 int json_events(const char *fn,
-	  int (*func)(void *data, char *name, char *event, char *desc,
-		      char *long_desc,
-		      char *pmu, char *unit, char *perpkg,
-		      char *metric_expr,
-		      char *metric_name, char *metric_group,
-		      char *deprecated, char *metric_constraint),
+	  int (*func)(void *data, char *name, char *compat, char *event,
+		      char *desc, char *long_desc, char *pmu, char *unit,
+		      char *perpkg, char *metric_expr, char *metric_name,
+		      char *metric_group, char *deprecated,
+		      char *metric_constraint),
 	  void *data)
 {
 	int err;
@@ -537,7 +558,7 @@ int json_events(const char *fn,
 	EXPECT(tokens->type == JSMN_ARRAY, tokens, "expected top level array");
 	tok = tokens + 1;
 	for (i = 0; i < tokens->size; i++) {
-		char *event = NULL, *desc = NULL, *name = NULL;
+		char *event = NULL, *desc = NULL, *name = NULL, *compat = NULL;
 		char *long_desc = NULL;
 		char *extra_desc = NULL;
 		char *pmu = NULL;
@@ -584,6 +605,8 @@ int json_events(const char *fn,
 				free(code);
 			} else if (json_streq(map, field, "EventName")) {
 				addfield(map, &name, "", "", val);
+			} else if (json_streq(map, field, "Compat")) {
+				addfield(map, &compat, "", "", val);
 			} else if (json_streq(map, field, "BriefDescription")) {
 				addfield(map, &desc, "", "", val);
 				fixdesc(desc);
@@ -680,13 +703,15 @@ int json_events(const char *fn,
 			if (err)
 				goto free_strings;
 		}
-		err = func(data, name, real_event(name, event), desc, long_desc,
-			   pmu, unit, perpkg, metric_expr, metric_name,
-			   metric_group, deprecated, metric_constraint);
+		err = func(data, name, compat, real_event(name, event), desc,
+			   long_desc, pmu, unit, perpkg, metric_expr,
+			   metric_name, metric_group, deprecated,
+			   metric_constraint);
 free_strings:
+		free(name);
+		free(compat);
 		free(event);
 		free(desc);
-		free(name);
 		free(long_desc);
 		free(extra_desc);
 		free(pmu);
@@ -750,6 +775,25 @@ static char *file_name_to_table_name(char *fname)
 	return tblname;
 }
 
+static bool is_sys_dir(char *fname)
+{
+	char *pos;
+
+	while (true) {
+		pos = strchr(fname, '/');
+
+		if (!pos) {
+			if (!strcmp(fname, "sys"))
+				return true;
+			return false;
+		}
+
+		fname = pos + 1;
+	}
+
+	return false;
+}
+
 static void print_mapping_table_prefix(FILE *outfp)
 {
 	fprintf(outfp, "struct pmu_events_map pmu_events_map[] = {\n");
@@ -784,6 +828,23 @@ static void print_mapping_test_table(FILE *outfp)
 	fprintf(outfp, "},\n");
 }
 
+static int process_system_event_tables(FILE *outfp)
+{
+	struct sys_event_table *sys_event_table;
+
+	fprintf(outfp, "struct pmu_sys_events pmu_sys_event_tables[] = {");
+
+	list_for_each_entry(sys_event_table, &sys_event_tables, list) {
+		fprintf(outfp, "\n\t{\n\t\t.table = %s,\n\t},",
+			sys_event_table->name);
+	}
+	fprintf(outfp, "\n\t{\n\t\t.table = 0\n\t},");
+
+	fprintf(outfp, "\n};\n");
+
+	return 0;
+}
+
 static int process_mapfile(FILE *outfp, char *fpath)
 {
 	int n = 16384;
@@ -1029,6 +1090,18 @@ static int process_one_file(const char *fpath, const struct stat *sb,
 			return -1;
 		}
 
+		if (is_sys_dir(bname)) {
+			struct sys_event_table *sys_event_table;
+
+			sys_event_table = malloc(sizeof(*sys_event_table));
+			if (!sys_event_table)
+				return -1;
+
+			sys_event_table->name = strdup(tblname);
+			list_add_tail(&sys_event_table->list,
+				      &sys_event_tables);
+		}
+
 		print_events_table_prefix(eventsfp, tblname);
 		return 0;
 	}
@@ -1180,7 +1253,6 @@ int main(int argc, char *argv[])
 	} else if (rc < 0) {
 		/* Make build fail */
 		fclose(eventsfp);
-		free_arch_std_events();
 		ret = 1;
 		goto out_free_mapfile;
 	} else if (rc) {
@@ -1206,27 +1278,31 @@ int main(int argc, char *argv[])
 	if (close_table)
 		print_events_table_suffix(eventsfp);
 
-	if (!mapfile) {
-		pr_info("%s: No CPU->JSON mapping?\n", prog);
-		goto empty_map;
+	if (mapfile) {
+		if (process_mapfile(eventsfp, mapfile)) {
+			pr_err("%s: Error processing mapfile %s\n", prog,
+			       mapfile);
+			/* Make build fail */
+			fclose(eventsfp);
+			ret = 1;
+		}
+	} else {
+		pr_err("%s: No CPU->JSON mapping?\n", prog);
 	}
 
-	if (process_mapfile(eventsfp, mapfile)) {
-		pr_info("%s: Error processing mapfile %s\n", prog, mapfile);
-		/* Make build fail */
+	if (process_system_event_tables(eventsfp)) {
 		fclose(eventsfp);
-		free_arch_std_events();
 		ret = 1;
 	}
 
-
 	goto out_free_mapfile;
 
 empty_map:
 	fclose(eventsfp);
 	create_empty_mapping(output_file);
-	free_arch_std_events();
 out_free_mapfile:
+	free_arch_std_events();
+	free_sys_event_tables();
 	free(mapfile);
 	return ret;
 }
diff --git a/tools/perf/pmu-events/jevents.h b/tools/perf/pmu-events/jevents.h
index 2afc8304529e..7e324b210747 100644
--- a/tools/perf/pmu-events/jevents.h
+++ b/tools/perf/pmu-events/jevents.h
@@ -3,12 +3,11 @@
 #define JEVENTS_H 1
 
 int json_events(const char *fn,
-		int (*func)(void *data, char *name, char *event, char *desc,
-				char *long_desc,
-				char *pmu,
-				char *unit, char *perpkg, char *metric_expr,
-				char *metric_name, char *metric_group,
-				char *deprecated, char *metric_constraint),
+		int (*func)(void *data, char *name, char *compat, char *event,
+			    char *desc, char *long_desc, char *pmu, char *unit,
+			    char *perpkg, char *metric_expr, char *metric_name,
+			    char *metric_group, char *deprecated,
+			    char *metric_constraint),
 		void *data);
 char *get_cpu_str(void);
 
diff --git a/tools/perf/pmu-events/pmu-events.h b/tools/perf/pmu-events/pmu-events.h
index 53e76d5d5b37..cc6de8c5af49 100644
--- a/tools/perf/pmu-events/pmu-events.h
+++ b/tools/perf/pmu-events/pmu-events.h
@@ -7,6 +7,7 @@
  */
 struct pmu_event {
 	const char *name;
+	const char *compat;
 	const char *event;
 	const char *desc;
 	const char *topic;
@@ -37,10 +38,15 @@ struct pmu_events_map {
 	struct pmu_event *table;
 };
 
+struct pmu_sys_events {
+	struct pmu_event *table;
+};
+
 /*
  * Global table mapping each known CPU for the architecture to its
  * table of PMU events.
  */
 extern struct pmu_events_map pmu_events_map[];
+extern struct pmu_sys_events pmu_sys_event_tables[];
 
 #endif
-- 
2.16.4

[RFC PATCH v2 04/13] perf vendor events arm64: Relocate hip08 events

[John Garry]

Relocate the core events JSONs to match to future structure, which will
have separate folders for CPU and system events.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 .../pmu-events/arch/arm64/hisilicon/hip08/{ => cpu}/core-imp-def.json   | 0
 .../pmu-events/arch/arm64/hisilicon/hip08/{ => cpu}/uncore-ddrc.json    | 0
 .../pmu-events/arch/arm64/hisilicon/hip08/{ => cpu}/uncore-hha.json     | 0
 .../pmu-events/arch/arm64/hisilicon/hip08/{ => cpu}/uncore-l3c.json     | 0
 tools/perf/pmu-events/arch/arm64/mapfile.csv                            | 2 +-
 5 files changed, 1 insertion(+), 1 deletion(-)
 rename tools/perf/pmu-events/arch/arm64/hisilicon/hip08/{ => cpu}/core-imp-def.json (100%)
 rename tools/perf/pmu-events/arch/arm64/hisilicon/hip08/{ => cpu}/uncore-ddrc.json (100%)
 rename tools/perf/pmu-events/arch/arm64/hisilicon/hip08/{ => cpu}/uncore-hha.json (100%)
 rename tools/perf/pmu-events/arch/arm64/hisilicon/hip08/{ => cpu}/uncore-l3c.json (100%)

diff --git a/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/core-imp-def.json b/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/cpu/core-imp-def.json
similarity index 100%
rename from tools/perf/pmu-events/arch/arm64/hisilicon/hip08/core-imp-def.json
rename to tools/perf/pmu-events/arch/arm64/hisilicon/hip08/cpu/core-imp-def.json
diff --git a/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/uncore-ddrc.json b/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/cpu/uncore-ddrc.json
similarity index 100%
rename from tools/perf/pmu-events/arch/arm64/hisilicon/hip08/uncore-ddrc.json
rename to tools/perf/pmu-events/arch/arm64/hisilicon/hip08/cpu/uncore-ddrc.json
diff --git a/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/uncore-hha.json b/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/cpu/uncore-hha.json
similarity index 100%
rename from tools/perf/pmu-events/arch/arm64/hisilicon/hip08/uncore-hha.json
rename to tools/perf/pmu-events/arch/arm64/hisilicon/hip08/cpu/uncore-hha.json
diff --git a/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/uncore-l3c.json b/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/cpu/uncore-l3c.json
similarity index 100%
rename from tools/perf/pmu-events/arch/arm64/hisilicon/hip08/uncore-l3c.json
rename to tools/perf/pmu-events/arch/arm64/hisilicon/hip08/cpu/uncore-l3c.json
diff --git a/tools/perf/pmu-events/arch/arm64/mapfile.csv b/tools/perf/pmu-events/arch/arm64/mapfile.csv
index 0d609149b82a..c92cb3b519fc 100644
--- a/tools/perf/pmu-events/arch/arm64/mapfile.csv
+++ b/tools/perf/pmu-events/arch/arm64/mapfile.csv
@@ -20,5 +20,5 @@
 0x00000000410fd0c0,v1,arm/cortex-a76-n1,core
 0x00000000420f5160,v1,cavium/thunderx2,core
 0x00000000430f0af0,v1,cavium/thunderx2,core
-0x00000000480fd010,v1,hisilicon/hip08,core
+0x00000000480fd010,v1,hisilicon/hip08/cpu,core
 0x00000000500f0000,v1,ampere/emag,core
-- 
2.16.4

[RFC PATCH v2 05/13] perf vendor events arm64: Add Architected events smmuv3-pmcg.json

[John Garry]

Add JSON for Architected events from [0], Section 10.3 .

[0] https://static.docs.arm.com/ihi0070/a/IHI_0070A_SMMUv3.pdf

Signed-off-by: John Garry <john.garry@huawei.com>
---
 tools/perf/pmu-events/arch/arm64/smmuv3-pmcg.json | 58 +++++++++++++++++++++++
 tools/perf/pmu-events/jevents.c                   |  2 +
 2 files changed, 60 insertions(+)
 create mode 100644 tools/perf/pmu-events/arch/arm64/smmuv3-pmcg.json

diff --git a/tools/perf/pmu-events/arch/arm64/smmuv3-pmcg.json b/tools/perf/pmu-events/arch/arm64/smmuv3-pmcg.json
new file mode 100644
index 000000000000..7ceb2b4372fa
--- /dev/null
+++ b/tools/perf/pmu-events/arch/arm64/smmuv3-pmcg.json
@@ -0,0 +1,58 @@
+[
+    {
+        "PublicDescription": "Clock cycles",
+        "EventCode": "0x00",
+        "EventName": "smmuv3_pmcg.CYCLES",
+        "BriefDescription": "Clock cycles"
+        "Unit": "smmuv3_pmcg",
+    },
+    {
+        "PublicDescription": "Transaction",
+        "EventCode": "0x01",
+        "EventName": "smmuv3_pmcg.TRANSACTION",
+        "BriefDescription": "Transaction"
+        "Unit": "smmuv3_pmcg",
+    },
+    {
+        "PublicDescription": "TLB miss caused by incomingtransaction or (ATS or non-ATS) translation request",
+        "EventCode": "0x02",
+        "EventName": "smmuv3_pmcg.TLB_MISS",
+        "BriefDescription": "TLB miss caused by incomingtransaction or (ATS or non-ATS) translation request"
+        "Unit": "smmuv3_pmcg",
+    },
+    {
+        "PublicDescription": "Configuration cache miss caused by transaction or(ATS or non-ATS)translation request",
+        "EventCode": "0x03",
+        "EventName": "smmuv3_pmcg.CONFIG_CACHE_MISS",
+        "BriefDescription": "Configuration cache miss caused by transaction or(ATS or non-ATS)translation request"
+        "Unit": "smmuv3_pmcg",
+    },
+    {
+        "PublicDescription": "Translation table walk access",
+        "EventCode": "0x04",
+        "EventName": "smmuv3_pmcg.TRANS_TABLE_WALK_ACCESS",
+        "BriefDescription": "Translation table walk access"
+        "Unit": "smmuv3_pmcg",
+    },
+    {
+        "PublicDescription": "Configuration structure access",
+        "EventCode": "0x05",
+        "EventName": "smmuv3_pmcg.CONFIG_STRUCT_ACCESS",
+        "BriefDescription": "Configuration structure access"
+        "Unit": "smmuv3_pmcg",
+    },
+    {
+        "PublicDescription": "PCIe ATS Translation Request received",
+        "EventCode": "0x06",
+        "EventName": "smmuv3_pmcg.PCIE_ATS_TRANS_RQ",
+        "BriefDescription": "PCIe ATS Translation Request received"
+        "Unit": "smmuv3_pmcg",
+    },
+    {
+        "PublicDescription": "PCIe ATS Translated Transaction passed through SMMU",
+        "EventCode": "0x07",
+        "EventName": "smmuv3_pmcg.PCIE_ATS_TRANS_PASSED",
+        "BriefDescription": "PCIe ATS Translated Transaction passed through SMMU"
+        "Unit": "smmuv3_pmcg",
+    }
+]
diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
index acb6b77bddc0..76a84ec2ffc8 100644
--- a/tools/perf/pmu-events/jevents.c
+++ b/tools/perf/pmu-events/jevents.c
@@ -256,6 +256,8 @@ static struct map {
 	{ "hisi_sccl,ddrc", "hisi_sccl,ddrc" },
 	{ "hisi_sccl,hha", "hisi_sccl,hha" },
 	{ "hisi_sccl,l3c", "hisi_sccl,l3c" },
+	/* it's not realistic to keep adding these, we need something more scalable ... */
+	{ "smmuv3_pmcg", "smmuv3_pmcg" },
 	{ "L3PMC", "amd_l3" },
 	{}
 };
-- 
2.16.4

[RFC PATCH v2 06/13] perf vendor events arm64: Add hip08 SMMUv3 PMCG events

[John Garry]

Add the SMMUv3 PMCG (Performance Monitor Event Group) events for hip08
platform.

This contains a mix of architected and IMP def events

Signed-off-by: John Garry <john.garry@huawei.com>
---
 .../arm64/hisilicon/hip08/sys/smmu-v3-pmcg.json    | 42 ++++++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100644 tools/perf/pmu-events/arch/arm64/hisilicon/hip08/sys/smmu-v3-pmcg.json

diff --git a/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/sys/smmu-v3-pmcg.json b/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/sys/smmu-v3-pmcg.json
new file mode 100644
index 000000000000..f2a1cb0332a6
--- /dev/null
+++ b/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/sys/smmu-v3-pmcg.json
@@ -0,0 +1,42 @@
+[
+   {
+	    "ArchStdEvent": "smmuv3_pmcg.CYCLES"
+	    "Compat": "hip08"
+   },
+   {
+	    "ArchStdEvent": "smmuv3_pmcg.TRANSACTION"
+	    "Compat": "hip08"
+   },
+   {
+	    "ArchStdEvent": "smmuv3_pmcg.TLB_MISS"
+	    "Compat": "hip08"
+   },
+   {
+	    "ArchStdEvent": "smmuv3_pmcg.CONFIG_CACHE_MISS"
+	    "Compat": "hip08"
+   },
+   {
+	    "ArchStdEvent": "smmuv3_pmcg.TRANS_TABLE_WALK_ACCESS"
+	    "Compat": "hip08"
+   },
+   {
+	    "ArchStdEvent": "smmuv3_pmcg.CONFIG_STRUCT_ACCESS"
+	    "Compat": "hip08"
+   },
+   {
+	    "ArchStdEvent": "smmuv3_pmcg.PCIE_ATS_TRANS_RQ"
+	    "Compat": "hip08"
+   },
+   {
+	    "ArchStdEvent": "smmuv3_pmcg.PCIE_ATS_TRANS_PASSED"
+	    "Compat": "hip08"
+   },
+   {
+	    "EventCode": "0x8a",
+	    "EventName": "smmuv3_pmcg.L1_TLB",
+	    "BriefDescription": "SMMUv3 PMCG L1 TABLE transation",
+	    "PublicDescription": "SMMUv3 PMCG L1 TABLE transation",
+	    "Unit": "smmuv3_pmcg",
+	    "Compat": "hip08"
+   },
+]
-- 
2.16.4

[RFC PATCH v2 07/13] perf pmu: Add pmu_id()

[John Garry]

Add a function to read the PMU id sysfs entry. We only do it for uncore
PMUs where this would be relevant.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 tools/perf/util/pmu.c | 36 ++++++++++++++++++++++++++++++++++++
 tools/perf/util/pmu.h |  1 +
 2 files changed, 37 insertions(+)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index ef6a63f3d386..6a67c6a28d08 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -594,6 +594,7 @@ static struct perf_cpu_map *__pmu_cpumask(const char *path)
  * Uncore PMUs have a "cpumask" file under sysfs. CPU PMUs (e.g. on arm/arm64)
  * may have a "cpus" file.
  */
+#define CPUS_TEMPLATE_ID	"%s/bus/event_source/devices/%s/identifier"
 #define CPUS_TEMPLATE_UNCORE	"%s/bus/event_source/devices/%s/cpumask"
 #define CPUS_TEMPLATE_CPU	"%s/bus/event_source/devices/%s/cpus"
 
@@ -632,6 +633,39 @@ static bool pmu_is_uncore(const char *name)
 	return file_available(path);
 }
 
+static char *pmu_id(const char *name)
+{
+	char path[PATH_MAX], *id;
+	const char *sysfs;
+	FILE *file;
+	int n;
+
+	sysfs = sysfs__mountpoint();
+	snprintf(path, PATH_MAX, CPUS_TEMPLATE_ID, sysfs, name);
+
+	id = malloc(PATH_MAX);
+	if (!id)
+		return NULL;
+
+	file = fopen(path, "r");
+	if (!file) {
+		free(id);
+		return NULL;
+	}
+
+	n = fscanf(file, "%s", id);
+
+	fclose(file);
+
+	if (!n) {
+		free(id);
+		return NULL;
+	}
+
+	return id;
+}
+
+
 /*
  *  PMU CORE devices have different name other than cpu in sysfs on some
  *  platforms.
@@ -844,6 +878,8 @@ static struct perf_pmu *pmu_lookup(const char *name)
 	pmu->name = strdup(name);
 	pmu->type = type;
 	pmu->is_uncore = pmu_is_uncore(name);
+	if (pmu->is_uncore)
+		pmu->id = pmu_id(name);
 	pmu->max_precise = pmu_max_precise(name);
 	pmu_add_cpu_aliases(&aliases, pmu);
 
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 5fb3f16828df..62ebca9481fe 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -24,6 +24,7 @@ struct perf_event_attr;
 
 struct perf_pmu {
 	char *name;
+	char *id;
 	__u32 type;
 	bool selectable;
 	bool is_uncore;
-- 
2.16.4

[RFC PATCH v2 08/13] perf pmu: Add pmu_add_sys_aliases()

[John Garry]

Add pmu_add_sys_aliases() to add system PMU events aliases.

For adding system PMU events, we iterate through all the events per
SoC event table in pmu_sys_event_tables[].

Matches must satisfy both:
- PMU identifier matches event "compat" value
- like uncore event matching, the event "Unit" member must match; this
  match under return value of pmu_uncore_alias_match() (maybe rename that
  function?)

Signed-off-by: John Garry <john.garry@huawei.com>
---
 tools/perf/util/pmu.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/pmu.h |  5 ++++
 2 files changed, 84 insertions(+)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 6a67c6a28d08..2f38465e8a73 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -827,6 +827,84 @@ static void pmu_add_cpu_aliases(struct list_head *head, struct perf_pmu *pmu)
 	pmu_add_cpu_aliases_map(head, pmu, map);
 }
 
+void pmu_for_each_sys_event(struct perf_pmu *pmu, pmu_sys_event_iter_fn fn,
+			    void *data)
+{
+	struct pmu_event *pe;
+	int i = 0;
+
+	if (!pmu->id)
+		return;
+
+	while (1) {
+		struct pmu_sys_events *event_table;
+		int j = 0;
+
+		event_table = &pmu_sys_event_tables[i++];
+
+		if (!event_table->table)
+			break;
+
+		while (1) {
+			int ret;
+
+			pe = &event_table->table[j++];
+
+			if (!pe->name && !pe->metric_group && !pe->metric_name)
+				break;
+
+			ret = fn(pmu, pe, data);
+			if (ret)
+				break;
+		}
+	}
+}
+
+struct pmu_sys_event_iter_data {
+	struct list_head *head;
+};
+
+static int pmu_add_sys_aliases_iter_fn(struct perf_pmu *pmu,
+				       struct pmu_event *pe, void *data)
+{
+	struct pmu_sys_event_iter_data *idata = data;
+
+	if (!pe->name) {
+		if (pe->metric_group || pe->metric_name)
+			return 0;
+		return -EINVAL;
+	}
+
+	if (!pe->compat || !pe->pmu)
+		return 0;
+
+	if (!strcmp(pmu->id, pe->compat) &&
+	    pmu_uncore_alias_match(pe->pmu, pmu->name)) {
+		__perf_pmu__new_alias(idata->head, NULL,
+				      (char *)pe->name,
+				      (char *)pe->desc,
+				      (char *)pe->event,
+				      (char *)pe->long_desc,
+				      (char *)pe->topic,
+				      (char *)pe->unit,
+				      (char *)pe->perpkg,
+				      (char *)pe->metric_expr,
+				      (char *)pe->metric_name,
+				      (char *)pe->deprecated);
+	}
+
+	return 0;
+}
+
+static void pmu_add_sys_aliases(struct list_head *head, struct perf_pmu *pmu)
+{
+	struct pmu_sys_event_iter_data idata = {
+		.head = head,
+	};
+
+	pmu_for_each_sys_event(pmu, pmu_add_sys_aliases_iter_fn, &idata);
+}
+
 struct perf_event_attr * __weak
 perf_pmu__get_default_config(struct perf_pmu *pmu __maybe_unused)
 {
@@ -882,6 +960,7 @@ static struct perf_pmu *pmu_lookup(const char *name)
 		pmu->id = pmu_id(name);
 	pmu->max_precise = pmu_max_precise(name);
 	pmu_add_cpu_aliases(&aliases, pmu);
+	pmu_add_sys_aliases(&aliases, pmu);
 
 	INIT_LIST_HEAD(&pmu->format);
 	INIT_LIST_HEAD(&pmu->aliases);
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 62ebca9481fe..e4bc9d192236 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -106,6 +106,11 @@ void pmu_add_cpu_aliases_map(struct list_head *head, struct perf_pmu *pmu,
 struct pmu_events_map *perf_pmu__find_map(struct perf_pmu *pmu);
 bool pmu_uncore_alias_match(const char *pmu_name, const char *name);
 
+typedef int (*pmu_sys_event_iter_fn)(struct perf_pmu *, struct pmu_event *pe,
+				     void *data);
+
+void pmu_for_each_sys_event(struct perf_pmu *pmu, pmu_sys_event_iter_fn fn,
+			    void *data);
 int perf_pmu__convert_scale(const char *scale, char **end, double *sval);
 
 #endif /* __PMU_H */
-- 
2.16.4

[RFC PATCH v2 09/13] perf vendor events: Add JSON metrics for imx8mm DDR Perf

[John Garry]

From: Joakim Zhang <qiangqing.zhang@nxp.com>

Add JSON metrics for imx8mm DDR Perf.

Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
Signed-off-by: John Garry <john.garry@huawei.com>
---
 .../arch/arm64/freescale/imx8mm/sys/ddrc.json      | 39 ++++++++++++++++++++++
 .../arch/arm64/freescale/imx8mm/sys/metrics.json   | 18 ++++++++++
 tools/perf/pmu-events/jevents.c                    |  1 +
 3 files changed, 58 insertions(+)
 create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/ddrc.json
 create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json

diff --git a/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/ddrc.json b/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/ddrc.json
new file mode 100644
index 000000000000..8a3dae61a48f
--- /dev/null
+++ b/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/ddrc.json
@@ -0,0 +1,39 @@
+[
+   {
+           "BriefDescription": "ddr cycles event",
+           "EventCode": "0x00",
+           "EventName": "imx8_ddr.cycles",
+           "Unit": "imx8_ddr",
+           "Compat": "i.mx8mm"
+   },
+   {
+           "BriefDescription": "ddr read-cycles event",
+           "EventCode": "0x2a",
+           "EventName": "imx8_ddr.read_cycles",
+           "Unit": "imx8_ddr",
+           "Compat": "i.mx8mm"
+   },
+   {
+           "BriefDescription": "ddr write-cycles event",
+           "EventCode": "0x2b",
+           "EventName": "imx8_ddr.write_cycles",
+           "Unit": "imx8_ddr",
+           "Compat": "i.mx8mm"
+   },
+   {
+           "BriefDescription": "ddr read event",
+           "EventCode": "0x35",
+           "EventName": "imx8_ddr.read",
+           "Unit": "imx8_ddr",
+           "Compat": "i.mx8mm"
+   },
+   {
+           "BriefDescription": "ddr write event",
+           "EventCode": "0x38",
+           "EventName": "imx8_ddr.write",
+           "Unit": "imx8_ddr",
+           "Compat": "i.mx8mm"
+   }
+]
+
+
diff --git a/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json b/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json
new file mode 100644
index 000000000000..b6a776ca7cc2
--- /dev/null
+++ b/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json
@@ -0,0 +1,18 @@
+[
+   {
+	    "BriefDescription": "bytes all masters read from ddr based on read-cycles event",
+	    "MetricName": "imx8mm_ddr_read.all",
+	    "MetricExpr": "imx8_ddr.read_cycles * 4 * 4",
+	    "ScaleUnit": "9.765625e-4MB",
+	    "Unit": "imx8_ddr",
+	    "Compat": "i.mx8mm"
+    },
+   {
+	    "BriefDescription": "bytes all masters write to ddr based on write-cycles event",
+	    "MetricName": "imx8mm_ddr_write.all",
+	    "MetricExpr": "imx8_ddr.write_cycles * 4 * 4",
+	    "ScaleUnit": "9.765625e-4MB",
+	    "Unit": "imx8_ddr",
+	    "Compat": "i.mx8mm"
+    }
+]
\ No newline at end of file
diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
index 76a84ec2ffc8..efdade0194af 100644
--- a/tools/perf/pmu-events/jevents.c
+++ b/tools/perf/pmu-events/jevents.c
@@ -257,6 +257,7 @@ static struct map {
 	{ "hisi_sccl,hha", "hisi_sccl,hha" },
 	{ "hisi_sccl,l3c", "hisi_sccl,l3c" },
 	/* it's not realistic to keep adding these, we need something more scalable ... */
+	{ "imx8_ddr", "imx8_ddr" },
 	{ "smmuv3_pmcg", "smmuv3_pmcg" },
 	{ "L3PMC", "amd_l3" },
 	{}
-- 
2.16.4

[RFC PATCH v2 10/13] perf metricgroup: Split up metricgroup__add_metric()

[John Garry]

To aid supporting system event metric groups, break up the function
metricgroup__add_metric() into a part which iterates metrics and a part
which actually "adds" the metric.

No functional change intended.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 tools/perf/util/metricgroup.c | 75 ++++++++++++++++++++++++++-----------------
 1 file changed, 45 insertions(+), 30 deletions(-)

diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 926449a7cdbf..d1033756a1bc 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -231,6 +231,12 @@ static bool match_metric(const char *n, const char *list)
 	return false;
 }
 
+static bool match_pe_metric(struct pmu_event *pe, const char *metric)
+{
+	return match_metric(pe->metric_group, metric) ||
+	       match_metric(pe->metric_name, metric);
+}
+
 struct mep {
 	struct rb_node nd;
 	const char *name;
@@ -485,6 +491,40 @@ static bool metricgroup__has_constraint(struct pmu_event *pe)
 	return false;
 }
 
+static int metricgroup__add_metric_pmu_event(struct pmu_event *pe,
+					     struct strbuf *events,
+					     struct list_head *group_list)
+{
+	const char **ids;
+	int idnum;
+	struct egroup *eg;
+
+	pr_debug("metric expr %s for %s\n", pe->metric_expr, pe->metric_name);
+
+	if (expr__find_other(pe->metric_expr, NULL, &ids, &idnum) < 0)
+		return 0;
+
+	if (events->len > 0)
+		strbuf_addf(events, ",");
+
+	if (metricgroup__has_constraint(pe))
+		metricgroup__add_metric_non_group(events, ids, idnum);
+	else
+		metricgroup__add_metric_weak_group(events, ids, idnum);
+
+	eg = malloc(sizeof(*eg));
+	if (!eg)
+		return -ENOMEM;
+	eg->ids = ids;
+	eg->idnum = idnum;
+	eg->metric_name = pe->metric_name;
+	eg->metric_expr = pe->metric_expr;
+	eg->metric_unit = pe->unit;
+	list_add_tail(&eg->nd, group_list);
+
+	return 0;
+}
+
 static int metricgroup__add_metric(const char *metric, struct strbuf *events,
 				   struct list_head *group_list)
 {
@@ -502,37 +542,12 @@ static int metricgroup__add_metric(const char *metric, struct strbuf *events,
 			break;
 		if (!pe->metric_expr)
 			continue;
-		if (match_metric(pe->metric_group, metric) ||
-		    match_metric(pe->metric_name, metric)) {
-			const char **ids;
-			int idnum;
-			struct egroup *eg;
-
-			pr_debug("metric expr %s for %s\n", pe->metric_expr, pe->metric_name);
 
-			if (expr__find_other(pe->metric_expr,
-					     NULL, &ids, &idnum) < 0)
-				continue;
-			if (events->len > 0)
-				strbuf_addf(events, ",");
-
-			if (metricgroup__has_constraint(pe))
-				metricgroup__add_metric_non_group(events, ids, idnum);
-			else
-				metricgroup__add_metric_weak_group(events, ids, idnum);
-
-			eg = malloc(sizeof(struct egroup));
-			if (!eg) {
-				ret = -ENOMEM;
-				break;
-			}
-			eg->ids = ids;
-			eg->idnum = idnum;
-			eg->metric_name = pe->metric_name;
-			eg->metric_expr = pe->metric_expr;
-			eg->metric_unit = pe->unit;
-			list_add_tail(&eg->nd, group_list);
-			ret = 0;
+		if (match_pe_metric(pe, metric)) {
+			ret = metricgroup__add_metric_pmu_event(pe, events,
+								group_list);
+			if (ret)
+				return ret;
 		}
 	}
 	return ret;
-- 
2.16.4

[RFC PATCH v2 11/13] perf metricgroup: Split up metricgroup__print()

[John Garry]

To aid supporting system event metric groups, break up the function
metricgroup__print() into a part which iterates metrics and a part
which actually "prints" the metric.

No functional change intended.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 tools/perf/util/metricgroup.c | 117 ++++++++++++++++++++++++------------------
 1 file changed, 66 insertions(+), 51 deletions(-)

diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index d1033756a1bc..31e97e24c2b0 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -315,6 +315,68 @@ static void metricgroup__print_strlist(struct strlist *metrics, bool raw)
 		putchar('\n');
 }
 
+
+static void metricgroup__print_pmu_event(struct pmu_event *pe,
+					 bool metricgroups, char *filter,
+					 bool raw, bool details,
+					 struct rblist *groups,
+					 struct strlist *metriclist)
+{
+	const char *g;
+
+	g = pe->metric_group;
+	if (!g && pe->metric_name) {
+		if (pe->name)
+			return;
+		g = "No_group";
+	}
+
+	if (g) {
+		char *omg;
+		char *mg = strdup(g);
+
+		if (!mg)
+			return;
+		omg = mg;
+		while ((g = strsep(&mg, ";")) != NULL) {
+			struct mep *me;
+			char *s;
+
+			g = skip_spaces(g);
+			if (*g == 0)
+				g = "No_group";
+			if (filter && !strstr(g, filter))
+				continue;
+			if (raw)
+				s = (char *)pe->metric_name;
+			else {
+				if (asprintf(&s, "%s\n%*s%s]", pe->metric_name,
+					     8, "[", pe->desc) < 0)
+					return;
+
+				if (details) {
+					if (asprintf(&s, "%s\n%*s%s]", s, 8,
+						     "[", pe->metric_expr) < 0)
+						return;
+				}
+			}
+
+			if (!s)
+				continue;
+
+			if (!metricgroups) {
+				strlist__add(metriclist, s);
+			} else {
+				me = mep_lookup(groups, g);
+				if (!me)
+					continue;
+				strlist__add(me->metrics, s);
+			}
+		}
+		free(omg);
+	}
+}
+
 void metricgroup__print(bool metrics, bool metricgroups, char *filter,
 			bool raw, bool details)
 {
@@ -339,63 +401,15 @@ void metricgroup__print(bool metrics, bool metricgroups, char *filter,
 	groups.node_cmp = mep_cmp;
 	groups.node_delete = mep_delete;
 	for (i = 0; ; i++) {
-		const char *g;
 		pe = &map->table[i];
 
 		if (!pe->name && !pe->metric_group && !pe->metric_name)
 			break;
 		if (!pe->metric_expr)
 			continue;
-		g = pe->metric_group;
-		if (!g && pe->metric_name) {
-			if (pe->name)
-				continue;
-			g = "No_group";
-		}
-		if (g) {
-			char *omg;
-			char *mg = strdup(g);
-
-			if (!mg)
-				return;
-			omg = mg;
-			while ((g = strsep(&mg, ";")) != NULL) {
-				struct mep *me;
-				char *s;
-
-				g = skip_spaces(g);
-				if (*g == 0)
-					g = "No_group";
-				if (filter && !strstr(g, filter))
-					continue;
-				if (raw)
-					s = (char *)pe->metric_name;
-				else {
-					if (asprintf(&s, "%s\n%*s%s]",
-						     pe->metric_name, 8, "[", pe->desc) < 0)
-						return;
-
-					if (details) {
-						if (asprintf(&s, "%s\n%*s%s]",
-							     s, 8, "[", pe->metric_expr) < 0)
-							return;
-					}
-				}
-
-				if (!s)
-					continue;
 
-				if (!metricgroups) {
-					strlist__add(metriclist, s);
-				} else {
-					me = mep_lookup(&groups, g);
-					if (!me)
-						continue;
-					strlist__add(me->metrics, s);
-				}
-			}
-			free(omg);
-		}
+		metricgroup__print_pmu_event(pe, metricgroups, filter, raw,
+					     details, &groups, metriclist);
 	}
 
 	if (metricgroups && !raw)
@@ -407,7 +421,8 @@ void metricgroup__print(bool metrics, bool metricgroups, char *filter,
 		struct mep *me = container_of(node, struct mep, nd);
 
 		if (metricgroups)
-			printf("%s%s%s", me->name, metrics && !raw ? ":" : "", raw ? " " : "\n");
+			printf("%s%s%s", me->name, metrics && !raw ? ":" : "",
+			       raw ? " " : "\n");
 		if (metrics)
 			metricgroup__print_strlist(me->metrics, raw);
 		next = rb_next(node);
-- 
2.16.4

[RFC PATCH v2 12/13] perf metricgroup: Support printing metric groups for system PMUs

[John Garry]

Currently only metricgroups for core- or uncore-based events is supported.
Extend this for system events.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 tools/perf/util/metricgroup.c | 61 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 57 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 31e97e24c2b0..81a6aa04a601 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -377,19 +377,55 @@ static void metricgroup__print_pmu_event(struct pmu_event *pe,
 	}
 }
 
+struct metricgroup_print_sys_idata {
+	struct strlist *metriclist;
+	bool metricgroups;
+	char *filter;
+	bool raw;
+	bool details;
+	struct rblist *groups;
+};
+
+typedef int (*metricgroup_sys_event_iter_fn)(struct pmu_event *pe, void *);
+
+struct metricgroup_iter_data {
+	metricgroup_sys_event_iter_fn fn;
+	void *data;
+};
+
+static int metricgroup__sys_event_iter(struct perf_pmu *pmu,
+				       struct pmu_event *pe,
+				       void *data)
+{
+	struct metricgroup_iter_data *d = data;
+
+	if (!pe->metric_expr || !pe->compat || strcmp(pe->compat, pmu->id))
+		return 0;
+
+	return d->fn(pe, d->data);
+}
+
+static int metricgroup__print_sys_event_iter(struct pmu_event *pe, void *data)
+{
+	struct metricgroup_print_sys_idata *d = data;
+
+	metricgroup__print_pmu_event(pe, d->metricgroups, d->filter, d->raw,
+				     d->details, d->groups, d->metriclist);
+
+	return 0;
+}
+
 void metricgroup__print(bool metrics, bool metricgroups, char *filter,
 			bool raw, bool details)
 {
 	struct pmu_events_map *map = perf_pmu__find_map(NULL);
 	struct pmu_event *pe;
+	struct perf_pmu *pmu = NULL;
 	int i;
 	struct rblist groups;
 	struct rb_node *node, *next;
 	struct strlist *metriclist = NULL;
 
-	if (!map)
-		return;
-
 	if (!metricgroups) {
 		metriclist = strlist__new(NULL, NULL);
 		if (!metriclist)
@@ -400,7 +436,7 @@ void metricgroup__print(bool metrics, bool metricgroups, char *filter,
 	groups.node_new = mep_new;
 	groups.node_cmp = mep_cmp;
 	groups.node_delete = mep_delete;
-	for (i = 0; ; i++) {
+	for (i = 0; map; i++) {
 		pe = &map->table[i];
 
 		if (!pe->name && !pe->metric_group && !pe->metric_name)
@@ -412,6 +448,23 @@ void metricgroup__print(bool metrics, bool metricgroups, char *filter,
 					     details, &groups, metriclist);
 	}
 
+	while ((pmu = perf_pmu__scan(pmu)) != NULL) {
+		struct metricgroup_iter_data data = {
+			.fn = metricgroup__print_sys_event_iter,
+			.data = (void *) &(struct metricgroup_print_sys_idata){
+				.metriclist = metriclist,
+				.metricgroups = metricgroups,
+				.filter = filter,
+				.raw = raw,
+				.details = details,
+				.groups = &groups,
+			},
+		};
+
+		pmu_for_each_sys_event(pmu, metricgroup__sys_event_iter,
+				       &data);
+	}
+
 	if (metricgroups && !raw)
 		printf("\nMetric Groups:\n\n");
 	else if (metrics && !raw)
-- 
2.16.4

[RFC PATCH v2 13/13] perf metricgroup: Support adding metrics for system PMUs

[John Garry]

Currently only adding metrics for core- or uncore-based events is
supported. Extend this for system events.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 tools/perf/util/metricgroup.c | 40 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 36 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 81a6aa04a601..85bf5333e4c6 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -593,17 +593,34 @@ static int metricgroup__add_metric_pmu_event(struct pmu_event *pe,
 	return 0;
 }
 
+struct metricgroup_add_iter_data {
+	const char *metric;
+	struct strbuf *events;
+	struct list_head *group_list;
+	int *ret;
+};
+
+static int metricgroup__add_metric_sys_event_iter(struct pmu_event *pe,
+						  void *data)
+{
+	struct metricgroup_add_iter_data *d = data;
+
+	if (!match_pe_metric(pe, d->metric))
+		return 0;
+
+	return (*d->ret = metricgroup__add_metric_pmu_event(pe, d->events,
+							    d->group_list));
+}
+
 static int metricgroup__add_metric(const char *metric, struct strbuf *events,
 				   struct list_head *group_list)
 {
 	struct pmu_events_map *map = perf_pmu__find_map(NULL);
+	struct perf_pmu *pmu = NULL;
 	struct pmu_event *pe;
 	int i, ret = -EINVAL;
 
-	if (!map)
-		return 0;
-
-	for (i = 0; ; i++) {
+	for (i = 0; map; i++) {
 		pe = &map->table[i];
 
 		if (!pe->name && !pe->metric_group && !pe->metric_name)
@@ -618,6 +635,21 @@ static int metricgroup__add_metric(const char *metric, struct strbuf *events,
 				return ret;
 		}
 	}
+
+	while ((pmu = perf_pmu__scan(pmu)) != NULL) {
+		struct metricgroup_iter_data data = {
+			.fn = metricgroup__add_metric_sys_event_iter,
+			.data = (void *) &(struct metricgroup_add_iter_data){
+				.metric = metric,
+				.events = events,
+				.group_list = group_list,
+				.ret = &ret,
+			},
+		};
+
+		pmu_for_each_sys_event(pmu, metricgroup__sys_event_iter, &data);
+	}
+
 	return ret;
 }
 
-- 
2.16.4

Re: [RFC PATCH v2 05/13] perf vendor events arm64: Add Architected events smmuv3-pmcg.json

[Ian Rogers]

On Fri, Apr 17, 2020 at 3:45 AM John Garry <john.garry@huawei.com> wrote:
>
> Add JSON for Architected events from [0], Section 10.3 .
>
> [0] https://static.docs.arm.com/ihi0070/a/IHI_0070A_SMMUv3.pdf
>
> Signed-off-by: John Garry <john.garry@huawei.com>
> ---
>  tools/perf/pmu-events/arch/arm64/smmuv3-pmcg.json | 58 +++++++++++++++++++++++
>  tools/perf/pmu-events/jevents.c                   |  2 +
>  2 files changed, 60 insertions(+)
>  create mode 100644 tools/perf/pmu-events/arch/arm64/smmuv3-pmcg.json
>
> diff --git a/tools/perf/pmu-events/arch/arm64/smmuv3-pmcg.json b/tools/perf/pmu-events/arch/arm64/smmuv3-pmcg.json
> new file mode 100644
> index 000000000000..7ceb2b4372fa
> --- /dev/null
> +++ b/tools/perf/pmu-events/arch/arm64/smmuv3-pmcg.json
> @@ -0,0 +1,58 @@
> +[
> +    {
> +        "PublicDescription": "Clock cycles",
> +        "EventCode": "0x00",
> +        "EventName": "smmuv3_pmcg.CYCLES",
> +        "BriefDescription": "Clock cycles"
> +        "Unit": "smmuv3_pmcg",
> +    },
> +    {
> +        "PublicDescription": "Transaction",
> +        "EventCode": "0x01",
> +        "EventName": "smmuv3_pmcg.TRANSACTION",
> +        "BriefDescription": "Transaction"
> +        "Unit": "smmuv3_pmcg",
> +    },
> +    {
> +        "PublicDescription": "TLB miss caused by incomingtransaction or (ATS or non-ATS) translation request",

It looks like a space was missed in "incomingtransaction".

> +        "EventCode": "0x02",
> +        "EventName": "smmuv3_pmcg.TLB_MISS",
> +        "BriefDescription": "TLB miss caused by incomingtransaction or (ATS or non-ATS) translation request"

And here.

Thanks,
Ian

> +        "Unit": "smmuv3_pmcg",
> +    },
> +    {
> +        "PublicDescription": "Configuration cache miss caused by transaction or(ATS or non-ATS)translation request",
> +        "EventCode": "0x03",
> +        "EventName": "smmuv3_pmcg.CONFIG_CACHE_MISS",
> +        "BriefDescription": "Configuration cache miss caused by transaction or(ATS or non-ATS)translation request"
> +        "Unit": "smmuv3_pmcg",
> +    },
> +    {
> +        "PublicDescription": "Translation table walk access",
> +        "EventCode": "0x04",
> +        "EventName": "smmuv3_pmcg.TRANS_TABLE_WALK_ACCESS",
> +        "BriefDescription": "Translation table walk access"
> +        "Unit": "smmuv3_pmcg",
> +    },
> +    {
> +        "PublicDescription": "Configuration structure access",
> +        "EventCode": "0x05",
> +        "EventName": "smmuv3_pmcg.CONFIG_STRUCT_ACCESS",
> +        "BriefDescription": "Configuration structure access"
> +        "Unit": "smmuv3_pmcg",
> +    },
> +    {
> +        "PublicDescription": "PCIe ATS Translation Request received",
> +        "EventCode": "0x06",
> +        "EventName": "smmuv3_pmcg.PCIE_ATS_TRANS_RQ",
> +        "BriefDescription": "PCIe ATS Translation Request received"
> +        "Unit": "smmuv3_pmcg",
> +    },
> +    {
> +        "PublicDescription": "PCIe ATS Translated Transaction passed through SMMU",
> +        "EventCode": "0x07",
> +        "EventName": "smmuv3_pmcg.PCIE_ATS_TRANS_PASSED",
> +        "BriefDescription": "PCIe ATS Translated Transaction passed through SMMU"
> +        "Unit": "smmuv3_pmcg",
> +    }
> +]
> diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
> index acb6b77bddc0..76a84ec2ffc8 100644
> --- a/tools/perf/pmu-events/jevents.c
> +++ b/tools/perf/pmu-events/jevents.c
> @@ -256,6 +256,8 @@ static struct map {
>         { "hisi_sccl,ddrc", "hisi_sccl,ddrc" },
>         { "hisi_sccl,hha", "hisi_sccl,hha" },
>         { "hisi_sccl,l3c", "hisi_sccl,l3c" },
> +       /* it's not realistic to keep adding these, we need something more scalable ... */
> +       { "smmuv3_pmcg", "smmuv3_pmcg" },
>         { "L3PMC", "amd_l3" },
>         {}
>  };
> --
> 2.16.4
>

Re: [RFC PATCH v2 05/13] perf vendor events arm64: Add Architected events smmuv3-pmcg.json

[John Garry]

On 17/04/2020 16:13, Ian Rogers wrote:
>> +        "PublicDescription": "TLB miss caused by incomingtransaction or (ATS or non-ATS) translation request",
> It looks like a space was missed in "incomingtransaction".
> 
>> +        "EventCode": "0x02",
>> +        "EventName": "smmuv3_pmcg.TLB_MISS",
>> +        "BriefDescription": "TLB miss caused by incomingtransaction or (ATS or non-ATS) translation request"
> And here.

Right, a copy-and-paste formatting error.

Cheers,
john

> 
> Thanks,
> Ian
> 
>> +

RE: [RFC PATCH v2 09/13] perf vendor events: Add JSON metrics for imx8mm DDR Perf

[Joakim Zhang]

> -----Original Message-----
> From: John Garry <john.garry@huawei.com>
> Sent: 2020年4月17日 18:41
> To: peterz@infradead.org; mingo@redhat.com; acme@kernel.org;
> mark.rutland@arm.com; alexander.shishkin@linux.intel.com;
> jolsa@redhat.com; namhyung@kernel.org; will@kernel.org
> Cc: ak@linux.intel.com; linuxarm@huawei.com; linux-kernel@vger.kernel.org;
> Joakim Zhang <qiangqing.zhang@nxp.com>; irogers@google.com;
> robin.murphy@arm.com; zhangshaokun@hisilicon.com;
> linux-arm-kernel@lists.infradead.org; John Garry <john.garry@huawei.com>
> Subject: [RFC PATCH v2 09/13] perf vendor events: Add JSON metrics for
> imx8mm DDR Perf
> 
> From: Joakim Zhang <qiangqing.zhang@nxp.com>
> 
> Add JSON metrics for imx8mm DDR Perf.
> 
> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> Signed-off-by: John Garry <john.garry@huawei.com>
> ---
>  .../arch/arm64/freescale/imx8mm/sys/ddrc.json      | 39
> ++++++++++++++++++++++
>  .../arch/arm64/freescale/imx8mm/sys/metrics.json   | 18 ++++++++++
>  tools/perf/pmu-events/jevents.c                    |  1 +
>  3 files changed, 58 insertions(+)
>  create mode 100644
> tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/ddrc.json
>  create mode 100644
> tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json
> 
> diff --git
> a/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/ddrc.json
> b/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/ddrc.json
> new file mode 100644
> index 000000000000..8a3dae61a48f
> --- /dev/null
> +++ b/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/ddrc.json
> @@ -0,0 +1,39 @@
> +[
> +   {
> +           "BriefDescription": "ddr cycles event",
> +           "EventCode": "0x00",
> +           "EventName": "imx8_ddr.cycles",
> +           "Unit": "imx8_ddr",
> +           "Compat": "i.mx8mm"
> +   },
> +   {
> +           "BriefDescription": "ddr read-cycles event",
> +           "EventCode": "0x2a",
> +           "EventName": "imx8_ddr.read_cycles",
> +           "Unit": "imx8_ddr",
> +           "Compat": "i.mx8mm"
> +   },
> +   {
> +           "BriefDescription": "ddr write-cycles event",
> +           "EventCode": "0x2b",
> +           "EventName": "imx8_ddr.write_cycles",
> +           "Unit": "imx8_ddr",
> +           "Compat": "i.mx8mm"
> +   },
> +   {
> +           "BriefDescription": "ddr read event",
> +           "EventCode": "0x35",
> +           "EventName": "imx8_ddr.read",
> +           "Unit": "imx8_ddr",
> +           "Compat": "i.mx8mm"
> +   },
> +   {
> +           "BriefDescription": "ddr write event",
> +           "EventCode": "0x38",
> +           "EventName": "imx8_ddr.write",
> +           "Unit": "imx8_ddr",
> +           "Compat": "i.mx8mm"
> +   }
> +]
Hi John,

Tested from branch: private-topic-perf-5.7-sys-pmu-events-v2

DDR events test is okay on both 8MM and 8QM.


> diff --git
> a/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json
> b/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json
> new file mode 100644
> index 000000000000..b6a776ca7cc2
> --- /dev/null
> +++ b/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json
> @@ -0,0 +1,18 @@
> +[
> +   {
> +	    "BriefDescription": "bytes all masters read from ddr based on
> read-cycles event",
> +	    "MetricName": "imx8mm_ddr_read.all",
> +	    "MetricExpr": "imx8_ddr.read_cycles * 4 * 4",
> +	    "ScaleUnit": "9.765625e-4MB",
> +	    "Unit": "imx8_ddr",
> +	    "Compat": "i.mx8mm"
> +    },
> +   {
> +	    "BriefDescription": "bytes all masters write to ddr based on
> write-cycles event",
> +	    "MetricName": "imx8mm_ddr_write.all",
> +	    "MetricExpr": "imx8_ddr.write_cycles * 4 * 4",
> +	    "ScaleUnit": "9.765625e-4MB",
> +	    "Unit": "imx8_ddr",
> +	    "Compat": "i.mx8mm"
> +    }
> +]

However, it seems that there are small defects from metric.

Firstly, could you help change "ScaleUnit": "9.765625e-4MB" into "ScaleUnit": "9.765625e-4KB", this is a mistake.

Then, you can see that test is okay from 8MM. However, metric would add twice once time from 8QM which has two ddr perf(ddr0/ddr1), it looks incorrect.

8MM:
root@imx8mmevk:~# ./perf stat -v -a -I 1000 -M imx8mm_ddr_write.all
Using CPUID 0x00000000410fd030
metric expr imx8_ddr.write_cycles * 4 * 4 for imx8mm_ddr_write.all
found event imx8_ddr.write_cycles
adding {imx8_ddr.write_cycles}:W
imx8_ddr.write_cycles -> imx8_ddr0/event=0x2b/
imx8_ddr.write_cycles: 13153 1000495125 1000495125
#           time             counts unit events
     1.000476625              13153      imx8_ddr.write_cycles     #    205.5 MB  imx8mm_ddr_write.all
imx8_ddr.write_cycles: 3582 1000681375 1000681375
     2.001167750               3582      imx8_ddr.write_cycles     #     56.0 MB  imx8mm_ddr_write.all


8QM:
root@imx8qmmek:~# ./perf stat -v -a -I 1000 -M imx8qm_ddr_read.all
Using CPUID 0x00000000410fd030
metric expr imx8_ddr.read_cycles * 4 * 4 for imx8qm_ddr_read.all
found event imx8_ddr.read_cycles
metric expr imx8_ddr.read_cycles * 4 * 4 for imx8qm_ddr_read.all
found event imx8_ddr.read_cycles
adding {imx8_ddr.read_cycles}:W,{imx8_ddr.read_cycles}:W
imx8_ddr.read_cycles -> imx8_ddr0/event=0x2a/
imx8_ddr.read_cycles -> imx8_ddr1/event=0x2a/
imx8_ddr.read_cycles -> imx8_ddr0/event=0x2a/
imx8_ddr.read_cycles -> imx8_ddr1/event=0x2a/
imx8_ddr.read_cycles: 22748 1000378750 1000378750
imx8_ddr.read_cycles: 24640 1000376625 1000376625
imx8_ddr.read_cycles: 22800 1000375125 1000375125
imx8_ddr.read_cycles: 24616 1000372625 1000372625
#           time             counts unit events
     1.000377250              47388      imx8_ddr.read_cycles      #    740.4 MB  imx8qm_ddr_read.all
     1.000377250              47416      imx8_ddr.read_cycles
imx8_ddr.read_cycles: 32672 1000454375 1000454375
imx8_ddr.read_cycles: 37888 1000457250 1000457250
imx8_ddr.read_cycles: 32736 1000460250 1000460250
imx8_ddr.read_cycles: 38012 1000463000 1000463000
     2.000812375              70560      imx8_ddr.read_cycles      #   1102.5 MB  imx8qm_ddr_read.all
     2.000812375              70748      imx8_ddr.read_cycles


Best Regards,
Joakim Zhang
> \ No newline at end of file
> diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
> index 76a84ec2ffc8..efdade0194af 100644
> --- a/tools/perf/pmu-events/jevents.c
> +++ b/tools/perf/pmu-events/jevents.c
> @@ -257,6 +257,7 @@ static struct map {
>  	{ "hisi_sccl,hha", "hisi_sccl,hha" },
>  	{ "hisi_sccl,l3c", "hisi_sccl,l3c" },
>  	/* it's not realistic to keep adding these, we need something more
> scalable ... */
> +	{ "imx8_ddr", "imx8_ddr" },
>  	{ "smmuv3_pmcg", "smmuv3_pmcg" },
>  	{ "L3PMC", "amd_l3" },
>  	{}
> --
> 2.16.4

Re: [RFC PATCH v2 09/13] perf vendor events: Add JSON metrics for imx8mm DDR Perf

[John Garry]

On 20/04/2020 05:17, Joakim Zhang wrote:
> However, it seems that there are small defects from metric.
> 
> Firstly, could you help change "ScaleUnit": "9.765625e-4MB" into "ScaleUnit": "9.765625e-4KB", this is a mistake.

ok

> 
> Then, you can see that test is okay from 8MM. However, metric would add twice once time from 8QM which has two ddr perf(ddr0/ddr1), it looks incorrect.
> 
> 8MM:
> root@imx8mmevk:~# ./perf stat -v -a -I 1000 -M imx8mm_ddr_write.all
> Using CPUID 0x00000000410fd030
> metric expr imx8_ddr.write_cycles * 4 * 4 for imx8mm_ddr_write.all
> found event imx8_ddr.write_cycles
> adding {imx8_ddr.write_cycles}:W
> imx8_ddr.write_cycles -> imx8_ddr0/event=0x2b/
> imx8_ddr.write_cycles: 13153 1000495125 1000495125
> #           time             counts unit events
>       1.000476625              13153      imx8_ddr.write_cycles     #    205.5 MB  imx8mm_ddr_write.all
> imx8_ddr.write_cycles: 3582 1000681375 1000681375
>       2.001167750               3582      imx8_ddr.write_cycles     #     56.0 MB  imx8mm_ddr_write.all
> 
> 
> 8QM:
> root@imx8qmmek:~# ./perf stat -v -a -I 1000 -M imx8qm_ddr_read.all

Note: for this example, I don't know why you didn't use 
imx8mm_ddr_write.all, as for your 8MM test, so we can compare the same.

> Using CPUID 0x00000000410fd030
> metric expr imx8_ddr.read_cycles * 4 * 4 for imx8qm_ddr_read.all
> found event imx8_ddr.read_cycles
> metric expr imx8_ddr.read_cycles * 4 * 4 for imx8qm_ddr_read.all
> found event imx8_ddr.read_cycles
> adding {imx8_ddr.read_cycles}:W,{imx8_ddr.read_cycles}:W
> imx8_ddr.read_cycles -> imx8_ddr0/event=0x2a/
> imx8_ddr.read_cycles -> imx8_ddr1/event=0x2a/
> imx8_ddr.read_cycles -> imx8_ddr0/event=0x2a/
> imx8_ddr.read_cycles -> imx8_ddr1/event=0x2a/
> imx8_ddr.read_cycles: 22748 1000378750 1000378750
> imx8_ddr.read_cycles: 24640 1000376625 1000376625
> imx8_ddr.read_cycles: 22800 1000375125 1000375125
> imx8_ddr.read_cycles: 24616 1000372625 1000372625
> #           time             counts unit events
>       1.000377250              47388      imx8_ddr.read_cycles      #    740.4 MB  imx8qm_ddr_read.all
>       1.000377250              47416      imx8_ddr.read_cycles
> imx8_ddr.read_cycles: 32672 1000454375 1000454375
> imx8_ddr.read_cycles: 37888 1000457250 1000457250
> imx8_ddr.read_cycles: 32736 1000460250 1000460250
> imx8_ddr.read_cycles: 38012 1000463000 1000463000
>       2.000812375              70560      imx8_ddr.read_cycles      #   1102.5 MB  imx8qm_ddr_read.all
>       2.000812375              70748      imx8_ddr.read_cycles
> 

I that is just how the aliases work. But how about trying:

./perf stat -v -a -I 1000 -M imx8_ddr0/imx8qm_ddr_read.all/


for just ddr0

I know that the following worked for non-metrics for aliases on a 
specific HW PMU, so I guess should also work for metrics:

./perf stat -e smmuv3_pmcg_200148020/smmuv3_pmcg.l1_tlb/

Thanks,
John

RE: [RFC PATCH v2 09/13] perf vendor events: Add JSON metrics for imx8mm DDR Perf

[Joakim Zhang]

> -----Original Message-----
> From: John Garry <john.garry@huawei.com>
> Sent: 2020年4月20日 18:51
> To: Joakim Zhang <qiangqing.zhang@nxp.com>; peterz@infradead.org;
> mingo@redhat.com; acme@kernel.org; mark.rutland@arm.com;
> alexander.shishkin@linux.intel.com; jolsa@redhat.com;
> namhyung@kernel.org; will@kernel.org
> Cc: irogers@google.com; ak@linux.intel.com; Linuxarm
> <linuxarm@huawei.com>; linux-kernel@vger.kernel.org; Zhangshaokun
> <zhangshaokun@hisilicon.com>; robin.murphy@arm.com;
> linux-arm-kernel@lists.infradead.org
> Subject: Re: [RFC PATCH v2 09/13] perf vendor events: Add JSON metrics for
> imx8mm DDR Perf
> 
> On 20/04/2020 05:17, Joakim Zhang wrote:
> > However, it seems that there are small defects from metric.
> >
> > Firstly, could you help change "ScaleUnit": "9.765625e-4MB" into
> "ScaleUnit": "9.765625e-4KB", this is a mistake.
> 
> ok
> 
> >
> > Then, you can see that test is okay from 8MM. However, metric would add
> twice once time from 8QM which has two ddr perf(ddr0/ddr1), it looks
> incorrect.
> >
> > 8MM:
> > root@imx8mmevk:~# ./perf stat -v -a -I 1000 -M imx8mm_ddr_write.all
> > Using CPUID 0x00000000410fd030 metric expr imx8_ddr.write_cycles * 4 *
> > 4 for imx8mm_ddr_write.all found event imx8_ddr.write_cycles adding
> > {imx8_ddr.write_cycles}:W imx8_ddr.write_cycles ->
> > imx8_ddr0/event=0x2b/
> > imx8_ddr.write_cycles: 13153 1000495125 1000495125
> > #           time             counts unit events
> >       1.000476625              13153      imx8_ddr.write_cycles
> #    205.5 MB  imx8mm_ddr_write.all
> > imx8_ddr.write_cycles: 3582 1000681375 1000681375
> >       2.001167750               3582      imx8_ddr.write_cycles
> #     56.0 MB  imx8mm_ddr_write.all
> >
> >
> > 8QM:
> > root@imx8qmmek:~# ./perf stat -v -a -I 1000 -M imx8qm_ddr_read.all
> 
> Note: for this example, I don't know why you didn't use imx8mm_ddr_write.all,
> as for your 8MM test, so we can compare the same.

Yes, I use the imx8mm_ddr_write.all, I just re-name the metric, change nothing else.

> > Using CPUID 0x00000000410fd030
> > metric expr imx8_ddr.read_cycles * 4 * 4 for imx8qm_ddr_read.all found
> > event imx8_ddr.read_cycles metric expr imx8_ddr.read_cycles * 4 * 4
> > for imx8qm_ddr_read.all found event imx8_ddr.read_cycles adding
> > {imx8_ddr.read_cycles}:W,{imx8_ddr.read_cycles}:W
> > imx8_ddr.read_cycles -> imx8_ddr0/event=0x2a/ imx8_ddr.read_cycles ->
> > imx8_ddr1/event=0x2a/ imx8_ddr.read_cycles -> imx8_ddr0/event=0x2a/
> > imx8_ddr.read_cycles -> imx8_ddr1/event=0x2a/
> > imx8_ddr.read_cycles: 22748 1000378750 1000378750
> > imx8_ddr.read_cycles: 24640 1000376625 1000376625
> > imx8_ddr.read_cycles: 22800 1000375125 1000375125
> > imx8_ddr.read_cycles: 24616 1000372625 1000372625
> > #           time             counts unit events
> >       1.000377250              47388      imx8_ddr.read_cycles
> #    740.4 MB  imx8qm_ddr_read.all
> >       1.000377250              47416      imx8_ddr.read_cycles
> > imx8_ddr.read_cycles: 32672 1000454375 1000454375
> > imx8_ddr.read_cycles: 37888 1000457250 1000457250
> > imx8_ddr.read_cycles: 32736 1000460250 1000460250
> > imx8_ddr.read_cycles: 38012 1000463000 1000463000
> >       2.000812375              70560      imx8_ddr.read_cycles
> #   1102.5 MB  imx8qm_ddr_read.all
> >       2.000812375              70748      imx8_ddr.read_cycles
> >
> 
> I that is just how the aliases work. But how about trying:
> 
> ./perf stat -v -a -I 1000 -M imx8_ddr0/imx8qm_ddr_read.all/
> 
> 
> for just ddr0
> 
> I know that the following worked for non-metrics for aliases on a specific HW
> PMU, so I guess should also work for metrics:
> 
> ./perf stat -e smmuv3_pmcg_200148020/smmuv3_pmcg.l1_tlb/

Yes, this can work for non-metrics, I tried before, it seems unsupported for metric.

You may misunderstand me, now I doesn't ask for a specific HW PMU(ddr0 or ddr1), the issue is that it would add and calculate twice when more than one HW PMU.

I also did below change which is implemented at my local side, but it can't work on your branch.
--- a/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json
+++ b/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json
@@ -2,7 +2,7 @@
    {
            "BriefDescription": "bytes all masters read from ddr based on read-cycles event",
            "MetricName": "imx8mm_ddr_read.all",
-           "MetricExpr": "imx8_ddr.read_cycles * 4 * 4",
+           "MetricExpr": "imx8_ddr0\\/read\\-cycles\\/ * 4 * 4",
            "ScaleUnit": "9.765625e-4MB",
            "Unit": "imx8_ddr",
            "Compat": "i.mx8mm"
@@ -10,9 +10,9 @@
    {
            "BriefDescription": "bytes all masters write to ddr based on write-cycles event",
            "MetricName": "imx8mm_ddr_write.all",
-           "MetricExpr": "imx8_ddr.write_cycles * 4 * 4",
+           "MetricExpr": "imx8_ddr0\\/read\\-cycles\\/ * 4 * 4",
            "ScaleUnit": "9.765625e-4MB",
            "Unit": "imx8_ddr",
            "Compat": "i.mx8mm"
     }
-]
\ No newline at end of file
+]

root@imx8qmmek:~# ./perf stat -v -a -I 1000 -M imx8mm_ddr_read.all
Using CPUID 0x00000000410fd080
metric expr imx8_ddr0\/read\-cycles\/ * 4 * 4 for imx8mm_ddr_read.all
found event imx8_ddr0
found event read-cycles
metric expr imx8_ddr0\/read\-cycles\/ * 4 * 4 for imx8mm_ddr_read.all
found event imx8_ddr0
found event read-cycles
adding {imx8_ddr0,read-cycles}:W,{imx8_ddr0,read-cycles}:W
event syntax error: '{imx8_ddr0,read-cycles}:W,{imx8_ddr0,read-cycles}:W'
                      \___ parser error

 Usage: perf stat [<options>] [<command>]

    -M, --metrics <metric/metric group list>
                          monitor specified metrics or metric groups (separated by ,)

Now the metricgroup can't parse above metric expression, it shouldn't be.

Best Regards,
Joakim Zhang
> Thanks,
> John

Re: [RFC PATCH v2 09/13] perf vendor events: Add JSON metrics for imx8mm DDR Perf

[John Garry]

On 20/04/2020 12:25, Joakim Zhang wrote:
>>> imx8_ddr.write_cycles: 13153 1000495125 1000495125
>>> #           time             counts unit events
>>>        1.000476625              13153      imx8_ddr.write_cycles
>> #    205.5 MB  imx8mm_ddr_write.all
>>> imx8_ddr.write_cycles: 3582 1000681375 1000681375
>>>        2.001167750               3582      imx8_ddr.write_cycles
>> #     56.0 MB  imx8mm_ddr_write.all
>>>
>>> 8QM:
>>> root@imx8qmmek:~# ./perf stat -v -a -I 1000 -M imx8qm_ddr_read.all
>> Note: for this example, I don't know why you didn't use imx8mm_ddr_write.all,
>> as for your 8MM test, so we can compare the same.
> Yes, I use the imx8mm_ddr_write.all, I just re-name the metric, change nothing else.

Well it's hard to even keep up - let alone help -  when you're debugging 
QM support, which is not supported in this series (only MM is), and I 
don't know exactly what is in this JSON who have created (for QM).

For a start, the MM json will use "i.mx8mm" compat, which I figure 
should not work for QM. Please explain this.

Thanks,
John

> 
>>> Using CPUID 0x00000000410fd030
>>> metric expr imx8_ddr.read_cycles * 4 * 4 for i

RE: [RFC PATCH v2 09/13] perf vendor events: Add JSON metrics for imx8mm DDR Perf

[Joakim Zhang]

> -----Original Message-----
> From: John Garry <john.garry@huawei.com>
> Sent: 2020年4月20日 22:20
> To: Joakim Zhang <qiangqing.zhang@nxp.com>; peterz@infradead.org;
> mingo@redhat.com; acme@kernel.org; mark.rutland@arm.com;
> alexander.shishkin@linux.intel.com; jolsa@redhat.com;
> namhyung@kernel.org; will@kernel.org
> Cc: irogers@google.com; ak@linux.intel.com; Linuxarm
> <linuxarm@huawei.com>; linux-kernel@vger.kernel.org; Zhangshaokun
> <zhangshaokun@hisilicon.com>; robin.murphy@arm.com;
> linux-arm-kernel@lists.infradead.org
> Subject: Re: [RFC PATCH v2 09/13] perf vendor events: Add JSON metrics for
> imx8mm DDR Perf
> 
> On 20/04/2020 12:25, Joakim Zhang wrote:
> >>> imx8_ddr.write_cycles: 13153 1000495125 1000495125
> >>> #           time             counts unit events
> >>>        1.000476625              13153
> imx8_ddr.write_cycles
> >> #    205.5 MB  imx8mm_ddr_write.all
> >>> imx8_ddr.write_cycles: 3582 1000681375 1000681375
> >>>        2.001167750               3582
> imx8_ddr.write_cycles
> >> #     56.0 MB  imx8mm_ddr_write.all
> >>>
> >>> 8QM:
> >>> root@imx8qmmek:~# ./perf stat -v -a -I 1000 -M imx8qm_ddr_read.all
> >> Note: for this example, I don't know why you didn't use
> >> imx8mm_ddr_write.all, as for your 8MM test, so we can compare the same.
> > Yes, I use the imx8mm_ddr_write.all, I just re-name the metric, change
> nothing else.
> 
> Well it's hard to even keep up - let alone help -  when you're debugging QM
> support, which is not supported in this series (only MM is), and I don't know
> exactly what is in this JSON who have created (for QM).
> 
> For a start, the MM json will use "i.mx8mm" compat, which I figure should not
> work for QM. Please explain this.

For common events, cycles(event=0x00), read-cycles(event=0x2a), write-cycles(event=0x2b), read(event=0x35), write(event=0x38), all these events listed in file (tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/ddrc.json) are compatible for all i.MX8 DDR Perf, only AXI events are various from each SoC. These events tested okay for MX8MM and MX8QM.

Same situation, metrics listed in file (tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json) is also compatible for all i.MX8 DDR Perf, since metric expression only contains read-cycles(event=0x2a) and write-cycles(event=0x2b).

Generally speaking, now pmu events and metrics on your branch should support both MX8MM and MX8QM without any change, as long as they export "i.mx8mm" identifier.

As I mentioned before, pmu events tested okay for MX8MM and MX8QM. Metric also tested okay for MX8MM.
For MX8QM which has two HW PMU(ddr0/ddr1), metric can work, but it would add metric twice which I think if it is possible to improve it in your serials. 

I guess the root cause is that "imx8_ddr.read_cycles" contains two HW PMU events (imx8_ddr0/read-cycles/ and imx8_ddr1/read-cycles/) and metricgroup can't handle it at present.

8QM:
root@imx8qmmek:~# ./perf stat -v -a -I 1000 -M imx8mm_ddr_read.all 
Using CPUID 0x00000000410fd030
metric expr imx8_ddr.read_cycles * 4 * 4 for imx8mm_ddr_read.all 
found event imx8_ddr.read_cycles
metric expr imx8_ddr.read_cycles * 4 * 4 for imx8mm_ddr_read.all 
found event imx8_ddr.read_cycles
adding {imx8_ddr.read_cycles}:W,{imx8_ddr.read_cycles}:W
imx8_ddr.read_cycles -> imx8_ddr0/event=0x2a/
imx8_ddr.read_cycles -> imx8_ddr1/event=0x2a/
imx8_ddr.read_cycles -> imx8_ddr0/event=0x2a/
imx8_ddr.read_cycles -> imx8_ddr1/event=0x2a/
imx8_ddr.read_cycles: 22748 1000378750 1000378750
imx8_ddr.read_cycles: 24640 1000376625 1000376625
imx8_ddr.read_cycles: 22800 1000375125 1000375125
imx8_ddr.read_cycles: 24616 1000372625 1000372625
#           time             counts unit events
     1.000377250              47388      imx8_ddr.read_cycles      #    740.4 MB  imx8qm_ddr_read.all
     1.000377250              47416      imx8_ddr.read_cycles

Best Regards,
Joakim Zhang
> Thanks,
> John
> 
> >
> >>> Using CPUID 0x00000000410fd030
> >>> metric expr imx8_ddr.read_cycles * 4 * 4 for i

Re: [RFC PATCH v2 09/13] perf vendor events: Add JSON metrics for imx8mm DDR Perf

[John Garry]

On 21/04/2020 03:40, Joakim Zhang wrote:
> For common events, cycles(event=0x00), read-cycles(event=0x2a), write-cycles(event=0x2b), read(event=0x35), write(event=0x38), all these events listed in file (tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/ddrc.json) are compatible for all i.MX8 DDR Perf, only AXI events are various from each SoC. These events tested okay for MX8MM and MX8QM.
> 
> Same situation, metrics listed in file (tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json) is also compatible for all i.MX8 DDR Perf, since metric expression only contains read-cycles(event=0x2a) and write-cycles(event=0x2b).
> 
> Generally speaking, now pmu events and metrics on your branch should support both MX8MM and MX8QM without any change, as long as they export "i.mx8mm" identifier.

Right, but MX8QM should export "i.mx8qm" identifier for upstream eventually.

> 
> As I mentioned before, pmu events tested okay for MX8MM and MX8QM. Metric also tested okay for MX8MM.
> For MX8QM which has two HW PMU(ddr0/ddr1), metric can work, but it would add metric twice which I think if it is possible to improve it in your serials.
> 
> I guess the root cause is that "imx8_ddr.read_cycles" contains two HW PMU events (imx8_ddr0/read-cycles/ and imx8_ddr1/read-cycles/) and metricgroup can't handle it at present.

It should be ok, but I'll check it.

> 
> 8QM:
> root@imx8qmmek:~# ./perf stat -v -a -I 1000 -M imx8mm_ddr_read.all
> Using CPUID 0x00000000410fd030
> metric expr imx8_ddr.read_cycles * 4 * 4 for imx8mm_ddr_read.all
> found event imx8_ddr.read_cycles
> metric expr imx8_ddr.read_cycles * 4 * 4 for imx8mm_ddr_read.all
> found event imx8_ddr.read_cycles
> adding {imx8_ddr.read_cycles}:W,{imx8_ddr.read_cycles}:W
> imx8_ddr.read_cycles -> imx8_ddr0/event=0x2a/
> imx8_ddr.read_cycles -> imx8_ddr1/event=0x2a/
> imx8_ddr.read_cycles -> imx8_ddr0/event=0x2a/
> imx8_ddr.read_cycles -> imx8_ddr1/event=0x2a/
> imx8_ddr.read_cycles: 22748 1000378750 1000378750
> imx8_ddr.read_cycles: 24640 1000376625 1000376625
> imx8_ddr.read_cycles: 22800 1000375125 1000375125
> imx8_ddr.read_cycles: 24616 1000372625 1000372625
> #           time             counts unit events
>       1.000377250              47388      imx8_ddr.read_cycles      #    740.4 MB  imx8qm_ddr_read.all
>       1.000377250              47416      imx8_ddr.read_cycles

john

Re: [RFC PATCH v2 07/13] perf pmu: Add pmu_id()

[Jiri Olsa]

On Fri, Apr 17, 2020 at 06:41:18PM +0800, John Garry wrote:
> Add a function to read the PMU id sysfs entry. We only do it for uncore
> PMUs where this would be relevant.
> 
> Signed-off-by: John Garry <john.garry@huawei.com>
> ---
>  tools/perf/util/pmu.c | 36 ++++++++++++++++++++++++++++++++++++
>  tools/perf/util/pmu.h |  1 +
>  2 files changed, 37 insertions(+)
> 
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index ef6a63f3d386..6a67c6a28d08 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -594,6 +594,7 @@ static struct perf_cpu_map *__pmu_cpumask(const char *path)
>   * Uncore PMUs have a "cpumask" file under sysfs. CPU PMUs (e.g. on arm/arm64)
>   * may have a "cpus" file.
>   */
> +#define CPUS_TEMPLATE_ID	"%s/bus/event_source/devices/%s/identifier"
>  #define CPUS_TEMPLATE_UNCORE	"%s/bus/event_source/devices/%s/cpumask"
>  #define CPUS_TEMPLATE_CPU	"%s/bus/event_source/devices/%s/cpus"
>  
> @@ -632,6 +633,39 @@ static bool pmu_is_uncore(const char *name)
>  	return file_available(path);
>  }
>  
> +static char *pmu_id(const char *name)
> +{
> +	char path[PATH_MAX], *id;
> +	const char *sysfs;
> +	FILE *file;
> +	int n;
> +
> +	sysfs = sysfs__mountpoint();
> +	snprintf(path, PATH_MAX, CPUS_TEMPLATE_ID, sysfs, name);
> +
> +	id = malloc(PATH_MAX);
> +	if (!id)
> +		return NULL;
> +
> +	file = fopen(path, "r");
> +	if (!file) {
> +		free(id);
> +		return NULL;
> +	}
> +
> +	n = fscanf(file, "%s", id);
> +
> +	fclose(file);
> +
> +	if (!n) {
> +		free(id);
> +		return NULL;
> +	}
> +
> +	return id;
> +}

I still need to go through this patchset in more detail,
but just quick note, that we have sysfs__read_str that you
could use in here

jirka

Re: [RFC PATCH v2 10/13] perf metricgroup: Split up metricgroup__add_metric()

[Jiri Olsa]

On Fri, Apr 17, 2020 at 06:41:21PM +0800, John Garry wrote:

SNIP

>  static int metricgroup__add_metric(const char *metric, struct strbuf *events,
>  				   struct list_head *group_list)
>  {
> @@ -502,37 +542,12 @@ static int metricgroup__add_metric(const char *metric, struct strbuf *events,
>  			break;
>  		if (!pe->metric_expr)
>  			continue;
> -		if (match_metric(pe->metric_group, metric) ||
> -		    match_metric(pe->metric_name, metric)) {
> -			const char **ids;
> -			int idnum;
> -			struct egroup *eg;
> -
> -			pr_debug("metric expr %s for %s\n", pe->metric_expr, pe->metric_name);
>  
> -			if (expr__find_other(pe->metric_expr,
> -					     NULL, &ids, &idnum) < 0)
> -				continue;
> -			if (events->len > 0)
> -				strbuf_addf(events, ",");
> -
> -			if (metricgroup__has_constraint(pe))
> -				metricgroup__add_metric_non_group(events, ids, idnum);
> -			else
> -				metricgroup__add_metric_weak_group(events, ids, idnum);
> -
> -			eg = malloc(sizeof(struct egroup));
> -			if (!eg) {
> -				ret = -ENOMEM;
> -				break;
> -			}
> -			eg->ids = ids;
> -			eg->idnum = idnum;
> -			eg->metric_name = pe->metric_name;
> -			eg->metric_expr = pe->metric_expr;
> -			eg->metric_unit = pe->unit;
> -			list_add_tail(&eg->nd, group_list);
> -			ret = 0;

also this place got changed just recently a lot,
so you might want to rebase to the Arnaldo's latest perf/core

jirka

> +		if (match_pe_metric(pe, metric)) {
> +			ret = metricgroup__add_metric_pmu_event(pe, events,
> +								group_list);
> +			if (ret)
> +				return ret;
>  		}
>  	}
>  	return ret;
> -- 
> 2.16.4
> 

Re: [RFC PATCH v2 07/13] perf pmu: Add pmu_id()

[John Garry]

  Wi
On 22/04/2020 12:41, Jiri Olsa wrote:
> On Fri, Apr 17, 2020 at 06:41:18PM +0800, John Garry wrote:
>> Add a function to read the PMU id sysfs entry. We only do it for uncore
>> PMUs where this would be relevant.
>>
>> Signed-off-by: John Garry <john.garry@huawei.com>
>> ---
>>   tools/perf/util/pmu.c | 36 ++++++++++++++++++++++++++++++++++++
>>   tools/perf/util/pmu.h |  1 +
>>   2 files changed, 37 insertions(+)
>>
>> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
>> index ef6a63f3d386..6a67c6a28d08 100644
>> --- a/tools/perf/util/pmu.c
>> +++ b/tools/perf/util/pmu.c
>> @@ -594,6 +594,7 @@ static struct perf_cpu_map *__pmu_cpumask(const char *path)
>>    * Uncore PMUs have a "cpumask" file under sysfs. CPU PMUs (e.g. on arm/arm64)
>>    * may have a "cpus" file.
>>    */
>> +#define CPUS_TEMPLATE_ID	"%s/bus/event_source/devices/%s/identifier"
>>   #define CPUS_TEMPLATE_UNCORE	"%s/bus/event_source/devices/%s/cpumask"
>>   #define CPUS_TEMPLATE_CPU	"%s/bus/event_source/devices/%s/cpus"
>>   
>> @@ -632,6 +633,39 @@ static bool pmu_is_uncore(const char *name)
>>   	return file_available(path);
>>   }
>>   
>> +static char *pmu_id(const char *name)
>> +{
>> +	char path[PATH_MAX], *id;
>> +	const char *sysfs;
>> +	FILE *file;
>> +	int n;
>> +
>> +	sysfs = sysfs__mountpoint();
>> +	snprintf(path, PATH_MAX, CPUS_TEMPLATE_ID, sysfs, name);
>> +
>> +	id = malloc(PATH_MAX);
>> +	if (!id)
>> +		return NULL;
>> +
>> +	file = fopen(path, "r");
>> +	if (!file) {
>> +		free(id);
>> +		return NULL;
>> +	}
>> +
>> +	n = fscanf(file, "%s", id);
>> +
>> +	fclose(file);
>> +
>> +	if (!n) {
>> +		free(id);
>> +		return NULL;
>> +	}
>> +
>> +	return id;
>> +}
> 
> I still need to go through this patchset in more detail,

ok, great.

But, could you check patch #1 also, as this *may* be fixing something 
broken in mainline? Not sure. Without it, we get a spew of warnings for 
metrics.

> but just quick note, that we have sysfs__read_str that you
> could use in here
> 

ok, there may be more functions in current pmu.c then which can use 
this. I can check.

Cheers,
John

Re: [RFC PATCH v2 10/13] perf metricgroup: Split up metricgroup__add_metric()

[John Garry]

On 22/04/2020 12:44, Jiri Olsa wrote:
>>   static int metricgroup__add_metric(const char *metric, struct strbuf *events,
>>   				   struct list_head *group_list)
>>   {
>> @@ -502,37 +542,12 @@ static int metricgroup__add_metric(const char *metric, struct strbuf *events,
>>   			break;
>>   		if (!pe->metric_expr)
>>   			continue;
>> -		if (match_metric(pe->metric_group, metric) ||
>> -		    match_metric(pe->metric_name, metric)) {
>> -			const char **ids;
>> -			int idnum;
>> -			struct egroup *eg;
>> -
>> -			pr_debug("metric expr %s for %s\n", pe->metric_expr, pe->metric_name);
>>   
>> -			if (expr__find_other(pe->metric_expr,
>> -					     NULL, &ids, &idnum) < 0)
>> -				continue;
>> -			if (events->len > 0)
>> -				strbuf_addf(events, ",");
>> -
>> -			if (metricgroup__has_constraint(pe))
>> -				metricgroup__add_metric_non_group(events, ids, idnum);
>> -			else
>> -				metricgroup__add_metric_weak_group(events, ids, idnum);
>> -
>> -			eg = malloc(sizeof(struct egroup));
>> -			if (!eg) {
>> -				ret = -ENOMEM;
>> -				break;
>> -			}
>> -			eg->ids = ids;
>> -			eg->idnum = idnum;
>> -			eg->metric_name = pe->metric_name;
>> -			eg->metric_expr = pe->metric_expr;
>> -			eg->metric_unit = pe->unit;
>> -			list_add_tail(&eg->nd, group_list);
>> -			ret = 0;
> also this place got changed just recently a lot,
> so you might want to rebase to the Arnaldo's latest perf/core

Hi jirka,

Yeah, I saw that. I can check.

TBH, apart from that, I would be welcome to opinion on this latter patch 
of the series, concerned with metrics. I just split (butcher) the 
function and call common parts from 2x places now. Maybe there's a more 
fluid way to do this.

Cheers,
John

Re: [RFC PATCH v2 09/13] perf vendor events: Add JSON metrics for imx8mm DDR Perf

[John Garry]

On 21/04/2020 13:28, John Garry wrote:
>> cles(event=0x00), read-cycles(event=0x2a), write-cycles(event=0x2b), 
>> read(event=0x35), write(event=0x38), all these events listed in file 
>> (tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/ddrc.json) are 
>> compatible for all i.MX8 DDR Perf, only AXI events are various from 
>> each SoC. These events tested okay for MX8MM and MX8QM.
>>
>> Same situation, metrics listed in file 
>> (tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json) 
>> is also compatible for all i.MX8 DDR Perf, since metric expression 
>> only contains read-cycles(event=0x2a) and write-cycles(event=0x2b).
>>
>> Generally speaking, now pmu events and metrics on your branch should 
>> support both MX8MM and MX8QM without any change, as long as they 
>> export "i.mx8mm" identifier.
> 
> Right, but MX8QM should export "i.mx8qm" identifier for upstream 
> eventually.
> 
>>
>> As I mentioned before, pmu events tested okay for MX8MM and MX8QM. 
>> Metric also tested okay for MX8MM.
>> For MX8QM which has two HW PMU(ddr0/ddr1), metric can work, but it 
>> would add metric twice which I think if it is possible to improve it 
>> in your serials.
>>
>> I guess the root cause is that "imx8_ddr.read_cycles" contains two HW 
>> PMU events (imx8_ddr0/read-cycles/ and imx8_ddr1/read-cycles/) and 
>> metricgroup can't handle it at present.
> 
> It should be ok, but I'll check it.
> 

ok, I think I see the issue here. We add a metric per PMU erroneously. 
We don't see an issue for printing metrics, as the code does not error 
when adding clones when printing (which we do).

I'll try to fix this week.

Thanks,
John

>>
>> 8QM:
>> root@imx8qmmek:~# ./perf stat -v -a -I 1000 -M imx8mm_ddr_read.all
>> Using CPUID 0x00000000410fd030
>> metric expr imx8_ddr.read_cycles * 4 * 4 for imx8mm_ddr_read.all
>> found event imx8_ddr.read_cycles
>> metric expr imx8_ddr.read_cycles * 4 * 4 for imx8mm_ddr_read.all
>> found event imx8_ddr.read_cycles
>> adding {imx8_ddr.read_cycles}:W,{imx8_ddr.read_cycles}:W
>> imx8_ddr.read_cycles -> imx8_ddr0/event=0x2a/
>> imx8_ddr.read_cycles -> imx8_ddr1/event=0x2a/
>> imx8_ddr.read_cycles -> imx8_ddr0/event=0x2a/
>> imx8_ddr.read_cycles -> imx8_ddr1/event=0x2a/
>> imx8_ddr.read_cycles: 22748 1000378750 1000378750
>> imx8_ddr.read_cycles: 24640 1000376625 100

Re: [RFC PATCH v2 01/13] perf parse-events: Fix comparison of evsel and leader pmu name

[Jiri Olsa]

On Fri, Apr 17, 2020 at 06:41:12PM +0800, John Garry wrote:
> Since we now strdup() the pmu name for the event selector, use strcmp()
> instead of pointer equality for comparison.
> 
> Fixes: d4953f7ef1a2 ("perf parse-events: Fix 3 use after frees found with clang ASANutil/parse-events.c")
> Signed-off-by: John Garry <john.garry@huawei.com>

I don't ee this change in your branch:
  private-topic-perf-5.6-sys-pmu-events-v2-upstream

do you have some updated tree?

thanks,
jirka

> ---
> 
> I am not 100% sure that this is the right fix....
> 
>  tools/perf/util/parse-events.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 10107747b361..90ddade1ba23 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -1629,7 +1629,7 @@ parse_events__set_leader_for_uncore_aliase(char *name, struct list_head *list,
>  		 * event. That can be used to distinguish the leader from
>  		 * other members, even they have the same event name.
>  		 */
> -		if ((leader != evsel) && (leader->pmu_name == evsel->pmu_name)) {
> +		if ((leader != evsel) && !strcmp(leader->pmu_name, evsel->pmu_name)) {
>  			is_leader = false;
>  			continue;
>  		}
> -- 
> 2.16.4
> 

Re: [RFC PATCH v2 01/13] perf parse-events: Fix comparison of evsel and leader pmu name

[John Garry]

On 27/04/2020 09:16, Jiri Olsa wrote:
> On Fri, Apr 17, 2020 at 06:41:12PM +0800, John Garry wrote:
>> Since we now strdup() the pmu name for the event selector, use strcmp()
>> instead of pointer equality for comparison.
>>
>> Fixes: d4953f7ef1a2 ("perf parse-events: Fix 3 use after frees found with clang ASANutil/parse-events.c")
>> Signed-off-by: John Garry <john.garry@huawei.com>
> 
> I don't ee this change in your branch:
>    private-topic-perf-5.6-sys-pmu-events-v2-upstream
> 
> do you have some updated tree?

I have started rebase work here:

https://github.com/hisilicon/kernel-dev/commits/private-topic-perf-5.7-sys-pmu-events-v2

Without this patch, I get this spewed for metric events:

	assertion failed at util/parse-events.c:1637

However, if there was a problem on mainline, I would expect some other 
reports now.

Thanks,
John



> 
>> ---
>>
>> I am not 100% sure that this is the right fix....
>>
>>   tools/perf/util/parse-events.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
>> index 10107747b361..90ddade1ba23 100644
>> --- a/tools/perf/util/parse-events.c
>> +++ b/tools/perf/util/parse-events.c
>> @@ -1629,7 +1629,7 @@ parse_events__set_leader_for_uncore_aliase(char *name, struct list_head *list,
>>   		 * event. That can be used to distinguish the leader from
>>   		 * other members, even they have the same event name.
>>   		 */
>> -		if ((leader != evsel) && (leader->pmu_name == evsel->pmu_name)) {
>> +		if ((leader != evsel) && !strcmp(leader->pmu_name, evsel->pmu_name)) {
>>   			is_leader = false;
>>   			continue;
>>   		}
>> -- 
>> 2.16.4
>>
> 
> .
>