From 65eefbe42218cf25dd776a6a996258cc214c2769 Mon Sep 17 00:00:00 2001 From: John Baldwin Date: Wed, 17 Jan 2018 23:11:25 +0000 Subject: [PATCH] Save and restore guest debug registers. Currently most of the debug registers are not saved and restored during VM transitions allowing guest and host debug register values to leak into the opposite context. One result is that hardware watchpoints do not work reliably within a guest under VT-x. Due to differences in SVM and VT-x, slightly different approaches are used. For VT-x: - Enable debug register save/restore for VM entry/exit in the VMCS for DR7 and MSR_DEBUGCTL. - Explicitly save DR0-3,6 of the guest. - Explicitly save DR0-3,6-7, MSR_DEBUGCTL, and the trap flag from %rflags for the host. Note that because DR6 is "software" managed and not stored in the VMCS a kernel debugger which single steps through VM entry could corrupt the guest DR6 (since a single step trap taken after loading the guest DR6 could alter the DR6 register). To avoid this, explicitly disable single-stepping via the trace flag before loading the guest DR6. A determined debugger could still defeat this by setting a breakpoint after the guest DR6 was loaded and then single-stepping. For SVM: - Enable debug register caching in the VMCB for DR6/DR7. - Explicitly save DR0-3 of the guest. - Explicitly save DR0-3,6-7, and MSR_DEBUGCTL for the host. Since SVM saves the guest DR6 in the VMCB, the race with single-stepping described for VT-x does not exist. For both platforms, expose all of the guest DRx values via --get-drX and --set-drX flags to bhyvectl. Discussed with: avg, grehan Tested by: avg (SVM), myself (VT-x) MFC after: 1 month Differential Revision: https://reviews.freebsd.org/D13229 --- sys/amd64/include/vmm.h | 5 ++ sys/amd64/vmm/amd/svm.c | 69 ++++++++++++++++++++++++ sys/amd64/vmm/amd/svm.h | 12 +++++ sys/amd64/vmm/amd/vmcb.c | 10 ++++ sys/amd64/vmm/intel/vmx.c | 94 ++++++++++++++++++++++++++++++-- sys/amd64/vmm/intel/vmx.h | 16 ++++-- usr.sbin/bhyvectl/bhyvectl.c | 101 ++++++++++++++++++++++++++++++++++- 7 files changed, 297 insertions(+), 10 deletions(-) diff --git a/sys/amd64/include/vmm.h b/sys/amd64/include/vmm.h index bc8f553f2962..bc8fdfd34cd2 100644 --- a/sys/amd64/include/vmm.h +++ b/sys/amd64/include/vmm.h @@ -85,6 +85,11 @@ enum vm_reg_name { VM_REG_GUEST_PDPTE2, VM_REG_GUEST_PDPTE3, VM_REG_GUEST_INTR_SHADOW, + VM_REG_GUEST_DR0, + VM_REG_GUEST_DR1, + VM_REG_GUEST_DR2, + VM_REG_GUEST_DR3, + VM_REG_GUEST_DR6, VM_REG_LAST }; diff --git a/sys/amd64/vmm/amd/svm.c b/sys/amd64/vmm/amd/svm.c index fa5b5793839e..5a3de1ecc372 100644 --- a/sys/amd64/vmm/amd/svm.c +++ b/sys/amd64/vmm/amd/svm.c @@ -87,6 +87,7 @@ SYSCTL_NODE(_hw_vmm, OID_AUTO, svm, CTLFLAG_RW, NULL, NULL); VMCB_CACHE_TPR | \ VMCB_CACHE_CR2 | \ VMCB_CACHE_CR | \ + VMCB_CACHE_DR | \ VMCB_CACHE_DT | \ VMCB_CACHE_SEG | \ VMCB_CACHE_NP) @@ -504,6 +505,10 @@ vmcb_init(struct svm_softc *sc, int vcpu, uint64_t iopm_base_pa, PAT_VALUE(5, PAT_WRITE_THROUGH) | PAT_VALUE(6, PAT_UNCACHED) | PAT_VALUE(7, PAT_UNCACHEABLE); + + /* Set up DR6/7 to power-on state */ + state->dr6 = 0xffff0ff0; + state->dr7 = 0x400; } /* @@ -1911,6 +1916,60 @@ enable_gintr(void) __asm __volatile("stgi"); } +static __inline void +svm_dr_enter_guest(struct svm_regctx *gctx) +{ + + /* Save host control debug registers. */ + gctx->host_dr7 = rdr7(); + gctx->host_debugctl = rdmsr(MSR_DEBUGCTLMSR); + + /* + * Disable debugging in DR7 and DEBUGCTL to avoid triggering + * exceptions in the host based on the guest DRx values. The + * guest DR6, DR7, and DEBUGCTL are saved/restored in the + * VMCB. + */ + load_dr7(0); + wrmsr(MSR_DEBUGCTLMSR, 0); + + /* Save host debug registers. */ + gctx->host_dr0 = rdr0(); + gctx->host_dr1 = rdr1(); + gctx->host_dr2 = rdr2(); + gctx->host_dr3 = rdr3(); + gctx->host_dr6 = rdr6(); + + /* Restore guest debug registers. */ + load_dr0(gctx->sctx_dr0); + load_dr1(gctx->sctx_dr1); + load_dr2(gctx->sctx_dr2); + load_dr3(gctx->sctx_dr3); +} + +static __inline void +svm_dr_leave_guest(struct svm_regctx *gctx) +{ + + /* Save guest debug registers. */ + gctx->sctx_dr0 = rdr0(); + gctx->sctx_dr1 = rdr1(); + gctx->sctx_dr2 = rdr2(); + gctx->sctx_dr3 = rdr3(); + + /* + * Restore host debug registers. Restore DR7 and DEBUGCTL + * last. + */ + load_dr0(gctx->host_dr0); + load_dr1(gctx->host_dr1); + load_dr2(gctx->host_dr2); + load_dr3(gctx->host_dr3); + load_dr6(gctx->host_dr6); + wrmsr(MSR_DEBUGCTLMSR, gctx->host_debugctl); + load_dr7(gctx->host_dr7); +} + /* * Start vcpu with specified RIP. */ @@ -2023,7 +2082,9 @@ svm_vmrun(void *arg, int vcpu, register_t rip, pmap_t pmap, /* Launch Virtual Machine. */ VCPU_CTR1(vm, vcpu, "Resume execution at %#lx", state->rip); + svm_dr_enter_guest(gctx); svm_launch(vmcb_pa, gctx, &__pcpu[curcpu]); + svm_dr_leave_guest(gctx); CPU_CLR_ATOMIC(curcpu, &pmap->pm_active); @@ -2092,6 +2153,14 @@ swctx_regptr(struct svm_regctx *regctx, int reg) return (®ctx->sctx_r14); case VM_REG_GUEST_R15: return (®ctx->sctx_r15); + case VM_REG_GUEST_DR0: + return (®ctx->sctx_dr0); + case VM_REG_GUEST_DR1: + return (®ctx->sctx_dr1); + case VM_REG_GUEST_DR2: + return (®ctx->sctx_dr2); + case VM_REG_GUEST_DR3: + return (®ctx->sctx_dr3); default: return (NULL); } diff --git a/sys/amd64/vmm/amd/svm.h b/sys/amd64/vmm/amd/svm.h index 4a931ae7f82d..bab612c86a3e 100644 --- a/sys/amd64/vmm/amd/svm.h +++ b/sys/amd64/vmm/amd/svm.h @@ -49,6 +49,18 @@ struct svm_regctx { register_t sctx_r13; register_t sctx_r14; register_t sctx_r15; + register_t sctx_dr0; + register_t sctx_dr1; + register_t sctx_dr2; + register_t sctx_dr3; + + register_t host_dr0; + register_t host_dr1; + register_t host_dr2; + register_t host_dr3; + register_t host_dr6; + register_t host_dr7; + uint64_t host_debugctl; }; void svm_launch(uint64_t pa, struct svm_regctx *gctx, struct pcpu *pcpu); diff --git a/sys/amd64/vmm/amd/vmcb.c b/sys/amd64/vmm/amd/vmcb.c index d8601690c4d2..b1232c713dfe 100644 --- a/sys/amd64/vmm/amd/vmcb.c +++ b/sys/amd64/vmm/amd/vmcb.c @@ -187,6 +187,10 @@ vmcb_read(struct svm_softc *sc, int vcpu, int ident, uint64_t *retval) *retval = state->cr4; break; + case VM_REG_GUEST_DR6: + *retval = state->dr6; + break; + case VM_REG_GUEST_DR7: *retval = state->dr7; break; @@ -278,8 +282,14 @@ vmcb_write(struct svm_softc *sc, int vcpu, int ident, uint64_t val) svm_set_dirty(sc, vcpu, VMCB_CACHE_CR); break; + case VM_REG_GUEST_DR6: + state->dr6 = val; + svm_set_dirty(sc, vcpu, VMCB_CACHE_DR); + break; + case VM_REG_GUEST_DR7: state->dr7 = val; + svm_set_dirty(sc, vcpu, VMCB_CACHE_DR); break; case VM_REG_GUEST_EFER: diff --git a/sys/amd64/vmm/intel/vmx.c b/sys/amd64/vmm/intel/vmx.c index 5b4a17f4fc4d..25e87f58c7e2 100644 --- a/sys/amd64/vmm/intel/vmx.c +++ b/sys/amd64/vmm/intel/vmx.c @@ -98,18 +98,20 @@ __FBSDID("$FreeBSD$"); #define PROCBASED_CTLS2_ZERO_SETTING 0 #define VM_EXIT_CTLS_ONE_SETTING \ - (VM_EXIT_HOST_LMA | \ + (VM_EXIT_SAVE_DEBUG_CONTROLS | \ + VM_EXIT_HOST_LMA | \ VM_EXIT_SAVE_EFER | \ VM_EXIT_LOAD_EFER | \ VM_EXIT_ACKNOWLEDGE_INTERRUPT) -#define VM_EXIT_CTLS_ZERO_SETTING VM_EXIT_SAVE_DEBUG_CONTROLS +#define VM_EXIT_CTLS_ZERO_SETTING 0 -#define VM_ENTRY_CTLS_ONE_SETTING (VM_ENTRY_LOAD_EFER) +#define VM_ENTRY_CTLS_ONE_SETTING \ + (VM_ENTRY_LOAD_DEBUG_CONTROLS | \ + VM_ENTRY_LOAD_EFER) #define VM_ENTRY_CTLS_ZERO_SETTING \ - (VM_ENTRY_LOAD_DEBUG_CONTROLS | \ - VM_ENTRY_INTO_SMM | \ + (VM_ENTRY_INTO_SMM | \ VM_ENTRY_DEACTIVATE_DUAL_MONITOR) #define HANDLED 1 @@ -916,6 +918,9 @@ vmx_vminit(struct vm *vm, pmap_t pmap) exc_bitmap = 1 << IDT_MC; error += vmwrite(VMCS_EXCEPTION_BITMAP, exc_bitmap); + vmx->ctx[i].guest_dr6 = 0xffff0ff0; + error += vmwrite(VMCS_GUEST_DR7, 0x400); + if (virtual_interrupt_delivery) { error += vmwrite(VMCS_APIC_ACCESS, APIC_ACCESS_ADDRESS); error += vmwrite(VMCS_VIRTUAL_APIC, @@ -2572,6 +2577,73 @@ vmx_exit_handle_nmi(struct vmx *vmx, int vcpuid, struct vm_exit *vmexit) } } +static __inline void +vmx_dr_enter_guest(struct vmxctx *vmxctx) +{ + register_t rflags; + + /* Save host control debug registers. */ + vmxctx->host_dr7 = rdr7(); + vmxctx->host_debugctl = rdmsr(MSR_DEBUGCTLMSR); + + /* + * Disable debugging in DR7 and DEBUGCTL to avoid triggering + * exceptions in the host based on the guest DRx values. The + * guest DR7 and DEBUGCTL are saved/restored in the VMCS. + */ + load_dr7(0); + wrmsr(MSR_DEBUGCTLMSR, 0); + + /* + * Disable single stepping the kernel to avoid corrupting the + * guest DR6. A debugger might still be able to corrupt the + * guest DR6 by setting a breakpoint after this point and then + * single stepping. + */ + rflags = read_rflags(); + vmxctx->host_tf = rflags & PSL_T; + write_rflags(rflags & ~PSL_T); + + /* Save host debug registers. */ + vmxctx->host_dr0 = rdr0(); + vmxctx->host_dr1 = rdr1(); + vmxctx->host_dr2 = rdr2(); + vmxctx->host_dr3 = rdr3(); + vmxctx->host_dr6 = rdr6(); + + /* Restore guest debug registers. */ + load_dr0(vmxctx->guest_dr0); + load_dr1(vmxctx->guest_dr1); + load_dr2(vmxctx->guest_dr2); + load_dr3(vmxctx->guest_dr3); + load_dr6(vmxctx->guest_dr6); +} + +static __inline void +vmx_dr_leave_guest(struct vmxctx *vmxctx) +{ + + /* Save guest debug registers. */ + vmxctx->guest_dr0 = rdr0(); + vmxctx->guest_dr1 = rdr1(); + vmxctx->guest_dr2 = rdr2(); + vmxctx->guest_dr3 = rdr3(); + vmxctx->guest_dr6 = rdr6(); + + /* + * Restore host debug registers. Restore DR7, DEBUGCTL, and + * PSL_T last. + */ + load_dr0(vmxctx->host_dr0); + load_dr1(vmxctx->host_dr1); + load_dr2(vmxctx->host_dr2); + load_dr3(vmxctx->host_dr3); + load_dr6(vmxctx->host_dr6); + wrmsr(MSR_DEBUGCTLMSR, vmxctx->host_debugctl); + load_dr7(vmxctx->host_dr7); + write_rflags(read_rflags() | vmxctx->host_tf); +} + static int vmx_run(void *arg, int vcpu, register_t rip, pmap_t pmap, struct vm_eventinfo *evinfo) @@ -2670,7 +2742,9 @@ vmx_run(void *arg, int vcpu, register_t rip, pmap_t pmap, } vmx_run_trace(vmx, vcpu); + vmx_dr_enter_guest(vmxctx); rc = vmx_enter_guest(vmxctx, vmx, launched); + vmx_dr_leave_guest(vmxctx); /* Collect some information for VM exit processing */ vmexit->rip = rip = vmcs_guest_rip(); @@ -2770,6 +2844,16 @@ vmxctx_regptr(struct vmxctx *vmxctx, int reg) return (&vmxctx->guest_r15); case VM_REG_GUEST_CR2: return (&vmxctx->guest_cr2); + case VM_REG_GUEST_DR0: + return (&vmxctx->guest_dr0); + case VM_REG_GUEST_DR1: + return (&vmxctx->guest_dr1); + case VM_REG_GUEST_DR2: + return (&vmxctx->guest_dr2); + case VM_REG_GUEST_DR3: + return (&vmxctx->guest_dr3); + case VM_REG_GUEST_DR6: + return (&vmxctx->guest_dr6); default: break; } diff --git a/sys/amd64/vmm/intel/vmx.h b/sys/amd64/vmm/intel/vmx.h index 720145a00fa8..422b903ac80a 100644 --- a/sys/amd64/vmm/intel/vmx.h +++ b/sys/amd64/vmm/intel/vmx.h @@ -52,6 +52,11 @@ struct vmxctx { register_t guest_r14; register_t guest_r15; register_t guest_cr2; + register_t guest_dr0; + register_t guest_dr1; + register_t guest_dr2; + register_t guest_dr3; + register_t guest_dr6; register_t host_r15; /* Host state */ register_t host_r14; @@ -60,9 +65,14 @@ struct vmxctx { register_t host_rbp; register_t host_rsp; register_t host_rbx; - /* - * XXX todo debug registers and fpu state - */ + register_t host_dr0; + register_t host_dr1; + register_t host_dr2; + register_t host_dr3; + register_t host_dr6; + register_t host_dr7; + uint64_t host_debugctl; + int host_tf; int inst_fail_status; diff --git a/usr.sbin/bhyvectl/bhyvectl.c b/usr.sbin/bhyvectl/bhyvectl.c index 18a3bf1f3f82..b3ee0bcbf4f7 100644 --- a/usr.sbin/bhyvectl/bhyvectl.c +++ b/usr.sbin/bhyvectl/bhyvectl.c @@ -113,6 +113,16 @@ usage(bool cpu_intel) " [--get-cr3]\n" " [--set-cr4=]\n" " [--get-cr4]\n" + " [--set-dr0=]\n" + " [--get-dr0]\n" + " [--set-dr1=]\n" + " [--get-dr1]\n" + " [--set-dr2=]\n" + " [--get-dr2]\n" + " [--set-dr3=]\n" + " [--get-dr3]\n" + " [--set-dr6=]\n" + " [--get-dr6]\n" " [--set-dr7=]\n" " [--get-dr7]\n" " [--set-rsp=]\n" @@ -246,6 +256,11 @@ static int get_active_cpus, get_suspended_cpus; static uint64_t memsize; static int set_cr0, get_cr0, set_cr3, get_cr3, set_cr4, get_cr4; static int set_efer, get_efer; +static int set_dr0, get_dr0; +static int set_dr1, get_dr1; +static int set_dr2, get_dr2; +static int set_dr3, get_dr3; +static int set_dr6, get_dr6; static int set_dr7, get_dr7; static int set_rsp, get_rsp, set_rip, get_rip, set_rflags, get_rflags; static int set_rax, get_rax; @@ -538,6 +553,11 @@ enum { SET_CR0, SET_CR3, SET_CR4, + SET_DR0, + SET_DR1, + SET_DR2, + SET_DR3, + SET_DR6, SET_DR7, SET_RSP, SET_RIP, @@ -642,7 +662,8 @@ cpu_vendor_intel(void) static int get_all_registers(struct vmctx *ctx, int vcpu) { - uint64_t cr0, cr3, cr4, dr7, rsp, rip, rflags, efer; + uint64_t cr0, cr3, cr4, dr0, dr1, dr2, dr3, dr6, dr7; + uint64_t rsp, rip, rflags, efer; uint64_t rax, rbx, rcx, rdx, rsi, rdi, rbp; uint64_t r8, r9, r10, r11, r12, r13, r14, r15; int error = 0; @@ -671,6 +692,36 @@ get_all_registers(struct vmctx *ctx, int vcpu) printf("cr4[%d]\t\t0x%016lx\n", vcpu, cr4); } + if (!error && (get_dr0 || get_all)) { + error = vm_get_register(ctx, vcpu, VM_REG_GUEST_DR0, &dr0); + if (error == 0) + printf("dr0[%d]\t\t0x%016lx\n", vcpu, dr0); + } + + if (!error && (get_dr1 || get_all)) { + error = vm_get_register(ctx, vcpu, VM_REG_GUEST_DR1, &dr1); + if (error == 0) + printf("dr1[%d]\t\t0x%016lx\n", vcpu, dr1); + } + + if (!error && (get_dr2 || get_all)) { + error = vm_get_register(ctx, vcpu, VM_REG_GUEST_DR2, &dr2); + if (error == 0) + printf("dr2[%d]\t\t0x%016lx\n", vcpu, dr2); + } + + if (!error && (get_dr3 || get_all)) { + error = vm_get_register(ctx, vcpu, VM_REG_GUEST_DR3, &dr3); + if (error == 0) + printf("dr3[%d]\t\t0x%016lx\n", vcpu, dr3); + } + + if (!error && (get_dr6 || get_all)) { + error = vm_get_register(ctx, vcpu, VM_REG_GUEST_DR6, &dr6); + if (error == 0) + printf("dr6[%d]\t\t0x%016lx\n", vcpu, dr6); + } + if (!error && (get_dr7 || get_all)) { error = vm_get_register(ctx, vcpu, VM_REG_GUEST_DR7, &dr7); if (error == 0) @@ -1273,6 +1324,11 @@ setup_options(bool cpu_intel) { "set-cr0", REQ_ARG, 0, SET_CR0 }, { "set-cr3", REQ_ARG, 0, SET_CR3 }, { "set-cr4", REQ_ARG, 0, SET_CR4 }, + { "set-dr0", REQ_ARG, 0, SET_DR0 }, + { "set-dr1", REQ_ARG, 0, SET_DR1 }, + { "set-dr2", REQ_ARG, 0, SET_DR2 }, + { "set-dr3", REQ_ARG, 0, SET_DR3 }, + { "set-dr6", REQ_ARG, 0, SET_DR6 }, { "set-dr7", REQ_ARG, 0, SET_DR7 }, { "set-rsp", REQ_ARG, 0, SET_RSP }, { "set-rip", REQ_ARG, 0, SET_RIP }, @@ -1330,6 +1386,11 @@ setup_options(bool cpu_intel) { "get-cr0", NO_ARG, &get_cr0, 1 }, { "get-cr3", NO_ARG, &get_cr3, 1 }, { "get-cr4", NO_ARG, &get_cr4, 1 }, + { "get-dr0", NO_ARG, &get_dr0, 1 }, + { "get-dr1", NO_ARG, &get_dr1, 1 }, + { "get-dr2", NO_ARG, &get_dr2, 1 }, + { "get-dr3", NO_ARG, &get_dr3, 1 }, + { "get-dr6", NO_ARG, &get_dr6, 1 }, { "get-dr7", NO_ARG, &get_dr7, 1 }, { "get-rsp", NO_ARG, &get_rsp, 1 }, { "get-rip", NO_ARG, &get_rip, 1 }, @@ -1607,7 +1668,8 @@ main(int argc, char *argv[]) int error, ch, vcpu, ptenum; vm_paddr_t gpa_pmap; struct vm_exit vmexit; - uint64_t rax, cr0, cr3, cr4, dr7, rsp, rip, rflags, efer, pat; + uint64_t rax, cr0, cr3, cr4, dr0, dr1, dr2, dr3, dr6, dr7; + uint64_t rsp, rip, rflags, efer, pat; uint64_t eptp, bm, addr, u64, pteval[4], *pte, info[2]; struct vmctx *ctx; cpuset_t cpus; @@ -1654,6 +1716,26 @@ main(int argc, char *argv[]) cr4 = strtoul(optarg, NULL, 0); set_cr4 = 1; break; + case SET_DR0: + dr0 = strtoul(optarg, NULL, 0); + set_dr0 = 1; + break; + case SET_DR1: + dr1 = strtoul(optarg, NULL, 0); + set_dr1 = 1; + break; + case SET_DR2: + dr2 = strtoul(optarg, NULL, 0); + set_dr2 = 1; + break; + case SET_DR3: + dr3 = strtoul(optarg, NULL, 0); + set_dr3 = 1; + break; + case SET_DR6: + dr6 = strtoul(optarg, NULL, 0); + set_dr6 = 1; + break; case SET_DR7: dr7 = strtoul(optarg, NULL, 0); set_dr7 = 1; @@ -1795,6 +1877,21 @@ main(int argc, char *argv[]) if (!error && set_cr4) error = vm_set_register(ctx, vcpu, VM_REG_GUEST_CR4, cr4); + if (!error && set_dr0) + error = vm_set_register(ctx, vcpu, VM_REG_GUEST_DR0, dr0); + + if (!error && set_dr1) + error = vm_set_register(ctx, vcpu, VM_REG_GUEST_DR1, dr1); + + if (!error && set_dr2) + error = vm_set_register(ctx, vcpu, VM_REG_GUEST_DR2, dr2); + + if (!error && set_dr3) + error = vm_set_register(ctx, vcpu, VM_REG_GUEST_DR3, dr3); + + if (!error && set_dr6) + error = vm_set_register(ctx, vcpu, VM_REG_GUEST_DR6, dr6); + if (!error && set_dr7) error = vm_set_register(ctx, vcpu, VM_REG_GUEST_DR7, dr7);