set_current_tf only applies to user trapframes
authorBarret Rhoden <brho@cs.berkeley.edu>
Fri, 9 Apr 2010 22:18:01 +0000 (15:18 -0700)
committerKevin Klues <klueska@cs.berkeley.edu>
Thu, 3 Nov 2011 00:35:41 +0000 (17:35 -0700)
This bug could have manifested itself with craziness when trying to
return from a syscall if an interrupt arrived while servicing the
syscall.

Documentation/process-internals.txt
kern/arch/i686/trap.c
kern/arch/sparc/trap.c
kern/include/process.h

index 918fdec..d80488c 100644 (file)
@@ -9,9 +9,10 @@ they muck with how processes work.
 Contents:
 1. Reference Counting
 2. When Do We Really Leave "Process Context"?
-3. Leaving the Kernel Stack:
-4. Preemption and Notification Issues:
-5. TBD
+3. Leaving the Kernel Stack
+4. Preemption and Notification Issues
+5. Current_tf
+6. TBD
 
 1. Reference Counting
 ===========================
@@ -650,5 +651,28 @@ We also considered using the transition stack as a signal that a process is in
 a notification handler.  The kernel can inspect the stack pointer to determine
 this.  It's possible, but unnecessary.
 
-5. TBD
+5. current_tf
+===========================
+current_tf is a per-core macro that returns a struct trapframe * that points
+back on the kernel stack to the user context that was running on the given core
+when an interrupt or trap happened.  Saving the reference to the TF helps
+simplify code that needs to do something with the TF (like save it and pop
+another TF).  This way, we don't need to pass the context all over the place,
+especially through code that might not care.
+
+current_tf should go along with current.  It's the current_tf of the current
+process.  Withouth 'current', it has no meaning.
+
+It does not point to kernel trapframes, which is important when we receive an
+interrupt in the kernel.  At one point, we were (hypothetically) clobbering the
+reference to the user trapframe, and were unable to recover.  We can get away
+with this because the kernel always returns to its previous context from a
+nested handler (via iret on x86).  
+
+In the future, we may need to save kernel contexts and may not always return via
+iret.  At which point, if the code path is deep enough that we don't want to
+carry the TF pointer, we may revisit this.  Until then, current_tf is just for
+userspace contexts, and is simply stored in per_cpu_info.
+
+6. TBD
 ===========================
index f7b1f92..b8f2810 100644 (file)
@@ -235,7 +235,8 @@ trap(trapframe_t *tf)
         * In general, only save the tf and any silly state once you know it
         * is necessary (blocking).  And only save it in env_tf when you know you
         * are single core (PROC_RUNNING_S) */
-       set_current_tf(tf);
+       if (!in_kernel(tf))
+               set_current_tf(tf);
 
        if ((tf->tf_cs & ~3) != GD_UT && (tf->tf_cs & ~3) != GD_KT) {
                print_trapframe(tf);
@@ -252,8 +253,8 @@ trap(trapframe_t *tf)
 void
 irq_handler(trapframe_t *tf)
 {
-       // save a per-core reference to the tf
-       set_current_tf(tf);
+       if (!in_kernel(tf))
+               set_current_tf(tf);
        //if (core_id())
        //      cprintf("Incoming IRQ, ISR: %d on core %d\n", tf->tf_trapno, core_id());
        // merge this with alltraps?  other than the EOI... or do the same in all traps
@@ -364,8 +365,8 @@ void sysenter_init(void)
 /* This is called from sysenter's asm, with the tf on the kernel stack. */
 void sysenter_callwrapper(struct trapframe *tf)
 {
-       // save a per-core reference to the tf
-       set_current_tf(tf);
+       if (!in_kernel(tf))
+               set_current_tf(tf);
 
        // syscall code wants an edible reference for current
        proc_incref(current, 1);
index 25ada49..ad3b3f7 100644 (file)
@@ -384,7 +384,8 @@ handle_syscall(trapframe_t* state)
         * In general, only save the tf and any silly state once you know it
         * is necessary (blocking).  And only save it in env_tf when you know you
         * are single core (PROC_RUNNING_S) */
-       set_current_tf(state);
+       if (!in_kernel(state))
+               set_current_tf(state);
 
        // syscall code wants an edible reference for current
        proc_incref(current, 1);
index 0213b10..d530cc7 100644 (file)
@@ -132,10 +132,13 @@ void proc_decref(struct proc *SAFE p, size_t count);
 #define current per_cpu_info[core_id()].cur_proc
 #define set_current_proc(p) per_cpu_info[core_id()].cur_proc = (p)
 
-/* Allows the kernel to figure out what tf is on this core's stack.  Can be used
+/* Allows the kernel to figure out what *user* tf is on this core's stack.  Can be used
  * just like a pointer to a struct Trapframe.  Need these to be macros due to
  * some circular dependencies with smp.h.  This is done here instead of
- * elsewhere (like trap.h) for other elliptical reasons. */
+ * elsewhere (like trap.h) for other elliptical reasons.  Note the distinction
+ * between kernel and user contexts.  The kernel always returns to its nested,
+ * interrupted contexts via iret/etc.  We don't always do that for user
+ * contexts. */
 #define current_tf per_cpu_info[core_id()].cur_tf
 #define set_current_tf(tf) per_cpu_info[core_id()].cur_tf = (tf)