[PATCH v6 0/4] Add support for ThunderX2 pmu events using json files

[PATCH v6 0/4] Add support for ThunderX2 pmu events using json files

[Ganapatrao Kulkarni]

Extending json/jevent framework for parsing arm64 event files.
Adding jevents for ThunderX2 implementation defined PMU events.

v6 : Addressed comments [1] [2]

[1] https://patchwork.kernel.org/patch/9903099/
[2] https://patchwork.kernel.org/patch/9853899/

v5:
   - Addressed comments from Arnaldo.
   - Rebased to 4.13-rc5

v4:
   - Rebased to 4.13-rc1

v3:
   - Addressed comments from Will Deacon and Jayachandran C.
   - Rebased to 4.12-rc1

v2:
   - Updated as per Mark Rutland's suggestions.
   - Added provision for get_cpuid_str to get cpu id string
     from associated cpus of pmu core device.

v1: Initial patchset.

Ganapatrao Kulkarni (4):
  perf utils: passing pmu as a parameter to function get_cpuid_str
  perf tools arm64: Add support for get_cpuid_str function.
  perf utils: Add helper function is_pmu_core to detect PMU CORE devices
  perf vendor events arm64: Add ThunderX2 implementation defined pmu
    core events

 tools/perf/arch/arm64/util/Build                   |  1 +
 tools/perf/arch/arm64/util/header.c                | 60 +++++++++++++++++++++
 tools/perf/arch/powerpc/util/header.c              |  2 +-
 tools/perf/arch/x86/util/header.c                  |  2 +-
 tools/perf/pmu-events/arch/arm64/mapfile.csv       | 15 ++++++
 .../arm64/thunderx2/implementation-defined.json    | 62 ++++++++++++++++++++++
 tools/perf/util/header.h                           |  3 +-
 tools/perf/util/pmu.c                              | 48 ++++++++++++++---
 8 files changed, 182 insertions(+), 11 deletions(-)
 create mode 100644 tools/perf/arch/arm64/util/header.c
 create mode 100644 tools/perf/pmu-events/arch/arm64/mapfile.csv
 create mode 100644 tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json

-- 
2.9.4

[PATCH v6 0/4] Add support for ThunderX2 pmu events using json files

[Ganapatrao Kulkarni]

Extending json/jevent framework for parsing arm64 event files.
Adding jevents for ThunderX2 implementation defined PMU events.

v6 : Addressed comments [1] [2]

[1] https://patchwork.kernel.org/patch/9903099/
[2] https://patchwork.kernel.org/patch/9853899/

v5:
   - Addressed comments from Arnaldo.
   - Rebased to 4.13-rc5

v4:
   - Rebased to 4.13-rc1

v3:
   - Addressed comments from Will Deacon and Jayachandran C.
   - Rebased to 4.12-rc1

v2:
   - Updated as per Mark Rutland's suggestions.
   - Added provision for get_cpuid_str to get cpu id string
     from associated cpus of pmu core device.

v1: Initial patchset.

Ganapatrao Kulkarni (4):
  perf utils: passing pmu as a parameter to function get_cpuid_str
  perf tools arm64: Add support for get_cpuid_str function.
  perf utils: Add helper function is_pmu_core to detect PMU CORE devices
  perf vendor events arm64: Add ThunderX2 implementation defined pmu
    core events

 tools/perf/arch/arm64/util/Build                   |  1 +
 tools/perf/arch/arm64/util/header.c                | 60 +++++++++++++++++++++
 tools/perf/arch/powerpc/util/header.c              |  2 +-
 tools/perf/arch/x86/util/header.c                  |  2 +-
 tools/perf/pmu-events/arch/arm64/mapfile.csv       | 15 ++++++
 .../arm64/thunderx2/implementation-defined.json    | 62 ++++++++++++++++++++++
 tools/perf/util/header.h                           |  3 +-
 tools/perf/util/pmu.c                              | 48 ++++++++++++++---
 8 files changed, 182 insertions(+), 11 deletions(-)
 create mode 100644 tools/perf/arch/arm64/util/header.c
 create mode 100644 tools/perf/pmu-events/arch/arm64/mapfile.csv
 create mode 100644 tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json

-- 
2.9.4

[PATCH v6 1/4] perf utils: passing pmu as a parameter to function get_cpuid_str

[Ganapatrao Kulkarni]

cpuid string will not be same on all CPUs on heterogeneous
platforms like ARM's big.LITTLE, adding provision(using pmu->cpus)
to find cpuid string from associated CPUs of PMU CORE device.

Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
---
 tools/perf/arch/powerpc/util/header.c | 2 +-
 tools/perf/arch/x86/util/header.c     | 2 +-
 tools/perf/util/header.h              | 3 ++-
 tools/perf/util/pmu.c                 | 9 +++++----
 4 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/tools/perf/arch/powerpc/util/header.c b/tools/perf/arch/powerpc/util/header.c
index 9aaa6f5..2953681 100644
--- a/tools/perf/arch/powerpc/util/header.c
+++ b/tools/perf/arch/powerpc/util/header.c
@@ -34,7 +34,7 @@ get_cpuid(char *buffer, size_t sz)
 }
 
 char *
-get_cpuid_str(void)
+get_cpuid_str(struct perf_pmu *pmu __maybe_unused)
 {
 	char *bufp;
 
diff --git a/tools/perf/arch/x86/util/header.c b/tools/perf/arch/x86/util/header.c
index a74a48d..d52bc27 100644
--- a/tools/perf/arch/x86/util/header.c
+++ b/tools/perf/arch/x86/util/header.c
@@ -65,7 +65,7 @@ get_cpuid(char *buffer, size_t sz)
 }
 
 char *
