From b173f6f654f1f671b26e5bc2e050e2fda16a18a2 Mon Sep 17 00:00:00 2001 From: Michael Zhao Date: Tue, 29 Nov 2022 20:35:35 +0800 Subject: [PATCH] vmm,devices: Change Gic snapshot and restore path The snapshot and restore of AArch64 Gic was done in Vm. Now it is moved to DeviceManager. The benefit is that the restore can be done while the Gic is created in DeviceManager. While the moving of state data from Vm snapshot to DeviceManager snapshot breaks the compatability of migration from older versions. Signed-off-by: Michael Zhao --- devices/src/gic.rs | 32 +++++++------ devices/src/interrupt_controller.rs | 4 ++ vmm/src/device_manager.rs | 33 +++++++++++-- vmm/src/vm.rs | 73 ----------------------------- 4 files changed, 51 insertions(+), 91 deletions(-) diff --git a/devices/src/gic.rs b/devices/src/gic.rs index 80c8b6599..61eb8481b 100644 --- a/devices/src/gic.rs +++ b/devices/src/gic.rs @@ -8,7 +8,7 @@ use anyhow::anyhow; use arch::layout; use hypervisor::{ arch::aarch64::gic::{Vgic, VgicConfig}, - CpuState, + CpuState, GicState, }; use std::result; use std::sync::{Arc, Mutex}; @@ -25,6 +25,7 @@ type Result = result::Result; // Reserve 32 IRQs for legacy devices. pub const IRQ_LEGACY_BASE: usize = layout::IRQ_BASE as usize; pub const IRQ_LEGACY_COUNT: usize = 32; +pub const GIC_SNAPSHOT_ID: &str = "gic-v3-its"; // Gic (Generic Interupt Controller) struct provides all the functionality of a // GIC device. It wraps a hypervisor-emulated GIC device (Vgic) provided by the @@ -63,6 +64,21 @@ impl Gic { Ok(gic) } + pub fn restore_vgic( + &mut self, + state: Option, + saved_vcpu_states: &Vec, + ) -> Result<()> { + self.set_gicr_typers(saved_vcpu_states); + self.vgic + .clone() + .unwrap() + .lock() + .unwrap() + .set_state(&state.unwrap()) + .map_err(Error::RestoreGic) + } + fn enable(&self) -> Result<()> { // Set irqfd for legacy interrupts self.interrupt_source_group @@ -130,10 +146,9 @@ impl InterruptController for Gic { } } -pub const GIC_V3_ITS_SNAPSHOT_ID: &str = "gic-v3-its"; impl Snapshottable for Gic { fn id(&self) -> String { - GIC_V3_ITS_SNAPSHOT_ID.to_string() + GIC_SNAPSHOT_ID.to_string() } fn snapshot(&mut self) -> std::result::Result { @@ -141,17 +156,6 @@ impl Snapshottable for Gic { let state = vgic.lock().unwrap().state().unwrap(); Snapshot::new_from_state(&self.id(), &state) } - - fn restore(&mut self, snapshot: Snapshot) -> std::result::Result<(), MigratableError> { - let vgic = self.vgic.as_ref().unwrap().clone(); - vgic.lock() - .unwrap() - .set_state(&snapshot.to_state(&self.id())?) - .map_err(|e| { - MigratableError::Restore(anyhow!("Could not restore GICv3ITS state {:?}", e)) - })?; - Ok(()) - } } impl Pausable for Gic { diff --git a/devices/src/interrupt_controller.rs b/devices/src/interrupt_controller.rs index be83ea0d4..c92eadb1b 100644 --- a/devices/src/interrupt_controller.rs +++ b/devices/src/interrupt_controller.rs @@ -24,8 +24,12 @@ pub enum Error { UpdateInterrupt(io::Error), /// Failed enabling the interrupt. EnableInterrupt(io::Error), + #[cfg(target_arch = "aarch64")] /// Failed creating GIC device. CreateGic(hypervisor::HypervisorVmError), + #[cfg(target_arch = "aarch64")] + /// Failed restoring GIC device. + RestoreGic(hypervisor::arch::aarch64::gic::Error), } type Result = result::Result; diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index b56015974..ce7085978 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -1381,10 +1381,35 @@ impl DeviceManager { self.interrupt_controller = Some(interrupt_controller.clone()); - // Unlike x86_64, the "interrupt_controller" here for AArch64 is only - // a `Gic` object that implements the `InterruptController` to provide - // interrupt delivery service. This is not the real GIC device so that - // we do not need to insert it to the device tree. + // Restore the vGic if this is in the process of restoration + let id = String::from(gic::GIC_SNAPSHOT_ID); + if let Some(vgic_snapshot) = snapshot_from_id(self.snapshot.as_ref(), &id) { + // PMU support is optional. Nothing should be impacted if the PMU initialization failed. + if self + .cpu_manager + .lock() + .unwrap() + .init_pmu(arch::aarch64::fdt::AARCH64_PMU_IRQ + 16) + .is_err() + { + info!("Failed to initialize PMU"); + } + + let vgic_state = vgic_snapshot + .to_state(&id) + .map_err(DeviceManagerError::RestoreGetState)?; + let saved_vcpu_states = self.cpu_manager.lock().unwrap().get_saved_states(); + interrupt_controller + .lock() + .unwrap() + .restore_vgic(vgic_state, &saved_vcpu_states) + .unwrap(); + } + + self.device_tree + .lock() + .unwrap() + .insert(id.clone(), device_node!(id, interrupt_controller)); Ok(interrupt_controller) } diff --git a/vmm/src/vm.rs b/vmm/src/vm.rs index 1386be32b..8256ac2b9 100644 --- a/vmm/src/vm.rs +++ b/vmm/src/vm.rs @@ -49,8 +49,6 @@ use arch::EntryPoint; use arch::PciSpaceInfo; use arch::{NumaNode, NumaNodes}; #[cfg(target_arch = "aarch64")] -use devices::gic::GIC_V3_ITS_SNAPSHOT_ID; -#[cfg(target_arch = "aarch64")] use devices::interrupt_controller; use devices::AcpiNotificationFlags; #[cfg(all(target_arch = "aarch64", feature = "guest_debug"))] @@ -2135,70 +2133,6 @@ impl Vm { .map(|state| *state) } - #[cfg(target_arch = "aarch64")] - /// Add the vGIC section to the VM snapshot. - fn add_vgic_snapshot_section( - &self, - vm_snapshot: &mut Snapshot, - ) -> std::result::Result<(), MigratableError> { - vm_snapshot.add_snapshot( - self.device_manager - .lock() - .unwrap() - .get_interrupt_controller() - .unwrap() - .lock() - .unwrap() - .snapshot()?, - ); - - Ok(()) - } - - #[cfg(target_arch = "aarch64")] - /// Restore the vGIC from the VM snapshot and enable the interrupt controller routing. - fn restore_vgic_and_enable_interrupt( - &self, - vm_snapshot: &Snapshot, - ) -> std::result::Result<(), MigratableError> { - let saved_vcpu_states = self.cpu_manager.lock().unwrap().get_saved_states(); - - // PMU interrupt sticks to PPI, so need to be added by 16 to get real irq number. - self.cpu_manager - .lock() - .unwrap() - .init_pmu(arch::aarch64::fdt::AARCH64_PMU_IRQ + 16) - .map_err(|e| MigratableError::Restore(anyhow!("Error init PMU: {:?}", e)))?; - - // Here we prepare the GICR_TYPER registers from the restored vCPU states. - self.device_manager - .lock() - .unwrap() - .get_interrupt_controller() - .unwrap() - .lock() - .unwrap() - .set_gicr_typers(&saved_vcpu_states); - - // Restore GIC states. - if let Some(gicv3_its_snapshot) = vm_snapshot.snapshots.get(GIC_V3_ITS_SNAPSHOT_ID) { - self.device_manager - .lock() - .unwrap() - .get_interrupt_controller() - .unwrap() - .lock() - .unwrap() - .restore(*gicv3_its_snapshot.clone())?; - } else { - return Err(MigratableError::Restore(anyhow!( - "Missing GicV3Its snapshot" - ))); - } - - Ok(()) - } - /// Gets the actual size of the balloon. pub fn balloon_size(&self) -> u64 { self.device_manager.lock().unwrap().balloon_size() @@ -2550,10 +2484,6 @@ impl Snapshottable for Vm { vm_snapshot.add_snapshot(self.cpu_manager.lock().unwrap().snapshot()?); vm_snapshot.add_snapshot(self.memory_manager.lock().unwrap().snapshot()?); - #[cfg(target_arch = "aarch64")] - self.add_vgic_snapshot_section(&mut vm_snapshot) - .map_err(|e| MigratableError::Snapshot(e.into()))?; - vm_snapshot.add_snapshot(self.device_manager.lock().unwrap().snapshot()?); vm_snapshot.add_data_section(SnapshotDataSection { id: format!("{}-section", VM_SNAPSHOT_ID), @@ -2575,9 +2505,6 @@ impl Snapshottable for Vm { MigratableError::Restore(anyhow!("Could not restore VM state: {:#?}", e)) })?; - #[cfg(target_arch = "aarch64")] - self.restore_vgic_and_enable_interrupt(&_snapshot)?; - // Now we can start all vCPUs from here. self.cpu_manager .lock()