| Message ID | 20220605161056.293920-2-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/arm: SCR_EL3 RES0, RAO/WI tweaks | expand |
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 --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;
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(-)