[kvm-unit-tests PATCH 0/3] x86/msr: Add SPEC_CTRL coverage

[kvm-unit-tests PATCH 0/3] x86/msr: Add SPEC_CTRL coverage

[Sean Christopherson]

Add test coverage for SPEC_CTRL, which detects the bug pointed by Chao[1]
when running on hosts with V_SPEC_CTRL.

Note, this applies on top of the X86_FEATURE_XXX cleanup[2].

[1] https://lore.kernel.org/all/aEE4BEHAHdhNTGoG@intel.com
[2] https://lore.kernel.org/all/20250529221929.3807680-1-seanjc@google.com

Sean Christopherson (3):
  x86/msr: Treat PRED_CMD as support if CPU has SBPB
  x86/msr: Add a testcase to verify SPEC_CTRL exists (or not) as
    expected
  x86/msr: Add an "msr64" test configuration to validate negative cases

 lib/x86/msr.h       |  8 ++++++--
 lib/x86/processor.h | 10 ++++++++--
 x86/msr.c           | 34 +++++++++++++++++++++++++++++++---
 x86/unittests.cfg   |  7 +++++++
 4 files changed, 52 insertions(+), 7 deletions(-)


base-commit: a93196db8339df0c09d900f9a465820a9d932c1b
-- 
2.50.0.rc0.604.gd4ff7b7c86-goog

[kvm-unit-tests PATCH 1/3] x86/msr: Treat PRED_CMD as support if CPU has SBPB

[Sean Christopherson]

The PRED_CMD MSR also exists if the CPU supports SBPB.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 lib/x86/processor.h | 1 +
 x86/msr.c           | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/x86/processor.h b/lib/x86/processor.h
