refactor(vm): move ResourceAllocator inside Vm

ResourceAllocator object was part of DeviceManager since it is (mainly)
devices that use it. ResourceAllocator is as well the object that
implements (in a dummy way, for the moment) the DeviceRelocation trait
which PciDevices use to move the address space of a PciDevice when
triggered from the guest.

Problem with DeviceRelocation is that it also needs the Vm file
descriptor to perform the relocation, because we need to move register
the new IO event fd for VirtIO devices.

To make things simpler, move ResourceAllocator inside the Vm object. In
subsequent commit we will remove the DeviceRelocation from
ResourceAllocator and move it to Vm instead.

This has the nice secondary effect that we were able to simplify the
signature of many device-related methods that received Vm and
ResourceAllocator arguments.

Signed-off-by: Babis Chalios <[email protected]>

Author: bchalios

src/vmm/src/acpi/mod.rs

@@ -12,8 +12,8 @@ use crate::acpi::x86_64::{
 };
 use crate::arch::x86_64::layout;
 use crate::device_manager::DeviceManager;
-use crate::device_manager::resources::ResourceAllocator;
 use crate::vstate::memory::{GuestAddress, GuestMemoryMmap};
+use crate::vstate::resources::ResourceAllocator;
 
 mod x86_64;
 
@@ -80,7 +80,11 @@ impl AcpiTableWriter<'_> {
     }
 
     /// Build the DSDT table for the guest
