From 46744c4a13ec21e0818a49f31dbc3ad6ad592eed Mon Sep 17 00:00:00 2001
From: Robert Morris <rtm@csail.mit.edu>
Date: Thu, 13 Jun 2019 10:00:50 -0400
Subject: [PATCH] for spinlocks, use gcc intrinsics, but say what they compile
 to

---
 kernel/spinlock.c | 18 +++++++++---------
 kernel/virtio.h   | 24 ++++++++++++------------
 2 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/kernel/spinlock.c b/kernel/spinlock.c
index bbb7cb5..557a86c 100644
--- a/kernel/spinlock.c
+++ b/kernel/spinlock.c
@@ -27,9 +27,10 @@ acquire(struct spinlock *lk)
   if(holding(lk))
     panic("acquire");
 
-  // The xchg is atomic.
-  //while(xchg(&lk->locked, 1) != 0)
-  //  ;
+  // On RISC-V, this turns into an atomic swap:
+  //   a5 = 1
+  //   s1 = &lk->locked
+  //   amoswap.w.aq a5, a5, (s1)
   while(__sync_lock_test_and_set(&lk->locked, 1) != 0)
     ;
 
@@ -51,19 +52,18 @@ release(struct spinlock *lk)
 
   lk->cpu = 0;
 
-  // Tell the C compiler and the processor to not move loads or stores
+  // 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 cores before the lock is released.
-  // Both the C compiler and the hardware may re-order loads and
-  // stores; __sync_synchronize() tells them both not to.
   // On RISC-V, this turns into a fence instruction.
   __sync_synchronize();
 
   // Release the lock, equivalent to lk->locked = 0.
   // This code can't use a C assignment, since it might
-  // not be atomic. A real OS would use C atomics here.
-  // On RISC-V, use an amoswap instruction.
-  //asm volatile("movl $0, %0" : "+m" (lk->locked) : );
+  // not be atomic.
+  // On RISC-V, this turns into an atomic swap:
+  //   s1 = &lk->locked
+  //   amoswap.w zero, zero, (s1)
   __sync_lock_release(&lk->locked);
 
   pop_off();
diff --git a/kernel/virtio.h b/kernel/virtio.h
index bac215d..c142af9 100644
--- a/kernel/virtio.h
+++ b/kernel/virtio.h
@@ -35,13 +35,13 @@
 #define VIRTIO_CONFIG_S_FEATURES_OK	8
 
 // device feature bits
-#define VIRTIO_BLK_F_RO		5	/* Disk is read-only */
-#define VIRTIO_BLK_F_SCSI	7	/* Supports scsi command passthru */
-#define VIRTIO_BLK_F_CONFIG_WCE	11	/* Writeback mode available in config */
-#define VIRTIO_BLK_F_MQ		12	/* support more than one vq */
-#define VIRTIO_F_ANY_LAYOUT		27
-#define VIRTIO_RING_F_INDIRECT_DESC	28
-#define VIRTIO_RING_F_EVENT_IDX		29
+#define VIRTIO_BLK_F_RO              5	/* Disk is read-only */
+#define VIRTIO_BLK_F_SCSI            7	/* Supports scsi command passthru */
+#define VIRTIO_BLK_F_CONFIG_WCE     11	/* Writeback mode available in config */
+#define VIRTIO_BLK_F_MQ             12	/* support more than one vq */
+#define VIRTIO_F_ANY_LAYOUT         27
+#define VIRTIO_RING_F_INDIRECT_DESC 28
+#define VIRTIO_RING_F_EVENT_IDX     29
 
 struct VRingDesc {
   uint64 addr;
@@ -49,14 +49,14 @@ struct VRingDesc {
   uint16 flags;
   uint16 next;
 };
-#define VRING_DESC_F_NEXT	1
-#define VRING_DESC_F_WRITE	2 // device writes (vs read)
+#define VRING_DESC_F_NEXT  1 // chained with another descriptor
+#define VRING_DESC_F_WRITE 2 // device writes (vs read)
 
 struct VRingUsedElem {
-  uint32 id; // index of start of completed descriptor chain
+  uint32 id;   // index of start of completed descriptor chain
   uint32 len;
 };
 
 // for disk ops
-#define VIRTIO_BLK_T_IN		0
-#define VIRTIO_BLK_T_OUT	1
+#define VIRTIO_BLK_T_IN  0 // read the disk
+#define VIRTIO_BLK_T_OUT 1 // write the disk