[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Stable-8.2.7 16/53] hw/virtio: Fix the de-initialization of vhost-user
From: |
Michael Tokarev |
Subject: |
[Stable-8.2.7 16/53] hw/virtio: Fix the de-initialization of vhost-user devices |
Date: |
Fri, 6 Sep 2024 09:53:46 +0300 |
From: Thomas Huth <thuth@redhat.com>
The unrealize functions of the various vhost-user devices are
calling the corresponding vhost_*_set_status() functions with a
status of 0 to shut down the device correctly.
Now these vhost_*_set_status() functions all follow this scheme:
bool should_start = virtio_device_should_start(vdev, status);
if (vhost_dev_is_started(&vvc->vhost_dev) == should_start) {
return;
}
if (should_start) {
/* ... do the initialization stuff ... */
} else {
/* ... do the cleanup stuff ... */
}
The problem here is virtio_device_should_start(vdev, 0) currently
always returns "true" since it internally only looks at vdev->started
instead of looking at the "status" parameter. Thus once the device
got started once, virtio_device_should_start() always returns true
and thus the vhost_*_set_status() functions return early, without
ever doing any clean-up when being called with status == 0. This
causes e.g. problems when trying to hot-plug and hot-unplug a vhost
user devices multiple times since the de-initialization step is
completely skipped during the unplug operation.
This bug has been introduced in commit 9f6bcfd99f ("hw/virtio: move
vm_running check to virtio_device_started") which replaced
should_start = status & VIRTIO_CONFIG_S_DRIVER_OK;
with
should_start = virtio_device_started(vdev, status);
which later got replaced by virtio_device_should_start(). This blocked
the possibility to set should_start to false in case the status flag
VIRTIO_CONFIG_S_DRIVER_OK was not set.
Fix it by adjusting the virtio_device_should_start() function to
only consider the status flag instead of vdev->started. Since this
function is only used in the various vhost_*_set_status() functions
for exactly the same purpose, it should be fine to fix it in this
central place there without any risk to change the behavior of other
code.
Fixes: 9f6bcfd99f ("hw/virtio: move vm_running check to virtio_device_started")
Buglink: https://issues.redhat.com/browse/RHEL-40708
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20240618121958.88673-1-thuth@redhat.com>
Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
(cherry picked from commit d72479b11797c28893e1e3fc565497a9cae5ca16)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 7d5ffdc145..2eafad17b8 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -470,9 +470,9 @@ static inline bool virtio_device_started(VirtIODevice
*vdev, uint8_t status)
* @vdev - the VirtIO device
* @status - the devices status bits
*
- * This is similar to virtio_device_started() but also encapsulates a
- * check on the VM status which would prevent a device starting
- * anyway.
+ * This is similar to virtio_device_started() but ignores vdev->started
+ * and also encapsulates a check on the VM status which would prevent a
+ * device from starting anyway.
*/
static inline bool virtio_device_should_start(VirtIODevice *vdev, uint8_t
status)
{
@@ -480,7 +480,7 @@ static inline bool virtio_device_should_start(VirtIODevice
*vdev, uint8_t status
return false;
}
- return virtio_device_started(vdev, status);
+ return status & VIRTIO_CONFIG_S_DRIVER_OK;
}
static inline void virtio_set_started(VirtIODevice *vdev, bool started)
--
2.39.2
- [Stable-8.2.7 08/53] hw/cxl/cxl-host: Fix segmentation fault when getting cxl-fmw property, (continued)
- [Stable-8.2.7 08/53] hw/cxl/cxl-host: Fix segmentation fault when getting cxl-fmw property, Michael Tokarev, 2024/09/06
- [Stable-8.2.7 06/53] hvf: arm: Do not advance PC when raising an exception, Michael Tokarev, 2024/09/06
- [Stable-8.2.7 09/53] virtio-snd: add max size bounds check in input cb, Michael Tokarev, 2024/09/06
- [Stable-8.2.7 10/53] virtio-snd: check for invalid param shift operands, Michael Tokarev, 2024/09/06
- [Stable-8.2.7 11/53] intel_iommu: fix FRCD construction macro, Michael Tokarev, 2024/09/06
- [Stable-8.2.7 14/53] hw/intc/loongson_ipi: Access memory in little endian, Michael Tokarev, 2024/09/06
- [Stable-8.2.7 15/53] util/async.c: Forbid negative min/max in aio_context_set_thread_pool_params(), Michael Tokarev, 2024/09/06
- [Stable-8.2.7 12/53] target/i386: do not crash if microvm guest uses SGX CPUID leaves, Michael Tokarev, 2024/09/06
- [Stable-8.2.7 13/53] chardev/char-win-stdio.c: restore old console mode, Michael Tokarev, 2024/09/06
- [Stable-8.2.7 17/53] target/rx: Use target_ulong for address in LI, Michael Tokarev, 2024/09/06
- [Stable-8.2.7 16/53] hw/virtio: Fix the de-initialization of vhost-user devices,
Michael Tokarev <=
- [Stable-8.2.7 18/53] hw/char/bcm2835_aux: Fix assert when receive FIFO fills up, Michael Tokarev, 2024/09/06
- [Stable-8.2.7 19/53] hw/misc/bcm2835_property: Fix handling of FRAMEBUFFER_SET_PALETTE, Michael Tokarev, 2024/09/06
- [Stable-8.2.7 20/53] target/arm: Don't assert for 128-bit tile accesses when SVL is 128, Michael Tokarev, 2024/09/06
- [Stable-8.2.7 21/53] target/arm: Fix UMOPA/UMOPS of 16-bit values, Michael Tokarev, 2024/09/06
- [Stable-8.2.7 22/53] target/arm: Avoid shifts by -1 in tszimm_shr() and tszimm_shl(), Michael Tokarev, 2024/09/06
- [Stable-8.2.7 23/53] target/arm: Ignore SMCR_EL2.LEN and SVCR_EL2.LEN if EL2 is not enabled, Michael Tokarev, 2024/09/06
- [Stable-8.2.7 24/53] docs/sphinx/depfile.py: Handle env.doc2path() returning a Path not a str, Michael Tokarev, 2024/09/06
- [Stable-8.2.7 25/53] hw/i386/amd_iommu: Don't leak memory in amdvi_update_iotlb(), Michael Tokarev, 2024/09/06
- [Stable-8.2.7 26/53] hw/arm/mps2-tz.c: fix RX/TX interrupts order, Michael Tokarev, 2024/09/06
- [Stable-8.2.7 28/53] virtio-net: Ensure queue index fits with RSS, Michael Tokarev, 2024/09/06