vmm: Overhaul how vthread_create works
authorBarret Rhoden <brho@cs.berkeley.edu>
Mon, 11 Sep 2017 21:56:48 +0000 (17:56 -0400)
committerBarret Rhoden <brho@cs.berkeley.edu>
Thu, 14 Sep 2017 20:38:44 +0000 (16:38 -0400)
vthread_attr_init() is gone.  That was a vestige of the "let me create the
threads implicitly and then have them ready later."

This splits vthread_create() into several helpers: allocation, context
initialization, and running.  The important part is splitting the
"creation" (allocation, setup, etc) from actually running it.

Some users, like dune, want to do stuff to the context after creating it,
but before creating the thread.  That's a mess.  You shouldn't access the
vm_tf before the thread was created!  We were getting by because we
declared the max number of vthreads in advance.

The old vthread_create() still works, though it returns a pointer to the
thread instead of success/fail.  It's just a convenience wrapper calling
the helpers.  It still takes the gpcid (guest), but that will change
shortly.  I kept the name intact, since it's similar to what people expect
from e.g. pthread_create.

As a side note, it was a bad sign that we didn't have a vthread struct or
header.  We might end up making vthreads more distinct from VMs in the
future.

Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
tests/dune/dune.c
tests/mmap_file_vmm.c
user/vmm/include/vmm/sched.h
user/vmm/include/vmm/vmm.h
user/vmm/include/vmm/vthread.h [new file with mode: 0644]
user/vmm/vthread.c

index abad37f..3d5d51d 100644 (file)
@@ -30,6 +30,7 @@
 #include <err.h>
 #include <vmm/linuxemu.h>
 #include <vmm/vmm.h>
+#include <vmm/vthread.h>
 
 struct vmm_gpcore_init gpci;
 bool linuxemu(struct guest_thread *gth, struct vm_trapframe *tf);
@@ -136,7 +137,6 @@ void dune_test(void *stack)
 static struct option long_options[] = {
        {"aux",           required_argument, 0, 'a'},
        {"debug",         no_argument,       0, 'd'},
-       {"vmmflags",      required_argument, 0, 'v'},
        {"memsize",       required_argument, 0, 'm'},
        {"memstart",      required_argument, 0, 'M'},
        {"cmdline_extra", required_argument, 0, 'c'},
@@ -231,11 +231,8 @@ int main(int argc, char **argv)
        void *tos;
        int envc, auxc, extrac = 0;
        struct elf_aux *auxv, *extra = NULL;
-       int vmmflags = 0;
        uint64_t entry = 0;
-       int ret;
-       struct vm_trapframe *vm_tf;
-       struct guest_thread *gth;
+       struct vthread *vth;
        int c;
        int test = 0;
        int option_index;
@@ -263,9 +260,6 @@ int main(int argc, char **argv)
                        fprintf(stderr, "SET DEBUG\n");
                        dune_debug++;
                        break;
-               case 'v':
-                       vmmflags = strtoull(optarg, 0, 0);
-                       break;
                case 'm':
                        memsize = strtoull(optarg, 0, 0);
                        break;
@@ -341,22 +335,12 @@ int main(int argc, char **argv)
                fprintf(stderr, "populated stack at %p; argc %d, envc %d, auxc %d\n",
                        tos, ac, envc, auxc);
 
-       ret = vthread_attr_init(&vm, vmmflags);
-       if (ret) {
-               fprintf(stderr, "vmm_init failed: %r\n");
-               exit(1);
-       }
-
-       gth = gpcid_to_gth(&vm, 0);
-       vm_tf = gth_to_vmtf(gth);
-
-       /* we can't use the default stack since we set one up
-        * ourselves. */
-       vm_tf->tf_rsp = (uint64_t)tos;
        if (dune_debug)
                fprintf(stderr, "stack is %p\n", tos);
 
-       vthread_create(&vm, 0, (void *)entry, tos);
+       vth = vthread_alloc(&vm, 0);
+       vthread_init_ctx(vth, entry, (uintptr_t)tos, (uintptr_t)tos);
+       vthread_run(vth);
 
        uthread_sleep_forever();
        return 0;
index 243c3c4..e66e625 100644 (file)
@@ -20,6 +20,7 @@
 #include <unistd.h>
 
 #include <vmm/vmm.h>
+#include <vmm/vthread.h>
 
 static struct virtual_machine vm = {.root_mtx = UTH_MUTEX_INIT};
 
@@ -82,13 +83,6 @@ int main(void)
                exit(-1);
        }
 
