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

From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
To: BALATON Zoltan <balaton@eik.bme.hu>,
	qemu-devel@nongnu.org, qemu-ppc@nongnu.org
Cc: David Gibson <david@gibson.dropbear.id.au>,
	Howard Spoelstra <hsp.cat7@gmail.com>
Subject: Re: [PATCH v5 07/11] mac_oldworld: Map macio to expected address at reset
Date: Fri, 26 Jun 2020 14:03:08 +0100	[thread overview]
Message-ID: <d8db3e1f-6421-ea36-8598-60e9d98f54fa@ilande.co.uk> (raw)
In-Reply-To: <a0b567bf6e5b266a19de4eb6f6551270ffe2ce7b.1592315226.git.balaton@eik.bme.hu>

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.