ioctls: Add new MshvError type derived from thiserror::Error

MshvError allows different kinds of errors to be returned from the library. Notably, details about a failed hypercall can be exposed to the VMM so useful error messages can be printed. Replace all uses of errno::Error in the API with MshvError. Signed-off-by: Nuno Das Neves <[email protected]>
rust-vmm · Apr 3, 2024 · 9179ab5 · 9179ab5
1 parent 1cf5b69
commit 9179ab5
Show file tree

Hide file tree

Showing 9 changed files with 130 additions and 58 deletions.
diff --git a/mshv-bindings/Cargo.toml b/mshv-bindings/Cargo.toml
@@ -14,8 +14,8 @@ libc = ">=0.2.39"
 serde = { version = ">=1.0.27", optional = true }
 serde_derive = { version = ">=1.0.27", optional = true }
 vmm-sys-util = ">=0.12.1"
-
 zerocopy = { version = "0.7", features = ["derive"] }
+num_enum = "0.7"
 
 [dev-dependencies]
 random-number = "0.1.6"

diff --git a/mshv-bindings/src/hvdef.rs b/mshv-bindings/src/hvdef.rs
@@ -3,6 +3,7 @@
 // SPDX-License-Identifier: Apache-2.0 OR BSD-3-Clause
 //
 #![allow(dead_code)]
+use num_enum::TryFromPrimitive;
 use zerocopy::{AsBytes, FromBytes, FromZeroes};
 
 pub const HV_CPUID_FUNCTION_VERSION_AND_FEATURES: u32 = 0x00000001;
@@ -113,7 +114,7 @@ pub const MSR_HYPERCALL_ADDR_MASK: u64 = !0xfff;
 pub const MSR_SIEFP_SIMP_ACTIVE: u64 = 1;
 pub const MSR_SIEFP_SIMP_ADDR_MASK: u64 = !0xfff;
 
