[Qemu-devel] [PATCH v2 07/10] target-arm: remove last users of cpsr_write

["Alex Bennée" <alex.bennee@linaro.org>]

And use the new machinery to to save and restore program state. The old
cpsr_write function did some special handling for mode switches which
has been moved into the helper function.

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

---
v2:
  - rebase
  - add mask helper function
  - checkpatch fixes

diff --git a/linux-user/main.c b/linux-user/main.c
index 9101541..5f7cc31 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -4184,7 +4184,7 @@ int main(int argc, char **argv, char **envp)
 #elif defined(TARGET_ARM)
     {
         int i;
-        cpsr_write(env, regs->uregs[16], 0xffffffff);
+        restore_state_from_spsr(env, regs->uregs[16]);
         for(i = 0; i < 16; i++) {
             env->regs[i] = regs->uregs[i];
         }
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 9c6727b..b6f9ef4 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -1599,38 +1599,39 @@ get_sigframe(struct target_sigaction *ka, CPUARMState *regs, int framesize)
 
 static void
 setup_return(CPUARMState *env, struct target_sigaction *ka,
-	     abi_ulong *rc, abi_ulong frame_addr, int usig, abi_ulong rc_addr)
+             abi_ulong *rc, abi_ulong frame_addr, int usig, abi_ulong rc_addr)
 {
-	abi_ulong handler = ka->_sa_handler;
-	abi_ulong retcode;
-	int thumb = handler & 1;
-	uint32_t cpsr = save_state_to_spsr(env);
+        abi_ulong handler = ka->_sa_handler;
+        abi_ulong retcode;
+        int thumb = handler & 1;
+        uint32_t cpsr = save_state_to_spsr(env);
 
-	cpsr &= ~CPSR_IT;
-	if (thumb) {
-		cpsr |= CPSR_T;
-	} else {
-		cpsr &= ~CPSR_T;
-	}
+        cpsr &= ~CPSR_IT;
+        if (thumb) {
+                cpsr |= CPSR_T;
+        } else {
+                cpsr &= ~CPSR_T;
+        }
 
-	if (ka->sa_flags & TARGET_SA_RESTORER) {
-		retcode = ka->sa_restorer;
-	} else {
-		unsigned int idx = thumb;
+        if (ka->sa_flags & TARGET_SA_RESTORER) {
+                retcode = ka->sa_restorer;
+        } else {
+                unsigned int idx = thumb;
 
-		if (ka->sa_flags & TARGET_SA_SIGINFO)
-			idx += 2;
+                if (ka->sa_flags & TARGET_SA_SIGINFO) {
+                        idx += 2;
+                }
 
-        __put_user(retcodes[idx], rc);
+                __put_user(retcodes[idx], rc);
 
-		retcode = rc_addr + thumb;
-	}
+                retcode = rc_addr + thumb;
+        }
 
-	env->regs[0] = usig;
-	env->regs[13] = frame_addr;
-	env->regs[14] = retcode;
-	env->regs[15] = handler & (thumb ? ~1 : ~3);
-	cpsr_write(env, cpsr, 0xffffffff);
+        env->regs[0] = usig;
+        env->regs[13] = frame_addr;
+        env->regs[14] = retcode;
+        env->regs[15] = handler & (thumb ? ~1 : ~3);
+        restore_state_from_spsr(env, cpsr);
 }
 
 static abi_ulong *setup_sigframe_v2_vfp(abi_ulong *regspace, CPUARMState *env)
@@ -1858,12 +1859,14 @@ restore_sigcontext(CPUARMState *env, struct target_sigcontext *sc)
     __get_user(env->regs[15], &sc->arm_pc);
 #ifdef TARGET_CONFIG_CPU_32
     __get_user(cpsr, &sc->arm_cpsr);
-        cpsr_write(env, cpsr, CPSR_USER | CPSR_EXEC);
+    restore_state_from_masked_spsr(env,
+                                   (CPSR_USER | CPSR_EXEC),
+                                   cpsr);
 #endif
 
-	err |= !valid_user_regs(env);
+    err |= !valid_user_regs(env);
 
-	return err;
+    return err;
 }
 
 static long do_sigreturn_v1(CPUARMState *env)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 3f23167..b56f1a8 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -795,6 +795,15 @@ static inline void restore_state_from_spsr(CPUARMState *env,
     }
 }
 
+/* Restore a few masked bits of the program state */
+static inline void restore_state_from_masked_spsr(CPUARMState *env,
+                                                  uint32_t mask,
+                                                  uint32_t saved_state)
+{
+    uint32_t spsr = (save_state_to_spsr(env) & ~mask);
+    spsr |= (saved_state & mask);
+    return restore_state_from_spsr(env, spsr);
+}
 
 void arm_cpu_list(FILE *f, fprintf_function cpu_fprintf);
 
diff --git a/target-arm/gdbstub.c b/target-arm/gdbstub.c
index ec25f30..5e60589 100644
--- a/target-arm/gdbstub.c
+++ b/target-arm/gdbstub.c
@@ -93,8 +93,12 @@ int arm_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
         }
         return 4;
     case 25:
