[PATCH v4 00/24] q800: add support for booting MacOS Classic - part 1

[PATCH v4 00/24] q800: add support for booting MacOS Classic - part 1

[Mark Cave-Ayland]

[MCA: the original series has now been split into 2 separate parts based upon
Phil's comments re: QOM parenting for objects in Q800MachineState. Part 1
consists of the Q800MachineState patches along with QOM parenting fixes and
the 2 mac_via RTC patches.]

This series contains the remaining patches needed to allow QEMU's q800
machine to boot MacOS Classic when used in conjunction with a real
Quadra 800 ROM image. In fact with this series applied it is possible
to boot all of the following OSs:

  - MacOS 7.1 - 8.1, with or without virtual memory enabled
  - A/UX 3.0.1
  - NetBSD 9.3
  - Linux (via EMILE)

If you are ready to experience some 90s nostalgia then all you need is
to grab yourself a copy of the Quadra 800 ROM (checksum 0xf1acad13) and a
suitable install ISO as follows:

  # Prepare a PRAM image
  $ qemu-img create -f raw pram.img 256b

  # Launch QEMU with blank disk and install CDROM
  $ ./qemu-system-m68k \
      -M q800 \
      -m 128 \
      -bios Quadra800.rom \
      -drive file=pram.img,format=raw,if=mtd \
      -drive file=disk.img,media=disk,format=raw,if=none,id=hd \
      -device scsi-hd,scsi-id=0,drive=hd \
      -drive file=cdrom.iso,media=cdrom,if=none,id=cd \
      -device scsi-cd,scsi-id=3,drive=cd

And off you go! For more in-depth information about the installation process
I highly recommend the installation guide over at emaculation.com [1].
Compatibility is generally very good, and I'm pleased to report it is possible
to run one of the most popular productivity apps from the 90s [2].

I'd like to add a big thank you to all the people who have helped me work on
this series, including testing on real hardware, answering questions about
MacOS Classic internals and helping to diagnose and fix bugs in the 68k
emulation. In particular thanks go to Laurent Vivier, Finn Thain, Howard
Spoelstra, Volker Rümelin, Richard Henderson, Martin Husemann, Rin Okuyama,
Elliot Nunn, and SolraBizna.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

[1] https://www.emaculation.com/doku.php/qemu
[2] https://www.youtube.com/watch?v=yI21gURQ1Ew


Patches missing review tags: 8 (new)

v4:
- Rebase onto master
- Add R-B tags from Phil and Laurent
- Use qdev_realize() in patch 5 as suggested by Markus
- Add new patch 8 to switch q800-glue.c to use the DEFINE_TYPES macro
  as suggested by Phil

v3:
- Add R-B tags from Phil and Laurent
- Add missing headers in patches indicated by Phil
- Change patch 5 to use valid_cpu_types Machine class property and the cpu_type
  Machine property to initialise the CPU
- Remove osdep.h header from dp8393x.h in patch 13 noticed by Phil
- Change sysbus_realize_and_unref() to sysbus_realize() in patch 19
- Use memory_region_add_subregion() instead of sysbus_mmio_map() in patch 19

v2:
- Split series into 2 parts (this is part 1)
- Update QOM parenting for objects in Q800MachineState (Phil)
- Split GLUE device into separate glue.c and glue.h files
- Split TYPE_DP8393X and dp8393xState into dp8393x.h
- Add R-B tags from Laurent (where I still believe they are valid)


Mark Cave-Ayland (24):
  q800: fix up minor spacing issues in hw_compat_q800 GlobalProperty
    array
  q800: add missing space after parent object in GLUEState
  q800: introduce Q800MachineState
  q800: rename q800_init() to q800_machine_init()
  q800: move CPU object into Q800MachineState
  q800: move ROM memory region to Q800MachineState
  q800: move GLUE device into separate q800-glue.c file
  q800-glue.c: switch TypeInfo registration to use DEFINE_TYPES() macro
  q800: move GLUE device to Q800MachineState
  q800: introduce mac-io container memory region
  q800: reimplement mac-io region aliasing using IO memory region
  q800: move VIA1 device to Q800MachineState
  q800: move VIA2 device to Q800MachineState
  hw/net/dp8393x.c: move TYPE_DP8393X and dp8393xState into dp8393x.h
  q800: move dp8393x device to Q800MachineState
  q800: move ESCC device to Q800MachineState
  q800: move escc_orgate device to Q800MachineState
  q800: move ESP device to Q800MachineState
  q800: move SWIM device to Q800MachineState
  q800: move mac-nubus-bridge device to Q800MachineState
  q800: don't access Nubus bus directly from the mac-nubus-bridge device
  q800: move macfb device to Q800MachineState
  mac_via: fix rtc command decoding from PRAM addresses 0x0 to 0xf
  mac_via: fix rtc command decoding for the PRAM seconds registers

 MAINTAINERS                 |   3 +
 hw/m68k/meson.build         |   2 +-
 hw/m68k/q800-glue.c         | 249 +++++++++++++++++
 hw/m68k/q800.c              | 526 ++++++++++++++----------------------
 hw/misc/mac_via.c           |  13 +-
 hw/net/dp8393x.c            |  32 +--
 include/hw/m68k/q800-glue.h |  50 ++++
 include/hw/m68k/q800.h      |  66 +++++
 include/hw/net/dp8393x.h    |  60 ++++
 9 files changed, 635 insertions(+), 366 deletions(-)
 create mode 100644 hw/m68k/q800-glue.c
 create mode 100644 include/hw/m68k/q800-glue.h
 create mode 100644 include/hw/m68k/q800.h
 create mode 100644 include/hw/net/dp8393x.h

-- 
2.30.2

[PATCH v4 01/24] q800: fix up minor spacing issues in hw_compat_q800 GlobalProperty array

[Mark Cave-Ayland]

Ensure there is a space before the final closing brace for all global
properties.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/m68k/q800.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 68f0cd8cac..dda57c60bf 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -719,14 +719,14 @@ static void q800_init(MachineState *machine)
 }
 
 static GlobalProperty hw_compat_q800[] = {
-    { "scsi-hd", "quirk_mode_page_vendor_specific_apple", "on"},
+    { "scsi-hd", "quirk_mode_page_vendor_specific_apple", "on" },
     { "scsi-hd", "vendor", " SEAGATE" },
     { "scsi-hd", "product", "          ST225N" },
     { "scsi-hd", "ver", "1.0 " },
-    { "scsi-cd", "quirk_mode_page_apple_vendor", "on"},
-    { "scsi-cd", "quirk_mode_sense_rom_use_dbd", "on"},
-    { "scsi-cd", "quirk_mode_page_vendor_specific_apple", "on"},
-    { "scsi-cd", "quirk_mode_page_truncated", "on"},
+    { "scsi-cd", "quirk_mode_page_apple_vendor", "on" },
+    { "scsi-cd", "quirk_mode_sense_rom_use_dbd", "on" },
+    { "scsi-cd", "quirk_mode_page_vendor_specific_apple", "on" },
+    { "scsi-cd", "quirk_mode_page_truncated", "on" },
     { "scsi-cd", "vendor", "MATSHITA" },
     { "scsi-cd", "product", "CD-ROM CR-8005" },
     { "scsi-cd", "ver", "1.0k" },
-- 
2.30.2

[PATCH v4 02/24] q800: add missing space after parent object in GLUEState

[Mark Cave-Ayland]

This brings GLUEState in line with our current QOM guidelines.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/m68k/q800.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index dda57c60bf..465c510c18 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -100,6 +100,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(GLUEState, GLUE)
 
 struct GLUEState {
     SysBusDevice parent_obj;
+
     M68kCPU *cpu;
     uint8_t ipr;
     uint8_t auxmode;
-- 
2.30.2

[PATCH v4 03/24] q800: introduce Q800MachineState

[Mark Cave-Ayland]

This provides an overall container and owner for Machine-related objects such
as MemoryRegions.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 MAINTAINERS            |  1 +
 hw/m68k/q800.c         |  2 ++
 include/hw/m68k/q800.h | 40 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 43 insertions(+)
 create mode 100644 include/hw/m68k/q800.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 88b5a7ee0a..748a66fbaa 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1236,6 +1236,7 @@ F: include/hw/misc/mac_via.h
 F: include/hw/nubus/*
 F: include/hw/display/macfb.h
 F: include/hw/block/swim.h
+F: include/hw/m68k/q800.h
 
 virt
 M: Laurent Vivier <laurent@vivier.eu>
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 465c510c18..c0256c8a90 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -38,6 +38,7 @@
 #include "standard-headers/asm-m68k/bootinfo.h"
 #include "standard-headers/asm-m68k/bootinfo-mac.h"
 #include "bootinfo.h"
+#include "hw/m68k/q800.h"
 #include "hw/misc/mac_via.h"
 #include "hw/input/adb.h"
 #include "hw/nubus/mac-nubus-bridge.h"
@@ -749,6 +750,7 @@ static void q800_machine_class_init(ObjectClass *oc, void *data)
 static const TypeInfo q800_machine_typeinfo = {
     .name       = MACHINE_TYPE_NAME("q800"),
     .parent     = TYPE_MACHINE,
+    .instance_size = sizeof(Q800MachineState),
     .class_init = q800_machine_class_init,
 };
 
diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
new file mode 100644
index 0000000000..f3bc17aa1b
--- /dev/null
+++ b/include/hw/m68k/q800.h
@@ -0,0 +1,40 @@
+/*
+ * QEMU Motorla 680x0 Macintosh hardware System Emulator
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef HW_Q800_H
+#define HW_Q800_H
+
+#include "hw/boards.h"
+#include "qom/object.h"
+
+/*
+ * The main Q800 machine
+ */
+
+struct Q800MachineState {
+    MachineState parent_obj;
+};
+
+#define TYPE_Q800_MACHINE MACHINE_TYPE_NAME("q800")
+OBJECT_DECLARE_SIMPLE_TYPE(Q800MachineState, Q800_MACHINE)
+
+#endif
-- 
2.30.2

[PATCH v4 04/24] q800: rename q800_init() to q800_machine_init()

[Mark Cave-Ayland]

This will enable us later to distinguish between QOM initialisation and machine
initialisation.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/m68k/q800.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index c0256c8a90..062a3c6c76 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -362,7 +362,7 @@ static uint8_t fake_mac_rom[] = {
     0x60, 0xFE                          /* bras [self] */
 };
 
-static void q800_init(MachineState *machine)
+static void q800_machine_init(MachineState *machine)
 {
     M68kCPU *cpu = NULL;
     int linux_boot;
@@ -738,8 +738,9 @@ static const size_t hw_compat_q800_len = G_N_ELEMENTS(hw_compat_q800);
 static void q800_machine_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
+
     mc->desc = "Macintosh Quadra 800";
-    mc->init = q800_init;
+    mc->init = q800_machine_init;
     mc->default_cpu_type = M68K_CPU_TYPE_NAME("m68040");
     mc->max_cpus = 1;
     mc->block_default_type = IF_SCSI;
-- 
2.30.2

[PATCH v4 05/24] q800: move CPU object into Q800MachineState

[Mark Cave-Ayland]

Also change the instantiation of the CPU to use object_initialize_child()
followed by a separate realisation.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/m68k/q800.c         | 18 +++++++++++++-----
 include/hw/m68k/q800.h |  3 +++
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 062a3c6c76..2b651de3c1 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -364,7 +364,7 @@ static uint8_t fake_mac_rom[] = {
 
 static void q800_machine_init(MachineState *machine)
 {
-    M68kCPU *cpu = NULL;
+    Q800MachineState *m = Q800_MACHINE(machine);
     int linux_boot;
     int32_t kernel_size;
     uint64_t elf_entry;
@@ -407,8 +407,9 @@ static void q800_machine_init(MachineState *machine)
     }
 
     /* init CPUs */
-    cpu = M68K_CPU(cpu_create(machine->cpu_type));
-    qemu_register_reset(main_cpu_reset, cpu);
+    object_initialize_child(OBJECT(machine), "cpu", &m->cpu, machine->cpu_type);
+    qdev_realize(DEVICE(&m->cpu), NULL, &error_fatal);
+    qemu_register_reset(main_cpu_reset, &m->cpu);
 
     /* RAM */
     memory_region_add_subregion(get_system_memory(), 0, machine->ram);
@@ -430,7 +431,8 @@ static void q800_machine_init(MachineState *machine)
 
     /* IRQ Glue */
     glue = qdev_new(TYPE_GLUE);
-    object_property_set_link(OBJECT(glue), "cpu", OBJECT(cpu), &error_abort);
+    object_property_set_link(OBJECT(glue), "cpu", OBJECT(&m->cpu),
+                             &error_abort);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(glue), &error_fatal);
 
     /* VIA 1 */
@@ -605,7 +607,7 @@ static void q800_machine_init(MachineState *machine)
 
     macfb_mode = (NUBUS_MACFB(dev)->macfb).mode;
 
-    cs = CPU(cpu);
+    cs = CPU(&m->cpu);
     if (linux_boot) {
         uint64_t high;
         void *param_blob, *param_ptr, *param_rng_seed;
@@ -735,6 +737,11 @@ static GlobalProperty hw_compat_q800[] = {
 };
 static const size_t hw_compat_q800_len = G_N_ELEMENTS(hw_compat_q800);
 
+static const char *q800_machine_valid_cpu_types[] = {
+    M68K_CPU_TYPE_NAME("m68040"),
+    NULL
+};
+
 static void q800_machine_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
@@ -742,6 +749,7 @@ static void q800_machine_class_init(ObjectClass *oc, void *data)
     mc->desc = "Macintosh Quadra 800";
     mc->init = q800_machine_init;
     mc->default_cpu_type = M68K_CPU_TYPE_NAME("m68040");
+    mc->valid_cpu_types = q800_machine_valid_cpu_types;
     mc->max_cpus = 1;
     mc->block_default_type = IF_SCSI;
     mc->default_ram_id = "m68k_mac.ram";
diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
index f3bc17aa1b..4cb1a51dfe 100644
--- a/include/hw/m68k/q800.h
+++ b/include/hw/m68k/q800.h
@@ -25,6 +25,7 @@
 
 #include "hw/boards.h"
 #include "qom/object.h"
+#include "target/m68k/cpu-qom.h"
 
 /*
  * The main Q800 machine
@@ -32,6 +33,8 @@
 
 struct Q800MachineState {
     MachineState parent_obj;
+
+    M68kCPU cpu;
 };
 
 #define TYPE_Q800_MACHINE MACHINE_TYPE_NAME("q800")
-- 
2.30.2

[PATCH v4 06/24] q800: move ROM memory region to Q800MachineState

[Mark Cave-Ayland]

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/m68k/q800.c         | 13 +++++--------
 include/hw/m68k/q800.h |  2 ++
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 2b651de3c1..9f9668c2b4 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -372,7 +372,6 @@ static void q800_machine_init(MachineState *machine)
     int bios_size;
     ram_addr_t initrd_base;
     int32_t initrd_size;
-    MemoryRegion *rom;
     MemoryRegion *io;
     MemoryRegion *dp8393x_prom = g_new(MemoryRegion, 1);
     uint8_t *prom;
@@ -646,11 +645,10 @@ static void q800_machine_init(MachineState *machine)
         BOOTINFO1(param_ptr, BI_MAC_VROW, macfb_mode->stride);
         BOOTINFO1(param_ptr, BI_MAC_SCCBASE, SCC_BASE);
 
-        rom = g_malloc(sizeof(*rom));
-        memory_region_init_ram_ptr(rom, NULL, "m68k_fake_mac.rom",
+        memory_region_init_ram_ptr(&m->rom, NULL, "m68k_fake_mac.rom",
                                    sizeof(fake_mac_rom), fake_mac_rom);
-        memory_region_set_readonly(rom, true);
-        memory_region_add_subregion(get_system_memory(), MACROM_ADDR, rom);
+        memory_region_set_readonly(&m->rom, true);
+        memory_region_add_subregion(get_system_memory(), MACROM_ADDR, &m->rom);
 
         if (kernel_cmdline) {
             BOOTINFOSTR(param_ptr, BI_COMMAND_LINE,
@@ -692,11 +690,10 @@ static void q800_machine_init(MachineState *machine)
     } else {
         uint8_t *ptr;
         /* allocate and load BIOS */
-        rom = g_malloc(sizeof(*rom));
-        memory_region_init_rom(rom, NULL, "m68k_mac.rom", MACROM_SIZE,
+        memory_region_init_rom(&m->rom, NULL, "m68k_mac.rom", MACROM_SIZE,
                                &error_abort);
         filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
-        memory_region_add_subregion(get_system_memory(), MACROM_ADDR, rom);
+        memory_region_add_subregion(get_system_memory(), MACROM_ADDR, &m->rom);
 
         /* Load MacROM binary */
         if (filename) {
diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
index 4cb1a51dfe..d1f1ae4b88 100644
--- a/include/hw/m68k/q800.h
+++ b/include/hw/m68k/q800.h
@@ -26,6 +26,7 @@
 #include "hw/boards.h"
 #include "qom/object.h"
 #include "target/m68k/cpu-qom.h"
+#include "exec/memory.h"
 
 /*
  * The main Q800 machine
@@ -35,6 +36,7 @@ struct Q800MachineState {
     MachineState parent_obj;
 
     M68kCPU cpu;
+    MemoryRegion rom;
 };
 
 #define TYPE_Q800_MACHINE MACHINE_TYPE_NAME("q800")
-- 
2.30.2

[PATCH v4 07/24] q800: move GLUE device into separate q800-glue.c file

[Mark Cave-Ayland]

This will allow the q800-glue.h header to be included separately so that the
GLUE device can be referenced externally.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 MAINTAINERS                 |   2 +
 hw/m68k/meson.build         |   2 +-
 hw/m68k/q800-glue.c         | 252 ++++++++++++++++++++++++++++++++++++
 hw/m68k/q800.c              | 238 +---------------------------------
 include/hw/m68k/q800-glue.h |  50 +++++++
 5 files changed, 306 insertions(+), 238 deletions(-)
 create mode 100644 hw/m68k/q800-glue.c
 create mode 100644 include/hw/m68k/q800-glue.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 748a66fbaa..7f323cd2eb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1225,6 +1225,7 @@ q800
 M: Laurent Vivier <laurent@vivier.eu>
 S: Maintained
 F: hw/m68k/q800.c
+F: hw/m68k/q800-glue.c
 F: hw/misc/mac_via.c
 F: hw/nubus/*
 F: hw/display/macfb.c
@@ -1237,6 +1238,7 @@ F: include/hw/nubus/*
 F: include/hw/display/macfb.h
 F: include/hw/block/swim.h
 F: include/hw/m68k/q800.h
+F: include/hw/m68k/q800-glue.h
 
 virt
 M: Laurent Vivier <laurent@vivier.eu>
diff --git a/hw/m68k/meson.build b/hw/m68k/meson.build
index 31248641d3..84bc68fa4e 100644
--- a/hw/m68k/meson.build
+++ b/hw/m68k/meson.build
@@ -2,7 +2,7 @@ m68k_ss = ss.source_set()
 m68k_ss.add(when: 'CONFIG_AN5206', if_true: files('an5206.c', 'mcf5206.c'))
 m68k_ss.add(when: 'CONFIG_MCF5208', if_true: files('mcf5208.c', 'mcf_intc.c'))
 m68k_ss.add(when: 'CONFIG_NEXTCUBE', if_true: files('next-kbd.c', 'next-cube.c'))
-m68k_ss.add(when: 'CONFIG_Q800', if_true: files('q800.c'))
+m68k_ss.add(when: 'CONFIG_Q800', if_true: files('q800.c', 'q800-glue.c'))
 m68k_ss.add(when: 'CONFIG_M68K_VIRT', if_true: files('virt.c'))
 
 hw_arch += {'m68k': m68k_ss}
diff --git a/hw/m68k/q800-glue.c b/hw/m68k/q800-glue.c
new file mode 100644
index 0000000000..e81f9438f1
--- /dev/null
+++ b/hw/m68k/q800-glue.c
@@ -0,0 +1,252 @@
+/*
+ * QEMU q800 logic GLUE (General Logic Unit)
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "hw/m68k/q800-glue.h"
+#include "hw/boards.h"
+#include "hw/irq.h"
+#include "hw/nmi.h"
+#include "hw/qdev-properties.h"
+#include "migration/vmstate.h"
+
+/*
+ * The GLUE (General Logic Unit) is an Apple custom integrated circuit chip
+ * that performs a variety of functions (RAM management, clock generation, ...).
+ * The GLUE chip receives interrupt requests from various devices,
+ * assign priority to each, and asserts one or more interrupt line to the
+ * CPU.
+ */
+
+/*
+ * The GLUE logic on the Quadra 800 supports 2 different IRQ routing modes
+ * controlled from the VIA1 auxmode GPIO (port B bit 6) which are documented
+ * in NetBSD as follows:
+ *
+ * A/UX mode (Linux, NetBSD, auxmode GPIO low)
+ *
+ *   Level 0:        Spurious: ignored
+ *   Level 1:        Software
+ *   Level 2:        VIA2 (except ethernet, sound)
+ *   Level 3:        Ethernet
+ *   Level 4:        Serial (SCC)
+ *   Level 5:        Sound
+ *   Level 6:        VIA1
+ *   Level 7:        NMIs: parity errors, RESET button, YANCC error
+ *
+ * Classic mode (default: used by MacOS, A/UX 3.0.1, auxmode GPIO high)
+ *
+ *   Level 0:        Spurious: ignored
+ *   Level 1:        VIA1 (clock, ADB)
+ *   Level 2:        VIA2 (NuBus, SCSI)
+ *   Level 3:
+ *   Level 4:        Serial (SCC)
+ *   Level 5:
+ *   Level 6:
+ *   Level 7:        Non-maskable: parity errors, RESET button
+ *
+ * Note that despite references to A/UX mode in Linux and NetBSD, at least
+ * A/UX 3.0.1 still uses Classic mode.
+ */
+
+static void GLUE_set_irq(void *opaque, int irq, int level)
+{
+    GLUEState *s = opaque;
+    int i;
+
+    if (s->auxmode) {
+        /* Classic mode */
+        switch (irq) {
+        case GLUE_IRQ_IN_VIA1:
+            irq = 0;
+            break;
+
+        case GLUE_IRQ_IN_VIA2:
+            irq = 1;
+            break;
+
+        case GLUE_IRQ_IN_SONIC:
+            /* Route to VIA2 instead */
+            qemu_set_irq(s->irqs[GLUE_IRQ_NUBUS_9], level);
+            return;
+
+        case GLUE_IRQ_IN_ESCC:
+            irq = 3;
+            break;
+
+        case GLUE_IRQ_IN_NMI:
+            irq = 6;
+            break;
+
+        default:
+            g_assert_not_reached();
+        }
+    } else {
+        /* A/UX mode */
+        switch (irq) {
+        case GLUE_IRQ_IN_VIA1:
+            irq = 5;
+            break;
+
+        case GLUE_IRQ_IN_VIA2:
+            irq = 1;
+            break;
+
+        case GLUE_IRQ_IN_SONIC:
+            irq = 2;
+            break;
+
+        case GLUE_IRQ_IN_ESCC:
+            irq = 3;
+            break;
+
+        case GLUE_IRQ_IN_NMI:
+            irq = 6;
+            break;
+
+        default:
+            g_assert_not_reached();
+        }
+    }
+
+    if (level) {
+        s->ipr |= 1 << irq;
+    } else {
+        s->ipr &= ~(1 << irq);
+    }
+
+    for (i = 7; i >= 0; i--) {
+        if ((s->ipr >> i) & 1) {
+            m68k_set_irq_level(s->cpu, i + 1, i + 25);
+            return;
+        }
+    }
+    m68k_set_irq_level(s->cpu, 0, 0);
+}
+
+static void glue_auxmode_set_irq(void *opaque, int irq, int level)
+{
+    GLUEState *s = GLUE(opaque);
+
+    s->auxmode = level;
+}
+
+static void glue_nmi(NMIState *n, int cpu_index, Error **errp)
+{
+    GLUEState *s = GLUE(n);
+
+    /* Hold NMI active for 100ms */
+    GLUE_set_irq(s, GLUE_IRQ_IN_NMI, 1);
+    timer_mod(s->nmi_release, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 100);
+}
+
+static void glue_nmi_release(void *opaque)
+{
+    GLUEState *s = GLUE(opaque);
+
+    GLUE_set_irq(s, GLUE_IRQ_IN_NMI, 0);
+}
+
+static void glue_reset(DeviceState *dev)
+{
+    GLUEState *s = GLUE(dev);
+
+    s->ipr = 0;
+    s->auxmode = 0;
+
+    timer_del(s->nmi_release);
+}
+
+static const VMStateDescription vmstate_glue = {
+    .name = "q800-glue",
+    .version_id = 0,
+    .minimum_version_id = 0,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT8(ipr, GLUEState),
+        VMSTATE_UINT8(auxmode, GLUEState),
+        VMSTATE_TIMER_PTR(nmi_release, GLUEState),
+        VMSTATE_END_OF_LIST(),
+    },
+};
+
+/*
+ * If the m68k CPU implemented its inbound irq lines as GPIO lines
+ * rather than via the m68k_set_irq_level() function we would not need
+ * this cpu link property and could instead provide outbound IRQ lines
+ * that the board could wire up to the CPU.
+ */
+static Property glue_properties[] = {
+    DEFINE_PROP_LINK("cpu", GLUEState, cpu, TYPE_M68K_CPU, M68kCPU *),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void glue_finalize(Object *obj)
+{
+    GLUEState *s = GLUE(obj);
+
+    timer_free(s->nmi_release);
+}
+
+static void glue_init(Object *obj)
+{
+    DeviceState *dev = DEVICE(obj);
+    GLUEState *s = GLUE(dev);
+
+    qdev_init_gpio_in(dev, GLUE_set_irq, 8);
+    qdev_init_gpio_in_named(dev, glue_auxmode_set_irq, "auxmode", 1);
+
+    qdev_init_gpio_out(dev, s->irqs, 1);
+
+    /* NMI release timer */
+    s->nmi_release = timer_new_ms(QEMU_CLOCK_VIRTUAL, glue_nmi_release, s);
+}
+
+static void glue_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    NMIClass *nc = NMI_CLASS(klass);
+
+    dc->vmsd = &vmstate_glue;
+    dc->reset = glue_reset;
+    device_class_set_props(dc, glue_properties);
+    nc->nmi_monitor_handler = glue_nmi;
+}
+
+static const TypeInfo glue_info = {
+    .name = TYPE_GLUE,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(GLUEState),
+    .instance_init = glue_init,
+    .instance_finalize = glue_finalize,
+    .class_init = glue_class_init,
+    .interfaces = (InterfaceInfo[]) {
+         { TYPE_NMI },
+         { }
+    },
+};
+
+static void glue_register_types(void)
+{
+    type_register_static(&glue_info);
+}
+
+type_init(glue_register_types)
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 9f9668c2b4..9f9de2ebaf 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -28,7 +28,6 @@
 #include "cpu.h"
 #include "hw/boards.h"
 #include "hw/or-irq.h"
-#include "hw/nmi.h"
 #include "elf.h"
 #include "hw/loader.h"
 #include "ui/console.h"
@@ -39,6 +38,7 @@
 #include "standard-headers/asm-m68k/bootinfo-mac.h"
 #include "bootinfo.h"
 #include "hw/m68k/q800.h"
+#include "hw/m68k/q800-glue.h"
 #include "hw/misc/mac_via.h"
 #include "hw/input/adb.h"
 #include "hw/nubus/mac-nubus-bridge.h"
@@ -88,241 +88,6 @@
 #define Q800_NUBUS_SLOTS_AVAILABLE    (BIT(0x9) | BIT(0xc) | BIT(0xd) | \
                                        BIT(0xe))
 
-/*
- * The GLUE (General Logic Unit) is an Apple custom integrated circuit chip
- * that performs a variety of functions (RAM management, clock generation, ...).
- * The GLUE chip receives interrupt requests from various devices,
- * assign priority to each, and asserts one or more interrupt line to the
- * CPU.
- */
-
-#define TYPE_GLUE "q800-glue"
-OBJECT_DECLARE_SIMPLE_TYPE(GLUEState, GLUE)
-
-struct GLUEState {
-    SysBusDevice parent_obj;
-
-    M68kCPU *cpu;
-    uint8_t ipr;
-    uint8_t auxmode;
-    qemu_irq irqs[1];
-    QEMUTimer *nmi_release;
-};
-
-#define GLUE_IRQ_IN_VIA1       0
-#define GLUE_IRQ_IN_VIA2       1
-#define GLUE_IRQ_IN_SONIC      2
-#define GLUE_IRQ_IN_ESCC       3
-#define GLUE_IRQ_IN_NMI        4
-
-#define GLUE_IRQ_NUBUS_9       0
-
-/*
- * The GLUE logic on the Quadra 800 supports 2 different IRQ routing modes
- * controlled from the VIA1 auxmode GPIO (port B bit 6) which are documented
- * in NetBSD as follows:
- *
- * A/UX mode (Linux, NetBSD, auxmode GPIO low)
- *
- *   Level 0:        Spurious: ignored
- *   Level 1:        Software
- *   Level 2:        VIA2 (except ethernet, sound)
- *   Level 3:        Ethernet
- *   Level 4:        Serial (SCC)
- *   Level 5:        Sound
- *   Level 6:        VIA1
- *   Level 7:        NMIs: parity errors, RESET button, YANCC error
- *
- * Classic mode (default: used by MacOS, A/UX 3.0.1, auxmode GPIO high)
- *
- *   Level 0:        Spurious: ignored
- *   Level 1:        VIA1 (clock, ADB)
- *   Level 2:        VIA2 (NuBus, SCSI)
- *   Level 3:
- *   Level 4:        Serial (SCC)
- *   Level 5:
- *   Level 6:
- *   Level 7:        Non-maskable: parity errors, RESET button
- *
- * Note that despite references to A/UX mode in Linux and NetBSD, at least
- * A/UX 3.0.1 still uses Classic mode.
- */
-
-static void GLUE_set_irq(void *opaque, int irq, int level)
-{
-    GLUEState *s = opaque;
-    int i;
-
-    if (s->auxmode) {
-        /* Classic mode */
-        switch (irq) {
-        case GLUE_IRQ_IN_VIA1:
-            irq = 0;
-            break;
-
-        case GLUE_IRQ_IN_VIA2:
-            irq = 1;
-            break;
-
-        case GLUE_IRQ_IN_SONIC:
-            /* Route to VIA2 instead */
-            qemu_set_irq(s->irqs[GLUE_IRQ_NUBUS_9], level);
-            return;
-
-        case GLUE_IRQ_IN_ESCC:
-            irq = 3;
-            break;
-
-        case GLUE_IRQ_IN_NMI:
-            irq = 6;
-            break;
-
-        default:
-            g_assert_not_reached();
-        }
-    } else {
-        /* A/UX mode */
-        switch (irq) {
-        case GLUE_IRQ_IN_VIA1:
-            irq = 5;
-            break;
-
-        case GLUE_IRQ_IN_VIA2:
-            irq = 1;
-            break;
-
-        case GLUE_IRQ_IN_SONIC:
-            irq = 2;
-            break;
-
-        case GLUE_IRQ_IN_ESCC:
-            irq = 3;
-            break;
-
-        case GLUE_IRQ_IN_NMI:
-            irq = 6;
-            break;
-
-        default:
-            g_assert_not_reached();
-        }
-    }
-
-    if (level) {
-        s->ipr |= 1 << irq;
-    } else {
-        s->ipr &= ~(1 << irq);
-    }
-
-    for (i = 7; i >= 0; i--) {
-        if ((s->ipr >> i) & 1) {
-            m68k_set_irq_level(s->cpu, i + 1, i + 25);
-            return;
-        }
-    }
-    m68k_set_irq_level(s->cpu, 0, 0);
-}
-
-static void glue_auxmode_set_irq(void *opaque, int irq, int level)
-{
-    GLUEState *s = GLUE(opaque);
-
-    s->auxmode = level;
-}
-
-static void glue_nmi(NMIState *n, int cpu_index, Error **errp)
-{
-    GLUEState *s = GLUE(n);
-
-    /* Hold NMI active for 100ms */
-    GLUE_set_irq(s, GLUE_IRQ_IN_NMI, 1);
-    timer_mod(s->nmi_release, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 100);
-}
-
-static void glue_nmi_release(void *opaque)
-{
-    GLUEState *s = GLUE(opaque);
-
-    GLUE_set_irq(s, GLUE_IRQ_IN_NMI, 0);
-}
-
-static void glue_reset(DeviceState *dev)
-{
-    GLUEState *s = GLUE(dev);
-
-    s->ipr = 0;
-    s->auxmode = 0;
-
-    timer_del(s->nmi_release);
-}
-
-static const VMStateDescription vmstate_glue = {
-    .name = "q800-glue",
-    .version_id = 0,
-    .minimum_version_id = 0,
-    .fields = (VMStateField[]) {
-        VMSTATE_UINT8(ipr, GLUEState),
-        VMSTATE_UINT8(auxmode, GLUEState),
-        VMSTATE_TIMER_PTR(nmi_release, GLUEState),
-        VMSTATE_END_OF_LIST(),
-    },
-};
-
-/*
- * If the m68k CPU implemented its inbound irq lines as GPIO lines
- * rather than via the m68k_set_irq_level() function we would not need
- * this cpu link property and could instead provide outbound IRQ lines
- * that the board could wire up to the CPU.
- */
-static Property glue_properties[] = {
-    DEFINE_PROP_LINK("cpu", GLUEState, cpu, TYPE_M68K_CPU, M68kCPU *),
-    DEFINE_PROP_END_OF_LIST(),
-};
-
-static void glue_finalize(Object *obj)
-{
-    GLUEState *s = GLUE(obj);
-
-    timer_free(s->nmi_release);
-}
-
-static void glue_init(Object *obj)
-{
-    DeviceState *dev = DEVICE(obj);
-    GLUEState *s = GLUE(dev);
-
-    qdev_init_gpio_in(dev, GLUE_set_irq, 8);
-    qdev_init_gpio_in_named(dev, glue_auxmode_set_irq, "auxmode", 1);
-
-    qdev_init_gpio_out(dev, s->irqs, 1);
-
-    /* NMI release timer */
-    s->nmi_release = timer_new_ms(QEMU_CLOCK_VIRTUAL, glue_nmi_release, s);
-}
-
-static void glue_class_init(ObjectClass *klass, void *data)
-{
-    DeviceClass *dc = DEVICE_CLASS(klass);
-    NMIClass *nc = NMI_CLASS(klass);
-
-    dc->vmsd = &vmstate_glue;
-    dc->reset = glue_reset;
-    device_class_set_props(dc, glue_properties);
-    nc->nmi_monitor_handler = glue_nmi;
-}
-
-static const TypeInfo glue_info = {
-    .name = TYPE_GLUE,
-    .parent = TYPE_SYS_BUS_DEVICE,
-    .instance_size = sizeof(GLUEState),
-    .instance_init = glue_init,
-    .instance_finalize = glue_finalize,
-    .class_init = glue_class_init,
-    .interfaces = (InterfaceInfo[]) {
-         { TYPE_NMI },
-         { }
-    },
-};
 
 static void main_cpu_reset(void *opaque)
 {
@@ -763,7 +528,6 @@ static const TypeInfo q800_machine_typeinfo = {
 static void q800_machine_register_types(void)
 {
     type_register_static(&q800_machine_typeinfo);
-    type_register_static(&glue_info);
 }
 
 type_init(q800_machine_register_types)
diff --git a/include/hw/m68k/q800-glue.h b/include/hw/m68k/q800-glue.h
new file mode 100644
index 0000000000..c1817b01a5
--- /dev/null
+++ b/include/hw/m68k/q800-glue.h
@@ -0,0 +1,50 @@
+/*
+ * QEMU q800 logic glue
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef HW_Q800_GLUE_H
+#define HW_Q800_GLUE_H
+
+#include "qemu/osdep.h"
+#include "hw/sysbus.h"
+
+#define TYPE_GLUE "q800-glue"
+OBJECT_DECLARE_SIMPLE_TYPE(GLUEState, GLUE)
+
+struct GLUEState {
+    SysBusDevice parent_obj;
+
+    M68kCPU *cpu;
+    uint8_t ipr;
+    uint8_t auxmode;
+    qemu_irq irqs[1];
+    QEMUTimer *nmi_release;
+};
+
+#define GLUE_IRQ_IN_VIA1       0
+#define GLUE_IRQ_IN_VIA2       1
+#define GLUE_IRQ_IN_SONIC      2
+#define GLUE_IRQ_IN_ESCC       3
+#define GLUE_IRQ_IN_NMI        4
+
+#define GLUE_IRQ_NUBUS_9       0
+
+#endif
-- 
2.30.2

[PATCH v4 08/24] q800-glue.c: switch TypeInfo registration to use DEFINE_TYPES() macro

[Mark Cave-Ayland]

The use of the DEFINE_TYPES() macro will soon be recommended over the use of
calling type_init() directly.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/m68k/q800-glue.c | 29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/hw/m68k/q800-glue.c b/hw/m68k/q800-glue.c
index e81f9438f1..34c4f0e987 100644
--- a/hw/m68k/q800-glue.c
+++ b/hw/m68k/q800-glue.c
@@ -231,22 +231,19 @@ static void glue_class_init(ObjectClass *klass, void *data)
     nc->nmi_monitor_handler = glue_nmi;
 }
 
-static const TypeInfo glue_info = {
-    .name = TYPE_GLUE,
-    .parent = TYPE_SYS_BUS_DEVICE,
-    .instance_size = sizeof(GLUEState),
-    .instance_init = glue_init,
-    .instance_finalize = glue_finalize,
-    .class_init = glue_class_init,
-    .interfaces = (InterfaceInfo[]) {
-         { TYPE_NMI },
-         { }
+static const TypeInfo glue_info_types[] = {
+    {
+        .name = TYPE_GLUE,
+        .parent = TYPE_SYS_BUS_DEVICE,
+        .instance_size = sizeof(GLUEState),
+        .instance_init = glue_init,
+        .instance_finalize = glue_finalize,
+        .class_init = glue_class_init,
+        .interfaces = (InterfaceInfo[]) {
+             { TYPE_NMI },
+             { }
+        },
     },
 };
 
-static void glue_register_types(void)
-{
-    type_register_static(&glue_info);
-}
-
-type_init(glue_register_types)
+DEFINE_TYPES(glue_info_types)
-- 
2.30.2

[PATCH v4 09/24] q800: move GLUE device to Q800MachineState

[Mark Cave-Ayland]

Also change the instantiation of the GLUE device to use object_initialize_child().

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/m68k/q800.c         | 24 ++++++++++++++----------
 include/hw/m68k/q800.h |  2 ++
 2 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 9f9de2ebaf..505e50d4af 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -158,7 +158,6 @@ static void q800_machine_init(MachineState *machine)
     SysBusDevice *sysbus;
     BusState *adb_bus;
     NubusBus *nubus;
-    DeviceState *glue;
     DriveInfo *dinfo;
     uint8_t rng_seed[32];
 
@@ -194,10 +193,10 @@ static void q800_machine_init(MachineState *machine)
     }
 
     /* IRQ Glue */
-    glue = qdev_new(TYPE_GLUE);
-    object_property_set_link(OBJECT(glue), "cpu", OBJECT(&m->cpu),
+    object_initialize_child(OBJECT(machine), "glue", &m->glue, TYPE_GLUE);
+    object_property_set_link(OBJECT(&m->glue), "cpu", OBJECT(&m->cpu),
                              &error_abort);
-    sysbus_realize_and_unref(SYS_BUS_DEVICE(glue), &error_fatal);
+    sysbus_realize(SYS_BUS_DEVICE(&m->glue), &error_fatal);
 
     /* VIA 1 */
     via1_dev = qdev_new(TYPE_MOS6522_Q800_VIA1);
@@ -208,10 +207,12 @@ static void q800_machine_init(MachineState *machine)
     sysbus = SYS_BUS_DEVICE(via1_dev);
     sysbus_realize_and_unref(sysbus, &error_fatal);
     sysbus_mmio_map(sysbus, 1, VIA_BASE);
-    sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in(glue, GLUE_IRQ_IN_VIA1));
+    sysbus_connect_irq(sysbus, 0,
+                       qdev_get_gpio_in(DEVICE(&m->glue), GLUE_IRQ_IN_VIA1));
     /* A/UX mode */
     qdev_connect_gpio_out(via1_dev, 0,
-                          qdev_get_gpio_in_named(glue, "auxmode", 0));
+                          qdev_get_gpio_in_named(DEVICE(&m->glue),
+                                                 "auxmode", 0));
 
     adb_bus = qdev_get_child_bus(via1_dev, "adb.0");
     dev = qdev_new(TYPE_ADB_KEYBOARD);
@@ -224,7 +225,8 @@ static void q800_machine_init(MachineState *machine)
     sysbus = SYS_BUS_DEVICE(via2_dev);
     sysbus_realize_and_unref(sysbus, &error_fatal);
     sysbus_mmio_map(sysbus, 1, VIA_BASE + VIA_SIZE);
-    sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in(glue, GLUE_IRQ_IN_VIA2));
+    sysbus_connect_irq(sysbus, 0,
+                       qdev_get_gpio_in(DEVICE(&m->glue), GLUE_IRQ_IN_VIA2));
 
     /* MACSONIC */
 
@@ -257,7 +259,8 @@ static void q800_machine_init(MachineState *machine)
     sysbus = SYS_BUS_DEVICE(dev);
     sysbus_realize_and_unref(sysbus, &error_fatal);
     sysbus_mmio_map(sysbus, 0, SONIC_BASE);
-    sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in(glue, GLUE_IRQ_IN_SONIC));
+    sysbus_connect_irq(sysbus, 0,
+                       qdev_get_gpio_in(DEVICE(&m->glue), GLUE_IRQ_IN_SONIC));
 
     memory_region_init_rom(dp8393x_prom, NULL, "dp8393x-q800.prom",
                            SONIC_PROM_SIZE, &error_fatal);
@@ -294,7 +297,8 @@ static void q800_machine_init(MachineState *machine)
     sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in(escc_orgate, 0));
     sysbus_connect_irq(sysbus, 1, qdev_get_gpio_in(escc_orgate, 1));
     qdev_connect_gpio_out(escc_orgate, 0,
-                          qdev_get_gpio_in(glue, GLUE_IRQ_IN_ESCC));
+                          qdev_get_gpio_in(DEVICE(&m->glue),
+                                           GLUE_IRQ_IN_ESCC));
     sysbus_mmio_map(sysbus, 0, SCC_BASE);
 
     /* SCSI */
@@ -349,7 +353,7 @@ static void q800_machine_init(MachineState *machine)
      * Since the framebuffer in slot 0x9 uses a separate IRQ, wire the unused
      * IRQ via GLUE for use by SONIC Ethernet in classic mode
      */
-    qdev_connect_gpio_out(glue, GLUE_IRQ_NUBUS_9,
+    qdev_connect_gpio_out(DEVICE(&m->glue), GLUE_IRQ_NUBUS_9,
                           qdev_get_gpio_in_named(via2_dev, "nubus-irq",
                                                  VIA2_NUBUS_IRQ_9));
 
diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
index d1f1ae4b88..fda42e0a1c 100644
--- a/include/hw/m68k/q800.h
+++ b/include/hw/m68k/q800.h
@@ -27,6 +27,7 @@
 #include "qom/object.h"
 #include "target/m68k/cpu-qom.h"
 #include "exec/memory.h"
+#include "hw/m68k/q800-glue.h"
 
 /*
  * The main Q800 machine
@@ -37,6 +38,7 @@ struct Q800MachineState {
 
     M68kCPU cpu;
     MemoryRegion rom;
+    GLUEState glue;
 };
 
 #define TYPE_Q800_MACHINE MACHINE_TYPE_NAME("q800")
-- 
2.30.2

[PATCH v4 10/24] q800: introduce mac-io container memory region

[Mark Cave-Ayland]

Move all devices from the IO region to within the container in preparation
for updating the IO aliasing mechanism.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/m68k/q800.c         | 6 ++++++
 include/hw/m68k/q800.h | 1 +
 2 files changed, 7 insertions(+)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 505e50d4af..359bdf3443 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -177,6 +177,12 @@ static void q800_machine_init(MachineState *machine)
     /* RAM */
     memory_region_add_subregion(get_system_memory(), 0, machine->ram);
 
+    /*
+     * Create container for all IO devices
+     */
+    memory_region_init(&m->macio, OBJECT(machine), "mac-io", IO_SLICE);
+    memory_region_add_subregion(get_system_memory(), IO_BASE, &m->macio);
+
     /*
      * Memory from IO_BASE to IO_BASE + IO_SLICE is repeated
      * from IO_BASE + IO_SLICE to IO_BASE + IO_SIZE
diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
index fda42e0a1c..17067dfad7 100644
--- a/include/hw/m68k/q800.h
+++ b/include/hw/m68k/q800.h
@@ -39,6 +39,7 @@ struct Q800MachineState {
     M68kCPU cpu;
     MemoryRegion rom;
     GLUEState glue;
+    MemoryRegion macio;
 };
 
 #define TYPE_Q800_MACHINE MACHINE_TYPE_NAME("q800")
-- 
2.30.2

[PATCH v4 11/24] q800: reimplement mac-io region aliasing using IO memory region

[Mark Cave-Ayland]

The current use of aliased memory regions causes us 2 problems: firstly the
output of "info qom-tree" is absolutely huge and difficult to read, and
secondly we have already reached the internal limit for memory regions as
adding any new memory region into the mac-io region causes QEMU to assert
with "phys_section_add: Assertion `map->sections_nb < TARGET_PAGE_SIZE'
failed".

Implement the mac-io region aliasing using a single IO memory region that
applies IO_SLICE_MASK representing the maximum size of the aliased region and
then forwarding the access to the existing mac-io memory region using the
address space API.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/m68k/q800.c         | 100 +++++++++++++++++++++++++++++++++--------
 include/hw/m68k/q800.h |   1 +
 2 files changed, 82 insertions(+), 19 deletions(-)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 359bdf3443..51b8d8ec3c 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -59,6 +59,7 @@
 
 #define IO_BASE               0x50000000
 #define IO_SLICE              0x00040000
+#define IO_SLICE_MASK         (IO_SLICE - 1)
 #define IO_SIZE               0x04000000
 
 #define VIA_BASE              (IO_BASE + 0x00000)
@@ -127,6 +128,68 @@ static uint8_t fake_mac_rom[] = {
     0x60, 0xFE                          /* bras [self] */
 };
 
+static MemTxResult macio_alias_read(void *opaque, hwaddr addr, uint64_t *data,
+                                    unsigned size, MemTxAttrs attrs)
+{
+    MemTxResult r;
+    uint32_t val;
+
+    addr &= IO_SLICE_MASK;
+    addr |= IO_BASE;
+
+    switch (size) {
+    case 4:
+        val = address_space_ldl_be(&address_space_memory, addr, attrs, &r);
+        break;
+    case 2:
+        val = address_space_lduw_be(&address_space_memory, addr, attrs, &r);
+        break;
+    case 1:
+        val = address_space_ldub(&address_space_memory, addr, attrs, &r);
+        break;
+    default:
+        g_assert_not_reached();
+    }
+
+    *data = val;
+    return r;
+}
+
+static MemTxResult macio_alias_write(void *opaque, hwaddr addr, uint64_t value,
+                                     unsigned size, MemTxAttrs attrs)
+{
+    MemTxResult r;
+
+    addr &= IO_SLICE_MASK;
+    addr |= IO_BASE;
+
+    switch (size) {
+    case 4:
+        address_space_stl_be(&address_space_memory, addr, value, attrs, &r);
+        break;
+    case 2:
+        address_space_stw_be(&address_space_memory, addr, value, attrs, &r);
+        break;
+    case 1:
+        address_space_stb(&address_space_memory, addr, value, attrs, &r);
+        break;
+    default:
+        g_assert_not_reached();
+    }
+
+    return r;
+}
+
+static const MemoryRegionOps macio_alias_ops = {
+    .read_with_attrs = macio_alias_read,
+    .write_with_attrs = macio_alias_write,
+    .endianness = DEVICE_BIG_ENDIAN,
+    .valid = {
+        .min_access_size = 1,
+        .max_access_size = 4,
+    },
+};
+
 static void q800_machine_init(MachineState *machine)
 {
     Q800MachineState *m = Q800_MACHINE(machine);
@@ -137,10 +200,8 @@ static void q800_machine_init(MachineState *machine)
     int bios_size;
     ram_addr_t initrd_base;
     int32_t initrd_size;
-    MemoryRegion *io;
     MemoryRegion *dp8393x_prom = g_new(MemoryRegion, 1);
     uint8_t *prom;
-    const int io_slice_nb = (IO_SIZE / IO_SLICE) - 1;
     int i, checksum;
     MacFbMode *macfb_mode;
     ram_addr_t ram_size = machine->ram_size;
@@ -187,16 +248,10 @@ static void q800_machine_init(MachineState *machine)
      * Memory from IO_BASE to IO_BASE + IO_SLICE is repeated
      * from IO_BASE + IO_SLICE to IO_BASE + IO_SIZE
      */
-    io = g_new(MemoryRegion, io_slice_nb);
-    for (i = 0; i < io_slice_nb; i++) {
-        char *name = g_strdup_printf("mac_m68k.io[%d]", i + 1);
-
-        memory_region_init_alias(&io[i], NULL, name, get_system_memory(),
-                                 IO_BASE, IO_SLICE);
-        memory_region_add_subregion(get_system_memory(),
-                                    IO_BASE + (i + 1) * IO_SLICE, &io[i]);
-        g_free(name);
-    }
+    memory_region_init_io(&m->macio_alias, OBJECT(machine), &macio_alias_ops,
+                          &m->macio, "mac-io.alias", IO_SIZE - IO_SLICE);
+    memory_region_add_subregion(get_system_memory(), IO_BASE + IO_SLICE,
+                                &m->macio_alias);
 
     /* IRQ Glue */
     object_initialize_child(OBJECT(machine), "glue", &m->glue, TYPE_GLUE);
@@ -212,7 +267,8 @@ static void q800_machine_init(MachineState *machine)
     }
     sysbus = SYS_BUS_DEVICE(via1_dev);
     sysbus_realize_and_unref(sysbus, &error_fatal);
-    sysbus_mmio_map(sysbus, 1, VIA_BASE);
+    memory_region_add_subregion(&m->macio, VIA_BASE - IO_BASE,
+                                sysbus_mmio_get_region(sysbus, 1));
     sysbus_connect_irq(sysbus, 0,
                        qdev_get_gpio_in(DEVICE(&m->glue), GLUE_IRQ_IN_VIA1));
     /* A/UX mode */
@@ -230,7 +286,8 @@ static void q800_machine_init(MachineState *machine)
     via2_dev = qdev_new(TYPE_MOS6522_Q800_VIA2);
     sysbus = SYS_BUS_DEVICE(via2_dev);
     sysbus_realize_and_unref(sysbus, &error_fatal);
-    sysbus_mmio_map(sysbus, 1, VIA_BASE + VIA_SIZE);
+    memory_region_add_subregion(&m->macio, VIA_BASE - IO_BASE + VIA_SIZE,
+                                sysbus_mmio_get_region(sysbus, 1));
     sysbus_connect_irq(sysbus, 0,
                        qdev_get_gpio_in(DEVICE(&m->glue), GLUE_IRQ_IN_VIA2));
 
@@ -264,7 +321,8 @@ static void q800_machine_init(MachineState *machine)
                              OBJECT(get_system_memory()), &error_abort);
     sysbus = SYS_BUS_DEVICE(dev);
     sysbus_realize_and_unref(sysbus, &error_fatal);
