Split env_run into proc_startcore, early work
authorBarret Rhoden <brho@cs.berkeley.edu>
Thu, 30 Jul 2009 19:51:04 +0000 (12:51 -0700)
committerBarret Rhoden <brho@cs.berkeley.edu>
Thu, 30 Jul 2009 19:51:04 +0000 (12:51 -0700)
Lots of TODOs and gotchas in here, esp related to kernel stacks,
bundling process context, and process state.  I'm fairly confident the
lcr3 and refcnting is okay.  It probably isn't.  (TODO).

include/env.h
include/process.h
kern/src/env.c
kern/src/pmap.c
kern/src/process.c
kern/src/syscall.c
kern/src/trap.c

index 323a2f2..03c2bb5 100644 (file)
@@ -43,7 +43,7 @@ struct Env {
        trapframe_t env_tf;                     // Saved registers
        envid_t env_id;                         // Unique environment identifier
        envid_t env_parent_id;          // env_id of this env's parent
-       proc_state_t state;         // Status of the process
+       uint32_t state;                         // Status of the process
        uint32_t env_runs;                      // Number of times environment has run
        uint32_t env_refcnt;            // Reference count of kernel contexts using this
        uint32_t env_flags;
@@ -72,7 +72,7 @@ LIST_HEAD(env_list, Env);             // Declares 'struct env_list'
 typedef struct env_list env_list_t;
 
 void   env_init(void);
-int            env_alloc(env_t **e, envid_t parent_id);
+int            env_alloc(env_t *SAFE*SAFE e, envid_t parent_id);
 void   env_free(env_t *SAFE e);
 error_t        env_incref(env_t* e);
 void   env_decref(env_t *SAFE e);
index 40a7a66..4975a15 100644 (file)
 #ifndef ROS_KERN_PROCESS_H
 #define ROS_KERN_PROCESS_H
 
+#include <arch/types.h>
+
 /* Process States.  Not 100% on the names yet. */
-typedef enum {
-       ENV_FREE, // TODO don't use this shit for process allocation flagging
-       PROC_CREATED,
-       PROC_RUNNABLE_S,
-       PROC_RUNNING_S,
-       PROC_WAITING,             // can split out to INT and UINT
-       PROC_DYING,
-       PROC_RUNNABLE_M,          // ready, just needs all of its resources (cores)
-       PROC_RUNNING_M            // running, manycore style
-} proc_state_t;
+#define PROC_CREATED                   0x01
+#define PROC_RUNNABLE_S                        0x02
+#define PROC_RUNNING_S                 0x04
+#define PROC_WAITING                   0x08  // can split out to INT and UINT
+#define PROC_DYING                             0x10
+#define PROC_RUNNABLE_M                        0x20 // ready, needs all of its resources (cores)
+#define PROC_RUNNING_M                 0x40 // running, manycore style
+// TODO don't use this shit for process allocation flagging
+#define ENV_FREE                               0x80
 
 #include <env.h>
 
 // Till we remove the old struct Env
 #define proc Env
 
-int proc_set_state(struct proc *p, proc_state_t state) WRITES(p->state);
+int proc_set_state(struct proc *p, uint32_t state) WRITES(p->state);
 struct proc *get_proc(unsigned pid);
 bool proc_controls(struct proc *actor, struct proc *target);
+void proc_startcore(struct proc *p, trapframe_t *tf) __attribute__((noreturn));
 
 #endif // !ROS_KERN_PROCESS_H
index fbfbc6e..dd6fca9 100644 (file)
@@ -327,8 +327,6 @@ segment_alloc(env_t *e, void *SNT va, size_t len)
 //
 // Set up the initial program binary, stack, and processor flags
 // for a user process.
-// This function is ONLY called during kernel initialization,
-// before running the first user-mode environment.
 //
 // This function loads all loadable segments from the ELF binary image
 // into the environment's user memory, starting at the appropriate
@@ -337,47 +335,10 @@ segment_alloc(env_t *e, void *SNT va, size_t len)
 // that are marked in the program header as being mapped
 // but not actually present in the ELF file - i.e., the program's bss section.
 //
-// All this is very similar to what our boot loader does, except the boot
-// loader also needs to read the code from disk.  Take a look at
-// boot/main.c to get ideas.
-//
 // Finally, this function maps one page for the program's initial stack.
-//
-// load_icode panics if it encounters problems.
-//  - How might load_icode fail?  What might be wrong with the given input?
-//
 static void
-load_icode(env_t *e, uint8_t *COUNT(size) binary, size_t size)
+load_icode(env_t *SAFE e, uint8_t *COUNT(size) binary, size_t size)
 {
-       // Hints:
-       //  Load each program segment into virtual memory
-       //  at the address specified in the ELF section header.
-       //  You should only load segments with ph->p_type == ELF_PROG_LOAD.
-       //  Each segment's virtual address can be found in ph->p_va
-       //  and its size in memory can be found in ph->p_memsz.
-       //  The ph->p_filesz bytes from the ELF binary, starting at
-       //  'binary + ph->p_offset', should be copied to virtual address
-       //  ph->p_va.  Any remaining memory bytes should be cleared to zero.
-       //  (The ELF header should have ph->p_filesz <= ph->p_memsz.)
-       //  Use functions from the previous lab to allocate and map pages.
-       //
-       //  All page protection bits should be user read/write for now.
-       //  ELF segments are not necessarily page-aligned, but you can
-       //  assume for this function that no two segments will touch
-       //  the same virtual page.
-       //
-       //  You may find a function like segment_alloc useful.
-       //
-       //  Loading the segments is much simpler if you can move data
-       //  directly into the virtual addresses stored in the ELF binary.
-       //  So which page directory should be in force during
-       //  this function?
-       //
-       // Hint:
-       //  You must also do something with the program's entry point,
-       //  to make sure that the environment starts executing there.
-       //  What?  (See env_run() and env_pop_tf() below.)
-
        elf_t *elfhdr = (elf_t *)binary;
        int i, r;
 
@@ -389,6 +350,19 @@ load_icode(env_t *e, uint8_t *COUNT(size) binary, size_t size)
        // to actually access any pages alloc'd for this environment, we
        // need to have the hardware use this environment's page tables.
        uintreg_t old_cr3 = rcr3();
+       /*
+        * Even though we'll decref later and no one should be killing us at this
+        * stage, we're still going to wrap the lcr3s with incref/decref.
+        *
+        * Note we never decref on the old_cr3, since we aren't willing to let it
+        * die.  It's also not clear who the previous process is - sometimes it
+        * isn't even a process (when the kernel loads on its own, and not in
+        * response to a syscall).  Probably need to think more about this (TODO)
+        *
+        * This can get a bit tricky if this code blocks (will need to think about a
+        * decref then), if we try to change states, etc.
+        */
+       env_incref(e);
        lcr3(e->env_cr3);
 
        // TODO: how do we do a runtime COUNT?
@@ -414,6 +388,7 @@ load_icode(env_t *e, uint8_t *COUNT(size) binary, size_t size)
 
        // reload the original address space
        lcr3(old_cr3);
+       env_decref(e);
 }
 
 //
@@ -443,7 +418,9 @@ env_free(env_t *e)
        physaddr_t pa;
 
        // Note the environment's demise.
-       cprintf("[%08x] free env %08x\n", current ? current->env_id : 0, e->env_id);
+       printk("[%08x] free env %08x\n", current ? current->env_id : 0, e->env_id);
+       // All parts of the kernel should have decref'd before env_free was called. 
+       assert(e->env_refcnt == 0);
 
        // Flush all mapped pages in the user portion of the address space
        static_assert(UTOP % PTSIZE == 0);
@@ -468,10 +445,6 @@ env_free(env_t *e)
                page_decref(pa2page(pa));
        }
 
