[PATCH v2 00/13] Misc ppc/mac machines clean up

[PATCH v2 00/13] Misc ppc/mac machines clean up

[BALATON Zoltan]

This series includes some clean ups to mac_newworld and mac_oldworld
to make them a bit simpler and more readable, It also removes the
shared mac.h file that turns out was more of a random collection of
unrelated things. Getting rid of this mac.h improves the locality of
device models and reduces unnecessary interdependency.

v2: Split some patches and add a few more I've noticed now and address
review comments

BALATON Zoltan (13):
  mac_newworld: Drop some variables
  mac_oldworld: Drop some more variables
  mac_{old|new}world: Set tbfreq at declaration
  mac_{old|new}world: Avoid else branch by setting default value
  mac_oldworld: Do not open code sysbus_mmio_map()
  mac_newworld: Simplify creation of Uninorth devices
  mac_{old|new}world: Reduce number of QOM casts
  hw/ppc/mac.h: Move newworld specific parts out from shared header
  hw/ppc/mac.h: Move macio specific parts out from shared header
  hw/ppc/mac.h: Move grackle-pcihost declaration out from shared header
  hw/ppc/mac.h: Move PROM and KERNEL defines to board code
  hw/ppc/mac.h: Rename to include/hw/nvram/mac_nvram.h
  mac_nvram: Use NVRAM_SIZE constant

 MAINTAINERS                   |   1 +
 hw/ide/macio.c                |   1 -
 hw/intc/heathrow_pic.c        |   1 -
 hw/intc/openpic.c             |   1 -
 hw/misc/macio/cuda.c          |   1 -
 hw/misc/macio/gpio.c          |   1 -
 hw/misc/macio/macio.c         |   8 +-
 hw/misc/macio/pmu.c           |   1 -
 hw/nvram/mac_nvram.c          |   2 +-
 hw/pci-host/grackle.c         |   2 +-
 hw/pci-host/uninorth.c        |   1 -
 hw/ppc/mac.h                  | 105 ----------------
 hw/ppc/mac_newworld.c         | 223 ++++++++++++++++------------------
 hw/ppc/mac_oldworld.c         | 113 +++++++----------
 include/hw/misc/macio/macio.h |  23 +++-
 include/hw/nvram/mac_nvram.h  |  51 ++++++++
 16 files changed, 230 insertions(+), 305 deletions(-)
 delete mode 100644 hw/ppc/mac.h
 create mode 100644 include/hw/nvram/mac_nvram.h

-- 
2.30.4

[PATCH v2 01/13] mac_newworld: Drop some variables

[BALATON Zoltan]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 7766 bytes --]

Values not used frequently enough may not worth putting in a local
variable, especially with names almost as long as the original value
because that does not improve readability, to the contrary it makes it
harder to see what value is used. Drop a few such variables. This is
the same clean up that was done for mac_oldworld in commit b8df32555ce5.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/ppc/mac_newworld.c | 65 +++++++++++++++++++------------------------
 1 file changed, 29 insertions(+), 36 deletions(-)

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index cf7eb72391..27e4e8d136 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -106,18 +106,13 @@ static void ppc_core99_reset(void *opaque)
 /* PowerPC Mac99 hardware initialisation */
 static void ppc_core99_init(MachineState *machine)
 {
-    ram_addr_t ram_size = machine->ram_size;
-    const char *bios_name = machine->firmware ?: PROM_FILENAME;
-    const char *kernel_filename = machine->kernel_filename;
-    const char *kernel_cmdline = machine->kernel_cmdline;
-    const char *initrd_filename = machine->initrd_filename;
-    const char *boot_device = machine->boot_config.order;
     Core99MachineState *core99_machine = CORE99_MACHINE(machine);
     PowerPCCPU *cpu = NULL;
     CPUPPCState *env = NULL;
     char *filename;
     IrqLines *openpic_irqs;
-    int linux_boot, i, j, k;
+    int i, j, k, ppc_boot_device, machine_arch, bios_size;
+    const char *bios_name = machine->firmware ?: PROM_FILENAME;
     MemoryRegion *bios = g_new(MemoryRegion, 1);
     hwaddr kernel_base, initrd_base, cmdline_base = 0;
     long kernel_size, initrd_size;
@@ -129,22 +124,16 @@ static void ppc_core99_init(MachineState *machine)
     MACIOIDEState *macio_ide;
     BusState *adb_bus;
     MacIONVRAMState *nvr;
-    int bios_size;
-    int ppc_boot_device;
     DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
     void *fw_cfg;
-    int machine_arch;
     SysBusDevice *s;
     DeviceState *dev, *pic_dev;
     DeviceState *uninorth_internal_dev = NULL, *uninorth_agp_dev = NULL;
     hwaddr nvram_addr = 0xFFF04000;
     uint64_t tbfreq;
-    unsigned int smp_cpus = machine->smp.cpus;
-
-    linux_boot = (kernel_filename != NULL);
 
     /* init CPUs */
-    for (i = 0; i < smp_cpus; i++) {
+    for (i = 0; i < machine->smp.cpus; i++) {
         cpu = POWERPC_CPU(cpu_create(machine->cpu_type));
         env = &cpu->env;
 
@@ -184,7 +173,7 @@ static void ppc_core99_init(MachineState *machine)
         exit(1);
     }
 
-    if (linux_boot) {
+    if (machine->kernel_filename) {
         int bswap_needed;
 
 #ifdef BSWAP_NEEDED
@@ -194,29 +183,31 @@ static void ppc_core99_init(MachineState *machine)
 #endif
         kernel_base = KERNEL_LOAD_ADDR;
 
-        kernel_size = load_elf(kernel_filename, NULL,
+        kernel_size = load_elf(machine->kernel_filename, NULL,
                                translate_kernel_address, NULL, NULL, NULL,
                                NULL, NULL, 1, PPC_ELF_MACHINE, 0, 0);
         if (kernel_size < 0)
-            kernel_size = load_aout(kernel_filename, kernel_base,
-                                    ram_size - kernel_base, bswap_needed,
-                                    TARGET_PAGE_SIZE);
+            kernel_size = load_aout(machine->kernel_filename, kernel_base,
+                                    machine->ram_size - kernel_base,
+                                    bswap_needed, TARGET_PAGE_SIZE);
         if (kernel_size < 0)
-            kernel_size = load_image_targphys(kernel_filename,
+            kernel_size = load_image_targphys(machine->kernel_filename,
                                               kernel_base,
-                                              ram_size - kernel_base);
+                                              machine->ram_size - kernel_base);
         if (kernel_size < 0) {
-            error_report("could not load kernel '%s'", kernel_filename);
+            error_report("could not load kernel '%s'",
+                         machine->kernel_filename);
             exit(1);
         }
         /* load initrd */
-        if (initrd_filename) {
+        if (machine->initrd_filename) {
             initrd_base = TARGET_PAGE_ALIGN(kernel_base + kernel_size + KERNEL_GAP);
-            initrd_size = load_image_targphys(initrd_filename, initrd_base,
-                                              ram_size - initrd_base);
+            initrd_size = load_image_targphys(machine->initrd_filename,
+                                              initrd_base,
+                                              machine->ram_size - initrd_base);
             if (initrd_size < 0) {
                 error_report("could not load initial ram disk '%s'",
-                             initrd_filename);
+                             machine->initrd_filename);
                 exit(1);
             }
             cmdline_base = TARGET_PAGE_ALIGN(initrd_base + initrd_size);
@@ -235,9 +226,10 @@ static void ppc_core99_init(MachineState *machine)
         /* We consider that NewWorld PowerMac never have any floppy drive
          * For now, OHW cannot boot from the network.
          */
-        for (i = 0; boot_device[i] != '\0'; i++) {
-            if (boot_device[i] >= 'c' && boot_device[i] <= 'f') {
-                ppc_boot_device = boot_device[i];
+        for (i = 0; machine->boot_config.order[i] != '\0'; i++) {
+            if (machine->boot_config.order[i] >= 'c' &&
+                machine->boot_config.order[i] <= 'f') {
+                ppc_boot_device = machine->boot_config.order[i];
                 break;
             }
         }
@@ -254,8 +246,8 @@ static void ppc_core99_init(MachineState *machine)
     memory_region_add_subregion(get_system_memory(), 0xf8000000,
                                 sysbus_mmio_get_region(s, 0));
 
-    openpic_irqs = g_new0(IrqLines, smp_cpus);
-    for (i = 0; i < smp_cpus; i++) {
+    openpic_irqs = g_new0(IrqLines, machine->smp.cpus);
+    for (i = 0; i < machine->smp.cpus; i++) {
         /* Mac99 IRQ connection between OpenPIC outputs pins
          * and PowerPC input pins
          */
@@ -398,7 +390,7 @@ static void ppc_core99_init(MachineState *machine)
     /* OpenPIC */
     s = SYS_BUS_DEVICE(pic_dev);
     k = 0;
-    for (i = 0; i < smp_cpus; i++) {
+    for (i = 0; i < machine->smp.cpus; i++) {
         for (j = 0; j < OPENPIC_OUTPUT_NB; j++) {
             sysbus_connect_irq(s, k++, openpic_irqs[i].irq[j]);
         }
@@ -480,15 +472,16 @@ static void ppc_core99_init(MachineState *machine)
     sysbus_mmio_map(s, 0, CFG_ADDR);
     sysbus_mmio_map(s, 1, CFG_ADDR + 2);
 
-    fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
+    fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)machine->smp.cpus);
     fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)machine->smp.max_cpus);
-    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
+    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)machine->ram_size);
     fw_cfg_add_i16(fw_cfg, FW_CFG_MACHINE_ID, machine_arch);
     fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, kernel_base);
     fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, kernel_size);
-    if (kernel_cmdline) {
+    if (machine->kernel_cmdline) {
         fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, cmdline_base);
-        pstrcpy_targphys("cmdline", cmdline_base, TARGET_PAGE_SIZE, kernel_cmdline);
+        pstrcpy_targphys("cmdline", cmdline_base, TARGET_PAGE_SIZE,
+                         machine->kernel_cmdline);
     } else {
         fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, 0);
     }
-- 
2.30.4

[PATCH v2 02/13] mac_oldworld: Drop some more variables

[BALATON Zoltan]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 6091 bytes --]

Drop some more local variables additionally to commit b8df32555ce5 to
match clean ups done to mac_newwold in previous patch.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/ppc/mac_oldworld.c | 43 +++++++++++++++++++++----------------------
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 03732ca7ed..86512d31ad 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -80,14 +80,13 @@ static void ppc_heathrow_reset(void *opaque)
 
 static void ppc_heathrow_init(MachineState *machine)
 {
-    ram_addr_t ram_size = machine->ram_size;
     const char *bios_name = machine->firmware ?: PROM_FILENAME;
-    const char *boot_device = machine->boot_config.order;
     PowerPCCPU *cpu = NULL;
     CPUPPCState *env = NULL;
     char *filename;
-    int i;
+    int i, bios_size;
     MemoryRegion *bios = g_new(MemoryRegion, 1);
+    uint64_t bios_addr;
     uint32_t kernel_base, initrd_base, cmdline_base = 0;
     int32_t kernel_size, initrd_size;
     PCIBus *pci_bus;
@@ -97,16 +96,13 @@ static void ppc_heathrow_init(MachineState *machine)
     SysBusDevice *s;
     DeviceState *dev, *pic_dev, *grackle_dev;
     BusState *adb_bus;
-    uint64_t bios_addr;
-    int bios_size;
-    unsigned int smp_cpus = machine->smp.cpus;
     uint16_t ppc_boot_device;
     DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
     void *fw_cfg;
     uint64_t tbfreq;
 
     /* init CPUs */
-    for (i = 0; i < smp_cpus; i++) {
+    for (i = 0; i < machine->smp.cpus; i++) {
         cpu = POWERPC_CPU(cpu_create(machine->cpu_type));
         env = &cpu->env;
 
@@ -116,9 +112,9 @@ static void ppc_heathrow_init(MachineState *machine)
     }
 
     /* allocate RAM */
-    if (ram_size > 2047 * MiB) {
+    if (machine->ram_size > 2047 * MiB) {
         error_report("Too much memory for this machine: %" PRId64 " MB, "
-                     "maximum 2047 MB", ram_size / MiB);
+                     "maximum 2047 MB", machine->ram_size / MiB);
         exit(1);
     }
 
@@ -165,12 +161,12 @@ static void ppc_heathrow_init(MachineState *machine)
                                NULL, NULL, 1, PPC_ELF_MACHINE, 0, 0);
         if (kernel_size < 0)
             kernel_size = load_aout(machine->kernel_filename, kernel_base,
-                                    ram_size - kernel_base, bswap_needed,
-                                    TARGET_PAGE_SIZE);
+                                    machine->ram_size - kernel_base,
+                                    bswap_needed, TARGET_PAGE_SIZE);
         if (kernel_size < 0)
             kernel_size = load_image_targphys(machine->kernel_filename,
                                               kernel_base,
-                                              ram_size - kernel_base);
+                                              machine->ram_size - kernel_base);
         if (kernel_size < 0) {
             error_report("could not load kernel '%s'",
                          machine->kernel_filename);
@@ -182,7 +178,7 @@ static void ppc_heathrow_init(MachineState *machine)
                                             KERNEL_GAP);
             initrd_size = load_image_targphys(machine->initrd_filename,
                                               initrd_base,
-                                              ram_size - initrd_base);
+                                              machine->ram_size - initrd_base);
             if (initrd_size < 0) {
                 error_report("could not load initial ram disk '%s'",
                              machine->initrd_filename);
@@ -201,19 +197,22 @@ static void ppc_heathrow_init(MachineState *machine)
         initrd_base = 0;
         initrd_size = 0;
         ppc_boot_device = '\0';
-        for (i = 0; boot_device[i] != '\0'; i++) {
-            /* TOFIX: for now, the second IDE channel is not properly
+        for (i = 0; machine->boot_config.order[i] != '\0'; i++) {
+            /*
+             * TOFIX: for now, the second IDE channel is not properly
              *        used by OHW. The Mac floppy disk are not emulated.
              *        For now, OHW cannot boot from the network.
              */
 #if 0
-            if (boot_device[i] >= 'a' && boot_device[i] <= 'f') {
-                ppc_boot_device = boot_device[i];
+            if (machine->boot_config.order[i] >= 'a' &&
+                machine->boot_config.order[i] <= 'f') {
+                ppc_boot_device = machine->boot_config.order[i];
                 break;
             }
 #else
-            if (boot_device[i] >= 'c' && boot_device[i] <= 'd') {
-                ppc_boot_device = boot_device[i];
+            if (machine->boot_config.order[i] >= 'c' &&
+                machine->boot_config.order[i] <= 'd') {
+                ppc_boot_device = machine->boot_config.order[i];
                 break;
             }
 #endif
@@ -266,7 +265,7 @@ static void ppc_heathrow_init(MachineState *machine)
     }
 
     /* Connect the heathrow PIC outputs to the 6xx bus */
-    for (i = 0; i < smp_cpus; i++) {
+    for (i = 0; i < machine->smp.cpus; i++) {
         switch (PPC_INPUT(env)) {
         case PPC_FLAGS_INPUT_6xx:
             /* XXX: we register only 1 output pin for heathrow PIC */
@@ -323,9 +322,9 @@ static void ppc_heathrow_init(MachineState *machine)
     sysbus_mmio_map(s, 0, CFG_ADDR);
     sysbus_mmio_map(s, 1, CFG_ADDR + 2);
 
-    fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
+    fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)machine->smp.cpus);
     fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)machine->smp.max_cpus);
-    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
+    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)machine->ram_size);
     fw_cfg_add_i16(fw_cfg, FW_CFG_MACHINE_ID, ARCH_HEATHROW);
     fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, kernel_base);
     fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, kernel_size);
-- 
2.30.4

[PATCH v2 03/13] mac_{old|new}world: Set tbfreq at declaration

[BALATON Zoltan]

The tbfreq variable is only set once in an if-else which can be done
at the variable declaration saving some lines of code and making it
simpler.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/ppc/mac_newworld.c | 9 +--------
 hw/ppc/mac_oldworld.c | 9 +--------
 2 files changed, 2 insertions(+), 16 deletions(-)

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 27e4e8d136..6327694f85 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -130,7 +130,7 @@ static void ppc_core99_init(MachineState *machine)
     DeviceState *dev, *pic_dev;
     DeviceState *uninorth_internal_dev = NULL, *uninorth_agp_dev = NULL;
     hwaddr nvram_addr = 0xFFF04000;
-    uint64_t tbfreq;
+    uint64_t tbfreq = kvm_enabled() ? kvmppc_get_tbfreq() : TBFREQ;
 
     /* init CPUs */
     for (i = 0; i < machine->smp.cpus; i++) {
@@ -343,13 +343,6 @@ static void ppc_core99_init(MachineState *machine)
     has_adb = (core99_machine->via_config == CORE99_VIA_CONFIG_CUDA ||
                core99_machine->via_config == CORE99_VIA_CONFIG_PMU_ADB);
 
-    /* Timebase Frequency */
-    if (kvm_enabled()) {
-        tbfreq = kvmppc_get_tbfreq();
-    } else {
-        tbfreq = TBFREQ;
-    }
-
     /* init basic PC hardware */
     pci_bus = PCI_HOST_BRIDGE(uninorth_pci)->bus;
 
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 86512d31ad..5cabc410e7 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -99,7 +99,7 @@ static void ppc_heathrow_init(MachineState *machine)
     uint16_t ppc_boot_device;
     DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
     void *fw_cfg;
-    uint64_t tbfreq;
+    uint64_t tbfreq = kvm_enabled() ? kvmppc_get_tbfreq() : TBFREQ;
 
     /* init CPUs */
     for (i = 0; i < machine->smp.cpus; i++) {
@@ -223,13 +223,6 @@ static void ppc_heathrow_init(MachineState *machine)
         }
     }
 
-    /* Timebase Frequency */
-    if (kvm_enabled()) {
-        tbfreq = kvmppc_get_tbfreq();
-    } else {
-        tbfreq = TBFREQ;
-    }
-
     /* Grackle PCI host bridge */
     grackle_dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
     qdev_prop_set_uint32(grackle_dev, "ofw-addr", 0x80000000);
-- 
2.30.4

Re: [PATCH v2 03/13] mac_{old|new}world: Set tbfreq at declaration

[Mark Cave-Ayland]

On 25/09/2022 13:38, BALATON Zoltan wrote:

> The tbfreq variable is only set once in an if-else which can be done
> at the variable declaration saving some lines of code and making it
> simpler.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>   hw/ppc/mac_newworld.c | 9 +--------
>   hw/ppc/mac_oldworld.c | 9 +--------
>   2 files changed, 2 insertions(+), 16 deletions(-)
> 
> diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
> index 27e4e8d136..6327694f85 100644
> --- a/hw/ppc/mac_newworld.c
> +++ b/hw/ppc/mac_newworld.c
> @@ -130,7 +130,7 @@ static void ppc_core99_init(MachineState *machine)
>       DeviceState *dev, *pic_dev;
>       DeviceState *uninorth_internal_dev = NULL, *uninorth_agp_dev = NULL;
>       hwaddr nvram_addr = 0xFFF04000;
> -    uint64_t tbfreq;
> +    uint64_t tbfreq = kvm_enabled() ? kvmppc_get_tbfreq() : TBFREQ;
>   
>       /* init CPUs */
>       for (i = 0; i < machine->smp.cpus; i++) {
> @@ -343,13 +343,6 @@ static void ppc_core99_init(MachineState *machine)
>       has_adb = (core99_machine->via_config == CORE99_VIA_CONFIG_CUDA ||
>                  core99_machine->via_config == CORE99_VIA_CONFIG_PMU_ADB);
>   
> -    /* Timebase Frequency */
> -    if (kvm_enabled()) {
> -        tbfreq = kvmppc_get_tbfreq();
> -    } else {
> -        tbfreq = TBFREQ;
> -    }
> -
>       /* init basic PC hardware */
>       pci_bus = PCI_HOST_BRIDGE(uninorth_pci)->bus;
>   
> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
> index 86512d31ad..5cabc410e7 100644
> --- a/hw/ppc/mac_oldworld.c
> +++ b/hw/ppc/mac_oldworld.c
> @@ -99,7 +99,7 @@ static void ppc_heathrow_init(MachineState *machine)
>       uint16_t ppc_boot_device;
>       DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
>       void *fw_cfg;
> -    uint64_t tbfreq;
> +    uint64_t tbfreq = kvm_enabled() ? kvmppc_get_tbfreq() : TBFREQ;
>   
>       /* init CPUs */
>       for (i = 0; i < machine->smp.cpus; i++) {
> @@ -223,13 +223,6 @@ static void ppc_heathrow_init(MachineState *machine)
>           }
>       }
>   
> -    /* Timebase Frequency */
> -    if (kvm_enabled()) {
> -        tbfreq = kvmppc_get_tbfreq();
> -    } else {
> -        tbfreq = TBFREQ;
> -    }
> -
>       /* Grackle PCI host bridge */
>       grackle_dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
>       qdev_prop_set_uint32(grackle_dev, "ofw-addr", 0x80000000);

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


ATB,

Mark.

[PATCH v2 04/13] mac_{old|new}world: Avoid else branch by setting default value

[BALATON Zoltan]

Several variables are set in if-else branches where the else branch
can be removed by setting a default value at the variable declaration
which leads to simlpler code that is easier to follow.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/ppc/mac_newworld.c | 19 ++++---------------
 hw/ppc/mac_oldworld.c | 18 ++++--------------
 2 files changed, 8 insertions(+), 29 deletions(-)

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 6327694f85..6bc3bd19be 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -111,11 +111,11 @@ static void ppc_core99_init(MachineState *machine)
     CPUPPCState *env = NULL;
     char *filename;
     IrqLines *openpic_irqs;
-    int i, j, k, ppc_boot_device, machine_arch, bios_size;
+    int i, j, k, ppc_boot_device, machine_arch, bios_size = -1;
     const char *bios_name = machine->firmware ?: PROM_FILENAME;
     MemoryRegion *bios = g_new(MemoryRegion, 1);
-    hwaddr kernel_base, initrd_base, cmdline_base = 0;
-    long kernel_size, initrd_size;
+    hwaddr kernel_base = 0, initrd_base = 0, cmdline_base = 0;
+    long kernel_size = 0, initrd_size = 0;
     UNINHostState *uninorth_pci;
     PCIBus *pci_bus;
     PCIDevice *macio;
@@ -165,8 +165,6 @@ static void ppc_core99_init(MachineState *machine)
             bios_size = load_image_targphys(filename, PROM_BASE, PROM_SIZE);
         }
         g_free(filename);
-    } else {
-        bios_size = -1;
     }
     if (bios_size < 0 || bios_size > PROM_SIZE) {
         error_report("could not load PowerPC bios '%s'", bios_name);
@@ -174,15 +172,12 @@ static void ppc_core99_init(MachineState *machine)
     }
 
     if (machine->kernel_filename) {
-        int bswap_needed;
+        int bswap_needed = 0;
 
 #ifdef BSWAP_NEEDED
         bswap_needed = 1;
-#else
-        bswap_needed = 0;
 #endif
         kernel_base = KERNEL_LOAD_ADDR;
-
         kernel_size = load_elf(machine->kernel_filename, NULL,
                                translate_kernel_address, NULL, NULL, NULL,
                                NULL, NULL, 1, PPC_ELF_MACHINE, 0, 0);
@@ -212,16 +207,10 @@ static void ppc_core99_init(MachineState *machine)
             }
             cmdline_base = TARGET_PAGE_ALIGN(initrd_base + initrd_size);
         } else {
-            initrd_base = 0;
-            initrd_size = 0;
             cmdline_base = TARGET_PAGE_ALIGN(kernel_base + kernel_size + KERNEL_GAP);
         }
         ppc_boot_device = 'm';
     } else {
-        kernel_base = 0;
-        kernel_size = 0;
-        initrd_base = 0;
-        initrd_size = 0;
         ppc_boot_device = '\0';
         /* We consider that NewWorld PowerMac never have any floppy drive
          * For now, OHW cannot boot from the network.
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 5cabc410e7..cb67e44081 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -84,11 +84,11 @@ static void ppc_heathrow_init(MachineState *machine)
     PowerPCCPU *cpu = NULL;
     CPUPPCState *env = NULL;
     char *filename;
-    int i, bios_size;
+    int i, bios_size = -1;
     MemoryRegion *bios = g_new(MemoryRegion, 1);
     uint64_t bios_addr;
-    uint32_t kernel_base, initrd_base, cmdline_base = 0;
-    int32_t kernel_size, initrd_size;
+    uint32_t kernel_base = 0, initrd_base = 0, cmdline_base = 0;
+    int32_t kernel_size = 0, initrd_size = 0;
     PCIBus *pci_bus;
     PCIDevice *macio;
     MACIOIDEState *macio_ide;
@@ -139,8 +139,6 @@ static void ppc_heathrow_init(MachineState *machine)
             bios_addr = PROM_BASE;
         }
         g_free(filename);
-    } else {
-        bios_size = -1;
     }
     if (bios_size < 0 || bios_addr - PROM_BASE + bios_size > PROM_SIZE) {
         error_report("could not load PowerPC bios '%s'", bios_name);
@@ -148,12 +146,10 @@ static void ppc_heathrow_init(MachineState *machine)
     }
 
     if (machine->kernel_filename) {
-        int bswap_needed;
+        int bswap_needed = 0;
 
 #ifdef BSWAP_NEEDED
         bswap_needed = 1;
-#else
-        bswap_needed = 0;
 #endif
         kernel_base = KERNEL_LOAD_ADDR;
         kernel_size = load_elf(machine->kernel_filename, NULL,
@@ -186,16 +182,10 @@ static void ppc_heathrow_init(MachineState *machine)
             }
             cmdline_base = TARGET_PAGE_ALIGN(initrd_base + initrd_size);
         } else {
-            initrd_base = 0;
-            initrd_size = 0;
             cmdline_base = TARGET_PAGE_ALIGN(kernel_base + kernel_size + KERNEL_GAP);
         }
         ppc_boot_device = 'm';
     } else {
-        kernel_base = 0;
-        kernel_size = 0;
-        initrd_base = 0;
-        initrd_size = 0;
         ppc_boot_device = '\0';
         for (i = 0; machine->boot_config.order[i] != '\0'; i++) {
             /*
-- 
2.30.4

[PATCH v2 05/13] mac_oldworld: Do not open code sysbus_mmio_map()

[BALATON Zoltan]

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/ppc/mac_oldworld.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index cb67e44081..75fbd2a7df 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -218,16 +218,12 @@ static void ppc_heathrow_init(MachineState *machine)
     qdev_prop_set_uint32(grackle_dev, "ofw-addr", 0x80000000);
     s = SYS_BUS_DEVICE(grackle_dev);
     sysbus_realize_and_unref(s, &error_fatal);
-
     sysbus_mmio_map(s, 0, GRACKLE_BASE);
     sysbus_mmio_map(s, 1, GRACKLE_BASE + 0x200000);
     /* PCI hole */
-    memory_region_add_subregion(get_system_memory(), 0x80000000ULL,
-                                sysbus_mmio_get_region(s, 2));
+    sysbus_mmio_map(s, 2, 0x80000000);
     /* Register 2 MB of ISA IO space */
-    memory_region_add_subregion(get_system_memory(), 0xfe000000,
-                                sysbus_mmio_get_region(s, 3));
-
+    sysbus_mmio_map(s, 3, 0xfe000000);
     pci_bus = PCI_HOST_BRIDGE(grackle_dev)->bus;
 
     /* MacIO */
-- 
2.30.4

Re: [PATCH v2 05/13] mac_oldworld: Do not open code sysbus_mmio_map()

[Mark Cave-Ayland]

On 25/09/2022 13:38, BALATON Zoltan wrote:

> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>   hw/ppc/mac_oldworld.c | 8 ++------
>   1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
> index cb67e44081..75fbd2a7df 100644
> --- a/hw/ppc/mac_oldworld.c
> +++ b/hw/ppc/mac_oldworld.c
> @@ -218,16 +218,12 @@ static void ppc_heathrow_init(MachineState *machine)
>       qdev_prop_set_uint32(grackle_dev, "ofw-addr", 0x80000000);
>       s = SYS_BUS_DEVICE(grackle_dev);
>       sysbus_realize_and_unref(s, &error_fatal);
> -
>       sysbus_mmio_map(s, 0, GRACKLE_BASE);
>       sysbus_mmio_map(s, 1, GRACKLE_BASE + 0x200000);
>       /* PCI hole */
> -    memory_region_add_subregion(get_system_memory(), 0x80000000ULL,
> -                                sysbus_mmio_get_region(s, 2));
> +    sysbus_mmio_map(s, 2, 0x80000000);
>       /* Register 2 MB of ISA IO space */
> -    memory_region_add_subregion(get_system_memory(), 0xfe000000,
> -                                sysbus_mmio_get_region(s, 3));
> -
> +    sysbus_mmio_map(s, 3, 0xfe000000);
>       pci_bus = PCI_HOST_BRIDGE(grackle_dev)->bus;
>   
>       /* MacIO */

Please drop this patch for now. The code was written on assumption that both sysbus 
and sysbus devices would be going away soon, and there are certainly discussions 
under way about coming up with a migration strategy to allow them to be completely 
removed.


ATB,

Mark.

Re: [PATCH v2 05/13] mac_oldworld: Do not open code sysbus_mmio_map()

[BALATON Zoltan]

On Thu, 29 Sep 2022, Mark Cave-Ayland wrote:
> On 25/09/2022 13:38, BALATON Zoltan wrote:
>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>>   hw/ppc/mac_oldworld.c | 8 ++------
>>   1 file changed, 2 insertions(+), 6 deletions(-)
>> 
>> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
>> index cb67e44081..75fbd2a7df 100644
>> --- a/hw/ppc/mac_oldworld.c
>> +++ b/hw/ppc/mac_oldworld.c
>> @@ -218,16 +218,12 @@ static void ppc_heathrow_init(MachineState *machine)
>>       qdev_prop_set_uint32(grackle_dev, "ofw-addr", 0x80000000);
>>       s = SYS_BUS_DEVICE(grackle_dev);
>>       sysbus_realize_and_unref(s, &error_fatal);
>> -
>>       sysbus_mmio_map(s, 0, GRACKLE_BASE);
>>       sysbus_mmio_map(s, 1, GRACKLE_BASE + 0x200000);
>>       /* PCI hole */
>> -    memory_region_add_subregion(get_system_memory(), 0x80000000ULL,
>> -                                sysbus_mmio_get_region(s, 2));
>> +    sysbus_mmio_map(s, 2, 0x80000000);
>>       /* Register 2 MB of ISA IO space */
>> -    memory_region_add_subregion(get_system_memory(), 0xfe000000,
>> -                                sysbus_mmio_get_region(s, 3));
>> -
>> +    sysbus_mmio_map(s, 3, 0xfe000000);
>>       pci_bus = PCI_HOST_BRIDGE(grackle_dev)->bus;
>>         /* MacIO */
>
> Please drop this patch for now. The code was written on assumption that both 
> sysbus and sysbus devices would be going away soon, and there are certainly 
> discussions under way about coming up with a migration strategy to allow them 
> to be completely removed.

This patch actually simplifies transition from sysbus to whatever else 
will be decided because then you'll surely have a way to replace 
sysbus_mmio_map() that's used everywhere else with something. This file 
now has both sysbus_mmio_map and sysbus mmio_get_region so using only one 
will make it easier to convert it and until then it's easier to read so I 
don't agree with this suggestion and want to stick to these patches (same 
with uninorth). Please reconsider your decision.

