All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 00/19] Misc ppc/mac machines clean up
@ 2022-10-25 16:44 BALATON Zoltan
  2022-10-25 16:44 ` [PATCH v4 01/19] mac_newworld: Drop some variables BALATON Zoltan
                   ` (23 more replies)
  0 siblings, 24 replies; 38+ messages in thread
From: BALATON Zoltan @ 2022-10-25 16:44 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Mark Cave-Ayland

Since only one week is left until freeze starts I've included some
more patches in this version that I've intended to submit after the
clean ups but we're running out of time now. The last 3 patches could
be squashed together, I've just split these up because I expect
resistence from Mark to any changes so maybe it's easier to digest
piece by piece and can cherry pick parts easier this way but ideally
these should be in one patch.

I'd appreciate very much if this series would get in before the
freeze, it is very discouraging to spend time with something that gets
ignored and then postponed for the rest of the year just to start
again the same in January. This might be a reason why not many people
contribute to this part of QEMU besides that maybe only a few people
are still interested so those who are interested should be served
better to not scare them off even more.

Regards,
BALATON Zoltan

v4: Add some more patches that I've found since v3 or was intended in
separate series
v3: Some more patch spliting and changes I've noticed and address more
review comments
v2: Split some patches and add a few more I've noticed now and address
review comments

BALATON Zoltan (19):
  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_{old|new}world: Simplify cmdline_base calculation
  mac_newworld: Clean up 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 type declaration out to a 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
  mac_{old|new}world: Code style fix adding missing braces to if-s
  mac_newworld: Turn CORE99_VIA_CONFIG defines into an enum
  mac_newworld: Add machine types for different mac99 configs
  mac_newworld: Deprecate mac99 with G5 CPU
  mac_newworld: Deprecate mac99 "via" option
  mac_newworld: Document deprecation

 MAINTAINERS                   |   2 +
 docs/about/deprecated.rst     |   7 +
 docs/system/ppc/powermac.rst  |  12 +-
 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         |  15 +-
 hw/pci-host/uninorth.c        |   1 -
 hw/ppc/mac.h                  | 105 -----------
 hw/ppc/mac_newworld.c         | 341 ++++++++++++++++++++++------------
 hw/ppc/mac_oldworld.c         | 120 ++++++------
 include/hw/misc/macio/macio.h |  23 ++-
 include/hw/nvram/mac_nvram.h  |  51 +++++
 include/hw/pci-host/grackle.h |  44 +++++
 19 files changed, 415 insertions(+), 322 deletions(-)
 delete mode 100644 hw/ppc/mac.h
 create mode 100644 include/hw/nvram/mac_nvram.h
 create mode 100644 include/hw/pci-host/grackle.h

-- 
2.30.4



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

* [PATCH v4 01/19] mac_newworld: Drop some variables
  2022-10-25 16:44 [PATCH v4 00/19] Misc ppc/mac machines clean up BALATON Zoltan
@ 2022-10-25 16:44 ` BALATON Zoltan
  2022-10-25 16:44 ` [PATCH v4 02/19] mac_oldworld: Drop some more variables BALATON Zoltan
                   ` (22 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: BALATON Zoltan @ 2022-10-25 16:44 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Mark Cave-Ayland

[-- 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



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

* [PATCH v4 02/19] mac_oldworld: Drop some more variables
  2022-10-25 16:44 [PATCH v4 00/19] Misc ppc/mac machines clean up BALATON Zoltan
  2022-10-25 16:44 ` [PATCH v4 01/19] mac_newworld: Drop some variables BALATON Zoltan
@ 2022-10-25 16:44 ` BALATON Zoltan
  2022-10-25 16:44 ` [PATCH v4 03/19] mac_{old|new}world: Set tbfreq at declaration BALATON Zoltan
                   ` (21 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: BALATON Zoltan @ 2022-10-25 16:44 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Mark Cave-Ayland

[-- 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



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

* [PATCH v4 03/19] mac_{old|new}world: Set tbfreq at declaration
  2022-10-25 16:44 [PATCH v4 00/19] Misc ppc/mac machines clean up BALATON Zoltan
  2022-10-25 16:44 ` [PATCH v4 01/19] mac_newworld: Drop some variables BALATON Zoltan
  2022-10-25 16:44 ` [PATCH v4 02/19] mac_oldworld: Drop some more variables BALATON Zoltan
@ 2022-10-25 16:44 ` BALATON Zoltan
  2022-10-25 16:44 ` [PATCH v4 04/19] mac_{old|new}world: Avoid else branch by setting default value BALATON Zoltan
                   ` (20 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: BALATON Zoltan @ 2022-10-25 16:44 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Mark Cave-Ayland

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>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 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



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

* [PATCH v4 04/19] mac_{old|new}world: Avoid else branch by setting default value
  2022-10-25 16:44 [PATCH v4 00/19] Misc ppc/mac machines clean up BALATON Zoltan
                   ` (2 preceding siblings ...)
  2022-10-25 16:44 ` [PATCH v4 03/19] mac_{old|new}world: Set tbfreq at declaration BALATON Zoltan
@ 2022-10-25 16:44 ` BALATON Zoltan
  2022-10-25 16:44 ` [PATCH v4 05/19] mac_{old|new}world: Simplify cmdline_base calculation BALATON Zoltan
                   ` (19 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: BALATON Zoltan @ 2022-10-25 16:44 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Mark Cave-Ayland

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



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

* [PATCH v4 05/19] mac_{old|new}world: Simplify cmdline_base calculation
  2022-10-25 16:44 [PATCH v4 00/19] Misc ppc/mac machines clean up BALATON Zoltan
                   ` (3 preceding siblings ...)
  2022-10-25 16:44 ` [PATCH v4 04/19] mac_{old|new}world: Avoid else branch by setting default value BALATON Zoltan
@ 2022-10-25 16:44 ` BALATON Zoltan
  2022-10-25 16:44 ` [PATCH v4 06/19] mac_newworld: Clean up creation of Uninorth devices BALATON Zoltan
                   ` (18 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: BALATON Zoltan @ 2022-10-25 16:44 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Mark Cave-Ayland

By slight reorganisation we can avoid an else branch and some code
duplication which makes it easier to follow the code.

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

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 6bc3bd19be..73b01e8c6d 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -194,9 +194,11 @@ static void ppc_core99_init(MachineState *machine)
                          machine->kernel_filename);
             exit(1);
         }
+        cmdline_base = TARGET_PAGE_ALIGN(kernel_base + kernel_size +
+                                         KERNEL_GAP);
         /* load initrd */
         if (machine->initrd_filename) {
-            initrd_base = TARGET_PAGE_ALIGN(kernel_base + kernel_size + KERNEL_GAP);
+            initrd_base = cmdline_base;
             initrd_size = load_image_targphys(machine->initrd_filename,
                                               initrd_base,
                                               machine->ram_size - initrd_base);
@@ -206,8 +208,6 @@ static void ppc_core99_init(MachineState *machine)
                 exit(1);
             }
             cmdline_base = TARGET_PAGE_ALIGN(initrd_base + initrd_size);
-        } else {
-            cmdline_base = TARGET_PAGE_ALIGN(kernel_base + kernel_size + KERNEL_GAP);
         }
         ppc_boot_device = 'm';
     } else {
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index cb67e44081..b424729a39 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -168,10 +168,11 @@ static void ppc_heathrow_init(MachineState *machine)
                          machine->kernel_filename);
             exit(1);
         }
+        cmdline_base = TARGET_PAGE_ALIGN(kernel_base + kernel_size +
+                                         KERNEL_GAP);
         /* load initrd */
         if (machine->initrd_filename) {
-            initrd_base = TARGET_PAGE_ALIGN(kernel_base + kernel_size +
-                                            KERNEL_GAP);
+            initrd_base = cmdline_base;
             initrd_size = load_image_targphys(machine->initrd_filename,
                                               initrd_base,
                                               machine->ram_size - initrd_base);
@@ -181,8 +182,6 @@ static void ppc_heathrow_init(MachineState *machine)
                 exit(1);
             }
             cmdline_base = TARGET_PAGE_ALIGN(initrd_base + initrd_size);
-        } else {
-            cmdline_base = TARGET_PAGE_ALIGN(kernel_base + kernel_size + KERNEL_GAP);
         }
         ppc_boot_device = 'm';
     } else {
-- 
2.30.4



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

* [PATCH v4 06/19] mac_newworld: Clean up creation of Uninorth devices
  2022-10-25 16:44 [PATCH v4 00/19] Misc ppc/mac machines clean up BALATON Zoltan
                   ` (4 preceding siblings ...)
  2022-10-25 16:44 ` [PATCH v4 05/19] mac_{old|new}world: Simplify cmdline_base calculation BALATON Zoltan
@ 2022-10-25 16:44 ` BALATON Zoltan
  2022-10-25 16:44 ` [PATCH v4 07/19] mac_{old|new}world: Reduce number of QOM casts BALATON Zoltan
                   ` (17 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: BALATON Zoltan @ 2022-10-25 16:44 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Mark Cave-Ayland

Map regions in ascending otder and eorganise code a bit to avoid some
casts and move Uninorth parts together.

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

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 73b01e8c6d..ccc52d8514 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,30 @@ 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);
+    memory_region_add_subregion(get_system_memory(), 0xf8000000,
+                                sysbus_mmio_get_region(s, 0));
+
     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);
+        sysbus_realize_and_unref(s, &error_fatal);
+        sysbus_mmio_map(s, 0, 0xf0800000);
+        sysbus_mmio_map(s, 1, 0xf0c00000);
         /* PCI hole */
-        memory_region_add_subregion(get_system_memory(), 0x80000000ULL,
+        memory_region_add_subregion(get_system_memory(), 0x80000000,
                                     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_mmio_map(s, 0, 0xf0800000);
-        sysbus_mmio_map(s, 1, 0xf0c00000);
-
-        machine_arch = ARCH_MAC99_U3;
     } 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);
@@ -309,22 +308,20 @@ static void ppc_core99_init(MachineState *machine)
         sysbus_mmio_map(s, 0, 0xf4800000);
         sysbus_mmio_map(s, 1, 0xf4c00000);
 
-        /* Uninorth main bus */
+        /* Uninorth main bus - this must be last to make it the default */
         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);
+        sysbus_realize_and_unref(s, &error_fatal);
+        sysbus_mmio_map(s, 0, 0xf2800000);
+        sysbus_mmio_map(s, 1, 0xf2c00000);
         /* PCI hole */
-        memory_region_add_subregion(get_system_memory(), 0x80000000ULL,
+        memory_region_add_subregion(get_system_memory(), 0x80000000,
                                     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_mmio_map(s, 0, 0xf2800000);
-        sysbus_mmio_map(s, 1, 0xf2c00000);
-
-        machine_arch = ARCH_MAC99;
     }
 
     machine->usb |= defaults_enabled() && !machine->usb_disabled;
-- 
2.30.4



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

* [PATCH v4 07/19] mac_{old|new}world: Reduce number of QOM casts
  2022-10-25 16:44 [PATCH v4 00/19] Misc ppc/mac machines clean up BALATON Zoltan
                   ` (5 preceding siblings ...)
  2022-10-25 16:44 ` [PATCH v4 06/19] mac_newworld: Clean up creation of Uninorth devices BALATON Zoltan
@ 2022-10-25 16:44 ` BALATON Zoltan
  2022-10-25 16:44 ` [PATCH v4 08/19] hw/ppc/mac.h: Move newworld specific parts out from shared header BALATON Zoltan
                   ` (16 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: BALATON Zoltan @ 2022-10-25 16:44 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Mark Cave-Ayland

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 10395 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 ccc52d8514..b176af66eb 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:
@@ -278,9 +277,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);
@@ -309,10 +307,9 @@ static void ppc_core99_init(MachineState *machine)
         sysbus_mmio_map(s, 1, 0xf4c00000);
 
         /* Uninorth main bus - this must be last to make it the default */
-        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);
@@ -330,24 +327,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));
     }
 
@@ -379,19 +376,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 b424729a39..be06ea04ff 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;
@@ -230,17 +229,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));
@@ -268,16 +266,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



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

* [PATCH v4 08/19] hw/ppc/mac.h: Move newworld specific parts out from shared header
  2022-10-25 16:44 [PATCH v4 00/19] Misc ppc/mac machines clean up BALATON Zoltan
                   ` (6 preceding siblings ...)
  2022-10-25 16:44 ` [PATCH v4 07/19] mac_{old|new}world: Reduce number of QOM casts BALATON Zoltan
