Move the pegasos2 specific chipset reset out from machine reset to a
separate function and move generic parts that are not pegasos2
specific from build_fdt to machine reset so now build_fdt only
contains pegasos2 specific parts and can be renamed accordingly.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/ppc/pegasos2.c | 79 ++++++++++++++++++++++++-----------------------
1 file changed, 41 insertions(+), 38 deletions(-)
diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index 2f9bd3eac5..ed3070204b 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -57,10 +57,6 @@
#define BUS_FREQ_HZ 133333333
-#define PCI0_CFG_ADDR 0xcf8
-#define PCI1_CFG_ADDR 0xc78
-#define PCI1_IO_BASE 0xfe000000
-
#define TYPE_PEGASOS2_MACHINE MACHINE_TYPE_NAME("pegasos2")
OBJECT_DECLARE_TYPE(Pegasos2MachineState, MachineClass, PEGASOS2_MACHINE)
@@ -82,7 +78,7 @@ struct Pegasos2MachineState {
uint64_t initrd_size;
};
-static void *build_fdt(MachineState *machine, int *fdt_size);
+static void *pegasos2_build_fdt(Pegasos2MachineState *pm, int *fdt_size);
static void pegasos2_cpu_reset(void *opaque)
{
@@ -284,6 +280,9 @@ static void pegasos2_mv_reg_write(Pegasos2MachineState *pm, uint32_t addr,
MEMTXATTRS_UNSPECIFIED);
}
+#define PCI0_CFG_ADDR 0xcf8
+#define PCI1_CFG_ADDR 0xc78
+
static uint32_t pegasos2_pci_config_read(Pegasos2MachineState *pm, int bus,
uint32_t addr, uint32_t len)
{
@@ -308,23 +307,12 @@ static void pegasos2_pci_config_write(Pegasos2MachineState *pm, int bus,
static void pegasos2_superio_write(uint8_t addr, uint8_t val)
{
- cpu_physical_memory_write(PCI1_IO_BASE + 0x3f0, &addr, 1);
- cpu_physical_memory_write(PCI1_IO_BASE + 0x3f1, &val, 1);
+ cpu_physical_memory_write(0xfe0003f0, &addr, 1);
+ cpu_physical_memory_write(0xfe0003f1, &val, 1);
}
-static void pegasos2_machine_reset(MachineState *machine, ResetType type)
+static void pegasos2_chipset_reset(Pegasos2MachineState *pm)
{
- Pegasos2MachineState *pm = PEGASOS2_MACHINE(machine);
- void *fdt;
- uint64_t d[2];
- int sz;
-
- qemu_devices_reset(type);
- if (!pm->vof) {
- return; /* Firmware should set up machine so nothing to do */
- }
-
- /* Otherwise, set up devices that board firmware would normally do */
pegasos2_mv_reg_write(pm, 0, 4, 0x28020ff);
pegasos2_mv_reg_write(pm, 0x278, 4, 0xa31fc);
pegasos2_mv_reg_write(pm, 0xf300, 4, 0x11ff0400);
@@ -387,6 +375,23 @@ static void pegasos2_machine_reset(MachineState *machine, ResetType type)
pegasos2_pci_config_write(pm, 1, (PCI_DEVFN(12, 6) << 8) |
PCI_INTERRUPT_LINE, 2, 0x309);
+}
+
+static void pegasos2_machine_reset(MachineState *machine, ResetType type)
+{
+ Pegasos2MachineState *pm = PEGASOS2_MACHINE(machine);
+ void *fdt;
+ uint32_t c[2];
+ uint64_t d[2];
+ int sz;
+
+ qemu_devices_reset(type);
+ if (!pm->vof) {
+ return; /* Firmware should set up machine so nothing to do */
+ }
+
+ /* Otherwise, set up devices that board firmware would normally do */
+ pegasos2_chipset_reset(pm);
/* Device tree and VOF set up */
vof_init(pm->vof, machine->ram_size, &error_fatal);
@@ -405,10 +410,25 @@ static void pegasos2_machine_reset(MachineState *machine, ResetType type)
exit(1);
}
- fdt = build_fdt(machine, &sz);
+ fdt = pegasos2_build_fdt(pm, &sz);
if (!fdt) {
exit(1);
}
+
+ /* Set memory size */
+ c[0] = 0;
+ c[1] = cpu_to_be32(machine->ram_size);
+ qemu_fdt_setprop(fdt, "/memory@0", "reg", c, sizeof(c));
+
+ /* Boot parameters */
+ if (pm->initrd_addr && pm->initrd_size) {
+ qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-end",
+ pm->initrd_addr + pm->initrd_size);
+ qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-start",
+ pm->initrd_addr);
+ }
+ qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
+ machine->kernel_cmdline ?: "");
/* FIXME: VOF assumes entry is same as load address */
d[0] = cpu_to_be64(pm->kernel_entry);
d[1] = cpu_to_be64(pm->kernel_size - (pm->kernel_entry - pm->kernel_addr));
@@ -827,12 +847,10 @@ static void *load_dtb(const char *filename, int *fdt_size)
return fdt;
}
-static void *build_fdt(MachineState *machine, int *fdt_size)
+static void *pegasos2_build_fdt(Pegasos2MachineState *pm, int *fdt_size)
{
- Pegasos2MachineState *pm = PEGASOS2_MACHINE(machine);
FDTInfo fi;
PCIBus *pci_bus;
- uint32_t cells[2];
void *fdt = load_dtb("pegasos2.dtb", fdt_size);
if (!fdt) {
@@ -840,21 +858,6 @@ static void *build_fdt(MachineState *machine, int *fdt_size)
}
qemu_fdt_setprop_string(fdt, "/", "name", "bplan,Pegasos2");
- /* Set memory size */
- cells[0] = 0;
- cells[1] = cpu_to_be32(machine->ram_size);
- qemu_fdt_setprop(fdt, "/memory@0", "reg", cells, 2 * sizeof(cells[0]));
-
- /* Boot parameters */
- if (pm->initrd_addr && pm->initrd_size) {
- qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-end",
- pm->initrd_addr + pm->initrd_size);
- qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-start",
- pm->initrd_addr);
- }
- qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
- machine->kernel_cmdline ?: "");
-
add_cpu_info(fdt, pm->cpu);
fi.fdt = fdt;
--
2.41.3On 23/10/25 02:06, BALATON Zoltan wrote:
> Move the pegasos2 specific chipset reset out from machine reset to a
> separate function and move generic parts that are not pegasos2
> specific from build_fdt to machine reset so now build_fdt only
> contains pegasos2 specific parts and can be renamed accordingly.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> hw/ppc/pegasos2.c | 79 ++++++++++++++++++++++++-----------------------
> 1 file changed, 41 insertions(+), 38 deletions(-)
> -#define PCI1_IO_BASE 0xfe000000
Can't we keep such definition?
> @@ -308,23 +307,12 @@ static void pegasos2_pci_config_write(Pegasos2MachineState *pm, int bus,
>
> static void pegasos2_superio_write(uint8_t addr, uint8_t val)
> {
> - cpu_physical_memory_write(PCI1_IO_BASE + 0x3f0, &addr, 1);
> - cpu_physical_memory_write(PCI1_IO_BASE + 0x3f1, &val, 1);
> + cpu_physical_memory_write(0xfe0003f0, &addr, 1);
> + cpu_physical_memory_write(0xfe0003f1, &val, 1);
Otherwise it is harder to notice we are accessing the MMIO mapped ISA space.
> }
Consider renaming as pegasos_superio_write() since this method becomes
common to PegasOS I and II.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
On Thu, 23 Oct 2025, Philippe Mathieu-Daudé wrote:
> On 23/10/25 02:06, BALATON Zoltan wrote:
>> Move the pegasos2 specific chipset reset out from machine reset to a
>> separate function and move generic parts that are not pegasos2
>> specific from build_fdt to machine reset so now build_fdt only
>> contains pegasos2 specific parts and can be renamed accordingly.
>>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>> hw/ppc/pegasos2.c | 79 ++++++++++++++++++++++++-----------------------
>> 1 file changed, 41 insertions(+), 38 deletions(-)
>
>
>> -#define PCI1_IO_BASE 0xfe000000
>
> Can't we keep such definition?
>
>> @@ -308,23 +307,12 @@ static void
>> pegasos2_pci_config_write(Pegasos2MachineState *pm, int bus,
>> static void pegasos2_superio_write(uint8_t addr, uint8_t val)
>> {
>> - cpu_physical_memory_write(PCI1_IO_BASE + 0x3f0, &addr, 1);
>> - cpu_physical_memory_write(PCI1_IO_BASE + 0x3f1, &val, 1);
>> + cpu_physical_memory_write(0xfe0003f0, &addr, 1);
>> + cpu_physical_memory_write(0xfe0003f1, &val, 1);
>
> Otherwise it is harder to notice we are accessing the MMIO mapped ISA space.
It might be misleading as it's PCI1 on pegasos2 but PCI0 on pegasos1 so
the define does not clarify much either way even if I rename it to
PCI_IO_BASE. Generally I think for numbers that are only used once having
a define just obfuscates it as I then have to look up what the number is
elsewhere instead of seeing right away. I guess I'll leave it as it is now
that this version was pulled.
>> }
> Consider renaming as pegasos_superio_write() since this method becomes
> common to PegasOS I and II.
This could have removed one hunk from patch 12 and adding it here making
that patch shorter but this one longer so not much difference. Now that
these are reviewed I think it does not matter any more.
Regards,
BALATON Zoltan
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
>
On 10/23/25 12:16, Philippe Mathieu-Daudé wrote:
> On 23/10/25 02:06, BALATON Zoltan wrote:
>> Move the pegasos2 specific chipset reset out from machine reset to a
>> separate function and move generic parts that are not pegasos2
>> specific from build_fdt to machine reset so now build_fdt only
>> contains pegasos2 specific parts and can be renamed accordingly.
>>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>> hw/ppc/pegasos2.c | 79 ++++++++++++++++++++++++-----------------------
>> 1 file changed, 41 insertions(+), 38 deletions(-)
>
>
>> -#define PCI1_IO_BASE 0xfe000000
>
> Can't we keep such definition?
>
>> @@ -308,23 +307,12 @@ static void
>> pegasos2_pci_config_write(Pegasos2MachineState *pm, int bus,
>> static void pegasos2_superio_write(uint8_t addr, uint8_t val)
>> {
>> - cpu_physical_memory_write(PCI1_IO_BASE + 0x3f0, &addr, 1);
>> - cpu_physical_memory_write(PCI1_IO_BASE + 0x3f1, &val, 1);
>> + cpu_physical_memory_write(0xfe0003f0, &addr, 1);
>> + cpu_physical_memory_write(0xfe0003f1, &val, 1);
>
> Otherwise it is harder to notice we are accessing the MMIO mapped ISA
> space.
>
>> }
> Consider renaming as pegasos_superio_write() since this method becomes
> common to PegasOS I and II.
Thanks Philippe for reviewing the series.
Hi BALATON,
Would you mind addressing the above (and other?) review comments or I
can queue it in the interest of time if you can send a follow-up patch
later?
Thanks
Harsh
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
On Thu, 23 Oct 2025, Harsh Prateek Bora wrote:
> On 10/23/25 12:16, Philippe Mathieu-Daudé wrote:
>> On 23/10/25 02:06, BALATON Zoltan wrote:
>>> Move the pegasos2 specific chipset reset out from machine reset to a
>>> separate function and move generic parts that are not pegasos2
>>> specific from build_fdt to machine reset so now build_fdt only
>>> contains pegasos2 specific parts and can be renamed accordingly.
>>>
>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>> ---
>>> hw/ppc/pegasos2.c | 79 ++++++++++++++++++++++++-----------------------
>>> 1 file changed, 41 insertions(+), 38 deletions(-)
>>
>>
>>> -#define PCI1_IO_BASE 0xfe000000
>>
>> Can't we keep such definition?
>>
>>> @@ -308,23 +307,12 @@ static void
>>> pegasos2_pci_config_write(Pegasos2MachineState *pm, int bus,
>>> static void pegasos2_superio_write(uint8_t addr, uint8_t val)
>>> {
>>> - cpu_physical_memory_write(PCI1_IO_BASE + 0x3f0, &addr, 1);
>>> - cpu_physical_memory_write(PCI1_IO_BASE + 0x3f1, &val, 1);
>>> + cpu_physical_memory_write(0xfe0003f0, &addr, 1);
>>> + cpu_physical_memory_write(0xfe0003f1, &val, 1);
>>
>> Otherwise it is harder to notice we are accessing the MMIO mapped ISA
>> space.
>>
>>> }
>> Consider renaming as pegasos_superio_write() since this method becomes
>> common to PegasOS I and II.
>
> Thanks Philippe for reviewing the series.
>
> Hi BALATON,
> Would you mind addressing the above (and other?) review comments or I can
> queue it in the interest of time if you can send a follow-up patch later?
I'll send another version of this series after rebasing the prep series.
We still have time until the freeze begins so I think we can wait a few
more days.
Thank you,
BALATON Zoltan
> Thanks
> Harsh
>
>>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>
>
>
© 2016 - 2025 Red Hat, Inc.