-       ret = vthread_attr_init(&vm, 0);
-       if (ret) {
-               fprintf(stderr, "vmm_init failed: %r\n");
-               exit(1);
-       }
-
-       fprintf(stderr, "Vthread attr init finished\n");
        nr_pgs = 1;
        void *loc = (void*) 0x1000000;
 
index 6da75bb..efe95ce 100644 (file)
@@ -89,7 +89,4 @@ void start_guest_thread(struct guest_thread *gth);
 struct task_thread *vmm_run_task(struct virtual_machine *vm,
                                  void *(*func)(void *), void *arg);
 
-int vthread_attr_init(struct virtual_machine *vm, int vmmflags);
-int vthread_create(struct virtual_machine *vm, int guest, void *rip, void *arg);
-
 __END_DECLS
index 5edfde1..ff2c9e2 100644 (file)
@@ -54,7 +54,6 @@ struct virtual_machine {
         * up_gpcs refers to the number of guest pcores that have
         * been started so far. */
        unsigned int                            up_gpcs;
-       bool                                            vminit;
 
        /* TODO: put these in appropriate structures.  e.g., virtio things in
         * something related to virtio.  low4k in something related to the guest's
diff --git a/user/vmm/include/vmm/vthread.h b/user/vmm/include/vmm/vthread.h
new file mode 100644 (file)
index 0000000..4e76f8b
--- /dev/null
@@ -0,0 +1,35 @@
+/* Copyright (c) 2017 Google Inc.
+ * Barret Rhoden <brho@cs.berkeley.edu>
+ * See LICENSE for details. */
+
+#pragma once
+
+#include <vmm/sched.h>
+
+__BEGIN_DECLS
+
+/* This will hang off the gth->user_data for vthread_create, but not for
+ * vthread_alloc.  You're on your own there.  Using this is a sign that we may
+ * need our own 2LS for vthreads. */
+struct vthread_info {
+       uintptr_t                                       stacktop;
+};
+
+struct vthread {
+       struct guest_thread                     gth;
+       /* Don't add to this structure without changing how these are allocated. */
+};
+
+static struct vm_trapframe *vth_to_vmtf(struct vthread *vth)
+{
+       return gth_to_vmtf((struct guest_thread*)vth);
+}
+
+struct vthread *vthread_alloc(struct virtual_machine *vm, int guest);
+void vthread_init_ctx(struct vthread *vth, uintptr_t entry_pt, uintptr_t arg,
+                      uintptr_t stacktop);
+void vthread_run(struct vthread *vthread);
+struct vthread *vthread_create(struct virtual_machine *vm, int guest,
+                               void *entry, void *arg);
+
+__END_DECLS
index fdeefdc..ab92c31 100644 (file)
@@ -11,7 +11,7 @@
 #include <sys/mman.h>
 #include <sys/syscall.h>
 #include <vmm/vmm.h>
-
+#include <vmm/vthread.h>
 
 static void *page(void *addr, int count)
 {
@@ -23,17 +23,14 @@ static void *page(void *addr, int count)
        return mmap(addr, count * 4096, PROT_READ | PROT_WRITE, flags, -1, 0);
 }
 
-/* vmsetup is a basic helper function used by vthread_attr_init */
-static int vmsetup(struct virtual_machine *vm, int flags)
+static void vmsetup(void *arg)
 {
+       struct virtual_machine *vm = (struct virtual_machine *)arg;
        struct vm_trapframe *vm_tf;
        int i, ret;
        uint8_t *p;
        struct vmm_gpcore_init *gpcis;
 
-       if (vm->vminit)
-               return -EBUSY;
-
        if (vm->nr_gpcs == 0)
                vm->nr_gpcs = 1;
 
@@ -43,10 +40,8 @@ static int vmsetup(struct virtual_machine *vm, int flags)
         * all guests. Currently, the kernel requires them. */
        for (i = 0; i < vm->nr_gpcs; i++) {
                p = page(NULL, 3);
-               if (!p) {
-                       werrstr("Can't allocate 3 pages for guest %d: %r", i);
-                       return -1;
-               }
+               if (!p)
+                       panic("Can't allocate 3 pages for guest %d: %r", i);
                gpcis[i].posted_irq_desc = &p[0];
                gpcis[i].vapic_addr = &p[4096];
                gpcis[i].apic_addr = &p[8192];
@@ -59,61 +54,89 @@ static int vmsetup(struct virtual_machine *vm, int flags)
        /* Set up default page mappings. */
        setup_paging(vm);
 
-       ret = vmm_init(vm, gpcis, flags);
-       if (ret)
-               return ret;
+       ret = vmm_init(vm, gpcis, 0);
+       assert(!ret);
        free(gpcis);
 
        for (i = 0; i < vm->nr_gpcs; i++) {
                vm_tf = gpcid_to_vmtf(vm, i);
                vm_tf->tf_cr3 = (uint64_t) vm->root;
        }
-       vm->vminit = 1;
+}
+
+struct vthread *vthread_alloc(struct virtual_machine *vm, int guest)
+{
+       static parlib_once_t once = PARLIB_ONCE_INIT;
+
+       parlib_run_once(&once, vmsetup, vm);
+
+       if (guest > vm->nr_gpcs)
+               return NULL;
+       return (struct vthread*)gpcid_to_gth(vm, guest);
+}
+
+/* TODO: this is arch specific */
+void vthread_init_ctx(struct vthread *vth, uintptr_t entry_pt, uintptr_t arg,
+                      uintptr_t stacktop)
+{
+       struct vm_trapframe *vm_tf = vth_to_vmtf(vth);
 
-       return 0;
+       vm_tf->tf_rip = entry_pt;
+       vm_tf->tf_rdi = arg;
+       vm_tf->tf_rsp = stacktop;
 }
 
-/* vthread_addr sets up a virtual_machine struct such that functions
- * can start up VM guests.  It is like pthread_attr in that it sets up
- * default attributes and can be used in vthread_create calls. If
- * vm->nrgpcs is not set then the vm will be set up for 1 guest. */
-int vthread_attr_init(struct virtual_machine *vm, int vmmflags)
+void vthread_run(struct vthread *vthread)
 {
-       return vmsetup(vm, vmmflags);
+       start_guest_thread((struct guest_thread*)vthread);
 }
 
 #define DEFAULT_STACK_SIZE 65536
-/* vthread_create creates and starts a VM guest. The interface is intended
- * to be as much like pthread_create as possible. */
-int vthread_create(struct virtual_machine *vm, int guest, void *rip, void *arg)
+static uintptr_t alloc_stacktop(void)
 {
-       struct vm_trapframe *vm_tf;
        int ret;
-       uint64_t *stack, *tos;
+       uintptr_t *stack, *tos;
 
-       if (!vm->vminit) {
-               return -EAGAIN;
-       }
+       ret = posix_memalign((void **)&stack, PGSIZE, DEFAULT_STACK_SIZE);
+       if (ret)
+               return 0;
+       /* touch the top word on the stack so we don't page fault
+        * on that in the VM. */
+       tos = &stack[DEFAULT_STACK_SIZE / sizeof(uint64_t) - 1];
+       *tos = 0;
+       return (uintptr_t)tos;
+}
 
-       if (guest > vm->nr_gpcs)
-               return -ENOENT;
-
-       vm_tf = gpcid_to_vmtf(vm, guest);
-
-       /* For now we make the default VM stack pretty small.
-        * We can grow it as needed. */
-       if (!vm_tf->tf_rsp) {
-               ret = posix_memalign((void **)&stack, 4096, DEFAULT_STACK_SIZE);
-               if (ret)
-                       return ret;
-               /* touch the top word on the stack so we don't page fault
-                * on that in the VM. */
-               tos = &stack[DEFAULT_STACK_SIZE/sizeof(uint64_t) - 1];
-               *tos = 0;
-               vm_tf->tf_rsp = (uint64_t) tos;
+static uintptr_t vth_get_stack(struct vthread *vth)
+{
+       struct guest_thread *gth = (struct guest_thread*)vth;
+       struct vthread_info *info = (struct vthread_info*)gth->user_data;
+       uintptr_t stacktop;
+
+       if (info) {
+               assert(info->stacktop);
+               return info->stacktop;
        }
-       vm_tf->tf_rip = (uint64_t)rip;
-       vm_tf->tf_rdi = (uint64_t)arg;
-       start_guest_thread(gpcid_to_gth(vm, guest));
-       return 0;
+       stacktop = alloc_stacktop();
+       assert(stacktop);
+       /* Yes, an evil part of me thought of using the top of the stack for this
+        * struct's storage. */
+       gth->user_data = malloc(sizeof(struct vthread_info));
+       assert(gth->user_data);
+       info = (struct vthread_info*)gth->user_data;
+       info->stacktop = stacktop;
+       return stacktop;
+}
+
+struct vthread *vthread_create(struct virtual_machine *vm, int guest,
+                               void *entry, void *arg)
+{
+       struct vthread *vth;
+
+       vth = vthread_alloc(vm, guest);
+       if (!vth)
+               return NULL;
+       vthread_init_ctx(vth, (uintptr_t)entry, (uintptr_t)arg, vth_get_stack(vth));
+       vthread_run(vth);
+       return vth;
 }