[kvm-unit-tests PATCH 0/2] nVMX: Improve IA32_DEBUGCTLMSR test on debug controls

[kvm-unit-tests PATCH 0/2] nVMX: Improve IA32_DEBUGCTLMSR test on debug controls

[Chenyi Qiang]

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.

Try to avoid these error messages by using some valid bit in
IA32_DEBUGCTLMSR in a separate test.

Chenyi Qiang (2):
  nVMX: Remove the IA32_DEBUGCTLMSR access in debugctls test
  nVMX: Test IA32_DEBUGCTLMSR behavior on set and cleared save/load
    debug controls

 x86/vmx_tests.c | 134 ++++++++++++++++++++++++++++++++++++------------
 1 file changed, 102 insertions(+), 32 deletions(-)


base-commit: 989726f28e1e2137a5cafbe187fffc335cc15b2a
-- 
2.43.5

[kvm-unit-tests PATCH 1/2] nVMX: Remove the IA32_DEBUGCTLMSR access in debugctls test

[Chenyi Qiang]

Current debug controls test can pass but will trigger some error
messages because it tries to access LBR (bit 0) and BTF (bit 1) in
IA32_DEBUGCTLMSR:

  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

The IA32_DEBUGCTLMSR value isn't used as a criterion for determining
whether the test is passed. It only provides some hints on the expected
values with the control of {ENT_LOAD, EXIT_SAVE}_DBGCTLS. The reality is
different because KVM only allows the guest to access the valid bits
depending on the supported features. Luckily, KVM will exempt BTF and
LBR from validity check which makes the test survive.

Considering that IA32_DEBUGCTLMSR access is not practically effective
and will bring error messages, eliminate the related code and rename
the test to specifically address the DR7 check.

Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.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 0b3cfe50..1832bda3 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -1850,21 +1850,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);
@@ -1872,23 +1869,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) {
@@ -1899,46 +1892,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);
@@ -1946,10 +1930,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;
 		}
@@ -11402,7 +11384,7 @@ struct vmx_test vmx_tests[] = {
 		interrupt_exit_handler, NULL, {0} },
 	{ "nmi_hlt", nmi_hlt_init, nmi_hlt_main,
 		nmi_hlt_exit_handler, NULL, {0} },
