From nobody Thu Oct 9 20:24:11 2025 Received: from szxga06-in.huawei.com (szxga06-in.huawei.com [45.249.212.32]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C52FD26E718 for ; Mon, 16 Jun 2025 09:54:54 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=45.249.212.32 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1750067697; cv=none; b=ctDKrtE5MZ01KyORx+lMGHjPLYsSM6wDhAoolhhY5o+5LYxS9YdYj/hzMoh/Qf7KhrlsRjAlzlawDUwqyfLCESiZmA9jQJWnsteWbZmxFXTydgfyTNs0H4lSMZ1Poi+ZLRJtezcd+us8e5XmN/rnsP3w4XcAOXT2m4bvpQj5K9M= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1750067697; c=relaxed/simple; bh=JEKd/RHtuZES4E4FP2KSa/UAr3/Fi0ptLaIIDGahUwQ=; h=From:To:CC:Subject:Date:Message-ID:References:In-Reply-To: Content-Type:MIME-Version; b=SL6GjCkQsb2eA8S3gOESjHATIHXIFtmP9/OeB51Y3IeOwwAPA2irPjzrX1lQHt5hG0pGnlkRk1/mAYRIAz4a2DPhUm+ymjamKryAB3G/JoPHtbTu2ib3RziGKZ4bXft2r5p2GEBtgqHx55v/VlDOj9v0carHfz9bEulVORg9sNw= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com; spf=pass smtp.mailfrom=huawei.com; arc=none smtp.client-ip=45.249.212.32 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=huawei.com Received: from mail.maildlp.com (unknown [172.19.88.234]) by szxga06-in.huawei.com (SkyGuard) with ESMTP id 4bLQNr3LMcz2QVJG; Mon, 16 Jun 2025 17:55:40 +0800 (CST) Received: from dggemv705-chm.china.huawei.com (unknown [10.3.19.32]) by mail.maildlp.com (Postfix) with ESMTPS id EEF7D1400D2; Mon, 16 Jun 2025 17:54:46 +0800 (CST) Received: from kwepemq100018.china.huawei.com (7.202.194.200) by dggemv705-chm.china.huawei.com (10.3.19.32) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.11; Mon, 16 Jun 2025 17:54:46 +0800 Received: from frapeml500008.china.huawei.com (7.182.85.71) by kwepemq100018.china.huawei.com (7.202.194.200) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.11; Mon, 16 Jun 2025 17:54:45 +0800 Received: from frapeml500008.china.huawei.com ([7.182.85.71]) by frapeml500008.china.huawei.com ([7.182.85.71]) with mapi id 15.01.2507.039; Mon, 16 Jun 2025 11:54:43 +0200 From: Shameerali Kolothum Thodi To: James Clark , yangyicong , Arnaldo Carvalho de Melo , "linux-arm-kernel@lists.infradead.org" CC: Ali Saidi , Leo Yan , Will Deacon , James Morse , Catalin Marinas , yangjinqian , "Douglas Anderson" , Dmitry Baryshkov , Adrian Hunter , "Ian Rogers" , Jiri Olsa , Kan Liang , Namhyung Kim , "Linux Kernel Mailing List" , yangyicong Subject: RE: perf usage of arch/arm64/include/asm/cputype.h Thread-Topic: perf usage of arch/arm64/include/asm/cputype.h Thread-Index: AQHb3pQ7Mwqvf8oEbUSICRpXyr+LPbQFY7gAgAAmSkA= Date: Mon, 16 Jun 2025 09:54:43 +0000 Message-ID: <0b839ec1ae89439e95d7069adcbb95ab@huawei.com> References: <1762acd6-df55-c10b-e396-2c6ed37d16c1@huawei.com> <2abcf4ec-4725-4e79-b8d3-a4ddbc00caba@linaro.org> In-Reply-To: <2abcf4ec-4725-4e79-b8d3-a4ddbc00caba@linaro.org> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 > -----Original Message----- > From: James Clark > Sent: Monday, June 16, 2025 10:30 AM > To: yangyicong ; Arnaldo Carvalho de Melo > ; linux-arm-kernel@lists.infradead.org > Cc: Ali Saidi ; Leo Yan ; Will > Deacon ; James Morse ; Catalin > Marinas ; yangjinqian > ; Douglas Anderson > ; Dmitry Baryshkov > ; Adrian Hunter ; > Ian Rogers ; Jiri Olsa ; Kan Liang > ; Namhyung Kim ; > Linux Kernel Mailing List ; Shameerali > Kolothum Thodi ; yangyicong > > Subject: Re: perf usage of arch/arm64/include/asm/cputype.h >=20 >=20 >=20 > 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 > >> 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 > >> Tested-by: Leo Yan > >> > >> - Arnaldo > >> > >> =E2=AC=A2 [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 > >> =E2=AC=A2 [acme@toolbx perf-tools]$ cp arch/arm64/include/asm/cputype.h > >> tools/arch/arm64/include/asm/cputype.h > >> =E2=AC=A2 [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 =E2=80=98arm_spe__synth_ds=E2=80=99: > >> util/arm-spe.c:885:43: error: passing argument 1 of > =E2=80=98is_midr_in_range_list=E2=80=99 makes pointer from integer withou= t a cast [-Wint- > conversion] > >> 885 | if (is_midr_in_range_list(midr, > data_source_handles[i].midr_ranges)) { > >> | ^~~~ > >> | | > >> | u64 {aka long unsig= ned int} > >> In file included from util/arm-spe.c:37: > >> util/../../arch/arm64/include/asm/cputype.h:306:53: note: expected > =E2=80=98const struct midr_range *=E2=80=99 but argument is of type =E2= =80=98u64=E2=80=99 {aka =E2=80=98long > unsigned int=E2=80=99} > >> 306 | bool is_midr_in_range_list(struct midr_range const *ranges); > >> | ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~ > >> util/arm-spe.c:885:21: error: too many arguments to function > =E2=80=98is_midr_in_range_list=E2=80=99; 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' > >> =E2=AC=A2 [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 =C2=A0_midr_in_range_list() an expo= rted > 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 > > 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=3DUTF-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 =C2=A0_midr_in_range_list() an expo= rted > 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 > > --- > > 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 =3D=3D model && rv >=3D rv_min && rv <=3D 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 =3D 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 =3D 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 =3D 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 &=3D 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 !=3D SCOPE_LOCAL_CPU || preemptible()); > > - return is_midr_in_range_list(entry->midr_range_list); > > + return is_cpuid_in_range_list(entry->midr_range_list); >=20 > Looks ok to me. >=20 > You could do it with slightly less churn on the kernel side if you keep t= he > 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 |=3D 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 *spe= q, } for (i =3D 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 >=20 > I was also wondering if we could just diverge on the tools side, but in r= eality > it also has to stay compatible with the definitions of all the MIDRs so m= ight > as well keep the whole thing identical.