Error type expansion

Signed-off-by: Jonathan Woollett-Light <[email protected]>

Author: Jonathan Woollett-Light

Cargo.lock

@@ -11,6 +11,7 @@ serde = { version = ">=1.0.27", features = ["derive"] }
 serde_derive = ">=1.0.27"
 serde_json = ">=1.0.9"
 derive_more = { version = "0.99.17", default-features = false, features = ["from"] }
+thiserror = "1.0.32"
 
 logger = { path = "../logger" }
 micro_http = { git = "https://github.com/firecracker-microvm/micro-http", rev = "0a58eb1" }

src/api_server/Cargo.toml

@@ -34,22 +34,16 @@ pub type ApiRequest = Box<VmmAction>;
 pub type ApiResponse = Box<std::result::Result<VmmData, VmmActionError>>;
 
 /// Errors thrown when binding the API server to the socket path.
+#[derive(thiserror::Error)]
 pub enum Error {
     /// IO related error.
+    #[error("IO error: {0}")]
     Io(io::Error),
     /// EventFD related error.
+    #[error("EventFd error: {0}")]
     Eventfd(io::Error),
 }
 
-impl fmt::Display for Error {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        match *self {
-            Error::Io(ref err) => write!(f, "IO error: {}", err),
-            Error::Eventfd(ref err) => write!(f, "EventFd error: {}", err),
-        }
-    }
-}
-
 impl fmt::Debug for Error {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match *self {

src/api_server/src/lib.rs

@@ -14,6 +14,7 @@ versionize = ">=0.1.6"
 versionize_derive = ">=0.1.3"
 vm-fdt = "0.1.0"
 derive_more = { version = "0.99.17", default-features = false, features = ["from"] }
+thiserror = "1.0.32"
 bitflags = ">=1.0.4"
 
 arch_gen = { path = "../arch_gen" }

src/arch/Cargo.toml

@@ -16,15 +16,21 @@ pub use regs::GicState;
 use super::layout;
 
 /// Errors thrown while setting up the GIC.
-#[derive(Debug)]
+#[derive(Debug, thiserror::Error)]
 pub enum Error {
     /// Error while calling KVM ioctl for setting up the global interrupt controller.
+    #[error("Error while calling KVM ioctl for setting up the global interrupt controller: {0}")]
     CreateGIC(kvm_ioctls::Error),
     /// Error while setting or getting device attributes for the GIC.
+    #[error("Error while setting or getting device attributes for the GIC: {0}, {1}, {2}")]
     DeviceAttribute(kvm_ioctls::Error, bool, u32),
     /// The number of vCPUs in the GicState doesn't match the number of vCPUs on the system
+    #[error(
+        "The number of vCPUs in the GicState doesn't match the number of vCPUs on the system."
+    )]
     InconsistentVcpuCount,
     /// The VgicSysRegsState is invalid
+    #[error("The VgicSysRegsState is invalid.")]
     InvalidVgicSysRegState,
 }
 type Result<T> = result::Result<T, Error>;

src/arch/src/aarch64/gic/mod.rs

@@ -9,11 +9,13 @@ use kvm_bindings::kvm_lapic_state;
 use kvm_ioctls::VcpuFd;
 use utils::byte_order;
 /// Errors thrown while configuring the LAPIC.
-#[derive(Debug)]
+#[derive(Debug, thiserror::Error, PartialEq)]
 pub enum Error {
-    /// Failure in retrieving the LAPIC configuration.
+    /// Failure in getting the LAPIC configuration.
+    #[error("Failure in getting the LAPIC configuration: {0}")]
     GetLapic(kvm_ioctls::Error),
-    /// Failure in modifying the LAPIC configuration.
+    /// Failure in setting the LAPIC configuration.
+    #[error("Failure in setting the LAPIC configuration: {0}")]
     SetLapic(kvm_ioctls::Error),
 }
 type Result<T> = std::result::Result<T, Error>;

src/arch/src/x86_64/interrupts.rs

@@ -282,20 +282,44 @@ pub fn create_boot_msr_entries() -> Vec<kvm_msr_entry> {
     ]
 }
 