-	{ "debug controls", dbgctls_init, dbgctls_main, dbgctls_exit_handler,
+	{ "debug controls dr7", dbgctls_dr7_init, dbgctls_dr7_main, dbgctls_dr7_exit_handler,
 		NULL, {0} },
 	{ "MSR switch", msr_switch_init, msr_switch_main,
 		msr_switch_exit_handler, NULL, {0}, msr_switch_entry_failure },
-- 
2.43.5

Re: [kvm-unit-tests PATCH 1/2] nVMX: Remove the IA32_DEBUGCTLMSR access in debugctls test

[Xiaoyao Li]

On 8/11/2025 2:30 PM, Chenyi Qiang wrote:
> Current debug controls test can pass but will trigger some error
> messages because it tries to access LBR (bit 0) and BTF (bit 1) in
> IA32_DEBUGCTLMSR:
> 
>    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
> 
> The IA32_DEBUGCTLMSR value isn't used as a criterion for determining
> whether the test is passed. It only provides some hints on the expected
> values with the control of {ENT_LOAD, EXIT_SAVE}_DBGCTLS. The reality is
> different because KVM only allows the guest to access the valid bits
> depending on the supported features. Luckily, KVM will exempt BTF and
> LBR from validity check which makes the test survive.
> 
> Considering that IA32_DEBUGCTLMSR access is not practically effective
> and will bring error messages, eliminate the related code and rename
> the test to specifically address the DR7 check.

I would expect you explained it more clear, e.g.,

"debug controls" test was added by commit[0] 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 testcase.

Current KVM does support some bits of MSR_IA32_DEBUGCTL but they depend 
on the vcpu model exposed. To simplify the case and eliminate the 
confusing "Unhandled WRMSR(0x1d9)" log, remove the MSR_IA32_DEBUGCTL 
logic in the test and make it concentrate only on DR7. Following patch 
will bring back MSR_IA32_DEBUGCTL separately.

[0] dc5c01f17b1a ("VMX: Test behavior on set and cleared save/load debug 
controls")

> Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com>

For the code change,

Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.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 0b3cfe50..1832bda3 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -1850,21 +1850,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);
> @@ -1872,23 +1869,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) {
> @@ -1899,46 +1892,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);
> @@ -1946,10 +1930,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;
>   		}
> @@ -11402,7 +11384,7 @@ struct vmx_test vmx_tests[] = {
>   		interrupt_exit_handler, NULL, {0} },
>   	{ "nmi_hlt", nmi_hlt_init, nmi_hlt_main,
>   		nmi_hlt_exit_handler, NULL, {0} },
> -	{ "debug controls", dbgctls_init, dbgctls_main, dbgctls_exit_handler,
> +	{ "debug controls dr7", dbgctls_dr7_init, dbgctls_dr7_main, dbgctls_dr7_exit_handler,
>   		NULL, {0} },
>   	{ "MSR switch", msr_switch_init, msr_switch_main,
>   		msr_switch_exit_handler, NULL, {0}, msr_switch_entry_failure },

Re: [kvm-unit-tests PATCH 1/2] nVMX: Remove the IA32_DEBUGCTLMSR access in debugctls test

[Chenyi Qiang]

On 8/11/2025 3:29 PM, Xiaoyao Li wrote:
> On 8/11/2025 2:30 PM, Chenyi Qiang wrote:
>> Current debug controls test can pass but will trigger some error
>> messages because it tries to access LBR (bit 0) and BTF (bit 1) in
>> IA32_DEBUGCTLMSR:
>>
>>    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
>>
>> The IA32_DEBUGCTLMSR value isn't used as a criterion for determining
>> whether the test is passed. It only provides some hints on the expected
>> values with the control of {ENT_LOAD, EXIT_SAVE}_DBGCTLS. The reality is
>> different because KVM only allows the guest to access the valid bits
>> depending on the supported features. Luckily, KVM will exempt BTF and
>> LBR from validity check which makes the test survive.
>>
>> Considering that IA32_DEBUGCTLMSR access is not practically effective
>> and will bring error messages, eliminate the related code and rename
>> the test to specifically address the DR7 check.
> 
> I would expect you explained it more clear, e.g.,
> 
> "debug controls" test was added by commit[0] 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 testcase.
> 
> Current KVM does support some bits of MSR_IA32_DEBUGCTL but they depend on the vcpu model exposed. To simplify the case and eliminate the confusing "Unhandled WRMSR(0x1d9)" log, remove the MSR_IA32_DEBUGCTL logic in the test and make it concentrate only on DR7. Following patch will bring back MSR_IA32_DEBUGCTL separately.
> 
> [0] dc5c01f17b1a ("VMX: Test behavior on set and cleared save/load debug controls")

It looks much clearer. Thanks for the rewrite!

[kvm-unit-tests PATCH 2/2] nVMX: Test IA32_DEBUGCTLMSR behavior on set and cleared save/load debug controls

[Chenyi Qiang]

Besides the existing DR7 test on debug controls, introduce a similar
separate test for IA32_DEBUGCTLMSR.

Previously, the IA32_DEBUGCTLMSR was combined with the DR7 test. However,
it attempted to access the LBR and BTF bits in the MSR which can be
invalid. Although KVM will exempt these two bits from validity check,
they will be cleared and resulted in the unexpected MSR value.

In this new test, access a valid bit (DEBUGCTLMSR_BUS_LOCK_DETECT, bit 2)
based on the enumration of Bus Lock Detect.

Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com>
---
 x86/vmx_tests.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 88 insertions(+)

diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 1832bda3..9a2e598f 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -1944,6 +1944,92 @@ static int dbgctls_dr7_exit_handler(union exit_reason exit_reason)
 	return VMX_TEST_VMEXIT;
 }
 
