From cc6eee4de8b26b72519309a188c0deae654d6e4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=87a=C4=9Fatay=20Yi=C4=9Fit=20=C5=9Eahin?= Date: Fri, 5 Jun 2026 15:26:04 +0200 Subject: [PATCH] fix(virtio): do not fail on device configuration change if possible Some of the device configuration change interrupts are only relevant if configuration values are cached by the driver. However, we do not make use of any such configurations. Thus, only panic if the interrupt is sent because the device needs reset, which we currently cannot handle and must not ignore. --- src/drivers/console/mod.rs | 17 +++++++---------- src/drivers/fs/mod.rs | 17 +++++++---------- src/drivers/net/virtio/mod.rs | 17 +++++++---------- src/drivers/virtio/transport/mmio.rs | 8 ++++++++ src/drivers/virtio/transport/pci.rs | 7 +++++++ src/drivers/vsock/mod.rs | 17 +++++++---------- 6 files changed, 43 insertions(+), 40 deletions(-) diff --git a/src/drivers/console/mod.rs b/src/drivers/console/mod.rs index 01b0f731e5..798f25d383 100644 --- a/src/drivers/console/mod.rs +++ b/src/drivers/console/mod.rs @@ -287,16 +287,13 @@ impl VirtioConsoleDriver { pub fn handle_interrupt(&mut self) { let status = self.isr_stat.acknowledge(); - #[cfg(not(feature = "pci"))] - if status.contains(virtio::mmio::InterruptStatus::CONFIGURATION_CHANGE_NOTIFICATION) { - info!("Configuration changes are not possible! Aborting"); - todo!("Implement possibility to change config on the fly...") - } - - #[cfg(feature = "pci")] - if status.contains(virtio::pci::IsrStatus::DEVICE_CONFIGURATION_INTERRUPT) { - info!("Configuration changes are not possible! Aborting"); - todo!("Implement possibility to change config on the fly...") + if status.contains(cfg_select! { + feature = "pci" => virtio::pci::IsrStatus::DEVICE_CONFIGURATION_INTERRUPT, + _ => virtio::mmio::InterruptStatus::CONFIGURATION_CHANGE_NOTIFICATION, + }) && self.com_cfg.does_device_need_reset() + { + info!("Device resets are not possible! Aborting"); + todo!("Implement possibility to reset device on the fly...") } crate::console::CONSOLE_WAKER.lock().wake(); diff --git a/src/drivers/fs/mod.rs b/src/drivers/fs/mod.rs index cebad3e144..b3dd70ded0 100644 --- a/src/drivers/fs/mod.rs +++ b/src/drivers/fs/mod.rs @@ -161,16 +161,13 @@ impl VirtioFsDriver { pub fn handle_interrupt(&mut self) { let status = self.isr_stat.acknowledge(); - #[cfg(not(feature = "pci"))] - if status.contains(virtio::mmio::InterruptStatus::CONFIGURATION_CHANGE_NOTIFICATION) { - info!("Configuration changes are not possible! Aborting"); - todo!("Implement possibility to change config on the fly...") - } - - #[cfg(feature = "pci")] - if status.contains(virtio::pci::IsrStatus::DEVICE_CONFIGURATION_INTERRUPT) { - info!("Configuration changes are not possible! Aborting"); - todo!("Implement possibility to change config on the fly...") + if status.contains(cfg_select! { + feature = "pci" => virtio::pci::IsrStatus::DEVICE_CONFIGURATION_INTERRUPT, + _ => virtio::mmio::InterruptStatus::CONFIGURATION_CHANGE_NOTIFICATION, + }) && self.com_cfg.does_device_need_reset() + { + info!("Device resets are not possible! Aborting"); + todo!("Implement possibility to reset device on the fly...") } } } diff --git a/src/drivers/net/virtio/mod.rs b/src/drivers/net/virtio/mod.rs index 03fc66b399..4e6a0da0a9 100644 --- a/src/drivers/net/virtio/mod.rs +++ b/src/drivers/net/virtio/mod.rs @@ -522,16 +522,13 @@ impl NetworkDriver for VirtioNetDriver { fn handle_interrupt(&mut self) { let status = self.isr_stat.acknowledge(); - #[cfg(not(feature = "pci"))] - if status.contains(virtio::mmio::InterruptStatus::CONFIGURATION_CHANGE_NOTIFICATION) { - info!("Configuration changes are not possible! Aborting"); - todo!("Implement possibility to change config on the fly...") - } - - #[cfg(feature = "pci")] - if status.contains(virtio::pci::IsrStatus::DEVICE_CONFIGURATION_INTERRUPT) { - info!("Configuration changes are not possible! Aborting"); - todo!("Implement possibility to change config on the fly...") + if status.contains(cfg_select! { + feature = "pci" => virtio::pci::IsrStatus::DEVICE_CONFIGURATION_INTERRUPT, + _ => virtio::mmio::InterruptStatus::CONFIGURATION_CHANGE_NOTIFICATION, + }) && self.com_cfg.does_device_need_reset() + { + info!("Device resets are not possible! Aborting"); + todo!("Implement possibility to reset device on the fly...") } wake_network_waker(); diff --git a/src/drivers/virtio/transport/mmio.rs b/src/drivers/virtio/transport/mmio.rs index 27f2147da4..1a0f81dff6 100644 --- a/src/drivers/virtio/transport/mmio.rs +++ b/src/drivers/virtio/transport/mmio.rs @@ -208,6 +208,14 @@ impl ComCfg { .update(|status| status | DeviceStatus::DRIVER_OK); } + pub fn does_device_need_reset(&self) -> bool { + self.com_cfg + .as_ptr() + .status() + .read() + .contains(DeviceStatus::DEVICE_NEEDS_RESET) + } + pub fn print_information(&mut self) { let ptr = self.com_cfg.as_ptr(); diff --git a/src/drivers/virtio/transport/pci.rs b/src/drivers/virtio/transport/pci.rs index babe017063..b434f2a1c7 100644 --- a/src/drivers/virtio/transport/pci.rs +++ b/src/drivers/virtio/transport/pci.rs @@ -361,6 +361,11 @@ impl ComCfg { .device_status() .update(|s| s | DeviceStatus::DRIVER_OK); } + + pub fn does_device_need_reset(&self) -> bool { + let status = self.com_cfg.as_ptr().device_status().read(); + status.contains(DeviceStatus::DEVICE_NEEDS_RESET) + } } /// Notification Structure to handle virtqueue notification settings. @@ -483,6 +488,8 @@ impl IsrStatus { } pub fn acknowledge(&mut self) -> IsrStatusRaw { + //[D]river read of ISR status causes the device to de-assert an interrupt. + // VIRTIO spec. v1.4 sec. 4.1.4.5 self.isr_stat.as_ptr().read() } } diff --git a/src/drivers/vsock/mod.rs b/src/drivers/vsock/mod.rs index 77588be9d1..fd46087171 100644 --- a/src/drivers/vsock/mod.rs +++ b/src/drivers/vsock/mod.rs @@ -308,16 +308,13 @@ impl VirtioVsockDriver { pub fn handle_interrupt(&mut self) { let status = self.isr_stat.acknowledge(); - #[cfg(not(feature = "pci"))] - if status.contains(virtio::mmio::InterruptStatus::CONFIGURATION_CHANGE_NOTIFICATION) { - info!("Configuration changes are not possible! Aborting"); - todo!("Implement possibility to change config on the fly...") - } - - #[cfg(feature = "pci")] - if status.contains(virtio::pci::IsrStatus::DEVICE_CONFIGURATION_INTERRUPT) { - info!("Configuration changes are not possible! Aborting"); - todo!("Implement possibility to change config on the fly...") + if status.contains(cfg_select! { + feature = "pci" => virtio::pci::IsrStatus::DEVICE_CONFIGURATION_INTERRUPT, + _ => virtio::mmio::InterruptStatus::CONFIGURATION_CHANGE_NOTIFICATION, + }) && self.com_cfg.does_device_need_reset() + { + info!("Device resets are not possible! Aborting"); + todo!("Implement possibility to reset device on the fly...") } }