@ 2022-10-25 16:44 ` BALATON Zoltan
  2022-10-25 16:44 ` [PATCH v4 09/19] hw/ppc/mac.h: Move macio " BALATON Zoltan
                   ` (15 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: BALATON Zoltan @ 2022-10-25 16:44 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Mark Cave-Ayland

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 b176af66eb..957f301d6c 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 be06ea04ff..a10c884503 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



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

* [PATCH v4 09/19] hw/ppc/mac.h: Move macio specific parts out from shared header
  2022-10-25 16:44 [PATCH v4 00/19] Misc ppc/mac machines clean up BALATON Zoltan
                   ` (7 preceding siblings ...)
  2022-10-25 16:44 ` [PATCH v4 08/19] hw/ppc/mac.h: Move newworld specific parts out from shared header BALATON Zoltan
@ 2022-10-25 16:44 ` BALATON Zoltan
  2022-10-25 16:44 ` [PATCH v4 10/19] hw/ppc/mac.h: Move grackle-pcihost type declaration out to a header BALATON Zoltan
                   ` (14 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: BALATON Zoltan @ 2022-10-25 16:44 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Mark Cave-Ayland

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 3046 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>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 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



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

* [PATCH v4 10/19] hw/ppc/mac.h: Move grackle-pcihost type declaration out to a header
  2022-10-25 16:44 [PATCH v4 00/19] Misc ppc/mac machines clean up BALATON Zoltan
                   ` (8 preceding siblings ...)
  2022-10-25 16:44 ` [PATCH v4 09/19] hw/ppc/mac.h: Move macio " BALATON Zoltan
@ 2022-10-25 16:44 ` BALATON Zoltan
  2022-10-25 16:44 ` [PATCH v4 11/19] hw/ppc/mac.h: Move PROM and KERNEL defines to board code BALATON Zoltan
                   ` (13 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: BALATON Zoltan @ 2022-10-25 16:44 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Mark Cave-Ayland

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 MAINTAINERS                   |  1 +
 hw/pci-host/grackle.c         | 14 +----------
 hw/ppc/mac.h                  |  3 ---
 hw/ppc/mac_oldworld.c         |  1 +
 include/hw/pci-host/grackle.h | 44 +++++++++++++++++++++++++++++++++++
 5 files changed, 47 insertions(+), 16 deletions(-)
 create mode 100644 include/hw/pci-host/grackle.h

diff --git a/MAINTAINERS b/MAINTAINERS
index e3d5b7e09c..021b99492c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1345,6 +1345,7 @@ F: hw/intc/heathrow_pic.c
 F: hw/input/adb*
 F: include/hw/intc/heathrow_pic.h
 F: include/hw/input/adb*
+F: include/hw/pci-host/grackle.h
 F: pc-bios/qemu_vga.ndrv
 
 PReP
diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
index b05facf463..e4c7303859 100644
--- a/hw/pci-host/grackle.c
+++ b/hw/pci-host/grackle.c
@@ -24,7 +24,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"
@@ -33,18 +32,7 @@
 #include "qemu/module.h"
 #include "trace.h"
 #include "qom/object.h"
-
-OBJECT_DECLARE_SIMPLE_TYPE(GrackleState, GRACKLE_PCI_HOST_BRIDGE)
-
-struct GrackleState {
-    PCIHostState parent_obj;
-
-    uint32_t ofw_addr;
-    qemu_irq irqs[4];
-    MemoryRegion pci_mmio;
-    MemoryRegion pci_hole;
-    MemoryRegion pci_io;
-};
+#include "hw/pci-host/grackle.h"
 
 /* Don't know if this matches real hardware, but it agrees with OHW.  */
 static int pci_grackle_map_irq(PCIDevice *pci_dev, int irq_num)
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 a10c884503..e1a22f8eba 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -38,6 +38,7 @@
 #include "hw/isa/isa.h"
 #include "hw/pci/pci.h"
 #include "hw/pci/pci_host.h"
+#include "hw/pci-host/grackle.h"
 #include "hw/nvram/fw_cfg.h"
 #include "hw/char/escc.h"
 #include "hw/misc/macio/macio.h"
diff --git a/include/hw/pci-host/grackle.h b/include/hw/pci-host/grackle.h
new file mode 100644
index 0000000000..7ad3a779f0
--- /dev/null
+++ b/include/hw/pci-host/grackle.h
@@ -0,0 +1,44 @@
+/*
+ * QEMU Grackle PCI host (heathrow OldWorld PowerMac)
+ *
+ * Copyright (c) 2006-2007 Fabrice Bellard
+ * Copyright (c) 2007 Jocelyn Mayer
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef GRACKLE_H
+#define GRACKLE_H
+
+#include "hw/pci/pci_host.h"
+
+#define TYPE_GRACKLE_PCI_HOST_BRIDGE "grackle-pcihost"
+OBJECT_DECLARE_SIMPLE_TYPE(GrackleState, GRACKLE_PCI_HOST_BRIDGE)
+
+struct GrackleState {
+    PCIHostState parent_obj;
+
+    uint32_t ofw_addr;
+    qemu_irq irqs[4];
+    MemoryRegion pci_mmio;
+    MemoryRegion pci_hole;
+    MemoryRegion pci_io;
+};
+
+#endif
-- 
2.30.4



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

* [PATCH v4 11/19] hw/ppc/mac.h: Move PROM and KERNEL defines to board code
  2022-10-25 16:44 [PATCH v4 00/19] Misc ppc/mac machines clean up BALATON Zoltan
                   ` (9 preceding siblings ...)
  2022-10-25 16:44 ` [PATCH v4 10/19] hw/ppc/mac.h: Move grackle-pcihost type declaration out to a header BALATON Zoltan
@ 2022-10-25 16:44 ` BALATON Zoltan
  2022-10-25 16:44 ` [PATCH v4 12/19] hw/ppc/mac.h: Rename to include/hw/nvram/mac_nvram.h BALATON Zoltan
                   ` (12 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: BALATON Zoltan @ 2022-10-25 16:44 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Mark Cave-Ayland

[-- 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 957f301d6c..c05e29bf67 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 e1a22f8eba..5213cbcc04 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -58,10 +58,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



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

* [PATCH v4 12/19] hw/ppc/mac.h: Rename to include/hw/nvram/mac_nvram.h
  2022-10-25 16:44 [PATCH v4 00/19] Misc ppc/mac machines clean up BALATON Zoltan
                   ` (10 preceding siblings ...)
  2022-10-25 16:44 ` [PATCH v4 11/19] hw/ppc/mac.h: Move PROM and KERNEL defines to board code BALATON Zoltan
@ 2022-10-25 16:44 ` BALATON Zoltan
  2022-10-25 16:44 ` [PATCH v4 13/19] mac_nvram: Use NVRAM_SIZE constant BALATON Zoltan
                   ` (11 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: BALATON Zoltan @ 2022-10-25 16:44 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Mark Cave-Ayland

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 021b99492c..54553093af 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 e4c7303859..95945ac0f4 100644
--- a/hw/pci-host/grackle.c
+++ b/hw/pci-host/grackle.c
@@ -24,7 +24,6 @@
  */
 
 #include "qemu/osdep.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 c05e29bf67..0a72da89ca 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 5213cbcc04..e2b5dd0650 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



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

* [PATCH v4 13/19] mac_nvram: Use NVRAM_SIZE constant
  2022-10-25 16:44 [PATCH v4 00/19] Misc ppc/mac machines clean up BALATON Zoltan
                   ` (11 preceding siblings ...)
  2022-10-25 16:44 ` [PATCH v4 12/19] hw/ppc/mac.h: Rename to include/hw/nvram/mac_nvram.h BALATON Zoltan
@ 2022-10-25 16:44 ` BALATON Zoltan
  2022-10-25 16:44 ` [PATCH v4 14/19] mac_{old|new}world: Code style fix adding missing braces to if-s BALATON Zoltan
                   ` (10 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: BALATON Zoltan @ 2022-10-25 16:44 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Mark Cave-Ayland

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>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 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 0a72da89ca..5d2fd69f35 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -450,12 +450,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



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

* [PATCH v4 14/19] mac_{old|new}world: Code style fix adding missing braces to if-s
  2022-10-25 16:44 [PATCH v4 00/19] Misc ppc/mac machines clean up BALATON Zoltan
                   ` (12 preceding siblings ...)
  2022-10-25 16:44 ` [PATCH v4 13/19] mac_nvram: Use NVRAM_SIZE constant BALATON Zoltan
@ 2022-10-25 16:44 ` BALATON Zoltan
  2022-10-25 16:44 ` [PATCH v4 15/19] mac_newworld: Turn CORE99_VIA_CONFIG defines into an enum BALATON Zoltan
                   ` (9 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: BALATON Zoltan @ 2022-10-25 16:44 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Mark Cave-Ayland

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

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 5d2fd69f35..4f5876670f 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -202,14 +202,16 @@ static void ppc_core99_init(MachineState *machine)
         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)
+        if (kernel_size < 0) {
             kernel_size = load_aout(machine->kernel_filename, kernel_base,
                                     machine->ram_size - kernel_base,
                                     bswap_needed, TARGET_PAGE_SIZE);
-        if (kernel_size < 0)
+        }
+        if (kernel_size < 0) {
             kernel_size = load_image_targphys(machine->kernel_filename,
                                               kernel_base,
                                               machine->ram_size - kernel_base);
+        }
         if (kernel_size < 0) {
             error_report("could not load kernel '%s'",
                          machine->kernel_filename);
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index e2b5dd0650..eecc54da59 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -160,14 +160,16 @@ static void ppc_heathrow_init(MachineState *machine)
         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)
+        if (kernel_size < 0) {
             kernel_size = load_aout(machine->kernel_filename, kernel_base,
                                     machine->ram_size - kernel_base,
                                     bswap_needed, TARGET_PAGE_SIZE);
-        if (kernel_size < 0)
+        }
+        if (kernel_size < 0) {
             kernel_size = load_image_targphys(machine->kernel_filename,
                                               kernel_base,
                                               machine->ram_size - kernel_base);
+        }
         if (kernel_size < 0) {
             error_report("could not load kernel '%s'",
                          machine->kernel_filename);
@@ -290,8 +292,9 @@ static void ppc_heathrow_init(MachineState *machine)
         pci_create_simple(pci_bus, -1, "pci-ohci");
     }
 
-    if (graphic_depth != 15 && graphic_depth != 32 && graphic_depth != 8)
+    if (graphic_depth != 15 && graphic_depth != 32 && graphic_depth != 8) {
         graphic_depth = 15;
+    }
 
     /* No PCI init: the BIOS will do it */
 
-- 
2.30.4



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

* [PATCH v4 15/19] mac_newworld: Turn CORE99_VIA_CONFIG defines into an enum
  2022-10-25 16:44 [PATCH v4 00/19] Misc ppc/mac machines clean up BALATON Zoltan
                   ` (13 preceding siblings ...)
  2022-10-25 16:44 ` [PATCH v4 14/19] mac_{old|new}world: Code style fix adding missing braces to if-s BALATON Zoltan
@ 2022-10-25 16:44 ` BALATON Zoltan
  2022-10-25 16:44 ` [PATCH v4 16/19] mac_newworld: Add machine types for different mac99 configs BALATON Zoltan
                   ` (8 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: BALATON Zoltan @ 2022-10-25 16:44 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Mark Cave-Ayland

This might allow the compiler to check values.

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

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 4f5876670f..bcd6566ead 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -95,15 +95,17 @@ 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
+typedef enum {
+    CORE99_VIA_CONFIG_CUDA = 0,
+    CORE99_VIA_CONFIG_PMU,
+    CORE99_VIA_CONFIG_PMU_ADB
+} Core99ViaConfig;
 
 struct Core99MachineState {
     /*< private >*/
     MachineState parent;
 
-    uint8_t via_config;
+    Core99ViaConfig via_config;
 };
 
 static void fw_cfg_boot_set(void *opaque, const char *boot_device,
-- 
2.30.4



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

* [PATCH v4 16/19] mac_newworld: Add machine types for different mac99 configs
  2022-10-25 16:44 [PATCH v4 00/19] Misc ppc/mac machines clean up BALATON Zoltan
                   ` (14 preceding siblings ...)
  2022-10-25 16:44 ` [PATCH v4 15/19] mac_newworld: Turn CORE99_VIA_CONFIG defines into an enum BALATON Zoltan
@ 2022-10-25 16:44 ` BALATON Zoltan
  2022-10-28  9:37   ` Mark Cave-Ayland
  2022-10-25 16:44 ` [PATCH v4 17/19] mac_newworld: Deprecate mac99 with G5 CPU BALATON Zoltan
                   ` (7 subsequent siblings)
  23 siblings, 1 reply; 38+ messages in thread
From: BALATON Zoltan @ 2022-10-25 16:44 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Mark Cave-Ayland

The mac99 machine emulates different machines depending on machine
properties or even if it is run as qemu-system-ppc64 or
qemu-system-ppc. This is very confusing for users and many hours were
lost trying to explain it or finding out why commands users came up
with are not working as expected. (E.g. Windows users might think
qemu-system-ppc64 is just the 64 bit version of qemu-system-ppc and
then fail to boot a 32 bit OS with -M mac99 trying to follow an
example that had qemu-system-ppc.) To avoid such confusion, add
explicit machine types for the different configs which will work the
same with both qemu-system-ppc and qemu-system-ppc64 and also make the
command line clearer for new users.

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

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index bcd6566ead..7321ac925e 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -649,9 +649,103 @@ static const TypeInfo core99_machine_info = {
     },
 };
 
+static void powermac3_1_machine_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+
+    core99_machine_class_init(oc, data);
+    mc->desc = "Apple Power Mac G4 AGP (Sawtooth)";
+    mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("7400_v2.9");
+}
+
+static void powermac3_1_instance_init(Object *obj)
+{
+    Core99MachineState *cms = CORE99_MACHINE(obj);
+
+    cms->via_config = CORE99_VIA_CONFIG_PMU;
+    return;
+}
+
+static const TypeInfo powermac3_1_machine_info = {
+    .name          = MACHINE_TYPE_NAME("powermac3_1"),
+    .parent        = TYPE_MACHINE,
+    .class_init    = powermac3_1_machine_class_init,
+    .instance_init = powermac3_1_instance_init,
+    .instance_size = sizeof(Core99MachineState),
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_FW_PATH_PROVIDER },
+        { }
+    },
+};
+
+static void powerbook3_2_machine_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+
+    core99_machine_class_init(oc, data);
+    mc->desc = "Apple PowerBook G4 Titanium (Mercury)";
+    mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("7400_v2.9");
+}
+
+static void powerbook3_2_instance_init(Object *obj)
+{
+    Core99MachineState *cms = CORE99_MACHINE(obj);
+
+    cms->via_config = CORE99_VIA_CONFIG_PMU_ADB;
+    return;
+}
+
+static const TypeInfo powerbook3_2_machine_info = {
+    .name          = MACHINE_TYPE_NAME("powerbook3_2"),
+    .parent        = TYPE_MACHINE,
+    .class_init    = powerbook3_2_machine_class_init,
+    .instance_init = powerbook3_2_instance_init,
+    .instance_size = sizeof(Core99MachineState),
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_FW_PATH_PROVIDER },
+        { }
+    },
+};
+
+#ifdef TARGET_PPC64
+static void powermac7_3_machine_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+
+    core99_machine_class_init(oc, data);
+    mc->desc = "Apple Power Mac G5 (Niagara)";
+    mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("970fx_v3.1");
+}
+
+static void powermac7_3_instance_init(Object *obj)
+{
+    Core99MachineState *cms = CORE99_MACHINE(obj);
+
+    cms->via_config = CORE99_VIA_CONFIG_PMU;
+    return;
+}
+
+static const TypeInfo powermac7_3_machine_info = {
+    .name          = MACHINE_TYPE_NAME("powermac7_3"),
+    .parent        = TYPE_MACHINE,
+    .class_init    = powermac7_3_machine_class_init,
+    .instance_init = powermac7_3_instance_init,
+    .instance_size = sizeof(Core99MachineState),
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_FW_PATH_PROVIDER },
+        { }
+    },
+};
+#endif
+
 static void mac_machine_register_types(void)
 {
     type_register_static(&core99_machine_info);
+    type_register_static(&powermac3_1_machine_info);
+    type_register_static(&powerbook3_2_machine_info);
+#ifdef TARGET_PPC64
+    type_register_static(&powermac7_3_machine_info);
+#endif
 }
 
 type_init(mac_machine_register_types)
-- 
2.30.4



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

* [PATCH v4 17/19] mac_newworld: Deprecate mac99 with G5 CPU
  2022-10-25 16:44 [PATCH v4 00/19] Misc ppc/mac machines clean up BALATON Zoltan
                   ` (15 preceding siblings ...)
  2022-10-25 16:44 ` [PATCH v4 16/19] mac_newworld: Add machine types for different mac99 configs BALATON Zoltan
