Skip to content

Commit 3cf95e9

Browse files
authored
patina_dxe_core: Fix memory allocation tests for aarch64 runtime types (#1241)
Current implementations of several tests and hob construction logic use a single page for runtime memory types. On aarch64, the runtime page allocation granularity is 64kb, so all of these memory regions are dropped causing the tests to fail.
1 parent 48d4076 commit 3cf95e9

File tree

6 files changed

+75
-35
lines changed

6 files changed

+75
-35
lines changed

patina_dxe_core/src/allocator.rs

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1236,7 +1236,6 @@ pub(crate) unsafe fn reset_allocators() {
12361236

12371237
#[cfg(test)]
12381238
#[coverage(off)]
1239-
#[cfg(target_arch = "x86_64")] // Issue #1071
12401239
mod tests {
12411240

12421241
use crate::{
@@ -1303,7 +1302,7 @@ mod tests {
13031302
));
13041303

13051304
// Required memory allocation hob for stack
1306-
let mut stack_base_address = 0xEB000;
1305+
let mut stack_base_address = 0x18B000;
13071306
stack_base_address = (physical_hob_list as u64).wrapping_add(stack_base_address);
13081307
let stack_hob = Hob::MemoryAllocation(&patina::pi::hob::MemoryAllocation {
13091308
header: patina::pi::hob::header::Hob {
@@ -1352,8 +1351,8 @@ mod tests {
13521351
let mut hob_list = HobList::default();
13531352
hob_list.discover_hobs(physical_hob_list);
13541353

1355-
// Create a stack HOB at an address NOT in the GCD (such as 0xEB000)
1356-
let stack_base_address = 0xEB000;
1354+
// Create a stack HOB at an address NOT in the GCD (such as 0x18B000)
1355+
let stack_base_address = 0x18B000;
13571356
let stack_pages = 0x20;
13581357

13591358
let stack_hob = Hob::MemoryAllocation(&patina::pi::hob::MemoryAllocation {
@@ -1396,7 +1395,7 @@ mod tests {
13961395
hob_list.discover_hobs(physical_hob_list);
13971396

13981397
// Required memory allocation hob for stack
1399-
let mut stack_base_address = 0xEB000;
1398+
let mut stack_base_address = 0x18B000;
14001399
stack_base_address = (physical_hob_list as u64).wrapping_add(stack_base_address);
14011400

14021401
let stack_hob = Hob::MemoryAllocation(&patina::pi::hob::MemoryAllocation {
@@ -1436,11 +1435,25 @@ mod tests {
14361435
{
14371436
let allocator = allocators.get_allocator(*memory_type).unwrap();
14381437

1439-
if *memory_type == efi::BOOT_SERVICES_DATA {
1440-
assert_eq!(allocator.stats().claimed_pages, 3);
1441-
} else {
1442-
assert_eq!(allocator.stats().claimed_pages, 1);
1443-
}
1438+
let granularity = match *memory_type {
1439+
efi::RESERVED_MEMORY_TYPE
1440+
| efi::RUNTIME_SERVICES_CODE
1441+
| efi::RUNTIME_SERVICES_DATA
1442+
| efi::ACPI_MEMORY_NVS => RUNTIME_PAGE_ALLOCATION_GRANULARITY,
1443+
_ => DEFAULT_PAGE_ALLOCATION_GRANULARITY,
1444+
};
1445+
1446+
let expected_pages = match *memory_type {
1447+
efi::BOOT_SERVICES_DATA => 3, // Stack + build_test_hob_list allocation
1448+
_ => granularity / patina::base::SIZE_4KB,
1449+
};
1450+
1451+
let claimed = allocator.stats().claimed_pages;
1452+
assert_eq!(
1453+
claimed, expected_pages,
1454+
"For memory type {:?}: expected {}, got {}",
1455+
memory_type, expected_pages, claimed
1456+
);
14441457
}
14451458

14461459
// Locate stack hob.

patina_dxe_core/src/allocator/usage_tests/uefi_memory_map.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,6 @@
140140
//!
141141
142142
#[cfg(test)]
143-
#[cfg(target_arch = "x86_64")] // Issue #1071
144143
mod tests {
145144
use crate::{
146145
allocator::{get_memory_map, init_memory_support, reset_allocators},
@@ -648,7 +647,7 @@ mod tests {
648647
.with_memory_allocation(MemoryAllocationConfig {
649648
memory_type: efi::RUNTIME_SERVICES_DATA,
650649
memory_base_address: SIZE_4MB as u64,
651-
memory_length: SIZE_4KB as u64,
650+
memory_length: crate::allocator::RUNTIME_PAGE_ALLOCATION_GRANULARITY as u64,
652651
name: ZERO,
653652
})
654653
}

patina_dxe_core/src/config_tables/memory_attributes_table.rs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ mod tests {
257257
systemtables::init_system_table,
258258
test_support,
259259
};
260-
use patina::base::UEFI_PAGE_SIZE;
260+
use patina::{base::UEFI_PAGE_SIZE, uefi_size_to_pages};
261261

262262
fn with_locked_state<F: Fn() + std::panic::RefUnwindSafe>(f: F) {
263263
test_support::with_global_lock(|| {
@@ -283,7 +283,6 @@ mod tests {
283283
}
284284

285285
#[test]
286-
#[cfg(target_arch = "x86_64")] // Issue #1071
287286
fn test_memory_attributes_table_generation() {
288287
with_locked_state(|| {
289288
// Create a vector to store the allocated pages
@@ -305,23 +304,27 @@ mod tests {
305304
_ => (efi::BOOT_SERVICES_DATA, efi::MEMORY_XP),
306305
};
307306

307+
// Allocate in granularity chunks. Otherwise the allocation will auto-expand.
308+
let page_count =
309+
entry_count * uefi_size_to_pages!(crate::allocator::RUNTIME_PAGE_ALLOCATION_GRANULARITY);
310+
308311
let mut buffer_ptr: *mut u8 = core::ptr::null_mut();
309312
match core_allocate_pages(
310313
efi::ALLOCATE_ANY_PAGES,
311314
page_type.0,
312-
entry_count + 0x1,
315+
page_count,
313316
core::ptr::addr_of_mut!(buffer_ptr) as *mut efi::PhysicalAddress,
314317
None,
315318
) {
316319
// because we allocate top down, we need to insert at the front of the vector
317320
Ok(_) if page_type.0 != efi::BOOT_SERVICES_DATA => {
318-
allocated_pages.insert(0, (buffer_ptr, page_type, entry_count + 1))
321+
allocated_pages.insert(0, (buffer_ptr, page_type, page_count))
319322
}
320323
Ok(_) => (),
321324
_ => panic!("Failed to allocate pages"),
322325
}
323326

324-
let len = (entry_count + 1) * UEFI_PAGE_SIZE;
327+
let len = page_count * UEFI_PAGE_SIZE;
325328
// ignore failures here, we can't set attributes in the actual page table here, but the GCD will
326329
// get updated
327330
let _ = core_set_memory_space_capabilities(buffer_ptr as u64, len as u64, u64::MAX);

patina_dxe_core/src/gcd.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -756,7 +756,7 @@ mod tests {
756756
.unwrap();
757757
descriptors
758758
.iter()
759-
.find(|x| x.base_address == mem_base + 0xF0000 && x.memory_type == GcdMemoryType::Reserved)
759+
.find(|x| x.base_address == mem_base + 0x190000 && x.memory_type == GcdMemoryType::Reserved)
760760
.unwrap();
761761
//Note: resource descriptors 3 & are merged into a single contiguous region in GCD, so no separate entry exists.
762762
//So verify the length of the entry encompasses both.

patina_dxe_core/src/gcd/spin_locked_gcd.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2942,7 +2942,6 @@ impl<'a> Iterator for DescRangeIterator<'a> {
29422942

29432943
#[cfg(test)]
29442944
#[coverage(off)]
2945-
#[cfg(target_arch = "x86_64")] // Issue #1071
29462945
mod tests {
29472946
//! GCD (Global Coherency Domain) test module.
29482947
//!

patina_dxe_core/src/test_support.rs

Lines changed: 42 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
//!
99
//! SPDX-License-Identifier: Apache-2.0
1010
//!
11-
use crate::{GCD, protocols::PROTOCOL_DB};
11+
use crate::{GCD, allocator::DEFAULT_PAGE_ALLOCATION_GRANULARITY, protocols::PROTOCOL_DB};
1212
use core::ffi::c_void;
1313
use patina::{
1414
guids::ZERO,
@@ -211,13 +211,16 @@ pub(crate) fn with_global_lock<F: Fn() + std::panic::RefUnwindSafe>(f: F) -> Res
211211

212212
/// Allocates a chunk of memory of the specified size from the system allocator.
213213
///
214+
/// The memory allocated will be 64Kb aligned to simplify alignment requirements such
215+
/// as AArch64 runtime memory.
216+
///
214217
/// ## Safety
215218
/// This function is intended for test code only. The caller must ensure that the size is valid
216219
/// for allocation.
217220
pub(crate) unsafe fn get_memory(size: usize) -> &'static mut [u8] {
218221
// SAFETY: Test code - allocates memory from the system allocator with UEFI page alignment.
219222
// The returned slice is intentionally leaked for test simplicity and valid for 'static lifetime.
220-
let addr = unsafe { alloc::alloc::alloc(alloc::alloc::Layout::from_size_align(size, 0x1000).unwrap()) };
223+
let addr = unsafe { alloc::alloc::alloc(alloc::alloc::Layout::from_size_align(size, 0x10000).unwrap()) };
221224
// SAFETY: The allocated pointer is valid for `size` bytes and properly aligned.
222225
unsafe { core::slice::from_raw_parts_mut(addr, size) }
223226
}
@@ -285,16 +288,17 @@ pub(crate) fn build_test_hob_list(mem_size: u64) -> *const c_void {
285288
// SAFETY: Test code - allocates memory for the test HOB list.
286289
let mem = unsafe { get_memory(mem_size as usize) };
287290
let mem_base = mem.as_mut_ptr() as u64;
291+
assert!(mem_size >= 0x1B0000);
288292

289293
// Build a test HOB list that describes memory layout as follows:
290294
//
291295
// Base: offset 0 ************
292296
// HobList: offset base+0 HOBS
293297
// Empty: offset base+HobListSize N/A
294-
// SystemMemory offset base+0xE0000 SystemMemory (resource_descriptor1)
295-
// Reserved offset base+0xF0000 Untested SystemMemory (resource_descriptor2)
296-
// FreeMemory offset base+0x100000 FreeMemory (phit)
297-
// End offset base+0x200000 ************
298+
// SystemMemory offset base+0x0E0000 SystemMemory (resource_descriptor1)
299+
// Reserved offset base+0x190000 Untested SystemMemory (resource_descriptor2)
300+
// FreeMemory offset base+0x1A0000 FreeMemory (phit)
301+
// End offset base+mem_size ************
298302
//
299303
// The test HOB list will also include resource descriptor hobs that describe MMIO/IO as follows:
300304
// MMIO at 0x10000000 size 0x1000000 (resource_descriptor3)
@@ -304,9 +308,15 @@ pub(crate) fn build_test_hob_list(mem_size: u64) -> *const c_void {
304308
// Reserved Legacy I/O at 0x0000 size 0x1000 (resource_descriptor7)
305309
//
306310
// The test HOB list will also include resource allocation hobs that describe allocations as follows:
307-
// A Memory Allocation Hob for each memory type. This will be placed in the SystemMemory region at base+0xE0000 as
308-
// 4K allocations. There is also a Memory Allocation Hob for MMIO space at 0x10000000 for 0x2000 bytes.
309-
// A Firmware Volume HOB located in the FirmwareDevice region at 0x10000000
311+
// A Memory Allocation Hob for each memory type. This will be placed in the SystemMemory region at base+0xE0000 with
312+
// appropriate granularity for each type (64KB for runtime types on aarch64, 4KB otherwise). There is also a Memory
313+
// Allocation Hob for MMIO space at 0x10000000 for 0x2000 bytes. A Firmware Volume HOB located in the FirmwareDevice
314+
// region at 0x10000000
315+
//
316+
// The system memory is of length 0xB0000 bytes. This includes 0xA0000 for the regular allocations plus 0x10000 for
317+
// potential stack allocations. 0xA0000 bytes allows for each memory type to be aligned up to 64kb. Really, only
318+
// 0x70000 bytes is needed for that in the current layout of allocation hobs, but leaving room provides flexibility
319+
// for future changes.
310320
//
311321
let phit = hob::PhaseHandoffInformationTable {
312322
header: header::Hob {
@@ -319,11 +329,13 @@ pub(crate) fn build_test_hob_list(mem_size: u64) -> *const c_void {
319329
memory_top: mem_base + mem_size,
320330
memory_bottom: mem_base,
321331
free_memory_top: mem_base + mem_size,
322-
free_memory_bottom: mem_base + 0x100000,
332+
free_memory_bottom: mem_base + 0x1A0000,
323333
end_of_hob_list: mem_base
324334
+ core::mem::size_of::<hob::PhaseHandoffInformationTable>() as u64
325335
+ core::mem::size_of::<hob::Cpu>() as u64
326336
+ (core::mem::size_of::<ResourceDescriptorV2>() as u64) * 7
337+
+ (core::mem::size_of::<hob::MemoryAllocation>() as u64) * 11 // 10 memory type allocations + 1 MMIO
338+
+ core::mem::size_of::<hob::FirmwareVolume>() as u64
327339
+ core::mem::size_of::<header::Hob>() as u64,
328340
};
329341

@@ -345,7 +357,7 @@ pub(crate) fn build_test_hob_list(mem_size: u64) -> *const c_void {
345357
resource_type: hob::EFI_RESOURCE_SYSTEM_MEMORY,
346358
resource_attribute: hob::TESTED_MEMORY_ATTRIBUTES | hob::EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE,
347359
physical_start: mem_base + 0xE0000,
348-
resource_length: 0x10000,
360+
resource_length: 0xB0000,
349361
},
350362
attributes: efi::MEMORY_WB,
351363
};
@@ -360,7 +372,7 @@ pub(crate) fn build_test_hob_list(mem_size: u64) -> *const c_void {
360372
owner: efi::Guid::from_fields(0, 0, 0, 0, 0, &[0u8; 6]),
361373
resource_type: hob::EFI_RESOURCE_SYSTEM_MEMORY,
362374
resource_attribute: hob::INITIALIZED_MEMORY_ATTRIBUTES | hob::EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE,
363-
physical_start: mem_base + 0xF0000,
375+
physical_start: mem_base + 0x190000,
364376
resource_length: 0x10000,
365377
},
366378
attributes: efi::MEMORY_WB,
@@ -516,7 +528,8 @@ pub(crate) fn build_test_hob_list(mem_size: u64) -> *const c_void {
516528
cursor = cursor.offset(resource_descriptor7.v1.header.length as isize);
517529

518530
//memory allocation HOBs.
519-
for (idx, memory_type) in [
531+
let mut address: u64 = resource_descriptor1.v1.physical_start;
532+
for memory_type in [
520533
efi::RESERVED_MEMORY_TYPE,
521534
efi::LOADER_CODE,
522535
efi::LOADER_DATA,
@@ -529,16 +542,29 @@ pub(crate) fn build_test_hob_list(mem_size: u64) -> *const c_void {
529542
efi::PAL_CODE,
530543
]
531544
.iter()
532-
.enumerate()
533545
{
534-
allocation_hob_template.alloc_descriptor.memory_base_address =
535-
resource_descriptor1.v1.physical_start + idx as u64 * 0x1000;
546+
let granularity = match *memory_type {
547+
efi::RESERVED_MEMORY_TYPE
548+
| efi::RUNTIME_SERVICES_CODE
549+
| efi::RUNTIME_SERVICES_DATA
550+
| efi::ACPI_MEMORY_NVS => crate::allocator::RUNTIME_PAGE_ALLOCATION_GRANULARITY,
551+
_ => DEFAULT_PAGE_ALLOCATION_GRANULARITY,
552+
} as u64;
553+
554+
// Make sure the memory region is aligned as needed.
555+
address = patina::base::align_up(address, granularity).unwrap();
556+
allocation_hob_template.alloc_descriptor.memory_base_address = address;
536557
allocation_hob_template.alloc_descriptor.memory_type = *memory_type;
558+
allocation_hob_template.alloc_descriptor.memory_length = granularity;
537559

538560
core::ptr::copy(&allocation_hob_template, cursor as *mut hob::MemoryAllocation, 1);
539561
cursor = cursor.offset(allocation_hob_template.header.length as isize);
562+
address += granularity;
540563
}
541564

565+
// Double check this never over-runs the memory region.
566+
assert!(address <= resource_descriptor1.v1.physical_start + resource_descriptor1.v1.resource_length);
567+
542568
// memory allocation HOB for MMIO space
543569
allocation_hob_template.alloc_descriptor.memory_base_address = resource_descriptor3.v1.physical_start;
544570
allocation_hob_template.alloc_descriptor.memory_length = 0x2000;

0 commit comments

Comments
 (0)