Skip to content

Commit

Permalink
Utilized option instead of vector to store irq lines
Browse files Browse the repository at this point in the history
Each MMIO device in Firecracker only utilizes at most one irq line, so
capture this property at the type level.

Signed-off-by: Andrew Yao <andr3wy@gmail.com>
Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
  • Loading branch information
andr3wy authored and roypat committed Jul 16, 2024
1 parent 4488f10 commit 69b8c2c
Showing 1 changed file with 41 additions and 47 deletions.
88 changes: 41 additions & 47 deletions src/vmm/src/device_manager/mmio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

use std::collections::HashMap;
use std::fmt::Debug;
use std::num::NonZeroU32;
use std::sync::{Arc, Mutex};

#[cfg(target_arch = "x86_64")]
Expand Down Expand Up @@ -76,8 +77,8 @@ pub struct MMIODeviceInfo {
pub addr: u64,
/// Mmio addr range length.
pub len: u64,
/// Used Irq line(s) for the device.
pub irqs: Vec<u32>,
/// Used Irq line for the device.
pub irq: Option<NonZeroU32>, // NOTE: guaranteed to be a value not 0, 0 is not allowed
}

#[cfg(target_arch = "x86_64")]
Expand Down Expand Up @@ -142,15 +143,20 @@ impl MMIODeviceManager {
resource_allocator: &mut ResourceAllocator,
irq_count: u32,
) -> Result<MMIODeviceInfo, MmioError> {
let irqs = resource_allocator.allocate_gsi(irq_count)?;
let irq = match resource_allocator.allocate_gsi(irq_count)?[..] {
[] => None,
[irq] => NonZeroU32::new(irq),
_ => return Err(MmioError::InvalidIrqConfig),
};

let device_info = MMIODeviceInfo {
addr: resource_allocator.allocate_mmio_memory(
MMIO_LEN,
MMIO_LEN,
AllocPolicy::FirstMatch,
)?,
len: MMIO_LEN,
irqs,
irq,
};
Ok(device_info)
}
Expand Down Expand Up @@ -179,9 +185,9 @@ impl MMIODeviceManager {
) -> Result<(), MmioError> {
// Our virtio devices are currently hardcoded to use a single IRQ.
// Validate that requirement.
if device_info.irqs.len() != 1 {
let Some(irq) = device_info.irq else {
return Err(MmioError::InvalidIrqConfig);
}
};
let identifier;
{
let locked_device = mmio_device.locked_device();
Expand All @@ -193,11 +199,8 @@ 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,
device_info.irqs[0],
)
.map_err(MmioError::RegisterIrqFd)?;
vm.register_irqfd(&locked_device.interrupt_trigger().irq_evt, irq.get())
.map_err(MmioError::RegisterIrqFd)?;
}

self.register_mmio_device(
Expand All @@ -222,7 +225,7 @@ impl MMIODeviceManager {
.add_virtio_mmio_device(
device_info.len,
GuestAddress(device_info.addr),
device_info.irqs[0],
device_info.irq.unwrap().get(),
None,
)
.map_err(MmioError::Cmdline)
Expand All @@ -249,7 +252,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.irqs[0],
device_info.irq.unwrap().get(),
);
}
Ok(device_info)
Expand Down Expand Up @@ -281,7 +284,7 @@ impl MMIODeviceManager {
.unwrap()
.serial
.interrupt_evt(),
device_info.irqs[0],
device_info.irq.unwrap().get(),

Check warning on line 287 in src/vmm/src/device_manager/mmio.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/device_manager/mmio.rs#L287

Added line #L287 was not covered by tests
)
.map_err(MmioError::RegisterIrqFd)?;

Expand Down Expand Up @@ -507,7 +510,7 @@ impl DeviceInfoForFDT for MMIODeviceInfo {
self.addr
}
fn irq(&self) -> u32 {
self.irqs[0]
self.irq.unwrap().into()
}
fn length(&self) -> u64 {
self.len
Expand Down Expand Up @@ -555,11 +558,10 @@ mod tests {
#[cfg(target_arch = "x86_64")]
/// Gets the number of interrupts used by the devices registered.
pub fn used_irqs_count(&self) -> usize {
let mut irq_number = 0;
self.get_device_info()
.iter()
.for_each(|(_, device_info)| irq_number += device_info.irqs.len());
irq_number
.filter(|(_, device_info)| device_info.irq.is_some())
.count()
}
}

Expand Down Expand Up @@ -762,7 +764,10 @@ mod tests {
);
assert_eq!(
crate::arch::IRQ_BASE,
device_manager.id_to_dev_info[&(DeviceType::Virtio(type_id), id)].irqs[0]
device_manager.id_to_dev_info[&(DeviceType::Virtio(type_id), id)]
.irq
.unwrap()
.get()
);

let id = "bar";
Expand Down Expand Up @@ -799,50 +804,39 @@ mod tests {
}

#[test]
fn test_slot_irq_allocation() {
fn test_no_irq_allocation() {
let mut device_manager = MMIODeviceManager::new();
let mut resource_allocator = ResourceAllocator::new().unwrap();

let device_info = device_manager
.allocate_mmio_resources(&mut resource_allocator, 0)
.unwrap();
assert_eq!(device_info.irqs.len(), 0);
assert!(device_info.irq.is_none());
}

#[test]
fn test_irq_allocation() {
let mut device_manager = MMIODeviceManager::new();
let mut resource_allocator = ResourceAllocator::new().unwrap();

let device_info = device_manager
.allocate_mmio_resources(&mut resource_allocator, 1)
.unwrap();
assert_eq!(device_info.irqs[0], crate::arch::IRQ_BASE);
assert_eq!(
format!(
"{}",
device_manager
.allocate_mmio_resources(
&mut resource_allocator,
crate::arch::IRQ_MAX - crate::arch::IRQ_BASE + 1
)
.unwrap_err()
),
"Failed to allocate requested resource: The requested resource is not available."
.to_string()
);
assert_eq!(device_info.irq.unwrap().get(), crate::arch::IRQ_BASE);
}

let device_info = device_manager
.allocate_mmio_resources(
&mut resource_allocator,
crate::arch::IRQ_MAX - crate::arch::IRQ_BASE - 1,
)
.unwrap();
assert_eq!(device_info.irqs[16], crate::arch::IRQ_BASE + 17);
#[test]
fn test_allocation_failure() {
let mut device_manager = MMIODeviceManager::new();
let mut resource_allocator = ResourceAllocator::new().unwrap();
assert_eq!(
format!(
"{}",
device_manager
.allocate_mmio_resources(&mut resource_allocator, 2)
.unwrap_err()
),
"Failed to allocate requested resource: The requested resource is not available."
.to_string()
"Invalid MMIO IRQ configuration.".to_string()
);
device_manager
.allocate_mmio_resources(&mut resource_allocator, 0)
.unwrap();
}
}

0 comments on commit 69b8c2c

Please sign in to comment.