diff mbox

[v2,04/10] target-arm: replace cpsr/xpsr/pstate_read calls

Message ID 1405007407-23549-5-git-send-email-alex.bennee@linaro.org
State New
Headers show

Commit Message

Alex Bennée July 10, 2014, 3:50 p.m. UTC
Use the unified save_state_to_spsr. I've also updated the interrupt
helpers to restore via the restore_state_from_spsr() functions. In the
aarch32 case this also needs to call switch_mode() to do the appropriate
fiddling.

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

--

v2
  - include xpsr_read conversions
  - checkpatch fixes

Comments

Riku Voipio July 15, 2014, 1:40 p.m. UTC | #1
On Thu, Jul 10, 2014 at 04:50:01PM +0100, Alex Bennée wrote:
> Use the unified save_state_to_spsr. I've also updated the interrupt
> helpers to restore via the restore_state_from_spsr() functions. In the
> aarch32 case this also needs to call switch_mode() to do the appropriate
> fiddling.

For the linux-user part,

Acked-by: Riku Voipio <riku.voipio@linaro.org>

> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> 
> --
> 
> v2
>   - include xpsr_read conversions
>   - checkpatch fixes
> 
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index 60777fe..577e1d3 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -321,7 +321,7 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs, const CPUARMState *en
>      (*regs)[14] = tswapreg(env->regs[14]);
>      (*regs)[15] = tswapreg(env->regs[15]);
>  
> -    (*regs)[16] = tswapreg(cpsr_read((CPUARMState *)env));
> +    (*regs)[16] = tswapreg(save_state_to_spsr((CPUARMState *)env));
>      (*regs)[17] = tswapreg(env->regs[0]); /* XXX */
>  }
>  
> @@ -509,7 +509,7 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs,
>          (*regs)[i] = tswapreg(env->xregs[i]);
>      }
>      (*regs)[32] = tswapreg(env->pc);
> -    (*regs)[33] = tswapreg(pstate_read((CPUARMState *)env));
> +    (*regs)[33] = tswapreg(save_state_to_spsr((CPUARMState *)env));
>  }
>  
>  #define USE_ELF_CORE_DUMP
> diff --git a/linux-user/main.c b/linux-user/main.c
> index b453a39..8848e15 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -546,7 +546,7 @@ do_kernel_trap(CPUARMState *env)
>              operations. However things like ldrex/strex are much harder so
>              there's not much point trying.  */
>          start_exclusive();
> -        cpsr = cpsr_read(env);
> +        cpsr = save_state_to_spsr(env);
>          addr = env->regs[2];
>          /* FIXME: This should SEGV if the access fails.  */
>          if (get_user_u32(val, addr))
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 86fae3d..9c6727b 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -1224,7 +1224,7 @@ static int target_setup_sigframe(struct target_rt_sigframe *sf,
>      }
>      __put_user(env->xregs[31], &sf->uc.tuc_mcontext.sp);
>      __put_user(env->pc, &sf->uc.tuc_mcontext.pc);
> -    __put_user(pstate_read(env), &sf->uc.tuc_mcontext.pstate);
> +    __put_user(save_state_to_spsr(env), &sf->uc.tuc_mcontext.pstate);
>  
>      __put_user(env->exception.vaddress, &sf->uc.tuc_mcontext.fault_address);
>  
> @@ -1572,7 +1572,7 @@ setup_sigcontext(struct target_sigcontext *sc, /*struct _fpstate *fpstate,*/
>  	__put_user(env->regs[14], &sc->arm_lr);
>  	__put_user(env->regs[15], &sc->arm_pc);
>  #ifdef TARGET_CONFIG_CPU_32
> -	__put_user(cpsr_read(env), &sc->arm_cpsr);
> +        __put_user(save_state_to_spsr(env), &sc->arm_cpsr);
>  #endif
>  
>  	__put_user(/* current->thread.trap_no */ 0, &sc->trap_no);
> @@ -1604,7 +1604,7 @@ setup_return(CPUARMState *env, struct target_sigaction *ka,
>  	abi_ulong handler = ka->_sa_handler;
>  	abi_ulong retcode;
>  	int thumb = handler & 1;
> -	uint32_t cpsr = cpsr_read(env);
> +	uint32_t cpsr = save_state_to_spsr(env);
>  
>  	cpsr &= ~CPSR_IT;
>  	if (thumb) {
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index fe4d4f3..3f23167 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -480,30 +480,8 @@ int arm_cpu_handle_mmu_fault(CPUState *cpu, vaddr address, int rw,
>  #define PSTATE_MODE_EL1t 4
>  #define PSTATE_MODE_EL0t 0
>  
> -/* ARMv8 ARM D1.7 Process state, PSTATE
> - *
> - *  31  28 27  24 23   22 21 20 22    21  20 19  16 15   8 7   5 4    0
> - * +------+------+-------+-----+--------+---+------+------+-----+------+
> - * | NZCV | DAIF | SS IL |  EL | nRW SP | Q |  GE  |  IT  | JTE | Mode |
> - * +------+------+-------+-----+--------+---+------+------+-----+------+
> - *
> - * The PSTATE is an abstraction of a number of Return the current
> - * PSTATE value. This is only valid for A64 hardware although can be
> - * read when in AArch32 mode.
> - */
> -static inline uint32_t pstate_read(CPUARMState *env)
> -{
> -    int ZF;
> -
> -    g_assert(is_a64(env));
> -
> -    ZF = (env->ZF == 0);
> -    return (env->NF & 0x80000000) | (ZF << 30)
> -        | (env->CF << 29) | ((env->VF & 0x80000000) >> 3)
> -        | env->pstate | env->daif;
> -}
> -
> -/* Update the current PSTATE value. This doesn't include nRW which is */
> +/* Update the current PSTATE value. This doesn't include nRW which
> + * indicates if we are in 64 or 32 bit mode */
>  static inline void pstate_write(CPUARMState *env, uint32_t val)
>  {
>      g_assert(is_a64(env));
> @@ -520,26 +498,10 @@ static inline void pstate_write(CPUARMState *env, uint32_t val)
>   *
>   * Unlike the above PSTATE implementation these functions will attempt
>   * to switch processor mode when the M[4:0] bits are set.
> - */
> -uint32_t cpsr_read(CPUARMState *env);
> -/* Set the CPSR.  Note that some bits of mask must be all-set or all-clear.  */
> + *
> + * Note that some bits of mask must be all-set or all-clear.  */
>  void cpsr_write(CPUARMState *env, uint32_t val, uint32_t mask);
>  
> -/* ARMv7-M ARM B1.4.2, special purpose program status register xPSR */
> -static inline uint32_t xpsr_read(CPUARMState *env)
> -{
> -    int ZF;
> -
> -    g_assert(!is_a64(env));
> -
> -    ZF = (env->ZF == 0);
> -    return (env->NF & 0x80000000) | (ZF << 30)
> -        | (env->CF << 29) | ((env->VF & 0x80000000) >> 3) | (env->QF << 27)
> -        | (env->thumb << 24) | ((env->condexec_bits & 3) << 25)
> -        | ((env->condexec_bits & 0xfc) << 8)
> -        | env->v7m.exception;
> -}
> -
>  /* Set the xPSR.  Note that some bits of mask must be all-set or all-clear.  */
>  static inline void xpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)
>  {
> diff --git a/target-arm/gdbstub.c b/target-arm/gdbstub.c
> index 1c34396..ec25f30 100644
> --- a/target-arm/gdbstub.c
> +++ b/target-arm/gdbstub.c
> @@ -53,7 +53,7 @@ int arm_cpu_gdb_read_register(CPUState *cs, uint8_t *mem_buf, int n)
>          return gdb_get_reg32(mem_buf, 0);
>      case 25:
>          /* CPSR */
> -        return gdb_get_reg32(mem_buf, cpsr_read(env));
> +        return gdb_get_reg32(mem_buf, save_state_to_spsr(env));
>      }
>      /* Unknown register.  */
>      return 0;
> diff --git a/target-arm/gdbstub64.c b/target-arm/gdbstub64.c
> index 8f3b8d1..76d1b91 100644
> --- a/target-arm/gdbstub64.c
> +++ b/target-arm/gdbstub64.c
> @@ -35,7 +35,7 @@ int aarch64_cpu_gdb_read_register(CPUState *cs, uint8_t *mem_buf, int n)
>      case 32:
>          return gdb_get_reg64(mem_buf, env->pc);
>      case 33:
> -        return gdb_get_reg32(mem_buf, pstate_read(env));
> +        return gdb_get_reg32(mem_buf, save_state_to_spsr(env));
>      }
>      /* Unknown register.  */
>      return 0;
> @@ -62,7 +62,7 @@ int aarch64_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
>          env->pc = tmp;
>          return 8;
>      case 33:
> -        /* CPSR */
> +        /* SPSR */
>          pstate_write(env, tmp);
>          return 4;
>      }
> diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> index ec1fef5..03cd64f 100644
> --- a/target-arm/helper-a64.c
> +++ b/target-arm/helper-a64.c
> @@ -445,6 +445,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>      CPUARMState *env = &cpu->env;
>      target_ulong addr = env->cp15.vbar_el[1];
>      int i;
> +    uint32_t spsr = save_state_to_spsr(env);
>  
>      if (arm_current_pl(env) == 0) {
>          if (env->aarch64) {
> @@ -452,7 +453,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>          } else {
>              addr += 0x600;
>          }
> -    } else if (pstate_read(env) & PSTATE_SP) {
> +    } else if (spsr & PSTATE_SP) {
>          addr += 0x200;
>      }
>  
> @@ -488,12 +489,12 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>      }
>  
>      if (is_a64(env)) {
> -        env->banked_spsr[aarch64_banked_spsr_index(1)] = pstate_read(env);
> +        env->banked_spsr[aarch64_banked_spsr_index(1)] = spsr;
>          env->sp_el[arm_current_pl(env)] = env->xregs[31];
>          env->xregs[31] = env->sp_el[1];
>          env->elr_el[1] = env->pc;
>      } else {
> -        env->banked_spsr[0] = cpsr_read(env);
> +        env->banked_spsr[0] = spsr;
>          if (!env->thumb) {
>              env->cp15.esr_el[1] |= 1 << 25;
>          }
> @@ -506,6 +507,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>          env->condexec_bits = 0;
>      }
>  
> +    // TODO: restore_state_from_spsr()
>      env->aarch64 = 1;
>      pstate_write(env, PSTATE_DAIF | PSTATE_MODE_EL1h);
>  
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index d343856..030bcdd 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -2990,17 +2990,6 @@ static int bad_mode_switch(CPUARMState *env, int mode)
>      }
>  }
>  
> -uint32_t cpsr_read(CPUARMState *env)
> -{
> -    int ZF;
> -    ZF = (env->ZF == 0);
> -    return env->uncached_cpsr | (env->NF & 0x80000000) | (ZF << 30) |
> -        (env->CF << 29) | ((env->VF & 0x80000000) >> 3) | (env->QF << 27)
> -        | (env->thumb << 5) | ((env->condexec_bits & 3) << 25)
> -        | ((env->condexec_bits & 0xfc) << 8)
> -        | (env->GE << 16) | (env->daif & CPSR_AIF);
> -}
> -
>  void cpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)
>  {
>      if (mask & CPSR_NZCV) {
> @@ -3278,7 +3267,7 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
>  {
>      ARMCPU *cpu = ARM_CPU(cs);
>      CPUARMState *env = &cpu->env;
> -    uint32_t xpsr = xpsr_read(env);
> +    uint32_t xpsr = save_state_to_spsr(env);
>      uint32_t lr;
>      uint32_t addr;
>  
> @@ -3479,7 +3468,8 @@ void arm_cpu_do_interrupt(CPUState *cs)
>          addr += env->cp15.vbar_el[1];
>      }
>      switch_mode (env, new_mode);
> -    env->spsr = cpsr_read(env);
> +    env->spsr = save_state_to_spsr(env);
> +    /* TODO: restore_state_from_spsr */
>      /* Clear IT bits.  */
>      env->condexec_bits = 0;
>      /* Switch to the new mode, and to the correct instruction set.  */
> @@ -4212,19 +4202,19 @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg)
>  
>      switch (reg) {
>      case 0: /* APSR */
> -        return xpsr_read(env) & 0xf8000000;
> +        return save_state_to_spsr(env) & 0xf8000000;
>      case 1: /* IAPSR */
> -        return xpsr_read(env) & 0xf80001ff;
> +        return save_state_to_spsr(env) & 0xf80001ff;
>      case 2: /* EAPSR */
> -        return xpsr_read(env) & 0xff00fc00;
> +        return save_state_to_spsr(env) & 0xff00fc00;
>      case 3: /* xPSR */
> -        return xpsr_read(env) & 0xff00fdff;
> +        return save_state_to_spsr(env) & 0xff00fdff;
>      case 5: /* IPSR */
> -        return xpsr_read(env) & 0x000001ff;
> +        return save_state_to_spsr(env) & 0x000001ff;
>      case 6: /* EPSR */
> -        return xpsr_read(env) & 0x0700fc00;
> +        return save_state_to_spsr(env) & 0x0700fc00;
>      case 7: /* IEPSR */
> -        return xpsr_read(env) & 0x0700edff;
> +        return save_state_to_spsr(env) & 0x0700edff;
>      case 8: /* MSP */
>          return env->v7m.current_sp ? env->v7m.other_sp : env->regs[13];
>      case 9: /* PSP */
> diff --git a/target-arm/kvm32.c b/target-arm/kvm32.c
> index 5ec4eb1..789f52f 100644
> --- a/target-arm/kvm32.c
> +++ b/target-arm/kvm32.c
> @@ -384,7 +384,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
>      }
>  
>      /* Special cases which aren't a single CPUARMState field */
> -    cpsr = cpsr_read(env);
> +    cpsr = save_state_to_spsr(env);
>      r.id = KVM_REG_ARM | KVM_REG_SIZE_U32 |
>          KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(usr_regs.ARM_cpsr);
>      r.addr = (uintptr_t)(&cpsr);
> diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
> index 5d217ca..eaf6ff8 100644
> --- a/target-arm/kvm64.c
> +++ b/target-arm/kvm64.c
> @@ -153,7 +153,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
>      }
>  
>      /* Note that KVM thinks pstate is 64 bit but we use a uint32_t */
> -    val = pstate_read(env);
> +    val = save_state_to_spsr(env);
>      reg.id = AARCH64_CORE_REG(regs.pstate);
>      reg.addr = (uintptr_t) &val;
>      ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 9c1ef52..052a4bd 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -258,7 +258,7 @@ void HELPER(exception_with_syndrome)(CPUARMState *env, uint32_t excp,
>  
>  uint32_t HELPER(cpsr_read)(CPUARMState *env)
>  {
> -    return cpsr_read(env) & ~CPSR_EXEC;
> +    return save_state_to_spsr(env) & ~CPSR_EXEC;
>  }
>  
>  void HELPER(cpsr_write)(CPUARMState *env, uint32_t val, uint32_t mask)
> @@ -386,10 +386,9 @@ void HELPER(exception_return)(CPUARMState *env)
>  
>      if (spsr & PSTATE_nRW) {
>          /* TODO: We currently assume EL1/2/3 are running in AArch64.  */
> -        env->aarch64 = 0;
>          new_el = 0;
> -        env->uncached_cpsr = 0x10;
> -        cpsr_write(env, spsr, ~0);
> +        switch_mode(env, spsr & CPSR_M);
> +
>          for (i = 0; i < 15; i++) {
>              env->regs[i] = env->xregs[i];
>          }
> @@ -412,11 +411,11 @@ void HELPER(exception_return)(CPUARMState *env)
>              /* Return to EL0 with M[0] bit set */
>              goto illegal_return;
>          }
> -        env->aarch64 = 1;
> -        pstate_write(env, spsr);
>          env->xregs[31] = env->sp_el[new_el];
>          env->pc = env->elr_el[cur_el];
>      }
> +    /* This will set env->aarch64 as appropriate */
> +    restore_state_from_spsr(env, spsr);
>  
>      return;
>  
> @@ -431,8 +430,8 @@ illegal_return:
>      env->pstate |= PSTATE_IL;
>      env->pc = env->elr_el[cur_el];
>      spsr &= PSTATE_NZCV | PSTATE_DAIF;
> -    spsr |= pstate_read(env) & ~(PSTATE_NZCV | PSTATE_DAIF);
> -    pstate_write(env, spsr);
> +    spsr |= save_state_to_spsr(env) & ~(PSTATE_NZCV | PSTATE_DAIF);
> +    restore_state_from_spsr(env, spsr);
>  }
>  
>  /* ??? Flag setting arithmetic is awkward because we need to do comparisons.
> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
> index 33b5025..3b4a676 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -126,7 +126,7 @@ void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
>  {
>      ARMCPU *cpu = ARM_CPU(cs);
>      CPUARMState *env = &cpu->env;
> -    uint32_t psr = pstate_read(env);
> +    uint32_t psr = save_state_to_spsr(env);
>      int i;
>  
>      cpu_fprintf(f, "PC=%016"PRIx64"  SP=%016"PRIx64"\n",
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index cf4e767..8427396 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -11192,7 +11192,7 @@ void arm_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
>          else
>              cpu_fprintf(f, " ");
>      }
> -    psr = cpsr_read(env);
> +    psr = save_state_to_spsr(env);
>      cpu_fprintf(f, "PSR=%08x %c%c%c%c %c %s%d\n",
>                  psr,
>                  psr & (1 << 31) ? 'N' : '-',
> -- 
> 2.0.1
>
Peter Maydell Aug. 4, 2014, 12:59 p.m. UTC | #2
On 10 July 2014 16:50, Alex Bennée <alex.bennee@linaro.org> wrote:
> Use the unified save_state_to_spsr. I've also updated the interrupt
> helpers to restore via the restore_state_from_spsr() functions. In the
> aarch32 case this also needs to call switch_mode() to do the appropriate
> fiddling.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> --
>
> v2
>   - include xpsr_read conversions
>   - checkpatch fixes
>
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index 60777fe..577e1d3 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -321,7 +321,7 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs, const CPUARMState *en
>      (*regs)[14] = tswapreg(env->regs[14]);
>      (*regs)[15] = tswapreg(env->regs[15]);
>
> -    (*regs)[16] = tswapreg(cpsr_read((CPUARMState *)env));
> +    (*regs)[16] = tswapreg(save_state_to_spsr((CPUARMState *)env));

Using a function named "save_something" to *read* CPU
state is really confusing.

Also, the CPSR format is not the same as the AArch32 SPSR
format -- for instance bit 20 is IL in the SPSR, but we must not
allow a guest in AArch32 to read or write PSTATE.IL via a
CPSR read or write insn. We should check whether the value
the kernel exposes in places like the signal handler context
structs is the SPSR format or not.

>      (*regs)[17] = tswapreg(env->regs[0]); /* XXX */
>  }
>
> @@ -509,7 +509,7 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs,
>          (*regs)[i] = tswapreg(env->xregs[i]);
>      }
>      (*regs)[32] = tswapreg(env->pc);
> -    (*regs)[33] = tswapreg(pstate_read((CPUARMState *)env));
> +    (*regs)[33] = tswapreg(save_state_to_spsr((CPUARMState *)env));
>  }
>
>  #define USE_ELF_CORE_DUMP
> diff --git a/linux-user/main.c b/linux-user/main.c
> index b453a39..8848e15 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -546,7 +546,7 @@ do_kernel_trap(CPUARMState *env)
>              operations. However things like ldrex/strex are much harder so
>              there's not much point trying.  */
>          start_exclusive();
> -        cpsr = cpsr_read(env);
> +        cpsr = save_state_to_spsr(env);
>          addr = env->regs[2];
>          /* FIXME: This should SEGV if the access fails.  */
>          if (get_user_u32(val, addr))
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 86fae3d..9c6727b 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -1224,7 +1224,7 @@ static int target_setup_sigframe(struct target_rt_sigframe *sf,
>      }
>      __put_user(env->xregs[31], &sf->uc.tuc_mcontext.sp);
>      __put_user(env->pc, &sf->uc.tuc_mcontext.pc);
> -    __put_user(pstate_read(env), &sf->uc.tuc_mcontext.pstate);
> +    __put_user(save_state_to_spsr(env), &sf->uc.tuc_mcontext.pstate);
>
>      __put_user(env->exception.vaddress, &sf->uc.tuc_mcontext.fault_address);
>
> @@ -1572,7 +1572,7 @@ setup_sigcontext(struct target_sigcontext *sc, /*struct _fpstate *fpstate,*/
>         __put_user(env->regs[14], &sc->arm_lr);
>         __put_user(env->regs[15], &sc->arm_pc);
>  #ifdef TARGET_CONFIG_CPU_32
> -       __put_user(cpsr_read(env), &sc->arm_cpsr);
> +        __put_user(save_state_to_spsr(env), &sc->arm_cpsr);
>  #endif
>
>         __put_user(/* current->thread.trap_no */ 0, &sc->trap_no);
> @@ -1604,7 +1604,7 @@ setup_return(CPUARMState *env, struct target_sigaction *ka,
>         abi_ulong handler = ka->_sa_handler;
>         abi_ulong retcode;
>         int thumb = handler & 1;
> -       uint32_t cpsr = cpsr_read(env);
> +       uint32_t cpsr = save_state_to_spsr(env);
>
>         cpsr &= ~CPSR_IT;
>         if (thumb) {
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index fe4d4f3..3f23167 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -480,30 +480,8 @@ int arm_cpu_handle_mmu_fault(CPUState *cpu, vaddr address, int rw,
>  #define PSTATE_MODE_EL1t 4
>  #define PSTATE_MODE_EL0t 0
>
> -/* ARMv8 ARM D1.7 Process state, PSTATE
> - *
> - *  31  28 27  24 23   22 21 20 22    21  20 19  16 15   8 7   5 4    0
> - * +------+------+-------+-----+--------+---+------+------+-----+------+
> - * | NZCV | DAIF | SS IL |  EL | nRW SP | Q |  GE  |  IT  | JTE | Mode |
> - * +------+------+-------+-----+--------+---+------+------+-----+------+
> - *
> - * The PSTATE is an abstraction of a number of Return the current
> - * PSTATE value. This is only valid for A64 hardware although can be
> - * read when in AArch32 mode.
> - */
> -static inline uint32_t pstate_read(CPUARMState *env)
> -{
> -    int ZF;
> -
> -    g_assert(is_a64(env));
> -
> -    ZF = (env->ZF == 0);
> -    return (env->NF & 0x80000000) | (ZF << 30)
> -        | (env->CF << 29) | ((env->VF & 0x80000000) >> 3)
> -        | env->pstate | env->daif;
> -}

??? You just added this function and comment in an earlier
patch. Rebasing damage?

> -
> -/* Update the current PSTATE value. This doesn't include nRW which is */
> +/* Update the current PSTATE value. This doesn't include nRW which
> + * indicates if we are in 64 or 32 bit mode */
>  static inline void pstate_write(CPUARMState *env, uint32_t val)
>  {
>      g_assert(is_a64(env));
> @@ -520,26 +498,10 @@ static inline void pstate_write(CPUARMState *env, uint32_t val)
>   *
>   * Unlike the above PSTATE implementation these functions will attempt
>   * to switch processor mode when the M[4:0] bits are set.
> - */
> -uint32_t cpsr_read(CPUARMState *env);
> -/* Set the CPSR.  Note that some bits of mask must be all-set or all-clear.  */
> + *
> + * Note that some bits of mask must be all-set or all-clear.  */
>  void cpsr_write(CPUARMState *env, uint32_t val, uint32_t mask);
>
> -/* ARMv7-M ARM B1.4.2, special purpose program status register xPSR */
> -static inline uint32_t xpsr_read(CPUARMState *env)
> -{
> -    int ZF;
> -
> -    g_assert(!is_a64(env));
> -
> -    ZF = (env->ZF == 0);
> -    return (env->NF & 0x80000000) | (ZF << 30)
> -        | (env->CF << 29) | ((env->VF & 0x80000000) >> 3) | (env->QF << 27)
> -        | (env->thumb << 24) | ((env->condexec_bits & 3) << 25)
> -        | ((env->condexec_bits & 0xfc) << 8)
> -        | env->v7m.exception;
> -}
> -
>  /* Set the xPSR.  Note that some bits of mask must be all-set or all-clear.  */
>  static inline void xpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)
>  {
> diff --git a/target-arm/gdbstub.c b/target-arm/gdbstub.c
> index 1c34396..ec25f30 100644
> --- a/target-arm/gdbstub.c
> +++ b/target-arm/gdbstub.c
> @@ -53,7 +53,7 @@ int arm_cpu_gdb_read_register(CPUState *cs, uint8_t *mem_buf, int n)
>          return gdb_get_reg32(mem_buf, 0);
>      case 25:
>          /* CPSR */
> -        return gdb_get_reg32(mem_buf, cpsr_read(env));
> +        return gdb_get_reg32(mem_buf, save_state_to_spsr(env));
>      }
>      /* Unknown register.  */
>      return 0;
> diff --git a/target-arm/gdbstub64.c b/target-arm/gdbstub64.c
> index 8f3b8d1..76d1b91 100644
> --- a/target-arm/gdbstub64.c
> +++ b/target-arm/gdbstub64.c
> @@ -35,7 +35,7 @@ int aarch64_cpu_gdb_read_register(CPUState *cs, uint8_t *mem_buf, int n)
>      case 32:
>          return gdb_get_reg64(mem_buf, env->pc);
>      case 33:
> -        return gdb_get_reg32(mem_buf, pstate_read(env));
> +        return gdb_get_reg32(mem_buf, save_state_to_spsr(env));
>      }
>      /* Unknown register.  */
>      return 0;
> @@ -62,7 +62,7 @@ int aarch64_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
>          env->pc = tmp;
>          return 8;
>      case 33:
> -        /* CPSR */
> +        /* SPSR */
>          pstate_write(env, tmp);
>          return 4;
>      }
> diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> index ec1fef5..03cd64f 100644
> --- a/target-arm/helper-a64.c
> +++ b/target-arm/helper-a64.c
> @@ -445,6 +445,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>      CPUARMState *env = &cpu->env;
>      target_ulong addr = env->cp15.vbar_el[1];
>      int i;
> +    uint32_t spsr = save_state_to_spsr(env);
>
>      if (arm_current_pl(env) == 0) {
>          if (env->aarch64) {
> @@ -452,7 +453,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>          } else {
>              addr += 0x600;
>          }
> -    } else if (pstate_read(env) & PSTATE_SP) {
> +    } else if (spsr & PSTATE_SP) {
>          addr += 0x200;
>      }
>
> @@ -488,12 +489,12 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>      }
>
>      if (is_a64(env)) {
> -        env->banked_spsr[aarch64_banked_spsr_index(1)] = pstate_read(env);
> +        env->banked_spsr[aarch64_banked_spsr_index(1)] = spsr;
>          env->sp_el[arm_current_pl(env)] = env->xregs[31];
>          env->xregs[31] = env->sp_el[1];
>          env->elr_el[1] = env->pc;
>      } else {
> -        env->banked_spsr[0] = cpsr_read(env);
> +        env->banked_spsr[0] = spsr;
>          if (!env->thumb) {
>              env->cp15.esr_el[1] |= 1 << 25;
>          }
> @@ -506,6 +507,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>          env->condexec_bits = 0;
>      }
>
> +    // TODO: restore_state_from_spsr()

Don't leave TODO comments in patches...

>      env->aarch64 = 1;
>      pstate_write(env, PSTATE_DAIF | PSTATE_MODE_EL1h);
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index d343856..030bcdd 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -2990,17 +2990,6 @@ static int bad_mode_switch(CPUARMState *env, int mode)
>      }
>  }
>
> -uint32_t cpsr_read(CPUARMState *env)
> -{
> -    int ZF;
> -    ZF = (env->ZF == 0);
> -    return env->uncached_cpsr | (env->NF & 0x80000000) | (ZF << 30) |
> -        (env->CF << 29) | ((env->VF & 0x80000000) >> 3) | (env->QF << 27)
> -        | (env->thumb << 5) | ((env->condexec_bits & 3) << 25)
> -        | ((env->condexec_bits & 0xfc) << 8)
> -        | (env->GE << 16) | (env->daif & CPSR_AIF);
> -}
> -
>  void cpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)
>  {
>      if (mask & CPSR_NZCV) {
> @@ -3278,7 +3267,7 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
>  {
>      ARMCPU *cpu = ARM_CPU(cs);
>      CPUARMState *env = &cpu->env;
> -    uint32_t xpsr = xpsr_read(env);
> +    uint32_t xpsr = save_state_to_spsr(env);
>      uint32_t lr;
>      uint32_t addr;
>
> @@ -3479,7 +3468,8 @@ void arm_cpu_do_interrupt(CPUState *cs)
>          addr += env->cp15.vbar_el[1];
>      }
>      switch_mode (env, new_mode);
> -    env->spsr = cpsr_read(env);
> +    env->spsr = save_state_to_spsr(env);
> +    /* TODO: restore_state_from_spsr */

Another TODO comment here...

>      /* Clear IT bits.  */
>      env->condexec_bits = 0;
>      /* Switch to the new mode, and to the correct instruction set.  */
> @@ -4212,19 +4202,19 @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg)
>
>      switch (reg) {
>      case 0: /* APSR */
> -        return xpsr_read(env) & 0xf8000000;
> +        return save_state_to_spsr(env) & 0xf8000000;
>      case 1: /* IAPSR */
> -        return xpsr_read(env) & 0xf80001ff;
> +        return save_state_to_spsr(env) & 0xf80001ff;
>      case 2: /* EAPSR */
> -        return xpsr_read(env) & 0xff00fc00;
> +        return save_state_to_spsr(env) & 0xff00fc00;
>      case 3: /* xPSR */
> -        return xpsr_read(env) & 0xff00fdff;
> +        return save_state_to_spsr(env) & 0xff00fdff;
>      case 5: /* IPSR */
> -        return xpsr_read(env) & 0x000001ff;
> +        return save_state_to_spsr(env) & 0x000001ff;
>      case 6: /* EPSR */
> -        return xpsr_read(env) & 0x0700fc00;
> +        return save_state_to_spsr(env) & 0x0700fc00;
>      case 7: /* IEPSR */
> -        return xpsr_read(env) & 0x0700edff;
> +        return save_state_to_spsr(env) & 0x0700edff;
>      case 8: /* MSP */
>          return env->v7m.current_sp ? env->v7m.other_sp : env->regs[13];
>      case 9: /* PSP */

This all seems more confusing rather than less. What's wrong
with still having functions like xpsr_read() ?

thanks
-- PMM
diff mbox

Patch

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 60777fe..577e1d3 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -321,7 +321,7 @@  static void elf_core_copy_regs(target_elf_gregset_t *regs, const CPUARMState *en
     (*regs)[14] = tswapreg(env->regs[14]);
     (*regs)[15] = tswapreg(env->regs[15]);
 
-    (*regs)[16] = tswapreg(cpsr_read((CPUARMState *)env));
+    (*regs)[16] = tswapreg(save_state_to_spsr((CPUARMState *)env));
     (*regs)[17] = tswapreg(env->regs[0]); /* XXX */
 }
 
@@ -509,7 +509,7 @@  static void elf_core_copy_regs(target_elf_gregset_t *regs,
         (*regs)[i] = tswapreg(env->xregs[i]);
     }
     (*regs)[32] = tswapreg(env->pc);
-    (*regs)[33] = tswapreg(pstate_read((CPUARMState *)env));
+    (*regs)[33] = tswapreg(save_state_to_spsr((CPUARMState *)env));
 }
 
 #define USE_ELF_CORE_DUMP
diff --git a/linux-user/main.c b/linux-user/main.c
index b453a39..8848e15 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -546,7 +546,7 @@  do_kernel_trap(CPUARMState *env)
             operations. However things like ldrex/strex are much harder so
             there's not much point trying.  */
         start_exclusive();
-        cpsr = cpsr_read(env);
+        cpsr = save_state_to_spsr(env);
         addr = env->regs[2];
         /* FIXME: This should SEGV if the access fails.  */
         if (get_user_u32(val, addr))
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 86fae3d..9c6727b 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -1224,7 +1224,7 @@  static int target_setup_sigframe(struct target_rt_sigframe *sf,
     }
     __put_user(env->xregs[31], &sf->uc.tuc_mcontext.sp);
     __put_user(env->pc, &sf->uc.tuc_mcontext.pc);
-    __put_user(pstate_read(env), &sf->uc.tuc_mcontext.pstate);
+    __put_user(save_state_to_spsr(env), &sf->uc.tuc_mcontext.pstate);
 
     __put_user(env->exception.vaddress, &sf->uc.tuc_mcontext.fault_address);
 
@@ -1572,7 +1572,7 @@  setup_sigcontext(struct target_sigcontext *sc, /*struct _fpstate *fpstate,*/
 	__put_user(env->regs[14], &sc->arm_lr);
 	__put_user(env->regs[15], &sc->arm_pc);
 #ifdef TARGET_CONFIG_CPU_32
-	__put_user(cpsr_read(env), &sc->arm_cpsr);
+        __put_user(save_state_to_spsr(env), &sc->arm_cpsr);
 #endif
 
 	__put_user(/* current->thread.trap_no */ 0, &sc->trap_no);
@@ -1604,7 +1604,7 @@  setup_return(CPUARMState *env, struct target_sigaction *ka,
 	abi_ulong handler = ka->_sa_handler;
 	abi_ulong retcode;
 	int thumb = handler & 1;
-	uint32_t cpsr = cpsr_read(env);
+	uint32_t cpsr = save_state_to_spsr(env);
 
 	cpsr &= ~CPSR_IT;
 	if (thumb) {
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index fe4d4f3..3f23167 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -480,30 +480,8 @@  int arm_cpu_handle_mmu_fault(CPUState *cpu, vaddr address, int rw,
 #define PSTATE_MODE_EL1t 4
 #define PSTATE_MODE_EL0t 0
 
-/* ARMv8 ARM D1.7 Process state, PSTATE
- *
- *  31  28 27  24 23   22 21 20 22    21  20 19  16 15   8 7   5 4    0
- * +------+------+-------+-----+--------+---+------+------+-----+------+
- * | NZCV | DAIF | SS IL |  EL | nRW SP | Q |  GE  |  IT  | JTE | Mode |
- * +------+------+-------+-----+--------+---+------+------+-----+------+
- *
- * The PSTATE is an abstraction of a number of Return the current
- * PSTATE value. This is only valid for A64 hardware although can be
- * read when in AArch32 mode.
- */
-static inline uint32_t pstate_read(CPUARMState *env)
-{
-    int ZF;
-
-    g_assert(is_a64(env));
-
-    ZF = (env->ZF == 0);
-    return (env->NF & 0x80000000) | (ZF << 30)
-        | (env->CF << 29) | ((env->VF & 0x80000000) >> 3)
-        | env->pstate | env->daif;
-}
-
-/* Update the current PSTATE value. This doesn't include nRW which is */
+/* Update the current PSTATE value. This doesn't include nRW which
+ * indicates if we are in 64 or 32 bit mode */
 static inline void pstate_write(CPUARMState *env, uint32_t val)
 {
     g_assert(is_a64(env));
@@ -520,26 +498,10 @@  static inline void pstate_write(CPUARMState *env, uint32_t val)
  *
  * Unlike the above PSTATE implementation these functions will attempt
  * to switch processor mode when the M[4:0] bits are set.
- */
-uint32_t cpsr_read(CPUARMState *env);
-/* Set the CPSR.  Note that some bits of mask must be all-set or all-clear.  */
+ *
+ * Note that some bits of mask must be all-set or all-clear.  */
 void cpsr_write(CPUARMState *env, uint32_t val, uint32_t mask);
 
-/* ARMv7-M ARM B1.4.2, special purpose program status register xPSR */
-static inline uint32_t xpsr_read(CPUARMState *env)
-{
-    int ZF;
-
-    g_assert(!is_a64(env));
-
-    ZF = (env->ZF == 0);
-    return (env->NF & 0x80000000) | (ZF << 30)
-        | (env->CF << 29) | ((env->VF & 0x80000000) >> 3) | (env->QF << 27)
-        | (env->thumb << 24) | ((env->condexec_bits & 3) << 25)
-        | ((env->condexec_bits & 0xfc) << 8)
-        | env->v7m.exception;
-}
-
 /* Set the xPSR.  Note that some bits of mask must be all-set or all-clear.  */
 static inline void xpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)
 {
diff --git a/target-arm/gdbstub.c b/target-arm/gdbstub.c
index 1c34396..ec25f30 100644
--- a/target-arm/gdbstub.c
+++ b/target-arm/gdbstub.c
@@ -53,7 +53,7 @@  int arm_cpu_gdb_read_register(CPUState *cs, uint8_t *mem_buf, int n)
         return gdb_get_reg32(mem_buf, 0);
     case 25:
         /* CPSR */
-        return gdb_get_reg32(mem_buf, cpsr_read(env));
+        return gdb_get_reg32(mem_buf, save_state_to_spsr(env));
     }
     /* Unknown register.  */
     return 0;
diff --git a/target-arm/gdbstub64.c b/target-arm/gdbstub64.c
index 8f3b8d1..76d1b91 100644
--- a/target-arm/gdbstub64.c
+++ b/target-arm/gdbstub64.c
@@ -35,7 +35,7 @@  int aarch64_cpu_gdb_read_register(CPUState *cs, uint8_t *mem_buf, int n)
     case 32:
         return gdb_get_reg64(mem_buf, env->pc);
     case 33:
-        return gdb_get_reg32(mem_buf, pstate_read(env));
+        return gdb_get_reg32(mem_buf, save_state_to_spsr(env));
     }
     /* Unknown register.  */
     return 0;
@@ -62,7 +62,7 @@  int aarch64_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
         env->pc = tmp;
         return 8;
     case 33:
-        /* CPSR */
+        /* SPSR */
         pstate_write(env, tmp);
         return 4;
     }
diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index ec1fef5..03cd64f 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -445,6 +445,7 @@  void aarch64_cpu_do_interrupt(CPUState *cs)
     CPUARMState *env = &cpu->env;
     target_ulong addr = env->cp15.vbar_el[1];
     int i;
+    uint32_t spsr = save_state_to_spsr(env);
 
     if (arm_current_pl(env) == 0) {
         if (env->aarch64) {
@@ -452,7 +453,7 @@  void aarch64_cpu_do_interrupt(CPUState *cs)
         } else {
             addr += 0x600;
         }
-    } else if (pstate_read(env) & PSTATE_SP) {
+    } else if (spsr & PSTATE_SP) {
         addr += 0x200;
     }
 
@@ -488,12 +489,12 @@  void aarch64_cpu_do_interrupt(CPUState *cs)
     }
 
     if (is_a64(env)) {
-        env->banked_spsr[aarch64_banked_spsr_index(1)] = pstate_read(env);
+        env->banked_spsr[aarch64_banked_spsr_index(1)] = spsr;
         env->sp_el[arm_current_pl(env)] = env->xregs[31];
         env->xregs[31] = env->sp_el[1];
         env->elr_el[1] = env->pc;
     } else {
-        env->banked_spsr[0] = cpsr_read(env);
+        env->banked_spsr[0] = spsr;
         if (!env->thumb) {
             env->cp15.esr_el[1] |= 1 << 25;
         }
@@ -506,6 +507,7 @@  void aarch64_cpu_do_interrupt(CPUState *cs)
         env->condexec_bits = 0;
     }
 
+    // TODO: restore_state_from_spsr()
     env->aarch64 = 1;
     pstate_write(env, PSTATE_DAIF | PSTATE_MODE_EL1h);
 
diff --git a/target-arm/helper.c b/target-arm/helper.c
index d343856..030bcdd 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2990,17 +2990,6 @@  static int bad_mode_switch(CPUARMState *env, int mode)
     }
 }
 
