fix(msi): allocate GSI for MSI and legacy IRQ from different ranges

Currently, we're limited to 24 GSI lines, which is too little for PCI
devices. Keep the current ranges as "legacy IRQ", and create a new range
for "GSI" that goes up to the kvm theoretical maximum of 4096 lines.

Signed-off-by: Riccardo Mancini <mancio@amazon.com>

Author: Manciukic

src/vmm/src/arch/aarch64/layout.rs

@@ -78,9 +78,12 @@ pub const FDT_MAX_SIZE: usize = 0x20_0000;
 // * a multiple of 32.
 /// The highest usable SPI on aarch64.
 pub const IRQ_MAX: u32 = 128;
-
 /// First usable interrupt on aarch64.
 pub const IRQ_BASE: u32 = 32;
+/// The highest available GSI in KVM (KVM_MAX_IRQ_ROUTES=4096)
+pub const GSI_MAX: u32 = 4095;
+/// First GSI after legacy IRQ on aarch64.
+pub const GSI_BASE: u32 = 129;
 
 /// The start of the memory area reserved for MMIO 32-bit accesses.
 /// Below this address will reside the GIC, above this address will reside the MMIO devices.

src/vmm/src/arch/mod.rs

@@ -21,10 +21,11 @@ pub use aarch64::vm::{ArchVm, ArchVmError, VmState};
 #[cfg(target_arch = "aarch64")]
 pub use aarch64::{
     ConfigurationError, arch_memory_regions, configure_system_for_boot, get_kernel_start,
-    initrd_load_addr, layout::BOOT_DEVICE_MEM_START, layout::CMDLINE_MAX_SIZE, layout::IRQ_BASE,
-    layout::IRQ_MAX, layout::MEM_32BIT_DEVICES_SIZE, layout::MEM_32BIT_DEVICES_START,
-    layout::MEM_64BIT_DEVICES_SIZE, layout::MEM_64BIT_DEVICES_START, layout::MMIO32_MEM_SIZE,
-    layout::MMIO32_MEM_START, layout::PCI_MMCONFIG_SIZE, layout::PCI_MMCONFIG_START,
+    initrd_load_addr, layout::BOOT_DEVICE_MEM_START, layout::CMDLINE_MAX_SIZE, layout::GSI_BASE,
+    layout::GSI_MAX, layout::IRQ_BASE, layout::IRQ_MAX, layout::MEM_32BIT_DEVICES_SIZE,
+    layout::MEM_32BIT_DEVICES_START, layout::MEM_64BIT_DEVICES_SIZE,
+    layout::MEM_64BIT_DEVICES_START, layout::MMIO32_MEM_SIZE, layout::MMIO32_MEM_START,
+    layout::PCI_MMCONFIG_SIZE, layout::PCI_MMCONFIG_START,
     layout::PCI_MMIO_CONFIG_SIZE_PER_SEGMENT, layout::RTC_MEM_START, layout::SERIAL_MEM_START,
     layout::SYSTEM_MEM_SIZE, layout::SYSTEM_MEM_START, load_kernel,
 };
@@ -44,10 +45,10 @@ pub use x86_64::vm::{ArchVm, ArchVmError, VmState};
 pub use crate::arch::x86_64::{
     ConfigurationError, arch_memory_regions, configure_system_for_boot, get_kernel_start,
     initrd_load_addr, layout::APIC_ADDR, layout::BOOT_DEVICE_MEM_START, layout::CMDLINE_MAX_SIZE,
-    layout::IOAPIC_ADDR, layout::IRQ_BASE, layout::IRQ_MAX, layout::MEM_32BIT_DEVICES_SIZE,
-    layout::MEM_32BIT_DEVICES_START, layout::MEM_64BIT_DEVICES_SIZE,
-    layout::MEM_64BIT_DEVICES_START, layout::MMIO32_MEM_SIZE, layout::MMIO32_MEM_START,
-    layout::PCI_MMCONFIG_SIZE, layout::PCI_MMCONFIG_START,
+    layout::GSI_BASE, layout::GSI_MAX, layout::IOAPIC_ADDR, layout::IRQ_BASE, layout::IRQ_MAX,
+    layout::MEM_32BIT_DEVICES_SIZE, layout::MEM_32BIT_DEVICES_START,
+    layout::MEM_64BIT_DEVICES_SIZE, layout::MEM_64BIT_DEVICES_START, layout::MMIO32_MEM_SIZE,
+    layout::MMIO32_MEM_START, layout::PCI_MMCONFIG_SIZE, layout::PCI_MMCONFIG_START,
     layout::PCI_MMIO_CONFIG_SIZE_PER_SEGMENT, layout::SYSTEM_MEM_SIZE, layout::SYSTEM_MEM_START,
     load_kernel,
 };

