diff mbox

[v4,3/4] target-arm: Add 32/64-bit register sync

Message ID 1423565415-5844-4-git-send-email-greg.bellows@linaro.org
State New
Headers show

Commit Message

Greg Bellows Feb. 10, 2015, 10:50 a.m. UTC
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>

---

v3 -> v4
- Rework sync routines to cover various exception levels
- Move sync routines to helper.c

v2 -> v3
- Conditionalize interrupt handler update of aarch64.
---
 target-arm/cpu.h        |   2 +
 target-arm/helper-a64.c |   5 +-
 target-arm/helper.c     | 181 ++++++++++++++++++++++++++++++++++++++++++++++++
 target-arm/op_helper.c  |   6 +-
 4 files changed, 186 insertions(+), 8 deletions(-)

Comments

Peter Maydell Feb. 11, 2015, 4:13 a.m. UTC | #1
On 10 February 2015 at 10:50, Greg Bellows <greg.bellows@linaro.org> 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 <greg.bellows@linaro.org>
>
> ---
>
> v3 -> v4
> - Rework sync routines to cover various exception levels
> - Move sync routines to helper.c
>
> v2 -> v3
> - Conditionalize interrupt handler update of aarch64.
> ---
>  target-arm/cpu.h        |   2 +
>  target-arm/helper-a64.c |   5 +-
>  target-arm/helper.c     | 181 ++++++++++++++++++++++++++++++++++++++++++++++++
>  target-arm/op_helper.c  |   6 +-
>  4 files changed, 186 insertions(+), 8 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 1830a12..11845a6 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -495,6 +495,8 @@ typedef struct CPUARMState {
>  ARMCPU *cpu_arm_init(const char *cpu_model);
>  int cpu_arm_exec(CPUARMState *s);
>  uint32_t do_arm_semihosting(CPUARMState *env);
> +void aarch64_sync_32_to_64(CPUARMState *env);
> +void aarch64_sync_64_to_32(CPUARMState *env);
>
>  static inline bool is_a64(CPUARMState *env)
>  {
> diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> index 8aa40e9..7e0d038 100644
> --- a/target-arm/helper-a64.c
> +++ b/target-arm/helper-a64.c
> @@ -466,7 +466,6 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>      unsigned int new_el = arm_excp_target_el(cs, cs->exception_index);
>      target_ulong addr = env->cp15.vbar_el[new_el];
>      unsigned int new_mode = aarch64_pstate_mode(new_el, true);
> -    int i;
>
>      if (arm_current_el(env) < new_el) {
>          if (env->aarch64) {
> @@ -530,9 +529,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>          }
>          env->elr_el[new_el] = env->regs[15];
>
> -        for (i = 0; i < 15; i++) {
> -            env->xregs[i] = env->regs[i];
> -        }
> +        aarch64_sync_32_to_64(env);
>
>          env->condexec_bits = 0;
>      }
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 1a1a005..c1d6764 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -4096,6 +4096,15 @@ unsigned int arm_excp_target_el(CPUState *cs, unsigned int excp_idx)
>      return 1;
>  }
>
> +void aarch64_sync_64_to_32(CPUARMState *env)
> +{
> +    int i;
> +
> +    for (i = 0; i < 15; i++) {
> +        env->regs[i] = env->xregs[i];
> +    }
> +}

This is inside CONFIG_USER_ONLY, right? Is it called at all?
If so, when?

> +
>  #else
>
>  /* Map CPU modes onto saved register banks.  */
> @@ -4425,6 +4434,178 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
>      env->thumb = addr & 1;
>  }
>
> +/* Function used to synchronize QEMU's AArch64 register set with AArch32
> + * register set.  This is necessary when switching between AArch32 and AArch64
> + * execution state.

This is a bit vague...

> + */
> +void aarch64_sync_32_to_64(CPUARMState *env)
> +{
> +    int i;
> +    uint32_t mode = env->uncached_cpsr & CPSR_M;
> +
> +    /* We can blanket copy R[0:7] to X[0:7] */
> +    for (i = 0; i < 8; i++) {
> +        env->xregs[i] = env->regs[i];
> +    }
> +
> +    /* The latest copy of some registers depend on the current executing mode.

"depends"

> +     * The general purpose copy is used when the current mode corresponds to
> +     * the mapping for the given register.  Otherwise, the banked copy
> +     * corresponding to the register mapping is used.
> +     */
> +    if (mode == ARM_CPU_MODE_USR) {
> +        for (i = 8; i < 15; i++) {
> +            env->xregs[i] = env->regs[i];
> +        }
> +    } else {
> +        for (i = 8; i < 13; i++) {
> +            env->xregs[i] = env->usr_regs[i - 8];
> +        }
> +        env->xregs[13] = env->banked_r13[bank_number(ARM_CPU_MODE_USR)];
> +        env->xregs[14] = env->banked_r14[bank_number(ARM_CPU_MODE_USR)];
> +    }
> +
> +    if (mode == ARM_CPU_MODE_HYP) {
> +        env->xregs[15] = env->regs[13];
> +    } else {
> +        env->xregs[15] = env->banked_r13[bank_number(ARM_CPU_MODE_HYP)];
> +    }
> +
> +    if (mode == ARM_CPU_MODE_IRQ) {
> +        env->xregs[16] = env->regs[13];
> +        env->xregs[17] = env->regs[14];
> +    } else {
> +        env->xregs[16] = env->banked_r13[bank_number(ARM_CPU_MODE_IRQ)];
> +        env->xregs[17] = env->banked_r14[bank_number(ARM_CPU_MODE_IRQ)];
> +    }
> +
> +    if (mode == ARM_CPU_MODE_SVC) {
> +        env->xregs[18] = env->regs[13];
> +        env->xregs[19] = env->regs[14];
> +    } else {
> +        env->xregs[18] = env->banked_r13[bank_number(ARM_CPU_MODE_SVC)];
> +        env->xregs[19] = env->banked_r14[bank_number(ARM_CPU_MODE_SVC)];
> +    }
> +
> +    if (mode == ARM_CPU_MODE_ABT) {
> +        env->xregs[20] = env->regs[13];
> +        env->xregs[21] = env->regs[14];
> +    } else {
> +        env->xregs[20] = env->banked_r13[bank_number(ARM_CPU_MODE_ABT)];
> +        env->xregs[21] = env->banked_r14[bank_number(ARM_CPU_MODE_ABT)];
> +    }
> +
> +    if (mode == ARM_CPU_MODE_UND) {
> +        env->xregs[22] = env->regs[13];
> +        env->xregs[23] = env->regs[14];
> +    } else {
> +        env->xregs[22] = env->banked_r13[bank_number(ARM_CPU_MODE_UND)];
> +        env->xregs[23] = env->banked_r14[bank_number(ARM_CPU_MODE_UND)];
> +    }
> +
> +    if (mode == ARM_CPU_MODE_FIQ) {
> +        for (i = 24; i < 31; i++) {
> +            env->xregs[i] = env->regs[i - 16];   /* X[24:30] -> R[8:14] */
> +        }
> +    } else {
> +        for (i = 24; i < 29; i++) {
> +            env->xregs[i] = env->fiq_regs[i - 24];
> +        }
> +        env->xregs[29] = env->banked_r13[bank_number(ARM_CPU_MODE_FIQ)];
> +        env->xregs[30] = env->banked_r14[bank_number(ARM_CPU_MODE_FIQ)];
> +    }
> +
> +    env->pc = env->regs[15];
> +}
> +
> +/* Function used to synchronize QEMU's AArch32 register set with AArch64
> + * register set.  This is necessary when switching between AArch32 and AArch64
> + * execution state.
> + */
> +void aarch64_sync_64_to_32(CPUARMState *env)
> +{
> +    int i;
> +    uint32_t mode = env->uncached_cpsr & CPSR_M;
> +
> +    /* We can blanket copy X[0:7] to R[0:7] */
> +    for (i = 0; i < 8; i++) {
> +        env->regs[i] = env->xregs[i];
> +    }
> +
> +    /* The destination of some registers depend on the current executing mode.

"depends"

> +     * The AArch32 registers 8-12 are only sync'd when we are in USR or FIQ
> +     * mode as they are the only modes where AArch64 registers map to these
> +     * registers.  In all other cases, the respective USR and FIQ banks are
> +     * sync'd.
> +     * The AArch32 registers 13 & 14 are sync'd depending on the execution mode
> +     * we are in.  If we are not in a given mode, the bank corresponding to the
> +     * AArch64 register is sync'd.
> +     */
> +    if (mode == ARM_CPU_MODE_USR) {
> +        for (i = 8; i < 15; i++) {
> +            env->regs[i] = env->xregs[i];
> +        }

Something is wrong here, because we don't seem to be writing
anything to env->regs[8..15] if mode is neither USR nor FIQ.

I think you should rearrange this function. The 32->64 sync
needs to write a value to all of xregs[0..30], and it's
ordered such that we basically write them all in that order,
which makes it clear we don't miss any cases.

This function, on the other hand, needs to write to
regs[0..15] and also to the banked_* registers, so
it should be written to update them in that order,
rather than in xregs order.

You may also find it easier to always update the banked_*
copies regardless of the current mode -- it's safe to
write the value to them always, it just might be ignored
if we're currently executing for that mode.

-- PMM
Greg Bellows Feb. 11, 2015, 6:08 a.m. UTC | #2
On Tue, Feb 10, 2015 at 10:13 PM, Peter Maydell <peter.maydell@linaro.org>
wrote:

> On 10 February 2015 at 10:50, Greg Bellows <greg.bellows@linaro.org>
> 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 <greg.bellows@linaro.org>
> >
> > ---
> >
> > v3 -> v4
> > - Rework sync routines to cover various exception levels
> > - Move sync routines to helper.c
> >
> > v2 -> v3
> > - Conditionalize interrupt handler update of aarch64.
> > ---
> >  target-arm/cpu.h        |   2 +
> >  target-arm/helper-a64.c |   5 +-
> >  target-arm/helper.c     | 181
> ++++++++++++++++++++++++++++++++++++++++++++++++
> >  target-arm/op_helper.c  |   6 +-
> >  4 files changed, 186 insertions(+), 8 deletions(-)
> >
> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> > index 1830a12..11845a6 100644
> > --- a/target-arm/cpu.h
> > +++ b/target-arm/cpu.h
> > @@ -495,6 +495,8 @@ typedef struct CPUARMState {
> >  ARMCPU *cpu_arm_init(const char *cpu_model);
> >  int cpu_arm_exec(CPUARMState *s);
> >  uint32_t do_arm_semihosting(CPUARMState *env);
> > +void aarch64_sync_32_to_64(CPUARMState *env);
> > +void aarch64_sync_64_to_32(CPUARMState *env);
> >
> >  static inline bool is_a64(CPUARMState *env)
> >  {
> > diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> > index 8aa40e9..7e0d038 100644
> > --- a/target-arm/helper-a64.c
> > +++ b/target-arm/helper-a64.c
> > @@ -466,7 +466,6 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
> >      unsigned int new_el = arm_excp_target_el(cs, cs->exception_index);
> >      target_ulong addr = env->cp15.vbar_el[new_el];
> >      unsigned int new_mode = aarch64_pstate_mode(new_el, true);
> > -    int i;
> >
> >      if (arm_current_el(env) < new_el) {
> >          if (env->aarch64) {
> > @@ -530,9 +529,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
> >          }
> >          env->elr_el[new_el] = env->regs[15];
> >
> > -        for (i = 0; i < 15; i++) {
> > -            env->xregs[i] = env->regs[i];
> > -        }
> > +        aarch64_sync_32_to_64(env);
> >
> >          env->condexec_bits = 0;
> >      }
> > diff --git a/target-arm/helper.c b/target-arm/helper.c
> > index 1a1a005..c1d6764 100644
> > --- a/target-arm/helper.c
> > +++ b/target-arm/helper.c
> > @@ -4096,6 +4096,15 @@ unsigned int arm_excp_target_el(CPUState *cs,
> unsigned int excp_idx)
> >      return 1;
> >  }
> >
> > +void aarch64_sync_64_to_32(CPUARMState *env)
> > +{
> > +    int i;
> > +
> > +    for (i = 0; i < 15; i++) {
> > +        env->regs[i] = env->xregs[i];
> > +    }
> > +}
>
> This is inside CONFIG_USER_ONLY, right? Is it called at all?
> If so, when?
>

​The exception_return helper calls the function so I had to either add a
USER_CONFIG version of wrap the call in exception return with
CONFIG_USER_ONLY.  I chose the former, but either would work.  As you would
already know, the exception_return helper is likely not getting called in a
USER_ONLY build.

>
> > +
> >  #else
> >
> >  /* Map CPU modes onto saved register banks.  */
> > @@ -4425,6 +4434,178 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
> >      env->thumb = addr & 1;
> >  }
> >
> > +/* Function used to synchronize QEMU's AArch64 register set with AArch32
> > + * register set.  This is necessary when switching between AArch32 and
> AArch64
> > + * execution state.
>
> This is a bit vague...
>

​I'll elaborate.​


>
> > + */
> > +void aarch64_sync_32_to_64(CPUARMState *env)
> > +{
> > +    int i;
> > +    uint32_t mode = env->uncached_cpsr & CPSR_M;
> > +
> > +    /* We can blanket copy R[0:7] to X[0:7] */
> > +    for (i = 0; i < 8; i++) {
> > +        env->xregs[i] = env->regs[i];
> > +    }
> > +
> > +    /* The latest copy of some registers depend on the current
> executing mode.
>
> "depends"
>

Fixed in next version.

>
> > +     * The general purpose copy is used when the current mode
> corresponds to
> > +     * the mapping for the given register.  Otherwise, the banked copy
> > +     * corresponding to the register mapping is used.
> > +     */
> > +    if (mode == ARM_CPU_MODE_USR) {
> > +        for (i = 8; i < 15; i++) {
> > +            env->xregs[i] = env->regs[i];
> > +        }
> > +    } else {
> > +        for (i = 8; i < 13; i++) {
> > +            env->xregs[i] = env->usr_regs[i - 8];
> > +        }
> > +        env->xregs[13] = env->banked_r13[bank_number(ARM_CPU_MODE_USR)];
> > +        env->xregs[14] = env->banked_r14[bank_number(ARM_CPU_MODE_USR)];
> > +    }
> > +
> > +    if (mode == ARM_CPU_MODE_HYP) {
> > +        env->xregs[15] = env->regs[13];
> > +    } else {
> > +        env->xregs[15] = env->banked_r13[bank_number(ARM_CPU_MODE_HYP)];
> > +    }
> > +
> > +    if (mode == ARM_CPU_MODE_IRQ) {
> > +        env->xregs[16] = env->regs[13];
> > +        env->xregs[17] = env->regs[14];
> > +    } else {
> > +        env->xregs[16] = env->banked_r13[bank_number(ARM_CPU_MODE_IRQ)];
> > +        env->xregs[17] = env->banked_r14[bank_number(ARM_CPU_MODE_IRQ)];
> > +    }
> > +
> > +    if (mode == ARM_CPU_MODE_SVC) {
> > +        env->xregs[18] = env->regs[13];
> > +        env->xregs[19] = env->regs[14];
> > +    } else {
> > +        env->xregs[18] = env->banked_r13[bank_number(ARM_CPU_MODE_SVC)];
> > +        env->xregs[19] = env->banked_r14[bank_number(ARM_CPU_MODE_SVC)];
> > +    }
> > +
> > +    if (mode == ARM_CPU_MODE_ABT) {
> > +        env->xregs[20] = env->regs[13];
> > +        env->xregs[21] = env->regs[14];
> > +    } else {
> > +        env->xregs[20] = env->banked_r13[bank_number(ARM_CPU_MODE_ABT)];
> > +        env->xregs[21] = env->banked_r14[bank_number(ARM_CPU_MODE_ABT)];
> > +    }
> > +
> > +    if (mode == ARM_CPU_MODE_UND) {
> > +        env->xregs[22] = env->regs[13];
> > +        env->xregs[23] = env->regs[14];
> > +    } else {
> > +        env->xregs[22] = env->banked_r13[bank_number(ARM_CPU_MODE_UND)];
> > +        env->xregs[23] = env->banked_r14[bank_number(ARM_CPU_MODE_UND)];
> > +    }
> > +
> > +    if (mode == ARM_CPU_MODE_FIQ) {
> > +        for (i = 24; i < 31; i++) {
> > +            env->xregs[i] = env->regs[i - 16];   /* X[24:30] -> R[8:14]
> */
> > +        }
> > +    } else {
> > +        for (i = 24; i < 29; i++) {
> > +            env->xregs[i] = env->fiq_regs[i - 24];
> > +        }
> > +        env->xregs[29] = env->banked_r13[bank_number(ARM_CPU_MODE_FIQ)];
> > +        env->xregs[30] = env->banked_r14[bank_number(ARM_CPU_MODE_FIQ)];
> > +    }
> > +
> > +    env->pc = env->regs[15];
> > +}
> > +
> > +/* Function used to synchronize QEMU's AArch32 register set with AArch64
> > + * register set.  This is necessary when switching between AArch32 and
> AArch64
> > + * execution state.
> > + */
> > +void aarch64_sync_64_to_32(CPUARMState *env)
> > +{
> > +    int i;
> > +    uint32_t mode = env->uncached_cpsr & CPSR_M;
> > +
> > +    /* We can blanket copy X[0:7] to R[0:7] */
> > +    for (i = 0; i < 8; i++) {
> > +        env->regs[i] = env->xregs[i];
> > +    }
> > +
> > +    /* The destination of some registers depend on the current
> executing mode.
>
> "depends"
>

​Fixed in next version.​


>
> > +     * The AArch32 registers 8-12 are only sync'd when we are in USR or
> FIQ
> > +     * mode as they are the only modes where AArch64 registers map to
> these
> > +     * registers.  In all other cases, the respective USR and FIQ banks
> are
> > +     * sync'd.
> > +     * The AArch32 registers 13 & 14 are sync'd depending on the
> execution mode
> > +     * we are in.  If we are not in a given mode, the bank
> corresponding to the
> > +     * AArch64 register is sync'd.
> > +     */
> > +    if (mode == ARM_CPU_MODE_USR) {
> > +        for (i = 8; i < 15; i++) {
> > +            env->regs[i] = env->xregs[i];
> > +        }
>
> Something is wrong here, because we don't seem to be writing
> anything to env->regs[8..15] if mode is neither USR nor FIQ.
>
> ​I wrestled with this myself.  As I understand it, nothing maps to
regs[8..15] unless we are in USR or FIQ, which I covered.  This based on
the ARM ARM xregs[8:15] are defined to specifically map to USR,   Outside
of the these modes, what should be copied to regs[8..15]?
​


> I think you should rearrange this function. The 32->64 sync
> needs to write a value to all of xregs[0..30], and it's
> ordered such that we basically write them all in that order,
> which makes it clear we don't miss any cases.
>
> This function, on the other hand, needs to write to
> regs[0..15] and also to the banked_* registers, so
> it should be written to update them in that order,
> rather than in xregs order.
>
> You may also find it easier to always update the banked_*
> copies regardless of the current mode -- it's safe to
> write the value to them always, it just might be ignored
> if we're currently executing for that mode.
>
> -- PMM
>
Peter Maydell Feb. 11, 2015, 6:20 a.m. UTC | #3
On 11 February 2015 at 06:08, Greg Bellows <greg.bellows@linaro.org> wrote:
>
>
> On Tue, Feb 10, 2015 at 10:13 PM, Peter Maydell <peter.maydell@linaro.org>
> wrote:
>>
>> On 10 February 2015 at 10:50, Greg Bellows <greg.bellows@linaro.org>
>> wrote:
>> > +void aarch64_sync_64_to_32(CPUARMState *env)
>> > +{
>> > +    int i;
>> > +
>> > +    for (i = 0; i < 15; i++) {
>> > +        env->regs[i] = env->xregs[i];
>> > +    }
>> > +}
>>
>> This is inside CONFIG_USER_ONLY, right? Is it called at all?
>> If so, when?
>
>
> The exception_return helper calls the function so I had to either add a
> USER_CONFIG version of wrap the call in exception return with
> CONFIG_USER_ONLY.  I chose the former, but either would work.  As you would
> already know, the exception_return helper is likely not getting called in a
> USER_ONLY build.

Right, so make it just g_assert_not_reached().

>>
>> > +     * The AArch32 registers 8-12 are only sync'd when we are in USR or
>> > FIQ
>> > +     * mode as they are the only modes where AArch64 registers map to
>> > these
>> > +     * registers.  In all other cases, the respective USR and FIQ banks
>> > are
>> > +     * sync'd.
>> > +     * The AArch32 registers 13 & 14 are sync'd depending on the
>> > execution mode
>> > +     * we are in.  If we are not in a given mode, the bank
>> > corresponding to the
>> > +     * AArch64 register is sync'd.
>> > +     */
>> > +    if (mode == ARM_CPU_MODE_USR) {
>> > +        for (i = 8; i < 15; i++) {
>> > +            env->regs[i] = env->xregs[i];
>> > +        }
>>
>> Something is wrong here, because we don't seem to be writing
>> anything to env->regs[8..15] if mode is neither USR nor FIQ.
>>
> I wrestled with this myself.  As I understand it, nothing maps to
> regs[8..15] unless we are in USR or FIQ, which I covered.  This based on the
> ARM ARM xregs[8:15] are defined to specifically map to USR,   Outside of the
> these modes, what should be copied to regs[8..15]?

There is always *something* that is the architecturally defined
state for all the AArch32 registers. Otherwise a 64-bit hypervisor
would be unable to interrupt and restart a 32-bit guest.
(And all the registers r0..r15 exist in all AArch32 modes;
the question is just whether they're banked registers or
shared with some other mode).

All modes other than FIQ use the USR registers for r0..r12.
(See the v8 ARM ARM fig G1-3, and note the footnote about the
meaning of empty cells in the table.) r13 is the appropriate
sp for the mode (noting that system mode shares with usr),
and r14 ditto (but system and hyp share with usr).

-- PMM
diff mbox

Patch

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 1830a12..11845a6 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -495,6 +495,8 @@  typedef struct CPUARMState {
 ARMCPU *cpu_arm_init(const char *cpu_model);
 int cpu_arm_exec(CPUARMState *s);
 uint32_t do_arm_semihosting(CPUARMState *env);
+void aarch64_sync_32_to_64(CPUARMState *env);
+void aarch64_sync_64_to_32(CPUARMState *env);
 
 static inline bool is_a64(CPUARMState *env)
 {
diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index 8aa40e9..7e0d038 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -466,7 +466,6 @@  void aarch64_cpu_do_interrupt(CPUState *cs)
     unsigned int new_el = arm_excp_target_el(cs, cs->exception_index);
     target_ulong addr = env->cp15.vbar_el[new_el];
     unsigned int new_mode = aarch64_pstate_mode(new_el, true);
-    int i;
 
     if (arm_current_el(env) < new_el) {
         if (env->aarch64) {
@@ -530,9 +529,7 @@  void aarch64_cpu_do_interrupt(CPUState *cs)
         }
         env->elr_el[new_el] = env->regs[15];
 
-        for (i = 0; i < 15; i++) {
-            env->xregs[i] = env->regs[i];
-        }
+        aarch64_sync_32_to_64(env);
 
         env->condexec_bits = 0;
     }
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 1a1a005..c1d6764 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -4096,6 +4096,15 @@  unsigned int arm_excp_target_el(CPUState *cs, unsigned int excp_idx)
     return 1;
 }
 
+void aarch64_sync_64_to_32(CPUARMState *env)
+{
+    int i;
+
+    for (i = 0; i < 15; i++) {
+        env->regs[i] = env->xregs[i];
+    }
+}
+
 #else
 
 /* Map CPU modes onto saved register banks.  */
@@ -4425,6 +4434,178 @@  void arm_v7m_cpu_do_interrupt(CPUState *cs)
     env->thumb = addr & 1;
 }
 
+/* Function used to synchronize QEMU's AArch64 register set with AArch32
+ * register set.  This is necessary when switching between AArch32 and AArch64
+ * execution state.
+ */
+void aarch64_sync_32_to_64(CPUARMState *env)
+{
+    int i;
+    uint32_t mode = env->uncached_cpsr & CPSR_M;
+
+    /* We can blanket copy R[0:7] to X[0:7] */
+    for (i = 0; i < 8; i++) {
+        env->xregs[i] = env->regs[i];
+    }
+
+    /* The latest copy of some registers depend on the current executing mode.
+     * The general purpose copy is used when the current mode corresponds to
+     * the mapping for the given register.  Otherwise, the banked copy
+     * corresponding to the register mapping is used.
+     */
+    if (mode == ARM_CPU_MODE_USR) {
+        for (i = 8; i < 15; i++) {
+            env->xregs[i] = env->regs[i];
+        }
+    } else {
+        for (i = 8; i < 13; i++) {
+            env->xregs[i] = env->usr_regs[i - 8];
+        }
+        env->xregs[13] = env->banked_r13[bank_number(ARM_CPU_MODE_USR)];
+        env->xregs[14] = env->banked_r14[bank_number(ARM_CPU_MODE_USR)];
+    }
+
+    if (mode == ARM_CPU_MODE_HYP) {
+        env->xregs[15] = env->regs[13];
+    } else {
+        env->xregs[15] = env->banked_r13[bank_number(ARM_CPU_MODE_HYP)];
+    }
+
+    if (mode == ARM_CPU_MODE_IRQ) {
+        env->xregs[16] = env->regs[13];
+        env->xregs[17] = env->regs[14];
+    } else {
+        env->xregs[16] = env->banked_r13[bank_number(ARM_CPU_MODE_IRQ)];
+        env->xregs[17] = env->banked_r14[bank_number(ARM_CPU_MODE_IRQ)];
+    }
+
+    if (mode == ARM_CPU_MODE_SVC) {
+        env->xregs[18] = env->regs[13];
+        env->xregs[19] = env->regs[14];
+    } else {
+        env->xregs[18] = env->banked_r13[bank_number(ARM_CPU_MODE_SVC)];
+        env->xregs[19] = env->banked_r14[bank_number(ARM_CPU_MODE_SVC)];
+    }
+
+    if (mode == ARM_CPU_MODE_ABT) {
+        env->xregs[20] = env->regs[13];
+        env->xregs[21] = env->regs[14];
+    } else {
+        env->xregs[20] = env->banked_r13[bank_number(ARM_CPU_MODE_ABT)];
+        env->xregs[21] = env->banked_r14[bank_number(ARM_CPU_MODE_ABT)];
+    }
+
+    if (mode == ARM_CPU_MODE_UND) {
+        env->xregs[22] = env->regs[13];
+        env->xregs[23] = env->regs[14];
+    } else {
+        env->xregs[22] = env->banked_r13[bank_number(ARM_CPU_MODE_UND)];
+        env->xregs[23] = env->banked_r14[bank_number(ARM_CPU_MODE_UND)];
+    }
+
+    if (mode == ARM_CPU_MODE_FIQ) {
+        for (i = 24; i < 31; i++) {
+            env->xregs[i] = env->regs[i - 16];   /* X[24:30] -> R[8:14] */
+        }
+    } else {
+        for (i = 24; i < 29; i++) {
+            env->xregs[i] = env->fiq_regs[i - 24];
+        }
+        env->xregs[29] = env->banked_r13[bank_number(ARM_CPU_MODE_FIQ)];
+        env->xregs[30] = env->banked_r14[bank_number(ARM_CPU_MODE_FIQ)];
+    }
+
+    env->pc = env->regs[15];
+}
+
+/* Function used to synchronize QEMU's AArch32 register set with AArch64
+ * register set.  This is necessary when switching between AArch32 and AArch64
+ * execution state.
+ */
+void aarch64_sync_64_to_32(CPUARMState *env)
+{
+    int i;
+    uint32_t mode = env->uncached_cpsr & CPSR_M;
+
+    /* We can blanket copy X[0:7] to R[0:7] */
+    for (i = 0; i < 8; i++) {
+        env->regs[i] = env->xregs[i];
+    }
+
+    /* The destination of some registers depend on the current executing mode.
+     * The AArch32 registers 8-12 are only sync'd when we are in USR or FIQ
+     * mode as they are the only modes where AArch64 registers map to these
+     * registers.  In all other cases, the respective USR and FIQ banks are
+     * sync'd.
+     * The AArch32 registers 13 & 14 are sync'd depending on the execution mode
+     * we are in.  If we are not in a given mode, the bank corresponding to the
+     * AArch64 register is sync'd.
+     */
+    if (mode == ARM_CPU_MODE_USR) {
+        for (i = 8; i < 15; i++) {
+            env->regs[i] = env->xregs[i];
+        }
+    } else {
+        for (i = 8; i < 13; i++) {
+            env->usr_regs[i - 8] = env->xregs[i];
+        }
+        env->banked_r13[bank_number(ARM_CPU_MODE_USR)] = env->xregs[13];
+        env->banked_r14[bank_number(ARM_CPU_MODE_USR)] = env->xregs[14];
+    }
+
+    if (mode == ARM_CPU_MODE_HYP) {
+        env->regs[13] = env->xregs[15];
+    } else {
+        env->banked_r13[bank_number(ARM_CPU_MODE_HYP)] = env->xregs[15];
+    }
+
+    if (mode == ARM_CPU_MODE_IRQ) {
+        env->regs[13] = env->xregs[16];
+        env->regs[14] = env->xregs[17];
+    } else {
+        env->banked_r13[bank_number(ARM_CPU_MODE_IRQ)] = env->xregs[16];
+        env->banked_r14[bank_number(ARM_CPU_MODE_IRQ)] = env->xregs[17];
+    }
+
+    if (mode == ARM_CPU_MODE_SVC) {
+        env->regs[13] = env->xregs[18];
+        env->regs[14] = env->xregs[19];
+    } else {
+        env->banked_r13[bank_number(ARM_CPU_MODE_SVC)] = env->xregs[18];
+        env->banked_r14[bank_number(ARM_CPU_MODE_SVC)] = env->xregs[19];
+    }
+
+    if (mode == ARM_CPU_MODE_ABT) {
+        env->regs[13] = env->xregs[20];
+        env->regs[14] = env->xregs[21];
+    } else {
+        env->banked_r13[bank_number(ARM_CPU_MODE_ABT)] = env->xregs[20];
+        env->banked_r14[bank_number(ARM_CPU_MODE_ABT)] = env->xregs[21];
+    }
+
+    if (mode == ARM_CPU_MODE_UND) {
+        env->regs[13] = env->xregs[22];
+        env->regs[14] = env->xregs[23];
+    } else {
+        env->banked_r13[bank_number(ARM_CPU_MODE_UND)] = env->xregs[22];
+        env->banked_r14[bank_number(ARM_CPU_MODE_UND)] = env->xregs[23];
+    }
+
+    if (mode == ARM_CPU_MODE_FIQ) {
+        for (i = 8; i < 15; i++) {
+            env->regs[i] = env->xregs[i + 16];   /* X[24:30] -> R[8:14] */
+        }
+    } else {
+        for (i = 0; i < 5; i++) {
+            env->fiq_regs[i] = env->xregs[i + 24];
+        }
+        env->banked_r13[bank_number(ARM_CPU_MODE_FIQ)] = env->xregs[29];
+        env->banked_r14[bank_number(ARM_CPU_MODE_FIQ)] = env->xregs[30];
+    }
+
+    env->regs[15] = env->pc;
+}
+
 /* Handle a CPU exception.  */
 void arm_cpu_do_interrupt(CPUState *cs)
 {
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 = arm_current_el(env);
     unsigned int spsr_idx = aarch64_banked_spsr_index(cur_el);
     uint32_t spsr = 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 &= ~PSTATE_SS;
         }
-        for (i = 0; i < 15; i++) {
-            env->regs[i] = env->xregs[i];
-        }
+        aarch64_sync_64_to_32(env);
 
         env->regs[15] = env->elr_el[1] & ~0x1;
     } else {