Re: [PATCH v2 09/14] hw/arm: Prefer arm_feature(EL3) over object_property_find(has_el3)

[Philippe Mathieu-Daudé]

On 9/1/24 19:13, Philippe Mathieu-Daudé wrote:
> On 9/1/24 19:09, Philippe Mathieu-Daudé wrote:
> > The "has_el3" property is added to ARMCPU when the
> > ARM_FEATURE_EL3 feature is available. Rather than
> > checking whether the QOM property is present, directly
> > check the feature.
> >
> > Suggested-by: Markus Armbruster <armbru@redhat.com>
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> >   hw/arm/exynos4210.c   |  4 ++--
> >   hw/arm/integratorcp.c |  5 ++---
> >   hw/arm/realview.c     |  2 +-
> >   hw/arm/versatilepb.c  |  5 ++---
> >   hw/arm/xilinx_zynq.c  |  2 +-
> >   hw/cpu/a15mpcore.c    | 11 +++++++----
> >   hw/cpu/a9mpcore.c     |  6 +++---
> >   7 files changed, 18 insertions(+), 17 deletions(-)
>
>
> > diff --git a/hw/cpu/a9mpcore.c b/hw/cpu/a9mpcore.c
> > index d03f57e579..9355e8443b 100644
> > --- a/hw/cpu/a9mpcore.c
> > +++ b/hw/cpu/a9mpcore.c
> > @@ -52,7 +52,6 @@ static void a9mp_priv_realize(DeviceState *dev, 
> > Error **errp)
> >       SysBusDevice *scubusdev, *gicbusdev, *gtimerbusdev, *mptimerbusdev,
> >                    *wdtbusdev;
> >       int i;
> > -    bool has_el3;
> >       CPUState *cpu0;
> >       Object *cpuobj;
> > @@ -81,9 +80,10 @@ static void a9mp_priv_realize(DeviceState *dev, 
> > Error **errp)
> >       /* Make the GIC's TZ support match the CPUs. We assume that
> >        * either all the CPUs have TZ, or none do.
> >        */
> > -    has_el3 = object_property_find(cpuobj, "has_el3") &&
> > +    if (arm_feature(cpu_env(cpu0), ARM_FEATURE_EL3)) {
> >           object_property_get_bool(cpuobj, "has_el3", &error_abort);
>
> Oops, something is wrong here...

This should be:

-- >8 --
@@ -84,3 +83,5 @@ static void a9mp_priv_realize(DeviceState *dev, Error 
**errp)
-    has_el3 = object_property_find(cpuobj, "has_el3") &&
-        object_property_get_bool(cpuobj, "has_el3", &error_abort);
-    qdev_prop_set_bit(gicdev, "has-security-extensions", has_el3);
+    if (arm_feature(cpu_env(cpu0), ARM_FEATURE_EL3)) {
+        qdev_prop_set_bit(gicdev, "has-security-extensions",
+                          object_property_get_bool(cpuobj, "has_el3",
+                                                   &error_abort));
+    }
---

> > -    qdev_prop_set_bit(gicdev, "has-security-extensions", has_el3);
> > +        qdev_prop_set_bit(gicdev, "has-security-extensions", true);
> > +    }
> >       if (!sysbus_realize(SYS_BUS_DEVICE(&s->gic), errp)) {
> >           return;