lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251110232642.633672-13-yosry.ahmed@linux.dev>
Date: Mon, 10 Nov 2025 23:26:40 +0000
From: Yosry Ahmed <yosry.ahmed@...ux.dev>
To: Sean Christopherson <seanjc@...gle.com>
Cc: Paolo Bonzini <pbonzini@...hat.com>,
	Kevin Cheng <chengkev@...gle.com>,
	kvm@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Yosry Ahmed <yosry.ahmed@...ux.dev>
Subject: [PATCH v3 12/14] x86/svm: Cleanup LBRV tests

In LBRV tests, failures in the guest trigger a #UD and do not convey
useful debugging info (i.e. expected and actual values of MSRs). Also, a
lot of macros are used to perform branch checks, obscuring the tests to
an extent.

Instead, add a helper to read the branch IPs, and remove the check
macros. Consistently use TEST_EXPECT_EQ() in both virtual host and guest
code, instead of a mix of report(), TEST_EXPECT_EQ(), and #UD.

Opportunisitcally slightly reorder test checks to improve semantics, and
replace the report(true, ..) calls that document the test with comments.

Signed-off-by: Yosry Ahmed <yosry.ahmed@...ux.dev>
---
 x86/svm_tests.c | 138 +++++++++++++++++++++++-------------------------
 1 file changed, 65 insertions(+), 73 deletions(-)

diff --git a/x86/svm_tests.c b/x86/svm_tests.c
index 40e9e7e344ed8..33c92b17c87db 100644
--- a/x86/svm_tests.c
+++ b/x86/svm_tests.c
@@ -3006,34 +3006,17 @@ static void svm_no_nm_test(void)
 	       "fnop with CR0.TS and CR0.EM unset no #NM exception");
 }
 
-static u64 amd_get_lbr_rip(u32 msr)
+/* These functions have to be inlined to avoid affecting LBR registers */
+static __always_inline u64 amd_get_lbr_rip(u32 msr)
 {
 	return rdmsr(msr) & ~AMD_LBR_RECORD_MISPREDICT;
 }
 
