Re: [kvm-unit-tests PATCH v2 2/4] nVMX: Validate DEBUGCTLMSR_BUS_LOCK_DETECT states during VMX transitions

[Sean Christopherson <seanjc@google.com>]

On Wed, May 27, 2026, Chenyi Qiang wrote:
> On 5/26/2026 8:32 PM, Xiaoyao Li wrote:
> > On 5/26/2026 11:16 AM, Chenyi Qiang wrote:
> >> diff --git a/x86/unittests.cfg b/x86/unittests.cfg
> >> index 48835eba..c8426770 100644
> >> --- a/x86/unittests.cfg
> >> +++ b/x86/unittests.cfg
> >> @@ -471,6 +471,14 @@ arch = x86_64
> >>   groups = vmx
> >>   timeout = 240
> >>   +[vmx_bus_lock_detect_test]
> >> +file = vmx.flat
> >> +test_args = "vmx_bus_lock_detect_test"
> >> +qemu_params = -cpu max,+vmx
> >> +arch = x86_64
> >> +groups = vmx
> >> +timeout = 240

Why does this need a separate config, and with a massive timeout?  This should
be nearly instantaneous, no?

Ugh, because you copy+pasted vmx_cet.  That thing doesn't need to exist, the
testcase is run by the common "vmx" entry.  I'll send a patch to drop vmx_cet.

> >> +
> >>   [debug]
> >>   file = debug.flat
> >>   arch = x86_64
> >> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> >> index 91f417c6..a3e52d3b 100644
> >> --- a/x86/vmx_tests.c
> >> +++ b/x86/vmx_tests.c
> >> @@ -11737,6 +11737,100 @@ static void vmx_cet_test(void)
> >>       test_set_guest_finished();
> >>   }
> >>   +static struct vmx_debugctl_test_data {
> >> +    u64 mask;

It's not a mask, it's an explicit value.

> >> +    u64 guest_debugctl;
> >> +} vmx_debugctl_test_data;

There's no need for a struct, only a single u64 is needed to pass information
back and forth since there's a VMCALL after every step.

