pcie, virtio, nvme: preserve and quiesce emulated PCIe devices across guest reset

vm/devices/pci/pcie/src/port.rs

@@ -791,6 +791,24 @@ impl PcieDownstreamPort {
         bail!("port name does not match")
     }
 
+    /// Reset the port's config space, preserving presence for a still-connected
+    /// device.
+    ///
+    /// A VM reset resets the config space, which clears the PCIe slot-status
+    /// Presence Detect bit. But `self.link` (the connected device) survives the
+    /// reset, so the slot is still occupied. Without re-asserting presence here,
+    /// the guest's pciehp driver reads the port after reboot, sees Presence
+    /// Detect clear ("card not present"), and removes the still-attached device
+    /// even though its link is up - so an emulated PCIe disk/NIC disappears on
+    /// every guest reboot until a manual PCI rescan. Re-derive presence from the
+    /// link so a boot-present device stays present across reset.
+    pub fn reset(&mut self) {
+        self.cfg_space.reset();
+        if self.link.is_some() {
+            self.cfg_space.set_presence_detect_state(true);
+        }
+    }
+
     /// Hot-add a device to this port at runtime.
     ///
     /// Unlike `add_pcie_device`, this method verifies the port is hotplug-capable
@@ -1046,6 +1064,72 @@ mod tests {
         );
     }
 