-uint32_t cpsr_read(CPUARMState *env)
-{
-    int ZF;
-    ZF = (env->ZF == 0);
-    return env->uncached_cpsr | (env->NF & 0x80000000) | (ZF << 30) |
-        (env->CF << 29) | ((env->VF & 0x80000000) >> 3) | (env->QF << 27)
-        | (env->thumb << 5) | ((env->condexec_bits & 3) << 25)
-        | ((env->condexec_bits & 0xfc) << 8)
-        | (env->GE << 16) | (env->daif & CPSR_AIF);
-}
-
 void cpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)
 {
     if (mask & CPSR_NZCV) {
@@ -3278,7 +3267,7 @@  void arm_v7m_cpu_do_interrupt(CPUState *cs)
 {
     ARMCPU *cpu = ARM_CPU(cs);
     CPUARMState *env = &cpu->env;
-    uint32_t xpsr = xpsr_read(env);
+    uint32_t xpsr = save_state_to_spsr(env);
     uint32_t lr;
     uint32_t addr;
 
@@ -3479,7 +3468,8 @@  void arm_cpu_do_interrupt(CPUState *cs)
         addr += env->cp15.vbar_el[1];
     }
     switch_mode (env, new_mode);
-    env->spsr = cpsr_read(env);
+    env->spsr = save_state_to_spsr(env);
+    /* TODO: restore_state_from_spsr */
     /* Clear IT bits.  */
     env->condexec_bits = 0;
     /* Switch to the new mode, and to the correct instruction set.  */
@@ -4212,19 +4202,19 @@  uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg)
 
     switch (reg) {
     case 0: /* APSR */
-        return xpsr_read(env) & 0xf8000000;
+        return save_state_to_spsr(env) & 0xf8000000;
     case 1: /* IAPSR */
-        return xpsr_read(env) & 0xf80001ff;
+        return save_state_to_spsr(env) & 0xf80001ff;
     case 2: /* EAPSR */
-        return xpsr_read(env) & 0xff00fc00;
+        return save_state_to_spsr(env) & 0xff00fc00;
     case 3: /* xPSR */
-        return xpsr_read(env) & 0xff00fdff;
+        return save_state_to_spsr(env) & 0xff00fdff;
     case 5: /* IPSR */
-        return xpsr_read(env) & 0x000001ff;
+        return save_state_to_spsr(env) & 0x000001ff;
     case 6: /* EPSR */
-        return xpsr_read(env) & 0x0700fc00;
+        return save_state_to_spsr(env) & 0x0700fc00;
     case 7: /* IEPSR */
-        return xpsr_read(env) & 0x0700edff;
+        return save_state_to_spsr(env) & 0x0700edff;
     case 8: /* MSP */
         return env->v7m.current_sp ? env->v7m.other_sp : env->regs[13];
     case 9: /* PSP */
diff --git a/target-arm/kvm32.c b/target-arm/kvm32.c
index 5ec4eb1..789f52f 100644
--- a/target-arm/kvm32.c
+++ b/target-arm/kvm32.c
@@ -384,7 +384,7 @@  int kvm_arch_put_registers(CPUState *cs, int level)
     }
 
     /* Special cases which aren't a single CPUARMState field */
-    cpsr = cpsr_read(env);
+    cpsr = save_state_to_spsr(env);
     r.id = KVM_REG_ARM | KVM_REG_SIZE_U32 |
         KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(usr_regs.ARM_cpsr);
     r.addr = (uintptr_t)(&cpsr);
diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
index 5d217ca..eaf6ff8 100644
--- a/target-arm/kvm64.c
+++ b/target-arm/kvm64.c
@@ -153,7 +153,7 @@  int kvm_arch_put_registers(CPUState *cs, int level)
     }
 
     /* Note that KVM thinks pstate is 64 bit but we use a uint32_t */
