| Message ID | 20240109180930.90793-10-philmd@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | hw/arm: Prefer arm_feature() over object_property_find() | expand |
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... > - 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;
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; >
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/a15mpcore.c b/hw/cpu/a15mpcore.c > index bfd8aa5644..cebfe142cf 100644 > --- a/hw/cpu/a15mpcore.c > +++ b/hw/cpu/a15mpcore.c > @@ -53,7 +53,6 @@ static void a15mp_priv_realize(DeviceState *dev, Error **errp) > DeviceState *gicdev; > SysBusDevice *busdev; > int i; > - bool has_el3; > bool has_el2 = false; > Object *cpuobj; > > @@ -62,13 +61,17 @@ static void a15mp_priv_realize(DeviceState *dev, Error **errp) > qdev_prop_set_uint32(gicdev, "num-irq", s->num_irq); > > if (!kvm_irqchip_in_kernel()) { > + CPUState *cpu; > + > /* Make the GIC's TZ support match the CPUs. We assume that > * either all the CPUs have TZ, or none do. > */ > - cpuobj = OBJECT(qemu_get_cpu(0)); > - has_el3 = object_property_find(cpuobj, "has_el3") && > + cpu = qemu_get_cpu(0); > + cpuobj = OBJECT(cpu); > + if (arm_feature(cpu_env(cpu), ARM_FEATURE_EL3)) { > object_property_get_bool(cpuobj, "has_el3", &error_abort); This requires the same change than a9mp_priv_realize(), so squashing: -- >8 -- if (arm_feature(cpu_env(cpu), ARM_FEATURE_EL3)) { - object_property_get_bool(cpuobj, "has_el3", &error_abort); - qdev_prop_set_bit(gicdev, "has-security-extensions", true); + 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); > + } > /* Similarly for virtualization support */ > has_el2 = object_property_find(cpuobj, "has_el2") && > object_property_get_bool(cpuobj, "has_el2", &error_abort);
diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c index de39fb0ece..5efaa538cd 100644 --- a/hw/arm/exynos4210.c +++ b/hw/arm/exynos4210.c @@ -554,14 +554,14 @@ static void exynos4210_realize(DeviceState *socdev, Error **errp) for (n = 0; n < EXYNOS4210_NCPUS; n++) { Object *cpuobj = object_new(ARM_CPU_TYPE_NAME("cortex-a9")); + s->cpu[n] = ARM_CPU(cpuobj); /* By default A9 CPUs have EL3 enabled. This board does not currently * support EL3 so the CPU EL3 property is disabled before realization. */ - if (object_property_find(cpuobj, "has_el3")) { + if (arm_feature(&s->cpu[n]->env, ARM_FEATURE_EL3)) { object_property_set_bool(cpuobj, "has_el3", false, &error_fatal); } - s->cpu[n] = ARM_CPU(cpuobj); object_property_set_int(cpuobj, "mp-affinity", exynos4210_calc_affinity(n), &error_abort); object_property_set_int(cpuobj, "reset-cbar", diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c index 1830e1d785..7685527eb2 100644 --- a/hw/arm/integratorcp.c +++ b/hw/arm/integratorcp.c @@ -596,19 +596,18 @@ static void integratorcp_init(MachineState *machine) int i; cpuobj = object_new(machine->cpu_type); + cpu = ARM_CPU(cpuobj); /* By default ARM1176 CPUs have EL3 enabled. This board does not * currently support EL3 so the CPU EL3 property is disabled before * realization. */ - if (object_property_find(cpuobj, "has_el3")) { + if (arm_feature(&cpu->env, ARM_FEATURE_EL3)) { object_property_set_bool(cpuobj, "has_el3", false, &error_fatal); } qdev_realize(DEVICE(cpuobj), NULL, &error_fatal); - cpu = ARM_CPU(cpuobj); - /* ??? On a real system the first 1Mb is mapped as SSRAM or boot flash. */ /* ??? RAM should repeat to fill physical memory space. */ /* SDRAM at address zero*/ diff --git a/hw/arm/realview.c b/hw/arm/realview.c index 132217b2ed..433fe72ced 100644 --- a/hw/arm/realview.c +++ b/hw/arm/realview.c @@ -123,7 +123,7 @@ static void realview_init(MachineState *machine, * does not currently support EL3 so the CPU EL3 property is disabled * before realization. */ - if (object_property_find(cpuobj, "has_el3")) { + if (arm_feature(&cpu->env, ARM_FEATURE_EL3)) { object_property_set_bool(cpuobj, "has_el3", false, &error_fatal); } diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c index 4b2257787b..1969bb4608 100644 --- a/hw/arm/versatilepb.c +++ b/hw/arm/versatilepb.c @@ -208,19 +208,18 @@ static void versatile_init(MachineState *machine, int board_id) } cpuobj = object_new(machine->cpu_type); + cpu = ARM_CPU(cpuobj); /* By default ARM1176 CPUs have EL3 enabled. This board does not * currently support EL3 so the CPU EL3 property is disabled before * realization. */ - if (object_property_find(cpuobj, "has_el3")) { + if (arm_feature(&cpu->env, ARM_FEATURE_EL3)) { object_property_set_bool(cpuobj, "has_el3", false, &error_fatal); } qdev_realize(DEVICE(cpuobj), NULL, &error_fatal); - cpu = ARM_CPU(cpuobj); - /* ??? RAM should repeat to fill physical memory space. */ /* SDRAM at address zero. */ memory_region_add_subregion(sysmem, 0, machine->ram); diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c index dbb9793aa1..33e57dceef 100644 --- a/hw/arm/xilinx_zynq.c +++ b/hw/arm/xilinx_zynq.c @@ -198,7 +198,7 @@ static void zynq_init(MachineState *machine) * currently support EL3 so the CPU EL3 property is disabled before * realization. */ - if (object_property_find(OBJECT(cpu), "has_el3")) { + if (arm_feature(&cpu->env, ARM_FEATURE_EL3)) { object_property_set_bool(OBJECT(cpu), "has_el3", false, &error_fatal); } diff --git a/hw/cpu/a15mpcore.c b/hw/cpu/a15mpcore.c index bfd8aa5644..cebfe142cf 100644 --- a/hw/cpu/a15mpcore.c +++ b/hw/cpu/a15mpcore.c @@ -53,7 +53,6 @@ static void a15mp_priv_realize(DeviceState *dev, Error **errp) DeviceState *gicdev; SysBusDevice *busdev; int i; - bool has_el3; bool has_el2 = false; Object *cpuobj; @@ -62,13 +61,17 @@ static void a15mp_priv_realize(DeviceState *dev, Error **errp) qdev_prop_set_uint32(gicdev, "num-irq", s->num_irq); if (!kvm_irqchip_in_kernel()) { + CPUState *cpu; + /* Make the GIC's TZ support match the CPUs. We assume that * either all the CPUs have TZ, or none do. */ - cpuobj = OBJECT(qemu_get_cpu(0)); - has_el3 = object_property_find(cpuobj, "has_el3") && + cpu = qemu_get_cpu(0); + cpuobj = OBJECT(cpu); + if (arm_feature(cpu_env(cpu), ARM_FEATURE_EL3)) { 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); + } /* Similarly for virtualization support */ has_el2 = object_property_find(cpuobj, "has_el2") && object_property_get_bool(cpuobj, "has_el2", &error_abort); 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); - 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;
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(-)