Message-ID: <m1y3ke1h2j.fsf@dinechin.org>
Date:   Wed, 31 Jan 2018 11:03:21 +0100
From:   Christophe de Dinechin <christophe.de.dinechin@...il.com>
To:     KarimAllah Ahmed <karahmed@...zon.de>
Cc:     linux-kernel@...r.kernel.org, Andi Kleen <ak@...ux.intel.com>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Andy Lutomirski <luto@...nel.org>,
        Arjan van de Ven <arjan@...ux.intel.com>,
        Ashok Raj <ashok.raj@...el.com>,
        Asit Mallick <asit.k.mallick@...el.com>,
        Borislav Petkov <bp@...e.de>,
        Dan Williams <dan.j.williams@...el.com>,
        Dave Hansen <dave.hansen@...el.com>,
        David Woodhouse <dwmw@...zon.co.uk>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "H . Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...hat.com>,
        Janakarajan Natarajan <Janakarajan.Natarajan@....com>,
        Joerg Roedel <joro@...tes.org>,
        Jun Nakajima <jun.nakajima@...el.com>,
        Laura Abbott <labbott@...hat.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Radim Krčmář <rkrcmar@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Tim Chen <tim.c.chen@...ux.intel.com>,
        Tom Lendacky <thomas.lendacky@....com>, kvm@...r.kernel.org,
        x86@...nel.org
Subject: Re: [RFC 05/10] x86/speculation: Add basic IBRS support infrastructure


KarimAllah Ahmed writes:

