Re: [PATCH v2 1/1] target/arm: Fix SCR RES1 handling

[Rémi Denis-Courmont]

Le keskiviikkona 3. helmikuuta 2021, 18.55.52 EET michael. nawrocki--- via a 
écrit :
> The FW and AW bits of SCR_EL3 are RES1 only in some contexts. Force them
> to 1 only when there is no support for AArch32 at EL1 or above.
> 
> The reset value will be 0x30 only if the CPU is AArch64-only; if there
> is support for AArch32 at EL1 or above, it will be reset to 0.
> 
> Also adds helper function isar_feature_aa64_aa32_el1 to check if AArch32
> is supported at EL1 or above.
> 
> Signed-off-by: Mike Nawrocki <michael.nawrocki@gtri.gatech.edu>
> ---
>  target/arm/cpu.h    |  5 +++++
>  target/arm/helper.c | 16 ++++++++++++++--
>  2 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index d080239863..39633f73f3 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -4033,6 +4033,11 @@ static inline bool isar_feature_aa64_aa32(const
> ARMISARegisters *id) return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, EL0)
> >= 2;
>  }
> 
> +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_sve(const ARMISARegisters *id)
>  {
>      return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, SVE) != 0;
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 47e266d7e6..e529cdbfd0 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -2024,7 +2024,10 @@ static void scr_write(CPUARMState *env, const
> ARMCPRegInfo *ri, uint64_t value) ARMCPU *cpu = env_archcpu(env);
> 
>      if (ri->state == ARM_CP_STATE_AA64) {
> -        value |= SCR_FW | SCR_AW;   /* these two bits are RES1.  */
> +        if (arm_feature(env, ARM_FEATURE_AARCH64) &&

AFAICT, this is a tautology in this context.

> +            !cpu_isar_feature(aa64_aa32_el1, cpu)) {
> +                value |= SCR_FW | SCR_AW;   /* these two bits are RES1.  */
> +        }
>          valid_mask &= ~SCR_NET;
> 
>          if (cpu_isar_feature(aa64_lor, cpu)) {
> @@ -2063,6 +2066,15 @@ static void scr_write(CPUARMState *env, const
> ARMCPRegInfo *ri, uint64_t value) raw_write(env, ri, value);
>  }
> 
> +static void scr_reset(CPUARMState *env, const ARMCPRegInfo *ri)
> +{
> +    /*
> +     * scr_write will set the RES1 bits on an AArch64-only CPU.
> +     * The reset value will be 0x30 on an AArch64-only CPU and 0 otherwise.
> +     */
> +    scr_write(env, ri, 0);
> +}
> +
>  static CPAccessResult access_aa64_tid2(CPUARMState *env,
>                                         const ARMCPRegInfo *ri,
>                                         bool isread)
> @@ -5785,7 +5797,7 @@ static const ARMCPRegInfo el3_cp_reginfo[] = {
>      { .name = "SCR_EL3", .state = ARM_CP_STATE_AA64,
>        .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 1, .opc2 = 0,
>        .access = PL3_RW, .fieldoffset = offsetof(CPUARMState, cp15.scr_el3),
> -      .resetvalue = 0, .writefn = scr_write },
> +      .resetfn = scr_reset, .writefn = scr_write },
>      { .name = "SCR",  .type = ARM_CP_ALIAS | ARM_CP_NEWEL,
>        .cp = 15, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 0,
>        .access = PL1_RW, .accessfn = access_trap_aa32s_el1,


-- 
レミ・デニ-クールモン
http://www.remlab.net/