* [PATCH v9 1/3] drm/virtio: Freeze and restore hooks to support suspend and resume
2026-05-26 19:28 [PATCH v9 0/3] Virtio-GPU S4 support dongwon.kim
@ 2026-05-26 19:28 ` dongwon.kim
2026-05-27 4:17 ` Claude review: " Claude Code Review Bot
2026-05-26 19:28 ` [PATCH v9 2/3] drm/virtio: Add support for saving and restoring virtio_gpu_objects dongwon.kim
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: dongwon.kim @ 2026-05-26 19:28 UTC (permalink / raw)
To: dri-devel, airlied, kraxel, dmitry.osipenko
From: Dongwon Kim <dongwon.kim@intel.com>
virtio device needs to delete before VM suspend happens
then reinitialize all virtqueues again upon resume
Suggested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
Cc: Nirmoy Das <nirmoyd@nvidia.com>
Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
---
v2: 10ms sleep was added in virtgpu_freeze to avoid the situation
the driver is locked up during resumption.
v3: Plain 10ms delay was replaced with wait calls which wait until
the virtio queue is empty.
(Dmitry Osipenko)
v4: Change wait_event to wait_event_timeout to prevent permanent wait
(Nirmoy Das)
v5: Move duplicate wait_event_timeout blocks into a single
virtio_gpu_wait_queue helper function.
(Dmitry Osipenko)
Handle timeouts gracefully by returning -ETIMEDOUT to abort
the freeze operation if the host hangs.
(Dmitry Osipenko)
---
drivers/gpu/drm/virtio/virtgpu_drv.c | 68 +++++++++++++++++++++++++++-
drivers/gpu/drm/virtio/virtgpu_drv.h | 2 +
drivers/gpu/drm/virtio/virtgpu_kms.c | 23 +++++++---
drivers/gpu/drm/virtio/virtgpu_vq.c | 17 +++++++
4 files changed, 103 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c b/drivers/gpu/drm/virtio/virtgpu_drv.c
index 812ee3f5e4aa..fd3fbc7474dd 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.c
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
@@ -165,6 +165,68 @@ static unsigned int features[] = {
VIRTIO_GPU_F_CONTEXT_INIT,
VIRTIO_GPU_F_BLOB_ALIGNMENT,
};
+
+#ifdef CONFIG_PM_SLEEP
+static int virtgpu_freeze(struct virtio_device *vdev)
+{
+ struct drm_device *dev = vdev->priv;
+ struct virtio_gpu_device *vgdev = dev->dev_private;
+ int error;
+
+ error = drm_mode_config_helper_suspend(dev);
+ if (error) {
+ DRM_ERROR("suspend error %d\n", error);
+ return error;
+ }
+
+ flush_work(&vgdev->obj_free_work);
+ flush_work(&vgdev->ctrlq.dequeue_work);
+ flush_work(&vgdev->cursorq.dequeue_work);
+ flush_work(&vgdev->config_changed_work);
+
+ error = virtio_gpu_wait_queue(&vgdev->ctrlq, vgdev->ctrlq.vq->num_max);
+
+ if (error) {
+ DRM_ERROR("suspend error %d\n", error);
+ return error;
+ }
+
+ error = virtio_gpu_wait_queue(&vgdev->cursorq, vgdev->cursorq.vq->num_max);
+
+ if (error) {
+ DRM_ERROR("suspend error %d\n", error);
+ return error;
+ }
+
+ vdev->config->del_vqs(vdev);
+
+ return 0;
+}
+
+static int virtgpu_restore(struct virtio_device *vdev)
+{
+ struct drm_device *dev = vdev->priv;
+ struct virtio_gpu_device *vgdev = dev->dev_private;
+ int error;
+
+ error = virtio_gpu_find_vqs(vgdev);
+ if (error) {
+ DRM_ERROR("failed to find virt queues\n");
+ return error;
+ }
+
+ virtio_device_ready(vdev);
+
+ error = drm_mode_config_helper_resume(dev);
+ if (error) {
+ DRM_ERROR("resume error %d\n", error);
+ return error;
+ }
+
+ return 0;
+}
+#endif
+
static struct virtio_driver virtio_gpu_driver = {
.feature_table = features,
.feature_table_size = ARRAY_SIZE(features),
@@ -173,7 +235,11 @@ static struct virtio_driver virtio_gpu_driver = {
.probe = virtio_gpu_probe,
.remove = virtio_gpu_remove,
.shutdown = virtio_gpu_shutdown,
- .config_changed = virtio_gpu_config_changed
+ .config_changed = virtio_gpu_config_changed,
+#ifdef CONFIG_PM_SLEEP
+ .freeze = virtgpu_freeze,
+ .restore = virtgpu_restore,
+#endif
};
static int __init virtio_gpu_driver_init(void)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 7449907754a4..c630c75e308b 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -303,6 +303,7 @@ void virtio_gpu_deinit(struct drm_device *dev);
void virtio_gpu_release(struct drm_device *dev);
int virtio_gpu_driver_open(struct drm_device *dev, struct drm_file *file);
void virtio_gpu_driver_postclose(struct drm_device *dev, struct drm_file *file);
+int virtio_gpu_find_vqs(struct virtio_gpu_device *vgdev);
/* virtgpu_gem.c */
int virtio_gpu_gem_object_open(struct drm_gem_object *obj,
@@ -424,6 +425,7 @@ void virtio_gpu_dequeue_ctrl_func(struct work_struct *work);
void virtio_gpu_dequeue_cursor_func(struct work_struct *work);
void virtio_gpu_panic_notify(struct virtio_gpu_device *vgdev);
void virtio_gpu_notify(struct virtio_gpu_device *vgdev);
+int virtio_gpu_wait_queue(struct virtio_gpu_queue *vgvq, int num_elem);
int
virtio_gpu_cmd_resource_assign_uuid(struct virtio_gpu_device *vgdev,
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index cfde9f573df6..82baf1b3717e 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -115,15 +115,28 @@ static void virtio_gpu_get_capsets(struct virtio_gpu_device *vgdev,
vgdev->num_capsets = num_capsets;
}
-int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev)
+int virtio_gpu_find_vqs(struct virtio_gpu_device *vgdev)
{
struct virtqueue_info vqs_info[] = {
{ "control", virtio_gpu_ctrl_ack },
{ "cursor", virtio_gpu_cursor_ack },
};
- struct virtio_gpu_device *vgdev;
- /* this will expand later */
struct virtqueue *vqs[2];
+ int ret;
+
+ ret = virtio_find_vqs(vgdev->vdev, 2, vqs, vqs_info, NULL);
+ if (ret)
+ return ret;
+
+ vgdev->ctrlq.vq = vqs[0];
+ vgdev->cursorq.vq = vqs[1];
+
+ return 0;
+}
+
+int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev)
+{
+ struct virtio_gpu_device *vgdev;
u32 num_scanouts, num_capsets, blob_alignment;
int ret = 0;
@@ -215,13 +228,11 @@ int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev)
vgdev->has_context_init ? '+' : '-',
vgdev->has_blob_alignment ? '+' : '-');
- ret = virtio_find_vqs(vgdev->vdev, 2, vqs, vqs_info, NULL);
+ ret = virtio_gpu_find_vqs(vgdev);
if (ret) {
DRM_ERROR("failed to find virt queues\n");
goto err_vqs;
}
- vgdev->ctrlq.vq = vqs[0];
- vgdev->cursorq.vq = vqs[1];
ret = virtio_gpu_alloc_vbufs(vgdev);
if (ret) {
DRM_ERROR("failed to alloc vbufs\n");
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 67865810a2e7..22bf11497f16 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -369,6 +369,23 @@ static int virtio_gpu_panic_queue_ctrl_sgs(struct virtio_gpu_device *vgdev,
return 0;
}
+int virtio_gpu_wait_queue(struct virtio_gpu_queue *vgvq, int num_elem)
+{
+ int ret;
+
+ /* Cap num_elem at the queue's maximum capacity */
+ num_elem = min(num_elem, (int)vgvq->vq->num_max);
+
+ /* Wait up to 5 seconds for enough free slots to become available */
+ ret = wait_event_timeout(vgvq->ack_queue,
+ vgvq->vq->num_free >= num_elem,
+ 5 * HZ);
+ if (ret == 0)
+ return -ETIMEDOUT;
+
+ return 0;
+}
+
static int virtio_gpu_queue_ctrl_sgs(struct virtio_gpu_device *vgdev,
struct virtio_gpu_vbuffer *vbuf,
struct virtio_gpu_fence *fence,
--
2.34.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Claude review: drm/virtio: Freeze and restore hooks to support suspend and resume
2026-05-26 19:28 ` [PATCH v9 1/3] drm/virtio: Freeze and restore hooks to support suspend and resume dongwon.kim
@ 2026-05-27 4:17 ` Claude Code Review Bot
0 siblings, 0 replies; 9+ messages in thread
From: Claude Code Review Bot @ 2026-05-27 4:17 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Overall:** Clean extraction of `virtio_gpu_find_vqs()` helper and a well-structured `virtio_gpu_wait_queue()`. The freeze/restore flow is correct.
**Issue 1 — Missing error recovery in freeze (medium severity):**
If `virtio_gpu_wait_queue` times out after `drm_mode_config_helper_suspend()` has already succeeded, the function returns an error but leaves the DRM subsystem in a suspended state:
```c
error = drm_mode_config_helper_suspend(dev);
if (error) {
DRM_ERROR("suspend error %d\n", error);
return error;
}
flush_work(&vgdev->obj_free_work);
// ...
error = virtio_gpu_wait_queue(&vgdev->ctrlq, vgdev->ctrlq.vq->num_max);
if (error) {
DRM_ERROR("suspend error %d\n", error);
return error; // DRM still suspended — no recovery
}
```
The error paths after the successful suspend should call `drm_mode_config_helper_resume(dev)` before returning.
**Issue 2 — Redundant min() in wait_queue call site:**
The caller passes `vgdev->ctrlq.vq->num_max` and `virtio_gpu_wait_queue` caps at `vgvq->vq->num_max`. This is harmless (the helper is designed for general use), just noting it.
**Nit — Extra blank lines in freeze between call and error check:**
```c
error = virtio_gpu_wait_queue(&vgdev->ctrlq, vgdev->ctrlq.vq->num_max);
if (error) {
```
Typical kernel style omits the blank line between a function call and its error check. Minor.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v9 2/3] drm/virtio: Add support for saving and restoring virtio_gpu_objects
2026-05-26 19:28 [PATCH v9 0/3] Virtio-GPU S4 support dongwon.kim
2026-05-26 19:28 ` [PATCH v9 1/3] drm/virtio: Freeze and restore hooks to support suspend and resume dongwon.kim
@ 2026-05-26 19:28 ` dongwon.kim
2026-05-27 4:17 ` Claude review: " Claude Code Review Bot
2026-05-26 19:28 ` [PATCH v9 3/3] drm/virtio: Add PM notifier to restore objects after hibernation dongwon.kim
2026-05-27 4:17 ` Claude review: Virtio-GPU S4 support Claude Code Review Bot
3 siblings, 1 reply; 9+ messages in thread
From: dongwon.kim @ 2026-05-26 19:28 UTC (permalink / raw)
To: dri-devel, airlied, kraxel, dmitry.osipenko
From: Dongwon Kim <dongwon.kim@intel.com>
When the host KVM/QEMU resumes from hibernation, it loses all graphics
resources previously submitted by the guest OS, as the QEMU process is
terminated during the suspend-resume cycle. This leads to invalid resource
errors when the guest OS attempts to interact with the host using those
resources after resumption.
To resolve this, the virtio-gpu driver now tracks all active virtio_gpu_objects
and provides a mechanism to restore them by re-submitting the objects to QEMU
when needed (e.g., during resume from hibernation).
Cc: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
Cc: Nirmoy Das <nirmoyd@nvidia.com>
Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
---
v2: - Attach backing is done if bo->attached was set before
v3: - Restoration is no longer triggered via .restore; instead, it is handled
by a PM notifier only during hibernation.
v4: - Remove virtio_gpu_object from the restore list before freeing the object
to prevent an use-after-free situation.
(Nirmoy Das)
- Protect restore list operations with a spinlock
(Nirmoy Das)
- Initialize ret with 0 in virtio_gpu_object_restore_all
(Nirmoy Das)
- Move restore list node into virtio_gpu_bo struct to reduce memory usage
(Dmitry Osipenko)
v5: - Include object backed by imported dmabuf
(Dmitry Osipenko)
- Not storing virgl objects in the restore_list as virgl 3D objects are not
recoverable.
(Dmitry Osipenko)
- Change the name 'list',a node in restore_list to 'restore_node'
(Nirmoy Das)
- Use mutex instead of spinlock when updating restore_list
(Nirmoy Das)
- Initialize restore_node when virtio_gpu_object is created - this is to
determine whether the object should be removed from the restore_list with
'list_empty' function when it is time to free the object as not all objects
will be added to the list.
v6: - Add a helper, virtio_gpu_add_object_to_restore_list
(Dmitry Osipenko)
v7: - Add drm_print.h
v8: - Remove BO from restore list right before unref is called
---
drivers/gpu/drm/virtio/virtgpu_drv.h | 15 ++++++
drivers/gpu/drm/virtio/virtgpu_kms.c | 3 ++
drivers/gpu/drm/virtio/virtgpu_object.c | 70 +++++++++++++++++++++++++
drivers/gpu/drm/virtio/virtgpu_prime.c | 43 +++++++++++++++
drivers/gpu/drm/virtio/virtgpu_vram.c | 3 ++
5 files changed, 134 insertions(+)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index c630c75e308b..647e0f5f4f8e 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -99,6 +99,10 @@ struct virtio_gpu_object {
int uuid_state;
uuid_t uuid;
+
+ /* for restoration of objects after hibernation */
+ struct virtio_gpu_object_params params;
+ struct list_head restore_node;
};
#define gem_to_virtio_gpu_obj(gobj) \
container_of((gobj), struct virtio_gpu_object, base.base)
@@ -267,6 +271,8 @@ struct virtio_gpu_device {
struct work_struct obj_free_work;
spinlock_t obj_free_lock;
struct list_head obj_free_list;
+ struct mutex obj_restore_lock;
+ struct list_head obj_restore_list;
struct virtio_gpu_drv_capset *capsets;
uint32_t num_capsets;
@@ -472,6 +478,7 @@ void virtio_gpu_fence_event_process(struct virtio_gpu_device *vdev,
u64 fence_id);
/* virtgpu_object.c */
+void virtio_gpu_remove_from_restore_list(struct virtio_gpu_object *bo);
void virtio_gpu_cleanup_object(struct virtio_gpu_object *bo);
struct drm_gem_object *virtio_gpu_create_object(struct drm_device *dev,
size_t size);
@@ -484,6 +491,12 @@ bool virtio_gpu_is_shmem(struct virtio_gpu_object *bo);
int virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev,
uint32_t *resid);
+
+void virtio_gpu_add_object_to_restore_list(struct virtio_gpu_device *vgdev,
+ struct virtio_gpu_object *bo);
+
+int virtio_gpu_object_restore_all(struct virtio_gpu_device *vgdev);
+
/* virtgpu_prime.c */
int virtio_gpu_resource_assign_uuid(struct virtio_gpu_device *vgdev,
struct virtio_gpu_object *bo);
@@ -498,6 +511,8 @@ int virtgpu_dma_buf_import_sgt(struct virtio_gpu_mem_entry **ents,
unsigned int *nents,
struct virtio_gpu_object *bo,
struct dma_buf_attachment *attach);
+int virtgpu_dma_buf_obj_resubmit(struct virtio_gpu_device *vgdev,
+ struct virtio_gpu_object *bo);
/* virtgpu_debugfs.c */
void virtio_gpu_debugfs_init(struct drm_minor *minor);
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index 82baf1b3717e..9601f310875d 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -171,6 +171,8 @@ int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev)
virtio_gpu_array_put_free_work);
INIT_LIST_HEAD(&vgdev->obj_free_list);
spin_lock_init(&vgdev->obj_free_lock);
+ INIT_LIST_HEAD(&vgdev->obj_restore_list);
+ mutex_init(&vgdev->obj_restore_lock);
#ifdef __LITTLE_ENDIAN
if (virtio_has_feature(vgdev->vdev, VIRTIO_GPU_F_VIRGL))
@@ -307,6 +309,7 @@ void virtio_gpu_deinit(struct drm_device *dev)
flush_work(&vgdev->config_changed_work);
virtio_reset_device(vgdev->vdev);
vgdev->vdev->config->del_vqs(vgdev->vdev);
+ mutex_destroy(&vgdev->obj_restore_lock);
}
void virtio_gpu_release(struct drm_device *dev)
diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c
index ec9efacc6919..a1ea65f7b1de 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -63,6 +63,15 @@ static void virtio_gpu_resource_id_put(struct virtio_gpu_device *vgdev, uint32_t
ida_free(&vgdev->resource_ida, id - 1);
}
+void virtio_gpu_remove_from_restore_list(struct virtio_gpu_object *bo)
+{
+ struct virtio_gpu_device *vgdev = bo->base.base.dev->dev_private;
+
+ mutex_lock(&vgdev->obj_restore_lock);
+ list_del_init(&bo->restore_node);
+ mutex_unlock(&vgdev->obj_restore_lock);
+}
+
void virtio_gpu_cleanup_object(struct virtio_gpu_object *bo)
{
struct virtio_gpu_device *vgdev = bo->base.base.dev->dev_private;
@@ -94,6 +103,7 @@ static void virtio_gpu_free_object(struct drm_gem_object *obj)
struct virtio_gpu_device *vgdev = bo->base.base.dev->dev_private;
if (bo->created) {
+ virtio_gpu_remove_from_restore_list(bo);
virtio_gpu_cmd_unref_resource(vgdev, bo);
virtio_gpu_notify(vgdev);
/* completion handler calls virtio_gpu_cleanup_object() */
@@ -220,6 +230,8 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev,
return PTR_ERR(shmem_obj);
bo = gem_to_virtio_gpu_obj(&shmem_obj->base);
+ INIT_LIST_HEAD(&bo->restore_node);
+
ret = virtio_gpu_resource_id_get(vgdev, &bo->hw_res_handle);
if (ret < 0)
goto err_free_gem;
@@ -258,6 +270,12 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev,
virtio_gpu_object_attach(vgdev, bo, ents, nents);
}
+ if (!params->virgl) {
+ /* store non-virgl object with its param to the restore list */
+ bo->params = *params;
+ virtio_gpu_add_object_to_restore_list(vgdev, bo);
+ }
+
*bo_ptr = bo;
return 0;
@@ -271,3 +289,55 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev,
drm_gem_shmem_free(shmem_obj);
return ret;
}
+
+void virtio_gpu_add_object_to_restore_list(struct virtio_gpu_device *vgdev,
+ struct virtio_gpu_object *bo)
+{
+ mutex_lock(&vgdev->obj_restore_lock);
+ list_add_tail(&bo->restore_node, &vgdev->obj_restore_list);
+ mutex_unlock(&vgdev->obj_restore_lock);
+}
+
+int virtio_gpu_object_restore_all(struct virtio_gpu_device *vgdev)
+{
+ struct virtio_gpu_object *bo, *tmp;
+ struct virtio_gpu_mem_entry *ents;
+ unsigned int nents;
+ int ret = 0;
+
+ mutex_lock(&vgdev->obj_restore_lock);
+ list_for_each_entry_safe(bo, tmp, &vgdev->obj_restore_list,
+ restore_node) {
+ if (drm_gem_is_imported(&bo->base.base)) {
+ ret = virtgpu_dma_buf_obj_resubmit(vgdev, bo);
+ if (ret)
+ break;
+
+ continue;
+ }
+
+ if (bo->params.blob || bo->attached) {
+ ret = virtio_gpu_object_shmem_init(vgdev, bo, &ents,
+ &nents);
+ if (ret)
+ break;
+ }
+
+ if (bo->params.blob) {
+ virtio_gpu_cmd_resource_create_blob(vgdev, bo,
+ &bo->params,
+ ents, nents);
+ } else {
+ virtio_gpu_cmd_create_resource(vgdev, bo, &bo->params,
+ NULL, NULL);
+ if (bo->attached) {
+ bo->attached = false;
+ virtio_gpu_object_attach(vgdev, bo, ents,
+ nents);
+ }
+ }
+ }
+ mutex_unlock(&vgdev->obj_restore_lock);
+
+ return ret;
+}
diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c b/drivers/gpu/drm/virtio/virtgpu_prime.c
index 70b3b836e1c9..27eaab9f5cfa 100644
--- a/drivers/gpu/drm/virtio/virtgpu_prime.c
+++ b/drivers/gpu/drm/virtio/virtgpu_prime.c
@@ -23,6 +23,7 @@
*/
#include <drm/drm_prime.h>
+#include <drm/drm_print.h>
#include <linux/virtio_dma_buf.h>
#include "virtgpu_drv.h"
@@ -216,6 +217,7 @@ static void virtgpu_dma_buf_free_obj(struct drm_gem_object *obj)
}
if (bo->created) {
+ virtio_gpu_remove_from_restore_list(bo);
virtio_gpu_cmd_unref_resource(vgdev, bo);
virtio_gpu_notify(vgdev);
return;
@@ -262,6 +264,12 @@ static int virtgpu_dma_buf_init_obj(struct drm_device *dev,
dma_buf_unpin(attach);
dma_resv_unlock(resv);
+ /* store the dmabuf imported object with its params to
+ * the restore list
+ */
+ bo->params = params;
+ virtio_gpu_add_object_to_restore_list(vgdev, bo);
+
return 0;
err_import:
@@ -272,6 +280,39 @@ static int virtgpu_dma_buf_init_obj(struct drm_device *dev,
return ret;
}
+int virtgpu_dma_buf_obj_resubmit(struct virtio_gpu_device *vgdev,
+ struct virtio_gpu_object *bo)
+{
+ struct virtio_gpu_mem_entry *ents;
+ struct scatterlist *sl;
+ int i;
+
+ if (!bo->sgt) {
+ DRM_ERROR("no sgt bound to virtio_gpu_object\n");
+ return -ENOMEM;
+ }
+
+ ents = kvmalloc_array(bo->sgt->nents,
+ sizeof(struct virtio_gpu_mem_entry),
+ GFP_KERNEL);
+ if (!ents) {
+ DRM_ERROR("failed to allocate ent list\n");
+ return -ENOMEM;
+ }
+
+ for_each_sgtable_dma_sg(bo->sgt, sl, i) {
+ ents[i].addr = cpu_to_le64(sg_dma_address(sl));
+ ents[i].length = cpu_to_le32(sg_dma_len(sl));
+ ents[i].padding = 0;
+ }
+
+ virtio_gpu_cmd_resource_create_blob(vgdev, bo, &bo->params,
+ ents, bo->sgt->nents);
+
+ return 0;
+}
+
+
static const struct drm_gem_object_funcs virtgpu_gem_dma_buf_funcs = {
.free = virtgpu_dma_buf_free_obj,
};
@@ -317,6 +358,8 @@ struct drm_gem_object *virtgpu_gem_prime_import(struct drm_device *dev,
if (!bo)
return ERR_PTR(-ENOMEM);
+ INIT_LIST_HEAD(&bo->restore_node);
+
obj = &bo->base.base;
obj->resv = buf->resv;
obj->funcs = &virtgpu_gem_dma_buf_funcs;
diff --git a/drivers/gpu/drm/virtio/virtgpu_vram.c b/drivers/gpu/drm/virtio/virtgpu_vram.c
index 4ae3cbc35dd3..9ad11c7f1194 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vram.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vram.c
@@ -20,6 +20,7 @@ static void virtio_gpu_vram_free(struct drm_gem_object *obj)
if (unmap)
virtio_gpu_cmd_unmap(vgdev, bo);
+ virtio_gpu_remove_from_restore_list(bo);
virtio_gpu_cmd_unref_resource(vgdev, bo);
virtio_gpu_notify(vgdev);
return;
@@ -207,6 +208,8 @@ int virtio_gpu_vram_create(struct virtio_gpu_device *vgdev,
obj = &vram->base.base.base;
obj->funcs = &virtio_gpu_vram_funcs;
+ INIT_LIST_HEAD(&vram->base.restore_node);
+
params->size = PAGE_ALIGN(params->size);
drm_gem_private_object_init(vgdev->ddev, obj, params->size);
--
2.34.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Claude review: drm/virtio: Add support for saving and restoring virtio_gpu_objects
2026-05-26 19:28 ` [PATCH v9 2/3] drm/virtio: Add support for saving and restoring virtio_gpu_objects dongwon.kim
@ 2026-05-27 4:17 ` Claude Code Review Bot
0 siblings, 0 replies; 9+ messages in thread
From: Claude Code Review Bot @ 2026-05-27 4:17 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Overall:** The restore-list tracking approach is sensible. Objects are tracked at creation and removed before free, with proper mutex protection. The restore path correctly differentiates between blob, non-blob, and imported dmabuf objects.
**Issue 1 — VRAM objects silently excluded from restoration:**
`virtio_gpu_vram_create()` initializes the restore_node but never adds the object to the restore list:
```c
INIT_LIST_HEAD(&vram->base.restore_node);
// ... no call to virtio_gpu_add_object_to_restore_list()
```
Yet `virtio_gpu_vram_free()` calls `virtio_gpu_remove_from_restore_list(bo)`. The `list_del_init` on a self-referencing node is safe, so this won't crash, but VRAM objects won't survive hibernation. If this is intentional (VRAM objects are typically host-visible blobs that may require special handling), it should be documented in the commit message or with a comment.
**Issue 2 — Uninitialized `ents` on non-blob/non-attached path (not a bug):**
In `virtio_gpu_object_restore_all()`:
```c
if (bo->params.blob || bo->attached) {
ret = virtio_gpu_object_shmem_init(vgdev, bo, &ents, &nents);
if (ret)
break;
}
if (bo->params.blob) {
virtio_gpu_cmd_resource_create_blob(vgdev, bo, &bo->params,
ents, nents);
} else {
virtio_gpu_cmd_create_resource(vgdev, bo, &bo->params,
NULL, NULL);
if (bo->attached) {
bo->attached = false;
virtio_gpu_object_attach(vgdev, bo, ents, nents);
}
}
```
If `!bo->params.blob && !bo->attached`, `ents`/`nents` are never initialized, but they're also never used — the else-branch calls `virtio_gpu_cmd_create_resource` with `NULL, NULL` and skips attach. This is correct but could confuse a reader. An explicit `ents = NULL; nents = 0;` initialization at declaration would help.
**Issue 3 — virtgpu_dma_buf_obj_resubmit always uses DMA addresses:**
The resubmit path unconditionally uses `sg_dma_address`/`sg_dma_len`:
```c
for_each_sgtable_dma_sg(bo->sgt, sl, i) {
ents[i].addr = cpu_to_le64(sg_dma_address(sl));
ents[i].length = cpu_to_le32(sg_dma_len(sl));
```
This is correct for imported dmabufs (which always come through `dma_buf_map_attachment` and use DMA-mapped addresses), but differs from `virtio_gpu_object_shmem_init` which has a `use_dma_api` split for non-imported objects. Just confirming this is intentional — and it is, since imported objects always go through the DMA API path.
**Nit — Extra blank line after `virtgpu_dma_buf_obj_resubmit()`:**
```c
return 0;
}
static const struct drm_gem_object_funcs virtgpu_gem_dma_buf_funcs = {
```
Double blank line should be single.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v9 3/3] drm/virtio: Add PM notifier to restore objects after hibernation
2026-05-26 19:28 [PATCH v9 0/3] Virtio-GPU S4 support dongwon.kim
2026-05-26 19:28 ` [PATCH v9 1/3] drm/virtio: Freeze and restore hooks to support suspend and resume dongwon.kim
2026-05-26 19:28 ` [PATCH v9 2/3] drm/virtio: Add support for saving and restoring virtio_gpu_objects dongwon.kim
@ 2026-05-26 19:28 ` dongwon.kim
2026-05-27 4:17 ` Claude review: " Claude Code Review Bot
2026-05-27 4:17 ` Claude review: Virtio-GPU S4 support Claude Code Review Bot
3 siblings, 1 reply; 9+ messages in thread
From: dongwon.kim @ 2026-05-26 19:28 UTC (permalink / raw)
To: dri-devel, airlied, kraxel, dmitry.osipenko
From: Dongwon Kim <dongwon.kim@intel.com>
Register a PM notifier in virtio-gpu to handle suspend/hibernate
events. On PM_POST_HIBERNATION, restore all GPU objects so that the
driver can properly recover after resume.
Suggested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
---
v2: Remove unused header - drm_atomic_helper.h
(Dmitry Osipenko)
v3: Objects for virgl usecase can't be recovered after resume so
blocking S4 when virgl is enabled
(Dmitry Osipenko)
v4: Restoring objects in the PM notifier is too late, as virtio-gpu
message communication begins in virtgpu_restore once virtqueues
are re-established. To address this, a 'hibernation' flag is set
during the PM_HIBERNATION_PREPARE phase in the notifier. This flag
is then used in virtgpu_restore to detect if the system is resuming
from S4, allowing objects to be recovered immediately after virtqueues
are reconfigured.
v5: Unreference all objects before hibernation so they can be removed
on the host side, since they will be fully restored anyway. This
prevents the situation where host-side hibernation fails (leaving
all associated resources still alive) while the virtio-gpu driver
still attempts to restore those objects.
(Dmitry Osipenko)
---
drivers/gpu/drm/virtio/virtgpu_drv.c | 12 +++++++++++
drivers/gpu/drm/virtio/virtgpu_drv.h | 8 ++++++-
drivers/gpu/drm/virtio/virtgpu_kms.c | 28 +++++++++++++++++++++++++
drivers/gpu/drm/virtio/virtgpu_object.c | 17 ++++++++++++++-
drivers/gpu/drm/virtio/virtgpu_prime.c | 2 +-
drivers/gpu/drm/virtio/virtgpu_vq.c | 13 +++++++++---
drivers/gpu/drm/virtio/virtgpu_vram.c | 2 +-
7 files changed, 75 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c b/drivers/gpu/drm/virtio/virtgpu_drv.c
index fd3fbc7474dd..707e648f9367 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.c
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
@@ -179,6 +179,9 @@ static int virtgpu_freeze(struct virtio_device *vdev)
return error;
}
+ if (vgdev->hibernation)
+ virtio_gpu_object_unref_all(vgdev);
+
flush_work(&vgdev->obj_free_work);
flush_work(&vgdev->ctrlq.dequeue_work);
flush_work(&vgdev->cursorq.dequeue_work);
@@ -217,6 +220,15 @@ static int virtgpu_restore(struct virtio_device *vdev)
virtio_device_ready(vdev);
+ if (vgdev->hibernation) {
+ vgdev->hibernation = false;
+ error = virtio_gpu_object_restore_all(vgdev);
+ if (error) {
+ DRM_ERROR("Failed to recover virtio-gpu objects\n");
+ return error;
+ }
+ }
+
error = drm_mode_config_helper_resume(dev);
if (error) {
DRM_ERROR("resume error %d\n", error);
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 647e0f5f4f8e..505533653729 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -263,6 +263,7 @@ struct virtio_gpu_device {
bool has_host_visible;
bool has_context_init;
bool has_blob_alignment;
+ bool hibernation;
struct virtio_shm_region host_visible_region;
struct drm_mm host_visible_mm;
@@ -280,6 +281,8 @@ struct virtio_gpu_device {
struct list_head cap_cache;
uint32_t blob_alignment;
+ struct notifier_block pm_nb;
+
/* protects uuid state when exporting */
spinlock_t resource_export_lock;
/* protects map state and host_visible_mm */
@@ -345,7 +348,8 @@ void virtio_gpu_cmd_create_resource(struct virtio_gpu_device *vgdev,
struct virtio_gpu_object_array *objs,
struct virtio_gpu_fence *fence);
void virtio_gpu_cmd_unref_resource(struct virtio_gpu_device *vgdev,
- struct virtio_gpu_object *bo);
+ struct virtio_gpu_object *bo,
+ int no_cb);
int virtio_gpu_panic_cmd_transfer_to_host_2d(struct virtio_gpu_device *vgdev,
uint64_t offset,
uint32_t width, uint32_t height,
@@ -497,6 +501,8 @@ void virtio_gpu_add_object_to_restore_list(struct virtio_gpu_device *vgdev,
int virtio_gpu_object_restore_all(struct virtio_gpu_device *vgdev);
+void virtio_gpu_object_unref_all(struct virtio_gpu_device *vgdev);
+
/* virtgpu_prime.c */
int virtio_gpu_resource_assign_uuid(struct virtio_gpu_device *vgdev,
struct virtio_gpu_object *bo);
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index 9601f310875d..9cc88662b757 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -26,6 +26,8 @@
#include <linux/virtio.h>
#include <linux/virtio_config.h>
#include <linux/virtio_ring.h>
+#include <linux/suspend.h>
+#include <linux/pm_runtime.h>
#include <drm/drm_file.h>
#include <drm/drm_managed.h>
@@ -134,6 +136,25 @@ int virtio_gpu_find_vqs(struct virtio_gpu_device *vgdev)
return 0;
}
+static int virtio_gpu_pm_notifier(struct notifier_block *nb, unsigned long mode,
+ void *data)
+{
+ struct virtio_gpu_device *vgdev = container_of(nb,
+ struct virtio_gpu_device,
+ pm_nb);
+
+ if (mode == PM_HIBERNATION_PREPARE) {
+ if (vgdev->has_virgl_3d) {
+ DRM_ERROR("S4 not allowed when VIRGL is enabled\n");
+ return notifier_from_errno(-EPERM);
+ }
+
+ vgdev->hibernation = true;
+ }
+
+ return NOTIFY_DONE;
+}
+
int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev)
{
struct virtio_gpu_device *vgdev;
@@ -278,6 +299,12 @@ int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev)
wait_event_timeout(vgdev->resp_wq, !vgdev->display_info_pending,
5 * HZ);
}
+
+ vgdev->pm_nb.notifier_call = virtio_gpu_pm_notifier;
+ ret = register_pm_notifier(&vgdev->pm_nb);
+ if (ret)
+ goto err_scanouts;
+
return 0;
err_scanouts:
@@ -310,6 +337,7 @@ void virtio_gpu_deinit(struct drm_device *dev)
virtio_reset_device(vgdev->vdev);
vgdev->vdev->config->del_vqs(vgdev->vdev);
mutex_destroy(&vgdev->obj_restore_lock);
+ unregister_pm_notifier(&vgdev->pm_nb);
}
void virtio_gpu_release(struct drm_device *dev)
diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c
index a1ea65f7b1de..8d12633e6f4c 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -104,7 +104,7 @@ static void virtio_gpu_free_object(struct drm_gem_object *obj)
if (bo->created) {
virtio_gpu_remove_from_restore_list(bo);
- virtio_gpu_cmd_unref_resource(vgdev, bo);
+ virtio_gpu_cmd_unref_resource(vgdev, bo, false);
virtio_gpu_notify(vgdev);
/* completion handler calls virtio_gpu_cleanup_object() */
return;
@@ -341,3 +341,18 @@ int virtio_gpu_object_restore_all(struct virtio_gpu_device *vgdev)
return ret;
}
+
+void virtio_gpu_object_unref_all(struct virtio_gpu_device *vgdev)
+{
+ struct virtio_gpu_object *bo, *tmp;
+
+ mutex_lock(&vgdev->obj_restore_lock);
+ list_for_each_entry_safe(bo, tmp, &vgdev->obj_restore_list,
+ restore_node)
+ if (bo->created) {
+ virtio_gpu_cmd_unref_resource(vgdev, bo, true);
+ virtio_gpu_notify(vgdev);
+ }
+
+ mutex_unlock(&vgdev->obj_restore_lock);
+}
diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c b/drivers/gpu/drm/virtio/virtgpu_prime.c
index 27eaab9f5cfa..56686c453763 100644
--- a/drivers/gpu/drm/virtio/virtgpu_prime.c
+++ b/drivers/gpu/drm/virtio/virtgpu_prime.c
@@ -218,7 +218,7 @@ static void virtgpu_dma_buf_free_obj(struct drm_gem_object *obj)
if (bo->created) {
virtio_gpu_remove_from_restore_list(bo);
- virtio_gpu_cmd_unref_resource(vgdev, bo);
+ virtio_gpu_cmd_unref_resource(vgdev, bo, false);
virtio_gpu_notify(vgdev);
return;
}
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 22bf11497f16..fbd03e7abe66 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -643,14 +643,21 @@ static void virtio_gpu_cmd_unref_cb(struct virtio_gpu_device *vgdev,
}
void virtio_gpu_cmd_unref_resource(struct virtio_gpu_device *vgdev,
- struct virtio_gpu_object *bo)
+ struct virtio_gpu_object *bo,
+ int no_cb)
{
struct virtio_gpu_resource_unref *cmd_p;
struct virtio_gpu_vbuffer *vbuf;
int ret;
- cmd_p = virtio_gpu_alloc_cmd_cb(vgdev, &vbuf, sizeof(*cmd_p),
- virtio_gpu_cmd_unref_cb);
+ if (no_cb) {
+ cmd_p = virtio_gpu_alloc_cmd_cb(vgdev, &vbuf, sizeof(*cmd_p),
+ NULL);
+ } else {
+ cmd_p = virtio_gpu_alloc_cmd_cb(vgdev, &vbuf, sizeof(*cmd_p),
+ virtio_gpu_cmd_unref_cb);
+ }
+
memset(cmd_p, 0, sizeof(*cmd_p));
cmd_p->hdr.type = cpu_to_le32(VIRTIO_GPU_CMD_RESOURCE_UNREF);
diff --git a/drivers/gpu/drm/virtio/virtgpu_vram.c b/drivers/gpu/drm/virtio/virtgpu_vram.c
index 9ad11c7f1194..e2e799b42c61 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vram.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vram.c
@@ -21,7 +21,7 @@ static void virtio_gpu_vram_free(struct drm_gem_object *obj)
virtio_gpu_cmd_unmap(vgdev, bo);
virtio_gpu_remove_from_restore_list(bo);
- virtio_gpu_cmd_unref_resource(vgdev, bo);
+ virtio_gpu_cmd_unref_resource(vgdev, bo, false);
virtio_gpu_notify(vgdev);
return;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Claude review: drm/virtio: Add PM notifier to restore objects after hibernation
2026-05-26 19:28 ` [PATCH v9 3/3] drm/virtio: Add PM notifier to restore objects after hibernation dongwon.kim
@ 2026-05-27 4:17 ` Claude Code Review Bot
0 siblings, 0 replies; 9+ messages in thread
From: Claude Code Review Bot @ 2026-05-27 4:17 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Overall:** The notifier approach correctly gates the restore logic behind PM_HIBERNATION_PREPARE. The VIRGL 3D check is a reasonable guard since virgl resources can't be restored.
**Issue 1 — `no_cb` parameter should be `bool`, not `int`:**
```c
void virtio_gpu_cmd_unref_resource(struct virtio_gpu_device *vgdev,
struct virtio_gpu_object *bo,
int no_cb)
```
All callers pass `true` or `false`. Using `bool` would be more idiomatic and self-documenting.
**Issue 2 — Unnecessary `#include <linux/pm_runtime.h>`:**
```c
#include <linux/suspend.h>
#include <linux/pm_runtime.h>
```
Nothing from `pm_runtime.h` is used. Only `<linux/suspend.h>` is needed for `register_pm_notifier()` and `PM_HIBERNATION_PREPARE`.
**Issue 3 — `virtio_gpu_notify` called per-object inside the loop:**
In `virtio_gpu_object_unref_all()`:
```c
list_for_each_entry_safe(bo, tmp, &vgdev->obj_restore_list,
restore_node)
if (bo->created) {
virtio_gpu_cmd_unref_resource(vgdev, bo, true);
virtio_gpu_notify(vgdev);
}
```
The `virtio_gpu_notify` kicks the virtqueue. It could be called once after the loop instead of per-object. Not a correctness issue since this is the hibernation path, but it's cleaner.
**Issue 4 — Missing braces around multi-statement loop body:**
```c
list_for_each_entry_safe(bo, tmp, &vgdev->obj_restore_list,
restore_node)
if (bo->created) {
virtio_gpu_cmd_unref_resource(vgdev, bo, true);
virtio_gpu_notify(vgdev);
}
```
Kernel coding style requires braces when the loop body is more than a simple statement. The `if` block should be wrapped:
```c
list_for_each_entry_safe(bo, tmp, &vgdev->obj_restore_list,
restore_node) {
if (bo->created) {
virtio_gpu_cmd_unref_resource(vgdev, bo, true);
virtio_gpu_notify(vgdev);
}
}
```
**Issue 5 — `hibernation` flag not cleared on cancelled hibernation:**
If PM_HIBERNATION_PREPARE fires but hibernation is aborted before freeze is called, the `hibernation` flag remains `true` indefinitely. It's only cleared in `virtgpu_restore()`. While this is practically harmless (since `.freeze`/`.restore` are only called during S4 and the next S4 attempt would set the flag again anyway), handling `PM_POST_HIBERNATION` to clear the flag would be cleaner:
```c
if (mode == PM_POST_HIBERNATION)
vgdev->hibernation = false;
```
**Issue 6 — Error recovery in freeze with hibernation (same as patch 1):**
Patch 3 adds `virtio_gpu_object_unref_all()` before flush_work:
```c
if (vgdev->hibernation)
virtio_gpu_object_unref_all(vgdev);
flush_work(&vgdev->obj_free_work);
```
If the subsequent `wait_queue` calls fail, the objects have already been unreffed on the host side but the error path doesn't attempt to re-create them. This could leave the driver in an inconsistent state where guest objects exist but host resources have been destroyed. Combined with the missing `drm_mode_config_helper_resume` from patch 1, a timeout during hibernation freeze would leave the driver in a broken state.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 9+ messages in thread
* Claude review: Virtio-GPU S4 support
2026-05-26 19:28 [PATCH v9 0/3] Virtio-GPU S4 support dongwon.kim
` (2 preceding siblings ...)
2026-05-26 19:28 ` [PATCH v9 3/3] drm/virtio: Add PM notifier to restore objects after hibernation dongwon.kim
@ 2026-05-27 4:17 ` Claude Code Review Bot
3 siblings, 0 replies; 9+ messages in thread
From: Claude Code Review Bot @ 2026-05-27 4:17 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: Virtio-GPU S4 support
Author: dongwon.kim@intel.com
Patches: 4
Reviewed: 2026-05-27T14:17:08.027796
---
This v9 series adds S4 (hibernation) support for the virtio-gpu driver across 3 patches: freeze/restore hooks, object tracking/restoration, and a PM notifier. The architecture is sound — PM_HIBERNATION_PREPARE sets a flag, freeze unrefs all tracked objects to clean the host side, and restore re-submits them to a freshly started QEMU. The series has been through extensive review already (v9), and the core design is solid.
There are a few issues worth raising: missing error recovery in the freeze path (leaving DRM suspended on timeout), an unnecessary include, use of `int` instead of `bool` for the `no_cb` parameter, and VRAM objects silently not being restored. None are showstoppers, but the error recovery gap in freeze is worth fixing.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 9+ messages in thread