[PATCH v2 00/14] hw/arm: Prefer arm_feature() over object_property_find()

[PATCH v2 00/14] hw/arm: Prefer arm_feature() over object_property_find()

[Philippe Mathieu-Daudé]

Since RFC [*]:
- Split one patch per feature
- Addressed Peter's review comments

[*] https://lore.kernel.org/qemu-devel/20231214171447.44025-1-philmd@linaro.org/

Based-on: <20231123143813.42632-1-philmd@linaro.org>
  "hw: Simplify accesses to CPUState::'start-powered-off' property"

Philippe Mathieu-Daudé (14):
  target/arm/cpu: Simplify checking A64_MTE bit in FEATURE_ID register
  hw/arm/armv7m: Introduce cpudev variable in armv7m_realize()
  hw/arm/armv7m: Ensure requested CPU type implements ARM_FEATURE_M
  hw/arm/armv7m: Move code setting 'start-powered-off' property around
  hw/arm/armv7m: Always set 'init-nsvtor' property for Cortex-M CPUs
  hw/arm: Prefer arm_feature(M_SECURITY) over object_property_find()
  hw/arm: Prefer arm_feature(THUMB_DSP) over object_property_find(dsp)
  hw/arm: Prefer arm_feature(V7) over
    object_property_find(pmsav7-dregion)
  hw/arm: Prefer arm_feature(EL3) over object_property_find(has_el3)
  hw/arm: Prefer arm_feature(EL2) over object_property_find(has_el2)
  hw/arm: Prefer arm_feature(CBAR*) over
    object_property_find(reset-cbar)
  hw/arm: Prefer arm_feature(PMU) over object_property_find(pmu)
  hw/arm: Prefer cpu_isar_feature(aa64_mte) over
    property_find(tag-memory)
  hw/arm: Prefer arm_feature(GENERIC_TMR) over 'kvm-no-adjvtime'
    property

 hw/arm/armv7m.c       | 36 ++++++++++++++++--------------------
 hw/arm/exynos4210.c   |  4 ++--
 hw/arm/highbank.c     |  3 ++-
 hw/arm/integratorcp.c |  5 ++---
 hw/arm/realview.c     |  2 +-
 hw/arm/sbsa-ref.c     |  3 ++-
 hw/arm/versatilepb.c  |  5 ++---
 hw/arm/vexpress.c     |  6 ++++--
 hw/arm/virt.c         | 24 ++++++++++++------------
 hw/arm/xilinx_zynq.c  |  2 +-
 hw/cpu/a15mpcore.c    | 17 +++++++++++------
 hw/cpu/a9mpcore.c     |  6 +++---
 target/arm/cpu.c      |  3 +--
 13 files changed, 59 insertions(+), 57 deletions(-)

-- 
2.41.0

[PATCH v2 01/14] target/arm/cpu: Simplify checking A64_MTE bit in FEATURE_ID register

[Philippe Mathieu-Daudé]

cpu_isar_feature(aa64_mte, cpu) is testing a AArch64-only ID
register. The ARM_FEATURE_AARCH64 check is redundant.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/arm/cpu.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 1c8b787482..c828b333c9 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1690,8 +1690,7 @@ void arm_cpu_post_init(Object *obj)
     }
 
 #ifndef CONFIG_USER_ONLY
