1 files changed, 34 insertions, 30 deletions

diff --git a/kernel/virtio_disk.c b/kernel/virtio_disk.c
index 558d3b0..6bcad9c 100644
--- a/kernel/virtio_disk.c
+++ b/kernel/virtio_disk.c
@@ -17,8 +17,8 @@
 #include "buf.h"
 #include "virtio.h"
 
-// the address of a virtio mmio register.
-#define R(off) ((volatile uint32 *)(VIRTIO + (off)))
+// the address of virtio mmio register r.
+#define R(r) ((volatile uint32 *)(VIRTIO0 + (r)))
 
 struct spinlock virtio_disk_lock;
 
@@ -45,6 +45,7 @@ static uint16 used_idx; // we've looked this far in used[2..NUM].
 // indexed by first descriptor index of chain.
 static struct {
   struct buf *b;
+  char status;
 } info[NUM];
 
 void
@@ -54,8 +55,6 @@ virtio_disk_init(void)
 
   initlock(&virtio_disk_lock, "virtio_disk");
 
-  // qemu's virtio-mmio.c
-
   if(*R(VIRTIO_MMIO_MAGIC_VALUE) != 0x74726976 ||
      *R(VIRTIO_MMIO_VERSION) != 1 ||
      *R(VIRTIO_MMIO_DEVICE_ID) != 2 ||
@@ -90,9 +89,7 @@ virtio_disk_init(void)
 
   *R(VIRTIO_MMIO_GUEST_PAGE_SIZE) = PGSIZE;
 
-  // qemu's hw/virtio/virtio.c
-
-  // initialize queue 0
+  // initialize queue 0.
   *R(VIRTIO_MMIO_QUEUE_SEL) = 0;
   uint32 max = *R(VIRTIO_MMIO_QUEUE_NUM_MAX);
   if(max == 0)
@@ -113,6 +110,8 @@ virtio_disk_init(void)
 
   for(int i = 0; i < NUM; i++)
     free[i] = 1;
+
+  // plic.c and trap.c arrange for interrupts from VIRTIO0_IRQ.
 }
 
 // find a free descriptor, mark it non-free, return its index.
@@ -128,14 +127,30 @@ alloc_desc()
   return -1;
 }
 
-void
+// mark a descriptor as free.
+static void
 free_desc(int i)
 {
   if(i >= NUM)
     panic("virtio_disk_intr 1");
   if(free[i])
     panic("virtio_disk_intr 2");
+  desc[i].addr = 0;
   free[i] = 1;
+  wakeup(&free[0]);
+}
+
+// free a chain of descriptors.
+static void
+free_chain(int i)
+{
+  while(1){
+    free_desc(i);
+    if(desc[i].flags & VRING_DESC_F_NEXT)
+      i = desc[i].next;
+    else
+      break;
+  }
 }
 
 void
@@ -145,7 +160,7 @@ virtio_disk_rw(struct buf *b)
 
   acquire(&virtio_disk_lock);
 
-  // the spec says that legacy block operations always use three
+  // the spec says that legacy block operations use three
   // descriptors: one for type/reserved/sector, one for
   // the data, one for a 1-byte status result.
 
@@ -158,7 +173,6 @@ virtio_disk_rw(struct buf *b)
       if(idx[i] < 0){
         for(int j = 0; j < i; j++)
           free_desc(idx[j]);
-        wakeup(&free[0]);
         done = 0;
         break;
       }
@@ -198,8 +212,8 @@ virtio_disk_rw(struct buf *b)
   desc[idx[1]].flags |= VRING_DESC_F_NEXT;
   desc[idx[1]].next = idx[2];
 
-  char status = 0;
-  desc[idx[2]].addr = (uint64) &status;
+  info[idx[0]].status = 0;
+  desc[idx[2]].addr = (uint64) &info[idx[0]].status;
   desc[idx[2]].len = 1;
   desc[idx[2]].flags = VRING_DESC_F_WRITE; // device writes the status
   desc[idx[2]].next = 0;
@@ -232,16 +246,16 @@ virtio_disk_intr()
   // uint16 flags
   // uint16 idx
   // array of VRingUsedElem
-
-  // XXX spec says to read INTERRUPT_STATUS and
-  // write INTERRUPT_ACK
+  volatile uint16 *idxp = (uint16 *)(used + 2);
+  volatile struct VRingUsedElem *e0 = (struct VRingUsedElem *)(used + 4);
 
   acquire(&virtio_disk_lock);
-  
-  while((used_idx % NUM) != (*(volatile uint16 *)(used+2) % NUM)){
-    struct VRingUsedElem *ue = (struct VRingUsedElem *) (used + 4 + 8*used_idx);
 
-    // XXX check the one-byte status in the 3rd descriptor.
+  while((used_idx % NUM) != (*idxp % NUM)){
+    volatile struct VRingUsedElem *ue = &e0[used_idx];
+
+    if(info[ue->id].status != 0)
+      panic("virtio_disk_intr status");
 
     info[ue->id].b->flags |= B_VALID;
     info[ue->id].b->flags &= ~B_DIRTY;
@@ -249,17 +263,7 @@ virtio_disk_intr()
     wakeup(info[ue->id].b);
 
     info[ue->id].b = 0;
-
-    uint i = ue->id;
-    while(1){
-      desc[i].addr = 0;
-      free_desc(i);
-      if(desc[i].flags & VRING_DESC_F_NEXT)
-        i = desc[i].next;
-      else
-        break;
-    }
-    wakeup(&free[0]);
+    free_chain(ue->id);
 
     used_idx = (used_idx + 1) % NUM;
   }