From 2fa73a2fb3b7a97f8a36b5811dd23989e03c2eee Mon Sep 17 00:00:00 2001 From: Andrew Walbran Date: Wed, 6 Nov 2024 17:38:58 +0000 Subject: [PATCH] Use thiserror for error types. This reduces boilerplate, and adds a core::error::Error implementation. --- Cargo.toml | 1 + src/device/socket/error.rs | 44 ++++++++++----------------- src/lib.rs | 61 +++++++++++--------------------------- src/transport/mmio.rs | 22 +++----------- src/transport/pci.rs | 54 ++++++++------------------------- src/transport/pci/bus.rs | 12 ++------ 6 files changed, 54 insertions(+), 140 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index fe955b40..53c19a85 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,6 +19,7 @@ log = "0.4.22" bitflags = "2.6.0" enumn = "0.1.14" embedded-io = { version = "0.6.1", optional = true } +thiserror = { version = "2.0.0", default-features = false } zerocopy = { version = "0.8.7", features = ["derive"] } [features] diff --git a/src/device/socket/error.rs b/src/device/socket/error.rs index ea7e5870..a398f66f 100644 --- a/src/device/socket/error.rs +++ b/src/device/socket/error.rs @@ -1,61 +1,47 @@ //! This module contain the error from the VirtIO socket driver. -use core::{fmt, result}; +use core::result; +use thiserror::Error; /// The error type of VirtIO socket driver. -#[derive(Copy, Clone, Debug, Eq, PartialEq)] +#[derive(Copy, Clone, Debug, Eq, Error, PartialEq)] pub enum SocketError { /// There is an existing connection. + #[error("There is an existing connection. Please close the current connection before attempting to connect again.")] ConnectionExists, /// The device is not connected to any peer. + #[error("The device is not connected to any peer. Please connect it to a peer first.")] NotConnected, /// Peer socket is shutdown. + #[error("The peer socket is shutdown.")] PeerSocketShutdown, /// The given buffer is shorter than expected. + #[error("The given buffer is shorter than expected")] BufferTooShort, /// The given buffer for output is shorter than expected. + #[error("The given output buffer is too short. '{0}' bytes is needed for the output buffer.")] OutputBufferTooShort(usize), /// The given buffer has exceeded the maximum buffer size. + #[error("The given buffer length '{0}' has exceeded the maximum allowed buffer length '{1}'")] BufferTooLong(usize, usize), /// Unknown operation. + #[error("The operation code '{0}' is unknown")] UnknownOperation(u16), /// Invalid operation, + #[error("Invalid operation")] InvalidOperation, /// Invalid number. + #[error("Invalid number")] InvalidNumber, /// Unexpected data in packet. + #[error("No data is expected in the packet")] UnexpectedDataInPacket, /// Peer has insufficient buffer space, try again later. + #[error("Peer has insufficient buffer space, try again later")] InsufficientBufferSpaceInPeer, /// Recycled a wrong buffer. + #[error("Recycled a wrong buffer")] RecycledWrongBuffer, } -impl fmt::Display for SocketError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match self { - Self::ConnectionExists => write!( - f, - "There is an existing connection. Please close the current connection before attempting to connect again."), - Self::NotConnected => write!(f, "The device is not connected to any peer. Please connect it to a peer first."), - Self::PeerSocketShutdown => write!(f, "The peer socket is shutdown."), - Self::BufferTooShort => write!(f, "The given buffer is shorter than expected"), - Self::BufferTooLong(actual, max) => { - write!(f, "The given buffer length '{actual}' has exceeded the maximum allowed buffer length '{max}'") - } - Self::OutputBufferTooShort(expected) => { - write!(f, "The given output buffer is too short. '{expected}' bytes is needed for the output buffer.") - } - Self::UnknownOperation(op) => { - write!(f, "The operation code '{op}' is unknown") - } - Self::InvalidOperation => write!(f, "Invalid operation"), - Self::InvalidNumber => write!(f, "Invalid number"), - Self::UnexpectedDataInPacket => write!(f, "No data is expected in the packet"), - Self::InsufficientBufferSpaceInPeer => write!(f, "Peer has insufficient buffer space, try again later"), - Self::RecycledWrongBuffer => write!(f, "Recycled a wrong buffer"), - } - } -} - pub type Result = result::Result; diff --git a/src/lib.rs b/src/lib.rs index e219fe95..4e1d29c2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -57,10 +57,9 @@ mod queue; pub mod transport; mod volatile; -use core::{ - fmt::{self, Display, Formatter}, - ptr::{self, NonNull}, -}; +use core::ptr::{self, NonNull}; +use device::socket::SocketError; +use thiserror::Error; pub use self::hal::{BufferDirection, Hal, PhysAddr}; @@ -71,30 +70,41 @@ pub const PAGE_SIZE: usize = 0x1000; pub type Result = core::result::Result; /// The error type of VirtIO drivers. -#[derive(Copy, Clone, Debug, Eq, PartialEq)] +#[derive(Copy, Clone, Debug, Eq, Error, PartialEq)] pub enum Error { /// There are not enough descriptors available in the virtqueue, try again later. + #[error("Virtqueue is full")] QueueFull, /// The device is not ready. + #[error("Device not ready")] NotReady, /// The device used a different descriptor chain to the one we were expecting. + #[error("Device used a different descriptor chain to the one we were expecting")] WrongToken, /// The queue is already in use. + #[error("Virtqueue is already in use")] AlreadyUsed, /// Invalid parameter. + #[error("Invalid parameter")] InvalidParam, - /// Failed to alloc DMA memory. + /// Failed to allocate DMA memory. + #[error("Failed to allocate DMA memory")] DmaError, - /// I/O Error + /// I/O error + #[error("I/O error")] IoError, /// The request was not supported by the device. + #[error("Request not supported by device")] Unsupported, /// The config space advertised by the device is smaller than the driver expected. + #[error("Config space advertised by the device is smaller than expected")] ConfigSpaceTooSmall, /// The device doesn't have any config space, but the driver expects some. + #[error("The device doesn't have any config space, but the driver expects some")] ConfigSpaceMissing, /// Error from the socket device. - SocketDeviceError(device::socket::SocketError), + #[error("Error from the socket device: {0}")] + SocketDeviceError(#[from] SocketError), } #[cfg(feature = "alloc")] @@ -104,41 +114,6 @@ impl From for Error { } } -impl Display for Error { - fn fmt(&self, f: &mut Formatter) -> fmt::Result { - match self { - Self::QueueFull => write!(f, "Virtqueue is full"), - Self::NotReady => write!(f, "Device not ready"), - Self::WrongToken => write!( - f, - "Device used a different descriptor chain to the one we were expecting" - ), - Self::AlreadyUsed => write!(f, "Virtqueue is already in use"), - Self::InvalidParam => write!(f, "Invalid parameter"), - Self::DmaError => write!(f, "Failed to allocate DMA memory"), - Self::IoError => write!(f, "I/O Error"), - Self::Unsupported => write!(f, "Request not supported by device"), - Self::ConfigSpaceTooSmall => write!( - f, - "Config space advertised by the device is smaller than expected" - ), - Self::ConfigSpaceMissing => { - write!( - f, - "The device doesn't have any config space, but the driver expects some" - ) - } - Self::SocketDeviceError(e) => write!(f, "Error from the socket device: {e:?}"), - } - } -} - -impl From for Error { - fn from(e: device::socket::SocketError) -> Self { - Self::SocketDeviceError(e) - } -} - /// Align `size` up to a page. fn align_up(size: usize) -> usize { (size + PAGE_SIZE) & !(PAGE_SIZE - 1) diff --git a/src/transport/mmio.rs b/src/transport/mmio.rs index 9c5bb4dd..434060e3 100644 --- a/src/transport/mmio.rs +++ b/src/transport/mmio.rs @@ -9,7 +9,6 @@ use crate::{ }; use core::{ convert::{TryFrom, TryInto}, - fmt::{self, Display, Formatter}, mem::{align_of, size_of}, ptr::NonNull, }; @@ -51,32 +50,19 @@ impl From for u32 { } /// An error encountered initialising a VirtIO MMIO transport. -#[derive(Clone, Debug, Eq, PartialEq)] +#[derive(Clone, Debug, Eq, Error, PartialEq)] pub enum MmioError { /// The header doesn't start with the expected magic value 0x74726976. + #[error("Invalid magic value {0:#010x} (expected 0x74726976)")] BadMagic(u32), /// The header reports a version number that is neither 1 (legacy) nor 2 (modern). + #[error("Unsupported Virtio MMIO version {0}")] UnsupportedVersion(u32), /// The header reports a device ID of 0. + #[error("Device ID was zero")] ZeroDeviceId, } -impl Display for MmioError { - fn fmt(&self, f: &mut Formatter) -> fmt::Result { - match self { - Self::BadMagic(magic) => write!( - f, - "Invalid magic value {:#010x} (expected 0x74726976).", - magic - ), - Self::UnsupportedVersion(version) => { - write!(f, "Unsupported Virtio MMIO version {}.", version) - } - Self::ZeroDeviceId => write!(f, "Device ID was zero."), - } - } -} - /// MMIO Device Register Interface, both legacy and modern. /// /// Ref: 4.2.2 MMIO Device Register Layout and 4.2.4 Legacy interface diff --git a/src/transport/pci.rs b/src/transport/pci.rs index d6afa03e..94b4cdfa 100644 --- a/src/transport/pci.rs +++ b/src/transport/pci.rs @@ -13,7 +13,6 @@ use crate::{ Error, }; use core::{ - fmt::{self, Display, Formatter}, mem::{align_of, size_of}, ptr::{addr_of_mut, NonNull}, }; @@ -431,26 +430,37 @@ fn get_bar_region_slice( } /// An error encountered initialising a VirtIO PCI transport. -#[derive(Clone, Debug, Eq, PartialEq)] +#[derive(Clone, Debug, Eq, Error, PartialEq)] pub enum VirtioPciError { /// PCI device vender ID was not the VirtIO vendor ID. + #[error("PCI device vender ID {0:#06x} was not the VirtIO vendor ID {VIRTIO_VENDOR_ID:#06x}.")] InvalidVendorId(u16), /// No valid `VIRTIO_PCI_CAP_COMMON_CFG` capability was found. + #[error("No valid `VIRTIO_PCI_CAP_COMMON_CFG` capability was found.")] MissingCommonConfig, /// No valid `VIRTIO_PCI_CAP_NOTIFY_CFG` capability was found. + #[error("No valid `VIRTIO_PCI_CAP_NOTIFY_CFG` capability was found.")] MissingNotifyConfig, /// `VIRTIO_PCI_CAP_NOTIFY_CFG` capability has a `notify_off_multiplier` that is not a multiple /// of 2. + #[error("`VIRTIO_PCI_CAP_NOTIFY_CFG` capability has a `notify_off_multiplier` that is not a multiple of 2: {0}")] InvalidNotifyOffMultiplier(u32), /// No valid `VIRTIO_PCI_CAP_ISR_CFG` capability was found. + #[error("No valid `VIRTIO_PCI_CAP_ISR_CFG` capability was found.")] MissingIsrConfig, /// An IO BAR was provided rather than a memory BAR. + #[error("Unexpected IO BAR (expected memory BAR).")] UnexpectedIoBar, /// A BAR which we need was not allocated an address. + #[error("Bar {0} not allocated.")] BarNotAllocated(u8), /// The offset for some capability was greater than the length of the BAR. + #[error("Capability offset greater than BAR length.")] BarOffsetOutOfRange, /// The virtual address was not aligned as expected. + #[error( + "Virtual address {vaddr:#018?} was not aligned to a {alignment} byte boundary as expected." + )] Misaligned { /// The virtual address in question. vaddr: NonNull, @@ -458,48 +468,10 @@ pub enum VirtioPciError { alignment: usize, }, /// A generic PCI error, + #[error(transparent)] Pci(PciError), } -impl Display for VirtioPciError { - fn fmt(&self, f: &mut Formatter) -> fmt::Result { - match self { - Self::InvalidVendorId(vendor_id) => write!( - f, - "PCI device vender ID {:#06x} was not the VirtIO vendor ID {:#06x}.", - vendor_id, VIRTIO_VENDOR_ID - ), - Self::MissingCommonConfig => write!( - f, - "No valid `VIRTIO_PCI_CAP_COMMON_CFG` capability was found." - ), - Self::MissingNotifyConfig => write!( - f, - "No valid `VIRTIO_PCI_CAP_NOTIFY_CFG` capability was found." - ), - Self::InvalidNotifyOffMultiplier(notify_off_multiplier) => { - write!( - f, - "`VIRTIO_PCI_CAP_NOTIFY_CFG` capability has a `notify_off_multiplier` that is not a multiple of 2: {}", - notify_off_multiplier - ) - } - Self::MissingIsrConfig => { - write!(f, "No valid `VIRTIO_PCI_CAP_ISR_CFG` capability was found.") - } - Self::UnexpectedIoBar => write!(f, "Unexpected IO BAR (expected memory BAR)."), - Self::BarNotAllocated(bar_index) => write!(f, "Bar {} not allocated.", bar_index), - Self::BarOffsetOutOfRange => write!(f, "Capability offset greater than BAR length."), - Self::Misaligned { vaddr, alignment } => write!( - f, - "Virtual address {:#018?} was not aligned to a {} byte boundary as expected.", - vaddr, alignment - ), - Self::Pci(pci_error) => pci_error.fmt(f), - } - } -} - impl From for VirtioPciError { fn from(error: PciError) -> Self { Self::Pci(error) diff --git a/src/transport/pci/bus.rs b/src/transport/pci/bus.rs index 04c38d74..cbf43271 100644 --- a/src/transport/pci/bus.rs +++ b/src/transport/pci/bus.rs @@ -7,6 +7,7 @@ use core::{ fmt::{self, Display, Formatter}, }; use log::warn; +use thiserror::Error; const INVALID_READ: u32 = 0xffffffff; @@ -82,20 +83,13 @@ bitflags! { } /// Errors accessing a PCI device. -#[derive(Copy, Clone, Debug, Eq, PartialEq)] +#[derive(Copy, Clone, Debug, Eq, Error, PartialEq)] pub enum PciError { /// The device reported an invalid BAR type. + #[error("Invalid PCI BAR type")] InvalidBarType, } -impl Display for PciError { - fn fmt(&self, f: &mut Formatter) -> fmt::Result { - match self { - Self::InvalidBarType => write!(f, "Invalid PCI BAR type."), - } - } -} - /// The root complex of a PCI bus. #[derive(Debug)] pub struct PciRoot {