From 245348e8305c855ec16806c19e3a954d7b5847fe Mon Sep 17 00:00:00 2001
From: TianhuaTao <taotianhua@outlook.com>
Date: Wed, 12 May 2021 00:11:49 +0800
Subject: [PATCH] add exec with args

---
 Makefile                             |   7 +-
 README.md                            |   5 +-
 all.sh                               |   3 +
 doc/ucore-smp-theme.yml              |   8 ++
 doc/ucore-smp.adoc                   |  17 +++++
 os/arch/cpu.c                        |   8 +-
 os/arch/cpu.h                        |   1 +
 os/file/file.c                       |   2 +-
 os/proc/exec.c                       |  64 ++++++++++++++++
 os/proc/exec.h                       |   9 +++
 os/proc/proc.c                       |  20 +----
 os/proc/proc.h                       |  77 +++++++++----------
 os/syscall/syscall.c                 |  13 ++--
 os/syscall/syscall_impl.c            |  38 ++++++++--
 os/syscall/syscall_impl.h            |   2 +-
 os/trap/trap.c                       | 106 ++++++++++++++++++++-------
 os/trap/trap.h                       |  22 +++---
 os/ucore/defs.h                      |   4 +-
 os/utils/log.h                       |   4 +-
 user/include/string.h                |   7 +-
 user/include/unistd.h                |   3 +-
 user/lib/arch/riscv/crt.S            |   1 -
 user/lib/main.c                      |   7 +-
 user/lib/syscall.c                   |   7 +-
 user/src/echo.c                      |  20 +++++
 user/src/exec_simple.c               |   9 ---
 user/src/filetest.c                  |   4 +-
 user/src/getchar.c                   |   1 +
 user/src/getchar_simple.c            |  12 ---
 user/src/shell.c                     |  99 ++++++++++++++++++++-----
 user/src/{exec.c => test_exec.c}     |   0
 user/src/{exit.c => test_exit.c}     |   0
 user/src/test_main.c                 |  15 ++++
 user/src/{pipetest.c => test_pipe.c} |   0
 34 files changed, 427 insertions(+), 168 deletions(-)
 create mode 100755 all.sh
 create mode 100644 doc/ucore-smp-theme.yml
 create mode 100644 doc/ucore-smp.adoc
 create mode 100644 os/proc/exec.c
 create mode 100644 os/proc/exec.h
 create mode 100644 user/src/echo.c
 delete mode 100644 user/src/exec_simple.c
 delete mode 100644 user/src/getchar_simple.c
 rename user/src/{exec.c => test_exec.c} (100%)
 rename user/src/{exit.c => test_exit.c} (100%)
 create mode 100644 user/src/test_main.c
 rename user/src/{pipetest.c => test_pipe.c} (100%)

diff --git a/Makefile b/Makefile
index 4a890b0..ef29e73 100644
--- a/Makefile
+++ b/Makefile
@@ -1,4 +1,4 @@
-.PHONY: clean build_kernel all
+.PHONY: clean build_kernel all user
 all: build_kernel
 
 U = user
