Re: [PATCH v3 2/8] target/arm: v8.3 PAC ID_AA64ISAR[12] feature-detection

[Aaron Lindsay]

On Jun 09 13:51, Richard Henderson wrote:
> On 6/9/23 10:23, Aaron Lindsay wrote:
> > +static inline int isar_feature_pauth_get_features(const ARMISARegisters 
> > *id)
> > +{
> > +if (isar_feature_aa64_pauth_arch_qarma5(id)) {
> > +return FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, APA);
> > +} else if (isar_feature_aa64_pauth_arch_qarma3(id)) {
> > +return FIELD_EX64(id->id_aa64isar2, ID_AA64ISAR2, APA3);
> > +} else {
> > +return FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, API);
> > +}
> > +}
> 
> As I mentioned in previous review, exactly one of these fields will be
> non-zero, so you can just OR them all together without the conditionals.

Sorry I missed this last time around - I've queued this change for v4.

Thanks!

-Aaron

Re: [PATCH v3 2/8] target/arm: v8.3 PAC ID_AA64ISAR[12] feature-detection

[Richard Henderson]

On 6/9/23 10:23, Aaron Lindsay wrote:

+static inline int isar_feature_pauth_get_features(const ARMISARegisters *id)
+{
+if (isar_feature_aa64_pauth_arch_qarma5(id)) {
+return FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, APA);
+} else if (isar_feature_aa64_pauth_arch_qarma3(id)) {
+return FIELD_EX64(id->id_aa64isar2, ID_AA64ISAR2, APA3);
+} else {
+return FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, API);
+}
+}


As I mentioned in previous review, exactly one of these fields will be non-zero, so you 
can just OR them all together without the conditionals.



r~

Re: [PATCH v3 2/8] target/arm: v8.3 PAC ID_AA64ISAR[12] feature-detection

[Richard Henderson]

On 3/22/23 13:25, Aaron Lindsay wrote:

+static inline int isar_feature_pauth_get_features(const ARMISARegisters *id)
+{
+if (isar_feature_aa64_pauth_arch_qarma5(id)) {
+return FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, APA);
+} else if (isar_feature_aa64_pauth_arch_qarma3(id)) {
+return FIELD_EX64(id->id_aa64isar2, ID_AA64ISAR2, APA3);
+} else {
+return FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, API);
+}
+}


Only one of these fields is allowed to be non-zero, so we can avoid the tests and simply 
OR them all together.  With a comment to that effect, of course.


Otherwise,
Reviewed-by: Richard Henderson 


r~