[PATCH v2] drm/virtio: Extend blob UAPI with deferred-mapping hinting

[PATCH v2] drm/virtio: Extend blob UAPI with deferred-mapping hinting

[Dmitry Osipenko]

If userspace never maps GEM object, then BO wastes hostmem space
because VirtIO-GPU driver maps VRAM BO at the BO's creating time.

Make mappings on-demand by adding new RESOURCE_CREATE_BLOB IOCTL/UAPI
hinting flag telling that host mapping should be deferred until first
mapping is made when the flag is set by userspace.

Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Reviewed-by: Rob Clark <robdclark@gmail.com>
---

Changelog:

v2: Rebased on recent kernel. Intel i915 native ctx landed to Mesa, lets to
    proceed with merging the mapping hint as a first user of the feature.

 drivers/gpu/drm/virtio/virtgpu_drv.h   |  2 ++
 drivers/gpu/drm/virtio/virtgpu_ioctl.c |  1 +
 drivers/gpu/drm/virtio/virtgpu_vram.c  | 30 +++++++++++++++++++++-----
 include/uapi/drm/virtgpu_drm.h         |  4 ++++
 4 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index f17660a71a3e..6f49213e23f8 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -84,6 +84,7 @@ struct virtio_gpu_object_params {
 	uint32_t blob_mem;
 	uint32_t blob_flags;
 	uint64_t blob_id;
+	uint32_t blob_hints;
 };
 
 struct virtio_gpu_object {
@@ -507,6 +508,7 @@ struct sg_table *virtio_gpu_vram_map_dma_buf(struct virtio_gpu_object *bo,
 void virtio_gpu_vram_unmap_dma_buf(struct device *dev,
 				   struct sg_table *sgt,
 				   enum dma_data_direction dir);
+void virtio_gpu_vram_map_deferred(struct virtio_gpu_object_vram *vram);
 
 /* virtgpu_submit.c */
 int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index c33c057365f8..01daa72b1310 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -489,6 +489,7 @@ static int verify_blob(struct virtio_gpu_device *vgdev,
 	params->size = rc_blob->size;
 	params->blob = true;
 	params->blob_flags = rc_blob->blob_flags;
+	params->blob_hints = rc_blob->blob_hints;
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/virtio/virtgpu_vram.c b/drivers/gpu/drm/virtio/virtgpu_vram.c
index 084e80227433..4ae3cbc35dd3 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vram.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vram.c
@@ -3,6 +3,8 @@
 
 #include <linux/dma-mapping.h>
 
+static DEFINE_MUTEX(map_lock);
+
 static void virtio_gpu_vram_free(struct drm_gem_object *obj)
 {
 	struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(obj);
@@ -42,6 +44,11 @@ static int virtio_gpu_vram_mmap(struct drm_gem_object *obj,
 	if (!(bo->blob_flags & VIRTGPU_BLOB_FLAG_USE_MAPPABLE))
 		return -EINVAL;
 
+	virtio_gpu_vram_map_deferred(vram);
+
+	if (vram->map_state == STATE_INITIALIZING)
+		virtio_gpu_notify(vgdev);
+
 	wait_event(vgdev->resp_wq, vram->map_state != STATE_INITIALIZING);
 	if (vram->map_state != STATE_OK)
 		return -EINVAL;
@@ -218,14 +225,27 @@ int virtio_gpu_vram_create(struct virtio_gpu_device *vgdev,
 
 	virtio_gpu_cmd_resource_create_blob(vgdev, &vram->base, params, NULL,
 					    0);
-	if (params->blob_flags & VIRTGPU_BLOB_FLAG_USE_MAPPABLE) {
-		ret = virtio_gpu_vram_map(&vram->base);
-		if (ret) {
-			virtio_gpu_vram_free(obj);
-			return ret;
+	if (!(params->blob_hints & DRM_VIRTGPU_BLOB_FLAG_HINT_DEFER_MAPPING)) {
+		if (params->blob_flags & VIRTGPU_BLOB_FLAG_USE_MAPPABLE) {
+			ret = virtio_gpu_vram_map(&vram->base);
+			if (ret) {
+				virtio_gpu_vram_free(obj);
+				return ret;
+			}
 		}
 	}
 
 	*bo_ptr = &vram->base;
 	return 0;
 }
+
+void virtio_gpu_vram_map_deferred(struct virtio_gpu_object_vram *vram)
+{
+	if (!(vram->base.blob_flags & VIRTGPU_BLOB_FLAG_USE_MAPPABLE))
+		return;
+
+	mutex_lock(&map_lock);
+	if (!drm_mm_node_allocated(&vram->vram_node))
+		virtio_gpu_vram_map(&vram->base);
+	mutex_unlock(&map_lock);
+}
diff --git a/include/uapi/drm/virtgpu_drm.h b/include/uapi/drm/virtgpu_drm.h
index 9debb320c34b..ba09a4ee3e77 100644
--- a/include/uapi/drm/virtgpu_drm.h
+++ b/include/uapi/drm/virtgpu_drm.h
@@ -200,6 +200,10 @@ struct drm_virtgpu_resource_create_blob {
 	__u32 cmd_size;
 	__u64 cmd;
 	__u64 blob_id;
+
+#define DRM_VIRTGPU_BLOB_FLAG_HINT_DEFER_MAPPING        0x0001
+	__u32 blob_hints;
+	__u32 pad2;
 };
 
 #define VIRTGPU_CONTEXT_PARAM_CAPSET_ID       0x0001
-- 
2.53.0

Claude review: drm/virtio: Extend blob UAPI with deferred-mapping hinting

[Claude Code Review Bot]

Overall Series Review

Subject: drm/virtio: Extend blob UAPI with deferred-mapping hinting
Author: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Patches: 1
Reviewed: 2026-05-05T09:46:07.245825

---

This single-patch series adds a deferred-mapping hint to the virtio-gpu blob UAPI, allowing userspace to tell the kernel not to map VRAM BOs at creation time, deferring the host mapping until the first `mmap()` call. The motivation is sound — avoiding wasted host memory for BOs that are never CPU-mapped.

However, the patch has several issues ranging from a potential **indefinite hang** (the error path in deferred mapping), to missing UAPI input validation, to unhandled dma-buf export paths. These need to be addressed before merging.

---

---
Generated by Claude Code Patch Reviewer

Claude review: drm/virtio: Extend blob UAPI with deferred-mapping hinting

[Claude Code Review Bot]

Patch Review

**UAPI: Missing validation of unknown `blob_hints` bits**

In `verify_blob()`, `blob_flags` is validated against `VIRTGPU_BLOB_FLAG_USE_MASK` to reject unknown bits (line 448), but `blob_hints` is accepted unconditionally:

```c
params->blob_hints = rc_blob->blob_hints;
```

This violates the standard kernel UAPI convention: unknown flags/bits must be rejected so that userspace can probe for new features by checking for `-EINVAL`. Without this, old kernels will silently ignore future hint bits, and userspace cannot detect whether a hint is actually supported. A `blob_hints & ~DRM_VIRTGPU_BLOB_FLAG_HINT_DEFER_MAPPING` check returning `-EINVAL` is needed.

**Bug: `virtio_gpu_vram_map_deferred()` ignores the return value of `virtio_gpu_vram_map()`, causing potential indefinite hang**

```c
void virtio_gpu_vram_map_deferred(struct virtio_gpu_object_vram *vram)
{
	if (!(vram->base.blob_flags & VIRTGPU_BLOB_FLAG_USE_MAPPABLE))
		return;

	mutex_lock(&map_lock);
	if (!drm_mm_node_allocated(&vram->vram_node))
		virtio_gpu_vram_map(&vram->base);    /* return value ignored */
	mutex_unlock(&map_lock);
}
```

`virtio_gpu_vram_map()` can fail (e.g., `drm_mm_insert_node` fails when VRAM is exhausted, or `virtio_gpu_array_alloc` returns NULL). If it fails, no map command is sent to the host, so `map_state` stays at `STATE_INITIALIZING` (0, the kzalloc default). The caller in `virtio_gpu_vram_mmap()` then does:

```c
wait_event(vgdev->resp_wq, vram->map_state != STATE_INITIALIZING);
```

This will **block forever** because no response will ever arrive to change `map_state`. The function should return `int`, propagate the error, or at minimum set `vram->map_state = STATE_ERR` on failure so the wait_event unblocks and the mmap returns `-EINVAL`.

**dma-buf export path not handling deferred mapping**

`virtio_gpu_vram_map_dma_buf()` directly accesses `vram->vram_node.start` and `vram->vram_node.size` (virtgpu_vram.c:107-108):

```c
addr = dma_map_resource(dev, vram->vram_node.start,
			vram->vram_node.size, dir,
			DMA_ATTR_SKIP_CPU_SYNC);
```

If a BO was created with `DRM_VIRTGPU_BLOB_FLAG_HINT_DEFER_MAPPING`, the vram_node will not be allocated yet, and these fields will be zero/uninitialized. The dma-buf path needs to trigger the deferred mapping (and wait for completion) before accessing the node, or reject the export.

**Global static mutex is heavy-handed**

```c
static DEFINE_MUTEX(map_lock);
```

This serializes all deferred VRAM mappings across all virtio-gpu device instances in the system. A per-device or per-object lock would be more appropriate. Additionally, there's a locking inconsistency: `virtio_gpu_vram_free()` checks `drm_mm_node_allocated()` under `vgdev->host_visible_lock` (spinlock), while `virtio_gpu_vram_map_deferred()` checks it under this unrelated global mutex. While GEM refcounting should prevent races between free and deferred-map, the inconsistent locking protocol is fragile.

**UAPI naming inconsistency**

The existing flags in `struct drm_virtgpu_resource_create_blob` use the `VIRTGPU_BLOB_` prefix (e.g., `VIRTGPU_BLOB_FLAG_USE_MAPPABLE`), but the new hint uses `DRM_VIRTGPU_BLOB_FLAG_HINT_DEFER_MAPPING`. This breaks the naming convention within the same struct. Consider using `VIRTGPU_BLOB_HINT_DEFER_MAPPING` for consistency.

**Minor: `map_state` read without barrier**

In `virtio_gpu_vram_mmap()`:

```c
virtio_gpu_vram_map_deferred(vram);

if (vram->map_state == STATE_INITIALIZING)
    virtio_gpu_notify(vgdev);
```

The read of `map_state` here is outside any lock and has no memory barrier, while the write side (in `virtio_gpu_vq.c:1377`) is under `host_visible_lock`. The subsequent `wait_event` does a proper recheck, so this doesn't cause functional breakage — the worst case is a redundant `virtio_gpu_notify` call — but it's technically a data race.

**Summary of required changes:**
1. Add validation in `verify_blob()` to reject unknown `blob_hints` bits
2. Handle the return value of `virtio_gpu_vram_map()` in the deferred path to avoid hangs
3. Handle (or reject) deferred-mapping BOs in the dma-buf map path
4. Consider per-device locking instead of a global mutex
5. Fix the UAPI naming to match the existing convention

---
Generated by Claude Code Patch Reviewer