Reset queues when device is dropped. (#30)

* Queue index is 16 bit.

* Use constant for queue index.

* Add method to reset queue.

* Reset queues when device is dropped.

It would be nicer to do it in the VirtQueue itself, but it doesn't have
a reference to the transport.

src/blk.rs

@@ -6,6 +6,8 @@ use bitflags::*;
 use core::hint::spin_loop;
 use log::*;
 
+const QUEUE: u16 = 0;
+
 /// The virtio block device is a simple virtual block device (ie. disk).
 ///
 /// Read and write requests (and other exotic requests) are placed in the queue,
@@ -40,7 +42,7 @@ impl<H: Hal, T: Transport> VirtIOBlk<H, T> {
         };
         info!("found a block device of size {}KB", capacity / 2);
 
-        let queue = VirtQueue::new(&mut transport, 0, 16)?;
+        let queue = VirtQueue::new(&mut transport, QUEUE, 16)?;
         transport.finish_init();
 
         Ok(VirtIOBlk {
@@ -201,6 +203,14 @@ impl<H: Hal, T: Transport> VirtIOBlk<H, T> {
     }
 }
 
+impl<H: Hal, T: Transport> Drop for VirtIOBlk<H, T> {
+    fn drop(&mut self) {
+        // Clear any pointers pointing to DMA regions, so the device doesn't try to access them
+        // after they have been freed.
+        self.transport.queue_unset(QUEUE);
+    }
+}
+
 #[repr(C)]
 struct BlkConfig {
     /// Number of 512 Bytes sectors

src/console.rs

@@ -109,6 +109,15 @@ impl<H: Hal, T: Transport> VirtIOConsole<'_, H, T> {
     }
 }
 
+impl<H: Hal, T: Transport> Drop for VirtIOConsole<'_, H, T> {
+    fn drop(&mut self) {
+        // Clear any pointers pointing to DMA regions, so the device doesn't try to access them
+        // after they have been freed.
+        self.transport.queue_unset(QUEUE_RECEIVEQ_PORT_0);
+        self.transport.queue_unset(QUEUE_TRANSMITQ_PORT_0);
+    }
+}
+
 #[repr(C)]
 struct Config {
     cols: ReadOnly<u16>,

src/gpu.rs

@@ -283,6 +283,15 @@ impl<H: Hal, T: Transport> VirtIOGpu<'_, H, T> {
     }
 }
 
+impl<H: Hal, T: Transport> Drop for VirtIOGpu<'_, H, T> {
+    fn drop(&mut self) {
+        // Clear any pointers pointing to DMA regions, so the device doesn't try to access them
+        // after they have been freed.
+        self.transport.queue_unset(QUEUE_TRANSMIT);
+        self.transport.queue_unset(QUEUE_CURSOR);
+    }
+}
+
 #[repr(C)]
 struct Config {
     /// Signals pending events to the driver。

src/input.rs

@@ -94,6 +94,15 @@ impl<H: Hal, T: Transport> VirtIOInput<H, T> {
     }
 }
 
+impl<H: Hal, T: Transport> Drop for VirtIOInput<H, T> {
+    fn drop(&mut self) {
+        // Clear any pointers pointing to DMA regions, so the device doesn't try to access them
+        // after they have been freed.
+        self.transport.queue_unset(QUEUE_EVENT);
+        self.transport.queue_unset(QUEUE_STATUS);
+    }
+}
+
 /// Select value used for [`VirtIOInput::query_config_select()`].
 #[repr(u8)]
 #[derive(Debug, Clone, Copy)]

src/net.rs

@@ -101,6 +101,15 @@ impl<H: Hal, T: Transport> VirtIONet<H, T> {
     }
 }
 
