[PATCH v6 0/3] ui/console: Private QemuDmaBuf struct

[PATCH v6 0/3] ui/console: Private QemuDmaBuf struct

[dongwon.kim]

From: Dongwon Kim <dongwon.kim@intel.com>

This series introduces privacy enhancements to the QemuDmaBuf struct
and its contained data to bolster security. it accomplishes this by
introducing of helper functions for allocating, deallocating, and
accessing individual fields within the struct and replacing all direct
references to individual fields in the struct with methods using helpers
throughout the codebase.

This change was made based on a suggestion from Marc-André Lureau
<marcandre.lureau@redhat.com>

(Resumitting same patch series with this new cover-leter)

v6: fixed some typos in patch - 
    ui/console: Introduce dpy_gl_qemu_dmabuf_get_..() helpers)

Dongwon Kim (3):
  ui/console: Introduce dpy_gl_qemu_dmabuf_get_..() helpers
  ui/console: Introduce dpy_gl_qemu_dmabuf_set_..() helpers
  ui/console: Introduce dpy_gl_qemu_dmabuf_new() and free() helpers

 include/hw/vfio/vfio-common.h   |   2 +-
 include/hw/virtio/virtio-gpu.h  |   4 +-
 include/ui/console.h            |  28 +++++
 hw/display/vhost-user-gpu.c     |  32 +++---
 hw/display/virtio-gpu-udmabuf.c |  27 ++---
 hw/vfio/display.c               |  35 ++++---
 ui/console.c                    | 180 +++++++++++++++++++++++++++++++-
 ui/dbus-console.c               |   9 +-
 ui/dbus-listener.c              |  71 +++++++------
 ui/egl-headless.c               |  23 ++--
 ui/egl-helpers.c                |  59 ++++++-----
 ui/gtk-egl.c                    |  52 +++++----
 ui/gtk-gl-area.c                |  41 +++++---
 ui/gtk.c                        |   8 +-
 ui/spice-display.c              |  50 +++++----
 15 files changed, 449 insertions(+), 172 deletions(-)

-- 
2.34.1

[PATCH v6 1/3] ui/console: Introduce dpy_gl_qemu_dmabuf_get_..() helpers

[dongwon.kim]

From: Dongwon Kim <dongwon.kim@intel.com>

This commit introduces dpy_gl_qemu_dmabuf_get_... helpers to extract
specific fields from the QemuDmaBuf struct. It also updates all instances
where fields within the QemuDmaBuf struct are directly accessed, replacing
them with calls to these new helper functions.

v6: fix typos in helper names in ui/spice-display.c

Suggested-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
---
 include/ui/console.h            |  17 +++++
 hw/display/vhost-user-gpu.c     |   6 +-
 hw/display/virtio-gpu-udmabuf.c |   7 +-
 hw/vfio/display.c               |  15 +++--
 ui/console.c                    | 116 +++++++++++++++++++++++++++++++-
 ui/dbus-console.c               |   9 ++-
 ui/dbus-listener.c              |  43 +++++++-----
 ui/egl-headless.c               |  23 +++++--
 ui/egl-helpers.c                |  47 +++++++------
 ui/gtk-egl.c                    |  48 ++++++++-----
 ui/gtk-gl-area.c                |  37 ++++++----
 ui/gtk.c                        |   6 +-
 ui/spice-display.c              |  50 ++++++++------
 13 files changed, 316 insertions(+), 108 deletions(-)

diff --git a/include/ui/console.h b/include/ui/console.h
index 0bc7a00ac0..6292943a82 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -358,6 +358,23 @@ void dpy_gl_cursor_dmabuf(QemuConsole *con, QemuDmaBuf *dmabuf,
                           bool have_hot, uint32_t hot_x, uint32_t hot_y);
 void dpy_gl_cursor_position(QemuConsole *con,
                             uint32_t pos_x, uint32_t pos_y);