+static int dbgctls_msr_init(struct vmcs *vmcs)
+{
+	/* Check for DEBUGCTLMSR_BUS_LOCK_DETECT(bit 2) in IA32_DEBUGCTLMSR */
+	if (!(cpuid(7).c & (1 << 24))) {
+		report_skip("%s : \"Bus Lock Detect\" not supported", __func__);
+		return VMX_TEST_VMSKIP;
+	}
+
+	msr_bmp_init();
+	wrmsr(MSR_IA32_DEBUGCTLMSR, 0x0);
+	vmcs_write(GUEST_DEBUGCTL, 0x4);
+
+	vmcs_write(ENT_CONTROLS, vmcs_read(ENT_CONTROLS) | ENT_LOAD_DBGCTLS);
+	vmcs_write(EXI_CONTROLS, vmcs_read(EXI_CONTROLS) | EXI_SAVE_DBGCTLS);
+
+	return VMX_TEST_START;
+}
+
+static void dbgctls_msr_main(void)
+{
+	u64 debugctl;
+
+	debugctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
+	report(debugctl == 0x4, "DEBUGCTLMSR: Load debug controls");
+
+	vmx_set_test_stage(0);
+	vmcall();
+	report(vmx_get_test_stage() == 1, "DEBUGCTLMSR: Save debug controls");
+
+	if (ctrl_enter_rev.set & ENT_LOAD_DBGCTLS ||
+	    ctrl_exit_rev.set & EXI_SAVE_DBGCTLS) {
+		printf("\tDebug controls are always loaded/saved\n");
+		return;
+	}
+	vmx_set_test_stage(2);
+	vmcall();
+
+	debugctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
+	report(debugctl == 0x0,
+	       "DEBUGCTLMSR: Guest=host debug controls");
+
+	vmx_set_test_stage(3);
+	vmcall();
+	report(vmx_get_test_stage() == 4, "DEBUGCTLMSR: Don't save debug controls");
+}
+
+static int dbgctls_msr_exit_handler(union exit_reason exit_reason)
+{
+	u32 insn_len = vmcs_read(EXI_INST_LEN);
+	u64 guest_rip = vmcs_read(GUEST_RIP);
+	u64 debugctl;
+
+	debugctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
+
+	switch (exit_reason.basic) {
+	case VMX_VMCALL:
+		switch (vmx_get_test_stage()) {
+		case 0:
+			if (debugctl == 0 &&
+			    vmcs_read(GUEST_DEBUGCTL) == 0x4)
+				vmx_inc_test_stage();
+			break;
+		case 2:
+			wrmsr(MSR_IA32_DEBUGCTLMSR, 0x0);
+			vmcs_write(GUEST_DEBUGCTL, 0x4);
+
+			vmcs_write(ENT_CONTROLS,
+				vmcs_read(ENT_CONTROLS) & ~ENT_LOAD_DBGCTLS);
+			vmcs_write(EXI_CONTROLS,
+				vmcs_read(EXI_CONTROLS) & ~EXI_SAVE_DBGCTLS);
+			break;
+		case 3:
+			if (debugctl == 0 &&
+			    vmcs_read(GUEST_DEBUGCTL) == 0x4)
+				vmx_inc_test_stage();
+			break;
+		}
+		vmcs_write(GUEST_RIP, guest_rip + insn_len);
+		return VMX_TEST_RESUME;
+	default:
+		report_fail("Unknown exit reason, %d", exit_reason.full);
+		print_vmexit_info(exit_reason);
+	}
+	return VMX_TEST_VMEXIT;
+}
+
 struct vmx_msr_entry {
 	u32 index;
 	u32 reserved;
@@ -11386,6 +11472,8 @@ struct vmx_test vmx_tests[] = {
 		nmi_hlt_exit_handler, NULL, {0} },
 	{ "debug controls dr7", dbgctls_dr7_init, dbgctls_dr7_main, dbgctls_dr7_exit_handler,
 		NULL, {0} },
+	{ "debug controls msr", dbgctls_msr_init, dbgctls_msr_main, dbgctls_msr_exit_handler,
+		NULL, {0} },
 	{ "MSR switch", msr_switch_init, msr_switch_main,
 		msr_switch_exit_handler, NULL, {0}, msr_switch_entry_failure },
 	{ "vmmcall", vmmcall_init, vmmcall_main, vmmcall_exit_handler, NULL, {0} },
-- 
2.43.5

Re: [kvm-unit-tests PATCH 2/2] nVMX: Test IA32_DEBUGCTLMSR behavior on set and cleared save/load debug controls

[Xiaoyao Li]

On 8/11/2025 2:30 PM, Chenyi Qiang wrote:
> Besides the existing DR7 test on debug controls, introduce a similar
> separate test for IA32_DEBUGCTLMSR.
> 
> Previously, the IA32_DEBUGCTLMSR was combined with the DR7 test. However,
> it attempted to access the LBR and BTF bits in the MSR which can be
> invalid. Although KVM will exempt these two bits from validity check,
> they will be cleared and resulted in the unexpected MSR value.

Initially, LBR (bit 0) and BTF(bit 1) have been allowed to write by the 
guest but the value are dropped, as the workaround to not break some OS 
that writes to the MSR unconditionally.

BTF never gets supported by KVM. While LBR gained support but it depends 
on PMU and LBR being exposed to the guest, which requires additional 
parameters to the test configuration.

On the other hand, DEBUGCTLMSR_BUS_LOCK_DETECT chosen by this patch 
doesn't require additional parameter, but it requires the hardware 
support of the feature. IIRC, it needs to be SPR and later. So it has 
less coverage of hardwares than LBR.

I'm not sure the preference of Sean/Paolo. Let's see what they would say.

> In this new test, access a valid bit (DEBUGCTLMSR_BUS_LOCK_DETECT, bit 2)
> based on the enumration of Bus Lock Detect.
> 
> Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com>
> ---
>   x86/vmx_tests.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 88 insertions(+)
> 
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index 1832bda3..9a2e598f 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -1944,6 +1944,92 @@ static int dbgctls_dr7_exit_handler(union exit_reason exit_reason)
>   	return VMX_TEST_VMEXIT;
>   }
>   
> +static int dbgctls_msr_init(struct vmcs *vmcs)
> +{
> +	/* Check for DEBUGCTLMSR_BUS_LOCK_DETECT(bit 2) in IA32_DEBUGCTLMSR */
> +	if (!(cpuid(7).c & (1 << 24))) {
> +		report_skip("%s : \"Bus Lock Detect\" not supported", __func__);
> +		return VMX_TEST_VMSKIP;
> +	}
> +
> +	msr_bmp_init();
> +	wrmsr(MSR_IA32_DEBUGCTLMSR, 0x0);
> +	vmcs_write(GUEST_DEBUGCTL, 0x4);
> +
> +	vmcs_write(ENT_CONTROLS, vmcs_read(ENT_CONTROLS) | ENT_LOAD_DBGCTLS);
> +	vmcs_write(EXI_CONTROLS, vmcs_read(EXI_CONTROLS) | EXI_SAVE_DBGCTLS);
> +
> +	return VMX_TEST_START;
> +}
> +
> +static void dbgctls_msr_main(void)
> +{
> +	u64 debugctl;
> +
> +	debugctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
> +	report(debugctl == 0x4, "DEBUGCTLMSR: Load debug controls");
> +
> +	vmx_set_test_stage(0);
> +	vmcall();
> +	report(vmx_get_test_stage() == 1, "DEBUGCTLMSR: Save debug controls");
> +
> +	if (ctrl_enter_rev.set & ENT_LOAD_DBGCTLS ||
> +	    ctrl_exit_rev.set & EXI_SAVE_DBGCTLS) {
> +		printf("\tDebug controls are always loaded/saved\n");
> +		return;
> +	}
> +	vmx_set_test_stage(2);
> +	vmcall();
> +
> +	debugctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
> +	report(debugctl == 0x0,
> +	       "DEBUGCTLMSR: Guest=host debug controls");
> +
> +	vmx_set_test_stage(3);
> +	vmcall();
> +	report(vmx_get_test_stage() == 4, "DEBUGCTLMSR: Don't save debug controls");
> +}
> +
> +static int dbgctls_msr_exit_handler(union exit_reason exit_reason)
> +{
> +	u32 insn_len = vmcs_read(EXI_INST_LEN);
> +	u64 guest_rip = vmcs_read(GUEST_RIP);
> +	u64 debugctl;
> +
> +	debugctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
> +
> +	switch (exit_reason.basic) {
> +	case VMX_VMCALL:
> +		switch (vmx_get_test_stage()) {
> +		case 0:
> +			if (debugctl == 0 &&
> +			    vmcs_read(GUEST_DEBUGCTL) == 0x4)
> +				vmx_inc_test_stage();
> +			break;
> +		case 2:
> +			wrmsr(MSR_IA32_DEBUGCTLMSR, 0x0);
> +			vmcs_write(GUEST_DEBUGCTL, 0x4);
> +
> +			vmcs_write(ENT_CONTROLS,
> +				vmcs_read(ENT_CONTROLS) & ~ENT_LOAD_DBGCTLS);
> +			vmcs_write(EXI_CONTROLS,
> +				vmcs_read(EXI_CONTROLS) & ~EXI_SAVE_DBGCTLS);
> +			break;
> +		case 3:
> +			if (debugctl == 0 &&
> +			    vmcs_read(GUEST_DEBUGCTL) == 0x4)
> +				vmx_inc_test_stage();
> +			break;
> +		}
> +		vmcs_write(GUEST_RIP, guest_rip + insn_len);
> +		return VMX_TEST_RESUME;
> +	default:
> +		report_fail("Unknown exit reason, %d", exit_reason.full);
> +		print_vmexit_info(exit_reason);
> +	}
> +	return VMX_TEST_VMEXIT;
> +}
> +
>   struct vmx_msr_entry {
>   	u32 index;
>   	u32 reserved;
> @@ -11386,6 +11472,8 @@ struct vmx_test vmx_tests[] = {
>   		nmi_hlt_exit_handler, NULL, {0} },
>   	{ "debug controls dr7", dbgctls_dr7_init, dbgctls_dr7_main, dbgctls_dr7_exit_handler,
>   		NULL, {0} },
> +	{ "debug controls msr", dbgctls_msr_init, dbgctls_msr_main, dbgctls_msr_exit_handler,
> +		NULL, {0} },
>   	{ "MSR switch", msr_switch_init, msr_switch_main,
>   		msr_switch_exit_handler, NULL, {0}, msr_switch_entry_failure },
>   	{ "vmmcall", vmmcall_init, vmmcall_main, vmmcall_exit_handler, NULL, {0} },

Re: [kvm-unit-tests PATCH 2/2] nVMX: Test IA32_DEBUGCTLMSR behavior on set and cleared save/load debug controls

[Chenyi Qiang]

On 8/11/2025 3:47 PM, Xiaoyao Li wrote:
> On 8/11/2025 2:30 PM, Chenyi Qiang wrote:
>> Besides the existing DR7 test on debug controls, introduce a similar
>> separate test for IA32_DEBUGCTLMSR.
>>
>> Previously, the IA32_DEBUGCTLMSR was combined with the DR7 test. However,
>> it attempted to access the LBR and BTF bits in the MSR which can be
>> invalid. Although KVM will exempt these two bits from validity check,
>> they will be cleared and resulted in the unexpected MSR value.
> 
> Initially, LBR (bit 0) and BTF(bit 1) have been allowed to write by the guest but the value are dropped, as the workaround to not break some OS that writes to the MSR unconditionally.
> 
> BTF never gets supported by KVM. While LBR gained support but it depends on PMU and LBR being exposed to the guest, which requires additional parameters to the test configuration.
> 
> On the other hand, DEBUGCTLMSR_BUS_LOCK_DETECT chosen by this patch doesn't require additional parameter, but it requires the hardware support of the feature. IIRC, it needs to be SPR and later. So it has less coverage of hardwares than LBR.

Yes, and DEBUGCTLMSR_RTM_DEBUG can also support in earlier platform but it becomes valid recently..

I looked at my test code again. It seems it loses some test scope. i.e. DEBUGCTLMSR: Save debug controls
I missed to do wrmsr(DEBUGCTLMSR) with a different value in L2, so it cannot distinguish if vmcs12->debugctl comes from the original one or the debugctls saved one.
wrmsr(DEBUGCTLMSR) to 0 is not appropriate because we need to check if VM exit resets the debugctl msr value.

Maybe swapping the value of vmcs_write(GUEST_DEBUGCTL) and wrmsr(DEBUGCTLMSR) is a resolution.

diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 9a2e598f..6dd1735d 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -1953,8 +1953,8 @@ static int dbgctls_msr_init(struct vmcs *vmcs)
 	}
 
 	msr_bmp_init();
