| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <82b7c3e8-2c3e-4a71-9898-446f5d2dd39a@huawei.com>
Date: Sat, 26 Jul 2025 11:05:36 +0800
From: "zhenglifeng (A)" <zhenglifeng1@...wei.com>
To: Sudeep Holla <sudeep.holla@....com>
CC: <catalin.marinas@....com>, <will@...nel.org>, <beata.michalska@....com>,
<linux-arm-kernel@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
<linuxarm@...wei.com>, <jonathan.cameron@...wei.com>,
<viresh.kumar@...aro.org>, <vincent.guittot@...aro.org>,
<yangyicong@...ilicon.com>, <zhanjie9@...ilicon.com>, <lihuisong@...wei.com>,
<yubowen8@...wei.com>, <linhongye@...artners.com>
Subject: Re: [PATCH v2] arm64: topology: Setup AMU FIE for online CPUs only
On 2025/7/25 21:11, Sudeep Holla wrote:
> 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@...wei.com>
>> ---
>
> I have no idea what changed from v1->v2 and no link to v1 for me to
> refer to it and check the delta 🙁.
My fault.
Changes in 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/
>
>> 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);
>>
>> /*
>> * 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);
>> + amu_fie_setup(cpus_to_set);
>> + }
>> +
>> + free_cpumask_var(cpus_to_set);
>
> What am I missing here as I don't see the need to for this local
> copy `cpus_to_set`.
>
> Why can't you just call
> policy = cpufreq_cpu_get(cpu);
> if (policy)
> amu_fie_setup(cpus_to_set);
As mentioned in the commit log, when hotplug a CPU, the
freq_counters_valid() check should be done for all the online CPUs share
the same policy. Otherwise they may use different source of the freq scale
and cause some problems.
>
>
>> + 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;
>> + }
>> +
>
> Why can't you just set up cpuhp_* first and then cpufreq notifier to avoid
> this unregistering ?
If set up cpuhp state first, it should be remove when register notifier
fails. So I think there is no difference which one is called first.
Powered by blists - more mailing lists