[PATCH RFC 0/7] perf pmu-events: Support event aliasing for system PMUs

[PATCH RFC 0/7] perf pmu-events: Support event aliasing for system PMUs

[John Garry]

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

This series adds support for PMU event aliasing for system and other
uncore PMUs which are not fixed to a specific CPU.

For this, we introduce support for another per-arch mapfile, which maps a
particular system identifier to a set of system PMU events for that
system. This is much the same as what we do for CPU event aliasing.

To support this, we need to change how we match a PMU to a mapfile,
whether it should use a CPU or system mapfile. For this we do the
following:

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

- For an uncore or system PMU, we match first based on the SYSID (if set).
  If this fails, then we match on the CPUID.

  This works for x86, as x86 would not have any system mapfiles for uncore
  PMUs (and match on the CPUID).

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

Here is a sample output with this series:

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

  smmuv3_pmcg_100020/config_cache_miss/              [Kernel PMU event]
  smmuv3_pmcg_100020/config_struct_access/           [Kernel PMU event]
  smmuv3_pmcg_100020/cycles/                         [Kernel PMU event]
  smmuv3_pmcg_100020/pcie_ats_trans_passed/          [Kernel PMU event]
  smmuv3_pmcg_100020/pcie_ats_trans_rq/              [Kernel PMU event]
  smmuv3_pmcg_100020/tlb_miss/                       [Kernel PMU event]
  smmuv3_pmcg_100020/trans_table_walk_access/        [Kernel PMU event]
  smmuv3_pmcg_100020/transaction/                    [Kernel PMU event]

  [...]

smmu v3 pmcg:
  smmuv3_pmcg.l1_tlb                                
       [SMMUv3 PMCG l1_tlb. Unit: smmuv3_pmcg]

  [...]

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

 Performance counter stats for 'system wide':

               235      smmuv3_pmcg.l1_tlb                                          

       1.001263128 seconds time elapsed

root@ubuntu:/# 

Issues with this series which need to be addressed (aware to me):

- It would be good to have a universal method to identify the system from
  sysfs. Nothing exists which I know about. There is DMI, but this is not
  always available (or has correct info). Maybe systems which want to
  support this feature will need a "soc" driver, and a
  /sys/devices/socX/machine file (which I used for testing this series -
  this driver is out of tree currently).

- Maybe it is ok, but for systems which match on the system identifier,
  uncore PMUs should be in the system mapfile, and, as such, match on the
  system identifier and not CPU identifier.

- We need a better way in jevents.c to give a direct mapping of PMU name
  aliases, i.e. for any PMU name not prefixed with "uncore_", we need to
  add this to table unit_to_pmu[]. Not scalable.

  Having said that, maybe the kernel can introduce some future PMU naming
  convention in future.

John Garry (7):
  perf jevents: Add support for an extra directory level
  perf vendor events arm64: Relocate hip08 core events
  perf jevents: Add support for a system events PMU
  perf pmu: Rename uncore symbols to include system pmus
  perf pmu: Support matching by sysid
  perf vendor events arm64: Relocate uncore events for hip08
  perf vendor events arm64: Add hip08 SMMUv3 PMCG IMP DEF events

 tools/perf/arch/arm64/util/arm-spe.c          |   2 +-
 tools/perf/pmu-events/README                  |  47 ++++++--
 .../hip08/{ => cpu}/core-imp-def.json         |   0
 .../hisilicon/hip08/sys/smmu-v3-pmcg.json     |   9 ++
 .../hip08/{ => sys}/uncore-ddrc.json          |   0
 .../hisilicon/hip08/{ => sys}/uncore-hha.json |   0
 .../hisilicon/hip08/{ => sys}/uncore-l3c.json |   0
 tools/perf/pmu-events/arch/arm64/mapfile.csv  |   2 +-
 .../pmu-events/arch/arm64/mapfile_sys.csv     |  14 +++
 tools/perf/pmu-events/jevents.c               |  65 ++++++++--
 tools/perf/pmu-events/pmu-events.h            |   1 +
 tools/perf/util/evsel.h                       |   2 +-
 tools/perf/util/parse-events.c                |  12 +-
 tools/perf/util/pmu.c                         | 111 +++++++++++++++---
 tools/perf/util/pmu.h                         |   2 +-
 15 files changed, 221 insertions(+), 46 deletions(-)
 rename tools/perf/pmu-events/arch/arm64/hisilicon/hip08/{ => cpu}/core-imp-def.json (100%)
 create mode 100644 tools/perf/pmu-events/arch/arm64/hisilicon/hip08/sys/smmu-v3-pmcg.json
 rename tools/perf/pmu-events/arch/arm64/hisilicon/hip08/{ => sys}/uncore-ddrc.json (100%)
 rename tools/perf/pmu-events/arch/arm64/hisilicon/hip08/{ => sys}/uncore-hha.json (100%)
 rename tools/perf/pmu-events/arch/arm64/hisilicon/hip08/{ => sys}/uncore-l3c.json (100%)
 create mode 100644 tools/perf/pmu-events/arch/arm64/mapfile_sys.csv

-- 
2.17.1

[PATCH RFC 1/7] perf jevents: Add support for an extra directory level

[John Garry]

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

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

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

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

Re: [PATCH RFC 1/7] perf jevents: Add support for an extra directory level

[Jiri Olsa]

