Claude review: drm/virtio: Fix driver removal with disabled KMS

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

Patch Review

**Correctness of the fix: Good.**

The guard `drm_core_check_feature(dev, DRIVER_ATOMIC)` is the right check. When KMS is disabled, `virtgpu_kms.c:241` clears `DRIVER_ATOMIC` from `dev->driver_features`:

```c
dev->driver_features &= ~(DRIVER_MODESET | DRIVER_ATOMIC);
```

And `virtio_gpu_modeset_init()` returns early without calling `drmm_mode_config_init()`:

```c
if (!vgdev->num_scanouts)
    return 0;
```

So mode_config mutexes/locks and CRTC/connector lists are never initialized. The `drm_atomic_helper_shutdown()` path calls `DRM_MODESET_LOCK_ALL_BEGIN` which touches `mode_config.mutex` and modeset locks that were never initialized — that's the crash.

**Issue 1 — Patch does not apply to drm-next.** The hunk context at `@@ -138,7 +138,10 @@` includes:

```
 	virtio_gpu_release_vqs(dev);
 	drm_dev_unplug(dev);
```

But in the current drm-next tree (`virtgpu_drv.c:122-130`), `virtio_gpu_remove` has no `virtio_gpu_release_vqs` call. The patch needs rebasing.

**Issue 2 — Commit message could mention the root cause more precisely.** The message says "access of uninitialized data" which is accurate but vague. Mentioning that `drmm_mode_config_init()` is skipped when `num_scanouts == 0`, leaving mode_config locks uninitialized, would make the crash mechanism clearer for future readers.

**Issue 3 — Consider whether `drm_atomic_helper_shutdown` itself should guard.** The `DRM_MODESET_LOCK_ALL_BEGIN` macro already checks `drm_drv_uses_atomic_modeset(dev)` for the mutex path (line 176 of `drm_modeset_lock.h`):

```c
#define DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, flags, ret)		\
	if (!drm_drv_uses_atomic_modeset(dev))				\
		mutex_lock(&dev->mode_config.mutex);			\
```

...but it still proceeds unconditionally into `drm_modeset_lock_all_ctx` with potentially uninitialized locks. This suggests the core helper itself may be fragile when called on a device that never initialized mode_config. That's a broader DRM core issue, so fixing it in the driver is reasonable for now, but it might be worth a note or a follow-up to harden `drm_atomic_helper_shutdown` itself.

**Minor style nit:** The added blank lines around the `if` block are fine and follow kernel conventions, no issue there.

**Verdict:** The logic is correct and the Fixes tag is appropriate. The patch needs a rebase onto current drm-next to resolve the context mismatch.

---
Generated by Claude Code Patch Reviewer