From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33580) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YJ4w2-0000aJ-OV for qemu-devel@nongnu.org; Wed, 04 Feb 2015 13:44:25 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YJ4vz-0003aV-Fv for qemu-devel@nongnu.org; Wed, 04 Feb 2015 13:44:22 -0500 Received: from mail-qc0-f176.google.com ([209.85.216.176]:59573) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YJ4vz-0003aP-AW for qemu-devel@nongnu.org; Wed, 04 Feb 2015 13:44:19 -0500 Received: by mail-qc0-f176.google.com with SMTP id c9so2767509qcz.7 for ; Wed, 04 Feb 2015 10:44:19 -0800 (PST) MIME-Version: 1.0 In-Reply-To: References: <1422403117-16921-1-git-send-email-greg.bellows@linaro.org> <1422403117-16921-4-git-send-email-greg.bellows@linaro.org> Date: Wed, 4 Feb 2015 12:44:18 -0600 Message-ID: From: Greg Bellows Content-Type: multipart/alternative; boundary=001a11c11d12368e04050e4793ea Subject: Re: [Qemu-devel] [PATCH v3 3/4] target-arm: Add 32/64-bit register sync List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: =?UTF-8?B?QWxleCBCZW5uw6ll?= , QEMU Developers , Christoffer Dall --001a11c11d12368e04050e4793ea Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On Tue, Feb 3, 2015 at 12:54 PM, Peter Maydell wrote: > On 27 January 2015 at 23:58, Greg Bellows wrote= : > > Add AArch32 to AArch64 register sychronization functions. > > Replace manual register synchronization with new functions in > > aarch64_cpu_do_interrupt() and HELPER(exception_return)(). > > > > Signed-off-by: Greg Bellows > > > > --- > > > > v2 -> v3 > > - Conditionalize interrupt handler update of aarch64. > > --- > > target-arm/helper-a64.c | 9 +++-- > > target-arm/internals.h | 89 > +++++++++++++++++++++++++++++++++++++++++++++++++ > > target-arm/op_helper.c | 6 ++-- > > 3 files changed, 95 insertions(+), 9 deletions(-) > > > > diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c > > index 81066ca..8c6a100 100644 > > --- a/target-arm/helper-a64.c > > +++ b/target-arm/helper-a64.c > > @@ -448,7 +448,6 @@ void aarch64_cpu_do_interrupt(CPUState *cs) > > unsigned int new_el =3D arm_excp_target_el(cs, cs->exception_index= ); > > target_ulong addr =3D env->cp15.vbar_el[new_el]; > > unsigned int new_mode =3D aarch64_pstate_mode(new_el, true); > > - int i; > > > > if (arm_current_el(env) < new_el) { > > if (env->aarch64) { > > @@ -512,15 +511,15 @@ void aarch64_cpu_do_interrupt(CPUState *cs) > > } > > env->elr_el[new_el] =3D env->regs[15]; > > > > - for (i =3D 0; i < 15; i++) { > > - env->xregs[i] =3D env->regs[i]; > > - } > > + aarch64_sync_32_to_64(env); > > > > env->condexec_bits =3D 0; > > } > > > > pstate_write(env, PSTATE_DAIF | new_mode); > > - env->aarch64 =3D 1; > > + if (arm_feature(env, ARM_FEATURE_AARCH64)) { > > + env->aarch64 =3D 1; > > + } > > This doesn't look correct. If this CPU doesn't have the AArch64 > feature then pretty much all of what this function does is wrong, > because it is the "take exception to an EL that is using AArch64" > function. We need to ensure that CPUs without the feature > bit never call this, either by setting the CPU class method > pointer correctly (which is what happens at the moment for > the traditional 32-bit-only CPUs) or else we need to arrange that > we dynamically call the right function depending on the register > width of the EL we're about to take the exception to. We'll > definitely need to handle that latter case for 64-bit EL3 or EL2 > support (since suddenly EL1 can be 32-bit even in a CPU with > 64-bit support enabled). > > > aarch64_restore_sp(env, new_el); > > > > env->pc =3D addr; > > diff --git a/target-arm/internals.h b/target-arm/internals.h > > index bb171a7..626ea7d 100644 > > --- a/target-arm/internals.h > > +++ b/target-arm/internals.h > > @@ -128,6 +128,95 @@ static inline void aarch64_restore_sp(CPUARMState > *env, int el) > > } > > } > > > > +static inline void aarch64_sync_32_to_64(CPUARMState *env) > > These functions are pretty big; I would just put them in a > suitable .c file rather than have them be static inline in > a .h file. A few lines of doc comment describing the purpose > of the functions would be nice too. > =E2=80=8BMoved and commented in v4 =E2=80=8B > > > +{ > > + int i; > > + > > + /* We can blanket copy R[0:7] to X[0:7] */ > > + for (i =3D 0; i < 8; i++) { > > + env->xregs[i] =3D env->regs[i]; > > + } > > + > > + /* If we are in USR mode then we just want to complete the above > blanket > > + * copy so we get the accurate register values. If not, then we > have to go > > + * to the saved and banked user regs. > > + */ > > + if ((env->uncached_cpsr & 0x1f) =3D=3D ARM_CPU_MODE_USR) { > > + for (i =3D 8; i < 15; i++) { > > + env->xregs[i] =3D env->regs[i]; > > + } > > + } else { > > + for (i =3D 8; i < 13; i++) { > > + env->xregs[i] =3D env->usr_regs[i-8]; > > + } > > + env->xregs[13] =3D env->banked_r13[bank_number(ARM_CPU_MODE_US= R)]; > > + env->xregs[14] =3D env->banked_r14[bank_number(ARM_CPU_MODE_US= R)]; > > + } > > + env->pc =3D env->regs[15]; > > + > > + env->xregs[15] =3D env->banked_r13[bank_number(ARM_CPU_MODE_HYP)]; > > + env->xregs[16] =3D env->banked_r13[bank_number(ARM_CPU_MODE_IRQ)]; > > + env->xregs[17] =3D env->banked_r14[bank_number(ARM_CPU_MODE_IRQ)]; > > + env->xregs[18] =3D env->banked_r13[bank_number(ARM_CPU_MODE_SVC)]; > > + env->xregs[19] =3D env->banked_r14[bank_number(ARM_CPU_MODE_SVC)]; > > + env->xregs[20] =3D env->banked_r13[bank_number(ARM_CPU_MODE_ABT)]; > > + env->xregs[21] =3D env->banked_r14[bank_number(ARM_CPU_MODE_ABT)]; > > + env->xregs[22] =3D env->banked_r13[bank_number(ARM_CPU_MODE_UND)]; > > + env->xregs[23] =3D env->banked_r14[bank_number(ARM_CPU_MODE_UND)]; > > This isn't going to give the right answers if we happen to be in > one of these modes. For instance if we're in SVC mode then SVC's > r13 (which needs to be copied into xregs[18]) is going to be > in env->regs[13], not in the banked_r13 slot. > =E2=80=8BYes, you are correct and I dealt with this for USR mode but for so= me reason failed to do so for the other modes. Fixed in v4.=E2=80=8B > > > + > > + for (i =3D 0; i < 5; i++) { > > + env->xregs[24+i] =3D env->fiq_regs[i]; > > Similarly if we're in FIQ mode then the remainder of the FIQ > registers are in env->regs[8] to [14], not in the fiq_regs[] or > banked_r13[] or banked_r14[]. > > PS: spaces round operators like '+', please. > =E2=80=8BFixed in v4 > > > + } > > + env->xregs[29] =3D env->banked_r13[bank_number(ARM_CPU_MODE_FIQ)]; > > + env->xregs[30] =3D env->banked_r14[bank_number(ARM_CPU_MODE_FIQ)]; > > +} > > + > > +static inline void aarch64_sync_64_to_32(CPUARMState *env) > > +{ > > + int i; > > + > > + /* We can blanket copy R[0:7] to X[0:7] */ > > + for (i =3D 0; i < 8; i++) { > > + env->regs[i] =3D env->xregs[i]; > > + } > > + > > + /* If we are in USR mode then we want to complete the above blanke= t > > + * copy as the XREGs will contain the most recent value. > > + */ > > + if ((env->uncached_cpsr & 0x1f) =3D=3D ARM_CPU_MODE_USR) { > > use the CPSR_M mask, not 0x1f. > > > + for (i =3D 8; i < 15; i++) { > > + env->regs[i] =3D env->xregs[i]; > > + } > > + } > > This is missing the update of regs[8..14] for modes other than USR > (which is sort of the inverse of the problem in the preceding function). > Yes, this is due to the same omission you pointed out above. I should also point out that r8:r12 only get sync'd when in USR or FIQ mode. > > > + > > + /* Update the user copies and banked registers so they are also up > to > > + * date. > > + */ > > + for (i =3D 8; i < 13; i++) { > > + env->usr_regs[i-8] =3D env->xregs[i]; > > + } > > + env->banked_r13[bank_number(ARM_CPU_MODE_USR)] =3D env->xregs[13]; > > + env->banked_r14[bank_number(ARM_CPU_MODE_USR)] =3D env->xregs[14]; > > + > > + env->regs[15] =3D env->pc; > > This feels a bit out of place -- I'd put it up with the rest > of the updates of env->regs[]. > =E2=80=8BI originally put this at the end because regs[15] was the last reg= ister numerically to be updated. I think the end makes even more sense given my v4 changes. Let me know if you agree.=E2=80=8B > > > + env->banked_r13[bank_number(ARM_CPU_MODE_HYP)] =3D env->xregs[15]; > > + env->banked_r13[bank_number(ARM_CPU_MODE_IRQ)] =3D env->xregs[16]; > > + env->banked_r14[bank_number(ARM_CPU_MODE_IRQ)] =3D env->xregs[17]; > > + env->banked_r13[bank_number(ARM_CPU_MODE_SVC)] =3D env->xregs[18]; > > + env->banked_r14[bank_number(ARM_CPU_MODE_SVC)] =3D env->xregs[19]; > > + env->banked_r13[bank_number(ARM_CPU_MODE_ABT)] =3D env->xregs[20]; > > + env->banked_r14[bank_number(ARM_CPU_MODE_ABT)] =3D env->xregs[21]; > > + env->banked_r13[bank_number(ARM_CPU_MODE_UND)] =3D env->xregs[22]; > > + env->banked_r14[bank_number(ARM_CPU_MODE_UND)] =3D env->xregs[23]; > > + > > + for (i =3D 0; i < 5; i++) { > > + env->fiq_regs[i] =3D env->xregs[24+i]; > > + } > > + env->banked_r13[bank_number(ARM_CPU_MODE_FIQ)] =3D env->xregs[29]; > > + env->banked_r14[bank_number(ARM_CPU_MODE_FIQ)] =3D env->xregs[30]; > > +} > > + > > static inline void update_spsel(CPUARMState *env, uint32_t imm) > > { > > unsigned int cur_el =3D arm_current_el(env); > > diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c > > index 2bed914..7713022 100644 > > --- a/target-arm/op_helper.c > > +++ b/target-arm/op_helper.c > > @@ -465,7 +465,7 @@ void HELPER(exception_return)(CPUARMState *env) > > int cur_el =3D arm_current_el(env); > > unsigned int spsr_idx =3D aarch64_banked_spsr_index(cur_el); > > uint32_t spsr =3D env->banked_spsr[spsr_idx]; > > - int new_el, i; > > + int new_el; > > > > aarch64_save_sp(env, cur_el); > > > > @@ -491,9 +491,7 @@ void HELPER(exception_return)(CPUARMState *env) > > if (!arm_singlestep_active(env)) { > > env->uncached_cpsr &=3D ~PSTATE_SS; > > } > > - for (i =3D 0; i < 15; i++) { > > - env->regs[i] =3D env->xregs[i]; > > - } > > + aarch64_sync_64_to_32(env); > > > > env->regs[15] =3D env->elr_el[1] & ~0x1; > > } else { > > thanks > -- PMM > --001a11c11d12368e04050e4793ea Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable


On Tue, Feb 3, 2015 at 12:54 PM, Peter Maydell <peter.= maydell@linaro.org> wrote:
=
On 27 January 2015 at 23:58, Greg Bellows <greg.bellows@linaro.org> w= rote:
> Add AArch32 to AArch64 register sychronization functions.
> Replace manual register synchronization with new functions in
> aarch64_cpu_do_interrupt() and HELPER(exception_return)().
>
> Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
>
> ---
>
> v2 -> v3
> - Conditionalize interrupt handler update of aarch64.
> ---
>=C2=A0 target-arm/helper-a64.c |=C2=A0 9 +++--
>=C2=A0 target-arm/internals.h=C2=A0 | 89 ++++++++++++++++++++++++++++++= +++++++++++++++++++
>=C2=A0 target-arm/op_helper.c=C2=A0 |=C2=A0 6 ++--
>=C2=A0 3 files changed, 95 insertions(+), 9 deletions(-)
>
> diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> index 81066ca..8c6a100 100644
> --- a/target-arm/helper-a64.c
> +++ b/target-arm/helper-a64.c
> @@ -448,7 +448,6 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>=C2=A0 =C2=A0 =C2=A0 unsigned int new_el =3D arm_excp_target_el(cs, cs-= >exception_index);
>=C2=A0 =C2=A0 =C2=A0 target_ulong addr =3D env->cp15.vbar_el[new_el]= ;
>=C2=A0 =C2=A0 =C2=A0 unsigned int new_mode =3D aarch64_pstate_mode(new_= el, true);
> -=C2=A0 =C2=A0 int i;
>
>=C2=A0 =C2=A0 =C2=A0 if (arm_current_el(env) < new_el) {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (env->aarch64) {
> @@ -512,15 +511,15 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 }
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 env->elr_el[new_el] =3D env->r= egs[15];
>
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 for (i =3D 0; i < 15; i++) {
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 env->xregs[i] =3D env-&g= t;regs[i];
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 aarch64_sync_32_to_64(env);
>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 env->condexec_bits =3D 0;
>=C2=A0 =C2=A0 =C2=A0 }
>
>=C2=A0 =C2=A0 =C2=A0 pstate_write(env, PSTATE_DAIF | new_mode);
> -=C2=A0 =C2=A0 env->aarch64 =3D 1;
> +=C2=A0 =C2=A0 if (arm_feature(env, ARM_FEATURE_AARCH64)) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 env->aarch64 =3D 1;
> +=C2=A0 =C2=A0 }

This doesn't look correct. If this CPU doesn't have the= AArch64
feature then pretty much all of what this function does is wrong,
because it is the "take exception to an EL that is using AArch64"=
function. We need to ensure that CPUs without the feature
bit never call this, either by setting the CPU class method
pointer correctly (which is what happens at the moment for
the traditional 32-bit-only CPUs) or else we need to arrange that
we dynamically call the right function depending on the register
width of the EL we're about to take the exception to. We'll
definitely need to handle that latter case for 64-bit EL3 or EL2
support (since suddenly EL1 can be 32-bit even in a CPU with
64-bit support enabled).

>=C2=A0 =C2=A0 =C2=A0 aarch64_restore_sp(env, new_el);
>
>=C2=A0 =C2=A0 =C2=A0 env->pc =3D addr;
> diff --git a/target-arm/internals.h b/target-arm/internals.h
> index bb171a7..626ea7d 100644
> --- a/target-arm/internals.h
> +++ b/target-arm/internals.h
> @@ -128,6 +128,95 @@ static inline void aarch64_restore_sp(CPUARMState= *env, int el)
>=C2=A0 =C2=A0 =C2=A0 }
>=C2=A0 }
>
> +static inline void aarch64_sync_32_to_64(CPUARMState *env)

