Peter Maydell <peter.mayd...@linaro.org> writes:
> On 9 November 2018 at 17:35, Peter Maydell <peter.mayd...@linaro.org> wrote:
>> Hyp mode is an exception to the general rule that each AArch32
>> mode has its own r13, r14 and SPSR -- it has a banked r13 and
>> SPSR but shares its r14 with User and System mode. We were
>> incorrectly implementing it as banked, which meant that on
>> entry to Hyp mode r14 was 0 rather than the USR/SYS r14.
>>
>> We provide a new function r14_bank_number() which is like
>> the existing bank_number() but provides the index into
>> env->banked_r14[]; bank_number() provides the index to use
>> for env->banked_r13[] and env->banked_cpsr[].
>>
>> All the points in the code that were using bank_number()
>> to index into env->banked_r14[] are updated for consintency:
>> * switch_mode() -- this is the only place where we fix
>> an actual bug
>> * aarch64_sync_32_to_64() and aarch64_sync_64_to_32():
>> no behavioural change as we already special-cased Hyp R14
>> * kvm32.c: no behavioural change since the guest can't ever
>> be in Hyp mode, but conceptually the right thing to do
>> * msr_banked()/mrs_banked(): we can never get to the case
>> that accesses banked_r14[] with tgtmode == ARM_CPU_MODE_HYP,
>> so no behavioural change
>>
>> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
>> ---
>> target/arm/internals.h | 16 ++++++++++++++++
>> target/arm/helper.c | 29 +++++++++++++++--------------
>> target/arm/kvm32.c | 4 ++--
>> target/arm/op_helper.c | 2 +-
>> 4 files changed, 34 insertions(+), 17 deletions(-)
>
> Rats, this bit accidentally didn't make it into this patch:
>
> diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
> index 2b62c53f5b5..eb6fb82fb81 100644
> --- a/target/arm/op_helper.c
> +++ b/target/arm/op_helper.c
> @@ -725,7 +725,7 @@ uint32_t HELPER(mrs_banked)(CPUARMState *env,
> uint32_t tgtmode, uint32_t regno)
> case 13:
> return env->banked_r13[bank_number(tgtmode)];
> case 14:
> - return env->banked_r14[bank_number(tgtmode)];
> + return env->banked_r14[r14_bank_number(tgtmode)];
> case 8 ... 12:
> switch (tgtmode) {
> case ARM_CPU_MODE_USR:
>
>
> (it's one of the "no behavioural change" bits).
<snip>
With the fix:
Reviewed-by: Alex Bennée <alex.ben...@linaro.org>
--
Alex Bennée
