Re: [PATCH] target/arm: allow setting SCR_EL3.EnTP2 when FEAT_SME is implemented

[Andre Przywara <andre.przywara@arm.com>]

On Tue,  4 Oct 2022 09:23:54 +0200
Jerome Forissier <jerome.forissier@linaro.org> wrote:

Hi,

> Updates write_scr() to allow setting SCR_EL3.EnTP2 when FEAT_SME is
> implemented. SCR_EL3 being a 64-bit register, valid_mask is changed
> to uint64_t and the SCR_* constants in target/arm/cpu.h are extended
> to 64-bit so that masking and bitwise not (~) behave as expected.
> 
> This enables booting Linux with Trusted Firmware-A at EL3 with
> "-M virt,secure=on -cpu max".
> 
> Cc: qemu-stable@nongnu.org
> Fixes: 78cb9776662a ("target/arm: Enable SME for -cpu max")
> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>

Good catch!
So I can confirm that this fixes the issue, given a TF-A patch to actually
enable SME (and SVE).
Checked against the ARM ARM, also verified that the
defines don't accidentally changed their values.

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

> ---
>  target/arm/cpu.h    | 54 ++++++++++++++++++++++-----------------------
>  target/arm/helper.c |  5 ++++-
>  2 files changed, 31 insertions(+), 28 deletions(-)
> 
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 5168e3d837..d5e9949eb6 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -1653,33 +1653,33 @@ static inline void xpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)
>  
>  #define HPFAR_NS      (1ULL << 63)
>  
> -#define SCR_NS                (1U << 0)
> -#define SCR_IRQ               (1U << 1)
> -#define SCR_FIQ               (1U << 2)
> -#define SCR_EA                (1U << 3)
> -#define SCR_FW                (1U << 4)
> -#define SCR_AW                (1U << 5)
> -#define SCR_NET               (1U << 6)
> -#define SCR_SMD               (1U << 7)
> -#define SCR_HCE               (1U << 8)
> -#define SCR_SIF               (1U << 9)
> -#define SCR_RW                (1U << 10)
> -#define SCR_ST                (1U << 11)
> -#define SCR_TWI               (1U << 12)
> -#define SCR_TWE               (1U << 13)
> -#define SCR_TLOR              (1U << 14)
> -#define SCR_TERR              (1U << 15)
> -#define SCR_APK               (1U << 16)
> -#define SCR_API               (1U << 17)
> -#define SCR_EEL2              (1U << 18)
> -#define SCR_EASE              (1U << 19)
> -#define SCR_NMEA              (1U << 20)
> -#define SCR_FIEN              (1U << 21)
> -#define SCR_ENSCXT            (1U << 25)
> -#define SCR_ATA               (1U << 26)
> -#define SCR_FGTEN             (1U << 27)
> -#define SCR_ECVEN             (1U << 28)
> -#define SCR_TWEDEN            (1U << 29)
> +#define SCR_NS                (1ULL << 0)
> +#define SCR_IRQ               (1ULL << 1)
> +#define SCR_FIQ               (1ULL << 2)
> +#define SCR_EA                (1ULL << 3)
> +#define SCR_FW                (1ULL << 4)
> +#define SCR_AW                (1ULL << 5)
> +#define SCR_NET               (1ULL << 6)
> +#define SCR_SMD               (1ULL << 7)
> +#define SCR_HCE               (1ULL << 8)
> +#define SCR_SIF               (1ULL << 9)
> +#define SCR_RW                (1ULL << 10)
> +#define SCR_ST                (1ULL << 11)
> +#define SCR_TWI               (1ULL << 12)
> +#define SCR_TWE               (1ULL << 13)
> +#define SCR_TLOR              (1ULL << 14)
> +#define SCR_TERR              (1ULL << 15)
> +#define SCR_APK               (1ULL << 16)
> +#define SCR_API               (1ULL << 17)
> +#define SCR_EEL2              (1ULL << 18)
> +#define SCR_EASE              (1ULL << 19)
> +#define SCR_NMEA              (1ULL << 20)
> +#define SCR_FIEN              (1ULL << 21)
> +#define SCR_ENSCXT            (1ULL << 25)
> +#define SCR_ATA               (1ULL << 26)
> +#define SCR_FGTEN             (1ULL << 27)
> +#define SCR_ECVEN             (1ULL << 28)
> +#define SCR_TWEDEN            (1ULL << 29)
>  #define SCR_TWEDEL            MAKE_64BIT_MASK(30, 4)
>  #define SCR_TME               (1ULL << 34)
>  #define SCR_AMVOFFEN          (1ULL << 35)
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index d7bc467a2a..5cde8a0425 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -1669,7 +1669,7 @@ static void vbar_write(CPUARMState *env, const ARMCPRegInfo *ri,
>  static void scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
>  {
>      /* Begin with base v8.0 state.  */
> -    uint32_t valid_mask = 0x3fff;
> +    uint64_t valid_mask = 0x3fff;
>      ARMCPU *cpu = env_archcpu(env);
>  
>      /*
> @@ -1706,6 +1706,9 @@ static void scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
>          if (cpu_isar_feature(aa64_doublefault, cpu)) {
>              valid_mask |= SCR_EASE | SCR_NMEA;
>          }
> +        if (cpu_isar_feature(aa64_sme, cpu)) {
> +            valid_mask |= SCR_ENTP2;
> +        }
>      } else {
>          valid_mask &= ~(SCR_RW | SCR_ST);
>          if (cpu_isar_feature(aa32_ras, cpu)) {