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