From c649727402f669f0b3b5bb6f1ec800df364ea34c Mon Sep 17 00:00:00 2001 From: Nicholas Bishop Date: Sat, 8 Jun 2024 12:47:22 -0400 Subject: [PATCH 1/3] uefi: Remove panics from system_table_boot/system_table_runtime These functions now return an `Option`. --- uefi/src/table/mod.rs | 37 ++++++++++++++----------------------- 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/uefi/src/table/mod.rs b/uefi/src/table/mod.rs index 7282d8148..078c0f79f 100644 --- a/uefi/src/table/mod.rs +++ b/uefi/src/table/mod.rs @@ -34,45 +34,36 @@ pub unsafe fn set_system_table(ptr: *const uefi_raw::table::system::SystemTable) } /// Get the system table while boot services are active. -/// -/// # Panics -/// -/// Panics if the system table has not been set with `set_system_table`, or if -/// boot services are not available (e.g. if [`exit_boot_services`] has been -/// called). -/// -/// [`exit_boot_services`]: SystemTable::exit_boot_services -pub fn system_table_boot() -> SystemTable { +pub fn system_table_boot() -> Option> { let st = SYSTEM_TABLE.load(Ordering::Acquire); - assert!(!st.is_null()); + if st.is_null() { + return None; + } // SAFETY: the system table is valid per the requirements of `set_system_table`. unsafe { if (*st).boot_services.is_null() { - panic!("boot services are not active"); + None + } else { + Some(SystemTable::::from_ptr(st.cast()).unwrap()) } - - SystemTable::::from_ptr(st.cast()).unwrap() } } /// Get the system table while runtime services are active. -/// -/// # Panics -/// -/// Panics if the system table has not been set with `set_system_table`, or if -/// runtime services are not available. -pub fn system_table_runtime() -> SystemTable { +pub fn system_table_runtime() -> Option> { let st = SYSTEM_TABLE.load(Ordering::Acquire); - assert!(!st.is_null()); + if st.is_null() { + return None; + } // SAFETY: the system table is valid per the requirements of `set_system_table`. unsafe { if (*st).runtime_services.is_null() { - panic!("runtime services are not active"); + None + } else { + Some(SystemTable::::from_ptr(st.cast()).unwrap()) } - - SystemTable::::from_ptr(st.cast()).unwrap() } } From 2a63b9e95689563cd302ae0e1cfd05135ad89ae2 Mon Sep 17 00:00:00 2001 From: Nicholas Bishop Date: Sat, 8 Jun 2024 12:59:47 -0400 Subject: [PATCH 2/3] uefi: Drop duplicate global system table pointer Now that `uefi::table` has a pointer to the system table, drop the pointer from `uefi::helpers`. --- uefi/src/helpers/mod.rs | 44 +++---------------------------- uefi/src/helpers/panic_handler.rs | 6 ++--- uefi/src/helpers/println.rs | 5 ++-- 3 files changed, 9 insertions(+), 46 deletions(-) diff --git a/uefi/src/helpers/mod.rs b/uefi/src/helpers/mod.rs index 958d64c57..910929cdd 100644 --- a/uefi/src/helpers/mod.rs +++ b/uefi/src/helpers/mod.rs @@ -19,13 +19,9 @@ //! [println_macro]: uefi::println! use crate::prelude::{Boot, SystemTable}; -use crate::{Result, StatusExt}; -use core::ffi::c_void; -use core::ptr; -use core::sync::atomic::{AtomicPtr, Ordering}; +use crate::{table, Result}; #[doc(hidden)] pub use println::_print; -use uefi_raw::Status; #[cfg(feature = "global_allocator")] mod global_allocator; @@ -35,25 +31,6 @@ mod logger; mod panic_handler; mod println; -/// Reference to the system table. -/// -/// This table is only fully safe to use until UEFI boot services have been exited. -/// After that, some fields and methods are unsafe to use, see the documentation of -/// UEFI's ExitBootServices entry point for more details. -static SYSTEM_TABLE: AtomicPtr = AtomicPtr::new(core::ptr::null_mut()); - -#[must_use] -fn system_table_opt() -> Option> { - let ptr = SYSTEM_TABLE.load(Ordering::Acquire); - // Safety: the `SYSTEM_TABLE` pointer either be null or a valid system - // table. - // - // Null is the initial value, as well as the value set when exiting boot - // services. Otherwise, the value is set by the call to `init`, which - // requires a valid system table reference as input. - unsafe { SystemTable::from_ptr(ptr) } -} - /// Obtains a pointer to the system table. /// /// This is meant to be used by higher-level libraries, @@ -65,7 +42,7 @@ fn system_table_opt() -> Option> { #[must_use] // TODO do we want to keep this public? pub fn system_table() -> SystemTable { - system_table_opt().expect("The system table handle is not available") + table::system_table_boot().expect("boot services are not active") } /// Initialize all helpers defined in [`uefi::helpers`] whose Cargo features @@ -77,15 +54,8 @@ pub fn system_table() -> SystemTable { /// **PLEASE NOTE** that these helpers are meant for the pre exit boot service /// epoch. Limited functionality might work after exiting them, such as logging /// to the debugcon device. +#[allow(unused_variables)] // `st` is unused if logger and allocator are disabled pub fn init(st: &mut SystemTable) -> Result<()> { - if system_table_opt().is_some() { - // Avoid double initialization. - return Status::SUCCESS.to_result_with_val(|| ()); - } - - // Setup the system table singleton - SYSTEM_TABLE.store(st.as_ptr().cast_mut(), Ordering::Release); - // Setup logging and memory allocation #[cfg(feature = "logger")] @@ -102,14 +72,6 @@ pub fn init(st: &mut SystemTable) -> Result<()> { } pub(crate) fn exit() { - // DEBUG: The UEFI spec does not guarantee that this printout will work, as - // the services used by logging might already have been shut down. - // But it works on current OVMF, and can be used as a handy way to - // check that the callback does get called. - // - // info!("Shutting down the UEFI utility library"); - SYSTEM_TABLE.store(ptr::null_mut(), Ordering::Release); - #[cfg(feature = "logger")] logger::disable(); diff --git a/uefi/src/helpers/panic_handler.rs b/uefi/src/helpers/panic_handler.rs index c5f672a1a..5918395b8 100644 --- a/uefi/src/helpers/panic_handler.rs +++ b/uefi/src/helpers/panic_handler.rs @@ -1,5 +1,5 @@ -use crate::helpers::system_table_opt; use crate::println; +use crate::table::system_table_boot; use cfg_if::cfg_if; #[panic_handler] @@ -7,7 +7,7 @@ fn panic_handler(info: &core::panic::PanicInfo) -> ! { println!("[PANIC]: {}", info); // Give the user some time to read the message - if let Some(st) = system_table_opt() { + if let Some(st) = system_table_boot() { st.boot_services().stall(10_000_000); } else { let mut dummy = 0u64; @@ -28,7 +28,7 @@ fn panic_handler(info: &core::panic::PanicInfo) -> ! { qemu_exit_handle.exit_failure(); } else { // If the system table is available, use UEFI's standard shutdown mechanism - if let Some(st) = system_table_opt() { + if let Some(st) = system_table_boot() { use crate::table::runtime::ResetType; st.runtime_services() .reset(ResetType::SHUTDOWN, crate::Status::ABORTED, None); diff --git a/uefi/src/helpers/println.rs b/uefi/src/helpers/println.rs index 265911da1..2a30eb1d5 100644 --- a/uefi/src/helpers/println.rs +++ b/uefi/src/helpers/println.rs @@ -1,10 +1,11 @@ -use crate::helpers::system_table; +use crate::table::system_table_boot; use core::fmt::Write; /// INTERNAL API! Helper for print macros. #[doc(hidden)] pub fn _print(args: core::fmt::Arguments) { - system_table() + system_table_boot() + .expect("boot services are not active") .stdout() .write_fmt(args) .expect("Failed to write to stdout"); From 1970c81dbed94864c8f9a34bd48bc7b8d9395d98 Mon Sep 17 00:00:00 2001 From: Nicholas Bishop Date: Sat, 8 Jun 2024 13:05:23 -0400 Subject: [PATCH 3/3] uefi: Deprecate helpers::system_table This functionality is available with `table::system_table_boot` now. --- uefi/CHANGELOG.md | 1 + uefi/src/helpers/mod.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/uefi/CHANGELOG.md b/uefi/CHANGELOG.md index 70b95be05..bfba9babf 100644 --- a/uefi/CHANGELOG.md +++ b/uefi/CHANGELOG.md @@ -19,6 +19,7 @@ documentation for details of obligations for callers. - `BootServices::allocate_pool` now returns `NonZero` instead of `*mut u8`. +- `helpers::system_table` is deprecated, use `table::system_table_boot` instead. ## Removed - Removed the `panic-on-logger-errors` feature of the `uefi` crate. Logger diff --git a/uefi/src/helpers/mod.rs b/uefi/src/helpers/mod.rs index 910929cdd..e7afe9a6e 100644 --- a/uefi/src/helpers/mod.rs +++ b/uefi/src/helpers/mod.rs @@ -40,7 +40,7 @@ mod println; /// /// The returned pointer is only valid until boot services are exited. #[must_use] -// TODO do we want to keep this public? +#[deprecated(note = "use uefi::table::system_table_boot instead")] pub fn system_table() -> SystemTable { table::system_table_boot().expect("boot services are not active") }