refactor: store GuestMemoryMmap inside struct Vm

Semantically, this is where it belong - the memory is associated with
the virtual machine, not the virtual machine manager.

But independently of this, it is needed for borrowchk reasons: In the
future, each Vm will have two guest memory regions associated with it.
One for traditional guest memory, and another that is dedicated to I/O
(swiotlb region).  Storing these two GuestMemoryMmap in `struct Vm`
allows for a function as follows:

fn io_memory(&self) -> &GuestMemoryMmap {
	self.io_memory.as_ref()
		.unwrap_or(&self.normal_memory)
}

Such a function cannot live in `struct Vmm`, as that would mean
borrowing the guest memory implies borrowing the entire `Vmm` object
(because rustc is not intelligent enough to project split borrows past
function calls - e.g. it doesn't know the references returned by
io_memory() really only borrows the io_memory and normal_memory fields,
not everything else.). However, this means it cannot be used in
callsites where such splitting of borrows is required (e.g. everywhere
where both a guest memory reference _and_ another Vmm field are passed
by reference to a function).

The `Option`-ness of the the guest_memory field is transitory.

Signed-off-by: Patrick Roy <[email protected]>

Author: roypat

src/vmm/src/acpi/mod.rs

@@ -215,7 +215,7 @@ mod tests {
         // A mocke Vmm object with 128MBs of memory
         let mut vmm = default_vmm();
         let mut writer = AcpiTableWriter {
-            mem: &vmm.guest_memory,
+            mem: vmm.vm.guest_memory(),
             resource_allocator: &mut vmm.resource_allocator,
         };
 
@@ -264,13 +264,15 @@ mod tests {
     #[test]
     fn test_write_acpi_table_small_memory() {
         let mut vmm = default_vmm();
-        vmm.guest_memory = arch_mem(
-            (SYSTEM_MEM_START + SYSTEM_MEM_SIZE - 4096)
-                .try_into()
-                .unwrap(),
-        );
+        vmm.vm
+            .memory_init(arch_mem(
+                (SYSTEM_MEM_START + SYSTEM_MEM_SIZE - 4096)
+                    .try_into()
+                    .unwrap(),
+            ))
+            .unwrap();
         let mut writer = AcpiTableWriter {
-            mem: &vmm.guest_memory,
+            mem: vmm.vm.guest_memory(),
             resource_allocator: &mut vmm.resource_allocator,
         };
 

src/vmm/src/builder.rs

@@ -163,7 +163,7 @@ fn create_vmm_and_vcpus(
     // Set up Kvm Vm and register memory regions.
     // Build custom CPU config if a custom template is provided.
     let mut vm = Vm::new(&kvm)?;
-    vm.memory_init(&guest_memory)?;
+    vm.memory_init(guest_memory)?;
 
     let resource_allocator = ResourceAllocator::new()?;
 
@@ -199,7 +199,6 @@ fn create_vmm_and_vcpus(
         shutdown_exit_code: None,
         kvm,
         vm,
-        guest_memory,
         uffd,
         vcpus_handles: Vec::new(),
         vcpus_exit_evt,
@@ -488,7 +487,7 @@ pub fn build_microvm_from_snapshot(
 
     // Restore devices states.
     let mmio_ctor_args = MMIODevManagerConstructorArgs {
-        mem: &vmm.guest_memory,
+        mem: vmm.vm.guest_memory(),
         vm: vmm.vm.fd(),
         event_manager,
         resource_allocator: &mut vmm.resource_allocator,
@@ -504,7 +503,7 @@ pub fn build_microvm_from_snapshot(
 
     {
         let acpi_ctor_args = ACPIDeviceManagerConstructorArgs {
-            mem: &vmm.guest_memory,
+            mem: vmm.vm.guest_memory(),
             resource_allocator: &mut vmm.resource_allocator,
             vm: vmm.vm.fd(),
         };
@@ -795,7 +794,7 @@ pub fn configure_system_for_boot(
         )
         .map_err(LoadCommandline)?;
         crate::arch::x86_64::configure_system(
-            &vmm.guest_memory,
+            vmm.vm.guest_memory(),
             &mut vmm.resource_allocator,
             crate::vstate::memory::GuestAddress(crate::arch::x86_64::layout::CMDLINE_START),
             cmdline_size,
@@ -808,7 +807,7 @@ 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
         acpi::create_acpi_tables(
-            &vmm.guest_memory,
+            vmm.vm.guest_memory(),
             &mut vmm.resource_allocator,
             &vmm.mmio_device_manager,
             &vmm.acpi_device_manager,
@@ -835,7 +834,7 @@ pub fn configure_system_for_boot(
             .collect();
         let cmdline = boot_cmdline.as_cstring()?;
         crate::arch::aarch64::configure_system(
-            &vmm.guest_memory,
+            &vmm.vm.guest_memory(),
             cmdline,
             vcpu_mpidr,
             vmm.mmio_device_manager.get_device_info(),
@@ -891,7 +890,7 @@ pub(crate) fn attach_boot_timer_device(
 }
 
 fn attach_vmgenid_device(vmm: &mut Vmm) -> Result<(), StartMicrovmError> {
-    let vmgenid = VmGenId::new(&vmm.guest_memory, &mut vmm.resource_allocator)
+    let vmgenid = VmGenId::new(vmm.vm.guest_memory(), &mut vmm.resource_allocator)
         .map_err(StartMicrovmError::CreateVMGenID)?;
 
     vmm.acpi_device_manager
@@ -1088,7 +1087,7 @@ pub(crate) mod tests {
     }
 
     pub(crate) fn default_vmm() -> Vmm {
-        let (kvm, mut vm, guest_memory) = setup_vm_with_memory(128 << MIB_TO_BYTES_SHIFT);
+        let (kvm, mut vm) = setup_vm_with_memory(128 << MIB_TO_BYTES_SHIFT);
 
         let mmio_device_manager = MMIODeviceManager::new();
         let acpi_device_manager = ACPIDeviceManager::new();
@@ -1116,7 +1115,6 @@ pub(crate) mod tests {
             shutdown_exit_code: None,
             kvm,
             vm,
-            guest_memory,
             uffd: None,
             vcpus_handles: Vec::new(),
             vcpus_exit_evt,

src/vmm/src/device_manager/legacy.rs

@@ -248,7 +248,7 @@ mod tests {
 
     #[test]
     fn test_register_legacy_devices() {
-        let (_, vm, _) = setup_vm_with_memory(0x1000);
+        let (_, vm) = setup_vm_with_memory(0x1000);
         vm.setup_irqchip().unwrap();
         let mut ldm = PortIODeviceManager::new(
             Arc::new(Mutex::new(BusDevice::Serial(SerialDevice {

src/vmm/src/device_manager/mmio.rs

@@ -652,7 +652,7 @@ mod tests {
         let guest_mem = multi_region_mem(&[(start_addr1, 0x1000), (start_addr2, 0x1000)]);
         let kvm = Kvm::new(vec![]).expect("Cannot create Kvm");
         let mut vm = Vm::new(&kvm).unwrap();
-        vm.memory_init(&guest_mem).unwrap();
+        vm.memory_init(guest_mem).unwrap();
         let mut device_manager = MMIODeviceManager::new();
         let mut resource_allocator = ResourceAllocator::new().unwrap();
 
@@ -666,7 +666,7 @@ mod tests {
         device_manager
             .register_virtio_test_device(
                 vm.fd(),
-                guest_mem,
+                vm.guest_memory().clone(),
                 &mut resource_allocator,
                 dummy,
                 &mut cmdline,
@@ -683,7 +683,7 @@ mod tests {
         let guest_mem = multi_region_mem(&[(start_addr1, 0x1000), (start_addr2, 0x1000)]);
         let kvm = Kvm::new(vec![]).expect("Cannot create Kvm");
         let mut vm = Vm::new(&kvm).unwrap();
-        vm.memory_init(&guest_mem).unwrap();
+        vm.memory_init(guest_mem).unwrap();
         let mut device_manager = MMIODeviceManager::new();
         let mut resource_allocator = ResourceAllocator::new().unwrap();
 
@@ -697,7 +697,7 @@ mod tests {
             device_manager
                 .register_virtio_test_device(
                     vm.fd(),
-                    guest_mem.clone(),
+                    vm.guest_memory().clone(),
                     &mut resource_allocator,
                     Arc::new(Mutex::new(DummyDevice::new())),
                     &mut cmdline,
@@ -711,7 +711,7 @@ mod tests {
                 device_manager
                     .register_virtio_test_device(
                         vm.fd(),
-                        guest_mem,
+                        vm.guest_memory().clone(),
                         &mut resource_allocator,
                         Arc::new(Mutex::new(DummyDevice::new())),
                         &mut cmdline,
@@ -739,9 +739,7 @@ mod tests {
         let guest_mem = multi_region_mem(&[(start_addr1, 0x1000), (start_addr2, 0x1000)]);
         let kvm = Kvm::new(vec![]).expect("Cannot create Kvm");
         let mut vm = Vm::new(&kvm).unwrap();
-        vm.memory_init(&guest_mem).unwrap();
-
-        let mem_clone = guest_mem.clone();
+        vm.memory_init(guest_mem).unwrap();
 
         #[cfg(target_arch = "x86_64")]
         vm.setup_irqchip().unwrap();
@@ -758,7 +756,7 @@ mod tests {
         let addr = device_manager
             .register_virtio_test_device(
                 vm.fd(),
-                guest_mem,
+                vm.guest_memory().clone(),
                 &mut resource_allocator,
                 dummy,
                 &mut cmdline,
@@ -794,7 +792,7 @@ mod tests {
         device_manager
             .register_virtio_test_device(
                 vm.fd(),
-                mem_clone,
+                vm.guest_memory().clone(),
                 &mut resource_allocator,
                 dummy2,
                 &mut cmdline,

src/vmm/src/lib.rs

@@ -314,7 +314,6 @@ pub struct Vmm {
     // Guest VM core resources.
     kvm: Kvm,
     vm: Vm,
-    guest_memory: GuestMemoryMmap,
     // Save UFFD in order to keep it open in the Firecracker process, as well.
     uffd: Option<Uffd>,
     vcpus_handles: Vec<VcpuHandle>,
@@ -450,7 +449,7 @@ impl Vmm {
 
     /// Returns a reference to the inner `GuestMemoryMmap` object.
     pub fn guest_memory(&self) -> &GuestMemoryMmap {
-        &self.guest_memory
+        self.vm.guest_memory()
     }
 
     /// Sets RDA bit in serial console
@@ -602,7 +601,7 @@ impl Vmm {
 
     /// Retrieves the KVM dirty bitmap for each of the guest's memory regions.
     pub fn reset_dirty_bitmap(&self) {
-        self.guest_memory
+        self.guest_memory()
             .iter()
             .enumerate()
             .for_each(|(slot, region)| {
@@ -616,7 +615,7 @@ impl Vmm {
     /// Retrieves the KVM dirty bitmap for each of the guest's memory regions.
     pub fn get_dirty_bitmap(&self) -> Result<DirtyBitmap, VmmError> {
         let mut bitmap: DirtyBitmap = HashMap::new();
-        self.guest_memory
+        self.guest_memory()
             .iter()
             .enumerate()
             .try_for_each(|(slot, region)| {

src/vmm/src/vstate/vcpu/aarch64.rs

@@ -313,22 +313,21 @@ mod tests {
     use crate::cpu_config::templates::RegisterValueFilter;
     use crate::vcpu::VcpuConfig;
     use crate::vstate::kvm::Kvm;
-    use crate::vstate::memory::GuestMemoryMmap;
     use crate::vstate::vm::Vm;
     use crate::vstate::vm::tests::setup_vm_with_memory;
 
-    fn setup_vcpu(mem_size: usize) -> (Kvm, Vm, KvmVcpu, GuestMemoryMmap) {
-        let (kvm, mut vm, vm_mem) = setup_vm_with_memory(mem_size);
+    fn setup_vcpu(mem_size: usize) -> (Kvm, Vm, KvmVcpu) {
+        let (kvm, mut vm) = setup_vm_with_memory(mem_size);
         let mut vcpu = KvmVcpu::new(0, &vm).unwrap();
         vcpu.init(&[]).unwrap();
         vm.setup_irqchip(1).unwrap();
 
-        (kvm, vm, vcpu, vm_mem)
+        (kvm, vm, vcpu)
     }
 
     #[test]
     fn test_create_vcpu() {
-        let (_, vm, _) = setup_vm_with_memory(0x1000);
+        let (_, vm) = setup_vm_with_memory(0x1000);
 
         unsafe { libc::close(vm.fd().as_raw_fd()) };
 
@@ -344,7 +343,7 @@ mod tests {
 
     #[test]
     fn test_configure_vcpu() {
-        let (kvm, _, mut vcpu, vm_mem) = setup_vcpu(0x10000);
+        let (kvm, vm, mut vcpu) = setup_vcpu(0x10000);
         let optional_capabilities = kvm.optional_capabilities();
 
         let vcpu_config = VcpuConfig {
@@ -354,7 +353,7 @@ mod tests {
         };
 
         vcpu.configure(
-            &vm_mem,
+            vm.guest_memory(),
             EntryPoint {
                 entry_addr: GuestAddress(crate::arch::get_kernel_start()),
                 protocol: BootProtocol::LinuxBoot,
@@ -367,7 +366,7 @@ mod tests {
         unsafe { libc::close(vcpu.fd.as_raw_fd()) };
 
         let err = vcpu.configure(
-            &vm_mem,
+            vm.guest_memory(),
             EntryPoint {
                 entry_addr: GuestAddress(crate::arch::get_kernel_start()),
                 protocol: BootProtocol::LinuxBoot,
@@ -389,7 +388,7 @@ mod tests {
 
     #[test]
     fn test_init_vcpu() {
-        let (_, mut vm, _) = setup_vm_with_memory(0x1000);
+        let (_, mut vm) = setup_vm_with_memory(0x1000);
         let mut vcpu = KvmVcpu::new(0, &vm).unwrap();
         vm.setup_irqchip(1).unwrap();
 
@@ -408,7 +407,7 @@ mod tests {
 
     #[test]
     fn test_vcpu_save_restore_state() {
-        let (_, mut vm, _) = setup_vm_with_memory(0x1000);
+        let (_, mut vm) = setup_vm_with_memory(0x1000);
         let mut vcpu = KvmVcpu::new(0, &vm).unwrap();
         vm.setup_irqchip(1).unwrap();
 
@@ -452,7 +451,7 @@ mod tests {
         //
         // This should fail with ENOEXEC.
         // https://elixir.bootlin.com/linux/v5.10.176/source/arch/arm64/kvm/arm.c#L1165
-        let (_, mut vm, _) = setup_vm_with_memory(0x1000);
+        let (_, mut vm) = setup_vm_with_memory(0x1000);
         let vcpu = KvmVcpu::new(0, &vm).unwrap();
         vm.setup_irqchip(1).unwrap();
 
@@ -462,7 +461,7 @@ mod tests {
     #[test]
     fn test_dump_cpu_config_after_init() {
         // Test `dump_cpu_config()` after `KVM_VCPU_INIT`.
-        let (_, mut vm, _) = setup_vm_with_memory(0x1000);
+        let (_, mut vm) = setup_vm_with_memory(0x1000);
         let mut vcpu = KvmVcpu::new(0, &vm).unwrap();
         vm.setup_irqchip(1).unwrap();
         vcpu.init(&[]).unwrap();
@@ -472,7 +471,7 @@ mod tests {
 
     #[test]
     fn test_setup_non_boot_vcpu() {
-        let (_, vm, _) = setup_vm_with_memory(0x1000);
+        let (_, vm) = setup_vm_with_memory(0x1000);
         let mut vcpu1 = KvmVcpu::new(0, &vm).unwrap();
         vcpu1.init(&[]).unwrap();
         let mut vcpu2 = KvmVcpu::new(1, &vm).unwrap();
@@ -484,15 +483,15 @@ mod tests {
         // Test `get_regs()` with valid register IDs.
         // - X0: 0x6030 0000 0010 0000
         // - X1: 0x6030 0000 0010 0002
-        let (_, _, vcpu, _) = setup_vcpu(0x10000);
+        let (_, _, vcpu) = setup_vcpu(0x10000);
         let reg_list = Vec::<u64>::from([0x6030000000100000, 0x6030000000100002]);
         get_registers(&vcpu.fd, &reg_list, &mut Aarch64RegisterVec::default()).unwrap();
     }
 
     #[test]
     fn test_get_invalid_regs() {
         // Test `get_regs()` with invalid register IDs.
-        let (_, _, vcpu, _) = setup_vcpu(0x10000);
+        let (_, _, vcpu) = setup_vcpu(0x10000);
         let reg_list = Vec::<u64>::from([0x6030000000100001, 0x6030000000100003]);
         get_registers(&vcpu.fd, &reg_list, &mut Aarch64RegisterVec::default()).unwrap_err();
     }