[PATCH v5 00/11] Mac Old World ROM experiment

[PATCH v5 00/11] Mac Old World ROM experiment

[BALATON Zoltan]

v5: Rebased on master, added some more clean ups, CUDA i2c is still to
be sorted out, help with that is welcome.

Regards,
BALATON Zoltan

BALATON Zoltan (11):
  mac_oldworld: Allow loading binary ROM image
  mac_newworld: Allow loading binary ROM image
  mac_oldworld: Drop a variable, use get_system_memory() directly
  mac_oldworld: Drop some variables
  grackle: Set revision in PCI config to match hardware
  mac_oldworld: Rename ppc_heathrow_reset to ppc_heathrow_cpu_reset
  mac_oldworld: Map macio to expected address at reset
  mac_oldworld: Add machine ID register
  macio: Add dummy screamer register area
  WIP macio/cuda: Attempt to add i2c support
  mac_oldworld: Add SPD data to cover RAM

 hw/misc/macio/cuda.c         |  62 ++++++++++++++++-
 hw/misc/macio/macio.c        |  34 ++++++++++
 hw/pci-host/grackle.c        |   2 +-
 hw/ppc/mac.h                 |  15 ++++-
 hw/ppc/mac_newworld.c        |  22 +++---
 hw/ppc/mac_oldworld.c        | 127 ++++++++++++++++++++++++++---------
 include/hw/misc/macio/cuda.h |   1 +
 7 files changed, 219 insertions(+), 44 deletions(-)

-- 
2.21.3

[PATCH v5 05/11] 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 v5 05/11] grackle: Set revision in PCI config to match hardware

[Mark Cave-Ayland]

On 16/06/2020 14:47, 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

It seems the current value is 0x0, but in one of my dumps I see a value of 0x1, and
you're suggesting a value of 0x40. Given that there are clearly multiple revisions of
the hardware, I'd suggest dropping this for now since it's not clear to me yet
exactly which real hardware is being targetted and what side-effect this might have.


ATB,

Mark.

[PATCH v5 09/11] macio: Add dummy screamer register area

[BALATON Zoltan]

The only thing this returns is an idle status so the firmware
continues, otherwise just ignores and logs access for debugging. This
is a stop gap until proper implementation of this device lands.

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

diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index 8ba7af073c..c7e8556ca6 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -26,6 +26,7 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu/module.h"
+#include "qemu/log.h"
 #include "hw/ppc/mac.h"
 #include "hw/misc/macio/cuda.h"
 #include "hw/pci/pci.h"
@@ -94,6 +95,33 @@ static void macio_bar_setup(MacIOState *s)
     macio_escc_legacy_setup(s);
 }
 
+#define AWAC_CODEC_STATUS_REG 0x20
+
+#define AWAC_MAKER_CRYSTAL 1
+#define AWAC_REV_SCREAMER 3
+#define AWAC_VALID_DATA 0x40
+
+static uint64_t screamer_read(void *opaque, hwaddr addr, unsigned size)
+{
+    qemu_log_mask(LOG_UNIMP,
+                  "macio: screamer read %" HWADDR_PRIx "  %d\n", addr, size);
+    return (addr == AWAC_CODEC_STATUS_REG ? AWAC_VALID_DATA << 8 |
+            AWAC_MAKER_CRYSTAL << 16 | AWAC_REV_SCREAMER << 20 : 0);
+}
+
+static void screamer_write(void *opaque, hwaddr addr,
+                           uint64_t val, unsigned size)
+{
+    qemu_log_mask(LOG_UNIMP,
+                  "macio: screamer write %" HWADDR_PRIx "  %d = %"PRIx64"\n",
+                  addr, size, val);
+}
+
+const MemoryRegionOps screamer_ops = {
+    .read = screamer_read,
+    .write = screamer_write,
+};
+
 static void macio_common_realize(PCIDevice *d, Error **errp)
 {
     MacIOState *s = MACIO(d);
@@ -149,6 +177,7 @@ static void macio_oldworld_realize(PCIDevice *d, Error **errp)
     DeviceState *pic_dev = DEVICE(os->pic);
     Error *err = NULL;
     SysBusDevice *sysbus_dev;
+    MemoryRegion *screamer = g_new(MemoryRegion, 1);
 
     macio_common_realize(d, &err);
     if (err) {
@@ -208,6 +237,11 @@ static void macio_oldworld_realize(PCIDevice *d, Error **errp)
         error_propagate(errp, err);
         return;
     }
+
+    /* Dummy screamer sound device */
+    memory_region_init_io(screamer, OBJECT(d), &screamer_ops, NULL,
+                          "screamer", 0x2000);
+    memory_region_add_subregion(&s->bar, 0x14000, screamer);
 }
 
 static void macio_init_ide(MacIOState *s, MACIOIDEState *ide, int index)
-- 
2.21.3

Re: [PATCH v5 09/11] macio: Add dummy screamer register area

[Mark Cave-Ayland]

On 16/06/2020 14:47, BALATON Zoltan wrote:

> The only thing this returns is an idle status so the firmware
> continues, otherwise just ignores and logs access for debugging. This
> is a stop gap until proper implementation of this device lands.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>  hw/misc/macio/macio.c | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
> index 8ba7af073c..c7e8556ca6 100644
> --- a/hw/misc/macio/macio.c
> +++ b/hw/misc/macio/macio.c
> @@ -26,6 +26,7 @@
>  #include "qemu/osdep.h"
>  #include "qapi/error.h"
>  #include "qemu/module.h"
> +#include "qemu/log.h"
>  #include "hw/ppc/mac.h"
>  #include "hw/misc/macio/cuda.h"
>  #include "hw/pci/pci.h"
> @@ -94,6 +95,33 @@ static void macio_bar_setup(MacIOState *s)
>      macio_escc_legacy_setup(s);
>  }
>  
> +#define AWAC_CODEC_STATUS_REG 0x20
> +
> +#define AWAC_MAKER_CRYSTAL 1
> +#define AWAC_REV_SCREAMER 3
> +#define AWAC_VALID_DATA 0x40
> +
> +static uint64_t screamer_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    qemu_log_mask(LOG_UNIMP,
> +                  "macio: screamer read %" HWADDR_PRIx "  %d\n", addr, size);
> +    return (addr == AWAC_CODEC_STATUS_REG ? AWAC_VALID_DATA << 8 |
> +            AWAC_MAKER_CRYSTAL << 16 | AWAC_REV_SCREAMER << 20 : 0);
> +}
> +
> +static void screamer_write(void *opaque, hwaddr addr,
> +                           uint64_t val, unsigned size)
> +{
> +    qemu_log_mask(LOG_UNIMP,
> +                  "macio: screamer write %" HWADDR_PRIx "  %d = %"PRIx64"\n",
> +                  addr, size, val);
> +}
> +
> +const MemoryRegionOps screamer_ops = {
> +    .read = screamer_read,
> +    .write = screamer_write,
> +};

static const.

>  static void macio_common_realize(PCIDevice *d, Error **errp)
>  {
>      MacIOState *s = MACIO(d);
> @@ -149,6 +177,7 @@ static void macio_oldworld_realize(PCIDevice *d, Error **errp)
>      DeviceState *pic_dev = DEVICE(os->pic);
>      Error *err = NULL;
>      SysBusDevice *sysbus_dev;
> +    MemoryRegion *screamer = g_new(MemoryRegion, 1);
>  
>      macio_common_realize(d, &err);
>      if (err) {
> @@ -208,6 +237,11 @@ static void macio_oldworld_realize(PCIDevice *d, Error **errp)
>          error_propagate(errp, err);
>          return;
>      }
> +
> +    /* Dummy screamer sound device */
> +    memory_region_init_io(screamer, OBJECT(d), &screamer_ops, NULL,
> +                          "screamer", 0x2000);
> +    memory_region_add_subregion(&s->bar, 0x14000, screamer);
>  }
>  
>  static void macio_init_ide(MacIOState *s, MACIOIDEState *ide, int index)

Again I'm wary of adding empty devices here as the main issue around the screamer
code (and why it has not been submitted upstream) is that it can cause random hangs
for MacOS on startup. Does it regress any MacOS 9 through 10.5 boot tests?

FWIW I've rebased the latest version of my screamer branch at
https://github.com/mcayland/qemu/commits/screamer to git master if you want to see if
any noise comes out.


ATB,

Mark.

Re: [PATCH v5 09/11] macio: Add dummy screamer register area

[BALATON Zoltan]

On Fri, 26 Jun 2020, Mark Cave-Ayland wrote:
> Again I'm wary of adding empty devices here as the main issue around the screamer
> code (and why it has not been submitted upstream) is that it can cause random hangs
> for MacOS on startup. Does it regress any MacOS 9 through 10.5 boot tests?
>
> FWIW I've rebased the latest version of my screamer branch at
> https://github.com/mcayland/qemu/commits/screamer to git master if you want to see if
> any noise comes out.

It seems the dummy screamer patch is not enough so I've tried on top of 
your screamer branch now. It does not make sound but it crashes (this is 
with --audio-drv-list=alsa in case that matters):

17609@1593346435.329466:cuda_packet_receive length 5
17609@1593346435.329467:cuda_packet_receive_data [0] 0x01
17609@1593346435.329469:cuda_packet_receive_data [1] 0x22
17609@1593346435.329470:cuda_packet_receive_data [2] 0x8a
17609@1593346435.329471:cuda_packet_receive_data [3] 0x01
17609@1593346435.329472:cuda_packet_receive_data [4] 0x29
17609@1593346435.329473:cuda_receive_packet_cmd handling command GET_SET_IIC
CUDA: unimplemented GET_SET_IIC write 0x45 3
17609@1593346435.329476:i2c_event start(addr:0x50)
smbus: error: Unexpected send start condition in state -1
17609@1593346435.329478:cuda_packet_send length 3
17609@1593346435.329479:cuda_packet_send_data [0] 0x01
17609@1593346435.329480:cuda_packet_send_data [1] 0x00
17609@1593346435.329482:cuda_packet_send_data [2] 0x22
17609@1593346435.329483:cuda_delay_set_sr_int
17609@1593346435.329514:cuda_data_recv recv: 0x01
17609@1593346435.329516:cuda_delay_set_sr_int
17609@1593346435.329548:cuda_data_recv recv: 0x00
17609@1593346435.329550:cuda_delay_set_sr_int
17609@1593346435.329588:cuda_data_recv recv: 0x22
17609@1593346435.329590:cuda_delay_set_sr_int
17609@1593346435.366095:cuda_delay_set_sr_int
DBDMA[10]: writel 0x000000000000080c <= 0xffe40020
DBDMA[10]: channel 0x10 reg 0x3
DBDMA[10]: dbdma_cmdptr_load 0xffe40020
DBDMA[10]: writel 0x0000000000000800 <= 0xf0000000
DBDMA[10]: channel 0x10 reg 0x0
DBDMA[10]:  Clearing RUN !
DBDMA[10]:  clearing PAUSE !
DBDMA[10]:   -> ACTIVE down !
DBDMA[10]:  new status=0x00000000
DBDMA[10]: readl 0x0000000000000804 => 0x00000000
DBDMA[10]: channel 0x10 reg 0x1
DBDMA[10]: writel 0x0000000000000800 <= 0xf0008000
DBDMA[10]: channel 0x10 reg 0x0
DBDMA[10]:  Setting RUN !
DBDMA[10]:  clearing PAUSE !
DBDMA[10]:  -> ACTIVE up !
DBDMA[10]:  new status=0x00008400
DBDMA[10]: readl 0x0000000000000804 => 0x00008400
DBDMA[10]: channel 0x10 reg 0x1
DBDMA: -> DBDMA_run_bh
DBDMA[10]: channel_run
DBDMA[10]: dbdma_cmd 0x555556802c50
DBDMA[10]:     req_count 0x6238
DBDMA[10]:     command 0x1000
DBDMA[10]:     phy_addr 0xffe32c00
DBDMA[10]:     cmd_dep 0x00000000
DBDMA[10]:     res_count 0x0000
DBDMA[10]:     xfer_status 0x0000
DBDMA[10]: * OUTPUT_LAST *
DBDMA[10]: start_output
DBDMA[10]: addr 0xffe32c00 key 0x0
DBDMA: <- DBDMA_run_bh

Thread 1 "qemu-system-ppc" received signal SIGSEGV, Segmentation fault.
0x0000555555a13216 in pmac_screamer_tx_transfer (io=0x5555568083a8) at 
hw/audio/screamer.c:79
79	    io->dma_end(io);

