[PATCH v2 0/4] cpufreq: Fix some boost errors related to CPU online and offline.

[PATCH v2 0/4] cpufreq: Fix some boost errors related to CPU online and offline.

[Lifeng Zheng]

This patch series fix some boost errors related to CPU online and offline:

 - patch 1 fix an error that causes the CPU stay on base frequency after a
   specific operation

 - patch 2 introduce a more generic way to set default per-policy boost
   flag and fix a error that causes the per-policy boost flag remians true
   when cpufreq_driver boost disabled

 - patch 3 fix an error in cppc_cpufreq that causes the CPU stay on base
   frequency when boost flag is true

 - patch 4 remove the set_boost in acpi_cpufreq_cpu_init(), since it will
   be executed in cpufreq_online

Change since v1:
 - remove update of min_freq_req
 - optimize the conditions for executing set_boost in cpufreq_online
 - fix another error in cppc_cpufreq
 - remove set_boost in acpi_cpufreq_cpu_init()

Lifeng Zheng (4):
  cpufreq: Fix re-boost issue after hotplugging a cpu
  cpufreq: Introduce a more generic way to set default per-policy boost
    flag
  cpufreq: CPPC: Fix wrong max_freq in policy initialization
  cpufreq: ACPI: Remove set_boost in acpi_cpufreq_cpu_init()

 drivers/cpufreq/acpi-cpufreq.c |  5 -----
 drivers/cpufreq/cppc_cpufreq.c |  5 +++--
 drivers/cpufreq/cpufreq.c      | 20 ++++++++++++++++----
 3 files changed, 19 insertions(+), 11 deletions(-)

-- 
2.33.0

[PATCH v2 1/4] cpufreq: Fix re-boost issue after hotplugging a cpu

[Lifeng Zheng]

It turns out that cpuX will stay on the base frequency after performing
these operations:

1. boost all cpus: echo 1 > /sys/devices/system/cpu/cpufreq/boost

2. offline the cpu: echo 0 > /sys/devices/system/cpu/cpuX/online

3. deboost all cpus: echo 0 > /sys/devices/system/cpu/cpufreq/boost

4. online the cpu: echo 1 > /sys/devices/system/cpu/cpuX/online

5. boost all cpus again: echo 1 > /sys/devices/system/cpu/cpufreq/boost

This is because max_freq_req of the policy is not updated during the online
process, and the value of max_freq_req before the last offline is retained.
When the CPU is boosted again, freq_qos_update_request() will do nothing
because the old value is the same as the new one. This causes the CPU stay
on the base frequency. Update max_freq_req  in cpufreq_online() will solve
this problem.

Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
---
 drivers/cpufreq/cpufreq.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 1a4cae54a01b..5882d7f5e3c1 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1475,6 +1475,10 @@ static int cpufreq_online(unsigned int cpu)
 
 		blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
 				CPUFREQ_CREATE_POLICY, policy);
+	} else {
+		ret = freq_qos_update_request(policy->max_freq_req, policy->max);
+		if (ret < 0)
+			goto out_destroy_policy;
 	}
 
 	if (cpufreq_driver->get && has_target()) {
-- 
2.33.0

[PATCH v2 2/4] cpufreq: Introduce a more generic way to set default per-policy boost flag

[Lifeng Zheng]

In cpufreq_online() of cpufreq.c, the per-policy boost flag is already set
to mirror the cpufreq_driver boost during init but using freq_table to
judge if the policy has boost frequency. There are two drawbacks to this
approach:

1. It doesn't work for the cpufreq drivers that do not use a frequency
table. For now, acpi-cpufreq and amd-pstate have to enable boost in policy
initialization. And cppc_cpufreq never set policy to boost when going
online no matter what the cpufreq_driver boost flag is.

2. If the cpu goes offline when cpufreq_driver boost enabled and then goes
online when cpufreq_driver boost disabled, the per-policy boost flag will
unreasonably remain true.

Running set_boost at the end of the online process is a more generic way
for all cpufreq drivers.

Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
---
 drivers/cpufreq/cpufreq.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 5882d7f5e3c1..5a3566c2eb8d 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1409,10 +1409,6 @@ static int cpufreq_online(unsigned int cpu)
 			goto out_free_policy;
 		}
 
-		/* Let the per-policy boost flag mirror the cpufreq_driver boost during init */
-		if (cpufreq_boost_enabled() && policy_has_boost_freq(policy))
-			policy->boost_enabled = true;
-
 		/*
 		 * The initialization has succeeded and the policy is online.
 		 * If there is a problem with its frequency table, take it
@@ -1573,6 +1569,18 @@ static int cpufreq_online(unsigned int cpu)
 	if (new_policy && cpufreq_thermal_control_enabled(cpufreq_driver))
 		policy->cdev = of_cpufreq_cooling_register(policy);
 
+	/* Let the per-policy boost flag mirror the cpufreq_driver boost during init */
+	if (policy->boost_enabled != cpufreq_boost_enabled()) {
+		policy->boost_enabled = cpufreq_boost_enabled();
+		ret = cpufreq_driver->set_boost(policy, policy->boost_enabled);
+		if (ret) {
+			/* If the set_boost fails, the online operation is not affected */
+			pr_info("%s: CPU%d: Cannot %s BOOST\n", __func__, policy->cpu,
+				policy->boost_enabled ? "enable" : "disable");
+			policy->boost_enabled = !policy->boost_enabled;
+		}
+	}
+
 	pr_debug("initialization complete\n");
 
 	return 0;
-- 
2.33.0

[PATCH v2 3/4] cpufreq: CPPC: Fix wrong max_freq in policy initialization

[Lifeng Zheng]

In policy initialization, policy->max and policy->cpuinfo.max_freq always
set to the value calculated from caps->nominal_perf. This will cause the
frequency stay on base frequency even if the policy is already boosted when
a CPU is going online. Fix this by using policy->boost_enabled to determine
which value should be set.

Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
---
 drivers/cpufreq/cppc_cpufreq.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
