Re: [Qemu-devel] [PATCH v3 16/22] target-arm: A64: Generalize ERET to various ELs

[Peter Maydell]

On 19 May 2014 10:22, Edgar E. Iglesias <address@hidden> wrote:
> From: "Edgar E. Iglesias" <address@hidden>
>
> Adds support for ERET to Aarch64 EL2 and 3.

"AArch64". Also "to" here is ambiguous. The ARM ARM tries to
keep the terminology straight to avoid confusion: exceptions
are "taken from ELx" and "taken to ELx"; we "return from ELx"
and "return to ELx". This looks like it's talking about "return to
EL2/EL3" but probably doesn't really mean that.

If this patch is adding support for ERET from EL2/EL3
it needs to also fix the bit in the "returning to an exception
level which is 32 bit" which says "new_el = 0" since that's
not guaranteed to be true any more. (Also I think the register
mapping for AArch32 EL2/EL1 needs handling correctly.)

> Signed-off-by: Edgar E. Iglesias <address@hidden>
> ---
>  target-arm/op_helper.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index d89755a..c632dd6 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -386,13 +386,13 @@ void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t 
> op, uint32_t imm)
>
>  void HELPER(exception_return)(CPUARMState *env)
>  {
> -    unsigned int spsr_idx = is_a64(env) ? aarch64_banked_spsr_index(1) : 0;
> +    int cur_el = arm_current_pl(env);
> +    unsigned int spsr_idx = is_a64(env) ? aarch64_banked_spsr_index(cur_el) 
> : 0;
>      uint32_t spsr = env->banked_spsr[spsr_idx];
>      int new_el, i;
> -    int cur_el = arm_current_pl(env);
>
>      if (env->pstate & PSTATE_SP) {
> -        env->sp_el[1] = env->xregs[31];
> +        env->sp_el[cur_el] = env->xregs[31];
>      } else {
>          env->sp_el[0] = env->xregs[31];
>      }
> @@ -429,7 +429,7 @@ void HELPER(exception_return)(CPUARMState *env)
>          env->aarch64 = 1;
>          pstate_write(env, spsr);
>          env->xregs[31] = env->sp_el[new_el];
> -        env->pc = env->elr_el[1];
> +        env->pc = env->elr_el[cur_el];
>      }
>
>      return;
> @@ -443,7 +443,7 @@ illegal_return:
>       * no change to exception level, execution state or stack pointer
>       */
>      env->pstate |= PSTATE_IL;
> -    env->pc = env->elr_el[1];
> +    env->pc = env->elr_el[cur_el];
>      spsr &= PSTATE_NZCV | PSTATE_DAIF;
>      spsr |= pstate_read(env) & ~(PSTATE_NZCV | PSTATE_DAIF);
>      pstate_write(env, spsr);
> --
> 1.8.3.2
>

thanks
-- PMM