-	wrmsr(MSR_IA32_DEBUGCTLMSR, 0x0);
-	vmcs_write(GUEST_DEBUGCTL, 0x4);
+	wrmsr(MSR_IA32_DEBUGCTLMSR, 0x4);
+	vmcs_write(GUEST_DEBUGCTL, 0x0);
 
 	vmcs_write(ENT_CONTROLS, vmcs_read(ENT_CONTROLS) | ENT_LOAD_DBGCTLS);
 	vmcs_write(EXI_CONTROLS, vmcs_read(EXI_CONTROLS) | EXI_SAVE_DBGCTLS);
@@ -1967,8 +1967,9 @@ static void dbgctls_msr_main(void)
 	u64 debugctl;
 
 	debugctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
-	report(debugctl == 0x4, "DEBUGCTLMSR: Load debug controls");
+	report(debugctl == 0x0, "DEBUGCTLMSR: Load debug controls");
 
+	wrmsr(MSR_IA32_DEBUGCTLMSR, 0x4);
 	vmx_set_test_stage(0);
 	vmcall();
 	report(vmx_get_test_stage() == 1, "DEBUGCTLMSR: Save debug controls");
@@ -1982,7 +1983,7 @@ static void dbgctls_msr_main(void)
 	vmcall();
 
 	debugctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
