Re: [PATCH v2 4/9] x86/spec-ctrl: Don't use spec_ctrl_{enter,exit}_idle() for S3

[Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>]

On 28/01/2022 13:29, Andrew Cooper wrote:
> 'idle' here refers to hlt/mwait.  The S3 path isn't an idle path - it is a
> platform reset.
>
> Conditionally clearing IBRS and flushing the store buffers on the way down is
> a waste of time.
>
> Furthermore, we want to load default_xen_mcu_opt_ctrl unilaterally on the way
> back up.  Currently it happens as a side effect of X86_FEATURE_SC_MSR_IDLE or
> the next return-to-guest, but that's fragile behaviour.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Wei Liu <wl@xxxxxxx>
>
> v2:
>  * New
> ---
>  xen/arch/x86/acpi/power.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/x86/acpi/power.c b/xen/arch/x86/acpi/power.c
> index 31a56f02d083..ea2bd8bbfe93 100644
> --- a/xen/arch/x86/acpi/power.c
> +++ b/xen/arch/x86/acpi/power.c
> @@ -248,7 +248,6 @@ static int enter_state(u32 state)
>          error = 0;
>  
>      ci = get_cpu_info();
> -    spec_ctrl_enter_idle(ci);
>      /* Avoid NMI/#MC using MSR_SPEC_CTRL until we've reloaded microcode. */
>      ci->spec_ctrl_flags &= ~SCF_ist_wrmsr;
>  
> @@ -295,7 +294,9 @@ static int enter_state(u32 state)
>  
>      /* Re-enabled default NMI/#MC use of MSR_SPEC_CTRL. */
>      ci->spec_ctrl_flags |= (default_spec_ctrl_flags & SCF_ist_wrmsr);
> -    spec_ctrl_exit_idle(ci);
> +
> +    if ( boot_cpu_has(X86_FEATURE_IBRSB) )
> +        wrmsrl(MSR_SPEC_CTRL, default_xen_mcu_opt_ctrl);

This logic works rather better when it gets the right variable. 
default_xen_spec_ctrl.

~Andrew

>  
>      if ( boot_cpu_has(X86_FEATURE_SRBDS_CTRL) )
>          wrmsrl(MSR_MCU_OPT_CTRL, default_xen_mcu_opt_ctrl);