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

[Borislav Petkov <bp@alien8.de>]

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.