Re: [PATCH v2] x86/speculation: Add support for STIBP always-on preferred mode

Borislav Petkov
2018-12-12T00:05:28Z

On Tue, Dec 11, 2018 at 10:46:16PM +0000, Lendacky, Thomas wrote:
> Different AMD processors may have different implementations of STIBP.
> When STIBP is conditionally enabled, some implementations would benefit
> from having STIBP always on instead of toggling the STIBP bit through MSR
> writes. This preference is advertised through a CPUID feature bit.
>
> When conditional STIBP support is requested at boot and the CPU advertises
> STIBP always-on mode as preferred, switch to STIBP "on" support. Print a
> message to let the user know this occurred. Also, provide a boolean that
> be used in stibp_state() to return a message tailored to the always-on
> support.
>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
>
> This patch is against the x86/pti branch of the tip tree:
> git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/pti
>
> Changes from v1:
> - Removed explicit SPECTRE_V2_USER_STRICT_PREFERRED mode
> - Added a message when switching to always-on mode
> - Set and used a static boolean for the string in stibp_state()
>
> arch/x86/include/asm/cpufeatures.h | 1 +
> arch/x86/kernel/cpu/bugs.c | 17 ++++++++++++++++-
> 2 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index 28c4a50..df8e94e2 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -284,6 +284,7 @@
> #define X86_FEATURE_AMD_IBPB (13*32+12) /* "" Indirect Branch Prediction Barrier */
> #define X86_FEATURE_AMD_IBRS (13*32+14) /* "" Indirect Branch Restricted Speculation */
> #define X86_FEATURE_AMD_STIBP (13*32+15) /* "" Single Thread Indirect Branch Predictors */
> +#define X86_FEATURE_AMD_STIBP_ALWAYS_ON (13*32+17) /* "" Single Thread Indirect Branch Predictors always-on preferred */
> #define X86_FEATURE_AMD_SSBD (13*32+24) /* "" Speculative Store Bypass Disable */
> #define X86_FEATURE_VIRT_SSBD (13*32+25) /* Virtualized Speculative Store Bypass Disable */
> #define X86_FEATURE_AMD_SSB_NO (13*32+26) /* "" Speculative Store Bypass is fixed in hardware. */
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index 58689ac..db156e1 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -61,6 +61,8 @@
> /* Control unconditional IBPB in switch_mm() */
> DEFINE_STATIC_KEY_FALSE(switch_mm_always_ibpb);
>
> +static bool stibp_always_on;
> +
> void __init check_bugs(void)
> {
> identify_boot_cpu();
> @@ -355,6 +357,18 @@ static void __init spec_v2_user_print_cond(const char *reason, bool secure)
> break;
> }
>
> + /*
> + * At this point, an STIBP mode other than "off" has been set.
> + * If STIBP support is not being forced, check if STIBP always-on
> + * is preferred.
> + */
> + if (mode != SPECTRE_V2_USER_STRICT &&
> + boot_cpu_has(X86_FEATURE_AMD_STIBP_ALWAYS_ON)) {
> + stibp_always_on = true;
> + mode = SPECTRE_V2_USER_STRICT;
> + pr_info("mitigation: STIBP always-on is preferred\n");
> + }
> +
> /* Initialize Indirect Branch Prediction Barrier */
> if (boot_cpu_has(X86_FEATURE_IBPB)) {
> setup_force_cpu_cap(X86_FEATURE_USE_IBPB);
> @@ -1088,7 +1102,8 @@ static char *stibp_state(void)
> case SPECTRE_V2_USER_NONE:
> return ", STIBP: disabled";
> case SPECTRE_V2_USER_STRICT:
> - return ", STIBP: forced";
> + return stibp_always_on ? ", STIBP: always-on"
> + : ", STIBP: forced";

I still don't like that separate stibp_always_on variable when we can do
all the querying just by using mode and X86_FEATURE_AMD_STIBP_ALWAYS_ON.

--
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.