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

[PATCH RFC v3 00/12] 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/tree/private-topic-perf-5.7-sys-pmu-events-v3

Differences to v2:
- fixups for imx8mm JSONs
- fix for metrics being repeated per PMU
- use sysfs__read_str()
- fix typo in PMCG JSON
- drop evsel fix, which someone else fixed

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 (11):
  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                      | 295 +++++++++++++++------
 tools/perf/util/pmu.c                              |  96 +++++++
 tools/perf/util/pmu.h                              |   3 +
 15 files changed, 593 insertions(+), 129 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

[PATCH RFC v3 01/12] 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

[PATCH RFC v3 02/12] 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

[PATCH RFC v3 03/12] 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

[PATCH RFC v3 04/12] 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..8a59ce48bf06
--- /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 incoming transaction or (ATS or non-ATS) translation request",
+        "EventCode": "0x02",
+        "EventName": "smmuv3_pmcg.TLB_MISS",
+        "BriefDescription": "TLB miss caused by incoming transaction 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

[PATCH RFC v3 05/12] 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

[PATCH RFC v3 06/12] 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 | 18 ++++++++++++++++++
 tools/perf/util/pmu.h |  1 +
 2 files changed, 19 insertions(+)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index ef6a63f3d386..6f77c6af9e04 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	"./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,21 @@ static bool pmu_is_uncore(const char *name)
 	return file_available(path);
 }
 
