Re: perf usage of arch/arm64/include/asm/cputype.h

Re: perf usage of arch/arm64/include/asm/cputype.h

[Yicong Yang]

+ linux-arm-kernel

On 2025/6/14 4:13, Arnaldo Carvalho de Melo wrote:
> Hi,
> 
> tools/perf (and other tools/ living code) uses a file from the kernel, a
> copy, so that we don't break its build when something changes in the
> kernel that tooling uses.
> 
> There is this tools/perf/check-headers.sh that does the "copy coherency
> check", while trying to act on such a warning I stumbled on the report
> below.
> 
> More details at:
> 
>   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/include/uapi/README
> 
> 
> If you could please take a look at this that would be great, the initial
> copy was made at:
> 
> commit 1314376d495f2d79cc58753ff3034ccc503c43c9
> Author: Ali Saidi <alisaidi@amazon.com>
> Date:   Thu Mar 24 18:33:20 2022 +0000
> 
>     tools arm64: Import cputype.h
>     
>     Bring-in the kernel's arch/arm64/include/asm/cputype.h into tools/
>     for arm64 to make use of all the core-type definitions in perf.
>     
>     Replace sysreg.h with the version already imported into tools/.
>     
>     Committer notes:
>     
>     Added an entry to tools/perf/check-headers.sh, so that we get notified
>     when the original file in the kernel sources gets modified.
>     
>     Tester notes:
>     
>     LGTM. I did the testing on both my x86 and Arm64 platforms, thanks for
>     the fixing up.
>     
>     Signed-off-by: Ali Saidi <alisaidi@amazon.com>
>     Tested-by: Leo Yan <leo.yan@linaro.org>
> 
> - Arnaldo
> 
> ⬢ [acme@toolbx perf-tools]$ m
> rm: cannot remove '/home/acme/libexec/perf-core/scripts/python/Perf-Trace-Util/lib/Perf/Trace/__pycache__/Core.cpython-313.pyc': Permission denied
> make: Entering directory '/home/acme/git/perf-tools/tools/perf'
>   BUILD:   Doing 'make -j32' parallel build
> Warning: Kernel ABI header differences:
>   diff -u tools/arch/x86/include/asm/cpufeatures.h arch/x86/include/asm/cpufeatures.h
>   diff -u tools/arch/arm64/include/asm/cputype.h arch/arm64/include/asm/cputype.h
> 
> Auto-detecting system features:
> ...                                   libdw: [ on  ]
> ...                                   glibc: [ on  ]
> ...                                  libelf: [ on  ]
> ...                                 libnuma: [ on  ]
> ...                  numa_num_possible_cpus: [ on  ]
> ...                                 libperl: [ on  ]
> ...                               libpython: [ on  ]
> ...                               libcrypto: [ on  ]
> ...                             libcapstone: [ on  ]
> ...                               llvm-perf: [ on  ]
> ...                                    zlib: [ on  ]
> ...                                    lzma: [ on  ]
> ...                               get_cpuid: [ on  ]
> ...                                     bpf: [ on  ]
> ...                                  libaio: [ on  ]
> ...                                 libzstd: [ on  ]
> 
>   INSTALL libsubcmd_headers
>   INSTALL libperf_headers
>   INSTALL libapi_headers
>   INSTALL libsymbol_headers
>   INSTALL libbpf_headers
>   INSTALL binaries
>   INSTALL tests
>   INSTALL libperf-jvmti.so
>   INSTALL libexec
>   INSTALL perf-archive
>   INSTALL perf-iostat
>   INSTALL perl-scripts
>   INSTALL python-scripts
>   INSTALL dlfilters
>   INSTALL perf_completion-script
>   INSTALL perf-tip
> make: Leaving directory '/home/acme/git/perf-tools/tools/perf'
>  18: 'import perf' in python                                         : Ok
> ⬢ [acme@toolbx perf-tools]$ cp arch/arm64/include/asm/cputype.h tools/arch/arm64/include/asm/cputype.h
> ⬢ [acme@toolbx perf-tools]$ m
> rm: cannot remove '/home/acme/libexec/perf-core/scripts/python/Perf-Trace-Util/lib/Perf/Trace/__pycache__/Core.cpython-313.pyc': Permission denied
> make: Entering directory '/home/acme/git/perf-tools/tools/perf'
>   BUILD:   Doing 'make -j32' parallel build
> Warning: Kernel ABI header differences:
>   diff -u tools/arch/x86/include/asm/cpufeatures.h arch/x86/include/asm/cpufeatures.h
> 
> Auto-detecting system features:
> ...                                   libdw: [ on  ]
> ...                                   glibc: [ on  ]
> ...                                  libelf: [ on  ]
> ...                                 libnuma: [ on  ]
> ...                  numa_num_possible_cpus: [ on  ]
> ...                                 libperl: [ on  ]
> ...                               libpython: [ on  ]
> ...                               libcrypto: [ on  ]
> ...                             libcapstone: [ on  ]
> ...                               llvm-perf: [ on  ]
> ...                                    zlib: [ on  ]
> ...                                    lzma: [ on  ]
> ...                               get_cpuid: [ on  ]
> ...                                     bpf: [ on  ]
> ...                                  libaio: [ on  ]
> ...                                 libzstd: [ on  ]
> 
>   INSTALL libsubcmd_headers
>   INSTALL libperf_headers
>   INSTALL libapi_headers
>   INSTALL libsymbol_headers
>   INSTALL libbpf_headers
>   CC      /tmp/build/perf-tools/util/arm-spe.o
> util/arm-spe.c: In function ‘arm_spe__synth_ds’:
> util/arm-spe.c:885:43: error: passing argument 1 of ‘is_midr_in_range_list’ makes pointer from integer without a cast [-Wint-conversion]
>   885 |                 if (is_midr_in_range_list(midr, data_source_handles[i].midr_ranges)) {
>       |                                           ^~~~
>       |                                           |
>       |                                           u64 {aka long unsigned int}
> In file included from util/arm-spe.c:37:
> util/../../arch/arm64/include/asm/cputype.h:306:53: note: expected ‘const struct midr_range *’ but argument is of type ‘u64’ {aka ‘long unsigned int’}
>   306 | bool is_midr_in_range_list(struct midr_range const *ranges);
>       |                            ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~
> util/arm-spe.c:885:21: error: too many arguments to function ‘is_midr_in_range_list’; expected 1, have 2
>   885 |                 if (is_midr_in_range_list(midr, data_source_handles[i].midr_ranges)) {
>       |                     ^~~~~~~~~~~~~~~~~~~~~       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> util/../../arch/arm64/include/asm/cputype.h:306:6: note: declared here
>   306 | bool is_midr_in_range_list(struct midr_range const *ranges);
>       |      ^~~~~~~~~~~~~~~~~~~~~
> make[4]: *** [/home/acme/git/perf-tools/tools/build/Makefile.build:85: /tmp/build/perf-tools/util/arm-spe.o] Error 1
> make[3]: *** [/home/acme/git/perf-tools/tools/build/Makefile.build:142: util] Error 2
> make[2]: *** [Makefile.perf:798: /tmp/build/perf-tools/perf-util-in.o] Error 2
> make[1]: *** [Makefile.perf:290: sub-make] Error 2
> make: *** [Makefile:119: install-bin] Error 2
> make: Leaving directory '/home/acme/git/perf-tools/tools/perf'
> ⬢ [acme@toolbx perf-tools]$ 
> 
> 

The changes should be introduced by arm64's errata management on live migration[1], specifically:
- commit e3121298c7fc ("arm64: Modify _midr_range() functions to read MIDR/REVIDR internally")
  which changed the implementation of is_midr_in_range() that the MIDR to
  test is always read on the current CPU. This isn't true in perf since
  the MIDR is acquired from the perf.data.
- commit c8c2647e69be ("arm64: Make  _midr_in_range_list() an exported function")
  which moves the implementation out of the header file.

Below patch should keep the copy coherency of cputype.h to implement the _midr_in_range_list()
as before.

[1] https://lore.kernel.org/all/20250221140229.12588-1-shameerali.kolothum.thodi@huawei.com/

Thanks.

From 44900e7d3d9fa34c817396275f55a2aab611cd32 Mon Sep 17 00:00:00 2001
From: Yicong Yang <yangyicong@hisilicon.com>
Date: Mon, 16 Jun 2025 15:18:11 +0800
Subject: [PATCH] arm64: cputype: Allow copy coherency on cputype.h between
 tools/ and arch/
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

arch/arm64/include/asm/cputype.h is copied from arch/arm64 and used
by perf to parsing vendor specific SPE packets according to the MIDR.
The header diverge after errata management handling for VM live
migration merged [1]:
- commit e3121298c7fc ("arm64: Modify _midr_range() functions to read MIDR/REVIDR internally")
  which changed the implementation of is_midr_in_range() that the MIDR to
  test is always read on the current CPU. This isn't true in perf since
  the MIDR is acquired from the perf.data.
- commit c8c2647e69be ("arm64: Make  _midr_in_range_list() an exported function")
  which moves the implementation out of the header file.

In order to allow copy coherency on cputype.h [2], implement
is_midr_in_range_list() as before [1]. Introduce is_cpuid_in_range_list()
for kernel space to test the MIDR of current running CPU is within the
target MIDR ranges. Move cpu_errata_set_target_impl() and
is_cpuid_in_range_list() to cpufeature.h since they're only used by
errata management in the kernel space and don't needed by tools/.

No funtional changes intended.

[1] https://lore.kernel.org/all/20250221140229.12588-1-shameerali.kolothum.thodi@huawei.com/
[2] https://lore.kernel.org/lkml/aEyGg98z-MkcClXY@x1/#t
Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
 arch/arm64/include/asm/cpufeature.h           | 11 +++++
 arch/arm64/include/asm/cputype.h              | 40 +++++++++++--------
 arch/arm64/kernel/cpu_errata.c                | 30 +++++---------
 arch/arm64/kernel/cpufeature.c                |  6 +--
 arch/arm64/kernel/proton-pack.c               | 20 +++++-----
 arch/arm64/kvm/vgic/vgic-v3.c                 |  2 +-
 drivers/clocksource/arm_arch_timer.c          |  2 +-
 .../coresight/coresight-etm4x-core.c          |  2 +-
 8 files changed, 60 insertions(+), 53 deletions(-)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index c4326f1cb917..ba2d474fb393 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -1048,6 +1048,17 @@ static inline bool cpu_has_lpa2(void)
 #endif
 }

+struct target_impl_cpu {
+	u64 midr;
+	u64 revidr;
+	u64 aidr;
+};
+
+bool cpu_errata_set_target_impl(u64 num, void *impl_cpus);
+
+/* Different from is_midr_in_range() on using the MIDR of current CPU */
+bool is_cpuid_in_range_list(struct midr_range const *ranges);
+
 #endif /* __ASSEMBLY__ */

 #endif
diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
index 661735616787..89fd197e2f03 100644
--- a/arch/arm64/include/asm/cputype.h
+++ b/arch/arm64/include/asm/cputype.h
@@ -251,16 +251,6 @@

 #define read_cpuid(reg)			read_sysreg_s(SYS_ ## reg)

-/*
- * The CPU ID never changes at run time, so we might as well tell the
- * compiler that it's constant.  Use this function to read the CPU ID
- * rather than directly reading processor_id or read_cpuid() directly.
- */
-static inline u32 __attribute_const__ read_cpuid_id(void)
-{
-	return read_cpuid(MIDR_EL1);
-}
-
 /*
  * Represent a range of MIDR values for a given CPU model and a
  * range of variant/revision values.
@@ -296,14 +286,30 @@ static inline bool midr_is_cpu_model_range(u32 midr, u32 model, u32 rv_min,
 	return _model == model && rv >= rv_min && rv <= rv_max;
 }

-struct target_impl_cpu {
-	u64 midr;
-	u64 revidr;
-	u64 aidr;
-};
+static inline bool is_midr_in_range(u32 midr, struct midr_range const *range)
+{
+	return midr_is_cpu_model_range(midr, range->model,
+				       range->rv_min, range->rv_max);
+}

-bool cpu_errata_set_target_impl(u64 num, void *impl_cpus);
-bool is_midr_in_range_list(struct midr_range const *ranges);
+static inline bool
+is_midr_in_range_list(u32 midr, struct midr_range const *ranges)
+{
+	while (ranges->model)
+		if (is_midr_in_range(midr, ranges++))
+			return true;
+	return false;
+}
+
+/*
+ * The CPU ID never changes at run time, so we might as well tell the
+ * compiler that it's constant.  Use this function to read the CPU ID
+ * rather than directly reading processor_id or read_cpuid() directly.
+ */
+static inline u32 __attribute_const__ read_cpuid_id(void)
+{
+	return read_cpuid(MIDR_EL1);
+}

 static inline u64 __attribute_const__ read_cpuid_mpidr(void)
 {
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 59d723c9ab8f..531ae67c7086 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -27,38 +27,28 @@ bool cpu_errata_set_target_impl(u64 num, void *impl_cpus)
 	return true;
 }

-static inline bool is_midr_in_range(struct midr_range const *range)
+bool is_cpuid_in_range_list(struct midr_range const *ranges)
 {
+	u32 midr = read_cpuid_id();
 	int i;

 	if (!target_impl_cpu_num)
-		return midr_is_cpu_model_range(read_cpuid_id(), range->model,
-					       range->rv_min, range->rv_max);
+		return is_midr_in_range_list(midr, ranges);

-	for (i = 0; i < target_impl_cpu_num; i++) {
-		if (midr_is_cpu_model_range(target_impl_cpus[i].midr,
-					    range->model,
-					    range->rv_min, range->rv_max))
+	for (i = 0; i < target_impl_cpu_num; i++)
+		if (is_midr_in_range_list(midr, ranges))
 			return true;
-	}
-	return false;
-}

-bool is_midr_in_range_list(struct midr_range const *ranges)
-{
-	while (ranges->model)
-		if (is_midr_in_range(ranges++))
-			return true;
 	return false;
 }
-EXPORT_SYMBOL_GPL(is_midr_in_range_list);
+EXPORT_SYMBOL_GPL(is_cpuid_in_range_list);

 static bool __maybe_unused
 __is_affected_midr_range(const struct arm64_cpu_capabilities *entry,
 			 u32 midr, u32 revidr)
 {
 	const struct arm64_midr_revidr *fix;
-	if (!is_midr_in_range(&entry->midr_range))
+	if (!is_midr_in_range(midr, &entry->midr_range))
 		return false;

 	midr &= MIDR_REVISION_MASK | MIDR_VARIANT_MASK;
@@ -92,7 +82,7 @@ is_affected_midr_range_list(const struct arm64_cpu_capabilities *entry,
 			    int scope)
 {
 	WARN_ON(scope != SCOPE_LOCAL_CPU || preemptible());
-	return is_midr_in_range_list(entry->midr_range_list);
+	return is_cpuid_in_range_list(entry->midr_range_list);
 }

 static bool __maybe_unused
@@ -244,7 +234,7 @@ has_neoverse_n1_erratum_1542419(const struct arm64_cpu_capabilities *entry,
 	const struct midr_range range = MIDR_ALL_VERSIONS(MIDR_NEOVERSE_N1);

 	WARN_ON(scope != SCOPE_LOCAL_CPU || preemptible());
-	return is_midr_in_range(&range) && has_dic;
+	return is_cpuid_in_range_list(&range) && has_dic;
 }

 static const struct midr_range impdef_pmuv3_cpus[] = {
@@ -276,7 +266,7 @@ static bool has_impdef_pmuv3(const struct arm64_cpu_capabilities *entry, int sco
 	if (pmuver != ID_AA64DFR0_EL1_PMUVer_IMP_DEF)
 		return false;

-	return is_midr_in_range_list(impdef_pmuv3_cpus);
+	return is_cpuid_in_range_list(impdef_pmuv3_cpus);
 }

 static void cpu_enable_impdef_pmuv3_traps(const struct arm64_cpu_capabilities *__unused)
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index b34044e20128..e89bed0e7358 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1807,7 +1807,7 @@ static bool unmap_kernel_at_el0(const struct arm64_cpu_capabilities *entry,
 	char const *str = "kpti command line option";
 	bool meltdown_safe;

-	meltdown_safe = is_midr_in_range_list(kpti_safe_list);
+	meltdown_safe = is_cpuid_in_range_list(kpti_safe_list);

 	/* Defer to CPU feature registers */
 	if (has_cpuid_feature(entry, scope))
@@ -1877,7 +1877,7 @@ static bool has_nv1(const struct arm64_cpu_capabilities *entry, int scope)

 	return (__system_matches_cap(ARM64_HAS_NESTED_VIRT) &&
 		!(has_cpuid_feature(entry, scope) ||
-		  is_midr_in_range_list(nv1_ni_list)));
+		  is_cpuid_in_range_list(nv1_ni_list)));
 }

 #if defined(ID_AA64MMFR0_EL1_TGRAN_LPA2) && defined(ID_AA64MMFR0_EL1_TGRAN_2_SUPPORTED_LPA2)
@@ -2082,7 +2082,7 @@ static bool cpu_has_broken_dbm(void)
 		{},
 	};