-get_cpuid_str(void)
+get_cpuid_str(struct perf_pmu *pmu __maybe_unused)
 {
 	char *buf = malloc(128);
 
diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
index d30109b..05e5758 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -8,6 +8,7 @@
 #include <linux/types.h>
 #include "event.h"
 #include "env.h"
+#include "pmu.h"
 
 enum {
 	HEADER_RESERVED		= 0,	/* always cleared */
@@ -151,5 +152,5 @@ int write_padded(int fd, const void *bf, size_t count, size_t count_aligned);
  */
 int get_cpuid(char *buffer, size_t sz);
 
-char *get_cpuid_str(void);
+char *get_cpuid_str(struct perf_pmu *pmu __maybe_unused);
 #endif /* __PERF_HEADER_H */
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index ac16a9d..aefdbd1 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -511,7 +511,7 @@ static struct cpu_map *pmu_cpumask(const char *name)
  * Each architecture should provide a more precise id string that
  * can be use to match the architecture's "mapfile".
  */
-char * __weak get_cpuid_str(void)
+char * __weak get_cpuid_str(struct perf_pmu *pmu __maybe_unused)
 {
 	return NULL;
 }
@@ -521,7 +521,8 @@ char * __weak get_cpuid_str(void)
  * to the current running CPU. Then, add all PMU events from that table
  * as aliases.
  */
-static void pmu_add_cpu_aliases(struct list_head *head, const char *name)
+static void pmu_add_cpu_aliases(struct list_head *head, const char *name,
+		struct perf_pmu *pmu)
 {
 	int i;
 	struct pmu_events_map *map;
@@ -533,7 +534,7 @@ static void pmu_add_cpu_aliases(struct list_head *head, const char *name)
 	if (cpuid)
 		cpuid = strdup(cpuid);
 	if (!cpuid)
-		cpuid = get_cpuid_str();
+		cpuid = get_cpuid_str(pmu);
 	if (!cpuid)
 		return;
 
@@ -610,12 +611,12 @@ static struct perf_pmu *pmu_lookup(const char *name)
 	if (pmu_aliases(name, &aliases))
 		return NULL;
 
-	pmu_add_cpu_aliases(&aliases, name);
 	pmu = zalloc(sizeof(*pmu));
 	if (!pmu)
 		return NULL;
 
 	pmu->cpus = pmu_cpumask(name);
+	pmu_add_cpu_aliases(&aliases, name, pmu);
 
 	INIT_LIST_HEAD(&pmu->format);
 	INIT_LIST_HEAD(&pmu->aliases);
-- 
2.9.4

[PATCH v6 1/4] perf utils: passing pmu as a parameter to function get_cpuid_str

[Ganapatrao Kulkarni]

cpuid string will not be same on all CPUs on heterogeneous
platforms like ARM's big.LITTLE, adding provision(using pmu->cpus)
to find cpuid string from associated CPUs of PMU CORE device.

Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
---
 tools/perf/arch/powerpc/util/header.c | 2 +-
 tools/perf/arch/x86/util/header.c     | 2 +-
 tools/perf/util/header.h              | 3 ++-
 tools/perf/util/pmu.c                 | 9 +++++----
 4 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/tools/perf/arch/powerpc/util/header.c b/tools/perf/arch/powerpc/util/header.c
index 9aaa6f5..2953681 100644
--- a/tools/perf/arch/powerpc/util/header.c
+++ b/tools/perf/arch/powerpc/util/header.c
@@ -34,7 +34,7 @@ get_cpuid(char *buffer, size_t sz)
 }
 
 char *
-get_cpuid_str(void)
+get_cpuid_str(struct perf_pmu *pmu __maybe_unused)
 {
 	char *bufp;
 
diff --git a/tools/perf/arch/x86/util/header.c b/tools/perf/arch/x86/util/header.c
index a74a48d..d52bc27 100644
--- a/tools/perf/arch/x86/util/header.c
+++ b/tools/perf/arch/x86/util/header.c
@@ -65,7 +65,7 @@ get_cpuid(char *buffer, size_t sz)
 }
 
 char *
-get_cpuid_str(void)
+get_cpuid_str(struct perf_pmu *pmu __maybe_unused)
 {
 	char *buf = malloc(128);
 
diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
index d30109b..05e5758 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -8,6 +8,7 @@
 #include <linux/types.h>
 #include "event.h"
 #include "env.h"
+#include "pmu.h"
 
 enum {
 	HEADER_RESERVED		= 0,	/* always cleared */
@@ -151,5 +152,5 @@ int write_padded(int fd, const void *bf, size_t count, size_t count_aligned);
  */
 int get_cpuid(char *buffer, size_t sz);
 
-char *get_cpuid_str(void);
+char *get_cpuid_str(struct perf_pmu *pmu __maybe_unused);
 #endif /* __PERF_HEADER_H */
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index ac16a9d..aefdbd1 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -511,7 +511,7 @@ static struct cpu_map *pmu_cpumask(const char *name)
  * Each architecture should provide a more precise id string that
  * can be use to match the architecture's "mapfile".
  */
-char * __weak get_cpuid_str(void)
+char * __weak get_cpuid_str(struct perf_pmu *pmu __maybe_unused)
 {
 	return NULL;
 }
@@ -521,7 +521,8 @@ char * __weak get_cpuid_str(void)
  * to the current running CPU. Then, add all PMU events from that table
  * as aliases.
  */
-static void pmu_add_cpu_aliases(struct list_head *head, const char *name)
+static void pmu_add_cpu_aliases(struct list_head *head, const char *name,
+		struct perf_pmu *pmu)
 {
 	int i;
 	struct pmu_events_map *map;
@@ -533,7 +534,7 @@ static void pmu_add_cpu_aliases(struct list_head *head, const char *name)
 	if (cpuid)
 		cpuid = strdup(cpuid);
 	if (!cpuid)
-		cpuid = get_cpuid_str();
+		cpuid = get_cpuid_str(pmu);
 	if (!cpuid)
 		return;
 
@@ -610,12 +611,12 @@ static struct perf_pmu *pmu_lookup(const char *name)
 	if (pmu_aliases(name, &aliases))
 		return NULL;
 
-	pmu_add_cpu_aliases(&aliases, name);
 	pmu = zalloc(sizeof(*pmu));
 	if (!pmu)
 		return NULL;
 
 	pmu->cpus = pmu_cpumask(name);
+	pmu_add_cpu_aliases(&aliases, name, pmu);
 
 	INIT_LIST_HEAD(&pmu->format);
 	INIT_LIST_HEAD(&pmu->aliases);
-- 
2.9.4

[PATCH v6 2/4] perf tools arm64: Add support for get_cpuid_str function.

[Ganapatrao Kulkarni]

function get_cpuid_str returns MIDR string of the first online
cpu from the range of cpus associated with the pmu core device.

Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
---
 tools/perf/arch/arm64/util/Build    |  1 +
 tools/perf/arch/arm64/util/header.c | 60 +++++++++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+)
 create mode 100644 tools/perf/arch/arm64/util/header.c

diff --git a/tools/perf/arch/arm64/util/Build b/tools/perf/arch/arm64/util/Build
index cef6fb3..b1ab72d 100644
--- a/tools/perf/arch/arm64/util/Build
+++ b/tools/perf/arch/arm64/util/Build
@@ -1,3 +1,4 @@
+libperf-y += header.o
 libperf-$(CONFIG_DWARF)     += dwarf-regs.o
 libperf-$(CONFIG_LOCAL_LIBUNWIND) += unwind-libunwind.o
 
diff --git a/tools/perf/arch/arm64/util/header.c b/tools/perf/arch/arm64/util/header.c
new file mode 100644
index 0000000..f02a32e
--- /dev/null
+++ b/tools/perf/arch/arm64/util/header.c
@@ -0,0 +1,60 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <api/fs/fs.h>
+#include "header.h"
+
+#define MIDR "/regs/identification/midr_el1"
+#define MIDR_SIZE 19
+
+char *get_cpuid_str(struct perf_pmu *pmu)
+{
+	char *buf = NULL;
+	char path[PATH_MAX];
+	const char *sysfs = sysfs__mountpoint();
+	int cpu;
+	u64 midr = 0;
+	struct cpu_map *cpus;
+	FILE *file;
+
+	if (!sysfs || !pmu->cpus)
+		return NULL;
+
+	buf = malloc(MIDR_SIZE);
+	if (!buf)
+		return NULL;
+
+	/* read midr from list of cpus mapped to this pmu */
+	cpus = cpu_map__get(pmu->cpus);
+	for (cpu = 0; cpu < cpus->nr; cpu++) {
+		scnprintf(path, PATH_MAX, "%s/devices/system/cpu/cpu%d"MIDR,
+				sysfs, cpus->map[cpu]);
+
+		file = fopen(path, "r");
+		if (!file) {
+			pr_debug("fopen failed for file %s\n", path);
+			continue;
+		}
+
+		if (!fgets(buf, MIDR_SIZE, file))
+			continue;
+		fclose(file);
+
+		/* Ignore/clear Variant[23:20] and
+		 * Revision[3:0] of MIDR
+		 */
+		midr = strtoul(buf, NULL, 16);
+		midr &= (~(0xf << 20 | 0xf));
+		scnprintf(buf, MIDR_SIZE, "0x%016lx", midr);
+		/* got midr break loop */
+		break;
+	}
+
+	if (!midr) {
+		pr_err("failed to get cpuid string\n");
+		free(buf);
+		buf = NULL;
+	}
+
+	cpu_map__put(cpus);
+	return buf;
+}
-- 
2.9.4

[PATCH v6 2/4] perf tools arm64: Add support for get_cpuid_str function.

[Ganapatrao Kulkarni]

function get_cpuid_str returns MIDR string of the first online
cpu from the range of cpus associated with the pmu core device.

Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
---
 tools/perf/arch/arm64/util/Build    |  1 +
 tools/perf/arch/arm64/util/header.c | 60 +++++++++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+)
 create mode 100644 tools/perf/arch/arm64/util/header.c

diff --git a/tools/perf/arch/arm64/util/Build b/tools/perf/arch/arm64/util/Build
index cef6fb3..b1ab72d 100644
--- a/tools/perf/arch/arm64/util/Build
+++ b/tools/perf/arch/arm64/util/Build
@@ -1,3 +1,4 @@
+libperf-y += header.o
 libperf-$(CONFIG_DWARF)     += dwarf-regs.o
 libperf-$(CONFIG_LOCAL_LIBUNWIND) += unwind-libunwind.o
 
diff --git a/tools/perf/arch/arm64/util/header.c b/tools/perf/arch/arm64/util/header.c
new file mode 100644
index 0000000..f02a32e
--- /dev/null
+++ b/tools/perf/arch/arm64/util/header.c
@@ -0,0 +1,60 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <api/fs/fs.h>
+#include "header.h"
+
+#define MIDR "/regs/identification/midr_el1"
+#define MIDR_SIZE 19
+
+char *get_cpuid_str(struct perf_pmu *pmu)
+{
+	char *buf = NULL;
+	char path[PATH_MAX];
+	const char *sysfs = sysfs__mountpoint();
+	int cpu;
+	u64 midr = 0;
+	struct cpu_map *cpus;
+	FILE *file;
+
+	if (!sysfs || !pmu->cpus)
+		return NULL;
+
+	buf = malloc(MIDR_SIZE);
+	if (!buf)
+		return NULL;
+
+	/* read midr from list of cpus mapped to this pmu */
+	cpus = cpu_map__get(pmu->cpus);
+	for (cpu = 0; cpu < cpus->nr; cpu++) {
+		scnprintf(path, PATH_MAX, "%s/devices/system/cpu/cpu%d"MIDR,
+				sysfs, cpus->map[cpu]);
+
+		file = fopen(path, "r");
+		if (!file) {
+			pr_debug("fopen failed for file %s\n", path);
+			continue;
+		}
+
+		if (!fgets(buf, MIDR_SIZE, file))
+			continue;
+		fclose(file);
+
+		/* Ignore/clear Variant[23:20] and
+		 * Revision[3:0] of MIDR
+		 */
+		midr = strtoul(buf, NULL, 16);
+		midr &= (~(0xf << 20 | 0xf));
+		scnprintf(buf, MIDR_SIZE, "0x%016lx", midr);
+		/* got midr break loop */
+		break;
+	}
+
+	if (!midr) {
+		pr_err("failed to get cpuid string\n");
+		free(buf);
+		buf = NULL;
+	}
+
+	cpu_map__put(cpus);
+	return buf;
+}
-- 
2.9.4

[PATCH v6 3/4] perf utils: Add helper function is_pmu_core to detect PMU CORE devices

[Ganapatrao Kulkarni]

On some platforms, PMU core devices sysfs name is not cpu.
Adding function is_pmu_core to detect PMU core devices using
core device specific hints in sysfs.

For arm64 platforms, all core devices have file "cpus" in sysfs.

Tested-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
---
 tools/perf/util/pmu.c | 39 +++++++++++++++++++++++++++++++++++----
 1 file changed, 35 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index aefdbd1..c2fe70f 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -506,6 +506,34 @@ static struct cpu_map *pmu_cpumask(const char *name)
 }
 
 /*
+ *  PMU CORE devices have different name other than cpu in sysfs on some
+ *  platforms. looking for possible sysfs files to identify as core device.
+ */
+static int is_pmu_core(const char *name)
+{
+	struct stat st;
+	char path[PATH_MAX];
+	const char *sysfs = sysfs__mountpoint();
+
+	if (!sysfs)
+		return 0;
+
+	/* Look for cpu sysfs (x86 and others) */
+	scnprintf(path, PATH_MAX, "%s/bus/event_source/devices/cpu", sysfs);
+	if ((stat(path, &st) == 0) &&
+			(strncmp(name, "cpu", strlen("cpu")) == 0))
+		return 1;
+
+	/* Look for cpu sysfs (specific to arm) */
+	scnprintf(path, PATH_MAX, "%s/bus/event_source/devices/%s/cpus",
+				sysfs, name);
+	if (stat(path, &st) == 0)
+		return 1;
+
+	return 0;
+}
+
+/*
  * Return the CPU id as a raw string.
  *
  * Each architecture should provide a more precise id string that
@@ -558,15 +586,18 @@ static void pmu_add_cpu_aliases(struct list_head *head, const char *name,
 	 */
 	i = 0;
 	while (1) {
-		const char *pname;
 
 		pe = &map->table[i++];
 		if (!pe->name)
 			break;
 
-		pname = pe->pmu ? pe->pmu : "cpu";
-		if (strncmp(pname, name, strlen(pname)))
-			continue;
+		if (!is_pmu_core(name)) {
+			/* check for uncore devices */
+			if (pe->pmu == NULL)
+				continue;
+			if (strncmp(pe->pmu, name, strlen(pe->pmu)))
+				continue;
+		}
 
 		/* need type casts to override 'const' */
 		__perf_pmu__new_alias(head, NULL, (char *)pe->name,
-- 
2.9.4

[PATCH v6 3/4] perf utils: Add helper function is_pmu_core to detect PMU CORE devices

[Ganapatrao Kulkarni]

On some platforms, PMU core devices sysfs name is not cpu.
Adding function is_pmu_core to detect PMU core devices using
core device specific hints in sysfs.

For arm64 platforms, all core devices have file "cpus" in sysfs.

Tested-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
---
 tools/perf/util/pmu.c | 39 +++++++++++++++++++++++++++++++++++----
 1 file changed, 35 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index aefdbd1..c2fe70f 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -506,6 +506,34 @@ static struct cpu_map *pmu_cpumask(const char *name)
 }
 
 /*
+ *  PMU CORE devices have different name other than cpu in sysfs on some
+ *  platforms. looking for possible sysfs files to identify as core device.
+ */
+static int is_pmu_core(const char *name)
+{
+	struct stat st;
+	char path[PATH_MAX];
+	const char *sysfs = sysfs__mountpoint();
+
+	if (!sysfs)
+		return 0;
+
+	/* Look for cpu sysfs (x86 and others) */
+	scnprintf(path, PATH_MAX, "%s/bus/event_source/devices/cpu", sysfs);
+	if ((stat(path, &st) == 0) &&
+			(strncmp(name, "cpu", strlen("cpu")) == 0))
+		return 1;
+
+	/* Look for cpu sysfs (specific to arm) */
+	scnprintf(path, PATH_MAX, "%s/bus/event_source/devices/%s/cpus",
+				sysfs, name);
+	if (stat(path, &st) == 0)
+		return 1;
+
+	return 0;
+}
+
+/*
  * Return the CPU id as a raw string.
  *
  * Each architecture should provide a more precise id string that
@@ -558,15 +586,18 @@ static void pmu_add_cpu_aliases(struct list_head *head, const char *name,
 	 */
 	i = 0;
 	while (1) {
-		const char *pname;
 
 		pe = &map->table[i++];
 		if (!pe->name)
 			break;
 
-		pname = pe->pmu ? pe->pmu : "cpu";
-		if (strncmp(pname, name, strlen(pname)))
-			continue;
+		if (!is_pmu_core(name)) {
+			/* check for uncore devices */
+			if (pe->pmu == NULL)
+				continue;
+			if (strncmp(pe->pmu, name, strlen(pe->pmu)))
+				continue;
+		}
 
 		/* need type casts to override 'const' */
 		__perf_pmu__new_alias(head, NULL, (char *)pe->name,
-- 
2.9.4

[PATCH v6 4/4] perf vendor events arm64: Add ThunderX2 implementation defined pmu core events

[Ganapatrao Kulkarni]

This is not a full event list, but a short list of useful events.

Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
---
 tools/perf/pmu-events/arch/arm64/mapfile.csv       | 15 ++++++
 .../arm64/thunderx2/implementation-defined.json    | 62 ++++++++++++++++++++++
 2 files changed, 77 insertions(+)
 create mode 100644 tools/perf/pmu-events/arch/arm64/mapfile.csv
 create mode 100644 tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json

diff --git a/tools/perf/pmu-events/arch/arm64/mapfile.csv b/tools/perf/pmu-events/arch/arm64/mapfile.csv
new file mode 100644
index 0000000..7167086
--- /dev/null
+++ b/tools/perf/pmu-events/arch/arm64/mapfile.csv
@@ -0,0 +1,15 @@
+# Format:
+#	MIDR,Version,JSON/file/pathname,Type
+#
+# where
+#	MIDR	Processor version
+#		Variant[23:20] and Revision [3:0] should be zero.
+#	Version could be used to track version of of JSON file
+#		but currently unused.
+#	JSON/file/pathname is the path to JSON file, relative
+#		to tools/perf/pmu-events/arch/arm64/.
+#	Type is core, uncore etc
+#
+#
+#Family-model,Version,Filename,EventType
+0x00000000420f5160,v1,thunderx2,core
diff --git a/tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json b/tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json
new file mode 100644
index 0000000..2db45c4
--- /dev/null
+++ b/tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json
@@ -0,0 +1,62 @@
+[
+    {
+        "PublicDescription": "Attributable Level 1 data cache access, read",
+        "EventCode": "0x40",
+        "EventName": "l1d_cache_rd",
+        "BriefDescription": "L1D cache read",
+    },
+    {
+        "PublicDescription": "Attributable Level 1 data cache access, write ",
+        "EventCode": "0x41",
+        "EventName": "l1d_cache_wr",
+        "BriefDescription": "L1D cache write",
+    },
+    {
+        "PublicDescription": "Attributable Level 1 data cache refill, read",
+        "EventCode": "0x42",
+        "EventName": "l1d_cache_refill_rd",
+        "BriefDescription": "L1D cache refill read",
+    },
+    {
+        "PublicDescription": "Attributable Level 1 data cache refill, write",
+        "EventCode": "0x43",
+        "EventName": "l1d_cache_refill_wr",
+        "BriefDescription": "L1D refill write",
+    },
+    {
+        "PublicDescription": "Attributable Level 1 data TLB refill, read",
+        "EventCode": "0x4C",
+        "EventName": "l1d_tlb_refill_rd",
+        "BriefDescription": "L1D tlb refill read",
+    },
+    {
+        "PublicDescription": "Attributable Level 1 data TLB refill, write",
+        "EventCode": "0x4D",
+        "EventName": "l1d_tlb_refill_wr",
+        "BriefDescription": "L1D tlb refill write",
+    },
+    {
+        "PublicDescription": "Attributable Level 1 data or unified TLB access, read",
+        "EventCode": "0x4E",
+        "EventName": "l1d_tlb_rd",
+        "BriefDescription": "L1D tlb read",
+    },
+    {
+        "PublicDescription": "Attributable Level 1 data or unified TLB access, write",
+        "EventCode": "0x4F",
+        "EventName": "l1d_tlb_wr",
+        "BriefDescription": "L1D tlb write",
+    },
+    {
+        "PublicDescription": "Bus access read",
+        "EventCode": "0x60",
+        "EventName": "bus_access_rd",
+        "BriefDescription": "Bus access read",
+   },
+   {
+        "PublicDescription": "Bus access write",
+        "EventCode": "0x61",
+        "EventName": "bus_access_wr",
+        "BriefDescription": "Bus access write",
+   }
+]
-- 
2.9.4

[PATCH v6 4/4] perf vendor events arm64: Add ThunderX2 implementation defined pmu core events

[Ganapatrao Kulkarni]

This is not a full event list, but a short list of useful events.

Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
---
 tools/perf/pmu-events/arch/arm64/mapfile.csv       | 15 ++++++
 .../arm64/thunderx2/implementation-defined.json    | 62 ++++++++++++++++++++++
 2 files changed, 77 insertions(+)
 create mode 100644 tools/perf/pmu-events/arch/arm64/mapfile.csv
 create mode 100644 tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json

diff --git a/tools/perf/pmu-events/arch/arm64/mapfile.csv b/tools/perf/pmu-events/arch/arm64/mapfile.csv
new file mode 100644
index 0000000..7167086
--- /dev/null
+++ b/tools/perf/pmu-events/arch/arm64/mapfile.csv
@@ -0,0 +1,15 @@
+# Format:
+#	MIDR,Version,JSON/file/pathname,Type
+#
+# where
+#	MIDR	Processor version
+#		Variant[23:20] and Revision [3:0] should be zero.
+#	Version could be used to track version of of JSON file
+#		but currently unused.
+#	JSON/file/pathname is the path to JSON file, relative
+#		to tools/perf/pmu-events/arch/arm64/.
+#	Type is core, uncore etc
+#
+#
+#Family-model,Version,Filename,EventType
+0x00000000420f5160,v1,thunderx2,core
diff --git a/tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json b/tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json
new file mode 100644
index 0000000..2db45c4
--- /dev/null
+++ b/tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json
@@ -0,0 +1,62 @@
+[
+    {
+        "PublicDescription": "Attributable Level 1 data cache access, read",
+        "EventCode": "0x40",
+        "EventName": "l1d_cache_rd",
+        "BriefDescription": "L1D cache read",
+    },
+    {
+        "PublicDescription": "Attributable Level 1 data cache access, write ",
+        "EventCode": "0x41",
+        "EventName": "l1d_cache_wr",
+        "BriefDescription": "L1D cache write",
+    },
+    {
+        "PublicDescription": "Attributable Level 1 data cache refill, read",
+        "EventCode": "0x42",
+        "EventName": "l1d_cache_refill_rd",
+        "BriefDescription": "L1D cache refill read",
+    },
+    {
+        "PublicDescription": "Attributable Level 1 data cache refill, write",
+        "EventCode": "0x43",
+        "EventName": "l1d_cache_refill_wr",
+        "BriefDescription": "L1D refill write",
+    },
+    {
+        "PublicDescription": "Attributable Level 1 data TLB refill, read",
+        "EventCode": "0x4C",
+        "EventName": "l1d_tlb_refill_rd",
+        "BriefDescription": "L1D tlb refill read",
+    },
+    {
+        "PublicDescription": "Attributable Level 1 data TLB refill, write",
+        "EventCode": "0x4D",
+        "EventName": "l1d_tlb_refill_wr",
+        "BriefDescription": "L1D tlb refill write",
+    },
+    {
+        "PublicDescription": "Attributable Level 1 data or unified TLB access, read",
+        "EventCode": "0x4E",
+        "EventName": "l1d_tlb_rd",
+        "BriefDescription": "L1D tlb read",
+    },
+    {
+        "PublicDescription": "Attributable Level 1 data or unified TLB access, write",
+        "EventCode": "0x4F",
+        "EventName": "l1d_tlb_wr",
+        "BriefDescription": "L1D tlb write",
+    },
+    {
+        "PublicDescription": "Bus access read",
+        "EventCode": "0x60",
+        "EventName": "bus_access_rd",
+        "BriefDescription": "Bus access read",
+   },
+   {
+        "PublicDescription": "Bus access write",
+        "EventCode": "0x61",
+        "EventName": "bus_access_wr",
+        "BriefDescription": "Bus access write",
+   }
+]
-- 
2.9.4

Re: [PATCH v6 4/4] perf vendor events arm64: Add ThunderX2 implementation defined pmu core events

[Zhangshaokun]

Hi Ganapat,

On 2017/8/24 20:03, Ganapatrao Kulkarni wrote:
> This is not a full event list, but a short list of useful events.
> 
> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
> ---
>  tools/perf/pmu-events/arch/arm64/mapfile.csv       | 15 ++++++
>  .../arm64/thunderx2/implementation-defined.json    | 62 ++++++++++++++++++++++
>  2 files changed, 77 insertions(+)
>  create mode 100644 tools/perf/pmu-events/arch/arm64/mapfile.csv
>  create mode 100644 tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json
> 

I saw you also used thunderx2 in tools/perf/pmu-events/arch/arm64/, how about John's suggestion
that would use vendor sub-folder?
Of course, appreciate maintainer's comments.

Thanks,
Shaokun

> diff --git a/tools/perf/pmu-events/arch/arm64/mapfile.csv b/tools/perf/pmu-events/arch/arm64/mapfile.csv
> new file mode 100644
> index 0000000..7167086
> --- /dev/null
> +++ b/tools/perf/pmu-events/arch/arm64/mapfile.csv
> @@ -0,0 +1,15 @@
> +# Format:
> +#	MIDR,Version,JSON/file/pathname,Type
> +#
> +# where
> +#	MIDR	Processor version
> +#		Variant[23:20] and Revision [3:0] should be zero.
> +#	Version could be used to track version of of JSON file
> +#		but currently unused.
> +#	JSON/file/pathname is the path to JSON file, relative
> +#		to tools/perf/pmu-events/arch/arm64/.
> +#	Type is core, uncore etc
> +#
> +#
> +#Family-model,Version,Filename,EventType
> +0x00000000420f5160,v1,thunderx2,core
> diff --git a/tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json b/tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json
> new file mode 100644
> index 0000000..2db45c4
> --- /dev/null
> +++ b/tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json
> @@ -0,0 +1,62 @@
> +[
> +    {
> +        "PublicDescription": "Attributable Level 1 data cache access, read",
> +        "EventCode": "0x40",
> +        "EventName": "l1d_cache_rd",
> +        "BriefDescription": "L1D cache read",
> +    },
> +    {
> +        "PublicDescription": "Attributable Level 1 data cache access, write ",
> +        "EventCode": "0x41",
> +        "EventName": "l1d_cache_wr",
> +        "BriefDescription": "L1D cache write",
> +    },
> +    {
> +        "PublicDescription": "Attributable Level 1 data cache refill, read",
> +        "EventCode": "0x42",
> +        "EventName": "l1d_cache_refill_rd",
> +        "BriefDescription": "L1D cache refill read",
> +    },
> +    {
> +        "PublicDescription": "Attributable Level 1 data cache refill, write",
> +        "EventCode": "0x43",
> +        "EventName": "l1d_cache_refill_wr",
> +        "BriefDescription": "L1D refill write",
> +    },
> +    {
> +        "PublicDescription": "Attributable Level 1 data TLB refill, read",
> +        "EventCode": "0x4C",
> +        "EventName": "l1d_tlb_refill_rd",
> +        "BriefDescription": "L1D tlb refill read",
> +    },
> +    {
> +        "PublicDescription": "Attributable Level 1 data TLB refill, write",
> +        "EventCode": "0x4D",
> +        "EventName": "l1d_tlb_refill_wr",
> +        "BriefDescription": "L1D tlb refill write",
> +    },
> +    {
> +        "PublicDescription": "Attributable Level 1 data or unified TLB access, read",
> +        "EventCode": "0x4E",
> +        "EventName": "l1d_tlb_rd",
> +        "BriefDescription": "L1D tlb read",
> +    },
> +    {
> +        "PublicDescription": "Attributable Level 1 data or unified TLB access, write",
> +        "EventCode": "0x4F",
> +        "EventName": "l1d_tlb_wr",
> +        "BriefDescription": "L1D tlb write",
> +    },
> +    {
> +        "PublicDescription": "Bus access read",
> +        "EventCode": "0x60",
> +        "EventName": "bus_access_rd",
> +        "BriefDescription": "Bus access read",
> +   },
> +   {
> +        "PublicDescription": "Bus access write",
> +        "EventCode": "0x61",
> +        "EventName": "bus_access_wr",
> +        "BriefDescription": "Bus access write",
> +   }
> +]
> 

[PATCH v6 4/4] perf vendor events arm64: Add ThunderX2 implementation defined pmu core events

[Zhangshaokun]

Hi Ganapat,

On 2017/8/24 20:03, Ganapatrao Kulkarni wrote:
> This is not a full event list, but a short list of useful events.
> 
> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
> ---
>  tools/perf/pmu-events/arch/arm64/mapfile.csv       | 15 ++++++
>  .../arm64/thunderx2/implementation-defined.json    | 62 ++++++++++++++++++++++
>  2 files changed, 77 insertions(+)
>  create mode 100644 tools/perf/pmu-events/arch/arm64/mapfile.csv
>  create mode 100644 tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json
> 

I saw you also used thunderx2 in tools/perf/pmu-events/arch/arm64/, how about John's suggestion
that would use vendor sub-folder?
Of course, appreciate maintainer's comments.

Thanks,
Shaokun

> diff --git a/tools/perf/pmu-events/arch/arm64/mapfile.csv b/tools/perf/pmu-events/arch/arm64/mapfile.csv
> new file mode 100644
> index 0000000..7167086
> --- /dev/null
> +++ b/tools/perf/pmu-events/arch/arm64/mapfile.csv
> @@ -0,0 +1,15 @@
> +# Format:
> +#	MIDR,Version,JSON/file/pathname,Type
> +#
> +# where
> +#	MIDR	Processor version
> +#		Variant[23:20] and Revision [3:0] should be zero.
> +#	Version could be used to track version of of JSON file
> +#		but currently unused.
> +#	JSON/file/pathname is the path to JSON file, relative
> +#		to tools/perf/pmu-events/arch/arm64/.
> +#	Type is core, uncore etc
> +#
> +#
> +#Family-model,Version,Filename,EventType
> +0x00000000420f5160,v1,thunderx2,core
> diff --git a/tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json b/tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json
> new file mode 100644
> index 0000000..2db45c4
> --- /dev/null
> +++ b/tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json
> @@ -0,0 +1,62 @@
> +[
> +    {
> +        "PublicDescription": "Attributable Level 1 data cache access, read",
> +        "EventCode": "0x40",
> +        "EventName": "l1d_cache_rd",
> +        "BriefDescription": "L1D cache read",
> +    },
> +    {
> +        "PublicDescription": "Attributable Level 1 data cache access, write ",
> +        "EventCode": "0x41",
> +        "EventName": "l1d_cache_wr",
> +        "BriefDescription": "L1D cache write",
> +    },
> +    {
> +        "PublicDescription": "Attributable Level 1 data cache refill, read",
> +        "EventCode": "0x42",
> +        "EventName": "l1d_cache_refill_rd",
> +        "BriefDescription": "L1D cache refill read",
> +    },
> +    {
> +        "PublicDescription": "Attributable Level 1 data cache refill, write",
> +        "EventCode": "0x43",
> +        "EventName": "l1d_cache_refill_wr",
> +        "BriefDescription": "L1D refill write",
> +    },
> +    {
> +        "PublicDescription": "Attributable Level 1 data TLB refill, read",
> +        "EventCode": "0x4C",
> +        "EventName": "l1d_tlb_refill_rd",
> +        "BriefDescription": "L1D tlb refill read",
> +    },
> +    {
> +        "PublicDescription": "Attributable Level 1 data TLB refill, write",
> +        "EventCode": "0x4D",
> +        "EventName": "l1d_tlb_refill_wr",
> +        "BriefDescription": "L1D tlb refill write",
> +    },
> +    {
> +        "PublicDescription": "Attributable Level 1 data or unified TLB access, read",
> +        "EventCode": "0x4E",
> +        "EventName": "l1d_tlb_rd",
> +        "BriefDescription": "L1D tlb read",
> +    },
> +    {
> +        "PublicDescription": "Attributable Level 1 data or unified TLB access, write",
> +        "EventCode": "0x4F",
> +        "EventName": "l1d_tlb_wr",
> +        "BriefDescription": "L1D tlb write",
> +    },
> +    {
> +        "PublicDescription": "Bus access read",
> +        "EventCode": "0x60",
> +        "EventName": "bus_access_rd",
> +        "BriefDescription": "Bus access read",
> +   },
> +   {
> +        "PublicDescription": "Bus access write",
> +        "EventCode": "0x61",
> +        "EventName": "bus_access_wr",
> +        "BriefDescription": "Bus access write",
> +   }
> +]
> 

Re: [PATCH v6 4/4] perf vendor events arm64: Add ThunderX2 implementation defined pmu core events

[Robert Richter]

Shaokun,

On 29.08.17 17:26:00, Zhangshaokun wrote:
> On 2017/8/24 20:03, Ganapatrao Kulkarni wrote:
> > This is not a full event list, but a short list of useful events.
> > 
> > Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
> > ---
> >  tools/perf/pmu-events/arch/arm64/mapfile.csv       | 15 ++++++
> >  .../arm64/thunderx2/implementation-defined.json    | 62 ++++++++++++++++++++++
> >  2 files changed, 77 insertions(+)
> >  create mode 100644 tools/perf/pmu-events/arch/arm64/mapfile.csv
> >  create mode 100644 tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json
> > 
> 
> I saw you also used thunderx2 in tools/perf/pmu-events/arch/arm64/, how about John's suggestion
> that would use vendor sub-folder?
> Of course, appreciate maintainer's comments.

this would just add another level of subdirectories. I rather would
prefer to have a per platform dir comparable to what is listed in

 arch/arm64/Kconfig.platforms

This is the same as Ganapat has implemented it.

-Robert

[PATCH v6 4/4] perf vendor events arm64: Add ThunderX2 implementation defined pmu core events

[Robert Richter]

Shaokun,

On 29.08.17 17:26:00, Zhangshaokun wrote:
> On 2017/8/24 20:03, Ganapatrao Kulkarni wrote:
> > This is not a full event list, but a short list of useful events.
> > 
> > Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
> > ---
> >  tools/perf/pmu-events/arch/arm64/mapfile.csv       | 15 ++++++
> >  .../arm64/thunderx2/implementation-defined.json    | 62 ++++++++++++++++++++++
> >  2 files changed, 77 insertions(+)
> >  create mode 100644 tools/perf/pmu-events/arch/arm64/mapfile.csv
> >  create mode 100644 tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json
> > 
> 
> I saw you also used thunderx2 in tools/perf/pmu-events/arch/arm64/, how about John's suggestion
> that would use vendor sub-folder?
> Of course, appreciate maintainer's comments.

this would just add another level of subdirectories. I rather would
prefer to have a per platform dir comparable to what is listed in

 arch/arm64/Kconfig.platforms

This is the same as Ganapat has implemented it.

-Robert

Re: [PATCH v6 4/4] perf vendor events arm64: Add ThunderX2 implementation defined pmu core events

[Zhangshaokun]

Hi Robert,

Got it and thanks your reply.

Shaokun

On 2017/8/29 20:47, Robert Richter wrote:
> Shaokun,
> 
> On 29.08.17 17:26:00, Zhangshaokun wrote:
>> On 2017/8/24 20:03, Ganapatrao Kulkarni wrote:
>>> This is not a full event list, but a short list of useful events.
>>>
>>> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
>>> ---
>>>  tools/perf/pmu-events/arch/arm64/mapfile.csv       | 15 ++++++
>>>  .../arm64/thunderx2/implementation-defined.json    | 62 ++++++++++++++++++++++
>>>  2 files changed, 77 insertions(+)
>>>  create mode 100644 tools/perf/pmu-events/arch/arm64/mapfile.csv
>>>  create mode 100644 tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json
>>>
>>
>> I saw you also used thunderx2 in tools/perf/pmu-events/arch/arm64/, how about John's suggestion
>> that would use vendor sub-folder?
>> Of course, appreciate maintainer's comments.
> 
> this would just add another level of subdirectories. I rather would
> prefer to have a per platform dir comparable to what is listed in
> 
>  arch/arm64/Kconfig.platforms
> 
> This is the same as Ganapat has implemented it.
> 
> -Robert
> 
> 

[PATCH v6 4/4] perf vendor events arm64: Add ThunderX2 implementation defined pmu core events

[Zhangshaokun]

Hi Robert,

Got it and thanks your reply.

Shaokun

On 2017/8/29 20:47, Robert Richter wrote:
> Shaokun,
> 
> On 29.08.17 17:26:00, Zhangshaokun wrote:
>> On 2017/8/24 20:03, Ganapatrao Kulkarni wrote:
>>> This is not a full event list, but a short list of useful events.
>>>
>>> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
>>> ---
>>>  tools/perf/pmu-events/arch/arm64/mapfile.csv       | 15 ++++++
>>>  .../arm64/thunderx2/implementation-defined.json    | 62 ++++++++++++++++++++++
>>>  2 files changed, 77 insertions(+)
>>>  create mode 100644 tools/perf/pmu-events/arch/arm64/mapfile.csv
>>>  create mode 100644 tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json
>>>
>>
>> I saw you also used thunderx2 in tools/perf/pmu-events/arch/arm64/, how about John's suggestion
>> that would use vendor sub-folder?
>> Of course, appreciate maintainer's comments.
> 
> this would just add another level of subdirectories. I rather would
> prefer to have a per platform dir comparable to what is listed in
> 
>  arch/arm64/Kconfig.platforms
> 
> This is the same as Ganapat has implemented it.
> 
> -Robert
> 
> 

Re: [PATCH v6 4/4] perf vendor events arm64: Add ThunderX2 implementation defined pmu core events

[Zhangshaokun]

Hi Robert,

On 2017/8/29 20:47, Robert Richter wrote:
> Shaokun,
> 
> On 29.08.17 17:26:00, Zhangshaokun wrote:
>> On 2017/8/24 20:03, Ganapatrao Kulkarni wrote:
>>> This is not a full event list, but a short list of useful events.
>>>
>>> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
>>> ---
>>>  tools/perf/pmu-events/arch/arm64/mapfile.csv       | 15 ++++++
>>>  .../arm64/thunderx2/implementation-defined.json    | 62 ++++++++++++++++++++++
>>>  2 files changed, 77 insertions(+)
>>>  create mode 100644 tools/perf/pmu-events/arch/arm64/mapfile.csv
>>>  create mode 100644 tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json
>>>
>>
>> I saw you also used thunderx2 in tools/perf/pmu-events/arch/arm64/, how about John's suggestion
>> that would use vendor sub-folder?
>> Of course, appreciate maintainer's comments.
> 
> this would just add another level of subdirectories. I rather would
> prefer to have a per platform dir comparable to what is listed in
> 
>  arch/arm64/Kconfig.platforms
> 

Check it again that not all vendors have specific platform config per SoC family,
so this would not work for us (HiSilicon) and maybe some other vendors.

Thanks,
Shaokun

> This is the same as Ganapat has implemented it.
> 
> -Robert
> 
> 

[PATCH v6 4/4] perf vendor events arm64: Add ThunderX2 implementation defined pmu core events

[Zhangshaokun]

Hi Robert,

On 2017/8/29 20:47, Robert Richter wrote:
> Shaokun,
> 
> On 29.08.17 17:26:00, Zhangshaokun wrote:
>> On 2017/8/24 20:03, Ganapatrao Kulkarni wrote:
>>> This is not a full event list, but a short list of useful events.
>>>
>>> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
>>> ---
>>>  tools/perf/pmu-events/arch/arm64/mapfile.csv       | 15 ++++++
>>>  .../arm64/thunderx2/implementation-defined.json    | 62 ++++++++++++++++++++++
>>>  2 files changed, 77 insertions(+)
>>>  create mode 100644 tools/perf/pmu-events/arch/arm64/mapfile.csv
>>>  create mode 100644 tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json
>>>
>>
>> I saw you also used thunderx2 in tools/perf/pmu-events/arch/arm64/, how about John's suggestion
>> that would use vendor sub-folder?
>> Of course, appreciate maintainer's comments.
> 
> this would just add another level of subdirectories. I rather would
> prefer to have a per platform dir comparable to what is listed in
> 
>  arch/arm64/Kconfig.platforms
> 

Check it again that not all vendors have specific platform config per SoC family,
so this would not work for us (HiSilicon) and maybe some other vendors.

Thanks,
Shaokun

> This is the same as Ganapat has implemented it.
> 
> -Robert
> 
> 

Re: [PATCH v6 4/4] perf vendor events arm64: Add ThunderX2 implementation defined pmu core events

[Robert Richter]

Shaokun,

On 31.08.17 18:29:01, Zhangshaokun wrote:
> On 2017/8/29 20:47, Robert Richter wrote:
> > On 29.08.17 17:26:00, Zhangshaokun wrote:
> >> On 2017/8/24 20:03, Ganapatrao Kulkarni wrote:
> >>> This is not a full event list, but a short list of useful events.
> >>>
> >>> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
> >>> ---
> >>>  tools/perf/pmu-events/arch/arm64/mapfile.csv       | 15 ++++++
> >>>  .../arm64/thunderx2/implementation-defined.json    | 62 ++++++++++++++++++++++
> >>>  2 files changed, 77 insertions(+)
> >>>  create mode 100644 tools/perf/pmu-events/arch/arm64/mapfile.csv
> >>>  create mode 100644 tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json
> >>>
> >>
> >> I saw you also used thunderx2 in tools/perf/pmu-events/arch/arm64/, how about John's suggestion
> >> that would use vendor sub-folder?
> >> Of course, appreciate maintainer's comments.
> > 
> > this would just add another level of subdirectories. I rather would
> > prefer to have a per platform dir comparable to what is listed in
> > 
> >  arch/arm64/Kconfig.platforms
> > 
> 
> Check it again that not all vendors have specific platform config per SoC family,
> so this would not work for us (HiSilicon) and maybe some other vendors.

Even if there is no specific platform defined in Kconfig, you still
could create a platform dir in tools/perf/. There must not be a 1:1
mapping. DT compatible strings may give a hint for naming then.

-Robert

[PATCH v6 4/4] perf vendor events arm64: Add ThunderX2 implementation defined pmu core events

[Robert Richter]

Shaokun,

On 31.08.17 18:29:01, Zhangshaokun wrote:
> On 2017/8/29 20:47, Robert Richter wrote:
> > On 29.08.17 17:26:00, Zhangshaokun wrote:
> >> On 2017/8/24 20:03, Ganapatrao Kulkarni wrote:
> >>> This is not a full event list, but a short list of useful events.
> >>>
> >>> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
> >>> ---
> >>>  tools/perf/pmu-events/arch/arm64/mapfile.csv       | 15 ++++++
> >>>  .../arm64/thunderx2/implementation-defined.json    | 62 ++++++++++++++++++++++
> >>>  2 files changed, 77 insertions(+)
> >>>  create mode 100644 tools/perf/pmu-events/arch/arm64/mapfile.csv
> >>>  create mode 100644 tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json
> >>>
> >>
> >> I saw you also used thunderx2 in tools/perf/pmu-events/arch/arm64/, how about John's suggestion
> >> that would use vendor sub-folder?
> >> Of course, appreciate maintainer's comments.
> > 
> > this would just add another level of subdirectories. I rather would
> > prefer to have a per platform dir comparable to what is listed in
> > 
> >  arch/arm64/Kconfig.platforms
> > 
> 
> Check it again that not all vendors have specific platform config per SoC family,
> so this would not work for us (HiSilicon) and maybe some other vendors.

Even if there is no specific platform defined in Kconfig, you still
could create a platform dir in tools/perf/. There must not be a 1:1
mapping. DT compatible strings may give a hint for naming then.

-Robert

Re: [PATCH v6 0/4] Add support for ThunderX2 pmu events using json files

[Ganapatrao Kulkarni]

Hi Will,

any further comments?


On Thu, Aug 24, 2017 at 5:33 PM, Ganapatrao Kulkarni
<ganapatrao.kulkarni@cavium.com> wrote:
> Extending json/jevent framework for parsing arm64 event files.
> Adding jevents for ThunderX2 implementation defined PMU events.
>
> v6 : Addressed comments [1] [2]
>
> [1] https://patchwork.kernel.org/patch/9903099/
> [2] https://patchwork.kernel.org/patch/9853899/
>
> v5:
>    - Addressed comments from Arnaldo.
>    - Rebased to 4.13-rc5
>
> v4:
>    - Rebased to 4.13-rc1
>
> v3:
>    - Addressed comments from Will Deacon and Jayachandran C.
>    - Rebased to 4.12-rc1
>
> v2:
>    - Updated as per Mark Rutland's suggestions.
>    - Added provision for get_cpuid_str to get cpu id string
>      from associated cpus of pmu core device.
>
> v1: Initial patchset.
>
> Ganapatrao Kulkarni (4):
>   perf utils: passing pmu as a parameter to function get_cpuid_str
>   perf tools arm64: Add support for get_cpuid_str function.
>   perf utils: Add helper function is_pmu_core to detect PMU CORE devices
>   perf vendor events arm64: Add ThunderX2 implementation defined pmu
>     core events
>
>  tools/perf/arch/arm64/util/Build                   |  1 +
>  tools/perf/arch/arm64/util/header.c                | 60 +++++++++++++++++++++
>  tools/perf/arch/powerpc/util/header.c              |  2 +-
>  tools/perf/arch/x86/util/header.c                  |  2 +-
>  tools/perf/pmu-events/arch/arm64/mapfile.csv       | 15 ++++++
>  .../arm64/thunderx2/implementation-defined.json    | 62 ++++++++++++++++++++++
>  tools/perf/util/header.h                           |  3 +-
>  tools/perf/util/pmu.c                              | 48 ++++++++++++++---
>  8 files changed, 182 insertions(+), 11 deletions(-)
>  create mode 100644 tools/perf/arch/arm64/util/header.c
>  create mode 100644 tools/perf/pmu-events/arch/arm64/mapfile.csv
>  create mode 100644 tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json
>
> --
> 2.9.4
>

thanks
Ganapat

[PATCH v6 0/4] Add support for ThunderX2 pmu events using json files

[Ganapatrao Kulkarni]

Hi Will,

any further comments?


On Thu, Aug 24, 2017 at 5:33 PM, Ganapatrao Kulkarni
<ganapatrao.kulkarni@cavium.com> wrote:
> Extending json/jevent framework for parsing arm64 event files.
> Adding jevents for ThunderX2 implementation defined PMU events.
>
> v6 : Addressed comments [1] [2]
>
> [1] https://patchwork.kernel.org/patch/9903099/
> [2] https://patchwork.kernel.org/patch/9853899/
>
> v5:
>    - Addressed comments from Arnaldo.
>    - Rebased to 4.13-rc5
>
> v4:
>    - Rebased to 4.13-rc1
>
> v3:
>    - Addressed comments from Will Deacon and Jayachandran C.
>    - Rebased to 4.12-rc1
>
> v2:
>    - Updated as per Mark Rutland's suggestions.
>    - Added provision for get_cpuid_str to get cpu id string
>      from associated cpus of pmu core device.
>
> v1: Initial patchset.
>
> Ganapatrao Kulkarni (4):
>   perf utils: passing pmu as a parameter to function get_cpuid_str
>   perf tools arm64: Add support for get_cpuid_str function.
>   perf utils: Add helper function is_pmu_core to detect PMU CORE devices
>   perf vendor events arm64: Add ThunderX2 implementation defined pmu
>     core events
>
>  tools/perf/arch/arm64/util/Build                   |  1 +
>  tools/perf/arch/arm64/util/header.c                | 60 +++++++++++++++++++++
>  tools/perf/arch/powerpc/util/header.c              |  2 +-
>  tools/perf/arch/x86/util/header.c                  |  2 +-
>  tools/perf/pmu-events/arch/arm64/mapfile.csv       | 15 ++++++
>  .../arm64/thunderx2/implementation-defined.json    | 62 ++++++++++++++++++++++
>  tools/perf/util/header.h                           |  3 +-
>  tools/perf/util/pmu.c                              | 48 ++++++++++++++---
>  8 files changed, 182 insertions(+), 11 deletions(-)
>  create mode 100644 tools/perf/arch/arm64/util/header.c
>  create mode 100644 tools/perf/pmu-events/arch/arm64/mapfile.csv
>  create mode 100644 tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json
>
> --
> 2.9.4
>

thanks
Ganapat

Re: [PATCH v6 2/4] perf tools arm64: Add support for get_cpuid_str function.

[Will Deacon]

On Thu, Aug 24, 2017 at 05:33:47PM +0530, Ganapatrao Kulkarni wrote:
> function get_cpuid_str returns MIDR string of the first online
> cpu from the range of cpus associated with the pmu core device.
> 
> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
> ---
>  tools/perf/arch/arm64/util/Build    |  1 +
>  tools/perf/arch/arm64/util/header.c | 60 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 61 insertions(+)
>  create mode 100644 tools/perf/arch/arm64/util/header.c
> 
> diff --git a/tools/perf/arch/arm64/util/Build b/tools/perf/arch/arm64/util/Build
> index cef6fb3..b1ab72d 100644
> --- a/tools/perf/arch/arm64/util/Build
> +++ b/tools/perf/arch/arm64/util/Build
> @@ -1,3 +1,4 @@
> +libperf-y += header.o
>  libperf-$(CONFIG_DWARF)     += dwarf-regs.o
>  libperf-$(CONFIG_LOCAL_LIBUNWIND) += unwind-libunwind.o
>  
> diff --git a/tools/perf/arch/arm64/util/header.c b/tools/perf/arch/arm64/util/header.c
> new file mode 100644
> index 0000000..f02a32e
> --- /dev/null
> +++ b/tools/perf/arch/arm64/util/header.c
> @@ -0,0 +1,60 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <api/fs/fs.h>
> +#include "header.h"
> +
> +#define MIDR "/regs/identification/midr_el1"
> +#define MIDR_SIZE 19
> +
> +char *get_cpuid_str(struct perf_pmu *pmu)
> +{
> +	char *buf = NULL;
> +	char path[PATH_MAX];
> +	const char *sysfs = sysfs__mountpoint();
> +	int cpu;
> +	u64 midr = 0;
> +	struct cpu_map *cpus;
> +	FILE *file;
> +
> +	if (!sysfs || !pmu->cpus)
> +		return NULL;
> +
> +	buf = malloc(MIDR_SIZE);
> +	if (!buf)
> +		return NULL;
> +
> +	/* read midr from list of cpus mapped to this pmu */
> +	cpus = cpu_map__get(pmu->cpus);
> +	for (cpu = 0; cpu < cpus->nr; cpu++) {
> +		scnprintf(path, PATH_MAX, "%s/devices/system/cpu/cpu%d"MIDR,
> +				sysfs, cpus->map[cpu]);
> +
> +		file = fopen(path, "r");
> +		if (!file) {
> +			pr_debug("fopen failed for file %s\n", path);
> +			continue;
> +		}
> +
> +		if (!fgets(buf, MIDR_SIZE, file))
> +			continue;
> +		fclose(file);

Don't you want to fclose the file if the fgets fails?

> +
> +		/* Ignore/clear Variant[23:20] and
> +		 * Revision[3:0] of MIDR
> +		 */
> +		midr = strtoul(buf, NULL, 16);
> +		midr &= (~(0xf << 20 | 0xf));

It would be cleaner if you had #defines for the MIDR fields that you're
masking.

> +		scnprintf(buf, MIDR_SIZE, "0x%016lx", midr);
> +		/* got midr break loop */
> +		break;
> +	}
> +
> +	if (!midr) {
> +		pr_err("failed to get cpuid string\n");

Might be helpful to print the PMU name and the CPU map if there's a way 
to do that.

Will

[PATCH v6 2/4] perf tools arm64: Add support for get_cpuid_str function.

[Will Deacon]

On Thu, Aug 24, 2017 at 05:33:47PM +0530, Ganapatrao Kulkarni wrote:
> function get_cpuid_str returns MIDR string of the first online
> cpu from the range of cpus associated with the pmu core device.
> 
> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
> ---
>  tools/perf/arch/arm64/util/Build    |  1 +
>  tools/perf/arch/arm64/util/header.c | 60 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 61 insertions(+)
>  create mode 100644 tools/perf/arch/arm64/util/header.c
> 
> diff --git a/tools/perf/arch/arm64/util/Build b/tools/perf/arch/arm64/util/Build
> index cef6fb3..b1ab72d 100644
> --- a/tools/perf/arch/arm64/util/Build
> +++ b/tools/perf/arch/arm64/util/Build
> @@ -1,3 +1,4 @@
> +libperf-y += header.o
>  libperf-$(CONFIG_DWARF)     += dwarf-regs.o
>  libperf-$(CONFIG_LOCAL_LIBUNWIND) += unwind-libunwind.o
>  
> diff --git a/tools/perf/arch/arm64/util/header.c b/tools/perf/arch/arm64/util/header.c
> new file mode 100644
> index 0000000..f02a32e
> --- /dev/null
> +++ b/tools/perf/arch/arm64/util/header.c
> @@ -0,0 +1,60 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <api/fs/fs.h>
> +#include "header.h"
> +
> +#define MIDR "/regs/identification/midr_el1"
> +#define MIDR_SIZE 19
> +
> +char *get_cpuid_str(struct perf_pmu *pmu)
> +{
> +	char *buf = NULL;
> +	char path[PATH_MAX];
> +	const char *sysfs = sysfs__mountpoint();
> +	int cpu;
> +	u64 midr = 0;
> +	struct cpu_map *cpus;
> +	FILE *file;
> +
> +	if (!sysfs || !pmu->cpus)
> +		return NULL;
> +
> +	buf = malloc(MIDR_SIZE);
> +	if (!buf)
> +		return NULL;
> +
> +	/* read midr from list of cpus mapped to this pmu */
> +	cpus = cpu_map__get(pmu->cpus);
> +	for (cpu = 0; cpu < cpus->nr; cpu++) {
> +		scnprintf(path, PATH_MAX, "%s/devices/system/cpu/cpu%d"MIDR,
> +				sysfs, cpus->map[cpu]);
> +
> +		file = fopen(path, "r");
> +		if (!file) {
> +			pr_debug("fopen failed for file %s\n", path);
> +			continue;
> +		}
> +
> +		if (!fgets(buf, MIDR_SIZE, file))
> +			continue;
> +		fclose(file);

Don't you want to fclose the file if the fgets fails?

> +
> +		/* Ignore/clear Variant[23:20] and
> +		 * Revision[3:0] of MIDR
> +		 */
> +		midr = strtoul(buf, NULL, 16);
> +		midr &= (~(0xf << 20 | 0xf));

It would be cleaner if you had #defines for the MIDR fields that you're
masking.

> +		scnprintf(buf, MIDR_SIZE, "0x%016lx", midr);
> +		/* got midr break loop */
> +		break;
> +	}
> +
> +	if (!midr) {
> +		pr_err("failed to get cpuid string\n");

Might be helpful to print the PMU name and the CPU map if there's a way 
to do that.

Will

Re: [PATCH v6 3/4] perf utils: Add helper function is_pmu_core to detect PMU CORE devices

[Will Deacon]

On Thu, Aug 24, 2017 at 05:33:48PM +0530, Ganapatrao Kulkarni wrote:
> On some platforms, PMU core devices sysfs name is not cpu.
> Adding function is_pmu_core to detect PMU core devices using
> core device specific hints in sysfs.
> 
> For arm64 platforms, all core devices have file "cpus" in sysfs.
> 
> Tested-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
> ---
>  tools/perf/util/pmu.c | 39 +++++++++++++++++++++++++++++++++++----
>  1 file changed, 35 insertions(+), 4 deletions(-)

Acked-by: Will Deacon <will.deacon@arm.com>

Will

[PATCH v6 3/4] perf utils: Add helper function is_pmu_core to detect PMU CORE devices

[Will Deacon]

On Thu, Aug 24, 2017 at 05:33:48PM +0530, Ganapatrao Kulkarni wrote:
> On some platforms, PMU core devices sysfs name is not cpu.
> Adding function is_pmu_core to detect PMU core devices using
> core device specific hints in sysfs.
> 
> For arm64 platforms, all core devices have file "cpus" in sysfs.
> 
> Tested-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
> ---
>  tools/perf/util/pmu.c | 39 +++++++++++++++++++++++++++++++++++----
>  1 file changed, 35 insertions(+), 4 deletions(-)

Acked-by: Will Deacon <will.deacon@arm.com>

Will

Re: [PATCH v6 4/4] perf vendor events arm64: Add ThunderX2 implementation defined pmu core events

[Will Deacon]

On Tue, Aug 29, 2017 at 02:47:30PM +0200, Robert Richter wrote:
> Shaokun,
> 
> On 29.08.17 17:26:00, Zhangshaokun wrote:
> > On 2017/8/24 20:03, Ganapatrao Kulkarni wrote:
> > > This is not a full event list, but a short list of useful events.
> > > 
> > > Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
> > > ---
> > >  tools/perf/pmu-events/arch/arm64/mapfile.csv       | 15 ++++++
> > >  .../arm64/thunderx2/implementation-defined.json    | 62 ++++++++++++++++++++++
> > >  2 files changed, 77 insertions(+)
> > >  create mode 100644 tools/perf/pmu-events/arch/arm64/mapfile.csv
> > >  create mode 100644 tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json
> > > 
> > 
> > I saw you also used thunderx2 in tools/perf/pmu-events/arch/arm64/, how about John's suggestion
> > that would use vendor sub-folder?
> > Of course, appreciate maintainer's comments.
> 
> this would just add another level of subdirectories. I rather would
> prefer to have a per platform dir comparable to what is listed in
> 
>  arch/arm64/Kconfig.platforms
> 
> This is the same as Ganapat has implemented it.

FWIW, I agree with Zhangshaokun here that a silicon vendor subdirectory
would organise things better. It also matches what we do for
arch/arm64/boot/dts/

Will

[PATCH v6 4/4] perf vendor events arm64: Add ThunderX2 implementation defined pmu core events

[Will Deacon]

On Tue, Aug 29, 2017 at 02:47:30PM +0200, Robert Richter wrote:
> Shaokun,
> 
> On 29.08.17 17:26:00, Zhangshaokun wrote:
> > On 2017/8/24 20:03, Ganapatrao Kulkarni wrote:
> > > This is not a full event list, but a short list of useful events.
> > > 
> > > Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
> > > ---
> > >  tools/perf/pmu-events/arch/arm64/mapfile.csv       | 15 ++++++
> > >  .../arm64/thunderx2/implementation-defined.json    | 62 ++++++++++++++++++++++
> > >  2 files changed, 77 insertions(+)
> > >  create mode 100644 tools/perf/pmu-events/arch/arm64/mapfile.csv
> > >  create mode 100644 tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json
> > > 
> > 
> > I saw you also used thunderx2 in tools/perf/pmu-events/arch/arm64/, how about John's suggestion
> > that would use vendor sub-folder?
> > Of course, appreciate maintainer's comments.
> 
> this would just add another level of subdirectories. I rather would
> prefer to have a per platform dir comparable to what is listed in
> 
>  arch/arm64/Kconfig.platforms
> 
> This is the same as Ganapat has implemented it.

FWIW, I agree with Zhangshaokun here that a silicon vendor subdirectory
would organise things better. It also matches what we do for
arch/arm64/boot/dts/

Will

Re: [PATCH v6 4/4] perf vendor events arm64: Add ThunderX2 implementation defined pmu core events

[Robert Richter]

On 11.10.17 13:19:27, Will Deacon wrote:
> On Tue, Aug 29, 2017 at 02:47:30PM +0200, Robert Richter wrote:
> > Shaokun,
> > 
> > On 29.08.17 17:26:00, Zhangshaokun wrote:
> > > On 2017/8/24 20:03, Ganapatrao Kulkarni wrote:
> > > > This is not a full event list, but a short list of useful events.
> > > > 
> > > > Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
> > > > ---
> > > >  tools/perf/pmu-events/arch/arm64/mapfile.csv       | 15 ++++++
> > > >  .../arm64/thunderx2/implementation-defined.json    | 62 ++++++++++++++++++++++
> > > >  2 files changed, 77 insertions(+)
> > > >  create mode 100644 tools/perf/pmu-events/arch/arm64/mapfile.csv
> > > >  create mode 100644 tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json
> > > > 
> > > 
> > > I saw you also used thunderx2 in tools/perf/pmu-events/arch/arm64/, how about John's suggestion
> > > that would use vendor sub-folder?
> > > Of course, appreciate maintainer's comments.
> > 
> > this would just add another level of subdirectories. I rather would
> > prefer to have a per platform dir comparable to what is listed in
> > 
> >  arch/arm64/Kconfig.platforms
> > 
> > This is the same as Ganapat has implemented it.
> 
> FWIW, I agree with Zhangshaokun here that a silicon vendor subdirectory
> would organise things better. It also matches what we do for
> arch/arm64/boot/dts/

A file structure like:

 tools/perf/pmu-events/arch/arm64/cavium/thunderx2/implementation-defined.json

looks quite horible. In contrast to dts dir we will need another
subdir for each platform and will then have two levels of subdirs.
This is also different to other archs where there is no extra vendor
dir.

I haven't checked if implementation allows json files to reside deeper
in the subdir level.

Anyway, it is bike-shedding.

-Robert

[PATCH v6 4/4] perf vendor events arm64: Add ThunderX2 implementation defined pmu core events

[Robert Richter]

On 11.10.17 13:19:27, Will Deacon wrote:
> On Tue, Aug 29, 2017 at 02:47:30PM +0200, Robert Richter wrote:
> > Shaokun,
> > 
> > On 29.08.17 17:26:00, Zhangshaokun wrote:
> > > On 2017/8/24 20:03, Ganapatrao Kulkarni wrote:
> > > > This is not a full event list, but a short list of useful events.
> > > > 
> > > > Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
> > > > ---
> > > >  tools/perf/pmu-events/arch/arm64/mapfile.csv       | 15 ++++++
> > > >  .../arm64/thunderx2/implementation-defined.json    | 62 ++++++++++++++++++++++
> > > >  2 files changed, 77 insertions(+)
> > > >  create mode 100644 tools/perf/pmu-events/arch/arm64/mapfile.csv
> > > >  create mode 100644 tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json
> > > > 
> > > 
> > > I saw you also used thunderx2 in tools/perf/pmu-events/arch/arm64/, how about John's suggestion
> > > that would use vendor sub-folder?
> > > Of course, appreciate maintainer's comments.
> > 
> > this would just add another level of subdirectories. I rather would
> > prefer to have a per platform dir comparable to what is listed in
> > 
> >  arch/arm64/Kconfig.platforms
> > 
> > This is the same as Ganapat has implemented it.
> 
> FWIW, I agree with Zhangshaokun here that a silicon vendor subdirectory
> would organise things better. It also matches what we do for
> arch/arm64/boot/dts/

A file structure like:

 tools/perf/pmu-events/arch/arm64/cavium/thunderx2/implementation-defined.json

looks quite horible. In contrast to dts dir we will need another
subdir for each platform and will then have two levels of subdirs.
This is also different to other archs where there is no extra vendor
dir.

I haven't checked if implementation allows json files to reside deeper
in the subdir level.

Anyway, it is bike-shedding.

-Robert

Re: [PATCH v6 4/4] perf vendor events arm64: Add ThunderX2 implementation defined pmu core events

[Will Deacon]

On Wed, Oct 11, 2017 at 03:24:31PM +0200, Robert Richter wrote:
> On 11.10.17 13:19:27, Will Deacon wrote:
> > On Tue, Aug 29, 2017 at 02:47:30PM +0200, Robert Richter wrote:
> > > Shaokun,
> > > 
> > > On 29.08.17 17:26:00, Zhangshaokun wrote:
> > > > On 2017/8/24 20:03, Ganapatrao Kulkarni wrote:
> > > > > This is not a full event list, but a short list of useful events.
> > > > > 
> > > > > Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
> > > > > ---
> > > > >  tools/perf/pmu-events/arch/arm64/mapfile.csv       | 15 ++++++
> > > > >  .../arm64/thunderx2/implementation-defined.json    | 62 ++++++++++++++++++++++
> > > > >  2 files changed, 77 insertions(+)
> > > > >  create mode 100644 tools/perf/pmu-events/arch/arm64/mapfile.csv
> > > > >  create mode 100644 tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json
> > > > > 
> > > > 
> > > > I saw you also used thunderx2 in tools/perf/pmu-events/arch/arm64/, how about John's suggestion
> > > > that would use vendor sub-folder?
> > > > Of course, appreciate maintainer's comments.
> > > 
> > > this would just add another level of subdirectories. I rather would
> > > prefer to have a per platform dir comparable to what is listed in
> > > 
> > >  arch/arm64/Kconfig.platforms
> > > 
> > > This is the same as Ganapat has implemented it.
> > 
> > FWIW, I agree with Zhangshaokun here that a silicon vendor subdirectory
> > would organise things better. It also matches what we do for
> > arch/arm64/boot/dts/
> 
> A file structure like:
> 
>  tools/perf/pmu-events/arch/arm64/cavium/thunderx2/implementation-defined.json
> 
> looks quite horible. In contrast to dts dir we will need another
> subdir for each platform and will then have two levels of subdirs.

In which case, call the file thunderx2-imp-def.json or something. The
advantage then is that the SoC names are namespaced by vendor, which is
the best way to avoid confusion imo.

Will

[PATCH v6 4/4] perf vendor events arm64: Add ThunderX2 implementation defined pmu core events

[Will Deacon]

On Wed, Oct 11, 2017 at 03:24:31PM +0200, Robert Richter wrote:
> On 11.10.17 13:19:27, Will Deacon wrote:
> > On Tue, Aug 29, 2017 at 02:47:30PM +0200, Robert Richter wrote:
> > > Shaokun,
> > > 
> > > On 29.08.17 17:26:00, Zhangshaokun wrote:
> > > > On 2017/8/24 20:03, Ganapatrao Kulkarni wrote:
> > > > > This is not a full event list, but a short list of useful events.
> > > > > 
> > > > > Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
> > > > > ---
> > > > >  tools/perf/pmu-events/arch/arm64/mapfile.csv       | 15 ++++++
> > > > >  .../arm64/thunderx2/implementation-defined.json    | 62 ++++++++++++++++++++++
> > > > >  2 files changed, 77 insertions(+)
> > > > >  create mode 100644 tools/perf/pmu-events/arch/arm64/mapfile.csv
> > > > >  create mode 100644 tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json
> > > > > 
> > > > 
> > > > I saw you also used thunderx2 in tools/perf/pmu-events/arch/arm64/, how about John's suggestion
> > > > that would use vendor sub-folder?
> > > > Of course, appreciate maintainer's comments.
> > > 
> > > this would just add another level of subdirectories. I rather would
> > > prefer to have a per platform dir comparable to what is listed in
> > > 
> > >  arch/arm64/Kconfig.platforms
> > > 
> > > This is the same as Ganapat has implemented it.
> > 
> > FWIW, I agree with Zhangshaokun here that a silicon vendor subdirectory
> > would organise things better. It also matches what we do for
> > arch/arm64/boot/dts/
> 
> A file structure like:
> 
>  tools/perf/pmu-events/arch/arm64/cavium/thunderx2/implementation-defined.json
> 
> looks quite horible. In contrast to dts dir we will need another
> subdir for each platform and will then have two levels of subdirs.

In which case, call the file thunderx2-imp-def.json or something. The
advantage then is that the SoC names are namespaced by vendor, which is
the best way to avoid confusion imo.

Will

Re: [PATCH v6 4/4] perf vendor events arm64: Add ThunderX2 implementation defined pmu core events

[Ganapatrao Kulkarni]

On Wed, Oct 11, 2017 at 7:25 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Wed, Oct 11, 2017 at 03:24:31PM +0200, Robert Richter wrote:
>> On 11.10.17 13:19:27, Will Deacon wrote:
>> > On Tue, Aug 29, 2017 at 02:47:30PM +0200, Robert Richter wrote:
>> > > Shaokun,
>> > >
>> > > On 29.08.17 17:26:00, Zhangshaokun wrote:
>> > > > On 2017/8/24 20:03, Ganapatrao Kulkarni wrote:
>> > > > > This is not a full event list, but a short list of useful events.
>> > > > >
>> > > > > Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
>> > > > > ---
>> > > > >  tools/perf/pmu-events/arch/arm64/mapfile.csv       | 15 ++++++
>> > > > >  .../arm64/thunderx2/implementation-defined.json    | 62 ++++++++++++++++++++++
>> > > > >  2 files changed, 77 insertions(+)
>> > > > >  create mode 100644 tools/perf/pmu-events/arch/arm64/mapfile.csv
>> > > > >  create mode 100644 tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json
>> > > > >
>> > > >
>> > > > I saw you also used thunderx2 in tools/perf/pmu-events/arch/arm64/, how about John's suggestion
>> > > > that would use vendor sub-folder?
>> > > > Of course, appreciate maintainer's comments.
>> > >
>> > > this would just add another level of subdirectories. I rather would
>> > > prefer to have a per platform dir comparable to what is listed in
>> > >
>> > >  arch/arm64/Kconfig.platforms
>> > >
>> > > This is the same as Ganapat has implemented it.
>> >
>> > FWIW, I agree with Zhangshaokun here that a silicon vendor subdirectory
>> > would organise things better. It also matches what we do for
>> > arch/arm64/boot/dts/
>>
>> A file structure like:
>>
>>  tools/perf/pmu-events/arch/arm64/cavium/thunderx2/implementation-defined.json
>>
>> looks quite horible. In contrast to dts dir we will need another
>> subdir for each platform and will then have two levels of subdirs.
>
> In which case, call the file thunderx2-imp-def.json or something. The
> advantage then is that the SoC names are namespaced by vendor, which is
> the best way to avoid confusion imo.

thanks, i can rename json file as suggested.

>
> Will

thanks
Ganapat

[PATCH v6 4/4] perf vendor events arm64: Add ThunderX2 implementation defined pmu core events

[Ganapatrao Kulkarni]

On Wed, Oct 11, 2017 at 7:25 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Wed, Oct 11, 2017 at 03:24:31PM +0200, Robert Richter wrote:
>> On 11.10.17 13:19:27, Will Deacon wrote:
>> > On Tue, Aug 29, 2017 at 02:47:30PM +0200, Robert Richter wrote:
>> > > Shaokun,
>> > >
>> > > On 29.08.17 17:26:00, Zhangshaokun wrote:
>> > > > On 2017/8/24 20:03, Ganapatrao Kulkarni wrote:
>> > > > > This is not a full event list, but a short list of useful events.
>> > > > >
>> > > > > Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
>> > > > > ---
>> > > > >  tools/perf/pmu-events/arch/arm64/mapfile.csv       | 15 ++++++
>> > > > >  .../arm64/thunderx2/implementation-defined.json    | 62 ++++++++++++++++++++++
>> > > > >  2 files changed, 77 insertions(+)
>> > > > >  create mode 100644 tools/perf/pmu-events/arch/arm64/mapfile.csv
>> > > > >  create mode 100644 tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json
>> > > > >
>> > > >
>> > > > I saw you also used thunderx2 in tools/perf/pmu-events/arch/arm64/, how about John's suggestion
>> > > > that would use vendor sub-folder?
>> > > > Of course, appreciate maintainer's comments.
>> > >
>> > > this would just add another level of subdirectories. I rather would
>> > > prefer to have a per platform dir comparable to what is listed in
>> > >
>> > >  arch/arm64/Kconfig.platforms
>> > >
>> > > This is the same as Ganapat has implemented it.
>> >
>> > FWIW, I agree with Zhangshaokun here that a silicon vendor subdirectory
>> > would organise things better. It also matches what we do for
>> > arch/arm64/boot/dts/
>>
>> A file structure like:
>>
>>  tools/perf/pmu-events/arch/arm64/cavium/thunderx2/implementation-defined.json
>>
>> looks quite horible. In contrast to dts dir we will need another
>> subdir for each platform and will then have two levels of subdirs.
>
> In which case, call the file thunderx2-imp-def.json or something. The
> advantage then is that the SoC names are namespaced by vendor, which is
> the best way to avoid confusion imo.

thanks, i can rename json file as suggested.

>
> Will

thanks
Ganapat

Re: [PATCH v6 2/4] perf tools arm64: Add support for get_cpuid_str function.

[Ganapatrao Kulkarni]

On Wed, Oct 11, 2017 at 5:43 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Thu, Aug 24, 2017 at 05:33:47PM +0530, Ganapatrao Kulkarni wrote:
>> function get_cpuid_str returns MIDR string of the first online
>> cpu from the range of cpus associated with the pmu core device.
>>
>> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
>> ---
>>  tools/perf/arch/arm64/util/Build    |  1 +
>>  tools/perf/arch/arm64/util/header.c | 60 +++++++++++++++++++++++++++++++++++++
>>  2 files changed, 61 insertions(+)
>>  create mode 100644 tools/perf/arch/arm64/util/header.c
>>
>> diff --git a/tools/perf/arch/arm64/util/Build b/tools/perf/arch/arm64/util/Build
>> index cef6fb3..b1ab72d 100644
>> --- a/tools/perf/arch/arm64/util/Build
>> +++ b/tools/perf/arch/arm64/util/Build
>> @@ -1,3 +1,4 @@
>> +libperf-y += header.o
>>  libperf-$(CONFIG_DWARF)     += dwarf-regs.o
>>  libperf-$(CONFIG_LOCAL_LIBUNWIND) += unwind-libunwind.o
>>
>> diff --git a/tools/perf/arch/arm64/util/header.c b/tools/perf/arch/arm64/util/header.c
>> new file mode 100644
>> index 0000000..f02a32e
>> --- /dev/null
>> +++ b/tools/perf/arch/arm64/util/header.c
>> @@ -0,0 +1,60 @@
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <api/fs/fs.h>
>> +#include "header.h"
>> +
>> +#define MIDR "/regs/identification/midr_el1"
>> +#define MIDR_SIZE 19
>> +
>> +char *get_cpuid_str(struct perf_pmu *pmu)
>> +{
>> +     char *buf = NULL;
>> +     char path[PATH_MAX];
>> +     const char *sysfs = sysfs__mountpoint();
>> +     int cpu;
>> +     u64 midr = 0;
>> +     struct cpu_map *cpus;
>> +     FILE *file;
>> +
>> +     if (!sysfs || !pmu->cpus)
>> +             return NULL;
>> +
>> +     buf = malloc(MIDR_SIZE);
>> +     if (!buf)
>> +             return NULL;
>> +
>> +     /* read midr from list of cpus mapped to this pmu */
>> +     cpus = cpu_map__get(pmu->cpus);
>> +     for (cpu = 0; cpu < cpus->nr; cpu++) {
>> +             scnprintf(path, PATH_MAX, "%s/devices/system/cpu/cpu%d"MIDR,
>> +                             sysfs, cpus->map[cpu]);
>> +
>> +             file = fopen(path, "r");
>> +             if (!file) {
>> +                     pr_debug("fopen failed for file %s\n", path);
>> +                     continue;
>> +             }
>> +
>> +             if (!fgets(buf, MIDR_SIZE, file))
>> +                     continue;
>> +             fclose(file);
>
> Don't you want to fclose the file if the fgets fails?

thanks.
>
>> +
>> +             /* Ignore/clear Variant[23:20] and
>> +              * Revision[3:0] of MIDR
>> +              */
>> +             midr = strtoul(buf, NULL, 16);
>> +             midr &= (~(0xf << 20 | 0xf));
>
> It would be cleaner if you had #defines for the MIDR fields that you're
> masking.

ok, i can add it.
I felt, it is not to required, since it is not used anywhere else and
for better code readability.

>
>> +             scnprintf(buf, MIDR_SIZE, "0x%016lx", midr);
>> +             /* got midr break loop */
>> +             break;
>> +     }
>> +
>> +     if (!midr) {
>> +             pr_err("failed to get cpuid string\n");
>
> Might be helpful to print the PMU name and the CPU map if there's a way
> to do that.

ok, i can print name,  need to check for cpu map.
>
> Will

thanks
Ganapat

[PATCH v6 2/4] perf tools arm64: Add support for get_cpuid_str function.

[Ganapatrao Kulkarni]

On Wed, Oct 11, 2017 at 5:43 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Thu, Aug 24, 2017 at 05:33:47PM +0530, Ganapatrao Kulkarni wrote:
>> function get_cpuid_str returns MIDR string of the first online
>> cpu from the range of cpus associated with the pmu core device.
>>
>> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
>> ---
>>  tools/perf/arch/arm64/util/Build    |  1 +
>>  tools/perf/arch/arm64/util/header.c | 60 +++++++++++++++++++++++++++++++++++++
>>  2 files changed, 61 insertions(+)
>>  create mode 100644 tools/perf/arch/arm64/util/header.c
>>
>> diff --git a/tools/perf/arch/arm64/util/Build b/tools/perf/arch/arm64/util/Build
>> index cef6fb3..b1ab72d 100644
>> --- a/tools/perf/arch/arm64/util/Build
>> +++ b/tools/perf/arch/arm64/util/Build
>> @@ -1,3 +1,4 @@
>> +libperf-y += header.o
>>  libperf-$(CONFIG_DWARF)     += dwarf-regs.o
>>  libperf-$(CONFIG_LOCAL_LIBUNWIND) += unwind-libunwind.o
>>
>> diff --git a/tools/perf/arch/arm64/util/header.c b/tools/perf/arch/arm64/util/header.c
>> new file mode 100644
>> index 0000000..f02a32e
>> --- /dev/null
>> +++ b/tools/perf/arch/arm64/util/header.c
>> @@ -0,0 +1,60 @@
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <api/fs/fs.h>
>> +#include "header.h"
>> +
>> +#define MIDR "/regs/identification/midr_el1"
>> +#define MIDR_SIZE 19
>> +
>> +char *get_cpuid_str(struct perf_pmu *pmu)
>> +{
>> +     char *buf = NULL;
>> +     char path[PATH_MAX];
>> +     const char *sysfs = sysfs__mountpoint();
>> +     int cpu;
>> +     u64 midr = 0;
>> +     struct cpu_map *cpus;
>> +     FILE *file;
>> +
>> +     if (!sysfs || !pmu->cpus)
>> +             return NULL;
>> +
>> +     buf = malloc(MIDR_SIZE);
>> +     if (!buf)
>> +             return NULL;
>> +
>> +     /* read midr from list of cpus mapped to this pmu */
>> +     cpus = cpu_map__get(pmu->cpus);
>> +     for (cpu = 0; cpu < cpus->nr; cpu++) {
>> +             scnprintf(path, PATH_MAX, "%s/devices/system/cpu/cpu%d"MIDR,
>> +                             sysfs, cpus->map[cpu]);
>> +
>> +             file = fopen(path, "r");
>> +             if (!file) {
>> +                     pr_debug("fopen failed for file %s\n", path);
>> +                     continue;
>> +             }
>> +
>> +             if (!fgets(buf, MIDR_SIZE, file))
>> +                     continue;
>> +             fclose(file);
>
> Don't you want to fclose the file if the fgets fails?

thanks.
>
>> +
>> +             /* Ignore/clear Variant[23:20] and
>> +              * Revision[3:0] of MIDR
>> +              */
>> +             midr = strtoul(buf, NULL, 16);
>> +             midr &= (~(0xf << 20 | 0xf));
>
> It would be cleaner if you had #defines for the MIDR fields that you're
> masking.

ok, i can add it.
I felt, it is not to required, since it is not used anywhere else and
for better code readability.

>
>> +             scnprintf(buf, MIDR_SIZE, "0x%016lx", midr);
>> +             /* got midr break loop */
>> +             break;
>> +     }
>> +
>> +     if (!midr) {
>> +             pr_err("failed to get cpuid string\n");
>
> Might be helpful to print the PMU name and the CPU map if there's a way
> to do that.

ok, i can print name,  need to check for cpu map.
>
> Will

thanks
Ganapat