[PATCH 00/10] hw/arm/realview: Introduce abstract RealviewMachineClass

[PATCH 00/10] hw/arm/realview: Introduce abstract RealviewMachineClass

[Philippe Mathieu-Daudé]

Probably still spring housekeeping stuff.
QOM'ify Realview machines further by introducing a common
(abstract) MachineClass, and set the read-only fields in
the class_init() instead of machine_init() method.

At least the diff-stat is negative :)

Philippe Mathieu-Daudé (10):
  hw/arm/realview: Simplify using 'break' statement
  hw/arm/realview: Declare QOM types using DEFINE_TYPES() macro
  hw/arm/realview: Introduce abstract RealviewMachineClass
  hw/arm/realview: Factor realview_common_class_init() out
  hw/arm/realview: Move 'board_id' to RealviewMachineClass
  hw/arm/realview: Move 'is_pb' to RealviewMachineClass
  hw/arm/realview: Move 'mpcore_periphbase' to RealviewMachineClass
  hw/arm/realview: Move 'loader_start' to RealviewMachineClass
  hw/arm/realview: Use generic realview_common_machine_init()
  hw/arm/realview: Set MachineClass::default_nic in machine_class_init()

 hw/arm/realview.c | 177 +++++++++++++++++++---------------------------
 1 file changed, 74 insertions(+), 103 deletions(-)

-- 
2.38.1

[PATCH 01/10] hw/arm/realview: Simplify using 'break' statement

[Philippe Mathieu-Daudé]

