[PATCH v3 0/9] virtio-gpu: Do not wait for the main thread during reset

Akihiko Odaki posted 9 patches 2 weeks, 4 days ago
Failed in applying to current master (apply log)
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(-)
[PATCH v3 0/9] virtio-gpu: Do not wait for the main thread during reset
Posted by Akihiko Odaki 2 weeks, 4 days ago
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/20251014111234.3190346-9-alex.bennee@linaro.org/

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 <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
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/20260424-gpu-v2-0-9fd2fc0dd1bd@rsg.ci.i.u-tokyo.ac.jp

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/20251029-gpu-v1-1-e3e3c7eebc9e@rsg.ci.i.u-tokyo.ac.jp

---
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 <odaki@rsg.ci.i.u-tokyo.ac.jp>
Re: [PATCH v3 0/9] virtio-gpu: Do not wait for the main thread during reset
Posted by Gurchetan Singh 2 weeks, 1 day ago
On Mon, Sep 7, 2026 at 11:13 PM Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
wrote:

> 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.
>

Yes, deleting the MMIO is fine, but I curious what aspect of the API makes
it difficult say in comparison to virglrenderer/gfxstream?  They all use
guest memory blobs occasionally.  Or is the thread model?


> =======================================================================
>
> 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/20251014111234.3190346-9-alex.bennee@linaro.org/
>
> 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 <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---
> 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/20260424-gpu-v2-0-9fd2fc0dd1bd@rsg.ci.i.u-tokyo.ac.jp
>
> 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/20251029-gpu-v1-1-e3e3c7eebc9e@rsg.ci.i.u-tokyo.ac.jp
>
> ---
> 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 <odaki@rsg.ci.i.u-tokyo.ac.jp>
>
>
Re: [PATCH v3 0/9] virtio-gpu: Do not wait for the main thread during reset
Posted by Akihiko Odaki 2 weeks, 1 day ago
On 2026/09/11 7:43, Gurchetan Singh wrote:
> 
> 
> 
> 
> On Mon, Sep 7, 2026 at 11:13 PM Akihiko Odaki <odaki@rsg.ci.i.u- 
> tokyo.ac.jp <mailto:odaki@rsg.ci.i.u-tokyo.ac.jp>> wrote:
> 
>     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.
> 
> 
> Yes, deleting the MMIO is fine, but I curious what aspect of the API 
> makes it difficult say in comparison to virglrenderer/gfxstream?  They 
> all use guest memory blobs occasionally.  Or is the thread model?

Thanks for confirmation.

It's true that virglrenderer also uses guest memory blobs, and we apply 
the same thread model for virglrenderer and Rutabaga.

The difference is that Rutabaga has a feature that clearly requires 
waiting for proper reset completion: cross-domain. QEMU invokes both 
APIs from the main-loop thread, but cross-domain also has a worker that 
writes responses directly into guest backing. Stopping QEMU's command 
processing does not stop that worker. We therefore need to tear down the 
old Rutabaga instance and join its worker before reporting reset 
completion. Rutabaga requires that teardown to run on the thread that 
initialized it.

The reset timing implemented for virglrenderer looks like as follows:
1. The driver requests reset.
2. The device synchronously notifies reset completion, though actual
    reset is still pending. It makes the device compatible with
    virtio-mmio-bus, which requires synchronous reset completion.
3. The device asynchronously completes actual reset.

The assumption here is that the device may read blobs after notifying 
reset completion to the driver but may not write. Perhaps reading blobs 
after notifying reset completion may cause displaying garbage, but it's 
much better than deadlocks and potential use-after-free we currently have.

cross-domain breaks this assumption. With cross-domain, the following 
may happen:
1. The driver requests reset.
2. The device synchronously notifies reset completion.
3. The driver reuses the blob memory for something else.
4. The device writes to the blob memory, corrupting its content.
5. The device asynchronously completes actual reset.

So we cannot let the device synchronously notify reset completion with 
Rutabaga to make it compatible with virtio-mmio-bus.

Regards,
Akihiko Odaki

> 
> 
>     =======================================================================
> 
>     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/20251014111234.3190346-9-
>     alex.bennee@linaro.org/ <https://lore.kernel.org/qemu-
>     devel/20251014111234.3190346-9-alex.bennee@linaro.org/>
> 
>     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 <http://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 <http://self.log.info>("QEMU exited before the
>     shutdown request completed: %s",
>     +                          excp)
>     +
> 
>       if __name__ == '__main__':
>           LinuxKernelTest.main()
> 
>     Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp
>     <mailto:odaki@rsg.ci.i.u-tokyo.ac.jp>>
>     ---
>     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/20260424-gpu-
>     v2-0-9fd2fc0dd1bd@rsg.ci.i.u-tokyo.ac.jp <https://lore.kernel.org/
>     qemu-devel/20260424-gpu-v2-0-9fd2fc0dd1bd@rsg.ci.i.u-tokyo.ac.jp>
> 
>     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/20251029-gpu-v1-1-
>     e3e3c7eebc9e@rsg.ci.i.u-tokyo.ac.jp <https://lore.kernel.org/qemu-
>     devel/20251029-gpu-v1-1-e3e3c7eebc9e@rsg.ci.i.u-tokyo.ac.jp>
> 
>     ---
>     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 <odaki@rsg.ci.i.u-tokyo.ac.jp
>     <mailto:odaki@rsg.ci.i.u-tokyo.ac.jp>>
> 


[PATCH v3 1/9] virtio: Introduce virtio_complete_reset()
Posted by Akihiko Odaki 2 weeks, 4 days ago
Introduce virtio_complete_reset() as preparation to allow asynchronous
reset. It is unconditionally called in virtio_reset(), but a follow-up
change moves the call to each device implementation.

Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 include/hw/virtio/virtio.h |  1 +
 hw/virtio/virtio.c         | 14 +++++++++++---
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index c99cb19d8865..1431be1389f2 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -507,6 +507,7 @@ int virtio_queue_set_host_notifier_mr(VirtIODevice *vdev, int n,
                                       MemoryRegion *mr, bool assign);
 int virtio_set_status(VirtIODevice *vdev, uint8_t val);
 void virtio_reset(VirtIODevice *vdev);
