Refactors vmm events (#1131)

gui/src/debug/client.rs

@@ -60,7 +60,7 @@ impl DebugClient {
 }
 
 impl gdbstub::conn::Connection for DebugClient {
-    type Error = Error;
+    type Error = std::io::Error;
 
     fn write(&mut self, byte: u8) -> Result<(), Self::Error> {
         self.write_all(std::slice::from_ref(&byte))
@@ -77,7 +77,7 @@ impl gdbstub::conn::Connection for DebugClient {
             };
 
             if written == 0 {
-                return Err(Error::from(ErrorKind::WriteZero));
+                return Err(std::io::Error::from(ErrorKind::WriteZero));
             }
 
             buf = &buf[written..];

gui/src/error/mod.rs

@@ -1,19 +1,19 @@
 // SPDX-License-Identifier: MIT OR Apache-2.0
 use std::error::Error;
-use std::ffi::CString;
+use std::ffi::{CStr, CString};
 use std::fmt::{Display, Write};
 
 #[cfg(feature = "qt")]
 mod ffi;
 
 /// Error object managed by Rust side.
-pub struct RustError(CString);
+pub struct RustError(Box<CStr>);
 
 impl RustError {
     /// # Panics
     /// If `msg` contains NUL character.
     pub fn new(msg: impl Into<Vec<u8>>) -> Self {
-        Self(CString::new(msg).unwrap())
+        Self(CString::new(msg).unwrap().into_boxed_c_str())
     }
 
     pub fn with_source(msg: impl Display, src: impl Error) -> Self {
@@ -25,7 +25,7 @@ impl RustError {
             src = e.source();
         }
 
-        Self(CString::new(msg).unwrap())
+        Self(CString::new(msg).unwrap().into_boxed_c_str())
     }
 
     pub fn wrap(src: impl Error) -> Self {
@@ -37,7 +37,7 @@ impl RustError {
             src = e.source();
         }
 
-        Self(CString::new(msg).unwrap())
+        Self(CString::new(msg).unwrap().into_boxed_c_str())
     }
 
     pub fn into_c(self) -> *mut Self {

gui/src/main.rs

@@ -50,7 +50,7 @@ fn run() -> Result<(), ApplicationError> {
     // TODO: check if already configured and skip wizard
     run_wizard().map_err(ApplicationError::RunWizard)?;
 
-    if let Some(debug_addr) = args.debug_addr() {
+    let vmm = if let Some(debug_addr) = args.debug_addr() {
         let kernel_path = get_kernel_path(&args)?;
 
         let debug_server = DebugServer::new(debug_addr)
@@ -68,7 +68,7 @@ fn run() -> Result<(), ApplicationError> {
         // TODO: handle events
         let event_handler = |event| match event {
             VmmEvent::Breakpoint { stop } => {}
-            VmmEvent::Log { ty, data, len } => {}
+            VmmEvent::Log { ty, msg } => {}
             VmmEvent::Exiting { success } => {}
             VmmEvent::Error { reason } => {}
         };
@@ -81,22 +81,21 @@ fn run() -> Result<(), ApplicationError> {
             event_handler,
         )
         .map_err(ApplicationError::RunVmm)?;
-    }
 
-    // Run VMM launcher.
-    let vmm = match run_launcher()? {
-        Some(v) => v,
-        None => return Ok(()),
+        todo!()
+    } else {
+        run_launcher()?
+    };
+
+    let Some(vmm) = vmm else {
+        return Ok(());
     };
 
     // Setup VMM screen.
-    let mut screen =
-        DefaultScreen::new().map_err(|e| ApplicationError::CreateScreen(Box::new(e)))?;
+    let mut screen = DefaultScreen::new().map_err(ApplicationError::CreateScreen)?;
 
     // TODO: Start VMM.
-    screen
-        .run()
-        .map_err(|e| ApplicationError::RunScreen(Box::new(e)))?;
+    screen.run().map_err(ApplicationError::RunScreen)?;
 
     Ok(())
 }
@@ -312,8 +311,8 @@ enum ApplicationError {
     RunMainWindow(#[source] slint::PlatformError),
 
     #[error("couldn't create VMM screen")]
-    CreateScreen(#[source] Box<dyn std::error::Error>),
+    CreateScreen(#[source] screen::ScreenError),
 
     #[error("couldn't run VMM screen")]
-    RunScreen(#[source] Box<dyn std::error::Error>),
+    RunScreen(#[source] <DefaultScreen as Screen>::RunErr),
 }

gui/src/vmm/cpu/mod.rs

@@ -19,7 +19,6 @@ use gdbstub::target::{TargetError, TargetResult};
 use std::collections::{BTreeMap, HashMap};
 use std::num::NonZero;
 use std::ops::Deref;
-use std::ptr::null_mut;
 use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::{Arc, Mutex};
 use thiserror::Error;
@@ -110,14 +109,14 @@ impl<H: Hypervisor, S: Screen> CpuManager<H, S> {
             Ok(v) => v,
             Err(e) => {
                 let e = RustError::with_source("couldn't create main CPU", e);
-                (args.event)(VmmEvent::Error { reason: &e });
+                (args.event)(VmmEvent::Error { reason: e });
                 return;
             }
         };
 
         if let Err(e) = super::arch::setup_main_cpu(&mut cpu, entry, map, args.hv.cpu_features()) {
             let e = RustError::with_source("couldn't setup main CPU", e);
-            (args.event)(VmmEvent::Error { reason: &e });
+            (args.event)(VmmEvent::Error { reason: e });
             return;
         }
 
@@ -199,7 +198,7 @@ impl<H: Hypervisor, S: Screen> CpuManager<H, S> {
         };
 
         if let Some(e) = e {
-            (args.event)(VmmEvent::Error { reason: &e });
+            (args.event)(VmmEvent::Error { reason: e });
         }
 
         // Shutdown other CPUs.
@@ -281,10 +280,7 @@ impl<H: Hypervisor, S: Screen> CpuManager<H, S> {
         stop: Option<MultiThreadStopReason<u64>>,
     ) -> bool {
         // Convert stop reason.
-        let stop = stop
-            .map(KernelStop)
-            .map(Box::new)
-            .map_or(null_mut(), Box::into_raw);
+        let stop = stop.map(KernelStop);
 
         // Notify GUI. We need to allow only one CPU to enter the debugger dispatch loop.
         let lock = args.breakpoint.lock().unwrap();
@@ -305,15 +301,15 @@ impl<H: Hypervisor, S: Screen> CpuManager<H, S> {
                         Ok(v) => v,
                         Err(e) => {
                             let e = RustError::with_source("couldn't get CPU states", e);
-                            (args.event)(VmmEvent::Error { reason: &e });
+                            (args.event)(VmmEvent::Error { reason: e });
                             return false;
                         }
                     };
 
                     match Self::get_debug_regs(&mut states) {
                         Ok(v) => debug.send(DebugRes::Regs(v)),
                         Err(e) => {
-                            (args.event)(VmmEvent::Error { reason: &e });
+                            (args.event)(VmmEvent::Error { reason: e });
                             return false;
                         }
                     }
@@ -326,7 +322,7 @@ impl<H: Hypervisor, S: Screen> CpuManager<H, S> {
                             e,
                         );
 
-                        (args.event)(VmmEvent::Error { reason: &err });
+                        (args.event)(VmmEvent::Error { reason: err });
                         return false;
                     }
                 },

gui/src/vmm/hw/console/context.rs

@@ -44,30 +44,26 @@ impl<'a, H: Hypervisor, C: Cpu> DeviceContext<C> for Context<'a, H> {
             let len = self.msg_len.take().ok_or(ExecError::InvalidSequence)?;
             let data = read_ptr(exit, len, self.hv).map_err(|e| ExecError::ReadFailed(off, e))?;
 
-            self.msg
-                .extend_from_slice(unsafe { std::slice::from_raw_parts(data.as_ptr(), len.get()) });
+            self.msg.extend_from_slice(unsafe {
+                std::slice::from_raw_parts(data.as_ptr(), data.len().get())
+            });
         } else if off == offset_of!(ConsoleMemory, commit) {
             // Check if state valid.
             if self.msg_len.is_some() || self.msg.is_empty() {
                 return Err(Box::new(ExecError::InvalidSequence));
-            } else if std::str::from_utf8(&self.msg).is_err() {
-                return Err(Box::new(ExecError::InvalidMsg));
             }
 
             // Parse data.
             let commit = read_u8(exit).map_err(|e| ExecError::ReadFailed(off, e))?;
             let ty: ConsoleType = commit
                 .try_into()
-                .map_err(|_| Box::new(ExecError::InvalidCommit(commit)))?;
+                .map_err(|_| ExecError::InvalidCommit(commit))?;
 
-            // Trigger event.
-            let msg = std::mem::take(&mut self.msg);
+            // Trigger event.s
+            let bytes = std::mem::take(&mut self.msg);
+            let msg = String::from_utf8(bytes).map_err(ExecError::InvalidMsg)?;
 
-            (self.dev.event)(VmmEvent::Log {
-                ty: ty.into(),
-                data: msg.as_ptr().cast(),
-                len: msg.len(),
-            });
+            (self.dev.event)(VmmEvent::Log { ty: ty.into(), msg });
         } else {
             return Err(Box::new(ExecError::UnknownField(off)));
         }
@@ -89,7 +85,7 @@ enum ExecError {
     InvalidLen,
 
     #[error("invalid message")]
-    InvalidMsg,
+    InvalidMsg(#[source] std::string::FromUtf8Error),
 
     #[error("{0:#x} is not a valid commit")]
     InvalidCommit(u8),

gui/src/vmm/hw/mod.rs

@@ -34,16 +34,10 @@ pub fn setup_devices(
 }
 
 fn read_u8(exit: &mut impl CpuIo) -> Result<u8, MmioError> {
-    // Get data.
-    let IoBuf::Write(data) = exit.buffer() else {
-        return Err(MmioError::InvalidOperation);
-    };
-
-    // Parse data.
-    if data.len() != 1 {
-        Err(MmioError::InvalidData)
-    } else {
-        Ok(data[0])
+    match exit.buffer() {
+        IoBuf::Write(&[v]) => Ok(v),
+        IoBuf::Write(_) => Err(MmioError::InvalidData),
+        _ => Err(MmioError::InvalidOperation),
     }
 }
 

gui/src/vmm/mod.rs

@@ -440,23 +440,11 @@ pub struct VmmScreen {
 pub type VmmEventHandler = Arc<dyn Fn(VmmEvent) + Send + Sync + 'static>;
 
 /// Contains VMM event information.
-#[repr(C)]
-#[allow(dead_code)] // TODO: Figure out why Rust think fields in each enum are not used.
 pub enum VmmEvent {
-    Error {
-        reason: *const RustError,
-    },
-    Exiting {
-        success: bool,
-    },
-    Log {
-        ty: VmmLog,
-        data: *const c_char,
-        len: usize,
-    },
-    Breakpoint {
-        stop: *mut KernelStop,
-    },
+    Error { reason: RustError },
+    Exiting { success: bool },
+    Log { ty: VmmLog, msg: String },
+    Breakpoint { stop: Option<KernelStop> },
 }
 
 /// Log category.
@@ -486,15 +474,6 @@ impl From<ConsoleType> for VmmLog {
 #[allow(dead_code)]
 pub struct KernelStop(MultiThreadStopReason<u64>);
 
-/// Result of [`vmm_dispatch_debug()`].
-#[allow(dead_code)]
-#[repr(C)]
-pub enum DebugResult {
-    Ok,
-    Disconnected,
-    Error { reason: *mut RustError },
-}
-
 #[derive(Debug, Error)]
 pub enum VmmError {
     #[error("couldn't open kernel path {1}")]