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

[Richard Henderson <richard.henderson@linaro.org>]

On 10/14/24 03:48, Manos Pitsidianakis wrote:
> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@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~