These functions are pretty big; I would just put them in a
suitable .c file rather than have them be static inline in
a .h file. A few lines of doc comment describing the purpose
of the functions would be nice too.

=E2=80=8BMoved and commented in v4
=E2=80=8B
=C2=A0

> +{
> +=C2=A0 =C2=A0 int i;
> +
> +=C2=A0 =C2=A0 /* We can blanket copy R[0:7] to X[0:7] */
> +=C2=A0 =C2=A0 for (i =3D 0; i < 8; i++) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 env->xregs[i] =3D env->regs[i];
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 /* If we are in USR mode then we just want to complete = the above blanket
> +=C2=A0 =C2=A0 =C2=A0* copy so we get the accurate register values.=C2= =A0 If not, then we have to go
> +=C2=A0 =C2=A0 =C2=A0* to the saved and banked user regs.
> +=C2=A0 =C2=A0 =C2=A0*/
> +=C2=A0 =C2=A0 if ((env->uncached_cpsr & 0x1f) =3D=3D ARM_CPU_M= ODE_USR) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 for (i =3D 8; i < 15; i++) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 env->xregs[i] =3D env-&g= t;regs[i];
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 } else {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 for (i =3D 8; i < 13; i++) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 env->xregs[i] =3D env-&g= t;usr_regs[i-8];
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 env->xregs[13] =3D env->banked_r13[= bank_number(ARM_CPU_MODE_USR)];
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 env->xregs[14] =3D env->banked_r14[= bank_number(ARM_CPU_MODE_USR)];
> +=C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 env->pc =3D env->regs[15];
> +
> +=C2=A0 =C2=A0 env->xregs[15] =3D env->banked_r13[bank_number(AR= M_CPU_MODE_HYP)];
> +=C2=A0 =C2=A0 env->xregs[16] =3D env->banked_r13[bank_number(AR= M_CPU_MODE_IRQ)];
> +=C2=A0 =C2=A0 env->xregs[17] =3D env->banked_r14[bank_number(AR= M_CPU_MODE_IRQ)];
> +=C2=A0 =C2=A0 env->xregs[18] =3D env->banked_r13[bank_number(AR= M_CPU_MODE_SVC)];
> +=C2=A0 =C2=A0 env->xregs[19] =3D env->banked_r14[bank_number(AR= M_CPU_MODE_SVC)];
> +=C2=A0 =C2=A0 env->xregs[20] =3D env->banked_r13[bank_number(AR= M_CPU_MODE_ABT)];
> +=C2=A0 =C2=A0 env->xregs[21] =3D env->banked_r14[bank_number(AR= M_CPU_MODE_ABT)];
> +=C2=A0 =C2=A0 env->xregs[22] =3D env->banked_r13[bank_number(AR= M_CPU_MODE_UND)];
> +=C2=A0 =C2=A0 env->xregs[23] =3D env->banked_r14[bank_number(AR= M_CPU_MODE_UND)];

This isn't going to give the right answers if we happen to = be in
one of these modes. For instance if we're in SVC mode then SVC's r13 (which needs to be copied into=C2=A0 xregs[18]) is going to be
in env->regs[13], not in the banked_r13 slot.

<= /div>
=E2=80=8BYes, you are correct and I dealt with this for USR mo= de but for some reason failed to do so for the other modes.=C2=A0 Fixed in = v4.=E2=80=8B
=C2=A0

> +
> +=C2=A0 =C2=A0 for (i =3D 0; i < 5; i++) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 env->xregs[24+i] =3D env->fiq_regs[= i];

Similarly if we're in FIQ mode then the remainder of the FIQ
registers are in env->regs[8] to [14], not in the fiq_regs[] or
banked_r13[] or banked_r14[].

PS: spaces round operators like '+', please.
<= br>
=E2=80=8BFixed in v4
=C2=A0

> +=C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 env->xregs[29] =3D env->banked_r13[bank_number(AR= M_CPU_MODE_FIQ)];
> +=C2=A0 =C2=A0 env->xregs[30] =3D env->banked_r14[bank_number(AR= M_CPU_MODE_FIQ)];
> +}
> +
> +static inline void aarch64_sync_64_to_32(CPUARMState *env)
> +{
> +=C2=A0 =C2=A0 int i;
> +
> +=C2=A0 =C2=A0 /* We can blanket copy R[0:7] to X[0:7] */
> +=C2=A0 =C2=A0 for (i =3D 0; i < 8; i++) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 env->regs[i] =3D env->xregs[i];
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 /* If we are in USR mode then we want to complete the a= bove blanket
> +=C2=A0 =C2=A0 =C2=A0* copy as the XREGs will contain the most recent = value.
> +=C2=A0 =C2=A0 =C2=A0*/
> +=C2=A0 =C2=A0 if ((env->uncached_cpsr & 0x1f) =3D=3D ARM_CPU_M= ODE_USR) {

use the CPSR_M mask, not 0x1f.

> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 for (i =3D 8; i < 15; i++) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 env->regs[i] =3D env->= ;xregs[i];
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 }

This is missing the update of regs[8..14] for modes other than USR (which is sort of the inverse of the problem in the preceding function).

Yes, this is due to the same omission = you pointed out above.=C2=A0 I should also point out that r8:r12 only get s= ync'd when in USR or FIQ mode.
=C2=A0

> +
> +=C2=A0 =C2=A0 /* Update the user copies and banked registers so they = are also up to
> +=C2=A0 =C2=A0 =C2=A0* date.
> +=C2=A0 =C2=A0 =C2=A0*/
> +=C2=A0 =C2=A0 for (i =3D 8; i < 13; i++) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 env->usr_regs[i-8] =3D env->xregs[i= ];
> +=C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 env->banked_r13[bank_number(ARM_CPU_MODE_USR)] =3D e= nv->xregs[13];
> +=C2=A0 =C2=A0 env->banked_r14[bank_number(ARM_CPU_MODE_USR)] =3D e= nv->xregs[14];
> +
> +=C2=A0 =C2=A0 env->regs[15] =3D env->pc;

This feels a bit out of place -- I'd put it up with the rest
of the updates of env->regs[].

= =E2=80=8BI originally put this at the end because regs[15] was the last reg= ister numerically to be updated.=C2=A0 I think the end makes even more sens= e given my v4 changes.=C2=A0 Let me know if you agree.=E2=80=8B
=
=C2=A0

> +=C2=A0 =C2=A0 env->banked_r13[bank_number(ARM_CPU_MODE_HYP)] =3D e= nv->xregs[15];
> +=C2=A0 =C2=A0 env->banked_r13[bank_number(ARM_CPU_MODE_IRQ)] =3D e= nv->xregs[16];
> +=C2=A0 =C2=A0 env->banked_r14[bank_number(ARM_CPU_MODE_IRQ)] =3D e= nv->xregs[17];
> +=C2=A0 =C2=A0 env->banked_r13[bank_number(ARM_CPU_MODE_SVC)] =3D e= nv->xregs[18];
> +=C2=A0 =C2=A0 env->banked_r14[bank_number(ARM_CPU_MODE_SVC)] =3D e= nv->xregs[19];
> +=C2=A0 =C2=A0 env->banked_r13[bank_number(ARM_CPU_MODE_ABT)] =3D e= nv->xregs[20];
> +=C2=A0 =C2=A0 env->banked_r14[bank_number(ARM_CPU_MODE_ABT)] =3D e= nv->xregs[21];
> +=C2=A0 =C2=A0 env->banked_r13[bank_number(ARM_CPU_MODE_UND)] =3D e= nv->xregs[22];
> +=C2=A0 =C2=A0 env->banked_r14[bank_number(ARM_CPU_MODE_UND)] =3D e= nv->xregs[23];
> +
> +=C2=A0 =C2=A0 for (i =3D 0; i < 5; i++) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 env->fiq_regs[i] =3D env->xregs[24+= i];
> +=C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 env->banked_r13[bank_number(ARM_CPU_MODE_FIQ)] =3D e= nv->xregs[29];
> +=C2=A0 =C2=A0 env->banked_r14[bank_number(ARM_CPU_MODE_FIQ)] =3D e= nv->xregs[30];
> +}
> +
>=C2=A0 static inline void update_spsel(CPUARMState *env, uint32_t imm)<= br> >=C2=A0 {
>=C2=A0 =C2=A0 =C2=A0 unsigned int cur_el =3D arm_current_el(env);
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 2bed914..7713022 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -465,7 +465,7 @@ void HELPER(exception_return)(CPUARMState *env) >=C2=A0 =C2=A0 =C2=A0 int cur_el =3D arm_current_el(env);
>=C2=A0 =C2=A0 =C2=A0 unsigned int spsr_idx =3D aarch64_banked_spsr_inde= x(cur_el);
>=C2=A0 =C2=A0 =C2=A0 uint32_t spsr =3D env->banked_spsr[spsr_idx]; > -=C2=A0 =C2=A0 int new_el, i;
> +=C2=A0 =C2=A0 int new_el;
>
>=C2=A0 =C2=A0 =C2=A0 aarch64_save_sp(env, cur_el);
>
> @@ -491,9 +491,7 @@ void HELPER(exception_return)(CPUARMState *env) >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!arm_singlestep_active(env)) { >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 env->uncached_cpsr = &=3D ~PSTATE_SS;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 for (i =3D 0; i < 15; i++) {
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 env->regs[i] =3D env->= ;xregs[i];
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 aarch64_sync_64_to_32(env);
>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 env->regs[15] =3D env->elr_el[= 1] & ~0x1;
>=C2=A0 =C2=A0 =C2=A0 } else {

thanks
-- PMM

--001a11c11d12368e04050e4793ea--