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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aFJ-Za6oRGKKASN1@J2N7QTR9R3>
Date: Wed, 18 Jun 2025 09:52:53 +0100
From: Mark Rutland <mark.rutland@....com>
To: Leo Yan <leo.yan@....com>
Cc: Yicong Yang <yangyicong@...wei.com>,
	Shameerali Kolothum Thodi <shameerali.kolothum.thodi@...wei.com>,
	yangyicong@...ilicon.com, James Clark <james.clark@...aro.org>,
	Arnaldo Carvalho de Melo <acme@...nel.org>,
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>,
	Ali Saidi <alisaidi@...zon.com>, Leo Yan <leo.yan@...aro.org>,
	Will Deacon <will@...nel.org>, James Morse <james.morse@....com>,
	Catalin Marinas <catalin.marinas@....com>,
	yangjinqian <yangjinqian1@...wei.com>,
	Douglas Anderson <dianders@...omium.org>,
	Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
	Adrian Hunter <adrian.hunter@...el.com>,
	Ian Rogers <irogers@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
	Kan Liang <kan.liang@...ux.intel.com>,
	Namhyung Kim <namhyung@...nel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: perf usage of arch/arm64/include/asm/cputype.h

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