-    val = pstate_read(env);
+    val = save_state_to_spsr(env);
     reg.id = AARCH64_CORE_REG(regs.pstate);
     reg.addr = (uintptr_t) &val;
     ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 9c1ef52..052a4bd 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -258,7 +258,7 @@  void HELPER(exception_with_syndrome)(CPUARMState *env, uint32_t excp,
 
 uint32_t HELPER(cpsr_read)(CPUARMState *env)
 {
-    return cpsr_read(env) & ~CPSR_EXEC;
+    return save_state_to_spsr(env) & ~CPSR_EXEC;
 }
 
 void HELPER(cpsr_write)(CPUARMState *env, uint32_t val, uint32_t mask)
@@ -386,10 +386,9 @@  void HELPER(exception_return)(CPUARMState *env)
 
     if (spsr & PSTATE_nRW) {
         /* TODO: We currently assume EL1/2/3 are running in AArch64.  */
-        env->aarch64 = 0;
         new_el = 0;
-        env->uncached_cpsr = 0x10;
-        cpsr_write(env, spsr, ~0);
+        switch_mode(env, spsr & CPSR_M);
+
         for (i = 0; i < 15; i++) {
             env->regs[i] = env->xregs[i];
         }
@@ -412,11 +411,11 @@  void HELPER(exception_return)(CPUARMState *env)
             /* Return to EL0 with M[0] bit set */
             goto illegal_return;
         }
-        env->aarch64 = 1;
-        pstate_write(env, spsr);
         env->xregs[31] = env->sp_el[new_el];
         env->pc = env->elr_el[cur_el];
     }