+/// Error type for [`set_msrs`].
+#[derive(Debug, thiserror::Error)]
+pub enum SetMSRsError {
+    /// Failed to create [`vmm_sys_util::fam::FamStructWrapper`] for MSRs.
+    #[error("Could not create `vmm_sys_util::fam::FamStructWrapper` for MSRs")]
+    Create(utils::fam::Error),
+    /// Settings MSRs resulted in an error.
+    #[error("Setting MSRs resulted in an error: {0}")]
+    Set(#[from] kvm_ioctls::Error),
+    /// Not all given MSRs were set.
+    #[error("Not all given MSRs were set.")]
+    Incomplete,
+}
+
 /// Configure Model Specific Registers (MSRs) required to boot Linux for a given x86_64 vCPU.
 ///
 /// # Arguments
 ///
 /// * `vcpu` - Structure for the VCPU that holds the VCPU's fd.
-pub fn set_msrs(vcpu: &VcpuFd, msr_entries: &[kvm_msr_entry]) -> Result<()> {
-    let msrs = Msrs::from_entries(&msr_entries).map_err(Error::FamError)?;
+///
+/// # Errors
+///
+/// When:
+/// - Failed to create [`vmm_sys_util::fam::FamStructWrapper`] for MSRs.
+/// - [`kvm_ioctls::ioctls::vcpu::VcpuFd::set_msrs`] errors.
+/// - [`kvm_ioctls::ioctls::vcpu::VcpuFd::set_msrs`] fails to write all given MSRs entries.
+pub fn set_msrs(
+    vcpu: &VcpuFd,
+    msr_entries: &[kvm_msr_entry],
+) -> std::result::Result<(), SetMSRsError> {
+    let msrs = Msrs::from_entries(&msr_entries).map_err(SetMSRsError::Create)?;
     vcpu.set_msrs(&msrs)
-        .map_err(Error::SetModelSpecificRegisters)
+        .map_err(SetMSRsError::Set)
         .and_then(|msrs_written| {
-            if msrs_written as u32 != msrs.as_fam_struct_ref().nmsrs {
-                Err(Error::SetModelSpecificRegistersCount)
-            } else {
+            if msrs_written as u32 == msrs.as_fam_struct_ref().nmsrs {
                 Ok(())
+            } else {
+                Err(SetMSRsError::Incomplete)
             }
         })
 }

src/arch/src/x86_64/msr.rs

@@ -5,7 +5,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the THIRD-PARTY file.
 
-use std::mem;
+use std::{fmt, mem};
 
 use kvm_bindings::{kvm_fpu, kvm_regs, kvm_sregs};
 use kvm_ioctls::VcpuFd;
@@ -19,42 +19,76 @@ const PDPTE_START: u64 = 0xa000;
 const PDE_START: u64 = 0xb000;
 
 /// Errors thrown while setting up x86_64 registers.
-#[derive(Debug)]
+#[derive(Debug, thiserror::Error, PartialEq)]
 pub enum Error {
     /// Failed to get SREGs for this CPU.
+    #[error("Failed to get SREGs for this CPU: {0}")]
     GetStatusRegisters(kvm_ioctls::Error),
     /// Failed to set base registers for this CPU.
+    #[error("Failed to set base registers for this CPU: {0}")]
     SetBaseRegisters(kvm_ioctls::Error),
     /// Failed to configure the FPU.
+    #[error("Failed to configure the FPU: {0}")]
     SetFPURegisters(kvm_ioctls::Error),
     /// Failed to set SREGs for this CPU.
+    #[error("Failed to set SREGs for this CPU: {0}")]
     SetStatusRegisters(kvm_ioctls::Error),
     /// Writing the GDT to RAM failed.
+    #[error("Writing the GDT to RAM failed.")]
     WriteGDT,
     /// Writing the IDT to RAM failed.
+    #[error("Writing the IDT to RAM failed")]
     WriteIDT,
     /// Writing PDPTE to RAM failed.
+    #[error("WritePDPTEAddress")]
     WritePDPTEAddress,
     /// Writing PDE to RAM failed.
+    #[error("WritePDEAddress")]
     WritePDEAddress,
     /// Writing PML4 to RAM failed.
+    #[error("WritePML4Address")]
     WritePML4Address,
 }
 type Result<T> = std::result::Result<T, Error>;
 
+/// Error type for [`setup_fpu`].
+#[derive(Debug, derive_more::From, PartialEq)]
+pub struct SetupFpuError(utils::errno::Error);
+impl std::error::Error for SetupFpuError {}
+impl fmt::Display for SetupFpuError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "Failed to setup FPU: {}", self.0)
+    }
+}
+
 /// Configure Floating-Point Unit (FPU) registers for a given CPU.
 ///
 /// # Arguments
 ///
 /// * `vcpu` - Structure for the VCPU that holds the VCPU's fd.
-pub fn setup_fpu(vcpu: &VcpuFd) -> Result<()> {
+///
+/// # Errors
+///
+/// When [`kvm_ioctls::ioctls::vcpu::VcpuFd::set_fpu`] errors.
+pub fn setup_fpu(vcpu: &VcpuFd) -> std::result::Result<(), SetupFpuError> {
     let fpu: kvm_fpu = kvm_fpu {
         fcw: 0x37f,
         mxcsr: 0x1f80,
         ..Default::default()
     };
 
-    vcpu.set_fpu(&fpu).map_err(Error::SetFPURegisters)
+    let res = vcpu.set_fpu(&fpu)?;
+    Ok(res)
+}
+
+/// Error type of [`setup_regs`].
+#[derive(Debug, derive_more::From, PartialEq)]
+pub struct SetupRegistersError(utils::errno::Error);
+impl std::error::Error for SetupRegistersError {}
+impl fmt::Display for SetupRegistersError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "Failed to setup registers:{}", self.0)
+    }
 }
 
 /// Configure base registers for a given CPU.
