Re: [PATCH 2/3] x86/speculation: Support Automatic IBRS

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/5/22 6:10 AM, Peter Zijlstra wrote:
On Fri, Nov 04, 2022 at 10:52:13PM +0100, Borislav Petkov wrote:
On Fri, Nov 04, 2022 at 04:36:50PM -0500, Kim Phillips wrote:
  - Allow for spectre_v2=autoibrs in the kernel command line,
    reverting to auto-selection if the feature isn't available.

Why?

What the whole logic here should do is enable autoibrs when detected
automatically, without the need for the user to even select it as it is
the superior mitigation.

Well; perhaps the whole autoibrs thing should be mapped to the existing
eIBRS options. AFAICT this is the same thing under a new name, no need
to invent yet more options. bugs.c is quite insane enough already.

I've started a version that has AUTOIBRS reuse SPECTRE_V2_EIBRS
spectre_v2_mitigation enum, but, so far, it's change to bugs.c
looks bigger: 58 lines changed vs. 34 (see below).

Let me know if you want me to send it as a part of a v2 submission
after I take care of the kvm CPUID review.

Thanks,

Kim

Autoibrs-as-eibrs diff:

diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
index 2e9dd8823244..3ab90f23e7f7 100644
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -442,7 +442,6 @@ enum spectre_v2_mitigation {
 	SPECTRE_V2_EIBRS_RETPOLINE,
 	SPECTRE_V2_EIBRS_LFENCE,
 	SPECTRE_V2_IBRS,
-	SPECTRE_V2_AUTO_IBRS,
 };

 /* The indirect branch speculation control variants */
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 31e5af78baa0..ccfd8fb12095 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -1005,6 +1005,7 @@ static inline const char *spectre_v2_module_string(void) { return ""; }
 #endif

 #define SPECTRE_V2_LFENCE_MSG "WARNING: LFENCE mitigation is not recommended for this CPU, data leaks possible!\n"
+#define SPECTRE_V2_EIBRS_AMD_MSG "WARNING: AutoIBRS does not need additional RETPOLINE/LFENCE mitigations, not doing them\n"
 #define SPECTRE_V2_EIBRS_EBPF_MSG "WARNING: Unprivileged eBPF is enabled with eIBRS on, data leaks possible via Spectre v2 BHB attacks!\n"
 #define SPECTRE_V2_EIBRS_LFENCE_EBPF_SMT_MSG "WARNING: Unprivileged eBPF is enabled with eIBRS+LFENCE mitigation and SMT, data leaks possible via Spectre v2 BHB attacks!\n"
 #define SPECTRE_V2_IBRS_PERF_MSG "WARNING: IBRS mitigation selected on Enhanced IBRS CPU, this may cause unnecessary performance loss\n"
@@ -1125,7 +1126,7 @@ spectre_v2_parse_user_cmdline(void)
 	return SPECTRE_V2_USER_CMD_AUTO;
 }

