Re: [REPOST PATCH v6 3/3] arm64: topology: Handle AMU FIE setup on CPU hotplug

[Beata Michalska <beata.michalska@arm.com>]

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