From 6bb33601d08c102ee4227099856b9bc01e947d0e Mon Sep 17 00:00:00 2001 From: Jinank Jain Date: Thu, 27 Mar 2025 13:52:26 +0000 Subject: [PATCH] hypervisor: Avoid leaking KVM GIC state into common GIC state KVM supports GICv3-ITS emulation and the current GicState is modelled around the KVM implementation. We should refactor this to accomodate other hypervisor requirements. For example, MSHV only support GICv2M emulation for guests for delivering MSI interrupts instead of GICv3-ITS. Signed-off-by: Jinank Jain --- Cargo.lock | 1 + devices/src/gic.rs | 4 +-- hypervisor/Cargo.toml | 1 + hypervisor/src/arch/aarch64/gic.rs | 49 ++++++++++++++++++++++++++- hypervisor/src/kvm/aarch64/gic/mod.rs | 49 +++++++++++++++++++-------- hypervisor/src/kvm/mod.rs | 4 +-- hypervisor/src/lib.rs | 2 +- 7 files changed, 88 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 825b1ed8b..eb3e01ca4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -984,6 +984,7 @@ dependencies = [ "mshv-bindings", "mshv-ioctls", "serde", + "serde_json", "serde_with", "thiserror 2.0.6", "vfio-ioctls", diff --git a/devices/src/gic.rs b/devices/src/gic.rs index d6ea1a12d..afa5814a1 100644 --- a/devices/src/gic.rs +++ b/devices/src/gic.rs @@ -9,8 +9,8 @@ use std::sync::{Arc, Mutex}; use anyhow::anyhow; use arch::layout; -use hypervisor::arch::aarch64::gic::{Vgic, VgicConfig}; -use hypervisor::{CpuState, GicState}; +use hypervisor::arch::aarch64::gic::{GicState, Vgic, VgicConfig}; +use hypervisor::CpuState; use vm_device::interrupt::{ InterruptIndex, InterruptManager, InterruptSourceConfig, InterruptSourceGroup, LegacyIrqSourceConfig, MsiIrqGroupConfig, diff --git a/hypervisor/Cargo.toml b/hypervisor/Cargo.toml index 76caab56c..05fecff96 100644 --- a/hypervisor/Cargo.toml +++ b/hypervisor/Cargo.toml @@ -30,6 +30,7 @@ mshv-bindings = { workspace = true, features = [ ], optional = true } mshv-ioctls = { workspace = true, optional = true } serde = { version = "1.0.208", features = ["derive", "rc"] } +serde_json = "1.0.120" serde_with = { version = "3.9.0", default-features = false, features = [ "macros", ] } diff --git a/hypervisor/src/arch/aarch64/gic.rs b/hypervisor/src/arch/aarch64/gic.rs index 95330d2be..20aad9807 100644 --- a/hypervisor/src/arch/aarch64/gic.rs +++ b/hypervisor/src/arch/aarch64/gic.rs @@ -5,9 +5,12 @@ use std::any::Any; use std::result; +use serde::de::Error as SerdeError; +use serde::{Deserialize, Serialize}; +use serde_json; use thiserror::Error; -use crate::{CpuState, GicState, HypervisorDeviceError, HypervisorVmError}; +use crate::{CpuState, HypervisorDeviceError, HypervisorVmError}; /// Errors thrown while setting up the VGIC. #[derive(Debug, Error)] @@ -36,6 +39,50 @@ pub struct VgicConfig { pub nr_irqs: u32, } +#[derive(Clone, Serialize)] +pub enum GicState { + #[cfg(feature = "kvm")] + Kvm(crate::kvm::aarch64::gic::Gicv3ItsState), +} + +impl<'de> Deserialize<'de> for GicState { + fn deserialize(deserializer: D) -> std::result::Result + where + D: serde::Deserializer<'de>, + { + // GicStateDefaultDeserialize is a helper enum that mirrors GicState but also derives the Deserialize trait. + // This enables backward-compatible deserialization of GicState, facilitating live-upgrade scenarios. + #[derive(Deserialize)] + pub enum GicStateDefaultDeserialize { + #[cfg(feature = "kvm")] + Kvm(crate::kvm::aarch64::gic::Gicv3ItsState), + } + + const { + assert!( + std::mem::size_of::() + == std::mem::size_of::() + ) + }; + + let value: serde_json::Value = Deserialize::deserialize(deserializer)?; + + #[cfg(feature = "kvm")] + if let Ok(gicv3_its_state) = + crate::kvm::aarch64::gic::Gicv3ItsState::deserialize(value.clone()) + { + return Ok(GicState::Kvm(gicv3_its_state)); + } + + if let Ok(gic_state_de) = GicStateDefaultDeserialize::deserialize(value.clone()) { + return match gic_state_de { + #[cfg(feature = "kvm")] + GicStateDefaultDeserialize::Kvm(state) => Ok(GicState::Kvm(state)), + }; + } + Err(SerdeError::custom("Failed to deserialize GicState")) + } +} /// Hypervisor agnostic interface for a virtualized GIC pub trait Vgic: Send + Sync { /// Returns the fdt compatibility property of the device diff --git a/hypervisor/src/kvm/aarch64/gic/mod.rs b/hypervisor/src/kvm/aarch64/gic/mod.rs index 9b8b8668e..7df2b50b4 100644 --- a/hypervisor/src/kvm/aarch64/gic/mod.rs +++ b/hypervisor/src/kvm/aarch64/gic/mod.rs @@ -14,7 +14,7 @@ use kvm_ioctls::DeviceFd; use redist_regs::{construct_gicr_typers, get_redist_regs, set_redist_regs}; use serde::{Deserialize, Serialize}; -use crate::arch::aarch64::gic::{Error, Result, Vgic, VgicConfig}; +use crate::arch::aarch64::gic::{Error, GicState, Result, Vgic, VgicConfig}; use crate::device::HypervisorDeviceError; use crate::kvm::KvmVm; use crate::{CpuState, Vm}; @@ -125,6 +125,20 @@ pub struct Gicv3ItsState { its_baser: [u64; 8], } +impl From for Gicv3ItsState { + fn from(state: GicState) -> Self { + match state { + GicState::Kvm(state) => state, + } + } +} + +impl From for GicState { + fn from(state: Gicv3ItsState) -> Self { + GicState::Kvm(state) + } +} + impl KvmGicV3Its { /// Device trees specific constants pub const ARCH_GIC_V3_MAINT_IRQ: u32 = 9; @@ -316,7 +330,7 @@ impl Vgic for KvmGicV3Its { } /// Save the state of GICv3ITS. - fn state(&self) -> Result { + fn state(&self) -> Result { let gicr_typers = self.gicr_typers.clone(); let gicd_ctlr = read_ctlr(&self.device)?; @@ -366,7 +380,7 @@ impl Vgic for KvmGicV3Its { GITS_IIDR, )?; - Ok(Gicv3ItsState { + let gic_state: GicState = Gicv3ItsState { dist: dist_state, rdist: rdist_state, icc: icc_state, @@ -377,48 +391,53 @@ impl Vgic for KvmGicV3Its { its_cwriter: its_cwriter_state, its_creadr: its_creadr_state, its_baser: its_baser_state, - }) + } + .into(); + + Ok(gic_state) } /// Restore the state of GICv3ITS. - fn set_state(&mut self, state: &Gicv3ItsState) -> Result<()> { + fn set_state(&mut self, state: &GicState) -> Result<()> { + let kvm_state: Gicv3ItsState = state.clone().into(); + let gicr_typers = self.gicr_typers.clone(); - write_ctlr(&self.device, state.gicd_ctlr)?; + write_ctlr(&self.device, kvm_state.gicd_ctlr)?; - set_dist_regs(&self.device, &state.dist)?; + set_dist_regs(&self.device, &kvm_state.dist)?; - set_redist_regs(&self.device, &gicr_typers, &state.rdist)?; + set_redist_regs(&self.device, &gicr_typers, &kvm_state.rdist)?; - set_icc_regs(&self.device, &gicr_typers, &state.icc)?; + set_icc_regs(&self.device, &gicr_typers, &kvm_state.icc)?; //Restore GICv3ITS registers gicv3_its_attr_set( self.its_device.as_ref().unwrap(), kvm_bindings::KVM_DEV_ARM_VGIC_GRP_ITS_REGS, GITS_IIDR, - state.its_iidr, + kvm_state.its_iidr, )?; gicv3_its_attr_set( self.its_device.as_ref().unwrap(), kvm_bindings::KVM_DEV_ARM_VGIC_GRP_ITS_REGS, GITS_CBASER, - state.its_cbaser, + kvm_state.its_cbaser, )?; gicv3_its_attr_set( self.its_device.as_ref().unwrap(), kvm_bindings::KVM_DEV_ARM_VGIC_GRP_ITS_REGS, GITS_CREADR, - state.its_creadr, + kvm_state.its_creadr, )?; gicv3_its_attr_set( self.its_device.as_ref().unwrap(), kvm_bindings::KVM_DEV_ARM_VGIC_GRP_ITS_REGS, GITS_CWRITER, - state.its_cwriter, + kvm_state.its_cwriter, )?; for i in 0..8 { @@ -426,7 +445,7 @@ impl Vgic for KvmGicV3Its { self.its_device.as_ref().unwrap(), kvm_bindings::KVM_DEV_ARM_VGIC_GRP_ITS_REGS, GITS_BASER + i * 8, - state.its_baser[i as usize], + kvm_state.its_baser[i as usize], )?; } @@ -437,7 +456,7 @@ impl Vgic for KvmGicV3Its { self.its_device.as_ref().unwrap(), kvm_bindings::KVM_DEV_ARM_VGIC_GRP_ITS_REGS, GITS_CTLR, - state.its_ctlr, + kvm_state.its_ctlr, ) } diff --git a/hypervisor/src/kvm/mod.rs b/hypervisor/src/kvm/mod.rs index c7ba6bbb2..750f9671c 100644 --- a/hypervisor/src/kvm/mod.rs +++ b/hypervisor/src/kvm/mod.rs @@ -29,9 +29,7 @@ use vmm_sys_util::eventfd::EventFd; #[cfg(target_arch = "aarch64")] use crate::aarch64::gic::KvmGicV3Its; #[cfg(target_arch = "aarch64")] -pub use crate::aarch64::{ - check_required_kvm_extensions, gic::Gicv3ItsState as GicState, is_system_register, VcpuKvmState, -}; +pub use crate::aarch64::{check_required_kvm_extensions, is_system_register, VcpuKvmState}; #[cfg(target_arch = "aarch64")] use crate::arch::aarch64::gic::{Vgic, VgicConfig}; #[cfg(target_arch = "riscv64")] diff --git a/hypervisor/src/lib.rs b/hypervisor/src/lib.rs index 4ae288b42..6600fefcc 100644 --- a/hypervisor/src/lib.rs +++ b/hypervisor/src/lib.rs @@ -59,7 +59,7 @@ pub use cpu::CpuVendor; pub use cpu::{HypervisorCpuError, Vcpu, VmExit}; pub use device::HypervisorDeviceError; #[cfg(all(feature = "kvm", target_arch = "aarch64"))] -pub use kvm::{aarch64, GicState}; +pub use kvm::aarch64; #[cfg(all(feature = "kvm", target_arch = "riscv64"))] pub use kvm::{riscv64, AiaState}; pub use vm::{