Merge pull request #163 from rcore-os/configspace

Add methods to Transport to read and write config space, rather than returning a pointer.

examples/aarch64/src/main.rs

@@ -209,7 +209,7 @@ fn virtio_net<T: Transport>(transport: T) {
 fn virtio_console<T: Transport>(transport: T) {
     let mut console =
         VirtIOConsole::<HalImpl, T>::new(transport).expect("Failed to create console driver");
-    if let Some(size) = console.size() {
+    if let Some(size) = console.size().unwrap() {
         info!("VirtIO console {}", size);
     }
     for &c in b"Hello world on console!\n" {

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::{FromBytes, Immutable, IntoBytes, KnownLayout};
 
@@ -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(

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;
@@ -36,7 +34,7 @@ const SUPPORTED_FEATURES: Features = Features::RING_EVENT_IDX
 /// # fn example<HalImpl: Hal, T: Transport>(transport: T) -> Result<(), Error> {
 /// let mut console = VirtIOConsole::<HalImpl, _>::new(transport)?;
 ///
-/// let size = console.size().unwrap();
+/// let size = console.size().unwrap().unwrap();
 /// println!("VirtIO console {}x{}", size.rows, size.columns);
 ///
 /// for &c in b"Hello console!\n" {
@@ -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,
@@ -130,17 +125,14 @@ 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> {
+    pub fn size(&self) -> Result<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),
-                })
-            }
+            Ok(Some(Size {
+                columns: self.transport.read_config_space(offset_of!(Config, cols))?,
+                rows: self.transport.read_config_space(offset_of!(Config, rows))?,
+            }))
         } else {
-            None
+            Ok(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)
@@ -343,7 +333,7 @@ mod tests {
         };
         let console = VirtIOConsole::<FakeHal, FakeTransport<Config>>::new(transport).unwrap();
 
-        assert_eq!(console.size(), None);
+        assert_eq!(console.size(), Ok(None));
     }
 
     #[test]
@@ -369,10 +359,10 @@ mod tests {
 
         assert_eq!(
             console.size(),
-            Some(Size {
+            Ok(Some(Size {
                 columns: 80,
                 rows: 42
-            })
+            }))
         );
     }
 

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::{FromBytes, FromZeros, Immutable, IntoBytes, KnownLayout};
 
@@ -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,

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::{FromBytes, FromZeros, Immutable, IntoBytes, KnownLayout};
 
 /// 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,
         })
     }
 
@@ -107,19 +102,21 @@ impl<H: Hal, T: Transport> VirtIOInput<H, T> {
         select: InputConfigSelect,
         subsel: u8,
         out: &mut [u8],
-    ) -> u8 {
-        let size;
+    ) -> Result<u8, Error> {
+        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
+
+        Ok(size)
     }
 
     /// Queries a specific piece of information by `select` and `subsel`, allocates a sufficiently
@@ -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_zeroed_with_elems(size).unwrap();
-            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_zeroed_with_elems(size).unwrap();
+        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
@@ -172,7 +173,7 @@ impl<H: Hal, T: Transport> VirtIOInput<H, T> {
     /// Queries and returns the ID information of the device.
     pub fn ids(&mut self) -> Result<DevIDs, Error> {
         let mut ids = DevIDs::default();
-        let size = self.query_config_select(InputConfigSelect::IdDevids, 0, ids.as_mut_bytes());
+        let size = self.query_config_select(InputConfigSelect::IdDevids, 0, ids.as_mut_bytes())?;
         if usize::from(size) == size_of::<DevIDs>() {
             Ok(ids)
         } else {
@@ -195,7 +196,8 @@ impl<H: Hal, T: Transport> VirtIOInput<H, T> {
     /// Queries and returns information about the given axis of the device.
     pub fn abs_info(&mut self, axis: u8) -> Result<AbsInfo, Error> {
         let mut info = AbsInfo::default();
-        let size = self.query_config_select(InputConfigSelect::AbsInfo, axis, info.as_mut_bytes());
+        let size =
+            self.query_config_select(InputConfigSelect::AbsInfo, axis, info.as_mut_bytes())?;
         if usize::from(size) == size_of::<AbsInfo>() {
             Ok(info)
         } else {
@@ -322,7 +324,7 @@ mod tests {
         },
     };
     use alloc::{sync::Arc, vec};
-    use core::convert::TryInto;
+    use core::{convert::TryInto, ptr::NonNull};
     use std::sync::Mutex;
 
     #[test]

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::IntoBytes;
 
@@ -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,

src/device/net/mod.rs

@@ -81,9 +81,12 @@ bitflags! {
     }
 }
 
+#[derive(Copy, Clone, Debug, Default, Eq, FromBytes, Immutable, KnownLayout, 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;
     }