Re: [PATCH 8/8] x86/spec-ctrl: Mitigate the Zen1 DIV leakge

[Jan Beulich <jbeulich@xxxxxxxx>]

On 13.09.2023 22:27, Andrew Cooper wrote:
> @@ -378,6 +392,8 @@ UNLIKELY_DISPATCH_LABEL(\@_serialise):
>      verw STACK_CPUINFO_FIELD(verw_sel)(%r14)
>  .L\@_verw_skip:
>  
> +    ALTERNATIVE "", DO_SPEC_CTRL_DIV, X86_FEATURE_SC_DIV
> +
>  .L\@_skip_ist_exit:
>  .endm

While we did talk about using alternatives patching here, I'm now in
doubt again, in particular because the rest of the macro uses
conditionals anyway, and the code here is bypassed for non-IST exits. If
you're sure you want to stick to this, then I think some justification
wants adding to the description.

> @@ -955,6 +960,40 @@ static void __init srso_calculations(bool hw_smt_enabled)
>          setup_force_cpu_cap(X86_FEATURE_SRSO_NO);
>  }
>  
> +/*
> + * Div leakage is specific to the AMD Zen1 microarchitecure.  Use STIBP as a
> + * heuristic to select between Zen1 and Zen2 uarches.
> + */
> +static bool __init has_div_vuln(void)
> +{
> +    if ( !(boot_cpu_data.x86_vendor &
> +           (X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
> +        return false;
> +
> +    if ( (boot_cpu_data.x86 != 0x17 && boot_cpu_data.x86 != 0x18) ||
> +         !boot_cpu_has(X86_FEATURE_AMD_STIBP) )
> +        return false;
> +
> +    return true;
> +}
> +
> +static void __init div_calculations(bool hw_smt_enabled)
> +{
> +    bool cpu_bug_div = has_div_vuln();
> +
> +    if ( opt_div_scrub == -1 )
> +        opt_div_scrub = cpu_bug_div;
> +
> +    if ( opt_div_scrub )
> +        setup_force_cpu_cap(X86_FEATURE_SC_DIV);

Isn't this only lowering performance (even if just slightly) when SMT is
enabled, without gaining us very much?

> +    if ( opt_smt == -1 && cpu_bug_div && hw_smt_enabled )
> +        warning_add(
> +            "Booted on leaky-DIV hardware with SMT/Hyperthreading\n"
> +            "enabled.  Please assess your configuration and choose an\n"
> +            "explicit 'smt=<bool>' setting.  See XSA-439.\n");
> +}

What about us running virtualized? The topology we see may not be that
of the underlying hardware. Maybe check_smt_enabled() should return
true when cpu_has_hypervisor is true? (In-guest decisions would
similarly need to assume that they may be running on SMT-enabled
hardware, despite not themselves seeing this to be the case.)

Since we can't know for sure when running virtualized, that's a case
where I would consider it useful to enable the workaround nevertheless
(perhaps accompanied by a warning that whether this helps depends on
the next level hypervisor).

Jan