On Fri, Jan 24, 2020 at 10:34:59PM +0800, John Garry wrote:
> Currently we support upto a level 2 directory, and level 2 would be in the
> form vendor/platform.
> 
> Add support for a further level, to hold specific categories of events for
> when we want to segregate them for matching purposes.
> 
> Signed-off-by: John Garry <john.garry@huawei.com>
> ---
>  tools/perf/pmu-events/jevents.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
> index 079c77b6a2fd..8af05b94a37d 100644
> --- a/tools/perf/pmu-events/jevents.c
> +++ b/tools/perf/pmu-events/jevents.c
> @@ -960,15 +960,20 @@ static int process_one_file(const char *fpath, const struct stat *sb,
>  	int level   = ftwbuf->level;
>  	int err = 0;
>  
> -	if (level == 2 && is_dir) {
> +	if (level >= 2 && is_dir) {
> +		int count = 0;
>  		/*
>  		 * For level 2 directory, bname will include parent name,
>  		 * like vendor/platform. So search back from platform dir
>  		 * to find this.
> +		 * Something similar for level 3 directory, but we're a PMU
> +		 * category folder, like vendor/platform/cpu.
>  		 */
>  		bname = (char *) fpath + ftwbuf->base - 2;
>  		for (;;) {
>  			if (*bname == '/')
> +				count++;
> +			if (count == level - 1)
>  				break;
>  			bname--;

I was wondering why we just don't use different filename for that,
but it's true that the code transforms directory chain to the table
name.. so I guess another directory level is justified ;-)

jirka


>  		}
> @@ -981,13 +986,13 @@ static int process_one_file(const char *fpath, const struct stat *sb,
>  		 level, sb->st_size, bname, fpath);
>  
>  	/* base dir or too deep */
> -	if (level == 0 || level > 3)
> +	if (level == 0 || level > 4)
>  		return 0;
>  
>  
>  	/* model directory, reset topic */
>  	if ((level == 1 && is_dir && is_leaf_dir(fpath)) ||
> -	    (level == 2 && is_dir)) {
> +	    (level >= 2 && is_dir && is_leaf_dir(fpath))) {
>  		if (close_table)
>  			print_events_table_suffix(eventsfp);
>  
> -- 
> 2.17.1
> 

Re: [PATCH RFC 1/7] perf jevents: Add support for an extra directory level

[John Garry]

On 10/02/2020 12:07, Jiri Olsa wrote:
> On Fri, Jan 24, 2020 at 10:34:59PM +0800, John Garry wrote:
>> Currently we support upto a level 2 directory, and level 2 would be in the
>> form vendor/platform.
>>
>> Add support for a further level, to hold specific categories of events for
>> when we want to segregate them for matching purposes.
>>
>> Signed-off-by: John Garry <john.garry@huawei.com>
>> ---
>>   tools/perf/pmu-events/jevents.c | 11 ++++++++---
>>   1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
>> index 079c77b6a2fd..8af05b94a37d 100644
>> --- a/tools/perf/pmu-events/jevents.c
>> +++ b/tools/perf/pmu-events/jevents.c
>> @@ -960,15 +960,20 @@ static int process_one_file(const char *fpath, const struct stat *sb,
>>   	int level   = ftwbuf->level;
>>   	int err = 0;
>>   
>> -	if (level == 2 && is_dir) {
>> +	if (level >= 2 && is_dir) {
>> +		int count = 0;
>>   		/*
>>   		 * For level 2 directory, bname will include parent name,
>>   		 * like vendor/platform. So search back from platform dir
>>   		 * to find this.
>> +		 * Something similar for level 3 directory, but we're a PMU
>> +		 * category folder, like vendor/platform/cpu.
>>   		 */
>>   		bname = (char *) fpath + ftwbuf->base - 2;
>>   		for (;;) {
>>   			if (*bname == '/')
>> +				count++;
>> +			if (count == level - 1)
>>   				break;
>>   			bname--;
> 

Hi Jirka,

> I was wondering why we just don't use different filename for that,
> but it's true that the code transforms directory chain to the table
> name.. so I guess another directory level is justified ;-)

Yes, and we need to have separate tables for system and CPU/uncore PMU 
aliases.

Thanks,
John

> 
> jirka
> 
> 
>>   		}
>> @@ -981,13 +986,13 @@ static int process_one_file(const char *fpath, const struct stat *sb,
>>   		 level, sb->st_size, bname, fpath);
>>   
>>   	/* base dir or too deep */
>> -	if (level == 0 || level > 3)
>> +	if (level == 0 || level > 4)
>>   		return 0;
>>   
>>   
>>   	/* model directory, reset topic */
>>   	if ((level == 1 && is_dir && is_leaf_dir(fpath)) ||
>> -	    (level == 2 && is_dir)) {
>> +	    (level >= 2 && is_dir && is_leaf_dir(fpath))) {
>>   		if (close_table)
>>   			print_events_table_suffix(eventsfp);
>>   
>> -- 
>> 2.17.1
>>
> 
> .
> 

[PATCH RFC 2/7] perf vendor events arm64: Relocate hip08 core events

[John Garry]

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

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

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

[PATCH RFC 3/7] perf jevents: Add support for a system events PMU

[John Garry]

Currently CPU or uncore PMUs are supported for PMU event aliasing.

For fully integrated SoCs, we may have many other system PMUs and it is
useful to be able to alias events for those PMUs also. This could also
include the grey area of uncore PMUs, which need to be matched by CPU ID,
so need to be tightly coupled with the CPU. This is how it works for x86.
However, this may not work well for architectures where the CPU may not
have fixed uncore PMUs - ARM arch, for example.

Add support for a new mapfile - mapfile_sys.csv - which maps to system
event tables. For these, some SYS ID is used to match. For CPU PMUs,
mapfile.csv is still used to match any CPU event alises.

Generated pmu-events.c format will be like:

struct pmu_event pme_hisilicon_hip08_cpu[] = {
{
	.name = "l1d_cache_rd",
	.event = "event=0x40",
	.desc = "L1D cache access, read",
	.topic = "core imp def",
	.long_desc = "Attributable Level 1 data cache access, read",
},
{
	.name = "l1d_cache_wr",
	.event = "event=0x41",
	.desc = "L1D cache access, write",
	.topic = "core imp def",
	.long_desc = "Attributable Level 1 data cache access, write",
},
{
	.name = 0,
	.event = 0,
	.desc = 0,
},
};

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

struct pmu_events_map pmu_events_map[] = {
{
	.cpuid = "0x00000000480fd010",
	.version = "v1",
	.type = "core",
	.table = pme_hisilicon_hip08_cpu
},
{
	.sysid = "HIP08",
	.version = "v1",
	.type = "sys",
	.table = pme_hisilicon_hip08_sys
},
{
	.cpuid = 0,
	.sysid = 0,
	.version = 0,
	.type = 0,
	.table = 0,
},
};

Signed-off-by: John Garry <john.garry@huawei.com>
---
 tools/perf/pmu-events/README                  | 47 +++++++++++++----
 .../pmu-events/arch/arm64/mapfile_sys.csv     | 13 +++++
 tools/perf/pmu-events/jevents.c               | 52 ++++++++++++++++---
 tools/perf/pmu-events/pmu-events.h            |  1 +
 4 files changed, 94 insertions(+), 19 deletions(-)
 create mode 100644 tools/perf/pmu-events/arch/arm64/mapfile_sys.csv

diff --git a/tools/perf/pmu-events/README b/tools/perf/pmu-events/README
index de7efa2cebd1..e940cef73d2b 100644
--- a/tools/perf/pmu-events/README
+++ b/tools/perf/pmu-events/README
@@ -1,6 +1,7 @@
 
 The contents of this directory allow users to specify PMU events in their
-CPUs by their symbolic names rather than raw event codes (see example below).
+CPUs or other PMUs in the system by their symbolic names rather than raw
+event codes (see example below).
 
 The main program in this directory, is the 'jevents', which is built and
 executed _BEFORE_ the perf binary itself is built.
@@ -12,7 +13,12 @@ tree tools/perf/pmu-events/arch/foo.
 	  JSON files, each of which describes a set of PMU events.
 
 	- The CSV file that maps a specific CPU to its set of PMU events is to
-	  be named 'mapfile.csv' (see below for mapfile format).
+	  be named 'mapfile.csv'.
+
+	  An additional optional CSV file maps specific PMU to its set of PMU
+	  events is to be named 'mapfile_sys.csv'.
+
+	  See below for mapfile formats.
 
 	- Directories are traversed, but all other files are ignored.
 
@@ -22,10 +28,10 @@ tree tools/perf/pmu-events/arch/foo.
 	  Architecture standard JSONs must be located in the architecture root
 	  folder. Matching is based on the "EventName" field.
 
-The PMU events supported by a CPU model are expected to grouped into topics
-such as Pipelining, Cache, Memory, Floating-point etc. All events for a topic
-should be placed in a separate JSON file - where the file name identifies
-the topic. Eg: "Floating-point.json".
+The PMU events supported by a CPU model or PMU are expected to grouped into
+topics such as Pipelining, Cache, Memory, Floating-point etc. All events for
+a topic should be placed in a separate JSON file - where the file name
+identifies the topic. Eg: "Floating-point.json".
 
 All the topic JSON files for a CPU model/family should be in a separate
 sub directory. Thus for the Silvermont X86 CPU:
@@ -38,7 +44,11 @@ The JSONs folder for a CPU model/family may be placed in the root arch
 folder, or may be placed in a vendor sub-folder under the arch folder
 for instances where the arch and vendor are not the same.
 
-Using the JSON files and the mapfile, 'jevents' generates the C source file,
+The JSONS folder for a system PMU should be placed in a subfolder for
+the platform, separate to the CPU events folder. The reason is that different
+matching mechanism could be used for detecting CPU and system PMUs.
+
+Using the JSON files and the mapfile(s), 'jevents' generates the C source file,
 'pmu-events.c', which encodes the two sets of tables:
 
 	- Set of 'PMU events tables' for all known CPUs in the architecture,
@@ -83,11 +93,11 @@ NOTES:
 	2. The 'pmu-events.h' has an extern declaration for the mapping table
 	   and the generated 'pmu-events.c' defines this table.
 
-	3. _All_ known CPU tables for architecture are included in the perf
-	   binary.
+	3. _All_ known CPU and system tables for architecture are included in
+	   the perf binary.
 
-At run time, perf determines the actual CPU it is running on, finds the
-matching events table and builds aliases for those events. This allows
+At run time, perf determines the actual CPU or system it is running on, finds
+the matching events table and builds aliases for those events. This allows
 users to specify events by their name:
 
 	$ perf stat -e pm_1plus_ppc_cmpl sleep 1
@@ -150,3 +160,18 @@ where:
 
 	i.e the three CPU models use the JSON files (i.e PMU events) listed
 	in the directory 'tools/perf/pmu-events/arch/x86/silvermont'.
+
+The mapfile_sys.csv format is slightly different, in that it contains a SYSID
+instead of the CPUID:
+
+	Header line
+	SYSID,Version,Dir/path/name,Type
+
+where, same as mapfile.csv, except:
+
+	SYSID:
+		SYSID is a platform specific char string, that can be used
+		to identify thr system.
+
+	Type:
+		Should always be sys
diff --git a/tools/perf/pmu-events/arch/arm64/mapfile_sys.csv b/tools/perf/pmu-events/arch/arm64/mapfile_sys.csv
new file mode 100644
index 000000000000..701d8ff67354
--- /dev/null
+++ b/tools/perf/pmu-events/arch/arm64/mapfile_sys.csv
@@ -0,0 +1,13 @@
+# Format:
+#	SYS ID,Version,JSON/file/pathname,Type
+#
+# where
+#	SYS ID	Unique identifier for the system
+#		Could be DT machine ID, ACPI OEM ID, etc
+#	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 sys
+#
+#Family-model,Version,Filename,EventType
diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
index 8af05b94a37d..da6430c0d184 100644
--- a/tools/perf/pmu-events/jevents.c
+++ b/tools/perf/pmu-events/jevents.c
@@ -755,6 +755,7 @@ static void print_mapping_table_suffix(FILE *outfp)
 	 */
 	fprintf(outfp, "{\n");
 	fprintf(outfp, "\t.cpuid = 0,\n");
+	fprintf(outfp, "\t.sysid = 0,\n");
 	fprintf(outfp, "\t.version = 0,\n");
 	fprintf(outfp, "\t.type = 0,\n");
 	fprintf(outfp, "\t.table = 0,\n");
@@ -771,7 +772,7 @@ static int process_mapfile(FILE *outfp, char *fpath)
 	char *save = NULL;
 	char *line, *p;
 	int line_num;
-	char *tblname;
+	char *tblname, *table_id;
 	int ret = 0;
 
 	pr_info("%s: Processing mapfile %s\n", prog, fpath);
@@ -788,8 +789,6 @@ static int process_mapfile(FILE *outfp, char *fpath)
 		return -1;
 	}
 
-	print_mapping_table_prefix(outfp);
-
 	/* Skip first line (header) */
 	p = fgets(line, n, mapfp);
 	if (!p)
@@ -797,7 +796,7 @@ static int process_mapfile(FILE *outfp, char *fpath)
 
 	line_num = 1;
 	while (1) {
-		char *cpuid, *version, *type, *fname;
+		char *id, *version, *type, *fname;
 
 		line_num++;
 		p = fgets(line, n, mapfp);
@@ -816,14 +815,21 @@ static int process_mapfile(FILE *outfp, char *fpath)
 		}
 		line[strlen(line)-1] = '\0';
 
-		cpuid = fixregex(strtok_r(p, ",", &save));
+		id = fixregex(strtok_r(p, ",", &save));
 		version = strtok_r(NULL, ",", &save);
 		fname = strtok_r(NULL, ",", &save);
 		type = strtok_r(NULL, ",", &save);
 
+		/* We treat uncore as "cpu" events */
+		if (!strcmp(type, "core") || !strcmp(type, "uncore"))
+			table_id = "cpuid";
+		else if (!strcmp(type, "sys"))
+			table_id = "sysid";
+		else
+			table_id = "unknown";
 		tblname = file_name_to_table_name(fname);
 		fprintf(outfp, "{\n");
-		fprintf(outfp, "\t.cpuid = \"%s\",\n", cpuid);
+		fprintf(outfp, "\t.%s = \"%s\",\n", table_id, id);
 		fprintf(outfp, "\t.version = \"%s\",\n", version);
 		fprintf(outfp, "\t.type = \"%s\",\n", type);
 
@@ -841,12 +847,36 @@ static int process_mapfile(FILE *outfp, char *fpath)
 	}
 
 out:
-	print_mapping_table_suffix(outfp);
 	fclose(mapfp);
 	free(line);
 	return ret;
 }
 
+static int process_mapfiles(FILE *outfp, char *fpath, char *fpath_sys)
+{
+	char *save = NULL;
+	int line_num;
+	char *tblname;
+	char *table_id;
+	int ret;
+
+	pr_info("%s: Processing mapfiles %s fpath_sys=%s\n", prog, fpath, fpath_sys);
+
+	print_mapping_table_prefix(outfp);
+
+	ret = process_mapfile(outfp, fpath);
+	if (ret)
+		goto out;
+
+	if (fpath_sys)
+		ret = process_mapfile(outfp, fpath_sys);
+
+out:
+	print_mapping_table_suffix(outfp);
+
+	return ret;
+}
+
 /*
  * If we fail to locate/process JSON and map files, create a NULL mapping
  * table. This would at least allow perf to build even if we can't find/use
@@ -887,6 +917,7 @@ static int get_maxfds(void)
  */
 static FILE *eventsfp;
 static char *mapfile;
+static char *mapfile_sys;
 
 static int is_leaf_dir(const char *fpath)
 {
@@ -1024,6 +1055,11 @@ static int process_one_file(const char *fpath, const struct stat *sb,
 			return 0;
 		}
 
+		if (!strcmp(bname, "mapfile_sys.csv")) {
+			mapfile_sys = strdup(fpath);
+			return 0;
+		}
+
 		pr_info("%s: Ignoring file %s\n", prog, fpath);
 		return 0;
 	}
@@ -1174,7 +1210,7 @@ int main(int argc, char *argv[])
 		goto empty_map;
 	}
 
-	if (process_mapfile(eventsfp, mapfile)) {
+	if (process_mapfiles(eventsfp, mapfile, mapfile_sys)) {
 		pr_info("%s: Error processing mapfile %s\n", prog, mapfile);
 		/* Make build fail */
 		fclose(eventsfp);
diff --git a/tools/perf/pmu-events/pmu-events.h b/tools/perf/pmu-events/pmu-events.h
index caeb577d36c9..9964bdd2f6e1 100644
--- a/tools/perf/pmu-events/pmu-events.h
+++ b/tools/perf/pmu-events/pmu-events.h
@@ -31,6 +31,7 @@ struct pmu_event {
  */
 struct pmu_events_map {
 	const char *cpuid;
+	const char *sysid;
 	const char *version;
 	const char *type;		/* core, uncore etc */
 	struct pmu_event *table;
-- 
2.17.1

Re: [PATCH RFC 3/7] perf jevents: Add support for a system events PMU

[Jiri Olsa]

On Fri, Jan 24, 2020 at 10:35:01PM +0800, John Garry wrote:

SNIP

> +	return ret;
> +}
> +
>  /*
>   * If we fail to locate/process JSON and map files, create a NULL mapping
>   * table. This would at least allow perf to build even if we can't find/use
> @@ -887,6 +917,7 @@ static int get_maxfds(void)
>   */
>  static FILE *eventsfp;
>  static char *mapfile;
> +static char *mapfile_sys;
>  
>  static int is_leaf_dir(const char *fpath)
>  {
> @@ -1024,6 +1055,11 @@ static int process_one_file(const char *fpath, const struct stat *sb,
>  			return 0;
>  		}
>  
> +		if (!strcmp(bname, "mapfile_sys.csv")) {
> +			mapfile_sys = strdup(fpath);


we could release that in the cleanup code at the end of main
together with 'mapfile', which is also missing

jirka

Re: [PATCH RFC 3/7] perf jevents: Add support for a system events PMU

[Jiri Olsa]

On Fri, Jan 24, 2020 at 10:35:01PM +0800, John Garry wrote:

SNIP

>  	- Set of 'PMU events tables' for all known CPUs in the architecture,
> @@ -83,11 +93,11 @@ NOTES:
>  	2. The 'pmu-events.h' has an extern declaration for the mapping table
>  	   and the generated 'pmu-events.c' defines this table.
>  
> -	3. _All_ known CPU tables for architecture are included in the perf
> -	   binary.
> +	3. _All_ known CPU and system tables for architecture are included in
> +	   the perf binary.
>  
> -At run time, perf determines the actual CPU it is running on, finds the
> -matching events table and builds aliases for those events. This allows
> +At run time, perf determines the actual CPU or system it is running on, finds
> +the matching events table and builds aliases for those events. This allows
>  users to specify events by their name:
>  
>  	$ perf stat -e pm_1plus_ppc_cmpl sleep 1
> @@ -150,3 +160,18 @@ where:
>  
>  	i.e the three CPU models use the JSON files (i.e PMU events) listed
>  	in the directory 'tools/perf/pmu-events/arch/x86/silvermont'.
> +
> +The mapfile_sys.csv format is slightly different, in that it contains a SYSID
> +instead of the CPUID:
> +
> +	Header line
> +	SYSID,Version,Dir/path/name,Type

can't we just add prefix to SYSID types? like:

	SYSID-HIP08,v1,hisilicon/hip08/sys,sys
	0x00000000480fd010,v1,hisilicon/hip08/cpu,core
	0x00000000500f0000,v1,ampere/emag,core

because the rest of the line is the same, right?

seems to me that having one mapfile type would be less confusing

jirka

Re: [PATCH RFC 3/7] perf jevents: Add support for a system events PMU

[John Garry]

On 10/02/2020 12:07, Jiri Olsa wrote:
> On Fri, Jan 24, 2020 at 10:35:01PM +0800, John Garry wrote:
> 
> SNIP
> 
>>   	- Set of 'PMU events tables' for all known CPUs in the architecture,
>> @@ -83,11 +93,11 @@ NOTES:
>>   	2. The 'pmu-events.h' has an extern declaration for the mapping table
>>   	   and the generated 'pmu-events.c' defines this table.
>>   
>> -	3. _All_ known CPU tables for architecture are included in the perf
>> -	   binary.
>> +	3. _All_ known CPU and system tables for architecture are included in
>> +	   the perf binary.
>>   
>> -At run time, perf determines the actual CPU it is running on, finds the
>> -matching events table and builds aliases for those events. This allows
>> +At run time, perf determines the actual CPU or system it is running on, finds
>> +the matching events table and builds aliases for those events. This allows
>>   users to specify events by their name:
>>   
>>   	$ perf stat -e pm_1plus_ppc_cmpl sleep 1
>> @@ -150,3 +160,18 @@ where:
>>   
>>   	i.e the three CPU models use the JSON files (i.e PMU events) listed
>>   	in the directory 'tools/perf/pmu-events/arch/x86/silvermont'.
>> +
>> +The mapfile_sys.csv format is slightly different, in that it contains a SYSID
>> +instead of the CPUID:
>> +
>> +	Header line
>> +	SYSID,Version,Dir/path/name,Type
> 

Hi jirka,

> can't we just add prefix to SYSID types? like:
> 
> 	SYSID-HIP08,v1,hisilicon/hip08/sys,sys
> 	0x00000000480fd010,v1,hisilicon/hip08/cpu,core
> 	0x00000000500f0000,v1,ampere/emag,core
> 
> because the rest of the line is the same, right?

I did consider that already. It should be workable.

> 
> seems to me that having one mapfile type would be less confusing

I thought that having it all in a single file would be more confusing :)

> 
As for this separate comment:

 >> +		if (!strcmp(bname, "mapfile_sys.csv")) {
 >> +			mapfile_sys = strdup(fpath);
 >
 >
 > we could release that in the cleanup code at the end of main
 > together with 'mapfile',

That should now go away.

 >  which is also missing

Right, I'll look to fix that.

Thanks,
John

> .
> 

Re: [PATCH RFC 3/7] perf jevents: Add support for a system events PMU

[Jiri Olsa]

On Mon, Feb 10, 2020 at 03:55:00PM +0000, John Garry wrote:
> On 10/02/2020 12:07, Jiri Olsa wrote:
> > On Fri, Jan 24, 2020 at 10:35:01PM +0800, John Garry wrote:
> > 
> > SNIP
> > 
> > >   	- Set of 'PMU events tables' for all known CPUs in the architecture,
> > > @@ -83,11 +93,11 @@ NOTES:
> > >   	2. The 'pmu-events.h' has an extern declaration for the mapping table
> > >   	   and the generated 'pmu-events.c' defines this table.
> > > -	3. _All_ known CPU tables for architecture are included in the perf
> > > -	   binary.
> > > +	3. _All_ known CPU and system tables for architecture are included in
> > > +	   the perf binary.
> > > -At run time, perf determines the actual CPU it is running on, finds the
> > > -matching events table and builds aliases for those events. This allows
> > > +At run time, perf determines the actual CPU or system it is running on, finds
> > > +the matching events table and builds aliases for those events. This allows
> > >   users to specify events by their name:
> > >   	$ perf stat -e pm_1plus_ppc_cmpl sleep 1
> > > @@ -150,3 +160,18 @@ where:
> > >   	i.e the three CPU models use the JSON files (i.e PMU events) listed
> > >   	in the directory 'tools/perf/pmu-events/arch/x86/silvermont'.
> > > +
> > > +The mapfile_sys.csv format is slightly different, in that it contains a SYSID
> > > +instead of the CPUID:
> > > +
> > > +	Header line
> > > +	SYSID,Version,Dir/path/name,Type
> > 
> 
> Hi jirka,
> 
> > can't we just add prefix to SYSID types? like:
> > 
> > 	SYSID-HIP08,v1,hisilicon/hip08/sys,sys
> > 	0x00000000480fd010,v1,hisilicon/hip08/cpu,core
> > 	0x00000000500f0000,v1,ampere/emag,core
> > 
> > because the rest of the line is the same, right?
> 
> I did consider that already. It should be workable.
> 
> > 
> > seems to me that having one mapfile type would be less confusing
> 
> I thought that having it all in a single file would be more confusing :)

hum, I think that if we keep it separated like:

	SYSID-HIP08,v1,hisilicon/hip08/sys,sys
	SYSID-krava,v1,hisilicon/krava/sys,sys
	0x00000000480fd010,v1,hisilicon/hip08/cpu,core
	0x00000000500f0000,v1,ampere/emag,core

then we should be fine.. not too many humans read that file anyway ;-)

jirka

[PATCH RFC 4/7] perf pmu: Rename uncore symbols to include system PMUs

[John Garry]

We want to expand the perf PMU support to cover system PMUs, which are
essentially the same as uncore pmus (from a kernel sysfs perspective
anyway).

So rename pmu_is_uncore() et al to cover this.

Unfortunately we have no real way to detect if a PMU is uncore or system.
We could check the PMU name for "uncore_" prefix to detect if really uncore,
but this does not work for all uncore PMUs - maybe we should introduce
this kernel naming convention for future support.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 tools/perf/arch/arm64/util/arm-spe.c |  2 +-
 tools/perf/util/evsel.h              |  2 +-
 tools/perf/util/parse-events.c       | 12 ++++++------
 tools/perf/util/pmu.c                |  6 +++---
 tools/perf/util/pmu.h                |  2 +-
 5 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c
index eba6541ec0f1..4241ad6c9fa0 100644
--- a/tools/perf/arch/arm64/util/arm-spe.c
+++ b/tools/perf/arch/arm64/util/arm-spe.c
@@ -223,7 +223,7 @@ struct perf_event_attr
 	}
 
 	arm_spe_pmu->selectable = true;
-	arm_spe_pmu->is_uncore = false;
+	arm_spe_pmu->is_uncore_or_sys = false;
 
 	return attr;
 }
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index dc14f4a823cd..d583b2a64d93 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -75,7 +75,7 @@ struct evsel {
 	bool			precise_max;
 	bool			ignore_missing_thread;
 	bool			forced_leader;
-	bool			use_uncore_alias;
+	bool			use_uncore_or_system_alias;
 	/* parse modifier helper */
 	int			exclude_GH;
 	int			sample_read;
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index ed7c008b9c8b..89105d5f0f0b 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -367,7 +367,7 @@ __add_event(struct list_head *list, int *idx,
 	(*idx)++;
 	evsel->core.cpus   = perf_cpu_map__get(cpus);
 	evsel->core.own_cpus = perf_cpu_map__get(cpus);
-	evsel->core.system_wide = pmu ? pmu->is_uncore : false;
+	evsel->core.system_wide = pmu ? pmu->is_uncore_or_sys : false;
 	evsel->auto_merge_stats = auto_merge_stats;
 
 	if (name)
@@ -1404,7 +1404,7 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
 	struct perf_pmu *pmu;
 	struct evsel *evsel;
 	struct parse_events_error *err = parse_state->error;
-	bool use_uncore_alias;
+	bool use_uncore_or_system_alias;
 	LIST_HEAD(config_terms);
 
 	pmu = perf_pmu__find(name);
@@ -1425,7 +1425,7 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
 		memset(&attr, 0, sizeof(attr));
 	}
 
-	use_uncore_alias = (pmu->is_uncore && use_alias);
+	use_uncore_or_system_alias = (pmu->is_uncore_or_sys && use_alias);
 
 	if (!head_config) {
 		attr.type = pmu->type;
@@ -1433,7 +1433,7 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
 				    auto_merge_stats, NULL);
 		if (evsel) {
 			evsel->pmu_name = name;
-			evsel->use_uncore_alias = use_uncore_alias;
+			evsel->use_uncore_or_system_alias = use_uncore_or_system_alias;
 			return 0;
 		} else {
 			return -ENOMEM;
@@ -1481,7 +1481,7 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
 		evsel->metric_expr = info.metric_expr;
 		evsel->metric_name = info.metric_name;
 		evsel->pmu_name = name;
-		evsel->use_uncore_alias = use_uncore_alias;
+		evsel->use_uncore_or_system_alias = use_uncore_or_system_alias;
 		evsel->percore = config_term_percore(&evsel->config_terms);
 	}
 
@@ -1598,7 +1598,7 @@ parse_events__set_leader_for_uncore_aliase(char *name, struct list_head *list,
 	__evlist__for_each_entry(list, evsel) {
 
 		/* Only split the uncore group which members use alias */
-		if (!evsel->use_uncore_alias)
+		if (!evsel->use_uncore_or_system_alias)
 			goto out;
 
 		/* The events must be from the same uncore block */
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 8b99fd312aae..569aba4cec89 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -623,7 +623,7 @@ static struct perf_cpu_map *pmu_cpumask(const char *name)
 	return NULL;
 }
 
-static bool pmu_is_uncore(const char *name)
+static bool pmu_is_uncore_or_sys(const char *name)
 {
 	char path[PATH_MAX];
 	const char *sysfs;
@@ -769,7 +769,7 @@ static void pmu_add_cpu_aliases(struct list_head *head, struct perf_pmu *pmu)
 			break;
 		}
 
-		if (pmu_is_uncore(name) &&
+		if (pmu_is_uncore_or_sys(name) &&
 		    pmu_uncore_alias_match(pname, name))
 			goto new_alias;
 
@@ -838,7 +838,7 @@ static struct perf_pmu *pmu_lookup(const char *name)
 	pmu->cpus = pmu_cpumask(name);
 	pmu->name = strdup(name);
 	pmu->type = type;
-	pmu->is_uncore = pmu_is_uncore(name);
+	pmu->is_uncore_or_sys = pmu_is_uncore_or_sys(name);
 	pmu->max_precise = pmu_max_precise(name);
 	pmu_add_cpu_aliases(&aliases, pmu);
 
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 6737e3d5d568..67cf002c9458 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -25,7 +25,7 @@ struct perf_pmu {
 	char *name;
 	__u32 type;
 	bool selectable;
-	bool is_uncore;
+	bool is_uncore_or_sys;
 	bool auxtrace;
 	int max_precise;
 	struct perf_event_attr *default_config;
-- 
2.17.1

Re: [PATCH RFC 4/7] perf pmu: Rename uncore symbols to include system PMUs

[Jiri Olsa]

On Fri, Jan 24, 2020 at 10:35:02PM +0800, John Garry wrote:

SNIP

>  		/* Only split the uncore group which members use alias */
> -		if (!evsel->use_uncore_alias)
> +		if (!evsel->use_uncore_or_system_alias)
>  			goto out;
>  
>  		/* The events must be from the same uncore block */
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 8b99fd312aae..569aba4cec89 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -623,7 +623,7 @@ static struct perf_cpu_map *pmu_cpumask(const char *name)
>  	return NULL;
>  }
>  
> -static bool pmu_is_uncore(const char *name)
> +static bool pmu_is_uncore_or_sys(const char *name)

so we detect uncore PMU by checking for cpumask file

I don't see the connection here with the sysid or '_sys' checking,
that's just telling which ID to use when looking for an alias, no?
shouldn't that be separated?

jirka

Re: [PATCH RFC 4/7] perf pmu: Rename uncore symbols to include system PMUs

[John Garry]

On 10/02/2020 12:07, Jiri Olsa wrote:
> On Fri, Jan 24, 2020 at 10:35:02PM +0800, John Garry wrote:
> 
> SNIP
> 
>>   		/* Only split the uncore group which members use alias */
>> -		if (!evsel->use_uncore_alias)
>> +		if (!evsel->use_uncore_or_system_alias)
>>   			goto out;
>>   
>>   		/* The events must be from the same uncore block */
>> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
>> index 8b99fd312aae..569aba4cec89 100644
>> --- a/tools/perf/util/pmu.c
>> +++ b/tools/perf/util/pmu.c
>> @@ -623,7 +623,7 @@ static struct perf_cpu_map *pmu_cpumask(const char *name)
>>   	return NULL;
>>   }
>>   
>> -static bool pmu_is_uncore(const char *name)
>> +static bool pmu_is_uncore_or_sys(const char *name)
> 

Hi jirka,

> so we detect uncore PMU by checking for cpumask file
> 

For PMUs which could be considered "system" PMUs, they also have a 
cpumask, like the PMU I use as motivation for this series:

root@(none)$ pwd
/sys/bus/event_source/devices/smmuv3_pmcg_100020
root@(none)$ ls -l
total 0
-r--r--r--    1 root     root          4096 Feb 10 14:50 cpumask
drwxr-xr-x    2 root     root             0 Feb 10 14:50 events
drwxr-xr-x    2 root     root             0 Feb 10 14:50 format
-rw-r--r--    1 root     root          4096 Feb 10 14:50 
perf_event_mux_interval_ms
drwxr-xr-x    2 root     root             0 Feb 10 14:50 power
lrwxrwxrwx    1 root     root             0 Feb 10 14:50 subsystem -> 
../../bus/event_source
-r--r--r--    1 root     root          4096 Feb 10 14:50 type
-rw-r--r--    1 root     root          4096 Feb 10 14:50 uevent


Other PMU drivers which I have checked in drivers/perf also have the same.

Indeed I see no way to differentiate whether a PMU is an uncore or 
system. So that is why I change the name to cover both. Maybe there is a 
better name than the verbose pmu_is_uncore_or_sys().

> I don't see the connection here with the sysid or '_sys' checking,
> that's just telling which ID to use when looking for an alias, no?

So the connection is that in perf_pmu__find_map(), for a given PMU, the 
matching is now extended from only core or uncore PMUs to also these 
system PMUs. And I use the sysid to find an aliasing table for any 
system PMUs present.

> shouldn't that be separated?

Yes, I now think that this would be a better option. So currently I am 
combining it and it causes a problem, like I have noted in patch #5:

struct pmu_events_map *perf_pmu__find_map(struct perf_pmu *pmu)
{
[SNIP]
	sysid = perf_pmu__getsysid();

   /*
   * Match sysid as first perference for uncore/sys PMUs.
   *
   * x86 uncore events match by cpuid, but we would not have 	map->socid
* set for that arch (so any matching here would fail for that).
*/
if (pmu && pmu_is_uncore_or_sys(pmu->name) &&
    !is_arm_pmu_core(pmu->name) && sysid) {


Thanks,
John

Re: [PATCH RFC 4/7] perf pmu: Rename uncore symbols to include system PMUs

[Jiri Olsa]

On Mon, Feb 10, 2020 at 03:44:48PM +0000, John Garry wrote:
> On 10/02/2020 12:07, Jiri Olsa wrote:
> > On Fri, Jan 24, 2020 at 10:35:02PM +0800, John Garry wrote:
> > 
> > SNIP
> > 
> > >   		/* Only split the uncore group which members use alias */
> > > -		if (!evsel->use_uncore_alias)
> > > +		if (!evsel->use_uncore_or_system_alias)
> > >   			goto out;
> > >   		/* The events must be from the same uncore block */
> > > diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> > > index 8b99fd312aae..569aba4cec89 100644
> > > --- a/tools/perf/util/pmu.c
> > > +++ b/tools/perf/util/pmu.c
> > > @@ -623,7 +623,7 @@ static struct perf_cpu_map *pmu_cpumask(const char *name)
> > >   	return NULL;
> > >   }
> > > -static bool pmu_is_uncore(const char *name)
> > > +static bool pmu_is_uncore_or_sys(const char *name)
> > 
> 
> Hi jirka,
> 
> > so we detect uncore PMU by checking for cpumask file
> > 
> 
> For PMUs which could be considered "system" PMUs, they also have a cpumask,
> like the PMU I use as motivation for this series:
> 
> root@(none)$ pwd
> /sys/bus/event_source/devices/smmuv3_pmcg_100020
> root@(none)$ ls -l
> total 0
> -r--r--r--    1 root     root          4096 Feb 10 14:50 cpumask
> drwxr-xr-x    2 root     root             0 Feb 10 14:50 events
> drwxr-xr-x    2 root     root             0 Feb 10 14:50 format
> -rw-r--r--    1 root     root          4096 Feb 10 14:50
> perf_event_mux_interval_ms
> drwxr-xr-x    2 root     root             0 Feb 10 14:50 power
> lrwxrwxrwx    1 root     root             0 Feb 10 14:50 subsystem ->
> ../../bus/event_source
> -r--r--r--    1 root     root          4096 Feb 10 14:50 type
> -rw-r--r--    1 root     root          4096 Feb 10 14:50 uevent
> 
> 
> Other PMU drivers which I have checked in drivers/perf also have the same.
> 
> Indeed I see no way to differentiate whether a PMU is an uncore or system.
> So that is why I change the name to cover both. Maybe there is a better name
> than the verbose pmu_is_uncore_or_sys().
> 
> > I don't see the connection here with the sysid or '_sys' checking,
> > that's just telling which ID to use when looking for an alias, no?
> 
> So the connection is that in perf_pmu__find_map(), for a given PMU, the
> matching is now extended from only core or uncore PMUs to also these system
> PMUs. And I use the sysid to find an aliasing table for any system PMUs
> present.

I see.. can't we just check sysid for uncore PMUs? because
that's what the code is doing, right? having pmu_is_uncore_or_sys
makes me think there's some sysid-type PMU

jirka

Re: [PATCH RFC 4/7] perf pmu: Rename uncore symbols to include system PMUs

[John Garry]

On 11/02/2020 14:43, Jiri Olsa wrote:
>> root@(none)$ pwd
>> /sys/bus/event_source/devices/smmuv3_pmcg_100020
>> root@(none)$ ls -l
>> total 0
>> -r--r--r--    1 root     root          4096 Feb 10 14:50 cpumask
>> drwxr-xr-x    2 root     root             0 Feb 10 14:50 events
>> drwxr-xr-x    2 root     root             0 Feb 10 14:50 format
>> -rw-r--r--    1 root     root          4096 Feb 10 14:50
>> perf_event_mux_interval_ms
>> drwxr-xr-x    2 root     root             0 Feb 10 14:50 power
>> lrwxrwxrwx    1 root     root             0 Feb 10 14:50 subsystem ->
>> ../../bus/event_source
>> -r--r--r--    1 root     root          4096 Feb 10 14:50 type
>> -rw-r--r--    1 root     root          4096 Feb 10 14:50 uevent
>>
>>
>> Other PMU drivers which I have checked in drivers/perf also have the same.
>>
>> Indeed I see no way to differentiate whether a PMU is an uncore or system.
>> So that is why I change the name to cover both. Maybe there is a better name
>> than the verbose pmu_is_uncore_or_sys().
>>
>>> I don't see the connection here with the sysid or '_sys' checking,
>>> that's just telling which ID to use when looking for an alias, no?
>> So the connection is that in perf_pmu__find_map(), for a given PMU, the
>> matching is now extended from only core or uncore PMUs to also these system
>> PMUs. And I use the sysid to find an aliasing table for any system PMUs
>> present.

Hi Jirka,

 > I see.. can't we just check sysid for uncore PMUs?

x86 will still alias PMUs (uncore or CPU) based on an alias table 
matched to the cpuid, as it is today. x86 has the benefit of fixed 
uncore PMUs for a given cpuid.

For other archs whose uncore or system PMUs are not fixed for a given 
CPU - like arm - we will support matching uncore and system PMUs on 
cpuid or sysid.

Uncore PMUs are a grey area for arm, as they may or may not be tied to a 
specific cpuid, so we will need to support both matching methods.

because
 > that's what the code is doing, right?

Not exactly.

The code will match on an alias table matched to the cpuid and also an 
alias table matched to the sysid (if perf could actually get a sysid and 
there is a table matching that sysid).

I hope that this makes sense....

having pmu_is_uncore_or_sys
 > makes me think there's some sysid-type PMU
 >
 > jirka
 >

Thanks,
John

Re: [PATCH RFC 4/7] perf pmu: Rename uncore symbols to include system PMUs

[Jiri Olsa]

On Tue, Feb 11, 2020 at 03:36:39PM +0000, John Garry wrote:
> On 11/02/2020 14:43, Jiri Olsa wrote:
> > > root@(none)$ pwd
> > > /sys/bus/event_source/devices/smmuv3_pmcg_100020
> > > root@(none)$ ls -l
> > > total 0
> > > -r--r--r--    1 root     root          4096 Feb 10 14:50 cpumask
> > > drwxr-xr-x    2 root     root             0 Feb 10 14:50 events
> > > drwxr-xr-x    2 root     root             0 Feb 10 14:50 format
> > > -rw-r--r--    1 root     root          4096 Feb 10 14:50
> > > perf_event_mux_interval_ms
> > > drwxr-xr-x    2 root     root             0 Feb 10 14:50 power
> > > lrwxrwxrwx    1 root     root             0 Feb 10 14:50 subsystem ->
> > > ../../bus/event_source
> > > -r--r--r--    1 root     root          4096 Feb 10 14:50 type
> > > -rw-r--r--    1 root     root          4096 Feb 10 14:50 uevent
> > > 
> > > 
> > > Other PMU drivers which I have checked in drivers/perf also have the same.
> > > 
> > > Indeed I see no way to differentiate whether a PMU is an uncore or system.
> > > So that is why I change the name to cover both. Maybe there is a better name
> > > than the verbose pmu_is_uncore_or_sys().
> > > 
> > > > I don't see the connection here with the sysid or '_sys' checking,
> > > > that's just telling which ID to use when looking for an alias, no?
> > > So the connection is that in perf_pmu__find_map(), for a given PMU, the
> > > matching is now extended from only core or uncore PMUs to also these system
> > > PMUs. And I use the sysid to find an aliasing table for any system PMUs
> > > present.
> 
> Hi Jirka,
> 
> > I see.. can't we just check sysid for uncore PMUs?
> 
> x86 will still alias PMUs (uncore or CPU) based on an alias table matched to
> the cpuid, as it is today. x86 has the benefit of fixed uncore PMUs for a
> given cpuid.

ok, I did mean 'on addition' to the cpuid checks

> 
> For other archs whose uncore or system PMUs are not fixed for a given CPU -
> like arm - we will support matching uncore and system PMUs on cpuid or
> sysid.
> 
> Uncore PMUs are a grey area for arm, as they may or may not be tied to a
> specific cpuid, so we will need to support both matching methods.
> 
> because
> > that's what the code is doing, right?
> 
> Not exactly.
> 
> The code will match on an alias table matched to the cpuid and also an alias
> table matched to the sysid (if perf could actually get a sysid and there is
> a table matching that sysid).
> 
> I hope that this makes sense....

right,  please make sure this kind of explanation is in changelog
or better in the code comment 

thanks,
jirka

[PATCH RFC 5/7] perf pmu: Support matching by sysid

[John Garry]

Match system or uncore PMU aliases by system id, SYSID.

We use a SYSID read from sysfs or from an env variable to match against
uncore or system PMU events.

For x86, they want to match uncore events with cpuid - this still works
fine for x86 as it would not have system event tables for uncore PMUs.

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

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 569aba4cec89..4d4fe0c1ae22 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -672,11 +672,78 @@ static char *perf_pmu__getcpuid(struct perf_pmu *pmu)
 	return cpuid;
 }
 
+static char *get_sysid_str(void)
+{
+	char *buf = NULL;
+	char path[PATH_MAX];
+	const char *sysfs = sysfs__mountpoint();
+	FILE *file;
+	int s, i;
+
+	if (!sysfs)
+		return NULL;
+
+	buf = malloc(PATH_MAX);
+	if (!buf) {
+		pr_err("%s alloc failed\n", __func__);
+		return NULL;
+	}
+
+	scnprintf(path, PATH_MAX, "%s/devices/soc0/machine", sysfs);
+
+	file = fopen(path, "r");
+	if (!file) {
+		pr_debug("fopen failed for file %s\n", path);
+		free(buf);
+		return NULL;
+	}
+
+	if (!fgets(buf, PATH_MAX, file)) {
+		fclose(file);
+		pr_debug("gets failed for file %s\n", path);
+		free(buf);
+		return NULL;
+	}
+	fclose(file);
+
+	/* Remove any whitespace, this could be from ACPI HID */
+	s = strlen(buf);
+	for (i = 0; i < s; i++) {
+		if (buf[i] == ' ') {
+			buf[i] = 0;
+			break;
+		};
+	}
+
+	return buf;
+}
+
+static char *perf_pmu__getsysid(void)
+{
+	char *sysid;
+	static bool printed;
+
+	sysid = getenv("PERF_SYSID");
+	if (sysid)
+		sysid = strdup(sysid);
+
+	if (!sysid)
+		sysid = get_sysid_str();
+	if (!sysid)
+		return NULL;
+
+	if (!printed) {
+		pr_debug("Using SYSID %s\n", sysid);
+		printed = true;
+	}
+	return sysid;
+}
+
 struct pmu_events_map *perf_pmu__find_map(struct perf_pmu *pmu)
 {
-	struct pmu_events_map *map;
+	struct pmu_events_map *map, *found_map = NULL;
 	char *cpuid = perf_pmu__getcpuid(pmu);
-	int i;
+	char *sysid;
 
 	/* on some platforms which uses cpus map, cpuid can be NULL for
 	 * PMUs other than CORE PMUs.
@@ -684,19 +751,35 @@ struct pmu_events_map *perf_pmu__find_map(struct perf_pmu *pmu)
 	if (!cpuid)
 		return NULL;
 
-	i = 0;
-	for (;;) {
-		map = &pmu_events_map[i++];
-		if (!map->table) {
-			map = NULL;
-			break;
+	sysid = perf_pmu__getsysid();
+
+	/*
+	 * Match sysid as first perference for uncore/sys PMUs.
+	 *
+	 * x86 uncore events match by cpuid, but we would not have map->socid
+	 * set for that arch (so any matching here would fail for that).
+	 */
+	if (pmu && pmu_is_uncore_or_sys(pmu->name) &&
+	    !is_arm_pmu_core(pmu->name) && sysid) {
+		for (map = &pmu_events_map[0]; map->table; map++) {
+			if (map->sysid && !strcmp(map->sysid, sysid)) {
+				found_map = map;
+				goto out;
+			}
 		}
+	}
 
-		if (!strcmp_cpuid_str(map->cpuid, cpuid))
-			break;
+	for (map = &pmu_events_map[0]; map->table; map++) {
+		if (map->cpuid && cpuid &&
+		    !strcmp_cpuid_str(map->cpuid, cpuid)) {
+			found_map = map;
+			goto out;
+		}
 	}
+out:
 	free(cpuid);
-	return map;
+	free(sysid); /* Can safely handle is sysid is NULL */
+	return found_map;
 }
 
 static bool pmu_uncore_alias_match(const char *pmu_name, const char *name)
-- 
2.17.1

Re: [PATCH RFC 5/7] perf pmu: Support matching by sysid

[Jiri Olsa]

On Fri, Jan 24, 2020 at 10:35:03PM +0800, John Garry wrote:

SNIP

> +		fclose(file);
> +		pr_debug("gets failed for file %s\n", path);
> +		free(buf);
> +		return NULL;
> +	}
> +	fclose(file);
> +
> +	/* Remove any whitespace, this could be from ACPI HID */
> +	s = strlen(buf);
> +	for (i = 0; i < s; i++) {
> +		if (buf[i] == ' ') {
> +			buf[i] = 0;
> +			break;
> +		};
> +	}
> +
> +	return buf;
> +}
> +
> +static char *perf_pmu__getsysid(void)
> +{
> +	char *sysid;
> +	static bool printed;
> +
> +	sysid = getenv("PERF_SYSID");
> +	if (sysid)
> +		sysid = strdup(sysid);
> +
> +	if (!sysid)
> +		sysid = get_sysid_str();
> +	if (!sysid)
> +		return NULL;
> +
> +	if (!printed) {
> +		pr_debug("Using SYSID %s\n", sysid);
> +		printed = true;
> +	}
> +	return sysid;
> +}

this part is getting complicated and AFAIK we have no tests for it

if you could think of any tests that'd be great.. perhaps we could
load 'our' json test files and check appropriate events/aliasses
via in pmu object.. or via parse_events interface.. those test aliases
would have to be part of perf, but we have tests compiled in anyway

thanks,
jirka

Re: [PATCH RFC 5/7] perf pmu: Support matching by sysid

[John Garry]

Hi jirka,

> 
>> +		fclose(file);
>> +		pr_debug("gets failed for file %s\n", path);
>> +		free(buf);
>> +		return NULL;
>> +	}
>> +	fclose(file);
>> +
>> +	/* Remove any whitespace, this could be from ACPI HID */
>> +	s = strlen(buf);
>> +	for (i = 0; i < s; i++) {
>> +		if (buf[i] == ' ') {
>> +			buf[i] = 0;
>> +			break;
>> +		};
>> +	}
>> +
>> +	return buf;
>> +}
>> +

I have another series to add kernel support for a system identifier 
sysfs entry, which I sent after this series:

https://lore.kernel.org/linux-acpi/1580210059-199540-1-git-send-email-john.garry@huawei.com/

It is different to what I am relying on here - it uses a kernel soc 
driver for firmware ACPI PPTT identifier. Progress is somewhat blocked 
at the moment however and I may have to use a different method:

https://lore.kernel.org/linux-acpi/20200128123415.GB36168@bogus/

>> +static char *perf_pmu__getsysid(void)
>> +{
>> +	char *sysid;
>> +	static bool printed;
>> +
>> +	sysid = getenv("PERF_SYSID");
>> +	if (sysid)
>> +		sysid = strdup(sysid);
>> +
>> +	if (!sysid)
>> +		sysid = get_sysid_str();
>> +	if (!sysid)
>> +		return NULL;
>> +
>> +	if (!printed) {
>> +		pr_debug("Using SYSID %s\n", sysid);
>> +		printed = true;
>> +	}
>> +	return sysid;
>> +}
> 
> this part is getting complicated and AFAIK we have no tests for it
> 
> if you could think of any tests that'd be great.. Perhaps we could
> load 'our' json test files and check appropriate events/aliasses
> via in pmu object.. or via parse_events interface.. those test aliases
> would have to be part of perf, but we have tests compiled in anyway

Sorry, I don't fully follow.

Are you suggesting that we could load the specific JSONs tables for a 
system from the host filesystem?

Thanks,
John

Re: [PATCH RFC 5/7] perf pmu: Support matching by sysid

[Jiri Olsa]

On Mon, Feb 10, 2020 at 04:22:56PM +0000, John Garry wrote:
> Hi jirka,
> 
> > 
> > > +		fclose(file);
> > > +		pr_debug("gets failed for file %s\n", path);
> > > +		free(buf);
> > > +		return NULL;
> > > +	}
> > > +	fclose(file);
> > > +
> > > +	/* Remove any whitespace, this could be from ACPI HID */
> > > +	s = strlen(buf);
> > > +	for (i = 0; i < s; i++) {
> > > +		if (buf[i] == ' ') {
> > > +			buf[i] = 0;
> > > +			break;
> > > +		};
> > > +	}
> > > +
> > > +	return buf;
> > > +}
> > > +
> 
> I have another series to add kernel support for a system identifier sysfs
> entry, which I sent after this series:
> 
> https://lore.kernel.org/linux-acpi/1580210059-199540-1-git-send-email-john.garry@huawei.com/
> 
> It is different to what I am relying on here - it uses a kernel soc driver
> for firmware ACPI PPTT identifier. Progress is somewhat blocked at the
> moment however and I may have to use a different method:
> 
> https://lore.kernel.org/linux-acpi/20200128123415.GB36168@bogus/