+static char *pmu_id(const char *name)
+{
+	char path[PATH_MAX], *str;
+	size_t len;
+
+	snprintf(path, PATH_MAX, CPUS_TEMPLATE_ID, name);
+
+	if (sysfs__read_str(path, &str, &len) < 0)
+		return NULL;
+
+	str[len - 1] = 0; // remove line feed
+
+	return str;
+}
+
 /*
  *  PMU CORE devices have different name other than cpu in sysfs on some
  *  platforms.
@@ -844,6 +860,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

[PATCH RFC v3 07/12] 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 identifer 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 | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/pmu.h |  2 ++
 2 files changed, 80 insertions(+)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 6f77c6af9e04..3986d2360fc1 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -809,6 +809,83 @@ 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(pmu_sys_event_iter_fn fn, void *data)
+{
+	int i = 0;
+
+	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) {
+			struct pmu_event *pe = &event_table->table[j++];
+			int ret;
+
+			if (!pe->name && !pe->metric_group && !pe->metric_name)
+				break;
+
+			ret = fn(pe, data);
+			if (ret)
+				break;
+		}
+	}
+}
+
+struct pmu_sys_event_iter_data {
+	struct list_head *head;
+	struct perf_pmu *pmu;
+};
+
+static int pmu_add_sys_aliases_iter_fn(struct pmu_event *pe, void *data)
+{
+	struct pmu_sys_event_iter_data *idata = data;
+	struct perf_pmu *pmu = idata->pmu;
+
+	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 = pmu,
+	};
+
+	if (!pmu->id)
+		return;
+
+	pmu_for_each_sys_event(pmu_add_sys_aliases_iter_fn, &idata);
+}
+
 struct perf_event_attr * __weak
 perf_pmu__get_default_config(struct perf_pmu *pmu __maybe_unused)
 {
@@ -864,6 +941,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..f75f54afb354 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -106,6 +106,8 @@ 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 pmu_event *pe, void *data);
+void pmu_for_each_sys_event(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

[PATCH RFC v3 08/12] 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": "imx8mm_ddr.cycles",
+           "Unit": "imx8_ddr",
+           "Compat": "i.mx8mm"
+   },
+   {
+           "BriefDescription": "ddr read-cycles event",
+           "EventCode": "0x2a",
+           "EventName": "imx8mm_ddr.read_cycles",
+           "Unit": "imx8_ddr",
+           "Compat": "i.mx8mm"
+   },
+   {
+           "BriefDescription": "ddr write-cycles event",
+           "EventCode": "0x2b",
+           "EventName": "imx8mm_ddr.write_cycles",
+           "Unit": "imx8_ddr",
+           "Compat": "i.mx8mm"
+   },
+   {
+           "BriefDescription": "ddr read event",
+           "EventCode": "0x35",
+           "EventName": "imx8mm_ddr.read",
+           "Unit": "imx8_ddr",
+           "Compat": "i.mx8mm"
+   },
+   {
+           "BriefDescription": "ddr write event",
+           "EventCode": "0x38",
+           "EventName": "imx8mm_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..eb891ac896ad
--- /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": "imx8mm_ddr.read_cycles * 4 * 4",
+	    "ScaleUnit": "9.765625e-4KB",
+	    "Unit": "imx8_ddr",
+	    "Compat": "i.mx8mm"
+    },
+   {
+	    "BriefDescription": "bytes all masters write to ddr based on write-cycles event",
+	    "MetricName": "imx8mm_ddr_write.all",
+	    "MetricExpr": "imx8mm_ddr.write_cycles * 4 * 4",
+	    "ScaleUnit": "9.765625e-4KB",
+	    "Unit": "imx8_ddr",
+	    "Compat": "i.mx8mm"
+    }
+]
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

[PATCH RFC v3 09/12] 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

[PATCH RFC v3 10/12] 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.

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

[PATCH RFC v3 11/12] 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 | 66 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 62 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 31e97e24c2b0..ecf14aa01a59 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -377,6 +377,51 @@ 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 pmu_event *pe, void *data)
+{
+	struct metricgroup_iter_data *d = data;
+	struct perf_pmu *pmu = NULL;
+
+	if (!pe->metric_expr || !pe->compat)
+		return 0;
+
+	while ((pmu = perf_pmu__scan(pmu)) != NULL) {
+
+		if (!pmu->id || strcmp(pmu->id, pe->compat))
+			continue;
+
+		return d->fn(pe, d->data);
+	}
+
+	return 0;
+}
+
+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)
 {
@@ -387,9 +432,6 @@ void metricgroup__print(bool metrics, bool metricgroups, char *filter,
 	struct rb_node *node, *next;
 	struct strlist *metriclist = NULL;
 
-	if (!map)
-		return;
-
 	if (!metricgroups) {
 		metriclist = strlist__new(NULL, NULL);
 		if (!metriclist)
@@ -400,7 +442,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 +454,22 @@ void metricgroup__print(bool metrics, bool metricgroups, char *filter,
 					     details, &groups, metriclist);
 	}
 
+	{
+		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(metricgroup__sys_event_iter, &data);
+	}
+
 	if (metricgroups && !raw)
 		printf("\nMetric Groups:\n\n");
 	else if (metrics && !raw)
-- 
2.16.4

[PATCH RFC v3 12/12] 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 | 39 +++++++++++++++++++++++++++++++++++----
 1 file changed, 35 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index ecf14aa01a59..4062bb33912a 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -598,6 +598,25 @@ 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)
 {
@@ -605,10 +624,7 @@ static int metricgroup__add_metric(const char *metric, struct strbuf *events,
 	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)
@@ -623,6 +639,21 @@ static int metricgroup__add_metric(const char *metric, struct strbuf *events,
 				return ret;
 		}
 	}
+
+	{
+		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(metricgroup__sys_event_iter, &data);
+	}
+
 	return ret;
 }
 
-- 
2.16.4

RE: [PATCH RFC v3 00/12] perf pmu-events: Support event aliasing for system PMUs

[Joakim Zhang]

I did the test on MX8MM and MX8QM, both can work well.

So for the patch serials:
	Tested-by: Joakim Zhang <qiangqing.zhang@nxp.com>

Best Regards,
Joakim Zhang

> -----Original Message-----
> From: John Garry <john.garry@huawei.com>
> Sent: 2020年5月7日 19:58
> 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
> Cc: will@kernel.org; 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: [PATCH RFC v3 00/12] perf pmu-events: Support event aliasing for
> system PMUs
> 
> 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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.c
> om%2Fhisilicon%2Fkernel-dev%2Ftree%2Fprivate-topic-perf-5.7-sys-pmu-event
> s-v3&amp;data=02%7C01%7Cqiangqing.zhang%40nxp.com%7C7da833efd22b
> 439a131b08d7f27f53bc%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%
> 7C637244500996283083&amp;sdata=a0W9Xk7gMLGtoU1VlcXAKF3x1eK%2BJ
> oCf0%2FfSAzxCnpU%3D&amp;reserved=0
> 
> Differences to v2:
> - fixups for imx8mm JSONs
> - fix for metrics being repeated per PMU
> - use sysfs__read_str()
> - fix typo in PMCG JSON
> - drop evsel fix, which someone else fixed
> 
> 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 (11):
>   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                      | 295
> +++++++++++++++------
>  tools/perf/util/pmu.c                              |  96 +++++++
>  tools/perf/util/pmu.h                              |   3 +
>  15 files changed, 593 insertions(+), 129 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

Re: [PATCH RFC v3 09/12] perf metricgroup: Split up metricgroup__add_metric()

[Jiri Olsa]

On Thu, May 07, 2020 at 07:57:48PM +0800, John Garry wrote:
> 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.

this no longer applied on Arnaldo's perf/core,
it's very busy part now :-\

jirka

> 
> 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
> 

Re: [PATCH RFC v3 02/12] perf jevents: Add support for system events tables

[Jiri Olsa]

On Thu, May 07, 2020 at 07:57:41PM +0800, John Garry wrote:

SNIP

>  
> +static bool is_sys_dir(char *fname)
> +{
> +	char *pos;
> +
> +	while (true) {
> +		pos = strchr(fname, '/');

would strrchr be faster? also I thought there's something like
basename function that could be usable in here?

jirka

> +
> +		if (!pos) {
> +			if (!strcmp(fname, "sys"))
> +				return true;
> +			return false;
> +		}
> +
> +		fname = pos + 1;
> +	}
> +
> +	return false;
> +}

Re: [PATCH RFC v3 02/12] perf jevents: Add support for system events tables

[Jiri Olsa]

On Thu, May 07, 2020 at 07:57:41PM +0800, John Garry wrote:

SNIP

> +				      &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);

shouldn't we jump to empty_map in here? there still needs to be a
mapfile, right?

jirka

>  	}
>  
> -	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;
>  }

SNIP

Re: [PATCH RFC v3 02/12] perf jevents: Add support for system events tables

[Jiri Olsa]

On Thu, May 07, 2020 at 07:57:41PM +0800, John Garry wrote:

SNIP

>  
> +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},");

this will add extra tabs:

        {
                .table = 0
        },

while the rest of the file starts items without any indent

jirka

Re: [PATCH RFC v3 09/12] perf metricgroup: Split up metricgroup__add_metric()

[John Garry]

On 11/05/2020 12:01, Jiri Olsa wrote:
> On Thu, May 07, 2020 at 07:57:48PM +0800, John Garry wrote:
>> 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.
> 
> this no longer applied on Arnaldo's perf/core,


Hi jirka,

> it's very busy part now :-\

Right.

So I could rebase and resend, but I rather avoid that if possible since 
the metric code is so busy.

The point is that I would like to see progress on the kernel part first 
(to expose per-PMU sysfs identifier file). Once we agreement there, then 
I can promote this series to non-RFC and ensure I'm based on acme tip.

Hi Joakim, can you progress 
https://lore.kernel.org/linux-arm-kernel/20200226073433.5834-1-qiangqing.zhang@nxp.com/ 
to non-RFC now?

Thanks,
John


> 
> jirka
> 
>>
>> 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
>>
> 
> .
> 

RE: [PATCH RFC v3 09/12] perf metricgroup: Split up metricgroup__add_metric()

[Joakim Zhang]

> -----Original Message-----
> From: John Garry <john.garry@huawei.com>
> Sent: 2020年5月11日 19:25
> To: Jiri Olsa <jolsa@redhat.com>; Joakim Zhang <qiangqing.zhang@nxp.com>
> Cc: peterz@infradead.org; mingo@redhat.com; acme@kernel.org;
> mark.rutland@arm.com; alexander.shishkin@linux.intel.com;
> namhyung@kernel.org; will@kernel.org; ak@linux.intel.com;
> linuxarm@huawei.com; linux-kernel@vger.kernel.org; irogers@google.com;
> robin.murphy@arm.com; zhangshaokun@hisilicon.com;
> linux-arm-kernel@lists.infradead.org
> Subject: Re: [PATCH RFC v3 09/12] perf metricgroup: Split up
> metricgroup__add_metric()
> 
> On 11/05/2020 12:01, Jiri Olsa wrote:
> > On Thu, May 07, 2020 at 07:57:48PM +0800, John Garry wrote:
> >> 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.
> >
> > this no longer applied on Arnaldo's perf/core,
> 
> 
> Hi jirka,
> 
> > it's very busy part now :-\
> 
> Right.
> 
> So I could rebase and resend, but I rather avoid that if possible since the metric
> code is so busy.
> 
> The point is that I would like to see progress on the kernel part first (to expose
> per-PMU sysfs identifier file). Once we agreement there, then I can promote
> this series to non-RFC and ensure I'm based on acme tip.
> 
> Hi Joakim, can you progress
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.ker
> nel.org%2Flinux-arm-kernel%2F20200226073433.5834-1-qiangqing.zhang%40n
> xp.com%2F&amp;data=02%7C01%7Cqiangqing.zhang%40nxp.com%7Cf89617c
> e13bb4617a64d08d7f59e1e4e%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0
> %7C0%7C637247931771912817&amp;sdata=8vOLZrUzBQs69KijOaIXmVqt%2F
> cUtadDK3bCHRFD04kE%3D&amp;reserved=0
> to non-RFC now?

Hi John,

Okay, I will re-send the patch as non-RFC right now.

Best Regards,
Joakim Zhang
> Thanks,
> John
> 
> 
> >
> > jirka
> >
> >>
> >> 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
> >>
> >
> > .
> >

Re: [PATCH RFC v3 02/12] perf jevents: Add support for system events tables

[John Garry]

On 11/05/2020 12:01, Jiri Olsa wrote:
> On Thu, May 07, 2020 at 07:57:41PM +0800, John Garry wrote:
> 
> SNIP
> 
>>   
>> +static bool is_sys_dir(char *fname)
>> +{
>> +	char *pos;
>> +
>> +	while (true) {
>> +		pos = strchr(fname, '/');
> 
> would strrchr be faster?

maybe, I'll check the logic here to see if it can be reduced

  also I thought there's something like
> basename function that could be usable in here?

it's tricky, as we support putting JSONs in ./, or ./sys, or ./cpu

But I'll revisit this, as I cannot understand the logic at a glance, 
which is not good...

Thanks,
John

> 
> jirka
> 
>> +
>> +		if (!pos) {
>> +			if (!strcmp(fname, "sys"))
>> +				return true;
>> +			return false;
>> +		}
>> +
>> +		fname = pos + 1;
>> +	}
>> +
>> +	return false;
>> +}
> 
> .
> 

Re: [PATCH RFC v3 02/12] perf jevents: Add support for system events tables

[John Garry]

On 11/05/2020 12:01, Jiri Olsa wrote:
> On Thu, May 07, 2020 at 07:57:41PM +0800, John Garry wrote:
> 
> SNIP
> 
>> +				      &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);
> 
> shouldn't we jump to empty_map in here? there still needs to be a
> mapfile, right?

In theory we could only support sys events :)

But I'll now make this a (empty map) failure case. And I think that 
another error case handling needs fixing in my patch.


As for this:

  +	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},");
 >
 > this will add extra tabs:
 >
 >          {
 >                  .table = 0
 >          },
 >
 > while the rest of the file starts items without any indent
 >

I'll ensure the indent is the same.

BTW, is there anything to be said for removing the empty map feature 
(and always breaking the perf build instead)? I guess that it was just 
an early feature for dealing with unstable JSONs.

Thanks,
john

> 
> jirka
> 
>>   	}
>>   
>> -	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;
>>   }
> 
> SNIP
> 
> .
> 

Re: [PATCH RFC v3 02/12] perf jevents: Add support for system events tables

[Ian Rogers]

On Mon, May 11, 2020 at 8:03 AM John Garry <john.garry@huawei.com> wrote:
>
> On 11/05/2020 12:01, Jiri Olsa wrote:
> > On Thu, May 07, 2020 at 07:57:41PM +0800, John Garry wrote:
> >
> > SNIP
> >
> >> +                                  &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);
> >
> > shouldn't we jump to empty_map in here? there still needs to be a
> > mapfile, right?
>
> In theory we could only support sys events :)
>
> But I'll now make this a (empty map) failure case. And I think that
> another error case handling needs fixing in my patch.
>
>
> As for this:
>
>   +     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},");
>  >
>  > this will add extra tabs:
>  >
>  >          {
>  >                  .table = 0
>  >          },
>  >
>  > while the rest of the file starts items without any indent
>  >
>
> I'll ensure the indent is the same.
>
> BTW, is there anything to be said for removing the empty map feature
> (and always breaking the perf build instead)? I guess that it was just
> an early feature for dealing with unstable JSONs.

+1
I'd very much like it if JSON parse errors and the like didn't result
in an empty map but failed the build. I think ideally we could also
validate metric expressions using expr.y. If we include expr.y into
jevents then is there any need to parse the metric expression at
runtime? Could we just generate C code from jevents with a list of
events (aka ids) for programming and a dedicated print function for
each metric. The events would still be symbolic and checked at
runtime, but the expressions being C code could yield compile time
errors.

Thanks,
Ian

> Thanks,
> john
>
> >
> > jirka
> >
> >>      }
> >>
> >> -    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;
> >>   }
> >
> > SNIP
> >
> > .
> >
>

RE: [PATCH RFC v3 00/12] perf pmu-events: Support event aliasing for system PMUs

[Joakim Zhang]

> -----Original Message-----
> From: John Garry <john.garry@huawei.com>
> Sent: 2020年5月7日 19:58
> 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
> Cc: will@kernel.org; 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: [PATCH RFC v3 00/12] perf pmu-events: Support event aliasing for
> system PMUs
> 
> 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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.c
> om%2Fhisilicon%2Fkernel-dev%2Ftree%2Fprivate-topic-perf-5.7-sys-pmu-event
> s-v3&amp;data=02%7C01%7Cqiangqing.zhang%40nxp.com%7C7da833efd22b
> 439a131b08d7f27f53bc%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%
> 7C637244500996283083&amp;sdata=a0W9Xk7gMLGtoU1VlcXAKF3x1eK%2BJ
> oCf0%2FfSAzxCnpU%3D&amp;reserved=0
> 
> Differences to v2:
> - fixups for imx8mm JSONs
> - fix for metrics being repeated per PMU
> - use sysfs__read_str()
> - fix typo in PMCG JSON
> - drop evsel fix, which someone else fixed
> 
> 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.

Hi John,

I have an aside question, do you have any idea? Thanks a lot!

For DDR PMU, I want to add bandwidth usage metric, but it depends on DDR controller clock frequency.
For example, we have i.MX8MM LPDDR4 board which DDR controller clock is 800MHZ, and i.MX8MM DDR4 board which DDR controller is 600MHZ, but the SoC is the same.

So they can share all JSON metrics with identifier "i.mx8mm", except bandwidth metric. If I add separate JOSN metrics files for identifier "i.mx8mm-lpddr4" and identifier "i.mx8mm-ddr4", then it's going to be very redundant, since most metrics are same just the identifier is different.

Do you know how perf tool handle such case?

Best Regards,
Joakim Zhang
> Thanks,
> John
> 

Re: [PATCH RFC v3 00/12] perf pmu-events: Support event aliasing for system PMUs

[John Garry]

> I have an aside question, do you have any idea? Thanks a lot!
> 
> For DDR PMU, I want to add bandwidth usage metric, but it depends on DDR controller clock frequency.
> For example, we have i.MX8MM LPDDR4 board which DDR controller clock is 800MHZ, and i.MX8MM DDR4 board which DDR controller is 600MHZ, but the SoC is the same.
> 
> So they can share all JSON metrics with identifier "i.mx8mm", except bandwidth metric. 

what is the bandwidth metric? how is it supposed to be calculated?

If I add separate JOSN metrics files for identifier "i.mx8mm-lpddr4" and 
identifier "i.mx8mm-ddr4", then it's going to be very redundant, since 
most metrics are same just the identifier is different.
> 
> Do you know how perf tool handle such case?

jirka is supporting user-defined metric here:
https://lore.kernel.org/lkml/20200511205307.3107775-1-jolsa@kernel.org/

So maybe you can use that somehow with separate scripts.

Thanks,
John

Re: [PATCH RFC v3 02/12] perf jevents: Add support for system events tables

[Jiri Olsa]

On Mon, May 11, 2020 at 09:21:00AM -0700, Ian Rogers wrote:

SNIP

> >  >> +           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},");
> >  >
> >  > this will add extra tabs:
> >  >
> >  >          {
> >  >                  .table = 0
> >  >          },
> >  >
> >  > while the rest of the file starts items without any indent
> >  >
> >
> > I'll ensure the indent is the same.
> >
> > BTW, is there anything to be said for removing the empty map feature
> > (and always breaking the perf build instead)? I guess that it was just
> > an early feature for dealing with unstable JSONs.
> 
> +1
> I'd very much like it if JSON parse errors and the like didn't result
> in an empty map but failed the build. I think ideally we could also

yep, that seems like good approach to me

> validate metric expressions using expr.y. If we include expr.y into
> jevents then is there any need to parse the metric expression at
> runtime? Could we just generate C code from jevents with a list of
> events (aka ids) for programming and a dedicated print function for
> each metric. The events would still be symbolic and checked at
> runtime, but the expressions being C code could yield compile time
> errors.

nice idea.. not sure we are able to do that with just expr.y code,
like to generate specific C code for metric, but I'd like to see
patches for that ;-)

but we would still need expr.y int perf code for custom user metrics,
so it still needs to stay anyway

jirka

RE: [PATCH RFC v3 00/12] perf pmu-events: Support event aliasing for system PMUs

[Joakim Zhang]

> -----Original Message-----
> From: John Garry <john.garry@huawei.com>
> Sent: 2020年5月12日 18:13
> 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
> Cc: irogers@google.com; ak@linux.intel.com; robin.murphy@arm.com;
> Linuxarm <linuxarm@huawei.com>; linux-kernel@vger.kernel.org;
> Zhangshaokun <zhangshaokun@hisilicon.com>; will@kernel.org;
> linux-arm-kernel@lists.infradead.org
> Subject: Re: [PATCH RFC v3 00/12] perf pmu-events: Support event aliasing for
> system PMUs
> 
> 
> > I have an aside question, do you have any idea? Thanks a lot!
> >
> > For DDR PMU, I want to add bandwidth usage metric, but it depends on DDR
> controller clock frequency.
> > For example, we have i.MX8MM LPDDR4 board which DDR controller clock is
> 800MHZ, and i.MX8MM DDR4 board which DDR controller is 600MHZ, but the
> SoC is the same.
> >
> > So they can share all JSON metrics with identifier "i.mx8mm", except
> bandwidth metric.
>
> what is the bandwidth metric? how is it supposed to be calculated?

Something like below to calculate bandwidth usage:

i.MX8MM LPDDR4 board:  ((read-cycles + write-cycles) * 4 * 4 / duration_time) / (800 * 1000000 * 4 * 4)
i.MX8MM DDR4 board:  ((read-cycles + write-cycles) * 4 * 4 / duration_time) / (600 * 1000000 * 4 * 4)

So this should not be Soc specific, it is board specific, I don't know how to implement it from metric.

> If I add separate JOSN metrics files for identifier "i.mx8mm-lpddr4" and
> identifier "i.mx8mm-ddr4", then it's going to be very redundant, since most
> metrics are same just the identifier is different.
> >
> > Do you know how perf tool handle such case?
> 
> jirka is supporting user-defined metric here:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.ker
> nel.org%2Flkml%2F20200511205307.3107775-1-jolsa%40kernel.org%2F&amp;
> data=02%7C01%7Cqiangqing.zhang%40nxp.com%7Cbbd26d737cf44896ed360
> 8d7f65d33ad%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637248
> 752464922384&amp;sdata=wGqQ4%2B6rejJYrDj0SzeGJn09r660QIzOvRy0DCw
> EGVQ%3D&amp;reserved=0
> 
> So maybe you can use that somehow with separate scripts.

Thanks for your hint, I will research it to see if it is possible.

Best Regards,
Joakim Zhang
> Thanks,
> John