Re: [RFC PATCH 1/4] arm: Add FEAT_XS's TLBI NXS variants

[Richard Henderson]

On 10/14/24 03:48, Manos Pitsidianakis wrote:
Signed-off-by: Manos Pitsidianakis <manos.pitsidiana...@linaro.org>
---
  target/arm/cpu-features.h |   5 +
  target/arm/helper.c       | 366 +++++++++++++++++++++++++++-------------------
  2 files changed, 218 insertions(+), 153 deletions(-)
diff --git a/target/arm/cpu-features.h b/target/arm/cpu-features.h
index 
04ce2818263e2c3b99c59940001b65302e1d26d2..b4dcd429c3540e18c44d3c30f82f030be45719f2
 100644
--- a/target/arm/cpu-features.h
+++ b/target/arm/cpu-features.h
@@ -970,6 +970,11 @@ static inline bool isar_feature_aa64_sme_fa64(const 
ARMISARegisters *id)
      return FIELD_EX64(id->id_aa64smfr0, ID_AA64SMFR0, FA64);
  }
  
+static inline bool isar_feature_aa64_xs(const ARMISARegisters *id)
+{
+    return FIELD_SEX64(id->id_aa64isar1, ID_AA64ISAR1, XS) >= 0;
+}
+
  /*
   * Feature tests for "does this exist in either 32-bit or 64-bit?"
   */
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 
3f77b40734f2db831254a0e4eb205751aec0d1e5..3104a2d1dab6e58bf454c75afd478ec6d5fe521f
 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -5671,98 +5671,111 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
        .fgt = FGT_DCCISW,
        .access = PL1_W, .accessfn = access_tsw, .type = ARM_CP_NOP },
      /* TLBI operations */
-    { .name = "TLBI_VMALLE1IS", .state = ARM_CP_STATE_AA64,
+#define TLBI(name, opc0, opc1, crn, crm, opc2, access, accessfn, type, fgt, \
+             writefn)                                                       \
+{ name, .state = ARM_CP_STATE_AA64, opc0, opc1, crn, crm, opc2, access,     \
+  accessfn, type, fgt, writefn },                                           \
+{ name"NXS", .state = ARM_CP_STATE_AA64, opc0, opc1, crn + 1, crm, opc2,    \
+   access, accessfn, type, fgt, writefn }

You cannot insert the NXS operations into the existing arrays.
They must be separate, so that they are registered only if FEAT_XS is present.

You can see this with the split between v8_cp_reginfo[] and tlbirange_reginfo[].

@@ -9201,7 +9260,8 @@ void register_cp_regs_for_features(ARMCPU *cpu)
                                 R_ID_AA64ISAR1_SB_MASK |
                                 R_ID_AA64ISAR1_BF16_MASK |
                                 R_ID_AA64ISAR1_DGH_MASK |
-                               R_ID_AA64ISAR1_I8MM_MASK },
+                               R_ID_AA64ISAR1_I8MM_MASK |
+                               R_ID_AA64ISAR1_XS_MASK },

This is incorrect.  Here we are emulating

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/arch/arm64/cpu-feature-registers.rst#n208

and XS is not present.  Nor should it be, since cache flushing is not something that 
userland may do.


r~