Re: [PATCH v3 1/7] x86: suppress ERMS for internal use when MISC_ENABLE.FAST_STRING is clear

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 25/11/2024 2:27 pm, Jan Beulich wrote:
> Before we start actually adjusting behavior when ERMS is available,
> follow Linux commit 161ec53c702c ("x86, mem, intel: Initialize Enhanced
> REP MOVSB/STOSB") and zap the CPUID-derived feature flag when the MSR
> bit is clear. Don't extend the artificial clearing to guest view,
> though: Guests can take their own decision in this regard, as they can
> read (most of) MISC_ENABLE.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> TBD: Would be nice if "cpuid=no-erms" propagated to guest view (for
>      "cpuid=" generally meaning to affect guests as well as Xen), but
>      since both disabling paths use setup_clear_cpu_cap() they're
>      indistinguishable in guest_common_feature_adjustments(). A separate
>      boolean could take care of this, but would look clumsy to me.
> ---
> v3: New.

I'm not sure this is a terribly wise course of action.  First, ...

>
> --- a/xen/arch/x86/cpu/intel.c
> +++ b/xen/arch/x86/cpu/intel.c
> @@ -337,8 +337,18 @@ static void cf_check early_init_intel(st
>               paddr_bits = 36;
>  
>       if (c == &boot_cpu_data) {
> +             uint64_t misc_enable;
> +
>               check_memory_type_self_snoop_errata();
>  
> +             /*
> +              * If fast string is not enabled in IA32_MISC_ENABLE for any 
> reason,
> +              * clear the enhanced fast string CPU capability.
> +              */
> +             rdmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
> +             if (!(misc_enable & MSR_IA32_MISC_ENABLE_FAST_STRING))
> +                     setup_clear_cpu_cap(X86_FEATURE_ERMS);


... this is a thread scope MSR, and commonly does have a BIOS option,
and has been observed to be asymmetrically configured.  (On Intel at
least.  AMD has no equivalent control that I'm aware of.)

It needs checking in init_intel() and on every CPU, and to use plain
clear_cpu_cap().

But, we virtualise MSR_MISC_ENABLE (along with MSR_PLATFORM_INFO) to
*all* guests in order to advertise CPUID Faulting (even on AMD systems
which have an architectural CPUID faulting).  This means that all guests
reliably see FAST_STRINGS disabled, even when it happens to be active.

It turns out that Linux will hide ERMS because of this, adversely
affecting Linux's choices in the same way that PVShim is about to be
impacted.


I see no option but to virtualise MSR_MISC_ENABLE more properly on Intel
systems, but it's not without it's complexity.  One #MC errata
workaround involves clearing FAST_STRING and leaving it disabled until
the next warm reset (Xen has no knowledge about this, but Linux will if
provoked).

The ability to modify the FAST_STRING bit is without enumeration; It's
simply existed since the P4, and I'm unsure whether we want to honour a
guest's choice to disable fast strings, or simply ignore what the guest
wants and echo it's choice back to it.


Furthermore, ...

> +
>               intel_init_levelling();
>       }
>  
> --- a/xen/arch/x86/cpu-policy.c
> +++ b/xen/arch/x86/cpu-policy.c
> @@ -590,6 +590,15 @@ static void __init guest_common_feature_
>       */
>      if ( host_cpu_policy.feat.ibrsb )
>          __set_bit(X86_FEATURE_IBPB, fs);
> +
> +    /*
> +     * We expose MISC_ENABLE to guests, so our internal clearing of ERMS when
> +     * FAST_STRING is not set should not propagate to guest view.  Guests can
> +     * judge on their own whether to ignore the CPUID bit when the MSR bit is
> +     * clear.
> +     */
> +    if ( raw_cpu_policy.feat.erms )
> +        __set_bit(X86_FEATURE_ERMS, fs);

... this breaks migration of a VM between hosts with different Fast
String settings.  ERMS is perf-hint bit, so wants to be set in the Max
policy, with Default taking the host value.  There are several other
examples of this pattern, but I've not made a magic string for it yet.

~Andrew