Re: [PATCH 1/9] x86/mwait-idle: arrange for BSP MSR adjustments during S3 resume

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Mar 12, 2026 at 05:54:30PM +0100, Jan Beulich wrote:
> mwait_idle_cpu_init() is only called for APs, yet MSR writes will
> typically need re-doing post-S3 even for the BSP. When multiple cores /
> threads are present (and to come back online) in a package, for package
> scope MSRs this may be covered by APs doing the writes, but we can't rely
> on that.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> --- a/xen/arch/x86/acpi/power.c
> +++ b/xen/arch/x86/acpi/power.c
> @@ -28,6 +28,7 @@
>  #include <asm/io_apic.h>
>  #include <asm/irq.h>
>  #include <asm/microcode.h>
> +#include <asm/mwait.h>
>  #include <asm/prot-key.h>
>  #include <asm/spec_ctrl.h>
>  #include <asm/tboot.h>
> @@ -299,6 +300,7 @@ static int enter_state(u32 state)
>      acpi_sleep_post(state);
>      if ( hvm_cpu_up() )
>          BUG();
> +    mwait_idle_resume();
>      cpufreq_add_cpu(0);
>  
>   enable_cpu:
> --- a/xen/arch/x86/cpu/mwait-idle.c
> +++ b/xen/arch/x86/cpu/mwait-idle.c
> @@ -1680,6 +1680,28 @@ static int __init mwait_idle_probe(void)
>       return 0;
>  }
>  
> +static void mwait_idle_cpu_tweak(unsigned int cpu)
> +{
> +     if (icpu->auto_demotion_disable_flags)
> +             on_selected_cpus(cpumask_of(cpu), auto_demotion_disable, NULL, 
> 1);
> +
> +     if (icpu->byt_auto_demotion_disable_flag)
> +             on_selected_cpus(cpumask_of(cpu), byt_auto_demotion_disable, 
> NULL, 1);
> +
> +     switch (icpu->c1e_promotion) {
> +     case C1E_PROMOTION_DISABLE:
> +             on_selected_cpus(cpumask_of(cpu), c1e_promotion_disable, NULL, 
> 1);
> +             break;
> +
> +     case C1E_PROMOTION_ENABLE:
> +             on_selected_cpus(cpumask_of(cpu), c1e_promotion_enable, NULL, 
> 1);
> +             break;
> +
> +     case C1E_PROMOTION_PRESERVE:
> +             break;
> +     }
> +}
> +
>  static int cf_check mwait_idle_cpu_init(
>      struct notifier_block *nfb, unsigned long action, void *hcpu)
>  {
> @@ -1762,24 +1784,7 @@ static int cf_check mwait_idle_cpu_init(
>               dev->count++;
>       }
>  
> -     if (icpu->auto_demotion_disable_flags)
> -             on_selected_cpus(cpumask_of(cpu), auto_demotion_disable, NULL, 
> 1);
> -
> -     if (icpu->byt_auto_demotion_disable_flag)
> -             on_selected_cpus(cpumask_of(cpu), byt_auto_demotion_disable, 
> NULL, 1);
> -
> -     switch (icpu->c1e_promotion) {
> -     case C1E_PROMOTION_DISABLE:
> -             on_selected_cpus(cpumask_of(cpu), c1e_promotion_disable, NULL, 
> 1);
> -             break;
> -
> -     case C1E_PROMOTION_ENABLE:
> -             on_selected_cpus(cpumask_of(cpu), c1e_promotion_enable, NULL, 
> 1);
> -             break;

I'm possibly missing some context here, but why do we use
on_selected_cpus and the CPU_ONLINE hook?  Won't it be easier to use
CPU_STARTING and avoid the use of on_selected_cpus(), as CPU_STARTING
runs in the context of the CPU being onlined.

Thanks, Roger.