From a937696c39107baeddb1d74b491792853797e3a6 Mon Sep 17 00:00:00 2001 From: mlarkin Date: Tue, 20 Jun 2017 05:34:41 +0000 Subject: [PATCH] SVM: better cleanbits handling. Fixes an issue on Bulldozer CPUs causing #TF exceptions during guest VM boot ok brynet --- sys/arch/amd64/amd64/identcpu.c | 14 +++-- sys/arch/amd64/amd64/vmm.c | 80 +++++++++++++++++++++++++---- sys/arch/amd64/include/cpu.h | 4 +- sys/arch/amd64/include/specialreg.h | 25 ++++++++- 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 * @@ -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 *); @@ -1976,6 +1978,63 @@ vmx_setmsrbrw(struct vcpu *vcpu, uint32_t msr) vmx_setmsrbw(vcpu, 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 * @@ -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 -- 2.20.1