All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH-for-7.0 0/5] hw/display: QOM'ify vga_mmio_init() as TYPE_VGA_MMIO
@ 2021-11-19 17:11 Philippe Mathieu-Daudé
  2021-11-19 17:11 ` [PATCH-for-7.0 1/5] hw/display: Rename VGA_ISA_MM -> VGA_MMIO Philippe Mathieu-Daudé
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-11-19 17:11 UTC (permalink / raw)
  To: qemu-devel
  Cc: Thomas Huth, Hervé Poussineau, Gerd Hoffmann,
	Philippe Mathieu-Daudé

Convert the open-coded vga_mmio_init() to a sysbus device.

Philippe Mathieu-Daudé (5):
  hw/display: Rename VGA_ISA_MM -> VGA_MMIO
  hw/display/vga-mmio: Inline vga_mm_init()
  hw/display/vga-mmio: QOM'ify vga_mmio_init() as TYPE_VGA_MMIO
  hw/mips/jazz: Inline vga_mmio_init() and remove it
  tests/avocado: Test NetBSD 9.2 on the Jazz Magnum machine

 configs/devices/mips-softmmu/common.mak       |   2 +-
 include/hw/display/vga.h                      |   6 -
 hw/display/vga-isa-mm.c                       | 114 --------------
 hw/display/vga-mmio.c                         | 141 ++++++++++++++++++
 hw/mips/jazz.c                                |   9 +-
 hw/display/Kconfig                            |   2 +-
 hw/display/meson.build                        |   2 +-
 hw/mips/Kconfig                               |   2 +-
 .../avocado/machine_mips_jazz.d/nvram.bin.xz  | Bin 0 -> 700 bytes
 tests/avocado/machine_mips_jazz.py            |  99 ++++++++++++
 10 files changed, 252 insertions(+), 125 deletions(-)
 delete mode 100644 hw/display/vga-isa-mm.c
 create mode 100644 hw/display/vga-mmio.c
 create mode 100644 tests/avocado/machine_mips_jazz.d/nvram.bin.xz
 create mode 100644 tests/avocado/machine_mips_jazz.py

-- 
2.31.1



^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH-for-7.0 1/5] hw/display: Rename VGA_ISA_MM -> VGA_MMIO
  2021-11-19 17:11 [PATCH-for-7.0 0/5] hw/display: QOM'ify vga_mmio_init() as TYPE_VGA_MMIO Philippe Mathieu-Daudé
@ 2021-11-19 17:11 ` Philippe Mathieu-Daudé
  2021-11-19 18:15   ` BALATON Zoltan
  2021-11-22 15:06   ` Thomas Huth
  2021-11-19 17:11 ` [PATCH-for-7.0 2/5] hw/display/vga-mmio: Inline vga_mm_init() Philippe Mathieu-Daudé
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-11-19 17:11 UTC (permalink / raw)
  To: qemu-devel
  Cc: Thomas Huth, Hervé Poussineau, Gerd Hoffmann,
	Philippe Mathieu-Daudé

There is no ISA bus part in the MMIO VGA device, so rename:

 *  hw/display/vga-isa-mm.c -> hw/display/vga-mmio.c
 *  CONFIG_VGA_ISA_MM -> CONFIG_VGA_MMIO
 *  ISAVGAMMState -> VGAMmioState
 *  isa_vga_mm_init() -> vga_mmio_init()

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 configs/devices/mips-softmmu/common.mak |  2 +-
 include/hw/display/vga.h                |  2 +-
 hw/display/{vga-isa-mm.c => vga-mmio.c} | 16 ++++++++--------
 hw/mips/jazz.c                          |  2 +-
 hw/display/Kconfig                      |  2 +-
 hw/display/meson.build                  |  2 +-
 hw/mips/Kconfig                         |  2 +-
 7 files changed, 14 insertions(+), 14 deletions(-)
 rename hw/display/{vga-isa-mm.c => vga-mmio.c} (93%)

diff --git a/configs/devices/mips-softmmu/common.mak b/configs/devices/mips-softmmu/common.mak
index 752b62b1e63..d2202c839e0 100644
--- a/configs/devices/mips-softmmu/common.mak
+++ b/configs/devices/mips-softmmu/common.mak
@@ -7,7 +7,7 @@ CONFIG_ISA_BUS=y
 CONFIG_PCI=y
 CONFIG_PCI_DEVICES=y
 CONFIG_VGA_ISA=y
-CONFIG_VGA_ISA_MM=y
+CONFIG_VGA_MMIO=y
 CONFIG_VGA_CIRRUS=y
 CONFIG_VMWARE_VGA=y
 CONFIG_SERIAL=y
diff --git a/include/hw/display/vga.h b/include/hw/display/vga.h
index 5f7825e0e36..03c65a14218 100644
--- a/include/hw/display/vga.h
+++ b/include/hw/display/vga.h
@@ -24,7 +24,7 @@ enum vga_retrace_method {
 
 extern enum vga_retrace_method vga_retrace_method;
 
-int isa_vga_mm_init(hwaddr vram_base,
+int vga_mmio_init(hwaddr vram_base,
                     hwaddr ctrl_base, int it_shift,
                     MemoryRegion *address_space);
 
diff --git a/hw/display/vga-isa-mm.c b/hw/display/vga-mmio.c
similarity index 93%
rename from hw/display/vga-isa-mm.c
rename to hw/display/vga-mmio.c
index 7321b7a06d5..8aaf44e7b1d 100644
--- a/hw/display/vga-isa-mm.c
+++ b/hw/display/vga-mmio.c
@@ -1,5 +1,5 @@
 /*
- * QEMU ISA MM VGA Emulator.
+ * QEMU MMIO VGA Emulator.
  *
  * Copyright (c) 2003 Fabrice Bellard
  *
@@ -32,15 +32,15 @@
 
 #define VGA_RAM_SIZE (8 * MiB)
 
-typedef struct ISAVGAMMState {
+typedef struct VGAMmioState {
     VGACommonState vga;
     int it_shift;
-} ISAVGAMMState;
+} VGAMmioState;
 
 /* Memory mapped interface */
 static uint64_t vga_mm_read(void *opaque, hwaddr addr, unsigned size)
 {
-    ISAVGAMMState *s = opaque;
+    VGAMmioState *s = opaque;
 
     return vga_ioport_read(&s->vga, addr >> s->it_shift) &
         MAKE_64BIT_MASK(0, size * 8);
@@ -49,7 +49,7 @@ static uint64_t vga_mm_read(void *opaque, hwaddr addr, unsigned size)
 static void vga_mm_write(void *opaque, hwaddr addr, uint64_t value,
                          unsigned size)
 {
-    ISAVGAMMState *s = opaque;
+    VGAMmioState *s = opaque;
 
     vga_ioport_write(&s->vga, addr >> s->it_shift,
                      value & MAKE_64BIT_MASK(0, size * 8));
@@ -65,7 +65,7 @@ static const MemoryRegionOps vga_mm_ctrl_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static void vga_mm_init(ISAVGAMMState *s, hwaddr vram_base,
+static void vga_mm_init(VGAMmioState *s, hwaddr vram_base,
                         hwaddr ctrl_base, int it_shift,
                         MemoryRegion *address_space)
 {
@@ -91,11 +91,11 @@ static void vga_mm_init(ISAVGAMMState *s, hwaddr vram_base,
     memory_region_set_coalescing(vga_io_memory);
 }
 
-int isa_vga_mm_init(hwaddr vram_base,
+int vga_mmio_init(hwaddr vram_base,
                     hwaddr ctrl_base, int it_shift,
                     MemoryRegion *address_space)
 {
-    ISAVGAMMState *s;
+    VGAMmioState *s;
 
     s = g_malloc0(sizeof(*s));
 
diff --git a/hw/mips/jazz.c b/hw/mips/jazz.c
index f5a26e174d5..8f345afd137 100644
--- a/hw/mips/jazz.c
+++ b/hw/mips/jazz.c
@@ -274,7 +274,7 @@ static void mips_jazz_init(MachineState *machine,
         }
         break;
     case JAZZ_PICA61:
-        isa_vga_mm_init(0x40000000, 0x60000000, 0, get_system_memory());
+        vga_mmio_init(0x40000000, 0x60000000, 0, get_system_memory());
         break;
     default:
         break;
diff --git a/hw/display/Kconfig b/hw/display/Kconfig
index a2306b67d87..a1b159becd7 100644
--- a/hw/display/Kconfig
+++ b/hw/display/Kconfig
@@ -49,7 +49,7 @@ config VGA_ISA
     depends on ISA_BUS
     select VGA
 
-config VGA_ISA_MM
+config VGA_MMIO
     bool
     select VGA
 
diff --git a/hw/display/meson.build b/hw/display/meson.build
index 861c43ff984..adc53dd8b6c 100644
--- a/hw/display/meson.build
+++ b/hw/display/meson.build
@@ -18,7 +18,7 @@
 
 softmmu_ss.add(when: 'CONFIG_VGA_PCI', if_true: files('vga-pci.c'))
 softmmu_ss.add(when: 'CONFIG_VGA_ISA', if_true: files('vga-isa.c'))
-softmmu_ss.add(when: 'CONFIG_VGA_ISA_MM', if_true: files('vga-isa-mm.c'))
+softmmu_ss.add(when: 'CONFIG_VGA_MMIO', if_true: files('vga-mmio.c'))
 softmmu_ss.add(when: 'CONFIG_VMWARE_VGA', if_true: files('vmware_vga.c'))
 softmmu_ss.add(when: 'CONFIG_BOCHS_DISPLAY', if_true: files('bochs-display.c'))
 
diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
index b4c5549ce84..725525358d9 100644
--- a/hw/mips/Kconfig
+++ b/hw/mips/Kconfig
@@ -16,7 +16,7 @@ config JAZZ
     select I8254
     select I8257
     select PCSPK
-    select VGA_ISA_MM
+    select VGA_MMIO
     select G364FB
     select DP8393X
     select ESP
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH-for-7.0 2/5] hw/display/vga-mmio: Inline vga_mm_init()
  2021-11-19 17:11 [PATCH-for-7.0 0/5] hw/display: QOM'ify vga_mmio_init() as TYPE_VGA_MMIO Philippe Mathieu-Daudé
  2021-11-19 17:11 ` [PATCH-for-7.0 1/5] hw/display: Rename VGA_ISA_MM -> VGA_MMIO Philippe Mathieu-Daudé
@ 2021-11-19 17:11 ` Philippe Mathieu-Daudé
  2021-11-19 18:20   ` BALATON Zoltan
  2021-11-22 15:08   ` Thomas Huth
  2021-11-19 17:12 ` [PATCH-for-7.0 3/5] hw/display/vga-mmio: QOM'ify vga_mmio_init() as TYPE_VGA_MMIO Philippe Mathieu-Daudé
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-11-19 17:11 UTC (permalink / raw)
  To: qemu-devel
  Cc: Thomas Huth, Hervé Poussineau, Gerd Hoffmann,
	Philippe Mathieu-Daudé

Inline vga_mm_init() in vga_mmio_init() to simplify the
next patch review. Kind of.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/display/vga-mmio.c | 27 ++++++++++-----------------
 1 file changed, 10 insertions(+), 17 deletions(-)

