diff --git a/CHANGELOG.md b/CHANGELOG.md index 5977f24e39f..34140cbad71 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ and this project adheres to ### Added +- [#5891](https://github.com/firecracker-microvm/firecracker/pull/5891): Added + support for virtio device reset. - [#5983](https://github.com/firecracker-microvm/firecracker/pull/5983): Add two optional metrics fields, set via `PUT /metrics` or a config file: `emit_id` emits the microVM instance id under a top-level `id` field, and `properties` diff --git a/resources/seccomp/aarch64-unknown-linux-musl.json b/resources/seccomp/aarch64-unknown-linux-musl.json index 1e0047266e6..5a58fd2ccef 100644 --- a/resources/seccomp/aarch64-unknown-linux-musl.json +++ b/resources/seccomp/aarch64-unknown-linux-musl.json @@ -1029,6 +1029,18 @@ { "syscall": "close" }, + { + "syscall": "eventfd2", + "comment": "Needed for recreating the MSI-X vector eventfds when a PCI device is reset by the guest (MsixVectorGroup recreated from the vCPU thread)" + }, + { + "syscall": "io_uring_enter", + "comment": "Needed to drain the async block IO engine's io_uring when the device is reset by the guest (from the vCPU thread)" + }, + { + "syscall": "epoll_ctl", + "comment": "Needed to remove the vsock muxer's connection and listener epoll listeners when the device is reset by the guest (from the vCPU thread)" + }, { "syscall": "fstat", "comment": "Used for reading the local timezone from /etc/localtime" diff --git a/resources/seccomp/x86_64-unknown-linux-musl.json b/resources/seccomp/x86_64-unknown-linux-musl.json index ea4d49e98b5..35fae0db21b 100644 --- a/resources/seccomp/x86_64-unknown-linux-musl.json +++ b/resources/seccomp/x86_64-unknown-linux-musl.json @@ -1041,6 +1041,18 @@ { "syscall": "close" }, + { + "syscall": "eventfd2", + "comment": "Needed for recreating the MSI-X vector eventfds when a PCI device is reset by the guest (MsixVectorGroup recreated from the vCPU thread)" + }, + { + "syscall": "io_uring_enter", + "comment": "Needed to drain the async block IO engine's io_uring when the device is reset by the guest (from the vCPU thread)" + }, + { + "syscall": "epoll_ctl", + "comment": "Needed to remove the vsock muxer's connection and listener epoll listeners when the device is reset by the guest (from the vCPU thread)" + }, { "syscall": "fstat", "comment": "Used for reading the local timezone from /etc/localtime" diff --git a/src/vmm/src/device_manager/mmio.rs b/src/vmm/src/device_manager/mmio.rs index 78b7bbd8c44..fca77ad8014 100644 --- a/src/vmm/src/device_manager/mmio.rs +++ b/src/vmm/src/device_manager/mmio.rs @@ -592,6 +592,12 @@ pub(crate) mod tests { fn is_activated(&self) -> bool { false } + + fn deactivate(&mut self) {} + + fn _reset(&mut self) -> bool { + false + } } #[test] diff --git a/src/vmm/src/devices/virtio/balloon/device.rs b/src/vmm/src/devices/virtio/balloon/device.rs index 7c68f2bd40e..80588a166c7 100644 --- a/src/vmm/src/devices/virtio/balloon/device.rs +++ b/src/vmm/src/devices/virtio/balloon/device.rs @@ -976,6 +976,20 @@ impl VirtioDevice for Balloon { self.device_state.is_activated() } + fn deactivate(&mut self) { + self.device_state = DeviceState::Inactive; + } + + fn _reset(&mut self) -> bool { + self.config_space.actual_pages = 0; + self.config_space.free_page_hint_cmd_id = FREE_PAGE_HINT_STOP; + self.stats_timer.arm(Duration::ZERO, None); + self.stats_desc_index = None; + self.latest_stats = BalloonStats::default(); + self.hinting_state = Default::default(); + true + } + fn kick(&mut self) { if self.is_activated() { if self.free_page_hinting() { diff --git a/src/vmm/src/devices/virtio/balloon/event_handler.rs b/src/vmm/src/devices/virtio/balloon/event_handler.rs index a89cfe8477d..54bb8d1fc2e 100644 --- a/src/vmm/src/devices/virtio/balloon/event_handler.rs +++ b/src/vmm/src/devices/virtio/balloon/event_handler.rs @@ -143,6 +143,7 @@ impl MutEventSubscriber for Balloon { "Balloon: The device is not yet activated. Spurious event received: {:?}", source ); + self.drain_queue_events(); } } diff --git a/src/vmm/src/devices/virtio/block/device.rs b/src/vmm/src/devices/virtio/block/device.rs index 74e5c7ecda7..50a8084448e 100644 --- a/src/vmm/src/devices/virtio/block/device.rs +++ b/src/vmm/src/devices/virtio/block/device.rs @@ -207,6 +207,20 @@ impl VirtioDevice for Block { } } + fn deactivate(&mut self) { + match self { + Self::Virtio(b) => b.deactivate(), + Self::VhostUser(b) => b.deactivate(), + } + } + + fn _reset(&mut self) -> bool { + match self { + Self::Virtio(b) => b._reset(), + Self::VhostUser(b) => b._reset(), + } + } + fn prepare_save(&mut self) { match self { Self::Virtio(b) => b.prepare_save(), diff --git a/src/vmm/src/devices/virtio/block/vhost_user/device.rs b/src/vmm/src/devices/virtio/block/vhost_user/device.rs index 5f16d965954..61c776034e9 100644 --- a/src/vmm/src/devices/virtio/block/vhost_user/device.rs +++ b/src/vmm/src/devices/virtio/block/vhost_user/device.rs @@ -375,6 +375,14 @@ where fn is_activated(&self) -> bool { self.device_state.is_activated() } + + fn deactivate(&mut self) { + self.device_state = DeviceState::Inactive; + } + + fn _reset(&mut self) -> bool { + false + } } #[cfg(test)] diff --git a/src/vmm/src/devices/virtio/block/virtio/device.rs b/src/vmm/src/devices/virtio/block/virtio/device.rs index afdfe6ef812..eba1b3e0f70 100644 --- a/src/vmm/src/devices/virtio/block/virtio/device.rs +++ b/src/vmm/src/devices/virtio/block/virtio/device.rs @@ -670,6 +670,19 @@ impl VirtioDevice for VirtioBlock { fn is_activated(&self) -> bool { self.device_state.is_activated() } + + fn deactivate(&mut self) { + self.device_state = DeviceState::Inactive; + } + + fn _reset(&mut self) -> bool { + if let Err(err) = self.disk.file_engine.drain(true) { + error!("Failed to reset block IO engine: {:?}", err); + return false; + } + self.is_io_engine_throttled = false; + true + } } impl Drop for VirtioBlock { diff --git a/src/vmm/src/devices/virtio/block/virtio/event_handler.rs b/src/vmm/src/devices/virtio/block/virtio/event_handler.rs index 9f02862f814..3db09166978 100644 --- a/src/vmm/src/devices/virtio/block/virtio/event_handler.rs +++ b/src/vmm/src/devices/virtio/block/virtio/event_handler.rs @@ -95,6 +95,18 @@ impl MutEventSubscriber for VirtioBlock { "Block: The device is not yet activated. Spurious event received: {:?}", source ); + match source { + Self::PROCESS_QUEUE => self.drain_queue_events(), + Self::PROCESS_RATE_LIMITER => { + self.rate_limiter.event_handler(); + } + Self::PROCESS_ASYNC_COMPLETION => { + if let FileEngine::Async(ref engine) = self.disk.file_engine { + engine.completion_evt().read(); + } + } + _ => (), + } } } diff --git a/src/vmm/src/devices/virtio/device.rs b/src/vmm/src/devices/virtio/device.rs index 31c70496c04..49bcdc33ffd 100644 --- a/src/vmm/src/devices/virtio/device.rs +++ b/src/vmm/src/devices/virtio/device.rs @@ -194,12 +194,27 @@ pub trait VirtioDevice: AsAny + MutEventSubscriber + Send { /// Checks if the resources of this device are activated. fn is_activated(&self) -> bool; - /// Optionally deactivates this device and returns ownership of the guest memory map, interrupt - /// event, and queue events. - fn reset(&mut self) -> Option<(Arc, Vec)> { - None + /// Set the device state to Inactive + fn deactivate(&mut self); + + /// Reset the device. Returns true on success, false otherwise. + /// It must not be overridden. + fn reset(&mut self) -> bool { + if !self._reset() { + return false; + } + self.deactivate(); + self.set_acked_features(0); + for queue in self.queues_mut() { + *queue = Queue::new(queue.max_size); + } + true } + /// Backend-specific reset logic. Returns true on success, false if the + /// backend does not support reset. + fn _reset(&mut self) -> bool; + /// Mark pages used by queues as dirty. fn mark_queue_memory_dirty(&mut self, mem: &GuestMemoryMmap) -> Result<(), QueueError> { for queue in self.queues_mut() { @@ -224,6 +239,15 @@ pub trait VirtioDevice: AsAny + MutEventSubscriber + Send { } } + /// Drain all queue notification eventfds, discarding any pending + /// notifications. This is used if a notification arrives while a device + /// is being reset and before it's activated again. + fn drain_queue_events(&self) { + for event in self.queue_events() { + event.read(); + } + } + /// Kick the device, as if it had received external events. fn kick(&mut self) { if self.is_activated() { @@ -327,6 +351,14 @@ pub(crate) mod tests { fn is_activated(&self) -> bool { todo!() } + + fn deactivate(&mut self) { + todo!() + } + + fn _reset(&mut self) -> bool { + todo!() + } } #[test] diff --git a/src/vmm/src/devices/virtio/mem/device.rs b/src/vmm/src/devices/virtio/mem/device.rs index fe02479adef..ac01f0246e8 100644 --- a/src/vmm/src/devices/virtio/mem/device.rs +++ b/src/vmm/src/devices/virtio/mem/device.rs @@ -649,6 +649,21 @@ impl VirtioDevice for VirtioMem { self.device_state.is_activated() } + fn deactivate(&mut self) { + self.device_state = DeviceState::Inactive; + } + + fn _reset(&mut self) -> bool { + // Virtio spec, section 5.15.5.2: + // The device MUST NOT change the state of memory blocks during device + // reset. The device MUST NOT modify memory or memory properties of + // plugged memory blocks during device reset. + // + // Note: the Linux virtio-mem driver does not support rebinding when + // memory is plugged + true + } + fn activate( &mut self, mem: GuestMemoryMmap, diff --git a/src/vmm/src/devices/virtio/mem/event_handler.rs b/src/vmm/src/devices/virtio/mem/event_handler.rs index ae81ef12a64..8b3a71845c1 100644 --- a/src/vmm/src/devices/virtio/mem/event_handler.rs +++ b/src/vmm/src/devices/virtio/mem/event_handler.rs @@ -76,6 +76,7 @@ impl MutEventSubscriber for VirtioMem { if !self.is_activated() { warn!("virtio-mem: The device is not activated yet. Spurious event received: {source}"); + self.drain_queue_events(); return; } diff --git a/src/vmm/src/devices/virtio/mod.rs b/src/vmm/src/devices/virtio/mod.rs index b17db5b02ac..ec95368701c 100644 --- a/src/vmm/src/devices/virtio/mod.rs +++ b/src/vmm/src/devices/virtio/mod.rs @@ -67,6 +67,8 @@ pub enum ActivateError { QueueMemoryError(QueueError), /// The driver didn't acknowledge a required feature: {0} RequiredFeatureNotAcked(&'static str), + /// Vsock backend: {0} + VsockBackend(vsock::VsockError), } /// Trait that helps in upcasting an object to Any diff --git a/src/vmm/src/devices/virtio/net/device.rs b/src/vmm/src/devices/virtio/net/device.rs index 2fb6bfaf3ea..15831d06850 100644 --- a/src/vmm/src/devices/virtio/net/device.rs +++ b/src/vmm/src/devices/virtio/net/device.rs @@ -135,6 +135,15 @@ impl RxBuffers { }) } + /// Reset the RX buffers to their initial state. + fn clear(&mut self) { + self.iovec.clear(); + self.parsed_descriptors.clear(); + self.used_descriptors = 0; + self.used_bytes = 0; + self.min_buffer_size = 0; + } + /// Add a new `DescriptorChain` that we received from the RX queue in the buffer. /// /// SAFETY: The `DescriptorChain` cannot be referencing the same memory location as any other @@ -1072,6 +1081,16 @@ impl VirtioDevice for Net { self.device_state.is_activated() } + fn deactivate(&mut self) { + self.device_state = DeviceState::Inactive; + } + + fn _reset(&mut self) -> bool { + self.rx_buffer.clear(); + self.tx_buffer.clear(); + true + } + /// Prepare saving state fn prepare_save(&mut self) { // We shouldn't be messing with the queue if the device is not activated. @@ -2589,4 +2608,16 @@ pub mod tests { assert!(queues[RX_INDEX].uses_notif_suppression); assert!(queues[TX_INDEX].uses_notif_suppression); } + + #[test] + fn test_reset() { + let mem = single_region_mem(2 * MAX_BUFFER_SIZE); + let mut th = TestHelper::get_default(&mem); + th.activate_net(); + + assert!(th.net().is_activated()); + assert!(th.net().reset()); + assert!(!th.net().is_activated()); + assert_eq!(th.net().acked_features(), 0); + } } diff --git a/src/vmm/src/devices/virtio/net/event_handler.rs b/src/vmm/src/devices/virtio/net/event_handler.rs index 9d8c09a45f2..3fe80fef197 100644 --- a/src/vmm/src/devices/virtio/net/event_handler.rs +++ b/src/vmm/src/devices/virtio/net/event_handler.rs @@ -114,6 +114,16 @@ impl MutEventSubscriber for Net { "Net: The device is not yet activated. Spurious event received: {:?}", source ); + match source { + Self::PROCESS_VIRTQ_RX | Self::PROCESS_VIRTQ_TX => self.drain_queue_events(), + Self::PROCESS_RX_RATE_LIMITER => { + self.rx_rate_limiter.event_handler(); + } + Self::PROCESS_TX_RATE_LIMITER => { + self.tx_rate_limiter.event_handler(); + } + _ => (), + } } } diff --git a/src/vmm/src/devices/virtio/pmem/device.rs b/src/vmm/src/devices/virtio/pmem/device.rs index c81bb75d19f..783b59a22f2 100644 --- a/src/vmm/src/devices/virtio/pmem/device.rs +++ b/src/vmm/src/devices/virtio/pmem/device.rs @@ -579,6 +579,14 @@ impl VirtioDevice for Pmem { self.device_state.is_activated() } + fn deactivate(&mut self) { + self.device_state = DeviceState::Inactive; + } + + fn _reset(&mut self) -> bool { + true + } + fn kick(&mut self) { if self.is_activated() { info!("kick pmem {}.", self.config.id); diff --git a/src/vmm/src/devices/virtio/pmem/event_handler.rs b/src/vmm/src/devices/virtio/pmem/event_handler.rs index 2324c509a69..6c0d239927b 100644 --- a/src/vmm/src/devices/virtio/pmem/event_handler.rs +++ b/src/vmm/src/devices/virtio/pmem/event_handler.rs @@ -78,6 +78,13 @@ impl MutEventSubscriber for Pmem { if !self.is_activated() { warn!("pmem: The device is not activated yet. Spurious event received from {source}"); + match source { + Self::PROCESS_PMEM_QUEUE => self.drain_queue_events(), + Self::PROCESS_RATE_LIMITER => { + self.rate_limiter.event_handler(); + } + _ => (), + } return; } diff --git a/src/vmm/src/devices/virtio/queue.rs b/src/vmm/src/devices/virtio/queue.rs index b4ff6514ed3..4105570220b 100644 --- a/src/vmm/src/devices/virtio/queue.rs +++ b/src/vmm/src/devices/virtio/queue.rs @@ -683,19 +683,6 @@ impl Queue { new - used_event - Wrapping(1) < new - old } - - /// Resets the Virtio Queue - pub(crate) fn reset(&mut self) { - self.ready = false; - self.size = self.max_size; - self.desc_table_address = GuestAddress(0); - self.avail_ring_address = GuestAddress(0); - self.used_ring_address = GuestAddress(0); - self.next_avail = Wrapping(0); - self.next_used = Wrapping(0); - self.num_added = Wrapping(0); - self.uses_notif_suppression = false; - } } #[cfg(kani)] diff --git a/src/vmm/src/devices/virtio/rng/device.rs b/src/vmm/src/devices/virtio/rng/device.rs index cc98fb8b592..2481a8462d1 100644 --- a/src/vmm/src/devices/virtio/rng/device.rs +++ b/src/vmm/src/devices/virtio/rng/device.rs @@ -59,7 +59,7 @@ pub struct Entropy { queue_events: Vec, // Device specific fields - rate_limiter: RateLimiter, + pub(crate) rate_limiter: RateLimiter, buffer: IoVecBufferMut, } @@ -309,6 +309,14 @@ impl VirtioDevice for Entropy { self.device_state.is_activated() } + fn deactivate(&mut self) { + self.device_state = DeviceState::Inactive; + } + + fn _reset(&mut self) -> bool { + true + } + fn activate( &mut self, mem: GuestMemoryMmap, diff --git a/src/vmm/src/devices/virtio/rng/event_handler.rs b/src/vmm/src/devices/virtio/rng/event_handler.rs index dffda5d8845..fc95f8d9dde 100644 --- a/src/vmm/src/devices/virtio/rng/event_handler.rs +++ b/src/vmm/src/devices/virtio/rng/event_handler.rs @@ -83,6 +83,13 @@ impl MutEventSubscriber for Entropy { if !self.is_activated() { warn!("entropy: The device is not activated yet. Spurious event received: {source}"); + match source { + Self::PROCESS_ENTROPY_QUEUE => self.drain_queue_events(), + Self::PROCESS_RATE_LIMITER => { + self.rate_limiter.event_handler(); + } + _ => (), + } return; } diff --git a/src/vmm/src/devices/virtio/transport/mmio.rs b/src/vmm/src/devices/virtio/transport/mmio.rs index 7488e1b3a4f..a2eae50a5d5 100644 --- a/src/vmm/src/devices/virtio/transport/mmio.rs +++ b/src/vmm/src/devices/virtio/transport/mmio.rs @@ -153,9 +153,6 @@ impl MmioTransport { // . Keep interrupt_evt and queue_evts as is. There may be pending notifications in those // eventfds, but nothing will happen other than supurious wakeups. // . Do not reset config_generation and keep it monotonically increasing - for queue in self.locked_device().queues_mut() { - *queue = Queue::new(queue.max_size); - } } /// Update device status according to the state machine defined by VirtIO Spec 1.0. @@ -165,7 +162,6 @@ impl MmioTransport { /// of the driver initialization sequence specified in 3.1. The driver MUST NOT clear /// a device status bit. If the driver sets the FAILED bit, the driver MUST later reset /// the device before attempting to re-initialize. - #[allow(unused_assignments)] fn set_device_status(&mut self, status: u32) { use device_status::*; @@ -183,26 +179,13 @@ impl MmioTransport { // TODO: notify backend driver to stop the device self.device_status |= FAILED; } else if status == INIT { - { - let mut locked_device = self.device.lock().expect("Poisoned lock"); - if locked_device.is_activated() { - let mut device_status = self.device_status; - let reset_result = locked_device.reset(); - match reset_result { - Some((_interrupt_evt, mut _queue_evts)) => {} - None => { - device_status |= FAILED; - } - } - self.device_status = device_status; + if self.device_status != INIT { + if self.device.lock().expect("Poisoned lock").reset() { + self.reset(); + } else { + self.device_status |= FAILED; } } - - // If the backend device driver doesn't support reset, - // just leave the device marked as FAILED. - if self.device_status & FAILED == 0 { - self.reset(); - } } else if VALID_TRANSITIONS .iter() .any(|&(from, to)| self.device_status == from && status == to) @@ -223,6 +206,12 @@ impl MmioTransport { let _ = self.interrupt.trigger(VirtioInterruptType::Config); error!("Failed to activate virtio device: {}", err) + } else { + // A queue notification may have arrived after the guest set DRIVER_OK but + // before the device finished activating, in which case it was discarded as + // spurious. Re-notify the queues so any buffers the guest already made + // available get processed. + locked_device.notify_queue_events(); } } } @@ -506,6 +495,7 @@ pub(crate) mod tests { device_activated: bool, config_bytes: [u8; 0xeff], activate_should_error: bool, + reset_should_fail: bool, } impl DummyDevice { @@ -522,6 +512,7 @@ pub(crate) mod tests { device_activated: false, config_bytes: [0; 0xeff], activate_should_error: false, + reset_should_fail: false, } } @@ -600,6 +591,14 @@ pub(crate) mod tests { fn is_activated(&self) -> bool { self.device_activated } + + fn deactivate(&mut self) { + self.device_activated = false; + } + + fn _reset(&mut self) -> bool { + !self.reset_should_fail + } } fn set_device_status(d: &mut MmioTransport, status: u32) { @@ -613,8 +612,7 @@ pub(crate) mod tests { let m = single_region_mem(0x1000); let interrupt = Arc::new(IrqTrigger::new()); let mut dummy = DummyDevice::new(); - // Validate reset is no-op. - assert!(dummy.reset().is_none()); + assert!(dummy.reset()); let mut d = MmioTransport::new(m, interrupt, Arc::new(Mutex::new(dummy)), false); // We just make sure here that the implementation of a mmio device behaves as we expect, @@ -1144,11 +1142,32 @@ pub(crate) mod tests { assert_eq!(d.device_status, 0x8f); assert!(d.locked_device().is_activated()); - // Nothing happens when backend driver doesn't support reset + // Resetting the device should deactivate it write_le_u32(&mut buf[..], 0x0); d.write(0x0, 0x70, &buf[..]); - assert_eq!(d.device_status, 0x8f); + assert_eq!(d.device_status, device_status::INIT); + assert!(!d.locked_device().is_activated()); + assert_eq!(d.locked_device().acked_features(), 0); + } + + #[test] + fn test_bus_device_reset_failure() { + let m = single_region_mem(0x1000); + let interrupt = Arc::new(IrqTrigger::new()); + let device = DummyDevice { + reset_should_fail: true, + ..DummyDevice::new() + }; + let mut d = MmioTransport::new(m, interrupt, Arc::new(Mutex::new(device)), false); + + activate_device(&mut d); assert!(d.locked_device().is_activated()); + + // A backend that doesn't support reset must set FAILED. + let mut buf = [0; 4]; + write_le_u32(&mut buf[..], 0x0); + d.write(0x0, 0x70, &buf[..]); + assert_ne!(d.device_status & device_status::FAILED, 0); } #[test] diff --git a/src/vmm/src/devices/virtio/transport/pci/device.rs b/src/vmm/src/devices/virtio/transport/pci/device.rs index 050cc9d287f..da65ac90755 100644 --- a/src/vmm/src/devices/virtio/transport/pci/device.rs +++ b/src/vmm/src/devices/virtio/transport/pci/device.rs @@ -652,6 +652,68 @@ impl VirtioPciDevice { self.set_notification_ioevents(vm, false) } + /// Tear down the MSI-X configuration. Used on device reset. + fn reset_msix(&mut self) { + let (num_vectors, vm) = { + let msix_config = self.msix_config.lock().expect("Poisoned lock"); + ( + msix_config.vectors.num_vectors(), + msix_config.vectors.vm.clone(), + ) + }; + + // Build a fresh MSI-X vector group and configuration. + let msix_vectors = match KvmVm::create_msix_group(vm.clone(), num_vectors) { + Ok(vectors) => Arc::new(vectors), + Err(err) => { + error!("Failed to recreate MSI-X vector group on reset: {err:?}"); + return; + } + }; + + self.common_config + .msix_config + .store(VIRTQ_MSI_NO_VECTOR, Ordering::Release); + for vector in self + .common_config + .msix_queues + .lock() + .expect("Poisoned lock") + .iter_mut() + { + *vector = VIRTQ_MSI_NO_VECTOR; + } + + let msix_config = Arc::new(Mutex::new(MsixConfig::new(msix_vectors.clone(), self.sbdf))); + let interrupt = Arc::new(VirtioInterruptMsix::new( + msix_config.clone(), + self.common_config.msix_config.clone(), + self.common_config.msix_queues.clone(), + msix_vectors, + )); + + // Dropping the previous virtio_interrupt and msix_config releases the + // last references to the old MsixVectorGroup, whose Drop unregisters + // the irqfds, removes the GSI routes from the routing table and frees + // the GSIs. + self.virtio_interrupt = Some(interrupt); + self.msix_config = msix_config; + + // Flush the updated routing table + if let Err(err) = vm.set_gsi_routes() { + error!("Failed to update GSI routes after MSI-X reset: {err:?}"); + } + + // Clear the MSI-X Enable and Function Mask bits in the guest-visible + // PCI capability so config-space reads stay consistent with the + // freshly-reset MsixConfig. + let reg_idx = self.msix_config_cap_offset / 4; + let msg_ctl = (self.configuration.read_reg(reg_idx) >> 16) as u16; + let msg_ctl = msg_ctl & !((1 << 15) | (1 << 14)); // clear Enable + Function Mask + self.configuration + .write_config_register(reg_idx, 2, &msg_ctl.to_le_bytes()); + } + pub fn state(&self) -> VirtioPciDeviceState { VirtioPciDeviceState { sbdf: self.sbdf, @@ -953,13 +1015,18 @@ impl PciDevice for VirtioPciDevice { if self.needs_activation() { debug!("Activating device"); let interrupt = Arc::clone(self.virtio_interrupt.as_ref().unwrap()); - match self - .virtio_device() - .lock() - .unwrap() - .activate(self.memory.clone(), interrupt.clone()) - { - Ok(()) => self.device_activated.store(true, Ordering::SeqCst), + let device = self.virtio_device(); + let mut locked_device = device.lock().unwrap(); + match locked_device.activate(self.memory.clone(), interrupt.clone()) { + Ok(()) => { + self.device_activated.store(true, Ordering::SeqCst); + + // A queue notification may have arrived after the guest set DRIVER_OK but + // before the device finished activating, in which case it was discarded as + // spurious. Re-notify the queues so any buffers the guest already made + // available get processed. + locked_device.notify_queue_events(); + } Err(err) => { self.common_config.driver_status |= DEVICE_NEEDS_RESET; error!("Error activating device: {err:?}"); @@ -973,42 +1040,32 @@ impl PciDevice for VirtioPciDevice { // Device has been reset by the driver if self.device_activated.load(Ordering::SeqCst) && self.is_driver_init() { - let mut device = self.device.lock().unwrap(); - let reset_result = device.reset(); - match reset_result { - Some(_) => { - // Upon reset the device returns its interrupt EventFD - self.virtio_interrupt = None; - self.device_activated.store(false, Ordering::SeqCst); - - // Reset queue readiness (changes queue_enable), queue sizes - // and selected_queue as per spec for reset - self.virtio_device() - .lock() - .unwrap() - .queues_mut() - .iter_mut() - .for_each(Queue::reset); - self.common_config.queue_select = 0; - } - None => { - error!("Attempt to reset device when not implemented in underlying device"); - // The virtio spec does not specify what to do if reset fails. - // - // Our MMIO transport sets FAILED in this case, but we must NOT do that for PCI. - // During shutdown, the Linux kernel issues a reset to each virtio device. The - // virtio PCI driver then polls device_status until it reads back 0, unlike the - // virtio MMIO driver which simply writes 0 and returns. Setting FAILED would - // cause the poll to spin forever, breaking reboot command and Ctrl-Alt-Del. - // - PCI: https://elixir.bootlin.com/linux/v6.19.8/source/drivers/virtio/virtio_pci_modern.c#L546-L565 - // - MMIO: https://elixir.bootlin.com/linux/v6.19.8/source/drivers/virtio/virtio_mmio.c#L251-L258 - // - // Since device_status was already set to INIT by set_device_status(), we don't - // need to set it again here. However, the backend device is still active since - // reset() is unimplemented. The combination of device_activated == true and - // device_status == INIT will cause set_device_status() to block any - // re-initialization attempts. - } + let reset_succeeded = self.device.lock().unwrap().reset(); + if reset_succeeded { + self.device_activated.store(false, Ordering::SeqCst); + + self.common_config.queue_select = 0; + self.common_config.device_feature_select = 0; + self.common_config.driver_feature_select = 0; + + self.reset_msix(); + } else { + error!("Attempt to reset device when not implemented in underlying device"); + // The virtio spec does not specify what to do if reset fails. + // + // Our MMIO transport sets FAILED in this case, but we must NOT do that for PCI. + // During shutdown, the Linux kernel issues a reset to each virtio device. The + // virtio PCI driver then polls device_status until it reads back 0, unlike the + // virtio MMIO driver which simply writes 0 and returns. Setting FAILED would + // cause the poll to spin forever, breaking reboot command and Ctrl-Alt-Del. + // - PCI: https://elixir.bootlin.com/linux/v6.19.8/source/drivers/virtio/virtio_pci_modern.c#L546-L565 + // - MMIO: https://elixir.bootlin.com/linux/v6.19.8/source/drivers/virtio/virtio_mmio.c#L251-L258 + // + // Since device_status was already set to INIT by set_device_status(), we don't + // need to set it again here. However, the backend device is still active since + // reset() is unimplemented. The combination of device_activated == true and + // device_status == INIT will cause set_device_status() to block any + // re-initialization attempts. } } None @@ -1704,7 +1761,7 @@ mod tests { } #[test] - fn test_failed_reset_blocks_reinitialization() { + fn test_reset_and_reinitialization() { let mut vmm = create_vmm_with_virtio_pci_device(); let device = get_virtio_device(&vmm); let mut locked = device.lock().unwrap(); @@ -1720,41 +1777,20 @@ mod tests { assert!(locked.device_activated.load(Ordering::SeqCst)); // Write 0 to device_status to request a reset. - // Entropy's reset() returns None (unimplemented), so the reset fails. write_driver_status(&mut locked, 0); assert_eq!(read_driver_status(&mut locked), 0); - // device_activated stays true because the backend was not actually reset. - assert!(locked.device_activated.load(Ordering::SeqCst)); + // Device should be deactivated after successful reset. + assert!(!locked.device_activated.load(Ordering::SeqCst)); - // Attempt to re-initialize should be rejected because device_activated is - // still true while driver_status is INIT. + // Re-initialization should succeed after a successful reset. write_driver_status(&mut locked, ACKNOWLEDGE); - assert_eq!(read_driver_status(&mut locked), 0); - - // Save state and restore into a new device -- the combination of - // device_activated == true and driver_status == INIT is preserved in the - // snapshot, so the blocking behavior survives restore. - let saved_state = locked.state(); - drop(locked); - - // Fully drop the original device before constructing the restored copy: the restored - // copy reuses the same MSI-X GSIs, so the two `MsixVectorGroup` instances must never - // exist concurrently. This is how it will happen in real scenario anyway. - let kvm_vm = vmm.vm.as_kvm().unwrap().clone(); - let saved_allocator = kvm_vm.resource_allocator().save(); - drop(device); - drop(vmm); - // Restore allocator state through `ResourceAllocator::restore`, matching the VM restore - // path. This resets the MSI GSI allocator before the restored device replays its saved GSI - // allocations. - *kvm_vm.resource_allocator() = ResourceAllocator::restore((), &saved_allocator).unwrap(); - - let new_entropy = Arc::new(Mutex::new(Entropy::new(RateLimiter::default()).unwrap())); - let restored = - VirtioPciDevice::new_from_state("rng".to_string(), &kvm_vm, new_entropy, saved_state) - .unwrap(); - - assert!(restored.device_activated.load(Ordering::SeqCst)); - assert_eq!(restored.common_config.driver_status, 0); + assert_eq!(read_driver_status(&mut locked), ACKNOWLEDGE); + write_driver_status(&mut locked, ACKNOWLEDGE | DRIVER); + let features = read_device_features(&mut locked); + write_driver_features(&mut locked, features); + write_driver_status(&mut locked, ACKNOWLEDGE | DRIVER | FEATURES_OK); + setup_queues(&mut locked); + write_driver_status(&mut locked, ACKNOWLEDGE | DRIVER | FEATURES_OK | DRIVER_OK); + assert!(locked.device_activated.load(Ordering::SeqCst)); } } diff --git a/src/vmm/src/devices/virtio/vsock/device.rs b/src/vmm/src/devices/virtio/vsock/device.rs index ac022ee02cf..a9706d4b491 100644 --- a/src/vmm/src/devices/virtio/vsock/device.rs +++ b/src/vmm/src/devices/virtio/vsock/device.rs @@ -382,6 +382,11 @@ where } } + self.backend.activate().map_err(|err| { + METRICS.activate_fails.inc(); + ActivateError::VsockBackend(err) + })?; + if self.activate_evt.write(1).is_err() { METRICS.activate_fails.inc(); return Err(ActivateError::EventFd); @@ -396,6 +401,18 @@ where self.device_state.is_activated() } + fn deactivate(&mut self) { + self.device_state = DeviceState::Inactive; + } + + fn _reset(&mut self) -> bool { + self.backend.reset(); + self.rx_packet.clear(); + self.tx_packet.clear(); + self.pending_event_ack = false; + true + } + fn kick(&mut self) { if self.is_activated() { self.pending_event_ack = true; diff --git a/src/vmm/src/devices/virtio/vsock/event_handler.rs b/src/vmm/src/devices/virtio/vsock/event_handler.rs index 2a1a556a708..5da6fd70d24 100644 --- a/src/vmm/src/devices/virtio/vsock/event_handler.rs +++ b/src/vmm/src/devices/virtio/vsock/event_handler.rs @@ -221,6 +221,7 @@ where "Vsock: The device is not yet activated. Spurious event received: {:?}", source ); + self.drain_queue_events(); } } diff --git a/src/vmm/src/devices/virtio/vsock/mod.rs b/src/vmm/src/devices/virtio/vsock/mod.rs index c8160f33381..d20da8d660d 100644 --- a/src/vmm/src/devices/virtio/vsock/mod.rs +++ b/src/vmm/src/devices/virtio/vsock/mod.rs @@ -183,4 +183,11 @@ pub trait VsockChannel { /// The vsock backend, which is basically an epoll-event-driven vsock channel. /// Currently, the only implementation we have is `crate::devices::virtio::unix::muxer::VsockMuxer`, /// which translates guest-side vsock connections to host-side Unix domain socket connections. -pub trait VsockBackend: VsockChannel + VsockEpollListener + Send {} +pub trait VsockBackend: VsockChannel + VsockEpollListener + Send { + /// Activate the backend, adding its listeners to the poll set. + fn activate(&mut self) -> Result<(), VsockError>; + + /// Reset the backend, dropping all active connections and removing its listeners + /// from the poll set. + fn reset(&mut self); +} diff --git a/src/vmm/src/devices/virtio/vsock/packet.rs b/src/vmm/src/devices/virtio/vsock/packet.rs index 7253f41ce76..baac307e773 100644 --- a/src/vmm/src/devices/virtio/vsock/packet.rs +++ b/src/vmm/src/devices/virtio/vsock/packet.rs @@ -197,6 +197,12 @@ pub struct VsockPacketTx { } impl VsockPacketTx { + /// Clear the packet buffer. + pub fn clear(&mut self) { + self.hdr = Default::default(); + self.buffer.clear(); + } + /// Create the packet wrapper from a TX virtq chain head. /// /// ## Errors @@ -290,6 +296,12 @@ pub struct VsockPacketRx { } impl VsockPacketRx { + /// Clear the packet buffer. + pub fn clear(&mut self) { + self.hdr = Default::default(); + self.buffer.clear(); + } + /// Creates new VsockPacketRx. pub fn new() -> Result { let buffer = IoVecBufferMut::new().map_err(VsockError::IovDeque)?; diff --git a/src/vmm/src/devices/virtio/vsock/test_utils.rs b/src/vmm/src/devices/virtio/vsock/test_utils.rs index 4001f2b940d..8877690337a 100644 --- a/src/vmm/src/devices/virtio/vsock/test_utils.rs +++ b/src/vmm/src/devices/virtio/vsock/test_utils.rs @@ -102,7 +102,13 @@ impl VsockEpollListener for TestBackend { self.evset = Some(evset); } } -impl VsockBackend for TestBackend {} +impl VsockBackend for TestBackend { + fn activate(&mut self) -> Result<(), VsockError> { + Ok(()) + } + + fn reset(&mut self) {} +} #[derive(Debug)] pub struct TestContext { diff --git a/src/vmm/src/devices/virtio/vsock/unix/muxer.rs b/src/vmm/src/devices/virtio/vsock/unix/muxer.rs index 526bdb708d5..d7d0eb89d72 100644 --- a/src/vmm/src/devices/virtio/vsock/unix/muxer.rs +++ b/src/vmm/src/devices/virtio/vsock/unix/muxer.rs @@ -301,7 +301,31 @@ impl VsockEpollListener for VsockMuxer { } } -impl VsockBackend for VsockMuxer {} +impl VsockBackend for VsockMuxer { + fn activate(&mut self) -> Result<(), VsockError> { + // Listen on the host-initiated socket, for incoming connections. + self.add_listener(self.host_sock.as_raw_fd(), EpollListener::HostSock) + .map_err(VsockError::VsockUdsBackend) + } + + fn reset(&mut self) { + // Remove all connections and their epoll listeners. + let keys: Vec = self.conn_map.keys().copied().collect(); + for key in keys { + self.remove_connection(key); + } + + let fds: Vec = self.listener_map.keys().copied().collect(); + for fd in fds { + self.remove_listener(fd); + } + + self.rxq = MuxerRxQ::new(); + self.killq = MuxerKillQ::new(); + self.local_port_set.clear(); + self.local_port_last = (1u32 << 30) - 1; + } +} impl VsockMuxer { /// Muxer constructor. @@ -312,7 +336,7 @@ impl VsockMuxer { .and_then(|sock| sock.set_nonblocking(true).map(|_| sock)) .map_err(VsockUnixBackendError::UnixBind)?; - let mut muxer = Self { + let muxer = Self { cid, host_sock, host_sock_path, @@ -325,8 +349,6 @@ impl VsockMuxer { local_port_set: HashSet::with_capacity(defs::MAX_CONNECTIONS), }; - // Listen on the host initiated socket, for incoming connections. - muxer.add_listener(muxer.host_sock.as_raw_fd(), EpollListener::HostSock)?; Ok(muxer) } @@ -851,7 +873,8 @@ mod tests { ) .unwrap(); - let muxer = VsockMuxer::new(PEER_CID, get_file(name)).unwrap(); + let mut muxer = VsockMuxer::new(PEER_CID, get_file(name)).unwrap(); + muxer.activate().unwrap(); Self { _vsock_test_ctx: vsock_test_ctx, rx_pkt, diff --git a/tests/integration_tests/functional/test_balloon.py b/tests/integration_tests/functional/test_balloon.py index de4d0f0d5bb..7201dd9d6a4 100644 --- a/tests/integration_tests/functional/test_balloon.py +++ b/tests/integration_tests/functional/test_balloon.py @@ -622,3 +622,60 @@ def test_memory_scrub(uvm, method): microvm.ssh.check_output("/usr/local/bin/readmem {} {}".format(60, 1)) check_guest_dmesg_for_stalls(microvm.ssh) + + +def test_device_reset(uvm): + """ + Test that virtio-balloon device reset works. + """ + vm = uvm + vm.spawn() + vm.basic_config() + vm.add_net_iface() + vm.api.balloon.put(amount_mib=0, deflate_on_oom=True, stats_polling_interval_s=0) + vm.start() + + meminfo = MeminfoGuest(vm) + free_initial = meminfo.get().mem_free.kib() + + # Inflate the balloon by 64 MiB and confirm guest free memory drops. + vm.api.balloon.patch(amount_mib=64) + get_stable_rss_mem(vm) + free_inflated = meminfo.get().mem_free.kib() + # Inflating 64 MiB should reclaim at least 85% of that from guest free + # memory. The 15% slack accounts for kernel accounting overhead. + inflated_drop = 64 * 1024 * 85 // 100 + assert free_inflated <= free_initial - inflated_drop + + # Find the virtio balloon device. + virtio_dev = vm.ssh.check_output( + "ls -d /sys/bus/virtio/drivers/virtio_balloon/virtio* | xargs -n1 basename" + ).stdout.strip() + + # Reset the device by unbinding the driver. + vm.ssh.check_output( + f"echo {virtio_dev} > /sys/bus/virtio/drivers/virtio_balloon/unbind" + ) + + # Verify the balloon is gone. + ret = vm.ssh.run("ls /sys/bus/virtio/drivers/virtio_balloon/virtio*") + assert ret.returncode != 0 + + # Rebind and make sure the device node is back. + vm.ssh.check_output( + f"echo {virtio_dev} > /sys/bus/virtio/drivers/virtio_balloon/bind" + ) + vm.ssh.check_output("ls /sys/bus/virtio/drivers/virtio_balloon/virtio*") + + # The inflation target is preserved across reset, assert that the driver + # re-inflates the balloon. + get_stable_rss_mem(vm) + free_after_reset = meminfo.get().mem_free.kib() + assert free_after_reset <= free_initial - inflated_drop + + # Deflate to make sure the device is functional in both directions + vm.api.balloon.patch(amount_mib=0) + get_stable_rss_mem(vm) + free_deflated = meminfo.get().mem_free.kib() + DEFLATE_SLACK_KIB = 8 * 1024 # arbitrary 8 MiB tolerance for measurement noise + assert free_deflated >= free_initial - DEFLATE_SLACK_KIB diff --git a/tests/integration_tests/functional/test_drive_virtio.py b/tests/integration_tests/functional/test_drive_virtio.py index 6830dad3bd1..bc792ffba9e 100644 --- a/tests/integration_tests/functional/test_drive_virtio.py +++ b/tests/integration_tests/functional/test_drive_virtio.py @@ -386,3 +386,38 @@ def _check_mount(ssh_connection, dev_path): assert stderr == "" _, _, stderr = ssh_connection.run("umount /tmp", timeout=30.0) assert stderr == "" + + +def test_device_reset(uvm, io_engine): + """ + Test that virtio-block device reset works. + """ + vm = uvm + vm.spawn() + vm.basic_config() + vm.add_net_iface() + + fs = drive_tools.FilesystemFile(os.path.join(vm.fsfiles, "scratch"), size=2) + vm.add_drive("scratch", fs.path, io_engine=io_engine) + vm.start() + + # Verify the scratch drive is accessible. + vm.ssh.check_output("mount /dev/vdb /tmp && umount /tmp") + + # Find the virtio device backing vdb and unbind it. + virtio_dev = vm.ssh.check_output( + "basename $(readlink /sys/block/vdb/device)" + ).stdout.strip() + + vm.ssh.check_output( + f"echo {virtio_dev} > /sys/bus/virtio/drivers/virtio_blk/unbind" + ) + + # Verify the drive is gone. + ret = vm.ssh.run("ls /dev/vdb") + assert ret.returncode != 0 + + # Rebind and verify the drive is back. + vm.ssh.check_output(f"echo {virtio_dev} > /sys/bus/virtio/drivers/virtio_blk/bind") + vm.ssh.check_output("ls /dev/vdb") + vm.ssh.check_output("mount /dev/vdb /tmp && umount /tmp") diff --git a/tests/integration_tests/functional/test_net.py b/tests/integration_tests/functional/test_net.py index c3e6c3fbe8f..9dade56d156 100644 --- a/tests/integration_tests/functional/test_net.py +++ b/tests/integration_tests/functional/test_net.py @@ -11,6 +11,13 @@ import host_tools.network as net_tools from framework import utils from framework.artifacts import GUEST_KERNEL_DEFAULT, pin_guest_kernel +from framework.utils import ( + configure_mmds, + generate_mmds_get_request, + generate_mmds_session_token, +) + +MMDS_IPV4_ADDR = "169.254.169.254" # The iperf version to run this tests with IPERF_BINARY = "iperf3" @@ -171,3 +178,63 @@ def test_tap_mtu_advertised_to_guest(uvm): f"{iface_name} (guest: {guest_if}): VIRTIO_NET_F_MTU (bit {VIRTIO_NET_F_MTU_BIT})" f" not set in negotiated features: {features!r}" ) + + +def test_device_reset(uvm): + """ + Test that virtio-net device reset works. + + MMDS is configured on the reset interface and exercised before and after + the reset. + """ + vm = uvm + vm.spawn() + vm.basic_config() + vm.add_net_iface() # eth0 - used for SSH + iface2 = vm.add_net_iface() # eth1 - will be reset + configure_mmds(vm, iface_ids=["eth1"], version="V2") + vm.api.mmds.put(**{"key": "value"}) + vm.start() + + guest_ip2 = iface2.guest_ip + host_ip2 = iface2.host_ip + virtio_dev = vm.ssh.check_output( + "basename $(readlink /sys/class/net/eth1/device)" + ).stdout.strip() + + def configure_eth1(): + vm.ssh.check_output( + f"ip addr flush dev eth1 && " + f"ip addr add {guest_ip2}/30 dev eth1 && " + f"ip link set eth1 up && " + f"ip route replace {MMDS_IPV4_ADDR} dev eth1" + ) + + def check_mmds(): + token = generate_mmds_session_token(vm.ssh, MMDS_IPV4_ADDR, token_ttl=60) + cmd = ( + generate_mmds_get_request(MMDS_IPV4_ADDR, token=token, app_json=False) + + "key" + ) + _, stdout, _ = vm.ssh.check_output(cmd) + assert stdout == "value" + + configure_eth1() + vm.ssh.check_output(f"ping -c 1 {host_ip2}") + check_mmds() + + # Reset eth1 by unbinding and rebinding its virtio driver. + vm.ssh.check_output( + f"echo {virtio_dev} > /sys/bus/virtio/drivers/virtio_net/unbind" + ) + + # Verify that ping fails after unbind. + ret = vm.ssh.run(f"ping -c 1 -W 1 {host_ip2}") + assert ret.returncode != 0 + + # Re-bind and check again + vm.ssh.check_output(f"echo {virtio_dev} > /sys/bus/virtio/drivers/virtio_net/bind") + + configure_eth1() + vm.ssh.check_output(f"ping -c 1 {host_ip2}") + check_mmds() diff --git a/tests/integration_tests/functional/test_pmem.py b/tests/integration_tests/functional/test_pmem.py index 51cd9548c5f..cf109ac17a3 100644 --- a/tests/integration_tests/functional/test_pmem.py +++ b/tests/integration_tests/functional/test_pmem.py @@ -7,6 +7,7 @@ import re import pytest +from tenacity import Retrying, stop_after_delay, wait_fixed import host_tools.drive as drive_tools from framework import utils @@ -228,3 +229,47 @@ def test_pmem_dax_memory_saving( assert ( pmem_rss_usage < block_rss_usage ), f"{block_cache_usage} <= {pmem_cache_usage}" + + +def test_device_reset(uvm): + """ + Test that virtio-pmem device reset works. + """ + vm = uvm + vm.spawn() + vm.basic_config(add_root_device=True) + vm.add_net_iface() + + fs = drive_tools.FilesystemFile(os.path.join(vm.fsfiles, "scratch"), size=2) + vm.add_pmem("pmem_scratch", fs.path, False, False) + vm.start() + + # Verify the pmem device is accessible. + vm.ssh.check_output("ls /dev/pmem0") + + virtio_dev = vm.ssh.check_output( + "basename $(realpath /sys/block/pmem0/device/../../..)" + ).stdout.strip() + + vm.ssh.check_output( + f"echo {virtio_dev} > /sys/bus/virtio/drivers/virtio_pmem/unbind" + ) + + # Verify the device is gone. + ret = vm.ssh.run("ls /dev/pmem0") + assert ret.returncode != 0 + + # Rebind and verify the device is functional. + vm.ssh.check_output(f"echo {virtio_dev} > /sys/bus/virtio/drivers/virtio_pmem/bind") + # The NVDIMM subsystem creates the device node asynchronously after driver + # probe, so we need to wait for it. + for attempt in Retrying( + wait=wait_fixed(0.1), stop=stop_after_delay(5.0), reraise=True + ): + with attempt: + vm.ssh.check_output("ls /dev/pmem0") + vm.ssh.check_output("mount /dev/pmem0 /tmp") + vm.ssh.check_output("echo reset_test > /tmp/testfile") + ret = vm.ssh.check_output("cat /tmp/testfile") + assert "reset_test" in ret.stdout + vm.ssh.check_output("umount /tmp") diff --git a/tests/integration_tests/functional/test_rng.py b/tests/integration_tests/functional/test_rng.py index e7706947b65..648f545f18b 100644 --- a/tests/integration_tests/functional/test_rng.py +++ b/tests/integration_tests/functional/test_rng.py @@ -232,3 +232,35 @@ def test_rng_bw_rate_limiter(uvm_any): # Check the rate limiter using a request size equal to the size # of the token bucket. _check_entropy_rate_limited(vm.ssh, size, expected_kbps) + + +def test_device_reset(uvm): + """ + Test that virtio-rng device reset works. + """ + vm = uvm + vm.spawn() + vm.basic_config() + vm.add_net_iface() + vm.api.entropy.put() + vm.start() + + # Verify the rng device works. + vm.ssh.check_output("dd if=/dev/hwrng of=/dev/null bs=32 count=1") + + # Find the virtio rng device. + virtio_dev = vm.ssh.check_output( + "ls -d /sys/bus/virtio/drivers/virtio_rng/virtio* | xargs -n1 basename" + ).stdout.strip() + + vm.ssh.check_output( + f"echo {virtio_dev} > /sys/bus/virtio/drivers/virtio_rng/unbind" + ) + + # Verify the rng device is gone. + ret = vm.ssh.run("ls /sys/bus/virtio/drivers/virtio_rng/virtio*") + assert ret.returncode != 0 + + # Rebind and verify it works again. + vm.ssh.check_output(f"echo {virtio_dev} > /sys/bus/virtio/drivers/virtio_rng/bind") + vm.ssh.check_output("dd if=/dev/hwrng of=/dev/null bs=32 count=1") diff --git a/tests/integration_tests/functional/test_vsock.py b/tests/integration_tests/functional/test_vsock.py index fc5afffb74c..ad264ff433e 100644 --- a/tests/integration_tests/functional/test_vsock.py +++ b/tests/integration_tests/functional/test_vsock.py @@ -538,3 +538,47 @@ def test_snapshot_restore_with_inflight_vsock_tx( new_vm.path, make_host_port_path(VSOCK_UDS_PATH, ECHO_SERVER_PORT) ) check_guest_connections(new_vm, path, vm_blob_path, blob_hash) + + +def test_device_reset(uvm): + """ + Test that virtio-vsock device reset works. + """ + vm = uvm + vm.spawn() + vm.basic_config() + vm.add_net_iface() + vm.api.vsock.put(guest_cid=3, vsock_id="vsock0", uds_path="/" + VSOCK_UDS_PATH) + vm.start() + + uds_path = start_guest_echo_server(vm) + blob_path, blob_hash = make_blob(vm.path) + check_host_connections(uds_path, blob_path, blob_hash) + + # Keep a connection open across the reset. + held_conn = vsock_connect_to_guest(uds_path, ECHO_SERVER_PORT) + + # Find the virtio vsock device. + virtio_dev = vm.ssh.check_output( + "ls -d /sys/bus/virtio/drivers/vmw_vsock_virtio_transport/virtio*" + " | xargs -n1 basename" + ).stdout.strip() + + vm.ssh.check_output( + f"echo {virtio_dev} > /sys/bus/virtio/drivers/vmw_vsock_virtio_transport/unbind" + ) + + held_conn.close() + + # Verify the vsock device is gone. + ret = vm.ssh.run("ls /sys/bus/virtio/drivers/vmw_vsock_virtio_transport/virtio*") + assert ret.returncode != 0 + + # Rebind and verify the data path works by starting a guest echo server + # and sending data from the host through the vsock. + vm.ssh.check_output( + f"echo {virtio_dev} > /sys/bus/virtio/drivers/vmw_vsock_virtio_transport/bind" + ) + uds_path = start_guest_echo_server(vm) + blob_path, blob_hash = make_blob(vm.path) + check_host_connections(uds_path, blob_path, blob_hash) diff --git a/tests/integration_tests/performance/test_hotplug_memory.py b/tests/integration_tests/performance/test_hotplug_memory.py index 84b5e09f674..e481057f355 100644 --- a/tests/integration_tests/performance/test_hotplug_memory.py +++ b/tests/integration_tests/performance/test_hotplug_memory.py @@ -514,3 +514,29 @@ def test_memory_hotplug_latency( timed_memory_hotplug(uvm, 0, metrics, "hotunplug", "unplug_agg") timed_memory_hotplug(uvm, hotplug_size, metrics, "hotplug_2nd", "plug_agg") uvm.kill() + + +def test_device_reset(uvm): + """ + Test that virtio-mem device reset works. + + Note: the Linux virtio-mem driver does not support rebinding when memory is + plugged (the resource region can't be re-registered), so we reset without + any plugged memory and verify the device is functional afterwards. + """ + config = {"total_size_mib": 1024, "slot_size_mib": 128, "block_size_mib": 2} + uvm = uvm_booted_memhp(uvm, None, None, False, config, None, None, None) + + # Reset the device via driver unbind/bind. + virtio_dev = uvm.ssh.check_output( + "ls -d /sys/bus/virtio/drivers/virtio_mem/virtio* | xargs -n1 basename" + ).stdout.strip() + + uvm.ssh.check_output( + f"echo {virtio_dev} > /sys/bus/virtio/drivers/virtio_mem/unbind" + ) + uvm.ssh.check_output(f"echo {virtio_dev} > /sys/bus/virtio/drivers/virtio_mem/bind") + + # Verify the device is functional after reset by hotplugging memory. + # check_hotplug() asserts that guest mem_total reflects the new size. + check_hotplug(uvm, 256)