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

[Aboorva Devarajan <aboorvad@linux.ibm.com>]

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