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 | 79 +++++++++++++++++++++++++---------
include/hw/virtio/virtio-gpu.h | 5 ++-
3 files changed, 67 insertions(+), 23 deletions(-)
diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index b640900fc6f1..bca7d489c1e3 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) < 0) {
+ gl->renderer_state = RS_INIT_FAILED;
+ 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 ae8de8ee47da..24d329022da9 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -311,14 +311,44 @@ 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)
+{
+ struct iovec *res_iovs = NULL;
+ int num_iovs = 0;
+
+#if VIRGL_VERSION_MAJOR >= 1
+ if (virtio_gpu_virgl_unmap_resource_blob(g, res, cmd_suspended) < 0) {
+ return -1;
+ }
+ if (*cmd_suspended) {
+ return 0;
+ }
+#endif
+
+ virgl_renderer_resource_detach_iov(res->base.resource_id,
+ &res_iovs,
+ &num_iovs);
+ if (res_iovs != NULL && num_iovs != 0) {
+ virtio_gpu_cleanup_mapping_iov(g, res_iovs, num_iovs);
+ }
+ virgl_renderer_resource_unref(res->base.resource_id);
+
+ QTAILQ_REMOVE(&g->reslist, &res->base, next);
+
+ g_free(res);
+
+ return 0;
+}
+
static void virgl_cmd_resource_unref(VirtIOGPU *g,
struct virtio_gpu_ctrl_command *cmd,
bool *cmd_suspended)
{
struct virtio_gpu_resource_unref unref;
struct virtio_gpu_virgl_resource *res;
- struct iovec *res_iovs = NULL;
- int num_iovs = 0;
VIRTIO_GPU_FILL_CMD(unref);
trace_virtio_gpu_cmd_res_unref(unref.resource_id);
@@ -331,27 +361,21 @@ static void virgl_cmd_resource_unref(VirtIOGPU *g,
return;
}
-#if VIRGL_VERSION_MAJOR >= 1
- if (virtio_gpu_virgl_unmap_resource_blob(g, res, cmd_suspended) < 0) {
- cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
- return;
- }
- if (*cmd_suspended) {
- return;
- }
-#endif
+ virtio_gpu_virgl_resource_unref(g, res, cmd_suspended);
+}
- virgl_renderer_resource_detach_iov(unref.resource_id,
- &res_iovs,
- &num_iovs);
- if (res_iovs != NULL && num_iovs != 0) {
- virtio_gpu_cleanup_mapping_iov(g, res_iovs, num_iovs);
- }
- virgl_renderer_resource_unref(unref.resource_id);
+void virtio_gpu_virgl_resource_destroy(VirtIOGPU *g,
+ struct virtio_gpu_simple_resource *base,
+ Error **errp)
+{
+ struct virtio_gpu_virgl_resource *res;
+ bool suspended = false;
- QTAILQ_REMOVE(&g->reslist, &res->base, next);
+ res = container_of(base, struct virtio_gpu_virgl_resource, base);
- g_free(res);
+ if (virtio_gpu_virgl_resource_unref(g, res, &suspended) < 0) {
+ error_setg(errp, "failed to destroy virgl resource");
+ }
}
static void virgl_cmd_context_create(VirtIOGPU *g,
@@ -1270,11 +1294,24 @@ void virtio_gpu_virgl_reset_scanout(VirtIOGPU *g)
}
}
-void virtio_gpu_virgl_reset(VirtIOGPU *g)
+int virtio_gpu_virgl_reset(VirtIOGPU *g)
{
+ struct virtio_gpu_simple_resource *res, *tmp;
+
+ QTAILQ_FOREACH_SAFE(res, &g->reslist, next, tmp) {
+ virtio_gpu_virgl_resource_destroy(g, res, NULL);
+ }
+
+ if (!QTAILQ_EMPTY(&g->reslist)) {
+ error_report("failed to reset virgl resources");
+ return -1;
+ }
+
virgl_renderer_reset();
virtio_gpu_virgl_reset_async_fences(g);
+
+ return 0;
}
int virtio_gpu_virgl_init(VirtIOGPU *g)
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 718332284305..9e1473d1bb66 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -389,9 +389,12 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
struct virtio_gpu_ctrl_command *cmd);
void virtio_gpu_virgl_fence_poll(VirtIOGPU *g);
void virtio_gpu_virgl_reset_scanout(VirtIOGPU *g);
-void virtio_gpu_virgl_reset(VirtIOGPU *g);
+int virtio_gpu_virgl_reset(VirtIOGPU *g);
int virtio_gpu_virgl_init(VirtIOGPU *g);
GArray *virtio_gpu_virgl_get_capsets(VirtIOGPU *g);
void virtio_gpu_virgl_reset_async_fences(VirtIOGPU *g);
+void virtio_gpu_virgl_resource_destroy(VirtIOGPU *g,
+ struct virtio_gpu_simple_resource *res,
+ Error **errp);
#endif
--
2.51.1On 2025/11/20 13:06, 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 | 79 +++++++++++++++++++++++++---------
> include/hw/virtio/virtio-gpu.h | 5 ++-
> 3 files changed, 67 insertions(+), 23 deletions(-)
>
> diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
> index b640900fc6f1..bca7d489c1e3 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) < 0) {
> + gl->renderer_state = RS_INIT_FAILED;
> + 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 ae8de8ee47da..24d329022da9 100644
> --- a/hw/display/virtio-gpu-virgl.c
> +++ b/hw/display/virtio-gpu-virgl.c
> @@ -311,14 +311,44 @@ 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)
> +{
> + struct iovec *res_iovs = NULL;
> + int num_iovs = 0;
> +
> +#if VIRGL_VERSION_MAJOR >= 1
> + if (virtio_gpu_virgl_unmap_resource_blob(g, res, cmd_suspended) < 0) {
> + return -1;
> + }
> + if (*cmd_suspended) {
> + return 0;
> + }
> +#endif
> +
> + virgl_renderer_resource_detach_iov(res->base.resource_id,
> + &res_iovs,
> + &num_iovs);
> + if (res_iovs != NULL && num_iovs != 0) {
> + virtio_gpu_cleanup_mapping_iov(g, res_iovs, num_iovs);
> + }
> + virgl_renderer_resource_unref(res->base.resource_id);
> +
> + QTAILQ_REMOVE(&g->reslist, &res->base, next);
> +
> + g_free(res);
> +
> + return 0;
> +}
> +
> static void virgl_cmd_resource_unref(VirtIOGPU *g,
> struct virtio_gpu_ctrl_command *cmd,
> bool *cmd_suspended)
> {
> struct virtio_gpu_resource_unref unref;
> struct virtio_gpu_virgl_resource *res;
> - struct iovec *res_iovs = NULL;
> - int num_iovs = 0;
>
> VIRTIO_GPU_FILL_CMD(unref);
> trace_virtio_gpu_cmd_res_unref(unref.resource_id);
> @@ -331,27 +361,21 @@ static void virgl_cmd_resource_unref(VirtIOGPU *g,
> return;
> }
>
> -#if VIRGL_VERSION_MAJOR >= 1
> - if (virtio_gpu_virgl_unmap_resource_blob(g, res, cmd_suspended) < 0) {
> - cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
> - return;
> - }
> - if (*cmd_suspended) {
> - return;
> - }
> -#endif
> + virtio_gpu_virgl_resource_unref(g, res, cmd_suspended);
> +}
>
> - virgl_renderer_resource_detach_iov(unref.resource_id,
> - &res_iovs,
> - &num_iovs);
> - if (res_iovs != NULL && num_iovs != 0) {
> - virtio_gpu_cleanup_mapping_iov(g, res_iovs, num_iovs);
> - }
> - virgl_renderer_resource_unref(unref.resource_id);
> +void virtio_gpu_virgl_resource_destroy(VirtIOGPU *g,
> + struct virtio_gpu_simple_resource *base,
> + Error **errp)
> +{
> + struct virtio_gpu_virgl_resource *res;
> + bool suspended = false;
>
> - QTAILQ_REMOVE(&g->reslist, &res->base, next);
> + res = container_of(base, struct virtio_gpu_virgl_resource, base);
>
> - g_free(res);
> + if (virtio_gpu_virgl_resource_unref(g, res, &suspended) < 0) {
> + error_setg(errp, "failed to destroy virgl resource");
> + }
> }
>
> static void virgl_cmd_context_create(VirtIOGPU *g,
> @@ -1270,11 +1294,24 @@ void virtio_gpu_virgl_reset_scanout(VirtIOGPU *g)
> }
> }
>
> -void virtio_gpu_virgl_reset(VirtIOGPU *g)
> +int virtio_gpu_virgl_reset(VirtIOGPU *g)
> {
> + struct virtio_gpu_simple_resource *res, *tmp;
> +
> + QTAILQ_FOREACH_SAFE(res, &g->reslist, next, tmp) {
> + virtio_gpu_virgl_resource_destroy(g, res, NULL);
> + }
> +
> + if (!QTAILQ_EMPTY(&g->reslist)) {
> + error_report("failed to reset virgl resources");
> + return -1;
It shouldn't report an error if suspended.
> + }
> +
> virgl_renderer_reset();
>
> virtio_gpu_virgl_reset_async_fences(g);
> +
> + return 0;
> }
>
> int virtio_gpu_virgl_init(VirtIOGPU *g)
> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> index 718332284305..9e1473d1bb66 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -389,9 +389,12 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
> struct virtio_gpu_ctrl_command *cmd);
> void virtio_gpu_virgl_fence_poll(VirtIOGPU *g);
> void virtio_gpu_virgl_reset_scanout(VirtIOGPU *g);
> -void virtio_gpu_virgl_reset(VirtIOGPU *g);
> +int virtio_gpu_virgl_reset(VirtIOGPU *g);
> int virtio_gpu_virgl_init(VirtIOGPU *g);
> GArray *virtio_gpu_virgl_get_capsets(VirtIOGPU *g);
> void virtio_gpu_virgl_reset_async_fences(VirtIOGPU *g);
> +void virtio_gpu_virgl_resource_destroy(VirtIOGPU *g,
> + struct virtio_gpu_simple_resource *res,
> + Error **errp);
>
> #endif
On 11/20/25 08:07, Akihiko Odaki wrote:
>> +int virtio_gpu_virgl_reset(VirtIOGPU *g)
>> {
>> + struct virtio_gpu_simple_resource *res, *tmp;
>> +
>> + QTAILQ_FOREACH_SAFE(res, &g->reslist, next, tmp) {
>> + virtio_gpu_virgl_resource_destroy(g, res, NULL);
>> + }
>> +
>> + if (!QTAILQ_EMPTY(&g->reslist)) {
>> + error_report("failed to reset virgl resources");
>> + return -1;
>
> It shouldn't report an error if suspended.
Could you please expand on what you're meaning by "suspended"?
Suppose you're talking about guest's kernel suspension. There was a
discussion on [1] RE a need to use `x-pcie-pm-no-soft-reset=true` option
to avoid virtio-gpu resetting across S3 suspend-resume. This option
works with virtio-vga, but not with virtio-vga-gl device where VM hangs
on startup. So currently S3 doesn't work for virgl and needs further fixing.
[1]
https://lore.kernel.org/dri-devel/20250924194755.1265531-1-dongwon.kim@intel.com/
--
Best regards,
Dmitry
On 11/20/25 17:54, Dmitry Osipenko wrote:
> On 11/20/25 08:07, Akihiko Odaki wrote:
>>> +int virtio_gpu_virgl_reset(VirtIOGPU *g)
>>> {
>>> + struct virtio_gpu_simple_resource *res, *tmp;
>>> +
>>> + QTAILQ_FOREACH_SAFE(res, &g->reslist, next, tmp) {
>>> + virtio_gpu_virgl_resource_destroy(g, res, NULL);
>>> + }
>>> +
>>> + if (!QTAILQ_EMPTY(&g->reslist)) {
>>> + error_report("failed to reset virgl resources");
>>> + return -1;
>>
>> It shouldn't report an error if suspended.
>
> Could you please expand on what you're meaning by "suspended"?
>
> Suppose you're talking about guest's kernel suspension. There was a
> discussion on [1] RE a need to use `x-pcie-pm-no-soft-reset=true` option
> to avoid virtio-gpu resetting across S3 suspend-resume. This option
> works with virtio-vga, but not with virtio-vga-gl device where VM hangs
> on startup. So currently S3 doesn't work for virgl and needs further fixing.
>
> [1]
> https://lore.kernel.org/dri-devel/20250924194755.1265531-1-dongwon.kim@intel.com/
Correction: `x-pcie-pm-no-soft-reset=true` works with virgl without
enabled hostmem. It's hostmem that doesn't work with the additional PCI bus.
--
Best regards,
Dmitry
On 11/20/25 18:02, Dmitry Osipenko wrote:
> On 11/20/25 17:54, Dmitry Osipenko wrote:
>> On 11/20/25 08:07, Akihiko Odaki wrote:
>>>> +int virtio_gpu_virgl_reset(VirtIOGPU *g)
>>>> {
>>>> + struct virtio_gpu_simple_resource *res, *tmp;
>>>> +
>>>> + QTAILQ_FOREACH_SAFE(res, &g->reslist, next, tmp) {
>>>> + virtio_gpu_virgl_resource_destroy(g, res, NULL);
>>>> + }
>>>> +
>>>> + if (!QTAILQ_EMPTY(&g->reslist)) {
>>>> + error_report("failed to reset virgl resources");
>>>> + return -1;
>>>
>>> It shouldn't report an error if suspended.
>>
>> Could you please expand on what you're meaning by "suspended"?
>>
>> Suppose you're talking about guest's kernel suspension. There was a
>> discussion on [1] RE a need to use `x-pcie-pm-no-soft-reset=true` option
>> to avoid virtio-gpu resetting across S3 suspend-resume. This option
>> works with virtio-vga, but not with virtio-vga-gl device where VM hangs
>> on startup. So currently S3 doesn't work for virgl and needs further fixing.
>>
>> [1]
>> https://lore.kernel.org/dri-devel/20250924194755.1265531-1-dongwon.kim@intel.com/
>
> Correction: `x-pcie-pm-no-soft-reset=true` works with virgl without
> enabled hostmem. It's hostmem that doesn't work with the additional PCI bus.
I'll add a clarifying comment to the code in v4 telling that virgl shall
not be reset at runtime.
--
Best regards,
Dmitry
On 2025/11/25 6:54, Dmitry Osipenko wrote:
> On 11/20/25 18:02, Dmitry Osipenko wrote:
>> On 11/20/25 17:54, Dmitry Osipenko wrote:
>>> On 11/20/25 08:07, Akihiko Odaki wrote:
>>>>> +int virtio_gpu_virgl_reset(VirtIOGPU *g)
>>>>> {
>>>>> + struct virtio_gpu_simple_resource *res, *tmp;
>>>>> +
>>>>> + QTAILQ_FOREACH_SAFE(res, &g->reslist, next, tmp) {
>>>>> + virtio_gpu_virgl_resource_destroy(g, res, NULL);
>>>>> + }
>>>>> +
>>>>> + if (!QTAILQ_EMPTY(&g->reslist)) {
>>>>> + error_report("failed to reset virgl resources");
>>>>> + return -1;
>>>>
>>>> It shouldn't report an error if suspended.
>>>
>>> Could you please expand on what you're meaning by "suspended"?
>>>
>>> Suppose you're talking about guest's kernel suspension. There was a
>>> discussion on [1] RE a need to use `x-pcie-pm-no-soft-reset=true` option
>>> to avoid virtio-gpu resetting across S3 suspend-resume. This option
>>> works with virtio-vga, but not with virtio-vga-gl device where VM hangs
>>> on startup. So currently S3 doesn't work for virgl and needs further fixing.
>>>
>>> [1]
>>> https://lore.kernel.org/dri-devel/20250924194755.1265531-1-dongwon.kim@intel.com/
>>
>> Correction: `x-pcie-pm-no-soft-reset=true` works with virgl without
>> enabled hostmem. It's hostmem that doesn't work with the additional PCI bus.
No, I meant that virtio_gpu_virgl_resource_unref() may set
*cmd_suspended true. If that happens, QTAILQ_EMPTY(&g->reslist) will be
false, but it is fine so no error log should be emitted.
>
> I'll add a clarifying comment to the code in v4 telling that virgl shall
> not be reset at runtime.
It is confusing what "runtime" refers to. This code doesn't care about
S3 so such a comment shouldn't be necessary.
Regards,
Akihiko Odaki
© 2016 - 2026 Red Hat, Inc.