On Wed, May 21, 2014 at 08:20:20PM +0100, Peter Maydell wrote:
> On 19 May 2014 10:22, Edgar E. Iglesias <edgar.igles...@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.igles...@xilinx.com>
> >
> > Adds support for ERET to Aarch64 EL2 and 3.
>
> "AArch64". Also "to" here is ambiguous. The ARM ARM tries to
Changed to AArch64.
> 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
My first tests when starting booting in EL3 where to take exceptions
and return back to EL3 and EL2, the commit log msg didn't
evolve since but I guess I mean from and to EL2/3. Changed it to:
Adds support for ERET to and from AArch64 EL2 and 3.
If you have better suggestions I'm happy to change it to whatever.
> 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.)
I've tried to stay away from touching too much of the AArch32
code as I haven't had a setup to test 64/32 transitions
beyond a64/el1 and a32/el0.
I do have a follow-up patch (not very tested) for ERET to aarch32.
If you feel strongly about it I can squash the little I've got
for aarch32 allthough I'd rather do it incrementally and
leave it for follow-ups.
Cheers,
Edgar
>
> > Signed-off-by: Edgar E. Iglesias <edgar.igles...@xilinx.com>
> > ---
> > 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