+impl<H: Hal, T: Transport> Drop for VirtIONet<H, T> {
+    fn drop(&mut self) {
+        // Clear any pointers pointing to DMA regions, so the device doesn't try to access them
+        // after they have been freed.
+        self.transport.queue_unset(QUEUE_RECEIVE);
+        self.transport.queue_unset(QUEUE_TRANSMIT);
+    }
+}
+
 bitflags! {
     struct Features: u64 {
         /// Device handles packets with partial checksum.

src/queue.rs

@@ -23,7 +23,7 @@ pub struct VirtQueue<H: Hal> {
     used: NonNull<UsedRing>,
 
     /// The index of queue
-    queue_idx: u32,
+    queue_idx: u16,
     /// The size of the queue.
     ///
     /// This is both the number of descriptors, and the number of slots in the available and used
@@ -81,7 +81,7 @@ impl<H: Hal> VirtQueue<H> {
             avail,
             used,
             queue_size: size,
-            queue_idx: idx as u32,
+            queue_idx: idx,
             num_used: 0,
             free_head: 0,
             avail_idx: 0,

src/transport/fake.rs

@@ -61,6 +61,14 @@ impl<C> Transport for FakeTransport<C> {
         state.queues[queue as usize].device_area = device_area;
     }
 
+    fn queue_unset(&mut self, queue: u16) {
+        let mut state = self.state.lock().unwrap();
+        state.queues[queue as usize].size = 0;
+        state.queues[queue as usize].descriptors = 0;
+        state.queues[queue as usize].driver_area = 0;
+        state.queues[queue as usize].device_area = 0;
+    }
+
     fn queue_used(&mut self, queue: u16) -> bool {
         self.state.lock().unwrap().queues[queue as usize].descriptors != 0
     }

src/transport/mmio.rs

@@ -418,6 +418,34 @@ impl Transport for MmioTransport {
         }
     }
 
+    fn queue_unset(&mut self, queue: u16) {
+        match self.version {
+            MmioVersion::Legacy => {
+                // Safe because self.header points to a valid VirtIO MMIO region.
+                unsafe {
+                    volwrite!(self.header, queue_sel, queue.into());
+                    volwrite!(self.header, queue_num, 0);
+                    volwrite!(self.header, legacy_queue_align, 0);
+                    volwrite!(self.header, legacy_queue_pfn, 0);
+                }
+            }
+            MmioVersion::Modern => {
+                // Safe because self.header points to a valid VirtIO MMIO region.
+                unsafe {
+                    volwrite!(self.header, queue_sel, queue.into());
+                    volwrite!(self.header, queue_ready, 0);
+                    volwrite!(self.header, queue_num, 0);
+                    volwrite!(self.header, queue_desc_low, 0);
+                    volwrite!(self.header, queue_desc_high, 0);
+                    volwrite!(self.header, queue_driver_low, 0);
+                    volwrite!(self.header, queue_driver_high, 0);
+                    volwrite!(self.header, queue_device_low, 0);
+                    volwrite!(self.header, queue_device_high, 0);
+                }
+            }
+        }
+    }
+
     fn queue_used(&mut self, queue: u16) -> bool {
         // Safe because self.header points to a valid VirtIO MMIO region.
         unsafe {

src/transport/mod.rs

@@ -40,6 +40,9 @@ pub trait Transport {
         device_area: PhysAddr,
     );
 
+    /// Disables and resets the given queue.
+    fn queue_unset(&mut self, queue: u16);
+
     /// Returns whether the queue is in use, i.e. has a nonzero PFN or is marked as ready.
     fn queue_used(&mut self, queue: u16) -> bool;
 

src/transport/pci.rs

@@ -283,6 +283,19 @@ impl Transport for PciTransport {
         }
     }
 
+    fn queue_unset(&mut self, queue: u16) {
+        // Safe because the common config pointer is valid and we checked in get_bar_region that it
+        // was aligned.
+        unsafe {
+            volwrite!(self.common_cfg, queue_enable, 0);
+            volwrite!(self.common_cfg, queue_select, queue);
+            volwrite!(self.common_cfg, queue_size, 0);
+            volwrite!(self.common_cfg, queue_desc, 0);
+            volwrite!(self.common_cfg, queue_driver, 0);
+            volwrite!(self.common_cfg, queue_device, 0);
+        }
+    }
+
     fn queue_used(&mut self, queue: u16) -> bool {
         // Safe because the common config pointer is valid and we checked in get_bar_region that it
         // was aligned.