From 69562648f9c35382041d96dc267768cbcc008a41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Fri, 17 Nov 2023 18:03:16 +0400 Subject: vl: revert behaviour for -display none MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 1bec1cc0d ("ui/console: allow to override the default VC") changed the behaviour of the "-display none" option, so that it now creates a QEMU monitor on the terminal. "-display none" should not be tangled up with whether we create a monitor or a serial terminal; it should purely and only disable the graphical window. Changing its behaviour like this breaks command lines which, for example, use semihosting for their output and don't want a graphical window, as they now get a monitor they never asked for. It also breaks the command line we document for Xen in docs/system/i386/xen.html: $ ./qemu-system-x86_64 --accel kvm,xen-version=0x40011,kernel-irqchip=split \ -display none -chardev stdio,mux=on,id=char0,signal=off -mon char0 \ -device xen-console,chardev=char0 -drive file=${GUEST_IMAGE},if=xen qemu-system-x86_64: cannot use stdio by multiple character devices qemu-system-x86_64: could not connect serial device to character backend 'stdio' When qemu is compiled without PIXMAN, by default the serials aren't muxed with the monitor anymore on stdio. The serials are redirected to "null" instead, and the monitor isn't set up. Fixes: commit 1bec1cc0d ("ui/console: allow to override the default VC") Signed-off-by: Marc-André Lureau Tested-by: Peter Maydell Reviewed-by: Peter Maydell Tested-by: David Woodhouse Reviewed-by: David Woodhouse --- system/vl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/vl.c b/system/vl.c index 5af7ced2a1..14bf0cf0bf 100644 --- a/system/vl.c +++ b/system/vl.c @@ -1391,7 +1391,7 @@ static void qemu_create_default_devices(void) } } - if (nographic || (!vc && !is_daemonized() && isatty(STDOUT_FILENO))) { + if (nographic) { if (default_parallel) { add_device_config(DEV_PARALLEL, "null"); } -- cgit v1.2.3 From b7f1bb38b011efd13784e8781dafeedcc6e900a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Fri, 17 Nov 2023 14:55:07 +0400 Subject: ui: use "vc" chardev for dbus, gtk & spice-app MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Those display have their own implementation of "vc" chardev, which doesn't use pixman. They also don't implement the width/height/cols/rows options, so qemu_display_get_vc() should return a compatible argument. This patch was meant to be with the pixman series, when the "vc" field was introduced. It fixes a regression where VC are created on the tty (or null) instead of the display own "vc" implementation. Signed-off-by: Marc-André Lureau Acked-by: Thomas Huth --- ui/dbus.c | 1 + ui/gtk.c | 1 + ui/spice-app.c | 1 + 3 files changed, 3 insertions(+) diff --git a/ui/dbus.c b/ui/dbus.c index 866467ad2e..e08b5de064 100644 --- a/ui/dbus.c +++ b/ui/dbus.c @@ -518,6 +518,7 @@ static QemuDisplay qemu_display_dbus = { .type = DISPLAY_TYPE_DBUS, .early_init = early_dbus_init, .init = dbus_init, + .vc = "vc", }; static void register_dbus(void) diff --git a/ui/gtk.c b/ui/gtk.c index be047a41ad..810d7fc796 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -2534,6 +2534,7 @@ static QemuDisplay qemu_display_gtk = { .type = DISPLAY_TYPE_GTK, .early_init = early_gtk_display_init, .init = gtk_display_init, + .vc = "vc", }; static void register_gtk(void) diff --git a/ui/spice-app.c b/ui/spice-app.c index 405fb7f9f5..a10b4a58fe 100644 --- a/ui/spice-app.c +++ b/ui/spice-app.c @@ -220,6 +220,7 @@ static QemuDisplay qemu_display_spice_app = { .type = DISPLAY_TYPE_SPICE_APP, .early_init = spice_app_display_early_init, .init = spice_app_display_init, + .vc = "vc", }; static void register_spice_app(void) -- cgit v1.2.3 From 0e8823072e1d6c5320864f734d01f11210109320 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Wed, 8 Nov 2023 17:25:22 +0400 Subject: ui/console: fix default VC when there are no display MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When display is "none", we may still have remote displays (I think it would be simpler if VNC/Spice were regular display btw). Return the default VC then, and set them up to fix a regression when using remote display and it used the TTY instead. Fixes: https://gitlab.com/qemu-project/qemu/-/issues/1989 Fixes: commit 1bec1cc0d ("ui/console: allow to override the default VC") Reported-by: German Maglione Signed-off-by: Marc-André Lureau Acked-by: Thomas Huth --- ui/console.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/ui/console.c b/ui/console.c index 8e688d3569..7db921e3b7 100644 --- a/ui/console.c +++ b/ui/console.c @@ -1679,19 +1679,17 @@ void qemu_display_init(DisplayState *ds, DisplayOptions *opts) const char *qemu_display_get_vc(DisplayOptions *opts) { - assert(opts->type < DISPLAY_TYPE__MAX); - if (opts->type == DISPLAY_TYPE_NONE) { - return NULL; - } - assert(dpys[opts->type] != NULL); - if (dpys[opts->type]->vc) { - return dpys[opts->type]->vc; - } else { #ifdef CONFIG_PIXMAN - return "vc:80Cx24C"; + const char *vc = "vc:80Cx24C"; +#else + const char *vc = NULL; #endif + + assert(opts->type < DISPLAY_TYPE__MAX); + if (dpys[opts->type] && dpys[opts->type]->vc) { + vc = dpys[opts->type]->vc; } - return NULL; + return vc; } void qemu_display_help(void) -- cgit v1.2.3 From ff2a5bed5f28cc59b25de76cb90196329da6c1f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Fri, 17 Nov 2023 15:15:13 +0400 Subject: vl: add missing display_remote++ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We should also consider -display vnc= as setting up a remote display, and not attempt to add another default one. The display_remote++ in qemu_setup_display() isn't necessary at this point, but is there for completeness and further usages of the variable. Fixes: https://gitlab.com/qemu-project/qemu/-/issues/1988 Fixes: commit 484629fc81 ("vl: simplify display_remote logic ") Signed-off-by: Marc-André Lureau --- system/vl.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/system/vl.c b/system/vl.c index 14bf0cf0bf..da2654aa77 100644 --- a/system/vl.c +++ b/system/vl.c @@ -1110,6 +1110,7 @@ static void parse_display(const char *p) */ if (*opts == '=') { vnc_parse(opts + 1); + display_remote++; } else { error_report("VNC requires a display argument vnc="); exit(1); @@ -1359,6 +1360,7 @@ static void qemu_setup_display(void) dpy.type = DISPLAY_TYPE_NONE; #if defined(CONFIG_VNC) vnc_parse("localhost:0,to=99,id=default"); + display_remote++; #endif } } -- cgit v1.2.3 From e0c58720bfd8c0553f170b64717278b07438d2f5 Mon Sep 17 00:00:00 2001 From: Manos Pitsidianakis Date: Tue, 21 Nov 2023 11:38:39 +0200 Subject: ui/pixman-minimal.h: fix empty allocation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the minimal pixman API stub that is used when the real pixman dependency is missing a NULL dereference happens when virtio-gpu-rutabaga allocates a pixman image with bits = NULL and rowstride_bytes = zero. A buffer of rowstride_bytes * height is allocated which is NULL. However, in that scenario pixman calculates a new stride value based on given width, height and format size. This commit adds a helper function that performs the same logic as pixman. Signed-off-by: Manos Pitsidianakis Reviewed-by: Marc-André Lureau Message-Id: <20231121093840.2121195-1-manos.pitsidianakis@linaro.org> --- include/ui/pixman-minimal.h | 48 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/include/ui/pixman-minimal.h b/include/ui/pixman-minimal.h index efcf570c9e..6dd7de1c7e 100644 --- a/include/ui/pixman-minimal.h +++ b/include/ui/pixman-minimal.h @@ -113,6 +113,45 @@ typedef struct pixman_color { uint16_t alpha; } pixman_color_t; +static inline uint32_t *create_bits(pixman_format_code_t format, + int width, + int height, + int *rowstride_bytes) +{ + int stride = 0; + size_t buf_size = 0; + int bpp = PIXMAN_FORMAT_BPP(format); + + /* + * Calculate the following while checking for overflow truncation: + * stride = ((width * bpp + 0x1f) >> 5) * sizeof(uint32_t); + */ + + if (unlikely(__builtin_mul_overflow(width, bpp, &stride))) { + return NULL; + } + + if (unlikely(__builtin_add_overflow(stride, 0x1f, &stride))) { + return NULL; + } + + stride >>= 5; + + stride *= sizeof(uint32_t); + + if (unlikely(__builtin_mul_overflow((size_t) height, + (size_t) stride, + &buf_size))) { + return NULL; + } + + if (rowstride_bytes) { + *rowstride_bytes = stride; + } + + return g_malloc0(buf_size); +} + static inline pixman_image_t *pixman_image_create_bits(pixman_format_code_t format, int width, int height, @@ -123,13 +162,18 @@ static inline pixman_image_t *pixman_image_create_bits(pixman_format_code_t form i->width = width; i->height = height; - i->stride = rowstride_bytes ?: width * DIV_ROUND_UP(PIXMAN_FORMAT_BPP(format), 8); i->format = format; if (bits) { i->data = bits; } else { - i->free_me = i->data = g_malloc0(rowstride_bytes * height); + i->free_me = i->data = + create_bits(format, width, height, &rowstride_bytes); + if (width && height) { + assert(i->data); + } } + i->stride = rowstride_bytes ? rowstride_bytes : + width * DIV_ROUND_UP(PIXMAN_FORMAT_BPP(format), 8); i->ref_count = 1; return i; -- cgit v1.2.3