diff --git a/CHANGELOG.md b/CHANGELOG.md index 5234d13a5..2b3d7608b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,14 +14,25 @@ Now you can compare everything that is `AsRef` (such as `String` and `str` from the standard library) to uefi strings. Please head to the documentation of `EqStrUntilNul` to find out limitations and further information. +- Added `BootServices::image_handle` to get the handle of the executing + image. The image is set automatically by the `#[entry]` macro; if a + program does not use that macro then it should call + `BootServices::set_image_handle`. +- Added `BootServices::open_protocol_exclusive`. This provides a safe + and convenient subset of `open_protocol` that can be used whenever a + resource doesn't need to be shared. In same cases sharing is useful + (e.g. you might want to draw to the screen using the graphics + protocol, but still allow stdout output to go to the screen as + well), and in those cases `open_protocol` can still be used. ### Changed - Marked `BootServices::handle_protocol` as `unsafe`. (This method is - also deprecated -- use `open_protocol` instead.) + also deprecated -- use `open_protocol_exclusive` or `open_protocol` instead.) - Deprecated `BootServices::locate_protocol` and marked it `unsafe`. Use `BootServices::get_handle_for_protocol` and - `BootServices::open_protocol` instead. + `BootServices::open_protocol_exclusive` (or + `BootServices::open_protocol`) instead. - renamed feature `ignore-logger-errors` to `panic-on-logger-errors` so that it is additive. It is now a default feature. diff --git a/src/table/boot.rs b/src/table/boot.rs index cd7e8ae25..6a81bee82 100644 --- a/src/table/boot.rs +++ b/src/table/boot.rs @@ -18,6 +18,20 @@ use core::ops::{Deref, DerefMut}; use core::ptr::NonNull; use core::{ptr, slice}; +// TODO: this similar to `SyncUnsafeCell`. Once that is stabilized we +// can use it instead. +struct GlobalImageHandle { + handle: UnsafeCell>, +} + +// Safety: reads and writes are managed via `set_image_handle` and +// `BootServices::image_handle`. +unsafe impl Sync for GlobalImageHandle {} + +static IMAGE_HANDLE: GlobalImageHandle = GlobalImageHandle { + handle: UnsafeCell::new(None), +}; + /// Contains pointers to all of the boot services. /// /// # Accessing `BootServices` @@ -29,13 +43,14 @@ use core::{ptr, slice}; /// # Accessing protocols /// /// Protocols can be opened using several methods of `BootServices`. Most -/// commonly, [`open_protocol`] should be used. This returns a +/// commonly, [`open_protocol_exclusive`] should be used. This ensures that +/// nothing else can use the protocol until it is closed, and returns a /// [`ScopedProtocol`] that takes care of closing the protocol when it is -/// dropped. If the protocol is opened in [`Exclusive`] mode, UEFI ensures that -/// nothing else can use the protocol until it is closed. +/// dropped. /// /// Other methods for opening protocols: /// +/// * [`open_protocol`] /// * [`get_image_file_system`] /// * [`handle_protocol`] /// * [`locate_protocol`] @@ -43,7 +58,7 @@ use core::{ptr, slice}; /// For protocol definitions, see the [`proto`] module. /// /// [`proto`]: crate::proto -/// [`Exclusive`]: OpenProtocolAttributes::Exclusive +/// [`open_protocol_exclusive`]: BootServices::open_protocol_exclusive /// [`open_protocol`]: BootServices::open_protocol /// [`get_image_file_system`]: BootServices::get_image_file_system /// [`locate_protocol`]: BootServices::locate_protocol @@ -238,6 +253,46 @@ pub struct BootServices { } impl BootServices { + /// Get the [`Handle`] of the currently-executing image. + pub fn image_handle(&self) -> Handle { + // Safety: + // + // `IMAGE_HANDLE` is only set by `set_image_handle`, see that + // documentation for more details. + // + // Additionally, `image_handle` takes a `&self` which ensures it + // can only be called while boot services are active. (After + // exiting boot services, the image handle should not be + // considered valid.) + unsafe { + IMAGE_HANDLE + .handle + .get() + .read() + .expect("set_image_handle has not been called") + } + } + + /// Update the global image [`Handle`]. + /// + /// This is called automatically in the `main` entry point as part + /// of [`uefi_macros::entry`]. It should not be called at any other + /// point in time, unless the executable does not use + /// [`uefi_macros::entry`], in which case it should be called once + /// before calling other `BootServices` functions. + /// + /// # Safety + /// + /// This function should only be called as described above. The + /// safety guarantees of [`BootServices::open_protocol_exclusive`] + /// rely on the global image handle being correct. + pub unsafe fn set_image_handle(&self, image_handle: Handle) { + // As with `image_handle`, `&self` isn't actually used, but it + // enforces that this function is only called while boot + // services are active. + IMAGE_HANDLE.handle.get().write(Some(image_handle)); + } + /// Raises a task's priority level and returns its previous level. /// /// The effect of calling `raise_tpl` with a `Tpl` that is below the current @@ -569,7 +624,7 @@ impl BootServices { /// based on the protocol GUID. /// /// It is recommended that all new drivers and applications use - /// [`open_protocol`] instead of `handle_protocol`. + /// [`open_protocol_exclusive`] or [`open_protocol`] instead of `handle_protocol`. /// /// UEFI protocols are neither thread-safe nor reentrant, but the firmware /// provides no mechanism to protect against concurrent usage. Such @@ -581,10 +636,14 @@ impl BootServices { /// This method is unsafe because the handle database is not /// notified that the handle and protocol are in use; there is no /// guarantee that they will remain valid for the duration of their - /// use. Use [`open_protocol`] instead. + /// use. Use [`open_protocol_exclusive`] if possible, otherwise use + /// [`open_protocol`]. /// /// [`open_protocol`]: BootServices::open_protocol - #[deprecated(note = "it is recommended to use `open_protocol` instead")] + /// [`open_protocol_exclusive`]: BootServices::open_protocol_exclusive + #[deprecated( + note = "it is recommended to use `open_protocol_exclusive` or `open_protocol` instead" + )] pub unsafe fn handle_protocol( &self, handle: Handle, @@ -683,14 +742,7 @@ impl BootServices { /// # let boot_services: &BootServices = get_fake_val(); /// # let image_handle: Handle = get_fake_val(); /// let handle = boot_services.get_handle_for_protocol::()?; - /// let device_path_to_text = boot_services.open_protocol::( - /// OpenProtocolParams { - /// handle, - /// agent: image_handle, - /// controller: None, - /// }, - /// OpenProtocolAttributes::Exclusive, - /// )?; + /// let device_path_to_text = boot_services.open_protocol_exclusive::(handle)?; /// # Ok(()) /// # } /// ``` @@ -909,11 +961,14 @@ impl BootServices { /// Open a protocol interface for a handle. /// + /// See also [`open_protocol_exclusive`], which provides a safe + /// subset of this functionality. + /// /// This function attempts to get the protocol implementation of a /// handle, based on the protocol GUID. It is an extended version of /// [`handle_protocol`]. It is recommended that all - /// new drivers and applications use `open_protocol` instead of - /// `handle_protocol`. + /// new drivers and applications use `open_protocol_exclusive` or + /// `open_protocol` instead of `handle_protocol`. /// /// See [`OpenProtocolParams`] and [`OpenProtocolAttributes`] for /// details of the input parameters. @@ -926,8 +981,19 @@ impl BootServices { /// protections must be implemented by user-level code, for example via a /// global `HashSet`. /// + /// # Safety + /// + /// This function is unsafe because it can be used to open a + /// protocol in ways that don't get tracked by the UEFI + /// implementation. This could allow the protocol to be removed from + /// a handle, or for the handle to be deleted entirely, while a + /// reference to the protocol is still active. The caller is + /// responsible for ensuring that the handle and protocol remain + /// valid until the `ScopedProtocol` is dropped. + /// /// [`handle_protocol`]: BootServices::handle_protocol - pub fn open_protocol( + /// [`open_protocol_exclusive`]: BootServices::open_protocol_exclusive + pub unsafe fn open_protocol( &self, params: OpenProtocolParams, attributes: OpenProtocolAttributes, @@ -941,7 +1007,7 @@ impl BootServices { params.controller, attributes as u32, ) - .into_with_val(|| unsafe { + .into_with_val(|| { let interface = P::mut_ptr_from_ffi(interface) as *const UnsafeCell

