diff --git a/CHANGELOG.md b/CHANGELOG.md index 579f1193877..0646e0510e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,9 @@ and this project adheres to - [#5260](https://github.com/firecracker-microvm/firecracker/pull/5260): Fixed a bug allowing the block device to starve all other devices when backed by a sufficiently slow drive. +- [#4207](https://github.com/firecracker-microvm/firecracker/issues/4207): Fixed + GSI numbering on aarch64 to correctly allow up to 96 devices being attached + simultaneously. ## [1.12.0] diff --git a/docs/device-api.md b/docs/device-api.md index dd2b5e4c9f6..de57fdcd74c 100644 --- a/docs/device-api.md +++ b/docs/device-api.md @@ -125,12 +125,6 @@ specification: | | track_dirty_pages | O | O | O | O | O | O | | | vcpu_count | O | O | O | O | O | O | -## Known device limitations - -If more than 64 devices are configured for a VM in total on aarch64, only first -64 of them are functional -([related issue](https://github.com/firecracker-microvm/firecracker/issues/4207)). - ## Instance Actions All instance actions can be found in the [Swagger](https://swagger.io) diff --git a/src/vmm/src/arch/aarch64/fdt.rs b/src/vmm/src/arch/aarch64/fdt.rs index 61200cb2148..7d7f7d748a9 100644 --- a/src/vmm/src/arch/aarch64/fdt.rs +++ b/src/vmm/src/arch/aarch64/fdt.rs @@ -362,7 +362,7 @@ fn create_virtio_node(fdt: &mut FdtWriter, dev_info: &MMIODeviceInfo) -> Result< "interrupts", &[ GIC_FDT_IRQ_TYPE_SPI, - dev_info.irq.unwrap().into(), + dev_info.irq.unwrap(), IRQ_TYPE_EDGE_RISING, ], )?; @@ -383,7 +383,7 @@ fn create_serial_node(fdt: &mut FdtWriter, dev_info: &MMIODeviceInfo) -> Result< "interrupts", &[ GIC_FDT_IRQ_TYPE_SPI, - dev_info.irq.unwrap().into(), + dev_info.irq.unwrap(), IRQ_TYPE_EDGE_RISING, ], )?; @@ -439,7 +439,6 @@ fn create_devices_node( #[cfg(test)] mod tests { use std::ffi::CString; - use std::num::NonZeroU32; use kvm_ioctls::Kvm; @@ -470,7 +469,7 @@ mod tests { (DeviceType::Serial, DeviceType::Serial.to_string()), MMIODeviceInfo { addr: 0x00, - irq: NonZeroU32::new(1), + irq: Some(1u32), len: LEN, }, ), @@ -478,7 +477,7 @@ mod tests { (DeviceType::Virtio(1), "virtio".to_string()), MMIODeviceInfo { addr: LEN, - irq: NonZeroU32::new(2), + irq: Some(2u32), len: LEN, }, ), @@ -486,7 +485,7 @@ mod tests { (DeviceType::Rtc, "rtc".to_string()), MMIODeviceInfo { addr: 2 * LEN, - irq: NonZeroU32::new(3), + irq: Some(3u32), len: LEN, }, ), diff --git a/src/vmm/src/arch/aarch64/layout.rs b/src/vmm/src/arch/aarch64/layout.rs index 922cfbb66e6..8f95519830e 100644 --- a/src/vmm/src/arch/aarch64/layout.rs +++ b/src/vmm/src/arch/aarch64/layout.rs @@ -80,5 +80,13 @@ 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; + /// Below this address will reside the GIC, above this address will reside the MMIO devices. pub const MAPPED_IO_START: u64 = 1 << 30; // 1 GB diff --git a/src/vmm/src/arch/mod.rs b/src/vmm/src/arch/mod.rs index 61d65fea1a5..ebd270a2e61 100644 --- a/src/vmm/src/arch/mod.rs +++ b/src/vmm/src/arch/mod.rs @@ -22,8 +22,8 @@ pub use aarch64::vm::{ArchVm, ArchVmError, VmState}; pub use aarch64::{ ConfigurationError, MMIO_MEM_SIZE, MMIO_MEM_START, arch_memory_regions, configure_system_for_boot, get_kernel_start, initrd_load_addr, layout::CMDLINE_MAX_SIZE, - layout::IRQ_BASE, layout::IRQ_MAX, layout::SYSTEM_MEM_SIZE, layout::SYSTEM_MEM_START, - load_kernel, + layout::GSI_BASE, layout::GSI_MAX, layout::IRQ_BASE, layout::IRQ_MAX, layout::SYSTEM_MEM_SIZE, + layout::SYSTEM_MEM_START, load_kernel, }; /// Module for x86_64 related functionality. @@ -41,8 +41,9 @@ pub use x86_64::vm::{ArchVm, ArchVmError, VmState}; pub use crate::arch::x86_64::{ ConfigurationError, MMIO_MEM_SIZE, MMIO_MEM_START, arch_memory_regions, configure_system_for_boot, get_kernel_start, initrd_load_addr, layout::APIC_ADDR, - layout::CMDLINE_MAX_SIZE, layout::IOAPIC_ADDR, layout::IRQ_BASE, layout::IRQ_MAX, - layout::SYSTEM_MEM_SIZE, layout::SYSTEM_MEM_START, load_kernel, + layout::CMDLINE_MAX_SIZE, layout::GSI_BASE, layout::GSI_MAX, layout::IOAPIC_ADDR, + layout::IRQ_BASE, layout::IRQ_MAX, layout::SYSTEM_MEM_SIZE, layout::SYSTEM_MEM_START, + load_kernel, }; /// Types of devices that can get attached to this platform. diff --git a/src/vmm/src/arch/x86_64/layout.rs b/src/vmm/src/arch/x86_64/layout.rs index 18d718a49b8..a4c2f036906 100644 --- a/src/vmm/src/arch/x86_64/layout.rs +++ b/src/vmm/src/arch/x86_64/layout.rs @@ -24,6 +24,12 @@ 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; + /// Address for the TSS setup. pub const KVM_TSS_ADDRESS: u64 = 0xfffb_d000; diff --git a/src/vmm/src/device_manager/mmio.rs b/src/vmm/src/device_manager/mmio.rs index 394935fe5c1..9a7dc775295 100644 --- a/src/vmm/src/device_manager/mmio.rs +++ b/src/vmm/src/device_manager/mmio.rs @@ -7,7 +7,6 @@ use std::collections::HashMap; use std::fmt::Debug; -use std::num::NonZeroU32; use std::sync::{Arc, Mutex}; #[cfg(target_arch = "x86_64")] @@ -79,7 +78,7 @@ pub struct MMIODeviceInfo { /// Mmio addr range length. pub len: u64, /// Used Irq line for the device. - pub irq: Option, // NOTE: guaranteed to be a value not 0, 0 is not allowed + pub irq: Option, } #[cfg(target_arch = "x86_64")] @@ -89,7 +88,7 @@ fn add_virtio_aml( len: u64, irq: u32, ) -> Result<(), aml::AmlError> { - let dev_id = irq - crate::arch::IRQ_BASE; + let dev_id = irq - crate::arch::GSI_BASE; debug!( "acpi: Building AML for VirtIO device _SB_.V{:03}. memory range: {:#010x}:{} irq: {}", dev_id, addr, len, irq @@ -151,7 +150,7 @@ impl MMIODeviceManager { ) -> Result { let irq = match resource_allocator.allocate_gsi(irq_count)?[..] { [] => None, - [irq] => NonZeroU32::new(irq), + [irq] => Some(irq), _ => return Err(MmioError::InvalidIrqConfig), }; @@ -205,7 +204,7 @@ impl MMIODeviceManager { vm.register_ioevent(queue_evt, &io_addr, u32::try_from(i).unwrap()) .map_err(MmioError::RegisterIoEvent)?; } - vm.register_irqfd(&locked_device.interrupt_trigger().irq_evt, irq.get()) + vm.register_irqfd(&locked_device.interrupt_trigger().irq_evt, irq) .map_err(MmioError::RegisterIrqFd)?; } @@ -231,7 +230,7 @@ impl MMIODeviceManager { .add_virtio_mmio_device( device_info.len, GuestAddress(device_info.addr), - device_info.irq.unwrap().get(), + device_info.irq.unwrap(), None, ) .map_err(MmioError::Cmdline) @@ -258,7 +257,7 @@ impl MMIODeviceManager { device_info.len, // We are sure that `irqs` has at least one element; allocate_mmio_resources makes // sure of it. - device_info.irq.unwrap().get(), + device_info.irq.unwrap(), )?; } Ok(device_info) @@ -290,7 +289,7 @@ impl MMIODeviceManager { .unwrap() .serial .interrupt_evt(), - device_info.irq.unwrap().get(), + device_info.irq.unwrap(), ) .map_err(MmioError::RegisterIrqFd)?; @@ -693,7 +692,7 @@ mod tests { #[cfg(target_arch = "aarch64")] vm.setup_irqchip(1).unwrap(); - for _i in crate::arch::IRQ_BASE..=crate::arch::IRQ_MAX { + for _i in crate::arch::GSI_BASE..=crate::arch::GSI_MAX { device_manager .register_virtio_test_device( vm.fd(), @@ -773,11 +772,10 @@ mod tests { device_manager.id_to_dev_info[&(DeviceType::Virtio(type_id), id.clone())].addr ); assert_eq!( - crate::arch::IRQ_BASE, + crate::arch::GSI_BASE, device_manager.id_to_dev_info[&(DeviceType::Virtio(type_id), id)] .irq .unwrap() - .get() ); let id = "bar"; @@ -834,7 +832,7 @@ mod tests { let device_info = device_manager .allocate_mmio_resources(&mut resource_allocator, 1) .unwrap(); - assert_eq!(device_info.irq.unwrap().get(), crate::arch::IRQ_BASE); + assert_eq!(device_info.irq.unwrap(), crate::arch::GSI_BASE); } #[test] diff --git a/src/vmm/src/device_manager/resources.rs b/src/vmm/src/device_manager/resources.rs index 821148794ec..719426a1f55 100644 --- a/src/vmm/src/device_manager/resources.rs +++ b/src/vmm/src/device_manager/resources.rs @@ -27,7 +27,7 @@ impl ResourceAllocator { /// Create a new resource allocator for Firecracker devices pub fn new() -> Result { Ok(Self { - gsi_allocator: IdAllocator::new(arch::IRQ_BASE, arch::IRQ_MAX)?, + gsi_allocator: IdAllocator::new(arch::GSI_BASE, arch::GSI_MAX)?, mmio_memory: AddressAllocator::new(arch::MMIO_MEM_START, arch::MMIO_MEM_SIZE)?, system_memory: AddressAllocator::new(arch::SYSTEM_MEM_START, arch::SYSTEM_MEM_SIZE)?, }) @@ -102,7 +102,7 @@ mod tests { use super::ResourceAllocator; use crate::arch; - const MAX_IRQS: u32 = arch::IRQ_MAX - arch::IRQ_BASE + 1; + const MAX_IRQS: u32 = arch::GSI_MAX - arch::GSI_BASE + 1; #[test] fn test_allocate_gsi() { @@ -117,7 +117,7 @@ mod tests { // But allocating all of them at once should work assert_eq!( allocator.allocate_gsi(MAX_IRQS), - Ok((arch::IRQ_BASE..=arch::IRQ_MAX).collect::>()) + Ok((arch::GSI_BASE..=arch::GSI_MAX).collect::>()) ); // And now we ran out of GSIs assert_eq!( @@ -129,16 +129,16 @@ mod tests { let mut allocator = ResourceAllocator::new().unwrap(); // 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), 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])); } } diff --git a/tests/integration_tests/functional/test_max_devices.py b/tests/integration_tests/functional/test_max_devices.py index fc8397445c9..85cf2f1399c 100644 --- a/tests/integration_tests/functional/test_max_devices.py +++ b/tests/integration_tests/functional/test_max_devices.py @@ -6,23 +6,27 @@ import pytest -# IRQs are available from 5 to 23. We always use one IRQ for VMGenID device, so -# the maximum number of devices supported at the same time is 18. -MAX_DEVICES_ATTACHED = 18 +# On x86_64, IRQs are available from 5 to 23. We always use one IRQ for VMGenID +# device, so the maximum number of devices supported at the same time is 18. +# On aarch64, IRQs are available from 32 to 127. We always use one IRQ each for +# the VMGenID and RTC devices, so the maximum number of devices supported +# at the same time is 94. +MAX_DEVICES_ATTACHED = {"x86_64": 18, "aarch64": 94}.get(platform.machine()) -@pytest.mark.skipif( - platform.machine() != "x86_64", reason="Firecracker supports 24 IRQs on x86_64." -) -def test_attach_maximum_devices(uvm_plain_any): + +def test_attach_maximum_devices(microvm_factory, guest_kernel, rootfs): """ Test attaching maximum number of devices to the microVM. """ - test_microvm = uvm_plain_any + if MAX_DEVICES_ATTACHED is None: + pytest.skip("Unsupported platform for this test.") + + test_microvm = microvm_factory.build(guest_kernel, rootfs, monitor_memory=False) test_microvm.spawn() - # Set up a basic microVM. - test_microvm.basic_config() + # The default 256mib is not enough for 94 ssh connections on aarch64. + test_microvm.basic_config(mem_size_mib=512) # Add (`MAX_DEVICES_ATTACHED` - 1) devices because the rootfs # has already been configured in the `basic_config()`function. @@ -36,14 +40,14 @@ def test_attach_maximum_devices(uvm_plain_any): test_microvm.ssh_iface(i).check_output("sync") -@pytest.mark.skipif( - platform.machine() != "x86_64", reason="Firecracker supports 24 IRQs on x86_64." -) -def test_attach_too_many_devices(uvm_plain): +def test_attach_too_many_devices(microvm_factory, guest_kernel, rootfs): """ Test attaching to a microVM more devices than available IRQs. """ - test_microvm = uvm_plain + if MAX_DEVICES_ATTACHED is None: + pytest.skip("Unsupported platform for this test.") + + test_microvm = microvm_factory.build(guest_kernel, rootfs, monitor_memory=False) test_microvm.spawn() # Set up a basic microVM.