From: Chenyi Qiang "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 Signed-off-by: Chenyi Qiang [sean: massage changelog, name testcase dbgctls_dr7] Signed-off-by: Sean Christopherson --- 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 From: Chenyi Qiang 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 Co-developed-by: Sean Christopherson Signed-off-by: Sean Christopherson --- 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