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

Implement additional BootServices functions #550

Merged
merged 4 commits into from
Nov 12, 2022
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
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@
- Added structs to represent each type of device path node. All node
types specified in the UEFI 2.10 Specification are now supported.
- Added `DevicePathBuilder` for building new device paths.
- Added `BootServices::install_protocol_interface`,
`BootServices::uninstall_protocol_interface`, and
`BootServices::reinstall_protocol_interface`.
- Added `BootServices::register_protocol_notify`.
- Added `SearchType::ByRegisterNotify`and `ProtocolSearchKey`.

### Changed

Expand Down
155 changes: 142 additions & 13 deletions src/table/boot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,17 +128,35 @@ pub struct BootServices {
check_event: unsafe extern "efiapi" fn(event: Event) -> Status,

// Protocol handlers
install_protocol_interface: usize,
reinstall_protocol_interface: usize,
uninstall_protocol_interface: usize,
install_protocol_interface: unsafe extern "efiapi" fn(
handle: &mut Option<Handle>,
guid: &Guid,
interface_type: InterfaceType,
interface: *mut c_void,
) -> Status,
reinstall_protocol_interface: unsafe extern "efiapi" fn(
handle: Handle,
protocol: &Guid,
old_interface: *mut c_void,
new_interface: *mut c_void,
) -> Status,
uninstall_protocol_interface: unsafe extern "efiapi" fn(
handle: Handle,
protocol: &Guid,
interface: *mut c_void,
) -> Status,
handle_protocol:
extern "efiapi" fn(handle: Handle, proto: &Guid, out_proto: &mut *mut c_void) -> Status,
_reserved: usize,
register_protocol_notify: usize,
register_protocol_notify: extern "efiapi" fn(
protocol: &Guid,
event: Event,
registration: *mut ProtocolSearchKey,
) -> Status,
locate_handle: unsafe extern "efiapi" fn(
search_ty: i32,
proto: *const Guid,
key: *mut c_void,
proto: Option<&Guid>,
key: Option<ProtocolSearchKey>,
buf_sz: &mut usize,
buf: *mut MaybeUninit<Handle>,
) -> Status,
Expand Down Expand Up @@ -221,8 +239,8 @@ pub struct BootServices {
) -> Status,
locate_handle_buffer: unsafe extern "efiapi" fn(
search_ty: i32,
proto: *const Guid,
key: *const c_void,
proto: Option<&Guid>,
key: Option<ProtocolSearchKey>,
no_handles: &mut usize,
buf: &mut *mut Handle,
) -> Status,
Expand Down Expand Up @@ -618,6 +636,72 @@ impl BootServices {
}
}

/// Installs a protocol interface on a device handle. If the inner `Option` in `handle` is `None`,
/// one will be created and added to the list of handles in the system and then returned.
///
/// When a protocol interface is installed, firmware will call all functions that have registered
/// to wait for that interface to be installed.
///
/// # Safety
///
/// The caller is responsible for ensuring that they pass a valid `Guid` for `protocol`.
pub unsafe fn install_protocol_interface(
&self,
mut handle: Option<Handle>,
protocol: &Guid,
interface: *mut c_void,
) -> Result<Handle> {
((self.install_protocol_interface)(
&mut handle,
protocol,
InterfaceType::NATIVE_INTERFACE,
interface,
))
// this `unwrapped_unchecked` is safe, `handle` is guaranteed to be Some() if this call is
// successful
.into_with_val(|| handle.unwrap_unchecked())
}

/// Reinstalls a protocol interface on a device handle. `old_interface` is replaced with `new_interface`.
/// These interfaces may be the same, in which case the registered protocol notifies occur for the handle
/// without replacing the interface.
///
/// As with `install_protocol_interface`, any process that has registered to wait for the installation of
/// the interface is notified.
///
/// # Safety
///
/// The caller is responsible for ensuring that there are no references to the `old_interface` that is being
/// removed.
pub unsafe fn reinstall_protocol_interface(
&self,
handle: Handle,
protocol: &Guid,
old_interface: *mut c_void,
new_interface: *mut c_void,
) -> Result<()> {
(self.reinstall_protocol_interface)(handle, protocol, old_interface, new_interface).into()
}

/// Removes a protocol interface from a device handle.
///
/// # Safety
///
/// The caller is responsible for ensuring that there are no references to a protocol interface
/// that has been removed. Some protocols may not be able to be removed as there is no information
/// available regarding the references. This includes Console I/O, Block I/O, Disk I/o, and handles
/// to device protocols.
///
/// The caller is responsible for ensuring that they pass a valid `Guid` for `protocol`.
pub unsafe fn uninstall_protocol_interface(
&self,
handle: Handle,
protocol: &Guid,
interface: *mut c_void,
) -> Result<()> {
(self.uninstall_protocol_interface)(handle, protocol, interface).into()
}