@ 2022-10-25 16:44 ` BALATON Zoltan
  2022-10-25 16:44 ` [PATCH v4 18/19] mac_newworld: Deprecate mac99 "via" option BALATON Zoltan
                   ` (6 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: BALATON Zoltan @ 2022-10-25 16:44 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Mark Cave-Ayland

Besides resolving the confusing behaviour mentioned in previous commit
this might also allow unifying qemu-system-ppc and qemu-system-ppc64
in the future.

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

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 7321ac925e..d6f504eef2 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -165,6 +165,12 @@ static void ppc_core99_init(MachineState *machine)
         qemu_register_reset(ppc_core99_reset, cpu);
     }
 
+    if (object_property_find(OBJECT(machine), "via")) {
+        if (PPC_INPUT(env) == PPC_FLAGS_INPUT_970) {
+            warn_report("mac99 with G5 CPU is deprecated, "
+                        "use powermac7_3 instead");
+        }
+    }
     /* allocate RAM */
     if (machine->ram_size > 2 * GiB) {
         error_report("RAM size more than 2 GiB is not supported");
-- 
2.30.4



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

* [PATCH v4 18/19] mac_newworld: Deprecate mac99 "via" option
  2022-10-25 16:44 [PATCH v4 00/19] Misc ppc/mac machines clean up BALATON Zoltan
                   ` (16 preceding siblings ...)
  2022-10-25 16:44 ` [PATCH v4 17/19] mac_newworld: Deprecate mac99 with G5 CPU BALATON Zoltan
@ 2022-10-25 16:44 ` BALATON Zoltan
  2022-10-25 16:44 ` [PATCH v4 19/19] mac_newworld: Document deprecation BALATON Zoltan
                   ` (5 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: BALATON Zoltan @ 2022-10-25 16:44 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Mark Cave-Ayland

Setting emulated machine type with a property called "via" is
confusing users so deprecate the "via" option in favour of newly added
explicit machine types. The default via=cuda option is not a valid
config (no real Mac has this combination of hardware) so no machine
type could be defined for that therefore it is kept for backwards
compatibility with older QEMU versions for now but other options
resembling real machines are deprecated.

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

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index d6f504eef2..de4a7bae12 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -169,6 +169,15 @@ static void ppc_core99_init(MachineState *machine)
         if (PPC_INPUT(env) == PPC_FLAGS_INPUT_970) {
             warn_report("mac99 with G5 CPU is deprecated, "
                         "use powermac7_3 instead");
+        } else {
+            if (core99_machine->via_config == CORE99_VIA_CONFIG_PMU) {
+                warn_report("mac99,via=pmu is deprecated, "
+                            "use powermac3_1 instead");
+            }
+            if (core99_machine->via_config == CORE99_VIA_CONFIG_PMU_ADB) {
+                warn_report("mac99,via=pmu-adb is deprecated, "
+                            "use powerbook3_2 instead");
+            }
         }
     }
     /* allocate RAM */
-- 
2.30.4



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

* [PATCH v4 19/19] mac_newworld: Document deprecation
  2022-10-25 16:44 [PATCH v4 00/19] Misc ppc/mac machines clean up BALATON Zoltan
                   ` (17 preceding siblings ...)
  2022-10-25 16:44 ` [PATCH v4 18/19] mac_newworld: Deprecate mac99 "via" option BALATON Zoltan
@ 2022-10-25 16:44 ` BALATON Zoltan
  2022-10-28  9:41   ` Mark Cave-Ayland
  2022-10-25 21:31 ` [PATCH v5 19/20] " BALATON Zoltan
                   ` (4 subsequent siblings)
  23 siblings, 1 reply; 38+ messages in thread
From: BALATON Zoltan @ 2022-10-25 16:44 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Mark Cave-Ayland

Also update PowerMac family docs with some more recent info.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 docs/about/deprecated.rst    |  7 +++++++
 docs/system/ppc/powermac.rst | 12 ++++++++----
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 93affe3669..07661af7fe 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -248,6 +248,13 @@ These old machine types are quite neglected nowadays and thus might have
 various pitfalls with regards to live migration. Use a newer machine type
 instead.
 
+``mac99`` variants other than the default qemu-system-ppc version (since 7.2)
+'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+The ``mac99`` machine emulates different hardware depending on using
+qemu-system-ppc64 or ``via`` property. To avoid confusion new machine
+types has been added for these variants which are now preferred over
+``mac99``.
 
 Backend options
 ---------------
diff --git a/docs/system/ppc/powermac.rst b/docs/system/ppc/powermac.rst
index 04334ba210..9a37e69b1b 100644
--- a/docs/system/ppc/powermac.rst
+++ b/docs/system/ppc/powermac.rst
@@ -4,8 +4,12 @@ PowerMac family boards (``g3beige``, ``mac99``)
 Use the executable ``qemu-system-ppc`` to simulate a complete PowerMac
 PowerPC system.
 
-- ``g3beige``              Heathrow based PowerMAC
-- ``mac99``                Mac99 based PowerMAC
+- ``g3beige``           Heathrow based old world Power Macintosh G3
+- ``mac99``             Core99 based generic PowerMac
+- ``powermac3_1``       Power Mac G4 AGP (Sawtooth)
+- ``powerbook3_2``      PowerBook G4 Titanium (Mercury)
+- ``powermac7_3``       Power Mac G5 (Niagara) (only in ``qemu-system-ppc64``)
+
 
 Supported devices
 -----------------
@@ -15,9 +19,9 @@ QEMU emulates the following PowerMac peripherals:
  *  UniNorth or Grackle PCI Bridge
  *  PCI VGA compatible card with VESA Bochs Extensions
  *  2 PMAC IDE interfaces with hard disk and CD-ROM support
- *  NE2000 PCI adapters
+ *  Sungem PCI network adapter
  *  Non Volatile RAM
- *  VIA-CUDA with ADB keyboard and mouse.
+ *  VIA-CUDA or VIA-PMU99 with ot without ADB or USB keyboard and mouse.
 
 
 Missing devices
-- 
2.30.4



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

* [PATCH v5 19/20] mac_newworld: Document deprecation
  2022-10-25 16:44 [PATCH v4 00/19] Misc ppc/mac machines clean up BALATON Zoltan
                   ` (18 preceding siblings ...)
  2022-10-25 16:44 ` [PATCH v4 19/19] mac_newworld: Document deprecation BALATON Zoltan
@ 2022-10-25 21:31 ` BALATON Zoltan
  2022-10-25 21:31 ` [PATCH v5 20/20] mac_{old, new}world: Pass MacOS VGA NDRV in card ROM instead of fw_cfg BALATON Zoltan
                   ` (3 subsequent siblings)
  23 siblings, 0 replies; 38+ messages in thread
From: BALATON Zoltan @ 2022-10-25 21:31 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Mark Cave-Ayland

Also update PowerMac family docs with some more recent info.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 docs/about/deprecated.rst    |  7 +++++++
 docs/system/ppc/powermac.rst | 12 ++++++++----
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 93affe3669..07661af7fe 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -248,6 +248,13 @@ These old machine types are quite neglected nowadays and thus might have
 various pitfalls with regards to live migration. Use a newer machine type
 instead.
 
+``mac99`` variants other than the default qemu-system-ppc version (since 7.2)
+'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+The ``mac99`` machine emulates different hardware depending on using
+qemu-system-ppc64 or ``via`` property. To avoid confusion new machine
+types has been added for these variants which are now preferred over
+``mac99``.
 
 Backend options
 ---------------
diff --git a/docs/system/ppc/powermac.rst b/docs/system/ppc/powermac.rst
index 04334ba210..d4a47a6881 100644
--- a/docs/system/ppc/powermac.rst
+++ b/docs/system/ppc/powermac.rst
@@ -4,8 +4,12 @@ PowerMac family boards (``g3beige``, ``mac99``)
 Use the executable ``qemu-system-ppc`` to simulate a complete PowerMac
 PowerPC system.
 
-- ``g3beige``              Heathrow based PowerMAC
-- ``mac99``                Mac99 based PowerMAC
+- ``g3beige``           Heathrow based old world Power Macintosh G3
+- ``mac99``             Core99 based generic PowerMac
+- ``powermac3_1``       Power Mac G4 AGP (Sawtooth)
+- ``powerbook3_2``      PowerBook G4 Titanium (Mercury)
+- ``powermac7_3``       Power Mac G5 (Niagara) (only in ``qemu-system-ppc64``)
+
 
 Supported devices
 -----------------
@@ -15,9 +19,9 @@ QEMU emulates the following PowerMac peripherals:
  *  UniNorth or Grackle PCI Bridge
  *  PCI VGA compatible card with VESA Bochs Extensions
  *  2 PMAC IDE interfaces with hard disk and CD-ROM support
- *  NE2000 PCI adapters
+ *  Sungem PCI network adapter
  *  Non Volatile RAM
- *  VIA-CUDA with ADB keyboard and mouse.
+ *  VIA-CUDA or VIA-PMU99 with or without ADB or USB keyboard and mouse.
 
 
 Missing devices
-- 
2.30.4



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

* [PATCH v5 20/20] mac_{old, new}world: Pass MacOS VGA NDRV in card ROM instead of fw_cfg
  2022-10-25 16:44 [PATCH v4 00/19] Misc ppc/mac machines clean up BALATON Zoltan
                   ` (19 preceding siblings ...)
  2022-10-25 21:31 ` [PATCH v5 19/20] " BALATON Zoltan
@ 2022-10-25 21:31 ` BALATON Zoltan
  2022-10-28  9:51   ` Mark Cave-Ayland
  2022-10-25 21:35 ` [PATCH v4 00/19] Misc ppc/mac machines clean up BALATON Zoltan
                   ` (2 subsequent siblings)
  23 siblings, 1 reply; 38+ messages in thread
From: BALATON Zoltan @ 2022-10-25 21:31 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Mark Cave-Ayland

OpenBIOS cannot run FCode ROMs yet but it can detect NDRV in VGA card
ROM and add it to the device tree for MacOS. Pass the NDRV this way
instead of via fw_cfg. This solves the problem with OpenBIOS also
adding the NDRV to ati-vga which it does not work with. This does not
need any changes to OpenBIOS as this NDRV ROM handling is already
there but this patch also allows simplifying OpenBIOS later to remove
the fw_cfg ndrv handling from the vga FCode and also drop the
vga-ndrv? option which is not needed any more as users can disable the
ndrv with -device VGA,romfile="" (or override it with their own NDRV
or ROM). Once FCode support is implemented in OpenBIOS, the proper
FCode ROM can be set the same way so this paves the way to remove some
hacks.

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

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index de4a7bae12..1d12bd85ed 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -526,18 +526,6 @@ static void ppc_core99_init(MachineState *machine)
     fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_BUSFREQ, BUSFREQ);
     fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_NVRAM_ADDR, nvram_addr);
 
-    /* MacOS NDRV VGA driver */
-    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, NDRV_VGA_FILENAME);
-    if (filename) {
-        gchar *ndrv_file;
-        gsize ndrv_size;
-
-        if (g_file_get_contents(filename, &ndrv_file, &ndrv_size, NULL)) {
-            fw_cfg_add_file(fw_cfg, "ndrv/qemu_vga.ndrv", ndrv_file, ndrv_size);
-        }
-        g_free(filename);
-    }
-
     qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
 }
 
@@ -581,6 +569,11 @@ static int core99_kvm_type(MachineState *machine, const char *arg)
     return 2;
 }
 
+static GlobalProperty props[] = {
+    /* MacOS NDRV VGA driver */
+    { "VGA", "romfile", NDRV_VGA_FILENAME },
+};
+
 static void core99_machine_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
@@ -601,6 +594,7 @@ static void core99_machine_class_init(ObjectClass *oc, void *data)
 #endif
     mc->default_ram_id = "ppc_core99.ram";
     mc->ignore_boot_device_suffixes = true;
+    compat_props_add(mc->compat_props, props, G_N_ELEMENTS(props));
     fwc->get_dev_path = core99_fw_dev_path;
 }
 
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index eecc54da59..e7d35135d6 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -344,18 +344,6 @@ static void ppc_heathrow_init(MachineState *machine)
     fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_CLOCKFREQ, CLOCKFREQ);
     fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_BUSFREQ, BUSFREQ);
 
-    /* MacOS NDRV VGA driver */
-    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, NDRV_VGA_FILENAME);
-    if (filename) {
-        gchar *ndrv_file;
-        gsize ndrv_size;
-
-        if (g_file_get_contents(filename, &ndrv_file, &ndrv_size, NULL)) {
-            fw_cfg_add_file(fw_cfg, "ndrv/qemu_vga.ndrv", ndrv_file, ndrv_size);
-        }
-        g_free(filename);
-    }
-
     qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
 }
 
@@ -400,6 +388,11 @@ static int heathrow_kvm_type(MachineState *machine, const char *arg)
     return 2;
 }
 
+static GlobalProperty props[] = {
+    /* MacOS NDRV VGA driver */
+    { "VGA", "romfile", NDRV_VGA_FILENAME },
+};
+
 static void heathrow_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
@@ -420,6 +413,7 @@ static void heathrow_class_init(ObjectClass *oc, void *data)
     mc->default_display = "std";
     mc->ignore_boot_device_suffixes = true;
     mc->default_ram_id = "ppc_heathrow.ram";
+    compat_props_add(mc->compat_props, props, G_N_ELEMENTS(props));
     fwc->get_dev_path = heathrow_fw_dev_path;
 }
 
-- 
2.30.4



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

* Re: [PATCH v4 00/19] Misc ppc/mac machines clean up
  2022-10-25 16:44 [PATCH v4 00/19] Misc ppc/mac machines clean up BALATON Zoltan
                   ` (20 preceding siblings ...)
  2022-10-25 21:31 ` [PATCH v5 20/20] mac_{old, new}world: Pass MacOS VGA NDRV in card ROM instead of fw_cfg BALATON Zoltan
@ 2022-10-25 21:35 ` BALATON Zoltan
  2022-10-27  5:41 ` Howard Spoelstra
  2022-10-28  9:07 ` Mark Cave-Ayland
  23 siblings, 0 replies; 38+ messages in thread
From: BALATON Zoltan @ 2022-10-25 21:35 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Mark Cave-Ayland

On Tue, 25 Oct 2022, BALATON Zoltan wrote:
> Since only one week is left until freeze starts I've included some
> more patches in this version that I've intended to submit after the
> clean ups but we're running out of time now. The last 3 patches could
> be squashed together, I've just split these up because I expect
> resistence from Mark to any changes so maybe it's easier to digest
> piece by piece and can cherry pick parts easier this way but ideally
> these should be in one patch.
>
> I'd appreciate very much if this series would get in before the
> freeze, it is very discouraging to spend time with something that gets
> ignored and then postponed for the rest of the year just to start
> again the same in January. This might be a reason why not many people
> contribute to this part of QEMU besides that maybe only a few people
> are still interested so those who are interested should be served
> better to not scare them off even more.

Found a typo in the last (docs) patch so resent a v5 for just this patch 
and added another patch that could go in now as it works without OpenBIOS 
changes now so changes this allows in OpenBIOS can be done later at any 
time independently but it fixes the problem with ati-vga and qemu_vga.ndrv 
so it's useful for that alone.

Regards,
BALATON Zoltan

