Skip to content

Commit c7725c2

Browse files
committed
treewide: entry_size -> desc_size
This helps to prevent confusions. MemoryDescriptors and their reported size are already a pitfall. So at least we should refrain from using non-standard names for them.
1 parent eda7382 commit c7725c2

File tree

3 files changed

+57
-27
lines changed

3 files changed

+57
-27
lines changed

uefi-test-runner/src/boot/memory.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ fn memory_map(bt: &BootServices) {
6767
let sizes = bt.memory_map_size();
6868

6969
// 2 extra descriptors should be enough.
70-
let buf_sz = sizes.map_size + 2 * sizes.entry_size;
70+
let buf_sz = sizes.map_size + 2 * sizes.desc_size;
7171

7272
// We will use vectors for convenience.
7373
let mut buffer = vec![0_u8; buf_sz];

uefi/src/table/boot.rs

Lines changed: 40 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -189,38 +189,49 @@ impl BootServices {
189189
pub fn memory_map_size(&self) -> MemoryMapSize {
190190
let mut map_size = 0;
191191
let mut map_key = MemoryMapKey(0);
192-
let mut entry_size = 0;
192+
let mut desc_size = 0;
193193
let mut entry_version = 0;
194194

195195
let status = unsafe {
196196
(self.0.get_memory_map)(
197197
&mut map_size,
198198
ptr::null_mut(),
199199
&mut map_key.0,
200-
&mut entry_size,
200+
&mut desc_size,
201201
&mut entry_version,
202202
)
203203
};
204204
assert_eq!(status, Status::BUFFER_TOO_SMALL);
205205

206+
assert_eq!(
207+
map_size % desc_size,
208+
0,
209+
"Memory map must be a multiple of the reported descriptor size."
210+
);
211+
206212
MemoryMapSize {
207-
entry_size,
213+
desc_size,
208214
map_size,
209215
}
210216
}
211217

212-
/// Retrieves the current memory map.
218+
/// Stores the current UEFI memory map in the provided buffer.
213219
///
214-
/// The allocated buffer should be big enough to contain the memory map,
215-
/// and a way of estimating how big it should be is by calling `memory_map_size`.
220+
/// The allocated buffer must be at least aligned to a [`MemoryDescriptor`]
221+
/// and should be big enough to store the whole map. To estimating how big
222+
/// the map will be, you can call [`Self::memory_map_size`].
216223
///
217-
/// The buffer must be aligned like a `MemoryDescriptor`.
224+
/// The memory map contains entries of type [`MemoryDescriptor`]. However,
225+
/// the relevant step size is always the reported `desc_size` but never
226+
/// `size_of::<MemoryDescriptor>()`.
218227
///
219228
/// The returned key is a unique identifier of the current configuration of
220229
/// memory. Any allocations or such will change the memory map's key.
221230
///
222231
/// If you want to store the resulting memory map without having to keep
223232
/// the buffer around, you can use `.copied().collect()` on the iterator.
233+
/// Note that this will change the current memory map again, if the UEFI
234+
/// allocator is used under the hood.
224235
///
225236
/// # Errors
226237
///
@@ -233,7 +244,7 @@ impl BootServices {
233244
MemoryDescriptor::assert_aligned(buffer);
234245
let map_buffer = buffer.as_mut_ptr().cast::<MemoryDescriptor>();
235246
let mut map_key = MemoryMapKey(0);
236-
let mut entry_size = 0;
247+
let mut desc_size = 0;
237248
let mut entry_version = 0;
238249

239250
assert_eq!(
@@ -247,17 +258,17 @@ impl BootServices {
247258
&mut map_size,
248259
map_buffer,
249260
&mut map_key.0,
250-
&mut entry_size,
261+
&mut desc_size,
251262
&mut entry_version,
252263
)
253264
}
254265
.to_result_with_val(move || {
255-
let len = map_size / entry_size;
266+
let len = map_size / desc_size;
256267

257268
MemoryMap {
258269
key: map_key,
259270
buf: buffer,
260-
entry_size,
271+
desc_size,
261272
len,
262273
}
263274
})
@@ -1609,7 +1620,7 @@ pub struct MemoryMapKey(usize);
16091620
#[derive(Debug)]
16101621
pub struct MemoryMapSize {
16111622
/// Size of a single memory descriptor in bytes
1612-
pub entry_size: usize,
1623+
pub desc_size: usize,
16131624
/// Size of the entire memory map in bytes
16141625
pub map_size: usize,
16151626
}
@@ -1638,7 +1649,7 @@ pub struct MemoryMap<'buf> {
16381649
buf: &'buf mut [u8],
16391650
/// Usually bound to the size of a [`MemoryDescriptor`] but can indicate if
16401651
/// this field is ever extended by a new UEFI standard.
1641-
entry_size: usize,
1652+
desc_size: usize,
16421653
len: usize,
16431654
}
16441655

@@ -1649,13 +1660,19 @@ impl<'buf> MemoryMap<'buf> {
16491660
///
16501661
/// This allows parsing a memory map provided by a kernel after boot
16511662
/// services have already exited.
1652-
pub fn from_raw(buf: &'buf mut [u8], entry_size: usize) -> Self {
1653-
assert!(entry_size >= mem::size_of::<MemoryDescriptor>());
1654-
let len = buf.len() / entry_size;
1663+
pub fn from_raw(buf: &'buf mut [u8], desc_size: usize) -> Self {
1664+
assert!(!buf.is_empty());
1665+
assert_eq!(
1666+
buf.len() % desc_size,
1667+
0,
1668+
"The buffer length must be a multiple of the desc_size"
1669+
);
1670+
assert!(desc_size >= mem::size_of::<MemoryDescriptor>());
1671+
let len = buf.len() / desc_size;
16551672
MemoryMap {
16561673
key: MemoryMapKey(0),
16571674
buf,
1658-
entry_size,
1675+
desc_size,
16591676
len,
16601677
}
16611678
}
@@ -1723,15 +1740,15 @@ impl<'buf> MemoryMap<'buf> {
17231740

17241741
unsafe {
17251742
ptr::swap_nonoverlapping(
1726-
base.add(index1 * self.entry_size),
1727-
base.add(index2 * self.entry_size),
1728-
self.entry_size,
1743+
base.add(index1 * self.desc_size),
1744+
base.add(index2 * self.desc_size),
1745+
self.desc_size,
17291746
);
17301747
}
17311748
}
17321749

17331750
fn get_element_phys_addr(&self, index: usize) -> PhysicalAddress {
1734-
let offset = index.checked_mul(self.entry_size).unwrap();
1751+
let offset = index.checked_mul(self.desc_size).unwrap();
17351752
let elem = unsafe { &*self.buf.as_ptr().add(offset).cast::<MemoryDescriptor>() };
17361753
elem.phys_start
17371754
}
@@ -1763,7 +1780,7 @@ impl<'buf> MemoryMap<'buf> {
17631780
&*self
17641781
.buf
17651782
.as_ptr()
1766-
.add(self.entry_size * index)
1783+
.add(self.desc_size * index)
17671784
.cast::<MemoryDescriptor>()
17681785
};
17691786

@@ -1781,7 +1798,7 @@ impl<'buf> MemoryMap<'buf> {
17811798
&mut *self
17821799
.buf
17831800
.as_mut_ptr()
1784-
.add(self.entry_size * index)
1801+
.add(self.desc_size * index)
17851802
.cast::<MemoryDescriptor>()
17861803
};
17871804

uefi/src/table/system.rs

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ impl SystemTable<Boot> {
165165
let extra_entries = 8;
166166

167167
let memory_map_size = self.boot_services().memory_map_size();
168-
let extra_size = memory_map_size.entry_size.checked_mul(extra_entries)?;
168+
let extra_size = memory_map_size.desc_size.checked_mul(extra_entries)?;
169169
memory_map_size.map_size.checked_add(extra_size)
170170
}
171171

@@ -253,7 +253,12 @@ impl SystemTable<Boot> {
253253
let boot_services = self.boot_services();
254254

255255
// Reboot the device.
256-
let reset = |status| -> ! { self.runtime_services().reset(ResetType::COLD, status, None) };
256+
let reset = |status| -> ! {
257+
{
258+
log::warn!("Resetting the machine");
259+
self.runtime_services().reset(ResetType::COLD, status, None)
260+
}
261+
};
257262

258263
// Get the size of the buffer to allocate. If that calculation
259264
// overflows treat it as an unrecoverable error.
@@ -269,6 +274,11 @@ impl SystemTable<Boot> {
269274
Err(err) => reset(err.status()),
270275
};
271276

277+
/*let buf: &mut [u8] = unsafe { slice::from_raw_parts_mut(buf, buf_size) };
278+
279+
let buf = unsafe { Box::from_raw(buf) };
280+
let mut buf = ManuallyDrop::new(buf);*/
281+
272282
// Calling `exit_boot_services` can fail if the memory map key is not
273283
// current. Retry a second time if that occurs. This matches the
274284
// behavior of the Linux kernel:
@@ -284,7 +294,10 @@ impl SystemTable<Boot> {
284294
};
285295
return (st, memory_map);
286296
}
287-
Err(err) => status = err.status(),
297+
Err(err) => {
298+
log::error!("Error retrieving the memory map for exiting the boot services");
299+
status = err.status()
300+
}
288301
}
289302
}
290303

0 commit comments

Comments
 (0)