On 2026/08/28 16:53, [email protected] wrote:
From: Marc-André Lureau <[email protected]>
All allocation sites used g_new0() which zero-initializes fields.
However share_handle must be SHAREABLE_NONE (-1 on Unix, NULL on
Windows) and dmabuf_fd must be -1, not 0. Centralize allocation and
field initialization in new constructors fix this and reduce code
duplication.
This fixes -display dbus with virtio-gpu blob resources. The other
end is currently receiving qemu fd 0.
The bug was introduced by 5f899c34af1d (“virtio-gpu: allocate
shareable 2d resources on !win32”). Please add the corresponding Fixes:
tag, as required by: docs/devel/submitting-a-patch.rst
Signed-off-by: Marc-André Lureau <[email protected]>
---
v2: add virgl_gpu_virgl_resource_new()
I think you meant virtio_gpu_virgl_resource_new().
---
hw/display/virtio-gpu-rutabaga.c | 20 +++++-------------
hw/display/virtio-gpu-virgl.c | 34 ++++++++++++++++++-------------
hw/display/virtio-gpu.c | 35 +++++++++++++++++++++-----------
include/hw/virtio/virtio-gpu.h | 4 ++++
4 files changed, 52 insertions(+), 41 deletions(-)
diff --git a/hw/display/virtio-gpu-rutabaga.c b/hw/display/virtio-gpu-rutabaga.c
index 041216a10d04..234fe94b178b 100644
--- a/hw/display/virtio-gpu-rutabaga.c
+++ b/hw/display/virtio-gpu-rutabaga.c
@@ -100,12 +100,8 @@ rutabaga_cmd_create_resource_2d(VirtIOGPU *g,
result = rutabaga_resource_create_3d(vr->rutabaga, c2d.resource_id,
&rc_3d);
CHECK(!result, cmd);
- res = g_new0(struct virtio_gpu_simple_resource, 1);
- res->width = c2d.width;
- res->height = c2d.height;
- res->format = c2d.format;
- res->resource_id = c2d.resource_id;
-
+ res = virtio_gpu_simple_resource_new(c2d.resource_id, c2d.width,
+ c2d.height, c2d.format);
QTAILQ_INSERT_HEAD(&g->reslist, res, next);
}
@@ -139,12 +135,8 @@ rutabaga_cmd_create_resource_3d(VirtIOGPU *g,
result = rutabaga_resource_create_3d(vr->rutabaga, c3d.resource_id,
&rc_3d);
CHECK(!result, cmd);
- res = g_new0(struct virtio_gpu_simple_resource, 1);
- res->width = c3d.width;
- res->height = c3d.height;
- res->format = c3d.format;
- res->resource_id = c3d.resource_id;
-
+ res = virtio_gpu_simple_resource_new(c3d.resource_id, c3d.width,
+ c3d.height, c3d.format);
QTAILQ_INSERT_HEAD(&g->reslist, res, next);
}
@@ -634,9 +626,7 @@ rutabaga_cmd_resource_create_blob(VirtIOGPU *g,
CHECK(cblob.resource_id != 0, cmd);
- res = g_new0(struct virtio_gpu_simple_resource, 1);
-
- res->resource_id = cblob.resource_id;
+ res = virtio_gpu_simple_resource_new(cblob.resource_id, 0, 0, 0);
res->blob_size = cblob.size;
if (cblob.blob_mem != VIRTIO_GPU_BLOB_MEM_HOST3D) {
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 9bda572426b2..d32801c3893a 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -309,6 +309,23 @@ virtio_gpu_virgl_unmap_resource_blob(VirtIOGPU *g,
}
#endif
+static struct virtio_gpu_virgl_resource *
+virtio_gpu_virgl_resource_new(uint32_t resource_id, uint32_t width,
+ uint32_t height, uint32_t format)
+{
+ struct virtio_gpu_virgl_resource *res = g_new0(struct
virtio_gpu_virgl_resource, 1);
+
+ res->base.share_handle = SHAREABLE_NONE;
+ res->base.dmabuf_fd = -1;
+
+ res->base.resource_id = resource_id;
+ res->base.width = width;
+ res->base.height = height;
+ res->base.format = format;
I think it's better to omit the width, height, and format assignments.
They only matter for non-blob resources, so they do not fit well in the
common function. blob_size, which is only valid for blob resources, is
also assigned outside this function, so we can make them consistent.
Having four integer parameters is also slightly confusing. Reducing the
number of parameters may make sense in this regard too.
With the four assignments removed, this and
virtio_gpu_simple_resource_new() will become quite small, so I'm not
sure if we should have virtio_gpu_simple_resource_init() or something
similar to share the remaining assignments; I'm kind of netural in that
aspect.
+
+ return res;
+}
+
static void virgl_cmd_create_resource_2d(VirtIOGPU *g,
struct virtio_gpu_ctrl_command *cmd)
{
@@ -335,12 +352,7 @@ static void virgl_cmd_create_resource_2d(VirtIOGPU *g,
return;
}
- res = g_new0(struct virtio_gpu_virgl_resource, 1);
- res->base.width = c2d.width;
- res->base.height = c2d.height;
- res->base.format = c2d.format;
- res->base.resource_id = c2d.resource_id;
- res->base.dmabuf_fd = -1;
+ res = virtio_gpu_virgl_resource_new(c2d.resource_id, c2d.width,
c2d.height, c2d.format);
QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next);
args.handle = c2d.resource_id;
@@ -383,12 +395,7 @@ static void virgl_cmd_create_resource_3d(VirtIOGPU *g,
return;
}
- res = g_new0(struct virtio_gpu_virgl_resource, 1);
- res->base.width = c3d.width;
- res->base.height = c3d.height;
- res->base.format = c3d.format;
- res->base.resource_id = c3d.resource_id;
- res->base.dmabuf_fd = -1;
+ res = virtio_gpu_virgl_resource_new(c3d.resource_id, c3d.width,
c3d.height, c3d.format);
QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next);
args.handle = c3d.resource_id;
@@ -853,8 +860,7 @@ static void virgl_cmd_resource_create_blob(VirtIOGPU *g,
return;
}
- res = g_new0(struct virtio_gpu_virgl_resource, 1);
- res->base.resource_id = cblob.resource_id;
+ res = virtio_gpu_virgl_resource_new(cblob.resource_id, 0, 0, 0);
res->base.blob_size = cblob.size;
res->base.dmabuf_fd = -1;
This dmabuf_fd assignment is now redundant.
Regards,
Akihiko Odaki
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 55a1c7f80fb8..d5405f0c715f 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -41,6 +41,24 @@ virtio_gpu_find_check_resource(VirtIOGPU *g, uint32_t
resource_id,
static void virtio_gpu_reset_bh(void *opaque);
+struct virtio_gpu_simple_resource *
+virtio_gpu_simple_resource_new(uint32_t resource_id, uint32_t width,
+ uint32_t height, uint32_t format)
+{
+ struct virtio_gpu_simple_resource *res =
+ g_new0(struct virtio_gpu_simple_resource, 1);
+
+ res->share_handle = SHAREABLE_NONE;
+ res->dmabuf_fd = -1;
+
+ res->resource_id = resource_id;
+ res->width = width;
+ res->height = height;
+ res->format = format;
+
+ return res;
+}
+
void virtio_gpu_update_cursor_data(VirtIOGPU *g,
struct virtio_gpu_scanout *s,
uint32_t resource_id)
@@ -259,12 +277,8 @@ static void virtio_gpu_resource_create_2d(VirtIOGPU *g,
return;
}
- res = g_new0(struct virtio_gpu_simple_resource, 1);
-
- res->width = c2d.width;
- res->height = c2d.height;
- res->format = c2d.format;
- res->resource_id = c2d.resource_id;
+ res = virtio_gpu_simple_resource_new(c2d.resource_id, c2d.width,
+ c2d.height, c2d.format);
pformat = virtio_gpu_get_pixman_format(c2d.format);
if (!pformat) {
@@ -345,8 +359,7 @@ static void virtio_gpu_resource_create_blob(VirtIOGPU *g,
return;
}
- res = g_new0(struct virtio_gpu_simple_resource, 1);
- res->resource_id = cblob.resource_id;
+ res = virtio_gpu_simple_resource_new(cblob.resource_id, 0, 0, 0);
res->blob_size = cblob.size;
if (cblob.nr_entries) {
@@ -1442,8 +1455,7 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque,
size_t size,
return -EINVAL;
}
- res = g_new0(struct virtio_gpu_simple_resource, 1);
- res->resource_id = resource_id;
+ res = virtio_gpu_simple_resource_new(resource_id, 0, 0, 0);
res->width = qemu_get_be32(f);
res->height = qemu_get_be32(f);
res->format = qemu_get_be32(f);
@@ -1555,8 +1567,7 @@ static int virtio_gpu_blob_load(QEMUFile *f, void
*opaque, size_t size,
return -EINVAL;
}
- res = g_new0(struct virtio_gpu_simple_resource, 1);
- res->resource_id = resource_id;
+ res = virtio_gpu_simple_resource_new(resource_id, 0, 0, 0);
res->blob_size = qemu_get_be32(f);
res->iov_cnt = qemu_get_be32(f);
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 69b5ee2e382f..090719aa0522 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -64,6 +64,10 @@ struct virtio_gpu_simple_resource {
QTAILQ_ENTRY(virtio_gpu_simple_resource) next;
};
+struct virtio_gpu_simple_resource *
+virtio_gpu_simple_resource_new(uint32_t resource_id, uint32_t width,
+ uint32_t height, uint32_t format);
+
struct virtio_gpu_framebuffer {
pixman_format_code_t format;
uint32_t width, height;
