vmm: Refactor AArch64 GIC initialization process

In the new process, `device::Gic::new()` covers additional actions:
1. Creating `hypervisor::vGic`
2. Initializing interrupt routings

The change makes the vGic device ready in the beginning of
`DeviceManager::create_devices()`. This can unblock the GIC related
devices initialization in the `DeviceManager`.

Signed-off-by: Michael Zhao <michael.zhao@arm.com>

devices/src/gic.rs

@@ -39,8 +39,9 @@ pub struct Gic {
 
 impl Gic {
     pub fn new(
-        _vcpu_count: u8,
+        vcpu_count: u8,
         interrupt_manager: Arc<dyn InterruptManager<GroupConfig = MsiIrqGroupConfig>>,
+        vm: Arc<dyn hypervisor::Vm>,
     ) -> Result<Gic> {
         let interrupt_source_group = interrupt_manager
             .create_group(MsiIrqGroupConfig {
@@ -49,45 +50,19 @@ impl Gic {
             })
             .map_err(Error::CreateInterruptSourceGroup)?;
 
-        Ok(Gic {
+        let vgic = vm
+            .create_vgic(Gic::create_default_config(vcpu_count as u64))
+            .map_err(Error::CreateGic)?;
+
+        let gic = Gic {
             interrupt_source_group,
-            vgic: None,
+            vgic: Some(vgic),
-        })
+        };
+        gic.enable()?;
+
+        Ok(gic)
     }
 
-    /// Default config implied by arch::layout
-    pub fn create_default_config(vcpu_count: u64) -> VgicConfig {
-        let redists_size = layout::GIC_V3_REDIST_SIZE * vcpu_count;
-        let redists_addr = layout::GIC_V3_DIST_START.raw_value() - redists_size;
-        VgicConfig {
-            vcpu_count,
-            dist_addr: layout::GIC_V3_DIST_START.raw_value(),
-            dist_size: layout::GIC_V3_DIST_SIZE,
-            redists_addr,
-            redists_size,
-            msi_addr: redists_addr - layout::GIC_V3_ITS_SIZE,
-            msi_size: layout::GIC_V3_ITS_SIZE,
-            nr_irqs: layout::IRQ_NUM,
-        }
-    }
-
-    pub fn create_vgic(
-        &mut self,
-        vm: &Arc<dyn hypervisor::Vm>,
-        config: VgicConfig,
-    ) -> Result<Arc<Mutex<dyn Vgic>>> {
-        let vgic = vm.create_vgic(config).map_err(Error::CreateGic)?;
-        self.vgic = Some(vgic.clone());
-        Ok(vgic.clone())
-    }
-
-    pub fn set_gicr_typers(&mut self, vcpu_states: &[CpuState]) {
-        let vgic = self.vgic.as_ref().unwrap().clone();
-        vgic.lock().unwrap().set_gicr_typers(vcpu_states);
-    }
-}
-
-impl InterruptController for Gic {
     fn enable(&self) -> Result<()> {
         // Set irqfd for legacy interrupts
         self.interrupt_source_group
@@ -113,6 +88,33 @@ impl InterruptController for Gic {
         Ok(())
     }
 
+    /// Default config implied by arch::layout
+    pub fn create_default_config(vcpu_count: u64) -> VgicConfig {
+        let redists_size = layout::GIC_V3_REDIST_SIZE * vcpu_count;
+        let redists_addr = layout::GIC_V3_DIST_START.raw_value() - redists_size;
+        VgicConfig {
+            vcpu_count,
+            dist_addr: layout::GIC_V3_DIST_START.raw_value(),
+            dist_size: layout::GIC_V3_DIST_SIZE,
+            redists_addr,
+            redists_size,
+            msi_addr: redists_addr - layout::GIC_V3_ITS_SIZE,
+            msi_size: layout::GIC_V3_ITS_SIZE,
+            nr_irqs: layout::IRQ_NUM,
+        }
+    }
+
+    pub fn get_vgic(&mut self) -> Result<Arc<Mutex<dyn Vgic>>> {
+        Ok(self.vgic.clone().unwrap())
+    }
+
+    pub fn set_gicr_typers(&mut self, vcpu_states: &[CpuState]) {
+        let vgic = self.vgic.as_ref().unwrap().clone();
+        vgic.lock().unwrap().set_gicr_typers(vcpu_states);
+    }
+}
+
+impl InterruptController for Gic {
     // This should be called anytime an interrupt needs to be injected into the
     // running guest.
     fn service_irq(&mut self, irq: usize) -> Result<()> {

devices/src/interrupt_controller.rs

@@ -55,8 +55,6 @@ pub struct MsiMessage {
 // IOAPIC (X86) or GIC (Arm).
 pub trait InterruptController: Send {
     fn service_irq(&mut self, irq: usize) -> Result<()>;
-    #[cfg(target_arch = "aarch64")]
-    fn enable(&self) -> Result<()>;
     #[cfg(target_arch = "x86_64")]
     fn end_of_interrupt(&mut self, vec: u8);
     fn notifier(&self, irq: usize) -> Option<EventFd>;

vmm/src/device_manager.rs

@@ -549,7 +549,7 @@ pub(crate) struct AddressManager {
     #[cfg(target_arch = "x86_64")]
     pub(crate) io_bus: Arc<Bus>,
     pub(crate) mmio_bus: Arc<Bus>,
-    vm: Arc<dyn hypervisor::Vm>,
+    pub(crate) vm: Arc<dyn hypervisor::Vm>,
     device_tree: Arc<Mutex<DeviceTree>>,
     pci_mmio_allocators: Vec<Arc<Mutex<AddressAllocator>>>,
 }
@@ -1376,6 +1376,7 @@ impl DeviceManager {
             gic::Gic::new(
                 self.config.lock().unwrap().cpus.boot_vcpus,
                 Arc::clone(&self.msi_interrupt_manager),
+                self.address_manager.vm.clone(),
             )
             .map_err(DeviceManagerError::CreateInterruptController)?,
         ));

vmm/src/vm.rs

@@ -47,9 +47,9 @@ use arch::EntryPoint;
 use arch::PciSpaceInfo;
 use arch::{NumaNode, NumaNodes};
 #[cfg(target_arch = "aarch64")]
-use devices::gic::{Gic, GIC_V3_ITS_SNAPSHOT_ID};
+use devices::gic::GIC_V3_ITS_SNAPSHOT_ID;
 #[cfg(target_arch = "aarch64")]
-use devices::interrupt_controller::{self, InterruptController};
+use devices::interrupt_controller;
 use devices::AcpiNotificationFlags;
 #[cfg(all(target_arch = "aarch64", feature = "guest_debug"))]
 use gdbstub_arch::aarch64::reg::AArch64CoreRegs as CoreRegs;
@@ -445,7 +445,7 @@ pub struct Vm {
     state: RwLock<VmState>,
     cpu_manager: Arc<Mutex<cpu::CpuManager>>,
     memory_manager: Arc<Mutex<MemoryManager>>,
-    #[cfg_attr(not(feature = "kvm"), allow(dead_code))]
+    #[cfg_attr(any(not(feature = "kvm"), target_arch = "aarch64"), allow(dead_code))]
     // The hypervisor abstracted virtual machine.
     vm: Arc<dyn hypervisor::Vm>,
     #[cfg(all(feature = "kvm", target_arch = "x86_64"))]
@@ -1156,7 +1156,6 @@ impl Vm {
             .as_ref()
             .map(|(v, _)| *v);
 
-        let vcpu_count = self.cpu_manager.lock().unwrap().boot_vcpus() as u64;
         let vgic = self
             .device_manager
             .lock()
@@ -1165,10 +1164,7 @@ impl Vm {
             .unwrap()
             .lock()
             .unwrap()
-            .create_vgic(
+            .get_vgic()
-                &self.memory_manager.lock().as_ref().unwrap().vm,
-                Gic::create_default_config(vcpu_count),
-            )
             .map_err(|_| {
                 Error::ConfigureSystem(arch::Error::PlatformSpecific(
                     arch::aarch64::Error::SetupGic,
@@ -1202,17 +1198,6 @@ impl Vm {
         )
         .map_err(Error::ConfigureSystem)?;
 
-        // Activate gic device
-        self.device_manager
-            .lock()
-            .unwrap()
-            .get_interrupt_controller()
-            .unwrap()
-            .lock()
-            .unwrap()
-            .enable()
-            .map_err(Error::EnableInterruptController)?;
-
         Ok(())
     }
 
@@ -2224,22 +2209,6 @@ impl Vm {
         vm_snapshot: &Snapshot,
     ) -> std::result::Result<(), MigratableError> {
         let saved_vcpu_states = self.cpu_manager.lock().unwrap().get_saved_states();
-        // The number of vCPUs is the same as the number of saved vCPU states.
-        let vcpu_numbers = saved_vcpu_states.len();
-
-        // Creating a GIC device here, as the GIC will not be created when
-        // restoring the device manager. Note that currently only the bare GICv3
-        // without ITS is supported.
-        let vcpu_count = vcpu_numbers.try_into().unwrap();
-        self.device_manager
-            .lock()
-            .unwrap()
-            .get_interrupt_controller()
-            .unwrap()
-            .lock()
-            .unwrap()
-            .create_vgic(&self.vm, Gic::create_default_config(vcpu_count))
-            .map_err(|e| MigratableError::Restore(anyhow!("Could not create GIC: {:#?}", e)))?;
 
         // PMU interrupt sticks to PPI, so need to be added by 16 to get real irq number.
         self.cpu_manager
@@ -2274,22 +2243,6 @@ impl Vm {
             )));
         }
 
-        // Activate gic device
-        self.device_manager
-            .lock()
-            .unwrap()
-            .get_interrupt_controller()
-            .unwrap()
-            .lock()
-            .unwrap()
-            .enable()
-            .map_err(|e| {
-                MigratableError::Restore(anyhow!(
-                    "Could not enable interrupt controller routing: {:#?}",
-                    e
-                ))
-            })?;
-
         Ok(())
     }
 
@@ -3241,6 +3194,7 @@ mod tests {
     use arch::aarch64::fdt::create_fdt;
     use arch::aarch64::layout;
     use arch::{DeviceType, MmioDeviceInfo};
+    use devices::gic::Gic;
 
     const LEN: u64 = 4096;