[PATCH v2 2/5] mac_oldworld: Add machine ID register

[PATCH v2 2/5] mac_oldworld: Add machine ID register

[BALATON Zoltan]

The G3 beige machine has a machine ID register that is accessed by the
firmware to deternine the board config. Add basic emulation of it.

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

diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 3812adc441..4dd872c1a3 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -80,6 +80,15 @@ static void ppc_heathrow_reset(void *opaque)
     cpu_reset(CPU(cpu));
 }
 
+static uint64_t machine_id_read(void *opaque, hwaddr addr, unsigned size)
+{
+    return (addr == 0 && size == 2 ? 0x3d8c : 0);
+}
+
+const MemoryRegionOps machine_id_reg_ops = {
+    .read = machine_id_read,
+};
+
 static void ppc_heathrow_init(MachineState *machine)
 {
     ram_addr_t ram_size = machine->ram_size;
@@ -93,6 +102,7 @@ static void ppc_heathrow_init(MachineState *machine)
     char *filename;
     int linux_boot, i;
     MemoryRegion *bios = g_new(MemoryRegion, 1);
+    MemoryRegion *machine_id = g_new(MemoryRegion, 1);
     uint32_t kernel_base, initrd_base, cmdline_base = 0;
     int32_t kernel_size, initrd_size;
     PCIBus *pci_bus;
@@ -227,6 +237,10 @@ static void ppc_heathrow_init(MachineState *machine)
         }
     }
 
+    memory_region_init_io(machine_id, OBJECT(machine), &machine_id_reg_ops,
+                          NULL, "machine_id", 2);
+    memory_region_add_subregion(get_system_memory(), 0xff000004, machine_id);
+
     /* XXX: we register only 1 output pin for heathrow PIC */
     pic_dev = qdev_create(NULL, TYPE_HEATHROW);
     qdev_init_nofail(pic_dev);
-- 
2.21.3

[PATCH v2 0/5] Mac Old World ROM experiment

[BALATON Zoltan]

Version 2 with some more tweaks this now starts but drops in a Serial
Test Manager (see below) presumably because some POST step is failing,
I let others who know more about this machine figure out what's
missing from here.

Regards,
BALATON Zoltan


      1 :pci_update_mappings_add d=0x55a1bb6254a0 00:01.0 0,0xf3000000+0x80000
      1 pci_cfg_read grackle 00:0 @0x0 -> 0x21057
      1 pci_cfg_read grackle 00:0 @0xa8 -> 0x0
      1 pci_cfg_write grackle 00:0 @0xa8 <- 0x40e0c
      1 pci_cfg_read grackle 00:0 @0xac -> 0x0
      1 pci_cfg_write grackle 00:0 @0xac <- 0x12000000
      1 pci_cfg_read grackle 00:0 @0xac -> 0x12000000
      1 pci_cfg_write grackle 00:0 @0xac <- 0x2000000
      1 pci_cfg_read grackle 00:0 @0x70 -> 0x0
      1 pci_cfg_write grackle 00:0 @0x70 <- 0x11000000
      1 machine_id_read(0, 2)
      1 pci_cfg_read grackle 00:0 @0x8 -> 0x6000140
      1 pci_cfg_read grackle 00:0 @0xf0 -> 0x0
      1 pci_cfg_write grackle 00:0 @0xf0 <- 0x12900000
      1 machine_id_read(0, 2)
      1 portA_write unimplemented
      1 CUDA: unknown command 0x22
      1 CUDA: unknown command 0x26
      3 CUDA: unknown command 0x25
      1 pci_cfg_write grackle 00:0 @0x80 <- 0xffffffff
      1 pci_cfg_write grackle 00:0 @0x88 <- 0xffffffff
      1 pci_cfg_write grackle 00:0 @0x90 <- 0xffffffff
      1 pci_cfg_write grackle 00:0 @0x98 <- 0xffffffff
      1 pci_cfg_write grackle 00:0 @0x84 <- 0xffffffff
      1 pci_cfg_write grackle 00:0 @0x8c <- 0xffffffff
      1 pci_cfg_write grackle 00:0 @0x94 <- 0xffffffff
      1 pci_cfg_write grackle 00:0 @0x9c <- 0xffffffff
      1 pci_cfg_write grackle 00:0 @0xa0 <- 0x0
      1 pci_cfg_read grackle 00:0 @0xf0 -> 0x12900000
      1 pci_cfg_write grackle 00:0 @0xf0 <- 0x12900000
      1 machine_id_read(0, 2)
      1 pci_cfg_read grackle 00:0 @0x8 -> 0x6000140
      1 pci_cfg_read grackle 00:0 @0xf0 -> 0x12900000
      1 pci_cfg_write grackle 00:0 @0xf0 <- 0x12940000
      1 pci_cfg_write grackle 00:0 @0xf0 <- 0x12940000
      1 pci_cfg_write grackle 00:0 @0xf4 <- 0x40010fe4
      1 pci_cfg_write grackle 00:0 @0xf8 <- 0x7302293
      1 pci_cfg_write grackle 00:0 @0xfc <- 0x25302220
      1 pci_cfg_read grackle 00:0 @0xa0 -> 0x0
      1 pci_cfg_write grackle 00:0 @0xa0 <- 0x67000000
      1 pci_cfg_read grackle 00:0 @0xf0 -> 0x12940000
      1 pci_cfg_write grackle 00:0 @0xf0 <- 0x129c0000
 550755 Unassigned mem read 00000000f3014020
      1 
      1 >?
      1 ***************************************
      1 *                                     *
      1 *         Serial Test Manager         *
      1 *                                     *
      1 ***************************************
      1 
      1 T)  Execute a test, single test number follows command
      1 T0) Run ROM Checksum Test
      1 T1) Run Address Line Test
      1 T2) Run Data Line Test
      1 T3) Run Simple RAM Test
      1 T4) Run Mod3 Forward Test
      1 T5) Run Mod3 Reverse Test
      1 T6) Run NVRAM Test
      1 T8) Run AddrPattern Test
      1 T9) Run NTAWord Test
      1 A)  Execute all ROM-based tests
      1 Q)  Quick test
      1 X)  Exit STM (Continue booting)
      1 ?)  Print this menu
      1 
      1 >

BALATON Zoltan (5):
  mac_oldworld: Allow loading binary ROM image
  mac_oldworld: Add machine ID register
  grackle: Set revision in PCI config to match hardware
  mac_oldworld: Rename ppc_heathrow_reset reset to
    ppc_heathrow_cpu_reset
  mac_oldworld: Map macio to expected address at reset

 hw/pci-host/grackle.c |  2 +-
 hw/ppc/mac.h          | 12 +++++++++
 hw/ppc/mac_oldworld.c | 59 +++++++++++++++++++++++++++++++++----------
 3 files changed, 59 insertions(+), 14 deletions(-)

-- 
2.21.3

[PATCH v2 4/5] mac_oldworld: Rename ppc_heathrow_reset reset to ppc_heathrow_cpu_reset

[BALATON Zoltan]

This function resets a CPU not the whole machine so reflect that in
its name.

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

diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 4dd872c1a3..9138752ccb 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -73,7 +73,7 @@ static uint64_t translate_kernel_address(void *opaque, uint64_t addr)
     return (addr & 0x0fffffff) + KERNEL_LOAD_ADDR;
 }
 
