[PATCH v1 0/3] arm64: Support Cortex-X4 CPU for Perf Arm SPE

[PATCH v1 0/3] arm64: Support Cortex-X4 CPU for Perf Arm SPE

[Leo Yan]

This series support Cortex-X4 CPU in Perf Arm SPE.

The Arm64 and tools both include the header cputype.h for CPU part and
MIDR definitions, to de-couple between the tools and the kernel, the
tools doesn't directly use the kernel's header, alternatively, the tools
maintain a copy and sync with kernel's header.

To keep the exact same content between kernel and tools' headers, this
series firstly adds Cortex-X4 CPU part and MIDR definitions in the
kernel header; then the second patch syncs the change into the tools'
header.  The first patch is to support the Cortex-X4 in perf Arm SPE
with the new CPU definitions.

I don't have Cortex-X4 machine in hand, so just verified with
compilation perf tool.


Leo Yan (3):
  arm64: Add Cortex-X4 CPU part definitions
  tools headers arm64: Sync Cortex-X4 CPU part definitions
  perf arm-spe: Support data source for Cortex-X4 CPU

 arch/arm64/include/asm/cputype.h       |  2 ++
 tools/arch/arm64/include/asm/cputype.h |  2 ++
 tools/perf/util/arm-spe.c              | 14 ++++++++------
 3 files changed, 12 insertions(+), 6 deletions(-)

-- 
2.34.1

[PATCH v1 1/3] arm64: Add Cortex-X4 CPU part definitions

[Leo Yan]

Add the part number and MIDR definitions for Cortex-X4.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 arch/arm64/include/asm/cputype.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
index 5f6f84837a49..415be1a000c6 100644
--- a/arch/arm64/include/asm/cputype.h
+++ b/arch/arm64/include/asm/cputype.h
@@ -84,6 +84,7 @@
 #define ARM_CPU_PART_CORTEX_X2		0xD48
 #define ARM_CPU_PART_NEOVERSE_N2	0xD49
 #define ARM_CPU_PART_CORTEX_A78C	0xD4B
+#define ARM_CPU_PART_CORTEX_X4		0xD82
 
 #define APM_CPU_PART_POTENZA		0x000
 
@@ -153,6 +154,7 @@
 #define MIDR_CORTEX_X2 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_X2)
 #define MIDR_NEOVERSE_N2 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_NEOVERSE_N2)
 #define MIDR_CORTEX_A78C	MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A78C)
+#define MIDR_CORTEX_X4 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_X4)
 #define MIDR_THUNDERX	MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX)
 #define MIDR_THUNDERX_81XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_81XX)
 #define MIDR_THUNDERX_83XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_83XX)
-- 
2.34.1

[PATCH v1 2/3] tools headers arm64: Sync Cortex-X4 CPU part definitions

[Leo Yan]

Sync Cortex-X4 CPU part number and MIDR definitions with the kernel
header.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 tools/arch/arm64/include/asm/cputype.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/arch/arm64/include/asm/cputype.h b/tools/arch/arm64/include/asm/cputype.h
index 5f6f84837a49..415be1a000c6 100644
--- a/tools/arch/arm64/include/asm/cputype.h
+++ b/tools/arch/arm64/include/asm/cputype.h
@@ -84,6 +84,7 @@
 #define ARM_CPU_PART_CORTEX_X2		0xD48
 #define ARM_CPU_PART_NEOVERSE_N2	0xD49
 #define ARM_CPU_PART_CORTEX_A78C	0xD4B
+#define ARM_CPU_PART_CORTEX_X4		0xD82
 
 #define APM_CPU_PART_POTENZA		0x000
 
@@ -153,6 +154,7 @@
 #define MIDR_CORTEX_X2 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_X2)
 #define MIDR_NEOVERSE_N2 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_NEOVERSE_N2)
 #define MIDR_CORTEX_A78C	MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A78C)
+#define MIDR_CORTEX_X4 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_X4)
 #define MIDR_THUNDERX	MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX)
 #define MIDR_THUNDERX_81XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_81XX)
 #define MIDR_THUNDERX_83XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_83XX)
-- 
2.34.1

