From 1242429b20ddcd7f1ad046ac8750c486938f2cbb Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Thu, 4 Oct 2018 09:05:22 -0500 Subject: [PATCH 1/4] edid: Ignore built binary Added in commit 72d277a7. Signed-off-by: Eric Blake Message-id: 20181004140522.945682-1-eblake@redhat.com Signed-off-by: Gerd Hoffmann --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 5668d02782..64efdfd929 100644 --- a/.gitignore +++ b/.gitignore @@ -107,6 +107,7 @@ /qemu-doc.html /qemu-doc.info /qemu-doc.txt +/qemu-edid /qemu-img /qemu-nbd /qemu-options.def From 5a358b39f52a28a84b380c1685c93010987b3412 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Thu, 27 Sep 2018 16:55:38 +0100 Subject: [PATCH 2/4] hw/display/qxl: Suppress clang-7 warning about misaligned atomic operation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If QEMU is compiled with clang-7 it results in the warning: hw/display/qxl.c:1884:19: error: misaligned or large atomic operation may incur significant performance penalty [-Werror,-Watomic-alignment] old_pending = atomic_fetch_or(&d->ram->int_pending, le_events); ^ This is because the Spice headers forgot to define the QXLRam struct with the '__aligned__(4)' attribute. clang 7 and newer will thus warn that the access here to int_pending might not be 4-aligned (because the QXLRam object d->ram points at might start at a misaligned address). In fact we set up d->ram in init_qxl_ram() so it always starts at a 4K boundary, so we know the atomic access here is OK. Newer Spice versions (with Spice commit beda5ec7a6848be20c0cac2a9a8ef2a41e8069c1) will fix the bug; for older Spice versions, work around it by telling the compiler explicitly that the alignment is OK using __builtin_assume_aligned(). Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson Reviewed-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé Message-id: 20180927155538.699-1-peter.maydell@linaro.org Signed-off-by: Gerd Hoffmann --- hw/display/qxl.c | 26 +++++++++++++++++++++++++- include/qemu/compiler.h | 9 +++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/hw/display/qxl.c b/hw/display/qxl.c index 747986478f..e628cf1286 100644 --- a/hw/display/qxl.c +++ b/hw/display/qxl.c @@ -1893,7 +1893,31 @@ static void qxl_send_events(PCIQXLDevice *d, uint32_t events) trace_qxl_send_events_vm_stopped(d->id, events); return; } - old_pending = atomic_fetch_or(&d->ram->int_pending, le_events); + /* + * Older versions of Spice forgot to define the QXLRam struct + * with the '__aligned__(4)' attribute. clang 7 and newer will + * thus warn that atomic_fetch_or(&d->ram->int_pending, ...) + * might be a misaligned atomic access, and will generate an + * out-of-line call for it, which results in a link error since + * we don't currently link against libatomic. + * + * In fact we set up d->ram in init_qxl_ram() so it always starts + * at a 4K boundary, so we know that &d->ram->int_pending is + * naturally aligned for a uint32_t. Newer Spice versions + * (with Spice commit beda5ec7a6848be20c0cac2a9a8ef2a41e8069c1) + * will fix the bug directly. To deal with older versions, + * we tell the compiler to assume the address really is aligned. + * Any compiler which cares about the misalignment will have + * __builtin_assume_aligned. + */ +#ifdef HAS_ASSUME_ALIGNED +#define ALIGNED_UINT32_PTR(P) ((uint32_t *)__builtin_assume_aligned(P, 4)) +#else +#define ALIGNED_UINT32_PTR(P) ((uint32_t *)P) +#endif + + old_pending = atomic_fetch_or(ALIGNED_UINT32_PTR(&d->ram->int_pending), + le_events); if ((old_pending & le_events) == le_events) { return; } diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h index 5843812710..bf47e7bee4 100644 --- a/include/qemu/compiler.h +++ b/include/qemu/compiler.h @@ -122,6 +122,15 @@ #ifndef __has_feature #define __has_feature(x) 0 /* compatibility with non-clang compilers */ #endif + +#ifndef __has_builtin +#define __has_builtin(x) 0 /* compatibility with non-clang compilers */ +#endif + +#if __has_builtin(__builtin_assume_aligned) || QEMU_GNUC_PREREQ(4, 7) +#define HAS_ASSUME_ALIGNED +#endif + /* Implement C11 _Generic via GCC builtins. Example: * * QEMU_GENERIC(x, (float, sinf), (long double, sinl), sin) (x) From 0ab90e617040e5590f7fc403b5d34f76f53f6bdc Mon Sep 17 00:00:00 2001 From: "remy.noel" Date: Tue, 2 Oct 2018 14:19:35 +0200 Subject: [PATCH 3/4] secondary-vga: delete mmio subregions upon exit 93abfc88bd649de1933588bfc7175605331b3ea9 introduced a reference cycle in the vga-pci devices, preventing cleanup of the object upon hotblug. This patch allows to break the cycle. Signed-off-by: remy.noel Message-id: 20181002121935.23706-1-remy.noel@blade-group.com [ kraxel: delete the recently added edid region too ] Signed-off-by: Gerd Hoffmann --- hw/display/vga-pci.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/hw/display/vga-pci.c b/hw/display/vga-pci.c index 24ca1b3e1f..a17c96e703 100644 --- a/hw/display/vga-pci.c +++ b/hw/display/vga-pci.c @@ -309,6 +309,14 @@ static void pci_secondary_vga_exit(PCIDevice *dev) VGACommonState *s = &d->vga; graphic_console_close(s->con); + memory_region_del_subregion(&d->mmio, &d->mrs[0]); + memory_region_del_subregion(&d->mmio, &d->mrs[1]); + if (d->flags & (1 << PCI_VGA_FLAG_ENABLE_QEXT)) { + memory_region_del_subregion(&d->mmio, &d->mrs[2]); + } + if (d->flags & (1 << PCI_VGA_FLAG_ENABLE_EDID)) { + memory_region_del_subregion(&d->mmio, &d->mrs[3]); + } } static void pci_secondary_vga_init(Object *obj) From edbc4b24bbb179fa9562d4e5313470f10569433c Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Fri, 5 Oct 2018 11:19:34 +0200 Subject: [PATCH 4/4] edid: fix vendor default "EMU" actually is "Emulex Corporation", so not a good idea to use that by default. Lets use the Red Hat vendor id instead, which is in line with the pci ids which are allocated from Red Hat vendor ids too. Vendor list is available from http://www.uefi.org/pnp_id_list Signed-off-by: Gerd Hoffmann Message-id: 20181005091934.12143-1-kraxel@redhat.com --- hw/display/edid-generate.c | 2 +- include/hw/display/edid.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/display/edid-generate.c b/hw/display/edid-generate.c index c80397ea96..37e60fe42a 100644 --- a/hw/display/edid-generate.c +++ b/hw/display/edid-generate.c @@ -301,7 +301,7 @@ void qemu_edid_generate(uint8_t *edid, size_t size, /* =============== set defaults =============== */ if (!info->vendor || strlen(info->vendor) != 3) { - info->vendor = "EMU"; + info->vendor = "RHT"; } if (!info->name) { info->name = "QEMU Monitor"; diff --git a/include/hw/display/edid.h b/include/hw/display/edid.h index bd51d26916..bacf170889 100644 --- a/include/hw/display/edid.h +++ b/include/hw/display/edid.h @@ -4,7 +4,7 @@ #include "hw/hw.h" typedef struct qemu_edid_info { - const char *vendor; + const char *vendor; /* http://www.uefi.org/pnp_id_list */ const char *name; const char *serial; uint32_t dpi;