[PATCH v21 07/18] virtio-gpu: Ensure BHs are invoked only from main-loop thread

Dmitry Osipenko posted 18 patches 1 month, 1 week ago
Maintainers: Pierrick Bouvier <pierrick.bouvier@linaro.org>, "Alex Bennée" <alex.bennee@linaro.org>, Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>, Dmitry Osipenko <dmitry.osipenko@collabora.com>, "Michael S. Tsirkin" <mst@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>
There is a newer version of this series
[PATCH v21 07/18] virtio-gpu: Ensure BHs are invoked only from main-loop thread
Posted by Dmitry Osipenko 1 month, 1 week ago
QEMU's display GL core is tied to main-loop thread and virtio-gpu
interacts with display while processing GPU commands. Virtio-gpu BHs
work in generic AIO context that can be invoked on vCPU thread, while
GL and UI toolkits are bound to the main-loop thread.

Make virtio-gpu BHs use iohandler AIO context that is handled in a
main-loop thread only.

 0  SDL_GL_MakeCurrent() (libSDL3)
 1  SDL_GL_MakeCurrent_REAL() (libSDL2)
 2  sdl2_gl_make_context_current() (ui/sdl2-gl.c:201)
 3  make_current() (virglrenderer.c:639)
 4  vrend_finish_context_switch() (vrend_renderer.c:11630)
 5  vrend_hw_switch_context() (vrend_renderer.c:11613)
 6  vrend_renderer_force_ctx_0() (vrend_renderer.c:12986)
 7  virgl_renderer_force_ctx_0() (virglrenderer.c:460)
 8  virtio_gpu_virgl_process_cmd() (virtio-gpu-virgl.c:1013)
 9  virtio_gpu_process_cmdq() (virtio-gpu.c:1050)
 10 virtio_gpu_gl_handle_ctrl() (virtio-gpu-gl.c:86)
 11 aio_bh_poll() (util/async.c)
 12 aio_poll() (util/aio-posix.c)
 13 blk_pwrite() (block/block-gen.c:1985)
 14 pflash_update() (pflash_cfi01.c:396)
 15 pflash_write() (pflash_cfi01.c:541)
 16 memory_region_dispatch_write() (system/memory.c:1554)
 17 flatview_write() (system/physmem.c:3333)
 18 address_space_write() (system/physmem.c:3453)
 19 kvm_cpu_exec() (accel/kvm/kall-all.c:3248)
 20 kvm_vcpu_thread_fn() (accel/kvm/kaccel-ops.c:53)

Cc: qemu-stable@nongnu.org
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
 hw/display/virtio-gpu-virgl.c | 4 +---
 hw/display/virtio-gpu.c       | 6 +++---
 include/hw/virtio/virtio.h    | 7 +++++++
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 0f754829fb71..97baf9b8ca96 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -1203,9 +1203,7 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
     }
 
 #if VIRGL_VERSION_MAJOR >= 1
-    gl->cmdq_resume_bh = aio_bh_new(qemu_get_aio_context(),
-                                    virtio_gpu_virgl_resume_cmdq_bh,
-                                    g);
+    gl->cmdq_resume_bh = virtio_bh_io_new(virtio_gpu_virgl_resume_cmdq_bh, g);
 #endif
 
     return 0;
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 643e91ca2a7a..0d88e7b3cdff 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1526,9 +1526,9 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
 
     g->ctrl_vq = virtio_get_queue(vdev, 0);
     g->cursor_vq = virtio_get_queue(vdev, 1);