[PATCH v1 3/3] perf arm-spe: Support data source for Cortex-X4 CPU

[Leo Yan]

We have a CPU list to maintain Neoverse CPUs (N1/N2/V2), this list is
used for parsing data source packet.  Since Cortex-x4 CPU shares the
same data source format with Neoverse CPUs, this commit adds Cortex-x4
CPU into the CPU list so we can reuse the parsing logic.

The CPU list was assumed for only Neoverse CPUs, but now Cortex-X4 has
been added into the list.  To avoid Neoverse specific naming, this patch
renames the variables and function as the default data source format.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 tools/perf/util/arm-spe.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/arm-spe.c b/tools/perf/util/arm-spe.c
index afbd5869f6bf..c2cdb9f2e188 100644
--- a/tools/perf/util/arm-spe.c
+++ b/tools/perf/util/arm-spe.c
@@ -409,15 +409,16 @@ static int arm_spe__synth_instruction_sample(struct arm_spe_queue *speq,
 	return arm_spe_deliver_synth_event(spe, speq, event, &sample);
 }
 
-static const struct midr_range neoverse_spe[] = {
+static const struct midr_range cpus_use_default_data_src[] = {
 	MIDR_ALL_VERSIONS(MIDR_NEOVERSE_N1),
 	MIDR_ALL_VERSIONS(MIDR_NEOVERSE_N2),
 	MIDR_ALL_VERSIONS(MIDR_NEOVERSE_V1),
+	MIDR_ALL_VERSIONS(MIDR_CORTEX_X4),
 	{},
 };
 
-static void arm_spe__synth_data_source_neoverse(const struct arm_spe_record *record,
-						union perf_mem_data_src *data_src)
+static void arm_spe__synth_data_source_default(const struct arm_spe_record *record,
+					       union perf_mem_data_src *data_src)
 {
 	/*
 	 * Even though four levels of cache hierarchy are possible, no known
@@ -518,7 +519,8 @@ static void arm_spe__synth_data_source_generic(const struct arm_spe_record *reco
 static u64 arm_spe__synth_data_source(const struct arm_spe_record *record, u64 midr)
 {
 	union perf_mem_data_src	data_src = { .mem_op = PERF_MEM_OP_NA };
-	bool is_neoverse = is_midr_in_range_list(midr, neoverse_spe);
+	bool is_default_dc =
+		is_midr_in_range_list(midr, cpus_use_default_data_src);
 
 	if (record->op & ARM_SPE_OP_LD)
 		data_src.mem_op = PERF_MEM_OP_LOAD;
@@ -527,8 +529,8 @@ static u64 arm_spe__synth_data_source(const struct arm_spe_record *record, u64 m
 	else
 		return 0;
 
-	if (is_neoverse)
-		arm_spe__synth_data_source_neoverse(record, &data_src);
+	if (is_default_dc)
+		arm_spe__synth_data_source_default(record, &data_src);
 	else
 		arm_spe__synth_data_source_generic(record, &data_src);
 
-- 
2.34.1

Re: [PATCH v1 0/3] arm64: Support Cortex-X4 CPU for Perf Arm SPE

[Ali Saidi]

Hi Leo,

On Mon, 17 Jul 2023 05:43:24 +0000, Leo Yan wrote:
> This series support Cortex-X4 CPU in Perf Arm SPE.
> 
> The Arm64 and tools both include the header cputype.h for CPU part and
> MIDR definitions, to de-couple between the tools and the kernel, the
> tools doesn't directly use the kernel's header, alternatively, the tools
> maintain a copy and sync with kernel's header.
> 
> To keep the exact same content between kernel and tools' headers, this
> series firstly adds Cortex-X4 CPU part and MIDR definitions in the
> kernel header; then the second patch syncs the change into the tools'
> header.  The first patch is to support the Cortex-X4 in perf Arm SPE
> with the new CPU definitions.
> 
> I don't have Cortex-X4 machine in hand, so just verified with
> compilation perf tool.

This looks good to me, but can we add the other cores that operate the
same way now too? Flipping through the TRMs A78, X3, V2, X1, A715,
A720, and A78C all have the same encodings. 

Reviewed-by: Ali Saidi <alisaidi@amazon.com>

Thanks!
Ali

Re: [PATCH v1 1/3] arm64: Add Cortex-X4 CPU part definitions

[Anshuman Khandual]

On 7/17/23 11:13, Leo Yan wrote:
> Add the part number and MIDR definitions for Cortex-X4.
> 
> Signed-off-by: Leo Yan <leo.yan@linaro.org>

LGTM per https://developer.arm.com/documentation/102484/0001/AArch64-registers/AArch64-Identification-registers-summary/MIDR-EL1--Main-ID-Register

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

> ---
>  arch/arm64/include/asm/cputype.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
> index 5f6f84837a49..415be1a000c6 100644
> --- a/arch/arm64/include/asm/cputype.h
> +++ b/arch/arm64/include/asm/cputype.h
> @@ -84,6 +84,7 @@
>  #define ARM_CPU_PART_CORTEX_X2		0xD48
>  #define ARM_CPU_PART_NEOVERSE_N2	0xD49
>  #define ARM_CPU_PART_CORTEX_A78C	0xD4B
> +#define ARM_CPU_PART_CORTEX_X4		0xD82
>  
>  #define APM_CPU_PART_POTENZA		0x000
>  
> @@ -153,6 +154,7 @@
>  #define MIDR_CORTEX_X2 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_X2)
>  #define MIDR_NEOVERSE_N2 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_NEOVERSE_N2)
>  #define MIDR_CORTEX_A78C	MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A78C)
> +#define MIDR_CORTEX_X4 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_X4)
>  #define MIDR_THUNDERX	MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX)
>  #define MIDR_THUNDERX_81XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_81XX)
>  #define MIDR_THUNDERX_83XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_83XX)

Re: [PATCH v1 1/3] arm64: Add Cortex-X4 CPU part definitions

[Anshuman Khandual]

On 7/17/23 11:13, Leo Yan wrote:
> Add the part number and MIDR definitions for Cortex-X4.
> 
> Signed-off-by: Leo Yan <leo.yan@linaro.org>

LGTM per https://developer.arm.com/documentation/102484/0001/AArch64-registers/AArch64-Identification-registers-summary/MIDR-EL1--Main-ID-Register

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

> ---
>  arch/arm64/include/asm/cputype.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
> index 5f6f84837a49..415be1a000c6 100644
> --- a/arch/arm64/include/asm/cputype.h
> +++ b/arch/arm64/include/asm/cputype.h
> @@ -84,6 +84,7 @@
>  #define ARM_CPU_PART_CORTEX_X2		0xD48
>  #define ARM_CPU_PART_NEOVERSE_N2	0xD49
>  #define ARM_CPU_PART_CORTEX_A78C	0xD4B
> +#define ARM_CPU_PART_CORTEX_X4		0xD82
>  
>  #define APM_CPU_PART_POTENZA		0x000
>  
> @@ -153,6 +154,7 @@
>  #define MIDR_CORTEX_X2 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_X2)
>  #define MIDR_NEOVERSE_N2 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_NEOVERSE_N2)
>  #define MIDR_CORTEX_A78C	MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A78C)
> +#define MIDR_CORTEX_X4 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_X4)
>  #define MIDR_THUNDERX	MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX)
>  #define MIDR_THUNDERX_81XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_81XX)
>  #define MIDR_THUNDERX_83XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_83XX)

Re: [PATCH v1 3/3] perf arm-spe: Support data source for Cortex-X4 CPU

[Anshuman Khandual]

On 7/17/23 11:13, Leo Yan wrote:
> We have a CPU list to maintain Neoverse CPUs (N1/N2/V2), this list is
> used for parsing data source packet.  Since Cortex-x4 CPU shares the
> same data source format with Neoverse CPUs, this commit adds Cortex-x4
> CPU into the CPU list so we can reuse the parsing logic.
> 
> The CPU list was assumed for only Neoverse CPUs, but now Cortex-X4 has
> been added into the list.  To avoid Neoverse specific naming, this patch
> renames the variables and function as the default data source format.
> 
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
>  tools/perf/util/arm-spe.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/perf/util/arm-spe.c b/tools/perf/util/arm-spe.c
> index afbd5869f6bf..c2cdb9f2e188 100644
> --- a/tools/perf/util/arm-spe.c
> +++ b/tools/perf/util/arm-spe.c
> @@ -409,15 +409,16 @@ static int arm_spe__synth_instruction_sample(struct arm_spe_queue *speq,
>  	return arm_spe_deliver_synth_event(spe, speq, event, &sample);
>  }
>  
> -static const struct midr_range neoverse_spe[] = {
> +static const struct midr_range cpus_use_default_data_src[] = {

Is not 'cpus_use_default_data_src' too long ? 'use' could be dropped ?

>  	MIDR_ALL_VERSIONS(MIDR_NEOVERSE_N1),
>  	MIDR_ALL_VERSIONS(MIDR_NEOVERSE_N2),
>  	MIDR_ALL_VERSIONS(MIDR_NEOVERSE_V1),
> +	MIDR_ALL_VERSIONS(MIDR_CORTEX_X4),
>  	{},
>  };
>  
> -static void arm_spe__synth_data_source_neoverse(const struct arm_spe_record *record,
> -						union perf_mem_data_src *data_src)
> +static void arm_spe__synth_data_source_default(const struct arm_spe_record *record,
> +					       union perf_mem_data_src *data_src)
>  {
>  	/*
>  	 * Even though four levels of cache hierarchy are possible, no known
> @@ -518,7 +519,8 @@ static void arm_spe__synth_data_source_generic(const struct arm_spe_record *reco
>  static u64 arm_spe__synth_data_source(const struct arm_spe_record *record, u64 midr)
>  {
>  	union perf_mem_data_src	data_src = { .mem_op = PERF_MEM_OP_NA };
> -	bool is_neoverse = is_midr_in_range_list(midr, neoverse_spe);
> +	bool is_default_dc =

_dc stands for ?

> +		is_midr_in_range_list(midr, cpus_use_default_data_src);
>  
>  	if (record->op & ARM_SPE_OP_LD)
>  		data_src.mem_op = PERF_MEM_OP_LOAD;
> @@ -527,8 +529,8 @@ static u64 arm_spe__synth_data_source(const struct arm_spe_record *record, u64 m
>  	else
>  		return 0;
>  
> -	if (is_neoverse)
> -		arm_spe__synth_data_source_neoverse(record, &data_src);
> +	if (is_default_dc)
> +		arm_spe__synth_data_source_default(record, &data_src);
>  	else
>  		arm_spe__synth_data_source_generic(record, &data_src);
>  

Re: [PATCH v1 3/3] perf arm-spe: Support data source for Cortex-X4 CPU

[Leo Yan]

Hi Anshuman,

On Mon, Jul 24, 2023 at 12:27:31PM +0530, Anshuman Khandual wrote:

[...]

> > -static const struct midr_range neoverse_spe[] = {
> > +static const struct midr_range cpus_use_default_data_src[] = {
> 
> Is not 'cpus_use_default_data_src' too long ? 'use' could be dropped ?

Okay, I can drop 'use'.

> >  	MIDR_ALL_VERSIONS(MIDR_NEOVERSE_N1),
> >  	MIDR_ALL_VERSIONS(MIDR_NEOVERSE_N2),
> >  	MIDR_ALL_VERSIONS(MIDR_NEOVERSE_V1),
> > +	MIDR_ALL_VERSIONS(MIDR_CORTEX_X4),
> >  	{},
> >  };

[...]

> >  static u64 arm_spe__synth_data_source(const struct arm_spe_record *record, u64 midr)
> >  {
> >  	union perf_mem_data_src	data_src = { .mem_op = PERF_MEM_OP_NA };
> > -	bool is_neoverse = is_midr_in_range_list(midr, neoverse_spe);
> > +	bool is_default_dc =
> 
> _dc stands for ?

Thanks for pointing out this; actually I mean '_ds' which stands for
data source.  Will spin a new patch for this.

Thanks for review!

Leo

Re: [PATCH v1 0/3] arm64: Support Cortex-X4 CPU for Perf Arm SPE

[Leo Yan]

Hi Ali,

On Fri, Jul 21, 2023 at 06:16:53PM +0000, Ali Saidi wrote:

> > This series support Cortex-X4 CPU in Perf Arm SPE.

[...]

> This looks good to me, but can we add the other cores that operate the
> same way now too? Flipping through the TRMs A78, X3, V2, X1, A715,
> A720, and A78C all have the same encodings. 

Thanks a lot for exploring more CPU variants which share the same data
source packet format.

The latest Linux kernel have defined the CPU part number and MIDR for
below CPU variants:

- A78
- X1
- A715
- A78C

I would like to use a patch to support these CPUs in perf tool.  Given
other CPU variants (X3/V2/A720) have not been supported in the kernel,
and so far no one requests them, I would like leave them out.

Please let me know if this okay for you or not.


> Reviewed-by: Ali Saidi <alisaidi@amazon.com>

Thanks for review, I will add your review tags in the new patch set.

Leo

Re: [PATCH v1 3/3] perf arm-spe: Support data source for Cortex-X4 CPU

[Will Deacon]

On Mon, Jul 24, 2023 at 07:05:09PM +0800, Leo Yan wrote:
> On Mon, Jul 24, 2023 at 12:27:31PM +0530, Anshuman Khandual wrote:
> 
> [...]
> 
> > > -static const struct midr_range neoverse_spe[] = {
> > > +static const struct midr_range cpus_use_default_data_src[] = {
> > 
> > Is not 'cpus_use_default_data_src' too long ? 'use' could be dropped ?
> 
> Okay, I can drop 'use'.
> 
> > >  	MIDR_ALL_VERSIONS(MIDR_NEOVERSE_N1),
> > >  	MIDR_ALL_VERSIONS(MIDR_NEOVERSE_N2),
> > >  	MIDR_ALL_VERSIONS(MIDR_NEOVERSE_V1),
> > > +	MIDR_ALL_VERSIONS(MIDR_CORTEX_X4),
> > >  	{},
> > >  };
> 
> [...]
> 
> > >  static u64 arm_spe__synth_data_source(const struct arm_spe_record *record, u64 midr)
> > >  {
> > >  	union perf_mem_data_src	data_src = { .mem_op = PERF_MEM_OP_NA };
> > > -	bool is_neoverse = is_midr_in_range_list(midr, neoverse_spe);
> > > +	bool is_default_dc =
> > 
> > _dc stands for ?
> 
> Thanks for pointing out this; actually I mean '_ds' which stands for
> data source.  Will spin a new patch for this.

Thanks. Please can you put patch 2 (the one touching tools) at the end of
the series, too? That way I can easily pick up the kernel changes.

Will

Re: [PATCH v1 0/3] arm64: Support Cortex-X4 CPU for Perf Arm SPE

[Arnaldo Carvalho de Melo]

Em Mon, Jul 24, 2023 at 07:30:13PM +0800, Leo Yan escreveu:
> Hi Ali,
> 
> On Fri, Jul 21, 2023 at 06:16:53PM +0000, Ali Saidi wrote:
> 
> > > This series support Cortex-X4 CPU in Perf Arm SPE.
> 
> [...]
> 
> > This looks good to me, but can we add the other cores that operate the
> > same way now too? Flipping through the TRMs A78, X3, V2, X1, A715,
> > A720, and A78C all have the same encodings. 
> 
> Thanks a lot for exploring more CPU variants which share the same data
> source packet format.
> 
> The latest Linux kernel have defined the CPU part number and MIDR for
> below CPU variants:
> 
> - A78
> - X1
> - A715
> - A78C
> 
> I would like to use a patch to support these CPUs in perf tool.  Given
> other CPU variants (X3/V2/A720) have not been supported in the kernel,
> and so far no one requests them, I would like leave them out.
> 
> Please let me know if this okay for you or not.
> 
> 
> > Reviewed-by: Ali Saidi <alisaidi@amazon.com>
> 
> Thanks for review, I will add your review tags in the new patch set.

Ok, waiting for the new patch set.

- Arnaldo