From 5c9b2a54a0e48e1aec9519fa78b4d35ab4ff4646 Mon Sep 17 00:00:00 2001 From: Kat Perez Date: Thu, 22 Jan 2026 20:59:09 -0500 Subject: [PATCH 1/9] [feature/patina-boot] patina_boot: Add boot orchestration crate (#1225) Adds BootOrchestration component, simple console discovery, simple BootOption Config - [x] Impacts functionality? - [ ] Impacts security? - [ ] Breaking change? - [x] Includes tests? - [x] Includes documentation? QEMU Platform Integration: - Q35 - SBSA N/A --- Cargo.toml | 1 + components/patina_boot/Cargo.toml | 25 + components/patina_boot/README.md | 33 ++ components/patina_boot/src/component.rs | 18 + .../src/component/boot_orchestrator.rs | 104 +++++ .../src/component/console_discovery.rs | 42 ++ components/patina_boot/src/config.rs | 174 +++++++ components/patina_boot/src/helpers.rs | 430 ++++++++++++++++++ components/patina_boot/src/lib.rs | 30 ++ cspell.yml | 1 + patina_dxe_core/src/component_dispatcher.rs | 2 +- sdk/patina/src/component/struct_component.rs | 7 + 12 files changed, 866 insertions(+), 1 deletion(-) create mode 100644 components/patina_boot/Cargo.toml create mode 100644 components/patina_boot/README.md create mode 100644 components/patina_boot/src/component.rs create mode 100644 components/patina_boot/src/component/boot_orchestrator.rs create mode 100644 components/patina_boot/src/component/console_discovery.rs create mode 100644 components/patina_boot/src/config.rs create mode 100644 components/patina_boot/src/helpers.rs create mode 100644 components/patina_boot/src/lib.rs diff --git a/Cargo.toml b/Cargo.toml index 7495d1f71..a7f42be78 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -42,6 +42,7 @@ patina_lzma_rs = { version = "0.3.1", default-features = false } patina_macro = { version = "21.0.1", path = "sdk/patina_macro" } patina_mm = { version = "21.0.1", path = "components/patina_mm" } patina_mtrr = { version = "^1.1.6" } +patina_boot = { version = "21.0.1", path = "components/patina_boot" } patina_paging = { version = "11.0.2" } patina_performance = { version = "21.0.1", path = "components/patina_performance" } patina_smbios = { version = "21.0.1", path = "components/patina_smbios" } diff --git a/components/patina_boot/Cargo.toml b/components/patina_boot/Cargo.toml new file mode 100644 index 000000000..80c8e0c1b --- /dev/null +++ b/components/patina_boot/Cargo.toml @@ -0,0 +1,25 @@ +[package] +name = "patina_boot" +resolver = "2" +version.workspace = true +repository.workspace = true +license.workspace = true +edition.workspace = true +description = "Boot orchestration components for Patina firmware." + +[lints] +workspace = true + +[dependencies] +log = { workspace = true } +patina = { workspace = true, features = ["unstable-device-path"] } +patina_macro = { workspace = true } +r-efi = { workspace = true } + +[dev-dependencies] +patina = { path = "../../sdk/patina", features = ["mockall", "unstable-device-path"] } +mockall = { workspace = true } + +[features] +default = [] +std = [] diff --git a/components/patina_boot/README.md b/components/patina_boot/README.md new file mode 100644 index 000000000..fdc450dfd --- /dev/null +++ b/components/patina_boot/README.md @@ -0,0 +1,33 @@ +# Patina Boot + +Boot orchestration component for Patina-based firmware implementing UEFI boot manager functionality. + +## Components + +- **BootOrchestrator**: Orchestrates device enumeration, BDS phase events, and boot option execution. +- **ConsoleDiscovery**: Discovers console devices and populates UEFI console variables. + +## Usage + +```rust +use patina_boot::{component::BootOrchestrator, config::BootOptions}; + +// Configure boot options (devices are tried in order) +let config = BootOptions::new() + .with_device(primary_boot_path) + .with_device(fallback_boot_path) + .with_failure_handler(|| { /* handle boot failure */ }); + +// Add BootOrchestrator as a platform component +add.component(BootOrchestrator); +``` + +## Helper Functions + +For custom boot flows, use the helper functions in the `helpers` module: + +- `connect_all()` - Connect all controllers for device enumeration +- `signal_bds_phase_entry()` - Signal EndOfDxe event +- `signal_ready_to_boot()` - Signal ReadyToBoot event +- `discover_console_devices()` - Populate console variables +- `boot_from_device_path()` - Load and start a boot image diff --git a/components/patina_boot/src/component.rs b/components/patina_boot/src/component.rs new file mode 100644 index 000000000..fb6840a95 --- /dev/null +++ b/components/patina_boot/src/component.rs @@ -0,0 +1,18 @@ +//! Boot orchestration components. +//! +//! This module provides the core boot components: +//! - [`ConsoleDiscovery`]: Discovers console devices and populates ConIn/ConOut/ErrOut variables +//! - [`BootOrchestrator`]: Orchestrates the boot flow with device enumeration, event signaling, and boot execution +//! +//! ## License +//! +//! Copyright (c) Microsoft Corporation. +//! +//! SPDX-License-Identifier: Apache-2.0 +//! + +mod boot_orchestrator; +mod console_discovery; + +pub use boot_orchestrator::BootOrchestrator; +pub use console_discovery::ConsoleDiscovery; diff --git a/components/patina_boot/src/component/boot_orchestrator.rs b/components/patina_boot/src/component/boot_orchestrator.rs new file mode 100644 index 000000000..182120db8 --- /dev/null +++ b/components/patina_boot/src/component/boot_orchestrator.rs @@ -0,0 +1,104 @@ +//! Boot orchestrator component. +//! +//! Orchestrates the boot flow with device enumeration, event signaling, and boot execution. +//! +//! ## Rationale +//! +//! Boot orchestration is a component that represents the primary platform customization +//! point. Platforms provide boot options as configuration data, allowing different boot policies +//! without changing orchestration logic. Platforms needing entirely different boot flows can +//! replace the component. +//! +//! ## License +//! +//! Copyright (c) Microsoft Corporation. +//! +//! SPDX-License-Identifier: Apache-2.0 +//! +use patina::{ + boot_services::{BootServices, StandardBootServices, protocol_handler::HandleSearchType}, + component::{ + component, + params::{Config, Handle}, + }, + error::{EfiError, Result}, + runtime_services::StandardRuntimeServices, +}; +use r_efi::efi; + +use crate::{config::BootOptions, helpers}; + +/// Boot orchestrator component. +/// +/// Orchestrates boot execution using boot options provided via [`Config`]. +/// Connects all controllers for device topology enumeration, signals BDS phase events +/// (EndOfDxe, ReadyToBoot), and executes boot options via `LoadImage()`/`StartImage()` +/// with 5-minute watchdog per UEFI Section 3.1.2. +/// +/// Handles boot failures by attempting subsequent options. Never returns on success. +pub struct BootOrchestrator; + +#[component] +impl BootOrchestrator { + /// Entry point for the boot orchestrator component. + /// + /// # Flow + /// + /// 1. Connect all controllers for device enumeration + /// 2. Signal EndOfDxe (security components perform lockdown) + /// 3. Discover consoles + /// 4. Signal ReadyToBoot + /// 5. Execute boot options from config + /// 6. If all boot options fail, call failure handler + #[coverage(off)] // Component integration - tested via integration tests + fn entry_point( + self, + boot_services: StandardBootServices, + runtime_services: StandardRuntimeServices, + boot_options: Config, + image_handle: Option, + ) -> Result<()> { + helpers::connect_all(boot_services.as_ref())?; + helpers::signal_bds_phase_entry(boot_services.as_ref())?; + helpers::discover_console_devices(boot_services.as_ref(), runtime_services.as_ref())?; + helpers::signal_ready_to_boot(boot_services.as_ref())?; + + // Use the provided image handle, or fall back to finding one via LocateHandleBuffer. + // Per UEFI spec, the parent handle must be a valid image handle (has LoadedImage protocol). + let parent_handle = if let Some(handle) = image_handle { + *handle + } else { + log::warn!("Handle not provided, falling back to LocateHandleBuffer"); + let image_handles = boot_services + .locate_handle_buffer(HandleSearchType::ByProtocol(&efi::protocols::loaded_image::PROTOCOL_GUID)) + .map_err(|status| { + log::error!("Failed to locate image handles: {:?}", status); + EfiError::from(status) + })?; + + image_handles.first().copied().ok_or_else(|| { + log::error!("No image handles available for parent handle"); + EfiError::NotReady + })? + }; + + for device_path in boot_options.devices() { + match helpers::boot_from_device_path(boot_services.as_ref(), parent_handle, device_path) { + Ok(()) => { + // Boot image returned control (e.g., EFI application exited). + // Continue to try next boot option. + log::warn!("Boot option returned, trying next..."); + } + Err(_) => { + log::warn!("Boot option failed, trying next..."); + } + } + } + + boot_options.handle_failure(); + log::error!("All boot options exhausted and failure handler returned"); + loop { + core::hint::spin_loop(); + } + } +} diff --git a/components/patina_boot/src/component/console_discovery.rs b/components/patina_boot/src/component/console_discovery.rs new file mode 100644 index 000000000..b7d51f7c1 --- /dev/null +++ b/components/patina_boot/src/component/console_discovery.rs @@ -0,0 +1,42 @@ +//! Console discovery component. +//! +//! Locates GOP and SimpleTextInput protocol handles, creates device paths, +//! and writes ConIn/ConOut/ErrOut variables. +//! +//! ## Rationale +//! +//! Console setup is a component because platforms may need custom console configuration +//! (serial-only, specific GOP preference, headless operation). As a component, platforms +//! can replace it entirely without modifying orchestration code. +//! +//! ## Prerequisites +//! +//! Device controllers exposing GOP and input protocols must be connected before +//! this component runs. +//! +//! ## License +//! +//! Copyright (c) Microsoft Corporation. +//! +//! SPDX-License-Identifier: Apache-2.0 +//! +use patina::{ + boot_services::StandardBootServices, component::component, error::Result, runtime_services::StandardRuntimeServices, +}; + +use crate::helpers; + +/// Console discovery component. +/// +/// Scans for GOP and SimpleTextInput protocol handles, creates device paths, +/// and writes `ConIn`/`ConOut`/`ErrOut` UEFI variables. +pub struct ConsoleDiscovery; + +#[component] +impl ConsoleDiscovery { + /// Entry point for the console discovery component. + #[coverage(off)] // Component integration - tested via integration tests + fn entry_point(self, boot_services: StandardBootServices, runtime_services: StandardRuntimeServices) -> Result<()> { + helpers::discover_console_devices(boot_services.as_ref(), runtime_services.as_ref()) + } +} diff --git a/components/patina_boot/src/config.rs b/components/patina_boot/src/config.rs new file mode 100644 index 000000000..f0ee2fe49 --- /dev/null +++ b/components/patina_boot/src/config.rs @@ -0,0 +1,174 @@ +//! Boot configuration types. +//! +//! This module provides configuration types for boot orchestration, including +//! [`BootOptions`] which specifies platform-provided boot paths. +//! +//! ## License +//! +//! Copyright (c) Microsoft Corporation. +//! +//! SPDX-License-Identifier: Apache-2.0 +//! +extern crate alloc; + +use alloc::{boxed::Box, vec::Vec}; +use patina::uefi_protocol::device_path::DevicePathBuf; + +/// Boot options provided by the platform. +/// +/// Platforms configure boot behavior by providing this configuration to the +/// [`BootOrchestrator`](crate::component::BootOrchestrator) component. +/// +/// ## Example +/// +/// ```rust,ignore +/// use patina_boot::config::BootOptions; +/// +/// let options = BootOptions::new() +/// .with_device(nvme_device_path) +/// .with_device(usb_device_path) +/// .with_failure_handler(|| show_error_screen()); +/// ``` +#[derive(Default)] +pub struct BootOptions { + /// Boot device paths in priority order. + devices: Vec, + /// Optional hotkey for boot override (e.g., F12 for boot menu). + hotkey: Option, + /// Handler called when all boot options fail. + failure_handler: Option>, +} + +impl BootOptions { + /// Create new empty boot options. + pub fn new() -> Self { + Self::default() + } + + /// Add a boot device path. + /// + /// Device paths are tried in the order they are added. + pub fn with_device(mut self, device: DevicePathBuf) -> Self { + self.devices.push(device); + self + } + + /// Add a hotkey scancode for boot override (not yet implemented). + /// + /// This field is reserved for future boot menu functionality. + /// Currently, the hotkey is stored but not acted upon. + pub fn with_hotkey(mut self, scancode: u16) -> Self { + self.hotkey = Some(scancode); + self + } + + /// Add a failure handler called when all boot options fail. + pub fn with_failure_handler(mut self, handler: F) -> Self + where + F: Fn() + Send + Sync + 'static, + { + self.failure_handler = Some(Box::new(handler)); + self + } + + /// Get the hotkey scancode, if configured. + pub fn hotkey(&self) -> Option { + self.hotkey + } + + /// Returns an iterator over all configured boot device paths. + pub fn devices(&self) -> impl Iterator { + self.devices.iter() + } + + /// Call the failure handler if configured. + /// + /// This is called when all boot options have been exhausted. + pub fn handle_failure(&self) { + if let Some(handler) = &self.failure_handler { + handler(); + } + } +} + +#[cfg(test)] +mod tests { + extern crate alloc; + extern crate std; + + use super::*; + use alloc::sync::Arc; + use core::sync::atomic::{AtomicBool, Ordering}; + use patina::uefi_protocol::device_path::nodes::EndEntire; + + fn create_test_device_path() -> DevicePathBuf { + DevicePathBuf::from_device_path_node_iter(core::iter::once(EndEntire)) + } + + #[test] + fn test_default_boot_options() { + let options = BootOptions::default(); + assert!(options.hotkey().is_none()); + assert_eq!(options.devices().count(), 0); + } + + #[test] + fn test_new_boot_options() { + let options = BootOptions::new(); + assert_eq!(options.devices().count(), 0); + } + + #[test] + fn test_with_single_device() { + let device = create_test_device_path(); + let options = BootOptions::new().with_device(device); + assert_eq!(options.devices().count(), 1); + } + + #[test] + fn test_with_multiple_devices() { + let device1 = create_test_device_path(); + let device2 = create_test_device_path(); + let device3 = create_test_device_path(); + let options = BootOptions::new().with_device(device1).with_device(device2).with_device(device3); + assert_eq!(options.devices().count(), 3); + } + + #[test] + fn test_with_hotkey() { + let options = BootOptions::new().with_hotkey(0x86); // F12 + assert_eq!(options.hotkey(), Some(0x86)); + } + + #[test] + fn test_failure_handler_called() { + let called = Arc::new(AtomicBool::new(false)); + let called_clone = called.clone(); + + let device = create_test_device_path(); + let options = BootOptions::new().with_device(device).with_failure_handler(move || { + called_clone.store(true, Ordering::SeqCst); + }); + + assert!(!called.load(Ordering::SeqCst)); + options.handle_failure(); + assert!(called.load(Ordering::SeqCst)); + } + + #[test] + fn test_failure_handler_not_configured() { + let options = BootOptions::default(); + // Should not panic when no handler is configured + options.handle_failure(); + } + + #[test] + fn test_devices_iterator_order() { + let device1 = create_test_device_path(); + let device2 = create_test_device_path(); + let options = BootOptions::new().with_device(device1).with_device(device2); + + let devices: Vec<_> = options.devices().collect(); + assert_eq!(devices.len(), 2); + } +} diff --git a/components/patina_boot/src/helpers.rs b/components/patina_boot/src/helpers.rs new file mode 100644 index 000000000..fe22ad0a3 --- /dev/null +++ b/components/patina_boot/src/helpers.rs @@ -0,0 +1,430 @@ +//! Helper functions for boot orchestration. +//! +//! This module provides helper functions for platforms implementing custom boot flows. +//! The [`BootOrchestrator`](crate::component::BootOrchestrator) component uses these +//! internally, and platforms can use them directly for custom orchestration. +//! +//! ## License +//! +//! Copyright (c) Microsoft Corporation. +//! +//! SPDX-License-Identifier: Apache-2.0 +//! +extern crate alloc; + +use alloc::vec::Vec; +use core::ptr; + +use patina::{ + boot_services::{BootServices, event::EventType, protocol_handler::HandleSearchType, tpl::Tpl}, + error::{EfiError, Result}, + guids::EVENT_GROUP_END_OF_DXE, + runtime_services::RuntimeServices, + uefi_protocol::device_path::DevicePathBuf, +}; +use r_efi::{efi, system::EVENT_GROUP_READY_TO_BOOT}; + +/// Watchdog timeout in seconds per UEFI Specification Section 3.1.2. +const WATCHDOG_TIMEOUT_SECONDS: usize = 300; // 5 minutes + +/// Load and start a boot image with UEFI spec compliance. +/// +/// Enables a 5-minute watchdog timer before `StartImage()` per UEFI Specification +/// Section 3.1.2. Disables watchdog when boot returns control. +/// +/// # Arguments +/// +/// * `boot_services` - Boot services interface +/// * `parent_handle` - Parent image handle for the loaded image (typically the calling image's handle) +/// * `device_path` - Device path to the boot image +/// +/// # Returns +/// +/// Returns `Ok(())` if the boot image was successfully started (which typically +/// means it returned control). Returns an error if loading or starting fails. +pub fn boot_from_device_path( + boot_services: &B, + parent_handle: efi::Handle, + device_path: &DevicePathBuf, +) -> Result<()> { + // Load the image + let device_path_ptr = device_path.as_ref() as *const _ as *mut efi::protocols::device_path::Protocol; + let image_handle = match boot_services.load_image(true, parent_handle, device_path_ptr, None) { + Ok(handle) => handle, + Err(status) => { + log::error!("LoadImage failed with status: {:?}", status); + return Err(EfiError::from(status)); + } + }; + + // Enable 5-minute watchdog timer per UEFI spec Section 3.1.2 + boot_services.set_watchdog_timer(WATCHDOG_TIMEOUT_SECONDS).map_err(EfiError::from)?; + + // Start the image + let result = boot_services.start_image(image_handle); + + // Disable watchdog timer when boot option returns control + let _ = boot_services.set_watchdog_timer(0); + + match result { + Ok(()) => Ok(()), + Err((status, _exit_data)) => Err(EfiError::from(status)), + } +} + +/// Connect all controllers recursively for device enumeration. +/// +/// Connects all handles in the system recursively until the device topology +/// stabilizes (no new handles are created). +/// +/// # Arguments +/// +/// * `boot_services` - Boot services interface +/// +/// # Returns +/// +/// Returns `Ok(())` when device topology enumeration is complete. +pub fn connect_all(boot_services: &B) -> Result<()> { + // Loop until the number of handles stabilizes, indicating device topology is complete. + // This is needed because connecting a PCI bus creates new handles for PCI devices, + // which then need to be connected to bind drivers like NVMe, which creates namespace + // handles, etc. + const MAX_ITERATIONS: usize = 10; + let mut prev_handle_count = 0; + + for _iteration in 0..MAX_ITERATIONS { + // Get all handles in the system + let handles = boot_services.locate_handle_buffer(HandleSearchType::AllHandle).map_err(EfiError::from)?; + let current_handle_count = handles.len(); + + // Connect each handle recursively + for &handle in handles.iter() { + // SAFETY: Empty driver handle list and null device path are valid per UEFI spec + let _ = unsafe { boot_services.connect_controller(handle, Vec::new(), ptr::null_mut(), true) }; + } + + // Check if handle count has stabilized + if current_handle_count == prev_handle_count { + break; + } + + prev_handle_count = current_handle_count; + } + + Ok(()) +} + +/// Signal EndOfDxe event for platforms implementing custom orchestration. +/// +/// Signals `gEfiEndOfDxeEventGroupGuid` to notify security components that +/// DXE phase initialization is complete. Security components (e.g., SMM/MM) +/// register for this event and perform lockdown. +/// +/// # Arguments +/// +/// * `boot_services` - Boot services interface +pub fn signal_bds_phase_entry(boot_services: &B) -> Result<()> { + // Create and signal EndOfDxe event + // SAFETY: Null context is valid for signal-only events + let event = unsafe { + boot_services.create_event_ex_unchecked::<()>( + EventType::NOTIFY_SIGNAL, + Tpl::CALLBACK, + signal_event_noop, + ptr::null_mut(), + &EVENT_GROUP_END_OF_DXE, + ) + } + .map_err(EfiError::from)?; + + let signal_result = boot_services.signal_event(event); + // Always close the event, even if signal failed + let close_result = boot_services.close_event(event); + + signal_result.map_err(EfiError::from)?; + close_result.map_err(EfiError::from)?; + + Ok(()) +} + +/// Signal ReadyToBoot event for platforms implementing custom orchestration. +/// +/// Signals `gEfiEventReadyToBootGuid` immediately before attempting the first +/// boot option. This event notifies drivers that boot is imminent. +/// +/// # Arguments +/// +/// * `boot_services` - Boot services interface +pub fn signal_ready_to_boot(boot_services: &B) -> Result<()> { + // Create and signal ReadyToBoot event + // SAFETY: Null context is valid for signal-only events + let event = unsafe { + boot_services.create_event_ex_unchecked::<()>( + EventType::NOTIFY_SIGNAL, + Tpl::CALLBACK, + signal_event_noop, + ptr::null_mut(), + &EVENT_GROUP_READY_TO_BOOT, + ) + } + .map_err(EfiError::from)?; + + let signal_result = boot_services.signal_event(event); + // Always close the event, even if signal failed + let close_result = boot_services.close_event(event); + + signal_result.map_err(EfiError::from)?; + close_result.map_err(EfiError::from)?; + + Ok(()) +} + +/// Discover console devices (stub implementation). +/// +/// This is a placeholder that locates GOP and SimpleTextInput handles but does +/// not yet write the `ConIn`, `ConOut`, and `ErrOut` UEFI variables. +/// +/// Platforms requiring full console variable support should implement this +/// functionality or use platform-specific console initialization. +/// +/// # Arguments +/// +/// * `boot_services` - Boot services interface +/// * `runtime_services` - Runtime services interface (unused in stub) +#[allow(unused_variables)] +pub fn discover_console_devices( + boot_services: &B, + runtime_services: &R, +) -> Result<()> { + // Stub: Locate handles to verify protocols exist, but don't write variables. + // Full implementation would create multi-instance device paths and write + // ConIn/ConOut/ErrOut variables via runtime_services.set_variable(). + let _gop_handles = boot_services + .locate_handle_buffer(HandleSearchType::ByProtocol(&efi::protocols::graphics_output::PROTOCOL_GUID)) + .ok(); + + let _input_handles = boot_services + .locate_handle_buffer(HandleSearchType::ByProtocol(&efi::protocols::simple_text_input::PROTOCOL_GUID)) + .ok(); + + Ok(()) +} + +/// No-op event callback for signal-only events. +extern "efiapi" fn signal_event_noop(_event: *mut core::ffi::c_void, _context: *mut ()) {} + +#[cfg(test)] +mod tests { + extern crate alloc; + extern crate std; + + use super::*; + use core::sync::atomic::{AtomicUsize, Ordering}; + use patina::{boot_services::MockBootServices, uefi_protocol::device_path::nodes::EndEntire}; + + fn create_test_device_path() -> DevicePathBuf { + DevicePathBuf::from_device_path_node_iter(core::iter::once(EndEntire)) + } + + fn dummy_parent_handle() -> efi::Handle { + std::ptr::dangling_mut::() + } + + #[test] + fn test_boot_from_device_path_success() { + let device_path = create_test_device_path(); + let mut mock = MockBootServices::new(); + + // Expect load_image to succeed + mock.expect_load_image().returning(|_, _, _, _| Ok(core::ptr::null_mut())); + + // Expect watchdog to be set to 5 minutes + mock.expect_set_watchdog_timer().withf(|timeout| *timeout == WATCHDOG_TIMEOUT_SECONDS).returning(|_| Ok(())); + + // Expect start_image to succeed (return Ok) + mock.expect_start_image().returning(|_| Ok(())); + + // Expect watchdog to be disabled after boot returns + mock.expect_set_watchdog_timer().withf(|timeout| *timeout == 0).returning(|_| Ok(())); + + let result = boot_from_device_path(&mock, dummy_parent_handle(), &device_path); + assert!(result.is_ok()); + } + + #[test] + fn test_boot_from_device_path_load_failure() { + let device_path = create_test_device_path(); + let mut mock = MockBootServices::new(); + + // Expect load_image to fail + mock.expect_load_image().returning(|_, _, _, _| Err(efi::Status::NOT_FOUND)); + + let result = boot_from_device_path(&mock, dummy_parent_handle(), &device_path); + assert!(result.is_err()); + } + + #[test] + fn test_boot_from_device_path_start_failure() { + let device_path = create_test_device_path(); + let mut mock = MockBootServices::new(); + + // Expect load_image to succeed + mock.expect_load_image().returning(|_, _, _, _| Ok(core::ptr::null_mut())); + + // Expect watchdog to be set + mock.expect_set_watchdog_timer().returning(|_| Ok(())); + + // Expect start_image to fail + mock.expect_start_image().returning(|_| Err((efi::Status::LOAD_ERROR, None))); + + // Expect watchdog to be disabled even on failure + mock.expect_set_watchdog_timer().returning(|_| Ok(())); + + let result = boot_from_device_path(&mock, dummy_parent_handle(), &device_path); + assert!(result.is_err()); + } + + #[test] + fn test_boot_from_device_path_watchdog_disabled_on_failure() { + let device_path = create_test_device_path(); + let mut mock = MockBootServices::new(); + + static WATCHDOG_DISABLE_CALLED: AtomicUsize = AtomicUsize::new(0); + + mock.expect_load_image().returning(|_, _, _, _| Ok(core::ptr::null_mut())); + + mock.expect_set_watchdog_timer().returning(|timeout| { + if timeout == 0 { + WATCHDOG_DISABLE_CALLED.fetch_add(1, Ordering::SeqCst); + } + Ok(()) + }); + + mock.expect_start_image().returning(|_| Err((efi::Status::ABORTED, None))); + + let _ = boot_from_device_path(&mock, dummy_parent_handle(), &device_path); + + // Verify watchdog was disabled (timeout=0 was called) + assert!(WATCHDOG_DISABLE_CALLED.load(Ordering::SeqCst) >= 1); + } + + #[test] + fn test_signal_bds_phase_entry_signals_end_of_dxe() { + let mut mock = MockBootServices::new(); + + // Expect event creation with proper type annotation + mock.expect_create_event_ex_unchecked::<()>().returning(|_, _, _, _, _| Ok(core::ptr::null_mut())); + + // Expect event to be signaled + mock.expect_signal_event().returning(|_| Ok(())); + + // Expect event to be closed + mock.expect_close_event().returning(|_| Ok(())); + + let result = signal_bds_phase_entry(&mock); + assert!(result.is_ok()); + } + + #[test] + fn test_signal_ready_to_boot() { + let mut mock = MockBootServices::new(); + + mock.expect_create_event_ex_unchecked::<()>().returning(|_, _, _, _, _| Ok(core::ptr::null_mut())); + mock.expect_signal_event().returning(|_| Ok(())); + mock.expect_close_event().returning(|_| Ok(())); + + let result = signal_ready_to_boot(&mock); + assert!(result.is_ok()); + } + + #[test] + fn test_connect_all_locate_failure() { + let mut mock = MockBootServices::new(); + + // locate_handle_buffer fails on first call + mock.expect_locate_handle_buffer().returning(|_| Err(efi::Status::NOT_FOUND)); + + let result = connect_all(&mock); + assert!(result.is_err()); + } + + #[test] + fn test_discover_console_devices_handles_missing_protocols() { + use patina::runtime_services::MockRuntimeServices; + + let mut boot_mock = MockBootServices::new(); + let runtime_mock = MockRuntimeServices::new(); + + // Protocols not found - returns error but function should still succeed + boot_mock.expect_locate_handle_buffer().returning(|_| Err(efi::Status::NOT_FOUND)); + + // Function should still succeed even with no console devices + let result = discover_console_devices(&boot_mock, &runtime_mock); + assert!(result.is_ok()); + } + + #[test] + fn test_signal_bds_phase_entry_create_event_failure() { + let mut mock = MockBootServices::new(); + + // Event creation fails + mock.expect_create_event_ex_unchecked::<()>().returning(|_, _, _, _, _| Err(efi::Status::OUT_OF_RESOURCES)); + + let result = signal_bds_phase_entry(&mock); + assert!(result.is_err()); + } + + #[test] + fn test_signal_bds_phase_entry_signal_failure() { + let mut mock = MockBootServices::new(); + + mock.expect_create_event_ex_unchecked::<()>().returning(|_, _, _, _, _| Ok(core::ptr::null_mut())); + + // Signal fails + mock.expect_signal_event().returning(|_| Err(efi::Status::INVALID_PARAMETER)); + + // close_event is always called, even on signal failure + mock.expect_close_event().returning(|_| Ok(())); + + let result = signal_bds_phase_entry(&mock); + assert!(result.is_err()); + } + + #[test] + fn test_signal_bds_phase_entry_close_event_failure() { + let mut mock = MockBootServices::new(); + + mock.expect_create_event_ex_unchecked::<()>().returning(|_, _, _, _, _| Ok(core::ptr::null_mut())); + mock.expect_signal_event().returning(|_| Ok(())); + + // Close fails + mock.expect_close_event().returning(|_| Err(efi::Status::INVALID_PARAMETER)); + + let result = signal_bds_phase_entry(&mock); + assert!(result.is_err()); + } + + #[test] + fn test_signal_ready_to_boot_create_event_failure() { + let mut mock = MockBootServices::new(); + + mock.expect_create_event_ex_unchecked::<()>().returning(|_, _, _, _, _| Err(efi::Status::OUT_OF_RESOURCES)); + + let result = signal_ready_to_boot(&mock); + assert!(result.is_err()); + } + + #[test] + fn test_boot_from_device_path_watchdog_set_failure() { + let device_path = create_test_device_path(); + let mut mock = MockBootServices::new(); + + mock.expect_load_image().returning(|_, _, _, _| Ok(core::ptr::null_mut())); + + // Watchdog set fails + mock.expect_set_watchdog_timer().returning(|_| Err(efi::Status::DEVICE_ERROR)); + + let result = boot_from_device_path(&mock, dummy_parent_handle(), &device_path); + assert!(result.is_err()); + } +} diff --git a/components/patina_boot/src/lib.rs b/components/patina_boot/src/lib.rs new file mode 100644 index 000000000..d60bb0d06 --- /dev/null +++ b/components/patina_boot/src/lib.rs @@ -0,0 +1,30 @@ +//! Boot Orchestration Components +//! +//! This crate provides boot orchestration components for Patina firmware, implementing +//! UEFI Specification 2.11 Chapter 3 (Boot Manager) and PI Specification BDS phase requirements. +//! +//! ## Components +//! +//! - [`component::ConsoleDiscovery`]: Discovers console devices and populates ConIn/ConOut/ErrOut variables +//! - [`component::BootOrchestrator`]: Orchestrates the boot flow with device enumeration, event signaling, and boot execution +//! +//! ## Configuration +//! +//! - [`config::BootOptions`]: Platform-provided boot options as device paths +//! +//! ## Helper Functions +//! +//! The [`helpers`] module provides helper functions for platforms implementing custom boot flows. +//! +//! ## License +//! +//! Copyright (c) Microsoft Corporation. +//! +//! SPDX-License-Identifier: Apache-2.0 +//! +#![cfg_attr(not(feature = "std"), no_std)] +#![feature(coverage_attribute)] + +pub mod component; +pub mod config; +pub mod helpers; diff --git a/cspell.yml b/cspell.yml index 227f81fa9..83d6ac0e6 100644 --- a/cspell.yml +++ b/cspell.yml @@ -41,6 +41,7 @@ words: - armasm - autocfg - bitvec + - bootaa - bootx - bucketize - bumpalo diff --git a/patina_dxe_core/src/component_dispatcher.rs b/patina_dxe_core/src/component_dispatcher.rs index 4da367f02..33dd9693f 100644 --- a/patina_dxe_core/src/component_dispatcher.rs +++ b/patina_dxe_core/src/component_dispatcher.rs @@ -123,7 +123,7 @@ impl ComponentDispatcher { /// Creates a new locked ComponentDispatcher. /// /// Uses TPL_APPLICATION so that component entry points can use boot services - /// that are restricted at higher TPL levels. + /// that are restricted at higher TPL levels (per UEFI spec Section 7.1). #[inline(always)] pub(crate) const fn new_locked() -> TplMutex { TplMutex::new(efi::TPL_APPLICATION, Self::new(), "ComponentDispatcher") diff --git a/sdk/patina/src/component/struct_component.rs b/sdk/patina/src/component/struct_component.rs index 537806625..473fd6f51 100644 --- a/sdk/patina/src/component/struct_component.rs +++ b/sdk/patina/src/component/struct_component.rs @@ -188,14 +188,21 @@ mod tests { fn test_component_run_handling_works_as_expected() { let mut storage = crate::component::storage::Storage::new(); + // Test component with Config - requires locked configs let mut test_struct = TestStructSuccess { x: 5 }.into_component(); test_struct.initialize(&mut storage); + storage.lock_configs(); // Lock configs so Config is accessible assert!(test_struct.run(&mut storage).is_ok_and(|res| res)); let mut test_enum = TestEnumSuccess::A.into_component(); test_enum.initialize(&mut storage); assert!(test_enum.run(&mut storage).is_ok_and(|res| res)); + // Unlock configs for the next test + let config_id = storage.register_config::(); + storage.unlock_config(config_id); + + // Test component with ConfigMut - configs are now locked, so it should fail let mut test_struct = TestStructNotDispatched { x: 5 }.into_component(); test_struct.initialize(&mut storage); storage.lock_configs(); // Lock it so the ConfigMut can't be accessed From 81ebf38abcd584c3280816648b2ec29ff1c79072 Mon Sep 17 00:00:00 2001 From: Kat Perez Date: Mon, 2 Feb 2026 11:32:24 -0500 Subject: [PATCH 2/9] [feature/patina-boot] patina_boot: Add hotkey detection and alternate boot options (#1272) ## Description Add hotkey detection support to the boot orchestrator, allowing platforms to configure alternate boot options that are used when a hotkey (e.g., F12) is pressed during boot. Changes: - Add `detect_hotkey()` helper function to check for hotkey press via SimpleTextInput protocol - Add `hotkey_devices` field and `with_hotkey_device()` builder to `BootOptions` - Update `BootOrchestrator` to use alternate boot options when hotkey is detected --- - [x] Impacts functionality? - [ ] Impacts security? - [ ] Breaking change? - [x] Includes tests? - [ ] Includes documentation? ## How This Was Tested - Unit tests for `detect_hotkey()` (no input handles case) - Unit tests for `hotkey_devices` config (single device, multiple devices, combined with hotkey) - `cargo test -p patina_boot` passes ## Integration Instructions Platforms can configure hotkey boot options: ```rust BootOptions::new() .with_device(primary_device) .with_hotkey(0x16) // F12 scancode .with_hotkey_device(alternate_device) ``` Closes #1228 --- .../src/component/boot_orchestrator.rs | 51 +++++----- components/patina_boot/src/config.rs | 68 +++++++++++++- components/patina_boot/src/helpers.rs | 93 ++++++++++++++++++- 3 files changed, 184 insertions(+), 28 deletions(-) diff --git a/components/patina_boot/src/component/boot_orchestrator.rs b/components/patina_boot/src/component/boot_orchestrator.rs index 182120db8..88db2c8e4 100644 --- a/components/patina_boot/src/component/boot_orchestrator.rs +++ b/components/patina_boot/src/component/boot_orchestrator.rs @@ -15,8 +15,10 @@ //! //! SPDX-License-Identifier: Apache-2.0 //! +extern crate alloc; + use patina::{ - boot_services::{BootServices, StandardBootServices, protocol_handler::HandleSearchType}, + boot_services::StandardBootServices, component::{ component, params::{Config, Handle}, @@ -24,7 +26,6 @@ use patina::{ error::{EfiError, Result}, runtime_services::StandardRuntimeServices, }; -use r_efi::efi; use crate::{config::BootOptions, helpers}; @@ -47,9 +48,10 @@ impl BootOrchestrator { /// 1. Connect all controllers for device enumeration /// 2. Signal EndOfDxe (security components perform lockdown) /// 3. Discover consoles - /// 4. Signal ReadyToBoot - /// 5. Execute boot options from config - /// 6. If all boot options fail, call failure handler + /// 4. Check for hotkey press; if detected, use alternate boot options + /// 5. Signal ReadyToBoot + /// 6. Execute boot options from config (or hotkey_devices if hotkey detected) + /// 7. If all boot options fail, call failure handler #[coverage(off)] // Component integration - tested via integration tests fn entry_point( self, @@ -61,29 +63,34 @@ impl BootOrchestrator { helpers::connect_all(boot_services.as_ref())?; helpers::signal_bds_phase_entry(boot_services.as_ref())?; helpers::discover_console_devices(boot_services.as_ref(), runtime_services.as_ref())?; + + // Check for hotkey press after devices are connected and consoles discovered + let use_hotkey_devices = if let Some(hotkey) = boot_options.hotkey() { + helpers::detect_hotkey(boot_services.as_ref(), hotkey) + } else { + false + }; + helpers::signal_ready_to_boot(boot_services.as_ref())?; - // Use the provided image handle, or fall back to finding one via LocateHandleBuffer. // Per UEFI spec, the parent handle must be a valid image handle (has LoadedImage protocol). - let parent_handle = if let Some(handle) = image_handle { - *handle - } else { - log::warn!("Handle not provided, falling back to LocateHandleBuffer"); - let image_handles = boot_services - .locate_handle_buffer(HandleSearchType::ByProtocol(&efi::protocols::loaded_image::PROTOCOL_GUID)) - .map_err(|status| { - log::error!("Failed to locate image handles: {:?}", status); - EfiError::from(status) - })?; + // The Handle must be provided by the component framework - we cannot safely guess + // which handle is correct from the handle database as ordering is not guaranteed. + let parent_handle = image_handle.ok_or_else(|| { + log::error!("Handle not provided - required for LoadImage parent handle"); + EfiError::InvalidParameter + })?; - image_handles.first().copied().ok_or_else(|| { - log::error!("No image handles available for parent handle"); - EfiError::NotReady - })? + // Select boot devices based on hotkey detection + let boot_devices: alloc::vec::Vec<_> = if use_hotkey_devices { + log::info!("Using alternate boot options (hotkey detected)"); + boot_options.hotkey_devices().collect() + } else { + boot_options.devices().collect() }; - for device_path in boot_options.devices() { - match helpers::boot_from_device_path(boot_services.as_ref(), parent_handle, device_path) { + for device_path in boot_devices { + match helpers::boot_from_device_path(boot_services.as_ref(), *parent_handle, device_path) { Ok(()) => { // Boot image returned control (e.g., EFI application exited). // Continue to try next boot option. diff --git a/components/patina_boot/src/config.rs b/components/patina_boot/src/config.rs index f0ee2fe49..cd23c4304 100644 --- a/components/patina_boot/src/config.rs +++ b/components/patina_boot/src/config.rs @@ -27,6 +27,8 @@ use patina::uefi_protocol::device_path::DevicePathBuf; /// let options = BootOptions::new() /// .with_device(nvme_device_path) /// .with_device(usb_device_path) +/// .with_hotkey(0x16) // F12 (UEFI scan code) +/// .with_hotkey_device(usb_device_path) // Boot from USB when F12 pressed /// .with_failure_handler(|| show_error_screen()); /// ``` #[derive(Default)] @@ -35,6 +37,8 @@ pub struct BootOptions { devices: Vec, /// Optional hotkey for boot override (e.g., F12 for boot menu). hotkey: Option, + /// Alternate boot device paths used when hotkey is detected. + hotkey_devices: Vec, /// Handler called when all boot options fail. failure_handler: Option>, } @@ -53,15 +57,29 @@ impl BootOptions { self } - /// Add a hotkey scancode for boot override (not yet implemented). + /// Add a hotkey scancode for boot override. /// - /// This field is reserved for future boot menu functionality. - /// Currently, the hotkey is stored but not acted upon. + /// When this hotkey is detected during boot, the orchestrator will use + /// the alternate boot options configured via [`with_hotkey_device`](Self::with_hotkey_device) + /// instead of the primary boot devices. + /// + /// Note: Hotkey detection reads and consumes all pending keystrokes from the + /// keyboard buffer. Any keys pressed before detection will not be available + /// to subsequent code. pub fn with_hotkey(mut self, scancode: u16) -> Self { self.hotkey = Some(scancode); self } + /// Add an alternate boot device path used when the hotkey is detected. + /// + /// Hotkey devices are tried in the order they are added, but only when + /// the configured hotkey is detected during boot. + pub fn with_hotkey_device(mut self, device: DevicePathBuf) -> Self { + self.hotkey_devices.push(device); + self + } + /// Add a failure handler called when all boot options fail. pub fn with_failure_handler(mut self, handler: F) -> Self where @@ -81,6 +99,11 @@ impl BootOptions { self.devices.iter() } + /// Returns an iterator over alternate boot device paths used when hotkey is detected. + pub fn hotkey_devices(&self) -> impl Iterator { + self.hotkey_devices.iter() + } + /// Call the failure handler if configured. /// /// This is called when all boot options have been exhausted. @@ -136,8 +159,43 @@ mod tests { #[test] fn test_with_hotkey() { - let options = BootOptions::new().with_hotkey(0x86); // F12 - assert_eq!(options.hotkey(), Some(0x86)); + let options = BootOptions::new().with_hotkey(0x16); // F12 + assert_eq!(options.hotkey(), Some(0x16)); + } + + #[test] + fn test_with_hotkey_device() { + let device = create_test_device_path(); + let options = BootOptions::new().with_hotkey_device(device); + assert_eq!(options.hotkey_devices().count(), 1); + } + + #[test] + fn test_with_multiple_hotkey_devices() { + let device1 = create_test_device_path(); + let device2 = create_test_device_path(); + let options = BootOptions::new().with_hotkey_device(device1).with_hotkey_device(device2); + assert_eq!(options.hotkey_devices().count(), 2); + } + + #[test] + fn test_hotkey_with_hotkey_devices() { + let primary = create_test_device_path(); + let alternate = create_test_device_path(); + let options = BootOptions::new() + .with_device(primary) + .with_hotkey(0x16) // F12 + .with_hotkey_device(alternate); + + assert_eq!(options.devices().count(), 1); + assert_eq!(options.hotkey(), Some(0x16)); + assert_eq!(options.hotkey_devices().count(), 1); + } + + #[test] + fn test_default_has_no_hotkey_devices() { + let options = BootOptions::default(); + assert_eq!(options.hotkey_devices().count(), 0); } #[test] diff --git a/components/patina_boot/src/helpers.rs b/components/patina_boot/src/helpers.rs index fe22ad0a3..bad7c30c6 100644 --- a/components/patina_boot/src/helpers.rs +++ b/components/patina_boot/src/helpers.rs @@ -22,11 +22,83 @@ use patina::{ runtime_services::RuntimeServices, uefi_protocol::device_path::DevicePathBuf, }; -use r_efi::{efi, system::EVENT_GROUP_READY_TO_BOOT}; +use r_efi::{efi, protocols::simple_text_input, system::EVENT_GROUP_READY_TO_BOOT}; /// Watchdog timeout in seconds per UEFI Specification Section 3.1.2. const WATCHDOG_TIMEOUT_SECONDS: usize = 300; // 5 minutes +/// Check if a hotkey was pressed during boot. +/// +/// Reads any pending keystrokes from all SimpleTextInput protocol instances +/// and returns `true` if any key matches the specified scancode. +/// +/// This is a non-blocking check that consumes any buffered keystrokes. +/// +/// # Arguments +/// +/// * `boot_services` - Boot services interface +/// * `hotkey_scancode` - The scancode to check for (e.g., 0x16 for F12) +/// +/// # Returns +/// +/// Returns `true` if the hotkey was detected, `false` otherwise. +pub fn detect_hotkey(boot_services: &B, hotkey_scancode: u16) -> bool { + // Locate all SimpleTextInput handles + let handles = + match boot_services.locate_handle_buffer(HandleSearchType::ByProtocol(&simple_text_input::PROTOCOL_GUID)) { + Ok(handles) => handles, + Err(_) => return false, + }; + + // SAFETY: Handles are valid from locate_handle_buffer, protocol_ptr is valid from handle_protocol + unsafe { detect_hotkey_from_handles(boot_services, &handles, hotkey_scancode) } +} + +/// Inner hotkey detection loop over handles. +/// +/// This function is separated from `detect_hotkey` because it uses raw protocol +/// function pointers that cannot be unit tested with mocks. Integration tests +/// verify this code path on real hardware/emulators. +/// +/// # Safety +/// +/// - `handles` must contain valid handles obtained from `locate_handle_buffer` +/// - Each handle must support the `SimpleTextInput` protocol for `handle_protocol` to succeed +#[coverage(off)] // Uses raw protocol function pointers - tested via integration tests +unsafe fn detect_hotkey_from_handles( + boot_services: &B, + handles: &[efi::Handle], + hotkey_scancode: u16, +) -> bool { + for &handle in handles.iter() { + // Get the protocol interface for this handle + // SAFETY: handle is valid per function contract (from locate_handle_buffer) + let protocol_ptr = match unsafe { boot_services.handle_protocol::(handle) } { + Ok(ptr) => ptr, + Err(_) => continue, + }; + + // Read any pending keystrokes (non-blocking) + // The protocol will return NOT_READY if no key is available + loop { + let mut key = simple_text_input::InputKey::default(); + let status = (protocol_ptr.read_key_stroke)(protocol_ptr, &mut key); + + if status == efi::Status::SUCCESS { + if key.scan_code == hotkey_scancode { + return true; + } + // Key didn't match, continue reading to drain buffer + } else { + // NOT_READY or error - no more keys in buffer + break; + } + } + } + + false +} + /// Load and start a boot image with UEFI spec compliance. /// /// Enables a 5-minute watchdog timer before `StartImage()` per UEFI Specification @@ -84,6 +156,13 @@ pub fn boot_from_device_path( /// # Returns /// /// Returns `Ok(())` when device topology enumeration is complete. +/// +/// # Coverage +/// +/// This function is marked `#[coverage(off)]` because `BootServicesBox` return +/// values from `locate_handle_buffer` cannot be created in unit tests. The +/// function is tested via integration tests on real hardware/emulators. +#[coverage(off)] // BootServicesBox return type cannot be mocked - tested via integration tests pub fn connect_all(boot_services: &B) -> Result<()> { // Loop until the number of handles stabilizes, indicating device topology is complete. // This is needed because connecting a PCI bus creates new handles for PCI devices, @@ -211,6 +290,7 @@ pub fn discover_console_devices( } /// No-op event callback for signal-only events. +#[coverage(off)] // Extern callback - tested via integration tests extern "efiapi" fn signal_event_noop(_event: *mut core::ffi::c_void, _context: *mut ()) {} #[cfg(test)] @@ -427,4 +507,15 @@ mod tests { let result = boot_from_device_path(&mock, dummy_parent_handle(), &device_path); assert!(result.is_err()); } + + #[test] + fn test_detect_hotkey_no_input_handles() { + let mut mock = MockBootServices::new(); + + // No SimpleTextInput handles found + mock.expect_locate_handle_buffer().returning(|_| Err(efi::Status::NOT_FOUND)); + + let result = detect_hotkey(&mock, 0x16); // F12 + assert!(!result); + } } From 2d93ab56e705e851834a36f66c8677d03132ce1b Mon Sep 17 00:00:00 2001 From: Kat Perez Date: Thu, 5 Feb 2026 17:14:18 -0500 Subject: [PATCH 3/9] [feature/patina-boot] patina_dxe_core: Fix TPL violation in initialize_system_table (#1284) Release SYSTEM_TABLE lock (TPL_NOTIFY) before accessing ComponentDispatcher (TPL_APPLICATION) to avoid TPL violation. PR #1225 lowered ComponentDispatcher from TPL_NOTIFY to TPL_APPLICATION to allow components to use boot services, but this created a conflict when initialize_system_table held SYSTEM_TABLE while setting boot/runtime services on ComponentDispatcher. Fix: Extract boot/runtime services pointers while holding SYSTEM_TABLE lock, then release it before accessing ComponentDispatcher. - [x] Impacts functionality? - [ ] Impacts security? - [ ] Breaking change? - [ ] Includes tests? - [ ] Includes documentation? - Unit tests pass - QEMU Q35 boots without TPL violation panic N/A --- patina_dxe_core/src/lib.rs | 75 ++++++++++++++++++++------------------ 1 file changed, 39 insertions(+), 36 deletions(-) diff --git a/patina_dxe_core/src/lib.rs b/patina_dxe_core/src/lib.rs index 848060b04..7ffbd2eda 100644 --- a/patina_dxe_core/src/lib.rs +++ b/patina_dxe_core/src/lib.rs @@ -514,42 +514,45 @@ impl Core

{ // Instantiate system table. systemtables::init_system_table(); - let mut st_guard = systemtables::SYSTEM_TABLE.lock(); - let st = st_guard.as_mut().expect("System Table not initialized!"); - - allocator::install_memory_services(st); - gcd::init_paging(self.hob_list()); - events::init_events_support(st); - protocols::init_protocol_support(st); - misc_boot_services::init_misc_boot_services_support(st); - config_tables::init_config_tables_support(st); - runtime::init_runtime_support(); - self.pi_dispatcher.init(self.hob_list(), st); - self.install_dxe_services_table(st); - driver_services::init_driver_services(st); - - memory_attributes_protocol::install_memory_attributes_protocol(); - - // re-checksum the system tables after above initialization. - st.checksum_all(); - - // Install HobList configuration table - config_tables::core_install_configuration_table(patina::guids::HOB_LIST.into_inner(), physical_hob_list, st) - .expect("Unable to create configuration table due to invalid table entry."); - - // Install Memory Type Info configuration table. - allocator::install_memory_type_info_table(st).expect("Unable to create Memory Type Info Table"); - - memory_attributes_table::init_memory_attributes_table_support(); - - // The component dispatcher has a TPL_APPLICATION TPLMutex, so we need to drop the TPL_NOTIFY st_guard before - // attempting to unlock the component dispatcher to prevent TPL inversion - let boot_services = StandardBootServices::new(st.boot_services().as_mut_ptr()); - let runtime_services = StandardRuntimeServices::new(st.runtime_services().as_mut_ptr()); - drop(st_guard); - - self.component_dispatcher.lock().set_boot_services(boot_services); - self.component_dispatcher.lock().set_runtime_services(runtime_services); + // Extract boot/runtime services pointers while holding SYSTEM_TABLE lock (TPL_NOTIFY). + // We must release this lock before accessing component_dispatcher (TPL_APPLICATION) + // to avoid TPL violation - you cannot lower TPL while holding a higher TPL lock. + let (boot_services_ptr, runtime_services_ptr) = { + let mut st = systemtables::SYSTEM_TABLE.lock(); + let st = st.as_mut().expect("System Table not initialized!"); + + allocator::install_memory_services(st); + gcd::init_paging(self.hob_list()); + events::init_events_support(st); + protocols::init_protocol_support(st); + misc_boot_services::init_misc_boot_services_support(st); + config_tables::init_config_tables_support(st); + runtime::init_runtime_support(); + self.pi_dispatcher.init(self.hob_list(), st); + self.install_dxe_services_table(st); + driver_services::init_driver_services(st); + + memory_attributes_protocol::install_memory_attributes_protocol(); + + // re-checksum the system tables after above initialization. + st.checksum_all(); + + // Install HobList configuration table + config_tables::core_install_configuration_table(patina::guids::HOB_LIST.into_inner(), physical_hob_list, st) + .expect("Unable to create configuration table due to invalid table entry."); + + // Install Memory Type Info configuration table. + allocator::install_memory_type_info_table(st).expect("Unable to create Memory Type Info Table"); + + memory_attributes_table::init_memory_attributes_table_support(); + + // Extract pointers before releasing the lock + (st.boot_services().as_mut_ptr(), st.runtime_services().as_mut_ptr()) + }; // SYSTEM_TABLE lock released here + + // Now safe to access component_dispatcher at TPL_APPLICATION + self.component_dispatcher.lock().set_boot_services(StandardBootServices::new(boot_services_ptr)); + self.component_dispatcher.lock().set_runtime_services(StandardRuntimeServices::new(runtime_services_ptr)); self.component_dispatcher.lock().set_image_handle(protocol_db::DXE_CORE_HANDLE); Ok(()) From b23078e388bbf022e3176ec23198adfb564f1718 Mon Sep 17 00:00:00 2001 From: Kat Perez Date: Thu, 12 Feb 2026 15:05:35 -0500 Subject: [PATCH 4/9] [feature/patina-boot] patina_boot: Support partial device path expansion (#1290) Add support for expanding partial (short-form) device paths to full device paths by matching against the device topology. - Add `is_partial_device_path()` to detect partial paths (start with Media/Messaging nodes instead of Hardware/ACPI) - Add `expand_device_path()` to find matching full paths by enumerating device handles - Wire expansion into `boot_from_device_path()` for transparent handling - Currently supports HardDrive nodes with GPT partition signature matching This enables booting from Boot#### variables containing partial device paths like `HD(1,GPT,)\EFI\BOOT\BOOTX64.EFI`. - [x] Impacts functionality? - [ ] Impacts security? - [ ] Breaking change? - [x] Includes tests? - [ ] Includes documentation? - Unit tests for `is_partial_device_path()`, `expand_device_path()`, and signature matching - `cargo test -p patina_boot` passes (32 tests) - QEMU Q35 platform test passes N/A Closes #1280 --- components/patina_boot/src/helpers.rs | 271 +++++++++++++++++++- patina_dxe_core/src/component_dispatcher.rs | 1 - 2 files changed, 267 insertions(+), 5 deletions(-) diff --git a/components/patina_boot/src/helpers.rs b/components/patina_boot/src/helpers.rs index bad7c30c6..ffedd299b 100644 --- a/components/patina_boot/src/helpers.rs +++ b/components/patina_boot/src/helpers.rs @@ -20,7 +20,7 @@ use patina::{ error::{EfiError, Result}, guids::EVENT_GROUP_END_OF_DXE, runtime_services::RuntimeServices, - uefi_protocol::device_path::DevicePathBuf, + uefi_protocol::device_path::{DevicePath, DevicePathBuf, nodes::DevicePathType}, }; use r_efi::{efi, protocols::simple_text_input, system::EVENT_GROUP_READY_TO_BOOT}; @@ -119,8 +119,15 @@ pub fn boot_from_device_path( parent_handle: efi::Handle, device_path: &DevicePathBuf, ) -> Result<()> { + // Expand partial device paths to full paths + let full_path = if is_partial_device_path(device_path.as_ref()) { + expand_device_path(boot_services, device_path.as_ref())? + } else { + device_path.clone() + }; + // Load the image - let device_path_ptr = device_path.as_ref() as *const _ as *mut efi::protocols::device_path::Protocol; + let device_path_ptr = full_path.as_ref() as *const _ as *mut efi::protocols::device_path::Protocol; let image_handle = match boot_services.load_image(true, parent_handle, device_path_ptr, None) { Ok(handle) => handle, Err(status) => { @@ -293,6 +300,106 @@ pub fn discover_console_devices( #[coverage(off)] // Extern callback - tested via integration tests extern "efiapi" fn signal_event_noop(_event: *mut core::ffi::c_void, _context: *mut ()) {} +/// Returns true if the device path is a partial (short-form) device path. +/// +/// Full device paths start with Hardware (type 1) or ACPI (type 2) root nodes, +/// representing the complete path from system root to device. +/// +/// Partial device paths start with other node types (e.g., Media type 4 for HD nodes, +/// Messaging type 3 for NVMe without root) and must be expanded by matching against +/// the current device topology before they can be used for booting. +/// +/// # Arguments +/// +/// * `device_path` - The device path to check +/// +/// # Returns +/// +/// `true` if the device path is partial (does not start with Hardware or ACPI node), +/// `false` if it's a full device path or empty. +pub fn is_partial_device_path(device_path: &DevicePath) -> bool { + let Some(first_node) = device_path.iter().next() else { + return false; + }; + + // Full paths start with Hardware (1) or ACPI (2) nodes + // Partial paths start with Media (4), Messaging (3), or other nodes + let node_type = first_node.header.r#type; + node_type != DevicePathType::Hardware as u8 + && node_type != DevicePathType::Acpi as u8 + && node_type != DevicePathType::End as u8 +} + +/// Expands a partial device path to a full device path by matching against device topology. +/// +/// This function takes a partial (short-form) device path and finds the corresponding +/// full device path by enumerating all device handles and matching against the partial +/// path's identifying characteristics (e.g., partition GUID for HardDrive nodes). +/// +/// If the input is already a full device path (starts with Hardware or ACPI node), +/// it is returned unchanged. +/// +/// # Arguments +/// +/// * `boot_services` - Boot services for handle enumeration +/// * `partial_path` - The device path to expand (may be full or partial) +/// +/// # Returns +/// +/// * `Ok(DevicePathBuf)` - The expanded full device path, or the original if already full +/// * `Err(EfiError::InvalidParameter)` - If the partial path is empty +/// * `Err(EfiError::NotFound)` - If no matching device was found in the topology +/// +/// # Supported Partial Path Types +/// +/// Currently supports: +/// - **HardDrive (Media type 4, subtype 1)**: Matches by partition signature and signature type +/// +/// Future enhancements may add support for: +/// - FilePath-only paths (require filesystem enumeration) +/// - Messaging node paths without root +#[coverage(off)] // Uses raw protocol pointers - tested via integration tests +pub fn expand_device_path(boot_services: &B, partial_path: &DevicePath) -> Result { + // Return unchanged if already a full path + if !is_partial_device_path(partial_path) { + return Ok(partial_path.into()); + } + + // Use LocateDevicePath to find the handle with the best matching device path. + // This is more efficient than enumerating all handles manually. + let mut device_path_ptr = + partial_path as *const DevicePath as *const u8 as *mut efi::protocols::device_path::Protocol; + // SAFETY: device_path_ptr points to a valid device path from partial_path. + let handle = + unsafe { boot_services.locate_device_path(&efi::protocols::device_path::PROTOCOL_GUID, &mut device_path_ptr) } + .map_err(EfiError::from)?; + + // Get the full device path from the matched handle + // SAFETY: handle_protocol is safe when the handle is valid (from locate_device_path) + // and we're requesting the device path protocol. + let full_dp_ptr = unsafe { boot_services.handle_protocol::(handle) } + .map_err(EfiError::from)?; + + // SAFETY: The device path pointer comes from a valid protocol interface. + let full_path = + unsafe { DevicePath::try_from_ptr(full_dp_ptr as *const _ as *const u8) }.map_err(|_| EfiError::DeviceError)?; + + // Combine the full path prefix with the remaining partial path. + // The remaining path (after the matched portion) needs to be appended. + let mut result = DevicePathBuf::from(full_path); + + // SAFETY: device_path_ptr was updated by locate_device_path to point to the remaining path. + let remaining_path = unsafe { DevicePath::try_from_ptr(device_path_ptr as *const u8) }; + if let Ok(remaining) = remaining_path { + // Only append if there's a meaningful remaining path (not just EndEntire) + if remaining.iter().any(|node| node.header.r#type != DevicePathType::End as u8) { + result.append_device_path(&DevicePathBuf::from(remaining)); + } + } + + Ok(result) +} + #[cfg(test)] mod tests { extern crate alloc; @@ -300,10 +407,14 @@ mod tests { use super::*; use core::sync::atomic::{AtomicUsize, Ordering}; - use patina::{boot_services::MockBootServices, uefi_protocol::device_path::nodes::EndEntire}; + use patina::{ + boot_services::MockBootServices, + uefi_protocol::device_path::nodes::{Acpi, EndEntire, HardDrive}, + }; fn create_test_device_path() -> DevicePathBuf { - DevicePathBuf::from_device_path_node_iter(core::iter::once(EndEntire)) + // Create a full device path (starts with ACPI node) so it won't trigger partial path expansion + DevicePathBuf::from_device_path_node_iter([Acpi::new_pci_root(0)].into_iter()) } fn dummy_parent_handle() -> efi::Handle { @@ -518,4 +629,156 @@ mod tests { let result = detect_hotkey(&mock, 0x16); // F12 assert!(!result); } + + // Tests for partial device path expansion + + use patina::uefi_protocol::device_path::nodes::Pci; + + /// Helper to build a partial device path starting with HD node. + fn build_partial_hd_path(guid: [u8; 16]) -> DevicePathBuf { + DevicePathBuf::from_device_path_node_iter([HardDrive::new_gpt(1, 2048, 1000000, guid)].into_iter()) + } + + /// Helper to build a full device path starting with ACPI root. + fn build_full_path_with_hd(guid: [u8; 16]) -> DevicePathBuf { + let mut path = DevicePathBuf::from_device_path_node_iter([Acpi::new_pci_root(0)].into_iter()); + let pci_path = DevicePathBuf::from_device_path_node_iter([Pci { function: 0, device: 0x1D }].into_iter()); + path.append_device_path(&pci_path); + let hd_path = + DevicePathBuf::from_device_path_node_iter([HardDrive::new_gpt(1, 2048, 1000000, guid)].into_iter()); + path.append_device_path(&hd_path); + path + } + + #[test] + fn test_is_partial_with_hd_node() { + let partial = build_partial_hd_path([0xAA; 16]); + assert!(is_partial_device_path(&partial)); + } + + #[test] + fn test_is_partial_with_full_path_acpi() { + let full = build_full_path_with_hd([0xAA; 16]); + assert!(!is_partial_device_path(&full)); + } + + #[test] + fn test_is_partial_empty_path() { + let empty = DevicePathBuf::from_device_path_node_iter([EndEntire].into_iter()); + // EndEntire is type 0x7F (End) - an end-only path is not a meaningful partial path + assert!(!is_partial_device_path(&empty)); + } + + #[test] + fn test_expand_already_full_returns_unchanged() { + let full = build_full_path_with_hd([0xAA; 16]); + + let mock = MockBootServices::new(); + // No mock setup needed since full paths return early + + let result = expand_device_path(&mock, &full); + assert!(result.is_ok()); + assert_eq!(result.unwrap(), full); + } + + #[test] + fn test_expand_partial_path_success() { + use alloc::boxed::Box; + use patina::uefi_protocol::device_path::nodes::FilePath; + + let guid = [0xAA; 16]; + + // Create the partial path: HD(1,GPT,)/File(\EFI\BOOT\BOOTX64.EFI) + let mut partial = build_partial_hd_path(guid); + let file_path = + DevicePathBuf::from_device_path_node_iter([FilePath::new("\\EFI\\BOOT\\BOOTX64.EFI")].into_iter()); + partial.append_device_path(&file_path); + + // Create the full path that the handle will have (ACPI/PCI/HD) + let full_handle_path = build_full_path_with_hd(guid); + + // Expected result: ACPI/PCI/HD/File (full path + remaining file path) + let mut expected = full_handle_path.clone(); + expected.append_device_path(&file_path); + + // Clone the device path bytes into a Vec and leak it so we can return a pointer + let path_ref: &DevicePath = full_handle_path.as_ref(); + // SAFETY: path_ref is a valid DevicePath reference and size() returns its exact byte length. + let bytes: alloc::vec::Vec = unsafe { + alloc::vec::Vec::from(core::slice::from_raw_parts(path_ref as *const _ as *const u8, path_ref.size())) + }; + let leaked_bytes = Box::leak(bytes.into_boxed_slice()); + let full_path_ptr: usize = leaked_bytes.as_ptr() as usize; + + // Create a fake handle as usize for Send + let fake_handle_addr: usize = 0x12345678; + + let mut mock = MockBootServices::new(); + + // Mock locate_device_path to return the fake handle and update the device path pointer + // to point to the remaining path (the FilePath node) + mock.expect_locate_device_path().returning(move |_protocol, device_path_ptr| { + // The device_path_ptr points to the partial path (HD/File) + // After matching, it should point to the remaining path (File) + // For this test, we'll advance it past the HD node to point at FilePath + + // SAFETY: Test code - we're simulating what locate_device_path does + unsafe { + // Read the current device path to find the HD node size + let current_ptr = *device_path_ptr as *const u8; + let header = current_ptr as *const efi::protocols::device_path::Protocol; + let hd_node_size = u16::from_le_bytes([(*header).length[0], (*header).length[1]]) as usize; + + // Advance past the HD node to point to FilePath + *device_path_ptr = current_ptr.add(hd_node_size) as *mut efi::protocols::device_path::Protocol; + } + Ok(fake_handle_addr as *mut core::ffi::c_void) + }); + + // Mock handle_protocol to return the full device path + mock.expect_handle_protocol::().returning(move |_handle| { + // SAFETY: Test code - returning reference to leaked bytes + Ok(unsafe { &mut *(full_path_ptr as *mut efi::protocols::device_path::Protocol) }) + }); + + let result = expand_device_path(&mock, &partial); + assert!(result.is_ok(), "expand_device_path should succeed"); + + let expanded = result.unwrap(); + assert_eq!(expanded, expected, "Expanded path should match expected full path with file"); + + // Note: leaked_bytes is intentionally leaked for the test - in tests this is acceptable + } + + #[test] + fn test_expand_partial_path_not_found() { + let partial = build_partial_hd_path([0xBB; 16]); + + let mut mock = MockBootServices::new(); + + // Mock locate_device_path to return NOT_FOUND + mock.expect_locate_device_path().returning(|_protocol, _device_path_ptr| Err(efi::Status::NOT_FOUND)); + + let result = expand_device_path(&mock, &partial); + assert!(result.is_err(), "expand_device_path should fail when device not found"); + } + + #[test] + fn test_expand_partial_path_handle_protocol_fails() { + let partial = build_partial_hd_path([0xCC; 16]); + let fake_handle_addr: usize = 0x87654321; + + let mut mock = MockBootServices::new(); + + // Mock locate_device_path to succeed + mock.expect_locate_device_path() + .returning(move |_protocol, _device_path_ptr| Ok(fake_handle_addr as *mut core::ffi::c_void)); + + // Mock handle_protocol to fail + mock.expect_handle_protocol::() + .returning(|_handle| Err(efi::Status::UNSUPPORTED)); + + let result = expand_device_path(&mock, &partial); + assert!(result.is_err(), "expand_device_path should fail when handle_protocol fails"); + } } diff --git a/patina_dxe_core/src/component_dispatcher.rs b/patina_dxe_core/src/component_dispatcher.rs index 33dd9693f..0f379e031 100644 --- a/patina_dxe_core/src/component_dispatcher.rs +++ b/patina_dxe_core/src/component_dispatcher.rs @@ -180,7 +180,6 @@ impl ComponentDispatcher { /// Sets the core Image Handle in storage. #[coverage(off)] - #[inline(always)] pub(crate) fn set_image_handle(&mut self, handle: efi::Handle) { self.storage.set_image_handle(handle); } From b9b501c85b7b1b2fcbd741ccb0ca962c19404a33 Mon Sep 17 00:00:00 2001 From: Kat Perez Date: Fri, 6 Mar 2026 12:08:46 -0500 Subject: [PATCH 5/9] [feature/patina-boot] patina_boot: Refactor to trait-based boot orchestration design (#1333) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Refactors `patina_boot` from a monolithic `BootOrchestrator` component + `Config` pattern to a trait-based design: - **`BootOrchestrator` trait** — defines the boot flow interface with `execute() -> Result`, enforcing at the type level that successful boot never returns - **`BootDispatcher`** — the Patina component that installs the BDS architectural protocol and delegates to a `BootOrchestrator` implementation - **`SimpleBootManager`** — a default `BootOrchestrator` implementation for platforms with straightforward boot topologies - **`BootConfig`** — unified boot configuration (replaces previous `BootOptions` + `SimpleBootConfig` split), requires at least one device at construction (compile-time enforcement) Also updates `helpers.rs` imports from removed `uefi_protocol::device_path` to `device_path::paths`/`device_path::node_defs`. `patina_dxe_core` changes (image handle plumbing) split out to #1374. - [x] Impacts functionality? - [ ] Impacts security? - [x] Breaking change? - [x] Includes tests? - [ ] Includes documentation? ## How This Was Tested 1. Unit tests: 35 pass (`cargo test -p patina_boot`) 2. Integration tested on QEMU Q35 — all components dispatched, BDS phase ran, boot options attempted, failure handler fired correctly 3. CI: fmt, clippy, all platforms pass ## Integration Instructions Update boot orchestration usage from: ```rust use patina_boot::{component::BootOrchestrator, config::BootOptions}; // In configs(): add.config(BootOptions::new()...); // In components(): add.component(BootOrchestrator); ``` To: ```rust use patina_boot::{BootDispatcher, SimpleBootManager, config::BootConfig}; add.component(BootDispatcher::new(SimpleBootManager::new( BootConfig::new(primary_device_path()) .with_device(fallback_device_path()) .with_hotkey(0x16) .with_hotkey_device(usb_device_path()) .with_failure_handler(|| { /* ... */ }), ))); ``` --- components/patina_boot/Cargo.toml | 1 + components/patina_boot/src/boot_dispatcher.rs | 166 ++++++++++++++++ .../patina_boot/src/boot_orchestrator.rs | 70 +++++++ components/patina_boot/src/component.rs | 18 -- .../src/component/boot_orchestrator.rs | 111 ----------- .../src/component/console_discovery.rs | 42 ---- components/patina_boot/src/config.rs | 114 +++++------ components/patina_boot/src/helpers.rs | 15 +- components/patina_boot/src/lib.rs | 23 ++- components/patina_boot/src/orchestrators.rs | 12 ++ .../src/orchestrators/simple_boot_manager.rs | 187 ++++++++++++++++++ 11 files changed, 506 insertions(+), 253 deletions(-) create mode 100644 components/patina_boot/src/boot_dispatcher.rs create mode 100644 components/patina_boot/src/boot_orchestrator.rs delete mode 100644 components/patina_boot/src/component.rs delete mode 100644 components/patina_boot/src/component/boot_orchestrator.rs delete mode 100644 components/patina_boot/src/component/console_discovery.rs create mode 100644 components/patina_boot/src/orchestrators.rs create mode 100644 components/patina_boot/src/orchestrators/simple_boot_manager.rs diff --git a/components/patina_boot/Cargo.toml b/components/patina_boot/Cargo.toml index 80c8e0c1b..b58463042 100644 --- a/components/patina_boot/Cargo.toml +++ b/components/patina_boot/Cargo.toml @@ -15,6 +15,7 @@ log = { workspace = true } patina = { workspace = true, features = ["unstable-device-path"] } patina_macro = { workspace = true } r-efi = { workspace = true } +spin = { workspace = true } [dev-dependencies] patina = { path = "../../sdk/patina", features = ["mockall", "unstable-device-path"] } diff --git a/components/patina_boot/src/boot_dispatcher.rs b/components/patina_boot/src/boot_dispatcher.rs new file mode 100644 index 000000000..139249b74 --- /dev/null +++ b/components/patina_boot/src/boot_dispatcher.rs @@ -0,0 +1,166 @@ +//! Boot dispatcher component. +//! +//! [`BootDispatcher`] is the Patina component that installs the BDS architectural +//! protocol and delegates to a [`BootOrchestrator`] +//! implementation when invoked by the DXE core. +//! +//! ## License +//! +//! Copyright (c) Microsoft Corporation. +//! +//! SPDX-License-Identifier: Apache-2.0 +//! +extern crate alloc; + +use alloc::boxed::Box; +use core::ffi::c_void; + +use patina::{ + boot_services::{BootServices, StandardBootServices}, + component::{component, params::Handle}, + error::{EfiError, Result}, + pi::protocols::bds, + runtime_services::StandardRuntimeServices, +}; +use spin::Once; + +use crate::boot_orchestrator::BootOrchestrator; + +/// Context stored in a static for the BDS protocol callback to access. +struct BdsContext { + orchestrator: Box, + boot_services: StandardBootServices, + runtime_services: StandardRuntimeServices, + image_handle: r_efi::efi::Handle, +} + +// SAFETY: BdsContext is only accessed from the BDS entry point which runs on the +// BSP (Bootstrap Processor) at TPL_APPLICATION. UEFI is single-threaded at this point. +unsafe impl Send for BdsContext {} +// SAFETY: BdsContext is stored in a spin::Once and only read after initialization. +// Access is single-threaded (BSP at TPL_APPLICATION during BDS phase). +unsafe impl Sync for BdsContext {} + +/// Static storage for the BDS context. Initialized once during component dispatch, +/// consumed once when the DXE core invokes the BDS protocol. +static BDS_CONTEXT: Once = Once::new(); + +/// Boot dispatcher component. +/// +/// This is the single Patina component for driving boot orchestration. It: +/// - Accepts a [`BootOrchestrator`] implementation via [`BootDispatcher::new()`] +/// - Installs the BDS architectural protocol during component dispatch +/// - When the DXE core invokes BDS: delegates to `orchestrator.execute()` +/// +/// ## Usage +/// +/// ```rust,ignore +/// use patina_boot::{BootDispatcher, SimpleBootManager, config::BootConfig}; +/// +/// // Minimal boot: +/// add.component(BootDispatcher::new(SimpleBootManager::new( +/// BootConfig::new(nvme_esp_path()) +/// .with_device(nvme_recovery_path()), +/// ))); +/// +/// // Custom orchestrator: +/// add.component(BootDispatcher::new(MyCustomOrchestrator::new())); +/// ``` +pub struct BootDispatcher { + orchestrator: Box, +} + +impl BootDispatcher { + /// Create a new `BootDispatcher` with the given orchestrator. + /// + /// The orchestrator is boxed internally — callers pass any type that + /// implements [`BootOrchestrator`]. + pub fn new(orchestrator: impl BootOrchestrator) -> Self { + Self { orchestrator: Box::new(orchestrator) } + } +} + +#[component] +impl BootDispatcher { + /// Entry point: stores context and installs the BDS architectural protocol. + /// + /// The actual boot flow does not execute here. It executes later when the + /// DXE core calls `bds_entry_point` after all architectural protocols are + /// satisfied and all DXE drivers have been dispatched. + #[coverage(off)] // Component integration — tested via integration tests + fn entry_point( + self, + boot_services: StandardBootServices, + runtime_services: StandardRuntimeServices, + image_handle: Option, + ) -> Result<()> { + let handle = image_handle.ok_or_else(|| { + log::error!("Handle not provided — required for LoadImage parent handle"); + EfiError::InvalidParameter + })?; + + // Store the orchestrator and services for the BDS callback + BDS_CONTEXT.call_once(|| BdsContext { + orchestrator: self.orchestrator, + boot_services: boot_services.clone(), + runtime_services: runtime_services.clone(), + image_handle: *handle, + }); + + // Install the BDS architectural protocol + let protocol = Box::leak(Box::new(bds::Protocol { entry: bds_entry_point })); + + // SAFETY: protocol is a valid, leaked BDS protocol struct with a valid entry function pointer. + // Using unchecked variant because bds::Protocol does not implement ProtocolInterface. + unsafe { + boot_services.as_ref().install_protocol_interface_unchecked( + None, + &bds::PROTOCOL_GUID, + protocol as *mut _ as *mut c_void, + ) + } + .map_err(EfiError::from)?; + + Ok(()) + } +} + +/// BDS architectural protocol entry point. +/// +/// Called by the DXE core after all architectural protocols are installed and +/// all DXE drivers have been dispatched. Retrieves the stored context and +/// delegates to the orchestrator. +#[coverage(off)] // Extern "efiapi" callback — tested via integration tests +extern "efiapi" fn bds_entry_point(_this: *mut bds::Protocol) { + let Some(context) = BDS_CONTEXT.get() else { + panic!("BDS context not initialized — BootDispatcher entry_point was not called"); + }; + + let Err(e) = context.orchestrator.execute(&context.boot_services, &context.runtime_services, context.image_handle); + panic!("BootOrchestrator::execute() failed: {e:?}"); +} + +#[cfg(test)] +mod tests { + use super::*; + use patina::{boot_services::StandardBootServices, runtime_services::StandardRuntimeServices}; + use r_efi::efi; + + struct MockOrchestrator; + + impl BootOrchestrator for MockOrchestrator { + fn execute( + &self, + _boot_services: &StandardBootServices, + _runtime_services: &StandardRuntimeServices, + _image_handle: efi::Handle, + ) -> core::result::Result { + Err(patina::error::EfiError::NotFound) + } + } + + #[test] + fn test_new_boot_dispatcher() { + let _dispatcher = BootDispatcher::new(MockOrchestrator); + } +} diff --git a/components/patina_boot/src/boot_orchestrator.rs b/components/patina_boot/src/boot_orchestrator.rs new file mode 100644 index 000000000..798e8b812 --- /dev/null +++ b/components/patina_boot/src/boot_orchestrator.rs @@ -0,0 +1,70 @@ +//! Boot orchestrator trait definition. +//! +//! Defines the [`BootOrchestrator`] trait that platforms implement to customize +//! boot behavior. The [`BootDispatcher`](crate::BootDispatcher) component holds +//! a `Box` and delegates to it when the DXE core invokes +//! the BDS architectural protocol. +//! +//! ## License +//! +//! Copyright (c) Microsoft Corporation. +//! +//! SPDX-License-Identifier: Apache-2.0 +//! +use patina::{boot_services::StandardBootServices, error::EfiError, runtime_services::StandardRuntimeServices}; +use r_efi::efi; + +/// Trait for boot orchestration. +/// +/// Platforms implement this trait to define custom boot flows. The implementation +/// is passed to [`BootDispatcher::new()`](crate::BootDispatcher::new) and invoked +/// when the DXE core calls the BDS architectural protocol entry point. +/// +/// ## Built-in Implementation +/// +/// [`SimpleBootManager`](crate::SimpleBootManager) provides a default implementation +/// for platforms with straightforward boot topologies (primary/secondary devices, +/// optional hotkey). +/// +/// ## Custom Implementation +/// +/// ```rust,ignore +/// use patina_boot::BootOrchestrator; +/// +/// struct MyCustomBoot { /* ... */ } +/// +/// impl BootOrchestrator for MyCustomBoot { +/// fn execute( +/// &self, +/// boot_services: &StandardBootServices, +/// runtime_services: &StandardRuntimeServices, +/// image_handle: efi::Handle, +/// ) -> Result { +/// // Custom boot flow... +/// // Return Err if all boot options are exhausted +/// Err(EfiError::NotFound) +/// } +/// } +/// ``` +pub trait BootOrchestrator: Send + Sync + 'static { + /// Execute the boot flow. + /// + /// Called by [`BootDispatcher`](crate::BootDispatcher) when the DXE core invokes + /// the BDS architectural protocol. This method should: + /// + /// 1. Enumerate devices (e.g., `connect_all()`) + /// 2. Signal BDS phase events (EndOfDxe, ReadyToBoot) + /// 3. Attempt to boot from configured device paths + /// 4. Handle boot failures + /// + /// A successful boot transfers control to the boot image and never returns. + /// If all boot options are exhausted, the implementation returns + /// `Err(EfiError)`. The `Ok` variant is uninhabitable (`!`), enforcing at + /// the type level that this method can only "succeed" by not returning. + fn execute( + &self, + boot_services: &StandardBootServices, + runtime_services: &StandardRuntimeServices, + image_handle: efi::Handle, + ) -> Result; +} diff --git a/components/patina_boot/src/component.rs b/components/patina_boot/src/component.rs deleted file mode 100644 index fb6840a95..000000000 --- a/components/patina_boot/src/component.rs +++ /dev/null @@ -1,18 +0,0 @@ -//! Boot orchestration components. -//! -//! This module provides the core boot components: -//! - [`ConsoleDiscovery`]: Discovers console devices and populates ConIn/ConOut/ErrOut variables -//! - [`BootOrchestrator`]: Orchestrates the boot flow with device enumeration, event signaling, and boot execution -//! -//! ## License -//! -//! Copyright (c) Microsoft Corporation. -//! -//! SPDX-License-Identifier: Apache-2.0 -//! - -mod boot_orchestrator; -mod console_discovery; - -pub use boot_orchestrator::BootOrchestrator; -pub use console_discovery::ConsoleDiscovery; diff --git a/components/patina_boot/src/component/boot_orchestrator.rs b/components/patina_boot/src/component/boot_orchestrator.rs deleted file mode 100644 index 88db2c8e4..000000000 --- a/components/patina_boot/src/component/boot_orchestrator.rs +++ /dev/null @@ -1,111 +0,0 @@ -//! Boot orchestrator component. -//! -//! Orchestrates the boot flow with device enumeration, event signaling, and boot execution. -//! -//! ## Rationale -//! -//! Boot orchestration is a component that represents the primary platform customization -//! point. Platforms provide boot options as configuration data, allowing different boot policies -//! without changing orchestration logic. Platforms needing entirely different boot flows can -//! replace the component. -//! -//! ## License -//! -//! Copyright (c) Microsoft Corporation. -//! -//! SPDX-License-Identifier: Apache-2.0 -//! -extern crate alloc; - -use patina::{ - boot_services::StandardBootServices, - component::{ - component, - params::{Config, Handle}, - }, - error::{EfiError, Result}, - runtime_services::StandardRuntimeServices, -}; - -use crate::{config::BootOptions, helpers}; - -/// Boot orchestrator component. -/// -/// Orchestrates boot execution using boot options provided via [`Config`]. -/// Connects all controllers for device topology enumeration, signals BDS phase events -/// (EndOfDxe, ReadyToBoot), and executes boot options via `LoadImage()`/`StartImage()` -/// with 5-minute watchdog per UEFI Section 3.1.2. -/// -/// Handles boot failures by attempting subsequent options. Never returns on success. -pub struct BootOrchestrator; - -#[component] -impl BootOrchestrator { - /// Entry point for the boot orchestrator component. - /// - /// # Flow - /// - /// 1. Connect all controllers for device enumeration - /// 2. Signal EndOfDxe (security components perform lockdown) - /// 3. Discover consoles - /// 4. Check for hotkey press; if detected, use alternate boot options - /// 5. Signal ReadyToBoot - /// 6. Execute boot options from config (or hotkey_devices if hotkey detected) - /// 7. If all boot options fail, call failure handler - #[coverage(off)] // Component integration - tested via integration tests - fn entry_point( - self, - boot_services: StandardBootServices, - runtime_services: StandardRuntimeServices, - boot_options: Config, - image_handle: Option, - ) -> Result<()> { - helpers::connect_all(boot_services.as_ref())?; - helpers::signal_bds_phase_entry(boot_services.as_ref())?; - helpers::discover_console_devices(boot_services.as_ref(), runtime_services.as_ref())?; - - // Check for hotkey press after devices are connected and consoles discovered - let use_hotkey_devices = if let Some(hotkey) = boot_options.hotkey() { - helpers::detect_hotkey(boot_services.as_ref(), hotkey) - } else { - false - }; - - helpers::signal_ready_to_boot(boot_services.as_ref())?; - - // Per UEFI spec, the parent handle must be a valid image handle (has LoadedImage protocol). - // The Handle must be provided by the component framework - we cannot safely guess - // which handle is correct from the handle database as ordering is not guaranteed. - let parent_handle = image_handle.ok_or_else(|| { - log::error!("Handle not provided - required for LoadImage parent handle"); - EfiError::InvalidParameter - })?; - - // Select boot devices based on hotkey detection - let boot_devices: alloc::vec::Vec<_> = if use_hotkey_devices { - log::info!("Using alternate boot options (hotkey detected)"); - boot_options.hotkey_devices().collect() - } else { - boot_options.devices().collect() - }; - - for device_path in boot_devices { - match helpers::boot_from_device_path(boot_services.as_ref(), *parent_handle, device_path) { - Ok(()) => { - // Boot image returned control (e.g., EFI application exited). - // Continue to try next boot option. - log::warn!("Boot option returned, trying next..."); - } - Err(_) => { - log::warn!("Boot option failed, trying next..."); - } - } - } - - boot_options.handle_failure(); - log::error!("All boot options exhausted and failure handler returned"); - loop { - core::hint::spin_loop(); - } - } -} diff --git a/components/patina_boot/src/component/console_discovery.rs b/components/patina_boot/src/component/console_discovery.rs deleted file mode 100644 index b7d51f7c1..000000000 --- a/components/patina_boot/src/component/console_discovery.rs +++ /dev/null @@ -1,42 +0,0 @@ -//! Console discovery component. -//! -//! Locates GOP and SimpleTextInput protocol handles, creates device paths, -//! and writes ConIn/ConOut/ErrOut variables. -//! -//! ## Rationale -//! -//! Console setup is a component because platforms may need custom console configuration -//! (serial-only, specific GOP preference, headless operation). As a component, platforms -//! can replace it entirely without modifying orchestration code. -//! -//! ## Prerequisites -//! -//! Device controllers exposing GOP and input protocols must be connected before -//! this component runs. -//! -//! ## License -//! -//! Copyright (c) Microsoft Corporation. -//! -//! SPDX-License-Identifier: Apache-2.0 -//! -use patina::{ - boot_services::StandardBootServices, component::component, error::Result, runtime_services::StandardRuntimeServices, -}; - -use crate::helpers; - -/// Console discovery component. -/// -/// Scans for GOP and SimpleTextInput protocol handles, creates device paths, -/// and writes `ConIn`/`ConOut`/`ErrOut` UEFI variables. -pub struct ConsoleDiscovery; - -#[component] -impl ConsoleDiscovery { - /// Entry point for the console discovery component. - #[coverage(off)] // Component integration - tested via integration tests - fn entry_point(self, boot_services: StandardBootServices, runtime_services: StandardRuntimeServices) -> Result<()> { - helpers::discover_console_devices(boot_services.as_ref(), runtime_services.as_ref()) - } -} diff --git a/components/patina_boot/src/config.rs b/components/patina_boot/src/config.rs index cd23c4304..928a18be2 100644 --- a/components/patina_boot/src/config.rs +++ b/components/patina_boot/src/config.rs @@ -1,7 +1,7 @@ //! Boot configuration types. //! -//! This module provides configuration types for boot orchestration, including -//! [`BootOptions`] which specifies platform-provided boot paths. +//! [`BootConfig`] provides the platform boot configuration used by +//! [`BootOrchestrator`](crate::BootOrchestrator) implementations. //! //! ## License //! @@ -12,27 +12,25 @@ extern crate alloc; use alloc::{boxed::Box, vec::Vec}; -use patina::uefi_protocol::device_path::DevicePathBuf; +use patina::device_path::paths::DevicePathBuf; /// Boot options provided by the platform. /// /// Platforms configure boot behavior by providing this configuration to the -/// [`BootOrchestrator`](crate::component::BootOrchestrator) component. +/// [`SimpleBootManager`](crate::SimpleBootManager). /// /// ## Example /// /// ```rust,ignore -/// use patina_boot::config::BootOptions; +/// use patina_boot::config::BootConfig; /// -/// let options = BootOptions::new() -/// .with_device(nvme_device_path) +/// let config = BootConfig::new(nvme_device_path) /// .with_device(usb_device_path) /// .with_hotkey(0x16) // F12 (UEFI scan code) /// .with_hotkey_device(usb_device_path) // Boot from USB when F12 pressed /// .with_failure_handler(|| show_error_screen()); /// ``` -#[derive(Default)] -pub struct BootOptions { +pub struct BootConfig { /// Boot device paths in priority order. devices: Vec, /// Optional hotkey for boot override (e.g., F12 for boot menu). @@ -43,10 +41,13 @@ pub struct BootOptions { failure_handler: Option>, } -impl BootOptions { - /// Create new empty boot options. - pub fn new() -> Self { - Self::default() +impl BootConfig { + /// Create a new boot configuration with an initial boot device. + /// + /// At least one boot device is required. Additional devices can be added + /// with [`with_device`](Self::with_device). + pub fn new(device: DevicePathBuf) -> Self { + Self { devices: alloc::vec![device], hotkey: None, hotkey_devices: Vec::new(), failure_handler: None } } /// Add a boot device path. @@ -122,80 +123,57 @@ mod tests { use super::*; use alloc::sync::Arc; use core::sync::atomic::{AtomicBool, Ordering}; - use patina::uefi_protocol::device_path::nodes::EndEntire; + use patina::device_path::node_defs::EndEntire; fn create_test_device_path() -> DevicePathBuf { DevicePathBuf::from_device_path_node_iter(core::iter::once(EndEntire)) } #[test] - fn test_default_boot_options() { - let options = BootOptions::default(); - assert!(options.hotkey().is_none()); - assert_eq!(options.devices().count(), 0); - } - - #[test] - fn test_new_boot_options() { - let options = BootOptions::new(); - assert_eq!(options.devices().count(), 0); + fn test_new_requires_device() { + let config = BootConfig::new(create_test_device_path()); + assert_eq!(config.devices().count(), 1); + assert!(config.hotkey().is_none()); + assert_eq!(config.hotkey_devices().count(), 0); } #[test] - fn test_with_single_device() { - let device = create_test_device_path(); - let options = BootOptions::new().with_device(device); - assert_eq!(options.devices().count(), 1); - } - - #[test] - fn test_with_multiple_devices() { - let device1 = create_test_device_path(); - let device2 = create_test_device_path(); - let device3 = create_test_device_path(); - let options = BootOptions::new().with_device(device1).with_device(device2).with_device(device3); - assert_eq!(options.devices().count(), 3); + fn test_with_additional_devices() { + let config = BootConfig::new(create_test_device_path()) + .with_device(create_test_device_path()) + .with_device(create_test_device_path()); + assert_eq!(config.devices().count(), 3); } #[test] fn test_with_hotkey() { - let options = BootOptions::new().with_hotkey(0x16); // F12 - assert_eq!(options.hotkey(), Some(0x16)); + let config = BootConfig::new(create_test_device_path()).with_hotkey(0x16); // F12 + assert_eq!(config.hotkey(), Some(0x16)); } #[test] fn test_with_hotkey_device() { - let device = create_test_device_path(); - let options = BootOptions::new().with_hotkey_device(device); - assert_eq!(options.hotkey_devices().count(), 1); + let config = BootConfig::new(create_test_device_path()).with_hotkey_device(create_test_device_path()); + assert_eq!(config.hotkey_devices().count(), 1); } #[test] fn test_with_multiple_hotkey_devices() { - let device1 = create_test_device_path(); - let device2 = create_test_device_path(); - let options = BootOptions::new().with_hotkey_device(device1).with_hotkey_device(device2); - assert_eq!(options.hotkey_devices().count(), 2); + let config = BootConfig::new(create_test_device_path()) + .with_hotkey_device(create_test_device_path()) + .with_hotkey_device(create_test_device_path()); + assert_eq!(config.hotkey_devices().count(), 2); } #[test] fn test_hotkey_with_hotkey_devices() { - let primary = create_test_device_path(); - let alternate = create_test_device_path(); - let options = BootOptions::new() - .with_device(primary) + let config = BootConfig::new(create_test_device_path()) .with_hotkey(0x16) // F12 - .with_hotkey_device(alternate); + .with_hotkey_device(create_test_device_path()); - assert_eq!(options.devices().count(), 1); - assert_eq!(options.hotkey(), Some(0x16)); - assert_eq!(options.hotkey_devices().count(), 1); - } - - #[test] - fn test_default_has_no_hotkey_devices() { - let options = BootOptions::default(); - assert_eq!(options.hotkey_devices().count(), 0); + assert_eq!(config.devices().count(), 1); + assert_eq!(config.hotkey(), Some(0x16)); + assert_eq!(config.hotkey_devices().count(), 1); } #[test] @@ -203,30 +181,26 @@ mod tests { let called = Arc::new(AtomicBool::new(false)); let called_clone = called.clone(); - let device = create_test_device_path(); - let options = BootOptions::new().with_device(device).with_failure_handler(move || { + let config = BootConfig::new(create_test_device_path()).with_failure_handler(move || { called_clone.store(true, Ordering::SeqCst); }); assert!(!called.load(Ordering::SeqCst)); - options.handle_failure(); + config.handle_failure(); assert!(called.load(Ordering::SeqCst)); } #[test] fn test_failure_handler_not_configured() { - let options = BootOptions::default(); + let config = BootConfig::new(create_test_device_path()); // Should not panic when no handler is configured - options.handle_failure(); + config.handle_failure(); } #[test] fn test_devices_iterator_order() { - let device1 = create_test_device_path(); - let device2 = create_test_device_path(); - let options = BootOptions::new().with_device(device1).with_device(device2); - - let devices: Vec<_> = options.devices().collect(); + let config = BootConfig::new(create_test_device_path()).with_device(create_test_device_path()); + let devices: Vec<_> = config.devices().collect(); assert_eq!(devices.len(), 2); } } diff --git a/components/patina_boot/src/helpers.rs b/components/patina_boot/src/helpers.rs index ffedd299b..0c7528289 100644 --- a/components/patina_boot/src/helpers.rs +++ b/components/patina_boot/src/helpers.rs @@ -1,8 +1,8 @@ //! Helper functions for boot orchestration. //! //! This module provides helper functions for platforms implementing custom boot flows. -//! The [`BootOrchestrator`](crate::component::BootOrchestrator) component uses these -//! internally, and platforms can use them directly for custom orchestration. +//! The [`SimpleBootManager`](crate::SimpleBootManager) uses these internally, and +//! platforms can use them directly for custom orchestration. //! //! ## License //! @@ -17,10 +17,13 @@ use core::ptr; use patina::{ boot_services::{BootServices, event::EventType, protocol_handler::HandleSearchType, tpl::Tpl}, + device_path::{ + node_defs::DevicePathType, + paths::{DevicePath, DevicePathBuf}, + }, error::{EfiError, Result}, guids::EVENT_GROUP_END_OF_DXE, runtime_services::RuntimeServices, - uefi_protocol::device_path::{DevicePath, DevicePathBuf, nodes::DevicePathType}, }; use r_efi::{efi, protocols::simple_text_input, system::EVENT_GROUP_READY_TO_BOOT}; @@ -409,7 +412,7 @@ mod tests { use core::sync::atomic::{AtomicUsize, Ordering}; use patina::{ boot_services::MockBootServices, - uefi_protocol::device_path::nodes::{Acpi, EndEntire, HardDrive}, + device_path::node_defs::{Acpi, EndEntire, HardDrive}, }; fn create_test_device_path() -> DevicePathBuf { @@ -632,7 +635,7 @@ mod tests { // Tests for partial device path expansion - use patina::uefi_protocol::device_path::nodes::Pci; + use patina::device_path::node_defs::Pci; /// Helper to build a partial device path starting with HD node. fn build_partial_hd_path(guid: [u8; 16]) -> DevicePathBuf { @@ -684,7 +687,7 @@ mod tests { #[test] fn test_expand_partial_path_success() { use alloc::boxed::Box; - use patina::uefi_protocol::device_path::nodes::FilePath; + use patina::device_path::node_defs::FilePath; let guid = [0xAA; 16]; diff --git a/components/patina_boot/src/lib.rs b/components/patina_boot/src/lib.rs index d60bb0d06..685828fcd 100644 --- a/components/patina_boot/src/lib.rs +++ b/components/patina_boot/src/lib.rs @@ -1,16 +1,20 @@ //! Boot Orchestration Components //! -//! This crate provides boot orchestration components for Patina firmware, implementing +//! This crate provides boot orchestration for Patina firmware, implementing //! UEFI Specification 2.11 Chapter 3 (Boot Manager) and PI Specification BDS phase requirements. //! -//! ## Components +//! ## Architecture //! -//! - [`component::ConsoleDiscovery`]: Discovers console devices and populates ConIn/ConOut/ErrOut variables -//! - [`component::BootOrchestrator`]: Orchestrates the boot flow with device enumeration, event signaling, and boot execution +//! - [`BootOrchestrator`]: A trait defining the boot flow interface. Platforms implement this +//! trait to customize boot behavior. +//! - [`BootDispatcher`]: The Patina component that installs the BDS architectural protocol and +//! delegates to a `BootOrchestrator` implementation when invoked by the DXE core. +//! - [`SimpleBootManager`]: A default `BootOrchestrator` implementation for platforms with +//! straightforward boot topologies. //! //! ## Configuration //! -//! - [`config::BootOptions`]: Platform-provided boot options as device paths +//! - [`config::BootConfig`]: Boot configuration for `BootOrchestrator` implementations //! //! ## Helper Functions //! @@ -24,7 +28,14 @@ //! #![cfg_attr(not(feature = "std"), no_std)] #![feature(coverage_attribute)] +#![feature(never_type)] -pub mod component; +pub mod boot_dispatcher; +pub mod boot_orchestrator; pub mod config; pub mod helpers; +pub mod orchestrators; + +pub use boot_dispatcher::BootDispatcher; +pub use boot_orchestrator::BootOrchestrator; +pub use orchestrators::SimpleBootManager; diff --git a/components/patina_boot/src/orchestrators.rs b/components/patina_boot/src/orchestrators.rs new file mode 100644 index 000000000..a5771c2d8 --- /dev/null +++ b/components/patina_boot/src/orchestrators.rs @@ -0,0 +1,12 @@ +//! Built-in [`BootOrchestrator`](crate::BootOrchestrator) implementations. +//! +//! ## License +//! +//! Copyright (c) Microsoft Corporation. +//! +//! SPDX-License-Identifier: Apache-2.0 +//! + +mod simple_boot_manager; + +pub use simple_boot_manager::SimpleBootManager; diff --git a/components/patina_boot/src/orchestrators/simple_boot_manager.rs b/components/patina_boot/src/orchestrators/simple_boot_manager.rs new file mode 100644 index 000000000..7250c9ba6 --- /dev/null +++ b/components/patina_boot/src/orchestrators/simple_boot_manager.rs @@ -0,0 +1,187 @@ +//! Simple boot manager implementation. +//! +//! [`SimpleBootManager`] implements the [`BootOrchestrator`](crate::BootOrchestrator) +//! trait for platforms with straightforward boot topologies. It supports: +//! +//! - Flexible multi-device boot via [`BootConfig`](crate::config::BootConfig) +//! - Optional hotkey detection for alternate boot paths +//! - Configurable failure handler +//! +//! ## License +//! +//! Copyright (c) Microsoft Corporation. +//! +//! SPDX-License-Identifier: Apache-2.0 +//! +extern crate alloc; + +use alloc::vec::Vec; + +use patina::{ + boot_services::StandardBootServices, device_path::paths::DevicePathBuf, error::EfiError, + runtime_services::StandardRuntimeServices, +}; +use r_efi::efi; + +use crate::{boot_orchestrator::BootOrchestrator, config::BootConfig, helpers}; + +/// Simple boot manager implementing [`BootOrchestrator`]. +/// +/// Provides a default boot flow suitable for platforms with straightforward +/// boot topologies. +/// +/// ## Boot Flow +/// +/// 1. Connect all controllers for device enumeration +/// 2. Signal EndOfDxe (security lockdown) +/// 3. Discover console devices +/// 4. Detect hotkey (if configured); select alternate devices if pressed +/// 5. Signal ReadyToBoot +/// 6. Iterate boot devices, attempt `LoadImage()`/`StartImage()` for each +/// 7. Call failure handler if all options exhausted +pub struct SimpleBootManager { + config: BootConfig, +} + +impl SimpleBootManager { + /// Create a `SimpleBootManager` from a boot configuration. + /// + /// ## Example + /// + /// ```rust,ignore + /// use patina_boot::{SimpleBootManager, config::BootConfig}; + /// + /// let manager = SimpleBootManager::new( + /// BootConfig::new(nvme_esp_path()) + /// .with_device(nvme_recovery_path()) + /// .with_hotkey(0x16) + /// .with_hotkey_device(usb_device_path()) + /// .with_failure_handler(|| show_error_screen("Boot failed")), + /// ); + /// ``` + pub fn new(config: BootConfig) -> Self { + Self { config } + } +} + +// Expose config for test assertions +#[cfg(test)] +impl SimpleBootManager { + pub(crate) fn config(&self) -> &BootConfig { + &self.config + } +} + +impl BootOrchestrator for SimpleBootManager { + #[coverage(off)] // Integration point — delegates to helper functions which are individually tested + fn execute( + &self, + boot_services: &StandardBootServices, + runtime_services: &StandardRuntimeServices, + image_handle: efi::Handle, + ) -> Result { + if let Err(e) = helpers::connect_all(boot_services) { + log::error!("connect_all failed: {:?}", e); + } + + if let Err(e) = helpers::signal_bds_phase_entry(boot_services) { + log::error!("signal_bds_phase_entry failed: {:?}", e); + } + + if let Err(e) = helpers::discover_console_devices(boot_services, runtime_services) { + log::error!("discover_console_devices failed: {:?}", e); + } + + // Check for hotkey press after devices are connected and consoles discovered + let use_hotkey_devices = + if let Some(hotkey) = self.config.hotkey() { helpers::detect_hotkey(boot_services, hotkey) } else { false }; + + if let Err(e) = helpers::signal_ready_to_boot(boot_services) { + log::error!("signal_ready_to_boot failed: {:?}", e); + } + + // Select boot devices based on hotkey detection + let boot_devices: Vec<&DevicePathBuf> = if use_hotkey_devices { + log::info!("Using alternate boot options (hotkey detected)"); + self.config.hotkey_devices().collect() + } else { + self.config.devices().collect() + }; + + for device_path in boot_devices { + match helpers::boot_from_device_path(boot_services, image_handle, device_path) { + Ok(()) => { + // Boot image returned control (e.g., EFI application exited). + // Continue to try next boot option. + log::warn!("Boot option returned, trying next..."); + } + Err(_) => { + log::warn!("Boot option failed, trying next..."); + } + } + } + + self.config.handle_failure(); + log::error!("All boot options exhausted"); + Err(EfiError::NotFound) + } +} + +#[cfg(test)] +mod tests { + extern crate alloc; + + use super::*; + use alloc::sync::Arc; + use core::sync::atomic::{AtomicBool, Ordering}; + use patina::device_path::{node_defs::EndEntire, paths::DevicePathBuf}; + + fn test_device_path() -> DevicePathBuf { + DevicePathBuf::from_device_path_node_iter(core::iter::once(EndEntire)) + } + + #[test] + fn test_new() { + let config = BootConfig::new(test_device_path()).with_hotkey(0x16).with_hotkey_device(test_device_path()); + let manager = SimpleBootManager::new(config); + assert_eq!(manager.config().hotkey(), Some(0x16)); + assert_eq!(manager.config().devices().count(), 1); + assert_eq!(manager.config().hotkey_devices().count(), 1); + } + + #[test] + fn test_with_hotkey() { + let config = BootConfig::new(test_device_path()) + .with_device(test_device_path()) + .with_hotkey(0x16) + .with_hotkey_device(test_device_path()); + let manager = SimpleBootManager::new(config); + assert_eq!(manager.config().hotkey(), Some(0x16)); + assert_eq!(manager.config().devices().count(), 2); + assert_eq!(manager.config().hotkey_devices().count(), 1); + } + + #[test] + fn test_without_hotkey() { + let config = BootConfig::new(test_device_path()).with_device(test_device_path()); + let manager = SimpleBootManager::new(config); + assert!(manager.config().hotkey().is_none()); + assert_eq!(manager.config().devices().count(), 2); + assert_eq!(manager.config().hotkey_devices().count(), 0); + } + + #[test] + fn test_with_failure_handler() { + let called = Arc::new(AtomicBool::new(false)); + let called_clone = called.clone(); + + let config = BootConfig::new(test_device_path()).with_failure_handler(move || { + called_clone.store(true, Ordering::SeqCst); + }); + let manager = SimpleBootManager::new(config); + + assert!(!called.load(Ordering::SeqCst)); + manager.config().handle_failure(); + assert!(called.load(Ordering::SeqCst)); + } +} From 9c444f2e0fb8f240090b2654b1cccec517b2f18e Mon Sep 17 00:00:00 2001 From: Kat Perez Date: Tue, 17 Mar 2026 08:32:49 -0400 Subject: [PATCH 6/9] [feature/patina-boot] patina_boot: Implement full console discovery (#1375) ## Description Rewrite `discover_console_devices()` from a stub into a full implementation that enumerates console protocol handles, builds multi-instance device paths, and writes `ConIn`, `ConOut`, and `ErrOut` UEFI global variables via `SetVariable`. - Adds `EFI_GLOBAL_VARIABLE` GUID to `patina::guids` - Adds `build_multi_instance_device_path()` helper for constructing multi-instance device paths from protocol handles - Updates `is_partial_device_path()` to recognize FV/FvFile paths as non-partial - Includes get_variable readback verification with device path display logging Depends on #1333. Closes #1230 - [x] Impacts functionality? - [ ] Impacts security? - [ ] Breaking change? - [ ] Includes tests? - [ ] Includes documentation? ## How This Was Tested Verified on QEMU Q35 with VGA enabled. Console variables written and read back successfully: - ConIn: 24 bytes (SimpleTextInput) - ConOut: 60 bytes (SimpleTextOutput + GOP) - ErrOut: 30 bytes (SimpleTextOutput) ## Integration Instructions N/A --- components/patina_boot/src/helpers.rs | 660 +++++++++++++++++++----- patina_dxe_core/src/lib.rs | 8 +- sdk/patina/src/device_path/node_defs.rs | 10 + sdk/patina/src/device_path/paths.rs | 5 + sdk/patina/src/guids.rs | 13 + 5 files changed, 575 insertions(+), 121 deletions(-) diff --git a/components/patina_boot/src/helpers.rs b/components/patina_boot/src/helpers.rs index 0c7528289..6819e4490 100644 --- a/components/patina_boot/src/helpers.rs +++ b/components/patina_boot/src/helpers.rs @@ -18,14 +18,18 @@ use core::ptr; use patina::{ boot_services::{BootServices, event::EventType, protocol_handler::HandleSearchType, tpl::Tpl}, device_path::{ - node_defs::DevicePathType, + node_defs::{DevicePathType, HardDrive, MediaSubType}, paths::{DevicePath, DevicePathBuf}, }, error::{EfiError, Result}, - guids::EVENT_GROUP_END_OF_DXE, + guids::{EFI_GLOBAL_VARIABLE, EVENT_GROUP_END_OF_DXE}, runtime_services::RuntimeServices, }; -use r_efi::{efi, protocols::simple_text_input, system::EVENT_GROUP_READY_TO_BOOT}; +use r_efi::{ + efi, + protocols::simple_text_input, + system::{EVENT_GROUP_READY_TO_BOOT, VARIABLE_BOOTSERVICE_ACCESS, VARIABLE_NON_VOLATILE, VARIABLE_RUNTIME_ACCESS}, +}; /// Watchdog timeout in seconds per UEFI Specification Section 3.1.2. const WATCHDOG_TIMEOUT_SECONDS: usize = 300; // 5 minutes @@ -67,7 +71,6 @@ pub fn detect_hotkey(boot_services: &B, hotkey_scancode: u16) - /// /// - `handles` must contain valid handles obtained from `locate_handle_buffer` /// - Each handle must support the `SimpleTextInput` protocol for `handle_protocol` to succeed -#[coverage(off)] // Uses raw protocol function pointers - tested via integration tests unsafe fn detect_hotkey_from_handles( boot_services: &B, handles: &[efi::Handle], @@ -167,12 +170,6 @@ pub fn boot_from_device_path( /// /// Returns `Ok(())` when device topology enumeration is complete. /// -/// # Coverage -/// -/// This function is marked `#[coverage(off)]` because `BootServicesBox` return -/// values from `locate_handle_buffer` cannot be created in unit tests. The -/// function is tested via integration tests on real hardware/emulators. -#[coverage(off)] // BootServicesBox return type cannot be mocked - tested via integration tests pub fn connect_all(boot_services: &B) -> Result<()> { // Loop until the number of handles stabilizes, indicating device topology is complete. // This is needed because connecting a PCI bus creates new handles for PCI devices, @@ -268,37 +265,103 @@ pub fn signal_ready_to_boot(boot_services: &B) -> Result<()> { Ok(()) } -/// Discover console devices (stub implementation). +/// Discover console devices and write ConIn, ConOut, and ErrOut UEFI variables. /// -/// This is a placeholder that locates GOP and SimpleTextInput handles but does -/// not yet write the `ConIn`, `ConOut`, and `ErrOut` UEFI variables. +/// Enumerates all handles supporting console-related protocols (SimpleTextInput, +/// SimpleTextOutput, GraphicsOutput) and writes multi-instance device paths to the +/// corresponding UEFI global variables. These variables allow UEFI applications and +/// OS loaders to discover available console devices. /// -/// Platforms requiring full console variable support should implement this -/// functionality or use platform-specific console initialization. +/// Individual variable failures are non-fatal — the function logs a warning and +/// continues with the remaining variables. /// /// # Arguments /// -/// * `boot_services` - Boot services interface -/// * `runtime_services` - Runtime services interface (unused in stub) -#[allow(unused_variables)] +/// * `boot_services` - Boot services interface for handle enumeration +/// * `runtime_services` - Runtime services interface for writing UEFI variables pub fn discover_console_devices( boot_services: &B, runtime_services: &R, ) -> Result<()> { - // Stub: Locate handles to verify protocols exist, but don't write variables. - // Full implementation would create multi-instance device paths and write - // ConIn/ConOut/ErrOut variables via runtime_services.set_variable(). - let _gop_handles = boot_services - .locate_handle_buffer(HandleSearchType::ByProtocol(&efi::protocols::graphics_output::PROTOCOL_GUID)) - .ok(); - - let _input_handles = boot_services - .locate_handle_buffer(HandleSearchType::ByProtocol(&efi::protocols::simple_text_input::PROTOCOL_GUID)) - .ok(); + let attrs = VARIABLE_NON_VOLATILE | VARIABLE_BOOTSERVICE_ACCESS | VARIABLE_RUNTIME_ACCESS; + + // UTF-16 null-terminated variable names + let con_in_name: &[u16] = &[b'C' as u16, b'o' as u16, b'n' as u16, b'I' as u16, b'n' as u16, 0]; + let con_out_name: &[u16] = &[b'C' as u16, b'o' as u16, b'n' as u16, b'O' as u16, b'u' as u16, b't' as u16, 0]; + let err_out_name: &[u16] = &[b'E' as u16, b'r' as u16, b'r' as u16, b'O' as u16, b'u' as u16, b't' as u16, 0]; + + let console_vars: &[(&str, &[u16], &[&'static efi::Guid])] = &[ + ("ConIn", con_in_name, &[&simple_text_input::PROTOCOL_GUID]), + ( + "ConOut", + con_out_name, + &[&efi::protocols::simple_text_output::PROTOCOL_GUID, &efi::protocols::graphics_output::PROTOCOL_GUID], + ), + ("ErrOut", err_out_name, &[&efi::protocols::simple_text_output::PROTOCOL_GUID]), + ]; + + for &(label, name, guids) in console_vars { + let device_path = build_multi_instance_device_path(boot_services, guids); + + if let Some(dp) = device_path { + let bytes = dp.as_ref().as_bytes().to_vec(); + + if let Err(e) = runtime_services.set_variable(name, &EFI_GLOBAL_VARIABLE, attrs, &bytes) { + log::error!("{label}: failed to set variable: {e:?}"); + } + } + } Ok(()) } +/// Build a multi-instance device path from all handles supporting the given protocols. +/// +/// For each protocol GUID, locates all handles via `locate_handle_buffer` and extracts +/// their device paths, combining them into a single multi-instance device path separated +/// by `EndInstance` nodes. +/// +fn build_multi_instance_device_path( + boot_services: &B, + protocol_guids: &[&'static efi::Guid], +) -> Option { + let mut result: Option = None; + let mut seen_handles: Vec = Vec::new(); + + for &guid in protocol_guids { + let handles = match boot_services.locate_handle_buffer(HandleSearchType::ByProtocol(guid)) { + Ok(handles) => handles, + Err(_) => continue, + }; + + for &handle in handles.iter() { + if seen_handles.contains(&handle) { + continue; + } + seen_handles.push(handle); + // SAFETY: handle is valid from locate_handle_buffer, requesting device path protocol. + let dp_ptr = match unsafe { boot_services.handle_protocol::(handle) } + { + Ok(ptr) => ptr, + Err(_) => continue, + }; + + // SAFETY: The device path pointer comes from a valid protocol interface. + let device_path = match unsafe { DevicePath::try_from_ptr(dp_ptr as *const _ as *const u8) } { + Ok(dp) => dp, + Err(_) => continue, + }; + + match &mut result { + Some(multi) => multi.append_device_path_instances(device_path), + None => result = Some(DevicePathBuf::from(device_path)), + } + } + } + + result +} + /// No-op event callback for signal-only events. #[coverage(off)] // Extern callback - tested via integration tests extern "efiapi" fn signal_event_noop(_event: *mut core::ffi::c_void, _context: *mut ()) {} @@ -325,9 +388,19 @@ pub fn is_partial_device_path(device_path: &DevicePath) -> bool { return false; }; - // Full paths start with Hardware (1) or ACPI (2) nodes - // Partial paths start with Media (4), Messaging (3), or other nodes + // Full paths start with Hardware (1) or ACPI (2) nodes. + // Media FV/FvFile paths are also complete — LoadImage resolves them directly. + // Partial paths start with Media HardDrive (4/1), Messaging (3), or other nodes. let node_type = first_node.header.r#type; + let node_subtype = first_node.header.sub_type; + + if node_type == DevicePathType::Media as u8 + && (node_subtype == MediaSubType::PiwgFirmwareFile as u8 + || node_subtype == MediaSubType::PiwgFirmwareVolume as u8) + { + return false; + } + node_type != DevicePathType::Hardware as u8 && node_type != DevicePathType::Acpi as u8 && node_type != DevicePathType::End as u8 @@ -361,46 +434,93 @@ pub fn is_partial_device_path(device_path: &DevicePath) -> bool { /// Future enhancements may add support for: /// - FilePath-only paths (require filesystem enumeration) /// - Messaging node paths without root -#[coverage(off)] // Uses raw protocol pointers - tested via integration tests pub fn expand_device_path(boot_services: &B, partial_path: &DevicePath) -> Result { // Return unchanged if already a full path if !is_partial_device_path(partial_path) { return Ok(partial_path.into()); } - // Use LocateDevicePath to find the handle with the best matching device path. - // This is more efficient than enumerating all handles manually. - let mut device_path_ptr = - partial_path as *const DevicePath as *const u8 as *mut efi::protocols::device_path::Protocol; - // SAFETY: device_path_ptr points to a valid device path from partial_path. - let handle = - unsafe { boot_services.locate_device_path(&efi::protocols::device_path::PROTOCOL_GUID, &mut device_path_ptr) } - .map_err(EfiError::from)?; + // Parse the HardDrive node from the partial path to extract the partition signature. + let target_sig = partial_path.iter().find_map(|node| { + let hd = HardDrive::try_from_node(&node)?; + Some((hd.partition_signature.to_vec(), hd.signature_type)) + }); + + let (target_sig, target_sig_type) = match target_sig { + Some(s) => s, + None => { + log::error!("expand_device_path: no HardDrive node found in partial path"); + return Err(EfiError::InvalidParameter); + } + }; + + // Collect remaining nodes after the HardDrive node in the partial path. + // Typically this is the FilePath node (e.g., \EFI\Boot\BOOTX64.efi). + let remaining_nodes: Vec<_> = { + let mut past_hd = false; + partial_path + .iter() + .filter(move |node| { + if past_hd && node.header.r#type != DevicePathType::End as u8 { + return true; + } + if HardDrive::try_from_node(node).is_some() { + past_hd = true; + } + false + }) + .collect() + }; - // Get the full device path from the matched handle - // SAFETY: handle_protocol is safe when the handle is valid (from locate_device_path) - // and we're requesting the device path protocol. - let full_dp_ptr = unsafe { boot_services.handle_protocol::(handle) } + // Enumerate all handles with DevicePath protocol + let handles = boot_services + .locate_handle_buffer(HandleSearchType::ByProtocol(&efi::protocols::device_path::PROTOCOL_GUID)) .map_err(EfiError::from)?; - // SAFETY: The device path pointer comes from a valid protocol interface. - let full_path = - unsafe { DevicePath::try_from_ptr(full_dp_ptr as *const _ as *const u8) }.map_err(|_| EfiError::DeviceError)?; + // Search for a handle whose device path contains a matching HardDrive node + for &handle in handles.iter() { + // SAFETY: handle is valid from locate_handle_buffer, requesting device path protocol. + let dp_ptr = match unsafe { boot_services.handle_protocol::(handle) } { + Ok(ptr) => ptr, + Err(_) => continue, + }; - // Combine the full path prefix with the remaining partial path. - // The remaining path (after the matched portion) needs to be appended. - let mut result = DevicePathBuf::from(full_path); + // SAFETY: The device path pointer comes from a valid protocol interface. + let handle_path = match unsafe { DevicePath::try_from_ptr(dp_ptr as *const _ as *const u8) } { + Ok(path) => path, + Err(_) => continue, + }; - // SAFETY: device_path_ptr was updated by locate_device_path to point to the remaining path. - let remaining_path = unsafe { DevicePath::try_from_ptr(device_path_ptr as *const u8) }; - if let Ok(remaining) = remaining_path { - // Only append if there's a meaningful remaining path (not just EndEntire) - if remaining.iter().any(|node| node.header.r#type != DevicePathType::End as u8) { - result.append_device_path(&DevicePathBuf::from(remaining)); + // Walk the handle's device path looking for a matching HardDrive node. + // Collect nodes up to and including the HD node so we truncate any nodes + // beyond HD (e.g., filesystem handles may extend past HD with FilePath nodes + // that would conflict with remaining_nodes from the partial path). + let mut prefix_nodes = Vec::new(); + let mut found = false; + for node in handle_path.iter() { + if node.header.r#type == DevicePathType::End as u8 { + break; + } + let is_match = HardDrive::try_from_node(&node).is_some_and(|hd| { + hd.partition_signature == target_sig.as_slice() && hd.signature_type == target_sig_type + }); + prefix_nodes.push(node); + if is_match { + found = true; + break; + } + } + + if found { + let mut result = DevicePathBuf::from_device_path_node_iter(prefix_nodes.into_iter()); + let remaining_dp = DevicePathBuf::from_device_path_node_iter(remaining_nodes.into_iter()); + result.append_device_path(&remaining_dp); + return Ok(result); } } - Ok(result) + log::error!("expand_device_path: no matching partition found in device topology"); + Err(EfiError::NotFound) } #[cfg(test)] @@ -408,10 +528,12 @@ mod tests { extern crate alloc; extern crate std; + use alloc::boxed::Box; + use super::*; use core::sync::atomic::{AtomicUsize, Ordering}; use patina::{ - boot_services::MockBootServices, + boot_services::{MockBootServices, boxed::BootServicesBox}, device_path::node_defs::{Acpi, EndEntire, HardDrive}, }; @@ -685,103 +807,403 @@ mod tests { } #[test] - fn test_expand_partial_path_success() { - use alloc::boxed::Box; - use patina::device_path::node_defs::FilePath; + fn test_expand_partial_path_locate_fails() { + let partial = build_partial_hd_path([0xBB; 16]); - let guid = [0xAA; 16]; + let mut mock = MockBootServices::new(); - // Create the partial path: HD(1,GPT,)/File(\EFI\BOOT\BOOTX64.EFI) - let mut partial = build_partial_hd_path(guid); - let file_path = - DevicePathBuf::from_device_path_node_iter([FilePath::new("\\EFI\\BOOT\\BOOTX64.EFI")].into_iter()); - partial.append_device_path(&file_path); + // locate_handle_buffer fails — no device path handles available + mock.expect_locate_handle_buffer().returning(|_| Err(efi::Status::NOT_FOUND)); - // Create the full path that the handle will have (ACPI/PCI/HD) - let full_handle_path = build_full_path_with_hd(guid); + let result = expand_device_path(&mock, &partial); + assert!(result.is_err(), "expand_device_path should fail when no handles found"); + } - // Expected result: ACPI/PCI/HD/File (full path + remaining file path) - let mut expected = full_handle_path.clone(); - expected.append_device_path(&file_path); + /// Helper: leaked MockBootServices whose only job is to accept free_pool calls + /// when a BootServicesBox is dropped. + fn leaked_boot_services_for_box() -> &'static MockBootServices { + Box::leak(Box::new({ + let mut m = MockBootServices::new(); + m.expect_free_pool().returning(|_| Ok(())); + m + })) + } - // Clone the device path bytes into a Vec and leak it so we can return a pointer - let path_ref: &DevicePath = full_handle_path.as_ref(); - // SAFETY: path_ref is a valid DevicePath reference and size() returns its exact byte length. - let bytes: alloc::vec::Vec = unsafe { - alloc::vec::Vec::from(core::slice::from_raw_parts(path_ref as *const _ as *const u8, path_ref.size())) - }; - let leaked_bytes = Box::leak(bytes.into_boxed_slice()); - let full_path_ptr: usize = leaked_bytes.as_ptr() as usize; + /// Helper: build a BootServicesBox<[Handle]> for use in test mock returns. + /// + /// Handle storage is intentionally leaked; the mock's free_pool is a no-op. + fn mock_handle_buffer( + handle_addrs: &[usize], + boot_services: &'static MockBootServices, + ) -> BootServicesBox<'static, [efi::Handle], MockBootServices> { + let handles: Vec = handle_addrs.iter().map(|&a| a as efi::Handle).collect(); + let leaked = handles.leak(); + // SAFETY: leaked is a valid pointer+length from Vec::leak. + unsafe { BootServicesBox::from_raw_parts_mut(leaked.as_mut_ptr(), leaked.len(), boot_services) } + } - // Create a fake handle as usize for Send - let fake_handle_addr: usize = 0x12345678; + #[test] + fn test_build_multi_instance_device_path_single_protocol() { + let dp = DevicePathBuf::from_device_path_node_iter([Acpi::new_pci_root(0)].into_iter()); + let dp_addr = dp.as_ref() as *const DevicePath as *const u8 as usize; + let handle_addr: usize = 1; + let box_mock = leaked_boot_services_for_box(); let mut mock = MockBootServices::new(); - // Mock locate_device_path to return the fake handle and update the device path pointer - // to point to the remaining path (the FilePath node) - mock.expect_locate_device_path().returning(move |_protocol, device_path_ptr| { - // The device_path_ptr points to the partial path (HD/File) - // After matching, it should point to the remaining path (File) - // For this test, we'll advance it past the HD node to point at FilePath + mock.expect_locate_handle_buffer().returning(move |_| Ok(mock_handle_buffer(&[handle_addr], box_mock))); - // SAFETY: Test code - we're simulating what locate_device_path does - unsafe { - // Read the current device path to find the HD node size - let current_ptr = *device_path_ptr as *const u8; - let header = current_ptr as *const efi::protocols::device_path::Protocol; - let hd_node_size = u16::from_le_bytes([(*header).length[0], (*header).length[1]]) as usize; + // SAFETY: Test code — returning a pointer to a valid DevicePathBuf kept alive by the test. + unsafe { + mock.expect_handle_protocol::() + .returning(move |_| Ok((dp_addr as *mut efi::protocols::device_path::Protocol).as_mut().unwrap())); + } - // Advance past the HD node to point to FilePath - *device_path_ptr = current_ptr.add(hd_node_size) as *mut efi::protocols::device_path::Protocol; - } - Ok(fake_handle_addr as *mut core::ffi::c_void) - }); + let result = build_multi_instance_device_path(&mock, &[&simple_text_input::PROTOCOL_GUID]); + assert!(result.is_some()); + + let multi = result.unwrap(); + assert_eq!(multi.as_ref().as_bytes(), dp.as_ref().as_bytes()); + } + + #[test] + fn test_build_multi_instance_device_path_deduplicates_handles() { + let dp = DevicePathBuf::from_device_path_node_iter([Acpi::new_pci_root(0)].into_iter()); + let dp_addr = dp.as_ref() as *const DevicePath as *const u8 as usize; + + // Same handle for both protocols — should appear only once + let handle_addr: usize = 1; + let box_mock = leaked_boot_services_for_box(); + + let mut mock = MockBootServices::new(); + + mock.expect_locate_handle_buffer().returning(move |_| Ok(mock_handle_buffer(&[handle_addr], box_mock))); + + // SAFETY: Test code — returning pointer to valid DevicePathBuf. + unsafe { + mock.expect_handle_protocol::() + .times(1) // Called only once despite two GUIDs — handle is deduplicated + .returning(move |_| Ok((dp_addr as *mut efi::protocols::device_path::Protocol).as_mut().unwrap())); + } + + let result = build_multi_instance_device_path( + &mock, + &[&efi::protocols::simple_text_output::PROTOCOL_GUID, &efi::protocols::graphics_output::PROTOCOL_GUID], + ); + assert!(result.is_some()); + } + + #[test] + fn test_build_multi_instance_device_path_handle_protocol_failure() { + let handle_addr: usize = 1; + let box_mock = leaked_boot_services_for_box(); + + let mut mock = MockBootServices::new(); + + mock.expect_locate_handle_buffer().returning(move |_| Ok(mock_handle_buffer(&[handle_addr], box_mock))); + + // handle_protocol fails — handle has no device path + mock.expect_handle_protocol::() + .returning(|_| Err(efi::Status::UNSUPPORTED)); + + let result = build_multi_instance_device_path(&mock, &[&simple_text_input::PROTOCOL_GUID]); + assert!(result.is_none()); + } + + #[test] + fn test_discover_console_devices_sets_variables() { + use patina::runtime_services::MockRuntimeServices; + + let dp = DevicePathBuf::from_device_path_node_iter([Acpi::new_pci_root(0)].into_iter()); + let dp_addr = dp.as_ref() as *const DevicePath as *const u8 as usize; + let handle_addr: usize = 1; + let box_mock = leaked_boot_services_for_box(); + + let mut boot_mock = MockBootServices::new(); + let mut runtime_mock = MockRuntimeServices::new(); + + boot_mock.expect_locate_handle_buffer().returning(move |_| Ok(mock_handle_buffer(&[handle_addr], box_mock))); + + // SAFETY: Test code — returning pointer to valid DevicePathBuf. + unsafe { + boot_mock + .expect_handle_protocol::() + .returning(move |_| Ok((dp_addr as *mut efi::protocols::device_path::Protocol).as_mut().unwrap())); + } + + runtime_mock + .expect_set_variable::>() + .times(3) // ConIn, ConOut, ErrOut + .returning(|_, _, _, _| Ok(())); + + let result = discover_console_devices(&boot_mock, &runtime_mock); + assert!(result.is_ok()); + } + + #[test] + fn test_discover_console_devices_set_variable_failure_is_non_fatal() { + use patina::runtime_services::MockRuntimeServices; + + let dp = DevicePathBuf::from_device_path_node_iter([Acpi::new_pci_root(0)].into_iter()); + let dp_addr = dp.as_ref() as *const DevicePath as *const u8 as usize; + let handle_addr: usize = 1; + let box_mock = leaked_boot_services_for_box(); + + let mut boot_mock = MockBootServices::new(); + let mut runtime_mock = MockRuntimeServices::new(); + + boot_mock.expect_locate_handle_buffer().returning(move |_| Ok(mock_handle_buffer(&[handle_addr], box_mock))); + + // SAFETY: Test code — returning pointer to valid DevicePathBuf. + unsafe { + boot_mock + .expect_handle_protocol::() + .returning(move |_| Ok((dp_addr as *mut efi::protocols::device_path::Protocol).as_mut().unwrap())); + } - // Mock handle_protocol to return the full device path - mock.expect_handle_protocol::().returning(move |_handle| { - // SAFETY: Test code - returning reference to leaked bytes - Ok(unsafe { &mut *(full_path_ptr as *mut efi::protocols::device_path::Protocol) }) + // set_variable fails — should still return Ok + runtime_mock.expect_set_variable::>().returning(|_, _, _, _| Err(efi::Status::OUT_OF_RESOURCES)); + + let result = discover_console_devices(&boot_mock, &runtime_mock); + assert!(result.is_ok(), "set_variable failure should be non-fatal"); + } + + // Tests for connect_all + + #[test] + fn test_connect_all_stabilizes_after_two_iterations() { + let box_mock = leaked_boot_services_for_box(); + let call_count = std::sync::Arc::new(AtomicUsize::new(0)); + let call_count_clone = call_count.clone(); + + let mut mock = MockBootServices::new(); + + // First call returns 1 handle, second returns 2 (new device discovered), + // third returns 2 again (stabilized). + mock.expect_locate_handle_buffer().returning(move |_| { + let n = call_count_clone.fetch_add(1, Ordering::SeqCst); + let count = if n == 0 { 1 } else { 2 }; + let addrs: Vec = (1..=count).collect(); + Ok(mock_handle_buffer(&addrs, box_mock)) }); + mock.expect_connect_controller().returning(|_, _, _, _| Ok(())); + + let result = connect_all(&mock); + assert!(result.is_ok()); + // 3 iterations: 1 handle, 2 handles, 2 handles (stabilized) + assert_eq!(call_count.load(Ordering::SeqCst), 3); + } + + // Tests for detect_hotkey_from_handles + + /// Test extern "efiapi" read_key_stroke that returns F12 then NOT_READY. + extern "efiapi" fn mock_read_key_stroke_f12( + _this: *mut simple_text_input::Protocol, + key: *mut simple_text_input::InputKey, + ) -> efi::Status { + static CALL_COUNT: AtomicUsize = AtomicUsize::new(0); + let n = CALL_COUNT.fetch_add(1, Ordering::SeqCst); + if n == 0 { + // SAFETY: key is a valid pointer provided by the caller. + unsafe { + (*key).scan_code = 0x16; // F12 + (*key).unicode_char = 0; + } + efi::Status::SUCCESS + } else { + efi::Status::NOT_READY + } + } + + /// Test extern "efiapi" read_key_stroke that always returns NOT_READY. + extern "efiapi" fn mock_read_key_stroke_empty( + _this: *mut simple_text_input::Protocol, + _key: *mut simple_text_input::InputKey, + ) -> efi::Status { + efi::Status::NOT_READY + } + + /// Test extern "efiapi" reset (unused, required for struct completeness). + extern "efiapi" fn mock_reset(_this: *mut simple_text_input::Protocol, _extended: efi::Boolean) -> efi::Status { + efi::Status::SUCCESS + } + + #[test] + fn test_detect_hotkey_from_handles_finds_matching_key() { + let mut protocol = simple_text_input::Protocol { + reset: mock_reset, + read_key_stroke: mock_read_key_stroke_f12, + wait_for_key: ptr::null_mut(), + }; + let protocol_addr = &mut protocol as *mut _ as usize; + + let mut mock = MockBootServices::new(); + + // SAFETY: Test code — returning pointer to a valid Protocol kept alive by the test. + unsafe { + mock.expect_handle_protocol::() + .returning(move |_| Ok((protocol_addr as *mut simple_text_input::Protocol).as_mut().unwrap())); + } + + let handle: efi::Handle = 1usize as efi::Handle; + // SAFETY: handle is a test value, mock returns a valid protocol. + let result = unsafe { detect_hotkey_from_handles(&mock, &[handle], 0x16) }; + assert!(result, "F12 hotkey should be detected"); + } + + #[test] + fn test_detect_hotkey_from_handles_no_keys_buffered() { + let mut protocol = simple_text_input::Protocol { + reset: mock_reset, + read_key_stroke: mock_read_key_stroke_empty, + wait_for_key: ptr::null_mut(), + }; + let protocol_addr = &mut protocol as *mut _ as usize; + + let mut mock = MockBootServices::new(); + + // SAFETY: Test code — returning pointer to a valid Protocol kept alive by the test. + unsafe { + mock.expect_handle_protocol::() + .returning(move |_| Ok((protocol_addr as *mut simple_text_input::Protocol).as_mut().unwrap())); + } + + let handle: efi::Handle = 1usize as efi::Handle; + // SAFETY: handle is a test value, mock returns a valid protocol. + let result = unsafe { detect_hotkey_from_handles(&mock, &[handle], 0x16) }; + assert!(!result, "No keys in buffer should return false"); + } + + #[test] + fn test_detect_hotkey_from_handles_protocol_failure() { + let mut mock = MockBootServices::new(); + + // handle_protocol fails — no SimpleTextInput on this handle + mock.expect_handle_protocol::().returning(|_| Err(efi::Status::UNSUPPORTED)); + + let handle: efi::Handle = 1usize as efi::Handle; + // SAFETY: handle is a test value, mock returns Err. + let result = unsafe { detect_hotkey_from_handles(&mock, &[handle], 0x16) }; + assert!(!result, "Protocol failure should return false"); + } + + // Tests for expand_device_path success path + + #[test] + fn test_expand_partial_path_matches_partition() { + let guid = [0xAA; 16]; + let partial = build_partial_hd_path(guid); + let full = build_full_path_with_hd(guid); + let full_addr = full.as_ref() as *const DevicePath as *const u8 as usize; + + let handle_addr: usize = 1; + let box_mock = leaked_boot_services_for_box(); + + let mut mock = MockBootServices::new(); + + mock.expect_locate_handle_buffer().returning(move |_| Ok(mock_handle_buffer(&[handle_addr], box_mock))); + + // SAFETY: Test code — returning pointer to valid DevicePathBuf. + unsafe { + mock.expect_handle_protocol::() + .returning(move |_| Ok((full_addr as *mut efi::protocols::device_path::Protocol).as_mut().unwrap())); + } + let result = expand_device_path(&mock, &partial); - assert!(result.is_ok(), "expand_device_path should succeed"); + assert!(result.is_ok(), "Should find matching partition"); + // The expanded path should start with ACPI root (from the full path) let expanded = result.unwrap(); - assert_eq!(expanded, expected, "Expanded path should match expected full path with file"); + assert!(!is_partial_device_path(&expanded), "Expanded path should be a full path"); + } - // Note: leaked_bytes is intentionally leaked for the test - in tests this is acceptable + #[test] + fn test_expand_partial_path_no_matching_partition() { + let partial = build_partial_hd_path([0xAA; 16]); + // Full path has a different GUID — no match + let full = build_full_path_with_hd([0xBB; 16]); + let full_addr = full.as_ref() as *const DevicePath as *const u8 as usize; + + let handle_addr: usize = 1; + let box_mock = leaked_boot_services_for_box(); + + let mut mock = MockBootServices::new(); + + mock.expect_locate_handle_buffer().returning(move |_| Ok(mock_handle_buffer(&[handle_addr], box_mock))); + + // SAFETY: Test code — returning pointer to valid DevicePathBuf. + unsafe { + mock.expect_handle_protocol::() + .returning(move |_| Ok((full_addr as *mut efi::protocols::device_path::Protocol).as_mut().unwrap())); + } + + let result = expand_device_path(&mock, &partial); + assert!(result.is_err(), "Should fail when no partition matches"); } #[test] - fn test_expand_partial_path_not_found() { - let partial = build_partial_hd_path([0xBB; 16]); + /// Verify that expansion truncates the handle's device path at the matched node, + /// discarding any trailing nodes. This prevents duplication when a handle's path + /// extends past the node we match on (e.g., filesystem handles that include + /// FilePath nodes after HD). The same principle applies to any future partial + /// path types — we must only use the prefix up to the matched node. + fn test_expand_partial_path_truncates_at_matched_node() { + use patina::device_path::node_defs::FilePath; + + let guid = [0xAA; 16]; + // Partial path: HD()/FilePath(\EFI\Boot\BOOTX64.efi) + let mut partial = + DevicePathBuf::from_device_path_node_iter([HardDrive::new_gpt(1, 2048, 1000000, guid)].into_iter()); + let fp = DevicePathBuf::from_device_path_node_iter([FilePath::new("\\EFI\\Boot\\BOOTX64.efi")].into_iter()); + partial.append_device_path(&fp); + + // Handle has nodes beyond the matched node — simulates a handle whose device + // path extends past the point we match on (e.g., a filesystem handle). + // ACPI/PCI/HD()/FilePath(\some\other\path) + let mut handle_dp = build_full_path_with_hd(guid); + let extra_fp = DevicePathBuf::from_device_path_node_iter([FilePath::new("\\some\\other\\path")].into_iter()); + handle_dp.append_device_path(&extra_fp); + + let handle_dp_addr = handle_dp.as_ref() as *const DevicePath as *const u8 as usize; + let handle_addr: usize = 1; + let box_mock = leaked_boot_services_for_box(); let mut mock = MockBootServices::new(); - // Mock locate_device_path to return NOT_FOUND - mock.expect_locate_device_path().returning(|_protocol, _device_path_ptr| Err(efi::Status::NOT_FOUND)); + mock.expect_locate_handle_buffer().returning(move |_| Ok(mock_handle_buffer(&[handle_addr], box_mock))); + + // SAFETY: Test code — returning pointer to valid DevicePathBuf. + unsafe { + mock.expect_handle_protocol::().returning(move |_| { + Ok((handle_dp_addr as *mut efi::protocols::device_path::Protocol).as_mut().unwrap()) + }); + } let result = expand_device_path(&mock, &partial); - assert!(result.is_err(), "expand_device_path should fail when device not found"); + assert!(result.is_ok()); + + let expanded = result.unwrap(); + let node_count = expanded.as_ref().node_count(); + assert_eq!(node_count, 5, "Should have prefix(3) + remaining(1) + End, got {node_count} nodes"); } #[test] - fn test_expand_partial_path_handle_protocol_fails() { - let partial = build_partial_hd_path([0xCC; 16]); - let fake_handle_addr: usize = 0x87654321; + fn test_expand_partial_path_handle_protocol_failure() { + let partial = build_partial_hd_path([0xAA; 16]); + + let handle_addr: usize = 1; + let box_mock = leaked_boot_services_for_box(); let mut mock = MockBootServices::new(); - // Mock locate_device_path to succeed - mock.expect_locate_device_path() - .returning(move |_protocol, _device_path_ptr| Ok(fake_handle_addr as *mut core::ffi::c_void)); + mock.expect_locate_handle_buffer().returning(move |_| Ok(mock_handle_buffer(&[handle_addr], box_mock))); - // Mock handle_protocol to fail + // handle_protocol fails mock.expect_handle_protocol::() - .returning(|_handle| Err(efi::Status::UNSUPPORTED)); + .returning(|_| Err(efi::Status::UNSUPPORTED)); let result = expand_device_path(&mock, &partial); - assert!(result.is_err(), "expand_device_path should fail when handle_protocol fails"); + assert!(result.is_err(), "Should fail when handle_protocol fails"); } } diff --git a/patina_dxe_core/src/lib.rs b/patina_dxe_core/src/lib.rs index 7ffbd2eda..c9de0deb8 100644 --- a/patina_dxe_core/src/lib.rs +++ b/patina_dxe_core/src/lib.rs @@ -538,8 +538,12 @@ impl Core

{ st.checksum_all(); // Install HobList configuration table - config_tables::core_install_configuration_table(patina::guids::HOB_LIST.into_inner(), physical_hob_list, st) - .expect("Unable to create configuration table due to invalid table entry."); + config_tables::core_install_configuration_table( + patina::guids::HOB_LIST.into_inner(), + physical_hob_list, + st, + ) + .expect("Unable to create configuration table due to invalid table entry."); // Install Memory Type Info configuration table. allocator::install_memory_type_info_table(st).expect("Unable to create Memory Type Info Table"); diff --git a/sdk/patina/src/device_path/node_defs.rs b/sdk/patina/src/device_path/node_defs.rs index 3d3d3a0be..e9e8f96c4 100644 --- a/sdk/patina/src/device_path/node_defs.rs +++ b/sdk/patina/src/device_path/node_defs.rs @@ -659,6 +659,16 @@ impl HardDrive { /// Signature type: MBR 32-bit signature pub const SIGNATURE_TYPE_MBR: u8 = 0x01; + /// Try to parse a `HardDrive` from a raw device path node. + /// + /// Returns `None` if the node type/subtype doesn't match or the data is malformed. + pub fn try_from_node(node: &parse_node::UnknownDevicePathNode<'_>) -> Option { + if !Self::is_type(node.header.r#type, node.header.sub_type) { + return None; + } + node.data.pread_with(0, scroll::LE).ok() + } + /// Create a new HardDrive device path node for a GPT partition. /// /// # Arguments diff --git a/sdk/patina/src/device_path/paths.rs b/sdk/patina/src/device_path/paths.rs index 3ec984b26..e5ee37fe1 100644 --- a/sdk/patina/src/device_path/paths.rs +++ b/sdk/patina/src/device_path/paths.rs @@ -214,6 +214,11 @@ impl DevicePath { self.buffer.len() } + /// Return the raw byte representation of the device path. + pub fn as_bytes(&self) -> &[u8] { + &self.buffer + } + /// Return the number of nodes in the device path. pub fn node_count(&self) -> usize { self.iter().count() diff --git a/sdk/patina/src/guids.rs b/sdk/patina/src/guids.rs index ed4ed92dc..fac07170f 100644 --- a/sdk/patina/src/guids.rs +++ b/sdk/patina/src/guids.rs @@ -259,6 +259,19 @@ pub const SMM_COMMUNICATION_PROTOCOL: crate::BinaryGuid = /// ``` pub const ZERO: crate::BinaryGuid = crate::BinaryGuid::from_string("00000000-0000-0000-0000-000000000000"); +/// EFI Global Variable GUID +/// +/// The namespace GUID for UEFI-defined global variables such as `ConIn`, `ConOut`, +/// `ErrOut`, `Boot####`, `BootOrder`, etc. See UEFI Specification Table 3-1. +/// +/// (`8BE4DF61-93CA-11D2-AA0D-00E098032B8C`) +/// ``` +/// # use patina::{Guid, guids::EFI_GLOBAL_VARIABLE}; +/// # assert_eq!("8BE4DF61-93CA-11D2-AA0D-00E098032B8C", format!("{:?}", Guid::from_ref(&EFI_GLOBAL_VARIABLE))); +/// ``` +pub const EFI_GLOBAL_VARIABLE: crate::BinaryGuid = + crate::BinaryGuid::from_string("8BE4DF61-93CA-11D2-AA0D-00E098032B8C"); + /// EFI_HOB_MEMORY_ALLOC_STACK_GUID /// /// Describes the memory stack that is produced by the HOB producer phase and upon which all post From f808a3e7f7f19f3c3d2587a7f6e8bfd522c8bcf7 Mon Sep 17 00:00:00 2001 From: Kat Perez Date: Thu, 26 Mar 2026 10:42:32 -0400 Subject: [PATCH 7/9] [feature/patina-boot] patina_boot: Add connect-dispatch interleaving with DxeServices (#1422) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Interleave controller connection with DXE driver dispatch during device enumeration. Connecting controllers can discover new firmware volumes (e.g., PCI option ROMs) that contain drivers for devices behind that controller. Those drivers must be dispatched before the next round of enumeration, otherwise the devices they serve will not be found. `SimpleBootManager` uses `interleave_connect_and_dispatch()` to alternate between connecting controllers and dispatching newly-discovered drivers until both stabilize. The `DxeDispatch` service trait (from #1421) is consumed via dependency injection. Note: `interleave_connect_and_dispatch()` currently uses `connect_all()` which connects every controller on every round. This is functional but inefficient for platforms with large device topologies — a future optimization could connect only newly-discovered controllers. - [x] Impacts functionality? - [ ] Impacts security? - [ ] Breaking change? - [x] Includes tests? - [ ] Includes documentation? ## How This Was Tested - Built SBSA DXE core binary with `BootDispatcher` + `SimpleBootManager` replacing TianoCore BdsDxe - Booted Windows ARM64 under QEMU SBSA-ref emulation with Patina BDS handling the full boot flow - Verified interleaving: controller connection discovered AHCI device, partial device path expanded to full path, Windows bootloader loaded, ExitBootServices completed ## Integration Instructions Depends on #1421 (`DxeDispatch` service trait) for platform binary integration. Remove TianoCore `BdsDxe.inf` from platform DSC/FDF since the `BootDispatcher` provides the BDS architectural protocol. --- components/patina_boot/README.md | 68 ++++---- components/patina_boot/src/boot_dispatcher.rs | 23 ++- .../patina_boot/src/boot_orchestrator.rs | 7 +- .../src/orchestrators/simple_boot_manager.rs | 150 +++++++++++++++++- 4 files changed, 206 insertions(+), 42 deletions(-) diff --git a/components/patina_boot/README.md b/components/patina_boot/README.md index fdc450dfd..f2439b741 100644 --- a/components/patina_boot/README.md +++ b/components/patina_boot/README.md @@ -1,33 +1,35 @@ -# Patina Boot - -Boot orchestration component for Patina-based firmware implementing UEFI boot manager functionality. - -## Components - -- **BootOrchestrator**: Orchestrates device enumeration, BDS phase events, and boot option execution. -- **ConsoleDiscovery**: Discovers console devices and populates UEFI console variables. - -## Usage - -```rust -use patina_boot::{component::BootOrchestrator, config::BootOptions}; - -// Configure boot options (devices are tried in order) -let config = BootOptions::new() - .with_device(primary_boot_path) - .with_device(fallback_boot_path) - .with_failure_handler(|| { /* handle boot failure */ }); - -// Add BootOrchestrator as a platform component -add.component(BootOrchestrator); -``` - -## Helper Functions - -For custom boot flows, use the helper functions in the `helpers` module: - -- `connect_all()` - Connect all controllers for device enumeration -- `signal_bds_phase_entry()` - Signal EndOfDxe event -- `signal_ready_to_boot()` - Signal ReadyToBoot event -- `discover_console_devices()` - Populate console variables -- `boot_from_device_path()` - Load and start a boot image +# Patina Boot + +Boot orchestration component for Patina-based firmware implementing UEFI boot manager functionality. + +## Components + +- **BootDispatcher**: Installs the BDS architectural protocol and delegates to a `BootOrchestrator` implementation. +- **BootOrchestrator**: Trait for custom boot flows. Platforms implement this to define boot behavior. +- **SimpleBootManager**: Default `BootOrchestrator` for platforms with straightforward boot topologies. + +## Usage + +```rust +use patina_boot::{BootDispatcher, SimpleBootManager, config::BootConfig}; + +// Minimal boot: +let orchestrator = SimpleBootManager::new( + BootConfig::new(nvme_esp_path()) + .with_device(nvme_recovery_path()), +); +add.component(BootDispatcher::new(orchestrator)); + +// Custom orchestrator: +add.component(BootDispatcher::new(MyCustomOrchestrator::new())); +``` + +## Helper Functions + +For custom boot flows, use the helper functions in the `helpers` module: + +- `connect_all()` - Connect all controllers for device enumeration +- `signal_bds_phase_entry()` - Signal EndOfDxe event +- `signal_ready_to_boot()` - Signal ReadyToBoot event +- `discover_console_devices()` - Populate console variables +- `boot_from_device_path()` - Load and start a boot image diff --git a/components/patina_boot/src/boot_dispatcher.rs b/components/patina_boot/src/boot_dispatcher.rs index 139249b74..0eb74bf65 100644 --- a/components/patina_boot/src/boot_dispatcher.rs +++ b/components/patina_boot/src/boot_dispatcher.rs @@ -17,7 +17,11 @@ use core::ffi::c_void; use patina::{ boot_services::{BootServices, StandardBootServices}, - component::{component, params::Handle}, + component::{ + component, + params::Handle, + service::{Service, dxe_dispatch::DxeDispatch}, + }, error::{EfiError, Result}, pi::protocols::bds, runtime_services::StandardRuntimeServices, @@ -29,6 +33,7 @@ use crate::boot_orchestrator::BootOrchestrator; /// Context stored in a static for the BDS protocol callback to access. struct BdsContext { orchestrator: Box, + dxe_dispatch: &'static dyn DxeDispatch, boot_services: StandardBootServices, runtime_services: StandardRuntimeServices, image_handle: r_efi::efi::Handle, @@ -50,6 +55,7 @@ static BDS_CONTEXT: Once = Once::new(); /// This is the single Patina component for driving boot orchestration. It: /// - Accepts a [`BootOrchestrator`] implementation via [`BootDispatcher::new()`] /// - Installs the BDS architectural protocol during component dispatch +/// - Consumes the [`DxeDispatch`] service via dependency injection /// - When the DXE core invokes BDS: delegates to `orchestrator.execute()` /// /// ## Usage @@ -92,6 +98,7 @@ impl BootDispatcher { self, boot_services: StandardBootServices, runtime_services: StandardRuntimeServices, + dxe_dispatch: Service, image_handle: Option, ) -> Result<()> { let handle = image_handle.ok_or_else(|| { @@ -102,6 +109,7 @@ impl BootDispatcher { // Store the orchestrator and services for the BDS callback BDS_CONTEXT.call_once(|| BdsContext { orchestrator: self.orchestrator, + dxe_dispatch: *dxe_dispatch, boot_services: boot_services.clone(), runtime_services: runtime_services.clone(), image_handle: *handle, @@ -136,14 +144,22 @@ extern "efiapi" fn bds_entry_point(_this: *mut bds::Protocol) { panic!("BDS context not initialized — BootDispatcher entry_point was not called"); }; - let Err(e) = context.orchestrator.execute(&context.boot_services, &context.runtime_services, context.image_handle); + let Err(e) = context.orchestrator.execute( + &context.boot_services, + &context.runtime_services, + context.dxe_dispatch, + context.image_handle, + ); panic!("BootOrchestrator::execute() failed: {e:?}"); } #[cfg(test)] mod tests { use super::*; - use patina::{boot_services::StandardBootServices, runtime_services::StandardRuntimeServices}; + use patina::{ + boot_services::StandardBootServices, component::service::dxe_dispatch::DxeDispatch, + runtime_services::StandardRuntimeServices, + }; use r_efi::efi; struct MockOrchestrator; @@ -153,6 +169,7 @@ mod tests { &self, _boot_services: &StandardBootServices, _runtime_services: &StandardRuntimeServices, + _dxe_dispatch: &dyn DxeDispatch, _image_handle: efi::Handle, ) -> core::result::Result { Err(patina::error::EfiError::NotFound) diff --git a/components/patina_boot/src/boot_orchestrator.rs b/components/patina_boot/src/boot_orchestrator.rs index 798e8b812..4132e216b 100644 --- a/components/patina_boot/src/boot_orchestrator.rs +++ b/components/patina_boot/src/boot_orchestrator.rs @@ -11,7 +11,10 @@ //! //! SPDX-License-Identifier: Apache-2.0 //! -use patina::{boot_services::StandardBootServices, error::EfiError, runtime_services::StandardRuntimeServices}; +use patina::{ + boot_services::StandardBootServices, component::service::dxe_dispatch::DxeDispatch, error::EfiError, + runtime_services::StandardRuntimeServices, +}; use r_efi::efi; /// Trait for boot orchestration. @@ -38,6 +41,7 @@ use r_efi::efi; /// &self, /// boot_services: &StandardBootServices, /// runtime_services: &StandardRuntimeServices, +/// dxe_services: &dyn DxeDispatch, /// image_handle: efi::Handle, /// ) -> Result { /// // Custom boot flow... @@ -65,6 +69,7 @@ pub trait BootOrchestrator: Send + Sync + 'static { &self, boot_services: &StandardBootServices, runtime_services: &StandardRuntimeServices, + dxe_services: &dyn DxeDispatch, image_handle: efi::Handle, ) -> Result; } diff --git a/components/patina_boot/src/orchestrators/simple_boot_manager.rs b/components/patina_boot/src/orchestrators/simple_boot_manager.rs index 7250c9ba6..16ea46b94 100644 --- a/components/patina_boot/src/orchestrators/simple_boot_manager.rs +++ b/components/patina_boot/src/orchestrators/simple_boot_manager.rs @@ -23,8 +23,40 @@ use patina::{ }; use r_efi::efi; +use patina::component::service::dxe_dispatch::DxeDispatch; + +use patina::boot_services::BootServices; + use crate::{boot_orchestrator::BootOrchestrator, config::BootConfig, helpers}; +/// Interleave controller connection with DXE driver dispatch. +/// +/// Alternates between connecting all controllers and dispatching newly loaded +/// drivers (e.g., PCI option ROMs) until both the device topology stabilizes +/// and no new drivers are dispatched. +/// +/// This ensures that drivers loaded from firmware volumes during device +/// enumeration (such as PCI option ROM drivers) are dispatched before +/// continuing enumeration, allowing those drivers to bind to newly +/// discovered controllers. +fn interleave_connect_and_dispatch( + boot_services: &B, + dxe_services: &D, +) -> patina::error::Result<()> { + const MAX_ROUNDS: usize = 10; + + for _round in 0..MAX_ROUNDS { + helpers::connect_all(boot_services)?; + if !dxe_services.dispatch()? { + return Ok(()); + } + } + + debug_assert!(false, "connect-dispatch interleaving did not converge after {MAX_ROUNDS} rounds"); + + Ok(()) +} + /// Simple boot manager implementing [`BootOrchestrator`]. /// /// Provides a default boot flow suitable for platforms with straightforward @@ -32,7 +64,7 @@ use crate::{boot_orchestrator::BootOrchestrator, config::BootConfig, helpers}; /// /// ## Boot Flow /// -/// 1. Connect all controllers for device enumeration +/// 1. Interleave controller connection with DXE driver dispatch for device enumeration /// 2. Signal EndOfDxe (security lockdown) /// 3. Discover console devices /// 4. Detect hotkey (if configured); select alternate devices if pressed @@ -78,10 +110,11 @@ impl BootOrchestrator for SimpleBootManager { &self, boot_services: &StandardBootServices, runtime_services: &StandardRuntimeServices, + dxe_dispatch: &dyn DxeDispatch, image_handle: efi::Handle, ) -> Result { - if let Err(e) = helpers::connect_all(boot_services) { - log::error!("connect_all failed: {:?}", e); + if let Err(e) = interleave_connect_and_dispatch(boot_services, dxe_dispatch) { + log::error!("interleave_connect_and_dispatch failed: {:?}", e); } if let Err(e) = helpers::signal_bds_phase_entry(boot_services) { @@ -132,14 +165,121 @@ mod tests { extern crate alloc; use super::*; - use alloc::sync::Arc; + use alloc::{boxed::Box, sync::Arc}; use core::sync::atomic::{AtomicBool, Ordering}; - use patina::device_path::{node_defs::EndEntire, paths::DevicePathBuf}; + use patina::{ + boot_services::{MockBootServices, boxed::BootServicesBox}, + device_path::{node_defs::EndEntire, paths::DevicePathBuf}, + }; fn test_device_path() -> DevicePathBuf { DevicePathBuf::from_device_path_node_iter(core::iter::once(EndEntire)) } + // Tests for interleave_connect_and_dispatch + + struct MockDxeDispatcher { + results: spin::Mutex>>, + } + + impl MockDxeDispatcher { + fn new(results: &[patina::error::Result]) -> Self { + Self { results: spin::Mutex::new(results.iter().cloned().collect()) } + } + } + + impl DxeDispatch for MockDxeDispatcher { + fn dispatch(&self) -> patina::error::Result { + self.results.lock().pop_front().expect("MockDxeDispatcher: unexpected dispatch call") + } + } + + fn leaked_boot_services_for_box() -> &'static MockBootServices { + Box::leak(Box::new({ + let mut m = MockBootServices::new(); + m.expect_free_pool().returning(|_| Ok(())); + m + })) + } + + fn mock_handle_buffer( + handle_addrs: &[usize], + boot_services: &'static MockBootServices, + ) -> BootServicesBox<'static, [efi::Handle], MockBootServices> { + let handles: Vec = handle_addrs.iter().map(|&a| a as efi::Handle).collect(); + let leaked = handles.leak(); + // SAFETY: leaked is a valid pointer+length from Vec::leak. + unsafe { BootServicesBox::from_raw_parts_mut(leaked.as_mut_ptr(), leaked.len(), boot_services) } + } + + #[test] + fn test_interleave_single_round_no_drivers_dispatched() { + let box_mock = leaked_boot_services_for_box(); + let mut boot_mock = MockBootServices::new(); + + boot_mock.expect_locate_handle_buffer().returning(move |_| Ok(mock_handle_buffer(&[1], box_mock))); + boot_mock.expect_connect_controller().returning(|_, _, _, _| Ok(())); + + let dxe_mock = MockDxeDispatcher::new(&[Ok(false)]); + + let result = interleave_connect_and_dispatch(&boot_mock, &dxe_mock); + assert!(result.is_ok()); + } + + #[test] + fn test_interleave_multiple_rounds() { + let box_mock = leaked_boot_services_for_box(); + let mut boot_mock = MockBootServices::new(); + + boot_mock.expect_locate_handle_buffer().returning(move |_| Ok(mock_handle_buffer(&[1], box_mock))); + boot_mock.expect_connect_controller().returning(|_, _, _, _| Ok(())); + + let dxe_mock = MockDxeDispatcher::new(&[Ok(true), Ok(false)]); + + let result = interleave_connect_and_dispatch(&boot_mock, &dxe_mock); + assert!(result.is_ok()); + } + + #[test] + fn test_interleave_connect_failure_propagates() { + let mut boot_mock = MockBootServices::new(); + + boot_mock.expect_locate_handle_buffer().returning(|_| Err(efi::Status::NOT_FOUND)); + + let dxe_mock = MockDxeDispatcher::new(&[]); + + let result = interleave_connect_and_dispatch(&boot_mock, &dxe_mock); + assert!(result.is_err()); + } + + #[test] + fn test_interleave_dispatch_failure_propagates() { + let box_mock = leaked_boot_services_for_box(); + let mut boot_mock = MockBootServices::new(); + + boot_mock.expect_locate_handle_buffer().returning(move |_| Ok(mock_handle_buffer(&[1], box_mock))); + boot_mock.expect_connect_controller().returning(|_, _, _, _| Ok(())); + + let dxe_mock = MockDxeDispatcher::new(&[Err(EfiError::DeviceError)]); + + let result = interleave_connect_and_dispatch(&boot_mock, &dxe_mock); + assert!(result.is_err()); + } + + #[test] + fn test_interleave_stops_at_max_rounds() { + let box_mock = leaked_boot_services_for_box(); + let mut boot_mock = MockBootServices::new(); + + boot_mock.expect_locate_handle_buffer().returning(move |_| Ok(mock_handle_buffer(&[1], box_mock))); + boot_mock.expect_connect_controller().returning(|_, _, _, _| Ok(())); + + let dxe_mock = MockDxeDispatcher::new(&[Ok(true); 10]); + + let result = interleave_connect_and_dispatch(&boot_mock, &dxe_mock); + assert!(result.is_ok()); + } + #[test] fn test_new() { let config = BootConfig::new(test_device_path()).with_hotkey(0x16).with_hotkey_device(test_device_path()); From 73d7c9d1e05af21b7721e82816a817f01fca5344 Mon Sep 17 00:00:00 2001 From: Kat Perez Date: Mon, 20 Apr 2026 19:54:06 -0400 Subject: [PATCH 8/9] [feature/patina-boot] patina_boot: Add discover_boot_options helper (#1447) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Add `discover_boot_options()` helper to `patina_boot::helpers` that reads UEFI `BootOrder` and `Boot####` variables to build a `BootConfig` from standard UEFI boot options. This enables any `BootOrchestrator` implementation that consumes `BootConfig` to use UEFI-compliant boot variables instead of requiring platforms to hardcode device paths. The function: - Reads `BootOrder` to determine boot attempt order - Parses each `Boot####` `EFI_LOAD_OPTION` structure to extract device paths - Filters out inactive boot options (`LOAD_OPTION_ACTIVE`) - Gracefully skips unreadable or malformed entries - Returns a populated `BootConfig` with discovered devices in priority order --- - [ ] Impacts functionality? - [ ] Impacts security? - [ ] Breaking change? - [x] Includes tests? - [ ] Includes documentation? ## How This Was Tested - Unit tests covering: single/multiple boot options, inactive option filtering, unreadable variable handling, truncated load option data, empty BootOrder, and hex variable name generation - Integration tested with patina-dxe-core-qemu `feature/patina-boot` on QEMU Q35 — full boot to UEFI Shell 2.0 ## Integration Instructions Platforms can call `discover_boot_options()` with runtime services to automatically populate a `BootConfig` from UEFI boot variables instead of constructing device paths manually. This works with any `BootOrchestrator` implementation that accepts a `BootConfig`: ```rust let config = discover_boot_options(&runtime_services)?; add.component(BootDispatcher::new(SimpleBootManager::new(config))); ``` --- components/patina_boot/Cargo.toml | 2 + components/patina_boot/src/helpers.rs | 411 ++++++++++++++++++ .../src/orchestrators/simple_boot_manager.rs | 9 +- 3 files changed, 418 insertions(+), 4 deletions(-) diff --git a/components/patina_boot/Cargo.toml b/components/patina_boot/Cargo.toml index b58463042..14ab85e11 100644 --- a/components/patina_boot/Cargo.toml +++ b/components/patina_boot/Cargo.toml @@ -16,6 +16,8 @@ patina = { workspace = true, features = ["unstable-device-path"] } patina_macro = { workspace = true } r-efi = { workspace = true } spin = { workspace = true } +zerocopy = { workspace = true } +zerocopy-derive = { workspace = true } [dev-dependencies] patina = { path = "../../sdk/patina", features = ["mockall", "unstable-device-path"] } diff --git a/components/patina_boot/src/helpers.rs b/components/patina_boot/src/helpers.rs index 6819e4490..21b6ee3d9 100644 --- a/components/patina_boot/src/helpers.rs +++ b/components/patina_boot/src/helpers.rs @@ -523,6 +523,163 @@ pub fn expand_device_path(boot_services: &B, partial_path: &Dev Err(EfiError::NotFound) } +const LOAD_OPTION_ACTIVE: u32 = 0x00000001; + +use zerocopy::FromBytes; +use zerocopy_derive::*; + +#[derive(FromBytes, KnownLayout, Immutable)] +#[repr(C, packed)] +struct LoadOptionHeader { + attributes: zerocopy::little_endian::U32, + file_path_list_length: zerocopy::little_endian::U16, +} + +/// Discover boot options from UEFI `BootOrder` and `Boot####` variables. +/// +/// Reads the `BootOrder` variable to determine boot attempt order, then reads +/// each corresponding `Boot####` variable and parses the `EFI_LOAD_OPTION` +/// structure to extract device paths. Only active boot options are returned. +/// +/// Returns a [`BootConfig`](crate::config::BootConfig) populated with the discovered device paths, or +/// an error if `BootOrder` cannot be read. +pub fn discover_boot_options(runtime_services: &R) -> Result { + let namespace = EFI_GLOBAL_VARIABLE.into_inner(); + + let boot_order_name: Vec = "BootOrder\0".encode_utf16().collect(); + let boot_order_name = boot_order_name.as_slice(); + + let (boot_order_bytes, _attributes): (Vec, u32) = + runtime_services.get_variable(boot_order_name, &namespace, None)?; + + if boot_order_bytes.len() % 2 != 0 || boot_order_bytes.is_empty() { + log::error!("discover_boot_options: invalid BootOrder variable length"); + return Err(EfiError::NotFound); + } + + let boot_order: Vec = + boot_order_bytes.chunks_exact(2).map(|chunk| u16::from_le_bytes([chunk[0], chunk[1]])).collect(); + + let mut device_paths: Vec = Vec::new(); + + for option_number in &boot_order { + let var_name = boot_option_variable_name(*option_number); + + let load_option_bytes = match runtime_services.get_variable::>(&var_name, &namespace, None) { + Ok((bytes, _)) => bytes, + Err(e) => { + log::warn!("discover_boot_options: failed to read Boot{:04X}: {:?}", option_number, e); + continue; + } + }; + + if let Some(device_path_buf) = parse_load_option(&load_option_bytes) { + device_paths.push(device_path_buf); + } + } + + let mut iter = device_paths.into_iter(); + let first = iter.next().ok_or(EfiError::NotFound)?; + let config = iter.fold(super::config::BootConfig::new(first), |config, dp| config.with_device(dp)); + + Ok(config) +} + +/// Build a null-terminated UTF-16 variable name for `Boot####`. +fn boot_option_variable_name(option_number: u16) -> Vec { + let mut name = alloc::format!("Boot{:04X}", option_number).encode_utf16().collect::>(); + name.push(0); + name +} + +/// Validate that all device path node lengths stay within the buffer. +/// +/// Walks the node list checking that each node's Length field doesn't exceed the +/// remaining buffer and that the path terminates with an EndEntire node. +fn validate_device_path_nodes(buffer: &[u8]) -> bool { + const NODE_HEADER_SIZE: usize = 4; + const END_ENTIRE_TYPE: u8 = 0x7F; + const END_ENTIRE_SUBTYPE: u8 = 0xFF; + + let mut pos = 0; + loop { + if pos + NODE_HEADER_SIZE > buffer.len() { + return false; + } + + let node_type = buffer[pos]; + let node_subtype = buffer[pos + 1]; + let node_length = u16::from_le_bytes([buffer[pos + 2], buffer[pos + 3]]) as usize; + + if node_length < NODE_HEADER_SIZE { + return false; + } + if pos + node_length > buffer.len() { + return false; + } + + if node_type == END_ENTIRE_TYPE && node_subtype == END_ENTIRE_SUBTYPE { + return true; + } + + pos += node_length; + } +} + +/// Parse an `EFI_LOAD_OPTION` structure and return the device path if active. +/// +/// EFI_LOAD_OPTION layout: +/// u32 Attributes +/// u16 FilePathListLength +/// [u16] Description (null-terminated UTF-16) +/// [u8] FilePathList (device path, FilePathListLength bytes) +/// [u8] OptionalData (remaining bytes, ignored) +fn parse_load_option(data: &[u8]) -> Option { + let (header, rest) = LoadOptionHeader::read_from_prefix(data).ok()?; + + if header.attributes.get() & LOAD_OPTION_ACTIVE == 0 { + return None; + } + + let file_path_list_length = header.file_path_list_length.get() as usize; + + // Skip past the null-terminated UTF-16 description string + let mut offset = 0; + loop { + if offset + 1 >= rest.len() { + return None; + } + let ch = u16::from_le_bytes([rest[offset], rest[offset + 1]]); + offset += 2; + if ch == 0 { + break; + } + } + + let file_path_end = offset + file_path_list_length; + if file_path_end > rest.len() { + return None; + } + + let file_path_bytes = &rest[offset..file_path_end]; + + if !validate_device_path_nodes(file_path_bytes) { + log::warn!("discover_boot_options: device path node lengths exceed buffer"); + return None; + } + + // SAFETY: file_path_bytes contains a validated device path — all node lengths + // are within the buffer and a terminating EndEntire node is present. + let device_path = unsafe { DevicePath::try_from_ptr(file_path_bytes.as_ptr()) }; + match device_path { + Ok(dp) => Some(DevicePathBuf::from(dp)), + Err(e) => { + log::warn!("discover_boot_options: invalid device path in load option: {}", e); + None + } + } +} + #[cfg(test)] mod tests { extern crate alloc; @@ -1206,4 +1363,258 @@ mod tests { let result = expand_device_path(&mock, &partial); assert!(result.is_err(), "Should fail when handle_protocol fails"); } + + // Tests for discover_boot_options / parse_load_option / boot_option_variable_name + + fn build_load_option(attributes: u32, description: &str, device_path: &DevicePathBuf) -> Vec { + let mut data = Vec::new(); + data.extend_from_slice(&attributes.to_le_bytes()); + let dp_bytes = device_path.as_ref().as_bytes(); + data.extend_from_slice(&(dp_bytes.len() as u16).to_le_bytes()); + for c in description.encode_utf16() { + data.extend_from_slice(&c.to_le_bytes()); + } + data.extend_from_slice(&0u16.to_le_bytes()); // null terminator + data.extend_from_slice(dp_bytes); + data + } + + fn build_boot_order(option_numbers: &[u16]) -> Vec { + option_numbers.iter().flat_map(|n| n.to_le_bytes()).collect() + } + + #[test] + fn test_boot_option_variable_name() { + let name = boot_option_variable_name(0x0001); + let expected: Vec = "Boot0001\0".encode_utf16().collect(); + assert_eq!(name, expected); + } + + #[test] + fn test_boot_option_variable_name_hex() { + let name = boot_option_variable_name(0x00AB); + let expected: Vec = "Boot00AB\0".encode_utf16().collect(); + assert_eq!(name, expected); + } + + #[test] + fn test_parse_load_option_active() { + let dp = create_test_device_path(); + let data = build_load_option(LOAD_OPTION_ACTIVE, "Test", &dp); + let result = parse_load_option(&data); + assert!(result.is_some()); + } + + #[test] + fn test_parse_load_option_inactive() { + let dp = create_test_device_path(); + let data = build_load_option(0, "Test", &dp); + let result = parse_load_option(&data); + assert!(result.is_none()); + } + + #[test] + fn test_parse_load_option_too_short() { + let result = parse_load_option(&[0; 4]); + assert!(result.is_none()); + } + + #[test] + fn test_validate_device_path_nodes_valid() { + // EndEntire node: type=0x7F, subtype=0xFF, length=4 + let buffer = [0x7F, 0xFF, 0x04, 0x00]; + assert!(validate_device_path_nodes(&buffer)); + } + + #[test] + fn test_validate_device_path_nodes_multi_node() { + // HW node (type=1, subtype=1, length=6) + 2 pad bytes + EndEntire + let buffer = [0x01, 0x01, 0x06, 0x00, 0xAA, 0xBB, 0x7F, 0xFF, 0x04, 0x00]; + assert!(validate_device_path_nodes(&buffer)); + } + + #[test] + fn test_validate_device_path_nodes_length_exceeds_buffer() { + // Node claims length=100 but buffer is only 4 bytes + let buffer = [0x01, 0x01, 0x64, 0x00]; + assert!(!validate_device_path_nodes(&buffer)); + } + + #[test] + fn test_validate_device_path_nodes_length_too_small() { + // Node length < 4 (minimum header size) + let buffer = [0x01, 0x01, 0x02, 0x00]; + assert!(!validate_device_path_nodes(&buffer)); + } + + #[test] + fn test_validate_device_path_nodes_no_end_node() { + // Valid node but no EndEntire — runs past buffer + let buffer = [0x01, 0x01, 0x04, 0x00]; + assert!(!validate_device_path_nodes(&buffer)); + } + + #[test] + fn test_validate_device_path_nodes_empty_buffer() { + assert!(!validate_device_path_nodes(&[])); + } + + #[test] + fn test_parse_load_option_malformed_device_path_node_length() { + // Build a load option with a device path whose node claims a huge length + let mut data = Vec::new(); + data.extend_from_slice(&LOAD_OPTION_ACTIVE.to_le_bytes()); // attributes + let fake_dp = [0x01, 0x01, 0xFF, 0x00]; // node type=1, subtype=1, length=255 (OOB) + data.extend_from_slice(&(fake_dp.len() as u16).to_le_bytes()); // file path list length + data.extend_from_slice(&[0x00, 0x00]); // empty description (null terminator) + data.extend_from_slice(&fake_dp); // malformed device path + let result = parse_load_option(&data); + assert!(result.is_none(), "malformed device path node length must be rejected"); + } + + #[test] + fn test_parse_load_option_truncated_description() { + // Active attributes + file path length but no null terminator for description + let mut data = Vec::new(); + data.extend_from_slice(&LOAD_OPTION_ACTIVE.to_le_bytes()); + data.extend_from_slice(&0u16.to_le_bytes()); // file path length + data.extend_from_slice(&[0x41, 0x00]); // 'A' in UTF-16 but no null terminator + let result = parse_load_option(&data); + assert!(result.is_none()); + } + + #[test] + fn test_discover_boot_options_single_option() { + use patina::runtime_services::MockRuntimeServices; + + let dp = create_test_device_path(); + let load_option = build_load_option(LOAD_OPTION_ACTIVE, "Windows", &dp); + let boot_order = build_boot_order(&[0x0001]); + + let mut runtime_mock = MockRuntimeServices::new(); + + runtime_mock.expect_get_variable::>().returning(move |name, _, _| { + if name[0] == 'B' as u16 && name[4] == 'O' as u16 { + Ok((boot_order.clone(), 0)) + } else { + Ok((load_option.clone(), 0)) + } + }); + + let result = discover_boot_options(&runtime_mock); + assert!(result.is_ok()); + assert_eq!(result.unwrap().devices().count(), 1); + } + + #[test] + fn test_discover_boot_options_multiple_options() { + use patina::runtime_services::MockRuntimeServices; + + let dp = create_test_device_path(); + let load_option = build_load_option(LOAD_OPTION_ACTIVE, "Option", &dp); + let boot_order = build_boot_order(&[0x0001, 0x0002, 0x0003]); + + let mut runtime_mock = MockRuntimeServices::new(); + + runtime_mock.expect_get_variable::>().returning(move |name, _, _| { + if name[0] == 'B' as u16 && name[4] == 'O' as u16 { + Ok((boot_order.clone(), 0)) + } else { + Ok((load_option.clone(), 0)) + } + }); + + let result = discover_boot_options(&runtime_mock); + assert!(result.is_ok()); + assert_eq!(result.unwrap().devices().count(), 3); + } + + #[test] + fn test_discover_boot_options_skips_inactive() { + use patina::runtime_services::MockRuntimeServices; + + let dp = create_test_device_path(); + let active = build_load_option(LOAD_OPTION_ACTIVE, "Active", &dp); + let inactive = build_load_option(0, "Inactive", &dp); + let boot_order = build_boot_order(&[0x0001, 0x0002]); + + let call_count = std::sync::Arc::new(AtomicUsize::new(0)); + let call_count_clone = call_count.clone(); + + let mut runtime_mock = MockRuntimeServices::new(); + + runtime_mock.expect_get_variable::>().returning(move |name, _, _| { + if name[0] == 'B' as u16 && name[4] == 'O' as u16 { + Ok((boot_order.clone(), 0)) + } else { + let n = call_count_clone.fetch_add(1, Ordering::SeqCst); + if n == 0 { Ok((active.clone(), 0)) } else { Ok((inactive.clone(), 0)) } + } + }); + + let result = discover_boot_options(&runtime_mock); + assert!(result.is_ok()); + assert_eq!(result.unwrap().devices().count(), 1); + } + + #[test] + fn test_discover_boot_options_boot_order_not_found() { + use patina::runtime_services::MockRuntimeServices; + + let mut runtime_mock = MockRuntimeServices::new(); + + runtime_mock.expect_get_variable::>().returning(|_, _, _| Err(efi::Status::NOT_FOUND)); + + let result = discover_boot_options(&runtime_mock); + assert!(result.is_err()); + } + + #[test] + fn test_discover_boot_options_all_inactive() { + use patina::runtime_services::MockRuntimeServices; + + let dp = create_test_device_path(); + let inactive = build_load_option(0, "Inactive", &dp); + let boot_order = build_boot_order(&[0x0001]); + + let mut runtime_mock = MockRuntimeServices::new(); + + runtime_mock.expect_get_variable::>().returning(move |name, _, _| { + if name[0] == 'B' as u16 && name[4] == 'O' as u16 { + Ok((boot_order.clone(), 0)) + } else { + Ok((inactive.clone(), 0)) + } + }); + + let result = discover_boot_options(&runtime_mock); + assert!(result.is_err()); + } + + #[test] + fn test_discover_boot_options_skips_unreadable_option() { + use patina::runtime_services::MockRuntimeServices; + + let dp = create_test_device_path(); + let active = build_load_option(LOAD_OPTION_ACTIVE, "Good", &dp); + let boot_order = build_boot_order(&[0x0001, 0x0002]); + + let call_count = std::sync::Arc::new(AtomicUsize::new(0)); + let call_count_clone = call_count.clone(); + + let mut runtime_mock = MockRuntimeServices::new(); + + runtime_mock.expect_get_variable::>().returning(move |name, _, _| { + if name[0] == 'B' as u16 && name[4] == 'O' as u16 { + Ok((boot_order.clone(), 0)) + } else { + let n = call_count_clone.fetch_add(1, Ordering::SeqCst); + if n == 0 { Err(efi::Status::NOT_FOUND) } else { Ok((active.clone(), 0)) } + } + }); + + let result = discover_boot_options(&runtime_mock); + assert!(result.is_ok()); + assert_eq!(result.unwrap().devices().count(), 1); + } } diff --git a/components/patina_boot/src/orchestrators/simple_boot_manager.rs b/components/patina_boot/src/orchestrators/simple_boot_manager.rs index 16ea46b94..ae2e3e8d8 100644 --- a/components/patina_boot/src/orchestrators/simple_boot_manager.rs +++ b/components/patina_boot/src/orchestrators/simple_boot_manager.rs @@ -129,10 +129,6 @@ impl BootOrchestrator for SimpleBootManager { let use_hotkey_devices = if let Some(hotkey) = self.config.hotkey() { helpers::detect_hotkey(boot_services, hotkey) } else { false }; - if let Err(e) = helpers::signal_ready_to_boot(boot_services) { - log::error!("signal_ready_to_boot failed: {:?}", e); - } - // Select boot devices based on hotkey detection let boot_devices: Vec<&DevicePathBuf> = if use_hotkey_devices { log::info!("Using alternate boot options (hotkey detected)"); @@ -142,6 +138,11 @@ impl BootOrchestrator for SimpleBootManager { }; for device_path in boot_devices { + // Signal ReadyToBoot before each boot attempt per UEFI 2.11 §3 + if let Err(e) = helpers::signal_ready_to_boot(boot_services) { + log::error!("signal_ready_to_boot failed: {:?}", e); + } + match helpers::boot_from_device_path(boot_services, image_handle, device_path) { Ok(()) => { // Boot image returned control (e.g., EFI application exited). From f057b488052a1f2db42f49576b53b9cd430c44ed Mon Sep 17 00:00:00 2001 From: Kat Perez Date: Wed, 29 Apr 2026 22:00:02 -0400 Subject: [PATCH 9/9] patina_boot: Add NVMe boot-partition write-lock helper Introduces lock_partition_write() in helpers.rs. Resolves the NVMe controller via locate_device_path against EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL and issues NVMe Set Features FID 0x11 (Boot Partition Write Protection Configuration), placing both BP0 and BP1 in "Write Protect Until Power Cycle" state per NVMe Base Spec section 5.27.1.17. Inner FFI dispatch is split into an unsafe helper following the existing detect_hotkey_from_handles pattern; the raw-pointer path is integration- tested rather than mocked. Adds three unit tests covering locate_device_path failure, handle_protocol failure, and the CDW10/CDW11 encoding contract. Closes #61. --- components/patina_boot/src/lib.rs | 1 + components/patina_boot/src/partition.rs | 363 ++++++++++++++++++++++++ cspell.yml | 1 + 3 files changed, 365 insertions(+) create mode 100644 components/patina_boot/src/partition.rs diff --git a/components/patina_boot/src/lib.rs b/components/patina_boot/src/lib.rs index 685828fcd..b18a9930c 100644 --- a/components/patina_boot/src/lib.rs +++ b/components/patina_boot/src/lib.rs @@ -35,6 +35,7 @@ pub mod boot_orchestrator; pub mod config; pub mod helpers; pub mod orchestrators; +pub mod partition; pub use boot_dispatcher::BootDispatcher; pub use boot_orchestrator::BootOrchestrator; diff --git a/components/patina_boot/src/partition.rs b/components/patina_boot/src/partition.rs new file mode 100644 index 000000000..1b7a6dafb --- /dev/null +++ b/components/patina_boot/src/partition.rs @@ -0,0 +1,363 @@ +//! Partition I/O helpers for boot orchestrators. +//! +//! Functions in this module operate on partitions identified by a `DevicePathBuf` and use the +//! UEFI protocols already published by the platform's storage stack. They are intended to be +//! called from platforms implementing custom boot flows. +//! +//! ## License +//! +//! Copyright (c) Microsoft Corporation. +//! +//! SPDX-License-Identifier: Apache-2.0 +//! +use core::ptr; + +use patina::{ + boot_services::BootServices, + device_path::paths::DevicePathBuf, + error::{EfiError, Result}, +}; +use r_efi::efi; + +/// Write-protect the NVMe boot partition addressed by `device_path` until the next power cycle. +/// +/// Resolves the controller handle by walking `device_path` for the NVMe Pass-Thru protocol, +/// then issues an NVMe Set Features admin command for FID 0x11 (Boot Partition Write Protection +/// Configuration). Both BP0 and BP1 are placed in "Write Protect Until Power Cycle" state (001b). +/// +/// The lock is volatile: a controller reset or power cycle clears it. +/// +/// # Arguments +/// +/// * `boot_services` - Boot services interface +/// * `device_path` - Device path resolving to (or descending from) an NVMe controller. The path +/// is consumed for protocol lookup only; partition or namespace nodes are tolerated. +/// +/// # Returns +/// +/// Returns `Ok(())` once the controller acknowledges the Set Features command. Returns an error +/// if no NVMe Pass-Thru protocol is reachable on the path, or if the controller rejects the +/// command. +pub fn lock_partition_write(boot_services: &B, device_path: &DevicePathBuf) -> Result<()> { + let mut path_ptr = device_path.as_ref() as *const _ as *mut efi::protocols::device_path::Protocol; + + // SAFETY: path_ptr points into a valid DevicePathBuf for the duration of this call. + let handle = unsafe { boot_services.locate_device_path(&nvme_pass_thru::PROTOCOL_GUID, &mut path_ptr) } + .map_err(EfiError::from)?; + + // SAFETY: handle was returned by locate_device_path for the NVMe Pass-Thru GUID. + let protocol = unsafe { + boot_services.handle_protocol_unchecked(handle, &nvme_pass_thru::PROTOCOL_GUID).map_err(EfiError::from)? + } as *mut nvme_pass_thru::Protocol; + + // SAFETY: protocol is a non-null, properly aligned pointer to a Protocol owned by the controller. + unsafe { lock_partition_write_inner(protocol) } +} + +/// Issue the NVMe Set Features admin command for BPWPS via the supplied Pass-Thru protocol. +/// +/// Separated from `lock_partition_write` because it dereferences a raw protocol pointer and +/// invokes its function pointer directly. Tests use mock protocol function pointers to exercise +/// the dispatch path. +/// +/// # Safety +/// +/// `protocol` must be a valid, non-null pointer to an `EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL` +/// instance owned by an NVMe controller for the duration of this call. +unsafe fn lock_partition_write_inner(protocol: *mut nvme_pass_thru::Protocol) -> Result<()> { + use nvme_pass_thru::{ + BPWPS_LOCK_BP0_BP1, CMD_FLAG_CDW10_VALID, CMD_FLAG_CDW11_VALID, Command, CommandPacket, Completion, + FID_BOOT_PARTITION_WRITE_PROTECTION, OPCODE_SET_FEATURES, QUEUE_TYPE_ADMIN, TIMEOUT_NS_1_SEC, + }; + + let mut nvme_cmd = Command { cdw0: OPCODE_SET_FEATURES as u32, ..Command::zero() }; + nvme_cmd.flags = CMD_FLAG_CDW10_VALID | CMD_FLAG_CDW11_VALID; + nvme_cmd.cdw10 = FID_BOOT_PARTITION_WRITE_PROTECTION as u32; + nvme_cmd.cdw11 = BPWPS_LOCK_BP0_BP1; + + let mut completion = Completion::zero(); + + let mut packet = CommandPacket { + command_timeout: TIMEOUT_NS_1_SEC, + transfer_buffer: ptr::null_mut(), + transfer_length: 0, + metadata_buffer: ptr::null_mut(), + metadata_length: 0, + queue_type: QUEUE_TYPE_ADMIN, + nvme_cmd: &mut nvme_cmd, + nvme_completion: &mut completion, + }; + + // SAFETY: caller guarantees `protocol` is valid; packet pointers are kept alive across the call. + let pass_thru = unsafe { (*protocol).pass_thru }; + let status = pass_thru(protocol, 0, &mut packet, ptr::null_mut()); + if status != efi::Status::SUCCESS { + return Err(EfiError::from(status)); + } + + // NVMe completion DW3 carries Status Field in bits 31:17. Non-zero indicates the controller + // rejected the command (e.g., feature unsupported or BP already permanently locked). + let status_field = (completion.dw3 >> 17) & 0x7FFF; + if status_field != 0 { + log::error!("NVMe Set Features BPWPS rejected: status field {:#x}", status_field); + return Err(EfiError::from(efi::Status::DEVICE_ERROR)); + } + + Ok(()) +} + +/// Minimal FFI bindings for `EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL`. +mod nvme_pass_thru { + use core::ffi::c_void; + use r_efi::efi; + + pub const PROTOCOL_GUID: efi::Guid = + efi::Guid::from_fields(0x52c78312, 0x8edc, 0x4233, 0x98, 0xf2, &[0x1a, 0x1a, 0xa5, 0xe3, 0x88, 0xa5]); + + pub const OPCODE_SET_FEATURES: u8 = 0x09; + pub const FID_BOOT_PARTITION_WRITE_PROTECTION: u8 = 0x11; + + pub const CMD_FLAG_CDW10_VALID: u8 = 1 << 2; + pub const CMD_FLAG_CDW11_VALID: u8 = 1 << 3; + + pub const QUEUE_TYPE_ADMIN: u8 = 0; + + /// 1-second timeout in 100-ns units, per the protocol contract. + pub const TIMEOUT_NS_1_SEC: u64 = 10_000_000; + + /// CDW11 value placing both BP0 and BP1 in "Write Protect Until Power Cycle" (state 001b). + /// Layout: BP0WPS in bits 2:0, BP1WPS in bits 5:3. + pub const BPWPS_LOCK_BP0_BP1: u32 = (0b001 << 3) | 0b001; + + pub type PassThruFn = extern "efiapi" fn( + this: *mut Protocol, + namespace_id: u32, + packet: *mut CommandPacket, + event: *mut c_void, + ) -> efi::Status; + + #[repr(C)] + pub struct Protocol { + pub mode: *mut c_void, + pub pass_thru: PassThruFn, + pub get_next_namespace: *mut c_void, + pub build_device_path: *mut c_void, + pub get_namespace: *mut c_void, + } + + #[repr(C)] + pub struct CommandPacket { + pub command_timeout: u64, + pub transfer_buffer: *mut c_void, + pub transfer_length: u32, + pub metadata_buffer: *mut c_void, + pub metadata_length: u32, + pub queue_type: u8, + pub nvme_cmd: *mut Command, + pub nvme_completion: *mut Completion, + } + + #[repr(C)] + #[derive(Copy, Clone)] + pub struct Command { + pub cdw0: u32, + pub flags: u8, + pub nsid: u32, + pub cdw2: u32, + pub cdw3: u32, + pub cdw10: u32, + pub cdw11: u32, + pub cdw12: u32, + pub cdw13: u32, + pub cdw14: u32, + pub cdw15: u32, + } + + impl Command { + pub const fn zero() -> Self { + Self { + cdw0: 0, + flags: 0, + nsid: 0, + cdw2: 0, + cdw3: 0, + cdw10: 0, + cdw11: 0, + cdw12: 0, + cdw13: 0, + cdw14: 0, + cdw15: 0, + } + } + } + + #[repr(C)] + #[derive(Copy, Clone)] + pub struct Completion { + pub dw0: u32, + pub dw1: u32, + pub dw2: u32, + pub dw3: u32, + } + + impl Completion { + pub const fn zero() -> Self { + Self { dw0: 0, dw1: 0, dw2: 0, dw3: 0 } + } + } +} + +#[cfg(test)] +mod tests { + extern crate std; + + use super::*; + use core::sync::atomic::{AtomicUsize, Ordering}; + use patina::{ + boot_services::MockBootServices, + device_path::{node_defs::EndEntire, paths::DevicePathBuf}, + }; + + fn create_test_device_path() -> DevicePathBuf { + DevicePathBuf::from_device_path_node_iter(core::iter::once(EndEntire)) + } + + #[test] + fn test_lock_partition_write_locate_failure() { + let device_path = create_test_device_path(); + let mut mock = MockBootServices::new(); + + mock.expect_locate_device_path().returning(|_, _| Err(efi::Status::NOT_FOUND)); + + let result = lock_partition_write(&mock, &device_path); + assert!(result.is_err(), "missing NVMe Pass-Thru on path must surface as Err"); + } + + #[test] + fn test_lock_partition_write_handle_protocol_failure() { + let device_path = create_test_device_path(); + let mut mock = MockBootServices::new(); + + let handle_addr: usize = 1; + mock.expect_locate_device_path().returning(move |_, _| Ok(handle_addr as efi::Handle)); + mock.expect_handle_protocol_unchecked().returning(|_, _| Err(efi::Status::UNSUPPORTED)); + + let result = lock_partition_write(&mock, &device_path); + assert!(result.is_err(), "unsupported protocol on located handle must surface as Err"); + } + + #[test] + fn test_lock_partition_write_set_features_payload() { + use super::nvme_pass_thru::{ + BPWPS_LOCK_BP0_BP1, FID_BOOT_PARTITION_WRITE_PROTECTION, OPCODE_SET_FEATURES, QUEUE_TYPE_ADMIN, + }; + + assert_eq!(OPCODE_SET_FEATURES, 0x09, "Set Features admin opcode"); + assert_eq!(FID_BOOT_PARTITION_WRITE_PROTECTION, 0x11, "BPWPS feature identifier"); + assert_eq!(QUEUE_TYPE_ADMIN, 0, "admin queue selector"); + assert_eq!(BPWPS_LOCK_BP0_BP1, 0x09, "CDW11 must encode BP0WPS=001b in bits 2:0 and BP1WPS=001b in bits 5:3"); + } + + // Tests for lock_partition_write_inner — exercise the unsafe FFI dispatch path with mock + // protocol function pointers, mirroring the detect_hotkey_from_handles test pattern. + + static CAPTURED_PASS_THRU_OPCODE: AtomicUsize = AtomicUsize::new(0); + static CAPTURED_PASS_THRU_FLAGS: AtomicUsize = AtomicUsize::new(0); + static CAPTURED_PASS_THRU_CDW10: AtomicUsize = AtomicUsize::new(0); + static CAPTURED_PASS_THRU_CDW11: AtomicUsize = AtomicUsize::new(0); + static CAPTURED_PASS_THRU_QUEUE_TYPE: AtomicUsize = AtomicUsize::new(0); + + extern "efiapi" fn mock_pass_thru_capture_success( + _this: *mut nvme_pass_thru::Protocol, + _namespace_id: u32, + packet: *mut nvme_pass_thru::CommandPacket, + _event: *mut core::ffi::c_void, + ) -> efi::Status { + // SAFETY: caller (helper) constructs a valid CommandPacket whose nvme_cmd points to a + // valid Command. Test storage of captured values is single-threaded. + unsafe { + let pkt = &*packet; + let cmd = &*pkt.nvme_cmd; + CAPTURED_PASS_THRU_OPCODE.store((cmd.cdw0 & 0xFF) as usize, Ordering::SeqCst); + CAPTURED_PASS_THRU_FLAGS.store(cmd.flags as usize, Ordering::SeqCst); + CAPTURED_PASS_THRU_CDW10.store(cmd.cdw10 as usize, Ordering::SeqCst); + CAPTURED_PASS_THRU_CDW11.store(cmd.cdw11 as usize, Ordering::SeqCst); + CAPTURED_PASS_THRU_QUEUE_TYPE.store(pkt.queue_type as usize, Ordering::SeqCst); + } + efi::Status::SUCCESS + } + + extern "efiapi" fn mock_pass_thru_returns_error( + _this: *mut nvme_pass_thru::Protocol, + _namespace_id: u32, + _packet: *mut nvme_pass_thru::CommandPacket, + _event: *mut core::ffi::c_void, + ) -> efi::Status { + efi::Status::DEVICE_ERROR + } + + extern "efiapi" fn mock_pass_thru_nonzero_completion_status( + _this: *mut nvme_pass_thru::Protocol, + _namespace_id: u32, + packet: *mut nvme_pass_thru::CommandPacket, + _event: *mut core::ffi::c_void, + ) -> efi::Status { + // SAFETY: caller-provided pointers are valid for the lifetime of the call. + unsafe { + let pkt = &*packet; + (*pkt.nvme_completion).dw3 = 0x2 << 17; // Status Field = 0x2 (Invalid Field in Command) + } + efi::Status::SUCCESS + } + + #[test] + fn test_lock_partition_write_inner_issues_correct_set_features_command() { + let mut protocol = nvme_pass_thru::Protocol { + mode: ptr::null_mut(), + pass_thru: mock_pass_thru_capture_success, + get_next_namespace: ptr::null_mut(), + build_device_path: ptr::null_mut(), + get_namespace: ptr::null_mut(), + }; + + // SAFETY: protocol is a valid Protocol kept alive on the test stack. + let result = unsafe { lock_partition_write_inner(&mut protocol) }; + + assert!(result.is_ok(), "successful pass-thru must produce Ok"); + assert_eq!(CAPTURED_PASS_THRU_OPCODE.load(Ordering::SeqCst), 0x09, "Set Features opcode"); + assert_eq!(CAPTURED_PASS_THRU_FLAGS.load(Ordering::SeqCst), 0b0000_1100, "CDW10 + CDW11 marked valid"); + assert_eq!(CAPTURED_PASS_THRU_CDW10.load(Ordering::SeqCst), 0x11, "FID = BPWPS"); + assert_eq!(CAPTURED_PASS_THRU_CDW11.load(Ordering::SeqCst), 0x09, "BP0WPS=001b, BP1WPS=001b"); + assert_eq!(CAPTURED_PASS_THRU_QUEUE_TYPE.load(Ordering::SeqCst), 0, "admin queue"); + } + + #[test] + fn test_lock_partition_write_inner_passthru_failure_propagates() { + let mut protocol = nvme_pass_thru::Protocol { + mode: ptr::null_mut(), + pass_thru: mock_pass_thru_returns_error, + get_next_namespace: ptr::null_mut(), + build_device_path: ptr::null_mut(), + get_namespace: ptr::null_mut(), + }; + + // SAFETY: protocol is a valid Protocol kept alive on the test stack. + let result = unsafe { lock_partition_write_inner(&mut protocol) }; + assert!(result.is_err(), "pass-thru DEVICE_ERROR must surface as Err"); + } + + #[test] + fn test_lock_partition_write_inner_nonzero_completion_status_rejected() { + let mut protocol = nvme_pass_thru::Protocol { + mode: ptr::null_mut(), + pass_thru: mock_pass_thru_nonzero_completion_status, + get_next_namespace: ptr::null_mut(), + build_device_path: ptr::null_mut(), + get_namespace: ptr::null_mut(), + }; + + // SAFETY: protocol is a valid Protocol kept alive on the test stack. + let result = unsafe { lock_partition_write_inner(&mut protocol) }; + assert!(result.is_err(), "non-zero completion status field must surface as Err"); + } +} diff --git a/cspell.yml b/cspell.yml index 83d6ac0e6..cb0b3a10f 100644 --- a/cspell.yml +++ b/cspell.yml @@ -43,6 +43,7 @@ words: - bitvec - bootaa - bootx + - bpwps - bucketize - bumpalo - cbindgen