index b656ebf6..9e3659d4 100644
--- a/lib/x86/processor.h
+++ b/lib/x86/processor.h
@@ -329,6 +329,7 @@ struct x86_cpu_feature {
 #define X86_FEATURE_SME_COHERENT	X86_CPU_FEATURE(0x8000001F, 0, EAX, 10)
 #define X86_FEATURE_DEBUG_SWAP		X86_CPU_FEATURE(0x8000001F, 0, EAX, 14)
 #define X86_FEATURE_SVSM		X86_CPU_FEATURE(0x8000001F, 0, EAX, 28)
+#define	X86_FEATURE_SBPB		X86_CPU_FEATURE(0x80000021, 0, EAX, 27)
 #define	X86_FEATURE_AMD_PMU_V2		X86_CPU_FEATURE(0x80000022, 0, EAX, 0)
 
 /*
diff --git a/x86/msr.c b/x86/msr.c
index f582a584..ac12d127 100644
--- a/x86/msr.c
+++ b/x86/msr.c
@@ -296,7 +296,8 @@ static void test_cmd_msrs(void)
 
 	test_rdmsr_fault(MSR_IA32_PRED_CMD, "PRED_CMD");
 	if (this_cpu_has(X86_FEATURE_SPEC_CTRL) ||
-	    this_cpu_has(X86_FEATURE_AMD_IBPB)) {
+	    this_cpu_has(X86_FEATURE_AMD_IBPB) ||
+	    this_cpu_has(X86_FEATURE_SBPB)) {
 		test_wrmsr(MSR_IA32_PRED_CMD, "PRED_CMD", 0);
 		test_wrmsr(MSR_IA32_PRED_CMD, "PRED_CMD", PRED_CMD_IBPB);
 	} else {
-- 
2.50.0.rc0.604.gd4ff7b7c86-goog

[kvm-unit-tests PATCH 2/3] x86/msr: Add a testcase to verify SPEC_CTRL exists (or not) as expected

[Sean Christopherson]

Verify that SPEC_CTRL can be read when it should exist, #GPs on all reads
and writes does not exist, and that various bits can be set when they're
supported.

Opportunistically define more AMD mitigation features.

Cc: Chao Gao <chao.gao@intel.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 lib/x86/msr.h       |  8 ++++++--
 lib/x86/processor.h |  9 +++++++--
 x86/msr.c           | 31 +++++++++++++++++++++++++++++--
 3 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/lib/x86/msr.h b/lib/x86/msr.h
index ccfd6bdd..cc4cb855 100644
--- a/lib/x86/msr.h
+++ b/lib/x86/msr.h
@@ -32,8 +32,12 @@
 #define EFER_FFXSR		(1<<_EFER_FFXSR)
 
 /* Intel MSRs. Some also available on other CPUs */
-#define MSR_IA32_SPEC_CTRL              0x00000048
-#define MSR_IA32_PRED_CMD               0x00000049
+#define MSR_IA32_SPEC_CTRL		0x00000048
+#define SPEC_CTRL_IBRS			BIT(0)
+#define SPEC_CTRL_STIBP			BIT(1)
+#define SPEC_CTRL_SSBD			BIT(2)
+
+#define MSR_IA32_PRED_CMD		0x00000049
 #define PRED_CMD_IBPB			BIT(0)
 
 #define MSR_IA32_FLUSH_CMD		0x0000010b
diff --git a/lib/x86/processor.h b/lib/x86/processor.h
index 9e3659d4..cbfaa018 100644
--- a/lib/x86/processor.h
+++ b/lib/x86/processor.h
@@ -288,13 +288,13 @@ struct x86_cpu_feature {
 #define	X86_FEATURE_LA57		X86_CPU_FEATURE(0x7, 0, ECX, 16)
 #define	X86_FEATURE_RDPID		X86_CPU_FEATURE(0x7, 0, ECX, 22)
 #define	X86_FEATURE_SHSTK		X86_CPU_FEATURE(0x7, 0, ECX, 7)
+#define	X86_FEATURE_PKS			X86_CPU_FEATURE(0x7, 0, ECX, 31)
 #define	X86_FEATURE_IBT			X86_CPU_FEATURE(0x7, 0, EDX, 20)
 #define	X86_FEATURE_SPEC_CTRL		X86_CPU_FEATURE(0x7, 0, EDX, 26)
 #define	X86_FEATURE_FLUSH_L1D		X86_CPU_FEATURE(0x7, 0, EDX, 28)
 #define	X86_FEATURE_ARCH_CAPABILITIES	X86_CPU_FEATURE(0x7, 0, EDX, 29)
-#define	X86_FEATURE_PKS			X86_CPU_FEATURE(0x7, 0, ECX, 31)
+#define X86_FEATURE_SSBD		X86_CPU_FEATURE(0x7, 0, EDX, 31)
 #define	X86_FEATURE_LAM			X86_CPU_FEATURE(0x7, 1, EAX, 26)
-
 /*
  * KVM defined leafs
  */
@@ -312,6 +312,11 @@ struct x86_cpu_feature {
 #define	X86_FEATURE_LM			X86_CPU_FEATURE(0x80000001, 0, EDX, 29)
 #define	X86_FEATURE_RDPRU		X86_CPU_FEATURE(0x80000008, 0, EBX, 4)
 #define	X86_FEATURE_AMD_IBPB		X86_CPU_FEATURE(0x80000008, 0, EBX, 12)
+#define	X86_FEATURE_AMD_IBRS		X86_CPU_FEATURE(0x80000008, 0, EBX, 14)
+#define X86_FEATURE_AMD_STIBP		X86_CPU_FEATURE(0x80000008, 0, EBX, 15)
+#define X86_FEATURE_AMD_STIBP_ALWAYS_ON	X86_CPU_FEATURE(0x80000008, 0, EBX, 17)
+#define X86_FEATURE_AMD_IBRS_SAME_MODE	X86_CPU_FEATURE(0x80000008, 0, EBX, 19)
+#define X86_FEATURE_AMD_SSBD		X86_CPU_FEATURE(0x80000008, 0, EBX, 24)
 #define	X86_FEATURE_NPT			X86_CPU_FEATURE(0x8000000A, 0, EDX, 0)
 #define	X86_FEATURE_LBRV		X86_CPU_FEATURE(0x8000000A, 0, EDX, 1)
 #define	X86_FEATURE_NRIPS		X86_CPU_FEATURE(0x8000000A, 0, EDX, 3)
diff --git a/x86/msr.c b/x86/msr.c
index ac12d127..ca265fac 100644
--- a/x86/msr.c
+++ b/x86/msr.c
@@ -290,10 +290,37 @@ static void test_x2apic_msrs(void)
 	__test_x2apic_msrs(true);
 }
 
-static void test_cmd_msrs(void)
+static void test_mitigation_msrs(void)
 {
+	u64 spec_ctrl_bits = 0, val;
 	int i;
 
+	if (this_cpu_has(X86_FEATURE_SPEC_CTRL) || this_cpu_has(X86_FEATURE_AMD_IBRS))
+		spec_ctrl_bits |= SPEC_CTRL_IBRS;
+
+	if (this_cpu_has(X86_FEATURE_SPEC_CTRL) || this_cpu_has(X86_FEATURE_AMD_STIBP))
+		spec_ctrl_bits |= SPEC_CTRL_STIBP;
+
+	if (this_cpu_has(X86_FEATURE_SSBD) || this_cpu_has(X86_FEATURE_AMD_SSBD))
+		spec_ctrl_bits |= SPEC_CTRL_SSBD;
+
+	if (spec_ctrl_bits) {
+		for (val = 0; val <= spec_ctrl_bits; val++) {
+			/*
+			 * Test only values that are guaranteed not to fault,
+			 * virtualization of SPEC_CTRL has myriad holes that
+			 * won't be ever closed.
+			 */
+			if ((val & spec_ctrl_bits) != val)
+				continue;
+
+			test_msr_rw(MSR_IA32_SPEC_CTRL, "SPEC_CTRL", val);
+		}
+	} else {
+		test_rdmsr_fault(MSR_IA32_SPEC_CTRL, "SPEC_CTRL");
+		test_wrmsr_fault(MSR_IA32_SPEC_CTRL, "SPEC_CTRL", 0);
+	}
+
 	test_rdmsr_fault(MSR_IA32_PRED_CMD, "PRED_CMD");
 	if (this_cpu_has(X86_FEATURE_SPEC_CTRL) ||
 	    this_cpu_has(X86_FEATURE_AMD_IBPB) ||
@@ -332,7 +359,7 @@ int main(int ac, char **av)
 		test_misc_msrs();
 		test_mce_msrs();
 		test_x2apic_msrs();
-		test_cmd_msrs();
+		test_mitigation_msrs();
 	}
 
 	return report_summary();
-- 
2.50.0.rc0.604.gd4ff7b7c86-goog

Re: [kvm-unit-tests PATCH 2/3] x86/msr: Add a testcase to verify SPEC_CTRL exists (or not) as expected

[Chao Gao]

On Thu, Jun 05, 2025 at 12:26:42PM -0700, Sean Christopherson wrote:
>Verify that SPEC_CTRL can be read when it should exist, #GPs on all reads
>and writes does not exist, and that various bits can be set when they're
>supported.
>
>Opportunistically define more AMD mitigation features.
>
>Cc: Chao Gao <chao.gao@intel.com>
>Signed-off-by: Sean Christopherson <seanjc@google.com>
>---
> lib/x86/msr.h       |  8 ++++++--
> lib/x86/processor.h |  9 +++++++--
> x86/msr.c           | 31 +++++++++++++++++++++++++++++--
> 3 files changed, 42 insertions(+), 6 deletions(-)
>
>diff --git a/lib/x86/msr.h b/lib/x86/msr.h
>index ccfd6bdd..cc4cb855 100644
>--- a/lib/x86/msr.h
>+++ b/lib/x86/msr.h
>@@ -32,8 +32,12 @@
> #define EFER_FFXSR		(1<<_EFER_FFXSR)
> 
> /* Intel MSRs. Some also available on other CPUs */
>-#define MSR_IA32_SPEC_CTRL              0x00000048
>-#define MSR_IA32_PRED_CMD               0x00000049
>+#define MSR_IA32_SPEC_CTRL		0x00000048
>+#define SPEC_CTRL_IBRS			BIT(0)
>+#define SPEC_CTRL_STIBP			BIT(1)
>+#define SPEC_CTRL_SSBD			BIT(2)
>+
>+#define MSR_IA32_PRED_CMD		0x00000049
> #define PRED_CMD_IBPB			BIT(0)
> 
> #define MSR_IA32_FLUSH_CMD		0x0000010b
>diff --git a/lib/x86/processor.h b/lib/x86/processor.h
>index 9e3659d4..cbfaa018 100644
>--- a/lib/x86/processor.h
>+++ b/lib/x86/processor.h
>@@ -288,13 +288,13 @@ struct x86_cpu_feature {
> #define	X86_FEATURE_LA57		X86_CPU_FEATURE(0x7, 0, ECX, 16)
> #define	X86_FEATURE_RDPID		X86_CPU_FEATURE(0x7, 0, ECX, 22)
> #define	X86_FEATURE_SHSTK		X86_CPU_FEATURE(0x7, 0, ECX, 7)
>+#define	X86_FEATURE_PKS			X86_CPU_FEATURE(0x7, 0, ECX, 31)
> #define	X86_FEATURE_IBT			X86_CPU_FEATURE(0x7, 0, EDX, 20)
> #define	X86_FEATURE_SPEC_CTRL		X86_CPU_FEATURE(0x7, 0, EDX, 26)
> #define	X86_FEATURE_FLUSH_L1D		X86_CPU_FEATURE(0x7, 0, EDX, 28)
> #define	X86_FEATURE_ARCH_CAPABILITIES	X86_CPU_FEATURE(0x7, 0, EDX, 29)
>-#define	X86_FEATURE_PKS			X86_CPU_FEATURE(0x7, 0, ECX, 31)
>+#define X86_FEATURE_SSBD		X86_CPU_FEATURE(0x7, 0, EDX, 31)

nit: looks adding a tab after "#define" is the convention in this file

> #define	X86_FEATURE_LAM			X86_CPU_FEATURE(0x7, 1, EAX, 26)
>-
> /*
>  * KVM defined leafs
>  */
>@@ -312,6 +312,11 @@ struct x86_cpu_feature {
> #define	X86_FEATURE_LM			X86_CPU_FEATURE(0x80000001, 0, EDX, 29)
> #define	X86_FEATURE_RDPRU		X86_CPU_FEATURE(0x80000008, 0, EBX, 4)
> #define	X86_FEATURE_AMD_IBPB		X86_CPU_FEATURE(0x80000008, 0, EBX, 12)
>+#define	X86_FEATURE_AMD_IBRS		X86_CPU_FEATURE(0x80000008, 0, EBX, 14)
>+#define X86_FEATURE_AMD_STIBP		X86_CPU_FEATURE(0x80000008, 0, EBX, 15)
>+#define X86_FEATURE_AMD_STIBP_ALWAYS_ON	X86_CPU_FEATURE(0x80000008, 0, EBX, 17)
>+#define X86_FEATURE_AMD_IBRS_SAME_MODE	X86_CPU_FEATURE(0x80000008, 0, EBX, 19)
>+#define X86_FEATURE_AMD_SSBD		X86_CPU_FEATURE(0x80000008, 0, EBX, 24)

ditto

> #define	X86_FEATURE_NPT			X86_CPU_FEATURE(0x8000000A, 0, EDX, 0)
> #define	X86_FEATURE_LBRV		X86_CPU_FEATURE(0x8000000A, 0, EDX, 1)
> #define	X86_FEATURE_NRIPS		X86_CPU_FEATURE(0x8000000A, 0, EDX, 3)
>diff --git a/x86/msr.c b/x86/msr.c
>index ac12d127..ca265fac 100644
>--- a/x86/msr.c
>+++ b/x86/msr.c
>@@ -290,10 +290,37 @@ static void test_x2apic_msrs(void)
> 	__test_x2apic_msrs(true);
> }
> 
>-static void test_cmd_msrs(void)
>+static void test_mitigation_msrs(void)
> {
>+	u64 spec_ctrl_bits = 0, val;
> 	int i;
> 
>+	if (this_cpu_has(X86_FEATURE_SPEC_CTRL) || this_cpu_has(X86_FEATURE_AMD_IBRS))
>+		spec_ctrl_bits |= SPEC_CTRL_IBRS;
>+
>+	if (this_cpu_has(X86_FEATURE_SPEC_CTRL) || this_cpu_has(X86_FEATURE_AMD_STIBP))
>+		spec_ctrl_bits |= SPEC_CTRL_STIBP;

CPUID.(EAX=07H, ECX=0):EDX[26] enumerates IBRS and IBPB support, but it doesn't
enumerate STIBP support. EDX[27] does.

Aside from this, the patch looks good to me.

Reviewed-by: Chao Gao <chao.gao@intel.com>

>+
>+	if (this_cpu_has(X86_FEATURE_SSBD) || this_cpu_has(X86_FEATURE_AMD_SSBD))
>+		spec_ctrl_bits |= SPEC_CTRL_SSBD;
>+
>+	if (spec_ctrl_bits) {
>+		for (val = 0; val <= spec_ctrl_bits; val++) {
>+			/*
>+			 * Test only values that are guaranteed not to fault,
>+			 * virtualization of SPEC_CTRL has myriad holes that
>+			 * won't be ever closed.
>+			 */
>+			if ((val & spec_ctrl_bits) != val)
>+				continue;
>+
>+			test_msr_rw(MSR_IA32_SPEC_CTRL, "SPEC_CTRL", val);
>+		}
>+	} else {
>+		test_rdmsr_fault(MSR_IA32_SPEC_CTRL, "SPEC_CTRL");
>+		test_wrmsr_fault(MSR_IA32_SPEC_CTRL, "SPEC_CTRL", 0);
>+	}
>+
> 	test_rdmsr_fault(MSR_IA32_PRED_CMD, "PRED_CMD");
> 	if (this_cpu_has(X86_FEATURE_SPEC_CTRL) ||
> 	    this_cpu_has(X86_FEATURE_AMD_IBPB) ||
>@@ -332,7 +359,7 @@ int main(int ac, char **av)
> 		test_misc_msrs();
> 		test_mce_msrs();
> 		test_x2apic_msrs();
>-		test_cmd_msrs();
>+		test_mitigation_msrs();
> 	}
> 
> 	return report_summary();
>-- 
>2.50.0.rc0.604.gd4ff7b7c86-goog
>

Re: [kvm-unit-tests PATCH 2/3] x86/msr: Add a testcase to verify SPEC_CTRL exists (or not) as expected

[Sean Christopherson]

On Fri, Jun 06, 2025, Chao Gao wrote:
> On Thu, Jun 05, 2025 at 12:26:42PM -0700, Sean Christopherson wrote:
> >diff --git a/x86/msr.c b/x86/msr.c
> >index ac12d127..ca265fac 100644
> >--- a/x86/msr.c
> >+++ b/x86/msr.c
> >@@ -290,10 +290,37 @@ static void test_x2apic_msrs(void)
> > 	__test_x2apic_msrs(true);
> > }
> > 
> >-static void test_cmd_msrs(void)
> >+static void test_mitigation_msrs(void)
> > {
> >+	u64 spec_ctrl_bits = 0, val;
> > 	int i;
> > 
> >+	if (this_cpu_has(X86_FEATURE_SPEC_CTRL) || this_cpu_has(X86_FEATURE_AMD_IBRS))
> >+		spec_ctrl_bits |= SPEC_CTRL_IBRS;
> >+
> >+	if (this_cpu_has(X86_FEATURE_SPEC_CTRL) || this_cpu_has(X86_FEATURE_AMD_STIBP))
> >+		spec_ctrl_bits |= SPEC_CTRL_STIBP;
> 
> CPUID.(EAX=07H, ECX=0):EDX[26] enumerates IBRS and IBPB support, but it doesn't
> enumerate STIBP support. EDX[27] does.

Drat, that's what I get for trying to reverse engineer the kernel's mitigations.
It's super obvious when I actually look at the CPUID feature definitions.

Thanks!

[kvm-unit-tests PATCH 3/3] x86/msr: Add an "msr64" test configuration to validate negative cases

[Sean Christopherson]

Add "msr64" to run the MSR test with a CPU model of "qemu64" instead of
"max" to provide coverage for MSRs that exist in the underlying hardware,
but not in the guest's CPU model.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/unittests.cfg | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index a0eef541..a2b351ff 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -171,6 +171,13 @@ arch = x86_64
 file = msr.flat
 extra_params = -cpu max,vendor=GenuineIntel
 
+[msr64]
+# Same as above, but with a minimal 64-bit CPU model to validate cases where an
+# MSR is supported in the underlying hardware, but not the guest's CPU model.
+arch = x86_64
+file = msr.flat
+extra_params = -cpu qemu64,vendor=GenuineIntel
+
 [pmu]
 file = pmu.flat
 extra_params = -cpu max
-- 
2.50.0.rc0.604.gd4ff7b7c86-goog

Re: [kvm-unit-tests PATCH 0/3] x86/msr: Add SPEC_CTRL coverage

[Sean Christopherson]

On Thu, 05 Jun 2025 12:26:40 -0700, Sean Christopherson wrote:
> Add test coverage for SPEC_CTRL, which detects the bug pointed by Chao[1]
> when running on hosts with V_SPEC_CTRL.
> 
> Note, this applies on top of the X86_FEATURE_XXX cleanup[2].
> 
> [1] https://lore.kernel.org/all/aEE4BEHAHdhNTGoG@intel.com
> [2] https://lore.kernel.org/all/20250529221929.3807680-1-seanjc@google.com
> 
> [...]

Applied to kvm-x86 next, with X86_FEATURE_STIBP gating STIBP (thanks Chao!).

[1/3] x86/msr: Treat PRED_CMD as support if CPU has SBPB
      https://github.com/kvm-x86/kvm-unit-tests/commit/5cd94b1b09aa
[2/3] x86/msr: Add a testcase to verify SPEC_CTRL exists (or not) as expected
      https://github.com/kvm-x86/kvm-unit-tests/commit/70445405573d
[3/3] x86/msr: Add an "msr64" test configuration to validate negative cases
      https://github.com/kvm-x86/kvm-unit-tests/commit/55c6fc875a60

--
https://github.com/kvm-x86/kvm-unit-tests/tree/next