#0  0x0000555555a13216 in pmac_screamer_tx_transfer (io=0x5555568083a8) at hw/audio/screamer.c:79
#1  0x00005555559dc7bb in audio_run_out (s=0x555556868dc0) at audio/audio.c:1181
#2  0x00005555559dc7bb in audio_run (s=0x555556868dc0, msg=msg@entry=0x555555e69664 "alsa run (prepared)") at audio/audio.c:1372
#3  0x0000555555bab458 in alsa_poll_handler (opaque=0x55555770eb50) at audio/alsaaudio.c:199
#4  0x0000555555cad63b in aio_dispatch_handler (ctx=ctx@entry=0x555556487860, node=0x55555770e700) at util/aio-posix.c:328
#5  0x0000555555cadf0c in aio_dispatch_handlers (ctx=0x555556487860) at util/aio-posix.c:371
#6  0x0000555555cadf0c in aio_dispatch (ctx=0x555556487860) at util/aio-posix.c:381
#7  0x0000555555cbe60e in aio_ctx_dispatch (source=<optimized out>, callback=<optimized out>, user_data=<optimized out>) at util/async.c:306
#8  0x00007ffff7cc6665 in g_main_context_dispatch () at /lib64/libglib-2.0.so.0
#9  0x0000555555cc3938 in glib_pollfds_poll () at util/main-loop.c:219
#10 0x0000555555cc3938 in os_host_main_loop_wait (timeout=<optimized out>) at util/main-loop.c:242
#11 0x0000555555cc3938 in main_loop_wait (nonblocking=nonblocking@entry=0) at util/main-loop.c:518
#12 0x0000555555932a49 in qemu_main_loop () at qemu/softmmu/vl.c:1664
#13 0x000055555585138e in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at qemu/softmmu/main.c:49

Does the same on master with your patches and my series.

Regards,
BALATON Zoltan

Re: [PATCH v5 09/11] macio: Add dummy screamer register area

[BALATON Zoltan]

Here it is with --enable-debug and additional screamer debug:

SCREAMER: screamer_read: addr 0000000000000000 -> 0
SCREAMER: screamer_write: addr 0000000000000000 val 11
SCREAMER: screamer_control_write: val 17
SCREAMER: basic rate: 44100
DBDMA[10]: writel 0x000000000000080c <= 0x00000010
DBDMA[10]: channel 0x10 reg 0x3
DBDMA[10]: dbdma_cmdptr_load 0x00000010
DBDMA[10]: writel 0x0000000000000800 <= 0xf0000000
DBDMA[10]: channel 0x10 reg 0x0
DBDMA[10]:  Clearing RUN !
DBDMA[10]:  clearing PAUSE !
DBDMA[10]:   -> ACTIVE down !
DBDMA[10]:  new status=0x00000000
SCREAMER: DMA TX flush!
DBDMA[10]: readl 0x0000000000000804 => 0x00000000
DBDMA[10]: channel 0x10 reg 0x1
DBDMA[10]: writel 0x0000000000000800 <= 0xf0008000
DBDMA[10]: channel 0x10 reg 0x0
DBDMA[10]:  Setting RUN !
DBDMA[10]:  clearing PAUSE !
DBDMA[10]:  -> ACTIVE up !
DBDMA[10]:  new status=0x00008400
DBDMA[10]: readl 0x0000000000000804 => 0x00008400
DBDMA[10]: channel 0x10 reg 0x1
DBDMA: -> DBDMA_run_bh
DBDMA[10]: channel_run
DBDMA[10]: dbdma_cmd 0x555556aac340
DBDMA[10]:     req_count 0x8000
DBDMA[10]:     command 0x0000
DBDMA[10]:     phy_addr 0x00000100
DBDMA[10]:     cmd_dep 0x00000000
DBDMA[10]:     res_count 0x0000
DBDMA[10]:     xfer_status 0x0000
DBDMA[10]: * OUTPUT_MORE *
DBDMA[10]: start_output
DBDMA[10]: addr 0x100 key 0x0
SCREAMER: DMA TX defer interrupt!
DBDMA: <- DBDMA_run_bh
SCREAMER: Processing deferred buffer
SCREAMER: DMA TX transfer: addr 100 len: 8000  bpos: 0

Thread 1 "qemu-system-ppc" received signal SIGSEGV, Segmentation fault.
0x0000000094ff7c19 in ?? ()

(gdb) bt
#0  0x0000000094ff7c19 in  ()
#1  0x0000555555acb1e2 in pmac_screamer_tx_transfer (io=0x555556ab1a98) at hw/audio/screamer.c:79
#2  0x0000555555acb4dd in screamerspk_callback (opaque=0x555556aad630, avail=16384) at hw/audio/screamer.c:155
#3  0x0000555555a6af3d in audio_run_out (s=0x555556b12bd0) at audio/audio.c:1181
#4  0x0000555555a6b886 in audio_run (s=0x555556b12bd0, msg=0x55555609d4a9 "alsa run (prepared)") at audio/audio.c:1372
#5  0x0000555555d00ce9 in alsa_poll_handler (opaque=0x555557959c60) at audio/alsaaudio.c:199
#6  0x0000555555e57079 in aio_dispatch_handler (ctx=0x5555567257f0, node=0x555557a0c6b0) at util/aio-posix.c:328
#7  0x0000555555e57232 in aio_dispatch_handlers (ctx=0x5555567257f0) at util/aio-posix.c:371
#8  0x0000555555e57288 in aio_dispatch (ctx=0x5555567257f0) at util/aio-posix.c:381
#9  0x0000555555e6d373 in aio_ctx_dispatch (source=0x5555567257f0, callback=0x0, user_data=0x0) at util/async.c:306
#10 0x00007ffff7cc6665 in g_main_context_dispatch () at /lib64/libglib-2.0.so.0
#11 0x0000555555e74898 in glib_pollfds_poll () at util/main-loop.c:219
#12 0x0000555555e74912 in os_host_main_loop_wait (timeout=28915159) at util/main-loop.c:242
#13 0x0000555555e74a17 in main_loop_wait (nonblocking=0) at util/main-loop.c:518
#14 0x0000555555981d35 in qemu_main_loop () at qemu/softmmu/vl.c:1664
#15 0x0000555555df59dc in main (argc=17, argv=0x7fffffffdf28, envp=0x7fffffffdfb8) at qemu/softmmu/main.c:49
(gdb) up
#1  0x0000555555acb1e2 in pmac_screamer_tx_transfer (io=0x555556ab1a98) at hw/audio/screamer.c:79
79	    io->dma_end(io);
(gdb) p/x *io
$1 = {opaque = 0xa2140923, channel = 0x79130821, addr = 0x14137e1f, len = 0x0, is_last = 0x0, is_dma_out = 0x3408f81a, dma_end = 0x94ff7c19, processing = 0x19,  dma_mem = 0x53f5351b, dma_len = 0xc7f99f1e, dir = 0x21fbe921}

Looks like dma_end is not pointing to the expected end procedure. Maybe something has overwritten it?

Regards,
BALATON Zoltan

Re: [PATCH v5 09/11] macio: Add dummy screamer register area

[BALATON Zoltan]

On Sun, 28 Jun 2020, BALATON Zoltan wrote:
> Here it is with --enable-debug and additional screamer debug:
>
> SCREAMER: screamer_read: addr 0000000000000000 -> 0
> SCREAMER: screamer_write: addr 0000000000000000 val 11
> SCREAMER: screamer_control_write: val 17
> SCREAMER: basic rate: 44100
> DBDMA[10]: writel 0x000000000000080c <= 0x00000010
> DBDMA[10]: channel 0x10 reg 0x3
> DBDMA[10]: dbdma_cmdptr_load 0x00000010
> DBDMA[10]: writel 0x0000000000000800 <= 0xf0000000
> DBDMA[10]: channel 0x10 reg 0x0
> DBDMA[10]:  Clearing RUN !
> DBDMA[10]:  clearing PAUSE !
> DBDMA[10]:   -> ACTIVE down !
> DBDMA[10]:  new status=0x00000000
> SCREAMER: DMA TX flush!
> DBDMA[10]: readl 0x0000000000000804 => 0x00000000
> DBDMA[10]: channel 0x10 reg 0x1
> DBDMA[10]: writel 0x0000000000000800 <= 0xf0008000
> DBDMA[10]: channel 0x10 reg 0x0
> DBDMA[10]:  Setting RUN !
> DBDMA[10]:  clearing PAUSE !
> DBDMA[10]:  -> ACTIVE up !
> DBDMA[10]:  new status=0x00008400
> DBDMA[10]: readl 0x0000000000000804 => 0x00008400
> DBDMA[10]: channel 0x10 reg 0x1
> DBDMA: -> DBDMA_run_bh
> DBDMA[10]: channel_run
> DBDMA[10]: dbdma_cmd 0x555556aac340
> DBDMA[10]:     req_count 0x8000
> DBDMA[10]:     command 0x0000
> DBDMA[10]:     phy_addr 0x00000100
> DBDMA[10]:     cmd_dep 0x00000000
> DBDMA[10]:     res_count 0x0000
> DBDMA[10]:     xfer_status 0x0000
> DBDMA[10]: * OUTPUT_MORE *
> DBDMA[10]: start_output
> DBDMA[10]: addr 0x100 key 0x0
> SCREAMER: DMA TX defer interrupt!
> DBDMA: <- DBDMA_run_bh
> SCREAMER: Processing deferred buffer
> SCREAMER: DMA TX transfer: addr 100 len: 8000  bpos: 0
>
> Thread 1 "qemu-system-ppc" received signal SIGSEGV, Segmentation fault.
> 0x0000000094ff7c19 in ?? ()
>
> (gdb) bt
> #0  0x0000000094ff7c19 in  ()
> #1  0x0000555555acb1e2 in pmac_screamer_tx_transfer (io=0x555556ab1a98) at 
> hw/audio/screamer.c:79
> #2  0x0000555555acb4dd in screamerspk_callback (opaque=0x555556aad630, 
> avail=16384) at hw/audio/screamer.c:155
> #3  0x0000555555a6af3d in audio_run_out (s=0x555556b12bd0) at 
> audio/audio.c:1181
> #4  0x0000555555a6b886 in audio_run (s=0x555556b12bd0, msg=0x55555609d4a9 
> "alsa run (prepared)") at audio/audio.c:1372
> #5  0x0000555555d00ce9 in alsa_poll_handler (opaque=0x555557959c60) at 
> audio/alsaaudio.c:199
> #6  0x0000555555e57079 in aio_dispatch_handler (ctx=0x5555567257f0, 
> node=0x555557a0c6b0) at util/aio-posix.c:328
> #7  0x0000555555e57232 in aio_dispatch_handlers (ctx=0x5555567257f0) at 
> util/aio-posix.c:371
> #8  0x0000555555e57288 in aio_dispatch (ctx=0x5555567257f0) at 
> util/aio-posix.c:381
> #9  0x0000555555e6d373 in aio_ctx_dispatch (source=0x5555567257f0, 
> callback=0x0, user_data=0x0) at util/async.c:306
> #10 0x00007ffff7cc6665 in g_main_context_dispatch () at 
> /lib64/libglib-2.0.so.0
> #11 0x0000555555e74898 in glib_pollfds_poll () at util/main-loop.c:219
> #12 0x0000555555e74912 in os_host_main_loop_wait (timeout=28915159) at 
> util/main-loop.c:242
> #13 0x0000555555e74a17 in main_loop_wait (nonblocking=0) at 
> util/main-loop.c:518
> #14 0x0000555555981d35 in qemu_main_loop () at qemu/softmmu/vl.c:1664
> #15 0x0000555555df59dc in main (argc=17, argv=0x7fffffffdf28, 
> envp=0x7fffffffdfb8) at qemu/softmmu/main.c:49
> (gdb) up
> #1  0x0000555555acb1e2 in pmac_screamer_tx_transfer (io=0x555556ab1a98) at 
> hw/audio/screamer.c:79
> 79	    io->dma_end(io);
> (gdb) p/x *io
> $1 = {opaque = 0xa2140923, channel = 0x79130821, addr = 0x14137e1f, len = 
> 0x0, is_last = 0x0, is_dma_out = 0x3408f81a, dma_end = 0x94ff7c19, processing 
> = 0x19,  dma_mem = 0x53f5351b, dma_len = 0xc7f99f1e, dir = 0x21fbe921}
>
> Looks like dma_end is not pointing to the expected end procedure. Maybe 
> something has overwritten it?

Looks like the dma op itself corrupts the struct:

(gdb) b pmac_screamer_tx_transfer
Breakpoint 1 at 0x555555acb12c: file hw/audio/screamer.c, line 66.
[...]
DBDMA: -> DBDMA_run_bh
DBDMA[10]: channel_run
DBDMA[10]: dbdma_cmd 0x555556aac340
DBDMA[10]:     req_count 0x8000
DBDMA[10]:     command 0x0000
DBDMA[10]:     phy_addr 0x00000100
DBDMA[10]:     cmd_dep 0x00000000
DBDMA[10]:     res_count 0x0000
DBDMA[10]:     xfer_status 0x0000
DBDMA[10]: * OUTPUT_MORE *
DBDMA[10]: start_output
DBDMA[10]: addr 0x100 key 0x0
SCREAMER: DMA TX defer interrupt!
DBDMA: <- DBDMA_run_bh
SCREAMER: Processing deferred buffer

