From c9fe5ad0f6967a9fa55644c9b4a9654a629f4983 Mon Sep 17 00:00:00 2001 From: Andrew Yao Date: Tue, 7 May 2024 11:25:44 -0500 Subject: [PATCH] --Utilized Option instead of Vector to store IRQ lines for MMIO devices. -Add test to test this. -Update existing tests --- src/vmm/src/device_manager/mmio.rs | 85 +++++++++++++++--------------- 1 file changed, 42 insertions(+), 43 deletions(-) diff --git a/src/vmm/src/device_manager/mmio.rs b/src/vmm/src/device_manager/mmio.rs index 68dfa3dfc8d6..40fa3093ade3 100644 --- a/src/vmm/src/device_manager/mmio.rs +++ b/src/vmm/src/device_manager/mmio.rs @@ -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")] @@ -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, + /// Used Irq line for the device. + pub irq: Option, // NOTE: guaranteed to be a value not 0, 0 is not allowed } #[cfg(target_arch = "x86_64")] @@ -142,7 +143,12 @@ impl MMIODeviceManager { resource_allocator: &mut ResourceAllocator, irq_count: u32, ) -> Result { - 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, @@ -150,7 +156,7 @@ impl MMIODeviceManager { AllocPolicy::FirstMatch, )?, len: MMIO_LEN, - irqs, + irq, }; Ok(device_info) } @@ -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(); @@ -193,7 +199,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_evt(), device_info.irqs[0]) + vm.register_irqfd(locked_device.interrupt_evt(), irq.get()) .map_err(MmioError::RegisterIrqFd)?; } @@ -219,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) @@ -246,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) @@ -278,7 +284,7 @@ impl MMIODeviceManager { .unwrap() .serial .interrupt_evt(), - device_info.irqs[0], + device_info.irq.unwrap().get(), ) .map_err(MmioError::RegisterIrqFd)?; @@ -504,7 +510,7 @@ impl DeviceInfoForFDT for MMIODeviceInfo { self.addr } fn irq(&self) -> u32 { - self.irqs[0] + self.irq.unwrap() } fn length(&self) -> u64 { self.len @@ -552,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()); + .map(|(_, device_info)| device_info.irq.is_some()) + .count(); irq_number } } @@ -763,7 +770,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"; @@ -800,38 +810,31 @@ 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!( "{}", @@ -839,11 +842,7 @@ mod tests { .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(); } }