[Qemu-devel] [PULL 0/4] Vga 20181005 patches

[Qemu-devel] [PULL 0/4] Vga 20181005 patches

[Gerd Hoffmann]

The following changes since commit dafd95053611aa14dda40266857608d12ddce658:

  Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2018-10-02 18:27:18 +0100)

are available in the git repository at:

  git://git.kraxel.org/qemu tags/vga-20181005-pull-request

for you to fetch changes up to edbc4b24bbb179fa9562d4e5313470f10569433c:

  edid: fix vendor default (2018-10-05 11:26:56 +0200)

----------------------------------------------------------------
vga: edid fixes, qxl clang workaround, vga mmio subregion fix.

----------------------------------------------------------------

Eric Blake (1):
  edid: Ignore built binary

Gerd Hoffmann (1):
  edid: fix vendor default

Peter Maydell (1):
  hw/display/qxl: Suppress clang-7 warning about misaligned atomic
    operation

remy.noel (1):
  secondary-vga: delete mmio subregions upon exit

 include/hw/display/edid.h  |  2 +-
 include/qemu/compiler.h    |  9 +++++++++
 hw/display/edid-generate.c |  2 +-
 hw/display/qxl.c           | 26 +++++++++++++++++++++++++-
 hw/display/vga-pci.c       |  8 ++++++++
 .gitignore                 |  1 +
 6 files changed, 45 insertions(+), 3 deletions(-)

-- 
2.9.3

[Qemu-devel] [PULL 1/4] edid: Ignore built binary

[Gerd Hoffmann]

From: Eric Blake <eblake@redhat.com>

Added in commit 72d277a7.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-id: 20181004140522.945682-1-eblake@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 .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
-- 
2.9.3

[Qemu-devel] [PULL 2/4] hw/display/qxl: Suppress clang-7 warning about misaligned atomic operation

[Gerd Hoffmann]

From: Peter Maydell <peter.maydell@linaro.org>

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 <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20180927155538.699-1-peter.maydell@linaro.org
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/qemu/compiler.h |  9 +++++++++
 hw/display/qxl.c        | 26 +++++++++++++++++++++++++-
 2 files changed, 34 insertions(+), 1 deletion(-)

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)
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;
     }
-- 
2.9.3

[Qemu-devel] [PULL 3/4] secondary-vga: delete mmio subregions upon exit

[Gerd Hoffmann]

From: "remy.noel" <remy.noel@blade-group.com>

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 <remy.noel@blade-group.com>
Message-id: 20181002121935.23706-1-remy.noel@blade-group.com

[ kraxel: delete the recently added edid region too ]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 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)
-- 
2.9.3

[Qemu-devel] [PULL 4/4] edid: fix vendor default

[Gerd Hoffmann]

"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 <kraxel@redhat.com>
Message-id: 20181005091934.12143-1-kraxel@redhat.com
---
 include/hw/display/edid.h  | 2 +-
 hw/display/edid-generate.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

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;
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";
-- 
2.9.3

Re: [Qemu-devel] [PULL 0/4] Vga 20181005 patches

[Peter Maydell]

On 5 October 2018 at 12:11, Gerd Hoffmann <kraxel@redhat.com> wrote:
> The following changes since commit dafd95053611aa14dda40266857608d12ddce658:
>
>   Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2018-10-02 18:27:18 +0100)
>
> are available in the git repository at:
>
>   git://git.kraxel.org/qemu tags/vga-20181005-pull-request
>
> for you to fetch changes up to edbc4b24bbb179fa9562d4e5313470f10569433c:
>
>   edid: fix vendor default (2018-10-05 11:26:56 +0200)
>
> ----------------------------------------------------------------
> vga: edid fixes, qxl clang workaround, vga mmio subregion fix.
>
> ----------------------------------------------------------------
Applied, thanks.

-- PMM