Regards,
BALATON Zoltan

Re: [PATCH v2 05/13] mac_oldworld: Do not open code sysbus_mmio_map()

[Mark Cave-Ayland]

On 29/09/2022 12:32, BALATON Zoltan wrote:

> On Thu, 29 Sep 2022, Mark Cave-Ayland wrote:
>> On 25/09/2022 13:38, BALATON Zoltan wrote:
>>
>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>> ---
>>>   hw/ppc/mac_oldworld.c | 8 ++------
>>>   1 file changed, 2 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
>>> index cb67e44081..75fbd2a7df 100644
>>> --- a/hw/ppc/mac_oldworld.c
>>> +++ b/hw/ppc/mac_oldworld.c
>>> @@ -218,16 +218,12 @@ static void ppc_heathrow_init(MachineState *machine)
>>>       qdev_prop_set_uint32(grackle_dev, "ofw-addr", 0x80000000);
>>>       s = SYS_BUS_DEVICE(grackle_dev);
>>>       sysbus_realize_and_unref(s, &error_fatal);
>>> -
>>>       sysbus_mmio_map(s, 0, GRACKLE_BASE);
>>>       sysbus_mmio_map(s, 1, GRACKLE_BASE + 0x200000);
>>>       /* PCI hole */
>>> -    memory_region_add_subregion(get_system_memory(), 0x80000000ULL,
>>> -                                sysbus_mmio_get_region(s, 2));
>>> +    sysbus_mmio_map(s, 2, 0x80000000);
>>>       /* Register 2 MB of ISA IO space */
>>> -    memory_region_add_subregion(get_system_memory(), 0xfe000000,
>>> -                                sysbus_mmio_get_region(s, 3));
>>> -
>>> +    sysbus_mmio_map(s, 3, 0xfe000000);
>>>       pci_bus = PCI_HOST_BRIDGE(grackle_dev)->bus;
>>>         /* MacIO */
>>
>> Please drop this patch for now. The code was written on assumption that both sysbus 
>> and sysbus devices would be going away soon, and there are certainly discussions 
>> under way about coming up with a migration strategy to allow them to be completely 
>> removed.
> 
> This patch actually simplifies transition from sysbus to whatever else will be 
> decided because then you'll surely have a way to replace sysbus_mmio_map() that's 
> used everywhere else with something. This file now has both sysbus_mmio_map and 
> sysbus mmio_get_region so using only one will make it easier to convert it and until 
> then it's easier to read so I don't agree with this suggestion and want to stick to 
> these patches (same with uninorth). Please reconsider your decision.

When sysbus eventually goes then mapping devices will most likely be handled by the 
memory API as above rather than using an explicit _map() API, so let's keep that 
rather than converting everything to use sysbus_mmio_map().


ATB,

Mark.

Re: [PATCH v2 05/13] mac_oldworld: Do not open code sysbus_mmio_map()

[BALATON Zoltan]

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

On Mon, 3 Oct 2022, Mark Cave-Ayland wrote:
> On 29/09/2022 12:32, BALATON Zoltan wrote:
>
>> On Thu, 29 Sep 2022, Mark Cave-Ayland wrote:
>>> On 25/09/2022 13:38, BALATON Zoltan wrote:
>>> 
>>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>>> ---
>>>>   hw/ppc/mac_oldworld.c | 8 ++------
>>>>   1 file changed, 2 insertions(+), 6 deletions(-)
>>>> 
>>>> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
>>>> index cb67e44081..75fbd2a7df 100644
>>>> --- a/hw/ppc/mac_oldworld.c
>>>> +++ b/hw/ppc/mac_oldworld.c
>>>> @@ -218,16 +218,12 @@ static void ppc_heathrow_init(MachineState 
>>>> *machine)
>>>>       qdev_prop_set_uint32(grackle_dev, "ofw-addr", 0x80000000);
>>>>       s = SYS_BUS_DEVICE(grackle_dev);
>>>>       sysbus_realize_and_unref(s, &error_fatal);
>>>> -
>>>>       sysbus_mmio_map(s, 0, GRACKLE_BASE);
>>>>       sysbus_mmio_map(s, 1, GRACKLE_BASE + 0x200000);
>>>>       /* PCI hole */
>>>> -    memory_region_add_subregion(get_system_memory(), 0x80000000ULL,
>>>> -                                sysbus_mmio_get_region(s, 2));
>>>> +    sysbus_mmio_map(s, 2, 0x80000000);
>>>>       /* Register 2 MB of ISA IO space */
>>>> -    memory_region_add_subregion(get_system_memory(), 0xfe000000,
>>>> -                                sysbus_mmio_get_region(s, 3));
>>>> -
>>>> +    sysbus_mmio_map(s, 3, 0xfe000000);
>>>>       pci_bus = PCI_HOST_BRIDGE(grackle_dev)->bus;
>>>>         /* MacIO */
>>> 
>>> Please drop this patch for now. The code was written on assumption that 
>>> both sysbus and sysbus devices would be going away soon, and there are 
>>> certainly discussions under way about coming up with a migration strategy 
>>> to allow them to be completely removed.
>> 
>> This patch actually simplifies transition from sysbus to whatever else will 
>> be decided because then you'll surely have a way to replace 
>> sysbus_mmio_map() that's used everywhere else with something. This file now 
>> has both sysbus_mmio_map and sysbus mmio_get_region so using only one will 
>> make it easier to convert it and until then it's easier to read so I don't 
>> agree with this suggestion and want to stick to these patches (same with 
>> uninorth). Please reconsider your decision.
>
> When sysbus eventually goes then mapping devices will most likely be handled 
> by the memory API as above rather than using an explicit _map() API, so let's 
> keep that rather than converting everything to use sysbus_mmio_map().

Hopefully not as that would make code very unreadable and hard to get for 
people unfamiliar with QOM so to not scare off potential contributors 
please invent an easier way, at least define a macro or a function for 
such common operation when sysus goes away. But it may be a long time 
until sysbus will be gone and until then we leave this code inconsistent 
now using two ways to map areaswhich I don't agree with but since you 
cannot be convinced I've dropped these changes for now.

Regards,
BALATON Zoltan

[PATCH v2 06/13] mac_newworld: Simplify creation of Uninorth devices

[BALATON Zoltan]

Avoid open coding sysbus_mmio_map() and map regions in ascending
otder. Reorganise code a bit to avoid some casts.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/ppc/mac_newworld.c | 42 +++++++++++++++++-------------------------
 1 file changed, 17 insertions(+), 25 deletions(-)

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 6bc3bd19be..b4ad43cc05 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -228,13 +228,6 @@ static void ppc_core99_init(MachineState *machine)
         }
     }
 
