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

[PATCH 0/3] 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.

Ganapatrao Kulkarni (3):
  perf jevents: Add support to use core pmu name other than cpu
  perf tools arm64: implement function get_cpuid_str
  perf tool, arm64, thunderx2: Add implementation defined events for
    thunderx2

 tools/perf/arch/arm64/util/Build                   |  1 +
 tools/perf/arch/arm64/util/header.c                | 51 +++++++++++++++
 tools/perf/pmu-events/arch/arm64/mapfile.csv       |  2 +
 .../arm64/thunderx2/implementation-defined.json    | 72 ++++++++++++++++++++++
 tools/perf/pmu-events/jevents.c                    | 15 +++--
 tools/perf/pmu-events/jevents.h                    |  3 +-
 tools/perf/pmu-events/pmu-events.h                 |  1 +
 tools/perf/util/pmu.c                              |  2 +-
 8 files changed, 140 insertions(+), 7 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

-- 
1.8.1.4

[PATCH 1/3] perf jevents: Add support to use core pmu name other than cpu

[Ganapatrao Kulkarni]

jevents assumes core event devices are listed as cpu in
sysfs, however some architecture like arm64 have custom names.

Adding provision in json files to define CPU name and
required changes in jevents.c to parse.

Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
---
 tools/perf/pmu-events/jevents.c    | 15 ++++++++++-----
 tools/perf/pmu-events/jevents.h    |  3 ++-
 tools/perf/pmu-events/pmu-events.h |  1 +
 tools/perf/util/pmu.c              |  2 +-
 4 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
