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 --- 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 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 --- 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