exec compiles but argstr() doesn't work yet

Makefile

@@ -19,7 +19,9 @@ OBJS = \
   sleeplock.o \
   file.o \
   pipe.o \
-  ramdisk.o
+  ramdisk.o \
+  exec.o \
+  sysfile.o
 
 XXXOBJS = \
 	bio.o\

defs.h

@@ -107,6 +107,8 @@ int             cpuid(void);
 void            exit(void);
 int             fork(void);
 int             growproc(int);
+pagetable_t     proc_pagetable(struct proc *);
+void            proc_freepagetable(pagetable_t, uint64);
 int             kill(int);
 struct cpu*     mycpu(void);
 struct cpu*     getmycpu(void);
@@ -178,11 +180,14 @@ void            kvminit(void);
 void            kvmswitch(void);
 pagetable_t     uvmcreate(void);
 void            uvminit(pagetable_t, char *, uint);
-int             uvmdealloc(pagetable_t, uint64, uint64);
+uint64          uvmalloc(pagetable_t, uint64, uint64);
+uint64          uvmdealloc(pagetable_t, uint64, uint64);
 void            uvmcopy(pagetable_t, pagetable_t, uint64);
 void            uvmfree(pagetable_t, uint64);
 void            mappages(pagetable_t, uint64, uint64, uint64, int);
 void            unmappages(pagetable_t, uint64, uint64, int);
+uint64          walkaddr(pagetable_t, uint64);
+int             copyout(pagetable_t, uint64, char *, uint64);
 
 // number of elements in fixed-size array
 #define NELEM(x) (sizeof(x)/sizeof((x)[0]))

exec.c

@@ -1,14 +1,13 @@
 #include "types.h"
 #include "param.h"
 #include "memlayout.h"
-#include "mmu.h"
+#include "riscv.h"
 #include "proc.h"
 #include "defs.h"
-#include "traps.h"
-#include "msr.h"
-#include "x86.h"
 #include "elf.h"
 
+static int loadseg(pde_t *pgdir, uint64 addr, struct inode *ip, uint offset, uint sz);
+
 int
 exec(char *path, char **argv)
 {
@@ -18,9 +17,11 @@ exec(char *path, char **argv)
   struct elfhdr elf;
   struct inode *ip;
   struct proghdr ph;
-  pde_t *pgdir, *oldpgdir;
+  pagetable_t pagetable = 0, oldpagetable;
   struct proc *p = myproc();
   uint64 oldsz = p->sz;
+
+  printf("EXEC\n");
   
   begin_op();
 
@@ -29,7 +30,6 @@ exec(char *path, char **argv)
     return -1;
   }
   ilock(ip);
-  pgdir = 0;
 
   // Check ELF header
   if(readi(ip, (char*)&elf, 0, sizeof(elf)) != sizeof(elf))
@@ -37,7 +37,7 @@ exec(char *path, char **argv)
   if(elf.magic != ELF_MAGIC)
     goto bad;
 
-  if((pgdir = setupkvm()) == 0)
+  if((pagetable = proc_pagetable(p)) == 0)
     goto bad;
 
   // Load program into memory.
@@ -51,11 +51,11 @@ exec(char *path, char **argv)
       goto bad;
     if(ph.vaddr + ph.memsz < ph.vaddr)
       goto bad;
-    if((sz = allocuvm(pgdir, sz, ph.vaddr + ph.memsz)) == 0)
+    if((sz = uvmalloc(pagetable, sz, ph.vaddr + ph.memsz)) == 0)
       goto bad;
     if(ph.vaddr % PGSIZE != 0)
       goto bad;
-    if(loaduvm(pgdir, (char*)ph.vaddr, ip, ph.off, ph.filesz) < 0)
+    if(loadseg(pagetable, ph.vaddr, ip, ph.off, ph.filesz) < 0)
       goto bad;
   }
   iunlockput(ip);