I'll try to check ;-)

> 
> > > +static char *perf_pmu__getsysid(void)
> > > +{
> > > +	char *sysid;
> > > +	static bool printed;
> > > +
> > > +	sysid = getenv("PERF_SYSID");
> > > +	if (sysid)
> > > +		sysid = strdup(sysid);
> > > +
> > > +	if (!sysid)
> > > +		sysid = get_sysid_str();
> > > +	if (!sysid)
> > > +		return NULL;
> > > +
> > > +	if (!printed) {
> > > +		pr_debug("Using SYSID %s\n", sysid);
> > > +		printed = true;
> > > +	}
> > > +	return sysid;
> > > +}
> > 
> > this part is getting complicated and AFAIK we have no tests for it
> > 
> > if you could think of any tests that'd be great.. Perhaps we could
> > load 'our' json test files and check appropriate events/aliasses
> > via in pmu object.. or via parse_events interface.. those test aliases
> > would have to be part of perf, but we have tests compiled in anyway
> 
> Sorry, I don't fully follow.
> 
> Are you suggesting that we could load the specific JSONs tables for a system
> from the host filesystem?

I wish to see some test for all this.. I can only think about having
'test' json files compiled with perf and 'perf test' that looks them
up and checks that all is in the proper place

jirka

Re: [PATCH RFC 5/7] perf pmu: Support matching by sysid