+    /* This will set env->aarch64 as appropriate */
+    restore_state_from_spsr(env, spsr);
 
     return;
 
@@ -431,8 +430,8 @@  illegal_return:
     env->pstate |= PSTATE_IL;
     env->pc = env->elr_el[cur_el];
     spsr &= PSTATE_NZCV | PSTATE_DAIF;
-    spsr |= pstate_read(env) & ~(PSTATE_NZCV | PSTATE_DAIF);
-    pstate_write(env, spsr);
+    spsr |= save_state_to_spsr(env) & ~(PSTATE_NZCV | PSTATE_DAIF);
+    restore_state_from_spsr(env, spsr);
 }
 
 /* ??? Flag setting arithmetic is awkward because we need to do comparisons.
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 33b5025..3b4a676 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -126,7 +126,7 @@  void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
 {
     ARMCPU *cpu = ARM_CPU(cs);
     CPUARMState *env = &cpu->env;
-    uint32_t psr = pstate_read(env);
+    uint32_t psr = save_state_to_spsr(env);
     int i;
 
     cpu_fprintf(f, "PC=%016"PRIx64"  SP=%016"PRIx64"\n",
diff --git a/target-arm/translate.c b/target-arm/translate.c
index cf4e767..8427396 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -11192,7 +11192,7 @@  void arm_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
         else
             cpu_fprintf(f, " ");
     }
-    psr = cpsr_read(env);
+    psr = save_state_to_spsr(env);
     cpu_fprintf(f, "PSR=%08x %c%c%c%c %c %s%d\n",
                 psr,
                 psr & (1 << 31) ? 'N' : '-',