diff mbox series

[v5,07/11] mac_oldworld: Map macio to expected address at reset

Message ID a0b567bf6e5b266a19de4eb6f6551270ffe2ce7b.1592315226.git.balaton@eik.bme.hu (mailing list archive)
State New, archived
Headers show
Series Mac Old World ROM experiment | expand

Commit Message

BALATON Zoltan June 16, 2020, 1:47 p.m. UTC 
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(-)

Comments

Mark Cave-Ayland June 26, 2020, 1:03 p.m. UTC | #1 
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.
BALATON Zoltan June 26, 2020, 10:25 p.m. UTC | #2 
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
diff mbox series

Patch

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);
+}
+
 static void ppc_heathrow_cpu_reset(void *opaque)
 {
     PowerPCCPU *cpu = opaque;
@@ -82,6 +91,7 @@  static void ppc_heathrow_cpu_reset(void *opaque)
 
 static void ppc_heathrow_init(MachineState *machine)
 {
+    HeathrowMachineState *hm = HEATHROW_MACHINE(machine);
     ram_addr_t ram_size = machine->ram_size;
     const char *boot_device = machine->boot_order;
     PowerPCCPU *cpu = NULL;
@@ -287,6 +297,7 @@  static void ppc_heathrow_init(MachineState *machine)
 
     /* MacIO */
     macio = pci_new(-1, TYPE_OLDWORLD_MACIO);
+    hm->macio = macio;
     dev = DEVICE(macio);
     qdev_prop_set_uint64(dev, "frequency", tbfreq);
     object_property_set_link(OBJECT(macio), OBJECT(pic_dev), "pic",
@@ -439,6 +450,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
@@ -455,9 +467,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 },
         { }