diff mbox series

[9/9] hw/arm/armv7m: Pass CPU/NVIC using object_property_add_const_link()

Message ID 20230206121714.85084-10-philmd@linaro.org
State New
Headers show
Series target/arm: Housekeeping around NVIC | expand

Commit Message

Philippe Mathieu-Daudé Feb. 6, 2023, 12:17 p.m. UTC 
Avoid having QOM objects poke at each other internals.
Instead, pass references using QOM link properties.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/arm/armv7m.c       | 4 ++--
 hw/intc/armv7m_nvic.c | 3 ++-
 target/arm/cpu.c      | 3 ++-
 3 files changed, 6 insertions(+), 4 deletions(-)

Comments

Philippe Mathieu-Daudé Feb. 6, 2023, 2:35 p.m. UTC | #1 
On 6/2/23 13:17, Philippe Mathieu-Daudé wrote:
> Avoid having QOM objects poke at each other internals.
> Instead, pass references using QOM link properties.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   hw/arm/armv7m.c       | 4 ++--
>   hw/intc/armv7m_nvic.c | 3 ++-
>   target/arm/cpu.c      | 3 ++-
>   3 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
> index 50a9507c0b..edde774da2 100644
> --- a/hw/arm/armv7m.c
> +++ b/hw/arm/armv7m.c
> @@ -338,8 +338,8 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
>        * Tell the CPU where the NVIC is; it will fail realize if it doesn't
>        * have one. Similarly, tell the NVIC where its CPU is.
>        */
> -    s->cpu->env.nvic = &s->nvic;
> -    s->nvic.cpu = s->cpu;
> +    object_property_add_const_link(OBJECT(s->cpu), "nvic", OBJECT(&s->nvic));
> +    object_property_add_const_link(OBJECT(&s->nvic), "cpu", OBJECT(s->cpu));

Markus mentioned this converts build time types checks to runtime.
Also harder to debug. And the code becomes harder to read.

>       if (!qdev_realize(DEVICE(s->cpu), NULL, errp)) {
>           return;
> diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
> index d9c7e414bc..e43898a9e0 100644
> --- a/hw/intc/armv7m_nvic.c
> +++ b/hw/intc/armv7m_nvic.c
> @@ -2668,7 +2668,8 @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp)
>       NVICState *s = NVIC(dev);
>   
>       /* The armv7m container object will have set our CPU pointer */
> -    if (!s->cpu || !arm_feature(&s->cpu->env, ARM_FEATURE_M)) {
> +    s->cpu = ARM_CPU(object_property_get_link(OBJECT(dev), "cpu", &error_abort));
> +    if (!arm_feature(&s->cpu->env, ARM_FEATURE_M)) {
>           error_setg(errp, "The NVIC can only be used with a Cortex-M CPU");
>           return;
>       }
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 876ab8f3bf..f081861947 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1573,12 +1573,13 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
>        * error and will result in segfaults if not caught here.
>        */
>       if (arm_feature(env, ARM_FEATURE_M)) {
> +        env->nvic = NVIC(object_property_get_link(OBJECT(dev), "nvic", NULL));
>           if (!env->nvic) {
>               error_setg(errp, "This board cannot be used with Cortex-M CPUs");
>               return;
>           }
>       } else {
> -        if (env->nvic) {
> +        if (object_property_find(OBJECT(dev), "nvic")) {
>               error_setg(errp, "This board can only be used with Cortex-M CPUs");
>               return;
>           }
diff mbox series

Patch

diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index 50a9507c0b..edde774da2 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -338,8 +338,8 @@  static void armv7m_realize(DeviceState *dev, Error **errp)
      * Tell the CPU where the NVIC is; it will fail realize if it doesn't
      * have one. Similarly, tell the NVIC where its CPU is.
      */
-    s->cpu->env.nvic = &s->nvic;
-    s->nvic.cpu = s->cpu;
+    object_property_add_const_link(OBJECT(s->cpu), "nvic", OBJECT(&s->nvic));
+    object_property_add_const_link(OBJECT(&s->nvic), "cpu", OBJECT(s->cpu));
 
     if (!qdev_realize(DEVICE(s->cpu), NULL, errp)) {
         return;
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index d9c7e414bc..e43898a9e0 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -2668,7 +2668,8 @@  static void armv7m_nvic_realize(DeviceState *dev, Error **errp)
     NVICState *s = NVIC(dev);
 
     /* The armv7m container object will have set our CPU pointer */
-    if (!s->cpu || !arm_feature(&s->cpu->env, ARM_FEATURE_M)) {
+    s->cpu = ARM_CPU(object_property_get_link(OBJECT(dev), "cpu", &error_abort));
+    if (!arm_feature(&s->cpu->env, ARM_FEATURE_M)) {
         error_setg(errp, "The NVIC can only be used with a Cortex-M CPU");
         return;
     }
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 876ab8f3bf..f081861947 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1573,12 +1573,13 @@  static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
      * error and will result in segfaults if not caught here.
      */
     if (arm_feature(env, ARM_FEATURE_M)) {
+        env->nvic = NVIC(object_property_get_link(OBJECT(dev), "nvic", NULL));
         if (!env->nvic) {
             error_setg(errp, "This board cannot be used with Cortex-M CPUs");
             return;
         }
     } else {
-        if (env->nvic) {
+        if (object_property_find(OBJECT(dev), "nvic")) {
             error_setg(errp, "This board can only be used with Cortex-M CPUs");
             return;
         }