[John Garry]

On 11/02/2020 13:47, Jiri Olsa wrote:

Hi Jirka,

>>>> +
>>>> +	return buf;
>>>> +}
>>>> +
>>
>> I have another series to add kernel support for a system identifier sysfs
>> entry, which I sent after this series:
>>
>> https://lore.kernel.org/linux-acpi/1580210059-199540-1-git-send-email-john.garry@huawei.com/
>>
>> It is different to what I am relying on here - it uses a kernel soc driver
>> for firmware ACPI PPTT identifier. Progress is somewhat blocked at the
>> moment however and I may have to use a different method:
>>
>> https://lore.kernel.org/linux-acpi/20200128123415.GB36168@bogus/
> 
> I'll try to check ;-)

Summary is that there exists an ACPI firmware field which we could 
expose to userspace via sysfs - this would provide the system id. 
However there is a proposal to deprecate it in the ACPI standard and, as 
such, would prefer that we don't add kernel support for it at this stage.

So I am evaluating the alternative in the meantime, which again is some 
firmware method which should allow us to expose a system id to userspace 
via sysfs. Unfortunately this is arm specific. However, other archs can 
still provide their own method, maybe a soc driver:

Documentation/ABI/testing/sysfs-devices-soc#n15

> 
>>
>>>> +static char *perf_pmu__getsysid(void)
>>>> +{
>>>> +	char *sysid;
>>>> +	static bool printed;
>>>> +
>>>> +	sysid = getenv("PERF_SYSID");
>>>> +	if (sysid)
>>>> +		sysid = strdup(sysid);
>>>> +
>>>> +	if (!sysid)
>>>> +		sysid = get_sysid_str();
>>>> +	if (!sysid)
>>>> +		return NULL;
>>>> +
>>>> +	if (!printed) {
>>>> +		pr_debug("Using SYSID %s\n", sysid);
>>>> +		printed = true;
>>>> +	}
>>>> +	return sysid;
>>>> +}
>>>
>>> this part is getting complicated and AFAIK we have no tests for it
>>>
>>> if you could think of any tests that'd be great.. Perhaps we could
>>> load 'our' json test files and check appropriate events/aliasses
>>> via in pmu object.. or via parse_events interface.. those test aliases
>>> would have to be part of perf, but we have tests compiled in anyway
>>
>> Sorry, I don't fully follow.
>>
>> Are you suggesting that we could load the specific JSONs tables for a system
>> from the host filesystem?
> 
> I wish to see some test for all this.. I can only think about having
> 'test' json files compiled with perf and 'perf test' that looks them
> up and checks that all is in the proper place

OK, let me consider this part for perf test support.

Thanks,
John

Re: [PATCH RFC 5/7] perf pmu: Support matching by sysid

[John Garry]

On 11/02/2020 15:07, John Garry wrote:
> On 11/02/2020 13:47, Jiri Olsa wrote:
> 
> Hi Jirka,
> 
>>>>> +
>>>>> +    return buf;
>>>>> +}
>>>>> +
>>>
>>> I have another series to add kernel support for a system identifier 
>>> sysfs
>>> entry, which I sent after this series:
>>>
>>> https://lore.kernel.org/linux-acpi/1580210059-199540-1-git-send-email-john.garry@huawei.com/ 
>>>
>>>
>>> It is different to what I am relying on here - it uses a kernel soc 
>>> driver
>>> for firmware ACPI PPTT identifier. Progress is somewhat blocked at the
>>> moment however and I may have to use a different method:
>>>
>>> https://lore.kernel.org/linux-acpi/20200128123415.GB36168@bogus/
>>
>> I'll try to check ;-)
> 
> Summary is that there exists an ACPI firmware field which we could 
> expose to userspace via sysfs - this would provide the system id. 
> However there is a proposal to deprecate it in the ACPI standard and, as 
> such, would prefer that we don't add kernel support for it at this stage.
> 
> So I am evaluating the alternative in the meantime, which again is some 
> firmware method which should allow us to expose a system id to userspace 
> via sysfs. Unfortunately this is arm specific. However, other archs can 
> still provide their own method, maybe a soc driver:
> 
> Documentation/ABI/testing/sysfs-devices-soc#n15
> 
>>
>>>
>>>>> +static char *perf_pmu__getsysid(void)
>>>>> +{
>>>>> +    char *sysid;
>>>>> +    static bool printed;
>>>>> +
>>>>> +    sysid = getenv("PERF_SYSID");
>>>>> +    if (sysid)
>>>>> +        sysid = strdup(sysid);
>>>>> +
>>>>> +    if (!sysid)
>>>>> +        sysid = get_sysid_str();
>>>>> +    if (!sysid)
>>>>> +        return NULL;
>>>>> +
>>>>> +    if (!printed) {
>>>>> +        pr_debug("Using SYSID %s\n", sysid);
>>>>> +        printed = true;
>>>>> +    }
>>>>> +    return sysid;
>>>>> +}
>>>>
>>>> this part is getting complicated and AFAIK we have no tests for it
>>>>
>>>> if you could think of any tests that'd be great.. Perhaps we could
>>>> load 'our' json test files and check appropriate events/aliasses
>>>> via in pmu object.. or via parse_events interface.. those test aliases
>>>> would have to be part of perf, but we have tests compiled in anyway
>>>
>>> Sorry, I don't fully follow.
>>>
>>> Are you suggesting that we could load the specific JSONs tables for a 
>>> system
>>> from the host filesystem?
>>
>> I wish to see some test for all this.. I can only think about having
>> 'test' json files compiled with perf and 'perf test' that looks them
>> up and checks that all is in the proper place
> 
> OK, let me consider this part for perf test support.

I will note that perf test has many issues on my arm64 board:

do] password for john:
  1: vmlinux symtab matches kallsyms                       : Skip
  2: Detect openat syscall event                           : FAILED!
  3: Detect openat syscall event on all cpus               : FAILED!
  4: Read samples using the mmap interface                 : FAILED!
  5: Test data source output                               : Ok
  6: Parse event definition strings                        : FAILED!
  7: Simple expression parser                              : Ok
  8: PERF_RECORD_* events & perf_sample fields             : Ok
  9: Parse perf pmu format                                 : Ok
10: DSO data read                                         : Ok
11: DSO data cache                                        : Ok
12: DSO data reopen                                       : Ok
13: Roundtrip evsel->name                                 : Ok
14: Parse sched tracepoints fields                        : FAILED!
15: syscalls:sys_enter_openat event fields                : FAILED!
16: Setup struct perf_event_attr                          : Skip
17: Match and link multiple hists                         : Ok
18: 'import perf' in python                               : Ok
21: Breakpoint accounting                                 : Ok
22: Watchpoint                                            :
22.1: Read Only Watchpoint                                : Ok
22.2: Write Only Watchpoint                               : Ok
22.3: Read / Write Watchpoint                             : Ok
22.4: Modify Watchpoint                                   : Ok
23: Number of exit events of a simple workload            : Ok
24: Software clock events period values                   : Ok
25: Object code reading                                   : Ok
26: Sample parsing                                        : Ok
27: Use a dummy software event to keep tracking           : Ok
28: Parse with no sample_id_all bit set                   : Ok
29: Filter hist entries                                   : Ok
30: Lookup mmap thread                                    : Ok
31: Share thread maps                                     : Ok
32: Sort output of hist entries                           : Ok
33: Cumulate child hist entries                           : Ok
34: Track with sched_switch                               : Ok
35: Filter fds with revents mask in a fdarray             : Ok
36: Add fd to a fdarray, making it autogrow               : Ok
37: kmod_path__parse                                      : Ok
38: Thread map                                            : Ok
39: LLVM search and compile                               :
39.1: Basic BPF llvm compile                              : Skip
39.2: kbuild searching                                    : Skip
39.3: Compile source for BPF prologue generation          : Skip
39.4: Compile source for BPF relocation                   : Skip
40: Session topology                                      : FAILED!
41: BPF filter                                            :
41.1: Basic BPF filtering                                 : Skip
41.2: BPF pinning                                         : Skip
41.3: BPF prologue generation                             : Skip
41.4: BPF relocation checker                              : Skip
42: Synthesize thread map                                 : Ok
43: Remove thread map                                     : Ok
44: Synthesize cpu map                                    : Ok
45: Synthesize stat config                                : Ok
46: Synthesize stat                                       : Ok
47: Synthesize stat round                                 : Ok
48: Synthesize attr update                                : Ok
49: Event times                                           : Ok
50: Read backward ring buffer                             : FAILED!
51: Print cpu map                                         : Ok
52: Merge cpu map                                         : Ok
53: Probe SDT events                                      : Ok
54: is_printable_array                                    : Ok
55: Print bitmap                                          : Ok
56: perf hooks                          umber__scnprintf 
                : Ok
