Module Name: src
Committed By: maxv
Date: Thu Feb 21 12:17:52 UTC 2019
Modified Files:
src/sys/arch/x86/x86: x86_tlb.c
src/sys/dev/nvmm/x86: nvmm_x86_svm.c nvmm_x86_vmx.c
Log Message:
Another locking issue in NVMM: the {svm,vmx}_tlb_flush functions take VCPU
mutexes which can sleep, but their context does not allow it.
Rewrite the TLB handling code to fix that. It becomes a bit complex. In
short, we use a per-VM generation number, which we increase on each TLB
flush, before sending a broadcast IPI to everybody. The IPIs cause a
#VMEXIT of each VCPU, and each VCPU Loop will synchronize the per-VM gen
with a per-VCPU copy, and apply the flushes as neededi lazily.
The behavior differs between AMD and Intel; in short, on Intel we don't
flush the hTLB (EPT cache) if a context switch of a VCPU occurs, so now,
we need to maintain a kcpuset to know which VCPU's hTLBs are active on
which hCPU. This creates some redundancy on Intel, ie there are cases
where we flush the hTLB several times unnecessarily; but hTLB flushes are
very rare, so there is no real performance regression.
The thing is lock-less and non-blocking, so it solves our problem.
To generate a diff of this commit:
cvs rdiff -u -r1.5 -r1.6 src/sys/arch/x86/x86/x86_tlb.c
cvs rdiff -u -r1.28 -r1.29 src/sys/dev/nvmm/x86/nvmm_x86_svm.c
cvs rdiff -u -r1.8 -r1.9 src/sys/dev/nvmm/x86/nvmm_x86_vmx.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/arch/x86/x86/x86_tlb.c
diff -u src/sys/arch/x86/x86/x86_tlb.c:1.5 src/sys/arch/x86/x86/x86_tlb.c:1.6
--- src/sys/arch/x86/x86/x86_tlb.c:1.5 Mon Feb 11 14:59:33 2019
+++ src/sys/arch/x86/x86/x86_tlb.c Thu Feb 21 12:17:52 2019
@@ -1,4 +1,4 @@
-/* $NetBSD: x86_tlb.c,v 1.5 2019/02/11 14:59:33 cherry Exp $ */
+/* $NetBSD: x86_tlb.c,v 1.6 2019/02/21 12:17:52 maxv Exp $ */
/*-
* Copyright (c) 2008-2012 The NetBSD Foundation, Inc.
@@ -40,7 +40,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: x86_tlb.c,v 1.5 2019/02/11 14:59:33 cherry Exp $");
+__KERNEL_RCSID(0, "$NetBSD: x86_tlb.c,v 1.6 2019/02/21 12:17:52 maxv Exp $");
#include <sys/param.h>
#include <sys/kernel.h>
@@ -229,6 +229,7 @@ pmap_tlb_shootdown(struct pmap *pm, vadd
if (__predict_false(pm->pm_tlb_flush != NULL)) {
(*pm->pm_tlb_flush)(pm);
+ return;
}
/*
Index: src/sys/dev/nvmm/x86/nvmm_x86_svm.c
diff -u src/sys/dev/nvmm/x86/nvmm_x86_svm.c:1.28 src/sys/dev/nvmm/x86/nvmm_x86_svm.c:1.29
--- src/sys/dev/nvmm/x86/nvmm_x86_svm.c:1.28 Thu Feb 21 11:58:04 2019
+++ src/sys/dev/nvmm/x86/nvmm_x86_svm.c Thu Feb 21 12:17:52 2019
@@ -1,4 +1,4 @@
-/* $NetBSD: nvmm_x86_svm.c,v 1.28 2019/02/21 11:58:04 maxv Exp $ */
+/* $NetBSD: nvmm_x86_svm.c,v 1.29 2019/02/21 12:17:52 maxv Exp $ */
/*
* Copyright (c) 2018 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: nvmm_x86_svm.c,v 1.28 2019/02/21 11:58:04 maxv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: nvmm_x86_svm.c,v 1.29 2019/02/21 12:17:52 maxv Exp $");
#include <sys/param.h>
#include <sys/systm.h>
@@ -493,6 +493,7 @@ static uint64_t svm_xcr0_mask __read_mos
struct svm_machdata {
bool cpuidpresent[SVM_NCPUIDS];
struct nvmm_x86_conf_cpuid cpuid[SVM_NCPUIDS];
+ volatile uint64_t mach_htlb_gen;
};
static const size_t svm_conf_sizes[NVMM_X86_NCONF] = {
@@ -503,6 +504,7 @@ struct svm_cpudata {
/* General */
bool shared_asid;
bool gtlb_want_flush;
+ uint64_t vcpu_htlb_gen;
/* VMCB */
struct vmcb *vmcb;
@@ -1101,6 +1103,8 @@ error:
svm_inject_gp(mach, vcpu);
}
+/* -------------------------------------------------------------------------- */
+
static void
svm_vcpu_guest_fpu_enter(struct nvmm_cpu *vcpu)
{
@@ -1197,18 +1201,57 @@ svm_gtlb_catchup(struct nvmm_cpu *vcpu,
}
}
+static inline void
+svm_htlb_catchup(struct nvmm_cpu *vcpu, int hcpu)
+{
+ /*
+ * Nothing to do. If an hTLB flush was needed, either the VCPU was
+ * executing on this hCPU and the hTLB already got flushed, or it
+ * was executing on another hCPU in which case the catchup is done
+ * in svm_gtlb_catchup().
+ */
+}
+
+static inline uint64_t
+svm_htlb_flush(struct svm_machdata *machdata, struct svm_cpudata *cpudata)
+{
+ struct vmcb *vmcb = cpudata->vmcb;
+ uint64_t machgen;
+
+ machgen = machdata->mach_htlb_gen;
+ if (__predict_true(machgen == cpudata->vcpu_htlb_gen)) {
+ return machgen;
+ }
+
+ vmcb->ctrl.tlb_ctrl = svm_ctrl_tlb_flush;
+ return machgen;
+}
+
+static inline void
+svm_htlb_flush_ack(struct svm_cpudata *cpudata, uint64_t machgen)
+{
+ struct vmcb *vmcb = cpudata->vmcb;
+
+ if (__predict_true(vmcb->ctrl.exitcode != VMCB_EXITCODE_INVALID)) {
+ cpudata->vcpu_htlb_gen = machgen;
+ }
+}
+
static int
svm_vcpu_run(struct nvmm_machine *mach, struct nvmm_cpu *vcpu,
struct nvmm_exit *exit)
{
+ struct svm_machdata *machdata = mach->machdata;
struct svm_cpudata *cpudata = vcpu->cpudata;
struct vmcb *vmcb = cpudata->vmcb;
+ uint64_t machgen;
int hcpu, s;
kpreempt_disable();
hcpu = cpu_number();
svm_gtlb_catchup(vcpu, hcpu);
+ svm_htlb_catchup(vcpu, hcpu);
if (vcpu->hcpu_last != hcpu) {
vmcb->ctrl.tsc_offset = cpudata->tsc_offset +
@@ -1227,9 +1270,11 @@ svm_vcpu_run(struct nvmm_machine *mach,
}
s = splhigh();
+ machgen = svm_htlb_flush(machdata, cpudata);
svm_vcpu_guest_fpu_enter(vcpu);
svm_vmrun(cpudata->vmcb_pa, cpudata->gprs);
svm_vcpu_guest_fpu_leave(vcpu);
+ svm_htlb_flush_ack(cpudata, machgen);
splx(s);
svm_vmcb_cache_default(vmcb);
@@ -1982,30 +2027,28 @@ static void
svm_tlb_flush(struct pmap *pm)
{
struct nvmm_machine *mach = pm->pm_data;
- struct svm_cpudata *cpudata;
- struct nvmm_cpu *vcpu;
- int error;
- size_t i;
+ struct svm_machdata *machdata = mach->machdata;
- /* Request TLB flushes. */
- for (i = 0; i < NVMM_MAX_VCPUS; i++) {
- error = nvmm_vcpu_get(mach, i, &vcpu);
- if (error)
- continue;
- cpudata = vcpu->cpudata;
- cpudata->gtlb_want_flush = true;
- nvmm_vcpu_put(vcpu);
- }
+ atomic_inc_64(&machdata->mach_htlb_gen);
+
+ /* Generates IPIs, which cause #VMEXITs. */
+ pmap_tlb_shootdown(pmap_kernel(), -1, PG_G, TLBSHOOT_UPDATE);
}
static void
svm_machine_create(struct nvmm_machine *mach)
{
+ struct svm_machdata *machdata;
+
/* Fill in pmap info. */
mach->vm->vm_map.pmap->pm_data = (void *)mach;
mach->vm->vm_map.pmap->pm_tlb_flush = svm_tlb_flush;
- mach->machdata = kmem_zalloc(sizeof(struct svm_machdata), KM_SLEEP);
+ machdata = kmem_zalloc(sizeof(struct svm_machdata), KM_SLEEP);
+ mach->machdata = machdata;
+
+ /* Start with an hTLB flush everywhere. */
+ machdata->mach_htlb_gen = 1;
}
static void
Index: src/sys/dev/nvmm/x86/nvmm_x86_vmx.c
diff -u src/sys/dev/nvmm/x86/nvmm_x86_vmx.c:1.8 src/sys/dev/nvmm/x86/nvmm_x86_vmx.c:1.9
--- src/sys/dev/nvmm/x86/nvmm_x86_vmx.c:1.8 Thu Feb 21 11:58:04 2019
+++ src/sys/dev/nvmm/x86/nvmm_x86_vmx.c Thu Feb 21 12:17:52 2019
@@ -1,4 +1,4 @@
-/* $NetBSD: nvmm_x86_vmx.c,v 1.8 2019/02/21 11:58:04 maxv Exp $ */
+/* $NetBSD: nvmm_x86_vmx.c,v 1.9 2019/02/21 12:17:52 maxv Exp $ */
/*
* Copyright (c) 2018 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: nvmm_x86_vmx.c,v 1.8 2019/02/21 11:58:04 maxv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: nvmm_x86_vmx.c,v 1.9 2019/02/21 12:17:52 maxv Exp $");
#include <sys/param.h>
#include <sys/systm.h>
@@ -627,7 +627,7 @@ static uint64_t vmx_xcr0_mask __read_mos
struct vmx_machdata {
bool cpuidpresent[VMX_NCPUIDS];
struct nvmm_x86_conf_cpuid cpuid[VMX_NCPUIDS];
- kcpuset_t *ept_want_flush;
+ volatile uint64_t mach_htlb_gen;
};
static const size_t vmx_conf_sizes[NVMM_X86_NCONF] = {
@@ -638,6 +638,8 @@ struct vmx_cpudata {
/* General */
uint64_t asid;
bool gtlb_want_flush;
+ uint64_t vcpu_htlb_gen;
+ kcpuset_t *htlb_want_flush;
/* VMCS */
struct vmcs *vmcs;
@@ -1510,6 +1512,8 @@ vmx_exit_epf(struct nvmm_machine *mach,
exit->u.mem.inst_len = 0;
}
+/* -------------------------------------------------------------------------- */
+
static void
vmx_vcpu_guest_fpu_enter(struct nvmm_cpu *vcpu)
{
@@ -1601,7 +1605,7 @@ vmx_vcpu_guest_misc_leave(struct nvmm_cp
wrmsr(MSR_KERNELGSBASE, cpudata->kernelgsbase);
}
-/* --------------------------------------------------------------------- */
+/* -------------------------------------------------------------------------- */
#define VMX_INVVPID_ADDRESS 0
#define VMX_INVVPID_CONTEXT 1
@@ -1621,6 +1625,49 @@ vmx_gtlb_catchup(struct nvmm_cpu *vcpu,
}
}
+static inline void
+vmx_htlb_catchup(struct nvmm_cpu *vcpu, int hcpu)
+{
+ struct vmx_cpudata *cpudata = vcpu->cpudata;
+ struct ept_desc ept_desc;
+
+ if (__predict_true(!kcpuset_isset(cpudata->htlb_want_flush, hcpu))) {
+ return;
+ }
+
+ vmx_vmread(VMCS_EPTP, &ept_desc.eptp);
+ ept_desc.mbz = 0;
+ vmx_invept(vmx_ept_flush_op, &ept_desc);
+ kcpuset_clear(cpudata->htlb_want_flush, hcpu);
+}
+
+static inline uint64_t
+vmx_htlb_flush(struct vmx_machdata *machdata, struct vmx_cpudata *cpudata)
+{
+ struct ept_desc ept_desc;
+ uint64_t machgen;
+
+ machgen = machdata->mach_htlb_gen;
+ if (__predict_true(machgen == cpudata->vcpu_htlb_gen)) {
+ return machgen;
+ }
+
+ kcpuset_copy(cpudata->htlb_want_flush, kcpuset_running);
+
+ vmx_vmread(VMCS_EPTP, &ept_desc.eptp);
+ ept_desc.mbz = 0;
+ vmx_invept(vmx_ept_flush_op, &ept_desc);
+
+ return machgen;
+}
+
+static inline void
+vmx_htlb_flush_ack(struct vmx_cpudata *cpudata, uint64_t machgen)
+{
+ cpudata->vcpu_htlb_gen = machgen;
+ kcpuset_clear(cpudata->htlb_want_flush, cpu_number());
+}
+
static int
vmx_vcpu_run(struct nvmm_machine *mach, struct nvmm_cpu *vcpu,
struct nvmm_exit *exit)
@@ -1628,10 +1675,10 @@ vmx_vcpu_run(struct nvmm_machine *mach,
struct vmx_machdata *machdata = mach->machdata;
struct vmx_cpudata *cpudata = vcpu->cpudata;
struct vpid_desc vpid_desc;
- struct ept_desc ept_desc;
struct cpu_info *ci;
uint64_t exitcode;
uint64_t intstate;
+ uint64_t machgen;
int hcpu, s, ret;
bool launched = false;
@@ -1640,13 +1687,7 @@ vmx_vcpu_run(struct nvmm_machine *mach,
hcpu = cpu_number();
vmx_gtlb_catchup(vcpu, hcpu);
-
- if (__predict_false(kcpuset_isset(machdata->ept_want_flush, hcpu))) {
- vmx_vmread(VMCS_EPTP, &ept_desc.eptp);
- ept_desc.mbz = 0;
- vmx_invept(vmx_ept_flush_op, &ept_desc);
- kcpuset_clear(machdata->ept_want_flush, hcpu);
- }
+ vmx_htlb_catchup(vcpu, hcpu);
if (vcpu->hcpu_last != hcpu) {
vmx_vmwrite(VMCS_HOST_TR_SELECTOR, ci->ci_tss_sel);
@@ -1670,6 +1711,7 @@ vmx_vcpu_run(struct nvmm_machine *mach,
}
s = splhigh();
+ machgen = vmx_htlb_flush(machdata, cpudata);
vmx_vcpu_guest_fpu_enter(vcpu);
lcr2(cpudata->gcr2);
if (launched) {
@@ -1679,6 +1721,7 @@ vmx_vcpu_run(struct nvmm_machine *mach,
}
cpudata->gcr2 = rcr2();
vmx_vcpu_guest_fpu_leave(vcpu);
+ vmx_htlb_flush_ack(cpudata, machgen);
splx(s);
if (__predict_false(ret != 0)) {
@@ -2089,6 +2132,8 @@ vmx_vcpu_create(struct nvmm_machine *mac
if (error)
goto error;
+ kcpuset_create(&cpudata->htlb_want_flush, true);
+
/* Init the VCPU info. */
vmx_vcpu_init(mach, vcpu);
@@ -2120,6 +2165,8 @@ vmx_vcpu_destroy(struct nvmm_machine *ma
vmx_asid_free(vcpu);
vmx_vmcs_leave(vcpu);
+ kcpuset_destroy(cpudata->htlb_want_flush);
+
vmx_memfree(cpudata->vmcs_pa, (vaddr_t)cpudata->vmcs, VMCS_NPAGES);
vmx_memfree(cpudata->msrbm_pa, (vaddr_t)cpudata->msrbm, MSRBM_NPAGES);
vmx_memfree(cpudata->gmsr_pa, (vaddr_t)cpudata->gmsr, 1);
@@ -2450,23 +2497,11 @@ vmx_tlb_flush(struct pmap *pm)
{
struct nvmm_machine *mach = pm->pm_data;
struct vmx_machdata *machdata = mach->machdata;
- struct nvmm_cpu *vcpu;
- int error;
- size_t i;
- kcpuset_atomicly_merge(machdata->ept_want_flush, kcpuset_running);
+ atomic_inc_64(&machdata->mach_htlb_gen);
- /*
- * Not as dumb as it seems. We want to make sure that when we leave
- * this function, each VCPU got halted at some point, and possibly
- * resumed with the updated kcpuset.
- */
- for (i = 0; i < NVMM_MAX_VCPUS; i++) {
- error = nvmm_vcpu_get(mach, i, &vcpu);
- if (error)
- continue;
- nvmm_vcpu_put(vcpu);
- }
+ /* Generates IPIs, which cause #VMEXITs. */
+ pmap_tlb_shootdown(pmap_kernel(), -1, PG_G, TLBSHOOT_UPDATE);
}
static void
@@ -2483,11 +2518,10 @@ vmx_machine_create(struct nvmm_machine *
pmap->pm_tlb_flush = vmx_tlb_flush;
machdata = kmem_zalloc(sizeof(struct vmx_machdata), KM_SLEEP);
- kcpuset_create(&machdata->ept_want_flush, true);
mach->machdata = machdata;
- /* Start with an EPT flush everywhere. */
- kcpuset_copy(machdata->ept_want_flush, kcpuset_running);
+ /* Start with an hTLB flush everywhere. */
+ machdata->mach_htlb_gen = 1;
}
static void
@@ -2495,7 +2529,6 @@ vmx_machine_destroy(struct nvmm_machine
{
struct vmx_machdata *machdata = mach->machdata;
- kcpuset_destroy(machdata->ept_want_flush);
kmem_free(machdata, sizeof(struct vmx_machdata));
}