Thread 1 "qemu-system-ppc" hit Breakpoint 1, pmac_screamer_tx_transfer 
(io=0x555556ab1a98) at hw/audio/screamer.c:66
66	    ScreamerState *s = io->opaque;
(gdb) p/x *io
$4 = {opaque = 0x555556aad630, channel = 0x555556aac290, addr = 0x100, len = 0x8000, is_last = 0x0, is_dma_out = 0x1, dma_end = 0x555555b7d2aa, processing = 0x1, dma_mem = 0x0, dma_len = 0x0, dir = 0x0}
(gdb) p dbdma_end
$5 = {void (DBDMA_io *)} 0x555555b7d2aa <dbdma_end>
(gdb) n
68	    SCREAMER_DPRINTF("DMA TX transfer: addr %" HWADDR_PRIx
(gdb)
SCREAMER: DMA TX transfer: addr 100 len: 8000  bpos: 0
71	    dma_memory_read(&address_space_memory, io->addr, 
&s->buf[s->bpos], io->len);
(gdb) n
73	    s->bpos += io->len;
(gdb) p/x *io
$6 = {opaque = 0xa2140923, channel = 0x79130821, addr = 0x14137e1f, len = 0x60f3d1d, is_last = 0x0, is_dma_out = 0x3408f81a, dma_end = 0x94ff7c19, processing = 0x19, dma_mem = 0x53f5351b, dma_len = 0xc7f99f1e, dir = 0x21fbe921}

Regards,
BALATON Zoltan

Re: [PATCH v5 09/11] macio: Add dummy screamer register area

[BALATON Zoltan]

With increasing screamer buffer size (you may want to fix this and 
prevent buffer overflows, maybe should allocate it dynamically based on 
what the guest programs?):

diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
index 75f1853a7b..05b289e086 100644
--- a/hw/ppc/mac.h
+++ b/hw/ppc/mac.h
@@ -108,7 +108,7 @@ typedef struct Core99MachineState {
  #define TYPE_SCREAMER "screamer"
  #define SCREAMER(obj) OBJECT_CHECK(ScreamerState, (obj), TYPE_SCREAMER)

-#define SCREAMER_BUFFER_SIZE 0x4000
+#define SCREAMER_BUFFER_SIZE 0x8000

  typedef struct ScreamerState {
      /*< private >*/

it now plays the startup chord and goes further but ends up accessing some 
unassigned memory (I think it probably wants to map framebuffer or detect 
PCI devices but something is not yet right, I don't see it talking to PCI 
devices yet):

SCREAMER: Processing deferred buffer
SCREAMER: DMA TX transfer: addr 58100 len: 8000  bpos: 0
SCREAMER: DMA TX defer interrupt!
SCREAMER: ########### AUDIO WRITE! 0 / 32768 - 11152
SCREAMER: ########### AUDIO WRITE! 11152 / 32768 - 4128
SCREAMER: ########### AUDIO WRITE! 15280 / 32768 - 1104
SCREAMER: ########### AUDIO WRITE! 16384 / 32768 - 7024
SCREAMER: ########### AUDIO WRITE! 23408 / 32768 - 4128
SCREAMER: ########### AUDIO WRITE! 27536 / 32768 - 4128
SCREAMER: ########### AUDIO WRITE! 31664 / 32768 - 1104
SCREAMER: Processing deferred buffer
SCREAMER: DMA TX transfer: addr 60100 len: 2b60  bpos: 0
SCREAMER: screamer_write: addr 0000000000000001 val 1280
SCREAMER: screamer_codec_write: addr 0000000000000001 val 280
SCREAMER: setting mute: 0, attenuation L: 0 R: 0
SCREAMER: screamer_read: addr 0000000000000002 -> 403100
SCREAMER: screamer_read: addr 0000000000000001 -> 1280
SCREAMER: screamer_write: addr 0000000000000001 val 6002
SCREAMER: screamer_codec_write: addr 0000000000000006 val 2
cuda_delay_set_sr_int
cuda_delay_set_sr_int
cuda_data_send send: 0x01
cuda_delay_set_sr_int
cuda_data_send send: 0x22
cuda_delay_set_sr_int
cuda_data_send send: 0x8a
cuda_delay_set_sr_int
cuda_data_send send: 0x01
cuda_delay_set_sr_int
cuda_data_send send: 0x6f
cuda_delay_set_sr_int
cuda_delay_set_sr_int
cuda_packet_receive length 5
cuda_packet_receive_data [0] 0x01
cuda_packet_receive_data [1] 0x22
cuda_packet_receive_data [2] 0x8a
cuda_packet_receive_data [3] 0x01
cuda_packet_receive_data [4] 0x6f
cuda_receive_packet_cmd handling command GET_SET_IIC
CUDA: unimplemented GET_SET_IIC write 0x45 3
CUDA: GET_SET_IIC: wrong parameters 4
cuda_packet_send length 4
cuda_packet_send_data [0] 0x02
cuda_packet_send_data [1] 0x05
cuda_packet_send_data [2] 0x01
cuda_packet_send_data [3] 0x22
cuda_delay_set_sr_int
cuda_data_recv recv: 0x02
cuda_delay_set_sr_int
cuda_data_recv recv: 0x05
cuda_delay_set_sr_int
cuda_data_recv recv: 0x01
cuda_delay_set_sr_int
cuda_data_recv recv: 0x22
cuda_delay_set_sr_int
cuda_delay_set_sr_int
pci_cfg_read grackle 00:0 @0x8 -> 0x6000140
pci_cfg_read grackle 00:0 @0xa8 -> 0x40e0c
pci_cfg_write grackle 00:0 @0xa8 <- 0x40e0e
pci_cfg_read grackle 00:0 @0xac -> 0x2000000
pci_cfg_write grackle 00:0 @0xac <- 0x82470284
pci_cfg_write grackle 00:0 @0xac <- 0xc2470284
SCREAMER: ########### AUDIO WRITE! 0 / 11104 - 11104
Unassigned mem write 00000000f3011190 = 0x9000000
Unassigned mem write 00000000f3011190 = 0xb000000
Unassigned mem write 00000000f3011190 = 0x9000000
Unassigned mem write 00000000f3011190 = 0x9000000
Unassigned mem write 00000000f3011190 = 0xb000000
Unassigned mem write 00000000f3011190 = 0x9000000
Unassigned mem write 00000000f3011190 = 0x1000000
Unassigned mem write 00000000f3011190 = 0x3000000
Unassigned mem write 00000000f3011190 = 0x1000000
Unassigned mem write 00000000f3011190 = 0x1000000
Unassigned mem write 00000000f3011190 = 0x3000000
Unassigned mem write 00000000f3011190 = 0x1000000
Unassigned mem write 00000000f3011190 = 0x1000000
Unassigned mem write 00000000f3011190 = 0x3000000
Unassigned mem write 00000000f3011190 = 0x1000000
Unassigned mem write 00000000f3011190 = 0x9000000
Unassigned mem write 00000000f3011190 = 0xb000000
Unassigned mem write 00000000f3011190 = 0x9000000
Unassigned mem write 00000000f3011190 = 0x9000000
Unassigned mem write 00000000f3011190 = 0xb000000
Unassigned mem write 00000000f3011190 = 0x9000000
Unassigned mem write 00000000f3011190 = 0x1000000
Unassigned mem write 00000000f3011190 = 0x3000000
Unassigned mem write 00000000f3011190 = 0x1000000
Unassigned mem write 00000000f3011190 = 0x1000000
Unassigned mem write 00000000f3011190 = 0x3000000
Unassigned mem write 00000000f3011190 = 0x1000000
Unassigned mem write 00000000f3011190 = 0x3000000
Unassigned mem read 00000000f3011190
Unassigned mem write 00000000f3011190 = 0x1000000
Unassigned mem write 00000000f3011190 = 0x3000000
Unassigned mem read 00000000f3011190
Unassigned mem write 00000000f3011190 = 0x1000000
Unassigned mem write 00000000f3011190 = 0x3000000
Unassigned mem read 00000000f3011190
Unassigned mem write 00000000f3011190 = 0x1000000
Unassigned mem write 00000000f3011190 = 0x3000000
Unassigned mem read 00000000f3011190
Unassigned mem write 00000000f3011190 = 0x1000000
Unassigned mem write 00000000f3011190 = 0x3000000
Unassigned mem read 00000000f3011190
[bunch of more accesses to this address...]
Unassigned mem read 00000000f3011170
Unassigned mem read 00000000f3011170
cuda_delay_set_sr_int
cuda_delay_set_sr_int
cuda_data_send send: 0x01
cuda_delay_set_sr_int
cuda_data_send send: 0x25
cuda_delay_set_sr_int
cuda_data_send send: 0xa6
cuda_delay_set_sr_int
cuda_data_send send: 0x00
cuda_delay_set_sr_int
cuda_data_send send: 0xa7
cuda_delay_set_sr_int
cuda_delay_set_sr_int
cuda_packet_receive length 5
cuda_packet_receive_data [0] 0x01
cuda_packet_receive_data [1] 0x25
cuda_packet_receive_data [2] 0xa6
cuda_packet_receive_data [3] 0x00
cuda_packet_receive_data [4] 0xa7
cuda_receive_packet_cmd handling command COMBINED_FORMAT_IIC
CUDA: COMBINED_FORMAT_IIC: wrong parameters 4
cuda_packet_send length 4
cuda_packet_send_data [0] 0x02
cuda_packet_send_data [1] 0x05
cuda_packet_send_data [2] 0x01
cuda_packet_send_data [3] 0x25
cuda_delay_set_sr_int
cuda_data_recv recv: 0x02
cuda_delay_set_sr_int
cuda_data_recv recv: 0x05
cuda_delay_set_sr_int
cuda_data_recv recv: 0x01
cuda_delay_set_sr_int
cuda_data_recv recv: 0x25
cuda_delay_set_sr_int
cuda_data_send send: 0x00
cuda_delay_set_sr_int
cuda_delay_set_sr_int
cuda_packet_receive length 1
cuda_packet_receive_data [0] 0x00
cuda_packet_send length 3
cuda_packet_send_data [0] 0x00
cuda_packet_send_data [1] 0x00
cuda_packet_send_data [2] 0x25
cuda_delay_set_sr_int
cuda_data_recv recv: 0x00
cuda_delay_set_sr_int
cuda_data_recv recv: 0x00
cuda_delay_set_sr_int
cuda_data_recv recv: 0x25
cuda_delay_set_sr_int
cuda_delay_set_sr_int
cuda_data_send send: 0x01
cuda_delay_set_sr_int
cuda_data_send send: 0x01
cuda_delay_set_sr_int
cuda_data_send send: 0xff
cuda_delay_set_sr_int
cuda_delay_set_sr_int
cuda_packet_receive length 3
cuda_packet_receive_data [0] 0x01
cuda_packet_receive_data [1] 0x01
cuda_packet_receive_data [2] 0xff
cuda_receive_packet_cmd handling command AUTOPOLL
cuda_packet_send length 3
cuda_packet_send_data [0] 0x01
cuda_packet_send_data [1] 0x00
cuda_packet_send_data [2] 0x01
cuda_delay_set_sr_int
cuda_data_recv recv: 0x01
cuda_delay_set_sr_int
cuda_data_recv recv: 0x00
cuda_delay_set_sr_int
cuda_data_recv recv: 0x01
cuda_delay_set_sr_int
cuda_delay_set_sr_int
cuda_data_send send: 0x00
cuda_delay_set_sr_int
cuda_data_send send: 0x2f
cuda_delay_set_sr_int
cuda_delay_set_sr_int
cuda_packet_receive length 2
cuda_packet_receive_data [0] 0x00
cuda_packet_receive_data [1] 0x2f
cuda_packet_send length 4
cuda_packet_send_data [0] 0x00
cuda_packet_send_data [1] 0x00
cuda_packet_send_data [2] 0x02
cuda_packet_send_data [3] 0x01
cuda_delay_set_sr_int
cuda_data_recv recv: 0x00
cuda_delay_set_sr_int
cuda_data_recv recv: 0x00
cuda_delay_set_sr_int
cuda_data_recv recv: 0x02
cuda_delay_set_sr_int
cuda_data_recv recv: 0x01
cuda_delay_set_sr_int
cuda_delay_set_sr_int
pci_cfg_read grackle 00:0 @0x3c -> 0x0
pci_cfg_read grackle 00:0 @0x3c -> 0x0
pci_cfg_write grackle 00:0 @0x3f <- 0x0
pci_cfg_write grackle 00:0 @0x3c <- 0x0
pci_cfg_write grackle 00:0 @0x3d <- 0x0
pci_cfg_write grackle 00:0 @0x3e <- 0x0
cuda_data_send send: 0x00
cuda_delay_set_sr_int
cuda_data_send send: 0x00
cuda_delay_set_sr_int
cuda_delay_set_sr_int
cuda_packet_receive length 2
cuda_packet_receive_data [0] 0x00
cuda_packet_receive_data [1] 0x00
cuda_packet_send length 3
cuda_packet_send_data [0] 0x00
cuda_packet_send_data [1] 0x00
cuda_packet_send_data [2] 0x00
cuda_delay_set_sr_int
Unassigned mem read 000000000fefe7b0
Unassigned mem write 000000000feffffc = 0x0
Unassigned mem write 000000000feffff8 = 0x0
[...]
Unassigned mem write 000000000febe018 = 0x0
Unassigned mem write 000000000febe014 = 0x0
Unassigned mem write 000000000febe010 = 0x0
Unassigned mem write 000000000febe00c = 0x0
Unassigned mem write 000000000febe008 = 0x0
Unassigned mem write 000000000febe004 = 0x0
Unassigned mem write 000000000febe000 = 0x0
Unassigned mem write 000000000fefe908 = 0x0
Unassigned mem write 000000000fefe90c = 0x0
Unassigned mem write 000000000fefefbc = 0x0
Unassigned mem write 000000000fefefb8 = 0x0
Unassigned mem write 000000000fefefb4 = 0x0
Unassigned mem write 000000000fefefb0 = 0x0
Unassigned mem write 000000000fefefac = 0x0
Unassigned mem write 000000000fefefa8 = 0x0
Unassigned mem write 000000000fefefa4 = 0x20000
Unassigned mem write 000000000fefefa0 = 0x2
Unassigned mem write 000000000fefef9c = 0x10000
Unassigned mem write 000000000fefef98 = 0x0
Unassigned mem write 000000000fefef94 = 0x0
Unassigned mem write 000000000fefef90 = 0x0
Unassigned mem write 000000000fefef8c = 0x0
Unassigned mem write 000000000fefef88 = 0x0
Unassigned mem write 000000000fefef84 = 0x0
Unassigned mem write 000000000fefef80 = 0x0
Unassigned mem write 000000000fefef7c = 0x0
Unassigned mem write 000000000fefef78 = 0x0
Unassigned mem write 000000000fefef74 = 0x0
Unassigned mem write 000000000fefef70 = 0x0
Unassigned mem write 000000000fefef6c = 0xfee5e8
Unassigned mem write 000000000fefef68 = 0x3fb97a0
Unassigned mem write 000000000fefef64 = 0x3fb97a0
Unassigned mem write 000000000fefef60 = 0x0
Unassigned mem write 000000000fefed5c = 0x0
Unassigned mem write 000000000fefed58 = 0x0
Unassigned mem write 000000000fefed54 = 0x0
Unassigned mem write 000000000fefed50 = 0x0
Unassigned mem write 000000000fefed4c = 0x0
Unassigned mem write 000000000fefed48 = 0x0
Unassigned mem write 000000000fefed44 = 0x0
Unassigned mem write 000000000fefed40 = 0x0
Unassigned mem write 000000000fefed3c = 0x0
Unassigned mem write 000000000fefed38 = 0x0
[...]
Unassigned mem write 000000000fefed64 = 0x0
Unassigned mem write 000000000fefed60 = 0x0
Unassigned mem write 000000000fefe630 = 0xfff0d000
Unassigned mem write 000000000fefe5b0 = 0xfff16880
Unassigned mem write 000000000fefec4c = 0x100000
Unassigned mem write 000000000fefe634 = 0xfeff000
Unassigned mem write 000000000fefe638 = 0xfebe000
Unassigned mem write 000000000fefe63c = 0x10000000
Unassigned mem write 000000000fefe640 = 0x0
Unassigned mem write 000000000fefe644 = 0x0
Unassigned mem write 000000000fefe648 = 0x6806e740
Unassigned mem write 000000000fefe64c = 0xfff10000
Unassigned mem write 000000000fefe650 = 0xfff11400
Unassigned mem write 000000000fefe654 = 0x68fff100
Unassigned mem write 000000000fefe658 = 0xfeff100
Unassigned mem write 000000000fefe65c = 0xfeff100
Unassigned mem write 000000000fefe670 = 0x200000
Unassigned mem write 000000000fefe678 = 0xff9fffff
Unassigned mem write 000000000fefe674 = 0xe00000
Unassigned mem write 000000000fefe67c = 0xfeff070
Unassigned mem write 000000000fefe680 = 0x7c
Unassigned mem write 000000000fefe688 = 0x12
Unassigned mem write 000000000fefe684 = 0xfefe920
Unassigned mem write 000000000fefe66c = 0xfefeab8
Unassigned mem write 000000000fefe910 = 0x0
Unassigned mem write 000000000fefefc0 = 0x5fffefc0
Unassigned mem write 000000000fefefc4 = 0x0
Unassigned mem write 000000000fefefc8 = 0x5fffeb20
Unassigned mem write 000000000fefefcc = 0x100
Unassigned mem write 000000000fefefce = 0x80
Unassigned mem write 000000000fefefd0 = 0x5fffeba0
Unassigned mem write 000000000fefefd4 = 0x102
Unassigned mem write 000000000fefefd6 = 0xa0
Unassigned mem write 000000000fefefd8 = 0x5fffef60
Unassigned mem write 000000000fefefdc = 0x101
Unassigned mem write 000000000fefefde = 0x60
Unassigned mem write 000000000fefefe0 = 0x5fffee60
Unassigned mem write 000000000fefefe4 = 0x101
Unassigned mem write 000000000fefefe6 = 0x100
Unassigned mem write 000000000fefefe8 = 0x5fffed60
Unassigned mem write 000000000fefefec = 0x100
Unassigned mem write 000000000fefefee = 0x100
Unassigned mem write 000000000fefeff0 = 0x5fffec40
Unassigned mem write 000000000fefeff4 = 0x105
Unassigned mem write 000000000fefeff6 = 0x120
Unassigned mem write 000000000fefeff8 = 0x5fffef60
Unassigned mem write 000000000fefeffc = 0x101
Unassigned mem write 000000000fefeffe = 0x60
Unassigned mem write 000000000fefff00 = 0x426f6f74
Unassigned mem write 000000000fefff04 = 0x20476f73
Unassigned mem write 000000000fefff08 = 0x73616d65
Unassigned mem write 000000000fefff0c = 0x7220302e
Unassigned mem write 000000000feff184 = 0x6806e6f4
Unassigned mem write 000000000feff194 = 0x68fff000
Unassigned mem write 000000000feff19c = 0x68080000
Unassigned mem read 000000000fefe648
Unassigned mem write 000000000feff15c = 0x0
Unassigned mem read 000000000fefe640
Unassigned mem read 000000000fefe640
Unassigned mem read 000000000fefec10
Unassigned mem write 000000000fefef60 = 0x80301
Unassigned mem read 000000000fefe64c
Unassigned mem write 000000000fefef90 = 0x0
Unassigned mem write 000000000fefef8c = 0x0
Unassigned mem write 000000000fefef88 = 0x0
Unassigned mem write 000000000fefef84 = 0x0
Unassigned mem write 000000000fefef80 = 0x0
Unassigned mem write 000000000fefef7c = 0x0
Unassigned mem write 000000000fefef78 = 0x0
Unassigned mem write 000000000fefef74 = 0x0
Unassigned mem write 000000000fefef70 = 0x0
Trying to write invalid spr 0 (0x000) at fff10d38
Unassigned mem read 000000000fefe64c
Unassigned mem write 000000000fefe37c = 0x44e0
Unassigned mem read 000000000fefe37c
invalid/unsupported opcode: 00 - 00 - 00 - 00 (00000000) 00000000 0
Unassigned mem read 000000000fefe37c
Unassigned mem read 000000000fefe37c
Unassigned mem read 000000000fefe37c
Unassigned mem read 000000000fefe37c
Unassigned mem read 000000000fefe37c

Then hangs here. Any idea what should be at 0xf3000000 and how that should 
end up there and why it pokes 0x0fexxxxx (is that where it thinks the 
frame buffer or some other devices should be because of something not read 
correctly from 0xf3000000 area? Should that be the PCI devices but those 
are expected to be at 0xfe000000 not 0x0fe00000. I stop here now waiting 
for contribution to fix this up, this needs more knowledge about the 
hardware that I have or want to learn at the moment.

Regards,
BALATON Zoltan

Re: [PATCH v5 09/11] macio: Add dummy screamer register area

[BALATON Zoltan]

On Sun, 28 Jun 2020, BALATON Zoltan wrote:
> SCREAMER: ########### AUDIO WRITE! 0 / 11104 - 11104
> Unassigned mem write 00000000f3011190 = 0x9000000
> Unassigned mem write 00000000f3011190 = 0xb000000
> Unassigned mem write 00000000f3011190 = 0x9000000
> Unassigned mem write 00000000f3011190 = 0x9000000
> Unassigned mem write 00000000f3011190 = 0xb000000
> Unassigned mem write 00000000f3011190 = 0x9000000
> Unassigned mem write 00000000f3011190 = 0x1000000
> Unassigned mem write 00000000f3011190 = 0x3000000
> Unassigned mem write 00000000f3011190 = 0x1000000
> Unassigned mem write 00000000f3011190 = 0x1000000
> Unassigned mem write 00000000f3011190 = 0x3000000
> Unassigned mem write 00000000f3011190 = 0x1000000
> Unassigned mem write 00000000f3011190 = 0x1000000
> Unassigned mem write 00000000f3011190 = 0x3000000
> Unassigned mem write 00000000f3011190 = 0x1000000
> Unassigned mem write 00000000f3011190 = 0x9000000
> Unassigned mem write 00000000f3011190 = 0xb000000
> Unassigned mem write 00000000f3011190 = 0x9000000
> Unassigned mem write 00000000f3011190 = 0x9000000
> Unassigned mem write 00000000f3011190 = 0xb000000
> Unassigned mem write 00000000f3011190 = 0x9000000
> Unassigned mem write 00000000f3011190 = 0x1000000
> Unassigned mem write 00000000f3011190 = 0x3000000
> Unassigned mem write 00000000f3011190 = 0x1000000
> Unassigned mem write 00000000f3011190 = 0x1000000
> Unassigned mem write 00000000f3011190 = 0x3000000
> Unassigned mem write 00000000f3011190 = 0x1000000
> Unassigned mem write 00000000f3011190 = 0x3000000
> Unassigned mem read 00000000f3011190
> Unassigned mem write 00000000f3011190 = 0x1000000
> Unassigned mem write 00000000f3011190 = 0x3000000
> Unassigned mem read 00000000f3011190
> Unassigned mem write 00000000f3011190 = 0x1000000
> Unassigned mem write 00000000f3011190 = 0x3000000
> Unassigned mem read 00000000f3011190
> Unassigned mem write 00000000f3011190 = 0x1000000
> Unassigned mem write 00000000f3011190 = 0x3000000
> Unassigned mem read 00000000f3011190
> Unassigned mem write 00000000f3011190 = 0x1000000
> Unassigned mem write 00000000f3011190 = 0x3000000
> Unassigned mem read 00000000f3011190
> [bunch of more accesses to this address...]
> Unassigned mem read 00000000f3011170
> Unassigned mem read 00000000f3011170

According to 
https://github.com/dingusdev/dingusppc/blob/master/devices/macio.h this is 
probably the ethernet port, may want to map some unimplemented device 
there to log access, otherwise it probably OK ans could do without it for 
now.

> Unassigned mem read 000000000fefe7b0
> Unassigned mem write 000000000feffffc = 0x0
> Unassigned mem write 000000000feffff8 = 0x0

Still not sure about this one I'd expect it to write to frame buffer at 
this point but probably it's accessing wrong area due to something 
nonsense returned by some earlier read of some register and it did not 
seem to talk to PCI devices to set up the BARs earlier so either we're 
missing some register writes that are silently discarded in some 
partially emulated device or it expects things to be already set up some 
way at startup.

Regards,
BALATON Zoltan

[PATCH v5 08/11] 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>
---
v4: Move MermoryRegion to MachineState, use constants

 hw/ppc/mac.h          |  1 +
 hw/ppc/mac_oldworld.c | 24 ++++++++++++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
index 79ccf8775d..32b7928a96 100644
--- a/hw/ppc/mac.h
+++ b/hw/ppc/mac.h
@@ -64,6 +64,7 @@ typedef struct HeathrowMachineState {
     /*< private >*/
     MachineState parent;
 
+    MemoryRegion machine_id;
     PCIDevice *macio;
 } HeathrowMachineState;
 
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 13562e26e6..14a191ff88 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -52,6 +52,9 @@
 
 #define MAX_IDE_BUS 2
 #define CFG_ADDR 0xf0000510
+#define MACHINE_ID_ADDR 0xff000004
+#define MACHINE_ID_VAL 0x3d8c
+
 #define TBFREQ 16600000UL
 #define CLOCKFREQ 266000000UL
 #define BUSFREQ 66000000UL
@@ -89,6 +92,22 @@ static void ppc_heathrow_cpu_reset(void *opaque)
     cpu_reset(CPU(cpu));
 }
 
+static uint64_t machine_id_read(void *opaque, hwaddr addr, unsigned size)
+{
+    return (addr == 0 && size == 2 ? MACHINE_ID_VAL : 0);
+}
+
+static void machine_id_write(void *opaque, hwaddr addr,
+                             uint64_t val, unsigned size)
+{
+    return;
+}
+
+const MemoryRegionOps machine_id_reg_ops = {
+    .read = machine_id_read,
+    .write = machine_id_write,
+};
+
 static void ppc_heathrow_init(MachineState *machine)
 {
     HeathrowMachineState *hm = HEATHROW_MACHINE(machine);
@@ -239,6 +258,11 @@ static void ppc_heathrow_init(MachineState *machine)
         }
     }
 
+    memory_region_init_io(&hm->machine_id, OBJECT(machine),
+                          &machine_id_reg_ops, NULL, "machine_id", 2);
+    memory_region_add_subregion(get_system_memory(), MACHINE_ID_ADDR,
+                                &hm->machine_id);
+
     /* XXX: we register only 1 output pin for heathrow PIC */
     pic_dev = qdev_new(TYPE_HEATHROW);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(pic_dev), &error_fatal);
-- 
2.21.3

Re: [PATCH v5 08/11] mac_oldworld: Add machine ID register

[Mark Cave-Ayland]

On 16/06/2020 14:47, BALATON Zoltan wrote:

> 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>
> ---
> v4: Move MermoryRegion to MachineState, use constants
> 
>  hw/ppc/mac.h          |  1 +
>  hw/ppc/mac_oldworld.c | 24 ++++++++++++++++++++++++
>  2 files changed, 25 insertions(+)
> 
> diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
> index 79ccf8775d..32b7928a96 100644
> --- a/hw/ppc/mac.h
> +++ b/hw/ppc/mac.h
> @@ -64,6 +64,7 @@ typedef struct HeathrowMachineState {
>      /*< private >*/
>      MachineState parent;
>  
> +    MemoryRegion machine_id;
>      PCIDevice *macio;
>  } HeathrowMachineState;
>  
> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
> index 13562e26e6..14a191ff88 100644
> --- a/hw/ppc/mac_oldworld.c
> +++ b/hw/ppc/mac_oldworld.c
> @@ -52,6 +52,9 @@
>  
>  #define MAX_IDE_BUS 2
>  #define CFG_ADDR 0xf0000510
> +#define MACHINE_ID_ADDR 0xff000004
> +#define MACHINE_ID_VAL 0x3d8c
> +
>  #define TBFREQ 16600000UL
>  #define CLOCKFREQ 266000000UL
>  #define BUSFREQ 66000000UL
> @@ -89,6 +92,22 @@ static void ppc_heathrow_cpu_reset(void *opaque)
>      cpu_reset(CPU(cpu));
>  }
>  
> +static uint64_t machine_id_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    return (addr == 0 && size == 2 ? MACHINE_ID_VAL : 0);
> +}
> +
> +static void machine_id_write(void *opaque, hwaddr addr,
> +                             uint64_t val, unsigned size)
> +{
> +    return;
> +}
> +
> +const MemoryRegionOps machine_id_reg_ops = {
> +    .read = machine_id_read,
> +    .write = machine_id_write,
> +};

