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

@@ -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
 };
 

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 (&regctx->sctx_r14);
 	case VM_REG_GUEST_R15:
 		return (&regctx->sctx_r15);
+	case VM_REG_GUEST_DR0:
+		return (&regctx->sctx_dr0);
+	case VM_REG_GUEST_DR1:
+		return (&regctx->sctx_dr1);
+	case VM_REG_GUEST_DR2:
+		return (&regctx->sctx_dr2);
+	case VM_REG_GUEST_DR3:
+		return (&regctx->sctx_dr3);
 	default:
 		return (NULL);
 	}

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

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:

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

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;
 

usr.sbin/bhyvectl/bhyvectl.c

@@ -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;
@@ -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);