diff mbox

target-arm: Fix incorrect setting of E bit in CPSR

Message ID 1393594802-27281-1-git-send-email-peter.maydell@linaro.org
State Superseded
Headers show

Commit Message

Peter Maydell Feb. 28, 2014, 1:40 p.m. UTC
Commit 4cc35614a moved the exception mask bits out of env->uncached_cpsr
and into env->daif. However the env->daif contents are AArch64 style
mask bits, which include not just the AArch32 AIF bits but also the
new D bit (masks debug exceptions). This means that when reconstructing
the AArch32 CPSR value we must not allow the D bit in env->daif to get
into the CPSR, because the corresponding bit in the CPSR is E, the
endianness bit.

This bug didn't affect execution under TCG because we don't implement
endianness-swapping and so simply ignored the E bit; however it meant
that kernel booting under KVM failed, because KVM does honour the E bit.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Whoops... I clearly need to put "check KVM still works" into my
regular testing process for patches.

 target-arm/helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Peter Maydell Feb. 28, 2014, 2:51 p.m. UTC | #1
On 28 February 2014 13:40, Peter Maydell <peter.maydell@linaro.org> wrote:
> Commit 4cc35614a moved the exception mask bits out of env->uncached_cpsr
> and into env->daif. However the env->daif contents are AArch64 style
> mask bits, which include not just the AArch32 AIF bits but also the
> new D bit (masks debug exceptions). This means that when reconstructing
> the AArch32 CPSR value we must not allow the D bit in env->daif to get
> into the CPSR, because the corresponding bit in the CPSR is E, the
> endianness bit.
>
> This bug didn't affect execution under TCG because we don't implement
> endianness-swapping and so simply ignored the E bit; however it meant
> that kernel booting under KVM failed, because KVM does honour the E bit.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

This was reported on IRC so I didn't have the reporter's email
at the time I posted this. Here it is:

Reported-by: Alexey Ignatov <lexszero@gmail.com>

Thanks, and apologies for the breakage.

> ---
> Whoops... I clearly need to put "check KVM still works" into my
> regular testing process for patches.
>
>  target-arm/helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 90f85f1..d44e603 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -2478,7 +2478,7 @@ uint32_t cpsr_read(CPUARMState *env)
>          (env->CF << 29) | ((env->VF & 0x80000000) >> 3) | (env->QF << 27)
>          | (env->thumb << 5) | ((env->condexec_bits & 3) << 25)
>          | ((env->condexec_bits & 0xfc) << 8)
> -        | (env->GE << 16) | env->daif;
> +        | (env->GE << 16) | (env->daif & CPSR_AIF);
>  }
>
>  void cpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)
> --
> 1.9.0
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
diff mbox

Patch

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 90f85f1..d44e603 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2478,7 +2478,7 @@  uint32_t cpsr_read(CPUARMState *env)
         (env->CF << 29) | ((env->VF & 0x80000000) >> 3) | (env->QF << 27)
         | (env->thumb << 5) | ((env->condexec_bits & 3) << 25)
         | ((env->condexec_bits & 0xfc) << 8)
-        | (env->GE << 16) | env->daif;
+        | (env->GE << 16) | (env->daif & CPSR_AIF);
 }
 
 void cpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)