From c5c74690aed780aad3adaca8b856e6662b103a87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=91=A8=E7=9D=BF?= Date: Mon, 13 Jul 2026 17:24:06 +0800 Subject: [PATCH] refactor(axdevice): replace errno contracts --- Cargo.lock | 9 +- virtualization/arm_vgic/Cargo.toml | 2 +- virtualization/arm_vgic/src/devops_impl.rs | 22 +- virtualization/arm_vgic/src/error.rs | 66 ++++ virtualization/arm_vgic/src/lib.rs | 3 + virtualization/arm_vgic/src/v3/gits.rs | 16 +- virtualization/arm_vgic/src/v3/utils.rs | 7 +- virtualization/arm_vgic/src/v3/vgicd.rs | 47 ++- virtualization/arm_vgic/src/v3/vgicr.rs | 16 +- virtualization/arm_vgic/src/vgic.rs | 9 +- .../arm_vgic/src/vtimer/cntp_ctl_el0.rs | 8 +- .../arm_vgic/src/vtimer/cntp_tval_el0.rs | 8 +- .../arm_vgic/src/vtimer/cntpct_el0.rs | 8 +- .../arm_vgic/tests/error_contract.rs | 49 +++ virtualization/axdevice/Cargo.toml | 2 +- virtualization/axdevice/src/device.rs | 349 +++++++++-------- virtualization/axdevice/src/error.rs | 132 +++++++ virtualization/axdevice/src/factory.rs | 48 ++- virtualization/axdevice/src/fw_cfg.rs | 61 ++- virtualization/axdevice/src/lib.rs | 2 + .../axdevice/src/loongarch_pch_pic.rs | 7 +- virtualization/axdevice/src/registration.rs | 5 +- .../axdevice/tests/error_contract.rs | 46 +++ virtualization/axdevice_base/Cargo.toml | 2 +- virtualization/axdevice_base/src/adapter.rs | 18 +- virtualization/axdevice_base/src/device.rs | 67 +++- virtualization/axdevice_base/src/irq.rs | 99 ++++- virtualization/axdevice_base/src/lib.rs | 40 +- .../axdevice_base/tests/error_contract.rs | 44 +++ virtualization/axdevice_base/tests/irq.rs | 52 ++- virtualization/axdevice_base/tests/test.rs | 13 +- virtualization/axvm/src/arch/aarch64/vm.rs | 22 +- virtualization/axvm/src/arch/riscv64/irq.rs | 58 +-- virtualization/axvm/src/arch/x86_64/port.rs | 13 +- virtualization/axvm/src/error.rs | 75 ++++ virtualization/axvm/src/irq/mod.rs | 42 +- virtualization/axvm/src/vm/mod.rs | 65 ++-- virtualization/axvm/src/vm/prepare.rs | 5 +- virtualization/axvm/src/vm/prepare/devices.rs | 5 +- virtualization/riscv_vplic/Cargo.toml | 2 +- virtualization/riscv_vplic/src/devops_impl.rs | 365 ++++++++++-------- virtualization/riscv_vplic/src/error.rs | 102 +++++ virtualization/riscv_vplic/src/lib.rs | 7 +- virtualization/riscv_vplic/src/utils.rs | 7 +- virtualization/riscv_vplic/src/vplic.rs | 42 +- .../riscv_vplic/tests/error_contract.rs | 36 ++ .../riscv_vplic/tests/vplic_tests.rs | 61 +-- .../virtualization-tests/Cargo.toml | 1 - .../virtualization-tests/tests/axdevice.rs | 217 ++++++----- .../virtualization-tests/tests/axvm_irq.rs | 45 ++- 50 files changed, 1653 insertions(+), 774 deletions(-) create mode 100644 virtualization/arm_vgic/src/error.rs create mode 100644 virtualization/arm_vgic/tests/error_contract.rs create mode 100644 virtualization/axdevice/src/error.rs create mode 100644 virtualization/axdevice/tests/error_contract.rs create mode 100644 virtualization/axdevice_base/tests/error_contract.rs create mode 100644 virtualization/riscv_vplic/src/error.rs create mode 100644 virtualization/riscv_vplic/tests/error_contract.rs diff --git a/Cargo.lock b/Cargo.lock index 085f5597eb..e127b78a51 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -426,7 +426,6 @@ dependencies = [ "aarch64-cpu 11.2.0", "aarch64_sysreg", "ax-crate-interface", - "ax-errno 0.6.1", "ax-kspin", "ax-memory-addr", "axdevice_base", @@ -434,6 +433,7 @@ dependencies = [ "bitmaps", "log", "spin", + "thiserror 2.0.18", "tock-registers 0.10.1", ] @@ -1242,7 +1242,6 @@ name = "axdevice" version = "0.5.4" dependencies = [ "arm_vgic", - "ax-errno 0.6.1", "ax-kspin", "ax-memory-addr", "axdevice_base", @@ -1253,6 +1252,7 @@ dependencies = [ "paste", "rand 0.10.1", "riscv_vplic", + "thiserror 2.0.18", "x86_vlapic", ] @@ -1260,11 +1260,11 @@ dependencies = [ name = "axdevice_base" version = "0.6.1" dependencies = [ - "ax-errno 0.6.1", "ax-memory-addr", "axvm-types", "log", "serde", + "thiserror 2.0.18", ] [[package]] @@ -6653,7 +6653,6 @@ name = "riscv_vplic" version = "0.4.20" dependencies = [ "ax-crate-interface", - "ax-errno 0.6.1", "ax-kspin", "ax-memory-addr", "axdevice_base", @@ -6661,6 +6660,7 @@ dependencies = [ "bitmaps", "log", "riscv-h", + "thiserror 2.0.18", ] [[package]] @@ -8973,7 +8973,6 @@ dependencies = [ name = "virtualization-tests" version = "0.1.0" dependencies = [ - "ax-errno 0.6.1", "ax-plat", "axdevice", "axdevice_base", diff --git a/virtualization/arm_vgic/Cargo.toml b/virtualization/arm_vgic/Cargo.toml index ded77cd4f1..b753b8fb60 100644 --- a/virtualization/arm_vgic/Cargo.toml +++ b/virtualization/arm_vgic/Cargo.toml @@ -28,9 +28,9 @@ axdevice_base = { workspace = true } axvm-types = { workspace = true } aarch64-cpu = "11.0" aarch64_sysreg = { workspace = true } -ax-errno = { workspace = true } bitmaps = {version = "3.2", default-features = false} log = "0.4" ax-memory-addr = { workspace = true } spin = { workspace = true } tock-registers = "0.10" +thiserror = { workspace = true } diff --git a/virtualization/arm_vgic/src/devops_impl.rs b/virtualization/arm_vgic/src/devops_impl.rs index 9e4cb4d720..ccea980517 100644 --- a/virtualization/arm_vgic/src/devops_impl.rs +++ b/virtualization/arm_vgic/src/devops_impl.rs @@ -12,8 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use ax_errno::AxResult; -use axdevice_base::{AccessWidth, BaseDeviceOps, DeviceAddrRange, EmuDeviceType}; +use axdevice_base::{AccessWidth, BaseDeviceOps, DeviceAddrRange, DeviceResult, EmuDeviceType}; use axvm_types::GuestPhysAddrRange; use crate::vgic::Vgic; @@ -52,32 +51,33 @@ impl BaseDeviceOps for Vgic { /// - `width`: The width of the data to be read, determining the size of the read operation. /// /// Returns: - /// - `AxResult`: The result of the read operation, including any errors and the size of the data read. + /// - `DeviceResult`: The result of the read operation, including any errors and the size of the data read. fn handle_read( &self, addr: ::Addr, width: AccessWidth, - ) -> AxResult { + ) -> DeviceResult { // Perform bitwise operation to ensure the address is aligned to byte boundaries let addr = addr.as_usize() & 0xfff; // Match different read operations based on the width parameter - match width { + let value = match width { AccessWidth::Byte => { // Handle 1-byte read - self.handle_read8(addr) + self.handle_read8(addr)? } AccessWidth::Word => { // Handle 2-byte read - self.handle_read16(addr) + self.handle_read16(addr)? } AccessWidth::Dword => { // Handle 4-byte read - self.handle_read32(addr) + self.handle_read32(addr)? } // Return success for unsupported widths without performing any operation - _ => Ok(0), - } + _ => 0, + }; + Ok(value) } /// Handles write operations of different widths. /// @@ -94,7 +94,7 @@ impl BaseDeviceOps for Vgic { addr: ::Addr, width: AccessWidth, val: usize, - ) -> AxResult { + ) -> DeviceResult { // Convert the physical address to a `usize` and apply a mask to ensure proper alignment let addr = addr.as_usize() & 0xfff; diff --git a/virtualization/arm_vgic/src/error.rs b/virtualization/arm_vgic/src/error.rs new file mode 100644 index 0000000000..d185530f27 --- /dev/null +++ b/virtualization/arm_vgic/src/error.rs @@ -0,0 +1,66 @@ +//! Typed errors reported by the virtual Generic Interrupt Controller. + +use alloc::string::String; + +use axdevice_base::{AccessWidth, DeviceError}; + +/// Result type returned by VGIC operations. +pub type VgicResult = Result; + +/// Errors reported by the virtual Generic Interrupt Controller. +#[derive(Clone, Debug, Eq, PartialEq, thiserror::Error)] +pub enum VgicError { + /// An IRQ identifier is outside the supported range. + #[error("VGIC IRQ {irq} is outside the supported range 0..{max}")] + InvalidIrq { + /// The rejected IRQ identifier. + irq: usize, + /// The exclusive upper bound for valid IRQ identifiers. + max: usize, + }, + /// A register access has an invalid address or width. + #[error("invalid VGIC {operation} at offset {offset:#x} with width {width:?}")] + InvalidAccess { + /// Whether the access is a read or write. + operation: &'static str, + /// Register offset from the controller base. + offset: usize, + /// Width of the register access. + width: AccessWidth, + }, + /// A register or controller operation is unsupported. + #[error("unsupported VGIC operation {operation}: {detail}")] + Unsupported { + /// The unsupported operation. + operation: &'static str, + /// Diagnostic detail describing the limitation. + detail: String, + }, + /// A host GIC or MMIO backend operation failed. + #[error("VGIC backend operation {operation} failed: {detail}")] + Backend { + /// The backend operation that failed. + operation: &'static str, + /// Diagnostic detail from the backend. + detail: String, + }, +} + +impl From for DeviceError { + fn from(error: VgicError) -> Self { + match error { + VgicError::InvalidIrq { .. } | VgicError::InvalidAccess { .. } => Self::InvalidInput { + operation: "access ARM VGIC", + detail: alloc::format!("{error}"), + }, + VgicError::Unsupported { .. } => Self::Unsupported { + operation: "access ARM VGIC", + detail: alloc::format!("{error}"), + }, + VgicError::Backend { .. } => Self::Backend { + operation: "access ARM VGIC", + detail: alloc::format!("{error}"), + }, + } + } +} diff --git a/virtualization/arm_vgic/src/lib.rs b/virtualization/arm_vgic/src/lib.rs index ee73c964e5..0a4120b50a 100644 --- a/virtualization/arm_vgic/src/lib.rs +++ b/virtualization/arm_vgic/src/lib.rs @@ -22,8 +22,11 @@ extern crate alloc; mod devops_impl; +mod error; pub mod host; +pub use error::{VgicError, VgicResult}; + /// Virtual GIC implementation module. pub mod vgic; pub use vgic::Vgic; diff --git a/virtualization/arm_vgic/src/v3/gits.rs b/virtualization/arm_vgic/src/v3/gits.rs index 7591b867e1..142de3a44f 100644 --- a/virtualization/arm_vgic/src/v3/gits.rs +++ b/virtualization/arm_vgic/src/v3/gits.rs @@ -18,7 +18,7 @@ use core::ptr; use ax_kspin::SpinNoIrq; use ax_memory_addr::PhysAddr; -use axdevice_base::{AccessWidth, BaseDeviceOps}; +use axdevice_base::{AccessWidth, BaseDeviceOps, DeviceResult}; use axvm_types::{GuestPhysAddr, GuestPhysAddrRange, HostPhysAddr}; use log::{debug, trace, warn}; use spin::Once; @@ -122,7 +122,7 @@ impl BaseDeviceOps for Gits { &self, addr: ::Addr, width: AccessWidth, - ) -> ax_errno::AxResult { + ) -> DeviceResult { let gits_base = self.host_gits_base; let reg = addr - self.addr; // let reg = mmio.address; @@ -136,7 +136,7 @@ impl BaseDeviceOps for Gits { ); // mmio_perform_access(gits_base, mmio); - match reg { + let result = match reg { GITS_CTRL => perform_mmio_read(gits_base + reg, width), GITS_CBASER => Ok(self.with_regs(|r| r.cbaser)), GITS_DT_BASER => { @@ -159,7 +159,8 @@ impl BaseDeviceOps for Gits { GITS_CREADR => Ok(self.with_regs(|r| r.creadr)), GITS_TYPER => perform_mmio_read(gits_base + reg, width), _ => perform_mmio_read(gits_base + reg, width), - } + }; + Ok(result?) } fn handle_write( @@ -167,7 +168,7 @@ impl BaseDeviceOps for Gits { addr: ::Addr, width: AccessWidth, val: usize, - ) -> ax_errno::AxResult { + ) -> DeviceResult { let gits_base = self.host_gits_base; let reg = addr - self.addr; // let reg = mmio.address; @@ -182,7 +183,7 @@ impl BaseDeviceOps for Gits { ); // mmio_perform_access(gits_base, mmio); - match reg { + let result = match reg { GITS_CTRL => perform_mmio_write(gits_base + reg, width, val), GITS_CBASER => { if self.is_root_vm { @@ -233,7 +234,8 @@ impl BaseDeviceOps for Gits { } GITS_TYPER => perform_mmio_write(gits_base + reg, width, val), _ => perform_mmio_write(gits_base + reg, width, val), - } + }; + Ok(result?) } } diff --git a/virtualization/arm_vgic/src/v3/utils.rs b/virtualization/arm_vgic/src/v3/utils.rs index 71133270da..cc3e1c388e 100644 --- a/virtualization/arm_vgic/src/v3/utils.rs +++ b/virtualization/arm_vgic/src/v3/utils.rs @@ -12,13 +12,12 @@ // See the License for the specific language governing permissions and // limitations under the License. -use ax_errno::AxResult; use axdevice_base::AccessWidth; use axvm_types::HostPhysAddr; -use crate::host; +use crate::{VgicResult, host}; -pub(crate) fn perform_mmio_read(addr: HostPhysAddr, width: AccessWidth) -> AxResult { +pub(crate) fn perform_mmio_read(addr: HostPhysAddr, width: AccessWidth) -> VgicResult { let addr = host::phys_to_virt(addr).as_ptr(); match width { @@ -33,7 +32,7 @@ pub(crate) fn perform_mmio_write( addr: HostPhysAddr, width: AccessWidth, val: usize, -) -> AxResult<()> { +) -> VgicResult<()> { let addr = host::phys_to_virt(addr).as_mut_ptr(); match width { diff --git a/virtualization/arm_vgic/src/v3/vgicd.rs b/virtualization/arm_vgic/src/v3/vgicd.rs index cbf1122625..5915430f5a 100644 --- a/virtualization/arm_vgic/src/v3/vgicd.rs +++ b/virtualization/arm_vgic/src/v3/vgicd.rs @@ -12,9 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -use ax_errno::AxResult; use ax_kspin::SpinNoIrq; -use axdevice_base::{AccessWidth, BaseDeviceOps, EmuDeviceType}; +use axdevice_base::{AccessWidth, BaseDeviceOps, DeviceResult, EmuDeviceType}; use axvm_types::{GuestPhysAddr, GuestPhysAddrRange, HostPhysAddr}; use bitmaps::Bitmap; use log::debug; @@ -23,7 +22,7 @@ use super::{ registers::*, utils::{perform_mmio_read, perform_mmio_write}, }; -use crate::host; +use crate::{VgicError, VgicResult, host}; /// Default size for GICD region. pub const DEFAULT_GICD_SIZE: usize = 0x10000; // 64K @@ -47,6 +46,17 @@ pub struct VGicD { } impl VGicD { + /// Validates that an IRQ identifier can be represented by the VGIC. + pub fn validate_irq(irq: u32) -> VgicResult { + if irq >= MAX_IRQ_V3 as u32 { + return Err(VgicError::InvalidIrq { + irq: irq as usize, + max: MAX_IRQ_V3, + }); + } + Ok(()) + } + /// Creates a new VGicD instance. pub fn new(addr: GuestPhysAddr, size: Option) -> Self { let size = size.unwrap_or(DEFAULT_GICD_SIZE); @@ -60,15 +70,18 @@ impl VGicD { } /// Assigns an IRQ to a specific CPU. - pub fn assign_irq(&self, irq: u32, cpu_phys_id: usize, target_cpu_affinity: (u8, u8, u8, u8)) { + pub fn assign_irq( + &self, + irq: u32, + cpu_phys_id: usize, + target_cpu_affinity: (u8, u8, u8, u8), + ) -> VgicResult { debug!( "Physically assigning IRQ {irq} to CPU {cpu_phys_id} with affinity \ {target_cpu_affinity:?}" ); - if irq >= MAX_IRQ_V3 as u32 { - panic!("IRQ {} is out of range for VGicD", irq); - } + Self::validate_irq(irq)?; self.assigned_irqs.lock().set(irq as usize, true); // TODO: update host GICD_ITARGETSR and GICD_IROUTER registers @@ -80,7 +93,6 @@ impl VGicD { 1u8 << (cpu_phys_id), ); } - let gicd_irouter_paddr = self.host_gicd_addr + GICD_IROUTER + (irq as usize) * 8; let gicd_irouter_vaddr = host::phys_to_virt(gicd_irouter_paddr); unsafe { @@ -93,6 +105,7 @@ impl VGicD { | target_cpu_affinity.3 as u64, ); } + Ok(()) } } @@ -109,13 +122,13 @@ impl BaseDeviceOps for VGicD { &self, addr: ::Addr, width: AccessWidth, - ) -> ax_errno::AxResult { + ) -> DeviceResult { let gicd_base = self.host_gicd_addr; let reg = addr - self.addr; debug!("vGICD read reg {reg:#x} width {width:?}"); - match reg { + let result = match reg { reg if GICD_IROUTER_RANGE.contains(®) => { let irq = (reg - GICD_IROUTER) as u32 / 8; @@ -172,7 +185,8 @@ impl BaseDeviceOps for VGicD { _ => { todo!("vgicdv3 read unimplemented for reg {:#x}", reg); } - } + }; + Ok(result?) } fn handle_write( @@ -180,13 +194,13 @@ impl BaseDeviceOps for VGicD { addr: ::Addr, width: AccessWidth, val: usize, - ) -> ax_errno::AxResult { + ) -> DeviceResult { let gicd_base = self.host_gicd_addr; let reg = addr - self.addr; debug!("vGICD write reg {reg:#x} width {width:?} val {val:#x}"); - match reg { + let result = match reg { reg if GICD_IROUTER_RANGE.contains(®) => { let irq = (reg - GICD_IROUTER) as u32 / 8; @@ -243,7 +257,8 @@ impl BaseDeviceOps for VGicD { _ => { todo!("vgicdv3 write unimplemented for reg {:#x}", reg); } - } + }; + Ok(result?) } } @@ -305,7 +320,7 @@ impl VGicD { bits_per_irq_shift: usize, width: AccessWidth, _is_poke: bool, - ) -> AxResult { + ) -> VgicResult { let mask = self.irq_access_mask(reg_offset, bits_per_irq_shift, width); Ok(perform_mmio_read(self.host_gicd_addr + offset, width)? & mask) @@ -320,7 +335,7 @@ impl VGicD { width: AccessWidth, is_poke: bool, val: usize, - ) -> AxResult<()> { + ) -> VgicResult<()> { let mask = self.irq_access_mask(reg_offset, bits_per_irq_shift, width); if is_poke { diff --git a/virtualization/arm_vgic/src/v3/vgicr.rs b/virtualization/arm_vgic/src/v3/vgicr.rs index f34ac2bb0a..bef2bef13a 100644 --- a/virtualization/arm_vgic/src/v3/vgicr.rs +++ b/virtualization/arm_vgic/src/v3/vgicr.rs @@ -16,7 +16,7 @@ use core::ptr; use ax_kspin::SpinNoIrq; use ax_memory_addr::PhysAddr; -use axdevice_base::{AccessWidth, BaseDeviceOps}; +use axdevice_base::{AccessWidth, BaseDeviceOps, DeviceResult}; use axvm_types::{GuestPhysAddr, GuestPhysAddrRange, HostPhysAddr}; use log::{debug, trace}; use spin::Once; @@ -91,7 +91,7 @@ impl BaseDeviceOps for VGicR { &self, addr: ::Addr, width: AccessWidth, - ) -> ax_errno::AxResult { + ) -> DeviceResult { let gicr_base = self.host_gicr_base_this_cpu; let reg = addr - self.addr; @@ -100,7 +100,7 @@ impl BaseDeviceOps for VGicR { self.cpu_id, self.addr, reg, width ); - match reg { + let result = match reg { GICR_CTLR => { // TODO: is cross vcpu access allowed? perform_mmio_read(gicr_base + reg, width) @@ -152,7 +152,8 @@ impl BaseDeviceOps for VGicR { _ => { todo!("vgicr read unimplemented for reg {:#x}", reg); } - } + }; + Ok(result?) } fn handle_write( @@ -160,7 +161,7 @@ impl BaseDeviceOps for VGicR { addr: ::Addr, width: AccessWidth, value: usize, - ) -> ax_errno::AxResult<()> { + ) -> DeviceResult<()> { let gicr_base = self.host_gicr_base_this_cpu; let reg = addr - self.addr; @@ -169,7 +170,7 @@ impl BaseDeviceOps for VGicR { self.cpu_id, self.addr, reg, width, value ); - match reg { + let result = match reg { GICR_CTLR => { // TODO: is cross zone access allowed? perform_mmio_write(gicr_base + reg, width, value) @@ -216,7 +217,8 @@ impl BaseDeviceOps for VGicR { _ => { todo!("vgicr write unimplemented for reg {:#x}", reg); } - } + }; + Ok(result?) } } diff --git a/virtualization/arm_vgic/src/vgic.rs b/virtualization/arm_vgic/src/vgic.rs index a03f64d738..8aed351d50 100644 --- a/virtualization/arm_vgic/src/vgic.rs +++ b/virtualization/arm_vgic/src/vgic.rs @@ -12,10 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. -use ax_errno::AxResult; use ax_kspin::SpinNoIrq as Mutex; -use crate::{host, interrupt::VgicInt, registers::GicRegister, vgicd::Vgicd}; +use crate::{VgicResult, host, interrupt::VgicInt, registers::GicRegister, vgicd::Vgicd}; /// Virtual Generic Interrupt Controller. /// @@ -37,18 +36,18 @@ impl Vgic { vgicd: Mutex::new(Vgicd::new()), } } - pub(crate) fn handle_read8(&self, addr: usize) -> AxResult { + pub(crate) fn handle_read8(&self, addr: usize) -> VgicResult { let value = self.handle_read32(addr)?; Ok((value >> (8 * (addr & 0x3))) & 0xff) } - pub(crate) fn handle_read16(&self, addr: usize) -> AxResult { + pub(crate) fn handle_read16(&self, addr: usize) -> VgicResult { let value = self.handle_read32(addr)?; Ok((value >> (8 * (addr & 0x3))) & 0xffff) } /// Handles 32-bit read access to VGIC registers. - pub fn handle_read32(&self, addr: usize) -> AxResult { + pub fn handle_read32(&self, addr: usize) -> VgicResult { match GicRegister::from_addr(addr as u32) { Some(reg) => match reg { GicRegister::GicdCtlr => Ok(self.vgicd.lock().ctrlr as usize), diff --git a/virtualization/arm_vgic/src/vtimer/cntp_ctl_el0.rs b/virtualization/arm_vgic/src/vtimer/cntp_ctl_el0.rs index 9e74adac83..389c8709e4 100644 --- a/virtualization/arm_vgic/src/vtimer/cntp_ctl_el0.rs +++ b/virtualization/arm_vgic/src/vtimer/cntp_ctl_el0.rs @@ -13,9 +13,9 @@ // limitations under the License. use aarch64_sysreg::SystemRegType; -use ax_errno::AxResult; use axdevice_base::{ - AccessWidth, BaseDeviceOps, DeviceAddrRange, EmuDeviceType, SysRegAddr, SysRegAddrRange, + AccessWidth, BaseDeviceOps, DeviceAddrRange, DeviceResult, EmuDeviceType, SysRegAddr, + SysRegAddrRange, }; use log::info; @@ -35,7 +35,7 @@ impl BaseDeviceOps for SysCntpCtlEl0 { &self, _addr: ::Addr, _width: AccessWidth, - ) -> AxResult { + ) -> DeviceResult { Ok(0) } @@ -44,7 +44,7 @@ impl BaseDeviceOps for SysCntpCtlEl0 { addr: ::Addr, _width: AccessWidth, val: usize, - ) -> AxResult { + ) -> DeviceResult { info!("Write to emulator register: {addr:?}, value: {val}"); Ok(()) } diff --git a/virtualization/arm_vgic/src/vtimer/cntp_tval_el0.rs b/virtualization/arm_vgic/src/vtimer/cntp_tval_el0.rs index d639ed9c66..5a6fd0005d 100644 --- a/virtualization/arm_vgic/src/vtimer/cntp_tval_el0.rs +++ b/virtualization/arm_vgic/src/vtimer/cntp_tval_el0.rs @@ -18,9 +18,9 @@ use alloc::boxed::Box; use core::time::Duration; use aarch64_sysreg::SystemRegType; -use ax_errno::AxResult; use axdevice_base::{ - AccessWidth, BaseDeviceOps, DeviceAddrRange, EmuDeviceType, SysRegAddr, SysRegAddrRange, + AccessWidth, BaseDeviceOps, DeviceAddrRange, DeviceResult, EmuDeviceType, SysRegAddr, + SysRegAddrRange, }; use log::info; @@ -42,7 +42,7 @@ impl BaseDeviceOps for SysCntpTvalEl0 { &self, _addr: ::Addr, _width: AccessWidth, - ) -> AxResult { + ) -> DeviceResult { todo!() } @@ -51,7 +51,7 @@ impl BaseDeviceOps for SysCntpTvalEl0 { addr: ::Addr, _width: AccessWidth, val: usize, - ) -> AxResult { + ) -> DeviceResult { info!("Write to emulator register: {addr:?}, value: {val}"); let now = host::current_time_nanos(); info!("Current time: {}, deadline: {}", now, now + val as u64); diff --git a/virtualization/arm_vgic/src/vtimer/cntpct_el0.rs b/virtualization/arm_vgic/src/vtimer/cntpct_el0.rs index e4e0618235..ff5ae42aaf 100644 --- a/virtualization/arm_vgic/src/vtimer/cntpct_el0.rs +++ b/virtualization/arm_vgic/src/vtimer/cntpct_el0.rs @@ -14,9 +14,9 @@ use aarch64_cpu::registers::{CNTPCT_EL0, Readable}; use aarch64_sysreg::SystemRegType; -use ax_errno::AxResult; use axdevice_base::{ - AccessWidth, BaseDeviceOps, DeviceAddrRange, EmuDeviceType, SysRegAddr, SysRegAddrRange, + AccessWidth, BaseDeviceOps, DeviceAddrRange, DeviceResult, EmuDeviceType, SysRegAddr, + SysRegAddrRange, }; use log::info; @@ -36,7 +36,7 @@ impl BaseDeviceOps for SysCntpctEl0 { &self, _addr: ::Addr, _width: AccessWidth, - ) -> AxResult { + ) -> DeviceResult { Ok(CNTPCT_EL0.get() as usize) } @@ -45,7 +45,7 @@ impl BaseDeviceOps for SysCntpctEl0 { addr: ::Addr, _width: AccessWidth, val: usize, - ) -> AxResult { + ) -> DeviceResult { info!("Write to emulator register: {addr:?}, value: {val}"); Ok(()) } diff --git a/virtualization/arm_vgic/tests/error_contract.rs b/virtualization/arm_vgic/tests/error_contract.rs new file mode 100644 index 0000000000..d3358dadec --- /dev/null +++ b/virtualization/arm_vgic/tests/error_contract.rs @@ -0,0 +1,49 @@ +use std::{fs, path::Path}; + +use arm_vgic::VgicError; +use axdevice_base::{AccessWidth, DeviceError}; + +#[test] +fn crate_uses_typed_errors_without_errno_contracts() { + let crate_dir = Path::new(env!("CARGO_MANIFEST_DIR")); + let manifest = fs::read_to_string(crate_dir.join("Cargo.toml")).unwrap(); + assert!(!manifest.contains(&["ax", "errno"].join("-"))); + assert_directory_excludes(&crate_dir.join("src"), &["ax", "errno"].join("_")); +} + +#[test] +fn invalid_vgic_access_converts_to_device_input_error() { + let error = VgicError::InvalidAccess { + operation: "read", + offset: 0x80, + width: AccessWidth::Qword, + }; + let device: DeviceError = error.into(); + + assert!(matches!(device, DeviceError::InvalidInput { .. })); + assert!(device.to_string().contains("0x80")); +} + +#[cfg(feature = "vgicv3")] +#[test] +fn vgic_rejects_out_of_range_irq_without_panicking() { + assert!(matches!( + arm_vgic::v3::vgicd::VGicD::validate_irq(1024), + Err(VgicError::InvalidIrq { + irq: 1024, + max: 1024, + }) + )); +} + +fn assert_directory_excludes(directory: &Path, forbidden: &str) { + for entry in fs::read_dir(directory).unwrap() { + let path = entry.unwrap().path(); + if path.is_dir() { + assert_directory_excludes(&path, forbidden); + } else if path.extension().is_some_and(|extension| extension == "rs") { + let source = fs::read_to_string(&path).unwrap(); + assert!(!source.contains(forbidden), "{}", path.display()); + } + } +} diff --git a/virtualization/axdevice/Cargo.toml b/virtualization/axdevice/Cargo.toml index d94993a522..cf99ce9816 100644 --- a/virtualization/axdevice/Cargo.toml +++ b/virtualization/axdevice/Cargo.toml @@ -20,10 +20,10 @@ cfg-if = "1.0" ax-kspin.workspace = true # System independent crates provided by ArceOS. -ax-errno = { workspace = true } ax-memory-addr = { workspace = true } axdevice_base = { workspace = true } axvm-types = { workspace = true } +thiserror = { workspace = true } [target.'cfg(target_arch = "aarch64")'.dependencies] arm_vgic = { workspace = true, features = ["vgicv3"] } [target.'cfg(target_arch = "riscv64")'.dependencies] diff --git a/virtualization/axdevice/src/device.rs b/virtualization/axdevice/src/device.rs index 64fb16c3cd..9e2156a6d3 100644 --- a/virtualization/axdevice/src/device.rs +++ b/virtualization/axdevice/src/device.rs @@ -17,7 +17,6 @@ use core::ops::Range; #[cfg(target_arch = "aarch64")] use arm_vgic::Vgic; -use ax_errno::{AxResult, ax_err, ax_err_type}; use ax_kspin::SpinNoIrq as Mutex; #[cfg(target_arch = "aarch64")] use ax_memory_addr::PhysAddr; @@ -34,8 +33,8 @@ use riscv_vplic::VPlicGlobal; use x86_vlapic::{IoApicEoi, IoApicInterrupt}; use crate::{ - AxVmDeviceConfig, DeviceBuildContext, DeviceBundle, DeviceFactoryRegistry, FwCfg, - PollableDeviceOps, range_alloc::RangeAllocator, + AxVmDeviceConfig, DeviceBuildContext, DeviceBundle, DeviceFactoryRegistry, DeviceManagerError, + DeviceManagerResult, FwCfg, PollableDeviceOps, range_alloc::RangeAllocator, }; #[cfg(target_arch = "loongarch64")] use crate::{LoongArchPchPic, PchPicOutputEvent}; @@ -114,7 +113,7 @@ impl AxVmDevices { } /// According AxVmDeviceConfig to init the AxVmDevices - pub fn new(config: AxVmDeviceConfig) -> AxResult { + pub fn new(config: AxVmDeviceConfig) -> DeviceManagerResult { let mut this = Self::empty(); Self::init(&mut this, &config.emu_configs)?; @@ -126,7 +125,7 @@ impl AxVmDevices { config: AxVmDeviceConfig, factories: &DeviceFactoryRegistry, context: &DeviceBuildContext<'_>, - ) -> AxResult { + ) -> DeviceManagerResult { let mut this = Self::empty(); for config in &config.emu_configs { if factories.get(config.emu_type).is_some() { @@ -134,13 +133,13 @@ impl AxVmDevices { } else if Self::is_legacy_fallback(config.emu_type) { Self::init(&mut this, core::slice::from_ref(config))?; } else { - return ax_err!( - Unsupported, - format_args!( + return Err(DeviceManagerError::Unsupported { + operation: "build emulated device", + detail: format!( "no factory is registered for emulated device '{}' of type {}", config.name, config.emu_type - ) - ); + ), + }); } } Ok(this) @@ -152,7 +151,7 @@ impl AxVmDevices { config: &EmulatedDeviceConfig, factories: &DeviceFactoryRegistry, context: &DeviceBuildContext<'_>, - ) -> AxResult { + ) -> DeviceManagerResult { let bundle = factories.build(config, context)?; self.register_bundle(bundle) } @@ -174,8 +173,24 @@ impl AxVmDevices { ) } + #[cfg(any(target_arch = "aarch64", target_arch = "riscv64"))] + fn config_argument( + config: &EmulatedDeviceConfig, + index: usize, + expected: &'static str, + ) -> DeviceManagerResult { + config + .cfg_list + .get(index) + .copied() + .ok_or_else(|| DeviceManagerError::InvalidConfig { + operation: "initialize emulated device", + detail: format!("device '{}' requires {expected}", config.name), + }) + } + /// According the emu_configs to init every specific device - fn init(this: &mut Self, emu_configs: &[EmulatedDeviceConfig]) -> AxResult { + fn init(this: &mut Self, emu_configs: &[EmulatedDeviceConfig]) -> DeviceManagerResult { for config in emu_configs { match config.emu_type { EmulatedDeviceType::InterruptController => { @@ -184,10 +199,7 @@ impl AxVmDevices { #[allow(clippy::arc_with_non_send_sync)] this.register( MmioDeviceAdapter::from_arc(Arc::new(Vgic::new())) as Arc - ) - .map_err(|e| { - ax_err_type!(InvalidInput, alloc::format!("register vgic: {e:?}")) - })?; + )?; } #[cfg(not(target_arch = "aarch64"))] { @@ -200,24 +212,11 @@ impl AxVmDevices { EmulatedDeviceType::GPPTRedistributor => { #[cfg(target_arch = "aarch64")] { - const GPPT_GICR_ARG_ERR_MSG: &str = - "expect 3 args for gppt redistributor (cpu_num, stride, pcpu_id)"; - - let cpu_num = config - .cfg_list - .first() - .copied() - .expect(GPPT_GICR_ARG_ERR_MSG); - let stride = config - .cfg_list - .get(1) - .copied() - .expect(GPPT_GICR_ARG_ERR_MSG); - let pcpu_id = config - .cfg_list - .get(2) - .copied() - .expect(GPPT_GICR_ARG_ERR_MSG); + const GPPT_GICR_ARGS: &str = "three arguments (cpu_num, stride, pcpu_id)"; + + let cpu_num = Self::config_argument(config, 0, GPPT_GICR_ARGS)?; + let stride = Self::config_argument(config, 1, GPPT_GICR_ARGS)?; + let pcpu_id = Self::config_argument(config, 2, GPPT_GICR_ARGS)?; for i in 0..cpu_num { let addr = config.base_gpa + i * stride; @@ -229,13 +228,7 @@ impl AxVmDevices { Some(size), pcpu_id + i, ), - )) as Arc) - .map_err(|e| { - ax_err_type!( - InvalidInput, - alloc::format!("register gicr: {e:?}") - ) - })?; + )) as Arc)?; info!( "GPPT Redistributor initialized for vCPU {i} with base GPA \ @@ -260,10 +253,7 @@ impl AxVmDevices { config.base_gpa.into(), Some(config.length), ), - )) as Arc) - .map_err(|e| { - ax_err_type!(InvalidInput, alloc::format!("register gicd: {e:?}")) - })?; + )) as Arc)?; info!( "GPPT Distributor initialized with base GPA {base_gpa:#x} and length \ @@ -283,12 +273,9 @@ impl AxVmDevices { EmulatedDeviceType::GPPTITS => { #[cfg(target_arch = "aarch64")] { - let host_gits_base = config - .cfg_list - .first() - .copied() - .map(PhysAddr::from_usize) - .expect("expect 1 arg for gppt its (host_gits_base)"); + let host_gits_base = + Self::config_argument(config, 0, "one argument (host_gits_base)") + .map(PhysAddr::from_usize)?; #[allow(clippy::arc_with_non_send_sync)] this.register(MmioDeviceAdapter::from_arc(Arc::new( @@ -298,10 +285,7 @@ impl AxVmDevices { host_gits_base, false, ), - )) as Arc) - .map_err(|e| { - ax_err_type!(InvalidInput, alloc::format!("register gits: {e:?}")) - })?; + )) as Arc)?; info!( "GPPT ITS initialized with base GPA {base_gpa:#x} and length \ @@ -322,19 +306,22 @@ impl AxVmDevices { EmulatedDeviceType::PPPTGlobal => { #[cfg(target_arch = "riscv64")] { - let context_num = config - .cfg_list - .first() - .copied() - .expect("expect 1 arg for pppt global (context_num)"); - this.register(MmioDeviceAdapter::from_arc(Arc::new(VPlicGlobal::new( + let context_num = + Self::config_argument(config, 0, "one argument (context_num)")?; + let vplic = VPlicGlobal::new( config.base_gpa.into(), Some(config.length), context_num, - ))) as Arc) - .map_err(|e| { - ax_err_type!(InvalidInput, alloc::format!("register pppt: {e:?}")) - })?; + ) + .map_err(|error| { + DeviceManagerError::InvalidConfig { + operation: "initialize virtual PLIC", + detail: format!("device '{}': {error}", config.name), + } + })?; + this.register( + MmioDeviceAdapter::from_arc(Arc::new(vplic)) as Arc + )?; // PLIC Partial Passthrough Global. info!( "Partial PLIC Passthrough Global initialized with base GPA {:#x} and \ @@ -395,13 +382,7 @@ impl AxVmDevices { let pch_pic = Arc::new(LoongArchPchPic::new(config.base_gpa.into(), config.length)); this.register(MmioDeviceAdapter::from_arc(pch_pic.clone()) - as Arc) - .map_err(|e| { - ax_err_type!( - InvalidInput, - format!("register loongarch pch-pic: {e:?}") - ) - })?; + as Arc)?; this.loongarch_pch_pic = Some(pch_pic); info!( "LoongArch PCH-PIC initialized with base GPA {:#x} and length {:#x}", @@ -448,12 +429,18 @@ impl AxVmDevices { } /// Allocates an IVC (Inter-VM Communication) channel of the specified size. - pub fn alloc_ivc_channel(&self, size: usize) -> AxResult { + pub fn alloc_ivc_channel(&self, size: usize) -> DeviceManagerResult { if size == 0 { - return ax_err!(InvalidInput, "Size must be greater than 0"); + return Err(DeviceManagerError::InvalidInput { + operation: "allocate IVC channel", + detail: "size must be greater than zero".into(), + }); } if !is_aligned_4k(size) { - return ax_err!(InvalidInput, "Size must be aligned to 4K"); + return Err(DeviceManagerError::InvalidInput { + operation: "allocate IVC channel", + detail: format!("size {size:#x} is not aligned to 4 KiB"), + }); } if let Some(allocator) = &self.ivc_channel { @@ -462,24 +449,35 @@ impl AxVmDevices { .allocate_range(size) .ok_or_else(|| { warn!("Failed to allocate IVC channel range with size {size:#x}"); - ax_errno::ax_err_type!(NoMemory, "IVC channel allocation failed") + DeviceManagerError::OutOfMemory { + operation: "allocate IVC channel", + } }) .map(|range| { debug!("Allocated IVC channel range: {range:x?}"); GuestPhysAddr::from_usize(range.start) }) } else { - ax_err!(InvalidInput, "IVC channel not exists") + Err(DeviceManagerError::ResourceNotFound { + operation: "allocate IVC channel", + resource: "IVC channel allocator".into(), + }) } } /// Releases an IVC channel at the specified address and size. - pub fn release_ivc_channel(&self, addr: GuestPhysAddr, size: usize) -> AxResult { + pub fn release_ivc_channel(&self, addr: GuestPhysAddr, size: usize) -> DeviceManagerResult { if size == 0 { - return ax_err!(InvalidInput, "Size must be greater than 0"); + return Err(DeviceManagerError::InvalidInput { + operation: "release IVC channel", + detail: "size must be greater than zero".into(), + }); } if !is_aligned_4k(size) { - return ax_err!(InvalidInput, "Size must be aligned to 4K"); + return Err(DeviceManagerError::InvalidInput { + operation: "release IVC channel", + detail: format!("size {size:#x} is not aligned to 4 KiB"), + }); } if let Some(allocator) = &self.ivc_channel { @@ -488,17 +486,23 @@ impl AxVmDevices { debug!("Released IVC channel range: {range:x?}"); Ok(()) } else { - ax_err!(InvalidInput, "Invalid IVC channel range") + Err(DeviceManagerError::InvalidInput { + operation: "release IVC channel", + detail: format!("range {range:x?} is not allocated"), + }) } } else { - ax_err!(InvalidInput, "IVC channel not exists") + Err(DeviceManagerError::ResourceNotFound { + operation: "release IVC channel", + resource: "IVC channel allocator".into(), + }) } } /// Registers a bundle atomically. If any device fails to register, /// already-registered devices in this bundle are rolled back via /// `pop()` + index-key removal. - pub fn register_bundle(&mut self, bundle: DeviceBundle) -> AxResult { + pub fn register_bundle(&mut self, bundle: DeviceBundle) -> DeviceManagerResult { for (index, pollable) in bundle.pollable.iter().enumerate() { if self .pollable_devices @@ -506,10 +510,10 @@ impl AxVmDevices { .chain(bundle.pollable[..index].iter()) .any(|existing| Arc::ptr_eq(existing, pollable)) { - return ax_err!( - AlreadyExists, - "failed to register pollable device: the same capability is already registered" - ); + return Err(DeviceManagerError::ResourceConflict { + operation: "register pollable device", + detail: "the same pollable capability is already registered".into(), + }); } } @@ -535,14 +539,7 @@ impl AxVmDevices { } } } - let kind = match &e { - RegistryError::AddressConflict { .. } => ax_errno::AxError::AddrInUse, - _ => ax_errno::AxError::InvalidInput, - }; - return Err(ax_err_type!( - kind, - format!("device registration failed: {e:?}") - )); + return Err(e.into()); } } } @@ -941,46 +938,42 @@ impl AxVmDevices { /// Add an x86 IOAPIC device to the generic registry and x86 runtime handle. #[cfg(target_arch = "x86_64")] - pub fn add_x86_ioapic_dev(&mut self, dev: Arc) -> AxResult + pub fn add_x86_ioapic_dev(&mut self, dev: Arc) -> DeviceManagerResult where D: Device + X86IoApicDeviceOps + 'static, { - self.register(dev.clone() as Arc) - .map_err(|e| ax_err_type!(InvalidInput, format!("register x86 ioapic: {e:?}")))?; + self.register(dev.clone() as Arc)?; self.x86_ioapic = Some(dev); Ok(()) } /// Add an x86 PIT device to the generic registry and x86 runtime handle. #[cfg(target_arch = "x86_64")] - pub fn add_x86_pit_dev(&mut self, dev: Arc) -> AxResult + pub fn add_x86_pit_dev(&mut self, dev: Arc) -> DeviceManagerResult where D: Device + X86PitDeviceOps + 'static, { - self.register(dev.clone() as Arc) - .map_err(|e| ax_err_type!(InvalidInput, format!("register x86 pit: {e:?}")))?; + self.register(dev.clone() as Arc)?; self.x86_pit = Some(dev); Ok(()) } /// Add an x86 COM1 device to the generic registry and x86 runtime handle. #[cfg(target_arch = "x86_64")] - pub fn add_x86_serial_dev(&mut self, dev: Arc) -> AxResult + pub fn add_x86_serial_dev(&mut self, dev: Arc) -> DeviceManagerResult where D: Device + X86SerialDeviceOps + 'static, { - self.register(dev.clone() as Arc) - .map_err(|e| ax_err_type!(InvalidInput, format!("register x86 serial: {e:?}")))?; + self.register(dev.clone() as Arc)?; self.x86_serial = Some(dev); Ok(()) } /// Add a QEMU fw_cfg MMIO device to the device list. - pub fn add_fw_cfg_dev(&mut self, dev: Arc) -> AxResult { + pub fn add_fw_cfg_dev(&mut self, dev: Arc) -> DeviceManagerResult { self.register( MmioDeviceAdapter::from_arc(dev.clone()) as Arc - ) - .map_err(|e| ax_err_type!(InvalidInput, format!("register fw_cfg: {e:?}")))?; + )?; self.fw_cfg = Some(dev); Ok(()) } @@ -1052,7 +1045,11 @@ impl AxVmDevices { // ─── Hot-path dispatch handlers ───────────────────────────────── /// Handle the MMIO read by GuestPhysAddr and data width. - pub fn handle_mmio_read(&self, addr: GuestPhysAddr, width: AccessWidth) -> AxResult { + pub fn handle_mmio_read( + &self, + addr: GuestPhysAddr, + width: AccessWidth, + ) -> DeviceManagerResult { let access = BusAccess { kind: BusKind::Mmio, is_read: true, @@ -1060,15 +1057,20 @@ impl AxVmDevices { width, data: 0, }; - match self.dispatch(&access) { - Ok(BusResponse::Read { value }) => Ok(value as usize), - Ok(BusResponse::Write) => { - Err(ax_err_type!(BadState, "expected read response, got write")) - } - Err(err) => { - error!("emu_device mmio read failed: {err:?} at {addr:#x} width {width:?}"); - Err(ax_err_type!(BadState, format!("mmio read: {err:?}"))) - } + match self + .dispatch(&access) + .map_err(|source| DeviceManagerError::Access { + operation: "read", + bus: BusKind::Mmio, + addr: access.addr, + width, + source, + })? { + BusResponse::Read { value } => Ok(value as usize), + BusResponse::Write => Err(DeviceManagerError::UnexpectedResponse { + operation: "read MMIO device", + detail: "device returned a write acknowledgement".into(), + }), } } @@ -1078,7 +1080,7 @@ impl AxVmDevices { addr: GuestPhysAddr, width: AccessWidth, val: usize, - ) -> AxResult { + ) -> DeviceManagerResult { let access = BusAccess { kind: BusKind::Mmio, is_read: false, @@ -1086,15 +1088,23 @@ impl AxVmDevices { width, data: val as u64, }; - if let Err(err) = self.dispatch(&access) { - error!("emu_device mmio write failed: {err:?} at {addr:#x} width {width:?}"); - return Err(ax_err_type!(BadState, format!("mmio write: {err:?}"))); - } + self.dispatch(&access) + .map_err(|source| DeviceManagerError::Access { + operation: "write", + bus: BusKind::Mmio, + addr: access.addr, + width, + source, + })?; Ok(()) } /// Handle the system register read by SysRegAddr and data width. - pub fn handle_sys_reg_read(&self, addr: SysRegAddr, width: AccessWidth) -> AxResult { + pub fn handle_sys_reg_read( + &self, + addr: SysRegAddr, + width: AccessWidth, + ) -> DeviceManagerResult { let access = BusAccess { kind: BusKind::SysReg, is_read: true, @@ -1102,18 +1112,20 @@ impl AxVmDevices { width, data: 0, }; - match self.dispatch(&access) { - Ok(BusResponse::Read { value }) => Ok(value as usize), - Ok(BusResponse::Write) => { - Err(ax_err_type!(BadState, "expected read response, got write")) - } - Err(err) => { - error!( - "emu_device sys_reg read failed: {err:?} at {:#x} width {width:?}", - addr.0 - ); - Err(ax_err_type!(BadState, format!("sysreg read: {err:?}"))) - } + match self + .dispatch(&access) + .map_err(|source| DeviceManagerError::Access { + operation: "read", + bus: BusKind::SysReg, + addr: access.addr, + width, + source, + })? { + BusResponse::Read { value } => Ok(value as usize), + BusResponse::Write => Err(DeviceManagerError::UnexpectedResponse { + operation: "read system register device", + detail: "device returned a write acknowledgement".into(), + }), } } @@ -1123,7 +1135,7 @@ impl AxVmDevices { addr: SysRegAddr, width: AccessWidth, val: usize, - ) -> AxResult { + ) -> DeviceManagerResult { let access = BusAccess { kind: BusKind::SysReg, is_read: false, @@ -1131,18 +1143,19 @@ impl AxVmDevices { width, data: val as u64, }; - if let Err(err) = self.dispatch(&access) { - error!( - "emu_device sys_reg write failed: {err:?} at {:#x} width {width:?}", - addr.0 - ); - return Err(ax_err_type!(BadState, format!("sysreg write: {err:?}"))); - } + self.dispatch(&access) + .map_err(|source| DeviceManagerError::Access { + operation: "write", + bus: BusKind::SysReg, + addr: access.addr, + width, + source, + })?; Ok(()) } /// Handle the port read by port number and data width. - pub fn handle_port_read(&self, port: Port, width: AccessWidth) -> AxResult { + pub fn handle_port_read(&self, port: Port, width: AccessWidth) -> DeviceManagerResult { let access = BusAccess { kind: BusKind::Port, is_read: true, @@ -1150,23 +1163,30 @@ impl AxVmDevices { width, data: 0, }; - match self.dispatch(&access) { - Ok(BusResponse::Read { value }) => Ok(value as usize), - Ok(BusResponse::Write) => { - Err(ax_err_type!(BadState, "expected read response, got write")) - } - Err(err) => { - error!( - "emu_device port read failed: {err:?} at {:#x} width {width:?}", - port.0 - ); - Err(ax_err_type!(BadState, format!("port read: {err:?}"))) - } + match self + .dispatch(&access) + .map_err(|source| DeviceManagerError::Access { + operation: "read", + bus: BusKind::Port, + addr: access.addr, + width, + source, + })? { + BusResponse::Read { value } => Ok(value as usize), + BusResponse::Write => Err(DeviceManagerError::UnexpectedResponse { + operation: "read port device", + detail: "device returned a write acknowledgement".into(), + }), } } /// Handle the port write by port number, data width and the value need to write. - pub fn handle_port_write(&self, port: Port, width: AccessWidth, val: usize) -> AxResult { + pub fn handle_port_write( + &self, + port: Port, + width: AccessWidth, + val: usize, + ) -> DeviceManagerResult { let access = BusAccess { kind: BusKind::Port, is_read: false, @@ -1174,13 +1194,14 @@ impl AxVmDevices { width, data: val as u64, }; - if let Err(err) = self.dispatch(&access) { - error!( - "emu_device port write failed: {err:?} at {:#x} width {width:?}", - port.0 - ); - return Err(ax_err_type!(BadState, format!("port write: {err:?}"))); - } + self.dispatch(&access) + .map_err(|source| DeviceManagerError::Access { + operation: "write", + bus: BusKind::Port, + addr: access.addr, + width, + source, + })?; Ok(()) } } diff --git a/virtualization/axdevice/src/error.rs b/virtualization/axdevice/src/error.rs new file mode 100644 index 0000000000..9ff54b0cb0 --- /dev/null +++ b/virtualization/axdevice/src/error.rs @@ -0,0 +1,132 @@ +//! AxDevice-owned error contract. + +use alloc::string::String; + +use axdevice_base::{AccessWidth, BusKind, DeviceError, IrqError, RegistryError}; + +/// Result type returned by device manager operations. +pub type DeviceManagerResult = Result; + +/// Errors reported while configuring, registering, or accessing VM devices. +#[derive(Clone, Debug, Eq, PartialEq, thiserror::Error)] +pub enum DeviceManagerError { + /// Device configuration is malformed or inconsistent. + #[error("invalid device configuration for {operation}: {detail}")] + InvalidConfig { + /// The configuration operation that failed. + operation: &'static str, + /// Diagnostic detail describing the invalid configuration. + detail: String, + }, + /// An operation received an invalid argument. + #[error("invalid input for device operation {operation}: {detail}")] + InvalidInput { + /// The operation that rejected the input. + operation: &'static str, + /// Diagnostic detail describing the invalid input. + detail: String, + }, + /// A required device resource was not found. + #[error("device resource {resource} was not found during {operation}")] + ResourceNotFound { + /// The operation that required the resource. + operation: &'static str, + /// The missing resource. + resource: String, + }, + /// A device resource conflicts with an existing resource. + #[error("device resource conflict during {operation}: {detail}")] + ResourceConflict { + /// The operation that discovered the conflict. + operation: &'static str, + /// Diagnostic detail describing both resources. + detail: String, + }, + /// A device allocation failed. + #[error("out of memory during device operation {operation}")] + OutOfMemory { + /// The operation that attempted the allocation. + operation: &'static str, + }, + /// The requested device operation is unsupported. + #[error("unsupported device operation {operation}: {detail}")] + Unsupported { + /// The unsupported operation. + operation: &'static str, + /// Diagnostic detail describing the limitation. + detail: String, + }, + /// A device returned a response that does not match the request. + #[error("unexpected response during device operation {operation}: {detail}")] + UnexpectedResponse { + /// The operation that received the response. + operation: &'static str, + /// Diagnostic detail describing the response. + detail: String, + }, + /// A bus access failed with address and width context. + #[error("device {operation} failed on {bus:?} bus at {addr:#x} with width {width:?}: {source}")] + Access { + /// Whether the access is a read or write. + operation: &'static str, + /// The bus that received the access. + bus: BusKind, + /// The raw bus address. + addr: u64, + /// The requested access width. + width: AccessWidth, + /// The low-level device failure. + #[source] + source: DeviceError, + }, + /// A low-level device access failed. + #[error(transparent)] + Device(#[from] DeviceError), + /// Device registration or resource validation failed. + #[error(transparent)] + Registry(#[from] RegistryError), + /// IRQ resolution or signaling failed. + #[error(transparent)] + Irq(#[from] IrqError), +} + +impl From for DeviceError { + fn from(error: DeviceManagerError) -> Self { + match error { + DeviceManagerError::Device(error) => error, + DeviceManagerError::OutOfMemory { operation } => Self::OutOfMemory { operation }, + DeviceManagerError::Unsupported { operation, detail } => { + Self::Unsupported { operation, detail } + } + DeviceManagerError::InvalidConfig { operation, detail } => { + Self::InvalidData { operation, detail } + } + DeviceManagerError::InvalidInput { operation, detail } => { + Self::InvalidInput { operation, detail } + } + DeviceManagerError::ResourceNotFound { + operation, + resource, + } => Self::InvalidState { + operation, + detail: alloc::format!("resource {resource} was not found"), + }, + DeviceManagerError::ResourceConflict { operation, detail } => Self::ResourceBusy { + operation, + resource: detail, + }, + DeviceManagerError::UnexpectedResponse { operation, detail } => { + Self::InvalidState { operation, detail } + } + DeviceManagerError::Access { source, .. } => source, + DeviceManagerError::Registry(error) => Self::InvalidInput { + operation: "register device", + detail: alloc::format!("{error}"), + }, + DeviceManagerError::Irq(error) => Self::Backend { + operation: "route device IRQ", + detail: alloc::format!("{error}"), + }, + } + } +} diff --git a/virtualization/axdevice/src/factory.rs b/virtualization/axdevice/src/factory.rs index 32d7064c3c..42a7bc2b31 100644 --- a/virtualization/axdevice/src/factory.rs +++ b/virtualization/axdevice/src/factory.rs @@ -16,16 +16,19 @@ use alloc::{sync::Arc, vec::Vec}; -use ax_errno::{AxResult, ax_err}; use axdevice_base::{InterruptTriggerMode, IrqLine}; use axvm_types::{EmulatedDeviceConfig, EmulatedDeviceType}; -use crate::DeviceBundle; +use crate::{DeviceBundle, DeviceManagerError, DeviceManagerResult}; /// Resolves a VM-local interrupt line for a device under construction. pub trait IrqResolver: Send + Sync { /// Resolves `line` with the requested trigger mode. - fn resolve_irq(&self, line: usize, trigger: InterruptTriggerMode) -> AxResult; + fn resolve_irq( + &self, + line: usize, + trigger: InterruptTriggerMode, + ) -> DeviceManagerResult; } /// VM-owned services available while a device factory is building a device. @@ -40,7 +43,11 @@ impl<'a> DeviceBuildContext<'a> { } /// Resolves a VM-local interrupt line. - pub fn resolve_irq(&self, line: usize, trigger: InterruptTriggerMode) -> AxResult { + pub fn resolve_irq( + &self, + line: usize, + trigger: InterruptTriggerMode, + ) -> DeviceManagerResult { self.irq_resolver.resolve_irq(line, trigger) } } @@ -55,7 +62,7 @@ pub trait DeviceFactory: Send + Sync { &self, config: &EmulatedDeviceConfig, context: &DeviceBuildContext<'_>, - ) -> AxResult; + ) -> DeviceManagerResult; } /// A registry containing at most one factory for each emulated device type. @@ -73,13 +80,15 @@ impl DeviceFactoryRegistry { } /// Registers a factory, rejecting a duplicate device type. - pub fn register(&mut self, factory: Arc) -> AxResult { + pub fn register(&mut self, factory: Arc) -> DeviceManagerResult { let device_type = factory.device_type(); if self.get(device_type).is_some() { - return ax_err!( - AlreadyExists, - format_args!("factory for device type {device_type} is already registered") - ); + return Err(DeviceManagerError::ResourceConflict { + operation: "register device factory", + detail: alloc::format!( + "factory for device type {device_type} is already registered" + ), + }); } self.factories.push((device_type, factory)); Ok(()) @@ -98,15 +107,16 @@ impl DeviceFactoryRegistry { &self, config: &EmulatedDeviceConfig, context: &DeviceBuildContext<'_>, - ) -> AxResult { + ) -> DeviceManagerResult { let Some(factory) = self.get(config.emu_type) else { - return ax_err!( - Unsupported, - format_args!( + return Err(DeviceManagerError::Unsupported { + operation: "build emulated device", + detail: alloc::format!( "no factory is registered for emulated device '{}' of type {}", - config.name, config.emu_type - ) - ); + config.name, + config.emu_type + ), + }); }; factory.build(config, context) } @@ -123,12 +133,12 @@ impl DeviceFactory for MetaDeviceFactory { &self, _config: &EmulatedDeviceConfig, _context: &DeviceBuildContext<'_>, - ) -> AxResult { + ) -> DeviceManagerResult { Ok(DeviceBundle::new()) } } /// Registers device factories that do not depend on an architecture backend. -pub fn register_builtin_factories(registry: &mut DeviceFactoryRegistry) -> AxResult { +pub fn register_builtin_factories(registry: &mut DeviceFactoryRegistry) -> DeviceManagerResult { registry.register(Arc::new(MetaDeviceFactory)) } diff --git a/virtualization/axdevice/src/fw_cfg.rs b/virtualization/axdevice/src/fw_cfg.rs index ce4b2da821..5018e4e49e 100644 --- a/virtualization/axdevice/src/fw_cfg.rs +++ b/virtualization/axdevice/src/fw_cfg.rs @@ -1,10 +1,11 @@ use alloc::{format, vec, vec::Vec}; -use ax_errno::{AxResult, ax_err, ax_err_type}; use ax_kspin::SpinNoIrq as Mutex; -use axdevice_base::{AccessWidth, BaseDeviceOps, EmuDeviceType}; +use axdevice_base::{AccessWidth, BaseDeviceOps, DeviceResult, EmuDeviceType}; use axvm_types::{GuestPhysAddr, GuestPhysAddrRange}; +use crate::{DeviceManagerError, DeviceManagerResult}; + const FW_CFG_SIGNATURE: u16 = 0x00; const FW_CFG_ID: u16 = 0x01; const FW_CFG_RAM_SIZE: u16 = 0x03; @@ -394,7 +395,7 @@ impl FwCfg { addr: GuestPhysAddr, width: AccessWidth, value: usize, - ) -> AxResult> { + ) -> DeviceManagerResult> { let offset = addr.as_usize() - self.base.as_usize(); if !self.is_dma_address(addr) { return Ok(None); @@ -421,7 +422,10 @@ impl FwCfg { "unsupported fw_cfg DMA address write: offset={:#x}, width={:?}", offset, width ); - ax_err!(InvalidInput, "unsupported fw_cfg DMA address write") + Err(DeviceManagerError::InvalidInput { + operation: "write fw_cfg DMA address", + detail: format!("offset {offset:#x} does not accept width {width:?}"), + }) } } } @@ -432,10 +436,10 @@ impl FwCfg { desc_addr: GuestPhysAddr, mut read_guest: R, mut write_guest: W, - ) -> AxResult + ) -> DeviceManagerResult where - R: FnMut(GuestPhysAddr, &mut [u8]) -> AxResult, - W: FnMut(GuestPhysAddr, &[u8]) -> AxResult, + R: FnMut(GuestPhysAddr, &mut [u8]) -> DeviceManagerResult, + W: FnMut(GuestPhysAddr, &[u8]) -> DeviceManagerResult, { let mut desc = [0u8; FW_CFG_DMA_DESC_SIZE]; read_guest(desc_addr, &mut desc)?; @@ -468,10 +472,10 @@ impl FwCfg { buffer_addr: GuestPhysAddr, read_guest: &mut R, write_guest: &mut W, - ) -> AxResult + ) -> DeviceManagerResult where - R: FnMut(GuestPhysAddr, &mut [u8]) -> AxResult, - W: FnMut(GuestPhysAddr, &[u8]) -> AxResult, + R: FnMut(GuestPhysAddr, &mut [u8]) -> DeviceManagerResult, + W: FnMut(GuestPhysAddr, &[u8]) -> DeviceManagerResult, { validate_dma_buffer(buffer_addr, length)?; @@ -509,17 +513,26 @@ impl FwCfg { } _ => { warn!("invalid fw_cfg DMA control {:#x}", control); - ax_err!(InvalidInput, "invalid fw_cfg DMA control") + Err(DeviceManagerError::InvalidInput { + operation: "process fw_cfg DMA command", + detail: format!("invalid control value {control:#x}"), + }) } } } } -fn validate_dma_buffer(buffer_addr: GuestPhysAddr, length: usize) -> AxResult { +fn validate_dma_buffer(buffer_addr: GuestPhysAddr, length: usize) -> DeviceManagerResult { buffer_addr .as_usize() .checked_add(length) - .ok_or_else(|| ax_err_type!(InvalidInput, "fw_cfg DMA buffer address overflow"))?; + .ok_or_else(|| DeviceManagerError::InvalidInput { + operation: "validate fw_cfg DMA buffer", + detail: format!( + "buffer at {:#x} with length {length:#x} overflows the guest address space", + buffer_addr.as_usize() + ), + })?; Ok(()) } @@ -529,9 +542,9 @@ fn dma_read_entry( length: usize, buffer_addr: GuestPhysAddr, write_guest: &mut W, -) -> AxResult +) -> DeviceManagerResult where - W: FnMut(GuestPhysAddr, &[u8]) -> AxResult, + W: FnMut(GuestPhysAddr, &[u8]) -> DeviceManagerResult, { let mut remaining = length; let mut guest_offset = 0usize; @@ -564,9 +577,9 @@ fn dma_discard_guest_write( length: usize, buffer_addr: GuestPhysAddr, read_guest: &mut R, -) -> AxResult +) -> DeviceManagerResult where - R: FnMut(GuestPhysAddr, &mut [u8]) -> AxResult, + R: FnMut(GuestPhysAddr, &mut [u8]) -> DeviceManagerResult, { let mut scratch = [0u8; FW_CFG_DMA_SCRATCH_SIZE]; let mut remaining = length; @@ -581,11 +594,17 @@ where Ok(()) } -fn add_guest_offset(base: GuestPhysAddr, offset: usize) -> AxResult { +fn add_guest_offset(base: GuestPhysAddr, offset: usize) -> DeviceManagerResult { base.as_usize() .checked_add(offset) .map(GuestPhysAddr::from_usize) - .ok_or_else(|| ax_err_type!(InvalidInput, "fw_cfg DMA buffer address overflow")) + .ok_or_else(|| DeviceManagerError::InvalidInput { + operation: "advance fw_cfg DMA buffer", + detail: format!( + "buffer at {:#x} with offset {offset:#x} overflows the guest address space", + base.as_usize() + ), + }) } struct FwCfgFile<'a> { @@ -1407,7 +1426,7 @@ impl BaseDeviceOps for FwCfg { GuestPhysAddrRange::from_start_size(self.base, self.size) } - fn handle_read(&self, addr: GuestPhysAddr, width: AccessWidth) -> AxResult { + fn handle_read(&self, addr: GuestPhysAddr, width: AccessWidth) -> DeviceResult { match addr.as_usize() - self.base.as_usize() { FW_CFG_DATA_OFFSET => Ok(self.read_data(width)), FW_CFG_SELECTOR_OFFSET => Ok(self.state.lock().selected as usize), @@ -1415,7 +1434,7 @@ impl BaseDeviceOps for FwCfg { } } - fn handle_write(&self, addr: GuestPhysAddr, width: AccessWidth, val: usize) -> AxResult { + fn handle_write(&self, addr: GuestPhysAddr, width: AccessWidth, val: usize) -> DeviceResult { let offset = addr.as_usize() - self.base.as_usize(); if offset == FW_CFG_SELECTOR_OFFSET { self.write_selector(width, val); diff --git a/virtualization/axdevice/src/lib.rs b/virtualization/axdevice/src/lib.rs index 00016c2abf..be23fcf454 100644 --- a/virtualization/axdevice/src/lib.rs +++ b/virtualization/axdevice/src/lib.rs @@ -29,6 +29,7 @@ extern crate log; mod adapter; mod config; mod device; +mod error; mod factory; mod fw_cfg; #[cfg(target_arch = "loongarch64")] @@ -47,6 +48,7 @@ pub use axdevice_base::{ pub use axvm_types::GuestPhysAddr; pub use config::AxVmDeviceConfig; pub use device::AxVmDevices; +pub use error::{DeviceManagerError, DeviceManagerResult}; pub use factory::{ DeviceBuildContext, DeviceFactory, DeviceFactoryRegistry, IrqResolver, register_builtin_factories, diff --git a/virtualization/axdevice/src/loongarch_pch_pic.rs b/virtualization/axdevice/src/loongarch_pch_pic.rs index 6c02dc5f90..a227b95ef0 100644 --- a/virtualization/axdevice/src/loongarch_pch_pic.rs +++ b/virtualization/axdevice/src/loongarch_pch_pic.rs @@ -1,8 +1,7 @@ use core::sync::atomic::{AtomicUsize, Ordering}; -use ax_errno::AxResult; use ax_kspin::SpinNoIrq as Mutex; -use axdevice_base::{AccessWidth, BaseDeviceOps, EmuDeviceType}; +use axdevice_base::{AccessWidth, BaseDeviceOps, DeviceResult, EmuDeviceType}; use axvm_types::{GuestPhysAddr, GuestPhysAddrRange}; const PCH_PIC_INT_ID_LO: usize = 0x000; @@ -164,7 +163,7 @@ impl BaseDeviceOps for LoongArchPchPic { GuestPhysAddrRange::from_start_size(self.base, self.size) } - fn handle_read(&self, addr: GuestPhysAddr, width: AccessWidth) -> AxResult { + fn handle_read(&self, addr: GuestPhysAddr, width: AccessWidth) -> DeviceResult { let offset = addr.as_usize() - self.base.as_usize(); let state = self.state.lock(); let value = match width { @@ -179,7 +178,7 @@ impl BaseDeviceOps for LoongArchPchPic { Ok(value) } - fn handle_write(&self, addr: GuestPhysAddr, width: AccessWidth, val: usize) -> AxResult { + fn handle_write(&self, addr: GuestPhysAddr, width: AccessWidth, val: usize) -> DeviceResult { let offset = addr.as_usize() - self.base.as_usize(); let mut state = self.state.lock(); log_pch_pic_io("write", offset, width, val); diff --git a/virtualization/axdevice/src/registration.rs b/virtualization/axdevice/src/registration.rs index b2d249c225..ef1ea0e65c 100644 --- a/virtualization/axdevice/src/registration.rs +++ b/virtualization/axdevice/src/registration.rs @@ -16,13 +16,14 @@ use alloc::{sync::Arc, vec::Vec}; -use ax_errno::AxResult; use axdevice_base::Device; +use crate::DeviceManagerResult; + /// A device capability that can be polled by the VM runtime. pub trait PollableDeviceOps: Send + Sync { /// Advances the device using the current monotonic time in nanoseconds. - fn poll(&self, now_ns: u64) -> AxResult; + fn poll(&self, now_ns: u64) -> DeviceManagerResult; } /// One strongly typed capability contributed by a device. diff --git a/virtualization/axdevice/tests/error_contract.rs b/virtualization/axdevice/tests/error_contract.rs new file mode 100644 index 0000000000..1e890284f0 --- /dev/null +++ b/virtualization/axdevice/tests/error_contract.rs @@ -0,0 +1,46 @@ +use std::{fs, path::Path}; + +use axdevice::DeviceManagerError; +use axdevice_base::{DeviceError, RegistryError}; + +#[test] +fn crate_uses_typed_errors_without_errno_contracts() { + let crate_dir = Path::new(env!("CARGO_MANIFEST_DIR")); + let manifest = fs::read_to_string(crate_dir.join("Cargo.toml")).unwrap(); + let dependency = ["ax", "errno"].join("-"); + let source_name = ["ax", "errno"].join("_"); + + assert!(!manifest.contains(&dependency)); + assert_directory_excludes(&crate_dir.join("src"), &source_name); +} + +#[test] +fn from_conversions_keep_device_and_registry_variants_matchable() { + let device: DeviceManagerError = DeviceError::NotFound.into(); + let registry: DeviceManagerError = RegistryError::BusKindNotSupported { + kind: axdevice_base::BusKind::Port, + arch: axdevice_base::Arch::AArch64, + } + .into(); + + assert!(matches!( + device, + DeviceManagerError::Device(DeviceError::NotFound) + )); + assert!(matches!( + registry, + DeviceManagerError::Registry(RegistryError::BusKindNotSupported { .. }) + )); +} + +fn assert_directory_excludes(directory: &Path, forbidden: &str) { + for entry in fs::read_dir(directory).unwrap() { + let path = entry.unwrap().path(); + if path.is_dir() { + assert_directory_excludes(&path, forbidden); + } else if path.extension().is_some_and(|extension| extension == "rs") { + let source = fs::read_to_string(&path).unwrap(); + assert!(!source.contains(forbidden), "{}", path.display()); + } + } +} diff --git a/virtualization/axdevice_base/Cargo.toml b/virtualization/axdevice_base/Cargo.toml index 4abdf81c26..06065f47bd 100644 --- a/virtualization/axdevice_base/Cargo.toml +++ b/virtualization/axdevice_base/Cargo.toml @@ -16,9 +16,9 @@ log = "0.4" serde = { version = "1.0", default-features = false, features = ["derive", "alloc"] } # ArceOS and AxVisor crates. -ax-errno = { workspace = true } ax-memory-addr = { workspace = true } axvm-types = { workspace = true } +thiserror = { workspace = true } [dev-dependencies] [package.metadata.docs.rs] diff --git a/virtualization/axdevice_base/src/adapter.rs b/virtualization/axdevice_base/src/adapter.rs index cfc5ab6350..ea0ba94300 100644 --- a/virtualization/axdevice_base/src/adapter.rs +++ b/virtualization/axdevice_base/src/adapter.rs @@ -129,12 +129,10 @@ where self.inner .handle_read(addr, access.width) .map(|v| BusResponse::Read { value: v as u64 }) - .map_err(|_| DeviceError::Internal) } else { self.inner .handle_write(addr, access.width, access.data as usize) .map(|_| BusResponse::Write) - .map_err(|_| DeviceError::Internal) } } @@ -210,12 +208,10 @@ where self.inner .handle_read(addr, access.width) .map(|v| BusResponse::Read { value: v as u64 }) - .map_err(|_| DeviceError::Internal) } else { self.inner .handle_write(addr, access.width, access.data as usize) .map(|_| BusResponse::Write) - .map_err(|_| DeviceError::Internal) } } @@ -291,12 +287,10 @@ where self.inner .handle_read(port, access.width) .map(|v| BusResponse::Read { value: v as u64 }) - .map_err(|_| DeviceError::Internal) } else { self.inner .handle_write(port, access.width, access.data as usize) .map(|_| BusResponse::Write) - .map_err(|_| DeviceError::Internal) } } @@ -307,12 +301,11 @@ where #[cfg(test)] mod tests { - use ax_errno::AxResult; use axvm_types::GuestPhysAddr; use super::MmioDeviceAdapter; use crate::{ - BaseDeviceOps, Device, EmuDeviceType, GuestPhysAddrRange, Resource, + BaseDeviceOps, Device, DeviceResult, EmuDeviceType, GuestPhysAddrRange, Resource, device::{AccessWidth, BusAccess, BusKind, BusResponse}, }; @@ -331,10 +324,15 @@ mod tests { .try_into() .unwrap() } - fn handle_read(&self, _addr: GuestPhysAddr, _width: AccessWidth) -> AxResult { + fn handle_read(&self, _addr: GuestPhysAddr, _width: AccessWidth) -> DeviceResult { Ok(self.read_val) } - fn handle_write(&self, _addr: GuestPhysAddr, _width: AccessWidth, _val: usize) -> AxResult { + fn handle_write( + &self, + _addr: GuestPhysAddr, + _width: AccessWidth, + _val: usize, + ) -> DeviceResult { Ok(()) } } diff --git a/virtualization/axdevice_base/src/device.rs b/virtualization/axdevice_base/src/device.rs index 88fd5d4527..8ca8f615a2 100644 --- a/virtualization/axdevice_base/src/device.rs +++ b/virtualization/axdevice_base/src/device.rs @@ -1,5 +1,6 @@ //! Device address and access width definitions. +use alloc::string::String; use core::fmt::{Debug, LowerHex}; use ax_memory_addr::AddrRange; @@ -178,11 +179,13 @@ pub enum BusResponse { } /// Errors that can occur during device access handling. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Eq, PartialEq, thiserror::Error)] pub enum DeviceError { /// No device found at the requested address. + #[error("no device was found for the requested bus access")] NotFound, /// The access width does not match what the register expects. + #[error("invalid device access width: expected {expected:?}, got {actual:?}")] InvalidWidth { /// The width the register expects. expected: AccessWidth, @@ -190,16 +193,78 @@ pub enum DeviceError { actual: AccessWidth, }, /// Attempted to write to a read-only register. + #[error("attempted to write a read-only device register")] ReadOnly, /// Attempted to read from a write-only register. + #[error("attempted to read a write-only device register")] WriteOnly, /// The address is outside the device's range. + #[error("device address {addr:#x} is outside the registered range")] OutOfRange { /// The address that was accessed. addr: u64, }, /// The requested functionality is not yet implemented. + #[error("device operation is not implemented")] Unimplemented, /// An internal error occurred in the device implementation. + #[error("internal device error")] Internal, + /// An operation received an invalid argument. + #[error("invalid input for device operation {operation}: {detail}")] + InvalidInput { + /// The operation that rejected the input. + operation: &'static str, + /// Diagnostic detail describing the invalid input. + detail: String, + }, + /// Device data is malformed or inconsistent. + #[error("invalid data for device operation {operation}: {detail}")] + InvalidData { + /// The operation that rejected the data. + operation: &'static str, + /// Diagnostic detail describing the malformed data. + detail: String, + }, + /// Device state does not allow the requested operation. + #[error("invalid state for device operation {operation}: {detail}")] + InvalidState { + /// The operation that cannot run in the current state. + operation: &'static str, + /// Diagnostic detail describing the current state. + detail: String, + }, + /// The device does not support the requested operation. + #[error("unsupported device operation {operation}: {detail}")] + Unsupported { + /// The unsupported operation. + operation: &'static str, + /// Diagnostic detail describing the limitation. + detail: String, + }, + /// A device allocation failed. + #[error("out of memory during device operation {operation}")] + OutOfMemory { + /// The operation that attempted the allocation. + operation: &'static str, + }, + /// A device resource is currently busy. + #[error("device resource {resource} is busy during {operation}")] + ResourceBusy { + /// The operation that attempted to use the resource. + operation: &'static str, + /// The busy resource. + resource: String, + }, + /// A device backend operation failed. + #[error("device backend operation {operation} failed: {detail}")] + Backend { + /// The backend operation that failed. + operation: &'static str, + /// Diagnostic detail from the backend. + detail: String, + }, } + +/// Result type returned by device access operations. +pub type DeviceResult = Result; diff --git a/virtualization/axdevice_base/src/irq.rs b/virtualization/axdevice_base/src/irq.rs index f26b906022..19dff11480 100644 --- a/virtualization/axdevice_base/src/irq.rs +++ b/virtualization/axdevice_base/src/irq.rs @@ -1,20 +1,72 @@ //! Architecture-independent interrupt line signaling. -use alloc::sync::Arc; +use alloc::{string::String, sync::Arc}; -use ax_errno::{AxError, AxResult}; use axvm_types::{InterruptTriggerMode, IrqLineId}; +/// Errors reported while routing or signaling a virtual interrupt line. +#[derive(Clone, Debug, Eq, PartialEq, thiserror::Error)] +pub enum IrqError { + /// The requested operation does not match the line's trigger mode. + #[error( + "IRQ line {line:?} uses {actual:?} triggering, but {operation} requires {expected:?} \ + triggering" + )] + InvalidTriggerMode { + /// The affected interrupt line. + line: IrqLineId, + /// The operation that was requested. + operation: &'static str, + /// The trigger mode required by the operation. + expected: InterruptTriggerMode, + /// The trigger mode configured for the line. + actual: InterruptTriggerMode, + }, + /// The interrupt line identifier is invalid for the sink. + #[error("invalid IRQ line {line:?} during {operation}: {detail}")] + InvalidLine { + /// The rejected interrupt line. + line: IrqLineId, + /// The operation that rejected the line. + operation: &'static str, + /// Diagnostic detail describing the valid range or assignment. + detail: String, + }, + /// The interrupt sink does not support the requested operation. + #[error("unsupported IRQ operation {operation} on line {line:?}: {detail}")] + Unsupported { + /// The affected interrupt line. + line: IrqLineId, + /// The unsupported operation. + operation: &'static str, + /// Diagnostic detail describing the limitation. + detail: String, + }, + /// The interrupt controller backend failed. + #[error("IRQ backend operation {operation} failed for line {line:?}: {detail}")] + Backend { + /// The affected interrupt line. + line: IrqLineId, + /// The backend operation that failed. + operation: &'static str, + /// Diagnostic detail from the backend. + detail: String, + }, +} + +/// Result type returned by virtual interrupt routing operations. +pub type IrqResult = Result; + /// Receives state changes and pulses from interrupt lines. /// /// Implementations route the line operation to a VM-specific interrupt /// controller backend. pub trait IrqSink: Send + Sync { /// Sets whether a level-triggered interrupt line is asserted. - fn set_level(&self, line: IrqLineId, asserted: bool) -> AxResult; + fn set_level(&self, line: IrqLineId, asserted: bool) -> IrqResult; /// Delivers one pulse from an edge-triggered interrupt line. - fn pulse(&self, line: IrqLineId) -> AxResult; + fn pulse(&self, line: IrqLineId) -> IrqResult; } /// A shareable interrupt line connected to an [`IrqSink`]. @@ -37,16 +89,16 @@ impl IrqLine { /// ``` /// use std::sync::Arc; /// - /// use axdevice_base::{AxResult, InterruptTriggerMode, IrqLine, IrqLineId, IrqSink}; + /// use axdevice_base::{InterruptTriggerMode, IrqLine, IrqLineId, IrqResult, IrqSink}; /// /// struct Sink; /// /// impl IrqSink for Sink { - /// fn set_level(&self, _line: IrqLineId, _asserted: bool) -> AxResult { + /// fn set_level(&self, _line: IrqLineId, _asserted: bool) -> IrqResult { /// Ok(()) /// } /// - /// fn pulse(&self, _line: IrqLineId) -> AxResult { + /// fn pulse(&self, _line: IrqLineId) -> IrqResult { /// Ok(()) /// } /// } @@ -64,30 +116,45 @@ impl IrqLine { /// Asserts a level-triggered interrupt line. /// - /// Returns [`AxError::InvalidInput`] for an edge-triggered line. - pub fn raise(&self) -> AxResult { + /// Returns [`IrqError::InvalidTriggerMode`] for an edge-triggered line. + pub fn raise(&self) -> IrqResult { if self.0.trigger != InterruptTriggerMode::LevelTriggered { - return Err(AxError::InvalidInput); + return Err(IrqError::InvalidTriggerMode { + line: self.0.id, + operation: "raise", + expected: InterruptTriggerMode::LevelTriggered, + actual: self.0.trigger, + }); } self.0.sink.set_level(self.0.id, true) } /// Deasserts a level-triggered interrupt line. /// - /// Returns [`AxError::InvalidInput`] for an edge-triggered line. - pub fn lower(&self) -> AxResult { + /// Returns [`IrqError::InvalidTriggerMode`] for an edge-triggered line. + pub fn lower(&self) -> IrqResult { if self.0.trigger != InterruptTriggerMode::LevelTriggered { - return Err(AxError::InvalidInput); + return Err(IrqError::InvalidTriggerMode { + line: self.0.id, + operation: "lower", + expected: InterruptTriggerMode::LevelTriggered, + actual: self.0.trigger, + }); } self.0.sink.set_level(self.0.id, false) } /// Pulses an edge-triggered interrupt line. /// - /// Returns [`AxError::InvalidInput`] for a level-triggered line. - pub fn pulse(&self) -> AxResult { + /// Returns [`IrqError::InvalidTriggerMode`] for a level-triggered line. + pub fn pulse(&self) -> IrqResult { if self.0.trigger != InterruptTriggerMode::EdgeTriggered { - return Err(AxError::InvalidInput); + return Err(IrqError::InvalidTriggerMode { + line: self.0.id, + operation: "pulse", + expected: InterruptTriggerMode::EdgeTriggered, + actual: self.0.trigger, + }); } self.0.sink.pulse(self.0.id) } diff --git a/virtualization/axdevice_base/src/lib.rs b/virtualization/axdevice_base/src/lib.rs index f273653642..d8fd357df5 100644 --- a/virtualization/axdevice_base/src/lib.rs +++ b/virtualization/axdevice_base/src/lib.rs @@ -39,7 +39,7 @@ //! ```rust,ignore //! use axdevice_base::{BaseDeviceOps, EmuDeviceType}; //! use axaddrspace::{GuestPhysAddrRange, device::AccessWidth}; -//! use ax_errno::AxResult; +//! use axdevice_base::DeviceResult; //! //! struct MyDevice { //! base_addr: usize, @@ -55,12 +55,12 @@ //! (self.base_addr..self.base_addr + self.size).try_into().unwrap() //! } //! -//! fn handle_read(&self, addr: GuestPhysAddr, width: AccessWidth) -> AxResult { +//! fn handle_read(&self, addr: GuestPhysAddr, width: AccessWidth) -> DeviceResult { //! // Handle read operation //! Ok(0) //! } //! -//! fn handle_write(&self, addr: GuestPhysAddr, width: AccessWidth, val: usize) -> AxResult { +//! fn handle_write(&self, addr: GuestPhysAddr, width: AccessWidth, val: usize) -> DeviceResult { //! // Handle write operation //! Ok(()) //! } @@ -89,17 +89,14 @@ mod device; use alloc::{string::String, sync::Arc, vec::Vec}; use core::any::Any; -#[doc(hidden)] -pub use ax_errno::AxError; -pub use ax_errno::AxResult; pub use axvm_types::{ EmulatedDeviceType as EmuDeviceType, GuestPhysAddr, GuestPhysAddrRange, InterruptTriggerMode, IrqLineId, }; pub use crate::device::{ - AccessWidth, BusAccess, BusKind, BusResponse, DeviceAddr, DeviceAddrRange, DeviceError, Port, - PortRange, SysRegAddr, SysRegAddrRange, + AccessWidth, BusAccess, BusKind, BusResponse, DeviceAddr, DeviceAddrRange, DeviceError, + DeviceResult, Port, PortRange, SysRegAddr, SysRegAddrRange, }; /// Represents the configuration of an emulated device for a virtual machine. @@ -227,7 +224,7 @@ pub trait BaseDeviceOps: Any { /// Implementations should respect the `width` parameter and only return /// data of the appropriate size. The returned value should be zero-extended /// if necessary. - fn handle_read(&self, addr: R::Addr, width: AccessWidth) -> AxResult; + fn handle_read(&self, addr: R::Addr, width: AccessWidth) -> DeviceResult; /// Handles a write operation on the emulated device. /// @@ -246,7 +243,7 @@ pub trait BaseDeviceOps: Any { /// /// Implementations should only use the lower bits of `val` corresponding /// to the specified `width`. - fn handle_write(&self, addr: R::Addr, width: AccessWidth, val: usize) -> AxResult; + fn handle_write(&self, addr: R::Addr, width: AccessWidth, val: usize) -> DeviceResult; } /// Attempts to downcast a device to a specific type and apply a function to it. @@ -383,7 +380,7 @@ pub enum Arch { /// /// The device manager uses this information for address-range conflict /// detection and architecture-suitability checks. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Eq, PartialEq)] pub enum Resource { /// An MMIO address window. MmioRange { @@ -410,27 +407,33 @@ pub enum Resource { /// The reason a resource was rejected as structurally invalid during /// validation. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Eq, PartialEq, thiserror::Error)] pub enum InvalidResourceReason { /// The resource has a size or count of zero. + #[error("resource size or count is zero")] ZeroSized, /// The resource's end address overflows the address space. + #[error("resource end address overflows")] AddressOverflow, /// The resource extends past the valid bus address range. + #[error("resource extends beyond the bus address range")] OutOfBusRange, /// The bus kind of the resource is not supported on the current /// architecture. + #[error("resource bus is unsupported on this architecture")] UnsupportedOnArchitecture, /// The device declared multiple resources of the same bus kind whose /// address ranges overlap each other, which would corrupt the /// dispatch index. + #[error("device resources overlap")] OverlappingResources, } /// Errors that can be returned when registering a device. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Eq, PartialEq, thiserror::Error)] pub enum RegistryError { /// The device declared a resource that is structurally invalid. + #[error("invalid device resource {resource:?}: {reason}")] InvalidResource { /// The invalid resource. resource: Resource, @@ -438,6 +441,10 @@ pub enum RegistryError { reason: InvalidResourceReason, }, /// Two devices claim overlapping address ranges. + #[error( + "device resource {resource:?} conflicts with {existing:?} owned by device \ + {existing_device:?}" + )] AddressConflict { /// The resource the new device is attempting to register. resource: Resource, @@ -448,6 +455,7 @@ pub enum RegistryError { }, /// The device requested a bus type that the current architecture does /// not support (e.g. Port I/O on AArch64). + #[error("device bus {kind:?} is unsupported on {arch:?}")] BusKindNotSupported { /// The unsupported bus kind. kind: BusKind, @@ -455,6 +463,10 @@ pub enum RegistryError { arch: Arch, }, /// The device is not compatible with the current target architecture. + #[error( + "device {device_name} requires {required_arch:?}, but the current architecture is \ + {current_arch:?}" + )] ArchNotSupported { /// Human-readable device name (for diagnostics). device_name: String, @@ -559,4 +571,4 @@ mod adapter; mod irq; pub use adapter::{MmioDeviceAdapter, PortDeviceAdapter, SysRegDeviceAdapter}; -pub use irq::{IrqLine, IrqSink}; +pub use irq::{IrqError, IrqLine, IrqResult, IrqSink}; diff --git a/virtualization/axdevice_base/tests/error_contract.rs b/virtualization/axdevice_base/tests/error_contract.rs new file mode 100644 index 0000000000..91b290ced5 --- /dev/null +++ b/virtualization/axdevice_base/tests/error_contract.rs @@ -0,0 +1,44 @@ +use std::{fs, path::Path}; + +use axdevice_base::{AccessWidth, DeviceError, IrqError, IrqLineId}; + +#[test] +fn crate_uses_typed_errors_without_errno_contracts() { + assert_manifest_and_sources_exclude_errno(Path::new(env!("CARGO_MANIFEST_DIR"))); +} + +#[test] +fn device_and_irq_errors_preserve_access_context() { + let device = DeviceError::InvalidWidth { + expected: AccessWidth::Dword, + actual: AccessWidth::Qword, + }; + let irq = IrqError::InvalidLine { + line: IrqLineId(9), + operation: "route", + detail: "line is not assigned".into(), + }; + + assert!(device.to_string().contains("Dword")); + assert!(irq.to_string().contains("line is not assigned")); +} + +fn assert_manifest_and_sources_exclude_errno(crate_dir: &Path) { + let forbidden = ["ax", "errno"].join("_"); + let manifest_dependency = ["ax", "errno"].join("-"); + let manifest = fs::read_to_string(crate_dir.join("Cargo.toml")).unwrap(); + assert!(!manifest.contains(&manifest_dependency)); + assert_directory_excludes(&crate_dir.join("src"), &forbidden); +} + +fn assert_directory_excludes(directory: &Path, forbidden: &str) { + for entry in fs::read_dir(directory).unwrap() { + let path = entry.unwrap().path(); + if path.is_dir() { + assert_directory_excludes(&path, forbidden); + } else if path.extension().is_some_and(|extension| extension == "rs") { + let source = fs::read_to_string(&path).unwrap(); + assert!(!source.contains(forbidden), "{}", path.display()); + } + } +} diff --git a/virtualization/axdevice_base/tests/irq.rs b/virtualization/axdevice_base/tests/irq.rs index 3a111c4539..f9c60e0000 100644 --- a/virtualization/axdevice_base/tests/irq.rs +++ b/virtualization/axdevice_base/tests/irq.rs @@ -14,8 +14,7 @@ use std::sync::{Arc, Mutex}; -use ax_errno::{AxError, AxResult}; -use axdevice_base::{InterruptTriggerMode, IrqLine, IrqLineId, IrqSink}; +use axdevice_base::{InterruptTriggerMode, IrqError, IrqLine, IrqLineId, IrqResult, IrqSink}; #[derive(Clone, Copy, Debug, Eq, PartialEq)] enum IrqEvent { @@ -25,11 +24,11 @@ enum IrqEvent { struct MockIrqSink { events: Mutex>, - error: Option, + error: Option, } impl MockIrqSink { - fn new(error: Option) -> Self { + fn new(error: Option) -> Self { Self { events: Mutex::new(Vec::new()), error, @@ -42,8 +41,8 @@ impl MockIrqSink { } impl IrqSink for MockIrqSink { - fn set_level(&self, line: IrqLineId, asserted: bool) -> AxResult { - if let Some(error) = self.error { + fn set_level(&self, line: IrqLineId, asserted: bool) -> IrqResult { + if let Some(error) = self.error.clone() { return Err(error); } self.events @@ -53,8 +52,8 @@ impl IrqSink for MockIrqSink { Ok(()) } - fn pulse(&self, line: IrqLineId) -> AxResult { - if let Some(error) = self.error { + fn pulse(&self, line: IrqLineId) -> IrqResult { + if let Some(error) = self.error.clone() { return Err(error); } self.events.lock().unwrap().push(IrqEvent::Pulse(line)); @@ -112,15 +111,38 @@ fn mismatched_line_operations_return_invalid_input() { sink.clone(), ); - assert_eq!(edge_line.raise(), Err(AxError::InvalidInput)); - assert_eq!(edge_line.lower(), Err(AxError::InvalidInput)); - assert_eq!(level_line.pulse(), Err(AxError::InvalidInput)); + assert!(matches!( + edge_line.raise(), + Err(IrqError::InvalidTriggerMode { + operation: "raise", + .. + }) + )); + assert!(matches!( + edge_line.lower(), + Err(IrqError::InvalidTriggerMode { + operation: "lower", + .. + }) + )); + assert!(matches!( + level_line.pulse(), + Err(IrqError::InvalidTriggerMode { + operation: "pulse", + .. + }) + )); assert!(sink.events().is_empty()); } #[test] fn sink_errors_are_propagated() { - let sink = Arc::new(MockIrqSink::new(Some(AxError::Io))); + let backend_error = IrqError::Backend { + line: IrqLineId(4), + operation: "signal", + detail: "controller unavailable".into(), + }; + let sink = Arc::new(MockIrqSink::new(Some(backend_error.clone()))); let edge_line = IrqLine::new( IrqLineId(4), InterruptTriggerMode::EdgeTriggered, @@ -128,7 +150,7 @@ fn sink_errors_are_propagated() { ); let level_line = IrqLine::new(IrqLineId(33), InterruptTriggerMode::LevelTriggered, sink); - assert_eq!(edge_line.pulse(), Err(AxError::Io)); - assert_eq!(level_line.raise(), Err(AxError::Io)); - assert_eq!(level_line.lower(), Err(AxError::Io)); + assert_eq!(edge_line.pulse(), Err(backend_error.clone())); + assert_eq!(level_line.raise(), Err(backend_error.clone())); + assert_eq!(level_line.lower(), Err(backend_error)); } diff --git a/virtualization/axdevice_base/tests/test.rs b/virtualization/axdevice_base/tests/test.rs index 82e7017a0c..35b3882e4f 100644 --- a/virtualization/axdevice_base/tests/test.rs +++ b/virtualization/axdevice_base/tests/test.rs @@ -16,10 +16,9 @@ extern crate alloc; use alloc::{sync::Arc, vec}; -use ax_errno::AxResult; use axdevice_base::{ - AccessWidth, BaseDeviceOps, BusAccess, BusKind, BusResponse, Device, EmuDeviceType, - MmioDeviceAdapter, + AccessWidth, BaseDeviceOps, BusAccess, BusKind, BusResponse, Device, DeviceResult, + EmuDeviceType, MmioDeviceAdapter, }; use axvm_types::{GuestPhysAddr, GuestPhysAddrRange}; @@ -36,11 +35,11 @@ impl BaseDeviceOps for DeviceA { (0x1000..0x2000).try_into().unwrap() } - fn handle_read(&self, addr: GuestPhysAddr, _width: AccessWidth) -> AxResult { + fn handle_read(&self, addr: GuestPhysAddr, _width: AccessWidth) -> DeviceResult { Ok(addr.as_usize()) } - fn handle_write(&self, _addr: GuestPhysAddr, _width: AccessWidth, _val: usize) -> AxResult { + fn handle_write(&self, _addr: GuestPhysAddr, _width: AccessWidth, _val: usize) -> DeviceResult { Ok(()) } } @@ -63,11 +62,11 @@ impl BaseDeviceOps for DeviceB { (0x2000..0x3000).try_into().unwrap() } - fn handle_read(&self, addr: GuestPhysAddr, _width: AccessWidth) -> AxResult { + fn handle_read(&self, addr: GuestPhysAddr, _width: AccessWidth) -> DeviceResult { Ok(addr.as_usize()) } - fn handle_write(&self, _addr: GuestPhysAddr, _width: AccessWidth, _val: usize) -> AxResult { + fn handle_write(&self, _addr: GuestPhysAddr, _width: AccessWidth, _val: usize) -> DeviceResult { Ok(()) } } diff --git a/virtualization/axvm/src/arch/aarch64/vm.rs b/virtualization/axvm/src/arch/aarch64/vm.rs index ca91b02eac..5a48c2f0fa 100644 --- a/virtualization/axvm/src/arch/aarch64/vm.rs +++ b/virtualization/axvm/src/arch/aarch64/vm.rs @@ -8,7 +8,7 @@ use axvm_types::{NestedPagingConfig, VMInterruptMode, VmArchVcpuOps}; use super::{Aarch64Arch, npt}; use crate::{ - AxVmResult, ax_err, ax_err_type, + AxVmError, AxVmResult, ax_err, config::AxVMConfig, vm::{ AxVM, AxVMResources, @@ -96,35 +96,37 @@ fn register_arch_devices( devices: &mut axdevice::AxVmDevices, ) -> AxVmResult { if config.interrupt_mode() == VMInterruptMode::Passthrough { - assign_passthrough_spis(vm, config, devices); + assign_passthrough_spis(vm, config, devices)?; } else { register_virtual_timers(devices)?; } Ok(()) } -fn assign_passthrough_spis(vm: &AxVM, config: &AxVMConfig, devices: &axdevice::AxVmDevices) { +fn assign_passthrough_spis( + vm: &AxVM, + config: &AxVMConfig, + devices: &axdevice::AxVmDevices, +) -> AxVmResult { let cpu_id = vm.id() - 1; // FIXME: get the real CPU id. let Some(gicd) = devices .devices() .find_map(|device| device.as_any().downcast_ref::()) else { warn!("Failed to assign SPIs: No VGicD found in device list"); - return; + return Ok(()); }; for spi in config.pass_through_spis() { - gicd.assign_irq(*spi + 32, cpu_id, (0, 0, 0, cpu_id as _)); + gicd.assign_irq(*spi + 32, cpu_id, (0, 0, 0, cpu_id as _)) + .map_err(|error| AxVmError::interrupt("assign passthrough SPI", error))?; } + Ok(()) } fn register_virtual_timers(devices: &mut axdevice::AxVmDevices) -> AxVmResult { for device in axdevice::create_vtimer_devices() { - devices - .register(Arc::from(device) as Arc) - .map_err(|err| { - ax_err_type!(InvalidInput, alloc::format!("register vtimer: {err:?}")) - })?; + devices.register(Arc::from(device) as Arc)?; } Ok(()) } diff --git a/virtualization/axvm/src/arch/riscv64/irq.rs b/virtualization/axvm/src/arch/riscv64/irq.rs index 39a370d7b7..fc4b540753 100644 --- a/virtualization/axvm/src/arch/riscv64/irq.rs +++ b/virtualization/axvm/src/arch/riscv64/irq.rs @@ -17,10 +17,10 @@ use alloc::sync::Arc; use axdevice::{ - DeviceBuildContext, DeviceBundle, DeviceFactory, DeviceFactoryRegistry, DeviceRegistration, - MmioDeviceAdapter, + DeviceBuildContext, DeviceBundle, DeviceFactory, DeviceFactoryRegistry, DeviceManagerError, + DeviceManagerResult, DeviceRegistration, MmioDeviceAdapter, }; -use axdevice_base::{AxError as DeviceError, AxResult as DeviceResult, IrqLineId, IrqSink}; +use axdevice_base::{IrqError, IrqLineId, IrqResult, IrqSink}; use axvm_types::{EmulatedDeviceConfig, EmulatedDeviceType, VMInterruptMode}; use riscv_vplic::{ PLIC_CONTEXT_CLAIM_COMPLETE_OFFSET, PLIC_CONTEXT_CTRL_OFFSET, PLIC_CONTEXT_STRIDE, VPlicGlobal, @@ -33,16 +33,27 @@ struct RiscvPlicIrqSink { } impl IrqSink for RiscvPlicIrqSink { - fn set_level(&self, line: IrqLineId, asserted: bool) -> DeviceResult { - if asserted { + fn set_level(&self, line: IrqLineId, asserted: bool) -> IrqResult { + let result = if asserted { self.vplic.set_pending(line.0) } else { self.vplic.clear_pending(line.0) - } + }; + result.map_err(|error| IrqError::Backend { + line, + operation: "set vPLIC line level", + detail: alloc::format!("{error}"), + }) } - fn pulse(&self, line: IrqLineId) -> DeviceResult { - self.vplic.set_pending(line.0) + fn pulse(&self, line: IrqLineId) -> IrqResult { + self.vplic + .set_pending(line.0) + .map_err(|error| IrqError::Backend { + line, + operation: "pulse vPLIC line", + detail: alloc::format!("{error}"), + }) } } @@ -62,12 +73,18 @@ impl DeviceFactory for RiscvPlicFactory { &self, config: &EmulatedDeviceConfig, _context: &DeviceBuildContext<'_>, - ) -> DeviceResult { + ) -> DeviceManagerResult { if config.base_gpa != self.base_gpa || config.length != self.length || config.cfg_list.as_slice() != [self.contexts_num] { - return Err(DeviceError::InvalidInput); + return Err(DeviceManagerError::InvalidConfig { + operation: "build virtual PLIC", + detail: alloc::format!( + "factory configuration does not match device '{}'", + config.name + ), + }); } Ok(DeviceRegistration::Device(MmioDeviceAdapter::from_arc(self.vplic.clone())).into()) } @@ -124,19 +141,16 @@ pub(crate) fn configure( } let contexts_num = validate_vplic_config(config)?; - let vplic = Arc::new(VPlicGlobal::new( - config.base_gpa.into(), - Some(config.length), + let vplic = Arc::new( + VPlicGlobal::new(config.base_gpa.into(), Some(config.length), contexts_num) + .map_err(AxVmError::invalid_config)?, + ); + factories.register(Arc::new(RiscvPlicFactory { + base_gpa: config.base_gpa, + length: config.length, contexts_num, - )); - factories - .register(Arc::new(RiscvPlicFactory { - base_gpa: config.base_gpa, - length: config.length, - contexts_num, - vplic: vplic.clone(), - })) - .map_err(|error| AxVmError::device("register virtual PLIC factory", error))?; + vplic: vplic.clone(), + }))?; InterruptFabric::with_sink(mode, Arc::new(RiscvPlicIrqSink { vplic })) } diff --git a/virtualization/axvm/src/arch/x86_64/port.rs b/virtualization/axvm/src/arch/x86_64/port.rs index 9c9c304f74..124da32d8d 100644 --- a/virtualization/axvm/src/arch/x86_64/port.rs +++ b/virtualization/axvm/src/arch/x86_64/port.rs @@ -1,8 +1,7 @@ //! Native x86 host I/O port passthrough devices. use axdevice_base::{ - AccessWidth, AxError as DeviceError, AxResult as DeviceResult, BaseDeviceOps, EmuDeviceType, - Port, PortRange, + AccessWidth, BaseDeviceOps, DeviceError, DeviceResult, EmuDeviceType, Port, PortRange, }; use crate::{AxVmResult, ax_err}; @@ -47,7 +46,10 @@ impl BaseDeviceOps for HostPortPassthrough { AccessWidth::Byte => Ok(unsafe { inb(port.number()) } as usize), AccessWidth::Word => Ok(unsafe { inw(port.number()) } as usize), AccessWidth::Dword => Ok(unsafe { inl(port.number()) } as usize), - AccessWidth::Qword => Err(DeviceError::Unsupported), + AccessWidth::Qword => Err(DeviceError::Unsupported { + operation: "read host I/O port", + detail: "x86 port I/O does not support 64-bit accesses".into(), + }), } } @@ -57,7 +59,10 @@ impl BaseDeviceOps for HostPortPassthrough { AccessWidth::Word => unsafe { outw(port.number(), value as u16) }, AccessWidth::Dword => unsafe { outl(port.number(), value as u32) }, AccessWidth::Qword => { - return Err(DeviceError::Unsupported); + return Err(DeviceError::Unsupported { + operation: "write host I/O port", + detail: "x86 port I/O does not support 64-bit accesses".into(), + }); } } Ok(()) diff --git a/virtualization/axvm/src/error.rs b/virtualization/axvm/src/error.rs index c725722b6c..b1a93fdab1 100644 --- a/virtualization/axvm/src/error.rs +++ b/virtualization/axvm/src/error.rs @@ -4,6 +4,8 @@ use alloc::{format, string::String}; use core::fmt::Display; use axaddrspace::AddrSpaceError; +use axdevice::DeviceManagerError; +use axdevice_base::{DeviceError, IrqError, RegistryError}; use crate::{VMId, VmStatus}; @@ -203,6 +205,79 @@ impl AxVmError { } } +impl From for AxVmError { + fn from(error: DeviceError) -> Self { + match error { + DeviceError::InvalidInput { operation, detail } => { + Self::InvalidInput { operation, detail } + } + DeviceError::InvalidData { detail, .. } => Self::InvalidConfig { detail }, + DeviceError::Unsupported { operation, detail } => { + Self::Unsupported { operation, detail } + } + DeviceError::OutOfMemory { operation } => Self::OutOfMemory { operation }, + DeviceError::ResourceBusy { + operation, + resource, + } => { + Self::resource_conflict("device resource", format_args!("{operation}: {resource}")) + } + error => Self::device("access virtual device", error), + } + } +} + +impl From for AxVmError { + fn from(error: IrqError) -> Self { + Self::interrupt("route virtual device interrupt", error) + } +} + +impl From for AxVmError { + fn from(error: RegistryError) -> Self { + match error { + RegistryError::AddressConflict { .. } => { + Self::resource_conflict("device address range", error) + } + RegistryError::InvalidResource { .. } => { + Self::invalid_input("register virtual device", error) + } + RegistryError::BusKindNotSupported { .. } | RegistryError::ArchNotSupported { .. } => { + Self::unsupported("register virtual device", error) + } + } + } +} + +impl From for AxVmError { + fn from(error: DeviceManagerError) -> Self { + match error { + DeviceManagerError::InvalidConfig { detail, .. } => Self::InvalidConfig { detail }, + DeviceManagerError::InvalidInput { operation, detail } => { + Self::InvalidInput { operation, detail } + } + DeviceManagerError::ResourceNotFound { + operation, + resource, + } => Self::resource_unavailable( + "device resource", + format_args!("{operation}: {resource}"), + ), + DeviceManagerError::ResourceConflict { operation, detail } => { + Self::resource_conflict("device resource", format_args!("{operation}: {detail}")) + } + DeviceManagerError::OutOfMemory { operation } => Self::OutOfMemory { operation }, + DeviceManagerError::Unsupported { operation, detail } => { + Self::Unsupported { operation, detail } + } + DeviceManagerError::Irq(error) => error.into(), + DeviceManagerError::Registry(error) => error.into(), + DeviceManagerError::Device(error) => error.into(), + error => Self::device("manage virtual devices", error), + } + } +} + macro_rules! ax_err_type { (InvalidInput $(, $detail:expr)?) => { $crate::AxVmError::invalid_input(module_path!(), $crate::ax_err_type!(@detail $($detail)?)) diff --git a/virtualization/axvm/src/irq/mod.rs b/virtualization/axvm/src/irq/mod.rs index 6082fb0a92..5ae6c5af25 100644 --- a/virtualization/axvm/src/irq/mod.rs +++ b/virtualization/axvm/src/irq/mod.rs @@ -16,14 +16,11 @@ use alloc::sync::Arc; -use axdevice::IrqResolver; -use axdevice_base::{ - AxError as DeviceError, AxResult as DeviceResult, InterruptTriggerMode, IrqLine, IrqLineId, - IrqSink, -}; +use axdevice::{DeviceManagerResult, IrqResolver}; +use axdevice_base::{InterruptTriggerMode, IrqError, IrqLine, IrqLineId, IrqResult, IrqSink}; use axvm_types::VMInterruptMode; -use crate::{AxVmError, AxVmResult, ax_err}; +use crate::{AxVmResult, ax_err}; /// Host platform hook for registering the RISC-V physical IRQ injector. #[ax_crate_interface::def_interface] @@ -96,30 +93,35 @@ impl InterruptFabric { self.sink.is_some() } - fn sink_for_line(&self, _line: usize) -> DeviceResult<&Arc> { + fn sink_for_line(&self, line: usize) -> IrqResult<&Arc> { let Some(sink) = &self.sink else { if self.mode == VMInterruptMode::NoIrq { - return Err(DeviceError::InvalidInput); + return Err(IrqError::InvalidLine { + line: IrqLineId(line), + operation: "resolve", + detail: "the VM is configured without interrupt delivery".into(), + }); } - return Err(DeviceError::Unsupported); + return Err(IrqError::Unsupported { + line: IrqLineId(line), + operation: "resolve", + detail: "no interrupt backend is installed".into(), + }); }; Ok(sink) } /// Sets the asserted state of a VM-local interrupt line. pub fn set_level(&self, line: usize, asserted: bool) -> AxVmResult { - self.sink_for_line(line) - .map_err(|error| AxVmError::interrupt("resolve interrupt line", error))? - .set_level(IrqLineId(line), asserted) - .map_err(|error| AxVmError::interrupt("set interrupt line level", error)) + self.sink_for_line(line)? + .set_level(IrqLineId(line), asserted)?; + Ok(()) } /// Delivers one pulse on a VM-local interrupt line. pub fn pulse(&self, line: usize) -> AxVmResult { - self.sink_for_line(line) - .map_err(|error| AxVmError::interrupt("resolve interrupt line", error))? - .pulse(IrqLineId(line)) - .map_err(|error| AxVmError::interrupt("pulse interrupt line", error)) + self.sink_for_line(line)?.pulse(IrqLineId(line))?; + Ok(()) } pub(crate) fn validate_mode(&self, mode: VMInterruptMode) -> AxVmResult { @@ -143,7 +145,11 @@ impl Default for InterruptFabric { } impl IrqResolver for InterruptFabric { - fn resolve_irq(&self, line: usize, trigger: InterruptTriggerMode) -> DeviceResult { + fn resolve_irq( + &self, + line: usize, + trigger: InterruptTriggerMode, + ) -> DeviceManagerResult { Ok(IrqLine::new( IrqLineId(line), trigger, diff --git a/virtualization/axvm/src/vm/mod.rs b/virtualization/axvm/src/vm/mod.rs index 1ac8073a49..e9795fc867 100644 --- a/virtualization/axvm/src/vm/mod.rs +++ b/virtualization/axvm/src/vm/mod.rs @@ -24,8 +24,8 @@ use ax_cpumask::CpuMask; use ax_kspin::SpinNoIrq as Mutex; use ax_memory_addr::align_up_4k; use axaddrspace::AddrSpace; -use axdevice::{AxVmDevices, FwCfg, FwCfgPlatformConfig}; -use axdevice_base::{AccessWidth, AxError as DeviceError}; +use axdevice::{AxVmDevices, DeviceManagerError, FwCfg, FwCfgPlatformConfig}; +use axdevice_base::AccessWidth; use axvm_types::{ GuestPhysAddr, HostPhysAddr, HostVirtAddr, MappingFlags, NestedPagingConfig, VmVcpuState, }; @@ -771,17 +771,15 @@ impl AxVM { pending.base.as_usize(), pending.base.as_usize() + pending.size ); - devices - .add_fw_cfg_dev(Arc::new(FwCfg::new( - pending.base, - pending.size, - pending.kernel, - pending.initrd, - pending.cmdline.as_deref(), - pending.cpu_num, - pending.platform, - ))) - .map_err(|error| AxVmError::device("register fw_cfg device", error))?; + devices.add_fw_cfg_dev(Arc::new(FwCfg::new( + pending.base, + pending.size, + pending.kernel, + pending.initrd, + pending.cmdline.as_deref(), + pending.cpu_num, + pending.platform, + )))?; } Ok(()) } @@ -794,30 +792,31 @@ impl AxVM { ) -> AxVmResult { let devices = self.get_devices()?; if let Some(fw_cfg) = devices.fw_cfg_for_dma_addr(addr) { - if let Some(desc_addr) = fw_cfg - .write_dma_address(addr, width, data) - .map_err(|error| AxVmError::device("write fw_cfg DMA address", error))? - { - fw_cfg - .process_dma( - desc_addr, - |gpa, buffer| { - self.read_from_guest(gpa, buffer) - .map_err(|_| DeviceError::InvalidData) - }, - |gpa, buffer| { - self.write_to_guest(gpa, buffer) - .map_err(|_| DeviceError::InvalidData) - }, - ) - .map_err(|error| AxVmError::device("process fw_cfg DMA request", error))?; + if let Some(desc_addr) = fw_cfg.write_dma_address(addr, width, data)? { + fw_cfg.process_dma( + desc_addr, + |gpa, buffer| { + self.read_from_guest(gpa, buffer).map_err(|error| { + DeviceManagerError::UnexpectedResponse { + operation: "read guest memory for fw_cfg DMA", + detail: alloc::format!("{error}"), + } + }) + }, + |gpa, buffer| { + self.write_to_guest(gpa, buffer).map_err(|error| { + DeviceManagerError::UnexpectedResponse { + operation: "write guest memory for fw_cfg DMA", + detail: alloc::format!("{error}"), + } + }) + }, + )?; } return Ok(()); } - devices - .handle_mmio_write(addr, width, data) - .map_err(|error| AxVmError::device("write guest MMIO", error))?; + devices.handle_mmio_write(addr, width, data)?; Ok(()) } diff --git a/virtualization/axvm/src/vm/prepare.rs b/virtualization/axvm/src/vm/prepare.rs index 6f7ef326d6..c2fa477eb5 100644 --- a/virtualization/axvm/src/vm/prepare.rs +++ b/virtualization/axvm/src/vm/prepare.rs @@ -10,7 +10,7 @@ use axdevice::{DeviceFactoryRegistry, register_builtin_factories}; use self::{devices::PreparedDevices, vcpus::PreparedVcpus}; use super::{AxVM, AxVMResources}; -use crate::{AxVmError, AxVmResult, ax_err, ax_err_type, irq::InterruptFabric}; +use crate::{AxVmResult, ax_err, ax_err_type, irq::InterruptFabric}; pub(crate) enum VmInitRequest<'a> { Default, @@ -55,8 +55,7 @@ impl AxVM { pub(crate) fn default_device_factories() -> AxVmResult { let mut factories = DeviceFactoryRegistry::new(); - register_builtin_factories(&mut factories) - .map_err(|error| AxVmError::device("register built-in device factories", error))?; + register_builtin_factories(&mut factories)?; Ok(factories) } diff --git a/virtualization/axvm/src/vm/prepare/devices.rs b/virtualization/axvm/src/vm/prepare/devices.rs index 8b783bff81..f9753b26ab 100644 --- a/virtualization/axvm/src/vm/prepare/devices.rs +++ b/virtualization/axvm/src/vm/prepare/devices.rs @@ -3,7 +3,7 @@ use axdevice::{AxVmDeviceConfig, AxVmDevices, DeviceBuildContext, DeviceFactoryRegistry}; use super::super::{AxVM, AxVMResources}; -use crate::{AxVmError, AxVmResult, irq::InterruptFabric}; +use crate::{AxVmResult, irq::InterruptFabric}; pub(crate) struct PreparedDevices { pub(crate) devices: AxVmDevices, @@ -22,8 +22,7 @@ impl PreparedDevices { }, factories, &build_context, - ) - .map_err(|error| AxVmError::device("build VM devices", error))?; + )?; Ok(Self { devices }) } diff --git a/virtualization/riscv_vplic/Cargo.toml b/virtualization/riscv_vplic/Cargo.toml index 76806505e2..ff08d14159 100644 --- a/virtualization/riscv_vplic/Cargo.toml +++ b/virtualization/riscv_vplic/Cargo.toml @@ -18,11 +18,11 @@ default = [] ax-kspin.workspace = true ax-crate-interface = { workspace = true } axdevice_base = { workspace = true } -ax-errno = { workspace = true } ax-memory-addr = { workspace = true } axvm-types = { workspace = true } bitmaps = { version = "3.2", default-features = false } log = "0.4" +thiserror = { workspace = true } riscv-h = { workspace = true } diff --git a/virtualization/riscv_vplic/src/devops_impl.rs b/virtualization/riscv_vplic/src/devops_impl.rs index a79688a29b..181609c453 100644 --- a/virtualization/riscv_vplic/src/devops_impl.rs +++ b/virtualization/riscv_vplic/src/devops_impl.rs @@ -2,12 +2,11 @@ //! //! Implements the `BaseDeviceOps` trait for MMIO read/write handling. -use ax_errno::{AxResult, ax_err}; -use axdevice_base::{AccessWidth, BaseDeviceOps, DeviceAddrRange, EmuDeviceType}; +use axdevice_base::{AccessWidth, BaseDeviceOps, DeviceAddrRange, DeviceResult, EmuDeviceType}; use axvm_types::{GuestPhysAddrRange, HostPhysAddr}; use bitmaps::Bitmap; -use crate::{consts::*, utils::*, vplic::VPlicGlobal}; +use crate::{VplicError, VplicResult, consts::*, utils::*, vplic::VPlicGlobal}; #[cfg(target_arch = "riscv64")] const VCAUSE_INTERRUPT_BIT: usize = 1usize << (usize::BITS - 1); @@ -16,33 +15,27 @@ const VCAUSE_VS_TIMER: usize = VCAUSE_INTERRUPT_BIT | 5; const PLIC_PENDING_WORDS: usize = PLIC_NUM_SOURCES / 32; impl VPlicGlobal { - fn validate_irq_id(irq_id: usize) -> AxResult { + fn validate_irq_id(irq_id: usize) -> VplicResult { if irq_id == 0 || irq_id >= PLIC_NUM_SOURCES { - return ax_err!( - InvalidInput, - format_args!( - "invalid PLIC source ID {irq_id}; valid source IDs are 1..{}", - PLIC_NUM_SOURCES - 1 - ) - ); + return Err(VplicError::InvalidSource { + source_id: irq_id, + max: PLIC_NUM_SOURCES, + }); } Ok(()) } - fn validate_assigned_irq(&self, irq_id: usize) -> AxResult { + fn validate_assigned_irq(&self, irq_id: usize) -> VplicResult { Self::validate_irq_id(irq_id)?; let assigned_irqs = self.assigned_irqs.lock(); if !assigned_irqs.is_empty() && !assigned_irqs.get(irq_id) { - return ax_err!( - PermissionDenied, - format_args!("PLIC source ID {irq_id} is not assigned to this virtual PLIC") - ); + return Err(VplicError::SourceNotAssigned { source_id: irq_id }); } Ok(()) } - fn update_pending_irq(&self, irq_id: usize, pending: bool) -> AxResult { + fn update_pending_irq(&self, irq_id: usize, pending: bool) -> VplicResult { self.validate_assigned_irq(irq_id)?; self.pending_irqs.lock().set(irq_id, pending); Ok(()) @@ -53,25 +46,25 @@ impl VPlicGlobal { /// Source ID 0 and IDs outside the PLIC source range are rejected. An /// empty assignment bitmap preserves the existing unrestricted behavior; /// once assignments are populated, only assigned sources are accepted. - pub fn set_pending(&self, irq_id: usize) -> AxResult { + pub fn set_pending(&self, irq_id: usize) -> VplicResult { self.update_pending_irq(irq_id, true)?; self.sync_all_guest_contexts_vseip() } /// Clears the pending state of one interrupt source. - pub fn clear_pending(&self, irq_id: usize) -> AxResult { + pub fn clear_pending(&self, irq_id: usize) -> VplicResult { self.update_pending_irq(irq_id, false)?; self.sync_all_guest_contexts_vseip() } /// Returns whether one interrupt source is pending. - pub fn is_pending(&self, irq_id: usize) -> AxResult { + pub fn is_pending(&self, irq_id: usize) -> VplicResult { self.validate_assigned_irq(irq_id)?; Ok(self.pending_irqs.lock().get(irq_id)) } /// Reads the priority of an interrupt source from the host PLIC. - fn irq_priority(&self, irq_id: usize) -> AxResult { + fn irq_priority(&self, irq_id: usize) -> VplicResult { let addr = HostPhysAddr::from_usize( self.host_plic_addr.as_usize() + PLIC_PRIORITY_OFFSET + irq_id * 4, ); @@ -80,7 +73,7 @@ impl VPlicGlobal { /// Reads the priority threshold configured for a PLIC context. #[cfg(target_arch = "riscv64")] - fn context_threshold(&self, context_id: usize) -> AxResult { + fn context_threshold(&self, context_id: usize) -> VplicResult { let addr = HostPhysAddr::from_usize( self.host_plic_addr.as_usize() + PLIC_CONTEXT_CTRL_OFFSET @@ -91,7 +84,7 @@ impl VPlicGlobal { } /// Reads one enable register word for a PLIC context. - fn context_enable_mask(&self, context_id: usize, reg_index: usize) -> AxResult { + fn context_enable_mask(&self, context_id: usize, reg_index: usize) -> VplicResult { let addr = HostPhysAddr::from_usize( self.host_plic_addr.as_usize() + PLIC_ENABLE_OFFSET @@ -116,7 +109,7 @@ impl VPlicGlobal { &self, context_id: usize, candidate_irqs: Bitmap<{ PLIC_NUM_SOURCES }>, - ) -> AxResult> { + ) -> VplicResult> { let mut best_irq = None; let mut best_priority = 0; let mut cached_enable_reg_index = usize::MAX; @@ -149,7 +142,7 @@ impl VPlicGlobal { /// Returns the next IRQ that should assert VSEIP for this context. #[cfg(target_arch = "riscv64")] - fn next_deliverable_irq(&self, context_id: usize) -> AxResult> { + fn next_deliverable_irq(&self, context_id: usize) -> VplicResult> { let threshold = self.context_threshold(context_id)?; let candidate_irqs = self.pending_inactive_irqs(); if let Some((irq_id, priority)) = @@ -162,7 +155,7 @@ impl VPlicGlobal { } /// Claims the next enabled pending IRQ and moves it to the active set. - fn claim_next_irq(&self, context_id: usize) -> AxResult> { + fn claim_next_irq(&self, context_id: usize) -> VplicResult> { loop { let candidate_irqs = self.pending_inactive_irqs(); let Some((irq_id, _priority)) = @@ -187,7 +180,7 @@ impl VPlicGlobal { /// Recomputes whether VSEIP should remain asserted for one context. #[cfg(target_arch = "riscv64")] - fn sync_vseip(&self, context_id: usize) -> AxResult<()> { + fn sync_vseip(&self, context_id: usize) -> VplicResult<()> { // VSEIP should track whether this context still has a deliverable // external interrupt, not merely whether some pending bit is set. if self.next_deliverable_irq(context_id)?.is_some() { @@ -210,12 +203,12 @@ impl VPlicGlobal { } #[cfg(not(target_arch = "riscv64"))] - fn sync_vseip(&self, _context_id: usize) -> AxResult<()> { + fn sync_vseip(&self, _context_id: usize) -> VplicResult<()> { Ok(()) } /// Recomputes VSEIP for all guest supervisor contexts. - fn sync_all_guest_contexts_vseip(&self) -> AxResult<()> { + fn sync_all_guest_contexts_vseip(&self) -> VplicResult<()> { for context_id in (1..self.contexts_num).step_by(2) { self.sync_vseip(context_id)?; } @@ -242,66 +235,80 @@ impl BaseDeviceOps for VPlicGlobal { &self, addr: ::Addr, width: AccessWidth, - ) -> ax_errno::AxResult { - assert_eq!(width, AccessWidth::Dword); - let reg = addr - self.addr; - let host_addr = HostPhysAddr::from_usize(reg + self.host_plic_addr.as_usize()); - // info!("vPlicGlobal read reg {reg:#x} width {width:?}"); - match reg { - // priority - PLIC_PRIORITY_OFFSET..PLIC_PENDING_OFFSET => perform_mmio_read(host_addr, width), - // pending - PLIC_PENDING_OFFSET..PLIC_ENABLE_OFFSET => { - let reg_index = (reg - PLIC_PENDING_OFFSET) / 4; - if reg_index >= PLIC_PENDING_WORDS { - return Ok(0); - } - let bit_index_start = reg_index * 32; - let mut val: u32 = 0; - let mut bit_mask: u32 = 1; - let pending_irqs = self.pending_irqs.lock(); - for i in 0..32 { - let irq_id = bit_index_start + i as usize; - if irq_id != 0 && pending_irqs.get(irq_id) { - val |= bit_mask; + ) -> DeviceResult { + let result = (|| -> VplicResult { + if width != AccessWidth::Dword { + return Err(VplicError::InvalidAccessWidth { + expected: AccessWidth::Dword, + actual: width, + }); + } + let reg = addr - self.addr; + let host_addr = HostPhysAddr::from_usize(reg + self.host_plic_addr.as_usize()); + // info!("vPlicGlobal read reg {reg:#x} width {width:?}"); + match reg { + // priority + PLIC_PRIORITY_OFFSET..PLIC_PENDING_OFFSET => perform_mmio_read(host_addr, width), + // pending + PLIC_PENDING_OFFSET..PLIC_ENABLE_OFFSET => { + let reg_index = (reg - PLIC_PENDING_OFFSET) / 4; + if reg_index >= PLIC_PENDING_WORDS { + return Ok(0); + } + let bit_index_start = reg_index * 32; + let mut val: u32 = 0; + let mut bit_mask: u32 = 1; + let pending_irqs = self.pending_irqs.lock(); + for i in 0..32 { + let irq_id = bit_index_start + i as usize; + if irq_id != 0 && pending_irqs.get(irq_id) { + val |= bit_mask; + } + bit_mask <<= 1; } - bit_mask <<= 1; + Ok(val as usize) } - Ok(val as usize) - } - // enable - PLIC_ENABLE_OFFSET..PLIC_CONTEXT_CTRL_OFFSET => perform_mmio_read(host_addr, width), - // threshold - offset - if offset >= PLIC_CONTEXT_CTRL_OFFSET - && (offset - PLIC_CONTEXT_CTRL_OFFSET).is_multiple_of(PLIC_CONTEXT_STRIDE) => - { - perform_mmio_read(host_addr, width) - } - // claim/complete - offset - if offset >= PLIC_CONTEXT_CTRL_OFFSET - && (offset - PLIC_CONTEXT_CTRL_OFFSET - PLIC_CONTEXT_CLAIM_COMPLETE_OFFSET) - .is_multiple_of(PLIC_CONTEXT_STRIDE) => - { - let context_id = - (offset - PLIC_CONTEXT_CTRL_OFFSET - PLIC_CONTEXT_CLAIM_COMPLETE_OFFSET) - / PLIC_CONTEXT_STRIDE; - assert!( - context_id < self.contexts_num, - "Invalid context id {context_id}" - ); - let Some(irq_id) = self.claim_next_irq(context_id)? else { + // enable + PLIC_ENABLE_OFFSET..PLIC_CONTEXT_CTRL_OFFSET => perform_mmio_read(host_addr, width), + // threshold + offset + if offset >= PLIC_CONTEXT_CTRL_OFFSET + && (offset - PLIC_CONTEXT_CTRL_OFFSET) + .is_multiple_of(PLIC_CONTEXT_STRIDE) => + { + perform_mmio_read(host_addr, width) + } + // claim/complete + offset + if offset >= PLIC_CONTEXT_CTRL_OFFSET + && (offset + - PLIC_CONTEXT_CTRL_OFFSET + - PLIC_CONTEXT_CLAIM_COMPLETE_OFFSET) + .is_multiple_of(PLIC_CONTEXT_STRIDE) => + { + let context_id = + (offset - PLIC_CONTEXT_CTRL_OFFSET - PLIC_CONTEXT_CLAIM_COMPLETE_OFFSET) + / PLIC_CONTEXT_STRIDE; + if context_id >= self.contexts_num { + return Err(VplicError::InvalidContext { + context: context_id, + contexts: self.contexts_num, + }); + } + let Some(irq_id) = self.claim_next_irq(context_id)? else { + self.sync_vseip(context_id)?; + return Ok(0); + }; self.sync_vseip(context_id)?; - return Ok(0); - }; - self.sync_vseip(context_id)?; - Ok(irq_id) - } - _ => { - unimplemented!("Unsupported vPlicGlobal read for reg {reg:#x}") + Ok(irq_id) + } + _ => Err(VplicError::UnsupportedRegister { + operation: "read", + offset: reg, + }), } - } + })(); + Ok(result?) } /// Handles MMIO write operations to the virtual PLIC. @@ -315,99 +322,117 @@ impl BaseDeviceOps for VPlicGlobal { addr: ::Addr, width: AccessWidth, val: usize, - ) -> ax_errno::AxResult { - assert_eq!(width, AccessWidth::Dword); - let reg = addr - self.addr; - let host_addr = HostPhysAddr::from_usize(reg + self.host_plic_addr.as_usize()); - // info!("vPlicGlobal write reg {reg:#x} width {width:?} val {val:#x}"); - match reg { - // priority - PLIC_PRIORITY_OFFSET..PLIC_PENDING_OFFSET => { - perform_mmio_write(host_addr, width, val)?; - self.sync_all_guest_contexts_vseip() + ) -> DeviceResult { + let result = (|| -> VplicResult { + if width != AccessWidth::Dword { + return Err(VplicError::InvalidAccessWidth { + expected: AccessWidth::Dword, + actual: width, + }); } - // pending (Here is uesd for hyperivosr to inject pending IRQs, later should move it to a separate interface) - PLIC_PENDING_OFFSET..PLIC_ENABLE_OFFSET => { - // Note: here append, not overwrite. - let reg_index = (reg - PLIC_PENDING_OFFSET) / 4; - if reg_index >= PLIC_PENDING_WORDS { - return Ok(()); + let reg = addr - self.addr; + let host_addr = HostPhysAddr::from_usize(reg + self.host_plic_addr.as_usize()); + // info!("vPlicGlobal write reg {reg:#x} width {width:?} val {val:#x}"); + match reg { + // priority + PLIC_PRIORITY_OFFSET..PLIC_PENDING_OFFSET => { + perform_mmio_write(host_addr, width, val)?; + self.sync_all_guest_contexts_vseip() } - let val = val as u32; - let mut bit_mask: u32 = 1; - for i in 0..32 { - if (val & bit_mask) != 0 { - let irq_id = reg_index * 32 + i; - if irq_id != 0 { - self.update_pending_irq(irq_id, true)?; + // pending (Here is uesd for hyperivosr to inject pending IRQs, later should move it to a separate interface) + PLIC_PENDING_OFFSET..PLIC_ENABLE_OFFSET => { + // Note: here append, not overwrite. + let reg_index = (reg - PLIC_PENDING_OFFSET) / 4; + if reg_index >= PLIC_PENDING_WORDS { + return Ok(()); + } + let val = val as u32; + let mut bit_mask: u32 = 1; + for i in 0..32 { + if (val & bit_mask) != 0 { + let irq_id = reg_index * 32 + i; + if irq_id != 0 { + self.update_pending_irq(irq_id, true)?; + } } + bit_mask <<= 1; } - bit_mask <<= 1; - } - self.sync_all_guest_contexts_vseip() - } - // enable - PLIC_ENABLE_OFFSET..PLIC_CONTEXT_CTRL_OFFSET => { - perform_mmio_write(host_addr, width, val)?; - let context_id = (reg - PLIC_ENABLE_OFFSET) / PLIC_ENABLE_STRIDE; - assert!( - context_id < self.contexts_num, - "Invalid context id {context_id}" - ); - // A mask update can instantly expose or hide already-pending IRQs. - self.sync_vseip(context_id) - } - // threshold - offset - if offset >= PLIC_CONTEXT_CTRL_OFFSET - && (offset - PLIC_CONTEXT_CTRL_OFFSET).is_multiple_of(PLIC_CONTEXT_STRIDE) => - { - let context_id = (offset - PLIC_CONTEXT_CTRL_OFFSET) / PLIC_CONTEXT_STRIDE; - assert!( - context_id < self.contexts_num, - "Invalid context id {context_id}" - ); - perform_mmio_write(host_addr, width, val)?; - // Threshold changes must be reflected on the hart line immediately. - self.sync_vseip(context_id) - } - // claim/complete - offset - if offset >= PLIC_CONTEXT_CTRL_OFFSET - && (offset - PLIC_CONTEXT_CTRL_OFFSET - PLIC_CONTEXT_CLAIM_COMPLETE_OFFSET) - .is_multiple_of(PLIC_CONTEXT_STRIDE) => - { - // info!("vPlicGlobal: Writing to CLAIM/COMPLETE reg {reg:#x} val {val:#x}"); - let context_id = - (offset - PLIC_CONTEXT_CTRL_OFFSET - PLIC_CONTEXT_CLAIM_COMPLETE_OFFSET) - / PLIC_CONTEXT_STRIDE; - assert!( - context_id < self.contexts_num, - "Invalid context id {context_id}" - ); - let irq_id = val; - - if irq_id == 0 || irq_id >= PLIC_NUM_SOURCES { - return self.sync_vseip(context_id); + self.sync_all_guest_contexts_vseip() } - let mut active_irqs = self.active_irqs.lock(); - if !active_irqs.get(irq_id) { - drop(active_irqs); - return self.sync_vseip(context_id); + // enable + PLIC_ENABLE_OFFSET..PLIC_CONTEXT_CTRL_OFFSET => { + perform_mmio_write(host_addr, width, val)?; + let context_id = (reg - PLIC_ENABLE_OFFSET) / PLIC_ENABLE_STRIDE; + if context_id >= self.contexts_num { + return Err(VplicError::InvalidContext { + context: context_id, + contexts: self.contexts_num, + }); + } + // A mask update can instantly expose or hide already-pending IRQs. + self.sync_vseip(context_id) } + // threshold + offset + if offset >= PLIC_CONTEXT_CTRL_OFFSET + && (offset - PLIC_CONTEXT_CTRL_OFFSET) + .is_multiple_of(PLIC_CONTEXT_STRIDE) => + { + let context_id = (offset - PLIC_CONTEXT_CTRL_OFFSET) / PLIC_CONTEXT_STRIDE; + if context_id >= self.contexts_num { + return Err(VplicError::InvalidContext { + context: context_id, + contexts: self.contexts_num, + }); + } + perform_mmio_write(host_addr, width, val)?; + // Threshold changes must be reflected on the hart line immediately. + self.sync_vseip(context_id) + } + // claim/complete + offset + if offset >= PLIC_CONTEXT_CTRL_OFFSET + && (offset + - PLIC_CONTEXT_CTRL_OFFSET + - PLIC_CONTEXT_CLAIM_COMPLETE_OFFSET) + .is_multiple_of(PLIC_CONTEXT_STRIDE) => + { + // info!("vPlicGlobal: Writing to CLAIM/COMPLETE reg {reg:#x} val {val:#x}"); + let context_id = + (offset - PLIC_CONTEXT_CTRL_OFFSET - PLIC_CONTEXT_CLAIM_COMPLETE_OFFSET) + / PLIC_CONTEXT_STRIDE; + if context_id >= self.contexts_num { + return Err(VplicError::InvalidContext { + context: context_id, + contexts: self.contexts_num, + }); + } + let irq_id = val; - // Write host PLIC. - perform_mmio_write(host_addr, width, irq_id)?; - // Clear the active bit only after the completion is accepted. - active_irqs.set(irq_id, false); - drop(active_irqs); - self.sync_vseip(context_id) - } - _ => { - unimplemented!("Unsupported vPlicGlobal read for reg {reg:#x}") + if irq_id == 0 || irq_id >= PLIC_NUM_SOURCES { + return self.sync_vseip(context_id); + } + let mut active_irqs = self.active_irqs.lock(); + if !active_irqs.get(irq_id) { + drop(active_irqs); + return self.sync_vseip(context_id); + } + + // Write host PLIC. + perform_mmio_write(host_addr, width, irq_id)?; + // Clear the active bit only after the completion is accepted. + active_irqs.set(irq_id, false); + drop(active_irqs); + self.sync_vseip(context_id) + } + _ => Err(VplicError::UnsupportedRegister { + operation: "write", + offset: reg, + }), } - } + })(); + Ok(result?) } } @@ -419,7 +444,7 @@ mod tests { #[test] fn pending_inactive_irqs_excludes_reserved_irq_zero() { - let vplic = VPlicGlobal::new(GuestPhysAddr::from(0x0c00_0000), Some(0x400000), 2); + let vplic = VPlicGlobal::new(GuestPhysAddr::from(0x0c00_0000), Some(0x400000), 2).unwrap(); { let mut pending_irqs = vplic.pending_irqs.lock(); diff --git a/virtualization/riscv_vplic/src/error.rs b/virtualization/riscv_vplic/src/error.rs new file mode 100644 index 0000000000..87e43464c6 --- /dev/null +++ b/virtualization/riscv_vplic/src/error.rs @@ -0,0 +1,102 @@ +//! Typed errors reported by the virtual platform-level interrupt controller. + +use alloc::string::String; + +use axdevice_base::{AccessWidth, DeviceError}; + +/// Result type returned by virtual PLIC operations. +pub type VplicResult = Result; + +/// Errors reported by the virtual platform-level interrupt controller. +#[derive(Clone, Debug, Eq, PartialEq, thiserror::Error)] +pub enum VplicError { + /// The virtual PLIC memory-region size was not provided. + #[error("vPLIC memory-region size is required")] + MissingRegionSize, + /// The virtual PLIC register range overflows the guest address space. + #[error("vPLIC register range overflows the guest address space")] + AddressOverflow, + /// The configured memory region does not cover all PLIC contexts. + #[error( + "vPLIC region [{base:#x}, {region_end:#x}) does not cover required end {required_end:#x}" + )] + InsufficientRegion { + /// Base guest physical address of the region. + base: usize, + /// Exclusive end of the configured region. + region_end: usize, + /// Exclusive end required by the configured contexts. + required_end: usize, + }, + /// A PLIC source identifier is outside the valid range. + #[error("vPLIC source ID {source_id} is outside the valid range 1..{max}")] + InvalidSource { + /// The rejected source identifier. + source_id: usize, + /// The exclusive upper bound for source identifiers. + max: usize, + }, + /// A source is not assigned to this virtual PLIC. + #[error("vPLIC source ID {source_id} is not assigned to this controller")] + SourceNotAssigned { + /// The unassigned source identifier. + source_id: usize, + }, + /// A context identifier is outside the configured range. + #[error("vPLIC context ID {context} is outside the configured range 0..{contexts}")] + InvalidContext { + /// The rejected context identifier. + context: usize, + /// The number of configured contexts. + contexts: usize, + }, + /// A register access uses an unsupported width. + #[error("invalid vPLIC access width: expected {expected:?}, got {actual:?}")] + InvalidAccessWidth { + /// The required register access width. + expected: AccessWidth, + /// The requested register access width. + actual: AccessWidth, + }, + /// A register operation is unsupported. + #[error("unsupported vPLIC {operation} at register offset {offset:#x}")] + UnsupportedRegister { + /// Whether the access is a read or write. + operation: &'static str, + /// The unsupported register offset. + offset: usize, + }, + /// A host PLIC or MMIO backend operation failed. + #[error("vPLIC backend operation {operation} failed: {detail}")] + Backend { + /// The backend operation that failed. + operation: &'static str, + /// Diagnostic detail from the backend. + detail: String, + }, +} + +impl From for DeviceError { + fn from(error: VplicError) -> Self { + match error { + VplicError::InvalidSource { .. } + | VplicError::SourceNotAssigned { .. } + | VplicError::InvalidContext { .. } + | VplicError::InvalidAccessWidth { .. } + | VplicError::MissingRegionSize + | VplicError::AddressOverflow + | VplicError::InsufficientRegion { .. } => Self::InvalidInput { + operation: "access RISC-V vPLIC", + detail: alloc::format!("{error}"), + }, + VplicError::UnsupportedRegister { .. } => Self::Unsupported { + operation: "access RISC-V vPLIC", + detail: alloc::format!("{error}"), + }, + VplicError::Backend { .. } => Self::Backend { + operation: "access RISC-V vPLIC", + detail: alloc::format!("{error}"), + }, + } + } +} diff --git a/virtualization/riscv_vplic/src/lib.rs b/virtualization/riscv_vplic/src/lib.rs index ded596d34e..e62b5918a0 100644 --- a/virtualization/riscv_vplic/src/lib.rs +++ b/virtualization/riscv_vplic/src/lib.rs @@ -15,16 +15,21 @@ //! use riscv_vplic::VPlicGlobal; //! //! // Create a virtual PLIC with 2 contexts -//! let vplic = VPlicGlobal::new(GuestPhysAddr::from(0x0c000000), Some(0x4000), 2); +//! let vplic = VPlicGlobal::new(GuestPhysAddr::from(0x0c000000), Some(0x4000), 2)?; +//! # Ok::<(), riscv_vplic::VplicError>(()) //! ``` #![cfg_attr(not(test), no_std)] +extern crate alloc; + mod consts; mod devops_impl; +mod error; pub mod host; mod utils; mod vplic; pub use consts::*; +pub use error::{VplicError, VplicResult}; pub use vplic::VPlicGlobal; diff --git a/virtualization/riscv_vplic/src/utils.rs b/virtualization/riscv_vplic/src/utils.rs index 28ae7eb1bf..d79a36ba3b 100644 --- a/virtualization/riscv_vplic/src/utils.rs +++ b/virtualization/riscv_vplic/src/utils.rs @@ -4,14 +4,13 @@ use core::result::Result::Ok; -use ax_errno::AxResult; use axdevice_base::AccessWidth; use axvm_types::HostPhysAddr; -use crate::host; +use crate::{VplicResult, host}; /// Performs a volatile MMIO read operation. -pub(crate) fn perform_mmio_read(addr: HostPhysAddr, width: AccessWidth) -> AxResult { +pub(crate) fn perform_mmio_read(addr: HostPhysAddr, width: AccessWidth) -> VplicResult { let addr = host::phys_to_virt(addr).as_ptr(); match width { @@ -27,7 +26,7 @@ pub(crate) fn perform_mmio_write( addr: HostPhysAddr, width: AccessWidth, val: usize, -) -> AxResult<()> { +) -> VplicResult<()> { let addr = host::phys_to_virt(addr).as_mut_ptr(); match width { diff --git a/virtualization/riscv_vplic/src/vplic.rs b/virtualization/riscv_vplic/src/vplic.rs index 50c9fbf9e8..022d7801fd 100644 --- a/virtualization/riscv_vplic/src/vplic.rs +++ b/virtualization/riscv_vplic/src/vplic.rs @@ -8,7 +8,7 @@ use ax_kspin::SpinNoIrq as Mutex; use axvm_types::{GuestPhysAddr, HostPhysAddr}; use bitmaps::Bitmap; -use crate::consts::*; +use crate::{VplicError, VplicResult, consts::*}; /// Virtual PLIC global controller. /// @@ -39,22 +39,28 @@ impl VPlicGlobal { /// * `size` - Size of the PLIC memory region in bytes /// * `contexts_num` - Number of interrupt contexts (typically equal to number of harts) /// - /// # Panics - /// Panics if the provided size is insufficient to hold all PLIC registers. - pub fn new(addr: GuestPhysAddr, size: Option, contexts_num: usize) -> Self { - let addr_end = addr.as_usize() - + contexts_num * PLIC_CONTEXT_STRIDE - + PLIC_CONTEXT_CTRL_OFFSET - + PLIC_CONTEXT_CLAIM_COMPLETE_OFFSET; - let size = size.expect("Size must be specified for VPlicGlobal"); - assert!( - addr.as_usize() + size > addr_end, - "End address 0x{:x} exceeds region [0x{:x}, 0x{:x}) ", - addr_end, - addr.as_usize(), - addr.as_usize() + size, - ); - Self { + /// # Errors + /// + /// Returns an error if `size` is absent, the address calculation + /// overflows, or the region cannot cover all configured contexts. + pub fn new(addr: GuestPhysAddr, size: Option, contexts_num: usize) -> VplicResult { + let base = addr.as_usize(); + let required_end = contexts_num + .checked_mul(PLIC_CONTEXT_STRIDE) + .and_then(|offset| offset.checked_add(PLIC_CONTEXT_CTRL_OFFSET)) + .and_then(|offset| offset.checked_add(PLIC_CONTEXT_CLAIM_COMPLETE_OFFSET)) + .and_then(|offset| base.checked_add(offset)) + .ok_or(VplicError::AddressOverflow)?; + let size = size.ok_or(VplicError::MissingRegionSize)?; + let region_end = base.checked_add(size).ok_or(VplicError::AddressOverflow)?; + if region_end <= required_end { + return Err(VplicError::InsufficientRegion { + base, + region_end, + required_end, + }); + } + Ok(Self { addr, size, assigned_irqs: Mutex::new(Bitmap::new()), @@ -62,7 +68,7 @@ impl VPlicGlobal { active_irqs: Mutex::new(Bitmap::new()), contexts_num, host_plic_addr: HostPhysAddr::from_usize(addr.as_usize()), /* Currently we assume host_plic_addr = guest_vplic_addr */ - } + }) } // pub fn assign_irq(&self, irq: u32, cpu_phys_id: usize, target_cpu_affinity: (u8, u8, u8, u8)) { diff --git a/virtualization/riscv_vplic/tests/error_contract.rs b/virtualization/riscv_vplic/tests/error_contract.rs new file mode 100644 index 0000000000..7f257c65eb --- /dev/null +++ b/virtualization/riscv_vplic/tests/error_contract.rs @@ -0,0 +1,36 @@ +use std::{fs, path::Path}; + +use axdevice_base::{AccessWidth, DeviceError}; +use riscv_vplic::VplicError; + +#[test] +fn crate_uses_typed_errors_without_errno_contracts() { + let crate_dir = Path::new(env!("CARGO_MANIFEST_DIR")); + let manifest = fs::read_to_string(crate_dir.join("Cargo.toml")).unwrap(); + assert!(!manifest.contains(&["ax", "errno"].join("-"))); + assert_directory_excludes(&crate_dir.join("src"), &["ax", "errno"].join("_")); +} + +#[test] +fn invalid_vplic_width_converts_to_device_input_error() { + let error = VplicError::InvalidAccessWidth { + expected: AccessWidth::Dword, + actual: AccessWidth::Qword, + }; + let device: DeviceError = error.into(); + + assert!(matches!(device, DeviceError::InvalidInput { .. })); + assert!(device.to_string().contains("Qword")); +} + +fn assert_directory_excludes(directory: &Path, forbidden: &str) { + for entry in fs::read_dir(directory).unwrap() { + let path = entry.unwrap().path(); + if path.is_dir() { + assert_directory_excludes(&path, forbidden); + } else if path.extension().is_some_and(|extension| extension == "rs") { + let source = fs::read_to_string(&path).unwrap(); + assert!(!source.contains(forbidden), "{}", path.display()); + } + } +} diff --git a/virtualization/riscv_vplic/tests/vplic_tests.rs b/virtualization/riscv_vplic/tests/vplic_tests.rs index ad058fa0af..b599ecc64c 100644 --- a/virtualization/riscv_vplic/tests/vplic_tests.rs +++ b/virtualization/riscv_vplic/tests/vplic_tests.rs @@ -1,12 +1,11 @@ use ax_crate_interface::impl_interface; -use ax_errno::AxError; use ax_memory_addr::{PhysAddr, VirtAddr}; use axdevice_base::{AccessWidth, BaseDeviceOps}; use axvm_types::GuestPhysAddr; use riscv_vplic::{ PLIC_CONTEXT_CLAIM_COMPLETE_OFFSET, PLIC_CONTEXT_CTRL_OFFSET, PLIC_CONTEXT_STRIDE, PLIC_ENABLE_OFFSET, PLIC_ENABLE_STRIDE, PLIC_NUM_SOURCES, PLIC_PENDING_OFFSET, - PLIC_PRIORITY_OFFSET, VPlicGlobal, host::RiscvVplicHostIf, + PLIC_PRIORITY_OFFSET, VPlicGlobal, VplicError, host::RiscvVplicHostIf, }; const HOST_PLIC_BASE: usize = 0x0c00_0000; @@ -43,7 +42,7 @@ fn test_vplic_global_creation() { let contexts_num = 2; let size = calculate_min_size(contexts_num); - let vplic = VPlicGlobal::new(addr, Some(size), contexts_num); + let vplic = VPlicGlobal::new(addr, Some(size), contexts_num).unwrap(); assert_eq!(vplic.addr, addr); assert_eq!(vplic.size, size); @@ -55,37 +54,40 @@ fn test_vplic_global_with_different_contexts() { let addr = GuestPhysAddr::from(0x0c000000); // Test with 1 context - let vplic = VPlicGlobal::new(addr, Some(0x400000), 1); + let vplic = VPlicGlobal::new(addr, Some(0x400000), 1).unwrap(); assert_eq!(vplic.contexts_num, 1); // Test with 4 contexts - let vplic = VPlicGlobal::new(addr, Some(0x400000), 4); + let vplic = VPlicGlobal::new(addr, Some(0x400000), 4).unwrap(); assert_eq!(vplic.contexts_num, 4); // Test with 8 contexts - let vplic = VPlicGlobal::new(addr, Some(0x400000), 8); + let vplic = VPlicGlobal::new(addr, Some(0x400000), 8).unwrap(); assert_eq!(vplic.contexts_num, 8); } #[test] -#[should_panic(expected = "Size must be specified")] -fn test_vplic_global_size_none_panics() { +fn test_vplic_global_missing_size_returns_typed_error() { let addr = GuestPhysAddr::from(0x0c000000); - let _ = VPlicGlobal::new(addr, None, 2); + assert!(matches!( + VPlicGlobal::new(addr, None, 2), + Err(VplicError::MissingRegionSize) + )); } #[test] -#[should_panic(expected = "exceeds region")] -fn test_vplic_global_insufficient_size_panics() { +fn test_vplic_global_insufficient_size_returns_typed_error() { let addr = GuestPhysAddr::from(0x0c000000); - // Size too small for 2 contexts - let _ = VPlicGlobal::new(addr, Some(0x1000), 2); + assert!(matches!( + VPlicGlobal::new(addr, Some(0x1000), 2), + Err(VplicError::InsufficientRegion { .. }) + )); } #[test] fn test_vplic_global_bitmaps_initialized_empty() { let addr = GuestPhysAddr::from(0x0c000000); - let vplic = VPlicGlobal::new(addr, Some(0x400000), 2); + let vplic = VPlicGlobal::new(addr, Some(0x400000), 2).unwrap(); assert!(vplic.assigned_irqs.lock().is_empty()); assert!(vplic.pending_irqs.lock().is_empty()); @@ -95,7 +97,7 @@ fn test_vplic_global_bitmaps_initialized_empty() { #[test] fn test_typed_pending_api_is_visible_through_mmio() { let addr = GuestPhysAddr::from(HOST_PLIC_BASE); - let vplic = VPlicGlobal::new(addr, Some(HOST_PLIC_SIZE), 2); + let vplic = VPlicGlobal::new(addr, Some(HOST_PLIC_SIZE), 2).unwrap(); vplic.set_pending(33).unwrap(); @@ -113,23 +115,36 @@ fn test_typed_pending_api_is_visible_through_mmio() { #[test] fn test_pending_api_rejects_reserved_unassigned_and_out_of_range_sources() { - let vplic = VPlicGlobal::new(GuestPhysAddr::from(HOST_PLIC_BASE), Some(HOST_PLIC_SIZE), 2); + let vplic = + VPlicGlobal::new(GuestPhysAddr::from(HOST_PLIC_BASE), Some(HOST_PLIC_SIZE), 2).unwrap(); - assert_eq!(vplic.set_pending(0), Err(AxError::InvalidInput)); + assert_eq!( + vplic.set_pending(0), + Err(VplicError::InvalidSource { + source_id: 0, + max: PLIC_NUM_SOURCES, + }) + ); assert_eq!( vplic.set_pending(PLIC_NUM_SOURCES), - Err(AxError::InvalidInput) + Err(VplicError::InvalidSource { + source_id: PLIC_NUM_SOURCES, + max: PLIC_NUM_SOURCES, + }) ); vplic.assigned_irqs.lock().set(5, true); - assert_eq!(vplic.set_pending(6), Err(AxError::PermissionDenied)); + assert_eq!( + vplic.set_pending(6), + Err(VplicError::SourceNotAssigned { source_id: 6 }) + ); assert_eq!(vplic.set_pending(5), Ok(())); } #[test] fn test_claim_and_complete_move_irq_between_pending_and_active() { let addr = GuestPhysAddr::from(HOST_PLIC_BASE); - let vplic = VPlicGlobal::new(addr, Some(HOST_PLIC_SIZE), 2); + let vplic = VPlicGlobal::new(addr, Some(HOST_PLIC_SIZE), 2).unwrap(); let irq_id = 7; let context_id = 1; @@ -168,8 +183,10 @@ fn test_claim_and_complete_move_irq_between_pending_and_active() { #[test] fn test_virtual_plic_instances_and_guest_addresses_are_independent() { - let first = VPlicGlobal::new(GuestPhysAddr::from(0x0c00_0000), Some(HOST_PLIC_SIZE), 2); - let second = VPlicGlobal::new(GuestPhysAddr::from(0x1c00_0000), Some(HOST_PLIC_SIZE), 2); + let first = + VPlicGlobal::new(GuestPhysAddr::from(0x0c00_0000), Some(HOST_PLIC_SIZE), 2).unwrap(); + let second = + VPlicGlobal::new(GuestPhysAddr::from(0x1c00_0000), Some(HOST_PLIC_SIZE), 2).unwrap(); first.set_pending(11).unwrap(); diff --git a/virtualization/test_crates/virtualization-tests/Cargo.toml b/virtualization/test_crates/virtualization-tests/Cargo.toml index 9798fbe832..dc0d9f91f9 100644 --- a/virtualization/test_crates/virtualization-tests/Cargo.toml +++ b/virtualization/test_crates/virtualization-tests/Cargo.toml @@ -9,7 +9,6 @@ repository.workspace = true [dependencies] [dev-dependencies] -ax-errno.workspace = true ax-plat.workspace = true axdevice.workspace = true axdevice_base.workspace = true diff --git a/virtualization/test_crates/virtualization-tests/tests/axdevice.rs b/virtualization/test_crates/virtualization-tests/tests/axdevice.rs index 5fceeb4e37..7c7653bbf4 100644 --- a/virtualization/test_crates/virtualization-tests/tests/axdevice.rs +++ b/virtualization/test_crates/virtualization-tests/tests/axdevice.rs @@ -14,15 +14,15 @@ use std::sync::{Arc, Mutex}; -use ax_errno::{AxError, AxResult}; use axdevice::{ AxVmDeviceConfig, AxVmDevices, DeviceBuildContext, DeviceBundle, DeviceFactory, - DeviceFactoryRegistry, DeviceRegistration, IrqResolver, MmioDeviceAdapter, PollableDeviceOps, - PortDeviceAdapter, SysRegDeviceAdapter, register_builtin_factories, + DeviceFactoryRegistry, DeviceManagerError, DeviceManagerResult, DeviceRegistration, + IrqResolver, MmioDeviceAdapter, PollableDeviceOps, PortDeviceAdapter, SysRegDeviceAdapter, + register_builtin_factories, }; use axdevice_base::{ - AccessWidth, BaseDeviceOps, DeviceRegistry as _, InterruptTriggerMode, IrqLine, Port, - PortRange, RegistryError, SysRegAddr, SysRegAddrRange, + AccessWidth, BaseDeviceOps, DeviceRegistry as _, DeviceResult, InterruptTriggerMode, IrqError, + IrqLine, IrqLineId, Port, PortRange, RegistryError, SysRegAddr, SysRegAddrRange, }; use axvm_types::{EmulatedDeviceConfig, EmulatedDeviceType, GuestPhysAddr, GuestPhysAddrRange}; @@ -30,14 +30,8 @@ use axvm_types::{EmulatedDeviceConfig, EmulatedDeviceType, GuestPhysAddr, GuestP fn register_mmio + Send + Sync + 'static>( devices: &mut AxVmDevices, dev: Arc, -) -> AxResult { - devices - .register(MmioDeviceAdapter::from_arc(dev)) - .map_err(|e| match e { - RegistryError::AddressConflict { .. } => AxError::AddrInUse, - RegistryError::InvalidResource { .. } => AxError::InvalidInput, - _ => AxError::InvalidInput, - })?; +) -> Result<(), RegistryError> { + devices.register(MmioDeviceAdapter::from_arc(dev))?; Ok(()) } @@ -45,14 +39,8 @@ fn register_mmio + Send + Sync + 'static>( fn register_port + Send + Sync + 'static>( devices: &mut AxVmDevices, dev: Arc, -) -> AxResult { - devices - .register(PortDeviceAdapter::from_arc(dev)) - .map_err(|e| match e { - RegistryError::AddressConflict { .. } => AxError::AddrInUse, - RegistryError::InvalidResource { .. } => AxError::InvalidInput, - _ => AxError::InvalidInput, - })?; +) -> Result<(), RegistryError> { + devices.register(PortDeviceAdapter::from_arc(dev))?; Ok(()) } @@ -60,14 +48,8 @@ fn register_port + Send + Sync + 'static>( fn register_sysreg + Send + Sync + 'static>( devices: &mut AxVmDevices, dev: Arc, -) -> AxResult { - devices - .register(SysRegDeviceAdapter::from_arc(dev)) - .map_err(|e| match e { - RegistryError::AddressConflict { .. } => AxError::AddrInUse, - RegistryError::InvalidResource { .. } => AxError::InvalidInput, - _ => AxError::InvalidInput, - })?; +) -> Result<(), RegistryError> { + devices.register(SysRegDeviceAdapter::from_arc(dev))?; Ok(()) } @@ -107,11 +89,11 @@ impl BaseDeviceOps for MockMmioDevice { EmulatedDeviceType::IVCChannel } - fn handle_read(&self, _addr: GuestPhysAddr, _width: AccessWidth) -> AxResult { + fn handle_read(&self, _addr: GuestPhysAddr, _width: AccessWidth) -> DeviceResult { Ok(0xDEAD_BEEF) } - fn handle_write(&self, addr: GuestPhysAddr, _width: AccessWidth, val: usize) -> AxResult { + fn handle_write(&self, addr: GuestPhysAddr, _width: AccessWidth, val: usize) -> DeviceResult { println!( "[Test] Device {} write: addr={:?}, val={:#x}", self.name, addr, val @@ -144,11 +126,11 @@ impl BaseDeviceOps for MockPortDevice { EmulatedDeviceType::Console } - fn handle_read(&self, _addr: Port, _width: AccessWidth) -> AxResult { + fn handle_read(&self, _addr: Port, _width: AccessWidth) -> DeviceResult { Ok(0) } - fn handle_write(&self, _addr: Port, _width: AccessWidth, _val: usize) -> AxResult { + fn handle_write(&self, _addr: Port, _width: AccessWidth, _val: usize) -> DeviceResult { Ok(()) } } @@ -184,17 +166,17 @@ impl BaseDeviceOps for MockMmioPollableDevice { EmulatedDeviceType::IVCChannel } - fn handle_read(&self, _addr: GuestPhysAddr, _width: AccessWidth) -> AxResult { + fn handle_read(&self, _addr: GuestPhysAddr, _width: AccessWidth) -> DeviceResult { Ok(0) } - fn handle_write(&self, _addr: GuestPhysAddr, _width: AccessWidth, _val: usize) -> AxResult { + fn handle_write(&self, _addr: GuestPhysAddr, _width: AccessWidth, _val: usize) -> DeviceResult { Ok(()) } } impl PollableDeviceOps for MockMmioPollableDevice { - fn poll(&self, now_ns: u64) -> AxResult { + fn poll(&self, now_ns: u64) -> DeviceManagerResult { self.polled_at.lock().unwrap().push(now_ns); Ok(()) } @@ -217,11 +199,11 @@ impl BaseDeviceOps for MockSysRegDevice { EmulatedDeviceType::InterruptController } - fn handle_read(&self, _addr: SysRegAddr, _width: AccessWidth) -> AxResult { + fn handle_read(&self, _addr: SysRegAddr, _width: AccessWidth) -> DeviceResult { Ok(0) } - fn handle_write(&self, _addr: SysRegAddr, _width: AccessWidth, _val: usize) -> AxResult { + fn handle_write(&self, _addr: SysRegAddr, _width: AccessWidth, _val: usize) -> DeviceResult { Ok(()) } } @@ -256,8 +238,17 @@ fn device_config( struct RejectingIrqResolver; impl IrqResolver for RejectingIrqResolver { - fn resolve_irq(&self, _line: usize, _trigger: InterruptTriggerMode) -> AxResult { - Err(AxError::Unsupported) + fn resolve_irq( + &self, + line: usize, + _trigger: InterruptTriggerMode, + ) -> DeviceManagerResult { + Err(IrqError::Unsupported { + line: IrqLineId(line), + operation: "resolve test IRQ", + detail: "test resolver rejects every line".into(), + } + .into()) } } @@ -272,12 +263,18 @@ impl DeviceFactory for MockMmioFactory { &self, config: &EmulatedDeviceConfig, _context: &DeviceBuildContext<'_>, - ) -> AxResult { + ) -> DeviceManagerResult { let Some(end) = config.base_gpa.checked_add(config.length) else { - return Err(AxError::InvalidInput); + return Err(DeviceManagerError::InvalidConfig { + operation: "build mock MMIO device", + detail: "device address range overflows".into(), + }); }; if config.length == 0 { - return Err(AxError::InvalidInput); + return Err(DeviceManagerError::InvalidConfig { + operation: "build mock MMIO device", + detail: "device range is empty".into(), + }); } Ok( @@ -357,30 +354,30 @@ fn test_mmio_duplicate_and_overlapping_ranges_are_rejected_without_modification( let existing = mmio_device("existing", 0x2000, 0x3000); assert_eq!(register_mmio(&mut devices, existing.clone()), Ok(())); - assert_eq!( + assert!(matches!( register_mmio(&mut devices, existing), - Err(AxError::AddrInUse) - ); - assert_eq!( + Err(RegistryError::AddressConflict { .. }) + )); + assert!(matches!( register_mmio(&mut devices, mmio_device("same-range", 0x2000, 0x3000)), - Err(AxError::AddrInUse) - ); - assert_eq!( + Err(RegistryError::AddressConflict { .. }) + )); + assert!(matches!( register_mmio(&mut devices, mmio_device("partial-left", 0x1800, 0x2800)), - Err(AxError::AddrInUse) - ); - assert_eq!( + Err(RegistryError::AddressConflict { .. }) + )); + assert!(matches!( register_mmio(&mut devices, mmio_device("partial-right", 0x2800, 0x3800)), - Err(AxError::AddrInUse) - ); - assert_eq!( + Err(RegistryError::AddressConflict { .. }) + )); + assert!(matches!( register_mmio(&mut devices, mmio_device("contains", 0x1000, 0x4000)), - Err(AxError::AddrInUse) - ); - assert_eq!( + Err(RegistryError::AddressConflict { .. }) + )); + assert!(matches!( register_mmio(&mut devices, mmio_device("contained", 0x2400, 0x2800)), - Err(AxError::AddrInUse) - ); + Err(RegistryError::AddressConflict { .. }) + )); assert_eq!(devices.devices().count(), 1); } @@ -401,22 +398,22 @@ fn test_empty_and_wrapped_ranges_are_rejected() { let invalid_port = Arc::new(MockPortDevice::new(0x400, 0x3ff)); let invalid_sysreg = Arc::new(MockSysRegDevice::new(0x101, 0x100)); - assert_eq!( + assert!(matches!( register_mmio(&mut devices, empty_mmio), - Err(AxError::InvalidInput) - ); - assert_eq!( + Err(RegistryError::InvalidResource { .. }) + )); + assert!(matches!( register_mmio(&mut devices, wrapped_mmio), - Err(AxError::InvalidInput) - ); - assert_eq!( + Err(RegistryError::InvalidResource { .. }) + )); + assert!(matches!( register_port(&mut devices, invalid_port), - Err(AxError::InvalidInput) - ); - assert_eq!( + Err(RegistryError::InvalidResource { .. }) + )); + assert!(matches!( register_sysreg(&mut devices, invalid_sysreg), - Err(AxError::InvalidInput) - ); + Err(RegistryError::InvalidResource { .. }) + )); assert_eq!(devices.devices().count(), 0); } @@ -428,10 +425,10 @@ fn test_port_inclusive_endpoint_overlap_is_rejected() { register_port(&mut devices, Arc::new(MockPortDevice::new(0x3f8, 0x3ff))), Ok(()) ); - assert_eq!( + assert!(matches!( register_port(&mut devices, Arc::new(MockPortDevice::new(0x3ff, 0x400))), - Err(AxError::AddrInUse) - ); + Err(RegistryError::AddressConflict { .. }) + )); assert_eq!( register_port(&mut devices, Arc::new(MockPortDevice::new(0x400, 0x400))), Ok(()) @@ -447,10 +444,10 @@ fn test_sysreg_inclusive_endpoint_overlap_is_rejected() { register_sysreg(&mut devices, Arc::new(MockSysRegDevice::new(0x100, 0x110))), Ok(()) ); - assert_eq!( + assert!(matches!( register_sysreg(&mut devices, Arc::new(MockSysRegDevice::new(0x110, 0x120))), - Err(AxError::AddrInUse) - ); + Err(RegistryError::AddressConflict { .. }) + )); assert_eq!( register_sysreg(&mut devices, Arc::new(MockSysRegDevice::new(0x111, 0x120))), Ok(()) @@ -499,15 +496,17 @@ fn test_conflicting_factory_device_config_returns_structured_error() { 0x1000, ); - assert_eq!( + assert!(matches!( AxVmDevices::build_with_factories( AxVmDeviceConfig::new(vec![first, overlap]), &factories, &context, ) .err(), - Some(AxError::AddrInUse) - ); + Some(DeviceManagerError::Registry( + RegistryError::AddressConflict { .. } + )) + )); } #[test] @@ -539,10 +538,12 @@ fn test_bundle_internal_conflict_is_atomic() { Arc::new(MockPortDevice::new(0x500, 0x50f)), ))); - assert_eq!( + assert!(matches!( devices.register_bundle(bundle).err(), - Some(AxError::AddrInUse) - ); + Some(DeviceManagerError::Registry( + RegistryError::AddressConflict { .. } + )) + )); assert_eq!(devices.devices().count(), 0); } @@ -563,10 +564,12 @@ fn test_bundle_existing_conflict_leaves_all_registries_unchanged() { Arc::new(MockSysRegDevice::new(0x200, 0x210)), ))); - assert_eq!( + assert!(matches!( devices.register_bundle(bundle).err(), - Some(AxError::AddrInUse) - ); + Some(DeviceManagerError::Registry( + RegistryError::AddressConflict { .. } + )) + )); assert_eq!(devices.devices().count(), count_before); } @@ -607,10 +610,10 @@ fn test_duplicate_pollable_rejects_entire_bundle() { ))); bundle.push(DeviceRegistration::Pollable(shared)); - assert_eq!( + assert!(matches!( devices.register_bundle(bundle).err(), - Some(AxError::AlreadyExists) - ); + Some(DeviceManagerError::ResourceConflict { .. }) + )); assert_eq!(devices.devices().count(), 0); assert_eq!(devices.iter_pollable_dev().count(), 1); } @@ -629,10 +632,10 @@ fn test_factory_registry_rejects_duplicate_device_type() { let mut factories = DeviceFactoryRegistry::new(); assert_eq!(factories.register(Arc::new(MockMmioFactory)), Ok(())); - assert_eq!( + assert!(matches!( factories.register(Arc::new(MockMmioFactory)), - Err(AxError::AlreadyExists) - ); + Err(DeviceManagerError::ResourceConflict { .. }) + )); } #[test] @@ -647,19 +650,19 @@ fn test_missing_factory_returns_unsupported() { 0x1000, ); - assert_eq!( + assert!(matches!( factories.build(&config, &context).err(), - Some(AxError::Unsupported) - ); - assert_eq!( + Some(DeviceManagerError::Unsupported { .. }) + )); + assert!(matches!( AxVmDevices::build_with_factories( AxVmDeviceConfig::new(vec![config]), &factories, &context, ) .err(), - Some(AxError::Unsupported) - ); + Some(DeviceManagerError::Unsupported { .. }) + )); } #[test] @@ -706,10 +709,10 @@ fn test_factory_validation_failure_leaves_devices_unchanged() { 0, ); - assert_eq!( + assert!(matches!( devices.register_factory_device(&invalid, &factories, &context), - Err(AxError::InvalidInput) - ); + Err(DeviceManagerError::InvalidConfig { .. }) + )); assert_eq!(devices.devices().count(), count_before); } @@ -744,10 +747,12 @@ fn test_wrapped_native_mmio_resource_is_rejected() { bundle.push(DeviceRegistration::Device(MmioDeviceAdapter::from_arc( mmio_device("zero-size", 0x1000, 0x1000), ))); - assert_eq!( + assert!(matches!( devices.register_bundle(bundle).err(), - Some(AxError::InvalidInput) - ); + Some(DeviceManagerError::Registry( + RegistryError::InvalidResource { .. } + )) + )); assert_eq!(devices.devices().count(), 0); } diff --git a/virtualization/test_crates/virtualization-tests/tests/axvm_irq.rs b/virtualization/test_crates/virtualization-tests/tests/axvm_irq.rs index 8016347e5e..2965a5fd27 100644 --- a/virtualization/test_crates/virtualization-tests/tests/axvm_irq.rs +++ b/virtualization/test_crates/virtualization-tests/tests/axvm_irq.rs @@ -14,14 +14,15 @@ use std::sync::{Arc, Mutex, Weak}; -use ax_errno::{AxError, AxResult}; use ax_plat::{console::ConsoleIf, time::TimeIf}; use axdevice::{ AxVmDeviceConfig, AxVmDevices, DeviceBuildContext, DeviceBundle, DeviceFactory, - DeviceFactoryRegistry, DeviceRegistration, IrqResolver, MmioDeviceAdapter, + DeviceFactoryRegistry, DeviceManagerError, DeviceManagerResult, DeviceRegistration, + IrqResolver, MmioDeviceAdapter, }; use axdevice_base::{ - AccessWidth, BaseDeviceOps, InterruptTriggerMode, IrqLine, IrqLineId, IrqSink, + AccessWidth, BaseDeviceOps, DeviceResult, InterruptTriggerMode, IrqError, IrqLine, IrqLineId, + IrqResult, IrqSink, }; use axvm::{AxVmError, InterruptFabric}; use axvm_types::{ @@ -100,7 +101,7 @@ impl RecordingIrqSink { } impl IrqSink for RecordingIrqSink { - fn set_level(&self, line: IrqLineId, asserted: bool) -> AxResult { + fn set_level(&self, line: IrqLineId, asserted: bool) -> IrqResult { self.events .lock() .unwrap() @@ -108,7 +109,7 @@ impl IrqSink for RecordingIrqSink { Ok(()) } - fn pulse(&self, line: IrqLineId) -> AxResult { + fn pulse(&self, line: IrqLineId) -> IrqResult { self.events.lock().unwrap().push(IrqEvent::Pulse(line)); Ok(()) } @@ -128,12 +129,17 @@ impl BaseDeviceOps for IrqMmioDevice { EmulatedDeviceType::VirtioNet } - fn handle_read(&self, _addr: GuestPhysAddr, _width: AccessWidth) -> AxResult { + fn handle_read(&self, _addr: GuestPhysAddr, _width: AccessWidth) -> DeviceResult { Ok(0) } - fn handle_write(&self, _addr: GuestPhysAddr, _width: AccessWidth, _val: usize) -> AxResult { - self.line.pulse() + fn handle_write(&self, _addr: GuestPhysAddr, _width: AccessWidth, _val: usize) -> DeviceResult { + self.line + .pulse() + .map_err(|error| axdevice_base::DeviceError::Backend { + operation: "pulse test device IRQ", + detail: error.to_string(), + }) } } @@ -148,9 +154,12 @@ impl DeviceFactory for IrqMmioFactory { &self, config: &EmulatedDeviceConfig, context: &DeviceBuildContext<'_>, - ) -> AxResult { + ) -> DeviceManagerResult { let Some(end) = config.base_gpa.checked_add(config.length) else { - return Err(AxError::InvalidInput); + return Err(DeviceManagerError::InvalidConfig { + operation: "build IRQ MMIO test device", + detail: "device address range overflows".into(), + }); }; let line = context.resolve_irq(config.irq_id, InterruptTriggerMode::EdgeTriggered)?; Ok( @@ -196,15 +205,15 @@ fn test_no_irq_fabric_rejects_backend_and_line_resolution() { let fabric = InterruptFabric::new(VMInterruptMode::NoIrq); let context = DeviceBuildContext::new(&fabric); - assert_eq!( + assert!(matches!( AxVmDevices::build_with_factories( AxVmDeviceConfig::new(vec![irq_device_config(0x6_0000, 12)]), &irq_factory_registry(), &context, ) .err(), - Some(AxError::InvalidInput) - ); + Some(DeviceManagerError::Irq(IrqError::InvalidLine { .. })) + )); } #[test] @@ -218,8 +227,14 @@ fn test_interrupt_fabric_preserves_event_order() { .resolve_irq(14, InterruptTriggerMode::EdgeTriggered) .unwrap(); - assert_eq!(level.pulse(), Err(AxError::InvalidInput)); - assert_eq!(edge.raise(), Err(AxError::InvalidInput)); + assert!(matches!( + level.pulse(), + Err(IrqError::InvalidTriggerMode { .. }) + )); + assert!(matches!( + edge.raise(), + Err(IrqError::InvalidTriggerMode { .. }) + )); level.raise().unwrap(); level.lower().unwrap(); edge.pulse().unwrap();