+void virtio_complete_reset(VirtIODevice *vdev);
 void virtio_queue_reset(VirtIODevice *vdev, uint32_t queue_index);
 void virtio_queue_enable(VirtIODevice *vdev, uint32_t queue_index);
 void virtio_update_irq(VirtIODevice *vdev);
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index daa5607338c9..8d5f58a02df6 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3423,11 +3423,10 @@ int virtio_set_features_ex(VirtIODevice *vdev, const uint64_t *features)
 void virtio_reset(VirtIODevice *vdev)
 {
     VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
-    VirtioSharedMemory *shmem;
-    uint64_t features[VIRTIO_FEATURES_NU64S];
-    int i;
 
     virtio_set_status(vdev, 0);
+    vdev->status = VIRTIO_CONFIG_S_FAILED;
+
     if (current_cpu) {
         /* Guest initiated reset */
         vdev->device_endian = virtio_current_cpu_endian();
@@ -3448,6 +3447,15 @@ void virtio_reset(VirtIODevice *vdev)
         k->reset(vdev);
     }
 
+    virtio_complete_reset(vdev);
+}
+
+void virtio_complete_reset(VirtIODevice *vdev)
+{
+    VirtioSharedMemory *shmem;
+    uint64_t features[VIRTIO_FEATURES_NU64S];
+    int i;
+
     vdev->start_on_kick = false;
     vdev->started = false;
     vdev->broken = false;

-- 
2.55.0
Re: [PATCH v3 1/9] virtio: Introduce virtio_complete_reset()
Posted by Stefan Hajnoczi 2 weeks, 4 days ago
On Tue, Sep 08, 2026 at 03:11:18PM +0900, Akihiko Odaki wrote:
> Introduce virtio_complete_reset() as preparation to allow asynchronous
> reset. It is unconditionally called in virtio_reset(), but a follow-up
> change moves the call to each device implementation.
> 
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---
>  include/hw/virtio/virtio.h |  1 +
>  hw/virtio/virtio.c         | 14 +++++++++++---
>  2 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index c99cb19d8865..1431be1389f2 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -507,6 +507,7 @@ int virtio_queue_set_host_notifier_mr(VirtIODevice *vdev, int n,
>                                        MemoryRegion *mr, bool assign);
>  int virtio_set_status(VirtIODevice *vdev, uint8_t val);
>  void virtio_reset(VirtIODevice *vdev);
> +void virtio_complete_reset(VirtIODevice *vdev);
>  void virtio_queue_reset(VirtIODevice *vdev, uint32_t queue_index);
>  void virtio_queue_enable(VirtIODevice *vdev, uint32_t queue_index);
>  void virtio_update_irq(VirtIODevice *vdev);
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index daa5607338c9..8d5f58a02df6 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -3423,11 +3423,10 @@ int virtio_set_features_ex(VirtIODevice *vdev, const uint64_t *features)
>  void virtio_reset(VirtIODevice *vdev)
>  {
>      VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
> -    VirtioSharedMemory *shmem;
> -    uint64_t features[VIRTIO_FEATURES_NU64S];
> -    int i;
>  
>      virtio_set_status(vdev, 0);
> +    vdev->status = VIRTIO_CONFIG_S_FAILED;

Why is the VIRTIO Device Status Register set to FAILED during reset?
I don't see this in the VIRTIO specification.

> +
>      if (current_cpu) {
>          /* Guest initiated reset */
>          vdev->device_endian = virtio_current_cpu_endian();
> @@ -3448,6 +3447,15 @@ void virtio_reset(VirtIODevice *vdev)
>          k->reset(vdev);
>      }
>  
> +    virtio_complete_reset(vdev);
> +}
> +
> +void virtio_complete_reset(VirtIODevice *vdev)
> +{
> +    VirtioSharedMemory *shmem;
> +    uint64_t features[VIRTIO_FEATURES_NU64S];
> +    int i;
> +
>      vdev->start_on_kick = false;
>      vdev->started = false;
>      vdev->broken = false;
> 
> -- 
> 2.55.0
> 
Re: [PATCH v3 1/9] virtio: Introduce virtio_complete_reset()
Posted by Akihiko Odaki 2 weeks, 3 days ago
On 2026/09/08 23:36, Stefan Hajnoczi wrote:
> On Tue, Sep 08, 2026 at 03:11:18PM +0900, Akihiko Odaki wrote:
>> Introduce virtio_complete_reset() as preparation to allow asynchronous
>> reset. It is unconditionally called in virtio_reset(), but a follow-up
>> change moves the call to each device implementation.
>>
>> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>> ---
>>   include/hw/virtio/virtio.h |  1 +
>>   hw/virtio/virtio.c         | 14 +++++++++++---
>>   2 files changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
>> index c99cb19d8865..1431be1389f2 100644
>> --- a/include/hw/virtio/virtio.h
>> +++ b/include/hw/virtio/virtio.h
>> @@ -507,6 +507,7 @@ int virtio_queue_set_host_notifier_mr(VirtIODevice *vdev, int n,
>>                                         MemoryRegion *mr, bool assign);
>>   int virtio_set_status(VirtIODevice *vdev, uint8_t val);
>>   void virtio_reset(VirtIODevice *vdev);
>> +void virtio_complete_reset(VirtIODevice *vdev);
>>   void virtio_queue_reset(VirtIODevice *vdev, uint32_t queue_index);
>>   void virtio_queue_enable(VirtIODevice *vdev, uint32_t queue_index);
>>   void virtio_update_irq(VirtIODevice *vdev);
>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
>> index daa5607338c9..8d5f58a02df6 100644
>> --- a/hw/virtio/virtio.c
>> +++ b/hw/virtio/virtio.c
>> @@ -3423,11 +3423,10 @@ int virtio_set_features_ex(VirtIODevice *vdev, const uint64_t *features)
>>   void virtio_reset(VirtIODevice *vdev)
>>   {
>>       VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
>> -    VirtioSharedMemory *shmem;
>> -    uint64_t features[VIRTIO_FEATURES_NU64S];
>> -    int i;
>>   
>>       virtio_set_status(vdev, 0);
>> +    vdev->status = VIRTIO_CONFIG_S_FAILED;
> 
> Why is the VIRTIO Device Status Register set to FAILED during reset?
> I don't see this in the VIRTIO specification.

The intent is to keep the status nonzero until virtio_complete_reset() 
finishes the reset. Any bit would work, but I chose FAILED because it 
looks saner than other options; the bit indicates that the guest is not 
going to interact with the device until the reset completes, which is 
indeed the case.

Regards,
Akihiko Odaki

> 
>> +
>>       if (current_cpu) {
>>           /* Guest initiated reset */
>>           vdev->device_endian = virtio_current_cpu_endian();
>> @@ -3448,6 +3447,15 @@ void virtio_reset(VirtIODevice *vdev)
>>           k->reset(vdev);
>>       }
>>   
>> +    virtio_complete_reset(vdev);
>> +}
>> +
>> +void virtio_complete_reset(VirtIODevice *vdev)
>> +{
>> +    VirtioSharedMemory *shmem;
>> +    uint64_t features[VIRTIO_FEATURES_NU64S];
>> +    int i;
>> +
>>       vdev->start_on_kick = false;
>>       vdev->started = false;
>>       vdev->broken = false;
>>
>> -- 
>> 2.55.0
>>
[PATCH v3 2/9] virtio: Move the virtio_complete_reset() call
Posted by Akihiko Odaki 2 weeks, 4 days ago
Move the virtio_complete_reset() call to VirtioDeviceClass::reset
implementations to allow changing the completion timing.

Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 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  | 3 +--
 hw/display/virtio-gpu-base.c | 2 ++
 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           | 7 ++-----
 14 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index 2774fc2290a8..c9de102d39c1 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -115,6 +115,7 @@ static void virtio_9p_reset(VirtIODevice *vdev)
     V9fsVirtioState *v = (V9fsVirtioState *)vdev;
 
     v9fs_reset(&v->state);
+    virtio_complete_reset(vdev);
 }
 
 static ssize_t coroutine_fn
diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index 694bcebb60f6..40bae0cb9c82 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -1383,6 +1383,8 @@ static void virtio_snd_reset(VirtIODevice *vdev)
             virtio_snd_ctrl_cmd_free(cmd);
         }
     }
+
+    virtio_complete_reset(vdev);
 }
 
 static void virtio_snd_class_init(ObjectClass *klass, const void *data)
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 2e5b3ae1b14b..09e52745b31c 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -348,6 +348,7 @@ static void vhost_user_blk_reset(VirtIODevice *vdev)
     VHostUserBlk *s = VHOST_USER_BLK(vdev);
 
     vhost_dev_free_inflight(s->inflight);
+    virtio_complete_reset(vdev);
 }
 
 static int vhost_user_blk_connect(DeviceState *dev, Error **errp)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 6b92066aff4c..a8ae8294fa47 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -1168,6 +1168,7 @@ static void virtio_blk_reset(VirtIODevice *vdev)
     }
 
     blk_set_enable_write_cache(s->blk, s->original_wce);