-/* Checks for Intel IBRS versions */
+/* Checks for IBRS versions */
 static inline bool spectre_v2_in_ibrs_mode(enum spectre_v2_mitigation mode)
 {
 	return mode == SPECTRE_V2_IBRS ||
@@ -1201,7 +1202,8 @@ spectre_v2_user_select_mitigation(void)
 	 */
 	if (!boot_cpu_has(X86_FEATURE_STIBP) ||
 	    !smt_possible ||
-	    spectre_v2_in_ibrs_mode(spectre_v2_enabled))
+	    (spectre_v2_in_ibrs_mode(spectre_v2_enabled) &&
+	     !boot_cpu_has(X86_FEATURE_AUTOIBRS)))
 		return;

 	/*
@@ -1231,11 +1233,10 @@ static const char * const spectre_v2_strings[] = {
 	[SPECTRE_V2_NONE]			= "Vulnerable",
 	[SPECTRE_V2_RETPOLINE]			= "Mitigation: Retpolines",
 	[SPECTRE_V2_LFENCE]			= "Mitigation: LFENCE",
-	[SPECTRE_V2_EIBRS]			= "Mitigation: Enhanced IBRS",
+	[SPECTRE_V2_EIBRS]			= "Mitigation: Enhanced / Automatic IBRS",
 	[SPECTRE_V2_EIBRS_LFENCE]		= "Mitigation: Enhanced IBRS + LFENCE",
 	[SPECTRE_V2_EIBRS_RETPOLINE]		= "Mitigation: Enhanced IBRS + Retpolines",
 	[SPECTRE_V2_IBRS]			= "Mitigation: IBRS",
-	[SPECTRE_V2_AUTO_IBRS]			= "Mitigation: Automatic IBRS",
 };

 static const struct {
@@ -1250,9 +1251,9 @@ static const struct {
 	{ "retpoline,lfence",	SPECTRE_V2_CMD_RETPOLINE_LFENCE,  false },
 	{ "retpoline,generic",	SPECTRE_V2_CMD_RETPOLINE_GENERIC, false },
 	{ "eibrs",		SPECTRE_V2_CMD_EIBRS,		  false },
+	{ "autoibrs",		SPECTRE_V2_CMD_EIBRS,		  false },
 	{ "eibrs,lfence",	SPECTRE_V2_CMD_EIBRS_LFENCE,	  false },
 	{ "eibrs,retpoline",	SPECTRE_V2_CMD_EIBRS_RETPOLINE,	  false },
-	{ "autoibrs",		SPECTRE_V2_CMD_AUTOIBRS,	  false },
 	{ "auto",		SPECTRE_V2_CMD_AUTO,		  false },
 	{ "ibrs",		SPECTRE_V2_CMD_IBRS,              false },
 };
@@ -1303,15 +1304,17 @@ static enum spectre_v2_mitigation_cmd __init spectre_v2_parse_cmdline(void)
 	if ((cmd == SPECTRE_V2_CMD_EIBRS ||
 	     cmd == SPECTRE_V2_CMD_EIBRS_LFENCE ||
 	     cmd == SPECTRE_V2_CMD_EIBRS_RETPOLINE) &&
-	    !boot_cpu_has(X86_FEATURE_IBRS_ENHANCED)) {
-		pr_err("%s selected but CPU doesn't have eIBRS. Switching to AUTO select\n",
+	    (!boot_cpu_has(X86_FEATURE_IBRS_ENHANCED) ||
+	     !boot_cpu_has(X86_FEATURE_AUTOIBRS))) {
+		pr_err("%s selected but CPU doesn't have Enhanced or Automatic IBRS. Switching to AUTO select\n",
 		       mitigation_options[i].option);
 		return SPECTRE_V2_CMD_AUTO;
 	}

-	if (cmd == SPECTRE_V2_CMD_AUTOIBRS &&
-	    !boot_cpu_has(X86_FEATURE_AUTOIBRS)) {
-		pr_err("%s selected but CPU doesn't have AMD Automatic IBRS. Switching to AUTO select\n",
+	if ((cmd == SPECTRE_V2_CMD_EIBRS_LFENCE ||
+	     cmd == SPECTRE_V2_CMD_EIBRS_RETPOLINE) &&
+	    boot_cpu_has(X86_FEATURE_AUTOIBRS)) {
+		pr_err("%s selected but AMD Automatic IBRS doesn't need extra retpoline mitigations. Switching to AUTO select\n",
 		       mitigation_options[i].option);
 		return SPECTRE_V2_CMD_AUTO;
 	}
@@ -1403,7 +1406,6 @@ static void __init spectre_v2_determine_rsb_fill_type_at_vmexit(enum spectre_v2_
 	 */
 	switch (mode) {
 	case SPECTRE_V2_NONE:
-	case SPECTRE_V2_AUTO_IBRS:
 		return;

 	case SPECTRE_V2_EIBRS_LFENCE:
@@ -1447,12 +1449,8 @@ static void __init spectre_v2_select_mitigation(void)

 	case SPECTRE_V2_CMD_FORCE:
 	case SPECTRE_V2_CMD_AUTO:
-		if (boot_cpu_has(X86_FEATURE_AUTOIBRS)) {
-			mode = SPECTRE_V2_AUTO_IBRS;
-			break;
-		}
-
-		if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED)) {
+		if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED) ||
+		    boot_cpu_has(X86_FEATURE_AUTOIBRS)) {
 			mode = SPECTRE_V2_EIBRS;
 			break;
 		}
