[PATCH v17 06/18] virtio-gpu: Unset context after GL operations

Dmitry Osipenko posted 18 patches 1 month, 3 weeks 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 v17 06/18] virtio-gpu: Unset context after GL operations
Posted by Dmitry Osipenko 1 month, 3 weeks ago
VirtIO-GPU commands may be processed outside of main-loop, from a CPU
thread. This results in GL context switch failure because context is held
by main-loop thread when GL is invoked from CPU thread. Always release GL
context after performing GL operations such that other threads could take
over the context.

 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)

Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
 hw/display/virtio-gpu-virgl.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 0f754829fb71..cde1a7a8426f 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -896,14 +896,19 @@ static void virgl_cmd_set_scanout_blob(VirtIOGPU *g,
 }
 #endif
 
-void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
-                                      struct virtio_gpu_ctrl_command *cmd)
+static void virtio_gpu_virgl_unset_gl_context(void)
+{
+    dpy_gl_ctx_make_current(qemu_console_lookup_default(), NULL);
+}
+
+static void
+virtio_gpu_virgl_do_process_cmd(VirtIOGPU *g,
+                                struct virtio_gpu_ctrl_command *cmd)
 {
     bool cmd_suspended = false;
 
     VIRTIO_GPU_FILL_CMD(cmd->cmd_hdr);
 
-    virgl_renderer_force_ctx_0();
     switch (cmd->cmd_hdr.type) {
     case VIRTIO_GPU_CMD_CTX_CREATE:
         virgl_cmd_context_create(g, cmd);
@@ -1010,6 +1015,14 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
     virgl_renderer_create_fence(cmd->cmd_hdr.fence_id, cmd->cmd_hdr.type);
 }
 
+void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
+                                  struct virtio_gpu_ctrl_command *cmd)
+{
+    virgl_renderer_force_ctx_0();
+    virtio_gpu_virgl_do_process_cmd(g, cmd);
+    virtio_gpu_virgl_unset_gl_context();
+}
+
 static void virgl_write_fence(void *opaque, uint32_t fence)
 {
     VirtIOGPU *g = opaque;
@@ -1136,8 +1149,10 @@ static void virtio_gpu_fence_poll(void *opaque)
     VirtIOGPU *g = opaque;
     VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
 
+    virgl_renderer_force_ctx_0();
     virgl_renderer_poll();
     virtio_gpu_process_cmdq(g);
+    virtio_gpu_virgl_unset_gl_context();
     if (!QTAILQ_EMPTY(&g->cmdq) || !QTAILQ_EMPTY(&g->fenceq)) {
         timer_mod(gl->fence_poll, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 10);
     }
-- 
2.52.0
Re: [PATCH v17 06/18] virtio-gpu: Unset context after GL operations
Posted by Akihiko Odaki 1 month, 3 weeks ago
On 2026/02/17 4:12, Dmitry Osipenko wrote:
> VirtIO-GPU commands may be processed outside of main-loop, from a CPU
> thread. This results in GL context switch failure because context is held
> by main-loop thread when GL is invoked from CPU thread. Always release GL
> context after performing GL operations such that other threads could take
> over the context.
> 
>   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)
> 
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> ---
>   hw/display/virtio-gpu-virgl.c | 21 ++++++++++++++++++---
>   1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> index 0f754829fb71..cde1a7a8426f 100644
> --- a/hw/display/virtio-gpu-virgl.c
> +++ b/hw/display/virtio-gpu-virgl.c
> @@ -896,14 +896,19 @@ static void virgl_cmd_set_scanout_blob(VirtIOGPU *g,
>   }
>   #endif
>   
> -void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
> -                                      struct virtio_gpu_ctrl_command *cmd)
> +static void virtio_gpu_virgl_unset_gl_context(void)
> +{
> +    dpy_gl_ctx_make_current(qemu_console_lookup_default(), NULL);

An appropriate console needs to be selected as 
virgl_make_context_current() does.

> +}
> +
> +static void
> +virtio_gpu_virgl_do_process_cmd(VirtIOGPU *g,

Although it is a common practice to differentiate functions with having 
a word like "do", I see it as a bad one. The word conveys no meaning at 
all. A more descriptive phrase like "with_ctx" may be better.

Regards,
Akihiko Odaki

> +                                struct virtio_gpu_ctrl_command *cmd)
>   {
>       bool cmd_suspended = false;
>   
>       VIRTIO_GPU_FILL_CMD(cmd->cmd_hdr);
>   
> -    virgl_renderer_force_ctx_0();
>       switch (cmd->cmd_hdr.type) {
>       case VIRTIO_GPU_CMD_CTX_CREATE:
>           virgl_cmd_context_create(g, cmd);
> @@ -1010,6 +1015,14 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
>       virgl_renderer_create_fence(cmd->cmd_hdr.fence_id, cmd->cmd_hdr.type);
>   }
>   
> +void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
> +                                  struct virtio_gpu_ctrl_command *cmd)
> +{
> +    virgl_renderer_force_ctx_0();
> +    virtio_gpu_virgl_do_process_cmd(g, cmd);
> +    virtio_gpu_virgl_unset_gl_context();
> +}
> +
>   static void virgl_write_fence(void *opaque, uint32_t fence)
>   {
>       VirtIOGPU *g = opaque;
> @@ -1136,8 +1149,10 @@ static void virtio_gpu_fence_poll(void *opaque)
>       VirtIOGPU *g = opaque;
>       VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
>   
> +    virgl_renderer_force_ctx_0();
>       virgl_renderer_poll();
>       virtio_gpu_process_cmdq(g);
> +    virtio_gpu_virgl_unset_gl_context();
>       if (!QTAILQ_EMPTY(&g->cmdq) || !QTAILQ_EMPTY(&g->fenceq)) {
>           timer_mod(gl->fence_poll, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 10);
>       }
Re: [PATCH v17 06/18] virtio-gpu: Unset context after GL operations
Posted by Dmitry Osipenko 1 month, 3 weeks ago
Hello Akihiko,

On 2/17/26 09:32, Akihiko Odaki wrote:
> On 2026/02/17 4:12, Dmitry Osipenko wrote:
>> VirtIO-GPU commands may be processed outside of main-loop, from a CPU
>> thread. This results in GL context switch failure because context is held
>> by main-loop thread when GL is invoked from CPU thread. Always release GL
>> context after performing GL operations such that other threads could take
>> over the context.
>>
>>   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)
>>
>> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
>> ---
>>   hw/display/virtio-gpu-virgl.c | 21 ++++++++++++++++++---
>>   1 file changed, 18 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-
>> virgl.c
>> index 0f754829fb71..cde1a7a8426f 100644
>> --- a/hw/display/virtio-gpu-virgl.c
>> +++ b/hw/display/virtio-gpu-virgl.c
>> @@ -896,14 +896,19 @@ static void virgl_cmd_set_scanout_blob(VirtIOGPU
>> *g,
>>   }
>>   #endif
>>   -void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
>> -                                      struct virtio_gpu_ctrl_command
>> *cmd)
>> +static void virtio_gpu_virgl_unset_gl_context(void)
>> +{
>> +    dpy_gl_ctx_make_current(qemu_console_lookup_default(), NULL);
> 
> An appropriate console needs to be selected as
> virgl_make_context_current() does.

See now that virglrender always sets ctx0 for scanout=0. Hence same
scanout=0 can be used when unsetting GL context. Will change it.

>> +}
>> +
>> +static void
>> +virtio_gpu_virgl_do_process_cmd(VirtIOGPU *g,
> 
> Although it is a common practice to differentiate functions with having
> a word like "do", I see it as a bad one. The word conveys no meaning at
> all. A more descriptive phrase like "with_ctx" may be better.

Will rename the func, thanks for the review.

-- 
Best regards,
Dmitry