From 9d803eae03dc11fe1d0464ccdee736a753eb4335 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Sun, 9 Nov 2025 18:36:52 +0100 Subject: [PATCH] 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 | 99 ++++++++++++++++------ 8 files changed, 80 insertions(+), 68 deletions(-) diff --git a/uefi-test-runner/src/bin/shell_launcher.rs b/uefi-test-runner/src/bin/shell_launcher.rs index f48e28c2..1abe0ce4 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 2d05191f..7d1b78e8 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 0d36e46f..c6f1fa15 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 5fdda7c9..af87f67e 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 5d66587c..e2c21433 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 c6449a42..bbe788dc 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 122a2eb7..7794466e 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 e357b964..89f34ceb 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. - #[cfg(feature = "alloc")] - pub fn with_vec(v: &'a mut Vec) -> Self { - v.clear(); + /// 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::Vec(v), + storage: BuilderStorage::UefiHeap(PoolAllocation::new(ptr)), + } + } + + /// Create a builder that will return [`BuiltDevicePath::Boxed`]. + #[cfg(feature = "alloc")] + pub const fn with_rust_heap() -> Self { + Self { + 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")] - Vec(&'a mut Vec), + 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")] + /// 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,