> v4: Add some more patches that I've found since v3 or was intended in
> separate series
> v3: Some more patch spliting and changes I've noticed and address more
> review comments
> v2: Split some patches and add a few more I've noticed now and address
> review comments
>
> BALATON Zoltan (19):
>  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_{old|new}world: Simplify cmdline_base calculation
>  mac_newworld: Clean up 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 type declaration out to a 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
>  mac_{old|new}world: Code style fix adding missing braces to if-s
>  mac_newworld: Turn CORE99_VIA_CONFIG defines into an enum
>  mac_newworld: Add machine types for different mac99 configs
>  mac_newworld: Deprecate mac99 with G5 CPU
>  mac_newworld: Deprecate mac99 "via" option
>  mac_newworld: Document deprecation
>
> MAINTAINERS                   |   2 +
> docs/about/deprecated.rst     |   7 +
> docs/system/ppc/powermac.rst  |  12 +-
> 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         |  15 +-
> hw/pci-host/uninorth.c        |   1 -
> hw/ppc/mac.h                  | 105 -----------
> hw/ppc/mac_newworld.c         | 341 ++++++++++++++++++++++------------
> hw/ppc/mac_oldworld.c         | 120 ++++++------
> include/hw/misc/macio/macio.h |  23 ++-
> include/hw/nvram/mac_nvram.h  |  51 +++++
> include/hw/pci-host/grackle.h |  44 +++++
> 19 files changed, 415 insertions(+), 322 deletions(-)
> delete mode 100644 hw/ppc/mac.h
> create mode 100644 include/hw/nvram/mac_nvram.h
> create mode 100644 include/hw/pci-host/grackle.h
>
>


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

* Re: [PATCH v4 00/19] Misc ppc/mac machines clean up
  2022-10-25 16:44 [PATCH v4 00/19] Misc ppc/mac machines clean up BALATON Zoltan
                   ` (21 preceding siblings ...)
  2022-10-25 21:35 ` [PATCH v4 00/19] Misc ppc/mac machines clean up BALATON Zoltan
@ 2022-10-27  5:41 ` Howard Spoelstra
  2022-10-27 12:22   ` BALATON Zoltan
  2022-10-28  9:07 ` Mark Cave-Ayland
  23 siblings, 1 reply; 38+ messages in thread
From: Howard Spoelstra @ 2022-10-27  5:41 UTC (permalink / raw)
  To: BALATON Zoltan; +Cc: qemu-devel, qemu-ppc, Mark Cave-Ayland

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

On Tue, Oct 25, 2022 at 6:49 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:

> Since only one week is left until freeze starts I've included some
> more patches in this version that I've intended to submit after the
> clean ups but we're running out of time now. The last 3 patches could
> be squashed together, I've just split these up because I expect
> resistence from Mark to any changes so maybe it's easier to digest
> piece by piece and can cherry pick parts easier this way but ideally
> these should be in one patch.
>
> I'd appreciate very much if this series would get in before the
> freeze, it is very discouraging to spend time with something that gets
> ignored and then postponed for the rest of the year just to start
> again the same in January. This might be a reason why not many people
> contribute to this part of QEMU besides that maybe only a few people
> are still interested so those who are interested should be served
> better to not scare them off even more.
>
> Regards,
> BALATON Zoltan
>
> v4: Add some more patches that I've found since v3 or was intended in
> separate series
> v3: Some more patch spliting and changes I've noticed and address more
> review comments
> v2: Split some patches and add a few more I've noticed now and address
> review comments
>
> BALATON Zoltan (19):
>   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_{old|new}world: Simplify cmdline_base calculation
>   mac_newworld: Clean up 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 type declaration out to a 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
>   mac_{old|new}world: Code style fix adding missing braces to if-s
>   mac_newworld: Turn CORE99_VIA_CONFIG defines into an enum
>   mac_newworld: Add machine types for different mac99 configs
>   mac_newworld: Deprecate mac99 with G5 CPU
>   mac_newworld: Deprecate mac99 "via" option
>   mac_newworld: Document deprecation
>
>  MAINTAINERS                   |   2 +
>  docs/about/deprecated.rst     |   7 +
>  docs/system/ppc/powermac.rst  |  12 +-
>  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         |  15 +-
>  hw/pci-host/uninorth.c        |   1 -
>  hw/ppc/mac.h                  | 105 -----------
>  hw/ppc/mac_newworld.c         | 341 ++++++++++++++++++++++------------
>  hw/ppc/mac_oldworld.c         | 120 ++++++------
>  include/hw/misc/macio/macio.h |  23 ++-
>  include/hw/nvram/mac_nvram.h  |  51 +++++
>  include/hw/pci-host/grackle.h |  44 +++++
>  19 files changed, 415 insertions(+), 322 deletions(-)
>  delete mode 100644 hw/ppc/mac.h
>  create mode 100644 include/hw/nvram/mac_nvram.h
>  create mode 100644 include/hw/pci-host/grackle.h
>
> --
> 2.30.4
>
>
>
Hi all,

I applied these patches and they seem to work as expected. I like the way
this makes it clearer which machine is actually emulated, even though it is
still not easy to understand which default hardware the emulated machine
actually presents.
I also like the more consistent way a new rom file for a VGA device can be
added. The deprecation warnings are clear.

Qemu-system-ppc defaults to the g3beige machine, which does not reflect the
(in my opinion) main use case of running Mac OS/X with the powermac3_1
machine and will not boot the main versions of ppc Mac OS/X anyway.

So for qemu-system-ppc:

Tested-by: Howard Spoelstra <hsp.cat7@gmail.com>

Best,
Howard

[-- Attachment #2: Type: text/html, Size: 5211 bytes --]

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

* Re: [PATCH v4 00/19] Misc ppc/mac machines clean up
  2022-10-27  5:41 ` Howard Spoelstra
@ 2022-10-27 12:22   ` BALATON Zoltan
  0 siblings, 0 replies; 38+ messages in thread
From: BALATON Zoltan @ 2022-10-27 12:22 UTC (permalink / raw)
  To: Howard Spoelstra; +Cc: qemu-devel, qemu-ppc, Mark Cave-Ayland

On Thu, 27 Oct 2022, Howard Spoelstra wrote:
> I applied these patches and they seem to work as expected. I like the way
> this makes it clearer which machine is actually emulated, even though it is
> still not easy to understand which default hardware the emulated machine
> actually presents.

Thanks for the feed back and testing. The emulation is not perfect so 
there are some differences from the actual machines. These could be 
documented in qemu/docs/system/ppc/powermac.rst patches are welcome). Some 
of these are not yet implemented like sound or i2c (see: 
https://osdn.net/projects/qmiga/wiki/SubprojectMac99I2C and 
https://patchew.org/QEMU/cover.1593456926.git.balaton@eik.bme.hu/93758f65ef21d977fe835364bb1386fb4c03a6ce.1593456926.git.balaton@eik.bme.hu/ 
if anybody is interested to finish these) or some are missing due to 
OpenBIOS can't yet handle it like a PCI bridge on some PCI bus which was 
there in code commented out for a while but looks like it's gone now or I 
couldn't find it. But the presented hardware should be close enough to 
these machines for OSes and it also shows what machines we should aim for 
so it's not an undefined machine any more. The mac99 machine may not be an 
actual existing config, according to

http://macos9lives.com/smforum/index.php/topic,2408.msg28843.html?PHPSESSID=ce15448df7a74e13c82c59eedf624db7#msg28843

which says no Mac had Uninorth, Keylargo and CUDA, although this forum 
post may not list every machine, e.g. powermac1,2 (the first PCI Power Mac 
G4) according to <https://en.wikipedia.org/wiki/Power_Mac_G4> had CUDA but 
used Grackle (the same motherboard as the Blue&White G3 PowerMac 
powermac1,1 <https://en.wikipedia.org/wiki/Power_Macintosh_G3_(Blue_and_White)> )
but had no ADB ports so you could not have ADB keyboard and mouse attached 
to it like we have in mac99. The powermac1,2 is maybe more similar to 
g3beige but g3beige has old world ROM while the B&W G3 powermac1,1 is the 
first new world ROM machine but may have more differences I don't know 
about. (That also means maybe our naming mac_oldworld and mac_newworld is 
misleading but that's OK for now as it's only in the source code and not 
user visible.)

> I also like the more consistent way a new rom file for a VGA device can be
> added. The deprecation warnings are clear.

Some more info on this last ndrv via romfile patch: OpenBIOS has two ways 
to add an NDRV in the device tree for MacOS to a vga card:

1. It adds it in openbios/drivers/pci.c::vga_config_cb() if the ROM 
contains an NDRV

2. Then in vga-driver-fcode defined in vga.fs (that OpenBIOS 
unconditionally calls for vga devices it knows about) it also checks for a 
file called ndrv/qemu_vga.ndrv in fw_cfg and adds that to the device tree. 
The vga-ndrv? option controls this second way and defaults to true.

Problems with 2.

- The ndrv/qemu_vga.ndrv is added by the machine not the card so it will 
be used for other cards (liek ati-vga) that it shouldn't be used for and 
there's no good way to control or fix it other than the user having to set 
vga-ndrv? to false when adding -device ati-vga.

- It's too complex for no good reason so after my patch this could be 
dropped altogether simpifying the code both in QEMU and OpenBIOS.

My patch sets the default value for the romfile property of the VGA device 
to qemu_vga.ndrv instead so QEMU will put the ndrv in the ROM and OpenBIOS 
detects that and adds it to the property without going through fw_cfg (it 
still checks fw_cfg but since we don't add the ndrv there any more that 
part won't do anything so that can be dropped later from OpenBIOS together 
with the vga-ndrv? option. If you want to disable the ndrv with my patch 
you can use -device VGA,romfile="" instead which replaces the default with 
empty romfile so OpenBIOS won't find it neither in the ROM not in fw_cfg. 
Additionally you can pass a real FCode ROM or different NDRV the same way 
via romfile now without having to replace the file in QEMU install which 
might come handy for someone developing NDRVs or experimenting with ROMs 
or pass-thorugh. So I think this simple patch really helps users and makes 
the code overall simpler too.

> Qemu-system-ppc defaults to the g3beige machine, which does not reflect the
> (in my opinion) main use case of running Mac OS/X with the powermac3_1
> machine and will not boot the main versions of ppc Mac OS/X anyway.

We can't easily change the default wihtout breaking existing commands and 
it's also debatable what should be a new default so I think we're stuck 
with that now. In any case we need an at least 2 release long deprecation 
period so what we could do is to deprecare g3beige as the default to 
require users to always specify a machine option explicitly so we can do 
something with it in the future but I don't know how to add such warning, 
i.e. how to detect if g3beige was chosen via -M or by default. Maybe this 
warning should be issued by command line parsing not the g3beige board 
code? So I've only added warnings for the mac99 with via option and G5 CPU 
for now and left qemu-system-ppc -M mac99 and g3beige alone for now. If 
you think these also need some warnings added now then we should find out 
how and what should be done instead. I could not decide on those so opted 
for preserving backwards compatibility for these.

Regards,
BALATON Zoltan

> So for qemu-system-ppc:
>
> Tested-by: Howard Spoelstra <hsp.cat7@gmail.com>
>
> Best,
> Howard



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

* Re: [PATCH v4 00/19] Misc ppc/mac machines clean up
  2022-10-25 16:44 [PATCH v4 00/19] Misc ppc/mac machines clean up BALATON Zoltan
                   ` (22 preceding siblings ...)
  2022-10-27  5:41 ` Howard Spoelstra
@ 2022-10-28  9:07 ` Mark Cave-Ayland
  2022-10-28 12:24   ` BALATON Zoltan
  23 siblings, 1 reply; 38+ messages in thread
From: Mark Cave-Ayland @ 2022-10-28  9:07 UTC (permalink / raw)
  To: BALATON Zoltan, qemu-devel, qemu-ppc

On 25/10/2022 17:44, BALATON Zoltan wrote:

> Since only one week is left until freeze starts I've included some
> more patches in this version that I've intended to submit after the
> clean ups but we're running out of time now. The last 3 patches could
> be squashed together, I've just split these up because I expect
> resistence from Mark to any changes so maybe it's easier to digest
> piece by piece and can cherry pick parts easier this way but ideally
> these should be in one patch.
> 
> I'd appreciate very much if this series would get in before the
> freeze, it is very discouraging to spend time with something that gets
> ignored and then postponed for the rest of the year just to start
> again the same in January. This might be a reason why not many people
> contribute to this part of QEMU besides that maybe only a few people
> are still interested so those who are interested should be served
> better to not scare them off even more.
> 
> Regards,
> BALATON Zoltan
> 
> v4: Add some more patches that I've found since v3 or was intended in
> separate series
> v3: Some more patch spliting and changes I've noticed and address more
> review comments
> v2: Split some patches and add a few more I've noticed now and address
> review comments

Oh wait, there's already a v4 with even more changes in? This is really confusing as 
a reviewer...


ATB,

Mark.


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

* Re: [PATCH v4 16/19] mac_newworld: Add machine types for different mac99 configs
  2022-10-25 16:44 ` [PATCH v4 16/19] mac_newworld: Add machine types for different mac99 configs BALATON Zoltan
@ 2022-10-28  9:37   ` Mark Cave-Ayland
  2022-10-28 12:18     ` BALATON Zoltan
  0 siblings, 1 reply; 38+ messages in thread
From: Mark Cave-Ayland @ 2022-10-28  9:37 UTC (permalink / raw)
  To: BALATON Zoltan, qemu-devel, qemu-ppc

On 25/10/2022 17:44, BALATON Zoltan wrote:

> The mac99 machine emulates different machines depending on machine
> properties or even if it is run as qemu-system-ppc64 or
> qemu-system-ppc. This is very confusing for users and many hours were
> lost trying to explain it or finding out why commands users came up
> with are not working as expected. (E.g. Windows users might think
> qemu-system-ppc64 is just the 64 bit version of qemu-system-ppc and
> then fail to boot a 32 bit OS with -M mac99 trying to follow an
> example that had qemu-system-ppc.) To avoid such confusion, add
> explicit machine types for the different configs which will work the
> same with both qemu-system-ppc and qemu-system-ppc64 and also make the
> command line clearer for new users.

What was the outcome of the discussion re: having separate machines for 32-bit and 
64-bit PPC targets? My understanding is the issue here was deciding what to do, 
rather than actually making the code changes.

Also what was your motivation for choosing the machine names? I see you've used 
powerbook for via=pmu-adb, but I think quite a few people use pmu-adb for older OS X 
server hardware. At the very least some pointers to reference device trees and some 
rationale behind the decision is needed for review.

> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>   hw/ppc/mac_newworld.c | 94 +++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 94 insertions(+)
> 
> diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
> index bcd6566ead..7321ac925e 100644
> --- a/hw/ppc/mac_newworld.c
> +++ b/hw/ppc/mac_newworld.c
> @@ -649,9 +649,103 @@ static const TypeInfo core99_machine_info = {
>       },
>   };
>   
> +static void powermac3_1_machine_class_init(ObjectClass *oc, void *data)
> +{
> +    MachineClass *mc = MACHINE_CLASS(oc);
> +
> +    core99_machine_class_init(oc, data);
> +    mc->desc = "Apple Power Mac G4 AGP (Sawtooth)";
> +    mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("7400_v2.9");
> +}
> +
> +static void powermac3_1_instance_init(Object *obj)
> +{
> +    Core99MachineState *cms = CORE99_MACHINE(obj);
> +
> +    cms->via_config = CORE99_VIA_CONFIG_PMU;
> +    return;
> +}
> +
> +static const TypeInfo powermac3_1_machine_info = {
> +    .name          = MACHINE_TYPE_NAME("powermac3_1"),
> +    .parent        = TYPE_MACHINE,
> +    .class_init    = powermac3_1_machine_class_init,
> +    .instance_init = powermac3_1_instance_init,
> +    .instance_size = sizeof(Core99MachineState),
> +    .interfaces = (InterfaceInfo[]) {
> +        { TYPE_FW_PATH_PROVIDER },
> +        { }
> +    },
> +};
> +
> +static void powerbook3_2_machine_class_init(ObjectClass *oc, void *data)
> +{
> +    MachineClass *mc = MACHINE_CLASS(oc);
> +
> +    core99_machine_class_init(oc, data);
> +    mc->desc = "Apple PowerBook G4 Titanium (Mercury)";
> +    mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("7400_v2.9");
> +}
> +
> +static void powerbook3_2_instance_init(Object *obj)
> +{
> +    Core99MachineState *cms = CORE99_MACHINE(obj);
> +
> +    cms->via_config = CORE99_VIA_CONFIG_PMU_ADB;
> +    return;
> +}
> +
> +static const TypeInfo powerbook3_2_machine_info = {
> +    .name          = MACHINE_TYPE_NAME("powerbook3_2"),
> +    .parent        = TYPE_MACHINE,
> +    .class_init    = powerbook3_2_machine_class_init,
> +    .instance_init = powerbook3_2_instance_init,
> +    .instance_size = sizeof(Core99MachineState),
> +    .interfaces = (InterfaceInfo[]) {
> +        { TYPE_FW_PATH_PROVIDER },
> +        { }
> +    },
> +};
> +
> +#ifdef TARGET_PPC64
> +static void powermac7_3_machine_class_init(ObjectClass *oc, void *data)
> +{
> +    MachineClass *mc = MACHINE_CLASS(oc);
> +
> +    core99_machine_class_init(oc, data);
> +    mc->desc = "Apple Power Mac G5 (Niagara)";
> +    mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("970fx_v3.1");
> +}
> +
> +static void powermac7_3_instance_init(Object *obj)
> +{
> +    Core99MachineState *cms = CORE99_MACHINE(obj);
> +
> +    cms->via_config = CORE99_VIA_CONFIG_PMU;
> +    return;
> +}
> +
> +static const TypeInfo powermac7_3_machine_info = {
> +    .name          = MACHINE_TYPE_NAME("powermac7_3"),
> +    .parent        = TYPE_MACHINE,
> +    .class_init    = powermac7_3_machine_class_init,
> +    .instance_init = powermac7_3_instance_init,
> +    .instance_size = sizeof(Core99MachineState),
> +    .interfaces = (InterfaceInfo[]) {
> +        { TYPE_FW_PATH_PROVIDER },
> +        { }
> +    },
> +};
> +#endif
> +
>   static void mac_machine_register_types(void)
>   {
>       type_register_static(&core99_machine_info);
> +    type_register_static(&powermac3_1_machine_info);
> +    type_register_static(&powerbook3_2_machine_info);
> +#ifdef TARGET_PPC64
> +    type_register_static(&powermac7_3_machine_info);
> +#endif
>   }
>   
>   type_init(mac_machine_register_types)


ATB,

Mark.


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

* Re: [PATCH v4 19/19] mac_newworld: Document deprecation
  2022-10-25 16:44 ` [PATCH v4 19/19] mac_newworld: Document deprecation BALATON Zoltan
@ 2022-10-28  9:41   ` Mark Cave-Ayland
  2022-10-28 12:20     ` BALATON Zoltan
  0 siblings, 1 reply; 38+ messages in thread
From: Mark Cave-Ayland @ 2022-10-28  9:41 UTC (permalink / raw)
  To: BALATON Zoltan, qemu-devel, qemu-ppc

On 25/10/2022 17:44, BALATON Zoltan wrote:

> Also update PowerMac family docs with some more recent info.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>   docs/about/deprecated.rst    |  7 +++++++
>   docs/system/ppc/powermac.rst | 12 ++++++++----
>   2 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index 93affe3669..07661af7fe 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -248,6 +248,13 @@ These old machine types are quite neglected nowadays and thus might have
>   various pitfalls with regards to live migration. Use a newer machine type
>   instead.
>   
> +``mac99`` variants other than the default qemu-system-ppc version (since 7.2)
> +'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
> +
> +The ``mac99`` machine emulates different hardware depending on using
> +qemu-system-ppc64 or ``via`` property. To avoid confusion new machine
> +types has been added for these variants which are now preferred over
> +``mac99``.
>   
>   Backend options
>   ---------------
> diff --git a/docs/system/ppc/powermac.rst b/docs/system/ppc/powermac.rst
> index 04334ba210..9a37e69b1b 100644
> --- a/docs/system/ppc/powermac.rst
> +++ b/docs/system/ppc/powermac.rst
> @@ -4,8 +4,12 @@ PowerMac family boards (``g3beige``, ``mac99``)
>   Use the executable ``qemu-system-ppc`` to simulate a complete PowerMac
>   PowerPC system.
>   
> -- ``g3beige``              Heathrow based PowerMAC
> -- ``mac99``                Mac99 based PowerMAC
> +- ``g3beige``           Heathrow based old world Power Macintosh G3
> +- ``mac99``             Core99 based generic PowerMac
> +- ``powermac3_1``       Power Mac G4 AGP (Sawtooth)
> +- ``powerbook3_2``      PowerBook G4 Titanium (Mercury)
> +- ``powermac7_3``       Power Mac G5 (Niagara) (only in ``qemu-system-ppc64``)
> +
>   
>   Supported devices
>   -----------------
> @@ -15,9 +19,9 @@ QEMU emulates the following PowerMac peripherals:
>    *  UniNorth or Grackle PCI Bridge
>    *  PCI VGA compatible card with VESA Bochs Extensions
>    *  2 PMAC IDE interfaces with hard disk and CD-ROM support
> - *  NE2000 PCI adapters
> + *  Sungem PCI network adapter
>    *  Non Volatile RAM
> - *  VIA-CUDA with ADB keyboard and mouse.
> + *  VIA-CUDA or VIA-PMU99 with ot without ADB or USB keyboard and mouse.
>   
>   
>   Missing devices

Documentation updates are always useful, but until there is consensus as to how the 
32-bit and 64-bit targets should be handled then I don't think we should go ahead 
with a potential compatibility break/deprecation until we have a clear path forward.

Given that freeze is so close, I suggest leaving this for 7.2 and resurrecting the 
appropriate thread from earlier in the year at the start of the 8.0 development cycle.


ATB,

Mark.


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

* Re: [PATCH v5 20/20] mac_{old, new}world: Pass MacOS VGA NDRV in card ROM instead of fw_cfg
  2022-10-25 21:31 ` [PATCH v5 20/20] mac_{old, new}world: Pass MacOS VGA NDRV in card ROM instead of fw_cfg BALATON Zoltan
@ 2022-10-28  9:51   ` Mark Cave-Ayland
  2022-10-28 12:32     ` BALATON Zoltan
  0 siblings, 1 reply; 38+ messages in thread
From: Mark Cave-Ayland @ 2022-10-28  9:51 UTC (permalink / raw)
  To: BALATON Zoltan, qemu-devel, qemu-ppc

On 25/10/2022 22:31, BALATON Zoltan wrote:

> OpenBIOS cannot run FCode ROMs yet but it can detect NDRV in VGA card
> ROM and add it to the device tree for MacOS. Pass the NDRV this way
> instead of via fw_cfg. This solves the problem with OpenBIOS also
> adding the NDRV to ati-vga which it does not work with. This does not
> need any changes to OpenBIOS as this NDRV ROM handling is already
> there but this patch also allows simplifying OpenBIOS later to remove
> the fw_cfg ndrv handling from the vga FCode and also drop the
> vga-ndrv? option which is not needed any more as users can disable the
> ndrv with -device VGA,romfile="" (or override it with their own NDRV
> or ROM). Once FCode support is implemented in OpenBIOS, the proper
> FCode ROM can be set the same way so this paves the way to remove some
> hacks.

This is not correct though: in a real option ROM the NDRV is included as part of the 
ROM payload and is not a standalone file. The IEEE-1275 PCI specification gives the 
correct format for an option ROM which at minimum contains a header, and likely some 
additional FCode.

Isn't the immediate problem here that the NDRV handling in OpenBIOS needs to be 
improved so that it can be disabled for particular VGA devices such as ATI?

> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>   hw/ppc/mac_newworld.c | 18 ++++++------------
>   hw/ppc/mac_oldworld.c | 18 ++++++------------
>   2 files changed, 12 insertions(+), 24 deletions(-)
> 
> diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
> index de4a7bae12..1d12bd85ed 100644
> --- a/hw/ppc/mac_newworld.c
> +++ b/hw/ppc/mac_newworld.c
> @@ -526,18 +526,6 @@ static void ppc_core99_init(MachineState *machine)
>       fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_BUSFREQ, BUSFREQ);
>       fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_NVRAM_ADDR, nvram_addr);
>   
> -    /* MacOS NDRV VGA driver */
> -    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, NDRV_VGA_FILENAME);
> -    if (filename) {
> -        gchar *ndrv_file;
> -        gsize ndrv_size;
> -
> -        if (g_file_get_contents(filename, &ndrv_file, &ndrv_size, NULL)) {
> -            fw_cfg_add_file(fw_cfg, "ndrv/qemu_vga.ndrv", ndrv_file, ndrv_size);
> -        }
> -        g_free(filename);
> -    }
> -
>       qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
>   }
>   
> @@ -581,6 +569,11 @@ static int core99_kvm_type(MachineState *machine, const char *arg)
>       return 2;
>   }
>   
> +static GlobalProperty props[] = {
> +    /* MacOS NDRV VGA driver */
> +    { "VGA", "romfile", NDRV_VGA_FILENAME },
> +};
> +
>   static void core99_machine_class_init(ObjectClass *oc, void *data)
>   {
>       MachineClass *mc = MACHINE_CLASS(oc);
> @@ -601,6 +594,7 @@ static void core99_machine_class_init(ObjectClass *oc, void *data)
>   #endif
>       mc->default_ram_id = "ppc_core99.ram";
>       mc->ignore_boot_device_suffixes = true;
> +    compat_props_add(mc->compat_props, props, G_N_ELEMENTS(props));
>       fwc->get_dev_path = core99_fw_dev_path;
>   }
>   
> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
> index eecc54da59..e7d35135d6 100644
> --- a/hw/ppc/mac_oldworld.c
> +++ b/hw/ppc/mac_oldworld.c
> @@ -344,18 +344,6 @@ static void ppc_heathrow_init(MachineState *machine)
>       fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_CLOCKFREQ, CLOCKFREQ);
>       fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_BUSFREQ, BUSFREQ);
>   
> -    /* MacOS NDRV VGA driver */
> -    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, NDRV_VGA_FILENAME);
> -    if (filename) {
> -        gchar *ndrv_file;
> -        gsize ndrv_size;
> -
> -        if (g_file_get_contents(filename, &ndrv_file, &ndrv_size, NULL)) {
> -            fw_cfg_add_file(fw_cfg, "ndrv/qemu_vga.ndrv", ndrv_file, ndrv_size);
> -        }
> -        g_free(filename);
> -    }
> -
>       qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
>   }
>   
> @@ -400,6 +388,11 @@ static int heathrow_kvm_type(MachineState *machine, const char *arg)
>       return 2;
>   }
>   
> +static GlobalProperty props[] = {
> +    /* MacOS NDRV VGA driver */
> +    { "VGA", "romfile", NDRV_VGA_FILENAME },
> +};
> +
>   static void heathrow_class_init(ObjectClass *oc, void *data)
>   {
>       MachineClass *mc = MACHINE_CLASS(oc);
> @@ -420,6 +413,7 @@ static void heathrow_class_init(ObjectClass *oc, void *data)
>       mc->default_display = "std";
>       mc->ignore_boot_device_suffixes = true;
>       mc->default_ram_id = "ppc_heathrow.ram";
> +    compat_props_add(mc->compat_props, props, G_N_ELEMENTS(props));
>       fwc->get_dev_path = heathrow_fw_dev_path;
>   }
>   


ATB,

Mark.


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

