Clean up handling of stack frames.

diff --git a/src/threads/init.c b/src/threads/init.c
index 3eb45add76b9a74a4828648974a83fde4ae7f00a..da1a98c69da9bcd70374255b98bf591d106b1051 100644 (file)
--- a/src/threads/init.c
+++ b/src/threads/init.c
@@ -32,7 +32,7 @@ void power_off (void);
 static void
 main_thread (void *aux UNUSED) 
 {
-  thread_execute ("a.out");
+  printk ("execute=%d\n", (int) thread_execute ("a.out"));
 }
 
 int

diff --git a/src/threads/interrupt.h b/src/threads/interrupt.h
index 834d796d971feb76ae63fbcc0006bd8f571d0ad8..2a2b3ef0818409fb7e10c62b50fe3f1affc4a140 100644 (file)
--- a/src/threads/interrupt.h
+++ b/src/threads/interrupt.h
@@ -17,10 +17,11 @@ enum if_level intr_disable (void);
 \f
 struct intr_args
   {
+    /* Pushed by the stubs. */
     uint32_t edi;
     uint32_t esi;
     uint32_t ebp;
-    uint32_t esp;
+    uint32_t esp_dummy;
     uint32_t ebx;
     uint32_t edx;
     uint32_t ecx;
@@ -28,8 +29,16 @@ struct intr_args
     uint16_t es, :16;
     uint16_t ds, :16;
     uint32_t vec_no;
+
+    /* Sometimes pushed by the CPU, otherwise by the stubs. */
     uint32_t error_code;
-    uint32_t eip;
+
+    /* Pushed by the CPU. */
+    void (*eip) (void);
+    uint16_t cs, :16;
+    uint32_t eflags;
+    uint32_t esp;
+    uint16_t ss, :16;
   };
 
 typedef void intr_handler_func (struct intr_args *);

diff --git a/src/threads/paging.c b/src/threads/paging.c
index 9fd568caf894107e02b09edda8dff05d9d060d9e..2fa549cd9424c73a0abc133b506aabf0cc13fe20 100644 (file)
--- a/src/threads/paging.c
+++ b/src/threads/paging.c
@@ -77,8 +77,7 @@ paging_init (void)
       pt[pte_idx] = make_pte (vaddr, true);
     }
 
-  /* Set the page table. */
-  asm volatile ("movl %0,%%cr3" :: "r" (vtop (pd)));
+  pagedir_activate (pd);
 }
 
 uint32_t *
@@ -225,4 +224,8 @@ pagedir_next (uint32_t *pd, void **upage)
   return kpage;
 }
 
-void pagedir_activate (uint32_t *pagedir);
+void
+pagedir_activate (uint32_t *pagedir) 
+{
+  asm volatile ("movl %0,%%cr3" :: "r" (vtop (pagedir)));
+}

diff --git a/src/threads/switch.S b/src/threads/switch.S
index a7c27fb03eb8fc3cb73ccee19f07ef21438d61ea..5b661ec1f1fd88924704dc95c2238987479dc6ab 100644 (file)
--- a/src/threads/switch.S
+++ b/src/threads/switch.S
@@ -1,7 +1,4 @@
-#### 0(%esp) - old registers
-#### 16(%esp) - return address
-#### 20(%esp) - current thread
-#### 24(%esp) - new thread
+#include "switch.h"
 
         .globl thread_switch 
 thread_switch:
@@ -18,14 +15,14 @@ thread_switch:
        mov thread_stack_ofs, %edx
 
        # Save current stack pointer to old thread's stack, if any.
-       movl 20(%esp), %eax
+       movl SWITCH_CUR(%esp), %eax
        test %eax, %eax
        jz 1f
        movl %esp, (%eax,%edx,1)
 1:
 
        # Restore stack pointer from new thread's stack.
-       movl 24(%esp), %ecx
+       movl SWITCH_NEXT(%esp), %ecx
        movl (%ecx,%edx,1), %esp
 
        # Restore caller's register state.

diff --git a/src/threads/switch.h b/src/threads/switch.h
new file mode 100644 (file)
index 0000000..c285e78
--- /dev/null
+++ b/src/threads/switch.h
@@ -0,0 +1,27 @@
+#ifndef HEADER_SWITCH_H
+#define HEADER_SWITCH_H 1
+
+#ifndef __ASSEMBLER__
+/* thread_switch()'s stack frame. */
+struct switch_frame 
+  {
+    uint32_t ebx;               /*  0: Saved %ebx. */
+    uint32_t ebp;               /*  4: Saved %ebp. */
+    uint32_t esi;               /*  8: Saved %esi. */
+    uint32_t edi;               /* 12: Saved %edi. */
+    void (*eip) (void);         /* 16: Return address. */
+    struct thread *cur;         /* 20: thread_switch()'s CUR argument. */
+    struct thread *next;        /* 24: thread_switch()'s NEXT argument. */
+  };
+
+/* Switches from CUR, which must be the running thread, to NEXT,
+   which must also be running thread_switch(), returning CUR in
+   NEXT's context. */
+struct thread *thread_switch (struct thread *cur, struct thread *next);
+#endif
+
+/* Offsets used by switch.S. */
+#define SWITCH_CUR      20
+#define SWITCH_NEXT     24
+
+#endif /* switch.h */

diff --git a/src/threads/thread.c b/src/threads/thread.c
index 9d75d18c1fb183cf222512188f087fbfc9b86e50..52f566ac0309802d602da4f6ceb8362fa0a62b74 100644 (file)
--- a/src/threads/thread.c
+++ b/src/threads/thread.c
@@ -6,21 +6,28 @@
 #include "mmu.h"
 #include "palloc.h"
 #include "random.h"
+#include "switch.h"
 
 uint32_t thread_stack_ofs = offsetof (struct thread, stack);
 
 static struct list run_queue;
 
-struct thread *thread_switch (struct thread *cur, struct thread *next);
-
 void
 thread_init (void) 
 {
   list_init (&run_queue);
 }
 
+struct thread_root_frame 
+  {
+    void *eip;                  /* Return address. */
+    void (*function) (void *);  /* Function to call. */
+    void *aux;                  /* Auxiliary data for function. */
+  };
+
 static void
-thread_root (void (*function) (void *aux), void *aux) 
+thread_root (struct thread *cur UNUSED, struct thread *next UNUSED,
+             void (*function) (void *aux), void *aux) 
 {
   ASSERT (function != NULL);
   
@@ -28,59 +35,80 @@ thread_root (void (*function) (void *aux), void *aux)
   thread_exit ();
 }
 
+static struct thread *
+new_thread (const char *name) 
+{
+  struct thread *t;
+
+  ASSERT (name != NULL);
+  
+  t = palloc_get (PAL_ZERO);
+  if (t != NULL)
+    {
+      strlcpy (t->name, name, sizeof t->name);
+      t->stack = (uint8_t *) t + PGSIZE;
+      t->status = THREAD_BLOCKED;
+    }
+  
+  return t;
+}
+
+static void *
+alloc_frame (struct thread *t, size_t size) 
+{
+  ASSERT (size % sizeof (uint32_t) == 0);
+
+  t->stack -= size;
+  return t->stack;
+}
+
 struct thread *
 thread_create (const char *name, void (*function) (void *aux), void *aux) 
 {
   struct thread *t;
+  struct thread_root_frame *rf;
+  struct switch_frame *sf;
 
-  ASSERT (name != NULL);
   ASSERT (function != NULL);
 
-  t = palloc_get (0);
-  if (t == NULL)
-    return NULL;
+  t = new_thread (name);
 
-  memset (t, 0, PGSIZE);
-  strlcpy (t->name, name, sizeof t->name);
+  /* Stack frame for thread_root(). */
+  rf = alloc_frame (t, sizeof *rf);
+  rf->eip = NULL;
+  rf->function = function;
+  rf->aux = aux;
 
-  /* Set up stack. */
-  t->stack = (uint32_t *) ((uint8_t *) t + PGSIZE);
-  *--t->stack = (uint32_t) aux;
-  *--t->stack = (uint32_t) function;
-  --t->stack;
-  *--t->stack = (uint32_t) thread_root;
-  t->stack -= 4;
+  /* Stack frame for thread_switch(). */
+  sf = alloc_frame (t, sizeof *sf);
+  sf->eip = (void (*) (void)) thread_root;
 
-  /* Add to run_queue. */
-  t->status = THREAD_BLOCKED;
+  /* Add to run queue. */
   thread_ready (t);
 
   return t;
 }
 
-static struct thread *
-stack_to_thread (uint32_t *stack) 
-{
-  return (struct thread *) ((uint32_t) (stack - 1) & ~((uint32_t) PGSIZE - 1));
-}
-
 struct thread *
 thread_current (void) 
 {
   uint32_t *esp;
   asm ("movl %%esp, %0\n" : "=g" (esp));
-  return stack_to_thread (esp);
+  return pg_round_down (esp);
 }
 
 #ifdef USERPROG
-void
+bool
 thread_execute (const char *filename) 
 {
-  struct thread *t = thread_current ();
+  struct thread *t = new_thread (filename);
+  if (t == NULL)
+    return false;
   
   if (!addrspace_load (&t->addrspace, filename)) 
     panic ("%s: program load failed", filename);
-  panic ("%s: loaded", filename);
+  printk ("%s: loaded\n", filename);
+  return true;
 }
 #endif
 
@@ -140,6 +168,10 @@ thread_schedule (void)
      switch. */
   asm volatile ("" : : : "memory");
 
+#ifdef USERPROG
+  addrspace_activate (&cur->addrspace);
+#endif
+
   if (prev != NULL && prev->status == THREAD_DYING) 
     thread_destroy (prev);
 

diff --git a/src/threads/thread.h b/src/threads/thread.h
index a9b3251249d11f6415065866cda95032f9a67194..8a0df6e254052ff7c8a8ee1b44fda01876fc1e4e 100644 (file)
--- a/src/threads/thread.h
+++ b/src/threads/thread.h
@@ -20,7 +20,7 @@ struct thread
   {
     enum thread_status status;
     char name[16];
-    uint32_t *stack;
+    uint8_t *stack;
     list_elem rq_elem;
 #ifdef USERPROG
     struct addrspace addrspace;
@@ -35,7 +35,7 @@ void thread_destroy (struct thread *);
 struct thread *thread_current (void);
 
 #ifdef USERPROG
-void thread_execute (const char *filename);
+bool thread_execute (const char *filename);
 #endif
 
 void thread_start (struct thread *);

diff --git a/src/userprog/addrspace.c b/src/userprog/addrspace.c
index 3837d4ed00a6e6edce942bdd2dcd25840041acee..28e89f7884ec2d5245ded6c056bddfe3b9fa885d 100644 (file)
--- a/src/userprog/addrspace.c
+++ b/src/userprog/addrspace.c
@@ -230,3 +230,12 @@ addrspace_destroy (struct addrspace *as)
   if (as != NULL && as->pagedir != NULL) 
     pagedir_destroy (as->pagedir); 
 }
+
+void
+addrspace_activate (struct addrspace *as) 
+{
+  ASSERT (as != NULL);
+  
+  if (as->pagedir != NULL)
+    pagedir_activate (as->pagedir);
+}

diff --git a/src/userprog/addrspace.h b/src/userprog/addrspace.h
index 8feb6b5c0388edb3e049b0f860d6c6875ed2ec23..8de4dfeca5f05de7fbe402b61a0f316ea55a7d8d 100644 (file)
--- a/src/userprog/addrspace.h
+++ b/src/userprog/addrspace.h
@@ -12,4 +12,6 @@ struct addrspace
 bool addrspace_load (struct addrspace *, const char *);
 void addrspace_destroy (struct addrspace *);
 
+void addrspace_activate (struct addrspace *);
+
 #endif /* addrspace.h */