index bd8f75accfa0..7fa89b601d2a 100644
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -611,7 +611,8 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
 	 * Section 8.4.7.1.1.5 of ACPI 6.1 spec)
 	 */
 	policy->min = cppc_perf_to_khz(caps, caps->lowest_nonlinear_perf);
-	policy->max = cppc_perf_to_khz(caps, caps->nominal_perf);
+	policy->max = cppc_perf_to_khz(caps, policy->boost_enabled ?
+						caps->highest_perf : caps->nominal_perf);
 
 	/*
 	 * Set cpuinfo.min_freq to Lowest to make the full range of performance
@@ -619,7 +620,7 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
 	 * nonlinear perf
 	 */
 	policy->cpuinfo.min_freq = cppc_perf_to_khz(caps, caps->lowest_perf);
-	policy->cpuinfo.max_freq = cppc_perf_to_khz(caps, caps->nominal_perf);
+	policy->cpuinfo.max_freq = policy->max;
 
 	policy->transition_delay_us = cppc_cpufreq_get_transition_delay_us(cpu);
 	policy->shared_type = cpu_data->shared_type;
-- 
2.33.0

[PATCH v2 4/4] cpufreq: ACPI: Remove set_boost in acpi_cpufreq_cpu_init()

[Lifeng Zheng]

At the end of cpufreq_online() in cpufreq.c, set_boost is executed and the
per-policy boost flag is set to mirror the cpufreq_driver boost. So it is
not necessary to run set_boost in acpi_cpufreq_cpu_init().

Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
---
 drivers/cpufreq/acpi-cpufreq.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
index c9ebacf5c88e..f4b5e455f173 100644
--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -891,11 +891,6 @@ static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy)
 	if (perf->states[0].core_frequency * 1000 != freq_table[0].frequency)
 		pr_warn(FW_WARN "P-state 0 is not max freq\n");
 
-	if (acpi_cpufreq_driver.set_boost) {
-		set_boost(policy, acpi_cpufreq_driver.boost_enabled);
-		policy->boost_enabled = acpi_cpufreq_driver.boost_enabled;
-	}
-
 	return result;
 
 err_unreg:
-- 
2.33.0

Re: [PATCH v2 1/4] cpufreq: Fix re-boost issue after hotplugging a cpu

[Viresh Kumar]

Hi,

I am bit confused by the sequence of events here and need some
clarification. Lets assume that CPU can go from 1 GHz to 1.5 GHz
without boost enabled and with boost it can go to 2 GHz.

On 17-01-25, 18:14, Lifeng Zheng wrote:
> It turns out that cpuX will stay on the base frequency after performing
> these operations:
> 
> 1. boost all cpus: echo 1 > /sys/devices/system/cpu/cpufreq/boost

Boost enabled here, max_freq_req = 2 GHz.

> 2. offline the cpu: echo 0 > /sys/devices/system/cpu/cpuX/online
> 
> 3. deboost all cpus: echo 0 > /sys/devices/system/cpu/cpufreq/boost
> 
> 4. online the cpu: echo 1 > /sys/devices/system/cpu/cpuX/online

Boost is disabled currently here, but max_freq_req = 2 GHz, which is
incorrect and the current change you are proposing fixes it I think.
But it is not what you are claiming to fix.

> 5. boost all cpus again: echo 1 > /sys/devices/system/cpu/cpufreq/boost

Boost enabled again here, and max_freq_req = 2 GHz is the correct
value.

So the CPU doesn't stay at base frequency here, but 2 GHz only.

> This is because max_freq_req of the policy is not updated during the online
> process, and the value of max_freq_req before the last offline is retained.

which was 2 GHz in your example.

> When the CPU is boosted again, freq_qos_update_request() will do nothing
> because the old value is the same as the new one. This causes the CPU stay
> on the base frequency. Update max_freq_req  in cpufreq_online() will solve
> this problem.

-- 
viresh

Re: [PATCH v2 2/4] cpufreq: Introduce a more generic way to set default per-policy boost flag

[Viresh Kumar]

On 17-01-25, 18:14, Lifeng Zheng wrote:
> In cpufreq_online() of cpufreq.c, the per-policy boost flag is already set
> to mirror the cpufreq_driver boost during init but using freq_table to
> judge if the policy has boost frequency. There are two drawbacks to this
> approach:
> 
> 1. It doesn't work for the cpufreq drivers that do not use a frequency
> table. For now, acpi-cpufreq and amd-pstate have to enable boost in policy
> initialization. And cppc_cpufreq never set policy to boost when going
> online no matter what the cpufreq_driver boost flag is.
> 
> 2. If the cpu goes offline when cpufreq_driver boost enabled and then goes
> online when cpufreq_driver boost disabled, the per-policy boost flag will
> unreasonably remain true.
> 
> Running set_boost at the end of the online process is a more generic way
> for all cpufreq drivers.
> 
> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
> ---
>  drivers/cpufreq/cpufreq.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 5882d7f5e3c1..5a3566c2eb8d 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1409,10 +1409,6 @@ static int cpufreq_online(unsigned int cpu)
>  			goto out_free_policy;
>  		}
>  
> -		/* Let the per-policy boost flag mirror the cpufreq_driver boost during init */
> -		if (cpufreq_boost_enabled() && policy_has_boost_freq(policy))
> -			policy->boost_enabled = true;
> -
>  		/*
>  		 * The initialization has succeeded and the policy is online.
>  		 * If there is a problem with its frequency table, take it
> @@ -1573,6 +1569,18 @@ static int cpufreq_online(unsigned int cpu)
>  	if (new_policy && cpufreq_thermal_control_enabled(cpufreq_driver))
>  		policy->cdev = of_cpufreq_cooling_register(policy);
>  
> +	/* Let the per-policy boost flag mirror the cpufreq_driver boost during init */
> +	if (policy->boost_enabled != cpufreq_boost_enabled()) {
> +		policy->boost_enabled = cpufreq_boost_enabled();
> +		ret = cpufreq_driver->set_boost(policy, policy->boost_enabled);

I though you agreed to do some optimization here ?

> +		if (ret) {
> +			/* If the set_boost fails, the online operation is not affected */
> +			pr_info("%s: CPU%d: Cannot %s BOOST\n", __func__, policy->cpu,
> +				policy->boost_enabled ? "enable" : "disable");
> +			policy->boost_enabled = !policy->boost_enabled;
> +		}
> +	}
> +
>  	pr_debug("initialization complete\n");
>  
>  	return 0;
> -- 
> 2.33.0