-       // Moved to page_decref
-       // need a known good pgdir before releasing the old one
-       //lcr3(boot_cr3);
-
        // free the page directory
        pa = e->env_cr3;
        e->env_pgdir = 0;
@@ -484,59 +457,90 @@ env_free(env_t *e)
 }
 
 /*
- * This allows the kernel to keep this process around, in case it is being used
- * in some asynchronous processing.
+ * The process refcnt is the number of places the process 'exists' in the
+ * system.  Creation counts as 1.  Having your page tables loaded somewhere
+ * (lcr3) counts as another 1.  A non-RUNNING_* process should have refcnt at
+ * least 1.  If the kernel is on another core and in a processes address space
+ * (like processing its backring), that counts as another 1.
+ *
+ * Note that the actual loading and unloading of cr3 is up to the caller, since
+ * that's not the only use for this (and decoupling is more flexible).
+ *
  * The refcnt should always be greater than 0 for processes that aren't dying.
  * When refcnt is 0, the process is dying and should not allow any more increfs.
- * TODO: Make sure this is never called from an interrupt handler (irq_save)
+ * A process can be dying with a refcnt greater than 0, since it could be
+ * waiting for other cores to "get the message" to die, or a kernel core can be
+ * finishing work in the processes's address space.
+ *
+ * Implementation aside, the important thing is that we atomically increment
+ * only if it wasn't already 0.  If it was 0, then we shouldn't be attaching to
+ * the process, so we return an error, which should be handled however is
+ * appropriate.  We currently use spinlocks, but some sort of clever atomics
+ * would work too.
+ *
+ * Also, no one should ever update the refcnt outside of these functions.
+ * Eventually, we'll have Ivy support for this. (TODO)
  */
 error_t env_incref(env_t* e)
 {
        error_t retval = 0;
-       spin_lock(&e->lock);
+       spin_lock_irqsave(&e->lock);
        if (e->env_refcnt)
                e->env_refcnt++;
        else
                retval = -EBADENV;
-       spin_unlock(&e->lock);
+       spin_unlock_irqsave(&e->lock);
        return retval;
 }
 
 /*
  * When the kernel is done with a process, it decrements its reference count.
  * When the count hits 0, no one is using it and it should be freed.
- * env_destroy calls this.
- * TODO: Make sure this is never called from an interrupt handler (irq_save)
+ * "Last one out" actually finalizes the death of the process.  This is tightly
+ * coupled with the previous function (incref)
+ * Be sure to load a different cr3 before calling this!
  */
 void env_decref(env_t* e)
 {
-       // need a known good pgdir before releasing the old one
-       // sometimes env_free is called on a different core than decref
-       lcr3(boot_cr3);
-
-       spin_lock(&e->lock);
+       spin_lock_irqsave(&e->lock);
        e->env_refcnt--;
-       spin_unlock(&e->lock);
+       spin_unlock_irqsave(&e->lock);
        // if we hit 0, no one else will increment and we can check outside the lock
        if (e->env_refcnt == 0)
                env_free(e);
 }
 
 
