[Qemu-devel] [PATCH 0/4] Disable FPU/DSP for CPU0 on musca-a and mps2-an521

[Qemu-devel] [PATCH 0/4] Disable FPU/DSP for CPU0 on musca-a and mps2-an521

[Peter Maydell]

The SSE-200 hardware has configurable integration settings which
determine whether its two CPUs have the FPU and DSP:
 * CPU0_FPU (default 0)
 * CPU0_DSP (default 0)
 * CPU1_FPU (default 1)
 * CPU1_DSP (default 1)

Similarly, the IoTKit has settings for its single CPU:
 * CPU0_FPU (default 1)
 * CPU0_DSP (default 1)

Of our four boards that use either the IoTKit or the SSE-200:
 * mps2-an505, mps2-an521 and musca-a use the default settings
 * musca-b1 enables FPU and DSP on both CPUs

Currently QEMU models all these boards using CPUs with
both FPU and DSP enabled. This means that we are incorrect
for mps2-an521 and musca-a, which should not have FPU or DSP
on CPU0.

This patchset fixes this (fairly minor) inaccuracy by
implementing properties on the CPU to disable the relevant
CPU features and then wiring them up through the armv7m
object and the ARMSSE SoC container object, so that our
IotKit and SSE200 models behave by default the same way as
the hardware default does, and our Musca-B1 board model
forces the FPU/DSP to be present on CPU, as the hardware does.

The 'neon' property is not strictly required for the M-profile
issues described above, but I implemented it on the CPU
object because disable-neon and disable-vfp interact
for A-profile CPUs.

thanks
-- PMM

Peter Maydell (4):
  target/arm: Allow VFP and Neon to be disabled via a CPU property
  target/arm: Allow M-profile CPUs to disable the DSP extension via CPU
    property
  hw/arm/armv7m: Forward "vfp" and "dsp" properties to CPU
  hw/arm: Correctly disable FPU/DSP for some ARMSSE-based boards

 include/hw/arm/armsse.h |   7 ++
 include/hw/arm/armv7m.h |   4 +
 target/arm/cpu.h        |   6 ++
 hw/arm/armsse.c         |  58 ++++++++++---
 hw/arm/armv7m.c         |  18 ++++
 hw/arm/musca.c          |   8 ++
 target/arm/cpu.c        | 179 ++++++++++++++++++++++++++++++++++++++--
 7 files changed, 262 insertions(+), 18 deletions(-)

-- 
2.20.1

[Qemu-devel] [PATCH 1/4] target/arm: Allow VFP and Neon to be disabled via a CPU property

[Peter Maydell]

Allow VFP and neon to be disabled via a CPU property. As with
the "pmu" property, we only allow these features to be removed
from CPUs which have it by default, not added to CPUs which
don't have it.

The primary motivation here is to be able to optionally
create Cortex-M33 CPUs with no FPU, but we provide switches
for both VFP and Neon because the two interact:
 * AArch64 can't have one without the other
 * Some ID register fields only change if both are disabled

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/cpu.h |   4 ++
 target/arm/cpu.c | 150 +++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 148 insertions(+), 6 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 733b840a712..778fb293e7c 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -797,6 +797,10 @@ struct ARMCPU {
     bool has_el3;
     /* CPU has PMU (Performance Monitor Unit) */
     bool has_pmu;
+    /* CPU has VFP */
+    bool has_vfp;
+    /* CPU has Neon */
+    bool has_neon;
 
     /* CPU has memory protection unit */
     bool has_mpu;
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 8eee1d8c59a..406fd360a2a 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -763,6 +763,12 @@ static Property arm_cpu_cfgend_property =
 static Property arm_cpu_has_pmu_property =
             DEFINE_PROP_BOOL("pmu", ARMCPU, has_pmu, true);
 
+static Property arm_cpu_has_vfp_property =
+            DEFINE_PROP_BOOL("vfp", ARMCPU, has_vfp, true);
+
+static Property arm_cpu_has_neon_property =
+            DEFINE_PROP_BOOL("neon", ARMCPU, has_neon, true);
+
 static Property arm_cpu_has_mpu_property =
             DEFINE_PROP_BOOL("has-mpu", ARMCPU, has_mpu, true);
 
@@ -803,6 +809,13 @@ void arm_cpu_post_init(Object *obj)
     if (arm_feature(&cpu->env, ARM_FEATURE_M)) {
         set_feature(&cpu->env, ARM_FEATURE_PMSA);
     }
+    /* Similarly for the VFP feature bits */
+    if (arm_feature(&cpu->env, ARM_FEATURE_VFP4)) {
+        set_feature(&cpu->env, ARM_FEATURE_VFP3);
+    }
+    if (arm_feature(&cpu->env, ARM_FEATURE_VFP3)) {
+        set_feature(&cpu->env, ARM_FEATURE_VFP);
+    }
 
     if (arm_feature(&cpu->env, ARM_FEATURE_CBAR) ||
         arm_feature(&cpu->env, ARM_FEATURE_CBAR_RO)) {
@@ -847,6 +860,27 @@ void arm_cpu_post_init(Object *obj)
                                  &error_abort);
     }
 
+    /*
+     * Allow user to turn off VFP and Neon support, but only for TCG --
+     * KVM does not currently allow us to lie to the guest about its
+     * ID/feature registers, so the guest always sees what the host has.
+     */
+    if (arm_feature(&cpu->env, ARM_FEATURE_VFP)) {
+        cpu->has_vfp = true;
+        if (!kvm_enabled()) {
+            qdev_property_add_static(DEVICE(obj), &arm_cpu_has_vfp_property,
+                                     &error_abort);
+        }
+    }
+
+    if (arm_feature(&cpu->env, ARM_FEATURE_NEON)) {
+        cpu->has_neon = true;
+        if (!kvm_enabled()) {
+            qdev_property_add_static(DEVICE(obj), &arm_cpu_has_neon_property,
+                                     &error_abort);
+        }
+    }
+
     if (arm_feature(&cpu->env, ARM_FEATURE_PMSA)) {
         qdev_property_add_static(DEVICE(obj), &arm_cpu_has_mpu_property,
                                  &error_abort);
@@ -956,6 +990,116 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
         return;
     }
 