-- 
viresh

Re: [PATCH v2 1/4] cpufreq: Fix re-boost issue after hotplugging a cpu

[zhenglifeng (A)]

On 2025/1/20 16:27, Viresh Kumar wrote:

> Hi,
> 
> I am bit confused by the sequence of events here and need some
> clarification. Lets assume that CPU can go from 1 GHz to 1.5 GHz
> without boost enabled and with boost it can go to 2 GHz.
> 
> On 17-01-25, 18:14, Lifeng Zheng wrote:
>> It turns out that cpuX will stay on the base frequency after performing
>> these operations:
>>
>> 1. boost all cpus: echo 1 > /sys/devices/system/cpu/cpufreq/boost
> 
> Boost enabled here, max_freq_req = 2 GHz.
> 
>> 2. offline the cpu: echo 0 > /sys/devices/system/cpu/cpuX/online
>>
>> 3. deboost all cpus: echo 0 > /sys/devices/system/cpu/cpufreq/boost
>>
>> 4. online the cpu: echo 1 > /sys/devices/system/cpu/cpuX/online
> 
> Boost is disabled currently here, but max_freq_req = 2 GHz, which is
> incorrect and the current change you are proposing fixes it I think.
> But it is not what you are claiming to fix.

Since boost is disabled, policy->max and policy->cpuinfo.max_freq will be
1.5GHz, this limits the actual frequency of the final.

> 
>> 5. boost all cpus again: echo 1 > /sys/devices/system/cpu/cpufreq/boost
> 
> Boost enabled again here, and max_freq_req = 2 GHz is the correct
> value.

In freq_qos_update_request(), if req->pnode.prio == new_value, it will
directly return 0 and not excecute freq_qos_apply(), in which will refresh
frequency. So the frequency will stay on base.

> 
> So the CPU doesn't stay at base frequency here, but 2 GHz only.
> 
>> This is because max_freq_req of the policy is not updated during the online
>> process, and the value of max_freq_req before the last offline is retained.
> 
> which was 2 GHz in your example.
> 
>> When the CPU is boosted again, freq_qos_update_request() will do nothing
>> because the old value is the same as the new one. This causes the CPU stay
>> on the base frequency. Update max_freq_req  in cpufreq_online() will solve
>> this problem.
> 

Re: [PATCH v2 1/4] cpufreq: Fix re-boost issue after hotplugging a cpu

[Viresh Kumar]

On 20-01-25, 17:10, zhenglifeng (A) wrote:
> On 2025/1/20 16:27, Viresh Kumar wrote:
> > On 17-01-25, 18:14, Lifeng Zheng wrote:
> >> 1. boost all cpus: echo 1 > /sys/devices/system/cpu/cpufreq/boost
> > 
> > Boost enabled here, max_freq_req = 2 GHz.
> > 
> >> 2. offline the cpu: echo 0 > /sys/devices/system/cpu/cpuX/online
> >>
> >> 3. deboost all cpus: echo 0 > /sys/devices/system/cpu/cpufreq/boost
> >>
> >> 4. online the cpu: echo 1 > /sys/devices/system/cpu/cpuX/online
> > 
> > Boost is disabled currently here, but max_freq_req = 2 GHz, which is
> > incorrect and the current change you are proposing fixes it I think.
> > But it is not what you are claiming to fix.
> 
> Since boost is disabled, policy->max and policy->cpuinfo.max_freq will be
> 1.5GHz, this limits the actual frequency of the final.

Okay, I was thinking about the case (!new_policy &&
cpufreq_driver->online), where cpufreq_table_validate_and_sort() isn't
called and max_freq isn't updated eventually. But for the other case,
we will see max-freq as 1.5GHz.

> >> 5. boost all cpus again: echo 1 > /sys/devices/system/cpu/cpufreq/boost
> > 
> > Boost enabled again here, and max_freq_req = 2 GHz is the correct
> > value.
> 
> In freq_qos_update_request(), if req->pnode.prio == new_value, it will
> directly return 0 and not excecute freq_qos_apply(), in which will refresh
> frequency. So the frequency will stay on base.

-- 
viresh

Re: [PATCH v2 1/4] cpufreq: Fix re-boost issue after hotplugging a cpu

[Viresh Kumar]

On 17-01-25, 18:14, Lifeng Zheng wrote:
> It turns out that cpuX will stay on the base frequency after performing
> these operations:
> 
> 1. boost all cpus: echo 1 > /sys/devices/system/cpu/cpufreq/boost
> 
> 2. offline the cpu: echo 0 > /sys/devices/system/cpu/cpuX/online
> 
> 3. deboost all cpus: echo 0 > /sys/devices/system/cpu/cpufreq/boost
> 
> 4. online the cpu: echo 1 > /sys/devices/system/cpu/cpuX/online
> 
> 5. boost all cpus again: echo 1 > /sys/devices/system/cpu/cpufreq/boost
> 
> This is because max_freq_req of the policy is not updated during the online
> process, and the value of max_freq_req before the last offline is retained.
> When the CPU is boosted again, freq_qos_update_request() will do nothing
> because the old value is the same as the new one. This causes the CPU stay
> on the base frequency. Update max_freq_req  in cpufreq_online() will solve
> this problem.
> 
> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
> ---
>  drivers/cpufreq/cpufreq.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 1a4cae54a01b..5882d7f5e3c1 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1475,6 +1475,10 @@ static int cpufreq_online(unsigned int cpu)
>  
>  		blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
>  				CPUFREQ_CREATE_POLICY, policy);
> +	} else {
> +		ret = freq_qos_update_request(policy->max_freq_req, policy->max);
> +		if (ret < 0)
> +			goto out_destroy_policy;
>  	}

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH v2 3/4] cpufreq: CPPC: Fix wrong max_freq in policy initialization

