VMM: Don't flush the EPT unnecessarily
authorBarret Rhoden <brho@cs.berkeley.edu>
Fri, 2 Dec 2016 19:52:52 +0000 (14:52 -0500)
committerBarret Rhoden <brho@cs.berkeley.edu>
Wed, 14 Dec 2016 00:43:24 +0000 (19:43 -0500)
We don't need to flush it when we load/unload the GPC, in general.  It's
more like flushing a TLB entry.

We still flush it when unloading the VMCS, since that's when we're leaving
the core for good.  There's a bit of paranoia there: we don't want any old
mappings around that other EPTs could use.  But I don't think that's
necessary.

The main thing is that lazy unload only works per-process.  When the
process leaves the core, we unload then too (__abandon_core()).  That's
when we need to flush the EPT entries: when the process leaves the core.
Otherwise, we could have an old mapping that doesn't get flushed, since
Akaros didn't send the tlb_shootdown() to the core.

Here's the scenario: (for a process 'P1')
- Lazy unload
- Abandon core, no longer belongs to P1
- Another core changes a mapping, sends TLB shootdowns to all cores of
  P1
- Allocate original core to P1
- P1 runs VM, same EPT.  Still has old mappings.

This is also a concern with cr3 and the regular KPT, which is why we
need to manage the EPT's TLB mappings similarly as the KPT.  I think the
current code is OK, but we should probably check it out.

Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
kern/arch/x86/vmm/intel/vmx.c
kern/arch/x86/vmm/vmm.c
kern/src/process.c

index 2ff99b0..2debc1b 100644 (file)
@@ -1361,6 +1361,7 @@ void vmx_clear_vmcs(void)
 
        if (gpc) {
                vmcs_clear(gpc->vmcs);
+               ept_sync_context(gpc_get_eptp(gpc));
                wmb(); /* write -1 after clearing */
                gpc->vmcs_core_id = -1;
                PERCPU_VAR(gpc_to_clear_to) = NULL;
index a500606..07a2cba 100644 (file)
@@ -179,7 +179,6 @@ struct guest_pcore *load_guest_pcore(struct proc *p, int guest_pcoreid,
        spin_unlock(&p->vmm.lock);
        /* 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, should_vmresume);
        /* Load guest's xcr0 */
        lxcr0(gpc->xcr0);
@@ -205,7 +204,6 @@ void unload_guest_pcore(struct proc *p, int guest_pcoreid)
        assert(gpc);
        spin_lock(&p->vmm.lock);
        assert(gpc->cpu != -1);
-       ept_sync_context(gpc_get_eptp(gpc));
        vmx_unload_guest_pcore(gpc);
        gpc->cpu = -1;
 
index 60171ba..b7f076c 100644 (file)
@@ -1891,7 +1891,12 @@ void proc_tlbshootdown(struct proc *p, uintptr_t start, uintptr_t end)
                        tlbflush();
                        break;
                case (PROC_RUNNING_M):
-                       /* TODO: (TLB) sanity checks and rounding on the ranges */
+                       /* TODO: (TLB) sanity checks and rounding on the ranges.
+                        *
+                        * We need to make sure that once a core that was online has been
+                        * removed from the online list, then it must receive a TLB flush
+                        * (abandon_core()) before running the process again.  Either that,
+                        * or make other decisions about who to TLB-shootdown. */
                        TAILQ_FOREACH(vc_i, &p->online_vcs, list) {
                                send_kernel_message(vc_i->pcoreid, __tlbshootdown, start, end,
                                                    0, KMSG_IMMEDIATE);