-        /* CPSR */
-        cpsr_write(env, tmp, 0xffffffff);
+        /* CPSR
+         * FIXME?: as restore_state_from_spsr() doesn't do aarch32
+         * special mode fixups this may break. However GDB doesn't
+         * seem to be able to handle tracing over a mode switch anyway
+         */
+        restore_state_from_spsr(env, tmp);
         return 4;
     }
     /* Unknown register.  */
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 030bcdd..6d755c0 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2970,68 +2970,6 @@ void arm_cp_reset_ignore(CPUARMState *env, const ARMCPRegInfo *opaque)
     /* Helper coprocessor reset function for do-nothing-on-reset registers */
 }
 
-static int bad_mode_switch(CPUARMState *env, int mode)
-{
-    /* Return true if it is not valid for us to switch to
-     * this CPU mode (ie all the UNPREDICTABLE cases in
-     * the ARM ARM CPSRWriteByInstr pseudocode).
-     */
-    switch (mode) {
-    case ARM_CPU_MODE_USR:
-    case ARM_CPU_MODE_SYS:
-    case ARM_CPU_MODE_SVC:
-    case ARM_CPU_MODE_ABT:
-    case ARM_CPU_MODE_UND:
-    case ARM_CPU_MODE_IRQ:
-    case ARM_CPU_MODE_FIQ:
-        return 0;
-    default:
-        return 1;
-    }
-}
-
-void cpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)
-{
-    if (mask & CPSR_NZCV) {
-        env->ZF = (~val) & CPSR_Z;
-        env->NF = val;
-        env->CF = (val >> 29) & 1;
-        env->VF = (val << 3) & 0x80000000;
-    }
-    if (mask & CPSR_Q)
-        env->QF = ((val & CPSR_Q) != 0);
-    if (mask & CPSR_T)
-        env->thumb = ((val & CPSR_T) != 0);
-    if (mask & CPSR_IT_0_1) {
-        env->condexec_bits &= ~3;
-        env->condexec_bits |= (val >> 25) & 3;
-    }
-    if (mask & CPSR_IT_2_7) {
-        env->condexec_bits &= 3;
-        env->condexec_bits |= (val >> 8) & 0xfc;
-    }
-    if (mask & CPSR_GE) {
-        env->GE = (val >> 16) & 0xf;
-    }
-
-    env->daif &= ~(CPSR_AIF & mask);
-    env->daif |= val & CPSR_AIF & mask;
-
-    if ((env->uncached_cpsr ^ val) & mask & CPSR_M) {
-        if (bad_mode_switch(env, val & CPSR_M)) {
-            /* Attempt to switch to an invalid mode: this is UNPREDICTABLE.
-             * We choose to ignore the attempt and leave the CPSR M field
-             * untouched.
-             */
-            mask &= ~CPSR_M;
-        } else {
-            switch_mode(env, val & CPSR_M);
-        }
-    }
-    mask &= ~CACHED_CPSR_BITS;
-    env->uncached_cpsr = (env->uncached_cpsr & ~mask) | (val & mask);
-}
-
 /* Sign/zero extend */
 uint32_t HELPER(sxtb16)(uint32_t x)
 {
diff --git a/target-arm/kvm32.c b/target-arm/kvm32.c
index 789f52f..39117c7 100644
--- a/target-arm/kvm32.c
+++ b/target-arm/kvm32.c
@@ -464,7 +464,7 @@ int kvm_arch_get_registers(CPUState *cs)
     if (ret) {
         return ret;
     }
-    cpsr_write(env, cpsr, 0xffffffff);
+    restore_state_from_spsr(env, cpsr);
 
     /* Make sure the current mode regs are properly set */
     mode = env->uncached_cpsr & CPSR_M;
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 052a4bd..c17bdba 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -261,9 +261,47 @@ uint32_t HELPER(cpsr_read)(CPUARMState *env)
     return save_state_to_spsr(env) & ~CPSR_EXEC;
 }
 
+static int bad_mode_switch(CPUARMState *env, int mode)
+{
+    /* Return true if it is not valid for us to switch to
+     * this CPU mode (ie all the UNPREDICTABLE cases in
+     * the ARM ARM CPSRWriteByInstr pseudocode).
+     */
+    switch (mode) {
+    case ARM_CPU_MODE_USR:
+    case ARM_CPU_MODE_SYS:
+    case ARM_CPU_MODE_SVC:
+    case ARM_CPU_MODE_ABT:
+    case ARM_CPU_MODE_UND:
+    case ARM_CPU_MODE_IRQ:
+    case ARM_CPU_MODE_FIQ:
+        return 0;
+    default:
+        return 1;
+    }
+}
+
 void HELPER(cpsr_write)(CPUARMState *env, uint32_t val, uint32_t mask)
 {
-    cpsr_write(env, val, mask);
+    uint32_t current_cpsr = save_state_to_spsr(env);
+    uint32_t new_cpsr;
+
+    /* we may be triggering a mode change */
+    if ((current_cpsr ^ val) & mask & CPSR_M) {
+        if (bad_mode_switch(env, val & CPSR_M)) {
+            /* Attempt to switch to an invalid mode: this is UNPREDICTABLE.
+             * We choose to ignore the attempt and leave the CPSR M field
+             * untouched.
+             */
+            mask &= ~CPSR_M;
+        } else {
+            switch_mode(env, val & CPSR_M);
+        }
+    }
+
+    new_cpsr = current_cpsr & ~mask;
+    new_cpsr |= (val & mask);
+    restore_state_from_spsr(env, new_cpsr);
 }
 
 /* Access to user mode registers from privileged modes.  */
-- 
2.0.1