diff --git a/hw/display/vga-mmio.c b/hw/display/vga-mmio.c
index 8aaf44e7b1d..0aefbcf53a0 100644
--- a/hw/display/vga-mmio.c
+++ b/hw/display/vga-mmio.c
@@ -65,12 +65,19 @@ static const MemoryRegionOps vga_mm_ctrl_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static void vga_mm_init(VGAMmioState *s, hwaddr vram_base,
-                        hwaddr ctrl_base, int it_shift,
-                        MemoryRegion *address_space)
+int vga_mmio_init(hwaddr vram_base,
+                    hwaddr ctrl_base, int it_shift,
+                    MemoryRegion *address_space)
 {
+    VGAMmioState *s;
     MemoryRegion *s_ioport_ctrl, *vga_io_memory;
 
+    s = g_malloc0(sizeof(*s));
+
+    s->vga.vram_size_mb = VGA_RAM_SIZE / MiB;
+    s->vga.global_vmstate = true;
+    vga_common_init(&s->vga, NULL);
+
     s->it_shift = it_shift;
     s_ioport_ctrl = g_malloc(sizeof(*s_ioport_ctrl));
     memory_region_init_io(s_ioport_ctrl, NULL, &vga_mm_ctrl_ops, s,
@@ -89,20 +96,6 @@ static void vga_mm_init(VGAMmioState *s, hwaddr vram_base,
     memory_region_add_subregion(address_space,
                                 vram_base + 0x000a0000, vga_io_memory);
     memory_region_set_coalescing(vga_io_memory);
-}
-
-int vga_mmio_init(hwaddr vram_base,
-                    hwaddr ctrl_base, int it_shift,
-                    MemoryRegion *address_space)
-{
-    VGAMmioState *s;
-
-    s = g_malloc0(sizeof(*s));
-
-    s->vga.vram_size_mb = VGA_RAM_SIZE / MiB;
-    s->vga.global_vmstate = true;
-    vga_common_init(&s->vga, NULL);
-    vga_mm_init(s, vram_base, ctrl_base, it_shift, address_space);
 
     s->vga.con = graphic_console_init(NULL, 0, s->vga.hw_ops, s);
 
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH-for-7.0 3/5] hw/display/vga-mmio: QOM'ify vga_mmio_init() as TYPE_VGA_MMIO
  2021-11-19 17:11 [PATCH-for-7.0 0/5] hw/display: QOM'ify vga_mmio_init() as TYPE_VGA_MMIO Philippe Mathieu-Daudé
  2021-11-19 17:11 ` [PATCH-for-7.0 1/5] hw/display: Rename VGA_ISA_MM -> VGA_MMIO Philippe Mathieu-Daudé
  2021-11-19 17:11 ` [PATCH-for-7.0 2/5] hw/display/vga-mmio: Inline vga_mm_init() Philippe Mathieu-Daudé
@ 2021-11-19 17:12 ` Philippe Mathieu-Daudé
  2021-11-19 18:40   ` BALATON Zoltan
  2021-11-19 17:12 ` [PATCH-for-7.0 4/5] hw/mips/jazz: Inline vga_mmio_init() and remove it Philippe Mathieu-Daudé
  2021-11-19 17:12 ` [NOTFORMERGE PATCH 5/5] tests/avocado: Test NetBSD 9.2 on the Jazz Magnum machine Philippe Mathieu-Daudé
  4 siblings, 1 reply; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-11-19 17:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: Thomas Huth, Hervé Poussineau, Gerd Hoffmann,
	Philippe Mathieu-Daudé

Introduce TYPE_VGA_MMIO, a sysbus device.

While there is no change in the vga_mmio_init()
interface, this is a migration compatibility break
of the MIPS Acer Pica 61 Jazz machine (pica61).

Suggested-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/display/vga-mmio.c | 134 +++++++++++++++++++++++++++++-------------
 1 file changed, 94 insertions(+), 40 deletions(-)

diff --git a/hw/display/vga-mmio.c b/hw/display/vga-mmio.c
index 0aefbcf53a0..d1c5f31c134 100644
--- a/hw/display/vga-mmio.c
+++ b/hw/display/vga-mmio.c
@@ -2,6 +2,7 @@
  * QEMU MMIO VGA Emulator.
  *
  * Copyright (c) 2003 Fabrice Bellard
+ * Copyright (c) 2021 Philippe Mathieu-Daudé
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -23,21 +24,34 @@
  */
 
 #include "qemu/osdep.h"
-#include "qemu/bitops.h"
-#include "qemu/units.h"
-#include "migration/vmstate.h"
+#include "qapi/error.h"
 #include "hw/display/vga.h"
+#include "hw/sysbus.h"
+#include "hw/qdev-properties.h"
 #include "vga_int.h"
-#include "ui/pixel_ops.h"
 
-#define VGA_RAM_SIZE (8 * MiB)
+/*
+ * QEMU interface:
+ *  + sysbus MMIO region 0: VGA I/O registers
+ *  + sysbus MMIO region 1: VGA MMIO registers
+ *  + sysbus MMIO region 2: VGA memory
+ */
 
-typedef struct VGAMmioState {
+#define TYPE_VGA_MMIO "vga-mmio"
+OBJECT_DECLARE_SIMPLE_TYPE(VGAMmioState, VGA_MMIO)
+
+struct VGAMmioState {
+    /*< private >*/
+    SysBusDevice parent_obj;
+
+    /*< public >*/
     VGACommonState vga;
-    int it_shift;
-} VGAMmioState;
+    MemoryRegion iomem;
+    MemoryRegion lowmem;
+
+    uint8_t it_shift;
+};
 
-/* Memory mapped interface */
 static uint64_t vga_mm_read(void *opaque, hwaddr addr, unsigned size)
 {
     VGAMmioState *s = opaque;
@@ -65,43 +79,83 @@ static const MemoryRegionOps vga_mm_ctrl_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
+static void vga_mmio_reset(DeviceState *dev)
+{
+    VGAMmioState *d = VGA_MMIO(dev);
+    VGACommonState *s = &d->vga;
+
+    vga_common_reset(s);
+}
+
 int vga_mmio_init(hwaddr vram_base,
                     hwaddr ctrl_base, int it_shift,
                     MemoryRegion *address_space)
 {
-    VGAMmioState *s;
-    MemoryRegion *s_ioport_ctrl, *vga_io_memory;
+    DeviceState *dev;
+    SysBusDevice *s;
 
-    s = g_malloc0(sizeof(*s));
+    dev = qdev_new(TYPE_VGA_MMIO);
+    qdev_prop_set_uint8(dev, "it_shift", it_shift);
+    s = SYS_BUS_DEVICE(dev);
+    sysbus_realize_and_unref(s, &error_fatal);
 
-    s->vga.vram_size_mb = VGA_RAM_SIZE / MiB;
-    s->vga.global_vmstate = true;
-    vga_common_init(&s->vga, NULL);
-
-    s->it_shift = it_shift;
-    s_ioport_ctrl = g_malloc(sizeof(*s_ioport_ctrl));
-    memory_region_init_io(s_ioport_ctrl, NULL, &vga_mm_ctrl_ops, s,
-                          "vga-mm-ctrl", 0x100000);
-    memory_region_set_flush_coalesced(s_ioport_ctrl);
-
-    vga_io_memory = g_malloc(sizeof(*vga_io_memory));
-    /* XXX: endianness? */
-    memory_region_init_io(vga_io_memory, NULL, &vga_mem_ops, &s->vga,
-                          "vga-mem", 0x20000);
-
-    vmstate_register(NULL, 0, &vmstate_vga_common, s);
-
-    memory_region_add_subregion(address_space, ctrl_base, s_ioport_ctrl);
-    s->vga.bank_offset = 0;
-    memory_region_add_subregion(address_space,
-                                vram_base + 0x000a0000, vga_io_memory);
-    memory_region_set_coalescing(vga_io_memory);
-
-    s->vga.con = graphic_console_init(NULL, 0, s->vga.hw_ops, s);
-
-    memory_region_add_subregion(address_space,
-                                VBE_DISPI_LFB_PHYSICAL_ADDRESS,
-                                &s->vga.vram);
+    sysbus_mmio_map(s, 0, ctrl_base);
+    sysbus_mmio_map(s, 1, vram_base + 0x000a0000);
+    sysbus_mmio_map(s, 2, VBE_DISPI_LFB_PHYSICAL_ADDRESS);
 
     return 0;
 }
+
+static void vga_mmio_realizefn(DeviceState *dev, Error **errp)
+{
+    VGAMmioState *s = VGA_MMIO(dev);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+
+    memory_region_init_io(&s->iomem, OBJECT(dev), &vga_mm_ctrl_ops, s,
+                          "vga-mmio", 0x100000);
+    memory_region_set_flush_coalesced(&s->iomem);
+    sysbus_init_mmio(sbd, &s->iomem);
+
+    /* XXX: endianness? */
+    memory_region_init_io(&s->lowmem, OBJECT(dev), &vga_mem_ops, &s->vga,
+                          "vga-lowmem", 0x20000);
+    memory_region_set_coalescing(&s->lowmem);
+    sysbus_init_mmio(sbd, &s->lowmem);
+
+    s->vga.bank_offset = 0;
+    s->vga.global_vmstate = true;
+    vga_common_init(&s->vga, OBJECT(dev));
+    sysbus_init_mmio(sbd, &s->vga.vram);
+    s->vga.con = graphic_console_init(dev, 0, s->vga.hw_ops, &s->vga);
+}
+
+static Property vga_mmio_properties[] = {
+    DEFINE_PROP_UINT8("it_shift", VGAMmioState, it_shift, 0),
+    DEFINE_PROP_UINT32("vgamem_mb", VGAMmioState, vga.vram_size_mb, 8),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void vga_mmio_class_initfn(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = vga_mmio_realizefn;
+    dc->reset = vga_mmio_reset;
+    dc->vmsd = &vmstate_vga_common;
+    device_class_set_props(dc, vga_mmio_properties);
+    set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories);
+}
+
+static const TypeInfo vga_mmio_info = {
+    .name          = TYPE_VGA_MMIO,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(VGAMmioState),
+    .class_init    = vga_mmio_class_initfn,
+};
+
+static void vga_mmio_register_types(void)
+{
+    type_register_static(&vga_mmio_info);
+}
+
+type_init(vga_mmio_register_types)
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH-for-7.0 4/5] hw/mips/jazz: Inline vga_mmio_init() and remove it
  2021-11-19 17:11 [PATCH-for-7.0 0/5] hw/display: QOM'ify vga_mmio_init() as TYPE_VGA_MMIO Philippe Mathieu-Daudé
                   ` (2 preceding siblings ...)
  2021-11-19 17:12 ` [PATCH-for-7.0 3/5] hw/display/vga-mmio: QOM'ify vga_mmio_init() as TYPE_VGA_MMIO Philippe Mathieu-Daudé
@ 2021-11-19 17:12 ` Philippe Mathieu-Daudé
  2021-11-19 18:43   ` BALATON Zoltan
  2021-11-19 17:12 ` [NOTFORMERGE PATCH 5/5] tests/avocado: Test NetBSD 9.2 on the Jazz Magnum machine Philippe Mathieu-Daudé
  4 siblings, 1 reply; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-11-19 17:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: Thomas Huth, Hervé Poussineau, Gerd Hoffmann,
	Philippe Mathieu-Daudé

vga_mmio_init() is used only one time and not very helpful,
inline and remove it.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/display/vga.h |  6 ------
 hw/display/vga-mmio.c    | 20 --------------------
 hw/mips/jazz.c           |  9 ++++++++-
 3 files changed, 8 insertions(+), 27 deletions(-)

diff --git a/include/hw/display/vga.h b/include/hw/display/vga.h
index 03c65a14218..451e4c9898c 100644
--- a/include/hw/display/vga.h
+++ b/include/hw/display/vga.h
@@ -9,8 +9,6 @@
 #ifndef QEMU_HW_DISPLAY_VGA_H
 #define QEMU_HW_DISPLAY_VGA_H
 
-#include "exec/hwaddr.h"
-
 /*
  * modules can reference this symbol to avoid being loaded
  * into system emulators without vga support
@@ -24,8 +22,4 @@ enum vga_retrace_method {
 
 extern enum vga_retrace_method vga_retrace_method;
 
-int vga_mmio_init(hwaddr vram_base,
-                    hwaddr ctrl_base, int it_shift,
-                    MemoryRegion *address_space);
-
 #endif
diff --git a/hw/display/vga-mmio.c b/hw/display/vga-mmio.c
index d1c5f31c134..af9229794c9 100644
--- a/hw/display/vga-mmio.c
+++ b/hw/display/vga-mmio.c
@@ -25,7 +25,6 @@
 
 #include "qemu/osdep.h"
 #include "qapi/error.h"
-#include "hw/display/vga.h"
 #include "hw/sysbus.h"
 #include "hw/qdev-properties.h"
 #include "vga_int.h"
@@ -87,25 +86,6 @@ static void vga_mmio_reset(DeviceState *dev)
     vga_common_reset(s);
 }
 
-int vga_mmio_init(hwaddr vram_base,
-                    hwaddr ctrl_base, int it_shift,
-                    MemoryRegion *address_space)
-{
-    DeviceState *dev;
-    SysBusDevice *s;
-
-    dev = qdev_new(TYPE_VGA_MMIO);
-    qdev_prop_set_uint8(dev, "it_shift", it_shift);
-    s = SYS_BUS_DEVICE(dev);
-    sysbus_realize_and_unref(s, &error_fatal);
-
-    sysbus_mmio_map(s, 0, ctrl_base);
-    sysbus_mmio_map(s, 1, vram_base + 0x000a0000);
-    sysbus_mmio_map(s, 2, VBE_DISPI_LFB_PHYSICAL_ADDRESS);
-
-    return 0;
-}
-
 static void vga_mmio_realizefn(DeviceState *dev, Error **errp)
 {
     VGAMmioState *s = VGA_MMIO(dev);
diff --git a/hw/mips/jazz.c b/hw/mips/jazz.c
index 8f345afd137..bd9815c773e 100644
--- a/hw/mips/jazz.c
+++ b/hw/mips/jazz.c
@@ -43,6 +43,7 @@
 #include "hw/rtc/mc146818rtc.h"
 #include "hw/timer/i8254.h"
 #include "hw/display/vga.h"
+#include "hw/display/bochs-vbe.h"
 #include "hw/audio/pcspk.h"
 #include "hw/input/i8042.h"
 #include "hw/sysbus.h"
@@ -274,7 +275,13 @@ static void mips_jazz_init(MachineState *machine,
         }
         break;
     case JAZZ_PICA61:
-        vga_mmio_init(0x40000000, 0x60000000, 0, get_system_memory());
+        dev = qdev_new("vga-mmio");
+        qdev_prop_set_uint8(dev, "it_shift", 0);
+        sysbus = SYS_BUS_DEVICE(dev);
+        sysbus_realize_and_unref(sysbus, &error_fatal);
+        sysbus_mmio_map(sysbus, 0, 0x60000000);
+        sysbus_mmio_map(sysbus, 1, 0x400a0000);
+        sysbus_mmio_map(sysbus, 2, VBE_DISPI_LFB_PHYSICAL_ADDRESS);
         break;
     default:
         break;
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [NOTFORMERGE PATCH 5/5] tests/avocado: Test NetBSD 9.2 on the Jazz Magnum machine
  2021-11-19 17:11 [PATCH-for-7.0 0/5] hw/display: QOM'ify vga_mmio_init() as TYPE_VGA_MMIO Philippe Mathieu-Daudé
                   ` (3 preceding siblings ...)
  2021-11-19 17:12 ` [PATCH-for-7.0 4/5] hw/mips/jazz: Inline vga_mmio_init() and remove it Philippe Mathieu-Daudé
@ 2021-11-19 17:12 ` Philippe Mathieu-Daudé
  2021-11-22 20:05   ` Willian Rampazzo
  4 siblings, 1 reply; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-11-19 17:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: Thomas Huth, Finn Thain, Mark Cave-Ayland,
	Philippe Mathieu-Daudé,
	Hervé Poussineau, Gerd Hoffmann

Test NetBSD 9.2 on the Jazz Magnum machine. As the firmware is not
redistributable, it has to be extracted from the floppy configuration
disk coming with a Mips Magnum 4000 system, then the NTPROM_BIN_PATH
environment variable has to be set. For convenience a NVRAM pre-
initialized to boot NetBSD is included. The test can be run as:

  $ NTPROM_BIN_PATH=/path/to/ntprom.bin \
    avocado --show=app,console \
    run -t machine:magnum tests/avocado/
  Fetching asset from tests/avocado/machine_mips_jazz.py:MipsJazz.test_magnum_netbsd_9_2
   (1/1) tests/avocado/machine_mips_jazz.py:MipsJazz.test_magnum_netbsd_9_2:
  console: EISA Bus 0 Initialization In Progress... Direct Memory Access (DMA) System Control Port B Timer 1 OK.
  console: ARC Multiboot Version 174 (SGI Version 2.6)
  console: Copyright (c) 1991,1992  Microsoft Corporation
  console: Actions:
  console: Start Windows NT
  console: Run a program
  console: Run setup
  console: Use the arrow keys to select.
  console: Press Enter to choose.
  console: Program to run:
  console: scsi(0)cdrom(2)fdisk(0)boot scsi(0)cdrom(2)fdisk(0)netbsd
  console: NetBSD/arc Bootstrap, Revision 1.1 (Wed May 12 13:15:55 UTC 2021)
  console: devopen: scsi(0)cdrom(2)fdisk(0) type disk file netbsd
  console: NetBSD 9.2 (RAMDISK) #0: Wed May 12 13:15:55 UTC 2021
  console: MIPS Magnum
  console: total memory = 128 MB
  console: avail memory = 117 MB
  console: mainbus0 (root)
  console: cpu0 at mainbus0: MIPS R4000 CPU (0x400) Rev. 0.0 with MIPS R4010 FPC Rev. 0.0
  console: cpu0: 8KB/16B direct-mapped L1 Instruction cache, 48 TLB entries
  console: cpu0: 8KB/16B direct-mapped write-back L1 Data cache
  console: jazzio0 at mainbus0
  console: timer0 at jazzio0 addr 0xe0000228
  console: mcclock0 at jazzio0 addr 0xe0004000: mc146818 compatible time-of-day clock
  console: LPT1 at jazzio0 addr 0xe0008000 intr 0 not configured
  console: fdc0 at jazzio0 addr 0xe0003000 intr 1
  console: fd0 at fdc0 drive 1: 1.44MB, 80 cyl, 2 head, 18 sec
  console: MAGNUM at jazzio0 addr 0xe000c000 intr 2 not configured
  console: VXL at jazzio0 addr 0xe0800000 intr 3 not configured
  console: sn0 at jazzio0 addr 0xe0001000 intr 4: SONIC Ethernet
  console: sn0: Ethernet address 00:00:00:00:00:00
  console: asc0 at jazzio0 addr 0xe0002000 intr 5: NCR53C94, 25MHz, SCSI ID 7
  console: scsibus0 at asc0: 8 targets, 8 luns per target
  console: pckbc0 at jazzio0 addr 0xe0005000 intr 6
  console: pckbd0 at pckbc0 (kbd slot)
  console: wskbd0 at pckbd0 (mux ignored)
  console: pms at jazzio0 addr 0xe0005000 intr 7 not configured
  console: com0 at jazzio0 addr 0xe0006000 intr 8: ns16550a, working fifo
  console: com0: txfifo disabled
  console: com0: console
  console: com1 at jazzio0 addr 0xe0007000 intr 9: ns16550a, working fifo
  console: com1: txfifo disabled
  console: jazzisabr0 at mainbus0
  console: isa0 at jazzisabr0
  console: isapnp0 at isa0 port 0x279: ISA Plug 'n Play device support
  console: scsibus0: waiting 2 seconds for devices to settle...
  console: cd0 at scsibus0 target 2 lun 0: <QEMU, QEMU CD-ROM, 2.5+> cdrom removable
  console: boot device: <unknown>
  console: root on md0a dumps on md0b
  console: root file system type: ffs
  console: WARNING: preposterous TOD clock time
  console: WARNING: using filesystem time
  console: WARNING: CHECK AND RESET THE DATE!
  console: erase ^H, werase ^W, kill ^U, intr ^C, status ^T
  console: Terminal type? [vt100]
  console: Erase is backspace.
  console: S
  console: (I)nstall, (S)hell or (H)alt ?
  console: #
  console: # ifconfig
  console: sn0: flags=0x8802<BROADCAST,SIMPLEX,MULTICAST> mtu 1500
  console: ec_capabilities=1<VLAN_MTU>
  console: ec_enabled=0
  console: address: 00:00:00:02:03:04
  console: lo0: flags=0x8048<LOOPBACK,RUNNING,MULTICAST> mtu 33160
  console: #
  console: # ifconfig sn0 10.0.2.3/24
  console: #
  console: # ping -c 3 10.0.2.2
  console: PING 10.0.2.2 (10.0.2.2): 56 data bytes
  console: 64 bytes from 10.0.2.2: icmp_seq=0 ttl=255 time=12.526 ms
  console: 64 bytes from 10.0.2.2: icmp_seq=1 ttl=255 time=2.324 ms
  console: 64 bytes from 10.0.2.2: icmp_seq=2 ttl=255 time=0.608 ms
  console: ----10.0.2.2 PING Statistics----
  console: 3 packets transmitted, 3 packets received, 0.0% packet loss
  console: # shutdown -r now
  console: round-trip min/avg/max/stddev = 0.608/5.153/12.526/6.443 ms
  console: # Shutdown NOW!
  console: shutdown: [pid 14]
  console: # sh: /usr/bin/wall: not found
  console: reboot by root:
  console: System shutdown time has arrived
  console: About to run shutdown hooks...
  console: .: Can't open /etc/rc.shutdown
  console: Done running shutdown hooks.
  console: syncing disks... done
  console: unmounting file systems... done
  console: rebooting...
  PASS (49.27 s)
  RESULTS    : PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
  JOB TIME   : 49.70 s

Inspired-by: Hervé Poussineau <hpoussin@reactos.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
v2:
- Test NetBSD 9.2 (Finn, Mark)
- Drop '-global ds1225y.size=8200' (Mark)
- Mention "Run a program" option (Mark)
- Check ARP (Finn)

Not for merge until nvram.bin is generated.

Cc: Finn Thain <fthain@linux-m68k.org>
Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 .../avocado/machine_mips_jazz.d/nvram.bin.xz  | Bin 0 -> 700 bytes
 tests/avocado/machine_mips_jazz.py            |  99 ++++++++++++++++++
 2 files changed, 99 insertions(+)
 create mode 100644 tests/avocado/machine_mips_jazz.d/nvram.bin.xz
 create mode 100644 tests/avocado/machine_mips_jazz.py

diff --git a/tests/avocado/machine_mips_jazz.d/nvram.bin.xz b/tests/avocado/machine_mips_jazz.d/nvram.bin.xz
new file mode 100644
index 0000000000000000000000000000000000000000..4648bb31a75bd1a6ee06818a1bf0f2109203ced3
GIT binary patch
literal 700
zcmV;t0z>`%H+ooF000E$*0e?f03iV!0000G&sfah5B~ysT>t=TewJhU2JbgNgw@93
z9ne@Of$dv11>H(&qfaP~{?QVNDD*)y9o<}llIpWo;zi4K%|O3oZX7;e0I%^w=#ho%
zm*3>5KcVeDb)hM{s=^@xl3*6CLiEr>=o;i$-UTTwy=`4vF_aQ@AM-}hS_gfV?dXL~
zr6}Ck7jr0Pd6X49Gycxm@5wylC8aWC%NAZ^DQmjk9v0R)a7u1hsY5)le9~S_G;wnI
zNEhT-#4+m{FhwCCTK#>itkJ+zGLq(>iH&;ourQ75=5X0GCmKgT34hXv`!lNS@Vv~9
z#dTZb1?<x%e;6UaEZ?$JT@1SBP5my9l7&XSs?W+Ufr||*z035xw~`at6Ee$`W+96+
z+GJc@N)TzeY@b#YPn@xQT6#fKq_~pFo)=4ZKP9K_BO1_^7pVTQNp$u}nKgZ>DkCrj
z^?l-ksd=`Q%7w&-3CkM)A-1nkM@y=-6N=HiA4b<WBKS^=)!oj-ZC_{*DGqZD2VzX1
zK{*4jgOsj!UPEwQ?*VD-2^<k{1^m(NV;8Ya)p-pwGWqQ7CAgX4w5vWiB095j=yfi%
zkXq_?C-K4b7)51+8S-t@;sSpTEtU7x1UZ|?WWEp59W6%y5<X+<RS_F7)N!aN8Lm!&
z-jRJ50VM#t_NLVKQ`#MuEWzq1$^|tn4L1MW?!$H&fc<(y1QHv6-#@=cN*d%QYMaA0
z+L!W3-k!*7dONwgWwK{?$5Yf5d8TRJIgw7WHR@zI6hLrbArpq|%Ax#6+d)m|&Wh%|
z!pU{tbl$cCNXE=B0Pf|j?*P-9X*vQHCH?48MX**q?{Zss*9HdGiu@Se*+j_z00023
i?D;I-mde`z0hR@TAOHX;UY-@P#Ao{g000001X)@jNK0n`

literal 0
HcmV?d00001

diff --git a/tests/avocado/machine_mips_jazz.py b/tests/avocado/machine_mips_jazz.py
new file mode 100644
index 00000000000..1441b7fe5bb
--- /dev/null
+++ b/tests/avocado/machine_mips_jazz.py
@@ -0,0 +1,99 @@
+# Functional tests for the Jazz machines.
+#
+# Copyright (c) 2021 Philippe Mathieu-Daudé <f4bug@amsat.org>
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or later.
+# See the COPYING file in the top-level directory.
+#
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+import os
+import lzma
+import shutil
+
+from avocado import skipUnless
+from avocado_qemu import QemuSystemTest
+from avocado_qemu import exec_command
+from avocado_qemu import exec_command_and_wait_for_pattern
+from avocado_qemu import interrupt_interactive_console_until_pattern
+from avocado_qemu import wait_for_console_pattern
+
+from tesseract_utils import tesseract_available, tesseract_ocr
+
+class MipsJazz(QemuSystemTest):
+
+    timeout = 60
+
+    @skipUnless(os.getenv('NTPROM_BIN_PATH'), 'NTPROM_BIN_PATH not available')
+    def test_magnum_netbsd_9_2(self):
+        """
+        :avocado: tags=arch:mips64el
+        :avocado: tags=machine:magnum
+        :avocado: tags=os:netbsd
+        :avocado: tags=device:sonic
+        :avocado: tags=device:esp
+        """
+        drive_url = ('http://cdn.netbsd.org/pub/NetBSD/'
+                     'NetBSD-9.2/images/NetBSD-9.2-arc.iso')
+        drive_hash = '409c61aee5459e762cdb120d2591ed2e'
+        drive_path = self.fetch_asset(drive_url, asset_hash=drive_hash,
+                                      algorithm='md5')
+        ntprom_hash = '316de17820192c89b8ee6d9936ab8364a739ca53'
+        ntprom_path = self.fetch_asset('file://' + os.getenv('NTPROM_BIN_PATH'),
+                                       asset_hash=ntprom_hash, algorithm='sha1')
+        nvram_size = 8200
+        nvram_path = 'nvram.bin'
+        nvram_xz_hash = '3d4565124ff2369706b97e1d0ef127a68c23d418'
+        nvram_xz_path = os.path.dirname(os.path.abspath(__file__)) \
+                        + '/machine_mips_jazz.d/nvram.bin.xz'
+        nvram_xz_path = self.fetch_asset('file://' + nvram_xz_path,
+                                         asset_hash=nvram_xz_hash,
+                                         algorithm='sha1')
+        mac = '00:00:00:02:03:04'
+
+        with lzma.open(nvram_xz_path, 'rb') as f_in:
+            with open(nvram_path, 'wb') as f_out:
+                shutil.copyfileobj(f_in, f_out)
+                f_out.seek(nvram_size)
+                f_out.write(b'\0')
+
+        self.vm.set_console()
+        self.vm.add_args('-bios', ntprom_path,
+                         '-drive', 'if=scsi,unit=2,media=cdrom,format=raw,file='
+                                   + drive_path,
+                         '-global', 'ds1225y.filename=' + nvram_path,
+                         '-nic', 'user,model=dp83932,mac=' + mac)
+        self.vm.launch()
+
+        console_pattern = 'ARC Multiboot Version 174 (SGI Version 2.6)'
+        wait_for_console_pattern(self, console_pattern)
+
+        wait_for_console_pattern(self, 'Use the arrow keys to select.')
+
+        # Press cursor control 'Down' to select the "Run a program" menu
+        exec_command(self, '\x1b[B')
+
+        program = 'scsi(0)cdrom(2)fdisk(0)boot scsi(0)cdrom(2)fdisk(0)netbsd'
+        exec_command(self, program)
+        wait_for_console_pattern(self, 'NetBSD/arc Bootstrap, Revision 1.1')
+
+        # Terminal type? [vt100]
+        console_pattern = 'erase ^H, werase ^W, kill ^U, intr ^C, status ^T'
+        wait_for_console_pattern(self, console_pattern)
+
+        # (I)nstall, (S)hell or (H)alt
+        exec_command_and_wait_for_pattern(self, '', 'Erase is backspace.')
+        exec_command(self, 'S')
+        interrupt_interactive_console_until_pattern(self, '#')
+
+        exec_command_and_wait_for_pattern(self, 'ifconfig', 'address: ' + mac)
+        interrupt_interactive_console_until_pattern(self, '#')
+
+        exec_command(self, 'ifconfig sn0 10.0.2.3/24')
+        interrupt_interactive_console_until_pattern(self, '#')
+
+        exec_command_and_wait_for_pattern(self, 'ping -c 3 10.0.2.2',
+                '3 packets transmitted, 3 packets received, 0.0% packet loss')
+
+        exec_command_and_wait_for_pattern(self, 'shutdown -r now',
+                                          'rebooting...')
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH-for-7.0 1/5] hw/display: Rename VGA_ISA_MM -> VGA_MMIO
  2021-11-19 17:11 ` [PATCH-for-7.0 1/5] hw/display: Rename VGA_ISA_MM -> VGA_MMIO Philippe Mathieu-Daudé
@ 2021-11-19 18:15   ` BALATON Zoltan
  2021-11-22 15:06   ` Thomas Huth
  1 sibling, 0 replies; 15+ messages in thread
From: BALATON Zoltan @ 2021-11-19 18:15 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Thomas Huth, Hervé Poussineau, qemu-devel, Gerd Hoffmann

[-- Attachment #1: Type: text/plain, Size: 878 bytes --]

On Fri, 19 Nov 2021, Philippe Mathieu-Daudé wrote:
> There is no ISA bus part in the MMIO VGA device, so rename:
>
> *  hw/display/vga-isa-mm.c -> hw/display/vga-mmio.c
> *  CONFIG_VGA_ISA_MM -> CONFIG_VGA_MMIO
> *  ISAVGAMMState -> VGAMmioState
> *  isa_vga_mm_init() -> vga_mmio_init()
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>

> ---
> configs/devices/mips-softmmu/common.mak |  2 +-
> include/hw/display/vga.h                |  2 +-
> hw/display/{vga-isa-mm.c => vga-mmio.c} | 16 ++++++++--------
> hw/mips/jazz.c                          |  2 +-
> hw/display/Kconfig                      |  2 +-
> hw/display/meson.build                  |  2 +-
> hw/mips/Kconfig                         |  2 +-
> 7 files changed, 14 insertions(+), 14 deletions(-)
> rename hw/display/{vga-isa-mm.c => vga-mmio.c} (93%)

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH-for-7.0 2/5] hw/display/vga-mmio: Inline vga_mm_init()
  2021-11-19 17:11 ` [PATCH-for-7.0 2/5] hw/display/vga-mmio: Inline vga_mm_init() Philippe Mathieu-Daudé
@ 2021-11-19 18:20   ` BALATON Zoltan
  2021-11-19 18:23     ` BALATON Zoltan
  2021-11-19 18:28     ` Philippe Mathieu-Daudé
  2021-11-22 15:08   ` Thomas Huth
  1 sibling, 2 replies; 15+ messages in thread
From: BALATON Zoltan @ 2021-11-19 18:20 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Thomas Huth, Hervé Poussineau, qemu-devel, Gerd Hoffmann

[-- Attachment #1: Type: text/plain, Size: 2199 bytes --]

On Fri, 19 Nov 2021, Philippe Mathieu-Daudé wrote:
> Inline vga_mm_init() in vga_mmio_init() to simplify the
> next patch review. Kind of.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> hw/display/vga-mmio.c | 27 ++++++++++-----------------
> 1 file changed, 10 insertions(+), 17 deletions(-)
>
> diff --git a/hw/display/vga-mmio.c b/hw/display/vga-mmio.c
> index 8aaf44e7b1d..0aefbcf53a0 100644
> --- a/hw/display/vga-mmio.c
> +++ b/hw/display/vga-mmio.c
> @@ -65,12 +65,19 @@ static const MemoryRegionOps vga_mm_ctrl_ops = {
>     .endianness = DEVICE_NATIVE_ENDIAN,
> };
>
> -static void vga_mm_init(VGAMmioState *s, hwaddr vram_base,
> -                        hwaddr ctrl_base, int it_shift,
> -                        MemoryRegion *address_space)
> +int vga_mmio_init(hwaddr vram_base,
> +                    hwaddr ctrl_base, int it_shift,
> +                    MemoryRegion *address_space)

Indentation? (But it's removed later so does not really matter.)

> {
> +    VGAMmioState *s;
>     MemoryRegion *s_ioport_ctrl, *vga_io_memory;
>
> +    s = g_malloc0(sizeof(*s));
> +
> +    s->vga.vram_size_mb = VGA_RAM_SIZE / MiB;
> +    s->vga.global_vmstate = true;
> +    vga_common_init(&s->vga, NULL);
> +
>     s->it_shift = it_shift;
>     s_ioport_ctrl = g_malloc(sizeof(*s_ioport_ctrl));
>     memory_region_init_io(s_ioport_ctrl, NULL, &vga_mm_ctrl_ops, s,
> @@ -89,20 +96,6 @@ static void vga_mm_init(VGAMmioState *s, hwaddr vram_base,
>     memory_region_add_subregion(address_space,
>                                 vram_base + 0x000a0000, vga_io_memory);
>     memory_region_set_coalescing(vga_io_memory);
> -}
> -
> -int vga_mmio_init(hwaddr vram_base,
> -                    hwaddr ctrl_base, int it_shift,
> -                    MemoryRegion *address_space)
> -{
> -    VGAMmioState *s;
> -
> -    s = g_malloc0(sizeof(*s));
> -
> -    s->vga.vram_size_mb = VGA_RAM_SIZE / MiB;
> -    s->vga.global_vmstate = true;
> -    vga_common_init(&s->vga, NULL);
> -    vga_mm_init(s, vram_base, ctrl_base, it_shift, address_space);

Where did this vga_mm_init() go?

Regards,
BALATON Zoltan

>     s->vga.con = graphic_console_init(NULL, 0, s->vga.hw_ops, s);
>
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH-for-7.0 2/5] hw/display/vga-mmio: Inline vga_mm_init()
  2021-11-19 18:20   ` BALATON Zoltan
@ 2021-11-19 18:23     ` BALATON Zoltan
  2021-11-19 18:28     ` Philippe Mathieu-Daudé
  1 sibling, 0 replies; 15+ messages in thread
From: BALATON Zoltan @ 2021-11-19 18:23 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Thomas Huth, Hervé Poussineau, qemu-devel, Gerd Hoffmann

[-- Attachment #1: Type: text/plain, Size: 2420 bytes --]

On Fri, 19 Nov 2021, BALATON Zoltan wrote:
> On Fri, 19 Nov 2021, Philippe Mathieu-Daudé wrote:
>> Inline vga_mm_init() in vga_mmio_init() to simplify the
>> next patch review. Kind of.
>> 
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> hw/display/vga-mmio.c | 27 ++++++++++-----------------
>> 1 file changed, 10 insertions(+), 17 deletions(-)
>> 
>> diff --git a/hw/display/vga-mmio.c b/hw/display/vga-mmio.c
>> index 8aaf44e7b1d..0aefbcf53a0 100644
>> --- a/hw/display/vga-mmio.c
>> +++ b/hw/display/vga-mmio.c
>> @@ -65,12 +65,19 @@ static const MemoryRegionOps vga_mm_ctrl_ops = {
>>     .endianness = DEVICE_NATIVE_ENDIAN,
>> };
>> 
>> -static void vga_mm_init(VGAMmioState *s, hwaddr vram_base,
>> -                        hwaddr ctrl_base, int it_shift,
>> -                        MemoryRegion *address_space)
>> +int vga_mmio_init(hwaddr vram_base,
>> +                    hwaddr ctrl_base, int it_shift,
>> +                    MemoryRegion *address_space)
>
> Indentation? (But it's removed later so does not really matter.)
>
>> {
>> +    VGAMmioState *s;
>>     MemoryRegion *s_ioport_ctrl, *vga_io_memory;
>> 
>> +    s = g_malloc0(sizeof(*s));
>> +
>> +    s->vga.vram_size_mb = VGA_RAM_SIZE / MiB;
>> +    s->vga.global_vmstate = true;
>> +    vga_common_init(&s->vga, NULL);
>> +
>>     s->it_shift = it_shift;
>>     s_ioport_ctrl = g_malloc(sizeof(*s_ioport_ctrl));
>>     memory_region_init_io(s_ioport_ctrl, NULL, &vga_mm_ctrl_ops, s,
>> @@ -89,20 +96,6 @@ static void vga_mm_init(VGAMmioState *s, hwaddr 
>> vram_base,
>>     memory_region_add_subregion(address_space,
>>                                 vram_base + 0x000a0000, vga_io_memory);
>>     memory_region_set_coalescing(vga_io_memory);
>> -}
>> -
>> -int vga_mmio_init(hwaddr vram_base,
>> -                    hwaddr ctrl_base, int it_shift,
>> -                    MemoryRegion *address_space)
>> -{
>> -    VGAMmioState *s;
>> -
>> -    s = g_malloc0(sizeof(*s));
>> -
>> -    s->vga.vram_size_mb = VGA_RAM_SIZE / MiB;
>> -    s->vga.global_vmstate = true;
>> -    vga_common_init(&s->vga, NULL);
>> -    vga_mm_init(s, vram_base, ctrl_base, it_shift, address_space);
>
> Where did this vga_mm_init() go?

Sorry, this is what's being inlined... So I mean

Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>

> Regards,
> BALATON Zoltan
>
>>     s->vga.con = graphic_console_init(NULL, 0, s->vga.hw_ops, s);
>> 
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH-for-7.0 2/5] hw/display/vga-mmio: Inline vga_mm_init()
  2021-11-19 18:20   ` BALATON Zoltan
  2021-11-19 18:23     ` BALATON Zoltan
@ 2021-11-19 18:28     ` Philippe Mathieu-Daudé
  1 sibling, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-11-19 18:28 UTC (permalink / raw)
  To: BALATON Zoltan
  Cc: Thomas Huth, Hervé Poussineau, qemu-devel, Gerd Hoffmann

On 11/19/21 19:20, BALATON Zoltan wrote:
> On Fri, 19 Nov 2021, Philippe Mathieu-Daudé wrote:
>> Inline vga_mm_init() in vga_mmio_init() to simplify the
>> next patch review. Kind of.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> hw/display/vga-mmio.c | 27 ++++++++++-----------------
>> 1 file changed, 10 insertions(+), 17 deletions(-)
>>
>> diff --git a/hw/display/vga-mmio.c b/hw/display/vga-mmio.c
>> index 8aaf44e7b1d..0aefbcf53a0 100644
>> --- a/hw/display/vga-mmio.c
>> +++ b/hw/display/vga-mmio.c
>> @@ -65,12 +65,19 @@ static const MemoryRegionOps vga_mm_ctrl_ops = {
>>     .endianness = DEVICE_NATIVE_ENDIAN,
>> };
>>
>> -static void vga_mm_init(VGAMmioState *s, hwaddr vram_base,
>> -                        hwaddr ctrl_base, int it_shift,
>> -                        MemoryRegion *address_space)
>> +int vga_mmio_init(hwaddr vram_base,
>> +                    hwaddr ctrl_base, int it_shift,
>> +                    MemoryRegion *address_space)
> 
> Indentation? (But it's removed later so does not really matter.)

Oops I didn't notice.

>> {
>> +    VGAMmioState *s;
>>     MemoryRegion *s_ioport_ctrl, *vga_io_memory;
>>
>> +    s = g_malloc0(sizeof(*s));
>> +
>> +    s->vga.vram_size_mb = VGA_RAM_SIZE / MiB;
>> +    s->vga.global_vmstate = true;
>> +    vga_common_init(&s->vga, NULL);

        ^---- here is vga_mm_init() inlined [*]

>>     s->it_shift = it_shift;
>>     s_ioport_ctrl = g_malloc(sizeof(*s_ioport_ctrl));
>>     memory_region_init_io(s_ioport_ctrl, NULL, &vga_mm_ctrl_ops, s,
>> @@ -89,20 +96,6 @@ static void vga_mm_init(VGAMmioState *s, hwaddr
>> vram_base,
>>     memory_region_add_subregion(address_space,
>>                                 vram_base + 0x000a0000, vga_io_memory);
>>     memory_region_set_coalescing(vga_io_memory);
>> -}
>> -
>> -int vga_mmio_init(hwaddr vram_base,
>> -                    hwaddr ctrl_base, int it_shift,
>> -                    MemoryRegion *address_space)
>> -{
>> -    VGAMmioState *s;
>> -
>> -    s = g_malloc0(sizeof(*s));
>> -
>> -    s->vga.vram_size_mb = VGA_RAM_SIZE / MiB;
>> -    s->vga.global_vmstate = true;
>> -    vga_common_init(&s->vga, NULL);
>> -    vga_mm_init(s, vram_base, ctrl_base, it_shift, address_space);
> 
> Where did this vga_mm_init() go?

Earlier in [*].

> Regards,
> BALATON Zoltan
> 
>>     s->vga.con = graphic_console_init(NULL, 0, s->vga.hw_ops, s);
>>
>>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH-for-7.0 3/5] hw/display/vga-mmio: QOM'ify vga_mmio_init() as TYPE_VGA_MMIO
  2021-11-19 17:12 ` [PATCH-for-7.0 3/5] hw/display/vga-mmio: QOM'ify vga_mmio_init() as TYPE_VGA_MMIO Philippe Mathieu-Daudé
@ 2021-11-19 18:40   ` BALATON Zoltan
  0 siblings, 0 replies; 15+ messages in thread
From: BALATON Zoltan @ 2021-11-19 18:40 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Thomas Huth, Hervé Poussineau, qemu-devel, Gerd Hoffmann

[-- Attachment #1: Type: text/plain, Size: 6691 bytes --]

On Fri, 19 Nov 2021, Philippe Mathieu-Daudé wrote:
> Introduce TYPE_VGA_MMIO, a sysbus device.
>
> While there is no change in the vga_mmio_init()
> interface, this is a migration compatibility break
> of the MIPS Acer Pica 61 Jazz machine (pica61).
>
> Suggested-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> hw/display/vga-mmio.c | 134 +++++++++++++++++++++++++++++-------------
> 1 file changed, 94 insertions(+), 40 deletions(-)
>
> diff --git a/hw/display/vga-mmio.c b/hw/display/vga-mmio.c
> index 0aefbcf53a0..d1c5f31c134 100644
> --- a/hw/display/vga-mmio.c
> +++ b/hw/display/vga-mmio.c
> @@ -2,6 +2,7 @@
>  * QEMU MMIO VGA Emulator.
>  *
>  * Copyright (c) 2003 Fabrice Bellard
> + * Copyright (c) 2021 Philippe Mathieu-Daudé

I think it's OK to add copyright line to record who's to blame if you 
change or rewrite significant parts of a file. In this case just 
QOM-ifying it may not be large enough change for me to add my copyright if 
I were doing this. I don't mind, just thought git blame in this case might 
be enough unless of course you want to also take maintainership of the 
file. :-)

>  *
>  * Permission is hereby granted, free of charge, to any person obtaining a copy
>  * of this software and associated documentation files (the "Software"), to deal
> @@ -23,21 +24,34 @@
>  */
>
> #include "qemu/osdep.h"
> -#include "qemu/bitops.h"
> -#include "qemu/units.h"
> -#include "migration/vmstate.h"
> +#include "qapi/error.h"
> #include "hw/display/vga.h"
> +#include "hw/sysbus.h"
> +#include "hw/qdev-properties.h"
> #include "vga_int.h"
> -#include "ui/pixel_ops.h"
>
> -#define VGA_RAM_SIZE (8 * MiB)
> +/*
> + * QEMU interface:
> + *  + sysbus MMIO region 0: VGA I/O registers
> + *  + sysbus MMIO region 1: VGA MMIO registers
> + *  + sysbus MMIO region 2: VGA memory
> + */
>
> -typedef struct VGAMmioState {
> +#define TYPE_VGA_MMIO "vga-mmio"
> +OBJECT_DECLARE_SIMPLE_TYPE(VGAMmioState, VGA_MMIO)
> +
> +struct VGAMmioState {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +
> +    /*< public >*/
>     VGACommonState vga;
> -    int it_shift;
> -} VGAMmioState;
> +    MemoryRegion iomem;
> +    MemoryRegion lowmem;
> +
> +    uint8_t it_shift;
> +};
>
> -/* Memory mapped interface */
> static uint64_t vga_mm_read(void *opaque, hwaddr addr, unsigned size)
> {
>     VGAMmioState *s = opaque;
> @@ -65,43 +79,83 @@ static const MemoryRegionOps vga_mm_ctrl_ops = {
>     .endianness = DEVICE_NATIVE_ENDIAN,
> };
>
> +static void vga_mmio_reset(DeviceState *dev)
> +{
> +    VGAMmioState *d = VGA_MMIO(dev);
> +    VGACommonState *s = &d->vga;
> +
> +    vga_common_reset(s);

Maybe there's no need for separate variable s here as it's only used once, 
you could just as well write vga_common_reset(&d->vga).

> +}
> +
> int vga_mmio_init(hwaddr vram_base,
>                     hwaddr ctrl_base, int it_shift,
>                     MemoryRegion *address_space)
> {
> -    VGAMmioState *s;
> -    MemoryRegion *s_ioport_ctrl, *vga_io_memory;
> +    DeviceState *dev;
> +    SysBusDevice *s;
>
> -    s = g_malloc0(sizeof(*s));
> +    dev = qdev_new(TYPE_VGA_MMIO);
> +    qdev_prop_set_uint8(dev, "it_shift", it_shift);
> +    s = SYS_BUS_DEVICE(dev);
> +    sysbus_realize_and_unref(s, &error_fatal);
>
> -    s->vga.vram_size_mb = VGA_RAM_SIZE / MiB;
> -    s->vga.global_vmstate = true;
> -    vga_common_init(&s->vga, NULL);
> -
> -    s->it_shift = it_shift;
> -    s_ioport_ctrl = g_malloc(sizeof(*s_ioport_ctrl));
> -    memory_region_init_io(s_ioport_ctrl, NULL, &vga_mm_ctrl_ops, s,
> -                          "vga-mm-ctrl", 0x100000);
> -    memory_region_set_flush_coalesced(s_ioport_ctrl);
> -
> -    vga_io_memory = g_malloc(sizeof(*vga_io_memory));
> -    /* XXX: endianness? */
> -    memory_region_init_io(vga_io_memory, NULL, &vga_mem_ops, &s->vga,
> -                          "vga-mem", 0x20000);
> -
> -    vmstate_register(NULL, 0, &vmstate_vga_common, s);
> -
> -    memory_region_add_subregion(address_space, ctrl_base, s_ioport_ctrl);
> -    s->vga.bank_offset = 0;
> -    memory_region_add_subregion(address_space,
> -                                vram_base + 0x000a0000, vga_io_memory);
> -    memory_region_set_coalescing(vga_io_memory);
> -
> -    s->vga.con = graphic_console_init(NULL, 0, s->vga.hw_ops, s);
> -
> -    memory_region_add_subregion(address_space,
> -                                VBE_DISPI_LFB_PHYSICAL_ADDRESS,
> -                                &s->vga.vram);
> +    sysbus_mmio_map(s, 0, ctrl_base);
> +    sysbus_mmio_map(s, 1, vram_base + 0x000a0000);
> +    sysbus_mmio_map(s, 2, VBE_DISPI_LFB_PHYSICAL_ADDRESS);
>
>     return 0;
> }
> +
> +static void vga_mmio_realizefn(DeviceState *dev, Error **errp)
> +{
> +    VGAMmioState *s = VGA_MMIO(dev);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> +
> +    memory_region_init_io(&s->iomem, OBJECT(dev), &vga_mm_ctrl_ops, s,

But maybe you can store OBJECT(dev) in a local as it's needed more than 
once here. Otherwise

Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>

> +                          "vga-mmio", 0x100000);
> +    memory_region_set_flush_coalesced(&s->iomem);
> +    sysbus_init_mmio(sbd, &s->iomem);
> +
> +    /* XXX: endianness? */
> +    memory_region_init_io(&s->lowmem, OBJECT(dev), &vga_mem_ops, &s->vga,
> +                          "vga-lowmem", 0x20000);
> +    memory_region_set_coalescing(&s->lowmem);
> +    sysbus_init_mmio(sbd, &s->lowmem);
> +
> +    s->vga.bank_offset = 0;
> +    s->vga.global_vmstate = true;
> +    vga_common_init(&s->vga, OBJECT(dev));
> +    sysbus_init_mmio(sbd, &s->vga.vram);
> +    s->vga.con = graphic_console_init(dev, 0, s->vga.hw_ops, &s->vga);
> +}
> +
> +static Property vga_mmio_properties[] = {
> +    DEFINE_PROP_UINT8("it_shift", VGAMmioState, it_shift, 0),
> +    DEFINE_PROP_UINT32("vgamem_mb", VGAMmioState, vga.vram_size_mb, 8),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void vga_mmio_class_initfn(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = vga_mmio_realizefn;
> +    dc->reset = vga_mmio_reset;
> +    dc->vmsd = &vmstate_vga_common;
> +    device_class_set_props(dc, vga_mmio_properties);
> +    set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories);
> +}
> +
> +static const TypeInfo vga_mmio_info = {
> +    .name          = TYPE_VGA_MMIO,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(VGAMmioState),
> +    .class_init    = vga_mmio_class_initfn,
> +};
> +
> +static void vga_mmio_register_types(void)
> +{
> +    type_register_static(&vga_mmio_info);
> +}
> +
> +type_init(vga_mmio_register_types)
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH-for-7.0 4/5] hw/mips/jazz: Inline vga_mmio_init() and remove it
  2021-11-19 17:12 ` [PATCH-for-7.0 4/5] hw/mips/jazz: Inline vga_mmio_init() and remove it Philippe Mathieu-Daudé
@ 2021-11-19 18:43   ` BALATON Zoltan
  0 siblings, 0 replies; 15+ messages in thread
From: BALATON Zoltan @ 2021-11-19 18:43 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Thomas Huth, Hervé Poussineau, qemu-devel, Gerd Hoffmann

[-- Attachment #1: Type: text/plain, Size: 3306 bytes --]

On Fri, 19 Nov 2021, Philippe Mathieu-Daudé wrote:
> vga_mmio_init() is used only one time and not very helpful,
> inline and remove it.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> include/hw/display/vga.h |  6 ------
> hw/display/vga-mmio.c    | 20 --------------------
> hw/mips/jazz.c           |  9 ++++++++-
> 3 files changed, 8 insertions(+), 27 deletions(-)
>
> diff --git a/include/hw/display/vga.h b/include/hw/display/vga.h
> index 03c65a14218..451e4c9898c 100644
> --- a/include/hw/display/vga.h
> +++ b/include/hw/display/vga.h
> @@ -9,8 +9,6 @@
> #ifndef QEMU_HW_DISPLAY_VGA_H
> #define QEMU_HW_DISPLAY_VGA_H
>
> -#include "exec/hwaddr.h"
> -
> /*
>  * modules can reference this symbol to avoid being loaded
>  * into system emulators without vga support
> @@ -24,8 +22,4 @@ enum vga_retrace_method {
>
> extern enum vga_retrace_method vga_retrace_method;
>
> -int vga_mmio_init(hwaddr vram_base,
> -                    hwaddr ctrl_base, int it_shift,
> -                    MemoryRegion *address_space);
> -
> #endif
> diff --git a/hw/display/vga-mmio.c b/hw/display/vga-mmio.c
> index d1c5f31c134..af9229794c9 100644
> --- a/hw/display/vga-mmio.c
> +++ b/hw/display/vga-mmio.c
> @@ -25,7 +25,6 @@
>
> #include "qemu/osdep.h"
> #include "qapi/error.h"
> -#include "hw/display/vga.h"
> #include "hw/sysbus.h"
> #include "hw/qdev-properties.h"
> #include "vga_int.h"
> @@ -87,25 +86,6 @@ static void vga_mmio_reset(DeviceState *dev)
>     vga_common_reset(s);
> }
>
> -int vga_mmio_init(hwaddr vram_base,
> -                    hwaddr ctrl_base, int it_shift,
> -                    MemoryRegion *address_space)
> -{
> -    DeviceState *dev;
> -    SysBusDevice *s;
> -
> -    dev = qdev_new(TYPE_VGA_MMIO);
> -    qdev_prop_set_uint8(dev, "it_shift", it_shift);
> -    s = SYS_BUS_DEVICE(dev);
> -    sysbus_realize_and_unref(s, &error_fatal);
> -
> -    sysbus_mmio_map(s, 0, ctrl_base);
> -    sysbus_mmio_map(s, 1, vram_base + 0x000a0000);
> -    sysbus_mmio_map(s, 2, VBE_DISPI_LFB_PHYSICAL_ADDRESS);
> -
> -    return 0;
> -}
> -
> static void vga_mmio_realizefn(DeviceState *dev, Error **errp)
> {
>     VGAMmioState *s = VGA_MMIO(dev);
> diff --git a/hw/mips/jazz.c b/hw/mips/jazz.c
> index 8f345afd137..bd9815c773e 100644
> --- a/hw/mips/jazz.c
> +++ b/hw/mips/jazz.c
> @@ -43,6 +43,7 @@
> #include "hw/rtc/mc146818rtc.h"
> #include "hw/timer/i8254.h"
> #include "hw/display/vga.h"
> +#include "hw/display/bochs-vbe.h"
> #include "hw/audio/pcspk.h"
> #include "hw/input/i8042.h"
> #include "hw/sysbus.h"
> @@ -274,7 +275,13 @@ static void mips_jazz_init(MachineState *machine,
>         }
>         break;
>     case JAZZ_PICA61:
> -        vga_mmio_init(0x40000000, 0x60000000, 0, get_system_memory());
> +        dev = qdev_new("vga-mmio");

Does it worth moving TYPE_VGA_MMIO to vga.h so you could use it here? 
Other than that:

Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>

> +        qdev_prop_set_uint8(dev, "it_shift", 0);
> +        sysbus = SYS_BUS_DEVICE(dev);
> +        sysbus_realize_and_unref(sysbus, &error_fatal);
> +        sysbus_mmio_map(sysbus, 0, 0x60000000);
> +        sysbus_mmio_map(sysbus, 1, 0x400a0000);
> +        sysbus_mmio_map(sysbus, 2, VBE_DISPI_LFB_PHYSICAL_ADDRESS);
>         break;
>     default:
>         break;
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH-for-7.0 1/5] hw/display: Rename VGA_ISA_MM -> VGA_MMIO
  2021-11-19 17:11 ` [PATCH-for-7.0 1/5] hw/display: Rename VGA_ISA_MM -> VGA_MMIO Philippe Mathieu-Daudé
  2021-11-19 18:15   ` BALATON Zoltan
@ 2021-11-22 15:06   ` Thomas Huth
  1 sibling, 0 replies; 15+ messages in thread
From: Thomas Huth @ 2021-11-22 15:06 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Hervé Poussineau, Gerd Hoffmann

On 19/11/2021 18.11, Philippe Mathieu-Daudé wrote:
> There is no ISA bus part in the MMIO VGA device, so rename:
> 
>   *  hw/display/vga-isa-mm.c -> hw/display/vga-mmio.c
>   *  CONFIG_VGA_ISA_MM -> CONFIG_VGA_MMIO
>   *  ISAVGAMMState -> VGAMmioState
>   *  isa_vga_mm_init() -> vga_mmio_init()
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   configs/devices/mips-softmmu/common.mak |  2 +-
>   include/hw/display/vga.h                |  2 +-
>   hw/display/{vga-isa-mm.c => vga-mmio.c} | 16 ++++++++--------
>   hw/mips/jazz.c                          |  2 +-
>   hw/display/Kconfig                      |  2 +-
>   hw/display/meson.build                  |  2 +-
>   hw/mips/Kconfig                         |  2 +-
>   7 files changed, 14 insertions(+), 14 deletions(-)
>   rename hw/display/{vga-isa-mm.c => vga-mmio.c} (93%)

Reviewed-by: Thomas Huth <thuth@redhat.com>



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH-for-7.0 2/5] hw/display/vga-mmio: Inline vga_mm_init()
  2021-11-19 17:11 ` [PATCH-for-7.0 2/5] hw/display/vga-mmio: Inline vga_mm_init() Philippe Mathieu-Daudé
  2021-11-19 18:20   ` BALATON Zoltan
@ 2021-11-22 15:08   ` Thomas Huth
  1 sibling, 0 replies; 15+ messages in thread
From: Thomas Huth @ 2021-11-22 15:08 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Hervé Poussineau, Gerd Hoffmann

On 19/11/2021 18.11, Philippe Mathieu-Daudé wrote:
> Inline vga_mm_init() in vga_mmio_init() to simplify the
> next patch review. Kind of.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   hw/display/vga-mmio.c | 27 ++++++++++-----------------
>   1 file changed, 10 insertions(+), 17 deletions(-)
> 
> diff --git a/hw/display/vga-mmio.c b/hw/display/vga-mmio.c
> index 8aaf44e7b1d..0aefbcf53a0 100644
> --- a/hw/display/vga-mmio.c
> +++ b/hw/display/vga-mmio.c
> @@ -65,12 +65,19 @@ static const MemoryRegionOps vga_mm_ctrl_ops = {
>       .endianness = DEVICE_NATIVE_ENDIAN,
>   };
>   
> -static void vga_mm_init(VGAMmioState *s, hwaddr vram_base,
> -                        hwaddr ctrl_base, int it_shift,
> -                        MemoryRegion *address_space)
> +int vga_mmio_init(hwaddr vram_base,
> +                    hwaddr ctrl_base, int it_shift,
> +                    MemoryRegion *address_space)
>   {
> +    VGAMmioState *s;
>       MemoryRegion *s_ioport_ctrl, *vga_io_memory;
>   
> +    s = g_malloc0(sizeof(*s));
> +
> +    s->vga.vram_size_mb = VGA_RAM_SIZE / MiB;
> +    s->vga.global_vmstate = true;
> +    vga_common_init(&s->vga, NULL);
> +
>       s->it_shift = it_shift;
>       s_ioport_ctrl = g_malloc(sizeof(*s_ioport_ctrl));
>       memory_region_init_io(s_ioport_ctrl, NULL, &vga_mm_ctrl_ops, s,
> @@ -89,20 +96,6 @@ static void vga_mm_init(VGAMmioState *s, hwaddr vram_base,
>       memory_region_add_subregion(address_space,
>                                   vram_base + 0x000a0000, vga_io_memory);
>       memory_region_set_coalescing(vga_io_memory);
> -}
> -
> -int vga_mmio_init(hwaddr vram_base,
> -                    hwaddr ctrl_base, int it_shift,
> -                    MemoryRegion *address_space)

Preferably with the indentation fixed (already in the first patch):

Reviewed-by: Thomas Huth <thuth@redhat.com>



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [NOTFORMERGE PATCH 5/5] tests/avocado: Test NetBSD 9.2 on the Jazz Magnum machine
  2021-11-19 17:12 ` [NOTFORMERGE PATCH 5/5] tests/avocado: Test NetBSD 9.2 on the Jazz Magnum machine Philippe Mathieu-Daudé
@ 2021-11-22 20:05   ` Willian Rampazzo
  0 siblings, 0 replies; 15+ messages in thread
From: Willian Rampazzo @ 2021-11-22 20:05 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Thomas Huth, Finn Thain, Mark Cave-Ayland, qemu-devel,
	Hervé Poussineau, Gerd Hoffmann

On Fri, Nov 19, 2021 at 2:29 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Test NetBSD 9.2 on the Jazz Magnum machine. As the firmware is not
> redistributable, it has to be extracted from the floppy configuration
> disk coming with a Mips Magnum 4000 system, then the NTPROM_BIN_PATH
> environment variable has to be set. For convenience a NVRAM pre-
> initialized to boot NetBSD is included. The test can be run as:
>
>   $ NTPROM_BIN_PATH=/path/to/ntprom.bin \
>     avocado --show=app,console \
>     run -t machine:magnum tests/avocado/
>   Fetching asset from tests/avocado/machine_mips_jazz.py:MipsJazz.test_magnum_netbsd_9_2
>    (1/1) tests/avocado/machine_mips_jazz.py:MipsJazz.test_magnum_netbsd_9_2:
>   console: EISA Bus 0 Initialization In Progress... Direct Memory Access (DMA) System Control Port B Timer 1 OK.
>   console: ARC Multiboot Version 174 (SGI Version 2.6)
>   console: Copyright (c) 1991,1992  Microsoft Corporation
>   console: Actions:
>   console: Start Windows NT
>   console: Run a program
>   console: Run setup
>   console: Use the arrow keys to select.
>   console: Press Enter to choose.
>   console: Program to run:
>   console: scsi(0)cdrom(2)fdisk(0)boot scsi(0)cdrom(2)fdisk(0)netbsd
>   console: NetBSD/arc Bootstrap, Revision 1.1 (Wed May 12 13:15:55 UTC 2021)
>   console: devopen: scsi(0)cdrom(2)fdisk(0) type disk file netbsd
>   console: NetBSD 9.2 (RAMDISK) #0: Wed May 12 13:15:55 UTC 2021
>   console: MIPS Magnum
>   console: total memory = 128 MB
>   console: avail memory = 117 MB
>   console: mainbus0 (root)
>   console: cpu0 at mainbus0: MIPS R4000 CPU (0x400) Rev. 0.0 with MIPS R4010 FPC Rev. 0.0
>   console: cpu0: 8KB/16B direct-mapped L1 Instruction cache, 48 TLB entries
>   console: cpu0: 8KB/16B direct-mapped write-back L1 Data cache
>   console: jazzio0 at mainbus0
>   console: timer0 at jazzio0 addr 0xe0000228
>   console: mcclock0 at jazzio0 addr 0xe0004000: mc146818 compatible time-of-day clock
>   console: LPT1 at jazzio0 addr 0xe0008000 intr 0 not configured
>   console: fdc0 at jazzio0 addr 0xe0003000 intr 1
>   console: fd0 at fdc0 drive 1: 1.44MB, 80 cyl, 2 head, 18 sec
>   console: MAGNUM at jazzio0 addr 0xe000c000 intr 2 not configured
>   console: VXL at jazzio0 addr 0xe0800000 intr 3 not configured
>   console: sn0 at jazzio0 addr 0xe0001000 intr 4: SONIC Ethernet
>   console: sn0: Ethernet address 00:00:00:00:00:00
>   console: asc0 at jazzio0 addr 0xe0002000 intr 5: NCR53C94, 25MHz, SCSI ID 7
>   console: scsibus0 at asc0: 8 targets, 8 luns per target
>   console: pckbc0 at jazzio0 addr 0xe0005000 intr 6
>   console: pckbd0 at pckbc0 (kbd slot)
>   console: wskbd0 at pckbd0 (mux ignored)
>   console: pms at jazzio0 addr 0xe0005000 intr 7 not configured
>   console: com0 at jazzio0 addr 0xe0006000 intr 8: ns16550a, working fifo
>   console: com0: txfifo disabled
>   console: com0: console
>   console: com1 at jazzio0 addr 0xe0007000 intr 9: ns16550a, working fifo
>   console: com1: txfifo disabled
>   console: jazzisabr0 at mainbus0
>   console: isa0 at jazzisabr0
>   console: isapnp0 at isa0 port 0x279: ISA Plug 'n Play device support
>   console: scsibus0: waiting 2 seconds for devices to settle...
>   console: cd0 at scsibus0 target 2 lun 0: <QEMU, QEMU CD-ROM, 2.5+> cdrom removable
>   console: boot device: <unknown>
>   console: root on md0a dumps on md0b
>   console: root file system type: ffs
>   console: WARNING: preposterous TOD clock time
>   console: WARNING: using filesystem time
>   console: WARNING: CHECK AND RESET THE DATE!
>   console: erase ^H, werase ^W, kill ^U, intr ^C, status ^T
>   console: Terminal type? [vt100]
>   console: Erase is backspace.
>   console: S
>   console: (I)nstall, (S)hell or (H)alt ?
>   console: #
>   console: # ifconfig
>   console: sn0: flags=0x8802<BROADCAST,SIMPLEX,MULTICAST> mtu 1500
>   console: ec_capabilities=1<VLAN_MTU>
>   console: ec_enabled=0
>   console: address: 00:00:00:02:03:04
>   console: lo0: flags=0x8048<LOOPBACK,RUNNING,MULTICAST> mtu 33160
>   console: #
>   console: # ifconfig sn0 10.0.2.3/24
>   console: #
>   console: # ping -c 3 10.0.2.2
>   console: PING 10.0.2.2 (10.0.2.2): 56 data bytes
>   console: 64 bytes from 10.0.2.2: icmp_seq=0 ttl=255 time=12.526 ms
>   console: 64 bytes from 10.0.2.2: icmp_seq=1 ttl=255 time=2.324 ms
>   console: 64 bytes from 10.0.2.2: icmp_seq=2 ttl=255 time=0.608 ms
>   console: ----10.0.2.2 PING Statistics----
>   console: 3 packets transmitted, 3 packets received, 0.0% packet loss
>   console: # shutdown -r now
>   console: round-trip min/avg/max/stddev = 0.608/5.153/12.526/6.443 ms
>   console: # Shutdown NOW!
>   console: shutdown: [pid 14]
>   console: # sh: /usr/bin/wall: not found
>   console: reboot by root:
>   console: System shutdown time has arrived
>   console: About to run shutdown hooks...
>   console: .: Can't open /etc/rc.shutdown
>   console: Done running shutdown hooks.
>   console: syncing disks... done
>   console: unmounting file systems... done
>   console: rebooting...
>   PASS (49.27 s)
>   RESULTS    : PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
>   JOB TIME   : 49.70 s
>
> Inspired-by: Hervé Poussineau <hpoussin@reactos.org>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> v2:
> - Test NetBSD 9.2 (Finn, Mark)
> - Drop '-global ds1225y.size=8200' (Mark)
> - Mention "Run a program" option (Mark)
> - Check ARP (Finn)
>
> Not for merge until nvram.bin is generated.
>
> Cc: Finn Thain <fthain@linux-m68k.org>
> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  .../avocado/machine_mips_jazz.d/nvram.bin.xz  | Bin 0 -> 700 bytes
>  tests/avocado/machine_mips_jazz.py            |  99 ++++++++++++++++++
>  2 files changed, 99 insertions(+)
>  create mode 100644 tests/avocado/machine_mips_jazz.d/nvram.bin.xz
>  create mode 100644 tests/avocado/machine_mips_jazz.py
>
> diff --git a/tests/avocado/machine_mips_jazz.d/nvram.bin.xz b/tests/avocado/machine_mips_jazz.d/nvram.bin.xz
> new file mode 100644
> index 0000000000000000000000000000000000000000..4648bb31a75bd1a6ee06818a1bf0f2109203ced3
> GIT binary patch
> literal 700
> zcmV;t0z>`%H+ooF000E$*0e?f03iV!0000G&sfah5B~ysT>t=TewJhU2JbgNgw@93
> z9ne@Of$dv11>H(&qfaP~{?QVNDD*)y9o<}llIpWo;zi4K%|O3oZX7;e0I%^w=#ho%
> zm*3>5KcVeDb)hM{s=^@xl3*6CLiEr>=o;i$-UTTwy=`4vF_aQ@AM-}hS_gfV?dXL~
> zr6}Ck7jr0Pd6X49Gycxm@5wylC8aWC%NAZ^DQmjk9v0R)a7u1hsY5)le9~S_G;wnI
> zNEhT-#4+m{FhwCCTK#>itkJ+zGLq(>iH&;ourQ75=5X0GCmKgT34hXv`!lNS@Vv~9
> z#dTZb1?<x%e;6UaEZ?$JT@1SBP5my9l7&XSs?W+Ufr||*z035xw~`at6Ee$`W+96+
> z+GJc@N)TzeY@b#YPn@xQT6#fKq_~pFo)=4ZKP9K_BO1_^7pVTQNp$u}nKgZ>DkCrj
> z^?l-ksd=`Q%7w&-3CkM)A-1nkM@y=-6N=HiA4b<WBKS^=)!oj-ZC_{*DGqZD2VzX1
> zK{*4jgOsj!UPEwQ?*VD-2^<k{1^m(NV;8Ya)p-pwGWqQ7CAgX4w5vWiB095j=yfi%
> zkXq_?C-K4b7)51+8S-t@;sSpTEtU7x1UZ|?WWEp59W6%y5<X+<RS_F7)N!aN8Lm!&
> z-jRJ50VM#t_NLVKQ`#MuEWzq1$^|tn4L1MW?!$H&fc<(y1QHv6-#@=cN*d%QYMaA0
> z+L!W3-k!*7dONwgWwK{?$5Yf5d8TRJIgw7WHR@zI6hLrbArpq|%Ax#6+d)m|&Wh%|
> z!pU{tbl$cCNXE=B0Pf|j?*P-9X*vQHCH?48MX**q?{Zss*9HdGiu@Se*+j_z00023
> i?D;I-mde`z0hR@TAOHX;UY-@P#Ao{g000001X)@jNK0n`
>
> literal 0
> HcmV?d00001
>
> diff --git a/tests/avocado/machine_mips_jazz.py b/tests/avocado/machine_mips_jazz.py
> new file mode 100644
> index 00000000000..1441b7fe5bb
> --- /dev/null
> +++ b/tests/avocado/machine_mips_jazz.py
> @@ -0,0 +1,99 @@
> +# Functional tests for the Jazz machines.
> +#
> +# Copyright (c) 2021 Philippe Mathieu-Daudé <f4bug@amsat.org>
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2 or later.
> +# See the COPYING file in the top-level directory.
> +#
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +import os
> +import lzma
> +import shutil
> +
> +from avocado import skipUnless
> +from avocado_qemu import QemuSystemTest
> +from avocado_qemu import exec_command
> +from avocado_qemu import exec_command_and_wait_for_pattern
> +from avocado_qemu import interrupt_interactive_console_until_pattern
> +from avocado_qemu import wait_for_console_pattern
> +
> +from tesseract_utils import tesseract_available, tesseract_ocr
> +
> +class MipsJazz(QemuSystemTest):
> +
> +    timeout = 60
> +
> +    @skipUnless(os.getenv('NTPROM_BIN_PATH'), 'NTPROM_BIN_PATH not available')
> +    def test_magnum_netbsd_9_2(self):
> +        """
> +        :avocado: tags=arch:mips64el
> +        :avocado: tags=machine:magnum
> +        :avocado: tags=os:netbsd
> +        :avocado: tags=device:sonic
> +        :avocado: tags=device:esp
> +        """
> +        drive_url = ('http://cdn.netbsd.org/pub/NetBSD/'
> +                     'NetBSD-9.2/images/NetBSD-9.2-arc.iso')
> +        drive_hash = '409c61aee5459e762cdb120d2591ed2e'
> +        drive_path = self.fetch_asset(drive_url, asset_hash=drive_hash,
> +                                      algorithm='md5')
> +        ntprom_hash = '316de17820192c89b8ee6d9936ab8364a739ca53'
> +        ntprom_path = self.fetch_asset('file://' + os.getenv('NTPROM_BIN_PATH'),
> +                                       asset_hash=ntprom_hash, algorithm='sha1')
> +        nvram_size = 8200
> +        nvram_path = 'nvram.bin'
> +        nvram_xz_hash = '3d4565124ff2369706b97e1d0ef127a68c23d418'
> +        nvram_xz_path = os.path.dirname(os.path.abspath(__file__)) \
> +                        + '/machine_mips_jazz.d/nvram.bin.xz'

If I understood correctly, you will not need this block when the
nvram.bin is built. Anyway, Avocado has libraries to handle files
needed by the tests. You can create a folder called
'test_name.py.data' and put your files there. Then, just use the
'get_data()' to get your file location without the need to handle the
paths.

The documentation about the data folder is here:
https://avocado-framework.readthedocs.io/en/latest/guides/writer/chapters/writing.html?highlight=data#accessing-test-data-files

> +        nvram_xz_path = self.fetch_asset('file://' + nvram_xz_path,
> +                                         asset_hash=nvram_xz_hash,
> +                                         algorithm='sha1')
> +        mac = '00:00:00:02:03:04'
> +
> +        with lzma.open(nvram_xz_path, 'rb') as f_in:
> +            with open(nvram_path, 'wb') as f_out:
> +                shutil.copyfileobj(f_in, f_out)
> +                f_out.seek(nvram_size)
> +                f_out.write(b'\0')
> +
> +        self.vm.set_console()
> +        self.vm.add_args('-bios', ntprom_path,
> +                         '-drive', 'if=scsi,unit=2,media=cdrom,format=raw,file='
> +                                   + drive_path,
> +                         '-global', 'ds1225y.filename=' + nvram_path,
> +                         '-nic', 'user,model=dp83932,mac=' + mac)
> +        self.vm.launch()
> +
> +        console_pattern = 'ARC Multiboot Version 174 (SGI Version 2.6)'
> +        wait_for_console_pattern(self, console_pattern)
> +
> +        wait_for_console_pattern(self, 'Use the arrow keys to select.')
> +
> +        # Press cursor control 'Down' to select the "Run a program" menu
> +        exec_command(self, '\x1b[B')
> +
> +        program = 'scsi(0)cdrom(2)fdisk(0)boot scsi(0)cdrom(2)fdisk(0)netbsd'
> +        exec_command(self, program)
> +        wait_for_console_pattern(self, 'NetBSD/arc Bootstrap, Revision 1.1')
> +
> +        # Terminal type? [vt100]
> +        console_pattern = 'erase ^H, werase ^W, kill ^U, intr ^C, status ^T'
> +        wait_for_console_pattern(self, console_pattern)
> +
> +        # (I)nstall, (S)hell or (H)alt
> +        exec_command_and_wait_for_pattern(self, '', 'Erase is backspace.')
> +        exec_command(self, 'S')
> +        interrupt_interactive_console_until_pattern(self, '#')
> +
> +        exec_command_and_wait_for_pattern(self, 'ifconfig', 'address: ' + mac)
> +        interrupt_interactive_console_until_pattern(self, '#')
> +
> +        exec_command(self, 'ifconfig sn0 10.0.2.3/24')
> +        interrupt_interactive_console_until_pattern(self, '#')
> +
> +        exec_command_and_wait_for_pattern(self, 'ping -c 3 10.0.2.2',
> +                '3 packets transmitted, 3 packets received, 0.0% packet loss')
> +
> +        exec_command_and_wait_for_pattern(self, 'shutdown -r now',
> +                                          'rebooting...')

Nice!

Except for the nvram.bin.xz,

Reviewed-by: Willian Rampazzo <willianr@redhat.com>



^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2021-11-22 20:07 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-19 17:11 [PATCH-for-7.0 0/5] hw/display: QOM'ify vga_mmio_init() as TYPE_VGA_MMIO Philippe Mathieu-Daudé
2021-11-19 17:11 ` [PATCH-for-7.0 1/5] hw/display: Rename VGA_ISA_MM -> VGA_MMIO Philippe Mathieu-Daudé
2021-11-19 18:15   ` BALATON Zoltan
2021-11-22 15:06   ` Thomas Huth
2021-11-19 17:11 ` [PATCH-for-7.0 2/5] hw/display/vga-mmio: Inline vga_mm_init() Philippe Mathieu-Daudé
2021-11-19 18:20   ` BALATON Zoltan
2021-11-19 18:23     ` BALATON Zoltan
2021-11-19 18:28     ` Philippe Mathieu-Daudé
2021-11-22 15:08   ` Thomas Huth
2021-11-19 17:12 ` [PATCH-for-7.0 3/5] hw/display/vga-mmio: QOM'ify vga_mmio_init() as TYPE_VGA_MMIO Philippe Mathieu-Daudé
2021-11-19 18:40   ` BALATON Zoltan
2021-11-19 17:12 ` [PATCH-for-7.0 4/5] hw/mips/jazz: Inline vga_mmio_init() and remove it Philippe Mathieu-Daudé
2021-11-19 18:43   ` BALATON Zoltan
2021-11-19 17:12 ` [NOTFORMERGE PATCH 5/5] tests/avocado: Test NetBSD 9.2 on the Jazz Magnum machine Philippe Mathieu-Daudé
2021-11-22 20:05   ` Willian Rampazzo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.