-	return is_midr_in_range_list(cpus);
+	return is_cpuid_in_range_list(cpus);
 }

 static bool cpu_can_use_dbm(const struct arm64_cpu_capabilities *cap)
diff --git a/arch/arm64/kernel/proton-pack.c b/arch/arm64/kernel/proton-pack.c
index edf1783ffc81..144441ad2aed 100644
--- a/arch/arm64/kernel/proton-pack.c
+++ b/arch/arm64/kernel/proton-pack.c
@@ -172,7 +172,7 @@ static enum mitigation_state spectre_v2_get_cpu_hw_mitigation_state(void)
 		return SPECTRE_UNAFFECTED;

 	/* Alternatively, we have a list of unaffected CPUs */
-	if (is_midr_in_range_list(spectre_v2_safe_list))
+	if (is_cpuid_in_range_list(spectre_v2_safe_list))
 		return SPECTRE_UNAFFECTED;

 	return SPECTRE_VULNERABLE;
@@ -331,7 +331,7 @@ bool has_spectre_v3a(const struct arm64_cpu_capabilities *entry, int scope)
 	};

 	WARN_ON(scope != SCOPE_LOCAL_CPU || preemptible());
-	return is_midr_in_range_list(spectre_v3a_unsafe_list);
+	return is_cpuid_in_range_list(spectre_v3a_unsafe_list);
 }

 void spectre_v3a_enable_mitigation(const struct arm64_cpu_capabilities *__unused)
@@ -475,7 +475,7 @@ static enum mitigation_state spectre_v4_get_cpu_hw_mitigation_state(void)
 		{ /* sentinel */ },
 	};

-	if (is_midr_in_range_list(spectre_v4_safe_list))
+	if (is_cpuid_in_range_list(spectre_v4_safe_list))
 		return SPECTRE_UNAFFECTED;

 	/* CPU features are detected first */
@@ -864,7 +864,7 @@ static bool is_spectre_bhb_safe(int scope)
 	if (scope != SCOPE_LOCAL_CPU)
 		return all_safe;

-	if (is_midr_in_range_list(spectre_bhb_safe_list))
+	if (is_cpuid_in_range_list(spectre_bhb_safe_list))
 		return true;

 	all_safe = false;
@@ -917,17 +917,17 @@ static u8 spectre_bhb_loop_affected(void)
 		{},
 	};

-	if (is_midr_in_range_list(spectre_bhb_k132_list))
+	if (is_cpuid_in_range_list(spectre_bhb_k132_list))
 		k = 132;
-	else if (is_midr_in_range_list(spectre_bhb_k38_list))
+	else if (is_cpuid_in_range_list(spectre_bhb_k38_list))
 		k = 38;
-	else if (is_midr_in_range_list(spectre_bhb_k32_list))
+	else if (is_cpuid_in_range_list(spectre_bhb_k32_list))
 		k = 32;
-	else if (is_midr_in_range_list(spectre_bhb_k24_list))
+	else if (is_cpuid_in_range_list(spectre_bhb_k24_list))
 		k = 24;
-	else if (is_midr_in_range_list(spectre_bhb_k11_list))
+	else if (is_cpuid_in_range_list(spectre_bhb_k11_list))
 		k = 11;
-	else if (is_midr_in_range_list(spectre_bhb_k8_list))
+	else if (is_cpuid_in_range_list(spectre_bhb_k8_list))
 		k =  8;

 	return k;
diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
index b9ad7c42c5b0..852f8d769244 100644
--- a/arch/arm64/kvm/vgic/vgic-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-v3.c
@@ -632,7 +632,7 @@ static const struct midr_range broken_seis[] = {
 static bool vgic_v3_broken_seis(void)
 {
 	return ((kvm_vgic_global_state.ich_vtr_el2 & ICH_VTR_EL2_SEIS) &&
-		is_midr_in_range_list(broken_seis));
+		is_cpuid_in_range_list(broken_seis));
 }

 /**
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 981a578043a5..c881ceb76ec2 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -842,7 +842,7 @@ static u64 __arch_timer_check_delta(void)
 		{},
 	};

-	if (is_midr_in_range_list(broken_cval_midrs)) {
+	if (is_cpuid_in_range_list(broken_cval_midrs)) {
 		pr_warn_once("Broken CNTx_CVAL_EL1, using 31 bit TVAL instead.\n");
 		return CLOCKSOURCE_MASK(31);
 	}
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 42e5d37403ad..2ea5ca6708d2 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1280,7 +1280,7 @@ static void etm4_fixup_wrong_ccitmin(struct etmv4_drvdata *drvdata)
 	 * recorded value for 'drvdata->ccitmin' to workaround
 	 * this problem.
 	 */
