From 6cff6c5aad5309c93914944e3af46f799ff17bb2 Mon Sep 17 00:00:00 2001 From: Nicholas Bishop Date: Thu, 2 May 2024 15:09:53 -0400 Subject: [PATCH 1/4] uefi: Sort the code in table/mod.rs --- uefi/src/table/mod.rs | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/uefi/src/table/mod.rs b/uefi/src/table/mod.rs index 4917946a4..c0afc07ba 100644 --- a/uefi/src/table/mod.rs +++ b/uefi/src/table/mod.rs @@ -1,21 +1,19 @@ //! Standard UEFI tables. +pub mod boot; +pub mod cfg; +pub mod runtime; + +mod header; +mod system; + +pub use header::Header; +pub use system::{Boot, Runtime, SystemTable}; +pub use uefi_raw::table::Revision; + /// Common trait implemented by all standard UEFI tables. pub trait Table { /// A unique number assigned by the UEFI specification /// to the standard tables. const SIGNATURE: u64; } - -mod header; -pub use header::Header; - -mod system; -pub use system::{Boot, Runtime, SystemTable}; - -pub mod boot; -pub mod runtime; - -pub mod cfg; - -pub use uefi_raw::table::Revision; From a42ddc5bf25ac4f41b851f821116d1c8ab655ff2 Mon Sep 17 00:00:00 2001 From: Nicholas Bishop Date: Thu, 2 May 2024 15:24:51 -0400 Subject: [PATCH 2/4] uefi: Add global system table pointer and API This is a minimal solution to providing a global system table so that users of the API don't have to pass around references to `SystemTable` / `BootServices` / `RuntimeServices` everywhere. --- uefi/CHANGELOG.md | 3 ++ uefi/src/table/mod.rs | 65 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/uefi/CHANGELOG.md b/uefi/CHANGELOG.md index 866baf204..19633769d 100644 --- a/uefi/CHANGELOG.md +++ b/uefi/CHANGELOG.md @@ -8,6 +8,9 @@ works on x86. It is activated by default (only on x86) and can be deactivated by removing the `log-debugcon` cargo feature. The major benefit is that one can get log messages even after one exited the boot services. +- Added `table::{set_system_table, system_table_boot, system_table_runtime}`. + This provides an initial API for global tables that do not require passing + around a reference. ## Removed - Removed the `panic-on-logger-errors` feature of the `uefi` crate. Logger diff --git a/uefi/src/table/mod.rs b/uefi/src/table/mod.rs index c0afc07ba..7282d8148 100644 --- a/uefi/src/table/mod.rs +++ b/uefi/src/table/mod.rs @@ -11,6 +11,71 @@ pub use header::Header; pub use system::{Boot, Runtime, SystemTable}; pub use uefi_raw::table::Revision; +use core::ptr; +use core::sync::atomic::{AtomicPtr, Ordering}; + +/// Global system table pointer. This is only modified by [`set_system_table`]. +static SYSTEM_TABLE: AtomicPtr = + AtomicPtr::new(ptr::null_mut()); + +/// Update the global system table pointer. +/// +/// This is called automatically in the `main` entry point as part of +/// [`uefi::entry`]. It should not be called at any other point in time, unless +/// the executable does not use [`uefi::entry`], in which case it should be +/// called once before calling any other API in this crate. +/// +/// # Safety +/// +/// This function should only be called as described above, and the +/// `ptr` must be a valid [`SystemTable`]. +pub unsafe fn set_system_table(ptr: *const uefi_raw::table::system::SystemTable) { + SYSTEM_TABLE.store(ptr.cast_mut(), Ordering::Release); +} + +/// 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 { + let st = SYSTEM_TABLE.load(Ordering::Acquire); + assert!(!st.is_null()); + + // 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"); + } + + 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 { + let st = SYSTEM_TABLE.load(Ordering::Acquire); + assert!(!st.is_null()); + + // 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"); + } + + SystemTable::::from_ptr(st.cast()).unwrap() + } +} + /// Common trait implemented by all standard UEFI tables. pub trait Table { /// A unique number assigned by the UEFI specification From ec956eb6f5cf2a4ea7a47025f965a2f1bb8e6197 Mon Sep 17 00:00:00 2001 From: Nicholas Bishop Date: Thu, 2 May 2024 15:36:48 -0400 Subject: [PATCH 3/4] uefi-macros: Call set_image_handle from the entry macro --- uefi-macros/CHANGELOG.md | 3 +++ uefi-macros/src/lib.rs | 1 + 2 files changed, 4 insertions(+) diff --git a/uefi-macros/CHANGELOG.md b/uefi-macros/CHANGELOG.md index b56ff7fce..a2e56ccd4 100644 --- a/uefi-macros/CHANGELOG.md +++ b/uefi-macros/CHANGELOG.md @@ -1,5 +1,8 @@ # uefi-macros - [Unreleased] +## Changed +- The `entry` macro now sets the global system table pointer with `uefi::set_system_table`. + ## Removed - Removed the `cstr8` and `cstr16` macros. Use the declarative macros of the same names exported by the `uefi` crate as a replacement. diff --git a/uefi-macros/src/lib.rs b/uefi-macros/src/lib.rs index 9f5b67da8..09a67716e 100644 --- a/uefi-macros/src/lib.rs +++ b/uefi-macros/src/lib.rs @@ -204,6 +204,7 @@ pub fn entry(args: TokenStream, input: TokenStream) -> TokenStream { parse_quote! { unsafe { #system_table_ident.boot_services().set_image_handle(#image_handle_ident); + ::uefi::table::set_system_table(#system_table_ident.as_ptr().cast()); } }, ); From c04e423cedfa0321e4110b75adb6ad368a63b996 Mon Sep 17 00:00:00 2001 From: Nicholas Bishop Date: Thu, 2 May 2024 16:04:25 -0400 Subject: [PATCH 4/4] uefi: Make exit_boot_services unsafe This method was already unsafe in practice, since it's very hard to statically ensure that no references to boot-services data exist. Change the signature to acknowledge that reality, and update the docstring with details. --- uefi-test-runner/src/main.rs | 2 +- uefi/CHANGELOG.md | 4 ++++ uefi/src/table/system.rs | 29 ++++++++++++++++++++++++++--- 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/uefi-test-runner/src/main.rs b/uefi-test-runner/src/main.rs index 39baced14..61d763c74 100644 --- a/uefi-test-runner/src/main.rs +++ b/uefi-test-runner/src/main.rs @@ -199,7 +199,7 @@ fn shutdown(mut st: SystemTable) -> ! { info!("Testing complete, exiting boot services..."); // Exit boot services as a proof that it works :) - let (st, mmap) = st.exit_boot_services(MemoryType::LOADER_DATA); + let (st, mmap) = unsafe { st.exit_boot_services(MemoryType::LOADER_DATA) }; info!("Memory Map:"); for desc in mmap.entries() { diff --git a/uefi/CHANGELOG.md b/uefi/CHANGELOG.md index 19633769d..d7c79efdc 100644 --- a/uefi/CHANGELOG.md +++ b/uefi/CHANGELOG.md @@ -12,6 +12,10 @@ This provides an initial API for global tables that do not require passing around a reference. +## Changed +- `SystemTable::exit_boot_services` is now `unsafe`. See that method's + documentation for details of obligations for callers. + ## Removed - Removed the `panic-on-logger-errors` feature of the `uefi` crate. Logger errors are now silently ignored. diff --git a/uefi/src/table/system.rs b/uefi/src/table/system.rs index d9cdec170..ab7cb3305 100644 --- a/uefi/src/table/system.rs +++ b/uefi/src/table/system.rs @@ -208,8 +208,27 @@ impl SystemTable { /// /// Note that once the boot services are exited, associated loggers and /// allocators can't use the boot services anymore. For the corresponding - /// abstractions provided by this crate, invoking this function will - /// automatically disable them. + /// abstractions provided by this crate (see the [`helpers`] module), + /// invoking this function will automatically disable them. If the + /// `global_allocator` feature is enabled, attempting to use the allocator + /// after exiting boot services will panic. + /// + /// # Safety + /// + /// The caller is responsible for ensuring that no references to + /// boot-services data remain. A non-exhaustive list of resources to check: + /// + /// * All protocols will be invalid after exiting boot services. This + /// includes the [`Output`] protocols attached to stdout/stderr. The + /// caller must ensure that no protocol references remain. + /// * The pool allocator is not usable after exiting boot services. Types + /// such as [`PoolString`] which call [`BootServices::free_pool`] on drop + /// must be cleaned up before calling `exit_boot_services`, or leaked to + /// avoid drop ever being called. + /// * All data in the memory map marked as + /// [`MemoryType::BOOT_SERVICES_CODE`] and + /// [`MemoryType::BOOT_SERVICES_DATA`] will become free memory, the caller + /// must ensure that no references to such memory exist. /// /// # Errors /// @@ -220,8 +239,12 @@ impl SystemTable { /// All errors are treated as unrecoverable because the system is /// now in an undefined state. Rather than returning control to the /// caller, the system will be reset. + /// + /// [`helpers`]: crate::helpers + /// [`Output`]: crate::proto::console::text::Output + /// [`PoolString`]: crate::proto::device_path::text::PoolString #[must_use] - pub fn exit_boot_services( + pub unsafe fn exit_boot_services( self, memory_type: MemoryType, ) -> (SystemTable, MemoryMap<'static>) {