; #[allow(deprecated)] @@ -953,6 +1019,31 @@ impl BootServices { }) } + /// Open a protocol interface for a handle in exclusive mode. + /// + /// If successful, a [`ScopedProtocol`] is returned that will + /// automatically close the protocol interface when dropped. + /// + /// [`handle_protocol`]: BootServices::handle_protocol + pub fn open_protocol_exclusive( + &self, + handle: Handle, + ) -> Result> { + // Safety: opening in exclusive mode with the correct agent + // handle set ensures that the protocol cannot be modified or + // removed while it is open, so this usage is safe. + unsafe { + self.open_protocol::

( + OpenProtocolParams { + handle, + agent: self.image_handle(), + controller: None, + }, + OpenProtocolAttributes::Exclusive, + ) + } + } + /// Test whether a handle supports a protocol. pub fn test_protocol(&self, params: OpenProtocolParams) -> Result<()> { const TEST_PROTOCOL: u32 = 0x04; @@ -1025,9 +1116,15 @@ impl BootServices { /// This method is unsafe because the handle database is not /// notified that the handle and protocol are in use; there is no /// guarantee that they will remain valid for the duration of their - /// use. Use [`BootServices::get_handle_for_protocol`] and - /// [`BootServices::open_protocol`] instead. - #[deprecated(note = "it is recommended to use `open_protocol` instead")] + /// use. Use [`get_handle_for_protocol`] and either + /// [`open_protocol_exclusive`] or [`open_protocol`] instead. + /// + /// [`get_handle_for_protocol`]: BootServices::get_handle_for_protocol + /// [`open_protocol`]: BootServices::open_protocol + /// [`open_protocol_exclusive`]: BootServices::open_protocol_exclusive + #[deprecated( + note = "it is recommended to use `open_protocol_exclusive` or `open_protocol` instead" + )] pub unsafe fn locate_protocol(&self) -> Result<&UnsafeCell

