qemu-arm
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Alex Bennée
Subject: Re: [Qemu-arm] [PATCH 1/2] target/arm: Hyp mode R14 is shared with User and System
Date: Mon, 12 Nov 2018 11:41:56 +0000
User-agent: mu4e 1.1.0; emacs 26.1.50

Peter Maydell <address@hidden> writes:

> On 9 November 2018 at 17:35, Peter Maydell <address@hidden> 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 <address@hidden>
>> ---
>>  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 <address@hidden>

--
Alex Bennée



reply via email to

[Prev in Thread] Current Thread [Next in Thread]