Merges obvirt into obconf (#1005)

.vscode/settings.json

@@ -18,6 +18,7 @@
   "lldb.launch.initCommands": [
     "settings set target.x86-disassembly-flavor intel"
   ],
+  "rust-analyzer.cargo.features": "all",
   "rust-analyzer.imports.granularity.group": "module",
   "rust-analyzer.imports.group.enable": false
 }

Cargo.toml

@@ -9,7 +9,6 @@ members = [
     "src/macros",
     "src/obconf",
     "src/obkrnl",
-    "src/obvirt",
     "src/param",
     "src/pfs",
     "src/pkg",

src/core.h

@@ -35,8 +35,8 @@ enum DisplayResolution {
  * Log category.
  *
  * The reason we need this because cbindgen is not good at exporting dependency types so we can't
- * use [`MsgType`] directly. See https://github.com/mozilla/cbindgen/issues/667 for an example of
- * the problem.
+ * use [`ConsoleType`] directly. See https://github.com/mozilla/cbindgen/issues/667 for an example
+ * of the problem.
  */
 enum VmmLog {
     VmmLog_Info,

src/core/Cargo.toml

@@ -10,9 +10,8 @@ crate-type = ["staticlib"]
 bitfield-struct = "0.8.0"
 humansize = "2.1.3"
 libc = "0.2.155"
-obconf = { path = "../obconf", features = ["serde"] }
+obconf = { path = "../obconf", features = ["serde", "virt"] }
 obfw = { git = "https://github.com/obhq/firmware-dumper.git", features = ["read", "std"] }
-obvirt = { path = "../obvirt" }
 param = { path = "../param" }
 pkg = { path = "../pkg" }
 ciborium = "0.2.2"

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

@@ -3,7 +3,7 @@ use super::Console;
 use crate::vmm::hv::{CpuIo, Hypervisor, IoBuf};
 use crate::vmm::hw::DeviceContext;
 use crate::vmm::VmmEvent;
-use obvirt::console::{Memory, MsgType};
+use obconf::{ConsoleMemory, ConsoleType};
 use std::error::Error;
 use std::mem::offset_of;
 use thiserror::Error;
@@ -13,7 +13,7 @@ pub struct Context<'a, H> {
     dev: &'a Console,
     hv: &'a H,
     msg_len: usize,
-    msg: String,
+    msg: Vec<u8>,
 }
 
 impl<'a, H: Hypervisor> Context<'a, H> {
@@ -22,7 +22,7 @@ impl<'a, H: Hypervisor> Context<'a, H> {
             dev,
             hv,
             msg_len: 0,
-            msg: String::new(),
+            msg: Vec::new(),
         }
     }
 
@@ -54,12 +54,12 @@ impl<'a, H: Hypervisor> Context<'a, H> {
             .map_err(|_| ExecError::InvalidData(off))
     }
 
-    fn read_str<'b>(
+    fn read_bin<'b>(
         &self,
         off: usize,
         exit: &'b mut dyn CpuIo,
         len: usize,
-    ) -> Result<&'b str, ExecError> {
+    ) -> Result<&'b [u8], ExecError> {
         // Get data.
         let buf = match exit.buffer() {
             IoBuf::Write(v) => v,
@@ -77,9 +77,8 @@ impl<'a, H: Hypervisor> Context<'a, H> {
 
         // Read data.
         let data = unsafe { self.hv.ram().host_addr().add(paddr) };
-        let data = unsafe { std::slice::from_raw_parts(data, len) };
 
-        Ok(std::str::from_utf8(data).unwrap())
+        Ok(unsafe { std::slice::from_raw_parts(data, len) })
     }
 }
 
