Re: perf usage of arch/arm64/include/asm/cputype.h
From: Mark Rutland <mark.rutland@arm.com>
Date: 2025-06-16 17:47:32
Also in:
lkml
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: [...]quoted
quoted
quoted
+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.