VMM: Lazily unload the VMCS
authorBarret Rhoden <brho@cs.berkeley.edu>
Fri, 2 Dec 2016 19:22:22 +0000 (14:22 -0500)
committerBarret Rhoden <brho@cs.berkeley.edu>
Wed, 14 Dec 2016 00:43:24 +0000 (19:43 -0500)
The idea is that when we unload a GPC, we keep the VMCS, and maybe a few
other things, loaded.  Then later when we need the VMCS loaded, if we're
on the same core, we're fine.  If we're not, then we tell the other core
to flush it and then we'll load the VMCS.

Note that we clear/unload the VMCS when the process leaves the core.
This is necessary for three reasons:
- Need to not have GPC references once the proc is freed (note the
  decref in __abandon_core()).
- We don't want to send unexpected interrupts to cores belonging to
  other processes.
- We need the EPT TLB entries to be unloaded, just like the KPT.  In
  previous commits/comments, this was referred to as aggressively
leaving the process's context.  Since the EPT == KPT, we need to treat
them similarly.

This commit is a little nasty (deadlock avoidance), but at least it's
contained in x86/VMX.

The change shaves off about 260ns from the loading/unloading overhead of a
guest pcore on my machine (from 1140ns to 880ns).

Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
kern/arch/x86/process64.c
kern/arch/x86/ros/trapframe64.h
kern/arch/x86/vmm/intel/vmx.c
kern/arch/x86/vmm/intel/vmx.h
kern/arch/x86/vmm/vmm.c
kern/arch/x86/vmm/vmm.h

index 32b62c9..810d453 100644 (file)
@@ -104,18 +104,24 @@ static void __attribute__((noreturn)) proc_pop_vmtf(struct vm_trapframe *tf)
        struct per_cpu_info *pcpui = &per_cpu_info[core_id()];
        struct proc *p = pcpui->cur_proc;
        struct guest_pcore *gpc;
+       bool should_vmresume;
 
        if (x86_vmtf_is_partial(tf)) {
                gpc = lookup_guest_pcore(p, tf->tf_guest_pcoreid);
                assert(gpc);
                assert(pcpui->guest_pcoreid == tf->tf_guest_pcoreid);
+               should_vmresume = TRUE;
        } else {
-               gpc = load_guest_pcore(p, tf->tf_guest_pcoreid);
+               gpc = load_guest_pcore(p, tf->tf_guest_pcoreid, &should_vmresume);
                if (!gpc) {
                        tf->tf_exit_reason = EXIT_REASON_GUEST_IN_USE;
                        handle_bad_vm_tf(tf);
                }
        }
+       if (should_vmresume)
+               tf->tf_flags |= VMCTX_FL_VMRESUME;
+       else
+               tf->tf_flags &= ~VMCTX_FL_VMRESUME;
        vmcs_write(GUEST_RSP, tf->tf_rsp);
        vmcs_write(GUEST_CR3, tf->tf_cr3);
        vmcs_write(GUEST_RIP, tf->tf_rip);
