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);
      }
  }


Reply via email to