refactor(builder): address comments

address comments

Signed-off-by: tommady <[email protected]>

Author: tommady

src/vmm/src/builder.rs

@@ -274,8 +274,6 @@ pub mod aarch64 {
             vcpus.push(vcpu);
         }
 
-        setup_interrupt_controller(vm, vcpu_count)?;
-
         Ok(vcpus)
     }
 
@@ -317,6 +315,9 @@ pub mod aarch64 {
         let vcpus = create_vcpus(&mut vmm.vm, vm_config.vcpu_count, &vmm.vcpus_exit_evt)
             .map_err(StartMicrovmError::Internal)?;
 
+        setup_interrupt_controller(&mut vmm.vm, vm_config.vcpu_count)
+            .map_err(StartMicrovmError::Internal)?;
+
         Ok((vmm, vcpus))
     }
 }
@@ -459,6 +460,33 @@ pub mod x86_64 {
             kvm_capabilities,
         )?;
 
+        let pio_device_manager = {
+            // Serial device setup.
+            let serial_device = setup_serial_device(std::io::stdin(), io::stdout())
+                .map_err(StartMicrovmError::Internal)?;
+
+            // x86_64 uses the i8042 reset event as the Vmm exit event.
+            let reset_evt = vmm
+                .vcpus_exit_evt
+                .try_clone()
+                .map_err(VmmError::EventFd)
+                .map_err(StartMicrovmError::Internal)?;
+
+            setup_interrupt_controller(&mut vmm.vm).map_err(StartMicrovmError::Internal)?;
+
+            // create pio dev manager with legacy devices
+            let pio_device_manager = {
+                // TODO Remove these unwraps.
+                let mut pio_dev_mgr = PortIODeviceManager::new(serial_device, reset_evt).unwrap();
+                pio_dev_mgr.register_devices(vmm.vm.fd()).unwrap();
+                pio_dev_mgr
+            };
+
+            pio_device_manager
+        };
+
+        vmm.pio_device_manager = Some(pio_device_manager);
+
         let vcpus = create_vcpus(&mut vmm.vm, vm_config.vcpu_count, &vmm.vcpus_exit_evt)
             .map_err(StartMicrovmError::Internal)?;
 
