Clean up emergency console and use panic for unexpected exceptions.
This removes `eprintln`, which was easy to misuse and not really sound.
Instead, use `emergency_uart`, which has been made unsafe with a clearly
documented safety requirement that it can only be called when the normal
UART instance will never be used again, e.g. just before rebooting.
Also changed `emergency_uart` to return an error rather than panicking,
so panic handler can ignore the error.
Test: atest vmbase_example.integration_test
Change-Id: I76ac1911cf905fde0010054cfd8bc239699298f6
diff --git a/libs/libvmbase/src/arch/aarch64/exceptions.rs b/libs/libvmbase/src/arch/aarch64/exceptions.rs
index 1868bf7..787ef37 100644
--- a/libs/libvmbase/src/arch/aarch64/exceptions.rs
+++ b/libs/libvmbase/src/arch/aarch64/exceptions.rs
@@ -14,12 +14,17 @@
//! Helper functions and structs for exception handlers.
-use crate::memory::{MemoryTrackerError, MEMORY};
use crate::{
- arch::aarch64::layout::UART_PAGE_ADDR, arch::VirtualAddress, eprintln, memory::page_4kb_of,
+ arch::{
+ aarch64::layout::UART_PAGE_ADDR,
+ platform::{emergency_uart, DEFAULT_EMERGENCY_CONSOLE_INDEX},
+ VirtualAddress,
+ },
+ memory::{page_4kb_of, MemoryTrackerError, MEMORY},
+ power::reboot,
read_sysreg,
};
-use core::fmt;
+use core::fmt::{self, Write};
use core::result;
/// Represents an error that can occur while handling an exception.
@@ -122,14 +127,23 @@
Self { esr, far: VirtualAddress(far) }
}
- /// Prints the details of an obj and the exception, excluding UART exceptions.
- pub fn print<T: fmt::Display>(&self, exception_name: &str, obj: T, elr: u64) {
+ /// Prints the details of an obj and the exception, excluding UART exceptions, and then reboots.
+ ///
+ /// This uses the emergency console so can safely be called even for synchronous exceptions
+ /// without causing a deadlock.
+ pub fn print_and_reboot<T: fmt::Display>(&self, exception_name: &str, obj: T, elr: u64) -> ! {
// Don't print to the UART if we are handling an exception it could raise.
if !self.is_uart_exception() {
- eprintln!("{exception_name}");
- eprintln!("{obj}");
- eprintln!("{}, elr={:#08x}", self, elr);
+ // SAFETY: We always reboot at the end of this method so there is no way for the
+ // original UART driver to be used after this.
+ if let Some(mut console) = unsafe { emergency_uart(DEFAULT_EMERGENCY_CONSOLE_INDEX) } {
+ // Ignore errors writing to emergency console, as we are about to reboot anyway.
+ _ = writeln!(console, "{exception_name}");
+ _ = writeln!(console, "{obj}");
+ _ = writeln!(console, "{}, elr={:#08x}", self, elr);
+ }
}
+ reboot();
}
fn is_uart_exception(&self) -> bool {
diff --git a/libs/libvmbase/src/arch/aarch64/platform.rs b/libs/libvmbase/src/arch/aarch64/platform.rs
index b33df68..7e002d0 100644
--- a/libs/libvmbase/src/arch/aarch64/platform.rs
+++ b/libs/libvmbase/src/arch/aarch64/platform.rs
@@ -104,18 +104,25 @@
/// Return platform uart with specific index
///
-/// Panics if console was not initialized by calling [`init`] first.
-pub fn uart(id: usize) -> &'static spin::mutex::SpinMutex<Uart> {
- CONSOLES[id].get().unwrap()
+/// Returns `None` if console was not initialized by calling [`init`] first.
+pub fn uart(id: usize) -> Option<&'static SpinMutex<Uart>> {
+ CONSOLES[id].get()
}
-/// Reinitializes the emergency UART driver and returns it.
+/// Reinitializes the n-th UART driver and returns it.
///
/// This is intended for use in situations where the UART may be in an unknown state or the global
-/// instance may be locked, such as in an exception handler or panic handler.
-pub fn emergency_uart() -> Uart {
+/// instance may be locked, such as in the synchronous exception handler.
+///
+/// # Safety
+///
+/// This takes over the UART from wherever it is being used, the existing UART instance should not
+/// be used after this is called. This should only be used immediately before aborting the VM.
+pub unsafe fn emergency_uart(id: usize) -> Option<Uart> {
+ let addr = *ADDRESSES[id].get()?;
+
// SAFETY: Initialization of UART using dedicated const address.
- unsafe { Uart::new(UART_ADDRESSES[DEFAULT_EMERGENCY_CONSOLE_INDEX]) }
+ Some(unsafe { Uart::new(addr) })
}
/// Makes a `PSCI_SYSTEM_OFF` call to shutdown the VM.
diff --git a/libs/libvmbase/src/console.rs b/libs/libvmbase/src/console.rs
index 6d9a4fe..dcaf1ad 100644
--- a/libs/libvmbase/src/console.rs
+++ b/libs/libvmbase/src/console.rs
@@ -14,33 +14,26 @@
//! Console driver for 8250 UART.
-use crate::arch::platform;
-use core::fmt::{write, Arguments, Write};
+use crate::arch::platform::{self, emergency_uart, DEFAULT_EMERGENCY_CONSOLE_INDEX};
+use crate::power::reboot;
+use core::fmt::{self, write, Arguments, Write};
+use core::panic::PanicInfo;
/// Writes a formatted string followed by a newline to the n-th console.
///
-/// Panics if the n-th console was not initialized by calling [`init`] first.
-pub fn writeln(n: usize, format_args: Arguments) {
- let uart = &mut *platform::uart(n).lock();
- write(uart, format_args).unwrap();
- let _ = uart.write_str("\n");
-}
+/// Returns an error if the n-th console was not initialized by calling [`init`] first.
+pub fn writeln(n: usize, format_args: Arguments) -> fmt::Result {
+ let uart = &mut *platform::uart(n).ok_or(fmt::Error)?.lock();
-/// Reinitializes the emergency UART driver and writes a formatted string followed by a newline to
-/// it.
-///
-/// This is intended for use in situations where the UART may be in an unknown state or the global
-/// instance may be locked, such as in an exception handler or panic handler.
-pub fn ewriteln(format_args: Arguments) {
- let mut uart = platform::emergency_uart();
- let _ = write(&mut uart, format_args);
- let _ = uart.write_str("\n");
+ write(uart, format_args)?;
+ uart.write_str("\n")?;
+ Ok(())
}
/// Prints the given formatted string to the n-th console, followed by a newline.
///
-/// Panics if the console has not yet been initialized. May hang if used in an exception context;
-/// use `eprintln!` instead.
+/// Returns an error if the console has not yet been initialized. May deadlock if used in a
+/// synchronous exception handler.
#[macro_export]
macro_rules! console_writeln {
($n:expr, $($arg:tt)*) => ({
@@ -48,27 +41,24 @@
})
}
-pub(crate) use console_writeln;
-
/// Prints the given formatted string to the console, followed by a newline.
///
-/// Panics if the console has not yet been initialized. May hang if used in an exception context;
-/// use `eprintln!` instead.
+/// Panics if the console has not yet been initialized. May hang if used in an exception context.
macro_rules! println {
($($arg:tt)*) => ({
- $crate::console::console_writeln!($crate::arch::platform::DEFAULT_CONSOLE_INDEX, $($arg)*)
+ $crate::console_writeln!($crate::arch::platform::DEFAULT_CONSOLE_INDEX, $($arg)*).unwrap()
})
}
pub(crate) use println; // Make it available in this crate.
-/// Prints the given string followed by a newline to the console in an emergency, such as an
-/// exception handler.
-///
-/// Never panics.
-#[macro_export]
-macro_rules! eprintln {
- ($($arg:tt)*) => ({
- $crate::console::ewriteln(format_args!($($arg)*))
- })
+#[panic_handler]
+fn panic(info: &PanicInfo) -> ! {
+ // SAFETY: We always reboot at the end of this method so there is no way for the
+ // original UART driver to be used after this.
+ if let Some(mut console) = unsafe { emergency_uart(DEFAULT_EMERGENCY_CONSOLE_INDEX) } {
+ // Ignore errors, to avoid a panic loop.
+ let _ = writeln!(console, "{}", info);
+ }
+ reboot()
}
diff --git a/libs/libvmbase/src/lib.rs b/libs/libvmbase/src/lib.rs
index d254038..afb62fc 100644
--- a/libs/libvmbase/src/lib.rs
+++ b/libs/libvmbase/src/lib.rs
@@ -32,12 +32,3 @@
pub mod uart;
pub mod util;
pub mod virtio;
-
-use core::panic::PanicInfo;
-use power::reboot;
-
-#[panic_handler]
-fn panic(info: &PanicInfo) -> ! {
- eprintln!("{}", info);
- reboot()
-}
diff --git a/libs/libvmbase/src/logger.rs b/libs/libvmbase/src/logger.rs
index 9130918..0059d11 100644
--- a/libs/libvmbase/src/logger.rs
+++ b/libs/libvmbase/src/logger.rs
@@ -16,7 +16,7 @@
//!
//! Internally uses the println! vmbase macro, which prints to crosvm's UART.
//! Note: may not work if the VM is in an inconsistent state. Exception handlers
-//! should avoid using this logger and instead print with eprintln!.
+//! should avoid using this logger.
use crate::console::println;
use core::sync::atomic::{AtomicBool, Ordering};