-    /* UniN init */
-    dev = qdev_new(TYPE_UNI_NORTH);
-    s = SYS_BUS_DEVICE(dev);
-    sysbus_realize_and_unref(s, &error_fatal);
-    memory_region_add_subregion(get_system_memory(), 0xf8000000,
-                                sysbus_mmio_get_region(s, 0));
-
     openpic_irqs = g_new0(IrqLines, machine->smp.cpus);
     for (i = 0; i < machine->smp.cpus; i++) {
         /* Mac99 IRQ connection between OpenPIC outputs pins
@@ -275,24 +268,27 @@ static void ppc_core99_init(MachineState *machine)
         }
     }
 
+    /* UniN init */
+    s = SYS_BUS_DEVICE(qdev_new(TYPE_UNI_NORTH));
+    sysbus_realize_and_unref(s, &error_fatal);
+    sysbus_mmio_map(s, 0, 0xf8000000);
+
     if (PPC_INPUT(env) == PPC_FLAGS_INPUT_970) {
+        machine_arch = ARCH_MAC99_U3;
         /* 970 gets a U3 bus */
         /* Uninorth AGP bus */
         dev = qdev_new(TYPE_U3_AGP_HOST_BRIDGE);
-        sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
         uninorth_pci = U3_AGP_HOST_BRIDGE(dev);
         s = SYS_BUS_DEVICE(dev);
-        /* PCI hole */
-        memory_region_add_subregion(get_system_memory(), 0x80000000ULL,
-                                    sysbus_mmio_get_region(s, 2));
-        /* Register 8 MB of ISA IO space */
-        memory_region_add_subregion(get_system_memory(), 0xf2000000,
-                                    sysbus_mmio_get_region(s, 3));
+        sysbus_realize_and_unref(s, &error_fatal);
         sysbus_mmio_map(s, 0, 0xf0800000);
         sysbus_mmio_map(s, 1, 0xf0c00000);
-
-        machine_arch = ARCH_MAC99_U3;
+        /* PCI hole */
+        sysbus_mmio_map(s, 2, 0x80000000);
+        /* Register 8 MB of ISA IO space */
+        sysbus_mmio_map(s, 3, 0xf2000000);
     } else {
+        machine_arch = ARCH_MAC99;
         /* Use values found on a real PowerMac */
         /* Uninorth AGP bus */
         uninorth_agp_dev = qdev_new(TYPE_UNI_NORTH_AGP_HOST_BRIDGE);
@@ -312,19 +308,15 @@ static void ppc_core99_init(MachineState *machine)
         /* Uninorth main bus */
         dev = qdev_new(TYPE_UNI_NORTH_PCI_HOST_BRIDGE);
         qdev_prop_set_uint32(dev, "ofw-addr", 0xf2000000);
-        sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
         uninorth_pci = UNI_NORTH_PCI_HOST_BRIDGE(dev);
         s = SYS_BUS_DEVICE(dev);
-        /* PCI hole */
-        memory_region_add_subregion(get_system_memory(), 0x80000000ULL,
-                                    sysbus_mmio_get_region(s, 2));
-        /* Register 8 MB of ISA IO space */
-        memory_region_add_subregion(get_system_memory(), 0xf2000000,
-                                    sysbus_mmio_get_region(s, 3));
+        sysbus_realize_and_unref(s, &error_fatal);
         sysbus_mmio_map(s, 0, 0xf2800000);
         sysbus_mmio_map(s, 1, 0xf2c00000);
-
-        machine_arch = ARCH_MAC99;
+        /* PCI hole */
+        sysbus_mmio_map(s, 2, 0x80000000);
+        /* Register 8 MB of ISA IO space */
+        sysbus_mmio_map(s, 3, 0xf2000000);
     }
 
     machine->usb |= defaults_enabled() && !machine->usb_disabled;
-- 
2.30.4

Re: [PATCH v2 06/13] mac_newworld: Simplify creation of Uninorth devices

[Mark Cave-Ayland]

On 25/09/2022 13:38, BALATON Zoltan wrote:

> Avoid open coding sysbus_mmio_map() and map regions in ascending
> otder. Reorganise code a bit to avoid some casts.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>   hw/ppc/mac_newworld.c | 42 +++++++++++++++++-------------------------
>   1 file changed, 17 insertions(+), 25 deletions(-)
> 
> diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
> index 6bc3bd19be..b4ad43cc05 100644
> --- a/hw/ppc/mac_newworld.c
> +++ b/hw/ppc/mac_newworld.c
> @@ -228,13 +228,6 @@ static void ppc_core99_init(MachineState *machine)
>           }
>       }
>   
> -    /* UniN init */
> -    dev = qdev_new(TYPE_UNI_NORTH);
> -    s = SYS_BUS_DEVICE(dev);
> -    sysbus_realize_and_unref(s, &error_fatal);
> -    memory_region_add_subregion(get_system_memory(), 0xf8000000,
> -                                sysbus_mmio_get_region(s, 0));
> -
>       openpic_irqs = g_new0(IrqLines, machine->smp.cpus);
>       for (i = 0; i < machine->smp.cpus; i++) {
>           /* Mac99 IRQ connection between OpenPIC outputs pins
> @@ -275,24 +268,27 @@ static void ppc_core99_init(MachineState *machine)
>           }
>       }
>   
> +    /* UniN init */
> +    s = SYS_BUS_DEVICE(qdev_new(TYPE_UNI_NORTH));
> +    sysbus_realize_and_unref(s, &error_fatal);
> +    sysbus_mmio_map(s, 0, 0xf8000000);
> +
>       if (PPC_INPUT(env) == PPC_FLAGS_INPUT_970) {
> +        machine_arch = ARCH_MAC99_U3;
>           /* 970 gets a U3 bus */
>           /* Uninorth AGP bus */
>           dev = qdev_new(TYPE_U3_AGP_HOST_BRIDGE);
> -        sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>           uninorth_pci = U3_AGP_HOST_BRIDGE(dev);
>           s = SYS_BUS_DEVICE(dev);
> -        /* PCI hole */
> -        memory_region_add_subregion(get_system_memory(), 0x80000000ULL,
> -                                    sysbus_mmio_get_region(s, 2));
> -        /* Register 8 MB of ISA IO space */
> -        memory_region_add_subregion(get_system_memory(), 0xf2000000,
> -                                    sysbus_mmio_get_region(s, 3));
> +        sysbus_realize_and_unref(s, &error_fatal);
>           sysbus_mmio_map(s, 0, 0xf0800000);
>           sysbus_mmio_map(s, 1, 0xf0c00000);
> -
> -        machine_arch = ARCH_MAC99_U3;
> +        /* PCI hole */
> +        sysbus_mmio_map(s, 2, 0x80000000);
> +        /* Register 8 MB of ISA IO space */
> +        sysbus_mmio_map(s, 3, 0xf2000000);
>       } else {
> +        machine_arch = ARCH_MAC99;
>           /* Use values found on a real PowerMac */
>           /* Uninorth AGP bus */
>           uninorth_agp_dev = qdev_new(TYPE_UNI_NORTH_AGP_HOST_BRIDGE);
> @@ -312,19 +308,15 @@ static void ppc_core99_init(MachineState *machine)
>           /* Uninorth main bus */
>           dev = qdev_new(TYPE_UNI_NORTH_PCI_HOST_BRIDGE);
>           qdev_prop_set_uint32(dev, "ofw-addr", 0xf2000000);
> -        sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>           uninorth_pci = UNI_NORTH_PCI_HOST_BRIDGE(dev);
>           s = SYS_BUS_DEVICE(dev);
> -        /* PCI hole */
> -        memory_region_add_subregion(get_system_memory(), 0x80000000ULL,
> -                                    sysbus_mmio_get_region(s, 2));
> -        /* Register 8 MB of ISA IO space */
> -        memory_region_add_subregion(get_system_memory(), 0xf2000000,
> -                                    sysbus_mmio_get_region(s, 3));
> +        sysbus_realize_and_unref(s, &error_fatal);
>           sysbus_mmio_map(s, 0, 0xf2800000);
>           sysbus_mmio_map(s, 1, 0xf2c00000);
> -
> -        machine_arch = ARCH_MAC99;
> +        /* PCI hole */
> +        sysbus_mmio_map(s, 2, 0x80000000);
> +        /* Register 8 MB of ISA IO space */
> +        sysbus_mmio_map(s, 3, 0xf2000000);
>       }
>   
>       machine->usb |= defaults_enabled() && !machine->usb_disabled;

Same comment here re: sysbus. Also the patch seems correct here, but it is worth 
noting that the PCI bus initialisation is order sensitive: the last bus created is 
the one that becomes the default PCI bus for -device, so changing this would break 
quite a few command lines...


ATB,

Mark.

Re: [PATCH v2 06/13] mac_newworld: Simplify creation of Uninorth devices

[Philippe Mathieu-Daudé via]

Cc'ing CLI refactor team.

On 29/9/22 09:39, Mark Cave-Ayland wrote:
> On 25/09/2022 13:38, BALATON Zoltan wrote:
> 
>> Avoid open coding sysbus_mmio_map() and map regions in ascending
>> otder. Reorganise code a bit to avoid some casts.
>>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>>   hw/ppc/mac_newworld.c | 42 +++++++++++++++++-------------------------
>>   1 file changed, 17 insertions(+), 25 deletions(-)

> Same comment here re: sysbus. Also the patch seems correct here, but it 
> is worth noting that the PCI bus initialisation is order sensitive: the 
> last bus created is the one that becomes the default PCI bus for 
> -device, so changing this would break quite a few command lines...

Eh, I was not aware of this API fragility. So when using -device without
expliciting the 'bus' key, the default is the latest bus created... OK.

Re: [PATCH v2 06/13] mac_newworld: Simplify creation of Uninorth devices

[Markus Armbruster]

Philippe Mathieu-Daudé <f4bug@amsat.org> writes:

> Cc'ing CLI refactor team.
>
> On 29/9/22 09:39, Mark Cave-Ayland wrote:
>> On 25/09/2022 13:38, BALATON Zoltan wrote:
>> 
>>> Avoid open coding sysbus_mmio_map() and map regions in ascending
>>> otder. Reorganise code a bit to avoid some casts.
>>>
>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>> ---
>>>   hw/ppc/mac_newworld.c | 42 +++++++++++++++++-------------------------
>>>   1 file changed, 17 insertions(+), 25 deletions(-)
>
>> Same comment here re: sysbus. Also the patch seems correct here, but it is worth noting that the PCI bus initialisation is order sensitive: the 
>> last bus created is the one that becomes the default PCI bus for -device, so changing this would break quite a few command lines...
>
> Eh, I was not aware of this API fragility. So when using -device without
> expliciting the 'bus' key, the default is the latest bus created... OK.

Yes, our external interface is in part defined implicitly by the order
in board code execution.  It goes deeper than just CLI, I'm afraid.

Omitting bus= is a convenience feature for users.  It's clearly useful.
But what's the default?  We walk the qdev tree rooted at the main system
bus looking for a bus of suitable bus type that is not full, or else of
suitable type that is full.  See qdev_find_recursive().  The tree walk
visits children in creation order.  Therefore, bus creation order is
ABI.

Related: bus names.

For user-created buses, the bus name depends on the owning device's qdev
ID (specified with id=ID).  If it has none, the system picks a bus name;
more on that below.  Else, it is ID.N, where N counts from zero.  When a
device creates multiple buses of the same type, its bus creation order
is ABI.

For onboard devices, the bus name can be specified by board code.  If
board code elects not to, the system picks a name.

System-picked names are TYPE.N, where TYPE is the bus type name such as
"pci" or "isa", and N counts from zero separately for each type.  Again,
bus creation order is ABI.

This is qbus_init_internal().

Letting users omit qdev IDs is a convenience feature.

Letting developers delegate bus name picking to the system is also a
convenience feature.  Without it, we'd need bus name logic in board code
and not just qbus_init_internal().

I dislike implicit ABI definitions.

Rarely met a QEMU convenience feature that didn't bite us in the
posterior later (the sensitivity of my posterior may well cloud my
memory, though).

Re: [PATCH v2 06/13] mac_newworld: Simplify creation of Uninorth devices

[Daniel P. Berrangé]

On Tue, Oct 04, 2022 at 08:39:57AM +0200, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <f4bug@amsat.org> writes:
> 
> > Cc'ing CLI refactor team.
> >
> > On 29/9/22 09:39, Mark Cave-Ayland wrote:
> >> On 25/09/2022 13:38, BALATON Zoltan wrote:
> >> 
> >>> Avoid open coding sysbus_mmio_map() and map regions in ascending
> >>> otder. Reorganise code a bit to avoid some casts.
> >>>
> >>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> >>> ---
> >>>   hw/ppc/mac_newworld.c | 42 +++++++++++++++++-------------------------
> >>>   1 file changed, 17 insertions(+), 25 deletions(-)
> >
> >> Same comment here re: sysbus. Also the patch seems correct here, but it is worth noting that the PCI bus initialisation is order sensitive: the 
> >> last bus created is the one that becomes the default PCI bus for -device, so changing this would break quite a few command lines...
> >
> > Eh, I was not aware of this API fragility. So when using -device without
> > expliciting the 'bus' key, the default is the latest bus created... OK.
> 
> Yes, our external interface is in part defined implicitly by the order
> in board code execution.  It goes deeper than just CLI, I'm afraid.
> 
> Omitting bus= is a convenience feature for users.  It's clearly useful.
> But what's the default?  We walk the qdev tree rooted at the main system
> bus looking for a bus of suitable bus type that is not full, or else of
> suitable type that is full.  See qdev_find_recursive().  The tree walk
> visits children in creation order.  Therefore, bus creation order is
> ABI.

There are different levels of ABI importance, however, depending on
which target/machine type we're talking about.

The critically important ABI is for anything that is part of, or used
by, machine types which are versioned. These are our top tier platforms,
typically associated with KVM and mgmt apps expect continuity from us,
and will usually be fully explicit in their configuration.

The less important ABI is for the non-versioned machine types. These
are typically associated with emulating legacy hardware platforms.
Users are more likely to use the convenience syntax, leaving out as
many implicit properties as possible.

ppc/mac_newworld is one of the latter class.

It is still desirable to avoid gratuitous changes that are likely
to break existing user's configurations of QEMU, but it is not
quite so critical. If someone can make a good case for why it is
better, it can be considered

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[PATCH v2 07/13] mac_{old|new}world: Reduce number of QOM casts

[BALATON Zoltan]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 10352 bytes --]

By storing the device pointers in a variable with the right type the
number of QOM casts can be reduced which also makes the code more
readable.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/ppc/mac_newworld.c | 61 ++++++++++++++++++++-----------------------
 hw/ppc/mac_oldworld.c | 26 ++++++++----------
 2 files changed, 39 insertions(+), 48 deletions(-)

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index b4ad43cc05..2acd103dd3 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -116,18 +116,16 @@ static void ppc_core99_init(MachineState *machine)
     MemoryRegion *bios = g_new(MemoryRegion, 1);
     hwaddr kernel_base = 0, initrd_base = 0, cmdline_base = 0;
     long kernel_size = 0, initrd_size = 0;
-    UNINHostState *uninorth_pci;
     PCIBus *pci_bus;
-    PCIDevice *macio;
-    ESCCState *escc;
     bool has_pmu, has_adb;
+    Object *macio;
     MACIOIDEState *macio_ide;
     BusState *adb_bus;
     MacIONVRAMState *nvr;
     DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
     void *fw_cfg;
     SysBusDevice *s;
-    DeviceState *dev, *pic_dev;
+    DeviceState *dev, *pic_dev, *uninorth_pci_dev;
     DeviceState *uninorth_internal_dev = NULL, *uninorth_agp_dev = NULL;
     hwaddr nvram_addr = 0xFFF04000;
     uint64_t tbfreq = kvm_enabled() ? kvmppc_get_tbfreq() : TBFREQ;
@@ -229,6 +227,7 @@ static void ppc_core99_init(MachineState *machine)
     }
 
     openpic_irqs = g_new0(IrqLines, machine->smp.cpus);
