[PATCH v3 10/13] hw/ppc/pegasos2: Add bus frequency to machine state

BALATON Zoltan posted 13 patches 2 weeks, 6 days ago
Maintainers: BALATON Zoltan <balaton@eik.bme.hu>, Alexey Kardashevskiy <aik@ozlabs.ru>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Yanan Wang <wangyanan55@huawei.com>, Zhao Liu <zhao1.liu@intel.com>
There is a newer version of this series
[PATCH v3 10/13] hw/ppc/pegasos2: Add bus frequency to machine state
Posted by BALATON Zoltan 2 weeks, 6 days ago
Store the bus frequency in the machine state and set it from instance
init method.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/ppc/pegasos2.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index f7999520e4..ae3f01231d 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -55,8 +55,6 @@
 #define H_PRIVILEGE  -3  /* Caller not privileged */
 #define H_PARAMETER  -4  /* Parameter invalid, out-of-range or conflicting */
 
-#define BUS_FREQ_HZ 133333333
-
 #define TYPE_PEGASOS_MACHINE MACHINE_TYPE_NAME("pegasos")
 OBJECT_DECLARE_SIMPLE_TYPE(PegasosMachineState, PEGASOS_MACHINE)
 
@@ -66,6 +64,7 @@ struct PegasosMachineState {
     PowerPCCPU *cpu;
     DeviceState *nb; /* north bridge */
     DeviceState *sb; /* south bridge */
+    int bus_freq_hz;
     IRQState pci_irqs[PCI_NUM_PINS];
     OrIRQState orirq[PCI_NUM_PINS];
     qemu_irq mv_pirq[PCI_NUM_PINS];
@@ -135,7 +134,7 @@ static void pegasos2_setup_pci_irq(PegasosMachineState *pm)
                                 qdev_get_gpio_in_named(pm->nb, "gpp", 31));
 }
 
-static void pegasos2_init(MachineState *machine)
+static void pegasos_init(MachineState *machine)
 {
     PegasosMachineState *pm = PEGASOS_MACHINE(machine);
     CPUPPCState *env;
@@ -158,7 +157,7 @@ static void pegasos2_init(MachineState *machine)
     }
 
     /* Set time-base frequency */
-    cpu_ppc_tb_init(env, BUS_FREQ_HZ / 4);
+    cpu_ppc_tb_init(env, pm->bus_freq_hz / 4);
     qemu_register_reset(pegasos2_cpu_reset, pm->cpu);
 
     /* RAM */
@@ -603,6 +602,13 @@ static bool pegasos2_setprop(MachineState *ms, const char *path,
     return true;
 }
 