-#define HOST_CHECK_LBR(from_expected, to_expected)					\
-do {											\
-	TEST_EXPECT_EQ((u64)from_expected, amd_get_lbr_rip(MSR_IA32_LASTBRANCHFROMIP));	\
-	TEST_EXPECT_EQ((u64)to_expected, amd_get_lbr_rip(MSR_IA32_LASTBRANCHTOIP));	\
-} while (0)
-
-/*
- * FIXME: Do something other than generate an exception to communicate failure.
- * Debugging without expected vs. actual is an absolute nightmare.
- */
-#define GUEST_CHECK_LBR(from_expected, to_expected)				\
-do {										\
-	if ((u64)(from_expected) != amd_get_lbr_rip(MSR_IA32_LASTBRANCHFROMIP))	\
-		asm volatile("ud2");						\
-	if ((u64)(to_expected) != amd_get_lbr_rip(MSR_IA32_LASTBRANCHTOIP))	\
-		asm volatile("ud2");						\
-} while (0)
-
-#define REPORT_GUEST_LBR_ERROR(vmcb)						\
-	report(false, "LBR guest test failed.  Exit reason 0x%x, RIP = %lx, from = %lx, to = %lx, ex from = %lx, ex to = %lx", \
-		       vmcb->control.exit_code, vmcb->save.rip,			\
-		       vmcb->save.br_from, vmcb->save.br_to,			\
-		       vmcb->save.last_excp_from, vmcb->save.last_excp_to)
+static __always_inline void get_lbr_ips(u64 *from, u64 *to)
+{
+	*from = amd_get_lbr_rip(MSR_IA32_LASTBRANCHFROMIP);
+	*to = amd_get_lbr_rip(MSR_IA32_LASTBRANCHTOIP);
+}
 
 #define DO_BRANCH(branch_name)				\
 	asm volatile (					\
@@ -3058,55 +3041,64 @@ u64 dbgctl;
 
 static void svm_lbrv_test_guest1(void)
 {
+	u64 from_ip, to_ip;
+
 	/*
 	 * This guest expects the LBR to be already enabled when it starts,
 	 * it does a branch, and then disables the LBR and then checks.
 	 */
+	dbgctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
+	TEST_EXPECT_EQ(dbgctl, DEBUGCTLMSR_LBR);
 
 	DO_BRANCH(guest_branch0);
 
-	dbgctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
+	/* Disable LBR before the checks to avoid changing the last branch */
 	wrmsr(MSR_IA32_DEBUGCTLMSR, 0);
+	dbgctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
+	TEST_EXPECT_EQ(dbgctl, 0);
 
-	if (dbgctl != DEBUGCTLMSR_LBR)
-		asm volatile("ud2\n");
-	if (rdmsr(MSR_IA32_DEBUGCTLMSR) != 0)
-		asm volatile("ud2\n");
+	get_lbr_ips(&from_ip, &to_ip);
+	TEST_EXPECT_EQ((u64)&guest_branch0_from, from_ip);
+	TEST_EXPECT_EQ((u64)&guest_branch0_to, to_ip);
 
-	GUEST_CHECK_LBR(&guest_branch0_from, &guest_branch0_to);
 	asm volatile ("vmmcall\n");
 }
 
 static void svm_lbrv_test_guest2(void)
 {
+	u64 from_ip, to_ip;
+
 	/*
 	 * This guest expects the LBR to be disabled when it starts,
 	 * enables it, does a branch, disables it and then checks.
 	 */
-
-	DO_BRANCH(guest_branch1);
 	dbgctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
+	TEST_EXPECT_EQ(dbgctl, 0);
 
-	if (dbgctl != 0)
-		asm volatile("ud2\n");
+	DO_BRANCH(guest_branch1);
 
-	GUEST_CHECK_LBR(&host_branch2_from, &host_branch2_to);
+	get_lbr_ips(&from_ip, &to_ip);
+	TEST_EXPECT_EQ((u64)&host_branch2_from, from_ip);
+	TEST_EXPECT_EQ((u64)&host_branch2_to, to_ip);
 
 	wrmsr(MSR_IA32_DEBUGCTLMSR, DEBUGCTLMSR_LBR);
 	dbgctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
+	TEST_EXPECT_EQ(dbgctl, DEBUGCTLMSR_LBR);
+
 	DO_BRANCH(guest_branch2);
 	wrmsr(MSR_IA32_DEBUGCTLMSR, 0);
 
-	if (dbgctl != DEBUGCTLMSR_LBR)
-		asm volatile("ud2\n");
-	GUEST_CHECK_LBR(&guest_branch2_from, &guest_branch2_to);
+	get_lbr_ips(&from_ip, &to_ip);
+	TEST_EXPECT_EQ((u64)&guest_branch2_from, from_ip);
+	TEST_EXPECT_EQ((u64)&guest_branch2_to, to_ip);
 
 	asm volatile ("vmmcall\n");
 }
 
 static void svm_lbrv_test0(void)
 {
-	report(true, "Basic LBR test");
+	u64 from_ip, to_ip;
+
 	wrmsr(MSR_IA32_DEBUGCTLMSR, DEBUGCTLMSR_LBR);
 	DO_BRANCH(host_branch0);
 	dbgctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
@@ -3116,12 +3108,15 @@ static void svm_lbrv_test0(void)
 	dbgctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
 	TEST_EXPECT_EQ(dbgctl, 0);
 
-	HOST_CHECK_LBR(&host_branch0_from, &host_branch0_to);
+	get_lbr_ips(&from_ip, &to_ip);
+	TEST_EXPECT_EQ((u64)&host_branch0_from, from_ip);
+	TEST_EXPECT_EQ((u64)&host_branch0_to, to_ip);
 }
 
+/* Test that without LBRV enabled, guest LBR state does 'leak' to the host(1) */
 static void svm_lbrv_test1(void)
 {
-	report(true, "Test that without LBRV enabled, guest LBR state does 'leak' to the host(1)");
+	u64 from_ip, to_ip;
 
 	svm_setup_vmrun((u64)svm_lbrv_test_guest1);
 	vmcb->control.virt_ext = 0;
@@ -3130,19 +3125,19 @@ static void svm_lbrv_test1(void)
 	DO_BRANCH(host_branch1);
 	SVM_BARE_VMRUN;
 	dbgctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
+	TEST_EXPECT_EQ(dbgctl, 0);
 
-	if (vmcb->control.exit_code != SVM_EXIT_VMMCALL) {
-		REPORT_GUEST_LBR_ERROR(vmcb);
-		return;
-	}
+	TEST_EXPECT_EQ(vmcb->control.exit_code, SVM_EXIT_VMMCALL);
 
-	TEST_EXPECT_EQ(dbgctl, 0);
-	HOST_CHECK_LBR(&guest_branch0_from, &guest_branch0_to);
+	get_lbr_ips(&from_ip, &to_ip);
+	TEST_EXPECT_EQ((u64)&guest_branch0_from, from_ip);
+	TEST_EXPECT_EQ((u64)&guest_branch0_to, to_ip);
 }
 
+/* Test that without LBRV enabled, guest LBR state does 'leak' to the host(2) */
 static void svm_lbrv_test2(void)
 {
-	report(true, "Test that without LBRV enabled, guest LBR state does 'leak' to the host(2)");
+	u64 from_ip, to_ip;
 
 	svm_setup_vmrun((u64)svm_lbrv_test_guest2);
 	vmcb->control.virt_ext = 0;
@@ -3152,25 +3147,25 @@ static void svm_lbrv_test2(void)
 	wrmsr(MSR_IA32_DEBUGCTLMSR, 0);
 	SVM_BARE_VMRUN;
 	dbgctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
-	wrmsr(MSR_IA32_DEBUGCTLMSR, 0);
+	TEST_EXPECT_EQ(dbgctl, 0);
 
-	if (vmcb->control.exit_code != SVM_EXIT_VMMCALL) {
-		REPORT_GUEST_LBR_ERROR(vmcb);
-		return;
-	}
+	TEST_EXPECT_EQ(vmcb->control.exit_code, SVM_EXIT_VMMCALL);
 
-	TEST_EXPECT_EQ(dbgctl, 0);
-	HOST_CHECK_LBR(&guest_branch2_from, &guest_branch2_to);
+	get_lbr_ips(&from_ip, &to_ip);
+	TEST_EXPECT_EQ((u64)&guest_branch2_from, from_ip);
+	TEST_EXPECT_EQ((u64)&guest_branch2_to, to_ip);
 }
 
+/* Test that with LBRV enabled, guest LBR state doesn't leak (1) */
 static void svm_lbrv_nested_test1(void)
 {
+	u64 from_ip, to_ip;
+
 	if (!lbrv_supported()) {
 		report_skip("LBRV not supported in the guest");
 		return;
 	}
 
-	report(true, "Test that with LBRV enabled, guest LBR state doesn't leak (1)");
 	svm_setup_vmrun((u64)svm_lbrv_test_guest1);
 	vmcb->control.virt_ext = LBR_CTL_ENABLE_MASK;
 	vmcb->save.dbgctl = DEBUGCTLMSR_LBR;
@@ -3181,28 +3176,26 @@ static void svm_lbrv_nested_test1(void)
 	dbgctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
 	wrmsr(MSR_IA32_DEBUGCTLMSR, 0);
 
-	if (vmcb->control.exit_code != SVM_EXIT_VMMCALL) {
-		REPORT_GUEST_LBR_ERROR(vmcb);
-		return;
-	}
-
-	if (vmcb->save.dbgctl != 0) {
-		report(false, "unexpected virtual guest MSR_IA32_DEBUGCTLMSR value 0x%lx", vmcb->save.dbgctl);
-		return;
-	}
+	TEST_EXPECT_EQ(vmcb->control.exit_code, SVM_EXIT_VMMCALL);
 
 	TEST_EXPECT_EQ(dbgctl, DEBUGCTLMSR_LBR);
-	HOST_CHECK_LBR(&host_branch3_from, &host_branch3_to);
+	TEST_EXPECT_EQ(vmcb->save.dbgctl, 0);
+
+	get_lbr_ips(&from_ip, &to_ip);
+	TEST_EXPECT_EQ((u64)&host_branch3_from, from_ip);
+	TEST_EXPECT_EQ((u64)&host_branch3_to, to_ip);
 }
 
+/* Test that with LBRV enabled, guest LBR state doesn't leak (2) */
 static void svm_lbrv_nested_test2(void)
 {
+	u64 from_ip, to_ip;
+
 	if (!lbrv_supported()) {
 		report_skip("LBRV not supported in the guest");
 		return;
 	}
 
-	report(true, "Test that with LBRV enabled, guest LBR state doesn't leak (2)");
 	svm_setup_vmrun((u64)svm_lbrv_test_guest2);
 	vmcb->control.virt_ext = LBR_CTL_ENABLE_MASK;
 
@@ -3215,14 +3208,13 @@ static void svm_lbrv_nested_test2(void)
 	SVM_BARE_VMRUN;
 	dbgctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
 	wrmsr(MSR_IA32_DEBUGCTLMSR, 0);
+	TEST_EXPECT_EQ(dbgctl, DEBUGCTLMSR_LBR);
 
-	if (vmcb->control.exit_code != SVM_EXIT_VMMCALL) {
-		REPORT_GUEST_LBR_ERROR(vmcb);
-		return;
-	}
+	TEST_EXPECT_EQ(vmcb->control.exit_code, SVM_EXIT_VMMCALL);
 
-	TEST_EXPECT_EQ(dbgctl, DEBUGCTLMSR_LBR);
-	HOST_CHECK_LBR(&host_branch4_from, &host_branch4_to);
+	get_lbr_ips(&from_ip, &to_ip);
+	TEST_EXPECT_EQ((u64)&host_branch4_from, from_ip);
+	TEST_EXPECT_EQ((u64)&host_branch4_to, to_ip);
 }
 
 
-- 
2.51.2.1041.gc1ab5b90ca-goog


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