[PATCH] virtio-gpu-virgl: Add virtio-gpu-virgl-hostmem-region type

Akihiko Odaki posted 1 patch 1 month, 3 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260214-region-v1-1-229f00ae1f38@rsg.ci.i.u-tokyo.ac.jp
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>, Dmitry Osipenko <dmitry.osipenko@collabora.com>, "Michael S. Tsirkin" <mst@redhat.com>
hw/display/virtio-gpu-virgl.c | 54 +++++++++++++++++++++++++++++++------------
1 file changed, 39 insertions(+), 15 deletions(-)
[PATCH] virtio-gpu-virgl: Add virtio-gpu-virgl-hostmem-region type
Posted by Akihiko Odaki 1 month, 3 weeks ago
Commit e27194e087ae ("virtio-gpu-virgl: correct parent for blob memory
region") made the name member of MemoryRegion unset, causing a NULL
pointer dereference[1]:
> Thread 2 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
> (gdb) bt
> #0  0x00007ffff56565e2 in __strcmp_evex () at /lib64/libc.so.6
> #1  0x0000555555841bdb in find_fd (head=0x5555572337d0 <cpr_state>,
> name=0x0, id=0) at ../migration/cpr.c:68
> #2  cpr_delete_fd (name=name@entry=0x0, id=id@entry=0) at
> ../migration/cpr.c:77
> #3  0x000055555582290a in qemu_ram_free (block=0x7ff7e93aa7f0) at
> ../system/physmem.c:2615
> #4  0x000055555581ae02 in memory_region_finalize (obj=<optimized out>)
> at ../system/memory.c:1816
> #5  0x0000555555a70ab9 in object_deinit (obj=<optimized out>,
> type=<optimized out>) at ../qom/object.c:715
> #6  object_finalize (data=0x7ff7e936eff0) at ../qom/object.c:729
> #7  object_unref (objptr=0x7ff7e936eff0) at ../qom/object.c:1232
> #8  0x0000555555814fae in memory_region_unref (mr=<optimized out>) at
> ../system/memory.c:1848
> #9  flatview_destroy (view=0x555559ed6c40) at ../system/memory.c:301
> #10 0x0000555555bfc122 in call_rcu_thread (opaque=<optimized out>) at
> ../util/rcu.c:324
> #11 0x0000555555bf17a7 in qemu_thread_start (args=0x555557b99520) at
> ../util/qemu-thread-posix.c:393
> #12 0x00007ffff556f464 in start_thread () at /lib64/libc.so.6
> #13 0x00007ffff55f25ac in __clone3 () at /lib64/libc.so.6

The intention of the aforementioned commit is to prevent a MemoryRegion
from parenting itself while its references is counted indendependently
of the device. To achieve the same goal, add a type of QOM objects that
count references and parent MemoryRegions.

[1] https://lore.kernel.org/qemu-devel/4eb93d7a-1fa9-4b3c-8ad7-a2eb64f025a0@collabora.com/

Cc: qemu-stable@nongnu.org
Fixes: e27194e087ae ("virtio-gpu-virgl: correct parent for blob memory region")
Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 hw/display/virtio-gpu-virgl.c | 54 +++++++++++++++++++++++++++++++------------
 1 file changed, 39 insertions(+), 15 deletions(-)

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index ecf8494f3676..0f754829fb71 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -52,11 +52,17 @@ virgl_get_egl_display(G_GNUC_UNUSED void *cookie)
 
 #if VIRGL_VERSION_MAJOR >= 1
 struct virtio_gpu_virgl_hostmem_region {
+    Object parent_obj;
     MemoryRegion mr;
     struct VirtIOGPU *g;
     bool finish_unmapping;
 };
 
+#define TYPE_VIRTIO_GPU_VIRGL_HOSTMEM_REGION "virtio-gpu-virgl-hostmem-region"
+
+OBJECT_DECLARE_SIMPLE_TYPE(virtio_gpu_virgl_hostmem_region,
+                           VIRTIO_GPU_VIRGL_HOSTMEM_REGION)
+
 static struct virtio_gpu_virgl_hostmem_region *
 to_hostmem_region(MemoryRegion *mr)
 {
@@ -70,14 +76,22 @@ static void virtio_gpu_virgl_resume_cmdq_bh(void *opaque)
     virtio_gpu_process_cmdq(g);
 }
 
