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

["Marc-André Lureau" <marcandre.lureau@redhat.com>]

On Sat, Oct 5, 2024 at 12:59 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> On 2024/10/03 20:22, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@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.lureau@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.

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