From 56583b1402a7f8fad0f8c3c296e26f12b1114c95 Mon Sep 17 00:00:00 2001
From: Robert Morris <rtm@csail.mit.edu>
Date: Thu, 3 Oct 2019 15:02:19 -0400
Subject: [PATCH] updated alarmtest

---
 kernel/defs.h     |  1 -
 kernel/exec.c     |  1 +
 kernel/plic.c     | 14 --------------
 kernel/proc.h     |  2 +-
 kernel/spinlock.c |  9 ++++++---
 user/alarmtest.c  | 30 ++++++++++++++++++++++++------
 6 files changed, 32 insertions(+), 25 deletions(-)

diff --git a/kernel/defs.h b/kernel/defs.h
index f893d28..9c5f643 100644
--- a/kernel/defs.h
+++ b/kernel/defs.h
@@ -173,7 +173,6 @@ int             copyinstr(pagetable_t, char *, uint64, uint64);
 // plic.c
 void            plicinit(void);
 void            plicinithart(void);
-uint64          plic_pending(void);
 int             plic_claim(void);
 void            plic_complete(int);
 
diff --git a/kernel/exec.c b/kernel/exec.c
index 74ef654..6a51499 100644
--- a/kernel/exec.c
+++ b/kernel/exec.c
@@ -112,6 +112,7 @@ exec(char *path, char **argv)
   p->tf->epc = elf.entry;  // initial program counter = main
   p->tf->sp = sp; // initial stack pointer
   proc_freepagetable(oldpagetable, oldsz);
+
   return argc; // this ends up in a0, the first argument to main(argc, argv)
 
  bad:
diff --git a/kernel/plic.c b/kernel/plic.c
index b569492..eed8316 100644
--- a/kernel/plic.c
+++ b/kernel/plic.c
@@ -28,20 +28,6 @@ plicinithart(void)
   *(uint32*)PLIC_SPRIORITY(hart) = 0;
 }
 
-// return a bitmap of which IRQs are waiting
-// to be served.
-uint64
-plic_pending(void)
-{
-  uint64 mask;
-
-  //mask = *(uint32*)(PLIC + 0x1000);
-  //mask |= (uint64)*(uint32*)(PLIC + 0x1004) << 32;
-  mask = *(uint64*)PLIC_PENDING;
-
-  return mask;
-}
-
 // ask the PLIC what interrupt we should serve.
 int
 plic_claim(void)
diff --git a/kernel/proc.h b/kernel/proc.h
index 538b48a..812c769 100644
--- a/kernel/proc.h
+++ b/kernel/proc.h
@@ -95,7 +95,7 @@ struct proc {
   int pid;                     // Process ID
 
   // these are private to the process, so p->lock need not be held.
-  uint64 kstack;               // Bottom of kernel stack for this process
+  uint64 kstack;               // Virtual address of kernel stack
   uint64 sz;                   // Size of process memory (bytes)
   pagetable_t pagetable;       // Page table
   struct trapframe *tf;        // data page for trampoline.S
diff --git a/kernel/spinlock.c b/kernel/spinlock.c
index 563532e..f192832 100644
--- a/kernel/spinlock.c
+++ b/kernel/spinlock.c
@@ -34,7 +34,8 @@ acquire(struct spinlock *lk)
 
   // Tell the C compiler and the processor to not move loads or stores
   // past this point, to ensure that the critical section's memory
-  // references happen after the lock is acquired.
+  // references happen strictly after the lock is acquired.
+  // On RISC-V, this emits a fence instruction.
   __sync_synchronize();
 
   // Record info about lock acquisition for holding() and debugging.
@@ -52,8 +53,10 @@ release(struct spinlock *lk)
 
   // Tell the C compiler and the CPU to not move loads or stores
   // past this point, to ensure that all the stores in the critical
-  // section are visible to other CPUs before the lock is released.
-  // On RISC-V, this turns into a fence instruction.
+  // section are visible to other CPUs before the lock is released,
+  // and that loads in the critical section occur strictly before
+  // the lock is released.
+  // On RISC-V, this emits a fence instruction.
   __sync_synchronize();
 
   // Release the lock, equivalent to lk->locked = 0.
diff --git a/user/alarmtest.c b/user/alarmtest.c
index ca3db23..d3746c4 100644
--- a/user/alarmtest.c
+++ b/user/alarmtest.c
@@ -21,7 +21,7 @@ main(int argc, char *argv[])
 {
   test0();
   test1();
-  exit();
+  exit(0);
 }
 
 volatile static int count;
@@ -44,7 +44,7 @@ test0()
   count = 0;
   sigalarm(2, periodic);
   for(i = 0; i < 1000*500000; i++){
-    if((i % 250000) == 0)
+    if((i % 1000000) == 0)
       write(2, ".", 1);
     if(count > 0)
       break;
@@ -53,7 +53,7 @@ test0()
   if(count > 0){
     printf("test0 passed\n");
   } else {
-    printf("test0 failed\n");
+    printf("\ntest0 failed: the kernel never called the alarm handler\n");
   }
 }
 
@@ -64,6 +64,14 @@ void __attribute__ ((noinline)) foo(int i, int *j) {
   *j += 1;
 }
 
+//
+// tests that the kernel calls the handler multiple times.
+//
+// tests that, when the handler returns, it returns to
+// the point in the program where the timer interrupt
+// occurred, with all registers holding the same values they
+// held when the interrupt occurred.
+//
 void
 test1()
 {
@@ -79,9 +87,19 @@ test1()
       break;
     foo(i, &j);
   }
-  if(i != j || count < 10){
-    // i should equal j
-    printf("test1 failed\n");
+  if(count < 10){
+    printf("\ntest1 failed: too few calls to the handler\n");
+    exit(1);
+  } else if(i != j){
+    // the loop should have called foo() i times, and foo() should
+    // have incremented j once per call, so j should equal i.
+    // once possible source of errors is that the handler may
+    // return somewhere other than where the timer interrupt
+    // occurred; another is that that registers may not be
+    // restored correctly, causing i or j or the address ofj
+    // to get an incorrect value.
+    printf("\ntest1 failed: foo() executed fewer times than it was called\n");
+    exit(1);
   } else {
     printf("test1 passed\n");
   }