Re: [PATCH v2] arm64: topology: Setup AMU FIE for online CPUs only

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

Hi Lifeng,

Apologies for late reply.

On Fri, Jul 25, 2025 at 06:28:13PM +0800, Lifeng Zheng wrote:
> When boot with maxcpu=1 restrict, and LPI(Low Power Idle States) is on,
> only CPU0 will go online. The support AMU flag of CPU0 will be set but the
> flags of other CPUs will not. This will cause AMU FIE set up fail for CPU0
> when it shares a cpufreq policy with other CPU(s). After that, when other
> CPUs are finally online and the support AMU flags of them are set, they'll
> never have a chance to set up AMU FIE, even though they're eligible.
> 
> To solve this problem, the process of setting up AMU FIE needs to be
> modified as follows:
> 
> 1. Set up AMU FIE only for the online CPUs.
> 
> 2. Try to set up AMU FIE each time a CPU goes online and do the
> freq_counters_valid() check for all the online CPUs share the same policy.
> If this check fails, clear scale freq source of these CPUs, in case they
> use different source of the freq scale.
> 
> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
> ---
>  arch/arm64/kernel/topology.c | 49 ++++++++++++++++++++++++++++++++----
>  1 file changed, 44 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
> index 5d07ee85bdae..d578c496d457 100644
> --- a/arch/arm64/kernel/topology.c
> +++ b/arch/arm64/kernel/topology.c
> @@ -357,12 +357,15 @@ static void amu_fie_setup(const struct cpumask *cpus)
>  
>  	/* We are already set since the last insmod of cpufreq driver */
>  	if (cpumask_available(amu_fie_cpus) &&
> -	    unlikely(cpumask_subset(cpus, amu_fie_cpus)))
> +	    cpumask_subset(cpus, amu_fie_cpus))
>  		return;
>  
> -	for_each_cpu(cpu, cpus)
> -		if (!freq_counters_valid(cpu))
> +	for_each_cpu(cpu, cpus) {
> +		if (!freq_counters_valid(cpu)) {
> +			topology_clear_scale_freq_source(SCALE_FREQ_SOURCE_ARCH, cpus);
>  			return;
> +		}
> +	}
>  
>  	if (!cpumask_available(amu_fie_cpus) &&
>  	    !zalloc_cpumask_var(&amu_fie_cpus, GFP_KERNEL)) {
> @@ -385,7 +388,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);
Right, so this will only work for the AMU side of things. The cpufreq core still
considers `related_cpus` which should be aligned (if possible).
>  
>  	/*
>  	 * We don't need to handle CPUFREQ_REMOVE_POLICY event as the AMU
> @@ -404,10 +407,46 @@ 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 __free(put_cpufreq_policy);
> +	cpumask_var_t cpus_to_set;
> +
> +	if (!zalloc_cpumask_var(&cpus_to_set, GFP_KERNEL))
> +		return -ENOMEM;
> +
> +	cpumask_copy(cpus_to_set, cpumask_of(cpu));
> +
> +	policy = cpufreq_cpu_get(cpu);
> +	if (policy) {
> +		cpumask_or(cpus_to_set, cpus_to_set, policy->cpus);
This should be available via `amu_fie_cpus` mask (as of subset).
So it should be enough to test only the CPU at hand?
Additionally there is no tracking whether that CPU hasn't been verified already.
> +		amu_fie_setup(cpus_to_set);
> +	}
> +
> +	free_cpumask_var(cpus_to_set);
> +	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);
We should check whether we really have to set it up, as of all CPUs are already
online).
> +	if (ret < 0) {
> +		cpufreq_unregister_notifier(&init_amu_fie_notifier,
> +					    CPUFREQ_POLICY_NOTIFIER);
> +		return ret;
> +	}
> +
> +	return 0;
>  }
>  core_initcall(init_amu_fie);
>  
> -- 
> 2.33.0
>