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: Drop BootServices arg from device path <-> text conversions #1327

Merged
merged 1 commit into from
Aug 13, 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
1 change: 0 additions & 1 deletion uefi-test-runner/examples/loaded_image.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ fn print_image_path(boot_services: &BootServices) -> Result {
loaded_image.file_path().expect("File path is not set");
let image_device_path_text = device_path_to_text
.convert_device_path_to_text(
boot_services,
image_device_path,
DisplayOnly(true),
AllowShortcuts(false),
Expand Down
6 changes: 3 additions & 3 deletions uefi-test-runner/src/proto/device_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ pub fn test(bt: &BootServices) {
);

let text = device_path_to_text
.convert_device_node_to_text(bt, path, DisplayOnly(true), AllowShortcuts(false))
.convert_device_node_to_text(path, DisplayOnly(true), AllowShortcuts(false))
.expect("Failed to convert device path to text");
let text = &*text;
info!("path name: {text}");
Expand Down Expand Up @@ -81,7 +81,7 @@ pub fn test(bt: &BootServices) {

let path_components = device_path
.node_iter()
.map(|node| node.to_string(bt, DisplayOnly(false), AllowShortcuts(false)))
.map(|node| node.to_string(DisplayOnly(false), AllowShortcuts(false)))
.map(|str| str.unwrap().to_string())
.collect::<Vec<_>>();

Expand All @@ -107,7 +107,7 @@ pub fn test(bt: &BootServices) {

// Test that to_string works for device_paths
let path = device_path
.to_string(bt, DisplayOnly(false), AllowShortcuts(false))
.to_string(DisplayOnly(false), AllowShortcuts(false))
.unwrap()
.to_string();

Expand Down
2 changes: 2 additions & 0 deletions uefi/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ details of a significant change to the API in this release.

## Changed
- **Breaking:** `uefi::helpers::init` no longer takes an argument.
- **Breaking:** The conversion functions between device paths and text no longer
take a `BootServices` argument. The global system table is used instead.
- The lifetime of the `SearchType` returned from
`BootServices::register_protocol_notify` is now tied to the protocol GUID.
The old `MemoryMap` was renamed to `MemoryMapOwned`.
Expand Down
25 changes: 9 additions & 16 deletions uefi/src/proto/device_path/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,8 @@ use ptr_meta::Pointee;

#[cfg(feature = "alloc")]
use {
crate::boot::{self, OpenProtocolAttributes, OpenProtocolParams, ScopedProtocol, SearchType},
crate::proto::device_path::text::{AllowShortcuts, DevicePathToText, DisplayOnly},
crate::table::boot::{
BootServices, OpenProtocolAttributes, OpenProtocolParams, ScopedProtocol, SearchType,
},
crate::{CString16, Identify},
alloc::borrow::ToOwned,
alloc::boxed::Box,
Expand Down Expand Up @@ -232,14 +230,13 @@ impl DevicePathNode {
#[cfg(feature = "alloc")]
pub fn to_string(
&self,
bs: &BootServices,
display_only: DisplayOnly,
allow_shortcuts: AllowShortcuts,
) -> Result<CString16, DevicePathToTextError> {
let to_text_protocol = open_text_protocol(bs)?;
let to_text_protocol = open_text_protocol()?;

to_text_protocol
.convert_device_node_to_text(bs, self, display_only, allow_shortcuts)
.convert_device_node_to_text(self, display_only, allow_shortcuts)
.map(|pool_string| {
let cstr16 = &*pool_string;
// Another allocation; pool string is dropped. This overhead
Expand Down Expand Up @@ -484,14 +481,13 @@ impl DevicePath {
#[cfg(feature = "alloc")]
pub fn to_string(
&self,
bs: &BootServices,
display_only: DisplayOnly,
allow_shortcuts: AllowShortcuts,
) -> Result<CString16, DevicePathToTextError> {
let to_text_protocol = open_text_protocol(bs)?;
let to_text_protocol = open_text_protocol()?;

to_text_protocol
.convert_device_path_to_text(bs, self, display_only, allow_shortcuts)
.convert_device_path_to_text(self, display_only, allow_shortcuts)
.map(|pool_string| {
let cstr16 = &*pool_string;
// Another allocation; pool string is dropped. This overhead
Expand Down Expand Up @@ -878,20 +874,17 @@ impl core::error::Error for DevicePathToTextError {
/// Helper function to open the [`DevicePathToText`] protocol using the boot
/// services.
#[cfg(feature = "alloc")]
fn open_text_protocol(
bs: &BootServices,
) -> Result<ScopedProtocol<DevicePathToText>, DevicePathToTextError> {
let &handle = bs
.locate_handle_buffer(SearchType::ByProtocol(&DevicePathToText::GUID))
fn open_text_protocol() -> Result<ScopedProtocol<DevicePathToText>, DevicePathToTextError> {
let &handle = boot::locate_handle_buffer(SearchType::ByProtocol(&DevicePathToText::GUID))
.map_err(DevicePathToTextError::CantLocateHandleBuffer)?
.first()
.ok_or(DevicePathToTextError::NoHandle)?;

unsafe {
bs.open_protocol::<DevicePathToText>(
boot::open_protocol::<DevicePathToText>(
OpenProtocolParams {
handle,
agent: bs.image_handle(),
agent: boot::image_handle(),
controller: None,
},
OpenProtocolAttributes::GetProtocol,
Expand Down
47 changes: 18 additions & 29 deletions uefi/src/proto/device_path/text.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@

use crate::proto::device_path::{DevicePath, DevicePathNode};
use crate::proto::unsafe_protocol;
use crate::table::boot::BootServices;
use crate::{CStr16, Char16, Result, Status};
use crate::{boot, CStr16, Char16, Result, Status};
use core::ops::Deref;
use core::ptr::NonNull;
use uefi_raw::protocol::device_path::{DevicePathFromTextProtocol, DevicePathToTextProtocol};

/// This struct is a wrapper of `display_only` parameter
Expand Down Expand Up @@ -42,36 +42,27 @@ pub struct AllowShortcuts(pub bool);
/// Wrapper for a string internally allocated from
/// UEFI boot services memory.
#[derive(Debug)]
pub struct PoolString<'a> {
boot_services: &'a BootServices,
text: *const Char16,
}
pub struct PoolString(NonNull<Char16>);

impl<'a> PoolString<'a> {
fn new(boot_services: &'a BootServices, text: *const Char16) -> Result<Self> {
if text.is_null() {
Err(Status::OUT_OF_RESOURCES.into())
} else {
Ok(Self {
boot_services,
text,
})
}
impl PoolString {
fn new(text: *const Char16) -> Result<Self> {
NonNull::new(text.cast_mut())
.map(Self)
.ok_or(Status::OUT_OF_RESOURCES.into())
}
}

impl<'a> Deref for PoolString<'a> {
impl Deref for PoolString {
type Target = CStr16;

fn deref(&self) -> &Self::Target {
unsafe { CStr16::from_ptr(self.text) }
unsafe { CStr16::from_ptr(self.0.as_ptr()) }
}
}

impl Drop for PoolString<'_> {
impl Drop for PoolString {
fn drop(&mut self) {
let addr = self.text as *mut u8;
unsafe { self.boot_services.free_pool(addr) }.expect("Failed to free pool [{addr:#?}]");
unsafe { boot::free_pool(self.0.cast()) }.expect("Failed to free pool [{addr:#?}]");
}
}

Expand All @@ -91,21 +82,20 @@ impl DevicePathToText {
/// memory for the conversion.
///
/// [`OUT_OF_RESOURCES`]: Status::OUT_OF_RESOURCES
pub fn convert_device_node_to_text<'boot>(
pub fn convert_device_node_to_text(
&self,
boot_services: &'boot BootServices,
device_node: &DevicePathNode,
display_only: DisplayOnly,
allow_shortcuts: AllowShortcuts,
) -> Result<PoolString<'boot>> {
) -> Result<PoolString> {
let text_device_node = unsafe {
(self.0.convert_device_node_to_text)(
device_node.as_ffi_ptr().cast(),
display_only.0,
allow_shortcuts.0,
)
};
PoolString::new(boot_services, text_device_node.cast())
PoolString::new(text_device_node.cast())
}

/// Convert a device path to its text representation.
Expand All @@ -114,21 +104,20 @@ impl DevicePathToText {
/// memory for the conversion.
///
/// [`OUT_OF_RESOURCES`]: Status::OUT_OF_RESOURCES
pub fn convert_device_path_to_text<'boot>(
pub fn convert_device_path_to_text(
&self,
boot_services: &'boot BootServices,
device_path: &DevicePath,
display_only: DisplayOnly,
allow_shortcuts: AllowShortcuts,
) -> Result<PoolString<'boot>> {
) -> Result<PoolString> {
let text_device_path = unsafe {
(self.0.convert_device_path_to_text)(
device_path.as_ffi_ptr().cast(),
display_only.0,
allow_shortcuts.0,
)
};
PoolString::new(boot_services, text_device_path.cast())
PoolString::new(text_device_path.cast())
}
}

Expand Down