Skip to content

Commit c431ae4

Browse files
uefi: Change IMAGE_HANDLE to an atomic pointer
This aligns with how the std uefi target works: https://github.com/rust-lang/rust/blob/f654229c27267334023a22233795b88b75fc340e/library/std/src/os/uefi/env.rs#L9 And also avoids some unsafe; a static atomic be written without being `static mut`, and reads and writes are safe operations.
1 parent 1f0d83b commit c431ae4

File tree

1 file changed

+11
-31
lines changed

1 file changed

+11
-31
lines changed

uefi/src/table/boot.rs

Lines changed: 11 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ use core::ffi::c_void;
1313
use core::mem::{self, MaybeUninit};
1414
use core::ops::{Deref, DerefMut};
1515
use core::ptr::NonNull;
16+
use core::sync::atomic::{AtomicPtr, Ordering};
1617
use core::{ptr, slice};
1718

1819
#[cfg(feature = "alloc")]
@@ -22,19 +23,9 @@ pub use uefi_raw::table::boot::{
2223
EventType, InterfaceType, MemoryAttribute, MemoryDescriptor, MemoryType, Tpl,
2324
};
2425

25-
// TODO: this similar to `SyncUnsafeCell`. Once that is stabilized we
26-
// can use it instead.
27-
struct GlobalImageHandle {
28-
handle: UnsafeCell<Option<Handle>>,
29-
}
30-
31-
// Safety: reads and writes are managed via `set_image_handle` and
32-
// `BootServices::image_handle`.
33-
unsafe impl Sync for GlobalImageHandle {}
34-
35-
static IMAGE_HANDLE: GlobalImageHandle = GlobalImageHandle {
36-
handle: UnsafeCell::new(None),
37-
};
26+
/// Global image handle. This is only set by `BootServices::set_image_handle`,
27+
/// and it is only read by `BootServices::image_handle`.
28+
static IMAGE_HANDLE: AtomicPtr<c_void> = AtomicPtr::new(ptr::null_mut());
3829

3930
/// Size in bytes of a UEFI page.
4031
///
@@ -95,22 +86,10 @@ pub struct BootServices(uefi_raw::table::boot::BootServices);
9586
impl BootServices {
9687
/// Get the [`Handle`] of the currently-executing image.
9788
pub fn image_handle(&self) -> Handle {
98-
// Safety:
99-
//
100-
// `IMAGE_HANDLE` is only set by `set_image_handle`, see that
101-
// documentation for more details.
102-
//
103-
// Additionally, `image_handle` takes a `&self` which ensures it
104-
// can only be called while boot services are active. (After
105-
// exiting boot services, the image handle should not be
106-
// considered valid.)
107-
unsafe {
108-
IMAGE_HANDLE
109-
.handle
110-
.get()
111-
.read()
112-
.expect("set_image_handle has not been called")
113-
}
89+
let ptr = IMAGE_HANDLE.load(Ordering::Acquire);
90+
// Safety: the image handle must be valid. We know it is, because it was
91+
// set by `set_image_handle`, which has that same safety requirement.
92+
unsafe { Handle::from_ptr(ptr) }.expect("set_image_handle has not been called")
11493
}
11594

11695
/// Update the global image [`Handle`].
@@ -123,14 +102,15 @@ impl BootServices {
123102
///
124103
/// # Safety
125104
///
126-
/// This function should only be called as described above. The
105+
/// This function should only be called as described above,
106+
/// and the `image_handle` must be a valid image [`Handle`]. Then
127107
/// safety guarantees of [`BootServices::open_protocol_exclusive`]
128108
/// rely on the global image handle being correct.
129109
pub unsafe fn set_image_handle(&self, image_handle: Handle) {
130110
// As with `image_handle`, `&self` isn't actually used, but it
131111
// enforces that this function is only called while boot
132112
// services are active.
133-
IMAGE_HANDLE.handle.get().write(Some(image_handle));
113+
IMAGE_HANDLE.store(image_handle.as_ptr(), Ordering::Release);
134114
}
135115

136116
/// Raises a task's priority level and returns its previous level.

0 commit comments

Comments
 (0)