* Re: [PATCH v4 16/19] mac_newworld: Add machine types for different mac99 configs
  2022-10-28  9:37   ` Mark Cave-Ayland
@ 2022-10-28 12:18     ` BALATON Zoltan
  2022-10-29  7:48       ` Mark Cave-Ayland
  0 siblings, 1 reply; 38+ messages in thread
From: BALATON Zoltan @ 2022-10-28 12:18 UTC (permalink / raw)
  To: Mark Cave-Ayland; +Cc: qemu-devel, qemu-ppc

On Fri, 28 Oct 2022, Mark Cave-Ayland wrote:
> On 25/10/2022 17:44, BALATON Zoltan wrote:
>> The mac99 machine emulates different machines depending on machine
>> properties or even if it is run as qemu-system-ppc64 or
>> qemu-system-ppc. This is very confusing for users and many hours were
>> lost trying to explain it or finding out why commands users came up
>> with are not working as expected. (E.g. Windows users might think
>> qemu-system-ppc64 is just the 64 bit version of qemu-system-ppc and
>> then fail to boot a 32 bit OS with -M mac99 trying to follow an
>> example that had qemu-system-ppc.) To avoid such confusion, add
>> explicit machine types for the different configs which will work the
>> same with both qemu-system-ppc and qemu-system-ppc64 and also make the
>> command line clearer for new users.
>
> What was the outcome of the discussion re: having separate machines for 
> 32-bit and 64-bit PPC targets? My understanding is the issue here was 
> deciding what to do, rather than actually making the code changes.

Who do you think will or should decide about this? There are about 3 
people who care about Mac emulation on this list: you, Howard and me. You 
already have my and Howard's vote to introduce these machines types. Who 
else should vote or decide on this? Please apply this patch now and if it 
causes problem it can still be dropped duting the freeze but if you don't 
apply it now it can't get into before next spring.

> Also what was your motivation for choosing the machine names? I see you've 
> used powerbook for via=pmu-adb, but I think quite a few people use pmu-adb 
> for older OS X server hardware. At the very least some pointers to reference 
> device trees and some rationale behind the decision is needed for review.

See my reply to Howard's message with some more info and links. My 
immediate motivation was that we've lost about two days when somobody 
contacted me about VGA pass through sending logs about all kinds of 
failures he got. After many logs I've noticed that he was using 
qemu-system-ppc64 -M mac99,via=pmu thinking that on 64bit host that's the 
executable he should use. Unfortunately the commands were not shared just 
the logs so this took a while to notice. Also if you look at the forum 
Howard runs you can see this problem is coming up frequently and I think 
the've also lost countless hours due to this. It's about time to put an 
end on it and stop wasting othet's time. As for The machines, the powermac 
ones are straight forward as those are closest to what we emulate for G4 
and G5 Mac. I've chosen the powerbook becuase that's the only machine I 
know that had PMU and ADB but If someone knows a better machine we can 
change this (even as bug fix during the freeze). Here's some info on this 
powerbook: https://ppc.0penbsd.narkive.com/s49Kcx1u/x-on-powerbook-g4

Regards,
BALATON Zoltan

>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>>   hw/ppc/mac_newworld.c | 94 +++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 94 insertions(+)
>> 
>> diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
>> index bcd6566ead..7321ac925e 100644
>> --- a/hw/ppc/mac_newworld.c
>> +++ b/hw/ppc/mac_newworld.c
>> @@ -649,9 +649,103 @@ static const TypeInfo core99_machine_info = {
>>       },
>>   };
>>   +static void powermac3_1_machine_class_init(ObjectClass *oc, void *data)
>> +{
>> +    MachineClass *mc = MACHINE_CLASS(oc);
>> +
>> +    core99_machine_class_init(oc, data);
>> +    mc->desc = "Apple Power Mac G4 AGP (Sawtooth)";
>> +    mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("7400_v2.9");
>> +}
>> +
>> +static void powermac3_1_instance_init(Object *obj)
>> +{
>> +    Core99MachineState *cms = CORE99_MACHINE(obj);
>> +
>> +    cms->via_config = CORE99_VIA_CONFIG_PMU;
>> +    return;
>> +}
>> +
>> +static const TypeInfo powermac3_1_machine_info = {
>> +    .name          = MACHINE_TYPE_NAME("powermac3_1"),
>> +    .parent        = TYPE_MACHINE,
>> +    .class_init    = powermac3_1_machine_class_init,
>> +    .instance_init = powermac3_1_instance_init,
>> +    .instance_size = sizeof(Core99MachineState),
>> +    .interfaces = (InterfaceInfo[]) {
>> +        { TYPE_FW_PATH_PROVIDER },
>> +        { }
>> +    },
>> +};
>> +
>> +static void powerbook3_2_machine_class_init(ObjectClass *oc, void *data)
>> +{
>> +    MachineClass *mc = MACHINE_CLASS(oc);
>> +
>> +    core99_machine_class_init(oc, data);
>> +    mc->desc = "Apple PowerBook G4 Titanium (Mercury)";
>> +    mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("7400_v2.9");
>> +}
>> +
>> +static void powerbook3_2_instance_init(Object *obj)
>> +{
>> +    Core99MachineState *cms = CORE99_MACHINE(obj);
>> +
>> +    cms->via_config = CORE99_VIA_CONFIG_PMU_ADB;
>> +    return;
>> +}
>> +
>> +static const TypeInfo powerbook3_2_machine_info = {
>> +    .name          = MACHINE_TYPE_NAME("powerbook3_2"),
>> +    .parent        = TYPE_MACHINE,
>> +    .class_init    = powerbook3_2_machine_class_init,
>> +    .instance_init = powerbook3_2_instance_init,
>> +    .instance_size = sizeof(Core99MachineState),
>> +    .interfaces = (InterfaceInfo[]) {
>> +        { TYPE_FW_PATH_PROVIDER },
>> +        { }
>> +    },
>> +};
>> +
>> +#ifdef TARGET_PPC64
>> +static void powermac7_3_machine_class_init(ObjectClass *oc, void *data)
>> +{
>> +    MachineClass *mc = MACHINE_CLASS(oc);
>> +
>> +    core99_machine_class_init(oc, data);
>> +    mc->desc = "Apple Power Mac G5 (Niagara)";
>> +    mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("970fx_v3.1");
>> +}
>> +
>> +static void powermac7_3_instance_init(Object *obj)
>> +{
>> +    Core99MachineState *cms = CORE99_MACHINE(obj);
>> +
>> +    cms->via_config = CORE99_VIA_CONFIG_PMU;
>> +    return;
>> +}
>> +
>> +static const TypeInfo powermac7_3_machine_info = {
>> +    .name          = MACHINE_TYPE_NAME("powermac7_3"),
>> +    .parent        = TYPE_MACHINE,
>> +    .class_init    = powermac7_3_machine_class_init,
>> +    .instance_init = powermac7_3_instance_init,
>> +    .instance_size = sizeof(Core99MachineState),
>> +    .interfaces = (InterfaceInfo[]) {
>> +        { TYPE_FW_PATH_PROVIDER },
>> +        { }
>> +    },
>> +};
>> +#endif
>> +
>>   static void mac_machine_register_types(void)
>>   {
>>       type_register_static(&core99_machine_info);
>> +    type_register_static(&powermac3_1_machine_info);
>> +    type_register_static(&powerbook3_2_machine_info);
>> +#ifdef TARGET_PPC64
>> +    type_register_static(&powermac7_3_machine_info);
>> +#endif
>>   }
>>     type_init(mac_machine_register_types)
>
>
> ATB,
>
> Mark.
>
>


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

* Re: [PATCH v4 19/19] mac_newworld: Document deprecation
  2022-10-28  9:41   ` Mark Cave-Ayland
@ 2022-10-28 12:20     ` BALATON Zoltan
  2022-10-29  8:09       ` Mark Cave-Ayland
  0 siblings, 1 reply; 38+ messages in thread
From: BALATON Zoltan @ 2022-10-28 12:20 UTC (permalink / raw)
  To: Mark Cave-Ayland; +Cc: qemu-devel, qemu-ppc

On Fri, 28 Oct 2022, Mark Cave-Ayland wrote:
> On 25/10/2022 17:44, BALATON Zoltan wrote:
>> Also update PowerMac family docs with some more recent info.
>> 
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>>   docs/about/deprecated.rst    |  7 +++++++
>>   docs/system/ppc/powermac.rst | 12 ++++++++----
>>   2 files changed, 15 insertions(+), 4 deletions(-)
>> 
>> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
>> index 93affe3669..07661af7fe 100644
>> --- a/docs/about/deprecated.rst
>> +++ b/docs/about/deprecated.rst
>> @@ -248,6 +248,13 @@ These old machine types are quite neglected nowadays 
>> and thus might have
>>   various pitfalls with regards to live migration. Use a newer machine type
>>   instead.
>>   +``mac99`` variants other than the default qemu-system-ppc version (since 
>> 7.2)
>> +'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
>> +
>> +The ``mac99`` machine emulates different hardware depending on using
>> +qemu-system-ppc64 or ``via`` property. To avoid confusion new machine
>> +types has been added for these variants which are now preferred over
>> +``mac99``.
>>     Backend options
>>   ---------------
>> diff --git a/docs/system/ppc/powermac.rst b/docs/system/ppc/powermac.rst
>> index 04334ba210..9a37e69b1b 100644
>> --- a/docs/system/ppc/powermac.rst
>> +++ b/docs/system/ppc/powermac.rst
>> @@ -4,8 +4,12 @@ PowerMac family boards (``g3beige``, ``mac99``)
>>   Use the executable ``qemu-system-ppc`` to simulate a complete PowerMac
>>   PowerPC system.
>>   -- ``g3beige``              Heathrow based PowerMAC
>> -- ``mac99``                Mac99 based PowerMAC
>> +- ``g3beige``           Heathrow based old world Power Macintosh G3
>> +- ``mac99``             Core99 based generic PowerMac
>> +- ``powermac3_1``       Power Mac G4 AGP (Sawtooth)
>> +- ``powerbook3_2``      PowerBook G4 Titanium (Mercury)
>> +- ``powermac7_3``       Power Mac G5 (Niagara) (only in 
>> ``qemu-system-ppc64``)
>> +
>>     Supported devices
>>   -----------------
>> @@ -15,9 +19,9 @@ QEMU emulates the following PowerMac peripherals:
>>    *  UniNorth or Grackle PCI Bridge
>>    *  PCI VGA compatible card with VESA Bochs Extensions
>>    *  2 PMAC IDE interfaces with hard disk and CD-ROM support
>> - *  NE2000 PCI adapters
>> + *  Sungem PCI network adapter
>>    *  Non Volatile RAM
>> - *  VIA-CUDA with ADB keyboard and mouse.
>> + *  VIA-CUDA or VIA-PMU99 with ot without ADB or USB keyboard and mouse.
>>       Missing devices
>
> Documentation updates are always useful, but until there is consensus as to 
> how the 32-bit and 64-bit targets should be handled then I don't think we 
> should go ahead with a potential compatibility break/deprecation until we 
> have a clear path forward.
>
> Given that freeze is so close, I suggest leaving this for 7.2 and 
> resurrecting the appropriate thread from earlier in the year at the start of 
> the 8.0 development cycle.

Please don't postpone patches because you were not able to review in time. 
A better approach would be to merge these now and drop them during the 
freeze if any problem that can't be fixed is found. The deprecation 
process itself is also to allow backing off if this turns out to be a bad 
idea so no need to wait for more votes now and postpone this further.

Regards,
BALATON Zoltan


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

* Re: [PATCH v4 00/19] Misc ppc/mac machines clean up
  2022-10-28  9:07 ` Mark Cave-Ayland
@ 2022-10-28 12:24   ` BALATON Zoltan
  0 siblings, 0 replies; 38+ messages in thread
From: BALATON Zoltan @ 2022-10-28 12:24 UTC (permalink / raw)
  To: Mark Cave-Ayland; +Cc: qemu-devel, qemu-ppc

On Fri, 28 Oct 2022, Mark Cave-Ayland wrote:
> On 25/10/2022 17:44, BALATON Zoltan wrote:
>> Since only one week is left until freeze starts I've included some
>> more patches in this version that I've intended to submit after the
>> clean ups but we're running out of time now. The last 3 patches could
>> be squashed together, I've just split these up because I expect
>> resistence from Mark to any changes so maybe it's easier to digest
>> piece by piece and can cherry pick parts easier this way but ideally
>> these should be in one patch.
>> 
>> I'd appreciate very much if this series would get in before the
>> freeze, it is very discouraging to spend time with something that gets
>> ignored and then postponed for the rest of the year just to start
>> again the same in January. This might be a reason why not many people
>> contribute to this part of QEMU besides that maybe only a few people
>> are still interested so those who are interested should be served
>> better to not scare them off even more.
>> 
>> Regards,
>> BALATON Zoltan
>> 
>> v4: Add some more patches that I've found since v3 or was intended in
>> separate series
>> v3: Some more patch spliting and changes I've noticed and address more
>> review comments
>> v2: Split some patches and add a few more I've noticed now and address
>> review comments
>
> Oh wait, there's already a v4 with even more changes in? This is really 
> confusing as a reviewer...

I've intended to submit these last patches as a separate series after the 
simple clean ups but that series were under review for so long that I had 
to include these now to have a chance to meet the freeze deadline. Ideally 
the simple clean ups should not have taken more than 2 weeks then we had 
another 2 weeks for these.

Regards,
BALATON Zoltan


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

* Re: [PATCH v5 20/20] mac_{old, new}world: Pass MacOS VGA NDRV in card ROM instead of fw_cfg
  2022-10-28  9:51   ` Mark Cave-Ayland
@ 2022-10-28 12:32     ` BALATON Zoltan
  2022-10-29  8:22       ` Mark Cave-Ayland
  0 siblings, 1 reply; 38+ messages in thread
From: BALATON Zoltan @ 2022-10-28 12:32 UTC (permalink / raw)
  To: Mark Cave-Ayland; +Cc: qemu-devel, qemu-ppc

On Fri, 28 Oct 2022, Mark Cave-Ayland wrote:
> On 25/10/2022 22:31, BALATON Zoltan wrote:
>> OpenBIOS cannot run FCode ROMs yet but it can detect NDRV in VGA card
>> ROM and add it to the device tree for MacOS. Pass the NDRV this way
>> instead of via fw_cfg. This solves the problem with OpenBIOS also
>> adding the NDRV to ati-vga which it does not work with. This does not
>> need any changes to OpenBIOS as this NDRV ROM handling is already
>> there but this patch also allows simplifying OpenBIOS later to remove
>> the fw_cfg ndrv handling from the vga FCode and also drop the
>> vga-ndrv? option which is not needed any more as users can disable the
>> ndrv with -device VGA,romfile="" (or override it with their own NDRV
>> or ROM). Once FCode support is implemented in OpenBIOS, the proper
>> FCode ROM can be set the same way so this paves the way to remove some
>> hacks.
>
> This is not correct though: in a real option ROM the NDRV is included as part 
> of the ROM payload and is not a standalone file. The IEEE-1275 PCI 
> specification gives the correct format for an option ROM which at minimum 
> contains a header, and likely some additional FCode.

As the commit message says that does not work with OpenBIOS at the moment 
but passing the NDRV does. That it's not how real hardware works is not an 
argument after all real hardware does not have fw_cfg either and this way 
is much simpler than fw_cfg, it fixes the problem with ati-vga and it can 
be changed later to pass the real FCode ROM the same way so I think it's a 
better way to handle this now as what we have currently.

> Isn't the immediate problem here that the NDRV handling in OpenBIOS needs to 
> be improved so that it can be disabled for particular VGA devices such as 
> ATI?

No change is needed to OpenBIOS (I've discussed it more in the reply to 
Howard on the list yesterday). With this patch only VGA device will have 
qemu_vga.ndrv so OpenBIOS won't add it for ati-vga. Also the fw_cfg and 
vga_ndrv? stuff can be removed from OpenBIOS after this patch as it's not 
nedeed any more thus simplifying the vga.fs FCode in OpenBIOS a lot.

Regards,
BALATON Zoltan

>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>>   hw/ppc/mac_newworld.c | 18 ++++++------------
>>   hw/ppc/mac_oldworld.c | 18 ++++++------------
>>   2 files changed, 12 insertions(+), 24 deletions(-)
>> 
>> diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
>> index de4a7bae12..1d12bd85ed 100644
>> --- a/hw/ppc/mac_newworld.c
>> +++ b/hw/ppc/mac_newworld.c
>> @@ -526,18 +526,6 @@ static void ppc_core99_init(MachineState *machine)
>>       fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_BUSFREQ, BUSFREQ);
>>       fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_NVRAM_ADDR, nvram_addr);
>>   -    /* MacOS NDRV VGA driver */
>> -    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, NDRV_VGA_FILENAME);
>> -    if (filename) {
>> -        gchar *ndrv_file;
>> -        gsize ndrv_size;
>> -
>> -        if (g_file_get_contents(filename, &ndrv_file, &ndrv_size, NULL)) {
>> -            fw_cfg_add_file(fw_cfg, "ndrv/qemu_vga.ndrv", ndrv_file, 
>> ndrv_size);
>> -        }
>> -        g_free(filename);
>> -    }
>> -
>>       qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
>>   }
>>   @@ -581,6 +569,11 @@ static int core99_kvm_type(MachineState *machine, 
>> const char *arg)
>>       return 2;
>>   }
>>   +static GlobalProperty props[] = {
>> +    /* MacOS NDRV VGA driver */
>> +    { "VGA", "romfile", NDRV_VGA_FILENAME },
>> +};
>> +
>>   static void core99_machine_class_init(ObjectClass *oc, void *data)
>>   {
>>       MachineClass *mc = MACHINE_CLASS(oc);
>> @@ -601,6 +594,7 @@ static void core99_machine_class_init(ObjectClass *oc, 
>> void *data)
>>   #endif
>>       mc->default_ram_id = "ppc_core99.ram";
>>       mc->ignore_boot_device_suffixes = true;
>> +    compat_props_add(mc->compat_props, props, G_N_ELEMENTS(props));
>>       fwc->get_dev_path = core99_fw_dev_path;
>>   }
>>   diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
>> index eecc54da59..e7d35135d6 100644
>> --- a/hw/ppc/mac_oldworld.c
>> +++ b/hw/ppc/mac_oldworld.c
>> @@ -344,18 +344,6 @@ static void ppc_heathrow_init(MachineState *machine)
>>       fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_CLOCKFREQ, CLOCKFREQ);
>>       fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_BUSFREQ, BUSFREQ);
>>   -    /* MacOS NDRV VGA driver */
>> -    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, NDRV_VGA_FILENAME);
>> -    if (filename) {
>> -        gchar *ndrv_file;
>> -        gsize ndrv_size;
>> -
>> -        if (g_file_get_contents(filename, &ndrv_file, &ndrv_size, NULL)) {
>> -            fw_cfg_add_file(fw_cfg, "ndrv/qemu_vga.ndrv", ndrv_file, 
>> ndrv_size);
>> -        }
>> -        g_free(filename);
>> -    }
>> -
>>       qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
>>   }
>>   @@ -400,6 +388,11 @@ static int heathrow_kvm_type(MachineState *machine, 
>> const char *arg)
>>       return 2;
>>   }
>>   +static GlobalProperty props[] = {
>> +    /* MacOS NDRV VGA driver */
>> +    { "VGA", "romfile", NDRV_VGA_FILENAME },
>> +};
>> +
>>   static void heathrow_class_init(ObjectClass *oc, void *data)
>>   {
>>       MachineClass *mc = MACHINE_CLASS(oc);
>> @@ -420,6 +413,7 @@ static void heathrow_class_init(ObjectClass *oc, void 
>> *data)
>>       mc->default_display = "std";
>>       mc->ignore_boot_device_suffixes = true;
>>       mc->default_ram_id = "ppc_heathrow.ram";
>> +    compat_props_add(mc->compat_props, props, G_N_ELEMENTS(props));
>>       fwc->get_dev_path = heathrow_fw_dev_path;
>>   }
>> 
>
>
> ATB,
>
> Mark.
>
>


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

* Re: [PATCH v4 16/19] mac_newworld: Add machine types for different mac99 configs
  2022-10-28 12:18     ` BALATON Zoltan