+    virtio_complete_reset(vdev);
 }
 
 /* coalesce internal state, copy to pci i/o region 0
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index c1973f0248fc..5efca2505990 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -638,6 +638,7 @@ static void vser_reset(VirtIODevice *vdev)
 
     vser = VIRTIO_SERIAL(vdev);
     guest_reset(vser);
+    virtio_complete_reset(vdev);
 }
 
 static void virtio_serial_save_device(VirtIODevice *vdev, QEMUFile *f)
diff --git a/hw/display/vhost-user-gpu.c b/hw/display/vhost-user-gpu.c
index cd684d63639e..f8d640557c28 100644
--- a/hw/display/vhost-user-gpu.c
+++ b/hw/display/vhost-user-gpu.c
@@ -640,9 +640,8 @@ vhost_user_gpu_reset(VirtIODevice *vdev)
 {
     VhostUserGPU *g = VHOST_USER_GPU(vdev);
 
-    virtio_gpu_base_reset(VIRTIO_GPU_BASE(vdev));
-
     vhost_user_backend_stop(g->vhost);
+    virtio_gpu_base_reset(VIRTIO_GPU_BASE(vdev));
 }
 
 static int
diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index 270fbaae1029..d7032ac1def2 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -36,6 +36,8 @@ virtio_gpu_base_reset(VirtIOGPUBase *g)
         g->scanout[i].y = 0;
         g->scanout[i].ds = NULL;
     }
+
+    virtio_complete_reset(VIRTIO_DEVICE(g));
 }
 
 void
diff --git a/hw/input/virtio-input.c b/hw/input/virtio-input.c
index 6494cfbbe82d..0a533a6fff6e 100644
--- a/hw/input/virtio-input.c
+++ b/hw/input/virtio-input.c
@@ -216,6 +216,8 @@ static void virtio_input_reset(VirtIODevice *vdev)
             vic->change_active(vinput);
         }
     }
+
+    virtio_complete_reset(vdev);
 }
 
 static int virtio_input_post_load(void *opaque, int version_id)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 814b99a43d20..bd12780076f4 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -4151,6 +4151,7 @@ static void virtio_net_reset(VirtIODevice *vdev)
     }
 
     virtio_net_disable_rss(n);
+    virtio_complete_reset(vdev);
 }
 
 static void virtio_net_instance_init(Object *obj)
diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
index 5070178dc28b..6f525268f93a 100644
--- a/hw/scsi/vhost-user-scsi.c
+++ b/hw/scsi/vhost-user-scsi.c
@@ -371,6 +371,7 @@ static void vhost_user_scsi_reset(VirtIODevice *vdev)
     VHostSCSICommon *vsc = VHOST_SCSI_COMMON(s);
 
     vhost_dev_free_inflight(vsc->inflight);
+    virtio_complete_reset(vdev);
 }
 
 static struct vhost_dev *vhost_user_scsi_get_vhost(VirtIODevice *vdev)
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index bf64d1231a81..c70cef7ba122 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -1028,6 +1028,8 @@ static void virtio_scsi_reset(VirtIODevice *vdev)
     WITH_QEMU_LOCK_GUARD(&s->event_lock) {
         s->events_dropped = false;
     }
+
+    virtio_complete_reset(vdev);
 }
 
 typedef struct {
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 4c5f486ba238..e3a8545fa612 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -954,6 +954,7 @@ static void virtio_balloon_device_reset(VirtIODevice *vdev)
     }
 
     s->poison_val = 0;
+    virtio_complete_reset(vdev);
 }
 
 static int virtio_balloon_set_status(VirtIODevice *vdev, uint8_t status)
diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
index 79e2acb56cc4..bab09f6f8714 100644
--- a/hw/virtio/virtio-crypto.c
+++ b/hw/virtio/virtio-crypto.c
@@ -1015,6 +1015,8 @@ static void virtio_crypto_reset(VirtIODevice *vdev)
     } else {
         vcrypto->status |= VIRTIO_CRYPTO_S_HW_READY;
     }
+
+    virtio_complete_reset(vdev);
 }
 
 static uint32_t virtio_crypto_init_services(uint32_t qservices)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 8d5f58a02df6..ed43c8bcb0bd 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3443,11 +3443,7 @@ void virtio_reset(VirtIODevice *vdev)
         }
     }
 
-    if (k->reset) {
-        k->reset(vdev);
-    }
-
-    virtio_complete_reset(vdev);
+    k->reset(vdev);
 }
 
 void virtio_complete_reset(VirtIODevice *vdev)
@@ -4483,6 +4479,7 @@ static void virtio_device_class_init(ObjectClass *klass, const void *data)
     dc->unrealize = virtio_device_unrealize;
     dc->bus_type = TYPE_VIRTIO_BUS;
     device_class_set_props(dc, virtio_properties);
+    vdc->reset = virtio_complete_reset;
     vdc->start_ioeventfd = virtio_device_start_ioeventfd_impl;
     vdc->stop_ioeventfd = virtio_device_stop_ioeventfd_impl;
 

-- 
2.55.0
[PATCH v3 3/9] virtio: Preserve an error during reset
Posted by Akihiko Odaki 2 weeks, 4 days ago
A virtio device that had an error during reset will be marked as broken
and will not report reset completion to the guest.

Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 hw/virtio/virtio.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index ed43c8bcb0bd..0b245f73f9d1 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3443,6 +3443,7 @@ void virtio_reset(VirtIODevice *vdev)
         }
     }
 
+    vdev->broken = false;
     k->reset(vdev);
 }
 
@@ -3454,11 +3455,14 @@ void virtio_complete_reset(VirtIODevice *vdev)
 
     vdev->start_on_kick = false;
     vdev->started = false;
-    vdev->broken = false;
     virtio_features_clear(features);
     virtio_set_features_nocheck(vdev, features);
     vdev->queue_sel = 0;
-    vdev->status = 0;
+
+    if (!vdev->broken) {
+        vdev->status = 0;
+    }
+
     vdev->disabled = false;
     qatomic_set(&vdev->isr, 0);
     vdev->config_vector = VIRTIO_NO_VECTOR;

-- 
2.55.0
[PATCH v3 4/9] hw/display/virtio-gpu-rutabaga: Reset Rutabaga
Posted by Akihiko Odaki 2 weeks, 4 days ago
Properly reset Rutabaga so that Rutabaga will not continue to interact
with guest memory, for example.

Fixes: 1dcc6adbc168 ("gfxstream + rutabaga: add initial support for gfxstream")
Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 include/hw/virtio/virtio-gpu.h   |  1 +
 hw/display/virtio-gpu-rutabaga.c | 16 ++++++++++++++++
 hw/display/virtio-gpu.c          |  4 ++++
 3 files changed, 21 insertions(+)

diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 69b5ee2e382f..acae2b4778b3 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -226,6 +226,7 @@ struct VirtIOGPU {
 struct VirtIOGPUClass {
     VirtIOGPUBaseClass parent;
 
+    void (*reset)(VirtIOGPU *g);
     void (*handle_ctrl)(VirtIODevice *vdev, VirtQueue *vq);
     void (*process_cmd)(VirtIOGPU *g, struct virtio_gpu_ctrl_command *cmd);
     void (*update_cursor_data)(VirtIOGPU *g,
diff --git a/hw/display/virtio-gpu-rutabaga.c b/hw/display/virtio-gpu-rutabaga.c
index 041216a10d04..7bdeeeb5c023 100644
--- a/hw/display/virtio-gpu-rutabaga.c
+++ b/hw/display/virtio-gpu-rutabaga.c
@@ -1074,6 +1074,21 @@ virtio_gpu_rutabaga_get_num_capsets(VirtIOGPU *g, uint32_t *num_capsets, Error *
     return true;
 }
 
+static void virtio_gpu_rutabaga_reset(VirtIOGPU *g)
+{
+    VirtIOGPURutabaga *vr = VIRTIO_GPU_RUTABAGA(g);
+    Error *local_err = NULL;
+
+    if (vr->rutabaga) {
+        rutabaga_finish(&vr->rutabaga);
+    }
+
+    if (!virtio_gpu_rutabaga_init(g, &local_err)) {
+        virtio_error(VIRTIO_DEVICE(g), "%s", error_get_pretty(local_err));
+        error_free(local_err);
+    }
+}
+
 static void virtio_gpu_rutabaga_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
 {
     VirtIOGPU *g = VIRTIO_GPU(vdev);
@@ -1156,6 +1171,7 @@ static void virtio_gpu_rutabaga_class_init(ObjectClass *klass, const void *data)
     VirtIOGPUClass *vgc = VIRTIO_GPU_CLASS(klass);
 
     vbc->gl_flushed = virtio_gpu_rutabaga_gl_flushed;
+    vgc->reset = virtio_gpu_rutabaga_reset;
     vgc->handle_ctrl = virtio_gpu_rutabaga_handle_ctrl;
     vgc->process_cmd = virtio_gpu_rutabaga_process_cmd;
     vgc->update_cursor_data = virtio_gpu_rutabaga_update_cursor;
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 55a1c7f80fb8..b57f07bd26c8 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1761,6 +1761,10 @@ static void virtio_gpu_reset_bh(void *opaque)
         qemu_console_set_surface(g->parent_obj.scanout[i].con, NULL);
     }
 
+    if (vgc->reset) {
+        vgc->reset(g);
+    }
+
     g->reset_finished = true;
     qemu_cond_signal(&g->reset_cond);
 }

-- 
2.55.0
[PATCH v3 5/9] virtio-gpu: Do not wait for the main thread during reset
Posted by Akihiko Odaki 2 weeks, 4 days ago
virtio-gpu waits for the main thread to destroy resources and replace
surfaces, but it occasionally results in deadlock, so remove the code
to wait.

In particular, when running a test case[1] the main thread may wait for
the vCPUs to pause during shut down while a vCPU may be concurrently
resetting virtio-gpu.

That split also leaves a window after reset_bh has destroyed resources
and before virtio_gpu_reset() drains the queues. Other virtio-gpu BHs
can run in that window, so commands may be observed on the wrong side of
the reset boundary:

1.  vCPU thread B: Take the BQL
2.  vCPU thread B: Queue a command
3.  vCPU thread B: Drop the BQL
4.  vCPU thread A: Take the BQL
5.  vCPU thread A: Enter virtio_gpu_reset()
6.  vCPU thread A: Schedule reset_bh
7.  vCPU thread A: Wait in qemu_cond_wait_bql(&g->reset_cond)
8.  vCPU thread A: Drop the BQL while waiting
9.  Main thread:   Take the BQL
10. Main thread:   Run virtio_gpu_reset_bh()
11. Main thread:   Destroy resources
12. Main thread:   Signal g->reset_cond
13. Main thread:   Process the queued command
14. Main thread:   Drop the BQL
15. vCPU thread B: Take the BQL
16. vCPU thread B: Queue another command
17. vCPU thread B: Drop the BQL
18. vCPU thread A: Take the BQL
19. vCPU thread A: Leave qemu_cond_wait_bql(&g->reset_cond)
20. vCPU thread A: Discard the second command in the virtqueue

The first command is processed as if it happened after reset, while the
second command is discarded as if it happened before reset.

vCPU actually does not need to perform resource destruction and surface
replacement synchronously, but it only needs to ensure correct ordering
among virtio-gpu operations and migration stream consistency.
virtio-gpu-gl already exploits this fact to ensure that virglrenderer is
reset on the main thread; instead of synchronously resetting
virglrenderer when the device is being reset, it resets virglrenderer
just before processing the first command after the device reset arrives.

Take advantage of this fact by removing synchronization between the main
thread and the resetting vCPU thread. This also fixes the race condition
described above by avoiding unlocking the BQL inside
qemu_cond_wait_bql(&g->reset_cond).

The ordering with the control and cursor queues will be kept enforced by
running the reset operation before processing their commands as
virtio-gpu-gl does.

Migration requires its own treatment and virtio-gpu-gl does not provide
one since it doesn't support migration. For migration consistency,
synchronously clear the device-internal scanout state and also omit
resources from the migration stream if reset is pending. Note that the
cleared scanout will be asynchronously reflected to the UI.

Rutabaga also needs special care. Since it may write guest memory for
the cross-domain feature, we delay reset completion until Rutabaga has
been reset. This makes it incompatible with virtio-mmio-bus.

[1] https://lore.kernel.org/qemu-devel/20251014111234.3190346-9-alex.bennee@linaro.org/

Fixes: a41e2d97f92b ("virtio-gpu: reset gfx resources in main thread")
Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 include/hw/virtio/virtio-gpu.h   |   5 +-
 hw/display/virtio-gpu-rutabaga.c |   2 +
 hw/display/virtio-gpu.c          | 133 ++++++++++++++++++++++++---------------
 system/qdev-monitor.c            |   2 -
 4 files changed, 88 insertions(+), 54 deletions(-)

diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index acae2b4778b3..9404e488208a 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -195,9 +195,7 @@ struct VirtIOGPU {
 
     QEMUBH *ctrl_bh;
     QEMUBH *cursor_bh;
-    QEMUBH *reset_bh;
-    QemuCond reset_cond;
-    bool reset_finished;
+    bool reset_pending;
 
     QTAILQ_HEAD(, virtio_gpu_simple_resource) reslist;
     QTAILQ_HEAD(, virtio_gpu_ctrl_command) cmdq;
@@ -362,6 +360,7 @@ void virtio_gpu_cleanup_mapping(VirtIOGPU *g,
 void virtio_gpu_process_cmdq(VirtIOGPU *g);
 void virtio_gpu_device_realize(DeviceState *qdev, Error **errp);
 void virtio_gpu_reset(VirtIODevice *vdev);
+void virtio_gpu_complete_reset(VirtIOGPU *g);
 void virtio_gpu_simple_process_cmd(VirtIOGPU *g, struct virtio_gpu_ctrl_command *cmd);
 void virtio_gpu_update_cursor_data(VirtIOGPU *g,
                                    struct virtio_gpu_scanout *s,
diff --git a/hw/display/virtio-gpu-rutabaga.c b/hw/display/virtio-gpu-rutabaga.c
index 7bdeeeb5c023..f28daa844328 100644
--- a/hw/display/virtio-gpu-rutabaga.c
+++ b/hw/display/virtio-gpu-rutabaga.c
@@ -1087,6 +1087,8 @@ static void virtio_gpu_rutabaga_reset(VirtIOGPU *g)
         virtio_error(VIRTIO_DEVICE(g), "%s", error_get_pretty(local_err));
         error_free(local_err);
     }
+
+    virtio_gpu_complete_reset(g);
 }
 
 static void virtio_gpu_rutabaga_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index b57f07bd26c8..d520f69e6453 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -26,6 +26,7 @@
 #include "hw/virtio/virtio-gpu-bswap.h"
 #include "hw/virtio/virtio-gpu-pixman.h"
 #include "hw/virtio/virtio-bus.h"
+#include "hw/virtio/virtio-mmio.h"
 #include "hw/core/qdev-properties.h"
 #include "qemu/log.h"
 #include "qemu/memfd.h"
@@ -39,7 +40,7 @@ static struct virtio_gpu_simple_resource *
 virtio_gpu_find_check_resource(VirtIOGPU *g, uint32_t resource_id,
                                const char *caller, uint32_t *error);
 
-static void virtio_gpu_reset_bh(void *opaque);
+static void virtio_gpu_reset_bh(VirtIOGPU *g);
 
 void virtio_gpu_update_cursor_data(VirtIOGPU *g,
                                    struct virtio_gpu_scanout *s,
@@ -380,6 +381,14 @@ static void virtio_gpu_resource_create_blob(VirtIOGPU *g,
     QTAILQ_INSERT_HEAD(&g->reslist, res, next);
 }
 
+static void virtio_gpu_clear_scanout(struct virtio_gpu_scanout *scanout)
+{
+    scanout->resource_id = 0;
+    scanout->ds = NULL;
+    scanout->width = 0;
+    scanout->height = 0;
+}
+
 void virtio_gpu_disable_scanout(VirtIOGPU *g, int scanout_id)
 {
     struct virtio_gpu_scanout *scanout = &g->parent_obj.scanout[scanout_id];
@@ -395,10 +404,7 @@ void virtio_gpu_disable_scanout(VirtIOGPU *g, int scanout_id)
     }
 
     qemu_console_set_surface(scanout->con, NULL);
-    scanout->resource_id = 0;
-    scanout->ds = NULL;
-    scanout->width = 0;
-    scanout->height = 0;
+    virtio_gpu_clear_scanout(scanout);
 }
 
 static void virtio_gpu_resource_destroy(VirtIOGPU *g,
@@ -1267,6 +1273,7 @@ static void virtio_gpu_ctrl_bh(void *opaque)
     VirtIOGPU *g = opaque;
     VirtIOGPUClass *vgc = VIRTIO_GPU_GET_CLASS(g);
 
+    virtio_gpu_reset_bh(g);
     vgc->handle_ctrl(VIRTIO_DEVICE(g), g->ctrl_vq);
 }
 
@@ -1305,6 +1312,8 @@ static void virtio_gpu_handle_cursor(VirtIODevice *vdev, VirtQueue *vq)
 static void virtio_gpu_cursor_bh(void *opaque)
 {
     VirtIOGPU *g = opaque;
+
+    virtio_gpu_reset_bh(g);
     virtio_gpu_handle_cursor(&g->parent_obj.parent_obj, g->cursor_vq);
 }
 
@@ -1371,21 +1380,23 @@ static int virtio_gpu_save(QEMUFile *f, void *opaque, size_t size,
     /* in 2d mode we should never find unprocessed commands here */
     assert(QTAILQ_EMPTY(&g->cmdq));
 