@@ -88,17 +87,17 @@ impl<'a, H: Hypervisor> DeviceContext for Context<'a, H> {
         // Check field.
         let off = exit.addr() - self.dev.addr;
 
-        if off == offset_of!(Memory, msg_len) {
+        if off == offset_of!(ConsoleMemory, msg_len) {
             self.msg_len = self.read_usize(off, exit)?;
-        } else if off == offset_of!(Memory, msg_addr) {
-            self.msg.push_str(self.read_str(off, exit, self.msg_len)?);
-        } else if off == offset_of!(Memory, commit) {
+        } else if off == offset_of!(ConsoleMemory, msg_addr) {
+            self.msg
+                .extend_from_slice(self.read_bin(off, exit, self.msg_len)?);
+        } else if off == offset_of!(ConsoleMemory, commit) {
             // Parse data.
             let commit = self.read_u8(off, exit)?;
-            let ty = match MsgType::from_u8(commit) {
-                Some(v) => v,
-                None => return Err(Box::new(ExecError::InvalidCommit(commit))),
-            };
+            let ty: ConsoleType = commit
+                .try_into()
+                .map_err(|_| Box::new(ExecError::InvalidCommit(commit)))?;
 
             // Trigger event.
             let msg = std::mem::take(&mut self.msg);

src/core/src/vmm/hw/console/mod.rs

@@ -3,7 +3,7 @@ use self::context::Context;
 use super::{Device, DeviceContext};
 use crate::vmm::hv::Hypervisor;
 use crate::vmm::VmmEventHandler;
-use obvirt::console::Memory;
+use obconf::ConsoleMemory;
 use std::num::NonZero;
 
 mod context;
@@ -17,7 +17,7 @@ pub struct Console {
 
 impl Console {
     pub fn new(addr: usize, block_size: NonZero<usize>, event: VmmEventHandler) -> Self {
-        let len = size_of::<Memory>()
+        let len = size_of::<ConsoleMemory>()
             .checked_next_multiple_of(block_size.get())
             .and_then(NonZero::new)
             .unwrap();

src/core/src/vmm/mod.rs

@@ -8,8 +8,7 @@ use self::ram::{Ram, RamMap};
 use self::screen::Screen;
 use crate::error::RustError;
 use crate::profile::Profile;
-use obconf::{BootEnv, Vm};
-use obvirt::console::MsgType;
+use obconf::{BootEnv, ConsoleType, Vm};
 use std::cmp::max;
 use std::collections::BTreeMap;
 use std::error::Error;
@@ -646,8 +645,8 @@ pub enum VmmEvent {
 /// Log category.
 ///
 /// The reason we need this because cbindgen is not good at exporting dependency types so we can't
-/// use [`MsgType`] directly. See https://github.com/mozilla/cbindgen/issues/667 for an example of
-/// the problem.
+/// use [`ConsoleType`] directly. See https://github.com/mozilla/cbindgen/issues/667 for an example
+/// of the problem.
 #[repr(C)]
 #[derive(Clone, Copy)]
 pub enum VmmLog {
@@ -656,12 +655,12 @@ pub enum VmmLog {
     Error,
 }
 
-impl From<MsgType> for VmmLog {
-    fn from(value: MsgType) -> Self {
+impl From<ConsoleType> for VmmLog {
+    fn from(value: ConsoleType) -> Self {
         match value {
-            MsgType::Info => Self::Info,
-            MsgType::Warn => Self::Warn,
-            MsgType::Error => Self::Error,
+            ConsoleType::Info => Self::Info,
+            ConsoleType::Warn => Self::Warn,
+            ConsoleType::Error => Self::Error,
         }
     }
 }

src/obconf/Cargo.toml

@@ -3,5 +3,9 @@ name = "obconf"
 version = "0.1.0"
 edition = "2021"
 
+[features]
+virt = ["dep:num_enum"]
+
 [dependencies]
+num_enum = { version = "0.7.3", default-features = false, optional = true }
 serde = { version = "1.0.210", features = ["derive"], default-features = false, optional = true }

src/obconf/src/env/vm.rs

@@ -3,8 +3,38 @@ use core::num::NonZero;
 /// Provides boot information when booting on a Virtual Machine.
 #[repr(C)]
 pub struct Vm {
-    /// Physical address of one page for console memory.
+    /// Address of [ConsoleMemory].
     pub console: usize,
     /// Page size on the host.
     pub host_page_size: NonZero<usize>,
 }
+
+/// Layout of console memory.
+///
+/// The sequence of operations on a console memory is per-cpu. The kernel will start each log by:
+///
+/// 1. Write [`Self::msg_len`] then [`Self::msg_addr`].
+/// 2. Repeat step 1 until the whole message has been written.
+/// 3. Write [`Self::commit`].
+///
+/// Beware that each write to [`Self::msg_len`] except the last one may not cover the full message.
+/// The consequence of this is [`Self::msg_addr`] may point to an incomplete UTF-8 byte sequence.
+/// That mean you should buffer the message until [`Self::commit`] has been written before validate
+/// if it is a valid UTF-8.
+#[cfg(feature = "virt")]
+#[repr(C)]
+pub struct ConsoleMemory {
+    pub msg_len: NonZero<usize>,
+    pub msg_addr: usize,
+    pub commit: ConsoleType,
+}
+
+/// Type of console message.
+#[cfg(feature = "virt")]
+#[repr(u8)]
+#[derive(Clone, Copy, num_enum::IntoPrimitive, num_enum::TryFromPrimitive)]
+pub enum ConsoleType {
+    Info,
+    Warn,
+    Error,
+}

src/obkrnl/Cargo.toml

@@ -8,7 +8,6 @@ anstyle = { version = "1.0.8", default-features = false }
 bitfield-struct = "0.8.0"
 hashbrown = "0.14.5"
 macros = { path = "../macros" }
-obconf = { path = "../obconf" }
-obvirt = { path = "../obvirt" }
+obconf = { path = "../obconf", features = ["virt"] }
 spin = { version = "0.9.8", features = ["spin_mutex"], default-features = false }
 talc = { version = "4.4.1", default-features = false }

src/obkrnl/src/console/mod.rs

@@ -1,8 +1,7 @@
 use crate::config::boot_env;
 use anstyle::{AnsiColor, Color, Style};
 use core::fmt::{Display, Formatter};
-use obconf::BootEnv;
-use obvirt::console::MsgType;
+use obconf::{BootEnv, ConsoleType};
 
 mod vm;
 
@@ -13,27 +12,31 @@ mod vm;
 ///
 /// The LF character will be automatically appended.
 ///
+/// # Context safety
+/// This macro does not require a CPU context as long as [`Display`] implementation on all arguments
+/// does not.
+///
 /// # Interupt safety
 /// This macro is interupt safe as long as [`Display`] implementation on all arguments are interupt
 /// safe (e.g. no heap allocation).
 #[macro_export]
 macro_rules! info {
     ($($args:tt)*) => {
-        // This macro is not allowed to access the CPU context due to it can be called before the
-        // context has been activated.
         $crate::console::info(file!(), line!(), format_args!($($args)*))
     };
 }
 
+/// # Context safety
+/// This function does not require a CPU context as long as [`Display`] implementation on `msg` does
+/// not.
+///
 /// # Interupt safety
 /// This function is interupt safe as long as [`Display`] implementation on `msg` are interupt safe
 /// (e.g. no heap allocation).
 #[inline(never)]
 pub fn info(file: &str, line: u32, msg: impl Display) {
-    // This function is not allowed to access the CPU context due to it can be called before the
-    // context has been activated.
     print(
-        MsgType::Info,
+        ConsoleType::Info,
         Log {
             style: Style::new().fg_color(Some(Color::Ansi(AnsiColor::BrightCyan))),
             cat: 'I',
@@ -44,15 +47,17 @@ pub fn info(file: &str, line: u32, msg: impl Display) {
     );
 }
 
+/// # Context safety
+/// This function does not require a CPU context as long as [`Display`] implementation on `msg` does
+/// not.
+///
 /// # Interupt safety
 /// This function is interupt safe as long as [`Display`] implementation on `msg` are interupt safe
 /// (e.g. no heap allocation).
 #[inline(never)]
 pub fn error(file: &str, line: u32, msg: impl Display) {
-    // This function is not allowed to access the CPU context due to it can be called before the
-    // context has been activated.
     print(
-        MsgType::Error,
+        ConsoleType::Error,
         Log {
             style: Style::new().fg_color(Some(Color::Ansi(AnsiColor::BrightRed))),
             cat: 'E',
@@ -63,18 +68,24 @@ pub fn error(file: &str, line: u32, msg: impl Display) {
     )
 }
 
+/// # Context safety
+/// This function does not require a CPU context as long as [`Display`] implementation on `msg` does
+/// not.
+///
 /// # Interupt safety
 /// This function is interupt safe as long as [`Display`] implementation on `msg` are interupt safe
 /// (e.g. no heap allocation).
-fn print(vty: MsgType, msg: impl Display) {
-    // This function is not allowed to access the CPU context due to it can be called before the
-    // context has been activated.
+fn print(ty: ConsoleType, msg: impl Display) {
     match boot_env() {
-        BootEnv::Vm(env) => self::vm::print(env, vty, msg),
+        BootEnv::Vm(env) => self::vm::print(env, ty, msg),
     }
 }
 
 /// [`Display`] implementation to format each log.
+///
+/// # Context safety
+/// [`Display`] implementation on this type does not require a CPU context as long as [`Log::msg`]
+/// does not.
 struct Log<'a, M: Display> {
     style: Style,
     cat: char,
@@ -85,8 +96,6 @@ struct Log<'a, M: Display> {
 
 impl<'a, M: Display> Display for Log<'a, M> {
     fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result {
-        // This implementation must be interupt safe and is not allowed to access the CPU context
-        // due to it can be called before the context has been activated.
         writeln!(
             f,
             "{}++++++++++++++++++ {} {}:{}{0:#}",

src/obkrnl/src/console/vm.rs

@@ -1,30 +1,81 @@
+use core::cmp::min;
 use core::fmt::{Display, Write};
+use core::num::NonZero;
 use core::ptr::{addr_of_mut, write_volatile};
-use obconf::Vm;
-use obvirt::console::{Memory, MsgType};
+use obconf::{ConsoleMemory, ConsoleType, Vm};
 
+/// # Context safety
+/// This function does not require a CPU context as long as [`Display`] implementation on `msg` does
+/// not.
+///
 /// # Interupt safety
 /// This function is interupt safe as long as [`Display`] implementation on `msg` are interupt safe
 /// (e.g. no heap allocation).
-pub fn print(env: &Vm, ty: MsgType, msg: impl Display) {
-    // This function is not allowed to access the CPU context due to it can be called before the
-    // context has been activated.
-    let c = env.console as *mut Memory;
-    let mut w = Writer(c);
+pub fn print(env: &Vm, ty: ConsoleType, msg: impl Display) {
+    let c = env.console as *mut ConsoleMemory;
+    let mut w = Writer {
+        con: c,
+        buf: [0; 1024],
+        len: 0,
+    };
 
     writeln!(w, "{msg}").unwrap();
+    drop(w);
 
     unsafe { write_volatile(addr_of_mut!((*c).commit), ty) };
 }
 
-struct Writer(*mut Memory);
+/// [Write] implementation to write the message to the VMM console.
+///
+/// # Context safety
+/// [Write] implementation on this type does not require a CPU context.
+struct Writer {
+    con: *mut ConsoleMemory,
+    buf: [u8; 1024],
+    len: usize,
+}
+
+impl Writer {
+    fn flush(&mut self) {
+        let len = match NonZero::new(self.len) {
+            Some(v) => v,
+            None => return,
+        };
+
+        unsafe { write_volatile(addr_of_mut!((*self.con).msg_len), len) };
+        unsafe { write_volatile(addr_of_mut!((*self.con).msg_addr), self.buf.as_ptr() as _) };
+
+        self.len = 0;
+    }
+}
+
+impl Drop for Writer {
+    fn drop(&mut self) {
+        self.flush();
+    }
+}
 
 impl Write for Writer {
     fn write_str(&mut self, s: &str) -> core::fmt::Result {
-        // This implementation must be interupt safe and is not allowed to access the CPU context
-        // due to it can be called before the context has been activated.
-        unsafe { write_volatile(addr_of_mut!((*self.0).msg_len), s.len()) };
-        unsafe { write_volatile(addr_of_mut!((*self.0).msg_addr), s.as_ptr() as usize) };
+        let mut s = s.as_bytes();
+
+        while !s.is_empty() {
+            // Append to the available buffer.
+            let available = self.buf.len() - self.len;
+            let len = min(s.len(), available);
+            let (src, remain) = s.split_at(len);
+
+            self.buf[self.len..(self.len + len)].copy_from_slice(src);
+            self.len += len;
+
+            // Flush if the buffer is full.
+            if self.len == self.buf.len() {
+                self.flush();
+            }
+
+            s = remain;
+        }
+
         Ok(())
     }
 }

src/obvirt/Cargo.toml

@@ -1,6 +0,0 @@
-[package]
-name = "obvirt"
-version = "0.1.0"
-edition = "2021"
-
-[dependencies]

src/obvirt/src/console/mod.rs

@@ -1,35 +0,0 @@
-/// Layout of console memory.
-///
-/// The sequence of operations on a console memory is per-cpu. The kernel will start each log by:
-///
-/// 1. Write [`Self::msg_len`] then [`Self::msg_addr`].
-/// 2. Repeat step 1 until the whole message has been written.
-/// 3. Write [`Self::commit`].
-#[repr(C)]
-pub struct Memory {
-    pub msg_len: usize,
-    pub msg_addr: usize,
-    pub commit: MsgType,
-}
-
-/// Type of console message.
-#[repr(u8)]
-#[derive(Clone, Copy)]
-pub enum MsgType {
-    Info,
-    Warn,
-    Error,
-}
-
-impl MsgType {
-    pub fn from_u8(v: u8) -> Option<Self> {
-        let v = match v {
-            v if v == Self::Info as u8 => Self::Info,
-            v if v == Self::Warn as u8 => Self::Warn,
-            v if v == Self::Error as u8 => Self::Error,
-            _ => return None,
-        };
-
-        Some(v)
-    }
-}

src/obvirt/src/lib.rs

@@ -1,3 +0,0 @@
-#![no_std]
-
-pub mod console;