diff --git a/vmm/src/interrupt.rs b/vmm/src/interrupt.rs index 80ddd46ae..f08aaab7f 100644 --- a/vmm/src/interrupt.rs +++ b/vmm/src/interrupt.rs @@ -20,32 +20,49 @@ use vmm_sys_util::eventfd::EventFd; type Result = std::io::Result; struct InterruptRoute { - gsi: u32, + gsi: Option, irq_fd: Option, registered: bool, } impl InterruptRoute { - fn new(allocator: &mut SystemAllocator) -> Result { - Self::new_with_fd(allocator, Some(EventFd::new(libc::EFD_NONBLOCK)?)) + fn new() -> Result { + // The irq_fd must be created eagerly because external components + // (say, VFIO) need the fd at device initialization time via notifier(). + Self::new_with_fd(Some(EventFd::new(libc::EFD_NONBLOCK)?)) } - fn new_with_fd(allocator: &mut SystemAllocator, irq_fd: Option) -> Result { - let gsi = allocator - .allocate_gsi() - .ok_or_else(|| io::Error::other("Failed allocating new GSI"))?; - + fn new_with_fd(irq_fd: Option) -> Result { Ok(InterruptRoute { - gsi, + gsi: None, irq_fd, registered: false, }) } + fn allocate_gsi(&mut self, allocator: &mut SystemAllocator) -> Result { + match self.gsi { + Some(existing) => Ok(existing), + None => { + let new_gsi = allocator + .allocate_gsi() + .ok_or_else(|| io::Error::other("Failed allocating new GSI"))?; + self.gsi = Some(new_gsi); + Ok(new_gsi) + } + } + } + fn enable(&mut self, vm: &dyn hypervisor::Vm) -> Result<()> { + let gsi = match self.gsi { + Some(gsi) => gsi, + // Do nothing if no GSI was ever allocated for this route, which means the interrupt is still masked. + None => return Ok(()), + }; + if !self.registered { if let Some(ref irq_fd) = self.irq_fd { - vm.register_irqfd(irq_fd, self.gsi) + vm.register_irqfd(irq_fd, gsi) .map_err(|e| io::Error::other(format!("Failed registering irq_fd: {e}")))?; } @@ -57,9 +74,15 @@ impl InterruptRoute { } fn disable(&mut self, vm: &dyn hypervisor::Vm) -> Result<()> { + let gsi = match self.gsi { + Some(gsi) => gsi, + // Do nothing if no GSI was ever allocated for this route, which means the interrupt is still masked. + None => return Ok(()), + }; + if self.registered { if let Some(ref irq_fd) = self.irq_fd { - vm.unregister_irqfd(irq_fd, self.gsi) + vm.unregister_irqfd(irq_fd, gsi) .map_err(|e| io::Error::other(format!("Failed unregistering irq_fd: {e}")))?; } @@ -93,14 +116,17 @@ impl InterruptRoute { fn set_notifier(&mut self, eventfd: Option, vm: &dyn hypervisor::Vm) -> Result<()> { let old_irqfd = core::mem::replace(&mut self.irq_fd, eventfd); if self.registered { + // A registered route must have a GSI allocated, since enable() + // only sets registered=true after using a valid GSI. + let gsi = self.gsi.expect("registered route has no GSI allocated"); if let Some(ref irq_fd) = self.irq_fd { - vm.register_irqfd(irq_fd, self.gsi) + vm.register_irqfd(irq_fd, gsi) .map_err(|e| io::Error::other(format!("Failed registering irq_fd: {e}")))?; } // If the irqfd cannot be unregistered, what to do? Spin? // Returning an error isn't helpful as the new irqfd is already registered. if let Some(old_irq_fd) = old_irqfd { - match vm.unregister_irqfd(&old_irq_fd, self.gsi) { + match vm.unregister_irqfd(&old_irq_fd, gsi) { Ok(()) => {} Err(e) => log::warn!("Failed unregistering old irqfd: {e}"), } @@ -119,6 +145,7 @@ struct MsiInterruptGroup { vm: Arc, gsi_msi_routes: Arc>>, irq_routes: HashMap>, + allocator: Arc>, } impl MsiInterruptGroup { @@ -126,11 +153,13 @@ impl MsiInterruptGroup { vm: Arc, gsi_msi_routes: Arc>>, irq_routes: HashMap>, + allocator: Arc>, ) -> Self { MsiInterruptGroup { vm, gsi_msi_routes, irq_routes, + allocator, } } @@ -194,8 +223,20 @@ impl InterruptSourceGroup for MsiInterruptGroup { ) -> Result<()> { if let Some(route) = self.irq_routes.get(&index) { let mut route = route.lock().unwrap(); + let gsi = if masked { + match route.gsi { + Some(gsi) => gsi, + // No update needed if masked and no GSI was ever allocated + None => return Ok(()), + } + } else { + // Allocate a GSI when the interrupt vector is first unmasked + let mut allocator = self.allocator.lock().unwrap(); + route.allocate_gsi(&mut allocator)? + }; + let entry = RoutingEntry { - route: self.vm.make_routing_entry(route.gsi, &config), + route: self.vm.make_routing_entry(gsi, &config), masked, }; @@ -208,7 +249,7 @@ impl InterruptSourceGroup for MsiInterruptGroup { } let mut routes = self.gsi_msi_routes.lock().unwrap(); - routes.insert(route.gsi, entry); + routes.insert(gsi, entry); if set_gsi { self.set_gsi_routes(&routes)?; } @@ -338,17 +379,17 @@ impl MsiInterruptManager { &self, config: ::GroupConfig, ) -> Result { - let mut allocator = self.allocator.lock().unwrap(); let mut irq_routes: HashMap> = HashMap::with_capacity(config.count as usize); for i in config.base..config.base + config.count { - irq_routes.insert(i, Mutex::new(InterruptRoute::new(&mut allocator)?)); + irq_routes.insert(i, Mutex::new(InterruptRoute::new()?)); } Ok(MsiInterruptGroup::new( self.vm.clone(), self.gsi_msi_routes.clone(), irq_routes, + self.allocator.clone(), )) } } @@ -357,17 +398,17 @@ impl InterruptManager for MsiInterruptManager { type GroupConfig = MsiIrqGroupConfig; fn create_group(&self, config: Self::GroupConfig) -> Result> { - let mut allocator = self.allocator.lock().unwrap(); let mut irq_routes: HashMap> = HashMap::with_capacity(config.count as usize); for i in config.base..config.base + config.count { - irq_routes.insert(i, Mutex::new(InterruptRoute::new(&mut allocator)?)); + irq_routes.insert(i, Mutex::new(InterruptRoute::new()?)); } Ok(Arc::new(MsiInterruptGroup::new( self.vm.clone(), self.gsi_msi_routes.clone(), irq_routes, + self.allocator.clone(), ))) }