@ 2022-10-29  7:48       ` Mark Cave-Ayland
  2022-10-29 12:35         ` BALATON Zoltan
  0 siblings, 1 reply; 38+ messages in thread
From: Mark Cave-Ayland @ 2022-10-29  7:48 UTC (permalink / raw)
  To: BALATON Zoltan; +Cc: qemu-devel, qemu-ppc

On 28/10/2022 13:18, BALATON Zoltan wrote:

> On Fri, 28 Oct 2022, Mark Cave-Ayland wrote:
>> On 25/10/2022 17:44, BALATON Zoltan wrote:
>>> The mac99 machine emulates different machines depending on machine
>>> properties or even if it is run as qemu-system-ppc64 or
>>> qemu-system-ppc. This is very confusing for users and many hours were
>>> lost trying to explain it or finding out why commands users came up
>>> with are not working as expected. (E.g. Windows users might think
>>> qemu-system-ppc64 is just the 64 bit version of qemu-system-ppc and
>>> then fail to boot a 32 bit OS with -M mac99 trying to follow an
>>> example that had qemu-system-ppc.) To avoid such confusion, add
>>> explicit machine types for the different configs which will work the
>>> same with both qemu-system-ppc and qemu-system-ppc64 and also make the
>>> command line clearer for new users.
>>
>> What was the outcome of the discussion re: having separate machines for 32-bit and 
>> 64-bit PPC targets? My understanding is the issue here was deciding what to do, 
>> rather than actually making the code changes.
> 
> Who do you think will or should decide about this? There are about 3 people who care 
> about Mac emulation on this list: you, Howard and me. You already have my and 
> Howard's vote to introduce these machines types. Who else should vote or decide on 
> this? Please apply this patch now and if it causes problem it can still be dropped 
> duting the freeze but if you don't apply it now it can't get into before next spring.

This is not restricted to qemu-system-ppc though: there was a discussion (which was 
still ongoing) as to how all of QEMU should handle 32-bit and 64-bit machines i.e. 
should qemu-system-ppc64 only contain 64-bit machines and qemu-system-ppc only 
contain 32-bit machines? If we wish to make a change here, we should wait for the 
outcome of this to ensure consistency here.

>> Also what was your motivation for choosing the machine names? I see you've used 
>> powerbook for via=pmu-adb, but I think quite a few people use pmu-adb for older OS 
>> X server hardware. At the very least some pointers to reference device trees and 
>> some rationale behind the decision is needed for review.
> 
> See my reply to Howard's message with some more info and links. My immediate 
> motivation was that we've lost about two days when somobody contacted me about VGA 
> pass through sending logs about all kinds of failures he got. After many logs I've 
> noticed that he was using qemu-system-ppc64 -M mac99,via=pmu thinking that on 64bit 
> host that's the executable he should use. Unfortunately the commands were not shared 
> just the logs so this took a while to notice. Also if you look at the forum Howard 
> runs you can see this problem is coming up frequently and I think the've also lost 
> countless hours due to this. It's about time to put an end on it and stop wasting 
> othet's time. As for The machines, the powermac ones are straight forward as those 
> are closest to what we emulate for G4 and G5 Mac. I've chosen the powerbook becuase 
> that's the only machine I know that had PMU and ADB but If someone knows a better 
> machine we can change this (even as bug fix during the freeze). Here's some info on 
> this powerbook: https://ppc.0penbsd.narkive.com/s49Kcx1u/x-on-powerbook-g4

In all my time working on QEMU this has happened to me once: yes, I agree it can be 
annoying but given how rare it happens I don't see a need to make a rushed decision now.

In terms of choosing the machines for QEMU we will need a full copy of the DT from 
real hardware for comparison with OpenBIOS, and ideally a Linux dmesg.


ATB,

Mark.


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

* Re: [PATCH v4 19/19] mac_newworld: Document deprecation
  2022-10-28 12:20     ` BALATON Zoltan
@ 2022-10-29  8:09       ` Mark Cave-Ayland
  0 siblings, 0 replies; 38+ messages in thread
From: Mark Cave-Ayland @ 2022-10-29  8:09 UTC (permalink / raw)
  To: BALATON Zoltan; +Cc: qemu-devel, qemu-ppc

On 28/10/2022 13:20, BALATON Zoltan wrote:

> On Fri, 28 Oct 2022, Mark Cave-Ayland wrote:
>> On 25/10/2022 17:44, BALATON Zoltan wrote:
>>> Also update PowerMac family docs with some more recent info.
>>>
>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>> ---
>>>   docs/about/deprecated.rst    |  7 +++++++
>>>   docs/system/ppc/powermac.rst | 12 ++++++++----
>>>   2 files changed, 15 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
>>> index 93affe3669..07661af7fe 100644
>>> --- a/docs/about/deprecated.rst
>>> +++ b/docs/about/deprecated.rst
>>> @@ -248,6 +248,13 @@ These old machine types are quite neglected nowadays and thus 
>>> might have
>>>   various pitfalls with regards to live migration. Use a newer machine type
>>>   instead.
>>>   +``mac99`` variants other than the default qemu-system-ppc version (since 7.2)
>>> +'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
>>> +
>>> +The ``mac99`` machine emulates different hardware depending on using
>>> +qemu-system-ppc64 or ``via`` property. To avoid confusion new machine
>>> +types has been added for these variants which are now preferred over
>>> +``mac99``.
>>>     Backend options
>>>   ---------------
>>> diff --git a/docs/system/ppc/powermac.rst b/docs/system/ppc/powermac.rst
>>> index 04334ba210..9a37e69b1b 100644
>>> --- a/docs/system/ppc/powermac.rst
>>> +++ b/docs/system/ppc/powermac.rst
>>> @@ -4,8 +4,12 @@ PowerMac family boards (``g3beige``, ``mac99``)
>>>   Use the executable ``qemu-system-ppc`` to simulate a complete PowerMac
>>>   PowerPC system.
>>>   -- ``g3beige``              Heathrow based PowerMAC
>>> -- ``mac99``                Mac99 based PowerMAC
>>> +- ``g3beige``           Heathrow based old world Power Macintosh G3
>>> +- ``mac99``             Core99 based generic PowerMac
>>> +- ``powermac3_1``       Power Mac G4 AGP (Sawtooth)
>>> +- ``powerbook3_2``      PowerBook G4 Titanium (Mercury)
>>> +- ``powermac7_3``       Power Mac G5 (Niagara) (only in ``qemu-system-ppc64``)
>>> +
>>>     Supported devices
>>>   -----------------
>>> @@ -15,9 +19,9 @@ QEMU emulates the following PowerMac peripherals:
>>>    *  UniNorth or Grackle PCI Bridge
>>>    *  PCI VGA compatible card with VESA Bochs Extensions
>>>    *  2 PMAC IDE interfaces with hard disk and CD-ROM support
>>> - *  NE2000 PCI adapters
>>> + *  Sungem PCI network adapter
>>>    *  Non Volatile RAM
>>> - *  VIA-CUDA with ADB keyboard and mouse.
>>> + *  VIA-CUDA or VIA-PMU99 with ot without ADB or USB keyboard and mouse.
>>>       Missing devices
>>
>> Documentation updates are always useful, but until there is consensus as to how the 
>> 32-bit and 64-bit targets should be handled then I don't think we should go ahead 
>> with a potential compatibility break/deprecation until we have a clear path forward.
>>
>> Given that freeze is so close, I suggest leaving this for 7.2 and resurrecting the 
>> appropriate thread from earlier in the year at the start of the 8.0 development cycle.
> 
> Please don't postpone patches because you were not able to review in time. A better 
> approach would be to merge these now and drop them during the freeze if any problem 
> that can't be fixed is found. The deprecation process itself is also to allow backing 
> off if this turns out to be a bad idea so no need to wait for more votes now and 
> postpone this further.

As I've previously mentioned, there is no commercial sponsorship for the QEMU Mac 
machines which is why the Mac machines are marked as Maintained, and so patch review 
is often limited by my available spare time. This does not mean that we should bypass 
the entire QEMU review process though.


ATB,

Mark.


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

* Re: [PATCH v5 20/20] mac_{old, new}world: Pass MacOS VGA NDRV in card ROM instead of fw_cfg
  2022-10-28 12:32     ` BALATON Zoltan
@ 2022-10-29  8:22       ` Mark Cave-Ayland
  2022-10-29 12:18         ` BALATON Zoltan
  0 siblings, 1 reply; 38+ messages in thread
From: Mark Cave-Ayland @ 2022-10-29  8:22 UTC (permalink / raw)
  To: BALATON Zoltan; +Cc: qemu-devel, qemu-ppc

On 28/10/2022 13:32, BALATON Zoltan wrote:

> On Fri, 28 Oct 2022, Mark Cave-Ayland wrote:
>> On 25/10/2022 22:31, BALATON Zoltan wrote:
>>> OpenBIOS cannot run FCode ROMs yet but it can detect NDRV in VGA card
>>> ROM and add it to the device tree for MacOS. Pass the NDRV this way
>>> instead of via fw_cfg. This solves the problem with OpenBIOS also
>>> adding the NDRV to ati-vga which it does not work with. This does not
>>> need any changes to OpenBIOS as this NDRV ROM handling is already
>>> there but this patch also allows simplifying OpenBIOS later to remove
>>> the fw_cfg ndrv handling from the vga FCode and also drop the
>>> vga-ndrv? option which is not needed any more as users can disable the
>>> ndrv with -device VGA,romfile="" (or override it with their own NDRV
>>> or ROM). Once FCode support is implemented in OpenBIOS, the proper
>>> FCode ROM can be set the same way so this paves the way to remove some
>>> hacks.
>>
>> This is not correct though: in a real option ROM the NDRV is included as part of 
>> the ROM payload and is not a standalone file. The IEEE-1275 PCI specification gives 
>> the correct format for an option ROM which at minimum contains a header, and likely 
>> some additional FCode.
> 
> As the commit message says that does not work with OpenBIOS at the moment but passing 
> the NDRV does. That it's not how real hardware works is not an argument after all 
> real hardware does not have fw_cfg either and this way is much simpler than fw_cfg, 
> it fixes the problem with ati-vga and it can be changed later to pass the real FCode 
> ROM the same way so I think it's a better way to handle this now as what we have 
> currently.

Right, passing the NDRV directly only happens to work because Ben's original hack is 
still in OpenBIOS. The longer term aim is to move towards the IEEE-1275 PCI 
specification: I can't see how switching from one custom mechanism to a different 
custom mechanism benefits anything here.

The problem you're actually trying to solve is that the ati-vga device should not be 
picking up the NDRV, so that's where the focus should be.

>> Isn't the immediate problem here that the NDRV handling in OpenBIOS needs to be 
>> improved so that it can be disabled for particular VGA devices such as ATI?
> 
> No change is needed to OpenBIOS (I've discussed it more in the reply to Howard on the 
> list yesterday). With this patch only VGA device will have qemu_vga.ndrv so OpenBIOS 
> won't add it for ati-vga. Also the fw_cfg and vga_ndrv? stuff can be removed from 
> OpenBIOS after this patch as it's not nedeed any more thus simplifying the vga.fs 
> FCode in OpenBIOS a lot.

The vga-ndrv? option was added for a reason though: the NDRV doesn't (yet?) work with 
KVM-PR on real Mac hardware, so to run MacOS on KVM you need a separate mechanism to 
disable the NDRV. This becomes more important when OpenBIOS gets to the stage where 
the FCode can create the DT nodes itself.

Also if we do decide to change this, it would be a compatibility break for a lot of 
existing documentation and examples: this is something we could manage going forward, 
but it needs some planning and isn't something we should be doing a few days before 
freeze.


ATB,

Mark.


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

* Re: [PATCH v5 20/20] mac_{old, new}world: Pass MacOS VGA NDRV in card ROM instead of fw_cfg
  2022-10-29  8:22       ` Mark Cave-Ayland
