Skip to content

Commit 4f39170

Browse files
committed
refactor: simplify ResourceAllocator internals
Instead of storing internal allocators of ResourceAllocator within an Arc<Mutex<>> container, just store `ResourceAllocator` itself in an `Arc<Mutex<>>`. Apart from that, we get rid of the `ResourceAllocatorState` state object, and just clone `ResourceAllocator` itself when we want to save/restore. Also, make the creation of `ResourceAllocato` infallible, since we know that the ranges we are using are correct. Finally, fix saving/restoring the state of ResourceAllocator. We were actually not resetting it correctly upon snapshot restore. The reason why this was not a problem is that we don't actually need to perform any new allocations post restore at the moment. However, like this we are ready when we need to perform any hot-plugging operations. Also, add a unit-test to ensure that this logic works correctly. Signed-off-by: Babis Chalios <bchalios@amazon.es>
1 parent 0c940cd commit 4f39170

File tree

17 files changed

+185
-174
lines changed

17 files changed

+185
-174
lines changed

Cargo.lock

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/pci/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ libc = "0.2.172"
1818
log = "0.4.27"
1919
serde = { version = "1.0.219", features = ["derive"] }
2020
thiserror = "2.0.12"
21-
vm-allocator = "0.1.2"
21+
vm-allocator = "0.1.3"
2222
vm-device = { path = "../vm-device" }
2323
vm-memory = { version = "0.16.1", features = [
2424
"backend-mmap",

src/vmm/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ userfaultfd = "0.8.1"
5252
utils = { path = "../utils" }
5353
uuid = "1.16.0"
5454
vhost = { version = "0.14.0", features = ["vhost-user-frontend"] }
55-
vm-allocator = { version = "0.1.2", features = ["serde"] }
55+
vm-allocator = { version = "0.1.3", features = ["serde"] }
5656
vm-device = { path = "../vm-device" }
5757
vm-memory = { version = "0.16.2", features = [
5858
"backend-mmap",

src/vmm/src/acpi/mod.rs

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ impl AcpiTableWriter<'_> {
5454
/// buffer. It returns the address in which it wrote the table.
5555
fn write_acpi_table<S>(
5656
&mut self,
57-
resource_allocator: &ResourceAllocator,
57+
resource_allocator: &mut ResourceAllocator,
5858
table: &mut S,
5959
) -> Result<u64, AcpiError>
6060
where
@@ -83,7 +83,7 @@ impl AcpiTableWriter<'_> {
8383
fn build_dsdt(
8484
&mut self,
8585
device_manager: &mut DeviceManager,
86-
resource_allocator: &ResourceAllocator,
86+
resource_allocator: &mut ResourceAllocator,
8787
) -> Result<u64, AcpiError> {
8888
let mut dsdt_data = Vec::new();
8989

@@ -111,7 +111,7 @@ impl AcpiTableWriter<'_> {
111111
/// This includes a pointer with the location of the DSDT in guest memory
112112
fn build_fadt(
113113
&mut self,
114-
resource_allocator: &ResourceAllocator,
114+
resource_allocator: &mut ResourceAllocator,
115115
dsdt_addr: u64,
116116
) -> Result<u64, AcpiError> {
117117
let mut fadt = Fadt::new(OEM_ID, *b"FCVMFADT", OEM_REVISION);
@@ -129,7 +129,7 @@ impl AcpiTableWriter<'_> {
129129
/// This includes information about the interrupt controllers supported in the platform
130130
fn build_madt(
131131
&mut self,
132-
resource_allocator: &ResourceAllocator,
132+
resource_allocator: &mut ResourceAllocator,
133133
nr_vcpus: u8,
134134
) -> Result<u64, AcpiError> {
135135
let mut madt = Madt::new(
@@ -147,7 +147,7 @@ impl AcpiTableWriter<'_> {
147147
/// Currently, we pass to the guest just FADT and MADT tables.
148148
fn build_xsdt(
149149
&mut self,
150-
resource_allocator: &ResourceAllocator,
150+
resource_allocator: &mut ResourceAllocator,
151151
fadt_addr: u64,
152152
madt_addr: u64,
153153
mcfg_addr: u64,
@@ -164,7 +164,7 @@ impl AcpiTableWriter<'_> {
164164
/// Build the MCFG table for the guest.
165165
fn build_mcfg(
166166
&mut self,
167-
resource_allocator: &ResourceAllocator,
167+
resource_allocator: &mut ResourceAllocator,
168168
pci_mmio_config_addr: u64,
169169
) -> Result<u64, AcpiError> {
170170
let mut mcfg = Mcfg::new(OEM_ID, *b"FCMVMCFG", OEM_REVISION, pci_mmio_config_addr);
@@ -197,7 +197,7 @@ impl AcpiTableWriter<'_> {
197197
pub(crate) fn create_acpi_tables(
198198
mem: &GuestMemoryMmap,
199199
device_manager: &mut DeviceManager,
200-
resource_allocator: &ResourceAllocator,
200+
resource_allocator: &mut ResourceAllocator,
201201
vcpus: &[Vcpu],
202202
) -> Result<(), AcpiError> {
203203
let mut writer = AcpiTableWriter { mem };
@@ -249,18 +249,19 @@ mod tests {
249249
let mut writer = AcpiTableWriter {
250250
mem: vmm.vm.guest_memory(),
251251
};
252+
let mut resource_allocator = vmm.vm.resource_allocator();
252253

253254
// This should succeed
254255
let mut sdt = MockSdt(vec![0; 4096]);
255256
let addr = writer
256-
.write_acpi_table(&vmm.vm.common.resource_allocator, &mut sdt)
257+
.write_acpi_table(&mut resource_allocator, &mut sdt)
257258
.unwrap();
258259
assert_eq!(addr, SYSTEM_MEM_START);
259260

260261
// Let's try to write two 4K pages plus one byte
261262
let mut sdt = MockSdt(vec![0; usize::try_from(SYSTEM_MEM_SIZE + 1).unwrap()]);
262263
let err = writer
263-
.write_acpi_table(&vmm.vm.common.resource_allocator, &mut sdt)
264+
.write_acpi_table(&mut resource_allocator, &mut sdt)
264265
.unwrap_err();
265266
assert!(
266267
matches!(
@@ -275,27 +276,27 @@ mod tests {
275276
// succeed.
276277
let mut sdt = MockSdt(vec![0; 5]);
277278
let addr = writer
278-
.write_acpi_table(&vmm.vm.common.resource_allocator, &mut sdt)
279+
.write_acpi_table(&mut resource_allocator, &mut sdt)
279280
.unwrap();
280281
assert_eq!(addr, SYSTEM_MEM_START + 4096);
281282
let mut sdt = MockSdt(vec![0; 2]);
282283
let addr = writer
283-
.write_acpi_table(&vmm.vm.common.resource_allocator, &mut sdt)
284+
.write_acpi_table(&mut resource_allocator, &mut sdt)
284285
.unwrap();
285286
assert_eq!(addr, SYSTEM_MEM_START + 4101);
286287
let mut sdt = MockSdt(vec![0; 4]);
287288
let addr = writer
288-
.write_acpi_table(&vmm.vm.common.resource_allocator, &mut sdt)
289+
.write_acpi_table(&mut resource_allocator, &mut sdt)
289290
.unwrap();
290291
assert_eq!(addr, SYSTEM_MEM_START + 4103);
291292
let mut sdt = MockSdt(vec![0; 8]);
292293
let addr = writer
293-
.write_acpi_table(&vmm.vm.common.resource_allocator, &mut sdt)
294+
.write_acpi_table(&mut resource_allocator, &mut sdt)
294295
.unwrap();
295296
assert_eq!(addr, SYSTEM_MEM_START + 4107);
296297
let mut sdt = MockSdt(vec![0; 16]);
297298
let addr = writer
298-
.write_acpi_table(&vmm.vm.common.resource_allocator, &mut sdt)
299+
.write_acpi_table(&mut resource_allocator, &mut sdt)
299300
.unwrap();
300301
assert_eq!(addr, SYSTEM_MEM_START + 4115);
301302
}
@@ -312,11 +313,11 @@ mod tests {
312313
let mut writer = AcpiTableWriter {
313314
mem: vm.guest_memory(),
314315
};
315-
let resource_allocator = ResourceAllocator::new().unwrap();
316+
let mut resource_allocator = ResourceAllocator::new();
316317

317318
let mut sdt = MockSdt(vec![0; usize::try_from(SYSTEM_MEM_SIZE).unwrap()]);
318319
let err = writer
319-
.write_acpi_table(&resource_allocator, &mut sdt)
320+
.write_acpi_table(&mut resource_allocator, &mut sdt)
320321
.unwrap_err();
321322
assert!(
322323
matches!(

src/vmm/src/arch/aarch64/vm.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
// Copyright 2025 Amazon.com, Inc. or its affiliates. All Rights Reserved.
22
// SPDX-License-Identifier: Apache-2.0
33

4+
use std::sync::Mutex;
5+
46
use serde::{Deserialize, Serialize};
57

68
use crate::Kvm;
79
use crate::arch::aarch64::gic::GicState;
810
use crate::vstate::memory::{GuestMemoryExtension, GuestMemoryState};
11+
use crate::vstate::resources::ResourceAllocator;
912
use crate::vstate::vm::{VmCommon, VmError};
1013

1114
/// Structure representing the current architecture's understand of what a "virtual machine" is.
@@ -74,6 +77,7 @@ impl ArchVm {
7477
.get_irqchip()
7578
.save_device(mpidrs)
7679
.map_err(ArchVmError::SaveGic)?,
80+
resource_allocator: self.resource_allocator().clone(),
7781
})
7882
}
7983

@@ -86,6 +90,7 @@ impl ArchVm {
8690
self.get_irqchip()
8791
.restore_device(mpidrs, &state.gic)
8892
.map_err(ArchVmError::RestoreGic)?;
93+
self.common.resource_allocator = Mutex::new(state.resource_allocator.clone());
8994

9095
Ok(())
9196
}
@@ -98,4 +103,6 @@ pub struct VmState {
98103
pub memory: GuestMemoryState,
99104
/// GIC state.
100105
pub gic: GicState,
106+
/// resource allocator
107+
pub resource_allocator: ResourceAllocator,
101108
}

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

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ pub fn configure_system_for_boot(
217217
// Note that this puts the mptable at the last 1k of Linux's 640k base RAM
218218
mptable::setup_mptable(
219219
vm.guest_memory(),
220-
&vm.common.resource_allocator,
220+
&mut vm.resource_allocator(),
221221
vcpu_config.vcpu_count,
222222
)
223223
.map_err(ConfigurationError::MpTableSetup)?;
@@ -241,7 +241,7 @@ pub fn configure_system_for_boot(
241241
create_acpi_tables(
242242
vm.guest_memory(),
243243
device_manager,
244-
&vm.common.resource_allocator,
244+
&mut vm.resource_allocator(),
245245
vcpus,
246246
)?;
247247
Ok(())
@@ -607,8 +607,8 @@ mod tests {
607607
fn test_system_configuration() {
608608
let no_vcpus = 4;
609609
let gm = single_region_mem(0x10000);
610-
let resource_allocator = ResourceAllocator::new().unwrap();
611-
let err = mptable::setup_mptable(&gm, &resource_allocator, 1);
610+
let mut resource_allocator = ResourceAllocator::new();
611+
let err = mptable::setup_mptable(&gm, &mut resource_allocator, 1);
612612
assert!(matches!(
613613
err.unwrap_err(),
614614
mptable::MptableError::NotEnoughMemory
@@ -617,24 +617,24 @@ mod tests {
617617
// Now assigning some memory that falls before the 32bit memory hole.
618618
let mem_size = mib_to_bytes(128);
619619
let gm = arch_mem(mem_size);
620-
let resource_allocator = ResourceAllocator::new().unwrap();
621-
mptable::setup_mptable(&gm, &resource_allocator, no_vcpus).unwrap();
620+
let mut resource_allocator = ResourceAllocator::new();
621+
mptable::setup_mptable(&gm, &mut resource_allocator, no_vcpus).unwrap();
622622
configure_64bit_boot(&gm, GuestAddress(0), 0, &None).unwrap();
623623
configure_pvh(&gm, GuestAddress(0), &None).unwrap();
624624

625625
// Now assigning some memory that is equal to the start of the 32bit memory hole.
626626
let mem_size = mib_to_bytes(3328);
627627
let gm = arch_mem(mem_size);
628-
let resource_allocator = ResourceAllocator::new().unwrap();
629-
mptable::setup_mptable(&gm, &resource_allocator, no_vcpus).unwrap();
628+
let mut resource_allocator = ResourceAllocator::new();
629+
mptable::setup_mptable(&gm, &mut resource_allocator, no_vcpus).unwrap();
630630
configure_64bit_boot(&gm, GuestAddress(0), 0, &None).unwrap();
631631
configure_pvh(&gm, GuestAddress(0), &None).unwrap();
632632

633633
// Now assigning some memory that falls after the 32bit memory hole.
634634
let mem_size = mib_to_bytes(3330);
635635
let gm = arch_mem(mem_size);
636-
let resource_allocator = ResourceAllocator::new().unwrap();
637-
mptable::setup_mptable(&gm, &resource_allocator, no_vcpus).unwrap();
636+
let mut resource_allocator = ResourceAllocator::new();
637+
mptable::setup_mptable(&gm, &mut resource_allocator, no_vcpus).unwrap();
638638
configure_64bit_boot(&gm, GuestAddress(0), 0, &None).unwrap();
639639
configure_pvh(&gm, GuestAddress(0), &None).unwrap();
640640
}

src/vmm/src/arch/x86_64/mptable.rs

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ fn compute_mp_size(num_cpus: u8) -> usize {
116116
/// Performs setup of the MP table for the given `num_cpus`.
117117
pub fn setup_mptable(
118118
mem: &GuestMemoryMmap,
119-
resource_allocator: &ResourceAllocator,
119+
resource_allocator: &mut ResourceAllocator,
120120
num_cpus: u8,
121121
) -> Result<(), MptableError> {
122122
if num_cpus > MAX_SUPPORTED_CPUS {
@@ -334,27 +334,27 @@ mod tests {
334334
fn bounds_check() {
335335
let num_cpus = 4;
336336
let mem = single_region_mem_at(SYSTEM_MEM_START, compute_mp_size(num_cpus));
337-
let resource_allocator = ResourceAllocator::new().unwrap();
337+
let mut resource_allocator = ResourceAllocator::new();
338338

339-
setup_mptable(&mem, &resource_allocator, num_cpus).unwrap();
339+
setup_mptable(&mem, &mut resource_allocator, num_cpus).unwrap();
340340
}
341341

342342
#[test]
343343
fn bounds_check_fails() {
344344
let num_cpus = 4;
345345
let mem = single_region_mem_at(SYSTEM_MEM_START, compute_mp_size(num_cpus) - 1);
346-
let resource_allocator = ResourceAllocator::new().unwrap();
346+
let mut resource_allocator = ResourceAllocator::new();
347347

348-
setup_mptable(&mem, &resource_allocator, num_cpus).unwrap_err();
348+
setup_mptable(&mem, &mut resource_allocator, num_cpus).unwrap_err();
349349
}
350350

351351
#[test]
352352
fn mpf_intel_checksum() {
353353
let num_cpus = 1;
354354
let mem = single_region_mem_at(SYSTEM_MEM_START, compute_mp_size(num_cpus));
355-
let resource_allocator = ResourceAllocator::new().unwrap();
355+
let mut resource_allocator = ResourceAllocator::new();
356356

357-
setup_mptable(&mem, &resource_allocator, num_cpus).unwrap();
357+
setup_mptable(&mem, &mut resource_allocator, num_cpus).unwrap();
358358

359359
let mpf_intel: mpspec::mpf_intel = mem.read_obj(GuestAddress(SYSTEM_MEM_START)).unwrap();
360360

@@ -365,9 +365,9 @@ mod tests {
365365
fn mpc_table_checksum() {
366366
let num_cpus = 4;
367367
let mem = single_region_mem_at(SYSTEM_MEM_START, compute_mp_size(num_cpus));
368-
let resource_allocator = ResourceAllocator::new().unwrap();
368+
let mut resource_allocator = ResourceAllocator::new();
369369

370-
setup_mptable(&mem, &resource_allocator, num_cpus).unwrap();
370+
setup_mptable(&mem, &mut resource_allocator, num_cpus).unwrap();
371371

372372
let mpf_intel: mpspec::mpf_intel = mem.read_obj(GuestAddress(SYSTEM_MEM_START)).unwrap();
373373
let mpc_offset = GuestAddress(u64::from(mpf_intel.physptr));
@@ -388,9 +388,9 @@ mod tests {
388388
fn mpc_entry_count() {
389389
let num_cpus = 1;
390390
let mem = single_region_mem_at(SYSTEM_MEM_START, compute_mp_size(num_cpus));
391-
let resource_allocator = ResourceAllocator::new().unwrap();
391+
let mut resource_allocator = ResourceAllocator::new();
392392

393-
setup_mptable(&mem, &resource_allocator, num_cpus).unwrap();
393+
setup_mptable(&mem, &mut resource_allocator, num_cpus).unwrap();
394394

395395
let mpf_intel: mpspec::mpf_intel = mem.read_obj(GuestAddress(SYSTEM_MEM_START)).unwrap();
396396
let mpc_offset = GuestAddress(u64::from(mpf_intel.physptr));
@@ -419,8 +419,9 @@ mod tests {
419419
let mem = single_region_mem_at(SYSTEM_MEM_START, compute_mp_size(MAX_SUPPORTED_CPUS));
420420

421421
for i in 0..MAX_SUPPORTED_CPUS {
422-
let resource_allocator = ResourceAllocator::new().unwrap();
423-
setup_mptable(&mem, &resource_allocator, i).unwrap();
422+
let mut resource_allocator = ResourceAllocator::new();
423+
424+
setup_mptable(&mem, &mut resource_allocator, i).unwrap();
424425

425426
let mpf_intel: mpspec::mpf_intel =
426427
mem.read_obj(GuestAddress(SYSTEM_MEM_START)).unwrap();
@@ -450,9 +451,9 @@ mod tests {
450451
fn cpu_entry_count_max() {
451452
let cpus = MAX_SUPPORTED_CPUS + 1;
452453
let mem = single_region_mem_at(SYSTEM_MEM_START, compute_mp_size(cpus));
453-
let resource_allocator = ResourceAllocator::new().unwrap();
454+
let mut resource_allocator = ResourceAllocator::new();
454455

455-
let result = setup_mptable(&mem, &resource_allocator, cpus).unwrap_err();
456+
let result = setup_mptable(&mem, &mut resource_allocator, cpus).unwrap_err();
456457
assert_eq!(result, MptableError::TooManyCpus);
457458
}
458459
}

src/vmm/src/arch/x86_64/vm.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// SPDX-License-Identifier: Apache-2.0
33

44
use std::fmt;
5-
use std::sync::Arc;
5+
use std::sync::{Arc, Mutex};
66

77
use kvm_bindings::{
88
KVM_CLOCK_TSC_STABLE, KVM_IRQCHIP_IOAPIC, KVM_IRQCHIP_PIC_MASTER, KVM_IRQCHIP_PIC_SLAVE,
@@ -15,7 +15,7 @@ use crate::arch::x86_64::msr::MsrError;
1515
use crate::snapshot::Persist;
1616
use crate::utils::u64_to_usize;
1717
use crate::vstate::memory::{GuestMemoryExtension, GuestMemoryState};
18-
use crate::vstate::resources::ResourceAllocatorState;
18+
use crate::vstate::resources::ResourceAllocator;
1919
use crate::vstate::vm::{VmCommon, VmError};
2020

2121
/// Error type for [`Vm::restore_state`]
@@ -142,6 +142,7 @@ impl ArchVm {
142142
self.fd()
143143
.set_irqchip(&state.ioapic)
144144
.map_err(ArchVmError::SetIrqChipIoAPIC)?;
145+
self.common.resource_allocator = Mutex::new(state.resource_allocator.clone());
145146
Ok(())
146147
}
147148

@@ -195,7 +196,7 @@ impl ArchVm {
195196

196197
Ok(VmState {
197198
memory: self.common.guest_memory.describe(),
198-
resource_allocator: self.common.resource_allocator.save(),
199+
resource_allocator: self.resource_allocator().save(),
199200
pitstate,
200201
clock,
201202
pic_master,
@@ -221,7 +222,7 @@ pub struct VmState {
221222
/// guest memory state
222223
pub memory: GuestMemoryState,
223224
/// resource allocator
224-
pub resource_allocator: ResourceAllocatorState,
225+
pub resource_allocator: ResourceAllocator,
225226
pitstate: kvm_pit_state2,
226227
clock: kvm_clock_data,
227228
// TODO: rename this field to adopt inclusive language once Linux updates it, too.

0 commit comments

Comments
 (0)