-    g->ctrl_bh = virtio_bh_new_guarded(qdev, virtio_gpu_ctrl_bh, g);
-    g->cursor_bh = virtio_bh_new_guarded(qdev, virtio_gpu_cursor_bh, g);
-    g->reset_bh = qemu_bh_new(virtio_gpu_reset_bh, g);
+    g->ctrl_bh = virtio_bh_io_new(virtio_gpu_ctrl_bh, g);
+    g->cursor_bh = virtio_bh_io_new(virtio_gpu_cursor_bh, g);
+    g->reset_bh = virtio_bh_io_new(virtio_gpu_reset_bh, g);
     qemu_cond_init(&g->reset_cond);
     QTAILQ_INIT(&g->reslist);
     QTAILQ_INIT(&g->cmdq);
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 9dd93cf965ae..bc1ddd3bc4e0 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -547,4 +547,11 @@ QEMUBH *virtio_bh_new_guarded_full(DeviceState *dev,
 #define virtio_bh_new_guarded(dev, cb, opaque) \
     virtio_bh_new_guarded_full((dev), (cb), (opaque), (stringify(cb)))
 
+/*
+ * The "_io" variant runs BH only on a main-loop thread, while generic BH
+ * may run on a vCPU thread.
+ */
+#define virtio_bh_io_new(cb, opaque) \
+    aio_bh_new(iohandler_get_aio_context(), (cb), (opaque))
+
 #endif
-- 
2.52.0
Re: [PATCH v21 07/18] virtio-gpu: Ensure BHs are invoked only from main-loop thread
Posted by Akihiko Odaki 1 month, 1 week ago
On 2026/03/02 23:59, Dmitry Osipenko wrote:
> QEMU's display GL core is tied to main-loop thread and virtio-gpu
> interacts with display while processing GPU commands. Virtio-gpu BHs
> work in generic AIO context that can be invoked on vCPU thread, while
> GL and UI toolkits are bound to the main-loop thread.
> 
> Make virtio-gpu BHs use iohandler AIO context that is handled in a
> main-loop thread only.
> 
>   0  SDL_GL_MakeCurrent() (libSDL3)
>   1  SDL_GL_MakeCurrent_REAL() (libSDL2)
>   2  sdl2_gl_make_context_current() (ui/sdl2-gl.c:201)
>   3  make_current() (virglrenderer.c:639)
>   4  vrend_finish_context_switch() (vrend_renderer.c:11630)
>   5  vrend_hw_switch_context() (vrend_renderer.c:11613)
>   6  vrend_renderer_force_ctx_0() (vrend_renderer.c:12986)
>   7  virgl_renderer_force_ctx_0() (virglrenderer.c:460)
>   8  virtio_gpu_virgl_process_cmd() (virtio-gpu-virgl.c:1013)
>   9  virtio_gpu_process_cmdq() (virtio-gpu.c:1050)
>   10 virtio_gpu_gl_handle_ctrl() (virtio-gpu-gl.c:86)
>   11 aio_bh_poll() (util/async.c)
>   12 aio_poll() (util/aio-posix.c)
>   13 blk_pwrite() (block/block-gen.c:1985)
>   14 pflash_update() (pflash_cfi01.c:396)
>   15 pflash_write() (pflash_cfi01.c:541)
>   16 memory_region_dispatch_write() (system/memory.c:1554)
>   17 flatview_write() (system/physmem.c:3333)
>   18 address_space_write() (system/physmem.c:3453)
>   19 kvm_cpu_exec() (accel/kvm/kall-all.c:3248)
>   20 kvm_vcpu_thread_fn() (accel/kvm/kaccel-ops.c:53)
> 
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> ---
>   hw/display/virtio-gpu-virgl.c | 4 +---
>   hw/display/virtio-gpu.c       | 6 +++---
>   include/hw/virtio/virtio.h    | 7 +++++++
>   3 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> index 0f754829fb71..97baf9b8ca96 100644
> --- a/hw/display/virtio-gpu-virgl.c
> +++ b/hw/display/virtio-gpu-virgl.c
> @@ -1203,9 +1203,7 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
>       }
>   
>   #if VIRGL_VERSION_MAJOR >= 1
> -    gl->cmdq_resume_bh = aio_bh_new(qemu_get_aio_context(),
> -                                    virtio_gpu_virgl_resume_cmdq_bh,
> -                                    g);
> +    gl->cmdq_resume_bh = virtio_bh_io_new(virtio_gpu_virgl_resume_cmdq_bh, g);
>   #endif
>   
>       return 0;
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 643e91ca2a7a..0d88e7b3cdff 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -1526,9 +1526,9 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
>   
>       g->ctrl_vq = virtio_get_queue(vdev, 0);
>       g->cursor_vq = virtio_get_queue(vdev, 1);
> -    g->ctrl_bh = virtio_bh_new_guarded(qdev, virtio_gpu_ctrl_bh, g);
> -    g->cursor_bh = virtio_bh_new_guarded(qdev, virtio_gpu_cursor_bh, g);
> -    g->reset_bh = qemu_bh_new(virtio_gpu_reset_bh, g);
> +    g->ctrl_bh = virtio_bh_io_new(virtio_gpu_ctrl_bh, g);
> +    g->cursor_bh = virtio_bh_io_new(virtio_gpu_cursor_bh, g);
> +    g->reset_bh = virtio_bh_io_new(virtio_gpu_reset_bh, g);
>       qemu_cond_init(&g->reset_cond);
>       QTAILQ_INIT(&g->reslist);
>       QTAILQ_INIT(&g->cmdq);
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index 9dd93cf965ae..bc1ddd3bc4e0 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -547,4 +547,11 @@ QEMUBH *virtio_bh_new_guarded_full(DeviceState *dev,
>   #define virtio_bh_new_guarded(dev, cb, opaque) \
>       virtio_bh_new_guarded_full((dev), (cb), (opaque), (stringify(cb)))
>   
> +/*
> + * The "_io" variant runs BH only on a main-loop thread, while generic BH
> + * may run on a vCPU thread.
> + */
> +#define virtio_bh_io_new(cb, opaque) \
> +    aio_bh_new(iohandler_get_aio_context(), (cb), (opaque))
> +

The reentrancy guard virtio_bh_new_guarded() implements should be preserved.

Regards,
Akihiko Odaki

>   #endif
Re: [PATCH v21 07/18] virtio-gpu: Ensure BHs are invoked only from main-loop thread
Posted by Dmitry Osipenko 1 month, 1 week ago
On 3/3/26 07:30, Akihiko Odaki wrote:
> On 2026/03/02 23:59, Dmitry Osipenko wrote:
>> QEMU's display GL core is tied to main-loop thread and virtio-gpu
>> interacts with display while processing GPU commands. Virtio-gpu BHs
>> work in generic AIO context that can be invoked on vCPU thread, while
>> GL and UI toolkits are bound to the main-loop thread.
>>
>> Make virtio-gpu BHs use iohandler AIO context that is handled in a
>> main-loop thread only.
>>
>>   0  SDL_GL_MakeCurrent() (libSDL3)
>>   1  SDL_GL_MakeCurrent_REAL() (libSDL2)
>>   2  sdl2_gl_make_context_current() (ui/sdl2-gl.c:201)
>>   3  make_current() (virglrenderer.c:639)
>>   4  vrend_finish_context_switch() (vrend_renderer.c:11630)
>>   5  vrend_hw_switch_context() (vrend_renderer.c:11613)
>>   6  vrend_renderer_force_ctx_0() (vrend_renderer.c:12986)
>>   7  virgl_renderer_force_ctx_0() (virglrenderer.c:460)
>>   8  virtio_gpu_virgl_process_cmd() (virtio-gpu-virgl.c:1013)
>>   9  virtio_gpu_process_cmdq() (virtio-gpu.c:1050)
>>   10 virtio_gpu_gl_handle_ctrl() (virtio-gpu-gl.c:86)
>>   11 aio_bh_poll() (util/async.c)
>>   12 aio_poll() (util/aio-posix.c)
>>   13 blk_pwrite() (block/block-gen.c:1985)
>>   14 pflash_update() (pflash_cfi01.c:396)
>>   15 pflash_write() (pflash_cfi01.c:541)
>>   16 memory_region_dispatch_write() (system/memory.c:1554)
>>   17 flatview_write() (system/physmem.c:3333)
>>   18 address_space_write() (system/physmem.c:3453)
>>   19 kvm_cpu_exec() (accel/kvm/kall-all.c:3248)
>>   20 kvm_vcpu_thread_fn() (accel/kvm/kaccel-ops.c:53)
>>
>> Cc: qemu-stable@nongnu.org
>> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
>> ---
>>   hw/display/virtio-gpu-virgl.c | 4 +---
>>   hw/display/virtio-gpu.c       | 6 +++---
>>   include/hw/virtio/virtio.h    | 7 +++++++
>>   3 files changed, 11 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-
>> virgl.c
>> index 0f754829fb71..97baf9b8ca96 100644
>> --- a/hw/display/virtio-gpu-virgl.c
>> +++ b/hw/display/virtio-gpu-virgl.c
>> @@ -1203,9 +1203,7 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
>>       }
>>     #if VIRGL_VERSION_MAJOR >= 1
>> -    gl->cmdq_resume_bh = aio_bh_new(qemu_get_aio_context(),
>> -                                    virtio_gpu_virgl_resume_cmdq_bh,
>> -                                    g);
>> +    gl->cmdq_resume_bh =
>> virtio_bh_io_new(virtio_gpu_virgl_resume_cmdq_bh, g);
>>   #endif
>>         return 0;
>> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
>> index 643e91ca2a7a..0d88e7b3cdff 100644
>> --- a/hw/display/virtio-gpu.c
>> +++ b/hw/display/virtio-gpu.c
>> @@ -1526,9 +1526,9 @@ void virtio_gpu_device_realize(DeviceState
>> *qdev, Error **errp)
>>         g->ctrl_vq = virtio_get_queue(vdev, 0);
>>       g->cursor_vq = virtio_get_queue(vdev, 1);
>> -    g->ctrl_bh = virtio_bh_new_guarded(qdev, virtio_gpu_ctrl_bh, g);
>> -    g->cursor_bh = virtio_bh_new_guarded(qdev, virtio_gpu_cursor_bh, g);
>> -    g->reset_bh = qemu_bh_new(virtio_gpu_reset_bh, g);
>> +    g->ctrl_bh = virtio_bh_io_new(virtio_gpu_ctrl_bh, g);
>> +    g->cursor_bh = virtio_bh_io_new(virtio_gpu_cursor_bh, g);
>> +    g->reset_bh = virtio_bh_io_new(virtio_gpu_reset_bh, g);
>>       qemu_cond_init(&g->reset_cond);
>>       QTAILQ_INIT(&g->reslist);
>>       QTAILQ_INIT(&g->cmdq);
>> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
>> index 9dd93cf965ae..bc1ddd3bc4e0 100644
>> --- a/include/hw/virtio/virtio.h
>> +++ b/include/hw/virtio/virtio.h
>> @@ -547,4 +547,11 @@ QEMUBH *virtio_bh_new_guarded_full(DeviceState *dev,
>>   #define virtio_bh_new_guarded(dev, cb, opaque) \
>>       virtio_bh_new_guarded_full((dev), (cb), (opaque), (stringify(cb)))
>>   +/*
>> + * The "_io" variant runs BH only on a main-loop thread, while
>> generic BH
>> + * may run on a vCPU thread.
>> + */
>> +#define virtio_bh_io_new(cb, opaque) \
>> +    aio_bh_new(iohandler_get_aio_context(), (cb), (opaque))
>> +
> 
> The reentrancy guard virtio_bh_new_guarded() implements should be
> preserved.

Thanks, now I see that the guard was addressing bug report that should
be reproducible. Will take a closer look as it wasn't obvious to me how
re-entrance could happen with virtio-gpu.

-- 
Best regards,
Dmitry