Re: [RFC PATCH v6 3/4] virtio-gpu: Destroy virgl resources on virtio-gpu reset

[Dmitry Osipenko <dmitry.osipenko@collabora.com>]

On 12/5/25 07:10, Akihiko Odaki wrote:
> On 2025/12/02 8:34, Dmitry Osipenko wrote:
>> Properly destroy virgl resources on virtio-gpu reset to not leak
>> resources
>> on a hot reboot of a VM.
>>
>> Suggested-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
>> ---
>>   hw/display/virtio-gpu-gl.c     |  6 ++-
>>   hw/display/virtio-gpu-virgl.c  | 84 +++++++++++++++++++++++++---------
>>   include/hw/virtio/virtio-gpu.h |  5 +-
>>   3 files changed, 72 insertions(+), 23 deletions(-)
>>
>> diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
>> index b640900fc6f1..bf3fd75e9e6b 100644
>> --- a/hw/display/virtio-gpu-gl.c
>> +++ b/hw/display/virtio-gpu-gl.c
>> @@ -72,7 +72,10 @@ static void virtio_gpu_gl_handle_ctrl(VirtIODevice
>> *vdev, VirtQueue *vq)
>>         switch (gl->renderer_state) {
>>       case RS_RESET:
>> -        virtio_gpu_virgl_reset(g);
>> +        if (virtio_gpu_virgl_reset(g)) {
>> +            gl->renderer_state = RS_INIT_FAILED;
> 
> The state should stay RS_RESET so that the suspended work will be
> resumed later.
> 
>> +            return;
>> +        }
>>           /* fallthrough */
>>       case RS_START:
>>           if (virtio_gpu_virgl_init(g)) {
>> @@ -201,6 +204,7 @@ static void virtio_gpu_gl_class_init(ObjectClass
>> *klass, const void *data)
>>       vgc->process_cmd = virtio_gpu_virgl_process_cmd;
>>       vgc->update_cursor_data = virtio_gpu_gl_update_cursor_data;
>>   +    vgc->resource_destroy = virtio_gpu_virgl_resource_destroy;
>>       vdc->realize = virtio_gpu_gl_device_realize;
>>       vdc->unrealize = virtio_gpu_gl_device_unrealize;
>>       vdc->reset = virtio_gpu_gl_reset;
>> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-
>> virgl.c
>> index 6a2aac0b6e5c..2fc5c746396c 100644
>> --- a/hw/display/virtio-gpu-virgl.c
>> +++ b/hw/display/virtio-gpu-virgl.c
>> @@ -304,14 +304,46 @@ static void
>> virgl_cmd_create_resource_3d(VirtIOGPU *g,
>>       virgl_renderer_resource_create(&args, NULL, 0);
>>   }
>>   +static int
>> +virtio_gpu_virgl_resource_unref(VirtIOGPU *g,
>> +                                struct virtio_gpu_virgl_resource *res,
>> +                                bool *cmd_suspended)
> 
> "cmd_suspended" is now a bit confusing name because it is triggered with
> device reset which is not a command in the virtio terminology.
> "suspended" will work and is a bit shorter too.

Will change it all in v17, thanks!

-- 
Best regards,
Dmitry