Claude review: drm/virtio: Add support for saving and restoring virtio_gpu_objects

Claude review: drm/virtio: Add support for saving and restoring virtio_gpu_objects

[Claude Code Review Bot]

Patch Review

This patch adds the object tracking infrastructure: a per-device restore list protected by a mutex, and logic to re-create objects on the host after hibernation.

**Issue 1 (minor): VRAM objects initialized but never tracked**

In `virtgpu_vram.c`:
```c
INIT_LIST_HEAD(&vram->base.restore_node);
```

VRAM objects get their `restore_node` initialized but are never added to the restore list. The `virtio_gpu_remove_from_restore_list()` call in `virtio_gpu_vram_free()` would be a harmless no-op on an initialized-but-not-linked list node. If VRAM objects are intentionally excluded from restoration (like virgl objects), a brief comment explaining why would be helpful, as the restore_node removal in the free path suggests they might have been intended for tracking.

**Issue 2 (minor): Misleading error code in `virtgpu_dma_buf_obj_resubmit()`**

```c
if (!bo->sgt) {
    DRM_ERROR("no sgt bound to virtio_gpu_object\n");
    return -ENOMEM;
}
```

`-ENOMEM` doesn't describe "no sgt bound". `-EINVAL` or `-ENOENT` would be more accurate.

**Issue 3 (nit): Inconsistent allocation macro**

`virtgpu_dma_buf_obj_resubmit()` uses `kvmalloc_array()` directly:
```c
ents = kvmalloc_array(bo->sgt->nents,
              sizeof(struct virtio_gpu_mem_entry),
              GFP_KERNEL);
```

while `virtio_gpu_object_shmem_init()` uses the `kvmalloc_objs` macro for the same type. Minor inconsistency.

**Issue 4 (nit): Extra blank line**

After `virtgpu_dma_buf_obj_resubmit()` there are two blank lines before `static const struct drm_gem_object_funcs`. Should be one.

**Observation: Restore logic correctness**

The `virtio_gpu_object_restore_all()` logic correctly handles three cases:
1. Imported dma-buf objects → `virtgpu_dma_buf_obj_resubmit()`
2. Blob objects → `shmem_init` + `cmd_resource_create_blob()`
3. Regular (non-blob, non-virgl) objects → `shmem_init` + `cmd_create_resource()` + `object_attach()`

The conditional `if (bo->params.blob || bo->attached)` before `shmem_init` correctly gates the ents/nents initialization to paths that use them. However, `ents` and `nents` are uninitialized at declaration — adding `= NULL` / `= 0` would silence potential compiler warnings and improve clarity.

---

---
Generated by Claude Code Patch Reviewer

[PATCH v9 0/3] Virtio-GPU S4 support

[dongwon.kim]

From: Dongwon Kim <dongwon.kim@intel.com>

This patch series implements the .freeze and .restore hooks for the virtio-gpu
driver, alongside a PM notifier to properly handle object tracking and 
restoration for S4 (hibernation) scenarios.

The first patch adds the `virtgpu_freeze` and `virtgpu_restore` functions. 
These functions orchestrate flushing pending work, safely deleting virtio 
queues before suspension once they are empty, and recreating them during 
the restoration process.

The second patch implements a tracking and recreation mechanism for active 
`virtio_gpu_object` instances. This is necessary because the host hypervisor 
(e.g., QEMU) terminates or resets the backend during a full hibernation cycle, 
wiping out all previously established graphics resources and causing post-resume 
errors in the guest.

The third patch integrates a PM notifier block into the driver. It captures 
the PM_HIBERNATION_PREPARE phase to check compatibility (blocking S4 if Virgl 
3D is active) and sets a state flag. This flag instructs `virtgpu_restore` to 
fully re-submit and restore active GPU objects immediately after the fresh 
virtqueues are reconfigured.

These changes ensure that the virtio-gpu driver can cleanly survive guest 
hibernation without experiencing host-side resource loss.

v2: 10ms sleep is added in virtgpu_freeze to avoid the situation
    the driver is locked up during resumption.

v3: Plain 10ms delay (v2) is replaced with wait calls which wait until
    the virtio queue is empty.
    (Dmitry Osipenko)

v4: New version of patchset only covers S4 case because loss of resources in S3
    case can be avoided by skipping virtio-gpu-reset in QEMU (hw/display/virtio-gpu.c).
    To skip virtio-gpu-reset (soft-reset), virtio-gpu-pci device should be attached to
    PCIE bus AND a PCIE option, 'x-pcie-pm-no-soft-reset' should added and set to 'true'.
    (e.g. -device virtio-gpu-pci,bus=port,x-pcie-pm-no-soft-reset=true)

v5: 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)

    Move restore list node into virtio_gpu_bo struct to reduce memory usage
    (Dmitry Osipenko)

    Remove unused header - drm_atomic_helper.h
    (Dmitry Osipenko)

v6: 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
    check if the node is in the list with 'list_empty' before removing it.

    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.

v7: Add a helper, virtio_gpu_add_object_to_restore_list
    (Dmitry Osipenko)

    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)

v8: Add drm_print.h to virtgpu_prime.c

v9: 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)

Dongwon Kim (3):
  drm/virtio: Freeze and restore hooks to support suspend and resume
  drm/virtio: Add support for saving and restoring virtio_gpu_objects
  drm/virtio: Add PM notifier to restore objects after hibernation

 drivers/gpu/drm/virtio/virtgpu_drv.c    | 80 ++++++++++++++++++++++-
 drivers/gpu/drm/virtio/virtgpu_drv.h    | 25 ++++++-
 drivers/gpu/drm/virtio/virtgpu_kms.c    | 54 +++++++++++++--
 drivers/gpu/drm/virtio/virtgpu_object.c | 87 ++++++++++++++++++++++++-
 drivers/gpu/drm/virtio/virtgpu_prime.c  | 45 ++++++++++++-
 drivers/gpu/drm/virtio/virtgpu_vq.c     | 30 ++++++++-
 drivers/gpu/drm/virtio/virtgpu_vram.c   |  5 +-
 7 files changed, 312 insertions(+), 14 deletions(-)

-- 
2.34.1

[PATCH v9 1/3] drm/virtio: Freeze and restore hooks to support suspend and resume

[dongwon.kim]

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

[PATCH v9 2/3] drm/virtio: Add support for saving and restoring virtio_gpu_objects

[dongwon.kim]

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

[PATCH v9 3/3] drm/virtio: Add PM notifier to restore objects after hibernation

[dongwon.kim]

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

Claude review: Virtio-GPU S4 support

[Claude Code Review Bot]

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

Claude review: drm/virtio: Freeze and restore hooks to support suspend and resume

[Claude Code Review Bot]

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

Claude review: drm/virtio: Add support for saving and restoring virtio_gpu_objects

[Claude Code Review Bot]

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

Claude review: drm/virtio: Add PM notifier to restore objects after hibernation

[Claude Code Review Bot]

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