From dd87436e653cff45df91e467b4b8e252d5b461d6 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Sun, 11 Aug 2024 12:43:37 +0200 Subject: [PATCH 1/6] uefi: add BootPolicy type This type is used in three functions of the UEFI spec. Having an enum instead of a boolean simplifies the interface as the variants can be properly documented. --- uefi/src/proto/boot_policy.rs | 107 ++++++++++++++++++++++++++++++++++ uefi/src/proto/mod.rs | 4 ++ 2 files changed, 111 insertions(+) create mode 100644 uefi/src/proto/boot_policy.rs diff --git a/uefi/src/proto/boot_policy.rs b/uefi/src/proto/boot_policy.rs new file mode 100644 index 000000000..f11d8db39 --- /dev/null +++ b/uefi/src/proto/boot_policy.rs @@ -0,0 +1,107 @@ +//! Module for the [`BootPolicy`] helper type. + +use core::fmt::{Display, Formatter}; + +/// Errors that can happen when working with [`BootPolicy`]. +#[derive(Debug, Copy, Clone, PartialOrd, PartialEq, Eq, Ord)] +pub enum BootPolicyError { + /// Only `0` and `1` are valid integers, all other values are undefined. + InvalidInteger(u8), +} + +impl Display for BootPolicyError { + fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result { + let s = match self { + Self::InvalidInteger(_) => { + "Only `0` and `1` are valid integers, all other values are undefined." + } + }; + f.write_str(s) + } +} + +#[cfg(feature = "unstable")] +impl core::error::Error for BootPolicyError {} + +/// The UEFI boot policy is a property that influences the behaviour of +/// various UEFI functions that load files (typically UEFI images). +/// +/// This type is not ABI compatible. On the ABI level, this is an UEFI +/// boolean. +#[derive(Debug, Copy, Clone, PartialOrd, PartialEq, Eq, Ord)] +pub enum BootPolicy { + /// Indicates that the request originates from the boot manager, and that + /// the boot manager is attempting to load the provided `file_path` as a + /// boot selection. + /// + /// Boot selection refers to what a user has chosen in the (GUI) boot menu. + /// + /// This corresponds to the `TRUE` value in the UEFI spec. + BootSelection, + /// The provided `file_path` must match an exact file to be loaded. + /// + /// This corresponds to the `FALSE` value in the UEFI spec. + ExactMatch, +} + +impl From for bool { + fn from(value: BootPolicy) -> Self { + match value { + BootPolicy::BootSelection => true, + BootPolicy::ExactMatch => false, + } + } +} + +impl From for BootPolicy { + fn from(value: bool) -> Self { + match value { + true => Self::BootSelection, + false => Self::ExactMatch, + } + } +} + +impl From for u8 { + fn from(value: BootPolicy) -> Self { + match value { + BootPolicy::BootSelection => 1, + BootPolicy::ExactMatch => 0, + } + } +} + +impl TryFrom for BootPolicy { + type Error = BootPolicyError; + fn try_from(value: u8) -> Result { + match value { + 0 => Ok(Self::ExactMatch), + 1 => Ok(Self::BootSelection), + err => Err(Self::Error::InvalidInteger(err)), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn boot_policy() { + assert_eq!(bool::from(BootPolicy::ExactMatch), false); + assert_eq!(bool::from(BootPolicy::BootSelection), true); + + assert_eq!(BootPolicy::from(false), BootPolicy::ExactMatch); + assert_eq!(BootPolicy::from(true), BootPolicy::BootSelection); + + assert_eq!(u8::from(BootPolicy::ExactMatch), 0); + assert_eq!(u8::from(BootPolicy::BootSelection), 1); + + assert_eq!(BootPolicy::try_from(0), Ok(BootPolicy::ExactMatch)); + assert_eq!(BootPolicy::try_from(1), Ok(BootPolicy::BootSelection)); + assert_eq!( + BootPolicy::try_from(2), + Err(BootPolicyError::InvalidInteger(2)) + ); + } +} diff --git a/uefi/src/proto/mod.rs b/uefi/src/proto/mod.rs index 5919b7afc..510114f7a 100644 --- a/uefi/src/proto/mod.rs +++ b/uefi/src/proto/mod.rs @@ -9,6 +9,8 @@ //! //! [`BootServices`]: crate::table::boot::BootServices#accessing-protocols +pub use boot_policy::BootPolicy; + use crate::Identify; use core::ffi::c_void; @@ -81,3 +83,5 @@ pub mod shell_params; pub mod shim; pub mod string; pub mod tcg; + +mod boot_policy; From a7f0189e2614e02eeda4631efbb7fe7b1556380f Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Tue, 13 Aug 2024 14:37:12 +0200 Subject: [PATCH 2/6] uefi: boot policy: reorder imports and modules - public modules - private modules - public uses - private uses This is consistent with other parts of the code. --- uefi/src/proto/mod.rs | 41 ++++++++++++++++++++--------------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/uefi/src/proto/mod.rs b/uefi/src/proto/mod.rs index 510114f7a..218f3ffb0 100644 --- a/uefi/src/proto/mod.rs +++ b/uefi/src/proto/mod.rs @@ -9,7 +9,26 @@ //! //! [`BootServices`]: crate::table::boot::BootServices#accessing-protocols -pub use boot_policy::BootPolicy; +pub mod console; +pub mod debug; +pub mod device_path; +pub mod driver; +pub mod loaded_image; +pub mod media; +pub mod misc; +pub mod network; +pub mod pi; +pub mod rng; +pub mod security; +pub mod shell_params; +pub mod shim; +pub mod string; +pub mod tcg; + +mod boot_policy; + +pub use boot_policy::{BootPolicy, BootPolicyError}; +pub use uefi_macros::unsafe_protocol; use crate::Identify; use core::ffi::c_void; @@ -65,23 +84,3 @@ where ptr.cast::() } } - -pub use uefi_macros::unsafe_protocol; - -pub mod console; -pub mod debug; -pub mod device_path; -pub mod driver; -pub mod loaded_image; -pub mod media; -pub mod misc; -pub mod network; -pub mod pi; -pub mod rng; -pub mod security; -pub mod shell_params; -pub mod shim; -pub mod string; -pub mod tcg; - -mod boot_policy; From 19b20cf5f2e36177ea82a93644a3925d713d8aab Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Tue, 13 Aug 2024 15:11:28 +0200 Subject: [PATCH 3/6] uefi: LoadImageSource: use `BootPolicy` This adds the new `BootPolicy` type into `LoadImageSource`. --- uefi-test-runner/src/bin/shell_launcher.rs | 3 ++- uefi-test-runner/src/boot/mod.rs | 3 ++- uefi/CHANGELOG.md | 2 ++ uefi/src/boot.rs | 14 ++++++------ uefi/src/table/boot.rs | 25 ++++++++++------------ 5 files changed, 24 insertions(+), 23 deletions(-) diff --git a/uefi-test-runner/src/bin/shell_launcher.rs b/uefi-test-runner/src/bin/shell_launcher.rs index 4b3310eda..a087293ad 100644 --- a/uefi-test-runner/src/bin/shell_launcher.rs +++ b/uefi-test-runner/src/bin/shell_launcher.rs @@ -18,6 +18,7 @@ use uefi::prelude::*; use uefi::proto::device_path::build::{self, DevicePathBuilder}; use uefi::proto::device_path::{DevicePath, DeviceSubType, DeviceType, LoadedImageDevicePath}; use uefi::proto::loaded_image::LoadedImage; +use uefi::proto::BootPolicy; /// Get the device path of the shell app. This is the same as the /// currently-loaded image's device path, but with the file path part changed. @@ -53,7 +54,7 @@ fn efi_main() -> Status { boot::image_handle(), LoadImageSource::FromDevicePath { device_path: shell_image_path, - from_boot_manager: false, + boot_policy: BootPolicy::ExactMatch, }, ) .expect("failed to load shell app"); diff --git a/uefi-test-runner/src/boot/mod.rs b/uefi-test-runner/src/boot/mod.rs index 86b524070..512b6ab98 100644 --- a/uefi-test-runner/src/boot/mod.rs +++ b/uefi-test-runner/src/boot/mod.rs @@ -3,6 +3,7 @@ use uefi::fs::FileSystem; use uefi::proto::console::text::Output; use uefi::proto::device_path::media::FilePath; use uefi::proto::device_path::{DevicePath, LoadedImageDevicePath}; +use uefi::proto::BootPolicy; use uefi::table::boot::{BootServices, LoadImageSource, SearchType}; use uefi::table::{Boot, SystemTable}; use uefi::{boot, CString16, Identify}; @@ -122,7 +123,7 @@ fn test_load_image(bt: &BootServices) { { let load_source = LoadImageSource::FromDevicePath { device_path: image_device_path, - from_boot_manager: false, + boot_policy: BootPolicy::ExactMatch, }; let _ = bt .load_image(bt.image_handle(), load_source) diff --git a/uefi/CHANGELOG.md b/uefi/CHANGELOG.md index 00f0cac26..2fa8275b5 100644 --- a/uefi/CHANGELOG.md +++ b/uefi/CHANGELOG.md @@ -43,6 +43,8 @@ details of a significant change to the API in this release. > use uefi::mem::memory_map::{MemoryMap, MemoryMapMut, MemoryType}; > use uefi::table::boot::BootServices; ``` +- **Breaking:** Added a new `BootPolicy` type which breaks existing usages + of `LoadImageSource`. [funcmigrate]: ../docs/funcs_migration.md diff --git a/uefi/src/boot.rs b/uefi/src/boot.rs index 013240309..bfa339ca6 100644 --- a/uefi/src/boot.rs +++ b/uefi/src/boot.rs @@ -21,11 +21,11 @@ use core::{mem, slice}; use uefi::{table, Char16, Error, Event, Guid, Handle, Result, Status, StatusExt}; use uefi_raw::table::boot::InterfaceType; -#[cfg(feature = "alloc")] -use {alloc::vec::Vec, uefi::ResultExt}; - #[cfg(doc)] use crate::proto::device_path::LoadedImageDevicePath; +use uefi::proto::BootPolicy; +#[cfg(feature = "alloc")] +use {alloc::vec::Vec, uefi::ResultExt}; pub use uefi::table::boot::{ AllocateType, EventNotifyFn, LoadImageSource, OpenProtocolAttributes, OpenProtocolParams, @@ -1002,7 +1002,7 @@ pub fn load_image(parent_image_handle: Handle, source: LoadImageSource) -> Resul match source { LoadImageSource::FromBuffer { buffer, file_path } => { // Boot policy is ignored when loading from source buffer. - boot_policy = 0; + boot_policy = BootPolicy::ExactMatch; device_path = file_path.map(|p| p.as_ffi_ptr()).unwrap_or(ptr::null()); source_buffer = buffer.as_ptr(); @@ -1010,9 +1010,9 @@ pub fn load_image(parent_image_handle: Handle, source: LoadImageSource) -> Resul } LoadImageSource::FromDevicePath { device_path: file_path, - from_boot_manager, + boot_policy: new_boot_policy, } => { - boot_policy = u8::from(from_boot_manager); + boot_policy = new_boot_policy; device_path = file_path.as_ffi_ptr(); source_buffer = ptr::null(); source_size = 0; @@ -1022,7 +1022,7 @@ pub fn load_image(parent_image_handle: Handle, source: LoadImageSource) -> Resul let mut image_handle = ptr::null_mut(); unsafe { (bt.load_image)( - boot_policy, + boot_policy.into(), parent_image_handle.as_ptr(), device_path.cast(), source_buffer, diff --git a/uefi/src/table/boot.rs b/uefi/src/table/boot.rs index ed35ae922..c34d04303 100644 --- a/uefi/src/table/boot.rs +++ b/uefi/src/table/boot.rs @@ -10,7 +10,7 @@ use crate::mem::memory_map::*; use crate::proto::device_path::DevicePath; use crate::proto::loaded_image::LoadedImage; use crate::proto::media::fs::SimpleFileSystem; -use crate::proto::{Protocol, ProtocolPointer}; +use crate::proto::{BootPolicy, Protocol, ProtocolPointer}; use crate::util::opt_nonnull_to_ptr; use crate::{Char16, Error, Event, Guid, Handle, Result, Status, StatusExt}; #[cfg(feature = "alloc")] @@ -848,7 +848,7 @@ impl BootServices { match source { LoadImageSource::FromBuffer { buffer, file_path } => { // Boot policy is ignored when loading from source buffer. - boot_policy = 0; + boot_policy = BootPolicy::ExactMatch; device_path = file_path.map(|p| p.as_ffi_ptr()).unwrap_or(ptr::null()); source_buffer = buffer.as_ptr(); @@ -856,9 +856,9 @@ impl BootServices { } LoadImageSource::FromDevicePath { device_path: file_path, - from_boot_manager, + boot_policy: new_boot_policy, } => { - boot_policy = u8::from(from_boot_manager); + boot_policy = new_boot_policy; device_path = file_path.as_ffi_ptr(); source_buffer = ptr::null(); source_size = 0; @@ -868,7 +868,7 @@ impl BootServices { let mut image_handle = ptr::null_mut(); unsafe { (self.0.load_image)( - boot_policy, + boot_policy.into(), parent_image_handle.as_ptr(), device_path.cast(), source_buffer, @@ -1403,9 +1403,10 @@ pub enum LoadImageSource<'a> { /// Load an image via the [`SimpleFileSystem`] protocol. If there is /// no instance of that protocol associated with the path then the - /// behavior depends on `from_boot_manager`. If `true`, attempt to - /// load via the `LoadFile` protocol. If `false`, attempt to load - /// via the `LoadFile2` protocol, then fall back to `LoadFile`. + /// behavior depends on [`BootPolicy`]. If [`BootPolicy::BootSelection`], + /// attempt to load via the `LoadFile` protocol. If + /// [`BootPolicy::ExactMatch`], attempt to load via the `LoadFile2` + /// protocol, then fall back to `LoadFile`. FromDevicePath { /// The full device path from which to load the image. /// @@ -1416,12 +1417,8 @@ pub enum LoadImageSource<'a> { /// and not just `\\EFI\\BOOT\\BOOTX64.EFI`. device_path: &'a DevicePath, - /// If there is no instance of [`SimpleFileSystem`] protocol associated - /// with the given device path, then this function will attempt to use - /// `LoadFileProtocol` (`from_boot_manager` is `true`) or - /// `LoadFile2Protocol`, and then `LoadFileProtocol` - /// (`from_boot_manager` is `false`). - from_boot_manager: bool, + /// The [`BootPolicy`] to use. + boot_policy: BootPolicy, }, } From 1d1138e63e1aa8f5af96ccf3cb78466577619306 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Tue, 13 Aug 2024 15:12:09 +0200 Subject: [PATCH 4/6] uefi: remove code duplication The existing code duplication is just temporary until the old API is deleted. Nevertheless, it is nicer to have this conversion close to the actual type. --- uefi/src/boot.rs | 25 +---------------- uefi/src/table/boot.rs | 63 +++++++++++++++++++++++++++--------------- 2 files changed, 41 insertions(+), 47 deletions(-) diff --git a/uefi/src/boot.rs b/uefi/src/boot.rs index bfa339ca6..d4b6d978a 100644 --- a/uefi/src/boot.rs +++ b/uefi/src/boot.rs @@ -23,7 +23,6 @@ use uefi_raw::table::boot::InterfaceType; #[cfg(doc)] use crate::proto::device_path::LoadedImageDevicePath; -use uefi::proto::BootPolicy; #[cfg(feature = "alloc")] use {alloc::vec::Vec, uefi::ResultExt}; @@ -995,29 +994,7 @@ pub fn load_image(parent_image_handle: Handle, source: LoadImageSource) -> Resul let bt = boot_services_raw_panicking(); let bt = unsafe { bt.as_ref() }; - let boot_policy; - let device_path; - let source_buffer; - let source_size; - match source { - LoadImageSource::FromBuffer { buffer, file_path } => { - // Boot policy is ignored when loading from source buffer. - boot_policy = BootPolicy::ExactMatch; - - device_path = file_path.map(|p| p.as_ffi_ptr()).unwrap_or(ptr::null()); - source_buffer = buffer.as_ptr(); - source_size = buffer.len(); - } - LoadImageSource::FromDevicePath { - device_path: file_path, - boot_policy: new_boot_policy, - } => { - boot_policy = new_boot_policy; - device_path = file_path.as_ffi_ptr(); - source_buffer = ptr::null(); - source_size = 0; - } - }; + let (boot_policy, device_path, source_buffer, source_size) = source.to_ffi_params(); let mut image_handle = ptr::null_mut(); unsafe { diff --git a/uefi/src/table/boot.rs b/uefi/src/table/boot.rs index c34d04303..b6d9f46e8 100644 --- a/uefi/src/table/boot.rs +++ b/uefi/src/table/boot.rs @@ -8,6 +8,7 @@ use super::Revision; use crate::data_types::PhysicalAddress; use crate::mem::memory_map::*; use crate::proto::device_path::DevicePath; +use crate::proto::device_path::FfiDevicePath; use crate::proto::loaded_image::LoadedImage; use crate::proto::media::fs::SimpleFileSystem; use crate::proto::{BootPolicy, Protocol, ProtocolPointer}; @@ -841,29 +842,7 @@ impl BootServices { parent_image_handle: Handle, source: LoadImageSource, ) -> uefi::Result { - let boot_policy; - let device_path; - let source_buffer; - let source_size; - match source { - LoadImageSource::FromBuffer { buffer, file_path } => { - // Boot policy is ignored when loading from source buffer. - boot_policy = BootPolicy::ExactMatch; - - device_path = file_path.map(|p| p.as_ffi_ptr()).unwrap_or(ptr::null()); - source_buffer = buffer.as_ptr(); - source_size = buffer.len(); - } - LoadImageSource::FromDevicePath { - device_path: file_path, - boot_policy: new_boot_policy, - } => { - boot_policy = new_boot_policy; - device_path = file_path.as_ffi_ptr(); - source_buffer = ptr::null(); - source_size = 0; - } - }; + let (boot_policy, device_path, source_buffer, source_size) = source.to_ffi_params(); let mut image_handle = ptr::null_mut(); unsafe { @@ -1422,6 +1401,44 @@ pub enum LoadImageSource<'a> { }, } +impl<'a> LoadImageSource<'a> { + /// Returns the raw FFI parameters for `load_image`. + #[must_use] + pub(crate) fn to_ffi_params( + &self, + ) -> ( + BootPolicy, + *const FfiDevicePath, + *const u8, /* buffer */ + usize, /* buffer length */ + ) { + let boot_policy; + let device_path; + let source_buffer; + let source_size; + match self { + LoadImageSource::FromBuffer { buffer, file_path } => { + // Boot policy is ignored when loading from source buffer. + boot_policy = BootPolicy::ExactMatch; + + device_path = file_path.map(|p| p.as_ffi_ptr()).unwrap_or(ptr::null()); + source_buffer = buffer.as_ptr(); + source_size = buffer.len(); + } + LoadImageSource::FromDevicePath { + device_path: d_path, + boot_policy: b_policy, + } => { + boot_policy = *b_policy; + device_path = d_path.as_ffi_ptr(); + source_buffer = ptr::null(); + source_size = 0; + } + }; + (boot_policy, device_path, source_buffer, source_size) + } +} + /// RAII guard for task priority level changes /// /// Will automatically restore the former task priority level when dropped. From 8f0a6b5ceec9f2d981b98b1a4eb4ebf7fbf88622 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Tue, 13 Aug 2024 15:14:02 +0200 Subject: [PATCH 5/6] uefi: fix imports --- uefi/src/boot.rs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/uefi/src/boot.rs b/uefi/src/boot.rs index d4b6d978a..5337e5ff8 100644 --- a/uefi/src/boot.rs +++ b/uefi/src/boot.rs @@ -2,36 +2,35 @@ //! //! These functions will panic if called after exiting boot services. +pub use crate::table::boot::{ + AllocateType, EventNotifyFn, LoadImageSource, OpenProtocolAttributes, OpenProtocolParams, + ProtocolSearchKey, SearchType, TimerTrigger, +}; +pub use uefi_raw::table::boot::{EventType, MemoryAttribute, MemoryDescriptor, MemoryType, Tpl}; + use crate::data_types::PhysicalAddress; use crate::mem::memory_map::{MemoryMapBackingMemory, MemoryMapKey, MemoryMapMeta, MemoryMapOwned}; use crate::polyfill::maybe_uninit_slice_assume_init_ref; use crate::proto::device_path::DevicePath; +#[cfg(doc)] +use crate::proto::device_path::LoadedImageDevicePath; use crate::proto::loaded_image::LoadedImage; use crate::proto::media::fs::SimpleFileSystem; use crate::proto::{Protocol, ProtocolPointer}; use crate::runtime::{self, ResetType}; use crate::table::Revision; use crate::util::opt_nonnull_to_ptr; +use crate::{table, Char16, Error, Event, Guid, Handle, Result, Status, StatusExt}; use core::ffi::c_void; use core::mem::MaybeUninit; use core::ops::{Deref, DerefMut}; use core::ptr::{self, NonNull}; use core::sync::atomic::{AtomicPtr, Ordering}; use core::{mem, slice}; -use uefi::{table, Char16, Error, Event, Guid, Handle, Result, Status, StatusExt}; use uefi_raw::table::boot::InterfaceType; - -#[cfg(doc)] -use crate::proto::device_path::LoadedImageDevicePath; #[cfg(feature = "alloc")] use {alloc::vec::Vec, uefi::ResultExt}; -pub use uefi::table::boot::{ - AllocateType, EventNotifyFn, LoadImageSource, OpenProtocolAttributes, OpenProtocolParams, - ProtocolSearchKey, SearchType, TimerTrigger, -}; -pub use uefi_raw::table::boot::{EventType, MemoryAttribute, MemoryDescriptor, MemoryType, Tpl}; - /// Global image handle. This is only set by [`set_image_handle`], and it is /// only read by [`image_handle`]. static IMAGE_HANDLE: AtomicPtr = AtomicPtr::new(ptr::null_mut()); From b3d031ac5d4edacb5b6ed9a4c6d0ae45fd2ee79b Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Tue, 13 Aug 2024 17:08:51 +0200 Subject: [PATCH 6/6] uefi: boot-policy: derive Default --- uefi/src/proto/boot_policy.rs | 3 ++- uefi/src/table/boot.rs | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/uefi/src/proto/boot_policy.rs b/uefi/src/proto/boot_policy.rs index f11d8db39..d46edf2ff 100644 --- a/uefi/src/proto/boot_policy.rs +++ b/uefi/src/proto/boot_policy.rs @@ -28,7 +28,7 @@ impl core::error::Error for BootPolicyError {} /// /// This type is not ABI compatible. On the ABI level, this is an UEFI /// boolean. -#[derive(Debug, Copy, Clone, PartialOrd, PartialEq, Eq, Ord)] +#[derive(Copy, Clone, Debug, Default, Eq, Ord, PartialEq, PartialOrd)] pub enum BootPolicy { /// Indicates that the request originates from the boot manager, and that /// the boot manager is attempting to load the provided `file_path` as a @@ -41,6 +41,7 @@ pub enum BootPolicy { /// The provided `file_path` must match an exact file to be loaded. /// /// This corresponds to the `FALSE` value in the UEFI spec. + #[default] ExactMatch, } diff --git a/uefi/src/table/boot.rs b/uefi/src/table/boot.rs index b6d9f46e8..eaca7f9fb 100644 --- a/uefi/src/table/boot.rs +++ b/uefi/src/table/boot.rs @@ -1419,7 +1419,7 @@ impl<'a> LoadImageSource<'a> { match self { LoadImageSource::FromBuffer { buffer, file_path } => { // Boot policy is ignored when loading from source buffer. - boot_policy = BootPolicy::ExactMatch; + boot_policy = BootPolicy::default(); device_path = file_path.map(|p| p.as_ffi_ptr()).unwrap_or(ptr::null()); source_buffer = buffer.as_ptr();