From 67ae04273736f103b31fb5d1f9e5170dfee3d64c Mon Sep 17 00:00:00 2001 From: Carwyn Ellis Date: Sun, 6 Feb 2022 18:39:56 +0000 Subject: [PATCH 1/9] hw/display/vmware_vga: do not discard screen updates In certain circumstances, typically when there is lots changing on the screen, updates will be discarded resulting in garbled output. This change simplifies the traversal of the display update FIFO queue when applying updates. We just track the queue length and iterate up to the end of the queue. Additionally when adding updates to the queue, if the buffer reaches capacity we force a flush before accepting further events. Signed-off-by: Carwyn Ellis Message-Id: <20220206183956.10694-3-carwynellis@gmail.com> Signed-off-by: Gerd Hoffmann --- hw/display/trace-events | 1 + hw/display/vmware_vga.c | 41 +++++++++++++++++++++++------------------ 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/hw/display/trace-events b/hw/display/trace-events index 91efc88f04..0c0ffcbe42 100644 --- a/hw/display/trace-events +++ b/hw/display/trace-events @@ -24,6 +24,7 @@ vmware_setmode(uint32_t w, uint32_t h, uint32_t bpp) "%dx%d @ %d bpp" vmware_verify_rect_less_than_zero(const char *name, const char *param, int x) "%s: %s was < 0 (%d)" vmware_verify_rect_greater_than_bound(const char *name, const char *param, int bound, int x) "%s: %s was > %d (%d)" vmware_verify_rect_surface_bound_exceeded(const char *name, const char *component, int bound, const char *param1, int value1, const char *param2, int value2) "%s: %s > %d (%s: %d, %s: %d)" +vmware_update_rect_delayed_flush(void) "display update FIFO full - forcing flush" # virtio-gpu-base.c virtio_gpu_features(bool virgl) "virgl %d" diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c index 45d06cbe25..cedbbde522 100644 --- a/hw/display/vmware_vga.c +++ b/hw/display/vmware_vga.c @@ -80,7 +80,7 @@ struct vmsvga_state_s { struct vmsvga_rect_s { int x, y, w, h; } redraw_fifo[REDRAW_FIFO_LEN]; - int redraw_fifo_first, redraw_fifo_last; + int redraw_fifo_last; }; #define TYPE_VMWARE_SVGA "vmware-svga" @@ -380,33 +380,39 @@ static inline void vmsvga_update_rect(struct vmsvga_state_s *s, dpy_gfx_update(s->vga.con, x, y, w, h); } -static inline void vmsvga_update_rect_delayed(struct vmsvga_state_s *s, - int x, int y, int w, int h) -{ - struct vmsvga_rect_s *rect = &s->redraw_fifo[s->redraw_fifo_last++]; - - s->redraw_fifo_last &= REDRAW_FIFO_LEN - 1; - rect->x = x; - rect->y = y; - rect->w = w; - rect->h = h; -} - static inline void vmsvga_update_rect_flush(struct vmsvga_state_s *s) { struct vmsvga_rect_s *rect; if (s->invalidated) { - s->redraw_fifo_first = s->redraw_fifo_last; + s->redraw_fifo_last = 0; return; } /* Overlapping region updates can be optimised out here - if someone * knows a smart algorithm to do that, please share. */ - while (s->redraw_fifo_first != s->redraw_fifo_last) { - rect = &s->redraw_fifo[s->redraw_fifo_first++]; - s->redraw_fifo_first &= REDRAW_FIFO_LEN - 1; + for (int i = 0; i < s->redraw_fifo_last; i++) { + rect = &s->redraw_fifo[i]; vmsvga_update_rect(s, rect->x, rect->y, rect->w, rect->h); } + + s->redraw_fifo_last = 0; +} + +static inline void vmsvga_update_rect_delayed(struct vmsvga_state_s *s, + int x, int y, int w, int h) +{ + + if (s->redraw_fifo_last >= REDRAW_FIFO_LEN) { + trace_vmware_update_rect_delayed_flush(); + vmsvga_update_rect_flush(s); + } + + struct vmsvga_rect_s *rect = &s->redraw_fifo[s->redraw_fifo_last++]; + + rect->x = x; + rect->y = y; + rect->w = w; + rect->h = h; } #ifdef HW_RECT_ACCEL @@ -1161,7 +1167,6 @@ static void vmsvga_reset(DeviceState *dev) s->config = 0; s->svgaid = SVGA_ID; s->cursor.on = 0; - s->redraw_fifo_first = 0; s->redraw_fifo_last = 0; s->syncing = 0; From 95f8510ef428f988897176b9585b8ba1432f939f Mon Sep 17 00:00:00 2001 From: Kshitij Suri Date: Fri, 8 Apr 2022 07:13:34 +0000 Subject: [PATCH 2/9] Replacing CONFIG_VNC_PNG with CONFIG_PNG MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Libpng is only detected if VNC is enabled currently. This patch adds a generalised png option in the meson build which is aimed to replace use of CONFIG_VNC_PNG with CONFIG_PNG. Signed-off-by: Kshitij Suri Reviewed-by: Daniel P. Berrangé Message-Id: <20220408071336.99839-2-kshitij.suri@nutanix.com> [ kraxel: add meson-buildoptions.sh updates ] [ kraxel: fix centos8 testcase ] [ kraxel: update --enable-vnc-png too ] Signed-off-by: Gerd Hoffmann --enable-vnc-png fixup Signed-off-by: Gerd Hoffmann --- meson.build | 12 +++++++----- meson_options.txt | 4 ++-- .../ci/org.centos/stream/8/x86_64/configure | 4 ++-- scripts/coverity-scan/run-coverity-scan | 2 +- scripts/meson-buildoptions.sh | 6 +++--- ui/vnc-enc-tight.c | 18 +++++++++--------- ui/vnc.c | 4 ++-- ui/vnc.h | 2 +- 8 files changed, 27 insertions(+), 25 deletions(-) diff --git a/meson.build b/meson.build index d083c6b7bf..0c38e491f4 100644 --- a/meson.build +++ b/meson.build @@ -1115,14 +1115,16 @@ if gtkx11.found() x11 = dependency('x11', method: 'pkg-config', required: gtkx11.found(), kwargs: static_kwargs) endif -vnc = not_found png = not_found +if get_option('png').allowed() and have_system + png = dependency('libpng', required: get_option('png'), + method: 'pkg-config', kwargs: static_kwargs) +endif +vnc = not_found jpeg = not_found sasl = not_found if get_option('vnc').allowed() and have_system vnc = declare_dependency() # dummy dependency - png = dependency('libpng', required: get_option('vnc_png'), - method: 'pkg-config', kwargs: static_kwargs) jpeg = dependency('libjpeg', required: get_option('vnc_jpeg'), method: 'pkg-config', kwargs: static_kwargs) sasl = cc.find_library('sasl2', has_headers: ['sasl/sasl.h'], @@ -1554,9 +1556,9 @@ config_host_data.set('CONFIG_TPM', have_tpm) config_host_data.set('CONFIG_USB_LIBUSB', libusb.found()) config_host_data.set('CONFIG_VDE', vde.found()) config_host_data.set('CONFIG_VHOST_USER_BLK_SERVER', have_vhost_user_blk_server) +config_host_data.set('CONFIG_PNG', png.found()) config_host_data.set('CONFIG_VNC', vnc.found()) config_host_data.set('CONFIG_VNC_JPEG', jpeg.found()) -config_host_data.set('CONFIG_VNC_PNG', png.found()) config_host_data.set('CONFIG_VNC_SASL', sasl.found()) config_host_data.set('CONFIG_VIRTFS', have_virtfs) config_host_data.set('CONFIG_VTE', vte.found()) @@ -3667,11 +3669,11 @@ summary_info += {'curses support': curses} summary_info += {'virgl support': virgl} summary_info += {'curl support': curl} summary_info += {'Multipath support': mpathpersist} +summary_info += {'PNG support': png} summary_info += {'VNC support': vnc} if vnc.found() summary_info += {'VNC SASL support': sasl} summary_info += {'VNC JPEG support': jpeg} - summary_info += {'VNC PNG support': png} endif if targetos not in ['darwin', 'haiku', 'windows'] summary_info += {'OSS support': oss} diff --git a/meson_options.txt b/meson_options.txt index 52b11cead4..d85734f8e6 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -177,12 +177,12 @@ option('vde', type : 'feature', value : 'auto', description: 'vde network backend support') option('virglrenderer', type : 'feature', value : 'auto', description: 'virgl rendering support') +option('png', type : 'feature', value : 'auto', + description: 'PNG support with libpng') option('vnc', type : 'feature', value : 'auto', description: 'VNC server') option('vnc_jpeg', type : 'feature', value : 'auto', description: 'JPEG lossy compression for VNC server') -option('vnc_png', type : 'feature', value : 'auto', - description: 'PNG compression for VNC server') option('vnc_sasl', type : 'feature', value : 'auto', description: 'SASL authentication for VNC server') option('vte', type : 'feature', value : 'auto', diff --git a/scripts/ci/org.centos/stream/8/x86_64/configure b/scripts/ci/org.centos/stream/8/x86_64/configure index 9850dd4444..08225ee514 100755 --- a/scripts/ci/org.centos/stream/8/x86_64/configure +++ b/scripts/ci/org.centos/stream/8/x86_64/configure @@ -142,7 +142,7 @@ --disable-virtiofsd \ --disable-vnc \ --disable-vnc-jpeg \ ---disable-vnc-png \ +--disable-png \ --disable-vnc-sasl \ --disable-vte \ --disable-vvfat \ @@ -200,7 +200,7 @@ --enable-vhost-vdpa \ --enable-vhost-vsock \ --enable-vnc \ ---enable-vnc-png \ +--enable-png \ --enable-vnc-sasl \ --enable-werror \ --enable-xkbcommon diff --git a/scripts/coverity-scan/run-coverity-scan b/scripts/coverity-scan/run-coverity-scan index 181bdcb263..129672c86f 100755 --- a/scripts/coverity-scan/run-coverity-scan +++ b/scripts/coverity-scan/run-coverity-scan @@ -394,7 +394,7 @@ echo "Configuring..." --enable-opengl --enable-vte --enable-gnutls \ --enable-nettle --enable-curses --enable-curl \ --audio-drv-list=oss,alsa,sdl,pa --enable-virtfs \ - --enable-vnc --enable-vnc-sasl --enable-vnc-jpeg --enable-vnc-png \ + --enable-vnc --enable-vnc-sasl --enable-vnc-jpeg --enable-png \ --enable-xen --enable-brlapi \ --enable-linux-aio --enable-attr \ --enable-cap-ng --enable-trace-backends=log --enable-spice --enable-rbd \ diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh index 1e26f4571e..ef0dcd4a77 100644 --- a/scripts/meson-buildoptions.sh +++ b/scripts/meson-buildoptions.sh @@ -94,6 +94,7 @@ meson_options_help() { printf "%s\n" ' oss OSS sound support' printf "%s\n" ' pa PulseAudio sound support' printf "%s\n" ' parallels parallels image format support' + printf "%s\n" ' png PNG support with libpng' printf "%s\n" ' qcow1 qcow1 image format support' printf "%s\n" ' qed qed image format support' printf "%s\n" ' qga-vss build QGA VSS support (broken with MinGW)' @@ -123,7 +124,6 @@ meson_options_help() { printf "%s\n" ' virtiofsd build virtiofs daemon (virtiofsd)' printf "%s\n" ' vnc VNC server' printf "%s\n" ' vnc-jpeg JPEG lossy compression for VNC server' - printf "%s\n" ' vnc-png PNG compression for VNC server' printf "%s\n" ' vnc-sasl SASL authentication for VNC server' printf "%s\n" ' vte vte support for the gtk UI' printf "%s\n" ' vvfat vvfat image format support' @@ -277,6 +277,8 @@ _meson_option_parse() { --disable-pa) printf "%s" -Dpa=disabled ;; --enable-parallels) printf "%s" -Dparallels=enabled ;; --disable-parallels) printf "%s" -Dparallels=disabled ;; + --enable-png) printf "%s" -Dpng=enabled ;; + --disable-png) printf "%s" -Dpng=disabled ;; --enable-profiler) printf "%s" -Dprofiler=true ;; --disable-profiler) printf "%s" -Dprofiler=false ;; --enable-qcow1) printf "%s" -Dqcow1=enabled ;; @@ -347,8 +349,6 @@ _meson_option_parse() { --disable-vnc) printf "%s" -Dvnc=disabled ;; --enable-vnc-jpeg) printf "%s" -Dvnc_jpeg=enabled ;; --disable-vnc-jpeg) printf "%s" -Dvnc_jpeg=disabled ;; - --enable-vnc-png) printf "%s" -Dvnc_png=enabled ;; - --disable-vnc-png) printf "%s" -Dvnc_png=disabled ;; --enable-vnc-sasl) printf "%s" -Dvnc_sasl=enabled ;; --disable-vnc-sasl) printf "%s" -Dvnc_sasl=disabled ;; --enable-vte) printf "%s" -Dvte=enabled ;; diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c index 5a4b8a4fc0..09200d71b8 100644 --- a/ui/vnc-enc-tight.c +++ b/ui/vnc-enc-tight.c @@ -32,7 +32,7 @@ INT32 definitions between jmorecfg.h (included by jpeglib.h) and Win32 basetsd.h (included by windows.h). */ -#ifdef CONFIG_VNC_PNG +#ifdef CONFIG_PNG /* The following define is needed by pngconf.h. Otherwise it won't compile, because setjmp.h was already included by osdep.h. */ #define PNG_SKIP_SETJMP_CHECK @@ -95,7 +95,7 @@ static const struct { }; #endif -#ifdef CONFIG_VNC_PNG +#ifdef CONFIG_PNG static const struct { int png_zlib_level, png_filters; } tight_png_conf[] = { @@ -919,7 +919,7 @@ static int send_full_color_rect(VncState *vs, int x, int y, int w, int h) int stream = 0; ssize_t bytes; -#ifdef CONFIG_VNC_PNG +#ifdef CONFIG_PNG if (tight_can_send_png_rect(vs, w, h)) { return send_png_rect(vs, x, y, w, h, NULL); } @@ -966,7 +966,7 @@ static int send_mono_rect(VncState *vs, int x, int y, int stream = 1; int level = tight_conf[vs->tight->compression].mono_zlib_level; -#ifdef CONFIG_VNC_PNG +#ifdef CONFIG_PNG if (tight_can_send_png_rect(vs, w, h)) { int ret; int bpp = vs->client_pf.bytes_per_pixel * 8; @@ -1020,7 +1020,7 @@ static int send_mono_rect(VncState *vs, int x, int y, struct palette_cb_priv { VncState *vs; uint8_t *header; -#ifdef CONFIG_VNC_PNG +#ifdef CONFIG_PNG png_colorp png_palette; #endif }; @@ -1082,7 +1082,7 @@ static int send_palette_rect(VncState *vs, int x, int y, int colors; ssize_t bytes; -#ifdef CONFIG_VNC_PNG +#ifdef CONFIG_PNG if (tight_can_send_png_rect(vs, w, h)) { return send_png_rect(vs, x, y, w, h, palette); } @@ -1233,7 +1233,7 @@ static int send_jpeg_rect(VncState *vs, int x, int y, int w, int h, int quality) /* * PNG compression stuff. */ -#ifdef CONFIG_VNC_PNG +#ifdef CONFIG_PNG static void write_png_palette(int idx, uint32_t pix, void *opaque) { struct palette_cb_priv *priv = opaque; @@ -1379,7 +1379,7 @@ static int send_png_rect(VncState *vs, int x, int y, int w, int h, buffer_reset(&vs->tight->png); return 1; } -#endif /* CONFIG_VNC_PNG */ +#endif /* CONFIG_PNG */ static void vnc_tight_start(VncState *vs) { @@ -1706,7 +1706,7 @@ void vnc_tight_clear(VncState *vs) #ifdef CONFIG_VNC_JPEG buffer_free(&vs->tight->jpeg); #endif -#ifdef CONFIG_VNC_PNG +#ifdef CONFIG_PNG buffer_free(&vs->tight->png); #endif } diff --git a/ui/vnc.c b/ui/vnc.c index badf1d7664..7d55e1500a 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -2165,7 +2165,7 @@ static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings) vs->features |= VNC_FEATURE_TIGHT_MASK; vs->vnc_encoding = enc; break; -#ifdef CONFIG_VNC_PNG +#ifdef CONFIG_PNG case VNC_ENCODING_TIGHT_PNG: vs->features |= VNC_FEATURE_TIGHT_PNG_MASK; vs->vnc_encoding = enc; @@ -3256,7 +3256,7 @@ static void vnc_connect(VncDisplay *vd, QIOChannelSocket *sioc, #ifdef CONFIG_VNC_JPEG buffer_init(&vs->tight->jpeg, "vnc-tight-jpeg/%p", sioc); #endif -#ifdef CONFIG_VNC_PNG +#ifdef CONFIG_PNG buffer_init(&vs->tight->png, "vnc-tight-png/%p", sioc); #endif buffer_init(&vs->zlib.zlib, "vnc-zlib/%p", sioc); diff --git a/ui/vnc.h b/ui/vnc.h index a7149831f9..a60fb13115 100644 --- a/ui/vnc.h +++ b/ui/vnc.h @@ -201,7 +201,7 @@ typedef struct VncTight { #ifdef CONFIG_VNC_JPEG Buffer jpeg; #endif -#ifdef CONFIG_VNC_PNG +#ifdef CONFIG_PNG Buffer png; #endif int levels[4]; From 9a0a119a382867dc9a5c2ae9348003bf79d84af2 Mon Sep 17 00:00:00 2001 From: Kshitij Suri Date: Fri, 8 Apr 2022 07:13:35 +0000 Subject: [PATCH 3/9] Added parameter to take screenshot with screendump as PNG MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently screendump only supports PPM format, which is un-compressed. Added a "format" parameter to QMP and HMP screendump command to support PNG image capture using libpng. QMP example usage: { "execute": "screendump", "arguments": { "filename": "/tmp/image", "format":"png" } } HMP example usage: screendump /tmp/image -f png Resolves: https://gitlab.com/qemu-project/qemu/-/issues/718 Signed-off-by: Kshitij Suri Reviewed-by: Daniel P. Berrangé Acked-by: Markus Armbruster Acked-by: Dr. David Alan Gilbert Message-Id: <20220408071336.99839-3-kshitij.suri@nutanix.com> Signed-off-by: Gerd Hoffmann --- hmp-commands.hx | 11 ++--- monitor/hmp-cmds.c | 12 +++++- qapi/ui.json | 24 +++++++++-- ui/console.c | 101 +++++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 136 insertions(+), 12 deletions(-) diff --git a/hmp-commands.hx b/hmp-commands.hx index 8476277aa9..808020d005 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -244,11 +244,12 @@ ERST { .name = "screendump", - .args_type = "filename:F,device:s?,head:i?", - .params = "filename [device [head]]", - .help = "save screen from head 'head' of display device 'device' " - "into PPM image 'filename'", - .cmd = hmp_screendump, + .args_type = "filename:F,format:-fs,device:s?,head:i?", + .params = "filename [-f format] [device [head]]", + .help = "save screen from head 'head' of display device 'device'" + "in specified format 'format' as image 'filename'." + "Currently only 'png' and 'ppm' formats are supported.", + .cmd = hmp_screendump, .coroutine = true, }, diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c index 634968498b..2442bfa989 100644 --- a/monitor/hmp-cmds.c +++ b/monitor/hmp-cmds.c @@ -1720,9 +1720,19 @@ hmp_screendump(Monitor *mon, const QDict *qdict) const char *filename = qdict_get_str(qdict, "filename"); const char *id = qdict_get_try_str(qdict, "device"); int64_t head = qdict_get_try_int(qdict, "head", 0); + const char *input_format = qdict_get_try_str(qdict, "format"); Error *err = NULL; + ImageFormat format; - qmp_screendump(filename, id != NULL, id, id != NULL, head, &err); + format = qapi_enum_parse(&ImageFormat_lookup, input_format, + IMAGE_FORMAT_PPM, &err); + if (err) { + goto end; + } + + qmp_screendump(filename, id != NULL, id, id != NULL, head, + input_format != NULL, format, &err); +end: hmp_handle_error(mon, err); } diff --git a/qapi/ui.json b/qapi/ui.json index 13a8bb82aa..596f37fc37 100644 --- a/qapi/ui.json +++ b/qapi/ui.json @@ -157,12 +157,27 @@ ## { 'command': 'expire_password', 'boxed': true, 'data': 'ExpirePasswordOptions' } +## +# @ImageFormat: +# +# Supported image format types. +# +# @png: PNG format +# +# @ppm: PPM format +# +# Since: 7.1 +# +## +{ 'enum': 'ImageFormat', + 'data': ['ppm', 'png'] } + ## # @screendump: # -# Write a PPM of the VGA screen to a file. +# Capture the contents of a screen and write it to a file. # -# @filename: the path of a new PPM file to store the image +# @filename: the path of a new file to store the image # # @device: ID of the display device that should be dumped. If this parameter # is missing, the primary display will be used. (Since 2.12) @@ -171,6 +186,8 @@ # parameter is missing, head #0 will be used. Also note that the head # can only be specified in conjunction with the device ID. (Since 2.12) # +# @format: image format for screendump. (default: ppm) (Since 7.1) +# # Returns: Nothing on success # # Since: 0.14 @@ -183,7 +200,8 @@ # ## { 'command': 'screendump', - 'data': {'filename': 'str', '*device': 'str', '*head': 'int'}, + 'data': {'filename': 'str', '*device': 'str', '*head': 'int', + '*format': 'ImageFormat'}, 'coroutine': true } ## diff --git a/ui/console.c b/ui/console.c index 1752f2ec88..15d0f6affd 100644 --- a/ui/console.c +++ b/ui/console.c @@ -37,6 +37,9 @@ #include "exec/memory.h" #include "io/channel-file.h" #include "qom/object.h" +#ifdef CONFIG_PNG +#include +#endif #define DEFAULT_BACKSCROLL 512 #define CONSOLE_CURSOR_PERIOD 500 @@ -291,6 +294,89 @@ void graphic_hw_invalidate(QemuConsole *con) } } +#ifdef CONFIG_PNG +/** + * png_save: Take a screenshot as PNG + * + * Saves screendump as a PNG file + * + * Returns true for success or false for error. + * + * @fd: File descriptor for PNG file. + * @image: Image data in pixman format. + * @errp: Pointer to an error. + */ +static bool png_save(int fd, pixman_image_t *image, Error **errp) +{ + int width = pixman_image_get_width(image); + int height = pixman_image_get_height(image); + g_autofree png_struct *png_ptr = NULL; + g_autofree png_info *info_ptr = NULL; + g_autoptr(pixman_image_t) linebuf = + qemu_pixman_linebuf_create(PIXMAN_a8r8g8b8, width); + uint8_t *buf = (uint8_t *)pixman_image_get_data(linebuf); + FILE *f = fdopen(fd, "wb"); + int y; + if (!f) { + error_setg_errno(errp, errno, + "Failed to create file from file descriptor"); + return false; + } + + png_ptr = png_create_write_struct(PNG_LIBPNG_VER_STRING, NULL, + NULL, NULL); + if (!png_ptr) { + error_setg(errp, "PNG creation failed. Unable to write struct"); + fclose(f); + return false; + } + + info_ptr = png_create_info_struct(png_ptr); + + if (!info_ptr) { + error_setg(errp, "PNG creation failed. Unable to write info"); + fclose(f); + png_destroy_write_struct(&png_ptr, &info_ptr); + return false; + } + + png_init_io(png_ptr, f); + + png_set_IHDR(png_ptr, info_ptr, width, height, 8, + PNG_COLOR_TYPE_RGB_ALPHA, PNG_INTERLACE_NONE, + PNG_COMPRESSION_TYPE_BASE, PNG_FILTER_TYPE_BASE); + + png_write_info(png_ptr, info_ptr); + + for (y = 0; y < height; ++y) { + qemu_pixman_linebuf_fill(linebuf, image, width, 0, y); + png_write_row(png_ptr, buf); + } + qemu_pixman_image_unref(linebuf); + + png_write_end(png_ptr, NULL); + + png_destroy_write_struct(&png_ptr, &info_ptr); + + if (fclose(f) != 0) { + error_setg_errno(errp, errno, + "PNG creation failed. Unable to close file"); + return false; + } + + return true; +} + +#else /* no png support */ + +static bool png_save(int fd, pixman_image_t *image, Error **errp) +{ + error_setg(errp, "Enable PNG support with libpng for screendump"); + return false; +} + +#endif /* CONFIG_PNG */ + static bool ppm_save(int fd, pixman_image_t *image, Error **errp) { int width = pixman_image_get_width(image); @@ -329,7 +415,8 @@ static void graphic_hw_update_bh(void *con) /* Safety: coroutine-only, concurrent-coroutine safe, main thread only */ void coroutine_fn qmp_screendump(const char *filename, bool has_device, const char *device, - bool has_head, int64_t head, Error **errp) + bool has_head, int64_t head, + bool has_format, ImageFormat format, Error **errp) { g_autoptr(pixman_image_t) image = NULL; QemuConsole *con; @@ -385,8 +472,16 @@ qmp_screendump(const char *filename, bool has_device, const char *device, * yields and releases the BQL. It could produce corrupted dump, but * it should be otherwise safe. */ - if (!ppm_save(fd, image, errp)) { - qemu_unlink(filename); + if (has_format && format == IMAGE_FORMAT_PNG) { + /* PNG format specified for screendump */ + if (!png_save(fd, image, errp)) { + qemu_unlink(filename); + } + } else { + /* PPM format specified/default for screendump */ + if (!ppm_save(fd, image, errp)) { + qemu_unlink(filename); + } } } From d708f99d8508ca6de2ff07cf9723334e6c8ecacd Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Fri, 1 Apr 2022 17:39:34 +0300 Subject: [PATCH 4/9] ui/vnc: refactor arrays of addresses to SocketAddressList MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Let's use SocketAddressList instead of dynamic arrays. Benefits: - Automatic cleanup: don't need specific freeing function and drop some gotos. - Less indirection: no triple asterix anymore! - Prepare for the following commit, which will reuse new interface of vnc_display_listen(). Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Marc-André Lureau Reviewed-by: Daniel P. Berrangé Message-Id: <20220401143936.356460-2-vsementsov@openvz.org> Signed-off-by: Gerd Hoffmann --- ui/vnc.c | 129 ++++++++++++++++++++++--------------------------------- 1 file changed, 51 insertions(+), 78 deletions(-) diff --git a/ui/vnc.c b/ui/vnc.c index 7d55e1500a..77a660fccb 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -3820,30 +3820,19 @@ static int vnc_display_get_address(const char *addrstr, return ret; } -static void vnc_free_addresses(SocketAddress ***retsaddr, - size_t *retnsaddr) -{ - size_t i; - - for (i = 0; i < *retnsaddr; i++) { - qapi_free_SocketAddress((*retsaddr)[i]); - } - g_free(*retsaddr); - - *retsaddr = NULL; - *retnsaddr = 0; -} - static int vnc_display_get_addresses(QemuOpts *opts, bool reverse, - SocketAddress ***retsaddr, - size_t *retnsaddr, - SocketAddress ***retwsaddr, - size_t *retnwsaddr, + SocketAddressList **saddr_list_ret, + SocketAddressList **wsaddr_list_ret, Error **errp) { SocketAddress *saddr = NULL; SocketAddress *wsaddr = NULL; + g_autoptr(SocketAddressList) saddr_list = NULL; + SocketAddressList **saddr_tail = &saddr_list; + SocketAddress *single_saddr = NULL; + g_autoptr(SocketAddressList) wsaddr_list = NULL; + SocketAddressList **wsaddr_tail = &wsaddr_list; QemuOptsIter addriter; const char *addr; int to = qemu_opt_get_number(opts, "to", 0); @@ -3852,23 +3841,16 @@ static int vnc_display_get_addresses(QemuOpts *opts, bool ipv4 = qemu_opt_get_bool(opts, "ipv4", false); bool ipv6 = qemu_opt_get_bool(opts, "ipv6", false); int displaynum = -1; - int ret = -1; - - *retsaddr = NULL; - *retnsaddr = 0; - *retwsaddr = NULL; - *retnwsaddr = 0; addr = qemu_opt_get(opts, "vnc"); if (addr == NULL || g_str_equal(addr, "none")) { - ret = 0; - goto cleanup; + return 0; } if (qemu_opt_get(opts, "websocket") && !qcrypto_hash_supports(QCRYPTO_HASH_ALG_SHA1)) { error_setg(errp, "SHA1 hash support is required for websockets"); - goto cleanup; + return -1; } qemu_opt_iter_init(&addriter, opts, "vnc"); @@ -3879,7 +3861,7 @@ static int vnc_display_get_addresses(QemuOpts *opts, ipv4, ipv6, &saddr, errp); if (rv < 0) { - goto cleanup; + return -1; } /* Historical compat - first listen address can be used * to set the default websocket port @@ -3887,13 +3869,16 @@ static int vnc_display_get_addresses(QemuOpts *opts, if (displaynum == -1) { displaynum = rv; } - *retsaddr = g_renew(SocketAddress *, *retsaddr, *retnsaddr + 1); - (*retsaddr)[(*retnsaddr)++] = saddr; + QAPI_LIST_APPEND(saddr_tail, saddr); } - /* If we had multiple primary displays, we don't do defaults - * for websocket, and require explicit config instead. */ - if (*retnsaddr > 1) { + if (saddr_list && !saddr_list->next) { + single_saddr = saddr_list->value; + } else { + /* + * If we had multiple primary displays, we don't do defaults + * for websocket, and require explicit config instead. + */ displaynum = -1; } @@ -3903,57 +3888,50 @@ static int vnc_display_get_addresses(QemuOpts *opts, has_ipv4, has_ipv6, ipv4, ipv6, &wsaddr, errp) < 0) { - goto cleanup; + return -1; } /* Historical compat - if only a single listen address was * provided, then this is used to set the default listen * address for websocket too */ - if (*retnsaddr == 1 && - (*retsaddr)[0]->type == SOCKET_ADDRESS_TYPE_INET && + if (single_saddr && + single_saddr->type == SOCKET_ADDRESS_TYPE_INET && wsaddr->type == SOCKET_ADDRESS_TYPE_INET && g_str_equal(wsaddr->u.inet.host, "") && - !g_str_equal((*retsaddr)[0]->u.inet.host, "")) { + !g_str_equal(single_saddr->u.inet.host, "")) { g_free(wsaddr->u.inet.host); - wsaddr->u.inet.host = g_strdup((*retsaddr)[0]->u.inet.host); + wsaddr->u.inet.host = g_strdup(single_saddr->u.inet.host); } - *retwsaddr = g_renew(SocketAddress *, *retwsaddr, *retnwsaddr + 1); - (*retwsaddr)[(*retnwsaddr)++] = wsaddr; + QAPI_LIST_APPEND(wsaddr_tail, wsaddr); } - ret = 0; - cleanup: - if (ret < 0) { - vnc_free_addresses(retsaddr, retnsaddr); - vnc_free_addresses(retwsaddr, retnwsaddr); - } - return ret; + *saddr_list_ret = g_steal_pointer(&saddr_list); + *wsaddr_list_ret = g_steal_pointer(&wsaddr_list); + return 0; } static int vnc_display_connect(VncDisplay *vd, - SocketAddress **saddr, - size_t nsaddr, - SocketAddress **wsaddr, - size_t nwsaddr, + SocketAddressList *saddr_list, + SocketAddressList *wsaddr_list, Error **errp) { /* connect to viewer */ QIOChannelSocket *sioc = NULL; - if (nwsaddr != 0) { + if (wsaddr_list) { error_setg(errp, "Cannot use websockets in reverse mode"); return -1; } - if (nsaddr != 1) { + if (!saddr_list || saddr_list->next) { error_setg(errp, "Expected a single address in reverse mode"); return -1; } /* TODO SOCKET_ADDRESS_TYPE_FD when fd has AF_UNIX */ - vd->is_unix = saddr[0]->type == SOCKET_ADDRESS_TYPE_UNIX; + vd->is_unix = saddr_list->value->type == SOCKET_ADDRESS_TYPE_UNIX; sioc = qio_channel_socket_new(); qio_channel_set_name(QIO_CHANNEL(sioc), "vnc-reverse"); - if (qio_channel_socket_connect_sync(sioc, saddr[0], errp) < 0) { + if (qio_channel_socket_connect_sync(sioc, saddr_list->value, errp) < 0) { object_unref(OBJECT(sioc)); return -1; } @@ -3964,20 +3942,18 @@ static int vnc_display_connect(VncDisplay *vd, static int vnc_display_listen(VncDisplay *vd, - SocketAddress **saddr, - size_t nsaddr, - SocketAddress **wsaddr, - size_t nwsaddr, + SocketAddressList *saddr_list, + SocketAddressList *wsaddr_list, Error **errp) { - size_t i; + SocketAddressList *el; - if (nsaddr) { + if (saddr_list) { vd->listener = qio_net_listener_new(); qio_net_listener_set_name(vd->listener, "vnc-listen"); - for (i = 0; i < nsaddr; i++) { + for (el = saddr_list; el; el = el->next) { if (qio_net_listener_open_sync(vd->listener, - saddr[i], 1, + el->value, 1, errp) < 0) { return -1; } @@ -3987,12 +3963,12 @@ static int vnc_display_listen(VncDisplay *vd, vnc_listen_io, vd, NULL); } - if (nwsaddr) { + if (wsaddr_list) { vd->wslistener = qio_net_listener_new(); qio_net_listener_set_name(vd->wslistener, "vnc-ws-listen"); - for (i = 0; i < nwsaddr; i++) { + for (el = wsaddr_list; el; el = el->next) { if (qio_net_listener_open_sync(vd->wslistener, - wsaddr[i], 1, + el->value, 1, errp) < 0) { return -1; } @@ -4010,8 +3986,8 @@ void vnc_display_open(const char *id, Error **errp) { VncDisplay *vd = vnc_display_find(id); QemuOpts *opts = qemu_opts_find(&qemu_vnc_opts, id); - SocketAddress **saddr = NULL, **wsaddr = NULL; - size_t nsaddr, nwsaddr; + g_autoptr(SocketAddressList) saddr_list = NULL; + g_autoptr(SocketAddressList) wsaddr_list = NULL; const char *share, *device_id; QemuConsole *con; bool password = false; @@ -4036,8 +4012,8 @@ void vnc_display_open(const char *id, Error **errp) } reverse = qemu_opt_get_bool(opts, "reverse", false); - if (vnc_display_get_addresses(opts, reverse, &saddr, &nsaddr, - &wsaddr, &nwsaddr, errp) < 0) { + if (vnc_display_get_addresses(opts, reverse, &saddr_list, &wsaddr_list, + errp) < 0) { goto fail; } @@ -4219,16 +4195,16 @@ void vnc_display_open(const char *id, Error **errp) } qkbd_state_set_delay(vd->kbd, key_delay_ms); - if (saddr == NULL) { - goto cleanup; + if (saddr_list == NULL) { + return; } if (reverse) { - if (vnc_display_connect(vd, saddr, nsaddr, wsaddr, nwsaddr, errp) < 0) { + if (vnc_display_connect(vd, saddr_list, wsaddr_list, errp) < 0) { goto fail; } } else { - if (vnc_display_listen(vd, saddr, nsaddr, wsaddr, nwsaddr, errp) < 0) { + if (vnc_display_listen(vd, saddr_list, wsaddr_list, errp) < 0) { goto fail; } } @@ -4237,14 +4213,11 @@ void vnc_display_open(const char *id, Error **errp) vnc_display_print_local_addr(vd); } - cleanup: - vnc_free_addresses(&saddr, &nsaddr); - vnc_free_addresses(&wsaddr, &nwsaddr); + /* Success */ return; fail: vnc_display_close(vd); - goto cleanup; } void vnc_display_add_client(const char *id, int csock, bool skipauth) From abea19468e94821d098e546f0cafc9da759bc929 Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Fri, 1 Apr 2022 17:39:35 +0300 Subject: [PATCH 5/9] qapi/ui: add 'display-update' command for changing listen address MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add possibility to change addresses where VNC server listens for new connections. Prior to 6.0 this functionality was available through 'change' qmp command which was deleted. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Daniel P. Berrangé Message-Id: <20220401143936.356460-3-vsementsov@openvz.org> Signed-off-by: Gerd Hoffmann --- docs/about/removed-features.rst | 3 +- include/ui/console.h | 1 + monitor/qmp-cmds.c | 15 ++++++++ qapi/ui.json | 65 +++++++++++++++++++++++++++++++++ ui/vnc.c | 23 ++++++++++++ 5 files changed, 106 insertions(+), 1 deletion(-) diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst index 4b831ea291..b367418ca7 100644 --- a/docs/about/removed-features.rst +++ b/docs/about/removed-features.rst @@ -355,7 +355,8 @@ documentation of ``query-hotpluggable-cpus`` for additional details. ``change`` (removed in 6.0) ''''''''''''''''''''''''''' -Use ``blockdev-change-medium`` or ``change-vnc-password`` instead. +Use ``blockdev-change-medium`` or ``change-vnc-password`` or +``display-update`` instead. ``query-events`` (removed in 6.0) ''''''''''''''''''''''''''''''''' diff --git a/include/ui/console.h b/include/ui/console.h index 0f84861933..c44b28a972 100644 --- a/include/ui/console.h +++ b/include/ui/console.h @@ -518,6 +518,7 @@ int vnc_display_pw_expire(const char *id, time_t expires); void vnc_parse(const char *str); int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp); bool vnc_display_reload_certs(const char *id, Error **errp); +bool vnc_display_update(DisplayUpdateOptionsVNC *arg, Error **errp); /* input.c */ int index_from_key(const char *key, size_t key_length); diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c index 5e7302cbb9..1ebb89f46c 100644 --- a/monitor/qmp-cmds.c +++ b/monitor/qmp-cmds.c @@ -346,6 +346,21 @@ void qmp_display_reload(DisplayReloadOptions *arg, Error **errp) } } +void qmp_display_update(DisplayUpdateOptions *arg, Error **errp) +{ + switch (arg->type) { + case DISPLAY_UPDATE_TYPE_VNC: +#ifdef CONFIG_VNC + vnc_display_update(&arg->u.vnc, errp); +#else + error_setg(errp, "vnc is invalid, missing 'CONFIG_VNC'"); +#endif + break; + default: + abort(); + } +} + static int qmp_x_query_rdma_foreach(Object *obj, void *opaque) { RdmaProvider *rdma; diff --git a/qapi/ui.json b/qapi/ui.json index 596f37fc37..059302a5ef 100644 --- a/qapi/ui.json +++ b/qapi/ui.json @@ -1468,3 +1468,68 @@ { 'command': 'display-reload', 'data': 'DisplayReloadOptions', 'boxed' : true } + +## +# @DisplayUpdateType: +# +# Available DisplayUpdate types. +# +# @vnc: VNC display +# +# Since: 7.1 +# +## +{ 'enum': 'DisplayUpdateType', + 'data': ['vnc'] } + +## +# @DisplayUpdateOptionsVNC: +# +# Specify the VNC reload options. +# +# @addresses: If specified, change set of addresses +# to listen for connections. Addresses configured +# for websockets are not touched. +# +# Since: 7.1 +# +## +{ 'struct': 'DisplayUpdateOptionsVNC', + 'data': { '*addresses': ['SocketAddress'] } } + +## +# @DisplayUpdateOptions: +# +# Options of the display configuration reload. +# +# @type: Specify the display type. +# +# Since: 7.1 +# +## +{ 'union': 'DisplayUpdateOptions', + 'base': {'type': 'DisplayUpdateType'}, + 'discriminator': 'type', + 'data': { 'vnc': 'DisplayUpdateOptionsVNC' } } + +## +# @display-update: +# +# Update display configuration. +# +# Returns: Nothing on success. +# +# Since: 7.1 +# +# Example: +# +# -> { "execute": "display-update", +# "arguments": { "type": "vnc", "addresses": +# [ { "type": "inet", "host": "0.0.0.0", +# "port": "5901" } ] } } +# <- { "return": {} } +# +## +{ 'command': 'display-update', + 'data': 'DisplayUpdateOptions', + 'boxed' : true } diff --git a/ui/vnc.c b/ui/vnc.c index 77a660fccb..b02cb3f405 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -3981,6 +3981,29 @@ static int vnc_display_listen(VncDisplay *vd, return 0; } +bool vnc_display_update(DisplayUpdateOptionsVNC *arg, Error **errp) +{ + VncDisplay *vd = vnc_display_find(NULL); + + if (!vd) { + error_setg(errp, "Can not find vnc display"); + return false; + } + + if (arg->has_addresses) { + if (vd->listener) { + qio_net_listener_disconnect(vd->listener); + object_unref(OBJECT(vd->listener)); + vd->listener = NULL; + } + + if (vnc_display_listen(vd, arg->addresses, NULL, errp) < 0) { + return false; + } + } + + return true; +} void vnc_display_open(const char *id, Error **errp) { From 0d90da82030c208c525a6757b0babdc481257fd7 Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Fri, 1 Apr 2022 17:39:36 +0300 Subject: [PATCH 6/9] avocado/vnc: add test_change_listen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add simple test-case for new display-update qmp command. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Daniel P. Berrangé Message-Id: <20220401143936.356460-4-vsementsov@openvz.org> Signed-off-by: Gerd Hoffmann --- tests/avocado/vnc.py | 63 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/tests/avocado/vnc.py b/tests/avocado/vnc.py index 096432988f..187fd3febc 100644 --- a/tests/avocado/vnc.py +++ b/tests/avocado/vnc.py @@ -8,9 +8,48 @@ # This work is licensed under the terms of the GNU GPL, version 2 or # later. See the COPYING file in the top-level directory. +import socket +from typing import List + from avocado_qemu import QemuSystemTest +VNC_ADDR = '127.0.0.1' +VNC_PORT_START = 32768 +VNC_PORT_END = VNC_PORT_START + 1024 + + +def check_bind(port: int) -> bool: + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: + try: + sock.bind((VNC_ADDR, port)) + except OSError: + return False + + return True + + +def check_connect(port: int) -> bool: + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: + try: + sock.connect((VNC_ADDR, port)) + except ConnectionRefusedError: + return False + + return True + + +def find_free_ports(count: int) -> List[int]: + result = [] + for port in range(VNC_PORT_START, VNC_PORT_END): + if check_bind(port): + result.append(port) + if len(result) >= count: + break + assert len(result) == count + return result + + class Vnc(QemuSystemTest): """ :avocado: tags=vnc,quick @@ -51,3 +90,27 @@ class Vnc(QemuSystemTest): set_password_response = self.vm.qmp('change-vnc-password', password='new_password') self.assertEqual(set_password_response['return'], {}) + + def test_change_listen(self): + a, b, c = find_free_ports(3) + self.assertFalse(check_connect(a)) + self.assertFalse(check_connect(b)) + self.assertFalse(check_connect(c)) + + self.vm.add_args('-nodefaults', '-S', '-vnc', f'{VNC_ADDR}:{a - 5900}') + self.vm.launch() + self.assertEqual(self.vm.qmp('query-vnc')['return']['service'], str(a)) + self.assertTrue(check_connect(a)) + self.assertFalse(check_connect(b)) + self.assertFalse(check_connect(c)) + + res = self.vm.qmp('display-update', type='vnc', + addresses=[{'type': 'inet', 'host': VNC_ADDR, + 'port': str(b)}, + {'type': 'inet', 'host': VNC_ADDR, + 'port': str(c)}]) + self.assertEqual(res['return'], {}) + self.assertEqual(self.vm.qmp('query-vnc')['return']['service'], str(b)) + self.assertFalse(check_connect(a)) + self.assertTrue(check_connect(b)) + self.assertTrue(check_connect(c)) From 2aa6a39bc8ec1b94d926fb5a23483a23ff895a2a Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Mon, 25 Apr 2022 15:50:49 +0200 Subject: [PATCH 7/9] i386: move bios load error message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Switch to usual goto-end-of-function error handling style. No functional change. Signed-off-by: Gerd Hoffmann Tested-by: Xiaoyao Li Reviewed-by: Daniel P. Berrangé Reviewed-by: Philippe Mathieu-Daudé Acked-by: Michael S. Tsirkin Message-Id: <20220425135051.551037-2-kraxel@redhat.com> --- hw/i386/x86.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/hw/i386/x86.c b/hw/i386/x86.c index bb67272790..ced31f67b9 100644 --- a/hw/i386/x86.c +++ b/hw/i386/x86.c @@ -1120,9 +1120,7 @@ void x86_bios_rom_init(MachineState *ms, const char *default_firmware, } ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1); if (ret != 0) { - bios_error: - fprintf(stderr, "qemu: could not load PC BIOS '%s'\n", bios_name); - exit(1); + goto bios_error; } g_free(filename); @@ -1143,6 +1141,11 @@ void x86_bios_rom_init(MachineState *ms, const char *default_firmware, memory_region_add_subregion(rom_memory, (uint32_t)(-bios_size), bios); + return; + +bios_error: + fprintf(stderr, "qemu: could not load PC BIOS '%s'\n", bios_name); + exit(1); } bool x86_machine_is_smm_enabled(const X86MachineState *x86ms) From 966f1ca56ffe6e2c4d1333df59bcec233587aabc Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Mon, 25 Apr 2022 15:50:50 +0200 Subject: [PATCH 8/9] i386: factor out x86_firmware_configure() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit move sev firmware setup to separate function so it can be used from other code paths. No functional change. Signed-off-by: Gerd Hoffmann Tested-by: Xiaoyao Li Reviewed-by: Daniel P. Berrangé Reviewed-by: Philippe Mathieu-Daudé Acked-by: Michael S. Tsirkin Message-Id: <20220425135051.551037-3-kraxel@redhat.com> --- hw/i386/pc_sysfw.c | 36 ++++++++++++++++++++++-------------- include/hw/i386/x86.h | 3 +++ 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c index 0540047bad..c8d9e71b88 100644 --- a/hw/i386/pc_sysfw.c +++ b/hw/i386/pc_sysfw.c @@ -147,7 +147,6 @@ static void pc_system_flash_map(PCMachineState *pcms, MemoryRegion *flash_mem; void *flash_ptr; int flash_size; - int ret; assert(PC_MACHINE_GET_CLASS(pcms)->pci_enabled); @@ -195,19 +194,7 @@ static void pc_system_flash_map(PCMachineState *pcms, if (sev_enabled()) { flash_ptr = memory_region_get_ram_ptr(flash_mem); flash_size = memory_region_size(flash_mem); - /* - * OVMF places a GUIDed structures in the flash, so - * search for them - */ - pc_system_parse_ovmf_flash(flash_ptr, flash_size); - - ret = sev_es_save_reset_vector(flash_ptr, flash_size); - if (ret) { - error_report("failed to locate and/or save reset vector"); - exit(1); - } - - sev_encrypt_flash(flash_ptr, flash_size, &error_fatal); + x86_firmware_configure(flash_ptr, flash_size); } } } @@ -259,3 +246,24 @@ void pc_system_firmware_init(PCMachineState *pcms, pc_system_flash_cleanup_unused(pcms); } + +void x86_firmware_configure(void *ptr, int size) +{ + int ret; + + /* + * OVMF places a GUIDed structures in the flash, so + * search for them + */ + pc_system_parse_ovmf_flash(ptr, size); + + if (sev_enabled()) { + ret = sev_es_save_reset_vector(ptr, size); + if (ret) { + error_report("failed to locate and/or save reset vector"); + exit(1); + } + + sev_encrypt_flash(ptr, size, &error_fatal); + } +} diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h index 916cc325ee..4841a49f86 100644 --- a/include/hw/i386/x86.h +++ b/include/hw/i386/x86.h @@ -140,4 +140,7 @@ void gsi_handler(void *opaque, int n, int level); void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name); DeviceState *ioapic_init_secondary(GSIState *gsi_state); +/* pc_sysfw.c */ +void x86_firmware_configure(void *ptr, int size); + #endif From a8152c4e4613c70c2f0573a82babbc8acc00cf90 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Mon, 25 Apr 2022 15:50:51 +0200 Subject: [PATCH 9/9] i386: firmware parsing and sev setup for -bios loaded firmware MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Don't register firmware as rom, not needed (see comment). Add x86_firmware_configure() call for proper sev initialization. Signed-off-by: Gerd Hoffmann Tested-by: Xiaoyao Li Reviewed-by: Daniel P. Berrangé Tested-by: Daniel P. Berrangé Reviewed-by: Philippe Mathieu-Daudé Acked-by: Michael S. Tsirkin Message-Id: <20220425135051.551037-4-kraxel@redhat.com> --- hw/i386/x86.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/hw/i386/x86.c b/hw/i386/x86.c index ced31f67b9..79ebdface6 100644 --- a/hw/i386/x86.c +++ b/hw/i386/x86.c @@ -1115,12 +1115,25 @@ void x86_bios_rom_init(MachineState *ms, const char *default_firmware, } bios = g_malloc(sizeof(*bios)); memory_region_init_ram(bios, NULL, "pc.bios", bios_size, &error_fatal); - if (!isapc_ram_fw) { - memory_region_set_readonly(bios, true); - } - ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1); - if (ret != 0) { - goto bios_error; + if (sev_enabled()) { + /* + * The concept of a "reset" simply doesn't exist for + * confidential computing guests, we have to destroy and + * re-launch them instead. So there is no need to register + * the firmware as rom to properly re-initialize on reset. + * Just go for a straight file load instead. + */ + void *ptr = memory_region_get_ram_ptr(bios); + load_image_size(filename, ptr, bios_size); + x86_firmware_configure(ptr, bios_size); + } else { + if (!isapc_ram_fw) { + memory_region_set_readonly(bios, true); + } + ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1); + if (ret != 0) { + goto bios_error; + } } g_free(filename);