Ping -- does anybody have suggestions for the best fix for this regression ?
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
