[PATCH v1] drm/virtio: Fix driver removal with disabled KMS

[PATCH v1] drm/virtio: Fix driver removal with disabled KMS

[Dmitry Osipenko]

DRM atomic and modesetting aren't initialized if virtio-gpu driver built
with disabled KMS, leading to access of uninitialized data on driver
removal/unbinding and crashing kernel. Fix it by skipping shutting down
atomic core with unavailable KMS.

Fixes: 72122c69d717 ("drm/virtio: Add option to disable KMS support")
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
 drivers/gpu/drm/virtio/virtgpu_drv.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c b/drivers/gpu/drm/virtio/virtgpu_drv.c
index f0fb784c0f6f..2aaa7cb08085 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.c
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
@@ -138,7 +138,10 @@ static void virtio_gpu_remove(struct virtio_device *vdev)
 
 	virtio_gpu_release_vqs(dev);
 	drm_dev_unplug(dev);
-	drm_atomic_helper_shutdown(dev);
+
+	if (drm_core_check_feature(dev, DRIVER_ATOMIC))
+		drm_atomic_helper_shutdown(dev);
+
 	virtio_gpu_deinit(dev);
 	drm_dev_put(dev);
 }
-- 
2.54.0

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

[Claude Code Review Bot]

Overall Series Review

Subject: drm/virtio: Fix driver removal with disabled KMS
Author: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Patches: 1
Reviewed: 2026-06-05T06:30:56.726523

---

This is a single-patch fix for a kernel crash that occurs when the virtio-gpu driver is removed (unbound) while KMS support is disabled (`CONFIG_DRM_VIRTIO_GPU_KMS=n` or the device reports zero scanouts). The fix is correct in concept: when KMS is disabled, `dev->driver_features` has `DRIVER_MODESET | DRIVER_ATOMIC` cleared and `drmm_mode_config_init()` is never called, so calling `drm_atomic_helper_shutdown()` unconditionally accesses uninitialized mode_config state (uninitialized mutexes, zeroed list heads) and crashes.

The patch does **not apply cleanly** to current drm-next. The diff context shows `virtio_gpu_release_vqs(dev)` preceding `drm_dev_unplug(dev)` in `virtio_gpu_remove()`, but the current drm-next tree has no such call — the function is just `drm_dev_unplug` / `drm_atomic_helper_shutdown` / `virtio_gpu_deinit` / `drm_dev_put`. The author appears to have based this on a different tree version. A rebase is needed.

---
Generated by Claude Code Patch Reviewer

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

[Claude Code Review Bot]

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