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 <nudasnev@microsoft.com>

devices/src/gic.rs

@@ -74,18 +74,9 @@ impl Gic {
     pub fn create_vgic(
         &mut self,
         vm: &Arc<dyn hypervisor::Vm>,
-        vcpu_count: u64,
+        config: VgicConfig,
     ) -> Result<Arc<Mutex<dyn Vgic>>> {
-        let vgic = vm
+        let vgic = vm.create_vgic(config).map_err(Error::CreateGic)?;
-            .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)?;
         self.vgic = Some(vgic.clone());
         Ok(vgic.clone())
     }

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.
+        /* Setting up the distributor attribute. */
-         We are placing the GIC below 1GB so we need to substract the size of the distributor.
-        */
         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.
+        /* Setting up the redistributors' attribute. */
-        We are calculating here the start of the redistributors address. We have one per CPU.
-        */
         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(
+    pub fn new(vm: &dyn Vm, config: VgicConfig) -> Result<KvmGicV3Its> {
-        vm: &dyn Vm,
-        vcpu_count: u64,
-        dist_addr: u64,
-        dist_size: u64,
-        redist_size: u64,
-        msi_size: u64,
-        nr_irqs: u32,
-    ) -> Result<KvmGicV3Its> {
         // This is inside KVM module
         let vm = vm.as_any().downcast_ref::<KvmVm>().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()],
+            gicr_typers: vec![0; config.vcpu_count.try_into().unwrap()],
-            dist_addr,
+            dist_addr: config.dist_addr,
-            dist_size,
+            dist_size: config.dist_size,
-            redists_addr,
+            redists_addr: config.redists_addr,
-            redists_size,
+            redists_size: config.redists_size,
-            msi_addr,
+            msi_addr: config.msi_addr,
-            msi_size,
+            msi_size: config.msi_size,
-            vcpu_count,
+            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(
+        assert!(KvmGicV3Its::new(&*vm, create_test_vgic_config()).is_ok());
-            &*vm,
-            1,
-            0x0900_0000 - 0x01_0000,
-            0x01_0000,
-            0x02_0000,
-            0x02_0000,
-            256
-        )
-        .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(
+        let gic = KvmGicV3Its::new(&*vm, create_test_vgic_config()).expect("Cannot create gic");
-            &*vm,
-            1,
-            0x0900_0000 - 0x01_0000,
-            0x01_0000,
-            0x02_0000,
-            0x02_0000,
-            256,
-        )
-        .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(
+        let gic = KvmGicV3Its::new(&*vm, create_test_vgic_config()).expect("Cannot create gic");
-            &*vm,
-            1,
-            0x0900_0000 - 0x01_0000,
-            0x01_0000,
-            0x02_0000,
-            0x02_0000,
-            256,
-        )
-        .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(
+        let gic = KvmGicV3Its::new(&*vm, create_test_vgic_config()).expect("Cannot create gic");
-            &*vm,
-            1,
-            0x0900_0000 - 0x01_0000,
-            0x01_0000,
-            0x02_0000,
-            0x02_0000,
-            256,
-        )
-        .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(
+            .create_vgic(create_test_vgic_config())
-                1,
-                0x0900_0000 - 0x01_0000,
-                0x01_0000,
-                0x02_0000,
-                0x02_0000,
-                256,
-            )
             .expect("Cannot create gic");
 
         assert!(gic.lock().unwrap().save_data_tables().is_ok());

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(
+    fn create_vgic(&self, config: VgicConfig) -> vm::Result<Arc<Mutex<dyn Vgic>>> {
-        &self,
+        let gic_device = KvmGicV3Its::new(self, config)
-        vcpu_count: u64,
+            .map_err(|e| vm::HypervisorVmError::CreateVgic(anyhow!("Vgic error {:?}", e)))?;
-        dist_addr: u64,
-        dist_size: u64,
-        redist_size: u64,
-        msi_size: u64,
-        nr_irqs: u32,
-    ) -> vm::Result<Arc<Mutex<dyn Vgic>>> {
-        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)))?;
         Ok(Arc::new(Mutex::new(gic_device)))
     }
     ///

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<Arc<dyn VmOps>>) -> Result<Arc<dyn Vcpu>>;
     #[cfg(target_arch = "aarch64")]
-    fn create_vgic(
+    fn create_vgic(&self, config: VgicConfig) -> Result<Arc<Mutex<dyn Vgic>>>;
-        &self,
-        vcpu_count: u64,
-        dist_addr: u64,
-        dist_size: u64,
-        redist_size: u64,
-        msi_size: u64,
-        nr_irqs: u32,
-    ) -> Result<Arc<Mutex<dyn Vgic>>>;
 
     /// Registers an event to be signaled whenever a certain address is written to.
     fn register_ioevent(

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,
+                base_address: vgic_config.redists_addr,
-                range_length: gicr_size,
+                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);

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()
+            let vgic_config = gic::Gic::create_default_config(vcpus.into());
-                - arch::layout::GIC_V3_REDIST_SIZE * (vcpus as u64)
+            (
-                - arch::layout::GIC_V3_ITS_SIZE;
+                vgic_config.msi_addr,
-            let msi_end = msi_start + arch::layout::GIC_V3_ITS_SIZE - 1;
+                vgic_config.msi_addr + vgic_config.msi_size - 1,
-            (msi_start, msi_end)
+            )
         }
         #[cfg(target_arch = "x86_64")]
         (0xfee0_0000, 0xfeef_ffff)

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(
+            .create_vgic(Gic::create_default_config(1))
-                1,
-                0x0900_0000 - 0x01_0000,
-                0x01_0000,
-                0x02_0000,
-                0x02_0000,
-                256,
-            )
             .expect("Cannot create gic");
         assert!(create_fdt(
             &mem,