From: Michael S. Tsirkin mst@redhat.com Sent: 25 June 2025 06:09 PM
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@redhat.com
Is this as simple as this?
Didn't audit the code where else the changes needed. I had similar change a while ago, but I also recall hitting an assert in the pci layer somewhere during the vp_reset() flow.
Do not have lab access today. Will check back tomorrow and update.
-->
Signed-off-by: Michael S. Tsirkin mst@redhat.com
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;- }
- 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;- }
} EXPORT_SYMBOL_GPL(vp_modern_set_queue_reset);