1 files changed, 34 insertions, 17 deletions

diff --git a/kernel/virtio_disk.c b/kernel/virtio_disk.c
index 06e0645..1dbd6ed 100644
--- a/kernel/virtio_disk.c
+++ b/kernel/virtio_disk.c
@@ -130,10 +130,13 @@ static void
 free_desc(int i)
 {
   if(i >= NUM)
-    panic("virtio_disk_intr 1");
+    panic("free_desc 1");
   if(disk.free[i])
-    panic("virtio_disk_intr 2");
+    panic("free_desc 2");
   disk.desc[i].addr = 0;
+  disk.desc[i].len = 0;
+  disk.desc[i].flags = 0;
+  disk.desc[i].next = 0;
   disk.free[i] = 1;
   wakeup(&disk.free[0]);
 }
@@ -143,9 +146,11 @@ static void
 free_chain(int i)
 {
   while(1){
+    int flag = disk.desc[i].flags;
+    int nxt = disk.desc[i].next;
     free_desc(i);
-    if(disk.desc[i].flags & VRING_DESC_F_NEXT)
-      i = disk.desc[i].next;
+    if(flag & VRING_DESC_F_NEXT)
+      i = nxt;
     else
       break;
   }
@@ -184,7 +189,7 @@ virtio_disk_rw(struct buf *b, int write)
     }
     sleep(&disk.free[0], &disk.vdisk_lock);
   }
-  
+
   // format the three descriptors.
   // qemu's virtio-blk.c reads them.
 
@@ -217,7 +222,7 @@ virtio_disk_rw(struct buf *b, int write)
   disk.desc[idx[1]].flags |= VRING_DESC_F_NEXT;
   disk.desc[idx[1]].next = idx[2];
 
-  disk.info[idx[0]].status = 0;
+  disk.info[idx[0]].status = 0xff; // device writes 0 on success
   disk.desc[idx[2]].addr = (uint64) &disk.info[idx[0]].status;
   disk.desc[idx[2]].len = 1;
   disk.desc[idx[2]].flags = VRING_DESC_F_WRITE; // device writes the status
@@ -227,13 +232,17 @@ virtio_disk_rw(struct buf *b, int write)
   b->disk = 1;
   disk.info[idx[0]].b = b;
 
-  // avail[0] is flags
-  // avail[1] tells the device how far to look in avail[2...].
-  // avail[2...] are desc[] indices the device should process.
+  // avail[0] is flags (always zero)
+  // avail[1] is an index into avail[2...] telling where we'll write next
+  // avail[2...] is a ring of NUM indices the device should process
   // we only tell device the first index in our chain of descriptors.
   disk.avail[2 + (disk.avail[1] % NUM)] = idx[0];
+
+  __sync_synchronize();
+
+  disk.avail[1] = disk.avail[1] + 1; // not % NUM ...
+
   __sync_synchronize();
-  disk.avail[1] = disk.avail[1] + 1;
 
   *R(VIRTIO_MMIO_QUEUE_NOTIFY) = 0; // value is queue number
 
@@ -253,18 +262,26 @@ virtio_disk_intr()
 {
   acquire(&disk.vdisk_lock);
 
-  while((disk.used_idx % NUM) != (disk.used->id % NUM)){
-    int id = disk.used->elems[disk.used_idx].id;
+  // this ack may race with the device writing new notifications to
+  // the "used" ring, in which case we may get an interrupt we don't
+  // need, which is harmless.
+  *R(VIRTIO_MMIO_INTERRUPT_ACK) = *R(VIRTIO_MMIO_INTERRUPT_STATUS) & 0x3;
+
+  __sync_synchronize();
+
+  while(disk.used_idx != disk.used->id){
+    __sync_synchronize();
+    int id = disk.used->elems[disk.used_idx % NUM].id;
 
     if(disk.info[id].status != 0)
       panic("virtio_disk_intr status");
-    
-    disk.info[id].b->disk = 0;   // disk is done with buf
-    wakeup(disk.info[id].b);
 
-    disk.used_idx = (disk.used_idx + 1) % NUM;
+    struct buf *b = disk.info[id].b;
+    b->disk = 0;   // disk is done with buf
+    wakeup(b);
+
+    disk.used_idx += 1;
   }
-  *R(VIRTIO_MMIO_INTERRUPT_ACK) = *R(VIRTIO_MMIO_INTERRUPT_STATUS) & 0x3;
 
   release(&disk.vdisk_lock);
 }