[RFC PATCH-for-9.0 09/11] hw/arm/bcm2836: Allocate ARM CPU state with object_new()

Philippe Mathieu-Daudé posted 11 patches 1 year ago
Maintainers: Peter Maydell <peter.maydell@linaro.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Eduardo Habkost <eduardo@habkost.net>
[RFC PATCH-for-9.0 09/11] hw/arm/bcm2836: Allocate ARM CPU state with object_new()
Posted by Philippe Mathieu-Daudé 1 year ago
The ARMCPU type is forward declared as a pointer to all hw/ files.
Its declaration is restricted to target/arm/ files. By using a
pointer in BCM283XState instead of embedding the whole CPU state,
we don't need to include "cpu.h" which is target-specific.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/arm/bcm2836.h |  4 ++--
 hw/arm/bcm2836.c         | 19 ++++++++++---------
 hw/arm/raspi.c           |  2 +-
 3 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/include/hw/arm/bcm2836.h b/include/hw/arm/bcm2836.h
index 6f90cabfa3..784bab0aad 100644
--- a/include/hw/arm/bcm2836.h
+++ b/include/hw/arm/bcm2836.h
@@ -14,7 +14,7 @@
 
 #include "hw/arm/bcm2835_peripherals.h"
 #include "hw/intc/bcm2836_control.h"
-#include "target/arm/cpu.h"
+#include "target/arm/cpu-qom.h"
 #include "qom/object.h"
 
 #define TYPE_BCM283X "bcm283x"
@@ -38,7 +38,7 @@ struct BCM283XState {
     uint32_t enabled_cpus;
 
     struct {
-        ARMCPU core;
+        ARMCPU *core;
     } cpu[BCM283X_NCPUS];
     BCM2836ControlState control;
     BCM2835PeripheralState peripherals;
diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
index 8031a74600..4f5acee77e 100644
--- a/hw/arm/bcm2836.c
+++ b/hw/arm/bcm2836.c
@@ -39,8 +39,9 @@ static void bcm2836_init(Object *obj)
     int n;
 
     for (n = 0; n < bc->core_count; n++) {
-        object_initialize_child(obj, "cpu[*]", &s->cpu[n].core,
-                                bc->cpu_type);
+        s->cpu[n].core = ARM_CPU(object_new(bc->cpu_type));
+        object_property_add_child(obj, "cpu[*]", OBJECT(s->cpu[n].core));
+        qdev_realize_and_unref(DEVICE(s->cpu[n].core), NULL, &error_abort);
     }
     if (bc->core_count > 1) {
         qdev_property_add_static(DEVICE(obj), &bcm2836_enabled_cores_property);
@@ -139,24 +140,24 @@ static void bcm2836_realize(DeviceState *dev, Error **errp)
         object_property_set_bool(OBJECT(&s->cpu[n].core), "start-powered-off",
                                  n >= s->enabled_cpus, &error_abort);
 
-        if (!qdev_realize(DEVICE(&s->cpu[n].core), NULL, errp)) {
+        if (!qdev_realize(DEVICE(s->cpu[n].core), NULL, errp)) {
             return;
         }
 
         /* Connect irq/fiq outputs from the interrupt controller. */
         qdev_connect_gpio_out_named(DEVICE(&s->control), "irq", n,
-                qdev_get_gpio_in(DEVICE(&s->cpu[n].core), ARM_CPU_IRQ));
+                qdev_get_gpio_in(DEVICE(s->cpu[n].core), ARM_CPU_IRQ));
         qdev_connect_gpio_out_named(DEVICE(&s->control), "fiq", n,
-                qdev_get_gpio_in(DEVICE(&s->cpu[n].core), ARM_CPU_FIQ));
+                qdev_get_gpio_in(DEVICE(s->cpu[n].core), ARM_CPU_FIQ));
 
         /* Connect timers from the CPU to the interrupt controller */
-        qdev_connect_gpio_out(DEVICE(&s->cpu[n].core), GTIMER_PHYS,
+        qdev_connect_gpio_out(DEVICE(s->cpu[n].core), GTIMER_PHYS,
                 qdev_get_gpio_in_named(DEVICE(&s->control), "cntpnsirq", n));
-        qdev_connect_gpio_out(DEVICE(&s->cpu[n].core), GTIMER_VIRT,
+        qdev_connect_gpio_out(DEVICE(s->cpu[n].core), GTIMER_VIRT,
                 qdev_get_gpio_in_named(DEVICE(&s->control), "cntvirq", n));
-        qdev_connect_gpio_out(DEVICE(&s->cpu[n].core), GTIMER_HYP,
+        qdev_connect_gpio_out(DEVICE(s->cpu[n].core), GTIMER_HYP,
                 qdev_get_gpio_in_named(DEVICE(&s->control), "cnthpirq", n));
-        qdev_connect_gpio_out(DEVICE(&s->cpu[n].core), GTIMER_SEC,
+        qdev_connect_gpio_out(DEVICE(s->cpu[n].core), GTIMER_SEC,
                 qdev_get_gpio_in_named(DEVICE(&s->control), "cntpsirq", n));
     }
 }
diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index cc4c4ec9bf..01c391b90a 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -252,7 +252,7 @@ static void setup_boot(MachineState *machine, RaspiProcessorId processor_id,
         s->binfo.firmware_loaded = true;
     }
 
-    arm_load_kernel(&s->soc.cpu[0].core, machine, &s->binfo);
+    arm_load_kernel(s->soc.cpu[0].core, machine, &s->binfo);
 }
 
 static void raspi_machine_init(MachineState *machine)
-- 
2.41.0


Re: [RFC PATCH-for-9.0 09/11] hw/arm/bcm2836: Allocate ARM CPU state with object_new()
Posted by Richard Henderson 12 months ago
On 11/22/23 12:30, Philippe Mathieu-Daudé wrote:
> The ARMCPU type is forward declared as a pointer to all hw/ files.
> Its declaration is restricted to target/arm/ files. By using a
> pointer in BCM283XState instead of embedding the whole CPU state,
> we don't need to include "cpu.h" which is target-specific.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/hw/arm/bcm2836.h |  4 ++--
>   hw/arm/bcm2836.c         | 19 ++++++++++---------
>   hw/arm/raspi.c           |  2 +-
>   3 files changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/include/hw/arm/bcm2836.h b/include/hw/arm/bcm2836.h
> index 6f90cabfa3..784bab0aad 100644
> --- a/include/hw/arm/bcm2836.h
> +++ b/include/hw/arm/bcm2836.h
> @@ -14,7 +14,7 @@
>   
>   #include "hw/arm/bcm2835_peripherals.h"
>   #include "hw/intc/bcm2836_control.h"
> -#include "target/arm/cpu.h"
> +#include "target/arm/cpu-qom.h"
>   #include "qom/object.h"
>   
>   #define TYPE_BCM283X "bcm283x"
> @@ -38,7 +38,7 @@ struct BCM283XState {
>       uint32_t enabled_cpus;
>   
>       struct {
> -        ARMCPU core;
> +        ARMCPU *core;
>       } cpu[BCM283X_NCPUS];

I'd be tempted to drop the unused struct:

     ARMCPU *cpu[BCM283X_NCPUS];

while you're at it.  Anyway,

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


r~