static const here?

>  static void ppc_heathrow_init(MachineState *machine)
>  {
>      HeathrowMachineState *hm = HEATHROW_MACHINE(machine);
> @@ -239,6 +258,11 @@ static void ppc_heathrow_init(MachineState *machine)
>          }
>      }
>  
> +    memory_region_init_io(&hm->machine_id, OBJECT(machine),
> +                          &machine_id_reg_ops, NULL, "machine_id", 2);
> +    memory_region_add_subregion(get_system_memory(), MACHINE_ID_ADDR,
> +                                &hm->machine_id);
> +
>      /* XXX: we register only 1 output pin for heathrow PIC */
>      pic_dev = qdev_new(TYPE_HEATHROW);
>      sysbus_realize_and_unref(SYS_BUS_DEVICE(pic_dev), &error_fatal);

Have you done some boot tests with MacOS 9 and X to ensure that it doesn't cause any
regressions there?


ATB,

Mark.

[PATCH v5 10/11] WIP macio/cuda: Attempt to add i2c support

[BALATON Zoltan]

This is a non-working RFC patch attempt to implement i2c bus in CUDA
needed for firmware to access SPD data of installed RAM. The skeleton
is there but actual operation fails because I don't know how this is
supposed to work and the i2c bus state becomes invalid quickly. Also
sending back results may be missing or wrong. Help fixing and
finishing this is welcome, I don't plan to spend more time with this
so just submitted it for whoever picks this up.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/misc/macio/cuda.c         | 62 +++++++++++++++++++++++++++++++++++-
 include/hw/misc/macio/cuda.h |  1 +
 2 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