[Viresh Kumar]

On 17-01-25, 18:14, Lifeng Zheng wrote:
> In policy initialization, policy->max and policy->cpuinfo.max_freq always
> set to the value calculated from caps->nominal_perf. This will cause the
> frequency stay on base frequency even if the policy is already boosted when
> a CPU is going online. Fix this by using policy->boost_enabled to determine
> which value should be set.
> 
> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
> ---
>  drivers/cpufreq/cppc_cpufreq.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index bd8f75accfa0..7fa89b601d2a 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -611,7 +611,8 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
>  	 * Section 8.4.7.1.1.5 of ACPI 6.1 spec)
>  	 */
>  	policy->min = cppc_perf_to_khz(caps, caps->lowest_nonlinear_perf);
> -	policy->max = cppc_perf_to_khz(caps, caps->nominal_perf);
> +	policy->max = cppc_perf_to_khz(caps, policy->boost_enabled ?
> +						caps->highest_perf : caps->nominal_perf);
>  
>  	/*
>  	 * Set cpuinfo.min_freq to Lowest to make the full range of performance
> @@ -619,7 +620,7 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
>  	 * nonlinear perf
>  	 */
>  	policy->cpuinfo.min_freq = cppc_perf_to_khz(caps, caps->lowest_perf);
> -	policy->cpuinfo.max_freq = cppc_perf_to_khz(caps, caps->nominal_perf);
> +	policy->cpuinfo.max_freq = policy->max;
>  
>  	policy->transition_delay_us = cppc_cpufreq_get_transition_delay_us(cpu);
>  	policy->shared_type = cpu_data->shared_type;

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH v2 2/4] cpufreq: Introduce a more generic way to set default per-policy boost flag

[zhenglifeng (A)]

On 2025/1/20 17:01, Viresh Kumar wrote:

> On 17-01-25, 18:14, Lifeng Zheng wrote:
>> In cpufreq_online() of cpufreq.c, the per-policy boost flag is already set
>> to mirror the cpufreq_driver boost during init but using freq_table to
>> judge if the policy has boost frequency. There are two drawbacks to this
>> approach:
>>
>> 1. It doesn't work for the cpufreq drivers that do not use a frequency
>> table. For now, acpi-cpufreq and amd-pstate have to enable boost in policy
>> initialization. And cppc_cpufreq never set policy to boost when going
>> online no matter what the cpufreq_driver boost flag is.
>>
>> 2. If the cpu goes offline when cpufreq_driver boost enabled and then goes
>> online when cpufreq_driver boost disabled, the per-policy boost flag will
>> unreasonably remain true.
>>
>> Running set_boost at the end of the online process is a more generic way
>> for all cpufreq drivers.
>>
>> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
>> ---
>>  drivers/cpufreq/cpufreq.c | 16 ++++++++++++----
>>  1 file changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
>> index 5882d7f5e3c1..5a3566c2eb8d 100644
>> --- a/drivers/cpufreq/cpufreq.c
>> +++ b/drivers/cpufreq/cpufreq.c
>> @@ -1409,10 +1409,6 @@ static int cpufreq_online(unsigned int cpu)
>>  			goto out_free_policy;
>>  		}
>>  
>> -		/* Let the per-policy boost flag mirror the cpufreq_driver boost during init */
>> -		if (cpufreq_boost_enabled() && policy_has_boost_freq(policy))
>> -			policy->boost_enabled = true;
>> -
>>  		/*
>>  		 * The initialization has succeeded and the policy is online.
>>  		 * If there is a problem with its frequency table, take it
>> @@ -1573,6 +1569,18 @@ static int cpufreq_online(unsigned int cpu)
>>  	if (new_policy && cpufreq_thermal_control_enabled(cpufreq_driver))
>>  		policy->cdev = of_cpufreq_cooling_register(policy);
>>  
>> +	/* Let the per-policy boost flag mirror the cpufreq_driver boost during init */
>> +	if (policy->boost_enabled != cpufreq_boost_enabled()) {
>> +		policy->boost_enabled = cpufreq_boost_enabled();
>> +		ret = cpufreq_driver->set_boost(policy, policy->boost_enabled);
> 
> I though you agreed to do some optimization here ?

Sorry. Do I miss something here?

> 
>> +		if (ret) {
>> +			/* If the set_boost fails, the online operation is not affected */
>> +			pr_info("%s: CPU%d: Cannot %s BOOST\n", __func__, policy->cpu,
>> +				policy->boost_enabled ? "enable" : "disable");
>> +			policy->boost_enabled = !policy->boost_enabled;
>> +		}
>> +	}
>> +
>>  	pr_debug("initialization complete\n");
>>  
>>  	return 0;
>> -- 
>> 2.33.0
> 

Re: [PATCH v2 2/4] cpufreq: Introduce a more generic way to set default per-policy boost flag

[Viresh Kumar]

On 21-01-25, 09:45, zhenglifeng (A) wrote:
> On 2025/1/20 17:01, Viresh Kumar wrote:
> > On 17-01-25, 18:14, Lifeng Zheng wrote:
> >> +	/* Let the per-policy boost flag mirror the cpufreq_driver boost during init */
> >> +	if (policy->boost_enabled != cpufreq_boost_enabled()) {
> >> +		policy->boost_enabled = cpufreq_boost_enabled();
> >> +		ret = cpufreq_driver->set_boost(policy, policy->boost_enabled);
> > 
> > I though you agreed to do some optimization here ?
> 
> Sorry. Do I miss something here?

https://lore.kernel.org/all/17c7ed77-21f1-4093-91fc-f3eaa863d312@huawei.com/

-- 
viresh

Re: [PATCH v2 4/4] cpufreq: ACPI: Remove set_boost in acpi_cpufreq_cpu_init()

[Viresh Kumar]

On 17-01-25, 18:14, Lifeng Zheng wrote:
> At the end of cpufreq_online() in cpufreq.c, set_boost is executed and the
> per-policy boost flag is set to mirror the cpufreq_driver boost. So it is
> not necessary to run set_boost in acpi_cpufreq_cpu_init().
> 
> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
> ---
>  drivers/cpufreq/acpi-cpufreq.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
> index c9ebacf5c88e..f4b5e455f173 100644
> --- a/drivers/cpufreq/acpi-cpufreq.c
> +++ b/drivers/cpufreq/acpi-cpufreq.c
> @@ -891,11 +891,6 @@ static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy)
>  	if (perf->states[0].core_frequency * 1000 != freq_table[0].frequency)
>  		pr_warn(FW_WARN "P-state 0 is not max freq\n");
>  
> -	if (acpi_cpufreq_driver.set_boost) {
> -		set_boost(policy, acpi_cpufreq_driver.boost_enabled);
> -		policy->boost_enabled = acpi_cpufreq_driver.boost_enabled;
> -	}
> -
>  	return result;
>  
>  err_unreg:

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