> >> +static void vmx_debugctl_test_guest(void)
> >> +{
> >> +    vmx_debugctl_test_data.guest_debugctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
> >> +    vmcall();
> >> +
> >> +    wrmsr(MSR_IA32_DEBUGCTLMSR, vmx_debugctl_test_data.mask);
> >> +    vmcall();
> >> +
> >> +    vmx_debugctl_test_data.guest_debugctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
> >> +    vmcall();
> >> +
> >> +    wrmsr(MSR_IA32_DEBUGCTLMSR, vmx_debugctl_test_data.mask);
> >> +    vmcall();
> >> +}
> >> +
> >> +static void vmx_debugctl_transition_test(const char *label, u64 mask)
> >> +{
> >> +    u64 debugctl;
> >> +
> >> +    vmx_debugctl_test_data.mask = mask;
> >> +
> >> +    msr_bmp_init();
> >> +    wrmsr(MSR_IA32_DEBUGCTLMSR, mask);
> >> +    vmcs_write(GUEST_DEBUGCTL, 0x0);
> >> +
> >> +    /* Validate when save/load debug controls are set */
> >> +    vmcs_set_bits(ENT_CONTROLS, ENT_LOAD_DBGCTLS);
> >> +    vmcs_set_bits(EXI_CONTROLS, EXI_SAVE_DBGCTLS);
> >> +
> >> +    test_set_guest(vmx_debugctl_test_guest);
> >> +
> >> +    enter_guest();
> >> +    skip_exit_vmcall();
> >> +    /* Validate VM-entry loads the guest DEBUGCTL value from GUEST_DEBUGCTL. */
> >> +    report(vmx_debugctl_test_data.guest_debugctl == 0x0,
> >> +           "%s: Load debug controls", label);
> >> +
> >> +    enter_guest();
> >> +    skip_exit_vmcall();
> >> +    debugctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
> >> +    /* Validate VM-exit clears host DEBUGCTL and saves guest DEBUGCTL into the VMCS. */
> >> +    report(debugctl == 0x0 && vmcs_read(GUEST_DEBUGCTL) == mask,
> >> +           "%s: Save debug controls", label);
> >> +
> >> +    if (ctrl_enter_rev.set & ENT_LOAD_DBGCTLS ||
> >> +        ctrl_exit_rev.set & EXI_SAVE_DBGCTLS) {
> >> +        printf("\tDebug controls are always loaded/saved\n");
> >> +        test_set_guest_finished();
> >> +        return;
> >> +    }
> >> +
> >> +    wrmsr(MSR_IA32_DEBUGCTLMSR, mask);
> >> +    vmcs_write(GUEST_DEBUGCTL, 0x0);
> >> +    /* Validate when save/load debug controls are clear */

We should also test with only of the controls set.

> >> +    vmcs_clear_bits(ENT_CONTROLS, ENT_LOAD_DBGCTLS);
> >> +    vmcs_clear_bits(EXI_CONTROLS, EXI_SAVE_DBGCTLS);
> >> +
> >> +    enter_guest();
> >> +    skip_exit_vmcall();
> >> +    /* Validate guest observes the host's DEBUGCTL value. */
> >> +    report(vmx_debugctl_test_data.guest_debugctl == mask,
> >> +           "%s: Guest=host debug controls", label);
> >> +
> >> +    wrmsr(MSR_IA32_DEBUGCTLMSR, 0x0);
> >> +    vmcs_write(GUEST_DEBUGCTL, 0x0);
> >> +
> >> +    enter_guest();
> >> +    skip_exit_vmcall();
> >> +    debugctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
> >> +    /* Validate VM-exit clears host DEBUGCTL and does not save guest DEBUGCTL into the VMCS. */

This can/should be done on *every* VM-Exit, using report_fail() to avoid polluting
the PASS results with a relatively uninteresting testcase.

> >> +    report(debugctl == 0x0 && vmcs_read(GUEST_DEBUGCTL) == 0x0,
> >> +           "%s: Don't save debug controls", label);
> >> +
> >> +    enter_guest();
> >> +}
> >> +
> >> +static void vmx_bus_lock_detect_test(void)
> > 
> > the name reads ambiguous to me.

The name would be great, _if_ the test were actually testing BUS_LOCK_DETECT,
i.e. needs to actually generate a bus lock in L2 and verify L1 gets the VM-Exit.
That's what I was suggesting in my earlier feedback: actually test BUS_LOCK_DETECT,
and verify vmcs.GUEST_DEBUGCTL handling as a side effect.

But since you've already gone thorugh most of the effort... :-)

Rather than split this into three tests, bundle it all into one test.  Splitting
makes sense _if_ the guest is actually validating functionality, but if the goal
is purely to validate the values and not the semantics, then bundling is much
more interesting as it allows testing combinations.

And since only three bits are set, just validate all combinations, for both the
host and the guest.  It's a total of 8 values, so at most 64 total testcases to
cover both host and guest.

This works for me, I'll post a v3.

---
 lib/x86/msr.h   |   2 +
 x86/vmx_tests.c | 150 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 152 insertions(+)

diff --git a/lib/x86/msr.h b/lib/x86/msr.h
index 7397809c..8b019f7e 100644
--- a/lib/x86/msr.h
+++ b/lib/x86/msr.h
@@ -103,12 +103,14 @@
 /* DEBUGCTLMSR bits (others vary by model): */
 #define DEBUGCTLMSR_LBR			(1UL <<  0) /* last branch recording */
 #define DEBUGCTLMSR_BTF			(1UL <<  1) /* single-step on branches */
+#define DEBUGCTLMSR_BUS_LOCK_DETECT	(1UL <<  2)
 #define DEBUGCTLMSR_TR			(1UL <<  6)
 #define DEBUGCTLMSR_BTS			(1UL <<  7)
 #define DEBUGCTLMSR_BTINT		(1UL <<  8)
 #define DEBUGCTLMSR_BTS_OFF_OS		(1UL <<  9)
 #define DEBUGCTLMSR_BTS_OFF_USR		(1UL << 10)
 #define DEBUGCTLMSR_FREEZE_LBRS_ON_PMI	(1UL << 11)