index 47aa3b0552..cfe4713527 100644
--- a/hw/misc/macio/cuda.c
+++ b/hw/misc/macio/cuda.c
@@ -28,6 +28,7 @@
 #include "hw/ppc/mac.h"
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
+#include "hw/i2c/i2c.h"
 #include "hw/input/adb.h"
 #include "hw/misc/mos6522.h"
 #include "hw/misc/macio/cuda.h"
@@ -371,6 +372,61 @@ static bool cuda_cmd_set_time(CUDAState *s,
     return true;
 }
 
+static bool cuda_cmd_get_set_iic(CUDAState *s,
+                                 const uint8_t *in_data, int in_len,
+                                 uint8_t *out_data, int *out_len)
+{
+    int i;
+
+    qemu_log_mask(LOG_UNIMP, "CUDA: unimplemented GET_SET_IIC %s 0x%x %d\n",
+                  (in_data[0] & 1 ? "read" : "write"), in_data[0] >> 1,
+                  in_len);
+    if (i2c_start_transfer(s->i2c_bus, in_data[0] >> 1, in_data[0] & 1)) {
+        return false;
+    }
+    for (i = 0; i < in_len - 3; i++) {
+        if (i2c_send(s->i2c_bus, in_data[i])) {
+            i2c_end_transfer(s->i2c_bus);
+            return false;
+        }
+    }
+    return true;
+}
+
+static bool cuda_cmd_combined_iic(CUDAState *s,
+                                  const uint8_t *in_data, int in_len,
+                                  uint8_t *out_data, int *out_len)
+{
+    int i;
+
+    if (in_len < 3) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "CUDA: COMBINED_FORMAT_IIC too few input bytes\n");
+        return false;
+    }
+    if ((in_data[0] & 0xfe) != (in_data[2] & 0xfe)) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "CUDA: COMBINED_FORMAT_IIC address mismatch\n");
+        return false;
+    }
+
+    uint8_t data = in_data[1];
+    if (i2c_start_transfer(s->i2c_bus, in_data[0] >> 1, in_data[0] & 1) ||
+        i2c_send_recv(s->i2c_bus, &data, in_data[0] & 1)) {
+        return false;
+    } else {
+        for (i = 0; i < in_len - 3; i++) {
+            data = in_data[3 + i];
+            if (i2c_send_recv(s->i2c_bus, (in_data[2] & 1 ? &out_data[i] :
+                              &data), in_data[2] & 1)) {
+                i2c_end_transfer(s->i2c_bus);
+                return false;
+            }
+        }
+    }
+    return true;
+}
+
 static const CudaCommand handlers[] = {
     { CUDA_AUTOPOLL, "AUTOPOLL", cuda_cmd_autopoll },
     { CUDA_SET_AUTO_RATE, "SET_AUTO_RATE",  cuda_cmd_set_autorate },
@@ -383,6 +439,8 @@ static const CudaCommand handlers[] = {
       cuda_cmd_set_power_message },
     { CUDA_GET_TIME, "GET_TIME", cuda_cmd_get_time },
     { CUDA_SET_TIME, "SET_TIME", cuda_cmd_set_time },
+    { CUDA_GET_SET_IIC, "GET_SET_IIC", cuda_cmd_get_set_iic },
+    { CUDA_COMBINED_FORMAT_IIC, "COMBINED_FORMAT_IIC", cuda_cmd_combined_iic },
 };
 
 static void cuda_receive_packet(CUDAState *s,
@@ -553,6 +611,7 @@ static void cuda_init(Object *obj)
 {
     CUDAState *s = CUDA(obj);
     SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+    DeviceState *dev = DEVICE(obj);
 
     object_initialize_child(obj, "mos6522-cuda", &s->mos6522_cuda,
                             TYPE_MOS6522_CUDA);
@@ -561,7 +620,8 @@ static void cuda_init(Object *obj)
     sysbus_init_mmio(sbd, &s->mem);
 
     qbus_create_inplace(&s->adb_bus, sizeof(s->adb_bus), TYPE_ADB_BUS,
-                        DEVICE(obj), "adb.0");
+                        dev, "adb.0");
+    s->i2c_bus = i2c_init_bus(dev, "i2c");
 }
 
 static Property cuda_properties[] = {
diff --git a/include/hw/misc/macio/cuda.h b/include/hw/misc/macio/cuda.h
index 5768075ac5..0c798100dc 100644
--- a/include/hw/misc/macio/cuda.h
+++ b/include/hw/misc/macio/cuda.h
@@ -79,6 +79,7 @@ typedef struct CUDAState {
 
     ADBBusState adb_bus;
     MOS6522CUDAState mos6522_cuda;
+    I2CBus *i2c_bus;
 
     uint32_t tick_offset;
     uint64_t tb_frequency;
-- 
2.21.3

[RFC PATCH] WIP macio/cuda: Attempt to add i2c support

[BALATON Zoltan]

This is a non-working RFC patch attempt to implement i2c bus in CUDA
needed for firmware to access SPD data of installed RAM. The skeleton
is there but actual operation fails because I don't know how this is
supposed to work and the i2c bus state becomes invalid quickly. Also
sending back results may be missing or wrong. Help fixing and
finishing this is welcome, I don't plan to spend more time with this
so just submitted it for whoever picks this up.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
This is still RFC and only for testing but with this the ROM seems to 
detect some RAM now:

cuda_packet_receive length 5
cuda_packet_receive_data [0] 0x01
cuda_packet_receive_data [1] 0x25
cuda_packet_receive_data [2] 0xa0
cuda_packet_receive_data [3] 0x02
cuda_packet_receive_data [4] 0xa1
cuda_receive_packet_cmd handling command COMBINED_FORMAT_IIC
i2c_event start(addr:0x50)
smbus(50): Incoming data
i2c_send send(addr:0x50) data:0x02
smbus(50): Write data 02
i2c_event finish(addr:0x50)
smbus(50): Command 2 len 1
eeprom_write_byte: addr=0x50 cmd=0x02 val=0x00
i2c_event start(addr:0x50)
smbus(50): Read mode
eeprom_receive_byte: addr=0x50 val=0x04
smbus(50): Read data 04
i2c_recv recv(addr:0x50) data:0x04
eeprom_receive_byte: addr=0x50 val=0x0d
smbus(50): Read data 0d
i2c_recv recv(addr:0x50) data:0x0d
eeprom_receive_byte: addr=0x50 val=0x0a
smbus(50): Read data 0a
i2c_recv recv(addr:0x50) data:0x0a
eeprom_receive_byte: addr=0x50 val=0x02
smbus(50): Read data 02
i2c_recv recv(addr:0x50) data:0x02
eeprom_receive_byte: addr=0x50 val=0x40
smbus(50): Read data 40
i2c_recv recv(addr:0x50) data:0x40
i2c_event finish(addr:0x50)
smbus(50): Quick Command 1
cuda_packet_send length 8
cuda_packet_send_data [0] 0x01
cuda_packet_send_data [1] 0x00
cuda_packet_send_data [2] 0x25
cuda_packet_send_data [3] 0x04
cuda_packet_send_data [4] 0x0d
cuda_packet_send_data [5] 0x0a
cuda_packet_send_data [6] 0x02
cuda_packet_send_data [7] 0x40
cuda_delay_set_sr_int
cuda_data_recv recv: 0x01
cuda_delay_set_sr_int
cuda_data_recv recv: 0x00
cuda_delay_set_sr_int
cuda_data_recv recv: 0x25
cuda_delay_set_sr_int
cuda_data_recv recv: 0x04
cuda_delay_set_sr_int
cuda_data_recv recv: 0x0d
cuda_delay_set_sr_int
cuda_data_recv recv: 0x0a
cuda_delay_set_sr_int
cuda_data_recv recv: 0x02
cuda_delay_set_sr_int
cuda_data_recv recv: 0x40
cuda_delay_set_sr_int
cuda_delay_set_sr_int
cuda_delay_set_sr_int
cuda_delay_set_sr_int
cuda_data_send send: 0x01
cuda_delay_set_sr_int
cuda_data_send send: 0x25
cuda_delay_set_sr_int
cuda_data_send send: 0xa2
cuda_delay_set_sr_int
cuda_data_send send: 0x02
cuda_delay_set_sr_int
cuda_data_send send: 0xa3
cuda_delay_set_sr_int
cuda_delay_set_sr_int
cuda_packet_receive length 5
cuda_packet_receive_data [0] 0x01
cuda_packet_receive_data [1] 0x25
cuda_packet_receive_data [2] 0xa2
cuda_packet_receive_data [3] 0x02
cuda_packet_receive_data [4] 0xa3
cuda_receive_packet_cmd handling command COMBINED_FORMAT_IIC
CUDA: COMBINED_FORMAT_IIC: wrong parameters 4
[...]
pci_cfg_write grackle 00:0 @0x80 <- 0xffff8000
pci_cfg_write grackle 00:0 @0x88 <- 0xffff0000
pci_cfg_write grackle 00:0 @0x90 <- 0xffffff7f
pci_cfg_write grackle 00:0 @0x98 <- 0xffff0000

^^^ these were all 0xffffffff before

pci_cfg_write grackle 00:0 @0x84 <- 0xffffffff
pci_cfg_write grackle 00:0 @0x8c <- 0xffffffff
pci_cfg_write grackle 00:0 @0x94 <- 0xffffffff
pci_cfg_write grackle 00:0 @0x9c <- 0xffffffff
pci_cfg_write grackle 00:0 @0xa0 <- 0x3
pci_cfg_read grackle 00:0 @0xf0 -> 0x12900000
pci_cfg_write grackle 00:0 @0xf0 <- 0x12900005
pci_cfg_read grackle 00:0 @0x8 -> 0x6000140
pci_cfg_read grackle 00:0 @0xf0 -> 0x12900005
pci_cfg_write grackle 00:0 @0xf0 <- 0x12940005
pci_cfg_write grackle 00:0 @0xf0 <- 0x12940005
pci_cfg_write grackle 00:0 @0xf4 <- 0x40010fe4
pci_cfg_write grackle 00:0 @0xf8 <- 0x7302293
pci_cfg_write grackle 00:0 @0xfc <- 0x25302220
pci_cfg_read grackle 00:0 @0xa0 -> 0x3
pci_cfg_write grackle 00:0 @0xa0 <- 0x67000003
pci_cfg_read grackle 00:0 @0xf0 -> 0x12940005
pci_cfg_write grackle 00:0 @0xf0 <- 0x129c0005

In my understanding after an I2C command CUDA should enter in a mode 
whereby reading the SR reg will return bytes from the I2C device but not 
sure what terminates that mode and how to model it correctly so I just 
return the expected number of bytes in this patch to make the ROM go 
further so I can test what else is needed. Then it crashes in screamer.

  hw/misc/macio/cuda.c         | 76 +++++++++++++++++++++++++++++++++++-
  include/hw/misc/macio/cuda.h |  1 +
  2 files changed, 76 insertions(+), 1 deletion(-)

diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
index 5bbc7770fa..3fc9773717 100644
--- a/hw/misc/macio/cuda.c
+++ b/hw/misc/macio/cuda.c
@@ -28,6 +28,7 @@
  #include "hw/ppc/mac.h"
  #include "hw/qdev-properties.h"
  #include "migration/vmstate.h"
+#include "hw/i2c/i2c.h"
  #include "hw/input/adb.h"
  #include "hw/misc/mos6522.h"
  #include "hw/misc/macio/cuda.h"
@@ -370,6 +371,75 @@ static bool cuda_cmd_set_time(CUDAState *s,
      return true;
  }

+static bool cuda_cmd_get_set_iic(CUDAState *s,
+                                 const uint8_t *in_data, int in_len,
+                                 uint8_t *out_data, int *out_len)
+{
+    int i;
+
+    qemu_log_mask(LOG_UNIMP, "CUDA: unimplemented GET_SET_IIC %s 0x%x %d\n",
+                  (in_data[0] & 1 ? "read" : "write"), in_data[0] >> 1,
+                  in_len);
+    if (i2c_start_transfer(s->i2c_bus, in_data[0] >> 1, in_data[0] & 1)) {
+        return false;
+    }
+    for (i = 0; i < in_len - 3; i++) {
+        if (i2c_send(s->i2c_bus, in_data[i])) {
+            i2c_end_transfer(s->i2c_bus);
+            return false;
+        }
+    }
+    return true;
+}
+
+static bool cuda_cmd_combined_iic(CUDAState *s,
+                                  const uint8_t *in_data, int in_len,
+                                  uint8_t *out_data, int *out_len)
+{
+    int i;
+
+    if (in_len < 3) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "CUDA: COMBINED_FORMAT_IIC too few input bytes\n");
+        return false;
+    }
+    if ((in_data[0] & 0xfe) != (in_data[2] & 0xfe)) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "CUDA: COMBINED_FORMAT_IIC address mismatch\n");
+        return false;
+    }
+
+    uint8_t data = in_data[1];
+    if (i2c_start_transfer(s->i2c_bus, in_data[0] >> 1, in_data[0] & 1) ||
+        i2c_send_recv(s->i2c_bus, &data, in_data[0] & 1)) {
+        return false;
+    }
+    i2c_end_transfer(s->i2c_bus);
+    if (in_data[2] & 1) {
+        if (i2c_start_transfer(s->i2c_bus, in_data[2] >> 1, in_data[2] & 1)) {
+            i2c_end_transfer(s->i2c_bus);
+            return false;
+        }
+        for (i = 0; i < 5; i++) {
+            if (i2c_send_recv(s->i2c_bus, &out_data[i], in_data[2] & 1)) {
+                i2c_end_transfer(s->i2c_bus);
+                return false;
+            }
+        }
+        *out_len = i;
+        i2c_end_transfer(s->i2c_bus);
+    } else {
+        for (i = 0; i < in_len - 3; i++) {
+            data = in_data[3 + i];
+            if (i2c_send_recv(s->i2c_bus, &data, in_data[2] & 1)) {
+                i2c_end_transfer(s->i2c_bus);
+                return false;
+            }
+        }
+    }
+    return true;
+}
+
  static const CudaCommand handlers[] = {
      { CUDA_AUTOPOLL, "AUTOPOLL", cuda_cmd_autopoll },
      { CUDA_SET_AUTO_RATE, "SET_AUTO_RATE",  cuda_cmd_set_autorate },
@@ -382,6 +452,8 @@ static const CudaCommand handlers[] = {
        cuda_cmd_set_power_message },
      { CUDA_GET_TIME, "GET_TIME", cuda_cmd_get_time },
      { CUDA_SET_TIME, "SET_TIME", cuda_cmd_set_time },
+    { CUDA_GET_SET_IIC, "GET_SET_IIC", cuda_cmd_get_set_iic },
+    { CUDA_COMBINED_FORMAT_IIC, "COMBINED_FORMAT_IIC", cuda_cmd_combined_iic },
  };

  static void cuda_receive_packet(CUDAState *s,
@@ -549,6 +621,7 @@ static void cuda_init(Object *obj)
  {
      CUDAState *s = CUDA(obj);
      SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+    DeviceState *dev = DEVICE(obj);

      object_initialize_child(obj, "mos6522-cuda", &s->mos6522_cuda,
                              TYPE_MOS6522_CUDA);
@@ -557,7 +630,8 @@ static void cuda_init(Object *obj)
      sysbus_init_mmio(sbd, &s->mem);

      qbus_create_inplace(&s->adb_bus, sizeof(s->adb_bus), TYPE_ADB_BUS,
-                        DEVICE(obj), "adb.0");
+                        dev, "adb.0");
+    s->i2c_bus = i2c_init_bus(dev, "i2c");
  }

  static Property cuda_properties[] = {
diff --git a/include/hw/misc/macio/cuda.h b/include/hw/misc/macio/cuda.h
index a8cf0be1ec..6856ed7704 100644
--- a/include/hw/misc/macio/cuda.h
+++ b/include/hw/misc/macio/cuda.h
@@ -79,6 +79,7 @@ typedef struct CUDAState {

      ADBBusState adb_bus;
      MOS6522CUDAState mos6522_cuda;
+    I2CBus *i2c_bus;

      uint32_t tick_offset;
      uint64_t tb_frequency;
-- 
2.21.3

[PATCH v5 06/11] mac_oldworld: Rename ppc_heathrow_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>
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 4200008851..f97f241e0c 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;
 
@@ -112,7 +112,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

Re: [PATCH v5 06/11] mac_oldworld: Rename ppc_heathrow_reset to ppc_heathrow_cpu_reset

[Mark Cave-Ayland]

On 16/06/2020 14:47, 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 4200008851..f97f241e0c 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;
>  
> @@ -112,7 +112,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 */

As per my previous comment on your earlier version, I don't agree with this - the
reset is being registered at board level, it just so happens that as it's only
touching the CPU due to the opaque being passed in.

I'd be inclined to pass in a suitable HeathrowMachineState object containing a
reference to the CPU instead.


ATB,

Mark.

Re: [PATCH v5 06/11] mac_oldworld: Rename ppc_heathrow_reset to ppc_heathrow_cpu_reset

[BALATON Zoltan]

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

On Fri, 26 Jun 2020, Mark Cave-Ayland wrote:
> On 16/06/2020 14:47, 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 4200008851..f97f241e0c 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;
>>
>> @@ -112,7 +112,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 */
>
> As per my previous comment on your earlier version, I don't agree with this - the
> reset is being registered at board level, it just so happens that as it's only
> touching the CPU due to the opaque being passed in.
>
> I'd be inclined to pass in a suitable HeathrowMachineState object containing a
> reference to the CPU instead.

It's not a board level reset func but a CPU level one. See where it's 
registered here:

https://git.qemu.org/?p=qemu.git;a=blob;f=hw/ppc/mac_oldworld.c;h=f8c204ead73843098084bf5213ac4046d7d843c4;hb=HEAD#l111

One for each CPU and as there could be more than one CPU, this won't work 
with a single reference in HeathrowMachineState. We could reset CPUs in a 
board level reset func (added by next patch) but I don't know how to 
access CPU objects from MachineState (it did not look trivial when I've 
looked) so I just left it as it is for later clean up separate from this 
series. I've just renamed it to avoid confusion with board level reset 
func which is usually named *_reset but I could call that 
ppc_heathrow_board_reset and then we don't need this patch but I think 
this is cleaner.

I don't even know why we need a reset func to reset the CPU, I'd expect 
that to be the default behaviour of any board reset without needing to 
register a func to do it.

Regards,
BALATON Zoltan

[PATCH v5 07/11] 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 | 15 ++++++++++++++-
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
index a0d9e47031..79ccf8775d 100644
--- a/hw/ppc/mac.h
+++ b/hw/ppc/mac.h
@@ -55,6 +55,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;
+} 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 f97f241e0c..13562e26e6 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_COMMAND, PCI_COMMAND_MEMORY, 2);
+    pci_default_write_config(m->macio, PCI_BASE_ADDRESS_0, 0xf3000000, 4);
+}
+
 static void ppc_heathrow_cpu_reset(void *opaque)
 {
     PowerPCCPU *cpu = opaque;
@@ -82,6 +91,7 @@ static void ppc_heathrow_cpu_reset(void *opaque)
 
 static void ppc_heathrow_init(MachineState *machine)
 {
+    HeathrowMachineState *hm = HEATHROW_MACHINE(machine);
     ram_addr_t ram_size = machine->ram_size;
     const char *boot_device = machine->boot_order;
     PowerPCCPU *cpu = NULL;
@@ -287,6 +297,7 @@ static void ppc_heathrow_init(MachineState *machine)
 
     /* MacIO */
     macio = pci_new(-1, TYPE_OLDWORLD_MACIO);
+    hm->macio = macio;
     dev = DEVICE(macio);
     qdev_prop_set_uint64(dev, "frequency", tbfreq);
     object_property_set_link(OBJECT(macio), OBJECT(pic_dev), "pic",
@@ -439,6 +450,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
@@ -455,9 +467,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

Re: [PATCH v5 07/11] mac_oldworld: Map macio to expected address at reset

[Mark Cave-Ayland]

On 16/06/2020 14:47, 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 | 15 ++++++++++++++-
>  2 files changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
> index a0d9e47031..79ccf8775d 100644
> --- a/hw/ppc/mac.h
> +++ b/hw/ppc/mac.h
> @@ -55,6 +55,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;
> +} 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 f97f241e0c..13562e26e6 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_COMMAND, PCI_COMMAND_MEMORY, 2);
> +    pci_default_write_config(m->macio, PCI_BASE_ADDRESS_0, 0xf3000000, 4);
> +}