@@ -141,7 +147,7 @@ static void __attribute__((noreturn)) proc_pop_vmtf(struct vm_trapframe *tf)
         * Partial contexts have already been launched, so we resume them. */
        asm volatile (".globl __asm_pop_vmtf_start;"
                      "__asm_pop_vmtf_start:     "
-                     "testl $"STRINGIFY(VMCTX_FL_PARTIAL)", %c[flags](%0);"
+                     "testl $"STRINGIFY(VMCTX_FL_VMRESUME)", %c[flags](%0);"
                      "pushq %%rbp;              "      /* save in case we fail */
                      "movq %c[rbx](%0), %%rbx;  "
                      "movq %c[rcx](%0), %%rcx;  "
@@ -158,10 +164,10 @@ static void __attribute__((noreturn)) proc_pop_vmtf(struct vm_trapframe *tf)
                      "movq %c[r14](%0), %%r14;  "
                      "movq %c[r15](%0), %%r15;  "
                      "movq %c[rax](%0), %%rax;  "      /* clobber our *tf last */
-                     "jnz 1f;                   "      /* jump if partial */
-                     ASM_VMX_VMLAUNCH";         "      /* non-partial gets launched */
+                     "jnz 1f;                   "      /* jump if resume */
+                     ASM_VMX_VMLAUNCH";         "      /* non-resume gets launched */
                      "jmp 2f;                   "
-                     "1: "ASM_VMX_VMRESUME";    "      /* partials get resumed */
+                     "1: "ASM_VMX_VMRESUME";    "
                      "2: popq %%rbp;            "      /* vmlaunch failed */
                      ".globl __asm_pop_vmtf_end;"
                      "__asm_pop_vmtf_end:       "
@@ -333,7 +339,9 @@ void proc_secure_ctx(struct user_context *ctx)
 void __abandon_core(void)
 {
        struct per_cpu_info *pcpui = &per_cpu_info[core_id()];
+
        lcr3(boot_cr3);
+       vmx_clear_vmcs();
        proc_decref(pcpui->cur_proc);
        pcpui->cur_proc = 0;
 }
index fcbc97b..f7ca1bd 100644 (file)
@@ -67,6 +67,7 @@ struct sw_trapframe {
 
 #define VMCTX_FL_PARTIAL               (1 << 0)
 #define VMCTX_FL_HAS_FAULT             (1 << 1)
+#define VMCTX_FL_VMRESUME              (1 << 2)
 
 struct vm_trapframe {
        /* Actual processor state */
index 90e9c1a..2ff99b0 100644 (file)
 #include <arch/types.h>
 #include <syscall.h>
 #include <arch/io.h>
+#include <percpu.h>
 
 #include <ros/vmm.h>
 #include "vmx.h"
@@ -1066,6 +1067,8 @@ struct guest_pcore *create_guest_pcore(struct proc *p,
                                        struct vmm_gpcore_init *gpci)
 {
        ERRSTACK(2);
+       int8_t state = 0;
+       bool ignore;
        struct guest_pcore *gpc = kmalloc(sizeof(struct guest_pcore), MEM_WAIT);
 
        if (!gpc)
@@ -1087,11 +1090,15 @@ struct guest_pcore *create_guest_pcore(struct proc *p,
        }
        printd("%d: gpc->vmcs is %p\n", core_id(), gpc->vmcs);
        gpc->cpu = -1;
+       gpc->vmcs_core_id = -1;
 
-       vmx_load_guest_pcore(gpc);
+       disable_irqsave(&state);
+       vmx_load_guest_pcore(gpc, &ignore);
        vmx_setup_vmcs(gpc);
        vmx_setup_initial_guest_state(p, gpci);
        vmx_unload_guest_pcore(gpc);
+       enable_irqsave(&state);
+
        gpc->xcr0 = __proc_global_info.x86_default_xcr0;
 
        gpc->posted_irq_desc = gpci->posted_irq_desc;
@@ -1342,13 +1349,73 @@ void vapic_status_dump_kernel(void *vapic)
        printk("-- END KERNEL APIC STATUS DUMP --\n");
 }
 
-void vmx_load_guest_pcore(struct guest_pcore *gpc)
+static DEFINE_PERCPU(struct guest_pcore *, gpc_to_clear_to);
+
+/* Note this is set up to allow spurious pokes.  Someone could arbitrarily send
+ * us this KMSG at any time.  We only actually clear when we've previously
+ * unloaded the GPC.  gpc_to_clear_to is only set once we're just 'caching' it.
+ * */
+void vmx_clear_vmcs(void)
+{
+       struct guest_pcore *gpc = PERCPU_VAR(gpc_to_clear_to);
+
+       if (gpc) {
+               vmcs_clear(gpc->vmcs);
+               wmb(); /* write -1 after clearing */
+               gpc->vmcs_core_id = -1;
+               PERCPU_VAR(gpc_to_clear_to) = NULL;
+       }
+}
+
+static void __clear_vmcs(uint32_t srcid, long a0, long a1, long a2)
 {
+       vmx_clear_vmcs();
+}
+
+/* We are safe from races on GPC, other than vmcs and vmcs_core_id.  For
+ * instance, only one core can be loading or unloading a particular GPC at a
+ * time.  Other cores write to our GPC's vmcs_core_id and vmcs (doing a
+ * vmcs_clear).  Once they write vmcs_core_id != -1, it's ours. */
+void vmx_load_guest_pcore(struct guest_pcore *gpc, bool *should_vmresume)
+{
+       int remote_core;
+
+       assert(!irq_is_enabled());
+       if (gpc->vmcs_core_id == core_id()) {
+               PERCPU_VAR(gpc_to_clear_to) = NULL;
+               *should_vmresume = TRUE;
+               return;
+       }
+       /* Clear ours *before* waiting on someone else; avoids deadlock (circular
+        * wait). */
+       __clear_vmcs(0, 0, 0, 0);
+       remote_core = ACCESS_ONCE(gpc->vmcs_core_id);
+       if (remote_core != -1) {
+               /* This is a bit nasty.  It requires the remote core to receive
+                * interrupts, which means we're now waiting indefinitely for them to
+                * enable IRQs.  They can wait on another core, and so on.  We cleared
+                * our vmcs first, so that we won't deadlock on *this*.
+                *
+                * However, this means we can't wait on another core with IRQs disabled
+                * for any *other* reason.  For instance, if some other subsystem
+                * decides to have one core wait with IRQs disabled on another, the core
+                * that has our VMCS could be waiting on us to do something that we'll
+                * never do. */
+               send_kernel_message(remote_core, __clear_vmcs, 0, 0, 0, KMSG_IMMEDIATE);
+               while (gpc->vmcs_core_id != -1)
+                       cpu_relax();
+       }
        vmcs_load(gpc->vmcs);
        __vmx_setup_pcpu(gpc);
+       gpc->vmcs_core_id = core_id();
+       *should_vmresume = FALSE;
 }
 
 void vmx_unload_guest_pcore(struct guest_pcore *gpc)
 {
-       vmcs_clear(gpc->vmcs);
+       /* We don't have to worry about races yet.  No one will try to load gpc
+        * until we've returned and unlocked, and no one will clear an old VMCS to
+        * this GPC, since it was cleared before we finished loading (above). */
+       gpc->vmcs_core_id = core_id();
+       PERCPU_VAR(gpc_to_clear_to) = gpc;
 }
index 39371f8..631fc5a 100644 (file)
@@ -314,5 +314,6 @@ struct vmxec {
        uint32_t try_set_0;
 };
 
-void vmx_load_guest_pcore(struct guest_pcore *gpc);
+void vmx_load_guest_pcore(struct guest_pcore *gpc, bool *should_vmresume);
 void vmx_unload_guest_pcore(struct guest_pcore *gpc);
+void vmx_clear_vmcs(void);
index 392a3aa..a500606 100644 (file)
@@ -160,7 +160,8 @@ struct guest_pcore *lookup_guest_pcore(struct proc *p, int guest_pcoreid)
        return p->vmm.guest_pcores[guest_pcoreid];
 }
 
-struct guest_pcore *load_guest_pcore(struct proc *p, int guest_pcoreid)
+struct guest_pcore *load_guest_pcore(struct proc *p, int guest_pcoreid,
+                                     bool *should_vmresume)
 {
        struct guest_pcore *gpc;
        struct per_cpu_info *pcpui = &per_cpu_info[core_id()];
@@ -179,7 +180,7 @@ struct guest_pcore *load_guest_pcore(struct proc *p, int guest_pcoreid)
        /* We've got dibs on the gpc; we don't need to hold the lock any longer. */
        pcpui->guest_pcoreid = guest_pcoreid;
        ept_sync_context(gpc_get_eptp(gpc));
-       vmx_load_guest_pcore(gpc);
+       vmx_load_guest_pcore(gpc, should_vmresume);
        /* Load guest's xcr0 */
        lxcr0(gpc->xcr0);
 
index 9ef530d..ab30b7e 100644 (file)
@@ -21,6 +21,7 @@ struct guest_pcore {
        struct proc *proc;
        unsigned long *posted_irq_desc;
        struct vmcs *vmcs;
+       int vmcs_core_id;
        uint64_t xcr0;
        uint64_t msr_kern_gs_base;
        uint64_t msr_star;
@@ -74,7 +75,8 @@ uint64_t construct_eptp(physaddr_t root_hpa);
 void ept_flush(uint64_t eptp);
 
 struct guest_pcore *lookup_guest_pcore(struct proc *p, int guest_pcoreid);
-struct guest_pcore *load_guest_pcore(struct proc *p, int guest_pcoreid);
+struct guest_pcore *load_guest_pcore(struct proc *p, int guest_pcoreid,
+                                     bool *should_vmresume);
 void unload_guest_pcore(struct proc *p, int guest_pcoreid);
 
 #define VMM_MSR_EMU_READ               1