59: mem2node                                              : Ok
60: time utils                                            : Ok
61: Test jit_write_elf                                    : Ok
62: maps__merge_in                                        : Ok
63: DWARF unwind                                          : Ok
64: Check open filename arg using perf trace + vfs_getname: FAILED!
65: Add vfs_getname probe to get syscall args filenames   : FAILED!
66: Use vfs_getname probe to get syscall args filenames   : FAILED!
67: Zstd perf.data compression/decompression              : Ok
68: probe libc's inet_pton & backtrace it with ping       : Skip
john@ubuntu:~/linux$

I know that the perf tool definitely has issues for system topology for 
arm64, which I need to check on.

Maybe I can conscribe help internally to help check the rest...

Thanks,
john

Re: [PATCH RFC 5/7] perf pmu: Support matching by sysid

[Jiri Olsa]

On Wed, Feb 12, 2020 at 10:08:44AM +0000, John Garry wrote:

SNIP

> > > 
> > > I wish to see some test for all this.. I can only think about having
> > > 'test' json files compiled with perf and 'perf test' that looks them
> > > up and checks that all is in the proper place
> > 
> > OK, let me consider this part for perf test support.
> 
> I will note that perf test has many issues on my arm64 board:
> 
> do] password for john:
>  1: vmlinux symtab matches kallsyms                       : Skip
>  2: Detect openat syscall event                           : FAILED!
>  3: Detect openat syscall event on all cpus               : FAILED!
>  4: Read samples using the mmap interface                 : FAILED!
>  5: Test data source output                               : Ok
>  6: Parse event definition strings                        : FAILED!
>  7: Simple expression parser                              : Ok
>  8: PERF_RECORD_* events & perf_sample fields             : Ok
>  9: Parse perf pmu format                                 : Ok
> 10: DSO data read                                         : Ok
> 11: DSO data cache                                        : Ok
> 12: DSO data reopen                                       : Ok
> 13: Roundtrip evsel->name                                 : Ok
> 14: Parse sched tracepoints fields                        : FAILED!
> 15: syscalls:sys_enter_openat event fields                : FAILED!

looks like some issue with tracepoints

> 16: Setup struct perf_event_attr                          : Skip
> 17: Match and link multiple hists                         : Ok
> 18: 'import perf' in python                               : Ok
> 21: Breakpoint accounting                                 : Ok
> 22: Watchpoint                                            :
> 22.1: Read Only Watchpoint                                : Ok
> 22.2: Write Only Watchpoint                               : Ok
> 22.3: Read / Write Watchpoint                             : Ok
> 22.4: Modify Watchpoint                                   : Ok
> 23: Number of exit events of a simple workload            : Ok
> 24: Software clock events period values                   : Ok
> 25: Object code reading                                   : Ok
> 26: Sample parsing                                        : Ok
> 27: Use a dummy software event to keep tracking           : Ok
> 28: Parse with no sample_id_all bit set                   : Ok
> 29: Filter hist entries                                   : Ok
> 30: Lookup mmap thread                                    : Ok
> 31: Share thread maps                                     : Ok
> 32: Sort output of hist entries                           : Ok
> 33: Cumulate child hist entries                           : Ok
> 34: Track with sched_switch                               : Ok
> 35: Filter fds with revents mask in a fdarray             : Ok
> 36: Add fd to a fdarray, making it autogrow               : Ok
> 37: kmod_path__parse                                      : Ok
> 38: Thread map                                            : Ok
> 39: LLVM search and compile                               :
> 39.1: Basic BPF llvm compile                              : Skip
> 39.2: kbuild searching                                    : Skip
> 39.3: Compile source for BPF prologue generation          : Skip
> 39.4: Compile source for BPF relocation                   : Skip

Skip is fine ;-)

> 40: Session topology                                      : FAILED!

I'd expect that one to fail if we don't have special
code to support arm in there

> 41: BPF filter                                            :
> 41.1: Basic BPF filtering                                 : Skip
> 41.2: BPF pinning                                         : Skip
> 41.3: BPF prologue generation                             : Skip
> 41.4: BPF relocation checker                              : Skip
> 42: Synthesize thread map                                 : Ok
> 43: Remove thread map                                     : Ok
> 44: Synthesize cpu map                                    : Ok
> 45: Synthesize stat config                                : Ok
> 46: Synthesize stat                                       : Ok
> 47: Synthesize stat round                                 : Ok
> 48: Synthesize attr update                                : Ok
> 49: Event times                                           : Ok
> 50: Read backward ring buffer                             : FAILED!

hum, I thought this was generic code that would work across archs

> 51: Print cpu map                                         : Ok
> 52: Merge cpu map                                         : Ok
> 53: Probe SDT events                                      : Ok
> 54: is_printable_array                                    : Ok
> 55: Print bitmap                                          : Ok
> 56: perf hooks                          umber__scnprintf                : Ok
> 59: mem2node                                              : Ok
> 60: time utils                                            : Ok
> 61: Test jit_write_elf                                    : Ok
> 62: maps__merge_in                                        : Ok
> 63: DWARF unwind                                          : Ok
> 64: Check open filename arg using perf trace + vfs_getname: FAILED!
> 65: Add vfs_getname probe to get syscall args filenames   : FAILED!
> 66: Use vfs_getname probe to get syscall args filenames   : FAILED!

with these we have always a problem across archs,
it's tricky to make script test that works everywhere :-\

> 67: Zstd perf.data compression/decompression              : Ok
> 68: probe libc's inet_pton & backtrace it with ping       : Skip
> john@ubuntu:~/linux$
> 
> I know that the perf tool definitely has issues for system topology for
> arm64, which I need to check on.
> 
> Maybe I can conscribe help internally to help check the rest...

the json/alias test would be also to make sure the x86 still works,
so regardless of some tests failing on arm, I think it's still better
to have that test

thanks,
jirka

Re: [PATCH RFC 5/7] perf pmu: Support matching by sysid

[John Garry]

On 12/02/2020 12:16, Jiri Olsa wrote:
>>> et me consider this part for perf test support.
>> I will note that perf test has many issues on my arm64 board:
>>
>> do] password for john:
>>   1: vmlinux symtab matches kallsyms                       : Skip
>>   2: Detect openat syscall event                           : FAILED!
>>   3: Detect openat syscall event on all cpus               : FAILED!
>>   4: Read samples using the mmap interface                 : FAILED!
>>   5: Test data source output                               : Ok
>>   6: Parse event definition strings                        : FAILED!
>>   7: Simple expression parser                              : Ok
>>   8: PERF_RECORD_* events & perf_sample fields             : Ok
>>   9: Parse perf pmu format                                 : Ok
>> 10: DSO data read                                         : Ok
>> 11: DSO data cache                                        : Ok
>> 12: DSO data reopen                                       : Ok
>> 13: Roundtrip evsel->name                                 : Ok
>> 14: Parse sched tracepoints fields                        : FAILED!
>> 15: syscalls:sys_enter_openat event fields                : FAILED!
> looks like some issue with tracepoints
> 
>> 16: Setup struct perf_event_attr                          : Skip
>> 17: Match and link multiple hists                         : Ok
>> 18: 'import perf' in python                               : Ok
>> 21: Breakpoint accounting                                 : Ok
>> 22: Watchpoint                                            :
>> 22.1: Read Only Watchpoint                                : Ok
>> 22.2: Write Only Watchpoint                               : Ok
>> 22.3: Read / Write Watchpoint                             : Ok
>> 22.4: Modify Watchpoint                                   : Ok
>> 23: Number of exit events of a simple workload            : Ok
>> 24: Software clock events period values                   : Ok
>> 25: Object code reading                                   : Ok
>> 26: Sample parsing                                        : Ok
>> 27: Use a dummy software event to keep tracking           : Ok
>> 28: Parse with no sample_id_all bit set                   : Ok
>> 29: Filter hist entries                                   : Ok
>> 30: Lookup mmap thread                                    : Ok
>> 31: Share thread maps                                     : Ok
>> 32: Sort output of hist entries                           : Ok
>> 33: Cumulate child hist entries                           : Ok
>> 34: Track with sched_switch                               : Ok
>> 35: Filter fds with revents mask in a fdarray             : Ok
>> 36: Add fd to a fdarray, making it autogrow               : Ok
>> 37: kmod_path__parse                                      : Ok
>> 38: Thread map                                            : Ok
>> 39: LLVM search and compile                               :
>> 39.1: Basic BPF llvm compile                              : Skip
>> 39.2: kbuild searching                                    : Skip
>> 39.3: Compile source for BPF prologue generation          : Skip
>> 39.4: Compile source for BPF relocation                   : Skip
> Skip is fine;-)
> 
>> 40: Session topology                                      : FAILED!
> I'd expect that one to fail if we don't have special
> code to support arm in there
> 
>> 41: BPF filter                                            :
>> 41.1: Basic BPF filtering                                 : Skip
>> 41.2: BPF pinning                                         : Skip
>> 41.3: BPF prologue generation                             : Skip
>> 41.4: BPF relocation checker                              : Skip
>> 42: Synthesize thread map                                 : Ok
>> 43: Remove thread map                                     : Ok
>> 44: Synthesize cpu map                                    : Ok
>> 45: Synthesize stat config                                : Ok
>> 46: Synthesize stat                                       : Ok
>> 47: Synthesize stat round                                 : Ok
>> 48: Synthesize attr update                                : Ok
>> 49: Event times                                           : Ok
>> 50: Read backward ring buffer                             : FAILED!
> hum, I thought this was generic code that would work across archs
> 
>> 51: Print cpu map                                         : Ok
>> 52: Merge cpu map                                         : Ok
>> 53: Probe SDT events                                      : Ok
>> 54: is_printable_array                                    : Ok
>> 55: Print bitmap                                          : Ok
>> 56: perf hooks                          umber__scnprintf                : Ok
>> 59: mem2node                                              : Ok
>> 60: time utils                                            : Ok
>> 61: Test jit_write_elf                                    : Ok
>> 62: maps__merge_in                                        : Ok
>> 63: DWARF unwind                                          : Ok
>> 64: Check open filename arg using perf trace + vfs_getname: FAILED!
>> 65: Add vfs_getname probe to get syscall args filenames   : FAILED!
>> 66: Use vfs_getname probe to get syscall args filenames   : FAILED!
> with these we have always a problem across archs,
> it's tricky to make script test that works everywhere:-\
> 
>> 67: Zstd perf.data compression/decompression              : Ok
>> 68: probe libc's inet_pton & backtrace it with ping       : Skip
>> john@ubuntu:~/linux$
>>
>> I know that the perf tool definitely has issues for system topology for
>> arm64, which I need to check on.
>>
>> Maybe I can conscribe help internally to help check the rest...

Hi jirka,

> the json/alias test would be also to make sure the x86 still works,
> so regardless of some tests failing on arm, I think it's still better
> to have that test

OK, I can look at this separately now, and it won't be blocked like this 
series is on the kernel sysid issue.

Thanks,
john

[PATCH RFC 6/7] perf vendor events arm64: Relocate uncore events for hip08

[John Garry]

We will need to match uncore events via SYSID when we want to add any other
system event PMU aliasing in future, so relocate the uncore JSONs now.

We use HIP08 as the system id.

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

diff --git a/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/uncore-ddrc.json b/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/sys/uncore-ddrc.json
similarity index 100%
rename from tools/perf/pmu-events/arch/arm64/hisilicon/hip08/uncore-ddrc.json
rename to tools/perf/pmu-events/arch/arm64/hisilicon/hip08/sys/uncore-ddrc.json
diff --git a/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/uncore-hha.json b/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/sys/uncore-hha.json
similarity index 100%
rename from tools/perf/pmu-events/arch/arm64/hisilicon/hip08/uncore-hha.json
rename to tools/perf/pmu-events/arch/arm64/hisilicon/hip08/sys/uncore-hha.json
diff --git a/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/uncore-l3c.json b/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/sys/uncore-l3c.json
similarity index 100%
rename from tools/perf/pmu-events/arch/arm64/hisilicon/hip08/uncore-l3c.json
rename to tools/perf/pmu-events/arch/arm64/hisilicon/hip08/sys/uncore-l3c.json
diff --git a/tools/perf/pmu-events/arch/arm64/mapfile_sys.csv b/tools/perf/pmu-events/arch/arm64/mapfile_sys.csv
index 701d8ff67354..d2baadcbbbed 100644
--- a/tools/perf/pmu-events/arch/arm64/mapfile_sys.csv
+++ b/tools/perf/pmu-events/arch/arm64/mapfile_sys.csv
@@ -11,3 +11,4 @@
 #	Type is sys
 #
 #Family-model,Version,Filename,EventType
+HIP08,v1,hisilicon/hip08/sys,sys
-- 
2.17.1

[PATCH RFC 7/7] perf vendor events arm64: Add hip08 SMMUv3 PMCG IMP DEF events

[John Garry]

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

Only a single event is added, but this is just an example for now.

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

diff --git a/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/sys/smmu-v3-pmcg.json b/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/sys/smmu-v3-pmcg.json
new file mode 100644
index 000000000000..ff2414a5ebc4
--- /dev/null
+++ b/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/sys/smmu-v3-pmcg.json
@@ -0,0 +1,9 @@
+[
+   {
+	    "EventCode": "0x8a",
+	    "EventName": "smmuv3_pmcg.l1_tlb",
+	    "BriefDescription": "SMMUv3 PMCG l1_tlb",
+	    "PublicDescription": "SMMUv3 PMCG l1_tlb",
+	    "Unit": "smmuv3_pmcg"
+   },
+]
diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
index da6430c0d184..01541825a6c7 100644
--- a/tools/perf/pmu-events/jevents.c
+++ b/tools/perf/pmu-events/jevents.c
@@ -239,6 +239,8 @@ static struct map {
 	{ "hisi_sccl,ddrc", "hisi_sccl,ddrc" },
 	{ "hisi_sccl,hha", "hisi_sccl,hha" },
 	{ "hisi_sccl,l3c", "hisi_sccl,l3c" },
+	/* it's not realistic to keep adding these, we need something more scalable ... */
+	{ "smmuv3_pmcg", "smmuv3_pmcg" },
 	{ "L3PMC", "amd_l3" },
 	{}
 };
-- 
2.17.1

Re: [PATCH RFC 0/7] perf pmu-events: Support event aliasing for system PMUs

[James Clark]

Hi John,

I tested this on an Arm N1 board and see the same list of CPU events from the JSONs
picked up so it looks ok from that point of view.

James

