Re: [PATCH 12/16] ui/surface: allocate shared memory on !win32

[Akihiko Odaki]

On 2024/10/07 20:47, Marc-André Lureau wrote:
On Sat, Oct 5, 2024 at 12:59 PM Akihiko Odaki <akihiko.od...@daynix.com> wrote:

On 2024/10/03 20:22, marcandre.lur...@redhat.com wrote:
From: Marc-André Lureau <marcandre.lur...@redhat.com>

Use qemu_memfd_alloc() to allocate the display surface memory, which
will fallback on tmpfile/mmap() on systems without memfd, and allow to
share the display with other processes.

This is similar to how display memory is allocated on win32 since commit
09b4c198 ("console/win32: allocate shareable display surface").

Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
---
   include/ui/surface.h |  8 ++++++++
   ui/console.c         | 30 ++++++++++++++++++++++++++++--
   2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/include/ui/surface.h b/include/ui/surface.h
index 345b19169d..dacf12ffe2 100644
--- a/include/ui/surface.h
+++ b/include/ui/surface.h
@@ -23,6 +23,10 @@ typedef struct DisplaySurface {
       GLenum gltype;
       GLuint texture;
   #endif
+#ifndef WIN32
+    int shmfd;
+    uint32_t shmfd_offset;
+#endif
   #ifdef WIN32
       HANDLE handle;

What about defining a new struct that contains either of shmfd or
handle? We can then have a unified set of functions that uses the struct
to allocate/free a shared pixman image and to set one to DisplaySurface.

Well, that structure is pretty much DisplaySurface. I am not sure if
it's valuable to introduce another abstraction.

I was thinking using the abstraction for virtio-gpu to save ifdefs. It 
already has resource_set_image_destroy to abstract away the resource 
deallocation so ui/console.c can absorb it.



       uint32_t handle_offset;
@@ -37,6 +41,10 @@ DisplaySurface *qemu_create_displaysurface_from(int width, 
int height,
   DisplaySurface *qemu_create_displaysurface_pixman(pixman_image_t *image);
   DisplaySurface *qemu_create_placeholder_surface(int w, int h,
                                                   const char *msg);
+#ifndef WIN32
+void qemu_displaysurface_set_shmfd(DisplaySurface *surface,
+                                   int shmfd, uint32_t offset);
+#endif
   #ifdef WIN32
   void qemu_displaysurface_win32_set_handle(DisplaySurface *surface,
                                             HANDLE h, uint32_t offset);
diff --git a/ui/console.c b/ui/console.c
index fdd76c2be4..56f2462c3d 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -37,6 +37,7 @@
   #include "trace.h"
   #include "exec/memory.h"
   #include "qom/object.h"
+#include "qemu/memfd.h"

   #include "console-priv.h"

@@ -452,6 +453,17 @@ qemu_graphic_console_init(Object *obj)
   {
   }

+#ifndef WIN32
+void qemu_displaysurface_set_shmfd(DisplaySurface *surface,
+                                   int shmfd, uint32_t offset)
+{
+    assert(surface->shmfd == -1);
+
+    surface->shmfd = shmfd;
+    surface->shmfd_offset = offset;
+}
+#endif
+
   #ifdef WIN32
   void qemu_displaysurface_win32_set_handle(DisplaySurface *surface,
                                             HANDLE h, uint32_t offset)
@@ -469,12 +481,16 @@ DisplaySurface *qemu_create_displaysurface(int width, int 
height)
       void *bits = NULL;
   #ifdef WIN32
       HANDLE handle = NULL;
+#else
+    int shmfd = -1;
   #endif

       trace_displaysurface_create(width, height);

   #ifdef WIN32
       bits = qemu_win32_map_alloc(width * height * 4, &handle, &error_abort);
+#else
+    bits = qemu_memfd_alloc("displaysurface", width * height * 4, 0, &shmfd, 
&error_abort);
   #endif

       surface = qemu_create_displaysurface_from(
@@ -486,9 +502,13 @@ DisplaySurface *qemu_create_displaysurface(int width, int 
height)

   #ifdef WIN32
       qemu_displaysurface_win32_set_handle(surface, handle, 0);
-    pixman_image_set_destroy_function(surface->image,
-                                      qemu_pixman_shared_image_destroy, 
handle);
+    void *data = handle;
+#else
+    qemu_displaysurface_set_shmfd(surface, shmfd, 0);
+    void *data = GINT_TO_POINTER(shmfd);
   #endif
+    pixman_image_set_destroy_function(surface->image, 
qemu_pixman_shared_image_destroy, data);
+
       return surface;
   }

@@ -499,6 +519,9 @@ DisplaySurface *qemu_create_displaysurface_from(int width, 
int height,
       DisplaySurface *surface = g_new0(DisplaySurface, 1);

       trace_displaysurface_create_from(surface, width, height, format);
+#ifndef WIN32
+    surface->shmfd = -1;
+#endif
       surface->image = pixman_image_create_bits(format,
                                                 width, height,
                                                 (void *)data, linesize);
@@ -512,6 +535,9 @@ DisplaySurface 
*qemu_create_displaysurface_pixman(pixman_image_t *image)
       DisplaySurface *surface = g_new0(DisplaySurface, 1);

       trace_displaysurface_create_pixman(surface);
+#ifndef WIN32
+    surface->shmfd = -1;
+#endif
       surface->image = pixman_image_ref(image);

       return surface;