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

Jerome Forissier posted 1 patch 1 year, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20221004072354.27037-1-jerome.forissier@linaro.org
Maintainers: Peter Maydell <peter.maydell@linaro.org>
target/arm/cpu.h    | 54 ++++++++++++++++++++++-----------------------
target/arm/helper.c |  5 ++++-
2 files changed, 31 insertions(+), 28 deletions(-)
[PATCH] target/arm: allow setting SCR_EL3.EnTP2 when FEAT_SME is implemented
Posted by Jerome Forissier 1 year, 6 months ago
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>
---
 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)) {
-- 
2.34.1
Re: [PATCH] target/arm: allow setting SCR_EL3.EnTP2 when FEAT_SME is implemented
Posted by Peter Maydell 1 year, 6 months ago
On Tue, 4 Oct 2022 at 08:24, Jerome Forissier
<jerome.forissier@linaro.org> wrote:
>
> 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>



Applied to target-arm.next, thanks.

-- PMM
Re: [PATCH] target/arm: allow setting SCR_EL3.EnTP2 when FEAT_SME is implemented
Posted by Richard Henderson 1 year, 6 months ago
On 10/4/22 00:23, Jerome Forissier wrote:
> 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>
> ---
>   target/arm/cpu.h    | 54 ++++++++++++++++++++++-----------------------
>   target/arm/helper.c |  5 ++++-
>   2 files changed, 31 insertions(+), 28 deletions(-)

Whoops, sorry about that.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~
Re: [PATCH] target/arm: allow setting SCR_EL3.EnTP2 when FEAT_SME is implemented
Posted by Andre Przywara 1 year, 6 months ago
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)) {