On 1/24/20 2:34 PM, John Garry wrote:
> Currently event aliasing for only CPU and uncore PMUs is supported. In
> fact, only uncore PMUs aliasing for when the uncore PMUs are fixed for a
> CPU is supported, which may not always be the case for certain
> architectures.
>
> This series adds support for PMU event aliasing for system and other
> uncore PMUs which are not fixed to a specific CPU.
>
> For this, we introduce support for another per-arch mapfile, which maps a
> particular system identifier to a set of system PMU events for that
> system. This is much the same as what we do for CPU event aliasing.
>
> To support this, we need to change how we match a PMU to a mapfile,
> whether it should use a CPU or system mapfile. For this we do the
> following:
>
> - For CPU PMU, we always match for the event mapfile based on the CPUID.
>   This has not changed.
>
> - For an uncore or system PMU, we match first based on the SYSID (if set).
>   If this fails, then we match on the CPUID.
>
>   This works for x86, as x86 would not have any system mapfiles for uncore
>   PMUs (and match on the CPUID).
>
> Initial reference support is also added for ARM SMMUv3 PMCG (Performance
> Monitor Event Group) PMU for HiSilicon hip08 platform with only a single
> event so far - see driver in drivers/perf/arm_smmuv3_pmu.c for that driver.
>
> Here is a sample output with this series:
>
> root@ubuntu:/# ./perf list
>   [...]
>
>   smmuv3_pmcg_100020/config_cache_miss/              [Kernel PMU event]
>   smmuv3_pmcg_100020/config_struct_access/           [Kernel PMU event]
>   smmuv3_pmcg_100020/cycles/                         [Kernel PMU event]
>   smmuv3_pmcg_100020/pcie_ats_trans_passed/          [Kernel PMU event]
>   smmuv3_pmcg_100020/pcie_ats_trans_rq/              [Kernel PMU event]
>   smmuv3_pmcg_100020/tlb_miss/                       [Kernel PMU event]
>   smmuv3_pmcg_100020/trans_table_walk_access/        [Kernel PMU event]
>   smmuv3_pmcg_100020/transaction/                    [Kernel PMU event]
>
>   [...]
>
> smmu v3 pmcg:
>   smmuv3_pmcg.l1_tlb
>        [SMMUv3 PMCG l1_tlb. Unit: smmuv3_pmcg]
>
>   [...]
>
> root@ubuntu:/# ./perf stat -v -e smmuv3_pmcg.l1_tlb sleep 1
> Using CPUID 0x00000000480fd010
> Using SYSID HIP08
>  -> smmuv3_pmcg_200100020/event=0x8a/
>  -> smmuv3_pmcg_200140020/event=0x8a/
>  -> smmuv3_pmcg_100020/event=0x8a/
>  -> smmuv3_pmcg_140020/event=0x8a/
>  -> smmuv3_pmcg_200148020/event=0x8a/
>  -> smmuv3_pmcg_148020/event=0x8a/
> smmuv3_pmcg.l1_tlb: 0 1001221690 1001221690
> smmuv3_pmcg.l1_tlb: 0 1001220090 1001220090
> smmuv3_pmcg.l1_tlb: 101 1001219660 1001219660
> smmuv3_pmcg.l1_tlb: 0 1001219010 1001219010
> smmuv3_pmcg.l1_tlb: 0 1001218360 1001218360
> smmuv3_pmcg.l1_tlb: 134 1001217850 1001217850
>
>  Performance counter stats for 'system wide':
>
>                235      smmuv3_pmcg.l1_tlb
>
>        1.001263128 seconds time elapsed
>
> root@ubuntu:/#
>
> Issues with this series which need to be addressed (aware to me):
>
> - It would be good to have a universal method to identify the system from
>   sysfs. Nothing exists which I know about. There is DMI, but this is not
>   always available (or has correct info). Maybe systems which want to
>   support this feature will need a "soc" driver, and a
>   /sys/devices/socX/machine file (which I used for testing this series -
>   this driver is out of tree currently).
>
> - Maybe it is ok, but for systems which match on the system identifier,
>   uncore PMUs should be in the system mapfile, and, as such, match on the
>   system identifier and not CPU identifier.
>
> - We need a better way in jevents.c to give a direct mapping of PMU name
>   aliases, i.e. for any PMU name not prefixed with "uncore_", we need to
>   add this to table unit_to_pmu[]. Not scalable.
>
>   Having said that, maybe the kernel can introduce some future PMU naming
>   convention in future.
>
> John Garry (7):
>   perf jevents: Add support for an extra directory level
>   perf vendor events arm64: Relocate hip08 core events
>   perf jevents: Add support for a system events PMU
>   perf pmu: Rename uncore symbols to include system pmus
>   perf pmu: Support matching by sysid
>   perf vendor events arm64: Relocate uncore events for hip08
>   perf vendor events arm64: Add hip08 SMMUv3 PMCG IMP DEF events
>
>  tools/perf/arch/arm64/util/arm-spe.c          |   2 +-
>  tools/perf/pmu-events/README                  |  47 ++++++--
>  .../hip08/{ => cpu}/core-imp-def.json         |   0
>  .../hisilicon/hip08/sys/smmu-v3-pmcg.json     |   9 ++
>  .../hip08/{ => sys}/uncore-ddrc.json          |   0
>  .../hisilicon/hip08/{ => sys}/uncore-hha.json |   0
>  .../hisilicon/hip08/{ => sys}/uncore-l3c.json |   0
>  tools/perf/pmu-events/arch/arm64/mapfile.csv  |   2 +-
>  .../pmu-events/arch/arm64/mapfile_sys.csv     |  14 +++
>  tools/perf/pmu-events/jevents.c               |  65 ++++++++--
>  tools/perf/pmu-events/pmu-events.h            |   1 +
>  tools/perf/util/evsel.h                       |   2 +-
>  tools/perf/util/parse-events.c                |  12 +-
>  tools/perf/util/pmu.c                         | 111 +++++++++++++++---
>  tools/perf/util/pmu.h                         |   2 +-
>  15 files changed, 221 insertions(+), 46 deletions(-)
>  rename tools/perf/pmu-events/arch/arm64/hisilicon/hip08/{ => cpu}/core-imp-def.json (100%)
>  create mode 100644 tools/perf/pmu-events/arch/arm64/hisilicon/hip08/sys/smmu-v3-pmcg.json
>  rename tools/perf/pmu-events/arch/arm64/hisilicon/hip08/{ => sys}/uncore-ddrc.json (100%)
>  rename tools/perf/pmu-events/arch/arm64/hisilicon/hip08/{ => sys}/uncore-hha.json (100%)
>  rename tools/perf/pmu-events/arch/arm64/hisilicon/hip08/{ => sys}/uncore-l3c.json (100%)
>  create mode 100644 tools/perf/pmu-events/arch/arm64/mapfile_sys.csv
>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Re: [PATCH RFC 0/7] perf pmu-events: Support event aliasing for system PMUs

[John Garry]

On 11/02/2020 15:24, James Clark wrote:
> Hi John,

Hi James,

> 
> I tested this on an Arm N1 board and see the same list of CPU events from the JSONs
> picked up so it looks ok from that point of view.

ok, good.

So does this platform have other PMUs (with a kernel driver) which we 
may want to add aliases for, like uncore PMUs, SMMUv3 PMCG, etc?

Thanks,
John

> 
> James
> 
> On 1/24/20 2:34 PM, John Garry wrote:
>> Currently event aliasing for only CPU and uncore PMUs is supported. In
>> fact, only uncore PMUs aliasing for when the uncore PMUs are fixed for a
>> CPU is supported, which may not always be the case for certain
>> architectures.
>>
>> This series adds support for PMU event aliasing for system and other
>> uncore PMUs which are not fixed to a specific CPU.
>>
>> For this, we introduce support for another per-arch mapfile, which maps a
>> particular system identifier to a set of system PMU events for that
>> system. This is much the same as what we do for CPU event aliasing.
>>
>> To support this, we need to change how we match a PMU to a mapfile,
>> whether it should use a CPU or system mapfile. For this we do the
>> following:
>>
>> - For CPU PMU, we always match for the event mapfile based on the CPUID.
>>    This has not changed.
>>
>> - For an uncore or system PMU, we match first based on the SYSID (if set).
>>    If this fails, then we match on the CPUID.
>>
>>    This works for x86, as x86 would not have any system mapfiles for uncore
>>    PMUs (and match on the CPUID).
>>
>> Initial reference support is also added for ARM SMMUv3 PMCG (Performance
>> Monitor Event Group) PMU for HiSilicon hip08 platform with only a single
>> event so far - see driver in drivers/perf/arm_smmuv3_pmu.c for that driver.
>>
>> Here is a sample output with this series:
>>
>> root@ubuntu:/# ./perf list
>>    [...]
>>
>>    smmuv3_pmcg_100020/config_cache_miss/              [Kernel PMU event]
>>    smmuv3_pmcg_100020/config_struct_access/           [Kernel PMU event]
>>    smmuv3_pmcg_100020/cycles/                         [Kernel PMU event]
>>    smmuv3_pmcg_100020/pcie_ats_trans_passed/          [Kernel PMU event]
>>    smmuv3_pmcg_100020/pcie_ats_trans_rq/              [Kernel PMU event]
>>    smmuv3_pmcg_100020/tlb_miss/                       [Kernel PMU event]
>>    smmuv3_pmcg_100020/trans_table_walk_access/        [Kernel PMU event]
>>    smmuv3_pmcg_100020/transaction/                    [Kernel PMU event]
>>
>>    [...]
>>
>> smmu v3 pmcg:
>>    smmuv3_pmcg.l1_tlb
>>         [SMMUv3 PMCG l1_tlb. Unit: smmuv3_pmcg]
>>
>>    [...]
>>
>> root@ubuntu:/# ./perf stat -v -e smmuv3_pmcg.l1_tlb sleep 1
>> Using CPUID 0x00000000480fd010
>> Using SYSID HIP08
>>   -> smmuv3_pmcg_200100020/event=0x8a/
>>   -> smmuv3_pmcg_200140020/event=0x8a/
>>   -> smmuv3_pmcg_100020/event=0x8a/
>>   -> smmuv3_pmcg_140020/event=0x8a/
>>   -> smmuv3_pmcg_200148020/event=0x8a/
>>   -> smmuv3_pmcg_148020/event=0x8a/
>> smmuv3_pmcg.l1_tlb: 0 1001221690 1001221690
>> smmuv3_pmcg.l1_tlb: 0 1001220090 1001220090
>> smmuv3_pmcg.l1_tlb: 101 1001219660 1001219660
>> smmuv3_pmcg.l1_tlb: 0 1001219010 1001219010
>> smmuv3_pmcg.l1_tlb: 0 1001218360 1001218360
>> smmuv3_pmcg.l1_tlb: 134 1001217850 1001217850
>>
>>   Performance counter stats for 'system wide':
>>
>>                 235      smmuv3_pmcg.l1_tlb
>>
>>         1.001263128 seconds time elapsed
>>
>> root@ubuntu:/#
>>
>> Issues with this series which need to be addressed (aware to me):
>>
>> - It would be good to have a universal method to identify the system from
>>    sysfs. Nothing exists which I know about. There is DMI, but this is not
>>    always available (or has correct info). Maybe systems which want to
>>    support this feature will need a "soc" driver, and a
>>    /sys/devices/socX/machine file (which I used for testing this series -
>>    this driver is out of tree currently).
>>
>> - Maybe it is ok, but for systems which match on the system identifier,
>>    uncore PMUs should be in the system mapfile, and, as such, match on the
>>    system identifier and not CPU identifier.
>>
>> - We need a better way in jevents.c to give a direct mapping of PMU name
>>    aliases, i.e. for any PMU name not prefixed with "uncore_", we need to
>>    add this to table unit_to_pmu[]. Not scalable.
>>
>>    Having said that, maybe the kernel can introduce some future PMU naming
>>    convention in future.
>>
>> John Garry (7):
>>    perf jevents: Add support for an extra directory level
>>    perf vendor events arm64: Relocate hip08 core events
>>    perf jevents: Add support for a system events PMU
>>    perf pmu: Rename uncore symbols to include system pmus
>>    perf pmu: Support matching by sysid
>>    perf vendor events arm64: Relocate uncore events for hip08
>>    perf vendor events arm64: Add hip08 SMMUv3 PMCG IMP DEF events
>>
>>   tools/perf/arch/arm64/util/arm-spe.c          |   2 +-
>>   tools/perf/pmu-events/README                  |  47 ++++++--
>>   .../hip08/{ => cpu}/core-imp-def.json         |   0
>>   .../hisilicon/hip08/sys/smmu-v3-pmcg.json     |   9 ++
>>   .../hip08/{ => sys}/uncore-ddrc.json          |   0
>>   .../hisilicon/hip08/{ => sys}/uncore-hha.json |   0
>>   .../hisilicon/hip08/{ => sys}/uncore-l3c.json |   0
>>   tools/perf/pmu-events/arch/arm64/mapfile.csv  |   2 +-
>>   .../pmu-events/arch/arm64/mapfile_sys.csv     |  14 +++
>>   tools/perf/pmu-events/jevents.c               |  65 ++++++++--
>>   tools/perf/pmu-events/pmu-events.h            |   1 +
>>   tools/perf/util/evsel.h                       |   2 +-
>>   tools/perf/util/parse-events.c                |  12 +-
>>   tools/perf/util/pmu.c                         | 111 +++++++++++++++---
>>   tools/perf/util/pmu.h                         |   2 +-
>>   15 files changed, 221 insertions(+), 46 deletions(-)
>>   rename tools/perf/pmu-events/arch/arm64/hisilicon/hip08/{ => cpu}/core-imp-def.json (100%)
>>   create mode 100644 tools/perf/pmu-events/arch/arm64/hisilicon/hip08/sys/smmu-v3-pmcg.json
>>   rename tools/perf/pmu-events/arch/arm64/hisilicon/hip08/{ => sys}/uncore-ddrc.json (100%)
>>   rename tools/perf/pmu-events/arch/arm64/hisilicon/hip08/{ => sys}/uncore-hha.json (100%)
>>   rename tools/perf/pmu-events/arch/arm64/hisilicon/hip08/{ => sys}/uncore-l3c.json (100%)
>>   create mode 100644 tools/perf/pmu-events/arch/arm64/mapfile_sys.csv
>>
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> .
> 

Re: [PATCH RFC 0/7] perf pmu-events: Support event aliasing for system PMUs

[Will Deacon]

On Fri, Jan 24, 2020 at 10:34:58PM +0800, John Garry wrote:
> Currently event aliasing for only CPU and uncore PMUs is supported. In
> fact, only uncore PMUs aliasing for when the uncore PMUs are fixed for a
> CPU is supported, which may not always be the case for certain
> architectures.
> 
> This series adds support for PMU event aliasing for system and other
> uncore PMUs which are not fixed to a specific CPU.
> 
> For this, we introduce support for another per-arch mapfile, which maps a
> particular system identifier to a set of system PMU events for that
> system. This is much the same as what we do for CPU event aliasing.
> 
> To support this, we need to change how we match a PMU to a mapfile,
> whether it should use a CPU or system mapfile. For this we do the
> following:
> 
> - For CPU PMU, we always match for the event mapfile based on the CPUID.
>   This has not changed.
> 
> - For an uncore or system PMU, we match first based on the SYSID (if set).
>   If this fails, then we match on the CPUID.
> 
>   This works for x86, as x86 would not have any system mapfiles for uncore
>   PMUs (and match on the CPUID).
> 
> Initial reference support is also added for ARM SMMUv3 PMCG (Performance
> Monitor Event Group) PMU for HiSilicon hip08 platform with only a single
> event so far - see driver in drivers/perf/arm_smmuv3_pmu.c for that driver.

Why don't we just expose SMMU_IIDR in the SMMUv3 PMU directory, so that
you can key off that? I'm nervous about coming up with a global "SYSID"
when we don't have the ability to standardise anything in that space.

Will

Re: [PATCH RFC 0/7] perf pmu-events: Support event aliasing for system PMUs

[John Garry]

On 18/02/2020 12:57, Will Deacon wrote:
> On Fri, Jan 24, 2020 at 10:34:58PM +0800, John Garry wrote:
>> Currently event aliasing for only CPU and uncore PMUs is supported. In
>> fact, only uncore PMUs aliasing for when the uncore PMUs are fixed for a
>> CPU is supported, which may not always be the case for certain
>> architectures.
>>
>> This series adds support for PMU event aliasing for system and other
>> uncore PMUs which are not fixed to a specific CPU.
>>
>> For this, we introduce support for another per-arch mapfile, which maps a
>> particular system identifier to a set of system PMU events for that
>> system. This is much the same as what we do for CPU event aliasing.
>>
>> To support this, we need to change how we match a PMU to a mapfile,
>> whether it should use a CPU or system mapfile. For this we do the
>> following:
>>
>> - For CPU PMU, we always match for the event mapfile based on the CPUID.
>>    This has not changed.
>>
>> - For an uncore or system PMU, we match first based on the SYSID (if set).
>>    If this fails, then we match on the CPUID.
>>
>>    This works for x86, as x86 would not have any system mapfiles for uncore
>>    PMUs (and match on the CPUID).
>>
>> Initial reference support is also added for ARM SMMUv3 PMCG (Performance
>> Monitor Event Group) PMU for HiSilicon hip08 platform with only a single
>> event so far - see driver in drivers/perf/arm_smmuv3_pmu.c for that driver.
> 
> Why don't we just expose SMMU_IIDR in the SMMUv3 PMU directory, so that
> you can key off that? 

That does not sound like a standard sysfs interface.

Anyway, I don't think that works for every case, quoting from 
https://lkml.org/lkml/2019/10/16/465:

"> Note: I do acknowledge that an overall issue is that we assume all 
PMCG IMP DEF events are same for a given SMMU model.

That assumption does technically fail already - I know MMU-600 has
different IMP-DEF events for its TCU and TBUs, however as long as we can
get as far as "this is some part of an MMU-600" the driver should be
able to figure out the rest ..."

So even if it is solvable here, the kernel driver(s) will need to be 
reworked. And that is just solving one case in many.

I'm nervous about coming up with a global "SYSID"
> when we don't have the ability to standardise anything in that space.

I understand totally, especially if any sysid is based on DT bindings.

But this is some sort of standardization:
https://developer.arm.com/docs/den0028/c, see SMCCC_ARCH_SOC_ID

John

Re: [PATCH RFC 0/7] perf pmu-events: Support event aliasing for system PMUs

[Will Deacon]

