[PATCH for-8.1] hw/display: Compile vga.c as target-independent code

Thomas Huth posted 1 patch 1 year ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230412163501.36770-1-thuth@redhat.com
Maintainers: Gerd Hoffmann <kraxel@redhat.com>
hw/display/vga.c       | 31 ++++++++++++++++++++++---------
hw/display/meson.build |  2 +-
2 files changed, 23 insertions(+), 10 deletions(-)
[PATCH for-8.1] hw/display: Compile vga.c as target-independent code
Posted by Thomas Huth 1 year ago
The target checks here are only during the initialization, so they
are not performance critical. We can switch these to runtime checks
to avoid that we have to compile this file multiple times during
the build, and make the code ready for an universal build one day.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/display/vga.c       | 31 ++++++++++++++++++++++---------
 hw/display/meson.build |  2 +-
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/hw/display/vga.c b/hw/display/vga.c
index 7a5fdff649..37557c3442 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -26,7 +26,9 @@
 #include "qemu/units.h"
 #include "sysemu/reset.h"
 #include "qapi/error.h"
+#include "hw/core/cpu.h"
 #include "hw/display/vga.h"
+#include "hw/i386/x86.h"
 #include "hw/pci/pci.h"
 #include "vga_int.h"
 #include "vga_regs.h"
@@ -2244,11 +2246,8 @@ bool vga_common_init(VGACommonState *s, Object *obj, Error **errp)
      * into a device attribute set by the machine/platform to remove
      * all target endian dependencies from this file.
      */
-#if TARGET_BIG_ENDIAN
-    s->default_endian_fb = true;
-#else
-    s->default_endian_fb = false;
-#endif
+    s->default_endian_fb = target_words_bigendian();
+
     vga_dirty_log_start(s);
 
     return true;
@@ -2263,11 +2262,15 @@ static const MemoryRegionPortio vga_portio_list[] = {
     PORTIO_END_OF_LIST(),
 };
 
-static const MemoryRegionPortio vbe_portio_list[] = {
+static const MemoryRegionPortio vbe_portio_list_x86[] = {
     { 0, 1, 2, .read = vbe_ioport_read_index, .write = vbe_ioport_write_index },
-# ifdef TARGET_I386
     { 1, 1, 2, .read = vbe_ioport_read_data, .write = vbe_ioport_write_data },
-# endif
+    { 2, 1, 2, .read = vbe_ioport_read_data, .write = vbe_ioport_write_data },
+    PORTIO_END_OF_LIST(),
+};
+
+static const MemoryRegionPortio vbe_portio_list_no_x86[] = {
+    { 0, 1, 2, .read = vbe_ioport_read_index, .write = vbe_ioport_write_index },
     { 2, 1, 2, .read = vbe_ioport_read_data, .write = vbe_ioport_write_data },
     PORTIO_END_OF_LIST(),
 };
@@ -2278,9 +2281,19 @@ MemoryRegion *vga_init_io(VGACommonState *s, Object *obj,
                           const MemoryRegionPortio **vbe_ports)
 {
     MemoryRegion *vga_mem;
+    MachineState *ms = MACHINE(qdev_get_machine());
+
+    /*
+     * We unfortunately need two VBE lists since non-x86 machines might
+     * not be able to do 16-bit accesses at unaligned addresses (0x1cf)
+     */
+    if (object_dynamic_cast(OBJECT(ms), TYPE_X86_MACHINE)) {
+        *vbe_ports = vbe_portio_list_x86;
+    } else {
+        *vbe_ports = vbe_portio_list_no_x86;
+    }
 
     *vga_ports = vga_portio_list;
-    *vbe_ports = vbe_portio_list;
 
     vga_mem = g_malloc(sizeof(*vga_mem));
     memory_region_init_io(vga_mem, obj, &vga_mem_ops, s,
diff --git a/hw/display/meson.build b/hw/display/meson.build
index 4191694380..17165bd536 100644
--- a/hw/display/meson.build
+++ b/hw/display/meson.build
@@ -36,7 +36,7 @@ softmmu_ss.add(when: 'CONFIG_CG3', if_true: files('cg3.c'))
 softmmu_ss.add(when: 'CONFIG_MACFB', if_true: files('macfb.c'))
 softmmu_ss.add(when: 'CONFIG_NEXTCUBE', if_true: files('next-fb.c'))
 
-specific_ss.add(when: 'CONFIG_VGA', if_true: files('vga.c'))
+softmmu_ss.add(when: 'CONFIG_VGA', if_true: files('vga.c'))
 
 if (config_all_devices.has_key('CONFIG_VGA_CIRRUS') or
     config_all_devices.has_key('CONFIG_VGA_PCI') or
-- 
2.31.1
Re: [PATCH for-8.1] hw/display: Compile vga.c as target-independent code
Posted by Fabiano Rosas 1 year ago
Thomas Huth <thuth@redhat.com> writes:

> The target checks here are only during the initialization, so they
> are not performance critical. We can switch these to runtime checks
> to avoid that we have to compile this file multiple times during
> the build, and make the code ready for an universal build one day.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Fabiano Rosas <farosas@suse.de>