[PATCH] target/arm: Correct condition of aa64_atomics feature function

Peter Maydell posted 1 patch 2 months, 3 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20250819145659.2165160-1-peter.maydell@linaro.org
Maintainers: Peter Maydell <peter.maydell@linaro.org>
target/arm/cpu-features.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] target/arm: Correct condition of aa64_atomics feature function
Posted by Peter Maydell 2 months, 3 weeks ago
The ARMv8.1-Atomics feature (renamed FEAT_LSE in more modern versions
of the Arm ARM) has always ben indicated by ID_AA64ISAR0.ATOMIC being
0b0010 or greater; 0b0001 is a reserved unused value.

We were incorrectly checking for != 0; this had no harmful effects
because all the CPUs set their value for this field to either 0
(for not having the feature) or 2 (if they do have it), but it's
better to match what the architecture specifies here.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
I think it makes more sense to fix the condition first and rename
the feature second, though there's not much difference in it.
I'm happy to fix up the trivial conflict in target-arm.next.

 target/arm/cpu-features.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/cpu-features.h b/target/arm/cpu-features.h
index 5876162428a..a4d00a001d3 100644
--- a/target/arm/cpu-features.h
+++ b/target/arm/cpu-features.h
@@ -408,7 +408,7 @@ static inline bool isar_feature_aa64_crc32(const ARMISARegisters *id)
 
 static inline bool isar_feature_aa64_atomics(const ARMISARegisters *id)
 {
-    return FIELD_EX64_IDREG(id, ID_AA64ISAR0, ATOMIC) != 0;
+    return FIELD_EX64_IDREG(id, ID_AA64ISAR0, ATOMIC) >= 2;
 }
 
 static inline bool isar_feature_aa64_rdm(const ARMISARegisters *id)
-- 
2.43.0
Re: [PATCH] target/arm: Correct condition of aa64_atomics feature function
Posted by Richard Henderson 2 months, 3 weeks ago
On 8/20/25 00:56, Peter Maydell wrote:
> The ARMv8.1-Atomics feature (renamed FEAT_LSE in more modern versions
> of the Arm ARM) has always ben indicated by ID_AA64ISAR0.ATOMIC being
> 0b0010 or greater; 0b0001 is a reserved unused value.
> 
> We were incorrectly checking for != 0; this had no harmful effects
> because all the CPUs set their value for this field to either 0
> (for not having the feature) or 2 (if they do have it), but it's
> better to match what the architecture specifies here.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> I think it makes more sense to fix the condition first and rename
> the feature second, though there's not much difference in it.
> I'm happy to fix up the trivial conflict in target-arm.next.
> 
>   target/arm/cpu-features.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/arm/cpu-features.h b/target/arm/cpu-features.h
> index 5876162428a..a4d00a001d3 100644
> --- a/target/arm/cpu-features.h
> +++ b/target/arm/cpu-features.h
> @@ -408,7 +408,7 @@ static inline bool isar_feature_aa64_crc32(const ARMISARegisters *id)
>   
>   static inline bool isar_feature_aa64_atomics(const ARMISARegisters *id)
>   {
> -    return FIELD_EX64_IDREG(id, ID_AA64ISAR0, ATOMIC) != 0;
> +    return FIELD_EX64_IDREG(id, ID_AA64ISAR0, ATOMIC) >= 2;
>   }
>   
>   static inline bool isar_feature_aa64_rdm(const ARMISARegisters *id)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~