diff mbox series

[1/2] target/arm: SCR_EL3 bits 4,5 are always res0

Message ID 20220605161056.293920-2-richard.henderson@linaro.org
State New
Headers show
Series target/arm: SCR_EL3 RES0, RAO/WI tweaks | expand

Commit Message

Richard Henderson June 5, 2022, 4:10 p.m. UTC
These bits do not depend on whether or not el1 supports aa32.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/helper.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

Comments

Peter Maydell June 9, 2022, 3:08 p.m. UTC | #1
On Sun, 5 Jun 2022 at 17:14, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> These bits do not depend on whether or not el1 supports aa32.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Isn't this effectively reverting commit 10d0ef3e6cfe2, which
got applied as a bug fix last year ?

In particular, the reason we need to check something is that even
if the CPU is entirely AArch32-only, reset of the register is
handled by scr_reset() on the AArch64 reginfo struct, so on reset
we need to give the correct answer for the CPU and not assume
"regdef is AA64" implies "EL3 is AA64".

We should probably be checking "is EL3 AArch64 or AArch32" rather
than "does EL1 support AArch32", though...

There's a testcase in the original patch cover letter for the
bug it's trying to fix:
https://lore.kernel.org/qemu-devel/20210203165552.16306-1-michael.nawrocki@gtri.gatech.edu/

thanks
-- PMM
diff mbox series

Patch

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 40da63913c..c262b00c3c 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -1752,11 +1752,8 @@  static void scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
     ARMCPU *cpu = env_archcpu(env);
 
     if (ri->state == ARM_CP_STATE_AA64) {
-        if (arm_feature(env, ARM_FEATURE_AARCH64) &&
-            !cpu_isar_feature(aa64_aa32_el1, cpu)) {
-                value |= SCR_FW | SCR_AW;   /* these two bits are RES1.  */
-        }
-        valid_mask &= ~SCR_NET;
+        value |= SCR_FW | SCR_AW;      /* RES1 */
+        valid_mask &= ~SCR_NET;        /* RES0 */
 
         if (cpu_isar_feature(aa64_ras, cpu)) {
             valid_mask |= SCR_TERR;