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 <[email protected]>
Signed-off-by: Patrick Roy <[email protected]>
  • Loading branch information
@andr3wy @roypat
andr3wy authored and roypat committed Jul 15, 2024
1 parent de48fd6 commit 810402e
Showing 1 changed file with 43 additions and 47 deletions.
90 changes: 43 additions & 47 deletions src/vmm/src/device_manager/mmio.rs
View file
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(),
)
.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,10 +558,11 @@ 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()
let irq_number = self
.get_device_info()
.iter()
.for_each(|(_, device_info)| irq_number += device_info.irqs.len());
.filter(|(_, device_info)| device_info.irq.is_some())
.count();
irq_number
}
}
Expand Down Expand Up @@ -762,7 +766,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 +806,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 810402e

Please sign in to comment.