[PATCH v2 0/2] target/arm: SCR_EL3 RES0, RAO/WI tweaks

[PATCH v2 0/2] target/arm: SCR_EL3 RES0, RAO/WI tweaks

[Richard Henderson]

Adjust RW, fixing #1062, and adjusting bits [4:2].

Changes for v2:
  * Fix patch 1 vs reset.


r~

Richard Henderson (2):
  target/arm: Adjust format test in scr_write
  target/arm: SCR_EL3.RW is RAO/WI without AArch32 EL[12]

 target/arm/cpu.h    |  5 +++++
 target/arm/helper.c | 18 ++++++++++++------
 2 files changed, 17 insertions(+), 6 deletions(-)

-- 
2.34.1

[PATCH v2 1/2] target/arm: Adjust format test in scr_write

[Richard Henderson]

Because reset always initializes the AA64 version, SCR_EL3,
test the mode of EL3 instead of the type of the cpreg.

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

diff --git a/target/arm/helper.c b/target/arm/helper.c
index ac9942d750..d6818f281a 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -1738,12 +1738,14 @@ static void scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
     uint32_t valid_mask = 0x3fff;
     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;
+    /*
+     * Because SCR_EL3 is the "real" cpreg and SCR is the alias, reset always
+     * passes the reginfo for SCR_EL3, which has type ARM_CP_STATE_AA64.
+     * Instead, choose the format based on the mode of EL3.
+     */
+    if (arm_el_is_aa64(env, 3)) {
+        value |= SCR_FW | SCR_AW;      /* RES1 */
+        valid_mask &= ~SCR_NET;        /* RES0 */
 
         if (cpu_isar_feature(aa64_ras, cpu)) {
             valid_mask |= SCR_TERR;
-- 
2.34.1

[PATCH v2 2/2] target/arm: SCR_EL3.RW is RAO/WI without AArch32 EL[12]

[Richard Henderson]

Since DDI0487F.a, the RW bit is RAO/WI.  When specifically
targeting such a cpu, e.g. cortex-a76, it is legitimate to
ignore the bit within the secure monitor.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1062
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/cpu.h    | 5 +++++
 target/arm/helper.c | 4 ++++
 2 files changed, 9 insertions(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 78dbcb5592..c489519923 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -3978,6 +3978,11 @@ static inline bool isar_feature_aa64_aa32_el1(const ARMISARegisters *id)
     return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, EL1) >= 2;
 }
 
+static inline bool isar_feature_aa64_aa32_el2(const ARMISARegisters *id)
+{
+    return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, EL2) >= 2;
+}
+
 static inline bool isar_feature_aa64_ras(const ARMISARegisters *id)
 {
     return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, RAS) != 0;
diff --git a/target/arm/helper.c b/target/arm/helper.c
index d6818f281a..0d1670f89b 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -1747,6 +1747,10 @@ static void scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
         value |= SCR_FW | SCR_AW;      /* RES1 */
         valid_mask &= ~SCR_NET;        /* RES0 */
 
+        if (!cpu_isar_feature(aa64_aa32_el1, cpu) &&
+            !cpu_isar_feature(aa64_aa32_el2, cpu)) {
+            value |= SCR_RW;           /* RAO/WI*/
+        }
         if (cpu_isar_feature(aa64_ras, cpu)) {
             valid_mask |= SCR_TERR;
         }
-- 
2.34.1

Re: [PATCH v2 0/2] target/arm: SCR_EL3 RES0, RAO/WI tweaks

[Peter Maydell]

On Thu, 9 Jun 2022 at 22:52, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Adjust RW, fixing #1062, and adjusting bits [4:2].
>
> Changes for v2:
>   * Fix patch 1 vs reset.
>

Applied to target-arm.next, thanks. (Fixed a missing space
at end of comment in patch 2.)

-- PMM