-	report(debugctl == 0x0,
+	report(debugctl == 0x4,
 	       "DEBUGCTLMSR: Guest=host debug controls");
 
 	vmx_set_test_stage(3);
@@ -2007,8 +2008,8 @@ static int dbgctls_msr_exit_handler(union exit_reason exit_reason)
 				vmx_inc_test_stage();
 			break;
 		case 2:
-			wrmsr(MSR_IA32_DEBUGCTLMSR, 0x0);
-			vmcs_write(GUEST_DEBUGCTL, 0x4);
+			wrmsr(MSR_IA32_DEBUGCTLMSR, 0x4);
+			vmcs_write(GUEST_DEBUGCTL, 0x0);
 
 			vmcs_write(ENT_CONTROLS,
 				vmcs_read(ENT_CONTROLS) & ~ENT_LOAD_DBGCTLS);
@@ -2017,7 +2018,7 @@ static int dbgctls_msr_exit_handler(union exit_reason exit_reason)
 			break;
 		case 3:
 			if (debugctl == 0 &&
-			    vmcs_read(GUEST_DEBUGCTL) == 0x4)
+			    vmcs_read(GUEST_DEBUGCTL) == 0x0)
 				vmx_inc_test_stage();
 			break;
 		}

> 
> I'm not sure the preference of Sean/Paolo. Let's see what they would say.
> 
>> In this new test, access a valid bit (DEBUGCTLMSR_BUS_LOCK_DETECT, bit 2)
>> based on the enumration of Bus Lock Detect.
>>
>> Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com>
>> ---
>>   x86/vmx_tests.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 88 insertions(+)
>>
>> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
>> index 1832bda3..9a2e598f 100644
>> --- a/x86/vmx_tests.c
>> +++ b/x86/vmx_tests.c
>> @@ -1944,6 +1944,92 @@ static int dbgctls_dr7_exit_handler(union exit_reason exit_reason)
>>       return VMX_TEST_VMEXIT;
>>   }
>>   +static int dbgctls_msr_init(struct vmcs *vmcs)
>> +{
>> +    /* Check for DEBUGCTLMSR_BUS_LOCK_DETECT(bit 2) in IA32_DEBUGCTLMSR */
>> +    if (!(cpuid(7).c & (1 << 24))) {
>> +        report_skip("%s : \"Bus Lock Detect\" not supported", __func__);
>> +        return VMX_TEST_VMSKIP;
>> +    }
>> +
>> +    msr_bmp_init();
>> +    wrmsr(MSR_IA32_DEBUGCTLMSR, 0x0);
>> +    vmcs_write(GUEST_DEBUGCTL, 0x4);
>> +
>> +    vmcs_write(ENT_CONTROLS, vmcs_read(ENT_CONTROLS) | ENT_LOAD_DBGCTLS);
>> +    vmcs_write(EXI_CONTROLS, vmcs_read(EXI_CONTROLS) | EXI_SAVE_DBGCTLS);
>> +
>> +    return VMX_TEST_START;
>> +}
>> +
>> +static void dbgctls_msr_main(void)
>> +{
>> +    u64 debugctl;
>> +
>> +    debugctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
>> +    report(debugctl == 0x4, "DEBUGCTLMSR: Load debug controls");
>> +
>> +    vmx_set_test_stage(0);
>> +    vmcall();
>> +    report(vmx_get_test_stage() == 1, "DEBUGCTLMSR: Save debug controls");
>> +
>> +    if (ctrl_enter_rev.set & ENT_LOAD_DBGCTLS ||
>> +        ctrl_exit_rev.set & EXI_SAVE_DBGCTLS) {
>> +        printf("\tDebug controls are always loaded/saved\n");
>> +        return;
>> +    }
>> +    vmx_set_test_stage(2);
>> +    vmcall();
>> +
>> +    debugctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
>> +    report(debugctl == 0x0,
>> +           "DEBUGCTLMSR: Guest=host debug controls");
>> +
>> +    vmx_set_test_stage(3);
>> +    vmcall();
>> +    report(vmx_get_test_stage() == 4, "DEBUGCTLMSR: Don't save debug controls");
>> +}
>> +
>> +static int dbgctls_msr_exit_handler(union exit_reason exit_reason)
>> +{
>> +    u32 insn_len = vmcs_read(EXI_INST_LEN);
>> +    u64 guest_rip = vmcs_read(GUEST_RIP);
>> +    u64 debugctl;
>> +
>> +    debugctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
>> +
>> +    switch (exit_reason.basic) {
>> +    case VMX_VMCALL:
>> +        switch (vmx_get_test_stage()) {
>> +        case 0:
>> +            if (debugctl == 0 &&
>> +                vmcs_read(GUEST_DEBUGCTL) == 0x4)
>> +                vmx_inc_test_stage();
>> +            break;
>> +        case 2:
>> +            wrmsr(MSR_IA32_DEBUGCTLMSR, 0x0);
>> +            vmcs_write(GUEST_DEBUGCTL, 0x4);
>> +
>> +            vmcs_write(ENT_CONTROLS,
>> +                vmcs_read(ENT_CONTROLS) & ~ENT_LOAD_DBGCTLS);
>> +            vmcs_write(EXI_CONTROLS,
>> +                vmcs_read(EXI_CONTROLS) & ~EXI_SAVE_DBGCTLS);
>> +            break;
>> +        case 3:
>> +            if (debugctl == 0 &&
>> +                vmcs_read(GUEST_DEBUGCTL) == 0x4)
>> +                vmx_inc_test_stage();
>> +            break;
>> +        }
>> +        vmcs_write(GUEST_RIP, guest_rip + insn_len);
>> +        return VMX_TEST_RESUME;
>> +    default:
>> +        report_fail("Unknown exit reason, %d", exit_reason.full);
>> +        print_vmexit_info(exit_reason);
>> +    }
>> +    return VMX_TEST_VMEXIT;
>> +}
>> +
>>   struct vmx_msr_entry {
>>       u32 index;
>>       u32 reserved;
>> @@ -11386,6 +11472,8 @@ struct vmx_test vmx_tests[] = {
>>           nmi_hlt_exit_handler, NULL, {0} },
>>       { "debug controls dr7", dbgctls_dr7_init, dbgctls_dr7_main, dbgctls_dr7_exit_handler,
>>           NULL, {0} },
>> +    { "debug controls msr", dbgctls_msr_init, dbgctls_msr_main, dbgctls_msr_exit_handler,
>> +        NULL, {0} },
>>       { "MSR switch", msr_switch_init, msr_switch_main,
>>           msr_switch_exit_handler, NULL, {0}, msr_switch_entry_failure },
>>       { "vmmcall", vmmcall_init, vmmcall_main, vmmcall_exit_handler, NULL, {0} },
> 

Re: [kvm-unit-tests PATCH 0/2] nVMX: Improve IA32_DEBUGCTLMSR test on debug controls

[Chenyi Qiang]

A kindly ping.

Hi Paolo/Sean, any comments on changing the nested debugctls test to suppress the warning in this manner? 

On 8/11/2025 2:30 PM, Chenyi Qiang wrote:
> 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.
> 
> Try to avoid these error messages by using some valid bit in
> IA32_DEBUGCTLMSR in a separate test.
> 
> Chenyi Qiang (2):
>   nVMX: Remove the IA32_DEBUGCTLMSR access in debugctls test
>   nVMX: Test IA32_DEBUGCTLMSR behavior on set and cleared save/load
>     debug controls
> 
>  x86/vmx_tests.c | 134 ++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 102 insertions(+), 32 deletions(-)
> 
> 
> base-commit: 989726f28e1e2137a5cafbe187fffc335cc15b2a