[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 3/4] target-arm: Add 32/64-bit register sync
|
From: |
Peter Maydell |
|
Subject: |
Re: [Qemu-devel] [PATCH v6 3/4] target-arm: Add 32/64-bit register sync |
|
Date: |
Thu, 12 Feb 2015 06:01:32 +0000 |
On 12 February 2015 at 05:53, 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>
>
> ---
>
> v5 -> v6
> - Add r14 set in 32_to_64
> - Reorder conditionals in 64_to_32 fomr reg order to xreg order
>
> 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 | 209
> ++++++++++++++++++++++++++++++++++++++++++++++++
> target-arm/op_helper.c | 6 +-
> 4 files changed, 214 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..4628525 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,210 @@ 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 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. Registers
> + * r13 and r14 are only copied if we are in that mode, otherwise we copy
> + * from the mode banked register.
> + */
> + 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)];
> + /* HYP is an exception in that it is copied from r14 */
> + if (mode == ARM_CPU_MODE_HYP) {
> + env->xregs[14] = env->regs[14];
> + } else {
> + 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)];
> + }
> +
> + /* Registers x24-x30 are mapped to r8-r14 in FIQ mode. If we are in FIQ
> + * mode, then we can copy from r8-r14. Otherwise, we copy from the
> + * FIQ bank 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 user registers x8-x12.
> + * Otherwise, they come from the FIQ mapped x24-x30..
> + */
This comment seems like it was from an earlier version? At any rate
it doesn't match the code.
> + if (mode == ARM_CPU_MODE_FIQ) {
> + for (i = 8; i < 13; i++) {
> + env->usr_regs[i - 8] = env->xregs[i];
> + }
> + } 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.
> + * Otherwise, we copy x13 and x14 into banked storage corresponding to
> the
> + * mode.
> + */
This comment seems a bit off too.
> + if (mode == ARM_CPU_MODE_USR || mode == ARM_CPU_MODE_SYS) {
> + env->regs[13] = env->xregs[13];
> + env->regs[14] = env->xregs[14];
> + } else {
> + env->banked_r13[bank_number(ARM_CPU_MODE_USR)] = env->xregs[13];
> +
> + /* HYP is an exception in that it is copied from r14 */
You mean "in that it does not have its own banked r14 but shares
the USR r14".
> + if (mode == ARM_CPU_MODE_HYP) {
> + env->regs[14] = env->xregs[14];
> + } else {
> + 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 {
> + /* 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];
> + }
> +
> + /* Registers x24-x30 are mapped to r8-r14 in FIQ mode. If we are in FIQ
> + * mode, then we can copy from r8-r14. Otherwise, we copy from the
> + * FIQ bank for r8-r14.
> + */
Cut-n-paste from the other function? Here we are copying *to*, not from...
> + if (mode == ARM_CPU_MODE_FIQ) {
> + for (i = 24; i < 31; i++) {
> + env->regs[i - 16] = env->xregs[i]; /* X[24:30] -> R[8:14] */
> + }
> + } else {
> + for (i = 24; i < 29; i++) {
> + env->fiq_regs[i - 24] = env->xregs[i];
> + }
> + 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
>
Comment nits aside,
Reviewed-by: Peter Maydell <address@hidden>
thanks
-- PMM