SVM: better cleanbits handling. Fixes an issue on Bulldozer CPUs causing

#TF exceptions during guest VM boot

ok brynet

4 files changed, 105 insertions, 18 deletions

diff --git a/sys/arch/amd64/amd64/identcpu.c b/sys/arch/amd64/amd64/identcpu.c
index 0ee3e28b241..a448b885ba7 100644
--- a/sys/arch/amd64/amd64/identcpu.c
+++ b/sys/arch/amd64/amd64/identcpu.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: identcpu.c,v 1.86 2017/05/30 15:11:32 deraadt Exp $	*/
+/*	$OpenBSD: identcpu.c,v 1.87 2017/06/20 05:34:41 mlarkin Exp $	*/
 /*	$NetBSD: identcpu.c,v 1.1 2003/04/26 18:39:28 fvdl Exp $	*/
 
 /*
@@ -870,7 +870,7 @@ void
 cpu_check_vmm_cap(struct cpu_info *ci)
 {
 	uint64_t msr;
-	uint32_t cap, dummy;
+	uint32_t cap, dummy, edx;
 
 	/*
 	 * Check for workable VMX
@@ -949,11 +949,17 @@ cpu_check_vmm_cap(struct cpu_info *ci)
 		if (!(msr & AMD_SVMDIS))
 			ci->ci_vmm_flags |= CI_VMM_SVM;
 
-		CPUID(0x8000000A, dummy, ci->ci_vmm_cap.vcc_svm.svm_max_asid,
-		    dummy, dummy);
+		CPUID(CPUID_AMD_SVM_CAP, dummy,
+		    ci->ci_vmm_cap.vcc_svm.svm_max_asid, dummy, edx);
 
 		if (ci->ci_vmm_cap.vcc_svm.svm_max_asid > 0xFFF)
 			ci->ci_vmm_cap.vcc_svm.svm_max_asid = 0xFFF;
+
+		if (edx & AMD_SVM_FLUSH_BY_ASID_CAP)
+			ci->ci_vmm_cap.vcc_svm.svm_flush_by_asid = 1;
+
+		if (edx & AMD_SVM_VMCB_CLEAN_CAP)
+			ci->ci_vmm_cap.vcc_svm.svm_vmcb_clean = 1;
 	}
 
 	/*
diff --git a/sys/arch/amd64/amd64/vmm.c b/sys/arch/amd64/amd64/vmm.c
index 8a7e4e16f91..66e55011aa4 100644
--- a/sys/arch/amd64/amd64/vmm.c
+++ b/sys/arch/amd64/amd64/vmm.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vmm.c,v 1.152 2017/05/30 20:31:24 mlarkin Exp $	*/
+/*	$OpenBSD: vmm.c,v 1.153 2017/06/20 05:34:41 mlarkin Exp $	*/
 /*
  * Copyright (c) 2014 Mike Larkin <mlarkin@openbsd.org>
  *
@@ -190,6 +190,8 @@ void svm_setmsrbrw(struct vcpu *, uint32_t);
 void vmx_setmsrbr(struct vcpu *, uint32_t);
 void vmx_setmsrbw(struct vcpu *, uint32_t);
 void vmx_setmsrbrw(struct vcpu *, uint32_t);
+void svm_set_clean(struct vcpu *, uint32_t);
+void svm_set_dirty(struct vcpu *, uint32_t);
 
 #ifdef VMM_DEBUG
 void dump_vcpu(struct vcpu *);
@@ -1977,6 +1979,63 @@ vmx_setmsrbrw(struct vcpu *vcpu, uint32_t msr)
 }
 
 /*
+ * svm_set_clean
+ *
+ * Sets (mark as unmodified) the VMCB clean bit set in 'value'. 
+ * For example, to set the clean bit for the VMCB intercepts (bit position 0),
+ * the caller provides 'SVM_CLEANBITS_I' (0x1) for the 'value' argument.
+ * Multiple cleanbits can be provided in 'value' at the same time (eg,
+ * "SVM_CLEANBITS_I | SVM_CLEANBITS_TPR").
+ *
+ * Note that this function does not clear any bits; to clear bits in the
+ * vmcb cleanbits bitfield, use 'svm_set_dirty'.
+ *
+ * Paramters:
+ *  vmcs: the VCPU whose VMCB clean value should be set
+ *  value: the value(s) to enable in the cleanbits mask
+ */
+void
+svm_set_clean(struct vcpu *vcpu, uint32_t value)
+{
+	struct vmcb *vmcb;
+
+	/* If no cleanbits support, do nothing */
+	if (!curcpu()->ci_vmm_cap.vcc_svm.svm_vmcb_clean)
+		return;
+
+	vmcb = (struct vmcb *)vcpu->vc_control_va;
+
+	vmcb->v_vmcb_clean_bits |= value;
+}
+
+/*
+ * svm_set_dirty
+ *
+ * Clears (mark as modified) the VMCB clean bit set in 'value'. 
+ * For example, to clear the bit for the VMCB intercepts (bit position 0)
+ * the caller provides 'SVM_CLEANBITS_I' (0x1) for the 'value' argument.
+ * Multiple dirty bits can be provided in 'value' at the same time (eg,
+ * "SVM_CLEANBITS_I | SVM_CLEANBITS_TPR").
+ *
+ * Paramters:
+ *  vmcs: the VCPU whose VMCB dirty value should be set
+ *  value: the value(s) to dirty in the cleanbits mask
+ */
+void
+svm_set_dirty(struct vcpu *vcpu, uint32_t value)
+{
+	struct vmcb *vmcb;
+
+	/* If no cleanbits support, do nothing */
+	if (!curcpu()->ci_vmm_cap.vcc_svm.svm_vmcb_clean)
+		return;
+
+	vmcb = (struct vmcb *)vcpu->vc_control_va;
+
+	vmcb->v_vmcb_clean_bits &= ~value;
+}
+
+/*
  * vcpu_reset_regs_vmx
  *
  * Initializes 'vcpu's registers to supplied state
@@ -4150,6 +4209,7 @@ svm_handle_exit(struct vcpu *vcpu)
 
 	/* Enable SVME in EFER (must always be set) */
 	vmcb->v_efer |= EFER_SVME;
