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

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

This patch ties everything together with the PM notifier and adds the unref-before-hibernate / restore-after-resume logic.

**Issue 1 (bug): Use-after-free risk in `virtio_gpu_cmd_unref_resource` with `no_cb=true`**

```c
vbuf->resp_cb_data = bo;
ret = virtio_gpu_queue_ctrl_buffer(vgdev, vbuf);
if (ret < 0)
    virtio_gpu_cleanup_object(bo);
```

When `no_cb=true` (called from `virtio_gpu_object_unref_all`), if `virtio_gpu_queue_ctrl_buffer()` fails, `virtio_gpu_cleanup_object(bo)` is called. This frees the BO while it's still on the restore list and still referenced by userspace GEM handles. The `no_cb` path needs different error handling — it should not call `virtio_gpu_cleanup_object()` since the intent is to keep the guest-side object alive. A simple approach:

```c
if (ret < 0 && !no_cb)
    virtio_gpu_cleanup_object(bo);
```

**Issue 2 (style): `no_cb` parameter type and naming**

```c
void virtio_gpu_cmd_unref_resource(struct virtio_gpu_device *vgdev,
                   struct virtio_gpu_object *bo,
                   int no_cb);
```

`int no_cb` should be `bool`. Also, negative-sense parameter names are confusing — `true` means "don't do the callback". Consider `bool skip_cleanup` for clarity.

**Issue 3 (style): Missing braces 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 `list_for_each_entry_safe` macro body should have braces around it per kernel coding style when the body is a compound statement. Also, `_safe` is not needed here since entries are not removed from the list during iteration — plain `list_for_each_entry` would suffice.

**Issue 4 (minor): Unnecessary include**

```c
#include <linux/pm_runtime.h>
```

in `virtgpu_kms.c` — the code only uses `register_pm_notifier()` / `unregister_pm_notifier()` from `<linux/suspend.h>`. `pm_runtime.h` appears unused.

**Issue 5 (minor): Per-object `virtio_gpu_notify` in unref_all loop**

```c
list_for_each_entry_safe(bo, tmp, ...) 
    if (bo->created) {
        virtio_gpu_cmd_unref_resource(vgdev, bo, true);
        virtio_gpu_notify(vgdev);
    }
```

Calling `virtio_gpu_notify()` after every single unref is inefficient. A single notify after the loop would batch the kick to the host.

**Issue 6 (minor): Stale `hibernation` flag after aborted hibernation**

The PM notifier sets `vgdev->hibernation = true` on `PM_HIBERNATION_PREPARE` but never clears it on `PM_POST_HIBERNATION` (which fires if hibernation is aborted). The flag is only cleared in `virtgpu_restore()`. If hibernation is attempted but aborts before freeze() runs, the flag remains set. While this is likely benign in practice (freeze/restore are only called during actual S4), handling `PM_POST_HIBERNATION` to clear the flag would be more robust:

```c
if (mode == PM_POST_HIBERNATION)
    vgdev->hibernation = false;
```

**Issue 7 (minor): Partial state on restore failure**

In `virtgpu_restore()`, if `virtio_gpu_object_restore_all()` fails:
```c
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;
    }
}
```

The function returns an error but the device is already marked ready (`virtio_device_ready(vdev)`) and virtqueues are established. The error path doesn't tear down the partially initialized state, though there may not be a clean way to recover at that point.

---
Generated by Claude Code Patch Reviewer