-	if (is_midr_in_range_list(etm_wrong_ccitmin_cpus)) {
+	if (is_cpuid_in_range_list(etm_wrong_ccitmin_cpus)) {
 		if (drvdata->ccitmin == 256)
 			drvdata->ccitmin = 4;
 	}
-- 
2.24.0

Re: perf usage of arch/arm64/include/asm/cputype.h

[James Clark]

On 16/06/2025 8:56 am, Yicong Yang wrote:
> + linux-arm-kernel
> 
> On 2025/6/14 4:13, Arnaldo Carvalho de Melo wrote:
>> Hi,
>>
>> tools/perf (and other tools/ living code) uses a file from the kernel, a
>> copy, so that we don't break its build when something changes in the
>> kernel that tooling uses.
>>
>> There is this tools/perf/check-headers.sh that does the "copy coherency
>> check", while trying to act on such a warning I stumbled on the report
>> below.
>>
>> More details at:
>>
>>    https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/include/uapi/README
>>
>>
>> If you could please take a look at this that would be great, the initial
>> copy was made at:
>>
>> commit 1314376d495f2d79cc58753ff3034ccc503c43c9
>> Author: Ali Saidi <alisaidi@amazon.com>
>> Date:   Thu Mar 24 18:33:20 2022 +0000
>>
>>      tools arm64: Import cputype.h
>>      
>>      Bring-in the kernel's arch/arm64/include/asm/cputype.h into tools/
>>      for arm64 to make use of all the core-type definitions in perf.
>>      
>>      Replace sysreg.h with the version already imported into tools/.
>>      
>>      Committer notes:
>>      
>>      Added an entry to tools/perf/check-headers.sh, so that we get notified
>>      when the original file in the kernel sources gets modified.
>>      
>>      Tester notes:
>>      
>>      LGTM. I did the testing on both my x86 and Arm64 platforms, thanks for
>>      the fixing up.
>>      
>>      Signed-off-by: Ali Saidi <alisaidi@amazon.com>
>>      Tested-by: Leo Yan <leo.yan@linaro.org>
>>
>> - Arnaldo
>>
>> ⬢ [acme@toolbx perf-tools]$ m
>> rm: cannot remove '/home/acme/libexec/perf-core/scripts/python/Perf-Trace-Util/lib/Perf/Trace/__pycache__/Core.cpython-313.pyc': Permission denied
>> make: Entering directory '/home/acme/git/perf-tools/tools/perf'
>>    BUILD:   Doing 'make -j32' parallel build
>> Warning: Kernel ABI header differences:
>>    diff -u tools/arch/x86/include/asm/cpufeatures.h arch/x86/include/asm/cpufeatures.h
>>    diff -u tools/arch/arm64/include/asm/cputype.h arch/arm64/include/asm/cputype.h
>>
>> Auto-detecting system features:
>> ...                                   libdw: [ on  ]
>> ...                                   glibc: [ on  ]
>> ...                                  libelf: [ on  ]
>> ...                                 libnuma: [ on  ]
>> ...                  numa_num_possible_cpus: [ on  ]
>> ...                                 libperl: [ on  ]
>> ...                               libpython: [ on  ]
>> ...                               libcrypto: [ on  ]
>> ...                             libcapstone: [ on  ]
>> ...                               llvm-perf: [ on  ]
>> ...                                    zlib: [ on  ]
>> ...                                    lzma: [ on  ]
>> ...                               get_cpuid: [ on  ]
>> ...                                     bpf: [ on  ]
>> ...                                  libaio: [ on  ]
>> ...                                 libzstd: [ on  ]
>>
>>    INSTALL libsubcmd_headers
>>    INSTALL libperf_headers
>>    INSTALL libapi_headers
>>    INSTALL libsymbol_headers
>>    INSTALL libbpf_headers
>>    INSTALL binaries
>>    INSTALL tests
>>    INSTALL libperf-jvmti.so
>>    INSTALL libexec
>>    INSTALL perf-archive
>>    INSTALL perf-iostat
>>    INSTALL perl-scripts
>>    INSTALL python-scripts
>>    INSTALL dlfilters
>>    INSTALL perf_completion-script
>>    INSTALL perf-tip
>> make: Leaving directory '/home/acme/git/perf-tools/tools/perf'
>>   18: 'import perf' in python                                         : Ok
>> ⬢ [acme@toolbx perf-tools]$ cp arch/arm64/include/asm/cputype.h tools/arch/arm64/include/asm/cputype.h
>> ⬢ [acme@toolbx perf-tools]$ m
>> rm: cannot remove '/home/acme/libexec/perf-core/scripts/python/Perf-Trace-Util/lib/Perf/Trace/__pycache__/Core.cpython-313.pyc': Permission denied
>> make: Entering directory '/home/acme/git/perf-tools/tools/perf'
>>    BUILD:   Doing 'make -j32' parallel build
>> Warning: Kernel ABI header differences:
>>    diff -u tools/arch/x86/include/asm/cpufeatures.h arch/x86/include/asm/cpufeatures.h
>>
>> Auto-detecting system features:
>> ...                                   libdw: [ on  ]
>> ...                                   glibc: [ on  ]
>> ...                                  libelf: [ on  ]
>> ...                                 libnuma: [ on  ]
>> ...                  numa_num_possible_cpus: [ on  ]
>> ...                                 libperl: [ on  ]
>> ...                               libpython: [ on  ]
>> ...                               libcrypto: [ on  ]
>> ...                             libcapstone: [ on  ]
>> ...                               llvm-perf: [ on  ]
>> ...                                    zlib: [ on  ]
>> ...                                    lzma: [ on  ]
>> ...                               get_cpuid: [ on  ]
>> ...                                     bpf: [ on  ]
>> ...                                  libaio: [ on  ]
>> ...                                 libzstd: [ on  ]
>>
>>    INSTALL libsubcmd_headers
>>    INSTALL libperf_headers
>>    INSTALL libapi_headers
>>    INSTALL libsymbol_headers
>>    INSTALL libbpf_headers
>>    CC      /tmp/build/perf-tools/util/arm-spe.o
>> util/arm-spe.c: In function ‘arm_spe__synth_ds’:
>> util/arm-spe.c:885:43: error: passing argument 1 of ‘is_midr_in_range_list’ makes pointer from integer without a cast [-Wint-conversion]
>>    885 |                 if (is_midr_in_range_list(midr, data_source_handles[i].midr_ranges)) {
>>        |                                           ^~~~
>>        |                                           |
>>        |                                           u64 {aka long unsigned int}
>> In file included from util/arm-spe.c:37:
>> util/../../arch/arm64/include/asm/cputype.h:306:53: note: expected ‘const struct midr_range *’ but argument is of type ‘u64’ {aka ‘long unsigned int’}
>>    306 | bool is_midr_in_range_list(struct midr_range const *ranges);
>>        |                            ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~
>> util/arm-spe.c:885:21: error: too many arguments to function ‘is_midr_in_range_list’; expected 1, have 2
>>    885 |                 if (is_midr_in_range_list(midr, data_source_handles[i].midr_ranges)) {
>>        |                     ^~~~~~~~~~~~~~~~~~~~~       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> util/../../arch/arm64/include/asm/cputype.h:306:6: note: declared here
>>    306 | bool is_midr_in_range_list(struct midr_range const *ranges);
>>        |      ^~~~~~~~~~~~~~~~~~~~~
>> make[4]: *** [/home/acme/git/perf-tools/tools/build/Makefile.build:85: /tmp/build/perf-tools/util/arm-spe.o] Error 1
>> make[3]: *** [/home/acme/git/perf-tools/tools/build/Makefile.build:142: util] Error 2
>> make[2]: *** [Makefile.perf:798: /tmp/build/perf-tools/perf-util-in.o] Error 2
>> make[1]: *** [Makefile.perf:290: sub-make] Error 2
>> make: *** [Makefile:119: install-bin] Error 2
>> make: Leaving directory '/home/acme/git/perf-tools/tools/perf'
>> ⬢ [acme@toolbx perf-tools]$
>>
>>
> 
> The changes should be introduced by arm64's errata management on live migration[1], specifically:
> - commit e3121298c7fc ("arm64: Modify _midr_range() functions to read MIDR/REVIDR internally")
>    which changed the implementation of is_midr_in_range() that the MIDR to
>    test is always read on the current CPU. This isn't true in perf since
>    the MIDR is acquired from the perf.data.
> - commit c8c2647e69be ("arm64: Make  _midr_in_range_list() an exported function")
>    which moves the implementation out of the header file.
> 
> Below patch should keep the copy coherency of cputype.h to implement the _midr_in_range_list()
> as before.
> 
> [1] https://lore.kernel.org/all/20250221140229.12588-1-shameerali.kolothum.thodi@huawei.com/
> 
> Thanks.
> 
>  From 44900e7d3d9fa34c817396275f55a2aab611cd32 Mon Sep 17 00:00:00 2001
> From: Yicong Yang <yangyicong@hisilicon.com>
> Date: Mon, 16 Jun 2025 15:18:11 +0800
> Subject: [PATCH] arm64: cputype: Allow copy coherency on cputype.h between
>   tools/ and arch/
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> arch/arm64/include/asm/cputype.h is copied from arch/arm64 and used
> by perf to parsing vendor specific SPE packets according to the MIDR.
> The header diverge after errata management handling for VM live
> migration merged [1]:
> - commit e3121298c7fc ("arm64: Modify _midr_range() functions to read MIDR/REVIDR internally")
>    which changed the implementation of is_midr_in_range() that the MIDR to
>    test is always read on the current CPU. This isn't true in perf since
>    the MIDR is acquired from the perf.data.
> - commit c8c2647e69be ("arm64: Make  _midr_in_range_list() an exported function")
>    which moves the implementation out of the header file.
> 
> In order to allow copy coherency on cputype.h [2], implement
> is_midr_in_range_list() as before [1]. Introduce is_cpuid_in_range_list()
> for kernel space to test the MIDR of current running CPU is within the
> target MIDR ranges. Move cpu_errata_set_target_impl() and
> is_cpuid_in_range_list() to cpufeature.h since they're only used by
> errata management in the kernel space and don't needed by tools/.
> 
> No funtional changes intended.
> 
> [1] https://lore.kernel.org/all/20250221140229.12588-1-shameerali.kolothum.thodi@huawei.com/
> [2] https://lore.kernel.org/lkml/aEyGg98z-MkcClXY@x1/#t
> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> ---
>   arch/arm64/include/asm/cpufeature.h           | 11 +++++
>   arch/arm64/include/asm/cputype.h              | 40 +++++++++++--------
>   arch/arm64/kernel/cpu_errata.c                | 30 +++++---------
>   arch/arm64/kernel/cpufeature.c                |  6 +--
>   arch/arm64/kernel/proton-pack.c               | 20 +++++-----
>   arch/arm64/kvm/vgic/vgic-v3.c                 |  2 +-
>   drivers/clocksource/arm_arch_timer.c          |  2 +-
>   .../coresight/coresight-etm4x-core.c          |  2 +-
>   8 files changed, 60 insertions(+), 53 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index c4326f1cb917..ba2d474fb393 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -1048,6 +1048,17 @@ static inline bool cpu_has_lpa2(void)
>   #endif
>   }
> 
> +struct target_impl_cpu {
> +	u64 midr;
> +	u64 revidr;
> +	u64 aidr;
> +};
> +
> +bool cpu_errata_set_target_impl(u64 num, void *impl_cpus);
> +
> +/* Different from is_midr_in_range() on using the MIDR of current CPU */
> +bool is_cpuid_in_range_list(struct midr_range const *ranges);
> +
>   #endif /* __ASSEMBLY__ */
> 
>   #endif
> diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
> index 661735616787..89fd197e2f03 100644
> --- a/arch/arm64/include/asm/cputype.h
> +++ b/arch/arm64/include/asm/cputype.h
> @@ -251,16 +251,6 @@
> 
>   #define read_cpuid(reg)			read_sysreg_s(SYS_ ## reg)
> 
> -/*
> - * The CPU ID never changes at run time, so we might as well tell the
> - * compiler that it's constant.  Use this function to read the CPU ID
> - * rather than directly reading processor_id or read_cpuid() directly.
> - */
> -static inline u32 __attribute_const__ read_cpuid_id(void)
> -{
> -	return read_cpuid(MIDR_EL1);
> -}
> -
>   /*
>    * Represent a range of MIDR values for a given CPU model and a
>    * range of variant/revision values.
> @@ -296,14 +286,30 @@ static inline bool midr_is_cpu_model_range(u32 midr, u32 model, u32 rv_min,
>   	return _model == model && rv >= rv_min && rv <= rv_max;
>   }
> 
> -struct target_impl_cpu {
> -	u64 midr;
> -	u64 revidr;
> -	u64 aidr;
> -};
> +static inline bool is_midr_in_range(u32 midr, struct midr_range const *range)
> +{
> +	return midr_is_cpu_model_range(midr, range->model,
> +				       range->rv_min, range->rv_max);
> +}
> 
> -bool cpu_errata_set_target_impl(u64 num, void *impl_cpus);
> -bool is_midr_in_range_list(struct midr_range const *ranges);
> +static inline bool
> +is_midr_in_range_list(u32 midr, struct midr_range const *ranges)
> +{
> +	while (ranges->model)
> +		if (is_midr_in_range(midr, ranges++))
> +			return true;
> +	return false;
> +}
> +
> +/*
> + * The CPU ID never changes at run time, so we might as well tell the
> + * compiler that it's constant.  Use this function to read the CPU ID
> + * rather than directly reading processor_id or read_cpuid() directly.
> + */
> +static inline u32 __attribute_const__ read_cpuid_id(void)
> +{
> +	return read_cpuid(MIDR_EL1);
> +}
> 
>   static inline u64 __attribute_const__ read_cpuid_mpidr(void)
>   {
> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> index 59d723c9ab8f..531ae67c7086 100644
> --- a/arch/arm64/kernel/cpu_errata.c
> +++ b/arch/arm64/kernel/cpu_errata.c
> @@ -27,38 +27,28 @@ bool cpu_errata_set_target_impl(u64 num, void *impl_cpus)
>   	return true;
>   }
> 
> -static inline bool is_midr_in_range(struct midr_range const *range)
> +bool is_cpuid_in_range_list(struct midr_range const *ranges)
>   {
> +	u32 midr = read_cpuid_id();
>   	int i;
> 
>   	if (!target_impl_cpu_num)
> -		return midr_is_cpu_model_range(read_cpuid_id(), range->model,
> -					       range->rv_min, range->rv_max);
> +		return is_midr_in_range_list(midr, ranges);
> 
> -	for (i = 0; i < target_impl_cpu_num; i++) {
> -		if (midr_is_cpu_model_range(target_impl_cpus[i].midr,
> -					    range->model,
> -					    range->rv_min, range->rv_max))
> +	for (i = 0; i < target_impl_cpu_num; i++)
> +		if (is_midr_in_range_list(midr, ranges))
>   			return true;
> -	}
> -	return false;
> -}
> 
> -bool is_midr_in_range_list(struct midr_range const *ranges)
> -{
> -	while (ranges->model)
> -		if (is_midr_in_range(ranges++))
> -			return true;
>   	return false;
>   }
> -EXPORT_SYMBOL_GPL(is_midr_in_range_list);
> +EXPORT_SYMBOL_GPL(is_cpuid_in_range_list);
> 
>   static bool __maybe_unused
>   __is_affected_midr_range(const struct arm64_cpu_capabilities *entry,
>   			 u32 midr, u32 revidr)
>   {
>   	const struct arm64_midr_revidr *fix;
> -	if (!is_midr_in_range(&entry->midr_range))
> +	if (!is_midr_in_range(midr, &entry->midr_range))
>   		return false;
> 
>   	midr &= MIDR_REVISION_MASK | MIDR_VARIANT_MASK;
> @@ -92,7 +82,7 @@ is_affected_midr_range_list(const struct arm64_cpu_capabilities *entry,
>   			    int scope)
>   {
>   	WARN_ON(scope != SCOPE_LOCAL_CPU || preemptible());
> -	return is_midr_in_range_list(entry->midr_range_list);
> +	return is_cpuid_in_range_list(entry->midr_range_list);

Looks ok to me.

You could do it with slightly less churn on the kernel side if you keep 
the function name and arguments the same there. There's only one usage 
in Perf so that one could be renamed and have the midr argument added 
back in.

I was also wondering if we could just diverge on the tools side, but in 
reality it also has to stay compatible with the definitions of all the 
MIDRs so might as well keep the whole thing identical.

RE: perf usage of arch/arm64/include/asm/cputype.h

[Shameerali Kolothum Thodi]

> -----Original Message-----
> From: James Clark <james.clark@linaro.org>
> Sent: Monday, June 16, 2025 10:30 AM
> To: yangyicong <yangyicong@huawei.com>; Arnaldo Carvalho de Melo
> <acme@kernel.org>; linux-arm-kernel@lists.infradead.org
> Cc: Ali Saidi <alisaidi@amazon.com>; Leo Yan <leo.yan@linaro.org>; Will
> Deacon <will@kernel.org>; James Morse <james.morse@arm.com>; Catalin
> Marinas <catalin.marinas@arm.com>; yangjinqian
> <yangjinqian1@huawei.com>; Douglas Anderson
> <dianders@chromium.org>; Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org>; Adrian Hunter <adrian.hunter@intel.com>;
> Ian Rogers <irogers@google.com>; Jiri Olsa <jolsa@kernel.org>; Kan Liang
> <kan.liang@linux.intel.com>; Namhyung Kim <namhyung@kernel.org>;
> Linux Kernel Mailing List <linux-kernel@vger.kernel.org>; Shameerali
> Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>; yangyicong
> <yangyicong@huawei.com>
> Subject: Re: perf usage of arch/arm64/include/asm/cputype.h
> 
> 
> 
> On 16/06/2025 8:56 am, Yicong Yang wrote:
> > + linux-arm-kernel
> >
> > On 2025/6/14 4:13, Arnaldo Carvalho de Melo wrote:
> >> Hi,
> >>
> >> tools/perf (and other tools/ living code) uses a file from the
> >> kernel, a copy, so that we don't break its build when something
> >> changes in the kernel that tooling uses.
> >>
> >> There is this tools/perf/check-headers.sh that does the "copy
> >> coherency check", while trying to act on such a warning I stumbled on
> >> the report below.
> >>
> >> More details at:
> >>
> >>
> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tr
> >> ee/tools/include/uapi/README
> >>
> >>
> >> If you could please take a look at this that would be great, the
> >> initial copy was made at:
> >>
> >> commit 1314376d495f2d79cc58753ff3034ccc503c43c9
> >> Author: Ali Saidi <alisaidi@amazon.com>
> >> Date:   Thu Mar 24 18:33:20 2022 +0000
> >>
> >>      tools arm64: Import cputype.h
> >>
> >>      Bring-in the kernel's arch/arm64/include/asm/cputype.h into tools/
> >>      for arm64 to make use of all the core-type definitions in perf.
> >>
> >>      Replace sysreg.h with the version already imported into tools/.
> >>
> >>      Committer notes:
> >>
> >>      Added an entry to tools/perf/check-headers.sh, so that we get
> notified
> >>      when the original file in the kernel sources gets modified.
> >>
> >>      Tester notes:
> >>
> >>      LGTM. I did the testing on both my x86 and Arm64 platforms, thanks
> for
> >>      the fixing up.
> >>
> >>      Signed-off-by: Ali Saidi <alisaidi@amazon.com>
> >>      Tested-by: Leo Yan <leo.yan@linaro.org>
> >>
> >> - Arnaldo
> >>
> >> ⬢ [acme@toolbx perf-tools]$ m
> >> rm: cannot remove
> >> '/home/acme/libexec/perf-core/scripts/python/Perf-Trace-Util/lib/Perf
> >> /Trace/__pycache__/Core.cpython-313.pyc': Permission denied
> >> make: Entering directory '/home/acme/git/perf-tools/tools/perf'
> >>    BUILD:   Doing 'make -j32' parallel build
> >> Warning: Kernel ABI header differences:
> >>    diff -u tools/arch/x86/include/asm/cpufeatures.h
> arch/x86/include/asm/cpufeatures.h
> >>    diff -u tools/arch/arm64/include/asm/cputype.h
> >> arch/arm64/include/asm/cputype.h
> >>
> >> Auto-detecting system features:
> >> ...                                   libdw: [ on  ]
> >> ...                                   glibc: [ on  ]
> >> ...                                  libelf: [ on  ]
> >> ...                                 libnuma: [ on  ]
> >> ...                  numa_num_possible_cpus: [ on  ]
> >> ...                                 libperl: [ on  ]
> >> ...                               libpython: [ on  ]
> >> ...                               libcrypto: [ on  ]
> >> ...                             libcapstone: [ on  ]
> >> ...                               llvm-perf: [ on  ]
> >> ...                                    zlib: [ on  ]
> >> ...                                    lzma: [ on  ]
> >> ...                               get_cpuid: [ on  ]
> >> ...                                     bpf: [ on  ]
> >> ...                                  libaio: [ on  ]
> >> ...                                 libzstd: [ on  ]
> >>
> >>    INSTALL libsubcmd_headers
> >>    INSTALL libperf_headers
> >>    INSTALL libapi_headers
> >>    INSTALL libsymbol_headers
> >>    INSTALL libbpf_headers
> >>    INSTALL binaries
> >>    INSTALL tests
> >>    INSTALL libperf-jvmti.so
> >>    INSTALL libexec
> >>    INSTALL perf-archive
> >>    INSTALL perf-iostat
> >>    INSTALL perl-scripts
> >>    INSTALL python-scripts
> >>    INSTALL dlfilters
> >>    INSTALL perf_completion-script
> >>    INSTALL perf-tip
> >> make: Leaving directory '/home/acme/git/perf-tools/tools/perf'
> >>   18: 'import perf' in python                                         : Ok
> >> ⬢ [acme@toolbx perf-tools]$ cp arch/arm64/include/asm/cputype.h
> >> tools/arch/arm64/include/asm/cputype.h
> >> ⬢ [acme@toolbx perf-tools]$ m
> >> rm: cannot remove
> >> '/home/acme/libexec/perf-core/scripts/python/Perf-Trace-Util/lib/Perf
> >> /Trace/__pycache__/Core.cpython-313.pyc': Permission denied
> >> make: Entering directory '/home/acme/git/perf-tools/tools/perf'
> >>    BUILD:   Doing 'make -j32' parallel build
> >> Warning: Kernel ABI header differences:
> >>    diff -u tools/arch/x86/include/asm/cpufeatures.h
> >> arch/x86/include/asm/cpufeatures.h
> >>
> >> Auto-detecting system features:
> >> ...                                   libdw: [ on  ]
> >> ...                                   glibc: [ on  ]
> >> ...                                  libelf: [ on  ]
> >> ...                                 libnuma: [ on  ]
> >> ...                  numa_num_possible_cpus: [ on  ]
> >> ...                                 libperl: [ on  ]
> >> ...                               libpython: [ on  ]
> >> ...                               libcrypto: [ on  ]
> >> ...                             libcapstone: [ on  ]
> >> ...                               llvm-perf: [ on  ]
> >> ...                                    zlib: [ on  ]
> >> ...                                    lzma: [ on  ]
> >> ...                               get_cpuid: [ on  ]
> >> ...                                     bpf: [ on  ]
> >> ...                                  libaio: [ on  ]
> >> ...                                 libzstd: [ on  ]
> >>
> >>    INSTALL libsubcmd_headers
> >>    INSTALL libperf_headers
> >>    INSTALL libapi_headers
> >>    INSTALL libsymbol_headers
> >>    INSTALL libbpf_headers
> >>    CC      /tmp/build/perf-tools/util/arm-spe.o
> >> util/arm-spe.c: In function ‘arm_spe__synth_ds’:
> >> util/arm-spe.c:885:43: error: passing argument 1 of
> ‘is_midr_in_range_list’ makes pointer from integer without a cast [-Wint-
> conversion]
> >>    885 |                 if (is_midr_in_range_list(midr,
> data_source_handles[i].midr_ranges)) {
> >>        |                                           ^~~~
> >>        |                                           |
> >>        |                                           u64 {aka long unsigned int}
> >> In file included from util/arm-spe.c:37:
> >> util/../../arch/arm64/include/asm/cputype.h:306:53: note: expected
> ‘const struct midr_range *’ but argument is of type ‘u64’ {aka ‘long
> unsigned int’}
> >>    306 | bool is_midr_in_range_list(struct midr_range const *ranges);
> >>        |                            ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~
> >> util/arm-spe.c:885:21: error: too many arguments to function
> ‘is_midr_in_range_list’; expected 1, have 2
> >>    885 |                 if (is_midr_in_range_list(midr,
> data_source_handles[i].midr_ranges)) {
> >>        |                     ^~~~~~~~~~~~~~~~~~~~~
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> util/../../arch/arm64/include/asm/cputype.h:306:6: note: declared here
> >>    306 | bool is_midr_in_range_list(struct midr_range const *ranges);
> >>        |      ^~~~~~~~~~~~~~~~~~~~~
> >> make[4]: ***
> >> [/home/acme/git/perf-tools/tools/build/Makefile.build:85:
> >> /tmp/build/perf-tools/util/arm-spe.o] Error 1
> >> make[3]: ***
> >> [/home/acme/git/perf-tools/tools/build/Makefile.build:142: util]
> >> Error 2
> >> make[2]: *** [Makefile.perf:798:
> >> /tmp/build/perf-tools/perf-util-in.o] Error 2
> >> make[1]: *** [Makefile.perf:290: sub-make] Error 2
> >> make: *** [Makefile:119: install-bin] Error 2
> >> make: Leaving directory '/home/acme/git/perf-tools/tools/perf'
> >> ⬢ [acme@toolbx perf-tools]$
> >>
> >>
> >
> > The changes should be introduced by arm64's errata management on live
> migration[1], specifically:
> > - commit e3121298c7fc ("arm64: Modify _midr_range() functions to read
> MIDR/REVIDR internally")
> >    which changed the implementation of is_midr_in_range() that the MIDR
> to
> >    test is always read on the current CPU. This isn't true in perf since
> >    the MIDR is acquired from the perf.data.
> > - commit c8c2647e69be ("arm64: Make  _midr_in_range_list() an exported
> function")
> >    which moves the implementation out of the header file.
> >
> > Below patch should keep the copy coherency of cputype.h to implement
> > the _midr_in_range_list() as before.
> >
> > [1]
> > https://lore.kernel.org/all/20250221140229.12588-1-shameerali.kolothum
> > .thodi@huawei.com/
> >
> > Thanks.
> >
> >  From 44900e7d3d9fa34c817396275f55a2aab611cd32 Mon Sep 17
> 00:00:00
> > 2001
> > From: Yicong Yang <yangyicong@hisilicon.com>
> > Date: Mon, 16 Jun 2025 15:18:11 +0800
> > Subject: [PATCH] arm64: cputype: Allow copy coherency on cputype.h
> between
> >   tools/ and arch/
> > MIME-Version: 1.0
> > Content-Type: text/plain; charset=UTF-8
> > Content-Transfer-Encoding: 8bit
> >
> > arch/arm64/include/asm/cputype.h is copied from arch/arm64 and used
> by
> > perf to parsing vendor specific SPE packets according to the MIDR.
> > The header diverge after errata management handling for VM live
> > migration merged [1]:
> > - commit e3121298c7fc ("arm64: Modify _midr_range() functions to read
> MIDR/REVIDR internally")
> >    which changed the implementation of is_midr_in_range() that the MIDR
> to
> >    test is always read on the current CPU. This isn't true in perf since
> >    the MIDR is acquired from the perf.data.
> > - commit c8c2647e69be ("arm64: Make  _midr_in_range_list() an exported
> function")
> >    which moves the implementation out of the header file.
> >
> > In order to allow copy coherency on cputype.h [2], implement
> > is_midr_in_range_list() as before [1]. Introduce
> > is_cpuid_in_range_list() for kernel space to test the MIDR of current
> > running CPU is within the target MIDR ranges. Move
> > cpu_errata_set_target_impl() and
> > is_cpuid_in_range_list() to cpufeature.h since they're only used by
> > errata management in the kernel space and don't needed by tools/.
> >
> > No funtional changes intended.
> >
> > [1]
> > https://lore.kernel.org/all/20250221140229.12588-1-shameerali.kolothum
> > .thodi@huawei.com/ [2]
> > https://lore.kernel.org/lkml/aEyGg98z-MkcClXY@x1/#t
> > Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> > ---
> >   arch/arm64/include/asm/cpufeature.h           | 11 +++++
> >   arch/arm64/include/asm/cputype.h              | 40 +++++++++++--------
> >   arch/arm64/kernel/cpu_errata.c                | 30 +++++---------
> >   arch/arm64/kernel/cpufeature.c                |  6 +--
> >   arch/arm64/kernel/proton-pack.c               | 20 +++++-----
> >   arch/arm64/kvm/vgic/vgic-v3.c                 |  2 +-
> >   drivers/clocksource/arm_arch_timer.c          |  2 +-
> >   .../coresight/coresight-etm4x-core.c          |  2 +-
> >   8 files changed, 60 insertions(+), 53 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/cpufeature.h
> > b/arch/arm64/include/asm/cpufeature.h
> > index c4326f1cb917..ba2d474fb393 100644
> > --- a/arch/arm64/include/asm/cpufeature.h
> > +++ b/arch/arm64/include/asm/cpufeature.h
> > @@ -1048,6 +1048,17 @@ static inline bool cpu_has_lpa2(void)
> >   #endif
> >   }
> >
> > +struct target_impl_cpu {
> > +	u64 midr;
> > +	u64 revidr;
> > +	u64 aidr;
> > +};
> > +
> > +bool cpu_errata_set_target_impl(u64 num, void *impl_cpus);
> > +
> > +/* Different from is_midr_in_range() on using the MIDR of current CPU
> > +*/ bool is_cpuid_in_range_list(struct midr_range const *ranges);
> > +
> >   #endif /* __ASSEMBLY__ */
> >
> >   #endif
> > diff --git a/arch/arm64/include/asm/cputype.h
> > b/arch/arm64/include/asm/cputype.h
> > index 661735616787..89fd197e2f03 100644
> > --- a/arch/arm64/include/asm/cputype.h
> > +++ b/arch/arm64/include/asm/cputype.h
> > @@ -251,16 +251,6 @@
> >
> >   #define read_cpuid(reg)			read_sysreg_s(SYS_ ## reg)
> >
> > -/*
> > - * The CPU ID never changes at run time, so we might as well tell the
> > - * compiler that it's constant.  Use this function to read the CPU ID
> > - * rather than directly reading processor_id or read_cpuid() directly.
> > - */
> > -static inline u32 __attribute_const__ read_cpuid_id(void) -{
> > -	return read_cpuid(MIDR_EL1);
> > -}
> > -
> >   /*
> >    * Represent a range of MIDR values for a given CPU model and a
> >    * range of variant/revision values.
> > @@ -296,14 +286,30 @@ static inline bool midr_is_cpu_model_range(u32
> midr, u32 model, u32 rv_min,
> >   	return _model == model && rv >= rv_min && rv <= rv_max;
> >   }
> >
> > -struct target_impl_cpu {
> > -	u64 midr;
> > -	u64 revidr;
> > -	u64 aidr;
> > -};
> > +static inline bool is_midr_in_range(u32 midr, struct midr_range const
> > +*range) {
> > +	return midr_is_cpu_model_range(midr, range->model,
> > +				       range->rv_min, range->rv_max); }
> >
> > -bool cpu_errata_set_target_impl(u64 num, void *impl_cpus); -bool
> > is_midr_in_range_list(struct midr_range const *ranges);
> > +static inline bool
> > +is_midr_in_range_list(u32 midr, struct midr_range const *ranges) {
> > +	while (ranges->model)
> > +		if (is_midr_in_range(midr, ranges++))
> > +			return true;
> > +	return false;
> > +}
> > +
> > +/*
> > + * The CPU ID never changes at run time, so we might as well tell the
> > + * compiler that it's constant.  Use this function to read the CPU ID
> > + * rather than directly reading processor_id or read_cpuid() directly.
> > + */
> > +static inline u32 __attribute_const__ read_cpuid_id(void) {
> > +	return read_cpuid(MIDR_EL1);
> > +}
> >
> >   static inline u64 __attribute_const__ read_cpuid_mpidr(void)
> >   {
> > diff --git a/arch/arm64/kernel/cpu_errata.c
> > b/arch/arm64/kernel/cpu_errata.c index 59d723c9ab8f..531ae67c7086
> > 100644
> > --- a/arch/arm64/kernel/cpu_errata.c
> > +++ b/arch/arm64/kernel/cpu_errata.c
> > @@ -27,38 +27,28 @@ bool cpu_errata_set_target_impl(u64 num, void
> *impl_cpus)
> >   	return true;
> >   }
> >
> > -static inline bool is_midr_in_range(struct midr_range const *range)
> > +bool is_cpuid_in_range_list(struct midr_range const *ranges)
> >   {
> > +	u32 midr = read_cpuid_id();
> >   	int i;
> >
> >   	if (!target_impl_cpu_num)
> > -		return midr_is_cpu_model_range(read_cpuid_id(), range-
> >model,
> > -					       range->rv_min, range->rv_max);
> > +		return is_midr_in_range_list(midr, ranges);
> >
> > -	for (i = 0; i < target_impl_cpu_num; i++) {
> > -		if (midr_is_cpu_model_range(target_impl_cpus[i].midr,
> > -					    range->model,
> > -					    range->rv_min, range->rv_max))
> > +	for (i = 0; i < target_impl_cpu_num; i++)
> > +		if (is_midr_in_range_list(midr, ranges))
> >   			return true;
> > -	}
> > -	return false;
> > -}
> >
> > -bool is_midr_in_range_list(struct midr_range const *ranges) -{
> > -	while (ranges->model)
> > -		if (is_midr_in_range(ranges++))
> > -			return true;
> >   	return false;
> >   }
> > -EXPORT_SYMBOL_GPL(is_midr_in_range_list);
> > +EXPORT_SYMBOL_GPL(is_cpuid_in_range_list);
> >
> >   static bool __maybe_unused
> >   __is_affected_midr_range(const struct arm64_cpu_capabilities *entry,
> >   			 u32 midr, u32 revidr)
> >   {
> >   	const struct arm64_midr_revidr *fix;
> > -	if (!is_midr_in_range(&entry->midr_range))
> > +	if (!is_midr_in_range(midr, &entry->midr_range))
> >   		return false;
> >
> >   	midr &= MIDR_REVISION_MASK | MIDR_VARIANT_MASK; @@ -92,7
> +82,7 @@
> > is_affected_midr_range_list(const struct arm64_cpu_capabilities *entry,
> >   			    int scope)
> >   {
> >   	WARN_ON(scope != SCOPE_LOCAL_CPU || preemptible());
> > -	return is_midr_in_range_list(entry->midr_range_list);
> > +	return is_cpuid_in_range_list(entry->midr_range_list);
> 
> Looks ok to me.
> 
> You could do it with slightly less churn on the kernel side if you keep the
> function name and arguments the same there. There's only one usage in
> Perf so that one could be renamed and have the midr argument added back
> in.

+1.

Can we use a separate one for perf here, something like below(untested)?

--- a/tools/perf/util/arm-spe.c
+++ b/tools/perf/util/arm-spe.c
@@ -842,6 +842,18 @@ static void arm_spe__synth_memory_level(const
struct arm_spe_record *record,
                data_src->mem_lvl |= PERF_MEM_LVL_REM_CCE1;
 }

+static bool is_perf_midr_in_range_list(u32 midr, struct midr_range
const *ranges)
+{
+       while (ranges->model) {
+               if (midr_is_cpu_model_range(midr, ranges->model,
+                                           ranges->rv_min, ranges->rv_max)) {
+                       return true;
+               }
+               ranges++;
+       }
+       return false;
+}
+
 static bool arm_spe__synth_ds(struct arm_spe_queue *speq,
                              const struct arm_spe_record *record,
                              union perf_mem_data_src *data_src)
@@ -882,7 +894,7 @@ static bool arm_spe__synth_ds(struct arm_spe_queue *speq,
        }

        for (i = 0; i < ARRAY_SIZE(data_source_handles); i++) {
-               if (is_midr_in_range_list(midr,
data_source_handles[i].midr_ranges)) {
+               if (is_perf_midr_in_range_list(midr,
data_source_handles[i].midr_ranges)) {
                        data_source_handles[i].ds_synth(record, data_src);
                        return true;
                }
Thanks,
Shameer

> 
> I was also wondering if we could just diverge on the tools side, but in reality
> it also has to stay compatible with the definitions of all the MIDRs so might
> as well keep the whole thing identical.

Re: perf usage of arch/arm64/include/asm/cputype.h

[Leo Yan]

On Mon, Jun 16, 2025 at 09:54:43AM +0000, Shameerali Kolothum Thodi wrote:

[...]

> > > -bool is_midr_in_range_list(struct midr_range const *ranges) -{
> > > -	while (ranges->model)
> > > -		if (is_midr_in_range(ranges++))
> > > -			return true;
> > >   	return false;
> > >   }

> > Looks ok to me.
> > 
> > You could do it with slightly less churn on the kernel side if you keep the
> > function name and arguments the same there. There's only one usage in
> > Perf so that one could be renamed and have the midr argument added back
> > in.
> 
> +1.
> 
> Can we use a separate one for perf here, something like below(untested)?

Thanks for working on this. Agreed.

> --- a/tools/perf/util/arm-spe.c
> +++ b/tools/perf/util/arm-spe.c
> @@ -842,6 +842,18 @@ static void arm_spe__synth_memory_level(const
> struct arm_spe_record *record,
>                 data_src->mem_lvl |= PERF_MEM_LVL_REM_CCE1;
>  }
> 
> +static bool is_perf_midr_in_range_list(u32 midr, struct midr_range
> const *ranges)
> +{
> +       while (ranges->model) {
> +               if (midr_is_cpu_model_range(midr, ranges->model,
> +                                           ranges->rv_min, ranges->rv_max)) {
> +                       return true;
> +               }
> +               ranges++;
> +       }
> +       return false;
> +}

Maybe we can make it more general. For example, move this function into
a common header such as tools/perf/arch/arm64/include/cputype.h. Then,
util/arm-spe.c can include this header.

Thanks,
Leo

Re: perf usage of arch/arm64/include/asm/cputype.h

[Yicong Yang]

On 2025/6/16 21:07, Leo Yan wrote:
> On Mon, Jun 16, 2025 at 09:54:43AM +0000, Shameerali Kolothum Thodi wrote:
> 
> [...]
> 
>>>> -bool is_midr_in_range_list(struct midr_range const *ranges) -{
>>>> -	while (ranges->model)
>>>> -		if (is_midr_in_range(ranges++))
>>>> -			return true;
>>>>   	return false;
>>>>   }
> 
>>> Looks ok to me.
>>>
>>> You could do it with slightly less churn on the kernel side if you keep the
>>> function name and arguments the same there. There's only one usage in
>>> Perf so that one could be renamed and have the midr argument added back
>>> in.
>>
>> +1.
>>
>> Can we use a separate one for perf here, something like below(untested)?
> 
> Thanks for working on this. Agreed.
> 
>> --- a/tools/perf/util/arm-spe.c
>> +++ b/tools/perf/util/arm-spe.c
>> @@ -842,6 +842,18 @@ static void arm_spe__synth_memory_level(const
>> struct arm_spe_record *record,
>>                 data_src->mem_lvl |= PERF_MEM_LVL_REM_CCE1;
>>  }
>>
>> +static bool is_perf_midr_in_range_list(u32 midr, struct midr_range
>> const *ranges)
>> +{
>> +       while (ranges->model) {
>> +               if (midr_is_cpu_model_range(midr, ranges->model,
>> +                                           ranges->rv_min, ranges->rv_max)) {
>> +                       return true;
>> +               }
>> +               ranges++;
>> +       }
>> +       return false;
>> +}
> 
> Maybe we can make it more general. For example, move this function into
> a common header such as tools/perf/arch/arm64/include/cputype.h. Then,
> util/arm-spe.c can include this header.
> 

ok this sounds just like as before except rename the midr check function and modify the
users in perf. will do in below steps:
- move cpu_errata_set_target_impl()/is_midr_in_range_list() out of cputype.h
  since they're only used in the kernel with errata information
- introduce is_target_midr_in_range_list() in cputype.h to test certain MIDR
  is within the ranges. (is_perf_midr_in_range_list() only make sense in
  userspace and is a bit strange to me in a kernel header). maybe reimplement
  is_midr_in_range_list() with is_target_midr_in_range_list() otherwise there's
  no users in kernel
- copy cputype.h to userspace and make users use new is_target_midr_in_range_list()

this will avoid touching the kernel too much and userspace don't need to implement
a separate function.

Thanks.

Re: perf usage of arch/arm64/include/asm/cputype.h

[Leo Yan]

On Mon, Jun 16, 2025 at 11:04:08PM +0800, Yicong Yang wrote:

[...]

> >> +static bool is_perf_midr_in_range_list(u32 midr, struct midr_range
> >> const *ranges)
> >> +{
> >> +       while (ranges->model) {
> >> +               if (midr_is_cpu_model_range(midr, ranges->model,
> >> +                                           ranges->rv_min, ranges->rv_max)) {
> >> +                       return true;
> >> +               }
> >> +               ranges++;
> >> +       }
> >> +       return false;
> >> +}
> > 
> > Maybe we can make it more general. For example, move this function into
> > a common header such as tools/perf/arch/arm64/include/cputype.h. Then,
> > util/arm-spe.c can include this header.
> > 
> 
> ok this sounds just like as before except rename the midr check function and modify the
> users in perf. will do in below steps:
> - move cpu_errata_set_target_impl()/is_midr_in_range_list() out of cputype.h
>   since they're only used in the kernel with errata information
> - introduce is_target_midr_in_range_list() in cputype.h to test certain MIDR
>   is within the ranges. (is_perf_midr_in_range_list() only make sense in
>   userspace and is a bit strange to me in a kernel header). maybe reimplement
>   is_midr_in_range_list() with is_target_midr_in_range_list() otherwise there's
>   no users in kernel
> - copy cputype.h to userspace and make users use new is_target_midr_in_range_list()
> 
> this will avoid touching the kernel too much and userspace don't need to implement
> a separate function.

My understanding is we don't need to touch anything in kernel side, we
simply add a wrapper in perf tool to call midr_is_cpu_model_range().

When introduce is_target_midr_in_range_list() in kernel's cputype.h,
if no consumers in kernel use it and only useful for perf tool, then
it is unlikely to be accepted.

Thanks,
Leo

Re: perf usage of arch/arm64/include/asm/cputype.h

[Mark Rutland]

On Mon, Jun 16, 2025 at 05:08:11PM +0100, Leo Yan wrote:
> On Mon, Jun 16, 2025 at 11:04:08PM +0800, Yicong Yang wrote:
> 
> [...]
> 
> > >> +static bool is_perf_midr_in_range_list(u32 midr, struct midr_range
> > >> const *ranges)
> > >> +{
> > >> +       while (ranges->model) {
> > >> +               if (midr_is_cpu_model_range(midr, ranges->model,
> > >> +                                           ranges->rv_min, ranges->rv_max)) {
> > >> +                       return true;
> > >> +               }
> > >> +               ranges++;
> > >> +       }
> > >> +       return false;
> > >> +}
> > > 
> > > Maybe we can make it more general. For example, move this function into
> > > a common header such as tools/perf/arch/arm64/include/cputype.h. Then,
> > > util/arm-spe.c can include this header.
> > > 
> > 
> > ok this sounds just like as before except rename the midr check function and modify the
> > users in perf. will do in below steps:
> > - move cpu_errata_set_target_impl()/is_midr_in_range_list() out of cputype.h
> >   since they're only used in the kernel with errata information
> > - introduce is_target_midr_in_range_list() in cputype.h to test certain MIDR
> >   is within the ranges. (is_perf_midr_in_range_list() only make sense in
> >   userspace and is a bit strange to me in a kernel header). maybe reimplement
> >   is_midr_in_range_list() with is_target_midr_in_range_list() otherwise there's
> >   no users in kernel
> > - copy cputype.h to userspace and make users use new is_target_midr_in_range_list()
> > 
> > this will avoid touching the kernel too much and userspace don't need to implement
> > a separate function.
> 
> My understanding is we don't need to touch anything in kernel side, we
> simply add a wrapper in perf tool to call midr_is_cpu_model_range().
> 
> When introduce is_target_midr_in_range_list() in kernel's cputype.h,
> if no consumers in kernel use it and only useful for perf tool, then
> it is unlikely to be accepted.

I think all of this is just working around the problem that
asm/cputype.h was never intended to be used in userspace. Likewise with
the other headers that we copy into tools/.

If there are bits that we *want* to share with tools/, let's factor that
out. The actual MIDR values are a good candidate for that -- we can
follow the same approach as with sysreg-defs.h.

Other than that, I think that userspace should just maintain its own
infrastructure, and only pull in things from kernel sources when there's
a specific reason to. Otherwise we're just creating busywork.

Mark.

Re: perf usage of arch/arm64/include/asm/cputype.h

[Leo Yan]

On Mon, Jun 16, 2025 at 06:47:25PM +0100, Mark Rutland wrote:

[...]

> > > ok this sounds just like as before except rename the midr check function and modify the
> > > users in perf. will do in below steps:
> > > - move cpu_errata_set_target_impl()/is_midr_in_range_list() out of cputype.h
> > >   since they're only used in the kernel with errata information
> > > - introduce is_target_midr_in_range_list() in cputype.h to test certain MIDR
> > >   is within the ranges. (is_perf_midr_in_range_list() only make sense in
> > >   userspace and is a bit strange to me in a kernel header). maybe reimplement
> > >   is_midr_in_range_list() with is_target_midr_in_range_list() otherwise there's
> > >   no users in kernel
> > > - copy cputype.h to userspace and make users use new is_target_midr_in_range_list()
> > > 
> > > this will avoid touching the kernel too much and userspace don't need to implement
> > > a separate function.
> > 
> > My understanding is we don't need to touch anything in kernel side, we
> > simply add a wrapper in perf tool to call midr_is_cpu_model_range().
> > 
> > When introduce is_target_midr_in_range_list() in kernel's cputype.h,
> > if no consumers in kernel use it and only useful for perf tool, then
> > it is unlikely to be accepted.
> 
> I think all of this is just working around the problem that
> asm/cputype.h was never intended to be used in userspace. Likewise with
> the other headers that we copy into tools/.
> 
> If there are bits that we *want* to share with tools/, let's factor that
> out. The actual MIDR values are a good candidate for that -- we can
> follow the same approach as with sysreg-defs.h.

Thanks for suggestion, Mark.

It makes sense to me for extracting MIDR and sharing between kernel and
tools/. I have created a task for following up the refactoring.

> Other than that, I think that userspace should just maintain its own
> infrastructure, and only pull in things from kernel sources when there's
> a specific reason to. Otherwise we're just creating busywork.

I agree with the methodology.

Since Arnaldo is facing build failure when sync headers between kernel
and perf tool, to avoid long latency, let us split the refactoriing
into separate steps.

As a first step, I think my previous suggestion is valid, we can create a
header tools/perf/arch/arm64/include/cputype.h with below code:

  #include "../../../../arch/arm64/include/asm/cputype.h"

  static bool is_perf_midr_in_range_list(u32 midr,
                                         struct midr_range const *ranges)
  {
          while (ranges->model) {
                  if (midr_is_cpu_model_range(midr, ranges->model,
                                  ranges->rv_min, ranges->rv_max))
                          return true;
                  ranges++;
          }

          return false;
  }

Then, once we can generate a dynamic MIDR header file, we can use that
header and define the midr_range structure specifically in the perf.
In the end, perf can avoid to include kernel's cputype.h.

If no objection, Yicong, do you mind preparing the patch mentioned
above? Thanks!

Leo

Re: perf usage of arch/arm64/include/asm/cputype.h

[Yicong Yang]

On 2025/6/17 22:18, Leo Yan wrote:
> On Mon, Jun 16, 2025 at 06:47:25PM +0100, Mark Rutland wrote:
> 
> [...]
> 
>>>> ok this sounds just like as before except rename the midr check function and modify the
>>>> users in perf. will do in below steps:
>>>> - move cpu_errata_set_target_impl()/is_midr_in_range_list() out of cputype.h
>>>>   since they're only used in the kernel with errata information
>>>> - introduce is_target_midr_in_range_list() in cputype.h to test certain MIDR
>>>>   is within the ranges. (is_perf_midr_in_range_list() only make sense in
>>>>   userspace and is a bit strange to me in a kernel header). maybe reimplement
>>>>   is_midr_in_range_list() with is_target_midr_in_range_list() otherwise there's
>>>>   no users in kernel
>>>> - copy cputype.h to userspace and make users use new is_target_midr_in_range_list()
>>>>
>>>> this will avoid touching the kernel too much and userspace don't need to implement
>>>> a separate function.
>>>
>>> My understanding is we don't need to touch anything in kernel side, we
>>> simply add a wrapper in perf tool to call midr_is_cpu_model_range().
>>>
>>> When introduce is_target_midr_in_range_list() in kernel's cputype.h,
>>> if no consumers in kernel use it and only useful for perf tool, then
>>> it is unlikely to be accepted.
>>
>> I think all of this is just working around the problem that
>> asm/cputype.h was never intended to be used in userspace. Likewise with
>> the other headers that we copy into tools/.
>>
>> If there are bits that we *want* to share with tools/, let's factor that
>> out. The actual MIDR values are a good candidate for that -- we can
>> follow the same approach as with sysreg-defs.h.
> 
> Thanks for suggestion, Mark.
> 
> It makes sense to me for extracting MIDR and sharing between kernel and
> tools/. I have created a task for following up the refactoring.
> 
>> Other than that, I think that userspace should just maintain its own
>> infrastructure, and only pull in things from kernel sources when there's
>> a specific reason to. Otherwise we're just creating busywork.
> 
> I agree with the methodology.
> 
> Since Arnaldo is facing build failure when sync headers between kernel
> and perf tool, to avoid long latency, let us split the refactoriing
> into separate steps.
> 
> As a first step, I think my previous suggestion is valid, we can create a
> header tools/perf/arch/arm64/include/cputype.h with below code:
> 
>   #include "../../../../arch/arm64/include/asm/cputype.h"
> 
>   static bool is_perf_midr_in_range_list(u32 midr,
>                                          struct midr_range const *ranges)
>   {
>           while (ranges->model) {
>                   if (midr_is_cpu_model_range(midr, ranges->model,
>                                   ranges->rv_min, ranges->rv_max))
>                           return true;
>                   ranges++;
>           }
> 
>           return false;
>   }
> 
> Then, once we can generate a dynamic MIDR header file, we can use that
> header and define the midr_range structure specifically in the perf.
> In the end, perf can avoid to include kernel's cputype.h.
> 
> If no objection, Yicong, do you mind preparing the patch mentioned
> above? Thanks!
> 

sure. will post today.

Thanks.

Re: perf usage of arch/arm64/include/asm/cputype.h

[Mark Rutland]

On Tue, Jun 17, 2025 at 03:18:10PM +0100, Leo Yan wrote:
> On Mon, Jun 16, 2025 at 06:47:25PM +0100, Mark Rutland wrote:
> 
> [...]
> 
> > > > ok this sounds just like as before except rename the midr check function and modify the
> > > > users in perf. will do in below steps:
> > > > - move cpu_errata_set_target_impl()/is_midr_in_range_list() out of cputype.h
> > > >   since they're only used in the kernel with errata information
> > > > - introduce is_target_midr_in_range_list() in cputype.h to test certain MIDR
> > > >   is within the ranges. (is_perf_midr_in_range_list() only make sense in
> > > >   userspace and is a bit strange to me in a kernel header). maybe reimplement
> > > >   is_midr_in_range_list() with is_target_midr_in_range_list() otherwise there's
> > > >   no users in kernel
> > > > - copy cputype.h to userspace and make users use new is_target_midr_in_range_list()
> > > > 
> > > > this will avoid touching the kernel too much and userspace don't need to implement
> > > > a separate function.
> > > 
> > > My understanding is we don't need to touch anything in kernel side, we
> > > simply add a wrapper in perf tool to call midr_is_cpu_model_range().
> > > 
> > > When introduce is_target_midr_in_range_list() in kernel's cputype.h,
> > > if no consumers in kernel use it and only useful for perf tool, then
> > > it is unlikely to be accepted.
> > 
> > I think all of this is just working around the problem that
> > asm/cputype.h was never intended to be used in userspace. Likewise with
> > the other headers that we copy into tools/.
> > 
> > If there are bits that we *want* to share with tools/, let's factor that
> > out. The actual MIDR values are a good candidate for that -- we can
> > follow the same approach as with sysreg-defs.h.
> 
> Thanks for suggestion, Mark.
> 
> It makes sense to me for extracting MIDR and sharing between kernel and
> tools/. I have created a task for following up the refactoring.
> 
> > Other than that, I think that userspace should just maintain its own
> > infrastructure, and only pull in things from kernel sources when there's
> > a specific reason to. Otherwise we're just creating busywork.
> 
> I agree with the methodology.
> 
> Since Arnaldo is facing build failure when sync headers between kernel
> and perf tool, to avoid long latency, let us split the refactoriing
> into separate steps.
> 
> As a first step, I think my previous suggestion is valid, we can create a
> header tools/perf/arch/arm64/include/cputype.h with below code:
> 
>   #include "../../../../arch/arm64/include/asm/cputype.h"

Directly including the kernel header introduces the very fragility that
having a copy was intended to avoid. NAK to that.

I've replied to the same effect Yicong's patch [1,2].

If we want to share headers between userspace and kernel, we should
refactor those headers such that this is safe by construction.

There is no need to update the userspace headers just because the kernel
headers have changed, so the simple solution in the short term is to
suppress the warning from check-headers.sh.

[1] https://lore.kernel.org/linux-arm-kernel/dc5afc5c-060c-8bcb-c3a7-0de49a7455fb@huawei.com/T/#m23dfbea6af559f3765d89b9d8427213588871ffd
[2] https://lore.kernel.org/linux-arm-kernel/dc5afc5c-060c-8bcb-c3a7-0de49a7455fb@huawei.com/T/#m6acbfa00002af8ee791266ea86a58f8f994ed710

Mark.

> 
>   static bool is_perf_midr_in_range_list(u32 midr,
>                                          struct midr_range const *ranges)
>   {
>           while (ranges->model) {
>                   if (midr_is_cpu_model_range(midr, ranges->model,
>                                   ranges->rv_min, ranges->rv_max))
>                           return true;
>                   ranges++;
>           }
> 
>           return false;
>   }
> 
> Then, once we can generate a dynamic MIDR header file, we can use that
> header and define the midr_range structure specifically in the perf.
> In the end, perf can avoid to include kernel's cputype.h.
> 
> If no objection, Yicong, do you mind preparing the patch mentioned
> above? Thanks!
> 
> Leo

Re: perf usage of arch/arm64/include/asm/cputype.h

[Leo Yan]

On Wed, Jun 18, 2025 at 09:52:53AM +0100, Mark Rutland wrote:

[...]

> > > Other than that, I think that userspace should just maintain its own
> > > infrastructure, and only pull in things from kernel sources when there's
> > > a specific reason to. Otherwise we're just creating busywork.
> > 
> > I agree with the methodology.
> > 
> > Since Arnaldo is facing build failure when sync headers between kernel
> > and perf tool, to avoid long latency, let us split the refactoriing
> > into separate steps.
> > 
> > As a first step, I think my previous suggestion is valid, we can create a
> > header tools/perf/arch/arm64/include/cputype.h with below code:
> > 
> >   #include "../../../../arch/arm64/include/asm/cputype.h"
> 
> Directly including the kernel header introduces the very fragility that
> having a copy was intended to avoid. NAK to that.

My suggestion is not to include the kernel header, nor to modify the
copy header. :)

Instead, I suggested creating a new header within the perf tool (under
perf's arm64 folder) and then include the copy header in tools:

  tools/arch/arm64/include/asm/cputype.h

> I've replied to the same effect Yicong's patch [1,2].
> 
> If we want to share headers between userspace and kernel, we should
> refactor those headers such that this is safe by construction.
> 
> There is no need to update the userspace headers just because the kernel
> headers have changed, so the simple solution in the short term is to
> suppress the warning from check-headers.sh.

Sure, makes sense for me.

@Arnaldo, as Mark suggested, do you want me to send a patch to remove
cputype.h checking in check-headers.sh or it is fine to keep the warning
until finish the header refactoring?

@Yicong, could you confirm if you proceed to refactor the MIDR? thanks!

Just note, I searched tools folder and found kselftest also uses the
cputype.h header. The refactoring should not break the files below.

$ git grep cputype.h
perf/check-headers.sh:check arch/arm64/include/asm/cputype.h '-I "^#include [<\"]\(asm/\)*sysreg.h"'
perf/util/arm-spe.c:#include "../../arch/arm64/include/asm/cputype.h"
testing/selftests/kvm/arm64/psci_test.c:#include <asm/cputype.h>
testing/selftests/kvm/lib/arm64/vgic.c:#include <asm/cputype.h>

Thanks,
Leo

> [1] https://lore.kernel.org/linux-arm-kernel/dc5afc5c-060c-8bcb-c3a7-0de49a7455fb@huawei.com/T/#m23dfbea6af559f3765d89b9d8427213588871ffd
> [2] https://lore.kernel.org/linux-arm-kernel/dc5afc5c-060c-8bcb-c3a7-0de49a7455fb@huawei.com/T/#m6acbfa00002af8ee791266ea86a58f8f994ed710
> 
> Mark.
> 
> > 
> >   static bool is_perf_midr_in_range_list(u32 midr,
> >                                          struct midr_range const *ranges)
> >   {
> >           while (ranges->model) {
> >                   if (midr_is_cpu_model_range(midr, ranges->model,
> >                                   ranges->rv_min, ranges->rv_max))
> >                           return true;
> >                   ranges++;
> >           }
> > 
> >           return false;
> >   }
> > 
> > Then, once we can generate a dynamic MIDR header file, we can use that
> > header and define the midr_range structure specifically in the perf.
> > In the end, perf can avoid to include kernel's cputype.h.
> > 
> > If no objection, Yicong, do you mind preparing the patch mentioned
> > above? Thanks!
> > 
> > Leo

Re: perf usage of arch/arm64/include/asm/cputype.h

[Yicong Yang]

On 2025/6/18 19:24, Leo Yan wrote:
> On Wed, Jun 18, 2025 at 09:52:53AM +0100, Mark Rutland wrote:
> 
> [...]
> 
>>>> Other than that, I think that userspace should just maintain its own
>>>> infrastructure, and only pull in things from kernel sources when there's
>>>> a specific reason to. Otherwise we're just creating busywork.
>>>
>>> I agree with the methodology.
>>>
>>> Since Arnaldo is facing build failure when sync headers between kernel
>>> and perf tool, to avoid long latency, let us split the refactoriing
>>> into separate steps.
>>>
>>> As a first step, I think my previous suggestion is valid, we can create a
>>> header tools/perf/arch/arm64/include/cputype.h with below code:
>>>
>>>   #include "../../../../arch/arm64/include/asm/cputype.h"
>>
>> Directly including the kernel header introduces the very fragility that
>> having a copy was intended to avoid. NAK to that.
> 
> My suggestion is not to include the kernel header, nor to modify the
> copy header. :)
> 
> Instead, I suggested creating a new header within the perf tool (under
> perf's arm64 folder) and then include the copy header in tools:
> 
>   tools/arch/arm64/include/asm/cputype.h
> 

sorry for the misunderstood.:(
in this way we still have the divergency in the long term and as a workaround
this works same if we partly update the tools/arch/arm64/include/asm/cputype.h
with only necessary MIDR updates and keep is_midr_in_range_list() unchanged.

>> I've replied to the same effect Yicong's patch [1,2].
>>
>> If we want to share headers between userspace and kernel, we should
>> refactor those headers such that this is safe by construction.
>>
>> There is no need to update the userspace headers just because the kernel
>> headers have changed, so the simple solution in the short term is to
>> suppress the warning from check-headers.sh.
> 
> Sure, makes sense for me.
> 
> @Arnaldo, as Mark suggested, do you want me to send a patch to remove
> cputype.h checking in check-headers.sh or it is fine to keep the warning
> until finish the header refactoring?
> 
> @Yicong, could you confirm if you proceed to refactor the MIDR? thanks!
> 

please feel free to take this over.

> Just note, I searched tools folder and found kselftest also uses the
> cputype.h header. The refactoring should not break the files below.
> 

they shouldn't affected. I did a kselftest build test with my latest patch
and they were not affected.

thanks.

> $ git grep cputype.h
> perf/check-headers.sh:check arch/arm64/include/asm/cputype.h '-I "^#include [<\"]\(asm/\)*sysreg.h"'
> perf/util/arm-spe.c:#include "../../arch/arm64/include/asm/cputype.h"
> testing/selftests/kvm/arm64/psci_test.c:#include <asm/cputype.h>
> testing/selftests/kvm/lib/arm64/vgic.c:#include <asm/cputype.h>
> 
> Thanks,
> Leo
> 
>> [1] https://lore.kernel.org/linux-arm-kernel/dc5afc5c-060c-8bcb-c3a7-0de49a7455fb@huawei.com/T/#m23dfbea6af559f3765d89b9d8427213588871ffd
>> [2] https://lore.kernel.org/linux-arm-kernel/dc5afc5c-060c-8bcb-c3a7-0de49a7455fb@huawei.com/T/#m6acbfa00002af8ee791266ea86a58f8f994ed710
>>
>> Mark.
>>
>>>
>>>   static bool is_perf_midr_in_range_list(u32 midr,
>>>                                          struct midr_range const *ranges)
>>>   {
>>>           while (ranges->model) {
>>>                   if (midr_is_cpu_model_range(midr, ranges->model,
>>>                                   ranges->rv_min, ranges->rv_max))
>>>                           return true;
>>>                   ranges++;
>>>           }
>>>
>>>           return false;
>>>   }
>>>
>>> Then, once we can generate a dynamic MIDR header file, we can use that
>>> header and define the midr_range structure specifically in the perf.
>>> In the end, perf can avoid to include kernel's cputype.h.
>>>
>>> If no objection, Yicong, do you mind preparing the patch mentioned
>>> above? Thanks!
>>>
>>> Leo
> 
> .
> 

Re: perf usage of arch/arm64/include/asm/cputype.h

[Leo Yan]

On Wed, Jun 18, 2025 at 07:51:03PM +0800, Yicong Yang wrote:

[...]

> >> Directly including the kernel header introduces the very fragility that
> >> having a copy was intended to avoid. NAK to that.
> > 
> > My suggestion is not to include the kernel header, nor to modify the
> > copy header. :)
> > 
> > Instead, I suggested creating a new header within the perf tool (under
> > perf's arm64 folder) and then include the copy header in tools:
> > 
> >   tools/arch/arm64/include/asm/cputype.h
> > 
> 
> sorry for the misunderstood.:(
> in this way we still have the divergency in the long term and as a workaround
> this works same if we partly update the tools/arch/arm64/include/asm/cputype.h
> with only necessary MIDR updates and keep is_midr_in_range_list() unchanged.

Yes. So Mark's suggestion is reasonable that we can do refactoring first
to avoid syncing header.

[...]

> > @Yicong, could you confirm if you proceed to refactor the MIDR? thanks!
> 
> please feel free to take this over.

Thanks a lot for confirmation!  And thanks for working on the reported
issue.

> > Just note, I searched tools folder and found kselftest also uses the
> > cputype.h header. The refactoring should not break the files below.
> > 
> 
> they shouldn't affected. I did a kselftest build test with my latest patch
> and they were not affected.

I expect tools/arch/arm64/include/asm/cputype.h will be removed, and
a generated header (something like sys-midr.h) for MIDR refactoring.
If this is true, then we need to take care kselftest.

Thanks,
Leo

Re: perf usage of arch/arm64/include/asm/cputype.h

[Arnaldo Carvalho de Melo]

On Wed, Jun 18, 2025 at 12:24:40PM +0100, Leo Yan wrote:
> On Wed, Jun 18, 2025 at 09:52:53AM +0100, Mark Rutland wrote:
> > If we want to share headers between userspace and kernel, we should
> > refactor those headers such that this is safe by construction.

> > There is no need to update the userspace headers just because the kernel
> > headers have changed, so the simple solution in the short term is to
> > suppress the warning from check-headers.sh.
 
> Sure, makes sense for me.
> 
> @Arnaldo, as Mark suggested, do you want me to send a patch to remove
> cputype.h checking in check-headers.sh or it is fine to keep the warning
> until finish the header refactoring?

It is ok to have the warning, its just this one at this point and it is
serving its purpose.

When the refactoring gets done, it will go away. Think of it as a
reminder :-)

- Arnaldo

Re: perf usage of arch/arm64/include/asm/cputype.h

[Leo Yan]

On Wed, Jun 18, 2025 at 10:15:04AM -0300, Arnaldo Carvalho de Melo wrote:

[...]

> > @Arnaldo, as Mark suggested, do you want me to send a patch to remove
> > cputype.h checking in check-headers.sh or it is fine to keep the warning
> > until finish the header refactoring?
> 
> It is ok to have the warning, its just this one at this point and it is
> serving its purpose.
> 
> When the refactoring gets done, it will go away. Think of it as a
> reminder :-)

Fair point. Thanks for confirmation!

Re: perf usage of arch/arm64/include/asm/cputype.h

[Mark Rutland]

On Mon, Jun 16, 2025 at 10:29:48AM +0100, James Clark wrote:
> I was also wondering if we could just diverge on the tools side, but in
> reality it also has to stay compatible with the definitions of all the MIDRs
> so might as well keep the whole thing identical.

I think that'd be the best option overall.

The only thing we really care about updating periodically is the set of
MIDRs, and we can factor that out to a cputype-defs.h header, and
possibly generate that too (as with sysreg-defs.h).

Mark.