As per my comment on a previous version, this doesn't feel right at all - it's either
mapped at a fixed address (in which case it should be done in the macio device,
probably via a property), or the BIOS should be programming the BAR accordingly.


ATB,

Mark.

Re: [PATCH v5 07/11] mac_oldworld: Map macio to expected address at reset

[BALATON Zoltan]

On Fri, 26 Jun 2020, Mark Cave-Ayland wrote:
> On 16/06/2020 14:47, 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 | 15 ++++++++++++++-
>>  2 files changed, 26 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
>> index a0d9e47031..79ccf8775d 100644
>> --- a/hw/ppc/mac.h
>> +++ b/hw/ppc/mac.h
>> @@ -55,6 +55,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;
>> +} 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 f97f241e0c..13562e26e6 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_COMMAND, PCI_COMMAND_MEMORY, 2);
>> +    pci_default_write_config(m->macio, PCI_BASE_ADDRESS_0, 0xf3000000, 4);
>> +}
>
> As per my comment on a previous version, this doesn't feel right at all - it's either
> mapped at a fixed address (in which case it should be done in the macio device,
> probably via a property), or the BIOS should be programming the BAR accordingly.

The ROM does not seem to do anything to map the BAR before it accesses it. 
We also can't map it in macio's realize because on reset the pci-host will 
clear bars of attached devices so the only way (I know) to do this is from 
a board level reset func. But if you have a simpler way please send 
alternative patch, for me this was the only way I could make it work.

Regards,
BALATON Zoltan

[PATCH v5 02/11] mac_newworld: Allow loading binary ROM image

