[Qemu-devel] [PATCH 09/15] target/arm: Don't store M profile PRIMASK and FAULTMASK in daif

[Peter Maydell]

We currently store the M profile CPU register state PRIMASK and
FAULTMASK in the daif field of the CPU state in its I and F
bits. This is a legacy from the original implementation, which
tried to share the cpu_exec_interrupt code between A profile
and M profile. We've since separated out the two cases because
they are significantly different, so now there is no common
code between M and A profile which looks at env->daif: all the
uses are either in A-only or M-only code paths. Sharing the state
fields now is just confusing, and will make things awkward
when we implement v8M, where the PRIMASK and FAULTMASK
registers are banked between security states.

Switch M profile over to using v7m.faultmask and v7m.primask
fields for these registers.

Signed-off-by: Peter Maydell <address@hidden>
---
 hw/intc/armv7m_nvic.c |  4 ++--
 target/arm/cpu.c      |  5 -----
 target/arm/cpu.h      |  4 +++-
 target/arm/helper.c   | 18 +++++-------------
 target/arm/machine.c  | 33 +++++++++++++++++++++++++++++++++
 5 files changed, 43 insertions(+), 21 deletions(-)

diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 2e8166a..343bc16 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -167,9 +167,9 @@ static inline int nvic_exec_prio(NVICState *s)
     CPUARMState *env = &s->cpu->env;
     int running;
 
-    if (env->daif & PSTATE_F) { /* FAULTMASK */
+    if (env->v7m.faultmask) {
         running = -1;
-    } else if (env->daif & PSTATE_I) { /* PRIMASK */
+    } else if (env->v7m.primask) {
         running = 0;
     } else if (env->v7m.basepri > 0) {
         running = env->v7m.basepri & nvic_gprio_mask(s);
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 05c038b..b241a63 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -185,11 +185,6 @@ static void arm_cpu_reset(CPUState *s)
         uint32_t initial_pc; /* Loaded from 0x4 */
         uint8_t *rom;
 
-        /* For M profile we store FAULTMASK and PRIMASK in the
-         * PSTATE F and I bits; these are both clear at reset.
-         */
-        env->daif &= ~(PSTATE_I | PSTATE_F);
-
         /* The reset value of this bit is IMPDEF, but ARM recommends
          * that it resets to 1, so QEMU always does that rather than making
          * it dependent on CPU model.
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 1f06de0..da90b7a 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -418,6 +418,8 @@ typedef struct CPUARMState {
         uint32_t bfar; /* BusFault Address */
         unsigned mpu_ctrl; /* MPU_CTRL */
         int exception;
+        uint32_t primask;
+        uint32_t faultmask;
     } v7m;
 
     /* Information associated with an exception about to be taken:
@@ -2179,7 +2181,7 @@ static inline int cpu_mmu_index(CPUARMState *env, bool 
ifetch)
          * we're in a HardFault or NMI handler.
          */
         if ((env->v7m.exception > 0 && env->v7m.exception <= 3)
-            || env->daif & PSTATE_F) {
+            || env->v7m.faultmask) {
             return arm_to_core_mmu_idx(ARMMMUIdx_MNegPri);
         }
 
diff --git a/target/arm/helper.c b/target/arm/helper.c
index f087d42..b64ddb1 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6172,7 +6172,7 @@ static void do_v7m_exception_exit(ARMCPU *cpu)
 
     if (env->v7m.exception != ARMV7M_EXCP_NMI) {
         /* Auto-clear FAULTMASK on return from other than NMI */
-        env->daif &= ~PSTATE_F;
+        env->v7m.faultmask = 0;
     }
 
     switch (armv7m_nvic_complete_irq(env->nvic, env->v7m.exception)) {
@@ -8718,12 +8718,12 @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg)
         return (env->v7m.control & R_V7M_CONTROL_SPSEL_MASK) ?
             env->regs[13] : env->v7m.other_sp;
     case 16: /* PRIMASK */
-        return (env->daif & PSTATE_I) != 0;
+        return env->v7m.primask;
     case 17: /* BASEPRI */
     case 18: /* BASEPRI_MAX */
         return env->v7m.basepri;
     case 19: /* FAULTMASK */
-        return (env->daif & PSTATE_F) != 0;
+        return env->v7m.faultmask;
     default:
         qemu_log_mask(LOG_GUEST_ERROR, "Attempt to read unknown special"
                                        " register %d\n", reg);
@@ -8778,11 +8778,7 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, 
uint32_t val)
         }
         break;
     case 16: /* PRIMASK */
-        if (val & 1) {
-            env->daif |= PSTATE_I;
-        } else {
-            env->daif &= ~PSTATE_I;
-        }
+        env->v7m.primask = val & 1;
         break;
     case 17: /* BASEPRI */
         env->v7m.basepri = val & 0xff;
@@ -8793,11 +8789,7 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, 
uint32_t val)
             env->v7m.basepri = val;
         break;
     case 19: /* FAULTMASK */
-        if (val & 1) {
-            env->daif |= PSTATE_F;
-        } else {
-            env->daif &= ~PSTATE_F;
-        }
+        env->v7m.faultmask = val & 1;
         break;
     case 20: /* CONTROL */
         /* Writing to the SPSEL bit only has an effect if we are in
diff --git a/target/arm/machine.c b/target/arm/machine.c
index 1f66da4..2fb4b76 100644
--- a/target/arm/machine.c
+++ b/target/arm/machine.c
@@ -97,6 +97,17 @@ static bool m_needed(void *opaque)
     return arm_feature(env, ARM_FEATURE_M);
 }
 
+static const VMStateDescription vmstate_m_faultmask_primask = {
+    .name = "cpu/m/faultmask-primask",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(env.v7m.faultmask, ARMCPU),
+        VMSTATE_UINT32(env.v7m.primask, ARMCPU),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const VMStateDescription vmstate_m = {
     .name = "cpu/m",
     .version_id = 4,
@@ -115,6 +126,10 @@ static const VMStateDescription vmstate_m = {
         VMSTATE_UINT32(env.v7m.mpu_ctrl, ARMCPU),
         VMSTATE_INT32(env.v7m.exception, ARMCPU),
         VMSTATE_END_OF_LIST()
+    },
+    .subsections = (const VMStateDescription*[]) {
+        &vmstate_m_faultmask_primask,
+        NULL
     }
 };
 
@@ -201,6 +216,24 @@ static int get_cpsr(QEMUFile *f, void *opaque, size_t size,
     CPUARMState *env = &cpu->env;
     uint32_t val = qemu_get_be32(f);
 
+    if (arm_feature(env, ARM_FEATURE_M)) {
+        /* If the I or F bits are set then this is a migration from
+         * an old QEMU which still stored the M profile FAULTMASK
+         * and PRIMASK in env->daif. Set v7m.faultmask and v7m.primask
+         * accordingly, and then clear the bits so they don't confuse
+         * cpsr_write(). For a new QEMU, the bits here will always be
+         * clear, and the data is transferred using the
+         * vmstate_m_faultmask_primask subsection.
+         */
+        if (val & CPSR_F) {
+            env->v7m.faultmask = 1;
+        }
+        if (val & CPSR_I) {
+            env->v7m.primask = 1;
+        }
+        val &= ~(CPSR_F | CPSR_I);
+    }
+
     env->aarch64 = ((val & PSTATE_nRW) == 0);
 
     if (is_a64(env)) {
-- 
2.7.4