* [kvm-unit-tests PATCH v3 0/2] nVMX: Improve DEBUGCTL test coverage
@ 2026-05-27 15:12 Sean Christopherson
2026-05-27 15:12 ` [kvm-unit-tests PATCH v3 1/2] nVMX: Remove the IA32_DEBUGCTLMSR access in debugctls test Sean Christopherson
2026-05-27 15:12 ` [kvm-unit-tests PATCH v3 2/2] x86/vmx: (Re)Add a nVMX test to validate load/save of DEBUGCTL Sean Christopherson
0 siblings, 2 replies; 7+ messages in thread
From: Sean Christopherson @ 2026-05-27 15:12 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm, Xiaoyao Li, Chenyi Qiang, Sean Christopherson
Current nested debugctls test focuses on DR7 value and fakes the result of
IA32_DEBUGCTLMSR in comment. Although the test can pass because DR7 is the
only criterion to determine the result, there are some error messages
appearing in dmesg due to accessing to BTF | LBR bits in IA32_DEBUGCTLMSR
with report_ignored_msrs KVM parameter enabled.
Clean up the old test so that it focuses on DR7, then add a separate test for
DEBUGCTLMSR bits that are actually supported and can be validated during VMX
transitions.
v3:
- Massage changelog in patch 1. [Xiaoyao]
- Combine all DEBUGCTL tests into one testcase. [Sean]
- Validate all supported combinations. [Sean]
- Independently validate LOAD/SAVE controls. [Sean]
- Drop unnecessary configuration entry. [Sean]
v2:
- Modify the commit message in patch 1 (Xiaoyao)
- Add dedicated tests (for BUS_LOCK_DETECT and LBR) covering the DEBUGCTL
reads and writes during VMX transitions (Sean)
- Add one additional test for RTM_DEBUG bit that was not mentioned in v1.
v1: https://lore.kernel.org/kvm/20250811063035.12626-1-chenyi.qiang@intel.com
Chenyi Qiang (2):
nVMX: Remove the IA32_DEBUGCTLMSR access in debugctls test
x86/vmx: (Re)Add a nVMX test to validate load/save of DEBUGCTL
lib/x86/msr.h | 2 +
x86/vmx_tests.c | 196 ++++++++++++++++++++++++++++++++++++++++--------
2 files changed, 166 insertions(+), 32 deletions(-)
base-commit: 05b7715ffd4ae4ef8f0a5dea869f1543d89f936d
--
2.54.0.823.g6e5bcc1fc9-goog
^ permalink raw reply [flat|nested] 7+ messages in thread
* [kvm-unit-tests PATCH v3 1/2] nVMX: Remove the IA32_DEBUGCTLMSR access in debugctls test
2026-05-27 15:12 [kvm-unit-tests PATCH v3 0/2] nVMX: Improve DEBUGCTL test coverage Sean Christopherson
@ 2026-05-27 15:12 ` Sean Christopherson
2026-05-27 15:12 ` [kvm-unit-tests PATCH v3 2/2] x86/vmx: (Re)Add a nVMX test to validate load/save of DEBUGCTL Sean Christopherson
1 sibling, 0 replies; 7+ messages in thread
From: Sean Christopherson @ 2026-05-27 15:12 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm, Xiaoyao Li, Chenyi Qiang, Sean Christopherson
From: Chenyi Qiang <chenyi.qiang@intel.com>
"debug controls" test was added by commit dc5c01f17b1a ("VMX: Test
behavior on set and cleared save/load debug controls") to verify that
"VM-Entry load debug controls" and "VM-Exit save debug controls" are
correctly emulated by KVM for nested VMX. But due to the limitation that
KVM didn't support MSR_IA32_DEBUGCTL for guest at that time, the test
commented out all the value comparison of MSR_IA32_DEBUGCTL and leave
it to future when KVM supports the MSR.
The test doesn't check the functionality of save/restore guest
MSR_IA32_DEBUGCTL on vm-exit/entry, but it keeps the write of
MSR_IA32_DEBUGCTL. It leads to
kvm_intel: kvm [18663]: vcpu0, guest rIP: 0x407de7 Unhandled WRMSR(0x1d9) = 0x1
kvm_intel: kvm [18663]: vcpu0, guest rIP: 0x0 Unhandled WRMSR(0x1d9) = 0x2
kvm_intel: kvm [18663]: vcpu0, guest rIP: 0x40936f Unhandled WRMSR(0x1d9) = 0x3
kvm_intel: kvm [18663]: vcpu0, guest rIP: 0x40cf09 Unhandled WRMSR(0x1d9) = 0x1
kvm_intel: kvm [18663]: vcpu0, guest rIP: 0x40940d Unhandled WRMSR(0x1d9) = 0x3
to the kernel log. Though it doesn't break the test, the log confuses
people to think something wrong happened in the test case, and the test
isn't actually validating anything useful.
Now that KVM supports some MSR_IA32_DEBUGCTL based features (dependent on
the underlying hardware), drop the old, half-baked code for DEBUGCTL in
anticipation of adding a dedicated functional test for DEBUTCTL. Keep the
existing DR7 coverage, but rename the testcase to explicitly scope it to
just DR7.
Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com>
[sean: massage changelog, name testcase dbgctls_dr7]
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
x86/vmx_tests.c | 46 ++++++++++++++--------------------------------
1 file changed, 14 insertions(+), 32 deletions(-)
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 7aa3194c..e1f53fbe 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -1858,21 +1858,18 @@ static int nmi_hlt_exit_handler(union exit_reason exit_reason)
}
-static int dbgctls_init(struct vmcs *vmcs)
+static int dbgctls_dr7_init(struct vmcs *vmcs)
{
u64 dr7 = 0x402;
u64 zero = 0;
- msr_bmp_init();
asm volatile(
"mov %0,%%dr0\n\t"
"mov %0,%%dr1\n\t"
"mov %0,%%dr2\n\t"
"mov %1,%%dr7\n\t"
: : "r" (zero), "r" (dr7));
- wrmsr(MSR_IA32_DEBUGCTLMSR, 0x1);
vmcs_write(GUEST_DR7, 0x404);
- vmcs_write(GUEST_DEBUGCTL, 0x2);
vmcs_write(ENT_CONTROLS, vmcs_read(ENT_CONTROLS) | ENT_LOAD_DBGCTLS);
vmcs_write(EXI_CONTROLS, vmcs_read(EXI_CONTROLS) | EXI_SAVE_DBGCTLS);
@@ -1880,23 +1877,19 @@ static int dbgctls_init(struct vmcs *vmcs)
return VMX_TEST_START;
}
-static void dbgctls_main(void)
+static void dbgctls_dr7_main(void)
{
- u64 dr7, debugctl;
+ u64 dr7;
asm volatile("mov %%dr7,%0" : "=r" (dr7));
- debugctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
- /* Commented out: KVM does not support DEBUGCTL so far */
- (void)debugctl;
- report(dr7 == 0x404, "Load debug controls" /* && debugctl == 0x2 */);
+ report(dr7 == 0x404, "DR7: Load debug controls");
dr7 = 0x408;
asm volatile("mov %0,%%dr7" : : "r" (dr7));
- wrmsr(MSR_IA32_DEBUGCTLMSR, 0x3);
vmx_set_test_stage(0);
vmcall();
- report(vmx_get_test_stage() == 1, "Save debug controls");
+ report(vmx_get_test_stage() == 1, "DR7: Save debug controls");
if (ctrl_enter_rev.set & ENT_LOAD_DBGCTLS ||
ctrl_exit_rev.set & EXI_SAVE_DBGCTLS) {
@@ -1907,46 +1900,37 @@ static void dbgctls_main(void)
vmcall();
asm volatile("mov %%dr7,%0" : "=r" (dr7));
- debugctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
- /* Commented out: KVM does not support DEBUGCTL so far */
- (void)debugctl;
report(dr7 == 0x402,
- "Guest=host debug controls" /* && debugctl == 0x1 */);
+ "DR7: Guest=host debug controls");
dr7 = 0x408;
asm volatile("mov %0,%%dr7" : : "r" (dr7));
- wrmsr(MSR_IA32_DEBUGCTLMSR, 0x3);
vmx_set_test_stage(3);
vmcall();
- report(vmx_get_test_stage() == 4, "Don't save debug controls");
+ report(vmx_get_test_stage() == 4, "DR7: Don't save debug controls");
}
-static int dbgctls_exit_handler(union exit_reason exit_reason)
+static int dbgctls_dr7_exit_handler(union exit_reason exit_reason)
{
u32 insn_len = vmcs_read(EXI_INST_LEN);
u64 guest_rip = vmcs_read(GUEST_RIP);
- u64 dr7, debugctl;
+ u64 dr7;
asm volatile("mov %%dr7,%0" : "=r" (dr7));
- debugctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
switch (exit_reason.basic) {
case VMX_VMCALL:
switch (vmx_get_test_stage()) {
case 0:
- if (dr7 == 0x400 && debugctl == 0 &&
- vmcs_read(GUEST_DR7) == 0x408 /* &&
- Commented out: KVM does not support DEBUGCTL so far
- vmcs_read(GUEST_DEBUGCTL) == 0x3 */)
+ if (dr7 == 0x400 &&
+ vmcs_read(GUEST_DR7) == 0x408)
vmx_inc_test_stage();
break;
case 2:
dr7 = 0x402;
asm volatile("mov %0,%%dr7" : : "r" (dr7));
- wrmsr(MSR_IA32_DEBUGCTLMSR, 0x1);
vmcs_write(GUEST_DR7, 0x404);
- vmcs_write(GUEST_DEBUGCTL, 0x2);
vmcs_write(ENT_CONTROLS,
vmcs_read(ENT_CONTROLS) & ~ENT_LOAD_DBGCTLS);
@@ -1954,10 +1938,8 @@ static int dbgctls_exit_handler(union exit_reason exit_reason)
vmcs_read(EXI_CONTROLS) & ~EXI_SAVE_DBGCTLS);
break;
case 3:
- if (dr7 == 0x400 && debugctl == 0 &&
- vmcs_read(GUEST_DR7) == 0x404 /* &&
- Commented out: KVM does not support DEBUGCTL so far
- vmcs_read(GUEST_DEBUGCTL) == 0x2 */)
+ if (dr7 == 0x400 &&
+ vmcs_read(GUEST_DR7) == 0x404)
vmx_inc_test_stage();
break;
}
@@ -11813,7 +11795,7 @@ struct vmx_test vmx_tests[] = {
{ "PML", pml_init, pml_main, pml_exit_handler },
{ "interrupt", interrupt_init, interrupt_main, interrupt_exit_handler },
{ "nmi_hlt", nmi_hlt_init, nmi_hlt_main, nmi_hlt_exit_handler },
- { "debug controls", dbgctls_init, dbgctls_main, dbgctls_exit_handler },
+ { "dbgctls_dr7", dbgctls_dr7_init, dbgctls_dr7_main, dbgctls_dr7_exit_handler },
{ "MSR switch", msr_switch_init, msr_switch_main,
msr_switch_exit_handler, msr_switch_entry_failure },
{ "vmmcall", vmmcall_init, vmmcall_main, vmmcall_exit_handler },
--
2.54.0.823.g6e5bcc1fc9-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [kvm-unit-tests PATCH v3 2/2] x86/vmx: (Re)Add a nVMX test to validate load/save of DEBUGCTL
2026-05-27 15:12 [kvm-unit-tests PATCH v3 0/2] nVMX: Improve DEBUGCTL test coverage Sean Christopherson
2026-05-27 15:12 ` [kvm-unit-tests PATCH v3 1/2] nVMX: Remove the IA32_DEBUGCTLMSR access in debugctls test Sean Christopherson
@ 2026-05-27 15:12 ` Sean Christopherson
2026-05-27 15:19 ` Xiaoyao Li
` (2 more replies)
1 sibling, 3 replies; 7+ messages in thread
From: Sean Christopherson @ 2026-05-27 15:12 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm, Xiaoyao Li, Chenyi Qiang, Sean Christopherson
From: Chenyi Qiang <chenyi.qiang@intel.com>
Add a nVMX test to verify that DEBUGCTL is/isn't loaded/saved on entry/exit
as per vmcs12's controls. Validate all combinations of legal values (only
three bits are supported, i.e. there are only 8 unique combinations), for
both the host and the guest, along with '0'.
Validate that:
- DEBUGCTL is loaded from vmcs.GUEST_DEBUGCTL iff "Load debug controls"
is set, otherwise the guest should see the host's value.
- DEBUGCTL is saved to vmcs.GUEST_DEBUGCTL iff "Save debug controls" is
set, otherwise the VMCS should retain its previous value.
- DEBUGCTL is *always* zeroed on VM-Exit, irrespective of guest controls.
Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com>
Co-developed-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
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 e1f53fbe..f914924d 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 },
};
--
2.54.0.823.g6e5bcc1fc9-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [kvm-unit-tests PATCH v3 2/2] x86/vmx: (Re)Add a nVMX test to validate load/save of DEBUGCTL
2026-05-27 15:12 ` [kvm-unit-tests PATCH v3 2/2] x86/vmx: (Re)Add a nVMX test to validate load/save of DEBUGCTL Sean Christopherson
@ 2026-05-27 15:19 ` Xiaoyao Li
2026-05-27 15:21 ` Sean Christopherson
2026-05-28 1:43 ` Chenyi Qiang
2026-05-28 7:16 ` Xiaoyao Li
2 siblings, 1 reply; 7+ messages in thread
From: Xiaoyao Li @ 2026-05-27 15:19 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini; +Cc: kvm, Chenyi Qiang
On 5/27/2026 11:12 PM, Sean Christopherson wrote:
> + 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);
> + }
> + }
What a clever idea!
A minor flaw: Both i and j should start from 0, since 0 is a valid value.
(I will go through the whole patch tomorrow.)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [kvm-unit-tests PATCH v3 2/2] x86/vmx: (Re)Add a nVMX test to validate load/save of DEBUGCTL
2026-05-27 15:19 ` Xiaoyao Li
@ 2026-05-27 15:21 ` Sean Christopherson
0 siblings, 0 replies; 7+ messages in thread
From: Sean Christopherson @ 2026-05-27 15:21 UTC (permalink / raw)
To: Xiaoyao Li; +Cc: Paolo Bonzini, kvm, Chenyi Qiang
On Wed, May 27, 2026, Xiaoyao Li wrote:
> On 5/27/2026 11:12 PM, Sean Christopherson wrote:
> > + 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);
> > + }
> > + }
>
> What a clever idea!
>
> A minor flaw: Both i and j should start from 0, since 0 is a valid value.
Heh, I knew I should have added a comment. I deliberately started at '1', because
'0' is used as a "base" value, to be able to detect changes. I.e. '0' is getting
valiated on every test anyways.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [kvm-unit-tests PATCH v3 2/2] x86/vmx: (Re)Add a nVMX test to validate load/save of DEBUGCTL
2026-05-27 15:12 ` [kvm-unit-tests PATCH v3 2/2] x86/vmx: (Re)Add a nVMX test to validate load/save of DEBUGCTL Sean Christopherson
2026-05-27 15:19 ` Xiaoyao Li
@ 2026-05-28 1:43 ` Chenyi Qiang
2026-05-28 7:16 ` Xiaoyao Li
2 siblings, 0 replies; 7+ messages in thread
From: Chenyi Qiang @ 2026-05-28 1:43 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini; +Cc: kvm, Xiaoyao Li
On 5/27/2026 11:12 PM, Sean Christopherson wrote:
> From: Chenyi Qiang <chenyi.qiang@intel.com>
>
> Add a nVMX test to verify that DEBUGCTL is/isn't loaded/saved on entry/exit
> as per vmcs12's controls. Validate all combinations of legal values (only
> three bits are supported, i.e. there are only 8 unique combinations), for
> both the host and the guest, along with '0'.
>
> Validate that:
>
> - DEBUGCTL is loaded from vmcs.GUEST_DEBUGCTL iff "Load debug controls"
> is set, otherwise the guest should see the host's value.
> - DEBUGCTL is saved to vmcs.GUEST_DEBUGCTL iff "Save debug controls" is
> set, otherwise the VMCS should retain its previous value.
> - DEBUGCTL is *always* zeroed on VM-Exit, irrespective of guest controls.
>
> Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com>
> Co-developed-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> 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 e1f53fbe..f914924d 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);
^
See an extra "0" here during my test :)
And as mentioned in another thread, it requires the "migratable=no" thing in "vmx" testcase to cover LBR bit on my Skylake test.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [kvm-unit-tests PATCH v3 2/2] x86/vmx: (Re)Add a nVMX test to validate load/save of DEBUGCTL
2026-05-27 15:12 ` [kvm-unit-tests PATCH v3 2/2] x86/vmx: (Re)Add a nVMX test to validate load/save of DEBUGCTL Sean Christopherson
2026-05-27 15:19 ` Xiaoyao Li
2026-05-28 1:43 ` Chenyi Qiang
@ 2026-05-28 7:16 ` Xiaoyao Li
2 siblings, 0 replies; 7+ messages in thread
From: Xiaoyao Li @ 2026-05-28 7:16 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini; +Cc: kvm, Chenyi Qiang
On 5/27/2026 11:12 PM, Sean Christopherson wrote:
> From: Chenyi Qiang <chenyi.qiang@intel.com>
>
> Add a nVMX test to verify that DEBUGCTL is/isn't loaded/saved on entry/exit
> as per vmcs12's controls. Validate all combinations of legal values (only
> three bits are supported, i.e. there are only 8 unique combinations), for
> both the host and the guest, along with '0'.
>
> Validate that:
>
> - DEBUGCTL is loaded from vmcs.GUEST_DEBUGCTL iff "Load debug controls"
> is set, otherwise the guest should see the host's value.
> - DEBUGCTL is saved to vmcs.GUEST_DEBUGCTL iff "Save debug controls" is
> set, otherwise the VMCS should retain its previous value.
> - DEBUGCTL is *always* zeroed on VM-Exit, irrespective of guest controls.
>
> Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com>
> Co-developed-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
> +static void __vmx_debugctl_test(u64 host_val, u64 guest_val)
> +{
> + u64 val, rand;
> +
> + /* Validate VM-Entrywhen save/load debug controls are set */
missing a space between "VM-Entry" and "when".
But I don't understand why we need such a comment at all. Does it
explain the purpose of following vmcs write is to validate that the two
vmcs bits are allowed/supported on VM-Entry?
> + vmcs_set_bits(ENT_CONTROLS, ENT_LOAD_DBGCTLS);
> + vmcs_set_bits(EXI_CONTROLS, EXI_SAVE_DBGCTLS);
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-05-28 7:16 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-27 15:12 [kvm-unit-tests PATCH v3 0/2] nVMX: Improve DEBUGCTL test coverage Sean Christopherson
2026-05-27 15:12 ` [kvm-unit-tests PATCH v3 1/2] nVMX: Remove the IA32_DEBUGCTLMSR access in debugctls test Sean Christopherson
2026-05-27 15:12 ` [kvm-unit-tests PATCH v3 2/2] x86/vmx: (Re)Add a nVMX test to validate load/save of DEBUGCTL Sean Christopherson
2026-05-27 15:19 ` Xiaoyao Li
2026-05-27 15:21 ` Sean Christopherson
2026-05-28 1:43 ` Chenyi Qiang
2026-05-28 7:16 ` Xiaoyao Li
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.