From 784a3aaf3c417b5a171075498f0d242c73fa23a1 Mon Sep 17 00:00:00 2001 From: Nuno Das Neves Date: Tue, 30 Aug 2022 00:49:57 +0000 Subject: [PATCH] devices: gic: use VgicConfig everywhere Use VgicConfig to initialize Vgic. Use Gic::create_default_config everywhere so we don't always recompute redist/msi registers. Add a helper create_test_vgic_config for tests in hypervisor crate. Signed-off-by: Nuno Das Neves --- devices/src/gic.rs | 13 +-- hypervisor/src/kvm/aarch64/gic/mod.rs | 110 ++++++++------------------ hypervisor/src/kvm/mod.rs | 24 +----- hypervisor/src/vm.rs | 12 +-- vmm/src/cpu.rs | 14 ++-- vmm/src/device_manager.rs | 10 +-- vmm/src/vm.rs | 17 ++-- 7 files changed, 60 insertions(+), 140 deletions(-) diff --git a/devices/src/gic.rs b/devices/src/gic.rs index 94693b35b..e04d6a0ca 100644 --- a/devices/src/gic.rs +++ b/devices/src/gic.rs @@ -74,18 +74,9 @@ impl Gic { pub fn create_vgic( &mut self, vm: &Arc, - vcpu_count: u64, + config: VgicConfig, ) -> Result>> { - let vgic = vm - .create_vgic( - vcpu_count, - layout::GIC_V3_DIST_START.raw_value(), - layout::GIC_V3_DIST_SIZE, - layout::GIC_V3_REDIST_SIZE, - layout::GIC_V3_ITS_SIZE, - layout::IRQ_NUM, - ) - .map_err(Error::CreateGic)?; + let vgic = vm.create_vgic(config).map_err(Error::CreateGic)?; self.vgic = Some(vgic.clone()); Ok(vgic.clone()) } diff --git a/hypervisor/src/kvm/aarch64/gic/mod.rs b/hypervisor/src/kvm/aarch64/gic/mod.rs index 3e7e3bd45..d90b23378 100644 --- a/hypervisor/src/kvm/aarch64/gic/mod.rs +++ b/hypervisor/src/kvm/aarch64/gic/mod.rs @@ -4,7 +4,7 @@ mod dist_regs; mod icc_regs; mod redist_regs; -use crate::arch::aarch64::gic::{Error, Result, Vgic}; +use crate::arch::aarch64::gic::{Error, Result, Vgic, VgicConfig}; use crate::device::HypervisorDeviceError; use crate::kvm::{kvm_bindings, KvmVm}; use crate::{CpuState, Vm}; @@ -133,9 +133,7 @@ impl KvmGicV3Its { /// Setup the device-specific attributes fn init_device_attributes(&mut self, vm: &KvmVm, nr_irqs: u32) -> Result<()> { // GicV3 part attributes - /* Setting up the distributor attribute. - We are placing the GIC below 1GB so we need to substract the size of the distributor. - */ + /* Setting up the distributor attribute. */ Self::set_device_attribute( &self.device, kvm_bindings::KVM_DEV_ARM_VGIC_GRP_ADDR, @@ -144,9 +142,7 @@ impl KvmGicV3Its { 0, )?; - /* Setting up the redistributors' attribute. - We are calculating here the start of the redistributors address. We have one per CPU. - */ + /* Setting up the redistributors' attribute. */ Self::set_device_attribute( &self.device, kvm_bindings::KVM_DEV_ARM_VGIC_GRP_ADDR, @@ -248,37 +244,26 @@ impl KvmGicV3Its { /// Method to initialize the GIC device #[allow(clippy::new_ret_no_self)] - pub fn new( - vm: &dyn Vm, - vcpu_count: u64, - dist_addr: u64, - dist_size: u64, - redist_size: u64, - msi_size: u64, - nr_irqs: u32, - ) -> Result { + pub fn new(vm: &dyn Vm, config: VgicConfig) -> Result { // This is inside KVM module let vm = vm.as_any().downcast_ref::().expect("Wrong VM type?"); let vgic = Self::create_device(vm)?; - let redists_size: u64 = redist_size * vcpu_count; - let redists_addr: u64 = dist_addr - redists_size; - let msi_addr: u64 = redists_addr - msi_size; let mut gic_device = KvmGicV3Its { device: vgic, its_device: None, - gicr_typers: vec![0; vcpu_count.try_into().unwrap()], - dist_addr, - dist_size, - redists_addr, - redists_size, - msi_addr, - msi_size, - vcpu_count, + gicr_typers: vec![0; config.vcpu_count.try_into().unwrap()], + dist_addr: config.dist_addr, + dist_size: config.dist_size, + redists_addr: config.redists_addr, + redists_size: config.redists_size, + msi_addr: config.msi_addr, + msi_size: config.msi_size, + vcpu_count: config.vcpu_count, }; - gic_device.init_device_attributes(vm, nr_irqs)?; + gic_device.init_device_attributes(vm, config.nr_irqs)?; Ok(gic_device) } @@ -500,23 +485,30 @@ mod tests { use crate::aarch64::gic::{ get_dist_regs, get_icc_regs, get_redist_regs, set_dist_regs, set_icc_regs, set_redist_regs, }; + use crate::arch::aarch64::gic::VgicConfig; use crate::kvm::KvmGicV3Its; + fn create_test_vgic_config() -> VgicConfig { + VgicConfig { + vcpu_count: 1, + dist_addr: 0x0900_0000 - 0x01_0000, + dist_size: 0x01_0000, + // dist_addr - redists_size + redists_addr: 0x0900_0000 - 0x01_0000 - 0x02_0000, + redists_size: 0x02_0000, + // redists_addr - msi_size + msi_addr: 0x0900_0000 - 0x01_0000 - 0x02_0000 - 0x02_0000, + msi_size: 0x02_0000, + nr_irqs: 256, + } + } + #[test] fn test_create_gic() { let hv = crate::new().unwrap(); let vm = hv.create_vm().unwrap(); - assert!(KvmGicV3Its::new( - &*vm, - 1, - 0x0900_0000 - 0x01_0000, - 0x01_0000, - 0x02_0000, - 0x02_0000, - 256 - ) - .is_ok()); + assert!(KvmGicV3Its::new(&*vm, create_test_vgic_config()).is_ok()); } #[test] @@ -524,16 +516,7 @@ mod tests { let hv = crate::new().unwrap(); let vm = hv.create_vm().unwrap(); let _ = vm.create_vcpu(0, None).unwrap(); - let gic = KvmGicV3Its::new( - &*vm, - 1, - 0x0900_0000 - 0x01_0000, - 0x01_0000, - 0x02_0000, - 0x02_0000, - 256, - ) - .expect("Cannot create gic"); + let gic = KvmGicV3Its::new(&*vm, create_test_vgic_config()).expect("Cannot create gic"); let res = get_dist_regs(&gic.device); assert!(res.is_ok()); @@ -549,16 +532,7 @@ mod tests { let hv = crate::new().unwrap(); let vm = hv.create_vm().unwrap(); let _ = vm.create_vcpu(0, None).unwrap(); - let gic = KvmGicV3Its::new( - &*vm, - 1, - 0x0900_0000 - 0x01_0000, - 0x01_0000, - 0x02_0000, - 0x02_0000, - 256, - ) - .expect("Cannot create gic"); + let gic = KvmGicV3Its::new(&*vm, create_test_vgic_config()).expect("Cannot create gic"); let gicr_typer = vec![123]; let res = get_redist_regs(&gic.device, &gicr_typer); @@ -575,16 +549,7 @@ mod tests { let hv = crate::new().unwrap(); let vm = hv.create_vm().unwrap(); let _ = vm.create_vcpu(0, None).unwrap(); - let gic = KvmGicV3Its::new( - &*vm, - 1, - 0x0900_0000 - 0x01_0000, - 0x01_0000, - 0x02_0000, - 0x02_0000, - 256, - ) - .expect("Cannot create gic"); + let gic = KvmGicV3Its::new(&*vm, create_test_vgic_config()).expect("Cannot create gic"); let gicr_typer = vec![123]; let res = get_icc_regs(&gic.device, &gicr_typer); @@ -602,14 +567,7 @@ mod tests { let vm = hv.create_vm().unwrap(); let _ = vm.create_vcpu(0, None).unwrap(); let gic = vm - .create_vgic( - 1, - 0x0900_0000 - 0x01_0000, - 0x01_0000, - 0x02_0000, - 0x02_0000, - 256, - ) + .create_vgic(create_test_vgic_config()) .expect("Cannot create gic"); assert!(gic.lock().unwrap().save_data_tables().is_ok()); diff --git a/hypervisor/src/kvm/mod.rs b/hypervisor/src/kvm/mod.rs index 862906daf..ea9ab6a99 100644 --- a/hypervisor/src/kvm/mod.rs +++ b/hypervisor/src/kvm/mod.rs @@ -16,7 +16,7 @@ pub use crate::aarch64::{ VcpuKvmState, }; #[cfg(target_arch = "aarch64")] -use crate::arch::aarch64::gic::Vgic; +use crate::arch::aarch64::gic::{Vgic, VgicConfig}; use crate::cpu; use crate::hypervisor; use crate::vec_with_array_field; @@ -414,25 +414,9 @@ impl vm::Vm for KvmVm { /// /// Creates a virtual GIC device. /// - fn create_vgic( - &self, - vcpu_count: u64, - dist_addr: u64, - dist_size: u64, - redist_size: u64, - msi_size: u64, - nr_irqs: u32, - ) -> vm::Result>> { - let gic_device = KvmGicV3Its::new( - self, - vcpu_count, - dist_addr, - dist_size, - redist_size, - msi_size, - nr_irqs, - ) - .map_err(|e| vm::HypervisorVmError::CreateVgic(anyhow!("Vgic error {:?}", e)))?; + fn create_vgic(&self, config: VgicConfig) -> vm::Result>> { + let gic_device = KvmGicV3Its::new(self, config) + .map_err(|e| vm::HypervisorVmError::CreateVgic(anyhow!("Vgic error {:?}", e)))?; Ok(Arc::new(Mutex::new(gic_device))) } /// diff --git a/hypervisor/src/vm.rs b/hypervisor/src/vm.rs index db1e6bf73..6361844d6 100644 --- a/hypervisor/src/vm.rs +++ b/hypervisor/src/vm.rs @@ -11,7 +11,7 @@ #[cfg(target_arch = "aarch64")] use crate::aarch64::VcpuInit; #[cfg(target_arch = "aarch64")] -use crate::arch::aarch64::gic::Vgic; +use crate::arch::aarch64::gic::{Vgic, VgicConfig}; #[cfg(feature = "tdx")] use crate::arch::x86::CpuIdEntry; use crate::cpu::Vcpu; @@ -273,15 +273,7 @@ pub trait Vm: Send + Sync + Any { /// Creates a new KVM vCPU file descriptor and maps the memory corresponding fn create_vcpu(&self, id: u8, vm_ops: Option>) -> Result>; #[cfg(target_arch = "aarch64")] - fn create_vgic( - &self, - vcpu_count: u64, - dist_addr: u64, - dist_size: u64, - redist_size: u64, - msi_size: u64, - nr_irqs: u32, - ) -> Result>>; + fn create_vgic(&self, config: VgicConfig) -> Result>>; /// Registers an event to be signaled whenever a certain address is written to. fn register_ioevent( diff --git a/vmm/src/cpu.rs b/vmm/src/cpu.rs index 32e3ecabf..36b9dcebf 100644 --- a/vmm/src/cpu.rs +++ b/vmm/src/cpu.rs @@ -33,6 +33,8 @@ use anyhow::anyhow; use arch::aarch64::regs; use arch::EntryPoint; use arch::NumaNodes; +#[cfg(target_arch = "aarch64")] +use devices::gic::Gic; use devices::interrupt_controller::InterruptController; #[cfg(all(target_arch = "aarch64", feature = "gdb"))] use gdbstub_arch::aarch64::reg::AArch64CoreRegs as CoreRegs; @@ -1333,6 +1335,7 @@ impl CpuManager { madt.append(gicc); } + let vgic_config = Gic::create_default_config(self.config.boot_vcpus.into()); // GIC Distributor structure. See section 5.2.12.15 in ACPI spec. let gicd = GicD { @@ -1340,7 +1343,7 @@ impl CpuManager { length: 24, reserved0: 0, gic_id: 0, - base_address: arch::layout::GIC_V3_DIST_START.0, + base_address: vgic_config.dist_addr, global_irq_base: 0, version: 3, reserved1: [0; 3], @@ -1348,15 +1351,12 @@ impl CpuManager { madt.append(gicd); // See 5.2.12.17 GIC Redistributor (GICR) Structure in ACPI spec. - let gicr_size: u32 = - (arch::layout::GIC_V3_REDIST_SIZE * self.config.boot_vcpus as u64) as u32; - let gicr_base: u64 = arch::layout::GIC_V3_DIST_START.0 - gicr_size as u64; let gicr = GicR { r#type: acpi::ACPI_APIC_GENERIC_REDISTRIBUTOR, length: 16, reserved: 0, - base_address: gicr_base, - range_length: gicr_size, + base_address: vgic_config.redists_addr, + range_length: vgic_config.redists_size as u32, }; madt.append(gicr); @@ -1366,7 +1366,7 @@ impl CpuManager { length: 20, reserved0: 0, translation_id: 0, - base_address: gicr_base - arch::layout::GIC_V3_ITS_SIZE, + base_address: vgic_config.msi_addr, reserved1: 0, }; madt.append(gicits); diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index 627aa9d55..20d7557da 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -1205,11 +1205,11 @@ impl DeviceManager { #[cfg(target_arch = "aarch64")] { let vcpus = self.config.lock().unwrap().cpus.boot_vcpus; - let msi_start = arch::layout::GIC_V3_DIST_START.raw_value() - - arch::layout::GIC_V3_REDIST_SIZE * (vcpus as u64) - - arch::layout::GIC_V3_ITS_SIZE; - let msi_end = msi_start + arch::layout::GIC_V3_ITS_SIZE - 1; - (msi_start, msi_end) + let vgic_config = gic::Gic::create_default_config(vcpus.into()); + ( + vgic_config.msi_addr, + vgic_config.msi_addr + vgic_config.msi_size - 1, + ) } #[cfg(target_arch = "x86_64")] (0xfee0_0000, 0xfeef_ffff) diff --git a/vmm/src/vm.rs b/vmm/src/vm.rs index 152fb14a9..9264b3da7 100644 --- a/vmm/src/vm.rs +++ b/vmm/src/vm.rs @@ -47,7 +47,7 @@ use arch::EntryPoint; use arch::PciSpaceInfo; use arch::{NumaNode, NumaNodes}; #[cfg(target_arch = "aarch64")] -use devices::gic::GIC_V3_ITS_SNAPSHOT_ID; +use devices::gic::{Gic, GIC_V3_ITS_SNAPSHOT_ID}; #[cfg(target_arch = "aarch64")] use devices::interrupt_controller::{self, InterruptController}; use devices::AcpiNotificationFlags; @@ -1262,6 +1262,7 @@ 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() @@ -1272,7 +1273,7 @@ impl Vm { .unwrap() .create_vgic( &self.memory_manager.lock().as_ref().unwrap().vm, - self.cpu_manager.lock().unwrap().boot_vcpus() as u64, + Gic::create_default_config(vcpu_count), ) .map_err(|_| { Error::ConfigureSystem(arch::Error::PlatformSpecific( @@ -2306,6 +2307,7 @@ impl Vm { // 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() @@ -2313,7 +2315,7 @@ impl Vm { .unwrap() .lock() .unwrap() - .create_vgic(&self.vm, vcpu_numbers.try_into().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. @@ -3407,14 +3409,7 @@ mod tests { let hv = hypervisor::new().unwrap(); let vm = hv.create_vm().unwrap(); let gic = vm - .create_vgic( - 1, - 0x0900_0000 - 0x01_0000, - 0x01_0000, - 0x02_0000, - 0x02_0000, - 256, - ) + .create_vgic(Gic::create_default_config(1)) .expect("Cannot create gic"); assert!(create_fdt( &mem,