From 03d9825d15e17e444e00bd4caa9edb0d57022794 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radim=20Kr=C4=8Dm=C3=A1=C5=99?= Date: Tue, 17 Feb 2015 17:30:50 +0100 Subject: [PATCH 1/7] qxl: document minimal video memory for new modes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The alternative to removing existing comments. Signed-off-by: Radim Krčmář Signed-off-by: Gerd Hoffmann --- hw/display/qxl.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hw/display/qxl.c b/hw/display/qxl.c index 61df477264..6e9079783e 100644 --- a/hw/display/qxl.c +++ b/hw/display/qxl.c @@ -120,9 +120,12 @@ static QXLMode qxl_modes[] = { QXL_MODE_EX(2560, 2048), QXL_MODE_EX(2800, 2100), QXL_MODE_EX(3200, 2400), + /* these modes need more than 32 MB video memory */ QXL_MODE_EX(3840, 2160), /* 4k mainstream */ QXL_MODE_EX(4096, 2160), /* 4k */ + /* these modes need more than 64 MB video memory */ QXL_MODE_EX(7680, 4320), /* 8k mainstream */ + /* these modes need more than 128 MB video memory */ QXL_MODE_EX(8192, 4320), /* 8k */ }; From 876d516311c1538a7d29f2abec48b7cda0645eea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radim=20Kr=C4=8Dm=C3=A1=C5=99?= Date: Tue, 17 Feb 2015 17:30:51 +0100 Subject: [PATCH 2/7] spice: fix invalid memory access to vga.vram MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit vga_common_init() doesn't allow more than 256 MiB vram size and silently shrinks any larger value. qxl_dirty_surfaces() used the unshrinked size via qxl->shadow_rom.surface0_area_size when accessing the memory, which resulted in segfault. Add a workaround for this case and an assert if it happens again. We have to bump the vga memory limit too, because 256 MiB wouldn't have allowed 8k (it requires more than 128 MiB). 1024 MiB doesn't work, but 512 MiB seems fine. Proposed-by: Gerd Hoffmann Signed-off-by: Radim Krčmář Signed-off-by: Gerd Hoffmann --- hw/display/qxl.c | 8 ++++++++ hw/display/vga.c | 4 ++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/hw/display/qxl.c b/hw/display/qxl.c index 6e9079783e..92f2d5025d 100644 --- a/hw/display/qxl.c +++ b/hw/display/qxl.c @@ -370,6 +370,8 @@ static void init_qxl_rom(PCIQXLDevice *d) num_pages -= surface0_area_size; num_pages = num_pages / QXL_PAGE_SIZE; + assert(ram_header_size + surface0_area_size <= d->vga.vram_size); + rom->draw_area_offset = cpu_to_le32(0); rom->surface0_area_size = cpu_to_le32(surface0_area_size); rom->pages_offset = cpu_to_le32(surface0_area_size); @@ -1883,6 +1885,12 @@ static void qxl_init_ramsize(PCIQXLDevice *qxl) if (qxl->vgamem_size_mb < 8) { qxl->vgamem_size_mb = 8; } + /* XXX: we round vgamem_size_mb up to a nearest power of two and it must be + * less than vga_common_init()'s maximum on qxl->vga.vram_size (512 now). + */ + if (qxl->vgamem_size_mb > 256) { + qxl->vgamem_size_mb = 256; + } qxl->vgamem_size = qxl->vgamem_size_mb * 1024 * 1024; /* vga ram (bar 0, total) */ diff --git a/hw/display/vga.c b/hw/display/vga.c index c8c49abc6e..6e4ca7e9ab 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -2121,10 +2121,10 @@ void vga_common_init(VGACommonState *s, Object *obj, bool global_vmstate) expand4to8[i] = v; } - /* valid range: 1 MB -> 256 MB */ + /* valid range: 1 MB -> 512 MB */ s->vram_size = 1024 * 1024; while (s->vram_size < (s->vram_size_mb << 20) && - s->vram_size < (256 << 20)) { + s->vram_size < (512 << 20)) { s->vram_size <<= 1; } s->vram_size_mb = s->vram_size >> 20; From bb7443f6d6f09411ea10f06e6cb0d416bd1ccebd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radim=20Kr=C4=8Dm=C3=A1=C5=99?= Date: Tue, 17 Feb 2015 17:30:52 +0100 Subject: [PATCH 3/7] qxl: refactor rounding up to a nearest power of 2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We already have pow2floor, mirror it and use instead of a function with similar results (same in used domain), to clarify our intent. Signed-off-by: Radim Krčmář Signed-off-by: Gerd Hoffmann --- hw/display/qxl.c | 23 +++++------------------ include/qemu-common.h | 3 +++ util/cutils.c | 14 ++++++++++++++ 3 files changed, 22 insertions(+), 18 deletions(-) diff --git a/hw/display/qxl.c b/hw/display/qxl.c index 92f2d5025d..94ff52a959 100644 --- a/hw/display/qxl.c +++ b/hw/display/qxl.c @@ -300,19 +300,6 @@ void qxl_spice_reset_cursor(PCIQXLDevice *qxl) qxl->ssd.cursor = cursor_builtin_hidden(); } - -static inline uint32_t msb_mask(uint32_t val) -{ - uint32_t mask; - - do { - mask = ~(val - 1) & val; - val &= ~mask; - } while (mask < val); - - return mask; -} - static ram_addr_t qxl_rom_size(void) { uint32_t required_rom_size = sizeof(QXLRom) + sizeof(QXLModes) + @@ -1921,10 +1908,10 @@ static void qxl_init_ramsize(PCIQXLDevice *qxl) qxl->vram32_size = 4096; qxl->vram_size = 4096; } - qxl->vgamem_size = msb_mask(qxl->vgamem_size * 2 - 1); - qxl->vga.vram_size = msb_mask(qxl->vga.vram_size * 2 - 1); - qxl->vram32_size = msb_mask(qxl->vram32_size * 2 - 1); - qxl->vram_size = msb_mask(qxl->vram_size * 2 - 1); + qxl->vgamem_size = pow2ceil(qxl->vgamem_size); + qxl->vga.vram_size = pow2ceil(qxl->vga.vram_size); + qxl->vram32_size = pow2ceil(qxl->vram32_size); + qxl->vram_size = pow2ceil(qxl->vram_size); } static int qxl_init_common(PCIQXLDevice *qxl) @@ -1956,7 +1943,7 @@ static int qxl_init_common(PCIQXLDevice *qxl) break; case 4: /* qxl-4 */ pci_device_rev = QXL_REVISION_STABLE_V12; - io_size = msb_mask(QXL_IO_RANGE_SIZE * 2 - 1); + io_size = pow2ceil(QXL_IO_RANGE_SIZE); break; default: error_report("Invalid revision %d for qxl device (max %d)", diff --git a/include/qemu-common.h b/include/qemu-common.h index 644b46dcdd..1b5cffb403 100644 --- a/include/qemu-common.h +++ b/include/qemu-common.h @@ -418,6 +418,9 @@ static inline bool is_power_of_2(uint64_t value) /* round down to the nearest power of 2*/ int64_t pow2floor(int64_t value); +/* round up to the nearest power of 2 (0 if overflow) */ +uint64_t pow2ceil(uint64_t value); + #include "qemu/module.h" /* diff --git a/util/cutils.c b/util/cutils.c index dbe7412bd8..c2250d1ba5 100644 --- a/util/cutils.c +++ b/util/cutils.c @@ -483,6 +483,20 @@ int64_t pow2floor(int64_t value) return value; } +/* round up to the nearest power of 2 (0 if overflow) */ +uint64_t pow2ceil(uint64_t value) +{ + uint8_t nlz = clz64(value); + + if (is_power_of_2(value)) { + return value; + } + if (!nlz) { + return 0; + } + return 1ULL << (64 - nlz); +} + /* * Implementation of ULEB128 (http://en.wikipedia.org/wiki/LEB128) * Input is limited to 14-bit numbers From 619616ce31a5a5d167bf26f40d920b26da0a7bfd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radim=20Kr=C4=8Dm=C3=A1=C5=99?= Date: Tue, 17 Feb 2015 17:30:53 +0100 Subject: [PATCH 4/7] vga: refactor vram_size clamping and rounding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make the code a bit more obvious. We don't have min/max, so a general helper for clamp probably isn't acceptable either. Signed-off-by: Radim Krčmář Signed-off-by: Gerd Hoffmann --- hw/display/vga.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/hw/display/vga.c b/hw/display/vga.c index 6e4ca7e9ab..c0f7b343bb 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -2094,6 +2094,17 @@ static const GraphicHwOps vga_ops = { .text_update = vga_update_text, }; +static inline uint32_t uint_clamp(uint32_t val, uint32_t vmin, uint32_t vmax) +{ + if (val < vmin) { + return vmin; + } + if (val > vmax) { + return vmax; + } + return val; +} + void vga_common_init(VGACommonState *s, Object *obj, bool global_vmstate) { int i, j, v, b; @@ -2121,13 +2132,10 @@ void vga_common_init(VGACommonState *s, Object *obj, bool global_vmstate) expand4to8[i] = v; } - /* valid range: 1 MB -> 512 MB */ - s->vram_size = 1024 * 1024; - while (s->vram_size < (s->vram_size_mb << 20) && - s->vram_size < (512 << 20)) { - s->vram_size <<= 1; - } - s->vram_size_mb = s->vram_size >> 20; + s->vram_size_mb = uint_clamp(s->vram_size_mb, 1, 512); + s->vram_size_mb = pow2ceil(s->vram_size_mb); + s->vram_size = s->vram_size_mb << 20; + if (!s->vbe_size) { s->vbe_size = s->vram_size; } From 20ca3763abbb77ae9942f3e854bdeec36a147a29 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Mon, 2 Mar 2015 17:01:50 +0100 Subject: [PATCH 5/7] qxl: drop update_displaychangelistener call for secondary qxl devices Commit 3dcadce5076d4b42fa395c39662d65e050b77784 added three update_displaychangelistener call sites: Two for primary qxl cards, when entering/leaving vga mode, which are correct. One for secondary qxl cards, which is wrong because we don't register a displaychangelistener in the first place for secondary cards. Remove it. Reported-by: Brad Campbell Tested-by: Brad Campbell Signed-off-by: Gerd Hoffmann --- hw/display/qxl.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/display/qxl.c b/hw/display/qxl.c index 94ff52a959..762f75dc2d 100644 --- a/hw/display/qxl.c +++ b/hw/display/qxl.c @@ -1147,7 +1147,6 @@ static void qxl_soft_reset(PCIQXLDevice *d) qxl_enter_vga_mode(d); } else { d->mode = QXL_MODE_UNDEFINED; - update_displaychangelistener(&d->ssd.dcl, GUI_REFRESH_INTERVAL_IDLE); } } From 22fa7da0005c939c940634da069ecb9bbb199a2a Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Sun, 1 Mar 2015 09:29:18 -0500 Subject: [PATCH 6/7] hmp: info spice: Show string channel name Useful for debugging. https://bugzilla.redhat.com/show_bug.cgi?id=822418 Signed-off-by: Cole Robinson Reviewed-by: Eric Blake Signed-off-by: Gerd Hoffmann --- hmp.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/hmp.c b/hmp.c index 735097c03b..eacfb1b5a9 100644 --- a/hmp.c +++ b/hmp.c @@ -29,6 +29,10 @@ #include "block/qapi.h" #include "qemu-io.h" +#ifdef CONFIG_SPICE +#include +#endif + static void hmp_handle_error(Monitor *mon, Error **errp) { assert(errp); @@ -545,6 +549,20 @@ void hmp_info_spice(Monitor *mon, const QDict *qdict) { SpiceChannelList *chan; SpiceInfo *info; + const char *channel_name; + const char * const channel_names[] = { + [SPICE_CHANNEL_MAIN] = "main", + [SPICE_CHANNEL_DISPLAY] = "display", + [SPICE_CHANNEL_INPUTS] = "inputs", + [SPICE_CHANNEL_CURSOR] = "cursor", + [SPICE_CHANNEL_PLAYBACK] = "playback", + [SPICE_CHANNEL_RECORD] = "record", + [SPICE_CHANNEL_TUNNEL] = "tunnel", + [SPICE_CHANNEL_SMARTCARD] = "smartcard", + [SPICE_CHANNEL_USBREDIR] = "usbredir", + [SPICE_CHANNEL_PORT] = "port", + [SPICE_CHANNEL_WEBDAV] = "webdav", + }; info = qmp_query_spice(NULL); @@ -581,6 +599,15 @@ void hmp_info_spice(Monitor *mon, const QDict *qdict) chan->value->connection_id); monitor_printf(mon, " channel: %" PRId64 ":%" PRId64 "\n", chan->value->channel_type, chan->value->channel_id); + + channel_name = "unknown"; + if (chan->value->channel_type > 0 && + chan->value->channel_type < ARRAY_SIZE(channel_names) && + channel_names[chan->value->channel_type]) { + channel_name = channel_names[chan->value->channel_type]; + } + + monitor_printf(mon, " channel name: %s\n", channel_name); } } From 7c6044a94e52db8aef9a71d616c7a0914adb71ab Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 3 Mar 2015 09:27:28 +0100 Subject: [PATCH 7/7] hmp: info spice: take out webdav Obvious suggestion for the next spice-protocol release: Add some way to #ifdef new stuff. Signed-off-by: Gerd Hoffmann Reviewed-by: Cole Robinson --- hmp.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/hmp.c b/hmp.c index eacfb1b5a9..71c28bc816 100644 --- a/hmp.c +++ b/hmp.c @@ -561,7 +561,12 @@ void hmp_info_spice(Monitor *mon, const QDict *qdict) [SPICE_CHANNEL_SMARTCARD] = "smartcard", [SPICE_CHANNEL_USBREDIR] = "usbredir", [SPICE_CHANNEL_PORT] = "port", +#if 0 + /* minimum spice-protocol is 0.12.3, webdav was added in 0.12.7, + * no easy way to #ifdef (SPICE_CHANNEL_* is a enum). Disable + * as quick fix for build failures with older versions. */ [SPICE_CHANNEL_WEBDAV] = "webdav", +#endif }; info = qmp_query_spice(NULL);