@@ -63,11 +63,10 @@ exec(char *path, char **argv)
   ip = 0;
 
   // Allocate two pages at the next page boundary.
-  // Make the first inaccessible.  Use the second as the user stack.
+  // Use the second as the user stack.
   sz = PGROUNDUP(sz);
-  if((sz = allocuvm(pgdir, sz, sz + 2*PGSIZE)) == 0)
+  if((sz = uvmalloc(pagetable, sz, sz + 2*PGSIZE)) == 0)
     goto bad;
-  clearpteu(pgdir, (char*)(sz - 2*PGSIZE));
   sp = sz;
 
   // Push argument strings, prepare rest of stack in ustack.
@@ -75,7 +74,7 @@ exec(char *path, char **argv)
     if(argc >= MAXARG)
       goto bad;
     sp = (sp - (strlen(argv[argc]) + 1)) & ~(sizeof(uint64)-1);
-    if(copyout(pgdir, sp, argv[argc], strlen(argv[argc]) + 1) < 0)
+    if(copyout(pagetable, sp, argv[argc], strlen(argv[argc]) + 1) < 0)
       goto bad;
     ustack[3+argc] = sp;
   }
@@ -85,11 +84,12 @@ exec(char *path, char **argv)
   ustack[1] = argc;
   ustack[2] = sp - (argc+1)*sizeof(uint64);  // argv pointer
 
-  p->sf->rdi = argc;
+  // arguments to user main(argc, argv)
-  p->sf->rsi = sp - (argc+1)*sizeof(uint64);
+  p->tf->a0 = argc;
+  p->tf->a1 = sp - (argc+1)*sizeof(uint64);
 
   sp -= (3+argc+1) * sizeof(uint64);
-  if(copyout(pgdir, sp, ustack, (3+argc+1)*sizeof(uint64)) < 0)
+  if(copyout(pagetable, sp, (char *)ustack, (3+argc+1)*sizeof(uint64)) < 0)
     goto bad;
 
   // Save program name for debugging.
@@ -99,21 +99,47 @@ exec(char *path, char **argv)
   safestrcpy(p->name, last, sizeof(p->name));
     
   // Commit to the user image.
-  oldpgdir = p->pgdir;
+  oldpagetable = p->pagetable;
-  p->pgdir = pgdir;
+  p->pagetable = pagetable;
   p->sz = sz;
-  p->sf->rcx = elf.entry;  // main
+  p->tf->epc = elf.entry;  // initial program counter = main
-  p->sf->rsp = sp;
+  p->tf->sp = sp; // initial stack pointer
-  switchuvm(p);
+  proc_freepagetable(oldpagetable, oldsz);
-  freevm(oldpgdir, oldsz);
   return 0;
 
  bad:
-  if(pgdir)
+  if(pagetable)
-    freevm(pgdir, sz);
+    proc_freepagetable(pagetable, sz);
   if(ip){
     iunlockput(ip);
     end_op();
   }
   return -1;
 }
+
+// Load a program segment into pagetable at virtual address va.
+// va must be page-aligned
+// and the pages from va to va+sz must already be mapped.
+// Returns 0 on success, -1 on failure.
+static int
+loadseg(pagetable_t pagetable, uint64 va, struct inode *ip, uint offset, uint sz)
+{
+  uint i, n;
+  uint64 pa;
+  pte_t *pte;
+
+  if((va % PGSIZE) != 0)
+    panic("loadseg: va must be page aligned");
+  for(i = 0; i < sz; i += PGSIZE){
+    pa = walkaddr(pagetable, va + i);
+    if(pa == 0)
+      panic("loadseg: address should exist");
+    if(sz - i < PGSIZE)
+      n = sz - i;
+    else
+      n = PGSIZE;
+    if(readi(ip, (char *)pa, offset+i, n) != n)
+      return -1;
+  }
+  return 0;
+}

proc.c

