Hi Peter

On Thu, Jun 11, 2026 at 12:42 PM Peter Maydell <[email protected]> wrote:
>
> Ping -- does anybody have suggestions for the best fix
> for this regression ?
>

Sorry I missed it, I will send fixes.
thanks

> thanks
> -- PMM
>
> On Fri, 29 May 2026 at 14:34, Peter Maydell <[email protected]> wrote:
> >
> > On Mon, 14 Oct 2024 at 14:41, <[email protected]> wrote:
> > >
> > > From: Marc-André Lureau <[email protected]>
> > >
> > > Use a common shareable type for win32 & unix, and helper functions.
> > > This simplify the code as it avoids a lot of #ifdef'ery.
> > >
> > > Note: if it helps review, commits could be reordered to introduce the
> > > common type before introducing shareable memory for unix.
> > >
> > > Suggested-by: Akihiko Odaki <[email protected]>
> > > Signed-off-by: Marc-André Lureau <[email protected]>
> > > Reviewed-by: Akihiko Odaki <[email protected]>
> > > Message-ID: <[email protected]>
> >
> > Hi; this is a rather old commit (now 1ff788db9781615b in git),
> > but I have just noticed that it seems to break the usage of
> > qemu_create_displaysurface_from() by hw/display/xlnx_dp.c.
> >
> >
> > > @@ -517,14 +482,25 @@ 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);
> >
> > Before this refactoring, qemu_create_displaysurface_from() always
> > calls pixman_image_create_bits(). That pixman function allows
> > you to pass NULL to mean "allocate me the memory for the data".
> > If you do that, pixman will always compute the stride for the
> > data (by looking at the pixel format and the width), so the
> > input linesize here is ignored. The xlnx_dp.c code takes
> > advantage of this in calls like:
> >
> >         s->g_plane.surface
> >                 = qemu_create_displaysurface_from(width, height,
> >                                                   s->g_plane.format, 0, 
> > NULL);
> >
> > > -    assert(surface->image != NULL);
> > > +    surface->share_handle = SHAREABLE_NONE;
> > >
> > > +    if (data) {
> > > +        surface->image = pixman_image_create_bits(format,
> > > +                                                  width, height,
> > > +                                                  (void *)data, 
> > > linesize);
> > > +    } else {
> > > +        qemu_pixman_image_new_shareable(&surface->image,
> > > +                                        &surface->share_handle,
> > > +                                        "displaysurface",
> > > +                                        format,
> > > +                                        width,
> > > +                                        height,
> > > +                                        linesize,
> > > +                                        &error_abort);
> > > +        surface->flags = QEMU_ALLOCATED_FLAG;
> > > +    }
> >
> > In the refactored version, although non-NULL data still
> > results in a call to pixman_image_create_bits(), now a NULL
> > data pointer means we call qemu_pixman_image_new_shareable().
> >
> > > +bool
> > > +qemu_pixman_image_new_shareable(pixman_image_t **image,
> > > +                                qemu_pixman_shareable *handle,
> > > +                                const char *name,
> > > +                                pixman_format_code_t format,
> > > +                                int width,
> > > +                                int height,
> > > +                                int rowstride_bytes,
> > > +                                Error **errp)
> > > +{
> > > +    ERRP_GUARD();
> > > +    size_t size = height * rowstride_bytes;
> >
> > This function assumes that rowstride_bytes is not zero,
> > so the previously-working "pass 0, NULL" now results in
> > a size value of 0 here and an attempt to allocate 0 bytes
> > in qemu_pixman_shareable_alloc(), which winds up triggering
> > an abort in qemu_memfd_alloc().
> >
> > Presumably:
> > (1) qemu_pixman_image_new_shareable() should calculate the
> > rowstride_bytes as pixman does, rather than requiring it
> > to be passed in
> > (2) qemu_memfd_alloc() ought not to abort on zero size
> >
> >
> > For a repro case:
> >
> > $ cat /tmp/xlnx-dp.txt
> > writel 0xfd4a0194 0x00000780
> > writel 0xfd4a0198 0x00000438
> > $ ./build/arm/qemu-system-aarch64 -accel qtest -M xlnx-zcu102
> > -display none -chardev
> > file,id=repro,path=/dev/null,input-path=/tmp/xlnx-dp.txt -qtest
> > chardev:repro
> > [I 0.000000] OPENED
> > pulseaudio: set_sink_input_volume() failed Reason: Invalid argument
> > pulseaudio: set_sink_input_mute() failed Reason: Invalid argument
> > [R +0.121066] writel 0xfd4a0194 0x00000780
> > [S +0.121092] OK
> > [R +0.121096] writel 0xfd4a0198 0x00000438
> > Unexpected error in qemu_memfd_alloc() at ../../util/memfd.c:143:
> > qemu-system-aarch64: failed to allocate shared memory: Invalid argument
> > Aborted (core dumped)
> >
> > thanks
> > -- PMM
>


Reply via email to