-    QTAILQ_FOREACH(res, &g->reslist, next) {
-        if (!res->image) {
-            continue;
-        }
-        qemu_put_be32(f, res->resource_id);
-        qemu_put_be32(f, res->width);
-        qemu_put_be32(f, res->height);
-        qemu_put_be32(f, res->format);
-        qemu_put_be32(f, res->iov_cnt);
-        for (i = 0; i < res->iov_cnt; i++) {
-            qemu_put_be64(f, res->addrs[i]);
-            qemu_put_be32(f, res->iov[i].iov_len);
+    if (!g->reset_pending) {
+        QTAILQ_FOREACH(res, &g->reslist, next) {
+            if (!res->image) {
+                continue;
+            }
+            qemu_put_be32(f, res->resource_id);
+            qemu_put_be32(f, res->width);
+            qemu_put_be32(f, res->height);
+            qemu_put_be32(f, res->format);
+            qemu_put_be32(f, res->iov_cnt);
+            for (i = 0; i < res->iov_cnt; i++) {
+                qemu_put_be64(f, res->addrs[i]);
+                qemu_put_be32(f, res->iov[i].iov_len);
+            }
+            qemu_put_buffer(f, (void *)pixman_image_get_data(res->image),
+                            pixman_image_get_stride(res->image) * res->height);
         }
-        qemu_put_buffer(f, (void *)pixman_image_get_data(res->image),
-                        pixman_image_get_stride(res->image) * res->height);
     }
     qemu_put_be32(f, 0); /* end of list */
 
@@ -1522,17 +1533,19 @@ static int virtio_gpu_blob_save(QEMUFile *f, void *opaque, size_t size,
     /* in 2d mode we should never find unprocessed commands here */
     assert(QTAILQ_EMPTY(&g->cmdq));
 
-    QTAILQ_FOREACH(res, &g->reslist, next) {
-        if (res->image) {
-            continue;
-        }
-        assert(!res->image);
-        qemu_put_be32(f, res->resource_id);
-        qemu_put_be32(f, res->blob_size);
-        qemu_put_be32(f, res->iov_cnt);
-        for (i = 0; i < res->iov_cnt; i++) {
-            qemu_put_be64(f, res->addrs[i]);
-            qemu_put_be32(f, res->iov[i].iov_len);
+    if (!g->reset_pending) {
+        QTAILQ_FOREACH(res, &g->reslist, next) {
+            if (res->image) {
+                continue;
+            }
+            assert(!res->image);
+            qemu_put_be32(f, res->resource_id);
+            qemu_put_be32(f, res->blob_size);
+            qemu_put_be32(f, res->iov_cnt);
+            for (i = 0; i < res->iov_cnt; i++) {
+                qemu_put_be64(f, res->addrs[i]);
+                qemu_put_be32(f, res->iov[i].iov_len);
+            }
         }
     }
     qemu_put_be32(f, 0); /* end of list */
@@ -1654,8 +1667,16 @@ static int virtio_gpu_post_load(void *opaque, int version_id)
 
 void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
 {
+    BusState *qbus = qdev_get_parent_bus(qdev);
     VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
     VirtIOGPU *g = VIRTIO_GPU(qdev);
+    VirtIOGPUClass *vgc = VIRTIO_GPU_GET_CLASS(g);
+
+    /* virtio-mmio-bus does not support asynchronous reset completion. */
+    if (vgc->reset && object_dynamic_cast(OBJECT(qbus), TYPE_VIRTIO_MMIO_BUS)) {
+        error_setg(errp, "MMIO bus is not supported");
+        return;
+    }
 
     if (virtio_gpu_blob_enabled(g->parent_obj.conf)) {
         if (!virtio_gpu_rutabaga_enabled(g->parent_obj.conf) &&
@@ -1716,8 +1737,6 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
     g->cursor_vq = virtio_get_queue(vdev, 1);
     g->ctrl_bh = virtio_bh_io_new_guarded(qdev, virtio_gpu_ctrl_bh, g);
     g->cursor_bh = virtio_bh_io_new_guarded(qdev, virtio_gpu_cursor_bh, g);
-    g->reset_bh = virtio_bh_io_new_guarded(qdev, virtio_gpu_reset_bh, g);
-    qemu_cond_init(&g->reset_cond);
     QTAILQ_INIT(&g->reslist);
     QTAILQ_INIT(&g->cmdq);
     QTAILQ_INIT(&g->fenceq);
@@ -1729,20 +1748,25 @@ static void virtio_gpu_device_unrealize(DeviceState *qdev)
 
     g_clear_pointer(&g->ctrl_bh, qemu_bh_delete);
     g_clear_pointer(&g->cursor_bh, qemu_bh_delete);
-    g_clear_pointer(&g->reset_bh, qemu_bh_delete);
-    qemu_cond_destroy(&g->reset_cond);
     virtio_gpu_base_device_unrealize(qdev);
 }
 
-static void virtio_gpu_reset_bh(void *opaque)
+static void virtio_gpu_reset_bh(VirtIOGPU *g)
 {
-    VirtIOGPU *g = VIRTIO_GPU(opaque);
     VirtIOGPUClass *vgc = VIRTIO_GPU_GET_CLASS(g);
     struct virtio_gpu_simple_resource *res, *tmp;
     uint32_t resource_id;
     Error *local_err = NULL;
     int i = 0;
 
+    if (!g->reset_pending) {
+        return;
+    }
+
+    for (i = 0; i < g->parent_obj.conf.max_outputs; i++) {
+        qemu_console_set_surface(g->parent_obj.scanout[i].con, NULL);
+    }
+
     QTAILQ_FOREACH_SAFE(res, &g->reslist, next, tmp) {
         resource_id = res->resource_id;
         vgc->resource_destroy(g, res, &local_err);
@@ -1757,32 +1781,43 @@ static void virtio_gpu_reset_bh(void *opaque)
         }
     }
 
-    for (i = 0; i < g->parent_obj.conf.max_outputs; i++) {
-        qemu_console_set_surface(g->parent_obj.scanout[i].con, NULL);
-    }
-
     if (vgc->reset) {
         vgc->reset(g);
     }
 
-    g->reset_finished = true;
-    qemu_cond_signal(&g->reset_cond);
+    g->reset_pending = false;
 }
 
 void virtio_gpu_reset(VirtIODevice *vdev)
 {
     VirtIOGPU *g = VIRTIO_GPU(vdev);
-    struct virtio_gpu_ctrl_command *cmd;
+    VirtIOGPUClass *vgc = VIRTIO_GPU_GET_CLASS(g);
+    int i;
+
+    g->reset_pending = true;
 
     if (qemu_in_vcpu_thread()) {
-        g->reset_finished = false;
-        qemu_bh_schedule(g->reset_bh);
-        while (!g->reset_finished) {
-            qemu_cond_wait_bql(&g->reset_cond);
+        /*
+         * Clear scanouts synchronously so that they are properly migrated
+         * after resetting and before reaching virtio_gpu_reset_bh().
+         */
+        for (i = 0; i < g->parent_obj.conf.max_outputs; i++) {
+            virtio_gpu_clear_scanout(&g->parent_obj.scanout[i]);
         }
+
+        qemu_bh_schedule(g->ctrl_bh);
     } else {
-        aio_bh_call(g->reset_bh);
+        virtio_gpu_reset_bh(g);
+    }
+
+    if (!vgc->reset) {
+        virtio_gpu_complete_reset(g);
     }
+}
+
+void virtio_gpu_complete_reset(VirtIOGPU *g)
+{
+    struct virtio_gpu_ctrl_command *cmd;
 
     while (!QTAILQ_EMPTY(&g->cmdq)) {
         cmd = QTAILQ_FIRST(&g->cmdq);
@@ -1799,7 +1834,7 @@ void virtio_gpu_reset(VirtIODevice *vdev)
         g_free(cmd);
     }
 
-    virtio_gpu_base_reset(VIRTIO_GPU_BASE(vdev));
+    virtio_gpu_base_reset(VIRTIO_GPU_BASE(g));
 }
 
 static void
diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
index 00fed791cce1..19c89a5ad11c 100644
--- a/system/qdev-monitor.c
+++ b/system/qdev-monitor.c
@@ -94,8 +94,6 @@ static const QDevAlias qdev_alias_table[] = {
     { "virtio-gpu-pci", "virtio-gpu", QEMU_ARCH_VIRTIO_PCI },
     { "virtio-gpu-gl-device", "virtio-gpu-gl", QEMU_ARCH_VIRTIO_MMIO },
     { "virtio-gpu-gl-pci", "virtio-gpu-gl", QEMU_ARCH_VIRTIO_PCI },
-    { "virtio-gpu-rutabaga-device", "virtio-gpu-rutabaga",
-      QEMU_ARCH_VIRTIO_MMIO },
     { "virtio-gpu-rutabaga-pci", "virtio-gpu-rutabaga", QEMU_ARCH_VIRTIO_PCI },
     { "virtio-input-host-device", "virtio-input-host", QEMU_ARCH_VIRTIO_MMIO },
     { "virtio-input-host-ccw", "virtio-input-host", QEMU_ARCH_VIRTIO_CCW },

-- 
2.55.0
[PATCH v3 6/9] hw/display/virtio-gpu: Fix fenced blob unmapping
Posted by Akihiko Odaki 2 weeks, 4 days ago
Blob unmapping may be suspended because of remaining uses of the
mapping. virtio_gpu_process_cmdq() uses the following condition to
detect a suspended command:

    !cmd->finished && !(cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_FENCE)

However, the guest may set VIRTIO_GPU_FLAG_FENCE for a command that
performs blob unmapping. Such a command will be incorrectly assumed
as not being suspended. Add a dedicated flag for the suspended state
to avoid overloading VIRTIO_GPU_FLAG_FENCE.

Fixes: 640f9149c3dc ("virtio-gpu: Support suspension of commands processing")
Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 include/hw/virtio/virtio-gpu.h |  1 +
 hw/display/virtio-gpu-virgl.c  | 17 +++++++----------
 hw/display/virtio-gpu.c        |  5 ++---
 3 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 9404e488208a..3eccc4bc39c3 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -141,6 +141,7 @@ struct virtio_gpu_ctrl_command {
     struct virtio_gpu_ctrl_hdr cmd_hdr;
     uint32_t error;
     bool finished;
+    bool suspended;
     QTAILQ_ENTRY(virtio_gpu_ctrl_command) next;
 };
 
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 9bda572426b2..c37df91e3ed1 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -440,8 +440,7 @@ virtio_gpu_virgl_resource_unref(VirtIOGPU *g,
 }
 
 static void virgl_cmd_resource_unref(VirtIOGPU *g,
-                                     struct virtio_gpu_ctrl_command *cmd,
-                                     bool *cmd_suspended)
+                                     struct virtio_gpu_ctrl_command *cmd)
 {
     struct virtio_gpu_resource_unref unref;
     struct virtio_gpu_virgl_resource *res;
@@ -457,7 +456,7 @@ static void virgl_cmd_resource_unref(VirtIOGPU *g,
         return;
     }
 
-    virtio_gpu_virgl_resource_unref(g, res, cmd_suspended);
+    virtio_gpu_virgl_resource_unref(g, res, &cmd->suspended);
 }
 
 void virtio_gpu_virgl_resource_destroy(VirtIOGPU *g,
@@ -946,8 +945,7 @@ static void virgl_cmd_resource_map_blob(VirtIOGPU *g,
 }
 
 static void virgl_cmd_resource_unmap_blob(VirtIOGPU *g,
-                                          struct virtio_gpu_ctrl_command *cmd,
-                                          bool *cmd_suspended)
+                                          struct virtio_gpu_ctrl_command *cmd)
 {
     struct virtio_gpu_resource_unmap_blob ublob;
     struct virtio_gpu_virgl_resource *res;
@@ -964,7 +962,7 @@ static void virgl_cmd_resource_unmap_blob(VirtIOGPU *g,
         return;
     }
 
-    ret = virtio_gpu_virgl_unmap_resource_blob(g, res, cmd_suspended);
+    ret = virtio_gpu_virgl_unmap_resource_blob(g, res, &cmd->suspended);
     if (ret) {
         cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
         return;
@@ -1036,7 +1034,6 @@ static void virgl_cmd_set_scanout_blob(VirtIOGPU *g,
 void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
                                       struct virtio_gpu_ctrl_command *cmd)
 {
-    bool cmd_suspended = false;
     int ret;
 
     VIRTIO_GPU_FILL_CMD(cmd->cmd_hdr);
@@ -1080,7 +1077,7 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
         virgl_cmd_resource_flush(g, cmd);
         break;
     case VIRTIO_GPU_CMD_RESOURCE_UNREF:
-        virgl_cmd_resource_unref(g, cmd, &cmd_suspended);
+        virgl_cmd_resource_unref(g, cmd);
         break;
     case VIRTIO_GPU_CMD_CTX_ATTACH_RESOURCE:
         /* TODO add security */
@@ -1110,7 +1107,7 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
         virgl_cmd_resource_map_blob(g, cmd);
         break;
     case VIRTIO_GPU_CMD_RESOURCE_UNMAP_BLOB:
-        virgl_cmd_resource_unmap_blob(g, cmd, &cmd_suspended);
+        virgl_cmd_resource_unmap_blob(g, cmd);
         break;
     case VIRTIO_GPU_CMD_SET_SCANOUT_BLOB:
         virgl_cmd_set_scanout_blob(g, cmd);
@@ -1121,7 +1118,7 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
         break;
     }
 
-    if (cmd_suspended || cmd->finished) {
+    if (cmd->suspended || cmd->finished) {
         return;
     }
     if (cmd->error) {
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index d520f69e6453..dd6c09d1ddcf 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1193,11 +1193,10 @@ void virtio_gpu_process_cmdq(VirtIOGPU *g)
             break;
         }
 
-        /* process command */
+        cmd->suspended = false;
         vgc->process_cmd(g, cmd);
 
-        /* command suspended */
-        if (!cmd->finished && !(cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_FENCE)) {
+        if (cmd->suspended) {
             trace_virtio_gpu_cmd_suspended(cmd->cmd_hdr.type);
             break;
         }

-- 
2.55.0
Re: [PATCH v3 6/9] hw/display/virtio-gpu: Fix fenced blob unmapping
Posted by Marc-André Lureau 2 weeks, 3 days ago
On Tue, Sep 8, 2026 at 10:13 AM Akihiko Odaki
<odaki@rsg.ci.i.u-tokyo.ac.jp> wrote:
>
> Blob unmapping may be suspended because of remaining uses of the
> mapping. virtio_gpu_process_cmdq() uses the following condition to
> detect a suspended command:
>
>     !cmd->finished && !(cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_FENCE)
>
> However, the guest may set VIRTIO_GPU_FLAG_FENCE for a command that
> performs blob unmapping. Such a command will be incorrectly assumed
> as not being suspended. Add a dedicated flag for the suspended state
> to avoid overloading VIRTIO_GPU_FLAG_FENCE.
>
> Fixes: 640f9149c3dc ("virtio-gpu: Support suspension of commands processing")
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  include/hw/virtio/virtio-gpu.h |  1 +
>  hw/display/virtio-gpu-virgl.c  | 17 +++++++----------
>  hw/display/virtio-gpu.c        |  5 ++---
>  3 files changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> index 9404e488208a..3eccc4bc39c3 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -141,6 +141,7 @@ struct virtio_gpu_ctrl_command {
>      struct virtio_gpu_ctrl_hdr cmd_hdr;
>      uint32_t error;
>      bool finished;
> +    bool suspended;
>      QTAILQ_ENTRY(virtio_gpu_ctrl_command) next;
>  };
>
> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> index 9bda572426b2..c37df91e3ed1 100644
> --- a/hw/display/virtio-gpu-virgl.c
> +++ b/hw/display/virtio-gpu-virgl.c
> @@ -440,8 +440,7 @@ virtio_gpu_virgl_resource_unref(VirtIOGPU *g,
>  }
>
>  static void virgl_cmd_resource_unref(VirtIOGPU *g,
> -                                     struct virtio_gpu_ctrl_command *cmd,
> -                                     bool *cmd_suspended)
> +                                     struct virtio_gpu_ctrl_command *cmd)
>  {
>      struct virtio_gpu_resource_unref unref;
>      struct virtio_gpu_virgl_resource *res;
> @@ -457,7 +456,7 @@ static void virgl_cmd_resource_unref(VirtIOGPU *g,
>          return;
>      }
>
> -    virtio_gpu_virgl_resource_unref(g, res, cmd_suspended);
> +    virtio_gpu_virgl_resource_unref(g, res, &cmd->suspended);
>  }
>
>  void virtio_gpu_virgl_resource_destroy(VirtIOGPU *g,
> @@ -946,8 +945,7 @@ static void virgl_cmd_resource_map_blob(VirtIOGPU *g,
>  }
>
>  static void virgl_cmd_resource_unmap_blob(VirtIOGPU *g,
> -                                          struct virtio_gpu_ctrl_command *cmd,
> -                                          bool *cmd_suspended)
> +                                          struct virtio_gpu_ctrl_command *cmd)
>  {
>      struct virtio_gpu_resource_unmap_blob ublob;
>      struct virtio_gpu_virgl_resource *res;
> @@ -964,7 +962,7 @@ static void virgl_cmd_resource_unmap_blob(VirtIOGPU *g,
>          return;
>      }
>
> -    ret = virtio_gpu_virgl_unmap_resource_blob(g, res, cmd_suspended);
> +    ret = virtio_gpu_virgl_unmap_resource_blob(g, res, &cmd->suspended);
>      if (ret) {
>          cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
>          return;
> @@ -1036,7 +1034,6 @@ static void virgl_cmd_set_scanout_blob(VirtIOGPU *g,
>  void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
>                                        struct virtio_gpu_ctrl_command *cmd)
>  {
> -    bool cmd_suspended = false;
>      int ret;
>
>      VIRTIO_GPU_FILL_CMD(cmd->cmd_hdr);
> @@ -1080,7 +1077,7 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
>          virgl_cmd_resource_flush(g, cmd);
>          break;
>      case VIRTIO_GPU_CMD_RESOURCE_UNREF:
> -        virgl_cmd_resource_unref(g, cmd, &cmd_suspended);
> +        virgl_cmd_resource_unref(g, cmd);
>          break;
>      case VIRTIO_GPU_CMD_CTX_ATTACH_RESOURCE:
>          /* TODO add security */
> @@ -1110,7 +1107,7 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
>          virgl_cmd_resource_map_blob(g, cmd);
>          break;
>      case VIRTIO_GPU_CMD_RESOURCE_UNMAP_BLOB:
> -        virgl_cmd_resource_unmap_blob(g, cmd, &cmd_suspended);
> +        virgl_cmd_resource_unmap_blob(g, cmd);
>          break;
>      case VIRTIO_GPU_CMD_SET_SCANOUT_BLOB:
>          virgl_cmd_set_scanout_blob(g, cmd);
> @@ -1121,7 +1118,7 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
>          break;
>      }
>
> -    if (cmd_suspended || cmd->finished) {
> +    if (cmd->suspended || cmd->finished) {
>          return;
>      }
>      if (cmd->error) {
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index d520f69e6453..dd6c09d1ddcf 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -1193,11 +1193,10 @@ void virtio_gpu_process_cmdq(VirtIOGPU *g)
>              break;
>          }
>
> -        /* process command */
> +        cmd->suspended = false;
>          vgc->process_cmd(g, cmd);
>
> -        /* command suspended */
> -        if (!cmd->finished && !(cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_FENCE)) {
> +        if (cmd->suspended) {
>              trace_virtio_gpu_cmd_suspended(cmd->cmd_hdr.type);
>              break;
>          }
>
> --
> 2.55.0
>
Re: [PATCH v3 6/9] hw/display/virtio-gpu: Fix fenced blob unmapping
Posted by Marc-André Lureau 2 weeks, 4 days ago
Hi

On Tue, Sep 8, 2026 at 10:14 AM Akihiko Odaki
<odaki@rsg.ci.i.u-tokyo.ac.jp> wrote:
>
> Blob unmapping may be suspended because of remaining uses of the
> mapping. virtio_gpu_process_cmdq() uses the following condition to
> detect a suspended command:
>
>     !cmd->finished && !(cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_FENCE)
>
> However, the guest may set VIRTIO_GPU_FLAG_FENCE for a command that
> performs blob unmapping. Such a command will be incorrectly assumed
> as not being suspended. Add a dedicated flag for the suspended state
> to avoid overloading VIRTIO_GPU_FLAG_FENCE.
>
> Fixes: 640f9149c3dc ("virtio-gpu: Support suspension of commands processing")
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---
>  include/hw/virtio/virtio-gpu.h |  1 +
>  hw/display/virtio-gpu-virgl.c  | 17 +++++++----------
>  hw/display/virtio-gpu.c        |  5 ++---
>  3 files changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> index 9404e488208a..3eccc4bc39c3 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -141,6 +141,7 @@ struct virtio_gpu_ctrl_command {
>      struct virtio_gpu_ctrl_hdr cmd_hdr;
>      uint32_t error;
>      bool finished;
> +    bool suspended;

>      QTAILQ_ENTRY(virtio_gpu_ctrl_command) next;
>  };
>
> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> index 9bda572426b2..c37df91e3ed1 100644
> --- a/hw/display/virtio-gpu-virgl.c
> +++ b/hw/display/virtio-gpu-virgl.c
> @@ -440,8 +440,7 @@ virtio_gpu_virgl_resource_unref(VirtIOGPU *g,
>  }
>
>  static void virgl_cmd_resource_unref(VirtIOGPU *g,
> -                                     struct virtio_gpu_ctrl_command *cmd,
> -                                     bool *cmd_suspended)
> +                                     struct virtio_gpu_ctrl_command *cmd)
>  {
>      struct virtio_gpu_resource_unref unref;
>      struct virtio_gpu_virgl_resource *res;
> @@ -457,7 +456,7 @@ static void virgl_cmd_resource_unref(VirtIOGPU *g,
>          return;
>      }
>
> -    virtio_gpu_virgl_resource_unref(g, res, cmd_suspended);
> +    virtio_gpu_virgl_resource_unref(g, res, &cmd->suspended);
>  }
>
>  void virtio_gpu_virgl_resource_destroy(VirtIOGPU *g,
> @@ -946,8 +945,7 @@ static void virgl_cmd_resource_map_blob(VirtIOGPU *g,
>  }
>
>  static void virgl_cmd_resource_unmap_blob(VirtIOGPU *g,
> -                                          struct virtio_gpu_ctrl_command *cmd,
> -                                          bool *cmd_suspended)
> +                                          struct virtio_gpu_ctrl_command *cmd)
>  {
>      struct virtio_gpu_resource_unmap_blob ublob;
>      struct virtio_gpu_virgl_resource *res;
> @@ -964,7 +962,7 @@ static void virgl_cmd_resource_unmap_blob(VirtIOGPU *g,
>          return;
>      }
>
> -    ret = virtio_gpu_virgl_unmap_resource_blob(g, res, cmd_suspended);
> +    ret = virtio_gpu_virgl_unmap_resource_blob(g, res, &cmd->suspended);
>      if (ret) {
>          cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
>          return;
> @@ -1036,7 +1034,6 @@ static void virgl_cmd_set_scanout_blob(VirtIOGPU *g,
>  void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
>                                        struct virtio_gpu_ctrl_command *cmd)
>  {
> -    bool cmd_suspended = false;
>      int ret;
>
>      VIRTIO_GPU_FILL_CMD(cmd->cmd_hdr);
> @@ -1080,7 +1077,7 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
>          virgl_cmd_resource_flush(g, cmd);
>          break;
>      case VIRTIO_GPU_CMD_RESOURCE_UNREF:
> -        virgl_cmd_resource_unref(g, cmd, &cmd_suspended);
> +        virgl_cmd_resource_unref(g, cmd);
>          break;
>      case VIRTIO_GPU_CMD_CTX_ATTACH_RESOURCE:
>          /* TODO add security */
> @@ -1110,7 +1107,7 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
>          virgl_cmd_resource_map_blob(g, cmd);
>          break;
>      case VIRTIO_GPU_CMD_RESOURCE_UNMAP_BLOB:
> -        virgl_cmd_resource_unmap_blob(g, cmd, &cmd_suspended);
> +        virgl_cmd_resource_unmap_blob(g, cmd);
>          break;
>      case VIRTIO_GPU_CMD_SET_SCANOUT_BLOB:
>          virgl_cmd_set_scanout_blob(g, cmd);
> @@ -1121,7 +1118,7 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
>          break;
>      }
>
> -    if (cmd_suspended || cmd->finished) {
> +    if (cmd->suspended || cmd->finished) {
>          return;
>      }
>      if (cmd->error) {
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index d520f69e6453..dd6c09d1ddcf 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -1193,11 +1193,10 @@ void virtio_gpu_process_cmdq(VirtIOGPU *g)
>              break;
>          }
>
> -        /* process command */
> +        cmd->suspended = false;

The field could be initialized after virtqueue_pop(), otherwise it has
a random allocation value. Then perhaps, drop this assignment?

>          vgc->process_cmd(g, cmd);
>
> -        /* command suspended */
> -        if (!cmd->finished && !(cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_FENCE)) {
> +        if (cmd->suspended) {
>              trace_virtio_gpu_cmd_suspended(cmd->cmd_hdr.type);
>              break;
>          }
>
> --
> 2.55.0
>
>
Re: [PATCH v3 6/9] hw/display/virtio-gpu: Fix fenced blob unmapping
Posted by Akihiko Odaki 2 weeks, 3 days ago
On 2026/09/08 19:27, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Sep 8, 2026 at 10:14 AM Akihiko Odaki
> <odaki@rsg.ci.i.u-tokyo.ac.jp> wrote:
>>
>> Blob unmapping may be suspended because of remaining uses of the
>> mapping. virtio_gpu_process_cmdq() uses the following condition to
>> detect a suspended command:
>>
>>      !cmd->finished && !(cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_FENCE)
>>
>> However, the guest may set VIRTIO_GPU_FLAG_FENCE for a command that
>> performs blob unmapping. Such a command will be incorrectly assumed
>> as not being suspended. Add a dedicated flag for the suspended state
>> to avoid overloading VIRTIO_GPU_FLAG_FENCE.
>>
>> Fixes: 640f9149c3dc ("virtio-gpu: Support suspension of commands processing")
>> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>> ---
>>   include/hw/virtio/virtio-gpu.h |  1 +
>>   hw/display/virtio-gpu-virgl.c  | 17 +++++++----------
>>   hw/display/virtio-gpu.c        |  5 ++---
>>   3 files changed, 10 insertions(+), 13 deletions(-)
>>
>> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
>> index 9404e488208a..3eccc4bc39c3 100644
>> --- a/include/hw/virtio/virtio-gpu.h
>> +++ b/include/hw/virtio/virtio-gpu.h
>> @@ -141,6 +141,7 @@ struct virtio_gpu_ctrl_command {
>>       struct virtio_gpu_ctrl_hdr cmd_hdr;
>>       uint32_t error;
>>       bool finished;
>> +    bool suspended;
> 
>>       QTAILQ_ENTRY(virtio_gpu_ctrl_command) next;
>>   };
>>
>> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
>> index 9bda572426b2..c37df91e3ed1 100644
>> --- a/hw/display/virtio-gpu-virgl.c
>> +++ b/hw/display/virtio-gpu-virgl.c
>> @@ -440,8 +440,7 @@ virtio_gpu_virgl_resource_unref(VirtIOGPU *g,
>>   }
>>
>>   static void virgl_cmd_resource_unref(VirtIOGPU *g,
>> -                                     struct virtio_gpu_ctrl_command *cmd,
>> -                                     bool *cmd_suspended)
>> +                                     struct virtio_gpu_ctrl_command *cmd)
>>   {
>>       struct virtio_gpu_resource_unref unref;
>>       struct virtio_gpu_virgl_resource *res;
>> @@ -457,7 +456,7 @@ static void virgl_cmd_resource_unref(VirtIOGPU *g,
>>           return;
>>       }
>>
>> -    virtio_gpu_virgl_resource_unref(g, res, cmd_suspended);
>> +    virtio_gpu_virgl_resource_unref(g, res, &cmd->suspended);
>>   }
>>
>>   void virtio_gpu_virgl_resource_destroy(VirtIOGPU *g,
>> @@ -946,8 +945,7 @@ static void virgl_cmd_resource_map_blob(VirtIOGPU *g,
>>   }
>>
>>   static void virgl_cmd_resource_unmap_blob(VirtIOGPU *g,
>> -                                          struct virtio_gpu_ctrl_command *cmd,
>> -                                          bool *cmd_suspended)
>> +                                          struct virtio_gpu_ctrl_command *cmd)
>>   {
>>       struct virtio_gpu_resource_unmap_blob ublob;
>>       struct virtio_gpu_virgl_resource *res;
>> @@ -964,7 +962,7 @@ static void virgl_cmd_resource_unmap_blob(VirtIOGPU *g,
>>           return;
>>       }
>>
>> -    ret = virtio_gpu_virgl_unmap_resource_blob(g, res, cmd_suspended);
>> +    ret = virtio_gpu_virgl_unmap_resource_blob(g, res, &cmd->suspended);
>>       if (ret) {
>>           cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
>>           return;
>> @@ -1036,7 +1034,6 @@ static void virgl_cmd_set_scanout_blob(VirtIOGPU *g,
>>   void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
>>                                         struct virtio_gpu_ctrl_command *cmd)
>>   {
>> -    bool cmd_suspended = false;
>>       int ret;
>>
>>       VIRTIO_GPU_FILL_CMD(cmd->cmd_hdr);
>> @@ -1080,7 +1077,7 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
>>           virgl_cmd_resource_flush(g, cmd);
>>           break;
>>       case VIRTIO_GPU_CMD_RESOURCE_UNREF:
>> -        virgl_cmd_resource_unref(g, cmd, &cmd_suspended);
>> +        virgl_cmd_resource_unref(g, cmd);
>>           break;
>>       case VIRTIO_GPU_CMD_CTX_ATTACH_RESOURCE:
>>           /* TODO add security */
>> @@ -1110,7 +1107,7 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
>>           virgl_cmd_resource_map_blob(g, cmd);
>>           break;
>>       case VIRTIO_GPU_CMD_RESOURCE_UNMAP_BLOB:
>> -        virgl_cmd_resource_unmap_blob(g, cmd, &cmd_suspended);
>> +        virgl_cmd_resource_unmap_blob(g, cmd);
>>           break;
>>       case VIRTIO_GPU_CMD_SET_SCANOUT_BLOB:
>>           virgl_cmd_set_scanout_blob(g, cmd);
>> @@ -1121,7 +1118,7 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
>>           break;
>>       }
>>
>> -    if (cmd_suspended || cmd->finished) {
>> +    if (cmd->suspended || cmd->finished) {
>>           return;
>>       }
>>       if (cmd->error) {
>> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
>> index d520f69e6453..dd6c09d1ddcf 100644
>> --- a/hw/display/virtio-gpu.c
>> +++ b/hw/display/virtio-gpu.c
>> @@ -1193,11 +1193,10 @@ void virtio_gpu_process_cmdq(VirtIOGPU *g)
>>               break;
>>           }
>>
>> -        /* process command */
>> +        cmd->suspended = false;
> 
> The field could be initialized after virtqueue_pop(), otherwise it has
> a random allocation value. Then perhaps, drop this assignment?

The assignment is needed before every processing attempt, including
retries. A suspended command stays at the head of cmdq and is processed
again when unmapping can complete. The unmap helper sets suspended to
true when waiting, but does not clear it on completion. Initializing it
only after virtqueue_pop() would leave it true on retry, preventing the
command from completing and blocking subsequent commands.

The field is uninitialized immediately after allocation, but nothing
reads it before this assignment, so there is no uninitialized read.

Regards,
Akihiko Odaki

> 
>>           vgc->process_cmd(g, cmd);
>>
>> -        /* command suspended */
>> -        if (!cmd->finished && !(cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_FENCE)) {
>> +        if (cmd->suspended) {
>>               trace_virtio_gpu_cmd_suspended(cmd->cmd_hdr.type);
>>               break;
>>           }
>>
>> --
>> 2.55.0
>>
>>


[PATCH v3 7/9] hw/display/virtio-gpu: Balance inflight counter changes
Posted by Akihiko Odaki 2 weeks, 4 days ago
virtio_gpu_virgl_async_fence_bh() decrements g->inflight for each
asynchronous fence, but it is incremented for each fenced command though
an asynchronous fence may complete several or no fenced commands.
Change it to decrement g->inflight for each fenced command to balance
the increment and the decrement.

Fixes: 6698eae8a1f6 ("virtio-gpu: Support asynchronous fencing")
Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 hw/display/virtio-gpu-virgl.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index c37df91e3ed1..20a5d06d9a3a 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -1266,14 +1266,14 @@ static void virtio_gpu_virgl_async_fence_bh(void *opaque)
             virtio_gpu_ctrl_response_nodata(g, cmd, VIRTIO_GPU_RESP_OK_NODATA);
             QTAILQ_REMOVE(&g->fenceq, cmd, next);
             g_free(cmd);
+            g->inflight--;
+            if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
+                trace_virtio_gpu_dec_inflight_fences(g->inflight);
+            }
         }
 
         trace_virtio_gpu_fence_resp(f->fence_id);
         g_free(f);
-        g->inflight--;
-        if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
-            trace_virtio_gpu_dec_inflight_fences(g->inflight);
-        }
     }
 }
 

-- 
2.55.0
[PATCH v3 8/9] hw/display/virtio-gpu: Reject queue_reset when unsupported
Posted by Akihiko Odaki 2 weeks, 4 days ago
Making queue_reset functional with virtio-gpu-gl, virtio-gpu-rutabaga,
and vhost-user-gpu requires adding corresponding APIs to their backends.
queue_reset is useless for them after all since they do not have
multiple queues that contend for buffers. It is not necessary to keep
queue_reset available for migration compatibility since they don't
support migration. Reject queue_reset for these devices.

Fixes: 69e1c14aa222 ("virtio: core: vq reset feature negotation support")
Fixes: 1dcc6adbc168 ("gfxstream + rutabaga: add initial support for gfxstream")
Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 hw/display/vhost-user-gpu.c      |  7 +++++++
 hw/display/virtio-gpu-gl.c       | 12 ++++++++++++
 hw/display/virtio-gpu-rutabaga.c | 11 +++++++++++
 3 files changed, 30 insertions(+)

diff --git a/hw/display/vhost-user-gpu.c b/hw/display/vhost-user-gpu.c
index f8d640557c28..880a9fecc81a 100644
--- a/hw/display/vhost-user-gpu.c
+++ b/hw/display/vhost-user-gpu.c
@@ -625,6 +625,8 @@ vhost_user_gpu_instance_init(Object *obj)
     g->vhost = VHOST_USER_BACKEND(object_new(TYPE_VHOST_USER_BACKEND));
     object_property_add_alias(obj, "chardev",
                               OBJECT(g->vhost), "chardev");
+
+    object_property_set_bool(obj, "queue_reset", false, &error_abort);
 }
 
 static void
@@ -661,6 +663,11 @@ vhost_user_gpu_device_realize(DeviceState *qdev, Error **errp)
     VhostUserGPU *g = VHOST_USER_GPU(qdev);
     VirtIODevice *vdev = VIRTIO_DEVICE(g);
 
+    if (virtio_host_has_feature(VIRTIO_DEVICE(qdev), VIRTIO_F_RING_RESET)) {
+        error_setg(errp, "queue_reset is not supported");
+        return;
+    }
+
     vhost_dev_set_config_notifier(&g->vhost->dev, &config_ops);
     if (vhost_user_backend_dev_init(g->vhost, vdev, 2, errp) < 0) {
         return;
diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index 2b7a41c46643..d215e897c1d5 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -17,6 +17,7 @@
 #include "qemu/module.h"
 #include "qemu/error-report.h"
 #include "qapi/error.h"
+#include "qom/object.h"
 #include "system/system.h"
 #include "hw/virtio/virtio.h"
 #include "hw/virtio/virtio-gpu.h"
@@ -119,6 +120,11 @@ static void virtio_gpu_gl_device_realize(DeviceState *qdev, Error **errp)
     return;
 #endif
 
+    if (virtio_host_has_feature(VIRTIO_DEVICE(qdev), VIRTIO_F_RING_RESET)) {
+        error_setg(errp, "queue_reset is not supported");
+        return;
+    }
+
     if (!object_resolve_path_type("", TYPE_VIRTIO_GPU_GL, NULL)) {
         error_setg(errp, "at most one %s device is permitted", TYPE_VIRTIO_GPU_GL);
         return;
@@ -209,6 +215,11 @@ static void virtio_gpu_gl_device_unrealize(DeviceState *qdev)
      */
 }
 
+static void virtio_gpu_gl_instance_init(Object *obj)
+{
+    object_property_set_bool(obj, "queue_reset", false, &error_abort);
+}
+
 static void virtio_gpu_gl_class_init(ObjectClass *klass, const void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -232,6 +243,7 @@ static const TypeInfo virtio_gpu_gl_info = {
     .name = TYPE_VIRTIO_GPU_GL,
     .parent = TYPE_VIRTIO_GPU,
     .instance_size = sizeof(VirtIOGPUGL),
+    .instance_init = virtio_gpu_gl_instance_init,
     .class_init = virtio_gpu_gl_class_init,
 };
 module_obj(TYPE_VIRTIO_GPU_GL);
diff --git a/hw/display/virtio-gpu-rutabaga.c b/hw/display/virtio-gpu-rutabaga.c
index f28daa844328..c77e72c88e96 100644
--- a/hw/display/virtio-gpu-rutabaga.c
+++ b/hw/display/virtio-gpu-rutabaga.c
@@ -1124,6 +1124,11 @@ static void virtio_gpu_rutabaga_realize(DeviceState *qdev, Error **errp)
     return;
 #endif
 
+    if (virtio_host_has_feature(VIRTIO_DEVICE(qdev), VIRTIO_F_RING_RESET)) {
+        error_setg(errp, "queue_reset is not supported");
+        return;
+    }
+
     error_setg(&bdev->migration_blocker, "rutabaga is not yet migratable");
     if (migrate_add_blocker(&bdev->migration_blocker, errp) < 0) {
         return;
@@ -1165,6 +1170,11 @@ static const Property virtio_gpu_rutabaga_properties[] = {
     DEFINE_PROP_STRING("wsi", VirtIOGPURutabaga, wsi),
 };
 
+static void virtio_gpu_rutabaga_instance_init(Object *obj)
+{
+    object_property_set_bool(obj, "queue_reset", false, &error_abort);
+}
+
 static void virtio_gpu_rutabaga_class_init(ObjectClass *klass, const void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -1187,6 +1197,7 @@ static const TypeInfo virtio_gpu_rutabaga_info[] = {
         .name = TYPE_VIRTIO_GPU_RUTABAGA,
         .parent = TYPE_VIRTIO_GPU,
         .instance_size = sizeof(VirtIOGPURutabaga),
+        .instance_init = virtio_gpu_rutabaga_instance_init,
         .class_init = virtio_gpu_rutabaga_class_init,
     },
 };

-- 
2.55.0
[PATCH v3 9/9] hw/display/virtio-gpu: Handle virtqueue reset
Posted by Akihiko Odaki 2 weeks, 4 days ago
Implement the missing logic to reset the state specific to virtio-gpu
associated with the command queue for virtqueue reset.

Fixes: 69e1c14aa222 ("virtio: core: vq reset feature negotation support")
Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 hw/display/virtio-gpu.c | 44 +++++++++++++++++++++++++++-----------------
 1 file changed, 27 insertions(+), 17 deletions(-)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index dd6c09d1ddcf..a4b722d7f139 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1750,6 +1750,31 @@ static void virtio_gpu_device_unrealize(DeviceState *qdev)
     virtio_gpu_base_device_unrealize(qdev);
 }
 
+static void virtio_gpu_queue_reset(VirtIODevice *vdev, uint32_t queue_index)
+{
+    VirtIOGPU *g = VIRTIO_GPU(vdev);
+    struct virtio_gpu_ctrl_command *cmd;
+
+    if (queue_index) {
+        return;
+    }
+
+    while (!QTAILQ_EMPTY(&g->cmdq)) {
+        cmd = QTAILQ_FIRST(&g->cmdq);
+        QTAILQ_REMOVE(&g->cmdq, cmd, next);
+        virtqueue_detach_element(cmd->vq, &cmd->elem, 0);
+        g_free(cmd);
+    }
+
+    while (!QTAILQ_EMPTY(&g->fenceq)) {
+        cmd = QTAILQ_FIRST(&g->fenceq);
+        QTAILQ_REMOVE(&g->fenceq, cmd, next);
+        virtqueue_detach_element(cmd->vq, &cmd->elem, 0);
+        g->inflight--;
+        g_free(cmd);
+    }
+}
+
 static void virtio_gpu_reset_bh(VirtIOGPU *g)
 {
     VirtIOGPUClass *vgc = VIRTIO_GPU_GET_CLASS(g);
@@ -1816,23 +1841,7 @@ void virtio_gpu_reset(VirtIODevice *vdev)
 
 void virtio_gpu_complete_reset(VirtIOGPU *g)
 {
-    struct virtio_gpu_ctrl_command *cmd;
-
-    while (!QTAILQ_EMPTY(&g->cmdq)) {
-        cmd = QTAILQ_FIRST(&g->cmdq);
-        QTAILQ_REMOVE(&g->cmdq, cmd, next);
-        virtqueue_detach_element(cmd->vq, &cmd->elem, 0);
-        g_free(cmd);
-    }
-
-    while (!QTAILQ_EMPTY(&g->fenceq)) {
-        cmd = QTAILQ_FIRST(&g->fenceq);
-        QTAILQ_REMOVE(&g->fenceq, cmd, next);
-        virtqueue_detach_element(cmd->vq, &cmd->elem, 0);
-        g->inflight--;
-        g_free(cmd);
-    }
-
+    virtio_gpu_queue_reset(VIRTIO_DEVICE(g), 0);
     virtio_gpu_base_reset(VIRTIO_GPU_BASE(g));
 }
 
@@ -1940,6 +1949,7 @@ static void virtio_gpu_class_init(ObjectClass *klass, const void *data)
     vdc->realize = virtio_gpu_device_realize;
     vdc->unrealize = virtio_gpu_device_unrealize;
     vdc->reset = virtio_gpu_reset;
+    vdc->queue_reset = virtio_gpu_queue_reset;
     vdc->get_config = virtio_gpu_get_config;
     vdc->set_config = virtio_gpu_set_config;
 

-- 
2.55.0