Re: [Qemu-devel] [Qemu-arm] [PATCH 1/2] target/arm: Hyp mode R14 is shared with User and System

[Peter Maydell <peter.maydell@linaro.org>]

On 9 November 2018 at 17:35, Peter Maydell <peter.maydell@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.maydell@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).

thanks
-- PMM