-static void virtio_gpu_virgl_hostmem_region_free(void *obj)
+/*
+ * MR could outlive the resource if MR's reference is held outside of
+ * virtio-gpu. In order to prevent unmapping resource while MR is alive,
+ * and thus, making the data pointer invalid, we will block virtio-gpu
+ * command processing until MR is fully unreferenced and freed.
+ */
+static void virtio_gpu_virgl_hostmem_region_finalize(Object *obj)
 {
-    MemoryRegion *mr = MEMORY_REGION(obj);
-    struct virtio_gpu_virgl_hostmem_region *vmr;
+    struct virtio_gpu_virgl_hostmem_region *vmr = VIRTIO_GPU_VIRGL_HOSTMEM_REGION(obj);
     VirtIOGPUBase *b;
     VirtIOGPUGL *gl;
 
-    vmr = to_hostmem_region(mr);
+    if (!vmr->g) {
+        return;
+    }
+
     vmr->finish_unmapping = true;
 
     b = VIRTIO_GPU_BASE(vmr->g);
@@ -92,11 +106,26 @@ static void virtio_gpu_virgl_hostmem_region_free(void *obj)
     qemu_bh_schedule(gl->cmdq_resume_bh);
 }
 
+static const TypeInfo virtio_gpu_virgl_hostmem_region_info = {
+    .parent = TYPE_OBJECT,
+    .name = TYPE_VIRTIO_GPU_VIRGL_HOSTMEM_REGION,
+    .instance_size = sizeof(struct virtio_gpu_virgl_hostmem_region),
+    .instance_finalize = virtio_gpu_virgl_hostmem_region_finalize
+};
+
+static void virtio_gpu_virgl_types(void)
+{
+    type_register_static(&virtio_gpu_virgl_hostmem_region_info);
+}
+
+type_init(virtio_gpu_virgl_types)
+
 static int
 virtio_gpu_virgl_map_resource_blob(VirtIOGPU *g,
                                    struct virtio_gpu_virgl_resource *res,
                                    uint64_t offset)
 {
+    g_autofree char *name = NULL;
     struct virtio_gpu_virgl_hostmem_region *vmr;
     VirtIOGPUBase *b = VIRTIO_GPU_BASE(g);
     MemoryRegion *mr;
@@ -117,21 +146,16 @@ virtio_gpu_virgl_map_resource_blob(VirtIOGPU *g,
     }
 
     vmr = g_new0(struct virtio_gpu_virgl_hostmem_region, 1);
+    name = g_strdup_printf("blob[%" PRIu32 "]", res->base.resource_id);
+    object_initialize_child(OBJECT(g), name, vmr,
+                            TYPE_VIRTIO_GPU_VIRGL_HOSTMEM_REGION);
     vmr->g = g;
 
     mr = &vmr->mr;
-    memory_region_init_ram_ptr(mr, OBJECT(mr), NULL, size, data);
+    memory_region_init_ram_ptr(mr, OBJECT(vmr), "mr", 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
-     * virtio-gpu. In order to prevent unmapping resource while MR is alive,
-     * and thus, making the data pointer invalid, we will block virtio-gpu
-     * command processing until MR is fully unreferenced and freed.
-     */
-    OBJECT(mr)->free = virtio_gpu_virgl_hostmem_region_free;
-
     res->mr = mr;
 
     trace_virtio_gpu_cmd_res_map_blob(res->base.resource_id, vmr, mr);
@@ -163,7 +187,7 @@ virtio_gpu_virgl_unmap_resource_blob(VirtIOGPU *g,
      * 1. Begin async unmapping with memory_region_del_subregion()
      *    and suspend/block cmd processing.
      * 2. Wait for res->mr to be freed and cmd processing resumed
-     *    asynchronously by virtio_gpu_virgl_hostmem_region_free().
+     *    asynchronously by virtio_gpu_virgl_hostmem_region_finalize().
      * 3. Finish the unmapping with final virgl_renderer_resource_unmap().
      */
     if (vmr->finish_unmapping) {
@@ -186,7 +210,7 @@ virtio_gpu_virgl_unmap_resource_blob(VirtIOGPU *g,
         /* 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_unref(OBJECT(mr));
+        object_unparent(OBJECT(vmr));
     }
 
     return 0;

---
base-commit: ece408818d27f745ef1b05fb3cc99a1e7a5bf580
change-id: 20260214-region-690a177c0475

Best regards,
--  
Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Re: [PATCH] virtio-gpu-virgl: Add virtio-gpu-virgl-hostmem-region type
Posted by Dmitry Osipenko 1 month, 3 weeks ago
Hello Akihiko,

On 2/14/26 07:33, Akihiko Odaki wrote:
> Commit e27194e087ae ("virtio-gpu-virgl: correct parent for blob memory
> region") made the name member of MemoryRegion unset, causing a NULL
> pointer dereference[1]:
>> Thread 2 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
>> (gdb) bt
>> #0  0x00007ffff56565e2 in __strcmp_evex () at /lib64/libc.so.6
>> #1  0x0000555555841bdb in find_fd (head=0x5555572337d0 <cpr_state>,
>> name=0x0, id=0) at ../migration/cpr.c:68
>> #2  cpr_delete_fd (name=name@entry=0x0, id=id@entry=0) at
>> ../migration/cpr.c:77
>> #3  0x000055555582290a in qemu_ram_free (block=0x7ff7e93aa7f0) at
>> ../system/physmem.c:2615
>> #4  0x000055555581ae02 in memory_region_finalize (obj=<optimized out>)
>> at ../system/memory.c:1816
>> #5  0x0000555555a70ab9 in object_deinit (obj=<optimized out>,
>> type=<optimized out>) at ../qom/object.c:715
>> #6  object_finalize (data=0x7ff7e936eff0) at ../qom/object.c:729
>> #7  object_unref (objptr=0x7ff7e936eff0) at ../qom/object.c:1232
>> #8  0x0000555555814fae in memory_region_unref (mr=<optimized out>) at
>> ../system/memory.c:1848
>> #9  flatview_destroy (view=0x555559ed6c40) at ../system/memory.c:301
>> #10 0x0000555555bfc122 in call_rcu_thread (opaque=<optimized out>) at
>> ../util/rcu.c:324
>> #11 0x0000555555bf17a7 in qemu_thread_start (args=0x555557b99520) at
>> ../util/qemu-thread-posix.c:393
>> #12 0x00007ffff556f464 in start_thread () at /lib64/libc.so.6
>> #13 0x00007ffff55f25ac in __clone3 () at /lib64/libc.so.6
> 
> The intention of the aforementioned commit is to prevent a MemoryRegion
> from parenting itself while its references is counted indendependently
> of the device. To achieve the same goal, add a type of QOM objects that
> count references and parent MemoryRegions.
> 
> [1] https://lore.kernel.org/qemu-devel/4eb93d7a-1fa9-4b3c-8ad7-a2eb64f025a0@collabora.com/
> 
> Cc: qemu-stable@nongnu.org
> Fixes: e27194e087ae ("virtio-gpu-virgl: correct parent for blob memory region")
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---
>  hw/display/virtio-gpu-virgl.c | 54 +++++++++++++++++++++++++++++++------------
>  1 file changed, 39 insertions(+), 15 deletions(-)

Thanks a lot for the fix:

Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>

-- 
Best regards,
Dmitry
Re: [PATCH] virtio-gpu-virgl: Add virtio-gpu-virgl-hostmem-region type
Posted by Joelle van Dyne 1 month, 3 weeks ago
On Fri, Feb 13, 2026 at 8:34 PM Akihiko Odaki
<odaki@rsg.ci.i.u-tokyo.ac.jp> wrote:
>
> Commit e27194e087ae ("virtio-gpu-virgl: correct parent for blob memory
> region") made the name member of MemoryRegion unset, causing a NULL
> pointer dereference[1]:
> > Thread 2 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
> > (gdb) bt
> > #0  0x00007ffff56565e2 in __strcmp_evex () at /lib64/libc.so.6
> > #1  0x0000555555841bdb in find_fd (head=0x5555572337d0 <cpr_state>,
> > name=0x0, id=0) at ../migration/cpr.c:68
> > #2  cpr_delete_fd (name=name@entry=0x0, id=id@entry=0) at
> > ../migration/cpr.c:77
> > #3  0x000055555582290a in qemu_ram_free (block=0x7ff7e93aa7f0) at
> > ../system/physmem.c:2615
> > #4  0x000055555581ae02 in memory_region_finalize (obj=<optimized out>)
> > at ../system/memory.c:1816
> > #5  0x0000555555a70ab9 in object_deinit (obj=<optimized out>,
> > type=<optimized out>) at ../qom/object.c:715
> > #6  object_finalize (data=0x7ff7e936eff0) at ../qom/object.c:729
> > #7  object_unref (objptr=0x7ff7e936eff0) at ../qom/object.c:1232
> > #8  0x0000555555814fae in memory_region_unref (mr=<optimized out>) at
> > ../system/memory.c:1848
> > #9  flatview_destroy (view=0x555559ed6c40) at ../system/memory.c:301
> > #10 0x0000555555bfc122 in call_rcu_thread (opaque=<optimized out>) at
> > ../util/rcu.c:324
> > #11 0x0000555555bf17a7 in qemu_thread_start (args=0x555557b99520) at
> > ../util/qemu-thread-posix.c:393
> > #12 0x00007ffff556f464 in start_thread () at /lib64/libc.so.6
> > #13 0x00007ffff55f25ac in __clone3 () at /lib64/libc.so.6
>
> The intention of the aforementioned commit is to prevent a MemoryRegion
> from parenting itself while its references is counted indendependently
> of the device. To achieve the same goal, add a type of QOM objects that
> count references and parent MemoryRegions.
>
> [1] https://lore.kernel.org/qemu-devel/4eb93d7a-1fa9-4b3c-8ad7-a2eb64f025a0@collabora.com/
>
> Cc: qemu-stable@nongnu.org
> Fixes: e27194e087ae ("virtio-gpu-virgl: correct parent for blob memory region")
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---
> base-commit: ece408818d27f745ef1b05fb3cc99a1e7a5bf580
> change-id: 20260214-region-690a177c0475
>
> Best regards,
> --
> Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>

Tested-by: Joelle van Dyne <j@getutm.app>