> { let mut ptr = ptr::null_mut(); (self.locate_protocol)(&P::GUID, ptr::null_mut(), &mut ptr).into_with_val(|| { @@ -1092,34 +1189,13 @@ impl BootServices { &self, image_handle: Handle, ) -> Result> { - let loaded_image = self.open_protocol::( - OpenProtocolParams { - handle: image_handle, - agent: image_handle, - controller: None, - }, - OpenProtocolAttributes::Exclusive, - )?; - - let device_path = self.open_protocol::( - OpenProtocolParams { - handle: loaded_image.device(), - agent: image_handle, - controller: None, - }, - OpenProtocolAttributes::Exclusive, - )?; + let loaded_image = self.open_protocol_exclusive::(image_handle)?; + + let device_path = self.open_protocol_exclusive::(loaded_image.device())?; let device_handle = self.locate_device_path::(&mut &*device_path)?; - self.open_protocol::( - OpenProtocolParams { - handle: device_handle, - agent: image_handle, - controller: None, - }, - OpenProtocolAttributes::Exclusive, - ) + self.open_protocol_exclusive(device_handle) } } diff --git a/uefi-macros/src/lib.rs b/uefi-macros/src/lib.rs index dba437f4e..38717dfdb 100644 --- a/uefi-macros/src/lib.rs +++ b/uefi-macros/src/lib.rs @@ -8,9 +8,9 @@ use proc_macro2::{TokenStream as TokenStream2, TokenTree}; use quote::{quote, ToTokens, TokenStreamExt}; use syn::{ parse::{Parse, ParseStream}, - parse_macro_input, + parse_macro_input, parse_quote, spanned::Spanned, - DeriveInput, Error, Generics, Ident, ItemFn, ItemType, LitStr, Visibility, + DeriveInput, Error, FnArg, Generics, Ident, ItemFn, ItemType, LitStr, Pat, Visibility, }; /// Parses a type definition, extracts its identifier and generic parameters @@ -157,7 +157,69 @@ pub fn derive_protocol(item: TokenStream) -> TokenStream { result.into() } -/// Custom attribute for a UEFI executable entrypoint +/// Get the name of a function's argument at `arg_index`. +fn get_function_arg_name(f: &ItemFn, arg_index: usize, errors: &mut TokenStream2) -> Option { + if let Some(FnArg::Typed(arg)) = f.sig.inputs.iter().nth(arg_index) { + if let Pat::Ident(pat_ident) = &*arg.pat { + // The argument has a valid name such as `handle` or `_handle`. + Some(pat_ident.ident.clone()) + } else { + // The argument is unnamed, i.e. `_`. + errors.append_all(err!(arg.span(), "Entry method's arguments must be named")); + None + } + } else { + // Either there are too few arguments, or it's the wrong kind of + // argument (e.g. `self`). + // + // Don't append an error in this case. The error will be caught + // by the typecheck later on, which will give a better error + // message. + None + } +} + +/// Custom attribute for a UEFI executable entry point. +/// +/// This attribute modifies a function to mark it as the entry point for +/// a UEFI executable. The function must have two parameters, [`Handle`] +/// and [`SystemTable`], and return a [`Status`]. The function can +/// optionally be `unsafe`. +/// +/// Due to internal implementation details the parameters must both be +/// named, so `arg` or `_arg` are allowed, but not `_`. +/// +/// The [`BootServices::set_image_handle`] function will be called +/// automatically with the image [`Handle`] argument. +/// +/// # Examples +/// +/// ```no_run +/// #![no_main] +/// #![no_std] +/// #![feature(abi_efiapi)] +/// # // A bit of boilerplate needed to make the example compile in the +/// # // context of `cargo test`. +/// # #![feature(lang_items)] +/// # #[lang = "eh_personality"] +/// # fn eh_personality() {} +/// # #[panic_handler] +/// # fn panic_handler(info: &core::panic::PanicInfo) -> ! { +/// # loop {} +/// # } +/// +/// use uefi::prelude::*; +/// +/// #[entry] +/// fn main(image: Handle, st: SystemTable) -> Status { +/// Status::SUCCESS +/// } +/// ``` +/// +/// [`Handle`]: https://docs.rs/uefi/latest/uefi/data_types/struct.Handle.html +/// [`SystemTable`]: https://docs.rs/uefi/latest/uefi/table/struct.SystemTable.html +/// [`Status`]: https://docs.rs/uefi/latest/uefi/struct.Status.html +/// [`BootServices::set_image_handle`]: https://docs.rs/uefi/latest/uefi/table/boot/struct.BootServices.html#method.set_image_handle #[proc_macro_attribute] pub fn entry(args: TokenStream, input: TokenStream) -> TokenStream { // This code is inspired by the approach in this embedded Rust crate: @@ -190,6 +252,9 @@ pub fn entry(args: TokenStream, input: TokenStream) -> TokenStream { )); } + let image_handle_ident = get_function_arg_name(&f, 0, &mut errors); + let system_table_ident = get_function_arg_name(&f, 1, &mut errors); + // show most errors at once instead of one by one if !errors.is_empty() { return errors.into(); @@ -199,6 +264,18 @@ pub fn entry(args: TokenStream, input: TokenStream) -> TokenStream { let unsafety = f.sig.unsafety.take(); // strip any visibility modifiers f.vis = Visibility::Inherited; + // Set the global image handle. If `image_handle_ident` is `None` + // then the typecheck is going to fail anyway. + if let Some(image_handle_ident) = image_handle_ident { + f.block.stmts.insert( + 0, + parse_quote! { + unsafe { + #system_table_ident.boot_services().set_image_handle(#image_handle_ident); + } + }, + ); + } let ident = &f.sig.ident; diff --git a/uefi-macros/tests/ui/entry_unnamed_image_arg.rs b/uefi-macros/tests/ui/entry_unnamed_image_arg.rs new file mode 100644 index 000000000..866baa691 --- /dev/null +++ b/uefi-macros/tests/ui/entry_unnamed_image_arg.rs @@ -0,0 +1,11 @@ +#![allow(unused_imports)] +#![no_main] +#![feature(abi_efiapi)] + +use uefi::prelude::*; +use uefi_macros::entry; + +#[entry] +fn unnamed_image_arg(_: Handle, _st: SystemTable) -> Status { + Status::SUCCESS +} diff --git a/uefi-macros/tests/ui/entry_unnamed_image_arg.stderr b/uefi-macros/tests/ui/entry_unnamed_image_arg.stderr new file mode 100644 index 000000000..bdf628d40 --- /dev/null +++ b/uefi-macros/tests/ui/entry_unnamed_image_arg.stderr @@ -0,0 +1,5 @@ +error: Entry method's arguments must be named + --> tests/ui/entry_unnamed_image_arg.rs:9:22 + | +9 | fn unnamed_image_arg(_: Handle, _st: SystemTable) -> Status { + | ^^^^^^^^^ diff --git a/uefi-macros/tests/ui/entry_unnamed_table_arg.rs b/uefi-macros/tests/ui/entry_unnamed_table_arg.rs new file mode 100644 index 000000000..3da0c43ae --- /dev/null +++ b/uefi-macros/tests/ui/entry_unnamed_table_arg.rs @@ -0,0 +1,11 @@ +#![allow(unused_imports)] +#![no_main] +#![feature(abi_efiapi)] + +use uefi::prelude::*; +use uefi_macros::entry; + +#[entry] +fn unnamed_table_arg(_image: Handle, _: SystemTable) -> Status { + Status::SUCCESS +} diff --git a/uefi-macros/tests/ui/entry_unnamed_table_arg.stderr b/uefi-macros/tests/ui/entry_unnamed_table_arg.stderr new file mode 100644 index 000000000..45143cd16 --- /dev/null +++ b/uefi-macros/tests/ui/entry_unnamed_table_arg.stderr @@ -0,0 +1,5 @@ +error: Entry method's arguments must be named + --> tests/ui/entry_unnamed_table_arg.rs:9:38 + | +9 | fn unnamed_table_arg(_image: Handle, _: SystemTable) -> Status { + | ^^^^^^^^^^^^^^^^^^^^ diff --git a/uefi-test-runner/src/main.rs b/uefi-test-runner/src/main.rs index 2d18969b5..cb807dbd2 100644 --- a/uefi-test-runner/src/main.rs +++ b/uefi-test-runner/src/main.rs @@ -10,7 +10,6 @@ extern crate alloc; use alloc::string::String; use uefi::prelude::*; use uefi::proto::console::serial::Serial; -use uefi::table::boot::{OpenProtocolAttributes, OpenProtocolParams}; use uefi_services::{print, println}; mod boot; @@ -81,7 +80,7 @@ fn check_revision(rev: uefi::table::Revision) { /// This functionality is very specific to our QEMU-based test runner. Outside /// of it, we just pause the tests for a couple of seconds to allow visual /// inspection of the output. -fn check_screenshot(image: Handle, bt: &BootServices, name: &str) { +fn check_screenshot(bt: &BootServices, name: &str) { if cfg!(feature = "qemu") { let serial_handles = bt .find_handles::() @@ -96,14 +95,7 @@ fn check_screenshot(image: Handle, bt: &BootServices, name: &str) { .expect("Second serial device is missing"); let mut serial = bt - .open_protocol::( - OpenProtocolParams { - handle: serial_handle, - agent: image, - controller: None, - }, - OpenProtocolAttributes::Exclusive, - ) + .open_protocol_exclusive::(serial_handle) .expect("Could not open serial protocol"); // Set a large timeout to avoid problems with Travis diff --git a/uefi-test-runner/src/proto/console/gop.rs b/uefi-test-runner/src/proto/console/gop.rs index 83c06b1a3..0269a4302 100644 --- a/uefi-test-runner/src/proto/console/gop.rs +++ b/uefi-test-runner/src/proto/console/gop.rs @@ -2,7 +2,7 @@ use uefi::prelude::*; use uefi::proto::console::gop::{BltOp, BltPixel, FrameBuffer, GraphicsOutput, PixelFormat}; use uefi::table::boot::{BootServices, OpenProtocolAttributes, OpenProtocolParams}; -pub fn test(image: Handle, bt: &BootServices) { +pub unsafe fn test(image: Handle, bt: &BootServices) { info!("Running graphics output protocol test"); if let Ok(handle) = bt.get_handle_for_protocol::() { let gop = &mut bt @@ -23,7 +23,7 @@ pub fn test(image: Handle, bt: &BootServices) { fill_color(gop); draw_fb(gop); - crate::check_screenshot(image, bt, "gop_test"); + crate::check_screenshot(bt, "gop_test"); } else { // No tests can be run. warn!("UEFI Graphics Output Protocol is not supported"); diff --git a/uefi-test-runner/src/proto/console/mod.rs b/uefi-test-runner/src/proto/console/mod.rs index 68b6f8b85..2708e845e 100644 --- a/uefi-test-runner/src/proto/console/mod.rs +++ b/uefi-test-runner/src/proto/console/mod.rs @@ -6,9 +6,11 @@ pub fn test(image: Handle, st: &mut SystemTable) { stdout::test(st.stdout()); let bt = st.boot_services(); - serial::test(image, bt); - gop::test(image, bt); - pointer::test(image, bt); + unsafe { + serial::test(image, bt); + gop::test(image, bt); + } + pointer::test(bt); } mod gop; diff --git a/uefi-test-runner/src/proto/console/pointer.rs b/uefi-test-runner/src/proto/console/pointer.rs index da1d163a1..933e3809a 100644 --- a/uefi-test-runner/src/proto/console/pointer.rs +++ b/uefi-test-runner/src/proto/console/pointer.rs @@ -1,19 +1,11 @@ use uefi::proto::console::pointer::Pointer; -use uefi::table::boot::{BootServices, OpenProtocolAttributes, OpenProtocolParams}; -use uefi::Handle; +use uefi::table::boot::BootServices; -pub fn test(image: Handle, bt: &BootServices) { +pub fn test(bt: &BootServices) { info!("Running pointer protocol test"); if let Ok(handle) = bt.get_handle_for_protocol::() { let mut pointer = bt - .open_protocol::( - OpenProtocolParams { - handle, - agent: image, - controller: None, - }, - OpenProtocolAttributes::Exclusive, - ) + .open_protocol_exclusive::(handle) .expect("failed to open pointer protocol"); pointer diff --git a/uefi-test-runner/src/proto/console/serial.rs b/uefi-test-runner/src/proto/console/serial.rs index e69604bb0..2160b2b6b 100644 --- a/uefi-test-runner/src/proto/console/serial.rs +++ b/uefi-test-runner/src/proto/console/serial.rs @@ -2,7 +2,7 @@ use uefi::proto::console::serial::{ControlBits, Serial}; use uefi::table::boot::{BootServices, OpenProtocolAttributes, OpenProtocolParams}; use uefi::Handle; -pub fn test(image: Handle, bt: &BootServices) { +pub unsafe fn test(image: Handle, bt: &BootServices) { info!("Running serial protocol test"); if let Ok(handle) = bt.get_handle_for_protocol::() { let mut serial = bt diff --git a/uefi-test-runner/src/proto/debug.rs b/uefi-test-runner/src/proto/debug.rs index 37b5e23f6..1884f9cb1 100644 --- a/uefi-test-runner/src/proto/debug.rs +++ b/uefi-test-runner/src/proto/debug.rs @@ -1,20 +1,12 @@ use core::ffi::c_void; use uefi::proto::debug::{DebugSupport, ExceptionType, ProcessorArch, SystemContext}; -use uefi::table::boot::{BootServices, OpenProtocolAttributes, OpenProtocolParams}; -use uefi::Handle; +use uefi::table::boot::BootServices; -pub fn test(image: Handle, bt: &BootServices) { +pub fn test(bt: &BootServices) { info!("Running UEFI debug connection protocol test"); if let Ok(handles) = bt.find_handles::() { for handle in handles { - if let Ok(mut debug_support) = bt.open_protocol::( - OpenProtocolParams { - handle, - agent: image, - controller: None, - }, - OpenProtocolAttributes::Exclusive, - ) { + if let Ok(mut debug_support) = bt.open_protocol_exclusive::(handle) { // make sure that the max processor index is a sane value, i.e. it works let maximum_processor_index = debug_support.get_maximum_processor_index(); assert_ne!( diff --git a/uefi-test-runner/src/proto/device_path.rs b/uefi-test-runner/src/proto/device_path.rs index b30f6ccd6..8aa99459b 100644 --- a/uefi-test-runner/src/proto/device_path.rs +++ b/uefi-test-runner/src/proto/device_path.rs @@ -1,56 +1,30 @@ use uefi::prelude::*; use uefi::proto::device_path::{text::*, DevicePath}; use uefi::proto::loaded_image::LoadedImage; -use uefi::table::boot::{BootServices, OpenProtocolAttributes, OpenProtocolParams}; +use uefi::table::boot::BootServices; pub fn test(image: Handle, bt: &BootServices) { info!("Running device path protocol test"); let loaded_image = bt - .open_protocol::( - OpenProtocolParams { - handle: image, - agent: image, - controller: None, - }, - OpenProtocolAttributes::Exclusive, - ) + .open_protocol_exclusive::(image) .expect("Failed to open LoadedImage protocol"); let device_path = bt - .open_protocol::( - OpenProtocolParams { - handle: loaded_image.device(), - agent: image, - controller: None, - }, - OpenProtocolAttributes::Exclusive, - ) + .open_protocol_exclusive::(loaded_image.device()) .expect("Failed to open DevicePath protocol"); let device_path_to_text = bt - .open_protocol::( - OpenProtocolParams { - handle: bt - .get_handle_for_protocol::() - .expect("Failed to get DevicePathToText handle"), - agent: image, - controller: None, - }, - OpenProtocolAttributes::Exclusive, + .open_protocol_exclusive::( + bt.get_handle_for_protocol::() + .expect("Failed to get DevicePathToText handle"), ) .expect("Failed to open DevicePathToText protocol"); let device_path_from_text = bt - .open_protocol::( - OpenProtocolParams { - handle: bt - .get_handle_for_protocol::() - .expect("Failed to get DevicePathFromText handle"), - agent: image, - controller: None, - }, - OpenProtocolAttributes::Exclusive, + .open_protocol_exclusive::( + bt.get_handle_for_protocol::() + .expect("Failed to get DevicePathFromText handle"), ) .expect("Failed to open DevicePathFromText protocol"); diff --git a/uefi-test-runner/src/proto/loaded_image.rs b/uefi-test-runner/src/proto/loaded_image.rs index e21580d91..76b7e1fe5 100644 --- a/uefi-test-runner/src/proto/loaded_image.rs +++ b/uefi-test-runner/src/proto/loaded_image.rs @@ -1,19 +1,12 @@ use uefi::prelude::*; use uefi::proto::loaded_image::LoadedImage; -use uefi::table::boot::{BootServices, OpenProtocolAttributes, OpenProtocolParams}; +use uefi::table::boot::BootServices; pub fn test(image: Handle, bt: &BootServices) { info!("Running loaded image protocol test"); let loaded_image = bt - .open_protocol::( - OpenProtocolParams { - handle: image, - agent: image, - controller: None, - }, - OpenProtocolAttributes::Exclusive, - ) + .open_protocol_exclusive::(image) .expect("Failed to open LoadedImage protocol"); let load_options = loaded_image.load_options_as_bytes(); diff --git a/uefi-test-runner/src/proto/media/known_disk.rs b/uefi-test-runner/src/proto/media/known_disk.rs index 6b69390c6..72926755c 100644 --- a/uefi-test-runner/src/proto/media/known_disk.rs +++ b/uefi-test-runner/src/proto/media/known_disk.rs @@ -141,38 +141,40 @@ fn test_create_file(directory: &mut Directory) { file.write(b"test output data").unwrap(); } +/// Get the media ID via the BlockIO protocol. +fn get_block_media_id(handle: Handle, bt: &BootServices) -> u32 { + // This cannot be opened in `EXCLUSIVE` mode, as doing so + // unregisters the `DiskIO` protocol from the handle. + unsafe { + let block_io = bt + .open_protocol::( + OpenProtocolParams { + handle, + agent: bt.image_handle(), + controller: None, + }, + OpenProtocolAttributes::GetProtocol, + ) + .expect("Failed to get block I/O protocol"); + block_io.media().media_id() + } +} + /// Tests raw disk I/O. -fn test_raw_disk_io(handle: Handle, image: Handle, bt: &BootServices) { +fn test_raw_disk_io(handle: Handle, bt: &BootServices) { info!("Testing raw disk I/O"); - // Open the block I/O protocol on the handle - let block_io = bt - .open_protocol::( - OpenProtocolParams { - handle, - agent: image, - controller: None, - }, - OpenProtocolAttributes::GetProtocol, - ) - .expect("Failed to get block I/O protocol"); + let media_id = get_block_media_id(handle, bt); // Open the disk I/O protocol on the input handle let disk_io = bt - .open_protocol::( - OpenProtocolParams { - handle, - agent: image, - controller: None, - }, - OpenProtocolAttributes::GetProtocol, - ) + .open_protocol_exclusive::(handle) .expect("Failed to get disk I/O protocol"); // Read from the first sector of the disk into the buffer let mut buf = vec![0; 512]; disk_io - .read_disk(block_io.media().media_id(), 0, &mut buf) + .read_disk(media_id, 0, &mut buf) .expect("Failed to read from disk"); // Verify that the disk's MBR signature is correct @@ -192,29 +194,12 @@ struct DiskIoTask { } /// Tests raw disk I/O through the DiskIo2 protocol. -fn test_raw_disk_io2(handle: Handle, image: Handle, bt: &BootServices) { +fn test_raw_disk_io2(handle: Handle, bt: &BootServices) { info!("Testing raw disk I/O 2"); // Open the disk I/O protocol on the input handle - if let Ok(disk_io2) = bt.open_protocol::( - OpenProtocolParams { - handle, - agent: image, - controller: None, - }, - OpenProtocolAttributes::GetProtocol, - ) { - // Open the block I/O protocol on the handle - let block_io = bt - .open_protocol::( - OpenProtocolParams { - handle, - agent: image, - controller: None, - }, - OpenProtocolAttributes::GetProtocol, - ) - .expect("Failed to get block I/O protocol"); + if let Ok(disk_io2) = bt.open_protocol_exclusive::(handle) { + let media_id = get_block_media_id(handle, bt); unsafe { // Create the completion event @@ -234,7 +219,7 @@ fn test_raw_disk_io2(handle: Handle, image: Handle, bt: &BootServices) { // Initiate the asynchronous read operation disk_io2 .read_disk_raw( - block_io.media().media_id(), + media_id, 0, NonNull::new(&mut task.token as _), task.buffer.len(), @@ -258,7 +243,7 @@ fn test_raw_disk_io2(handle: Handle, image: Handle, bt: &BootServices) { /// Run various tests on a special test disk. The disk is created by /// xtask/src/disk.rs. -pub fn test_known_disk(image: Handle, bt: &BootServices) { +pub fn test_known_disk(bt: &BootServices) { // This test is only valid when running in the specially-prepared // qemu with the test disk. if !cfg!(feature = "qemu") { @@ -272,47 +257,41 @@ pub fn test_known_disk(image: Handle, bt: &BootServices) { let mut found_test_disk = false; for handle in handles { - let mut sfs = bt - .open_protocol::( - OpenProtocolParams { - handle, - agent: image, - controller: None, - }, - OpenProtocolAttributes::Exclusive, - ) - .expect("Failed to get simple file system"); - let mut directory = sfs.open_volume().unwrap(); - - let mut fs_info_buf = vec![0; 128]; - let fs_info = directory - .get_info::(&mut fs_info_buf) - .unwrap(); - - if fs_info.volume_label().to_string() == "MbrTestDisk" { - info!("Checking MbrTestDisk"); - found_test_disk = true; - } else { - continue; + { + let mut sfs = bt + .open_protocol_exclusive::(handle) + .expect("Failed to get simple file system"); + let mut directory = sfs.open_volume().unwrap(); + + let mut fs_info_buf = vec![0; 128]; + let fs_info = directory + .get_info::(&mut fs_info_buf) + .unwrap(); + + if fs_info.volume_label().to_string() == "MbrTestDisk" { + info!("Checking MbrTestDisk"); + found_test_disk = true; + } else { + continue; + } + + assert!(!fs_info.read_only()); + assert_eq!(fs_info.volume_size(), 512 * 1192); + assert_eq!(fs_info.free_space(), 512 * 1190); + assert_eq!(fs_info.block_size(), 512); + + // Check that `get_boxed_info` returns the same info. + let boxed_fs_info = directory.get_boxed_info::().unwrap(); + assert_eq!(*fs_info, *boxed_fs_info); + + test_existing_dir(&mut directory); + test_delete_warning(&mut directory); + test_existing_file(&mut directory); + test_create_file(&mut directory); } - // Test raw disk I/O first - test_raw_disk_io(handle, image, bt); - test_raw_disk_io2(handle, image, bt); - - assert!(!fs_info.read_only()); - assert_eq!(fs_info.volume_size(), 512 * 1192); - assert_eq!(fs_info.free_space(), 512 * 1190); - assert_eq!(fs_info.block_size(), 512); - - // Check that `get_boxed_info` returns the same info. - let boxed_fs_info = directory.get_boxed_info::().unwrap(); - assert_eq!(*fs_info, *boxed_fs_info); - - test_existing_dir(&mut directory); - test_delete_warning(&mut directory); - test_existing_file(&mut directory); - test_create_file(&mut directory); + test_raw_disk_io(handle, bt); + test_raw_disk_io2(handle, bt); } if !found_test_disk { diff --git a/uefi-test-runner/src/proto/media/mod.rs b/uefi-test-runner/src/proto/media/mod.rs index 44865cecf..83d5aa663 100644 --- a/uefi-test-runner/src/proto/media/mod.rs +++ b/uefi-test-runner/src/proto/media/mod.rs @@ -4,7 +4,6 @@ use uefi::prelude::*; use uefi::proto::media::file::{Directory, File, FileSystemInfo, FileSystemVolumeLabel}; use uefi::proto::media::fs::SimpleFileSystem; use uefi::proto::media::partition::PartitionInfo; -use uefi::table::boot::{OpenProtocolAttributes, OpenProtocolParams}; /// Test `FileSystemInfo` and `FileSystemVolumeLabel`. fn test_file_system_info(directory: &mut Directory) { @@ -24,19 +23,12 @@ fn test_file_system_info(directory: &mut Directory) { assert_eq!(fs_info.volume_label(), fs_vol.volume_label()); } -pub fn test(image: Handle, bt: &BootServices) { +pub fn test(bt: &BootServices) { info!("Testing Media Access protocols"); if let Ok(handle) = bt.get_handle_for_protocol::() { let mut sfs = bt - .open_protocol::( - OpenProtocolParams { - handle, - agent: image, - controller: None, - }, - OpenProtocolAttributes::Exclusive, - ) + .open_protocol_exclusive::(handle) .expect("failed to open SimpleFileSystem protocol"); let mut directory = sfs.open_volume().unwrap(); @@ -73,14 +65,7 @@ pub fn test(image: Handle, bt: &BootServices) { for handle in handles { let pi = bt - .open_protocol::( - OpenProtocolParams { - handle, - agent: image, - controller: None, - }, - OpenProtocolAttributes::Exclusive, - ) + .open_protocol_exclusive::(handle) .expect("Failed to get partition info"); if let Some(mbr) = pi.mbr_partition_record() { @@ -92,5 +77,5 @@ pub fn test(image: Handle, bt: &BootServices) { } } - known_disk::test_known_disk(image, bt); + known_disk::test_known_disk(bt); } diff --git a/uefi-test-runner/src/proto/mod.rs b/uefi-test-runner/src/proto/mod.rs index 15ce02537..d254e71cb 100644 --- a/uefi-test-runner/src/proto/mod.rs +++ b/uefi-test-runner/src/proto/mod.rs @@ -12,13 +12,13 @@ pub fn test(image: Handle, st: &mut SystemTable) { find_protocol(bt); test_protocols_per_handle(image, bt); - debug::test(image, bt); + debug::test(bt); device_path::test(image, bt); loaded_image::test(image, bt); - media::test(image, bt); - network::test(image, bt); - pi::test(image, bt); - rng::test(image, bt); + media::test(bt); + network::test(bt); + pi::test(bt); + rng::test(bt); #[cfg(any( target_arch = "i386", @@ -26,7 +26,7 @@ pub fn test(image: Handle, st: &mut SystemTable) { target_arch = "arm", target_arch = "aarch64" ))] - shim::test(image, bt); + shim::test(bt); } fn find_protocol(bt: &BootServices) { diff --git a/uefi-test-runner/src/proto/network.rs b/uefi-test-runner/src/proto/network.rs index a1dc49e31..f19ba9026 100644 --- a/uefi-test-runner/src/proto/network.rs +++ b/uefi-test-runner/src/proto/network.rs @@ -4,25 +4,15 @@ use uefi::{ pxe::{BaseCode, DhcpV4Packet, IpFilter, IpFilters, UdpOpFlags}, IpAddress, }, - table::boot::{OpenProtocolAttributes, OpenProtocolParams}, - CStr8, Handle, + CStr8, }; -pub fn test(image: Handle, bt: &BootServices) { +pub fn test(bt: &BootServices) { info!("Testing Network protocols"); if let Ok(handles) = bt.find_handles::() { for handle in handles { - let mut base_code = bt - .open_protocol::( - OpenProtocolParams { - handle, - agent: image, - controller: None, - }, - OpenProtocolAttributes::Exclusive, - ) - .unwrap(); + let mut base_code = bt.open_protocol_exclusive::(handle).unwrap(); info!("Starting PXE Base Code"); base_code diff --git a/uefi-test-runner/src/proto/pi/mod.rs b/uefi-test-runner/src/proto/pi/mod.rs index 5ded0f6ae..85c7ff2d6 100644 --- a/uefi-test-runner/src/proto/pi/mod.rs +++ b/uefi-test-runner/src/proto/pi/mod.rs @@ -1,9 +1,9 @@ use uefi::prelude::*; -pub fn test(image: Handle, bt: &BootServices) { +pub fn test(bt: &BootServices) { info!("Testing Platform Initialization protocols"); - mp::test(image, bt); + mp::test(bt); } mod mp; diff --git a/uefi-test-runner/src/proto/pi/mp.rs b/uefi-test-runner/src/proto/pi/mp.rs index cadd15840..a21d53ecd 100644 --- a/uefi-test-runner/src/proto/pi/mp.rs +++ b/uefi-test-runner/src/proto/pi/mp.rs @@ -2,13 +2,13 @@ use core::ffi::c_void; use core::sync::atomic::{AtomicUsize, Ordering}; use core::time::Duration; use uefi::proto::pi::mp::MpServices; -use uefi::table::boot::{BootServices, OpenProtocolAttributes, OpenProtocolParams}; -use uefi::{Handle, Status}; +use uefi::table::boot::BootServices; +use uefi::Status; /// Number of cores qemu is configured to have const NUM_CPUS: usize = 4; -pub fn test(image: Handle, bt: &BootServices) { +pub fn test(bt: &BootServices) { // These tests break CI. See #103. if cfg!(feature = "ci") { return; @@ -17,14 +17,7 @@ pub fn test(image: Handle, bt: &BootServices) { info!("Running UEFI multi-processor services protocol test"); if let Ok(handle) = bt.get_handle_for_protocol::() { let mp_support = &bt - .open_protocol::( - OpenProtocolParams { - handle, - agent: image, - controller: None, - }, - OpenProtocolAttributes::Exclusive, - ) + .open_protocol_exclusive::(handle) .expect("failed to open multi-processor services protocol"); test_get_number_of_processors(mp_support); diff --git a/uefi-test-runner/src/proto/rng.rs b/uefi-test-runner/src/proto/rng.rs index 0bd63c9fa..572588396 100644 --- a/uefi-test-runner/src/proto/rng.rs +++ b/uefi-test-runner/src/proto/rng.rs @@ -1,21 +1,13 @@ -use uefi::prelude::*; use uefi::proto::rng::{Rng, RngAlgorithmType}; -use uefi::table::boot::{BootServices, OpenProtocolAttributes, OpenProtocolParams}; +use uefi::table::boot::BootServices; -pub fn test(image: Handle, bt: &BootServices) { +pub fn test(bt: &BootServices) { info!("Running rng protocol test"); let handle = bt.get_handle_for_protocol::().expect("No Rng handles"); let mut rng = bt - .open_protocol::( - OpenProtocolParams { - handle, - agent: image, - controller: None, - }, - OpenProtocolAttributes::Exclusive, - ) + .open_protocol_exclusive::(handle) .expect("Failed to open Rng protocol"); let mut list = [RngAlgorithmType::EMPTY_ALGORITHM; 4]; diff --git a/uefi-test-runner/src/proto/shim.rs b/uefi-test-runner/src/proto/shim.rs index e19527dd8..333c1da86 100644 --- a/uefi-test-runner/src/proto/shim.rs +++ b/uefi-test-runner/src/proto/shim.rs @@ -1,20 +1,12 @@ use uefi::proto::shim::ShimLock; -use uefi::table::boot::{BootServices, OpenProtocolAttributes, OpenProtocolParams}; -use uefi::Handle; +use uefi::table::boot::BootServices; -pub fn test(image: Handle, bt: &BootServices) { +pub fn test(bt: &BootServices) { info!("Running shim lock protocol test"); if let Ok(handle) = bt.get_handle_for_protocol::() { let shim_lock = bt - .open_protocol::( - OpenProtocolParams { - handle, - agent: image, - controller: None, - }, - OpenProtocolAttributes::Exclusive, - ) + .open_protocol_exclusive::(handle) .expect("failed to open shim lock protocol"); // An empty buffer should definitely be invalid, so expect