Skip to content

Allow for more MSIs for PCI devices #5299

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/vmm/src/arch/aarch64/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
17 changes: 9 additions & 8 deletions src/vmm/src/arch/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand All @@ -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,
};
Expand Down
4 changes: 4 additions & 0 deletions src/vmm/src/arch/x86_64/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
10 changes: 5 additions & 5 deletions src/vmm/src/device_manager/mmio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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]),
}
};

Expand Down Expand Up @@ -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]),
}
};

Expand Down
4 changes: 2 additions & 2 deletions src/vmm/src/devices/acpi/vmgenid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
100 changes: 87 additions & 13 deletions src/vmm/src/vstate/resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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!(
Expand All @@ -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]));
}
}
Expand All @@ -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();
Expand All @@ -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();
Expand Down
6 changes: 3 additions & 3 deletions src/vmm/src/vstate/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down
Loading