Re: [PATCH 09/11] target/arm: Set PAN bit as required on exception entry

[Peter Maydell]

On Tue, 3 Dec 2019 at 22:53, Richard Henderson
<address@hidden> wrote:
>
> The PAN bit is preserved, or set as per SCTLR_ELx.SPAN,
> plus several other conditions listed in the ARM ARM.
>
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  target/arm/helper.c | 42 +++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 39 insertions(+), 3 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index a1dbafb9b2..043e44d73d 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -8634,8 +8634,12 @@ static void take_aarch32_exception(CPUARMState *env, 
> int new_mode,
>                                     uint32_t mask, uint32_t offset,
>                                     uint32_t newpc)
>  {
> +    int new_el;
> +
>      /* Change the CPU state so as to actually take the exception. */
>      switch_mode(env, new_mode);
> +    new_el = arm_current_el(env);
> +
>      /*
>       * For exceptions taken to AArch32 we must clear the SS bit in both
>       * PSTATE and in the old-state value we save to SPSR_<mode>, so zero it 
> now.
> @@ -8648,7 +8652,7 @@ static void take_aarch32_exception(CPUARMState *env, 
> int new_mode,
>      env->uncached_cpsr = (env->uncached_cpsr & ~CPSR_M) | new_mode;
>      /* Set new mode endianness */
>      env->uncached_cpsr &= ~CPSR_E;
> -    if (env->cp15.sctlr_el[arm_current_el(env)] & SCTLR_EE) {
> +    if (env->cp15.sctlr_el[new_el] & SCTLR_EE) {
>          env->uncached_cpsr |= CPSR_E;
>      }
>      /* J and IL must always be cleared for exception entry */
> @@ -8659,6 +8663,14 @@ static void take_aarch32_exception(CPUARMState *env, 
> int new_mode,
>          env->thumb = (env->cp15.sctlr_el[2] & SCTLR_TE) != 0;
>          env->elr_el[2] = env->regs[15];
>      } else {
> +        /* CPSR.PAN is preserved unless target is EL1 and SCTLR.SPAN == 0. */
> +        if (cpu_isar_feature(aa64_pan, env_archcpu(env))) {
> +            env->uncached_cpsr |=
> +                (new_el == 1 &&
> +                 (env->cp15.sctlr_el[1] & SCTLR_SPAN) == 0
> +                 ? CPSR_PAN
> +                 : env->spsr & CPSR_PAN);

env->uncached_cpsr isn't wiped by this function, so the default
behaviour is "same as it was previously" without needing to fish
the bit out of env->spsr again, I think.

> +        }
>          /*
>           * this is a lie, as there was no c1_sys on V4T/V5, but who cares
>           * and we should just guard the thumb mode on V4

thanks
-- PMM