-    sysbus_mmio_map(sysbus, 0, SONIC_BASE);
+    memory_region_add_subregion(&m->macio, SONIC_BASE - IO_BASE,
+                                sysbus_mmio_get_region(sysbus, 0));
     sysbus_connect_irq(sysbus, 0,
                        qdev_get_gpio_in(DEVICE(&m->glue), GLUE_IRQ_IN_SONIC));
 
@@ -305,7 +363,8 @@ static void q800_machine_init(MachineState *machine)
     qdev_connect_gpio_out(escc_orgate, 0,
                           qdev_get_gpio_in(DEVICE(&m->glue),
                                            GLUE_IRQ_IN_ESCC));
-    sysbus_mmio_map(sysbus, 0, SCC_BASE);
+    memory_region_add_subregion(&m->macio, SCC_BASE - IO_BASE,
+                                sysbus_mmio_get_region(sysbus, 0));
 
     /* SCSI */
 
@@ -325,8 +384,10 @@ static void q800_machine_init(MachineState *machine)
                                                   VIA2_IRQ_SCSI_BIT)));
     sysbus_connect_irq(sysbus, 1, qemu_irq_invert(qdev_get_gpio_in(via2_dev,
                                                   VIA2_IRQ_SCSI_DATA_BIT)));
-    sysbus_mmio_map(sysbus, 0, ESP_BASE);
-    sysbus_mmio_map(sysbus, 1, ESP_PDMA);
+    memory_region_add_subregion(&m->macio, ESP_BASE - IO_BASE,
+                                sysbus_mmio_get_region(sysbus, 0));
+    memory_region_add_subregion(&m->macio, ESP_PDMA - IO_BASE,
+                                sysbus_mmio_get_region(sysbus, 1));
 
     scsi_bus_legacy_handle_cmdline(&esp->bus);
 
@@ -334,7 +395,8 @@ static void q800_machine_init(MachineState *machine)
 
     dev = qdev_new(TYPE_SWIM);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
-    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, SWIM_BASE);
+    memory_region_add_subregion(&m->macio, SWIM_BASE - IO_BASE,
+                                sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0));
 
     /* NuBus */
 
diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
index 17067dfad7..1ed38bf0b1 100644
--- a/include/hw/m68k/q800.h
+++ b/include/hw/m68k/q800.h
@@ -40,6 +40,7 @@ struct Q800MachineState {
     MemoryRegion rom;
     GLUEState glue;
     MemoryRegion macio;
+    MemoryRegion macio_alias;
 };
 
 #define TYPE_Q800_MACHINE MACHINE_TYPE_NAME("q800")
-- 
2.30.2

[PATCH v4 12/24] q800: move VIA1 device to Q800MachineState

[Mark Cave-Ayland]

Also change the instantiation of the VIA1 device to use object_initialize_child().

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/m68k/q800.c         | 16 +++++++++-------
 include/hw/m68k/q800.h |  2 ++
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 51b8d8ec3c..fccdad5f3c 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -212,7 +212,7 @@ static void q800_machine_init(MachineState *machine)
     hwaddr parameters_base;
     CPUState *cs;
     DeviceState *dev;
-    DeviceState *via1_dev, *via2_dev;
+    DeviceState *via2_dev;
     DeviceState *escc_orgate;
     SysBusESPState *sysbus_esp;
     ESPState *esp;
@@ -260,23 +260,25 @@ static void q800_machine_init(MachineState *machine)
     sysbus_realize(SYS_BUS_DEVICE(&m->glue), &error_fatal);
 
     /* VIA 1 */
-    via1_dev = qdev_new(TYPE_MOS6522_Q800_VIA1);
+    object_initialize_child(OBJECT(machine), "via1", &m->via1,
+                            TYPE_MOS6522_Q800_VIA1);
     dinfo = drive_get(IF_MTD, 0, 0);
     if (dinfo) {
-        qdev_prop_set_drive(via1_dev, "drive", blk_by_legacy_dinfo(dinfo));
+        qdev_prop_set_drive(DEVICE(&m->via1), "drive",
+                            blk_by_legacy_dinfo(dinfo));
     }
-    sysbus = SYS_BUS_DEVICE(via1_dev);
-    sysbus_realize_and_unref(sysbus, &error_fatal);
+    sysbus = SYS_BUS_DEVICE(&m->via1);
+    sysbus_realize(sysbus, &error_fatal);
     memory_region_add_subregion(&m->macio, VIA_BASE - IO_BASE,
                                 sysbus_mmio_get_region(sysbus, 1));
     sysbus_connect_irq(sysbus, 0,
                        qdev_get_gpio_in(DEVICE(&m->glue), GLUE_IRQ_IN_VIA1));
     /* A/UX mode */
