Re: [PATCH 2/9] x86/mwait-idle: clean up BYT/CHT auto demotion disable

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

On Thu, Mar 12, 2026 at 05:54:56PM +0100, Jan Beulich wrote:
> Bay Trail (BYT) and Cherry Trail (CHT) platforms have a very specific way
> of disabling auto-demotion via specific MSR bits. Clean up the code so that
> BYT/CHT-specifics do not show up in the common 'struct idle_cpu' data
> structure.
> 
> Remove the 'byt_auto_demotion_disable_flag' flag from 'struct idle_cpu',
> because a better coding pattern is to avoid very case-specific fields like
> 'bool byt_auto_demotion_disable_flag' in a common data structure, which is
> used for all platforms, not only BYT/CHT. The code is just more readable
> when common data structures contain only commonly used fields.
> 
> Instead, match BYT/CHT in the 'intel_idle_init_cstates_icpu()' function,
> and introduce a small helper to take care of BYT/CHT auto-demotion. This
> is consistent with how platform-specific things are done for other
> platforms.
> 
> No intended functional changes.
> 
> Inspired by (and description largely taken from) Linux'es c93d13b661a6
> ("intel_idle: clean up BYT/CHT auto demotion disable").
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> --- a/xen/arch/x86/cpu/mwait-idle.c
> +++ b/xen/arch/x86/cpu/mwait-idle.c
> @@ -104,7 +104,6 @@ struct idle_cpu {
>        * Indicate which enable bits to clear here.
>        */
>       unsigned long auto_demotion_disable_flags;
> -     bool byt_auto_demotion_disable_flag;
>       enum c1e_promotion c1e_promotion;
>  };
>  
> @@ -1144,7 +1143,7 @@ static void cf_check auto_demotion_disab
>       wrmsrl(MSR_PKG_CST_CONFIG_CONTROL, msr_bits);
>  }
>  
> -static void cf_check byt_auto_demotion_disable(void *dummy)
> +static void byt_cht_auto_demotion_disable(void)
>  {
>       wrmsrl(MSR_CC6_DEMOTION_POLICY_CONFIG, 0);
>       wrmsrl(MSR_MC6_DEMOTION_POLICY_CONFIG, 0);
> @@ -1195,13 +1194,11 @@ static const struct idle_cpu idle_cpu_sn
>  static const struct idle_cpu idle_cpu_byt = {
>       .state_table = byt_cstates,
>       .c1e_promotion = C1E_PROMOTION_DISABLE,
> -     .byt_auto_demotion_disable_flag = true,
>  };
>  
>  static const struct idle_cpu idle_cpu_cht = {
>       .state_table = cht_cstates,
>       .c1e_promotion = C1E_PROMOTION_DISABLE,
> -     .byt_auto_demotion_disable_flag = true,
>  };
>  
>  static const struct idle_cpu idle_cpu_ivb = {
> @@ -1680,14 +1677,11 @@ static int __init mwait_idle_probe(void)
>       return 0;
>  }
>  
> -static void mwait_idle_cpu_tweak(unsigned int cpu)
> +static void mwait_idle_cpu_tweak(unsigned int cpu, bool bsp)
>  {
>       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);
> @@ -1700,12 +1694,24 @@ static void mwait_idle_cpu_tweak(unsigne
>       case C1E_PROMOTION_PRESERVE:
>               break;
>       }
> +
> +     /* Pkg-scope MSRs on 1-socket-only systems need writing only once. */
> +     if (!bsp)
> +             return;
> +
> +     switch (boot_cpu_data.vfm) {
> +     case INTEL_ATOM_SILVERMONT:
> +     case INTEL_ATOM_AIRMONT:
> +             byt_cht_auto_demotion_disable();
> +             break;
> +     }
>  }
>  
>  static int cf_check mwait_idle_cpu_init(
>      struct notifier_block *nfb, unsigned long action, void *hcpu)
>  {
>       unsigned int cpu = (unsigned long)hcpu, cstate;
> +     static bool first;

I think you want to init first = true here, so that after the first
call to mwait_idle_cpu_tweak() it gets set to false for future calls?

Thanks, Roger.