From 69c5174d6b15927d05cf4abb0175926736164aeb Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Sun, 9 Nov 2025 17:28:38 +0100 Subject: [PATCH 1/5] uefi: improve doc --- uefi/src/proto/device_path/build.rs | 3 ++- uefi/src/proto/device_path/mod.rs | 16 +++++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/uefi/src/proto/device_path/build.rs b/uefi/src/proto/device_path/build.rs index 66efe2c2f..3adcb7422 100644 --- a/uefi/src/proto/device_path/build.rs +++ b/uefi/src/proto/device_path/build.rs @@ -99,7 +99,7 @@ impl<'a> DevicePathBuilder<'a> { /// Add a node to the device path. /// /// An error will be returned if an [`END_ENTIRE`] node is passed to - /// this function, as that node will be added when `finalize` is + /// this function, as that node will be added when [`Self::finalize`] is /// called. /// /// [`END_ENTIRE`]: uefi::proto::device_path::DeviceSubType::END_ENTIRE @@ -150,6 +150,7 @@ impl<'a> DevicePathBuilder<'a> { } } +/// Reference to the backup storage for [`DevicePathBuilder`] #[derive(Debug)] enum BuilderStorage<'a> { Buf { diff --git a/uefi/src/proto/device_path/mod.rs b/uefi/src/proto/device_path/mod.rs index 254fd5d6a..b5b863f3c 100644 --- a/uefi/src/proto/device_path/mod.rs +++ b/uefi/src/proto/device_path/mod.rs @@ -75,6 +75,8 @@ //! other types in this module are DSTs, so pointers to the type are //! "fat" and not suitable for FFI. //! +//! * [`PoolDevicePath`] is an owned device path on the UEFI heap. +//! //! All of these types use a packed layout and may appear on any byte //! boundary. //! @@ -133,6 +135,10 @@ opaque_type! { } /// Device path allocated from UEFI pool memory. +/// +/// Please note that this differs from Box<[DevicePath]>. Although +/// both represent owned values, a Box<[DevicePath]> is on the Rust +/// heap which may or may not be backed by the UEFI heap. #[derive(Debug)] pub struct PoolDevicePath(pub(crate) PoolAllocation); @@ -392,11 +398,15 @@ impl DevicePathInstance { } /// Returns a boxed copy of that value. + /// + /// The semantics slightly differs from a [`PoolDevicePath`] but is + /// generally more idiomatic to use. #[cfg(feature = "alloc")] #[must_use] pub fn to_boxed(&self) -> Box { let data = self.data.to_owned(); let data = data.into_boxed_slice(); + // SAFETY: This is safe as a DevicePath has the same layout. unsafe { mem::transmute(data) } } } @@ -581,12 +591,16 @@ impl DevicePath { &self.data } - /// Returns a boxed copy of that value. + /// Returns a boxed copy of that value on the Rust heap. + /// + /// The semantics slightly differs from a [`PoolDevicePath`] but is + /// generally more idiomatic to use. #[cfg(feature = "alloc")] #[must_use] pub fn to_boxed(&self) -> Box { let data = self.data.to_owned(); let data = data.into_boxed_slice(); + // SAFETY: This is safe as a DevicePath has the same layout. unsafe { mem::transmute(data) } } From 859cfc35d842b645c31c00888b3757ca440ade6c Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Sun, 9 Nov 2025 17:29:23 +0100 Subject: [PATCH 2/5] uefi: tests: improve readability --- uefi/src/proto/device_path/build.rs | 57 +++++++++++++++-------------- 1 file changed, 30 insertions(+), 27 deletions(-) diff --git a/uefi/src/proto/device_path/build.rs b/uefi/src/proto/device_path/build.rs index 3adcb7422..e357b9648 100644 --- a/uefi/src/proto/device_path/build.rs +++ b/uefi/src/proto/device_path/build.rs @@ -247,11 +247,6 @@ mod tests { use crate::proto::device_path::messaging::{ Ipv4AddressOrigin, IscsiLoginOptions, IscsiProtocol, RestServiceAccessMode, RestServiceType, }; - use core::slice; - - const fn path_to_bytes(path: &DevicePath) -> &[u8] { - unsafe { slice::from_raw_parts(path.as_ffi_ptr().cast::(), size_of_val(path)) } - } /// Test building an ACPI ADR node. #[test] @@ -268,10 +263,8 @@ mod tests { let node: &crate::proto::device_path::acpi::Adr = path.node_iter().next().unwrap().try_into().unwrap(); assert_eq!(node.adr().iter().collect::>(), [1, 2]); - - let bytes = path_to_bytes(path); #[rustfmt::skip] - assert_eq!(bytes, [ + assert_eq!(path.as_bytes(), [ // ACPI ADR node 0x02, 0x03, 0x0c, 0x00, // Values @@ -309,9 +302,8 @@ mod tests { assert_eq!(node.uid_str(), b"bc\0"); assert_eq!(node.cid_str(), b"def\0"); - let bytes = path_to_bytes(path); #[rustfmt::skip] - assert_eq!(bytes, [ + assert_eq!(path.as_bytes(), [ // ACPI Expanded node 0x02, 0x02, 0x19, 0x00, // HID @@ -366,9 +358,8 @@ mod tests { assert_eq!(node.vendor_guid_and_data().unwrap().0, vendor_guid); assert_eq!(node.vendor_guid_and_data().unwrap().1, &[1, 2, 3, 4, 5]); - let bytes = path_to_bytes(path); #[rustfmt::skip] - assert_eq!(bytes, [ + assert_eq!(path.as_bytes(), [ // Messaging REST Service node. 0x03, 0x21, 0x06, 0x00, // Type and access mode @@ -429,27 +420,37 @@ mod tests { /// from the UEFI Specification. #[test] fn test_fibre_channel_ex_device_path_example() -> Result<(), BuildError> { - // Arbitrarily choose this test to use a statically-sized - // buffer, just to make sure that code path is tested. - let mut buf = [MaybeUninit::uninit(); 256]; - let path = DevicePathBuilder::with_buf(&mut buf) - .push(&acpi::Acpi { + let nodes: &[&dyn BuildNode] = &[ + &acpi::Acpi { hid: 0x41d0_0a03, uid: 0x0000_0000, - })? - .push(&hardware::Pci { + }, + &hardware::Pci { function: 0x00, device: 0x1f, - })? - .push(&messaging::FibreChannelEx { + }, + &messaging::FibreChannelEx { world_wide_name: [0, 1, 2, 3, 4, 5, 6, 7], logical_unit_number: [0, 1, 2, 3, 4, 5, 6, 7], - })? + }, + ]; + + // Arbitrarily choose this test to use a statically-sized + // buffer, just to make sure that code path is tested. + let mut buf = [MaybeUninit::uninit(); 256]; + let path1 = DevicePathBuilder::with_buf(&mut buf) + .push(nodes[0])? + .push(nodes[1])? + .push(nodes[2])? + .finalize()?; + let path2 = OwnedDevicePathBuilder::new() + .push(nodes[0])? + .push(nodes[1])? + .push(nodes[2])? .finalize()?; - let bytes = path_to_bytes(path); #[rustfmt::skip] - assert_eq!(bytes, [ + const EXPECTED: [u8; 46] = [ // ACPI node 0x02, 0x01, 0x0c, 0x00, // HID @@ -478,7 +479,10 @@ mod tests { // End-entire node 0x7f, 0xff, 0x04, 0x00, - ]); + ]; + + assert_eq!(path1.as_bytes(), EXPECTED); + assert_eq!(path2.as_bytes(), EXPECTED); Ok(()) } @@ -533,9 +537,8 @@ mod tests { })? .finalize()?; - let bytes = path_to_bytes(path); #[rustfmt::skip] - assert_eq!(bytes, [ + assert_eq!(path.as_bytes(), [ // ACPI node 0x02, 0x01, 0x0c, 0x00, // HID From d6b58116107c6b53d71e4c30a8f9e9c013a81f61 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Sun, 9 Nov 2025 17:29:43 +0100 Subject: [PATCH 3/5] new builder lol --- uefi-test-runner/src/proto/shell.rs | 5 +- uefi/src/proto/device_path/build.rs | 80 +++++++++++++++++++++++++---- 2 files changed, 73 insertions(+), 12 deletions(-) diff --git a/uefi-test-runner/src/proto/shell.rs b/uefi-test-runner/src/proto/shell.rs index f4aa88c50..d474655f3 100644 --- a/uefi-test-runner/src/proto/shell.rs +++ b/uefi-test-runner/src/proto/shell.rs @@ -1,8 +1,11 @@ // SPDX-License-Identifier: MIT OR Apache-2.0 -use uefi::boot::ScopedProtocol; +use uefi::boot::{OpenProtocolAttributes, OpenProtocolParams, ScopedProtocol}; +use uefi::proto::device_path::DevicePath; +use uefi::proto::device_path::text::{AllowShortcuts, DisplayOnly}; use uefi::proto::shell::Shell; use uefi::{Error, Status, boot, cstr16}; +use uefi_raw::protocol::shell::ShellProtocol; /// Test `current_dir()` and `set_current_dir()` pub fn test_current_dir(shell: &ScopedProtocol) { diff --git a/uefi/src/proto/device_path/build.rs b/uefi/src/proto/device_path/build.rs index e357b9648..0383d7d0d 100644 --- a/uefi/src/proto/device_path/build.rs +++ b/uefi/src/proto/device_path/build.rs @@ -2,8 +2,11 @@ //! Utilities for creating new [`DevicePaths`]. //! -//! This module contains [`DevicePathBuilder`], as well as submodules -//! containing types for building each type of device path node. +//! This module provides builders to construct device paths, as well as +//! submodules containing types for building each type of device path node. +//! +//! - [`DevicePathBuilder`] to construct device paths into pre-allocated buffers +//! - [`OwnedDevicePathBuilder`] for construction on the Rust heap //! //! [`DevicePaths`]: DevicePath @@ -15,7 +18,10 @@ use core::fmt::{self, Display, Formatter}; use core::mem::MaybeUninit; #[cfg(feature = "alloc")] -use alloc::vec::Vec; +use { + alloc::{boxed::Box, vec::Vec}, + core::mem, +}; /// A builder for [`DevicePaths`]. /// @@ -74,14 +80,14 @@ use alloc::vec::Vec; /// ``` #[derive(Debug)] pub struct DevicePathBuilder<'a> { - storage: BuilderStorage<'a>, + storage: BuilderStorageRef<'a>, } impl<'a> DevicePathBuilder<'a> { /// Create a builder backed by a statically-sized buffer. pub const fn with_buf(buf: &'a mut [MaybeUninit]) -> Self { Self { - storage: BuilderStorage::Buf { buf, offset: 0 }, + storage: BuilderStorageRef::Buf { buf, offset: 0 }, } } @@ -92,7 +98,7 @@ impl<'a> DevicePathBuilder<'a> { pub fn with_vec(v: &'a mut Vec) -> Self { v.clear(); Self { - storage: BuilderStorage::Vec(v), + storage: BuilderStorageRef::Vec(v), } } @@ -107,7 +113,7 @@ impl<'a> DevicePathBuilder<'a> { let node_size = usize::from(node.size_in_bytes()?); match &mut self.storage { - BuilderStorage::Buf { buf, offset } => { + BuilderStorageRef::Buf { buf, offset } => { node.write_data( buf.get_mut(*offset..*offset + node_size) .ok_or(BuildError::BufferTooSmall)?, @@ -115,7 +121,7 @@ impl<'a> DevicePathBuilder<'a> { *offset += node_size; } #[cfg(feature = "alloc")] - BuilderStorage::Vec(vec) => { + BuilderStorageRef::Vec(vec) => { let old_size = vec.len(); vec.reserve(node_size); let buf = &mut vec.spare_capacity_mut()[..node_size]; @@ -138,11 +144,11 @@ impl<'a> DevicePathBuilder<'a> { let this = self.push(&end::Entire)?; let data: &[u8] = match &this.storage { - BuilderStorage::Buf { buf, offset } => unsafe { + BuilderStorageRef::Buf { buf, offset } => unsafe { maybe_uninit_slice_assume_init_ref(&buf[..*offset]) }, #[cfg(feature = "alloc")] - BuilderStorage::Vec(vec) => vec, + BuilderStorageRef::Vec(vec) => vec, }; let ptr: *const () = data.as_ptr().cast(); @@ -152,7 +158,7 @@ impl<'a> DevicePathBuilder<'a> { /// Reference to the backup storage for [`DevicePathBuilder`] #[derive(Debug)] -enum BuilderStorage<'a> { +enum BuilderStorageRef<'a> { Buf { buf: &'a mut [MaybeUninit], offset: usize, @@ -194,6 +200,58 @@ impl Display for BuildError { impl core::error::Error for BuildError {} +/// Variant of [`DevicePathBuilder`] to construct a boxed [`DevicePath`]. +/// +/// Using this builder is equivalent to calling [`DevicePath::to_boxed`] on a +/// device path constructed by the normal builder. +#[derive(Debug)] +#[cfg(feature = "alloc")] +pub struct OwnedDevicePathBuilder { + vec: Vec, +} + +#[cfg(feature = "alloc")] +impl OwnedDevicePathBuilder { + /// Creates a new builder. + pub fn new() -> Self { + let vec = Vec::new(); + Self { vec } + } + + /// Add a node to the device path. + /// + /// An error will be returned if an [`END_ENTIRE`] node is passed to + /// this function, as that node will be added when [`Self::finalize`] is + /// called. + /// + /// [`END_ENTIRE`]: uefi::proto::device_path::DeviceSubType::END_ENTIRE + pub fn push(mut self, node: &dyn BuildNode) -> Result { + let node_size = usize::from(node.size_in_bytes()?); + + let old_size = self.vec.len(); + self.vec.reserve(node_size); + let buf = &mut self.vec.spare_capacity_mut()[..node_size]; + node.write_data(buf); + unsafe { + self.vec.set_len(old_size + node_size); + } + Ok(self) + } + + /// Add an [`END_ENTIRE`] node and return the resulting [`DevicePath`]. + /// + /// This method consumes the builder. + /// + /// [`END_ENTIRE`]: uefi::proto::device_path::DeviceSubType::END_ENTIRE + pub fn finalize(self) -> Result, BuildError> { + let this = self.push(&end::Entire)?; + let boxed = this.vec.into_boxed_slice(); + // SAFETY: This is safe as a DevicePath has the same layout. + let dvp = unsafe { mem::transmute(boxed) }; + Ok(dvp) + } +} + /// Trait for types that can be used to build a node via /// [`DevicePathBuilder::push`]. /// From 4bba239b34ba2398424dbd2afdee8a83711a0c12 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Sun, 9 Nov 2025 17:29:51 +0100 Subject: [PATCH 4/5] Revert "new builder lol" This reverts commit d6b58116107c6b53d71e4c30a8f9e9c013a81f61. --- uefi-test-runner/src/proto/shell.rs | 5 +- uefi/src/proto/device_path/build.rs | 80 ++++------------------------- 2 files changed, 12 insertions(+), 73 deletions(-) diff --git a/uefi-test-runner/src/proto/shell.rs b/uefi-test-runner/src/proto/shell.rs index d474655f3..f4aa88c50 100644 --- a/uefi-test-runner/src/proto/shell.rs +++ b/uefi-test-runner/src/proto/shell.rs @@ -1,11 +1,8 @@ // SPDX-License-Identifier: MIT OR Apache-2.0 -use uefi::boot::{OpenProtocolAttributes, OpenProtocolParams, ScopedProtocol}; -use uefi::proto::device_path::DevicePath; -use uefi::proto::device_path::text::{AllowShortcuts, DisplayOnly}; +use uefi::boot::ScopedProtocol; use uefi::proto::shell::Shell; use uefi::{Error, Status, boot, cstr16}; -use uefi_raw::protocol::shell::ShellProtocol; /// Test `current_dir()` and `set_current_dir()` pub fn test_current_dir(shell: &ScopedProtocol) { diff --git a/uefi/src/proto/device_path/build.rs b/uefi/src/proto/device_path/build.rs index 0383d7d0d..e357b9648 100644 --- a/uefi/src/proto/device_path/build.rs +++ b/uefi/src/proto/device_path/build.rs @@ -2,11 +2,8 @@ //! Utilities for creating new [`DevicePaths`]. //! -//! This module provides builders to construct device paths, as well as -//! submodules containing types for building each type of device path node. -//! -//! - [`DevicePathBuilder`] to construct device paths into pre-allocated buffers -//! - [`OwnedDevicePathBuilder`] for construction on the Rust heap +//! This module contains [`DevicePathBuilder`], as well as submodules +//! containing types for building each type of device path node. //! //! [`DevicePaths`]: DevicePath @@ -18,10 +15,7 @@ use core::fmt::{self, Display, Formatter}; use core::mem::MaybeUninit; #[cfg(feature = "alloc")] -use { - alloc::{boxed::Box, vec::Vec}, - core::mem, -}; +use alloc::vec::Vec; /// A builder for [`DevicePaths`]. /// @@ -80,14 +74,14 @@ use { /// ``` #[derive(Debug)] pub struct DevicePathBuilder<'a> { - storage: BuilderStorageRef<'a>, + storage: BuilderStorage<'a>, } impl<'a> DevicePathBuilder<'a> { /// Create a builder backed by a statically-sized buffer. pub const fn with_buf(buf: &'a mut [MaybeUninit]) -> Self { Self { - storage: BuilderStorageRef::Buf { buf, offset: 0 }, + storage: BuilderStorage::Buf { buf, offset: 0 }, } } @@ -98,7 +92,7 @@ impl<'a> DevicePathBuilder<'a> { pub fn with_vec(v: &'a mut Vec) -> Self { v.clear(); Self { - storage: BuilderStorageRef::Vec(v), + storage: BuilderStorage::Vec(v), } } @@ -113,7 +107,7 @@ impl<'a> DevicePathBuilder<'a> { let node_size = usize::from(node.size_in_bytes()?); match &mut self.storage { - BuilderStorageRef::Buf { buf, offset } => { + BuilderStorage::Buf { buf, offset } => { node.write_data( buf.get_mut(*offset..*offset + node_size) .ok_or(BuildError::BufferTooSmall)?, @@ -121,7 +115,7 @@ impl<'a> DevicePathBuilder<'a> { *offset += node_size; } #[cfg(feature = "alloc")] - BuilderStorageRef::Vec(vec) => { + BuilderStorage::Vec(vec) => { let old_size = vec.len(); vec.reserve(node_size); let buf = &mut vec.spare_capacity_mut()[..node_size]; @@ -144,11 +138,11 @@ impl<'a> DevicePathBuilder<'a> { let this = self.push(&end::Entire)?; let data: &[u8] = match &this.storage { - BuilderStorageRef::Buf { buf, offset } => unsafe { + BuilderStorage::Buf { buf, offset } => unsafe { maybe_uninit_slice_assume_init_ref(&buf[..*offset]) }, #[cfg(feature = "alloc")] - BuilderStorageRef::Vec(vec) => vec, + BuilderStorage::Vec(vec) => vec, }; let ptr: *const () = data.as_ptr().cast(); @@ -158,7 +152,7 @@ impl<'a> DevicePathBuilder<'a> { /// Reference to the backup storage for [`DevicePathBuilder`] #[derive(Debug)] -enum BuilderStorageRef<'a> { +enum BuilderStorage<'a> { Buf { buf: &'a mut [MaybeUninit], offset: usize, @@ -200,58 +194,6 @@ impl Display for BuildError { impl core::error::Error for BuildError {} -/// Variant of [`DevicePathBuilder`] to construct a boxed [`DevicePath`]. -/// -/// Using this builder is equivalent to calling [`DevicePath::to_boxed`] on a -/// device path constructed by the normal builder. -#[derive(Debug)] -#[cfg(feature = "alloc")] -pub struct OwnedDevicePathBuilder { - vec: Vec, -} - -#[cfg(feature = "alloc")] -impl OwnedDevicePathBuilder { - /// Creates a new builder. - pub fn new() -> Self { - let vec = Vec::new(); - Self { vec } - } - - /// Add a node to the device path. - /// - /// An error will be returned if an [`END_ENTIRE`] node is passed to - /// this function, as that node will be added when [`Self::finalize`] is - /// called. - /// - /// [`END_ENTIRE`]: uefi::proto::device_path::DeviceSubType::END_ENTIRE - pub fn push(mut self, node: &dyn BuildNode) -> Result { - let node_size = usize::from(node.size_in_bytes()?); - - let old_size = self.vec.len(); - self.vec.reserve(node_size); - let buf = &mut self.vec.spare_capacity_mut()[..node_size]; - node.write_data(buf); - unsafe { - self.vec.set_len(old_size + node_size); - } - Ok(self) - } - - /// Add an [`END_ENTIRE`] node and return the resulting [`DevicePath`]. - /// - /// This method consumes the builder. - /// - /// [`END_ENTIRE`]: uefi::proto::device_path::DeviceSubType::END_ENTIRE - pub fn finalize(self) -> Result, BuildError> { - let this = self.push(&end::Entire)?; - let boxed = this.vec.into_boxed_slice(); - // SAFETY: This is safe as a DevicePath has the same layout. - let dvp = unsafe { mem::transmute(boxed) }; - Ok(dvp) - } -} - /// Trait for types that can be used to build a node via /// [`DevicePathBuilder::push`]. /// From 9d803eae03dc11fe1d0464ccdee736a753eb4335 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Sun, 9 Nov 2025 18:36:52 +0100 Subject: [PATCH 5/5] xxx The idea is to enable a more convenient builder experience: - in buffer - on uefi heap - in rust heap --- uefi-test-runner/src/bin/shell_launcher.rs | 2 +- uefi-test-runner/src/main.rs | 2 +- uefi-test-runner/src/proto/device_path.rs | 2 +- uefi-test-runner/src/proto/load.rs | 2 +- uefi-test-runner/src/proto/mod.rs | 36 -------- uefi/src/boot.rs | 1 + uefi/src/mem/mod.rs | 4 + uefi/src/proto/device_path/build.rs | 97 ++++++++++++++++------ 8 files changed, 79 insertions(+), 67 deletions(-) diff --git a/uefi-test-runner/src/bin/shell_launcher.rs b/uefi-test-runner/src/bin/shell_launcher.rs index f48e28c2d..1abe0ce4f 100644 --- a/uefi-test-runner/src/bin/shell_launcher.rs +++ b/uefi-test-runner/src/bin/shell_launcher.rs @@ -29,7 +29,7 @@ fn get_shell_app_device_path(storage: &mut Vec) -> &DevicePath { boot::open_protocol_exclusive::(boot::image_handle()) .expect("failed to open LoadedImageDevicePath protocol"); - let mut builder = DevicePathBuilder::with_vec(storage); + let mut builder = DevicePathBuilder::with_rust_heap(storage); for node in loaded_image_device_path.node_iter() { if node.full_type() == (DeviceType::MEDIA, DeviceSubType::MEDIA_FILE_PATH) { break; diff --git a/uefi-test-runner/src/main.rs b/uefi-test-runner/src/main.rs index 2d05191f6..7d1b78e86 100644 --- a/uefi-test-runner/src/main.rs +++ b/uefi-test-runner/src/main.rs @@ -141,7 +141,7 @@ fn reconnect_serial_to_console(serial_handle: Handle) { } else { Vendor::VT_UTF8 }; - let terminal_device_path = DevicePathBuilder::with_vec(&mut storage) + let terminal_device_path = DevicePathBuilder::with_rust_heap(&mut storage) .push(&build::messaging::Vendor { vendor_guid: terminal_guid, vendor_defined_data: &[], diff --git a/uefi-test-runner/src/proto/device_path.rs b/uefi-test-runner/src/proto/device_path.rs index 0d36e46f9..c6f1fa15a 100644 --- a/uefi-test-runner/src/proto/device_path.rs +++ b/uefi-test-runner/src/proto/device_path.rs @@ -48,7 +48,7 @@ pub fn test() { fn create_test_device_path() -> Box { let mut v = Vec::new(); - DevicePathBuilder::with_vec(&mut v) + DevicePathBuilder::with_rust_heap(&mut v) // Add an ATAPI node because edk2 displays it differently depending on // the value of `DisplayOnly`. .push(&build::messaging::Atapi { diff --git a/uefi-test-runner/src/proto/load.rs b/uefi-test-runner/src/proto/load.rs index 5fdda7c9b..af87f67e9 100644 --- a/uefi-test-runner/src/proto/load.rs +++ b/uefi-test-runner/src/proto/load.rs @@ -98,7 +98,7 @@ pub fn test() { } let mut dvp_vec = Vec::new(); - let dummy_dvp = DevicePathBuilder::with_vec(&mut dvp_vec); + let dummy_dvp = DevicePathBuilder::with_rust_heap(&mut dvp_vec); let dummy_dvp = dummy_dvp.finalize().unwrap(); let mut load_file_protocol = boot::open_protocol_exclusive::(image).unwrap(); diff --git a/uefi-test-runner/src/proto/mod.rs b/uefi-test-runner/src/proto/mod.rs index 5d66587c4..e2c21433f 100644 --- a/uefi-test-runner/src/proto/mod.rs +++ b/uefi-test-runner/src/proto/mod.rs @@ -7,43 +7,7 @@ use uefi::{Identify, proto}; pub fn test() { info!("Testing various protocols"); - console::test(); - - find_protocol(); - test_protocols_per_handle(); - test_test_protocol(); - - debug::test(); - device_path::test(); - driver::test(); - load::test(); - loaded_image::test(); - media::test(); - network::test(); - pci::test(); - pi::test(); - rng::test(); - shell_params::test(); - string::test(); - usb::test(); - misc::test(); - - // disable the ATA test on aarch64 for now. The aarch64 UEFI Firmware does not yet seem - // to support SATA controllers (and providing an AtaPassThru protocol instance for them). - #[cfg(any(target_arch = "x86", target_arch = "x86_64"))] - ata::test(); - scsi::test(); - nvme::test(); - - #[cfg(any( - target_arch = "x86", - target_arch = "x86_64", - target_arch = "arm", - target_arch = "aarch64" - ))] - shim::test(); shell::test(); - tcg::test(); } fn find_protocol() { diff --git a/uefi/src/boot.rs b/uefi/src/boot.rs index c6449a42f..bbe788dc0 100644 --- a/uefi/src/boot.rs +++ b/uefi/src/boot.rs @@ -230,6 +230,7 @@ pub unsafe fn free_pages(ptr: NonNull, count: usize) -> Result { /// * [`Status::OUT_OF_RESOURCES`]: allocation failed. /// * [`Status::INVALID_PARAMETER`]: `mem_ty` is [`MemoryType::PERSISTENT_MEMORY`], /// [`MemoryType::UNACCEPTED`], or in the range [MemoryType::MAX]..=0x6fff_ffff. +// TODO should we return PoolAllocation here? pub fn allocate_pool(memory_type: MemoryType, size: usize) -> Result> { let bt = boot_services_raw_panicking(); let bt = unsafe { bt.as_ref() }; diff --git a/uefi/src/mem/mod.rs b/uefi/src/mem/mod.rs index 122a2eb7f..7794466e5 100644 --- a/uefi/src/mem/mod.rs +++ b/uefi/src/mem/mod.rs @@ -25,6 +25,10 @@ pub use aligned_buffer::{AlignedBuffer, AlignmentError}; pub(crate) struct PoolAllocation(NonNull); impl PoolAllocation { + /// This creates a new [`PoolAllocation`] from a `ptr` that represents + /// the allocation. + /// + /// This function doesn't allocate. pub(crate) const fn new(ptr: NonNull) -> Self { Self(ptr) } diff --git a/uefi/src/proto/device_path/build.rs b/uefi/src/proto/device_path/build.rs index e357b9648..89f34ceb4 100644 --- a/uefi/src/proto/device_path/build.rs +++ b/uefi/src/proto/device_path/build.rs @@ -6,8 +6,9 @@ //! containing types for building each type of device path node. //! //! [`DevicePaths`]: DevicePath - pub use crate::proto::device_path::device_path_gen::build::*; +use alloc::boxed::Box; +use alloc::vec; use crate::polyfill::{maybe_uninit_slice_as_mut_ptr, maybe_uninit_slice_assume_init_ref}; use crate::proto::device_path::{DevicePath, DevicePathNode}; @@ -16,11 +17,20 @@ use core::mem::MaybeUninit; #[cfg(feature = "alloc")] use alloc::vec::Vec; +use core::mem; +use uefi::boot; +use uefi::mem::PoolAllocation; +use uefi::proto::device_path::PoolDevicePath; +use uefi_raw::table::boot::MemoryType; /// A builder for [`DevicePaths`]. /// -/// The builder can be constructed with either a fixed-length buffer or -/// (if the `alloc` feature is enabled) a `Vec`. +/// The builder has multiple constructors affecting the ownership of the +/// resulting device path: +/// +/// - [`DevicePathBuilder::with_buf`] +/// - [`DevicePathBuilder::with_uefi_heap`] +/// - [`DevicePathBuilder::with_rust_heap`] /// /// Nodes are added via the [`push`] method. To construct a node, use one /// of the structs in these submodules: @@ -78,21 +88,27 @@ pub struct DevicePathBuilder<'a> { } impl<'a> DevicePathBuilder<'a> { - /// Create a builder backed by a statically-sized buffer. + /// Create a builder that will return [`BuiltDevicePath::Buf`]. pub const fn with_buf(buf: &'a mut [MaybeUninit]) -> Self { Self { storage: BuilderStorage::Buf { buf, offset: 0 }, } } - /// Create a builder backed by a `Vec`. - /// - /// The `Vec` is cleared before use. + /// Create a builder that will return [`BuiltDevicePath::UefiHeap`]. + pub fn with_uefi_heap() -> Self { + // TODO propagate result? + let ptr = { boot::allocate_pool(MemoryType::LOADER_DATA, 1).unwrap() }; + Self { + storage: BuilderStorage::UefiHeap(PoolAllocation::new(ptr)), + } + } + + /// Create a builder that will return [`BuiltDevicePath::Boxed`]. #[cfg(feature = "alloc")] - pub fn with_vec(v: &'a mut Vec) -> Self { - v.clear(); + pub const fn with_rust_heap() -> Self { Self { - storage: BuilderStorage::Vec(v), + storage: BuilderStorage::Boxed(vec::Vec::new()), } } @@ -114,8 +130,11 @@ impl<'a> DevicePathBuilder<'a> { ); *offset += node_size; } + BuiltDevicePath::UefiHeap(_pool) => { + todo!() + } #[cfg(feature = "alloc")] - BuilderStorage::Vec(vec) => { + BuilderStorage::Boxed(vec) => { let old_size = vec.len(); vec.reserve(node_size); let buf = &mut vec.spare_capacity_mut()[..node_size]; @@ -129,24 +148,33 @@ impl<'a> DevicePathBuilder<'a> { Ok(self) } - /// Add an [`END_ENTIRE`] node and return the resulting [`DevicePath`]. + // todo extend method (multiple push) + + /// Add an [`END_ENTIRE`] node and return [`BuiltDevicePath`], depending + /// on the chosen builder strategy. /// /// This method consumes the builder. /// /// [`END_ENTIRE`]: uefi::proto::device_path::DeviceSubType::END_ENTIRE - pub fn finalize(self) -> Result<&'a DevicePath, BuildError> { + pub fn finalize(self) -> Result, BuildError> { let this = self.push(&end::Entire)?; - let data: &[u8] = match &this.storage { + match this.storage { BuilderStorage::Buf { buf, offset } => unsafe { - maybe_uninit_slice_assume_init_ref(&buf[..*offset]) + let data = maybe_uninit_slice_assume_init_ref(&buf[..offset]); + let ptr: *const () = data.as_ptr().cast(); + let path = &*ptr_meta::from_raw_parts(ptr, data.len()); + Ok(BuiltDevicePath::Buf(path)) }, + BuilderStorage::UefiHeap(pool) => Ok(BuiltDevicePath::UefiHeap(PoolDevicePath(pool))), #[cfg(feature = "alloc")] - BuilderStorage::Vec(vec) => vec, - }; - - let ptr: *const () = data.as_ptr().cast(); - Ok(unsafe { &*ptr_meta::from_raw_parts(ptr, data.len()) }) + BuilderStorage::Boxed(vec) => { + let data = vec.into_boxed_slice(); + // SAFETY: Same layout, trivially safe. + let device_path = unsafe { mem::transmute(data) }; + Ok(BuiltDevicePath::Boxed(device_path)) + } + } } } @@ -158,8 +186,23 @@ enum BuilderStorage<'a> { offset: usize, }, + UefiHeap(PoolAllocation), + + #[cfg(feature = "alloc")] + Boxed(Vec), +} + +/// The device path that is build by [`DevicePathBuilder`]. +/// +/// This directly depends on [`BuilderStorage`]. +pub enum BuiltDevicePath<'a> { + /// Returns a reference to the provided buffer ([`BuilderStorage::Boxed`]). + Buf(&'a DevicePath), + /// Owned value on the UEFI heap. + UefiHeap(PoolDevicePath), #[cfg(feature = "alloc")] - Vec(&'a mut Vec), + /// Boxed value on the Rust heap a reference to the provided buffer ([`BuilderStorage::Boxed`]). + Boxed(Box), } /// Error type used by [`DevicePathBuilder`]. @@ -254,7 +297,7 @@ mod tests { assert!(acpi::AdrSlice::new(&[]).is_none()); let mut v = Vec::new(); - let path = DevicePathBuilder::with_vec(&mut v) + let path = DevicePathBuilder::with_rust_heap(&mut v) .push(&acpi::Adr { adr: acpi::AdrSlice::new(&[1, 2]).unwrap(), })? @@ -282,7 +325,7 @@ mod tests { #[test] fn test_acpi_expanded() -> Result<(), BuildError> { let mut v = Vec::new(); - let path = DevicePathBuilder::with_vec(&mut v) + let path = DevicePathBuilder::with_rust_heap(&mut v) .push(&acpi::Expanded { hid: 1, uid: 2, @@ -334,7 +377,7 @@ mod tests { fn test_messaging_rest_service() -> Result<(), BuildError> { let mut v = Vec::new(); let vendor_guid = guid!("a1005a90-6591-4596-9bab-1c4249a6d4ff"); - let path = DevicePathBuilder::with_vec(&mut v) + let path = DevicePathBuilder::with_rust_heap(&mut v) .push(&messaging::RestService { service_type: RestServiceType::REDFISH, access_mode: RestServiceAccessMode::IN_BAND, @@ -390,7 +433,7 @@ mod tests { fn test_build_with_packed_node() -> Result<(), BuildError> { // Build a path with both a statically-sized and DST nodes. let mut v = Vec::new(); - let path1 = DevicePathBuilder::with_vec(&mut v) + let path1 = DevicePathBuilder::with_rust_heap(&mut v) .push(&acpi::Acpi { hid: 0x41d0_0a03, uid: 0x0000_0000, @@ -404,7 +447,7 @@ mod tests { // Create a second path by copying in the packed nodes from the // first path. let mut v = Vec::new(); - let mut builder = DevicePathBuilder::with_vec(&mut v); + let mut builder = DevicePathBuilder::with_rust_heap(&mut v); for node in path1.node_iter() { builder = builder.push(&node)?; } @@ -492,7 +535,7 @@ mod tests { #[test] fn test_ipv4_configuration_example() -> Result<(), BuildError> { let mut v = Vec::new(); - let path = DevicePathBuilder::with_vec(&mut v) + let path = DevicePathBuilder::with_rust_heap(&mut v) .push(&acpi::Acpi { hid: 0x41d0_0a03, uid: 0x0000_0000,