-    qdev_connect_gpio_out(via1_dev, 0,
+    qdev_connect_gpio_out(DEVICE(&m->via1), 0,
                           qdev_get_gpio_in_named(DEVICE(&m->glue),
                                                  "auxmode", 0));
 
-    adb_bus = qdev_get_child_bus(via1_dev, "adb.0");
+    adb_bus = qdev_get_child_bus(DEVICE(&m->via1), "adb.0");
     dev = qdev_new(TYPE_ADB_KEYBOARD);
     qdev_realize_and_unref(dev, adb_bus, &error_fatal);
     dev = qdev_new(TYPE_ADB_MOUSE);
diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
index 1ed38bf0b1..5cf66d08a0 100644
--- a/include/hw/m68k/q800.h
+++ b/include/hw/m68k/q800.h
@@ -28,6 +28,7 @@
 #include "target/m68k/cpu-qom.h"
 #include "exec/memory.h"
 #include "hw/m68k/q800-glue.h"
+#include "hw/misc/mac_via.h"
 
 /*
  * The main Q800 machine
@@ -39,6 +40,7 @@ struct Q800MachineState {
     M68kCPU cpu;
     MemoryRegion rom;
     GLUEState glue;
+    MOS6522Q800VIA1State via1;
     MemoryRegion macio;
     MemoryRegion macio_alias;
 };
-- 
2.30.2

[PATCH v4 13/24] q800: move VIA2 device to Q800MachineState

[Mark Cave-Ayland]

Also change the instantiation of the VIA2 device to use object_initialize_child().

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/m68k/q800.c         | 27 ++++++++++++++++-----------
 include/hw/m68k/q800.h |  1 +
 2 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index fccdad5f3c..988b4981b8 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -212,7 +212,6 @@ static void q800_machine_init(MachineState *machine)
     hwaddr parameters_base;
     CPUState *cs;
     DeviceState *dev;
-    DeviceState *via2_dev;
     DeviceState *escc_orgate;
     SysBusESPState *sysbus_esp;
     ESPState *esp;
@@ -285,9 +284,10 @@ static void q800_machine_init(MachineState *machine)
     qdev_realize_and_unref(dev, adb_bus, &error_fatal);
 
     /* VIA 2 */
-    via2_dev = qdev_new(TYPE_MOS6522_Q800_VIA2);
-    sysbus = SYS_BUS_DEVICE(via2_dev);
-    sysbus_realize_and_unref(sysbus, &error_fatal);
+    object_initialize_child(OBJECT(machine), "via2", &m->via2,
+                            TYPE_MOS6522_Q800_VIA2);
+    sysbus = SYS_BUS_DEVICE(&m->via2);
+    sysbus_realize(sysbus, &error_fatal);
     memory_region_add_subregion(&m->macio, VIA_BASE - IO_BASE + VIA_SIZE,
                                 sysbus_mmio_get_region(sysbus, 1));
     sysbus_connect_irq(sysbus, 0,
@@ -382,10 +382,14 @@ static void q800_machine_init(MachineState *machine)
     sysbus = SYS_BUS_DEVICE(dev);
     sysbus_realize_and_unref(sysbus, &error_fatal);
     /* SCSI and SCSI data IRQs are negative edge triggered */
-    sysbus_connect_irq(sysbus, 0, qemu_irq_invert(qdev_get_gpio_in(via2_dev,
-                                                  VIA2_IRQ_SCSI_BIT)));
-    sysbus_connect_irq(sysbus, 1, qemu_irq_invert(qdev_get_gpio_in(via2_dev,
-                                                  VIA2_IRQ_SCSI_DATA_BIT)));
+    sysbus_connect_irq(sysbus, 0,
+                       qemu_irq_invert(
+                           qdev_get_gpio_in(DEVICE(&m->via2),
+                                                   VIA2_IRQ_SCSI_BIT)));
+    sysbus_connect_irq(sysbus, 1,
+                       qemu_irq_invert(
+                           qdev_get_gpio_in(DEVICE(&m->via2),
+                                                   VIA2_IRQ_SCSI_DATA_BIT)));
     memory_region_add_subregion(&m->macio, ESP_BASE - IO_BASE,
                                 sysbus_mmio_get_region(sysbus, 0));
     memory_region_add_subregion(&m->macio, ESP_PDMA - IO_BASE,
@@ -411,11 +415,12 @@ static void q800_machine_init(MachineState *machine)
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, NUBUS_SLOT_BASE +
                     MAC_NUBUS_FIRST_SLOT * NUBUS_SLOT_SIZE);
     qdev_connect_gpio_out(dev, 9,
-                          qdev_get_gpio_in_named(via2_dev, "nubus-irq",
+                          qdev_get_gpio_in_named(DEVICE(&m->via2), "nubus-irq",
                           VIA2_NUBUS_IRQ_INTVIDEO));
     for (i = 1; i < VIA2_NUBUS_IRQ_NB; i++) {
         qdev_connect_gpio_out(dev, 9 + i,
-                              qdev_get_gpio_in_named(via2_dev, "nubus-irq",
+                              qdev_get_gpio_in_named(DEVICE(&m->via2),
+                                                     "nubus-irq",
                                                      VIA2_NUBUS_IRQ_9 + i));
     }
 
@@ -424,7 +429,7 @@ static void q800_machine_init(MachineState *machine)
      * IRQ via GLUE for use by SONIC Ethernet in classic mode
      */
     qdev_connect_gpio_out(DEVICE(&m->glue), GLUE_IRQ_NUBUS_9,
-                          qdev_get_gpio_in_named(via2_dev, "nubus-irq",
+                          qdev_get_gpio_in_named(DEVICE(&m->via2), "nubus-irq",
                                                  VIA2_NUBUS_IRQ_9));
 
     nubus = &NUBUS_BRIDGE(dev)->bus;
diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
index 5cf66d08a0..06c771635b 100644
--- a/include/hw/m68k/q800.h
+++ b/include/hw/m68k/q800.h
@@ -41,6 +41,7 @@ struct Q800MachineState {
     MemoryRegion rom;
     GLUEState glue;
     MOS6522Q800VIA1State via1;
+    MOS6522Q800VIA2State via2;
     MemoryRegion macio;
     MemoryRegion macio_alias;
 };
-- 
2.30.2

[PATCH v4 14/24] hw/net/dp8393x.c: move TYPE_DP8393X and dp8393xState into dp8393x.h

[Mark Cave-Ayland]

This is to enable them to be used outside of dp8393x.c.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
CC: Jason Wang <jasowang@redhat.com>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/net/dp8393x.c         | 32 +--------------------
 include/hw/net/dp8393x.h | 60 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+), 31 deletions(-)
 create mode 100644 include/hw/net/dp8393x.h

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 45b954e46c..a596f7fbc6 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -20,6 +20,7 @@
 #include "qemu/osdep.h"
 #include "hw/irq.h"
 #include "hw/qdev-properties.h"
+#include "hw/net/dp8393x.h"
 #include "hw/sysbus.h"
 #include "migration/vmstate.h"
 #include "net/net.h"
@@ -85,7 +86,6 @@ static const char *reg_names[] = {
 #define SONIC_MPT    0x2e
 #define SONIC_MDT    0x2f
 #define SONIC_DCR2   0x3f
-#define SONIC_REG_COUNT  0x40
 
 #define SONIC_CR_HTX     0x0001
 #define SONIC_CR_TXP     0x0002
@@ -139,36 +139,6 @@ static const char *reg_names[] = {
 #define SONIC_DESC_EOL   0x0001
 #define SONIC_DESC_ADDR  0xFFFE
 
-#define TYPE_DP8393X "dp8393x"
-OBJECT_DECLARE_SIMPLE_TYPE(dp8393xState, DP8393X)
-
-struct dp8393xState {
-    SysBusDevice parent_obj;
-
-    /* Hardware */
-    uint8_t it_shift;
-    bool big_endian;
-    bool last_rba_is_full;
-    qemu_irq irq;
-    int irq_level;
-    QEMUTimer *watchdog;
-    int64_t wt_last_update;
-    NICConf conf;
-    NICState *nic;
-    MemoryRegion mmio;
-
-    /* Registers */
-    uint16_t cam[16][3];
-    uint16_t regs[SONIC_REG_COUNT];
-
-    /* Temporaries */
-    uint8_t tx_buffer[0x10000];
-    int loopback_packet;
-
-    /* Memory access */
-    MemoryRegion *dma_mr;
-    AddressSpace as;
-};
 
 /*
  * Accessor functions for values which are formed by
diff --git a/include/hw/net/dp8393x.h b/include/hw/net/dp8393x.h
new file mode 100644
index 0000000000..4a3f7478be
--- /dev/null
+++ b/include/hw/net/dp8393x.h
@@ -0,0 +1,60 @@
+/*
+ * QEMU NS SONIC DP8393x netcard
+ *
+ * Copyright (c) 2008-2009 Herve Poussineau
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef HW_NET_DP8393X_H
+#define HW_NET_DP8393X_H
+
+#include "hw/sysbus.h"
+#include "net/net.h"
+#include "exec/memory.h"
+
+#define SONIC_REG_COUNT  0x40
+
+#define TYPE_DP8393X "dp8393x"
+OBJECT_DECLARE_SIMPLE_TYPE(dp8393xState, DP8393X)
+
+struct dp8393xState {
+    SysBusDevice parent_obj;
+
+    /* Hardware */
+    uint8_t it_shift;
+    bool big_endian;
+    bool last_rba_is_full;
+    qemu_irq irq;
+    int irq_level;
+    QEMUTimer *watchdog;
+    int64_t wt_last_update;
+    NICConf conf;
+    NICState *nic;
+    MemoryRegion mmio;
+
+    /* Registers */
+    uint16_t cam[16][3];
+    uint16_t regs[SONIC_REG_COUNT];
+
+    /* Temporaries */
+    uint8_t tx_buffer[0x10000];
+    int loopback_packet;
+
+    /* Memory access */
+    MemoryRegion *dma_mr;
+    AddressSpace as;
+};
+
+#endif
-- 
2.30.2

[PATCH v4 15/24] q800: move dp8393x device to Q800MachineState

[Mark Cave-Ayland]

Also change the instantiation of the dp8393x device to use object_initialize_child().

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
CC: Jason Wang <jasowang@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/m68k/q800.c         | 6 ++++--
 include/hw/m68k/q800.h | 2 ++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 988b4981b8..13806613fa 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -315,14 +315,16 @@ static void q800_machine_init(MachineState *machine)
     nd_table[0].macaddr.a[1] = 0x00;
     nd_table[0].macaddr.a[2] = 0x07;
 
-    dev = qdev_new("dp8393x");
+    object_initialize_child(OBJECT(machine), "dp8393x", &m->dp8393x,
+                            TYPE_DP8393X);
+    dev = DEVICE(&m->dp8393x);
     qdev_set_nic_properties(dev, &nd_table[0]);
     qdev_prop_set_uint8(dev, "it_shift", 2);
     qdev_prop_set_bit(dev, "big_endian", true);
     object_property_set_link(OBJECT(dev), "dma_mr",
                              OBJECT(get_system_memory()), &error_abort);
     sysbus = SYS_BUS_DEVICE(dev);
-    sysbus_realize_and_unref(sysbus, &error_fatal);
+    sysbus_realize(sysbus, &error_fatal);
     memory_region_add_subregion(&m->macio, SONIC_BASE - IO_BASE,
                                 sysbus_mmio_get_region(sysbus, 0));
     sysbus_connect_irq(sysbus, 0,
diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
index 06c771635b..d11bc020ed 100644
--- a/include/hw/m68k/q800.h
+++ b/include/hw/m68k/q800.h
@@ -29,6 +29,7 @@
 #include "exec/memory.h"
 #include "hw/m68k/q800-glue.h"
 #include "hw/misc/mac_via.h"
+#include "hw/net/dp8393x.h"
 
 /*
  * The main Q800 machine
@@ -42,6 +43,7 @@ struct Q800MachineState {
     GLUEState glue;
     MOS6522Q800VIA1State via1;
     MOS6522Q800VIA2State via2;
+    dp8393xState dp8393x;
     MemoryRegion macio;
     MemoryRegion macio_alias;
 };
-- 
2.30.2

[PATCH v4 16/24] q800: move ESCC device to Q800MachineState

[Mark Cave-Ayland]

Also change the instantiation of the ESCC device to use object_initialize_child().

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/m68k/q800.c         | 6 ++++--
 include/hw/m68k/q800.h | 2 ++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 13806613fa..8bf94b2511 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -346,7 +346,9 @@ static void q800_machine_init(MachineState *machine)
 
     /* SCC */
 
-    dev = qdev_new(TYPE_ESCC);
+    object_initialize_child(OBJECT(machine), "escc", &m->escc,
+                            TYPE_ESCC);
+    dev = DEVICE(&m->escc);
     qdev_prop_set_uint32(dev, "disabled", 0);
     qdev_prop_set_uint32(dev, "frequency", MAC_CLOCK);
     qdev_prop_set_uint32(dev, "it_shift", 1);
@@ -356,7 +358,7 @@ static void q800_machine_init(MachineState *machine)
     qdev_prop_set_uint32(dev, "chnBtype", 0);
     qdev_prop_set_uint32(dev, "chnAtype", 0);
     sysbus = SYS_BUS_DEVICE(dev);
-    sysbus_realize_and_unref(sysbus, &error_fatal);
+    sysbus_realize(sysbus, &error_fatal);
 
     /* Logically OR both its IRQs together */
     escc_orgate = DEVICE(object_new(TYPE_OR_IRQ));
diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
index d11bc020ed..9e76a3fe7c 100644
--- a/include/hw/m68k/q800.h
+++ b/include/hw/m68k/q800.h
@@ -30,6 +30,7 @@
 #include "hw/m68k/q800-glue.h"
 #include "hw/misc/mac_via.h"
 #include "hw/net/dp8393x.h"
+#include "hw/char/escc.h"
 
 /*
  * The main Q800 machine
@@ -44,6 +45,7 @@ struct Q800MachineState {
     MOS6522Q800VIA1State via1;
     MOS6522Q800VIA2State via2;
     dp8393xState dp8393x;
+    ESCCState escc;
     MemoryRegion macio;
     MemoryRegion macio_alias;
 };
-- 
2.30.2

[PATCH v4 17/24] q800: move escc_orgate device to Q800MachineState

[Mark Cave-Ayland]

Also change the instantiation of the escc_orgate device to use object_initialize_child().

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/m68k/q800.c         | 16 +++++++++-------
 include/hw/m68k/q800.h |  2 ++
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 8bf94b2511..c6314c6bf9 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -212,7 +212,6 @@ static void q800_machine_init(MachineState *machine)
     hwaddr parameters_base;
     CPUState *cs;
     DeviceState *dev;
-    DeviceState *escc_orgate;
     SysBusESPState *sysbus_esp;
     ESPState *esp;
     SysBusDevice *sysbus;
@@ -361,12 +360,15 @@ static void q800_machine_init(MachineState *machine)
     sysbus_realize(sysbus, &error_fatal);
 
     /* Logically OR both its IRQs together */
-    escc_orgate = DEVICE(object_new(TYPE_OR_IRQ));
-    object_property_set_int(OBJECT(escc_orgate), "num-lines", 2, &error_fatal);
-    qdev_realize_and_unref(escc_orgate, NULL, &error_fatal);
-    sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in(escc_orgate, 0));
-    sysbus_connect_irq(sysbus, 1, qdev_get_gpio_in(escc_orgate, 1));
-    qdev_connect_gpio_out(escc_orgate, 0,
+    object_initialize_child(OBJECT(machine), "escc_orgate", &m->escc_orgate,
+                            TYPE_OR_IRQ);
+    object_property_set_int(OBJECT(&m->escc_orgate), "num-lines", 2,
+                            &error_fatal);
+    dev = DEVICE(&m->escc_orgate);
+    qdev_realize(dev, NULL, &error_fatal);
+    sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in(dev, 0));
+    sysbus_connect_irq(sysbus, 1, qdev_get_gpio_in(dev, 1));
+    qdev_connect_gpio_out(dev, 0,
                           qdev_get_gpio_in(DEVICE(&m->glue),
                                            GLUE_IRQ_IN_ESCC));
     memory_region_add_subregion(&m->macio, SCC_BASE - IO_BASE,
diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
index 9e76a3fe7c..36e1bd8e4e 100644
--- a/include/hw/m68k/q800.h
+++ b/include/hw/m68k/q800.h
@@ -31,6 +31,7 @@
 #include "hw/misc/mac_via.h"
 #include "hw/net/dp8393x.h"
 #include "hw/char/escc.h"
+#include "hw/or-irq.h"
 
 /*
  * The main Q800 machine
@@ -46,6 +47,7 @@ struct Q800MachineState {
     MOS6522Q800VIA2State via2;
     dp8393xState dp8393x;
     ESCCState escc;
+    OrIRQState escc_orgate;
     MemoryRegion macio;
     MemoryRegion macio_alias;
 };
-- 
2.30.2

[PATCH v4 18/24] q800: move ESP device to Q800MachineState

[Mark Cave-Ayland]

Also change the instantiation of the ESP device to use object_initialize_child().

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/m68k/q800.c         | 9 +++++----
 include/hw/m68k/q800.h | 2 ++
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index c6314c6bf9..9da46f4456 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -376,8 +376,9 @@ static void q800_machine_init(MachineState *machine)
 
     /* SCSI */
 
-    dev = qdev_new(TYPE_SYSBUS_ESP);
-    sysbus_esp = SYSBUS_ESP(dev);
+    object_initialize_child(OBJECT(machine), "esp", &m->esp,
+                            TYPE_SYSBUS_ESP);
+    sysbus_esp = SYSBUS_ESP(&m->esp);
     esp = &sysbus_esp->esp;
     esp->dma_memory_read = NULL;
     esp->dma_memory_write = NULL;
@@ -385,8 +386,8 @@ static void q800_machine_init(MachineState *machine)
     sysbus_esp->it_shift = 4;
     esp->dma_enabled = 1;
 
-    sysbus = SYS_BUS_DEVICE(dev);
-    sysbus_realize_and_unref(sysbus, &error_fatal);
+    sysbus = SYS_BUS_DEVICE(&m->esp);
+    sysbus_realize(sysbus, &error_fatal);
     /* SCSI and SCSI data IRQs are negative edge triggered */
     sysbus_connect_irq(sysbus, 0,
                        qemu_irq_invert(
diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
index 36e1bd8e4e..8f23e0c4c6 100644
--- a/include/hw/m68k/q800.h
+++ b/include/hw/m68k/q800.h
@@ -32,6 +32,7 @@
 #include "hw/net/dp8393x.h"
 #include "hw/char/escc.h"
 #include "hw/or-irq.h"
+#include "hw/scsi/esp.h"
 
 /*
  * The main Q800 machine
@@ -48,6 +49,7 @@ struct Q800MachineState {
     dp8393xState dp8393x;
     ESCCState escc;
     OrIRQState escc_orgate;
+    SysBusESPState esp;
     MemoryRegion macio;
     MemoryRegion macio_alias;
 };
-- 
2.30.2

[PATCH v4 19/24] q800: move SWIM device to Q800MachineState

[Mark Cave-Ayland]

Also change the instantiation of the SWIM device to use object_initialize_child().

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/m68k/q800.c         | 8 +++++---
 include/hw/m68k/q800.h | 2 ++
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 9da46f4456..50fc7de9a2 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -406,10 +406,12 @@ static void q800_machine_init(MachineState *machine)
 
     /* SWIM floppy controller */
 
-    dev = qdev_new(TYPE_SWIM);
-    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
+    object_initialize_child(OBJECT(machine), "swim", &m->swim,
+                            TYPE_SWIM);
+    sysbus = SYS_BUS_DEVICE(&m->swim);
+    sysbus_realize(sysbus, &error_fatal);
     memory_region_add_subregion(&m->macio, SWIM_BASE - IO_BASE,
-                                sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0));
+                                sysbus_mmio_get_region(sysbus, 0));
 
     /* NuBus */
 
diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
index 8f23e0c4c6..06e095ae29 100644
--- a/include/hw/m68k/q800.h
+++ b/include/hw/m68k/q800.h
@@ -33,6 +33,7 @@
 #include "hw/char/escc.h"
 #include "hw/or-irq.h"
 #include "hw/scsi/esp.h"
+#include "hw/block/swim.h"
 
 /*
  * The main Q800 machine
@@ -50,6 +51,7 @@ struct Q800MachineState {
     ESCCState escc;
     OrIRQState escc_orgate;
     SysBusESPState esp;
+    Swim swim;
     MemoryRegion macio;
     MemoryRegion macio_alias;
 };
-- 
2.30.2

[PATCH v4 20/24] q800: move mac-nubus-bridge device to Q800MachineState

[Mark Cave-Ayland]

Also change the instantiation of the mac-nubus-bridge device to use
object_initialize_child() and map the Nubus address space using
memory_region_add_subregion() instead of sysbus_mmio_map().

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/m68k/q800.c         | 21 ++++++++++++++-------
 include/hw/m68k/q800.h |  2 ++
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 50fc7de9a2..b22651931a 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -415,14 +415,21 @@ static void q800_machine_init(MachineState *machine)
 
     /* NuBus */
 
-    dev = qdev_new(TYPE_MAC_NUBUS_BRIDGE);
-    qdev_prop_set_uint32(dev, "slot-available-mask",
+    object_initialize_child(OBJECT(machine), "mac-nubus-bridge",
+                            &m->mac_nubus_bridge,
+                            TYPE_MAC_NUBUS_BRIDGE);
+    sysbus = SYS_BUS_DEVICE(&m->mac_nubus_bridge);
+    dev = DEVICE(&m->mac_nubus_bridge);
+    qdev_prop_set_uint32(DEVICE(&m->mac_nubus_bridge), "slot-available-mask",
                          Q800_NUBUS_SLOTS_AVAILABLE);
-    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
-    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0,
-                    MAC_NUBUS_FIRST_SLOT * NUBUS_SUPER_SLOT_SIZE);
-    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, NUBUS_SLOT_BASE +
-                    MAC_NUBUS_FIRST_SLOT * NUBUS_SLOT_SIZE);
+    sysbus_realize(sysbus, &error_fatal);
+    memory_region_add_subregion(get_system_memory(),
+                                MAC_NUBUS_FIRST_SLOT * NUBUS_SUPER_SLOT_SIZE,
+                                sysbus_mmio_get_region(sysbus, 0));
+    memory_region_add_subregion(get_system_memory(),
+                                NUBUS_SLOT_BASE +
+                                MAC_NUBUS_FIRST_SLOT * NUBUS_SLOT_SIZE,
+                                sysbus_mmio_get_region(sysbus, 1));
     qdev_connect_gpio_out(dev, 9,
                           qdev_get_gpio_in_named(DEVICE(&m->via2), "nubus-irq",
                           VIA2_NUBUS_IRQ_INTVIDEO));
diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
index 06e095ae29..8f2c572a81 100644
--- a/include/hw/m68k/q800.h
+++ b/include/hw/m68k/q800.h
@@ -34,6 +34,7 @@
 #include "hw/or-irq.h"
 #include "hw/scsi/esp.h"
 #include "hw/block/swim.h"
+#include "hw/nubus/mac-nubus-bridge.h"
 
 /*
  * The main Q800 machine
@@ -52,6 +53,7 @@ struct Q800MachineState {
     OrIRQState escc_orgate;
     SysBusESPState esp;
     Swim swim;
+    MacNubusBridge mac_nubus_bridge;
     MemoryRegion macio;
     MemoryRegion macio_alias;
 };
-- 
2.30.2

[PATCH v4 21/24] q800: don't access Nubus bus directly from the mac-nubus-bridge device

[Mark Cave-Ayland]

Instead use the qdev_get_child_bus() function which is intended for this exact
purpose.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/m68k/q800.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index b22651931a..a32e6fbf8d 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -448,7 +448,7 @@ static void q800_machine_init(MachineState *machine)
                           qdev_get_gpio_in_named(DEVICE(&m->via2), "nubus-irq",
                                                  VIA2_NUBUS_IRQ_9));
 
-    nubus = &NUBUS_BRIDGE(dev)->bus;
+    nubus = NUBUS_BUS(qdev_get_child_bus(dev, "nubus-bus.0"));
 
     /* framebuffer in nubus slot #9 */
 
-- 
2.30.2

[PATCH v4 22/24] q800: move macfb device to Q800MachineState

[Mark Cave-Ayland]

Also change the instantiation of the macfb device to use object_initialize_child().

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/m68k/q800.c         | 6 ++++--
 include/hw/m68k/q800.h | 2 ++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index a32e6fbf8d..b770b71d54 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -452,7 +452,9 @@ static void q800_machine_init(MachineState *machine)
 
     /* framebuffer in nubus slot #9 */
 
-    dev = qdev_new(TYPE_NUBUS_MACFB);
+    object_initialize_child(OBJECT(machine), "macfb", &m->macfb,
+                            TYPE_NUBUS_MACFB);
+    dev = DEVICE(&m->macfb);
     qdev_prop_set_uint32(dev, "slot", 9);
     qdev_prop_set_uint32(dev, "width", graphic_width);
     qdev_prop_set_uint32(dev, "height", graphic_height);
@@ -462,7 +464,7 @@ static void q800_machine_init(MachineState *machine)
     } else {
         qdev_prop_set_uint8(dev, "display", MACFB_DISPLAY_VGA);
     }
-    qdev_realize_and_unref(dev, BUS(nubus), &error_fatal);
+    qdev_realize(dev, BUS(nubus), &error_fatal);
 
     macfb_mode = (NUBUS_MACFB(dev)->macfb).mode;
 
diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
index 8f2c572a81..b3d77f1cba 100644
--- a/include/hw/m68k/q800.h
+++ b/include/hw/m68k/q800.h
@@ -35,6 +35,7 @@
 #include "hw/scsi/esp.h"
 #include "hw/block/swim.h"
 #include "hw/nubus/mac-nubus-bridge.h"
+#include "hw/display/macfb.h"
 
 /*
  * The main Q800 machine
@@ -54,6 +55,7 @@ struct Q800MachineState {
     SysBusESPState esp;
     Swim swim;
     MacNubusBridge mac_nubus_bridge;
+    MacfbNubusState macfb;
     MemoryRegion macio;
     MemoryRegion macio_alias;
 };
-- 
2.30.2

[PATCH v4 23/24] mac_via: fix rtc command decoding from PRAM addresses 0x0 to 0xf

[Mark Cave-Ayland]

A comparison between the rtc command table included in the comment and the code
itself shows that the decoding for PRAM addresses 0x0 to 0xf is being done on
the raw command, and not the shifted version held in value.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/misc/mac_via.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
index 076d18e5fd..85c2e65856 100644
--- a/hw/misc/mac_via.c
+++ b/hw/misc/mac_via.c
@@ -399,7 +399,7 @@ static int via1_rtc_compact_cmd(uint8_t value)
         } else if ((value & 0x1c) == 0x08) {
             /* RAM address 0x10 to 0x13 */
             return read | (REG_PRAM_ADDR + 0x10 + (value & 0x03));
-        } else if ((value & 0x43) == 0x41) {
+        } else if ((value & 0x10) == 0x10) {
             /* RAM address 0x00 to 0x0f */
             return read | (REG_PRAM_ADDR + (value & 0x0f));
         }
-- 
2.30.2

[PATCH v4 24/24] mac_via: fix rtc command decoding for the PRAM seconds registers

[Mark Cave-Ayland]

Analysis of the MacOS toolbox ROM code shows that on startup it attempts 2
separate reads of the seconds registers with commands 0x9d...0x91 followed by
0x8d..0x81 without resetting the command to its initial value. The PRAM seconds
value is only accepted when the values of the 2 separate reads match.

From this we conclude that bit 4 of the rtc command is not decoded or we don't
care about its value when reading the PRAM seconds registers. Implement this
decoding change so that both reads return successfully which allows the MacOS
toolbox ROM to correctly set the date/time.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/misc/mac_via.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
index 85c2e65856..0787a0268d 100644
--- a/hw/misc/mac_via.c
+++ b/hw/misc/mac_via.c
@@ -362,10 +362,10 @@ static void pram_update(MOS6522Q800VIA1State *v1s)
  *
  * Command byte    Register addressed by the command
  *
- * z0000001        Seconds register 0 (lowest-order byte)
- * z0000101        Seconds register 1
- * z0001001        Seconds register 2
- * z0001101        Seconds register 3 (highest-order byte)
+ * z00x0001        Seconds register 0 (lowest-order byte)
+ * z00x0101        Seconds register 1
+ * z00x1001        Seconds register 2
+ * z00x1101        Seconds register 3 (highest-order byte)
  * 00110001        Test register (write-only)
  * 00110101        Write-Protect Register (write-only)
  * z010aa01        RAM address 100aa ($10-$13) (first 20 bytes only)
@@ -373,6 +373,7 @@ static void pram_update(MOS6522Q800VIA1State *v1s)
  * z0111aaa        Extended memory designator and sector number
  *
  * For a read request, z=1, for a write z=0
+ * The letter x indicates don't care
  * The letter a indicates bits whose value depend on what parameter
  * RAM byte you want to address
  */
@@ -389,7 +390,7 @@ static int via1_rtc_compact_cmd(uint8_t value)
     }
     if ((value & 0x03) == 0x01) {
         value >>= 2;
-        if ((value & 0x1c) == 0) {
+        if ((value & 0x18) == 0) {
             /* seconds registers */
             return read | (REG_0 + (value & 0x03));
         } else if ((value == 0x0c) && !read) {
-- 
2.30.2

Re: [PATCH v4 00/24] q800: add support for booting MacOS Classic - part 1

[Philippe Mathieu-Daudé]

On 21/6/23 10:53, Mark Cave-Ayland wrote:
> [MCA: the original series has now been split into 2 separate parts based upon
> Phil's comments re: QOM parenting for objects in Q800MachineState. Part 1
> consists of the Q800MachineState patches along with QOM parenting fixes and
> the 2 mac_via RTC patches.]
> 
> This series contains the remaining patches needed to allow QEMU's q800
> machine to boot MacOS Classic when used in conjunction with a real
> Quadra 800 ROM image. In fact with this series applied it is possible
> to boot all of the following OSs:
> 
>    - MacOS 7.1 - 8.1, with or without virtual memory enabled
>    - A/UX 3.0.1
>    - NetBSD 9.3
>    - Linux (via EMILE)
> 
> If you are ready to experience some 90s nostalgia then all you need is
> to grab yourself a copy of the Quadra 800 ROM (checksum 0xf1acad13) and a
> suitable install ISO as follows:
> 
>    # Prepare a PRAM image
>    $ qemu-img create -f raw pram.img 256b
> 
>    # Launch QEMU with blank disk and install CDROM
>    $ ./qemu-system-m68k \
>        -M q800 \
>        -m 128 \
>        -bios Quadra800.rom \
>        -drive file=pram.img,format=raw,if=mtd \
>        -drive file=disk.img,media=disk,format=raw,if=none,id=hd \
>        -device scsi-hd,scsi-id=0,drive=hd \
>        -drive file=cdrom.iso,media=cdrom,if=none,id=cd \
>        -device scsi-cd,scsi-id=3,drive=cd
> 
> And off you go! For more in-depth information about the installation process
> I highly recommend the installation guide over at emaculation.com [1].
> Compatibility is generally very good, and I'm pleased to report it is possible
> to run one of the most popular productivity apps from the 90s [2].

Could you add an Avocado test for this machine? See how the
MipsFuloong2e test (tests/avocado/machine_mips_fuloong2e.py)
handles the firmware (RESCUE_YL_PATH).

Re: [PATCH v4 08/24] q800-glue.c: switch TypeInfo registration to use DEFINE_TYPES() macro

[Philippe Mathieu-Daudé]

On 21/6/23 10:53, Mark Cave-Ayland wrote:
> The use of the DEFINE_TYPES() macro will soon be recommended over the use of
> calling type_init() directly.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   hw/m68k/q800-glue.c | 29 +++++++++++++----------------
>   1 file changed, 13 insertions(+), 16 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v4 07/24] q800: move GLUE device into separate q800-glue.c file

[Philippe Mathieu-Daudé]

On 21/6/23 10:53, Mark Cave-Ayland wrote:
> This will allow the q800-glue.h header to be included separately so that the
> GLUE device can be referenced externally.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> ---
>   MAINTAINERS                 |   2 +
>   hw/m68k/meson.build         |   2 +-
>   hw/m68k/q800-glue.c         | 252 ++++++++++++++++++++++++++++++++++++
>   hw/m68k/q800.c              | 238 +---------------------------------
>   include/hw/m68k/q800-glue.h |  50 +++++++
>   5 files changed, 306 insertions(+), 238 deletions(-)
>   create mode 100644 hw/m68k/q800-glue.c
>   create mode 100644 include/hw/m68k/q800-glue.h


> diff --git a/hw/m68k/q800-glue.c b/hw/m68k/q800-glue.c
> new file mode 100644
> index 0000000000..e81f9438f1
> --- /dev/null
> +++ b/hw/m68k/q800-glue.c
> @@ -0,0 +1,252 @@
> +/*
> + * QEMU q800 logic GLUE (General Logic Unit)
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "cpu.h"
> +#include "hw/m68k/q800-glue.h"
> +#include "hw/boards.h"

"hw/boards.h" shouldn't be necessary here.

> +#include "hw/irq.h"
> +#include "hw/nmi.h"
> +#include "hw/qdev-properties.h"
> +#include "migration/vmstate.h"


> diff --git a/include/hw/m68k/q800-glue.h b/include/hw/m68k/q800-glue.h
> new file mode 100644
> index 0000000000..c1817b01a5
> --- /dev/null
> +++ b/include/hw/m68k/q800-glue.h
> @@ -0,0 +1,50 @@
> +/*
> + * QEMU q800 logic glue

"QEMU q800 logic GLUE (General Logic Unit)" similar to .c?
(could be changed when applying, no need to respin)

Otherwise,
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */

Re: [PATCH v4 03/24] q800: introduce Q800MachineState

[BALATON Zoltan]

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

On Wed, 21 Jun 2023, Mark Cave-Ayland wrote:
> This provides an overall container and owner for Machine-related objects such
> as MemoryRegions.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> MAINTAINERS            |  1 +
> hw/m68k/q800.c         |  2 ++
> include/hw/m68k/q800.h | 40 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 43 insertions(+)
> create mode 100644 include/hw/m68k/q800.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 88b5a7ee0a..748a66fbaa 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1236,6 +1236,7 @@ F: include/hw/misc/mac_via.h
> F: include/hw/nubus/*
> F: include/hw/display/macfb.h
> F: include/hw/block/swim.h
> +F: include/hw/m68k/q800.h
>
> virt
> M: Laurent Vivier <laurent@vivier.eu>
> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
> index 465c510c18..c0256c8a90 100644
> --- a/hw/m68k/q800.c
> +++ b/hw/m68k/q800.c
> @@ -38,6 +38,7 @@
> #include "standard-headers/asm-m68k/bootinfo.h"
> #include "standard-headers/asm-m68k/bootinfo-mac.h"
> #include "bootinfo.h"
> +#include "hw/m68k/q800.h"
> #include "hw/misc/mac_via.h"
> #include "hw/input/adb.h"
> #include "hw/nubus/mac-nubus-bridge.h"
> @@ -749,6 +750,7 @@ static void q800_machine_class_init(ObjectClass *oc, void *data)
> static const TypeInfo q800_machine_typeinfo = {
>     .name       = MACHINE_TYPE_NAME("q800"),
>     .parent     = TYPE_MACHINE,
> +    .instance_size = sizeof(Q800MachineState),
>     .class_init = q800_machine_class_init,
> };
>
> diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
> new file mode 100644
> index 0000000000..f3bc17aa1b
> --- /dev/null
> +++ b/include/hw/m68k/q800.h

Why is this defined in a public header? Moving struct definitions of 
devices to allow them to be embedded in other structs makes sense but is 
there ever a reason to embed a machine state anywhere else than using it 
in q800.c? I don't think so, thus to preserve locality and save some lines 
in this series I think this machine state should just be in q800.c like I 
have similar struct in pegasos2.c. It may only make sense to put it in a 
header if q800.c was split up to multiple files but even then it should be 
a local header in hw/m68k and not a public header in my opinion.

Regards,
BALATON Zoltan

> @@ -0,0 +1,40 @@
> +/*
> + * QEMU Motorla 680x0 Macintosh hardware System Emulator
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#ifndef HW_Q800_H
> +#define HW_Q800_H
> +
> +#include "hw/boards.h"
> +#include "qom/object.h"
> +
> +/*
> + * The main Q800 machine
> + */
> +
> +struct Q800MachineState {
> +    MachineState parent_obj;
> +};
> +
> +#define TYPE_Q800_MACHINE MACHINE_TYPE_NAME("q800")
> +OBJECT_DECLARE_SIMPLE_TYPE(Q800MachineState, Q800_MACHINE)
> +
> +#endif
>

Re: [PATCH v4 02/24] q800: add missing space after parent object in GLUEState

[BALATON Zoltan]

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

On Wed, 21 Jun 2023, Mark Cave-Ayland wrote:
> This brings GLUEState in line with our current QOM guidelines.

Are these guidelines documented somewhere? I like this better than the 
public/private comments (although I prefer no space at all with just 
documenting that QOM object parents should not be accessed directly) but I 
haven't seen it discussed and agreed upon so it looks like a convention 
you defined but not documented anywhere. But it could be I missed the 
patch to coding style or QOM docs to establish this convention.

If we really want to make these QOM object states stand out we might even 
consider formatting these as

struct GLUEState { SysBusDevice parent_obj;
     M68kCPU *cpu;
     ...
}

unless checkpatch would not like that or something.

Regards,
BALATON Zoltan

> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> ---
> hw/m68k/q800.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
> index dda57c60bf..465c510c18 100644
> --- a/hw/m68k/q800.c
> +++ b/hw/m68k/q800.c
> @@ -100,6 +100,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(GLUEState, GLUE)
>
> struct GLUEState {
>     SysBusDevice parent_obj;
> +
>     M68kCPU *cpu;
>     uint8_t ipr;
>     uint8_t auxmode;
>

Re: [PATCH v4 05/24] q800: move CPU object into Q800MachineState

[BALATON Zoltan]

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

On Wed, 21 Jun 2023, Mark Cave-Ayland wrote:
> Also change the instantiation of the CPU to use object_initialize_child()
> followed by a separate realisation.

Also seems to restrict valid CPU types but not mentioned in commit 
message. Should this patch be split up?

Regards,
BALATON Zoltan

> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/m68k/q800.c         | 18 +++++++++++++-----
> include/hw/m68k/q800.h |  3 +++
> 2 files changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
> index 062a3c6c76..2b651de3c1 100644
> --- a/hw/m68k/q800.c
> +++ b/hw/m68k/q800.c
> @@ -364,7 +364,7 @@ static uint8_t fake_mac_rom[] = {
>
> static void q800_machine_init(MachineState *machine)
> {
> -    M68kCPU *cpu = NULL;
> +    Q800MachineState *m = Q800_MACHINE(machine);
>     int linux_boot;
>     int32_t kernel_size;
>     uint64_t elf_entry;
> @@ -407,8 +407,9 @@ static void q800_machine_init(MachineState *machine)
>     }
>
>     /* init CPUs */
> -    cpu = M68K_CPU(cpu_create(machine->cpu_type));
> -    qemu_register_reset(main_cpu_reset, cpu);
> +    object_initialize_child(OBJECT(machine), "cpu", &m->cpu, machine->cpu_type);
> +    qdev_realize(DEVICE(&m->cpu), NULL, &error_fatal);
> +    qemu_register_reset(main_cpu_reset, &m->cpu);
>
>     /* RAM */
>     memory_region_add_subregion(get_system_memory(), 0, machine->ram);
> @@ -430,7 +431,8 @@ static void q800_machine_init(MachineState *machine)
>
>     /* IRQ Glue */
>     glue = qdev_new(TYPE_GLUE);
> -    object_property_set_link(OBJECT(glue), "cpu", OBJECT(cpu), &error_abort);
> +    object_property_set_link(OBJECT(glue), "cpu", OBJECT(&m->cpu),
> +                             &error_abort);
>     sysbus_realize_and_unref(SYS_BUS_DEVICE(glue), &error_fatal);
>
>     /* VIA 1 */
> @@ -605,7 +607,7 @@ static void q800_machine_init(MachineState *machine)
>
>     macfb_mode = (NUBUS_MACFB(dev)->macfb).mode;
>
> -    cs = CPU(cpu);
> +    cs = CPU(&m->cpu);
>     if (linux_boot) {
>         uint64_t high;
>         void *param_blob, *param_ptr, *param_rng_seed;
> @@ -735,6 +737,11 @@ static GlobalProperty hw_compat_q800[] = {
> };
> static const size_t hw_compat_q800_len = G_N_ELEMENTS(hw_compat_q800);
>
> +static const char *q800_machine_valid_cpu_types[] = {
> +    M68K_CPU_TYPE_NAME("m68040"),
> +    NULL
> +};
> +
> static void q800_machine_class_init(ObjectClass *oc, void *data)
> {
>     MachineClass *mc = MACHINE_CLASS(oc);
> @@ -742,6 +749,7 @@ static void q800_machine_class_init(ObjectClass *oc, void *data)
>     mc->desc = "Macintosh Quadra 800";
>     mc->init = q800_machine_init;
>     mc->default_cpu_type = M68K_CPU_TYPE_NAME("m68040");
> +    mc->valid_cpu_types = q800_machine_valid_cpu_types;
>     mc->max_cpus = 1;
>     mc->block_default_type = IF_SCSI;
>     mc->default_ram_id = "m68k_mac.ram";
> diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
> index f3bc17aa1b..4cb1a51dfe 100644
> --- a/include/hw/m68k/q800.h
> +++ b/include/hw/m68k/q800.h
> @@ -25,6 +25,7 @@
>
> #include "hw/boards.h"
> #include "qom/object.h"
> +#include "target/m68k/cpu-qom.h"
>
> /*
>  * The main Q800 machine
> @@ -32,6 +33,8 @@
>
> struct Q800MachineState {
>     MachineState parent_obj;
> +
> +    M68kCPU cpu;
> };
>
> #define TYPE_Q800_MACHINE MACHINE_TYPE_NAME("q800")
>

Re: [PATCH v4 07/24] q800: move GLUE device into separate q800-glue.c file

[BALATON Zoltan]

On Wed, 21 Jun 2023, Mark Cave-Ayland wrote:
> This will allow the q800-glue.h header to be included separately so that the
> GLUE device can be referenced externally.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> ---
> MAINTAINERS                 |   2 +
> hw/m68k/meson.build         |   2 +-
> hw/m68k/q800-glue.c         | 252 ++++++++++++++++++++++++++++++++++++
> hw/m68k/q800.c              | 238 +---------------------------------
> include/hw/m68k/q800-glue.h |  50 +++++++
> 5 files changed, 306 insertions(+), 238 deletions(-)
> create mode 100644 hw/m68k/q800-glue.c
> create mode 100644 include/hw/m68k/q800-glue.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 748a66fbaa..7f323cd2eb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1225,6 +1225,7 @@ q800
> M: Laurent Vivier <laurent@vivier.eu>
> S: Maintained
> F: hw/m68k/q800.c
> +F: hw/m68k/q800-glue.c
> F: hw/misc/mac_via.c
> F: hw/nubus/*
> F: hw/display/macfb.c
> @@ -1237,6 +1238,7 @@ F: include/hw/nubus/*
> F: include/hw/display/macfb.h
> F: include/hw/block/swim.h
> F: include/hw/m68k/q800.h
> +F: include/hw/m68k/q800-glue.h
>
> virt
> M: Laurent Vivier <laurent@vivier.eu>
> diff --git a/hw/m68k/meson.build b/hw/m68k/meson.build
> index 31248641d3..84bc68fa4e 100644
> --- a/hw/m68k/meson.build
> +++ b/hw/m68k/meson.build
> @@ -2,7 +2,7 @@ m68k_ss = ss.source_set()
> m68k_ss.add(when: 'CONFIG_AN5206', if_true: files('an5206.c', 'mcf5206.c'))
> m68k_ss.add(when: 'CONFIG_MCF5208', if_true: files('mcf5208.c', 'mcf_intc.c'))
> m68k_ss.add(when: 'CONFIG_NEXTCUBE', if_true: files('next-kbd.c', 'next-cube.c'))
> -m68k_ss.add(when: 'CONFIG_Q800', if_true: files('q800.c'))
> +m68k_ss.add(when: 'CONFIG_Q800', if_true: files('q800.c', 'q800-glue.c'))
> m68k_ss.add(when: 'CONFIG_M68K_VIRT', if_true: files('virt.c'))
>
> hw_arch += {'m68k': m68k_ss}
> diff --git a/hw/m68k/q800-glue.c b/hw/m68k/q800-glue.c
> new file mode 100644
> index 0000000000..e81f9438f1
> --- /dev/null
> +++ b/hw/m68k/q800-glue.c
> @@ -0,0 +1,252 @@
> +/*
> + * QEMU q800 logic GLUE (General Logic Unit)

Maybe clearer:

q800 General Logic Unit (aka GLUE)

Sorry for coming late in this review but I just had some time to look more 
closely at these.

Regards,
BALATON Zoltan

> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "cpu.h"
> +#include "hw/m68k/q800-glue.h"
> +#include "hw/boards.h"
> +#include "hw/irq.h"
> +#include "hw/nmi.h"
> +#include "hw/qdev-properties.h"
> +#include "migration/vmstate.h"
> +
> +/*
> + * The GLUE (General Logic Unit) is an Apple custom integrated circuit chip
> + * that performs a variety of functions (RAM management, clock generation, ...).
> + * The GLUE chip receives interrupt requests from various devices,
> + * assign priority to each, and asserts one or more interrupt line to the
> + * CPU.
> + */
> +
> +/*
> + * The GLUE logic on the Quadra 800 supports 2 different IRQ routing modes
> + * controlled from the VIA1 auxmode GPIO (port B bit 6) which are documented
> + * in NetBSD as follows:
> + *
> + * A/UX mode (Linux, NetBSD, auxmode GPIO low)
> + *
> + *   Level 0:        Spurious: ignored
> + *   Level 1:        Software
> + *   Level 2:        VIA2 (except ethernet, sound)
> + *   Level 3:        Ethernet
> + *   Level 4:        Serial (SCC)
> + *   Level 5:        Sound
> + *   Level 6:        VIA1
> + *   Level 7:        NMIs: parity errors, RESET button, YANCC error
> + *
> + * Classic mode (default: used by MacOS, A/UX 3.0.1, auxmode GPIO high)
> + *
> + *   Level 0:        Spurious: ignored
> + *   Level 1:        VIA1 (clock, ADB)
> + *   Level 2:        VIA2 (NuBus, SCSI)
> + *   Level 3:
> + *   Level 4:        Serial (SCC)
> + *   Level 5:
> + *   Level 6:
> + *   Level 7:        Non-maskable: parity errors, RESET button
> + *
> + * Note that despite references to A/UX mode in Linux and NetBSD, at least
> + * A/UX 3.0.1 still uses Classic mode.
> + */
> +
> +static void GLUE_set_irq(void *opaque, int irq, int level)
> +{
> +    GLUEState *s = opaque;
> +    int i;
> +
> +    if (s->auxmode) {
> +        /* Classic mode */
> +        switch (irq) {
> +        case GLUE_IRQ_IN_VIA1:
> +            irq = 0;
> +            break;
> +
> +        case GLUE_IRQ_IN_VIA2:
> +            irq = 1;
> +            break;
> +
> +        case GLUE_IRQ_IN_SONIC:
> +            /* Route to VIA2 instead */
> +            qemu_set_irq(s->irqs[GLUE_IRQ_NUBUS_9], level);
> +            return;
> +
> +        case GLUE_IRQ_IN_ESCC:
> +            irq = 3;
> +            break;
> +
> +        case GLUE_IRQ_IN_NMI:
> +            irq = 6;
> +            break;
> +
> +        default:
> +            g_assert_not_reached();
> +        }
> +    } else {
> +        /* A/UX mode */
> +        switch (irq) {
> +        case GLUE_IRQ_IN_VIA1:
> +            irq = 5;
> +            break;
> +
> +        case GLUE_IRQ_IN_VIA2:
> +            irq = 1;
> +            break;
> +
> +        case GLUE_IRQ_IN_SONIC:
> +            irq = 2;
> +            break;
> +
> +        case GLUE_IRQ_IN_ESCC:
> +            irq = 3;
> +            break;
> +
> +        case GLUE_IRQ_IN_NMI:
> +            irq = 6;
> +            break;
> +
> +        default:
> +            g_assert_not_reached();
> +        }
> +    }
> +
> +    if (level) {
> +        s->ipr |= 1 << irq;
> +    } else {
> +        s->ipr &= ~(1 << irq);
> +    }
> +
> +    for (i = 7; i >= 0; i--) {
> +        if ((s->ipr >> i) & 1) {
> +            m68k_set_irq_level(s->cpu, i + 1, i + 25);
> +            return;
> +        }
> +    }
> +    m68k_set_irq_level(s->cpu, 0, 0);
> +}
> +
> +static void glue_auxmode_set_irq(void *opaque, int irq, int level)
> +{
> +    GLUEState *s = GLUE(opaque);
> +
> +    s->auxmode = level;
> +}
> +
> +static void glue_nmi(NMIState *n, int cpu_index, Error **errp)
> +{
> +    GLUEState *s = GLUE(n);
> +
> +    /* Hold NMI active for 100ms */
> +    GLUE_set_irq(s, GLUE_IRQ_IN_NMI, 1);
> +    timer_mod(s->nmi_release, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 100);
> +}
> +
> +static void glue_nmi_release(void *opaque)
> +{
> +    GLUEState *s = GLUE(opaque);
> +
> +    GLUE_set_irq(s, GLUE_IRQ_IN_NMI, 0);
> +}
> +
> +static void glue_reset(DeviceState *dev)
> +{
> +    GLUEState *s = GLUE(dev);
> +
> +    s->ipr = 0;
> +    s->auxmode = 0;
> +
> +    timer_del(s->nmi_release);
> +}
> +
> +static const VMStateDescription vmstate_glue = {
> +    .name = "q800-glue",
> +    .version_id = 0,
> +    .minimum_version_id = 0,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT8(ipr, GLUEState),
> +        VMSTATE_UINT8(auxmode, GLUEState),
> +        VMSTATE_TIMER_PTR(nmi_release, GLUEState),
> +        VMSTATE_END_OF_LIST(),
> +    },
> +};
> +
> +/*
> + * If the m68k CPU implemented its inbound irq lines as GPIO lines
> + * rather than via the m68k_set_irq_level() function we would not need
> + * this cpu link property and could instead provide outbound IRQ lines
> + * that the board could wire up to the CPU.
> + */
> +static Property glue_properties[] = {
> +    DEFINE_PROP_LINK("cpu", GLUEState, cpu, TYPE_M68K_CPU, M68kCPU *),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void glue_finalize(Object *obj)
> +{
> +    GLUEState *s = GLUE(obj);
> +
> +    timer_free(s->nmi_release);
> +}
> +
> +static void glue_init(Object *obj)
> +{
> +    DeviceState *dev = DEVICE(obj);
> +    GLUEState *s = GLUE(dev);
> +
> +    qdev_init_gpio_in(dev, GLUE_set_irq, 8);
> +    qdev_init_gpio_in_named(dev, glue_auxmode_set_irq, "auxmode", 1);
> +
> +    qdev_init_gpio_out(dev, s->irqs, 1);
> +
> +    /* NMI release timer */
> +    s->nmi_release = timer_new_ms(QEMU_CLOCK_VIRTUAL, glue_nmi_release, s);
> +}
> +
> +static void glue_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    NMIClass *nc = NMI_CLASS(klass);
> +
> +    dc->vmsd = &vmstate_glue;
> +    dc->reset = glue_reset;
> +    device_class_set_props(dc, glue_properties);
> +    nc->nmi_monitor_handler = glue_nmi;
> +}
> +
> +static const TypeInfo glue_info = {
> +    .name = TYPE_GLUE,
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(GLUEState),
> +    .instance_init = glue_init,
> +    .instance_finalize = glue_finalize,
> +    .class_init = glue_class_init,
> +    .interfaces = (InterfaceInfo[]) {
> +         { TYPE_NMI },
> +         { }
> +    },
> +};
> +
> +static void glue_register_types(void)
> +{
> +    type_register_static(&glue_info);
> +}
> +
> +type_init(glue_register_types)
> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
> index 9f9668c2b4..9f9de2ebaf 100644
> --- a/hw/m68k/q800.c
> +++ b/hw/m68k/q800.c
> @@ -28,7 +28,6 @@
> #include "cpu.h"
> #include "hw/boards.h"
> #include "hw/or-irq.h"
> -#include "hw/nmi.h"
> #include "elf.h"
> #include "hw/loader.h"
> #include "ui/console.h"
> @@ -39,6 +38,7 @@
> #include "standard-headers/asm-m68k/bootinfo-mac.h"
> #include "bootinfo.h"
> #include "hw/m68k/q800.h"
> +#include "hw/m68k/q800-glue.h"
> #include "hw/misc/mac_via.h"
> #include "hw/input/adb.h"
> #include "hw/nubus/mac-nubus-bridge.h"
> @@ -88,241 +88,6 @@
> #define Q800_NUBUS_SLOTS_AVAILABLE    (BIT(0x9) | BIT(0xc) | BIT(0xd) | \
>                                        BIT(0xe))
>
> -/*
> - * The GLUE (General Logic Unit) is an Apple custom integrated circuit chip
> - * that performs a variety of functions (RAM management, clock generation, ...).
> - * The GLUE chip receives interrupt requests from various devices,
> - * assign priority to each, and asserts one or more interrupt line to the
> - * CPU.
> - */
> -
> -#define TYPE_GLUE "q800-glue"
> -OBJECT_DECLARE_SIMPLE_TYPE(GLUEState, GLUE)
> -
> -struct GLUEState {
> -    SysBusDevice parent_obj;
> -
> -    M68kCPU *cpu;
> -    uint8_t ipr;
> -    uint8_t auxmode;
> -    qemu_irq irqs[1];
> -    QEMUTimer *nmi_release;
> -};
> -
> -#define GLUE_IRQ_IN_VIA1       0
> -#define GLUE_IRQ_IN_VIA2       1
> -#define GLUE_IRQ_IN_SONIC      2
> -#define GLUE_IRQ_IN_ESCC       3
> -#define GLUE_IRQ_IN_NMI        4
> -
> -#define GLUE_IRQ_NUBUS_9       0
> -
> -/*
> - * The GLUE logic on the Quadra 800 supports 2 different IRQ routing modes
> - * controlled from the VIA1 auxmode GPIO (port B bit 6) which are documented
> - * in NetBSD as follows:
> - *
> - * A/UX mode (Linux, NetBSD, auxmode GPIO low)
> - *
> - *   Level 0:        Spurious: ignored
> - *   Level 1:        Software
> - *   Level 2:        VIA2 (except ethernet, sound)
> - *   Level 3:        Ethernet
> - *   Level 4:        Serial (SCC)
> - *   Level 5:        Sound
> - *   Level 6:        VIA1
> - *   Level 7:        NMIs: parity errors, RESET button, YANCC error
> - *
> - * Classic mode (default: used by MacOS, A/UX 3.0.1, auxmode GPIO high)
> - *
> - *   Level 0:        Spurious: ignored
> - *   Level 1:        VIA1 (clock, ADB)
> - *   Level 2:        VIA2 (NuBus, SCSI)
> - *   Level 3:
> - *   Level 4:        Serial (SCC)
> - *   Level 5:
> - *   Level 6:
> - *   Level 7:        Non-maskable: parity errors, RESET button
> - *
> - * Note that despite references to A/UX mode in Linux and NetBSD, at least
> - * A/UX 3.0.1 still uses Classic mode.
> - */
> -
> -static void GLUE_set_irq(void *opaque, int irq, int level)
> -{
> -    GLUEState *s = opaque;
> -    int i;
> -
> -    if (s->auxmode) {
> -        /* Classic mode */
> -        switch (irq) {
> -        case GLUE_IRQ_IN_VIA1:
> -            irq = 0;
> -            break;
> -
> -        case GLUE_IRQ_IN_VIA2:
> -            irq = 1;
> -            break;
> -
> -        case GLUE_IRQ_IN_SONIC:
> -            /* Route to VIA2 instead */
> -            qemu_set_irq(s->irqs[GLUE_IRQ_NUBUS_9], level);
> -            return;
> -
> -        case GLUE_IRQ_IN_ESCC:
> -            irq = 3;
> -            break;
> -
> -        case GLUE_IRQ_IN_NMI:
> -            irq = 6;
> -            break;
> -
> -        default:
> -            g_assert_not_reached();
> -        }
> -    } else {
> -        /* A/UX mode */
> -        switch (irq) {
> -        case GLUE_IRQ_IN_VIA1:
> -            irq = 5;
> -            break;
> -
> -        case GLUE_IRQ_IN_VIA2:
> -            irq = 1;
> -            break;
> -
> -        case GLUE_IRQ_IN_SONIC:
> -            irq = 2;
> -            break;
> -
> -        case GLUE_IRQ_IN_ESCC:
> -            irq = 3;
> -            break;
> -
> -        case GLUE_IRQ_IN_NMI:
> -            irq = 6;
> -            break;
> -
> -        default:
> -            g_assert_not_reached();
> -        }
> -    }
> -
> -    if (level) {
> -        s->ipr |= 1 << irq;
> -    } else {
> -        s->ipr &= ~(1 << irq);
> -    }
> -
> -    for (i = 7; i >= 0; i--) {
> -        if ((s->ipr >> i) & 1) {
> -            m68k_set_irq_level(s->cpu, i + 1, i + 25);
> -            return;
> -        }
> -    }
> -    m68k_set_irq_level(s->cpu, 0, 0);
> -}
> -
> -static void glue_auxmode_set_irq(void *opaque, int irq, int level)
> -{
> -    GLUEState *s = GLUE(opaque);
> -
> -    s->auxmode = level;
> -}
> -
> -static void glue_nmi(NMIState *n, int cpu_index, Error **errp)
> -{
> -    GLUEState *s = GLUE(n);
> -
> -    /* Hold NMI active for 100ms */
> -    GLUE_set_irq(s, GLUE_IRQ_IN_NMI, 1);
> -    timer_mod(s->nmi_release, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 100);
> -}
> -
> -static void glue_nmi_release(void *opaque)
> -{
> -    GLUEState *s = GLUE(opaque);
> -
> -    GLUE_set_irq(s, GLUE_IRQ_IN_NMI, 0);
> -}
> -
> -static void glue_reset(DeviceState *dev)
> -{
> -    GLUEState *s = GLUE(dev);
> -
> -    s->ipr = 0;
> -    s->auxmode = 0;
> -
> -    timer_del(s->nmi_release);
> -}
> -
> -static const VMStateDescription vmstate_glue = {
> -    .name = "q800-glue",
> -    .version_id = 0,
> -    .minimum_version_id = 0,
> -    .fields = (VMStateField[]) {
> -        VMSTATE_UINT8(ipr, GLUEState),
> -        VMSTATE_UINT8(auxmode, GLUEState),
> -        VMSTATE_TIMER_PTR(nmi_release, GLUEState),
> -        VMSTATE_END_OF_LIST(),
> -    },
> -};
> -
> -/*
> - * If the m68k CPU implemented its inbound irq lines as GPIO lines
> - * rather than via the m68k_set_irq_level() function we would not need
> - * this cpu link property and could instead provide outbound IRQ lines
> - * that the board could wire up to the CPU.
> - */
> -static Property glue_properties[] = {
> -    DEFINE_PROP_LINK("cpu", GLUEState, cpu, TYPE_M68K_CPU, M68kCPU *),
> -    DEFINE_PROP_END_OF_LIST(),
> -};
> -
> -static void glue_finalize(Object *obj)
> -{
> -    GLUEState *s = GLUE(obj);
> -
> -    timer_free(s->nmi_release);
> -}
> -
> -static void glue_init(Object *obj)
> -{
> -    DeviceState *dev = DEVICE(obj);
> -    GLUEState *s = GLUE(dev);
> -
> -    qdev_init_gpio_in(dev, GLUE_set_irq, 8);
> -    qdev_init_gpio_in_named(dev, glue_auxmode_set_irq, "auxmode", 1);
> -
> -    qdev_init_gpio_out(dev, s->irqs, 1);
> -
> -    /* NMI release timer */
> -    s->nmi_release = timer_new_ms(QEMU_CLOCK_VIRTUAL, glue_nmi_release, s);
> -}
> -
> -static void glue_class_init(ObjectClass *klass, void *data)
> -{
> -    DeviceClass *dc = DEVICE_CLASS(klass);
> -    NMIClass *nc = NMI_CLASS(klass);
> -
> -    dc->vmsd = &vmstate_glue;
> -    dc->reset = glue_reset;
> -    device_class_set_props(dc, glue_properties);
> -    nc->nmi_monitor_handler = glue_nmi;
> -}
> -
> -static const TypeInfo glue_info = {
> -    .name = TYPE_GLUE,
> -    .parent = TYPE_SYS_BUS_DEVICE,
> -    .instance_size = sizeof(GLUEState),
> -    .instance_init = glue_init,
> -    .instance_finalize = glue_finalize,
> -    .class_init = glue_class_init,
> -    .interfaces = (InterfaceInfo[]) {
> -         { TYPE_NMI },
> -         { }
> -    },
> -};
>
> static void main_cpu_reset(void *opaque)
> {
> @@ -763,7 +528,6 @@ static const TypeInfo q800_machine_typeinfo = {
> static void q800_machine_register_types(void)
> {
>     type_register_static(&q800_machine_typeinfo);
> -    type_register_static(&glue_info);
> }
>
> type_init(q800_machine_register_types)
> diff --git a/include/hw/m68k/q800-glue.h b/include/hw/m68k/q800-glue.h
> new file mode 100644
> index 0000000000..c1817b01a5
> --- /dev/null
> +++ b/include/hw/m68k/q800-glue.h
> @@ -0,0 +1,50 @@
> +/*
> + * QEMU q800 logic glue
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#ifndef HW_Q800_GLUE_H
> +#define HW_Q800_GLUE_H
> +
> +#include "qemu/osdep.h"
> +#include "hw/sysbus.h"
> +
> +#define TYPE_GLUE "q800-glue"
> +OBJECT_DECLARE_SIMPLE_TYPE(GLUEState, GLUE)
> +
> +struct GLUEState {
> +    SysBusDevice parent_obj;
> +
> +    M68kCPU *cpu;
> +    uint8_t ipr;
> +    uint8_t auxmode;
> +    qemu_irq irqs[1];
> +    QEMUTimer *nmi_release;
> +};
> +
> +#define GLUE_IRQ_IN_VIA1       0
> +#define GLUE_IRQ_IN_VIA2       1
> +#define GLUE_IRQ_IN_SONIC      2
> +#define GLUE_IRQ_IN_ESCC       3
> +#define GLUE_IRQ_IN_NMI        4
> +
> +#define GLUE_IRQ_NUBUS_9       0
> +
> +#endif
>

Re: [PATCH v4 05/24] q800: move CPU object into Q800MachineState

[BALATON Zoltan]

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

On Wed, 21 Jun 2023, Mark Cave-Ayland wrote:
> Also change the instantiation of the CPU to use object_initialize_child()
> followed by a separate realisation.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/m68k/q800.c         | 18 +++++++++++++-----
> include/hw/m68k/q800.h |  3 +++
> 2 files changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
> index 062a3c6c76..2b651de3c1 100644
> --- a/hw/m68k/q800.c
> +++ b/hw/m68k/q800.c
> @@ -364,7 +364,7 @@ static uint8_t fake_mac_rom[] = {
>
> static void q800_machine_init(MachineState *machine)
> {
> -    M68kCPU *cpu = NULL;
> +    Q800MachineState *m = Q800_MACHINE(machine);
>     int linux_boot;
>     int32_t kernel_size;
>     uint64_t elf_entry;
> @@ -407,8 +407,9 @@ static void q800_machine_init(MachineState *machine)
>     }
>
>     /* init CPUs */
> -    cpu = M68K_CPU(cpu_create(machine->cpu_type));
> -    qemu_register_reset(main_cpu_reset, cpu);
> +    object_initialize_child(OBJECT(machine), "cpu", &m->cpu, machine->cpu_type);

As OBJECT(machine) will be used frequently maybe you can make this more 
readable by adding a local var for that. Also renaming sysbus local to sbd 
may improve readability somewhat.

Regards,
BALATON Zoltan

> +    qdev_realize(DEVICE(&m->cpu), NULL, &error_fatal);
> +    qemu_register_reset(main_cpu_reset, &m->cpu);
>
>     /* RAM */
>     memory_region_add_subregion(get_system_memory(), 0, machine->ram);
> @@ -430,7 +431,8 @@ static void q800_machine_init(MachineState *machine)
>
>     /* IRQ Glue */
>     glue = qdev_new(TYPE_GLUE);
> -    object_property_set_link(OBJECT(glue), "cpu", OBJECT(cpu), &error_abort);
> +    object_property_set_link(OBJECT(glue), "cpu", OBJECT(&m->cpu),
> +                             &error_abort);
>     sysbus_realize_and_unref(SYS_BUS_DEVICE(glue), &error_fatal);
>
>     /* VIA 1 */
> @@ -605,7 +607,7 @@ static void q800_machine_init(MachineState *machine)
>
>     macfb_mode = (NUBUS_MACFB(dev)->macfb).mode;
>
> -    cs = CPU(cpu);
> +    cs = CPU(&m->cpu);
>     if (linux_boot) {
>         uint64_t high;
>         void *param_blob, *param_ptr, *param_rng_seed;
> @@ -735,6 +737,11 @@ static GlobalProperty hw_compat_q800[] = {
> };
> static const size_t hw_compat_q800_len = G_N_ELEMENTS(hw_compat_q800);
>
> +static const char *q800_machine_valid_cpu_types[] = {
> +    M68K_CPU_TYPE_NAME("m68040"),
> +    NULL
> +};
> +
> static void q800_machine_class_init(ObjectClass *oc, void *data)
> {
>     MachineClass *mc = MACHINE_CLASS(oc);
> @@ -742,6 +749,7 @@ static void q800_machine_class_init(ObjectClass *oc, void *data)
>     mc->desc = "Macintosh Quadra 800";
>     mc->init = q800_machine_init;
>     mc->default_cpu_type = M68K_CPU_TYPE_NAME("m68040");
> +    mc->valid_cpu_types = q800_machine_valid_cpu_types;
>     mc->max_cpus = 1;
>     mc->block_default_type = IF_SCSI;
>     mc->default_ram_id = "m68k_mac.ram";
> diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
> index f3bc17aa1b..4cb1a51dfe 100644
> --- a/include/hw/m68k/q800.h
> +++ b/include/hw/m68k/q800.h
> @@ -25,6 +25,7 @@
>
> #include "hw/boards.h"
> #include "qom/object.h"
> +#include "target/m68k/cpu-qom.h"
>
> /*
>  * The main Q800 machine
> @@ -32,6 +33,8 @@
>
> struct Q800MachineState {
>     MachineState parent_obj;
> +
> +    M68kCPU cpu;
> };
>
> #define TYPE_Q800_MACHINE MACHINE_TYPE_NAME("q800")
>

Re: [PATCH v4 00/24] q800: add support for booting MacOS Classic - part 1

[Mark Cave-Ayland]

On 21/06/2023 10:42, Philippe Mathieu-Daudé wrote:

> On 21/6/23 10:53, Mark Cave-Ayland wrote:
>> [MCA: the original series has now been split into 2 separate parts based upon
>> Phil's comments re: QOM parenting for objects in Q800MachineState. Part 1
>> consists of the Q800MachineState patches along with QOM parenting fixes and
>> the 2 mac_via RTC patches.]
>>
>> This series contains the remaining patches needed to allow QEMU's q800
>> machine to boot MacOS Classic when used in conjunction with a real
>> Quadra 800 ROM image. In fact with this series applied it is possible
>> to boot all of the following OSs:
>>
>>    - MacOS 7.1 - 8.1, with or without virtual memory enabled
>>    - A/UX 3.0.1
>>    - NetBSD 9.3
>>    - Linux (via EMILE)
>>
>> If you are ready to experience some 90s nostalgia then all you need is
>> to grab yourself a copy of the Quadra 800 ROM (checksum 0xf1acad13) and a
>> suitable install ISO as follows:
>>
>>    # Prepare a PRAM image
>>    $ qemu-img create -f raw pram.img 256b
>>
>>    # Launch QEMU with blank disk and install CDROM
>>    $ ./qemu-system-m68k \
>>        -M q800 \
>>        -m 128 \
>>        -bios Quadra800.rom \
>>        -drive file=pram.img,format=raw,if=mtd \
>>        -drive file=disk.img,media=disk,format=raw,if=none,id=hd \
>>        -device scsi-hd,scsi-id=0,drive=hd \
>>        -drive file=cdrom.iso,media=cdrom,if=none,id=cd \
>>        -device scsi-cd,scsi-id=3,drive=cd
>>
>> And off you go! For more in-depth information about the installation process
>> I highly recommend the installation guide over at emaculation.com [1].
>> Compatibility is generally very good, and I'm pleased to report it is possible
>> to run one of the most popular productivity apps from the 90s [2].
> 
> Could you add an Avocado test for this machine? See how the
> MipsFuloong2e test (tests/avocado/machine_mips_fuloong2e.py)
> handles the firmware (RESCUE_YL_PATH).

In theory it is possible to do this, but how do we handle booting proprietary OS ISOs 
that are still within copyright? Also this is only part 1 of the required changes, 
you'll need part 2 applied in order to achieve a successful boot :)


ATB,

Mark.

Re: [PATCH v4 07/24] q800: move GLUE device into separate q800-glue.c file

[Mark Cave-Ayland]

On 21/06/2023 10:46, Philippe Mathieu-Daudé wrote:

> On 21/6/23 10:53, Mark Cave-Ayland wrote:
>> This will allow the q800-glue.h header to be included separately so that the
>> GLUE device can be referenced externally.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>> ---
>>   MAINTAINERS                 |   2 +
>>   hw/m68k/meson.build         |   2 +-
>>   hw/m68k/q800-glue.c         | 252 ++++++++++++++++++++++++++++++++++++
>>   hw/m68k/q800.c              | 238 +---------------------------------
>>   include/hw/m68k/q800-glue.h |  50 +++++++
>>   5 files changed, 306 insertions(+), 238 deletions(-)
>>   create mode 100644 hw/m68k/q800-glue.c
>>   create mode 100644 include/hw/m68k/q800-glue.h
> 
> 
>> diff --git a/hw/m68k/q800-glue.c b/hw/m68k/q800-glue.c
>> new file mode 100644
>> index 0000000000..e81f9438f1
>> --- /dev/null
>> +++ b/hw/m68k/q800-glue.c
>> @@ -0,0 +1,252 @@
>> +/*
>> + * QEMU q800 logic GLUE (General Logic Unit)
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a copy
>> + * of this software and associated documentation files (the "Software"), to deal
>> + * in the Software without restriction, including without limitation the rights
>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>> + * copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>> + * THE SOFTWARE.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "cpu.h"
>> +#include "hw/m68k/q800-glue.h"
>> +#include "hw/boards.h"
> 
> "hw/boards.h" shouldn't be necessary here.

Okay.

>> +#include "hw/irq.h"
>> +#include "hw/nmi.h"
>> +#include "hw/qdev-properties.h"
>> +#include "migration/vmstate.h"
> 
> 
>> diff --git a/include/hw/m68k/q800-glue.h b/include/hw/m68k/q800-glue.h
>> new file mode 100644
>> index 0000000000..c1817b01a5
>> --- /dev/null
>> +++ b/include/hw/m68k/q800-glue.h
>> @@ -0,0 +1,50 @@
>> +/*
>> + * QEMU q800 logic glue
> 
> "QEMU q800 logic GLUE (General Logic Unit)" similar to .c?
> (could be changed when applying, no need to respin)

Ah so I updated it in the .c file but forgot to do the same for the .h file.

Laurent, are you able to squash this or would you like me to send a v5?

> Otherwise,
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Thanks!


ATB,

Mark.

Re: [PATCH v4 03/24] q800: introduce Q800MachineState

[Mark Cave-Ayland]

On 21/06/2023 12:33, BALATON Zoltan wrote:

> On Wed, 21 Jun 2023, Mark Cave-Ayland wrote:
>> This provides an overall container and owner for Machine-related objects such
>> as MemoryRegions.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> MAINTAINERS            |  1 +
>> hw/m68k/q800.c         |  2 ++
>> include/hw/m68k/q800.h | 40 ++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 43 insertions(+)
>> create mode 100644 include/hw/m68k/q800.h
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 88b5a7ee0a..748a66fbaa 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -1236,6 +1236,7 @@ F: include/hw/misc/mac_via.h
>> F: include/hw/nubus/*
>> F: include/hw/display/macfb.h
>> F: include/hw/block/swim.h
>> +F: include/hw/m68k/q800.h
>>
>> virt
>> M: Laurent Vivier <laurent@vivier.eu>
>> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
>> index 465c510c18..c0256c8a90 100644
>> --- a/hw/m68k/q800.c
>> +++ b/hw/m68k/q800.c
>> @@ -38,6 +38,7 @@
>> #include "standard-headers/asm-m68k/bootinfo.h"
>> #include "standard-headers/asm-m68k/bootinfo-mac.h"
>> #include "bootinfo.h"
>> +#include "hw/m68k/q800.h"
>> #include "hw/misc/mac_via.h"
>> #include "hw/input/adb.h"
>> #include "hw/nubus/mac-nubus-bridge.h"
>> @@ -749,6 +750,7 @@ static void q800_machine_class_init(ObjectClass *oc, void *data)
>> static const TypeInfo q800_machine_typeinfo = {
>>     .name       = MACHINE_TYPE_NAME("q800"),
>>     .parent     = TYPE_MACHINE,
>> +    .instance_size = sizeof(Q800MachineState),
>>     .class_init = q800_machine_class_init,
>> };
>>
>> diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
>> new file mode 100644
>> index 0000000000..f3bc17aa1b
>> --- /dev/null
>> +++ b/include/hw/m68k/q800.h
> 
> Why is this defined in a public header? Moving struct definitions of devices to allow 
> them to be embedded in other structs makes sense but is there ever a reason to embed 
> a machine state anywhere else than using it in q800.c? I don't think so, thus to 
> preserve locality and save some lines in this series I think this machine state 
> should just be in q800.c like I have similar struct in pegasos2.c. It may only make 
> sense to put it in a header if q800.c was split up to multiple files but even then it 
> should be a local header in hw/m68k and not a public header in my opinion.

This is just following our standard guidelines since MachineState is a QOM object of 
TYPE_MACHINE. Note that there are also a number of existing examples of this 
currently within the QEMU tree.


ATB,

Mark.

Re: [PATCH v4 02/24] q800: add missing space after parent object in GLUEState

[Mark Cave-Ayland]

On 21/06/2023 12:41, BALATON Zoltan wrote:

> On Wed, 21 Jun 2023, Mark Cave-Ayland wrote:
>> This brings GLUEState in line with our current QOM guidelines.
> 
> Are these guidelines documented somewhere? I like this better than the public/private 
> comments (although I prefer no space at all with just documenting that QOM object 
> parents should not be accessed directly) but I haven't seen it discussed and agreed 
> upon so it looks like a convention you defined but not documented anywhere. But it 
> could be I missed the patch to coding style or QOM docs to establish this convention.

Alex documented this earlier in the year: you can find this online at 
https://qemu.readthedocs.io/en/master/devel/style.html#qemu-specific-idioms.

> If we really want to make these QOM object states stand out we might even consider 
> formatting these as
> 
> struct GLUEState { SysBusDevice parent_obj;
>      M68kCPU *cpu;
>      ...
> }
> 
> unless checkpatch would not like that or something.

I'm not overly convinced by this, and yes I suspect it would also require some 
hacking on checkpatch.pl for it to work.


ATB,

Mark.

Re: [PATCH v4 05/24] q800: move CPU object into Q800MachineState

[Mark Cave-Ayland]

On 21/06/2023 12:56, BALATON Zoltan wrote:

> On Wed, 21 Jun 2023, Mark Cave-Ayland wrote:
>> Also change the instantiation of the CPU to use object_initialize_child()
>> followed by a separate realisation.
> 
> Also seems to restrict valid CPU types but not mentioned in commit message. Should 
> this patch be split up?

Hmmm good point. Laurent, would you like me to split this into a separate patch or 
would updating the commit message be sufficient?

>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> hw/m68k/q800.c         | 18 +++++++++++++-----
>> include/hw/m68k/q800.h |  3 +++
>> 2 files changed, 16 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
>> index 062a3c6c76..2b651de3c1 100644
>> --- a/hw/m68k/q800.c
>> +++ b/hw/m68k/q800.c
>> @@ -364,7 +364,7 @@ static uint8_t fake_mac_rom[] = {
>>
>> static void q800_machine_init(MachineState *machine)
>> {
>> -    M68kCPU *cpu = NULL;
>> +    Q800MachineState *m = Q800_MACHINE(machine);
>>     int linux_boot;
>>     int32_t kernel_size;
>>     uint64_t elf_entry;
>> @@ -407,8 +407,9 @@ static void q800_machine_init(MachineState *machine)
>>     }
>>
>>     /* init CPUs */
>> -    cpu = M68K_CPU(cpu_create(machine->cpu_type));
>> -    qemu_register_reset(main_cpu_reset, cpu);
>> +    object_initialize_child(OBJECT(machine), "cpu", &m->cpu, machine->cpu_type);
>> +    qdev_realize(DEVICE(&m->cpu), NULL, &error_fatal);
>> +    qemu_register_reset(main_cpu_reset, &m->cpu);
>>
>>     /* RAM */
>>     memory_region_add_subregion(get_system_memory(), 0, machine->ram);
>> @@ -430,7 +431,8 @@ static void q800_machine_init(MachineState *machine)
>>
>>     /* IRQ Glue */
>>     glue = qdev_new(TYPE_GLUE);
>> -    object_property_set_link(OBJECT(glue), "cpu", OBJECT(cpu), &error_abort);
>> +    object_property_set_link(OBJECT(glue), "cpu", OBJECT(&m->cpu),
>> +                             &error_abort);
>>     sysbus_realize_and_unref(SYS_BUS_DEVICE(glue), &error_fatal);
>>
>>     /* VIA 1 */
>> @@ -605,7 +607,7 @@ static void q800_machine_init(MachineState *machine)
>>
>>     macfb_mode = (NUBUS_MACFB(dev)->macfb).mode;
>>
>> -    cs = CPU(cpu);
>> +    cs = CPU(&m->cpu);
>>     if (linux_boot) {
>>         uint64_t high;
>>         void *param_blob, *param_ptr, *param_rng_seed;
>> @@ -735,6 +737,11 @@ static GlobalProperty hw_compat_q800[] = {
>> };
>> static const size_t hw_compat_q800_len = G_N_ELEMENTS(hw_compat_q800);
>>
>> +static const char *q800_machine_valid_cpu_types[] = {
>> +    M68K_CPU_TYPE_NAME("m68040"),
>> +    NULL
>> +};
>> +
>> static void q800_machine_class_init(ObjectClass *oc, void *data)
>> {
>>     MachineClass *mc = MACHINE_CLASS(oc);
>> @@ -742,6 +749,7 @@ static void q800_machine_class_init(ObjectClass *oc, void *data)
>>     mc->desc = "Macintosh Quadra 800";
>>     mc->init = q800_machine_init;
>>     mc->default_cpu_type = M68K_CPU_TYPE_NAME("m68040");
>> +    mc->valid_cpu_types = q800_machine_valid_cpu_types;
>>     mc->max_cpus = 1;
>>     mc->block_default_type = IF_SCSI;
>>     mc->default_ram_id = "m68k_mac.ram";
>> diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
>> index f3bc17aa1b..4cb1a51dfe 100644
>> --- a/include/hw/m68k/q800.h
>> +++ b/include/hw/m68k/q800.h
>> @@ -25,6 +25,7 @@
>>
>> #include "hw/boards.h"
>> #include "qom/object.h"
>> +#include "target/m68k/cpu-qom.h"
>>
>> /*
>>  * The main Q800 machine
>> @@ -32,6 +33,8 @@
>>
>> struct Q800MachineState {
>>     MachineState parent_obj;
>> +
>> +    M68kCPU cpu;
>> };
>>
>> #define TYPE_Q800_MACHINE MACHINE_TYPE_NAME("q800")

ATB,

Mark.

Re: [PATCH v4 07/24] q800: move GLUE device into separate q800-glue.c file

[Mark Cave-Ayland]

On 21/06/2023 13:00, BALATON Zoltan wrote:

> On Wed, 21 Jun 2023, Mark Cave-Ayland wrote:
>> This will allow the q800-glue.h header to be included separately so that the
>> GLUE device can be referenced externally.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>> ---
>> MAINTAINERS                 |   2 +
>> hw/m68k/meson.build         |   2 +-
>> hw/m68k/q800-glue.c         | 252 ++++++++++++++++++++++++++++++++++++
>> hw/m68k/q800.c              | 238 +---------------------------------
>> include/hw/m68k/q800-glue.h |  50 +++++++
>> 5 files changed, 306 insertions(+), 238 deletions(-)
>> create mode 100644 hw/m68k/q800-glue.c
>> create mode 100644 include/hw/m68k/q800-glue.h
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 748a66fbaa..7f323cd2eb 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -1225,6 +1225,7 @@ q800
>> M: Laurent Vivier <laurent@vivier.eu>
>> S: Maintained
>> F: hw/m68k/q800.c
>> +F: hw/m68k/q800-glue.c
>> F: hw/misc/mac_via.c
>> F: hw/nubus/*
>> F: hw/display/macfb.c
>> @@ -1237,6 +1238,7 @@ F: include/hw/nubus/*
>> F: include/hw/display/macfb.h
>> F: include/hw/block/swim.h
>> F: include/hw/m68k/q800.h
>> +F: include/hw/m68k/q800-glue.h
>>
>> virt
>> M: Laurent Vivier <laurent@vivier.eu>
>> diff --git a/hw/m68k/meson.build b/hw/m68k/meson.build
>> index 31248641d3..84bc68fa4e 100644
>> --- a/hw/m68k/meson.build
>> +++ b/hw/m68k/meson.build
>> @@ -2,7 +2,7 @@ m68k_ss = ss.source_set()
>> m68k_ss.add(when: 'CONFIG_AN5206', if_true: files('an5206.c', 'mcf5206.c'))
>> m68k_ss.add(when: 'CONFIG_MCF5208', if_true: files('mcf5208.c', 'mcf_intc.c'))
>> m68k_ss.add(when: 'CONFIG_NEXTCUBE', if_true: files('next-kbd.c', 'next-cube.c'))
>> -m68k_ss.add(when: 'CONFIG_Q800', if_true: files('q800.c'))
>> +m68k_ss.add(when: 'CONFIG_Q800', if_true: files('q800.c', 'q800-glue.c'))
>> m68k_ss.add(when: 'CONFIG_M68K_VIRT', if_true: files('virt.c'))
>>
>> hw_arch += {'m68k': m68k_ss}
>> diff --git a/hw/m68k/q800-glue.c b/hw/m68k/q800-glue.c
>> new file mode 100644
>> index 0000000000..e81f9438f1
>> --- /dev/null
>> +++ b/hw/m68k/q800-glue.c
>> @@ -0,0 +1,252 @@
>> +/*
>> + * QEMU q800 logic GLUE (General Logic Unit)
> 
> Maybe clearer:
> 
> q800 General Logic Unit (aka GLUE)

The current wording was suggested by Phil during a previous round of review :)

> Sorry for coming late in this review but I just had some time to look more closely at 
> these.

No worries.

> Regards,
> BALATON Zoltan
> 
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a copy
>> + * of this software and associated documentation files (the "Software"), to deal
>> + * in the Software without restriction, including without limitation the rights
>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>> + * copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>> + * THE SOFTWARE.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "cpu.h"
>> +#include "hw/m68k/q800-glue.h"
>> +#include "hw/boards.h"
>> +#include "hw/irq.h"
>> +#include "hw/nmi.h"
>> +#include "hw/qdev-properties.h"
>> +#include "migration/vmstate.h"
>> +
>> +/*
>> + * The GLUE (General Logic Unit) is an Apple custom integrated circuit chip
>> + * that performs a variety of functions (RAM management, clock generation, ...).
>> + * The GLUE chip receives interrupt requests from various devices,
>> + * assign priority to each, and asserts one or more interrupt line to the
>> + * CPU.
>> + */
>> +
>> +/*
>> + * The GLUE logic on the Quadra 800 supports 2 different IRQ routing modes
>> + * controlled from the VIA1 auxmode GPIO (port B bit 6) which are documented
>> + * in NetBSD as follows:
>> + *
>> + * A/UX mode (Linux, NetBSD, auxmode GPIO low)
>> + *
>> + *   Level 0:        Spurious: ignored
>> + *   Level 1:        Software
>> + *   Level 2:        VIA2 (except ethernet, sound)
>> + *   Level 3:        Ethernet
>> + *   Level 4:        Serial (SCC)
>> + *   Level 5:        Sound
>> + *   Level 6:        VIA1
>> + *   Level 7:        NMIs: parity errors, RESET button, YANCC error
>> + *
>> + * Classic mode (default: used by MacOS, A/UX 3.0.1, auxmode GPIO high)
>> + *
>> + *   Level 0:        Spurious: ignored
>> + *   Level 1:        VIA1 (clock, ADB)
>> + *   Level 2:        VIA2 (NuBus, SCSI)
>> + *   Level 3:
>> + *   Level 4:        Serial (SCC)
>> + *   Level 5:
>> + *   Level 6:
>> + *   Level 7:        Non-maskable: parity errors, RESET button
>> + *
>> + * Note that despite references to A/UX mode in Linux and NetBSD, at least
>> + * A/UX 3.0.1 still uses Classic mode.
>> + */
>> +
>> +static void GLUE_set_irq(void *opaque, int irq, int level)
>> +{
>> +    GLUEState *s = opaque;
>> +    int i;
>> +
>> +    if (s->auxmode) {
>> +        /* Classic mode */
>> +        switch (irq) {
>> +        case GLUE_IRQ_IN_VIA1:
>> +            irq = 0;
>> +            break;
>> +
>> +        case GLUE_IRQ_IN_VIA2:
>> +            irq = 1;
>> +            break;
>> +
>> +        case GLUE_IRQ_IN_SONIC:
>> +            /* Route to VIA2 instead */
>> +            qemu_set_irq(s->irqs[GLUE_IRQ_NUBUS_9], level);
>> +            return;
>> +
>> +        case GLUE_IRQ_IN_ESCC:
>> +            irq = 3;
>> +            break;
>> +
>> +        case GLUE_IRQ_IN_NMI:
>> +            irq = 6;
>> +            break;
>> +
>> +        default:
>> +            g_assert_not_reached();
>> +        }
>> +    } else {
>> +        /* A/UX mode */
>> +        switch (irq) {
>> +        case GLUE_IRQ_IN_VIA1:
>> +            irq = 5;
>> +            break;
>> +
>> +        case GLUE_IRQ_IN_VIA2:
>> +            irq = 1;
>> +            break;
>> +
>> +        case GLUE_IRQ_IN_SONIC:
>> +            irq = 2;
>> +            break;
>> +
>> +        case GLUE_IRQ_IN_ESCC:
>> +            irq = 3;
>> +            break;
>> +
>> +        case GLUE_IRQ_IN_NMI:
>> +            irq = 6;
>> +            break;
>> +
>> +        default:
>> +            g_assert_not_reached();
>> +        }
>> +    }
>> +
>> +    if (level) {
>> +        s->ipr |= 1 << irq;
>> +    } else {
>> +        s->ipr &= ~(1 << irq);
>> +    }
>> +
>> +    for (i = 7; i >= 0; i--) {
>> +        if ((s->ipr >> i) & 1) {
>> +            m68k_set_irq_level(s->cpu, i + 1, i + 25);
>> +            return;
>> +        }
>> +    }
>> +    m68k_set_irq_level(s->cpu, 0, 0);
>> +}
>> +
>> +static void glue_auxmode_set_irq(void *opaque, int irq, int level)
>> +{
>> +    GLUEState *s = GLUE(opaque);
>> +
>> +    s->auxmode = level;
>> +}
>> +
>> +static void glue_nmi(NMIState *n, int cpu_index, Error **errp)
>> +{
>> +    GLUEState *s = GLUE(n);
>> +
>> +    /* Hold NMI active for 100ms */
>> +    GLUE_set_irq(s, GLUE_IRQ_IN_NMI, 1);
>> +    timer_mod(s->nmi_release, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 100);
>> +}
>> +
>> +static void glue_nmi_release(void *opaque)
>> +{
>> +    GLUEState *s = GLUE(opaque);
>> +
>> +    GLUE_set_irq(s, GLUE_IRQ_IN_NMI, 0);
>> +}
>> +
>> +static void glue_reset(DeviceState *dev)
>> +{
>> +    GLUEState *s = GLUE(dev);
>> +
>> +    s->ipr = 0;
>> +    s->auxmode = 0;
>> +
>> +    timer_del(s->nmi_release);
>> +}
>> +
>> +static const VMStateDescription vmstate_glue = {
>> +    .name = "q800-glue",
>> +    .version_id = 0,
>> +    .minimum_version_id = 0,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_UINT8(ipr, GLUEState),
>> +        VMSTATE_UINT8(auxmode, GLUEState),
>> +        VMSTATE_TIMER_PTR(nmi_release, GLUEState),
>> +        VMSTATE_END_OF_LIST(),
>> +    },
>> +};
>> +
>> +/*
>> + * If the m68k CPU implemented its inbound irq lines as GPIO lines
>> + * rather than via the m68k_set_irq_level() function we would not need
>> + * this cpu link property and could instead provide outbound IRQ lines
>> + * that the board could wire up to the CPU.
>> + */
>> +static Property glue_properties[] = {
>> +    DEFINE_PROP_LINK("cpu", GLUEState, cpu, TYPE_M68K_CPU, M68kCPU *),
>> +    DEFINE_PROP_END_OF_LIST(),
>> +};
>> +
>> +static void glue_finalize(Object *obj)
>> +{
>> +    GLUEState *s = GLUE(obj);
>> +
>> +    timer_free(s->nmi_release);
>> +}
>> +
>> +static void glue_init(Object *obj)
>> +{
>> +    DeviceState *dev = DEVICE(obj);
>> +    GLUEState *s = GLUE(dev);
>> +
>> +    qdev_init_gpio_in(dev, GLUE_set_irq, 8);
>> +    qdev_init_gpio_in_named(dev, glue_auxmode_set_irq, "auxmode", 1);
>> +
>> +    qdev_init_gpio_out(dev, s->irqs, 1);
>> +
>> +    /* NMI release timer */
>> +    s->nmi_release = timer_new_ms(QEMU_CLOCK_VIRTUAL, glue_nmi_release, s);
>> +}
>> +
>> +static void glue_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +    NMIClass *nc = NMI_CLASS(klass);
>> +
>> +    dc->vmsd = &vmstate_glue;
>> +    dc->reset = glue_reset;
>> +    device_class_set_props(dc, glue_properties);
>> +    nc->nmi_monitor_handler = glue_nmi;
>> +}
>> +
>> +static const TypeInfo glue_info = {
>> +    .name = TYPE_GLUE,
>> +    .parent = TYPE_SYS_BUS_DEVICE,
>> +    .instance_size = sizeof(GLUEState),
>> +    .instance_init = glue_init,
>> +    .instance_finalize = glue_finalize,
>> +    .class_init = glue_class_init,
>> +    .interfaces = (InterfaceInfo[]) {
>> +         { TYPE_NMI },
>> +         { }
>> +    },
>> +};
>> +
>> +static void glue_register_types(void)
>> +{
>> +    type_register_static(&glue_info);
>> +}
>> +
>> +type_init(glue_register_types)
>> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
>> index 9f9668c2b4..9f9de2ebaf 100644
>> --- a/hw/m68k/q800.c
>> +++ b/hw/m68k/q800.c
>> @@ -28,7 +28,6 @@
>> #include "cpu.h"
>> #include "hw/boards.h"
>> #include "hw/or-irq.h"
>> -#include "hw/nmi.h"
>> #include "elf.h"
>> #include "hw/loader.h"
>> #include "ui/console.h"
>> @@ -39,6 +38,7 @@
>> #include "standard-headers/asm-m68k/bootinfo-mac.h"
>> #include "bootinfo.h"
>> #include "hw/m68k/q800.h"
>> +#include "hw/m68k/q800-glue.h"
>> #include "hw/misc/mac_via.h"
>> #include "hw/input/adb.h"
>> #include "hw/nubus/mac-nubus-bridge.h"
>> @@ -88,241 +88,6 @@
>> #define Q800_NUBUS_SLOTS_AVAILABLE    (BIT(0x9) | BIT(0xc) | BIT(0xd) | \
>>                                        BIT(0xe))
>>
>> -/*
>> - * The GLUE (General Logic Unit) is an Apple custom integrated circuit chip
>> - * that performs a variety of functions (RAM management, clock generation, ...).
>> - * The GLUE chip receives interrupt requests from various devices,
>> - * assign priority to each, and asserts one or more interrupt line to the
>> - * CPU.
>> - */
>> -
>> -#define TYPE_GLUE "q800-glue"
>> -OBJECT_DECLARE_SIMPLE_TYPE(GLUEState, GLUE)
>> -
>> -struct GLUEState {
>> -    SysBusDevice parent_obj;
>> -
>> -    M68kCPU *cpu;
>> -    uint8_t ipr;
>> -    uint8_t auxmode;
>> -    qemu_irq irqs[1];
>> -    QEMUTimer *nmi_release;
>> -};
>> -
>> -#define GLUE_IRQ_IN_VIA1       0
>> -#define GLUE_IRQ_IN_VIA2       1
>> -#define GLUE_IRQ_IN_SONIC      2
>> -#define GLUE_IRQ_IN_ESCC       3
>> -#define GLUE_IRQ_IN_NMI        4
>> -
>> -#define GLUE_IRQ_NUBUS_9       0
>> -
>> -/*
>> - * The GLUE logic on the Quadra 800 supports 2 different IRQ routing modes
>> - * controlled from the VIA1 auxmode GPIO (port B bit 6) which are documented
>> - * in NetBSD as follows:
>> - *
>> - * A/UX mode (Linux, NetBSD, auxmode GPIO low)
>> - *
>> - *   Level 0:        Spurious: ignored
>> - *   Level 1:        Software
>> - *   Level 2:        VIA2 (except ethernet, sound)
>> - *   Level 3:        Ethernet
>> - *   Level 4:        Serial (SCC)
>> - *   Level 5:        Sound
>> - *   Level 6:        VIA1
>> - *   Level 7:        NMIs: parity errors, RESET button, YANCC error
>> - *
>> - * Classic mode (default: used by MacOS, A/UX 3.0.1, auxmode GPIO high)
>> - *
>> - *   Level 0:        Spurious: ignored
>> - *   Level 1:        VIA1 (clock, ADB)
>> - *   Level 2:        VIA2 (NuBus, SCSI)
>> - *   Level 3:
>> - *   Level 4:        Serial (SCC)
>> - *   Level 5:
>> - *   Level 6:
>> - *   Level 7:        Non-maskable: parity errors, RESET button
>> - *
>> - * Note that despite references to A/UX mode in Linux and NetBSD, at least
>> - * A/UX 3.0.1 still uses Classic mode.
>> - */
>> -
>> -static void GLUE_set_irq(void *opaque, int irq, int level)
>> -{
>> -    GLUEState *s = opaque;
>> -    int i;
>> -
>> -    if (s->auxmode) {
>> -        /* Classic mode */
>> -        switch (irq) {
>> -        case GLUE_IRQ_IN_VIA1:
>> -            irq = 0;
>> -            break;
>> -
>> -        case GLUE_IRQ_IN_VIA2:
>> -            irq = 1;
>> -            break;
>> -
>> -        case GLUE_IRQ_IN_SONIC:
>> -            /* Route to VIA2 instead */
>> -            qemu_set_irq(s->irqs[GLUE_IRQ_NUBUS_9], level);
>> -            return;
>> -
>> -        case GLUE_IRQ_IN_ESCC:
>> -            irq = 3;
>> -            break;
>> -
>> -        case GLUE_IRQ_IN_NMI:
>> -            irq = 6;
>> -            break;
>> -
>> -        default:
>> -            g_assert_not_reached();
>> -        }
>> -    } else {
>> -        /* A/UX mode */
>> -        switch (irq) {
>> -        case GLUE_IRQ_IN_VIA1:
>> -            irq = 5;
>> -            break;
>> -
>> -        case GLUE_IRQ_IN_VIA2:
>> -            irq = 1;
>> -            break;
>> -
>> -        case GLUE_IRQ_IN_SONIC:
>> -            irq = 2;
>> -            break;
>> -
>> -        case GLUE_IRQ_IN_ESCC:
>> -            irq = 3;
>> -            break;
>> -
>> -        case GLUE_IRQ_IN_NMI:
>> -            irq = 6;
>> -            break;
>> -
>> -        default:
>> -            g_assert_not_reached();
>> -        }
>> -    }
>> -
>> -    if (level) {
>> -        s->ipr |= 1 << irq;
>> -    } else {
>> -        s->ipr &= ~(1 << irq);
>> -    }
>> -
>> -    for (i = 7; i >= 0; i--) {
>> -        if ((s->ipr >> i) & 1) {
>> -            m68k_set_irq_level(s->cpu, i + 1, i + 25);
>> -            return;
>> -        }
>> -    }
>> -    m68k_set_irq_level(s->cpu, 0, 0);
>> -}
>> -
>> -static void glue_auxmode_set_irq(void *opaque, int irq, int level)
>> -{
>> -    GLUEState *s = GLUE(opaque);
>> -
>> -    s->auxmode = level;
>> -}
>> -
>> -static void glue_nmi(NMIState *n, int cpu_index, Error **errp)
>> -{
>> -    GLUEState *s = GLUE(n);
>> -
>> -    /* Hold NMI active for 100ms */
>> -    GLUE_set_irq(s, GLUE_IRQ_IN_NMI, 1);
>> -    timer_mod(s->nmi_release, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 100);
>> -}
>> -
>> -static void glue_nmi_release(void *opaque)
>> -{
>> -    GLUEState *s = GLUE(opaque);
>> -
>> -    GLUE_set_irq(s, GLUE_IRQ_IN_NMI, 0);
>> -}
>> -
>> -static void glue_reset(DeviceState *dev)
>> -{
>> -    GLUEState *s = GLUE(dev);
>> -
>> -    s->ipr = 0;
>> -    s->auxmode = 0;
>> -
>> -    timer_del(s->nmi_release);
>> -}
>> -
>> -static const VMStateDescription vmstate_glue = {
>> -    .name = "q800-glue",
>> -    .version_id = 0,
>> -    .minimum_version_id = 0,
>> -    .fields = (VMStateField[]) {
>> -        VMSTATE_UINT8(ipr, GLUEState),
>> -        VMSTATE_UINT8(auxmode, GLUEState),
>> -        VMSTATE_TIMER_PTR(nmi_release, GLUEState),
>> -        VMSTATE_END_OF_LIST(),
>> -    },
>> -};
>> -
>> -/*
>> - * If the m68k CPU implemented its inbound irq lines as GPIO lines
>> - * rather than via the m68k_set_irq_level() function we would not need
>> - * this cpu link property and could instead provide outbound IRQ lines
>> - * that the board could wire up to the CPU.
>> - */
>> -static Property glue_properties[] = {
>> -    DEFINE_PROP_LINK("cpu", GLUEState, cpu, TYPE_M68K_CPU, M68kCPU *),
>> -    DEFINE_PROP_END_OF_LIST(),
>> -};
>> -
>> -static void glue_finalize(Object *obj)
>> -{
>> -    GLUEState *s = GLUE(obj);
>> -
>> -    timer_free(s->nmi_release);
>> -}
>> -
>> -static void glue_init(Object *obj)
>> -{
>> -    DeviceState *dev = DEVICE(obj);
>> -    GLUEState *s = GLUE(dev);
>> -
>> -    qdev_init_gpio_in(dev, GLUE_set_irq, 8);
>> -    qdev_init_gpio_in_named(dev, glue_auxmode_set_irq, "auxmode", 1);
>> -
>> -    qdev_init_gpio_out(dev, s->irqs, 1);
>> -
>> -    /* NMI release timer */
>> -    s->nmi_release = timer_new_ms(QEMU_CLOCK_VIRTUAL, glue_nmi_release, s);
>> -}
>> -
>> -static void glue_class_init(ObjectClass *klass, void *data)
>> -{
>> -    DeviceClass *dc = DEVICE_CLASS(klass);
>> -    NMIClass *nc = NMI_CLASS(klass);
>> -
>> -    dc->vmsd = &vmstate_glue;
>> -    dc->reset = glue_reset;
>> -    device_class_set_props(dc, glue_properties);
>> -    nc->nmi_monitor_handler = glue_nmi;
>> -}
>> -
>> -static const TypeInfo glue_info = {
>> -    .name = TYPE_GLUE,
>> -    .parent = TYPE_SYS_BUS_DEVICE,
>> -    .instance_size = sizeof(GLUEState),
>> -    .instance_init = glue_init,
>> -    .instance_finalize = glue_finalize,
>> -    .class_init = glue_class_init,
>> -    .interfaces = (InterfaceInfo[]) {
>> -         { TYPE_NMI },
>> -         { }
>> -    },
>> -};
>>
>> static void main_cpu_reset(void *opaque)
>> {
>> @@ -763,7 +528,6 @@ static const TypeInfo q800_machine_typeinfo = {
>> static void q800_machine_register_types(void)
>> {
>>     type_register_static(&q800_machine_typeinfo);
>> -    type_register_static(&glue_info);
>> }
>>
>> type_init(q800_machine_register_types)
>> diff --git a/include/hw/m68k/q800-glue.h b/include/hw/m68k/q800-glue.h
>> new file mode 100644
>> index 0000000000..c1817b01a5
>> --- /dev/null
>> +++ b/include/hw/m68k/q800-glue.h
>> @@ -0,0 +1,50 @@
>> +/*
>> + * QEMU q800 logic glue
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a copy
>> + * of this software and associated documentation files (the "Software"), to deal
>> + * in the Software without restriction, including without limitation the rights
>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>> + * copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>> + * THE SOFTWARE.
>> + */
>> +
>> +#ifndef HW_Q800_GLUE_H
>> +#define HW_Q800_GLUE_H
>> +
>> +#include "qemu/osdep.h"
>> +#include "hw/sysbus.h"
>> +
>> +#define TYPE_GLUE "q800-glue"
>> +OBJECT_DECLARE_SIMPLE_TYPE(GLUEState, GLUE)
>> +
>> +struct GLUEState {
>> +    SysBusDevice parent_obj;
>> +
>> +    M68kCPU *cpu;
>> +    uint8_t ipr;
>> +    uint8_t auxmode;
>> +    qemu_irq irqs[1];
>> +    QEMUTimer *nmi_release;
>> +};
>> +
>> +#define GLUE_IRQ_IN_VIA1       0
>> +#define GLUE_IRQ_IN_VIA2       1
>> +#define GLUE_IRQ_IN_SONIC      2
>> +#define GLUE_IRQ_IN_ESCC       3
>> +#define GLUE_IRQ_IN_NMI        4
>> +
>> +#define GLUE_IRQ_NUBUS_9       0
>> +
>> +#endif


ATB,

Mark.

Re: [PATCH v4 02/24] q800: add missing space after parent object in GLUEState

[BALATON Zoltan]

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

On Wed, 21 Jun 2023, Mark Cave-Ayland wrote:
> On 21/06/2023 12:41, BALATON Zoltan wrote:
>
>> On Wed, 21 Jun 2023, Mark Cave-Ayland wrote:
>>> This brings GLUEState in line with our current QOM guidelines.
>> 
>> Are these guidelines documented somewhere? I like this better than the 
>> public/private comments (although I prefer no space at all with just 
>> documenting that QOM object parents should not be accessed directly) but I 
>> haven't seen it discussed and agreed upon so it looks like a convention you 
>> defined but not documented anywhere. But it could be I missed the patch to 
>> coding style or QOM docs to establish this convention.
>
> Alex documented this earlier in the year: you can find this online at 
> https://qemu.readthedocs.io/en/master/devel/style.html#qemu-specific-idioms.

The examples in
https://qemu.readthedocs.io/en/master/devel/qom.html
now contradict that by using parent instead of parent_obj there.

>> If we really want to make these QOM object states stand out we might even 
>> consider formatting these as
>> 
>> struct GLUEState { SysBusDevice parent_obj;
>>      M68kCPU *cpu;
>>      ...
>> }
>> 
>> unless checkpatch would not like that or something.
>
> I'm not overly convinced by this, and yes I suspect it would also require 
> some hacking on checkpatch.pl for it to work.

Nevermind, was just an idea. Blank line wiithout comments is also a good 
convention and less weird looking than the above.

Regards,
BALATON Zoltan

>
>
> ATB,
>
> Mark.
>
>

Re: [PATCH v4 03/24] q800: introduce Q800MachineState

[BALATON Zoltan]

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

On Wed, 21 Jun 2023, Mark Cave-Ayland wrote:
> On 21/06/2023 12:33, BALATON Zoltan wrote:
>
>> On Wed, 21 Jun 2023, Mark Cave-Ayland wrote:
>>> This provides an overall container and owner for Machine-related objects 
>>> such
>>> as MemoryRegions.
>>> 
>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>> MAINTAINERS            |  1 +
>>> hw/m68k/q800.c         |  2 ++
>>> include/hw/m68k/q800.h | 40 ++++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 43 insertions(+)
>>> create mode 100644 include/hw/m68k/q800.h
>>> 
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 88b5a7ee0a..748a66fbaa 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -1236,6 +1236,7 @@ F: include/hw/misc/mac_via.h
>>> F: include/hw/nubus/*
>>> F: include/hw/display/macfb.h
>>> F: include/hw/block/swim.h
>>> +F: include/hw/m68k/q800.h
>>> 
>>> virt
>>> M: Laurent Vivier <laurent@vivier.eu>
>>> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
>>> index 465c510c18..c0256c8a90 100644
>>> --- a/hw/m68k/q800.c
>>> +++ b/hw/m68k/q800.c
>>> @@ -38,6 +38,7 @@
>>> #include "standard-headers/asm-m68k/bootinfo.h"
>>> #include "standard-headers/asm-m68k/bootinfo-mac.h"
>>> #include "bootinfo.h"
>>> +#include "hw/m68k/q800.h"
>>> #include "hw/misc/mac_via.h"
>>> #include "hw/input/adb.h"
>>> #include "hw/nubus/mac-nubus-bridge.h"
>>> @@ -749,6 +750,7 @@ static void q800_machine_class_init(ObjectClass *oc, 
>>> void *data)
>>> static const TypeInfo q800_machine_typeinfo = {
>>>     .name       = MACHINE_TYPE_NAME("q800"),
>>>     .parent     = TYPE_MACHINE,
>>> +    .instance_size = sizeof(Q800MachineState),
>>>     .class_init = q800_machine_class_init,
>>> };
>>> 
>>> diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
>>> new file mode 100644
>>> index 0000000000..f3bc17aa1b
>>> --- /dev/null
>>> +++ b/include/hw/m68k/q800.h
>> 
>> Why is this defined in a public header? Moving struct definitions of 
>> devices to allow them to be embedded in other structs makes sense but is 
>> there ever a reason to embed a machine state anywhere else than using it in 
>> q800.c? I don't think so, thus to preserve locality and save some lines in 
>> this series I think this machine state should just be in q800.c like I have 
>> similar struct in pegasos2.c. It may only make sense to put it in a header 
>> if q800.c was split up to multiple files but even then it should be a local 
>> header in hw/m68k and not a public header in my opinion.
>
> This is just following our standard guidelines since MachineState is a QOM

Again, is this a documented guideline or something vaguely agreen upon? 
I'd argue that only objects that are used by other objects (such as 
devices that can be embedded in machines or other devices) should be 
declared in public headers but there could be local objects that are only 
used locally which don't have to be exported. This machine state is 
definitely a local object of q800 that nothing else should mess with so to 
make that clear I think it should not be in a public header.

> object of TYPE_MACHINE. Note that there are also a number of existing 
> examples of this currently within the QEMU tree.

There are examples for the opposite as well so without an rationale that 
alone is not explaining it.  (Also not having a public header may make 
this series considerably shorter but the main reason I suggest it is to 
ensure locality of this object to q800.)

Regards,
BALATON Zoltan

>
> ATB,
>
> Mark.
>
>

Re: [PATCH v4 07/24] q800: move GLUE device into separate q800-glue.c file

[BALATON Zoltan]

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

On Wed, 21 Jun 2023, Mark Cave-Ayland wrote:
> On 21/06/2023 13:00, BALATON Zoltan wrote:
>> On Wed, 21 Jun 2023, Mark Cave-Ayland wrote:
>>> This will allow the q800-glue.h header to be included separately so that 
>>> the
>>> GLUE device can be referenced externally.
>>> 
>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>>> ---
>>> MAINTAINERS                 |   2 +
>>> hw/m68k/meson.build         |   2 +-
>>> hw/m68k/q800-glue.c         | 252 ++++++++++++++++++++++++++++++++++++
>>> hw/m68k/q800.c              | 238 +---------------------------------
>>> include/hw/m68k/q800-glue.h |  50 +++++++
>>> 5 files changed, 306 insertions(+), 238 deletions(-)
>>> create mode 100644 hw/m68k/q800-glue.c
>>> create mode 100644 include/hw/m68k/q800-glue.h
>>> 
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 748a66fbaa..7f323cd2eb 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -1225,6 +1225,7 @@ q800
>>> M: Laurent Vivier <laurent@vivier.eu>
>>> S: Maintained
>>> F: hw/m68k/q800.c
>>> +F: hw/m68k/q800-glue.c
>>> F: hw/misc/mac_via.c
>>> F: hw/nubus/*
>>> F: hw/display/macfb.c
>>> @@ -1237,6 +1238,7 @@ F: include/hw/nubus/*
>>> F: include/hw/display/macfb.h
>>> F: include/hw/block/swim.h
>>> F: include/hw/m68k/q800.h
>>> +F: include/hw/m68k/q800-glue.h
>>> 
>>> virt
>>> M: Laurent Vivier <laurent@vivier.eu>
>>> diff --git a/hw/m68k/meson.build b/hw/m68k/meson.build
>>> index 31248641d3..84bc68fa4e 100644
>>> --- a/hw/m68k/meson.build
>>> +++ b/hw/m68k/meson.build
>>> @@ -2,7 +2,7 @@ m68k_ss = ss.source_set()
>>> m68k_ss.add(when: 'CONFIG_AN5206', if_true: files('an5206.c', 
>>> 'mcf5206.c'))
>>> m68k_ss.add(when: 'CONFIG_MCF5208', if_true: files('mcf5208.c', 
>>> 'mcf_intc.c'))
>>> m68k_ss.add(when: 'CONFIG_NEXTCUBE', if_true: files('next-kbd.c', 
>>> 'next-cube.c'))
>>> -m68k_ss.add(when: 'CONFIG_Q800', if_true: files('q800.c'))
>>> +m68k_ss.add(when: 'CONFIG_Q800', if_true: files('q800.c', 'q800-glue.c'))
>>> m68k_ss.add(when: 'CONFIG_M68K_VIRT', if_true: files('virt.c'))
>>> 
>>> hw_arch += {'m68k': m68k_ss}
>>> diff --git a/hw/m68k/q800-glue.c b/hw/m68k/q800-glue.c
>>> new file mode 100644
>>> index 0000000000..e81f9438f1
>>> --- /dev/null
>>> +++ b/hw/m68k/q800-glue.c
>>> @@ -0,0 +1,252 @@
>>> +/*
>>> + * QEMU q800 logic GLUE (General Logic Unit)
>> 
>> Maybe clearer:
>> 
>> q800 General Logic Unit (aka GLUE)
>
> The current wording was suggested by Phil during a previous round of review 
> :)

I think he just asked to include the expanded General Logic Unit name not 
just GLUE which might be confusing if you don't know Mac hardware but he 
did not suggest a specific wording. In any case this is just a minor nit 
so not really important.

Regards,
BALATON Zoltan

>> Sorry for coming late in this review but I just had some time to look more 
>> closely at these.
>
> No worries.
>
>> Regards,
>> BALATON Zoltan
>> 
>>> + *
>>> + * Permission is hereby granted, free of charge, to any person obtaining 
>>> a copy
>>> + * of this software and associated documentation files (the "Software"), 
>>> to deal
>>> + * in the Software without restriction, including without limitation the 
>>> rights
>>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or 
>>> sell
>>> + * copies of the Software, and to permit persons to whom the Software is
>>> + * furnished to do so, subject to the following conditions:
>>> + *
>>> + * The above copyright notice and this permission notice shall be 
>>> included in
>>> + * all copies or substantial portions of the Software.
>>> + *
>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
>>> EXPRESS OR
>>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
>>> MERCHANTABILITY,
>>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT 
>>> SHALL
>>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
>>> OTHER
>>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, 
>>> ARISING FROM,
>>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS 
>>> IN
>>> + * THE SOFTWARE.
>>> + */
>>> +
>>> +#include "qemu/osdep.h"
>>> +#include "cpu.h"
>>> +#include "hw/m68k/q800-glue.h"
>>> +#include "hw/boards.h"
>>> +#include "hw/irq.h"
>>> +#include "hw/nmi.h"
>>> +#include "hw/qdev-properties.h"
>>> +#include "migration/vmstate.h"
>>> +
>>> +/*
>>> + * The GLUE (General Logic Unit) is an Apple custom integrated circuit 
>>> chip
>>> + * that performs a variety of functions (RAM management, clock 
>>> generation, ...).
>>> + * The GLUE chip receives interrupt requests from various devices,
>>> + * assign priority to each, and asserts one or more interrupt line to the
>>> + * CPU.
>>> + */
>>> +
>>> +/*
>>> + * The GLUE logic on the Quadra 800 supports 2 different IRQ routing 
>>> modes
>>> + * controlled from the VIA1 auxmode GPIO (port B bit 6) which are 
>>> documented
>>> + * in NetBSD as follows:
>>> + *
>>> + * A/UX mode (Linux, NetBSD, auxmode GPIO low)
>>> + *
>>> + *   Level 0:        Spurious: ignored
>>> + *   Level 1:        Software
>>> + *   Level 2:        VIA2 (except ethernet, sound)
>>> + *   Level 3:        Ethernet
>>> + *   Level 4:        Serial (SCC)
>>> + *   Level 5:        Sound
>>> + *   Level 6:        VIA1
>>> + *   Level 7:        NMIs: parity errors, RESET button, YANCC error
>>> + *
>>> + * Classic mode (default: used by MacOS, A/UX 3.0.1, auxmode GPIO high)
>>> + *
>>> + *   Level 0:        Spurious: ignored
>>> + *   Level 1:        VIA1 (clock, ADB)
>>> + *   Level 2:        VIA2 (NuBus, SCSI)
>>> + *   Level 3:
>>> + *   Level 4:        Serial (SCC)
>>> + *   Level 5:
>>> + *   Level 6:
>>> + *   Level 7:        Non-maskable: parity errors, RESET button
>>> + *
>>> + * Note that despite references to A/UX mode in Linux and NetBSD, at 
>>> least
>>> + * A/UX 3.0.1 still uses Classic mode.
>>> + */
>>> +
>>> +static void GLUE_set_irq(void *opaque, int irq, int level)
>>> +{
>>> +    GLUEState *s = opaque;
>>> +    int i;
>>> +
>>> +    if (s->auxmode) {
>>> +        /* Classic mode */
>>> +        switch (irq) {
>>> +        case GLUE_IRQ_IN_VIA1:
>>> +            irq = 0;
>>> +            break;
>>> +
>>> +        case GLUE_IRQ_IN_VIA2:
>>> +            irq = 1;
>>> +            break;
>>> +
>>> +        case GLUE_IRQ_IN_SONIC:
>>> +            /* Route to VIA2 instead */
>>> +            qemu_set_irq(s->irqs[GLUE_IRQ_NUBUS_9], level);
>>> +            return;
>>> +
>>> +        case GLUE_IRQ_IN_ESCC:
>>> +            irq = 3;
>>> +            break;
>>> +
>>> +        case GLUE_IRQ_IN_NMI:
>>> +            irq = 6;
>>> +            break;
>>> +
>>> +        default:
>>> +            g_assert_not_reached();
>>> +        }
>>> +    } else {
>>> +        /* A/UX mode */
>>> +        switch (irq) {
>>> +        case GLUE_IRQ_IN_VIA1:
>>> +            irq = 5;
>>> +            break;
>>> +
>>> +        case GLUE_IRQ_IN_VIA2:
>>> +            irq = 1;
>>> +            break;
>>> +
>>> +        case GLUE_IRQ_IN_SONIC:
>>> +            irq = 2;
>>> +            break;
>>> +
>>> +        case GLUE_IRQ_IN_ESCC:
>>> +            irq = 3;
>>> +            break;
>>> +
>>> +        case GLUE_IRQ_IN_NMI:
>>> +            irq = 6;
>>> +            break;
>>> +
>>> +        default:
>>> +            g_assert_not_reached();
>>> +        }
>>> +    }
>>> +
>>> +    if (level) {
>>> +        s->ipr |= 1 << irq;
>>> +    } else {
>>> +        s->ipr &= ~(1 << irq);
>>> +    }
>>> +
>>> +    for (i = 7; i >= 0; i--) {
>>> +        if ((s->ipr >> i) & 1) {
>>> +            m68k_set_irq_level(s->cpu, i + 1, i + 25);
>>> +            return;
>>> +        }
>>> +    }
>>> +    m68k_set_irq_level(s->cpu, 0, 0);
>>> +}
>>> +
>>> +static void glue_auxmode_set_irq(void *opaque, int irq, int level)
>>> +{
>>> +    GLUEState *s = GLUE(opaque);
>>> +
>>> +    s->auxmode = level;
>>> +}
>>> +
>>> +static void glue_nmi(NMIState *n, int cpu_index, Error **errp)
>>> +{
>>> +    GLUEState *s = GLUE(n);
>>> +
>>> +    /* Hold NMI active for 100ms */
>>> +    GLUE_set_irq(s, GLUE_IRQ_IN_NMI, 1);
>>> +    timer_mod(s->nmi_release, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 
>>> 100);
>>> +}
>>> +
>>> +static void glue_nmi_release(void *opaque)
>>> +{
>>> +    GLUEState *s = GLUE(opaque);
>>> +
>>> +    GLUE_set_irq(s, GLUE_IRQ_IN_NMI, 0);
>>> +}
>>> +
>>> +static void glue_reset(DeviceState *dev)
>>> +{
>>> +    GLUEState *s = GLUE(dev);
>>> +
>>> +    s->ipr = 0;
>>> +    s->auxmode = 0;
>>> +
>>> +    timer_del(s->nmi_release);
>>> +}
>>> +
>>> +static const VMStateDescription vmstate_glue = {
>>> +    .name = "q800-glue",
>>> +    .version_id = 0,
>>> +    .minimum_version_id = 0,
>>> +    .fields = (VMStateField[]) {
>>> +        VMSTATE_UINT8(ipr, GLUEState),
>>> +        VMSTATE_UINT8(auxmode, GLUEState),
>>> +        VMSTATE_TIMER_PTR(nmi_release, GLUEState),
>>> +        VMSTATE_END_OF_LIST(),
>>> +    },
>>> +};
>>> +
>>> +/*
>>> + * If the m68k CPU implemented its inbound irq lines as GPIO lines
>>> + * rather than via the m68k_set_irq_level() function we would not need
>>> + * this cpu link property and could instead provide outbound IRQ lines
>>> + * that the board could wire up to the CPU.
>>> + */
>>> +static Property glue_properties[] = {
>>> +    DEFINE_PROP_LINK("cpu", GLUEState, cpu, TYPE_M68K_CPU, M68kCPU *),
>>> +    DEFINE_PROP_END_OF_LIST(),
>>> +};
>>> +
>>> +static void glue_finalize(Object *obj)
>>> +{
>>> +    GLUEState *s = GLUE(obj);
>>> +
>>> +    timer_free(s->nmi_release);
>>> +}
>>> +
>>> +static void glue_init(Object *obj)
>>> +{
>>> +    DeviceState *dev = DEVICE(obj);
>>> +    GLUEState *s = GLUE(dev);
>>> +
>>> +    qdev_init_gpio_in(dev, GLUE_set_irq, 8);
>>> +    qdev_init_gpio_in_named(dev, glue_auxmode_set_irq, "auxmode", 1);
>>> +
>>> +    qdev_init_gpio_out(dev, s->irqs, 1);
>>> +
>>> +    /* NMI release timer */
>>> +    s->nmi_release = timer_new_ms(QEMU_CLOCK_VIRTUAL, glue_nmi_release, 
>>> s);
>>> +}
>>> +
>>> +static void glue_class_init(ObjectClass *klass, void *data)
>>> +{
>>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>>> +    NMIClass *nc = NMI_CLASS(klass);
>>> +
>>> +    dc->vmsd = &vmstate_glue;
>>> +    dc->reset = glue_reset;
>>> +    device_class_set_props(dc, glue_properties);
>>> +    nc->nmi_monitor_handler = glue_nmi;
>>> +}
>>> +
>>> +static const TypeInfo glue_info = {
>>> +    .name = TYPE_GLUE,
>>> +    .parent = TYPE_SYS_BUS_DEVICE,
>>> +    .instance_size = sizeof(GLUEState),
>>> +    .instance_init = glue_init,
>>> +    .instance_finalize = glue_finalize,
>>> +    .class_init = glue_class_init,
>>> +    .interfaces = (InterfaceInfo[]) {
>>> +         { TYPE_NMI },
>>> +         { }
>>> +    },
>>> +};
>>> +
>>> +static void glue_register_types(void)
>>> +{
>>> +    type_register_static(&glue_info);
>>> +}
>>> +
>>> +type_init(glue_register_types)
>>> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
>>> index 9f9668c2b4..9f9de2ebaf 100644
>>> --- a/hw/m68k/q800.c
>>> +++ b/hw/m68k/q800.c
>>> @@ -28,7 +28,6 @@
>>> #include "cpu.h"
>>> #include "hw/boards.h"
>>> #include "hw/or-irq.h"
>>> -#include "hw/nmi.h"
>>> #include "elf.h"
>>> #include "hw/loader.h"
>>> #include "ui/console.h"
>>> @@ -39,6 +38,7 @@
>>> #include "standard-headers/asm-m68k/bootinfo-mac.h"
>>> #include "bootinfo.h"
>>> #include "hw/m68k/q800.h"
>>> +#include "hw/m68k/q800-glue.h"
>>> #include "hw/misc/mac_via.h"
>>> #include "hw/input/adb.h"
>>> #include "hw/nubus/mac-nubus-bridge.h"
>>> @@ -88,241 +88,6 @@
>>> #define Q800_NUBUS_SLOTS_AVAILABLE    (BIT(0x9) | BIT(0xc) | BIT(0xd) | \
>>>                                        BIT(0xe))
>>> 
>>> -/*
>>> - * The GLUE (General Logic Unit) is an Apple custom integrated circuit 
>>> chip
>>> - * that performs a variety of functions (RAM management, clock 
>>> generation, ...).
>>> - * The GLUE chip receives interrupt requests from various devices,
>>> - * assign priority to each, and asserts one or more interrupt line to the
>>> - * CPU.
>>> - */
>>> -
>>> -#define TYPE_GLUE "q800-glue"
>>> -OBJECT_DECLARE_SIMPLE_TYPE(GLUEState, GLUE)
>>> -
>>> -struct GLUEState {
>>> -    SysBusDevice parent_obj;
>>> -
>>> -    M68kCPU *cpu;
>>> -    uint8_t ipr;
>>> -    uint8_t auxmode;
>>> -    qemu_irq irqs[1];
>>> -    QEMUTimer *nmi_release;
>>> -};
>>> -
>>> -#define GLUE_IRQ_IN_VIA1       0
>>> -#define GLUE_IRQ_IN_VIA2       1
>>> -#define GLUE_IRQ_IN_SONIC      2
>>> -#define GLUE_IRQ_IN_ESCC       3
>>> -#define GLUE_IRQ_IN_NMI        4
>>> -
>>> -#define GLUE_IRQ_NUBUS_9       0
>>> -
>>> -/*
>>> - * The GLUE logic on the Quadra 800 supports 2 different IRQ routing 
>>> modes
>>> - * controlled from the VIA1 auxmode GPIO (port B bit 6) which are 
>>> documented
>>> - * in NetBSD as follows:
>>> - *
>>> - * A/UX mode (Linux, NetBSD, auxmode GPIO low)
>>> - *
>>> - *   Level 0:        Spurious: ignored
>>> - *   Level 1:        Software
>>> - *   Level 2:        VIA2 (except ethernet, sound)
>>> - *   Level 3:        Ethernet
>>> - *   Level 4:        Serial (SCC)
>>> - *   Level 5:        Sound
>>> - *   Level 6:        VIA1
>>> - *   Level 7:        NMIs: parity errors, RESET button, YANCC error
>>> - *
>>> - * Classic mode (default: used by MacOS, A/UX 3.0.1, auxmode GPIO high)
>>> - *
>>> - *   Level 0:        Spurious: ignored
>>> - *   Level 1:        VIA1 (clock, ADB)
>>> - *   Level 2:        VIA2 (NuBus, SCSI)
>>> - *   Level 3:
>>> - *   Level 4:        Serial (SCC)
>>> - *   Level 5:
>>> - *   Level 6:
>>> - *   Level 7:        Non-maskable: parity errors, RESET button
>>> - *
>>> - * Note that despite references to A/UX mode in Linux and NetBSD, at 
>>> least
>>> - * A/UX 3.0.1 still uses Classic mode.
>>> - */
>>> -
>>> -static void GLUE_set_irq(void *opaque, int irq, int level)
>>> -{
>>> -    GLUEState *s = opaque;
>>> -    int i;
>>> -
>>> -    if (s->auxmode) {
>>> -        /* Classic mode */
>>> -        switch (irq) {
>>> -        case GLUE_IRQ_IN_VIA1:
>>> -            irq = 0;
>>> -            break;
>>> -
>>> -        case GLUE_IRQ_IN_VIA2:
>>> -            irq = 1;
>>> -            break;
>>> -
>>> -        case GLUE_IRQ_IN_SONIC:
>>> -            /* Route to VIA2 instead */
>>> -            qemu_set_irq(s->irqs[GLUE_IRQ_NUBUS_9], level);
>>> -            return;
>>> -
>>> -        case GLUE_IRQ_IN_ESCC:
>>> -            irq = 3;
>>> -            break;
>>> -
>>> -        case GLUE_IRQ_IN_NMI:
>>> -            irq = 6;
>>> -            break;
>>> -
>>> -        default:
>>> -            g_assert_not_reached();
>>> -        }
>>> -    } else {
>>> -        /* A/UX mode */
>>> -        switch (irq) {
>>> -        case GLUE_IRQ_IN_VIA1:
>>> -            irq = 5;
>>> -            break;
>>> -
>>> -        case GLUE_IRQ_IN_VIA2:
>>> -            irq = 1;
>>> -            break;
>>> -
>>> -        case GLUE_IRQ_IN_SONIC:
>>> -            irq = 2;
>>> -            break;
>>> -
>>> -        case GLUE_IRQ_IN_ESCC:
>>> -            irq = 3;
>>> -            break;
>>> -
>>> -        case GLUE_IRQ_IN_NMI:
>>> -            irq = 6;
>>> -            break;
>>> -
>>> -        default:
>>> -            g_assert_not_reached();
>>> -        }
>>> -    }
>>> -
>>> -    if (level) {
>>> -        s->ipr |= 1 << irq;
>>> -    } else {
>>> -        s->ipr &= ~(1 << irq);
>>> -    }
>>> -
>>> -    for (i = 7; i >= 0; i--) {
>>> -        if ((s->ipr >> i) & 1) {
>>> -            m68k_set_irq_level(s->cpu, i + 1, i + 25);
>>> -            return;
>>> -        }
>>> -    }
>>> -    m68k_set_irq_level(s->cpu, 0, 0);
>>> -}
>>> -
>>> -static void glue_auxmode_set_irq(void *opaque, int irq, int level)
>>> -{
>>> -    GLUEState *s = GLUE(opaque);
>>> -
>>> -    s->auxmode = level;
>>> -}
>>> -
>>> -static void glue_nmi(NMIState *n, int cpu_index, Error **errp)
>>> -{
>>> -    GLUEState *s = GLUE(n);
>>> -
>>> -    /* Hold NMI active for 100ms */
>>> -    GLUE_set_irq(s, GLUE_IRQ_IN_NMI, 1);
>>> -    timer_mod(s->nmi_release, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 
>>> 100);
>>> -}
>>> -
>>> -static void glue_nmi_release(void *opaque)
>>> -{
>>> -    GLUEState *s = GLUE(opaque);
>>> -
>>> -    GLUE_set_irq(s, GLUE_IRQ_IN_NMI, 0);
>>> -}
>>> -
>>> -static void glue_reset(DeviceState *dev)
>>> -{
>>> -    GLUEState *s = GLUE(dev);
>>> -
>>> -    s->ipr = 0;
>>> -    s->auxmode = 0;
>>> -
>>> -    timer_del(s->nmi_release);
>>> -}
>>> -
>>> -static const VMStateDescription vmstate_glue = {
>>> -    .name = "q800-glue",
>>> -    .version_id = 0,
>>> -    .minimum_version_id = 0,
>>> -    .fields = (VMStateField[]) {
>>> -        VMSTATE_UINT8(ipr, GLUEState),
>>> -        VMSTATE_UINT8(auxmode, GLUEState),
>>> -        VMSTATE_TIMER_PTR(nmi_release, GLUEState),
>>> -        VMSTATE_END_OF_LIST(),
>>> -    },
>>> -};
>>> -
>>> -/*
>>> - * If the m68k CPU implemented its inbound irq lines as GPIO lines
>>> - * rather than via the m68k_set_irq_level() function we would not need
>>> - * this cpu link property and could instead provide outbound IRQ lines
>>> - * that the board could wire up to the CPU.
>>> - */
>>> -static Property glue_properties[] = {
>>> -    DEFINE_PROP_LINK("cpu", GLUEState, cpu, TYPE_M68K_CPU, M68kCPU *),
>>> -    DEFINE_PROP_END_OF_LIST(),
>>> -};
>>> -
>>> -static void glue_finalize(Object *obj)
>>> -{
>>> -    GLUEState *s = GLUE(obj);
>>> -
>>> -    timer_free(s->nmi_release);
>>> -}
>>> -
>>> -static void glue_init(Object *obj)
>>> -{
>>> -    DeviceState *dev = DEVICE(obj);
>>> -    GLUEState *s = GLUE(dev);
>>> -
>>> -    qdev_init_gpio_in(dev, GLUE_set_irq, 8);
>>> -    qdev_init_gpio_in_named(dev, glue_auxmode_set_irq, "auxmode", 1);
>>> -
>>> -    qdev_init_gpio_out(dev, s->irqs, 1);
>>> -
>>> -    /* NMI release timer */
>>> -    s->nmi_release = timer_new_ms(QEMU_CLOCK_VIRTUAL, glue_nmi_release, 
>>> s);
>>> -}
>>> -
>>> -static void glue_class_init(ObjectClass *klass, void *data)
>>> -{
>>> -    DeviceClass *dc = DEVICE_CLASS(klass);
>>> -    NMIClass *nc = NMI_CLASS(klass);
>>> -
>>> -    dc->vmsd = &vmstate_glue;
>>> -    dc->reset = glue_reset;
>>> -    device_class_set_props(dc, glue_properties);
>>> -    nc->nmi_monitor_handler = glue_nmi;
>>> -}
>>> -
>>> -static const TypeInfo glue_info = {
>>> -    .name = TYPE_GLUE,
>>> -    .parent = TYPE_SYS_BUS_DEVICE,
>>> -    .instance_size = sizeof(GLUEState),
>>> -    .instance_init = glue_init,
>>> -    .instance_finalize = glue_finalize,
>>> -    .class_init = glue_class_init,
>>> -    .interfaces = (InterfaceInfo[]) {
>>> -         { TYPE_NMI },
>>> -         { }
>>> -    },
>>> -};
>>> 
>>> static void main_cpu_reset(void *opaque)
>>> {
>>> @@ -763,7 +528,6 @@ static const TypeInfo q800_machine_typeinfo = {
>>> static void q800_machine_register_types(void)
>>> {
>>>     type_register_static(&q800_machine_typeinfo);
>>> -    type_register_static(&glue_info);
>>> }
>>> 
>>> type_init(q800_machine_register_types)
>>> diff --git a/include/hw/m68k/q800-glue.h b/include/hw/m68k/q800-glue.h
>>> new file mode 100644
>>> index 0000000000..c1817b01a5
>>> --- /dev/null
>>> +++ b/include/hw/m68k/q800-glue.h
>>> @@ -0,0 +1,50 @@
>>> +/*
>>> + * QEMU q800 logic glue
>>> + *
>>> + * Permission is hereby granted, free of charge, to any person obtaining 
>>> a copy
>>> + * of this software and associated documentation files (the "Software"), 
>>> to deal
>>> + * in the Software without restriction, including without limitation the 
>>> rights
>>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or 
>>> sell
>>> + * copies of the Software, and to permit persons to whom the Software is
>>> + * furnished to do so, subject to the following conditions:
>>> + *
>>> + * The above copyright notice and this permission notice shall be 
>>> included in
>>> + * all copies or substantial portions of the Software.
>>> + *
>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
>>> EXPRESS OR
>>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
>>> MERCHANTABILITY,
>>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT 
>>> SHALL
>>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
>>> OTHER
>>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, 
>>> ARISING FROM,
>>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS 
>>> IN
>>> + * THE SOFTWARE.
>>> + */
>>> +
>>> +#ifndef HW_Q800_GLUE_H
>>> +#define HW_Q800_GLUE_H
>>> +
>>> +#include "qemu/osdep.h"
>>> +#include "hw/sysbus.h"
>>> +
>>> +#define TYPE_GLUE "q800-glue"
>>> +OBJECT_DECLARE_SIMPLE_TYPE(GLUEState, GLUE)
>>> +
>>> +struct GLUEState {
>>> +    SysBusDevice parent_obj;
>>> +
>>> +    M68kCPU *cpu;
>>> +    uint8_t ipr;
>>> +    uint8_t auxmode;
>>> +    qemu_irq irqs[1];
>>> +    QEMUTimer *nmi_release;
>>> +};
>>> +
>>> +#define GLUE_IRQ_IN_VIA1       0
>>> +#define GLUE_IRQ_IN_VIA2       1
>>> +#define GLUE_IRQ_IN_SONIC      2
>>> +#define GLUE_IRQ_IN_ESCC       3
>>> +#define GLUE_IRQ_IN_NMI        4
>>> +
>>> +#define GLUE_IRQ_NUBUS_9       0
>>> +
>>> +#endif
>
>
> ATB,
>
> Mark.
>
>

Re: [PATCH v4 07/24] q800: move GLUE device into separate q800-glue.c file

[BALATON Zoltan]

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

On Wed, 21 Jun 2023, BALATON Zoltan wrote:
> On Wed, 21 Jun 2023, Mark Cave-Ayland wrote:
>> On 21/06/2023 13:00, BALATON Zoltan wrote:
>>> On Wed, 21 Jun 2023, Mark Cave-Ayland wrote:
>>>> This will allow the q800-glue.h header to be included separately so that 
>>>> the
>>>> GLUE device can be referenced externally.
>>>> 
>>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>>>> ---
>>>> MAINTAINERS                 |   2 +
>>>> hw/m68k/meson.build         |   2 +-
>>>> hw/m68k/q800-glue.c         | 252 ++++++++++++++++++++++++++++++++++++
>>>> hw/m68k/q800.c              | 238 +---------------------------------
>>>> include/hw/m68k/q800-glue.h |  50 +++++++
>>>> 5 files changed, 306 insertions(+), 238 deletions(-)
>>>> create mode 100644 hw/m68k/q800-glue.c
>>>> create mode 100644 include/hw/m68k/q800-glue.h
>>>> 
>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>> index 748a66fbaa..7f323cd2eb 100644
>>>> --- a/MAINTAINERS
>>>> +++ b/MAINTAINERS
>>>> @@ -1225,6 +1225,7 @@ q800
>>>> M: Laurent Vivier <laurent@vivier.eu>
>>>> S: Maintained
>>>> F: hw/m68k/q800.c
>>>> +F: hw/m68k/q800-glue.c
>>>> F: hw/misc/mac_via.c
>>>> F: hw/nubus/*
>>>> F: hw/display/macfb.c
>>>> @@ -1237,6 +1238,7 @@ F: include/hw/nubus/*
>>>> F: include/hw/display/macfb.h
>>>> F: include/hw/block/swim.h
>>>> F: include/hw/m68k/q800.h
>>>> +F: include/hw/m68k/q800-glue.h
>>>> 
>>>> virt
>>>> M: Laurent Vivier <laurent@vivier.eu>
>>>> diff --git a/hw/m68k/meson.build b/hw/m68k/meson.build
>>>> index 31248641d3..84bc68fa4e 100644
>>>> --- a/hw/m68k/meson.build
>>>> +++ b/hw/m68k/meson.build
>>>> @@ -2,7 +2,7 @@ m68k_ss = ss.source_set()
>>>> m68k_ss.add(when: 'CONFIG_AN5206', if_true: files('an5206.c', 
>>>> 'mcf5206.c'))
>>>> m68k_ss.add(when: 'CONFIG_MCF5208', if_true: files('mcf5208.c', 
>>>> 'mcf_intc.c'))
>>>> m68k_ss.add(when: 'CONFIG_NEXTCUBE', if_true: files('next-kbd.c', 
>>>> 'next-cube.c'))
>>>> -m68k_ss.add(when: 'CONFIG_Q800', if_true: files('q800.c'))
>>>> +m68k_ss.add(when: 'CONFIG_Q800', if_true: files('q800.c', 
>>>> 'q800-glue.c'))
>>>> m68k_ss.add(when: 'CONFIG_M68K_VIRT', if_true: files('virt.c'))
>>>> 
>>>> hw_arch += {'m68k': m68k_ss}
>>>> diff --git a/hw/m68k/q800-glue.c b/hw/m68k/q800-glue.c
>>>> new file mode 100644
>>>> index 0000000000..e81f9438f1
>>>> --- /dev/null
>>>> +++ b/hw/m68k/q800-glue.c
>>>> @@ -0,0 +1,252 @@
>>>> +/*
>>>> + * QEMU q800 logic GLUE (General Logic Unit)
>>> 
>>> Maybe clearer:
>>> 
>>> q800 General Logic Unit (aka GLUE)
>> 
>> The current wording was suggested by Phil during a previous round of review 
>> :)
>
> I think he just asked to include the expanded General Logic Unit name not 
> just GLUE which might be confusing if you don't know Mac hardware but he did 
> not suggest a specific wording. In any case this is just a minor nit so not 
> really important.

I think my problem is really that "logic" is now doubled and seems 
redundant. So either
QEMU q800 General Logic Unit (aka GLUE)
or
QEMU q800 GLUE (General Logic Unit)

Regards,
BALATON Zoltan

>>> Sorry for coming late in this review but I just had some time to look more 
>>> closely at these.
>> 
>> No worries.
>> 
>>> Regards,
>>> BALATON Zoltan
>>> 
>>>> + *
>>>> + * Permission is hereby granted, free of charge, to any person obtaining 
>>>> a copy
>>>> + * of this software and associated documentation files (the "Software"), 
>>>> to deal
>>>> + * in the Software without restriction, including without limitation the 
>>>> rights
>>>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or 
>>>> sell
>>>> + * copies of the Software, and to permit persons to whom the Software is
>>>> + * furnished to do so, subject to the following conditions:
>>>> + *
>>>> + * The above copyright notice and this permission notice shall be 
>>>> included in
>>>> + * all copies or substantial portions of the Software.
>>>> + *
>>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
>>>> EXPRESS OR
>>>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
>>>> MERCHANTABILITY,
>>>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT 
>>>> SHALL
>>>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
>>>> OTHER
>>>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, 
>>>> ARISING FROM,
>>>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
>>>> DEALINGS IN
>>>> + * THE SOFTWARE.
>>>> + */
>>>> +
>>>> +#include "qemu/osdep.h"
>>>> +#include "cpu.h"
>>>> +#include "hw/m68k/q800-glue.h"
>>>> +#include "hw/boards.h"
>>>> +#include "hw/irq.h"
>>>> +#include "hw/nmi.h"
>>>> +#include "hw/qdev-properties.h"
>>>> +#include "migration/vmstate.h"
>>>> +
>>>> +/*
>>>> + * The GLUE (General Logic Unit) is an Apple custom integrated circuit 
>>>> chip
>>>> + * that performs a variety of functions (RAM management, clock 
>>>> generation, ...).
>>>> + * The GLUE chip receives interrupt requests from various devices,
>>>> + * assign priority to each, and asserts one or more interrupt line to 
>>>> the
>>>> + * CPU.
>>>> + */
>>>> +
>>>> +/*
>>>> + * The GLUE logic on the Quadra 800 supports 2 different IRQ routing 
>>>> modes
>>>> + * controlled from the VIA1 auxmode GPIO (port B bit 6) which are 
>>>> documented
>>>> + * in NetBSD as follows:
>>>> + *
>>>> + * A/UX mode (Linux, NetBSD, auxmode GPIO low)
>>>> + *
>>>> + *   Level 0:        Spurious: ignored
>>>> + *   Level 1:        Software
>>>> + *   Level 2:        VIA2 (except ethernet, sound)
>>>> + *   Level 3:        Ethernet
>>>> + *   Level 4:        Serial (SCC)
>>>> + *   Level 5:        Sound
>>>> + *   Level 6:        VIA1
>>>> + *   Level 7:        NMIs: parity errors, RESET button, YANCC error
>>>> + *
>>>> + * Classic mode (default: used by MacOS, A/UX 3.0.1, auxmode GPIO high)
>>>> + *
>>>> + *   Level 0:        Spurious: ignored
>>>> + *   Level 1:        VIA1 (clock, ADB)
>>>> + *   Level 2:        VIA2 (NuBus, SCSI)
>>>> + *   Level 3:
>>>> + *   Level 4:        Serial (SCC)
>>>> + *   Level 5:
>>>> + *   Level 6:
>>>> + *   Level 7:        Non-maskable: parity errors, RESET button
>>>> + *
>>>> + * Note that despite references to A/UX mode in Linux and NetBSD, at 
>>>> least
>>>> + * A/UX 3.0.1 still uses Classic mode.
>>>> + */
>>>> +
>>>> +static void GLUE_set_irq(void *opaque, int irq, int level)
>>>> +{
>>>> +    GLUEState *s = opaque;
>>>> +    int i;
>>>> +
>>>> +    if (s->auxmode) {
>>>> +        /* Classic mode */
>>>> +        switch (irq) {
>>>> +        case GLUE_IRQ_IN_VIA1:
>>>> +            irq = 0;
>>>> +            break;
>>>> +
>>>> +        case GLUE_IRQ_IN_VIA2:
>>>> +            irq = 1;
>>>> +            break;
>>>> +
>>>> +        case GLUE_IRQ_IN_SONIC:
>>>> +            /* Route to VIA2 instead */
>>>> +            qemu_set_irq(s->irqs[GLUE_IRQ_NUBUS_9], level);
>>>> +            return;
>>>> +
>>>> +        case GLUE_IRQ_IN_ESCC:
>>>> +            irq = 3;
>>>> +            break;
>>>> +
>>>> +        case GLUE_IRQ_IN_NMI:
>>>> +            irq = 6;
>>>> +            break;
>>>> +
>>>> +        default:
>>>> +            g_assert_not_reached();
>>>> +        }
>>>> +    } else {
>>>> +        /* A/UX mode */
>>>> +        switch (irq) {
>>>> +        case GLUE_IRQ_IN_VIA1:
>>>> +            irq = 5;
>>>> +            break;
>>>> +
>>>> +        case GLUE_IRQ_IN_VIA2:
>>>> +            irq = 1;
>>>> +            break;
>>>> +
>>>> +        case GLUE_IRQ_IN_SONIC:
>>>> +            irq = 2;
>>>> +            break;
>>>> +
>>>> +        case GLUE_IRQ_IN_ESCC:
>>>> +            irq = 3;
>>>> +            break;
>>>> +
>>>> +        case GLUE_IRQ_IN_NMI:
>>>> +            irq = 6;
>>>> +            break;
>>>> +
>>>> +        default:
>>>> +            g_assert_not_reached();
>>>> +        }
>>>> +    }
>>>> +
>>>> +    if (level) {
>>>> +        s->ipr |= 1 << irq;
>>>> +    } else {
>>>> +        s->ipr &= ~(1 << irq);
>>>> +    }
>>>> +
>>>> +    for (i = 7; i >= 0; i--) {
>>>> +        if ((s->ipr >> i) & 1) {
>>>> +            m68k_set_irq_level(s->cpu, i + 1, i + 25);
>>>> +            return;
>>>> +        }
>>>> +    }
>>>> +    m68k_set_irq_level(s->cpu, 0, 0);
>>>> +}
>>>> +
>>>> +static void glue_auxmode_set_irq(void *opaque, int irq, int level)
>>>> +{
>>>> +    GLUEState *s = GLUE(opaque);
>>>> +
>>>> +    s->auxmode = level;
>>>> +}
>>>> +
>>>> +static void glue_nmi(NMIState *n, int cpu_index, Error **errp)
>>>> +{
>>>> +    GLUEState *s = GLUE(n);
>>>> +
>>>> +    /* Hold NMI active for 100ms */
>>>> +    GLUE_set_irq(s, GLUE_IRQ_IN_NMI, 1);
>>>> +    timer_mod(s->nmi_release, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 
>>>> 100);
>>>> +}
>>>> +
>>>> +static void glue_nmi_release(void *opaque)
>>>> +{
>>>> +    GLUEState *s = GLUE(opaque);
>>>> +
>>>> +    GLUE_set_irq(s, GLUE_IRQ_IN_NMI, 0);
>>>> +}
>>>> +
>>>> +static void glue_reset(DeviceState *dev)
>>>> +{
>>>> +    GLUEState *s = GLUE(dev);
>>>> +
>>>> +    s->ipr = 0;
>>>> +    s->auxmode = 0;
>>>> +
>>>> +    timer_del(s->nmi_release);
>>>> +}
>>>> +
>>>> +static const VMStateDescription vmstate_glue = {
>>>> +    .name = "q800-glue",
>>>> +    .version_id = 0,
>>>> +    .minimum_version_id = 0,
>>>> +    .fields = (VMStateField[]) {
>>>> +        VMSTATE_UINT8(ipr, GLUEState),
>>>> +        VMSTATE_UINT8(auxmode, GLUEState),
>>>> +        VMSTATE_TIMER_PTR(nmi_release, GLUEState),
>>>> +        VMSTATE_END_OF_LIST(),
>>>> +    },
>>>> +};
>>>> +
>>>> +/*
>>>> + * If the m68k CPU implemented its inbound irq lines as GPIO lines
>>>> + * rather than via the m68k_set_irq_level() function we would not need
>>>> + * this cpu link property and could instead provide outbound IRQ lines
>>>> + * that the board could wire up to the CPU.
>>>> + */
>>>> +static Property glue_properties[] = {
>>>> +    DEFINE_PROP_LINK("cpu", GLUEState, cpu, TYPE_M68K_CPU, M68kCPU *),
>>>> +    DEFINE_PROP_END_OF_LIST(),
>>>> +};
>>>> +
>>>> +static void glue_finalize(Object *obj)
>>>> +{
>>>> +    GLUEState *s = GLUE(obj);
>>>> +
>>>> +    timer_free(s->nmi_release);
>>>> +}
>>>> +
>>>> +static void glue_init(Object *obj)
>>>> +{
>>>> +    DeviceState *dev = DEVICE(obj);
>>>> +    GLUEState *s = GLUE(dev);
>>>> +
>>>> +    qdev_init_gpio_in(dev, GLUE_set_irq, 8);
>>>> +    qdev_init_gpio_in_named(dev, glue_auxmode_set_irq, "auxmode", 1);
>>>> +
>>>> +    qdev_init_gpio_out(dev, s->irqs, 1);
>>>> +
>>>> +    /* NMI release timer */
>>>> +    s->nmi_release = timer_new_ms(QEMU_CLOCK_VIRTUAL, glue_nmi_release, 
>>>> s);
>>>> +}
>>>> +
>>>> +static void glue_class_init(ObjectClass *klass, void *data)
>>>> +{
>>>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>>>> +    NMIClass *nc = NMI_CLASS(klass);
>>>> +
>>>> +    dc->vmsd = &vmstate_glue;
>>>> +    dc->reset = glue_reset;
>>>> +    device_class_set_props(dc, glue_properties);
>>>> +    nc->nmi_monitor_handler = glue_nmi;
>>>> +}
>>>> +
>>>> +static const TypeInfo glue_info = {
>>>> +    .name = TYPE_GLUE,
>>>> +    .parent = TYPE_SYS_BUS_DEVICE,
>>>> +    .instance_size = sizeof(GLUEState),
>>>> +    .instance_init = glue_init,
>>>> +    .instance_finalize = glue_finalize,
>>>> +    .class_init = glue_class_init,
>>>> +    .interfaces = (InterfaceInfo[]) {
>>>> +         { TYPE_NMI },
>>>> +         { }
>>>> +    },
>>>> +};
>>>> +
>>>> +static void glue_register_types(void)
>>>> +{
>>>> +    type_register_static(&glue_info);
>>>> +}
>>>> +
>>>> +type_init(glue_register_types)
>>>> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
>>>> index 9f9668c2b4..9f9de2ebaf 100644
>>>> --- a/hw/m68k/q800.c
>>>> +++ b/hw/m68k/q800.c
>>>> @@ -28,7 +28,6 @@
>>>> #include "cpu.h"
>>>> #include "hw/boards.h"
>>>> #include "hw/or-irq.h"
>>>> -#include "hw/nmi.h"
>>>> #include "elf.h"
>>>> #include "hw/loader.h"
>>>> #include "ui/console.h"
>>>> @@ -39,6 +38,7 @@
>>>> #include "standard-headers/asm-m68k/bootinfo-mac.h"
>>>> #include "bootinfo.h"
>>>> #include "hw/m68k/q800.h"
>>>> +#include "hw/m68k/q800-glue.h"
>>>> #include "hw/misc/mac_via.h"
>>>> #include "hw/input/adb.h"
>>>> #include "hw/nubus/mac-nubus-bridge.h"
>>>> @@ -88,241 +88,6 @@
>>>> #define Q800_NUBUS_SLOTS_AVAILABLE    (BIT(0x9) | BIT(0xc) | BIT(0xd) | \
>>>>                                        BIT(0xe))
>>>> 
>>>> -/*
>>>> - * The GLUE (General Logic Unit) is an Apple custom integrated circuit 
>>>> chip
>>>> - * that performs a variety of functions (RAM management, clock 
>>>> generation, ...).
>>>> - * The GLUE chip receives interrupt requests from various devices,
>>>> - * assign priority to each, and asserts one or more interrupt line to 
>>>> the
>>>> - * CPU.
>>>> - */
>>>> -
>>>> -#define TYPE_GLUE "q800-glue"
>>>> -OBJECT_DECLARE_SIMPLE_TYPE(GLUEState, GLUE)
>>>> -
>>>> -struct GLUEState {
>>>> -    SysBusDevice parent_obj;
>>>> -
>>>> -    M68kCPU *cpu;
>>>> -    uint8_t ipr;
>>>> -    uint8_t auxmode;
>>>> -    qemu_irq irqs[1];
>>>> -    QEMUTimer *nmi_release;
>>>> -};
>>>> -
>>>> -#define GLUE_IRQ_IN_VIA1       0
>>>> -#define GLUE_IRQ_IN_VIA2       1
>>>> -#define GLUE_IRQ_IN_SONIC      2
>>>> -#define GLUE_IRQ_IN_ESCC       3
>>>> -#define GLUE_IRQ_IN_NMI        4
>>>> -
>>>> -#define GLUE_IRQ_NUBUS_9       0
>>>> -
>>>> -/*
>>>> - * The GLUE logic on the Quadra 800 supports 2 different IRQ routing 
>>>> modes
>>>> - * controlled from the VIA1 auxmode GPIO (port B bit 6) which are 
>>>> documented
>>>> - * in NetBSD as follows:
>>>> - *
>>>> - * A/UX mode (Linux, NetBSD, auxmode GPIO low)
>>>> - *
>>>> - *   Level 0:        Spurious: ignored
>>>> - *   Level 1:        Software
>>>> - *   Level 2:        VIA2 (except ethernet, sound)
>>>> - *   Level 3:        Ethernet
>>>> - *   Level 4:        Serial (SCC)
>>>> - *   Level 5:        Sound
>>>> - *   Level 6:        VIA1
>>>> - *   Level 7:        NMIs: parity errors, RESET button, YANCC error
>>>> - *
>>>> - * Classic mode (default: used by MacOS, A/UX 3.0.1, auxmode GPIO high)
>>>> - *
>>>> - *   Level 0:        Spurious: ignored
>>>> - *   Level 1:        VIA1 (clock, ADB)
>>>> - *   Level 2:        VIA2 (NuBus, SCSI)
>>>> - *   Level 3:
>>>> - *   Level 4:        Serial (SCC)
>>>> - *   Level 5:
>>>> - *   Level 6:
>>>> - *   Level 7:        Non-maskable: parity errors, RESET button
>>>> - *
>>>> - * Note that despite references to A/UX mode in Linux and NetBSD, at 
>>>> least
>>>> - * A/UX 3.0.1 still uses Classic mode.
>>>> - */
>>>> -
>>>> -static void GLUE_set_irq(void *opaque, int irq, int level)
>>>> -{
>>>> -    GLUEState *s = opaque;
>>>> -    int i;
>>>> -
>>>> -    if (s->auxmode) {
>>>> -        /* Classic mode */
>>>> -        switch (irq) {
>>>> -        case GLUE_IRQ_IN_VIA1:
>>>> -            irq = 0;
>>>> -            break;
>>>> -
>>>> -        case GLUE_IRQ_IN_VIA2:
>>>> -            irq = 1;
>>>> -            break;
>>>> -
>>>> -        case GLUE_IRQ_IN_SONIC:
>>>> -            /* Route to VIA2 instead */
>>>> -            qemu_set_irq(s->irqs[GLUE_IRQ_NUBUS_9], level);
>>>> -            return;
>>>> -
>>>> -        case GLUE_IRQ_IN_ESCC:
>>>> -            irq = 3;
>>>> -            break;
>>>> -
>>>> -        case GLUE_IRQ_IN_NMI:
>>>> -            irq = 6;
>>>> -            break;
>>>> -
>>>> -        default:
>>>> -            g_assert_not_reached();
>>>> -        }
>>>> -    } else {
>>>> -        /* A/UX mode */
>>>> -        switch (irq) {
>>>> -        case GLUE_IRQ_IN_VIA1:
>>>> -            irq = 5;
>>>> -            break;
>>>> -
>>>> -        case GLUE_IRQ_IN_VIA2:
>>>> -            irq = 1;
>>>> -            break;
>>>> -
>>>> -        case GLUE_IRQ_IN_SONIC:
>>>> -            irq = 2;
>>>> -            break;
>>>> -
>>>> -        case GLUE_IRQ_IN_ESCC:
>>>> -            irq = 3;
>>>> -            break;
>>>> -
>>>> -        case GLUE_IRQ_IN_NMI:
>>>> -            irq = 6;
>>>> -            break;
>>>> -
>>>> -        default:
>>>> -            g_assert_not_reached();
>>>> -        }
>>>> -    }
>>>> -
>>>> -    if (level) {
>>>> -        s->ipr |= 1 << irq;
>>>> -    } else {
>>>> -        s->ipr &= ~(1 << irq);
>>>> -    }
>>>> -
>>>> -    for (i = 7; i >= 0; i--) {
>>>> -        if ((s->ipr >> i) & 1) {
>>>> -            m68k_set_irq_level(s->cpu, i + 1, i + 25);
>>>> -            return;
>>>> -        }
>>>> -    }
>>>> -    m68k_set_irq_level(s->cpu, 0, 0);
>>>> -}
>>>> -
>>>> -static void glue_auxmode_set_irq(void *opaque, int irq, int level)
>>>> -{
>>>> -    GLUEState *s = GLUE(opaque);
>>>> -
>>>> -    s->auxmode = level;
>>>> -}
>>>> -
>>>> -static void glue_nmi(NMIState *n, int cpu_index, Error **errp)
>>>> -{
>>>> -    GLUEState *s = GLUE(n);
>>>> -
>>>> -    /* Hold NMI active for 100ms */
>>>> -    GLUE_set_irq(s, GLUE_IRQ_IN_NMI, 1);
>>>> -    timer_mod(s->nmi_release, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 
>>>> 100);
>>>> -}
>>>> -
>>>> -static void glue_nmi_release(void *opaque)
>>>> -{
>>>> -    GLUEState *s = GLUE(opaque);
>>>> -
>>>> -    GLUE_set_irq(s, GLUE_IRQ_IN_NMI, 0);
>>>> -}
>>>> -
>>>> -static void glue_reset(DeviceState *dev)
>>>> -{
>>>> -    GLUEState *s = GLUE(dev);
>>>> -
>>>> -    s->ipr = 0;
>>>> -    s->auxmode = 0;
>>>> -
>>>> -    timer_del(s->nmi_release);
>>>> -}
>>>> -
>>>> -static const VMStateDescription vmstate_glue = {
>>>> -    .name = "q800-glue",
>>>> -    .version_id = 0,
>>>> -    .minimum_version_id = 0,
>>>> -    .fields = (VMStateField[]) {
>>>> -        VMSTATE_UINT8(ipr, GLUEState),
>>>> -        VMSTATE_UINT8(auxmode, GLUEState),
>>>> -        VMSTATE_TIMER_PTR(nmi_release, GLUEState),
>>>> -        VMSTATE_END_OF_LIST(),
>>>> -    },
>>>> -};
>>>> -
>>>> -/*
>>>> - * If the m68k CPU implemented its inbound irq lines as GPIO lines
>>>> - * rather than via the m68k_set_irq_level() function we would not need
>>>> - * this cpu link property and could instead provide outbound IRQ lines
>>>> - * that the board could wire up to the CPU.
>>>> - */
>>>> -static Property glue_properties[] = {
>>>> -    DEFINE_PROP_LINK("cpu", GLUEState, cpu, TYPE_M68K_CPU, M68kCPU *),
>>>> -    DEFINE_PROP_END_OF_LIST(),
>>>> -};
>>>> -
>>>> -static void glue_finalize(Object *obj)
>>>> -{
>>>> -    GLUEState *s = GLUE(obj);
>>>> -
>>>> -    timer_free(s->nmi_release);
>>>> -}
>>>> -
>>>> -static void glue_init(Object *obj)
>>>> -{
>>>> -    DeviceState *dev = DEVICE(obj);
>>>> -    GLUEState *s = GLUE(dev);
>>>> -
>>>> -    qdev_init_gpio_in(dev, GLUE_set_irq, 8);
>>>> -    qdev_init_gpio_in_named(dev, glue_auxmode_set_irq, "auxmode", 1);
>>>> -
>>>> -    qdev_init_gpio_out(dev, s->irqs, 1);
>>>> -
>>>> -    /* NMI release timer */
>>>> -    s->nmi_release = timer_new_ms(QEMU_CLOCK_VIRTUAL, glue_nmi_release, 
>>>> s);
>>>> -}
>>>> -
>>>> -static void glue_class_init(ObjectClass *klass, void *data)
>>>> -{
>>>> -    DeviceClass *dc = DEVICE_CLASS(klass);
>>>> -    NMIClass *nc = NMI_CLASS(klass);
>>>> -
>>>> -    dc->vmsd = &vmstate_glue;
>>>> -    dc->reset = glue_reset;
>>>> -    device_class_set_props(dc, glue_properties);
>>>> -    nc->nmi_monitor_handler = glue_nmi;
>>>> -}
>>>> -
>>>> -static const TypeInfo glue_info = {
>>>> -    .name = TYPE_GLUE,
>>>> -    .parent = TYPE_SYS_BUS_DEVICE,
>>>> -    .instance_size = sizeof(GLUEState),
>>>> -    .instance_init = glue_init,
>>>> -    .instance_finalize = glue_finalize,
>>>> -    .class_init = glue_class_init,
>>>> -    .interfaces = (InterfaceInfo[]) {
>>>> -         { TYPE_NMI },
>>>> -         { }
>>>> -    },
>>>> -};
>>>> 
>>>> static void main_cpu_reset(void *opaque)
>>>> {
>>>> @@ -763,7 +528,6 @@ static const TypeInfo q800_machine_typeinfo = {
>>>> static void q800_machine_register_types(void)
>>>> {
>>>>     type_register_static(&q800_machine_typeinfo);
>>>> -    type_register_static(&glue_info);
>>>> }
>>>> 
>>>> type_init(q800_machine_register_types)
>>>> diff --git a/include/hw/m68k/q800-glue.h b/include/hw/m68k/q800-glue.h
>>>> new file mode 100644
>>>> index 0000000000..c1817b01a5
>>>> --- /dev/null
>>>> +++ b/include/hw/m68k/q800-glue.h
>>>> @@ -0,0 +1,50 @@
>>>> +/*
>>>> + * QEMU q800 logic glue
>>>> + *
>>>> + * Permission is hereby granted, free of charge, to any person obtaining 
>>>> a copy
>>>> + * of this software and associated documentation files (the "Software"), 
>>>> to deal
>>>> + * in the Software without restriction, including without limitation the 
>>>> rights
>>>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or 
>>>> sell
>>>> + * copies of the Software, and to permit persons to whom the Software is
>>>> + * furnished to do so, subject to the following conditions:
>>>> + *
>>>> + * The above copyright notice and this permission notice shall be 
>>>> included in
>>>> + * all copies or substantial portions of the Software.
>>>> + *
>>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
>>>> EXPRESS OR
>>>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
>>>> MERCHANTABILITY,
>>>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT 
>>>> SHALL
>>>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
>>>> OTHER
>>>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, 
>>>> ARISING FROM,
>>>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
>>>> DEALINGS IN
>>>> + * THE SOFTWARE.
>>>> + */
>>>> +
>>>> +#ifndef HW_Q800_GLUE_H
>>>> +#define HW_Q800_GLUE_H
>>>> +
>>>> +#include "qemu/osdep.h"
>>>> +#include "hw/sysbus.h"
>>>> +
>>>> +#define TYPE_GLUE "q800-glue"
>>>> +OBJECT_DECLARE_SIMPLE_TYPE(GLUEState, GLUE)
>>>> +
>>>> +struct GLUEState {
>>>> +    SysBusDevice parent_obj;
>>>> +
>>>> +    M68kCPU *cpu;
>>>> +    uint8_t ipr;
>>>> +    uint8_t auxmode;
>>>> +    qemu_irq irqs[1];
>>>> +    QEMUTimer *nmi_release;
>>>> +};
>>>> +
>>>> +#define GLUE_IRQ_IN_VIA1       0
>>>> +#define GLUE_IRQ_IN_VIA2       1
>>>> +#define GLUE_IRQ_IN_SONIC      2
>>>> +#define GLUE_IRQ_IN_ESCC       3
>>>> +#define GLUE_IRQ_IN_NMI        4
>>>> +
>>>> +#define GLUE_IRQ_NUBUS_9       0
>>>> +
>>>> +#endif
>> 
>> 
>> ATB,
>> 
>> Mark.
>> 
>

Re: [PATCH v4 05/24] q800: move CPU object into Q800MachineState

[Laurent Vivier]

Le 21/06/2023 à 16:28, Mark Cave-Ayland a écrit :
> On 21/06/2023 12:56, BALATON Zoltan wrote:
> 
>> On Wed, 21 Jun 2023, Mark Cave-Ayland wrote:
>>> Also change the instantiation of the CPU to use object_initialize_child()
>>> followed by a separate realisation.
>>
>> Also seems to restrict valid CPU types but not mentioned in commit message. Should this patch be 
>> split up?
> 
> Hmmm good point. Laurent, would you like me to split this into a separate patch or would updating 
> the commit message be sufficient?

I'm going to merge the series. Could you send me the update of the commit message?

Thanks,
Laurent

Re: [PATCH v4 00/24] q800: add support for booting MacOS Classic - part 1

[Laurent Vivier]

Le 21/06/2023 à 10:53, Mark Cave-Ayland a écrit :
> [MCA: the original series has now been split into 2 separate parts based upon
> Phil's comments re: QOM parenting for objects in Q800MachineState. Part 1
> consists of the Q800MachineState patches along with QOM parenting fixes and
> the 2 mac_via RTC patches.]
> 
> This series contains the remaining patches needed to allow QEMU's q800
> machine to boot MacOS Classic when used in conjunction with a real
> Quadra 800 ROM image. In fact with this series applied it is possible
> to boot all of the following OSs:
> 
>    - MacOS 7.1 - 8.1, with or without virtual memory enabled
>    - A/UX 3.0.1
>    - NetBSD 9.3
>    - Linux (via EMILE)
> 
> If you are ready to experience some 90s nostalgia then all you need is
> to grab yourself a copy of the Quadra 800 ROM (checksum 0xf1acad13) and a
> suitable install ISO as follows:
> 
>    # Prepare a PRAM image
>    $ qemu-img create -f raw pram.img 256b
> 
>    # Launch QEMU with blank disk and install CDROM
>    $ ./qemu-system-m68k \
>        -M q800 \
>        -m 128 \
>        -bios Quadra800.rom \
>        -drive file=pram.img,format=raw,if=mtd \
>        -drive file=disk.img,media=disk,format=raw,if=none,id=hd \
>        -device scsi-hd,scsi-id=0,drive=hd \
>        -drive file=cdrom.iso,media=cdrom,if=none,id=cd \
>        -device scsi-cd,scsi-id=3,drive=cd
> 
> And off you go! For more in-depth information about the installation process
> I highly recommend the installation guide over at emaculation.com [1].
> Compatibility is generally very good, and I'm pleased to report it is possible
> to run one of the most popular productivity apps from the 90s [2].
> 
> I'd like to add a big thank you to all the people who have helped me work on
> this series, including testing on real hardware, answering questions about
> MacOS Classic internals and helping to diagnose and fix bugs in the 68k
> emulation. In particular thanks go to Laurent Vivier, Finn Thain, Howard
> Spoelstra, Volker Rümelin, Richard Henderson, Martin Husemann, Rin Okuyama,
> Elliot Nunn, and SolraBizna.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> 
> [1] https://www.emaculation.com/doku.php/qemu
> [2] https://www.youtube.com/watch?v=yI21gURQ1Ew
> 
> 
> Patches missing review tags: 8 (new)
> 
> v4:
> - Rebase onto master
> - Add R-B tags from Phil and Laurent
> - Use qdev_realize() in patch 5 as suggested by Markus
> - Add new patch 8 to switch q800-glue.c to use the DEFINE_TYPES macro
>    as suggested by Phil
> 
> v3:
> - Add R-B tags from Phil and Laurent
> - Add missing headers in patches indicated by Phil
> - Change patch 5 to use valid_cpu_types Machine class property and the cpu_type
>    Machine property to initialise the CPU
> - Remove osdep.h header from dp8393x.h in patch 13 noticed by Phil
> - Change sysbus_realize_and_unref() to sysbus_realize() in patch 19
> - Use memory_region_add_subregion() instead of sysbus_mmio_map() in patch 19
> 
> v2:
> - Split series into 2 parts (this is part 1)
> - Update QOM parenting for objects in Q800MachineState (Phil)
> - Split GLUE device into separate glue.c and glue.h files
> - Split TYPE_DP8393X and dp8393xState into dp8393x.h
> - Add R-B tags from Laurent (where I still believe they are valid)
> 
> 
> Mark Cave-Ayland (24):
>    q800: fix up minor spacing issues in hw_compat_q800 GlobalProperty
>      array
>    q800: add missing space after parent object in GLUEState
>    q800: introduce Q800MachineState
>    q800: rename q800_init() to q800_machine_init()
>    q800: move CPU object into Q800MachineState
>    q800: move ROM memory region to Q800MachineState
>    q800: move GLUE device into separate q800-glue.c file
>    q800-glue.c: switch TypeInfo registration to use DEFINE_TYPES() macro
>    q800: move GLUE device to Q800MachineState
>    q800: introduce mac-io container memory region
>    q800: reimplement mac-io region aliasing using IO memory region
>    q800: move VIA1 device to Q800MachineState
>    q800: move VIA2 device to Q800MachineState
>    hw/net/dp8393x.c: move TYPE_DP8393X and dp8393xState into dp8393x.h
>    q800: move dp8393x device to Q800MachineState
>    q800: move ESCC device to Q800MachineState
>    q800: move escc_orgate device to Q800MachineState
>    q800: move ESP device to Q800MachineState
>    q800: move SWIM device to Q800MachineState
>    q800: move mac-nubus-bridge device to Q800MachineState
>    q800: don't access Nubus bus directly from the mac-nubus-bridge device
>    q800: move macfb device to Q800MachineState
>    mac_via: fix rtc command decoding from PRAM addresses 0x0 to 0xf
>    mac_via: fix rtc command decoding for the PRAM seconds registers
> 
>   MAINTAINERS                 |   3 +
>   hw/m68k/meson.build         |   2 +-
>   hw/m68k/q800-glue.c         | 249 +++++++++++++++++
>   hw/m68k/q800.c              | 526 ++++++++++++++----------------------
>   hw/misc/mac_via.c           |  13 +-
>   hw/net/dp8393x.c            |  32 +--
>   include/hw/m68k/q800-glue.h |  50 ++++
>   include/hw/m68k/q800.h      |  66 +++++
>   include/hw/net/dp8393x.h    |  60 ++++
>   9 files changed, 635 insertions(+), 366 deletions(-)
>   create mode 100644 hw/m68k/q800-glue.c
>   create mode 100644 include/hw/m68k/q800-glue.h
>   create mode 100644 include/hw/m68k/q800.h
>   create mode 100644 include/hw/net/dp8393x.h
> 

Queued

Thanks,
Laurent

Re: [PATCH v4 00/24] q800: add support for booting MacOS Classic - part 1

[Philippe Mathieu-Daudé]

On 21/6/23 16:06, Mark Cave-Ayland wrote:
> On 21/06/2023 10:42, Philippe Mathieu-Daudé wrote:
> 
>> On 21/6/23 10:53, Mark Cave-Ayland wrote:
>>> [MCA: the original series has now been split into 2 separate parts 
>>> based upon
>>> Phil's comments re: QOM parenting for objects in Q800MachineState. 
>>> Part 1
>>> consists of the Q800MachineState patches along with QOM parenting 
>>> fixes and
>>> the 2 mac_via RTC patches.]
>>>
>>> This series contains the remaining patches needed to allow QEMU's q800
>>> machine to boot MacOS Classic when used in conjunction with a real
>>> Quadra 800 ROM image. In fact with this series applied it is possible
>>> to boot all of the following OSs:
>>>
>>>    - MacOS 7.1 - 8.1, with or without virtual memory enabled
>>>    - A/UX 3.0.1
>>>    - NetBSD 9.3
>>>    - Linux (via EMILE)
>>>
>>> If you are ready to experience some 90s nostalgia then all you need is
>>> to grab yourself a copy of the Quadra 800 ROM (checksum 0xf1acad13) 
>>> and a
>>> suitable install ISO as follows:
>>>
>>>    # Prepare a PRAM image
>>>    $ qemu-img create -f raw pram.img 256b
>>>
>>>    # Launch QEMU with blank disk and install CDROM
>>>    $ ./qemu-system-m68k \
>>>        -M q800 \
>>>        -m 128 \
>>>        -bios Quadra800.rom \
>>>        -drive file=pram.img,format=raw,if=mtd \
>>>        -drive file=disk.img,media=disk,format=raw,if=none,id=hd \
>>>        -device scsi-hd,scsi-id=0,drive=hd \
>>>        -drive file=cdrom.iso,media=cdrom,if=none,id=cd \
>>>        -device scsi-cd,scsi-id=3,drive=cd
>>>
>>> And off you go! For more in-depth information about the installation 
>>> process
>>> I highly recommend the installation guide over at emaculation.com [1].
>>> Compatibility is generally very good, and I'm pleased to report it is 
>>> possible
>>> to run one of the most popular productivity apps from the 90s [2].
>>
>> Could you add an Avocado test for this machine? See how the
>> MipsFuloong2e test (tests/avocado/machine_mips_fuloong2e.py)
>> handles the firmware (RESCUE_YL_PATH).
> 
> In theory it is possible to do this, but how do we handle booting 
> proprietary OS ISOs that are still within copyright?

Just provide the hash. You are not redistributing anything.

> Also this is only 
> part 1 of the required changes, you'll need part 2 applied in order to 
> achieve a successful boot :)
> 
> 
> ATB,
> 
> Mark.
> 

Re: [PATCH v4 00/24] q800: add support for booting MacOS Classic - part 1

[Mark Cave-Ayland]

On 22/06/2023 11:07, Philippe Mathieu-Daudé wrote:

> On 21/6/23 16:06, Mark Cave-Ayland wrote:
>> On 21/06/2023 10:42, Philippe Mathieu-Daudé wrote:
>>
>>> On 21/6/23 10:53, Mark Cave-Ayland wrote:
>>>> [MCA: the original series has now been split into 2 separate parts based upon
>>>> Phil's comments re: QOM parenting for objects in Q800MachineState. Part 1
>>>> consists of the Q800MachineState patches along with QOM parenting fixes and
>>>> the 2 mac_via RTC patches.]
>>>>
>>>> This series contains the remaining patches needed to allow QEMU's q800
>>>> machine to boot MacOS Classic when used in conjunction with a real
>>>> Quadra 800 ROM image. In fact with this series applied it is possible
>>>> to boot all of the following OSs:
>>>>
>>>>    - MacOS 7.1 - 8.1, with or without virtual memory enabled
>>>>    - A/UX 3.0.1
>>>>    - NetBSD 9.3
>>>>    - Linux (via EMILE)
>>>>
>>>> If you are ready to experience some 90s nostalgia then all you need is
>>>> to grab yourself a copy of the Quadra 800 ROM (checksum 0xf1acad13) and a
>>>> suitable install ISO as follows:
>>>>
>>>>    # Prepare a PRAM image
>>>>    $ qemu-img create -f raw pram.img 256b
>>>>
>>>>    # Launch QEMU with blank disk and install CDROM
>>>>    $ ./qemu-system-m68k \
>>>>        -M q800 \
>>>>        -m 128 \
>>>>        -bios Quadra800.rom \
>>>>        -drive file=pram.img,format=raw,if=mtd \
>>>>        -drive file=disk.img,media=disk,format=raw,if=none,id=hd \
>>>>        -device scsi-hd,scsi-id=0,drive=hd \
>>>>        -drive file=cdrom.iso,media=cdrom,if=none,id=cd \
>>>>        -device scsi-cd,scsi-id=3,drive=cd
>>>>
>>>> And off you go! For more in-depth information about the installation process
>>>> I highly recommend the installation guide over at emaculation.com [1].
>>>> Compatibility is generally very good, and I'm pleased to report it is possible
>>>> to run one of the most popular productivity apps from the 90s [2].
>>>
>>> Could you add an Avocado test for this machine? See how the
>>> MipsFuloong2e test (tests/avocado/machine_mips_fuloong2e.py)
>>> handles the firmware (RESCUE_YL_PATH).
>>
>> In theory it is possible to do this, but how do we handle booting proprietary OS 
>> ISOs that are still within copyright?
> 
> Just provide the hash. You are not redistributing anything.

Another couple of questions: even without distributing the ROM or ISO, do I still 
need to provide URLs to these resources? I see that ISOs generally have URLs but 
given that the Mac sites hosting these images tend to be quite small, I'm not sure 
that providing a link to these sites within QEMU is something they would be 
particularly happy with.

I'm also not sure what is the best way to set up the test to confirm boot is working 
correctly. Ideally we would want to boot a MacOS install CD to the desktop, but there 
isn't an easy way to determine when this has occurred. Perhaps take a screendump 
every 2s or so and use tesseract-ocr to parse for when the menu bar appears on screen?


ATB,

Mark.