swtch saves callee-saved registers

swtch idles on per-CPU stack, not on calling process's stack
fix pipe bugs
usertest.c tests pipes, fork, exit, close

Makefile

@@ -5,7 +5,7 @@ CC = i386-jos-elf-gcc
 LD = i386-jos-elf-ld
 OBJCOPY = i386-jos-elf-objcopy
 OBJDUMP = i386-jos-elf-objdump
-CFLAGS = -nostdinc -I. -O -Wall -MD
+CFLAGS = -nostdinc -I. -O2 -Wall -MD
 
 xv6.img : bootblock kernel
 	dd if=/dev/zero of=xv6.img count=10000

fd.c

@@ -37,19 +37,6 @@ fd_alloc()
   return 0;
 }
 
-void
-fd_close(struct fd *fd)
-{
-  if(fd->type == FD_CLOSED || fd->count <= 0)
-    panic("fd_close");
-  fd->count -= 1;
-  if(fd->count == 0){
-    if(fd->type == FD_PIPE)
-      pipe_close(fd->pipe, fd->writeable);
-    fd->type = FD_CLOSED;
-  }
-}
-
 /*
  * addr is a kernel address, pointing into some process's p->mem.
  */
@@ -78,3 +65,20 @@ fd_read(struct fd *fd, char *addr, int n)
     return -1;
   }
 }
+
+void
+fd_close(struct fd *fd)
+{
+  if(fd->count < 1 || fd->type == FD_CLOSED)
+    panic("fd_close");
+  fd->count -= 1;
+
+  if(fd->count == 0){
+    if(fd->type == FD_PIPE){
+      pipe_close(fd->pipe, fd->writeable);
+    } else {
+      panic("fd_close");
+    }
+    fd->type = FD_CLOSED;
+  }
+}

kalloc.c

@@ -45,10 +45,15 @@ kfree(char *cp, int len)
   struct run **rr;
   struct run *p = (struct run *) cp;
   struct run *pend = (struct run *) (cp + len);
+  int i;
 
   if(len % PAGE)
     panic("kfree");
 
+  // XXX fill with junk to help debug
+  for(i = 0; i < len; i++)
+    cp[i] = 1;
+
   rr = &freelist;
   while(*rr){
     struct run *rend = (struct run *) ((char *)(*rr) + (*rr)->len);

main.c

@@ -46,7 +46,7 @@ main()
   p = &proc[0];
   curproc[cpu()] = p;
   p->state = WAITING;
-  p->sz = PAGE;
+  p->sz = 4 * PAGE;
   p->mem = kalloc(p->sz);
   memset(p->mem, 0, p->sz);
   p->kstack = kalloc(KSTACKSIZE);

mp.c

@@ -6,6 +6,7 @@
 #include "x86.h"
 #include "traps.h"
 #include "mmu.h"
+#include "proc.h"
 
 /* 
  * Credit: Plan 9 sources, Intel MP spec, and Cliff Frey
@@ -92,16 +93,11 @@ enum {					/* LAPIC_TDCR */
 };
 
 #define APBOOTCODE 0x7000 // XXX hack
-#define MPSTACK 512
 
 static struct MP* mp;  // The MP floating point structure
 static uint32_t *lapicaddr;
-static struct cpu {
+struct cpu cpus[NCPU];
-  uint8_t apicid;       // Local APIC ID
+int ncpu;
-  int lintr[2];		// Local APIC
-  char mpstack[MPSTACK]; // per-cpu start-up stack, only used to get into main()
-} cpus[NCPU];
-static int ncpu;
 static struct cpu *bcpu;
 
 static int
@@ -130,7 +126,7 @@ lapic_timerinit()
 void
 lapic_timerintr()
 {
-  cprintf("%d: timer interrupt!\n", cpu());
+  // cprintf("%d: timer interrupt!\n", cpu());
   lapic_write (LAPIC_EOI, 0);
 }
 

pipe.c

@@ -28,6 +28,10 @@ pipe_alloc(struct fd **fd1, struct fd **fd2)
     goto oops;
   if((p = (struct pipe *) kalloc(PAGE)) == 0)
     goto oops;
+  p->readopen = 1;
+  p->writeopen = 1;
+  p->writep = 0;
+  p->readp = 0;
   (*fd1)->type = FD_PIPE;
   (*fd1)->readable = 1;
   (*fd1)->writeable = 0;
@@ -54,10 +58,13 @@ pipe_alloc(struct fd **fd1, struct fd **fd2)
 void
 pipe_close(struct pipe *p, int writeable)
 {
-  if(writeable)
+  if(writeable){
     p->writeopen = 0;
-  else
+    wakeup(&p->readp);
+  } else {
     p->readopen = 0;
+    wakeup(&p->writep);
+  }
   if(p->readopen == 0 && p->writeopen == 0)
     kfree((char *) p, PAGE);
 }
@@ -71,11 +78,13 @@ pipe_write(struct pipe *p, char *addr, int n)
     while(((p->writep + 1) % PIPESIZE) == p->readp){
       if(p->readopen == 0)
         return -1;
+      wakeup(&p->readp);
       sleep(&p->writep);
     }
     p->data[p->writep] = addr[i];
     p->writep = (p->writep + 1) % PIPESIZE;
   }
+  wakeup(&p->readp);
   return i;
 }
 
@@ -96,5 +105,6 @@ pipe_read(struct pipe *p, char *addr, int n)
     addr[i] = p->data[p->readp];
     p->readp = (p->readp + 1) % PIPESIZE;
   }
+  wakeup(&p->writep);
   return i;
 }

proc.c

@@ -104,37 +104,47 @@ swtch()
 {
   struct proc *np;
   struct proc *op = curproc[cpu()];
+  unsigned sp;
+  int i;
 
-  while(1){
+  // force push of callee-saved registers
-    np = op + 1;
+  asm volatile("nop" : : : "%edi", "%esi", "%ebx");
-    while(np != op){
-      if(np->state == RUNNABLE)
-        break;
-      np++;
-      if(np == &proc[NPROC])
-        np = &proc[0];
-    }
-    if(np->state == RUNNABLE)
-      break;
-    // cprintf("swtch: nothing to run\n");
-    release_spinlock(&kernel_lock);
-    acquire_spinlock(&kernel_lock);
-  }
 
-  // XXX this may be too late, should probably save on the way
+  // save calling process's stack pointers
-  // in, in case some other CPU decided to run curproc
-  // before we got here. in fact setting state=WAITING and
-  // setting these variables had better be atomic w.r.t. other CPUs.
   op->ebp = read_ebp();
   op->esp = read_esp();
 
-  cprintf("cpu %d swtch %x -> %x\n", cpu(), op, np);
+  // don't execute on calling process's stack
+  sp = (unsigned) cpus[cpu()].mpstack + MPSTACK - 32;
+  asm volatile("movl %0, %%esp" : : "g" (sp));
+  asm volatile("movl %0, %%ebp" : : "g" (sp));
+
+  // gcc might store op on the stack
+  np = curproc[cpu()];
+  np = np + 1;
+
+  while(1){
+    for(i = 0; i < NPROC; i++){
+      if(np >= &proc[NPROC])
+        np = &proc[0];
+      if(np->state == RUNNABLE)
+        break;
+      np++;
+    }
+    if(i < NPROC)
+      break;
+    // cprintf("swtch %d: nothing to run %d %d\n",
+            // cpu(), proc[1].state, proc[2].state);
+    release_spinlock(&kernel_lock);
+    acquire_spinlock(&kernel_lock);
+    np = &proc[0];
+  }
+  
+  cprintf("cpu %d swtch %x -> %x\n", cpu(), curproc[cpu()], np);
 
   curproc[cpu()] = np;
   np->state = RUNNING;
 
-  // XXX callee-saved registers?
-
   // h/w sets busy bit in TSS descriptor sometimes, and faults
   // if it's set in LTR. so clear tss descriptor busy bit.
   np->gdt[SEG_TSS].sd_type = STS_T32A;

proc.h

@@ -37,3 +37,14 @@ struct proc{
 
 extern struct proc proc[];
 extern struct proc *curproc[NCPU];
+
+#define MPSTACK 512
+
+struct cpu {
+  uint8_t apicid;       // Local APIC ID
+  int lintr[2];		// Local APIC
+  char mpstack[MPSTACK]; // per-cpu start-up stack, only used to get into main()
+};
+
+extern struct cpu cpus[NCPU];
+extern int ncpu;

syscall.c

@@ -91,7 +91,7 @@ sys_pipe()
 int
 sys_write()
 {
-  int fd, n;
+  int fd, n, ret;
   unsigned addr;
   struct proc *p = curproc[cpu()];
 
@@ -103,13 +103,14 @@ sys_write()
     return -1;
   if(addr + n > p->sz)
     return -1;
-  return fd_write(p->fds[fd], p->mem + addr, n);
+  ret = fd_write(p->fds[fd], p->mem + addr, n);
+  return ret;
 }
 
 int
 sys_read()
 {
-  int fd, n;
+  int fd, n, ret;
   unsigned addr;
   struct proc *p = curproc[cpu()];
 
@@ -121,7 +122,25 @@ sys_read()
     return -1;
   if(addr + n > p->sz)
     return -1;
-  return fd_read(p->fds[fd], p->mem + addr, n);
+  ret = fd_read(p->fds[fd], p->mem + addr, n);
+  return ret;
+}
+
+int
+sys_close()
+{
+  int fd;
+  struct proc *p = curproc[cpu()];
+
+  if(fetcharg(0, &fd) < 0)
+    return -1;
+  if(fd < 0 || fd >= NOFILE)
+    return -1;
+  if(p->fds[fd] == 0)
+    return -1;
+  fd_close(p->fds[fd]);
+  p->fds[fd] = 0;
+  return 0;
 }
 
 int
@@ -138,6 +157,14 @@ sys_exit()
 {
   struct proc *p;
   struct proc *cp = curproc[cpu()];
+  int fd;
+
+  for(fd = 0; fd < NOFILE; fd++){
+    if(cp->fds[fd]){
+      fd_close(cp->fds[fd]);
+      cp->fds[fd] = 0;
+    }
+  }
 
   cp->state = ZOMBIE;
 
@@ -227,6 +254,9 @@ syscall()
   case SYS_read:
     ret = sys_read();
     break;
+  case SYS_close:
+    ret = sys_close();
+    break;
   default:
     cprintf("unknown sys call %d\n", num);
     // XXX fault

syscall.h

@@ -5,3 +5,4 @@
 #define SYS_pipe 5
 #define SYS_write 6
 #define SYS_read 7
+#define SYS_close 8

trap.c

@@ -35,16 +35,28 @@ trap(struct Trapframe *tf)
 {
   int v = tf->tf_trapno;
 
+  if(tf->tf_cs == 0x8 && kernel_lock == cpu())
+    cprintf("cpu %d: trap from %x:%x with lock=%d\n",
+            cpu(), tf->tf_cs, tf->tf_eip, kernel_lock);
+
   acquire_spinlock(&kernel_lock); // released in trapret in trapasm.S
 
   if(v == T_SYSCALL){
-    curproc[cpu()]->tf = tf;
+    struct proc *cp = curproc[cpu()];
+    cp->tf = tf;
     syscall();
+    if(cp != curproc[cpu()])
+      panic("trap ret wrong curproc");
+    if(cp->state != RUNNING)
+      panic("trap ret but not RUNNING");
+    if(tf != cp->tf)
+      panic("trap ret wrong tf");
+    if(read_esp() < cp->kstack || read_esp() >= cp->kstack + KSTACKSIZE)
+      panic("trap ret esp wrong");
     return;
   }
 
   if(v == (IRQ_OFFSET + IRQ_TIMER)){
-    curproc[cpu()]->tf = tf;
     lapic_timerintr();
     return;
   }

ulib.c

@@ -5,6 +5,13 @@ fork()
   asm("int $48");
 }
 
+int
+exit()
+{
+  asm("mov $2, %eax");
+  asm("int $48");
+}
+
 void
 cons_putc(int c)
 {
@@ -42,3 +49,10 @@ write(int fd, char *buf, int n)
   asm("mov $6, %eax");
   asm("int $48");
 }
+
+int
+close(int fd)
+{
+  asm("mov $8, %eax");
+  asm("int $48");
+}

usertests.c

@@ -1,22 +1,48 @@
 // simple fork and pipe read/write
 
-char buf[32];
+char buf[2048];
 
 void
 pipe1()
 {
   int fds[2], pid;
+  int seq = 0, i, n, cc, total;
 
   pipe(fds);
-  pid = pipe();
+  pid = fork();
   if(pid == 0){
-    write(fds[1], "xyz", 4);
+    close(fds[0]);
-  } else {
+    for(n = 0; n < 5; n++){
-    read(fds[0], buf, sizeof(buf));
+      for(i = 0; i < 1033; i++)
-    if(buf[0] != 'x' || buf[1] != 'y'){
+        buf[i] = seq++;
-      puts("pipe1 oops\n");
+      if(write(fds[1], buf, 1033) != 1033){
-      return;
+        puts("pipe1 oops 1\n");
+        exit(1);
+      }
     }
+    exit(0);
+  } else {
+    close(fds[1]);
+    total = 0;
+    cc = 1;
+    while(1){
+      n = read(fds[0], buf, cc);
+      if(n < 1)
+        break;
+      for(i = 0; i < n; i++){
+        if((buf[i] & 0xff) != (seq++ & 0xff)){
+          puts("pipe1 oops 2\n");
+          return;
+        }
+      }
+      total += n;
+      cc = cc * 2;
+      if(cc > sizeof(buf))
+        cc = sizeof(buf);
+    }
+    if(total != 5 * 1033)
+      puts("pipe1 oops 3\n");
+    close(fds[0]);
   }
   puts("pipe1 ok\n");
 }

x86.h

@@ -244,6 +244,30 @@ read_esp(void)
         return esp;
 }
 
+static __inline uint32_t
+read_esi(void)
+{
+        uint32_t esi;
+        __asm __volatile("movl %%esi,%0" : "=r" (esi));
+        return esi;
+}
+
+static __inline uint32_t
+read_edi(void)
+{
+        uint32_t edi;
+        __asm __volatile("movl %%edi,%0" : "=r" (edi));
+        return edi;
+}
+
+static __inline uint32_t
+read_ebx(void)
+{
+        uint32_t ebx;
+        __asm __volatile("movl %%ebx,%0" : "=r" (ebx));
+        return ebx;
+}
+
 static __inline void
 cpuid(uint32_t info, uint32_t *eaxp, uint32_t *ebxp, uint32_t *ecxp, uint32_t *edxp)
 {
@@ -280,6 +304,20 @@ read_tsc(void)
         return tsc;
 }
 
+// disable interrupts
+static __inline void
+cli(void)
+{
+        __asm __volatile("cli");
+}
+
+// enable interrupts
+static __inline void
+sti(void)
+{
+        __asm __volatile("sti");
+}
+
 struct PushRegs {
     /* registers as pushed by pusha */
     uint32_t reg_edi;