/// Query a handle for a certain protocol.
///
/// This function attempts to get the protocol implementation of a handle,
Expand Down Expand Up @@ -655,6 +739,31 @@ impl BootServices {
})
}

/// Registers `event` to be signalled whenever a protocol interface is registered for
/// `protocol` by `install_protocol_interface()` or `reinstall_protocol_interface()`.
///
/// Once `event` has been signalled, `BootServices::locate_handle()` can be used to identify
/// the newly (re)installed handles that support `protocol`. The returned `SearchKey` on success
/// corresponds to the `search_key` parameter in `locate_handle()`.
///
/// Events can be unregistered from protocol interface notification by calling `close_event()`.
pub fn register_protocol_notify(
&self,
protocol: &Guid,
event: Event,
) -> Result<(Event, SearchType)> {
let mut key: MaybeUninit<ProtocolSearchKey> = MaybeUninit::uninit();
// Safety: we clone `event` a couple times, but there will be only one left once we return.
unsafe { (self.register_protocol_notify)(protocol, event.unsafe_clone(), key.as_mut_ptr()) }
// Safety: as long as this call is successful, `key` will be valid.
.into_with_val(|| unsafe {
(
event.unsafe_clone(),
SearchType::ByRegisterNotify(key.assume_init()),
)
})
}