The 'break' statement terminates the execution of the nearest
enclosing 'for' statement in which it appears.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/realview.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/arm/realview.c b/hw/arm/realview.c
index a5aa2f046a..a52ff35084 100644
--- a/hw/arm/realview.c
+++ b/hw/arm/realview.c
@@ -88,7 +88,6 @@ static void realview_init(MachineState *machine,
     I2CBus *i2c;
     int n;
     unsigned int smp_cpus = machine->smp.cpus;
-    int done_nic = 0;
     qemu_irq cpu_irq[4];
     int is_mpcore = 0;
     int is_pb = 0;
@@ -294,14 +293,13 @@ static void realview_init(MachineState *machine,
     for(n = 0; n < nb_nics; n++) {
         nd = &nd_table[n];
 
-        if (!done_nic && (!nd->model ||
-                    strcmp(nd->model, is_pb ? "lan9118" : "smc91c111") == 0)) {
+        if (!nd->model || strcmp(nd->model, is_pb ? "lan9118" : "smc91c111") == 0) {
             if (is_pb) {
                 lan9118_init(nd, 0x4e000000, pic[28]);
             } else {
                 smc91c111_init(nd, 0x4e000000, pic[28]);
             }
-            done_nic = 1;
+            break;
         } else {
             if (pci_bus) {
                 pci_nic_init_nofail(nd, pci_bus, "rtl8139", NULL);
-- 
2.38.1

Re: [PATCH 01/10] hw/arm/realview: Simplify using 'break' statement

[Richard Henderson]

On 5/24/23 07:58, Philippe Mathieu-Daudé wrote:
> The 'break' statement terminates the execution of the nearest
> enclosing 'for' statement in which it appears.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/arm/realview.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/arm/realview.c b/hw/arm/realview.c
> index a5aa2f046a..a52ff35084 100644
> --- a/hw/arm/realview.c
> +++ b/hw/arm/realview.c
> @@ -88,7 +88,6 @@ static void realview_init(MachineState *machine,
>       I2CBus *i2c;
>       int n;
>       unsigned int smp_cpus = machine->smp.cpus;
> -    int done_nic = 0;
>       qemu_irq cpu_irq[4];
>       int is_mpcore = 0;
>       int is_pb = 0;
> @@ -294,14 +293,13 @@ static void realview_init(MachineState *machine,
>       for(n = 0; n < nb_nics; n++) {
>           nd = &nd_table[n];
>   
> -        if (!done_nic && (!nd->model ||
> -                    strcmp(nd->model, is_pb ? "lan9118" : "smc91c111") == 0)) {
> +        if (!nd->model || strcmp(nd->model, is_pb ? "lan9118" : "smc91c111") == 0) {
>               if (is_pb) {
>                   lan9118_init(nd, 0x4e000000, pic[28]);
>               } else {
>                   smc91c111_init(nd, 0x4e000000, pic[28]);
>               }
> -            done_nic = 1;
> +            break;

While I agree this preserves existing behaviour, it doesn't seem like the logic is 
actually correct.  This will only ever connect 1 of nb_nics.


r~


>           } else {
>               if (pci_bus) {
>                   pci_nic_init_nofail(nd, pci_bus, "rtl8139", NULL);

Re: [PATCH 01/10] hw/arm/realview: Simplify using 'break' statement

[Peter Maydell]

On Wed, 24 May 2023 at 20:01, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 5/24/23 07:58, Philippe Mathieu-Daudé wrote:
> > The 'break' statement terminates the execution of the nearest
> > enclosing 'for' statement in which it appears.
> >
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> >   hw/arm/realview.c | 6 ++----
> >   1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/arm/realview.c b/hw/arm/realview.c
> > index a5aa2f046a..a52ff35084 100644
> > --- a/hw/arm/realview.c
> > +++ b/hw/arm/realview.c
> > @@ -88,7 +88,6 @@ static void realview_init(MachineState *machine,
> >       I2CBus *i2c;
> >       int n;
> >       unsigned int smp_cpus = machine->smp.cpus;
> > -    int done_nic = 0;
> >       qemu_irq cpu_irq[4];
> >       int is_mpcore = 0;
> >       int is_pb = 0;
> > @@ -294,14 +293,13 @@ static void realview_init(MachineState *machine,
> >       for(n = 0; n < nb_nics; n++) {
> >           nd = &nd_table[n];
> >
> > -        if (!done_nic && (!nd->model ||
> > -                    strcmp(nd->model, is_pb ? "lan9118" : "smc91c111") == 0)) {
> > +        if (!nd->model || strcmp(nd->model, is_pb ? "lan9118" : "smc91c111") == 0) {
> >               if (is_pb) {
> >                   lan9118_init(nd, 0x4e000000, pic[28]);
> >               } else {
> >                   smc91c111_init(nd, 0x4e000000, pic[28]);
> >               }
> > -            done_nic = 1;
> > +            break;
>
> While I agree this preserves existing behaviour, it doesn't seem like the logic is
> actually correct.  This will only ever connect 1 of nb_nics.

Does it preserve the existing behaviour, though? I think the
intent of the code is:
 * we only create one at most hard-wired NIC (the lan9118 or smc91c111,
   depending on the board type), and we do that for the
   first entry in the nd_table which specifies a matching 'model'
 * every other NIC is a PCI rtl8139 (or ignored if the board
   has no PCI)

Maybe I'm misreading the current code, but it looks to me
like it does this: done_nic is a flag for "did we create
the hardwired device", and we only create the hardwired
device if the flag is false and the NIC model matches.
Once we've created the hardwired device we set done_nic
to true and the if() will then always take us into the
rtl8139 part.

On the other hand, this patch using break means that
once the hardwired NIC has been created we'll exit the
loop entirely and won't create any subsequent PCI devices.

thanks
-- PMM

[PATCH 02/10] hw/arm/realview: Declare QOM types using DEFINE_TYPES() macro

[Philippe Mathieu-Daudé]

When multiple QOM types are registered in the same file,
it is simpler to use the the DEFINE_TYPES() macro. Replace
the type_init() / type_register_static() combination.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/realview.c | 50 ++++++++++++++++++-----------------------------
 1 file changed, 19 insertions(+), 31 deletions(-)

diff --git a/hw/arm/realview.c b/hw/arm/realview.c
index a52ff35084..07a80d0de3 100644
--- a/hw/arm/realview.c
+++ b/hw/arm/realview.c
@@ -412,12 +412,6 @@ static void realview_eb_class_init(ObjectClass *oc, void *data)
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("arm926");
 }
 
-static const TypeInfo realview_eb_type = {
-    .name = MACHINE_TYPE_NAME("realview-eb"),
-    .parent = TYPE_MACHINE,
-    .class_init = realview_eb_class_init,
-};
-
 static void realview_eb_mpcore_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
@@ -430,12 +424,6 @@ static void realview_eb_mpcore_class_init(ObjectClass *oc, void *data)
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("arm11mpcore");
 }
 
-static const TypeInfo realview_eb_mpcore_type = {
-    .name = MACHINE_TYPE_NAME("realview-eb-mpcore"),
-    .parent = TYPE_MACHINE,
-    .class_init = realview_eb_mpcore_class_init,
-};
-
 static void realview_pb_a8_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
@@ -446,12 +434,6 @@ static void realview_pb_a8_class_init(ObjectClass *oc, void *data)
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a8");
 }
 
-static const TypeInfo realview_pb_a8_type = {
-    .name = MACHINE_TYPE_NAME("realview-pb-a8"),
-    .parent = TYPE_MACHINE,
-    .class_init = realview_pb_a8_class_init,
-};
-
 static void realview_pbx_a9_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
@@ -463,18 +445,24 @@ static void realview_pbx_a9_class_init(ObjectClass *oc, void *data)
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a9");
 }
 
-static const TypeInfo realview_pbx_a9_type = {
-    .name = MACHINE_TYPE_NAME("realview-pbx-a9"),
-    .parent = TYPE_MACHINE,
-    .class_init = realview_pbx_a9_class_init,
+static const TypeInfo realview_machine_types[] = {
+    {
+        .name           = MACHINE_TYPE_NAME("realview-eb"),
+        .parent         = TYPE_MACHINE,
+        .class_init     = realview_eb_class_init,
+    }, {
+        .name           = MACHINE_TYPE_NAME("realview-eb-mpcore"),
+        .parent         = TYPE_MACHINE,
+        .class_init     = realview_eb_mpcore_class_init,
+    }, {
+        .name           = MACHINE_TYPE_NAME("realview-pb-a8"),
+        .parent         = TYPE_MACHINE,
+        .class_init     = realview_pb_a8_class_init,
+    }, {
+        .name           = MACHINE_TYPE_NAME("realview-pbx-a9"),
+        .parent         = TYPE_MACHINE,
+        .class_init     = realview_pbx_a9_class_init,
+    }
 };
 
-static void realview_machine_init(void)
-{
-    type_register_static(&realview_eb_type);
-    type_register_static(&realview_eb_mpcore_type);
-    type_register_static(&realview_pb_a8_type);
-    type_register_static(&realview_pbx_a9_type);
-}
-
-type_init(realview_machine_init)
+DEFINE_TYPES(realview_machine_types)
-- 
2.38.1

Re: [PATCH 02/10] hw/arm/realview: Declare QOM types using DEFINE_TYPES() macro

[Richard Henderson]

On 5/24/23 07:58, Philippe Mathieu-Daudé wrote:
> When multiple QOM types are registered in the same file,
> it is simpler to use the the DEFINE_TYPES() macro. Replace
> the type_init() / type_register_static() combination.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/arm/realview.c | 50 ++++++++++++++++++-----------------------------
>   1 file changed, 19 insertions(+), 31 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 03/10] hw/arm/realview: Introduce abstract RealviewMachineClass

[Philippe Mathieu-Daudé]

Introduce the abstract QOM TYPE_REALVIEW_MACHINE to
handle fields common to all Realview machines.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/realview.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/hw/arm/realview.c b/hw/arm/realview.c
index 07a80d0de3..f0a8a93b08 100644
--- a/hw/arm/realview.c
+++ b/hw/arm/realview.c
@@ -30,6 +30,15 @@
 #include "hw/i2c/arm_sbcon_i2c.h"
 #include "hw/sd/sd.h"
 
+struct RealviewMachineClass {
+    MachineClass parent_obj;
+};
+typedef struct RealviewMachineClass RealviewMachineClass;
+
+#define TYPE_REALVIEW_MACHINE   MACHINE_TYPE_NAME("realview-common")
+DECLARE_CLASS_CHECKERS(RealviewMachineClass,
+                       REALVIEW_MACHINE, TYPE_REALVIEW_MACHINE)
+
 #define SMP_BOOT_ADDR 0xe0000000
 #define SMP_BOOTREG_ADDR 0x10000030
 
@@ -448,20 +457,25 @@ static void realview_pbx_a9_class_init(ObjectClass *oc, void *data)
 static const TypeInfo realview_machine_types[] = {
     {
         .name           = MACHINE_TYPE_NAME("realview-eb"),
-        .parent         = TYPE_MACHINE,
+        .parent         = TYPE_REALVIEW_MACHINE,
         .class_init     = realview_eb_class_init,
     }, {
         .name           = MACHINE_TYPE_NAME("realview-eb-mpcore"),
-        .parent         = TYPE_MACHINE,
+        .parent         = TYPE_REALVIEW_MACHINE,
         .class_init     = realview_eb_mpcore_class_init,
     }, {
         .name           = MACHINE_TYPE_NAME("realview-pb-a8"),
-        .parent         = TYPE_MACHINE,
+        .parent         = TYPE_REALVIEW_MACHINE,
         .class_init     = realview_pb_a8_class_init,
     }, {
         .name           = MACHINE_TYPE_NAME("realview-pbx-a9"),
-        .parent         = TYPE_MACHINE,
+        .parent         = TYPE_REALVIEW_MACHINE,
         .class_init     = realview_pbx_a9_class_init,
+    }, {
+        .name           = TYPE_REALVIEW_MACHINE,
+        .parent         = TYPE_MACHINE,
+        .class_size     = sizeof(RealviewMachineClass),
+        .abstract       = true,
     }
 };
 
-- 
2.38.1

Re: [PATCH 03/10] hw/arm/realview: Introduce abstract RealviewMachineClass

[Richard Henderson]

On 5/24/23 07:58, Philippe Mathieu-Daudé wrote:
> Introduce the abstract QOM TYPE_REALVIEW_MACHINE to
> handle fields common to all Realview machines.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/arm/realview.c | 22 ++++++++++++++++++----
>   1 file changed, 18 insertions(+), 4 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 04/10] hw/arm/realview: Factor realview_common_class_init() out

[Philippe Mathieu-Daudé]

Introduce realview_common_class_init() where we'll set
fields common to all Realview classes.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/realview.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/hw/arm/realview.c b/hw/arm/realview.c
index f0a8a93b08..6970e8a469 100644
--- a/hw/arm/realview.c
+++ b/hw/arm/realview.c
@@ -410,6 +410,13 @@ static void realview_pbx_a9_init(MachineState *machine)
     realview_init(machine, BOARD_PBX_A9);
 }
 
+static void realview_common_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+
+    mc->ignore_memory_transaction_failures = true;
+}
+
 static void realview_eb_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
@@ -417,7 +424,6 @@ static void realview_eb_class_init(ObjectClass *oc, void *data)
     mc->desc = "ARM RealView Emulation Baseboard (ARM926EJ-S)";
     mc->init = realview_eb_init;
     mc->block_default_type = IF_SCSI;
-    mc->ignore_memory_transaction_failures = true;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("arm926");
 }
 
@@ -429,7 +435,6 @@ static void realview_eb_mpcore_class_init(ObjectClass *oc, void *data)
     mc->init = realview_eb_mpcore_init;
     mc->block_default_type = IF_SCSI;
     mc->max_cpus = 4;
-    mc->ignore_memory_transaction_failures = true;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("arm11mpcore");
 }
 
@@ -439,7 +444,6 @@ static void realview_pb_a8_class_init(ObjectClass *oc, void *data)
 
     mc->desc = "ARM RealView Platform Baseboard for Cortex-A8";
     mc->init = realview_pb_a8_init;
-    mc->ignore_memory_transaction_failures = true;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a8");
 }
 
@@ -450,7 +454,6 @@ static void realview_pbx_a9_class_init(ObjectClass *oc, void *data)
     mc->desc = "ARM RealView Platform Baseboard Explore for Cortex-A9";
     mc->init = realview_pbx_a9_init;
     mc->max_cpus = 4;
-    mc->ignore_memory_transaction_failures = true;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a9");
 }
 
@@ -475,6 +478,7 @@ static const TypeInfo realview_machine_types[] = {
         .name           = TYPE_REALVIEW_MACHINE,
         .parent         = TYPE_MACHINE,
         .class_size     = sizeof(RealviewMachineClass),
+        .class_init     = realview_common_class_init,
         .abstract       = true,
     }
 };
-- 
2.38.1

Re: [PATCH 04/10] hw/arm/realview: Factor realview_common_class_init() out

[Richard Henderson]

On 5/24/23 07:59, Philippe Mathieu-Daudé wrote:
> Introduce realview_common_class_init() where we'll set
> fields common to all Realview classes.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/arm/realview.c | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 05/10] hw/arm/realview: Move 'board_id' to RealviewMachineClass

[Philippe Mathieu-Daudé]

Instead of having each machine instance resolve its board ID,
set it once in their class_init() handler.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/realview.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/hw/arm/realview.c b/hw/arm/realview.c
index 6970e8a469..c354ce0cfa 100644
--- a/hw/arm/realview.c
+++ b/hw/arm/realview.c
@@ -32,6 +32,8 @@
 
 struct RealviewMachineClass {
     MachineClass parent_obj;
+
+    int board_id;
 };
 typedef struct RealviewMachineClass RealviewMachineClass;
 
@@ -49,7 +51,6 @@ static struct arm_boot_info realview_binfo = {
     .smp_bootreg_addr = SMP_BOOTREG_ADDR,
 };
 
-/* The following two lists must be consistent.  */
 enum realview_board_type {
     BOARD_EB,
     BOARD_EB_MPCORE,
@@ -57,13 +58,6 @@ enum realview_board_type {
     BOARD_PBX_A9,
 };
 
-static const int realview_board_id[] = {
-    0x33b,
-    0x33b,
-    0x769,
-    0x76d
-};
-
 static void split_irq_from_named(DeviceState *src, const char* outname,
                                  qemu_irq out1, qemu_irq out2) {
     DeviceState *splitter = qdev_new(TYPE_SPLIT_IRQ);
@@ -81,6 +75,7 @@ static void split_irq_from_named(DeviceState *src, const char* outname,
 static void realview_init(MachineState *machine,
                           enum realview_board_type board_type)
 {
+    RealviewMachineClass *rmc = REALVIEW_MACHINE_GET_CLASS(machine);
     ARMCPU *cpu = NULL;
     CPUARMState *env;
     MemoryRegion *sysmem = get_system_memory();
@@ -385,7 +380,7 @@ static void realview_init(MachineState *machine,
     memory_region_add_subregion(sysmem, SMP_BOOT_ADDR, ram_hack);
 
     realview_binfo.ram_size = ram_size;
-    realview_binfo.board_id = realview_board_id[board_type];
+    realview_binfo.board_id = rmc->board_id;
     realview_binfo.loader_start = (board_type == BOARD_PB_A8 ? 0x70000000 : 0);
     arm_load_kernel(ARM_CPU(first_cpu), machine, &realview_binfo);
 }
@@ -420,41 +415,49 @@ static void realview_common_class_init(ObjectClass *oc, void *data)
 static void realview_eb_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
+    RealviewMachineClass *rmc = REALVIEW_MACHINE_CLASS(oc);
 
     mc->desc = "ARM RealView Emulation Baseboard (ARM926EJ-S)";
     mc->init = realview_eb_init;
     mc->block_default_type = IF_SCSI;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("arm926");
+    rmc->board_id = 0x33b;
 }
 
 static void realview_eb_mpcore_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
+    RealviewMachineClass *rmc = REALVIEW_MACHINE_CLASS(oc);
 
     mc->desc = "ARM RealView Emulation Baseboard (ARM11MPCore)";
     mc->init = realview_eb_mpcore_init;
     mc->block_default_type = IF_SCSI;
     mc->max_cpus = 4;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("arm11mpcore");
+    rmc->board_id = 0x33b;
 }
 
 static void realview_pb_a8_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
+    RealviewMachineClass *rmc = REALVIEW_MACHINE_CLASS(oc);
 
     mc->desc = "ARM RealView Platform Baseboard for Cortex-A8";
     mc->init = realview_pb_a8_init;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a8");
+    rmc->board_id = 0x769;
 }
 
 static void realview_pbx_a9_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
+    RealviewMachineClass *rmc = REALVIEW_MACHINE_CLASS(oc);
 
     mc->desc = "ARM RealView Platform Baseboard Explore for Cortex-A9";
     mc->init = realview_pbx_a9_init;
     mc->max_cpus = 4;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a9");
+    rmc->board_id = 0x76d;
 }
 
 static const TypeInfo realview_machine_types[] = {
-- 
2.38.1

Re: [PATCH 05/10] hw/arm/realview: Move 'board_id' to RealviewMachineClass

[Richard Henderson]

On 5/24/23 07:59, Philippe Mathieu-Daudé wrote:
> Instead of having each machine instance resolve its board ID,
> set it once in their class_init() handler.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/arm/realview.c | 21 ++++++++++++---------
>   1 file changed, 12 insertions(+), 9 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 06/10] hw/arm/realview: Move 'is_pb' to RealviewMachineClass

[Philippe Mathieu-Daudé]

Instead of having each machine instance set whether EP/PB,
set it once in their class_init() handler.

Arguably this could be extracted from the board_id field.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/realview.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/arm/realview.c b/hw/arm/realview.c
index c354ce0cfa..1a3a64e0ce 100644
--- a/hw/arm/realview.c
+++ b/hw/arm/realview.c
@@ -34,6 +34,7 @@ struct RealviewMachineClass {
     MachineClass parent_obj;
 
     int board_id;
+    bool is_pb;
 };
 typedef struct RealviewMachineClass RealviewMachineClass;
 
@@ -94,7 +95,7 @@ static void realview_init(MachineState *machine,
     unsigned int smp_cpus = machine->smp.cpus;
     qemu_irq cpu_irq[4];
     int is_mpcore = 0;
-    int is_pb = 0;
+    bool is_pb = rmc->is_pb;
     uint32_t proc_id = 0;
     uint32_t sys_id;
     ram_addr_t low_ram_size;
@@ -109,11 +110,9 @@ static void realview_init(MachineState *machine,
         periphbase = 0x10100000;
         break;
     case BOARD_PB_A8:
-        is_pb = 1;
         break;
     case BOARD_PBX_A9:
         is_mpcore = 1;
-        is_pb = 1;
         periphbase = 0x1f000000;
         break;
     }
@@ -446,6 +445,7 @@ static void realview_pb_a8_class_init(ObjectClass *oc, void *data)
     mc->init = realview_pb_a8_init;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a8");
     rmc->board_id = 0x769;
+    rmc->is_pb = true;
 }
 
 static void realview_pbx_a9_class_init(ObjectClass *oc, void *data)
@@ -458,6 +458,7 @@ static void realview_pbx_a9_class_init(ObjectClass *oc, void *data)
     mc->max_cpus = 4;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a9");
     rmc->board_id = 0x76d;
+    rmc->is_pb = true;
 }
 
 static const TypeInfo realview_machine_types[] = {
-- 
2.38.1

Re: [PATCH 06/10] hw/arm/realview: Move 'is_pb' to RealviewMachineClass

[Richard Henderson]

On 5/24/23 07:59, Philippe Mathieu-Daudé wrote:
> Instead of having each machine instance set whether EP/PB,
> set it once in their class_init() handler.
> 
> Arguably this could be extracted from the board_id field.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/arm/realview.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 07/10] hw/arm/realview: Move 'mpcore_periphbase' to RealviewMachineClass

[Philippe Mathieu-Daudé]

Instead of having each machine instance resolve whether it is
MPCORE based and its peripheral base address, set it once in
their class_init() handler.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/realview.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/hw/arm/realview.c b/hw/arm/realview.c
index 1a3a64e0ce..8a3b5a48b1 100644
--- a/hw/arm/realview.c
+++ b/hw/arm/realview.c
@@ -35,6 +35,7 @@ struct RealviewMachineClass {
 
     int board_id;
     bool is_pb;
+    hwaddr mpcore_periphbase;
 };
 typedef struct RealviewMachineClass RealviewMachineClass;
 
@@ -94,26 +95,21 @@ static void realview_init(MachineState *machine,
     int n;
     unsigned int smp_cpus = machine->smp.cpus;
     qemu_irq cpu_irq[4];
-    int is_mpcore = 0;
+    bool is_mpcore = rmc->mpcore_periphbase != 0;
     bool is_pb = rmc->is_pb;
     uint32_t proc_id = 0;
     uint32_t sys_id;
     ram_addr_t low_ram_size;
     ram_addr_t ram_size = machine->ram_size;
-    hwaddr periphbase = 0;
 
     switch (board_type) {
     case BOARD_EB:
         break;
     case BOARD_EB_MPCORE:
-        is_mpcore = 1;
-        periphbase = 0x10100000;
         break;
     case BOARD_PB_A8:
         break;
     case BOARD_PBX_A9:
-        is_mpcore = 1;
-        periphbase = 0x1f000000;
         break;
     }
 
@@ -129,8 +125,8 @@ static void realview_init(MachineState *machine,
         }
 
         if (is_pb && is_mpcore) {
-            object_property_set_int(cpuobj, "reset-cbar", periphbase,
-                                    &error_fatal);
+            object_property_set_int(cpuobj, "reset-cbar",
+                                    rmc->mpcore_periphbase, &error_fatal);
         }
 
         qdev_realize(DEVICE(cpuobj), NULL, &error_fatal);
@@ -191,13 +187,13 @@ static void realview_init(MachineState *machine,
         qdev_prop_set_uint32(dev, "num-cpu", smp_cpus);
         busdev = SYS_BUS_DEVICE(dev);
         sysbus_realize_and_unref(busdev, &error_fatal);
-        sysbus_mmio_map(busdev, 0, periphbase);
+        sysbus_mmio_map(busdev, 0, rmc->mpcore_periphbase);
         for (n = 0; n < smp_cpus; n++) {
             sysbus_connect_irq(busdev, n, cpu_irq[n]);
         }
-        sysbus_create_varargs("l2x0", periphbase + 0x2000, NULL);
+        sysbus_create_varargs("l2x0", rmc->mpcore_periphbase + 0x2000, NULL);
         /* Both A9 and 11MPCore put the GIC CPU i/f at base + 0x100 */
-        realview_binfo.gic_cpu_if_addr = periphbase + 0x100;
+        realview_binfo.gic_cpu_if_addr = rmc->mpcore_periphbase + 0x100;
     } else {
         uint32_t gic_addr = is_pb ? 0x1e000000 : 0x10040000;
         /* For now just create the nIRQ GIC, and ignore the others.  */
@@ -434,6 +430,7 @@ static void realview_eb_mpcore_class_init(ObjectClass *oc, void *data)
     mc->max_cpus = 4;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("arm11mpcore");
     rmc->board_id = 0x33b;
+    rmc->mpcore_periphbase = 0x10100000;
 }
 
 static void realview_pb_a8_class_init(ObjectClass *oc, void *data)
@@ -459,6 +456,7 @@ static void realview_pbx_a9_class_init(ObjectClass *oc, void *data)
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a9");
     rmc->board_id = 0x76d;
     rmc->is_pb = true;
+    rmc->mpcore_periphbase = 0x1f000000;
 }
 
 static const TypeInfo realview_machine_types[] = {
-- 
2.38.1

Re: [PATCH 07/10] hw/arm/realview: Move 'mpcore_periphbase' to RealviewMachineClass

[Richard Henderson]

On 5/24/23 07:59, Philippe Mathieu-Daudé wrote:
> -    int is_mpcore = 0;
> +    bool is_mpcore = rmc->mpcore_periphbase != 0;
>       bool is_pb = rmc->is_pb;
>       uint32_t proc_id = 0;
>       uint32_t sys_id;
>       ram_addr_t low_ram_size;
>       ram_addr_t ram_size = machine->ram_size;
> -    hwaddr periphbase = 0;

hwaddr periphbase = rmc->mpcore_periphbase;
bool is_mpcore = periphbase != 0;

would be cleaner and require fewer changes.

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

[PATCH 08/10] hw/arm/realview: Move 'loader_start' to RealviewMachineClass

[Philippe Mathieu-Daudé]

Instead of having each machine instance resolve its loader
start address, set it once in their class_init() handler.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/realview.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/arm/realview.c b/hw/arm/realview.c
index 8a3b5a48b1..917a1b22df 100644
--- a/hw/arm/realview.c
+++ b/hw/arm/realview.c
@@ -36,6 +36,7 @@ struct RealviewMachineClass {
     int board_id;
     bool is_pb;
     hwaddr mpcore_periphbase;
+    hwaddr loader_start;
 };
 typedef struct RealviewMachineClass RealviewMachineClass;
 
@@ -376,7 +377,7 @@ static void realview_init(MachineState *machine,
 
     realview_binfo.ram_size = ram_size;
     realview_binfo.board_id = rmc->board_id;
-    realview_binfo.loader_start = (board_type == BOARD_PB_A8 ? 0x70000000 : 0);
+    realview_binfo.loader_start = rmc->loader_start;
     arm_load_kernel(ARM_CPU(first_cpu), machine, &realview_binfo);
 }
 
@@ -443,6 +444,7 @@ static void realview_pb_a8_class_init(ObjectClass *oc, void *data)
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a8");
     rmc->board_id = 0x769;
     rmc->is_pb = true;
+    rmc->loader_start = 0x70000000;
 }
 
 static void realview_pbx_a9_class_init(ObjectClass *oc, void *data)
-- 
2.38.1

Re: [PATCH 08/10] hw/arm/realview: Move 'loader_start' to RealviewMachineClass

[Richard Henderson]

On 5/24/23 07:59, Philippe Mathieu-Daudé wrote:
> Instead of having each machine instance resolve its loader
> start address, set it once in their class_init() handler.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/arm/realview.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

[PATCH 09/10] hw/arm/realview: Use generic realview_common_machine_init()

[Philippe Mathieu-Daudé]

The realview_board_type enum is now unused. Remove it and have
all instances use the common realview_common_machine_init()
method.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/realview.c | 46 ++--------------------------------------------
 1 file changed, 2 insertions(+), 44 deletions(-)

diff --git a/hw/arm/realview.c b/hw/arm/realview.c
index 917a1b22df..b7bb384000 100644
--- a/hw/arm/realview.c
+++ b/hw/arm/realview.c
@@ -54,13 +54,6 @@ static struct arm_boot_info realview_binfo = {
     .smp_bootreg_addr = SMP_BOOTREG_ADDR,
 };
 
-enum realview_board_type {
-    BOARD_EB,
-    BOARD_EB_MPCORE,
-    BOARD_PB_A8,
-    BOARD_PBX_A9,
-};
-
 static void split_irq_from_named(DeviceState *src, const char* outname,
                                  qemu_irq out1, qemu_irq out2) {
     DeviceState *splitter = qdev_new(TYPE_SPLIT_IRQ);
@@ -75,8 +68,7 @@ static void split_irq_from_named(DeviceState *src, const char* outname,
                                 qdev_get_gpio_in(splitter, 0));
 }
 
-static void realview_init(MachineState *machine,
-                          enum realview_board_type board_type)
+static void realview_common_machine_init(MachineState *machine)
 {
     RealviewMachineClass *rmc = REALVIEW_MACHINE_GET_CLASS(machine);
     ARMCPU *cpu = NULL;
@@ -103,17 +95,6 @@ static void realview_init(MachineState *machine,
     ram_addr_t low_ram_size;
     ram_addr_t ram_size = machine->ram_size;
 
-    switch (board_type) {
-    case BOARD_EB:
-        break;
-    case BOARD_EB_MPCORE:
-        break;
-    case BOARD_PB_A8:
-        break;
-    case BOARD_PBX_A9:
-        break;
-    }
-
     for (n = 0; n < smp_cpus; n++) {
         Object *cpuobj = object_new(machine->cpu_type);
 
@@ -381,30 +362,11 @@ static void realview_init(MachineState *machine,
     arm_load_kernel(ARM_CPU(first_cpu), machine, &realview_binfo);
 }
 
-static void realview_eb_init(MachineState *machine)
-{
-    realview_init(machine, BOARD_EB);
-}
-
-static void realview_eb_mpcore_init(MachineState *machine)
-{
-    realview_init(machine, BOARD_EB_MPCORE);
-}
-
-static void realview_pb_a8_init(MachineState *machine)
-{
-    realview_init(machine, BOARD_PB_A8);
-}
-
-static void realview_pbx_a9_init(MachineState *machine)
-{
-    realview_init(machine, BOARD_PBX_A9);
-}
-
 static void realview_common_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
 
+    mc->init = realview_common_machine_init;
     mc->ignore_memory_transaction_failures = true;
 }
 
@@ -414,7 +376,6 @@ static void realview_eb_class_init(ObjectClass *oc, void *data)
     RealviewMachineClass *rmc = REALVIEW_MACHINE_CLASS(oc);
 
     mc->desc = "ARM RealView Emulation Baseboard (ARM926EJ-S)";
-    mc->init = realview_eb_init;
     mc->block_default_type = IF_SCSI;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("arm926");
     rmc->board_id = 0x33b;
@@ -426,7 +387,6 @@ static void realview_eb_mpcore_class_init(ObjectClass *oc, void *data)
     RealviewMachineClass *rmc = REALVIEW_MACHINE_CLASS(oc);
 
     mc->desc = "ARM RealView Emulation Baseboard (ARM11MPCore)";
-    mc->init = realview_eb_mpcore_init;
     mc->block_default_type = IF_SCSI;
     mc->max_cpus = 4;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("arm11mpcore");
@@ -440,7 +400,6 @@ static void realview_pb_a8_class_init(ObjectClass *oc, void *data)
     RealviewMachineClass *rmc = REALVIEW_MACHINE_CLASS(oc);
 
     mc->desc = "ARM RealView Platform Baseboard for Cortex-A8";
-    mc->init = realview_pb_a8_init;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a8");
     rmc->board_id = 0x769;
     rmc->is_pb = true;
@@ -453,7 +412,6 @@ static void realview_pbx_a9_class_init(ObjectClass *oc, void *data)
     RealviewMachineClass *rmc = REALVIEW_MACHINE_CLASS(oc);
 
     mc->desc = "ARM RealView Platform Baseboard Explore for Cortex-A9";
-    mc->init = realview_pbx_a9_init;
     mc->max_cpus = 4;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a9");
     rmc->board_id = 0x76d;
-- 
2.38.1

Re: [PATCH 09/10] hw/arm/realview: Use generic realview_common_machine_init()

[Richard Henderson]

On 5/24/23 07:59, Philippe Mathieu-Daudé wrote:
> The realview_board_type enum is now unused. Remove it and have
> all instances use the common realview_common_machine_init()
> method.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/arm/realview.c | 46 ++--------------------------------------------
>   1 file changed, 2 insertions(+), 44 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 10/10] hw/arm/realview: Set MachineClass::default_nic in machine_class_init()

[Philippe Mathieu-Daudé]

Mark the default NIC via the new MachineClass->default_nic setting
so that the machine-defaults code in vl.c can decide whether the
default NIC is usable or not (for example when compiling with the
"--without-default-devices" configure switch).

Inspired-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/realview.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/arm/realview.c b/hw/arm/realview.c
index b7bb384000..0b2d6b82c8 100644
--- a/hw/arm/realview.c
+++ b/hw/arm/realview.c
@@ -70,6 +70,7 @@ static void split_irq_from_named(DeviceState *src, const char* outname,
 
 static void realview_common_machine_init(MachineState *machine)
 {
+    MachineClass *mc = MACHINE_GET_CLASS(machine);
     RealviewMachineClass *rmc = REALVIEW_MACHINE_GET_CLASS(machine);
     ARMCPU *cpu = NULL;
     CPUARMState *env;
@@ -274,7 +275,7 @@ static void realview_common_machine_init(MachineState *machine)
     for(n = 0; n < nb_nics; n++) {
         nd = &nd_table[n];
 
-        if (!nd->model || strcmp(nd->model, is_pb ? "lan9118" : "smc91c111") == 0) {
+        if (!nd->model || strcmp(nd->model, mc->default_nic) == 0) {
             if (is_pb) {
                 lan9118_init(nd, 0x4e000000, pic[28]);
             } else {
@@ -378,6 +379,7 @@ static void realview_eb_class_init(ObjectClass *oc, void *data)
     mc->desc = "ARM RealView Emulation Baseboard (ARM926EJ-S)";
     mc->block_default_type = IF_SCSI;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("arm926");
+    mc->default_nic = "smc91c111";
     rmc->board_id = 0x33b;
 }
 
@@ -390,6 +392,7 @@ static void realview_eb_mpcore_class_init(ObjectClass *oc, void *data)
     mc->block_default_type = IF_SCSI;
     mc->max_cpus = 4;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("arm11mpcore");
+    mc->default_nic = "smc91c111";
     rmc->board_id = 0x33b;
     rmc->mpcore_periphbase = 0x10100000;
 }
@@ -401,6 +404,7 @@ static void realview_pb_a8_class_init(ObjectClass *oc, void *data)
 
     mc->desc = "ARM RealView Platform Baseboard for Cortex-A8";
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a8");
+    mc->default_nic = "lan9118";
     rmc->board_id = 0x769;
     rmc->is_pb = true;
     rmc->loader_start = 0x70000000;
@@ -414,6 +418,7 @@ static void realview_pbx_a9_class_init(ObjectClass *oc, void *data)
     mc->desc = "ARM RealView Platform Baseboard Explore for Cortex-A9";
     mc->max_cpus = 4;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a9");
+    mc->default_nic = "lan9118";
     rmc->board_id = 0x76d;
     rmc->is_pb = true;
     rmc->mpcore_periphbase = 0x1f000000;
-- 
2.38.1

Re: [PATCH 10/10] hw/arm/realview: Set MachineClass::default_nic in machine_class_init()

[Richard Henderson]

On 5/24/23 07:59, Philippe Mathieu-Daudé wrote:
> Mark the default NIC via the new MachineClass->default_nic setting
> so that the machine-defaults code in vl.c can decide whether the
> default NIC is usable or not (for example when compiling with the
> "--without-default-devices" configure switch).
> 
> Inspired-by: Thomas Huth<thuth@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/arm/realview.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~