[BALATON Zoltan]

Fall back to load binary ROM image if loading ELF fails. This also
moves PROM_BASE and PROM_SIZE defines to board as these are matching
the ROM size and address on this board.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
Notes:
    Unlike mac_oldworld where the openbios-ppc image loads at end of ROM
    region here we only check size and assume ELF image is loaded from
    PROM_BASE, Checking the load addr here is tricky because this board is
    also be compiled both 64 and 32 bit and load_elf seems to always
    return 64 bit value so handling that could become a mess. If this is a
    problem then it's a preexisting one so should be fixed in a separate
    patch. This one just allows loading ROM binary too otherwise
    preserving previous behaviour.

 hw/ppc/mac.h          |  2 --
 hw/ppc/mac_newworld.c | 22 ++++++++++++++--------
 2 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
index 6af87d1fa0..a0d9e47031 100644
--- a/hw/ppc/mac.h
+++ b/hw/ppc/mac.h
@@ -38,10 +38,8 @@
 /* SMP is not enabled, for now */
 #define MAX_CPUS 1
 
-#define BIOS_SIZE        (1 * MiB)
 #define NVRAM_SIZE        0x2000
 #define PROM_FILENAME    "openbios-ppc"
-#define PROM_ADDR         0xfff00000
 
 #define KERNEL_LOAD_ADDR 0x01000000
 #define KERNEL_GAP       0x00100000
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 5f3a028e6a..eec62d1e90 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -82,6 +82,8 @@
 
 #define NDRV_VGA_FILENAME "qemu_vga.ndrv"
 
+#define PROM_BASE 0xfff00000
+#define PROM_SIZE (1 * MiB)
 
 static void fw_cfg_boot_set(void *opaque, const char *boot_device,
                             Error **errp)
@@ -100,7 +102,7 @@ static void ppc_core99_reset(void *opaque)
 
     cpu_reset(CPU(cpu));
     /* 970 CPUs want to get their initial IP as part of their boot protocol */
-    cpu->env.nip = PROM_ADDR + 0x100;
+    cpu->env.nip = PROM_BASE + 0x100;
 }
 
 /* PowerPC Mac99 hardware initialisation */
@@ -153,25 +155,29 @@ static void ppc_core99_init(MachineState *machine)
     /* allocate RAM */
     memory_region_add_subregion(get_system_memory(), 0, machine->ram);
 
