mbox series

[v3,0/6] target/arm: Fixes for RME

Message ID 20230809123706.1842548-1-jean-philippe@linaro.org
Headers show
Series target/arm: Fixes for RME | expand

Message

Jean-Philippe Brucker Aug. 9, 2023, 12:37 p.m. UTC
A few patches to fix RME support and allow booting a realm guest, based
on "[PATCH v2 00/15] target/arm/ptw: Cleanups and a few bugfixes"
https://lore.kernel.org/all/20230807141514.19075-1-peter.maydell@linaro.org/

Since v2:

* Updated the comment in patch 5. I also removed the check for FEAT_RME,
  because as pointed out in "target/arm: Catch illegal-exception-return
  from EL3 with bad NSE/NS", the SCR_NSE bit can only be set with
  FEAT_RME enabled. Because of this additional change, I didn't add the
  Reviewed-by.

* Added an EL-change hook to patch 6, to update the timer IRQ
  when changing the security state. I was wondering whether the
  el_change function should filter security state changes, since we only
  need to update IRQ state when switching between Root and
  Secure/NonSecure. But with a small syscall benchmark exercising
  EL0-EL1 switch with FEAT_RME enabled, I couldn't see any difference
  with and without the el_change hook, so I kept it simple.

* Also added the .raw_write callback for CNTHCTL_EL2.

v2: https://lore.kernel.org/all/20230802170157.401491-1-jean-philippe@linaro.org/

Jean-Philippe Brucker (6):
  target/arm/ptw: Load stage-2 tables from realm physical space
  target/arm/helper: Fix tlbmask and tlbbits for TLBI VAE2*
  target/arm: Skip granule protection checks for AT instructions
  target/arm: Pass security space rather than flag for AT instructions
  target/arm/helper: Check SCR_EL3.{NSE,NS} encoding for AT instructions
  target/arm/helper: Implement CNTHCTL_EL2.CNT[VP]MASK

 target/arm/cpu.h        |   4 +
 target/arm/internals.h  |  25 +++---
 target/arm/cpu.c        |   4 +
 target/arm/helper.c     | 184 ++++++++++++++++++++++++++++++----------
 target/arm/ptw.c        |  39 ++++++---
 target/arm/trace-events |   7 +-
 6 files changed, 188 insertions(+), 75 deletions(-)

Comments

Peter Maydell Aug. 10, 2023, 1:16 p.m. UTC | #1
On Wed, 9 Aug 2023 at 13:37, Jean-Philippe Brucker
<jean-philippe@linaro.org> wrote:
>
> A few patches to fix RME support and allow booting a realm guest, based
> on "[PATCH v2 00/15] target/arm/ptw: Cleanups and a few bugfixes"
> https://lore.kernel.org/all/20230807141514.19075-1-peter.maydell@linaro.org/
>
> Since v2:
>
> * Updated the comment in patch 5. I also removed the check for FEAT_RME,
>   because as pointed out in "target/arm: Catch illegal-exception-return
>   from EL3 with bad NSE/NS", the SCR_NSE bit can only be set with
>   FEAT_RME enabled. Because of this additional change, I didn't add the
>   Reviewed-by.
>
> * Added an EL-change hook to patch 6, to update the timer IRQ
>   when changing the security state. I was wondering whether the
>   el_change function should filter security state changes, since we only
>   need to update IRQ state when switching between Root and
>   Secure/NonSecure. But with a small syscall benchmark exercising
>   EL0-EL1 switch with FEAT_RME enabled, I couldn't see any difference
>   with and without the el_change hook, so I kept it simple.
>
> * Also added the .raw_write callback for CNTHCTL_EL2.
>
> v2: https://lore.kernel.org/all/20230802170157.401491-1-jean-philippe@linaro.org/

This didn't build for the linux-user targets. I squashed
this into patch 6:

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 7df1f7600b1..d906d2b1caa 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2169,9 +2169,11 @@ static void arm_cpu_realizefn(DeviceState *dev,
Error **errp)
         set_feature(env, ARM_FEATURE_VBAR);
     }

-    if (cpu_isar_feature(aa64_rme, cpu)) {
+#ifndef CONFIG_USER_ONLY
+    if (tcg_enabled() && cpu_isar_feature(aa64_rme, cpu)) {
         arm_register_el_change_hook(cpu, &gt_rme_post_el_change, 0);
     }
+#endif

     register_cp_regs_for_features(cpu);
     arm_cpu_register_gdb_regs_for_features(cpu);

With that, I've applied the series to target-arm-for-8.2.

thanks
-- PMM
Jean-Philippe Brucker Aug. 10, 2023, 1:36 p.m. UTC | #2
On Thu, Aug 10, 2023 at 02:16:56PM +0100, Peter Maydell wrote:
> This didn't build for the linux-user targets. I squashed
> this into patch 6:
> 
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 7df1f7600b1..d906d2b1caa 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -2169,9 +2169,11 @@ static void arm_cpu_realizefn(DeviceState *dev,
> Error **errp)
>          set_feature(env, ARM_FEATURE_VBAR);
>      }
> 
> -    if (cpu_isar_feature(aa64_rme, cpu)) {
> +#ifndef CONFIG_USER_ONLY
> +    if (tcg_enabled() && cpu_isar_feature(aa64_rme, cpu)) {
>          arm_register_el_change_hook(cpu, &gt_rme_post_el_change, 0);
>      }
> +#endif
> 
>      register_cp_regs_for_features(cpu);
>      arm_cpu_register_gdb_regs_for_features(cpu);
> 
> With that, I've applied the series to target-arm-for-8.2.

Thank you, sorry about the build error, I'll add linux-user to my tests

Thanks,
Jean