Miscellaneous PCI fixes and more test coverage

src/vmm/src/arch/aarch64/gic/gicv2/mod.rs

@@ -135,9 +135,9 @@ impl GICv2 {
         // On arm there are 3 types of interrupts: SGI (0-15), PPI (16-31), SPI (32-1020).
         // SPIs are used to signal interrupts from various peripherals accessible across
         // the whole system so these are the ones that we increment when adding a new virtio device.
-        // KVM_DEV_ARM_VGIC_GRP_NR_IRQS sets the highest SPI number. Consequently, we will have a
-        // total of `super::layout::IRQ_MAX - 32` usable SPIs in our microVM.
-        let nr_irqs: u32 = super::layout::IRQ_MAX;
+        // KVM_DEV_ARM_VGIC_GRP_NR_IRQS sets the number of interrupts (SGI, PPI, and SPI).
+        // Consequently, we need to add 32 to the number of SPIs ("legacy GSI").
+        let nr_irqs: u32 = crate::arch::GSI_LEGACY_NUM + super::layout::SPI_START;
         let nr_irqs_ptr = &nr_irqs as *const u32;
         Self::set_device_attribute(
             gic_device.device_fd(),

src/vmm/src/arch/aarch64/gic/gicv2/regs/dist_regs.rs

@@ -8,7 +8,7 @@ use kvm_ioctls::DeviceFd;
 
 use crate::arch::aarch64::gic::GicError;
 use crate::arch::aarch64::gic::regs::{GicRegState, MmioReg, SimpleReg, VgicRegEngine};
-use crate::arch::{IRQ_BASE, IRQ_MAX};
+use crate::arch::{GSI_LEGACY_NUM, SPI_START};
 
 // Distributor registers as detailed at page 75 from
 // https://developer.arm.com/documentation/ihi0048/latest/.
@@ -62,9 +62,9 @@ impl MmioReg for SharedIrqReg {
         // read-as-zero/write-ignore (RAZ/WI) policy.
         // The first part of a shared-irq register, the one corresponding to the
         // SGI and PPI IRQs (0-32) is RAZ/WI, so we skip it.
-        let start = self.offset + u64::from(IRQ_BASE) * u64::from(self.bits_per_irq) / 8;
+        let start = self.offset + u64::from(SPI_START) * u64::from(self.bits_per_irq) / 8;
 
-        let size_in_bits = u64::from(self.bits_per_irq) * u64::from(IRQ_MAX - IRQ_BASE);
+        let size_in_bits = u64::from(self.bits_per_irq) * u64::from(GSI_LEGACY_NUM);
         let mut size_in_bytes = size_in_bits / 8;
         if size_in_bits % 8 > 0 {
             size_in_bytes += 1;

src/vmm/src/arch/aarch64/gic/gicv3/mod.rs

@@ -184,9 +184,9 @@ impl GICv3 {
         // On arm there are 3 types of interrupts: SGI (0-15), PPI (16-31), SPI (32-1020).
         // SPIs are used to signal interrupts from various peripherals accessible across
         // the whole system so these are the ones that we increment when adding a new virtio device.
-        // KVM_DEV_ARM_VGIC_GRP_NR_IRQS sets the highest SPI number. Consequently, we will have a
-        // total of `super::layout::IRQ_MAX - 32` usable SPIs in our microVM.
-        let nr_irqs: u32 = super::layout::IRQ_MAX;
+        // KVM_DEV_ARM_VGIC_GRP_NR_IRQS sets the number of interrupts (SGI, PPI, and SPI).
+        // Consequently, we need to add 32 to the number of SPIs ("legacy GSI").
+        let nr_irqs: u32 = crate::arch::GSI_LEGACY_NUM + super::layout::SPI_START;
         let nr_irqs_ptr = &nr_irqs as *const u32;
         Self::set_device_attribute(
             gic_device.device_fd(),

src/vmm/src/arch/aarch64/gic/gicv3/regs/dist_regs.rs

@@ -8,7 +8,7 @@ use kvm_ioctls::DeviceFd;
 
 use crate::arch::aarch64::gic::GicError;
 use crate::arch::aarch64::gic::regs::{GicRegState, MmioReg, SimpleReg, VgicRegEngine};
-use crate::arch::{IRQ_BASE, IRQ_MAX};
+use crate::arch::{GSI_LEGACY_NUM, SPI_START};
 
 // Distributor registers as detailed at page 456 from
 // https://static.docs.arm.com/ihi0069/c/IHI0069C_gic_architecture_specification.pdf.
@@ -64,9 +64,9 @@ impl MmioReg for SharedIrqReg {
         // read-as-zero/write-ignore (RAZ/WI) policy.
         // The first part of a shared-irq register, the one corresponding to the
         // SGI and PPI IRQs (0-32) is RAZ/WI, so we skip it.
-        let start = self.offset + u64::from(IRQ_BASE) * u64::from(self.bits_per_irq) / 8;
+        let start = self.offset + u64::from(SPI_START) * u64::from(self.bits_per_irq) / 8;
 
-        let size_in_bits = u64::from(self.bits_per_irq) * u64::from(IRQ_MAX - IRQ_BASE);
+        let size_in_bits = u64::from(self.bits_per_irq) * u64::from(GSI_LEGACY_NUM);
         let mut size_in_bytes = size_in_bits / 8;
         if size_in_bits % 8 > 0 {
             size_in_bytes += 1;

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

@@ -76,19 +76,25 @@ pub const FDT_MAX_SIZE: usize = 0x20_0000;
 // * bigger than 32
 // * less than 1023 and
 // * 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 Linux kernel automatically shifts the GSI by 32 if it is an SPI,
-// allowing us to start numbering from 0 instead of 32.
-/// The first usable GSI on aarch64.
-pub const GSI_BASE: u32 = 0;
-
-/// The maximum usable GSI on aarch64.
-pub const GSI_MAX: u32 = IRQ_MAX - IRQ_BASE - 1;
+// The first 32 SPIs are reserved, but KVM already shifts the gsi we
+// pass, so we go from 0 to 95 for legacy gsis ("irq") and the remaining
+// we use for MSI.
+/// Offset of first SPI in the GIC
+pub const SPI_START: u32 = 32;
+/// Last possible SPI in the GIC (128 total SPIs)
+pub const SPI_END: u32 = 127;
+/// First usable GSI id on aarch64 (corresponds to SPI #32).
+pub const GSI_LEGACY_START: u32 = 0;
+/// There are 128 SPIs available, but the first 32 are reserved
+pub const GSI_LEGACY_NUM: u32 = SPI_END - SPI_START + 1;
+/// Last available GSI
+pub const GSI_LEGACY_END: u32 = GSI_LEGACY_START + GSI_LEGACY_NUM - 1;
+/// First GSI used by MSI after legacy GSI
+pub const GSI_MSI_START: u32 = GSI_LEGACY_END + 1;
+/// The highest available GSI in KVM (KVM_MAX_IRQ_ROUTES=4096)
+pub const GSI_MSI_END: u32 = 4095;
+/// Number of GSI available for MSI.
+pub const GSI_MSI_NUM: u32 = GSI_MSI_END - GSI_MSI_START + 1;
 
 /// 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,13 +21,14 @@ 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::GSI_BASE,
-    layout::GSI_MAX, layout::IRQ_BASE, layout::IRQ_MAX, layout::MEM_32BIT_DEVICES_SIZE,
+    initrd_load_addr, layout::BOOT_DEVICE_MEM_START, layout::CMDLINE_MAX_SIZE,
+    layout::GSI_LEGACY_END, layout::GSI_LEGACY_NUM, layout::GSI_LEGACY_START, layout::GSI_MSI_END,
+    layout::GSI_MSI_NUM, layout::GSI_MSI_START, 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,
+    layout::SPI_START, layout::SYSTEM_MEM_SIZE, layout::SYSTEM_MEM_START, load_kernel,
 };
 
 /// Module for x86_64 related functionality.
@@ -45,7 +46,8 @@ 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::GSI_BASE, layout::GSI_MAX, layout::IOAPIC_ADDR, layout::IRQ_BASE, layout::IRQ_MAX,
+    layout::GSI_LEGACY_END, layout::GSI_LEGACY_NUM, layout::GSI_LEGACY_START, layout::GSI_MSI_END,
+    layout::GSI_MSI_NUM, layout::GSI_MSI_START, layout::IOAPIC_ADDR,
     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,

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

@@ -21,17 +21,21 @@ pub const CMDLINE_MAX_SIZE: usize = 2048;
 /// Start of the high memory.
 pub const HIMEM_START: u64 = 0x0010_0000; // 1 MB.
 
-// Typically, on x86 systems 24 IRQs are used (0-23).
-/// First usable IRQ ID for virtio device interrupts on x86_64.
-pub const IRQ_BASE: u32 = 5;
-/// Last usable IRQ ID for virtio device interrupts on x86_64.
-pub const IRQ_MAX: u32 = 23;
-
-/// The first usable GSI on x86_64 is the same as the first usable IRQ ID.
-pub const GSI_BASE: u32 = IRQ_BASE;
-
-/// The maximum usable GSI on x86_64 is the same as the last usable IRQ ID.
-pub const GSI_MAX: u32 = IRQ_MAX;
+// Typically, on x86 systems 24 IRQs are used for legacy devices (0-23).
+// However, the first 5 are reserved.
+// We allocate the remaining GSIs to MSIs.
+/// First usable GSI for legacy interrupts (IRQ) on x86_64.
+pub const GSI_LEGACY_START: u32 = 5;
+/// Last usable GSI for legacy interrupts (IRQ) on x86_64.
+pub const GSI_LEGACY_END: u32 = 23;
+/// Number of legacy GSI (IRQ) available on x86_64.
+pub const GSI_LEGACY_NUM: u32 = GSI_LEGACY_END - GSI_LEGACY_START + 1;
+/// First GSI used by MSI after legacy GSI.
+pub const GSI_MSI_START: u32 = GSI_LEGACY_END + 1;
+/// The highest available GSI in KVM (KVM_MAX_IRQ_ROUTES=4096).
+pub const GSI_MSI_END: u32 = 4095;
+/// Number of GSI available for MSI.
+pub const GSI_MSI_NUM: u32 = GSI_MSI_END - GSI_MSI_START + 1;
 
 /// Address for the TSS setup.
 pub const KVM_TSS_ADDRESS: u64 = 0xfffb_d000;

src/vmm/src/arch/x86_64/mptable.rs

@@ -13,7 +13,7 @@ use libc::c_char;
 use log::debug;
 use vm_allocator::AllocPolicy;
 
-use crate::arch::IRQ_MAX;
+use crate::arch::GSI_LEGACY_END;
 use crate::arch::x86_64::generated::mpspec;
 use crate::vstate::memory::{
     Address, ByteValued, Bytes, GuestAddress, GuestMemory, GuestMemoryMmap,
@@ -109,7 +109,7 @@ fn compute_mp_size(num_cpus: u8) -> usize {
         + mem::size_of::<mpspec::mpc_cpu>() * (num_cpus as usize)
         + mem::size_of::<mpspec::mpc_ioapic>()
         + mem::size_of::<mpspec::mpc_bus>()
-        + mem::size_of::<mpspec::mpc_intsrc>() * (IRQ_MAX as usize + 1)
+        + mem::size_of::<mpspec::mpc_intsrc>() * (GSI_LEGACY_END as usize + 1)
         + mem::size_of::<mpspec::mpc_lintsrc>() * 2
 }
 
@@ -225,7 +225,7 @@ pub fn setup_mptable(
         mp_num_entries += 1;
     }
     // Per kvm_setup_default_irq_routing() in kernel
-    for i in 0..=u8::try_from(IRQ_MAX).map_err(|_| MptableError::TooManyIrqs)? {
+    for i in 0..=u8::try_from(GSI_LEGACY_END).map_err(|_| MptableError::TooManyIrqs)? {
         let size = mem::size_of::<mpspec::mpc_intsrc>() as u64;
         let mpc_intsrc = mpspec::mpc_intsrc {
             type_: mpspec::MP_INTSRC.try_into().unwrap(),
@@ -406,7 +406,7 @@ mod tests {
             // ISA Bus
             + 1
             // IRQ
-            + u16::try_from(IRQ_MAX).unwrap() + 1
+            + u16::try_from(GSI_LEGACY_END).unwrap() + 1
             // Interrupt source ExtINT
             + 1
             // Interrupt source NMI

src/vmm/src/device_manager/acpi.rs

@@ -64,7 +64,7 @@ impl Aml for ACPIDeviceManager {
                                 // We know that the maximum IRQ number fits in a u8. We have up to
                                 // 32 IRQs in x86 and up to 128 in
                                 // ARM (look into
-                                // `vmm::crate::arch::layout::IRQ_MAX`)
+                                // `vmm::crate::arch::layout::GSI_LEGACY_END`)
                                 #[allow(clippy::cast_possible_truncation)]
                                 &aml::Equal::new(&aml::Arg(0), &(vmgenid.gsi as u8)),
                                 vec![&aml::Notify::new(

src/vmm/src/device_manager/mmio.rs

@@ -82,12 +82,12 @@ fn add_virtio_aml(
     dsdt_data: &mut Vec<u8>,
     addr: u64,
     len: u64,
-    irq: u32,
+    gsi: u32,
 ) -> Result<(), aml::AmlError> {
-    let dev_id = irq - crate::arch::GSI_BASE;
+    let dev_id = gsi - crate::arch::GSI_LEGACY_START;
     debug!(
-        "acpi: Building AML for VirtIO device _SB_.V{:03}. memory range: {:#010x}:{} irq: {}",
-        dev_id, addr, len, irq
+        "acpi: Building AML for VirtIO device _SB_.V{:03}. memory range: {:#010x}:{} gsi: {}",
+        dev_id, addr, len, gsi
     );
     aml::Device::new(
         format!("V{:03}", dev_id).as_str().try_into()?,
@@ -103,7 +103,7 @@ fn add_virtio_aml(
                         addr.try_into().unwrap(),
                         len.try_into().unwrap(),
                     ),
-                    &aml::Interrupt::new(true, true, false, false, irq),
+                    &aml::Interrupt::new(true, true, false, false, gsi),
                 ]),
             )?,
         ],
@@ -156,9 +156,9 @@ impl MMIODeviceManager {
         resource_allocator: &mut ResourceAllocator,
         irq_count: u32,
     ) -> Result<MMIODeviceInfo, MmioError> {
-        let irq = match resource_allocator.allocate_gsi(irq_count)?[..] {
+        let gsi = match resource_allocator.allocate_gsi_legacy(irq_count)?[..] {
             [] => None,
-            [irq] => Some(irq),
+            [gsi] => Some(gsi),
             _ => return Err(MmioError::InvalidIrqConfig),
         };
 
@@ -169,7 +169,7 @@ impl MMIODeviceManager {
                 AllocPolicy::FirstMatch,
             )?,
             len: MMIO_LEN,
-            irq,
+            irq: gsi,
         };
         Ok(device_info)
     }
@@ -183,7 +183,7 @@ impl MMIODeviceManager {
     ) -> Result<(), MmioError> {
         // Our virtio devices are currently hardcoded to use a single IRQ.
         // Validate that requirement.
-        let irq = device.resources.irq.ok_or(MmioError::InvalidIrqConfig)?;
+        let gsi = device.resources.irq.ok_or(MmioError::InvalidIrqConfig)?;
         let identifier;
         {
             let mmio_device = device.inner.lock().expect("Poisoned lock");
@@ -197,7 +197,7 @@ impl MMIODeviceManager {
                     .register_ioevent(queue_evt, &io_addr, u32::try_from(i).unwrap())
                     .map_err(MmioError::RegisterIoEvent)?;
             }
-            vm.register_irq(&mmio_device.interrupt.irq_evt, irq)
+            vm.register_irq(&mmio_device.interrupt.irq_evt, gsi)
                 .map_err(MmioError::RegisterIrqFd)?;
         }
 
@@ -276,7 +276,7 @@ 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 gsi = vm.resource_allocator().allocate_gsi_legacy(1)?;
             MMIODeviceInfo {
                 addr: SERIAL_MEM_START,
                 len: MMIO_LEN,
@@ -335,7 +335,7 @@ 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 gsi = vm.resource_allocator().allocate_gsi_legacy(1)?;
             MMIODeviceInfo {
                 addr: RTC_MEM_START,
                 len: MMIO_LEN,
@@ -612,15 +612,15 @@ pub(crate) mod tests {
         let dev = device_manager.get_virtio_device(0, "dummy").unwrap();
         assert_eq!(dev.resources.addr, arch::MEM_32BIT_DEVICES_START);
         assert_eq!(dev.resources.len, MMIO_LEN);
-        assert_eq!(dev.resources.irq, Some(arch::GSI_BASE));
+        assert_eq!(dev.resources.irq, Some(arch::GSI_LEGACY_START));
 
         device_manager
             .for_each_virtio_device(|virtio_type, device_id, mmio_device| {
                 assert_eq!(*virtio_type, 0);
                 assert_eq!(device_id, "dummy");
                 assert_eq!(mmio_device.resources.addr, arch::MEM_32BIT_DEVICES_START);
                 assert_eq!(mmio_device.resources.len, MMIO_LEN);
-                assert_eq!(mmio_device.resources.irq, Some(arch::GSI_BASE));
+                assert_eq!(mmio_device.resources.irq, Some(arch::GSI_LEGACY_START));
                 Ok::<(), ()>(())
             })
             .unwrap();
@@ -643,7 +643,7 @@ pub(crate) mod tests {
         #[cfg(target_arch = "aarch64")]
         vm.setup_irqchip(1).unwrap();
 
-        for _i in crate::arch::GSI_BASE..=crate::arch::GSI_MAX {
+        for _i in crate::arch::GSI_LEGACY_START..=crate::arch::GSI_LEGACY_END {
             device_manager
                 .register_virtio_test_device(
                     &vm,
@@ -711,7 +711,7 @@ pub(crate) mod tests {
                 .addr
         );
         assert_eq!(
-            crate::arch::GSI_BASE,
+            crate::arch::GSI_LEGACY_START,
             device_manager.virtio_devices[&(type_id, id)]
                 .resources
                 .irq
@@ -762,7 +762,7 @@ pub(crate) mod tests {
         let device_info = device_manager
             .allocate_mmio_resources(&mut resource_allocator, 1)
             .unwrap();
-        assert_eq!(device_info.irq.unwrap(), crate::arch::GSI_BASE);
+        assert_eq!(device_info.irq.unwrap(), crate::arch::GSI_LEGACY_START);
     }
 
     #[test]

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

@@ -88,7 +88,7 @@ impl VmGenId {
         mem: &GuestMemoryMmap,
         resource_allocator: &mut ResourceAllocator,
     ) -> Result<Self, VmGenIdError> {
-        let gsi = resource_allocator.allocate_gsi(1)?;
+        let gsi = resource_allocator.allocate_gsi_legacy(1)?;
         // The generation ID needs to live in an 8-byte aligned buffer
         let addr = resource_allocator.allocate_system_memory(
             VMGENID_MEM_SIZE,