Date: Wed, 7 Mar 2018 20:50:29 +0000 (UTC) From: John Baldwin <jhb@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org Subject: svn commit: r330623 - in stable/11: sys/amd64/include sys/amd64/vmm/amd sys/amd64/vmm/intel usr.sbin/bhyvectl Message-ID: <201803072050.w27KoT7j036256@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: jhb Date: Wed Mar 7 20:50:28 2018 New Revision: 330623 URL: https://svnweb.freebsd.org/changeset/base/330623 Log: MFC 328102: 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. Modified: stable/11/sys/amd64/include/vmm.h stable/11/sys/amd64/vmm/amd/svm.c stable/11/sys/amd64/vmm/amd/svm.h stable/11/sys/amd64/vmm/amd/vmcb.c stable/11/sys/amd64/vmm/intel/vmx.c stable/11/sys/amd64/vmm/intel/vmx.h stable/11/usr.sbin/bhyvectl/bhyvectl.c Directory Properties: stable/11/ (props changed) Modified: stable/11/sys/amd64/include/vmm.h ============================================================================== --- stable/11/sys/amd64/include/vmm.h Wed Mar 7 18:45:24 2018 (r330622) +++ stable/11/sys/amd64/include/vmm.h Wed Mar 7 20:50:28 2018 (r330623) @@ -83,6 +83,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 }; Modified: stable/11/sys/amd64/vmm/amd/svm.c ============================================================================== --- stable/11/sys/amd64/vmm/amd/svm.c Wed Mar 7 18:45:24 2018 (r330622) +++ stable/11/sys/amd64/vmm/amd/svm.c Wed Mar 7 20:50:28 2018 (r330623) @@ -87,6 +87,7 @@ SYSCTL_NODE(_hw_vmm, OID_AUTO, svm, CTLFLAG_RW, 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 iop 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; } /* @@ -1861,6 +1866,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. */ @@ -1973,7 +2032,9 @@ svm_vmrun(void *arg, int vcpu, register_t rip, pmap_t /* 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); @@ -2042,6 +2103,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); } Modified: stable/11/sys/amd64/vmm/amd/svm.h ============================================================================== --- stable/11/sys/amd64/vmm/amd/svm.h Wed Mar 7 18:45:24 2018 (r330622) +++ stable/11/sys/amd64/vmm/amd/svm.h Wed Mar 7 20:50:28 2018 (r330623) @@ -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); Modified: stable/11/sys/amd64/vmm/amd/vmcb.c ============================================================================== --- stable/11/sys/amd64/vmm/amd/vmcb.c Wed Mar 7 18:45:24 2018 (r330622) +++ stable/11/sys/amd64/vmm/amd/vmcb.c Wed Mar 7 20:50:28 2018 (r330623) @@ -187,6 +187,10 @@ vmcb_read(struct svm_softc *sc, int vcpu, int ident, u *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, 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: Modified: stable/11/sys/amd64/vmm/intel/vmx.c ============================================================================== --- stable/11/sys/amd64/vmm/intel/vmx.c Wed Mar 7 18:45:24 2018 (r330622) +++ stable/11/sys/amd64/vmm/intel/vmx.c Wed Mar 7 20:50:28 2018 (r330623) @@ -96,18 +96,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 @@ -914,6 +916,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, @@ -2570,6 +2575,73 @@ vmx_exit_handle_nmi(struct vmx *vmx, int vcpuid, struc } } +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) @@ -2668,7 +2740,9 @@ vmx_run(void *arg, int vcpu, register_t rip, pmap_t pm } 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(); @@ -2768,6 +2842,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; } Modified: stable/11/sys/amd64/vmm/intel/vmx.h ============================================================================== --- stable/11/sys/amd64/vmm/intel/vmx.h Wed Mar 7 18:45:24 2018 (r330622) +++ stable/11/sys/amd64/vmm/intel/vmx.h Wed Mar 7 20:50:28 2018 (r330623) @@ -50,6 +50,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; @@ -58,9 +63,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; Modified: stable/11/usr.sbin/bhyvectl/bhyvectl.c ============================================================================== --- stable/11/usr.sbin/bhyvectl/bhyvectl.c Wed Mar 7 18:45:24 2018 (r330622) +++ stable/11/usr.sbin/bhyvectl/bhyvectl.c Wed Mar 7 20:50:28 2018 (r330623) @@ -113,6 +113,16 @@ usage(bool cpu_intel) " [--get-cr3]\n" " [--set-cr4=<CR4>]\n" " [--get-cr4]\n" + " [--set-dr0=<DR0>]\n" + " [--get-dr0]\n" + " [--set-dr1=<DR1>]\n" + " [--get-dr1]\n" + " [--set-dr2=<DR2>]\n" + " [--get-dr2]\n" + " [--set-dr3=<DR3>]\n" + " [--get-dr3]\n" + " [--set-dr6=<DR6>]\n" + " [--get-dr6]\n" " [--set-dr7=<DR7>]\n" " [--get-dr7]\n" " [--set-rsp=<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; @@ -1794,6 +1876,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);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201803072050.w27KoT7j036256>