| Message ID | 20230822042530.1026751-4-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Implement Most ARMv8.3 Pointer Authentication Features | expand |
On Tue, 22 Aug 2023 at 05:27, Richard Henderson <richard.henderson@linaro.org> wrote: > > From: Aaron Lindsay <aaron@os.amperecomputing.com> > > Rename isar_feature_aa64_pauth_arch to isar_feature_aa64_pauth_qarma5 > to distinguish the other architectural algorithm qarma3. > > Add ARMPauthFeature and isar_feature_pauth_feature to cover the > other pauth conditions. > > Signed-off-by: Aaron Lindsay <aaron@os.amperecomputing.com> > Message-Id: <20230609172324.982888-3-aaron@os.amperecomputing.com> > [rth: Add ARMPauthFeature and eliminate most other predicates] > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/arm/cpu.h | 49 +++++++++++++++++++++++++++++------ > target/arm/tcg/pauth_helper.c | 2 +- > 2 files changed, 42 insertions(+), 9 deletions(-) > > diff --git a/target/arm/cpu.h b/target/arm/cpu.h > index fbdbf2df7f..e9fe268453 100644 > --- a/target/arm/cpu.h > +++ b/target/arm/cpu.h > @@ -3794,28 +3794,61 @@ static inline bool isar_feature_aa64_fcma(const ARMISARegisters *id) > return FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, FCMA) != 0; > } > > +/* > + * These are the values from APA/API/APA3. > + * > + * They must be compared '>=', except EPAC should use '=='. > + * In the ARM pseudocode, EPAC is treated as not being implemented > + * by larger values. > + */ Yeah, but we use PauthFeat_EPAC in exactly one place and deliberately in a way where it doesn't matter if we use >=... (I think the pseudocode would be clearer if it was adjusted to do the same, personally. This ID register field follows the standard ID scheme which means that increasing values are supersets, so the "only if equal" part is weird.) > +typedef enum { > + PauthFeat_None = 0, > + PauthFeat_1 = 1, > + PauthFeat_EPAC = 2, > + PauthFeat_2 = 3, > + PauthFeat_FPAC = 4, > + PauthFeat_FPACCOMBINED = 5, > +} ARMPauthFeature; > + > +static inline ARMPauthFeature > +isar_feature_pauth_feature(const ARMISARegisters *id) > +{ > + /* > + * Architecturally, only one of {APA,API,APA3} may be active (non-zero) > + * and the other two must be zero. Thus we may avoid conditionals. > + */ > + return (FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, APA) | > + FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, API) | > + FIELD_EX64(id->id_aa64isar2, ID_AA64ISAR2, APA3)); > +} > + > static inline bool isar_feature_aa64_pauth(const ARMISARegisters *id) > { > /* > * Return true if any form of pauth is enabled, as this > * predicate controls migration of the 128-bit keys. > */ > - return (id->id_aa64isar1 & > - (FIELD_DP64(0, ID_AA64ISAR1, APA, 0xf) | > - FIELD_DP64(0, ID_AA64ISAR1, API, 0xf) | > - FIELD_DP64(0, ID_AA64ISAR1, GPA, 0xf) | > - FIELD_DP64(0, ID_AA64ISAR1, GPI, 0xf))) != 0; > + return isar_feature_pauth_feature(id) != PauthFeat_None; Having said "must be compared >=" you then use a != comparison here :-) Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
diff --git a/target/arm/cpu.h b/target/arm/cpu.h index fbdbf2df7f..e9fe268453 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -3794,28 +3794,61 @@ static inline bool isar_feature_aa64_fcma(const ARMISARegisters *id) return FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, FCMA) != 0; } +/* + * These are the values from APA/API/APA3. + * + * They must be compared '>=', except EPAC should use '=='. + * In the ARM pseudocode, EPAC is treated as not being implemented + * by larger values. + */ +typedef enum { + PauthFeat_None = 0, + PauthFeat_1 = 1, + PauthFeat_EPAC = 2, + PauthFeat_2 = 3, + PauthFeat_FPAC = 4, + PauthFeat_FPACCOMBINED = 5, +} ARMPauthFeature; + +static inline ARMPauthFeature +isar_feature_pauth_feature(const ARMISARegisters *id) +{ + /* + * Architecturally, only one of {APA,API,APA3} may be active (non-zero) + * and the other two must be zero. Thus we may avoid conditionals. + */ + return (FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, APA) | + FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, API) | + FIELD_EX64(id->id_aa64isar2, ID_AA64ISAR2, APA3)); +} + static inline bool isar_feature_aa64_pauth(const ARMISARegisters *id) { /* * Return true if any form of pauth is enabled, as this * predicate controls migration of the 128-bit keys. */ - return (id->id_aa64isar1 & - (FIELD_DP64(0, ID_AA64ISAR1, APA, 0xf) | - FIELD_DP64(0, ID_AA64ISAR1, API, 0xf) | - FIELD_DP64(0, ID_AA64ISAR1, GPA, 0xf) | - FIELD_DP64(0, ID_AA64ISAR1, GPI, 0xf))) != 0; + return isar_feature_pauth_feature(id) != PauthFeat_None; } -static inline bool isar_feature_aa64_pauth_arch(const ARMISARegisters *id) +static inline bool isar_feature_aa64_pauth_qarma5(const ARMISARegisters *id) { /* - * Return true if pauth is enabled with the architected QARMA algorithm. - * QEMU will always set APA+GPA to the same value. + * Return true if pauth is enabled with the architected QARMA5 algorithm. + * QEMU will always enable or disable both APA and GPA. */ return FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, APA) != 0; } +static inline bool isar_feature_aa64_pauth_qarma3(const ARMISARegisters *id) +{ + /* + * Return true if pauth is enabled with the architected QARMA3 algorithm. + * QEMU will always enable or disable both APA3 and GPA3. + */ + return FIELD_EX64(id->id_aa64isar2, ID_AA64ISAR2, APA3) != 0; +} + static inline bool isar_feature_aa64_tlbirange(const ARMISARegisters *id) { return FIELD_EX64(id->id_aa64isar0, ID_AA64ISAR0, TLB) == 2; diff --git a/target/arm/tcg/pauth_helper.c b/target/arm/tcg/pauth_helper.c index 62af569341..6271a84ec9 100644 --- a/target/arm/tcg/pauth_helper.c +++ b/target/arm/tcg/pauth_helper.c @@ -282,7 +282,7 @@ static uint64_t pauth_computepac_impdef(uint64_t data, uint64_t modifier, static uint64_t pauth_computepac(CPUARMState *env, uint64_t data, uint64_t modifier, ARMPACKey key) { - if (cpu_isar_feature(aa64_pauth_arch, env_archcpu(env))) { + if (cpu_isar_feature(aa64_pauth_qarma5, env_archcpu(env))) { return pauth_computepac_architected(data, modifier, key); } else { return pauth_computepac_impdef(data, modifier, key);