+	svm_set_dirty(vcpu, SVM_CLEANBITS_CR);
 
 	return (ret);
 }
@@ -5480,7 +5540,7 @@ vcpu_run_svm(struct vcpu *vcpu, struct vm_run_params *vrp)
 
 			if (ci != vcpu->vc_last_pcpu) {
 				vmcb->v_tlb_control = 3; /* Flush TLB */
-				vmcb->v_vmcb_clean_bits = 0; /* Flush cleanbits cache */
+				svm_set_dirty(vcpu, SVM_CLEANBITS_ALL);
 			}
 
 			vcpu->vc_last_pcpu = ci;
@@ -5501,18 +5561,17 @@ vcpu_run_svm(struct vcpu *vcpu, struct vm_run_params *vrp)
 				vmcb->v_intr_misc = 0x10; /* XXX #define ign_tpr */
 				vmcb->v_intr_vector = 0;
 				vmcb->v_intercept1 |= SVM_INTERCEPT_VINTR;
-				vmcb->v_vmcb_clean_bits &= ~(1 << 3);
-				vmcb->v_vmcb_clean_bits &= ~(1 << 0);
+				svm_set_dirty(vcpu, SVM_CLEANBITS_TPR |
+				    SVM_CLEANBITS_I);
 			}
 
 			irq = 0xFFFF;
 		} else if (!vcpu->vc_intr) {
 			/* Disable interrupt window */
-			vmcb->v_intercept1 &= ~SVM_INTERCEPT_VINTR;
-			vmcb->v_vmcb_clean_bits &= ~(1 << 0);
-			vmcb->v_vmcb_clean_bits &= ~(1 << 3);
 			vmcb->v_irq = 0;
 			vmcb->v_intr_vector = 0;
+			vmcb->v_intercept1 &= ~SVM_INTERCEPT_VINTR;
+			svm_set_dirty(vcpu, SVM_CLEANBITS_TPR | SVM_CLEANBITS_I);
 		}
 
 		/* Inject event if present */
@@ -5600,10 +5659,9 @@ vcpu_run_svm(struct vcpu *vcpu, struct vm_run_params *vrp)
 
 		enable_intr();
 
-
 		vcpu->vc_gueststate.vg_rip = vmcb->v_rip;
 		vmcb->v_tlb_control = 0;
