[RFC PATCH v2 0/2] Support mapping virtio-gpu virgl hostmem blobs using MAP_FIXED API

[RFC PATCH v2 0/2] Support mapping virtio-gpu virgl hostmem blobs using MAP_FIXED API

[Dmitry Osipenko]

Virglrender got a new unstable API that allows mapping host blobs at a given
memory address using MAP_FIXED mmap flag [1]. Usage of this new API brings
major performance and stability improvement for venus and drm native contexts,
see commit message of the RFC patch for details.

Sending early to collect review feeback and have patch prepared by the
time new version of libvirglrenderer will be released with the stabilized
API.

[1] https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/1374

Based-on: 20251116125641.2255794-1-dmitry.osipenko@collabora.com

Changelog:

v2: - Addressed v1 review comments from Akihiko Odaki

    - Added patch that removes unnecessary memory_region_set_enabled(),
      suggested by Akihiko Odaki

Dmitry Osipenko (2):
  virtio-gpu: Remove superfluous memory_region_set_enabled()
  virtio-gpu: Support mapping hostmem blobs with map_fixed

 hw/display/virtio-gpu-gl.c     | 45 +++++++++++++++++++-
 hw/display/virtio-gpu-virgl.c  | 76 ++++++++++++++++++++++++++++++++--
 include/hw/virtio/virtio-gpu.h |  3 ++
 3 files changed, 119 insertions(+), 5 deletions(-)

-- 
2.51.1

[RFC PATCH v2 1/2] virtio-gpu: Remove superfluous memory_region_set_enabled()

[Dmitry Osipenko]

There is no need to explicitly enable/disable memory region when it's
added or deleted respectively. Remove superfluous set_enabled() calls
for consistency.

Suggested-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
 hw/display/virtio-gpu-virgl.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 80f71c0b66aa..a6860f63b563 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -139,7 +139,6 @@ virtio_gpu_virgl_map_resource_blob(VirtIOGPU *g,
     mr = &vmr->mr;
     memory_region_init_ram_ptr(mr, OBJECT(mr), "blob", size, data);
     memory_region_add_subregion(&b->hostmem, offset, mr);
-    memory_region_set_enabled(mr, true);
 
     /*
      * MR could outlive the resource if MR's reference is held outside of
@@ -201,7 +200,6 @@ virtio_gpu_virgl_unmap_resource_blob(VirtIOGPU *g,
         b->renderer_blocked++;
 
         /* memory region owns self res->mr object and frees it by itself */
-        memory_region_set_enabled(mr, false);
         memory_region_del_subregion(&b->hostmem, mr);
         object_unparent(OBJECT(mr));
     }
-- 
2.51.1

[RFC PATCH v2 2/2] virtio-gpu: Support mapping hostmem blobs with map_fixed

[Dmitry Osipenko]

Support mapping virgl blobs to a fixed location of a hostmem memory
region using new virglrenderer MAP_FIXED API.

This new feature closes multiple problems for virtio-gpu on QEMU:

- Having dedicated memory region for each mapped blob works notoriously
slow due to QEMU's memory region software design built around RCU that
isn't optimized for frequent removal of the regions

- KVM isn't optimized for a frequent slot changes too

- QEMU/KVM has a limit for a total number of created memory regions,
crashing QEMU when limit is reached

This patch makes virtio-gpu-gl to pre-create a single anonymous memory
region covering whole hostmem area to which blobs will be mapped using
the MAP_FIXED API.

Not all virgl resources will support mapping at a fixed memory address. For
them, we will continue to create individual nested memory sub-regions. In
particular, vrend resources may not have MAP_FIXED capability.

Venus and DRM native contexts will largely benefit from the MAP_FIXED
feature in terms of performance and stability improvement.

Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
 hw/display/virtio-gpu-gl.c     | 45 ++++++++++++++++++++-
 hw/display/virtio-gpu-virgl.c  | 74 +++++++++++++++++++++++++++++++++-
 include/hw/virtio/virtio-gpu.h |  3 ++
 3 files changed, 119 insertions(+), 3 deletions(-)

diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index b640900fc6f1..23e0dd26c67b 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -121,7 +121,12 @@ static void virtio_gpu_gl_reset(VirtIODevice *vdev)
 static void virtio_gpu_gl_device_realize(DeviceState *qdev, Error **errp)
 {
     ERRP_GUARD();
-    VirtIOGPU *g = VIRTIO_GPU(qdev);
+    VirtIOGPUBase *b = VIRTIO_GPU_BASE(qdev);
+    VirtIOGPU *g = VIRTIO_GPU(b);
+#if !defined(CONFIG_WIN32)
+    VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
+    void *map;
+#endif
 
 #if HOST_BIG_ENDIAN
     error_setg(errp, "virgl is not supported on bigendian platforms");
@@ -152,6 +157,33 @@ static void virtio_gpu_gl_device_realize(DeviceState *qdev, Error **errp)
 #endif
 
     virtio_gpu_device_realize(qdev, errp);
+
+    /*
+     * Check whether virtio_gpu_device_realize() failed.
+     */
+    if (*errp) {
+        return;
+    }
+
+#if !defined(CONFIG_WIN32)
+    if (virtio_gpu_hostmem_enabled(b->conf)) {
+        map = mmap(NULL, b->conf.hostmem, PROT_READ | PROT_WRITE,
+                   MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+        if (map == MAP_FAILED) {
+            error_setg(errp,
+                       "virgl hostmem region could not be initialized: %s",
+                       strerror(errno));
+            return;
+        }
+
+        gl->hostmem_mmap = map;
+        gl->hostmem_background = g_new0(MemoryRegion, 1);
+        memory_region_init_ram_ptr(gl->hostmem_background, NULL,
+                                   "hostmem-background", b->conf.hostmem,
+                                   gl->hostmem_mmap);
+        memory_region_add_subregion(&b->hostmem, 0, gl->hostmem_background);
+    }
+#endif
 }
 
 static const Property virtio_gpu_gl_properties[] = {
@@ -165,6 +197,7 @@ static const Property virtio_gpu_gl_properties[] = {
 
 static void virtio_gpu_gl_device_unrealize(DeviceState *qdev)
 {
+    VirtIOGPUBase *b = VIRTIO_GPU_BASE(qdev);
     VirtIOGPU *g = VIRTIO_GPU(qdev);
     VirtIOGPUGL *gl = VIRTIO_GPU_GL(qdev);
 
@@ -187,6 +220,16 @@ static void virtio_gpu_gl_device_unrealize(DeviceState *qdev)
     gl->renderer_state = RS_START;
 
     g_array_unref(g->capset_ids);
+
+    /*
+     * It is not guaranteed that the memory region will be finalized
+     * immediately with memory_region_del_subregion(), there can be
+     * a remaining reference to gl->hostmem_mmap. VirtIO-GPU is not
+     * hotpluggable, hence no need to worry about the leaked mapping.
+     */
+    if (gl->hostmem_background) {
+        memory_region_del_subregion(&b->hostmem, gl->hostmem_background);
+    }
 }
 
 static void virtio_gpu_gl_class_init(ObjectClass *klass, const void *data)
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index a6860f63b563..5a663ad2acfa 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -41,9 +41,14 @@
      VIRGL_VERSION_MICRO >= (micro))
 #endif
 
+#if VIRGL_CHECK_VERSION(1, 2, 1) && !defined(CONFIG_WIN32)
+    #define VIRGL_HAS_MAP_FIXED
+#endif
+
 struct virtio_gpu_virgl_resource {
     struct virtio_gpu_simple_resource base;
     MemoryRegion *mr;
+    void *map_fixed;
 };
 
 static struct virtio_gpu_virgl_resource *
@@ -116,6 +121,7 @@ virtio_gpu_virgl_map_resource_blob(VirtIOGPU *g,
 {
     struct virtio_gpu_virgl_hostmem_region *vmr;
     VirtIOGPUBase *b = VIRTIO_GPU_BASE(g);
+    VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
     MemoryRegion *mr;
     uint64_t size;
     void *data;
@@ -126,6 +132,41 @@ virtio_gpu_virgl_map_resource_blob(VirtIOGPU *g,
         return -EOPNOTSUPP;
     }
 
+#ifdef VIRGL_HAS_MAP_FIXED
+    /*
+     * virgl_renderer_resource_map_fixed() allows to create multiple
+     * mappings of the same resource, while virgl_renderer_resource_map()
+     * not. Don't allow mapping same resource twice.
+     */
+    if (res->map_fixed || res->mr) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: failed to map(fixed) virgl resource: already mapped\n",
+                      __func__);
+        return -EBUSY;
+    }
+
+    ret = virgl_renderer_resource_map_fixed(res->base.resource_id,
+                                            gl->hostmem_mmap + offset);
+    switch (ret) {
+    case 0:
+        res->map_fixed = gl->hostmem_mmap + offset;
+        return 0;
+
+    case -EOPNOTSUPP:
+        /*
+         * -MAP_FIXED is unsupported by this resource.
+         * Mapping falls back to a blob subregion method in that case.
+         */
+        break;
+
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: failed to map(fixed) virgl resource: %s\n",
+                      __func__, strerror(-ret));
+        return ret;
+    }
+#endif
+
     ret = virgl_renderer_resource_map(res->base.resource_id, &data, &size);
     if (ret) {
         qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to map virgl resource: %s\n",
@@ -138,7 +179,7 @@ virtio_gpu_virgl_map_resource_blob(VirtIOGPU *g,
 
     mr = &vmr->mr;
     memory_region_init_ram_ptr(mr, OBJECT(mr), "blob", size, data);
-    memory_region_add_subregion(&b->hostmem, offset, mr);
+    memory_region_add_subregion_overlap(gl->hostmem_background, offset, mr, 1);
 
     /*
      * MR could outlive the resource if MR's reference is held outside of
@@ -162,9 +203,28 @@ virtio_gpu_virgl_unmap_resource_blob(VirtIOGPU *g,
 {
     struct virtio_gpu_virgl_hostmem_region *vmr;
     VirtIOGPUBase *b = VIRTIO_GPU_BASE(g);
+    VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
     MemoryRegion *mr = res->mr;
     int ret;
 
+#ifdef VIRGL_HAS_MAP_FIXED
+    if (res->map_fixed) {
+        res->map_fixed = mmap(res->map_fixed, res->base.blob_size,
+                              PROT_READ | PROT_WRITE,
+                              MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED,
+                              -1, 0);
+        if (res->map_fixed == MAP_FAILED) {
+            ret = -errno;
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "%s: failed to unmap(fixed) virgl resource: %s\n",
+                          __func__, strerror(-ret));
+            return ret;
+        }
+
+        res->map_fixed = NULL;
+    }
+#endif
+
     if (!mr) {
         return 0;
     }
@@ -200,7 +260,7 @@ virtio_gpu_virgl_unmap_resource_blob(VirtIOGPU *g,
         b->renderer_blocked++;
 
         /* memory region owns self res->mr object and frees it by itself */
-        memory_region_del_subregion(&b->hostmem, mr);
+        memory_region_del_subregion(gl->hostmem_background, mr);
         object_unparent(OBJECT(mr));
     }
 
@@ -1265,6 +1325,16 @@ void virtio_gpu_virgl_reset_scanout(VirtIOGPU *g)
 
 void virtio_gpu_virgl_reset(VirtIOGPU *g)
 {
+#ifdef VIRGL_HAS_MAP_FIXED
+    VirtIOGPUBase *b = VIRTIO_GPU_BASE(g);
+    VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
+
+    if (gl->hostmem_mmap &&
+        mmap(gl->hostmem_mmap, b->conf.hostmem, PROT_READ | PROT_WRITE,
+             MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0) == MAP_FAILED) {
+        error_report("failed to reset virgl hostmem: %s", strerror(errno));
+    }
+#endif
     virgl_renderer_reset();
 
     virtio_gpu_virgl_reset_async_fences(g);
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 718332284305..bfe85a4e0449 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -263,6 +263,9 @@ struct VirtIOGPUGL {
 
     QEMUBH *async_fence_bh;
     QSLIST_HEAD(, virtio_gpu_virgl_context_fence) async_fenceq;
+
+    MemoryRegion *hostmem_background;
+    void *hostmem_mmap;
 };
 
 struct VhostUserGPU {
-- 
2.51.1

Re: [RFC PATCH v2 2/2] virtio-gpu: Support mapping hostmem blobs with map_fixed

[Akihiko Odaki]

On 2025/11/16 23:14, Dmitry Osipenko wrote:
> Support mapping virgl blobs to a fixed location of a hostmem memory
> region using new virglrenderer MAP_FIXED API.
> 
> This new feature closes multiple problems for virtio-gpu on QEMU:
> 
> - Having dedicated memory region for each mapped blob works notoriously
> slow due to QEMU's memory region software design built around RCU that
> isn't optimized for frequent removal of the regions
> 
> - KVM isn't optimized for a frequent slot changes too
> 
> - QEMU/KVM has a limit for a total number of created memory regions,
> crashing QEMU when limit is reached
> 
> This patch makes virtio-gpu-gl to pre-create a single anonymous memory
> region covering whole hostmem area to which blobs will be mapped using
> the MAP_FIXED API.
> 
> Not all virgl resources will support mapping at a fixed memory address. For
> them, we will continue to create individual nested memory sub-regions. In
> particular, vrend resources may not have MAP_FIXED capability.
> 
> Venus and DRM native contexts will largely benefit from the MAP_FIXED
> feature in terms of performance and stability improvement.
> 
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> ---
>   hw/display/virtio-gpu-gl.c     | 45 ++++++++++++++++++++-
>   hw/display/virtio-gpu-virgl.c  | 74 +++++++++++++++++++++++++++++++++-
>   include/hw/virtio/virtio-gpu.h |  3 ++
>   3 files changed, 119 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
> index b640900fc6f1..23e0dd26c67b 100644
> --- a/hw/display/virtio-gpu-gl.c
> +++ b/hw/display/virtio-gpu-gl.c
> @@ -121,7 +121,12 @@ static void virtio_gpu_gl_reset(VirtIODevice *vdev)
>   static void virtio_gpu_gl_device_realize(DeviceState *qdev, Error **errp)
>   {
>       ERRP_GUARD();
> -    VirtIOGPU *g = VIRTIO_GPU(qdev);
> +    VirtIOGPUBase *b = VIRTIO_GPU_BASE(qdev);
> +    VirtIOGPU *g = VIRTIO_GPU(b);
> +#if !defined(CONFIG_WIN32)
> +    VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
> +    void *map;
> +#endif
>   
>   #if HOST_BIG_ENDIAN
>       error_setg(errp, "virgl is not supported on bigendian platforms");
> @@ -152,6 +157,33 @@ static void virtio_gpu_gl_device_realize(DeviceState *qdev, Error **errp)
>   #endif
>   
>       virtio_gpu_device_realize(qdev, errp);
> +
> +    /*
> +     * Check whether virtio_gpu_device_realize() failed.
> +     */
> +    if (*errp) {
> +        return;
> +    }
> +
> +#if !defined(CONFIG_WIN32)
> +    if (virtio_gpu_hostmem_enabled(b->conf)) {

hostmem should be kept enabled for Windows. I don't have any reason to 
believe hostmem does not work on Windows.

> +        map = mmap(NULL, b->conf.hostmem, PROT_READ | PROT_WRITE,
> +                   MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);

Use qemu_ram_mmap() that installs a guard page.

> +        if (map == MAP_FAILED) {
> +            error_setg(errp,
> +                       "virgl hostmem region could not be initialized: %s",
> +                       strerror(errno));
> +            return;
> +        }
> +
> +        gl->hostmem_mmap = map;
> +        gl->hostmem_background = g_new0(MemoryRegion, 1);

Embed hostmem_background to VirtIOGPUGL and avoid having g_new0(), just 
like hostmem of VirtIOGPUBase.

> +        memory_region_init_ram_ptr(gl->hostmem_background, NULL,
> +                                   "hostmem-background", b->conf.hostmem,
> +                                   gl->hostmem_mmap);
> +        memory_region_add_subregion(&b->hostmem, 0, gl->hostmem_background);
> +    }
> +#endif
>   }
>   
>   static const Property virtio_gpu_gl_properties[] = {
> @@ -165,6 +197,7 @@ static const Property virtio_gpu_gl_properties[] = {
>   
>   static void virtio_gpu_gl_device_unrealize(DeviceState *qdev)
>   {
> +    VirtIOGPUBase *b = VIRTIO_GPU_BASE(qdev);
>       VirtIOGPU *g = VIRTIO_GPU(qdev);
>       VirtIOGPUGL *gl = VIRTIO_GPU_GL(qdev);
>   
> @@ -187,6 +220,16 @@ static void virtio_gpu_gl_device_unrealize(DeviceState *qdev)
>       gl->renderer_state = RS_START;
>   
>       g_array_unref(g->capset_ids);
> +
> +    /*
> +     * It is not guaranteed that the memory region will be finalized
> +     * immediately with memory_region_del_subregion(), there can be
> +     * a remaining reference to gl->hostmem_mmap. VirtIO-GPU is not
> +     * hotpluggable, hence no need to worry about the leaked mapping.
> +     */
> +    if (gl->hostmem_background) {
> +        memory_region_del_subregion(&b->hostmem, gl->hostmem_background);
> +    }

This memory_region_del_subregion() is unnecessary because b->hostmem and 
gl->hostmem_background belong to the same device and will be gone at the 
same time.

>   }
>   
>   static void virtio_gpu_gl_class_init(ObjectClass *klass, const void *data)
> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> index a6860f63b563..5a663ad2acfa 100644
> --- a/hw/display/virtio-gpu-virgl.c
> +++ b/hw/display/virtio-gpu-virgl.c
> @@ -41,9 +41,14 @@
>        VIRGL_VERSION_MICRO >= (micro))
>   #endif
>   
> +#if VIRGL_CHECK_VERSION(1, 2, 1) && !defined(CONFIG_WIN32)
> +    #define VIRGL_HAS_MAP_FIXED
> +#endif

Simpler:

#define VIRGL_HAS_MAP_FIXED VIRGL_CHECK_VERSION(1, 2, 1) && 
!IS_ENABLED(CONFIG_WIN32)

And you can use it like:
#if VIRGL_HAS_MAP_FIXED

> +
>   struct virtio_gpu_virgl_resource {
>       struct virtio_gpu_simple_resource base;
>       MemoryRegion *mr;
> +    void *map_fixed;
>   };
>   
>   static struct virtio_gpu_virgl_resource *
> @@ -116,6 +121,7 @@ virtio_gpu_virgl_map_resource_blob(VirtIOGPU *g,
>   {
>       struct virtio_gpu_virgl_hostmem_region *vmr;
>       VirtIOGPUBase *b = VIRTIO_GPU_BASE(g);
> +    VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
>       MemoryRegion *mr;
>       uint64_t size;
>       void *data;
> @@ -126,6 +132,41 @@ virtio_gpu_virgl_map_resource_blob(VirtIOGPU *g,
>           return -EOPNOTSUPP;
>       }
>   
> +#ifdef VIRGL_HAS_MAP_FIXED
> +    /*
> +     * virgl_renderer_resource_map_fixed() allows to create multiple
> +     * mappings of the same resource, while virgl_renderer_resource_map()
> +     * not. Don't allow mapping same resource twice.
> +     */
> +    if (res->map_fixed || res->mr) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: failed to map(fixed) virgl resource: already mapped\n",
> +                      __func__);
> +        return -EBUSY;
> +    }
> +
> +    ret = virgl_renderer_resource_map_fixed(res->base.resource_id,
> +                                            gl->hostmem_mmap + offset);

I think you need a boundary check here.

> +    switch (ret) {
> +    case 0:
> +        res->map_fixed = gl->hostmem_mmap + offset;
> +        return 0;
> +
> +    case -EOPNOTSUPP:
> +        /*
> +         * -MAP_FIXED is unsupported by this resource.
> +         * Mapping falls back to a blob subregion method in that case.
> +         */
> +        break;
> +
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: failed to map(fixed) virgl resource: %s\n",
> +                      __func__, strerror(-ret));
> +        return ret;
> +    }
> +#endif
> +
>       ret = virgl_renderer_resource_map(res->base.resource_id, &data, &size);
>       if (ret) {
>           qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to map virgl resource: %s\n",
> @@ -138,7 +179,7 @@ virtio_gpu_virgl_map_resource_blob(VirtIOGPU *g,
>   
>       mr = &vmr->mr;
>       memory_region_init_ram_ptr(mr, OBJECT(mr), "blob", size, data);
> -    memory_region_add_subregion(&b->hostmem, offset, mr);
> +    memory_region_add_subregion_overlap(gl->hostmem_background, offset, mr, 1);

This should be:
memory_region_add_subregion_overlap(&b->hostmem, offset, mr, 1);

>   
>       /*
>        * MR could outlive the resource if MR's reference is held outside of
> @@ -162,9 +203,28 @@ virtio_gpu_virgl_unmap_resource_blob(VirtIOGPU *g,
>   {
>       struct virtio_gpu_virgl_hostmem_region *vmr;
>       VirtIOGPUBase *b = VIRTIO_GPU_BASE(g);
> +    VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
>       MemoryRegion *mr = res->mr;
>       int ret;
>   
> +#ifdef VIRGL_HAS_MAP_FIXED
> +    if (res->map_fixed) {
> +        res->map_fixed = mmap(res->map_fixed, res->base.blob_size,
> +                              PROT_READ | PROT_WRITE,
> +                              MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED,
> +                              -1, 0);
> +        if (res->map_fixed == MAP_FAILED) {
> +            ret = -errno;
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                          "%s: failed to unmap(fixed) virgl resource: %s\n",
> +                          __func__, strerror(-ret));
> +            return ret;

This code path leaves the blob mapped but overwrites res->map_fixed with 
MAP_FAILED.

Regards,
Akihiko Odaki

> +        }
> +
> +        res->map_fixed = NULL;
> +    }
> +#endif
> +
>       if (!mr) {
>           return 0;
>       }
> @@ -200,7 +260,7 @@ virtio_gpu_virgl_unmap_resource_blob(VirtIOGPU *g,
>           b->renderer_blocked++;
>   
>           /* memory region owns self res->mr object and frees it by itself */
> -        memory_region_del_subregion(&b->hostmem, mr);
> +        memory_region_del_subregion(gl->hostmem_background, mr);
>           object_unparent(OBJECT(mr));
>       }
>   
> @@ -1265,6 +1325,16 @@ void virtio_gpu_virgl_reset_scanout(VirtIOGPU *g)
>   
>   void virtio_gpu_virgl_reset(VirtIOGPU *g)
>   {
> +#ifdef VIRGL_HAS_MAP_FIXED
> +    VirtIOGPUBase *b = VIRTIO_GPU_BASE(g);
> +    VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
> +
> +    if (gl->hostmem_mmap &&
> +        mmap(gl->hostmem_mmap, b->conf.hostmem, PROT_READ | PROT_WRITE,
> +             MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0) == MAP_FAILED) {
> +        error_report("failed to reset virgl hostmem: %s", strerror(errno));
> +    }
> +#endif
>       virgl_renderer_reset();
>   
>       virtio_gpu_virgl_reset_async_fences(g);
> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> index 718332284305..bfe85a4e0449 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -263,6 +263,9 @@ struct VirtIOGPUGL {
>   
>       QEMUBH *async_fence_bh;
>       QSLIST_HEAD(, virtio_gpu_virgl_context_fence) async_fenceq;
> +
> +    MemoryRegion *hostmem_background;
> +    void *hostmem_mmap;
>   };
>   
>   struct VhostUserGPU {

Re: [RFC PATCH v2 2/2] virtio-gpu: Support mapping hostmem blobs with map_fixed

[Akihiko Odaki]

On 2025/11/17 10:57, Akihiko Odaki wrote:
> On 2025/11/16 23:14, Dmitry Osipenko wrote:
>> Support mapping virgl blobs to a fixed location of a hostmem memory
>> region using new virglrenderer MAP_FIXED API.
>>
>> This new feature closes multiple problems for virtio-gpu on QEMU:
>>
>> - Having dedicated memory region for each mapped blob works notoriously
>> slow due to QEMU's memory region software design built around RCU that
>> isn't optimized for frequent removal of the regions
>>
>> - KVM isn't optimized for a frequent slot changes too
>>
>> - QEMU/KVM has a limit for a total number of created memory regions,
>> crashing QEMU when limit is reached
>>
>> This patch makes virtio-gpu-gl to pre-create a single anonymous memory
>> region covering whole hostmem area to which blobs will be mapped using
>> the MAP_FIXED API.
>>
>> Not all virgl resources will support mapping at a fixed memory 
>> address. For
>> them, we will continue to create individual nested memory sub-regions. In
>> particular, vrend resources may not have MAP_FIXED capability.
>>
>> Venus and DRM native contexts will largely benefit from the MAP_FIXED
>> feature in terms of performance and stability improvement.
>>
>> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
>> ---
>>   hw/display/virtio-gpu-gl.c     | 45 ++++++++++++++++++++-
>>   hw/display/virtio-gpu-virgl.c  | 74 +++++++++++++++++++++++++++++++++-
>>   include/hw/virtio/virtio-gpu.h |  3 ++
>>   3 files changed, 119 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
>> index b640900fc6f1..23e0dd26c67b 100644
>> --- a/hw/display/virtio-gpu-gl.c
>> +++ b/hw/display/virtio-gpu-gl.c
>> @@ -121,7 +121,12 @@ static void virtio_gpu_gl_reset(VirtIODevice *vdev)
>>   static void virtio_gpu_gl_device_realize(DeviceState *qdev, Error 
>> **errp)
>>   {
>>       ERRP_GUARD();
>> -    VirtIOGPU *g = VIRTIO_GPU(qdev);
>> +    VirtIOGPUBase *b = VIRTIO_GPU_BASE(qdev);
>> +    VirtIOGPU *g = VIRTIO_GPU(b);
>> +#if !defined(CONFIG_WIN32)
>> +    VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
>> +    void *map;
>> +#endif
>>   #if HOST_BIG_ENDIAN
>>       error_setg(errp, "virgl is not supported on bigendian platforms");
>> @@ -152,6 +157,33 @@ static void 
>> virtio_gpu_gl_device_realize(DeviceState *qdev, Error **errp)
>>   #endif
>>       virtio_gpu_device_realize(qdev, errp);
>> +
>> +    /*
>> +     * Check whether virtio_gpu_device_realize() failed.
>> +     */
>> +    if (*errp) {
>> +        return;
>> +    }
>> +
>> +#if !defined(CONFIG_WIN32)
>> +    if (virtio_gpu_hostmem_enabled(b->conf)) {
> 
> hostmem should be kept enabled for Windows. I don't have any reason to 
> believe hostmem does not work on Windows.
> 
>> +        map = mmap(NULL, b->conf.hostmem, PROT_READ | PROT_WRITE,
>> +                   MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> 
> Use qemu_ram_mmap() that installs a guard page.
> 
>> +        if (map == MAP_FAILED) {
>> +            error_setg(errp,
>> +                       "virgl hostmem region could not be 
>> initialized: %s",
>> +                       strerror(errno));
>> +            return;
>> +        }
>> +
>> +        gl->hostmem_mmap = map;
>> +        gl->hostmem_background = g_new0(MemoryRegion, 1);
> 
> Embed hostmem_background to VirtIOGPUGL and avoid having g_new0(), just 
> like hostmem of VirtIOGPUBase.
> 
>> +        memory_region_init_ram_ptr(gl->hostmem_background, NULL,
>> +                                   "hostmem-background", b- 
>> >conf.hostmem,
>> +                                   gl->hostmem_mmap);
>> +        memory_region_add_subregion(&b->hostmem, 0, gl- 
>> >hostmem_background);
>> +    }
>> +#endif
>>   }
>>   static const Property virtio_gpu_gl_properties[] = {
>> @@ -165,6 +197,7 @@ static const Property virtio_gpu_gl_properties[] = {
>>   static void virtio_gpu_gl_device_unrealize(DeviceState *qdev)
>>   {
>> +    VirtIOGPUBase *b = VIRTIO_GPU_BASE(qdev);
>>       VirtIOGPU *g = VIRTIO_GPU(qdev);
>>       VirtIOGPUGL *gl = VIRTIO_GPU_GL(qdev);
>> @@ -187,6 +220,16 @@ static void 
>> virtio_gpu_gl_device_unrealize(DeviceState *qdev)
>>       gl->renderer_state = RS_START;
>>       g_array_unref(g->capset_ids);
>> +
>> +    /*
>> +     * It is not guaranteed that the memory region will be finalized
>> +     * immediately with memory_region_del_subregion(), there can be
>> +     * a remaining reference to gl->hostmem_mmap. VirtIO-GPU is not
>> +     * hotpluggable, hence no need to worry about the leaked mapping.
>> +     */
>> +    if (gl->hostmem_background) {
>> +        memory_region_del_subregion(&b->hostmem, gl- 
>> >hostmem_background);
>> +    }
> 
> This memory_region_del_subregion() is unnecessary because b->hostmem and 
> gl->hostmem_background belong to the same device and will be gone at the 
> same time.
> 
>>   }
>>   static void virtio_gpu_gl_class_init(ObjectClass *klass, const void 
>> *data)
>> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu- 
>> virgl.c
>> index a6860f63b563..5a663ad2acfa 100644
>> --- a/hw/display/virtio-gpu-virgl.c
>> +++ b/hw/display/virtio-gpu-virgl.c
>> @@ -41,9 +41,14 @@
>>        VIRGL_VERSION_MICRO >= (micro))
>>   #endif
>> +#if VIRGL_CHECK_VERSION(1, 2, 1) && !defined(CONFIG_WIN32)
>> +    #define VIRGL_HAS_MAP_FIXED
>> +#endif
> 
> Simpler:
> 
> #define VIRGL_HAS_MAP_FIXED VIRGL_CHECK_VERSION(1, 2, 1) && ! 
> IS_ENABLED(CONFIG_WIN32)
> 
> And you can use it like:
> #if VIRGL_HAS_MAP_FIXED
> 
>> +
>>   struct virtio_gpu_virgl_resource {
>>       struct virtio_gpu_simple_resource base;
>>       MemoryRegion *mr;
>> +    void *map_fixed;
>>   };
>>   static struct virtio_gpu_virgl_resource *
>> @@ -116,6 +121,7 @@ virtio_gpu_virgl_map_resource_blob(VirtIOGPU *g,
>>   {
>>       struct virtio_gpu_virgl_hostmem_region *vmr;
>>       VirtIOGPUBase *b = VIRTIO_GPU_BASE(g);
>> +    VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
>>       MemoryRegion *mr;
>>       uint64_t size;
>>       void *data;
>> @@ -126,6 +132,41 @@ virtio_gpu_virgl_map_resource_blob(VirtIOGPU *g,
>>           return -EOPNOTSUPP;
>>       }
>> +#ifdef VIRGL_HAS_MAP_FIXED
>> +    /*
>> +     * virgl_renderer_resource_map_fixed() allows to create multiple
>> +     * mappings of the same resource, while 
>> virgl_renderer_resource_map()
>> +     * not. Don't allow mapping same resource twice.
>> +     */
>> +    if (res->map_fixed || res->mr) {
>> +        qemu_log_mask(LOG_GUEST_ERROR,
>> +                      "%s: failed to map(fixed) virgl resource: 
>> already mapped\n",
>> +                      __func__);
>> +        return -EBUSY;
>> +    }
>> +
>> +    ret = virgl_renderer_resource_map_fixed(res->base.resource_id,
>> +                                            gl->hostmem_mmap + offset);
> 
> I think you need a boundary check here.
> 
>> +    switch (ret) {
>> +    case 0:
>> +        res->map_fixed = gl->hostmem_mmap + offset;
>> +        return 0;
>> +
>> +    case -EOPNOTSUPP:
>> +        /*
>> +         * -MAP_FIXED is unsupported by this resource.
>> +         * Mapping falls back to a blob subregion method in that case.
>> +         */
>> +        break;
>> +
>> +    default:
>> +        qemu_log_mask(LOG_GUEST_ERROR,
>> +                      "%s: failed to map(fixed) virgl resource: %s\n",
>> +                      __func__, strerror(-ret));
>> +        return ret;
>> +    }
>> +#endif
>> +
>>       ret = virgl_renderer_resource_map(res->base.resource_id, &data, 
>> &size);
>>       if (ret) {
>>           qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to map virgl 
>> resource: %s\n",
>> @@ -138,7 +179,7 @@ virtio_gpu_virgl_map_resource_blob(VirtIOGPU *g,
>>       mr = &vmr->mr;
>>       memory_region_init_ram_ptr(mr, OBJECT(mr), "blob", size, data);
>> -    memory_region_add_subregion(&b->hostmem, offset, mr);
>> +    memory_region_add_subregion_overlap(gl->hostmem_background, 
>> offset, mr, 1);
> 
> This should be:
> memory_region_add_subregion_overlap(&b->hostmem, offset, mr, 1);
> 
>>       /*
>>        * MR could outlive the resource if MR's reference is held 
>> outside of
>> @@ -162,9 +203,28 @@ virtio_gpu_virgl_unmap_resource_blob(VirtIOGPU *g,
>>   {
>>       struct virtio_gpu_virgl_hostmem_region *vmr;
>>       VirtIOGPUBase *b = VIRTIO_GPU_BASE(g);
>> +    VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
>>       MemoryRegion *mr = res->mr;
>>       int ret;
>> +#ifdef VIRGL_HAS_MAP_FIXED
>> +    if (res->map_fixed) {
>> +        res->map_fixed = mmap(res->map_fixed, res->base.blob_size,
>> +                              PROT_READ | PROT_WRITE,
>> +                              MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED,
>> +                              -1, 0);
>> +        if (res->map_fixed == MAP_FAILED) {
>> +            ret = -errno;
>> +            qemu_log_mask(LOG_GUEST_ERROR,
>> +                          "%s: failed to unmap(fixed) virgl resource: 
>> %s\n",
>> +                          __func__, strerror(-ret));
>> +            return ret;
> 
> This code path leaves the blob mapped but overwrites res->map_fixed with 
> MAP_FAILED.
> 
> Regards,
> Akihiko Odaki
> 
>> +        }
>> +
>> +        res->map_fixed = NULL;
>> +    }
>> +#endif
>> +
>>       if (!mr) {
>>           return 0;
>>       }
>> @@ -200,7 +260,7 @@ virtio_gpu_virgl_unmap_resource_blob(VirtIOGPU *g,
>>           b->renderer_blocked++;
>>           /* memory region owns self res->mr object and frees it by 
>> itself */
>> -        memory_region_del_subregion(&b->hostmem, mr);
>> +        memory_region_del_subregion(gl->hostmem_background, mr);
>>           object_unparent(OBJECT(mr));
>>       }
>> @@ -1265,6 +1325,16 @@ void virtio_gpu_virgl_reset_scanout(VirtIOGPU *g)
>>   void virtio_gpu_virgl_reset(VirtIOGPU *g)
>>   {
>> +#ifdef VIRGL_HAS_MAP_FIXED
>> +    VirtIOGPUBase *b = VIRTIO_GPU_BASE(g);
>> +    VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
>> +
>> +    if (gl->hostmem_mmap &&
>> +        mmap(gl->hostmem_mmap, b->conf.hostmem, PROT_READ | PROT_WRITE,
>> +             MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0) == 
>> MAP_FAILED) {
>> +        error_report("failed to reset virgl hostmem: %s", 
>> strerror(errno));
>> +    }
>> +#endif

Please call virgl_cmd_resource_unref() instead to cover both fixed and 
conventional mappings.

vgc->resource_destroy also needs to be overriden or resources will be 
gone before reaching virtio_gpu_virgl_reset().

>>       virgl_renderer_reset();
>>       virtio_gpu_virgl_reset_async_fences(g);
>> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/ 
>> virtio-gpu.h
>> index 718332284305..bfe85a4e0449 100644
>> --- a/include/hw/virtio/virtio-gpu.h
>> +++ b/include/hw/virtio/virtio-gpu.h
>> @@ -263,6 +263,9 @@ struct VirtIOGPUGL {
>>       QEMUBH *async_fence_bh;
>>       QSLIST_HEAD(, virtio_gpu_virgl_context_fence) async_fenceq;
>> +
>> +    MemoryRegion *hostmem_background;
>> +    void *hostmem_mmap;
>>   };
>>   struct VhostUserGPU {
> 

Re: [RFC PATCH v2 1/2] virtio-gpu: Remove superfluous memory_region_set_enabled()

[Akihiko Odaki]

On 2025/11/16 23:14, Dmitry Osipenko wrote:
> There is no need to explicitly enable/disable memory region when it's
> added or deleted respectively. Remove superfluous set_enabled() calls
> for consistency.
> 
> Suggested-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>

Nice.

Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>

Re: [RFC PATCH v2 2/2] virtio-gpu: Support mapping hostmem blobs with map_fixed

[Akihiko Odaki]

Sorry for sending a reply again. I keep missing some code...

On 2025/11/17 11:53, Akihiko Odaki wrote:
> On 2025/11/17 10:57, Akihiko Odaki wrote:
>> On 2025/11/16 23:14, Dmitry Osipenko wrote:
>>> Support mapping virgl blobs to a fixed location of a hostmem memory
>>> region using new virglrenderer MAP_FIXED API.
>>>
>>> This new feature closes multiple problems for virtio-gpu on QEMU:
>>>
>>> - Having dedicated memory region for each mapped blob works notoriously
>>> slow due to QEMU's memory region software design built around RCU that
>>> isn't optimized for frequent removal of the regions
>>>
>>> - KVM isn't optimized for a frequent slot changes too
>>>
>>> - QEMU/KVM has a limit for a total number of created memory regions,
>>> crashing QEMU when limit is reached
>>>
>>> This patch makes virtio-gpu-gl to pre-create a single anonymous memory
>>> region covering whole hostmem area to which blobs will be mapped using
>>> the MAP_FIXED API.
>>>
>>> Not all virgl resources will support mapping at a fixed memory 
>>> address. For
>>> them, we will continue to create individual nested memory sub- 
>>> regions. In
>>> particular, vrend resources may not have MAP_FIXED capability.
>>>
>>> Venus and DRM native contexts will largely benefit from the MAP_FIXED
>>> feature in terms of performance and stability improvement.
>>>
>>> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
>>> ---
>>>   hw/display/virtio-gpu-gl.c     | 45 ++++++++++++++++++++-
>>>   hw/display/virtio-gpu-virgl.c  | 74 +++++++++++++++++++++++++++++++++-
>>>   include/hw/virtio/virtio-gpu.h |  3 ++
>>>   3 files changed, 119 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
>>> index b640900fc6f1..23e0dd26c67b 100644
>>> --- a/hw/display/virtio-gpu-gl.c
>>> +++ b/hw/display/virtio-gpu-gl.c
>>> @@ -121,7 +121,12 @@ static void virtio_gpu_gl_reset(VirtIODevice *vdev)
>>>   static void virtio_gpu_gl_device_realize(DeviceState *qdev, Error 
>>> **errp)
>>>   {
>>>       ERRP_GUARD();
>>> -    VirtIOGPU *g = VIRTIO_GPU(qdev);
>>> +    VirtIOGPUBase *b = VIRTIO_GPU_BASE(qdev);
>>> +    VirtIOGPU *g = VIRTIO_GPU(b);
>>> +#if !defined(CONFIG_WIN32)
>>> +    VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
>>> +    void *map;
>>> +#endif
>>>   #if HOST_BIG_ENDIAN
>>>       error_setg(errp, "virgl is not supported on bigendian platforms");
>>> @@ -152,6 +157,33 @@ static void 
>>> virtio_gpu_gl_device_realize(DeviceState *qdev, Error **errp)
>>>   #endif
>>>       virtio_gpu_device_realize(qdev, errp);
>>> +
>>> +    /*
>>> +     * Check whether virtio_gpu_device_realize() failed.
>>> +     */
>>> +    if (*errp) {
>>> +        return;
>>> +    }
>>> +
>>> +#if !defined(CONFIG_WIN32)
>>> +    if (virtio_gpu_hostmem_enabled(b->conf)) {
>>
>> hostmem should be kept enabled for Windows. I don't have any reason to 
>> believe hostmem does not work on Windows.
>>
>>> +        map = mmap(NULL, b->conf.hostmem, PROT_READ | PROT_WRITE,
>>> +                   MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>>
>> Use qemu_ram_mmap() that installs a guard page.
>>
>>> +        if (map == MAP_FAILED) {
>>> +            error_setg(errp,
>>> +                       "virgl hostmem region could not be 
>>> initialized: %s",
>>> +                       strerror(errno));
>>> +            return;
>>> +        }
>>> +
>>> +        gl->hostmem_mmap = map;
>>> +        gl->hostmem_background = g_new0(MemoryRegion, 1);
>>
>> Embed hostmem_background to VirtIOGPUGL and avoid having g_new0(), 
>> just like hostmem of VirtIOGPUBase.
>>
>>> +        memory_region_init_ram_ptr(gl->hostmem_background, NULL,
>>> +                                   "hostmem-background", b- 
>>> >conf.hostmem,
>>> +                                   gl->hostmem_mmap);
>>> +        memory_region_add_subregion(&b->hostmem, 0, gl- 
>>> >hostmem_background);
>>> +    }
>>> +#endif
>>>   }
>>>   static const Property virtio_gpu_gl_properties[] = {
>>> @@ -165,6 +197,7 @@ static const Property virtio_gpu_gl_properties[] = {
>>>   static void virtio_gpu_gl_device_unrealize(DeviceState *qdev)
>>>   {
>>> +    VirtIOGPUBase *b = VIRTIO_GPU_BASE(qdev);
>>>       VirtIOGPU *g = VIRTIO_GPU(qdev);
>>>       VirtIOGPUGL *gl = VIRTIO_GPU_GL(qdev);
>>> @@ -187,6 +220,16 @@ static void 
>>> virtio_gpu_gl_device_unrealize(DeviceState *qdev)
>>>       gl->renderer_state = RS_START;
>>>       g_array_unref(g->capset_ids);
>>> +
>>> +    /*
>>> +     * It is not guaranteed that the memory region will be finalized
>>> +     * immediately with memory_region_del_subregion(), there can be
>>> +     * a remaining reference to gl->hostmem_mmap. VirtIO-GPU is not
>>> +     * hotpluggable, hence no need to worry about the leaked mapping.
>>> +     */
>>> +    if (gl->hostmem_background) {
>>> +        memory_region_del_subregion(&b->hostmem, gl- 
>>> >hostmem_background);
>>> +    }
>>
>> This memory_region_del_subregion() is unnecessary because b->hostmem 
>> and gl->hostmem_background belong to the same device and will be gone 
>> at the same time.
>>
>>>   }
>>>   static void virtio_gpu_gl_class_init(ObjectClass *klass, const void 
>>> *data)
>>> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu- 
>>> virgl.c
>>> index a6860f63b563..5a663ad2acfa 100644
>>> --- a/hw/display/virtio-gpu-virgl.c
>>> +++ b/hw/display/virtio-gpu-virgl.c
>>> @@ -41,9 +41,14 @@
>>>        VIRGL_VERSION_MICRO >= (micro))
>>>   #endif
>>> +#if VIRGL_CHECK_VERSION(1, 2, 1) && !defined(CONFIG_WIN32)
>>> +    #define VIRGL_HAS_MAP_FIXED
>>> +#endif
>>
>> Simpler:
>>
>> #define VIRGL_HAS_MAP_FIXED VIRGL_CHECK_VERSION(1, 2, 1) && ! 
>> IS_ENABLED(CONFIG_WIN32)
>>
>> And you can use it like:
>> #if VIRGL_HAS_MAP_FIXED
>>
>>> +
>>>   struct virtio_gpu_virgl_resource {
>>>       struct virtio_gpu_simple_resource base;
>>>       MemoryRegion *mr;
>>> +    void *map_fixed;
>>>   };
>>>   static struct virtio_gpu_virgl_resource *
>>> @@ -116,6 +121,7 @@ virtio_gpu_virgl_map_resource_blob(VirtIOGPU *g,
>>>   {
>>>       struct virtio_gpu_virgl_hostmem_region *vmr;
>>>       VirtIOGPUBase *b = VIRTIO_GPU_BASE(g);
>>> +    VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
>>>       MemoryRegion *mr;
>>>       uint64_t size;
>>>       void *data;
>>> @@ -126,6 +132,41 @@ virtio_gpu_virgl_map_resource_blob(VirtIOGPU *g,
>>>           return -EOPNOTSUPP;
>>>       }
>>> +#ifdef VIRGL_HAS_MAP_FIXED
>>> +    /*
>>> +     * virgl_renderer_resource_map_fixed() allows to create multiple
>>> +     * mappings of the same resource, while 
>>> virgl_renderer_resource_map()
>>> +     * not. Don't allow mapping same resource twice.
>>> +     */
>>> +    if (res->map_fixed || res->mr) {
>>> +        qemu_log_mask(LOG_GUEST_ERROR,
>>> +                      "%s: failed to map(fixed) virgl resource: 
>>> already mapped\n",
>>> +                      __func__);
>>> +        return -EBUSY;
>>> +    }
>>> +
>>> +    ret = virgl_renderer_resource_map_fixed(res->base.resource_id,
>>> +                                            gl->hostmem_mmap + offset);
>>
>> I think you need a boundary check here.
>>
>>> +    switch (ret) {
>>> +    case 0:
>>> +        res->map_fixed = gl->hostmem_mmap + offset;
>>> +        return 0;
>>> +
>>> +    case -EOPNOTSUPP:
>>> +        /*
>>> +         * -MAP_FIXED is unsupported by this resource.

s/-MAP_FIXED/MAP_FIXED/

>>> +         * Mapping falls back to a blob subregion method in that case.
>>> +         */
>>> +        break;
>>> +
>>> +    default:
>>> +        qemu_log_mask(LOG_GUEST_ERROR,
>>> +                      "%s: failed to map(fixed) virgl resource: %s\n",
>>> +                      __func__, strerror(-ret));
>>> +        return ret;
>>> +    }
>>> +#endif
>>> +
>>>       ret = virgl_renderer_resource_map(res->base.resource_id, &data, 
>>> &size);
>>>       if (ret) {
>>>           qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to map virgl 
>>> resource: %s\n",
>>> @@ -138,7 +179,7 @@ virtio_gpu_virgl_map_resource_blob(VirtIOGPU *g,
>>>       mr = &vmr->mr;
>>>       memory_region_init_ram_ptr(mr, OBJECT(mr), "blob", size, data);
>>> -    memory_region_add_subregion(&b->hostmem, offset, mr);
>>> +    memory_region_add_subregion_overlap(gl->hostmem_background, 
>>> offset, mr, 1);
>>
>> This should be:
>> memory_region_add_subregion_overlap(&b->hostmem, offset, mr, 1);
>>
>>>       /*
>>>        * MR could outlive the resource if MR's reference is held 
>>> outside of
>>> @@ -162,9 +203,28 @@ virtio_gpu_virgl_unmap_resource_blob(VirtIOGPU *g,
>>>   {
>>>       struct virtio_gpu_virgl_hostmem_region *vmr;
>>>       VirtIOGPUBase *b = VIRTIO_GPU_BASE(g);
>>> +    VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
>>>       MemoryRegion *mr = res->mr;
>>>       int ret;
>>> +#ifdef VIRGL_HAS_MAP_FIXED
>>> +    if (res->map_fixed) {
>>> +        res->map_fixed = mmap(res->map_fixed, res->base.blob_size,
>>> +                              PROT_READ | PROT_WRITE,
>>> +                              MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED,
>>> +                              -1, 0);
>>> +        if (res->map_fixed == MAP_FAILED) {
>>> +            ret = -errno;
>>> +            qemu_log_mask(LOG_GUEST_ERROR,
>>> +                          "%s: failed to unmap(fixed) virgl 
>>> resource: %s\n",
>>> +                          __func__, strerror(-ret));

This should be reported with error_report() since this error cannot be 
triggered by the guest.

>>> +            return ret;
>>
>> This code path leaves the blob mapped but overwrites res->map_fixed 
>> with MAP_FAILED.
>>
>> Regards,
>> Akihiko Odaki
>>
>>> +        }
>>> +
>>> +        res->map_fixed = NULL;
>>> +    }
>>> +#endif
>>> +
>>>       if (!mr) {
>>>           return 0;
>>>       }
>>> @@ -200,7 +260,7 @@ virtio_gpu_virgl_unmap_resource_blob(VirtIOGPU *g,
>>>           b->renderer_blocked++;
>>>           /* memory region owns self res->mr object and frees it by 
>>> itself */
>>> -        memory_region_del_subregion(&b->hostmem, mr);
>>> +        memory_region_del_subregion(gl->hostmem_background, mr);
>>>           object_unparent(OBJECT(mr));
>>>       }
>>> @@ -1265,6 +1325,16 @@ void virtio_gpu_virgl_reset_scanout(VirtIOGPU *g)
>>>   void virtio_gpu_virgl_reset(VirtIOGPU *g)
>>>   {
>>> +#ifdef VIRGL_HAS_MAP_FIXED
>>> +    VirtIOGPUBase *b = VIRTIO_GPU_BASE(g);
>>> +    VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
>>> +
>>> +    if (gl->hostmem_mmap &&
>>> +        mmap(gl->hostmem_mmap, b->conf.hostmem, PROT_READ | PROT_WRITE,
>>> +             MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0) == 
>>> MAP_FAILED) {
>>> +        error_report("failed to reset virgl hostmem: %s", 
>>> strerror(errno));
>>> +    }
>>> +#endif
> 
> Please call virgl_cmd_resource_unref() instead to cover both fixed and 
> conventional mappings.
> 
> vgc->resource_destroy also needs to be overriden or resources will be 
> gone before reaching virtio_gpu_virgl_reset().
> 
>>>       virgl_renderer_reset();
>>>       virtio_gpu_virgl_reset_async_fences(g);
>>> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/ 
>>> virtio-gpu.h
>>> index 718332284305..bfe85a4e0449 100644
>>> --- a/include/hw/virtio/virtio-gpu.h
>>> +++ b/include/hw/virtio/virtio-gpu.h
>>> @@ -263,6 +263,9 @@ struct VirtIOGPUGL {
>>>       QEMUBH *async_fence_bh;
>>>       QSLIST_HEAD(, virtio_gpu_virgl_context_fence) async_fenceq;
>>> +
>>> +    MemoryRegion *hostmem_background;
>>> +    void *hostmem_mmap;
>>>   };
>>>   struct VhostUserGPU {
>>
> 

Re: [RFC PATCH v2 0/2] Support mapping virtio-gpu virgl hostmem blobs using MAP_FIXED API

[Yiwei Zhang]

On Sun, Nov 16, 2025 at 6:15 AM Dmitry Osipenko
<dmitry.osipenko@collabora.com> wrote:
>
> Virglrender got a new unstable API that allows mapping host blobs at a given
> memory address using MAP_FIXED mmap flag [1]. Usage of this new API brings
> major performance and stability improvement for venus and drm native contexts,
> see commit message of the RFC patch for details.
>
> Sending early to collect review feeback and have patch prepared by the
> time new version of libvirglrenderer will be released with the stabilized
> API.
>
> [1] https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/1374
>
> Based-on: 20251116125641.2255794-1-dmitry.osipenko@collabora.com
>
> Changelog:
>
> v2: - Addressed v1 review comments from Akihiko Odaki
>
>     - Added patch that removes unnecessary memory_region_set_enabled(),
>       suggested by Akihiko Odaki
>
> Dmitry Osipenko (2):
>   virtio-gpu: Remove superfluous memory_region_set_enabled()
>   virtio-gpu: Support mapping hostmem blobs with map_fixed
>
>  hw/display/virtio-gpu-gl.c     | 45 +++++++++++++++++++-
>  hw/display/virtio-gpu-virgl.c  | 76 ++++++++++++++++++++++++++++++++--
>  include/hw/virtio/virtio-gpu.h |  3 ++
>  3 files changed, 119 insertions(+), 5 deletions(-)
>
> --
> 2.51.1
>

Nice work! I'd say Venus loves it soooo much ; )

Tested-by: Yiwei Zhang <zzyiwei@gmail.com>

Re: [RFC PATCH v2 2/2] virtio-gpu: Support mapping hostmem blobs with map_fixed

[Dmitry Osipenko]

On 11/17/25 06:03, Akihiko Odaki wrote:
> Sorry for sending a reply again. I keep missing some code...

Thanks a lot for the review. All comments look reasonable, will address
in v3.

-- 
Best regards,
Dmitry

Re: [RFC PATCH v2 0/2] Support mapping virtio-gpu virgl hostmem blobs using MAP_FIXED API

[Dmitry Osipenko]

On 11/17/25 08:56, Yiwei Zhang wrote:
> On Sun, Nov 16, 2025 at 6:15 AM Dmitry Osipenko
> <dmitry.osipenko@collabora.com> wrote:
>>
>> Virglrender got a new unstable API that allows mapping host blobs at a given
>> memory address using MAP_FIXED mmap flag [1]. Usage of this new API brings
>> major performance and stability improvement for venus and drm native contexts,
>> see commit message of the RFC patch for details.
>>
>> Sending early to collect review feeback and have patch prepared by the
>> time new version of libvirglrenderer will be released with the stabilized
>> API.
>>
>> [1] https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/1374
>>
>> Based-on: 20251116125641.2255794-1-dmitry.osipenko@collabora.com
>>
>> Changelog:
>>
>> v2: - Addressed v1 review comments from Akihiko Odaki
>>
>>     - Added patch that removes unnecessary memory_region_set_enabled(),
>>       suggested by Akihiko Odaki
>>
>> Dmitry Osipenko (2):
>>   virtio-gpu: Remove superfluous memory_region_set_enabled()
>>   virtio-gpu: Support mapping hostmem blobs with map_fixed
>>
>>  hw/display/virtio-gpu-gl.c     | 45 +++++++++++++++++++-
>>  hw/display/virtio-gpu-virgl.c  | 76 ++++++++++++++++++++++++++++++++--
>>  include/hw/virtio/virtio-gpu.h |  3 ++
>>  3 files changed, 119 insertions(+), 5 deletions(-)
>>
>> --
>> 2.51.1
>>
> 
> Nice work! I'd say Venus loves it soooo much ; )
> 
> Tested-by: Yiwei Zhang <zzyiwei@gmail.com>

Thanks!

-- 
Best regards,
Dmitry