virtio_reset_device() promises that vq callbacks have finished when it
returns. virtio-pci waits in vp_reset(), but other transports can return
with a callback still running.
Call virtio_synchronize_cbs() after config->reset() and drop the duplicate
waits from both PCI reset methods. Add the wait to virtio_device_shutdown()
too, since it calls config->reset() directly. Keep the pre-reset call under
CONFIG_VIRTIO_HARDEN_NOTIFICATION so callbacks see vq->broken.
Always take irq_lock in the classic virtio-ccw interrupt handler so it
pairs with synchronize_cbs even without notification hardening. Use
is_thinint to choose the lock: airq_info can stay allocated after a
fallback to classic interrupts.
The transport reset must still stop new callbacks before this wait.
Fixes: d9679d0013a6 ("virtio: wrap config->reset calls")
Suggested-by: Michael S. Tsirkin <[email protected]>
Assisted-by: LLM
Signed-off-by: Karl Mehltretter <[email protected]>
---
drivers/s390/virtio/virtio_ccw.c | 6 +-----
drivers/virtio/virtio.c | 2 ++
drivers/virtio/virtio_pci_legacy.c | 2 --
drivers/virtio/virtio_pci_modern.c | 3 ---
include/linux/virtio_config.h | 6 +++---
5 files changed, 6 insertions(+), 13 deletions(-)
diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
index bab6cad3fd5c..552d77998012 100644
--- a/drivers/s390/virtio/virtio_ccw.c
+++ b/drivers/s390/virtio/virtio_ccw.c
@@ -1062,7 +1062,7 @@ static void virtio_ccw_synchronize_cbs(struct
virtio_device *vdev)
struct virtio_ccw_device *vcdev = to_vc_device(vdev);
struct airq_info *info = vcdev->airq_info;
- if (info) {
+ if (vcdev->is_thinint && info) {
/*
* This device uses adapter interrupts: synchronize with
* vring_interrupt() called by virtio_airq_handler()
@@ -1204,13 +1204,11 @@ static void virtio_ccw_int_handler(struct ccw_device
*cdev,
vcdev->err = -EIO;
}
virtio_ccw_check_activity(vcdev, activity);
-#ifdef CONFIG_VIRTIO_HARDEN_NOTIFICATION
/*
* Paired with virtio_ccw_synchronize_cbs() and interrupts are
* disabled here.
*/
read_lock(&vcdev->irq_lock);
-#endif
for_each_set_bit(i, indicators(vcdev),
sizeof(*indicators(vcdev)) * BITS_PER_BYTE) {
/* The bit clear must happen before the vring kick. */
@@ -1219,9 +1217,7 @@ static void virtio_ccw_int_handler(struct ccw_device
*cdev,
vq = virtio_ccw_vq_by_ind(vcdev, i);
vring_interrupt(0, vq);
}
-#ifdef CONFIG_VIRTIO_HARDEN_NOTIFICATION
read_unlock(&vcdev->irq_lock);
-#endif
if (test_bit(0, indicators2(vcdev))) {
virtio_config_changed(&vcdev->vdev);
clear_bit(0, indicators2(vcdev));
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 75bb4ffe3b87..ad1c50b8a94e 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -264,6 +264,7 @@ void virtio_reset_device(struct virtio_device *dev)
#endif
dev->config->reset(dev);
+ virtio_synchronize_cbs(dev);
}
EXPORT_SYMBOL_GPL(virtio_reset_device);
@@ -424,6 +425,7 @@ void virtio_device_shutdown(struct virtio_device *dev)
* Some devices get wedged if this happens, so reset to make sure it
does not.
*/
dev->config->reset(dev);
+ virtio_synchronize_cbs(dev);
}
EXPORT_SYMBOL_GPL(virtio_device_shutdown);
diff --git a/drivers/virtio/virtio_pci_legacy.c
b/drivers/virtio/virtio_pci_legacy.c
index d9cbb02b35a1..8115aa39e01e 100644
--- a/drivers/virtio/virtio_pci_legacy.c
+++ b/drivers/virtio/virtio_pci_legacy.c
@@ -98,8 +98,6 @@ static void vp_reset(struct virtio_device *vdev)
/* Flush out the status write, and flush in device writes,
* including MSi-X interrupts, if any. */
vp_legacy_get_status(&vp_dev->ldev);
- /* Flush pending VQ/configuration callbacks. */
- vp_synchronize_vectors(vdev);
}
static u16 vp_config_vector(struct virtio_pci_device *vp_dev, u16 vector)
diff --git a/drivers/virtio/virtio_pci_modern.c
b/drivers/virtio/virtio_pci_modern.c
index 6d8ae2a6a8ca..c9e21317c51a 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -559,9 +559,6 @@ static void vp_reset(struct virtio_device *vdev)
msleep(1);
vp_modern_avq_cleanup(vdev);
-
- /* Flush pending VQ/configuration callbacks. */
- vp_synchronize_vectors(vdev);
}
static int vp_active_vq(struct virtqueue *vq, u16 msix_vec)
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 69f84ea85d71..8684a1e268ee 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -71,9 +71,9 @@ struct virtqueue_info {
* Returns 0 on success or error status
* @del_vqs: free virtqueues found by find_vqs().
* @synchronize_cbs: synchronize with the virtqueue callbacks (optional)
- * The function guarantees that all memory operations on the
- * queue before it are visible to the vring_interrupt() that is
- * called after it.
+ * Wait for running callbacks to complete. Memory operations on the
+ * queue before this call must be visible to vring_interrupt() calls
+ * that follow it.
* vdev: the virtio_device
* @get_features: get the array of feature bits for this device.
* vdev: the virtio_device
--
2.39.5 (Apple Git-154)