+#define DEBUGCTLMSR_RTM_DEBUG		(1UL << 15)
 
 #define MSR_LBR_NHM_FROM	0x00000680
 #define MSR_LBR_NHM_TO		0x000006c0
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index aca93af9..2d6eb451 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -11774,6 +11774,155 @@ static void vmx_cet_test(void)
 	test_set_guest_finished();
 }
 
+static u64 vmx_debugctl_test_val;
+
+static void vmx_debugctl_test_guest(void)
+{
+	for (;;) {
+		vmx_debugctl_test_val = rdmsr(MSR_IA32_DEBUGCTLMSR);
+		vmcall();
+
+		wrmsr(MSR_IA32_DEBUGCTLMSR, vmx_debugctl_test_val);
+		vmcall();
+	}
+}
+
+static void __run_vmx_debugctl_guest(void)
+{
+	u64 val;
+
+	enter_guest();
+	skip_exit_vmcall();
+
+	/* Verify DEBUGCTL is unconditionally zeroed on VM-Exit. */
+	val = rdmsr(MSR_IA32_DEBUGCTLMSR);
+	if (val)
+		report_fail("DEBUGCTL = 0x%lx (not zeroed on VM-Exit)", val);
+}
+
+static u64 run_vmx_debugctl_guest(u64 msr_val, u64 vmcs_val, u64 write_val)
+{
+	u64 read_val;
+
+	wrmsr(MSR_IA32_DEBUGCTLMSR, msr_val);
+	vmcs_write(GUEST_DEBUGCTL, vmcs_val);
+
+	__run_vmx_debugctl_guest();
+	read_val = vmx_debugctl_test_val;
+
+	vmx_debugctl_test_val = write_val;
+	__run_vmx_debugctl_guest();
+
+	return read_val;
+}
+
+static void __vmx_debugctl_test(u64 host_val, u64 guest_val)
+{
+	u64 val, rand;
+
+	/* Validate VM-Entrywhen save/load debug controls are set */
+	vmcs_set_bits(ENT_CONTROLS, ENT_LOAD_DBGCTLS);
+	vmcs_set_bits(EXI_CONTROLS, EXI_SAVE_DBGCTLS);
+
+	/* Verify guest DEBUGCTL is loaded/saved when the controls are set. */
+	val = run_vmx_debugctl_guest(host_val, guest_val, 0);
+	report(val == guest_val, "Load DEBUGCTL = 0x%lx, guest RDMSR = 0x%lx", guest_val, val);
+	val = vmcs_read(GUEST_DEBUGCTL);
+	report(!val, "Save DEBUGCTL = 0x0, guest WRMSR = 0x%lx", val);
+
+	/* Rerun the test with the guest values flipped. */
+	val = run_vmx_debugctl_guest(host_val, 0, guest_val);
+	report(!val, "Load DEBUGCTL = 0x0, guest RDMSR = 0x%lx", val);
+	val = vmcs_read(GUEST_DEBUGCTL);
+	report(val == guest_val, "Save DEBUGCTL = 0x0%lx, guest WRMSR = 0x%lx", val, guest_val);
+
+	/*
+	 * If running without the LOAD control set is supported, validate that
+	 * the guest sees the host's value.  Scribble vmcs.GUEST_DEBUGCTL with
+	 * a random value to verify it's ignored.
+	 */
+	if (!(ctrl_enter_rev.set & ENT_LOAD_DBGCTLS)) {
+		vmcs_clear_bits(ENT_CONTROLS, ENT_LOAD_DBGCTLS);
+
+		val = run_vmx_debugctl_guest(host_val, rdtsc(), guest_val);
+		report(val == host_val, "Host DEBUGCTL = 0x%lx, guest RDMSR = 0x%lx", host_val, val);
+
+		/* The guest value should still be saved on exit! */
+		val = vmcs_read(GUEST_DEBUGCTL);
+		report(val == guest_val, "Save DEBUGCTL = 0x%lx, guest WRMSR = 0x%lx", val, guest_val);
+
+		vmcs_set_bits(ENT_CONTROLS, ENT_LOAD_DBGCTLS);
+	}
+
+	if (!(ctrl_exit_rev.set & EXI_SAVE_DBGCTLS)) {
+		vmcs_clear_bits(EXI_CONTROLS, EXI_SAVE_DBGCTLS);
+
+		/* Verify the guest's value is NOT saved on exit.*/
+		val = run_vmx_debugctl_guest(host_val, guest_val, 0);
+		report(val == guest_val, "Load DEBUGCTL = 0x%lx, guest RDMSR = 0x%lx", guest_val, val);
+		val = vmcs_read(GUEST_DEBUGCTL);
+		report(val == guest_val, "VMREAD DEBUGCTL = 0x%lx, VMWRITE = 0x%lx", val, guest_val);
+
+		/* And again with the values flipped. */
+		val = run_vmx_debugctl_guest(host_val, 0, guest_val);
+		report(!val, "Load DEBUGCTL = 0x0, guest RDMSR = 0x%lx", val);
+		val = vmcs_read(GUEST_DEBUGCTL);
+		report(!val, "VMREAD DEBUGCTL = 0x%lx, VMWRITE = 0x0", val);
+
+		vmcs_set_bits(EXI_CONTROLS, EXI_SAVE_DBGCTLS);
+	}
+
+	if ((ctrl_enter_rev.set & ENT_LOAD_DBGCTLS) ||
+	    (ctrl_exit_rev.set & EXI_SAVE_DBGCTLS))
+		return;
+
+	vmcs_clear_bits(ENT_CONTROLS, ENT_LOAD_DBGCTLS);
+	vmcs_clear_bits(EXI_CONTROLS, EXI_SAVE_DBGCTLS);
+
+	rand = rdtsc();
+	val = run_vmx_debugctl_guest(host_val, rand, guest_val);
+	report(val == host_val, "Host DEBUGCTL = 0x%lx, guest RDMSR = 0x%lx", host_val, val);
+
+	val = vmcs_read(GUEST_DEBUGCTL);
+	report(val == rand, "VMWRITE DEBUGCTL = 0x%lx, VMREAD = 0x%lx", rand, val);
+}
+
+static void vmx_debugctl_test(void)
+{
+	u64 supported_bits = 0;
+	u64 i, j;
+
+	if (this_cpu_has(X86_FEATURE_BUS_LOCK_DETECT))
+		supported_bits |= DEBUGCTLMSR_BUS_LOCK_DETECT;
+
+	if (this_cpu_has(X86_FEATURE_RTM))
+		supported_bits |= DEBUGCTLMSR_RTM_DEBUG;
+
+	if (this_cpu_has(X86_FEATURE_PDCM) && pmu_lbr_version())
+		supported_bits |= DEBUGCTLMSR_LBR;
+
+	if (!supported_bits) {
+		report_skip("%s : No DEBUGCTL features supported", __func__);
+		return;
+	}
+
+	test_set_guest(vmx_debugctl_test_guest);
+	msr_bmp_init();
+
+	for (i = 1; i <= supported_bits; i++) {
+		if ((i & supported_bits) != i)
+			continue;
+
+		for (j = 1; j <= supported_bits; j++) {
+			if ((j & supported_bits) != j)
+				continue;
+
+			__vmx_debugctl_test(i, j);
+		}
+	}
+	test_set_guest_finished();
+}
+
 #define TEST(name) { #name, .v2 = name }
 
 /* name/init/guest_main/exit_handler/vmfail_handler */
@@ -11890,5 +12039,6 @@ struct vmx_test vmx_tests[] = {
 	TEST(vmx_canonical_test),
 	/* "Load CET" VM-entry/exit controls tests. */
 	TEST(vmx_cet_test),
+	TEST(vmx_debugctl_test),
 	{ NULL, NULL, NULL, NULL },
 };

base-commit: da6e732372e64a31010b0c8cbfc550a9d83195ba
--