@@ -94,18 +94,7 @@ found:
   }
 
   // An empty user page table.
-  p->pagetable = uvmcreate();
+  p->pagetable = proc_pagetable(p);
-
-  // map the trampoline code (for system call return)
-  // at the highest user virtual address.
-  // only the supervisor uses it, on the way
-  // to/from user space, so not PTE_U.
-  mappages(p->pagetable, TRAMPOLINE, PGSIZE,
-           (uint64)trampstart, PTE_R | PTE_X);
-
-  // map the trapframe, for trampoline.S.
-  mappages(p->pagetable, (TRAMPOLINE - PGSIZE), PGSIZE,
-           (uint64)(p->tf), PTE_R | PTE_W);
 
   // Set up new context to start executing at forkret,
   // which returns to user space.
@@ -116,6 +105,45 @@ found:
   return p;
 }
 
+// Create a page table for a given process,
+// with no users pages, but with trampoline pages.
+// Called both when creating a process, and
+// by exec() when building tentative new memory image,
+// which might fail.
+pagetable_t
+proc_pagetable(struct proc *p)
+{
+  pagetable_t pagetable;
+
+  // An empty user page table.
+  pagetable = uvmcreate();
+
+  // map the trampoline code (for system call return)
+  // at the highest user virtual address.
+  // only the supervisor uses it, on the way
+  // to/from user space, so not PTE_U.
+  mappages(pagetable, TRAMPOLINE, PGSIZE,
+           (uint64)trampstart, PTE_R | PTE_X);
+
+  // map the trapframe, for trampoline.S.
+  mappages(pagetable, (TRAMPOLINE - PGSIZE), PGSIZE,
+           (uint64)(p->tf), PTE_R | PTE_W);
+
+  return pagetable;
+}
+
+// Free a process's page table, and free the
+// physical memory the page table refers to.
+// Called both when a process exits and from
+// exec() if it fails.
+void
+proc_freepagetable(pagetable_t pagetable, uint64 sz)
+{
+  unmappages(pagetable, TRAMPOLINE, PGSIZE, 0);
+  unmappages(pagetable, TRAMPOLINE-PGSIZE, PGSIZE, 0);
+  uvmfree(pagetable, sz);
+}
+
 // a user program that calls exec("/init")
 // od -t xC initcode
 unsigned char initcode[] = {
@@ -295,9 +323,7 @@ wait(void)
         np->kstack = 0;
         kfree((void*)np->tf);
         np->tf = 0;
-        unmappages(np->pagetable, TRAMPOLINE, PGSIZE, 0);
+        proc_freepagetable(np->pagetable, np->sz);
-        unmappages(np->pagetable, TRAMPOLINE-PGSIZE, PGSIZE, 0);
-        uvmfree(np->pagetable, np->sz);
         np->pagetable = 0;
         np->pid = 0;
         np->parent = 0;

syscall.c

@@ -148,24 +148,24 @@ static int (*syscalls[])(void) = {
 [SYS_fork]    sys_fork,
 [SYS_exit]    sys_exit,
 [SYS_wait]    sys_wait,
-//[SYS_pipe]    sys_pipe,
+[SYS_pipe]    sys_pipe,
-//[SYS_read]    sys_read,
+[SYS_read]    sys_read,
 //[SYS_kill]    sys_kill,
-//[SYS_exec]    sys_exec,
+[SYS_exec]    sys_exec,
-//[SYS_fstat]   sys_fstat,
+[SYS_fstat]   sys_fstat,
-//[SYS_chdir]   sys_chdir,
+[SYS_chdir]   sys_chdir,
-//[SYS_dup]     sys_dup,
+[SYS_dup]     sys_dup,
 [SYS_getpid]  sys_getpid,
 //[SYS_sbrk]    sys_sbrk,
 //[SYS_sleep]   sys_sleep,
 //[SYS_uptime]  sys_uptime,
-//[SYS_open]    sys_open,
+[SYS_open]    sys_open,
-//[SYS_write]   sys_write,
+[SYS_write]   sys_write,
-//[SYS_mknod]   sys_mknod,
+[SYS_mknod]   sys_mknod,
-//[SYS_unlink]  sys_unlink,
+[SYS_unlink]  sys_unlink,
-//[SYS_link]    sys_link,
+[SYS_link]    sys_link,
-//[SYS_mkdir]   sys_mkdir,
+[SYS_mkdir]   sys_mkdir,
-//[SYS_close]   sys_close,
+[SYS_close]   sys_close,
 };
 
 static void

sysfile.c

@@ -5,10 +5,10 @@
 //
 
 #include "types.h"
+#include "riscv.h"
 #include "defs.h"
 #include "param.h"
 #include "stat.h"
-#include "mmu.h"
 #include "proc.h"
 #include "fs.h"
 #include "spinlock.h"
@@ -401,22 +401,32 @@ sys_exec(void)
   int i;
   uint64 uargv, uarg;
 
+  printf("sys_exec\n");
+
   if(argstr(0, &path) < 0 || argaddr(1, &uargv) < 0){
+    printf("error 1\n");
     return -1;
   }
   memset(argv, 0, sizeof(argv));
   for(i=0;; i++){
-    if(i >= NELEM(argv))
+    if(i >= NELEM(argv)){
+      printf("error 2\n");
       return -1;
-    if(fetchaddr(uargv+sizeof(uint64)*i, (uint64*)&uarg) < 0)
+    }
+    if(fetchaddr(uargv+sizeof(uint64)*i, (uint64*)&uarg) < 0){
+      printf("error 3\n");
       return -1;
+    }
     if(uarg == 0){
       argv[i] = 0;
       break;
     }
-    if(fetchstr(uarg, &argv[i]) < 0)
+    if(fetchstr(uarg, &argv[i]) < 0){
+      printf("error 4\n");
       return -1;
+    }
   }
+  printf("calling exec\n");
   return exec(path, argv);
 }
 