index eed0934..b20e7d1 100644
--- a/tools/perf/pmu-events/jevents.c
+++ b/tools/perf/pmu-events/jevents.c
@@ -289,7 +289,8 @@ 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,
+static int print_events_table_entry(void *data, char *name, char *cpu,
+				    char *event,
 				    char *desc, char *long_desc,
 				    char *pmu, char *unit, char *perpkg)
 {
@@ -304,6 +305,7 @@ static int print_events_table_entry(void *data, char *name, char *event,
 	fprintf(outfp, "{\n");
 
 	fprintf(outfp, "\t.name = \"%s\",\n", name);
+	fprintf(outfp, "\t.cpu = \"%s\",\n", cpu);
 	fprintf(outfp, "\t.event = \"%s\",\n", event);
 	fprintf(outfp, "\t.desc = \"%s\",\n", desc);
 	fprintf(outfp, "\t.topic = \"%s\",\n", topic);
@@ -360,8 +362,8 @@ static char *real_event(const char *name, char *event)
 
 /* 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,
+	  int (*func)(void *data, char *name, char *cpu, char *event,
+		      char *desc, char *long_desc,
 		      char *pmu, char *unit, char *perpkg),
 	  void *data)
 {
@@ -381,7 +383,8 @@ 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;
+		char *name = NULL, *cpu = NULL;
 		char *long_desc = NULL;
 		char *extra_desc = NULL;
 		char *pmu = NULL;
@@ -421,6 +424,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, "CPU")) {
+				addfield(map, &cpu, "", "", val);
 			} else if (json_streq(map, field, "BriefDescription")) {
 				addfield(map, &desc, "", "", val);
 				fixdesc(desc);
@@ -487,7 +492,7 @@ int json_events(const char *fn,
 			addfield(map, &event, ",", msr->pname, msrval);
 		fixname(name);
 
-		err = func(data, name, real_event(name, event), desc, long_desc,
+		err = func(data, name, cpu, real_event(name, event), desc, long_desc,
 				pmu, unit, perpkg);
 		free(event);
 		free(desc);
diff --git a/tools/perf/pmu-events/jevents.h b/tools/perf/pmu-events/jevents.h
index 71e13de..d423295 100644
--- a/tools/perf/pmu-events/jevents.h
+++ b/tools/perf/pmu-events/jevents.h
@@ -2,7 +2,8 @@
 #define JEVENTS_H 1
 
 int json_events(const char *fn,
-		int (*func)(void *data, char *name, char *event, char *desc,
+		int (*func)(void *data, char *name, char *cpu,
+				char *event, char *desc,
 				char *long_desc,
 				char *pmu,
 				char *unit, char *perpkg),
diff --git a/tools/perf/pmu-events/pmu-events.h b/tools/perf/pmu-events/pmu-events.h
index c669a3c..c8da522 100644
--- a/tools/perf/pmu-events/pmu-events.h
+++ b/tools/perf/pmu-events/pmu-events.h
@@ -6,6 +6,7 @@
  */
 struct pmu_event {
 	const char *name;
+	const char *cpu;
 	const char *event;
 	const char *desc;
 	const char *topic;
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 12f84dd..31b02d4 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -556,7 +556,7 @@ static void pmu_add_cpu_aliases(struct list_head *head, const char *name)
 		if (!pe->name)
 			break;
 
-		pname = pe->pmu ? pe->pmu : "cpu";
+		pname = pe->pmu ? pe->pmu : pe->cpu ? pe->cpu :"cpu";
 		if (strncmp(pname, name, strlen(pname)))
 			continue;
 
-- 
1.8.1.4

[PATCH 2/3] perf tools arm64: implement function get_cpuid_str

[Ganapatrao Kulkarni]

function get_cpuid_str returns midr_el1 value from first
online cpu available.

Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
---
 tools/perf/arch/arm64/util/Build    |  1 +
 tools/perf/arch/arm64/util/header.c | 51 +++++++++++++++++++++++++++++++++++++
 2 files changed, 52 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..c0468fe
--- /dev/null
+++ b/tools/perf/arch/arm64/util/header.c
@@ -0,0 +1,51 @@
+#include <fnmatch.h>
+#include <dirent.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <linux/stringify.h>
+#include "header.h"
+#include "util.h"
+
+#define LINUX_SYS_CPU_DIRECTORY "/sys/devices/system/cpu/"
+#define MIDR "/regs/identification/midr_el1"
+
+char *get_cpuid_str(void)
+{
+	char *buf = malloc(128);
+	FILE *file;
+	char *ret = NULL;
+	int cpu , nr_cpu;
+	char sys_file[256];
+
+	sprintf(sys_file, LINUX_SYS_CPU_DIRECTORY"kernel_max");
+
+	file = fopen(sys_file, "r");
+	if (!file)
+		return ret;
+
+	if (fscanf(file, "%d", &nr_cpu) != 1)
+		return ret;
+
+	/* kernel_max returns nr_cpu -1 */
+	nr_cpu++;
+
+	/*iterate till you read midr of first online cpu */
+	for(cpu = 0; cpu < nr_cpu; cpu++)
+	{
+		sprintf(sys_file, LINUX_SYS_CPU_DIRECTORY"cpu%d"MIDR, cpu);
+		file = fopen(sys_file, "r");
+		if (file) {
+			ret = fgets(buf, 128, file);
+			if (ret) {
+				buf[strlen(buf)-1] ='\0';
+				fclose(file);
+				return buf;
+			}
+			fclose(file);
+		}
+	}
+	free(buf);
+	return ret;
+}
-- 
1.8.1.4

Re: [PATCH 2/3] perf tools arm64: implement function get_cpuid_str

[Mark Rutland]

On Tue, Apr 04, 2017 at 01:06:42PM +0530, Ganapatrao Kulkarni wrote:
> function get_cpuid_str returns midr_el1 value from first
> online cpu available.

This isn't globally the same (e.g. in big.LITTLE systems).

It would be far better to have the function take a PMU, and look at the
first CPU in that PMU's mask.

Thanks,
Mark.

> 
> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
> ---
>  tools/perf/arch/arm64/util/Build    |  1 +
>  tools/perf/arch/arm64/util/header.c | 51 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 52 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..c0468fe
> --- /dev/null
> +++ b/tools/perf/arch/arm64/util/header.c
> @@ -0,0 +1,51 @@
> +#include <fnmatch.h>
> +#include <dirent.h>
> +#include <unistd.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <linux/stringify.h>
> +#include "header.h"
> +#include "util.h"
> +
> +#define LINUX_SYS_CPU_DIRECTORY "/sys/devices/system/cpu/"
> +#define MIDR "/regs/identification/midr_el1"
> +
> +char *get_cpuid_str(void)
> +{
> +	char *buf = malloc(128);
> +	FILE *file;
> +	char *ret = NULL;
> +	int cpu , nr_cpu;
> +	char sys_file[256];
> +
> +	sprintf(sys_file, LINUX_SYS_CPU_DIRECTORY"kernel_max");
> +
> +	file = fopen(sys_file, "r");
> +	if (!file)
> +		return ret;
> +
> +	if (fscanf(file, "%d", &nr_cpu) != 1)
> +		return ret;
> +
> +	/* kernel_max returns nr_cpu -1 */
> +	nr_cpu++;
> +
> +	/*iterate till you read midr of first online cpu */
> +	for(cpu = 0; cpu < nr_cpu; cpu++)
> +	{
> +		sprintf(sys_file, LINUX_SYS_CPU_DIRECTORY"cpu%d"MIDR, cpu);
> +		file = fopen(sys_file, "r");
> +		if (file) {
> +			ret = fgets(buf, 128, file);
> +			if (ret) {
> +				buf[strlen(buf)-1] ='\0';
> +				fclose(file);
> +				return buf;
> +			}
> +			fclose(file);
> +		}
> +	}
> +	free(buf);
> +	return ret;
> +}
> -- 
> 1.8.1.4
> 

Re: [PATCH 2/3] perf tools arm64: implement function get_cpuid_str

[Ganapatrao Kulkarni]

On Tue, Apr 4, 2017 at 5:55 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Tue, Apr 04, 2017 at 01:06:42PM +0530, Ganapatrao Kulkarni wrote:
>> function get_cpuid_str returns midr_el1 value from first
>> online cpu available.
>
> This isn't globally the same (e.g. in big.LITTLE systems).
>
> It would be far better to have the function take a PMU, and look at the
> first CPU in that PMU's mask.

thanks, will do.
>
> Thanks,
> Mark.
>
>>
>> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
>> ---
>>  tools/perf/arch/arm64/util/Build    |  1 +
>>  tools/perf/arch/arm64/util/header.c | 51 +++++++++++++++++++++++++++++++++++++
>>  2 files changed, 52 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..c0468fe
>> --- /dev/null
>> +++ b/tools/perf/arch/arm64/util/header.c
>> @@ -0,0 +1,51 @@
>> +#include <fnmatch.h>
>> +#include <dirent.h>
>> +#include <unistd.h>
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <string.h>
>> +#include <linux/stringify.h>
>> +#include "header.h"
>> +#include "util.h"
>> +
>> +#define LINUX_SYS_CPU_DIRECTORY "/sys/devices/system/cpu/"
>> +#define MIDR "/regs/identification/midr_el1"
>> +
>> +char *get_cpuid_str(void)
>> +{
>> +     char *buf = malloc(128);
>> +     FILE *file;
>> +     char *ret = NULL;
>> +     int cpu , nr_cpu;
>> +     char sys_file[256];
>> +
>> +     sprintf(sys_file, LINUX_SYS_CPU_DIRECTORY"kernel_max");
>> +
>> +     file = fopen(sys_file, "r");
>> +     if (!file)
>> +             return ret;
>> +
>> +     if (fscanf(file, "%d", &nr_cpu) != 1)
>> +             return ret;
>> +
>> +     /* kernel_max returns nr_cpu -1 */
>> +     nr_cpu++;
>> +
>> +     /*iterate till you read midr of first online cpu */
>> +     for(cpu = 0; cpu < nr_cpu; cpu++)
>> +     {
>> +             sprintf(sys_file, LINUX_SYS_CPU_DIRECTORY"cpu%d"MIDR, cpu);
>> +             file = fopen(sys_file, "r");
>> +             if (file) {
>> +                     ret = fgets(buf, 128, file);
>> +                     if (ret) {
>> +                             buf[strlen(buf)-1] ='\0';
>> +                             fclose(file);
>> +                             return buf;
>> +                     }
>> +                     fclose(file);
>> +             }
>> +     }
>> +     free(buf);
>> +     return ret;
>> +}
>> --
>> 1.8.1.4
>>

thanks
Ganapat

[PATCH 3/3] perf tool, arm64, thunderx2: Add implementation defined events for ThunderX2

[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       |  2 +
 .../arm64/thunderx2/implementation-defined.json    | 72 ++++++++++++++++++++++
 2 files changed, 74 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..ba30e43
--- /dev/null
+++ b/tools/perf/pmu-events/arch/arm64/mapfile.csv
@@ -0,0 +1,2 @@
+Family-model,Version,Filename,EventType
+0x00000000420f5161,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..360e084
--- /dev/null
+++ b/tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json
@@ -0,0 +1,72 @@
+[
+    {
+        "PublicDescription": "Attributable Level 1 data cache access, read",
+        "EventCode": "0x40",
+        "EventName": "l1d_cache_access_read",
+        "BriefDescription": "l1d cache access, read",
+	"CPU" :"armv8_pmuv3_0"
+    },
+    {
+        "PublicDescription": "Attributable Level 1 data cache access, write ",
+        "EventCode": "0x41",
+        "EventName": "l1d_cache_access_write",
+        "BriefDescription": "l1d cache access, write",
+	"CPU" :"armv8_pmuv3_0"
+    },
+    {
+        "PublicDescription": "Attributable Level 1 data cache refill, read",
+        "EventCode": "0x42",
+        "EventName": "l1d_cache_refill_read",
+        "BriefDescription": "l1d cache refill, read",
+	"CPU" :"armv8_pmuv3_0"
+    },
+    {
+        "PublicDescription": "Attributable Level 1 data cache refill, write",
+        "EventCode": "0x43",
+        "EventName": "l1d_cache_refill_write",
+        "BriefDescription": "l1d refill, write",
+	"CPU" :"armv8_pmuv3_0"
+    },
+    {
+        "PublicDescription": "Attributable Level 1 data TLB refill, read",
+        "EventCode": "0x4C",
+        "EventName": "l1d_tlb_refill_read",
+        "BriefDescription": "l1d tlb refill, read",
+	"CPU" :"armv8_pmuv3_0"
+    },
+    {
+        "PublicDescription": "Attributable Level 1 data TLB refill, write",
+        "EventCode": "0x4D",
+        "EventName": "l1d_tlb_refill_write",
+        "BriefDescription": "l1d tlb refill, write",
+	"CPU" :"armv8_pmuv3_0"
+    },
+    {
+        "PublicDescription": "Attributable Level 1 data or unified TLB access, read",
+        "EventCode": "0x4E",
+        "EventName": "l1d_tlb_read",
+        "BriefDescription": "l1d tlb, read",
+	"CPU" :"armv8_pmuv3_0"
+    },
+    {
+        "PublicDescription": "Attributable Level 1 data or unified TLB access, write",
+        "EventCode": "0x4F",
+        "EventName": "l1d_tlb_write",
+        "BriefDescription": "l1d tlb, write",
+	"CPU" :"armv8_pmuv3_0"
+    },
+    {
+        "PublicDescription": "Bus access, read",
+        "EventCode": "0x60",
+        "EventName": "bus_access_read",
+        "BriefDescription": "Bus access, read",
+	"CPU" :"armv8_pmuv3_0"
+   },
+   {
+        "PublicDescription": "Bus access, write",
+        "EventCode": "0x61",
+        "EventName": "bus_access_write",
+        "BriefDescription": "Bus access, write",
+	"CPU" :"armv8_pmuv3_0"
+   }
+]
-- 
1.8.1.4

Re: [PATCH 3/3] perf tool, arm64, thunderx2: Add implementation defined events for ThunderX2

[Mark Rutland]

On Tue, Apr 04, 2017 at 01:06:43PM +0530, 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       |  2 +
>  .../arm64/thunderx2/implementation-defined.json    | 72 ++++++++++++++++++++++
>  2 files changed, 74 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..ba30e43
> --- /dev/null
> +++ b/tools/perf/pmu-events/arch/arm64/mapfile.csv
> @@ -0,0 +1,2 @@
> +Family-model,Version,Filename,EventType
> +0x00000000420f5161,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..360e084
> --- /dev/null
> +++ b/tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json
> @@ -0,0 +1,72 @@
> +[
> +    {
> +        "PublicDescription": "Attributable Level 1 data cache access, read",
> +        "EventCode": "0x40",
> +        "EventName": "l1d_cache_access_read",
> +        "BriefDescription": "l1d cache access, read",
> +	"CPU" :"armv8_pmuv3_0"

Please let's not hard-code the name like this. Surely we can get rid of this?

The kernel doesn't currently name PMUs as armv8_pmuv3_*, and as that can
differ across DT/ACPI and in big.LITTLE, I don't think it makes sense to
try to rely one particular string regardless.

Thanks,
Mark.

Re: [PATCH 3/3] perf tool, arm64, thunderx2: Add implementation defined events for ThunderX2

[Ganapatrao Kulkarni]

On Tue, Apr 4, 2017 at 5:58 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Tue, Apr 04, 2017 at 01:06:43PM +0530, 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       |  2 +
>>  .../arm64/thunderx2/implementation-defined.json    | 72 ++++++++++++++++++++++
>>  2 files changed, 74 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..ba30e43
>> --- /dev/null
>> +++ b/tools/perf/pmu-events/arch/arm64/mapfile.csv
>> @@ -0,0 +1,2 @@
>> +Family-model,Version,Filename,EventType
>> +0x00000000420f5161,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..360e084
>> --- /dev/null
>> +++ b/tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json
>> @@ -0,0 +1,72 @@
>> +[
>> +    {
>> +        "PublicDescription": "Attributable Level 1 data cache access, read",
>> +        "EventCode": "0x40",
>> +        "EventName": "l1d_cache_access_read",
>> +        "BriefDescription": "l1d cache access, read",
>> +     "CPU" :"armv8_pmuv3_0"
>
> Please let's not hard-code the name like this. Surely we can get rid of this?
>
> The kernel doesn't currently name PMUs as armv8_pmuv3_*, and as that can
> differ across DT/ACPI and in big.LITTLE, I don't think it makes sense to
> try to rely one particular string regardless.

This string/name is fixed for a platform. having name here is essential to
know which devices among pmu (armv8_pmuv3_0, breakpoint, software)
devices, these jevents to be added.
also this json file is specific to a arch/soc/board, it is not a
generic file to be common.


>
> Thanks,
> Mark.

thanks
Ganapat

Re: [PATCH 3/3] perf tool, arm64, thunderx2: Add implementation defined events for ThunderX2

[Mark Rutland]

On Wed, Apr 05, 2017 at 02:42:39PM +0530, Ganapatrao Kulkarni wrote:
> On Tue, Apr 4, 2017 at 5:58 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Tue, Apr 04, 2017 at 01:06:43PM +0530, 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       |  2 +
> >>  .../arm64/thunderx2/implementation-defined.json    | 72 ++++++++++++++++++++++
> >>  2 files changed, 74 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..ba30e43
> >> --- /dev/null
> >> +++ b/tools/perf/pmu-events/arch/arm64/mapfile.csv
> >> @@ -0,0 +1,2 @@
> >> +Family-model,Version,Filename,EventType
> >> +0x00000000420f5161,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..360e084
> >> --- /dev/null
> >> +++ b/tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json
> >> @@ -0,0 +1,72 @@
> >> +[
> >> +    {
> >> +        "PublicDescription": "Attributable Level 1 data cache access, read",
> >> +        "EventCode": "0x40",
> >> +        "EventName": "l1d_cache_access_read",
> >> +        "BriefDescription": "l1d cache access, read",
> >> +     "CPU" :"armv8_pmuv3_0"
> >
> > Please let's not hard-code the name like this. Surely we can get rid of this?
> >
> > The kernel doesn't currently name PMUs as armv8_pmuv3_*, and as that can
> > differ across DT/ACPI and in big.LITTLE, I don't think it makes sense to
> > try to rely one particular string regardless.
> 
> This string/name is fixed for a platform. having name here is essential to
> know which devices among pmu (armv8_pmuv3_0, breakpoint, software)
> devices, these jevents to be added.
> also this json file is specific to a arch/soc/board, it is not a
> generic file to be common.

This file describe the events of a CPU PMU, and CPUs are not specific to
a platform in general. There are many systems using Cortex-A57, for
example.

Across big.LITTLE SoCs with Cortex-A57, there's no guarantee as to
whether the Cortex-A57 cores would be named armv8_pmuv3_0, or
armv8_pmuv3_1, etc. This would depend on the boot CPU, probe order of
secondaries, etc.

I appreciate that your platform is homnogeneous, and you may not expect
the core to be reused in any heterogeneous system. However, I think that
if we're going to make this work for arm64 we should handle the general
case, rather than only having it support a limited set of platforms.

Currently, we don't have an "official" way of identifying which PMUs are
CPU PMUs, but one way we could idtentify them would be to look at if
they have a "cpus" attribute under sysfs (rather than a "cpumask"
attribute).

Thanks,
Mark.

Re: [PATCH 3/3] perf tool, arm64, thunderx2: Add implementation defined events for ThunderX2

[Ganapatrao Kulkarni]

On Wed, Apr 5, 2017 at 3:35 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Wed, Apr 05, 2017 at 02:42:39PM +0530, Ganapatrao Kulkarni wrote:
>> On Tue, Apr 4, 2017 at 5:58 PM, Mark Rutland <mark.rutland@arm.com> wrote:
>> > On Tue, Apr 04, 2017 at 01:06:43PM +0530, 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       |  2 +
>> >>  .../arm64/thunderx2/implementation-defined.json    | 72 ++++++++++++++++++++++
>> >>  2 files changed, 74 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..ba30e43
>> >> --- /dev/null
>> >> +++ b/tools/perf/pmu-events/arch/arm64/mapfile.csv
>> >> @@ -0,0 +1,2 @@
>> >> +Family-model,Version,Filename,EventType
>> >> +0x00000000420f5161,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..360e084
>> >> --- /dev/null
>> >> +++ b/tools/perf/pmu-events/arch/arm64/thunderx2/implementation-defined.json
>> >> @@ -0,0 +1,72 @@
>> >> +[
>> >> +    {
>> >> +        "PublicDescription": "Attributable Level 1 data cache access, read",
>> >> +        "EventCode": "0x40",
>> >> +        "EventName": "l1d_cache_access_read",
>> >> +        "BriefDescription": "l1d cache access, read",
>> >> +     "CPU" :"armv8_pmuv3_0"
>> >
>> > Please let's not hard-code the name like this. Surely we can get rid of this?
>> >
>> > The kernel doesn't currently name PMUs as armv8_pmuv3_*, and as that can
>> > differ across DT/ACPI and in big.LITTLE, I don't think it makes sense to
>> > try to rely one particular string regardless.
>>
>> This string/name is fixed for a platform. having name here is essential to
>> know which devices among pmu (armv8_pmuv3_0, breakpoint, software)
>> devices, these jevents to be added.
>> also this json file is specific to a arch/soc/board, it is not a
>> generic file to be common.
>
> This file describe the events of a CPU PMU, and CPUs are not specific to
> a platform in general. There are many systems using Cortex-A57, for
> example.
>
> Across big.LITTLE SoCs with Cortex-A57, there's no guarantee as to
> whether the Cortex-A57 cores would be named armv8_pmuv3_0, or
> armv8_pmuv3_1, etc. This would depend on the boot CPU, probe order of
> secondaries, etc.

OK, we may not have complete name however, common part can be used to recognize
the PMU CORE devices from /sys/bus/event_source/devices
i.e we can have CPU id as "armv8_pmuv3".

same is extended to UNCORE as well.

mapfile.csv file will have entry for both BIG and LITTLE processors event files.
the jevents creates table of pmu_events_map for all entries present in
mapfile.csv file
while lookup, which ever pmu matches the cpuid of pmu_events_map
then corresponding table created from json file is used to add the
jevents to that PMU.

>
> I appreciate that your platform is homnogeneous, and you may not expect
> the core to be reused in any heterogeneous system. However, I think that
> if we're going to make this work for arm64 we should handle the general
> case, rather than only having it support a limited set of platforms.
>
> Currently, we don't have an "official" way of identifying which PMUs are
> CPU PMUs, but one way we could idtentify them would be to look at if
> they have a "cpus" attribute under sysfs (rather than a "cpumask"
> attribute).
>
> Thanks,
> Mark.

thanks
Ganapat

Re: [PATCH 3/3] perf tool, arm64, thunderx2: Add implementation defined events for ThunderX2

[Mark Rutland]

On Thu, Apr 06, 2017 at 09:50:33AM +0530, Ganapatrao Kulkarni wrote:
> On Wed, Apr 5, 2017 at 3:35 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Wed, Apr 05, 2017 at 02:42:39PM +0530, Ganapatrao Kulkarni wrote:
> >> On Tue, Apr 4, 2017 at 5:58 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> >> > On Tue, Apr 04, 2017 at 01:06:43PM +0530, Ganapatrao Kulkarni wrote:

> >> >> +     "CPU" :"armv8_pmuv3_0"
> >> >
> >> > Please let's not hard-code the name like this. Surely we can get rid of this?
> >> >
> >> > The kernel doesn't currently name PMUs as armv8_pmuv3_*, and as that can
> >> > differ across DT/ACPI and in big.LITTLE, I don't think it makes sense to
> >> > try to rely one particular string regardless.
> >>
> >> This string/name is fixed for a platform. having name here is essential to
> >> know which devices among pmu (armv8_pmuv3_0, breakpoint, software)
> >> devices, these jevents to be added.
> >> also this json file is specific to a arch/soc/board, it is not a
> >> generic file to be common.
> >
> > This file describe the events of a CPU PMU, and CPUs are not specific to
> > a platform in general. There are many systems using Cortex-A57, for
> > example.
> >
> > Across big.LITTLE SoCs with Cortex-A57, there's no guarantee as to
> > whether the Cortex-A57 cores would be named armv8_pmuv3_0, or
> > armv8_pmuv3_1, etc. This would depend on the boot CPU, probe order of
> > secondaries, etc.
> 
> OK, we may not have complete name however, common part can be used to recognize
> the PMU CORE devices from /sys/bus/event_source/devices
> i.e we can have CPU id as "armv8_pmuv3".

For better or worse, that's not the case on DT systems.

I'd much rather that we identified the CPU PMUs without requiring
particular names (e.g by looking for a "cpus" attribute).

> same is extended to UNCORE as well.

Could you elaborate on that? I'm not sure I follow.

> mapfile.csv file will have entry for both BIG and LITTLE processors event files.
> the jevents creates table of pmu_events_map for all entries present in
> mapfile.csv file
> while lookup, which ever pmu matches the cpuid of pmu_events_map
> then corresponding table created from json file is used to add the
> jevents to that PMU.

Sorry, but I don't follow how that's related to the above.

Thanks,
Mark.

Re: [PATCH 3/3] perf tool, arm64, thunderx2: Add implementation defined events for ThunderX2

[Ganapatrao Kulkarni]

Hi Mark,


On Thu, Apr 6, 2017 at 3:25 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Thu, Apr 06, 2017 at 09:50:33AM +0530, Ganapatrao Kulkarni wrote:
>> On Wed, Apr 5, 2017 at 3:35 PM, Mark Rutland <mark.rutland@arm.com> wrote:
>> > On Wed, Apr 05, 2017 at 02:42:39PM +0530, Ganapatrao Kulkarni wrote:
>> >> On Tue, Apr 4, 2017 at 5:58 PM, Mark Rutland <mark.rutland@arm.com> wrote:
>> >> > On Tue, Apr 04, 2017 at 01:06:43PM +0530, Ganapatrao Kulkarni wrote:
>
>> >> >> +     "CPU" :"armv8_pmuv3_0"
>> >> >
>> >> > Please let's not hard-code the name like this. Surely we can get rid of this?
>> >> >
>> >> > The kernel doesn't currently name PMUs as armv8_pmuv3_*, and as that can
>> >> > differ across DT/ACPI and in big.LITTLE, I don't think it makes sense to
>> >> > try to rely one particular string regardless.
>> >>
>> >> This string/name is fixed for a platform. having name here is essential to
>> >> know which devices among pmu (armv8_pmuv3_0, breakpoint, software)
>> >> devices, these jevents to be added.
>> >> also this json file is specific to a arch/soc/board, it is not a
>> >> generic file to be common.
>> >
>> > This file describe the events of a CPU PMU, and CPUs are not specific to
>> > a platform in general. There are many systems using Cortex-A57, for
>> > example.
>> >
>> > Across big.LITTLE SoCs with Cortex-A57, there's no guarantee as to
>> > whether the Cortex-A57 cores would be named armv8_pmuv3_0, or
>> > armv8_pmuv3_1, etc. This would depend on the boot CPU, probe order of
>> > secondaries, etc.

some of the applications(perf etc) use sysfs files of perf PMU CORE devices.
at present the names are created as per SOC/platform like
armv8_pmuv3, armv8_cavium_thunder,  armv8_cortex_a57 etc.

cpu_pmu->name                   = "armv8_cavium_thunder";

can we please have common name similar to x86(cpu) and call them as
cpu_0 and cpu_1?

>>
>> OK, we may not have complete name however, common part can be used to recognize
>> the PMU CORE devices from /sys/bus/event_source/devices
>> i.e we can have CPU id as "armv8_pmuv3".
>
> For better or worse, that's not the case on DT systems.
>
> I'd much rather that we identified the CPU PMUs without requiring
> particular names (e.g by looking for a "cpus" attribute).
>
>> same is extended to UNCORE as well.
>
> Could you elaborate on that? I'm not sure I follow.
>
>> mapfile.csv file will have entry for both BIG and LITTLE processors event files.
>> the jevents creates table of pmu_events_map for all entries present in
>> mapfile.csv file
>> while lookup, which ever pmu matches the cpuid of pmu_events_map
>> then corresponding table created from json file is used to add the
>> jevents to that PMU.
>
> Sorry, but I don't follow how that's related to the above.
>
> Thanks,
> Mark.

thanks
Ganapat

Re: [PATCH 3/3] perf tool, arm64, thunderx2: Add implementation defined events for ThunderX2

[Mark Rutland]

On Wed, Apr 19, 2017 at 11:37:31PM +0530, Ganapatrao Kulkarni wrote:
> Hi Mark,

Hi,

> On Thu, Apr 6, 2017 at 3:25 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Thu, Apr 06, 2017 at 09:50:33AM +0530, Ganapatrao Kulkarni wrote:
> >> On Wed, Apr 5, 2017 at 3:35 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> >> > On Wed, Apr 05, 2017 at 02:42:39PM +0530, Ganapatrao Kulkarni wrote:
> >> >> On Tue, Apr 4, 2017 at 5:58 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> >> >> > On Tue, Apr 04, 2017 at 01:06:43PM +0530, Ganapatrao Kulkarni wrote:
> >
> >> >> >> +     "CPU" :"armv8_pmuv3_0"
> >> >> >
> >> >> > Please let's not hard-code the name like this. Surely we can get rid of this?
> >> >> >
> >> >> > The kernel doesn't currently name PMUs as armv8_pmuv3_*, and as that can
> >> >> > differ across DT/ACPI and in big.LITTLE, I don't think it makes sense to
> >> >> > try to rely one particular string regardless.
> >> >>
> >> >> This string/name is fixed for a platform. having name here is essential to
> >> >> know which devices among pmu (armv8_pmuv3_0, breakpoint, software)
> >> >> devices, these jevents to be added.
> >> >> also this json file is specific to a arch/soc/board, it is not a
> >> >> generic file to be common.
> >> >
> >> > This file describe the events of a CPU PMU, and CPUs are not specific to
> >> > a platform in general. There are many systems using Cortex-A57, for
> >> > example.
> >> >
> >> > Across big.LITTLE SoCs with Cortex-A57, there's no guarantee as to
> >> > whether the Cortex-A57 cores would be named armv8_pmuv3_0, or
> >> > armv8_pmuv3_1, etc. This would depend on the boot CPU, probe order of
> >> > secondaries, etc.
> 
> some of the applications(perf etc) use sysfs files of perf PMU CORE devices.
> at present the names are created as per SOC/platform like
> armv8_pmuv3, armv8_cavium_thunder,  armv8_cortex_a57 etc.
> 
> cpu_pmu->name                   = "armv8_cavium_thunder";
> 
> can we please have common name similar to x86(cpu) and call them as
> cpu_0 and cpu_1?

I don't see how that helps in this case?

I'd rather that we expose some mechanism to determine whether a PMU is a
CPU PMU, other than the name. Userspace can then throw away the name if
it so wishes, and it doesn't have the potential to break existing users.

Thanks,
Mark.

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

[Mark Rutland]

Hi,

On Tue, Apr 04, 2017 at 01:06:40PM +0530, Ganapatrao Kulkarni wrote:
> Extending json/jevent framework for parsing arm64 event files.
> Adding jevents for ThunderX2 implementation defined PMU events.

Thanks for having a look at this!

While I agree this is the right approach generally, I have a couple of
issues with the implementation, which I have commented upon in
subsequent patches.

Thanks,
Mark.

> Ganapatrao Kulkarni (3):
>   perf jevents: Add support to use core pmu name other than cpu
>   perf tools arm64: implement function get_cpuid_str
>   perf tool, arm64, thunderx2: Add implementation defined events for
>     thunderx2
> 
>  tools/perf/arch/arm64/util/Build                   |  1 +
>  tools/perf/arch/arm64/util/header.c                | 51 +++++++++++++++
>  tools/perf/pmu-events/arch/arm64/mapfile.csv       |  2 +
>  .../arm64/thunderx2/implementation-defined.json    | 72 ++++++++++++++++++++++
>  tools/perf/pmu-events/jevents.c                    | 15 +++--
>  tools/perf/pmu-events/jevents.h                    |  3 +-
>  tools/perf/pmu-events/pmu-events.h                 |  1 +
>  tools/perf/util/pmu.c                              |  2 +-
>  8 files changed, 140 insertions(+), 7 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
> 
> -- 
> 1.8.1.4
>