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

[BALATON Zoltan]

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

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

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

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

Regards,
BALATON Zoltan