+    if (arm_feature(env, ARM_FEATURE_AARCH64) &&
+        cpu->has_vfp != cpu->has_neon) {
+        /*
+         * This is an architectural requirement for AArch64; AArch32 is
+         * more flexible and permits VFP-no-Neon and Neon-no-VFP.
+         */
+        error_setg(errp,
+                   "AArch64 CPUs must have both VFP and Neon or neither");
+        return;
+    }
+
+    if (!cpu->has_vfp) {
+        uint64_t t;
+        uint32_t u;
+
+        unset_feature(env, ARM_FEATURE_VFP);
+        unset_feature(env, ARM_FEATURE_VFP3);
+        unset_feature(env, ARM_FEATURE_VFP4);
+
+        t = cpu->isar.id_aa64isar1;
+        t = FIELD_DP64(t, ID_AA64ISAR1, JSCVT, 0);
+        cpu->isar.id_aa64isar1 = t;
+
+        t = cpu->isar.id_aa64pfr0;
+        t = FIELD_DP64(t, ID_AA64PFR0, FP, 0xf);
+        cpu->isar.id_aa64pfr0 = t;
+
+        u = cpu->isar.id_isar6;
+        u = FIELD_DP32(u, ID_ISAR6, JSCVT, 0);
+        cpu->isar.id_isar6 = u;
+
+        u = cpu->isar.mvfr0;
+        u = FIELD_DP32(u, MVFR0, FPSP, 0);
+        u = FIELD_DP32(u, MVFR0, FPDP, 0);
+        u = FIELD_DP32(u, MVFR0, FPTRAP, 0);
+        u = FIELD_DP32(u, MVFR0, FPDIVIDE, 0);
+        u = FIELD_DP32(u, MVFR0, FPSQRT, 0);
+        u = FIELD_DP32(u, MVFR0, FPSHVEC, 0);
+        u = FIELD_DP32(u, MVFR0, FPROUND, 0);
+        cpu->isar.mvfr0 = u;
+
+        u = cpu->isar.mvfr1;
+        u = FIELD_DP32(u, MVFR1, FPFTZ, 0);
+        u = FIELD_DP32(u, MVFR1, FPDNAN, 0);
+        u = FIELD_DP32(u, MVFR1, FPHP, 0);
+        cpu->isar.mvfr1 = u;
+
+        u = cpu->isar.mvfr2;
+        u = FIELD_DP32(u, MVFR2, FPMISC, 0);
+        cpu->isar.mvfr2 = u;
+    }
+
+    if (!cpu->has_neon) {
+        uint64_t t;
+        uint32_t u;
+
+        unset_feature(env, ARM_FEATURE_NEON);
+
+        t = cpu->isar.id_aa64isar0;
+        t = FIELD_DP64(t, ID_AA64ISAR0, DP, 0);
+        cpu->isar.id_aa64isar0 = t;
+
+        t = cpu->isar.id_aa64isar1;
+        t = FIELD_DP64(t, ID_AA64ISAR1, FCMA, 0);
+        cpu->isar.id_aa64isar1 = t;
+
+        t = cpu->isar.id_aa64pfr0;
+        t = FIELD_DP64(t, ID_AA64PFR0, ADVSIMD, 0xf);
+        cpu->isar.id_aa64pfr0 = t;
+
+        u = cpu->isar.id_isar5;
+        u = FIELD_DP32(u, ID_ISAR5, RDM, 0);
+        u = FIELD_DP32(u, ID_ISAR5, VCMA, 0);
+        cpu->isar.id_isar5 = u;
+
+        u = cpu->isar.id_isar6;
+        u = FIELD_DP32(u, ID_ISAR6, DP, 0);
+        u = FIELD_DP32(u, ID_ISAR6, FHM, 0);
+        cpu->isar.id_isar6 = u;
+
+        u = cpu->isar.mvfr1;
+        u = FIELD_DP32(u, MVFR1, SIMDLS, 0);
+        u = FIELD_DP32(u, MVFR1, SIMDINT, 0);
+        u = FIELD_DP32(u, MVFR1, SIMDSP, 0);
+        u = FIELD_DP32(u, MVFR1, SIMDHP, 0);
+        u = FIELD_DP32(u, MVFR1, SIMDFMAC, 0);
+        cpu->isar.mvfr1 = u;
+
+        u = cpu->isar.mvfr2;
+        u = FIELD_DP32(u, MVFR2, SIMDMISC, 0);
+        cpu->isar.mvfr2 = u;
+    }
+
+    if (!cpu->has_neon && !cpu->has_vfp) {
+        uint64_t t;
+        uint32_t u;
+
+        t = cpu->isar.id_aa64isar0;
+        t = FIELD_DP64(t, ID_AA64ISAR0, FHM, 0);
+        cpu->isar.id_aa64isar0 = t;
+
+        t = cpu->isar.id_aa64isar1;
+        t = FIELD_DP64(t, ID_AA64ISAR1, FRINTTS, 0);
+        cpu->isar.id_aa64isar1 = t;
+
+        u = cpu->isar.mvfr0;
+        u = FIELD_DP32(u, MVFR0, SIMDREG, 0);
+        cpu->isar.mvfr0 = u;
+    }
+
     /* Some features automatically imply others: */
     if (arm_feature(env, ARM_FEATURE_V8)) {
         if (arm_feature(env, ARM_FEATURE_M)) {
@@ -1016,12 +1160,6 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
     if (arm_feature(env, ARM_FEATURE_V5)) {
         set_feature(env, ARM_FEATURE_V4T);
     }
-    if (arm_feature(env, ARM_FEATURE_VFP4)) {
-        set_feature(env, ARM_FEATURE_VFP3);
-    }
-    if (arm_feature(env, ARM_FEATURE_VFP3)) {
-        set_feature(env, ARM_FEATURE_VFP);
-    }
     if (arm_feature(env, ARM_FEATURE_LPAE)) {
         set_feature(env, ARM_FEATURE_V7MP);
         set_feature(env, ARM_FEATURE_PXN);
-- 
2.20.1

[Qemu-devel] [PATCH 2/4] target/arm: Allow M-profile CPUs to disable the DSP extension via CPU property

[Peter Maydell]

Allow the DSP extension to be disabled via a CPU property for
M-profile CPUs. (A and R-profile CPUs don't have this extension
as a defined separate optional architecture extension, so
they don't need the property.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/cpu.h |  2 ++
 target/arm/cpu.c | 29 +++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 778fb293e7c..b1c7ab3ee94 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -801,6 +801,8 @@ struct ARMCPU {
     bool has_vfp;
     /* CPU has Neon */
     bool has_neon;
+    /* CPU has M-profile DSP extension */
+    bool has_dsp;
 
     /* CPU has memory protection unit */
     bool has_mpu;
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 406fd360a2a..c43139ba897 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -769,6 +769,9 @@ static Property arm_cpu_has_vfp_property =
 static Property arm_cpu_has_neon_property =
             DEFINE_PROP_BOOL("neon", ARMCPU, has_neon, true);
 
+static Property arm_cpu_has_dsp_property =
+            DEFINE_PROP_BOOL("dsp", ARMCPU, has_dsp, true);
+
 static Property arm_cpu_has_mpu_property =
             DEFINE_PROP_BOOL("has-mpu", ARMCPU, has_mpu, true);
 
@@ -881,6 +884,12 @@ void arm_cpu_post_init(Object *obj)
         }
     }
 
+    if (arm_feature(&cpu->env, ARM_FEATURE_M) &&
+        arm_feature(&cpu->env, ARM_FEATURE_THUMB_DSP)) {
+        qdev_property_add_static(DEVICE(obj), &arm_cpu_has_dsp_property,
+                                 &error_abort);
+    }
+
     if (arm_feature(&cpu->env, ARM_FEATURE_PMSA)) {
         qdev_property_add_static(DEVICE(obj), &arm_cpu_has_mpu_property,
                                  &error_abort);
@@ -1100,6 +1109,26 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
         cpu->isar.mvfr0 = u;
     }
 
+    if (arm_feature(env, ARM_FEATURE_M) && !cpu->has_dsp) {
+        uint32_t u;
+
+        unset_feature(env, ARM_FEATURE_THUMB_DSP);
+
+        u = cpu->isar.id_isar1;
+        u = FIELD_DP32(u, ID_ISAR1, EXTEND, 1);
+        cpu->isar.id_isar1 = u;
+
+        u = cpu->isar.id_isar2;
+        u = FIELD_DP32(u, ID_ISAR2, MULTU, 1);
+        u = FIELD_DP32(u, ID_ISAR2, MULTS, 1);
+        cpu->isar.id_isar2 = u;
+
+        u = cpu->isar.id_isar3;
+        u = FIELD_DP32(u, ID_ISAR3, SIMD, 1);
+        u = FIELD_DP32(u, ID_ISAR3, SATURATE, 0);
+        cpu->isar.id_isar3 = u;
+    }
+
     /* Some features automatically imply others: */
     if (arm_feature(env, ARM_FEATURE_V8)) {
         if (arm_feature(env, ARM_FEATURE_M)) {
-- 
2.20.1

[Qemu-devel] [PATCH 3/4] hw/arm/armv7m: Forward "vfp" and "dsp" properties to CPU

[Peter Maydell]

Create "vfp" and "dsp" properties on the armv7m container object
which will be forwarded to its CPU object, so that SoCs can
configure whether the CPU has these features.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/arm/armv7m.h |  4 ++++
 hw/arm/armv7m.c         | 18 ++++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/include/hw/arm/armv7m.h b/include/hw/arm/armv7m.h
index e96a98f8093..d2c74d3872a 100644
--- a/include/hw/arm/armv7m.h
+++ b/include/hw/arm/armv7m.h
@@ -43,6 +43,8 @@ typedef struct {
  *   devices will be automatically layered on top of this view.)
  * + Property "idau": IDAU interface (forwarded to CPU object)
  * + Property "init-svtor": secure VTOR reset value (forwarded to CPU object)
+ * + Property "vfp": enable VFP (forwarded to CPU object)
+ * + Property "dsp": enable DSP (forwarded to CPU object)
  * + Property "enable-bitband": expose bitbanded IO
  */
 typedef struct ARMv7MState {
@@ -66,6 +68,8 @@ typedef struct ARMv7MState {
     uint32_t init_svtor;
     bool enable_bitband;
     bool start_powered_off;
+    bool vfp;
+    bool dsp;
 } ARMv7MState;
 
 #endif
diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index c4b2a9a1f5c..7caf9bd3364 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -190,6 +190,22 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
             return;
         }
     }
+    if (object_property_find(OBJECT(s->cpu), "vfp", NULL)) {
+        object_property_set_bool(OBJECT(s->cpu), s->vfp,
+                                 "vfp", &err);
+        if (err != NULL) {
+            error_propagate(errp, err);
+            return;
+        }
+    }
+    if (object_property_find(OBJECT(s->cpu), "dsp", NULL)) {
+        object_property_set_bool(OBJECT(s->cpu), s->dsp,
+                                 "dsp", &err);
+        if (err != NULL) {
+            error_propagate(errp, err);
+            return;
+        }
+    }
 
     /*
      * Tell the CPU where the NVIC is; it will fail realize if it doesn't
@@ -260,6 +276,8 @@ static Property armv7m_properties[] = {
     DEFINE_PROP_BOOL("enable-bitband", ARMv7MState, enable_bitband, false),
     DEFINE_PROP_BOOL("start-powered-off", ARMv7MState, start_powered_off,
                      false),
+    DEFINE_PROP_BOOL("vfp", ARMv7MState, vfp, true),
+    DEFINE_PROP_BOOL("dsp", ARMv7MState, dsp, true),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.20.1

[Qemu-devel] [PATCH 4/4] hw/arm: Correctly disable FPU/DSP for some ARMSSE-based boards

[Peter Maydell]

The SSE-200 hardware has configurable integration settings which
determine whether its two CPUs have the FPU and DSP:
 * CPU0_FPU (default 0)
 * CPU0_DSP (default 0)
 * CPU1_FPU (default 1)
 * CPU1_DSP (default 1)

Similarly, the IoTKit has settings for its single CPU:
 * CPU0_FPU (default 1)
 * CPU0_DSP (default 1)

Of our four boards that use either the IoTKit or the SSE-200:
 * mps2-an505, mps2-an521 and musca-a use the default settings
 * musca-b1 enables FPU and DSP on both CPUs

Currently QEMU models all these boards using CPUs with
both FPU and DSP enabled. This means that we are incorrect
for mps2-an521 and musca-a, which should not have FPU or DSP
on CPU0.

Create QOM properties on the ARMSSE devices corresponding to the
default h/w integration settings, and make the Musca-B1 board
enable FPU and DSP on both CPUs. This fixes the mps2-an521
and musca-a behaviour, and leaves the musca-b1 and mps2-an505
behaviour unchanged.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/arm/armsse.h |  7 +++++
 hw/arm/armsse.c         | 58 ++++++++++++++++++++++++++++++++---------
 hw/arm/musca.c          |  8 ++++++
 3 files changed, 61 insertions(+), 12 deletions(-)

diff --git a/include/hw/arm/armsse.h b/include/hw/arm/armsse.h
index 81e082cccf8..84080c22993 100644
--- a/include/hw/arm/armsse.h
+++ b/include/hw/arm/armsse.h
@@ -50,6 +50,11 @@
  *    address of each SRAM bank (and thus the total amount of internal SRAM)
  *  + QOM property "init-svtor" sets the initial value of the CPU SVTOR register
  *    (where it expects to load the PC and SP from the vector table on reset)
+ *  + QOM properties "CPU0_FPU", "CPU0_DSP", "CPU1_FPU" and "CPU1_DSP" which
+ *    set whether the CPUs have the FPU and DSP features present. The default
+ *    (matching the hardware) is that for CPU0 in an IoTKit and CPU1 in an
+ *    SSE-200 both are present; CPU0 in an SSE-200 has neither.
+ *    Since the IoTKit has only one CPU, it does not have the CPU1_* properties.
  *  + Named GPIO inputs "EXP_IRQ" 0..n are the expansion interrupts for CPU 0,
  *    which are wired to its NVIC lines 32 .. n+32
  *  + Named GPIO inputs "EXP_CPU1_IRQ" 0..n are the expansion interrupts for
@@ -208,6 +213,8 @@ typedef struct ARMSSE {
     uint32_t mainclk_frq;
     uint32_t sram_addr_width;
     uint32_t init_svtor;
+    bool cpu_fpu[SSE_MAX_CPUS];
+    bool cpu_dsp[SSE_MAX_CPUS];
 } ARMSSE;
 
 typedef struct ARMSSEInfo ARMSSEInfo;
diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c
index 76cc6905798..e138aee673f 100644
--- a/hw/arm/armsse.c
+++ b/hw/arm/armsse.c
@@ -37,6 +37,33 @@ struct ARMSSEInfo {
     bool has_cachectrl;
     bool has_cpusecctrl;
     bool has_cpuid;
+    Property *props;
+};
+
+static Property iotkit_properties[] = {
+    DEFINE_PROP_LINK("memory", ARMSSE, board_memory, TYPE_MEMORY_REGION,
+                     MemoryRegion *),
+    DEFINE_PROP_UINT32("EXP_NUMIRQ", ARMSSE, exp_numirq, 64),
+    DEFINE_PROP_UINT32("MAINCLK", ARMSSE, mainclk_frq, 0),
+    DEFINE_PROP_UINT32("SRAM_ADDR_WIDTH", ARMSSE, sram_addr_width, 15),
+    DEFINE_PROP_UINT32("init-svtor", ARMSSE, init_svtor, 0x10000000),
+    DEFINE_PROP_BOOL("CPU0_FPU", ARMSSE, cpu_fpu[0], true),
+    DEFINE_PROP_BOOL("CPU0_DSP", ARMSSE, cpu_dsp[0], true),
+    DEFINE_PROP_END_OF_LIST()
+};
+
+static Property armsse_properties[] = {
+    DEFINE_PROP_LINK("memory", ARMSSE, board_memory, TYPE_MEMORY_REGION,
+                     MemoryRegion *),
+    DEFINE_PROP_UINT32("EXP_NUMIRQ", ARMSSE, exp_numirq, 64),
+    DEFINE_PROP_UINT32("MAINCLK", ARMSSE, mainclk_frq, 0),
+    DEFINE_PROP_UINT32("SRAM_ADDR_WIDTH", ARMSSE, sram_addr_width, 15),
+    DEFINE_PROP_UINT32("init-svtor", ARMSSE, init_svtor, 0x10000000),
+    DEFINE_PROP_BOOL("CPU0_FPU", ARMSSE, cpu_fpu[0], false),
+    DEFINE_PROP_BOOL("CPU0_DSP", ARMSSE, cpu_dsp[0], false),
+    DEFINE_PROP_BOOL("CPU1_FPU", ARMSSE, cpu_fpu[1], true),
+    DEFINE_PROP_BOOL("CPU1_DSP", ARMSSE, cpu_dsp[1], true),
+    DEFINE_PROP_END_OF_LIST()
 };
 
 static const ARMSSEInfo armsse_variants[] = {
@@ -52,6 +79,7 @@ static const ARMSSEInfo armsse_variants[] = {
         .has_cachectrl = false,
         .has_cpusecctrl = false,
         .has_cpuid = false,
+        .props = iotkit_properties,
     },
     {
         .name = TYPE_SSE200,
@@ -65,6 +93,7 @@ static const ARMSSEInfo armsse_variants[] = {
         .has_cachectrl = true,
         .has_cpusecctrl = true,
         .has_cpuid = true,
+        .props = armsse_properties,
     },
 };
 
@@ -532,6 +561,20 @@ static void armsse_realize(DeviceState *dev, Error **errp)
                 return;
             }
         }
+        if (!s->cpu_fpu[i]) {
+            object_property_set_bool(cpuobj, false, "vfp", &err);
+            if (err) {
+                error_propagate(errp, err);
+                return;
+            }
+        }
+        if (!s->cpu_dsp[i]) {
+            object_property_set_bool(cpuobj, false, "dsp", &err);
+            if (err) {
+                error_propagate(errp, err);
+                return;
+            }
+        }
 
         if (i > 0) {
             memory_region_add_subregion_overlap(&s->cpu_container[i], 0,
@@ -1221,16 +1264,6 @@ static const VMStateDescription armsse_vmstate = {
     }
 };
 
-static Property armsse_properties[] = {
-    DEFINE_PROP_LINK("memory", ARMSSE, board_memory, TYPE_MEMORY_REGION,
-                     MemoryRegion *),
-    DEFINE_PROP_UINT32("EXP_NUMIRQ", ARMSSE, exp_numirq, 64),
-    DEFINE_PROP_UINT32("MAINCLK", ARMSSE, mainclk_frq, 0),
-    DEFINE_PROP_UINT32("SRAM_ADDR_WIDTH", ARMSSE, sram_addr_width, 15),
-    DEFINE_PROP_UINT32("init-svtor", ARMSSE, init_svtor, 0x10000000),
-    DEFINE_PROP_END_OF_LIST()
-};
-
 static void armsse_reset(DeviceState *dev)
 {
     ARMSSE *s = ARMSSE(dev);
@@ -1243,13 +1276,14 @@ static void armsse_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     IDAUInterfaceClass *iic = IDAU_INTERFACE_CLASS(klass);
     ARMSSEClass *asc = ARMSSE_CLASS(klass);
+    const ARMSSEInfo *info = data;
 
     dc->realize = armsse_realize;
     dc->vmsd = &armsse_vmstate;
-    dc->props = armsse_properties;
+    dc->props = info->props;
     dc->reset = armsse_reset;
     iic->check = armsse_idau_check;
-    asc->info = data;
+    asc->info = info;
 }
 
 static const TypeInfo armsse_info = {
diff --git a/hw/arm/musca.c b/hw/arm/musca.c
index 23aff43f4bc..736f37b774c 100644
--- a/hw/arm/musca.c
+++ b/hw/arm/musca.c
@@ -385,6 +385,14 @@ static void musca_init(MachineState *machine)
     qdev_prop_set_uint32(ssedev, "init-svtor", mmc->init_svtor);
     qdev_prop_set_uint32(ssedev, "SRAM_ADDR_WIDTH", mmc->sram_addr_width);
     qdev_prop_set_uint32(ssedev, "MAINCLK", SYSCLK_FRQ);
+    /*
+     * Musca-A takes the default SSE-200 FPU/DSP settings (ie no for
+     * CPU0 and yes for CPU1); Musca-B1 explicitly enables them for CPU0.
+     */
+    if (mmc->type == MUSCA_B1) {
+        qdev_prop_set_bit(ssedev, "CPU0_FPU", true);
+        qdev_prop_set_bit(ssedev, "CPU0_DSP", true);
+    }
     object_property_set_bool(OBJECT(&mms->sse), true, "realized",
                              &error_fatal);
 
-- 
2.20.1

Re: [Qemu-devel] [PATCH 0/4] Disable FPU/DSP for CPU0 on musca-a and mps2-an521

[Peter Maydell]

Ping for code review, please?

thanks
-- PMM

On Fri, 17 May 2019 at 18:40, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> The SSE-200 hardware has configurable integration settings which
> determine whether its two CPUs have the FPU and DSP:
>  * CPU0_FPU (default 0)
>  * CPU0_DSP (default 0)
>  * CPU1_FPU (default 1)
>  * CPU1_DSP (default 1)
>
> Similarly, the IoTKit has settings for its single CPU:
>  * CPU0_FPU (default 1)
>  * CPU0_DSP (default 1)
>
> Of our four boards that use either the IoTKit or the SSE-200:
>  * mps2-an505, mps2-an521 and musca-a use the default settings
>  * musca-b1 enables FPU and DSP on both CPUs
>
> Currently QEMU models all these boards using CPUs with
> both FPU and DSP enabled. This means that we are incorrect
> for mps2-an521 and musca-a, which should not have FPU or DSP
> on CPU0.
>
> This patchset fixes this (fairly minor) inaccuracy by
> implementing properties on the CPU to disable the relevant
> CPU features and then wiring them up through the armv7m
> object and the ARMSSE SoC container object, so that our
> IotKit and SSE200 models behave by default the same way as
> the hardware default does, and our Musca-B1 board model
> forces the FPU/DSP to be present on CPU, as the hardware does.
>
> The 'neon' property is not strictly required for the M-profile
> issues described above, but I implemented it on the CPU
> object because disable-neon and disable-vfp interact
> for A-profile CPUs.
>
> thanks
> -- PMM
>
> Peter Maydell (4):
>   target/arm: Allow VFP and Neon to be disabled via a CPU property
>   target/arm: Allow M-profile CPUs to disable the DSP extension via CPU
>     property
>   hw/arm/armv7m: Forward "vfp" and "dsp" properties to CPU
>   hw/arm: Correctly disable FPU/DSP for some ARMSSE-based boards
>
>  include/hw/arm/armsse.h |   7 ++
>  include/hw/arm/armv7m.h |   4 +
>  target/arm/cpu.h        |   6 ++
>  hw/arm/armsse.c         |  58 ++++++++++---
>  hw/arm/armv7m.c         |  18 ++++
>  hw/arm/musca.c          |   8 ++
>  target/arm/cpu.c        | 179 ++++++++++++++++++++++++++++++++++++++--
>  7 files changed, 262 insertions(+), 18 deletions(-)
>
> --
> 2.20.1

Re: [Qemu-devel] [PATCH 2/4] target/arm: Allow M-profile CPUs to disable the DSP extension via CPU property

[Philippe Mathieu-Daudé]

On 5/17/19 7:40 PM, Peter Maydell wrote:
> Allow the DSP extension to be disabled via a CPU property for
> M-profile CPUs. (A and R-profile CPUs don't have this extension
> as a defined separate optional architecture extension, so
> they don't need the property.)
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target/arm/cpu.h |  2 ++
>  target/arm/cpu.c | 29 +++++++++++++++++++++++++++++
>  2 files changed, 31 insertions(+)
> 
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 778fb293e7c..b1c7ab3ee94 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -801,6 +801,8 @@ struct ARMCPU {
>      bool has_vfp;
>      /* CPU has Neon */
>      bool has_neon;
> +    /* CPU has M-profile DSP extension */
> +    bool has_dsp;
>  
>      /* CPU has memory protection unit */
>      bool has_mpu;
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 406fd360a2a..c43139ba897 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -769,6 +769,9 @@ static Property arm_cpu_has_vfp_property =
>  static Property arm_cpu_has_neon_property =
>              DEFINE_PROP_BOOL("neon", ARMCPU, has_neon, true);
>  
> +static Property arm_cpu_has_dsp_property =
> +            DEFINE_PROP_BOOL("dsp", ARMCPU, has_dsp, true);
> +
>  static Property arm_cpu_has_mpu_property =
>              DEFINE_PROP_BOOL("has-mpu", ARMCPU, has_mpu, true);
>  
> @@ -881,6 +884,12 @@ void arm_cpu_post_init(Object *obj)
>          }
>      }
>  
> +    if (arm_feature(&cpu->env, ARM_FEATURE_M) &&
> +        arm_feature(&cpu->env, ARM_FEATURE_THUMB_DSP)) {
> +        qdev_property_add_static(DEVICE(obj), &arm_cpu_has_dsp_property,
> +                                 &error_abort);

TIL qdev_property_add_static, clean.
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> +    }
> +
>      if (arm_feature(&cpu->env, ARM_FEATURE_PMSA)) {
>          qdev_property_add_static(DEVICE(obj), &arm_cpu_has_mpu_property,
>                                   &error_abort);
> @@ -1100,6 +1109,26 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
>          cpu->isar.mvfr0 = u;
>      }
>  
> +    if (arm_feature(env, ARM_FEATURE_M) && !cpu->has_dsp) {
> +        uint32_t u;
> +
> +        unset_feature(env, ARM_FEATURE_THUMB_DSP);
> +
> +        u = cpu->isar.id_isar1;
> +        u = FIELD_DP32(u, ID_ISAR1, EXTEND, 1);
> +        cpu->isar.id_isar1 = u;
> +
> +        u = cpu->isar.id_isar2;
> +        u = FIELD_DP32(u, ID_ISAR2, MULTU, 1);
> +        u = FIELD_DP32(u, ID_ISAR2, MULTS, 1);
> +        cpu->isar.id_isar2 = u;
> +
> +        u = cpu->isar.id_isar3;
> +        u = FIELD_DP32(u, ID_ISAR3, SIMD, 1);
> +        u = FIELD_DP32(u, ID_ISAR3, SATURATE, 0);
> +        cpu->isar.id_isar3 = u;
> +    }
> +
>      /* Some features automatically imply others: */
>      if (arm_feature(env, ARM_FEATURE_V8)) {
>          if (arm_feature(env, ARM_FEATURE_M)) {
> 

Re: [Qemu-devel] [PATCH 3/4] hw/arm/armv7m: Forward "vfp" and "dsp" properties to CPU

[Philippe Mathieu-Daudé]

On 5/17/19 7:40 PM, Peter Maydell wrote:
> Create "vfp" and "dsp" properties on the armv7m container object
> which will be forwarded to its CPU object, so that SoCs can
> configure whether the CPU has these features.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/hw/arm/armv7m.h |  4 ++++
>  hw/arm/armv7m.c         | 18 ++++++++++++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/include/hw/arm/armv7m.h b/include/hw/arm/armv7m.h
> index e96a98f8093..d2c74d3872a 100644
> --- a/include/hw/arm/armv7m.h
> +++ b/include/hw/arm/armv7m.h
> @@ -43,6 +43,8 @@ typedef struct {
>   *   devices will be automatically layered on top of this view.)
>   * + Property "idau": IDAU interface (forwarded to CPU object)
>   * + Property "init-svtor": secure VTOR reset value (forwarded to CPU object)
> + * + Property "vfp": enable VFP (forwarded to CPU object)
> + * + Property "dsp": enable DSP (forwarded to CPU object)
>   * + Property "enable-bitband": expose bitbanded IO
>   */
>  typedef struct ARMv7MState {
> @@ -66,6 +68,8 @@ typedef struct ARMv7MState {
>      uint32_t init_svtor;
>      bool enable_bitband;
>      bool start_powered_off;
> +    bool vfp;
> +    bool dsp;
>  } ARMv7MState;
>  
>  #endif
> diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
> index c4b2a9a1f5c..7caf9bd3364 100644
> --- a/hw/arm/armv7m.c
> +++ b/hw/arm/armv7m.c
> @@ -190,6 +190,22 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
>              return;
>          }
>      }
> +    if (object_property_find(OBJECT(s->cpu), "vfp", NULL)) {

And TIL object_property_find :)
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> +        object_property_set_bool(OBJECT(s->cpu), s->vfp,
> +                                 "vfp", &err);
> +        if (err != NULL) {
> +            error_propagate(errp, err);
> +            return;
> +        }
> +    }
> +    if (object_property_find(OBJECT(s->cpu), "dsp", NULL)) {
> +        object_property_set_bool(OBJECT(s->cpu), s->dsp,
> +                                 "dsp", &err);
> +        if (err != NULL) {
> +            error_propagate(errp, err);
> +            return;
> +        }
> +    }
>  
>      /*
>       * Tell the CPU where the NVIC is; it will fail realize if it doesn't
> @@ -260,6 +276,8 @@ static Property armv7m_properties[] = {
>      DEFINE_PROP_BOOL("enable-bitband", ARMv7MState, enable_bitband, false),
>      DEFINE_PROP_BOOL("start-powered-off", ARMv7MState, start_powered_off,
>                       false),
> +    DEFINE_PROP_BOOL("vfp", ARMv7MState, vfp, true),
> +    DEFINE_PROP_BOOL("dsp", ARMv7MState, dsp, true),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> 

Re: [Qemu-devel] [PATCH 1/4] target/arm: Allow VFP and Neon to be disabled via a CPU property

[Philippe Mathieu-Daudé]

On 5/17/19 7:40 PM, Peter Maydell wrote:
> Allow VFP and neon to be disabled via a CPU property. As with
> the "pmu" property, we only allow these features to be removed
> from CPUs which have it by default, not added to CPUs which
> don't have it.
> 
> The primary motivation here is to be able to optionally
> create Cortex-M33 CPUs with no FPU, but we provide switches
> for both VFP and Neon because the two interact:
>  * AArch64 can't have one without the other
>  * Some ID register fields only change if both are disabled
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target/arm/cpu.h |   4 ++
>  target/arm/cpu.c | 150 +++++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 148 insertions(+), 6 deletions(-)
> 
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 733b840a712..778fb293e7c 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -797,6 +797,10 @@ struct ARMCPU {
>      bool has_el3;
>      /* CPU has PMU (Performance Monitor Unit) */
>      bool has_pmu;
> +    /* CPU has VFP */
> +    bool has_vfp;
> +    /* CPU has Neon */
> +    bool has_neon;
>  
>      /* CPU has memory protection unit */
>      bool has_mpu;
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 8eee1d8c59a..406fd360a2a 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -763,6 +763,12 @@ static Property arm_cpu_cfgend_property =
>  static Property arm_cpu_has_pmu_property =
>              DEFINE_PROP_BOOL("pmu", ARMCPU, has_pmu, true);
>  
> +static Property arm_cpu_has_vfp_property =
> +            DEFINE_PROP_BOOL("vfp", ARMCPU, has_vfp, true);
> +
> +static Property arm_cpu_has_neon_property =
> +            DEFINE_PROP_BOOL("neon", ARMCPU, has_neon, true);
> +
>  static Property arm_cpu_has_mpu_property =
>              DEFINE_PROP_BOOL("has-mpu", ARMCPU, has_mpu, true);
>  
> @@ -803,6 +809,13 @@ void arm_cpu_post_init(Object *obj)
>      if (arm_feature(&cpu->env, ARM_FEATURE_M)) {
>          set_feature(&cpu->env, ARM_FEATURE_PMSA);
>      }
> +    /* Similarly for the VFP feature bits */
> +    if (arm_feature(&cpu->env, ARM_FEATURE_VFP4)) {
> +        set_feature(&cpu->env, ARM_FEATURE_VFP3);
> +    }
> +    if (arm_feature(&cpu->env, ARM_FEATURE_VFP3)) {
> +        set_feature(&cpu->env, ARM_FEATURE_VFP);
> +    }
>  
>      if (arm_feature(&cpu->env, ARM_FEATURE_CBAR) ||
>          arm_feature(&cpu->env, ARM_FEATURE_CBAR_RO)) {
> @@ -847,6 +860,27 @@ void arm_cpu_post_init(Object *obj)
>                                   &error_abort);
>      }
>  
> +    /*
> +     * Allow user to turn off VFP and Neon support, but only for TCG --
> +     * KVM does not currently allow us to lie to the guest about its
> +     * ID/feature registers, so the guest always sees what the host has.
> +     */

I wondered about that last week when refreshing Samuel TCG/KVM split,
because the VFP code pulls a lot of unnecessary code on KVM (in
particular the softfloat lib).

> +    if (arm_feature(&cpu->env, ARM_FEATURE_VFP)) {
> +        cpu->has_vfp = true;
> +        if (!kvm_enabled()) {
> +            qdev_property_add_static(DEVICE(obj), &arm_cpu_has_vfp_property,
> +                                     &error_abort);
> +        }
> +    }
> +
> +    if (arm_feature(&cpu->env, ARM_FEATURE_NEON)) {
> +        cpu->has_neon = true;
> +        if (!kvm_enabled()) {
> +            qdev_property_add_static(DEVICE(obj), &arm_cpu_has_neon_property,
> +                                     &error_abort);
> +        }
> +    }
> +
>      if (arm_feature(&cpu->env, ARM_FEATURE_PMSA)) {
>          qdev_property_add_static(DEVICE(obj), &arm_cpu_has_mpu_property,
>                                   &error_abort);
> @@ -956,6 +990,116 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
>          return;
>      }
>  
> +    if (arm_feature(env, ARM_FEATURE_AARCH64) &&
> +        cpu->has_vfp != cpu->has_neon) {
> +        /*
> +         * This is an architectural requirement for AArch64; AArch32 is
> +         * more flexible and permits VFP-no-Neon and Neon-no-VFP.
> +         */
> +        error_setg(errp,
> +                   "AArch64 CPUs must have both VFP and Neon or neither");
> +        return;
> +    }
> +
> +    if (!cpu->has_vfp) {
> +        uint64_t t;
> +        uint32_t u;
> +
> +        unset_feature(env, ARM_FEATURE_VFP);
> +        unset_feature(env, ARM_FEATURE_VFP3);
> +        unset_feature(env, ARM_FEATURE_VFP4);
> +
> +        t = cpu->isar.id_aa64isar1;
> +        t = FIELD_DP64(t, ID_AA64ISAR1, JSCVT, 0);
> +        cpu->isar.id_aa64isar1 = t;
> +
> +        t = cpu->isar.id_aa64pfr0;
> +        t = FIELD_DP64(t, ID_AA64PFR0, FP, 0xf);
> +        cpu->isar.id_aa64pfr0 = t;
> +
> +        u = cpu->isar.id_isar6;
> +        u = FIELD_DP32(u, ID_ISAR6, JSCVT, 0);
> +        cpu->isar.id_isar6 = u;
> +
> +        u = cpu->isar.mvfr0;
> +        u = FIELD_DP32(u, MVFR0, FPSP, 0);
> +        u = FIELD_DP32(u, MVFR0, FPDP, 0);
> +        u = FIELD_DP32(u, MVFR0, FPTRAP, 0);
> +        u = FIELD_DP32(u, MVFR0, FPDIVIDE, 0);
> +        u = FIELD_DP32(u, MVFR0, FPSQRT, 0);
> +        u = FIELD_DP32(u, MVFR0, FPSHVEC, 0);
> +        u = FIELD_DP32(u, MVFR0, FPROUND, 0);
> +        cpu->isar.mvfr0 = u;
> +
> +        u = cpu->isar.mvfr1;
> +        u = FIELD_DP32(u, MVFR1, FPFTZ, 0);
> +        u = FIELD_DP32(u, MVFR1, FPDNAN, 0);
> +        u = FIELD_DP32(u, MVFR1, FPHP, 0);
> +        cpu->isar.mvfr1 = u;
> +
> +        u = cpu->isar.mvfr2;
> +        u = FIELD_DP32(u, MVFR2, FPMISC, 0);
> +        cpu->isar.mvfr2 = u;
> +    }
> +
> +    if (!cpu->has_neon) {
> +        uint64_t t;
> +        uint32_t u;
> +
> +        unset_feature(env, ARM_FEATURE_NEON);
> +
> +        t = cpu->isar.id_aa64isar0;
> +        t = FIELD_DP64(t, ID_AA64ISAR0, DP, 0);
> +        cpu->isar.id_aa64isar0 = t;
> +
> +        t = cpu->isar.id_aa64isar1;
> +        t = FIELD_DP64(t, ID_AA64ISAR1, FCMA, 0);
> +        cpu->isar.id_aa64isar1 = t;
> +
> +        t = cpu->isar.id_aa64pfr0;
> +        t = FIELD_DP64(t, ID_AA64PFR0, ADVSIMD, 0xf);
> +        cpu->isar.id_aa64pfr0 = t;
> +
> +        u = cpu->isar.id_isar5;
> +        u = FIELD_DP32(u, ID_ISAR5, RDM, 0);
> +        u = FIELD_DP32(u, ID_ISAR5, VCMA, 0);
> +        cpu->isar.id_isar5 = u;
> +
> +        u = cpu->isar.id_isar6;
> +        u = FIELD_DP32(u, ID_ISAR6, DP, 0);
> +        u = FIELD_DP32(u, ID_ISAR6, FHM, 0);
> +        cpu->isar.id_isar6 = u;
> +
> +        u = cpu->isar.mvfr1;
> +        u = FIELD_DP32(u, MVFR1, SIMDLS, 0);
> +        u = FIELD_DP32(u, MVFR1, SIMDINT, 0);
> +        u = FIELD_DP32(u, MVFR1, SIMDSP, 0);
> +        u = FIELD_DP32(u, MVFR1, SIMDHP, 0);
> +        u = FIELD_DP32(u, MVFR1, SIMDFMAC, 0);
> +        cpu->isar.mvfr1 = u;
> +
> +        u = cpu->isar.mvfr2;
> +        u = FIELD_DP32(u, MVFR2, SIMDMISC, 0);
> +        cpu->isar.mvfr2 = u;
> +    }
> +
> +    if (!cpu->has_neon && !cpu->has_vfp) {
> +        uint64_t t;
> +        uint32_t u;
> +
> +        t = cpu->isar.id_aa64isar0;
> +        t = FIELD_DP64(t, ID_AA64ISAR0, FHM, 0);
> +        cpu->isar.id_aa64isar0 = t;
> +
> +        t = cpu->isar.id_aa64isar1;
> +        t = FIELD_DP64(t, ID_AA64ISAR1, FRINTTS, 0);
> +        cpu->isar.id_aa64isar1 = t;
> +
> +        u = cpu->isar.mvfr0;
> +        u = FIELD_DP32(u, MVFR0, SIMDREG, 0);
> +        cpu->isar.mvfr0 = u;
> +    }
> +
>      /* Some features automatically imply others: */
>      if (arm_feature(env, ARM_FEATURE_V8)) {
>          if (arm_feature(env, ARM_FEATURE_M)) {
> @@ -1016,12 +1160,6 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
>      if (arm_feature(env, ARM_FEATURE_V5)) {
>          set_feature(env, ARM_FEATURE_V4T);
>      }
> -    if (arm_feature(env, ARM_FEATURE_VFP4)) {
> -        set_feature(env, ARM_FEATURE_VFP3);
> -    }
> -    if (arm_feature(env, ARM_FEATURE_VFP3)) {
> -        set_feature(env, ARM_FEATURE_VFP);
> -    }
>      if (arm_feature(env, ARM_FEATURE_LPAE)) {
>          set_feature(env, ARM_FEATURE_V7MP);
>          set_feature(env, ARM_FEATURE_PXN);
> 

I really like the *_feature() API, it is very clean and powerful.

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [Qemu-devel] [Qemu-arm] [PATCH 1/4] target/arm: Allow VFP and Neon to be disabled via a CPU property

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> Allow VFP and neon to be disabled via a CPU property. As with
> the "pmu" property, we only allow these features to be removed
> from CPUs which have it by default, not added to CPUs which
> don't have it.
>
> The primary motivation here is to be able to optionally
> create Cortex-M33 CPUs with no FPU, but we provide switches
> for both VFP and Neon because the two interact:
>  * AArch64 can't have one without the other
>  * Some ID register fields only change if both are disabled
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  target/arm/cpu.h |   4 ++
>  target/arm/cpu.c | 150 +++++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 148 insertions(+), 6 deletions(-)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 733b840a712..778fb293e7c 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -797,6 +797,10 @@ struct ARMCPU {
>      bool has_el3;
>      /* CPU has PMU (Performance Monitor Unit) */
>      bool has_pmu;
> +    /* CPU has VFP */
> +    bool has_vfp;
> +    /* CPU has Neon */
> +    bool has_neon;
>
>      /* CPU has memory protection unit */
>      bool has_mpu;
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 8eee1d8c59a..406fd360a2a 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -763,6 +763,12 @@ static Property arm_cpu_cfgend_property =
>  static Property arm_cpu_has_pmu_property =
>              DEFINE_PROP_BOOL("pmu", ARMCPU, has_pmu, true);
>
> +static Property arm_cpu_has_vfp_property =
> +            DEFINE_PROP_BOOL("vfp", ARMCPU, has_vfp, true);
> +
> +static Property arm_cpu_has_neon_property =
> +            DEFINE_PROP_BOOL("neon", ARMCPU, has_neon, true);
> +
>  static Property arm_cpu_has_mpu_property =
>              DEFINE_PROP_BOOL("has-mpu", ARMCPU, has_mpu, true);
>
> @@ -803,6 +809,13 @@ void arm_cpu_post_init(Object *obj)
>      if (arm_feature(&cpu->env, ARM_FEATURE_M)) {
>          set_feature(&cpu->env, ARM_FEATURE_PMSA);
>      }
> +    /* Similarly for the VFP feature bits */
> +    if (arm_feature(&cpu->env, ARM_FEATURE_VFP4)) {
> +        set_feature(&cpu->env, ARM_FEATURE_VFP3);
> +    }
> +    if (arm_feature(&cpu->env, ARM_FEATURE_VFP3)) {
> +        set_feature(&cpu->env, ARM_FEATURE_VFP);
> +    }
>
>      if (arm_feature(&cpu->env, ARM_FEATURE_CBAR) ||
>          arm_feature(&cpu->env, ARM_FEATURE_CBAR_RO)) {
> @@ -847,6 +860,27 @@ void arm_cpu_post_init(Object *obj)
>                                   &error_abort);
>      }
>
> +    /*
> +     * Allow user to turn off VFP and Neon support, but only for TCG --
> +     * KVM does not currently allow us to lie to the guest about its
> +     * ID/feature registers, so the guest always sees what the host has.
> +     */
> +    if (arm_feature(&cpu->env, ARM_FEATURE_VFP)) {
> +        cpu->has_vfp = true;
> +        if (!kvm_enabled()) {
> +            qdev_property_add_static(DEVICE(obj), &arm_cpu_has_vfp_property,
> +                                     &error_abort);
> +        }
> +    }
> +
> +    if (arm_feature(&cpu->env, ARM_FEATURE_NEON)) {
> +        cpu->has_neon = true;
> +        if (!kvm_enabled()) {
> +            qdev_property_add_static(DEVICE(obj), &arm_cpu_has_neon_property,
> +                                     &error_abort);
> +        }
> +    }
> +
>      if (arm_feature(&cpu->env, ARM_FEATURE_PMSA)) {
>          qdev_property_add_static(DEVICE(obj), &arm_cpu_has_mpu_property,
>                                   &error_abort);
> @@ -956,6 +990,116 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
>          return;
>      }
>
> +    if (arm_feature(env, ARM_FEATURE_AARCH64) &&
> +        cpu->has_vfp != cpu->has_neon) {
> +        /*
> +         * This is an architectural requirement for AArch64; AArch32 is
> +         * more flexible and permits VFP-no-Neon and Neon-no-VFP.
> +         */
> +        error_setg(errp,
> +                   "AArch64 CPUs must have both VFP and Neon or neither");
> +        return;
> +    }
> +
> +    if (!cpu->has_vfp) {
> +        uint64_t t;
> +        uint32_t u;
> +
> +        unset_feature(env, ARM_FEATURE_VFP);
> +        unset_feature(env, ARM_FEATURE_VFP3);
> +        unset_feature(env, ARM_FEATURE_VFP4);
> +
> +        t = cpu->isar.id_aa64isar1;
> +        t = FIELD_DP64(t, ID_AA64ISAR1, JSCVT, 0);
> +        cpu->isar.id_aa64isar1 = t;
> +
> +        t = cpu->isar.id_aa64pfr0;
> +        t = FIELD_DP64(t, ID_AA64PFR0, FP, 0xf);
> +        cpu->isar.id_aa64pfr0 = t;
> +
> +        u = cpu->isar.id_isar6;
> +        u = FIELD_DP32(u, ID_ISAR6, JSCVT, 0);
> +        cpu->isar.id_isar6 = u;
> +
> +        u = cpu->isar.mvfr0;
> +        u = FIELD_DP32(u, MVFR0, FPSP, 0);
> +        u = FIELD_DP32(u, MVFR0, FPDP, 0);
> +        u = FIELD_DP32(u, MVFR0, FPTRAP, 0);
> +        u = FIELD_DP32(u, MVFR0, FPDIVIDE, 0);
> +        u = FIELD_DP32(u, MVFR0, FPSQRT, 0);
> +        u = FIELD_DP32(u, MVFR0, FPSHVEC, 0);
> +        u = FIELD_DP32(u, MVFR0, FPROUND, 0);
> +        cpu->isar.mvfr0 = u;
> +
> +        u = cpu->isar.mvfr1;
> +        u = FIELD_DP32(u, MVFR1, FPFTZ, 0);
> +        u = FIELD_DP32(u, MVFR1, FPDNAN, 0);
> +        u = FIELD_DP32(u, MVFR1, FPHP, 0);
> +        cpu->isar.mvfr1 = u;
> +
> +        u = cpu->isar.mvfr2;
> +        u = FIELD_DP32(u, MVFR2, FPMISC, 0);
> +        cpu->isar.mvfr2 = u;
> +    }
> +
> +    if (!cpu->has_neon) {
> +        uint64_t t;
> +        uint32_t u;
> +
> +        unset_feature(env, ARM_FEATURE_NEON);
> +
> +        t = cpu->isar.id_aa64isar0;
> +        t = FIELD_DP64(t, ID_AA64ISAR0, DP, 0);
> +        cpu->isar.id_aa64isar0 = t;
> +
> +        t = cpu->isar.id_aa64isar1;
> +        t = FIELD_DP64(t, ID_AA64ISAR1, FCMA, 0);
> +        cpu->isar.id_aa64isar1 = t;
> +
> +        t = cpu->isar.id_aa64pfr0;
> +        t = FIELD_DP64(t, ID_AA64PFR0, ADVSIMD, 0xf);
> +        cpu->isar.id_aa64pfr0 = t;
> +
> +        u = cpu->isar.id_isar5;
> +        u = FIELD_DP32(u, ID_ISAR5, RDM, 0);
> +        u = FIELD_DP32(u, ID_ISAR5, VCMA, 0);
> +        cpu->isar.id_isar5 = u;
> +
> +        u = cpu->isar.id_isar6;
> +        u = FIELD_DP32(u, ID_ISAR6, DP, 0);
> +        u = FIELD_DP32(u, ID_ISAR6, FHM, 0);
> +        cpu->isar.id_isar6 = u;
> +
> +        u = cpu->isar.mvfr1;
> +        u = FIELD_DP32(u, MVFR1, SIMDLS, 0);
> +        u = FIELD_DP32(u, MVFR1, SIMDINT, 0);
> +        u = FIELD_DP32(u, MVFR1, SIMDSP, 0);
> +        u = FIELD_DP32(u, MVFR1, SIMDHP, 0);
> +        u = FIELD_DP32(u, MVFR1, SIMDFMAC, 0);
> +        cpu->isar.mvfr1 = u;
> +
> +        u = cpu->isar.mvfr2;
> +        u = FIELD_DP32(u, MVFR2, SIMDMISC, 0);
> +        cpu->isar.mvfr2 = u;
> +    }
> +
> +    if (!cpu->has_neon && !cpu->has_vfp) {
> +        uint64_t t;
> +        uint32_t u;
> +
> +        t = cpu->isar.id_aa64isar0;
> +        t = FIELD_DP64(t, ID_AA64ISAR0, FHM, 0);
> +        cpu->isar.id_aa64isar0 = t;
> +
> +        t = cpu->isar.id_aa64isar1;
> +        t = FIELD_DP64(t, ID_AA64ISAR1, FRINTTS, 0);
> +        cpu->isar.id_aa64isar1 = t;
> +
> +        u = cpu->isar.mvfr0;
> +        u = FIELD_DP32(u, MVFR0, SIMDREG, 0);
> +        cpu->isar.mvfr0 = u;
> +    }
> +
>      /* Some features automatically imply others: */
>      if (arm_feature(env, ARM_FEATURE_V8)) {
>          if (arm_feature(env, ARM_FEATURE_M)) {
> @@ -1016,12 +1160,6 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
>      if (arm_feature(env, ARM_FEATURE_V5)) {
>          set_feature(env, ARM_FEATURE_V4T);
>      }
> -    if (arm_feature(env, ARM_FEATURE_VFP4)) {
> -        set_feature(env, ARM_FEATURE_VFP3);
> -    }
> -    if (arm_feature(env, ARM_FEATURE_VFP3)) {
> -        set_feature(env, ARM_FEATURE_VFP);
> -    }
>      if (arm_feature(env, ARM_FEATURE_LPAE)) {
>          set_feature(env, ARM_FEATURE_V7MP);
>          set_feature(env, ARM_FEATURE_PXN);


--
Alex Bennée

Re: [Qemu-devel] [PATCH 2/4] target/arm: Allow M-profile CPUs to disable the DSP extension via CPU property

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> Allow the DSP extension to be disabled via a CPU property for
> M-profile CPUs. (A and R-profile CPUs don't have this extension
> as a defined separate optional architecture extension, so
> they don't need the property.)
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  target/arm/cpu.h |  2 ++
>  target/arm/cpu.c | 29 +++++++++++++++++++++++++++++
>  2 files changed, 31 insertions(+)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 778fb293e7c..b1c7ab3ee94 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -801,6 +801,8 @@ struct ARMCPU {
>      bool has_vfp;
>      /* CPU has Neon */
>      bool has_neon;
> +    /* CPU has M-profile DSP extension */
> +    bool has_dsp;
>
>      /* CPU has memory protection unit */
>      bool has_mpu;
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 406fd360a2a..c43139ba897 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -769,6 +769,9 @@ static Property arm_cpu_has_vfp_property =
>  static Property arm_cpu_has_neon_property =
>              DEFINE_PROP_BOOL("neon", ARMCPU, has_neon, true);
>
> +static Property arm_cpu_has_dsp_property =
> +            DEFINE_PROP_BOOL("dsp", ARMCPU, has_dsp, true);
> +
>  static Property arm_cpu_has_mpu_property =
>              DEFINE_PROP_BOOL("has-mpu", ARMCPU, has_mpu, true);
>
> @@ -881,6 +884,12 @@ void arm_cpu_post_init(Object *obj)
>          }
>      }
>
> +    if (arm_feature(&cpu->env, ARM_FEATURE_M) &&
> +        arm_feature(&cpu->env, ARM_FEATURE_THUMB_DSP)) {
> +        qdev_property_add_static(DEVICE(obj), &arm_cpu_has_dsp_property,
> +                                 &error_abort);
> +    }
> +
>      if (arm_feature(&cpu->env, ARM_FEATURE_PMSA)) {
>          qdev_property_add_static(DEVICE(obj), &arm_cpu_has_mpu_property,
>                                   &error_abort);
> @@ -1100,6 +1109,26 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
>          cpu->isar.mvfr0 = u;
>      }
>
> +    if (arm_feature(env, ARM_FEATURE_M) && !cpu->has_dsp) {
> +        uint32_t u;
> +
> +        unset_feature(env, ARM_FEATURE_THUMB_DSP);
> +
> +        u = cpu->isar.id_isar1;
> +        u = FIELD_DP32(u, ID_ISAR1, EXTEND, 1);
> +        cpu->isar.id_isar1 = u;
> +
> +        u = cpu->isar.id_isar2;
> +        u = FIELD_DP32(u, ID_ISAR2, MULTU, 1);
> +        u = FIELD_DP32(u, ID_ISAR2, MULTS, 1);
> +        cpu->isar.id_isar2 = u;
> +
> +        u = cpu->isar.id_isar3;
> +        u = FIELD_DP32(u, ID_ISAR3, SIMD, 1);
> +        u = FIELD_DP32(u, ID_ISAR3, SATURATE, 0);
> +        cpu->isar.id_isar3 = u;
> +    }
> +
>      /* Some features automatically imply others: */
>      if (arm_feature(env, ARM_FEATURE_V8)) {
>          if (arm_feature(env, ARM_FEATURE_M)) {


--
Alex Bennée

Re: [Qemu-devel] [Qemu-arm] [PATCH 3/4] hw/arm/armv7m: Forward "vfp" and "dsp" properties to CPU

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> Create "vfp" and "dsp" properties on the armv7m container object
> which will be forwarded to its CPU object, so that SoCs can
> configure whether the CPU has these features.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  include/hw/arm/armv7m.h |  4 ++++
>  hw/arm/armv7m.c         | 18 ++++++++++++++++++
>  2 files changed, 22 insertions(+)
>
> diff --git a/include/hw/arm/armv7m.h b/include/hw/arm/armv7m.h
> index e96a98f8093..d2c74d3872a 100644
> --- a/include/hw/arm/armv7m.h
> +++ b/include/hw/arm/armv7m.h
> @@ -43,6 +43,8 @@ typedef struct {
>   *   devices will be automatically layered on top of this view.)
>   * + Property "idau": IDAU interface (forwarded to CPU object)
>   * + Property "init-svtor": secure VTOR reset value (forwarded to CPU object)
> + * + Property "vfp": enable VFP (forwarded to CPU object)
> + * + Property "dsp": enable DSP (forwarded to CPU object)
>   * + Property "enable-bitband": expose bitbanded IO
>   */
>  typedef struct ARMv7MState {
> @@ -66,6 +68,8 @@ typedef struct ARMv7MState {
>      uint32_t init_svtor;
>      bool enable_bitband;
>      bool start_powered_off;
> +    bool vfp;
> +    bool dsp;
>  } ARMv7MState;
>
>  #endif
> diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
> index c4b2a9a1f5c..7caf9bd3364 100644
> --- a/hw/arm/armv7m.c
> +++ b/hw/arm/armv7m.c
> @@ -190,6 +190,22 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
>              return;
>          }
>      }
> +    if (object_property_find(OBJECT(s->cpu), "vfp", NULL)) {
> +        object_property_set_bool(OBJECT(s->cpu), s->vfp,
> +                                 "vfp", &err);
> +        if (err != NULL) {
> +            error_propagate(errp, err);
> +            return;
> +        }
> +    }
> +    if (object_property_find(OBJECT(s->cpu), "dsp", NULL)) {
> +        object_property_set_bool(OBJECT(s->cpu), s->dsp,
> +                                 "dsp", &err);
> +        if (err != NULL) {
> +            error_propagate(errp, err);
> +            return;
> +        }
> +    }
>
>      /*
>       * Tell the CPU where the NVIC is; it will fail realize if it doesn't
> @@ -260,6 +276,8 @@ static Property armv7m_properties[] = {
>      DEFINE_PROP_BOOL("enable-bitband", ARMv7MState, enable_bitband, false),
>      DEFINE_PROP_BOOL("start-powered-off", ARMv7MState, start_powered_off,
>                       false),
> +    DEFINE_PROP_BOOL("vfp", ARMv7MState, vfp, true),
> +    DEFINE_PROP_BOOL("dsp", ARMv7MState, dsp, true),
>      DEFINE_PROP_END_OF_LIST(),
>  };


--
Alex Bennée

Re: [Qemu-devel] [Qemu-arm] [PATCH 4/4] hw/arm: Correctly disable FPU/DSP for some ARMSSE-based boards

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> The SSE-200 hardware has configurable integration settings which
> determine whether its two CPUs have the FPU and DSP:
>  * CPU0_FPU (default 0)
>  * CPU0_DSP (default 0)
>  * CPU1_FPU (default 1)
>  * CPU1_DSP (default 1)
>
> Similarly, the IoTKit has settings for its single CPU:
>  * CPU0_FPU (default 1)
>  * CPU0_DSP (default 1)
>
> Of our four boards that use either the IoTKit or the SSE-200:
>  * mps2-an505, mps2-an521 and musca-a use the default settings
>  * musca-b1 enables FPU and DSP on both CPUs
>
> Currently QEMU models all these boards using CPUs with
> both FPU and DSP enabled. This means that we are incorrect
> for mps2-an521 and musca-a, which should not have FPU or DSP
> on CPU0.
>
> Create QOM properties on the ARMSSE devices corresponding to the
> default h/w integration settings, and make the Musca-B1 board
> enable FPU and DSP on both CPUs. This fixes the mps2-an521
> and musca-a behaviour, and leaves the musca-b1 and mps2-an505
> behaviour unchanged.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Given the ever growing configurable nature of the v7m platform what do we do
to ensure the various combinations are tested on instantiating qemu? Or is
this a case of relying on the wider community to shout if users actually
find a combination that breaks? I guess fuzz testing would be a bit of a
sledgehammer approach.

Anyway:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  include/hw/arm/armsse.h |  7 +++++
>  hw/arm/armsse.c         | 58 ++++++++++++++++++++++++++++++++---------
>  hw/arm/musca.c          |  8 ++++++
>  3 files changed, 61 insertions(+), 12 deletions(-)
>
> diff --git a/include/hw/arm/armsse.h b/include/hw/arm/armsse.h
> index 81e082cccf8..84080c22993 100644
> --- a/include/hw/arm/armsse.h
> +++ b/include/hw/arm/armsse.h
> @@ -50,6 +50,11 @@
>   *    address of each SRAM bank (and thus the total amount of internal SRAM)
>   *  + QOM property "init-svtor" sets the initial value of the CPU SVTOR register
>   *    (where it expects to load the PC and SP from the vector table on reset)
> + *  + QOM properties "CPU0_FPU", "CPU0_DSP", "CPU1_FPU" and "CPU1_DSP" which
> + *    set whether the CPUs have the FPU and DSP features present. The default
> + *    (matching the hardware) is that for CPU0 in an IoTKit and CPU1 in an
> + *    SSE-200 both are present; CPU0 in an SSE-200 has neither.
> + *    Since the IoTKit has only one CPU, it does not have the CPU1_* properties.
>   *  + Named GPIO inputs "EXP_IRQ" 0..n are the expansion interrupts for CPU 0,
>   *    which are wired to its NVIC lines 32 .. n+32
>   *  + Named GPIO inputs "EXP_CPU1_IRQ" 0..n are the expansion interrupts for
> @@ -208,6 +213,8 @@ typedef struct ARMSSE {
>      uint32_t mainclk_frq;
>      uint32_t sram_addr_width;
>      uint32_t init_svtor;
> +    bool cpu_fpu[SSE_MAX_CPUS];
> +    bool cpu_dsp[SSE_MAX_CPUS];
>  } ARMSSE;
>
>  typedef struct ARMSSEInfo ARMSSEInfo;
> diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c
> index 76cc6905798..e138aee673f 100644
> --- a/hw/arm/armsse.c
> +++ b/hw/arm/armsse.c
> @@ -37,6 +37,33 @@ struct ARMSSEInfo {
>      bool has_cachectrl;
>      bool has_cpusecctrl;
>      bool has_cpuid;
> +    Property *props;
> +};
> +
> +static Property iotkit_properties[] = {
> +    DEFINE_PROP_LINK("memory", ARMSSE, board_memory, TYPE_MEMORY_REGION,
> +                     MemoryRegion *),
> +    DEFINE_PROP_UINT32("EXP_NUMIRQ", ARMSSE, exp_numirq, 64),
> +    DEFINE_PROP_UINT32("MAINCLK", ARMSSE, mainclk_frq, 0),
> +    DEFINE_PROP_UINT32("SRAM_ADDR_WIDTH", ARMSSE, sram_addr_width, 15),
> +    DEFINE_PROP_UINT32("init-svtor", ARMSSE, init_svtor, 0x10000000),
> +    DEFINE_PROP_BOOL("CPU0_FPU", ARMSSE, cpu_fpu[0], true),
> +    DEFINE_PROP_BOOL("CPU0_DSP", ARMSSE, cpu_dsp[0], true),
> +    DEFINE_PROP_END_OF_LIST()
> +};
> +
> +static Property armsse_properties[] = {
> +    DEFINE_PROP_LINK("memory", ARMSSE, board_memory, TYPE_MEMORY_REGION,
> +                     MemoryRegion *),
> +    DEFINE_PROP_UINT32("EXP_NUMIRQ", ARMSSE, exp_numirq, 64),
> +    DEFINE_PROP_UINT32("MAINCLK", ARMSSE, mainclk_frq, 0),
> +    DEFINE_PROP_UINT32("SRAM_ADDR_WIDTH", ARMSSE, sram_addr_width, 15),
> +    DEFINE_PROP_UINT32("init-svtor", ARMSSE, init_svtor, 0x10000000),
> +    DEFINE_PROP_BOOL("CPU0_FPU", ARMSSE, cpu_fpu[0], false),
> +    DEFINE_PROP_BOOL("CPU0_DSP", ARMSSE, cpu_dsp[0], false),
> +    DEFINE_PROP_BOOL("CPU1_FPU", ARMSSE, cpu_fpu[1], true),
> +    DEFINE_PROP_BOOL("CPU1_DSP", ARMSSE, cpu_dsp[1], true),
> +    DEFINE_PROP_END_OF_LIST()
>  };
>
>  static const ARMSSEInfo armsse_variants[] = {
> @@ -52,6 +79,7 @@ static const ARMSSEInfo armsse_variants[] = {
>          .has_cachectrl = false,
>          .has_cpusecctrl = false,
>          .has_cpuid = false,
> +        .props = iotkit_properties,
>      },
>      {
>          .name = TYPE_SSE200,
> @@ -65,6 +93,7 @@ static const ARMSSEInfo armsse_variants[] = {
>          .has_cachectrl = true,
>          .has_cpusecctrl = true,
>          .has_cpuid = true,
> +        .props = armsse_properties,
>      },
>  };
>
> @@ -532,6 +561,20 @@ static void armsse_realize(DeviceState *dev, Error **errp)
>                  return;
>              }
>          }
> +        if (!s->cpu_fpu[i]) {
> +            object_property_set_bool(cpuobj, false, "vfp", &err);
> +            if (err) {
> +                error_propagate(errp, err);
> +                return;
> +            }
> +        }
> +        if (!s->cpu_dsp[i]) {
> +            object_property_set_bool(cpuobj, false, "dsp", &err);
> +            if (err) {
> +                error_propagate(errp, err);
> +                return;
> +            }
> +        }
>
>          if (i > 0) {
>              memory_region_add_subregion_overlap(&s->cpu_container[i], 0,
> @@ -1221,16 +1264,6 @@ static const VMStateDescription armsse_vmstate = {
>      }
>  };
>
> -static Property armsse_properties[] = {
> -    DEFINE_PROP_LINK("memory", ARMSSE, board_memory, TYPE_MEMORY_REGION,
> -                     MemoryRegion *),
> -    DEFINE_PROP_UINT32("EXP_NUMIRQ", ARMSSE, exp_numirq, 64),
> -    DEFINE_PROP_UINT32("MAINCLK", ARMSSE, mainclk_frq, 0),
> -    DEFINE_PROP_UINT32("SRAM_ADDR_WIDTH", ARMSSE, sram_addr_width, 15),
> -    DEFINE_PROP_UINT32("init-svtor", ARMSSE, init_svtor, 0x10000000),
> -    DEFINE_PROP_END_OF_LIST()
> -};
> -
>  static void armsse_reset(DeviceState *dev)
>  {
>      ARMSSE *s = ARMSSE(dev);
> @@ -1243,13 +1276,14 @@ static void armsse_class_init(ObjectClass *klass, void *data)
>      DeviceClass *dc = DEVICE_CLASS(klass);
>      IDAUInterfaceClass *iic = IDAU_INTERFACE_CLASS(klass);
>      ARMSSEClass *asc = ARMSSE_CLASS(klass);
> +    const ARMSSEInfo *info = data;
>
>      dc->realize = armsse_realize;
>      dc->vmsd = &armsse_vmstate;
> -    dc->props = armsse_properties;
> +    dc->props = info->props;
>      dc->reset = armsse_reset;
>      iic->check = armsse_idau_check;
> -    asc->info = data;
> +    asc->info = info;
>  }
>
>  static const TypeInfo armsse_info = {
> diff --git a/hw/arm/musca.c b/hw/arm/musca.c
> index 23aff43f4bc..736f37b774c 100644
> --- a/hw/arm/musca.c
> +++ b/hw/arm/musca.c
> @@ -385,6 +385,14 @@ static void musca_init(MachineState *machine)
>      qdev_prop_set_uint32(ssedev, "init-svtor", mmc->init_svtor);
>      qdev_prop_set_uint32(ssedev, "SRAM_ADDR_WIDTH", mmc->sram_addr_width);
>      qdev_prop_set_uint32(ssedev, "MAINCLK", SYSCLK_FRQ);
> +    /*
> +     * Musca-A takes the default SSE-200 FPU/DSP settings (ie no for
> +     * CPU0 and yes for CPU1); Musca-B1 explicitly enables them for CPU0.
> +     */
> +    if (mmc->type == MUSCA_B1) {
> +        qdev_prop_set_bit(ssedev, "CPU0_FPU", true);
> +        qdev_prop_set_bit(ssedev, "CPU0_DSP", true);
> +    }
>      object_property_set_bool(OBJECT(&mms->sse), true, "realized",
>                               &error_fatal);


--
Alex Bennée

Re: [Qemu-devel] [Qemu-arm] [PATCH 4/4] hw/arm: Correctly disable FPU/DSP for some ARMSSE-based boards

[Peter Maydell]

On Thu, 13 Jun 2019 at 16:13, Alex Bennée <alex.bennee@linaro.org> wrote:
> Given the ever growing configurable nature of the v7m platform what do we do
> to ensure the various combinations are tested on instantiating qemu? Or is
> this a case of relying on the wider community to shout if users actually
> find a combination that breaks? I guess fuzz testing would be a bit of a
> sledgehammer approach.

It's not really all that much more configurable than the A-profile
cores; I've just been a bit more careful to actually make our
emulation reflect what the hardware actually does... a really
careful testing of the A profile cores would want to test
the various different cortex-* CPUs individually to make
sure they UNDEFfed on the features they didn't have, and so on.

thanks
-- PMM