@@ -63,7 +97,11 @@ pub fn setup_fpu(vcpu: &VcpuFd) -> Result<()> {
 ///
 /// * `vcpu` - Structure for the VCPU that holds the VCPU's fd.
 /// * `boot_ip` - Starting instruction pointer.
-pub fn setup_regs(vcpu: &VcpuFd, boot_ip: u64) -> Result<()> {
+///
+/// # Errors
+///
+/// When [`kvm_ioctls::ioctls::vcpu::VcpuFd::set_regs`] errors.
+pub fn setup_regs(vcpu: &VcpuFd, boot_ip: u64) -> std::result::Result<(), SetupRegistersError> {
     let regs: kvm_regs = kvm_regs {
         rflags: 0x0000_0000_0000_0002u64,
         rip: boot_ip,
@@ -79,22 +117,54 @@ pub fn setup_regs(vcpu: &VcpuFd, boot_ip: u64) -> Result<()> {
         ..Default::default()
     };
 
-    vcpu.set_regs(&regs).map_err(Error::SetBaseRegisters)
+    vcpu.set_regs(&regs).map_err(SetupRegistersError)
+}
+
+/// Error type for [`setup_sregs`].
+#[derive(Debug, thiserror::Error, PartialEq)]
+pub enum SetupSpecialRegistersError {
+    /// Failed to get special registers
+    #[error("Failed to get special registers: {0}")]
+    GetSpecialRegisters(utils::errno::Error),
+    /// Failed to configure segments and special registers
+    #[error("Failed to configure segments and special registers: {0}")]
+    ConfigureSegmentsAndSpecialRegisters(Error),
+    /// Failed to setup page tables
+    #[error("Failed to setup page tables: {0}")]
+    SetupPageTables(Error),
+    /// Failed to set special registers
+    #[error("Failed to set special registers: {0}")]
+    SetSpecialRegisters(utils::errno::Error),
 }
 
-/// Configures the segment registers and system page tables for a given CPU.
+/// Configures the special registers and system page tables for a given CPU.
 ///
 /// # Arguments
 ///
 /// * `mem` - The memory that will be passed to the guest.
 /// * `vcpu` - Structure for the VCPU that holds the VCPU's fd.
-pub fn setup_sregs(mem: &GuestMemoryMmap, vcpu: &VcpuFd) -> Result<()> {
-    let mut sregs: kvm_sregs = vcpu.get_sregs().map_err(Error::GetStatusRegisters)?;
-
-    configure_segments_and_sregs(mem, &mut sregs)?;
-    setup_page_tables(mem, &mut sregs)?; // TODO(dgreid) - Can this be done once per system instead?
-
-    vcpu.set_sregs(&sregs).map_err(Error::SetStatusRegisters)
+///
+/// # Errors
+///
+/// When:
+/// - [`kvm_ioctls::ioctls::vcpu::VcpuFd::get_sregs`] errors.
+/// - [`configure_segments_and_sregs`] errors.
+/// - [`setup_page_tables`] errors
+/// - [`kvm_ioctls::ioctls::vcpu::VcpuFd::set_sregs`] errors.
+pub fn setup_sregs(
+    mem: &GuestMemoryMmap,
+    vcpu: &VcpuFd,
+) -> std::result::Result<(), SetupSpecialRegistersError> {
+    let mut sregs: kvm_sregs = vcpu
+        .get_sregs()
+        .map_err(SetupSpecialRegistersError::GetSpecialRegisters)?;
+
+    configure_segments_and_sregs(mem, &mut sregs)
+        .map_err(SetupSpecialRegistersError::ConfigureSegmentsAndSpecialRegisters)?;
+    setup_page_tables(mem, &mut sregs).map_err(SetupSpecialRegistersError::SetupPageTables)?; // TODO(dgreid) - Can this be done once per system instead?
+
+    vcpu.set_sregs(&sregs)
+        .map_err(SetupSpecialRegistersError::SetSpecialRegisters)
 }
 
 const BOOT_GDT_OFFSET: u64 = 0x500;

src/arch/src/x86_64/regs.rs

@@ -9,6 +9,7 @@ license = "Apache-2.0"
 kvm-bindings = { version = ">=0.5.0", features = ["fam-wrappers"] }
 kvm-ioctls = ">=0.9.0"
 derive_more = { version = "0.99.17", default-features = false, features = ["from"] }
+thiserror = "1.0.32"
 
 utils = { path = "../utils"}
 arch = { path = "../arch" }

src/cpuid/Cargo.toml

@@ -18,11 +18,13 @@ pub const VENDOR_ID_INTEL: &[u8; 12] = b"GenuineIntel";
 pub const VENDOR_ID_AMD: &[u8; 12] = b"AuthenticAMD";
 
 /// cpuid related error.
-#[derive(Clone, Debug)]
+#[derive(Clone, Debug, thiserror::Error, PartialEq, Eq)]
 pub enum Error {
     /// The function was called with invalid parameters.
+    #[error("The function was called with invalid parameters.")]
     InvalidParameters(String),
     /// Function not supported on the current architecture.
+    #[error("Function not supported on the current architecture.")]
     NotSupported,
 }