-static void ppc_heathrow_reset(void *opaque)
+static void ppc_heathrow_cpu_reset(void *opaque)
 {
     PowerPCCPU *cpu = opaque;
 
@@ -127,7 +127,7 @@ static void ppc_heathrow_init(MachineState *machine)
 
         /* Set time-base frequency to 16.6 Mhz */
         cpu_ppc_tb_init(env,  TBFREQ);
-        qemu_register_reset(ppc_heathrow_reset, cpu);
+        qemu_register_reset(ppc_heathrow_cpu_reset, cpu);
     }
 
     /* allocate RAM */
-- 
2.21.3

[PATCH v2 5/5] mac_oldworld: Map macio to expected address at reset

[BALATON Zoltan]

Add a reset function that maps macio to the address expected by the
firmware of the board at startup.

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

diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
index 6af87d1fa0..35a5f21163 100644
--- a/hw/ppc/mac.h
+++ b/hw/ppc/mac.h
@@ -57,6 +57,18 @@
 #define OLDWORLD_IDE1_IRQ      0xe
 #define OLDWORLD_IDE1_DMA_IRQ  0x3
 
+/* g3beige machine */
+#define TYPE_HEATHROW_MACHINE MACHINE_TYPE_NAME("g3beige")
+#define HEATHROW_MACHINE(obj) OBJECT_CHECK(HeathrowMachineState, (obj), \
+                                           TYPE_HEATHROW_MACHINE)
+
+typedef struct HeathrowMachineState {
+    /*< private >*/
+    MachineState parent;
+
+    PCIDevice *macio_pci;
+} HeathrowMachineState;
+
 /* New World IRQs */
 #define NEWWORLD_CUDA_IRQ      0x19
 #define NEWWORLD_PMU_IRQ       0x19
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 9138752ccb..fa9527410d 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -73,6 +73,15 @@ static uint64_t translate_kernel_address(void *opaque, uint64_t addr)
     return (addr & 0x0fffffff) + KERNEL_LOAD_ADDR;
 }
 