-		vmcb->v_vmcb_clean_bits = 0xFFFFFFFF;
+		svm_set_clean(vcpu, SVM_CLEANBITS_ALL);
 
 		if (ret || exit_reason != SVM_VMEXIT_INTR) {
 			KERNEL_LOCK();
@@ -5641,8 +5699,8 @@ vcpu_run_svm(struct vcpu *vcpu, struct vm_run_params *vrp)
 			if (vcpu->vc_irqready == 0 && vcpu->vc_intr) {
 				vmcb->v_intercept1 |= SVM_INTERCEPT_VINTR;
 				vmcb->v_irq = 1;
-				vmcb->v_vmcb_clean_bits &= ~(1 << 0);
-				vmcb->v_vmcb_clean_bits &= ~(1 << 3);
+				svm_set_dirty(vcpu, SVM_CLEANBITS_TPR |
+				    SVM_CLEANBITS_I);
 			}
 
 			/*
diff --git a/sys/arch/amd64/include/cpu.h b/sys/arch/amd64/include/cpu.h
index 10e7292d763..77bfe72b034 100644
--- a/sys/arch/amd64/include/cpu.h
+++ b/sys/arch/amd64/include/cpu.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: cpu.h,v 1.111 2017/04/14 01:02:28 mlarkin Exp $	*/
+/*	$OpenBSD: cpu.h,v 1.112 2017/06/20 05:34:41 mlarkin Exp $	*/
 /*	$NetBSD: cpu.h,v 1.1 2003/04/26 18:39:39 fvdl Exp $	*/
 
 /*-
@@ -78,6 +78,8 @@ struct vmx {
  */
 struct svm {
 	uint32_t	svm_max_asid;
+	uint8_t		svm_flush_by_asid;
+	uint8_t		svm_vmcb_clean;
 };
 
 union vmm_cpu_cap {
diff --git a/sys/arch/amd64/include/specialreg.h b/sys/arch/amd64/include/specialreg.h
index 06571df0dda..00a8523e690 100644
--- a/sys/arch/amd64/include/specialreg.h
+++ b/sys/arch/amd64/include/specialreg.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: specialreg.h,v 1.56 2017/05/30 17:49:47 mlarkin Exp $	*/
+/*	$OpenBSD: specialreg.h,v 1.57 2017/06/20 05:34:41 mlarkin Exp $	*/
 /*	$NetBSD: specialreg.h,v 1.1 2003/04/26 18:39:48 fvdl Exp $	*/
 /*	$NetBSD: x86/specialreg.h,v 1.2 2003/04/25 21:54:30 fvdl Exp $	*/
 
@@ -1174,8 +1174,29 @@
 #define MSR_AMD_VM_CR			0xc0010114
 #define MSR_AMD_VM_HSAVE_PA		0xc0010117
 #define CPUID_AMD_SVM_CAP		0x8000000A
-#define AMD_SVMDIS			0x10
 #define AMD_SVM_NESTED_PAGING_CAP	(1 << 0)
+#define AMD_SVM_VMCB_CLEAN_CAP		(1 << 5)
+#define AMD_SVM_FLUSH_BY_ASID_CAP	(1 << 6)
+#define AMD_SVMDIS			0x10
+
+#define SVM_CLEANBITS_I			(1 << 0)
+#define SVM_CLEANBITS_IOPM		(1 << 1)
+#define SVM_CLEANBITS_ASID		(1 << 2)
+#define SVM_CLEANBITS_TPR		(1 << 3)
+#define SVM_CLEANBITS_NP		(1 << 4)
+#define SVM_CLEANBITS_CR		(1 << 5)
+#define SVM_CLEANBITS_DR		(1 << 6)
+#define SVM_CLEANBITS_DT		(1 << 7)
+#define SVM_CLEANBITS_SEG		(1 << 8)
+#define SVM_CLEANBITS_CR2		(1 << 9)
+#define SVM_CLEANBITS_LBR		(1 << 10)
+#define SVM_CLEANBITS_AVIC		(1 << 11)
+
+#define SVM_CLEANBITS_ALL \
+	(SVM_CLEANBITS_I | SVM_CLEANBITS_IOPM | SVM_CLEANBITS_ASID | \
+	 SVM_CLEANBITS_TPR | SVM_CLEANBITS_NP | SVM_CLEANBITS_CR | \
+	 SVM_CLEANBITS_DR | SVM_CLEANBITS_DT | SVM_CLEANBITS_SEG | \
+	 SVM_CLEANBITS_CR2 | SVM_CLEANBITS_LBR | SVM_CLEANBITS_AVIC )
 
 /*
  * SVM : VMCB intercepts