Re: [Qemu-devel] [PATCH v3 20/31] target-arm: Store AIF bits in env->pstate for AArch32

[Peter Crosthwaite]

On Sun, Feb 16, 2014 at 2:07 AM, Peter Maydell <address@hidden> wrote:
> To avoid complication in code that otherwise would not need to
> care about whether EL1 is AArch32 or AArch64, we should store
> the interrupt mask bits (CPSR.AIF in AArch32 and PSTATE.DAIF
> in AArch64) in one place consistently regardless of EL1's mode.
> Since AArch64 has an extra enable bit (D for debug exceptions)
> which isn't visible in AArch32, this means we need to keep
> the enables in env->pstate. (This is also consistent with the
> general approach we're taking that we handle 32 bit CPUs as
> being like AArch64/ARMv8 CPUs but which only run in 32 bit mode.)
>
> Signed-off-by: Peter Maydell <address@hidden>

Reviewed-by: Peter Crosthwaite <address@hidden>

> ---
>  cpu-exec.c          |  4 ++--
>  hw/arm/pxa2xx.c     |  4 ++--
>  target-arm/cpu.c    |  8 ++++----
>  target-arm/cpu.h    | 12 +++++++++---
>  target-arm/helper.c | 29 +++++++++++++++++------------
>  5 files changed, 34 insertions(+), 23 deletions(-)
>
> diff --git a/cpu-exec.c b/cpu-exec.c
> index a6c01f4..ba28e31 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -474,7 +474,7 @@ int cpu_exec(CPUArchState *env)
>                      }
>  #elif defined(TARGET_ARM)
>                      if (interrupt_request & CPU_INTERRUPT_FIQ
> -                        && !(env->uncached_cpsr & CPSR_F)) {
> +                        && !(env->daif & PSTATE_F)) {
>                          env->exception_index = EXCP_FIQ;
>                          cc->do_interrupt(cpu);
>                          next_tb = 0;
> @@ -490,7 +490,7 @@ int cpu_exec(CPUArchState *env)
>                         pc contains a magic address.  */
>                      if (interrupt_request & CPU_INTERRUPT_HARD
>                          && ((IS_M(env) && env->regs[15] < 0xfffffff0)
> -                            || !(env->uncached_cpsr & CPSR_I))) {
> +                            || !(env->daif & PSTATE_I))) {
>                          env->exception_index = EXCP_IRQ;
>                          cc->do_interrupt(cpu);
>                          next_tb = 0;
> diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c
> index be6b5f6..f102470 100644
> --- a/hw/arm/pxa2xx.c
> +++ b/hw/arm/pxa2xx.c
> @@ -272,8 +272,8 @@ static void pxa2xx_pwrmode_write(CPUARMState *env, const 
> ARMCPRegInfo *ri,
>          goto message;
>
>      case 3:
> -        s->cpu->env.uncached_cpsr =
> -            ARM_CPU_MODE_SVC | CPSR_A | CPSR_F | CPSR_I;
> +        s->cpu->env.uncached_cpsr = ARM_CPU_MODE_SVC;
> +        s->cpu->env.daif = PSTATE_A | PSTATE_F | PSTATE_I;
>          s->cpu->env.cp15.c1_sys = 0;
>          s->cpu->env.cp15.c1_coproc = 0;
>          s->cpu->env.cp15.ttbr0_el1 = 0;
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 2f94943..7384e17 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -94,8 +94,7 @@ static void arm_cpu_reset(CPUState *s)
>          /* Userspace expects access to CTL_EL0 and the cache ops */
>          env->cp15.c1_sys |= SCTLR_UCT | SCTLR_UCI;
>  #else
> -        env->pstate = PSTATE_D | PSTATE_A | PSTATE_I | PSTATE_F
> -            | PSTATE_MODE_EL1h;
> +        env->pstate = PSTATE_MODE_EL1h;
>  #endif
>      }
>
> @@ -110,13 +109,14 @@ static void arm_cpu_reset(CPUState *s)
>      }
>  #else
>      /* SVC mode with interrupts disabled.  */
> -    env->uncached_cpsr = ARM_CPU_MODE_SVC | CPSR_A | CPSR_F | CPSR_I;
> +    env->uncached_cpsr = ARM_CPU_MODE_SVC;
> +    env->daif = PSTATE_D | PSTATE_A | PSTATE_I | PSTATE_F;
>      /* On ARMv7-M the CPSR_I is the value of the PRIMASK register, and is
>         clear at reset.  Initial SP and PC are loaded from ROM.  */
>      if (IS_M(env)) {
>          uint32_t pc;
>          uint8_t *rom;
> -        env->uncached_cpsr &= ~CPSR_I;
> +        env->daif &= ~PSTATE_I;
>          rom = rom_ptr(0);
>          if (rom) {
>              /* We should really use ldl_phys here, in case the guest
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index ec0214d..571b033 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -135,6 +135,7 @@ typedef struct CPUARMState {
>       *  NZCV are kept in the split out env->CF/VF/NF/ZF, (which have the same
>       *    semantics as for AArch32, as described in the comments on each 
> field)
>       *  nRW (also known as M[4]) is kept, inverted, in env->aarch64
> +     *  DAIF (exception masks) are kept in env->daif
>       *  all other bits are stored in their correct places in env->pstate
>       */
>      uint32_t pstate;
> @@ -164,6 +165,7 @@ typedef struct CPUARMState {
>      uint32_t GE; /* cpsr[19:16] */
>      uint32_t thumb; /* cpsr[5]. 0 = arm mode, 1 = thumb mode. */
>      uint32_t condexec_bits; /* IT bits.  cpsr[15:10,26:25].  */
> +    uint32_t daif; /* exception masks, in the bits they are in in PSTATE */
>
>      /* System control coprocessor (cp15) */
>      struct {
> @@ -406,9 +408,11 @@ int cpu_arm_handle_mmu_fault (CPUARMState *env, 
> target_ulong address, int rw,
>  #define CPSR_Z (1U << 30)
>  #define CPSR_N (1U << 31)
>  #define CPSR_NZCV (CPSR_N | CPSR_Z | CPSR_C | CPSR_V)
> +#define CPSR_AIF (CPSR_A | CPSR_I | CPSR_F)
>
>  #define CPSR_IT (CPSR_IT_0_1 | CPSR_IT_2_7)
> -#define CACHED_CPSR_BITS (CPSR_T | CPSR_GE | CPSR_IT | CPSR_Q | CPSR_NZCV)
> +#define CACHED_CPSR_BITS (CPSR_T | CPSR_AIF | CPSR_GE | CPSR_IT | CPSR_Q \
> +    | CPSR_NZCV)
>  /* Bits writable in user mode.  */
>  #define CPSR_USER (CPSR_NZCV | CPSR_Q | CPSR_GE)
>  /* Execution state bits.  MRS read as zero, MSR writes ignored.  */
> @@ -431,7 +435,8 @@ int cpu_arm_handle_mmu_fault (CPUARMState *env, 
> target_ulong address, int rw,
>  #define PSTATE_Z (1U << 30)
>  #define PSTATE_N (1U << 31)
>  #define PSTATE_NZCV (PSTATE_N | PSTATE_Z | PSTATE_C | PSTATE_V)
> -#define CACHED_PSTATE_BITS (PSTATE_NZCV)
> +#define PSTATE_DAIF (PSTATE_D | PSTATE_A | PSTATE_I | PSTATE_F)
> +#define CACHED_PSTATE_BITS (PSTATE_NZCV | PSTATE_DAIF)
>  /* Mode values for AArch64 */
>  #define PSTATE_MODE_EL3h 13
>  #define PSTATE_MODE_EL3t 12
> @@ -452,7 +457,7 @@ static inline uint32_t pstate_read(CPUARMState *env)
>      ZF = (env->ZF == 0);
>      return (env->NF & 0x80000000) | (ZF << 30)
>          | (env->CF << 29) | ((env->VF & 0x80000000) >> 3)
> -        | env->pstate;
> +        | env->pstate | env->daif;
>  }
>
>  static inline void pstate_write(CPUARMState *env, uint32_t val)
> @@ -461,6 +466,7 @@ static inline void pstate_write(CPUARMState *env, 
> uint32_t val)
>      env->NF = val;
>      env->CF = (val >> 29) & 1;
>      env->VF = (val << 3) & 0x80000000;
> +    env->daif = val & PSTATE_DAIF;
>      env->pstate = val & ~CACHED_PSTATE_BITS;
>  }
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 5c2be64..367fbbe 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -2475,7 +2475,7 @@ uint32_t cpsr_read(CPUARMState *env)
>          (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->GE << 16) | env->daif;
>  }
>
>  void cpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)
> @@ -2502,6 +2502,9 @@ void cpsr_write(CPUARMState *env, uint32_t val, 
> uint32_t mask)
>          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.
> @@ -2961,7 +2964,7 @@ void arm_cpu_do_interrupt(CPUState *cs)
>      env->condexec_bits = 0;
>      /* Switch to the new mode, and to the correct instruction set.  */
>      env->uncached_cpsr = (env->uncached_cpsr & ~CPSR_M) | new_mode;
> -    env->uncached_cpsr |= mask;
> +    env->daif |= mask;
>      /* this is a lie, as the was no c1_sys on V4T/V5, but who cares
>       * and we should just guard the thumb mode on V4 */
>      if (arm_feature(env, ARM_FEATURE_V4T)) {
> @@ -3631,12 +3634,12 @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t 
> reg)
>      case 9: /* PSP */
>          return env->v7m.current_sp ? env->regs[13] : env->v7m.other_sp;
>      case 16: /* PRIMASK */
> -        return (env->uncached_cpsr & CPSR_I) != 0;
> +        return (env->daif & PSTATE_I) != 0;
>      case 17: /* BASEPRI */
>      case 18: /* BASEPRI_MAX */
>          return env->v7m.basepri;
>      case 19: /* FAULTMASK */
> -        return (env->uncached_cpsr & CPSR_F) != 0;
> +        return (env->daif & PSTATE_F) != 0;
>      case 20: /* CONTROL */
>          return env->v7m.control;
>      default:
> @@ -3683,10 +3686,11 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t reg, 
> uint32_t val)
>              env->v7m.other_sp = val;
>          break;
>      case 16: /* PRIMASK */
> -        if (val & 1)
> -            env->uncached_cpsr |= CPSR_I;
> -        else
> -            env->uncached_cpsr &= ~CPSR_I;
> +        if (val & 1) {
> +            env->daif |= PSTATE_I;
> +        } else {
> +            env->daif &= ~PSTATE_I;
> +        }
>          break;
>      case 17: /* BASEPRI */
>          env->v7m.basepri = val & 0xff;
> @@ -3697,10 +3701,11 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t reg, 
> uint32_t val)
>              env->v7m.basepri = val;
>          break;
>      case 19: /* FAULTMASK */
> -        if (val & 1)
> -            env->uncached_cpsr |= CPSR_F;
> -        else
> -            env->uncached_cpsr &= ~CPSR_F;
> +        if (val & 1) {
> +            env->daif |= PSTATE_F;
> +        } else {
> +            env->daif &= ~PSTATE_F;
> +        }
>          break;
>      case 20: /* CONTROL */
>          env->v7m.control = val & 3;
> --
> 1.8.5
>
>