2023-07-17 06:05:09

by Leo Yan

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

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



2023-07-17 06:24:45

by Leo Yan

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

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 <[email protected]>
---
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


2023-07-17 06:25:15

by Leo Yan

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

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

Signed-off-by: Leo Yan <[email protected]>
---
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


2023-07-21 18:52:18

by Ali Saidi

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

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 <[email protected]>

Thanks!
Ali




2023-07-24 07:35:14

by Anshuman Khandual

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



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 <[email protected]>
> ---
> 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);
>

2023-07-24 11:18:41

by Leo Yan

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

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

2023-07-24 11:47:41

by Leo Yan

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

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 <[email protected]>

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

Leo

2023-07-28 15:15:34

by Arnaldo Carvalho de Melo

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

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 <[email protected]>
>
> Thanks for review, I will add your review tags in the new patch set.

Ok, waiting for the new patch set.

- Arnaldo

2023-07-28 15:29:49

by Will Deacon

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

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