@ 2022-10-29 12:18         ` BALATON Zoltan
  0 siblings, 0 replies; 38+ messages in thread
From: BALATON Zoltan @ 2022-10-29 12:18 UTC (permalink / raw)
  To: Mark Cave-Ayland; +Cc: qemu-devel, qemu-ppc

On Sat, 29 Oct 2022, Mark Cave-Ayland wrote:
> On 28/10/2022 13:32, BALATON Zoltan wrote:
>> On Fri, 28 Oct 2022, Mark Cave-Ayland wrote:
>>> On 25/10/2022 22:31, BALATON Zoltan wrote:
>>>> OpenBIOS cannot run FCode ROMs yet but it can detect NDRV in VGA card
>>>> ROM and add it to the device tree for MacOS. Pass the NDRV this way
>>>> instead of via fw_cfg. This solves the problem with OpenBIOS also
>>>> adding the NDRV to ati-vga which it does not work with. This does not
>>>> need any changes to OpenBIOS as this NDRV ROM handling is already
>>>> there but this patch also allows simplifying OpenBIOS later to remove
>>>> the fw_cfg ndrv handling from the vga FCode and also drop the
>>>> vga-ndrv? option which is not needed any more as users can disable the
>>>> ndrv with -device VGA,romfile="" (or override it with their own NDRV
>>>> or ROM). Once FCode support is implemented in OpenBIOS, the proper
>>>> FCode ROM can be set the same way so this paves the way to remove some
>>>> hacks.
>>> 
>>> This is not correct though: in a real option ROM the NDRV is included as 
>>> part of the ROM payload and is not a standalone file. The IEEE-1275 PCI 
>>> specification gives the correct format for an option ROM which at minimum 
>>> contains a header, and likely some additional FCode.
>> 
>> As the commit message says that does not work with OpenBIOS at the moment 
>> but passing the NDRV does. That it's not how real hardware works is not an 
>> argument after all real hardware does not have fw_cfg either and this way 
>> is much simpler than fw_cfg, it fixes the problem with ati-vga and it can 
>> be changed later to pass the real FCode ROM the same way so I think it's a 
>> better way to handle this now as what we have currently.
>
> Right, passing the NDRV directly only happens to work because Ben's original 
> hack is still in OpenBIOS.

And this allows to remove your hack which is just doing the same in a more 
complex way and also breaks ati-vga. While this original hack is simpler 
and cleaner and works just as well until we can finally pass a real FCode 
ROM. (That probably won't be soon as OpenBIOS also progresses slowly due 
to your lack of time. So at least please let QEMU progress a bit now.)

> The longer term aim is to move towards the 
> IEEE-1275 PCI specification: I can't see how switching from one custom 
> mechanism to a different custom mechanism benefits anything here.

Can't you see really or don't want to see to keep your code instead? With 
this patch we pass the NDRV in ROM which can then simply be replaced with 
the FCode ROM once OpenBIOS can handle that and no change is needed for 
QEMU at that time, just replace qemu_vga.ndrv with the QEMU,VGA.fcode and 
done. If we keep your fw_cfg hack then it will need to be reverted at that 
point but we can do that now and simplify both QEMU and OpenBIOS by doing 
so.

> The problem you're actually trying to solve is that the ati-vga device should 
> not be picking up the NDRV, so that's where the focus should be.

And tis patch also solves that by moving the NDRV from the machine to the 
VGA device so it will only be added with that device and not with ati-vga 
so it won't be disturbing other vga cards. All this with less and simpler 
code than what we have now. What do you have against it other than it's 
making part of your code redundant? If your code is not better then you 
should not be holding on to that if there's a simpler way. (I hope it's 
not because https://en.wikipedia.org/wiki/Not_invented_here you could 
prove me wrong giving a valid reason why the current solution is better 
than this patch but I really can't see that. To me it's not about who wins 
or whose code will be in QEMU or OpenBIOS but which is the simpler and 
better working solution and in this case I think this patch is.)

>>> Isn't the immediate problem here that the NDRV handling in OpenBIOS needs 
>>> to be improved so that it can be disabled for particular VGA devices such 
>>> as ATI?
>> 
>> No change is needed to OpenBIOS (I've discussed it more in the reply to 
>> Howard on the list yesterday). With this patch only VGA device will have 
>> qemu_vga.ndrv so OpenBIOS won't add it for ati-vga. Also the fw_cfg and 
>> vga_ndrv? stuff can be removed from OpenBIOS after this patch as it's not 
>> nedeed any more thus simplifying the vga.fs FCode in OpenBIOS a lot.
>
> The vga-ndrv? option was added for a reason though: the NDRV doesn't (yet?) 
> work with KVM-PR on real Mac hardware, so to run MacOS on KVM you need a 
> separate mechanism to disable the NDRV.

You can still do that after this patch, just replace 
-prom-env='vga-ndrv?=false' with -device VGA.romfile="" and it won't add 
the NDRV. If you want to keep the non-standard vga-ndrv? option you've 
invented and is not part of IEEE-1275 so shouldn't be in OpenBIOS at all 
then you can change OpenBIOS to check this option in vga_config_cb before 
checking the ROM for NDRV and then even command lines using that option 
are backward compatibile so it's not an issue.

> This becomes more important when 
> OpenBIOS gets to the stage where the FCode can create the DT nodes itself.

It's not an issue either as fw_cfg already has a FW_CFG_PPC_IS_KVM 
variable so the FCode can handle it itself based on that variable. No 
cahange is needed to QEMU and no hack is needed in OpenBIOS for that only 
in the QEMU,VGA FCode to check for KVM and you can just add the FCode 
instead of the NDRV after this patch.

> Also if we do decide to change this, it would be a compatibility break for a 
> lot of existing documentation and examples: this is something we could manage 
> going forward, but it needs some planning and isn't something we should be 
> doing a few days before freeze.

I did submit the cleanup series in time well before the freeze and 
intended to follow up with these after that simple series. The reason it's 
got that late is your slow response time so don't blame me for that. 
Missing the freeze though means we lose almost half a year again before 
this can get to the users so I think it's better to merge this tentatively 
mow and then drop it during the freeze if some problem is found than just 
ignoring it right away so you don't need to think about it.

Regards,
BALATON Zoltan


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

* Re: [PATCH v4 16/19] mac_newworld: Add machine types for different mac99 configs
  2022-10-29  7:48       ` Mark Cave-Ayland
@ 2022-10-29 12:35         ` BALATON Zoltan
  0 siblings, 0 replies; 38+ messages in thread
From: BALATON Zoltan @ 2022-10-29 12:35 UTC (permalink / raw)
  To: Mark Cave-Ayland; +Cc: qemu-devel, qemu-ppc

On Sat, 29 Oct 2022, Mark Cave-Ayland wrote:
> On 28/10/2022 13:18, BALATON Zoltan wrote:
>> On Fri, 28 Oct 2022, Mark Cave-Ayland wrote:
>>> On 25/10/2022 17:44, BALATON Zoltan wrote:
>>>> The mac99 machine emulates different machines depending on machine
>>>> properties or even if it is run as qemu-system-ppc64 or
>>>> qemu-system-ppc. This is very confusing for users and many hours were
>>>> lost trying to explain it or finding out why commands users came up
>>>> with are not working as expected. (E.g. Windows users might think
>>>> qemu-system-ppc64 is just the 64 bit version of qemu-system-ppc and
>>>> then fail to boot a 32 bit OS with -M mac99 trying to follow an
>>>> example that had qemu-system-ppc.) To avoid such confusion, add
>>>> explicit machine types for the different configs which will work the
>>>> same with both qemu-system-ppc and qemu-system-ppc64 and also make the
>>>> command line clearer for new users.
>>> 
>>> What was the outcome of the discussion re: having separate machines for 
>>> 32-bit and 64-bit PPC targets? My understanding is the issue here was 
>>> deciding what to do, rather than actually making the code changes.
>> 
>> Who do you think will or should decide about this? There are about 3 people 
>> who care about Mac emulation on this list: you, Howard and me. You already 
>> have my and Howard's vote to introduce these machines types. Who else 
>> should vote or decide on this? Please apply this patch now and if it causes 
>> problem it can still be dropped duting the freeze but if you don't apply it 
>> now it can't get into before next spring.
>
> This is not restricted to qemu-system-ppc though: there was a discussion 
> (which was still ongoing) as to how all of QEMU should handle 32-bit and 
> 64-bit machines i.e. should qemu-system-ppc64 only contain 64-bit machines 
> and qemu-system-ppc only contain 32-bit machines? If we wish to make a change 
> here, we should wait for the outcome of this to ensure consistency here.

That's unrelated to mac99 then so we can sort that out independently now 
as this will be needed anyway to remove the confusing behaviour of mac99 
emulating different machines depending if it's part of qemu-system-ppc or 
qemu-system-ppc64. Whatever the decision with 32 vs 64 bit will be this 
will have to go so better deprecate it now so we can more easily adapt 
whatever that decision will be (if it will have a decision in the near 
fututre at all). I don't think that's a good reason to ditch this patch as 
I don't see this is getting to a decision soon which would need doing else 
than deprecating mac99.

>>> Also what was your motivation for choosing the machine names? I see you've 
>>> used powerbook for via=pmu-adb, but I think quite a few people use pmu-adb 
>>> for older OS X server hardware. At the very least some pointers to 
>>> reference device trees and some rationale behind the decision is needed 
>>> for review.
>> 
>> See my reply to Howard's message with some more info and links. My 
>> immediate motivation was that we've lost about two days when somobody 
>> contacted me about VGA pass through sending logs about all kinds of 
>> failures he got. After many logs I've noticed that he was using 
>> qemu-system-ppc64 -M mac99,via=pmu thinking that on 64bit host that's the 
>> executable he should use. Unfortunately the commands were not shared just 
>> the logs so this took a while to notice. Also if you look at the forum 
>> Howard runs you can see this problem is coming up frequently and I think 
>> the've also lost countless hours due to this. It's about time to put an end 
>> on it and stop wasting othet's time. As for The machines, the powermac ones 
>> are straight forward as those are closest to what we emulate for G4 and G5 
>> Mac. I've chosen the powerbook becuase that's the only machine I know that 
>> had PMU and ADB but If someone knows a better machine we can change this 
>> (even as bug fix during the freeze). Here's some info on this powerbook: 
>> https://ppc.0penbsd.narkive.com/s49Kcx1u/x-on-powerbook-g4
>
> In all my time working on QEMU this has happened to me once: yes, I agree it 
> can be annoying but given how rare it happens I don't see a need to make a 
> rushed decision now.

Maybe because you don't work with users like Howard or don't work with 
ati-vga like me. But we got this problem many times, I've got bitten by 
forgetting the vga-ndrv? option several times and had to remember this is 
breaking stuff so that this works for you is no reason to believe it's not 
causing problems for anybody. A better approach may be if a proposed 
solution is not breaking anything for you then accept it rather than 
having the basic view of resisting any change from anywhere. As a private 
developer you can so that but as a maintainer you should not lock out 
others from contributing and be more accepting or other solutions.

> In terms of choosing the machines for QEMU we will need a full copy of the DT 
> from real hardware for comparison with OpenBIOS, and ideally a Linux dmesg.

I don't use that pmu-adb config and I think maybe we don't even need it 
once USB emulation is fixed on powermac3_1 as those OSes should support 
powermac3_1 or g3beige so I'm OK with dropping this powerbook machine for 
now and only merging the powermac3_1 and powermac7_3 machines which I care 
more about. Can you change the patches accordingly before merge or you 
want me to submit another version with this chnage? You could also take 
the first 14 patches if you're OK with them now so I have to resend less.

Regards,
BALATON Zoltan


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

end of thread, other threads:[~2022-10-29 12:37 UTC | newest]

Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-25 16:44 [PATCH v4 00/19] Misc ppc/mac machines clean up BALATON Zoltan
2022-10-25 16:44 ` [PATCH v4 01/19] mac_newworld: Drop some variables BALATON Zoltan
2022-10-25 16:44 ` [PATCH v4 02/19] mac_oldworld: Drop some more variables BALATON Zoltan
2022-10-25 16:44 ` [PATCH v4 03/19] mac_{old|new}world: Set tbfreq at declaration BALATON Zoltan
2022-10-25 16:44 ` [PATCH v4 04/19] mac_{old|new}world: Avoid else branch by setting default value BALATON Zoltan
2022-10-25 16:44 ` [PATCH v4 05/19] mac_{old|new}world: Simplify cmdline_base calculation BALATON Zoltan
2022-10-25 16:44 ` [PATCH v4 06/19] mac_newworld: Clean up creation of Uninorth devices BALATON Zoltan
2022-10-25 16:44 ` [PATCH v4 07/19] mac_{old|new}world: Reduce number of QOM casts BALATON Zoltan
2022-10-25 16:44 ` [PATCH v4 08/19] hw/ppc/mac.h: Move newworld specific parts out from shared header BALATON Zoltan
2022-10-25 16:44 ` [PATCH v4 09/19] hw/ppc/mac.h: Move macio " BALATON Zoltan
2022-10-25 16:44 ` [PATCH v4 10/19] hw/ppc/mac.h: Move grackle-pcihost type declaration out to a header BALATON Zoltan
2022-10-25 16:44 ` [PATCH v4 11/19] hw/ppc/mac.h: Move PROM and KERNEL defines to board code BALATON Zoltan
2022-10-25 16:44 ` [PATCH v4 12/19] hw/ppc/mac.h: Rename to include/hw/nvram/mac_nvram.h BALATON Zoltan
2022-10-25 16:44 ` [PATCH v4 13/19] mac_nvram: Use NVRAM_SIZE constant BALATON Zoltan
2022-10-25 16:44 ` [PATCH v4 14/19] mac_{old|new}world: Code style fix adding missing braces to if-s BALATON Zoltan
2022-10-25 16:44 ` [PATCH v4 15/19] mac_newworld: Turn CORE99_VIA_CONFIG defines into an enum BALATON Zoltan
2022-10-25 16:44 ` [PATCH v4 16/19] mac_newworld: Add machine types for different mac99 configs BALATON Zoltan
2022-10-28  9:37   ` Mark Cave-Ayland
2022-10-28 12:18     ` BALATON Zoltan
2022-10-29  7:48       ` Mark Cave-Ayland
2022-10-29 12:35         ` BALATON Zoltan
2022-10-25 16:44 ` [PATCH v4 17/19] mac_newworld: Deprecate mac99 with G5 CPU BALATON Zoltan
2022-10-25 16:44 ` [PATCH v4 18/19] mac_newworld: Deprecate mac99 "via" option BALATON Zoltan
2022-10-25 16:44 ` [PATCH v4 19/19] mac_newworld: Document deprecation BALATON Zoltan
2022-10-28  9:41   ` Mark Cave-Ayland
2022-10-28 12:20     ` BALATON Zoltan
2022-10-29  8:09       ` Mark Cave-Ayland
2022-10-25 21:31 ` [PATCH v5 19/20] " BALATON Zoltan
2022-10-25 21:31 ` [PATCH v5 20/20] mac_{old, new}world: Pass MacOS VGA NDRV in card ROM instead of fw_cfg BALATON Zoltan
2022-10-28  9:51   ` Mark Cave-Ayland
2022-10-28 12:32     ` BALATON Zoltan
2022-10-29  8:22       ` Mark Cave-Ayland
2022-10-29 12:18         ` BALATON Zoltan
2022-10-25 21:35 ` [PATCH v4 00/19] Misc ppc/mac machines clean up BALATON Zoltan
2022-10-27  5:41 ` Howard Spoelstra
2022-10-27 12:22   ` BALATON Zoltan
2022-10-28  9:07 ` Mark Cave-Ayland
2022-10-28 12:24   ` BALATON Zoltan

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