+    #[test]
+    fn test_reset_preserves_presence_detect_for_connected_device() {
+        use pci_core::spec::hwid::{ClassCode, ProgrammingInterface, Subclass};
+
+        let hardware_ids = HardwareIds {
+            vendor_id: 0x1234,
+            device_id: 0x5678,
+            revision_id: 0,
+            prog_if: ProgrammingInterface::NONE,
+            sub_class: Subclass::BRIDGE_PCI_TO_PCI,
+            base_class: ClassCode::BRIDGE,
+            type0_sub_vendor_id: 0,
+            type0_sub_system_id: 0,
+        };
+
+        let msi_conn = pci_core::msi::MsiConnection::new();
+        let mut port = PcieDownstreamPort::new(
+            "test-port",
+            hardware_ids,
+            DevicePortType::RootPort,
+            false,
+            Some(1), // hotplug slot 1
+            &msi_conn.target(),
+            PciePortSettings::default(),
+            None,
+            None,
+        );
+
+        // presence_detect_state is bit 6 of slot status at cap offset 0x18.
+        fn presence(port: &mut PcieDownstreamPort) -> u32 {
+            let mut v = 0u32;
+            assert!(matches!(
+                port.cfg_space.read_u32(0x58, &mut v),
+                IoResult::Ok
+            ));
+            (v >> 22) & 0x1
+        }
+
+        // An empty port reports no presence after reset.
+        port.reset();
+        assert_eq!(
+            presence(&mut port),
+            0,
+            "empty port: no presence after reset"
+        );
+
+        // Connect a device (presence asserted), then reset as a guest reboot does.
+        port.add_pcie_device("test-port", "mock-device", Box::new(MockDevice))
+            .unwrap();
+        assert_eq!(
+            presence(&mut port),
+            1,
+            "presence asserted after connecting device"
+        );
+
+        // Regression: the connected device (self.link) survives the reset, so
+        // presence must remain set. If it clears, the guest's pciehp evicts the
+        // still-attached device on reboot (the PCIe-devices-vanish-on-reboot bug).
+        port.reset();
+        assert_eq!(
+            presence(&mut port),
+            1,
+            "presence must survive reset while a device is connected"
+        );
+    }
+
     #[test]
     fn test_add_pcie_device_without_hotplug() {
         use pci_core::spec::hwid::{ClassCode, ProgrammingInterface, Subclass};

vm/devices/pci/pcie/src/root.rs

@@ -575,7 +575,10 @@ impl ChangeDeviceState for GenericPcieRootComplex {
     async fn reset(&mut self) {
         for (_, d) in self.devices.iter_mut() {
             if let BusDevice::RootPort { port, .. } = d {
-                port.port.cfg_space.reset();
+                // Reset via the port so presence detect is re-asserted for a
+                // still-connected device; a bare cfg_space.reset() would clear
+                // it and the guest's pciehp would evict the device on reboot.
+                port.port.reset();
             }
         }
     }

vm/devices/pci/pcie/src/switch.rs

@@ -346,9 +346,12 @@ impl ChangeDeviceState for GenericPcieSwitch {
         // Reset the upstream port configuration space
         self.upstream_port.cfg_space.reset();
 
-        // Reset all downstream port configuration spaces
+        // Reset each downstream port via the port (not a bare cfg_space.reset),
+        // so presence detect is re-asserted for a still-connected device; a bare
+        // cfg_space.reset() would clear it and the guest's pciehp would evict a
+        // device attached behind the switch on reboot.
         for (_, _, downstream_port) in self.downstream_ports.iter_mut() {
-            downstream_port.port.cfg_space.reset();
+            downstream_port.port.reset();
         }
     }
 }
@@ -887,6 +890,60 @@ mod tests {
         assert!(port_numbers.contains(&2));
     }
 
+    #[pal_async::async_test]
+    async fn test_reset_preserves_presence_detect_behind_switch() {
+        use crate::test_helpers::TestPcieEndpoint;
+        use chipset_device::io::IoResult;
+
+        let mut switch = build(switch_def(
+            "test-switch",
+            2,
+            true, // hotplug-capable downstream ports
+            PciePortSettings::default(),
+            MsiTarget::disconnected(),
+        ));
+
+        // Presence Detect is bit 6 of the PCIe slot-status register (cap offset
+        // 0x18), i.e. bit 22 of the dword at config offset 0x58 on a downstream
+        // port.
+        fn presence(switch: &GenericPcieSwitch, idx: usize) -> u32 {
+            let mut v = 0u32;
+            assert!(matches!(
+                switch.downstream_ports[idx]
+                    .2
+                    .cfg_space()
+                    .read_u32(0x58, &mut v),
+                IoResult::Ok
+            ));
+            (v >> 22) & 0x1
+        }
+
+        // Connect a device to the first downstream port; presence asserts.
+        let devfn = switch.downstream_ports[0].0;
+        switch
+            .add_pcie_device(
+                devfn,
+                "downstream-dev",
+                Box::new(TestPcieEndpoint::new(
+                    |_, _| Some(IoResult::Ok),
+                    |_, _| Some(IoResult::Ok),
+                )),
+            )
+            .unwrap();
+        assert_eq!(presence(&switch, 0), 1, "presence asserted after connect");
+
+        // A guest reboot resets the switch; the connected device's link survives,
+        // so presence must remain set - otherwise the guest's pciehp evicts a
+        // device attached behind the switch on reboot (the same bug this fixes for
+        // root ports).
+        switch.reset().await;
+        assert_eq!(
+            presence(&switch, 0),
+            1,
+            "presence must survive switch reset while a device is connected"
+        );
+    }
+
     #[test]
     fn test_switch_device_connections() {
         use crate::test_helpers::TestPcieEndpoint;

vm/devices/storage/nvme/src/queue.rs

@@ -60,6 +60,17 @@ impl DoorbellMemory {
         Ok(())
     }
 
+    /// Restore the doorbell store to its power-on state: a fresh internal
+    /// host-side buffer with no guest-backed shadow-doorbell mapping.
+    ///
+    /// Called on controller reset. A shadow-doorbell mapping installed by
+    /// Doorbell Buffer Config otherwise survives the reset, so after the guest
+    /// tears the controller down and reboots, the controller writes shadow
+    /// event_idx into a guest address the rebooted guest has already reused.
+    pub fn reset(&mut self) {
+        *self = Self::new(self.wakers.len() as u16);
+    }
+
     pub fn try_write(&self, db_id: u16, value: u32) -> Result<(), InvalidDoorbell> {
         if (db_id as usize) >= self.wakers.len() {
             return Err(InvalidDoorbell);

vm/devices/storage/nvme/src/workers/coordinator.rs

@@ -82,6 +82,7 @@ impl NvmeWorkers {
             driver: driver.clone(),
             admin: TaskControl::new(handler),
             reset: None,
+            doorbells: doorbells.clone(),
         };
         let (send, recv) = mesh::mpsc_channel();
         let task = driver.spawn("nvme-coord", coordinator.run(recv));
@@ -208,6 +209,10 @@ struct Coordinator {
     admin: TaskControl<AdminHandler, AdminState>,
     #[inspect(with = "Option::is_some")]
     reset: Option<Rpc<(), ()>>,
+    // Cleared on every controller reset so a stale shadow-doorbell mapping
+    // cannot outlive the reset and corrupt a rebooted guest's reused memory.
+    #[inspect(skip)]
+    doorbells: Arc<RwLock<DoorbellMemory>>,
 }
 
 enum CoordinatorRequest {
@@ -240,6 +245,11 @@ impl Coordinator {
                         state.drain().await;
                         self.admin.remove();
                     }
+                    // Drop any shadow-doorbell mapping installed by Doorbell
+                    // Buffer Config. Otherwise its guest-memory address survives
+                    // the reset and the controller writes shadow event_idx into a
+                    // page the rebooted guest has reused. See DoorbellMemory::reset.
+                    self.doorbells.write().reset();
                 } else {
                     pending().await
                 }

vm/devices/virtio/virtio/src/device.rs

@@ -97,6 +97,24 @@ pub trait VirtioDevice: InspectMut + Send {
     /// iterating all queue indices, not just active ones.
     fn stop_queue(&mut self, idx: u16) -> impl Future<Output = Option<QueueState>> + Send;
 
+    /// Tear down a single queue on the reset/disable path.
+    ///
+    /// The queue is being torn down and the guest no longer owns the ring
+    /// memory, so a device with in-flight host IO must not publish its
+    /// completions to the used ring: publishing there (as `stop_queue` does)
+    /// writes into pages a rebooted guest has already reused. Such a device
+    /// overrides this to discard the completions, and must also ensure no
+    /// in-flight host IO writes guest memory after this returns (it cancels the
+    /// IO, or awaits it so the write lands while the guest is still stopped).
+    ///
+    /// The default forwards to `stop_queue`, which is correct only for a device
+    /// with no in-flight host IO to discard.
+    fn abort_queue(&mut self, idx: u16) -> impl Future<Output = ()> + Send {
+        async move {
+            let _ = self.stop_queue(idx).await;
+        }
+    }
+
     /// Reset device-internal state to initial values.
     ///
     /// Called after all queues have been stopped on guest-initiated reset.
@@ -161,6 +179,10 @@ pub trait DynVirtioDevice: InspectMut + Send {
         idx: u16,
     ) -> Pin<Box<dyn Future<Output = Option<QueueState>> + Send + '_>>;
 
+    /// Tear down a single queue on the reset/disable path (object-safe mirror
+    /// of `VirtioDevice::abort_queue`).
+    fn abort_queue(&mut self, idx: u16) -> Pin<Box<dyn Future<Output = ()> + Send + '_>>;
+
     /// Reset device-internal state.
     fn reset(&mut self) -> Pin<Box<dyn Future<Output = ()> + Send + '_>>;
 
@@ -222,6 +244,10 @@ impl<T: VirtioDevice> DynVirtioDevice for T {
         Box::pin(VirtioDevice::stop_queue(self, idx))
     }
 
+    fn abort_queue(&mut self, idx: u16) -> Pin<Box<dyn Future<Output = ()> + Send + '_>> {
+        Box::pin(VirtioDevice::abort_queue(self, idx))
+    }
+
     fn reset(&mut self) -> Pin<Box<dyn Future<Output = ()> + Send + '_>> {
         Box::pin(VirtioDevice::reset(self))
     }

vm/devices/virtio/virtio/src/transport/task.rs

@@ -231,8 +231,14 @@ impl DeviceTask {
     }
 
     async fn stop_all_queues(&mut self) {
+        // Reset/disable path: call `abort_queue`, which a device with in-flight
+        // host IO overrides to discard those completions rather than publish
+        // them (as `stop_queue` does). The queue is being torn down, so
+        // publishing a completion into its guest memory can corrupt a rebooted
+        // guest that has reused those pages. Suspend uses `stop()` instead,
+        // which drains and completes so the resumed guest sees its completions.
         for idx in 0..self.max_queues {
-            self.device.stop_queue(idx).await;
+            self.device.abort_queue(idx).await;
         }
     }
 }

vm/devices/virtio/virtio_blk/src/lib.rs

@@ -160,6 +160,27 @@ impl BlkWorker {
             }
         }
     }
