Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

uefi: add BootPolicy type #1326

Merged
merged 6 commits into from
Aug 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion uefi-test-runner/src/bin/shell_launcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use uefi::prelude::*;
use uefi::proto::device_path::build::{self, DevicePathBuilder};
use uefi::proto::device_path::{DevicePath, DeviceSubType, DeviceType, LoadedImageDevicePath};
use uefi::proto::loaded_image::LoadedImage;
use uefi::proto::BootPolicy;

/// Get the device path of the shell app. This is the same as the
/// currently-loaded image's device path, but with the file path part changed.
Expand Down Expand Up @@ -53,7 +54,7 @@ fn efi_main() -> Status {
boot::image_handle(),
LoadImageSource::FromDevicePath {
device_path: shell_image_path,
from_boot_manager: false,
boot_policy: BootPolicy::ExactMatch,
},
)
.expect("failed to load shell app");
Expand Down
3 changes: 2 additions & 1 deletion uefi-test-runner/src/boot/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use uefi::fs::FileSystem;
use uefi::proto::console::text::Output;
use uefi::proto::device_path::media::FilePath;
use uefi::proto::device_path::{DevicePath, LoadedImageDevicePath};
use uefi::proto::BootPolicy;
use uefi::table::boot::{BootServices, LoadImageSource, SearchType};
use uefi::table::{Boot, SystemTable};
use uefi::{boot, CString16, Identify};
Expand Down Expand Up @@ -122,7 +123,7 @@ fn test_load_image(bt: &BootServices) {
{
let load_source = LoadImageSource::FromDevicePath {
device_path: image_device_path,
from_boot_manager: false,
boot_policy: BootPolicy::ExactMatch,
};
let _ = bt
.load_image(bt.image_handle(), load_source)
Expand Down
2 changes: 2 additions & 0 deletions uefi/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ details of a significant change to the API in this release.
> use uefi::mem::memory_map::{MemoryMap, MemoryMapMut, MemoryType};
> use uefi::table::boot::BootServices;
```
- **Breaking:** Added a new `BootPolicy` type which breaks existing usages
of `LoadImageSource`.

[funcmigrate]: ../docs/funcs_migration.md

Expand Down
46 changes: 11 additions & 35 deletions uefi/src/boot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,37 +2,35 @@
//!
//! These functions will panic if called after exiting boot services.

pub use crate::table::boot::{
AllocateType, EventNotifyFn, LoadImageSource, OpenProtocolAttributes, OpenProtocolParams,
ProtocolSearchKey, SearchType, TimerTrigger,
};
pub use uefi_raw::table::boot::{EventType, MemoryAttribute, MemoryDescriptor, MemoryType, Tpl};

use crate::data_types::PhysicalAddress;
use crate::mem::memory_map::{MemoryMapBackingMemory, MemoryMapKey, MemoryMapMeta, MemoryMapOwned};
use crate::polyfill::maybe_uninit_slice_assume_init_ref;
use crate::proto::device_path::DevicePath;
#[cfg(doc)]
use crate::proto::device_path::LoadedImageDevicePath;
use crate::proto::loaded_image::LoadedImage;
use crate::proto::media::fs::SimpleFileSystem;
use crate::proto::{Protocol, ProtocolPointer};
use crate::runtime::{self, ResetType};
use crate::table::Revision;
use crate::util::opt_nonnull_to_ptr;
use crate::{table, Char16, Error, Event, Guid, Handle, Result, Status, StatusExt};
use core::ffi::c_void;
use core::mem::MaybeUninit;
use core::ops::{Deref, DerefMut};
use core::ptr::{self, NonNull};
use core::sync::atomic::{AtomicPtr, Ordering};
use core::{mem, slice};
use uefi::{table, Char16, Error, Event, Guid, Handle, Result, Status, StatusExt};
use uefi_raw::table::boot::InterfaceType;

#[cfg(feature = "alloc")]
use {alloc::vec::Vec, uefi::ResultExt};

#[cfg(doc)]
use crate::proto::device_path::LoadedImageDevicePath;

pub use uefi::table::boot::{
AllocateType, EventNotifyFn, LoadImageSource, OpenProtocolAttributes, OpenProtocolParams,
ProtocolSearchKey, SearchType, TimerTrigger,
};
pub use uefi_raw::table::boot::{EventType, MemoryAttribute, MemoryDescriptor, MemoryType, Tpl};

/// Global image handle. This is only set by [`set_image_handle`], and it is
/// only read by [`image_handle`].
static IMAGE_HANDLE: AtomicPtr<c_void> = AtomicPtr::new(ptr::null_mut());
Expand Down Expand Up @@ -995,34 +993,12 @@ pub fn load_image(parent_image_handle: Handle, source: LoadImageSource) -> Resul
let bt = boot_services_raw_panicking();
let bt = unsafe { bt.as_ref() };

let boot_policy;
let device_path;
let source_buffer;
let source_size;
match source {
LoadImageSource::FromBuffer { buffer, file_path } => {
// Boot policy is ignored when loading from source buffer.
boot_policy = 0;

device_path = file_path.map(|p| p.as_ffi_ptr()).unwrap_or(ptr::null());
source_buffer = buffer.as_ptr();
source_size = buffer.len();
}
LoadImageSource::FromDevicePath {
device_path: file_path,
from_boot_manager,
} => {
boot_policy = u8::from(from_boot_manager);
device_path = file_path.as_ffi_ptr();
source_buffer = ptr::null();
source_size = 0;
}
};
let (boot_policy, device_path, source_buffer, source_size) = source.to_ffi_params();

let mut image_handle = ptr::null_mut();
unsafe {
(bt.load_image)(
boot_policy,
boot_policy.into(),
parent_image_handle.as_ptr(),
device_path.cast(),
source_buffer,
Expand Down
108 changes: 108 additions & 0 deletions uefi/src/proto/boot_policy.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
//! Module for the [`BootPolicy`] helper type.

use core::fmt::{Display, Formatter};

/// Errors that can happen when working with [`BootPolicy`].
#[derive(Debug, Copy, Clone, PartialOrd, PartialEq, Eq, Ord)]
pub enum BootPolicyError {
/// Only `0` and `1` are valid integers, all other values are undefined.
InvalidInteger(u8),
}

impl Display for BootPolicyError {
fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result {
let s = match self {
Self::InvalidInteger(_) => {
"Only `0` and `1` are valid integers, all other values are undefined."
}
};
f.write_str(s)
}
}

#[cfg(feature = "unstable")]
impl core::error::Error for BootPolicyError {}

/// The UEFI boot policy is a property that influences the behaviour of
/// various UEFI functions that load files (typically UEFI images).
///
/// This type is not ABI compatible. On the ABI level, this is an UEFI
/// boolean.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like it could be made compatible: add repr(u8) and BootSelection = 1, ExactMatch = 0.

(As long as this type is only passed into UEFI APIs, and not read from them; in that case newtype_enum might be better.)

Copy link
Contributor Author

@phip1611 phip1611 Aug 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to postpone that decision/discussion to #1307?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as the type is only being passed into the API, UB shouldn't be a concern -- we'll always initialize BootPolicy correctly.

Copy link
Contributor Author

@phip1611 phip1611 Aug 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were you just adding thoughts to the discussion or do you want this doccomment to be removed before we merge it? That's not clear to me right now :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking we should make changes before merging, but actually since there is no declared repr right now, it wouldn't be a breaking change to add repr(u8) in the future I think. So let's go ahead and merge.

#[derive(Copy, Clone, Debug, Default, Eq, Ord, PartialEq, PartialOrd)]
pub enum BootPolicy {
/// Indicates that the request originates from the boot manager, and that
/// the boot manager is attempting to load the provided `file_path` as a
/// boot selection.
///
/// Boot selection refers to what a user has chosen in the (GUI) boot menu.
///
/// This corresponds to the `TRUE` value in the UEFI spec.
BootSelection,
/// The provided `file_path` must match an exact file to be loaded.
///
/// This corresponds to the `FALSE` value in the UEFI spec.
#[default]
ExactMatch,
}

impl From<BootPolicy> for bool {
fn from(value: BootPolicy) -> Self {
match value {
BootPolicy::BootSelection => true,
BootPolicy::ExactMatch => false,
}
}
}

impl From<bool> for BootPolicy {
fn from(value: bool) -> Self {
match value {
true => Self::BootSelection,
false => Self::ExactMatch,
}
}
}

impl From<BootPolicy> for u8 {
fn from(value: BootPolicy) -> Self {
match value {
BootPolicy::BootSelection => 1,
BootPolicy::ExactMatch => 0,
}
}
}

impl TryFrom<u8> for BootPolicy {
type Error = BootPolicyError;
fn try_from(value: u8) -> Result<Self, Self::Error> {
match value {
0 => Ok(Self::ExactMatch),
1 => Ok(Self::BootSelection),
err => Err(Self::Error::InvalidInteger(err)),
}
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn boot_policy() {
assert_eq!(bool::from(BootPolicy::ExactMatch), false);
assert_eq!(bool::from(BootPolicy::BootSelection), true);

assert_eq!(BootPolicy::from(false), BootPolicy::ExactMatch);
assert_eq!(BootPolicy::from(true), BootPolicy::BootSelection);

assert_eq!(u8::from(BootPolicy::ExactMatch), 0);
assert_eq!(u8::from(BootPolicy::BootSelection), 1);

assert_eq!(BootPolicy::try_from(0), Ok(BootPolicy::ExactMatch));
assert_eq!(BootPolicy::try_from(1), Ok(BootPolicy::BootSelection));
assert_eq!(
BootPolicy::try_from(2),
Err(BootPolicyError::InvalidInteger(2))
);
}
}
39 changes: 21 additions & 18 deletions uefi/src/proto/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,27 @@
//!
//! [`BootServices`]: crate::table::boot::BootServices#accessing-protocols

pub mod console;
pub mod debug;
pub mod device_path;
pub mod driver;
pub mod loaded_image;
pub mod media;
pub mod misc;
pub mod network;
pub mod pi;
pub mod rng;
pub mod security;
pub mod shell_params;
pub mod shim;
pub mod string;
pub mod tcg;

mod boot_policy;

pub use boot_policy::{BootPolicy, BootPolicyError};
pub use uefi_macros::unsafe_protocol;

use crate::Identify;
use core::ffi::c_void;

Expand Down Expand Up @@ -63,21 +84,3 @@ where
ptr.cast::<Self>()
}
}

pub use uefi_macros::unsafe_protocol;

pub mod console;
pub mod debug;
pub mod device_path;
pub mod driver;
pub mod loaded_image;
pub mod media;
pub mod misc;
pub mod network;
pub mod pi;
pub mod rng;
pub mod security;
pub mod shell_params;
pub mod shim;
pub mod string;
pub mod tcg;
Loading