+static void pegasos2_init(Object *obj)
+{
+    PegasosMachineState *pm = PEGASOS_MACHINE(obj);
+
+    pm->bus_freq_hz = 133333333;
+}
+
 static void pegasos2_machine_class_init(ObjectClass *oc, const void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
@@ -610,7 +616,7 @@ static void pegasos2_machine_class_init(ObjectClass *oc, const void *data)
     VofMachineIfClass *vmc = VOF_MACHINE_CLASS(oc);
 
     mc->desc = "Genesi/bPlan Pegasos II";
-    mc->init = pegasos2_init;
+    mc->init = pegasos_init;
     mc->reset = pegasos2_machine_reset;
     mc->block_default_type = IF_IDE;
     mc->default_boot_order = "cd";
@@ -640,6 +646,7 @@ static const TypeInfo pegasos_machine_types[] = {
         .name          = MACHINE_TYPE_NAME("pegasos2"),
         .parent        = TYPE_PEGASOS_MACHINE,
         .class_init    = pegasos2_machine_class_init,
+        .instance_init = pegasos2_init,
         .interfaces = (const InterfaceInfo[]) {
               { TYPE_PPC_VIRTUAL_HYPERVISOR },
               { TYPE_VOF_MACHINE_IF },
@@ -777,7 +784,7 @@ static void add_pci_device(PCIBus *bus, PCIDevice *d, void *opaque)
     g_string_free(node, TRUE);
 }
 
-static void add_cpu_info(void *fdt, PowerPCCPU *cpu)
+static void add_cpu_info(void *fdt, PowerPCCPU *cpu, int bus_freq)
 {
     uint32_t cells[2];
 
@@ -824,8 +831,8 @@ static void add_cpu_info(void *fdt, PowerPCCPU *cpu)
     qemu_fdt_setprop_cell(fdt, cp, "reservation-granule-size", 4);
     qemu_fdt_setprop_cell(fdt, cp, "timebase-frequency",
                           cpu->env.tb_env->tb_freq);
-    qemu_fdt_setprop_cell(fdt, cp, "bus-frequency", BUS_FREQ_HZ);
-    qemu_fdt_setprop_cell(fdt, cp, "clock-frequency", BUS_FREQ_HZ * 7.5);
+    qemu_fdt_setprop_cell(fdt, cp, "bus-frequency", bus_freq);
+    qemu_fdt_setprop_cell(fdt, cp, "clock-frequency", bus_freq * 7.5);
     qemu_fdt_setprop_cell(fdt, cp, "cpu-version", cpu->env.spr[SPR_PVR]);
     cells[0] = 0;
     cells[1] = 0;
@@ -861,7 +868,7 @@ static void *pegasos2_build_fdt(PegasosMachineState *pm, int *fdt_size)
     }
     qemu_fdt_setprop_string(fdt, "/", "name", "bplan,Pegasos2");
 
-    add_cpu_info(fdt, pm->cpu);
+    add_cpu_info(fdt, pm->cpu, pm->bus_freq_hz);
 
     fi.fdt = fdt;
     fi.path = "/pci@c0000000";
-- 
2.41.3
Re: [PATCH v3 10/13] hw/ppc/pegasos2: Add bus frequency to machine state
Posted by Philippe Mathieu-Daudé 2 weeks, 5 days ago
On 18/10/25 17:11, BALATON Zoltan wrote:
> Store the bus frequency in the machine state and set it from instance
> init method.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>   hw/ppc/pegasos2.c | 25 ++++++++++++++++---------
>   1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
> index f7999520e4..ae3f01231d 100644
> --- a/hw/ppc/pegasos2.c
> +++ b/hw/ppc/pegasos2.c
> @@ -55,8 +55,6 @@
>   #define H_PRIVILEGE  -3  /* Caller not privileged */
>   #define H_PARAMETER  -4  /* Parameter invalid, out-of-range or conflicting */
>   
> -#define BUS_FREQ_HZ 133333333
> -
>   #define TYPE_PEGASOS_MACHINE MACHINE_TYPE_NAME("pegasos")
>   OBJECT_DECLARE_SIMPLE_TYPE(PegasosMachineState, PEGASOS_MACHINE)
>   
> @@ -66,6 +64,7 @@ struct PegasosMachineState {
>       PowerPCCPU *cpu;
>       DeviceState *nb; /* north bridge */
>       DeviceState *sb; /* south bridge */
> +    int bus_freq_hz;

IMHO this field belongs to PegasosMachineClass, being read-only.

>       IRQState pci_irqs[PCI_NUM_PINS];
>       OrIRQState orirq[PCI_NUM_PINS];
>       qemu_irq mv_pirq[PCI_NUM_PINS];


> +static void pegasos2_init(Object *obj)
> +{
> +    PegasosMachineState *pm = PEGASOS_MACHINE(obj);
> +
> +    pm->bus_freq_hz = 133333333;
> +}
> +
>   static void pegasos2_machine_class_init(ObjectClass *oc, const void *data)
>   {
>       MachineClass *mc = MACHINE_CLASS(oc);
> @@ -610,7 +616,7 @@ static void pegasos2_machine_class_init(ObjectClass *oc, const void *data)
>       VofMachineIfClass *vmc = VOF_MACHINE_CLASS(oc);
>   
>       mc->desc = "Genesi/bPlan Pegasos II";
> -    mc->init = pegasos2_init;
> +    mc->init = pegasos_init;
>       mc->reset = pegasos2_machine_reset;
>       mc->block_default_type = IF_IDE;
>       mc->default_boot_order = "cd";
> @@ -640,6 +646,7 @@ static const TypeInfo pegasos_machine_types[] = {
>           .name          = MACHINE_TYPE_NAME("pegasos2"),
>           .parent        = TYPE_PEGASOS_MACHINE,
>           .class_init    = pegasos2_machine_class_init,
> +        .instance_init = pegasos2_init,
>           .interfaces = (const InterfaceInfo[]) {
>                 { TYPE_PPC_VIRTUAL_HYPERVISOR },
>                 { TYPE_VOF_MACHINE_IF },

If you want pegasos2_init(), move the definition here to avoid churn
in patch #12.
Re: [PATCH v3 10/13] hw/ppc/pegasos2: Add bus frequency to machine state
Posted by BALATON Zoltan 2 weeks, 5 days ago
On Mon, 20 Oct 2025, Philippe Mathieu-Daudé wrote:
> On 18/10/25 17:11, BALATON Zoltan wrote:
>> Store the bus frequency in the machine state and set it from instance
>> init method.
>> 
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>>   hw/ppc/pegasos2.c | 25 ++++++++++++++++---------
>>   1 file changed, 16 insertions(+), 9 deletions(-)
>> 
>> diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
>> index f7999520e4..ae3f01231d 100644
>> --- a/hw/ppc/pegasos2.c
>> +++ b/hw/ppc/pegasos2.c
>> @@ -55,8 +55,6 @@
>>   #define H_PRIVILEGE  -3  /* Caller not privileged */
>>   #define H_PARAMETER  -4  /* Parameter invalid, out-of-range or 
>> conflicting */
>>   -#define BUS_FREQ_HZ 133333333
>> -
>>   #define TYPE_PEGASOS_MACHINE MACHINE_TYPE_NAME("pegasos")
>>   OBJECT_DECLARE_SIMPLE_TYPE(PegasosMachineState, PEGASOS_MACHINE)
>>   @@ -66,6 +64,7 @@ struct PegasosMachineState {
>>       PowerPCCPU *cpu;
>>       DeviceState *nb; /* north bridge */
>>       DeviceState *sb; /* south bridge */
>> +    int bus_freq_hz;
>
> IMHO this field belongs to PegasosMachineClass, being read-only.

Reasons for not putting there:

1. I need it in machine init and build_fdt which get the machine state not 
machine class so would need to copy to machine state even if it was 
defined in the class.

2. We don't have a PegasosMachineState and it does not seem necessary to 
add one just for this single value which would then be never used other 
than copying it to the PegasosMachineState in init.

So for simplicity I've stored it the existing state along other values 
which seems to be simple enough. I don't see putting it in the class would 
be simpler.

>>       IRQState pci_irqs[PCI_NUM_PINS];
>>       OrIRQState orirq[PCI_NUM_PINS];
>>       qemu_irq mv_pirq[PCI_NUM_PINS];
>
>
>> +static void pegasos2_init(Object *obj)
>> +{
>> +    PegasosMachineState *pm = PEGASOS_MACHINE(obj);
>> +
>> +    pm->bus_freq_hz = 133333333;
>> +}
>> +
>>   static void pegasos2_machine_class_init(ObjectClass *oc, const void 
>> *data)
>>   {
>>       MachineClass *mc = MACHINE_CLASS(oc);
>> @@ -610,7 +616,7 @@ static void pegasos2_machine_class_init(ObjectClass 
>> *oc, const void *data)
>>       VofMachineIfClass *vmc = VOF_MACHINE_CLASS(oc);
>>         mc->desc = "Genesi/bPlan Pegasos II";
>> -    mc->init = pegasos2_init;
>> +    mc->init = pegasos_init;
>>       mc->reset = pegasos2_machine_reset;
>>       mc->block_default_type = IF_IDE;
>>       mc->default_boot_order = "cd";
>> @@ -640,6 +646,7 @@ static const TypeInfo pegasos_machine_types[] = {
>>           .name          = MACHINE_TYPE_NAME("pegasos2"),
>>           .parent        = TYPE_PEGASOS_MACHINE,
>>           .class_init    = pegasos2_machine_class_init,
>> +        .instance_init = pegasos2_init,
>>           .interfaces = (const InterfaceInfo[]) {
>>                 { TYPE_PPC_VIRTUAL_HYPERVISOR },
>>                 { TYPE_VOF_MACHINE_IF },
>
> If you want pegasos2_init(), move the definition here to avoid churn
> in patch #12.

I don't understand what causes the churn as the function is the same, 
patch 12 only adds one line to it but maybe renaming surrounding functions 
confused git to generate patch that moves this function and not ammends 
it. What can I move here to avoid that?

Regards,
BALATON Zoltan