Compatibility note: This series makes virtio-gpu-rutabaga-device incompatible with virtio-mmio-bus, as there is no straightforward way to fix reset for this combination. I believe nobody uses it, but please let me know if anyone relies on it.
Gurchetan, please confirm if this impacts the use case behind your original virtio-gpu-rutabaga contribution. ======================================================================= This series fixes a deadlock I previously observed with the test in [1]. It also fixes a command-ordering race exposed by the same BQL handoff, along with command and fence lifetime and accounting issues found while auditing device and virtqueue reset. Reset now drains command and fence queues without dropping the BQL, while resource destruction and surface replacement are deferred to the next control or cursor BH. Scanout state needed for migration is cleared synchronously before that deferred cleanup. The remaining patches detach popped queue elements before freeing commands, track suspension independently of fencing, balance inflight accounting, handle control-queue reset for the non-renderer backend, and stop advertising queue reset for virtio-gpu-gl, virtio-gpu-rutabaga, and vhost-user-gpu, whose current backend interfaces cannot quiesce a single queue. Finally, the virtio core changes split reset initiation from completion so device reset handlers can choose when reset is complete. An error during reset leaves the device broken and prevents its status from being cleared. Rutabaga's same-thread API requires whole-device reset to finish in the main-loop GPU BH, where the old backend is torn down and reinitialized before completion is reported. Note on the deadlock: I can no longer reproduce the issue reliably with the test in [1], so I used Codex, a coding agent, to write a more reliable local test case, shown below. I applied to Codex for Open Source to get access. The test case is not intended for merge: current policy prohibits that, and it is probably not worth carrying anyway because race-condition tests are inherently fragile. The remaining patches were written by me. [1] https://lore.kernel.org/qemu-devel/[email protected]/ Below is the Codex-written test case: diff --git a/tests/functional/aarch64/test_gpu_blob.py b/tests/functional/aarch64/test_gpu_blob.py index a913d3b29c84..52627b4541f9 100755 --- a/tests/functional/aarch64/test_gpu_blob.py +++ b/tests/functional/aarch64/test_gpu_blob.py @@ -13,7 +13,9 @@ # # SPDX-License-Identifier: GPL-2.0-or-later -from qemu.machine.machine import VMLaunchFailure +import subprocess + +from qemu.machine.machine import AbnormalShutdown, VMLaunchFailure from qemu_test import Asset from qemu_test import wait_for_console_pattern @@ -25,8 +27,7 @@ class Aarch64VirtBlobTest(LinuxKernelTest): 'download?path=%2Fblob-test&files=qemu-880.bin', '2f6ab85d0b156c94fcedd2c4c821c5cbd52925a2de107f8e2d569ea2e34e42eb') - def test_virtio_gpu_blob(self): - + def launch_blob_test(self): self.set_machine('virt') self.require_accelerator("tcg") @@ -65,9 +66,27 @@ def test_virtio_gpu_blob(self): self.log.info("unhandled launch failure: %s", excp.output) raise excp + def test_virtio_gpu_blob(self): + self.launch_blob_test() + self.wait_for_console_pattern('[INFO] virtio-gpu test finished') # the test should cleanly exit + def test_virtio_gpu_blob_shutdown_race(self): + self.launch_blob_test() + + self.wait_for_console_pattern('[INFO] unmapping blob object resource') + + try: + self.vm.shutdown(timeout=10) + except AbnormalShutdown as excp: + if isinstance(excp.__cause__, subprocess.TimeoutExpired): + raise AssertionError( + "QEMU failed to exit while virtio-gpu reset was racing " + "with shutdown") from excp + self.log.info("QEMU exited before the shutdown request completed: %s", + excp) + if __name__ == '__main__': LinuxKernelTest.main() Signed-off-by: Akihiko Odaki <[email protected]> --- Changes in v3: - Rebased. - Dropped the patch "virtio-gpu: Run reset cleanup in the same BH" and reworked reset cleanup to preserve control and cursor queue ordering and migration-visible state without dropping the BQL. - Detached popped virtqueue elements before freeing reset commands. - Added fixes for fenced blob unmapping and inflight counter accounting. - Added control-queue reset handling for the non-renderer backend. - Disabled queue reset for virtio-gpu-gl, virtio-gpu-rutabaga, and vhost-user-gpu, whose current backend interfaces cannot quiesce a single queue. - Split virtio reset initiation from completion, moved completion into device reset handlers, and preserved errors raised during reset. - Added Rutabaga teardown and reinitialization in the main-loop GPU BH before reporting whole-device reset completion. - Link to v2: https://lore.kernel.org/qemu-devel/[email protected] Changes in v2: - Added the patch "virtio-gpu: Run reset cleanup in the same BH". - My assumption about the ordering was incorrect, so I changed the patch to follow the approach used by virtio-gpu-gl. - Link to v1: https://lore.kernel.org/qemu-devel/[email protected] --- Akihiko Odaki (9): virtio: Introduce virtio_complete_reset() virtio: Move the virtio_complete_reset() call virtio: Preserve an error during reset hw/display/virtio-gpu-rutabaga: Reset Rutabaga virtio-gpu: Do not wait for the main thread during reset hw/display/virtio-gpu: Fix fenced blob unmapping hw/display/virtio-gpu: Balance inflight counter changes hw/display/virtio-gpu: Reject queue_reset when unsupported hw/display/virtio-gpu: Handle virtqueue reset include/hw/virtio/virtio-gpu.h | 7 +- include/hw/virtio/virtio.h | 1 + hw/9pfs/virtio-9p-device.c | 1 + hw/audio/virtio-snd.c | 2 + hw/block/vhost-user-blk.c | 1 + hw/block/virtio-blk.c | 1 + hw/char/virtio-serial-bus.c | 1 + hw/display/vhost-user-gpu.c | 10 ++- hw/display/virtio-gpu-base.c | 2 + hw/display/virtio-gpu-gl.c | 12 +++ hw/display/virtio-gpu-rutabaga.c | 29 +++++++ hw/display/virtio-gpu-virgl.c | 25 +++--- hw/display/virtio-gpu.c | 174 +++++++++++++++++++++++++-------------- hw/input/virtio-input.c | 2 + hw/net/virtio-net.c | 1 + hw/scsi/vhost-user-scsi.c | 1 + hw/scsi/virtio-scsi.c | 2 + hw/virtio/virtio-balloon.c | 1 + hw/virtio/virtio-crypto.c | 2 + hw/virtio/virtio.c | 25 ++++-- system/qdev-monitor.c | 2 - 21 files changed, 210 insertions(+), 92 deletions(-) --- base-commit: d2e570cc0f97b936902a5b1b86b73c0f5998b475 change-id: 20251029-gpu-c3f45747f7ba Best regards, -- Akihiko Odaki <[email protected]>
