Re: [Qemu-devel] [PATCH v3 07/10] target/arm: Introduce ARM_FEATURE_V8_ATOMICS and initial decode

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

On 05/08/2018 09:48 AM, Peter Maydell wrote:
> (A v8.x implementation can include any arbitrary
> subset of the v8.(x+1) features, so this is a legal combination.)

I didn't realize this was a possibility.

> In fact we already have a mandatory-in-v8.1 feature:
> ARM_FEATURE_V8_RDM, which we've just given its own feature bit.

Yes, I'd been planning to send follow-up patches to "tidy" this.

> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -1449,8 +1449,7 @@ enum arm_features {
>      ARM_FEATURE_V8_SHA3, /* implements SHA3 part of v8 Crypto Extensions */
>      ARM_FEATURE_V8_SM3, /* implements SM3 part of v8 Crypto Extensions */
>      ARM_FEATURE_V8_SM4, /* implements SM4 part of v8 Crypto Extensions */
> -    ARM_FEATURE_V8_1,
> -    ARM_FEATURE_V8_ATOMICS = ARM_FEATURE_V8_1, /* mandatory extension */
> +    ARM_FEATURE_V8_ATOMICS, /* ARMv8.1-Atomics feature */
>      ARM_FEATURE_V8_RDM, /* implements v8.1 simd round multiply */
>      ARM_FEATURE_V8_FP16, /* implements v8.2 half-precision float */
>      ARM_FEATURE_V8_FCMA, /* has complex number part of v8.3 extensions.  */
> 
> Is that OK?

Yes, that's fine.  Thanks.

> We can have a separate patch which adds ARM_FEATURE_V8_1 and the
> code in realizefn to do
>     if (arm_feature(env, ARM_FEATURE_V8_1)) {
>         set_feature(env, ARM_FEATURE_V8);
>         set_feature(env, ARM_FEATURE_V8_ATOMICS);
>         set_feature(env, ARM_FEATURE_V8_RDM);
>     }

Yep.


r~