Re: [Qemu-devel] [PATCH v3 14/32] target-arm: Respect SCR.FW, SCR.AW and SCTLR.NMFI

[Aggeler Fabian]

On 13 Jun 2014, at 00:43, Greg Bellows <address@hidden<mailto:address@hidden>> 
wrote:




On 10 June 2014 18:54, Fabian Aggeler <address@hidden<mailto:address@hidden>> 
wrote:
bits when modifying CPSR.

Signed-off-by: Fabian Aggeler <address@hidden<mailto:address@hidden>>
---
 target-arm/helper.c | 42 +++++++++++++++++++++++++++++++++++++++---
 1 file changed, 39 insertions(+), 3 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 2fbecfa..f6ff4aa 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3091,9 +3091,6 @@ void cpsr_write(CPUARMState *env, uint32_t val, uint32_t 
mask)
         env->GE = (val >> 16) & 0xf;
     }

-    env->daif &= ~(CPSR_AIF & mask);
-    env->daif |= val & CPSR_AIF & mask;
-
     if ((env->uncached_cpsr ^ val) & mask & CPSR_M) {
         if (bad_mode_switch(env, val & CPSR_M)) {
             /* Attempt to switch to an invalid mode: this is UNPREDICTABLE.
@@ -3105,6 +3102,45 @@ void cpsr_write(CPUARMState *env, uint32_t val, uint32_t 
mask)
             switch_mode(env, val & CPSR_M);
         }
     }
+
+    /* In an implementation that does not include Virtualization Extensions
+     * the SCR.FW and SCR.AW<http://scr.aw/> bit control whether non-secure 
software is allowed
+     * to change the CPSR_F and CPSR_A bits respectively.
+     */
+    if ((mask & CPSR_A)
+            && (val & CPSR_A) != (env->uncached_cpsr & CPSR_A)
+            && arm_feature(env, ARM_FEATURE_EL3)
+            && !(env->cp15.scr_el3 & SCR_AW) && !arm_is_secure(env)) {
+        qemu_log_mask(LOG_GUEST_ERROR, "Ignoring attempt to switch CPSR_A "
+                "flag from non-secure world with SCR.AW<http://scr.aw/> bit 
set\n");
+        mask &= ~CPSR_A;

Ignoring the write appears to only be valid in the case of ARMv7 (actually says 
"early implementations of v7") but not v8.  This is noted in the ARMv8 spec in 
the section "Effects of EL3 and EL2 on the CPSR.{A, F} bits".  According to 
this passage, the AW/FW bits only block the corresponding bits being updated 
when the SPSR is copied to the CPSR.  It sounds like they are distinguishing 
between this copy and a raw write.

Yes, I missed that bit. Adding a !arm_feature(env, ARM_FEATURE_V8) condition 
should help
in that case. And then do the check as well when copying SPSR to CPSR.


+    }
+
+    if ((mask & CPSR_F)) {
+        /* Pre ARMv8: Check whether non-maskable FIQ (NMFI) support is enabled.
+         * If this bit is set software is not allowed to mask FIQs,
+         * but is allowed to set CPSR_F to 0.
+         */
+        if (!arm_feature(env, ARM_FEATURE_V8) &&
+            (A32_BANKED_CURRENT_REG_GET(env, sctlr) & SCTLR_NMFI) &&
+            (val & CPSR_F)) {
+            qemu_log_mask(LOG_GUEST_ERROR, "Ignoring attempt to enable CPSR_F "
+                    "flag (non-maskable FIQ [NMFI] support enabled)\n");
+            mask &= ~CPSR_F;
+        }
+
+        if ((val & CPSR_F) != (env->uncached_cpsr & CPSR_F)
+                && arm_feature(env, ARM_FEATURE_EL3)
+                && !(env->cp15.scr_el3 & SCR_FW) && !arm_is_secure(env)) {
+            qemu_log_mask(LOG_GUEST_ERROR, "Ignoring attempt to switch CPSR_F "
+                    "flag from non-secure world with SCR.FW bit set\n");
+            mask &= ~CPSR_F;
+        }
+    }
+
+    env->daif &= ~(CPSR_AIF & mask);
+    env->daif |= val & CPSR_AIF & mask;
+
     mask &= ~CACHED_CPSR_BITS;
     env->uncached_cpsr = (env->uncached_cpsr & ~mask) | (val & mask);
 }
--
1.8.3.2