-    if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64) &&
-        cpu_isar_feature(aa64_mte, cpu)) {
+    if (cpu_isar_feature(aa64_mte, cpu)) {
         object_property_add_link(obj, "tag-memory",
                                  TYPE_MEMORY_REGION,
                                  (Object **)&cpu->tag_memory,
-- 
2.41.0

[PATCH v2 02/14] hw/arm/armv7m: Introduce cpudev variable in armv7m_realize()

[Philippe Mathieu-Daudé]

We are going to cast s->cpu as DeviceState multiple times.
Add a local 'cpudev' variable to simplify code review, having
a single DEVICE(s->cpu) conversion.

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

diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index 50c6c6b1f5..d239468558 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -277,6 +277,7 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
 {
     ARMv7MState *s = ARMV7M(dev);
     SysBusDevice *sbd;
+    DeviceState *cpudev;
     Error *err = NULL;
     int i;
 
@@ -299,6 +300,7 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
         error_propagate(errp, err);
         return;
     }
+    cpudev = DEVICE(s->cpu);
 
     object_property_set_link(OBJECT(s->cpu), "memory", OBJECT(&s->container),
                              &error_abort);
@@ -356,7 +358,7 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
     s->cpu->env.nvic = &s->nvic;
     s->nvic.cpu = s->cpu;
 
-    if (!qdev_realize(DEVICE(s->cpu), NULL, errp)) {
+    if (!qdev_realize(cpudev, NULL, errp)) {
         return;
     }
 
@@ -426,8 +428,7 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
 
     /* Wire the NVIC up to the CPU */
     sbd = SYS_BUS_DEVICE(&s->nvic);
-    sysbus_connect_irq(sbd, 0,
-                       qdev_get_gpio_in(DEVICE(s->cpu), ARM_CPU_IRQ));
+    sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(cpudev, ARM_CPU_IRQ));
 
     memory_region_add_subregion(&s->container, 0xe000e000,
                                 sysbus_mmio_get_region(sbd, 0));
-- 
2.41.0

[PATCH v2 03/14] hw/arm/armv7m: Ensure requested CPU type implements ARM_FEATURE_M

[Philippe Mathieu-Daudé]

ARMV7M container can only accept M-profile CPU types.
Check requested type is valid once to allow further simplifications.

Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/armv7m.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index d239468558..8900730e53 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -300,6 +300,10 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
         error_propagate(errp, err);
         return;
     }
+    if (!arm_feature(&s->cpu->env, ARM_FEATURE_M)) {
+        error_setg(errp, "armv7m: CPU must be of Cortex-M family");
+        return;
+    }
     cpudev = DEVICE(s->cpu);
 
     object_property_set_link(OBJECT(s->cpu), "memory", OBJECT(&s->container),
-- 
2.41.0

[PATCH v2 04/14] hw/arm/armv7m: Move code setting 'start-powered-off' property around

[Philippe Mathieu-Daudé]

Reorganize a bit by first setting properties which are not
dependent of CPU features (and can not fail).

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

diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index 8900730e53..b752049add 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -308,6 +308,7 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
 
     object_property_set_link(OBJECT(s->cpu), "memory", OBJECT(&s->container),
                              &error_abort);
+    qdev_prop_set_bit(cpudev, "start-powered-off", s->start_powered_off);
     if (object_property_find(OBJECT(s->cpu), "idau")) {
         object_property_set_link(OBJECT(s->cpu), "idau", s->idau,
                                  &error_abort);
@@ -334,7 +335,6 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
             return;
         }
     }
-    qdev_prop_set_bit(DEVICE(s->cpu), "start-powered-off", s->start_powered_off);
 
     /*
      * Real M-profile hardware can be configured with a different number of
-- 
2.41.0

[PATCH v2 05/14] hw/arm/armv7m: Always set 'init-nsvtor' property for Cortex-M CPUs

[Philippe Mathieu-Daudé]

All CPUs implementing ARM_FEATURE_M have the 'init-nsvtor' property.
Since setting the property can not fail, replace

   object_property_set_uint(..., "init-nsvtor", ..., &error_abort);

by:
   qdev_prop_set_uint32(..., "init-nsvtor", ...).

which is a one-to-one replacement.

Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/armv7m.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index b752049add..530729f42e 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -309,6 +309,8 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
     object_property_set_link(OBJECT(s->cpu), "memory", OBJECT(&s->container),
                              &error_abort);
     qdev_prop_set_bit(cpudev, "start-powered-off", s->start_powered_off);
+    qdev_prop_set_uint32(cpudev, "init-nsvtor", s->init_nsvtor);
+
     if (object_property_find(OBJECT(s->cpu), "idau")) {
         object_property_set_link(OBJECT(s->cpu), "idau", s->idau,
                                  &error_abort);
@@ -319,12 +321,6 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
             return;
         }
     }
-    if (object_property_find(OBJECT(s->cpu), "init-nsvtor")) {
-        if (!object_property_set_uint(OBJECT(s->cpu), "init-nsvtor",
-                                      s->init_nsvtor, errp)) {
-            return;
-        }
-    }
     if (object_property_find(OBJECT(s->cpu), "vfp")) {
         if (!object_property_set_bool(OBJECT(s->cpu), "vfp", s->vfp, errp)) {
             return;
-- 
2.41.0

[PATCH v2 06/14] hw/arm: Prefer arm_feature(M_SECURITY) over object_property_find()

[Philippe Mathieu-Daudé]

Both "idau" and "init-svtor" properties are added to ARMCPU
when the ARM_FEATURE_M_SECURITY feature is available. Rather
than checking whether the QOM properties are present, directly
check the feature.

Since we are sure the "init-svtor" is present, the
object_property_set_uint() can't fail.
Instead of using &error_abort, replace:

  object_property_set_uint(OBJECT(s->cpu), "init-svtor",
                           s->init_svtor, &error_abort);
by:

  qdev_prop_set_uint32(cpudev, "init-svtor", s->init_svtor);

which is a one-to-one replacement.

Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/armv7m.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index 530729f42e..8350267d96 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -311,16 +311,11 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
     qdev_prop_set_bit(cpudev, "start-powered-off", s->start_powered_off);
     qdev_prop_set_uint32(cpudev, "init-nsvtor", s->init_nsvtor);
 
-    if (object_property_find(OBJECT(s->cpu), "idau")) {
+    if (arm_feature(&s->cpu->env, ARM_FEATURE_M_SECURITY)) {
+        qdev_prop_set_uint32(cpudev, "init-svtor", s->init_svtor);
         object_property_set_link(OBJECT(s->cpu), "idau", s->idau,
                                  &error_abort);
     }
-    if (object_property_find(OBJECT(s->cpu), "init-svtor")) {
-        if (!object_property_set_uint(OBJECT(s->cpu), "init-svtor",
-                                      s->init_svtor, errp)) {
-            return;
-        }
-    }
     if (object_property_find(OBJECT(s->cpu), "vfp")) {
         if (!object_property_set_bool(OBJECT(s->cpu), "vfp", s->vfp, errp)) {
             return;
-- 
2.41.0

[PATCH v2 07/14] hw/arm: Prefer arm_feature(THUMB_DSP) over object_property_find(dsp)

[Philippe Mathieu-Daudé]

The "dsp" property is added to ARMCPU when the ARM_FEATURE_THUMB_DSP
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/armv7m.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index 8350267d96..0a7ad2b762 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -321,7 +321,7 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
             return;
         }
     }
-    if (object_property_find(OBJECT(s->cpu), "dsp")) {
+    if (arm_feature(&s->cpu->env, ARM_FEATURE_THUMB_DSP)) {
         if (!object_property_set_bool(OBJECT(s->cpu), "dsp", s->dsp, errp)) {
             return;
         }
-- 
2.41.0

[PATCH v2 08/14] hw/arm: Prefer arm_feature(V7) over object_property_find(pmsav7-dregion)

[Philippe Mathieu-Daudé]

The "pmsav7-dregion" property is added to ARMCPU when the
ARM_FEATURE_V7 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/armv7m.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index 0a7ad2b762..7f15318ae3 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -338,8 +338,8 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
                    "mpu-ns-regions and mpu-s-regions properties must have the same value");
         return;
     }
-    if (s->mpu_ns_regions != UINT_MAX &&
-        object_property_find(OBJECT(s->cpu), "pmsav7-dregion")) {
+    if (s->mpu_ns_regions != UINT_MAX && arm_feature(&s->cpu->env,
+                                                     ARM_FEATURE_V7)) {
         if (!object_property_set_uint(OBJECT(s->cpu), "pmsav7-dregion",
                                       s->mpu_ns_regions, errp)) {
             return;
-- 
2.41.0

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

[Philippe Mathieu-Daudé]

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/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;
-- 
2.41.0

[PATCH v2 10/14] hw/arm: Prefer arm_feature(EL2) over object_property_find(has_el2)

[Philippe Mathieu-Daudé]

The "has_el2" property is added to ARMCPU when the
ARM_FEATURE_EL2 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/vexpress.c  | 3 ++-
 hw/arm/virt.c      | 2 +-
 hw/cpu/a15mpcore.c | 6 ++++--
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index fd981f4c33..753a645c05 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -218,12 +218,13 @@ static void init_cpus(MachineState *ms, const char *cpu_type,
     /* Create the actual CPUs */
     for (n = 0; n < smp_cpus; n++) {
         Object *cpuobj = object_new(cpu_type);
+        ARMCPU *cpu = ARM_CPU(cpuobj);
 
         if (!secure) {
             object_property_set_bool(cpuobj, "has_el3", false, NULL);
         }
         if (!virt) {
-            if (object_property_find(cpuobj, "has_el2")) {
+            if (arm_feature(&cpu->env, ARM_FEATURE_EL2)) {
                 object_property_set_bool(cpuobj, "has_el2", false, NULL);
             }
         }
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 2793121cb4..35eb01a3dc 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2146,7 +2146,7 @@ static void machvirt_init(MachineState *machine)
             object_property_set_bool(cpuobj, "has_el3", false, NULL);
         }
 
-        if (!vms->virt && object_property_find(cpuobj, "has_el2")) {
+        if (!vms->virt &&  arm_feature(cpu_env(cs), ARM_FEATURE_EL2)) {
             object_property_set_bool(cpuobj, "has_el2", false, NULL);
         }
 
diff --git a/hw/cpu/a15mpcore.c b/hw/cpu/a15mpcore.c
index cebfe142cf..1fa079b3b8 100644
--- a/hw/cpu/a15mpcore.c
+++ b/hw/cpu/a15mpcore.c
@@ -73,9 +73,11 @@ static void a15mp_priv_realize(DeviceState *dev, Error **errp)
             qdev_prop_set_bit(gicdev, "has-security-extensions", true);
         }
         /* Similarly for virtualization support */
-        has_el2 = object_property_find(cpuobj, "has_el2") &&
+        has_el2 = arm_feature(cpu_env(cpu), ARM_FEATURE_EL2);
+        if (has_el2) {
             object_property_get_bool(cpuobj, "has_el2", &error_abort);
-        qdev_prop_set_bit(gicdev, "has-virtualization-extensions", has_el2);
+            qdev_prop_set_bit(gicdev, "has-virtualization-extensions", true);
+        }
     }
 
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->gic), errp)) {
-- 
2.41.0

[PATCH v2 11/14] hw/arm: Prefer arm_feature(CBAR*) over object_property_find(reset-cbar)

[Philippe Mathieu-Daudé]

The "reset-cbar" property is added to ARMCPU when the
ARM_FEATURE_CBAR[_RO] features are available. Rather than
checking whether the QOM property is present, directly
check the features.

Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/highbank.c | 3 ++-
 hw/arm/sbsa-ref.c | 3 ++-
 hw/arm/vexpress.c | 3 ++-
 hw/arm/virt.c     | 3 ++-
 4 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index c21e18d08f..b06a727c06 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -211,7 +211,8 @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id)
         object_property_set_int(cpuobj, "psci-conduit", QEMU_PSCI_CONDUIT_SMC,
                                 &error_abort);
 