@@ -499,33 +527,6 @@ fn build_vmm(
     // Instantiate ACPI device manager.
     let acpi_device_manager = ACPIDeviceManager::new();
 
-    // For x86_64 we need to create the interrupt controller before calling `KVM_CREATE_VCPUS`
-    // while on aarch64 we need to do it the other way around.
-    #[cfg(target_arch = "x86_64")]
-    let pio_device_manager = {
-        // Serial device setup.
-        let serial_device =
-            setup_serial_device(std::io::stdin(), io::stdout()).map_err(Internal)?;
-
-        // x86_64 uses the i8042 reset event as the Vmm exit event.
-        let reset_evt = vcpus_exit_evt
-            .try_clone()
-            .map_err(VmmError::EventFd)
-            .map_err(Internal)?;
-
-        x86_64::setup_interrupt_controller(&mut vm).map_err(Internal)?;
-
-        // create pio dev manager with legacy devices
-        let pio_device_manager = {
-            // TODO Remove these unwraps.
-            let mut pio_dev_mgr = PortIODeviceManager::new(serial_device, reset_evt).unwrap();
-            pio_dev_mgr.register_devices(vm.fd()).unwrap();
-            pio_dev_mgr
-        };
-
-        pio_device_manager
-    };
-
     Ok(Vmm {
         events_observer: Some(std::io::stdin()),
         instance_info: instance_info.clone(),
@@ -538,7 +539,7 @@ fn build_vmm(
         resource_allocator,
         mmio_device_manager,
         #[cfg(target_arch = "x86_64")]
-        pio_device_manager,
+        pio_device_manager: None,
         acpi_device_manager,
     })
 }
@@ -590,7 +591,9 @@ pub fn build_microvm_for_boot(
         cpu_template.kvm_capabilities.clone(),
     )?;
     #[cfg(target_arch = "x86_64")]
-    event_manager.add_subscriber(vmm.pio_device_manager.stdio_serial.clone());
+    if let Some(pio_manager) = &vmm.pio_device_manager {
+        event_manager.add_subscriber(pio_manager.stdio_serial.clone());
+    }
 
     #[cfg(target_arch = "aarch64")]
     let (mut vmm, mut vcpus) = aarch64::create_vmm_and_vcpus(
@@ -801,7 +804,9 @@ pub fn build_microvm_from_snapshot(
         microvm_state.vm_state.kvm_cap_modifiers.clone(),
     )?;
     #[cfg(target_arch = "x86_64")]
-    event_manager.add_subscriber(vmm.pio_device_manager.stdio_serial.clone());
+    if let Some(pio_manager) = &vmm.pio_device_manager {
+        event_manager.add_subscriber(pio_manager.stdio_serial.clone());
+    }
     #[cfg(target_arch = "aarch64")]
     let (mut vmm, mut vcpus) = aarch64::create_vmm_and_vcpus(
         instance_info,
@@ -1281,7 +1286,7 @@ pub mod tests {
             resource_allocator: ResourceAllocator::new().unwrap(),
             mmio_device_manager,
             #[cfg(target_arch = "x86_64")]
-            pio_device_manager,
+            pio_device_manager: Some(pio_device_manager),
             acpi_device_manager,
         }
     }

src/vmm/src/device_manager/persist.rs

@@ -423,6 +423,9 @@ impl<'a> Persist<'a> for MMIODeviceManager {
             for state in &state.legacy_devices {
                 if state.type_ == DeviceType::Serial {
                     let serial = setup_serial_device(std::io::stdin(), std::io::stdout())?;
+                    constructor_args
+                        .event_manager
+                        .add_subscriber(serial.clone());
 
                     constructor_args
                         .resource_allocator

src/vmm/src/lib.rs

@@ -322,7 +322,7 @@ pub struct Vmm {
     // Guest VM devices.
     mmio_device_manager: MMIODeviceManager,
     #[cfg(target_arch = "x86_64")]
-    pio_device_manager: PortIODeviceManager,
+    pio_device_manager: Option<PortIODeviceManager>,
     acpi_device_manager: ACPIDeviceManager,
 }
 
@@ -387,8 +387,9 @@ impl Vmm {
         for mut vcpu in vcpus.drain(..) {
             vcpu.set_mmio_bus(self.mmio_device_manager.bus.clone());
             #[cfg(target_arch = "x86_64")]
-            vcpu.kvm_vcpu
-                .set_pio_bus(self.pio_device_manager.io_bus.clone());
+            if let Some(pio_device_manager) = &self.pio_device_manager {
+                vcpu.kvm_vcpu.set_pio_bus(pio_device_manager.io_bus.clone());
+            }
 
             self.vcpus_handles
                 .push(vcpu.start_threaded(vcpu_seccomp_filter.clone(), barrier.clone())?);
@@ -480,33 +481,39 @@ impl Vmm {
         }
 
         #[cfg(target_arch = "x86_64")]
-        {
-            let mut guard = self
-                .pio_device_manager
-                .stdio_serial
-                .lock()
-                .expect("Poisoned lock");
-            let serial = guard.serial_mut().unwrap();
-
-            serial
-                .serial
-                .write(IER_RDA_OFFSET, IER_RDA_BIT)
-                .map_err(|_| EmulateSerialInitError(std::io::Error::last_os_error()))?;
-            Ok(())
+        match &self.pio_device_manager {
+            Some(pio_manager) => {
+                let mut guard = pio_manager.stdio_serial.lock().expect("Poisoned lock");
+                let serial = guard.serial_mut().unwrap();
+
+                serial
+                    .serial
+                    .write(IER_RDA_OFFSET, IER_RDA_BIT)
+                    .map_err(|_| EmulateSerialInitError(std::io::Error::last_os_error()))?;
+                Ok(())
+            }
+            None => Err(EmulateSerialInitError(std::io::Error::new(
+                std::io::ErrorKind::NotFound,
+                "pio_device_manager is None",
+            ))),
         }
     }
 
     /// Injects CTRL+ALT+DEL keystroke combo in the i8042 device.
     #[cfg(target_arch = "x86_64")]
     pub fn send_ctrl_alt_del(&mut self) -> Result<(), VmmError> {
-        self.pio_device_manager
-            .i8042
-            .lock()
-            .expect("i8042 lock was poisoned")
-            .i8042_device_mut()
-            .unwrap()
-            .trigger_ctrl_alt_del()
-            .map_err(VmmError::I8042Error)
+        match &self.pio_device_manager {
+            Some(pio_device_manager) => pio_device_manager
+                .i8042
+                .lock()
+                .expect("i8042 lock was poisoned")
+                .i8042_device_mut()
+                .unwrap()
+                .trigger_ctrl_alt_del()
+                .map_err(VmmError::I8042Error),
+            // should be unreachable
+            None => panic!("pio_device_manager is None"),
+        }
     }
 
     /// Saves the state of a paused Microvm.