[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 3/4] target-arm: Add 32/64-bit register sync
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH v5 3/4] target-arm: Add 32/64-bit register sync |
Date: |
Thu, 12 Feb 2015 03:56:43 +0000 |
On 12 February 2015 at 02:59, Greg Bellows <address@hidden> 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 <address@hidden>
>
> ---
>
> v4 -> v5
> - Rework sync routines a bit more.
>
> 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 | 198
> ++++++++++++++++++++++++++++++++++++++++++++++++
> target-arm/op_helper.c | 6 +-
> 4 files changed, 203 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..2b7c28e 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -4096,6 +4096,11 @@ unsigned int arm_excp_target_el(CPUState *cs, unsigned
> int excp_idx)
> return 1;
> }
>
> +void aarch64_sync_64_to_32(CPUARMState *env)
> +{
> + g_assert_not_reached();
> +}
> +
> #else
>
> /* Map CPU modes onto saved register banks. */
> @@ -4425,6 +4430,199 @@ 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];
> + }
> +
> + /* Unless we are in FIQ mode, x8-x12 come from the active user registers
> + * r8-r12. Otherwise, they come from the banked user regs.
> + */
> + if (mode == ARM_CPU_MODE_FIQ) {
> + for (i = 8; i < 13; i++) {
> + env->xregs[i] = env->usr_regs[i - 8];
> + }
> + } else {
> + for (i = 8; i < 13; i++) {
> + env->xregs[i] = env->regs[i];
> + }
> + }
> +
> + /* Registers x13-x23 are the various mode SP and FP registers. The
> active
> + * r13 and r14 are only copied if we are in that mode, otherwise we copy
> + * from the mode banked copy.
> + */
> + if (mode == ARM_CPU_MODE_USR || mode == ARM_CPU_MODE_SYS) {
> + env->xregs[13] = env->regs[13];
> + env->xregs[14] = env->regs[14];
> + } else {
> + env->xregs[13] = env->banked_r13[bank_number(ARM_CPU_MODE_USR)];
> + env->xregs[14] = env->banked_r14[bank_number(ARM_CPU_MODE_USR)];
> + }
Hyp mode uses the USR mode r14, so if we're in Hyp you need to copy
regs[14] into xregs[14]. (But not r13, which has a separate banked
Hyp version, so xregs[13] and [14] need their own if () { .. } else { .. },
and can't share the same one.)
Otherwise this function is correct now.
> +
> + 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)];
> + }
> +
> + /* Registers x24-x30 are mapped to r8-r14 in FIQ mode. If we are in FIQ
> + * mode, then we can copy the active r8-r14. Otherwise, we copy from the
> + * FIQ banks for r8-r14.
> + */
> + 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];
> + }
> +
> + /* Unless we are in FIQ mode, r8-r12 come from the active user registers
"active" here should be deleted...
> + * x8-x12. Otherwise, they come from the FIQ mapped x24-x30..
> + */
> + if (mode == ARM_CPU_MODE_FIQ) {
> + for (i = 8; i < 13; i++) {
> + env->regs[i] = env->xregs[i + 16]; /* X[24:30] -> R[8:12] */
> + }
> + } else {
> + for (i = 8; i < 13; i++) {
> + env->regs[i] = env->xregs[i];
> + }
> + }
> +
> + /* Registers r13 & r14 are dependent on the mode we are in. For the
> + * current mode we actually copy from the respective x13 & x14 registers.
This comment doesn't seem to make sense.
> + * Otherwise, we copy x13 and x14 into banked storage corresponding to
> the
> + * mode.
> + */
> + if (mode == ARM_CPU_MODE_USR || mode == ARM_CPU_MODE_SYS) {
> + env->regs[13] = env->xregs[13];
> + env->regs[14] = env->xregs[14];
> + } else {
> + for (i = 0; i < 5; i++) {
> + env->usr_regs[i] = env->xregs[i + 8];
> + }
> + env->banked_r13[bank_number(ARM_CPU_MODE_USR)] = env->xregs[13];
> + env->banked_r14[bank_number(ARM_CPU_MODE_USR)] = env->xregs[14];
> + }
I think a clearer way to structure this function is going to be
to have it basically proceed through the xregs[] in order.
Every xreg[] from 0 to 30 should get copied to exactly one
place (in regs, or a banked reg, or whatever). Then it's
clear that we've handled every register we need to sync and
haven't left out any cases.
> +
> + if (mode == ARM_CPU_MODE_HYP) {
> + env->regs[13] = env->xregs[15];
> + env->regs[14] = env->xregs[14];
> + } else {
> + /* Only x15 maps to something in HYP mode, specifically r13 */
> + 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) {
> + env->regs[13] = env->xregs[29];
> + env->regs[14] = env->xregs[30];
> + } 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 {
> --
> 1.8.3.2
>
thanks
-- PMM