src/vmm/src/arch/x86_64/layout.rs

@@ -26,6 +26,10 @@ pub const HIMEM_START: u64 = 0x0010_0000; // 1 MB.
 pub const IRQ_BASE: u32 = 5;
 /// Last usable IRQ ID for virtio device interrupts on x86_64.
 pub const IRQ_MAX: u32 = 23;
+/// First GSI after legacy interrupts/
+pub const GSI_BASE: u32 = 24;
+/// The highest available GSI in KVM (KVM_MAX_IRQ_ROUTES=4096)
+pub const GSI_MAX: u32 = 4095;
 
 /// Address for the TSS setup.
 pub const KVM_TSS_ADDRESS: u64 = 0xfffb_d000;

src/vmm/src/device_manager/mmio.rs

@@ -157,7 +157,7 @@ impl MMIODeviceManager {
         resource_allocator: &mut ResourceAllocator,
         irq_count: u32,
     ) -> Result<MMIODeviceInfo, MmioError> {
-        let irq = match resource_allocator.allocate_gsi(irq_count)?[..] {
+        let irq = match resource_allocator.allocate_irq(irq_count)?[..] {
             [] => None,
             [irq] => NonZeroU32::new(irq),
             _ => return Err(MmioError::InvalidIrqConfig),
@@ -277,11 +277,11 @@ impl MMIODeviceManager {
         let device_info = if let Some(device_info) = device_info_opt {
             device_info
         } else {
-            let gsi = vm.resource_allocator().allocate_gsi(1)?;
+            let irq = vm.resource_allocator().allocate_irq(1)?;
             MMIODeviceInfo {
                 addr: SERIAL_MEM_START,
                 len: MMIO_LEN,
-                irq: NonZeroU32::new(gsi[0]),
+                irq: NonZeroU32::new(irq[0]),
             }
         };
 
@@ -336,11 +336,11 @@ impl MMIODeviceManager {
         let device_info = if let Some(device_info) = device_info_opt {
             device_info
         } else {
-            let gsi = vm.resource_allocator().allocate_gsi(1)?;
+            let irq = vm.resource_allocator().allocate_irq(1)?;
             MMIODeviceInfo {
                 addr: RTC_MEM_START,
                 len: MMIO_LEN,
-                irq: NonZeroU32::new(gsi[0]),
+                irq: NonZeroU32::new(irq[0]),
             }
         };
 

src/vmm/src/devices/acpi/vmgenid.rs

@@ -88,15 +88,15 @@ impl VmGenId {
         mem: &GuestMemoryMmap,
         resource_allocator: &mut ResourceAllocator,
     ) -> Result<Self, VmGenIdError> {
-        let gsi = resource_allocator.allocate_gsi(1)?;
+        let irq = resource_allocator.allocate_irq(1)?;
         // The generation ID needs to live in an 8-byte aligned buffer
         let addr = resource_allocator.allocate_system_memory(
             VMGENID_MEM_SIZE,
             8,
             vm_allocator::AllocPolicy::LastMatch,
         )?;
 
-        Self::from_parts(GuestAddress(addr), gsi[0], mem)
+        Self::from_parts(GuestAddress(addr), irq[0], mem)
     }
 
     // Create a 16-bytes random number

src/vmm/src/vstate/resources.rs

@@ -19,7 +19,9 @@ use crate::snapshot::Persist;
 /// * Memory allocations in the MMIO address space
 #[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct ResourceAllocator {
-    /// Allocator for device interrupt lines
+    /// Allocator for legacy device interrupt lines
+    pub irq_allocator: IdAllocator,
+    /// Allocator for PCI device GSIs
     pub gsi_allocator: IdAllocator,
     /// Allocator for memory in the 32-bit MMIO address space
     pub mmio32_memory: AddressAllocator,
@@ -41,7 +43,8 @@ impl ResourceAllocator {
         // It is fine for us to unwrap the following since we know we are passing valid ranges for
         // all allocators
         Self {
-            gsi_allocator: IdAllocator::new(arch::IRQ_BASE, arch::IRQ_MAX).unwrap(),
+            irq_allocator: IdAllocator::new(arch::IRQ_BASE, arch::IRQ_MAX).unwrap(),
+            gsi_allocator: IdAllocator::new(arch::GSI_BASE, arch::GSI_MAX).unwrap(),
             mmio32_memory: AddressAllocator::new(
                 arch::MEM_32BIT_DEVICES_START,
                 arch::MEM_32BIT_DEVICES_SIZE,
@@ -57,6 +60,30 @@ impl ResourceAllocator {
         }
     }
 
+    /// Allocate a number of legacy IRQs
+    ///
+    /// # Arguments
+    ///
+    /// * `irq_count` - The number of legacy IRQs to allocate
+    pub fn allocate_irq(&mut self, irq_count: u32) -> Result<Vec<u32>, vm_allocator::Error> {
+        let mut irqs = Vec::with_capacity(irq_count as usize);
+
+        for _ in 0..irq_count {
+            match self.irq_allocator.allocate_id() {
+                Ok(irq) => irqs.push(irq),
+                Err(err) => {
+                    // It is ok to unwrap here, we just allocated the GSI
+                    irqs.into_iter().for_each(|irq| {
+                        self.irq_allocator.free_id(irq).unwrap();
+                    });
+                    return Err(err);
+                }
+            }
+        }
+
+        Ok(irqs)
+    }
+
     /// Allocate a number of GSIs
     ///
     /// # Arguments
@@ -167,25 +194,66 @@ mod tests {
     use vm_allocator::AllocPolicy;
 
     use super::ResourceAllocator;
-    use crate::arch::{self, IRQ_BASE};
+    use crate::arch::{self, GSI_BASE, IRQ_BASE};
     use crate::snapshot::{Persist, Snapshot};
 
     const MAX_IRQS: u32 = arch::IRQ_MAX - arch::IRQ_BASE + 1;
 
+    #[test]
+    fn test_allocate_irq() {
+        let mut allocator = ResourceAllocator::new();
+        // asking for 0 IRQs should return us an empty vector
+        assert_eq!(allocator.allocate_irq(0), Ok(vec![]));
+        // We cannot allocate more GSIs than available
+        assert_eq!(
+            allocator.allocate_irq(MAX_IRQS + 1),
+            Err(vm_allocator::Error::ResourceNotAvailable)
+        );
+        // But allocating all of them at once should work
+        assert_eq!(
+            allocator.allocate_irq(MAX_IRQS),
+            Ok((arch::IRQ_BASE..=arch::IRQ_MAX).collect::<Vec<_>>())
+        );
+        // And now we ran out of GSIs
+        assert_eq!(
+            allocator.allocate_irq(1),
+            Err(vm_allocator::Error::ResourceNotAvailable)
+        );
+        // But we should be able to ask for 0 GSIs
+        assert_eq!(allocator.allocate_irq(0), Ok(vec![]));
+
+        let mut allocator = ResourceAllocator::new();
+        // We should be able to allocate 1 GSI
+        assert_eq!(allocator.allocate_irq(1), Ok(vec![arch::IRQ_BASE]));
+        // We can't allocate MAX_IRQS any more
+        assert_eq!(
+            allocator.allocate_irq(MAX_IRQS),
+            Err(vm_allocator::Error::ResourceNotAvailable)
+        );
+        // We can allocate another one and it should be the second available
+        assert_eq!(allocator.allocate_irq(1), Ok(vec![arch::IRQ_BASE + 1]));
+        // Let's allocate the rest in a loop
+        for i in arch::IRQ_BASE + 2..=arch::IRQ_MAX {
+            assert_eq!(allocator.allocate_irq(1), Ok(vec![i]));
+        }
+    }
+
+    const MAX_GSIS: u32 = arch::GSI_MAX - arch::GSI_BASE + 1;
+
     #[test]
     fn test_allocate_gsi() {
         let mut allocator = ResourceAllocator::new();
         // asking for 0 IRQs should return us an empty vector
         assert_eq!(allocator.allocate_gsi(0), Ok(vec![]));
         // We cannot allocate more GSIs than available
         assert_eq!(
-            allocator.allocate_gsi(MAX_IRQS + 1),
+            allocator.allocate_gsi(MAX_GSIS + 1),
             Err(vm_allocator::Error::ResourceNotAvailable)
         );
         // But allocating all of them at once should work
         assert_eq!(
-            allocator.allocate_gsi(MAX_IRQS),
-            Ok((arch::IRQ_BASE..=arch::IRQ_MAX).collect::<Vec<_>>())
+            allocator.allocate_gsi(MAX_GSIS),
+            Ok((arch::GSI_BASE..=arch::GSI_MAX).collect::<Vec<_>>())
         );
         // And now we ran out of GSIs
         assert_eq!(
@@ -197,16 +265,16 @@ mod tests {
 
         let mut allocator = ResourceAllocator::new();
         // We should be able to allocate 1 GSI
-        assert_eq!(allocator.allocate_gsi(1), Ok(vec![arch::IRQ_BASE]));
+        assert_eq!(allocator.allocate_gsi(1), Ok(vec![arch::GSI_BASE]));
         // We can't allocate MAX_IRQS any more
         assert_eq!(
-            allocator.allocate_gsi(MAX_IRQS),
+            allocator.allocate_gsi(MAX_GSIS),
             Err(vm_allocator::Error::ResourceNotAvailable)
         );
         // We can allocate another one and it should be the second available
-        assert_eq!(allocator.allocate_gsi(1), Ok(vec![arch::IRQ_BASE + 1]));
+        assert_eq!(allocator.allocate_gsi(1), Ok(vec![arch::GSI_BASE + 1]));
         // Let's allocate the rest in a loop
-        for i in arch::IRQ_BASE + 2..=arch::IRQ_MAX {
+        for i in arch::GSI_BASE + 2..=arch::GSI_MAX {
             assert_eq!(allocator.allocate_gsi(1), Ok(vec![i]));
         }
     }
@@ -221,12 +289,16 @@ mod tests {
     #[test]
     fn test_save_restore() {
         let mut allocator0 = ResourceAllocator::new();
+        let irq_0 = allocator0.allocate_irq(1).unwrap()[0];
+        assert_eq!(irq_0, IRQ_BASE);
         let gsi_0 = allocator0.allocate_gsi(1).unwrap()[0];
-        assert_eq!(gsi_0, IRQ_BASE);
+        assert_eq!(gsi_0, GSI_BASE);
 
         let mut allocator1 = clone_allocator(&allocator0);
+        let irq_1 = allocator1.allocate_irq(1).unwrap()[0];
+        assert_eq!(irq_1, IRQ_BASE + 1);
         let gsi_1 = allocator1.allocate_gsi(1).unwrap()[0];
-        assert_eq!(gsi_1, IRQ_BASE + 1);
+        assert_eq!(gsi_1, GSI_BASE + 1);
         let mmio32_mem = allocator1
             .allocate_32bit_mmio_memory(0x42, 1, AllocPolicy::FirstMatch)
             .unwrap();
@@ -251,8 +323,10 @@ mod tests {
             .allocate_system_memory(0x42, 1, AllocPolicy::ExactMatch(system_mem))
             .unwrap_err();
 
+        let irq_2 = allocator2.allocate_irq(1).unwrap()[0];
+        assert_eq!(irq_2, IRQ_BASE + 2);
         let gsi_2 = allocator2.allocate_gsi(1).unwrap()[0];
-        assert_eq!(gsi_2, IRQ_BASE + 2);
+        assert_eq!(gsi_2, GSI_BASE + 2);
         let mmio32_mem = allocator1
             .allocate_32bit_mmio_memory(0x42, 1, AllocPolicy::FirstMatch)
             .unwrap();

src/vmm/src/vstate/vm.rs

@@ -877,7 +877,7 @@ pub(crate) mod tests {
         }
 
         for i in 0..4 {
-            let gsi = crate::arch::IRQ_BASE + i;
+            let gsi = crate::arch::GSI_BASE + i;
             let interrupts = vm.common.interrupts.lock().unwrap();
             let kvm_route = interrupts.get(&gsi).unwrap();
             assert!(kvm_route.masked);
@@ -894,7 +894,7 @@ pub(crate) mod tests {
         // Simply enabling the vectors should not update the registered IRQ routes
         msix_group.enable().unwrap();
         for i in 0..4 {
-            let gsi = crate::arch::IRQ_BASE + i;
+            let gsi = crate::arch::GSI_BASE + i;
             let interrupts = vm.common.interrupts.lock().unwrap();
             let kvm_route = interrupts.get(&gsi).unwrap();
             assert!(kvm_route.masked);
@@ -914,7 +914,7 @@ pub(crate) mod tests {
             .update(0, InterruptSourceConfig::MsiIrq(config), false, true)
             .unwrap();
         for i in 0..4 {
-            let gsi = crate::arch::IRQ_BASE + i;
+            let gsi = crate::arch::GSI_BASE + i;
             let interrupts = vm.common.interrupts.lock().unwrap();
             let kvm_route = interrupts.get(&gsi).unwrap();
             assert_eq!(kvm_route.masked, i != 0);