| Message ID | 1427130344-27986-2-git-send-email-alex.bennee@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On Mon, Mar 23, 2015 at 12:05 PM, Alex Bennée <alex.bennee@linaro.org> wrote: > From: Peter Maydell <peter.maydell@linaro.org> > > The AArch64 SPSR_EL1 register is architecturally mandated to > be mapped to the AArch32 SPSR_svc register. This means its > state should live in QEMU's env->banked_spsr[1] field. > Correct the various places in the code that incorrectly > put it in banked_spsr[0]. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > > diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c > index 7e0d038..861f6fa 100644 > --- a/target-arm/helper-a64.c > +++ b/target-arm/helper-a64.c > @@ -523,7 +523,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs) > aarch64_save_sp(env, arm_current_el(env)); > env->elr_el[new_el] = env->pc; > } else { > - env->banked_spsr[0] = cpsr_read(env); > + env->banked_spsr[aarch64_banked_spsr_index(new_el)] = cpsr_read(env); Are the other banks (2-5) only used for KVM? It seems we go out of our way to manage this larger SPSR array then not use all of the slots in QEMU itself. > if (!env->thumb) { > env->cp15.esr_el[new_el] |= 1 << 25; > } > diff --git a/target-arm/helper.c b/target-arm/helper.c > index 10886c5..d77c6de 100644 > --- a/target-arm/helper.c > +++ b/target-arm/helper.c > @@ -2438,7 +2438,7 @@ static const ARMCPRegInfo v8_cp_reginfo[] = { > { .name = "SPSR_EL1", .state = ARM_CP_STATE_AA64, > .type = ARM_CP_ALIAS, > .opc0 = 3, .opc1 = 0, .crn = 4, .crm = 0, .opc2 = 0, > - .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[0]) }, > + .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[1]) }, > /* We rely on the access checks not allowing the guest to write to the > * state field when SPSel indicates that it's being used as the stack > * pointer. > diff --git a/target-arm/internals.h b/target-arm/internals.h > index bb171a7..2cc3017 100644 > --- a/target-arm/internals.h > +++ b/target-arm/internals.h > @@ -82,11 +82,14 @@ static inline void arm_log_exception(int idx) > > /* > * For AArch64, map a given EL to an index in the banked_spsr array. > + * Note that this mapping and the AArch32 mapping defined in bank_number() > + * must agree such that the AArch64<->AArch32 SPSRs have the architecturally > + * mandated mapping between each other. > */ > static inline unsigned int aarch64_banked_spsr_index(unsigned int el) > { > static const unsigned int map[4] = { > - [1] = 0, /* EL1. */ > + [1] = 1, /* EL1. */ > [2] = 6, /* EL2. */ > [3] = 7, /* EL3. */ > }; > -- > 2.3.2 > >
On 24 March 2015 at 14:32, Greg Bellows <greg.bellows@linaro.org> wrote: > On Mon, Mar 23, 2015 at 12:05 PM, Alex Bennée <alex.bennee@linaro.org> wrote: >> From: Peter Maydell <peter.maydell@linaro.org> >> @@ -523,7 +523,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs) >> aarch64_save_sp(env, arm_current_el(env)); >> env->elr_el[new_el] = env->pc; >> } else { >> - env->banked_spsr[0] = cpsr_read(env); >> + env->banked_spsr[aarch64_banked_spsr_index(new_el)] = cpsr_read(env); > > Are the other banks (2-5) only used for KVM? It seems we go out of > our way to manage this larger SPSR array then not use all of the slots > in QEMU itself. They're used in AArch32 (where they are the SPSR for various 32 bit modes). In AArch64 you can access those registers via MSR/MRS (we probably haven't implemented those yet because they are only accessible at EL2 and above) so hypervisors can do worldswitches. But for exception entry and return (which is what this code is) we only use SPSR_EL0/SPSR_EL1/SPSR_EL2/SPSR_EL3 which is a subset of the AArch32 SPSRs. -- PMM
diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c index 7e0d038..861f6fa 100644 --- a/target-arm/helper-a64.c +++ b/target-arm/helper-a64.c @@ -523,7 +523,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs) aarch64_save_sp(env, arm_current_el(env)); env->elr_el[new_el] = env->pc; } else { - env->banked_spsr[0] = cpsr_read(env); + env->banked_spsr[aarch64_banked_spsr_index(new_el)] = cpsr_read(env); if (!env->thumb) { env->cp15.esr_el[new_el] |= 1 << 25; } diff --git a/target-arm/helper.c b/target-arm/helper.c index 10886c5..d77c6de 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -2438,7 +2438,7 @@ static const ARMCPRegInfo v8_cp_reginfo[] = { { .name = "SPSR_EL1", .state = ARM_CP_STATE_AA64, .type = ARM_CP_ALIAS, .opc0 = 3, .opc1 = 0, .crn = 4, .crm = 0, .opc2 = 0, - .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[0]) }, + .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[1]) }, /* We rely on the access checks not allowing the guest to write to the * state field when SPSel indicates that it's being used as the stack * pointer. diff --git a/target-arm/internals.h b/target-arm/internals.h index bb171a7..2cc3017 100644 --- a/target-arm/internals.h +++ b/target-arm/internals.h @@ -82,11 +82,14 @@ static inline void arm_log_exception(int idx) /* * For AArch64, map a given EL to an index in the banked_spsr array. + * Note that this mapping and the AArch32 mapping defined in bank_number() + * must agree such that the AArch64<->AArch32 SPSRs have the architecturally + * mandated mapping between each other. */ static inline unsigned int aarch64_banked_spsr_index(unsigned int el) { static const unsigned int map[4] = { - [1] = 0, /* EL1. */ + [1] = 1, /* EL1. */ [2] = 6, /* EL2. */ [3] = 7, /* EL3. */ };