> From: David Woodhouse <dwmw@...zon.co.uk>
>
> Not functional yet; just add the handling for it in the Spectre v2
> mitigation selection, and the X86_FEATURE_IBRS flag which will control
> the code to be added in later patches.
>
> Also take the #ifdef CONFIG_RETPOLINE from around the RSB-stuffing; IBRS
> mode will want that too.
>
> For now we are auto-selecting IBRS on Skylake. We will probably end up
> changing that but for now let's default to the safest option.
>
> XX: Do we want a microcode blacklist?
>
> [karahmed: simplify the switch block and get rid of all the magic]
>
> Signed-off-by: David Woodhouse <dwmw@...zon.co.uk>
> Signed-off-by: KarimAllah Ahmed <karahmed@...zon.de>
> ---
>  Documentation/admin-guide/kernel-parameters.txt |   1 +
>  arch/x86/include/asm/cpufeatures.h              |   1 +
>  arch/x86/include/asm/nospec-branch.h            |   2 -
>  arch/x86/kernel/cpu/bugs.c                      | 108 +++++++++++++++---------
>  4 files changed, 68 insertions(+), 44 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 8122b5f..e597650 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -3932,6 +3932,7 @@
>  			retpoline	  - replace indirect branches
>  			retpoline,generic - google's original retpoline
>  			retpoline,amd     - AMD-specific minimal thunk
> +			ibrs		  - Intel: Indirect Branch Restricted Speculation
>
>  			Not specifying this option is equivalent to
>  			spectre_v2=auto.
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index 8ec9588..ae86ad9 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -211,6 +211,7 @@
>  #define X86_FEATURE_AMD_PRED_CMD	( 7*32+17) /* Prediction Command MSR (AMD) */
>  #define X86_FEATURE_MBA			( 7*32+18) /* Memory Bandwidth Allocation */
>  #define X86_FEATURE_RSB_CTXSW		( 7*32+19) /* Fill RSB on context switches */
> +#define X86_FEATURE_IBRS		( 7*32+21) /* Use IBRS for Spectre v2 safety */
>
>  /* Virtualization flags: Linux defined, word 8 */
>  #define X86_FEATURE_TPR_SHADOW		( 8*32+ 0) /* Intel TPR Shadow */
> diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
> index c333c95..8759449 100644
> --- a/arch/x86/include/asm/nospec-branch.h
> +++ b/arch/x86/include/asm/nospec-branch.h
> @@ -205,7 +205,6 @@ extern char __indirect_thunk_end[];
>   */
>  static inline void vmexit_fill_RSB(void)
>  {
> -#ifdef CONFIG_RETPOLINE
>  	unsigned long loops;
>
>  	asm volatile (ANNOTATE_NOSPEC_ALTERNATIVE
> @@ -215,7 +214,6 @@ static inline void vmexit_fill_RSB(void)
>  		      "910:"
>  		      : "=r" (loops), ASM_CALL_CONSTRAINT
>  		      : : "memory" );
> -#endif
>  }
>
>  static inline void indirect_branch_prediction_barrier(void)
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index 96548ff..1d5e12f 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -79,6 +79,7 @@ enum spectre_v2_mitigation_cmd {
>  	SPECTRE_V2_CMD_RETPOLINE,
>  	SPECTRE_V2_CMD_RETPOLINE_GENERIC,
>  	SPECTRE_V2_CMD_RETPOLINE_AMD,
> +	SPECTRE_V2_CMD_IBRS,
>  };
>
>  static const char *spectre_v2_strings[] = {
> @@ -87,6 +88,7 @@ static const char *spectre_v2_strings[] = {
>  	[SPECTRE_V2_RETPOLINE_MINIMAL_AMD]	= "Vulnerable: Minimal AMD ASM retpoline",
>  	[SPECTRE_V2_RETPOLINE_GENERIC]		= "Mitigation: Full generic retpoline",
>  	[SPECTRE_V2_RETPOLINE_AMD]		= "Mitigation: Full AMD retpoline",
> +	[SPECTRE_V2_IBRS]			= "Mitigation: Indirect Branch Restricted Speculation",
>  };
>
>  #undef pr_fmt
> @@ -132,9 +134,17 @@ static enum spectre_v2_mitigation_cmd __init spectre_v2_parse_cmdline(void)
>  			spec2_print_if_secure("force enabled on command line.");
>  			return SPECTRE_V2_CMD_FORCE;
>  		} else if (match_option(arg, ret, "retpoline")) {
> +			if (!IS_ENABLED(CONFIG_RETPOLINE)) {
> +				pr_err("retpoline selected but not compiled in. Switching to AUTO select\n");
> +				return SPECTRE_V2_CMD_AUTO;
> +			}
>  			spec2_print_if_insecure("retpoline selected on command line.");
>  			return SPECTRE_V2_CMD_RETPOLINE;
>  		} else if (match_option(arg, ret, "retpoline,amd")) {
> +			if (!IS_ENABLED(CONFIG_RETPOLINE)) {
> +				pr_err("retpoline,amd selected but not compiled in. Switching to AUTO select\n");
> +				return SPECTRE_V2_CMD_AUTO;
> +			}
>  			if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD) {
>  				pr_err("retpoline,amd selected but CPU is not AMD. Switching to AUTO select\n");
>  				return SPECTRE_V2_CMD_AUTO;
> @@ -142,8 +152,19 @@ static enum spectre_v2_mitigation_cmd __init spectre_v2_parse_cmdline(void)
>  			spec2_print_if_insecure("AMD retpoline selected on command line.");
>  			return SPECTRE_V2_CMD_RETPOLINE_AMD;
>  		} else if (match_option(arg, ret, "retpoline,generic")) {
> +			if (!IS_ENABLED(CONFIG_RETPOLINE)) {
> +				pr_err("retpoline,generic selected but not compiled in. Switching to AUTO select\n");
> +				return SPECTRE_V2_CMD_AUTO;
> +			}
>  			spec2_print_if_insecure("generic retpoline selected on command line.");
>  			return SPECTRE_V2_CMD_RETPOLINE_GENERIC;
> +		} else if (match_option(arg, ret, "ibrs")) {
> +			if (!boot_cpu_has(X86_FEATURE_SPEC_CTRL)) {
> +				pr_err("IBRS selected but no CPU support. Switching to AUTO select\n");
> +				return SPECTRE_V2_CMD_AUTO;
> +			}
> +			spec2_print_if_insecure("IBRS seleted on command line.");
> +			return SPECTRE_V2_CMD_IBRS;
>  		} else if (match_option(arg, ret, "auto")) {
>  			return SPECTRE_V2_CMD_AUTO;
>  		}
> @@ -156,7 +177,7 @@ static enum spectre_v2_mitigation_cmd __init spectre_v2_parse_cmdline(void)
>  	return SPECTRE_V2_CMD_NONE;
>  }
>
> -/* Check for Skylake-like CPUs (for RSB handling) */
> +/* Check for Skylake-like CPUs (for RSB and IBRS handling) */
>  static bool __init is_skylake_era(void)
>  {
>  	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL &&
> @@ -178,55 +199,58 @@ static void __init spectre_v2_select_mitigation(void)
>  	enum spectre_v2_mitigation_cmd cmd = spectre_v2_parse_cmdline();
>  	enum spectre_v2_mitigation mode = SPECTRE_V2_NONE;
>
> -	/*
> -	 * If the CPU is not affected and the command line mode is NONE or AUTO
> -	 * then nothing to do.
> -	 */
> -	if (!boot_cpu_has_bug(X86_BUG_SPECTRE_V2) &&
> -	    (cmd == SPECTRE_V2_CMD_NONE || cmd == SPECTRE_V2_CMD_AUTO))
> -		return;
> -
>  	switch (cmd) {
>  	case SPECTRE_V2_CMD_NONE:
> +		if (boot_cpu_has_bug(X86_BUG_SPECTRE_V2))
> +			pr_err("kernel not compiled with retpoline; no mitigation available!");
>  		return;
> -
> -	case SPECTRE_V2_CMD_FORCE:
> -		/* FALLTRHU */
> -	case SPECTRE_V2_CMD_AUTO:
> -		goto retpoline_auto;
> -
> -	case SPECTRE_V2_CMD_RETPOLINE_AMD:
> -		if (IS_ENABLED(CONFIG_RETPOLINE))
> -			goto retpoline_amd;
> -		break;
> -	case SPECTRE_V2_CMD_RETPOLINE_GENERIC:
> -		if (IS_ENABLED(CONFIG_RETPOLINE))
> -			goto retpoline_generic;
> +	case SPECTRE_V2_CMD_IBRS:
> +		mode = SPECTRE_V2_IBRS;
> +		setup_force_cpu_cap(X86_FEATURE_IBRS);
>  		break;
> +	case SPECTRE_V2_CMD_AUTO:
> +		if (!boot_cpu_has_bug(X86_BUG_SPECTRE_V2))
> +			return;
> +		/* Fall through */
> +	case SPECTRE_V2_CMD_FORCE:
> +		/*
> +		 * If we have IBRS support, and either Skylake or !RETPOLINE,
> +		 * then that's what we do.
> +		 */
> +		if (boot_cpu_has(X86_FEATURE_SPEC_CTRL) &&
> +		    (is_skylake_era() || !retp_compiler())) {

As per Eduardo's comments and followups, it's unclear this will play
well under virtualization. Putting this under a separate function
with a name making it clear that what we care about is the host, not
guest CPU.

Under virtualization, you may want to force is_skylake() to return
true (unless there is a way to get a more precise answer about the
host CPU at that stage?)


> +			mode = SPECTRE_V2_IBRS;
> +			setup_force_cpu_cap(X86_FEATURE_IBRS);
> +			break;
> +		}
> +		/* Fall through */

Given the complexity of the decision and the number of fall-through
cases, it's probably a good idea to add some printouts for system mgmt
or debugging.


>  	case SPECTRE_V2_CMD_RETPOLINE:
> -		if (IS_ENABLED(CONFIG_RETPOLINE))
> -			goto retpoline_auto;
> -		break;
> -	}
> -	pr_err("kernel not compiled with retpoline; no mitigation available!");
> -	return;
> +	case SPECTRE_V2_CMD_RETPOLINE_AMD:
> +		if (IS_ENABLED(CONFIG_RETPOLINE) &&
> +		    boot_cpu_data.x86_vendor == X86_VENDOR_AMD) {
> +			if (boot_cpu_has(X86_FEATURE_LFENCE_RDTSC)) {
> +				mode = retp_compiler() ? SPECTRE_V2_RETPOLINE_AMD :
> +							 SPECTRE_V2_RETPOLINE_MINIMAL_AMD;
> +				setup_force_cpu_cap(X86_FEATURE_RETPOLINE_AMD);
> +				setup_force_cpu_cap(X86_FEATURE_RETPOLINE);
> +				break;
> +			}
>
> -retpoline_auto:
> -	if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) {
> -	retpoline_amd:
> -		if (!boot_cpu_has(X86_FEATURE_LFENCE_RDTSC)) {
>  			pr_err("LFENCE not serializing. Switching to generic retpoline\n");
> -			goto retpoline_generic;
>  		}
> -		mode = retp_compiler() ? SPECTRE_V2_RETPOLINE_AMD :
> -					 SPECTRE_V2_RETPOLINE_MINIMAL_AMD;
> -		setup_force_cpu_cap(X86_FEATURE_RETPOLINE_AMD);
> -		setup_force_cpu_cap(X86_FEATURE_RETPOLINE);
> -	} else {
> -	retpoline_generic:
> -		mode = retp_compiler() ? SPECTRE_V2_RETPOLINE_GENERIC :
> -					 SPECTRE_V2_RETPOLINE_MINIMAL;
> -		setup_force_cpu_cap(X86_FEATURE_RETPOLINE);
> +		/* Fall through */
> +	case SPECTRE_V2_CMD_RETPOLINE_GENERIC:
> +		if (IS_ENABLED(CONFIG_RETPOLINE)) {
> +			mode = retp_compiler() ? SPECTRE_V2_RETPOLINE_GENERIC :
> +						 SPECTRE_V2_RETPOLINE_MINIMAL;
> +			setup_force_cpu_cap(X86_FEATURE_RETPOLINE);
> +			break;
> +		}
> +		/* Fall through */
> +	default:
> +		if (boot_cpu_has_bug(X86_BUG_SPECTRE_V2))
> +			pr_err("kernel not compiled with retpoline; no mitigation available!");
> +		return;
>  	}
>
>  	spectre_v2_enabled = mode;


--
Cheers,
Christophe de Dinechin (IRC c3d)

Powered by blists - more mailing lists