Re: [PATCH v3 3/4] cpufreq: ACPI: Use IPI to update boost MSR in cpufreq_boost_down_prep()

[Rafael J. Wysocki]

On Fri, Jun 19, 2026 at 12:45 AM Jim Mattson <jmattson@xxxxxxxxxx> wrote:
>
> During driver exit or CPU hotplug, acpi_cpufreq_cpu_exit() calls
> cpufreq_boost_down_prep() to re-enable boost on the target CPU.  However,
> cpufreq_boost_down_prep() ignores the target CPU parameter and calls
> boost_set_msr(1) locally. Since this runs in an unbound process context, it
> updates the local CPU's MSR while leaving the target CPU's boost state
> unchanged.
>
> On Intel platforms, the open-coded read-modify-write of
> MSR_IA32_MISC_ENABLE in boost_set_msr() is vulnerable to preemption and
> thread migration if executed in process context.
>
> Fix both issues by routing the MSR update through
> smp_call_function_single() to execute boost_set_msr() synchronously on the
> target CPU. This ensures the correct CPU is updated and that the RMW
> sequence on Intel executes safely in IPI context with interrupts disabled.
>
> Fixes: a3605c46e0c0 ("cpufreq: acpi-cpufreq: drop rdmsr_on_cpus() usage")
> Reported-by: Sashiko <sashiko-bot@xxxxxxxxxx>
> Closes: https://sashiko.dev/#/patchset/20260612215729.1532175-1-jmattson%40google.com
> Assisted-by: Gemini:gemini-3.5-pro
> Signed-off-by: Jim Mattson <jmattson@xxxxxxxxxx>
> ---
>  drivers/cpufreq/acpi-cpufreq.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
> index 21639d9ac753..48d678812fd6 100644
> --- a/drivers/cpufreq/acpi-cpufreq.c
> +++ b/drivers/cpufreq/acpi-cpufreq.c
> @@ -528,10 +528,11 @@ static void free_acpi_perf_data(void)
>  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.
> +        * Clear the boost-disable bit on the target CPU so that
> +        * it cannot block the remaining ones from boosting.
>          */
> -       return boost_set_msr(1);
> +       return smp_call_function_single(cpu, boost_set_msr_each,
> +                                       (void *)1L, 1);

While I have not investigated the history of this code, I'm wondering
why cpufreq_boost_down_prep()  is a separate function and why it takes
the CPU number argument.

The only caller of it is acpi_cpufreq_cpu_exit() which could just do

set_boost(policy, 1);

instead of calling it (and might check if boost was supported at all
for that matter).

>  }
>
>  /*
> --