On 2025/09/03 7:42, Vivek Kasireddy wrote:
The res->blob pointer is only valid for blobs that have their
backing storage in memfd. Therefore, we cannot use it to determine
if a resource is a blob or not. Instead, we could use res->blob_size
to make this determination as it is non-zero for blob resources
regardless of where their backing storage is located.
I guess this change needs to be applied before "[RFC 3/6]
virtio-gpu-udmabuf: Create dmabuf for blobs associated with VFIO
devices"; without this patch, the "create dmabuf" patch will probably
create an invalid blob.
Cc: Marc-André Lureau <marcandre.lur...@redhat.com>
Cc: Alex Bennée <alex.ben...@linaro.org>
Cc: Akihiko Odaki <od...@rsg.ci.i.u-tokyo.ac.jp>
Cc: Dmitry Osipenko <dmitry.osipe...@collabora.com>
Signed-off-by: Vivek Kasireddy <vivek.kasire...@intel.com>
---
hw/display/virtio-gpu.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 0a1a625b0e..2f9133c3b6 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -57,7 +57,7 @@ void virtio_gpu_update_cursor_data(VirtIOGPU *g,
}
if (res->blob_size) {
- if (res->blob_size < (s->current_cursor->width *
+ if (!res->blob || res->blob_size < (s->current_cursor->width *
I doubt that rejecting a valid blob due to an implementation concern
(whether the backing storage is in memfd) is tolerated in the specification.
s->current_cursor->height * 4)) {
return;
}
@@ -144,7 +144,7 @@ virtio_gpu_find_check_resource(VirtIOGPU *g, uint32_t
resource_id,
}
if (require_backing) {
- if (!res->iov || (!res->image && !res->blob)) {
+ if (!res->iov || (!res->image && !res->blob_size)) {
qemu_log_mask(LOG_GUEST_ERROR, "%s: no backing storage %d\n",
caller, resource_id);
if (error) {
@@ -444,7 +444,7 @@ static void virtio_gpu_transfer_to_host_2d(VirtIOGPU *g,
res = virtio_gpu_find_check_resource(g, t2d.resource_id, true,
__func__, &cmd->error);
- if (!res || res->blob) {
+ if (!res || res->blob_size) {
return;
}
@@ -507,7 +507,7 @@ static void virtio_gpu_resource_flush(VirtIOGPU *g,
return;
}
- if (res->blob) {
+ if (res->blob_size) {
for (i = 0; i < g->parent_obj.conf.max_outputs; i++) {
scanout = &g->parent_obj.scanout[i];
if (scanout->resource_id == res->resource_id &&
@@ -538,7 +538,7 @@ static void virtio_gpu_resource_flush(VirtIOGPU *g,
}
}
- if (!res->blob &&
+ if (!res->blob_size &&
(rf.r.x > res->width ||
rf.r.y > res->height ||
rf.r.width > res->width ||
@@ -634,7 +634,7 @@ static bool virtio_gpu_do_set_scanout(VirtIOGPU *g,
g->parent_obj.enable = 1;
- if (res->blob) {
+ if (res->blob_size) {
if (console_has_gl(scanout->con)) {
if (!virtio_gpu_update_dmabuf(g, scanout_id, res, fb, r)) {
virtio_gpu_update_scanout(g, scanout_id, res, fb, r);
@@ -645,13 +645,16 @@ static bool virtio_gpu_do_set_scanout(VirtIOGPU *g,
return true;
}
+ if (!res->blob) {
+ return false;
+ }
data = res->blob;
} else {
data = (uint8_t *)pixman_image_get_data(res->image);
}
/* create a surface for this scanout */
- if ((res->blob && !console_has_gl(scanout->con)) ||
+ if ((res->blob_size && !console_has_gl(scanout->con)) ||
!scanout->ds ||
surface_data(scanout->ds) != data + fb->offset ||
scanout->width != r->width ||
@@ -899,7 +902,7 @@ void virtio_gpu_cleanup_mapping(VirtIOGPU *g,
g_free(res->addrs);
res->addrs = NULL;
- if (res->blob) {
+ if (res->blob_size) {
virtio_gpu_fini_udmabuf(res);
}
}