* [REPOST PATCH v6 0/3] arm64: topology: Handle AMU FIE setup on CPU hotplug
@ 2025-12-30 8:01 Lifeng Zheng
2025-12-30 8:01 ` [REPOST PATCH v6 1/3] arm64: topology: Skip already covered CPUs when setting freq source Lifeng Zheng
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Lifeng Zheng @ 2025-12-30 8:01 UTC (permalink / raw)
To: catalin.marinas, will, rafael, viresh.kumar, sudeep.holla, gregkh,
dakr, beata.michalska, ionela.voinescu
Cc: linux-arm-kernel, linux-pm, linuxarm, jonathan.cameron,
vincent.guittot, zhanjie9, lihuisong, yubowen8, zhangpengjie2,
wangzhi12, linhongye, zhenglifeng1
Solve a problem that causes CPUs Setup AMU FIE failed in a corner case,
even though they're eligible.
Changelog:
v6:
- discard the modifications in cpufreq.c, and instead, make
supports_scale_freq_counters() checks that at least one CPU in the
policy supports AMU FIE, instead of all
- based on Beata's feedback, optimize cpuhp_topology_online() to make it
more readable
- use pr_warn instead of WARN_ON to show warning message when the
freq_counters_valid() check fails in cpuhp_topology_online()
- modify commit message as Beata and Rafael suggested
v5:
- add a default implementation for cpufreq_cpu_policy() when
CONFIG_CPU_FREQ is not defined
v4:
- change the function's name in patch 2 from
'cpufreq_cpu_get_raw_no_check' to 'cpufreq_cpu_policy'
- use only one line in the function body of cpufreq_cpu_policy()
- use cpus mask instead of related_cpus when calling arch_set_freq_scale()
in cpufreq.c
- add a warning when the freq_counters_valid() check fails in
cpuhp_topology_online()
v3:
- add a patch to optimize amu_fie_setup()
- add a patch to add a function to get cpufreq policy without checking if
the CPU is online
- discard the reuse of amu_fie_setup() in cpuhp_topology_online() and keep
all the new logic in cpuhp_topology_online()
- test only the CPU which is going online in cpuhp_topology_online()
- when the freq_counters_valid() check fails, not only clear the scale
freq source but also clear all the related CPUs from amu_fie_cpus mask
- add some comments
v2:
- keep init_amu_fie_notifier for setting up AMU FIE when the cpufreq
policy is being created
- set up AMU FIE only for online CPUs instead of related_cpus in
init_amu_fie_callback()
- check and set all the online CPUs in the same policy when hotplug one
- clear scale freq source for all the online CPUs in the same policy to
avoid using different source of the freq scale
---
Discussions of previous version:
v1: https://lore.kernel.org/all/20250607094533.416368-1-zhenglifeng1@huawei.com/
v2: https://lore.kernel.org/all/20250725102813.1404322-1-zhenglifeng1@huawei.com/
v3: https://lore.kernel.org/all/20250805093330.3715444-1-zhenglifeng1@huawei.com/
v4: https://lore.kernel.org/all/20250814072853.3426386-1-zhenglifeng1@huawei.com/
v5: https://lore.kernel.org/all/20250819072931.1647431-1-zhenglifeng1@huawei.com/
Lifeng Zheng (3):
arm64: topology: Skip already covered CPUs when setting freq source
cpufreq: Add new helper function returning cpufreq policy
arm64: topology: Handle AMU FIE setup on CPU hotplug
arch/arm64/kernel/topology.c | 67 ++++++++++++++++++++++++++++++++++--
drivers/base/arch_topology.c | 9 ++++-
drivers/cpufreq/cpufreq.c | 6 ++++
include/linux/cpufreq.h | 5 +++
4 files changed, 83 insertions(+), 4 deletions(-)
--
2.33.0
^ permalink raw reply [flat|nested] 11+ messages in thread* [REPOST PATCH v6 1/3] arm64: topology: Skip already covered CPUs when setting freq source 2025-12-30 8:01 [REPOST PATCH v6 0/3] arm64: topology: Handle AMU FIE setup on CPU hotplug Lifeng Zheng @ 2025-12-30 8:01 ` Lifeng Zheng 2025-12-30 8:01 ` [REPOST PATCH v6 2/3] cpufreq: Add new helper function returning cpufreq policy Lifeng Zheng 2025-12-30 8:01 ` [REPOST PATCH v6 3/3] arm64: topology: Handle AMU FIE setup on CPU hotplug Lifeng Zheng 2 siblings, 0 replies; 11+ messages in thread From: Lifeng Zheng @ 2025-12-30 8:01 UTC (permalink / raw) To: catalin.marinas, will, rafael, viresh.kumar, sudeep.holla, gregkh, dakr, beata.michalska, ionela.voinescu Cc: linux-arm-kernel, linux-pm, linuxarm, jonathan.cameron, vincent.guittot, zhanjie9, lihuisong, yubowen8, zhangpengjie2, wangzhi12, linhongye, zhenglifeng1 The scale freq source of the CPUs in 'amu_fie_cpus' mask are already set to AMU tick before, so in amu_fie_setup(), only the CPUs in the 'cpus' mask should be set. Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com> Reviewed-by: Beata Michalska <beata.michalska@arm.com> Reviewed-by: Jie Zhan <zhanjie9@hisilicon.com> --- arch/arm64/kernel/topology.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c index 5d24dc53799b..cf9bb761af3a 100644 --- a/arch/arm64/kernel/topology.c +++ b/arch/arm64/kernel/topology.c @@ -272,7 +272,7 @@ static void amu_fie_setup(const struct cpumask *cpus) cpumask_or(amu_fie_cpus, amu_fie_cpus, cpus); - topology_set_scale_freq_source(&amu_sfd, amu_fie_cpus); + topology_set_scale_freq_source(&amu_sfd, cpus); pr_debug("CPUs[%*pbl]: counters will be used for FIE.", cpumask_pr_args(cpus)); -- 2.33.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [REPOST PATCH v6 2/3] cpufreq: Add new helper function returning cpufreq policy 2025-12-30 8:01 [REPOST PATCH v6 0/3] arm64: topology: Handle AMU FIE setup on CPU hotplug Lifeng Zheng 2025-12-30 8:01 ` [REPOST PATCH v6 1/3] arm64: topology: Skip already covered CPUs when setting freq source Lifeng Zheng @ 2025-12-30 8:01 ` Lifeng Zheng 2025-12-30 8:01 ` [REPOST PATCH v6 3/3] arm64: topology: Handle AMU FIE setup on CPU hotplug Lifeng Zheng 2 siblings, 0 replies; 11+ messages in thread From: Lifeng Zheng @ 2025-12-30 8:01 UTC (permalink / raw) To: catalin.marinas, will, rafael, viresh.kumar, sudeep.holla, gregkh, dakr, beata.michalska, ionela.voinescu Cc: linux-arm-kernel, linux-pm, linuxarm, jonathan.cameron, vincent.guittot, zhanjie9, lihuisong, yubowen8, zhangpengjie2, wangzhi12, linhongye, zhenglifeng1 cpufreq_cpu_get_raw() gets cpufreq policy only if the CPU is in policy->cpus mask, which means the CPU is already online. But in some cases, the policy is needed before the CPU is added to cpus mask. Add a function to get the policy in these cases. Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com> Reviewed-by: Rafael J. Wysocki (Intel) <rafael@kernel.org> Reviewed-by: Jie Zhan <zhanjie9@hisilicon.com> --- drivers/cpufreq/cpufreq.c | 6 ++++++ include/linux/cpufreq.h | 5 +++++ 2 files changed, 11 insertions(+) diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 50dde2980f1b..a7a69f4d7675 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -198,6 +198,12 @@ struct cpufreq_policy *cpufreq_cpu_get_raw(unsigned int cpu) } EXPORT_SYMBOL_GPL(cpufreq_cpu_get_raw); +struct cpufreq_policy *cpufreq_cpu_policy(unsigned int cpu) +{ + return per_cpu(cpufreq_cpu_data, cpu); +} +EXPORT_SYMBOL_GPL(cpufreq_cpu_policy); + unsigned int cpufreq_generic_get(unsigned int cpu) { struct cpufreq_policy *policy = cpufreq_cpu_get_raw(cpu); diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index 0465d1e6f72a..cc894fc38971 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -203,6 +203,7 @@ struct cpufreq_freqs { #ifdef CONFIG_CPU_FREQ struct cpufreq_policy *cpufreq_cpu_get_raw(unsigned int cpu); +struct cpufreq_policy *cpufreq_cpu_policy(unsigned int cpu); struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu); void cpufreq_cpu_put(struct cpufreq_policy *policy); #else @@ -210,6 +211,10 @@ static inline struct cpufreq_policy *cpufreq_cpu_get_raw(unsigned int cpu) { return NULL; } +static inline struct cpufreq_policy *cpufreq_cpu_policy(unsigned int cpu) +{ + return NULL; +} static inline struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu) { return NULL; -- 2.33.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [REPOST PATCH v6 3/3] arm64: topology: Handle AMU FIE setup on CPU hotplug 2025-12-30 8:01 [REPOST PATCH v6 0/3] arm64: topology: Handle AMU FIE setup on CPU hotplug Lifeng Zheng 2025-12-30 8:01 ` [REPOST PATCH v6 1/3] arm64: topology: Skip already covered CPUs when setting freq source Lifeng Zheng 2025-12-30 8:01 ` [REPOST PATCH v6 2/3] cpufreq: Add new helper function returning cpufreq policy Lifeng Zheng @ 2025-12-30 8:01 ` Lifeng Zheng 2026-01-13 10:51 ` Geert Uytterhoeven 2 siblings, 1 reply; 11+ messages in thread From: Lifeng Zheng @ 2025-12-30 8:01 UTC (permalink / raw) To: catalin.marinas, will, rafael, viresh.kumar, sudeep.holla, gregkh, dakr, beata.michalska, ionela.voinescu Cc: linux-arm-kernel, linux-pm, linuxarm, jonathan.cameron, vincent.guittot, zhanjie9, lihuisong, yubowen8, zhangpengjie2, wangzhi12, linhongye, zhenglifeng1 Currently, when a cpufreq policy is created, the AMU FIE setup process checks all CPUs in the policy -- including those that are offline. If any of these CPUs are offline at that time, their AMU capability flag hasn't been verified yet, leading the check fail. As a result, AMU FIE is not enabled, even if the CPUs that are online do support it. Later, when the previously offline CPUs come online and report AMU support, there's no mechanism in place to re-enable AMU FIE for the policy. This leaves the entire frequency domain without AMU FIE, despite being eligible. Restrict the initial AMU FIE check to only those CPUs that are online at the time the policy is created, and allow CPUs that come online later to join the policy with AMU FIE enabled. Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com> Acked-by: Beata Michalska <beata.michalska@arm.com> --- arch/arm64/kernel/topology.c | 65 ++++++++++++++++++++++++++++++++++-- drivers/base/arch_topology.c | 9 ++++- 2 files changed, 71 insertions(+), 3 deletions(-) diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c index cf9bb761af3a..539b38935182 100644 --- a/arch/arm64/kernel/topology.c +++ b/arch/arm64/kernel/topology.c @@ -284,7 +284,7 @@ static int init_amu_fie_callback(struct notifier_block *nb, unsigned long val, struct cpufreq_policy *policy = data; if (val == CPUFREQ_CREATE_POLICY) - amu_fie_setup(policy->related_cpus); + amu_fie_setup(policy->cpus); /* * We don't need to handle CPUFREQ_REMOVE_POLICY event as the AMU @@ -303,10 +303,71 @@ static struct notifier_block init_amu_fie_notifier = { .notifier_call = init_amu_fie_callback, }; +static int cpuhp_topology_online(unsigned int cpu) +{ + struct cpufreq_policy *policy = cpufreq_cpu_policy(cpu); + + /* Those are cheap checks */ + + /* + * Skip this CPU if: + * - it has no cpufreq policy assigned yet, + * - no policy exists that spans CPUs with AMU counters, or + * - it was already handled. + */ + if (unlikely(!policy) || !cpumask_available(amu_fie_cpus) || + cpumask_test_cpu(cpu, amu_fie_cpus)) + return 0; + + /* + * Only proceed if all already-online CPUs in this policy + * support AMU counters. + */ + if (unlikely(!cpumask_subset(policy->cpus, amu_fie_cpus))) + return 0; + + /* + * If the new online CPU cannot pass this check, all the CPUs related to + * the same policy should be clear from amu_fie_cpus mask, otherwise they + * may use different source of the freq scale. + */ + if (!freq_counters_valid(cpu)) { + pr_warn("CPU[%u] doesn't support AMU counters\n", cpu); + topology_clear_scale_freq_source(SCALE_FREQ_SOURCE_ARCH, + policy->related_cpus); + cpumask_andnot(amu_fie_cpus, amu_fie_cpus, policy->related_cpus); + return 0; + } + + cpumask_set_cpu(cpu, amu_fie_cpus); + + topology_set_scale_freq_source(&amu_sfd, cpumask_of(cpu)); + + pr_debug("CPU[%u]: counter will be used for FIE.", cpu); + + return 0; +} + static int __init init_amu_fie(void) { - return cpufreq_register_notifier(&init_amu_fie_notifier, + int ret; + + ret = cpufreq_register_notifier(&init_amu_fie_notifier, CPUFREQ_POLICY_NOTIFIER); + if (ret) + return ret; + + ret = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN, + "arm64/topology:online", + cpuhp_topology_online, + NULL); + if (ret < 0) { + cpufreq_unregister_notifier(&init_amu_fie_notifier, + CPUFREQ_POLICY_NOTIFIER); + return ret; + } + + return 0; } core_initcall(init_amu_fie); diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c index 84ec92bff642..c0ef6ea9c111 100644 --- a/drivers/base/arch_topology.c +++ b/drivers/base/arch_topology.c @@ -34,7 +34,14 @@ EXPORT_PER_CPU_SYMBOL_GPL(capacity_freq_ref); static bool supports_scale_freq_counters(const struct cpumask *cpus) { - return cpumask_subset(cpus, &scale_freq_counters_mask); + int i; + + for_each_cpu(i, cpus) { + if (cpumask_test_cpu(i, &scale_freq_counters_mask)) + return true; + } + + return false; } bool topology_scale_freq_invariant(void) -- 2.33.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [REPOST PATCH v6 3/3] arm64: topology: Handle AMU FIE setup on CPU hotplug 2025-12-30 8:01 ` [REPOST PATCH v6 3/3] arm64: topology: Handle AMU FIE setup on CPU hotplug Lifeng Zheng @ 2026-01-13 10:51 ` Geert Uytterhoeven 2026-01-13 15:58 ` Beata Michalska 0 siblings, 1 reply; 11+ messages in thread From: Geert Uytterhoeven @ 2026-01-13 10:51 UTC (permalink / raw) To: Lifeng Zheng Cc: catalin.marinas, will, rafael, viresh.kumar, sudeep.holla, gregkh, dakr, beata.michalska, ionela.voinescu, linux-arm-kernel, linux-pm, linuxarm, jonathan.cameron, vincent.guittot, zhanjie9, lihuisong, yubowen8, zhangpengjie2, wangzhi12, linhongye, Linux-Renesas Hi Lifeng, On Tue, 30 Dec 2025 at 09:02, Lifeng Zheng <zhenglifeng1@huawei.com> wrote: > Currently, when a cpufreq policy is created, the AMU FIE setup process > checks all CPUs in the policy -- including those that are offline. If any > of these CPUs are offline at that time, their AMU capability flag hasn't > been verified yet, leading the check fail. As a result, AMU FIE is not > enabled, even if the CPUs that are online do support it. > > Later, when the previously offline CPUs come online and report AMU support, > there's no mechanism in place to re-enable AMU FIE for the policy. This > leaves the entire frequency domain without AMU FIE, despite being eligible. > > Restrict the initial AMU FIE check to only those CPUs that are online at > the time the policy is created, and allow CPUs that come online later to > join the policy with AMU FIE enabled. > > Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com> > Acked-by: Beata Michalska <beata.michalska@arm.com> Thanks for your patch, which is now commit 6fd9be0b7b2e957d ("arm64: topology: Handle AMU FIE setup on CPU hotplug") in arm64/for-next/core (next-20260107 and later). > --- a/arch/arm64/kernel/topology.c > +++ b/arch/arm64/kernel/topology.c > @@ -284,7 +284,7 @@ static int init_amu_fie_callback(struct notifier_block *nb, unsigned long val, > struct cpufreq_policy *policy = data; > > if (val == CPUFREQ_CREATE_POLICY) > - amu_fie_setup(policy->related_cpus); > + amu_fie_setup(policy->cpus); > > /* > * We don't need to handle CPUFREQ_REMOVE_POLICY event as the AMU > @@ -303,10 +303,71 @@ static struct notifier_block init_amu_fie_notifier = { > .notifier_call = init_amu_fie_callback, > }; > > +static int cpuhp_topology_online(unsigned int cpu) > +{ > + struct cpufreq_policy *policy = cpufreq_cpu_policy(cpu); > + > + /* Those are cheap checks */ > + > + /* > + * Skip this CPU if: > + * - it has no cpufreq policy assigned yet, > + * - no policy exists that spans CPUs with AMU counters, or > + * - it was already handled. > + */ > + if (unlikely(!policy) || !cpumask_available(amu_fie_cpus) || > + cpumask_test_cpu(cpu, amu_fie_cpus)) > + return 0; > + > + /* > + * Only proceed if all already-online CPUs in this policy > + * support AMU counters. > + */ > + if (unlikely(!cpumask_subset(policy->cpus, amu_fie_cpus))) > + return 0; > + > + /* > + * If the new online CPU cannot pass this check, all the CPUs related to > + * the same policy should be clear from amu_fie_cpus mask, otherwise they > + * may use different source of the freq scale. > + */ > + if (!freq_counters_valid(cpu)) { > + pr_warn("CPU[%u] doesn't support AMU counters\n", cpu); This is triggered during resume from s2ram on Renesas R-Car H3 (big.LITTLE 4x Cortex-A57 + 4x Cortex-A53), when enabling the first little core: AMU: CPU[4] doesn't support AMU counters Adding debug code: pr_info("Calling topology_clear_scale_freq_source(SCALE_FREQ_SOURCE_ARCH, %*pbl)\n", cpumask_pr_args(policy->related_cpus)); pr_info("Calling cpumask_andnot(..., %*pbl, %*pbl)\n", cpumask_pr_args(amu_fie_cpus), cpumask_pr_args(policy->related_cpus)); gives: AMU: Calling topology_clear_scale_freq_source(SCALE_FREQ_SOURCE_ARCH, 4-7) AMU: Calling cpumask_andnot(..., , 4-7) so AMU is disabled for all little cores. Since this only happens during s2ram, and not during initial CPU bring-up on boot, this looks wrong to me? > + topology_clear_scale_freq_source(SCALE_FREQ_SOURCE_ARCH, > + policy->related_cpus); > + cpumask_andnot(amu_fie_cpus, amu_fie_cpus, policy->related_cpus); > + return 0; > + } > + > + cpumask_set_cpu(cpu, amu_fie_cpus); > + > + topology_set_scale_freq_source(&amu_sfd, cpumask_of(cpu)); > + > + pr_debug("CPU[%u]: counter will be used for FIE.", cpu); > + > + return 0; > +} > + > static int __init init_amu_fie(void) > { > - return cpufreq_register_notifier(&init_amu_fie_notifier, > + int ret; > + > + ret = cpufreq_register_notifier(&init_amu_fie_notifier, > CPUFREQ_POLICY_NOTIFIER); > + if (ret) > + return ret; > + > + ret = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN, > + "arm64/topology:online", > + cpuhp_topology_online, > + NULL); > + if (ret < 0) { > + cpufreq_unregister_notifier(&init_amu_fie_notifier, > + CPUFREQ_POLICY_NOTIFIER); > + return ret; > + } > + > + return 0; > } > core_initcall(init_amu_fie); > > diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c > index 84ec92bff642..c0ef6ea9c111 100644 > --- a/drivers/base/arch_topology.c > +++ b/drivers/base/arch_topology.c > @@ -34,7 +34,14 @@ EXPORT_PER_CPU_SYMBOL_GPL(capacity_freq_ref); > > static bool supports_scale_freq_counters(const struct cpumask *cpus) > { > - return cpumask_subset(cpus, &scale_freq_counters_mask); > + int i; > + > + for_each_cpu(i, cpus) { > + if (cpumask_test_cpu(i, &scale_freq_counters_mask)) > + return true; > + } > + > + return false; > } > > bool topology_scale_freq_invariant(void) Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [REPOST PATCH v6 3/3] arm64: topology: Handle AMU FIE setup on CPU hotplug 2026-01-13 10:51 ` Geert Uytterhoeven @ 2026-01-13 15:58 ` Beata Michalska 2026-01-14 13:54 ` Geert Uytterhoeven 0 siblings, 1 reply; 11+ messages in thread From: Beata Michalska @ 2026-01-13 15:58 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Lifeng Zheng, catalin.marinas, will, rafael, viresh.kumar, sudeep.holla, gregkh, dakr, ionela.voinescu, linux-arm-kernel, linux-pm, linuxarm, jonathan.cameron, vincent.guittot, zhanjie9, lihuisong, yubowen8, zhangpengjie2, wangzhi12, linhongye, Linux-Renesas Hi Geert, On Tue, Jan 13, 2026 at 11:51:45AM +0100, Geert Uytterhoeven wrote: > Hi Lifeng, > > On Tue, 30 Dec 2025 at 09:02, Lifeng Zheng <zhenglifeng1@huawei.com> wrote: > > Currently, when a cpufreq policy is created, the AMU FIE setup process > > checks all CPUs in the policy -- including those that are offline. If any > > of these CPUs are offline at that time, their AMU capability flag hasn't > > been verified yet, leading the check fail. As a result, AMU FIE is not > > enabled, even if the CPUs that are online do support it. > > > > Later, when the previously offline CPUs come online and report AMU support, > > there's no mechanism in place to re-enable AMU FIE for the policy. This > > leaves the entire frequency domain without AMU FIE, despite being eligible. > > > > Restrict the initial AMU FIE check to only those CPUs that are online at > > the time the policy is created, and allow CPUs that come online later to > > join the policy with AMU FIE enabled. > > > > Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com> > > Acked-by: Beata Michalska <beata.michalska@arm.com> > > Thanks for your patch, which is now commit 6fd9be0b7b2e957d > ("arm64: topology: Handle AMU FIE setup on CPU hotplug") in > arm64/for-next/core (next-20260107 and later). > > > --- a/arch/arm64/kernel/topology.c > > +++ b/arch/arm64/kernel/topology.c > > @@ -284,7 +284,7 @@ static int init_amu_fie_callback(struct notifier_block *nb, unsigned long val, > > struct cpufreq_policy *policy = data; > > > > if (val == CPUFREQ_CREATE_POLICY) > > - amu_fie_setup(policy->related_cpus); > > + amu_fie_setup(policy->cpus); > > > > /* > > * We don't need to handle CPUFREQ_REMOVE_POLICY event as the AMU > > @@ -303,10 +303,71 @@ static struct notifier_block init_amu_fie_notifier = { > > .notifier_call = init_amu_fie_callback, > > }; > > > > +static int cpuhp_topology_online(unsigned int cpu) > > +{ > > + struct cpufreq_policy *policy = cpufreq_cpu_policy(cpu); > > + > > + /* Those are cheap checks */ > > + > > + /* > > + * Skip this CPU if: > > + * - it has no cpufreq policy assigned yet, > > + * - no policy exists that spans CPUs with AMU counters, or > > + * - it was already handled. > > + */ > > + if (unlikely(!policy) || !cpumask_available(amu_fie_cpus) || > > + cpumask_test_cpu(cpu, amu_fie_cpus)) > > + return 0; > > + > > + /* > > + * Only proceed if all already-online CPUs in this policy > > + * support AMU counters. > > + */ > > + if (unlikely(!cpumask_subset(policy->cpus, amu_fie_cpus))) > > + return 0; > > + > > + /* > > + * If the new online CPU cannot pass this check, all the CPUs related to > > + * the same policy should be clear from amu_fie_cpus mask, otherwise they > > + * may use different source of the freq scale. > > + */ > > + if (!freq_counters_valid(cpu)) { > > + pr_warn("CPU[%u] doesn't support AMU counters\n", cpu); > > This is triggered during resume from s2ram on Renesas R-Car H3 > (big.LITTLE 4x Cortex-A57 + 4x Cortex-A53), when enabling the first > little core: > > AMU: CPU[4] doesn't support AMU counters > > Adding debug code: > > pr_info("Calling > topology_clear_scale_freq_source(SCALE_FREQ_SOURCE_ARCH, %*pbl)\n", > cpumask_pr_args(policy->related_cpus)); > pr_info("Calling cpumask_andnot(..., %*pbl, %*pbl)\n", > cpumask_pr_args(amu_fie_cpus), cpumask_pr_args(policy->related_cpus)); > > gives: > > AMU: Calling topology_clear_scale_freq_source(SCALE_FREQ_SOURCE_ARCH, 4-7) > AMU: Calling cpumask_andnot(..., , 4-7) > > so AMU is disabled for all little cores. > > Since this only happens during s2ram, and not during initial CPU > bring-up on boot, this looks wrong to me? This does look rather surprising. If that CPU was marked as supporting AMUs at the initial bring-up it should be part of amu_fie_cpus mask, so the hp callback should bail out straight away. Would you be able to add some logs to see what that mask actually contains ? Furthermore, freq_counters_valid is logging issues when validating the counters. Would you be able to re-run it with the debug level to see what might be happening under the hood, although I am still unsure why it is even reaching that point ... --- BR Beata > > > + topology_clear_scale_freq_source(SCALE_FREQ_SOURCE_ARCH, > > + policy->related_cpus); > > + cpumask_andnot(amu_fie_cpus, amu_fie_cpus, policy->related_cpus); > > + return 0; > > + } > > + > > + cpumask_set_cpu(cpu, amu_fie_cpus); > > + > > + topology_set_scale_freq_source(&amu_sfd, cpumask_of(cpu)); > > + > > + pr_debug("CPU[%u]: counter will be used for FIE.", cpu); > > + > > + return 0; > > +} > > + > > static int __init init_amu_fie(void) > > { > > - return cpufreq_register_notifier(&init_amu_fie_notifier, > > + int ret; > > + > > + ret = cpufreq_register_notifier(&init_amu_fie_notifier, > > CPUFREQ_POLICY_NOTIFIER); > > + if (ret) > > + return ret; > > + > > + ret = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN, > > + "arm64/topology:online", > > + cpuhp_topology_online, > > + NULL); > > + if (ret < 0) { > > + cpufreq_unregister_notifier(&init_amu_fie_notifier, > > + CPUFREQ_POLICY_NOTIFIER); > > + return ret; > > + } > > + > > + return 0; > > } > > core_initcall(init_amu_fie); > > > > diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c > > index 84ec92bff642..c0ef6ea9c111 100644 > > --- a/drivers/base/arch_topology.c > > +++ b/drivers/base/arch_topology.c > > @@ -34,7 +34,14 @@ EXPORT_PER_CPU_SYMBOL_GPL(capacity_freq_ref); > > > > static bool supports_scale_freq_counters(const struct cpumask *cpus) > > { > > - return cpumask_subset(cpus, &scale_freq_counters_mask); > > + int i; > > + > > + for_each_cpu(i, cpus) { > > + if (cpumask_test_cpu(i, &scale_freq_counters_mask)) > > + return true; > > + } > > + > > + return false; > > } > > > > bool topology_scale_freq_invariant(void) > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [REPOST PATCH v6 3/3] arm64: topology: Handle AMU FIE setup on CPU hotplug 2026-01-13 15:58 ` Beata Michalska @ 2026-01-14 13:54 ` Geert Uytterhoeven 2026-01-15 2:25 ` zhenglifeng (A) 0 siblings, 1 reply; 11+ messages in thread From: Geert Uytterhoeven @ 2026-01-14 13:54 UTC (permalink / raw) To: Beata Michalska Cc: Lifeng Zheng, catalin.marinas, will, rafael, viresh.kumar, sudeep.holla, gregkh, dakr, ionela.voinescu, linux-arm-kernel, linux-pm, linuxarm, jonathan.cameron, vincent.guittot, zhanjie9, lihuisong, yubowen8, zhangpengjie2, wangzhi12, linhongye, Linux-Renesas Hi Beata, On Tue, 13 Jan 2026 at 16:58, Beata Michalska <beata.michalska@arm.com> wrote: > On Tue, Jan 13, 2026 at 11:51:45AM +0100, Geert Uytterhoeven wrote: > > On Tue, 30 Dec 2025 at 09:02, Lifeng Zheng <zhenglifeng1@huawei.com> wrote: > > > Currently, when a cpufreq policy is created, the AMU FIE setup process > > > checks all CPUs in the policy -- including those that are offline. If any > > > of these CPUs are offline at that time, their AMU capability flag hasn't > > > been verified yet, leading the check fail. As a result, AMU FIE is not > > > enabled, even if the CPUs that are online do support it. > > > > > > Later, when the previously offline CPUs come online and report AMU support, > > > there's no mechanism in place to re-enable AMU FIE for the policy. This > > > leaves the entire frequency domain without AMU FIE, despite being eligible. > > > > > > Restrict the initial AMU FIE check to only those CPUs that are online at > > > the time the policy is created, and allow CPUs that come online later to > > > join the policy with AMU FIE enabled. > > > > > > Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com> > > > Acked-by: Beata Michalska <beata.michalska@arm.com> > > > > Thanks for your patch, which is now commit 6fd9be0b7b2e957d > > ("arm64: topology: Handle AMU FIE setup on CPU hotplug") in > > arm64/for-next/core (next-20260107 and later). > > > > > --- a/arch/arm64/kernel/topology.c > > > +++ b/arch/arm64/kernel/topology.c > > > @@ -284,7 +284,7 @@ static int init_amu_fie_callback(struct notifier_block *nb, unsigned long val, > > > struct cpufreq_policy *policy = data; > > > > > > if (val == CPUFREQ_CREATE_POLICY) > > > - amu_fie_setup(policy->related_cpus); > > > + amu_fie_setup(policy->cpus); > > > > > > /* > > > * We don't need to handle CPUFREQ_REMOVE_POLICY event as the AMU > > > @@ -303,10 +303,71 @@ static struct notifier_block init_amu_fie_notifier = { > > > .notifier_call = init_amu_fie_callback, > > > }; > > > > > > +static int cpuhp_topology_online(unsigned int cpu) > > > +{ > > > + struct cpufreq_policy *policy = cpufreq_cpu_policy(cpu); > > > + > > > + /* Those are cheap checks */ > > > + > > > + /* > > > + * Skip this CPU if: > > > + * - it has no cpufreq policy assigned yet, > > > + * - no policy exists that spans CPUs with AMU counters, or > > > + * - it was already handled. > > > + */ > > > + if (unlikely(!policy) || !cpumask_available(amu_fie_cpus) || > > > + cpumask_test_cpu(cpu, amu_fie_cpus)) > > > + return 0; > > > + > > > + /* > > > + * Only proceed if all already-online CPUs in this policy > > > + * support AMU counters. > > > + */ > > > + if (unlikely(!cpumask_subset(policy->cpus, amu_fie_cpus))) > > > + return 0; > > > + > > > + /* > > > + * If the new online CPU cannot pass this check, all the CPUs related to > > > + * the same policy should be clear from amu_fie_cpus mask, otherwise they > > > + * may use different source of the freq scale. > > > + */ > > > + if (!freq_counters_valid(cpu)) { > > > + pr_warn("CPU[%u] doesn't support AMU counters\n", cpu); > > > > This is triggered during resume from s2ram on Renesas R-Car H3 > > (big.LITTLE 4x Cortex-A57 + 4x Cortex-A53), when enabling the first > > little core: > > > > AMU: CPU[4] doesn't support AMU counters > > > > Adding debug code: > > > > pr_info("Calling > > topology_clear_scale_freq_source(SCALE_FREQ_SOURCE_ARCH, %*pbl)\n", > > cpumask_pr_args(policy->related_cpus)); > > pr_info("Calling cpumask_andnot(..., %*pbl, %*pbl)\n", > > cpumask_pr_args(amu_fie_cpus), cpumask_pr_args(policy->related_cpus)); > > > > gives: > > > > AMU: Calling topology_clear_scale_freq_source(SCALE_FREQ_SOURCE_ARCH, 4-7) > > AMU: Calling cpumask_andnot(..., , 4-7) > > > > so AMU is disabled for all little cores. > > > > Since this only happens during s2ram, and not during initial CPU > > bring-up on boot, this looks wrong to me? > This does look rather surprising. If that CPU was marked as supporting AMUs at > the initial bring-up it should be part of amu_fie_cpus mask, so the hp callback > should bail out straight away. Would you be able to add some logs to see what > that mask actually contains ? > Furthermore, freq_counters_valid is logging issues when validating the counters. > Would you be able to re-run it with the debug level to see what might be > happening under the hood, although I am still unsure why it is even reaching > that point ... Adding extra debugging info, and "#define DEBUG" at the top. During boot: AMU: amu_fie_setup:260: cpus 0-3 amu_fie_cpus ^^^ empty amu_fie_cpus AMU: CPU0: counters are not supported. ^^^ pr_debug AMU: amu_fie_setup:260: cpus 4-7 amu_fie_cpus ^^^ empty amu_fie_cpus AMU: CPU4: counters are not supported. ^^^ pr_debug During resume from s2ram: AMU: cpuhp_topology_online:314: cpu 1 amu_fie_cpus AMU: cpuhp_topology_online:343: skipped (!cpumask_subset(policy->cpus, amu_fie_cpus)) AMU: cpuhp_topology_online:314: cpu 2 amu_fie_cpus AMU: cpuhp_topology_online:343: skipped (!cpumask_subset(policy->cpus, amu_fie_cpus)) AMU: cpuhp_topology_online:314: cpu 3 amu_fie_cpus AMU: cpuhp_topology_online:343: skipped (!cpumask_subset(policy->cpus, amu_fie_cpus)) AMU: cpuhp_topology_online:314: cpu 4 amu_fie_cpus AMU: CPU4: counters are not supported. ^^^ pr_debug AMU: CPU[4] doesn't support AMU counters ^^^ pr_warn AMU: Calling topology_clear_scale_freq_source(SCALE_FREQ_SOURCE_ARCH, 4-7) AMU: Calling cpumask_andnot(..., , 4-7) AMU: cpuhp_topology_online:314: cpu 5 amu_fie_cpus AMU: cpuhp_topology_online:343: skipped (!cpumask_subset(policy->cpus, amu_fie_cpus)) AMU: cpuhp_topology_online:314: cpu 6 amu_fie_cpus AMU: cpuhp_topology_online:343: skipped (!cpumask_subset(policy->cpus, amu_fie_cpus)) AMU: cpuhp_topology_online:314: cpu 7 amu_fie_cpus AMU: cpuhp_topology_online:343: skipped (!cpumask_subset(policy->cpus, amu_fie_cpus)) Hence there is no issue, as AMU is not supported at all! The confusing part is in the (absence of) logging. If AMU is not supported, freq_counters_valid() uses: pr_debug("CPU%d: counters are not supported.\n", cpu); which is typically not printed, unless DEBUG is enabled. If freq_counters_valid() failed, the new cpuhp_topology_online() uses: pr_warn("CPU[%u] doesn't support AMU counters\n", cpu); which is always printed. Given freq_counters_valid() already prints a (debug) message, I think the pr_warn() should just be removed. Do you agree, or is there still another incorrect check that should prevent getting this far? Thanks! Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [REPOST PATCH v6 3/3] arm64: topology: Handle AMU FIE setup on CPU hotplug 2026-01-14 13:54 ` Geert Uytterhoeven @ 2026-01-15 2:25 ` zhenglifeng (A) 2026-01-15 8:37 ` Geert Uytterhoeven 0 siblings, 1 reply; 11+ messages in thread From: zhenglifeng (A) @ 2026-01-15 2:25 UTC (permalink / raw) To: Geert Uytterhoeven, Beata Michalska Cc: catalin.marinas, will, rafael, viresh.kumar, sudeep.holla, gregkh, dakr, ionela.voinescu, linux-arm-kernel, linux-pm, linuxarm, jonathan.cameron, vincent.guittot, zhanjie9, lihuisong, yubowen8, zhangpengjie2, wangzhi12, linhongye, Linux-Renesas On 2026/1/14 21:54, Geert Uytterhoeven wrote: > Hi Beata, > > On Tue, 13 Jan 2026 at 16:58, Beata Michalska <beata.michalska@arm.com> wrote: >> On Tue, Jan 13, 2026 at 11:51:45AM +0100, Geert Uytterhoeven wrote: >>> On Tue, 30 Dec 2025 at 09:02, Lifeng Zheng <zhenglifeng1@huawei.com> wrote: >>>> Currently, when a cpufreq policy is created, the AMU FIE setup process >>>> checks all CPUs in the policy -- including those that are offline. If any >>>> of these CPUs are offline at that time, their AMU capability flag hasn't >>>> been verified yet, leading the check fail. As a result, AMU FIE is not >>>> enabled, even if the CPUs that are online do support it. >>>> >>>> Later, when the previously offline CPUs come online and report AMU support, >>>> there's no mechanism in place to re-enable AMU FIE for the policy. This >>>> leaves the entire frequency domain without AMU FIE, despite being eligible. >>>> >>>> Restrict the initial AMU FIE check to only those CPUs that are online at >>>> the time the policy is created, and allow CPUs that come online later to >>>> join the policy with AMU FIE enabled. >>>> >>>> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com> >>>> Acked-by: Beata Michalska <beata.michalska@arm.com> >>> >>> Thanks for your patch, which is now commit 6fd9be0b7b2e957d >>> ("arm64: topology: Handle AMU FIE setup on CPU hotplug") in >>> arm64/for-next/core (next-20260107 and later). >>> >>>> --- a/arch/arm64/kernel/topology.c >>>> +++ b/arch/arm64/kernel/topology.c >>>> @@ -284,7 +284,7 @@ static int init_amu_fie_callback(struct notifier_block *nb, unsigned long val, >>>> struct cpufreq_policy *policy = data; >>>> >>>> if (val == CPUFREQ_CREATE_POLICY) >>>> - amu_fie_setup(policy->related_cpus); >>>> + amu_fie_setup(policy->cpus); >>>> >>>> /* >>>> * We don't need to handle CPUFREQ_REMOVE_POLICY event as the AMU >>>> @@ -303,10 +303,71 @@ static struct notifier_block init_amu_fie_notifier = { >>>> .notifier_call = init_amu_fie_callback, >>>> }; >>>> >>>> +static int cpuhp_topology_online(unsigned int cpu) >>>> +{ >>>> + struct cpufreq_policy *policy = cpufreq_cpu_policy(cpu); >>>> + >>>> + /* Those are cheap checks */ >>>> + >>>> + /* >>>> + * Skip this CPU if: >>>> + * - it has no cpufreq policy assigned yet, >>>> + * - no policy exists that spans CPUs with AMU counters, or >>>> + * - it was already handled. >>>> + */ >>>> + if (unlikely(!policy) || !cpumask_available(amu_fie_cpus) || >>>> + cpumask_test_cpu(cpu, amu_fie_cpus)) >>>> + return 0; >>>> + >>>> + /* >>>> + * Only proceed if all already-online CPUs in this policy >>>> + * support AMU counters. >>>> + */ >>>> + if (unlikely(!cpumask_subset(policy->cpus, amu_fie_cpus))) >>>> + return 0; >>>> + >>>> + /* >>>> + * If the new online CPU cannot pass this check, all the CPUs related to >>>> + * the same policy should be clear from amu_fie_cpus mask, otherwise they >>>> + * may use different source of the freq scale. >>>> + */ >>>> + if (!freq_counters_valid(cpu)) { >>>> + pr_warn("CPU[%u] doesn't support AMU counters\n", cpu); >>> >>> This is triggered during resume from s2ram on Renesas R-Car H3 >>> (big.LITTLE 4x Cortex-A57 + 4x Cortex-A53), when enabling the first >>> little core: >>> >>> AMU: CPU[4] doesn't support AMU counters >>> >>> Adding debug code: >>> >>> pr_info("Calling >>> topology_clear_scale_freq_source(SCALE_FREQ_SOURCE_ARCH, %*pbl)\n", >>> cpumask_pr_args(policy->related_cpus)); >>> pr_info("Calling cpumask_andnot(..., %*pbl, %*pbl)\n", >>> cpumask_pr_args(amu_fie_cpus), cpumask_pr_args(policy->related_cpus)); >>> >>> gives: >>> >>> AMU: Calling topology_clear_scale_freq_source(SCALE_FREQ_SOURCE_ARCH, 4-7) >>> AMU: Calling cpumask_andnot(..., , 4-7) >>> >>> so AMU is disabled for all little cores. >>> >>> Since this only happens during s2ram, and not during initial CPU >>> bring-up on boot, this looks wrong to me? >> This does look rather surprising. If that CPU was marked as supporting AMUs at >> the initial bring-up it should be part of amu_fie_cpus mask, so the hp callback >> should bail out straight away. Would you be able to add some logs to see what >> that mask actually contains ? >> Furthermore, freq_counters_valid is logging issues when validating the counters. >> Would you be able to re-run it with the debug level to see what might be >> happening under the hood, although I am still unsure why it is even reaching >> that point ... > > Adding extra debugging info, and "#define DEBUG" at the top. > > During boot: > > AMU: amu_fie_setup:260: cpus 0-3 amu_fie_cpus > ^^^ empty amu_fie_cpus > AMU: CPU0: counters are not supported. > ^^^ pr_debug > AMU: amu_fie_setup:260: cpus 4-7 amu_fie_cpus > ^^^ empty amu_fie_cpus > AMU: CPU4: counters are not supported. > ^^^ pr_debug > > During resume from s2ram: > > AMU: cpuhp_topology_online:314: cpu 1 amu_fie_cpus > AMU: cpuhp_topology_online:343: skipped > (!cpumask_subset(policy->cpus, amu_fie_cpus)) > AMU: cpuhp_topology_online:314: cpu 2 amu_fie_cpus > AMU: cpuhp_topology_online:343: skipped > (!cpumask_subset(policy->cpus, amu_fie_cpus)) > AMU: cpuhp_topology_online:314: cpu 3 amu_fie_cpus > AMU: cpuhp_topology_online:343: skipped > (!cpumask_subset(policy->cpus, amu_fie_cpus)) > AMU: cpuhp_topology_online:314: cpu 4 amu_fie_cpus > AMU: CPU4: counters are not supported. > ^^^ pr_debug > AMU: CPU[4] doesn't support AMU counters > ^^^ pr_warn > AMU: Calling topology_clear_scale_freq_source(SCALE_FREQ_SOURCE_ARCH, 4-7) > AMU: Calling cpumask_andnot(..., , 4-7) Something strange here. If AMU is not supported at all, amu_fie_cpus should never be available and cpuhp_topology_online() should return in the first 'if'. Why it runs this far? > AMU: cpuhp_topology_online:314: cpu 5 amu_fie_cpus > AMU: cpuhp_topology_online:343: skipped > (!cpumask_subset(policy->cpus, amu_fie_cpus)) > AMU: cpuhp_topology_online:314: cpu 6 amu_fie_cpus > AMU: cpuhp_topology_online:343: skipped > (!cpumask_subset(policy->cpus, amu_fie_cpus)) > AMU: cpuhp_topology_online:314: cpu 7 amu_fie_cpus > AMU: cpuhp_topology_online:343: skipped > (!cpumask_subset(policy->cpus, amu_fie_cpus)) > > Hence there is no issue, as AMU is not supported at all! > > The confusing part is in the (absence of) logging. > If AMU is not supported, freq_counters_valid() uses: > > pr_debug("CPU%d: counters are not supported.\n", cpu); > > which is typically not printed, unless DEBUG is enabled. > > If freq_counters_valid() failed, the new cpuhp_topology_online() uses: > > pr_warn("CPU[%u] doesn't support AMU counters\n", cpu); > > which is always printed. > > Given freq_counters_valid() already prints a (debug) message, I think > the pr_warn() should just be removed. Do you agree, or is there still > another incorrect check that should prevent getting this far? I'm OK with removing it. > > Thanks! > > Gr{oetje,eeting}s, > > Geert > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [REPOST PATCH v6 3/3] arm64: topology: Handle AMU FIE setup on CPU hotplug 2026-01-15 2:25 ` zhenglifeng (A) @ 2026-01-15 8:37 ` Geert Uytterhoeven 2026-01-15 8:53 ` Geert Uytterhoeven 2026-01-15 12:47 ` zhenglifeng (A) 0 siblings, 2 replies; 11+ messages in thread From: Geert Uytterhoeven @ 2026-01-15 8:37 UTC (permalink / raw) To: zhenglifeng (A) Cc: Beata Michalska, catalin.marinas, will, rafael, viresh.kumar, sudeep.holla, gregkh, dakr, ionela.voinescu, linux-arm-kernel, linux-pm, linuxarm, jonathan.cameron, vincent.guittot, zhanjie9, lihuisong, yubowen8, zhangpengjie2, wangzhi12, linhongye, Linux-Renesas Hi Lifeng, On Thu, 15 Jan 2026 at 03:25, zhenglifeng (A) <zhenglifeng1@huawei.com> wrote: > On 2026/1/14 21:54, Geert Uytterhoeven wrote: > > On Tue, 13 Jan 2026 at 16:58, Beata Michalska <beata.michalska@arm.com> wrote: > >> On Tue, Jan 13, 2026 at 11:51:45AM +0100, Geert Uytterhoeven wrote: > >>> On Tue, 30 Dec 2025 at 09:02, Lifeng Zheng <zhenglifeng1@huawei.com> wrote: > >>>> Currently, when a cpufreq policy is created, the AMU FIE setup process > >>>> checks all CPUs in the policy -- including those that are offline. If any > >>>> of these CPUs are offline at that time, their AMU capability flag hasn't > >>>> been verified yet, leading the check fail. As a result, AMU FIE is not > >>>> enabled, even if the CPUs that are online do support it. > >>>> > >>>> Later, when the previously offline CPUs come online and report AMU support, > >>>> there's no mechanism in place to re-enable AMU FIE for the policy. This > >>>> leaves the entire frequency domain without AMU FIE, despite being eligible. > >>>> > >>>> Restrict the initial AMU FIE check to only those CPUs that are online at > >>>> the time the policy is created, and allow CPUs that come online later to > >>>> join the policy with AMU FIE enabled. > >>>> > >>>> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com> > >>>> Acked-by: Beata Michalska <beata.michalska@arm.com> > >>> > >>> Thanks for your patch, which is now commit 6fd9be0b7b2e957d > >>> ("arm64: topology: Handle AMU FIE setup on CPU hotplug") in > >>> arm64/for-next/core (next-20260107 and later). > >>> > >>>> --- a/arch/arm64/kernel/topology.c > >>>> +++ b/arch/arm64/kernel/topology.c > >>>> @@ -284,7 +284,7 @@ static int init_amu_fie_callback(struct notifier_block *nb, unsigned long val, > >>>> struct cpufreq_policy *policy = data; > >>>> > >>>> if (val == CPUFREQ_CREATE_POLICY) > >>>> - amu_fie_setup(policy->related_cpus); > >>>> + amu_fie_setup(policy->cpus); > >>>> > >>>> /* > >>>> * We don't need to handle CPUFREQ_REMOVE_POLICY event as the AMU > >>>> @@ -303,10 +303,71 @@ static struct notifier_block init_amu_fie_notifier = { > >>>> .notifier_call = init_amu_fie_callback, > >>>> }; > >>>> > >>>> +static int cpuhp_topology_online(unsigned int cpu) > >>>> +{ > >>>> + struct cpufreq_policy *policy = cpufreq_cpu_policy(cpu); > >>>> + > >>>> + /* Those are cheap checks */ > >>>> + > >>>> + /* > >>>> + * Skip this CPU if: > >>>> + * - it has no cpufreq policy assigned yet, > >>>> + * - no policy exists that spans CPUs with AMU counters, or > >>>> + * - it was already handled. > >>>> + */ > >>>> + if (unlikely(!policy) || !cpumask_available(amu_fie_cpus) || > >>>> + cpumask_test_cpu(cpu, amu_fie_cpus)) > >>>> + return 0; > >>>> + > >>>> + /* > >>>> + * Only proceed if all already-online CPUs in this policy > >>>> + * support AMU counters. > >>>> + */ > >>>> + if (unlikely(!cpumask_subset(policy->cpus, amu_fie_cpus))) > >>>> + return 0; > >>>> + > >>>> + /* > >>>> + * If the new online CPU cannot pass this check, all the CPUs related to > >>>> + * the same policy should be clear from amu_fie_cpus mask, otherwise they > >>>> + * may use different source of the freq scale. > >>>> + */ > >>>> + if (!freq_counters_valid(cpu)) { > >>>> + pr_warn("CPU[%u] doesn't support AMU counters\n", cpu); > >>> > >>> This is triggered during resume from s2ram on Renesas R-Car H3 > >>> (big.LITTLE 4x Cortex-A57 + 4x Cortex-A53), when enabling the first > >>> little core: > >>> > >>> AMU: CPU[4] doesn't support AMU counters > >>> > >>> Adding debug code: > >>> > >>> pr_info("Calling > >>> topology_clear_scale_freq_source(SCALE_FREQ_SOURCE_ARCH, %*pbl)\n", > >>> cpumask_pr_args(policy->related_cpus)); > >>> pr_info("Calling cpumask_andnot(..., %*pbl, %*pbl)\n", > >>> cpumask_pr_args(amu_fie_cpus), cpumask_pr_args(policy->related_cpus)); > >>> > >>> gives: > >>> > >>> AMU: Calling topology_clear_scale_freq_source(SCALE_FREQ_SOURCE_ARCH, 4-7) > >>> AMU: Calling cpumask_andnot(..., , 4-7) > >>> > >>> so AMU is disabled for all little cores. > >>> > >>> Since this only happens during s2ram, and not during initial CPU > >>> bring-up on boot, this looks wrong to me? > >> This does look rather surprising. If that CPU was marked as supporting AMUs at > >> the initial bring-up it should be part of amu_fie_cpus mask, so the hp callback > >> should bail out straight away. Would you be able to add some logs to see what > >> that mask actually contains ? > >> Furthermore, freq_counters_valid is logging issues when validating the counters. > >> Would you be able to re-run it with the debug level to see what might be > >> happening under the hood, although I am still unsure why it is even reaching > >> that point ... > > > > Adding extra debugging info, and "#define DEBUG" at the top. > > > > During boot: > > > > AMU: amu_fie_setup:260: cpus 0-3 amu_fie_cpus > > ^^^ empty amu_fie_cpus > > AMU: CPU0: counters are not supported. > > ^^^ pr_debug > > AMU: amu_fie_setup:260: cpus 4-7 amu_fie_cpus > > ^^^ empty amu_fie_cpus > > AMU: CPU4: counters are not supported. > > ^^^ pr_debug > > > > During resume from s2ram: > > > > AMU: cpuhp_topology_online:314: cpu 1 amu_fie_cpus > > AMU: cpuhp_topology_online:343: skipped > > (!cpumask_subset(policy->cpus, amu_fie_cpus)) > > AMU: cpuhp_topology_online:314: cpu 2 amu_fie_cpus > > AMU: cpuhp_topology_online:343: skipped > > (!cpumask_subset(policy->cpus, amu_fie_cpus)) > > AMU: cpuhp_topology_online:314: cpu 3 amu_fie_cpus > > AMU: cpuhp_topology_online:343: skipped > > (!cpumask_subset(policy->cpus, amu_fie_cpus)) > > AMU: cpuhp_topology_online:314: cpu 4 amu_fie_cpus > > AMU: CPU4: counters are not supported. > > ^^^ pr_debug > > AMU: CPU[4] doesn't support AMU counters > > ^^^ pr_warn > > AMU: Calling topology_clear_scale_freq_source(SCALE_FREQ_SOURCE_ARCH, 4-7) > > AMU: Calling cpumask_andnot(..., , 4-7) > > Something strange here. If AMU is not supported at all, amu_fie_cpus should > never be available and cpuhp_topology_online() should return in the first > 'if'. Why it runs this far? You mean the "!cpumask_available(amu_fie_cpus)" test? include/linux/cpumask.h: #ifdef CONFIG_CPUMASK_OFFSTACK static __always_inline bool cpumask_available(cpumask_var_t mask) { return mask != NULL; } #else static __always_inline bool cpumask_available(cpumask_var_t mask) { return true; } #endif /* CONFIG_CPUMASK_OFFSTACK */ include/linux/cpumask_types.h: #ifdef CONFIG_CPUMASK_OFFSTACK typedef struct cpumask *cpumask_var_t; #else typedef struct cpumask cpumask_var_t[1]; #endif /* CONFIG_CPUMASK_OFFSTACK */ So if CONFIG_CPUMASK_OFFSTACK is not enabled, it always returns true. arch/arm64/Kconfig: config ARM64 [...] select CPUMASK_OFFSTACK if NR_CPUS > 256 > > AMU: cpuhp_topology_online:314: cpu 5 amu_fie_cpus > > AMU: cpuhp_topology_online:343: skipped > > (!cpumask_subset(policy->cpus, amu_fie_cpus)) > > AMU: cpuhp_topology_online:314: cpu 6 amu_fie_cpus > > AMU: cpuhp_topology_online:343: skipped > > (!cpumask_subset(policy->cpus, amu_fie_cpus)) > > AMU: cpuhp_topology_online:314: cpu 7 amu_fie_cpus > > AMU: cpuhp_topology_online:343: skipped > > (!cpumask_subset(policy->cpus, amu_fie_cpus)) > > > > Hence there is no issue, as AMU is not supported at all! > > > > The confusing part is in the (absence of) logging. > > If AMU is not supported, freq_counters_valid() uses: > > > > pr_debug("CPU%d: counters are not supported.\n", cpu); > > > > which is typically not printed, unless DEBUG is enabled. > > > > If freq_counters_valid() failed, the new cpuhp_topology_online() uses: > > > > pr_warn("CPU[%u] doesn't support AMU counters\n", cpu); > > > > which is always printed. > > > > Given freq_counters_valid() already prints a (debug) message, I think > > the pr_warn() should just be removed. Do you agree, or is there still > > another incorrect check that should prevent getting this far? > > I'm OK with removing it. OK, I will send a patch. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [REPOST PATCH v6 3/3] arm64: topology: Handle AMU FIE setup on CPU hotplug 2026-01-15 8:37 ` Geert Uytterhoeven @ 2026-01-15 8:53 ` Geert Uytterhoeven 2026-01-15 12:47 ` zhenglifeng (A) 1 sibling, 0 replies; 11+ messages in thread From: Geert Uytterhoeven @ 2026-01-15 8:53 UTC (permalink / raw) To: zhenglifeng (A) Cc: Beata Michalska, catalin.marinas, will, rafael, viresh.kumar, sudeep.holla, gregkh, dakr, ionela.voinescu, linux-arm-kernel, linux-pm, linuxarm, jonathan.cameron, vincent.guittot, zhanjie9, lihuisong, yubowen8, zhangpengjie2, wangzhi12, linhongye, Linux-Renesas On Thu, 15 Jan 2026 at 09:37, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > On Thu, 15 Jan 2026 at 03:25, zhenglifeng (A) <zhenglifeng1@huawei.com> wrote: > > On 2026/1/14 21:54, Geert Uytterhoeven wrote: > > > The confusing part is in the (absence of) logging. > > > If AMU is not supported, freq_counters_valid() uses: > > > > > > pr_debug("CPU%d: counters are not supported.\n", cpu); > > > > > > which is typically not printed, unless DEBUG is enabled. > > > > > > If freq_counters_valid() failed, the new cpuhp_topology_online() uses: > > > > > > pr_warn("CPU[%u] doesn't support AMU counters\n", cpu); > > > > > > which is always printed. > > > > > > Given freq_counters_valid() already prints a (debug) message, I think > > > the pr_warn() should just be removed. Do you agree, or is there still > > > another incorrect check that should prevent getting this far? > > > > I'm OK with removing it. > > OK, I will send a patch. https://lore.kernel.org/a8dbf49bfa44a6809fa4f34b918516847dc14460.1768466986.git.geert+renesas@glider.be/ Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [REPOST PATCH v6 3/3] arm64: topology: Handle AMU FIE setup on CPU hotplug 2026-01-15 8:37 ` Geert Uytterhoeven 2026-01-15 8:53 ` Geert Uytterhoeven @ 2026-01-15 12:47 ` zhenglifeng (A) 1 sibling, 0 replies; 11+ messages in thread From: zhenglifeng (A) @ 2026-01-15 12:47 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Beata Michalska, catalin.marinas, will, rafael, viresh.kumar, sudeep.holla, gregkh, dakr, ionela.voinescu, linux-arm-kernel, linux-pm, linuxarm, jonathan.cameron, vincent.guittot, zhanjie9, lihuisong, yubowen8, zhangpengjie2, wangzhi12, linhongye, Linux-Renesas On 2026/1/15 16:37, Geert Uytterhoeven wrote: > Hi Lifeng, > > On Thu, 15 Jan 2026 at 03:25, zhenglifeng (A) <zhenglifeng1@huawei.com> wrote: >> On 2026/1/14 21:54, Geert Uytterhoeven wrote: >>> On Tue, 13 Jan 2026 at 16:58, Beata Michalska <beata.michalska@arm.com> wrote: >>>> On Tue, Jan 13, 2026 at 11:51:45AM +0100, Geert Uytterhoeven wrote: >>>>> On Tue, 30 Dec 2025 at 09:02, Lifeng Zheng <zhenglifeng1@huawei.com> wrote: >>>>>> Currently, when a cpufreq policy is created, the AMU FIE setup process >>>>>> checks all CPUs in the policy -- including those that are offline. If any >>>>>> of these CPUs are offline at that time, their AMU capability flag hasn't >>>>>> been verified yet, leading the check fail. As a result, AMU FIE is not >>>>>> enabled, even if the CPUs that are online do support it. >>>>>> >>>>>> Later, when the previously offline CPUs come online and report AMU support, >>>>>> there's no mechanism in place to re-enable AMU FIE for the policy. This >>>>>> leaves the entire frequency domain without AMU FIE, despite being eligible. >>>>>> >>>>>> Restrict the initial AMU FIE check to only those CPUs that are online at >>>>>> the time the policy is created, and allow CPUs that come online later to >>>>>> join the policy with AMU FIE enabled. >>>>>> >>>>>> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com> >>>>>> Acked-by: Beata Michalska <beata.michalska@arm.com> >>>>> >>>>> Thanks for your patch, which is now commit 6fd9be0b7b2e957d >>>>> ("arm64: topology: Handle AMU FIE setup on CPU hotplug") in >>>>> arm64/for-next/core (next-20260107 and later). >>>>> >>>>>> --- a/arch/arm64/kernel/topology.c >>>>>> +++ b/arch/arm64/kernel/topology.c >>>>>> @@ -284,7 +284,7 @@ static int init_amu_fie_callback(struct notifier_block *nb, unsigned long val, >>>>>> struct cpufreq_policy *policy = data; >>>>>> >>>>>> if (val == CPUFREQ_CREATE_POLICY) >>>>>> - amu_fie_setup(policy->related_cpus); >>>>>> + amu_fie_setup(policy->cpus); >>>>>> >>>>>> /* >>>>>> * We don't need to handle CPUFREQ_REMOVE_POLICY event as the AMU >>>>>> @@ -303,10 +303,71 @@ static struct notifier_block init_amu_fie_notifier = { >>>>>> .notifier_call = init_amu_fie_callback, >>>>>> }; >>>>>> >>>>>> +static int cpuhp_topology_online(unsigned int cpu) >>>>>> +{ >>>>>> + struct cpufreq_policy *policy = cpufreq_cpu_policy(cpu); >>>>>> + >>>>>> + /* Those are cheap checks */ >>>>>> + >>>>>> + /* >>>>>> + * Skip this CPU if: >>>>>> + * - it has no cpufreq policy assigned yet, >>>>>> + * - no policy exists that spans CPUs with AMU counters, or >>>>>> + * - it was already handled. >>>>>> + */ >>>>>> + if (unlikely(!policy) || !cpumask_available(amu_fie_cpus) || >>>>>> + cpumask_test_cpu(cpu, amu_fie_cpus)) >>>>>> + return 0; >>>>>> + >>>>>> + /* >>>>>> + * Only proceed if all already-online CPUs in this policy >>>>>> + * support AMU counters. >>>>>> + */ >>>>>> + if (unlikely(!cpumask_subset(policy->cpus, amu_fie_cpus))) >>>>>> + return 0; >>>>>> + >>>>>> + /* >>>>>> + * If the new online CPU cannot pass this check, all the CPUs related to >>>>>> + * the same policy should be clear from amu_fie_cpus mask, otherwise they >>>>>> + * may use different source of the freq scale. >>>>>> + */ >>>>>> + if (!freq_counters_valid(cpu)) { >>>>>> + pr_warn("CPU[%u] doesn't support AMU counters\n", cpu); >>>>> >>>>> This is triggered during resume from s2ram on Renesas R-Car H3 >>>>> (big.LITTLE 4x Cortex-A57 + 4x Cortex-A53), when enabling the first >>>>> little core: >>>>> >>>>> AMU: CPU[4] doesn't support AMU counters >>>>> >>>>> Adding debug code: >>>>> >>>>> pr_info("Calling >>>>> topology_clear_scale_freq_source(SCALE_FREQ_SOURCE_ARCH, %*pbl)\n", >>>>> cpumask_pr_args(policy->related_cpus)); >>>>> pr_info("Calling cpumask_andnot(..., %*pbl, %*pbl)\n", >>>>> cpumask_pr_args(amu_fie_cpus), cpumask_pr_args(policy->related_cpus)); >>>>> >>>>> gives: >>>>> >>>>> AMU: Calling topology_clear_scale_freq_source(SCALE_FREQ_SOURCE_ARCH, 4-7) >>>>> AMU: Calling cpumask_andnot(..., , 4-7) >>>>> >>>>> so AMU is disabled for all little cores. >>>>> >>>>> Since this only happens during s2ram, and not during initial CPU >>>>> bring-up on boot, this looks wrong to me? >>>> This does look rather surprising. If that CPU was marked as supporting AMUs at >>>> the initial bring-up it should be part of amu_fie_cpus mask, so the hp callback >>>> should bail out straight away. Would you be able to add some logs to see what >>>> that mask actually contains ? >>>> Furthermore, freq_counters_valid is logging issues when validating the counters. >>>> Would you be able to re-run it with the debug level to see what might be >>>> happening under the hood, although I am still unsure why it is even reaching >>>> that point ... >>> >>> Adding extra debugging info, and "#define DEBUG" at the top. >>> >>> During boot: >>> >>> AMU: amu_fie_setup:260: cpus 0-3 amu_fie_cpus >>> ^^^ empty amu_fie_cpus >>> AMU: CPU0: counters are not supported. >>> ^^^ pr_debug >>> AMU: amu_fie_setup:260: cpus 4-7 amu_fie_cpus >>> ^^^ empty amu_fie_cpus >>> AMU: CPU4: counters are not supported. >>> ^^^ pr_debug >>> >>> During resume from s2ram: >>> >>> AMU: cpuhp_topology_online:314: cpu 1 amu_fie_cpus >>> AMU: cpuhp_topology_online:343: skipped >>> (!cpumask_subset(policy->cpus, amu_fie_cpus)) >>> AMU: cpuhp_topology_online:314: cpu 2 amu_fie_cpus >>> AMU: cpuhp_topology_online:343: skipped >>> (!cpumask_subset(policy->cpus, amu_fie_cpus)) >>> AMU: cpuhp_topology_online:314: cpu 3 amu_fie_cpus >>> AMU: cpuhp_topology_online:343: skipped >>> (!cpumask_subset(policy->cpus, amu_fie_cpus)) >>> AMU: cpuhp_topology_online:314: cpu 4 amu_fie_cpus >>> AMU: CPU4: counters are not supported. >>> ^^^ pr_debug >>> AMU: CPU[4] doesn't support AMU counters >>> ^^^ pr_warn >>> AMU: Calling topology_clear_scale_freq_source(SCALE_FREQ_SOURCE_ARCH, 4-7) >>> AMU: Calling cpumask_andnot(..., , 4-7) >> >> Something strange here. If AMU is not supported at all, amu_fie_cpus should >> never be available and cpuhp_topology_online() should return in the first >> 'if'. Why it runs this far? > > You mean the "!cpumask_available(amu_fie_cpus)" test? > > include/linux/cpumask.h: > > #ifdef CONFIG_CPUMASK_OFFSTACK > static __always_inline bool cpumask_available(cpumask_var_t mask) > { > return mask != NULL; > } > #else > static __always_inline bool cpumask_available(cpumask_var_t mask) > { > return true; > } > #endif /* CONFIG_CPUMASK_OFFSTACK */ > > include/linux/cpumask_types.h: > > #ifdef CONFIG_CPUMASK_OFFSTACK > typedef struct cpumask *cpumask_var_t; > #else > typedef struct cpumask cpumask_var_t[1]; > #endif /* CONFIG_CPUMASK_OFFSTACK */ > > So if CONFIG_CPUMASK_OFFSTACK is not enabled, it always returns true. > > arch/arm64/Kconfig: > > config ARM64 > [...] > select CPUMASK_OFFSTACK if NR_CPUS > 256 OK. Then nothing's wrong here. Thanks for explanation. > >>> AMU: cpuhp_topology_online:314: cpu 5 amu_fie_cpus >>> AMU: cpuhp_topology_online:343: skipped >>> (!cpumask_subset(policy->cpus, amu_fie_cpus)) >>> AMU: cpuhp_topology_online:314: cpu 6 amu_fie_cpus >>> AMU: cpuhp_topology_online:343: skipped >>> (!cpumask_subset(policy->cpus, amu_fie_cpus)) >>> AMU: cpuhp_topology_online:314: cpu 7 amu_fie_cpus >>> AMU: cpuhp_topology_online:343: skipped >>> (!cpumask_subset(policy->cpus, amu_fie_cpus)) >>> >>> Hence there is no issue, as AMU is not supported at all! >>> >>> The confusing part is in the (absence of) logging. >>> If AMU is not supported, freq_counters_valid() uses: >>> >>> pr_debug("CPU%d: counters are not supported.\n", cpu); >>> >>> which is typically not printed, unless DEBUG is enabled. >>> >>> If freq_counters_valid() failed, the new cpuhp_topology_online() uses: >>> >>> pr_warn("CPU[%u] doesn't support AMU counters\n", cpu); >>> >>> which is always printed. >>> >>> Given freq_counters_valid() already prints a (debug) message, I think >>> the pr_warn() should just be removed. Do you agree, or is there still >>> another incorrect check that should prevent getting this far? >> >> I'm OK with removing it. > > OK, I will send a patch. > > Gr{oetje,eeting}s, > > Geert > ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-01-15 12:48 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-12-30 8:01 [REPOST PATCH v6 0/3] arm64: topology: Handle AMU FIE setup on CPU hotplug Lifeng Zheng 2025-12-30 8:01 ` [REPOST PATCH v6 1/3] arm64: topology: Skip already covered CPUs when setting freq source Lifeng Zheng 2025-12-30 8:01 ` [REPOST PATCH v6 2/3] cpufreq: Add new helper function returning cpufreq policy Lifeng Zheng 2025-12-30 8:01 ` [REPOST PATCH v6 3/3] arm64: topology: Handle AMU FIE setup on CPU hotplug Lifeng Zheng 2026-01-13 10:51 ` Geert Uytterhoeven 2026-01-13 15:58 ` Beata Michalska 2026-01-14 13:54 ` Geert Uytterhoeven 2026-01-15 2:25 ` zhenglifeng (A) 2026-01-15 8:37 ` Geert Uytterhoeven 2026-01-15 8:53 ` Geert Uytterhoeven 2026-01-15 12:47 ` zhenglifeng (A)
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox