[PATCH final?] x86/bugs: KVM: Add support for SRSO_MSR_FIX

[PATCH final?] x86/bugs: KVM: Add support for SRSO_MSR_FIX

[Borislav Petkov]

So,

in the interest of finally making some progress here I'd like to commit this
below (will test it one more time just in case but it should work :-P). It is
simple and straight-forward and doesn't need an IBPB when the bit gets
cleared.

A potential future improvement is David's suggestion that there could be a way
for tracking when the first guest gets started, we set the bit then, we make
sure the bit gets set on each logical CPU when the guests migrate across the
machine and when the *last* guest exists, that bit gets cleared again.

It all depends on how much machinery is additionally needed and how much ugly
it becomes and how much noticeable the perf impact even is from simply setting
that bit blindly on every CPU...

But something we can chase later, once *some* fix is there first.

Thx.

---

Add support for

  CPUID Fn8000_0021_EAX[31] (SRSO_MSR_FIX). If this bit is 1, it
  indicates that software may use MSR BP_CFG[BpSpecReduce] to mitigate
  SRSO.

Enable BpSpecReduce to mitigate SRSO across guest/host boundaries.

Switch back to enabling the bit when virtualization is enabled and to
clear the bit when virtualization is disabled because using a MSR slot
would clear the bit when the guest is exited and any training the guest
has done, would potentially influence the host kernel when execution
enters the kernel and hasn't VMRUN the guest yet.

More detail on the public thread in Link.

Co-developed-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Link: https://lore.kernel.org/r/20241202120416.6054-1-bp@kernel.org
---
 Documentation/admin-guide/hw-vuln/srso.rst | 13 ++++++++++++
 arch/x86/include/asm/cpufeatures.h         |  4 ++++
 arch/x86/include/asm/msr-index.h           |  1 +
 arch/x86/kernel/cpu/bugs.c                 | 24 ++++++++++++++++++----
 arch/x86/kvm/svm/svm.c                     |  6 ++++++
 arch/x86/lib/msr.c                         |  2 ++
 6 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/Documentation/admin-guide/hw-vuln/srso.rst b/Documentation/admin-guide/hw-vuln/srso.rst
index 2ad1c05b8c88..66af95251a3d 100644
--- a/Documentation/admin-guide/hw-vuln/srso.rst
+++ b/Documentation/admin-guide/hw-vuln/srso.rst
@@ -104,7 +104,20 @@ The possible values in this file are:
 
    (spec_rstack_overflow=ibpb-vmexit)
 
+ * 'Mitigation: Reduced Speculation':
 
+   This mitigation gets automatically enabled when the above one "IBPB on
+   VMEXIT" has been selected and the CPU supports the BpSpecReduce bit.
+
+   It gets automatically enabled on machines which have the
+   SRSO_USER_KERNEL_NO=1 CPUID bit. In that case, the code logic is to switch
+   to the above =ibpb-vmexit mitigation because the user/kernel boundary is
+   not affected anymore and thus "safe RET" is not needed.
+
+   After enabling the IBPB on VMEXIT mitigation option, the BpSpecReduce bit
+   is detected (functionality present on all such machines) and that
+   practically overrides IBPB on VMEXIT as it has a lot less performance
+   impact and takes care of the guest->host attack vector too.
 
 In order to exploit vulnerability, an attacker needs to:
 
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 508c0dad116b..43653f2704c9 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -468,6 +468,10 @@
 #define X86_FEATURE_IBPB_BRTYPE		(20*32+28) /* MSR_PRED_CMD[IBPB] flushes all branch type predictions */
 #define X86_FEATURE_SRSO_NO		(20*32+29) /* CPU is not affected by SRSO */
 #define X86_FEATURE_SRSO_USER_KERNEL_NO	(20*32+30) /* CPU is not affected by SRSO across user/kernel boundaries */
+#define X86_FEATURE_SRSO_BP_SPEC_REDUCE	(20*32+31) /*
+						    * BP_CFG[BpSpecReduce] can be used to mitigate SRSO for VMs.
+						    * (SRSO_MSR_FIX in the official doc).
+						    */
 
 /*
  * Extended auxiliary flags: Linux defined - for features scattered in various
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 72765b2fe0d8..d35519b337ba 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -721,6 +721,7 @@
 
 /* Zen4 */
 #define MSR_ZEN4_BP_CFG                 0xc001102e
+#define MSR_ZEN4_BP_CFG_BP_SPEC_REDUCE_BIT 4
 #define MSR_ZEN4_BP_CFG_SHARED_BTB_FIX_BIT 5
 
 /* Fam 19h MSRs */
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index a5d0998d7604..1d7afc40f227 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -2522,6 +2522,7 @@ enum srso_mitigation {
 	SRSO_MITIGATION_SAFE_RET,
 	SRSO_MITIGATION_IBPB,
 	SRSO_MITIGATION_IBPB_ON_VMEXIT,
+	SRSO_MITIGATION_BP_SPEC_REDUCE,
 };
 
 enum srso_mitigation_cmd {
@@ -2539,7 +2540,8 @@ static const char * const srso_strings[] = {
 	[SRSO_MITIGATION_MICROCODE]		= "Vulnerable: Microcode, no safe RET",
 	[SRSO_MITIGATION_SAFE_RET]		= "Mitigation: Safe RET",
 	[SRSO_MITIGATION_IBPB]			= "Mitigation: IBPB",
-	[SRSO_MITIGATION_IBPB_ON_VMEXIT]	= "Mitigation: IBPB on VMEXIT only"
+	[SRSO_MITIGATION_IBPB_ON_VMEXIT]	= "Mitigation: IBPB on VMEXIT only",
+	[SRSO_MITIGATION_BP_SPEC_REDUCE]	= "Mitigation: Reduced Speculation"
 };
 
 static enum srso_mitigation srso_mitigation __ro_after_init = SRSO_MITIGATION_NONE;
@@ -2578,7 +2580,7 @@ static void __init srso_select_mitigation(void)
 	    srso_cmd == SRSO_CMD_OFF) {
 		if (boot_cpu_has(X86_FEATURE_SBPB))
 			x86_pred_cmd = PRED_CMD_SBPB;
-		return;
+		goto out;
 	}
 
 	if (has_microcode) {
@@ -2590,7 +2592,7 @@ static void __init srso_select_mitigation(void)
 		 */
 		if (boot_cpu_data.x86 < 0x19 && !cpu_smt_possible()) {
 			setup_force_cpu_cap(X86_FEATURE_SRSO_NO);
-			return;
+			goto out;
 		}
 
 		if (retbleed_mitigation == RETBLEED_MITIGATION_IBPB) {
@@ -2670,6 +2672,12 @@ static void __init srso_select_mitigation(void)
 
 ibpb_on_vmexit:
 	case SRSO_CMD_IBPB_ON_VMEXIT:
+		if (boot_cpu_has(X86_FEATURE_SRSO_BP_SPEC_REDUCE)) {
+			pr_notice("Reducing speculation to address VM/HV SRSO attack vector.\n");
+			srso_mitigation = SRSO_MITIGATION_BP_SPEC_REDUCE;
+			break;
+		}
+
 		if (IS_ENABLED(CONFIG_MITIGATION_IBPB_ENTRY)) {
 			if (has_microcode) {
 				setup_force_cpu_cap(X86_FEATURE_IBPB_ON_VMEXIT);
@@ -2691,7 +2699,15 @@ static void __init srso_select_mitigation(void)
 	}
 
 out:
-	pr_info("%s\n", srso_strings[srso_mitigation]);
+	/*
+	 * Clear the feature flag if this mitigation is not selected as that
+	 * feature flag controls the BpSpecReduce MSR bit toggling in KVM.
+	 */
+	if (srso_mitigation != SRSO_MITIGATION_BP_SPEC_REDUCE)
+		setup_clear_cpu_cap(X86_FEATURE_SRSO_BP_SPEC_REDUCE);
+
+	if (srso_mitigation != SRSO_MITIGATION_NONE)
+		pr_info("%s\n", srso_strings[srso_mitigation]);
 }
 
 #undef pr_fmt
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index a713c803a3a3..77ab66c5bb96 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -607,6 +607,9 @@ static void svm_disable_virtualization_cpu(void)
 	kvm_cpu_svm_disable();
 
 	amd_pmu_disable_virt();
+
+	if (cpu_feature_enabled(X86_FEATURE_SRSO_BP_SPEC_REDUCE))
+		msr_clear_bit(MSR_ZEN4_BP_CFG, MSR_ZEN4_BP_CFG_BP_SPEC_REDUCE_BIT);
 }
 
 static int svm_enable_virtualization_cpu(void)
@@ -684,6 +687,9 @@ static int svm_enable_virtualization_cpu(void)
 		rdmsr(MSR_TSC_AUX, sev_es_host_save_area(sd)->tsc_aux, msr_hi);
 	}
 
+	if (cpu_feature_enabled(X86_FEATURE_SRSO_BP_SPEC_REDUCE))
+		msr_set_bit(MSR_ZEN4_BP_CFG, MSR_ZEN4_BP_CFG_BP_SPEC_REDUCE_BIT);
+
 	return 0;
 }
 
diff --git a/arch/x86/lib/msr.c b/arch/x86/lib/msr.c
index 4bf4fad5b148..5a18ecc04a6c 100644
--- a/arch/x86/lib/msr.c
+++ b/arch/x86/lib/msr.c
@@ -103,6 +103,7 @@ int msr_set_bit(u32 msr, u8 bit)
 {
 	return __flip_bit(msr, bit, true);
 }
+EXPORT_SYMBOL_GPL(msr_set_bit);
 
 /**
  * msr_clear_bit - Clear @bit in a MSR @msr.
@@ -118,6 +119,7 @@ int msr_clear_bit(u32 msr, u8 bit)
 {
 	return __flip_bit(msr, bit, false);
 }
+EXPORT_SYMBOL_GPL(msr_clear_bit);
 
 #ifdef CONFIG_TRACEPOINTS
 void do_trace_write_msr(unsigned int msr, u64 val, int failed)
-- 
2.43.0


-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH final?] x86/bugs: KVM: Add support for SRSO_MSR_FIX

[Patrick Bellasi]

> in the interest of finally making some progress here I'd like to commit this
> below (will test it one more time just in case but it should work :-P). It is
> simple and straight-forward and doesn't need an IBPB when the bit gets
> cleared.

That's indeed simple and straight-forward for the time being.

Maybe a small improvement we could add on top is to have a separate and
dedicated cmdline option?

Indeed, with `X86_FEATURE_SRSO_USER_KERNEL_NO` we are not effectively using an
IBPB on VM-Exit anymore. Something like the diff down below?

Best,
Patrick

---
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 1d7afc40f2272..7609d80eda123 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -2531,6 +2531,7 @@ enum srso_mitigation_cmd {
        SRSO_CMD_SAFE_RET,
        SRSO_CMD_IBPB,
        SRSO_CMD_IBPB_ON_VMEXIT,
+       SRSO_CMD_BP_SPEC_REDUCE,
 };

 static const char * const srso_strings[] = {
@@ -2562,6 +2563,8 @@ static int __init srso_parse_cmdline(char *str)
                srso_cmd = SRSO_CMD_IBPB;
        else if (!strcmp(str, "ibpb-vmexit"))
                srso_cmd = SRSO_CMD_IBPB_ON_VMEXIT;
+       else if (!strcmp(str, "spec-reduce"))
+               srso_cmd = SRSO_CMD_BP_SPEC_REDUCE;
        else
                pr_err("Ignoring unknown SRSO option (%s).", str);

@@ -2617,7 +2620,7 @@ static void __init srso_select_mitigation(void)

        case SRSO_CMD_SAFE_RET:
                if (boot_cpu_has(X86_FEATURE_SRSO_USER_KERNEL_NO))
-                       goto ibpb_on_vmexit;
+                       goto spec_reduce;

                if (IS_ENABLED(CONFIG_MITIGATION_SRSO)) {
                        /*
@@ -2670,14 +2673,7 @@ static void __init srso_select_mitigation(void)
                }
                break;

-ibpb_on_vmexit:
        case SRSO_CMD_IBPB_ON_VMEXIT:
-               if (boot_cpu_has(X86_FEATURE_SRSO_BP_SPEC_REDUCE)) {
-                       pr_notice("Reducing speculation to address VM/HV SRSO attack vector.\n");
-                       srso_mitigation = SRSO_MITIGATION_BP_SPEC_REDUCE;
-                       break;
-               }
-
                if (IS_ENABLED(CONFIG_MITIGATION_IBPB_ENTRY)) {
                        if (has_microcode) {
                                setup_force_cpu_cap(X86_FEATURE_IBPB_ON_VMEXIT);
@@ -2694,6 +2690,14 @@ static void __init srso_select_mitigation(void)
                        pr_err("WARNING: kernel not compiled with MITIGATION_IBPB_ENTRY.\n");
                }
                break;
+
+spec_reduce:
+       case SRSO_CMD_BP_SPEC_REDUCE:
+               if (boot_cpu_has(X86_FEATURE_SRSO_BP_SPEC_REDUCE)) {
+                       pr_notice("Reducing speculation to address VM/HV SRSO attack vector.\n");
+                       srso_mitigation = SRSO_MITIGATION_BP_SPEC_REDUCE;
+                       break;
+               }
        default:
                break;
        }

Re: [PATCH final?] x86/bugs: KVM: Add support for SRSO_MSR_FIX

[Borislav Petkov]

On Tue, Feb 18, 2025 at 02:42:57PM +0000, Patrick Bellasi wrote:
> Maybe a small improvement we could add on top is to have a separate and
> dedicated cmdline option?
> 
> Indeed, with `X86_FEATURE_SRSO_USER_KERNEL_NO` we are not effectively using an
> IBPB on VM-Exit anymore. Something like the diff down below?

Except that I don't see the point of this yet one more cmdline option. Our
mitigations options space is a nightmare. Why do we want to add another one?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH final?] x86/bugs: KVM: Add support for SRSO_MSR_FIX

[Patrick Bellasi]

> On Tue, Feb 18, 2025 at 02:42:57PM +0000, Patrick Bellasi wrote:
> > Maybe a small improvement we could add on top is to have a separate and
> > dedicated cmdline option?
> > 
> > Indeed, with `X86_FEATURE_SRSO_USER_KERNEL_NO` we are not effectively using an
> > IBPB on VM-Exit anymore. Something like the diff down below?
> 
> Except that I don't see the point of this yet one more cmdline option. Our
> mitigations options space is a nightmare. Why do we want to add another one?

The changelog of the following patch provides the motivations.

Do you think something like the following self contained change can be added on
top of your change?

Best,
Patrick

---
From 62bd6151cdb5f8e3322d8c91166cf89b6ed9f5b6 Mon Sep 17 00:00:00 2001
From: Patrick Bellasi <derkling@google.com>
Date: Mon, 24 Feb 2025 17:41:30 +0000
Subject: [PATCH] x86/bugs: Add explicit BP_SPEC_REDUCE cmdline

Some AMD CPUs are vulnerable to SRSO only limited to the Guest->Host
attack vector. When no command line options are specified, the default
SRSO mitigation on these CPUs is "safe-ret", which is optimized to use
"ibpb-vmexit". A further optimization, introduced in [1], replaces IBPB
on VM-Exits with the more efficient BP_SPEC_REDUCE mitigation when the
CPU reports X86_FEATURE_SRSO_BP_SPEC_REDUCE support.

The current implementation in bugs.c automatically selects the best
mitigation for a CPU when no command line options are provided. However,
it lacks the ability to explicitly choose between IBPB and
BP_SPEC_REDUCE.
In some scenarios it could be interesting to mitigate SRSO only when the
low overhead of BP_SPEC_REDUCE is available, without overwise falling
back to an IBPB at each VM-Exit.
More in general, an explicit control is valuable for testing,
benchmarking, and comparing the behavior and overhead of IBPB versus
BP_SPEC_REDUCE.

Add a new kernel cmdline option to explicitly select BP_SPEC_REDUCE.
Do that with a minimal change that does not affect the current SafeRET
"fallback logic". Do warn when reduced speculation is required but the
support not available and properly report the vulnerability reason.

[1] https://lore.kernel.org/lkml/20250218111306.GFZ7RrQh3RD4JKj1lu@fat_crate.local/

Signed-off-by: Patrick Bellasi <derkling@google.com>
---
 arch/x86/kernel/cpu/bugs.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 7fafd98368b91..2d785b2ca4e2e 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -2523,6 +2523,7 @@ enum srso_mitigation {
 	SRSO_MITIGATION_IBPB,
 	SRSO_MITIGATION_IBPB_ON_VMEXIT,
 	SRSO_MITIGATION_BP_SPEC_REDUCE,
+	SRSO_MITIGATION_BP_SPEC_REDUCE_NA,
 };

 enum srso_mitigation_cmd {
@@ -2531,6 +2532,7 @@ enum srso_mitigation_cmd {
 	SRSO_CMD_SAFE_RET,
 	SRSO_CMD_IBPB,
 	SRSO_CMD_IBPB_ON_VMEXIT,
+	SRSO_CMD_BP_SPEC_REDUCE,
 };

 static const char * const srso_strings[] = {
@@ -2542,6 +2544,7 @@ static const char * const srso_strings[] = {
 	[SRSO_MITIGATION_IBPB]			= "Mitigation: IBPB",
 	[SRSO_MITIGATION_IBPB_ON_VMEXIT]	= "Mitigation: IBPB on VMEXIT only",
 	[SRSO_MITIGATION_BP_SPEC_REDUCE]	= "Mitigation: Reduced Speculation",
+	[SRSO_MITIGATION_BP_SPEC_REDUCE_NA]	= "Vulnerable: Reduced Speculation, not available",
 };

 static enum srso_mitigation srso_mitigation __ro_after_init = SRSO_MITIGATION_NONE;
@@ -2562,6 +2565,8 @@ static int __init srso_parse_cmdline(char *str)
 		srso_cmd = SRSO_CMD_IBPB;
 	else if (!strcmp(str, "ibpb-vmexit"))
 		srso_cmd = SRSO_CMD_IBPB_ON_VMEXIT;
+	else if (!strcmp(str, "bp-spec-reduce"))
+		srso_cmd = SRSO_CMD_BP_SPEC_REDUCE;
 	else
 		pr_err("Ignoring unknown SRSO option (%s).", str);

@@ -2672,12 +2677,8 @@ static void __init srso_select_mitigation(void)

 ibpb_on_vmexit:
 	case SRSO_CMD_IBPB_ON_VMEXIT:
-		if (boot_cpu_has(X86_FEATURE_SRSO_BP_SPEC_REDUCE)) {
-			pr_notice("Reducing speculation to address VM/HV SRSO attack vector.\n");
-			srso_mitigation = SRSO_MITIGATION_BP_SPEC_REDUCE;
-			break;
-		}
-
+		if (boot_cpu_has(X86_FEATURE_SRSO_BP_SPEC_REDUCE))
+			goto bp_spec_reduce;
 		if (IS_ENABLED(CONFIG_MITIGATION_IBPB_ENTRY)) {
 			if (has_microcode) {
 				setup_force_cpu_cap(X86_FEATURE_IBPB_ON_VMEXIT);
@@ -2694,6 +2695,17 @@ static void __init srso_select_mitigation(void)
 			pr_err("WARNING: kernel not compiled with MITIGATION_IBPB_ENTRY.\n");
 		}
 		break;
+
+	case SRSO_CMD_BP_SPEC_REDUCE:
+		if (boot_cpu_has(X86_FEATURE_SRSO_BP_SPEC_REDUCE)) {
+bp_spec_reduce:
+			pr_notice("Reducing speculation to address VM/HV SRSO attack vector.\n");
+			srso_mitigation = SRSO_MITIGATION_BP_SPEC_REDUCE;
+			break;
+		} else {
+			srso_mitigation = SRSO_MITIGATION_BP_SPEC_REDUCE_NA;
+			pr_warn("BP_SPEC_REDUCE not supported!\n");
+		}
 	default:
 		break;
 	}
--
2.48.1.711.g2feabab25a-goog

Re: [PATCH final?] x86/bugs: KVM: Add support for SRSO_MSR_FIX

[Yosry Ahmed]

On Wed, Feb 26, 2025 at 06:45:40PM +0000, Patrick Bellasi wrote:
> > On Tue, Feb 18, 2025 at 02:42:57PM +0000, Patrick Bellasi wrote:
> > > Maybe a small improvement we could add on top is to have a separate and
> > > dedicated cmdline option?
> > > 
> > > Indeed, with `X86_FEATURE_SRSO_USER_KERNEL_NO` we are not effectively using an
> > > IBPB on VM-Exit anymore. Something like the diff down below?
> > 
> > Except that I don't see the point of this yet one more cmdline option. Our
> > mitigations options space is a nightmare. Why do we want to add another one?
> 
> The changelog of the following patch provides the motivations.
> 
> Do you think something like the following self contained change can be added on
> top of your change?
> 
> Best,
> Patrick
> 
> ---
> From 62bd6151cdb5f8e3322d8c91166cf89b6ed9f5b6 Mon Sep 17 00:00:00 2001
> From: Patrick Bellasi <derkling@google.com>
> Date: Mon, 24 Feb 2025 17:41:30 +0000
> Subject: [PATCH] x86/bugs: Add explicit BP_SPEC_REDUCE cmdline
> 
> Some AMD CPUs are vulnerable to SRSO only limited to the Guest->Host
> attack vector. When no command line options are specified, the default
> SRSO mitigation on these CPUs is "safe-ret", which is optimized to use
> "ibpb-vmexit". A further optimization, introduced in [1], replaces IBPB
> on VM-Exits with the more efficient BP_SPEC_REDUCE mitigation when the
> CPU reports X86_FEATURE_SRSO_BP_SPEC_REDUCE support.
> 
> The current implementation in bugs.c automatically selects the best
> mitigation for a CPU when no command line options are provided. However,
> it lacks the ability to explicitly choose between IBPB and
> BP_SPEC_REDUCE.
> In some scenarios it could be interesting to mitigate SRSO only when the
> low overhead of BP_SPEC_REDUCE is available, without overwise falling
> back to an IBPB at each VM-Exit.

To add more details about this, we are using ASI as our main mitigation
for SRSO. However, it's likely that bp-spec-reduce is cheaper, so we
basically want to always use bp-spec-reduce if available, if not, we
don't want the ibpb-on-vmexit or safe-ret as they are a lot more
expensive than ASI.

So we want the cmdline option to basically say only use bp-spec-reduce
if it's available, but don't fallback if it isn't. On the other hand we
are enlighting ASI to skip mitigating SRSO if
X86_FEATURE_SRSO_BP_SPEC_REDUCE is enabled

> More in general, an explicit control is valuable for testing,
> benchmarking, and comparing the behavior and overhead of IBPB versus
> BP_SPEC_REDUCE.
> 
> Add a new kernel cmdline option to explicitly select BP_SPEC_REDUCE.
> Do that with a minimal change that does not affect the current SafeRET
> "fallback logic". Do warn when reduced speculation is required but the
> support not available and properly report the vulnerability reason.
> 
> [1] https://lore.kernel.org/lkml/20250218111306.GFZ7RrQh3RD4JKj1lu@fat_crate.local/
> 
> Signed-off-by: Patrick Bellasi <derkling@google.com>
> ---
>  arch/x86/kernel/cpu/bugs.c | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index 7fafd98368b91..2d785b2ca4e2e 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -2523,6 +2523,7 @@ enum srso_mitigation {
>  	SRSO_MITIGATION_IBPB,
>  	SRSO_MITIGATION_IBPB_ON_VMEXIT,
>  	SRSO_MITIGATION_BP_SPEC_REDUCE,
> +	SRSO_MITIGATION_BP_SPEC_REDUCE_NA,
>  };
> 
>  enum srso_mitigation_cmd {
> @@ -2531,6 +2532,7 @@ enum srso_mitigation_cmd {
>  	SRSO_CMD_SAFE_RET,
>  	SRSO_CMD_IBPB,
>  	SRSO_CMD_IBPB_ON_VMEXIT,
> +	SRSO_CMD_BP_SPEC_REDUCE,
>  };
> 
>  static const char * const srso_strings[] = {
> @@ -2542,6 +2544,7 @@ static const char * const srso_strings[] = {
>  	[SRSO_MITIGATION_IBPB]			= "Mitigation: IBPB",
>  	[SRSO_MITIGATION_IBPB_ON_VMEXIT]	= "Mitigation: IBPB on VMEXIT only",
>  	[SRSO_MITIGATION_BP_SPEC_REDUCE]	= "Mitigation: Reduced Speculation",
> +	[SRSO_MITIGATION_BP_SPEC_REDUCE_NA]	= "Vulnerable: Reduced Speculation, not available",
>  };
> 
>  static enum srso_mitigation srso_mitigation __ro_after_init = SRSO_MITIGATION_NONE;
> @@ -2562,6 +2565,8 @@ static int __init srso_parse_cmdline(char *str)
>  		srso_cmd = SRSO_CMD_IBPB;
>  	else if (!strcmp(str, "ibpb-vmexit"))
>  		srso_cmd = SRSO_CMD_IBPB_ON_VMEXIT;
> +	else if (!strcmp(str, "bp-spec-reduce"))
> +		srso_cmd = SRSO_CMD_BP_SPEC_REDUCE;
>  	else
>  		pr_err("Ignoring unknown SRSO option (%s).", str);
> 
> @@ -2672,12 +2677,8 @@ static void __init srso_select_mitigation(void)
> 
>  ibpb_on_vmexit:
>  	case SRSO_CMD_IBPB_ON_VMEXIT:
> -		if (boot_cpu_has(X86_FEATURE_SRSO_BP_SPEC_REDUCE)) {
> -			pr_notice("Reducing speculation to address VM/HV SRSO attack vector.\n");
> -			srso_mitigation = SRSO_MITIGATION_BP_SPEC_REDUCE;
> -			break;
> -		}
> -
> +		if (boot_cpu_has(X86_FEATURE_SRSO_BP_SPEC_REDUCE))
> +			goto bp_spec_reduce;
>  		if (IS_ENABLED(CONFIG_MITIGATION_IBPB_ENTRY)) {
>  			if (has_microcode) {
>  				setup_force_cpu_cap(X86_FEATURE_IBPB_ON_VMEXIT);
> @@ -2694,6 +2695,17 @@ static void __init srso_select_mitigation(void)
>  			pr_err("WARNING: kernel not compiled with MITIGATION_IBPB_ENTRY.\n");
>  		}
>  		break;
> +
> +	case SRSO_CMD_BP_SPEC_REDUCE:
> +		if (boot_cpu_has(X86_FEATURE_SRSO_BP_SPEC_REDUCE)) {
> +bp_spec_reduce:

We can put this label above 'case SRSO_CMD_BP_SPEC_REDUCE' for
consistency with the ibpb_on_vmexit label. The
X86_FEATURE_SRSO_BP_SPEC_REDUCE check will be repeated but it shouldn't
matter in practice and the compiler will probably optimize it anyway.

> +			pr_notice("Reducing speculation to address VM/HV SRSO attack vector.\n");
> +			srso_mitigation = SRSO_MITIGATION_BP_SPEC_REDUCE;
> +			break;
> +		} else {
> +			srso_mitigation = SRSO_MITIGATION_BP_SPEC_REDUCE_NA;

Why do we need SRSO_MITIGATION_BP_SPEC_REDUCE_NA? It seems like in other
cases, if some requirements are not met, we just leave srso_mitigation
set SRSO_MITIGATION_NONE.

> +			pr_warn("BP_SPEC_REDUCE not supported!\n");
> +		}
>  	default:
>  		break;
>  	}
> --
> 2.48.1.711.g2feabab25a-goog

Re: [PATCH final?] x86/bugs: KVM: Add support for SRSO_MSR_FIX

[Borislav Petkov]

On Wed, Feb 26, 2025 at 07:51:08PM +0000, Yosry Ahmed wrote:
> To add more details about this, we are using ASI as our main mitigation
> for SRSO. However, it's likely that bp-spec-reduce is cheaper, so we
> basically want to always use bp-spec-reduce if available, if not, we
> don't want the ibpb-on-vmexit or safe-ret as they are a lot more
> expensive than ASI.
> 
> So we want the cmdline option to basically say only use bp-spec-reduce
> if it's available, but don't fallback if it isn't.

Yap, that should also be a part of the commit message.

> On the other hand we are enlighting ASI to skip mitigating SRSO if
> X86_FEATURE_SRSO_BP_SPEC_REDUCE is enabled

Yap, makes sense.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH final?] x86/bugs: KVM: Add support for SRSO_MSR_FIX

[Borislav Petkov]

On Wed, Feb 26, 2025 at 06:45:40PM +0000, Patrick Bellasi wrote:
> +
> +	case SRSO_CMD_BP_SPEC_REDUCE:
> +		if (boot_cpu_has(X86_FEATURE_SRSO_BP_SPEC_REDUCE)) {
> +bp_spec_reduce:
> +			pr_notice("Reducing speculation to address VM/HV SRSO attack vector.\n");

Probably not needed anymore as that will be in srso_strings which is issued
later.

> +			srso_mitigation = SRSO_MITIGATION_BP_SPEC_REDUCE;
> +			break;
> +		} else {
> +			srso_mitigation = SRSO_MITIGATION_BP_SPEC_REDUCE_NA;
> +			pr_warn("BP_SPEC_REDUCE not supported!\n");
> +		}

This is the part I'm worried about: user hears somewhere "bp-spec-reduce" is
faster, sets it but doesn't know whether the hw even supports it. Machine
boots, warns which is a single line and waaay buried in dmesg and continues
unmitigated.

So *maybe* we can make this a lot more subtle and say:

srso=__dont_fall_back_to_ibpb_on_vmexit_if_bp_spec_reduce__

(joking about the name but that should be the gist of what it means)

and then act accordingly when that is specified along with a big fat:

WARN_ON(..."You should not use this as a mitigation option if you don't know
what you're doing")

along with a big fat splat in dmesg.

Hmmm...?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH final?] x86/bugs: KVM: Add support for SRSO_MSR_FIX

[Patrick Bellasi]

> On Wed, Feb 26, 2025 at 06:45:40PM +0000, Patrick Bellasi wrote:
> > +
> > +	case SRSO_CMD_BP_SPEC_REDUCE:
> > +		if (boot_cpu_has(X86_FEATURE_SRSO_BP_SPEC_REDUCE)) {
> > +bp_spec_reduce:
> > +			pr_notice("Reducing speculation to address VM/HV SRSO attack vector.\n");
>
> Probably not needed anymore as that will be in srso_strings which is issued
> later.

Good point, the above can certainly be dropped.

> > +			srso_mitigation = SRSO_MITIGATION_BP_SPEC_REDUCE;
> > +			break;
> > +		} else {
> > +			srso_mitigation = SRSO_MITIGATION_BP_SPEC_REDUCE_NA;
> > +			pr_warn("BP_SPEC_REDUCE not supported!\n");
> > +		}
> 
> This is the part I'm worried about: user hears somewhere "bp-spec-reduce" is
> faster, sets it but doesn't know whether the hw even supports it. Machine
> boots, warns which is a single line and waaay buried in dmesg and continues
> unmitigated.

That's why we are also going to detect this cases and set
SRSO_MITIGATION_BP_SPEC_REDUCE_NA, so that we get a:

  "Vulnerable: Reduced Speculation, not available"

from vulnerabilities/spec_rstack_overflow, which should be the only place users
look for to assess the effective mitigation posture, ins't it?

> So *maybe* we can make this a lot more subtle and say:
> 
> srso=__dont_fall_back_to_ibpb_on_vmexit_if_bp_spec_reduce__
> 
> (joking about the name but that should be the gist of what it means)

I can think about it, but this seems something different than the common
practice, i.e. specify at cmdline what you want and be prepares on possibly
not to get it.

> and then act accordingly when that is specified along with a big fat:
> 
> WARN_ON(..."You should not use this as a mitigation option if you don't know
> what you're doing")
> 
> along with a big fat splat in dmesg.
> 
> Hmmm...?

After all the above already happens, e.g. if we ask for ibpb-vmexit but the
machine has not the ucode. In this case we still have to check the
vulnerabilities report to know that we are:

  "Vulnerable: No microcode"

--
#include <best/regards.h>

Patrick Bellasi (derkling@)

Re: [PATCH final?] x86/bugs: KVM: Add support for SRSO_MSR_FIX

[Borislav Petkov]

On Mon, Mar 03, 2025 at 03:05:56PM +0000, Patrick Bellasi wrote:
> That's why we are also going to detect this cases and set
> SRSO_MITIGATION_BP_SPEC_REDUCE_NA, so that we get a:
> 
>   "Vulnerable: Reduced Speculation, not available"
> 
> from vulnerabilities/spec_rstack_overflow, which should be the only place users
> look for to assess the effective mitigation posture, ins't it?

If they even look. The strategy so far has been that the kernel should simply
DTRT (it being the default) if the user doesn't know anything about
mitigations etc.

So I have another idea: how about we upstream enough ASI bits - i.e., the
function which checks whether ASI is enabled - and use that in the mitigation
selection?

IOW:
	case SRSO_CMD_BP_SPEC_REDUCE:
		if ((boot_cpu_has(X86_FEATURE_SRSO_BP_SPEC_REDUCE)) {
			select it
		} else {
			if (ASI enabled)
				do not fall back to IBPB;
			else
				fallback to IBPB;
		}

"ASI enabled" will return false upstream - at least initially only, until ASI
is out-of-tree - and then it'll fall back.

On your kernels, it'll return true and there it won't fall back.

We just need to sync with Brendan what "ASI enabled" would be and then it
should work and your backports would be easy in that respect.

Until ASI is not upstream, that is.

Hmmmm?

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH final?] x86/bugs: KVM: Add support for SRSO_MSR_FIX

[Brendan Jackman]

On Tue, Mar 11, 2025 at 01:03:40PM +0100, Borislav Petkov wrote:
> On Mon, Mar 03, 2025 at 03:05:56PM +0000, Patrick Bellasi wrote:
> > That's why we are also going to detect this cases and set
> > SRSO_MITIGATION_BP_SPEC_REDUCE_NA, so that we get a:
> > 
> >   "Vulnerable: Reduced Speculation, not available"
> > 
> > from vulnerabilities/spec_rstack_overflow, which should be the only place users
> > look for to assess the effective mitigation posture, ins't it?
> 
> If they even look. The strategy so far has been that the kernel should simply
> DTRT (it being the default) if the user doesn't know anything about
> mitigations etc.
> 
> So I have another idea: how about we upstream enough ASI bits - i.e., the
> function which checks whether ASI is enabled - and use that in the mitigation
> selection?
> 
> IOW:
> 	case SRSO_CMD_BP_SPEC_REDUCE:
> 		if ((boot_cpu_has(X86_FEATURE_SRSO_BP_SPEC_REDUCE)) {
> 			select it
> 		} else {
> 			if (ASI enabled)
> 				do not fall back to IBPB;
> 			else
> 				fallback to IBPB;
> 		}
> 
> "ASI enabled" will return false upstream - at least initially only, until ASI
> is out-of-tree - and then it'll fall back.
> 
> On your kernels, it'll return true and there it won't fall back.
> 
> We just need to sync with Brendan what "ASI enabled" would be and then it
> should work and your backports would be easy in that respect.
> 
> Until ASI is not upstream, that is.
> 
> Hmmmm?

This seems like a good idea to me, assuming we want ASI in the code
eventually it seems worthwhile to make visible the places where we
know we'll want to update the code when we get it in.

In RFCv2 this would be static_asi_enabled() [1] - I think in the
current implementation it would be fine to use it directly, but in
general we do need to be aware of initializion order.

[0] (first half of)
https://lore.kernel.org/all/DS0PR12MB9273553AE4096FCCBBB4000E94D62@DS0PR12MB9273.namprd12.prod.outlook.com/

[1]
https://lore.kernel.org/linux-mm/20250110-asi-rfc-v2-v2-4-8419288bc805@google.com/

Of course I'm biased here, from my perspective having such mentions of
ASI in the code is unambiguously useful. But if others perceived it as
useless noise I would understand!

Re: [PATCH final?] x86/bugs: KVM: Add support for SRSO_MSR_FIX

[Borislav Petkov]

On Tue, Mar 11, 2025 at 01:36:54PM +0000, Brendan Jackman wrote:
> This seems like a good idea to me, assuming we want ASI in the code
> eventually it seems worthwhile to make visible the places where we
> know we'll want to update the code when we get it in.
> 
> In RFCv2 this would be static_asi_enabled() [1] - I think in the
> current implementation it would be fine to use it directly, but in
> general we do need to be aware of initializion order.

Right, I'd suggest you whack that thing and use cpu_feature_enabled()
directly. No need for the indirection.

And I see you're setting X86_FEATURE_ASI in asi_check_boottime_disable() - I'm
presuming that's early enough so that cpu_select_mitigations() in bugs.c can
see it so that srso_select_mitigation() can act accordingly...

> Of course I'm biased here, from my perspective having such mentions of
> ASI in the code is unambiguously useful. But if others perceived it as
> useless noise I would understand!

Yeah, well, it'll be a single feature check in srso_select_mitigation() with
a big-fat comment in it explaining why so I think that should be ok...

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette