On Fri, 5 Jun 2026 at 00:53, Richard Henderson
<[email protected]> wrote:
>
> Enable FEAT_SVE_AES instructions in streaming mode.
>
> Signed-off-by: Richard Henderson <[email protected]>
> ---
>  target/arm/cpu-features.h      |  5 +++++
>  linux-user/aarch64/elfload.c   |  1 +
>  target/arm/tcg/cpu64.c         |  1 +
>  target/arm/tcg/translate-sve.c | 20 +++++++++++---------
>  docs/system/arm/emulation.rst  |  1 +
>  5 files changed, 19 insertions(+), 9 deletions(-)

As far as this patch goes,
Reviewed-by: Peter Maydell <[email protected]>

but I was briefly confused by something in our pre-existing code for
these insns:

> @@ -8174,15 +8174,17 @@ TRANS_FEAT(SDOT_zzzz_2s, aa64_sme2_or_sve2p1, 
> gen_gvec_ool_arg_zzzz,
>  TRANS_FEAT(UDOT_zzzz_2s, aa64_sme2_or_sve2p1, gen_gvec_ool_arg_zzzz,
>             gen_helper_gvec_udot_2h, a, 0)
>
> -TRANS_FEAT_NONSTREAMING(AESMC, aa64_sve2_aes, gen_gvec_ool_zz,
> -                        gen_helper_crypto_aesmc, a->rd, a->rd, 0)
> -TRANS_FEAT_NONSTREAMING(AESIMC, aa64_sve2_aes, gen_gvec_ool_zz,
> -                        gen_helper_crypto_aesimc, a->rd, a->rd, 0)

We mark all of these as aa64_sve2_aes(), which we define as
    return FIELD_EX64_IDREG(id, ID_AA64ZFR0, AES) != 0;

But the Arm ARM calls the feature defined by AES >= 0 "FEAT_SVE_AES",
not "FEAT_SVE2_AES". I think this is because we implemented this
before the Arm ARM split out separate official feature names for
the sub-parts of SVE2, and we happened to choose something that
is almost but not quite the same as the official one.
(cf commit bc980d66308 adding the extra names to the docs).
The name we have has slight potential to confuse, because there
is also a FEAT_SVE_AES2 (for AES >= 3), which we don't implement yet.

Do you think it's worth updating our feature function names to
match the official architecture names, or shall we just leave it be?

-- PMM

Reply via email to