diff --git a/examples/aarch64/src/main.rs b/examples/aarch64/src/main.rs index 0158d686..2d7ad046 100644 --- a/examples/aarch64/src/main.rs +++ b/examples/aarch64/src/main.rs @@ -209,7 +209,7 @@ fn virtio_net(transport: T) { fn virtio_console(transport: T) { let mut console = VirtIOConsole::::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" { diff --git a/src/device/blk.rs b/src/device/blk.rs index 53f4bb53..1ac4803b 100644 --- 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::{FromBytes, Immutable, IntoBytes, KnownLayout}; @@ -55,12 +56,10 @@ impl VirtIOBlk { let negotiated_features = transport.begin_init(SUPPORTED_FEATURES); // Read configuration space. - let config = transport.config_space::()?; - 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::(offset_of!(BlkConfig, capacity_low))? + as u64 + | (transport.read_config_space::(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 index ad7431b7..37585517 100644 --- 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; @@ -36,7 +34,7 @@ const SUPPORTED_FEATURES: Features = Features::RING_EVENT_IDX /// # fn example(transport: T) -> Result<(), Error> { /// let mut console = VirtIOConsole::::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 { transport: T, negotiated_features: Features, - config_space: NonNull, receiveq: VirtQueue, transmitq: VirtQueue, queue_buf_rx: Box<[u8; PAGE_SIZE]>, @@ -94,7 +91,6 @@ impl VirtIOConsole { /// Creates a new VirtIO console driver. pub fn new(mut transport: T) -> Result { let negotiated_features = transport.begin_init(SUPPORTED_FEATURES); - let config_space = transport.config_space::()?; let receiveq = VirtQueue::new( &mut transport, QUEUE_RECEIVEQ_PORT_0, @@ -117,7 +113,6 @@ impl VirtIOConsole { let mut console = VirtIOConsole { transport, negotiated_features, - config_space, receiveq, transmitq, queue_buf_rx, @@ -130,17 +125,14 @@ impl VirtIOConsole { } /// Returns the size of the console, if the device supports reporting this. - pub fn size(&self) -> Option { + pub fn size(&self) -> Result> { 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 VirtIOConsole { /// 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::(offset_of!(Config, emerg_wr), chr.into())?; Ok(()) } else { Err(Error::Unsupported) @@ -343,7 +333,7 @@ mod tests { }; let console = VirtIOConsole::>::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 - }) + })) ); } diff --git a/src/device/gpu.rs b/src/device/gpu.rs index 6c1cd7db..d299a990 100644 --- 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::{FromBytes, FromZeros, Immutable, IntoBytes, KnownLayout}; @@ -43,15 +44,12 @@ impl VirtIOGpu { let negotiated_features = transport.begin_init(SUPPORTED_FEATURES); // read configuration space - let config_space = transport.config_space::()?; - 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::(offset_of!(Config, events_read))?; + let num_scanouts = transport.read_config_space::(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 index 4891c9f4..6259e391 100644 --- 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::{FromBytes, FromZeros, Immutable, IntoBytes, KnownLayout}; /// Virtual human interface devices such as keyboards, mice and tablets. @@ -22,7 +21,6 @@ pub struct VirtIOInput { event_queue: VirtQueue, status_queue: VirtQueue, event_buf: Box<[InputEvent; 32]>, - config: NonNull, } impl VirtIOInput { @@ -32,8 +30,6 @@ impl VirtIOInput { let negotiated_features = transport.begin_init(SUPPORTED_FEATURES); - let config = transport.config_space::()?; - let mut event_queue = VirtQueue::new( &mut transport, QUEUE_EVENT, @@ -62,7 +58,6 @@ impl VirtIOInput { event_queue, status_queue, event_buf, - config, }) } @@ -107,19 +102,21 @@ impl VirtIOInput { select: InputConfigSelect, subsel: u8, out: &mut [u8], - ) -> u8 { - let size; + ) -> Result { + 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::())?; } - size + + Ok(size) } /// Queries a specific piece of information by `select` and `subsel`, allocates a sufficiently @@ -129,20 +126,24 @@ impl VirtIOInput { select: InputConfigSelect, subsel: u8, ) -> Result, 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::(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::())?; } + Ok(buf) } /// Queries a specific piece of information by `select` and `subsel` into a newly-allocated @@ -172,7 +173,7 @@ impl VirtIOInput { /// Queries and returns the ID information of the device. pub fn ids(&mut self) -> Result { 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::() { Ok(ids) } else { @@ -195,7 +196,8 @@ impl VirtIOInput { /// Queries and returns information about the given axis of the device. pub fn abs_info(&mut self, axis: u8) -> Result { 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::() { 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] diff --git a/src/device/net/dev_raw.rs b/src/device/net/dev_raw.rs index 1f4b4c81..8f478673 100644 --- 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::IntoBytes; @@ -28,18 +29,12 @@ impl VirtIONetRaw Result { let negotiated_features = transport.begin_init(SUPPORTED_FEATURES); info!("negotiated_features {:?}", negotiated_features); - // read configuration space - let config = transport.config_space::()?; - 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::(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 index d40a2d4f..d55a93b2 100644 --- a/src/device/net/mod.rs +++ b/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; } diff --git a/src/device/socket/vsock.rs b/src/device/socket/vsock.rs index a90ee13f..0566b2e7 100644 --- 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::{FromBytes, IntoBytes}; @@ -248,12 +247,13 @@ impl VirtIOSocket()?; - 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::(offset_of!(VirtioVsockConfig, guest_cid_low))? + as u64 + | (transport.read_config_space::(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 index b055d280..1bd73e8c 100644 --- 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 VirtIOSound { )?; // read configuration space - let config_ptr = transport.config_space::()?; - // 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 index 77ed8f9b..c0aa39b6 100644 --- a/src/transport/fake.rs +++ b/src/transport/fake.rs @@ -1,11 +1,10 @@ use super::{DeviceStatus, DeviceType, Transport}; use crate::{ queue::{fake_read_write_queue, Descriptor}, - PhysAddr, Result, + Error, PhysAddr, }; use alloc::{sync::Arc, vec::Vec}; use core::{ - any::TypeId, ptr::NonNull, sync::atomic::{AtomicBool, Ordering}, time::Duration, @@ -97,11 +96,32 @@ impl Transport for FakeTransport { pending } - fn config_space(&self) -> Result> { - if TypeId::of::() == TypeId::of::() { - Ok(self.config_space.cast()) + fn read_config_space(&self, offset: usize) -> Result { + assert!(align_of::() <= 4, + "Driver expected config space alignment of {} bytes, but VirtIO only guarantees 4 byte alignment.", + align_of::()); + assert!(offset % align_of::() == 0); + + if size_of::() < offset + size_of::() { + Err(Error::ConfigSpaceTooSmall) + } else { + unsafe { Ok(self.config_space.cast::().byte_add(offset).read()) } + } + } + + fn write_config_space(&mut self, offset: usize, value: T) -> Result<(), Error> { + assert!(align_of::() <= 4, + "Driver expected config space alignment of {} bytes, but VirtIO only guarantees 4 byte alignment.", + align_of::()); + assert!(offset % align_of::() == 0); + + if size_of::() < offset + size_of::() { + Err(Error::ConfigSpaceTooSmall) } else { - panic!("Unexpected config space type."); + unsafe { + self.config_space.cast::().byte_add(offset).write(value); + } + Ok(()) } } } diff --git a/src/transport/mmio.rs b/src/transport/mmio.rs index 434060e3..1d00f751 100644 --- a/src/transport/mmio.rs +++ b/src/transport/mmio.rs @@ -484,15 +484,40 @@ impl Transport for MmioTransport { } } - fn config_space(&self) -> Result, Error> { - if align_of::() > 4 { - // Panic as this should only happen if the driver is written incorrectly. - panic!( - "Driver expected config space alignment of {} bytes, but VirtIO only guarantees 4 byte alignment.", - align_of::() - ); + fn read_config_space(&self, offset: usize) -> Result { + assert!(align_of::() <= 4, + "Driver expected config space alignment of {} bytes, but VirtIO only guarantees 4 byte alignment.", + align_of::()); + assert!(offset % align_of::() == 0); + + // SAFETY: The caller of `MmioTransport::new` guaranteed that the header pointer was valid, + // which includes the config space. + unsafe { + Ok(self + .header + .cast::() + .byte_add(CONFIG_SPACE_OFFSET) + .byte_add(offset) + .read_volatile()) + } + } + + fn write_config_space(&mut self, offset: usize, value: T) -> Result<(), Error> { + assert!(align_of::() <= 4, + "Driver expected config space alignment of {} bytes, but VirtIO only guarantees 4 byte alignment.", + align_of::()); + assert!(offset % align_of::() == 0); + + // SAFETY: The caller of `MmioTransport::new` guaranteed that the header pointer was valid, + // which includes the config space. + unsafe { + self.header + .cast::() + .byte_add(CONFIG_SPACE_OFFSET) + .byte_add(offset) + .write_volatile(value); } - Ok(NonNull::new((self.header.as_ptr() as usize + CONFIG_SPACE_OFFSET) as _).unwrap()) + Ok(()) } } diff --git a/src/transport/mod.rs b/src/transport/mod.rs index 69234750..f166ac3c 100644 --- a/src/transport/mod.rs +++ b/src/transport/mod.rs @@ -7,8 +7,9 @@ pub mod pci; use crate::{PhysAddr, Result, PAGE_SIZE}; use bitflags::{bitflags, Flags}; -use core::{fmt::Debug, ops::BitAnd, ptr::NonNull}; +use core::{fmt::Debug, ops::BitAnd}; use log::debug; +use zerocopy::{FromBytes, IntoBytes}; /// A VirtIO transport layer. pub trait Transport { @@ -98,8 +99,11 @@ pub trait Transport { ); } - /// Gets the pointer to the config space. - fn config_space(&self) -> Result>; + /// Reads a value from the device config space. + fn read_config_space(&self, offset: usize) -> Result; + + /// Writes a value to the device config space. + fn write_config_space(&mut self, offset: usize, value: T) -> Result<()>; } bitflags! { diff --git a/src/transport/pci.rs b/src/transport/pci.rs index fbbb9524..5ac41145 100644 --- a/src/transport/pci.rs +++ b/src/transport/pci.rs @@ -325,23 +325,46 @@ impl Transport for PciTransport { isr_status & 0x3 != 0 } - fn config_space(&self) -> Result, Error> { - if let Some(config_space) = self.config_space { - if size_of::() > config_space.len() * size_of::() { - Err(Error::ConfigSpaceTooSmall) - } else if align_of::() > 4 { - // Panic as this should only happen if the driver is written incorrectly. - panic!( - "Driver expected config space alignment of {} bytes, but VirtIO only guarantees 4 byte alignment.", - align_of::() - ); - } else { + fn read_config_space(&self, offset: usize) -> Result { + assert!(align_of::() <= 4, + "Driver expected config space alignment of {} bytes, but VirtIO only guarantees 4 byte alignment.", + align_of::()); + assert_eq!(offset % align_of::(), 0); + + let config_space = self.config_space.ok_or(Error::ConfigSpaceMissing)?; + if config_space.len() * size_of::() < offset + size_of::() { + Err(Error::ConfigSpaceTooSmall) + } else { + // SAFETY: If we have a config space pointer it must be valid for its length, and we just + // checked that the offset and size of the access was within the length. + unsafe { // TODO: Use NonNull::as_non_null_ptr once it is stable. - let config_space_ptr = NonNull::new(config_space.as_ptr() as *mut u32).unwrap(); - Ok(config_space_ptr.cast()) + Ok((config_space.as_ptr() as *mut T) + .byte_add(offset) + .read_volatile()) } + } + } + + fn write_config_space(&mut self, offset: usize, value: T) -> Result<(), Error> { + assert!(align_of::() <= 4, + "Driver expected config space alignment of {} bytes, but VirtIO only guarantees 4 byte alignment.", + align_of::()); + assert_eq!(offset % align_of::(), 0); + + let config_space = self.config_space.ok_or(Error::ConfigSpaceMissing)?; + if config_space.len() * size_of::() < offset + size_of::() { + Err(Error::ConfigSpaceTooSmall) } else { - Err(Error::ConfigSpaceMissing) + // SAFETY: If we have a config space pointer it must be valid for its length, and we just + // checked that the offset and size of the access was within the length. + unsafe { + // TODO: Use NonNull::as_non_null_ptr once it is stable. + (config_space.as_ptr() as *mut T) + .byte_add(offset) + .write_volatile(value); + } + Ok(()) } } }