Add methods to Transport to read and write config space, rather than

qwandor · qwandor · commit 0854150a3caf · 2024-10-28T15:50:15.000Z
returning a pointer.

The config space is not necessarily memory mapped, some other transports
require the config space to be accessed via other IO operations such as
hypercalls.
diff --git a/src/device/blk.rs b/src/device/blk.rs
@@ -3,9 +3,10 @@
 use crate::hal::Hal;
 use crate::queue::VirtQueue;
 use crate::transport::Transport;
-use crate::volatile::{volread, Volatile};
+use crate::volatile::Volatile;
 use crate::{Error, Result};
 use bitflags::bitflags;
+use core::mem::offset_of;
 use log::info;
 use zerocopy::{AsBytes, FromBytes, FromZeroes};
 
@@ -55,12 +56,10 @@ impl<H: Hal, T: Transport> VirtIOBlk<H, T> {
         let negotiated_features = transport.begin_init(SUPPORTED_FEATURES);
 
         // Read configuration space.
-        let config = transport.config_space::<BlkConfig>()?;
-        info!("config: {:?}", config);
-        // Safe because config is a valid pointer to the device configuration space.
-        let capacity = unsafe {
-            volread!(config, capacity_low) as u64 | (volread!(config, capacity_high) as u64) << 32
-        };
+        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;
         info!("found a block device of size {}KB", capacity / 2);
 
         let queue = VirtQueue::new(
diff --git a/src/device/console.rs b/src/device/console.rs
@@ -6,14 +6,12 @@ mod embedded_io;
 use crate::hal::Hal;
 use crate::queue::VirtQueue;
 use crate::transport::Transport;
-use crate::volatile::{volread, volwrite, ReadOnly, WriteOnly};
+use crate::volatile::{ReadOnly, WriteOnly};
 use crate::{Error, Result, PAGE_SIZE};
 use alloc::boxed::Box;
 use bitflags::bitflags;
-use core::{
-    fmt::{self, Display, Formatter, Write},
-    ptr::NonNull,
-};
+use core::fmt::{self, Display, Formatter, Write};
+use core::mem::offset_of;
 use log::error;
 
 const QUEUE_RECEIVEQ_PORT_0: u16 = 0;
@@ -51,7 +49,6 @@ const SUPPORTED_FEATURES: Features = Features::RING_EVENT_IDX
 pub struct VirtIOConsole<H: Hal, T: Transport> {
     transport: T,
     negotiated_features: Features,
-    config_space: NonNull<Config>,
     receiveq: VirtQueue<H, QUEUE_SIZE>,
     transmitq: VirtQueue<H, QUEUE_SIZE>,
     queue_buf_rx: Box<[u8; PAGE_SIZE]>,
@@ -94,7 +91,6 @@ impl<H: Hal, T: Transport> VirtIOConsole<H, T> {
     /// Creates a new VirtIO console driver.
     pub fn new(mut transport: T) -> Result<Self> {
         let negotiated_features = transport.begin_init(SUPPORTED_FEATURES);
-        let config_space = transport.config_space::<Config>()?;
         let receiveq = VirtQueue::new(
             &mut transport,
             QUEUE_RECEIVEQ_PORT_0,
@@ -117,7 +113,6 @@ impl<H: Hal, T: Transport> VirtIOConsole<H, T> {
         let mut console = VirtIOConsole {
             transport,
             negotiated_features,
-            config_space,
             receiveq,
             transmitq,
             queue_buf_rx,
@@ -132,13 +127,10 @@ impl<H: Hal, T: Transport> VirtIOConsole<H, T> {
     /// Returns the size of the console, if the device supports reporting this.
     pub fn size(&self) -> Option<Size> {
         if self.negotiated_features.contains(Features::SIZE) {
-            // SAFETY: self.config_space is a valid pointer to the device configuration space.
-            unsafe {
-                Some(Size {
-                    columns: volread!(self.config_space, cols),
-                    rows: volread!(self.config_space, rows),
-                })
-            }
+            Some(Size {
+                columns: self.transport.read_config_space(offset_of!(Config, cols)),
+                rows: self.transport.read_config_space(offset_of!(Config, rows)),
+            })
         } else {
             None
         }
@@ -246,10 +238,8 @@ impl<H: Hal, T: Transport> VirtIOConsole<H, T> {
     /// Returns an error if the device doesn't support emergency write.
     pub fn emergency_write(&mut self, chr: u8) -> Result<()> {
         if self.negotiated_features.contains(Features::EMERG_WRITE) {
-            // SAFETY: `self.config_space` is a valid pointer to the device configuration space.
-            unsafe {
-                volwrite!(self.config_space, emerg_wr, chr.into());
-            }
+            self.transport
+                .write_config_space::<u32>(offset_of!(Config, emerg_wr), chr.into());
             Ok(())
         } else {
             Err(Error::Unsupported)
diff --git a/src/device/gpu.rs b/src/device/gpu.rs
@@ -3,10 +3,11 @@
 use crate::hal::{BufferDirection, Dma, Hal};
 use crate::queue::VirtQueue;
 use crate::transport::Transport;
-use crate::volatile::{volread, ReadOnly, Volatile, WriteOnly};
+use crate::volatile::{ReadOnly, Volatile, WriteOnly};
 use crate::{pages, Error, Result, PAGE_SIZE};
 use alloc::boxed::Box;
 use bitflags::bitflags;
+use core::mem::offset_of;
 use log::info;
 use zerocopy::{AsBytes, FromBytes, FromZeroes};
 
@@ -43,15 +44,12 @@ impl<H: Hal, T: Transport> VirtIOGpu<H, T> {
         let negotiated_features = transport.begin_init(SUPPORTED_FEATURES);
 
         // read configuration space
-        let config_space = transport.config_space::<Config>()?;
-        unsafe {
-            let events_read = volread!(config_space, events_read);
-            let num_scanouts = volread!(config_space, num_scanouts);
-            info!(
-                "events_read: {:#x}, num_scanouts: {:#x}",
-                events_read, num_scanouts
-            );
-        }
+        let events_read = transport.read_config_space::<u32>(offset_of!(Config, events_read));
+        let num_scanouts = transport.read_config_space::<u32>(offset_of!(Config, num_scanouts));
+        info!(
+            "events_read: {:#x}, num_scanouts: {:#x}",
+            events_read, num_scanouts
+        );
 
         let control_queue = VirtQueue::new(
             &mut transport,
diff --git a/src/device/input.rs b/src/device/input.rs
@@ -4,12 +4,11 @@ use super::common::Feature;
 use crate::hal::Hal;
 use crate::queue::VirtQueue;
 use crate::transport::Transport;
-use crate::volatile::{volread, volwrite, ReadOnly, VolatileReadable, WriteOnly};
+use crate::volatile::{ReadOnly, WriteOnly};
 use crate::Error;
 use alloc::{boxed::Box, string::String};
 use core::cmp::min;
-use core::mem::size_of;
-use core::ptr::{addr_of, NonNull};
+use core::mem::{offset_of, size_of};
 use zerocopy::{AsBytes, FromBytes, FromZeroes};
 
 /// Virtual human interface devices such as keyboards, mice and tablets.
@@ -22,7 +21,6 @@ pub struct VirtIOInput<H: Hal, T: Transport> {
     event_queue: VirtQueue<H, QUEUE_SIZE>,
     status_queue: VirtQueue<H, QUEUE_SIZE>,
     event_buf: Box<[InputEvent; 32]>,
-    config: NonNull<Config>,
 }
 
 impl<H: Hal, T: Transport> VirtIOInput<H, T> {
@@ -32,8 +30,6 @@ impl<H: Hal, T: Transport> VirtIOInput<H, T> {
 
         let negotiated_features = transport.begin_init(SUPPORTED_FEATURES);
 
-        let config = transport.config_space::<Config>()?;
-
         let mut event_queue = VirtQueue::new(
             &mut transport,
             QUEUE_EVENT,
@@ -62,7 +58,6 @@ impl<H: Hal, T: Transport> VirtIOInput<H, T> {
             event_queue,
             status_queue,
             event_buf,
-            config,
         })
     }
 
@@ -108,17 +103,19 @@ impl<H: Hal, T: Transport> VirtIOInput<H, T> {
         subsel: u8,
         out: &mut [u8],
     ) -> u8 {
-        let size;
+        self.transport
+            .write_config_space(offset_of!(Config, select), select as u8);
+        self.transport
+            .write_config_space(offset_of!(Config, subsel), subsel);
+        let size: u8 = self.transport.read_config_space(offset_of!(Config, size));
         // Safe because config points to a valid MMIO region for the config space.
-        unsafe {
-            volwrite!(self.config, select, select as u8);
-            volwrite!(self.config, subsel, subsel);
-            size = volread!(self.config, size);
-            let size_to_copy = min(usize::from(size), out.len());
-            for (i, out_item) in out.iter_mut().take(size_to_copy).enumerate() {
-                *out_item = addr_of!((*self.config.as_ptr()).data[i]).vread();
-            }
+        let size_to_copy = min(usize::from(size), out.len());
+        for (i, out_item) in out.iter_mut().take(size_to_copy).enumerate() {
+            *out_item = self
+                .transport
+                .read_config_space(offset_of!(Config, data) + i * size_of::<u8>());
         }
+
         size
     }
 
@@ -129,20 +126,24 @@ impl<H: Hal, T: Transport> VirtIOInput<H, T> {
         select: InputConfigSelect,
         subsel: u8,
     ) -> Result<Box<[u8]>, Error> {
-        // Safe because config points to a valid MMIO region for the config space.
-        unsafe {
-            volwrite!(self.config, select, select as u8);
-            volwrite!(self.config, subsel, subsel);
-            let size = usize::from(volread!(self.config, size));
-            if size > CONFIG_DATA_MAX_LENGTH {
-                return Err(Error::IoError);
-            }
-            let mut buf = u8::new_box_slice_zeroed(size);
-            for i in 0..size {
-                buf[i] = addr_of!((*self.config.as_ptr()).data[i]).vread();
-            }
-            Ok(buf)
+        self.transport
+            .write_config_space(offset_of!(Config, select), select as u8);
+        self.transport
+            .write_config_space(offset_of!(Config, subsel), subsel);
+        let size = usize::from(
+            self.transport
+                .read_config_space::<u8>(offset_of!(Config, size)),
+        );
+        if size > CONFIG_DATA_MAX_LENGTH {
+            return Err(Error::IoError);
+        }
+        let mut buf = u8::new_box_slice_zeroed(size);
+        for i in 0..size {
+            buf[i] = self
+                .transport
+                .read_config_space(offset_of!(Config, data) + i * size_of::<u8>());
         }
+        Ok(buf)
     }
 
     /// Queries a specific piece of information by `select` and `subsel` into a newly-allocated
@@ -322,7 +323,7 @@ mod tests {
         },
     };
     use alloc::{sync::Arc, vec};
-    use core::convert::TryInto;
+    use core::{convert::TryInto, ptr::NonNull};
     use std::sync::Mutex;
 
     #[test]
diff --git a/src/device/net/dev_raw.rs b/src/device/net/dev_raw.rs
@@ -1,10 +1,11 @@
 use super::{Config, EthernetAddress, Features, VirtioNetHdr};
 use super::{MIN_BUFFER_LEN, NET_HDR_SIZE, QUEUE_RECEIVE, QUEUE_TRANSMIT, SUPPORTED_FEATURES};
+use crate::device::net::Status;
 use crate::hal::Hal;
 use crate::queue::VirtQueue;
 use crate::transport::Transport;
-use crate::volatile::volread;
 use crate::{Error, Result};
+use core::mem::offset_of;
 use log::{debug, info, warn};
 use zerocopy::AsBytes;
 
@@ -28,18 +29,12 @@ impl<H: Hal, T: Transport, const QUEUE_SIZE: usize> VirtIONetRaw<H, T, QUEUE_SIZ
     pub fn new(mut transport: T) -> Result<Self> {
         let negotiated_features = transport.begin_init(SUPPORTED_FEATURES);
         info!("negotiated_features {:?}", negotiated_features);
-        // read configuration space
-        let config = transport.config_space::<Config>()?;
-        let mac;
-        // Safe because config points to a valid MMIO region for the config space.
-        unsafe {
-            mac = volread!(config, mac);
-            debug!(
-                "Got MAC={:02x?}, status={:?}",
-                mac,
-                volread!(config, status)
-            );
-        }
+
+        // Read configuration space.
+        let mac = 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);
+
         let send_queue = VirtQueue::new(
             &mut transport,
             QUEUE_TRANSMIT,
diff --git a/src/device/net/mod.rs b/src/device/net/mod.rs
@@ -81,9 +81,12 @@ bitflags! {
     }
 }
 
+#[derive(Copy, Clone, Debug, Default, Eq, FromBytes, FromZeroes, PartialEq)]
+#[repr(transparent)]
+struct Status(u16);
+
 bitflags! {
-    #[derive(Copy, Clone, Debug, Default, Eq, PartialEq)]
-    struct Status: u16 {
+    impl Status: u16 {
         const LINK_UP = 1;
         const ANNOUNCE = 2;
     }
diff --git a/src/device/socket/vsock.rs b/src/device/socket/vsock.rs
@@ -9,9 +9,8 @@ use super::DEFAULT_RX_BUFFER_SIZE;
 use crate::hal::Hal;
 use crate::queue::{owning::OwningQueue, VirtQueue};
 use crate::transport::Transport;
-use crate::volatile::volread;
 use crate::Result;
-use core::mem::size_of;
+use core::mem::{offset_of, size_of};
 use log::debug;
 use zerocopy::{AsBytes, FromBytes};
 
@@ -248,12 +247,12 @@ impl<H: Hal, T: Transport, const RX_BUFFER_SIZE: usize> VirtIOSocket<H, T, RX_BU
 
         let negotiated_features = transport.begin_init(SUPPORTED_FEATURES);
 
-        let config = transport.config_space::<VirtioVsockConfig>()?;
-        debug!("config: {:?}", config);
         // Safe because config is a valid pointer to the device configuration space.
-        let guest_cid = unsafe {
-            volread!(config, guest_cid_low) as u64 | (volread!(config, guest_cid_high) as u64) << 32
-        };
+        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;
         debug!("guest cid: {guest_cid:?}");
 
         let rx = VirtQueue::new(
diff --git a/src/device/sound.rs b/src/device/sound.rs
@@ -7,7 +7,7 @@ use super::common::Feature;
 use crate::{
     queue::{owning::OwningQueue, VirtQueue},
     transport::Transport,
-    volatile::{volread, ReadOnly},
+    volatile::ReadOnly,
     Error, Hal, Result, PAGE_SIZE,
 };
 use alloc::{boxed::Box, collections::BTreeMap, vec, vec::Vec};
@@ -16,7 +16,7 @@ use core::{
     array,
     fmt::{self, Debug, Display, Formatter},
     hint::spin_loop,
-    mem::size_of,
+    mem::{offset_of, size_of},
     ops::RangeInclusive,
 };
 use enumn::N;
@@ -96,15 +96,9 @@ impl<H: Hal, T: Transport> VirtIOSound<H, T> {
         )?;
 
         // read configuration space
-        let config_ptr = transport.config_space::<VirtIOSoundConfig>()?;
-        // SAFETY: config_ptr is a valid pointer to the device configuration space.
-        let (jacks, streams, chmaps) = unsafe {
-            (
-                volread!(config_ptr, jacks),
-                volread!(config_ptr, streams),
-                volread!(config_ptr, chmaps),
-            )
-        };
+        let jacks = transport.read_config_space(offset_of!(VirtIOSoundConfig, jacks));
+        let streams = transport.read_config_space(offset_of!(VirtIOSoundConfig, streams));
+        let chmaps = transport.read_config_space(offset_of!(VirtIOSoundConfig, chmaps));
         info!(
             "[sound device] config: jacks: {}, streams: {}, chmaps: {}",
             jacks, streams, chmaps
diff --git a/src/transport/fake.rs b/src/transport/fake.rs
diff --git a/src/transport/mmio.rs b/src/transport/mmio.rs
diff --git a/src/transport/mod.rs b/src/transport/mod.rs
diff --git a/src/transport/pci.rs b/src/transport/pci.rs

Original file line number	Diff line number	Diff line change
`@@ -81,9 +81,12 @@ bitflags! {`
`81`	`81`	`}`
`82`	`82`	`}`
`83`	`83`
	`84`	`+#[derive(Copy, Clone, Debug, Default, Eq, FromBytes, FromZeroes, PartialEq)]`
	`85`	`+#[repr(transparent)]`
	`86`	`+struct Status(u16);`
	`87`	`+`
`84`	`88`	`bitflags! {`
`85`		`- #[derive(Copy, Clone, Debug, Default, Eq, PartialEq)]`
`86`		`- struct Status: u16 {`
	`89`	`+ impl Status: u16 {`
`87`	`90`	`const LINK_UP = 1;`
`88`	`91`	`const ANNOUNCE = 2;`
`89`	`92`	`}`