Merge pull request #169 from rcore-os/configspace

Use config_generation for safe multi-part config reads.

src/device/blk.rs

@@ -56,10 +56,14 @@ impl<H: Hal, T: Transport> VirtIOBlk<H, T> {
         let negotiated_features = transport.begin_init(SUPPORTED_FEATURES);
 
         // Read configuration space.
-        let capacity = transport.read_config_space::<u32>(offset_of!(BlkConfig, capacity_low))?
-            as u64
-            | (transport.read_config_space::<u32>(offset_of!(BlkConfig, capacity_high))? as u64)
-                << 32;
+        let capacity = transport.read_consistent(|| {
+            Ok(
+                transport.read_config_space::<u32>(offset_of!(BlkConfig, capacity_low))? as u64
+                    | (transport.read_config_space::<u32>(offset_of!(BlkConfig, capacity_high))?
+                        as u64)
+                        << 32,
+            )
+        })?;
         info!("found a block device of size {}KB", capacity / 2);
 
         let queue = VirtQueue::new(

src/device/console.rs

@@ -127,10 +127,12 @@ impl<H: Hal, T: Transport> VirtIOConsole<H, T> {
     /// Returns the size of the console, if the device supports reporting this.
     pub fn size(&self) -> Result<Option<Size>> {
         if self.negotiated_features.contains(Features::SIZE) {
-            Ok(Some(Size {
-                columns: self.transport.read_config_space(offset_of!(Config, cols))?,
-                rows: self.transport.read_config_space(offset_of!(Config, rows))?,
-            }))
+            self.transport.read_consistent(|| {
+                Ok(Some(Size {
+                    columns: self.transport.read_config_space(offset_of!(Config, cols))?,
+                    rows: self.transport.read_config_space(offset_of!(Config, rows))?,
+                }))
+            })
         } else {
             Ok(None)
         }

src/device/net/dev_raw.rs

@@ -31,7 +31,8 @@ impl<H: Hal, T: Transport, const QUEUE_SIZE: usize> VirtIONetRaw<H, T, QUEUE_SIZ
         info!("negotiated_features {:?}", negotiated_features);
 
         // Read configuration space.
-        let mac = transport.read_config_space(offset_of!(Config, mac))?;
+        let mac =
+            transport.read_consistent(|| transport.read_config_space(offset_of!(Config, mac)))?;
         let status = transport.read_config_space::<Status>(offset_of!(Config, status))?;
         debug!("Got MAC={:02x?}, status={:?}", mac, status);
 

src/device/socket/vsock.rs

@@ -247,13 +247,16 @@ impl<H: Hal, T: Transport, const RX_BUFFER_SIZE: usize> VirtIOSocket<H, T, RX_BU
 
         let negotiated_features = transport.begin_init(SUPPORTED_FEATURES);
 
-        // Safe because config is a valid pointer to the device configuration space.
-        let guest_cid = transport
-            .read_config_space::<u32>(offset_of!(VirtioVsockConfig, guest_cid_low))?
-            as u64
-            | (transport.read_config_space::<u32>(offset_of!(VirtioVsockConfig, guest_cid_high))?
-                as u64)
-                << 32;
+        let guest_cid = transport.read_consistent(|| {
+            Ok(
+                transport.read_config_space::<u32>(offset_of!(VirtioVsockConfig, guest_cid_low))?
+                    as u64
+                    | (transport
+                        .read_config_space::<u32>(offset_of!(VirtioVsockConfig, guest_cid_high))?
+                        as u64)
+                        << 32,
+            )
+        })?;
         debug!("guest cid: {guest_cid:?}");
 
         let rx = VirtQueue::new(

src/transport/fake.rs

@@ -96,6 +96,10 @@ impl<C> Transport for FakeTransport<C> {
         pending
     }
 
+    fn read_config_generation(&self) -> u32 {
+        self.state.lock().unwrap().config_generation
+    }
+
     fn read_config_space<T>(&self, offset: usize) -> Result<T, Error> {
         assert!(align_of::<T>() <= 4,
             "Driver expected config space alignment of {} bytes, but VirtIO only guarantees 4 byte alignment.",
@@ -133,6 +137,7 @@ pub struct State {
     pub guest_page_size: u32,
     pub interrupt_pending: bool,
     pub queues: Vec<QueueStatus>,
+    pub config_generation: u32,
 }
 
 impl State {

src/transport/mmio.rs

@@ -497,6 +497,11 @@ impl Transport for MmioTransport {
         }
     }
 
+    fn read_config_generation(&self) -> u32 {
+        // SAFETY: self.header points to a valid VirtIO MMIO region.
+        unsafe { volread!(self.header, config_generation) }
+    }
+
     fn read_config_space<T: FromBytes>(&self, offset: usize) -> Result<T, Error> {
         assert!(align_of::<T>() <= 4,
             "Driver expected config space alignment of {} bytes, but VirtIO only guarantees 4 byte alignment.",

src/transport/mod.rs

@@ -101,6 +101,9 @@ pub trait Transport {
         );
     }
 
+    /// Reads the configuration space generation.
+    fn read_config_generation(&self) -> u32;
+
     /// Reads a value from the device config space.
     fn read_config_space<T: FromBytes>(&self, offset: usize) -> Result<T>;
 
@@ -110,6 +113,19 @@ pub trait Transport {
         offset: usize,
         value: T,
     ) -> Result<()>;
+
+    /// Safely reads multiple fields from config space by ensuring that the config generation is the
+    /// same before and after all reads, and retrying if not.
+    fn read_consistent<T>(&self, f: impl Fn() -> Result<T>) -> Result<T> {
+        loop {
+            let before = self.read_config_generation();
+            let result = f();
+            let after = self.read_config_generation();
+            if before == after {
+                break result;
+            }
+        }
+    }
 }
 
 bitflags! {

src/transport/pci.rs

@@ -326,6 +326,11 @@ impl Transport for PciTransport {
         isr_status & 0x3 != 0
     }
 
+    fn read_config_generation(&self) -> u32 {
+        // SAFETY: self.header points to a valid VirtIO MMIO region.
+        unsafe { volread!(self.common_cfg, config_generation) }.into()
+    }
+
     fn read_config_space<T: FromBytes>(&self, offset: usize) -> Result<T, Error> {
         assert!(align_of::<T>() <= 4,
             "Driver expected config space alignment of {} bytes, but VirtIO only guarantees 4 byte alignment.",

src/transport/some.rs

@@ -123,6 +123,13 @@ impl Transport for SomeTransport {
         }
     }
 
+    fn read_config_generation(&self) -> u32 {
+        match self {
+            Self::Mmio(mmio) => mmio.read_config_generation(),
+            Self::Pci(pci) => pci.read_config_generation(),
+        }
+    }
+
     fn read_config_space<T: FromBytes>(&self, offset: usize) -> Result<T> {
         match self {
             Self::Mmio(mmio) => mmio.read_config_space(offset),