From 90b9481745d42da8be015a6c9f6d86e59062639c Mon Sep 17 00:00:00 2001 From: Anatol Belski Date: Sun, 10 May 2026 00:37:54 +0200 Subject: [PATCH] virtio-devices: Bump config_generation on device config read Set a config_changed flag in VirtioInterruptMsix when a Config interrupt fires, and increment config_generation only when the driver next reads the device specific configuration region. The flag is cleared by that read so the driver observes a stable value across the read and a fresh value on any later read. This avoids the wrap hazard of incrementing on every Config event, where a burst of interrupts could roll the 8 bit counter back to its previous value between two driver reads. Signed-off-by: Anatol Belski --- .../src/transport/pci_common_config.rs | 22 ++++++++++++++++++- virtio-devices/src/transport/pci_device.rs | 11 ++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/virtio-devices/src/transport/pci_common_config.rs b/virtio-devices/src/transport/pci_common_config.rs index 8098ba0eb..7e9e5456e 100644 --- a/virtio-devices/src/transport/pci_common_config.rs +++ b/virtio-devices/src/transport/pci_common_config.rs @@ -6,7 +6,7 @@ // // SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause -use std::sync::atomic::{AtomicU8, AtomicU16, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicU8, AtomicU16, Ordering}; use std::sync::{Arc, Mutex}; use byteorder::{ByteOrder, LittleEndian}; @@ -127,6 +127,10 @@ pub struct VirtioPciCommonConfig { pub device: Arc>, pub driver_status: Arc, pub config_generation: Arc, + /// Set when a Config interrupt fires. Cleared on the next read of + /// the device specific configuration region, which also bumps + /// config_generation. + pub config_changed: Arc, pub device_feature_select: u32, pub driver_feature_select: u32, pub queue_select: u16, @@ -140,6 +144,7 @@ impl VirtioPciCommonConfig { device, driver_status: Arc::new(AtomicU8::new(state.driver_status)), config_generation: Arc::new(AtomicU8::new(state.config_generation)), + config_changed: Arc::new(AtomicBool::new(false)), device_feature_select: state.device_feature_select, driver_feature_select: state.driver_feature_select, queue_select: state.queue_select, @@ -148,6 +153,16 @@ impl VirtioPciCommonConfig { } } + /// If a Config interrupt has fired since the last device specific + /// configuration read, increment config_generation and clear the + /// pending flag. + pub fn consume_config_change(&self) { + if self.config_changed.swap(false, Ordering::AcqRel) { + // Wrap at u8 max is intentional per the virtio spec. + self.config_generation.fetch_add(1, Ordering::Release); + } + } + fn state(&self) -> VirtioPciCommonConfigState { VirtioPciCommonConfigState { driver_status: self.driver_status.load(Ordering::Acquire), @@ -165,6 +180,7 @@ impl VirtioPciCommonConfig { /// observe. pub fn reset(&mut self) { self.driver_status.store(0, Ordering::Release); + self.config_changed.store(false, Ordering::Release); self.device_feature_select = 0; self.driver_feature_select = 0; self.queue_select = 0; @@ -462,6 +478,7 @@ mod unit_tests { device: dev.clone(), driver_status: Arc::new(AtomicU8::new(0xaa)), config_generation: Arc::new(AtomicU8::new(0x55)), + config_changed: Arc::new(AtomicBool::new(false)), device_feature_select: 0x0, driver_feature_select: 0x0, queue_select: 0xff, @@ -514,6 +531,7 @@ mod unit_tests { device: dev.clone(), driver_status: Arc::new(AtomicU8::new(0)), config_generation: Arc::new(AtomicU8::new(0)), + config_changed: Arc::new(AtomicBool::new(false)), device_feature_select: 0, driver_feature_select: 0, queue_select: 0, @@ -542,6 +560,7 @@ mod unit_tests { device: dev, driver_status: Arc::new(AtomicU8::new(0x55)), config_generation: Arc::new(AtomicU8::new(0xab)), + config_changed: Arc::new(AtomicBool::new(true)), device_feature_select: 1, driver_feature_select: 1, queue_select: 7, @@ -553,6 +572,7 @@ mod unit_tests { assert_eq!(regs.driver_status.load(Ordering::Acquire), 0); assert_eq!(regs.config_generation.load(Ordering::Acquire), 0xab); // unchanged across reset + assert!(!regs.config_changed.load(Ordering::Acquire)); assert_eq!(regs.device_feature_select, 0); assert_eq!(regs.driver_feature_select, 0); assert_eq!(regs.queue_select, 0); diff --git a/virtio-devices/src/transport/pci_device.rs b/virtio-devices/src/transport/pci_device.rs index 8784a4f8d..8e350428e 100644 --- a/virtio-devices/src/transport/pci_device.rs +++ b/virtio-devices/src/transport/pci_device.rs @@ -591,6 +591,7 @@ impl VirtioPciDevice { let virtio_interrupt = Arc::new(VirtioInterruptMsix::new( msix_config.clone(), common_config.msix_config.clone(), + common_config.config_changed.clone(), common_config.msix_queues.clone(), interrupt_source_group.clone(), )); @@ -851,6 +852,7 @@ impl VirtioTransport for VirtioPciDevice { pub struct VirtioInterruptMsix { msix_config: Arc>, config_vector: Arc, + config_changed: Arc, queues_vectors: Arc>>, interrupt_source_group: MaybeMutInterruptSourceGroup, msix_table_size: usize, @@ -860,6 +862,7 @@ impl VirtioInterruptMsix { pub fn new( msix_config: Arc>, config_vector: Arc, + config_changed: Arc, queues_vectors: Arc>>, interrupt_source_group: MaybeMutInterruptSourceGroup, ) -> Self { @@ -867,6 +870,7 @@ impl VirtioInterruptMsix { VirtioInterruptMsix { msix_config, config_vector, + config_changed, queues_vectors, interrupt_source_group, msix_table_size, @@ -876,6 +880,10 @@ impl VirtioInterruptMsix { impl VirtioInterrupt for VirtioInterruptMsix { fn trigger(&self, int_type: VirtioInterruptType) -> std::result::Result<(), std::io::Error> { + if matches!(int_type, VirtioInterruptType::Config) { + self.config_changed.store(true, Ordering::Release); + } + let vector = match int_type { VirtioInterruptType::Config => self.config_vector.load(Ordering::Acquire), VirtioInterruptType::Queue(queue_index) => { @@ -1159,6 +1167,7 @@ impl PciDevice for VirtioPciDevice { o if (DEVICE_CONFIG_BAR_OFFSET..DEVICE_CONFIG_BAR_OFFSET + DEVICE_CONFIG_SIZE) .contains(&o) => { + self.common_config.consume_config_change(); let device = self.device.lock().unwrap(); device.read_config(o - DEVICE_CONFIG_BAR_OFFSET, data); } @@ -1362,11 +1371,13 @@ mod unit_tests { .unwrap(), )); let config_vector = Arc::new(AtomicU16::new(VIRTQ_MSI_NO_VECTOR)); + let config_changed = Arc::new(AtomicBool::new(false)); let queues_vectors = Arc::new(Mutex::new(vec![VIRTQ_MSI_NO_VECTOR; 1])); VirtioInterruptMsix::new( msix_config, config_vector, + config_changed, queues_vectors, MaybeMutInterruptSourceGroup::Immutable(isg), )