diff options
| author | Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp> | 2026-02-14 13:33:36 +0900 |
|---|---|---|
| committer | Michael Tokarev <mjt@tls.msk.ru> | 2026-02-23 21:51:43 +0300 |
| commit | fbf071b77d3c5f110b125090711fc8b7a858bfde (patch) | |
| tree | 5e570fffd0e35b408a7ca54961f2098cca4cde3d | |
| parent | cfe35fe60a715059860ab5be22993c8cc8b11caf (diff) | |
| download | qemu-fbf071b77d3c5f110b125090711fc8b7a858bfde.tar.gz qemu-fbf071b77d3c5f110b125090711fc8b7a858bfde.zip | |
virtio-gpu-virgl: Add virtio-gpu-virgl-hostmem-region type
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")
Fixes: da9b1cd37f66 ("virtio-gpu-virgl: correct parent for blob memory region") in 10.1.x
Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Tested-by: Joelle van Dyne <j@getutm.app>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20260214-region-v1-1-229f00ae1f38@rsg.ci.i.u-tokyo.ac.jp>
(cherry picked from commit b2a279094c3b86667969cc645f7fb1087e08dd19)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
| -rw-r--r-- | hw/display/virtio-gpu-virgl.c | 54 |
1 files changed, 39 insertions, 15 deletions
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c index b25ddc0746..362828f54e 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; return 0; @@ -159,7 +183,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) { @@ -182,7 +206,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; |