@@ -113,4 +113,7 @@ QEMUGDB = $(shell if $(QEMU) -help | grep -q '^-gdb'; \
 debug: build/kernel .gdbinit
 	$(QEMU) $(QEMUOPTS) -S $(QEMUGDB) &
 	sleep 1
-	$(GDB)
\ No newline at end of file
+	$(GDB)
+
+user:
+	make -C user
diff --git a/README.md b/README.md
index 7232095..b67d608 100644
--- a/README.md
+++ b/README.md
@@ -10,13 +10,10 @@ git clone https://github.com/TianhuaTao/uCore-SMP.git
 cd uCore-SMP
 
 # make user programs (e.g. shell)
-cd user
-make
+make user
 
 # make kernel
-cd ../
 make
-```
 
 Run with QEMU:
 
diff --git a/all.sh b/all.sh
new file mode 100755
index 0000000..6e8ae07
--- /dev/null
+++ b/all.sh
@@ -0,0 +1,3 @@
+make user
+make clean
+make run
diff --git a/doc/ucore-smp-theme.yml b/doc/ucore-smp-theme.yml
new file mode 100644
index 0000000..ce00e22
--- /dev/null
+++ b/doc/ucore-smp-theme.yml
@@ -0,0 +1,8 @@
+extends: default
+image:
+  align: center
+caption:
+  align: center
+table:
+  caption:
+    side: bottom
diff --git a/doc/ucore-smp.adoc b/doc/ucore-smp.adoc
new file mode 100644
index 0000000..3fc563e
--- /dev/null
+++ b/doc/ucore-smp.adoc
@@ -0,0 +1,17 @@
+= uCore-SMP Documentation and Specification
+:author: Tianhua Tao
+:email: taotianhua@outlook.com
+:revnumber: 0.0.1
+:sectnums:
+:xrefstyle: short
+:toc: macro
+
+// table of contents
+toc::[]
+
+== Introduction
+
+
+
+== Syscall
+
diff --git a/os/arch/cpu.c b/os/arch/cpu.c
index 56c7918..e35d48a 100644
--- a/os/arch/cpu.c
+++ b/os/arch/cpu.c
@@ -19,6 +19,8 @@ int cpuid() {
     return id;
 }
 
+// Barrier
+// Will not return until all cores halted
 void wait_all_halt() {
     for (int i = 0; i < NCPU; i++) {
         while (!halted[i])
@@ -26,8 +28,8 @@ void wait_all_halt() {
     }
 }
 
-volatile int booted[NCPU];
-volatile int halted[NCPU];
+volatile int booted[NCPU];  // whether a core is booted
+volatile int halted[NCPU];  // whether a core is halted
 
 /**
  * set halted of this core to True
@@ -41,7 +43,7 @@ void init_cpu() {
     for (int i = 0; i < NCPU; i++) {
         cpus[i].proc = NULL;
         cpus[i].noff = 0;
-        cpus[i].base_interrupt_status = 0;
+        cpus[i].base_interrupt_status = FALSE;
         cpus[i].core_id = i;
     }
 }
diff --git a/os/arch/cpu.h b/os/arch/cpu.h
index ce21e59..ef353b4 100644
--- a/os/arch/cpu.h
+++ b/os/arch/cpu.h
@@ -41,6 +41,7 @@ struct cpu
   int core_id;
 };
 
+// debug print
 void print_cpu(struct cpu *c);
 extern struct cpu cpus[NCPU];
 
diff --git a/os/file/file.c b/os/file/file.c
index 31b0da6..eb11814 100644
--- a/os/file/file.c
+++ b/os/file/file.c
@@ -6,7 +6,7 @@
 #include <ucore/types.h>
 
 struct {
-    struct file files[FILE_MAX];
+    struct file files[FILE_MAX];    // system level files
     struct spinlock lock;
 } filepool;
 
diff --git a/os/proc/exec.c b/os/proc/exec.c
new file mode 100644
index 0000000..f579543
--- /dev/null
+++ b/os/proc/exec.c
@@ -0,0 +1,64 @@
+#include <proc/proc.h>
+#include <trap/trap.h>
+#include <utils/log.h>
+
+static void debug_print_args(char *name, int argc, const char **argv) {
+    tracecore("exec name=%s, argc=%d", name, argc);
+    for (int i = 0; i < argc; i++) {
+        tracecore("argv[%d]=%s", i, argv[i]);
+    }
+}
+
+int exec(char *name, int argc, const char **argv) {
+    debug_print_args(name, argc, argv);
+
+    int id = get_id_by_name(name);
+    if (id < 0)
+        return -1;
+    struct proc *p = curr_proc();
+    proc_free_mem_and_pagetable(p->pagetable, p->total_size);
+    p->total_size = 0;
+    p->pagetable = proc_pagetable(p);
+    if (p->pagetable == 0) {
+        panic("");
+    }
+    loader(id, p);
+
+    // push args
+    char *sp = (char *)p->trapframe->sp;
+    phex(sp);
+
+    // sp itself is on the boundary hence not mapped, but sp-1 is a valid address.
+    // we can calculate the physical address of sp
+    // but can NOT access sp_pa
+    char *sp_pa = (char *)(virt_addr_to_physical(p->pagetable, (uint64)sp - 1) + 1);
+
+    phex(sp_pa);
+
+    char *sp_pa_bottom = sp_pa; // keep a record
+
+    // the argv array (content of argv[i]) will be stored on ustack, at the very bottom
+    // and the real string value of argv array (content of *argv[i]) will be stored after that
+    sp_pa -= argc * sizeof(const char *);           // alloc space for argv
+    const char **argv_start = (const char **)sp_pa; // user main()'s argv value (physical here)
+    phex(sp_pa);
+    for (int i = 0; i < argc; i++) {
+        int arg_len = strlen(argv[i]);
+        sp_pa -= arg_len + 1; // alloc space for argv[i] string, including a null
+        strncpy(sp_pa, argv[i], arg_len);
+        sp_pa[arg_len] = '\0';                       // make sure null is there
+        argv_start[i] = (sp_pa - sp_pa_bottom) + sp; // point argv[i] to here, use user space
+    }
+
+    p->trapframe->sp += sp_pa - sp_pa_bottom; // update user sp
+    p->trapframe->a0 = (uint64)argc;
+
+    int64 offset = (uint64)argv_start - (uint64)sp_pa_bottom;   // offset of argv in user stack
+    p->trapframe->a1 = (uint64)(sp + offset);
+
+    tracecore("trapframe->sp=%p", p->trapframe->sp);
+    tracecore("trapframe->a0=%p", p->trapframe->a0);
+    tracecore("trapframe->a1=%p", p->trapframe->a1);
+    
+    return 0;
+}
diff --git a/os/proc/exec.h b/os/proc/exec.h
new file mode 100644
index 0000000..62708c1
--- /dev/null
+++ b/os/proc/exec.h
@@ -0,0 +1,9 @@
+#if !defined(EXEC_H)
+#define EXEC_H
+
+#define MAX_EXEC_ARG_COUNT 8
+#define MAX_EXEC_ARG_LENGTH 32
+
+int exec(char *name, int argc, const char **argv);
+
+#endif // EXEC_H
diff --git a/os/proc/proc.c b/os/proc/proc.c
index 1a273f2..17039b1 100644
--- a/os/proc/proc.c
+++ b/os/proc/proc.c
@@ -188,7 +188,7 @@ void sched(void) {
 
     base_interrupt_status = mycpu()->base_interrupt_status;
     // debugcore("in sched before swtch base_interrupt_status=%d", base_interrupt_status);
-    swtch(&p->context, &mycpu()->context);
+    swtch(&p->context, &mycpu()->context); // will goto scheduler()
     // debugcore("in sched after swtch");
     mycpu()->base_interrupt_status = base_interrupt_status;
 }
@@ -273,20 +273,7 @@ int fork() {
     return pid;
 }
 
-int exec(char *name) {
-    int id = get_id_by_name(name);
-    if (id < 0)
-        return -1;
-    struct proc *p = curr_proc();
-    proc_free_mem_and_pagetable(p->pagetable, p->total_size);
-    p->total_size = 0;
-    p->pagetable = proc_pagetable(p);
-    if (p->pagetable == 0) {
-        panic("");
-    }
-    loader(id, p);
-    return 0;
-}
+
 
 int spawn(char *filename) {
     int pid;
@@ -407,7 +394,8 @@ void exit(int code) {
     if (p->parent != NULL) {
         p->state = ZOMBIE;
     }else{
-        // TODO: ???
+        p->state = UNUSED;
+        freeproc(p);
     }
     infof("proc %d exit with %d\n", p->pid, code);
     sched();
diff --git a/os/proc/proc.h b/os/proc/proc.h
index 3c8bacc..1e4e709 100644
--- a/os/proc/proc.h
+++ b/os/proc/proc.h
@@ -3,60 +3,63 @@
 
 #include <ucore/ucore.h>
 
-#include <lock/lock.h>
+#include "exec.h"
 #include <file/file.h>
+#include <lock/lock.h>
 #define NPROC (256)
 #define KSTACK_SIZE (4096)
 #define USTACK_SIZE (4096)
 #define TRAPFRAME_SIZE (4096)
 #define FD_MAX (16)
-enum procstate
-{
-  UNUSED=0,
-  USED,
-  SLEEPING,
-  RUNNABLE,
-  RUNNING,
-  ZOMBIE
+enum procstate {
+    UNUSED = 0,
+    USED,
+    SLEEPING,
+    RUNNABLE,
+    RUNNING,
+    ZOMBIE
 };
 
 // Per-process state
-struct proc
-{
-  struct spinlock lock;
-
-  // p->lock must be held when using these:
-  enum procstate state; // Process state
-  int pid;              // Process ID
-  pagetable_t pagetable;       // User page table
-
-  uint64 ustack_bottom;                // Virtual address of user stack
-  uint64 kstack;               // Virtual address of kernel stack
-  struct trapframe *trapframe; // data page for trampoline.S, physical address
-  struct context context;      // swtch() here to run process
-  uint64 total_size;  // total memory used by this process
-  uint64 heap_sz;
-  struct proc *parent;         // Parent process
-  uint64 exit_code;
-  // uint64 entry;                // app bin start address
-  uint64 stride;
-  uint64 priority;
-  uint64 cpu_time;        // ms, user and kernel
-  uint64 last_start_time; // ms
-  void *waiting_target;
-  struct file *files[16];
-  struct mailbox mb;
+struct proc {
+    struct spinlock lock;
+
+    // p->lock must be held when using these:
+    enum procstate state;  // Process state
+    int pid;               // Process ID
+    pagetable_t pagetable; // User page table
+
+    uint64 ustack_bottom;        // Virtual address of user stack
+    uint64 kstack;               // Virtual address of kernel stack
+    struct trapframe *trapframe; // data page for trampoline.S, physical address
+    struct context context;      // swtch() here to run process
+    uint64 total_size;           // total memory used by this process
+    uint64 heap_sz;
+    struct proc *parent; // Parent process
+    uint64 exit_code;
+    // uint64 entry;                // app bin start address
+    uint64 stride;
+    uint64 priority;
+    uint64 cpu_time;        // ms, user and kernel
+    uint64 last_start_time; // ms
+    void *waiting_target;
+    struct file *files[16];
+    struct mailbox mb;
 };
-struct proc* findproc(int pid);
+struct proc *findproc(int pid);
 
 struct proc *curr_proc();
-int spawn(char* filename);
+int spawn(char *filename);
 extern struct proc pool[NPROC];
 extern struct spinlock pool_lock;
 
 void sleep(void *waiting_target, struct spinlock *lk);
 void wakeup(void *waiting_target);
 
-void print_proc(struct proc*proc);
+void print_proc(struct proc *proc);
 void forkret(void);
+
+void proc_free_mem_and_pagetable(pagetable_t pagetable, uint64 sz);
+struct proc *allocproc(void);
+pagetable_t proc_pagetable(struct proc *p);
 #endif // PROC_H
diff --git a/os/syscall/syscall.c b/os/syscall/syscall.c
index 23c426c..216917c 100644
--- a/os/syscall/syscall.c
+++ b/os/syscall/syscall.c
@@ -11,16 +11,17 @@
 #include <file/file.h>
 #include "syscall_impl.h"
 
-
+// dispatch syscalls to different functions
 void syscall()
 {
     struct proc *p = curr_proc();
     struct trapframe *trapframe = p->trapframe;
     int id = trapframe->a7, ret;
     uint64 args[7] = {trapframe->a0, trapframe->a1, trapframe->a2, trapframe->a3, trapframe->a4, trapframe->a5, trapframe->a6};
+
+    // ignore read and write so that shell command don't get interrupted
     if (id != SYS_write && id != SYS_read)
     {
-        // debugcore("syscall %d args:%p %p %p %p %p %p %p", id, args[0], args[1], args[2], args[3], args[4], args[5], args[6]);
         tracecore("syscall %d args:%p %p %p %p %p %p %p", id, args[0], args[1], args[2], args[3], args[4], args[5], args[6]);
     }
     switch (id)
@@ -48,7 +49,8 @@ void syscall()
         break;
     case SYS_getpriority:
         ret = -1;
-        warnf("not implemented\n");
+        // TODO: implement this
+        warnf("SYS_getpriority not implemented\n");
         break;
     case SYS_getpid:
         ret = sys_getpid();
@@ -66,7 +68,7 @@ void syscall()
         ret = sys_munmap((void *)args[0], args[1]);
         break;
     case SYS_execve:
-        ret = sys_exec(args[0]);
+        ret = sys_exec(args[0], (const char **)args[1]);
         break;
     case SYS_wait4:
         ret = sys_wait(args[0], args[1]);
@@ -90,10 +92,9 @@ void syscall()
         ret = -1;
         warnf("unknown syscall %d\n", id);
     }
-    trapframe->a0 = ret;
+    trapframe->a0 = ret;    // return value
     if (id != SYS_write && id != SYS_read)
     {
-        debugcore("syscall %d ret %d\n", id, ret);
         tracecore("syscall %d ret %d\n", id, ret);
     }
 }
\ No newline at end of file
diff --git a/os/syscall/syscall_impl.c b/os/syscall/syscall_impl.c
index 855213c..3f76899 100644
--- a/os/syscall/syscall_impl.c
+++ b/os/syscall/syscall_impl.c
@@ -97,12 +97,40 @@ uint64 sys_clone() {
     return fork();
 }
 
-uint64 sys_exec(uint64 va) {
+uint64 sys_exec(uint64 va, const char** argv_va) {
     struct proc *p = curr_proc();
     char name[200];
+    char argv_str[MAX_EXEC_ARG_COUNT][MAX_EXEC_ARG_LENGTH];
     copyinstr(p->pagetable, name, va, 200);
-    infof("sys_exec %s\n", name);
-    return exec(name);
+    infof("sys_exec %s", name);
+
+    int argc = 0;
+    const char *argv[MAX_EXEC_ARG_COUNT];
+    // tracecore("argv_va=%d argc=%d", argv_va, argc);
+    if (argv_va == NULL) {
+        // nothing
+    } else {
+        const char **argv_pa = (const char **)virt_addr_to_physical(p->pagetable, (uint64)argv_va);
+        if(argv_pa == NULL){
+            // invalid argv_va
+            // TODO: kill
+            return -1;
+        }
+        while(*argv_pa){
+            const char *argv_one_va = *argv_pa;
+
+            if (copyinstr(p->pagetable, argv_str[argc], (uint64)argv_one_va, MAX_EXEC_ARG_LENGTH) < 0) {
+                // TODO: failed, exit
+                return -1;
+            }
+            argv[argc] = argv_str[argc];
+            argc++;
+            argv_pa++;
+        }
+    }
+    tracecore("argv_va=%d argc=%d", argv_va, argc);
+
+    return exec(name, argc, argv);
 }
 
 uint64 sys_wait(int pid, uint64 va) {
@@ -135,7 +163,7 @@ int64 sys_gettimeofday(uint64 *timeval, int tz) {
     return 0;
 }
 uint64 sys_close(int fd) {
-    if (fd == 0 || fd == 1 || fd == 2)
+    if (fd == STDIN || fd == STDOUT || fd == STDERR)
         return 0;
     struct proc *p = curr_proc();
     struct file *f = p->files[fd];
@@ -144,7 +172,7 @@ uint64 sys_close(int fd) {
     //     panic("fileclose: unknown file type\n");
     // }
     fileclose(f);
-    p->files[fd] = 0;
+    p->files[fd] = NULL;
     return 0;
 }
 
diff --git a/os/syscall/syscall_impl.h b/os/syscall/syscall_impl.h
index fcaf7e9..b93e686 100644
--- a/os/syscall/syscall_impl.h
+++ b/os/syscall/syscall_impl.h
@@ -9,7 +9,7 @@ uint64 sys_exit(int code);
 uint64 sys_sched_yield();
 uint64 sys_getpid();
 uint64 sys_clone();
-uint64 sys_exec(uint64 va);
+uint64 sys_exec(uint64 va, const char **argv);
 uint64 sys_wait(int pid, uint64 va);
 uint64 sys_times();
 int64 sys_setpriority(int64 priority);
diff --git a/os/trap/trap.c b/os/trap/trap.c
index 7b438ab..612e257 100644
--- a/os/trap/trap.c
+++ b/os/trap/trap.c
@@ -16,10 +16,7 @@ void trapinit_hart() {
 void trapinit() {
 }
 
-void unknown_trap() {
-    printf("unknown trap: %p, stval = %p\n", r_scause(), r_stval());
-    exit(-1);
-}
+
 
 // set up to take exceptions and traps while in the kernel.
 void set_usertrap(void) {
@@ -62,9 +59,9 @@ void kernel_interrupt_handler(uint64 cause) {
     }
 }
 
-void user_interrupt_handler(uint64 cause) {
+void user_interrupt_handler(uint64 scause, uint64 stval, uint64 sepc) {
     int irq;
-    switch (cause) {
+    switch (scause& 0xff) {
     case SupervisorTimer:
         set_next_timer();
         // if form user, allow yield
@@ -88,40 +85,50 @@ void user_interrupt_handler(uint64 cause) {
         }
         break;
     default:
-        unknown_trap();
+        infof("Unknown interrupt in user application: %p, stval = %p sepc = %p\n", scause, stval, sepc);
+        exit(-1);
         break;
     }
 }
 void user_exception_handler(uint64 scause, uint64 stval, uint64 sepc) {
-    struct trapframe *trapframe;
-    switch (scause) {
+    struct trapframe *trapframe = curr_proc()->trapframe;
+    switch (scause & 0xff) {
     case UserEnvCall:
-        trapframe = curr_proc()->trapframe;
-
         trapframe->epc += 4;
         intr_on();
         syscall();
         break;
-    case StoreFault:
+    case StoreAccessFault:
+        infof("StoreAccessFault in user application: %p, stval = %p sepc = %p\n", scause, stval, sepc);
+        exit(-8);
+        break;
     case StorePageFault:
-    case InstructionFault:
+        infof("StorePageFault in user application: %p, stval = %p sepc = %p\n", scause, stval, sepc);
+        exit(-7);
+        break;
+    case InstructionAccessFault:
+        infof("InstructionAccessFault in user application: %p, stval = %p sepc = %p\n", scause, stval, sepc);
+        exit(-6);
+        break;
     case InstructionPageFault:
-    case LoadFault:
+        infof("InstructionPageFault in user application: %p, stval = %p sepc = %p\n", scause, stval, sepc);
+        exit(-5);
+        break;
+    case LoadAccessFault:
+        infof("LoadAccessFault in user application: %p, stval = %p sepc = %p\n", scause, stval, sepc);
+        exit(-4);
+        break;
     case LoadPageFault:
-        trapframe = curr_proc()->trapframe;
-        infof(
-            "scause = %d in application, bad addr = %p, bad instruction = %p, core dumped.\n",
-            scause,
-            r_stval(),
-            trapframe->epc);
+        infof("LoadPageFault in user application: %p, stval = %p sepc = %p\n", scause, stval, sepc);
         exit(-2);
         break;
     case IllegalInstruction:
-        infof("IllegalInstruction in application, core dumped.\n");
+        errorf("IllegalInstruction in user application: %p, stval = %p sepc = %p\n", scause, stval, sepc);
         exit(-3);
         break;
     default:
-        unknown_trap();
+        errorf("Unknown exception in user application: %p, stval = %p sepc = %p\n", scause, stval, sepc);
+        exit(-1);
         break;
     }
 }
@@ -137,7 +144,7 @@ void usertrap() {
     uint64 stval = r_stval();
 
     KERNEL_ASSERT(!intr_get(), "");
-    debugcore("Enter user trap handler scause=%p", scause);
+    // debugcore("Enter user trap handler scause=%p", scause);
 
     w_stvec((uint64)kernelvec & ~0x3); // DIRECT
     // debugcore("usertrap");
@@ -146,7 +153,7 @@ void usertrap() {
     KERNEL_ASSERT((sstatus & SSTATUS_SPP) == 0, "usertrap: not from user mode");
 
     if (scause & (1ULL << 63)) { // interrput = 1
-        user_interrupt_handler(scause & 0xff);
+        user_interrupt_handler(scause , stval, sepc);
     } else { // interrput = 0
         user_exception_handler(scause, stval, sepc);
     }
@@ -157,7 +164,7 @@ void usertrap() {
 // return to user space
 //
 void usertrapret() {
-    debugcore("About to return to user mode");
+    // debugcore("About to return to user mode");
 
     // print_cpu(mycpu());
     // we're about to switch the destination of traps from
@@ -194,7 +201,50 @@ void usertrapret() {
 }
 
 void kernel_exception_handler(uint64 scause, uint64 stval, uint64 sepc) {
-    errorf("invalid trap from kernel: %p, stval = %p sepc = %p\n", scause, stval, sepc);
+    switch (scause & 0xff) {
+    case InstructionMisaligned:
+        errorf("InstructionMisaligned in kernel: %p, stval = %p sepc = %p\n", scause, stval, sepc);
+        break;
+    case InstructionAccessFault:
+        errorf("SupervisorEnvCall in kernel: %p, stval = %p sepc = %p\n", scause, stval, sepc);
+        break;
+    case IllegalInstruction:
+        errorf("IllegalInstruction in kernel: %p, stval = %p sepc = %p\n", scause, stval, sepc);
+        break;
+    case Breakpoint:
+        errorf("Breakpoint in kernel: %p, stval = %p sepc = %p\n", scause, stval, sepc);
+        break;
+    case LoadMisaligned:
+        errorf("LoadMisaligned in kernel: %p, stval = %p sepc = %p\n", scause, stval, sepc);
+        break;
+    case LoadAccessFault:
+        errorf("LoadAccessFault in kernel: %p, stval = %p sepc = %p\n", scause, stval, sepc);
+        break;
+    case StoreMisaligned:
+        errorf("StoreMisaligned in kernel: %p, stval = %p sepc = %p\n", scause, stval, sepc);
+        break;
+    case StoreAccessFault:
+        errorf("StoreAccessFault in kernel: %p, stval = %p sepc = %p\n", scause, stval, sepc);
+        break;
+    case SupervisorEnvCall:
+        errorf("SupervisorEnvCall in kernel: %p, stval = %p sepc = %p\n", scause, stval, sepc);
+        break;
+    case MachineEnvCall:
+        errorf("MachineEnvCall in kernel: %p, stval = %p sepc = %p\n", scause, stval, sepc);
+        break;
+    case InstructionPageFault:
+        errorf("InstructionPageFault in kernel: %p, stval = %p sepc = %p\n", scause, stval, sepc);
+        break;
+    case LoadPageFault:
+        errorf("LoadPageFault in kernel: %p, stval = %p sepc = %p\n", scause, stval, sepc);
+        break;
+    case StorePageFault:
+        errorf("StorePageFault in kernel: %p, stval = %p sepc = %p\n", scause, stval, sepc);
+        break;
+    default:
+        errorf("Unknown exception in kernel: %p, stval = %p sepc = %p\n", scause, stval, sepc);
+        break;
+    }
     panic("kernel exception");
 }
 
@@ -208,7 +258,7 @@ void kerneltrap() {
 
     KERNEL_ASSERT(!intr_get(), "Interrupt can not be turned on in trap handler");
     KERNEL_ASSERT((sstatus & SSTATUS_SPP) != 0, "kerneltrap: not from supervisor mode");
-    debugcore("Enter kernel trap handler, scause=%p", scause);
+    // debugcore("Enter kernel trap handler, scause=%p", scause);
 
     if (scause & (1ULL << 63)) // interrput
     {
@@ -222,7 +272,7 @@ void kerneltrap() {
     // so restore trap registers for use by kernelvec.S's sepc instruction.
     w_sepc(sepc);
     w_sstatus(sstatus);
-    debugcore("About to return from kernel trap handler");
+    // debugcore("About to return from kernel trap handler");
 
     // go back to kernelvec.S
 }
diff --git a/os/trap/trap.h b/os/trap/trap.h
index 17f6f47..3608f65 100644
--- a/os/trap/trap.h
+++ b/os/trap/trap.h
@@ -55,18 +55,18 @@ struct trapframe {
 
 enum {
     InstructionMisaligned = 0,
-    InstructionFault,
-    IllegalInstruction,
-    Breakpoint,
-    LoadMisaligned,
-    LoadFault,
-    StoreMisaligned,
-    StoreFault,
-    UserEnvCall,
-    SupervisorEnvCall,
+    InstructionAccessFault=1,
+    IllegalInstruction=2,
+    Breakpoint=3,
+    LoadMisaligned=4,
+    LoadAccessFault=5,
+    StoreMisaligned=6,
+    StoreAccessFault=7,
+    UserEnvCall=8,
+    SupervisorEnvCall=9,
     MachineEnvCall = 11,
-    InstructionPageFault,
-    LoadPageFault,
+    InstructionPageFault=12,
+    LoadPageFault=13,
     StorePageFault = 15,
 };
 
diff --git a/os/ucore/defs.h b/os/ucore/defs.h
index 78cb951..b557a40 100644
--- a/os/ucore/defs.h
+++ b/os/ucore/defs.h
@@ -64,8 +64,8 @@ void scheduler(); // __attribute__((noreturn));
 void sched();
 void yield();
 int fork(void);
-int exec(char*);
-int wait(int, int*);
+int exec(char *name, int argc, const char **argv);
+int wait(int, int *);
 struct proc* allocproc();
 void init_scheduler();
 int fdalloc(struct file *);
diff --git a/os/utils/log.h b/os/utils/log.h
index 5b86c57..d9403a2 100644
--- a/os/utils/log.h
+++ b/os/utils/log.h
@@ -9,8 +9,8 @@ void printf(char *, ...);
 // #define LOG_LEVEL_CRITICAL
 // #define LOG_LEVEL_DEBUG
 // #define LOG_LEVEL_INFO
-#define LOG_LEVEL_TRACE
-// #define LOG_LEVEL_ALL
+// #define LOG_LEVEL_TRACE
+#define LOG_LEVEL_ALL
 
 #if defined(LOG_LEVEL_CRITICAL)
 
diff --git a/user/include/string.h b/user/include/string.h
index 1113812..c778c75 100644
--- a/user/include/string.h
+++ b/user/include/string.h
@@ -8,10 +8,11 @@ int isdigit(int c);
 int atoi(const char *s);
 void *memset(void *dest, int c, size_t n);
 int strcmp(const char *l, const char *r);
-size_t strlen(const char*);
-size_t strnlen(const char *s, size_t n);
-char* strncpy(char *restrict d, const char *restrict s, size_t n);
 int strncmp(const char *_l, const char *_r, size_t n);
+size_t strlen(const char *);
+size_t strnlen(const char *s, size_t n);
+char* stpncpy(char *restrict d, const char *restrict s, size_t n);
+int stpncmp(const char *_l, const char *_r, size_t n);
 
 #endif // __STRING_H__
 
diff --git a/user/include/unistd.h b/user/include/unistd.h
index 65b73cd..7a0a29a 100644
--- a/user/include/unistd.h
+++ b/user/include/unistd.h
@@ -14,7 +14,8 @@ int sched_yield(void);
 void exit(int);
 int fork(void);
 int exec(char*);
-int wait(int, int*);
+int execv(const char *pathname, char *const argv[]);
+int wait(int, int *);
 int pipe(void*);
 uint64 get_time();
 int sleep(unsigned long long);
diff --git a/user/lib/arch/riscv/crt.S b/user/lib/arch/riscv/crt.S
index 67d9a5a..85cb18a 100644
--- a/user/lib/arch/riscv/crt.S
+++ b/user/lib/arch/riscv/crt.S
@@ -1,5 +1,4 @@
 .text
 .globl _start
 _start:
-    mv a0, sp
     tail __start_main
diff --git a/user/lib/main.c b/user/lib/main.c
index 9be7b45..4735de4 100644
--- a/user/lib/main.c
+++ b/user/lib/main.c
@@ -1,9 +1,8 @@
 #include <unistd.h>
 
-extern int main();
+extern int main(int argc, char *argv[]);
 
-int __start_main(long* p)
+void __start_main(int argc, char* argv[])
 {
-    exit(main());
-    return 0;
+    exit(main(argc, argv));
 }
diff --git a/user/lib/syscall.c b/user/lib/syscall.c
index d562224..87633d1 100644
--- a/user/lib/syscall.c
+++ b/user/lib/syscall.c
@@ -40,7 +40,12 @@ int wait(int pid, int* code) {
 }
 
 int exec(char* name) {
-    return syscall(SYS_execve, name);
+    return syscall(SYS_execve, name, 0);
+}
+
+int execv(const char *pathname, char *const argv[])
+{
+    return syscall(SYS_execve, pathname, argv);
 }
 
 uint64 get_time() {
diff --git a/user/src/echo.c b/user/src/echo.c
new file mode 100644
index 0000000..2107d4e
--- /dev/null
+++ b/user/src/echo.c
@@ -0,0 +1,20 @@
+
+#include <stddef.h>
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+int
+main(int argc, char *argv[])
+{
+  int i;
+
+  for(i = 1; i < argc; i++){
+    write(1, argv[i], strlen(argv[i]));
+    if(i + 1 < argc){
+      write(1, " ", 1);
+    } else {
+      write(1, "\n", 1);
+    }
+  }
+  exit(0);
+}
diff --git a/user/src/exec_simple.c b/user/src/exec_simple.c
deleted file mode 100644
index 3e70065..0000000
--- a/user/src/exec_simple.c
+++ /dev/null
@@ -1,9 +0,0 @@
-#include <stdio.h>
-#include <stdlib.h>
-#include <unistd.h>
-
-int main() {
-    puts("execute hello");
-    exec("hello");
-    return 0;
-}
\ No newline at end of file
diff --git a/user/src/filetest.c b/user/src/filetest.c
index fdc7ee9..e1eb64b 100644
--- a/user/src/filetest.c
+++ b/user/src/filetest.c
@@ -6,7 +6,7 @@
 
 int main() {
     int exit_code;
-    int fd = open("test\0", O_CREATE | O_WRONLY, O_RDWR);
+    int fd = open("test", O_CREATE | O_WRONLY, O_RDWR);
     printf("open OK, fd = %d\n", fd);
     char str[100] = "hello world!\0";
     int len = strlen(str);
@@ -14,7 +14,7 @@ int main() {
     close(fd);
     puts("write over.");
     if(fork() == 0) {
-        int fd = open("test\0", O_RDONLY, O_RDWR);
+        int fd = open("test", O_RDONLY, O_RDWR);
         char str[100];
         str[read(fd, str, len)] = 0;
         puts(str);
diff --git a/user/src/getchar.c b/user/src/getchar.c
index 2ece5c6..d38849d 100644
--- a/user/src/getchar.c
+++ b/user/src/getchar.c
@@ -8,5 +8,6 @@ int main() {
         int c = getchar();
         putchar(c);
     }
+    putchar('\n');
     return 0;
 }
\ No newline at end of file
diff --git a/user/src/getchar_simple.c b/user/src/getchar_simple.c
deleted file mode 100644
index 2ece5c6..0000000
--- a/user/src/getchar_simple.c
+++ /dev/null
@@ -1,12 +0,0 @@
-#include <stdio.h>
-#include <stdlib.h>
-#include <unistd.h>
-
-int main() {
-    printf("getchar 10:");
-    for(int i = 0; i < 10; ++i) {
-        int c = getchar();
-        putchar(c);
-    }
-    return 0;
-}
\ No newline at end of file
diff --git a/user/src/shell.c b/user/src/shell.c
index 6cbd719..740c307 100644
--- a/user/src/shell.c
+++ b/user/src/shell.c
@@ -1,14 +1,14 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <unistd.h>
-
+#include <string.h>
 
 const unsigned char LF = 0x0a;
 const unsigned char CR = 0x0d;
 const unsigned char DL = 0x7f;
 const unsigned char BS = 0x08;
 
-char line[100] = {};
+char line[128] = {};
 
 int top = 0;
 
@@ -28,6 +28,84 @@ void clear() {
     top = 0;
 }
 
+void print_help(){
+    printf(
+        "C Shell Help\n"
+        "exit: Exit shell and shutdown.\n"
+        "help: Print this help message.\n"
+    );
+}
+
+char args_buffer[1024];
+char *argv[10]; 
+char *args_buffer_cur;
+void parse_line(){
+    args_buffer_cur = args_buffer;
+    int argc = 0;
+    char *start;
+    char *end;
+
+    start = line;
+    end = line;
+
+    while (*end)
+    {
+        while (*start == ' ') {
+            start++;
+        }
+
+        end = start;
+        while (*end != ' ' && *end != '\0') {
+            end++;
+        }
+
+        int arg_len = end - start;
+        if(arg_len==0)
+            break;
+        // printf("arg_len %d\n", arg_len);
+        argv[argc] = args_buffer_cur;
+        stpncpy(args_buffer_cur, start, arg_len);
+        args_buffer_cur += arg_len;
+        *args_buffer_cur = 0;
+        args_buffer_cur++;
+        argc++;
+        start = end;
+    }
+    argv[argc] = 0; // NULL terminate
+    printf("argc %d\n", argc);
+    for (int i = 0; i < argc; i++)
+    {
+        printf("argv[%d]=\"%s\"\n", i, argv[i]);
+    }
+    
+    if (argc == 0){
+        return;
+    }
+
+    if (strcmp(argv[0], "exit") == 0) {
+        exit(0);
+    } else if (strcmp(argv[0], "help") == 0) {
+        print_help();
+    } else {
+        int pid = fork();
+        if (pid == 0) {
+            // child process
+            if (execv(argv[0], argv) < 0) {
+                printf("Shell: %s: No such file\n", argv[0]);
+                exit(0);
+            }
+            panic("unreachable!");
+        } else {
+            int xstate = 0;
+            int exit_pid = 0;
+            exit_pid = wait(pid, &xstate);
+            assert(pid == exit_pid, -1);
+            printf("Shell: Process %d exited with code %d\n", pid, xstate);
+        }
+    }
+}
+
+
 int main() {
     printf("C user shell\n");
     printf(">> ");
@@ -39,21 +117,8 @@ int main() {
                 printf("\n");
                 if (!is_empty()) {
                     push('\0');
-                    int pid = fork();
-                    if (pid == 0) {
-                        // child process
-                        if (exec(line) < 0) {
-                            printf("Shell: %s: No such file\n", line);
-                            exit(0);
-                        }
-                        panic("unreachable!");
-                    } else {
-                        int xstate = 0;
-                        int exit_pid = 0;
-                        exit_pid = wait(pid, &xstate);
-                        assert(pid == exit_pid, -1);
-                        printf("Shell: Process %d exited with code %d\n", pid, xstate);
-                    }
+                    parse_line();
+
                     clear();
                 }
                 printf(">> ");
diff --git a/user/src/exec.c b/user/src/test_exec.c
similarity index 100%
rename from user/src/exec.c
rename to user/src/test_exec.c
diff --git a/user/src/exit.c b/user/src/test_exit.c
similarity index 100%
rename from user/src/exit.c
rename to user/src/test_exit.c
diff --git a/user/src/test_main.c b/user/src/test_main.c
new file mode 100644
index 0000000..47d372e
--- /dev/null
+++ b/user/src/test_main.c
@@ -0,0 +1,15 @@
+
+#include <stddef.h>
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+int main(int argc, char *argv[]) {
+    printf("argc=%d\n", argc);
+
+    for (int i = 0; i < argc; i++)
+    {
+        printf("argv[%d]=%s\n", i, argv[i]);
+    }
+
+    return 0;
+}
diff --git a/user/src/pipetest.c b/user/src/test_pipe.c
similarity index 100%
rename from user/src/pipetest.c
rename to user/src/test_pipe.c
-- 
GitLab