+
+    /// Await all in-flight IOs to completion, then drop their completions
+    /// without posting to the used ring.
+    ///
+    /// Used on the reset/disable teardown path. The IOs must be awaited (not
+    /// just dropped): dropping a pending disk read does not cancel the
+    /// underlying io_uring op, so it would complete asynchronously later and
+    /// write its disk data into a page a rebooted guest has already reused.
+    /// Awaiting lets that write land while the guest is stopped (memory still
+    /// valid). Each awaited IO still writes its data and status byte into the
+    /// guest's descriptor buffers; only its completion is dropped, so no
+    /// used-ring entry is posted into the torn-down queue's memory.
+    fn poll_drain_discard(&mut self, cx: &mut Context<'_>) -> Poll<()> {
+        loop {
+            match self.ios.poll_next_unpin(cx) {
+                Poll::Ready(Some(_completion)) => {}
+                Poll::Ready(None) => return Poll::Ready(()),
+                Poll::Pending => return Poll::Pending,
+            }
+        }
+    }
 }
 
 impl AsyncRun<BlkQueueState> for BlkWorker {
@@ -408,6 +429,22 @@ impl VirtioDevice for VirtioBlkDevice {
         Some(state)
     }
 
+    async fn abort_queue(&mut self, idx: u16) {
+        assert_eq!(idx, 0);
+        if !self.worker.has_state() {
+            return;
+        }
+        // Reset/disable teardown: await the in-flight IOs to completion, then
+        // discard them without posting to the used ring (unlike stop_queue). The
+        // reads must be awaited, not dropped: dropping a pending disk read does
+        // not cancel the io_uring op, which would then write into a page the
+        // rebooted guest has reused. See poll_drain_discard.
+        self.worker.stop().await;
+        let (worker, _state) = self.worker.get_mut();
+        poll_fn(|cx| worker.poll_drain_discard(cx)).await;
+        self.worker.remove();
+    }
+
     fn supports_save_restore(&self) -> bool {
         true
     }