Re: [Qemu-devel] [PATCH 10/14] target-arm: Make singlestate TB flags common between AArch32/64

[Edgar E. Iglesias]

On Tue, May 19, 2015 at 07:33:30PM +0100, Peter Maydell wrote:
> Currently we keep the TB flags PSTATE_SS and SS_ACTIVE in different
> bit positions for AArch64 and AArch32. Replace these separate
> definitions with a single common flag in the upper part of the
> flags word.
> 
> Signed-off-by: Peter Maydell <address@hidden>

Reviewed-by: Edgar E. Iglesias <address@hidden>


> ---
>  target-arm/cpu.h           | 69 
> ++++++++++++++++++----------------------------
>  target-arm/translate-a64.c |  4 +--
>  2 files changed, 29 insertions(+), 44 deletions(-)
> 
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 8cc4bc9..8aeb8aa 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1737,6 +1737,10 @@ static inline bool arm_singlestep_active(CPUARMState 
> *env)
>  #define ARM_TBFLAG_AARCH64_STATE_MASK  (1U << ARM_TBFLAG_AARCH64_STATE_SHIFT)
>  #define ARM_TBFLAG_MMUIDX_SHIFT 28
>  #define ARM_TBFLAG_MMUIDX_MASK (0x7 << ARM_TBFLAG_MMUIDX_SHIFT)
> +#define ARM_TBFLAG_SS_ACTIVE_SHIFT 27
> +#define ARM_TBFLAG_SS_ACTIVE_MASK (1 << ARM_TBFLAG_SS_ACTIVE_SHIFT)
> +#define ARM_TBFLAG_PSTATE_SS_SHIFT 26
> +#define ARM_TBFLAG_PSTATE_SS_MASK (1 << ARM_TBFLAG_PSTATE_SS_SHIFT)
>  
>  /* Bit usage when in AArch32 state: */
>  #define ARM_TBFLAG_THUMB_SHIFT      0
> @@ -1753,10 +1757,6 @@ static inline bool arm_singlestep_active(CPUARMState 
> *env)
>  #define ARM_TBFLAG_BSWAP_CODE_MASK  (1 << ARM_TBFLAG_BSWAP_CODE_SHIFT)
>  #define ARM_TBFLAG_CPACR_FPEN_SHIFT 17
>  #define ARM_TBFLAG_CPACR_FPEN_MASK  (1 << ARM_TBFLAG_CPACR_FPEN_SHIFT)
> -#define ARM_TBFLAG_SS_ACTIVE_SHIFT 18
> -#define ARM_TBFLAG_SS_ACTIVE_MASK (1 << ARM_TBFLAG_SS_ACTIVE_SHIFT)
> -#define ARM_TBFLAG_PSTATE_SS_SHIFT 19
> -#define ARM_TBFLAG_PSTATE_SS_MASK (1 << ARM_TBFLAG_PSTATE_SS_SHIFT)
>  /* We store the bottom two bits of the CPAR as TB flags and handle
>   * checks on the other bits at runtime
>   */
> @@ -1772,16 +1772,16 @@ static inline bool arm_singlestep_active(CPUARMState 
> *env)
>  /* Bit usage when in AArch64 state */
>  #define ARM_TBFLAG_AA64_FPEN_SHIFT  2
>  #define ARM_TBFLAG_AA64_FPEN_MASK   (1 << ARM_TBFLAG_AA64_FPEN_SHIFT)
> -#define ARM_TBFLAG_AA64_SS_ACTIVE_SHIFT 3
> -#define ARM_TBFLAG_AA64_SS_ACTIVE_MASK (1 << ARM_TBFLAG_AA64_SS_ACTIVE_SHIFT)
> -#define ARM_TBFLAG_AA64_PSTATE_SS_SHIFT 4
> -#define ARM_TBFLAG_AA64_PSTATE_SS_MASK (1 << ARM_TBFLAG_AA64_PSTATE_SS_SHIFT)
>  
>  /* some convenience accessor macros */
>  #define ARM_TBFLAG_AARCH64_STATE(F) \
>      (((F) & ARM_TBFLAG_AARCH64_STATE_MASK) >> ARM_TBFLAG_AARCH64_STATE_SHIFT)
>  #define ARM_TBFLAG_MMUIDX(F) \
>      (((F) & ARM_TBFLAG_MMUIDX_MASK) >> ARM_TBFLAG_MMUIDX_SHIFT)
> +#define ARM_TBFLAG_SS_ACTIVE(F) \
> +    (((F) & ARM_TBFLAG_SS_ACTIVE_MASK) >> ARM_TBFLAG_SS_ACTIVE_SHIFT)
> +#define ARM_TBFLAG_PSTATE_SS(F) \
> +    (((F) & ARM_TBFLAG_PSTATE_SS_MASK) >> ARM_TBFLAG_PSTATE_SS_SHIFT)
>  #define ARM_TBFLAG_THUMB(F) \
>      (((F) & ARM_TBFLAG_THUMB_MASK) >> ARM_TBFLAG_THUMB_SHIFT)
>  #define ARM_TBFLAG_VECLEN(F) \
> @@ -1796,18 +1796,10 @@ static inline bool arm_singlestep_active(CPUARMState 
> *env)
>      (((F) & ARM_TBFLAG_BSWAP_CODE_MASK) >> ARM_TBFLAG_BSWAP_CODE_SHIFT)
>  #define ARM_TBFLAG_CPACR_FPEN(F) \
>      (((F) & ARM_TBFLAG_CPACR_FPEN_MASK) >> ARM_TBFLAG_CPACR_FPEN_SHIFT)
> -#define ARM_TBFLAG_SS_ACTIVE(F) \
> -    (((F) & ARM_TBFLAG_SS_ACTIVE_MASK) >> ARM_TBFLAG_SS_ACTIVE_SHIFT)
> -#define ARM_TBFLAG_PSTATE_SS(F) \
> -    (((F) & ARM_TBFLAG_PSTATE_SS_MASK) >> ARM_TBFLAG_PSTATE_SS_SHIFT)
>  #define ARM_TBFLAG_XSCALE_CPAR(F) \
>      (((F) & ARM_TBFLAG_XSCALE_CPAR_MASK) >> ARM_TBFLAG_XSCALE_CPAR_SHIFT)
>  #define ARM_TBFLAG_AA64_FPEN(F) \
>      (((F) & ARM_TBFLAG_AA64_FPEN_MASK) >> ARM_TBFLAG_AA64_FPEN_SHIFT)
> -#define ARM_TBFLAG_AA64_SS_ACTIVE(F) \
> -    (((F) & ARM_TBFLAG_AA64_SS_ACTIVE_MASK) >> 
> ARM_TBFLAG_AA64_SS_ACTIVE_SHIFT)
> -#define ARM_TBFLAG_AA64_PSTATE_SS(F) \
> -    (((F) & ARM_TBFLAG_AA64_PSTATE_SS_MASK) >> 
> ARM_TBFLAG_AA64_PSTATE_SS_SHIFT)
>  #define ARM_TBFLAG_NS(F) \
>      (((F) & ARM_TBFLAG_NS_MASK) >> ARM_TBFLAG_NS_SHIFT)
>  
> @@ -1829,19 +1821,6 @@ static inline void cpu_get_tb_cpu_state(CPUARMState 
> *env, target_ulong *pc,
>          if (fpen == 3 || (fpen == 1 && arm_current_el(env) != 0)) {
>              *flags |= ARM_TBFLAG_AA64_FPEN_MASK;
>          }
> -        /* The SS_ACTIVE and PSTATE_SS bits correspond to the state machine
> -         * states defined in the ARM ARM for software singlestep:
> -         *  SS_ACTIVE   PSTATE.SS   State
> -         *     0            x       Inactive (the TB flag for SS is always 0)
> -         *     1            0       Active-pending
> -         *     1            1       Active-not-pending
> -         */
> -        if (arm_singlestep_active(env)) {
> -            *flags |= ARM_TBFLAG_AA64_SS_ACTIVE_MASK;
> -            if (env->pstate & PSTATE_SS) {
> -                *flags |= ARM_TBFLAG_AA64_PSTATE_SS_MASK;
> -            }
> -        }
>      } else {
>          *pc = env->regs[15];
>          *flags = (env->thumb << ARM_TBFLAG_THUMB_SHIFT)
> @@ -1859,24 +1838,30 @@ static inline void cpu_get_tb_cpu_state(CPUARMState 
> *env, target_ulong *pc,
>          if (fpen == 3 || (fpen == 1 && arm_current_el(env) != 0)) {
>              *flags |= ARM_TBFLAG_CPACR_FPEN_MASK;
>          }
> -        /* The SS_ACTIVE and PSTATE_SS bits correspond to the state machine
> -         * states defined in the ARM ARM for software singlestep:
> -         *  SS_ACTIVE   PSTATE.SS   State
> -         *     0            x       Inactive (the TB flag for SS is always 0)
> -         *     1            0       Active-pending
> -         *     1            1       Active-not-pending
> -         */
> -        if (arm_singlestep_active(env)) {
> -            *flags |= ARM_TBFLAG_SS_ACTIVE_MASK;
> -            if (env->uncached_cpsr & PSTATE_SS) {
> -                *flags |= ARM_TBFLAG_PSTATE_SS_MASK;
> -            }
> -        }
>          *flags |= (extract32(env->cp15.c15_cpar, 0, 2)
>                     << ARM_TBFLAG_XSCALE_CPAR_SHIFT);
>      }
>  
>      *flags |= (cpu_mmu_index(env) << ARM_TBFLAG_MMUIDX_SHIFT);
> +    /* The SS_ACTIVE and PSTATE_SS bits correspond to the state machine
> +     * states defined in the ARM ARM for software singlestep:
> +     *  SS_ACTIVE   PSTATE.SS   State
> +     *     0            x       Inactive (the TB flag for SS is always 0)
> +     *     1            0       Active-pending
> +     *     1            1       Active-not-pending
> +     */
> +    if (arm_singlestep_active(env)) {
> +        *flags |= ARM_TBFLAG_SS_ACTIVE_MASK;
> +        if (is_a64(env)) {
> +            if (env->pstate & PSTATE_SS) {
> +                *flags |= ARM_TBFLAG_PSTATE_SS_MASK;
> +            }
> +        } else {
> +            if (env->uncached_cpsr & PSTATE_SS) {
> +                *flags |= ARM_TBFLAG_PSTATE_SS_MASK;
> +            }
> +        }
> +    }
>  
>      *cs_base = 0;
>  }
> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
> index b1f44c9..b58778a 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -10975,8 +10975,8 @@ void gen_intermediate_code_internal_a64(ARMCPU *cpu,
>       *   emit code to generate a software step exception
>       *   end the TB
>       */
> -    dc->ss_active = ARM_TBFLAG_AA64_SS_ACTIVE(tb->flags);
> -    dc->pstate_ss = ARM_TBFLAG_AA64_PSTATE_SS(tb->flags);
> +    dc->ss_active = ARM_TBFLAG_SS_ACTIVE(tb->flags);
> +    dc->pstate_ss = ARM_TBFLAG_PSTATE_SS(tb->flags);
>      dc->is_ldex = false;
>      dc->ss_same_el = (arm_debug_target_el(env) == dc->current_el);
>  
> -- 
> 1.9.1
>