-        if (object_property_find(cpuobj, "reset-cbar")) {
+        if (arm_feature(&cpu->env, ARM_FEATURE_CBAR) ||
+            arm_feature(&cpu->env, ARM_FEATURE_CBAR_RO)) {
             object_property_set_int(cpuobj, "reset-cbar", MPCORE_PERIPHBASE,
                                     &error_abort);
         }
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index 477dca0637..c073c462c7 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -770,7 +770,8 @@ static void sbsa_ref_init(MachineState *machine)
         numa_cpu_pre_plug(&possible_cpus->cpus[cs->cpu_index], DEVICE(cpuobj),
                           &error_fatal);
 
-        if (object_property_find(cpuobj, "reset-cbar")) {
+        if (arm_feature(cpu_env(cs), ARM_FEATURE_CBAR) ||
+            arm_feature(cpu_env(cs), ARM_FEATURE_CBAR_RO)) {
             object_property_set_int(cpuobj, "reset-cbar",
                                     sbsa_ref_memmap[SBSA_CPUPERIPHS].base,
                                     &error_abort);
diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index 753a645c05..ea3c76f3e1 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -229,7 +229,8 @@ static void init_cpus(MachineState *ms, const char *cpu_type,
             }
         }
 
-        if (object_property_find(cpuobj, "reset-cbar")) {
+        if (arm_feature(&cpu->env, ARM_FEATURE_CBAR) ||
+            arm_feature(&cpu->env, ARM_FEATURE_CBAR_RO)) {
             object_property_set_int(cpuobj, "reset-cbar", periphbase,
                                     &error_abort);
         }
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 35eb01a3dc..7e7350fec2 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2168,7 +2168,8 @@ static void machvirt_init(MachineState *machine)
             object_property_set_bool(cpuobj, "lpa2", false, NULL);
         }
 
-        if (object_property_find(cpuobj, "reset-cbar")) {
+        if (arm_feature(cpu_env(cs), ARM_FEATURE_CBAR) ||
+            arm_feature(cpu_env(cs), ARM_FEATURE_CBAR_RO)) {
             object_property_set_int(cpuobj, "reset-cbar",
                                     vms->memmap[VIRT_CPUPERIPHS].base,
                                     &error_abort);
-- 
2.41.0

[PATCH v2 12/14] hw/arm: Prefer arm_feature(PMU) over object_property_find(pmu)

[Philippe Mathieu-Daudé]

The "pmu" property is added to ARMCPU when the
ARM_FEATURE_PMU 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/virt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 7e7350fec2..6d1cb24a6e 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2160,7 +2160,7 @@ static void machvirt_init(MachineState *machine)
             object_property_set_bool(cpuobj, "kvm-steal-time", false, NULL);
         }
 
-        if (vmc->no_pmu && object_property_find(cpuobj, "pmu")) {
+        if (arm_feature(cpu_env(cs), ARM_FEATURE_PMU) && vmc->no_pmu) {
             object_property_set_bool(cpuobj, "pmu", false, NULL);
         }
 
-- 
2.41.0

[PATCH v2 13/14] hw/arm: Prefer cpu_isar_feature(aa64_mte) over property_find(tag-memory)

[Philippe Mathieu-Daudé]

The "tag-memory" property is added to ARMCPU when the
A64_MTE bit is set in the feature ID register. Rather
than checking whether the QOM property is present, directly
check the feature bit.

Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/virt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 6d1cb24a6e..2ce4a18d73 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2189,7 +2189,7 @@ static void machvirt_init(MachineState *machine)
                  * The property exists only if MemTag is supported.
                  * If it is, we must allocate the ram to back that up.
                  */
-                if (!object_property_find(cpuobj, "tag-memory")) {
+                if (!cpu_isar_feature(aa64_mte, ARM_CPU(cs))) {
                     error_report("MTE requested, but not supported "
                                  "by the guest CPU");
                     exit(1);
-- 
2.41.0

[PATCH v2 14/14] hw/arm: Prefer arm_feature(GENERIC_TMR) over 'kvm-no-adjvtime' property

[Philippe Mathieu-Daudé]

First, the "kvm-no-adjvtime" and "kvm-steal-time" are only
available when KVM is available, so guard this block within
a 'kvm_enabled()' check. Since the "kvm-steal-time" property
is always available under KVM, directly set it.

Then, the "kvm-no-adjvtime" property is added to ARMCPU when
the ARM_FEATURE_GENERIC_TIMER feature is available. Rather than
checking whether the QOM property is present, directly check
the feature.

Finally, since we are sure the properties are available, we can
use &error_abort instead of NULL error. Replace:

  object_property_set_bool(..., PROPERTY, ..., &error_abort);

by:

  qdev_prop_set_bit(..., PROPERTY, ...);

which is a one-to-one replacement.

Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/virt.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 2ce4a18d73..6ac8fb19d2 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2150,14 +2150,13 @@ static void machvirt_init(MachineState *machine)
             object_property_set_bool(cpuobj, "has_el2", false, NULL);
         }
 
-        if (vmc->kvm_no_adjvtime &&
-            object_property_find(cpuobj, "kvm-no-adjvtime")) {
-            object_property_set_bool(cpuobj, "kvm-no-adjvtime", true, NULL);
-        }
-
-        if (vmc->no_kvm_steal_time &&
-            object_property_find(cpuobj, "kvm-steal-time")) {
-            object_property_set_bool(cpuobj, "kvm-steal-time", false, NULL);
+        if (kvm_enabled()) {
+            if (arm_feature(cpu_env(cs), ARM_FEATURE_GENERIC_TIMER)) {
+                qdev_prop_set_bit(DEVICE(cs), "kvm-no-adjvtime",
+                                  vmc->kvm_no_adjvtime);
+            }
+            qdev_prop_set_bit(DEVICE(cs), "kvm-steal-time",
+                              !vmc->no_kvm_steal_time);
         }
 
         if (arm_feature(cpu_env(cs), ARM_FEATURE_PMU) && vmc->no_pmu) {
-- 
2.41.0

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: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;

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;
> 

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: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);

Re: [PATCH v2 10/14] hw/arm: Prefer arm_feature(EL2) over object_property_find(has_el2)

[Philippe Mathieu-Daudé]

On 9/1/24 19:09, Philippe Mathieu-Daudé wrote:
> The "has_el2" property is added to ARMCPU when the
> ARM_FEATURE_EL2 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/vexpress.c  | 3 ++-
>   hw/arm/virt.c      | 2 +-
>   hw/cpu/a15mpcore.c | 6 ++++--
>   3 files changed, 7 insertions(+), 4 deletions(-)


> diff --git a/hw/cpu/a15mpcore.c b/hw/cpu/a15mpcore.c
> index cebfe142cf..1fa079b3b8 100644
> --- a/hw/cpu/a15mpcore.c
> +++ b/hw/cpu/a15mpcore.c
> @@ -73,9 +73,11 @@ static void a15mp_priv_realize(DeviceState *dev, Error **errp)
>               qdev_prop_set_bit(gicdev, "has-security-extensions", true);
>           }
>           /* Similarly for virtualization support */
> -        has_el2 = object_property_find(cpuobj, "has_el2") &&
> +        has_el2 = arm_feature(cpu_env(cpu), ARM_FEATURE_EL2);
> +        if (has_el2) {
>               object_property_get_bool(cpuobj, "has_el2", &error_abort);

Missing to be squashed on top:

-- >8 --
          if (has_el2) {
-            object_property_get_bool(cpuobj, "has_el2", &error_abort);
-            qdev_prop_set_bit(gicdev, "has-virtualization-extensions", 
true);
+            qdev_prop_set_bit(gicdev, "has-virtualization-extensions",
+                              object_property_get_bool(cpuobj, "has_el2",
+                                                       &error_abort));
          }
---

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

Re: [PATCH v2 01/14] target/arm/cpu: Simplify checking A64_MTE bit in FEATURE_ID register

[Richard Henderson]

On 1/10/24 05:09, Philippe Mathieu-Daudé wrote:
> cpu_isar_feature(aa64_mte, cpu) is testing a AArch64-only ID
> register. The ARM_FEATURE_AARCH64 check is redundant.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/arm/cpu.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 1c8b787482..c828b333c9 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1690,8 +1690,7 @@ void arm_cpu_post_init(Object *obj)
>       }
>   
>   #ifndef CONFIG_USER_ONLY
> -    if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64) &&
> -        cpu_isar_feature(aa64_mte, cpu)) {
> +    if (cpu_isar_feature(aa64_mte, cpu)) {
>           object_property_add_link(obj, "tag-memory",
>                                    TYPE_MEMORY_REGION,
>                                    (Object **)&cpu->tag_memory,

It is not redundant.

If !AARCH64, then the isar registers tested by aa64_mte are invalid.


r~

Re: [PATCH v2 01/14] target/arm/cpu: Simplify checking A64_MTE bit in FEATURE_ID register

[Peter Maydell]

On Wed, 10 Jan 2024 at 06:01, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 1/10/24 05:09, Philippe Mathieu-Daudé wrote:
> > cpu_isar_feature(aa64_mte, cpu) is testing a AArch64-only ID
> > register. The ARM_FEATURE_AARCH64 check is redundant.
> >
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> >   target/arm/cpu.c | 3 +--
> >   1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> > index 1c8b787482..c828b333c9 100644
> > --- a/target/arm/cpu.c
> > +++ b/target/arm/cpu.c
> > @@ -1690,8 +1690,7 @@ void arm_cpu_post_init(Object *obj)
> >       }
> >
> >   #ifndef CONFIG_USER_ONLY
> > -    if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64) &&
> > -        cpu_isar_feature(aa64_mte, cpu)) {
> > +    if (cpu_isar_feature(aa64_mte, cpu)) {
> >           object_property_add_link(obj, "tag-memory",
> >                                    TYPE_MEMORY_REGION,
> >                                    (Object **)&cpu->tag_memory,
>
> It is not redundant.
>
> If !AARCH64, then the isar registers tested by aa64_mte are invalid.

Ah, I think I steered Philippe wrongly on IRC on this one
because I forgot how our feature checking handled this,
but yes, you're right.

-- PMM