@@ -1488,19 +1486,24 @@ static void __init spectre_v2_select_mitigation(void)
 		break;

 	case SPECTRE_V2_CMD_EIBRS:
+	case SPECTRE_V2_CMD_AUTOIBRS:
 		mode = SPECTRE_V2_EIBRS;
 		break;

 	case SPECTRE_V2_CMD_EIBRS_LFENCE:
-		mode = SPECTRE_V2_EIBRS_LFENCE;
+		if (boot_cpu_has(X86_FEATURE_AUTOIBRS)) {
+			pr_err(SPECTRE_V2_EIBRS_AMD_MSG);
+			mode = SPECTRE_V2_EIBRS;
+		} else
+			mode = SPECTRE_V2_EIBRS_LFENCE;
 		break;

 	case SPECTRE_V2_CMD_EIBRS_RETPOLINE:
-		mode = SPECTRE_V2_EIBRS_RETPOLINE;
-		break;
-
-	case SPECTRE_V2_CMD_AUTOIBRS:
-		mode = SPECTRE_V2_AUTO_IBRS;
+		if (boot_cpu_has(X86_FEATURE_AUTOIBRS)) {
+			pr_err(SPECTRE_V2_EIBRS_AMD_MSG);
+			mode = SPECTRE_V2_EIBRS;
+		} else
+			mode = SPECTRE_V2_EIBRS_RETPOLINE;
 		break;
 	}

@@ -1508,8 +1511,13 @@ static void __init spectre_v2_select_mitigation(void)
 		pr_err(SPECTRE_V2_EIBRS_EBPF_MSG);

 	if (spectre_v2_in_ibrs_mode(mode)) {
-		x86_spec_ctrl_base |= SPEC_CTRL_IBRS;
-		write_spec_ctrl_current(x86_spec_ctrl_base, true);
+		if (boot_cpu_has(X86_FEATURE_AUTOIBRS)) {
+			rdmsrl(MSR_EFER, efer);
+			wrmsrl(MSR_EFER, efer | EFER_AUTOIBRS);
+		} else {
+			x86_spec_ctrl_base |= SPEC_CTRL_IBRS;
+			write_spec_ctrl_current(x86_spec_ctrl_base, true);
+		}
 	}

 	switch (mode) {
@@ -1517,11 +1525,6 @@ static void __init spectre_v2_select_mitigation(void)
 	case SPECTRE_V2_EIBRS:
 		break;

-	case SPECTRE_V2_AUTO_IBRS:
-		rdmsrl(MSR_EFER, efer);
-		wrmsrl(MSR_EFER, efer | EFER_AUTOIBRS);
-		break;
-
 	case SPECTRE_V2_IBRS:
 		setup_force_cpu_cap(X86_FEATURE_KERNEL_IBRS);
 		if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED))
@@ -1616,8 +1619,8 @@ static void __init spectre_v2_select_mitigation(void)
 			pr_info("Enabling Speculation Barrier for firmware calls\n");
 		}

-	} else if (boot_cpu_has(X86_FEATURE_IBRS) && !spectre_v2_in_ibrs_mode(mode) &&
-		   mode != SPECTRE_V2_AUTO_IBRS) {
+	} else if ((boot_cpu_has(X86_FEATURE_IBRS) && !spectre_v2_in_ibrs_mode(mode)) ||
+		   (boot_cpu_has(X86_FEATURE_AUTOIBRS) && !spectre_v2_in_ibrs_mode(mode))) {
 		setup_force_cpu_cap(X86_FEATURE_USE_IBRS_FW);
 		pr_info("Enabling Restricted Speculation for firmware calls\n");
 	}
@@ -2353,7 +2356,8 @@ static ssize_t mmio_stale_data_show_state(char *buf)

 static char *stibp_state(void)
 {
-	if (spectre_v2_in_ibrs_mode(spectre_v2_enabled))
+	if (spectre_v2_in_ibrs_mode(spectre_v2_enabled) &&
+	    !boot_cpu_has(X86_FEATURE_AUTOIBRS))
 		return "";

 	switch (spectre_v2_user_stibp) {



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux