xv6: formatting, cleanup, rev5 (take 2)

Makefile

@@ -107,8 +107,8 @@ initcode: initcode.S
 	$(OBJCOPY) -S -O binary initcode.out initcode
 	$(OBJDUMP) -S initcode.o > initcode.asm
 
-kernel: $(OBJS) multiboot.o bootother initcode
+kernel: $(OBJS) multiboot.o data.o bootother initcode
-	$(LD) $(LDFLAGS) -Ttext 0x100000 -e main -o kernel multiboot.o $(OBJS) -b binary initcode bootother fs.img
+	$(LD) $(LDFLAGS) -Ttext 0x100000 -e main -o kernel multiboot.o data.o $(OBJS) -b binary initcode bootother
 	$(OBJDUMP) -S kernel > kernel.asm
 	$(OBJDUMP) -t kernel | sed '1,/SYMBOL TABLE/d; s/ .* / /; /^$$/d' > kernel.sym
 
@@ -119,8 +119,8 @@ kernel: $(OBJS) multiboot.o bootother initcode
 # great for testing the kernel on real hardware without
 # needing a scratch disk.
 MEMFSOBJS = $(filter-out ide.o,$(OBJS)) memide.o
-kernelmemfs: $(MEMFSOBJS) multiboot.o bootother initcode fs.img
+kernelmemfs: $(MEMFSOBJS) multiboot.o data.o bootother initcode fs.img
-	$(LD) $(LDFLAGS) -Ttext 0x100000 -e main -o kernelmemfs multiboot.o $(MEMFSOBJS) -b binary initcode bootother fs.img
+	$(LD) $(LDFLAGS) -Ttext 0x100000 -e main -o kernelmemfs multiboot.o data.o $(MEMFSOBJS) -b binary initcode bootother fs.img
 	$(OBJDUMP) -S kernelmemfs > kernelmemfs.asm
 	$(OBJDUMP) -t kernelmemfs | sed '1,/SYMBOL TABLE/d; s/ .* / /; /^$$/d' > kernelmemfs.sym
 
@@ -251,14 +251,16 @@ dist-test:
 	rm -rf dist-test
 	mkdir dist-test
 	cp dist/* dist-test
-	cd dist-test; ../m print
+	cd dist-test; $(MAKE) print
-	cd dist-test; ../m bochs || true
+	cd dist-test; $(MAKE) bochs || true
-	cd dist-test; ../m qemu
+	cd dist-test; $(MAKE) qemu
 
-# update this rule (change rev1) when it is time to
+# update this rule (change rev#) when it is time to
 # make a new revision.
 tar:
 	rm -rf /tmp/xv6
 	mkdir -p /tmp/xv6
 	cp dist/* dist/.gdbinit.tmpl /tmp/xv6
-	(cd /tmp; tar cf - xv6) | gzip >xv6-rev4.tar.gz
+	(cd /tmp; tar cf - xv6) | gzip >xv6-rev5.tar.gz
+
+.PHONY: dist-test dist

bootasm.S

@@ -13,7 +13,7 @@
 .code16                       # Assemble for 16-bit mode
 .globl start
 start:
-  cli                         # BIOS enabled interrupts ; disable
+  cli                         # BIOS enabled interrupts; disable
 
   # Set up the important data segment registers (DS, ES, SS).
   xorw    %ax,%ax             # Segment number zero
@@ -21,10 +21,8 @@ start:
   movw    %ax,%es             # -> Extra Segment
   movw    %ax,%ss             # -> Stack Segment
 
-  # Enable A20:
+  # Physical address line A20 is tied to zero so that the first PCs 
-  #   For backwards compatibility with the earliest PCs, physical
+  # with 2 MB would run software that assumed 1 MB.  Undo that.
-  #   address line 20 is tied low, so that addresses higher than
-  #   1MB wrap around to zero by default.  This code undoes this.
 seta20.1:
   inb     $0x64,%al               # Wait for not busy
   testb   $0x2,%al
@@ -41,28 +39,21 @@ seta20.2:
   movb    $0xdf,%al               # 0xdf -> port 0x60
   outb    %al,$0x60
 
-//PAGEBREAK!
+  # Switch from real to protected mode.  Use a bootstrap GDT that makes
-  # Switch from real to protected mode, using a bootstrap GDT
+  # virtual addresses map dierctly to  physical addresses so that the
-  # and segment translation that makes virtual addresses 
+  # effective memory map doesn't change during the transition.
-  # identical to physical addresses, so that the 
-  # effective memory map does not change after subsequent
-  # loads of segment registers.
   lgdt    gdtdesc
   movl    %cr0, %eax
   orl     $CR0_PE, %eax
   movl    %eax, %cr0
 
-  # This ljmp is how you load the CS (Code Segment) register.
+//PAGEBREAK!
-  # SEG_ASM produces segment descriptors with the 32-bit mode
+  # Complete transition to 32-bit protected mode by using long jmp
-  # flag set (the D flag), so addresses and word operands will
+  # to reload %cs and %eip.  The segment registers are set up with no
-  # default to 32 bits after this jump.
+  # translation, so that the mapping is still the identity mapping.
   ljmp    $(SEG_KCODE<<3), $start32
 
-# tell the assembler to generate 0x66 prefixes for 16-bit
+.code32  # Tell assembler to generate 32-bit code now.
-# instructions like movw, and to generate 32-bit immediate
-# addresses.
-.code32
-
 start32:
   # Set up the protected-mode data segment registers
   movw    $(SEG_KDATA<<3), %ax    # Our data segment selector

bootother.S

@@ -34,12 +34,12 @@ start:
   movw    %ax,%es
   movw    %ax,%ss
 
-//PAGEBREAK!
   lgdt    gdtdesc
   movl    %cr0, %eax
   orl     $CR0_PE, %eax
   movl    %eax, %cr0
 
+//PAGEBREAK!
   ljmp    $(SEG_KCODE<<3), $start32
 
 .code32

data.S

@@ -1,5 +1,24 @@
-# Define "data" symbol to mark beginning of data segment.
+// The kernel layout is:
-# Must be linked before any other data on ld command line.
+//
+//     text
+//     rodata
+//     data
+//     bss
+//
+// Conventionally, Unix linkers provide pseudo-symbols
+// etext, edata, and end, at the end of the text, data, and bss.
+// For the kernel mapping, we need the address at the beginning
+// of the data section, but that's not one of the conventional
+// symbols, because the convention started before there was a
+// read-only rodata section between text and data.
+//
+// To get the address of the data section, we define a symbol
+// named data and make sure this is the first object passed to
+// the linker, so that it will be the first symbol in the data section.
+//
+// Alternative approaches would be to parse our own ELF header
+// or to write a linker script, but this is simplest.
+
 .data
 .globl data
 data:

exec.c

@@ -10,8 +10,8 @@ int
 exec(char *path, char **argv)
 {
   char *s, *last;
-  int i, off, argc;
+  int i, off;
-  uint sz, sp, strings[MAXARG];
+  uint argc, sz, sp, ustack[3+MAXARG+1];
   struct elfhdr elf;
   struct inode *ip;
   struct proghdr ph;
@@ -53,49 +53,25 @@ exec(char *path, char **argv)
   if((sz = allocuvm(pgdir, sz, sz + PGSIZE)) == 0)
     goto bad;
 
-  // initialize stack content:
+  // Push argument strings, prepare rest of stack in ustack.
-
-  // "argumentN"                      -- nul-terminated string
-  // ...
-  // "argument0"
-  // 0                                -- argv[argc]
-  // address of argumentN             
-  // ...
-  // address of argument0             -- argv[0]
-  // address of address of argument0  -- argv argument to main()
-  // argc                             -- argc argument to main()
-  // ffffffff                         -- return PC for main() call
-
   sp = sz;
-
+  for(argc = 0; argv[argc]; argc++) {
-  // count arguments
-  for(argc = 0; argv[argc]; argc++)
-    ;
     if(argc >= MAXARG)
       goto bad;
-
+    sp -= strlen(argv[argc]) + 1;
-  // push strings and remember where they are
+    sp &= ~3;
-  for(i = argc - 1; i >= 0; --i){
+    if(copyout(pgdir, sp, argv[argc], strlen(argv[argc]) + 1) < 0)
-    sp -= strlen(argv[i]) + 1;
+      goto bad;
-    strings[i] = sp;
+    ustack[3+argc] = sp;
-    copyout(pgdir, sp, argv[i], strlen(argv[i]) + 1);
   }
+  ustack[3+argc] = 0;
 
-#define PUSH(x){ int xx = (int)(x); sp -= 4; copyout(pgdir, sp, &xx, 4); }
+  ustack[0] = 0xffffffff;  // fake return PC
+  ustack[1] = argc;
+  ustack[2] = sp - (argc+1)*4;  // argv pointer
 
-  PUSH(0); // argv[argc] is zero
+  sp -= (3+argc+1) * 4;
-
+  if(copyout(pgdir, sp, ustack, (3+argc+1)*4) < 0)
-  // push argv[] elements
-  for(i = argc - 1; i >= 0; --i)
-    PUSH(strings[i]);
-
-  PUSH(sp); // argv
-
-  PUSH(argc);
-
-  PUSH(0xffffffff); // in case main tries to return
-
-  if(sp < sz - PGSIZE)
     goto bad;
 
   // Save program name for debugging.
@@ -110,9 +86,7 @@ exec(char *path, char **argv)
   proc->sz = sz;
   proc->tf->eip = elf.entry;  // main
   proc->tf->esp = sp;
-
   switchuvm(proc);
-
   freevm(oldpgdir);
 
   return 0;

fs.h

@@ -41,7 +41,6 @@ struct dinode {
 // Block containing bit for block b
 #define BBLOCK(b, ninodes) (b/BPB + (ninodes)/IPB + 3)
 
-// PAGEBREAK: 10
 // Directory is a file containing a sequence of dirent structures.
 #define DIRSIZ 14
 

ide.c

@@ -96,7 +96,7 @@ ideintr(void)
   acquire(&idelock);
   if((b = idequeue) == 0){
     release(&idelock);
-    cprintf("Spurious IDE interrupt.\n");
+    // cprintf("spurious IDE interrupt\n");
     return;
   }
   idequeue = b->qnext;

main.c

@@ -89,7 +89,8 @@ bootothers(void)
   char *stack;
 
   // Write bootstrap code to unused memory at 0x7000.
-  // The linker has placed the image of bootother.S in _binary_bootother_start.
+  // The linker has placed the image of bootother.S in
+  // _binary_bootother_start.
   code = (uchar*)0x7000;
   memmove(code, _binary_bootother_start, (uint)_binary_bootother_size);
 
@@ -111,3 +112,7 @@ bootothers(void)
       ;
   }
 }
+
+//PAGEBREAK!
+// Blank page.
+

mp.c

@@ -39,7 +39,6 @@ mpsearch1(uchar *addr, int len)
 {
   uchar *e, *p;
 
-  cprintf("mpsearch1 0x%x %d\n", addr, len);
   e = addr+len;
   for(p = addr; p < e; p += sizeof(struct mp))
     if(memcmp(p, "_MP_", 4) == 0 && sum(p, sizeof(struct mp)) == 0)
@@ -113,7 +112,6 @@ mpinit(void)
     switch(*p){
     case MPPROC:
       proc = (struct mpproc*)p;
-      cprintf("mpproc %d\n", proc->apicid);
       if(ncpu != proc->apicid){
         cprintf("mpinit: ncpu=%d apicid=%d\n", ncpu, proc->apicid);
         ismp = 0;

proc.c

@@ -25,44 +25,6 @@ pinit(void)
   initlock(&ptable.lock, "ptable");
 }
 
-//PAGEBREAK: 36
-// Print a process listing to console.  For debugging.
-// Runs when user types ^P on console.
-// No lock to avoid wedging a stuck machine further.
-void
-procdump(void)
-{
-  static char *states[] = {
-  [UNUSED]    "unused",
-  [EMBRYO]    "embryo",
-  [SLEEPING]  "sleep ",
-  [RUNNABLE]  "runble",
-  [RUNNING]   "run   ",
-  [ZOMBIE]    "zombie"
-  };
-  int i;
-  struct proc *p;
-  char *state;
-  uint pc[10];
-  
-  for(p = ptable.proc; p < &ptable.proc[NPROC]; p++){
-    if(p->state == UNUSED)
-      continue;
-    if(p->state >= 0 && p->state < NELEM(states) && states[p->state])
-      state = states[p->state];
-    else
-      state = "???";
-    cprintf("%d %s %s", p->pid, state, p->name);
-    if(p->state == SLEEPING){
-      getcallerpcs((uint*)p->context->ebp+2, pc);
-      for(i=0; i<10 && pc[i] != 0; i++)
-        cprintf(" %p", pc[i]);
-    }
-    cprintf("\n");
-  }
-}
-
-
 //PAGEBREAK: 32
 // Look in the process table for an UNUSED proc.
 // If found, change state to EMBRYO and initialize
@@ -447,3 +409,41 @@ kill(int pid)
   return -1;
 }
 
+//PAGEBREAK: 36
+// Print a process listing to console.  For debugging.
+// Runs when user types ^P on console.
+// No lock to avoid wedging a stuck machine further.
+void
+procdump(void)
+{
+  static char *states[] = {
+  [UNUSED]    "unused",
+  [EMBRYO]    "embryo",
+  [SLEEPING]  "sleep ",
+  [RUNNABLE]  "runble",
+  [RUNNING]   "run   ",
+  [ZOMBIE]    "zombie"
+  };
+  int i;
+  struct proc *p;
+  char *state;
+  uint pc[10];
+  
+  for(p = ptable.proc; p < &ptable.proc[NPROC]; p++){
+    if(p->state == UNUSED)
+      continue;
+    if(p->state >= 0 && p->state < NELEM(states) && states[p->state])
+      state = states[p->state];
+    else
+      state = "???";
+    cprintf("%d %s %s", p->pid, state, p->name);
+    if(p->state == SLEEPING){
+      getcallerpcs((uint*)p->context->ebp+2, pc);
+      for(i=0; i<10 && pc[i] != 0; i++)
+        cprintf(" %p", pc[i]);
+    }
+    cprintf("\n");
+  }
+}
+
+

runoff.list

@@ -22,6 +22,7 @@ proc.h
 proc.c
 swtch.S
 kalloc.c
+data.S
 vm.c
 # system calls
 traps.h
@@ -48,6 +49,7 @@ exec.c
 # pipes
 pipe.c
 
+
 # string operations
 string.c
 
@@ -62,6 +64,7 @@ kbd.c
 console.c
 timer.c
 uart.c
+multiboot.S
 
 # user-level
 initcode.S
@@ -72,3 +75,4 @@ sh.c
 
 
 
+

runoff.spec

@@ -6,8 +6,8 @@ sheet1: left
 # pages.  The file may start in either column.
 #
 # "even" and "odd" specify which column a file must start on.  "even"
-# means it must start in the left of the two columns.  "odd" means it
+# means it must start in the left of the two columns (00).  "odd" means it
-# must start in the right of the two columns.
+# must start in the right of the two columns (50).
 #
 # You'd think these would be the other way around.
 
@@ -33,23 +33,23 @@ left: spinlock.h  # mild preference
 even: spinlock.h  # mild preference
 
 # This gets struct proc and allocproc on the same spread
-right: proc.h
+left: proc.h
-odd: proc.h
+even: proc.h
 
 # goal is to have two action-packed 2-page spreads,
 # one with
 #     userinit growproc fork exit wait
 # and another with
 #     scheduler sched yield forkret sleep wakeup1 wakeup
-left: proc.c   # VERY important
+right: proc.c   # VERY important
-odd: proc.c   # VERY important
+even: proc.c   # VERY important
 
 # A few more action packed spreads
 # page table creation and process loading
 #     walkpgdir mappages setupkvm vmenable switch[ku]vm inituvm loaduvm
 # process memory management
 #     allocuvm deallocuvm freevm
-right: vm.c
+left: vm.c
 odd: vm.c
 
 # kalloc.c either
@@ -69,17 +69,25 @@ odd: vm.c
 # file.h either
 # fs.h either
 # fsvar.h either
-left: ide.c
+# left: ide.c # mild preference
 even: ide.c
 # odd: bio.c
+
+# with fs.c starting on 2nd column of a left page, we get these 2-page spreads:
+#	ialloc iupdate iget idup ilock iunlock iput iunlockput
+#	bmap itrunc stati readi writei
+#	namecmp dirlookup dirlink skipelem namex namei
+#	fielinit filealloc filedup fileclose filestat fileread filewrite
+# starting on 2nd column of a right page is not terrible either
 odd: fs.c   # VERY important
+left: fs.c  # mild preference
 # file.c either
 # exec.c either
 # sysfile.c either
 
 # even: pipe.c  # mild preference
 # string.c either
-left: kbd.h
+# left: kbd.h  # mild preference
 even: kbd.h
 even: console.c
 odd: sh.c

runoff1

@@ -33,7 +33,7 @@ for($i=0; $i<@lines; ){
 	last if $i>=@lines;
 
 	# If the rest of the file fits, use the whole thing.
-	if(@lines <= $i+50){
+	if(@lines <= $i+50 && !grep { /PAGEBREAK/ } @lines){
 		$breakbefore = @lines;
 	}else{
 		# Find a good next page break;

toc.ftr

@@ -6,8 +6,8 @@ on the same line as the name, the line number (or, in a few cases, numbers)
 where the name is defined.  Successive lines in an entry list the line
 numbers where the name is used.  For example, this entry:
 
-    swtch 2308
+    swtch 2358
-        0317 2128 2166 2307 2308
+        0317 2128 2166 2357 2358
 
-indicates that swtch is defined on line 2308 and is mentioned on five lines
+indicates that swtch is defined on line 2358 and is mentioned on five lines
 on sheets 03, 21, and 23.

trap.c

@@ -59,6 +59,9 @@ trap(struct trapframe *tf)
     ideintr();
     lapiceoi();
     break;
+  case T_IRQ0 + IRQ_IDE+1:
+    // Bochs generates spurious IDE1 interrupts.
+    break;
   case T_IRQ0 + IRQ_KBD:
     kbdintr();
     lapiceoi();

usertests.c

@@ -1445,11 +1445,11 @@ bigargtest(void)
   ppid = getpid();
   pid = fork();
   if(pid == 0){
-    char *args[32];
+    char *args[32+1];
     int i;
-    for(i = 0; i < 32-1; i++)
+    for(i = 0; i < 32; i++)
       args[i] = "bigargs test: failed\n                                                                                                                     ";
-    args[32-1] = 0;
+    args[32] = 0;
     printf(stdout, "bigarg test\n");
     exec("echo", args);
     printf(stdout, "bigarg test ok\n");

vm.c

@@ -6,8 +6,18 @@
 #include "proc.h"
 #include "elf.h"
 
+extern char data[];  // defined in data.S
+
 static pde_t *kpgdir;  // for use in scheduler()
 
+// Allocate one page table for the machine for the kernel address
+// space for scheduler processes.
+void
+kvmalloc(void)
+{
+  kpgdir = setupkvm();
+}
+
 // Set up CPU's kernel segment descriptors.
 // Run once at boot time on each CPU.
 void
@@ -72,7 +82,6 @@ mappages(pde_t *pgdir, void *la, uint size, uint pa, int perm)
   
   a = PGROUNDDOWN(la);
   last = PGROUNDDOWN(la + size - 1);
-
   for(;;){
     pte = walkpgdir(pgdir, a, 1);
     if(pte == 0)
@@ -110,40 +119,32 @@ mappages(pde_t *pgdir, void *la, uint size, uint pa, int perm)
 // range from 0 till 640KB (USERTOP), which where the I/O hole starts
 // (both in physical memory and in the kernel's virtual address
 // space).
-
+static struct kmap {
-// Allocate one page table for the machine for the kernel address
+  void *p;
-// space for scheduler processes.
+  void *e;
-void
+  int perm;
-kvmalloc(void)
+} kmap[] = {
-{
+  {(void*)USERTOP,    (void*)0x100000, PTE_W},  // I/O space
-  kpgdir = setupkvm();
+  {(void*)0x100000,   data,            0    },  // kernel text, rodata
-}
+  {data,              (void*)PHYSTOP,  PTE_W},  // kernel data, memory
+  {(void*)0xFE000000, 0,               PTE_W},  // device mappings
+};
 
 // Set up kernel part of a page table.
 pde_t*
 setupkvm(void)
 {
-  extern char etext[];
-  char *rwstart;
   pde_t *pgdir;
-  uint rwlen;
+  struct kmap *k;
 
-  rwstart = PGROUNDDOWN(etext);
-  rwlen = (uint)rwstart - 0x100000;
-
-  // Allocate page directory
   if((pgdir = (pde_t*)kalloc()) == 0)
     return 0;
   memset(pgdir, 0, PGSIZE);
-  if(// Map IO space from 640K to 1Mbyte
+  k = kmap;
-     mappages(pgdir, (void*)USERTOP, 0x60000, USERTOP, PTE_W) < 0 ||
+  for(k = kmap; k < &kmap[NELEM(kmap)]; k++)
-     // Map kernel instructions
+    if(mappages(pgdir, k->p, k->e - k->p, (uint)k->p, k->perm) < 0)
-     mappages(pgdir, (void*)0x100000, rwlen, 0x100000, 0) < 0 ||
-     // Map kernel data and free memory pool
-     mappages(pgdir, rwstart, PHYSTOP-(uint)rwstart, (uint)rwstart, PTE_W) < 0 ||
-     // Map devices such as ioapic, lapic, ...
-     mappages(pgdir, (void*)0xFE000000, 0x2000000, 0xFE000000, PTE_W) < 0)
       return 0;
+
   return pgdir;
 }
 
@@ -162,48 +163,27 @@ vmenable(void)
 // Switch h/w page table register to the kernel-only page table,
 // for when no process is running.
 void
-switchkvm()
+switchkvm(void)
 {
   lcr3(PADDR(kpgdir));   // switch to the kernel page table
 }
 
-// Switch h/w page table and TSS registers to point to process p.
+// Switch TSS and h/w page table to correspond to process p.
 void
 switchuvm(struct proc *p)
 {
   pushcli();
-
-  // Setup TSS
   cpu->gdt[SEG_TSS] = SEG16(STS_T32A, &cpu->ts, sizeof(cpu->ts)-1, 0);
   cpu->gdt[SEG_TSS].s = 0;
   cpu->ts.ss0 = SEG_KDATA << 3;
   cpu->ts.esp0 = (uint)proc->kstack + KSTACKSIZE;
   ltr(SEG_TSS << 3);
-
   if(p->pgdir == 0)
-    panic("switchuvm: no pgdir\n");
+    panic("switchuvm: no pgdir");
-
   lcr3(PADDR(p->pgdir));  // switch to new address space
   popcli();
 }
 
-// Return the physical address that a given user address
-// maps to.  The result is also a kernel logical address,
-// since the kernel maps the physical memory allocated to user
-// processes directly.
-char*
-uva2ka(pde_t *pgdir, char *uva)
-{
-  pte_t *pte;
-
-  pte = walkpgdir(pgdir, uva, 0);
-  if((*pte & PTE_P) == 0)
-    return 0;
-  if((*pte & PTE_U) == 0)
-    return 0;
-  return (char*)PTE_ADDR(*pte);
-}
-
 // Load the initcode into address 0 of pgdir.
 // sz must be less than a page.
 void
@@ -228,10 +208,10 @@ loaduvm(pde_t *pgdir, char *addr, struct inode *ip, uint offset, uint sz)
   pte_t *pte;
 
   if((uint)addr % PGSIZE != 0)
-    panic("loaduvm: addr must be page aligned\n");
+    panic("loaduvm: addr must be page aligned");
   for(i = 0; i < sz; i += PGSIZE){
     if((pte = walkpgdir(pgdir, addr+i, 0)) == 0)
-      panic("loaduvm: address should exist\n");
+      panic("loaduvm: address should exist");
     pa = PTE_ADDR(*pte);
     if(sz - i < PGSIZE)
       n = sz - i;
@@ -243,10 +223,8 @@ loaduvm(pde_t *pgdir, char *addr, struct inode *ip, uint offset, uint sz)
   return 0;
 }
 
-// Allocate memory to the process to bring its size from oldsz to
+// Allocate page tables and physical memory to grow process from oldsz to
-// newsz. Allocates physical memory and page table entries. oldsz and
+// newsz, which need not be page aligned.  Returns new size or 0 on error.
-// newsz need not be page-aligned, nor does newsz have to be larger
-// than oldsz.  Returns the new process size or 0 on error.
 int
 allocuvm(pde_t *pgdir, uint oldsz, uint newsz)
 {
@@ -330,9 +308,9 @@ copyuvm(pde_t *pgdir, uint sz)
     return 0;
   for(i = 0; i < sz; i += PGSIZE){
     if((pte = walkpgdir(pgdir, (void*)i, 0)) == 0)
-      panic("copyuvm: pte should exist\n");
+      panic("copyuvm: pte should exist");
     if(!(*pte & PTE_P))
-      panic("copyuvm: page not present\n");
+      panic("copyuvm: page not present");
     pa = PTE_ADDR(*pte);
     if((mem = kalloc()) == 0)
       goto bad;
@@ -347,16 +325,31 @@ bad:
   return 0;
 }
 
-// copy some data to user address va in page table pgdir.
+//PAGEBREAK!
-// most useful when pgdir is not the current page table.
+// Map user virtual address to kernel physical address.
+char*
+uva2ka(pde_t *pgdir, char *uva)
+{
+  pte_t *pte;
+
+  pte = walkpgdir(pgdir, uva, 0);
+  if((*pte & PTE_P) == 0)
+    return 0;
+  if((*pte & PTE_U) == 0)
+    return 0;
+  return (char*)PTE_ADDR(*pte);
+}
+
+// Copy len bytes from p to user address va in page table pgdir.
+// Most useful when pgdir is not the current page table.
 // uva2ka ensures this only works for PTE_U pages.
 int
-copyout(pde_t *pgdir, uint va, void *xbuf, uint len)
+copyout(pde_t *pgdir, uint va, void *p, uint len)
 {
   char *buf, *pa0;
   uint n, va0;
   
-  buf = (char*)xbuf;
+  buf = (char*)p;
   while(len > 0){
     va0 = (uint)PGROUNDDOWN(va);
     pa0 = uva2ka(pgdir, (char*)va0);