-    fn build_dsdt(&mut self, device_manager: &mut DeviceManager) -> Result<u64, AcpiError> {
+    fn build_dsdt(
+        &mut self,
+        device_manager: &mut DeviceManager,
+        resource_allocator: &ResourceAllocator,
+    ) -> Result<u64, AcpiError> {
         let mut dsdt_data = Vec::new();
 
         // Virtio-devices DSDT data
@@ -99,7 +103,7 @@ impl AcpiTableWriter<'_> {
         setup_arch_dsdt(&mut dsdt_data)?;
 
         let mut dsdt = Dsdt::new(OEM_ID, *b"FCVMDSDT", OEM_REVISION, dsdt_data);
-        self.write_acpi_table(&device_manager.resource_allocator, &mut dsdt)
+        self.write_acpi_table(resource_allocator, &mut dsdt)
     }
 
     /// Build the FADT table for the guest
@@ -193,26 +197,16 @@ impl AcpiTableWriter<'_> {
 pub(crate) fn create_acpi_tables(
     mem: &GuestMemoryMmap,
     device_manager: &mut DeviceManager,
+    resource_allocator: &ResourceAllocator,
     vcpus: &[Vcpu],
 ) -> Result<(), AcpiError> {
     let mut writer = AcpiTableWriter { mem };
-    let dsdt_addr = writer.build_dsdt(device_manager)?;
-
-    let fadt_addr = writer.build_fadt(&device_manager.resource_allocator, dsdt_addr)?;
-    let madt_addr = writer.build_madt(
-        &device_manager.resource_allocator,
-        vcpus.len().try_into().unwrap(),
-    )?;
-    let mcfg_addr = writer.build_mcfg(
-        &device_manager.resource_allocator,
-        layout::PCI_MMCONFIG_START,
-    )?;
-    let xsdt_addr = writer.build_xsdt(
-        &device_manager.resource_allocator,
-        fadt_addr,
-        madt_addr,
-        mcfg_addr,
-    )?;
+    let dsdt_addr = writer.build_dsdt(device_manager, resource_allocator)?;
+
+    let fadt_addr = writer.build_fadt(resource_allocator, dsdt_addr)?;
+    let madt_addr = writer.build_madt(resource_allocator, vcpus.len().try_into().unwrap())?;
+    let mcfg_addr = writer.build_mcfg(resource_allocator, layout::PCI_MMCONFIG_START)?;
+    let xsdt_addr = writer.build_xsdt(resource_allocator, fadt_addr, madt_addr, mcfg_addr)?;
     writer.build_rsdp(xsdt_addr)
 }
 
@@ -224,8 +218,8 @@ mod tests {
     use crate::acpi::{AcpiError, AcpiTableWriter};
     use crate::arch::x86_64::layout::{SYSTEM_MEM_SIZE, SYSTEM_MEM_START};
     use crate::builder::tests::default_vmm;
-    use crate::device_manager::resources::ResourceAllocator;
     use crate::utils::u64_to_usize;
+    use crate::vstate::resources::ResourceAllocator;
     use crate::vstate::vm::tests::setup_vm_with_memory;
 
     struct MockSdt(Vec<u8>);
@@ -259,14 +253,14 @@ mod tests {
         // This should succeed
         let mut sdt = MockSdt(vec![0; 4096]);
         let addr = writer
-            .write_acpi_table(&vmm.device_manager.resource_allocator, &mut sdt)
+            .write_acpi_table(&vmm.vm.common.resource_allocator, &mut sdt)
             .unwrap();
         assert_eq!(addr, SYSTEM_MEM_START);
 
         // Let's try to write two 4K pages plus one byte
         let mut sdt = MockSdt(vec![0; usize::try_from(SYSTEM_MEM_SIZE + 1).unwrap()]);
         let err = writer
-            .write_acpi_table(&vmm.device_manager.resource_allocator, &mut sdt)
+            .write_acpi_table(&vmm.vm.common.resource_allocator, &mut sdt)
             .unwrap_err();
         assert!(
             matches!(
@@ -281,27 +275,27 @@ mod tests {
         // succeed.
         let mut sdt = MockSdt(vec![0; 5]);
         let addr = writer
-            .write_acpi_table(&vmm.device_manager.resource_allocator, &mut sdt)
+            .write_acpi_table(&vmm.vm.common.resource_allocator, &mut sdt)
             .unwrap();
         assert_eq!(addr, SYSTEM_MEM_START + 4096);
         let mut sdt = MockSdt(vec![0; 2]);
         let addr = writer
-            .write_acpi_table(&vmm.device_manager.resource_allocator, &mut sdt)
+            .write_acpi_table(&vmm.vm.common.resource_allocator, &mut sdt)
             .unwrap();
         assert_eq!(addr, SYSTEM_MEM_START + 4101);
         let mut sdt = MockSdt(vec![0; 4]);
         let addr = writer
-            .write_acpi_table(&vmm.device_manager.resource_allocator, &mut sdt)
+            .write_acpi_table(&vmm.vm.common.resource_allocator, &mut sdt)
             .unwrap();
         assert_eq!(addr, SYSTEM_MEM_START + 4103);
         let mut sdt = MockSdt(vec![0; 8]);
         let addr = writer
-            .write_acpi_table(&vmm.device_manager.resource_allocator, &mut sdt)
+            .write_acpi_table(&vmm.vm.common.resource_allocator, &mut sdt)
             .unwrap();
         assert_eq!(addr, SYSTEM_MEM_START + 4107);
         let mut sdt = MockSdt(vec![0; 16]);
         let addr = writer
-            .write_acpi_table(&vmm.device_manager.resource_allocator, &mut sdt)
+            .write_acpi_table(&vmm.vm.common.resource_allocator, &mut sdt)
             .unwrap();
         assert_eq!(addr, SYSTEM_MEM_START + 4115);
     }

src/vmm/src/arch/aarch64/fdt.rs

@@ -536,14 +536,7 @@ mod tests {
         let dummy = Arc::new(Mutex::new(DummyDevice::new()));
         device_manager
             .mmio_devices
-            .register_virtio_test_device(
-                &vm,
-                mem.clone(),
-                &device_manager.resource_allocator,
-                dummy,
-                &mut cmdline,
-                "dummy",
-            )
+            .register_virtio_test_device(&vm, mem.clone(), dummy, &mut cmdline, "dummy")
             .unwrap();
 
         create_fdt(

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

@@ -217,7 +217,7 @@ pub fn configure_system_for_boot(
     // Note that this puts the mptable at the last 1k of Linux's 640k base RAM
     mptable::setup_mptable(
         vm.guest_memory(),
-        &device_manager.resource_allocator,
+        &vm.common.resource_allocator,
         vcpu_config.vcpu_count,
     )
     .map_err(ConfigurationError::MpTableSetup)?;
@@ -238,7 +238,12 @@ pub fn configure_system_for_boot(
 
     // Create ACPI tables and write them in guest memory
     // For the time being we only support ACPI in x86_64
-    create_acpi_tables(vm.guest_memory(), device_manager, vcpus)?;
+    create_acpi_tables(
+        vm.guest_memory(),
+        device_manager,
+        &vm.common.resource_allocator,
+        vcpus,
+    )?;
     Ok(())
 }
 
@@ -568,9 +573,9 @@ mod tests {
     use linux_loader::loader::bootparam::boot_e820_entry;
 
     use super::*;
-    use crate::device_manager::resources::ResourceAllocator;
     use crate::test_utils::{arch_mem, single_region_mem};
     use crate::utils::mib_to_bytes;
+    use crate::vstate::resources::ResourceAllocator;
 
     #[test]
     fn regions_lt_4gb() {

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

@@ -15,10 +15,10 @@ use vm_allocator::AllocPolicy;
 
 use crate::arch::IRQ_MAX;
 use crate::arch::x86_64::generated::mpspec;
-use crate::device_manager::resources::ResourceAllocator;
 use crate::vstate::memory::{
     Address, ByteValued, Bytes, GuestAddress, GuestMemory, GuestMemoryMmap,
 };
+use crate::vstate::resources::ResourceAllocator;
 
 // These `mpspec` wrapper types are only data, reading them from data is a safe initialization.
 // SAFETY: POD

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

@@ -11,8 +11,10 @@ use kvm_ioctls::Cap;
 use serde::{Deserialize, Serialize};
 
 use crate::arch::x86_64::msr::MsrError;
+use crate::snapshot::Persist;
 use crate::utils::u64_to_usize;
 use crate::vstate::memory::{GuestMemoryExtension, GuestMemoryState};
+use crate::vstate::resources::ResourceAllocatorState;
 use crate::vstate::vm::{VmCommon, VmError};
 
 /// Error type for [`Vm::restore_state`]
@@ -187,6 +189,7 @@ impl ArchVm {
 
         Ok(VmState {
             memory: self.common.guest_memory.describe(),
+            resource_allocator: self.common.resource_allocator.save(),
             pitstate,
             clock,
             pic_master,
@@ -211,6 +214,8 @@ impl ArchVm {
 pub struct VmState {
     /// guest memory state
     pub memory: GuestMemoryState,
+    /// resource allocator
+    pub resource_allocator: ResourceAllocatorState,
     pitstate: kvm_pit_state2,
     clock: kvm_clock_data,
     // TODO: rename this field to adopt inclusive language once Linux updates it, too.

src/vmm/src/builder.rs

@@ -47,6 +47,8 @@ use crate::vmm_config::instance_info::InstanceInfo;
 use crate::vmm_config::machine_config::MachineConfigError;
 use crate::vstate::kvm::{Kvm, KvmError};
 use crate::vstate::memory::GuestRegionMmap;
+#[cfg(target_arch = "aarch64")]
+use crate::vstate::resources::ResourceAllocator;
 use crate::vstate::vcpu::VcpuError;
 use crate::vstate::vm::{Vm, VmError};
 use crate::{EventManager, Vmm, VmmError};
@@ -188,7 +190,7 @@ pub fn build_microvm_for_boot(
         .collect::<Result<Vec<_>, _>>()?;
 
     if vm_resources.pci_enabled {
-        device_manager.enable_pci()?;
+        device_manager.enable_pci(&vm)?;
     } else {
         boot_cmdline.insert("pci", "off")?;
     }
@@ -197,7 +199,7 @@ pub fn build_microvm_for_boot(
     // to maintain the same MMIO address referenced in the documentation
     // and tests.
     if vm_resources.boot_timer {
-        device_manager.attach_boot_timer_device(request_ts)?;
+        device_manager.attach_boot_timer_device(&vm, request_ts)?;
     }
 
     if let Some(balloon) = vm_resources.balloon.get() {
@@ -252,7 +254,7 @@ pub fn build_microvm_for_boot(
 
     #[cfg(target_arch = "aarch64")]
     if vcpus[0].kvm_vcpu.supports_pvtime() {
-        setup_pvtime(&mut device_manager, &mut vcpus)?;
+        setup_pvtime(&vm.common.resource_allocator, &mut vcpus)?;
     } else {
         log::warn!("Vcpus do not support pvtime, steal time will not be reported to guest");
     }
@@ -513,13 +515,12 @@ const STEALTIME_STRUCT_MEM_SIZE: u64 = 64;
 /// Helper method to allocate steal time region
 #[cfg(target_arch = "aarch64")]
 fn allocate_pvtime_region(
-    device_manager: &mut DeviceManager,
+    resource_allocator: &ResourceAllocator,
     vcpu_count: usize,
     policy: vm_allocator::AllocPolicy,
 ) -> Result<GuestAddress, StartMicrovmError> {
     let size = STEALTIME_STRUCT_MEM_SIZE * vcpu_count as u64;
-    let addr = device_manager
-        .resource_allocator
+    let addr = resource_allocator
         .allocate_system_memory(size, STEALTIME_STRUCT_MEM_SIZE, policy)
         .map_err(StartMicrovmError::AllocateResources)?;
     Ok(GuestAddress(addr))
@@ -528,12 +529,12 @@ fn allocate_pvtime_region(
 /// Sets up pvtime for all vcpus
 #[cfg(target_arch = "aarch64")]
 fn setup_pvtime(
-    device_manager: &mut DeviceManager,
+    resource_allocator: &ResourceAllocator,
     vcpus: &mut [Vcpu],
 ) -> Result<(), StartMicrovmError> {
     // Alloc sys mem for steal time region
     let pvtime_mem: GuestAddress = allocate_pvtime_region(
-        device_manager,
+        resource_allocator,
         vcpus.len(),
         vm_allocator::AllocPolicy::LastMatch,
     )?;
@@ -1141,7 +1142,9 @@ pub(crate) mod tests {
         let mut vmm = default_vmm();
         let request_ts = TimestampUs::default();
 
-        let res = vmm.device_manager.attach_boot_timer_device(request_ts);
+        let res = vmm
+            .device_manager
+            .attach_boot_timer_device(&vmm.vm, request_ts);
         res.unwrap();
         assert!(vmm.device_manager.mmio_devices.boot_timer.is_some());
     }

src/vmm/src/device_manager/legacy.rs

@@ -97,11 +97,7 @@ impl PortIODeviceManager {
     }
 
     /// Register supported legacy devices.
-    pub fn register_devices(
-        &mut self,
-        io_bus: &vm_device::Bus,
-        vm: &Vm,
-    ) -> Result<(), LegacyDeviceError> {
+    pub fn register_devices(&mut self, vm: &Vm) -> Result<(), LegacyDeviceError> {
         let serial_2_4 = Arc::new(Mutex::new(SerialDevice {
             serial: Serial::with_events(
                 self.com_evt_2_4.try_clone()?.try_clone()?,
@@ -122,6 +118,8 @@ impl PortIODeviceManager {
             ),
             input: None,
         }));
+
+        let io_bus = &vm.common.resource_allocator.pio_bus;
         io_bus.insert(
             self.stdio_serial.clone(),
             Self::SERIAL_PORT_ADDRESSES[0],
@@ -243,7 +241,6 @@ mod tests {
     #[test]
     fn test_register_legacy_devices() {
         let (_, vm) = setup_vm_with_memory(0x1000);
-        let io_bus = vm_device::Bus::new();
         vm.setup_irqchip().unwrap();
         let mut ldm = PortIODeviceManager::new(
             Arc::new(Mutex::new(SerialDevice {
@@ -261,6 +258,6 @@ mod tests {
             )),
         )
         .unwrap();
-        ldm.register_devices(&io_bus, &vm).unwrap();
+        ldm.register_devices(&vm).unwrap();
     }
 }