From 4bc299735639d8012c232d81eb534dfa64054eb8 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Sun, 11 Aug 2024 12:28:20 +0200 Subject: [PATCH 1/4] uefi-raw: add opaque Boolean type This way we can be ABI-compatible and guarantee lack of UB, while being more precise in interfaces. --- uefi-raw/CHANGELOG.md | 4 +- uefi-raw/Cargo.toml | 4 ++ uefi-raw/src/lib.rs | 87 +++++++++++++++++++++++++- uefi-raw/src/protocol/block.rs | 14 ++--- uefi-raw/src/protocol/console.rs | 18 +++--- uefi-raw/src/protocol/device_path.rs | 10 +-- uefi-raw/src/protocol/file_system.rs | 4 +- uefi-raw/src/protocol/media.rs | 6 +- uefi-raw/src/protocol/network/dhcp4.rs | 4 +- uefi-raw/src/protocol/network/http.rs | 6 +- uefi-raw/src/table/boot.rs | 6 +- uefi-raw/src/table/runtime.rs | 4 +- uefi-test-runner/src/proto/load.rs | 4 +- uefi/src/boot.rs | 2 +- uefi/src/proto/console/pointer/mod.rs | 2 +- uefi/src/proto/console/text/input.rs | 2 +- uefi/src/proto/console/text/output.rs | 9 +-- uefi/src/proto/device_path/text.rs | 8 +-- uefi/src/proto/media/block.rs | 27 ++++---- uefi/src/proto/media/load_file.rs | 3 +- 20 files changed, 160 insertions(+), 64 deletions(-) diff --git a/uefi-raw/CHANGELOG.md b/uefi-raw/CHANGELOG.md index 9bf48d15b..cc3a79d8c 100644 --- a/uefi-raw/CHANGELOG.md +++ b/uefi-raw/CHANGELOG.md @@ -1,12 +1,14 @@ # uefi-raw - [Unreleased] +## Added +- `Boolean` type # uefi-raw - 0.8.0 (2024-09-09) ## Added - Added `PAGE_SIZE` constant. - +- Added a new `unstable` feature # uefi-raw - 0.7.0 (2024-08-20) diff --git a/uefi-raw/Cargo.toml b/uefi-raw/Cargo.toml index d13013354..f0e652f6e 100644 --- a/uefi-raw/Cargo.toml +++ b/uefi-raw/Cargo.toml @@ -23,5 +23,9 @@ bitflags.workspace = true ptr_meta.workspace = true uguid.workspace = true +[features] +default = [] +unstable = [] + [package.metadata.docs.rs] rustdoc-args = ["--cfg", "docsrs"] diff --git a/uefi-raw/src/lib.rs b/uefi-raw/src/lib.rs index 78d320bfc..8139d353d 100644 --- a/uefi-raw/src/lib.rs +++ b/uefi-raw/src/lib.rs @@ -31,11 +31,14 @@ pub mod time; mod status; -use core::ffi::c_void; -use core::fmt::{self, Debug, Formatter}; pub use status::Status; pub use uguid::{guid, Guid}; +#[cfg(feature = "unstable")] +use core::error::Error; +use core::ffi::c_void; +use core::fmt::{self, Debug, Display, Formatter}; + /// Handle to an event structure. pub type Event = *mut c_void; @@ -65,6 +68,58 @@ pub type PhysicalAddress = u64; /// of target platform. pub type VirtualAddress = u64; +/// The provided [`Boolean`] can't be converted to [`bool`] as it is neither +/// `0` nor `1`. +#[derive(Debug, Copy, Clone, PartialEq, Ord, PartialOrd, Eq)] +pub struct InvalidBooleanError(u8); + +impl Display for InvalidBooleanError { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + Debug::fmt(self, f) + } +} + +#[cfg(feature = "unstable")] +impl Error for InvalidBooleanError {} + +/// ABI-compatible UEFI boolean. +/// +/// Opaque 1-byte value holding either `0` for FALSE or a `1` for TRUE. This +/// type can be converted from and to `bool` via corresponding [`From`] +/// respectively [`TryFrom`] implementations. +#[derive(Copy, Clone, Debug, Default, PartialEq, Ord, PartialOrd, Eq, Hash)] +#[repr(transparent)] +pub struct Boolean(u8); + +impl Boolean { + /// [`Boolean`] representing `true`. + pub const TRUE: Self = Self(1); + + /// [`Boolean`] representing `false`. + pub const FALSE: Self = Self(0); +} + +impl From for Boolean { + fn from(value: bool) -> Self { + match value { + true => Self(1), + false => Self(0), + } + } +} + +impl TryFrom for bool { + type Error = InvalidBooleanError; + + fn try_from(value: Boolean) -> Result { + match value.0 { + 0 => Ok(false), + 1 => Ok(true), + x => Err(InvalidBooleanError(x)), + } + } +} + /// An IPv4 internet protocol address. #[derive(Clone, Copy, Debug, Default, Eq, PartialEq, Ord, PartialOrd, Hash)] #[repr(transparent)] @@ -133,3 +188,31 @@ impl Default for IpAddress { #[derive(Clone, Copy, Debug, Default, Eq, PartialEq, Ord, PartialOrd, Hash)] #[repr(transparent)] pub struct MacAddress(pub [u8; 32]); + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + /// Test the properties promised in [0]. This also applies for the other + /// architectures. + /// + /// [0] https://github.com/tianocore/edk2/blob/b0f43dd3fdec2363e3548ec31eb455dc1c4ac761/MdePkg/Include/X64/ProcessorBind.h#L192 + fn test_boolean_abi() { + assert_eq!(size_of::(), 1); + assert_eq!(Boolean::from(true).0, 1); + assert_eq!(Boolean::from(false).0, 0); + assert_eq!(Boolean::TRUE.0, 1); + assert_eq!(Boolean::FALSE.0, 0); + assert_eq!(bool::try_from(Boolean(0b0)), Ok(false)); + assert_eq!(bool::try_from(Boolean(0b1)), Ok(true)); + assert_eq!( + bool::try_from(Boolean(0b11)), + Err(InvalidBooleanError(0b11)) + ); + assert_eq!( + bool::try_from(Boolean(0b10)), + Err(InvalidBooleanError(0b10)) + ); + } +} diff --git a/uefi-raw/src/protocol/block.rs b/uefi-raw/src/protocol/block.rs index e688aebdf..1efd331f1 100644 --- a/uefi-raw/src/protocol/block.rs +++ b/uefi-raw/src/protocol/block.rs @@ -1,4 +1,4 @@ -use crate::{guid, Guid, Status}; +use crate::{guid, Boolean, Guid, Status}; use core::ffi::c_void; /// Logical block address. @@ -9,11 +9,11 @@ pub type Lba = u64; #[derive(Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)] pub struct BlockIoMedia { pub media_id: u32, - pub removable_media: bool, - pub media_present: bool, - pub logical_partition: bool, - pub read_only: bool, - pub write_caching: bool, + pub removable_media: Boolean, + pub media_present: Boolean, + pub logical_partition: Boolean, + pub read_only: Boolean, + pub write_caching: Boolean, pub block_size: u32, pub io_align: u32, pub last_block: Lba, @@ -31,7 +31,7 @@ pub struct BlockIoMedia { pub struct BlockIoProtocol { pub revision: u64, pub media: *const BlockIoMedia, - pub reset: unsafe extern "efiapi" fn(this: *mut Self, extended_verification: bool) -> Status, + pub reset: unsafe extern "efiapi" fn(this: *mut Self, extended_verification: Boolean) -> Status, pub read_blocks: unsafe extern "efiapi" fn( this: *const Self, media_id: u32, diff --git a/uefi-raw/src/protocol/console.rs b/uefi-raw/src/protocol/console.rs index 441b3c2ca..bd3c0172e 100644 --- a/uefi-raw/src/protocol/console.rs +++ b/uefi-raw/src/protocol/console.rs @@ -1,6 +1,6 @@ pub mod serial; -use crate::{guid, Char16, Event, Guid, PhysicalAddress, Status}; +use crate::{guid, Boolean, Char16, Event, Guid, PhysicalAddress, Status}; use bitflags::bitflags; use core::ptr; @@ -42,7 +42,7 @@ pub struct AbsolutePointerState { #[derive(Debug)] #[repr(C)] pub struct AbsolutePointerProtocol { - pub reset: unsafe extern "efiapi" fn(this: *mut Self, extended_verification: u8) -> Status, + pub reset: unsafe extern "efiapi" fn(this: *mut Self, extended_verification: Boolean) -> Status, pub get_state: unsafe extern "efiapi" fn(this: *const Self, state: *mut AbsolutePointerState) -> Status, pub wait_for_input: Event, @@ -63,7 +63,7 @@ pub struct InputKey { #[derive(Debug)] #[repr(C)] pub struct SimpleTextInputProtocol { - pub reset: unsafe extern "efiapi" fn(this: *mut Self, extended_verification: bool) -> Status, + pub reset: unsafe extern "efiapi" fn(this: *mut Self, extended_verification: Boolean) -> Status, pub read_key_stroke: unsafe extern "efiapi" fn(this: *mut Self, key: *mut InputKey) -> Status, pub wait_for_key: Event, } @@ -80,13 +80,13 @@ pub struct SimpleTextOutputMode { pub attribute: i32, pub cursor_column: i32, pub cursor_row: i32, - pub cursor_visible: bool, + pub cursor_visible: Boolean, } #[derive(Debug)] #[repr(C)] pub struct SimpleTextOutputProtocol { - pub reset: unsafe extern "efiapi" fn(this: *mut Self, extended: bool) -> Status, + pub reset: unsafe extern "efiapi" fn(this: *mut Self, extended: Boolean) -> Status, pub output_string: unsafe extern "efiapi" fn(this: *mut Self, string: *const Char16) -> Status, pub test_string: unsafe extern "efiapi" fn(this: *mut Self, string: *const Char16) -> Status, pub query_mode: unsafe extern "efiapi" fn( @@ -100,7 +100,7 @@ pub struct SimpleTextOutputProtocol { pub clear_screen: unsafe extern "efiapi" fn(this: *mut Self) -> Status, pub set_cursor_position: unsafe extern "efiapi" fn(this: *mut Self, column: usize, row: usize) -> Status, - pub enable_cursor: unsafe extern "efiapi" fn(this: *mut Self, visible: bool) -> Status, + pub enable_cursor: unsafe extern "efiapi" fn(this: *mut Self, visible: Boolean) -> Status, pub mode: *mut SimpleTextOutputMode, } @@ -124,8 +124,8 @@ pub struct SimplePointerState { pub relative_movement_x: i32, pub relative_movement_y: i32, pub relative_movement_z: i32, - pub left_button: u8, - pub right_button: u8, + pub left_button: Boolean, + pub right_button: Boolean, } #[derive(Debug)] @@ -133,7 +133,7 @@ pub struct SimplePointerState { pub struct SimplePointerProtocol { pub reset: unsafe extern "efiapi" fn( this: *mut SimplePointerProtocol, - extended_verification: bool, + extended_verification: Boolean, ) -> Status, pub get_state: unsafe extern "efiapi" fn( this: *mut SimplePointerProtocol, diff --git a/uefi-raw/src/protocol/device_path.rs b/uefi-raw/src/protocol/device_path.rs index 698fc03b4..55cd260f5 100644 --- a/uefi-raw/src/protocol/device_path.rs +++ b/uefi-raw/src/protocol/device_path.rs @@ -1,4 +1,4 @@ -use crate::{guid, Char16, Guid}; +use crate::{guid, Boolean, Char16, Guid}; /// Device path protocol. /// @@ -25,13 +25,13 @@ impl DevicePathProtocol { pub struct DevicePathToTextProtocol { pub convert_device_node_to_text: unsafe extern "efiapi" fn( device_node: *const DevicePathProtocol, - display_only: bool, - allow_shortcuts: bool, + display_only: Boolean, + allow_shortcuts: Boolean, ) -> *const Char16, pub convert_device_path_to_text: unsafe extern "efiapi" fn( device_path: *const DevicePathProtocol, - display_only: bool, - allow_shortcuts: bool, + display_only: Boolean, + allow_shortcuts: Boolean, ) -> *const Char16, } diff --git a/uefi-raw/src/protocol/file_system.rs b/uefi-raw/src/protocol/file_system.rs index a8b1367fb..f7605e6c8 100644 --- a/uefi-raw/src/protocol/file_system.rs +++ b/uefi-raw/src/protocol/file_system.rs @@ -1,5 +1,5 @@ use crate::time::Time; -use crate::{guid, Char16, Event, Guid, Status}; +use crate::{guid, Boolean, Char16, Event, Guid, Status}; use bitflags::bitflags; use core::ffi::c_void; @@ -151,7 +151,7 @@ impl FileInfo { #[repr(C)] pub struct FileSystemInfo { pub size: u64, - pub read_only: u8, + pub read_only: Boolean, pub volume_size: u64, pub free_space: u64, pub block_size: u32, diff --git a/uefi-raw/src/protocol/media.rs b/uefi-raw/src/protocol/media.rs index ceff445c0..1c893794d 100644 --- a/uefi-raw/src/protocol/media.rs +++ b/uefi-raw/src/protocol/media.rs @@ -1,5 +1,5 @@ use crate::protocol::device_path::DevicePathProtocol; -use crate::{guid, Guid, Status}; +use crate::{guid, Boolean, Guid, Status}; use core::ffi::c_void; #[derive(Debug)] @@ -8,7 +8,7 @@ pub struct LoadFileProtocol { pub load_file: unsafe extern "efiapi" fn( this: *mut LoadFileProtocol, file_path: *const DevicePathProtocol, - boot_policy: bool, + boot_policy: Boolean, buffer_size: *mut usize, buffer: *mut c_void, ) -> Status, @@ -24,7 +24,7 @@ pub struct LoadFile2Protocol { pub load_file: unsafe extern "efiapi" fn( this: *mut LoadFile2Protocol, file_path: *const DevicePathProtocol, - boot_policy: bool, + boot_policy: Boolean, buffer_size: *mut usize, buffer: *mut c_void, ) -> Status, diff --git a/uefi-raw/src/protocol/network/dhcp4.rs b/uefi-raw/src/protocol/network/dhcp4.rs index 7ff96833c..0dfccb0fe 100644 --- a/uefi-raw/src/protocol/network/dhcp4.rs +++ b/uefi-raw/src/protocol/network/dhcp4.rs @@ -1,4 +1,4 @@ -use crate::{guid, Char8, Event, Guid, Ipv4Address, MacAddress, Status}; +use crate::{guid, Boolean, Char8, Event, Guid, Ipv4Address, MacAddress, Status}; use core::ffi::c_void; newtype_enum! { @@ -148,7 +148,7 @@ pub struct Dhcp4Protocol { pub start: unsafe extern "efiapi" fn(this: *mut Self, completion_event: Event) -> Status, pub renew_rebind: unsafe extern "efiapi" fn( this: *mut Self, - rebind_request: bool, + rebind_request: Boolean, completion_event: Event, ) -> Status, pub release: unsafe extern "efiapi" fn(this: *mut Self) -> Status, diff --git a/uefi-raw/src/protocol/network/http.rs b/uefi-raw/src/protocol/network/http.rs index e49a9adf2..81086e9ed 100644 --- a/uefi-raw/src/protocol/network/http.rs +++ b/uefi-raw/src/protocol/network/http.rs @@ -1,4 +1,4 @@ -use crate::{guid, Char16, Char8, Event, Guid, Ipv4Address, Ipv6Address, Status}; +use crate::{guid, Boolean, Char16, Char8, Event, Guid, Ipv4Address, Ipv6Address, Status}; use core::ffi::c_void; use core::fmt::{self, Debug, Formatter}; @@ -7,7 +7,7 @@ use core::fmt::{self, Debug, Formatter}; pub struct HttpConfigData { pub http_version: HttpVersion, pub time_out_millisec: u32, - pub local_addr_is_ipv6: bool, + pub local_addr_is_ipv6: Boolean, pub access_point: HttpAccessPoint, } @@ -22,7 +22,7 @@ newtype_enum! { #[derive(Debug)] #[repr(C)] pub struct HttpV4AccessPoint { - pub use_default_addr: bool, + pub use_default_addr: Boolean, pub local_address: Ipv4Address, pub local_subnet: Ipv4Address, pub local_port: u16, diff --git a/uefi-raw/src/table/boot.rs b/uefi-raw/src/table/boot.rs index 8559a4c28..3107762b5 100644 --- a/uefi-raw/src/table/boot.rs +++ b/uefi-raw/src/table/boot.rs @@ -2,7 +2,7 @@ use crate::protocol::device_path::DevicePathProtocol; use crate::table::Header; -use crate::{Char16, Event, Guid, Handle, PhysicalAddress, Status, VirtualAddress}; +use crate::{Boolean, Char16, Event, Guid, Handle, PhysicalAddress, Status, VirtualAddress}; use bitflags::bitflags; use core::ffi::c_void; use core::ops::RangeInclusive; @@ -103,7 +103,7 @@ pub struct BootServices { // Image services pub load_image: unsafe extern "efiapi" fn( - boot_policy: u8, + boot_policy: Boolean, parent_image_handle: Handle, device_path: *const DevicePathProtocol, source_buffer: *const u8, @@ -140,7 +140,7 @@ pub struct BootServices { controller: Handle, driver_image: Handle, remaining_device_path: *const DevicePathProtocol, - recursive: bool, + recursive: Boolean, ) -> Status, pub disconnect_controller: unsafe extern "efiapi" fn( controller: Handle, diff --git a/uefi-raw/src/table/runtime.rs b/uefi-raw/src/table/runtime.rs index 69daa0f23..e817006f3 100644 --- a/uefi-raw/src/table/runtime.rs +++ b/uefi-raw/src/table/runtime.rs @@ -4,7 +4,7 @@ use crate::capsule::CapsuleHeader; use crate::table::boot::MemoryDescriptor; use crate::table::Header; use crate::time::Time; -use crate::{guid, Char16, Guid, PhysicalAddress, Status}; +use crate::{guid, Boolean, Char16, Guid, PhysicalAddress, Status}; use bitflags::bitflags; use core::ffi::c_void; @@ -115,7 +115,7 @@ pub struct TimeCapabilities { /// Whether a time set operation clears the device's time below the /// "resolution" reporting level. False for normal PC-AT CMOS RTC devices. - pub sets_to_zero: bool, + pub sets_to_zero: Boolean, } bitflags! { diff --git a/uefi-test-runner/src/proto/load.rs b/uefi-test-runner/src/proto/load.rs index 928e6abd1..6531d2e25 100644 --- a/uefi-test-runner/src/proto/load.rs +++ b/uefi-test-runner/src/proto/load.rs @@ -11,12 +11,12 @@ use uefi::proto::BootPolicy; use uefi::{boot, Guid, Handle}; use uefi_raw::protocol::device_path::DevicePathProtocol; use uefi_raw::protocol::media::{LoadFile2Protocol, LoadFileProtocol}; -use uefi_raw::Status; +use uefi_raw::{Boolean, Status}; unsafe extern "efiapi" fn raw_load_file( this: *mut LoadFile2Protocol, _file_path: *const DevicePathProtocol, - _boot_policy: bool, + _boot_policy: Boolean, buffer_size: *mut usize, buffer: *mut c_void, ) -> Status { diff --git a/uefi/src/boot.rs b/uefi/src/boot.rs index 3cc3d60f9..98bd33678 100644 --- a/uefi/src/boot.rs +++ b/uefi/src/boot.rs @@ -561,7 +561,7 @@ pub fn connect_controller( .map(|dp| dp.as_ffi_ptr()) .unwrap_or(ptr::null()) .cast(), - recursive, + recursive.into(), ) } .to_result_with_err(|_| ()) diff --git a/uefi/src/proto/console/pointer/mod.rs b/uefi/src/proto/console/pointer/mod.rs index 7d5ca335c..1c659e021 100644 --- a/uefi/src/proto/console/pointer/mod.rs +++ b/uefi/src/proto/console/pointer/mod.rs @@ -20,7 +20,7 @@ impl Pointer { /// /// - `DeviceError` if the device is malfunctioning and cannot be reset. pub fn reset(&mut self, extended_verification: bool) -> Result { - unsafe { (self.0.reset)(&mut self.0, extended_verification) }.to_result() + unsafe { (self.0.reset)(&mut self.0, extended_verification.into()) }.to_result() } /// Retrieves the pointer device's current state, if a state change occurred diff --git a/uefi/src/proto/console/text/input.rs b/uefi/src/proto/console/text/input.rs index 622e44bb3..585c10787 100644 --- a/uefi/src/proto/console/text/input.rs +++ b/uefi/src/proto/console/text/input.rs @@ -19,7 +19,7 @@ impl Input { /// /// - `DeviceError` if the device is malfunctioning and cannot be reset. pub fn reset(&mut self, extended_verification: bool) -> Result { - unsafe { (self.0.reset)(&mut self.0, extended_verification) }.to_result() + unsafe { (self.0.reset)(&mut self.0, extended_verification.into()) }.to_result() } /// Reads the next keystroke from the input device, if any. diff --git a/uefi/src/proto/console/text/output.rs b/uefi/src/proto/console/text/output.rs index 5dd599d27..6e394c0e6 100644 --- a/uefi/src/proto/console/text/output.rs +++ b/uefi/src/proto/console/text/output.rs @@ -28,7 +28,7 @@ pub struct Output(SimpleTextOutputProtocol); impl Output { /// Resets and clears the text output device hardware. pub fn reset(&mut self, extended: bool) -> Result { - unsafe { (self.0.reset)(&mut self.0, extended) }.to_result() + unsafe { (self.0.reset)(&mut self.0, extended.into()) }.to_result() } /// Clears the output screen. @@ -116,8 +116,9 @@ impl Output { /// Returns whether the cursor is currently shown or not. #[must_use] - pub const fn cursor_visible(&self) -> bool { - self.data().cursor_visible + pub fn cursor_visible(&self) -> bool { + // Panic: Misbehaving UEFI impls are so unlikely; just fail + self.data().cursor_visible.try_into().unwrap() } /// Make the cursor visible or invisible. @@ -125,7 +126,7 @@ impl Output { /// The output device may not support this operation, in which case an /// `Unsupported` error will be returned. pub fn enable_cursor(&mut self, visible: bool) -> Result { - unsafe { (self.0.enable_cursor)(&mut self.0, visible) }.to_result() + unsafe { (self.0.enable_cursor)(&mut self.0, visible.into()) }.to_result() } /// Returns the column and row of the cursor. diff --git a/uefi/src/proto/device_path/text.rs b/uefi/src/proto/device_path/text.rs index ebd725b3e..568eddeb6 100644 --- a/uefi/src/proto/device_path/text.rs +++ b/uefi/src/proto/device_path/text.rs @@ -91,8 +91,8 @@ impl DevicePathToText { let text_device_node = unsafe { (self.0.convert_device_node_to_text)( device_node.as_ffi_ptr().cast(), - display_only.0, - allow_shortcuts.0, + display_only.0.into(), + allow_shortcuts.0.into(), ) }; PoolString::new(text_device_node.cast()) @@ -113,8 +113,8 @@ impl DevicePathToText { let text_device_path = unsafe { (self.0.convert_device_path_to_text)( device_path.as_ffi_ptr().cast(), - display_only.0, - allow_shortcuts.0, + display_only.0.into(), + allow_shortcuts.0.into(), ) }; PoolString::new(text_device_path.cast()) diff --git a/uefi/src/proto/media/block.rs b/uefi/src/proto/media/block.rs index db216ae1f..ce80d8e6f 100644 --- a/uefi/src/proto/media/block.rs +++ b/uefi/src/proto/media/block.rs @@ -27,7 +27,7 @@ impl BlockIO { /// # Errors /// * `uefi::Status::DEVICE_ERROR` The block device is not functioning correctly and could not be reset. pub fn reset(&mut self, extended_verification: bool) -> Result { - unsafe { (self.0.reset)(&mut self.0, extended_verification) }.to_result() + unsafe { (self.0.reset)(&mut self.0, extended_verification.into()) }.to_result() } /// Read the requested number of blocks from the device. @@ -115,32 +115,37 @@ impl BlockIOMedia { /// True if the media is removable. #[must_use] - pub const fn is_removable_media(&self) -> bool { - self.0.removable_media + pub fn is_removable_media(&self) -> bool { + // Panic: Misbehaving UEFI impls are so unlikely; just fail + self.0.removable_media.try_into().unwrap() } /// True if there is a media currently present in the device. #[must_use] - pub const fn is_media_present(&self) -> bool { - self.0.media_present + pub fn is_media_present(&self) -> bool { + // Panic: Misbehaving UEFI impls are so unlikely; just fail + self.0.media_present.try_into().unwrap() } /// True if block IO was produced to abstract partition structure. #[must_use] - pub const fn is_logical_partition(&self) -> bool { - self.0.logical_partition + pub fn is_logical_partition(&self) -> bool { + // Panic: Misbehaving UEFI impls are so unlikely; just fail + self.0.logical_partition.try_into().unwrap() } /// True if the media is marked read-only. #[must_use] - pub const fn is_read_only(&self) -> bool { - self.0.read_only + pub fn is_read_only(&self) -> bool { + // Panic: Misbehaving UEFI impls are so unlikely; just fail + self.0.read_only.try_into().unwrap() } /// True if `writeBlocks` function writes data. #[must_use] - pub const fn is_write_caching(&self) -> bool { - self.0.write_caching + pub fn is_write_caching(&self) -> bool { + // Panic: Misbehaving UEFI impls are so unlikely; just fail + self.0.write_caching.try_into().unwrap() } /// The intrinsic block size of the device. diff --git a/uefi/src/proto/media/load_file.rs b/uefi/src/proto/media/load_file.rs index 6f5d74634..826139636 100644 --- a/uefi/src/proto/media/load_file.rs +++ b/uefi/src/proto/media/load_file.rs @@ -4,6 +4,7 @@ use crate::proto::unsafe_protocol; #[cfg(all(feature = "alloc", feature = "unstable"))] use alloc::alloc::Global; use uefi_raw::protocol::media::{LoadFile2Protocol, LoadFileProtocol}; +use uefi_raw::Boolean; #[cfg(feature = "alloc")] use { crate::{mem::make_boxed, proto::device_path::DevicePath, Result, StatusExt}, @@ -141,7 +142,7 @@ impl LoadFile2 { (self.0.load_file)( this, file_path.as_ffi_ptr().cast(), - false, /* always false - see spec */ + Boolean::FALSE, /* always false - see spec */ &mut size, buf.as_mut_ptr().cast(), ) From 6f9d1b02c3a24c1ac6df50047a5c7e9c58a1757d Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Tue, 1 Oct 2024 13:17:38 +0200 Subject: [PATCH 2/4] uefi: streamline BootPolicy and Boolean types --- uefi/CHANGELOG.md | 2 +- uefi/src/proto/boot_policy.rs | 90 ++++++++++------------------------- uefi/src/proto/mod.rs | 2 +- 3 files changed, 26 insertions(+), 68 deletions(-) diff --git a/uefi/CHANGELOG.md b/uefi/CHANGELOG.md index f41900204..be140009a 100644 --- a/uefi/CHANGELOG.md +++ b/uefi/CHANGELOG.md @@ -15,9 +15,9 @@ details of the deprecated items that were removed in this release. - **Breaking:** `FileSystem` no longer has a lifetime parameter, and the deprecated conversion from `uefi::table::boot::ScopedProtocol` has been removed. +- **Breaking:** Removed `BootPolicyError` as `BootPolicy` - Fixed `boot::open_protocol` to properly handle a null interface pointer. - # uefi - 0.32.0 (2024-09-09) See [Deprecating SystemTable/BootServices/RuntimeServices][funcmigrate] for diff --git a/uefi/src/proto/boot_policy.rs b/uefi/src/proto/boot_policy.rs index d46edf2ff..05aaed5d9 100644 --- a/uefi/src/proto/boot_policy.rs +++ b/uefi/src/proto/boot_policy.rs @@ -1,33 +1,12 @@ //! 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 {} +use uefi_raw::{Boolean, InvalidBooleanError}; /// 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. +/// This type is not ABI compatible. On the ABI level, this corresponds to +/// a [`Boolean`]. #[derive(Copy, Clone, Debug, Default, Eq, Ord, PartialEq, PartialOrd)] pub enum BootPolicy { /// Indicates that the request originates from the boot manager, and that @@ -36,50 +15,34 @@ pub enum BootPolicy { /// /// Boot selection refers to what a user has chosen in the (GUI) boot menu. /// - /// This corresponds to the `TRUE` value in the UEFI spec. + /// This corresponds to the underlying [`Boolean`] being `true`. BootSelection, /// The provided `file_path` must match an exact file to be loaded. /// - /// This corresponds to the `FALSE` value in the UEFI spec. + /// This corresponds to the underlying [`Boolean`] being `false`. #[default] ExactMatch, } -impl From for bool { +impl From for Boolean { fn from(value: BootPolicy) -> Self { match value { - BootPolicy::BootSelection => true, - BootPolicy::ExactMatch => false, + BootPolicy::BootSelection => true.into(), + BootPolicy::ExactMatch => false.into(), } } } -impl From for BootPolicy { - fn from(value: bool) -> Self { - match value { +impl TryFrom for BootPolicy { + type Error = InvalidBooleanError; + + fn try_from(value: Boolean) -> Result { + let boolean: bool = value.try_into()?; + let policy = match boolean { 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)), - } + }; + Ok(policy) } } @@ -89,20 +52,15 @@ mod tests { #[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)) + BootPolicy::try_from(Boolean::TRUE).unwrap(), + BootPolicy::BootSelection + ); + assert_eq!( + BootPolicy::try_from(Boolean::FALSE).unwrap(), + BootPolicy::ExactMatch ); + assert_eq!(Boolean::from(BootPolicy::BootSelection), Boolean::TRUE); + assert_eq!(Boolean::from(BootPolicy::ExactMatch), Boolean::FALSE); } } diff --git a/uefi/src/proto/mod.rs b/uefi/src/proto/mod.rs index c317d282c..64062826b 100644 --- a/uefi/src/proto/mod.rs +++ b/uefi/src/proto/mod.rs @@ -26,7 +26,7 @@ pub mod tcg; mod boot_policy; -pub use boot_policy::{BootPolicy, BootPolicyError}; +pub use boot_policy::BootPolicy; pub use uefi_macros::unsafe_protocol; use crate::Identify; From 89e8359584ec835ca8a2e988aec76fba193d90d1 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Wed, 2 Oct 2024 11:10:36 +0200 Subject: [PATCH 3/4] xtask/uefi-raw: fix/improve check-raw --- uefi-raw/src/lib.rs | 5 +++-- xtask/src/check_raw.rs | 27 +++++++++++++++++++++++---- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/uefi-raw/src/lib.rs b/uefi-raw/src/lib.rs index 8139d353d..101efb2b5 100644 --- a/uefi-raw/src/lib.rs +++ b/uefi-raw/src/lib.rs @@ -71,7 +71,8 @@ pub type VirtualAddress = u64; /// The provided [`Boolean`] can't be converted to [`bool`] as it is neither /// `0` nor `1`. #[derive(Debug, Copy, Clone, PartialEq, Ord, PartialOrd, Eq)] -pub struct InvalidBooleanError(u8); +#[repr(transparent)] +pub struct InvalidBooleanError(pub u8); impl Display for InvalidBooleanError { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { @@ -89,7 +90,7 @@ impl Error for InvalidBooleanError {} /// respectively [`TryFrom`] implementations. #[derive(Copy, Clone, Debug, Default, PartialEq, Ord, PartialOrd, Eq, Hash)] #[repr(transparent)] -pub struct Boolean(u8); +pub struct Boolean(pub u8); impl Boolean { /// [`Boolean`] representing `true`. diff --git a/xtask/src/check_raw.rs b/xtask/src/check_raw.rs index b6887afc6..5a9425f8d 100644 --- a/xtask/src/check_raw.rs +++ b/xtask/src/check_raw.rs @@ -31,6 +31,7 @@ enum ErrorKind { ForbiddenType, MalformedAttrs, MissingPub, + MissingRepr, MissingUnsafe, UnderscoreField, UnknownRepr, @@ -49,6 +50,7 @@ impl Display for ErrorKind { Self::ForbiddenType => "forbidden type", Self::MalformedAttrs => "malformed attribute contents", Self::MissingPub => "missing pub", + Self::MissingRepr => "missing repr", Self::MissingUnsafe => "missing unsafe", Self::UnderscoreField => "field name starts with `_`", Self::UnknownRepr => "unknown repr", @@ -105,17 +107,18 @@ fn is_pub(vis: &Visibility) -> bool { } /// Type repr. A type may have more than one of these (e.g. both `C` and `Packed`). -#[derive(Clone, Copy, Eq, PartialEq, Ord, PartialOrd)] +#[derive(Debug, Clone, Copy, Eq, PartialEq, Ord, PartialOrd)] enum Repr { Align(usize), C, + Rust, Packed, Transparent, } /// A restricted view of `Attribute`, limited to just the attributes that are /// expected in `uefi-raw`. -#[derive(Clone, Copy)] +#[derive(Debug, Clone, Copy)] enum ParsedAttr { Derive, Doc, @@ -137,6 +140,8 @@ fn parse_attrs(attrs: &[Attribute], src: &Path) -> Result, Error attr.parse_nested_meta(|meta| { if meta.path.is_ident("C") { va.push(ParsedAttr::Repr(Repr::C)); + } else if meta.path.is_ident("Rust") { + va.push(ParsedAttr::Repr(Repr::Rust)); } else if meta.path.is_ident("packed") { va.push(ParsedAttr::Repr(Repr::Packed)); } else if meta.path.is_ident("transparent") { @@ -259,7 +264,9 @@ fn check_type_attrs(attrs: &[Attribute], spanned: &dyn Spanned, src: &Path) -> R let allowed_reprs: &[&[Repr]] = &[&[Repr::C], &[Repr::C, Repr::Packed], &[Repr::Transparent]]; - if allowed_reprs.contains(&reprs.as_slice()) { + if reprs.is_empty() { + Err(Error::new(ErrorKind::MissingRepr, src, spanned)) + } else if allowed_reprs.contains(&reprs.as_slice()) { Ok(()) } else { Err(Error::new(ErrorKind::ForbiddenRepr, src, spanned)) @@ -408,6 +415,7 @@ mod tests { Path::new("test") } + #[track_caller] fn check_item_err(item: Item, expected_error: ErrorKind) { assert_eq!(check_item(&item, src()).unwrap_err().kind, expected_error); } @@ -545,9 +553,20 @@ mod tests { ErrorKind::UnderscoreField, ); + // Missing `repr`. + check_item_err( + parse_quote! { + pub struct S { + pub f: u32, + } + }, + ErrorKind::MissingRepr, + ); + // Forbidden `repr`. check_item_err( parse_quote! { + #[repr(Rust)] pub struct S { pub f: u32, } @@ -623,7 +642,7 @@ mod tests { pub f: u32, } }, - ErrorKind::ForbiddenRepr, + ErrorKind::MissingRepr, ); } } From 492f00258a9ec7e79317d9515be4c8ad7139da15 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Wed, 2 Oct 2024 11:24:26 +0200 Subject: [PATCH 4/4] uefi-raw: make Boolean less restrictive, allow any bit pattern This aligns the behaviour with r_efi [0]. [0] https://docs.rs/r-efi/5.1.0/src/r_efi/base.rs.html#488 --- uefi-raw/src/lib.rs | 49 +++++++-------------------- uefi/src/proto/boot_policy.rs | 15 ++++---- uefi/src/proto/console/text/output.rs | 2 +- uefi/src/proto/media/block.rs | 10 +++--- uefi/src/proto/media/load_file.rs | 2 +- 5 files changed, 26 insertions(+), 52 deletions(-) diff --git a/uefi-raw/src/lib.rs b/uefi-raw/src/lib.rs index 101efb2b5..8bdbf7139 100644 --- a/uefi-raw/src/lib.rs +++ b/uefi-raw/src/lib.rs @@ -34,10 +34,8 @@ mod status; pub use status::Status; pub use uguid::{guid, Guid}; -#[cfg(feature = "unstable")] -use core::error::Error; use core::ffi::c_void; -use core::fmt::{self, Debug, Display, Formatter}; +use core::fmt::{self, Debug, Formatter}; /// Handle to an event structure. pub type Event = *mut c_void; @@ -68,26 +66,11 @@ pub type PhysicalAddress = u64; /// of target platform. pub type VirtualAddress = u64; -/// The provided [`Boolean`] can't be converted to [`bool`] as it is neither -/// `0` nor `1`. -#[derive(Debug, Copy, Clone, PartialEq, Ord, PartialOrd, Eq)] -#[repr(transparent)] -pub struct InvalidBooleanError(pub u8); - -impl Display for InvalidBooleanError { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - Debug::fmt(self, f) - } -} - -#[cfg(feature = "unstable")] -impl Error for InvalidBooleanError {} - /// ABI-compatible UEFI boolean. /// /// Opaque 1-byte value holding either `0` for FALSE or a `1` for TRUE. This /// type can be converted from and to `bool` via corresponding [`From`] -/// respectively [`TryFrom`] implementations. +/// implementations. #[derive(Copy, Clone, Debug, Default, PartialEq, Ord, PartialOrd, Eq, Hash)] #[repr(transparent)] pub struct Boolean(pub u8); @@ -109,14 +92,13 @@ impl From for Boolean { } } -impl TryFrom for bool { - type Error = InvalidBooleanError; - - fn try_from(value: Boolean) -> Result { +impl From for bool { + #[allow(clippy::match_like_matches_macro)] + fn from(value: Boolean) -> Self { + // We handle it as in C: Any bit-pattern != 0 equals true match value.0 { - 0 => Ok(false), - 1 => Ok(true), - x => Err(InvalidBooleanError(x)), + 0 => false, + _ => true, } } } @@ -205,15 +187,10 @@ mod tests { assert_eq!(Boolean::from(false).0, 0); assert_eq!(Boolean::TRUE.0, 1); assert_eq!(Boolean::FALSE.0, 0); - assert_eq!(bool::try_from(Boolean(0b0)), Ok(false)); - assert_eq!(bool::try_from(Boolean(0b1)), Ok(true)); - assert_eq!( - bool::try_from(Boolean(0b11)), - Err(InvalidBooleanError(0b11)) - ); - assert_eq!( - bool::try_from(Boolean(0b10)), - Err(InvalidBooleanError(0b10)) - ); + assert_eq!(bool::from(Boolean(0b0)), false); + assert_eq!(bool::from(Boolean(0b1)), true); + // We do it a C: Every bit pattern not 0 is equal to true + assert_eq!(bool::from(Boolean(0b11111110)), true); + assert_eq!(bool::from(Boolean(0b11111111)), true); } } diff --git a/uefi/src/proto/boot_policy.rs b/uefi/src/proto/boot_policy.rs index 05aaed5d9..316f3716c 100644 --- a/uefi/src/proto/boot_policy.rs +++ b/uefi/src/proto/boot_policy.rs @@ -1,6 +1,6 @@ //! Module for the [`BootPolicy`] helper type. -use uefi_raw::{Boolean, InvalidBooleanError}; +use uefi_raw::Boolean; /// The UEFI boot policy is a property that influences the behaviour of /// various UEFI functions that load files (typically UEFI images). @@ -33,16 +33,13 @@ impl From for Boolean { } } -impl TryFrom for BootPolicy { - type Error = InvalidBooleanError; - - fn try_from(value: Boolean) -> Result { - let boolean: bool = value.try_into()?; - let policy = match boolean { +impl From for BootPolicy { + fn from(value: Boolean) -> Self { + let boolean: bool = value.into(); + match boolean { true => Self::BootSelection, false => Self::ExactMatch, - }; - Ok(policy) + } } } diff --git a/uefi/src/proto/console/text/output.rs b/uefi/src/proto/console/text/output.rs index 6e394c0e6..1ee956e6a 100644 --- a/uefi/src/proto/console/text/output.rs +++ b/uefi/src/proto/console/text/output.rs @@ -118,7 +118,7 @@ impl Output { #[must_use] pub fn cursor_visible(&self) -> bool { // Panic: Misbehaving UEFI impls are so unlikely; just fail - self.data().cursor_visible.try_into().unwrap() + self.data().cursor_visible.into() } /// Make the cursor visible or invisible. diff --git a/uefi/src/proto/media/block.rs b/uefi/src/proto/media/block.rs index ce80d8e6f..aee87dffc 100644 --- a/uefi/src/proto/media/block.rs +++ b/uefi/src/proto/media/block.rs @@ -117,35 +117,35 @@ impl BlockIOMedia { #[must_use] pub fn is_removable_media(&self) -> bool { // Panic: Misbehaving UEFI impls are so unlikely; just fail - self.0.removable_media.try_into().unwrap() + self.0.removable_media.into() } /// True if there is a media currently present in the device. #[must_use] pub fn is_media_present(&self) -> bool { // Panic: Misbehaving UEFI impls are so unlikely; just fail - self.0.media_present.try_into().unwrap() + self.0.media_present.into() } /// True if block IO was produced to abstract partition structure. #[must_use] pub fn is_logical_partition(&self) -> bool { // Panic: Misbehaving UEFI impls are so unlikely; just fail - self.0.logical_partition.try_into().unwrap() + self.0.logical_partition.into() } /// True if the media is marked read-only. #[must_use] pub fn is_read_only(&self) -> bool { // Panic: Misbehaving UEFI impls are so unlikely; just fail - self.0.read_only.try_into().unwrap() + self.0.read_only.into() } /// True if `writeBlocks` function writes data. #[must_use] pub fn is_write_caching(&self) -> bool { // Panic: Misbehaving UEFI impls are so unlikely; just fail - self.0.write_caching.try_into().unwrap() + self.0.write_caching.into() } /// The intrinsic block size of the device. diff --git a/uefi/src/proto/media/load_file.rs b/uefi/src/proto/media/load_file.rs index 826139636..b57913c1c 100644 --- a/uefi/src/proto/media/load_file.rs +++ b/uefi/src/proto/media/load_file.rs @@ -4,12 +4,12 @@ use crate::proto::unsafe_protocol; #[cfg(all(feature = "alloc", feature = "unstable"))] use alloc::alloc::Global; use uefi_raw::protocol::media::{LoadFile2Protocol, LoadFileProtocol}; -use uefi_raw::Boolean; #[cfg(feature = "alloc")] use { crate::{mem::make_boxed, proto::device_path::DevicePath, Result, StatusExt}, alloc::boxed::Box, uefi::proto::BootPolicy, + uefi_raw::Boolean, }; /// Load File Protocol.