qemu-arm
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-arm] [PATCH 03/20] target/arm: Prepare for CONTROL.SPSEL being


From: Philippe Mathieu-Daudé
Subject: Re: [Qemu-arm] [PATCH 03/20] target/arm: Prepare for CONTROL.SPSEL being nonzero in Handler mode
Date: Thu, 5 Oct 2017 00:25:38 -0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0

On 09/22/2017 11:59 AM, Peter Maydell wrote:
> In the v7M architecture, there is an invariant that if the CPU is
> in Handler mode then the CONTROL.SPSEL bit cannot be nonzero.
> This in turn means that the current stack pointer is always
> indicated by CONTROL.SPSEL, even though Handler mode always uses
> the Main stack pointer.
> 
> In v8M, this invariant is removed, and CONTROL.SPSEL may now
> be nonzero in Handler mode (though Handler mode still always
> uses the Main stack pointer). In preparation for this change,
> change how we handle this bit: rename switch_v7m_sp() to
> the now more accurate write_v7m_control_spsel(), and make it
> check both the handler mode state and the SPSEL bit.
> 
> Note that this implicitly changes the point at which we switch
> active SP on exception exit from before we pop the exception
> frame to after it.
> 
> Signed-off-by: Peter Maydell <address@hidden>

Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

> ---
>  target/arm/cpu.h      |  8 ++++++-
>  hw/intc/armv7m_nvic.c |  2 +-
>  target/arm/helper.c   | 65 
> ++++++++++++++++++++++++++++++++++-----------------
>  3 files changed, 51 insertions(+), 24 deletions(-)
> 
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 8afceca..ad6eff4 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -991,6 +991,11 @@ void pmccntr_sync(CPUARMState *env);
>  #define PSTATE_MODE_EL1t 4
>  #define PSTATE_MODE_EL0t 0
>  
> +/* Write a new value to v7m.exception, thus transitioning into or out
> + * of Handler mode; this may result in a change of active stack pointer.
> + */
> +void write_v7m_exception(CPUARMState *env, uint32_t new_exc);
> +
>  /* Map EL and handler into a PSTATE_MODE.  */
>  static inline unsigned int aarch64_pstate_mode(unsigned int el, bool handler)
>  {
> @@ -1071,7 +1076,8 @@ static inline void xpsr_write(CPUARMState *env, 
> uint32_t val, uint32_t mask)
>          env->condexec_bits |= (val >> 8) & 0xfc;
>      }
>      if (mask & XPSR_EXCP) {
> -        env->v7m.exception = val & XPSR_EXCP;
> +        /* Note that this only happens on exception exit */
> +        write_v7m_exception(env, val & XPSR_EXCP);
>      }
>  }
>  
> diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
> index bc7b66d..a1041c2 100644
> --- a/hw/intc/armv7m_nvic.c
> +++ b/hw/intc/armv7m_nvic.c
> @@ -616,7 +616,7 @@ bool armv7m_nvic_acknowledge_irq(void *opaque)
>      vec->active = 1;
>      vec->pending = 0;
>  
> -    env->v7m.exception = s->vectpending;
> +    write_v7m_exception(env, s->vectpending);
>  
>      nvic_irq_update(s);
>  
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index f13b99d..509a1aa 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -6052,21 +6052,44 @@ static bool v7m_using_psp(CPUARMState *env)
>          env->v7m.control[env->v7m.secure] & R_V7M_CONTROL_SPSEL_MASK;
>  }
>  
> -/* Switch to V7M main or process stack pointer.  */
> -static void switch_v7m_sp(CPUARMState *env, bool new_spsel)
> +/* Write to v7M CONTROL.SPSEL bit. This may change the current
> + * stack pointer between Main and Process stack pointers.
> + */
> +static void write_v7m_control_spsel(CPUARMState *env, bool new_spsel)
>  {
>      uint32_t tmp;
> -    uint32_t old_control = env->v7m.control[env->v7m.secure];
> -    bool old_spsel = old_control & R_V7M_CONTROL_SPSEL_MASK;
> +    bool new_is_psp, old_is_psp = v7m_using_psp(env);
> +
> +    env->v7m.control[env->v7m.secure] =
> +        deposit32(env->v7m.control[env->v7m.secure],
> +                  R_V7M_CONTROL_SPSEL_SHIFT,
> +                  R_V7M_CONTROL_SPSEL_LENGTH, new_spsel);
> +
> +    new_is_psp = v7m_using_psp(env);
>  
> -    if (old_spsel != new_spsel) {
> +    if (old_is_psp != new_is_psp) {
>          tmp = env->v7m.other_sp;
>          env->v7m.other_sp = env->regs[13];
>          env->regs[13] = tmp;
> +    }
> +}
> +
> +void write_v7m_exception(CPUARMState *env, uint32_t new_exc)
> +{
> +    /* Write a new value to v7m.exception, thus transitioning into or out
> +     * of Handler mode; this may result in a change of active stack pointer.
> +     */
> +    bool new_is_psp, old_is_psp = v7m_using_psp(env);
> +    uint32_t tmp;
>  
> -        env->v7m.control[env->v7m.secure] = deposit32(old_control,
> -                                     R_V7M_CONTROL_SPSEL_SHIFT,
> -                                     R_V7M_CONTROL_SPSEL_LENGTH, new_spsel);
> +    env->v7m.exception = new_exc;
> +
> +    new_is_psp = v7m_using_psp(env);
> +
> +    if (old_is_psp != new_is_psp) {
> +        tmp = env->v7m.other_sp;
> +        env->v7m.other_sp = env->regs[13];
> +        env->regs[13] = tmp;
>      }
>  }
>  
> @@ -6149,13 +6172,11 @@ static uint32_t *get_v7m_sp_ptr(CPUARMState *env, 
> bool secure, bool threadmode,
>      bool want_psp = threadmode && spsel;
>  
>      if (secure == env->v7m.secure) {
> -        /* Currently switch_v7m_sp switches SP as it updates SPSEL,
> -         * so the SP we want is always in regs[13].
> -         * When we decouple SPSEL from the actually selected SP
> -         * we need to check want_psp against v7m_using_psp()
> -         * to see whether we need regs[13] or v7m.other_sp.
> -         */
> -        return &env->regs[13];
> +        if (want_psp == v7m_using_psp(env)) {
> +            return &env->regs[13];
> +        } else {
> +            return &env->v7m.other_sp;
> +        }
>      } else {
>          if (want_psp) {
>              return &env->v7m.other_ss_psp;
> @@ -6198,7 +6219,7 @@ static void v7m_exception_taken(ARMCPU *cpu, uint32_t 
> lr)
>      uint32_t addr;
>  
>      armv7m_nvic_acknowledge_irq(env->nvic);
> -    switch_v7m_sp(env, 0);
> +    write_v7m_control_spsel(env, 0);
>      arm_clear_exclusive(env);
>      /* Clear IT bits */
>      env->condexec_bits = 0;
> @@ -6344,11 +6365,11 @@ static void do_v7m_exception_exit(ARMCPU *cpu)
>          return;
>      }
>  
> -    /* Set CONTROL.SPSEL from excret.SPSEL. For QEMU this currently
> -     * causes us to switch the active SP, but we will change this
> -     * later to not do that so we can support v8M.
> +    /* Set CONTROL.SPSEL from excret.SPSEL. Since we're still in
> +     * Handler mode (and will be until we write the new XPSR.Interrupt
> +     * field) this does not switch around the current stack pointer.
>       */
> -    switch_v7m_sp(env, return_to_sp_process);
> +    write_v7m_control_spsel(env, return_to_sp_process);
>  
>      {
>          /* The stack pointer we should be reading the exception frame from
> @@ -9163,11 +9184,11 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t 
> maskreg, uint32_t val)
>      case 20: /* CONTROL */
>          /* Writing to the SPSEL bit only has an effect if we are in
>           * thread mode; other bits can be updated by any privileged code.
> -         * switch_v7m_sp() deals with updating the SPSEL bit in
> +         * write_v7m_control_spsel() deals with updating the SPSEL bit in
>           * env->v7m.control, so we only need update the others.
>           */
>          if (!arm_v7m_is_handler_mode(env)) {
> -            switch_v7m_sp(env, (val & R_V7M_CONTROL_SPSEL_MASK) != 0);
> +            write_v7m_control_spsel(env, (val & R_V7M_CONTROL_SPSEL_MASK) != 
> 0);
>          }
>          env->v7m.control[env->v7m.secure] &= ~R_V7M_CONTROL_NPRIV_MASK;
>          env->v7m.control[env->v7m.secure] |= val & R_V7M_CONTROL_NPRIV_MASK;
> 



reply via email to

[Prev in Thread] Current Thread [Next in Thread]