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

[Peter Maydell]

On Sun, 5 Jun 2022 at 17:16, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> 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 c1865ad5da..a7c45d0d66 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -3947,6 +3947,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 c262b00c3c..84232a6437 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -1755,6 +1755,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*/
> +        }

True in principle, but we probably need to do something to handle
the reset case for AArch32 CPUs, where cpu_isar_feature() will
return false becaese id_aa64pfr0 is zero but the bit should
nonetheless be RES0.

thanks
-- PMM