On Tue, Feb 18, 2020 at 01:24:38PM +0000, John Garry wrote:
> On 18/02/2020 12:57, Will Deacon wrote:
> > On Fri, Jan 24, 2020 at 10:34:58PM +0800, John Garry wrote:
> > > Currently event aliasing for only CPU and uncore PMUs is supported. In
> > > fact, only uncore PMUs aliasing for when the uncore PMUs are fixed for a
> > > CPU is supported, which may not always be the case for certain
> > > architectures.
> > > 
> > > This series adds support for PMU event aliasing for system and other
> > > uncore PMUs which are not fixed to a specific CPU.
> > > 
> > > For this, we introduce support for another per-arch mapfile, which maps a
> > > particular system identifier to a set of system PMU events for that
> > > system. This is much the same as what we do for CPU event aliasing.
> > > 
> > > To support this, we need to change how we match a PMU to a mapfile,
> > > whether it should use a CPU or system mapfile. For this we do the
> > > following:
> > > 
> > > - For CPU PMU, we always match for the event mapfile based on the CPUID.
> > >    This has not changed.
> > > 
> > > - For an uncore or system PMU, we match first based on the SYSID (if set).
> > >    If this fails, then we match on the CPUID.
> > > 
> > >    This works for x86, as x86 would not have any system mapfiles for uncore
> > >    PMUs (and match on the CPUID).
> > > 
> > > Initial reference support is also added for ARM SMMUv3 PMCG (Performance
> > > Monitor Event Group) PMU for HiSilicon hip08 platform with only a single
> > > event so far - see driver in drivers/perf/arm_smmuv3_pmu.c for that driver.
> > 
> > Why don't we just expose SMMU_IIDR in the SMMUv3 PMU directory, so that
> > you can key off that?
> 
> That does not sound like a standard sysfs interface.

It's standard in the sense that PMUs already have their own directory under
sysfs where you can put things. For example, the "caps" directory is a
dumping ground for all sorts of PMU-specific information.

On the other hand, saying "please go figure out which SoC you're on"
certainly isn't standard and is likely to lead to unreliable, spaghetti
code.

> Anyway, I don't think that works for every case, quoting from
> https://lkml.org/lkml/2019/10/16/465:
> 
> "> Note: I do acknowledge that an overall issue is that we assume all PMCG
> IMP DEF events are same for a given SMMU model.
> 
> That assumption does technically fail already - I know MMU-600 has
> different IMP-DEF events for its TCU and TBUs, however as long as we can
> get as far as "this is some part of an MMU-600" the driver should be
> able to figure out the rest ..."

Perhaps I'm misreading this, but it sounds like if you knew it was an
MMU-600 then you'd be ok. I also don't understand how a SoC ID makes things
any easier in this regard.

> So even if it is solvable here, the kernel driver(s) will need to be
> reworked. And that is just solving one case in many.

PMU drivers will need to expose more information to userspace so that they
can be identified more precisely, yes. I wouldn't say they would need to be
"reworked".

> I'm nervous about coming up with a global "SYSID"
> > when we don't have the ability to standardise anything in that space.
> 
> I understand totally, especially if any sysid is based on DT bindings.

Well if this is going to be ACPI-only then it's a non-starter.

> But this is some sort of standardization:
> https://developer.arm.com/docs/den0028/c, see SMCCC_ARCH_SOC_ID

Yay, firmware :/

