From 0149e650817fc65321ab5a19bee9e4b2d6c883bb Mon Sep 17 00:00:00 2001 From: Yong He Date: Tue, 1 Aug 2023 14:41:23 +0800 Subject: [PATCH] vm-device: support batch update interrupt source group GSI Split interrupt source group restore into two steps, first restore the irqfd for each interrupt source entry, and second restore the GSI routing of the entire interrupt source group. This patch will reduce restore latency of interrupt source group, and in a 200-concurrent restore test, the patch reduced the average IOAPIC restore time from 15ms to 1ms. Signed-off-by: Yong He --- devices/src/gic.rs | 5 +++++ devices/src/ioapic.rs | 12 +++++++++--- devices/src/legacy/gpio_pl061.rs | 5 +++++ devices/src/legacy/rtc_pl031.rs | 5 +++++ devices/src/legacy/serial.rs | 4 ++++ devices/src/legacy/uart_pl011.rs | 4 ++++ fuzz/fuzz_targets/serial.rs | 4 ++++ pci/src/msi.rs | 6 ++++++ pci/src/msix.rs | 3 +++ vm-device/src/interrupt/mod.rs | 5 +++++ vmm/src/interrupt.rs | 17 ++++++++++++++++- 11 files changed, 66 insertions(+), 4 deletions(-) diff --git a/devices/src/gic.rs b/devices/src/gic.rs index b8c7ceb70..4d012a5e0 100644 --- a/devices/src/gic.rs +++ b/devices/src/gic.rs @@ -98,9 +98,14 @@ impl Gic { i as InterruptIndex, InterruptSourceConfig::LegacyIrq(config), false, + false, ) .map_err(Error::EnableInterrupt)?; } + + self.interrupt_source_group + .set_gsi() + .map_err(Error::EnableInterrupt)?; Ok(()) } diff --git a/devices/src/ioapic.rs b/devices/src/ioapic.rs index e9aacc62d..687979268 100644 --- a/devices/src/ioapic.rs +++ b/devices/src/ioapic.rs @@ -237,9 +237,14 @@ impl Ioapic { if state.is_some() { for (irq, entry) in ioapic.used_entries.iter().enumerate() { if *entry { - ioapic.update_entry(irq)?; + ioapic.update_entry(irq, false)?; } } + + ioapic + .interrupt_source_group + .set_gsi() + .map_err(Error::UpdateInterrupt)?; } Ok(ioapic) @@ -278,7 +283,7 @@ impl Ioapic { } // The entry must be updated through the interrupt source // group. - if let Err(e) = self.update_entry(index) { + if let Err(e) = self.update_entry(index, true) { error!("Failed updating IOAPIC entry: {:?}", e); } // Store the information this IRQ is now being used. @@ -329,7 +334,7 @@ impl Ioapic { } } - fn update_entry(&self, irq: usize) -> Result<()> { + fn update_entry(&self, irq: usize, set_gsi: bool) -> Result<()> { let entry = self.reg_entries[irq]; // Validate Destination Mode value, and retrieve Destination ID @@ -386,6 +391,7 @@ impl Ioapic { irq as InterruptIndex, InterruptSourceConfig::MsiIrq(config), interrupt_mask(entry) == 1, + set_gsi, ) .map_err(Error::UpdateInterrupt)?; diff --git a/devices/src/legacy/gpio_pl061.rs b/devices/src/legacy/gpio_pl061.rs index ed52cad60..5259e71da 100644 --- a/devices/src/legacy/gpio_pl061.rs +++ b/devices/src/legacy/gpio_pl061.rs @@ -361,10 +361,15 @@ mod tests { _index: InterruptIndex, _config: InterruptSourceConfig, _masked: bool, + _set_gsi: bool, ) -> result::Result<(), std::io::Error> { Ok(()) } + fn set_gsi(&self) -> result::Result<(), std::io::Error> { + Ok(()) + } + fn notifier(&self, _index: InterruptIndex) -> Option { Some(self.event_fd.try_clone().unwrap()) } diff --git a/devices/src/legacy/rtc_pl031.rs b/devices/src/legacy/rtc_pl031.rs index b9bd627bf..fd94aa7fc 100644 --- a/devices/src/legacy/rtc_pl031.rs +++ b/devices/src/legacy/rtc_pl031.rs @@ -413,10 +413,15 @@ mod tests { _index: InterruptIndex, _config: InterruptSourceConfig, _masked: bool, + _set_gsi: bool, ) -> result::Result<(), std::io::Error> { Ok(()) } + fn set_gsi(&self) -> result::Result<(), std::io::Error> { + Ok(()) + } + fn notifier(&self, _index: InterruptIndex) -> Option { Some(self.event_fd.try_clone().unwrap()) } diff --git a/devices/src/legacy/serial.rs b/devices/src/legacy/serial.rs index 5756f63ea..b1c14bae9 100644 --- a/devices/src/legacy/serial.rs +++ b/devices/src/legacy/serial.rs @@ -365,9 +365,13 @@ mod tests { _index: InterruptIndex, _config: InterruptSourceConfig, _masked: bool, + _set_gsi: bool, ) -> result::Result<(), std::io::Error> { Ok(()) } + fn set_gsi(&self) -> result::Result<(), std::io::Error> { + Ok(()) + } fn notifier(&self, _index: InterruptIndex) -> Option { Some(self.event_fd.try_clone().unwrap()) } diff --git a/devices/src/legacy/uart_pl011.rs b/devices/src/legacy/uart_pl011.rs index 538e0e404..cb4285ace 100644 --- a/devices/src/legacy/uart_pl011.rs +++ b/devices/src/legacy/uart_pl011.rs @@ -485,9 +485,13 @@ mod tests { _index: InterruptIndex, _config: InterruptSourceConfig, _masked: bool, + _set_gsi: bool, ) -> result::Result<(), std::io::Error> { Ok(()) } + fn set_gsi(&self) -> result::Result<(), std::io::Error> { + Ok(()) + } fn notifier(&self, _index: InterruptIndex) -> Option { Some(self.event_fd.try_clone().unwrap()) } diff --git a/fuzz/fuzz_targets/serial.rs b/fuzz/fuzz_targets/serial.rs index b8a17ad80..45f0618b0 100644 --- a/fuzz/fuzz_targets/serial.rs +++ b/fuzz/fuzz_targets/serial.rs @@ -59,9 +59,13 @@ impl InterruptSourceGroup for TestInterrupt { _index: InterruptIndex, _config: InterruptSourceConfig, _masked: bool, + _set_gsi: bool, ) -> Result<(), std::io::Error> { Ok(()) } + fn set_gsi(&self) -> Result<(), std::io::Error> { + Ok(()) + } fn notifier(&self, _index: InterruptIndex) -> Option { Some(self.event_fd.try_clone().unwrap()) } diff --git a/pci/src/msi.rs b/pci/src/msi.rs index 850016df1..58f3d9acd 100644 --- a/pci/src/msi.rs +++ b/pci/src/msi.rs @@ -205,10 +205,15 @@ impl MsiConfig { idx as InterruptIndex, InterruptSourceConfig::MsiIrq(config), state.cap.vector_masked(idx), + false, ) .map_err(Error::UpdateInterruptRoute)?; } + interrupt_source_group + .set_gsi() + .map_err(Error::EnableInterruptRoute)?; + interrupt_source_group .enable() .map_err(Error::EnableInterruptRoute)?; @@ -262,6 +267,7 @@ impl MsiConfig { idx as InterruptIndex, InterruptSourceConfig::MsiIrq(config), self.cap.vector_masked(idx), + true, ) { error!("Failed updating vector: {:?}", e); } diff --git a/pci/src/msix.rs b/pci/src/msix.rs index 81790ebee..22c282cf5 100644 --- a/pci/src/msix.rs +++ b/pci/src/msix.rs @@ -107,6 +107,7 @@ impl MsixConfig { idx as InterruptIndex, InterruptSourceConfig::MsiIrq(config), state.masked, + true, ) .map_err(Error::UpdateInterruptRoute)?; @@ -182,6 +183,7 @@ impl MsixConfig { idx as InterruptIndex, InterruptSourceConfig::MsiIrq(config), table_entry.masked(), + true, ) { error!("Failed updating vector: {:?}", e); } @@ -320,6 +322,7 @@ impl MsixConfig { index as InterruptIndex, InterruptSourceConfig::MsiIrq(config), table_entry.masked(), + true, ) { error!("Failed updating vector: {:?}", e); } diff --git a/vm-device/src/interrupt/mod.rs b/vm-device/src/interrupt/mod.rs index f4f7ebee8..9470f0fb0 100644 --- a/vm-device/src/interrupt/mod.rs +++ b/vm-device/src/interrupt/mod.rs @@ -147,10 +147,15 @@ pub trait InterruptSourceGroup: Send + Sync { /// * index: sub-index into the group. /// * config: configuration data for the interrupt source. /// * masked: if the interrupt is masked + /// * set_gsi: whehter update the GSI routing table. fn update( &self, index: InterruptIndex, config: InterruptSourceConfig, masked: bool, + set_gsi: bool, ) -> Result<()>; + + /// Set the interrupt group GSI routing table. + fn set_gsi(&self) -> Result<()>; } diff --git a/vmm/src/interrupt.rs b/vmm/src/interrupt.rs index a9cc16f8a..d6a68081b 100644 --- a/vmm/src/interrupt.rs +++ b/vmm/src/interrupt.rs @@ -170,6 +170,7 @@ impl InterruptSourceGroup for MsiInterruptGroup { index: InterruptIndex, config: InterruptSourceConfig, masked: bool, + set_gsi: bool, ) -> Result<()> { if let Some(route) = self.irq_routes.get(&index) { let entry = RoutingEntry { @@ -183,7 +184,11 @@ impl InterruptSourceGroup for MsiInterruptGroup { } let mut routes = self.gsi_msi_routes.lock().unwrap(); routes.insert(route.gsi, entry); - return self.set_gsi_routes(&routes); + if set_gsi { + return self.set_gsi_routes(&routes); + } else { + return Ok(()); + } } Err(io::Error::new( @@ -191,6 +196,11 @@ impl InterruptSourceGroup for MsiInterruptGroup { format!("update: Invalid interrupt index {index}"), )) } + + fn set_gsi(&self) -> Result<()> { + let routes = self.gsi_msi_routes.lock().unwrap(); + self.set_gsi_routes(&routes) + } } pub struct LegacyUserspaceInterruptGroup { @@ -223,10 +233,15 @@ impl InterruptSourceGroup for LegacyUserspaceInterruptGroup { _index: InterruptIndex, _config: InterruptSourceConfig, _masked: bool, + _set_gsi: bool, ) -> Result<()> { Ok(()) } + fn set_gsi(&self) -> Result<()> { + Ok(()) + } + fn notifier(&self, _index: InterruptIndex) -> Option { self.ioapic.lock().unwrap().notifier(self.irq as usize) }