On Tue, Jun 24, 2025 at 02:56:22PM -0400, Stefan Hajnoczi wrote:
> On Mon, Jun 02, 2025 at 02:44:33AM +0000, Parav Pandit wrote:
> > When the PCI device is surprise removed, requests may not complete
> > the device as the VQ is marked as broken. Due to this, the disk
> > deletion hangs.
>
> There are loops in the core virtio driver code that expect device
> register reads to eventually return 0:
> drivers/virtio/virtio_pci_modern.c:vp_reset()
> drivers/virtio/virtio_pci_modern_dev.c:vp_modern_set_queue_reset()
>
> Is there a hang if these loops are hit when a device has been surprise
> removed? I'm trying to understand whether surprise removal is fully
> supported or whether this patch is one step in that direction.
>
> Apart from that, I'm happy with the virtio_blk.c aspects of the patch:
> Reviewed-by: Stefan Hajnoczi <stefanha@xxxxxxxxxx>
Is this as simple as this?
-->
Signed-off-by: Michael S. Tsirkin <mst@xxxxxxxxxx>
---
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index 7182f43ed055..df983fa9046a 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -555,8 +555,12 @@ static void vp_reset(struct virtio_device *vdev)
* This will flush out the status write, and flush in device writes,
* including MSI-X interrupts, if any.
*/
- while (vp_modern_get_status(mdev))
+ while (vp_modern_get_status(mdev)) {
+ /* If device is removed meanwhile, it will never respond. */
+ if (!pci_device_is_present(vp_dev->pci_dev))
+ break;
msleep(1);
+ }
vp_modern_avq_cleanup(vdev);
diff --git a/drivers/virtio/virtio_pci_modern_dev.c b/drivers/virtio/virtio_pci_modern_dev.c
index 0d3dbfaf4b23..7177ce0d63be 100644
--- a/drivers/virtio/virtio_pci_modern_dev.c
+++ b/drivers/virtio/virtio_pci_modern_dev.c
@@ -523,11 +523,19 @@ void vp_modern_set_queue_reset(struct virtio_pci_modern_device *mdev, u16 index)
vp_iowrite16(index, &cfg->cfg.queue_select);
vp_iowrite16(1, &cfg->queue_reset);
- while (vp_ioread16(&cfg->queue_reset))
+ while (vp_ioread16(&cfg->queue_reset)) {
+ /* If device is removed meanwhile, it will never respond. */
+ if (!pci_device_is_present(vp_dev->pci_dev))
+ break;
msleep(1);
+ }
- while (vp_ioread16(&cfg->cfg.queue_enable))
+ while (vp_ioread16(&cfg->cfg.queue_enable)) {
+ /* If device is removed meanwhile, it will never respond. */
+ if (!pci_device_is_present(vp_dev->pci_dev))
+ break;
msleep(1);
+ }
}
EXPORT_SYMBOL_GPL(vp_modern_set_queue_reset);