There are more cleanups in drivers that can be done though. I will try
that once this series is merged.

-- 
viresh

Re: [PATCH v2 2/4] cpufreq: Introduce a more generic way to set default per-policy boost flag

[zhenglifeng (A)]

On 2025/1/21 12:20, Viresh Kumar wrote:

> On 21-01-25, 09:45, zhenglifeng (A) wrote:
>> On 2025/1/20 17:01, Viresh Kumar wrote:
>>> On 17-01-25, 18:14, Lifeng Zheng wrote:
>>>> +	/* Let the per-policy boost flag mirror the cpufreq_driver boost during init */
>>>> +	if (policy->boost_enabled != cpufreq_boost_enabled()) {
>>>> +		policy->boost_enabled = cpufreq_boost_enabled();
>>>> +		ret = cpufreq_driver->set_boost(policy, policy->boost_enabled);
>>>
>>> I though you agreed to do some optimization here ?
>>
>> Sorry. Do I miss something here?
> 
> https://lore.kernel.org/all/17c7ed77-21f1-4093-91fc-f3eaa863d312@huawei.com/
> 

I think I already done that, isn't it?

Re: [PATCH v2 2/4] cpufreq: Introduce a more generic way to set default per-policy boost flag

[Viresh Kumar]

On 21-01-25, 14:22, zhenglifeng (A) wrote:
> On 2025/1/21 12:20, Viresh Kumar wrote:
> 
> > On 21-01-25, 09:45, zhenglifeng (A) wrote:
> >> On 2025/1/20 17:01, Viresh Kumar wrote:
> >>> On 17-01-25, 18:14, Lifeng Zheng wrote:
> >>>> +	/* Let the per-policy boost flag mirror the cpufreq_driver boost during init */
> >>>> +	if (policy->boost_enabled != cpufreq_boost_enabled()) {
> >>>> +		policy->boost_enabled = cpufreq_boost_enabled();
> >>>> +		ret = cpufreq_driver->set_boost(policy, policy->boost_enabled);
> >>>
> >>> I though you agreed to do some optimization here ?
> >>
> >> Sorry. Do I miss something here?
> > 
> > https://lore.kernel.org/all/17c7ed77-21f1-4093-91fc-f3eaa863d312@huawei.com/
> > 
> 
> I think I already done that, isn't it?

And I misread /facepalm .

-- 
viresh

Re: [PATCH v2 0/4] cpufreq: Fix some boost errors related to CPU online and offline.

[Viresh Kumar]

On 17-01-25, 18:14, Lifeng Zheng wrote:
> This patch series fix some boost errors related to CPU online and offline:
> 
>  - patch 1 fix an error that causes the CPU stay on base frequency after a
>    specific operation
> 
>  - patch 2 introduce a more generic way to set default per-policy boost
>    flag and fix a error that causes the per-policy boost flag remians true
>    when cpufreq_driver boost disabled
> 
>  - patch 3 fix an error in cppc_cpufreq that causes the CPU stay on base
>    frequency when boost flag is true
> 
>  - patch 4 remove the set_boost in acpi_cpufreq_cpu_init(), since it will
>    be executed in cpufreq_online
> 
> Change since v1:
>  - remove update of min_freq_req
>  - optimize the conditions for executing set_boost in cpufreq_online
>  - fix another error in cppc_cpufreq
>  - remove set_boost in acpi_cpufreq_cpu_init()

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH v2 0/4] cpufreq: Fix some boost errors related to CPU online and offline.

[Rafael J. Wysocki]

On Tue, Jan 21, 2025 at 7:38 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 17-01-25, 18:14, Lifeng Zheng wrote:
> > This patch series fix some boost errors related to CPU online and offline:
> >
> >  - patch 1 fix an error that causes the CPU stay on base frequency after a
> >    specific operation
> >
> >  - patch 2 introduce a more generic way to set default per-policy boost
> >    flag and fix a error that causes the per-policy boost flag remians true
> >    when cpufreq_driver boost disabled
> >
> >  - patch 3 fix an error in cppc_cpufreq that causes the CPU stay on base
> >    frequency when boost flag is true
> >
> >  - patch 4 remove the set_boost in acpi_cpufreq_cpu_init(), since it will
> >    be executed in cpufreq_online
> >
> > Change since v1:
> >  - remove update of min_freq_req
> >  - optimize the conditions for executing set_boost in cpufreq_online
> >  - fix another error in cppc_cpufreq
> >  - remove set_boost in acpi_cpufreq_cpu_init()
>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

All patches in the series applied as 6.14-rc material, thanks!

Re: [PATCH v2 4/4] cpufreq: ACPI: Remove set_boost in acpi_cpufreq_cpu_init()

[Viresh Kumar]

On 21-01-25, 11:44, Viresh Kumar wrote:
> There are more cleanups in drivers that can be done though. I will try
> that once this series is merged.

https://lore.kernel.org/751338633b070ee570c3c7da053bd6b9497ee50e.1737707712.git.viresh.kumar@linaro.org

-- 
viresh

Re: [PATCH v2 4/4] cpufreq: ACPI: Remove set_boost in acpi_cpufreq_cpu_init()

[zhenglifeng (A)]

On 2025/1/24 16:59, Viresh Kumar wrote:

> On 21-01-25, 11:44, Viresh Kumar wrote:
>> There are more cleanups in drivers that can be done though. I will try
>> that once this series is merged.
> 
> https://lore.kernel.org/751338633b070ee570c3c7da053bd6b9497ee50e.1737707712.git.viresh.kumar@linaro.org
> 

Nice!

Re: [PATCH v2 2/4] cpufreq: Introduce a more generic way to set default per-policy boost flag

[Aboorva Devarajan]

On Fri, 2025-01-17 at 18:14 +0800, Lifeng Zheng wrote:

> In cpufreq_online() of cpufreq.c, the per-policy boost flag is already set
> to mirror the cpufreq_driver boost during init but using freq_table to
> judge if the policy has boost frequency. There are two drawbacks to this
> approach:
> 
> 1. It doesn't work for the cpufreq drivers that do not use a frequency
> table. For now, acpi-cpufreq and amd-pstate have to enable boost in policy
> initialization. And cppc_cpufreq never set policy to boost when going
> online no matter what the cpufreq_driver boost flag is.
> 
> 2. If the cpu goes offline when cpufreq_driver boost enabled and then goes
> online when cpufreq_driver boost disabled, the per-policy boost flag will
> unreasonably remain true.
> 
> Running set_boost at the end of the online process is a more generic way
> for all cpufreq drivers.
> 
> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
> ---
>  drivers/cpufreq/cpufreq.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 5882d7f5e3c1..5a3566c2eb8d 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1409,10 +1409,6 @@ static int cpufreq_online(unsigned int cpu)
>  			goto out_free_policy;
>  		}
>  
> -		/* Let the per-policy boost flag mirror the cpufreq_driver boost during init */
> -		if (cpufreq_boost_enabled() && policy_has_boost_freq(policy))
> -			policy->boost_enabled = true;
> -
>  		/*
>  		 * The initialization has succeeded and the policy is online.
>  		 * If there is a problem with its frequency table, take it
> @@ -1573,6 +1569,18 @@ static int cpufreq_online(unsigned int cpu)
>  	if (new_policy && cpufreq_thermal_control_enabled(cpufreq_driver))
>  		policy->cdev = of_cpufreq_cooling_register(policy);
>  
> +	/* Let the per-policy boost flag mirror the cpufreq_driver boost during init */
> +	if (policy->boost_enabled != cpufreq_boost_enabled()) {
> +		policy->boost_enabled = cpufreq_boost_enabled();
> +		ret = cpufreq_driver->set_boost(policy, policy->boost_enabled);
> +		if (ret) {
> +			/* If the set_boost fails, the online operation is not affected */
> +			pr_info("%s: CPU%d: Cannot %s BOOST\n", __func__, policy->cpu,
> +				policy->boost_enabled ? "enable" : "disable");
> +			policy->boost_enabled = !policy->boost_enabled;
> +		}
> +	}
> +
>  	pr_debug("initialization complete\n");
>  
>  
> 	return 0;

