Skip to content

Commit 90509c8

Browse files
committed
add offset argument to arch_regions
This allows having regions that start somewhere other than guest physical address 0. Useful in case not all regions are allocated at once, and each batch needs to be placed after all already allocated regions. Since the resulting code has some intricacies (and I actually got it wrong the first time around), write a kani harness to validate that we're computing the regions correctly. Signed-off-by: Patrick Roy <[email protected]>
1 parent 16164e4 commit 90509c8

File tree

4 files changed

+108
-16
lines changed

4 files changed

+108
-16
lines changed

src/vmm/src/arch/aarch64/mod.rs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,12 @@ pub const MMIO_MEM_SIZE: u64 = layout::DRAM_MEM_START - layout::MAPPED_IO_START;
5858

5959
/// Returns a Vec of the valid memory addresses for aarch64.
6060
/// See [`layout`](layout) module for a drawing of the specific memory model for this platform.
61-
pub fn arch_memory_regions(size: usize) -> Vec<(GuestAddress, usize)> {
62-
let dram_size = min(size, layout::DRAM_MEM_MAX_SIZE);
63-
vec![(GuestAddress(layout::DRAM_MEM_START), dram_size)]
61+
pub fn arch_memory_regions(offset: usize, size: usize) -> Vec<(GuestAddress, usize)> {
62+
let dram_size = min(size, layout::DRAM_MEM_MAX_SIZE - offset);
63+
vec![(
64+
GuestAddress(layout::DRAM_MEM_START + offset as u64),
65+
dram_size,
66+
)]
6467
}
6568

6669
/// Configures the system for booting Linux.
@@ -191,15 +194,15 @@ mod tests {
191194

192195
#[test]
193196
fn test_regions_lt_1024gb() {
194-
let regions = arch_memory_regions(1usize << 29);
197+
let regions = arch_memory_regions(0, 1usize << 29);
195198
assert_eq!(1, regions.len());
196199
assert_eq!(GuestAddress(super::layout::DRAM_MEM_START), regions[0].0);
197200
assert_eq!(1usize << 29, regions[0].1);
198201
}
199202

200203
#[test]
201204
fn test_regions_gt_1024gb() {
202-
let regions = arch_memory_regions(1usize << 41);
205+
let regions = arch_memory_regions(0, 1usize << 41);
203206
assert_eq!(1, regions.len());
204207
assert_eq!(GuestAddress(super::layout::DRAM_MEM_START), regions[0].0);
205208
assert_eq!(super::layout::DRAM_MEM_MAX_SIZE, regions[0].1);

src/vmm/src/arch/x86_64/mod.rs

Lines changed: 97 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -110,17 +110,33 @@ pub const MMIO_MEM_SIZE: u64 = MEM_32BIT_GAP_SIZE;
110110
/// These should be used to configure the GuestMemoryMmap structure for the platform.
111111
/// For x86_64 all addresses are valid from the start of the kernel except a
112112
/// carve out at the end of 32bit address space.
113-
pub fn arch_memory_regions(size: usize) -> Vec<(GuestAddress, usize)> {
113+
pub fn arch_memory_regions(offset: usize, size: usize) -> Vec<(GuestAddress, usize)> {
114+
// If we get here with size == 0 something has seriously gone wrong. Firecracker should never
115+
// try to allocate guest memory of size 0
116+
assert!(size > 0, "Attempt to allocate guest memory of length 0");
117+
assert!(
118+
offset.checked_add(size).is_some(),
119+
"Attempt to allocate guest memory such that the address space would wrap around"
120+
);
121+
114122
// It's safe to cast MMIO_MEM_START to usize because it fits in a u32 variable
115123
// (It points to an address in the 32 bit space).
116-
match size.checked_sub(usize::try_from(MMIO_MEM_START).unwrap()) {
124+
match (size + offset).checked_sub(u64_to_usize(MMIO_MEM_START)) {
117125
// case1: guest memory fits before the gap
118-
None | Some(0) => vec![(GuestAddress(0), size)],
119-
// case2: guest memory extends beyond the gap
120-
Some(remaining) => vec![
121-
(GuestAddress(0), usize::try_from(MMIO_MEM_START).unwrap()),
126+
None | Some(0) => vec![(GuestAddress(offset as u64), size)],
127+
// case2: starts before the gap, but doesn't completely fit
128+
Some(remaining) if (offset as u64) < MMIO_MEM_START => vec![
129+
(
130+
GuestAddress(offset as u64),
131+
u64_to_usize(MMIO_MEM_START) - offset,
132+
),
122133
(GuestAddress(FIRST_ADDR_PAST_32BITS), remaining),
123134
],
135+
// case3: guest memory start after the gap
136+
Some(_) => vec![(
137+
GuestAddress(FIRST_ADDR_PAST_32BITS.max(offset as u64)),
138+
size,
139+
)],
124140
}
125141
}
126142

@@ -456,6 +472,56 @@ pub fn load_kernel(
456472
})
457473
}
458474

475+
#[cfg(kani)]
476+
mod verification {
477+
use crate::arch::x86_64::FIRST_ADDR_PAST_32BITS;
478+
use crate::arch::{MMIO_MEM_START, arch_memory_regions};
479+
480+
#[kani::proof]
481+
#[kani::unwind(3)]
482+
fn verify_arch_memory_regions() {
483+
let offset: u64 = kani::any::<u64>();
484+
let len: u64 = kani::any::<u64>();
485+
486+
kani::assume(len > 0);
487+
kani::assume(offset.checked_add(len).is_some());
488+
489+
let regions = arch_memory_regions(offset as usize, len as usize);
490+
491+
// There's only one MMIO gap, so we can get either 1 or 2 regions
492+
assert!(regions.len() <= 2);
493+
assert!(regions.len() >= 1);
494+
495+
// The total length of all regions is what we requested
496+
assert_eq!(
497+
regions.iter().map(|&(_, len)| len).sum::<usize>(),
498+
len as usize
499+
);
500+
501+
// No region overlaps the MMIO gap
502+
assert!(
503+
regions
504+
.iter()
505+
.all(|&(start, len)| start.0 >= FIRST_ADDR_PAST_32BITS
506+
|| start.0 + len as u64 <= MMIO_MEM_START)
507+
);
508+
509+
// All regions start after our specified offset
510+
assert!(regions.iter().all(|&(start, _)| start.0 >= offset as u64));
511+
512+
// All regions have non-zero length
513+
assert!(regions.iter().all(|&(_, len)| len > 0));
514+
515+
// If there's two regions, they perfectly snuggle up to the MMIO gap
516+
if regions.len() == 2 {
517+
kani::cover!();
518+
519+
assert_eq!(regions[0].0.0 + regions[0].1 as u64, MMIO_MEM_START);
520+
assert_eq!(regions[1].0.0, FIRST_ADDR_PAST_32BITS);
521+
}
522+
}
523+
}
524+
459525
#[cfg(test)]
460526
mod tests {
461527
use linux_loader::loader::bootparam::boot_e820_entry;
@@ -466,18 +532,41 @@ mod tests {
466532

467533
#[test]
468534
fn regions_lt_4gb() {
469-
let regions = arch_memory_regions(1usize << 29);
535+
let regions = arch_memory_regions(0, 1usize << 29);
470536
assert_eq!(1, regions.len());
471537
assert_eq!(GuestAddress(0), regions[0].0);
472538
assert_eq!(1usize << 29, regions[0].1);
539+
540+
let regions = arch_memory_regions(1 << 28, 1 << 29);
541+
assert_eq!(1, regions.len());
542+
assert_eq!(regions[0], (GuestAddress(1 << 28), 1 << 29));
473543
}
474544

475545
#[test]
476546
fn regions_gt_4gb() {
477-
let regions = arch_memory_regions((1usize << 32) + 0x8000);
547+
const MEMORY_SIZE: usize = (1 << 32) + 0x8000;
548+
549+
let regions = arch_memory_regions(0, MEMORY_SIZE);
478550
assert_eq!(2, regions.len());
479551
assert_eq!(GuestAddress(0), regions[0].0);
480552
assert_eq!(GuestAddress(1u64 << 32), regions[1].0);
553+
554+
let regions = arch_memory_regions(1 << 31, MEMORY_SIZE);
555+
assert_eq!(2, regions.len());
556+
assert_eq!(
557+
regions[0],
558+
(
559+
GuestAddress(1 << 31),
560+
u64_to_usize(MMIO_MEM_START) - (1 << 31)
561+
)
562+
);
563+
assert_eq!(
564+
regions[1],
565+
(
566+
GuestAddress(FIRST_ADDR_PAST_32BITS),
567+
MEMORY_SIZE - regions[0].1
568+
)
569+
)
481570
}
482571

483572
#[test]

src/vmm/src/resources.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -458,7 +458,7 @@ impl VmResources {
458458
// a single way of backing guest memory for vhost-user and non-vhost-user cases,
459459
// that would not be worth the effort.
460460
let regions =
461-
crate::arch::arch_memory_regions(mib_to_bytes(self.machine_config.mem_size_mib));
461+
crate::arch::arch_memory_regions(0, mib_to_bytes(self.machine_config.mem_size_mib));
462462
if vhost_user_device_used {
463463
memory::memfd_backed(
464464
regions.as_ref(),

src/vmm/src/test_utils/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,11 @@ pub fn multi_region_mem_raw(regions: &[(GuestAddress, usize)]) -> Vec<GuestRegio
5858
/// Creates a [`GuestMemoryMmap`] of the given size with the contained regions laid out in
5959
/// accordance with the requirements of the architecture on which the tests are being run.
6060
pub fn arch_mem(mem_size_bytes: usize) -> GuestMemoryMmap {
61-
multi_region_mem(&crate::arch::arch_memory_regions(mem_size_bytes))
61+
multi_region_mem(&crate::arch::arch_memory_regions(0, mem_size_bytes))
6262
}
6363

6464
pub fn arch_mem_raw(mem_size_bytes: usize) -> Vec<GuestRegionMmap> {
65-
multi_region_mem_raw(&crate::arch::arch_memory_regions(mem_size_bytes))
65+
multi_region_mem_raw(&crate::arch::arch_memory_regions(0, mem_size_bytes))
6666
}
6767

6868
pub fn create_vmm(

0 commit comments

Comments
 (0)