+static void ppc_heathrow_reset(MachineState *machine)
+{
+    HeathrowMachineState *m = HEATHROW_MACHINE(machine);
+
+    qemu_devices_reset();
+    pci_default_write_config(m->macio_pci, PCI_COMMAND, PCI_COMMAND_MEMORY, 2);
+    pci_default_write_config(m->macio_pci, PCI_BASE_ADDRESS_0, 0xf3000000, 4);
+}
+
 static void ppc_heathrow_cpu_reset(void *opaque)
 {
     PowerPCCPU *cpu = opaque;
@@ -91,6 +100,7 @@ const MemoryRegionOps machine_id_reg_ops = {
 
 static void ppc_heathrow_init(MachineState *machine)
 {
+    HeathrowMachineState *hm = HEATHROW_MACHINE(machine);
     ram_addr_t ram_size = machine->ram_size;
     const char *kernel_filename = machine->kernel_filename;
     const char *kernel_cmdline = machine->kernel_cmdline;
@@ -298,7 +308,8 @@ static void ppc_heathrow_init(MachineState *machine)
     ide_drive_get(hd, ARRAY_SIZE(hd));
 
     /* MacIO */
-    macio = OLDWORLD_MACIO(pci_create(pci_bus, -1, TYPE_OLDWORLD_MACIO));
+    hm->macio_pci = pci_create(pci_bus, -1, TYPE_OLDWORLD_MACIO);
+    macio = OLDWORLD_MACIO(hm->macio_pci);
     dev = DEVICE(macio);
     qdev_prop_set_uint64(dev, "frequency", tbfreq);
     object_property_set_link(OBJECT(macio), OBJECT(pic_dev), "pic",
@@ -450,6 +461,7 @@ static void heathrow_class_init(ObjectClass *oc, void *data)
 
     mc->desc = "Heathrow based PowerMAC";
     mc->init = ppc_heathrow_init;
+    mc->reset = ppc_heathrow_reset;
     mc->block_default_type = IF_IDE;
     mc->max_cpus = MAX_CPUS;
 #ifndef TARGET_PPC64
@@ -466,9 +478,10 @@ static void heathrow_class_init(ObjectClass *oc, void *data)
 }
 
 static const TypeInfo ppc_heathrow_machine_info = {
-    .name          = MACHINE_TYPE_NAME("g3beige"),
+    .name          = TYPE_HEATHROW_MACHINE,
     .parent        = TYPE_MACHINE,
     .class_init    = heathrow_class_init,
+    .instance_size = sizeof(HeathrowMachineState),
     .interfaces = (InterfaceInfo[]) {
         { TYPE_FW_PATH_PROVIDER },
         { }
-- 
2.21.3

[PATCH v2 1/5] mac_oldworld: Allow loading binary ROM image

[BALATON Zoltan]

The G3 beige machine has a 4MB firmware ROM. Fix the size of the rom
region and allow loading a binary image with -bios. This makes it
possible to test emulation with a ROM image from real hardware.

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

diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 0b4c1c6373..3812adc441 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -59,6 +59,8 @@
 #define NDRV_VGA_FILENAME "qemu_vga.ndrv"
 
 #define GRACKLE_BASE 0xfec00000
+#define PROM_BASE 0xffc00000
+#define PROM_SIZE (4 * MiB)
 
 static void fw_cfg_boot_set(void *opaque, const char *boot_device,
                             Error **errp)
@@ -127,24 +129,28 @@ static void ppc_heathrow_init(MachineState *machine)
 
     memory_region_add_subregion(sysmem, 0, machine->ram);
 
-    /* allocate and load BIOS */
-    memory_region_init_rom(bios, NULL, "ppc_heathrow.bios", BIOS_SIZE,
+    /* allocate and load firmware ROM */
+    memory_region_init_rom(bios, NULL, "ppc_heathrow.bios", PROM_SIZE,
                            &error_fatal);
+    memory_region_add_subregion(sysmem, PROM_BASE, bios);
 
-    if (bios_name == NULL)
+    if (!bios_name) {
         bios_name = PROM_FILENAME;
+    }
     filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
-    memory_region_add_subregion(sysmem, PROM_ADDR, bios);
-
-    /* Load OpenBIOS (ELF) */
     if (filename) {
-        bios_size = load_elf(filename, NULL, 0, NULL, NULL, NULL, NULL, NULL,
-                             1, PPC_ELF_MACHINE, 0, 0);
+        /* Load OpenBIOS (ELF) */
+        bios_size = load_elf(filename, NULL, NULL, NULL, NULL, NULL, NULL,
+                             NULL, 1, PPC_ELF_MACHINE, 0, 0);
+        if (bios_size <= 0) {
+            /* or load binary ROM image */
+            bios_size = load_image_targphys(filename, PROM_BASE, PROM_SIZE);
+        }
         g_free(filename);
     } else {
         bios_size = -1;
     }
-    if (bios_size < 0 || bios_size > BIOS_SIZE) {
+    if (bios_size < 0 || bios_size > PROM_SIZE) {
         error_report("could not load PowerPC bios '%s'", bios_name);
         exit(1);
     }
-- 
2.21.3

[PATCH v2 3/5] grackle: Set revision in PCI config to match hardware

[BALATON Zoltan]

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/pci-host/grackle.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
index 4b3af0c704..48d11f13ab 100644
--- a/hw/pci-host/grackle.c
+++ b/hw/pci-host/grackle.c
@@ -130,7 +130,7 @@ static void grackle_pci_class_init(ObjectClass *klass, void *data)
     k->realize   = grackle_pci_realize;
     k->vendor_id = PCI_VENDOR_ID_MOTOROLA;
     k->device_id = PCI_DEVICE_ID_MOTOROLA_MPC106;
-    k->revision  = 0x00;
+    k->revision  = 0x40;
     k->class_id  = PCI_CLASS_BRIDGE_HOST;
     /*
      * PCI-facing part of the host bridge, not usable without the
-- 
2.21.3

Re: [PATCH v2 0/5] Mac Old World ROM experiment

[BALATON Zoltan]

On Sat, 13 Jun 2020, BALATON Zoltan wrote:
> Version 2 with some more tweaks this now starts but drops in a Serial
> Test Manager (see below) presumably because some POST step is failing,
> I let others who know more about this machine figure out what's
> missing from here.
>
> Regards,
> BALATON Zoltan
>
>
>      1 :pci_update_mappings_add d=0x55a1bb6254a0 00:01.0 0,0xf3000000+0x80000
>      1 pci_cfg_read grackle 00:0 @0x0 -> 0x21057
>      1 pci_cfg_read grackle 00:0 @0xa8 -> 0x0
>      1 pci_cfg_write grackle 00:0 @0xa8 <- 0x40e0c
>      1 pci_cfg_read grackle 00:0 @0xac -> 0x0
>      1 pci_cfg_write grackle 00:0 @0xac <- 0x12000000
>      1 pci_cfg_read grackle 00:0 @0xac -> 0x12000000
>      1 pci_cfg_write grackle 00:0 @0xac <- 0x2000000
>      1 pci_cfg_read grackle 00:0 @0x70 -> 0x0
>      1 pci_cfg_write grackle 00:0 @0x70 <- 0x11000000
>      1 machine_id_read(0, 2)
>      1 pci_cfg_read grackle 00:0 @0x8 -> 0x6000140
>      1 pci_cfg_read grackle 00:0 @0xf0 -> 0x0
>      1 pci_cfg_write grackle 00:0 @0xf0 <- 0x12900000
>      1 machine_id_read(0, 2)
>      1 portA_write unimplemented
>      1 CUDA: unknown command 0x22
>      1 CUDA: unknown command 0x26
>      3 CUDA: unknown command 0x25
>      1 pci_cfg_write grackle 00:0 @0x80 <- 0xffffffff
>      1 pci_cfg_write grackle 00:0 @0x88 <- 0xffffffff
>      1 pci_cfg_write grackle 00:0 @0x90 <- 0xffffffff
>      1 pci_cfg_write grackle 00:0 @0x98 <- 0xffffffff
>      1 pci_cfg_write grackle 00:0 @0x84 <- 0xffffffff
>      1 pci_cfg_write grackle 00:0 @0x8c <- 0xffffffff
>      1 pci_cfg_write grackle 00:0 @0x94 <- 0xffffffff
>      1 pci_cfg_write grackle 00:0 @0x9c <- 0xffffffff
>      1 pci_cfg_write grackle 00:0 @0xa0 <- 0x0
>      1 pci_cfg_read grackle 00:0 @0xf0 -> 0x12900000
>      1 pci_cfg_write grackle 00:0 @0xf0 <- 0x12900000
>      1 machine_id_read(0, 2)
>      1 pci_cfg_read grackle 00:0 @0x8 -> 0x6000140
>      1 pci_cfg_read grackle 00:0 @0xf0 -> 0x12900000
>      1 pci_cfg_write grackle 00:0 @0xf0 <- 0x12940000
>      1 pci_cfg_write grackle 00:0 @0xf0 <- 0x12940000
>      1 pci_cfg_write grackle 00:0 @0xf4 <- 0x40010fe4
>      1 pci_cfg_write grackle 00:0 @0xf8 <- 0x7302293
>      1 pci_cfg_write grackle 00:0 @0xfc <- 0x25302220
>      1 pci_cfg_read grackle 00:0 @0xa0 -> 0x0
>      1 pci_cfg_write grackle 00:0 @0xa0 <- 0x67000000
>      1 pci_cfg_read grackle 00:0 @0xf0 -> 0x12940000
>      1 pci_cfg_write grackle 00:0 @0xf0 <- 0x129c0000
> 550755 Unassigned mem read 00000000f3014020

So this seems to be the missing sound device (maybe trying to play the 
startup chime). Adding some dummy implementation there gets me a little 
bit further:

       2 macio: screamer read 20  4
       1 macio: screamer write 10  4 = 600000
       1 macio: screamer read 10  4
       2 macio: screamer read 20  4
       1 macio: screamer write 10  4 = 8220000
       1 macio: screamer read 10  4
       1 macio: screamer write 10  4 = 0
       1 macio: screamer read 10  4
       7 CUDA: unknown command 0x22
       2 macio: screamer read 20  4
       1 macio: screamer write 10  4 = 180000
       1 macio: screamer read 10  4
       1 CUDA: unknown command 0x22
       1 macio: screamer read 0  4
       1 macio: screamer write 0  4 = 11050000
       1 dbdma_unassigned_flush: use of unassigned channel 16
       1 dbdma_unassigned_rw: use of unassigned channel 16
       1 Unassigned mem write 0000000000240020 = 0x10006238
       1 Unassigned mem write 0000000000240024 = 0xffe32c00
       1 Unassigned mem write 0000000000240028 = 0x0
       1 Unassigned mem write 000000000024002c = 0x84006238

then stops here, I guess it may be waiting for an interrupt so probably we 
need Mark's screamer implementation to move on. Mark, any chance you can 
look at this sometimes? Why is your email address stripped from emails 
coming from the list? Is that a list setting to exclude you from replies?

Regards,
BALATON Zoltan

Re: [PATCH v2 4/5] mac_oldworld: Rename ppc_heathrow_reset reset to ppc_heathrow_cpu_reset

[Philippe Mathieu-Daudé]

On 6/13/20 3:36 PM, BALATON Zoltan wrote:
> This function resets a CPU not the whole machine so reflect that in
> its name.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>

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

> ---
>  hw/ppc/mac_oldworld.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
> index 4dd872c1a3..9138752ccb 100644
> --- a/hw/ppc/mac_oldworld.c
> +++ b/hw/ppc/mac_oldworld.c
> @@ -73,7 +73,7 @@ static uint64_t translate_kernel_address(void *opaque, uint64_t addr)
>      return (addr & 0x0fffffff) + KERNEL_LOAD_ADDR;
>  }
>  
> -static void ppc_heathrow_reset(void *opaque)
> +static void ppc_heathrow_cpu_reset(void *opaque)
>  {
>      PowerPCCPU *cpu = opaque;
>  
> @@ -127,7 +127,7 @@ static void ppc_heathrow_init(MachineState *machine)
>  
>          /* Set time-base frequency to 16.6 Mhz */
>          cpu_ppc_tb_init(env,  TBFREQ);
> -        qemu_register_reset(ppc_heathrow_reset, cpu);
> +        qemu_register_reset(ppc_heathrow_cpu_reset, cpu);
>      }
>  
>      /* allocate RAM */
> 

Re: [PATCH v2 5/5] mac_oldworld: Map macio to expected address at reset

[Philippe Mathieu-Daudé]

On 6/13/20 3:36 PM, BALATON Zoltan wrote:
> Add a reset function that maps macio to the address expected by the
> firmware of the board at startup.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>  hw/ppc/mac.h          | 12 ++++++++++++
>  hw/ppc/mac_oldworld.c | 17 +++++++++++++++--
>  2 files changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
> index 6af87d1fa0..35a5f21163 100644
> --- a/hw/ppc/mac.h
> +++ b/hw/ppc/mac.h
> @@ -57,6 +57,18 @@
>  #define OLDWORLD_IDE1_IRQ      0xe
>  #define OLDWORLD_IDE1_DMA_IRQ  0x3
>  
> +/* g3beige machine */
> +#define TYPE_HEATHROW_MACHINE MACHINE_TYPE_NAME("g3beige")
> +#define HEATHROW_MACHINE(obj) OBJECT_CHECK(HeathrowMachineState, (obj), \
> +                                           TYPE_HEATHROW_MACHINE)
> +
> +typedef struct HeathrowMachineState {
> +    /*< private >*/
> +    MachineState parent;
> +
> +    PCIDevice *macio_pci;
> +} HeathrowMachineState;
> +
>  /* New World IRQs */
>  #define NEWWORLD_CUDA_IRQ      0x19
>  #define NEWWORLD_PMU_IRQ       0x19
> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
> index 9138752ccb..fa9527410d 100644
> --- a/hw/ppc/mac_oldworld.c
> +++ b/hw/ppc/mac_oldworld.c
> @@ -73,6 +73,15 @@ static uint64_t translate_kernel_address(void *opaque, uint64_t addr)
>      return (addr & 0x0fffffff) + KERNEL_LOAD_ADDR;
>  }
>  
> +static void ppc_heathrow_reset(MachineState *machine)
> +{
> +    HeathrowMachineState *m = HEATHROW_MACHINE(machine);
> +
> +    qemu_devices_reset();
> +    pci_default_write_config(m->macio_pci, PCI_COMMAND, PCI_COMMAND_MEMORY, 2);
> +    pci_default_write_config(m->macio_pci, PCI_BASE_ADDRESS_0, 0xf3000000, 4);

Hmm either this should be the default reset state of the device,
or we miss a 'BIOS' boot code that sets this state before you can
run your code.

> +}
> +
>  static void ppc_heathrow_cpu_reset(void *opaque)
>  {
>      PowerPCCPU *cpu = opaque;
> @@ -91,6 +100,7 @@ const MemoryRegionOps machine_id_reg_ops = {
>  
>  static void ppc_heathrow_init(MachineState *machine)
>  {
> +    HeathrowMachineState *hm = HEATHROW_MACHINE(machine);
>      ram_addr_t ram_size = machine->ram_size;
>      const char *kernel_filename = machine->kernel_filename;
>      const char *kernel_cmdline = machine->kernel_cmdline;
> @@ -298,7 +308,8 @@ static void ppc_heathrow_init(MachineState *machine)
>      ide_drive_get(hd, ARRAY_SIZE(hd));
>  
>      /* MacIO */
> -    macio = OLDWORLD_MACIO(pci_create(pci_bus, -1, TYPE_OLDWORLD_MACIO));
> +    hm->macio_pci = pci_create(pci_bus, -1, TYPE_OLDWORLD_MACIO);
> +    macio = OLDWORLD_MACIO(hm->macio_pci);
>      dev = DEVICE(macio);
>      qdev_prop_set_uint64(dev, "frequency", tbfreq);
>      object_property_set_link(OBJECT(macio), OBJECT(pic_dev), "pic",
> @@ -450,6 +461,7 @@ static void heathrow_class_init(ObjectClass *oc, void *data)
>  
>      mc->desc = "Heathrow based PowerMAC";
>      mc->init = ppc_heathrow_init;
> +    mc->reset = ppc_heathrow_reset;
>      mc->block_default_type = IF_IDE;
>      mc->max_cpus = MAX_CPUS;
>  #ifndef TARGET_PPC64
> @@ -466,9 +478,10 @@ static void heathrow_class_init(ObjectClass *oc, void *data)
>  }
>  
>  static const TypeInfo ppc_heathrow_machine_info = {
> -    .name          = MACHINE_TYPE_NAME("g3beige"),
> +    .name          = TYPE_HEATHROW_MACHINE,
>      .parent        = TYPE_MACHINE,
>      .class_init    = heathrow_class_init,
> +    .instance_size = sizeof(HeathrowMachineState),
>      .interfaces = (InterfaceInfo[]) {
>          { TYPE_FW_PATH_PROVIDER },
>          { }
> 

Re: [PATCH v2 5/5] mac_oldworld: Map macio to expected address at reset

[BALATON Zoltan]

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

On Sat, 13 Jun 2020, Philippe Mathieu-Daudé wrote:
> On 6/13/20 3:36 PM, BALATON Zoltan wrote:
>> Add a reset function that maps macio to the address expected by the
>> firmware of the board at startup.
>>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>>  hw/ppc/mac.h          | 12 ++++++++++++
>>  hw/ppc/mac_oldworld.c | 17 +++++++++++++++--
>>  2 files changed, 27 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
>> index 6af87d1fa0..35a5f21163 100644
>> --- a/hw/ppc/mac.h
>> +++ b/hw/ppc/mac.h
>> @@ -57,6 +57,18 @@
>>  #define OLDWORLD_IDE1_IRQ      0xe
>>  #define OLDWORLD_IDE1_DMA_IRQ  0x3
>>
>> +/* g3beige machine */
>> +#define TYPE_HEATHROW_MACHINE MACHINE_TYPE_NAME("g3beige")
>> +#define HEATHROW_MACHINE(obj) OBJECT_CHECK(HeathrowMachineState, (obj), \
>> +                                           TYPE_HEATHROW_MACHINE)
>> +
>> +typedef struct HeathrowMachineState {
>> +    /*< private >*/
>> +    MachineState parent;
>> +
>> +    PCIDevice *macio_pci;
>> +} HeathrowMachineState;
>> +
>>  /* New World IRQs */
>>  #define NEWWORLD_CUDA_IRQ      0x19
>>  #define NEWWORLD_PMU_IRQ       0x19
>> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
>> index 9138752ccb..fa9527410d 100644
>> --- a/hw/ppc/mac_oldworld.c
>> +++ b/hw/ppc/mac_oldworld.c
>> @@ -73,6 +73,15 @@ static uint64_t translate_kernel_address(void *opaque, uint64_t addr)
>>      return (addr & 0x0fffffff) + KERNEL_LOAD_ADDR;
>>  }
>>
>> +static void ppc_heathrow_reset(MachineState *machine)
>> +{
>> +    HeathrowMachineState *m = HEATHROW_MACHINE(machine);
>> +
>> +    qemu_devices_reset();
>> +    pci_default_write_config(m->macio_pci, PCI_COMMAND, PCI_COMMAND_MEMORY, 2);
>> +    pci_default_write_config(m->macio_pci, PCI_BASE_ADDRESS_0, 0xf3000000, 4);
>
> Hmm either this should be the default reset state of the device,
> or we miss a 'BIOS' boot code that sets this state before you can
> run your code.

"My code" here that I've tried _is_ the "BIOS" (actually openprom) 
firmware ROM from real machine which does not seem to do anything to get 
this mapped there so this seems to be there on reset but I don't know how 
that gets there on real hardware. This change makes the ROM happy and does 
not seem to break OpenBIOS either but if anyone knows a better way let 
us know.

Regards,
BALATON Zoltan

Re: [PATCH v2 0/5] Mac Old World ROM experiment

[BALATON Zoltan]

On Sat, 13 Jun 2020, BALATON Zoltan wrote:
> On Sat, 13 Jun 2020, BALATON Zoltan wrote:
>> Version 2 with some more tweaks this now starts but drops in a Serial
>> Test Manager (see below) presumably because some POST step is failing,
>> I let others who know more about this machine figure out what's
>> missing from here.
>> 
>> Regards,
>> BALATON Zoltan
>> 
>>
>>      1 :pci_update_mappings_add d=0x55a1bb6254a0 00:01.0 
>> 0,0xf3000000+0x80000
>>      1 pci_cfg_read grackle 00:0 @0x0 -> 0x21057
>>      1 pci_cfg_read grackle 00:0 @0xa8 -> 0x0
>>      1 pci_cfg_write grackle 00:0 @0xa8 <- 0x40e0c
>>      1 pci_cfg_read grackle 00:0 @0xac -> 0x0
>>      1 pci_cfg_write grackle 00:0 @0xac <- 0x12000000
>>      1 pci_cfg_read grackle 00:0 @0xac -> 0x12000000
>>      1 pci_cfg_write grackle 00:0 @0xac <- 0x2000000
>>      1 pci_cfg_read grackle 00:0 @0x70 -> 0x0
>>      1 pci_cfg_write grackle 00:0 @0x70 <- 0x11000000
>>      1 machine_id_read(0, 2)
>>      1 pci_cfg_read grackle 00:0 @0x8 -> 0x6000140
>>      1 pci_cfg_read grackle 00:0 @0xf0 -> 0x0
>>      1 pci_cfg_write grackle 00:0 @0xf0 <- 0x12900000
>>      1 machine_id_read(0, 2)
>>      1 portA_write unimplemented
>>      1 CUDA: unknown command 0x22
>>      1 CUDA: unknown command 0x26
>>      3 CUDA: unknown command 0x25
>>      1 pci_cfg_write grackle 00:0 @0x80 <- 0xffffffff
>>      1 pci_cfg_write grackle 00:0 @0x88 <- 0xffffffff
>>      1 pci_cfg_write grackle 00:0 @0x90 <- 0xffffffff
>>      1 pci_cfg_write grackle 00:0 @0x98 <- 0xffffffff
>>      1 pci_cfg_write grackle 00:0 @0x84 <- 0xffffffff
>>      1 pci_cfg_write grackle 00:0 @0x8c <- 0xffffffff
>>      1 pci_cfg_write grackle 00:0 @0x94 <- 0xffffffff
>>      1 pci_cfg_write grackle 00:0 @0x9c <- 0xffffffff
>>      1 pci_cfg_write grackle 00:0 @0xa0 <- 0x0
>>      1 pci_cfg_read grackle 00:0 @0xf0 -> 0x12900000
>>      1 pci_cfg_write grackle 00:0 @0xf0 <- 0x12900000
>>      1 machine_id_read(0, 2)
>>      1 pci_cfg_read grackle 00:0 @0x8 -> 0x6000140
>>      1 pci_cfg_read grackle 00:0 @0xf0 -> 0x12900000
>>      1 pci_cfg_write grackle 00:0 @0xf0 <- 0x12940000
>>      1 pci_cfg_write grackle 00:0 @0xf0 <- 0x12940000
>>      1 pci_cfg_write grackle 00:0 @0xf4 <- 0x40010fe4
>>      1 pci_cfg_write grackle 00:0 @0xf8 <- 0x7302293
>>      1 pci_cfg_write grackle 00:0 @0xfc <- 0x25302220
>>      1 pci_cfg_read grackle 00:0 @0xa0 -> 0x0
>>      1 pci_cfg_write grackle 00:0 @0xa0 <- 0x67000000
>>      1 pci_cfg_read grackle 00:0 @0xf0 -> 0x12940000
>>      1 pci_cfg_write grackle 00:0 @0xf0 <- 0x129c0000
>> 550755 Unassigned mem read 00000000f3014020
>
> So this seems to be the missing sound device (maybe trying to play the 
> startup chime). Adding some dummy implementation there gets me a little bit 
> further:
>
>      2 macio: screamer read 20  4
>      1 macio: screamer write 10  4 = 600000
>      1 macio: screamer read 10  4
>      2 macio: screamer read 20  4
>      1 macio: screamer write 10  4 = 8220000
>      1 macio: screamer read 10  4
>      1 macio: screamer write 10  4 = 0
>      1 macio: screamer read 10  4
>      7 CUDA: unknown command 0x22
>      2 macio: screamer read 20  4
>      1 macio: screamer write 10  4 = 180000
>      1 macio: screamer read 10  4
>      1 CUDA: unknown command 0x22
>      1 macio: screamer read 0  4
>      1 macio: screamer write 0  4 = 11050000
>      1 dbdma_unassigned_flush: use of unassigned channel 16
>      1 dbdma_unassigned_rw: use of unassigned channel 16
>      1 Unassigned mem write 0000000000240020 = 0x10006238
>      1 Unassigned mem write 0000000000240024 = 0xffe32c00
>      1 Unassigned mem write 0000000000240028 = 0x0
>      1 Unassigned mem write 000000000024002c = 0x84006238
>
> then stops here, I guess it may be waiting for an interrupt so probably we

Or maybe it's the missing i2c bus in cuda (CUDA commands 0x22 and 0x25 
above) which I can imagine may try to get SPD data from RAM to configure 
memory.

Regards,
BALATON Zoltan

Re: [PATCH v2 1/5] mac_oldworld: Allow loading binary ROM image

[Mark Cave-Ayland]

On 13/06/2020 14:36, BALATON Zoltan wrote:

> The G3 beige machine has a 4MB firmware ROM. Fix the size of the rom
> region and allow loading a binary image with -bios. This makes it
> possible to test emulation with a ROM image from real hardware.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>  hw/ppc/mac_oldworld.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
> index 0b4c1c6373..3812adc441 100644
> --- a/hw/ppc/mac_oldworld.c
> +++ b/hw/ppc/mac_oldworld.c
> @@ -59,6 +59,8 @@
>  #define NDRV_VGA_FILENAME "qemu_vga.ndrv"
>  
>  #define GRACKLE_BASE 0xfec00000
> +#define PROM_BASE 0xffc00000
> +#define PROM_SIZE (4 * MiB)
>  
>  static void fw_cfg_boot_set(void *opaque, const char *boot_device,
>                              Error **errp)
> @@ -127,24 +129,28 @@ static void ppc_heathrow_init(MachineState *machine)
>  
>      memory_region_add_subregion(sysmem, 0, machine->ram);
>  
> -    /* allocate and load BIOS */
> -    memory_region_init_rom(bios, NULL, "ppc_heathrow.bios", BIOS_SIZE,
> +    /* allocate and load firmware ROM */
> +    memory_region_init_rom(bios, NULL, "ppc_heathrow.bios", PROM_SIZE,
>                             &error_fatal);
> +    memory_region_add_subregion(sysmem, PROM_BASE, bios);
>  
> -    if (bios_name == NULL)
> +    if (!bios_name) {
>          bios_name = PROM_FILENAME;
> +    }
>      filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
> -    memory_region_add_subregion(sysmem, PROM_ADDR, bios);
> -
> -    /* Load OpenBIOS (ELF) */
>      if (filename) {
> -        bios_size = load_elf(filename, NULL, 0, NULL, NULL, NULL, NULL, NULL,
> -                             1, PPC_ELF_MACHINE, 0, 0);
> +        /* Load OpenBIOS (ELF) */
> +        bios_size = load_elf(filename, NULL, NULL, NULL, NULL, NULL, NULL,
> +                             NULL, 1, PPC_ELF_MACHINE, 0, 0);
> +        if (bios_size <= 0) {
> +            /* or load binary ROM image */
> +            bios_size = load_image_targphys(filename, PROM_BASE, PROM_SIZE);
> +        }
>          g_free(filename);
>      } else {
>          bios_size = -1;
>      }
> -    if (bios_size < 0 || bios_size > BIOS_SIZE) {
> +    if (bios_size < 0 || bios_size > PROM_SIZE) {
>          error_report("could not load PowerPC bios '%s'", bios_name);
>          exit(1);
>      }

I think the logic could be improved a bit here: load_elf() can return the physical
address from the ELF, so it would make sense to use that as the address for
load_image_targphys() if present, and otherwise fall back to loading at 0xffc00000.

It may also make sense to split PROM_ADDR to PROM_ADDR_OLDWORLD and
PROM_ADDR_NEWWORLD (and similar for BIOS_SIZE) to allow these values to be adjusted
separately for each machine.


ATB,

Mark.

Re: [PATCH v2 3/5] grackle: Set revision in PCI config to match hardware

[Mark Cave-Ayland]

On 13/06/2020 14:36, BALATON Zoltan wrote:

> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>  hw/pci-host/grackle.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
> index 4b3af0c704..48d11f13ab 100644
> --- a/hw/pci-host/grackle.c
> +++ b/hw/pci-host/grackle.c
> @@ -130,7 +130,7 @@ static void grackle_pci_class_init(ObjectClass *klass, void *data)
>      k->realize   = grackle_pci_realize;
>      k->vendor_id = PCI_VENDOR_ID_MOTOROLA;
>      k->device_id = PCI_DEVICE_ID_MOTOROLA_MPC106;
> -    k->revision  = 0x00;
> +    k->revision  = 0x40;
>      k->class_id  = PCI_CLASS_BRIDGE_HOST;
>      /*
>       * PCI-facing part of the host bridge, not usable without the

Out of curiosity does the BIOS you are using require this, or is it purely for
cosmetic purposes? I'm sure I've seen device trees with several different revisions
here...


ATB,

Mark.

Re: [PATCH v2 4/5] mac_oldworld: Rename ppc_heathrow_reset reset to ppc_heathrow_cpu_reset

[Mark Cave-Ayland]

On 13/06/2020 14:36, BALATON Zoltan wrote:

> This function resets a CPU not the whole machine so reflect that in
> its name.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>  hw/ppc/mac_oldworld.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
> index 4dd872c1a3..9138752ccb 100644
> --- a/hw/ppc/mac_oldworld.c
> +++ b/hw/ppc/mac_oldworld.c
> @@ -73,7 +73,7 @@ static uint64_t translate_kernel_address(void *opaque, uint64_t addr)
>      return (addr & 0x0fffffff) + KERNEL_LOAD_ADDR;
>  }
>  
> -static void ppc_heathrow_reset(void *opaque)
> +static void ppc_heathrow_cpu_reset(void *opaque)
>  {
>      PowerPCCPU *cpu = opaque;
>  
> @@ -127,7 +127,7 @@ static void ppc_heathrow_init(MachineState *machine)
>  
>          /* Set time-base frequency to 16.6 Mhz */
>          cpu_ppc_tb_init(env,  TBFREQ);
> -        qemu_register_reset(ppc_heathrow_reset, cpu);
> +        qemu_register_reset(ppc_heathrow_cpu_reset, cpu);
>      }
>  
>      /* allocate RAM */

Technically this is a board level reset which just happens to pass the CPU for the
opaque, so I'm not quite sold on this one (as an example look at mac_newworld.c where
using the ELF load address for the PROM would require a dynamic NIP which is most
conveniently accessed via a MachineState).


ATB,

Mark.

Re: [PATCH v2 5/5] mac_oldworld: Map macio to expected address at reset

[Mark Cave-Ayland]

On 13/06/2020 14:36, BALATON Zoltan wrote:

> Add a reset function that maps macio to the address expected by the
> firmware of the board at startup.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>  hw/ppc/mac.h          | 12 ++++++++++++
>  hw/ppc/mac_oldworld.c | 17 +++++++++++++++--
>  2 files changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
> index 6af87d1fa0..35a5f21163 100644
> --- a/hw/ppc/mac.h
> +++ b/hw/ppc/mac.h
> @@ -57,6 +57,18 @@
>  #define OLDWORLD_IDE1_IRQ      0xe
>  #define OLDWORLD_IDE1_DMA_IRQ  0x3
>  
> +/* g3beige machine */
> +#define TYPE_HEATHROW_MACHINE MACHINE_TYPE_NAME("g3beige")
> +#define HEATHROW_MACHINE(obj) OBJECT_CHECK(HeathrowMachineState, (obj), \
> +                                           TYPE_HEATHROW_MACHINE)
> +
> +typedef struct HeathrowMachineState {
> +    /*< private >*/
> +    MachineState parent;
> +
> +    PCIDevice *macio_pci;
> +} HeathrowMachineState;
> +
>  /* New World IRQs */
>  #define NEWWORLD_CUDA_IRQ      0x19
>  #define NEWWORLD_PMU_IRQ       0x19
> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
> index 9138752ccb..fa9527410d 100644
> --- a/hw/ppc/mac_oldworld.c
> +++ b/hw/ppc/mac_oldworld.c
> @@ -73,6 +73,15 @@ static uint64_t translate_kernel_address(void *opaque, uint64_t addr)
>      return (addr & 0x0fffffff) + KERNEL_LOAD_ADDR;
>  }
>  
> +static void ppc_heathrow_reset(MachineState *machine)
> +{
> +    HeathrowMachineState *m = HEATHROW_MACHINE(machine);
> +
> +    qemu_devices_reset();
> +    pci_default_write_config(m->macio_pci, PCI_COMMAND, PCI_COMMAND_MEMORY, 2);
> +    pci_default_write_config(m->macio_pci, PCI_BASE_ADDRESS_0, 0xf3000000, 4);
> +}
> +
>  static void ppc_heathrow_cpu_reset(void *opaque)
>  {
>      PowerPCCPU *cpu = opaque;
> @@ -91,6 +100,7 @@ const MemoryRegionOps machine_id_reg_ops = {
>  
>  static void ppc_heathrow_init(MachineState *machine)
>  {
> +    HeathrowMachineState *hm = HEATHROW_MACHINE(machine);
>      ram_addr_t ram_size = machine->ram_size;
>      const char *kernel_filename = machine->kernel_filename;
>      const char *kernel_cmdline = machine->kernel_cmdline;
> @@ -298,7 +308,8 @@ static void ppc_heathrow_init(MachineState *machine)
>      ide_drive_get(hd, ARRAY_SIZE(hd));
>  
>      /* MacIO */
> -    macio = OLDWORLD_MACIO(pci_create(pci_bus, -1, TYPE_OLDWORLD_MACIO));
> +    hm->macio_pci = pci_create(pci_bus, -1, TYPE_OLDWORLD_MACIO);
> +    macio = OLDWORLD_MACIO(hm->macio_pci);
>      dev = DEVICE(macio);
>      qdev_prop_set_uint64(dev, "frequency", tbfreq);
>      object_property_set_link(OBJECT(macio), OBJECT(pic_dev), "pic",
> @@ -450,6 +461,7 @@ static void heathrow_class_init(ObjectClass *oc, void *data)
>  
>      mc->desc = "Heathrow based PowerMAC";
>      mc->init = ppc_heathrow_init;
> +    mc->reset = ppc_heathrow_reset;
>      mc->block_default_type = IF_IDE;
>      mc->max_cpus = MAX_CPUS;
>  #ifndef TARGET_PPC64
> @@ -466,9 +478,10 @@ static void heathrow_class_init(ObjectClass *oc, void *data)
>  }
>  
>  static const TypeInfo ppc_heathrow_machine_info = {
> -    .name          = MACHINE_TYPE_NAME("g3beige"),
> +    .name          = TYPE_HEATHROW_MACHINE,
>      .parent        = TYPE_MACHINE,
>      .class_init    = heathrow_class_init,
> +    .instance_size = sizeof(HeathrowMachineState),
>      .interfaces = (InterfaceInfo[]) {
>          { TYPE_FW_PATH_PROVIDER },
>          { }

This doesn't feel right to me - either the PROM should be configuring the BARs as it
requires before trying to use the macio device, or the macio device has a fixed
mapping. Possibly the latter could be true given that things are so early in the boot
process?

Are there any hints in the macio "reg" and "address" properties suggesting that this
might be the case?


ATB,

Mark.

Re: [PATCH v2 3/5] grackle: Set revision in PCI config to match hardware

[BALATON Zoltan]

On Sun, 14 Jun 2020, Mark Cave-Ayland wrote:
> On 13/06/2020 14:36, BALATON Zoltan wrote:
>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>>  hw/pci-host/grackle.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
>> index 4b3af0c704..48d11f13ab 100644
>> --- a/hw/pci-host/grackle.c
>> +++ b/hw/pci-host/grackle.c
>> @@ -130,7 +130,7 @@ static void grackle_pci_class_init(ObjectClass *klass, void *data)
>>      k->realize   = grackle_pci_realize;
>>      k->vendor_id = PCI_VENDOR_ID_MOTOROLA;
>>      k->device_id = PCI_DEVICE_ID_MOTOROLA_MPC106;
>> -    k->revision  = 0x00;
>> +    k->revision  = 0x40;
>>      k->class_id  = PCI_CLASS_BRIDGE_HOST;
>>      /*
>>       * PCI-facing part of the host bridge, not usable without the
>
> Out of curiosity does the BIOS you are using require this, or is it purely for
> cosmetic purposes? I'm sure I've seen device trees with several different revisions
> here...

Don't know, got it from here:

https://github.com/dingusdev/dingusppc/blob/master/devices/mpc106.h

Regards,
BALATON Zoltan

Re: [PATCH v2 4/5] mac_oldworld: Rename ppc_heathrow_reset reset to ppc_heathrow_cpu_reset

[BALATON Zoltan]

On Sun, 14 Jun 2020, Mark Cave-Ayland wrote:
> On 13/06/2020 14:36, BALATON Zoltan wrote:
>
>> This function resets a CPU not the whole machine so reflect that in
>> its name.
>>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>>  hw/ppc/mac_oldworld.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
>> index 4dd872c1a3..9138752ccb 100644
>> --- a/hw/ppc/mac_oldworld.c
>> +++ b/hw/ppc/mac_oldworld.c
>> @@ -73,7 +73,7 @@ static uint64_t translate_kernel_address(void *opaque, uint64_t addr)
>>      return (addr & 0x0fffffff) + KERNEL_LOAD_ADDR;
>>  }
>>
>> -static void ppc_heathrow_reset(void *opaque)
>> +static void ppc_heathrow_cpu_reset(void *opaque)
>>  {
>>      PowerPCCPU *cpu = opaque;
>>
>> @@ -127,7 +127,7 @@ static void ppc_heathrow_init(MachineState *machine)
>>
>>          /* Set time-base frequency to 16.6 Mhz */
>>          cpu_ppc_tb_init(env,  TBFREQ);
>> -        qemu_register_reset(ppc_heathrow_reset, cpu);
>> +        qemu_register_reset(ppc_heathrow_cpu_reset, cpu);
>>      }
>>
>>      /* allocate RAM */
>
> Technically this is a board level reset which just happens to pass the CPU for the
> opaque, so I'm not quite sold on this one (as an example look at mac_newworld.c where
> using the ELF load address for the PROM would require a dynamic NIP which is most
> conveniently accessed via a MachineState).

The mac_newworld also registers a per CPU reset function like this one. 
This could be done in the machine level reset I add in next patch but 
there could be multiple CPUs and I don't know how to access those from 
MachineState so I've left this CPU reset functions alone which could be 
cleaned up later.

Ideally I should not need a machine reset to set the initial BAR mapping 
but otherwise the sequence of registered reset funcs are not guaranteed 
and the PCI device is reset during qemu_devices_reset() which clears the 
BARs so it won't stay mapped otherwise. I could not find an easier way to 
map this BAR.

Regards,
BALATON Zoltan

Re: [PATCH v2 5/5] mac_oldworld: Map macio to expected address at reset

[BALATON Zoltan]

On Sun, 14 Jun 2020, Mark Cave-Ayland wrote:
> On 13/06/2020 14:36, BALATON Zoltan wrote:
>
>> Add a reset function that maps macio to the address expected by the
>> firmware of the board at startup.
>>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>>  hw/ppc/mac.h          | 12 ++++++++++++
>>  hw/ppc/mac_oldworld.c | 17 +++++++++++++++--
>>  2 files changed, 27 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
>> index 6af87d1fa0..35a5f21163 100644
>> --- a/hw/ppc/mac.h
>> +++ b/hw/ppc/mac.h
>> @@ -57,6 +57,18 @@
>>  #define OLDWORLD_IDE1_IRQ      0xe
>>  #define OLDWORLD_IDE1_DMA_IRQ  0x3
>>
>> +/* g3beige machine */
>> +#define TYPE_HEATHROW_MACHINE MACHINE_TYPE_NAME("g3beige")
>> +#define HEATHROW_MACHINE(obj) OBJECT_CHECK(HeathrowMachineState, (obj), \
>> +                                           TYPE_HEATHROW_MACHINE)
>> +
>> +typedef struct HeathrowMachineState {
>> +    /*< private >*/
>> +    MachineState parent;
>> +
>> +    PCIDevice *macio_pci;
>> +} HeathrowMachineState;
>> +
>>  /* New World IRQs */
>>  #define NEWWORLD_CUDA_IRQ      0x19
>>  #define NEWWORLD_PMU_IRQ       0x19
>> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
>> index 9138752ccb..fa9527410d 100644
>> --- a/hw/ppc/mac_oldworld.c
>> +++ b/hw/ppc/mac_oldworld.c
>> @@ -73,6 +73,15 @@ static uint64_t translate_kernel_address(void *opaque, uint64_t addr)
>>      return (addr & 0x0fffffff) + KERNEL_LOAD_ADDR;
>>  }
>>
>> +static void ppc_heathrow_reset(MachineState *machine)
>> +{
>> +    HeathrowMachineState *m = HEATHROW_MACHINE(machine);
>> +
>> +    qemu_devices_reset();
>> +    pci_default_write_config(m->macio_pci, PCI_COMMAND, PCI_COMMAND_MEMORY, 2);
>> +    pci_default_write_config(m->macio_pci, PCI_BASE_ADDRESS_0, 0xf3000000, 4);
>> +}
>> +
>>  static void ppc_heathrow_cpu_reset(void *opaque)
>>  {
>>      PowerPCCPU *cpu = opaque;
>> @@ -91,6 +100,7 @@ const MemoryRegionOps machine_id_reg_ops = {
>>
>>  static void ppc_heathrow_init(MachineState *machine)
>>  {
>> +    HeathrowMachineState *hm = HEATHROW_MACHINE(machine);
>>      ram_addr_t ram_size = machine->ram_size;
>>      const char *kernel_filename = machine->kernel_filename;
>>      const char *kernel_cmdline = machine->kernel_cmdline;
>> @@ -298,7 +308,8 @@ static void ppc_heathrow_init(MachineState *machine)
>>      ide_drive_get(hd, ARRAY_SIZE(hd));
>>
>>      /* MacIO */
>> -    macio = OLDWORLD_MACIO(pci_create(pci_bus, -1, TYPE_OLDWORLD_MACIO));
>> +    hm->macio_pci = pci_create(pci_bus, -1, TYPE_OLDWORLD_MACIO);
>> +    macio = OLDWORLD_MACIO(hm->macio_pci);
>>      dev = DEVICE(macio);
>>      qdev_prop_set_uint64(dev, "frequency", tbfreq);
>>      object_property_set_link(OBJECT(macio), OBJECT(pic_dev), "pic",
>> @@ -450,6 +461,7 @@ static void heathrow_class_init(ObjectClass *oc, void *data)
>>
>>      mc->desc = "Heathrow based PowerMAC";
>>      mc->init = ppc_heathrow_init;
>> +    mc->reset = ppc_heathrow_reset;
>>      mc->block_default_type = IF_IDE;
>>      mc->max_cpus = MAX_CPUS;
>>  #ifndef TARGET_PPC64
>> @@ -466,9 +478,10 @@ static void heathrow_class_init(ObjectClass *oc, void *data)
>>  }
>>
>>  static const TypeInfo ppc_heathrow_machine_info = {
>> -    .name          = MACHINE_TYPE_NAME("g3beige"),
>> +    .name          = TYPE_HEATHROW_MACHINE,
>>      .parent        = TYPE_MACHINE,
>>      .class_init    = heathrow_class_init,
>> +    .instance_size = sizeof(HeathrowMachineState),
>>      .interfaces = (InterfaceInfo[]) {
>>          { TYPE_FW_PATH_PROVIDER },
>>          { }
>
> This doesn't feel right to me - either the PROM should be configuring the BARs as it
> requires before trying to use the macio device, or the macio device has a fixed
> mapping. Possibly the latter could be true given that things are so early in the boot
> process?

It looks suspicious but unless there's something in the ROM before it 
starts accessig macio @ 0xf3000000 that I've missed I don't know how it 
would get there. I haven't check all the disassembly but logging PCI 
accesses that shows grackle config regs and unassigned mem writes did not 
show any signs of the ROM mapping it there so unless this should be done 
by a reg somewhere we implement but don't do the mapping it should be 
there from the beginning.

> Are there any hints in the macio "reg" and "address" properties suggesting that this
> might be the case?

I haven't got a device tree that I know coming from a real machine, the 
ones I've found show macio mapped to 0xf3000000 but that's after OF 
started, don't know how it got there from a device tree.

Regards,
BALATON Zoltan

Re: [PATCH v2 1/5] mac_oldworld: Allow loading binary ROM image

[BALATON Zoltan]

On Sun, 14 Jun 2020, Mark Cave-Ayland wrote:
> On 13/06/2020 14:36, BALATON Zoltan wrote:
>
>> The G3 beige machine has a 4MB firmware ROM. Fix the size of the rom
>> region and allow loading a binary image with -bios. This makes it
>> possible to test emulation with a ROM image from real hardware.
>>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>>  hw/ppc/mac_oldworld.c | 24 +++++++++++++++---------
>>  1 file changed, 15 insertions(+), 9 deletions(-)
>>
>> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
>> index 0b4c1c6373..3812adc441 100644
>> --- a/hw/ppc/mac_oldworld.c
>> +++ b/hw/ppc/mac_oldworld.c
>> @@ -59,6 +59,8 @@
>>  #define NDRV_VGA_FILENAME "qemu_vga.ndrv"
>>
>>  #define GRACKLE_BASE 0xfec00000
>> +#define PROM_BASE 0xffc00000
>> +#define PROM_SIZE (4 * MiB)
>>
>>  static void fw_cfg_boot_set(void *opaque, const char *boot_device,
>>                              Error **errp)
>> @@ -127,24 +129,28 @@ static void ppc_heathrow_init(MachineState *machine)
>>
>>      memory_region_add_subregion(sysmem, 0, machine->ram);
>>
>> -    /* allocate and load BIOS */
>> -    memory_region_init_rom(bios, NULL, "ppc_heathrow.bios", BIOS_SIZE,
>> +    /* allocate and load firmware ROM */
>> +    memory_region_init_rom(bios, NULL, "ppc_heathrow.bios", PROM_SIZE,
>>                             &error_fatal);
>> +    memory_region_add_subregion(sysmem, PROM_BASE, bios);
>>
>> -    if (bios_name == NULL)
>> +    if (!bios_name) {
>>          bios_name = PROM_FILENAME;
>> +    }
>>      filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
>> -    memory_region_add_subregion(sysmem, PROM_ADDR, bios);
>> -
>> -    /* Load OpenBIOS (ELF) */
>>      if (filename) {
>> -        bios_size = load_elf(filename, NULL, 0, NULL, NULL, NULL, NULL, NULL,
>> -                             1, PPC_ELF_MACHINE, 0, 0);
>> +        /* Load OpenBIOS (ELF) */
>> +        bios_size = load_elf(filename, NULL, NULL, NULL, NULL, NULL, NULL,
>> +                             NULL, 1, PPC_ELF_MACHINE, 0, 0);
>> +        if (bios_size <= 0) {
>> +            /* or load binary ROM image */
>> +            bios_size = load_image_targphys(filename, PROM_BASE, PROM_SIZE);
>> +        }
>>          g_free(filename);
>>      } else {
>>          bios_size = -1;
>>      }
>> -    if (bios_size < 0 || bios_size > BIOS_SIZE) {
>> +    if (bios_size < 0 || bios_size > PROM_SIZE) {
>>          error_report("could not load PowerPC bios '%s'", bios_name);
>>          exit(1);
>>      }
>
> I think the logic could be improved a bit here: load_elf() can return the physical
> address from the ELF, so it would make sense to use that as the address for
> load_image_targphys() if present, and otherwise fall back to loading at 0xffc00000.

I don't get this. No need to do it that way because load_elf already loads 
the image at address specified in ELF file (I guess because it still works 
with OpenBIOS after this patch) so don't have to call load_image_targphys 
for that case. The above tries load_elf and only if it did not succeed 
calls load_image_targphys to load a binary image to fill the ROM. I don't 
see how this logic could be simpler.

Maybe we need the load address from the ELF to check if an ELF would 
overflow the region as in elf_addr + bios_size > PROM_ADDR + PROM_SIZE but 
I'm not sure. Any suggestion?

> It may also make sense to split PROM_ADDR to PROM_ADDR_OLDWORLD and
> PROM_ADDR_NEWWORLD (and similar for BIOS_SIZE) to allow these values to be adjusted
> separately for each machine.

BIOS_SIZE is not used in this board after this patch any more so that's 
basically PROM_SIZE_NEWWORLD now which can be defined in mac_newworld and 
removed from mac.h. Then we have separate PROM_SIZE for each board. I've 
also defined PROM_ADDR here in mac_oldworld and similar define can be 
added to mac_newworld if needed. These should not be in mac.h I think as 
these are board specific. I regard the previous BIOS_* values specific to 
OpenBIOS not to boards so now that boards can use other ROMs not just 
OpenBIOS BIOS_SIZE may not be needed, what we need is the size of the ROM 
chip on board instead.

Regards,
BALATON Zoltan