trap.c

@@ -45,7 +45,6 @@ usertrap(void)
   
   if(r_scause() == 8){
     // system call
-    printf("usertrap(): system call pid=%d syscall=%d\n", p->pid, p->tf->a7);
 
     // sepc points to the ecall instruction,
     // but we want to return to the next instruction.

vm.c

@@ -71,9 +71,9 @@ kvmswitch(void)
 //   12..20 -- 9 bits of level-0 index.
 //    0..12 -- 12 bits of byte offset within the page.
 static pte_t *
-walk(pagetable_t pagetable, const void *va, int alloc)
+walk(pagetable_t pagetable, uint64 va, int alloc)
 {
-  if((uint64)va >= MAXVA)
+  if(va >= MAXVA)
     panic("walk");
 
   for(int level = 2; level > 0; level--) {
@@ -90,17 +90,38 @@ walk(pagetable_t pagetable, const void *va, int alloc)
   return &pagetable[PX(0, va)];
 }
 
+// Look up a virtual address, return the physical address,
+// Can only be used to look up user pages.
+// or 0 if not mapped.
+uint64
+walkaddr(pagetable_t pagetable, uint64 va)
+{
+  pte_t *pte;
+  uint64 pa;
+
+  pte = walk(pagetable, va, 0);
+  if(pte == 0)
+    return 0;
+  if((*pte & PTE_V) == 0)
+    return 0;
+  if((*pte & PTE_U) == 0)
+    return 0;
+  pa = PTE2PA(*pte);
+  return pa;
+}
+
+
 // Create PTEs for virtual addresses starting at va that refer to
 // physical addresses starting at pa. va and size might not
 // be page-aligned.
 void
 mappages(pagetable_t pagetable, uint64 va, uint64 size, uint64 pa, int perm)
 {
-  char *a, *last;
+  uint64 a, last;
   pte_t *pte;
 
-  a = (char*)PGROUNDDOWN(va);
+  a = PGROUNDDOWN(va);
-  last = (char*)PGROUNDDOWN(va + size - 1);
+  last = PGROUNDDOWN(va + size - 1);
   for(;;){
     if((pte = walk(pagetable, a, 1)) == 0)
       panic("mappages: walk");
@@ -120,12 +141,12 @@ mappages(pagetable_t pagetable, uint64 va, uint64 size, uint64 pa, int perm)
 void
 unmappages(pagetable_t pagetable, uint64 va, uint64 size, int do_free)
 {
-  char *a, *last;
+  uint64 a, last;
   pte_t *pte;
   uint64 pa;
 
-  a = (char*)PGROUNDDOWN(va);
+  a = PGROUNDDOWN(va);
-  last = (char*)PGROUNDDOWN(va + size - 1);
+  last = PGROUNDDOWN(va + size - 1);
   for(;;){
     if((pte = walk(pagetable, a, 0)) == 0)
       panic("unmappages: walk");
@@ -173,11 +194,35 @@ uvminit(pagetable_t pagetable, char *src, uint sz)
   memmove(mem, src, sz);
 }
 
+// Allocate PTEs and physical memory to grow process from oldsz to
+// newsz, which need not be page aligned.  Returns new size or 0 on error.
+uint64
+uvmalloc(pagetable_t pagetable, uint64 oldsz, uint64 newsz)
+{
+  char *mem;
+  uint64 a;
+
+  if(newsz < oldsz)
+    return oldsz;
+
+  a = PGROUNDUP(oldsz);
+  for(; a < newsz; a += PGSIZE){
+    mem = kalloc();
+    if(mem == 0){
+      uvmdealloc(pagetable, newsz, oldsz);
+      return 0;
+    }
+    memset(mem, 0, PGSIZE);
+    mappages(pagetable, a, PGSIZE, (uint64)mem, PTE_W|PTE_X|PTE_R);
+  }
+  return newsz;
+}
+
 // Deallocate user pages to bring the process size from oldsz to
 // newsz.  oldsz and newsz need not be page-aligned, nor does newsz
 // need to be less than oldsz.  oldsz can be larger than the actual
 // process size.  Returns the new process size.
-int
+uint64
 uvmdealloc(pagetable_t pagetable, uint64 oldsz, uint64 newsz)
 {
   if(newsz >= oldsz)
@@ -229,7 +274,7 @@ uvmcopy(pagetable_t old, pagetable_t new, uint64 sz)
   char *mem;
 
   for(i = 0; i < sz; i += PGSIZE){
-    if((pte = walk(old, (void *) i, 0)) == 0)
+    if((pte = walk(old, i, 0)) == 0)
       panic("copyuvm: pte should exist");
     if((*pte & PTE_V) == 0)
       panic("copyuvm: page not present");
@@ -241,3 +286,28 @@ uvmcopy(pagetable_t old, pagetable_t new, uint64 sz)
     mappages(new, i, PGSIZE, (uint64)mem, flags);
   }
 }
+
+// Copy len bytes from src to virtual address dstva in a given page table.
+// Most useful when pagetable is not the current page table.
+// Return 0 on success, -1 on error.
+int
+copyout(pagetable_t pagetable, uint64 dstva, char *src, uint64 len)
+{
+  uint64 n, va0, pa0;
+
+  while(len > 0){
+    va0 = (uint)PGROUNDDOWN(dstva);
+    pa0 = walkaddr(pagetable, va0);
+    if(pa0 == 0)
+      return -1;
+    n = PGSIZE - (dstva - va0);
+    if(n > len)
+      n = len;
+    memmove((void *)(pa0 + (dstva - va0)), src, n);
+
+    len -= n;
+    src += n;
+    dstva = va0 + PGSIZE;
+  }
+  return 0;
+}