/// Enumerates all handles installed on the system which match a certain query.
///
/// You should first call this function with `None` for the output buffer,
Expand All @@ -677,8 +786,9 @@ impl BootServices {

// Obtain the needed data from the parameters.
let (ty, guid, key) = match search_ty {
SearchType::AllHandles => (0, ptr::null(), ptr::null_mut()),
SearchType::ByProtocol(guid) => (2, guid as *const _, ptr::null_mut()),
SearchType::AllHandles => (0, None, None),
SearchType::ByRegisterNotify(registration) => (1, None, Some(registration)),
SearchType::ByProtocol(guid) => (2, Some(guid), None),
};

let status = unsafe { (self.locate_handle)(ty, guid, key, &mut buffer_size, buffer) };
Expand Down Expand Up @@ -1095,8 +1205,9 @@ impl BootServices {

// Obtain the needed data from the parameters.
let (ty, guid, key) = match search_ty {
SearchType::AllHandles => (0, ptr::null(), ptr::null_mut()),
SearchType::ByProtocol(guid) => (2, guid as *const _, ptr::null_mut()),
SearchType::AllHandles => (0, None, None),
SearchType::ByRegisterNotify(registration) => (1, None, Some(registration)),
SearchType::ByProtocol(guid) => (2, Some(guid), None),
};

unsafe { (self.locate_handle_buffer)(ty, guid, key, &mut num_handles, &mut buffer) }
Expand Down Expand Up @@ -1731,7 +1842,9 @@ pub enum SearchType<'guid> {
/// If the protocol implements the `Protocol` interface,
/// you can use the `from_proto` function to construct a new `SearchType`.
ByProtocol(&'guid Guid),
// TODO: add ByRegisterNotify once the corresponding function is implemented.
/// Return all handles that implement a protocol when an interface for that protocol
/// is (re)installed.
ByRegisterNotify(ProtocolSearchKey),
}

impl<'guid> SearchType<'guid> {
Expand Down Expand Up @@ -1844,3 +1957,19 @@ impl<'a> HandleBuffer<'a> {
unsafe { slice::from_raw_parts(self.buffer, self.count) }
}
}

newtype_enum! {
/// Interface type of a protocol interface
///
/// Only has one variant when this was written (v2.10 of the UEFI spec)
pub enum InterfaceType: i32 => {
/// Native interface
NATIVE_INTERFACE = 0,
}
}

/// Opaque pointer returned by [`BootServices::register_protocol_notify`] to be used
/// with [`BootServices::locate_handle`] via [`SearchType::ByRegisterNotify`].
#[derive(Debug, Clone, Copy)]
#[repr(transparent)]
pub struct ProtocolSearchKey(NonNull<c_void>);
80 changes: 77 additions & 3 deletions uefi-test-runner/src/boot/misc.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use core::ffi::c_void;
use core::ptr::NonNull;
use core::ptr::{self, NonNull};

use uefi::table::boot::{BootServices, EventType, TimerTrigger, Tpl};
use uefi::Event;
use uefi::proto::Protocol;
use uefi::table::boot::{BootServices, EventType, SearchType, TimerTrigger, Tpl};
use uefi::{unsafe_guid, Event, Identify};

pub fn test(bt: &BootServices) {
info!("Testing timer...");
Expand All @@ -12,6 +13,11 @@ pub fn test(bt: &BootServices) {
test_callback_with_ctx(bt);
info!("Testing watchdog...");
test_watchdog(bt);
info!("Testing protocol handler services...");
test_register_protocol_notify(bt);
test_install_protocol_interface(bt);
test_reinstall_protocol_interface(bt);
test_uninstall_protocol_interface(bt);
}

fn test_timer(bt: &BootServices) {
Expand Down Expand Up @@ -72,3 +78,71 @@ fn test_watchdog(bt: &BootServices) {
bt.set_watchdog_timer(0, 0x10000, None)
.expect("Could not set watchdog timer");
}

/// Dummy protocol for tests
#[unsafe_guid("1a972918-3f69-4b5d-8cb4-cece2309c7f5")]
#[derive(Protocol)]
struct TestProtocol {}

unsafe extern "efiapi" fn _test_notify(_event: Event, _context: Option<NonNull<c_void>>) {
info!("Protocol was (re)installed and this function notified.")
}

fn test_register_protocol_notify(bt: &BootServices) {
let protocol = &TestProtocol::GUID;
let event = unsafe {
bt.create_event(
EventType::NOTIFY_SIGNAL,
Tpl::NOTIFY,
Some(_test_notify),
None,
)
.expect("Failed to create an event")
};

bt.register_protocol_notify(protocol, event)
.expect("Failed to register protocol notify fn");
}

fn test_install_protocol_interface(bt: &BootServices) {
info!("Installing TestProtocol");

let _ = unsafe {
bt.install_protocol_interface(None, &TestProtocol::GUID, ptr::null_mut())
.expect("Failed to install protocol interface")
};

let _ = bt
Copy link
Contributor

@phip1611 phip1611 Nov 12, 2022

Choose a reason for hiding this comment

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

nit: These changes could be squashed with the commit(s) where the changes were introduced.

However, this is not a hard requirement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need to learn git a bit better anyway, so I'll see what I can do about cleaning up the history a bit today.

Copy link
Contributor

@phip1611 phip1611 Nov 12, 2022

Choose a reason for hiding this comment

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

👍 Moving things between existing git commits is kinda hard without experience. In the last months, I've spent some time on improving my skills. The following workflow worked for me. Maybe it helps you as well, hopefully:

  1. git rebase --interactive HEAD~4 (for four commits)
  2. in the editor, mark all commits you want to change with e/edit
  3. run cargo fmt && cargo clippy plus git add all changes
  4. run git rebase --continue
  5. While commits are left, goto 3.

Ideally, git should discard the "code style only commit" if you can pull these changes into earlier commits this way. Let me know what worked for you! :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That actually was a really easy way to understand how to split commits apart, thanks! I had no idea that git would detect and let you remove empty commits.

.locate_handle_buffer(SearchType::from_proto::<TestProtocol>())
.expect("Failed to find protocol after it was installed");
}

fn test_reinstall_protocol_interface(bt: &BootServices) {
info!("Reinstalling TestProtocol");
let handle = bt
.locate_handle_buffer(SearchType::from_proto::<TestProtocol>())
.expect("Failed to find protocol to uninstall")
.handles()[0];

unsafe {
let _ = bt.reinstall_protocol_interface(
handle,
&TestProtocol::GUID,
ptr::null_mut(),
ptr::null_mut(),
);
}
}

fn test_uninstall_protocol_interface(bt: &BootServices) {
info!("Uninstalling TestProtocol");
let handle = bt
.locate_handle_buffer(SearchType::from_proto::<TestProtocol>())
.expect("Failed to find protocol to uninstall")
.handles()[0];

unsafe {
bt.uninstall_protocol_interface(handle, &TestProtocol::GUID, ptr::null_mut())
.expect("Failed to uninstall protocol interface");
}
}
1 change: 1 addition & 0 deletions uefi-test-runner/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#![no_std]
#![no_main]
#![feature(abi_efiapi)]
#![feature(negative_impls)]
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't see why this is required. Can you please give some details?

Copy link
Contributor Author

@timrobertsdev timrobertsdev Nov 12, 2022

Choose a reason for hiding this comment

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

Yes, rustc said it was required when I was implementing my dummy protocol with #[derive(Protocol].

Specifically in uefi-test-runner/src/boot/misc.rs:

/// Dummy protocol for tests
#[unsafe_guid("1a972918-3f69-4b5d-8cb4-cece2309c7f5")]
#[derive(Protocol)]
struct TestProtocol {}

Copy link
Contributor

Choose a reason for hiding this comment

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

Alright, thanks for the pointer. What does @nicholasbishop say? Is this fine?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is fine, just a consequence of the way we implement protocols.

(There's a little bit of commentary on negative_impl in #452; it might be something we want to drop in the future. But for this PR it's fine.)


#[macro_use]
extern crate log;
Expand Down