public inbox for linux-pm@vger.kernel.org
 help / color / mirror / Atom feed
* [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:47 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