+
+int32_t dpy_gl_qemu_dmabuf_get_fd(QemuDmaBuf *dmabuf);
+uint32_t dpy_gl_qemu_dmabuf_get_width(QemuDmaBuf *dmabuf);
+uint32_t dpy_gl_qemu_dmabuf_get_height(QemuDmaBuf *dmabuf);
+uint32_t dpy_gl_qemu_dmabuf_get_stride(QemuDmaBuf *dmabuf);
+uint32_t dpy_gl_qemu_dmabuf_get_fourcc(QemuDmaBuf *dmabuf);
+uint64_t dpy_gl_qemu_dmabuf_get_modifier(QemuDmaBuf *dmabuf);
+uint32_t dpy_gl_qemu_dmabuf_get_texture(QemuDmaBuf *dmabuf);
+uint32_t dpy_gl_qemu_dmabuf_get_x(QemuDmaBuf *dmabuf);
+uint32_t dpy_gl_qemu_dmabuf_get_y(QemuDmaBuf *dmabuf);
+uint32_t dpy_gl_qemu_dmabuf_get_backing_width(QemuDmaBuf *dmabuf);
+uint32_t dpy_gl_qemu_dmabuf_get_backing_height(QemuDmaBuf *dmabuf);
+bool dpy_gl_qemu_dmabuf_get_y0_top(QemuDmaBuf *dmabuf);
+void *dpy_gl_qemu_dmabuf_get_sync(QemuDmaBuf *dmabuf);
+int32_t dpy_gl_qemu_dmabuf_get_fence_fd(QemuDmaBuf *dmabuf);
+bool dpy_gl_qemu_dmabuf_get_allow_fences(QemuDmaBuf *dmabuf);
+bool dpy_gl_qemu_dmabuf_get_draw_submitted(QemuDmaBuf *dmabuf);
 void dpy_gl_release_dmabuf(QemuConsole *con,
                            QemuDmaBuf *dmabuf);
 void dpy_gl_update(QemuConsole *con,
diff --git a/hw/display/vhost-user-gpu.c b/hw/display/vhost-user-gpu.c
index 709c8a02a1..87dcfbca10 100644
--- a/hw/display/vhost-user-gpu.c
+++ b/hw/display/vhost-user-gpu.c
@@ -249,6 +249,7 @@ vhost_user_gpu_handle_display(VhostUserGPU *g, VhostUserGpuMsg *msg)
     case VHOST_USER_GPU_DMABUF_SCANOUT: {
         VhostUserGpuDMABUFScanout *m = &msg->payload.dmabuf_scanout;
         int fd = qemu_chr_fe_get_msgfd(&g->vhost_chr);
+        int old_fd;
         QemuDmaBuf *dmabuf;
 
         if (m->scanout_id >= g->parent_obj.conf.max_outputs) {
@@ -262,8 +263,9 @@ vhost_user_gpu_handle_display(VhostUserGPU *g, VhostUserGpuMsg *msg)
         g->parent_obj.enable = 1;
         con = g->parent_obj.scanout[m->scanout_id].con;
         dmabuf = &g->dmabuf[m->scanout_id];
-        if (dmabuf->fd >= 0) {
-            close(dmabuf->fd);
+        old_fd = dpy_gl_qemu_dmabuf_get_fd(dmabuf);
+        if (old_fd >= 0) {
+            close(old_fd);
             dmabuf->fd = -1;
         }
         dpy_gl_release_dmabuf(con, dmabuf);
diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
index d51184d658..e3f358b575 100644
--- a/hw/display/virtio-gpu-udmabuf.c
+++ b/hw/display/virtio-gpu-udmabuf.c
@@ -206,6 +206,7 @@ int virtio_gpu_update_dmabuf(VirtIOGPU *g,
 {
     struct virtio_gpu_scanout *scanout = &g->parent_obj.scanout[scanout_id];
     VGPUDMABuf *new_primary, *old_primary = NULL;
+    uint32_t width, height;
 
     new_primary = virtio_gpu_create_dmabuf(g, scanout_id, res, fb, r);
     if (!new_primary) {
@@ -216,10 +217,10 @@ int virtio_gpu_update_dmabuf(VirtIOGPU *g,
         old_primary = g->dmabuf.primary[scanout_id];
     }
 
+    width = dpy_gl_qemu_dmabuf_get_width(&new_primary->buf);
+    height = dpy_gl_qemu_dmabuf_get_height(&new_primary->buf);
     g->dmabuf.primary[scanout_id] = new_primary;
-    qemu_console_resize(scanout->con,
-                        new_primary->buf.width,
-                        new_primary->buf.height);
+    qemu_console_resize(scanout->con, width, height);
     dpy_gl_scanout_dmabuf(scanout->con, &new_primary->buf);
 
     if (old_primary) {
diff --git a/hw/vfio/display.c b/hw/vfio/display.c
index 1aa440c663..f9c39cbd51 100644
--- a/hw/vfio/display.c
+++ b/hw/vfio/display.c
@@ -259,9 +259,13 @@ static VFIODMABuf *vfio_display_get_dmabuf(VFIOPCIDevice *vdev,
 
 static void vfio_display_free_one_dmabuf(VFIODisplay *dpy, VFIODMABuf *dmabuf)
 {
+    int fd;
+
     QTAILQ_REMOVE(&dpy->dmabuf.bufs, dmabuf, next);
+
+    fd = dpy_gl_qemu_dmabuf_get_fd(&dmabuf->buf);
     dpy_gl_release_dmabuf(dpy->con, &dmabuf->buf);
-    close(dmabuf->buf.fd);
+    close(fd);
     g_free(dmabuf);
 }
 
@@ -286,6 +290,7 @@ static void vfio_display_dmabuf_update(void *opaque)
     VFIOPCIDevice *vdev = opaque;
     VFIODisplay *dpy = vdev->dpy;
     VFIODMABuf *primary, *cursor;
+    uint32_t width, height;
     bool free_bufs = false, new_cursor = false;
 
     primary = vfio_display_get_dmabuf(vdev, DRM_PLANE_TYPE_PRIMARY);
@@ -296,10 +301,12 @@ static void vfio_display_dmabuf_update(void *opaque)
         return;
     }
 
+    width = dpy_gl_qemu_dmabuf_get_width(&primary->buf);
+    height = dpy_gl_qemu_dmabuf_get_height(&primary->buf);
+
     if (dpy->dmabuf.primary != primary) {
         dpy->dmabuf.primary = primary;
-        qemu_console_resize(dpy->con,
-                            primary->buf.width, primary->buf.height);
+        qemu_console_resize(dpy->con, width, height);
         dpy_gl_scanout_dmabuf(dpy->con, &primary->buf);
         free_bufs = true;
     }
@@ -328,7 +335,7 @@ static void vfio_display_dmabuf_update(void *opaque)
         cursor->pos_updates = 0;
     }
 
-    dpy_gl_update(dpy->con, 0, 0, primary->buf.width, primary->buf.height);
+    dpy_gl_update(dpy->con, 0, 0, width, height);
 
     if (free_bufs) {
         vfio_display_free_dmabufs(vdev);
diff --git a/ui/console.c b/ui/console.c
index 43226c5c14..5d5635f783 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1132,6 +1132,118 @@ void dpy_gl_cursor_position(QemuConsole *con,
     }
 }
 
+int32_t dpy_gl_qemu_dmabuf_get_fd(QemuDmaBuf *dmabuf)
+{
+    assert(dmabuf != NULL);
+
+    return dmabuf->fd;
+}
+
+uint32_t dpy_gl_qemu_dmabuf_get_width(QemuDmaBuf *dmabuf)
+{
+    assert(dmabuf != NULL);
+
+    return dmabuf->width;
+}
+
+uint32_t dpy_gl_qemu_dmabuf_get_height(QemuDmaBuf *dmabuf)
+{
+    assert(dmabuf != NULL);
+
+    return dmabuf->height;
+}
+
+uint32_t dpy_gl_qemu_dmabuf_get_stride(QemuDmaBuf *dmabuf)
+{
+    assert(dmabuf != NULL);
+
+    return dmabuf->stride;
+}
+
+uint32_t dpy_gl_qemu_dmabuf_get_fourcc(QemuDmaBuf *dmabuf)
+{
+    assert(dmabuf != NULL);
+
+    return dmabuf->fourcc;
+}
+
+uint64_t dpy_gl_qemu_dmabuf_get_modifier(QemuDmaBuf *dmabuf)
+{
+    assert(dmabuf != NULL);
+
+    return dmabuf->modifier;
+}
+
+uint32_t dpy_gl_qemu_dmabuf_get_texture(QemuDmaBuf *dmabuf)
+{
+    assert(dmabuf != NULL);
+
+    return dmabuf->texture;
+}
+
+uint32_t dpy_gl_qemu_dmabuf_get_x(QemuDmaBuf *dmabuf)
+{
+    assert(dmabuf != NULL);
+
+    return dmabuf->x;
+}
+
+uint32_t dpy_gl_qemu_dmabuf_get_y(QemuDmaBuf *dmabuf)
+{
+    assert(dmabuf != NULL);
+
+    return dmabuf->y;
+}
+
+uint32_t dpy_gl_qemu_dmabuf_get_backing_width(QemuDmaBuf *dmabuf)
+{
+    assert(dmabuf != NULL);
+
+    return dmabuf->backing_width;
+}
+
+uint32_t dpy_gl_qemu_dmabuf_get_backing_height(QemuDmaBuf *dmabuf)
+{
+    assert(dmabuf != NULL);
+
+    return dmabuf->backing_height;
+}
+
+bool dpy_gl_qemu_dmabuf_get_y0_top(QemuDmaBuf *dmabuf)
+{
+    assert(dmabuf != NULL);
+
+    return dmabuf->y0_top;
+}
+
+void *dpy_gl_qemu_dmabuf_get_sync(QemuDmaBuf *dmabuf)
+{
+    assert(dmabuf != NULL);
+
+    return dmabuf->sync;
+}
+
+int32_t dpy_gl_qemu_dmabuf_get_fence_fd(QemuDmaBuf *dmabuf)
+{
+    assert(dmabuf != NULL);
+
+    return dmabuf->fence_fd;
+}
+
+bool dpy_gl_qemu_dmabuf_get_allow_fences(QemuDmaBuf *dmabuf)
+{
+    assert(dmabuf != NULL);
+
+    return dmabuf->allow_fences;
+}
+
+bool dpy_gl_qemu_dmabuf_get_draw_submitted(QemuDmaBuf *dmabuf)
+{
+    assert(dmabuf != NULL);
+
+    return dmabuf->draw_submitted;
+}
+
 void dpy_gl_release_dmabuf(QemuConsole *con,
                           QemuDmaBuf *dmabuf)
 {
@@ -1459,7 +1571,7 @@ int qemu_console_get_width(QemuConsole *con, int fallback)
     }
     switch (con->scanout.kind) {
     case SCANOUT_DMABUF:
-        return con->scanout.dmabuf->width;
+        return dpy_gl_qemu_dmabuf_get_width(con->scanout.dmabuf);
     case SCANOUT_TEXTURE:
         return con->scanout.texture.width;
     case SCANOUT_SURFACE:
@@ -1476,7 +1588,7 @@ int qemu_console_get_height(QemuConsole *con, int fallback)
     }
     switch (con->scanout.kind) {
     case SCANOUT_DMABUF:
-        return con->scanout.dmabuf->height;
+        return dpy_gl_qemu_dmabuf_get_height(con->scanout.dmabuf);
     case SCANOUT_TEXTURE:
         return con->scanout.texture.height;
     case SCANOUT_SURFACE:
diff --git a/ui/dbus-console.c b/ui/dbus-console.c
index 49da9ccc83..124fe16253 100644
--- a/ui/dbus-console.c
+++ b/ui/dbus-console.c
@@ -110,11 +110,14 @@ static void
 dbus_gl_scanout_dmabuf(DisplayChangeListener *dcl,
                        QemuDmaBuf *dmabuf)
 {
+    uint32_t width, height;
+
     DBusDisplayConsole *ddc = container_of(dcl, DBusDisplayConsole, dcl);
 
-    dbus_display_console_set_size(ddc,
-                                  dmabuf->width,
-                                  dmabuf->height);
+    width = dpy_gl_qemu_dmabuf_get_width(dmabuf);
+    height = dpy_gl_qemu_dmabuf_get_height(dmabuf);
+
+    dbus_display_console_set_size(ddc, width, height);
 }
 
 static void
diff --git a/ui/dbus-listener.c b/ui/dbus-listener.c
index 4a0a5d78f9..c6c7d93753 100644
--- a/ui/dbus-listener.c
+++ b/ui/dbus-listener.c
@@ -278,29 +278,33 @@ static void dbus_scanout_dmabuf(DisplayChangeListener *dcl,
     DBusDisplayListener *ddl = container_of(dcl, DBusDisplayListener, dcl);
     g_autoptr(GError) err = NULL;
     g_autoptr(GUnixFDList) fd_list = NULL;
+    int fd;
+    uint32_t width, height, stride, fourcc;
+    uint64_t modifier;
+    bool y0_top;
 
+    fd = dpy_gl_qemu_dmabuf_get_fd(dmabuf);
     fd_list = g_unix_fd_list_new();
-    if (g_unix_fd_list_append(fd_list, dmabuf->fd, &err) != 0) {
+    if (g_unix_fd_list_append(fd_list, fd, &err) != 0) {
         error_report("Failed to setup dmabuf fdlist: %s", err->message);
         return;
     }
 
     ddl_discard_pending_messages(ddl);
 
+    width = dpy_gl_qemu_dmabuf_get_width(dmabuf);
+    height = dpy_gl_qemu_dmabuf_get_height(dmabuf);
+    stride = dpy_gl_qemu_dmabuf_get_stride(dmabuf);
+    fourcc = dpy_gl_qemu_dmabuf_get_fourcc(dmabuf);
+    modifier = dpy_gl_qemu_dmabuf_get_modifier(dmabuf);
+    y0_top = dpy_gl_qemu_dmabuf_get_y0_top(dmabuf);
+
     /* FIXME: add missing x/y/w/h support */
     qemu_dbus_display1_listener_call_scanout_dmabuf(
-        ddl->proxy,
-        g_variant_new_handle(0),
-        dmabuf->width,
-        dmabuf->height,
-        dmabuf->stride,
-        dmabuf->fourcc,
-        dmabuf->modifier,
-        dmabuf->y0_top,
-        G_DBUS_CALL_FLAGS_NONE,
-        -1,
-        fd_list,
-        NULL, NULL, NULL);
+        ddl->proxy, g_variant_new_handle(0),
+        width, height, stride, fourcc, modifier,
+        y0_top, G_DBUS_CALL_FLAGS_NONE,
+        -1, fd_list, NULL, NULL, NULL);
 }
 #endif /* GBM */
 #endif /* OPENGL */
@@ -488,6 +492,7 @@ static void dbus_cursor_dmabuf(DisplayChangeListener *dcl,
     DisplaySurface *ds;
     GVariant *v_data = NULL;
     egl_fb cursor_fb = EGL_FB_INIT;
+    uint32_t width, height, texture;
 
     if (!dmabuf) {
         qemu_dbus_display1_listener_call_mouse_set(
@@ -497,12 +502,16 @@ static void dbus_cursor_dmabuf(DisplayChangeListener *dcl,
     }
 
     egl_dmabuf_import_texture(dmabuf);
-    if (!dmabuf->texture) {
+    texture = dpy_gl_qemu_dmabuf_get_texture(dmabuf);
+    if (!texture) {
         return;
     }
-    egl_fb_setup_for_tex(&cursor_fb, dmabuf->width, dmabuf->height,
-                         dmabuf->texture, false);
-    ds = qemu_create_displaysurface(dmabuf->width, dmabuf->height);
+
+    width = dpy_gl_qemu_dmabuf_get_width(dmabuf);
+    height = dpy_gl_qemu_dmabuf_get_height(dmabuf);
+
+    egl_fb_setup_for_tex(&cursor_fb, width, height, texture, false);
+    ds = qemu_create_displaysurface(width, height);
     egl_fb_read(ds, &cursor_fb);
 
     v_data = g_variant_new_from_data(
diff --git a/ui/egl-headless.c b/ui/egl-headless.c
index d5637dadb2..158924379b 100644
--- a/ui/egl-headless.c
+++ b/ui/egl-headless.c
@@ -85,29 +85,38 @@ static void egl_scanout_texture(DisplayChangeListener *dcl,
 static void egl_scanout_dmabuf(DisplayChangeListener *dcl,
                                QemuDmaBuf *dmabuf)
 {
+    uint32_t width, height, texture;
+
     egl_dmabuf_import_texture(dmabuf);
-    if (!dmabuf->texture) {
+    texture = dpy_gl_qemu_dmabuf_get_texture(dmabuf);
+    if (!texture) {
         return;
     }
 
-    egl_scanout_texture(dcl, dmabuf->texture,
-                        false, dmabuf->width, dmabuf->height,
-                        0, 0, dmabuf->width, dmabuf->height, NULL);
+    width = dpy_gl_qemu_dmabuf_get_width(dmabuf);
+    height = dpy_gl_qemu_dmabuf_get_height(dmabuf);
+
+    egl_scanout_texture(dcl, texture, false, width, height, 0, 0,
+                        width, height, NULL);
 }
 
 static void egl_cursor_dmabuf(DisplayChangeListener *dcl,
                               QemuDmaBuf *dmabuf, bool have_hot,
                               uint32_t hot_x, uint32_t hot_y)
 {
+    uint32_t width, height, texture;
     egl_dpy *edpy = container_of(dcl, egl_dpy, dcl);
 
     if (dmabuf) {
         egl_dmabuf_import_texture(dmabuf);
-        if (!dmabuf->texture) {
+        texture = dpy_gl_qemu_dmabuf_get_texture(dmabuf);
+        if (!texture) {
             return;
         }
-        egl_fb_setup_for_tex(&edpy->cursor_fb, dmabuf->width, dmabuf->height,
-                             dmabuf->texture, false);
+
+        width = dpy_gl_qemu_dmabuf_get_width(dmabuf);
+        height = dpy_gl_qemu_dmabuf_get_height(dmabuf);
+        egl_fb_setup_for_tex(&edpy->cursor_fb, width, height, texture, false);
     } else {
         egl_fb_destroy(&edpy->cursor_fb);
     }
diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
index 3d19dbe382..86d64c68ce 100644
--- a/ui/egl-helpers.c
+++ b/ui/egl-helpers.c
@@ -146,10 +146,10 @@ void egl_fb_blit(egl_fb *dst, egl_fb *src, bool flip)
     glViewport(0, 0, dst->width, dst->height);
 
     if (src->dmabuf) {
-        x1 = src->dmabuf->x;
-        y1 = src->dmabuf->y;
-        w = src->dmabuf->width;
-        h = src->dmabuf->height;
+        x1 = dpy_gl_qemu_dmabuf_get_x(src->dmabuf);
+        y1 = dpy_gl_qemu_dmabuf_get_y(src->dmabuf);
+        w = dpy_gl_qemu_dmabuf_get_width(src->dmabuf);
+        h = dpy_gl_qemu_dmabuf_get_height(src->dmabuf);
     }
 
     w = (x1 + w) > src->width ? src->width - x1 : w;
@@ -308,30 +308,33 @@ void egl_dmabuf_import_texture(QemuDmaBuf *dmabuf)
     EGLImageKHR image = EGL_NO_IMAGE_KHR;
     EGLint attrs[64];
     int i = 0;
+    uint64_t modifier;
+    uint32_t texture = dpy_gl_qemu_dmabuf_get_texture(dmabuf);
 
-    if (dmabuf->texture != 0) {
+    if (texture != 0) {
         return;
     }
 
     attrs[i++] = EGL_WIDTH;
-    attrs[i++] = dmabuf->backing_width;
+    attrs[i++] = dpy_gl_qemu_dmabuf_get_backing_width(dmabuf);
     attrs[i++] = EGL_HEIGHT;
-    attrs[i++] = dmabuf->backing_height;
+    attrs[i++] = dpy_gl_qemu_dmabuf_get_backing_height(dmabuf);
     attrs[i++] = EGL_LINUX_DRM_FOURCC_EXT;
-    attrs[i++] = dmabuf->fourcc;
+    attrs[i++] = dpy_gl_qemu_dmabuf_get_fourcc(dmabuf);
 
     attrs[i++] = EGL_DMA_BUF_PLANE0_FD_EXT;
-    attrs[i++] = dmabuf->fd;
+    attrs[i++] = dpy_gl_qemu_dmabuf_get_fd(dmabuf);
     attrs[i++] = EGL_DMA_BUF_PLANE0_PITCH_EXT;
-    attrs[i++] = dmabuf->stride;
+    attrs[i++] = dpy_gl_qemu_dmabuf_get_stride(dmabuf);
     attrs[i++] = EGL_DMA_BUF_PLANE0_OFFSET_EXT;
     attrs[i++] = 0;
 #ifdef EGL_DMA_BUF_PLANE0_MODIFIER_LO_EXT
-    if (dmabuf->modifier) {
+    modifier = dpy_gl_qemu_dmabuf_get_modifier(dmabuf);
+    if (modifier) {
         attrs[i++] = EGL_DMA_BUF_PLANE0_MODIFIER_LO_EXT;
-        attrs[i++] = (dmabuf->modifier >>  0) & 0xffffffff;
+        attrs[i++] = (modifier >>  0) & 0xffffffff;
         attrs[i++] = EGL_DMA_BUF_PLANE0_MODIFIER_HI_EXT;
-        attrs[i++] = (dmabuf->modifier >> 32) & 0xffffffff;
+        attrs[i++] = (modifier >> 32) & 0xffffffff;
     }
 #endif
     attrs[i++] = EGL_NONE;
@@ -346,7 +349,8 @@ void egl_dmabuf_import_texture(QemuDmaBuf *dmabuf)
     }
 
     glGenTextures(1, &dmabuf->texture);
-    glBindTexture(GL_TEXTURE_2D, dmabuf->texture);
+    texture = dpy_gl_qemu_dmabuf_get_texture(dmabuf);
+    glBindTexture(GL_TEXTURE_2D, texture);
     glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
     glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
 
@@ -356,11 +360,14 @@ void egl_dmabuf_import_texture(QemuDmaBuf *dmabuf)
 
 void egl_dmabuf_release_texture(QemuDmaBuf *dmabuf)
 {
-    if (dmabuf->texture == 0) {
+    uint32_t texture;
+
+    texture = dpy_gl_qemu_dmabuf_get_texture(dmabuf);
+    if (texture == 0) {
         return;
     }
 
-    glDeleteTextures(1, &dmabuf->texture);
+    glDeleteTextures(1, &texture);
     dmabuf->texture = 0;
 }
 
@@ -382,10 +389,12 @@ void egl_dmabuf_create_sync(QemuDmaBuf *dmabuf)
 
 void egl_dmabuf_create_fence(QemuDmaBuf *dmabuf)
 {
-    if (dmabuf->sync) {
+    void *sync = dpy_gl_qemu_dmabuf_get_sync(dmabuf);
+
+    if (sync) {
         dmabuf->fence_fd = eglDupNativeFenceFDANDROID(qemu_egl_display,
-                                                      dmabuf->sync);
-        eglDestroySyncKHR(qemu_egl_display, dmabuf->sync);
+                                                      sync);
+        eglDestroySyncKHR(qemu_egl_display, sync);
         dmabuf->sync = NULL;
     }
 }
diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
index 3af5ac5bcf..c9469af9ed 100644
--- a/ui/gtk-egl.c
+++ b/ui/gtk-egl.c
@@ -70,6 +70,7 @@ void gd_egl_draw(VirtualConsole *vc)
     QemuDmaBuf *dmabuf = vc->gfx.guest_fb.dmabuf;
 #endif
     int ww, wh, ws;
+    int fence_fd;
 
     if (!vc->gfx.gls) {
         return;
@@ -83,7 +84,7 @@ void gd_egl_draw(VirtualConsole *vc)
     if (vc->gfx.scanout_mode) {
 #ifdef CONFIG_GBM
         if (dmabuf) {
-            if (!dmabuf->draw_submitted) {
+            if (!dpy_gl_qemu_dmabuf_get_draw_submitted(dmabuf)) {
                 return;
             } else {
                 dmabuf->draw_submitted = false;
@@ -99,8 +100,9 @@ void gd_egl_draw(VirtualConsole *vc)
 #ifdef CONFIG_GBM
         if (dmabuf) {
             egl_dmabuf_create_fence(dmabuf);
-            if (dmabuf->fence_fd > 0) {
-                qemu_set_fd_handler(dmabuf->fence_fd, gd_hw_gl_flushed, NULL, vc);
+            fence_fd = dpy_gl_qemu_dmabuf_get_fence_fd(dmabuf);
+            if (fence_fd > 0) {
+                qemu_set_fd_handler(fence_fd, gd_hw_gl_flushed, NULL, vc);
                 return;
             }
             graphic_hw_gl_block(vc->gfx.dcl.con, false);
@@ -149,7 +151,8 @@ void gd_egl_refresh(DisplayChangeListener *dcl)
     gd_update_monitor_refresh_rate(
             vc, vc->window ? vc->window : vc->gfx.drawing_area);
 
-    if (vc->gfx.guest_fb.dmabuf && vc->gfx.guest_fb.dmabuf->draw_submitted) {
+    if (vc->gfx.guest_fb.dmabuf &&
+        dpy_gl_qemu_dmabuf_get_draw_submitted(vc->gfx.guest_fb.dmabuf)) {
         return;
     }
 
@@ -264,22 +267,30 @@ void gd_egl_scanout_dmabuf(DisplayChangeListener *dcl,
 {
 #ifdef CONFIG_GBM
     VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
+    uint32_t x, y, width, height, backing_width, backing_height, texture;
+    bool y0_top;
 
     eglMakeCurrent(qemu_egl_display, vc->gfx.esurface,
                    vc->gfx.esurface, vc->gfx.ectx);
 
     egl_dmabuf_import_texture(dmabuf);
-    if (!dmabuf->texture) {
+    texture = dpy_gl_qemu_dmabuf_get_texture(dmabuf);
+    if (!texture) {
         return;
     }
 
-    gd_egl_scanout_texture(dcl, dmabuf->texture,
-                           dmabuf->y0_top,
-                           dmabuf->backing_width, dmabuf->backing_height,
-                           dmabuf->x, dmabuf->y, dmabuf->width,
-                           dmabuf->height, NULL);
+    x = dpy_gl_qemu_dmabuf_get_x(dmabuf);
+    y = dpy_gl_qemu_dmabuf_get_y(dmabuf);
+    width = dpy_gl_qemu_dmabuf_get_width(dmabuf);
+    height = dpy_gl_qemu_dmabuf_get_height(dmabuf);
+    backing_width = dpy_gl_qemu_dmabuf_get_backing_width(dmabuf);
+    backing_height = dpy_gl_qemu_dmabuf_get_backing_height(dmabuf);
+    y0_top = dpy_gl_qemu_dmabuf_get_y0_top(dmabuf);
 
-    if (dmabuf->allow_fences) {
+    gd_egl_scanout_texture(dcl, texture, y0_top, backing_width, backing_height,
+                           x, y, width, height, NULL);
+
+    if (dpy_gl_qemu_dmabuf_get_allow_fences(dmabuf)) {
         vc->gfx.guest_fb.dmabuf = dmabuf;
     }
 #endif
@@ -291,15 +302,19 @@ void gd_egl_cursor_dmabuf(DisplayChangeListener *dcl,
 {
 #ifdef CONFIG_GBM
     VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
+    uint32_t backing_width, backing_height, texture;
 
     if (dmabuf) {
         egl_dmabuf_import_texture(dmabuf);
-        if (!dmabuf->texture) {
+        texture = dpy_gl_qemu_dmabuf_get_texture(dmabuf);
+        if (!texture) {
             return;
         }
-        egl_fb_setup_for_tex(&vc->gfx.cursor_fb,
-                             dmabuf->backing_width, dmabuf->backing_height,
-                             dmabuf->texture, false);
+
+        backing_width = dpy_gl_qemu_dmabuf_get_backing_width(dmabuf);
+        backing_height = dpy_gl_qemu_dmabuf_get_backing_height(dmabuf);
+        egl_fb_setup_for_tex(&vc->gfx.cursor_fb, backing_width, backing_height,
+                             texture, false);
     } else {
         egl_fb_destroy(&vc->gfx.cursor_fb);
     }
@@ -363,7 +378,8 @@ void gd_egl_flush(DisplayChangeListener *dcl,
     VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
     GtkWidget *area = vc->gfx.drawing_area;
 
-    if (vc->gfx.guest_fb.dmabuf && !vc->gfx.guest_fb.dmabuf->draw_submitted) {
+    if (vc->gfx.guest_fb.dmabuf &&
+        !dpy_gl_qemu_dmabuf_get_draw_submitted(vc->gfx.guest_fb.dmabuf)) {
         graphic_hw_gl_block(vc->gfx.dcl.con, true);
         vc->gfx.guest_fb.dmabuf->draw_submitted = true;
         gtk_egl_set_scanout_mode(vc, true);
diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
index 52dcac161e..193862ecc2 100644
--- a/ui/gtk-gl-area.c
+++ b/ui/gtk-gl-area.c
@@ -60,7 +60,7 @@ void gd_gl_area_draw(VirtualConsole *vc)
 
 #ifdef CONFIG_GBM
         if (dmabuf) {
-            if (!dmabuf->draw_submitted) {
+            if (!dpy_gl_qemu_dmabuf_get_draw_submitted(dmabuf)) {
                 return;
             } else {
                 dmabuf->draw_submitted = false;
@@ -85,9 +85,11 @@ void gd_gl_area_draw(VirtualConsole *vc)
         glFlush();
 #ifdef CONFIG_GBM
         if (dmabuf) {
+            int fence_fd;
             egl_dmabuf_create_fence(dmabuf);
-            if (dmabuf->fence_fd > 0) {
-                qemu_set_fd_handler(dmabuf->fence_fd, gd_hw_gl_flushed, NULL, vc);
+            fence_fd = dpy_gl_qemu_dmabuf_get_fence_fd(dmabuf);
+            if (fence_fd > 0) {
+                qemu_set_fd_handler(fence_fd, gd_hw_gl_flushed, NULL, vc);
                 return;
             }
             graphic_hw_gl_block(vc->gfx.dcl.con, false);
@@ -125,7 +127,8 @@ void gd_gl_area_refresh(DisplayChangeListener *dcl)
 
     gd_update_monitor_refresh_rate(vc, vc->window ? vc->window : vc->gfx.drawing_area);
 
-    if (vc->gfx.guest_fb.dmabuf && vc->gfx.guest_fb.dmabuf->draw_submitted) {
+    if (vc->gfx.guest_fb.dmabuf &&
+        dpy_gl_qemu_dmabuf_get_draw_submitted(vc->gfx.guest_fb.dmabuf)) {
         return;
     }
 
@@ -285,7 +288,8 @@ void gd_gl_area_scanout_flush(DisplayChangeListener *dcl,
 {
     VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
 
-    if (vc->gfx.guest_fb.dmabuf && !vc->gfx.guest_fb.dmabuf->draw_submitted) {
+    if (vc->gfx.guest_fb.dmabuf &&
+        !dpy_gl_qemu_dmabuf_get_draw_submitted(vc->gfx.guest_fb.dmabuf)) {
         graphic_hw_gl_block(vc->gfx.dcl.con, true);
         vc->gfx.guest_fb.dmabuf->draw_submitted = true;
         gtk_gl_area_set_scanout_mode(vc, true);
@@ -298,20 +302,29 @@ void gd_gl_area_scanout_dmabuf(DisplayChangeListener *dcl,
 {
 #ifdef CONFIG_GBM
     VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
+    uint32_t x, y, width, height, backing_width, backing_height, texture;
+    bool y0_top;
 
     gtk_gl_area_make_current(GTK_GL_AREA(vc->gfx.drawing_area));
     egl_dmabuf_import_texture(dmabuf);
-    if (!dmabuf->texture) {
+    texture = dpy_gl_qemu_dmabuf_get_texture(dmabuf);
+    if (!texture) {
         return;
     }
 
-    gd_gl_area_scanout_texture(dcl, dmabuf->texture,
-                               dmabuf->y0_top,
-                               dmabuf->backing_width, dmabuf->backing_height,
-                               dmabuf->x, dmabuf->y, dmabuf->width,
-                               dmabuf->height, NULL);
+    x = dpy_gl_qemu_dmabuf_get_x(dmabuf);
+    y = dpy_gl_qemu_dmabuf_get_y(dmabuf);
+    width = dpy_gl_qemu_dmabuf_get_width(dmabuf);
+    height = dpy_gl_qemu_dmabuf_get_height(dmabuf);
+    backing_width = dpy_gl_qemu_dmabuf_get_backing_width(dmabuf);
+    backing_height = dpy_gl_qemu_dmabuf_get_backing_height(dmabuf);
+    y0_top = dpy_gl_qemu_dmabuf_get_y0_top(dmabuf);
 
-    if (dmabuf->allow_fences) {
+    gd_gl_area_scanout_texture(dcl, texture, y0_top,
+                               backing_width, backing_height,
+                               x, y, width, height, NULL);
+
+    if (dpy_gl_qemu_dmabuf_get_allow_fences(dmabuf)) {
         vc->gfx.guest_fb.dmabuf = dmabuf;
     }
 #endif
diff --git a/ui/gtk.c b/ui/gtk.c
index 810d7fc796..2c054a42ba 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -596,9 +596,11 @@ void gd_hw_gl_flushed(void *vcon)
 {
     VirtualConsole *vc = vcon;
     QemuDmaBuf *dmabuf = vc->gfx.guest_fb.dmabuf;
+    int fence_fd;
 
-    qemu_set_fd_handler(dmabuf->fence_fd, NULL, NULL, NULL);
-    close(dmabuf->fence_fd);
+    fence_fd = dpy_gl_qemu_dmabuf_get_fence_fd(dmabuf);
+    qemu_set_fd_handler(fence_fd, NULL, NULL, NULL);
+    close(fence_fd);
     dmabuf->fence_fd = -1;
     graphic_hw_gl_block(vc->gfx.dcl.con, false);
 }
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 6eb98a5a5c..1bbce4974b 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -976,6 +976,7 @@ static void qemu_spice_gl_cursor_dmabuf(DisplayChangeListener *dcl,
                                         uint32_t hot_x, uint32_t hot_y)
 {
     SimpleSpiceDisplay *ssd = container_of(dcl, SimpleSpiceDisplay, dcl);
+    uint32_t width, height, texture;
 
     ssd->have_hot = have_hot;
     ssd->hot_x = hot_x;
@@ -984,11 +985,13 @@ static void qemu_spice_gl_cursor_dmabuf(DisplayChangeListener *dcl,
     trace_qemu_spice_gl_cursor(ssd->qxl.id, dmabuf != NULL, have_hot);
     if (dmabuf) {
         egl_dmabuf_import_texture(dmabuf);
-        if (!dmabuf->texture) {
+        texture = dpy_gl_qemu_dmabuf_get_texture(dmabuf);
+        if (!texture) {
             return;
         }
-        egl_fb_setup_for_tex(&ssd->cursor_fb, dmabuf->width, dmabuf->height,
-                             dmabuf->texture, false);
+        width = dpy_gl_qemu_dmabuf_get_width(dmabuf);
+        height = dpy_gl_qemu_dmabuf_get_height(dmabuf);
+        egl_fb_setup_for_tex(&ssd->cursor_fb, width, height, texture, false);
     } else {
         egl_fb_destroy(&ssd->cursor_fb);
     }
@@ -1026,6 +1029,7 @@ static void qemu_spice_gl_update(DisplayChangeListener *dcl,
     bool y_0_top = false; /* FIXME */
     uint64_t cookie;
     int fd;
+    uint32_t width, height, texture;
 
     if (!ssd->have_scanout) {
         return;
@@ -1042,41 +1046,45 @@ static void qemu_spice_gl_update(DisplayChangeListener *dcl,
 
     if (ssd->guest_dmabuf_refresh) {
         QemuDmaBuf *dmabuf = ssd->guest_dmabuf;
+        width = dpy_gl_qemu_dmabuf_get_width(dmabuf);
+        height = dpy_gl_qemu_dmabuf_get_height(dmabuf);
+
         if (render_cursor) {
             egl_dmabuf_import_texture(dmabuf);
-            if (!dmabuf->texture) {
+            texture = dpy_gl_qemu_dmabuf_get_texture(dmabuf);
+            if (!texture) {
                 return;
             }
 
             /* source framebuffer */
-            egl_fb_setup_for_tex(&ssd->guest_fb,
-                                 dmabuf->width, dmabuf->height,
-                                 dmabuf->texture, false);
+            egl_fb_setup_for_tex(&ssd->guest_fb, width, height,
+                                 texture, false);
 
             /* dest framebuffer */
-            if (ssd->blit_fb.width  != dmabuf->width ||
-                ssd->blit_fb.height != dmabuf->height) {
-                trace_qemu_spice_gl_render_dmabuf(ssd->qxl.id, dmabuf->width,
-                                                  dmabuf->height);
+            if (ssd->blit_fb.width  != width ||
+                ssd->blit_fb.height != height) {
+                trace_qemu_spice_gl_render_dmabuf(ssd->qxl.id, width,
+                                                  height);
                 egl_fb_destroy(&ssd->blit_fb);
                 egl_fb_setup_new_tex(&ssd->blit_fb,
-                                     dmabuf->width, dmabuf->height);
+                                     width, height);
                 fd = egl_get_fd_for_texture(ssd->blit_fb.texture,
                                             &stride, &fourcc, NULL);
-                spice_qxl_gl_scanout(&ssd->qxl, fd,
-                                     dmabuf->width, dmabuf->height,
+                spice_qxl_gl_scanout(&ssd->qxl, fd, width, height,
                                      stride, fourcc, false);
             }
         } else {
-            trace_qemu_spice_gl_forward_dmabuf(ssd->qxl.id,
-                                               dmabuf->width, dmabuf->height);
+            stride = dpy_gl_qemu_dmabuf_get_stride(dmabuf);
+            fourcc = dpy_gl_qemu_dmabuf_get_fourcc(dmabuf);
+            y_0_top = dpy_gl_qemu_dmabuf_get_y0_top(dmabuf);
+            fd = dpy_gl_qemu_dmabuf_get_fd(dmabuf);
+
+            trace_qemu_spice_gl_forward_dmabuf(ssd->qxl.id, width, height);
             /* note: spice server will close the fd, so hand over a dup */
-            spice_qxl_gl_scanout(&ssd->qxl, dup(dmabuf->fd),
-                                 dmabuf->width, dmabuf->height,
-                                 dmabuf->stride, dmabuf->fourcc,
-                                 dmabuf->y0_top);
+            spice_qxl_gl_scanout(&ssd->qxl, dup(fd), width, height,
+                                 stride, fourcc, y_0_top);
         }
-        qemu_spice_gl_monitor_config(ssd, 0, 0, dmabuf->width, dmabuf->height);
+        qemu_spice_gl_monitor_config(ssd, 0, 0, width, height);
         ssd->guest_dmabuf_refresh = false;
     }
 
-- 
2.34.1

Re: [PATCH v6 1/3] ui/console: Introduce dpy_gl_qemu_dmabuf_get_..() helpers

[Marc-André Lureau]

Hi

On Wed, Apr 17, 2024 at 8:14 AM <dongwon.kim@intel.com> wrote:
>
> From: Dongwon Kim <dongwon.kim@intel.com>
>
> This commit introduces dpy_gl_qemu_dmabuf_get_... helpers to extract
> specific fields from the QemuDmaBuf struct. It also updates all instances
> where fields within the QemuDmaBuf struct are directly accessed, replacing
> them with calls to these new helper functions.
>
> v6: fix typos in helper names in ui/spice-display.c
>
> Suggested-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
> Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
> Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
> ---
>  include/ui/console.h            |  17 +++++
>  hw/display/vhost-user-gpu.c     |   6 +-
>  hw/display/virtio-gpu-udmabuf.c |   7 +-
>  hw/vfio/display.c               |  15 +++--
>  ui/console.c                    | 116 +++++++++++++++++++++++++++++++-
>  ui/dbus-console.c               |   9 ++-
>  ui/dbus-listener.c              |  43 +++++++-----
>  ui/egl-headless.c               |  23 +++++--
>  ui/egl-helpers.c                |  47 +++++++------
>  ui/gtk-egl.c                    |  48 ++++++++-----
>  ui/gtk-gl-area.c                |  37 ++++++----
>  ui/gtk.c                        |   6 +-
>  ui/spice-display.c              |  50 ++++++++------
>  13 files changed, 316 insertions(+), 108 deletions(-)
>
> diff --git a/include/ui/console.h b/include/ui/console.h
> index 0bc7a00ac0..6292943a82 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -358,6 +358,23 @@ void dpy_gl_cursor_dmabuf(QemuConsole *con, QemuDmaBuf *dmabuf,
>                            bool have_hot, uint32_t hot_x, uint32_t hot_y);
>  void dpy_gl_cursor_position(QemuConsole *con,
>                              uint32_t pos_x, uint32_t pos_y);
> +
> +int32_t dpy_gl_qemu_dmabuf_get_fd(QemuDmaBuf *dmabuf);
> +uint32_t dpy_gl_qemu_dmabuf_get_width(QemuDmaBuf *dmabuf);
> +uint32_t dpy_gl_qemu_dmabuf_get_height(QemuDmaBuf *dmabuf);
> +uint32_t dpy_gl_qemu_dmabuf_get_stride(QemuDmaBuf *dmabuf);
> +uint32_t dpy_gl_qemu_dmabuf_get_fourcc(QemuDmaBuf *dmabuf);
> +uint64_t dpy_gl_qemu_dmabuf_get_modifier(QemuDmaBuf *dmabuf);
> +uint32_t dpy_gl_qemu_dmabuf_get_texture(QemuDmaBuf *dmabuf);
> +uint32_t dpy_gl_qemu_dmabuf_get_x(QemuDmaBuf *dmabuf);
> +uint32_t dpy_gl_qemu_dmabuf_get_y(QemuDmaBuf *dmabuf);
> +uint32_t dpy_gl_qemu_dmabuf_get_backing_width(QemuDmaBuf *dmabuf);
> +uint32_t dpy_gl_qemu_dmabuf_get_backing_height(QemuDmaBuf *dmabuf);
> +bool dpy_gl_qemu_dmabuf_get_y0_top(QemuDmaBuf *dmabuf);
> +void *dpy_gl_qemu_dmabuf_get_sync(QemuDmaBuf *dmabuf);
> +int32_t dpy_gl_qemu_dmabuf_get_fence_fd(QemuDmaBuf *dmabuf);
> +bool dpy_gl_qemu_dmabuf_get_allow_fences(QemuDmaBuf *dmabuf);
> +bool dpy_gl_qemu_dmabuf_get_draw_submitted(QemuDmaBuf *dmabuf);
>  void dpy_gl_release_dmabuf(QemuConsole *con,
>                             QemuDmaBuf *dmabuf);
>  void dpy_gl_update(QemuConsole *con,
> diff --git a/hw/display/vhost-user-gpu.c b/hw/display/vhost-user-gpu.c
> index 709c8a02a1..87dcfbca10 100644
> --- a/hw/display/vhost-user-gpu.c
> +++ b/hw/display/vhost-user-gpu.c
> @@ -249,6 +249,7 @@ vhost_user_gpu_handle_display(VhostUserGPU *g, VhostUserGpuMsg *msg)
>      case VHOST_USER_GPU_DMABUF_SCANOUT: {
>          VhostUserGpuDMABUFScanout *m = &msg->payload.dmabuf_scanout;
>          int fd = qemu_chr_fe_get_msgfd(&g->vhost_chr);
> +        int old_fd;
>          QemuDmaBuf *dmabuf;
>
>          if (m->scanout_id >= g->parent_obj.conf.max_outputs) {
> @@ -262,8 +263,9 @@ vhost_user_gpu_handle_display(VhostUserGPU *g, VhostUserGpuMsg *msg)
>          g->parent_obj.enable = 1;
>          con = g->parent_obj.scanout[m->scanout_id].con;
>          dmabuf = &g->dmabuf[m->scanout_id];
> -        if (dmabuf->fd >= 0) {
> -            close(dmabuf->fd);
> +        old_fd = dpy_gl_qemu_dmabuf_get_fd(dmabuf);
> +        if (old_fd >= 0) {
> +            close(old_fd);
>              dmabuf->fd = -1;
>          }
>          dpy_gl_release_dmabuf(con, dmabuf);
> diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
> index d51184d658..e3f358b575 100644
> --- a/hw/display/virtio-gpu-udmabuf.c
> +++ b/hw/display/virtio-gpu-udmabuf.c
> @@ -206,6 +206,7 @@ int virtio_gpu_update_dmabuf(VirtIOGPU *g,
>  {
>      struct virtio_gpu_scanout *scanout = &g->parent_obj.scanout[scanout_id];
>      VGPUDMABuf *new_primary, *old_primary = NULL;
> +    uint32_t width, height;
>
>      new_primary = virtio_gpu_create_dmabuf(g, scanout_id, res, fb, r);
>      if (!new_primary) {
> @@ -216,10 +217,10 @@ int virtio_gpu_update_dmabuf(VirtIOGPU *g,
>          old_primary = g->dmabuf.primary[scanout_id];
>      }
>
> +    width = dpy_gl_qemu_dmabuf_get_width(&new_primary->buf);
> +    height = dpy_gl_qemu_dmabuf_get_height(&new_primary->buf);
>      g->dmabuf.primary[scanout_id] = new_primary;
> -    qemu_console_resize(scanout->con,
> -                        new_primary->buf.width,
> -                        new_primary->buf.height);
> +    qemu_console_resize(scanout->con, width, height);
>      dpy_gl_scanout_dmabuf(scanout->con, &new_primary->buf);
>
>      if (old_primary) {
> diff --git a/hw/vfio/display.c b/hw/vfio/display.c
> index 1aa440c663..f9c39cbd51 100644
> --- a/hw/vfio/display.c
> +++ b/hw/vfio/display.c
> @@ -259,9 +259,13 @@ static VFIODMABuf *vfio_display_get_dmabuf(VFIOPCIDevice *vdev,
>
>  static void vfio_display_free_one_dmabuf(VFIODisplay *dpy, VFIODMABuf *dmabuf)
>  {
> +    int fd;
> +
>      QTAILQ_REMOVE(&dpy->dmabuf.bufs, dmabuf, next);
> +
> +    fd = dpy_gl_qemu_dmabuf_get_fd(&dmabuf->buf);
>      dpy_gl_release_dmabuf(dpy->con, &dmabuf->buf);
> -    close(dmabuf->buf.fd);
> +    close(fd);
>      g_free(dmabuf);
>  }
>
> @@ -286,6 +290,7 @@ static void vfio_display_dmabuf_update(void *opaque)
>      VFIOPCIDevice *vdev = opaque;
>      VFIODisplay *dpy = vdev->dpy;
>      VFIODMABuf *primary, *cursor;
> +    uint32_t width, height;
>      bool free_bufs = false, new_cursor = false;
>
>      primary = vfio_display_get_dmabuf(vdev, DRM_PLANE_TYPE_PRIMARY);
> @@ -296,10 +301,12 @@ static void vfio_display_dmabuf_update(void *opaque)
>          return;
>      }
>
> +    width = dpy_gl_qemu_dmabuf_get_width(&primary->buf);
> +    height = dpy_gl_qemu_dmabuf_get_height(&primary->buf);
> +
>      if (dpy->dmabuf.primary != primary) {
>          dpy->dmabuf.primary = primary;
> -        qemu_console_resize(dpy->con,
> -                            primary->buf.width, primary->buf.height);
> +        qemu_console_resize(dpy->con, width, height);
>          dpy_gl_scanout_dmabuf(dpy->con, &primary->buf);
>          free_bufs = true;
>      }
> @@ -328,7 +335,7 @@ static void vfio_display_dmabuf_update(void *opaque)
>          cursor->pos_updates = 0;
>      }
>
> -    dpy_gl_update(dpy->con, 0, 0, primary->buf.width, primary->buf.height);
> +    dpy_gl_update(dpy->con, 0, 0, width, height);
>
>      if (free_bufs) {
>          vfio_display_free_dmabufs(vdev);
> diff --git a/ui/console.c b/ui/console.c
> index 43226c5c14..5d5635f783 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -1132,6 +1132,118 @@ void dpy_gl_cursor_position(QemuConsole *con,
>      }
>  }
>
> +int32_t dpy_gl_qemu_dmabuf_get_fd(QemuDmaBuf *dmabuf)
> +{
> +    assert(dmabuf != NULL);
> +
> +    return dmabuf->fd;
> +}
> +
> +uint32_t dpy_gl_qemu_dmabuf_get_width(QemuDmaBuf *dmabuf)
> +{
> +    assert(dmabuf != NULL);
> +
> +    return dmabuf->width;
> +}
> +
> +uint32_t dpy_gl_qemu_dmabuf_get_height(QemuDmaBuf *dmabuf)
> +{
> +    assert(dmabuf != NULL);
> +
> +    return dmabuf->height;
> +}
> +
> +uint32_t dpy_gl_qemu_dmabuf_get_stride(QemuDmaBuf *dmabuf)
> +{
> +    assert(dmabuf != NULL);
> +
> +    return dmabuf->stride;
> +}
> +
> +uint32_t dpy_gl_qemu_dmabuf_get_fourcc(QemuDmaBuf *dmabuf)
> +{
> +    assert(dmabuf != NULL);
> +
> +    return dmabuf->fourcc;
> +}
> +
> +uint64_t dpy_gl_qemu_dmabuf_get_modifier(QemuDmaBuf *dmabuf)
> +{
> +    assert(dmabuf != NULL);
> +
> +    return dmabuf->modifier;
> +}
> +
> +uint32_t dpy_gl_qemu_dmabuf_get_texture(QemuDmaBuf *dmabuf)
> +{
> +    assert(dmabuf != NULL);
> +
> +    return dmabuf->texture;
> +}
> +
> +uint32_t dpy_gl_qemu_dmabuf_get_x(QemuDmaBuf *dmabuf)
> +{
> +    assert(dmabuf != NULL);
> +
> +    return dmabuf->x;
> +}
> +
> +uint32_t dpy_gl_qemu_dmabuf_get_y(QemuDmaBuf *dmabuf)
> +{
> +    assert(dmabuf != NULL);
> +
> +    return dmabuf->y;
> +}
> +
> +uint32_t dpy_gl_qemu_dmabuf_get_backing_width(QemuDmaBuf *dmabuf)
> +{
> +    assert(dmabuf != NULL);
> +
> +    return dmabuf->backing_width;
> +}
> +
> +uint32_t dpy_gl_qemu_dmabuf_get_backing_height(QemuDmaBuf *dmabuf)
> +{
> +    assert(dmabuf != NULL);
> +
> +    return dmabuf->backing_height;
> +}
> +
> +bool dpy_gl_qemu_dmabuf_get_y0_top(QemuDmaBuf *dmabuf)
> +{
> +    assert(dmabuf != NULL);
> +
> +    return dmabuf->y0_top;
> +}
> +
> +void *dpy_gl_qemu_dmabuf_get_sync(QemuDmaBuf *dmabuf)
> +{
> +    assert(dmabuf != NULL);
> +
> +    return dmabuf->sync;
> +}
> +
> +int32_t dpy_gl_qemu_dmabuf_get_fence_fd(QemuDmaBuf *dmabuf)
> +{
> +    assert(dmabuf != NULL);
> +
> +    return dmabuf->fence_fd;
> +}
> +
> +bool dpy_gl_qemu_dmabuf_get_allow_fences(QemuDmaBuf *dmabuf)
> +{
> +    assert(dmabuf != NULL);
> +
> +    return dmabuf->allow_fences;
> +}
> +
> +bool dpy_gl_qemu_dmabuf_get_draw_submitted(QemuDmaBuf *dmabuf)
> +{
> +    assert(dmabuf != NULL);
> +
> +    return dmabuf->draw_submitted;
> +}
> +
>  void dpy_gl_release_dmabuf(QemuConsole *con,
>                            QemuDmaBuf *dmabuf)
>  {
> @@ -1459,7 +1571,7 @@ int qemu_console_get_width(QemuConsole *con, int fallback)
>      }
>      switch (con->scanout.kind) {
>      case SCANOUT_DMABUF:
> -        return con->scanout.dmabuf->width;
> +        return dpy_gl_qemu_dmabuf_get_width(con->scanout.dmabuf);
>      case SCANOUT_TEXTURE:
>          return con->scanout.texture.width;
>      case SCANOUT_SURFACE:
> @@ -1476,7 +1588,7 @@ int qemu_console_get_height(QemuConsole *con, int fallback)
>      }
>      switch (con->scanout.kind) {
>      case SCANOUT_DMABUF:
> -        return con->scanout.dmabuf->height;
> +        return dpy_gl_qemu_dmabuf_get_height(con->scanout.dmabuf);
>      case SCANOUT_TEXTURE:
>          return con->scanout.texture.height;
>      case SCANOUT_SURFACE:
> diff --git a/ui/dbus-console.c b/ui/dbus-console.c
> index 49da9ccc83..124fe16253 100644
> --- a/ui/dbus-console.c
> +++ b/ui/dbus-console.c
> @@ -110,11 +110,14 @@ static void
>  dbus_gl_scanout_dmabuf(DisplayChangeListener *dcl,
>                         QemuDmaBuf *dmabuf)
>  {
> +    uint32_t width, height;
> +
>      DBusDisplayConsole *ddc = container_of(dcl, DBusDisplayConsole, dcl);
>
> -    dbus_display_console_set_size(ddc,
> -                                  dmabuf->width,
> -                                  dmabuf->height);
> +    width = dpy_gl_qemu_dmabuf_get_width(dmabuf);
> +    height = dpy_gl_qemu_dmabuf_get_height(dmabuf);
> +
> +    dbus_display_console_set_size(ddc, width, height);
>  }
>
>  static void
> diff --git a/ui/dbus-listener.c b/ui/dbus-listener.c
> index 4a0a5d78f9..c6c7d93753 100644
> --- a/ui/dbus-listener.c
> +++ b/ui/dbus-listener.c
> @@ -278,29 +278,33 @@ static void dbus_scanout_dmabuf(DisplayChangeListener *dcl,
>      DBusDisplayListener *ddl = container_of(dcl, DBusDisplayListener, dcl);
>      g_autoptr(GError) err = NULL;
>      g_autoptr(GUnixFDList) fd_list = NULL;
> +    int fd;
> +    uint32_t width, height, stride, fourcc;
> +    uint64_t modifier;
> +    bool y0_top;
>
> +    fd = dpy_gl_qemu_dmabuf_get_fd(dmabuf);
>      fd_list = g_unix_fd_list_new();
> -    if (g_unix_fd_list_append(fd_list, dmabuf->fd, &err) != 0) {
> +    if (g_unix_fd_list_append(fd_list, fd, &err) != 0) {
>          error_report("Failed to setup dmabuf fdlist: %s", err->message);
>          return;
>      }
>
>      ddl_discard_pending_messages(ddl);
>
> +    width = dpy_gl_qemu_dmabuf_get_width(dmabuf);
> +    height = dpy_gl_qemu_dmabuf_get_height(dmabuf);
> +    stride = dpy_gl_qemu_dmabuf_get_stride(dmabuf);
> +    fourcc = dpy_gl_qemu_dmabuf_get_fourcc(dmabuf);
> +    modifier = dpy_gl_qemu_dmabuf_get_modifier(dmabuf);
> +    y0_top = dpy_gl_qemu_dmabuf_get_y0_top(dmabuf);
> +
>      /* FIXME: add missing x/y/w/h support */
>      qemu_dbus_display1_listener_call_scanout_dmabuf(
> -        ddl->proxy,
> -        g_variant_new_handle(0),
> -        dmabuf->width,
> -        dmabuf->height,
> -        dmabuf->stride,
> -        dmabuf->fourcc,
> -        dmabuf->modifier,
> -        dmabuf->y0_top,
> -        G_DBUS_CALL_FLAGS_NONE,
> -        -1,
> -        fd_list,
> -        NULL, NULL, NULL);
> +        ddl->proxy, g_variant_new_handle(0),
> +        width, height, stride, fourcc, modifier,
> +        y0_top, G_DBUS_CALL_FLAGS_NONE,
> +        -1, fd_list, NULL, NULL, NULL);
>  }
>  #endif /* GBM */
>  #endif /* OPENGL */
> @@ -488,6 +492,7 @@ static void dbus_cursor_dmabuf(DisplayChangeListener *dcl,
>      DisplaySurface *ds;
>      GVariant *v_data = NULL;
>      egl_fb cursor_fb = EGL_FB_INIT;
> +    uint32_t width, height, texture;
>
>      if (!dmabuf) {
>          qemu_dbus_display1_listener_call_mouse_set(
> @@ -497,12 +502,16 @@ static void dbus_cursor_dmabuf(DisplayChangeListener *dcl,
>      }
>
>      egl_dmabuf_import_texture(dmabuf);
> -    if (!dmabuf->texture) {
> +    texture = dpy_gl_qemu_dmabuf_get_texture(dmabuf);
> +    if (!texture) {
>          return;
>      }
> -    egl_fb_setup_for_tex(&cursor_fb, dmabuf->width, dmabuf->height,
> -                         dmabuf->texture, false);
> -    ds = qemu_create_displaysurface(dmabuf->width, dmabuf->height);
> +
> +    width = dpy_gl_qemu_dmabuf_get_width(dmabuf);
> +    height = dpy_gl_qemu_dmabuf_get_height(dmabuf);
> +
> +    egl_fb_setup_for_tex(&cursor_fb, width, height, texture, false);
> +    ds = qemu_create_displaysurface(width, height);
>      egl_fb_read(ds, &cursor_fb);
>
>      v_data = g_variant_new_from_data(
> diff --git a/ui/egl-headless.c b/ui/egl-headless.c
> index d5637dadb2..158924379b 100644
> --- a/ui/egl-headless.c
> +++ b/ui/egl-headless.c
> @@ -85,29 +85,38 @@ static void egl_scanout_texture(DisplayChangeListener *dcl,
>  static void egl_scanout_dmabuf(DisplayChangeListener *dcl,
>                                 QemuDmaBuf *dmabuf)
>  {
> +    uint32_t width, height, texture;
> +
>      egl_dmabuf_import_texture(dmabuf);
> -    if (!dmabuf->texture) {
> +    texture = dpy_gl_qemu_dmabuf_get_texture(dmabuf);
> +    if (!texture) {
>          return;
>      }
>
> -    egl_scanout_texture(dcl, dmabuf->texture,
> -                        false, dmabuf->width, dmabuf->height,
> -                        0, 0, dmabuf->width, dmabuf->height, NULL);
> +    width = dpy_gl_qemu_dmabuf_get_width(dmabuf);
> +    height = dpy_gl_qemu_dmabuf_get_height(dmabuf);
> +
> +    egl_scanout_texture(dcl, texture, false, width, height, 0, 0,
> +                        width, height, NULL);
>  }
>
>  static void egl_cursor_dmabuf(DisplayChangeListener *dcl,
>                                QemuDmaBuf *dmabuf, bool have_hot,
>                                uint32_t hot_x, uint32_t hot_y)
>  {
> +    uint32_t width, height, texture;
>      egl_dpy *edpy = container_of(dcl, egl_dpy, dcl);
>
>      if (dmabuf) {
>          egl_dmabuf_import_texture(dmabuf);
> -        if (!dmabuf->texture) {
> +        texture = dpy_gl_qemu_dmabuf_get_texture(dmabuf);
> +        if (!texture) {
>              return;
>          }
> -        egl_fb_setup_for_tex(&edpy->cursor_fb, dmabuf->width, dmabuf->height,
> -                             dmabuf->texture, false);
> +
> +        width = dpy_gl_qemu_dmabuf_get_width(dmabuf);
> +        height = dpy_gl_qemu_dmabuf_get_height(dmabuf);
> +        egl_fb_setup_for_tex(&edpy->cursor_fb, width, height, texture, false);
>      } else {
>          egl_fb_destroy(&edpy->cursor_fb);
>      }
> diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
> index 3d19dbe382..86d64c68ce 100644
> --- a/ui/egl-helpers.c
> +++ b/ui/egl-helpers.c
> @@ -146,10 +146,10 @@ void egl_fb_blit(egl_fb *dst, egl_fb *src, bool flip)
>      glViewport(0, 0, dst->width, dst->height);
>
>      if (src->dmabuf) {
> -        x1 = src->dmabuf->x;
> -        y1 = src->dmabuf->y;
> -        w = src->dmabuf->width;
> -        h = src->dmabuf->height;
> +        x1 = dpy_gl_qemu_dmabuf_get_x(src->dmabuf);
> +        y1 = dpy_gl_qemu_dmabuf_get_y(src->dmabuf);
> +        w = dpy_gl_qemu_dmabuf_get_width(src->dmabuf);
> +        h = dpy_gl_qemu_dmabuf_get_height(src->dmabuf);
>      }
>
>      w = (x1 + w) > src->width ? src->width - x1 : w;
> @@ -308,30 +308,33 @@ void egl_dmabuf_import_texture(QemuDmaBuf *dmabuf)
>      EGLImageKHR image = EGL_NO_IMAGE_KHR;
>      EGLint attrs[64];
>      int i = 0;
> +    uint64_t modifier;
> +    uint32_t texture = dpy_gl_qemu_dmabuf_get_texture(dmabuf);
>
> -    if (dmabuf->texture != 0) {
> +    if (texture != 0) {
>          return;
>      }
>
>      attrs[i++] = EGL_WIDTH;
> -    attrs[i++] = dmabuf->backing_width;
> +    attrs[i++] = dpy_gl_qemu_dmabuf_get_backing_width(dmabuf);
>      attrs[i++] = EGL_HEIGHT;
> -    attrs[i++] = dmabuf->backing_height;
> +    attrs[i++] = dpy_gl_qemu_dmabuf_get_backing_height(dmabuf);
>      attrs[i++] = EGL_LINUX_DRM_FOURCC_EXT;
> -    attrs[i++] = dmabuf->fourcc;
> +    attrs[i++] = dpy_gl_qemu_dmabuf_get_fourcc(dmabuf);
>
>      attrs[i++] = EGL_DMA_BUF_PLANE0_FD_EXT;
> -    attrs[i++] = dmabuf->fd;
> +    attrs[i++] = dpy_gl_qemu_dmabuf_get_fd(dmabuf);
>      attrs[i++] = EGL_DMA_BUF_PLANE0_PITCH_EXT;
> -    attrs[i++] = dmabuf->stride;
> +    attrs[i++] = dpy_gl_qemu_dmabuf_get_stride(dmabuf);
>      attrs[i++] = EGL_DMA_BUF_PLANE0_OFFSET_EXT;
>      attrs[i++] = 0;
>  #ifdef EGL_DMA_BUF_PLANE0_MODIFIER_LO_EXT
> -    if (dmabuf->modifier) {
> +    modifier = dpy_gl_qemu_dmabuf_get_modifier(dmabuf);
> +    if (modifier) {
>          attrs[i++] = EGL_DMA_BUF_PLANE0_MODIFIER_LO_EXT;
> -        attrs[i++] = (dmabuf->modifier >>  0) & 0xffffffff;
> +        attrs[i++] = (modifier >>  0) & 0xffffffff;
>          attrs[i++] = EGL_DMA_BUF_PLANE0_MODIFIER_HI_EXT;
> -        attrs[i++] = (dmabuf->modifier >> 32) & 0xffffffff;
> +        attrs[i++] = (modifier >> 32) & 0xffffffff;
>      }
>  #endif
>      attrs[i++] = EGL_NONE;
> @@ -346,7 +349,8 @@ void egl_dmabuf_import_texture(QemuDmaBuf *dmabuf)
>      }
>
>      glGenTextures(1, &dmabuf->texture);
> -    glBindTexture(GL_TEXTURE_2D, dmabuf->texture);
> +    texture = dpy_gl_qemu_dmabuf_get_texture(dmabuf);
> +    glBindTexture(GL_TEXTURE_2D, texture);
>      glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
>      glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
>
> @@ -356,11 +360,14 @@ void egl_dmabuf_import_texture(QemuDmaBuf *dmabuf)
>
>  void egl_dmabuf_release_texture(QemuDmaBuf *dmabuf)
>  {
> -    if (dmabuf->texture == 0) {
> +    uint32_t texture;
> +
> +    texture = dpy_gl_qemu_dmabuf_get_texture(dmabuf);
> +    if (texture == 0) {
>          return;
>      }
>
> -    glDeleteTextures(1, &dmabuf->texture);
> +    glDeleteTextures(1, &texture);
>      dmabuf->texture = 0;
>  }
>
> @@ -382,10 +389,12 @@ void egl_dmabuf_create_sync(QemuDmaBuf *dmabuf)
>
>  void egl_dmabuf_create_fence(QemuDmaBuf *dmabuf)
>  {
> -    if (dmabuf->sync) {
> +    void *sync = dpy_gl_qemu_dmabuf_get_sync(dmabuf);
> +
> +    if (sync) {
>          dmabuf->fence_fd = eglDupNativeFenceFDANDROID(qemu_egl_display,
> -                                                      dmabuf->sync);
> -        eglDestroySyncKHR(qemu_egl_display, dmabuf->sync);
> +                                                      sync);
> +        eglDestroySyncKHR(qemu_egl_display, sync);
>          dmabuf->sync = NULL;
>      }
>  }
> diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
> index 3af5ac5bcf..c9469af9ed 100644
> --- a/ui/gtk-egl.c
> +++ b/ui/gtk-egl.c
> @@ -70,6 +70,7 @@ void gd_egl_draw(VirtualConsole *vc)
>      QemuDmaBuf *dmabuf = vc->gfx.guest_fb.dmabuf;
>  #endif
>      int ww, wh, ws;
> +    int fence_fd;
>
>      if (!vc->gfx.gls) {
>          return;
> @@ -83,7 +84,7 @@ void gd_egl_draw(VirtualConsole *vc)
>      if (vc->gfx.scanout_mode) {
>  #ifdef CONFIG_GBM
>          if (dmabuf) {
> -            if (!dmabuf->draw_submitted) {
> +            if (!dpy_gl_qemu_dmabuf_get_draw_submitted(dmabuf)) {
>                  return;
>              } else {
>                  dmabuf->draw_submitted = false;
> @@ -99,8 +100,9 @@ void gd_egl_draw(VirtualConsole *vc)
>  #ifdef CONFIG_GBM
>          if (dmabuf) {
>              egl_dmabuf_create_fence(dmabuf);
> -            if (dmabuf->fence_fd > 0) {

>= I thought this was already fixed.

> -                qemu_set_fd_handler(dmabuf->fence_fd, gd_hw_gl_flushed, NULL, vc);
> +            fence_fd = dpy_gl_qemu_dmabuf_get_fence_fd(dmabuf);
> +            if (fence_fd > 0) {

same

> +                qemu_set_fd_handler(fence_fd, gd_hw_gl_flushed, NULL, vc);
>                  return;
>              }
>              graphic_hw_gl_block(vc->gfx.dcl.con, false);
> @@ -149,7 +151,8 @@ void gd_egl_refresh(DisplayChangeListener *dcl)
>      gd_update_monitor_refresh_rate(
>              vc, vc->window ? vc->window : vc->gfx.drawing_area);
>
> -    if (vc->gfx.guest_fb.dmabuf && vc->gfx.guest_fb.dmabuf->draw_submitted) {
> +    if (vc->gfx.guest_fb.dmabuf &&
> +        dpy_gl_qemu_dmabuf_get_draw_submitted(vc->gfx.guest_fb.dmabuf)) {
>          return;
>      }
>
> @@ -264,22 +267,30 @@ void gd_egl_scanout_dmabuf(DisplayChangeListener *dcl,
>  {
>  #ifdef CONFIG_GBM
>      VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
> +    uint32_t x, y, width, height, backing_width, backing_height, texture;
> +    bool y0_top;
>
>      eglMakeCurrent(qemu_egl_display, vc->gfx.esurface,
>                     vc->gfx.esurface, vc->gfx.ectx);
>
>      egl_dmabuf_import_texture(dmabuf);
> -    if (!dmabuf->texture) {
> +    texture = dpy_gl_qemu_dmabuf_get_texture(dmabuf);
> +    if (!texture) {
>          return;
>      }
>
> -    gd_egl_scanout_texture(dcl, dmabuf->texture,
> -                           dmabuf->y0_top,
> -                           dmabuf->backing_width, dmabuf->backing_height,
> -                           dmabuf->x, dmabuf->y, dmabuf->width,
> -                           dmabuf->height, NULL);
> +    x = dpy_gl_qemu_dmabuf_get_x(dmabuf);
> +    y = dpy_gl_qemu_dmabuf_get_y(dmabuf);
> +    width = dpy_gl_qemu_dmabuf_get_width(dmabuf);
> +    height = dpy_gl_qemu_dmabuf_get_height(dmabuf);
> +    backing_width = dpy_gl_qemu_dmabuf_get_backing_width(dmabuf);
> +    backing_height = dpy_gl_qemu_dmabuf_get_backing_height(dmabuf);
> +    y0_top = dpy_gl_qemu_dmabuf_get_y0_top(dmabuf);
>
> -    if (dmabuf->allow_fences) {
> +    gd_egl_scanout_texture(dcl, texture, y0_top, backing_width, backing_height,
> +                           x, y, width, height, NULL);
> +
> +    if (dpy_gl_qemu_dmabuf_get_allow_fences(dmabuf)) {
>          vc->gfx.guest_fb.dmabuf = dmabuf;
>      }
>  #endif
> @@ -291,15 +302,19 @@ void gd_egl_cursor_dmabuf(DisplayChangeListener *dcl,
>  {
>  #ifdef CONFIG_GBM
>      VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
> +    uint32_t backing_width, backing_height, texture;
>
>      if (dmabuf) {
>          egl_dmabuf_import_texture(dmabuf);
> -        if (!dmabuf->texture) {
> +        texture = dpy_gl_qemu_dmabuf_get_texture(dmabuf);
> +        if (!texture) {
>              return;
>          }
> -        egl_fb_setup_for_tex(&vc->gfx.cursor_fb,
> -                             dmabuf->backing_width, dmabuf->backing_height,
> -                             dmabuf->texture, false);
> +
> +        backing_width = dpy_gl_qemu_dmabuf_get_backing_width(dmabuf);
> +        backing_height = dpy_gl_qemu_dmabuf_get_backing_height(dmabuf);
> +        egl_fb_setup_for_tex(&vc->gfx.cursor_fb, backing_width, backing_height,
> +                             texture, false);
>      } else {
>          egl_fb_destroy(&vc->gfx.cursor_fb);
>      }
> @@ -363,7 +378,8 @@ void gd_egl_flush(DisplayChangeListener *dcl,
>      VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
>      GtkWidget *area = vc->gfx.drawing_area;
>
> -    if (vc->gfx.guest_fb.dmabuf && !vc->gfx.guest_fb.dmabuf->draw_submitted) {
> +    if (vc->gfx.guest_fb.dmabuf &&
> +        !dpy_gl_qemu_dmabuf_get_draw_submitted(vc->gfx.guest_fb.dmabuf)) {
>          graphic_hw_gl_block(vc->gfx.dcl.con, true);
>          vc->gfx.guest_fb.dmabuf->draw_submitted = true;
>          gtk_egl_set_scanout_mode(vc, true);
> diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
> index 52dcac161e..193862ecc2 100644
> --- a/ui/gtk-gl-area.c
> +++ b/ui/gtk-gl-area.c
> @@ -60,7 +60,7 @@ void gd_gl_area_draw(VirtualConsole *vc)
>
>  #ifdef CONFIG_GBM
>          if (dmabuf) {
> -            if (!dmabuf->draw_submitted) {
> +            if (!dpy_gl_qemu_dmabuf_get_draw_submitted(dmabuf)) {
>                  return;
>              } else {
>                  dmabuf->draw_submitted = false;
> @@ -85,9 +85,11 @@ void gd_gl_area_draw(VirtualConsole *vc)
>          glFlush();
>  #ifdef CONFIG_GBM
>          if (dmabuf) {
> +            int fence_fd;
>              egl_dmabuf_create_fence(dmabuf);
> -            if (dmabuf->fence_fd > 0) {
> -                qemu_set_fd_handler(dmabuf->fence_fd, gd_hw_gl_flushed, NULL, vc);
> +            fence_fd = dpy_gl_qemu_dmabuf_get_fence_fd(dmabuf);
> +            if (fence_fd > 0) {

same

> +                qemu_set_fd_handler(fence_fd, gd_hw_gl_flushed, NULL, vc);
>                  return;
>              }
>              graphic_hw_gl_block(vc->gfx.dcl.con, false);
> @@ -125,7 +127,8 @@ void gd_gl_area_refresh(DisplayChangeListener *dcl)
>
>      gd_update_monitor_refresh_rate(vc, vc->window ? vc->window : vc->gfx.drawing_area);
>
> -    if (vc->gfx.guest_fb.dmabuf && vc->gfx.guest_fb.dmabuf->draw_submitted) {
> +    if (vc->gfx.guest_fb.dmabuf &&
> +        dpy_gl_qemu_dmabuf_get_draw_submitted(vc->gfx.guest_fb.dmabuf)) {
>          return;
>      }
>
> @@ -285,7 +288,8 @@ void gd_gl_area_scanout_flush(DisplayChangeListener *dcl,
>  {
>      VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
>
> -    if (vc->gfx.guest_fb.dmabuf && !vc->gfx.guest_fb.dmabuf->draw_submitted) {
> +    if (vc->gfx.guest_fb.dmabuf &&
> +        !dpy_gl_qemu_dmabuf_get_draw_submitted(vc->gfx.guest_fb.dmabuf)) {
>          graphic_hw_gl_block(vc->gfx.dcl.con, true);
>          vc->gfx.guest_fb.dmabuf->draw_submitted = true;
>          gtk_gl_area_set_scanout_mode(vc, true);
> @@ -298,20 +302,29 @@ void gd_gl_area_scanout_dmabuf(DisplayChangeListener *dcl,
>  {
>  #ifdef CONFIG_GBM
>      VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
> +    uint32_t x, y, width, height, backing_width, backing_height, texture;
> +    bool y0_top;
>
>      gtk_gl_area_make_current(GTK_GL_AREA(vc->gfx.drawing_area));
>      egl_dmabuf_import_texture(dmabuf);
> -    if (!dmabuf->texture) {
> +    texture = dpy_gl_qemu_dmabuf_get_texture(dmabuf);
> +    if (!texture) {
>          return;
>      }
>
> -    gd_gl_area_scanout_texture(dcl, dmabuf->texture,
> -                               dmabuf->y0_top,
> -                               dmabuf->backing_width, dmabuf->backing_height,
> -                               dmabuf->x, dmabuf->y, dmabuf->width,
> -                               dmabuf->height, NULL);
> +    x = dpy_gl_qemu_dmabuf_get_x(dmabuf);
> +    y = dpy_gl_qemu_dmabuf_get_y(dmabuf);
> +    width = dpy_gl_qemu_dmabuf_get_width(dmabuf);
> +    height = dpy_gl_qemu_dmabuf_get_height(dmabuf);
> +    backing_width = dpy_gl_qemu_dmabuf_get_backing_width(dmabuf);
> +    backing_height = dpy_gl_qemu_dmabuf_get_backing_height(dmabuf);
> +    y0_top = dpy_gl_qemu_dmabuf_get_y0_top(dmabuf);
>
> -    if (dmabuf->allow_fences) {
> +    gd_gl_area_scanout_texture(dcl, texture, y0_top,
> +                               backing_width, backing_height,
> +                               x, y, width, height, NULL);
> +
> +    if (dpy_gl_qemu_dmabuf_get_allow_fences(dmabuf)) {
>          vc->gfx.guest_fb.dmabuf = dmabuf;
>      }
>  #endif
> diff --git a/ui/gtk.c b/ui/gtk.c
> index 810d7fc796..2c054a42ba 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -596,9 +596,11 @@ void gd_hw_gl_flushed(void *vcon)
>  {
>      VirtualConsole *vc = vcon;
>      QemuDmaBuf *dmabuf = vc->gfx.guest_fb.dmabuf;
> +    int fence_fd;
>
> -    qemu_set_fd_handler(dmabuf->fence_fd, NULL, NULL, NULL);
> -    close(dmabuf->fence_fd);
> +    fence_fd = dpy_gl_qemu_dmabuf_get_fence_fd(dmabuf);
> +    qemu_set_fd_handler(fence_fd, NULL, NULL, NULL);

and better check if fence_fd >= 0

> +    close(fence_fd);
>      dmabuf->fence_fd = -1;
>      graphic_hw_gl_block(vc->gfx.dcl.con, false);
>  }
> diff --git a/ui/spice-display.c b/ui/spice-display.c
> index 6eb98a5a5c..1bbce4974b 100644
> --- a/ui/spice-display.c
> +++ b/ui/spice-display.c
> @@ -976,6 +976,7 @@ static void qemu_spice_gl_cursor_dmabuf(DisplayChangeListener *dcl,
>                                          uint32_t hot_x, uint32_t hot_y)
>  {
>      SimpleSpiceDisplay *ssd = container_of(dcl, SimpleSpiceDisplay, dcl);
> +    uint32_t width, height, texture;
>
>      ssd->have_hot = have_hot;
>      ssd->hot_x = hot_x;
> @@ -984,11 +985,13 @@ static void qemu_spice_gl_cursor_dmabuf(DisplayChangeListener *dcl,
>      trace_qemu_spice_gl_cursor(ssd->qxl.id, dmabuf != NULL, have_hot);
>      if (dmabuf) {
>          egl_dmabuf_import_texture(dmabuf);
> -        if (!dmabuf->texture) {
> +        texture = dpy_gl_qemu_dmabuf_get_texture(dmabuf);
> +        if (!texture) {
>              return;
>          }
> -        egl_fb_setup_for_tex(&ssd->cursor_fb, dmabuf->width, dmabuf->height,
> -                             dmabuf->texture, false);
> +        width = dpy_gl_qemu_dmabuf_get_width(dmabuf);
> +        height = dpy_gl_qemu_dmabuf_get_height(dmabuf);
> +        egl_fb_setup_for_tex(&ssd->cursor_fb, width, height, texture, false);
>      } else {
>          egl_fb_destroy(&ssd->cursor_fb);
>      }
> @@ -1026,6 +1029,7 @@ static void qemu_spice_gl_update(DisplayChangeListener *dcl,
>      bool y_0_top = false; /* FIXME */
>      uint64_t cookie;
>      int fd;
> +    uint32_t width, height, texture;
>
>      if (!ssd->have_scanout) {
>          return;
> @@ -1042,41 +1046,45 @@ static void qemu_spice_gl_update(DisplayChangeListener *dcl,
>
>      if (ssd->guest_dmabuf_refresh) {
>          QemuDmaBuf *dmabuf = ssd->guest_dmabuf;
> +        width = dpy_gl_qemu_dmabuf_get_width(dmabuf);
> +        height = dpy_gl_qemu_dmabuf_get_height(dmabuf);
> +
>          if (render_cursor) {
>              egl_dmabuf_import_texture(dmabuf);
> -            if (!dmabuf->texture) {
> +            texture = dpy_gl_qemu_dmabuf_get_texture(dmabuf);
> +            if (!texture) {
>                  return;
>              }
>
>              /* source framebuffer */
> -            egl_fb_setup_for_tex(&ssd->guest_fb,
> -                                 dmabuf->width, dmabuf->height,
> -                                 dmabuf->texture, false);
> +            egl_fb_setup_for_tex(&ssd->guest_fb, width, height,
> +                                 texture, false);
>
>              /* dest framebuffer */
> -            if (ssd->blit_fb.width  != dmabuf->width ||
> -                ssd->blit_fb.height != dmabuf->height) {
> -                trace_qemu_spice_gl_render_dmabuf(ssd->qxl.id, dmabuf->width,
> -                                                  dmabuf->height);
> +            if (ssd->blit_fb.width  != width ||
> +                ssd->blit_fb.height != height) {
> +                trace_qemu_spice_gl_render_dmabuf(ssd->qxl.id, width,
> +                                                  height);
>                  egl_fb_destroy(&ssd->blit_fb);
>                  egl_fb_setup_new_tex(&ssd->blit_fb,
> -                                     dmabuf->width, dmabuf->height);
> +                                     width, height);
>                  fd = egl_get_fd_for_texture(ssd->blit_fb.texture,
>                                              &stride, &fourcc, NULL);
> -                spice_qxl_gl_scanout(&ssd->qxl, fd,
> -                                     dmabuf->width, dmabuf->height,
> +                spice_qxl_gl_scanout(&ssd->qxl, fd, width, height,
>                                       stride, fourcc, false);
>              }
>          } else {
> -            trace_qemu_spice_gl_forward_dmabuf(ssd->qxl.id,
> -                                               dmabuf->width, dmabuf->height);
> +            stride = dpy_gl_qemu_dmabuf_get_stride(dmabuf);
> +            fourcc = dpy_gl_qemu_dmabuf_get_fourcc(dmabuf);
> +            y_0_top = dpy_gl_qemu_dmabuf_get_y0_top(dmabuf);
> +            fd = dpy_gl_qemu_dmabuf_get_fd(dmabuf);
> +
> +            trace_qemu_spice_gl_forward_dmabuf(ssd->qxl.id, width, height);
>              /* note: spice server will close the fd, so hand over a dup */
> -            spice_qxl_gl_scanout(&ssd->qxl, dup(dmabuf->fd),
> -                                 dmabuf->width, dmabuf->height,
> -                                 dmabuf->stride, dmabuf->fourcc,
> -                                 dmabuf->y0_top);
> +            spice_qxl_gl_scanout(&ssd->qxl, dup(fd), width, height,
> +                                 stride, fourcc, y_0_top);
>          }
> -        qemu_spice_gl_monitor_config(ssd, 0, 0, dmabuf->width, dmabuf->height);
> +        qemu_spice_gl_monitor_config(ssd, 0, 0, width, height);
>          ssd->guest_dmabuf_refresh = false;
>      }
>
> --
> 2.34.1
>

Please add a patch before to fix the fence_fd usage. For this patch:
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Re: [PATCH v6 1/3] ui/console: Introduce dpy_gl_qemu_dmabuf_get_..() helpers

[Daniel P. Berrangé]

On Tue, Apr 16, 2024 at 09:09:52PM -0700, dongwon.kim@intel.com wrote:
> From: Dongwon Kim <dongwon.kim@intel.com>
> 
> This commit introduces dpy_gl_qemu_dmabuf_get_... helpers to extract
> specific fields from the QemuDmaBuf struct. It also updates all instances
> where fields within the QemuDmaBuf struct are directly accessed, replacing
> them with calls to these new helper functions.
> 
> v6: fix typos in helper names in ui/spice-display.c
> 
> Suggested-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
> Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
> Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
> ---
>  include/ui/console.h            |  17 +++++
>  hw/display/vhost-user-gpu.c     |   6 +-
>  hw/display/virtio-gpu-udmabuf.c |   7 +-
>  hw/vfio/display.c               |  15 +++--
>  ui/console.c                    | 116 +++++++++++++++++++++++++++++++-
>  ui/dbus-console.c               |   9 ++-
>  ui/dbus-listener.c              |  43 +++++++-----
>  ui/egl-headless.c               |  23 +++++--
>  ui/egl-helpers.c                |  47 +++++++------
>  ui/gtk-egl.c                    |  48 ++++++++-----
>  ui/gtk-gl-area.c                |  37 ++++++----
>  ui/gtk.c                        |   6 +-
>  ui/spice-display.c              |  50 ++++++++------
>  13 files changed, 316 insertions(+), 108 deletions(-)
> 
> diff --git a/include/ui/console.h b/include/ui/console.h
> index 0bc7a00ac0..6292943a82 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -358,6 +358,23 @@ void dpy_gl_cursor_dmabuf(QemuConsole *con, QemuDmaBuf *dmabuf,
>                            bool have_hot, uint32_t hot_x, uint32_t hot_y);
>  void dpy_gl_cursor_position(QemuConsole *con,
>                              uint32_t pos_x, uint32_t pos_y);
> +
> +int32_t dpy_gl_qemu_dmabuf_get_fd(QemuDmaBuf *dmabuf);
> +uint32_t dpy_gl_qemu_dmabuf_get_width(QemuDmaBuf *dmabuf);
> +uint32_t dpy_gl_qemu_dmabuf_get_height(QemuDmaBuf *dmabuf);
> +uint32_t dpy_gl_qemu_dmabuf_get_stride(QemuDmaBuf *dmabuf);
> +uint32_t dpy_gl_qemu_dmabuf_get_fourcc(QemuDmaBuf *dmabuf);
> +uint64_t dpy_gl_qemu_dmabuf_get_modifier(QemuDmaBuf *dmabuf);
> +uint32_t dpy_gl_qemu_dmabuf_get_texture(QemuDmaBuf *dmabuf);
> +uint32_t dpy_gl_qemu_dmabuf_get_x(QemuDmaBuf *dmabuf);
> +uint32_t dpy_gl_qemu_dmabuf_get_y(QemuDmaBuf *dmabuf);
> +uint32_t dpy_gl_qemu_dmabuf_get_backing_width(QemuDmaBuf *dmabuf);
> +uint32_t dpy_gl_qemu_dmabuf_get_backing_height(QemuDmaBuf *dmabuf);
> +bool dpy_gl_qemu_dmabuf_get_y0_top(QemuDmaBuf *dmabuf);
> +void *dpy_gl_qemu_dmabuf_get_sync(QemuDmaBuf *dmabuf);
> +int32_t dpy_gl_qemu_dmabuf_get_fence_fd(QemuDmaBuf *dmabuf);
> +bool dpy_gl_qemu_dmabuf_get_allow_fences(QemuDmaBuf *dmabuf);
> +bool dpy_gl_qemu_dmabuf_get_draw_submitted(QemuDmaBuf *dmabuf);

IMHO these method names don't need a "dpy_gl_" prefix on
them. Since they're accessors for the "QemuDmaBuf" struct,
I think its sufficient to just have "qemu_dmabuf_" as the
name prefix, making names more compact.

The console.{h,c} files are a bit of a dumping ground for
UI code. While QemuDmaBuf was just a struct with direct
field access that's OK.

With turning this into a more of an object with accessors,
I think it would also be desirable to move the struct
definition and all its methods into separate ui/dmabuf.{c,h}
files, so its fully self-contained.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

RE: [PATCH v6 1/3] ui/console: Introduce dpy_gl_qemu_dmabuf_get_..() helpers

[Kim, Dongwon]

Hi Daniel,

> -----Original Message-----
> From: Daniel P. Berrangé <berrange@redhat.com>
> Sent: Wednesday, April 17, 2024 4:05 AM
> To: Kim, Dongwon <dongwon.kim@intel.com>
> Cc: qemu-devel@nongnu.org; marcandre.lureau@redhat.com
> Subject: Re: [PATCH v6 1/3] ui/console: Introduce
> dpy_gl_qemu_dmabuf_get_..() helpers
> 
> On Tue, Apr 16, 2024 at 09:09:52PM -0700, dongwon.kim@intel.com wrote:
> > From: Dongwon Kim <dongwon.kim@intel.com>
> >
> > This commit introduces dpy_gl_qemu_dmabuf_get_... helpers to extract
> > specific fields from the QemuDmaBuf struct. It also updates all
> > instances where fields within the QemuDmaBuf struct are directly
> > accessed, replacing them with calls to these new helper functions.
> >
> > v6: fix typos in helper names in ui/spice-display.c
> >
> > Suggested-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
> > Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
> > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
> > ---
> >  include/ui/console.h            |  17 +++++
> >  hw/display/vhost-user-gpu.c     |   6 +-
> >  hw/display/virtio-gpu-udmabuf.c |   7 +-
> >  hw/vfio/display.c               |  15 +++--
> >  ui/console.c                    | 116 +++++++++++++++++++++++++++++++-
> >  ui/dbus-console.c               |   9 ++-
> >  ui/dbus-listener.c              |  43 +++++++-----
> >  ui/egl-headless.c               |  23 +++++--
> >  ui/egl-helpers.c                |  47 +++++++------
> >  ui/gtk-egl.c                    |  48 ++++++++-----
> >  ui/gtk-gl-area.c                |  37 ++++++----
> >  ui/gtk.c                        |   6 +-
> >  ui/spice-display.c              |  50 ++++++++------
> >  13 files changed, 316 insertions(+), 108 deletions(-)
> >
> > diff --git a/include/ui/console.h b/include/ui/console.h index
> > 0bc7a00ac0..6292943a82 100644
> > --- a/include/ui/console.h
> > +++ b/include/ui/console.h
> > @@ -358,6 +358,23 @@ void dpy_gl_cursor_dmabuf(QemuConsole *con,
> QemuDmaBuf *dmabuf,
> >                            bool have_hot, uint32_t hot_x, uint32_t
> > hot_y);  void dpy_gl_cursor_position(QemuConsole *con,
> >                              uint32_t pos_x, uint32_t pos_y);
> > +
> > +int32_t dpy_gl_qemu_dmabuf_get_fd(QemuDmaBuf *dmabuf); uint32_t
> > +dpy_gl_qemu_dmabuf_get_width(QemuDmaBuf *dmabuf); uint32_t
> > +dpy_gl_qemu_dmabuf_get_height(QemuDmaBuf *dmabuf); uint32_t
> > +dpy_gl_qemu_dmabuf_get_stride(QemuDmaBuf *dmabuf); uint32_t
> > +dpy_gl_qemu_dmabuf_get_fourcc(QemuDmaBuf *dmabuf); uint64_t
> > +dpy_gl_qemu_dmabuf_get_modifier(QemuDmaBuf *dmabuf); uint32_t
> > +dpy_gl_qemu_dmabuf_get_texture(QemuDmaBuf *dmabuf); uint32_t
> > +dpy_gl_qemu_dmabuf_get_x(QemuDmaBuf *dmabuf); uint32_t
> > +dpy_gl_qemu_dmabuf_get_y(QemuDmaBuf *dmabuf); uint32_t
> > +dpy_gl_qemu_dmabuf_get_backing_width(QemuDmaBuf *dmabuf);
> uint32_t
> > +dpy_gl_qemu_dmabuf_get_backing_height(QemuDmaBuf *dmabuf); bool
> > +dpy_gl_qemu_dmabuf_get_y0_top(QemuDmaBuf *dmabuf); void
> > +*dpy_gl_qemu_dmabuf_get_sync(QemuDmaBuf *dmabuf); int32_t
> > +dpy_gl_qemu_dmabuf_get_fence_fd(QemuDmaBuf *dmabuf); bool
> > +dpy_gl_qemu_dmabuf_get_allow_fences(QemuDmaBuf *dmabuf); bool
> > +dpy_gl_qemu_dmabuf_get_draw_submitted(QemuDmaBuf *dmabuf);
> 
> IMHO these method names don't need a "dpy_gl_" prefix on them. Since
> they're accessors for the "QemuDmaBuf" struct, I think its sufficient to just
> have "qemu_dmabuf_" as the name prefix, making names more compact.
> 
> The console.{h,c} files are a bit of a dumping ground for UI code. While
> QemuDmaBuf was just a struct with direct field access that's OK.
> 
> With turning this into a more of an object with accessors, I think it would also
> be desirable to move the struct definition and all its methods into separate
> ui/dmabuf.{c,h} files, so its fully self-contained.
 
[Kim, Dongwon] I am ok with changing function names and create
separate c and h for dmabuf helpers as you suggested. But I would
like to hear Marc-André's opinion about this suggestion before I make
such changes.

Marc-André, do you have any thought on Daniel's suggestion?

> 
> With regards,
> Daniel
> --
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v6 1/3] ui/console: Introduce dpy_gl_qemu_dmabuf_get_..() helpers

[Marc-André Lureau]

Hi

On Wed, Apr 17, 2024 at 9:06 PM Kim, Dongwon <dongwon.kim@intel.com> wrote:
>
> Hi Daniel,
>
> > -----Original Message-----
> > From: Daniel P. Berrangé <berrange@redhat.com>
> > Sent: Wednesday, April 17, 2024 4:05 AM
> > To: Kim, Dongwon <dongwon.kim@intel.com>
> > Cc: qemu-devel@nongnu.org; marcandre.lureau@redhat.com
> > Subject: Re: [PATCH v6 1/3] ui/console: Introduce
> > dpy_gl_qemu_dmabuf_get_..() helpers
> >
> > On Tue, Apr 16, 2024 at 09:09:52PM -0700, dongwon.kim@intel.com wrote:
> > > From: Dongwon Kim <dongwon.kim@intel.com>
> > >
> > > This commit introduces dpy_gl_qemu_dmabuf_get_... helpers to extract
> > > specific fields from the QemuDmaBuf struct. It also updates all
> > > instances where fields within the QemuDmaBuf struct are directly
> > > accessed, replacing them with calls to these new helper functions.
> > >
> > > v6: fix typos in helper names in ui/spice-display.c
> > >
> > > Suggested-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
> > > Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
> > > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
> > > ---
> > >  include/ui/console.h            |  17 +++++
> > >  hw/display/vhost-user-gpu.c     |   6 +-
> > >  hw/display/virtio-gpu-udmabuf.c |   7 +-
> > >  hw/vfio/display.c               |  15 +++--
> > >  ui/console.c                    | 116 +++++++++++++++++++++++++++++++-
> > >  ui/dbus-console.c               |   9 ++-
> > >  ui/dbus-listener.c              |  43 +++++++-----
> > >  ui/egl-headless.c               |  23 +++++--
> > >  ui/egl-helpers.c                |  47 +++++++------
> > >  ui/gtk-egl.c                    |  48 ++++++++-----
> > >  ui/gtk-gl-area.c                |  37 ++++++----
> > >  ui/gtk.c                        |   6 +-
> > >  ui/spice-display.c              |  50 ++++++++------
> > >  13 files changed, 316 insertions(+), 108 deletions(-)
> > >
> > > diff --git a/include/ui/console.h b/include/ui/console.h index
> > > 0bc7a00ac0..6292943a82 100644
> > > --- a/include/ui/console.h
> > > +++ b/include/ui/console.h
> > > @@ -358,6 +358,23 @@ void dpy_gl_cursor_dmabuf(QemuConsole *con,
> > QemuDmaBuf *dmabuf,
> > >                            bool have_hot, uint32_t hot_x, uint32_t
> > > hot_y);  void dpy_gl_cursor_position(QemuConsole *con,
> > >                              uint32_t pos_x, uint32_t pos_y);
> > > +
> > > +int32_t dpy_gl_qemu_dmabuf_get_fd(QemuDmaBuf *dmabuf); uint32_t
> > > +dpy_gl_qemu_dmabuf_get_width(QemuDmaBuf *dmabuf); uint32_t
> > > +dpy_gl_qemu_dmabuf_get_height(QemuDmaBuf *dmabuf); uint32_t
> > > +dpy_gl_qemu_dmabuf_get_stride(QemuDmaBuf *dmabuf); uint32_t
> > > +dpy_gl_qemu_dmabuf_get_fourcc(QemuDmaBuf *dmabuf); uint64_t
> > > +dpy_gl_qemu_dmabuf_get_modifier(QemuDmaBuf *dmabuf); uint32_t
> > > +dpy_gl_qemu_dmabuf_get_texture(QemuDmaBuf *dmabuf); uint32_t
> > > +dpy_gl_qemu_dmabuf_get_x(QemuDmaBuf *dmabuf); uint32_t
> > > +dpy_gl_qemu_dmabuf_get_y(QemuDmaBuf *dmabuf); uint32_t
> > > +dpy_gl_qemu_dmabuf_get_backing_width(QemuDmaBuf *dmabuf);
> > uint32_t
> > > +dpy_gl_qemu_dmabuf_get_backing_height(QemuDmaBuf *dmabuf); bool
> > > +dpy_gl_qemu_dmabuf_get_y0_top(QemuDmaBuf *dmabuf); void
> > > +*dpy_gl_qemu_dmabuf_get_sync(QemuDmaBuf *dmabuf); int32_t
> > > +dpy_gl_qemu_dmabuf_get_fence_fd(QemuDmaBuf *dmabuf); bool
> > > +dpy_gl_qemu_dmabuf_get_allow_fences(QemuDmaBuf *dmabuf); bool
> > > +dpy_gl_qemu_dmabuf_get_draw_submitted(QemuDmaBuf *dmabuf);
> >
> > IMHO these method names don't need a "dpy_gl_" prefix on them. Since
> > they're accessors for the "QemuDmaBuf" struct, I think its sufficient to just
> > have "qemu_dmabuf_" as the name prefix, making names more compact.
> >
> > The console.{h,c} files are a bit of a dumping ground for UI code. While
> > QemuDmaBuf was just a struct with direct field access that's OK.
> >
> > With turning this into a more of an object with accessors, I think it would also
> > be desirable to move the struct definition and all its methods into separate
> > ui/dmabuf.{c,h} files, so its fully self-contained.
>
> [Kim, Dongwon] I am ok with changing function names and create
> separate c and h for dmabuf helpers as you suggested. But I would
> like to hear Marc-André's opinion about this suggestion before I make
> such changes.
>
> Marc-André, do you have any thought on Daniel's suggestion?

Sure, I was about to ask the same. Anything we can do to slim
ui/console.c helps :)

[PATCH v6 2/3] ui/console: Introduce dpy_gl_qemu_dmabuf_set_..() helpers

[dongwon.kim]

From: Dongwon Kim <dongwon.kim@intel.com>

To enhance security in accessing the QemuDmaBuf struct, new helper
functions for setting specific fields within the struct were introduced.
And all occurrences where these fields were previously set directly
have been updated to utilize these helper functions.

Suggested-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
---
 include/ui/console.h |  5 +++++
 ui/console.c         | 30 ++++++++++++++++++++++++++++++
 ui/egl-helpers.c     | 16 +++++++++-------
 ui/gtk-egl.c         |  4 ++--
 ui/gtk-gl-area.c     |  4 ++--
 ui/gtk.c             |  2 +-
 6 files changed, 49 insertions(+), 12 deletions(-)

diff --git a/include/ui/console.h b/include/ui/console.h
index 6292943a82..3d9d8b9fce 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -375,6 +375,11 @@ void *dpy_gl_qemu_dmabuf_get_sync(QemuDmaBuf *dmabuf);
 int32_t dpy_gl_qemu_dmabuf_get_fence_fd(QemuDmaBuf *dmabuf);
 bool dpy_gl_qemu_dmabuf_get_allow_fences(QemuDmaBuf *dmabuf);
 bool dpy_gl_qemu_dmabuf_get_draw_submitted(QemuDmaBuf *dmabuf);
+void dpy_gl_qemu_dmabuf_set_texture(QemuDmaBuf *dmabuf, uint32_t texture);
+void dpy_gl_qemu_dmabuf_set_fence_fd(QemuDmaBuf *dmabuf, int32_t fence_fd);
+void dpy_gl_qemu_dmabuf_set_sync(QemuDmaBuf *dmabuf, void *sync);
+void dpy_gl_qemu_dmabuf_set_draw_submitted(QemuDmaBuf *dmabuf, bool draw_submitted);
+void dpy_gl_qemu_dmabuf_set_fd(QemuDmaBuf *dmabuf, int32_t fd);
 void dpy_gl_release_dmabuf(QemuConsole *con,
                            QemuDmaBuf *dmabuf);
 void dpy_gl_update(QemuConsole *con,
diff --git a/ui/console.c b/ui/console.c
index 5d5635f783..d4ca9e6e0f 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1244,6 +1244,36 @@ bool dpy_gl_qemu_dmabuf_get_draw_submitted(QemuDmaBuf *dmabuf)
     return dmabuf->draw_submitted;
 }
 
+void dpy_gl_qemu_dmabuf_set_texture(QemuDmaBuf *dmabuf, uint32_t texture)
+{
+    assert(dmabuf != NULL);
+    dmabuf->texture = texture;
+}
+
+void dpy_gl_qemu_dmabuf_set_fence_fd(QemuDmaBuf *dmabuf, int32_t fence_fd)
+{
+    assert(dmabuf != NULL);
+    dmabuf->fence_fd = fence_fd;
+}
+
+void dpy_gl_qemu_dmabuf_set_sync(QemuDmaBuf *dmabuf, void *sync)
+{
+    assert(dmabuf != NULL);
+    dmabuf->sync = sync;
+}
+
+void dpy_gl_qemu_dmabuf_set_draw_submitted(QemuDmaBuf *dmabuf, bool draw_submitted)
+{
+    assert(dmabuf != NULL);
+    dmabuf->draw_submitted = draw_submitted;
+}
+
+void dpy_gl_qemu_dmabuf_set_fd(QemuDmaBuf *dmabuf, int32_t fd)
+{
+    assert(dmabuf != NULL);
+    dmabuf->fd = fd;
+}
+
 void dpy_gl_release_dmabuf(QemuConsole *con,
                           QemuDmaBuf *dmabuf)
 {
diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
index 86d64c68ce..c71a2878c2 100644
--- a/ui/egl-helpers.c
+++ b/ui/egl-helpers.c
@@ -348,8 +348,8 @@ void egl_dmabuf_import_texture(QemuDmaBuf *dmabuf)
         return;
     }
 
-    glGenTextures(1, &dmabuf->texture);
-    texture = dpy_gl_qemu_dmabuf_get_texture(dmabuf);
+    glGenTextures(1, &texture);
+    dpy_gl_qemu_dmabuf_set_texture(dmabuf, texture);
     glBindTexture(GL_TEXTURE_2D, texture);
     glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
     glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
@@ -368,7 +368,7 @@ void egl_dmabuf_release_texture(QemuDmaBuf *dmabuf)
     }
 
     glDeleteTextures(1, &texture);
-    dmabuf->texture = 0;
+    dpy_gl_qemu_dmabuf_set_texture(dmabuf, 0);
 }
 
 void egl_dmabuf_create_sync(QemuDmaBuf *dmabuf)
@@ -382,7 +382,7 @@ void egl_dmabuf_create_sync(QemuDmaBuf *dmabuf)
         sync = eglCreateSyncKHR(qemu_egl_display,
                                 EGL_SYNC_NATIVE_FENCE_ANDROID, NULL);
         if (sync != EGL_NO_SYNC_KHR) {
-            dmabuf->sync = sync;
+            dpy_gl_qemu_dmabuf_set_sync(dmabuf, sync);
         }
     }
 }
@@ -390,12 +390,14 @@ void egl_dmabuf_create_sync(QemuDmaBuf *dmabuf)
 void egl_dmabuf_create_fence(QemuDmaBuf *dmabuf)
 {
     void *sync = dpy_gl_qemu_dmabuf_get_sync(dmabuf);
+    int fence_fd;
 
     if (sync) {
-        dmabuf->fence_fd = eglDupNativeFenceFDANDROID(qemu_egl_display,
-                                                      sync);
+        fence_fd = eglDupNativeFenceFDANDROID(qemu_egl_display,
+                                              sync);
+        dpy_gl_qemu_dmabuf_set_fence_fd(dmabuf, fence_fd);
         eglDestroySyncKHR(qemu_egl_display, sync);
-        dmabuf->sync = NULL;
+        dpy_gl_qemu_dmabuf_set_sync(dmabuf, NULL);
     }
 }
 
diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
index c9469af9ed..7494a34d7c 100644
--- a/ui/gtk-egl.c
+++ b/ui/gtk-egl.c
@@ -87,7 +87,7 @@ void gd_egl_draw(VirtualConsole *vc)
             if (!dpy_gl_qemu_dmabuf_get_draw_submitted(dmabuf)) {
                 return;
             } else {
-                dmabuf->draw_submitted = false;
+                dpy_gl_qemu_dmabuf_set_draw_submitted(dmabuf, false);
             }
         }
 #endif
@@ -381,7 +381,7 @@ void gd_egl_flush(DisplayChangeListener *dcl,
     if (vc->gfx.guest_fb.dmabuf &&
         !dpy_gl_qemu_dmabuf_get_draw_submitted(vc->gfx.guest_fb.dmabuf)) {
         graphic_hw_gl_block(vc->gfx.dcl.con, true);
-        vc->gfx.guest_fb.dmabuf->draw_submitted = true;
+        dpy_gl_qemu_dmabuf_set_draw_submitted(vc->gfx.guest_fb.dmabuf, true);
         gtk_egl_set_scanout_mode(vc, true);
         gtk_widget_queue_draw_area(area, x, y, w, h);
         return;
diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
index 193862ecc2..26b9689a5f 100644
--- a/ui/gtk-gl-area.c
+++ b/ui/gtk-gl-area.c
@@ -63,7 +63,7 @@ void gd_gl_area_draw(VirtualConsole *vc)
             if (!dpy_gl_qemu_dmabuf_get_draw_submitted(dmabuf)) {
                 return;
             } else {
-                dmabuf->draw_submitted = false;
+                dpy_gl_qemu_dmabuf_set_draw_submitted(dmabuf, false);
             }
         }
 #endif
@@ -291,7 +291,7 @@ void gd_gl_area_scanout_flush(DisplayChangeListener *dcl,
     if (vc->gfx.guest_fb.dmabuf &&
         !dpy_gl_qemu_dmabuf_get_draw_submitted(vc->gfx.guest_fb.dmabuf)) {
         graphic_hw_gl_block(vc->gfx.dcl.con, true);
-        vc->gfx.guest_fb.dmabuf->draw_submitted = true;
+        dpy_gl_qemu_dmabuf_set_draw_submitted(vc->gfx.guest_fb.dmabuf, true);
         gtk_gl_area_set_scanout_mode(vc, true);
     }
     gtk_gl_area_queue_render(GTK_GL_AREA(vc->gfx.drawing_area));
diff --git a/ui/gtk.c b/ui/gtk.c
index 2c054a42ba..b6a1f6f897 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -601,7 +601,7 @@ void gd_hw_gl_flushed(void *vcon)
     fence_fd = dpy_gl_qemu_dmabuf_get_fence_fd(dmabuf);
     qemu_set_fd_handler(fence_fd, NULL, NULL, NULL);
     close(fence_fd);
-    dmabuf->fence_fd = -1;
+    dpy_gl_qemu_dmabuf_set_fence_fd(dmabuf, -1);
     graphic_hw_gl_block(vc->gfx.dcl.con, false);
 }
 
-- 
2.34.1

[PATCH v6 3/3] ui/console: Introduce dpy_gl_qemu_dmabuf_new() and free() helpers

[dongwon.kim]

From: Dongwon Kim <dongwon.kim@intel.com>

This commit introduces utility functions for the creation and deallocation
of QemuDmaBuf instances. Additionally, it updates all relevant sections
of the codebase to utilize these new utility functions.

Suggested-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
---
 include/hw/vfio/vfio-common.h   |  2 +-
 include/hw/virtio/virtio-gpu.h  |  4 ++--
 include/ui/console.h            |  8 +++++++-
 hw/display/vhost-user-gpu.c     | 32 +++++++++++++++++--------------
 hw/display/virtio-gpu-udmabuf.c | 24 +++++++++--------------
 hw/vfio/display.c               | 26 ++++++++++++-------------
 ui/console.c                    | 34 +++++++++++++++++++++++++++++++++
 ui/dbus-listener.c              | 28 ++++++++++++---------------
 8 files changed, 95 insertions(+), 63 deletions(-)

diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index b9da6c08ef..d66e27db02 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -148,7 +148,7 @@ typedef struct VFIOGroup {
 } VFIOGroup;
 
 typedef struct VFIODMABuf {
-    QemuDmaBuf buf;
+    QemuDmaBuf *buf;
     uint32_t pos_x, pos_y, pos_updates;
     uint32_t hot_x, hot_y, hot_updates;
     int dmabuf_id;
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index ed44cdad6b..56d6e821bf 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -169,7 +169,7 @@ struct VirtIOGPUBaseClass {
     DEFINE_PROP_UINT32("yres", _state, _conf.yres, 800)
 
 typedef struct VGPUDMABuf {
-    QemuDmaBuf buf;
+    QemuDmaBuf *buf;
     uint32_t scanout_id;
     QTAILQ_ENTRY(VGPUDMABuf) next;
 } VGPUDMABuf;
@@ -238,7 +238,7 @@ struct VhostUserGPU {
     VhostUserBackend *vhost;
     int vhost_gpu_fd; /* closed by the chardev */
     CharBackend vhost_chr;
-    QemuDmaBuf dmabuf[VIRTIO_GPU_MAX_SCANOUTS];
+    QemuDmaBuf *dmabuf[VIRTIO_GPU_MAX_SCANOUTS];
     bool backend_blocked;
 };
 
diff --git a/include/ui/console.h b/include/ui/console.h
index 3d9d8b9fce..6d7c03b7c5 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -358,7 +358,13 @@ void dpy_gl_cursor_dmabuf(QemuConsole *con, QemuDmaBuf *dmabuf,
                           bool have_hot, uint32_t hot_x, uint32_t hot_y);
 void dpy_gl_cursor_position(QemuConsole *con,
                             uint32_t pos_x, uint32_t pos_y);
-
+QemuDmaBuf *dpy_gl_qemu_dmabuf_new(uint32_t width, uint32_t height,
+                                   uint32_t stride, uint32_t x,
+                                   uint32_t y, uint32_t backing_width,
+                                   uint32_t backing_height, uint32_t fourcc,
+                                   uint64_t modifier, uint32_t dmabuf_fd,
+                                   bool allow_fences, bool y0_top);
+void dpy_gl_qemu_dmabuf_free(QemuDmaBuf *dmabuf);
 int32_t dpy_gl_qemu_dmabuf_get_fd(QemuDmaBuf *dmabuf);
 uint32_t dpy_gl_qemu_dmabuf_get_width(QemuDmaBuf *dmabuf);
 uint32_t dpy_gl_qemu_dmabuf_get_height(QemuDmaBuf *dmabuf);
diff --git a/hw/display/vhost-user-gpu.c b/hw/display/vhost-user-gpu.c
index 87dcfbca10..4d8461e94a 100644
--- a/hw/display/vhost-user-gpu.c
+++ b/hw/display/vhost-user-gpu.c
@@ -250,6 +250,7 @@ vhost_user_gpu_handle_display(VhostUserGPU *g, VhostUserGpuMsg *msg)
         VhostUserGpuDMABUFScanout *m = &msg->payload.dmabuf_scanout;
         int fd = qemu_chr_fe_get_msgfd(&g->vhost_chr);
         int old_fd;
+        uint64_t modifier = 0;
         QemuDmaBuf *dmabuf;
 
         if (m->scanout_id >= g->parent_obj.conf.max_outputs) {
@@ -262,31 +263,34 @@ vhost_user_gpu_handle_display(VhostUserGPU *g, VhostUserGpuMsg *msg)
 
         g->parent_obj.enable = 1;
         con = g->parent_obj.scanout[m->scanout_id].con;
-        dmabuf = &g->dmabuf[m->scanout_id];
-        old_fd = dpy_gl_qemu_dmabuf_get_fd(dmabuf);
-        if (old_fd >= 0) {
-            close(old_fd);
-            dmabuf->fd = -1;
+        dmabuf = g->dmabuf[m->scanout_id];
+        if (dmabuf) {
+            old_fd = dpy_gl_qemu_dmabuf_get_fd(dmabuf);
+            if (old_fd >= 0) {
+                close(old_fd);
+                dpy_gl_qemu_dmabuf_set_fd(dmabuf, -1);
+            }
         }
         dpy_gl_release_dmabuf(con, dmabuf);
+        g_clear_pointer(&dmabuf, dpy_gl_qemu_dmabuf_free);
         if (fd == -1) {
             dpy_gl_scanout_disable(con);
             break;
         }
-        *dmabuf = (QemuDmaBuf) {
-            .fd = fd,
-            .width = m->fd_width,
-            .height = m->fd_height,
-            .stride = m->fd_stride,
-            .fourcc = m->fd_drm_fourcc,
-            .y0_top = m->fd_flags & VIRTIO_GPU_RESOURCE_FLAG_Y_0_TOP,
-        };
+
         if (msg->request == VHOST_USER_GPU_DMABUF_SCANOUT2) {
             VhostUserGpuDMABUFScanout2 *m2 = &msg->payload.dmabuf_scanout2;
-            dmabuf->modifier = m2->modifier;
+            modifier = m2->modifier;
         }
 
+        dmabuf = dpy_gl_qemu_dmabuf_new(m->fd_width, m->fd_height,
+                                        m->fd_stride, 0, 0, 0, 0,
+                                        m->fd_drm_fourcc, modifier,
+                                        fd, false, m->fd_flags &
+                                        VIRTIO_GPU_RESOURCE_FLAG_Y_0_TOP);
+
         dpy_gl_scanout_dmabuf(con, dmabuf);
+        g->dmabuf[m->scanout_id] = dmabuf;
         break;
     }
     case VHOST_USER_GPU_DMABUF_UPDATE: {
diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
index e3f358b575..79eafc7289 100644
--- a/hw/display/virtio-gpu-udmabuf.c
+++ b/hw/display/virtio-gpu-udmabuf.c
@@ -162,7 +162,8 @@ static void virtio_gpu_free_dmabuf(VirtIOGPU *g, VGPUDMABuf *dmabuf)
     struct virtio_gpu_scanout *scanout;
 
     scanout = &g->parent_obj.scanout[dmabuf->scanout_id];
-    dpy_gl_release_dmabuf(scanout->con, &dmabuf->buf);
+    dpy_gl_release_dmabuf(scanout->con, dmabuf->buf);
+    g_clear_pointer(&dmabuf->buf, dpy_gl_qemu_dmabuf_free);
     QTAILQ_REMOVE(&g->dmabuf.bufs, dmabuf, next);
     g_free(dmabuf);
 }
@@ -181,17 +182,10 @@ static VGPUDMABuf
     }
 
     dmabuf = g_new0(VGPUDMABuf, 1);
-    dmabuf->buf.width = r->width;
-    dmabuf->buf.height = r->height;
-    dmabuf->buf.stride = fb->stride;
-    dmabuf->buf.x = r->x;
-    dmabuf->buf.y = r->y;
-    dmabuf->buf.backing_width = fb->width;
-    dmabuf->buf.backing_height = fb->height;
-    dmabuf->buf.fourcc = qemu_pixman_to_drm_format(fb->format);
-    dmabuf->buf.fd = res->dmabuf_fd;
-    dmabuf->buf.allow_fences = true;
-    dmabuf->buf.draw_submitted = false;
+    dmabuf->buf = dpy_gl_qemu_dmabuf_new(r->width, r->height, fb->stride,
+                                         r->x, r->y, fb->width, fb->height,
+                                         qemu_pixman_to_drm_format(fb->format),
+                                         0, res->dmabuf_fd, false, 0);
     dmabuf->scanout_id = scanout_id;
     QTAILQ_INSERT_HEAD(&g->dmabuf.bufs, dmabuf, next);
 
@@ -217,11 +211,11 @@ int virtio_gpu_update_dmabuf(VirtIOGPU *g,
         old_primary = g->dmabuf.primary[scanout_id];
     }
 
-    width = dpy_gl_qemu_dmabuf_get_width(&new_primary->buf);
-    height = dpy_gl_qemu_dmabuf_get_height(&new_primary->buf);
+    width = dpy_gl_qemu_dmabuf_get_width(new_primary->buf);
+    height = dpy_gl_qemu_dmabuf_get_height(new_primary->buf);
     g->dmabuf.primary[scanout_id] = new_primary;
     qemu_console_resize(scanout->con, width, height);
-    dpy_gl_scanout_dmabuf(scanout->con, &new_primary->buf);
+    dpy_gl_scanout_dmabuf(scanout->con, new_primary->buf);
 
     if (old_primary) {
         virtio_gpu_free_dmabuf(g, old_primary);
diff --git a/hw/vfio/display.c b/hw/vfio/display.c
index f9c39cbd51..7e26d9667f 100644
--- a/hw/vfio/display.c
+++ b/hw/vfio/display.c
@@ -241,14 +241,11 @@ static VFIODMABuf *vfio_display_get_dmabuf(VFIOPCIDevice *vdev,
 
     dmabuf = g_new0(VFIODMABuf, 1);
     dmabuf->dmabuf_id  = plane.dmabuf_id;
-    dmabuf->buf.width  = plane.width;
-    dmabuf->buf.height = plane.height;
-    dmabuf->buf.backing_width = plane.width;
-    dmabuf->buf.backing_height = plane.height;
-    dmabuf->buf.stride = plane.stride;
-    dmabuf->buf.fourcc = plane.drm_format;
-    dmabuf->buf.modifier = plane.drm_format_mod;
-    dmabuf->buf.fd     = fd;
+    dmabuf->buf = dpy_gl_qemu_dmabuf_new(plane.width, plane.height,
+                                         plane.stride, 0, 0, plane.width,
+                                         plane.height, plane.drm_format,
+                                         plane.drm_format_mod, fd, false, 0);
+
     if (plane_type == DRM_PLANE_TYPE_CURSOR) {
         vfio_display_update_cursor(dmabuf, &plane);
     }
@@ -263,8 +260,9 @@ static void vfio_display_free_one_dmabuf(VFIODisplay *dpy, VFIODMABuf *dmabuf)
 
     QTAILQ_REMOVE(&dpy->dmabuf.bufs, dmabuf, next);
 
-    fd = dpy_gl_qemu_dmabuf_get_fd(&dmabuf->buf);
-    dpy_gl_release_dmabuf(dpy->con, &dmabuf->buf);
+    fd = dpy_gl_qemu_dmabuf_get_fd(dmabuf->buf);
+    dpy_gl_release_dmabuf(dpy->con, dmabuf->buf);
+    g_clear_pointer(&dmabuf->buf, dpy_gl_qemu_dmabuf_free);
     close(fd);
     g_free(dmabuf);
 }
@@ -301,13 +299,13 @@ static void vfio_display_dmabuf_update(void *opaque)
         return;
     }
 
-    width = dpy_gl_qemu_dmabuf_get_width(&primary->buf);
-    height = dpy_gl_qemu_dmabuf_get_height(&primary->buf);
+    width = dpy_gl_qemu_dmabuf_get_width(primary->buf);
+    height = dpy_gl_qemu_dmabuf_get_height(primary->buf);
 
     if (dpy->dmabuf.primary != primary) {
         dpy->dmabuf.primary = primary;
         qemu_console_resize(dpy->con, width, height);
-        dpy_gl_scanout_dmabuf(dpy->con, &primary->buf);
+        dpy_gl_scanout_dmabuf(dpy->con, primary->buf);
         free_bufs = true;
     }
 
@@ -321,7 +319,7 @@ static void vfio_display_dmabuf_update(void *opaque)
     if (cursor && (new_cursor || cursor->hot_updates)) {
         bool have_hot = (cursor->hot_x != 0xffffffff &&
                          cursor->hot_y != 0xffffffff);
-        dpy_gl_cursor_dmabuf(dpy->con, &cursor->buf, have_hot,
+        dpy_gl_cursor_dmabuf(dpy->con, cursor->buf, have_hot,
                              cursor->hot_x, cursor->hot_y);
         cursor->hot_updates = 0;
     } else if (!cursor && new_cursor) {
diff --git a/ui/console.c b/ui/console.c
index d4ca9e6e0f..ea23fd8af6 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1132,6 +1132,40 @@ void dpy_gl_cursor_position(QemuConsole *con,
     }
 }
 
+QemuDmaBuf *dpy_gl_qemu_dmabuf_new(uint32_t width, uint32_t height,
+                                   uint32_t stride, uint32_t x,
+                                   uint32_t y, uint32_t backing_width,
+                                   uint32_t backing_height, uint32_t fourcc,
+                                   uint64_t modifier, uint32_t dmabuf_fd,
+                                   bool allow_fences, bool y0_top) {
+    QemuDmaBuf *dmabuf;
+
+    dmabuf = g_new0(QemuDmaBuf, 1);
+
+    dmabuf->width = width;
+    dmabuf->height = height;
+    dmabuf->stride = stride;
+    dmabuf->x = x;
+    dmabuf->y = y;
+    dmabuf->backing_width = backing_width;
+    dmabuf->backing_height = backing_height;
+    dmabuf->fourcc = fourcc;
+    dmabuf->modifier = modifier;
+    dmabuf->fd = dmabuf_fd;
+    dmabuf->allow_fences = allow_fences;
+    dmabuf->y0_top = y0_top;
+    dmabuf->fence_fd = -1;
+
+    return dmabuf;
+}
+
+void dpy_gl_qemu_dmabuf_free(QemuDmaBuf *dmabuf)
+{
+    assert(dmabuf != NULL);
+
+    g_free(dmabuf);
+}
+
 int32_t dpy_gl_qemu_dmabuf_get_fd(QemuDmaBuf *dmabuf)
 {
     assert(dmabuf != NULL);
diff --git a/ui/dbus-listener.c b/ui/dbus-listener.c
index c6c7d93753..85d779a45c 100644
--- a/ui/dbus-listener.c
+++ b/ui/dbus-listener.c
@@ -442,28 +442,24 @@ static void dbus_scanout_texture(DisplayChangeListener *dcl,
     trace_dbus_scanout_texture(tex_id, backing_y_0_top,
                                backing_width, backing_height, x, y, w, h);
 #ifdef CONFIG_GBM
-    QemuDmaBuf dmabuf = {
-        .width = w,
-        .height = h,
-        .y0_top = backing_y_0_top,
-        .x = x,
-        .y = y,
-        .backing_width = backing_width,
-        .backing_height = backing_height,
-    };
+    int32_t fd;
+    uint32_t stride, fourcc;
+    uint64_t modifier;
+    QemuDmaBuf *dmabuf;
 
     assert(tex_id);
-    dmabuf.fd = egl_get_fd_for_texture(
-        tex_id, (EGLint *)&dmabuf.stride,
-        (EGLint *)&dmabuf.fourcc,
-        &dmabuf.modifier);
-    if (dmabuf.fd < 0) {
+    fd = egl_get_fd_for_texture(tex_id, (EGLint *)&stride, (EGLint *)&fourcc,
+                                &modifier);
+    if (fd < 0) {
         error_report("%s: failed to get fd for texture", __func__);
         return;
     }
+    dmabuf = dpy_gl_qemu_dmabuf_new(w, h, stride, x, y, backing_width,
+                                    backing_height, fourcc, modifier, fd,
+                                    false, backing_y_0_top);
 
-    dbus_scanout_dmabuf(dcl, &dmabuf);
-    close(dmabuf.fd);
+    dbus_scanout_dmabuf(dcl, dmabuf);
+    close(fd);
 #endif
 
 #ifdef WIN32
-- 
2.34.1

Re: [PATCH v6 3/3] ui/console: Introduce dpy_gl_qemu_dmabuf_new() and free() helpers

[Daniel P. Berrangé]

On Tue, Apr 16, 2024 at 09:09:54PM -0700, dongwon.kim@intel.com wrote:
> From: Dongwon Kim <dongwon.kim@intel.com>
> 
> This commit introduces utility functions for the creation and deallocation
> of QemuDmaBuf instances. Additionally, it updates all relevant sections
> of the codebase to utilize these new utility functions.
> 
> Suggested-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
> Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
> Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
> ---
>  include/hw/vfio/vfio-common.h   |  2 +-
>  include/hw/virtio/virtio-gpu.h  |  4 ++--
>  include/ui/console.h            |  8 +++++++-
>  hw/display/vhost-user-gpu.c     | 32 +++++++++++++++++--------------
>  hw/display/virtio-gpu-udmabuf.c | 24 +++++++++--------------
>  hw/vfio/display.c               | 26 ++++++++++++-------------
>  ui/console.c                    | 34 +++++++++++++++++++++++++++++++++
>  ui/dbus-listener.c              | 28 ++++++++++++---------------
>  8 files changed, 95 insertions(+), 63 deletions(-)
> 
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index b9da6c08ef..d66e27db02 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -148,7 +148,7 @@ typedef struct VFIOGroup {
>  } VFIOGroup;
>  
>  typedef struct VFIODMABuf {
> -    QemuDmaBuf buf;
> +    QemuDmaBuf *buf;
>      uint32_t pos_x, pos_y, pos_updates;
>      uint32_t hot_x, hot_y, hot_updates;
>      int dmabuf_id;
> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> index ed44cdad6b..56d6e821bf 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -169,7 +169,7 @@ struct VirtIOGPUBaseClass {
>      DEFINE_PROP_UINT32("yres", _state, _conf.yres, 800)
>  
>  typedef struct VGPUDMABuf {
> -    QemuDmaBuf buf;
> +    QemuDmaBuf *buf;
>      uint32_t scanout_id;
>      QTAILQ_ENTRY(VGPUDMABuf) next;
>  } VGPUDMABuf;
> @@ -238,7 +238,7 @@ struct VhostUserGPU {
>      VhostUserBackend *vhost;
>      int vhost_gpu_fd; /* closed by the chardev */
>      CharBackend vhost_chr;
> -    QemuDmaBuf dmabuf[VIRTIO_GPU_MAX_SCANOUTS];
> +    QemuDmaBuf *dmabuf[VIRTIO_GPU_MAX_SCANOUTS];
>      bool backend_blocked;
>  };
>  
> diff --git a/include/ui/console.h b/include/ui/console.h
> index 3d9d8b9fce..6d7c03b7c5 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -358,7 +358,13 @@ void dpy_gl_cursor_dmabuf(QemuConsole *con, QemuDmaBuf *dmabuf,
>                            bool have_hot, uint32_t hot_x, uint32_t hot_y);
>  void dpy_gl_cursor_position(QemuConsole *con,
>                              uint32_t pos_x, uint32_t pos_y);
> -
> +QemuDmaBuf *dpy_gl_qemu_dmabuf_new(uint32_t width, uint32_t height,
> +                                   uint32_t stride, uint32_t x,
> +                                   uint32_t y, uint32_t backing_width,
> +                                   uint32_t backing_height, uint32_t fourcc,
> +                                   uint64_t modifier, uint32_t dmabuf_fd,
> +                                   bool allow_fences, bool y0_top);
> +void dpy_gl_qemu_dmabuf_free(QemuDmaBuf *dmabuf);

Since you're creating a new allocator/deallocator pair for
this, please also call G_DEFINE_AUTOPTR_CLEANUP_FUNC so this
struct becomes usable with g_autoptr(), even if we don't
currently have an immediate need to use g_autoptr.

>  int32_t dpy_gl_qemu_dmabuf_get_fd(QemuDmaBuf *dmabuf);
>  uint32_t dpy_gl_qemu_dmabuf_get_width(QemuDmaBuf *dmabuf);
>  uint32_t dpy_gl_qemu_dmabuf_get_height(QemuDmaBuf *dmabuf);


> diff --git a/ui/console.c b/ui/console.c
> index d4ca9e6e0f..ea23fd8af6 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -1132,6 +1132,40 @@ void dpy_gl_cursor_position(QemuConsole *con,
>      }
>  }
>  
> +QemuDmaBuf *dpy_gl_qemu_dmabuf_new(uint32_t width, uint32_t height,
> +                                   uint32_t stride, uint32_t x,
> +                                   uint32_t y, uint32_t backing_width,
> +                                   uint32_t backing_height, uint32_t fourcc,
> +                                   uint64_t modifier, uint32_t dmabuf_fd,
> +                                   bool allow_fences, bool y0_top) {
> +    QemuDmaBuf *dmabuf;
> +
> +    dmabuf = g_new0(QemuDmaBuf, 1);
> +
> +    dmabuf->width = width;
> +    dmabuf->height = height;
> +    dmabuf->stride = stride;
> +    dmabuf->x = x;
> +    dmabuf->y = y;
> +    dmabuf->backing_width = backing_width;
> +    dmabuf->backing_height = backing_height;
> +    dmabuf->fourcc = fourcc;
> +    dmabuf->modifier = modifier;
> +    dmabuf->fd = dmabuf_fd;
> +    dmabuf->allow_fences = allow_fences;
> +    dmabuf->y0_top = y0_top;
> +    dmabuf->fence_fd = -1;
> +
> +    return dmabuf;
> +}
> +
> +void dpy_gl_qemu_dmabuf_free(QemuDmaBuf *dmabuf)
> +{
> +    assert(dmabuf != NULL);

It is normal practice for all XXX_free() funcs to
accept NULL as a valid argument, and operate as
a no-op. This makes code more robust, as it can
blindly call free without checking for NULL ahead
of time.

> +
> +    g_free(dmabuf);
> +}
> +

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v6 3/3] ui/console: Introduce dpy_gl_qemu_dmabuf_new() and free() helpers

[Marc-André Lureau]

On Wed, Apr 17, 2024 at 8:14 AM <dongwon.kim@intel.com> wrote:
>
> From: Dongwon Kim <dongwon.kim@intel.com>
>
> This commit introduces utility functions for the creation and deallocation
> of QemuDmaBuf instances. Additionally, it updates all relevant sections
> of the codebase to utilize these new utility functions.
>
> Suggested-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
> Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
> Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
> ---
>  include/hw/vfio/vfio-common.h   |  2 +-
>  include/hw/virtio/virtio-gpu.h  |  4 ++--
>  include/ui/console.h            |  8 +++++++-
>  hw/display/vhost-user-gpu.c     | 32 +++++++++++++++++--------------
>  hw/display/virtio-gpu-udmabuf.c | 24 +++++++++--------------
>  hw/vfio/display.c               | 26 ++++++++++++-------------
>  ui/console.c                    | 34 +++++++++++++++++++++++++++++++++
>  ui/dbus-listener.c              | 28 ++++++++++++---------------
>  8 files changed, 95 insertions(+), 63 deletions(-)
>
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index b9da6c08ef..d66e27db02 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -148,7 +148,7 @@ typedef struct VFIOGroup {
>  } VFIOGroup;
>
>  typedef struct VFIODMABuf {
> -    QemuDmaBuf buf;
> +    QemuDmaBuf *buf;
>      uint32_t pos_x, pos_y, pos_updates;
>      uint32_t hot_x, hot_y, hot_updates;
>      int dmabuf_id;
> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> index ed44cdad6b..56d6e821bf 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -169,7 +169,7 @@ struct VirtIOGPUBaseClass {
>      DEFINE_PROP_UINT32("yres", _state, _conf.yres, 800)
>
>  typedef struct VGPUDMABuf {
> -    QemuDmaBuf buf;
> +    QemuDmaBuf *buf;
>      uint32_t scanout_id;
>      QTAILQ_ENTRY(VGPUDMABuf) next;
>  } VGPUDMABuf;
> @@ -238,7 +238,7 @@ struct VhostUserGPU {
>      VhostUserBackend *vhost;
>      int vhost_gpu_fd; /* closed by the chardev */
>      CharBackend vhost_chr;
> -    QemuDmaBuf dmabuf[VIRTIO_GPU_MAX_SCANOUTS];
> +    QemuDmaBuf *dmabuf[VIRTIO_GPU_MAX_SCANOUTS];
>      bool backend_blocked;
>  };
>
> diff --git a/include/ui/console.h b/include/ui/console.h
> index 3d9d8b9fce..6d7c03b7c5 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -358,7 +358,13 @@ void dpy_gl_cursor_dmabuf(QemuConsole *con, QemuDmaBuf *dmabuf,
>                            bool have_hot, uint32_t hot_x, uint32_t hot_y);
>  void dpy_gl_cursor_position(QemuConsole *con,
>                              uint32_t pos_x, uint32_t pos_y);
> -
> +QemuDmaBuf *dpy_gl_qemu_dmabuf_new(uint32_t width, uint32_t height,
> +                                   uint32_t stride, uint32_t x,
> +                                   uint32_t y, uint32_t backing_width,
> +                                   uint32_t backing_height, uint32_t fourcc,
> +                                   uint64_t modifier, uint32_t dmabuf_fd,

An fd is an int.

> +                                   bool allow_fences, bool y0_top);
> +void dpy_gl_qemu_dmabuf_free(QemuDmaBuf *dmabuf);
>  int32_t dpy_gl_qemu_dmabuf_get_fd(QemuDmaBuf *dmabuf);
>  uint32_t dpy_gl_qemu_dmabuf_get_width(QemuDmaBuf *dmabuf);
>  uint32_t dpy_gl_qemu_dmabuf_get_height(QemuDmaBuf *dmabuf);
> diff --git a/hw/display/vhost-user-gpu.c b/hw/display/vhost-user-gpu.c
> index 87dcfbca10..4d8461e94a 100644
> --- a/hw/display/vhost-user-gpu.c
> +++ b/hw/display/vhost-user-gpu.c
> @@ -250,6 +250,7 @@ vhost_user_gpu_handle_display(VhostUserGPU *g, VhostUserGpuMsg *msg)
>          VhostUserGpuDMABUFScanout *m = &msg->payload.dmabuf_scanout;
>          int fd = qemu_chr_fe_get_msgfd(&g->vhost_chr);
>          int old_fd;
> +        uint64_t modifier = 0;
>          QemuDmaBuf *dmabuf;
>
>          if (m->scanout_id >= g->parent_obj.conf.max_outputs) {
> @@ -262,31 +263,34 @@ vhost_user_gpu_handle_display(VhostUserGPU *g, VhostUserGpuMsg *msg)
>
>          g->parent_obj.enable = 1;
>          con = g->parent_obj.scanout[m->scanout_id].con;
> -        dmabuf = &g->dmabuf[m->scanout_id];
> -        old_fd = dpy_gl_qemu_dmabuf_get_fd(dmabuf);
> -        if (old_fd >= 0) {
> -            close(old_fd);
> -            dmabuf->fd = -1;
> +        dmabuf = g->dmabuf[m->scanout_id];
> +        if (dmabuf) {
> +            old_fd = dpy_gl_qemu_dmabuf_get_fd(dmabuf);
> +            if (old_fd >= 0) {
> +                close(old_fd);
> +                dpy_gl_qemu_dmabuf_set_fd(dmabuf, -1);
> +            }
>          }
>          dpy_gl_release_dmabuf(con, dmabuf);
> +        g_clear_pointer(&dmabuf, dpy_gl_qemu_dmabuf_free);
>          if (fd == -1) {
>              dpy_gl_scanout_disable(con);
>              break;
>          }
> -        *dmabuf = (QemuDmaBuf) {
> -            .fd = fd,
> -            .width = m->fd_width,
> -            .height = m->fd_height,
> -            .stride = m->fd_stride,
> -            .fourcc = m->fd_drm_fourcc,
> -            .y0_top = m->fd_flags & VIRTIO_GPU_RESOURCE_FLAG_Y_0_TOP,
> -        };
> +
>          if (msg->request == VHOST_USER_GPU_DMABUF_SCANOUT2) {
>              VhostUserGpuDMABUFScanout2 *m2 = &msg->payload.dmabuf_scanout2;
> -            dmabuf->modifier = m2->modifier;
> +            modifier = m2->modifier;
>          }
>
> +        dmabuf = dpy_gl_qemu_dmabuf_new(m->fd_width, m->fd_height,
> +                                        m->fd_stride, 0, 0, 0, 0,
> +                                        m->fd_drm_fourcc, modifier,
> +                                        fd, false, m->fd_flags &
> +                                        VIRTIO_GPU_RESOURCE_FLAG_Y_0_TOP);
> +
>          dpy_gl_scanout_dmabuf(con, dmabuf);
> +        g->dmabuf[m->scanout_id] = dmabuf;
>          break;
>      }
>      case VHOST_USER_GPU_DMABUF_UPDATE: {
> diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
> index e3f358b575..79eafc7289 100644
> --- a/hw/display/virtio-gpu-udmabuf.c
> +++ b/hw/display/virtio-gpu-udmabuf.c
> @@ -162,7 +162,8 @@ static void virtio_gpu_free_dmabuf(VirtIOGPU *g, VGPUDMABuf *dmabuf)
>      struct virtio_gpu_scanout *scanout;
>
>      scanout = &g->parent_obj.scanout[dmabuf->scanout_id];
> -    dpy_gl_release_dmabuf(scanout->con, &dmabuf->buf);
> +    dpy_gl_release_dmabuf(scanout->con, dmabuf->buf);
> +    g_clear_pointer(&dmabuf->buf, dpy_gl_qemu_dmabuf_free);
>      QTAILQ_REMOVE(&g->dmabuf.bufs, dmabuf, next);
>      g_free(dmabuf);
>  }
> @@ -181,17 +182,10 @@ static VGPUDMABuf
>      }
>
>      dmabuf = g_new0(VGPUDMABuf, 1);
> -    dmabuf->buf.width = r->width;
> -    dmabuf->buf.height = r->height;
> -    dmabuf->buf.stride = fb->stride;
> -    dmabuf->buf.x = r->x;
> -    dmabuf->buf.y = r->y;
> -    dmabuf->buf.backing_width = fb->width;
> -    dmabuf->buf.backing_height = fb->height;
> -    dmabuf->buf.fourcc = qemu_pixman_to_drm_format(fb->format);
> -    dmabuf->buf.fd = res->dmabuf_fd;
> -    dmabuf->buf.allow_fences = true;
> -    dmabuf->buf.draw_submitted = false;
> +    dmabuf->buf = dpy_gl_qemu_dmabuf_new(r->width, r->height, fb->stride,
> +                                         r->x, r->y, fb->width, fb->height,
> +                                         qemu_pixman_to_drm_format(fb->format),
> +                                         0, res->dmabuf_fd, false, 0);
>      dmabuf->scanout_id = scanout_id;
>      QTAILQ_INSERT_HEAD(&g->dmabuf.bufs, dmabuf, next);
>
> @@ -217,11 +211,11 @@ int virtio_gpu_update_dmabuf(VirtIOGPU *g,
>          old_primary = g->dmabuf.primary[scanout_id];
>      }
>
> -    width = dpy_gl_qemu_dmabuf_get_width(&new_primary->buf);
> -    height = dpy_gl_qemu_dmabuf_get_height(&new_primary->buf);
> +    width = dpy_gl_qemu_dmabuf_get_width(new_primary->buf);
> +    height = dpy_gl_qemu_dmabuf_get_height(new_primary->buf);
>      g->dmabuf.primary[scanout_id] = new_primary;
>      qemu_console_resize(scanout->con, width, height);
> -    dpy_gl_scanout_dmabuf(scanout->con, &new_primary->buf);
> +    dpy_gl_scanout_dmabuf(scanout->con, new_primary->buf);
>
>      if (old_primary) {
>          virtio_gpu_free_dmabuf(g, old_primary);
> diff --git a/hw/vfio/display.c b/hw/vfio/display.c
> index f9c39cbd51..7e26d9667f 100644
> --- a/hw/vfio/display.c
> +++ b/hw/vfio/display.c
> @@ -241,14 +241,11 @@ static VFIODMABuf *vfio_display_get_dmabuf(VFIOPCIDevice *vdev,
>
>      dmabuf = g_new0(VFIODMABuf, 1);
>      dmabuf->dmabuf_id  = plane.dmabuf_id;
> -    dmabuf->buf.width  = plane.width;
> -    dmabuf->buf.height = plane.height;
> -    dmabuf->buf.backing_width = plane.width;
> -    dmabuf->buf.backing_height = plane.height;
> -    dmabuf->buf.stride = plane.stride;
> -    dmabuf->buf.fourcc = plane.drm_format;
> -    dmabuf->buf.modifier = plane.drm_format_mod;
> -    dmabuf->buf.fd     = fd;
> +    dmabuf->buf = dpy_gl_qemu_dmabuf_new(plane.width, plane.height,
> +                                         plane.stride, 0, 0, plane.width,
> +                                         plane.height, plane.drm_format,
> +                                         plane.drm_format_mod, fd, false, 0);
> +
>      if (plane_type == DRM_PLANE_TYPE_CURSOR) {
>          vfio_display_update_cursor(dmabuf, &plane);
>      }
> @@ -263,8 +260,9 @@ static void vfio_display_free_one_dmabuf(VFIODisplay *dpy, VFIODMABuf *dmabuf)
>
>      QTAILQ_REMOVE(&dpy->dmabuf.bufs, dmabuf, next);
>
> -    fd = dpy_gl_qemu_dmabuf_get_fd(&dmabuf->buf);
> -    dpy_gl_release_dmabuf(dpy->con, &dmabuf->buf);
> +    fd = dpy_gl_qemu_dmabuf_get_fd(dmabuf->buf);
> +    dpy_gl_release_dmabuf(dpy->con, dmabuf->buf);
> +    g_clear_pointer(&dmabuf->buf, dpy_gl_qemu_dmabuf_free);
>      close(fd);
>      g_free(dmabuf);
>  }
> @@ -301,13 +299,13 @@ static void vfio_display_dmabuf_update(void *opaque)
>          return;
>      }
>
> -    width = dpy_gl_qemu_dmabuf_get_width(&primary->buf);
> -    height = dpy_gl_qemu_dmabuf_get_height(&primary->buf);
> +    width = dpy_gl_qemu_dmabuf_get_width(primary->buf);
> +    height = dpy_gl_qemu_dmabuf_get_height(primary->buf);
>
>      if (dpy->dmabuf.primary != primary) {
>          dpy->dmabuf.primary = primary;
>          qemu_console_resize(dpy->con, width, height);
> -        dpy_gl_scanout_dmabuf(dpy->con, &primary->buf);
> +        dpy_gl_scanout_dmabuf(dpy->con, primary->buf);
>          free_bufs = true;
>      }
>
> @@ -321,7 +319,7 @@ static void vfio_display_dmabuf_update(void *opaque)
>      if (cursor && (new_cursor || cursor->hot_updates)) {
>          bool have_hot = (cursor->hot_x != 0xffffffff &&
>                           cursor->hot_y != 0xffffffff);
> -        dpy_gl_cursor_dmabuf(dpy->con, &cursor->buf, have_hot,
> +        dpy_gl_cursor_dmabuf(dpy->con, cursor->buf, have_hot,
>                               cursor->hot_x, cursor->hot_y);
>          cursor->hot_updates = 0;
>      } else if (!cursor && new_cursor) {
> diff --git a/ui/console.c b/ui/console.c
> index d4ca9e6e0f..ea23fd8af6 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -1132,6 +1132,40 @@ void dpy_gl_cursor_position(QemuConsole *con,
>      }
>  }
>
> +QemuDmaBuf *dpy_gl_qemu_dmabuf_new(uint32_t width, uint32_t height,
> +                                   uint32_t stride, uint32_t x,
> +                                   uint32_t y, uint32_t backing_width,
> +                                   uint32_t backing_height, uint32_t fourcc,
> +                                   uint64_t modifier, uint32_t dmabuf_fd,
> +                                   bool allow_fences, bool y0_top) {
> +    QemuDmaBuf *dmabuf;
> +
> +    dmabuf = g_new0(QemuDmaBuf, 1);
> +
> +    dmabuf->width = width;
> +    dmabuf->height = height;
> +    dmabuf->stride = stride;
> +    dmabuf->x = x;
> +    dmabuf->y = y;
> +    dmabuf->backing_width = backing_width;
> +    dmabuf->backing_height = backing_height;
> +    dmabuf->fourcc = fourcc;
> +    dmabuf->modifier = modifier;
> +    dmabuf->fd = dmabuf_fd;
> +    dmabuf->allow_fences = allow_fences;
> +    dmabuf->y0_top = y0_top;
> +    dmabuf->fence_fd = -1;
> +
> +    return dmabuf;
> +}

After those changes you should be able to make QemuDmaBuf a private
struct (defined in the a .c). This can be an additional patch to ease
review.

> +
> +void dpy_gl_qemu_dmabuf_free(QemuDmaBuf *dmabuf)
> +{
> +    assert(dmabuf != NULL);
> +
> +    g_free(dmabuf);
> +}
> +
>  int32_t dpy_gl_qemu_dmabuf_get_fd(QemuDmaBuf *dmabuf)
>  {
>      assert(dmabuf != NULL);
> diff --git a/ui/dbus-listener.c b/ui/dbus-listener.c
> index c6c7d93753..85d779a45c 100644
> --- a/ui/dbus-listener.c
> +++ b/ui/dbus-listener.c
> @@ -442,28 +442,24 @@ static void dbus_scanout_texture(DisplayChangeListener *dcl,
>      trace_dbus_scanout_texture(tex_id, backing_y_0_top,
>                                 backing_width, backing_height, x, y, w, h);
>  #ifdef CONFIG_GBM
> -    QemuDmaBuf dmabuf = {
> -        .width = w,
> -        .height = h,
> -        .y0_top = backing_y_0_top,
> -        .x = x,
> -        .y = y,
> -        .backing_width = backing_width,
> -        .backing_height = backing_height,
> -    };
> +    int32_t fd;
> +    uint32_t stride, fourcc;
> +    uint64_t modifier;
> +    QemuDmaBuf *dmabuf;
>
>      assert(tex_id);
> -    dmabuf.fd = egl_get_fd_for_texture(
> -        tex_id, (EGLint *)&dmabuf.stride,
> -        (EGLint *)&dmabuf.fourcc,
> -        &dmabuf.modifier);
> -    if (dmabuf.fd < 0) {
> +    fd = egl_get_fd_for_texture(tex_id, (EGLint *)&stride, (EGLint *)&fourcc,
> +                                &modifier);
> +    if (fd < 0) {
>          error_report("%s: failed to get fd for texture", __func__);
>          return;
>      }
> +    dmabuf = dpy_gl_qemu_dmabuf_new(w, h, stride, x, y, backing_width,
> +                                    backing_height, fourcc, modifier, fd,
> +                                    false, backing_y_0_top);
>
> -    dbus_scanout_dmabuf(dcl, &dmabuf);
> -    close(dmabuf.fd);
> +    dbus_scanout_dmabuf(dcl, dmabuf);
> +    close(fd);
>  #endif
>
>  #ifdef WIN32
> --
> 2.34.1
>

Re: [PATCH v6 0/3] ui/console: Private QemuDmaBuf struct

[Daniel P. Berrangé]

On Tue, Apr 16, 2024 at 09:09:51PM -0700, dongwon.kim@intel.com wrote:
> From: Dongwon Kim <dongwon.kim@intel.com>
> 
> This series introduces privacy enhancements to the QemuDmaBuf struct
> and its contained data to bolster security. it accomplishes this by
> introducing of helper functions for allocating, deallocating, and
> accessing individual fields within the struct and replacing all direct
> references to individual fields in the struct with methods using helpers
> throughout the codebase.

This series feels incomplete wrt the stated goal, because
the QemuDmaBuf struct definition remains public in
console.h at the end.  Ideally only "typedef struct QemuDmaBuf"
should remain in the header, with the struct definiton private
in a .c file.  Is there something that prevents this final step
being done ?


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|