Hi
On Tue, Sep 8, 2026 at 10:14 AM Akihiko Odaki
<[email protected]> 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 <[email protected]>
> ---
> 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
>
>