+    dev = DEVICE(cpu);
     for (i = 0; i < machine->smp.cpus; i++) {
         /* Mac99 IRQ connection between OpenPIC outputs pins
          * and PowerPC input pins
@@ -236,30 +235,30 @@ static void ppc_core99_init(MachineState *machine)
         switch (PPC_INPUT(env)) {
         case PPC_FLAGS_INPUT_6xx:
             openpic_irqs[i].irq[OPENPIC_OUTPUT_INT] =
-                qdev_get_gpio_in(DEVICE(cpu), PPC6xx_INPUT_INT);
+                qdev_get_gpio_in(dev, PPC6xx_INPUT_INT);
             openpic_irqs[i].irq[OPENPIC_OUTPUT_CINT] =
-                 qdev_get_gpio_in(DEVICE(cpu), PPC6xx_INPUT_INT);
+                 qdev_get_gpio_in(dev, PPC6xx_INPUT_INT);
             openpic_irqs[i].irq[OPENPIC_OUTPUT_MCK] =
-                qdev_get_gpio_in(DEVICE(cpu), PPC6xx_INPUT_MCP);
+                qdev_get_gpio_in(dev, PPC6xx_INPUT_MCP);
             /* Not connected ? */
             openpic_irqs[i].irq[OPENPIC_OUTPUT_DEBUG] = NULL;
             /* Check this */
             openpic_irqs[i].irq[OPENPIC_OUTPUT_RESET] =
-                qdev_get_gpio_in(DEVICE(cpu), PPC6xx_INPUT_HRESET);
+                qdev_get_gpio_in(dev, PPC6xx_INPUT_HRESET);
             break;
 #if defined(TARGET_PPC64)
         case PPC_FLAGS_INPUT_970:
             openpic_irqs[i].irq[OPENPIC_OUTPUT_INT] =
-                qdev_get_gpio_in(DEVICE(cpu), PPC970_INPUT_INT);
+                qdev_get_gpio_in(dev, PPC970_INPUT_INT);
             openpic_irqs[i].irq[OPENPIC_OUTPUT_CINT] =
-                qdev_get_gpio_in(DEVICE(cpu), PPC970_INPUT_INT);
+                qdev_get_gpio_in(dev, PPC970_INPUT_INT);
             openpic_irqs[i].irq[OPENPIC_OUTPUT_MCK] =
-                qdev_get_gpio_in(DEVICE(cpu), PPC970_INPUT_MCP);
+                qdev_get_gpio_in(dev, PPC970_INPUT_MCP);
             /* Not connected ? */
             openpic_irqs[i].irq[OPENPIC_OUTPUT_DEBUG] = NULL;
             /* Check this */
             openpic_irqs[i].irq[OPENPIC_OUTPUT_RESET] =
-                qdev_get_gpio_in(DEVICE(cpu), PPC970_INPUT_HRESET);
+                qdev_get_gpio_in(dev, PPC970_INPUT_HRESET);
             break;
 #endif /* defined(TARGET_PPC64) */
         default:
@@ -277,9 +276,8 @@ static void ppc_core99_init(MachineState *machine)
         machine_arch = ARCH_MAC99_U3;
         /* 970 gets a U3 bus */
         /* Uninorth AGP bus */
-        dev = qdev_new(TYPE_U3_AGP_HOST_BRIDGE);
-        uninorth_pci = U3_AGP_HOST_BRIDGE(dev);
-        s = SYS_BUS_DEVICE(dev);
+        uninorth_pci_dev = qdev_new(TYPE_U3_AGP_HOST_BRIDGE);
+        s = SYS_BUS_DEVICE(uninorth_pci_dev);
         sysbus_realize_and_unref(s, &error_fatal);
         sysbus_mmio_map(s, 0, 0xf0800000);
         sysbus_mmio_map(s, 1, 0xf0c00000);
@@ -306,10 +304,9 @@ static void ppc_core99_init(MachineState *machine)
         sysbus_mmio_map(s, 1, 0xf4c00000);
 
         /* Uninorth main bus */
-        dev = qdev_new(TYPE_UNI_NORTH_PCI_HOST_BRIDGE);
-        qdev_prop_set_uint32(dev, "ofw-addr", 0xf2000000);
-        uninorth_pci = UNI_NORTH_PCI_HOST_BRIDGE(dev);
-        s = SYS_BUS_DEVICE(dev);
+        uninorth_pci_dev = qdev_new(TYPE_UNI_NORTH_PCI_HOST_BRIDGE);
+        qdev_prop_set_uint32(uninorth_pci_dev, "ofw-addr", 0xf2000000);
+        s = SYS_BUS_DEVICE(uninorth_pci_dev);
         sysbus_realize_and_unref(s, &error_fatal);
         sysbus_mmio_map(s, 0, 0xf2800000);
         sysbus_mmio_map(s, 1, 0xf2c00000);
@@ -325,24 +322,24 @@ static void ppc_core99_init(MachineState *machine)
                core99_machine->via_config == CORE99_VIA_CONFIG_PMU_ADB);
 
     /* init basic PC hardware */
-    pci_bus = PCI_HOST_BRIDGE(uninorth_pci)->bus;
+    pci_bus = PCI_HOST_BRIDGE(uninorth_pci_dev)->bus;
 
     /* MacIO */
-    macio = pci_new(-1, TYPE_NEWWORLD_MACIO);
+    macio = OBJECT(pci_new(-1, TYPE_NEWWORLD_MACIO));
     dev = DEVICE(macio);
     qdev_prop_set_uint64(dev, "frequency", tbfreq);
     qdev_prop_set_bit(dev, "has-pmu", has_pmu);
     qdev_prop_set_bit(dev, "has-adb", has_adb);
 
-    escc = ESCC(object_resolve_path_component(OBJECT(macio), "escc"));
-    qdev_prop_set_chr(DEVICE(escc), "chrA", serial_hd(0));
-    qdev_prop_set_chr(DEVICE(escc), "chrB", serial_hd(1));
+    dev = DEVICE(object_resolve_path_component(macio, "escc"));
+    qdev_prop_set_chr(dev, "chrA", serial_hd(0));
+    qdev_prop_set_chr(dev, "chrB", serial_hd(1));
 
-    pci_realize_and_unref(macio, pci_bus, &error_fatal);
+    pci_realize_and_unref(PCI_DEVICE(macio), pci_bus, &error_fatal);
 
-    pic_dev = DEVICE(object_resolve_path_component(OBJECT(macio), "pic"));
+    pic_dev = DEVICE(object_resolve_path_component(macio, "pic"));
     for (i = 0; i < 4; i++) {
-        qdev_connect_gpio_out(DEVICE(uninorth_pci), i,
+        qdev_connect_gpio_out(uninorth_pci_dev, i,
                               qdev_get_gpio_in(pic_dev, 0x1b + i));
     }
 
@@ -374,19 +371,17 @@ static void ppc_core99_init(MachineState *machine)
     /* We only emulate 2 out of 3 IDE controllers for now */
     ide_drive_get(hd, ARRAY_SIZE(hd));
 
-    macio_ide = MACIO_IDE(object_resolve_path_component(OBJECT(macio),
-                                                        "ide[0]"));
+    macio_ide = MACIO_IDE(object_resolve_path_component(macio, "ide[0]"));
     macio_ide_init_drives(macio_ide, hd);
 
-    macio_ide = MACIO_IDE(object_resolve_path_component(OBJECT(macio),
-                                                        "ide[1]"));
+    macio_ide = MACIO_IDE(object_resolve_path_component(macio, "ide[1]"));
     macio_ide_init_drives(macio_ide, &hd[MAX_IDE_DEVS]);
 
     if (has_adb) {
         if (has_pmu) {
-            dev = DEVICE(object_resolve_path_component(OBJECT(macio), "pmu"));
+            dev = DEVICE(object_resolve_path_component(macio, "pmu"));
         } else {
-            dev = DEVICE(object_resolve_path_component(OBJECT(macio), "cuda"));
+            dev = DEVICE(object_resolve_path_component(macio, "cuda"));
         }
 
         adb_bus = qdev_get_child_bus(dev, "adb.0");
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 75fbd2a7df..9d651cf482 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -90,9 +90,8 @@ static void ppc_heathrow_init(MachineState *machine)
     uint32_t kernel_base = 0, initrd_base = 0, cmdline_base = 0;
     int32_t kernel_size = 0, initrd_size = 0;
     PCIBus *pci_bus;
-    PCIDevice *macio;
+    Object *macio;
     MACIOIDEState *macio_ide;
-    ESCCState *escc;
     SysBusDevice *s;
     DeviceState *dev, *pic_dev, *grackle_dev;
     BusState *adb_bus;
@@ -227,17 +226,16 @@ static void ppc_heathrow_init(MachineState *machine)
     pci_bus = PCI_HOST_BRIDGE(grackle_dev)->bus;
 
     /* MacIO */
-    macio = pci_new(PCI_DEVFN(16, 0), TYPE_OLDWORLD_MACIO);
-    dev = DEVICE(macio);
-    qdev_prop_set_uint64(dev, "frequency", tbfreq);
+    macio = OBJECT(pci_new(PCI_DEVFN(16, 0), TYPE_OLDWORLD_MACIO));
+    qdev_prop_set_uint64(DEVICE(macio), "frequency", tbfreq);
 
-    escc = ESCC(object_resolve_path_component(OBJECT(macio), "escc"));
-    qdev_prop_set_chr(DEVICE(escc), "chrA", serial_hd(0));
-    qdev_prop_set_chr(DEVICE(escc), "chrB", serial_hd(1));
+    dev = DEVICE(object_resolve_path_component(macio, "escc"));
+    qdev_prop_set_chr(dev, "chrA", serial_hd(0));
+    qdev_prop_set_chr(dev, "chrB", serial_hd(1));
 
-    pci_realize_and_unref(macio, pci_bus, &error_fatal);
+    pci_realize_and_unref(PCI_DEVICE(macio), pci_bus, &error_fatal);
 
-    pic_dev = DEVICE(object_resolve_path_component(OBJECT(macio), "pic"));
+    pic_dev = DEVICE(object_resolve_path_component(macio, "pic"));
     for (i = 0; i < 4; i++) {
         qdev_connect_gpio_out(grackle_dev, i,
                               qdev_get_gpio_in(pic_dev, 0x15 + i));
@@ -265,16 +263,14 @@ static void ppc_heathrow_init(MachineState *machine)
 
     /* MacIO IDE */
     ide_drive_get(hd, ARRAY_SIZE(hd));
-    macio_ide = MACIO_IDE(object_resolve_path_component(OBJECT(macio),
-                                                        "ide[0]"));
+    macio_ide = MACIO_IDE(object_resolve_path_component(macio, "ide[0]"));
     macio_ide_init_drives(macio_ide, hd);
 
-    macio_ide = MACIO_IDE(object_resolve_path_component(OBJECT(macio),
-                                                        "ide[1]"));
+    macio_ide = MACIO_IDE(object_resolve_path_component(macio, "ide[1]"));
     macio_ide_init_drives(macio_ide, &hd[MAX_IDE_DEVS]);
 
     /* MacIO CUDA/ADB */
-    dev = DEVICE(object_resolve_path_component(OBJECT(macio), "cuda"));
+    dev = DEVICE(object_resolve_path_component(macio, "cuda"));
     adb_bus = qdev_get_child_bus(dev, "adb.0");
     dev = qdev_new(TYPE_ADB_KEYBOARD);
     qdev_realize_and_unref(dev, adb_bus, &error_fatal);
-- 
2.30.4

[PATCH v2 08/13] hw/ppc/mac.h: Move newworld specific parts out from shared header

[BALATON Zoltan]

Move the parts specific to and only used by mac99 out from the shared
mac.h into mac_newworld.c where they better belong.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/ppc/mac.h          | 24 ------------------------
 hw/ppc/mac_newworld.c | 19 +++++++++++++++++++
 hw/ppc/mac_oldworld.c |  1 +
 3 files changed, 20 insertions(+), 24 deletions(-)

diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
index a1fa8f8e41..e97087c7e7 100644
--- a/hw/ppc/mac.h
+++ b/hw/ppc/mac.h
@@ -26,15 +26,8 @@
 #ifndef PPC_MAC_H
 #define PPC_MAC_H
 
-#include "qemu/units.h"
 #include "exec/memory.h"
-#include "hw/boards.h"
 #include "hw/sysbus.h"
-#include "hw/input/adb.h"
-#include "hw/misc/mos6522.h"
-#include "hw/pci/pci_host.h"
-#include "hw/pci-host/uninorth.h"
-#include "qom/object.h"
 
 #define NVRAM_SIZE        0x2000
 #define PROM_FILENAME    "openbios-ppc"
@@ -65,23 +58,6 @@
 #define NEWWORLD_EXTING_GPIO1  0x2f
 #define NEWWORLD_EXTING_GPIO9  0x37
 
-/* Core99 machine */
-#define TYPE_CORE99_MACHINE MACHINE_TYPE_NAME("mac99")
-typedef struct Core99MachineState Core99MachineState;
-DECLARE_INSTANCE_CHECKER(Core99MachineState, CORE99_MACHINE,
-                         TYPE_CORE99_MACHINE)
-
-#define CORE99_VIA_CONFIG_CUDA     0x0
-#define CORE99_VIA_CONFIG_PMU      0x1
-#define CORE99_VIA_CONFIG_PMU_ADB  0x2
-
-struct Core99MachineState {
-    /*< private >*/
-    MachineState parent;
-
-    uint8_t via_config;
-};
-
 /* Grackle PCI */
 #define TYPE_GRACKLE_PCI_HOST_BRIDGE "grackle-pcihost"
 
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 2acd103dd3..32ab730600 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -48,10 +48,13 @@
 
 #include "qemu/osdep.h"
 #include "qemu/datadir.h"
+#include "qemu/units.h"
 #include "qapi/error.h"
 #include "hw/ppc/ppc.h"
 #include "hw/qdev-properties.h"
 #include "hw/ppc/mac.h"
+#include "hw/boards.h"
+#include "hw/pci-host/uninorth.h"
 #include "hw/input/adb.h"
 #include "hw/ppc/mac_dbdma.h"
 #include "hw/pci/pci.h"
@@ -83,6 +86,22 @@
 #define PROM_BASE 0xfff00000
 #define PROM_SIZE (1 * MiB)
 
+#define TYPE_CORE99_MACHINE MACHINE_TYPE_NAME("mac99")
+typedef struct Core99MachineState Core99MachineState;
+DECLARE_INSTANCE_CHECKER(Core99MachineState, CORE99_MACHINE,
+                         TYPE_CORE99_MACHINE)
+
+#define CORE99_VIA_CONFIG_CUDA     0x0
+#define CORE99_VIA_CONFIG_PMU      0x1
+#define CORE99_VIA_CONFIG_PMU_ADB  0x2
+
+struct Core99MachineState {
+    /*< private >*/
+    MachineState parent;
+
+    uint8_t via_config;
+};
+
 static void fw_cfg_boot_set(void *opaque, const char *boot_device,
                             Error **errp)
 {
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 9d651cf482..1fa7b770b7 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -31,6 +31,7 @@
 #include "hw/ppc/ppc.h"
 #include "hw/qdev-properties.h"
 #include "mac.h"
+#include "hw/boards.h"
 #include "hw/input/adb.h"
 #include "sysemu/sysemu.h"
 #include "net/net.h"
-- 
2.30.4

[PATCH v2 09/13] hw/ppc/mac.h: Move macio specific parts out from shared header

[BALATON Zoltan]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2984 bytes --]

Move the parts specific to and only used by macio out from the shared
mac.h into macio.c where they better belong.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/misc/macio/macio.c         |  5 +++--
 hw/ppc/mac.h                  | 23 -----------------------
 include/hw/misc/macio/macio.h | 21 +++++++++++++++++++++
 3 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index c1fad43f6c..f9f0758b03 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -37,8 +37,9 @@
 #include "hw/intc/heathrow_pic.h"
 #include "trace.h"
 
-/* Note: this code is strongly inspirated from the corresponding code
- * in PearPC */
+#define ESCC_CLOCK 3686400
+
+/* Note: this code is strongly inspired by the corresponding code in PearPC */
 
 /*
  * The mac-io has two interfaces to the ESCC. One is called "escc-legacy",
diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
index e97087c7e7..55cb02c990 100644
--- a/hw/ppc/mac.h
+++ b/hw/ppc/mac.h
@@ -35,29 +35,6 @@
 #define KERNEL_LOAD_ADDR 0x01000000
 #define KERNEL_GAP       0x00100000
 
-#define ESCC_CLOCK 3686400
-
-/* Old World IRQs */
-#define OLDWORLD_CUDA_IRQ      0x12
-#define OLDWORLD_ESCCB_IRQ     0x10
-#define OLDWORLD_ESCCA_IRQ     0xf
-#define OLDWORLD_IDE0_IRQ      0xd
-#define OLDWORLD_IDE0_DMA_IRQ  0x2
-#define OLDWORLD_IDE1_IRQ      0xe
-#define OLDWORLD_IDE1_DMA_IRQ  0x3
-
-/* New World IRQs */
-#define NEWWORLD_CUDA_IRQ      0x19
-#define NEWWORLD_PMU_IRQ       0x19
-#define NEWWORLD_ESCCB_IRQ     0x24
-#define NEWWORLD_ESCCA_IRQ     0x25
-#define NEWWORLD_IDE0_IRQ      0xd
-#define NEWWORLD_IDE0_DMA_IRQ  0x2
-#define NEWWORLD_IDE1_IRQ      0xe
-#define NEWWORLD_IDE1_DMA_IRQ  0x3
-#define NEWWORLD_EXTING_GPIO1  0x2f
-#define NEWWORLD_EXTING_GPIO9  0x37
-
 /* Grackle PCI */
 #define TYPE_GRACKLE_PCI_HOST_BRIDGE "grackle-pcihost"
 
diff --git a/include/hw/misc/macio/macio.h b/include/hw/misc/macio/macio.h
index 6c05f3bfd2..26cf15b1ce 100644
--- a/include/hw/misc/macio/macio.h
+++ b/include/hw/misc/macio/macio.h
@@ -38,6 +38,27 @@
 #include "hw/ppc/openpic.h"
 #include "qom/object.h"
 
+/* Old World IRQs */
+#define OLDWORLD_CUDA_IRQ      0x12
+#define OLDWORLD_ESCCB_IRQ     0x10
+#define OLDWORLD_ESCCA_IRQ     0xf
+#define OLDWORLD_IDE0_IRQ      0xd
+#define OLDWORLD_IDE0_DMA_IRQ  0x2
+#define OLDWORLD_IDE1_IRQ      0xe
+#define OLDWORLD_IDE1_DMA_IRQ  0x3
+
+/* New World IRQs */
+#define NEWWORLD_CUDA_IRQ      0x19
+#define NEWWORLD_PMU_IRQ       0x19
+#define NEWWORLD_ESCCB_IRQ     0x24
+#define NEWWORLD_ESCCA_IRQ     0x25
+#define NEWWORLD_IDE0_IRQ      0xd
+#define NEWWORLD_IDE0_DMA_IRQ  0x2
+#define NEWWORLD_IDE1_IRQ      0xe
+#define NEWWORLD_IDE1_DMA_IRQ  0x3
+#define NEWWORLD_EXTING_GPIO1  0x2f
+#define NEWWORLD_EXTING_GPIO9  0x37
+
 /* MacIO virtual bus */
 #define TYPE_MACIO_BUS "macio-bus"
 OBJECT_DECLARE_SIMPLE_TYPE(MacIOBusState, MACIO_BUS)
-- 
2.30.4

Re: [PATCH v2 09/13] hw/ppc/mac.h: Move macio specific parts out from shared header

[Mark Cave-Ayland]

On 25/09/2022 13:38, BALATON Zoltan wrote:

> Move the parts specific to and only used by macio out from the shared
> mac.h into macio.c where they better belong.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   hw/misc/macio/macio.c         |  5 +++--
>   hw/ppc/mac.h                  | 23 -----------------------
>   include/hw/misc/macio/macio.h | 21 +++++++++++++++++++++
>   3 files changed, 24 insertions(+), 25 deletions(-)
> 
> diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
> index c1fad43f6c..f9f0758b03 100644
> --- a/hw/misc/macio/macio.c
> +++ b/hw/misc/macio/macio.c
> @@ -37,8 +37,9 @@
>   #include "hw/intc/heathrow_pic.h"
>   #include "trace.h"
>   
> -/* Note: this code is strongly inspirated from the corresponding code
> - * in PearPC */
> +#define ESCC_CLOCK 3686400
> +
> +/* Note: this code is strongly inspired by the corresponding code in PearPC */
>   
>   /*
>    * The mac-io has two interfaces to the ESCC. One is called "escc-legacy",
> diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
> index e97087c7e7..55cb02c990 100644
> --- a/hw/ppc/mac.h
> +++ b/hw/ppc/mac.h
> @@ -35,29 +35,6 @@
>   #define KERNEL_LOAD_ADDR 0x01000000
>   #define KERNEL_GAP       0x00100000
>   
> -#define ESCC_CLOCK 3686400
> -
> -/* Old World IRQs */
> -#define OLDWORLD_CUDA_IRQ      0x12
> -#define OLDWORLD_ESCCB_IRQ     0x10
> -#define OLDWORLD_ESCCA_IRQ     0xf
> -#define OLDWORLD_IDE0_IRQ      0xd
> -#define OLDWORLD_IDE0_DMA_IRQ  0x2
> -#define OLDWORLD_IDE1_IRQ      0xe
> -#define OLDWORLD_IDE1_DMA_IRQ  0x3
> -
> -/* New World IRQs */
> -#define NEWWORLD_CUDA_IRQ      0x19
> -#define NEWWORLD_PMU_IRQ       0x19
> -#define NEWWORLD_ESCCB_IRQ     0x24
> -#define NEWWORLD_ESCCA_IRQ     0x25
> -#define NEWWORLD_IDE0_IRQ      0xd
> -#define NEWWORLD_IDE0_DMA_IRQ  0x2
> -#define NEWWORLD_IDE1_IRQ      0xe
> -#define NEWWORLD_IDE1_DMA_IRQ  0x3
> -#define NEWWORLD_EXTING_GPIO1  0x2f
> -#define NEWWORLD_EXTING_GPIO9  0x37
> -
>   /* Grackle PCI */
>   #define TYPE_GRACKLE_PCI_HOST_BRIDGE "grackle-pcihost"
>   
> diff --git a/include/hw/misc/macio/macio.h b/include/hw/misc/macio/macio.h
> index 6c05f3bfd2..26cf15b1ce 100644
> --- a/include/hw/misc/macio/macio.h
> +++ b/include/hw/misc/macio/macio.h
> @@ -38,6 +38,27 @@
>   #include "hw/ppc/openpic.h"
>   #include "qom/object.h"
>   
> +/* Old World IRQs */
> +#define OLDWORLD_CUDA_IRQ      0x12
> +#define OLDWORLD_ESCCB_IRQ     0x10
> +#define OLDWORLD_ESCCA_IRQ     0xf
> +#define OLDWORLD_IDE0_IRQ      0xd
> +#define OLDWORLD_IDE0_DMA_IRQ  0x2
> +#define OLDWORLD_IDE1_IRQ      0xe
> +#define OLDWORLD_IDE1_DMA_IRQ  0x3
> +
> +/* New World IRQs */
> +#define NEWWORLD_CUDA_IRQ      0x19
> +#define NEWWORLD_PMU_IRQ       0x19
> +#define NEWWORLD_ESCCB_IRQ     0x24
> +#define NEWWORLD_ESCCA_IRQ     0x25
> +#define NEWWORLD_IDE0_IRQ      0xd
> +#define NEWWORLD_IDE0_DMA_IRQ  0x2
> +#define NEWWORLD_IDE1_IRQ      0xe
> +#define NEWWORLD_IDE1_DMA_IRQ  0x3
> +#define NEWWORLD_EXTING_GPIO1  0x2f
> +#define NEWWORLD_EXTING_GPIO9  0x37
> +
>   /* MacIO virtual bus */
>   #define TYPE_MACIO_BUS "macio-bus"
>   OBJECT_DECLARE_SIMPLE_TYPE(MacIOBusState, MACIO_BUS)

At some point I'd like to review whether or not this wiring should be done at the 
board level, however that's orthogonal to this series so:

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


ATB,

Mark.

[PATCH v2 10/13] hw/ppc/mac.h: Move grackle-pcihost declaration out from shared header

[BALATON Zoltan]

It is only used by mac_oldworld anyway and it already instantiates
a few devices by name so this allows reducing the shared header further.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/pci-host/grackle.c | 1 +
 hw/ppc/mac.h          | 3 ---
 hw/ppc/mac_oldworld.c | 2 +-
 3 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
index b05facf463..5282123004 100644
--- a/hw/pci-host/grackle.c
+++ b/hw/pci-host/grackle.c
@@ -34,6 +34,7 @@
 #include "trace.h"
 #include "qom/object.h"
 
+#define TYPE_GRACKLE_PCI_HOST_BRIDGE "grackle-pcihost"
 OBJECT_DECLARE_SIMPLE_TYPE(GrackleState, GRACKLE_PCI_HOST_BRIDGE)
 
 struct GrackleState {
diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
index 55cb02c990..fe77a6c6db 100644
--- a/hw/ppc/mac.h
+++ b/hw/ppc/mac.h
@@ -35,9 +35,6 @@
 #define KERNEL_LOAD_ADDR 0x01000000
 #define KERNEL_GAP       0x00100000
 
-/* Grackle PCI */
-#define TYPE_GRACKLE_PCI_HOST_BRIDGE "grackle-pcihost"
-
 /* Mac NVRAM */
 #define TYPE_MACIO_NVRAM "macio-nvram"
 OBJECT_DECLARE_SIMPLE_TYPE(MacIONVRAMState, MACIO_NVRAM)
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 1fa7b770b7..1355d032ff 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -214,7 +214,7 @@ static void ppc_heathrow_init(MachineState *machine)
     }
 
     /* Grackle PCI host bridge */
-    grackle_dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
+    grackle_dev = qdev_new("grackle-pcihost");
     qdev_prop_set_uint32(grackle_dev, "ofw-addr", 0x80000000);
     s = SYS_BUS_DEVICE(grackle_dev);
     sysbus_realize_and_unref(s, &error_fatal);
-- 
2.30.4

Re: [PATCH v2 10/13] hw/ppc/mac.h: Move grackle-pcihost declaration out from shared header

[Mark Cave-Ayland]

On 25/09/2022 13:38, BALATON Zoltan wrote:

> It is only used by mac_oldworld anyway and it already instantiates
> a few devices by name so this allows reducing the shared header further.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>   hw/pci-host/grackle.c | 1 +
>   hw/ppc/mac.h          | 3 ---
>   hw/ppc/mac_oldworld.c | 2 +-
>   3 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
> index b05facf463..5282123004 100644
> --- a/hw/pci-host/grackle.c
> +++ b/hw/pci-host/grackle.c
> @@ -34,6 +34,7 @@
>   #include "trace.h"
>   #include "qom/object.h"
>   
> +#define TYPE_GRACKLE_PCI_HOST_BRIDGE "grackle-pcihost"
>   OBJECT_DECLARE_SIMPLE_TYPE(GrackleState, GRACKLE_PCI_HOST_BRIDGE)
>   
>   struct GrackleState {
> diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
> index 55cb02c990..fe77a6c6db 100644
> --- a/hw/ppc/mac.h
> +++ b/hw/ppc/mac.h
> @@ -35,9 +35,6 @@
>   #define KERNEL_LOAD_ADDR 0x01000000
>   #define KERNEL_GAP       0x00100000
>   
> -/* Grackle PCI */
> -#define TYPE_GRACKLE_PCI_HOST_BRIDGE "grackle-pcihost"
> -
>   /* Mac NVRAM */
>   #define TYPE_MACIO_NVRAM "macio-nvram"
>   OBJECT_DECLARE_SIMPLE_TYPE(MacIONVRAMState, MACIO_NVRAM)
> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
> index 1fa7b770b7..1355d032ff 100644
> --- a/hw/ppc/mac_oldworld.c
> +++ b/hw/ppc/mac_oldworld.c
> @@ -214,7 +214,7 @@ static void ppc_heathrow_init(MachineState *machine)
>       }
>   
>       /* Grackle PCI host bridge */
> -    grackle_dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
> +    grackle_dev = qdev_new("grackle-pcihost");
>       qdev_prop_set_uint32(grackle_dev, "ofw-addr", 0x80000000);
>       s = SYS_BUS_DEVICE(grackle_dev);
>       sysbus_realize_and_unref(s, &error_fatal);

Why did you include this patch again in v2 when I nacked it in v1?


ATB,

Mark.

Re: [PATCH v2 10/13] hw/ppc/mac.h: Move grackle-pcihost declaration out from shared header

[BALATON Zoltan]

On Thu, 29 Sep 2022, Mark Cave-Ayland wrote:
> On 25/09/2022 13:38, BALATON Zoltan wrote:
>
>> It is only used by mac_oldworld anyway and it already instantiates
>> a few devices by name so this allows reducing the shared header further.
>> 
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>>   hw/pci-host/grackle.c | 1 +
>>   hw/ppc/mac.h          | 3 ---
>>   hw/ppc/mac_oldworld.c | 2 +-
>>   3 files changed, 2 insertions(+), 4 deletions(-)
>> 
>> diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
>> index b05facf463..5282123004 100644
>> --- a/hw/pci-host/grackle.c
>> +++ b/hw/pci-host/grackle.c
>> @@ -34,6 +34,7 @@
>>   #include "trace.h"
>>   #include "qom/object.h"
>>   +#define TYPE_GRACKLE_PCI_HOST_BRIDGE "grackle-pcihost"
>>   OBJECT_DECLARE_SIMPLE_TYPE(GrackleState, GRACKLE_PCI_HOST_BRIDGE)
>>     struct GrackleState {
>> diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
>> index 55cb02c990..fe77a6c6db 100644
>> --- a/hw/ppc/mac.h
>> +++ b/hw/ppc/mac.h
>> @@ -35,9 +35,6 @@
>>   #define KERNEL_LOAD_ADDR 0x01000000
>>   #define KERNEL_GAP       0x00100000
>>   -/* Grackle PCI */
>> -#define TYPE_GRACKLE_PCI_HOST_BRIDGE "grackle-pcihost"
>> -
>>   /* Mac NVRAM */
>>   #define TYPE_MACIO_NVRAM "macio-nvram"
>>   OBJECT_DECLARE_SIMPLE_TYPE(MacIONVRAMState, MACIO_NVRAM)
>> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
>> index 1fa7b770b7..1355d032ff 100644
>> --- a/hw/ppc/mac_oldworld.c
>> +++ b/hw/ppc/mac_oldworld.c
>> @@ -214,7 +214,7 @@ static void ppc_heathrow_init(MachineState *machine)
>>       }
>>         /* Grackle PCI host bridge */
>> -    grackle_dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
>> +    grackle_dev = qdev_new("grackle-pcihost");
>>       qdev_prop_set_uint32(grackle_dev, "ofw-addr", 0x80000000);
>>       s = SYS_BUS_DEVICE(grackle_dev);
>>       sysbus_realize_and_unref(s, &error_fatal);
>
> Why did you include this patch again in v2 when I nacked it in v1?

You did not nack it just said you'd prefer a header instead. As a reviwer 
you express your opinion not an absolute decision that can't be discussed. 
I've replied to that but could not drop this patch as it's needed for 
later patches to get mac.h cleaned so until we agree on something this has 
left unchanged. I'm not a fan of one line headers and splitting up files 
into separate directories that is harder to work with and also think 
reorganising grackle is a separate clean up out of scope for this series 
but OK, I'll move the TYPE define in a new header in the next version. 
I'll wait for your reply on sysbus_mmio_map before sending a new version.

Regards,
BALATON Zoltan

Re: [PATCH v2 10/13] hw/ppc/mac.h: Move grackle-pcihost declaration out from shared header

[Mark Cave-Ayland]

On 29/09/2022 12:42, BALATON Zoltan wrote:

> On Thu, 29 Sep 2022, Mark Cave-Ayland wrote:
>> On 25/09/2022 13:38, BALATON Zoltan wrote:
>>
>>> It is only used by mac_oldworld anyway and it already instantiates
>>> a few devices by name so this allows reducing the shared header further.
>>>
>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>> ---
>>>   hw/pci-host/grackle.c | 1 +
>>>   hw/ppc/mac.h          | 3 ---
>>>   hw/ppc/mac_oldworld.c | 2 +-
>>>   3 files changed, 2 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
>>> index b05facf463..5282123004 100644
>>> --- a/hw/pci-host/grackle.c
>>> +++ b/hw/pci-host/grackle.c
>>> @@ -34,6 +34,7 @@
>>>   #include "trace.h"
>>>   #include "qom/object.h"
>>>   +#define TYPE_GRACKLE_PCI_HOST_BRIDGE "grackle-pcihost"
>>>   OBJECT_DECLARE_SIMPLE_TYPE(GrackleState, GRACKLE_PCI_HOST_BRIDGE)
>>>     struct GrackleState {
>>> diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
>>> index 55cb02c990..fe77a6c6db 100644
>>> --- a/hw/ppc/mac.h
>>> +++ b/hw/ppc/mac.h
>>> @@ -35,9 +35,6 @@
>>>   #define KERNEL_LOAD_ADDR 0x01000000
>>>   #define KERNEL_GAP       0x00100000
>>>   -/* Grackle PCI */
>>> -#define TYPE_GRACKLE_PCI_HOST_BRIDGE "grackle-pcihost"
>>> -
>>>   /* Mac NVRAM */
>>>   #define TYPE_MACIO_NVRAM "macio-nvram"
>>>   OBJECT_DECLARE_SIMPLE_TYPE(MacIONVRAMState, MACIO_NVRAM)
>>> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
>>> index 1fa7b770b7..1355d032ff 100644
>>> --- a/hw/ppc/mac_oldworld.c
>>> +++ b/hw/ppc/mac_oldworld.c
>>> @@ -214,7 +214,7 @@ static void ppc_heathrow_init(MachineState *machine)
>>>       }
>>>         /* Grackle PCI host bridge */
>>> -    grackle_dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
>>> +    grackle_dev = qdev_new("grackle-pcihost");
>>>       qdev_prop_set_uint32(grackle_dev, "ofw-addr", 0x80000000);
>>>       s = SYS_BUS_DEVICE(grackle_dev);
>>>       sysbus_realize_and_unref(s, &error_fatal);
>>
>> Why did you include this patch again in v2 when I nacked it in v1?
> 
> You did not nack it just said you'd prefer a header instead. As a reviwer you express 
> your opinion not an absolute decision that can't be discussed. I've replied to that 
> but could not drop this patch as it's needed for later patches to get mac.h cleaned 
> so until we agree on something this has left unchanged. I'm not a fan of one line 
> headers and splitting up files into separate directories that is harder to work with 
> and also think reorganising grackle is a separate clean up out of scope for this 
> series but OK, I'll move the TYPE define in a new header in the next version. I'll 
> wait for your reply on sysbus_mmio_map before sending a new version.

I should add this obviously also includes moving the GrackleState struct to the new 
header file at the same time, as per typical QOM practice.


ATB,

Mark.

[PATCH v2 11/13] hw/ppc/mac.h: Move PROM and KERNEL defines to board code

[BALATON Zoltan]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2014 bytes --]

The PROM_FILENAME and KERNEL_* defines are used by mac_oldworld and
mac_newworld but they don't have to be identical so these could be
moved to the individual boards.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/ppc/mac.h          | 4 ----
 hw/ppc/mac_newworld.c | 4 ++++
 hw/ppc/mac_oldworld.c | 7 ++++++-
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
index fe77a6c6db..3e2df262ee 100644
--- a/hw/ppc/mac.h
+++ b/hw/ppc/mac.h
@@ -30,10 +30,6 @@
 #include "hw/sysbus.h"
 
 #define NVRAM_SIZE        0x2000
-#define PROM_FILENAME    "openbios-ppc"
-
-#define KERNEL_LOAD_ADDR 0x01000000
-#define KERNEL_GAP       0x00100000
 
 /* Mac NVRAM */
 #define TYPE_MACIO_NVRAM "macio-nvram"
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 32ab730600..94a8b72fb1 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -83,9 +83,13 @@
 
 #define NDRV_VGA_FILENAME "qemu_vga.ndrv"
 
+#define PROM_FILENAME "openbios-ppc"
 #define PROM_BASE 0xfff00000
 #define PROM_SIZE (1 * MiB)
 
+#define KERNEL_LOAD_ADDR 0x01000000
+#define KERNEL_GAP       0x00100000
+
 #define TYPE_CORE99_MACHINE MACHINE_TYPE_NAME("mac99")
 typedef struct Core99MachineState Core99MachineState;
 DECLARE_INSTANCE_CHECKER(Core99MachineState, CORE99_MACHINE,
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 1355d032ff..cfdee21cd2 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -57,10 +57,15 @@
 
 #define NDRV_VGA_FILENAME "qemu_vga.ndrv"
 
-#define GRACKLE_BASE 0xfec00000
+#define PROM_FILENAME "openbios-ppc"
 #define PROM_BASE 0xffc00000
 #define PROM_SIZE (4 * MiB)
 
+#define KERNEL_LOAD_ADDR 0x01000000
+#define KERNEL_GAP       0x00100000
+
+#define GRACKLE_BASE 0xfec00000
+
 static void fw_cfg_boot_set(void *opaque, const char *boot_device,
                             Error **errp)
 {
-- 
2.30.4

[PATCH v2 12/13] hw/ppc/mac.h: Rename to include/hw/nvram/mac_nvram.h

[BALATON Zoltan]

All that is left in mac.h now belongs to the nvram emulation so rename
it accordingly and only include it where it is really used.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 MAINTAINERS                                  |  1 +
 hw/ide/macio.c                               |  1 -
 hw/intc/heathrow_pic.c                       |  1 -
 hw/intc/openpic.c                            |  1 -
 hw/misc/macio/cuda.c                         |  1 -
 hw/misc/macio/gpio.c                         |  1 -
 hw/misc/macio/macio.c                        |  1 -
 hw/misc/macio/pmu.c                          |  1 -
 hw/nvram/mac_nvram.c                         |  2 +-
 hw/pci-host/grackle.c                        |  1 -
 hw/pci-host/uninorth.c                       |  1 -
 hw/ppc/mac_newworld.c                        |  2 +-
 hw/ppc/mac_oldworld.c                        |  1 -
 include/hw/misc/macio/macio.h                |  2 +-
 hw/ppc/mac.h => include/hw/nvram/mac_nvram.h | 11 ++++++-----
 15 files changed, 10 insertions(+), 18 deletions(-)
 rename hw/ppc/mac.h => include/hw/nvram/mac_nvram.h (89%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 738c4eb647..41a4700979 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1328,6 +1328,7 @@ F: hw/nvram/mac_nvram.c
 F: hw/input/adb*
 F: include/hw/misc/macio/
 F: include/hw/misc/mos6522.h
+F: include/hw/nvram/mac_nvram.h
 F: include/hw/ppc/mac_dbdma.h
 F: include/hw/pci-host/uninorth.h
 F: include/hw/input/adb*
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index 1c15c37ec5..e604466acb 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -24,7 +24,6 @@
  */
 
 #include "qemu/osdep.h"
-#include "hw/ppc/mac.h"
 #include "hw/ppc/mac_dbdma.h"
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
diff --git a/hw/intc/heathrow_pic.c b/hw/intc/heathrow_pic.c
index cb97c315da..13048a2735 100644
--- a/hw/intc/heathrow_pic.c
+++ b/hw/intc/heathrow_pic.c
@@ -24,7 +24,6 @@
  */
 
 #include "qemu/osdep.h"
-#include "hw/ppc/mac.h"
 #include "migration/vmstate.h"
 #include "qemu/module.h"
 #include "hw/intc/heathrow_pic.h"
diff --git a/hw/intc/openpic.c b/hw/intc/openpic.c
index b0787e8ee7..c757adbe53 100644
--- a/hw/intc/openpic.c
+++ b/hw/intc/openpic.c
@@ -32,7 +32,6 @@
 
 #include "qemu/osdep.h"
 #include "hw/irq.h"
-#include "hw/ppc/mac.h"
 #include "hw/pci/pci.h"
 #include "hw/ppc/openpic.h"
 #include "hw/ppc/ppc_e500.h"
diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
index 1498113cfc..0d4c13319a 100644
--- a/hw/misc/macio/cuda.c
+++ b/hw/misc/macio/cuda.c
@@ -25,7 +25,6 @@
 
 #include "qemu/osdep.h"
 #include "hw/irq.h"
-#include "hw/ppc/mac.h"
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
 #include "hw/input/adb.h"
diff --git a/hw/misc/macio/gpio.c b/hw/misc/macio/gpio.c
index b1bcf830c3..c8ac5633b2 100644
--- a/hw/misc/macio/gpio.c
+++ b/hw/misc/macio/gpio.c
@@ -24,7 +24,6 @@
  */
 
 #include "qemu/osdep.h"
-#include "hw/ppc/mac.h"
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
 #include "hw/misc/macio/macio.h"
diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index f9f0758b03..93a7c7bbc8 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -26,7 +26,6 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu/module.h"
-#include "hw/ppc/mac.h"
 #include "hw/misc/macio/cuda.h"
 #include "hw/pci/pci.h"
 #include "hw/ppc/mac_dbdma.h"
diff --git a/hw/misc/macio/pmu.c b/hw/misc/macio/pmu.c
index 336502a84b..70562ed8d0 100644
--- a/hw/misc/macio/pmu.c
+++ b/hw/misc/macio/pmu.c
@@ -29,7 +29,6 @@
  */
 
 #include "qemu/osdep.h"
-#include "hw/ppc/mac.h"
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
 #include "hw/input/adb.h"
diff --git a/hw/nvram/mac_nvram.c b/hw/nvram/mac_nvram.c
index 11f2d31cdb..3d9ddda217 100644
--- a/hw/nvram/mac_nvram.c
+++ b/hw/nvram/mac_nvram.c
@@ -25,7 +25,7 @@
 
 #include "qemu/osdep.h"
 #include "hw/nvram/chrp_nvram.h"
-#include "hw/ppc/mac.h"
+#include "hw/nvram/mac_nvram.h"
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
 #include "qemu/cutils.h"
diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
index 5282123004..d9c11d22e0 100644
--- a/hw/pci-host/grackle.c
+++ b/hw/pci-host/grackle.c
@@ -25,7 +25,6 @@
 
 #include "qemu/osdep.h"
 #include "hw/pci/pci_host.h"
-#include "hw/ppc/mac.h"
 #include "hw/qdev-properties.h"
 #include "hw/pci/pci.h"
 #include "hw/irq.h"
diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c
index d25b62d6a5..aebd44d265 100644
--- a/hw/pci-host/uninorth.c
+++ b/hw/pci-host/uninorth.c
@@ -24,7 +24,6 @@
 
 #include "qemu/osdep.h"
 #include "hw/irq.h"
-#include "hw/ppc/mac.h"
 #include "hw/qdev-properties.h"
 #include "qemu/module.h"
 #include "hw/pci/pci.h"
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 94a8b72fb1..c0b2173cbd 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -52,7 +52,7 @@
 #include "qapi/error.h"
 #include "hw/ppc/ppc.h"
 #include "hw/qdev-properties.h"
-#include "hw/ppc/mac.h"
+#include "hw/nvram/mac_nvram.h"
 #include "hw/boards.h"
 #include "hw/pci-host/uninorth.h"
 #include "hw/input/adb.h"
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index cfdee21cd2..b96b6cc141 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -30,7 +30,6 @@
 #include "qapi/error.h"
 #include "hw/ppc/ppc.h"
 #include "hw/qdev-properties.h"
-#include "mac.h"
 #include "hw/boards.h"
 #include "hw/input/adb.h"
 #include "sysemu/sysemu.h"
diff --git a/include/hw/misc/macio/macio.h b/include/hw/misc/macio/macio.h
index 26cf15b1ce..95d30a1745 100644
--- a/include/hw/misc/macio/macio.h
+++ b/include/hw/misc/macio/macio.h
@@ -33,7 +33,7 @@
 #include "hw/misc/macio/cuda.h"
 #include "hw/misc/macio/gpio.h"
 #include "hw/misc/macio/pmu.h"
-#include "hw/ppc/mac.h"
+#include "hw/nvram/mac_nvram.h"
 #include "hw/ppc/mac_dbdma.h"
 #include "hw/ppc/openpic.h"
 #include "qom/object.h"
diff --git a/hw/ppc/mac.h b/include/hw/nvram/mac_nvram.h
similarity index 89%
rename from hw/ppc/mac.h
rename to include/hw/nvram/mac_nvram.h
index 3e2df262ee..baa9f6a5a6 100644
--- a/hw/ppc/mac.h
+++ b/include/hw/nvram/mac_nvram.h
@@ -1,5 +1,5 @@
 /*
- * QEMU PowerMac emulation shared definitions and prototypes
+ * PowerMac NVRAM emulation
  *
  * Copyright (c) 2004-2007 Fabrice Bellard
  * Copyright (c) 2007 Jocelyn Mayer
@@ -23,8 +23,8 @@
  * THE SOFTWARE.
  */
 
-#ifndef PPC_MAC_H
-#define PPC_MAC_H
+#ifndef MAC_NVRAM_H
+#define MAC_NVRAM_H
 
 #include "exec/memory.h"
 #include "hw/sysbus.h"
@@ -47,5 +47,6 @@ struct MacIONVRAMState {
     uint8_t *data;
 };
 
-void pmac_format_nvram_partition (MacIONVRAMState *nvr, int len);
-#endif /* PPC_MAC_H */
+void pmac_format_nvram_partition(MacIONVRAMState *nvr, int len);
+
+#endif /* MAC_NVRAM_H */
-- 
2.30.4

[PATCH v2 13/13] mac_nvram: Use NVRAM_SIZE constant

[BALATON Zoltan]

The NVRAM_SIZE constant was defined but not used. Rename it to
MACIO_NVRAM_SIZE to match the device model and use it where appropriate.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/misc/macio/macio.c        | 2 +-
 hw/ppc/mac_newworld.c        | 4 ++--
 include/hw/nvram/mac_nvram.h | 3 +--
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index 93a7c7bbc8..08dbdd7fc0 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -226,7 +226,7 @@ static void macio_oldworld_init(Object *obj)
 
     object_initialize_child(OBJECT(s), "nvram", &os->nvram, TYPE_MACIO_NVRAM);
     dev = DEVICE(&os->nvram);
-    qdev_prop_set_uint32(dev, "size", 0x2000);
+    qdev_prop_set_uint32(dev, "size", MACIO_NVRAM_SIZE);
     qdev_prop_set_uint32(dev, "it_shift", 4);
 
     for (i = 0; i < 2; i++) {
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index c0b2173cbd..37fb7845f1 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -445,12 +445,12 @@ static void ppc_core99_init(MachineState *machine)
         nvram_addr = 0xFFE00000;
     }
     dev = qdev_new(TYPE_MACIO_NVRAM);
-    qdev_prop_set_uint32(dev, "size", 0x2000);
+    qdev_prop_set_uint32(dev, "size", MACIO_NVRAM_SIZE);
     qdev_prop_set_uint32(dev, "it_shift", 1);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, nvram_addr);
     nvr = MACIO_NVRAM(dev);
-    pmac_format_nvram_partition(nvr, 0x2000);
+    pmac_format_nvram_partition(nvr, MACIO_NVRAM_SIZE);
     /* No PCI init: the BIOS will do it */
 
     dev = qdev_new(TYPE_FW_CFG_MEM);
diff --git a/include/hw/nvram/mac_nvram.h b/include/hw/nvram/mac_nvram.h
index baa9f6a5a6..b780aca470 100644
--- a/include/hw/nvram/mac_nvram.h
+++ b/include/hw/nvram/mac_nvram.h
@@ -29,9 +29,8 @@
 #include "exec/memory.h"
 #include "hw/sysbus.h"
 
-#define NVRAM_SIZE        0x2000
+#define MACIO_NVRAM_SIZE 0x2000
 
-/* Mac NVRAM */
 #define TYPE_MACIO_NVRAM "macio-nvram"
 OBJECT_DECLARE_SIMPLE_TYPE(MacIONVRAMState, MACIO_NVRAM)
 
-- 
2.30.4

Re: [PATCH v2 13/13] mac_nvram: Use NVRAM_SIZE constant

[Mark Cave-Ayland]

On 25/09/2022 13:38, BALATON Zoltan wrote:

> The NVRAM_SIZE constant was defined but not used. Rename it to
> MACIO_NVRAM_SIZE to match the device model and use it where appropriate.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>   hw/misc/macio/macio.c        | 2 +-
>   hw/ppc/mac_newworld.c        | 4 ++--
>   include/hw/nvram/mac_nvram.h | 3 +--
>   3 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
> index 93a7c7bbc8..08dbdd7fc0 100644
> --- a/hw/misc/macio/macio.c
> +++ b/hw/misc/macio/macio.c
> @@ -226,7 +226,7 @@ static void macio_oldworld_init(Object *obj)
>   
>       object_initialize_child(OBJECT(s), "nvram", &os->nvram, TYPE_MACIO_NVRAM);
>       dev = DEVICE(&os->nvram);
> -    qdev_prop_set_uint32(dev, "size", 0x2000);
> +    qdev_prop_set_uint32(dev, "size", MACIO_NVRAM_SIZE);
>       qdev_prop_set_uint32(dev, "it_shift", 4);
>   
>       for (i = 0; i < 2; i++) {
> diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
> index c0b2173cbd..37fb7845f1 100644
> --- a/hw/ppc/mac_newworld.c
> +++ b/hw/ppc/mac_newworld.c
> @@ -445,12 +445,12 @@ static void ppc_core99_init(MachineState *machine)
>           nvram_addr = 0xFFE00000;
>       }
>       dev = qdev_new(TYPE_MACIO_NVRAM);
> -    qdev_prop_set_uint32(dev, "size", 0x2000);
> +    qdev_prop_set_uint32(dev, "size", MACIO_NVRAM_SIZE);
>       qdev_prop_set_uint32(dev, "it_shift", 1);
>       sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>       sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, nvram_addr);
>       nvr = MACIO_NVRAM(dev);
> -    pmac_format_nvram_partition(nvr, 0x2000);
> +    pmac_format_nvram_partition(nvr, MACIO_NVRAM_SIZE);
>       /* No PCI init: the BIOS will do it */
>   
>       dev = qdev_new(TYPE_FW_CFG_MEM);
> diff --git a/include/hw/nvram/mac_nvram.h b/include/hw/nvram/mac_nvram.h
> index baa9f6a5a6..b780aca470 100644
> --- a/include/hw/nvram/mac_nvram.h
> +++ b/include/hw/nvram/mac_nvram.h
> @@ -29,9 +29,8 @@
>   #include "exec/memory.h"
>   #include "hw/sysbus.h"
>   
> -#define NVRAM_SIZE        0x2000
> +#define MACIO_NVRAM_SIZE 0x2000
>   
> -/* Mac NVRAM */
>   #define TYPE_MACIO_NVRAM "macio-nvram"
>   OBJECT_DECLARE_SIMPLE_TYPE(MacIONVRAMState, MACIO_NVRAM)

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


ATB,

Mark.

Re: [PATCH v2 00/13] Misc ppc/mac machines clean up

[Daniel Henrique Barboza]

Mark,


It seems that you're usually push mac changes via a qemu-macppc PR (git log
says that the last one was Jan 2021), so feel free to keep doing so.

If it's convenient for you I can pick them via ppc-next as well. Just let me
know.


Thanks,


Daniel

On 9/25/22 09:38, BALATON Zoltan wrote:
> This series includes some clean ups to mac_newworld and mac_oldworld
> to make them a bit simpler and more readable, It also removes the
> shared mac.h file that turns out was more of a random collection of
> unrelated things. Getting rid of this mac.h improves the locality of
> device models and reduces unnecessary interdependency.
> 
> v2: Split some patches and add a few more I've noticed now and address
> review comments
> 
> BALATON Zoltan (13):
>    mac_newworld: Drop some variables
>    mac_oldworld: Drop some more variables
>    mac_{old|new}world: Set tbfreq at declaration
>    mac_{old|new}world: Avoid else branch by setting default value
>    mac_oldworld: Do not open code sysbus_mmio_map()
>    mac_newworld: Simplify creation of Uninorth devices
>    mac_{old|new}world: Reduce number of QOM casts
>    hw/ppc/mac.h: Move newworld specific parts out from shared header
>    hw/ppc/mac.h: Move macio specific parts out from shared header
>    hw/ppc/mac.h: Move grackle-pcihost declaration out from shared header
>    hw/ppc/mac.h: Move PROM and KERNEL defines to board code
>    hw/ppc/mac.h: Rename to include/hw/nvram/mac_nvram.h
>    mac_nvram: Use NVRAM_SIZE constant
> 
>   MAINTAINERS                   |   1 +
>   hw/ide/macio.c                |   1 -
>   hw/intc/heathrow_pic.c        |   1 -
>   hw/intc/openpic.c             |   1 -
>   hw/misc/macio/cuda.c          |   1 -
>   hw/misc/macio/gpio.c          |   1 -
>   hw/misc/macio/macio.c         |   8 +-
>   hw/misc/macio/pmu.c           |   1 -
>   hw/nvram/mac_nvram.c          |   2 +-
>   hw/pci-host/grackle.c         |   2 +-
>   hw/pci-host/uninorth.c        |   1 -
>   hw/ppc/mac.h                  | 105 ----------------
>   hw/ppc/mac_newworld.c         | 223 ++++++++++++++++------------------
>   hw/ppc/mac_oldworld.c         | 113 +++++++----------
>   include/hw/misc/macio/macio.h |  23 +++-
>   include/hw/nvram/mac_nvram.h  |  51 ++++++++
>   16 files changed, 230 insertions(+), 305 deletions(-)
>   delete mode 100644 hw/ppc/mac.h
>   create mode 100644 include/hw/nvram/mac_nvram.h
> 

Re: [PATCH v2 00/13] Misc ppc/mac machines clean up

[Mark Cave-Ayland]

On 26/09/2022 22:40, Daniel Henrique Barboza wrote:

> Mark,
>  
> It seems that you're usually push mac changes via a qemu-macppc PR (git log
> says that the last one was Jan 2021), so feel free to keep doing so.
> 
> If it's convenient for you I can pick them via ppc-next as well. Just let me
> know.
> 
> Thanks,
> 
> Daniel

Thanks Daniel, I really appreciate the offer of help - I'll let you know if I need to 
take you up on it :)


ATB,

Mark.