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

[Mark Cave-Ayland]

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

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

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

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


ATB,

Mark.