Hi,

This patch causes a boot-time crash on PowerNV (Power9-baremetal systems) when WoF
(Workload Optimized Frequency - boost) is enabled, starting from v6.14-rc1.

The crash happens due to null pointer dereference of the `set_boost` function.

`set_boost` is only assigned after the cpufreq driver is registered on PowerNV
as below, 

Initialization Flow: (powernv_cpufreq_init -> cpufreq_enable_boost_support -> 
                      initializes set_boost).

However, with this patch, `set_boost` is invoked in `cpufreq_register_driver`
before it is initialized.

Access Flow:         (powernv_cpufreq_init -> cpufreq_register_driver -> cpufreq_online -> 
                      attempts to access cpufreq_driver->set_boost,
                      which is still NULL at this point).

This causes a boot-time crash as follows:

[    9.393946][    T1] BUG: Unable to handle kernel instruction fetch (NULL pointer?)
[    9.393946][    T1] BUG: Unable to handle kernel instruction fetch (NULL pointer?)
[    9.396285][    T1] Faulting instruction address: 0x00000000
[    9.396285][    T1] Faulting instruction address: 0x00000000
[    9.398545][    T1] Oops: Kernel access of bad area, sig: 7 [#1]
[    9.398545][    T1] Oops: Kernel access of bad area, sig: 7 [#1]
[    9.398804][    T1] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA PowerNV
[    9.398804][    T1] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA PowerNV
[    9.400114][    T1] Modules linked in:
[    9.400114][    T1] Modules linked in:
[    9.400283][    T1] CPU: 19 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.14.0-rc1-dirty #23
[    9.400283][    T1] CPU: 19 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.14.0-rc1-dirty #23
[    9.400605][    T1] Hardware name: 0000000000000000 POWER9 0x4e1202 opal:v6.6-111-gd362ae4f-root-dirty-157d5e1 PowerNV
[    9.400605][    T1] Hardware name: 0000000000000000 POWER9 0x4e1202 opal:v6.6-111-gd362ae4f-root-dirty-157d5e1 PowerNV
[    9.403093][    T1] NIP:  0000000000000000 LR: c000000000f7a574 CTR: 0000000000000000
[    9.403093][    T1] NIP:  0000000000000000 LR: c000000000f7a574 CTR: 0000000000000000
[    9.404397][    T1] REGS: c00020000419f680 TRAP: 0400   Not tainted  (6.14.0-rc1-dirty)
[    9.404397][    T1] REGS: c00020000419f680 TRAP: 0400   Not tainted  (6.14.0-rc1-dirty)
[    9.407771][    T1] MSR:  9000000002089033 <SF,HV,VEC,EE,ME,IR,DR,RI,LE>  CR: 84000482  XER: 00000000
[    9.407771][    T1] MSR:  9000000002089033 <SF,HV,VEC,EE,ME,IR,DR,RI,LE>  CR: 84000482  XER: 00000000
[    9.409191][    T1] CFAR: c000000000f7a570 IRQMASK: 0 
[    9.409191][    T1] GPR00: c000000000f7a51c c00020000419f920 c000000001ea3200 c00020002455f800 
[    9.409191][    T1] GPR04: 0000000000000001 0000000000000000 0000000000000001 0000000000000000 
[    9.409191][    T1] GPR08: 0000000000000000 c000000002a1f280 0000000000000001 0000000000000000 
[    9.409191][    T1] GPR12: 0000000000000000 c000003ffefe0900 c00000000001101c 0000000000000000 
[    9.409191][    T1] GPR16: 0000000000000000 0000000000000000 c000000002c20b80 000000000000005a 
[    9.409191][    T1] GPR20: c00000007ffa0a14 c000000002973200 c000000002c1ff38 c00020002455fc48 
[    9.409191][    T1] GPR24: c0000000029752f8 c000000002c1ff10 c00020002455fcb8 0000000000000000 
[    9.409191][    T1] GPR28: c000000002a71be0 0000000000000000 c00020002455f800 c000000002a1f0a0 
[    9.409191][    T1] CFAR: c000000000f7a570 IRQMASK: 0 
[    9.409191][    T1] GPR00: c000000000f7a51c c00020000419f920 c000000001ea3200 c00020002455f800 
[    9.409191][    T1] GPR04: 0000000000000001 0000000000000000 0000000000000001 0000000000000000 
[    9.409191][    T1] GPR08: 0000000000000000 c000000002a1f280 0000000000000001 0000000000000000 
[    9.409191][    T1] GPR12: 0000000000000000 c000003ffefe0900 c00000000001101c 0000000000000000 
[    9.409191][    T1] GPR16: 0000000000000000 0000000000000000 c000000002c20b80 000000000000005a 
[    9.409191][    T1] GPR20: c00000007ffa0a14 c000000002973200 c000000002c1ff38 c00020002455fc48 
[    9.409191][    T1] GPR24: c0000000029752f8 c000000002c1ff10 c00020002455fcb8 0000000000000000 
[    9.409191][    T1] GPR28: c000000002a71be0 0000000000000000 c00020002455f800 c000000002a1f0a0 
[    9.415226][    T1] NIP [0000000000000000] 0x0
[    9.415226][    T1] NIP [0000000000000000] 0x0
[    9.417435][    T1] LR [c000000000f7a574] cpufreq_online+0x440/0xe14
[    9.417435][    T1] LR [c000000000f7a574] cpufreq_online+0x440/0xe14
[    9.419701][    T1] Call Trace:
[    9.419701][    T1] Call Trace:
[    9.422849][    T1] [c00020000419f920] [c000000000f7a51c] cpufreq_online+0x3e8/0xe14 (unreliable)
[    9.422849][    T1] [c00020000419f920] [c000000000f7a51c] cpufreq_online+0x3e8/0xe14 (unreliable)
[    9.430225][    T1] [c00020000419fa00] [c000000000f7b030] cpufreq_add_dev+0xb4/0xd8
[    9.430225][    T1] [c00020000419fa00] [c000000000f7b030] cpufreq_add_dev+0xb4/0xd8
[    9.434547][    T1] [c00020000419fa30] [c000000000c89e18] subsys_interface_register+0x18c/0x1d4
[    9.434547][    T1] [c00020000419fa30] [c000000000c89e18] subsys_interface_register+0x18c/0x1d4
[    9.440934][    T1] [c00020000419faa0] [c000000000f763a8] cpufreq_register_driver+0x1f0/0x370
[    9.440934][    T1] [c00020000419faa0] [c000000000f763a8] cpufreq_register_driver+0x1f0/0x370
[    9.444314][    T1] [c00020000419fb20] [c000000002093340] powernv_cpufreq_init+0x690/0xa10
[    9.444314][    T1] [c00020000419fb20] [c000000002093340] powernv_cpufreq_init+0x690/0xa10
[    9.444686][    T1] [c00020000419fc30] [c000000000010cb8] do_one_initcall+0x60/0x2c8
[    9.444686][    T1] [c00020000419fc30] [c000000000010cb8] do_one_initcall+0x60/0x2c8
[    9.445024][    T1] [c00020000419fd00] [c0000000020059ec] kernel_init_freeable+0x33c/0x530
[    9.445024][    T1] [c00020000419fd00] [c0000000020059ec] kernel_init_freeable+0x33c/0x530
[    9.446375][    T1] [c00020000419fde0] [c000000000011048] kernel_init+0x34/0x26c
[    9.446375][    T1] [c00020000419fde0] [c000000000011048] kernel_init+0x34/0x26c
[    9.448684][    T1] [c00020000419fe50] [c00000000000debc] ret_from_kernel_user_thread+0x14/0x1c
[    9.448684][    T1] [c00020000419fe50] [c00000000000debc] ret_from_kernel_user_thread+0x14/0x1c

[    9.455840][    T1] ---[ end trace 0000000000000000 ]---
[    9.455840][    T1] ---[ end trace 0000000000000000 ]---

The fix will be to initialize set_boost earlier in powernv_cpufreq_init before calling
cpufreq_register_driver.

Quickly tried this patch and it resolves the issue:

---

diff --git a/drivers/cpufreq/powernv-cpufreq.c b/drivers/cpufreq/powernv-cpufreq.c
index ae79d909943b..2dd61de34a28 100644
--- a/drivers/cpufreq/powernv-cpufreq.c
+++ b/drivers/cpufreq/powernv-cpufreq.c
@@ -1127,8 +1127,10 @@ static int __init powernv_cpufreq_init(void)
        if (rc)
                goto out;
 
-       if (powernv_pstate_info.wof_enabled)
+       if (powernv_pstate_info.wof_enabled) {
                powernv_cpufreq_driver.boost_enabled = true;
+               powernv_cpufreq_driver.set_boost = cpufreq_boost_set_sw;
+       }
        else
                powernv_cpu_freq_attr[SCALING_BOOST_FREQS_ATTR_INDEX] = NULL;
 
@@ -1138,9 +1140,6 @@ static int __init powernv_cpufreq_init(void)
                goto cleanup;
        }
 
-       if (powernv_pstate_info.wof_enabled)
-               cpufreq_enable_boost_support();
-
        register_reboot_notifier(&powernv_cpufreq_reboot_nb);
        opal_message_notifier_register(OPAL_MSG_OCC, &powernv_cpufreq_opal_nb);
 
---

I noticed that Viresh is working on a similar patch [1] as part of a broader patchset
to simplify boost handling, which should also resolve this issue.

Should we merge this patch [1] and related patches since this is causing a crash,
or submit a separate patch to fix this?

[1]: https://lore.kernel.org/all/9b4af20d5b415f41e866ddd8bde9cf6441c463b8.1737707712.git.viresh.kumar@linaro.org/

Regards,
Aboorva

Re: [PATCH v2 2/4] cpufreq: Introduce a more generic way to set default per-policy boost flag

[Viresh Kumar]

On 04-02-25, 22:11, Aboorva Devarajan wrote:
> I noticed that Viresh is working on a similar patch [1] as part of a broader patchset
> to simplify boost handling, which should also resolve this issue.
> 
> Should we merge this patch [1] and related patches since this is causing a crash,
> or submit a separate patch to fix this?

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index d434096b7515..7c1f7f5142da 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1590,7 +1590,8 @@ static int cpufreq_online(unsigned int cpu)
                policy->cdev = of_cpufreq_cooling_register(policy);

        /* Let the per-policy boost flag mirror the cpufreq_driver boost during init */
-       if (policy->boost_enabled != cpufreq_boost_enabled()) {
+       if (cpufreq_driver->set_boost &&
+           policy->boost_enabled != cpufreq_boost_enabled()) {
                policy->boost_enabled = cpufreq_boost_enabled();
                ret = cpufreq_driver->set_boost(policy, policy->boost_enabled);
                if (ret) {

I think the right fix for now should be something like this. My series
(which will be part of next merge window) can go in separately and
revert this change then (as we won't see this problem then).

Please send a fix with something like this if it works fine, so Rafael
can apply.

-- 
viresh

Re: [PATCH v2 2/4] cpufreq: Introduce a more generic way to set default per-policy boost flag

[Aboorva Devarajan]

On Wed, 2025-02-05 at 10:31 +0530, Viresh Kumar wrote:
> On 04-02-25, 22:11, Aboorva Devarajan wrote:
> > I noticed that Viresh is working on a similar patch [1] as part of a broader patchset
> > to simplify boost handling, which should also resolve this issue.
> > 
> > Should we merge this patch [1] and related patches since this is causing a crash,
> > or submit a separate patch to fix this?
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index d434096b7515..7c1f7f5142da 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1590,7 +1590,8 @@ static int cpufreq_online(unsigned int cpu)
>                 policy->cdev = of_cpufreq_cooling_register(policy);
> 
>         /* Let the per-policy boost flag mirror the cpufreq_driver boost during init */
> -       if (policy->boost_enabled != cpufreq_boost_enabled()) {
> +       if (cpufreq_driver->set_boost &&
> +           policy->boost_enabled != cpufreq_boost_enabled()) {
>                 policy->boost_enabled = cpufreq_boost_enabled();
>                 ret = cpufreq_driver->set_boost(policy, policy->boost_enabled);
>                 if (ret) {
> 
> I think the right fix for now should be something like this. My series
> (which will be part of next merge window) can go in separately and
> revert this change then (as we won't see this problem then).
> 
> Please send a fix with something like this if it works fine, so Rafael
> can apply.
> 

Hi Viresh,

Thanks, I have posted a patch for this:
https://lore.kernel.org/all/20250205181347.2079272-1-aboorvad@linux.ibm.com/

this should get past the boot-time crash for now, until your patchset 
to simplify boost handling is merged.

Regards,
Aboorva

Re: [PATCH v2 4/4] cpufreq: ACPI: Remove set_boost in acpi_cpufreq_cpu_init()

[Viresh Kumar]

On 17-01-25, 18:14, Lifeng Zheng wrote:
> At the end of cpufreq_online() in cpufreq.c, set_boost is executed and the
> per-policy boost flag is set to mirror the cpufreq_driver boost. So it is
> not necessary to run set_boost in acpi_cpufreq_cpu_init().
> 
> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
> ---
>  drivers/cpufreq/acpi-cpufreq.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
> index c9ebacf5c88e..f4b5e455f173 100644
> --- a/drivers/cpufreq/acpi-cpufreq.c
> +++ b/drivers/cpufreq/acpi-cpufreq.c
> @@ -891,11 +891,6 @@ static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy)
>  	if (perf->states[0].core_frequency * 1000 != freq_table[0].frequency)
>  		pr_warn(FW_WARN "P-state 0 is not max freq\n");
>  
> -	if (acpi_cpufreq_driver.set_boost) {
> -		set_boost(policy, acpi_cpufreq_driver.boost_enabled);
> -		policy->boost_enabled = acpi_cpufreq_driver.boost_enabled;
> -	}
> -
>  	return result;
>  
>  err_unreg:

https://bugzilla.kernel.org/show_bug.cgi?id=220013

"
On kernel 6.13.8, disabling boost by setting
/sys/devices/system/cpu/cpufreq/boost to 0 would persist after
resuming from suspend. After updating to 6.14.2, the system is able to
enter boost states after resuming from suspend despite the boost flag
still being set to 0. Toggling it to 1 and then back to 0 in this
state re-disables boost. My system uses the acpi-cpufreq driver.
"

This bug report is filed and git bisect points to this commit.

Rafael, I think the commit in question did the right thing and there
is something else in the driver that is causing the issue here.

I think the problem here is cpufreq_boost_down_prep(), which gets
called during suspend path and enables the boost.

But since the boost was never enabled from flag's point of view, it
never gets disabled again on resume.

I have suggested following for now to check if this is the case or
not:

diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
index 924314cdeebc..d8599ae7922f 100644
--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -538,6 +538,7 @@ static int cpufreq_boost_down_prep(unsigned int cpu)
         * Clear the boost-disable bit on the CPU_DOWN path so that
         * this cpu cannot block the remaining ones from boosting.
         */
+       policy->boost_enabled = true;
        return boost_set_msr(1);
 }

-- 
viresh