-    /* allocate and load BIOS */
-    memory_region_init_rom(bios, NULL, "ppc_core99.bios", BIOS_SIZE,
+    /* allocate and load firmware ROM */
+    memory_region_init_rom(bios, NULL, "ppc_core99.bios", PROM_SIZE,
                            &error_fatal);
+    memory_region_add_subregion(get_system_memory(), 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(get_system_memory(), PROM_ADDR, bios);
-
-    /* Load OpenBIOS (ELF) */
     if (filename) {
+        /* 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

Re: [PATCH v5 02/11] mac_newworld: Allow loading binary ROM image

[Mark Cave-Ayland]

On 16/06/2020 14:47, BALATON Zoltan wrote:

> Fall back to load binary ROM image if loading ELF fails. This also
> moves PROM_BASE and PROM_SIZE defines to board as these are matching
> the ROM size and address on this board.

Probably worth a quick mention here that you are removing BIOS_SIZE and PROM_ADDR
from the common Mac code.

> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> Notes:
>     Unlike mac_oldworld where the openbios-ppc image loads at end of ROM
>     region here we only check size and assume ELF image is loaded from
>     PROM_BASE, Checking the load addr here is tricky because this board is
>     also be compiled both 64 and 32 bit and load_elf seems to always
>     return 64 bit value so handling that could become a mess. If this is a
>     problem then it's a preexisting one so should be fixed in a separate
>     patch. This one just allows loading ROM binary too otherwise
>     preserving previous behaviour.
> 
>  hw/ppc/mac.h          |  2 --
>  hw/ppc/mac_newworld.c | 22 ++++++++++++++--------
>  2 files changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
> index 6af87d1fa0..a0d9e47031 100644
> --- a/hw/ppc/mac.h
> +++ b/hw/ppc/mac.h
> @@ -38,10 +38,8 @@
>  /* SMP is not enabled, for now */
>  #define MAX_CPUS 1
>  
> -#define BIOS_SIZE        (1 * MiB)
>  #define NVRAM_SIZE        0x2000
>  #define PROM_FILENAME    "openbios-ppc"
> -#define PROM_ADDR         0xfff00000
>  
>  #define KERNEL_LOAD_ADDR 0x01000000
>  #define KERNEL_GAP       0x00100000
> diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
> index 5f3a028e6a..eec62d1e90 100644
> --- a/hw/ppc/mac_newworld.c
> +++ b/hw/ppc/mac_newworld.c
> @@ -82,6 +82,8 @@
>  
>  #define NDRV_VGA_FILENAME "qemu_vga.ndrv"
>  
> +#define PROM_BASE 0xfff00000
> +#define PROM_SIZE (1 * MiB)
>  
>  static void fw_cfg_boot_set(void *opaque, const char *boot_device,
>                              Error **errp)
> @@ -100,7 +102,7 @@ static void ppc_core99_reset(void *opaque)
>  
>      cpu_reset(CPU(cpu));
>      /* 970 CPUs want to get their initial IP as part of their boot protocol */
> -    cpu->env.nip = PROM_ADDR + 0x100;
> +    cpu->env.nip = PROM_BASE + 0x100;
>  }
>  
>  /* PowerPC Mac99 hardware initialisation */
> @@ -153,25 +155,29 @@ static void ppc_core99_init(MachineState *machine)
>      /* allocate RAM */
>      memory_region_add_subregion(get_system_memory(), 0, machine->ram);
>  
> -    /* allocate and load BIOS */
> -    memory_region_init_rom(bios, NULL, "ppc_core99.bios", BIOS_SIZE,
> +    /* allocate and load firmware ROM */
> +    memory_region_init_rom(bios, NULL, "ppc_core99.bios", PROM_SIZE,
>                             &error_fatal);
> +    memory_region_add_subregion(get_system_memory(), 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(get_system_memory(), PROM_ADDR, bios);
> -
> -    /* Load OpenBIOS (ELF) */
>      if (filename) {
> +        /* 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);
>      }

Same as with the previous patch: presumably if you fix the issue with load_elf() then
this should work fine as it does with SPARC.


ATB,

Mark.

[PATCH v5 11/11] mac_oldworld: Add SPD data to cover RAM

[BALATON Zoltan]

OpenBIOS gets RAM size via fw_cfg but rhe original board firmware
detects RAM using SPD data so generate and add SDP eeproms to cover as
much RAM as possible to describe with SPD (this may be less than the
actual ram_size due to SDRAM size constraints).

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

diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 14a191ff88..fcc0d6d933 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -34,6 +34,7 @@
 #include "hw/input/adb.h"
 #include "sysemu/sysemu.h"
 #include "net/net.h"
+#include "hw/i2c/smbus_eeprom.h"
 #include "hw/isa/isa.h"
 #include "hw/pci/pci.h"
 #include "hw/pci/pci_host.h"
@@ -133,6 +134,8 @@ static void ppc_heathrow_init(MachineState *machine)
     DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
     void *fw_cfg;
     uint64_t tbfreq;
+    uint8_t *spd_data[3] = {};
+    I2CBus *i2c_bus;
 
     /* init CPUs */
     for (i = 0; i < smp_cpus; i++) {
@@ -150,8 +153,16 @@ static void ppc_heathrow_init(MachineState *machine)
                      "maximum 2047 MB", ram_size / MiB);
         exit(1);
     }
-
     memory_region_add_subregion(get_system_memory(), 0, machine->ram);
+    for (i = 0; i < 3; i++) {
+        int size_left = ram_size - i * 512 * MiB;
+        if (size_left > 0) {
+            uint32_t s = size_left / MiB;
+            s = (s > 512 ? 512 : s);
+            s = 1U << (31 - clz32(s));
+            spd_data[i] = spd_data_generate(SDR, s * MiB);
+        }
+    }
 
     /* allocate and load firmware ROM */
     memory_region_init_rom(bios, NULL, "ppc_heathrow.bios", PROM_SIZE,
@@ -337,6 +348,12 @@ static void ppc_heathrow_init(MachineState *machine)
     macio_ide_init_drives(macio_ide, &hd[MAX_IDE_DEVS]);
 
     dev = DEVICE(object_resolve_path_component(OBJECT(macio), "cuda"));
+    i2c_bus = I2C_BUS(qdev_get_child_bus(dev, "i2c"));
+    for (i = 0; i < 3; i++) {
+        if (spd_data[i]) {
+            smbus_eeprom_init_one(i2c_bus, 0x50 + i, spd_data[i]);
+        }
+    }
     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.21.3

[PATCH v5 01/11] mac_oldworld: Allow loading binary ROM image

[BALATON Zoltan]

The beige G3 Power Macintosh has a 4MB firmware ROM. Fix the size of
the rom region and fall back to loading a binary image with -bios if
loading ELF image failed. This allows testing emulation with a ROM
image from real hardware as well as using an ELF OpenBIOS image.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
v4: use load address from ELF to check if ROM is too big

 hw/ppc/mac_oldworld.c | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index f8c204ead7..baf3da6f90 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)
@@ -99,6 +101,7 @@ static void ppc_heathrow_init(MachineState *machine)
     SysBusDevice *s;
     DeviceState *dev, *pic_dev;
     BusState *adb_bus;
+    uint64_t bios_addr;
     int bios_size;
     unsigned int smp_cpus = machine->smp.cpus;
     uint16_t ppc_boot_device;
@@ -127,24 +130,32 @@ 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, &bios_addr,
+                             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);
+            bios_addr = PROM_BASE;
+        } else {
+            /* load_elf sets high 32 bits for some reason, strip those */
+            bios_addr &= 0xffffffffULL;
+        }
         g_free(filename);
     } else {
         bios_size = -1;
     }
-    if (bios_size < 0 || bios_size > BIOS_SIZE) {
+    if (bios_size < 0 || bios_addr - PROM_BASE + bios_size > PROM_SIZE) {
         error_report("could not load PowerPC bios '%s'", bios_name);
         exit(1);
     }
-- 
2.21.3

Re: [PATCH v5 01/11] mac_oldworld: Allow loading binary ROM image

[Mark Cave-Ayland]

On 16/06/2020 14:47, BALATON Zoltan wrote:

> The beige G3 Power Macintosh has a 4MB firmware ROM. Fix the size of
> the rom region and fall back to loading a binary image with -bios if
> loading ELF image failed. This allows testing emulation with a ROM
> image from real hardware as well as using an ELF OpenBIOS image.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> v4: use load address from ELF to check if ROM is too big
> 
>  hw/ppc/mac_oldworld.c | 29 ++++++++++++++++++++---------
>  1 file changed, 20 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
> index f8c204ead7..baf3da6f90 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)
> @@ -99,6 +101,7 @@ static void ppc_heathrow_init(MachineState *machine)
>      SysBusDevice *s;
>      DeviceState *dev, *pic_dev;
>      BusState *adb_bus;
> +    uint64_t bios_addr;
>      int bios_size;
>      unsigned int smp_cpus = machine->smp.cpus;
>      uint16_t ppc_boot_device;
> @@ -127,24 +130,32 @@ 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, &bios_addr,
> +                             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);
> +            bios_addr = PROM_BASE;
> +        } else {
> +            /* load_elf sets high 32 bits for some reason, strip those */
> +            bios_addr &= 0xffffffffULL;

This is certainly the approach I suggested, but this seems wrong - otherwise
load_elf() would be broken for quite a few use cases.

> +        }
>          g_free(filename);
>      } else {
>          bios_size = -1;
>      }
> -    if (bios_size < 0 || bios_size > BIOS_SIZE) {
> +    if (bios_size < 0 || bios_addr - PROM_BASE + bios_size > PROM_SIZE) {
>          error_report("could not load PowerPC bios '%s'", bios_name);
>          exit(1);
>      }

(goes and looks)

This is similar to how the SPARC32 loader works and it seems fine there:
https://git.qemu.org/?p=qemu.git;a=blob;f=hw/sparc/sun4m.c;h=ee52b5cbbcd22284384225c80ad50cdbd1415743;hb=HEAD#l721.
Looks like you might have the wrong addr parameter here?


ATB,

Mark.

Re: [PATCH v5 01/11] mac_oldworld: Allow loading binary ROM image

[BALATON Zoltan]

On Fri, 26 Jun 2020, Mark Cave-Ayland wrote:
> On 16/06/2020 14:47, BALATON Zoltan wrote:
>> The beige G3 Power Macintosh has a 4MB firmware ROM. Fix the size of
>> the rom region and fall back to loading a binary image with -bios if
>> loading ELF image failed. This allows testing emulation with a ROM
>> image from real hardware as well as using an ELF OpenBIOS image.
>>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>> v4: use load address from ELF to check if ROM is too big
>>
>>  hw/ppc/mac_oldworld.c | 29 ++++++++++++++++++++---------
>>  1 file changed, 20 insertions(+), 9 deletions(-)
>>
>> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
>> index f8c204ead7..baf3da6f90 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)
>> @@ -99,6 +101,7 @@ static void ppc_heathrow_init(MachineState *machine)
>>      SysBusDevice *s;
>>      DeviceState *dev, *pic_dev;
>>      BusState *adb_bus;
>> +    uint64_t bios_addr;
>>      int bios_size;
>>      unsigned int smp_cpus = machine->smp.cpus;
>>      uint16_t ppc_boot_device;
>> @@ -127,24 +130,32 @@ 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, &bios_addr,
>> +                             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);
>> +            bios_addr = PROM_BASE;
>> +        } else {
>> +            /* load_elf sets high 32 bits for some reason, strip those */
>> +            bios_addr &= 0xffffffffULL;
>
> This is certainly the approach I suggested, but this seems wrong - otherwise
> load_elf() would be broken for quite a few use cases.
>
>> +        }
>>          g_free(filename);
>>      } else {
>>          bios_size = -1;
>>      }
>> -    if (bios_size < 0 || bios_size > BIOS_SIZE) {
>> +    if (bios_size < 0 || bios_addr - PROM_BASE + bios_size > PROM_SIZE) {
>>          error_report("could not load PowerPC bios '%s'", bios_name);
>>          exit(1);
>>      }
>
> (goes and looks)
>
> This is similar to how the SPARC32 loader works and it seems fine there:
> https://git.qemu.org/?p=qemu.git;a=blob;f=hw/sparc/sun4m.c;h=ee52b5cbbcd22284384225c80ad50cdbd1415743;hb=HEAD#l721.
> Looks like you might have the wrong addr parameter here?

I don't get this. Can you explain more please what is wrong and what's the 
proposed solution?

Regards,
BALATON Zoltan

[PATCH v5 04/11] mac_oldworld: Drop some variables

[BALATON Zoltan]

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.

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

diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index d1c4244b1e..4200008851 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -83,14 +83,11 @@ static void ppc_heathrow_reset(void *opaque)
 static void ppc_heathrow_init(MachineState *machine)
 {
     ram_addr_t ram_size = machine->ram_size;
-    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_order;
     PowerPCCPU *cpu = NULL;
     CPUPPCState *env = NULL;
     char *filename;
-    int linux_boot, i;
+    int i;
     MemoryRegion *bios = g_new(MemoryRegion, 1);
     uint32_t kernel_base, initrd_base, cmdline_base = 0;
     int32_t kernel_size, initrd_size;
@@ -108,8 +105,6 @@ static void ppc_heathrow_init(MachineState *machine)
     void *fw_cfg;
     uint64_t tbfreq;
 
-    linux_boot = (kernel_filename != NULL);
-
     /* init CPUs */
     for (i = 0; i < smp_cpus; i++) {
         cpu = POWERPC_CPU(cpu_create(machine->cpu_type));
@@ -159,7 +154,7 @@ static void ppc_heathrow_init(MachineState *machine)
         exit(1);
     }
 
-    if (linux_boot) {
+    if (machine->kernel_filename) {
         uint64_t lowaddr = 0;
         int bswap_needed;
 
@@ -169,30 +164,33 @@ static void ppc_heathrow_init(MachineState *machine)
         bswap_needed = 0;
 #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, &lowaddr, NULL, NULL, 1, PPC_ELF_MACHINE,
                                0, 0);
         if (kernel_size < 0)
-            kernel_size = load_aout(kernel_filename, kernel_base,
+            kernel_size = load_aout(machine->kernel_filename, kernel_base,
                                     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);
         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) {
-            initrd_base = TARGET_PAGE_ALIGN(kernel_base + kernel_size + KERNEL_GAP);
-            initrd_size = load_image_targphys(initrd_filename, initrd_base,
+        if (machine->initrd_filename) {
+            initrd_base = TARGET_PAGE_ALIGN(kernel_base + kernel_size +
+                                            KERNEL_GAP);
+            initrd_size = load_image_targphys(machine->initrd_filename,
+                                              initrd_base,
                                               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);
@@ -336,9 +334,10 @@ static void ppc_heathrow_init(MachineState *machine)
     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);
-    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.21.3

Re: [PATCH v5 04/11] mac_oldworld: Drop some variables

[Mark Cave-Ayland]

On 16/06/2020 14:47, BALATON Zoltan wrote:

> 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.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>  hw/ppc/mac_oldworld.c | 33 ++++++++++++++++-----------------
>  1 file changed, 16 insertions(+), 17 deletions(-)
> 
> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
> index d1c4244b1e..4200008851 100644
> --- a/hw/ppc/mac_oldworld.c
> +++ b/hw/ppc/mac_oldworld.c
> @@ -83,14 +83,11 @@ static void ppc_heathrow_reset(void *opaque)
>  static void ppc_heathrow_init(MachineState *machine)
>  {
>      ram_addr_t ram_size = machine->ram_size;
> -    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_order;
>      PowerPCCPU *cpu = NULL;
>      CPUPPCState *env = NULL;
>      char *filename;
> -    int linux_boot, i;
> +    int i;
>      MemoryRegion *bios = g_new(MemoryRegion, 1);
>      uint32_t kernel_base, initrd_base, cmdline_base = 0;
>      int32_t kernel_size, initrd_size;
> @@ -108,8 +105,6 @@ static void ppc_heathrow_init(MachineState *machine)
>      void *fw_cfg;
>      uint64_t tbfreq;
>  
> -    linux_boot = (kernel_filename != NULL);
> -
>      /* init CPUs */
>      for (i = 0; i < smp_cpus; i++) {
>          cpu = POWERPC_CPU(cpu_create(machine->cpu_type));
> @@ -159,7 +154,7 @@ static void ppc_heathrow_init(MachineState *machine)
>          exit(1);
>      }
>  
> -    if (linux_boot) {
> +    if (machine->kernel_filename) {
>          uint64_t lowaddr = 0;
>          int bswap_needed;
>  
> @@ -169,30 +164,33 @@ static void ppc_heathrow_init(MachineState *machine)
>          bswap_needed = 0;
>  #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, &lowaddr, NULL, NULL, 1, PPC_ELF_MACHINE,
>                                 0, 0);
>          if (kernel_size < 0)
> -            kernel_size = load_aout(kernel_filename, kernel_base,
> +            kernel_size = load_aout(machine->kernel_filename, kernel_base,
>                                      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);
>          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) {
> -            initrd_base = TARGET_PAGE_ALIGN(kernel_base + kernel_size + KERNEL_GAP);
> -            initrd_size = load_image_targphys(initrd_filename, initrd_base,
> +        if (machine->initrd_filename) {
> +            initrd_base = TARGET_PAGE_ALIGN(kernel_base + kernel_size +
> +                                            KERNEL_GAP);
> +            initrd_size = load_image_targphys(machine->initrd_filename,
> +                                              initrd_base,
>                                                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);
> @@ -336,9 +334,10 @@ static void ppc_heathrow_init(MachineState *machine)
>      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);
> -    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);
>      }

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


ATB,

Mark.

[PATCH v5 03/11] mac_oldworld: Drop a variable, use get_system_memory() directly

[BALATON Zoltan]

Half of the occurances already use get_system_memory() directly
instead of sysmem variable, convert the two other uses to
get_system_memory() tii which seems to be more common and drop the
variable.

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

diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index baf3da6f90..d1c4244b1e 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -87,7 +87,6 @@ static void ppc_heathrow_init(MachineState *machine)
     const char *kernel_cmdline = machine->kernel_cmdline;
     const char *initrd_filename = machine->initrd_filename;
     const char *boot_device = machine->boot_order;
-    MemoryRegion *sysmem = get_system_memory();
     PowerPCCPU *cpu = NULL;
     CPUPPCState *env = NULL;
     char *filename;
@@ -128,12 +127,12 @@ static void ppc_heathrow_init(MachineState *machine)
         exit(1);
     }
 
-    memory_region_add_subregion(sysmem, 0, machine->ram);
+    memory_region_add_subregion(get_system_memory(), 0, machine->ram);
 
     /* 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);
+    memory_region_add_subregion(get_system_memory(), PROM_BASE, bios);
 
     if (!bios_name) {
         bios_name = PROM_FILENAME;
-- 
2.21.3

Re: [PATCH v5 03/11] mac_oldworld: Drop a variable, use get_system_memory() directly

[Mark Cave-Ayland]

On 16/06/2020 14:47, BALATON Zoltan wrote:

> Half of the occurances already use get_system_memory() directly
> instead of sysmem variable, convert the two other uses to
> get_system_memory() tii which seems to be more common and drop the
> variable.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>  hw/ppc/mac_oldworld.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
> index baf3da6f90..d1c4244b1e 100644
> --- a/hw/ppc/mac_oldworld.c
> +++ b/hw/ppc/mac_oldworld.c
> @@ -87,7 +87,6 @@ static void ppc_heathrow_init(MachineState *machine)
>      const char *kernel_cmdline = machine->kernel_cmdline;
>      const char *initrd_filename = machine->initrd_filename;
>      const char *boot_device = machine->boot_order;
> -    MemoryRegion *sysmem = get_system_memory();
>      PowerPCCPU *cpu = NULL;
>      CPUPPCState *env = NULL;
>      char *filename;
> @@ -128,12 +127,12 @@ static void ppc_heathrow_init(MachineState *machine)
>          exit(1);
>      }
>  
> -    memory_region_add_subregion(sysmem, 0, machine->ram);
> +    memory_region_add_subregion(get_system_memory(), 0, machine->ram);
>  
>      /* 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);
> +    memory_region_add_subregion(get_system_memory(), PROM_BASE, bios);
>  
>      if (!bios_name) {
>          bios_name = PROM_FILENAME;

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


ATB,

Mark.

Re: [PATCH v5 00/11] Mac Old World ROM experiment

[BALATON Zoltan]

Hello Mark,

On Tue, 16 Jun 2020, BALATON Zoltan wrote:
> v5: Rebased on master, added some more clean ups, CUDA i2c is still to
> be sorted out, help with that is welcome.

What about these patches? At least those that are finished (up to patch 9) 
could be merged. I've seen you sent a pull request but not including any 
of these. Will this need another rebase after your patches? If I rebase 
them will you consider merging them? (Otherwise I won't spend time with 
it.)

Thanks,
BALATON Zoltan

> BALATON Zoltan (11):
>  mac_oldworld: Allow loading binary ROM image
>  mac_newworld: Allow loading binary ROM image
>  mac_oldworld: Drop a variable, use get_system_memory() directly
>  mac_oldworld: Drop some variables
>  grackle: Set revision in PCI config to match hardware
>  mac_oldworld: Rename ppc_heathrow_reset to ppc_heathrow_cpu_reset
>  mac_oldworld: Map macio to expected address at reset
>  mac_oldworld: Add machine ID register
>  macio: Add dummy screamer register area
>  WIP macio/cuda: Attempt to add i2c support
>  mac_oldworld: Add SPD data to cover RAM
>
> hw/misc/macio/cuda.c         |  62 ++++++++++++++++-
> hw/misc/macio/macio.c        |  34 ++++++++++
> hw/pci-host/grackle.c        |   2 +-
> hw/ppc/mac.h                 |  15 ++++-
> hw/ppc/mac_newworld.c        |  22 +++---
> hw/ppc/mac_oldworld.c        | 127 ++++++++++++++++++++++++++---------
> include/hw/misc/macio/cuda.h |   1 +
> 7 files changed, 219 insertions(+), 44 deletions(-)
>
>

Re: [PATCH v5 00/11] Mac Old World ROM experiment

[Mark Cave-Ayland]

On 26/06/2020 11:21, BALATON Zoltan wrote:

> What about these patches? At least those that are finished (up to patch 9) could be
> merged. I've seen you sent a pull request but not including any of these. Will this
> need another rebase after your patches? If I rebase them will you consider merging
> them? (Otherwise I won't spend time with it.)

Ah sorry I missed the latest version of these. I'll take a quick look now.

(BTW it seems the patches in your patchset have started appearing in a random order
when sent to the list again?)


ATB,

Mark.

Re: [PATCH v5 00/11] Mac Old World ROM experiment

[BALATON Zoltan]

On Fri, 26 Jun 2020, Mark Cave-Ayland wrote:
> On 26/06/2020 11:21, BALATON Zoltan wrote:
>> What about these patches? At least those that are finished (up to patch 9) could be
>> merged. I've seen you sent a pull request but not including any of these. Will this
>> need another rebase after your patches? If I rebase them will you consider merging
>> them? (Otherwise I won't spend time with it.)
>
> Ah sorry I missed the latest version of these. I'll take a quick look now.

In case you missed that I've just sent the latest v6 version of the series 
with changes you've suggested and I made since.

> (BTW it seems the patches in your patchset have started appearing in a random order
> when sent to the list again?)

I've noticed this too but don't know why that happens or what to do to 
prevent it. I submit these in one batch to my mail server which then seems 
to send it off simultaneously and may end up out of order. I've tried 
adding a 1 sec delay now but apparently that did not solve it. Sorry for 
the inconvenience.

Regards,
BALATON Zoltan