Re: [PATCH v3] mac_oldworld: Add machine ID register

[Mark Cave-Ayland]

On 13/06/2020 18:57, 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>
> ---
> v3: add empty write function in case anything tries to write reg
> 
> hw/ppc/mac_oldworld.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
> index 3812adc441..acaf468458 100644
> --- a/hw/ppc/mac_oldworld.c
> +++ b/hw/ppc/mac_oldworld.c
> @@ -80,6 +80,22 @@ static void ppc_heathrow_reset(void *opaque)
>      cpu_reset(CPU(cpu));
>  }
> 
> +static uint64_t machine_id_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    return (addr == 0 && size == 2 ? 0x3d8c : 0);
> +}
> +
> +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)
>  {
>      ram_addr_t ram_size = machine->ram_size;
> @@ -93,6 +109,7 @@ static void ppc_heathrow_init(MachineState *machine)
>      char *filename;
>      int linux_boot, i;
>      MemoryRegion *bios = g_new(MemoryRegion, 1);
> +    MemoryRegion *machine_id = g_new(MemoryRegion, 1);
>      uint32_t kernel_base, initrd_base, cmdline_base = 0;
>      int32_t kernel_size, initrd_size;
>      PCIBus *pci_bus;
> @@ -227,6 +244,10 @@ static void ppc_heathrow_init(MachineState *machine)
>          }
>      }
> 
> +    memory_region_init_io(machine_id, OBJECT(machine), &machine_id_reg_ops,
> +                          NULL, "machine_id", 2);
> +    memory_region_add_subregion(get_system_memory(), 0xff000004, machine_id);
> +
>      /* XXX: we register only 1 output pin for heathrow PIC */
>      pic_dev = qdev_create(NULL, TYPE_HEATHROW);
>      qdev_init_nofail(pic_dev);

I'm guessing this supersedes the version from the v2 patchset? Given that you're
introducing a HeathrowMachineState in patch 5, I'd be inclined to store the
MemoryRegions there to prevent these references leaking after init(). A 
constant for
the address of the register would also be good here too.


ATB,

Mark.