Even if this was widely implemented (it's not), I still think that it's
the wrong level of abstraction. Why not do away with ACPI/DT entirely
and predicate everything off the SoC ID?

Will

Re: [PATCH RFC 0/7] perf pmu-events: Support event aliasing for system PMUs

[John Garry]

>>
>>> Why don't we just expose SMMU_IIDR in the SMMUv3 PMU directory, so that
>>> you can key off that?
>>
>> That does not sound like a standard sysfs interface.
> 
> It's standard in the sense that PMUs already have their own directory under
> sysfs where you can put things. 

Sure, but then the perf tool will need to be able to interpret all these 
custom PMU files, which has scalability issues.

Maybe this would work and I did consider it, but another concern is that 
the PMU drivers will have problems making available some 
implementation-specific identifier at all.

For example, the "caps" directory is a
> dumping ground for all sorts of PMU-specific information.
> 
> On the other hand, saying "please go figure out which SoC you're on"
> certainly isn't standard and is likely to lead to unreliable, spaghetti
> code.

I'm not sure how. The perf tool PMU event aliasing already takes a few 
certain steps to figure out which cpuid to use:

static char *perf_pmu__getcpuid(struct perf_pmu *pmu)
{
	char *cpuid;
	static bool printed;

	cpuid = getenv("PERF_CPUID");
	if (cpuid)
		cpuid = strdup(cpuid);
	if (!cpuid)
		cpuid = get_cpuid_str(pmu);
	if (!cpuid)
		return NULL;

	if (!printed) {
		pr_debug("Using CPUID %s\n", cpuid);
		printed = true;
	}
	return cpuid;
}

And this would be something similar - just read some sysfs file.

> 
>> Anyway, I don't think that works for every case, quoting from
>> https://lkml.org/lkml/2019/10/16/465:
>>
>> "> Note: I do acknowledge that an overall issue is that we assume all PMCG
>> IMP DEF events are same for a given SMMU model.
>>
>> That assumption does technically fail already - I know MMU-600 has
>> different IMP-DEF events for its TCU and TBUs, however as long as we can
>> get as far as "this is some part of an MMU-600" the driver should be
>> able to figure out the rest ..."
> 
> Perhaps I'm misreading this, but it sounds like if you knew it was an
> MMU-600 then you'd be ok. I also don't understand how a SoC ID makes things
> any easier in this regard.

It's doesn't necessarily make things easier in this regard. But using a 
SoC ID is an alternative to checking the SMMU_ID or the kernel driver 
having to know that it was a MMU-600 at all.

> 
>> So even if it is solvable here, the kernel driver(s) will need to be
>> reworked. And that is just solving one case in many.
> 
> PMU drivers will need to expose more information to userspace so that they
> can be identified more precisely, yes. I wouldn't say they would need to be
> "reworked".

OK, so some combination of changes would still be required for the SMMU 
PMCG, IORT, and SMMUv3 drivers.

These changes were included in my RFC.

> 
>> I'm nervous about coming up with a global "SYSID"
>>> when we don't have the ability to standardise anything in that space.
>>
>> I understand totally, especially if any sysid is based on DT bindings.
> 
> Well if this is going to be ACPI-only then it's a non-starter.

No, in fact I would rather not rely on ACPI or DT at all.

> 
>> But this is some sort of standardization:
>> https://developer.arm.com/docs/den0028/c, see SMCCC_ARCH_SOC_ID
> 
> Yay, firmware :/
>  > Even if this was widely implemented (it's not),

The spec is in beta stage now.

And if it's not implemented, then simply the perf tool cannot make PMU 
aliases for those DDRC or similar PMUs (and we would find that the DDRC 
or similar JSON files for those platforms would not be added until it 
does support it).

>I still think that it's
> the wrong level of abstraction. 

As I said above, we could try to expand the PMU sysfs entries for this, 
but I have concerns on how we make some imp specific identifier or what 
this would look like.

Why not do away with ACPI/DT entirely
> and predicate everything off the SoC ID?

As constantly checking what the SoC ID means throughout system 
components does not scale.

John

Re: [PATCH RFC 0/7] perf pmu-events: Support event aliasing for system PMUs

[Mark Rutland]

On Tue, Feb 18, 2020 at 04:19:32PM +0000, John Garry wrote:
> > > 
> > > > Why don't we just expose SMMU_IIDR in the SMMUv3 PMU directory, so that
> > > > you can key off that?
> > > 
> > > That does not sound like a standard sysfs interface.
> > 
> > It's standard in the sense that PMUs already have their own directory under
> > sysfs where you can put things.
> 
> Sure, but then the perf tool will need to be able to interpret all these
> custom PMU files, which has scalability issues.
> 
> Maybe this would work and I did consider it, but another concern is that the
> PMU drivers will have problems making available some implementation-specific
> identifier at all.
> 
> For example, the "caps" directory is a
> > dumping ground for all sorts of PMU-specific information.
> > 
> > On the other hand, saying "please go figure out which SoC you're on"
> > certainly isn't standard and is likely to lead to unreliable, spaghetti
> > code.
> 
> I'm not sure how. The perf tool PMU event aliasing already takes a few
> certain steps to figure out which cpuid to use:
> 
> static char *perf_pmu__getcpuid(struct perf_pmu *pmu)
> {
> 	char *cpuid;
> 	static bool printed;
> 
> 	cpuid = getenv("PERF_CPUID");
> 	if (cpuid)
> 		cpuid = strdup(cpuid);
> 	if (!cpuid)
> 		cpuid = get_cpuid_str(pmu);
> 	if (!cpuid)
> 		return NULL;
> 
> 	if (!printed) {
> 		pr_debug("Using CPUID %s\n", cpuid);
> 		printed = true;
> 	}
> 	return cpuid;
> }
> 
> And this would be something similar - just read some sysfs file.
> 
> > 
> > > Anyway, I don't think that works for every case, quoting from
> > > https://lkml.org/lkml/2019/10/16/465:
> > > 
> > > "> Note: I do acknowledge that an overall issue is that we assume all PMCG
> > > IMP DEF events are same for a given SMMU model.
> > > 
> > > That assumption does technically fail already - I know MMU-600 has
> > > different IMP-DEF events for its TCU and TBUs, however as long as we can
> > > get as far as "this is some part of an MMU-600" the driver should be
> > > able to figure out the rest ..."
> > 
> > Perhaps I'm misreading this, but it sounds like if you knew it was an
> > MMU-600 then you'd be ok. I also don't understand how a SoC ID makes things
> > any easier in this regard.
> 
> It's doesn't necessarily make things easier in this regard. But using a SoC
> ID is an alternative to checking the SMMU_ID or the kernel driver having to
> know that it was a MMU-600 at all.

Using SOC_ID means that going forward, userspace needs to learn about
the integration details of each SoC in order to identify a component. As
you said:

| As constantly checking what the SoC ID means throughout system components
| does not scale.

... and I think that equally applies to userspace in this case. Who knows how
many SoCs are going to have MMU-600?

I also know that SOC_ID is going to be optional, and I think it's near-certain
that someone will end up producing two SoCs exposing the same ID.

For system PMUs, I'd rather the system PMU driver exposed some sort of
implementation ID. e.g. the SMMU_ID for SMMU. We can give that a generic name,
and mandate that where a driver exposes it, the format/meaning is defined in
the documentation for the driver.

That can be namespace by driver, so e.g. keys would be smmu_sysfs_name/<id> and
ddrc_sysfs_name/<id>.

> > > So even if it is solvable here, the kernel driver(s) will need to be
> > > reworked. And that is just solving one case in many.
> > 
> > PMU drivers will need to expose more information to userspace so that they
> > can be identified more precisely, yes. I wouldn't say they would need to be
> > "reworked".
> 
> OK, so some combination of changes would still be required for the SMMU
> PMCG, IORT, and SMMUv3 drivers.

To expose the SMMU ID, surely that's just the driver? Or are there
implementations where the ID register is bogus and have to be overridden?

Thanks,
Mark.

Re: [PATCH RFC 0/7] perf pmu-events: Support event aliasing for system PMUs

[John Garry]

On 18/02/2020 17:08, Mark Rutland wrote:
>>> I also don't understand how a SoC ID makes things
>>> any easier in this regard.
>> It's doesn't necessarily make things easier in this regard. But using a SoC
>> ID is an alternative to checking the SMMU_ID or the kernel driver having to
>> know that it was a MMU-600 at all.
> Using SOC_ID means that going forward, userspace needs to learn about
> the integration details of each SoC in order to identify a component. As
> you said:
> 
> | As constantly checking what the SoC ID means throughout system components
> | does not scale.
> 
> ... and I think that equally applies to userspace in this case. Who knows how
> many SoCs are going to have MMU-600?
> 
> I also know that SOC_ID is going to be optional, and I think it's near-certain
> that someone will end up producing two SoCs exposing the same ID.

Wouldn't different SoCs having same SMC SOC_ID and revision be a 
(fixable) mistake in the SMC FW?

And if it's not implemented, then no PMU events aliasing in perf tool 
for those uncore PMUs - nothing gets broken though and no regression. 
But I do understand your concern here.

> 
> For system PMUs, I'd rather the system PMU driver exposed some sort of
> implementation ID. e.g. the SMMU_ID for SMMU. We can give that a generic name,
> and mandate that where a driver exposes it, the format/meaning is defined in
> the documentation for the driver.

Then doesn't that per-PMU ID qualify as brittle and non-standard also?

At least the SMC SoC ID is according to some standard.

And typically most PMU HW would have no ID reg, so where to even get 
this identification info? Joakim Zhang seems to have this problem for 
the imx8 DDRC PMU driver.

> 
> That can be namespace by driver, so e.g. keys would be smmu_sysfs_name/<id> and
> ddrc_sysfs_name/<id>.
> 
>>>> So even if it is solvable here, the kernel driver(s) will need to be
>>>> reworked. And that is just solving one case in many.
>>> PMU drivers will need to expose more information to userspace so that they
>>> can be identified more precisely, yes. I wouldn't say they would need to be
>>> "reworked".
>> OK, so some combination of changes would still be required for the SMMU
>> PMCG, IORT, and SMMUv3 drivers.
> To expose the SMMU ID, surely that's just the driver? 

This case is complicated, like others I anticipate.

So the SMMU PMCG HW has no ID register itself, and this idea relies on 
using the associated SMMUv3 IIDR in lieu. For that, we need to involve 
the IORT, SMMUv3, and SMMU PMCG drivers to create this linkage, and even 
then I still have my doubts on whether this is even proper.

Please see 
https://lore.kernel.org/linux-iommu/1569854031-237636-1-git-send-email-john.garry@huawei.com/ 
for reference.

Or are there
> implementations where the ID register is bogus and have to be overridden?
> 

I will also note that perf tool PMU events framework relies today on 
generating a table of events aliases per CPU and matching based on that. 
If you want to totally disassociate a CPU or any SoC ID mapping, then 
this will require big perf tool rework.

Thanks,
John

Re: [PATCH RFC 0/7] perf pmu-events: Support event aliasing for system PMUs

[Mark Rutland]

On Tue, Feb 18, 2020 at 05:58:46PM +0000, John Garry wrote:
> On 18/02/2020 17:08, Mark Rutland wrote:
> > > > I also don't understand how a SoC ID makes things
> > > > any easier in this regard.
> > > It's doesn't necessarily make things easier in this regard. But using a SoC
> > > ID is an alternative to checking the SMMU_ID or the kernel driver having to
> > > know that it was a MMU-600 at all.
> > Using SOC_ID means that going forward, userspace needs to learn about
> > the integration details of each SoC in order to identify a component. As
> > you said:
> > 
> > | As constantly checking what the SoC ID means throughout system components
> > | does not scale.
> > 
> > ... and I think that equally applies to userspace in this case. Who knows how
> > many SoCs are going to have MMU-600?
> > 
> > I also know that SOC_ID is going to be optional, and I think it's near-certain
> > that someone will end up producing two SoCs exposing the same ID.
> 
> Wouldn't different SoCs having same SMC SOC_ID and revision be a (fixable)
> mistake in the SMC FW?
> 
> And if it's not implemented, then no PMU events aliasing in perf tool for
> those uncore PMUs - nothing gets broken though and no regression. But I do
> understand your concern here.
> 
> > For system PMUs, I'd rather the system PMU driver exposed some sort of
> > implementation ID. e.g. the SMMU_ID for SMMU. We can give that a generic name,
> > and mandate that where a driver exposes it, the format/meaning is defined in
> > the documentation for the driver.
> 
> Then doesn't that per-PMU ID qualify as brittle and non-standard also?

Not in my mind; any instances of the same IP can have the same ID,
regardless of which SoC they're in. Once userspace learns about
device-foo-4000, it knows about it on all SoCs. That also means you can
support heterogeneous instances in the same SoC.

If a device varies so much on a SoC-by-SoC basis and or the driver has
no idea what to expose, it could be legitimate for the PMU driver to
expose the SoC ID as its PMU-specific ID, but I don't think we should
make that the common/only case.

> At least the SMC SoC ID is according to some standard.
> 
> And typically most PMU HW would have no ID reg, so where to even get this
> identification info? Joakim Zhang seems to have this problem for the imx8
> DDRC PMU driver.

For imx8, the DT compat string or additional properties on the DDRC node
could be used to imply the id.

> > That can be namespace by driver, so e.g. keys would be smmu_sysfs_name/<id> and
> > ddrc_sysfs_name/<id>.
> > 
> > > > > So even if it is solvable here, the kernel driver(s) will need to be
> > > > > reworked. And that is just solving one case in many.
> > > > PMU drivers will need to expose more information to userspace so that they
> > > > can be identified more precisely, yes. I wouldn't say they would need to be
> > > > "reworked".
> > > OK, so some combination of changes would still be required for the SMMU
> > > PMCG, IORT, and SMMUv3 drivers.
> > To expose the SMMU ID, surely that's just the driver?
> 
> This case is complicated, like others I anticipate.
> 
> So the SMMU PMCG HW has no ID register itself, and this idea relies on using
> the associated SMMUv3 IIDR in lieu. For that, we need to involve the IORT,
> SMMUv3, and SMMU PMCG drivers to create this linkage, and even then I still
> have my doubts on whether this is even proper.

Ok, I hadn't appreciated that the PMCG did not have an ID register
itself.

I think that the relationship between the SMMU and PMCG is a stronger
argument against using the SOC_ID. If the PMCGs in a system are
heterogeneous, then you must know the type of the specific instance.

> Please see https://lore.kernel.org/linux-iommu/1569854031-237636-1-git-send-email-john.garry@huawei.com/
> for reference.
> 
> Or are there
> > implementations where the ID register is bogus and have to be overridden?
> > 
> 
> I will also note that perf tool PMU events framework relies today on
> generating a table of events aliases per CPU and matching based on that. If
> you want to totally disassociate a CPU or any SoC ID mapping, then this will
> require big perf tool rework.

I think that might be necessary, as otherwise we're going to back
ourselves into a corner by building what's simple now.

Thanks,
Mark.

RE: [PATCH RFC 0/7] perf pmu-events: Support event aliasing for system PMUs

[Joakim Zhang]

> -----Original Message-----
> From: Mark Rutland <mark.rutland@arm.com>
> Sent: 2020年2月19日 2:14
> To: John Garry <john.garry@huawei.com>
> Cc: ak@linux.intel.com; Joakim Zhang <qiangqing.zhang@nxp.com>;
> suzuki.poulose@arm.com; peterz@infradead.org; Will Deacon
> <will@kernel.org>; Linuxarm <linuxarm@huawei.com>; acme@kernel.org;
> linux-kernel@vger.kernel.org; Zhangshaokun <zhangshaokun@hisilicon.com>;
> alexander.shishkin@linux.intel.com; mingo@redhat.com;
> james.clark@arm.com; namhyung@kernel.org; jolsa@redhat.com;
> linux-arm-kernel@lists.infradead.org; robin.murphy@arm.com; Sudeep Holla
> <sudeep.holla@arm.com>
> Subject: Re: [PATCH RFC 0/7] perf pmu-events: Support event aliasing for
> system PMUs

[...]
> > And typically most PMU HW would have no ID reg, so where to even get
> > this identification info? Joakim Zhang seems to have this problem for
> > the imx8 DDRC PMU driver.
> 
> For imx8, the DT compat string or additional properties on the DDRC node
> could be used to imply the id.

Hi Mark,

Yes, actually we can expose something like DDRC_ID to indicate each specific DDR controller, to point out the filter feature.
But, even the SoCs integrated the same DDRC_ID, just say that they have the same DDRC controller.

From user space, the usage is different, for example:

i.MX8MM and i.MX8MN, they use the same driver(DDRC_ID) and cortex-a53 integrated.

If we want to monitor VPU, their *master id* is different from SoCs.
On i.MX8MM, event is imx8_ddr0/axid-read,axi_id=0x08/
On i.MX8MN, event is imx8_ddr0/axid-read,axi_id=0x12/

I try to write a JSON file to use these events, for now, I only can locate the file at the directory: tools/perf/pmu-events/arch/arm64/arm/cortex-a53/

Perf tool loads all events when CPUID matched, which is now unreasonable, we want related events are loaded for specific SoC.

All events will also be loaded if we use DDRC_ID to match in the future, this seems to not be a good ideal.

Best Regards,
Joakim Zhang

[....]

Re: [PATCH RFC 0/7] perf pmu-events: Support event aliasing for system PMUs

[John Garry]

On 19/02/2020 01:55, Joakim Zhang wrote:
> 
>> -----Original Message-----
>> From: Mark Rutland <mark.rutland@arm.com>
>> Sent: 2020年2月19日 2:14
>> To: John Garry <john.garry@huawei.com>
>> Cc: ak@linux.intel.com; Joakim Zhang <qiangqing.zhang@nxp.com>;
>> suzuki.poulose@arm.com; peterz@infradead.org; Will Deacon
>> <will@kernel.org>; Linuxarm <linuxarm@huawei.com>; acme@kernel.org;
>> linux-kernel@vger.kernel.org; Zhangshaokun <zhangshaokun@hisilicon.com>;
>> alexander.shishkin@linux.intel.com; mingo@redhat.com;
>> james.clark@arm.com; namhyung@kernel.org; jolsa@redhat.com;
>> linux-arm-kernel@lists.infradead.org; robin.murphy@arm.com; Sudeep Holla
>> <sudeep.holla@arm.com>
>> Subject: Re: [PATCH RFC 0/7] perf pmu-events: Support event aliasing for
>> system PMUs
> 
> [...]
>>> And typically most PMU HW would have no ID reg, so where to even get
>>> this identification info? Joakim Zhang seems to have this problem for
>>> the imx8 DDRC PMU driver.
>>
>> For imx8, the DT compat string or additional properties on the DDRC node
>> could be used to imply the id.
> 
> Hi Mark,
> 
> Yes, actually we can expose something like DDRC_ID to indicate each specific DDR controller, to point out the filter feature.
> But, even the SoCs integrated the same DDRC_ID, just say that they have the same DDRC controller.
> 
>  From user space, the usage is different, for example:
> 
> i.MX8MM and i.MX8MN, they use the same driver(DDRC_ID) and cortex-a53 integrated.
> 
> If we want to monitor VPU, their *master id* is different from SoCs.
> On i.MX8MM, event is imx8_ddr0/axid-read,axi_id=0x08/
> On i.MX8MN, event is imx8_ddr0/axid-read,axi_id=0x12/
> 
> I try to write a JSON file to use these events, for now, I only can locate the file at the directory: tools/perf/pmu-events/arch/arm64/arm/cortex-a53/
> 
> Perf tool loads all events when CPUID matched, which is now unreasonable, we want related events are loaded for specific SoC.

so we could have a folder like .../arch/arm64/nxp/system for these 
JSONs. The perf tool can be updated to handle CPU and system events in 
separate folders.

> 
> All events will also be loaded if we use DDRC_ID to match in the future, this seems to not be a good ideal.

The important part is knowing which events are supported per 
implementation. Is there any method in the driver of knowing the 
specific implementation, like any DT compat string? Least preferred 
option would be DT machine ID.

> 

John

RE: [PATCH RFC 0/7] perf pmu-events: Support event aliasing for system PMUs

[Joakim Zhang]

> -----Original Message-----
> From: John Garry <john.garry@huawei.com>
> Sent: 2020年2月19日 16:44
> To: Joakim Zhang <qiangqing.zhang@nxp.com>; Mark Rutland
> <mark.rutland@arm.com>
> Cc: ak@linux.intel.com; suzuki.poulose@arm.com; peterz@infradead.org; Will
> Deacon <will@kernel.org>; Linuxarm <linuxarm@huawei.com>;
> acme@kernel.org; linux-kernel@vger.kernel.org; Zhangshaokun
> <zhangshaokun@hisilicon.com>; alexander.shishkin@linux.intel.com;
> mingo@redhat.com; james.clark@arm.com; namhyung@kernel.org;
> jolsa@redhat.com; linux-arm-kernel@lists.infradead.org;
> robin.murphy@arm.com; Sudeep Holla <sudeep.holla@arm.com>
> Subject: Re: [PATCH RFC 0/7] perf pmu-events: Support event aliasing for
> system PMUs
> 
> On 19/02/2020 01:55, Joakim Zhang wrote:
> >
> >> -----Original Message-----
> >> From: Mark Rutland <mark.rutland@arm.com>
> >> Sent: 2020年2月19日 2:14
> >> To: John Garry <john.garry@huawei.com>
> >> Cc: ak@linux.intel.com; Joakim Zhang <qiangqing.zhang@nxp.com>;
> >> suzuki.poulose@arm.com; peterz@infradead.org; Will Deacon
> >> <will@kernel.org>; Linuxarm <linuxarm@huawei.com>; acme@kernel.org;
> >> linux-kernel@vger.kernel.org; Zhangshaokun
> >> <zhangshaokun@hisilicon.com>; alexander.shishkin@linux.intel.com;
> >> mingo@redhat.com; james.clark@arm.com; namhyung@kernel.org;
> >> jolsa@redhat.com; linux-arm-kernel@lists.infradead.org;
> >> robin.murphy@arm.com; Sudeep Holla <sudeep.holla@arm.com>
> >> Subject: Re: [PATCH RFC 0/7] perf pmu-events: Support event aliasing
> >> for system PMUs
> >
> > [...]
> >>> And typically most PMU HW would have no ID reg, so where to even get
> >>> this identification info? Joakim Zhang seems to have this problem
> >>> for the imx8 DDRC PMU driver.
> >>
> >> For imx8, the DT compat string or additional properties on the DDRC
> >> node could be used to imply the id.
> >
> > Hi Mark,
> >
> > Yes, actually we can expose something like DDRC_ID to indicate each specific
> DDR controller, to point out the filter feature.
> > But, even the SoCs integrated the same DDRC_ID, just say that they have the
> same DDRC controller.
> >
> >  From user space, the usage is different, for example:
> >
> > i.MX8MM and i.MX8MN, they use the same driver(DDRC_ID) and cortex-a53
> integrated.
> >
> > If we want to monitor VPU, their *master id* is different from SoCs.
> > On i.MX8MM, event is imx8_ddr0/axid-read,axi_id=0x08/ On i.MX8MN,
> > event is imx8_ddr0/axid-read,axi_id=0x12/
> >
> > I try to write a JSON file to use these events, for now, I only can
> > locate the file at the directory:
> > tools/perf/pmu-events/arch/arm64/arm/cortex-a53/
> >
> > Perf tool loads all events when CPUID matched, which is now unreasonable,
> we want related events are loaded for specific SoC.
> 
> so we could have a folder like .../arch/arm64/nxp/system for these JSONs. The
> perf tool can be updated to handle CPU and system events in separate folders.
> 
> >
> > All events will also be loaded if we use DDRC_ID to match in the future, this
> seems to not be a good ideal.
> 
> The important part is knowing which events are supported per implementation.
> Is there any method in the driver of knowing the specific implementation, like
> any DT compat string? Least preferred option would be DT machine ID.

I think, NO, master id could be different even they use the same DT compatible string.

Best Regards,
Joakim Zhang
> >
> 
> John

Re: [PATCH RFC 0/7] perf pmu-events: Support event aliasing for system PMUs

[John Garry]

>>> i.MX8MM and i.MX8MN, they use the same driver(DDRC_ID) and cortex-a53
>> integrated.
>>>
>>> If we want to monitor VPU, their *master id* is different from SoCs.
>>> On i.MX8MM, event is imx8_ddr0/axid-read,axi_id=0x08/ On i.MX8MN,
>>> event is imx8_ddr0/axid-read,axi_id=0x12/
>>>

So it seems that this master id and the axi_id are the same, which is 
some filtering key. Indeed, the actual event number is the same between 
SoC implementations.

And metric groups do support filtering, AFAIU.

>>> I try to write a JSON file to use these events, for now, I only can
>>> locate the file at the directory:
>>> tools/perf/pmu-events/arch/arm64/arm/cortex-a53/
>>>
>>> Perf tool loads all events when CPUID matched, which is now unreasonable,
>> we want related events are loaded for specific SoC.
>>
>> so we could have a folder like .../arch/arm64/nxp/system for these JSONs. The
>> perf tool can be updated to handle CPU and system events in separate folders.
>>
>>>
>>> All events will also be loaded if we use DDRC_ID to match in the future, this
>> seems to not be a good ideal.
>>
>> The important part is knowing which events are supported per implementation.
>> Is there any method in the driver of knowing the specific implementation, like
>> any DT compat string? Least preferred option would be DT machine ID.
> 
> I think, NO, master id could be different even they use the same DT compatible string.

Are you sure? Checking the dts files for your SoCs, I see this:

~/kernel-dev6/arch/arm64$ git grep  "fsl,imx8m-ddr-pmu"
boot/dts/freescale/imx8mm.dtsi:                 compatible = 
"fsl,imx8mm-ddr-pmu", "fsl,imx8m-ddr-pmu";
boot/dts/freescale/imx8mn.dtsi:                 compatible = 
"fsl,imx8mn-ddr-pmu", "fsl,imx8m-ddr-pmu";
boot/dts/freescale/imx8mq.dtsi:                 compatible = 
"fsl,imx8mq-ddr-pmu", "fsl,imx8m-ddr-pmu";

So it seems that you could use "fsl,imx8mm-ddr-pmu" vs 
"fsl,imx8mn-ddr-pmu" to differentiate, right?

John

Re: [PATCH RFC 0/7] perf pmu-events: Support event aliasing for system PMUs

[John Garry]

>>
>>> For system PMUs, I'd rather the system PMU driver exposed some sort of
>>> implementation ID. e.g. the SMMU_ID for SMMU. We can give that a generic name,
>>> and mandate that where a driver exposes it, the format/meaning is defined in
>>> the documentation for the driver.
>>
>> Then doesn't that per-PMU ID qualify as brittle and non-standard also?
> 
> Not in my mind; any instances of the same IP can have the same ID,
> regardless of which SoC they're in. Once userspace learns about
> device-foo-4000, it knows about it on all SoCs. That also means you can
> support heterogeneous instances in the same SoC.

Sure, but this device-foo-4000 naming is a concern for standardization, 
stable ABI, and perf tool support.

> 
> If a device varies so much on a SoC-by-SoC basis and or the driver has
> no idea what to expose, it could be legitimate for the PMU driver to
> expose the SoC ID as its PMU-specific ID, but I don't think we should
> make that the common/only case.

But where does the PMU driver get the SoC ID? Does it have to check DT 
machine ID, ACPI OEM ID, or SMCCC SOC ID?

I can't imagine that you really want this stuff in the kernel PMU 
drivers, but that's your call.

> 
>> At least the SMC SoC ID is according to some standard.
>>
>> And typically most PMU HW would have no ID reg, so where to even get this
>> identification info? Joakim Zhang seems to have this problem for the imx8
>> DDRC PMU driver.
> 
> For imx8, the DT compat string or additional properties on the DDRC node
> could be used to imply the id.

Fine, but it's the ACPI case which is what I am concerned about.

> 
>>> That can be namespace by driver, so e.g. keys would be smmu_sysfs_name/<id> and
>>> ddrc_sysfs_name/<id>.
>>>
>>>>>> So even if it is solvable here, the kernel driver(s) will need to be
>>>>>> reworked. And that is just solving one case in many.
>>>>> PMU drivers will need to expose more information to userspace so that they
>>>>> can be identified more precisely, yes. I wouldn't say they would need to be
>>>>> "reworked".
>>>> OK, so some combination of changes would still be required for the SMMU
>>>> PMCG, IORT, and SMMUv3 drivers.
>>> To expose the SMMU ID, surely that's just the driver?
>>
>> This case is complicated, like others I anticipate.
>>
>> So the SMMU PMCG HW has no ID register itself, and this idea relies on using
>> the associated SMMUv3 IIDR in lieu. For that, we need to involve the IORT,
>> SMMUv3, and SMMU PMCG drivers to create this linkage, and even then I still
>> have my doubts on whether this is even proper.
> 
> Ok, I hadn't appreciated that the PMCG did not have an ID register
> itself.
> 
> I think that the relationship between the SMMU and PMCG is a stronger
> argument against using the SOC_ID. If the PMCGs in a system are
> heterogeneous, then you must know the type of the specific instance.

Perf tool PMU events can handle that. Each PMCG PMU instance has a 
different name - the name encoding includes the HW base address, so 
always unique per system - and then so the JSON can know this and have 
events specific per instance.

> 
>> Please see https://lore.kernel.org/linux-iommu/1569854031-237636-1-git-send-email-john.garry@huawei.com/
>> for reference.
>>
>> Or are there
>>> implementations where the ID register is bogus and have to be overridden?
>>>
>>
>> I will also note that perf tool PMU events framework relies today on
>> generating a table of events aliases per CPU and matching based on that. If
>> you want to totally disassociate a CPU or any SoC ID mapping, then this will
>> require big perf tool rework.
> 
> I think that might be necessary, as otherwise we're going to back
> ourselves into a corner by building what's simple now.

I appreciate what you're saying. I'll check this idea further.

Cheers,
John