-//
-// Frees environment e.
-// If e was the current env, then runs a new environment (and does not return
-// to the caller).
-//
+/*
+ * Destroys the given process.  Can be called by a different process (checked
+ * via current), though that's unable to handle an async call (TODO current does
+ * not work asyncly, though it could be made to in the async processing
+ * function. 
+ */
 void
 env_destroy(env_t *e)
 {
        // TODO: XME race condition with env statuses, esp when running / destroying
        proc_set_state(e, PROC_DYING);
 
-       env_decref(e);
+       /*
+        * If we are currently running this address space on our core, we need a
+        * known good pgdir before releasing the old one.  This is currently the
+        * major practical implication of the kernel caring about a processes
+        * existence (the inc and decref).  This decref corresponds to the incref in
+        * proc_startcore (though it's not the only one).
+        */
+       if (current == e) {
+               lcr3(boot_cr3);
+               env_decref(e); // this decref is for the cr3
+       }
+       env_decref(e); // this decref is for the process in general
        atomic_dec(&num_envs);
 
+       /*
+        * Could consider removing this from destroy and having the caller specify
+        * these actions
+        */
        // for old envs that die on user cores.  since env run never returns, cores
        // never get back to their old hlt/relaxed/spin state, so we need to force
        // them back to an idle function.
@@ -625,60 +629,12 @@ void env_pop_tf_sysexit(trapframe_t *tf)
 void
 env_run(env_t *e)
 {
-       // Step 1: If this is a context switch (a new environment is running),
-       //         then set 'current' to the new environment,
-       //         update its 'env_runs' counter, and
-       //         and use lcr3() to switch to its address space.
-       // Step 2: Use env_pop_tf() to restore the environment's
-       //         registers and drop into user mode in the
-       //         environment.
-
-       // Hint: This function loads the new environment's state from
-       //      e->env_tf.  Go back through the code you wrote above
-       //      and make sure you have set the relevant parts of
-       //      e->env_tf to sensible values.
-
        // TODO: XME race here with env destroy on the status and refcnt
        // Could up the refcnt and down it when a process is not running
        
-       // TODO XME need different code paths for retarting a core (pop) and
-       // running for the first time
-       if (e->state == PROC_RUNNABLE_S)
-               proc_set_state(e, PROC_RUNNING_S);
-
-       if (e != current) {
-               current = e;
-               e->env_runs++;
-               lcr3(e->env_cr3); // think about the refcnt here
-       }
-       /* If the process entered the kernel via sysenter, we need to leave via
-        * sysexit.  sysenter trapframes have 0 for a CS, which is pushed in
-        * sysenter_handler.
-        *
-        *           FFFFFFFFF UU     UU    CCCCCC  KK     KK  #
-        *           FF        UU     UU  CCC       KK    KK   #
-        *           FF        UU     UU CC         KK  KK     #
-        *           FFFFFF    UU     UU CC         KKKKK      #
-        *           FF        UU     UU CC         KK  KK     #
-        *           FF         UU   UU   CCC       KK    KK    
-        *           FF          UUUUU      CCCCCC  KK     KK  #
-        *
-        * TODO think about when we no longer need the stack, in case we are
-        * preempted and expected to run again from somewhere else.  we can't
-        * expect to have the kernel stack around anymore.
-        * I think we need to make it such that the kernel in "process context"
-        * never gets removed from the core (displaced from its stack)
-        * would like to leave interrupts on too, so long as we come back.
-        * Consider a moveable flag or something
-        */
-       /* workqueue with the todo item put there by the interrupt handler when
-        * it realizes we were in the kernel in the first place.  disable ints
-        * before checking the queue and deciding to pop out or whatever.
-        */
-       if (e->env_tf.tf_cs)
-               env_pop_tf(&e->env_tf);
-       else
-               env_pop_tf_sysexit(&e->env_tf);
+       proc_set_state(e, PROC_RUNNING_S);
+
+       proc_startcore(e, &e->env_tf);
 }
 
 /* This is the top-half of an interrupt handler, where the bottom half is
index ba4c5de..d61cab1 100644 (file)
@@ -538,7 +538,7 @@ i386_vm_init(void)
        pgdir[0] = 0;
 
        // Flush the TLB for good measure, to kill the pgdir[0] mapping.
-       lcr3(boot_cr3);
+       tlb_flush_global();
 }
 
 //
index 0e02b90..af90949 100644 (file)
@@ -13,9 +13,9 @@
  * opportunity to check for bad transitions.  Might compile these out later, so
  * we shouldn't rely on them for sanity checking from userspace.
  */
-int proc_set_state(struct proc *p, proc_state_t state)
+int proc_set_state(struct proc *p, uint32_t state)
 {
-       proc_state_t curstate = p->state;
+       uint32_t curstate = p->state;
        /* Valid transitions:
         * C   -> RBS
         * RBS -> RGS
@@ -29,9 +29,12 @@ int proc_set_state(struct proc *p, proc_state_t state)
         * RGS -> D
         * RGM -> D
         * 
-        * These ought to be implemented later
+        * These ought to be implemented later (allowed, not thought through yet).
         * RBS -> D
         * RBM -> D
+        *
+        * This isn't allowed yet, may be later.
+        * C   -> D
         */
        #if 1 // some sort of correctness flag
        switch (curstate) {
@@ -40,12 +43,12 @@ int proc_set_state(struct proc *p, proc_state_t state)
                                panic("Invalid State Transition! PROC_CREATED to %d", state);
                        break;
                case PROC_RUNNABLE_S:
-                       if (state != PROC_RUNNING_S && state != PROC_DYING)
+                       if (!(state & (PROC_RUNNING_S | PROC_DYING)))
                                panic("Invalid State Transition! PROC_RUNNABLE_S to %d", state);
                        break;
                case PROC_RUNNING_S:
-                       if (state != PROC_RUNNABLE_S && state != PROC_RUNNABLE_M &&
-                           state != PROC_WAITING && state != PROC_DYING)
+                       if (!(state & (PROC_RUNNABLE_S | PROC_RUNNABLE_M | PROC_WAITING |
+                                      PROC_DYING)))
                                panic("Invalid State Transition! PROC_RUNNING_S to %d", state);
                        break;
                case PROC_WAITING:
@@ -53,16 +56,15 @@ int proc_set_state(struct proc *p, proc_state_t state)
                                panic("Invalid State Transition! PROC_WAITING to %d", state);
                        break;
                case PROC_DYING:
-                       if (state != PROC_CREATED) // when it is reused
+                       if (state != PROC_CREATED) // when it is reused (TODO)
                                panic("Invalid State Transition! PROC_DYING to %d", state);
                        break;
                case PROC_RUNNABLE_M:
-                       if (state != PROC_RUNNING_M && state != PROC_DYING)
+                       if (!(state & (PROC_RUNNING_M | PROC_DYING)))
                                panic("Invalid State Transition! PROC_RUNNABLE_M to %d", state);
                        break;
                case PROC_RUNNING_M:
-                       if (state != PROC_RUNNABLE_S && state != PROC_RUNNABLE_M &&
-                           state != PROC_DYING)
+                       if (!(state & (PROC_RUNNABLE_S | PROC_RUNNABLE_M | PROC_DYING)))
                                panic("Invalid State Transition! PROC_RUNNING_M to %d", state);
                        break;
        }
@@ -83,3 +85,64 @@ bool proc_controls(struct proc *actor, struct proc *target)
 {
        return target->env_parent_id == actor->env_id;
 }
+
+/*
+ * Runs the given context (trapframe) of process p on the core this code
+ * executes on.  The refcnt tracks how many cores have "an interest" in this
+ * process, which so far just means it uses the process's page table.  See the
+ * massive comments around the incref function
+ *
+ * TODO: think about how an interrupt could abort this, esp when we want to
+ * destroy it.  need a way to not lose the kernel stack.  For example, we could
+ * receive an IPI that tells us to preempt this process (or even kill it) and
+ * run something different.
+ * TODO: in general, think about when we no longer need the stack, in case we
+ * are preempted and expected to run again from somewhere else.  we can't
+ * expect to have the kernel stack around anymore.
+ *
+ * I think we need to make it such that the kernel in "process context"
+ * never gets removed from the core (displaced from its stack)
+ * would like to leave interrupts on too, so long as we come back.
+ * Consider a moveable flag or something.
+ *
+ * Perhaps we could have a workqueue with the todo item put there by the
+ * interrupt handler when it realizes we were in the kernel in the first place.
+ * disable ints before checking the queue and deciding to pop out or whatever to
+ * ensure atomicity.
+ */
+void proc_startcore(struct proc *p, trapframe_t *tf) {
+       /*
+        * TODO: okay, we have this.  now handle scenarios based on these
+        * assumptions (transitions from these states) like:
+        *              death attempt
+        *              preempt attempt
+        */
+       assert(p->state & (PROC_RUNNING_S | PROC_RUNNING_M));
+       /* If the process wasn't here, then we need to load its address space. */
+       if (p != current) {
+               if (env_incref(p))
+                       // getting here would mean someone tried killing this while we tried
+                       // to start one of it's contexts (from scratch, o/w we had it's CR3
+                       // loaded already)
+                       panic("Proc is dying, handle me!"); lcr3(p->env_cr3);
+               // we unloaded the old cr3, so decref it (if it exists)
+               // TODO: Consider moving this to wherever we really "mean to leave the
+               // process's context".
+               if (current)
+                       env_decref(current);
+               current = p;
+               /* also need to load our silly state, though this implies it's the same
+                * context, and not just the same process
+                * TODO: this is probably a lie, think about page faults
+                */
+               // load_our_silly_state();
+       }
+       /* If the process entered the kernel via sysenter, we need to leave via
+        * sysexit.  sysenter trapframes have 0 for a CS, which is pushed in
+        * sysenter_handler.
+        */
+       if (tf->tf_cs)
+               env_pop_tf(tf);
+       else
+               env_pop_tf_sysexit(tf);
+}
index 85d2cb9..2e52bb7 100644 (file)
@@ -37,8 +37,9 @@ void sysenter_callwrapper(struct Trapframe *tf)
         * careful here - we need to make sure that this current is the right
         * process, which could be weird if the syscall blocked.  it would need to
         * restore the proper value in current before returning to here.
+        * likewise, tf could be pointing to random gibberish.
         */
-       env_run(current);
+       proc_startcore(current, tf);
 }
 
 //Do absolutely nothing.  Used for profiling.
@@ -230,6 +231,9 @@ static void sys_yield(struct proc *p)
        // and schedule/yielding
        assert(p->state == PROC_RUNNING_S);
        p->state = PROC_RUNNABLE_S;
+       // the implied thing here is that all state has been saved.  and you need
+       // todo that before changing the state to RUNNABLE_S, since the process can
+       // get picked up somewhere else. TODO
        schedule();
 
        /* TODO
@@ -380,6 +384,8 @@ intreg_t process_generic_syscalls(env_t* e, size_t max)
                //printk("DEBUG POST: sring->req_prod: %d, sring->rsp_prod: %d\n",\
                           sysbr->sring->req_prod, sysbr->sring->rsp_prod);
        }
+       // load sane page tables (and don't rely on decref to do it for you).
+       lcr3(boot_cr3);
        env_decref(e);
        return (intreg_t)count;
 }
index f49608e..6046a73 100644 (file)
@@ -175,7 +175,7 @@ static void
                                syscall(current, tf->tf_regs.reg_eax, tf->tf_regs.reg_edx,
                                        tf->tf_regs.reg_ecx, tf->tf_regs.reg_ebx,
                                        tf->tf_regs.reg_edi, tf->tf_regs.reg_esi);
-                       env_run(current); // Note the comment in syscall.c
+                       proc_startcore(current, tf); // Note the comment in syscall.c
                        break;
                default:
                        // Unexpected trap: The user process or the kernel has a bug.
@@ -220,7 +220,7 @@ void
        // so far we never get here
        assert(0);
        // Return to the current environment, which should be runnable.
-       env_run(current);
+       proc_startcore(current, tf); // Note the comment in syscall.c
 }
 
 void