-#[derive(Debug)]
+#[derive(TryFromPrimitive, Debug, Copy, Clone, PartialEq)]
 #[repr(u16)]
 pub enum HvError {
     InvalidHypercallCode = 0x0002,
@@ -172,8 +173,6 @@ pub enum HvError {
     EventBufferAlreadyFreed = 0x0074,
 }
 
-pub type HvResult<T> = Result<T, HvError>;
-
 #[repr(C)]
 #[derive(Copy, Clone, AsBytes, Debug, FromBytes, FromZeroes)]
 pub struct HvMessageHeader {

diff --git a/mshv-ioctls/Cargo.toml b/mshv-ioctls/Cargo.toml
@@ -9,3 +9,4 @@ license = "Apache-2.0 OR BSD-3-Clause"
 libc = ">=0.2.39"
 mshv-bindings = {path = "../mshv-bindings", features = ["fam-wrappers"]}
 vmm-sys-util = ">=0.12.1"
+thiserror = "1.0"
diff --git a/mshv-ioctls/src/ioctls/device.rs b/mshv-ioctls/src/ioctls/device.rs
@@ -4,10 +4,9 @@
 use std::fs::File;
 use std::os::unix::io::{AsRawFd, FromRawFd, RawFd};
 
-use crate::ioctls::Result;
+use crate::ioctls::{MshvError, Result};
 use crate::mshv_ioctls::{MSHV_GET_DEVICE_ATTR, MSHV_HAS_DEVICE_ATTR, MSHV_SET_DEVICE_ATTR};
 use mshv_bindings::mshv_device_attr;
-use vmm_sys_util::errno;
 use vmm_sys_util::ioctl::{ioctl_with_mut_ref, ioctl_with_ref};
 
 /// Wrapper over the file descriptor obtained when creating an emulated device in the kernel.
@@ -28,7 +27,7 @@ impl DeviceFd {
         // requirements.
         let ret = unsafe { ioctl_with_ref(self, MSHV_HAS_DEVICE_ATTR(), device_attr) };
         if ret != 0 {
-            return Err(errno::Error::last());
+            return Err(MshvError::from_last_errno());
         }
         Ok(())
     }
@@ -79,7 +78,7 @@ impl DeviceFd {
         // requirements.
         let ret = unsafe { ioctl_with_ref(self, MSHV_SET_DEVICE_ATTR(), device_attr) };
         if ret != 0 {
-            return Err(errno::Error::last());
+            return Err(MshvError::from_last_errno());
         }
         Ok(())
     }
@@ -108,7 +107,7 @@ impl DeviceFd {
         // requirements.
         let ret = unsafe { ioctl_with_mut_ref(self, MSHV_GET_DEVICE_ATTR(), device_attr) };
         if ret != 0 {
-            return Err(errno::Error::last());
+            return Err(MshvError::from_last_errno());
         }
         Ok(())
     }
@@ -183,6 +182,6 @@ mod tests {
         assert!(device.has_device_attr(&dist_attr).is_ok());
         assert!(device.get_device_attr(&mut dist_attr_mut).is_err());
         assert!(device.set_device_attr(&dist_attr).is_err());
-        assert_eq!(errno::Error::last().errno(), 14);
+        assert_eq!(MshvError::from_last_errno().errno(), 14);
     }
 }
diff --git a/mshv-ioctls/src/ioctls/mod.rs b/mshv-ioctls/src/ioctls/mod.rs
@@ -2,13 +2,89 @@
 //
 // SPDX-License-Identifier: Apache-2.0 OR BSD-3-Clause
 //
+use mshv_bindings::{mshv_root_hvcall, HvError};
+use thiserror::Error;
 use vmm_sys_util::errno;
 pub mod device;
 pub mod system;
 pub mod vcpu;
 pub mod vm;
+
+/// A specialized `Error` type for MSHV ioctls
+///
+/// Exposes either a regular linux errno, or details of a hypercall-related
+/// error.
+///
+/// For convenience, it can always be converted into an errno::Error
+#[derive(Error, Debug, Copy, Clone, PartialEq)]
+pub enum MshvError {
+    /// A regular linux errno
+    #[error("Kernel returned errno: {0}")]
+    Errno(#[from] errno::Error),
+
+    /// A failed hypercall
+    /// In case the caller requires an errno, this variant can be converted to
+    /// an EIO with into(), from(), or errno() for the raw value
+    #[error("Hypercall {code} failed with status {status:?}")]
+    Hypercall {
+        /// The control code, i.e. what type of hypercall it was
+        code: u16,
+        /// The status or result code returned from the hypercall
+        status: HvError,
+    },
+}
+
+impl MshvError {
+    /// Convert to errno
+    pub fn errno(&self) -> i32 {
+        errno::Error::from(*self).errno()
+    }
+    /// Get the last errno as an MshvError
+    pub fn from_last_errno() -> Self {
+        MshvError::Errno(errno::Error::last())
+    }
+}
+
+impl From<mshv_root_hvcall> for MshvError {
+    fn from(ret_args: mshv_root_hvcall) -> Self {
+        use std::convert::TryFrom;
+        let err_code = errno::Error::last().errno();
+        // EIO signals that the hypercall itself may have failed
+        if err_code == libc::EIO && ret_args.status != 0 {
+            if let Ok(hv_err) = HvError::try_from(ret_args.status) {
+                return MshvError::Hypercall {
+                    code: ret_args.code,
+                    status: hv_err,
+                };
+            }
+        }
+        err_code.into()
+    }
+}
+
+impl From<i32> for MshvError {
+    fn from(err: i32) -> Self {
+        MshvError::Errno(errno::Error::new(err))
+    }
+}
+
+impl From<MshvError> for errno::Error {
+    fn from(err: MshvError) -> Self {
+        match err {
+            MshvError::Errno(e) => e,
+            MshvError::Hypercall { .. } => errno::Error::new(libc::EIO),
+        }
+    }
+}
+
+impl From<MshvError> for std::io::Error {
+    fn from(err: MshvError) -> Self {
+        errno::Error::from(err).into()
+    }
+}
+
 /// A specialized `Result` type for MSHV ioctls.
 ///
 /// This typedef is generally used to avoid writing out errno::Error directly and
 /// is otherwise a direct mapping to Result.
-pub type Result<T> = std::result::Result<T, errno::Error>;
+pub type Result<T> = std::result::Result<T, MshvError>;
diff --git a/mshv-ioctls/src/ioctls/system.rs b/mshv-ioctls/src/ioctls/system.rs
@@ -3,14 +3,13 @@
 // SPDX-License-Identifier: Apache-2.0 OR BSD-3-Clause
 //
 use crate::ioctls::vm::{new_vmfd, VmFd, VmType};
-use crate::ioctls::Result;
+use crate::ioctls::{MshvError, Result};
 use crate::mshv_ioctls::*;
 use libc::{open, O_CLOEXEC, O_NONBLOCK};
 use mshv_bindings::*;
 use std::fs::File;
 use std::os::raw::c_char;
 use std::os::unix::io::{FromRawFd, RawFd};
-use vmm_sys_util::errno;
 use vmm_sys_util::ioctl::ioctl_with_ref;
 
 /// Wrapper over MSHV system ioctls.
@@ -252,7 +251,7 @@ impl Mshv {
         // SAFETY: we give a constant null-terminated string and verify the result.
         let ret = unsafe { open("/dev/mshv\0".as_ptr() as *const c_char, open_flags) };
         if ret < 0 {
-            Err(errno::Error::last())
+            Err(MshvError::from_last_errno())
         } else {
             Ok(ret)
         }
@@ -267,7 +266,7 @@ impl Mshv {
             let vm_file = unsafe { File::from_raw_fd(ret) };
             Ok(new_vmfd(vm_file))
         } else {
-            Err(errno::Error::last())
+            Err(MshvError::from_last_errno())
         }
     }
 

diff --git a/mshv-ioctls/src/ioctls/vcpu.rs b/mshv-ioctls/src/ioctls/vcpu.rs
@@ -2,15 +2,14 @@
 //
 // SPDX-License-Identifier: Apache-2.0 OR BSD-3-Clause
 //
-use crate::ioctls::Result;
+use crate::ioctls::{MshvError, Result};
 use crate::mshv_ioctls::*;
 use mshv_bindings::*;
 use std::convert::TryFrom;
 use std::fs::File;
 use std::os::unix::io::{AsRawFd, RawFd};
 #[cfg(test)]
 use std::slice;
-use vmm_sys_util::errno;
 use vmm_sys_util::ioctl::{ioctl_with_mut_ref, ioctl_with_ref};
 
 // Macro for setting up multiple 64 bit registers together
@@ -76,7 +75,7 @@ impl VcpuFd {
             ioctl_with_mut_ref(self, MSHV_GET_VP_REGISTERS(), &mut mshv_vp_register_args)
         };
         if ret != 0 {
-            return Err(errno::Error::last());
+            return Err(MshvError::from_last_errno());
         }
         Ok(())
     }
@@ -95,7 +94,7 @@ impl VcpuFd {
         // SAFETY: IOCTL call with correct types.
         let ret = unsafe { ioctl_with_ref(self, MSHV_SET_VP_REGISTERS(), &hv_vp_register_args) };
         if ret != 0 {
-            return Err(errno::Error::last());
+            return Err(MshvError::from_last_errno());
         }
         Ok(())
     }
@@ -393,7 +392,7 @@ impl VcpuFd {
         // we can't do this without the vm fd which isn't available here
         for bits in &sregs.interrupt_bitmap {
             if *bits != 0 {
-                return Err(errno::Error::new(libc::EINVAL));
+                return Err(libc::EINVAL.into());
             }
         }
 
@@ -639,7 +638,7 @@ impl VcpuFd {
         for i in 0..nmsrs {
             let name = match msr_to_hv_reg_name(msrs.as_slice()[i].index) {
                 Ok(n) => n,
-                Err(_) => return Err(errno::Error::new(libc::EINVAL)),
+                Err(_) => return Err(libc::EINVAL.into()),
             };
             reg_assocs.push(hv_register_assoc {
                 name,
@@ -669,7 +668,7 @@ impl VcpuFd {
         for i in 0..nmsrs {
             let name = match msr_to_hv_reg_name(msrs.as_slice()[i].index) {
                 Ok(n) => n,
-                Err(_) => return Err(errno::Error::new(libc::EINVAL)),
+                Err(_) => return Err(libc::EINVAL.into()),
             };
             reg_assocs.push(hv_register_assoc {
                 name,
@@ -688,7 +687,7 @@ impl VcpuFd {
         // SAFETY: we know that our file is a vCPU fd and we verify the return result.
         let ret = unsafe { ioctl_with_mut_ref(self, MSHV_RUN_VP(), &mut hv_message_input) };
         if ret != 0 {
-            return Err(errno::Error::last());
+            return Err(MshvError::from_last_errno());
         }
         Ok(hv_message_input)
     }
@@ -829,7 +828,7 @@ impl VcpuFd {
         // SAFETY: we know that our file is a vCPU fd and we verify the return result.
         let ret = unsafe { ioctl_with_mut_ref(self, MSHV_GET_VP_STATE(), state) };
         if ret != 0 {
-            return Err(errno::Error::last());
+            return Err(MshvError::from_last_errno());
         }
         Ok(())
     }
@@ -839,7 +838,7 @@ impl VcpuFd {
         // SAFETY: IOCTL call with correct types
         let ret = unsafe { ioctl_with_ref(self, MSHV_SET_VP_STATE(), state) };
         if ret != 0 {
-            return Err(errno::Error::last());
+            return Err(MshvError::from_last_errno());
         }
         Ok(())
     }
@@ -853,7 +852,7 @@ impl VcpuFd {
             ..Default::default()
         };
         self.get_vp_state_ioctl(&mut vp_state)?;
-        LapicState::try_from(buffer)
+        Ok(LapicState::try_from(buffer)?)
     }
     /// Sets the state of the LAPIC (Local Advanced Programmable Interrupt Controller).
     pub fn set_lapic(&self, lapic_state: &LapicState) -> Result<()> {
@@ -876,7 +875,7 @@ impl VcpuFd {
             ..Default::default()
         };
         self.get_vp_state_ioctl(&mut vp_state)?;
-        XSave::try_from(buffer)
+        Ok(XSave::try_from(buffer)?)
     }
     /// Set the xsave data
     pub fn set_xsave(&self, data: &XSave) -> Result<()> {
@@ -903,7 +902,7 @@ impl VcpuFd {
         // SAFETY: we know that our file is a vCPU fd, we know the kernel honours its ABI.
         let ret = unsafe { ioctl_with_mut_ref(self, MSHV_VP_TRANSLATE_GVA(), &mut args) };
         if ret != 0 {
-            return Err(errno::Error::last());
+            return Err(MshvError::from_last_errno());
         }
 
         Ok((gpa, result))
@@ -986,7 +985,7 @@ impl VcpuFd {
         };
         let ret = unsafe { ioctl_with_ref(self, MSHV_VP_REGISTER_INTERCEPT_RESULT(), &args) };
         if ret != 0 {
-            return Err(errno::Error::last());
+            return Err(MshvError::from_last_errno());
         }
 
         Ok(())
@@ -1040,7 +1039,7 @@ impl VcpuFd {
         // correct amount of memory from our pointer, and we verify the return result.
         let ret = unsafe { ioctl_with_mut_ref(self, MSHV_GET_VP_CPUID_VALUES(), &mut parms) };
         if ret != 0 {
-            return Err(errno::Error::last());
+            return Err(MshvError::from_last_errno());
         }
         Ok([parms.eax, parms.ebx, parms.ecx, parms.edx])
     }
@@ -1049,7 +1048,7 @@ impl VcpuFd {
         // SAFETY: we know that our file is a vCPU fd, we know the kernel honours its ABI.
         let ret = unsafe { ioctl_with_mut_ref(self, MSHV_READ_GPA(), input) };
         if ret != 0 {
-            return Err(errno::Error::last());
+            return Err(MshvError::from_last_errno());
         }
 
         Ok(*input)
@@ -1059,7 +1058,7 @@ impl VcpuFd {
         // SAFETY: we know that our file is a vCPU fd, we know the kernel honours its ABI.
         let ret = unsafe { ioctl_with_mut_ref(self, MSHV_WRITE_GPA(), input) };
         if ret != 0 {
-            return Err(errno::Error::last());
+            return Err(MshvError::from_last_errno());
         }
 
         Ok(*input)