[PATCH v2 0/3] target/arm: SME1/SVE2 fixes

[PATCH v2 0/3] target/arm: SME1/SVE2 fixes

[Richard Henderson]

Supercedes: 20250622175052.180728-1-richard.henderson@linaro.org
("target/arm: Fix SME vs AdvSIMD exception priority")

A couple of fixes for EC_SMETRAP, plus some insns that missed
being updated for non-streaming.


r~


Richard Henderson (3):
  target/arm: Fix SME vs AdvSIMD exception priority
  target/arm: Fix sve_access_check for SME
  target/arm: Fix 128-bit element ZIP, UZP, TRN

 target/arm/tcg/translate-a64.c | 30 +++++++++++++++++-------
 target/arm/tcg/translate-sve.c | 43 ++++++++++++++++++++++++----------
 2 files changed, 51 insertions(+), 22 deletions(-)

-- 
2.43.0

[PATCH v2 1/3] target/arm: Fix SME vs AdvSIMD exception priority

[Richard Henderson]

We failed to raise an exception when
sme_excp_el == 0 and fp_excp_el == 1.

Cc: qemu-stable@nongnu.org
Fixes: 3d74825f4d6 ("target/arm: Add SME enablement checks")
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/tcg/translate-a64.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
index ac80f572a2..bb49a2ce90 100644
--- a/target/arm/tcg/translate-a64.c
+++ b/target/arm/tcg/translate-a64.c
@@ -1494,7 +1494,8 @@ bool sme_enabled_check(DisasContext *s)
      * to be zero when fp_excp_el has priority.  This is because we need
      * sme_excp_el by itself for cpregs access checks.
      */
-    if (!s->fp_excp_el || s->sme_excp_el < s->fp_excp_el) {
+    if (s->sme_excp_el
+        && (!s->fp_excp_el || s->sme_excp_el <= s->fp_excp_el)) {
         bool ret = sme_access_check(s);
         s->fp_access_checked = (ret ? 1 : -1);
         return ret;
-- 
2.43.0

Re: [PATCH v2 1/3] target/arm: Fix SME vs AdvSIMD exception priority

[Peter Maydell]

On Sun, 22 Jun 2025 at 22:36, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> We failed to raise an exception when
> sme_excp_el == 0 and fp_excp_el == 1.
>
> Cc: qemu-stable@nongnu.org
> Fixes: 3d74825f4d6 ("target/arm: Add SME enablement checks")
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/tcg/translate-a64.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)


Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[PATCH v2 2/3] target/arm: Fix sve_access_check for SME

[Richard Henderson]

Do not assume SME implies SVE.  Ensure that the
non-streaming check is present along the SME path,
since it is not implied by sme_*_enabled_check.

Cc: qemu-stable@nongnu.org
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/tcg/translate-a64.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
index bb49a2ce90..d7b0c81773 100644
--- a/target/arm/tcg/translate-a64.c
+++ b/target/arm/tcg/translate-a64.c
@@ -1387,11 +1387,8 @@ static bool fp_access_check_only(DisasContext *s)
     return true;
 }
 
-static bool fp_access_check(DisasContext *s)
+static bool nonstreaming_check(DisasContext *s)
 {
-    if (!fp_access_check_only(s)) {
-        return false;
-    }
     if (s->sme_trap_nonstreaming && s->is_nonstreaming) {
         gen_exception_insn(s, 0, EXCP_UDEF,
                            syn_smetrap(SME_ET_Streaming, false));
@@ -1400,6 +1397,11 @@ static bool fp_access_check(DisasContext *s)
     return true;
 }
 
+static bool fp_access_check(DisasContext *s)
+{
+    return fp_access_check_only(s) && nonstreaming_check(s);
+}
+
 /*
  * Return <0 for non-supported element sizes, with MO_16 controlled by
  * FEAT_FP16; return 0 for fp disabled; otherwise return >0 for success.
@@ -1450,11 +1452,20 @@ static int fp_access_check_vector_hsd(DisasContext *s, bool is_q, MemOp esz)
  */
 bool sve_access_check(DisasContext *s)
 {
-    if (s->pstate_sm || !dc_isar_feature(aa64_sve, s)) {
-        bool ret;
+    bool ret;
 
-        assert(dc_isar_feature(aa64_sme, s));
-        ret = sme_sm_enabled_check(s);
+    switch (dc_isar_feature(aa64_sme, s)) {
+    case true:
+        if (s->pstate_sm) {
+            ret = sme_enabled_check(s);
+        } else if (!dc_isar_feature(aa64_sve, s)) {
+            ret = sme_sm_enabled_check(s);
+        } else {
+            break;
+        }
+        if (ret) {
+            ret = nonstreaming_check(s);
+        }
         s->sve_access_checked = (ret ? 1 : -1);
         return ret;
     }
-- 
2.43.0

Re: [PATCH v2 2/3] target/arm: Fix sve_access_check for SME

[Peter Maydell]

On Sun, 22 Jun 2025 at 22:36, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Do not assume SME implies SVE.  Ensure that the
> non-streaming check is present along the SME path,
> since it is not implied by sme_*_enabled_check.
>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/tcg/translate-a64.c | 27 +++++++++++++++++++--------
>  1 file changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
> index bb49a2ce90..d7b0c81773 100644
> --- a/target/arm/tcg/translate-a64.c
> +++ b/target/arm/tcg/translate-a64.c
> @@ -1387,11 +1387,8 @@ static bool fp_access_check_only(DisasContext *s)
>      return true;
>  }
>
> -static bool fp_access_check(DisasContext *s)
> +static bool nonstreaming_check(DisasContext *s)
>  {
> -    if (!fp_access_check_only(s)) {
> -        return false;
> -    }
>      if (s->sme_trap_nonstreaming && s->is_nonstreaming) {
>          gen_exception_insn(s, 0, EXCP_UDEF,
>                             syn_smetrap(SME_ET_Streaming, false));
> @@ -1400,6 +1397,11 @@ static bool fp_access_check(DisasContext *s)
>      return true;
>  }
>
> +static bool fp_access_check(DisasContext *s)
> +{
> +    return fp_access_check_only(s) && nonstreaming_check(s);
> +}
> +
>  /*
>   * Return <0 for non-supported element sizes, with MO_16 controlled by
>   * FEAT_FP16; return 0 for fp disabled; otherwise return >0 for success.
> @@ -1450,11 +1452,20 @@ static int fp_access_check_vector_hsd(DisasContext *s, bool is_q, MemOp esz)
>   */
>  bool sve_access_check(DisasContext *s)
>  {
> -    if (s->pstate_sm || !dc_isar_feature(aa64_sve, s)) {
> -        bool ret;
> +    bool ret;
>
> -        assert(dc_isar_feature(aa64_sme, s));
> -        ret = sme_sm_enabled_check(s);
> +    switch (dc_isar_feature(aa64_sme, s)) {
> +    case true:

Why this rather than
       if (dc_isar_feature(aa64_sme, s)) {

?

> +        if (s->pstate_sm) {
> +            ret = sme_enabled_check(s);
> +        } else if (!dc_isar_feature(aa64_sve, s)) {
> +            ret = sme_sm_enabled_check(s);
> +        } else {
> +            break;
> +        }
> +        if (ret) {
> +            ret = nonstreaming_check(s);
> +        }
>          s->sve_access_checked = (ret ? 1 : -1);
>          return ret;
>      }

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH v2 2/3] target/arm: Fix sve_access_check for SME

[Richard Henderson]

On 6/24/25 06:58, Peter Maydell wrote:
>> +    switch (dc_isar_feature(aa64_sme, s)) {
>> +    case true:
> 
> Why this rather than
>         if (dc_isar_feature(aa64_sme, s)) {
> 
> ?
> 
>> +        if (s->pstate_sm) {
>> +            ret = sme_enabled_check(s);
>> +        } else if (!dc_isar_feature(aa64_sve, s)) {
>> +            ret = sme_sm_enabled_check(s);
>> +        } else {
>> +            break;
>> +        }

I used break instead of a goto, or replicating


>> +        if (ret) {
>> +            ret = nonstreaming_check(s);
>> +        }
>>           s->sve_access_checked = (ret ? 1 : -1);
>>           return ret;

this block.


r~

[PATCH v2 3/3] target/arm: Fix 128-bit element ZIP, UZP, TRN

[Richard Henderson]

We missed the instructions UDEF when the vector size is too small.
We missed marking the instructions non-streaming with SME.

Cc: qemu-stable@nongnu.org
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/tcg/translate-sve.c | 43 ++++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 13 deletions(-)

diff --git a/target/arm/tcg/translate-sve.c b/target/arm/tcg/translate-sve.c
index f3cf028cb9..588a5b006b 100644
--- a/target/arm/tcg/translate-sve.c
+++ b/target/arm/tcg/translate-sve.c
@@ -2352,6 +2352,23 @@ TRANS_FEAT(PUNPKHI, aa64_sve, do_perm_pred2, a, 1, gen_helper_sve_punpk_p)
  *** SVE Permute - Interleaving Group
  */
 
+static bool do_interleave_q(DisasContext *s, gen_helper_gvec_3 *fn,
+                            arg_rrr_esz *a, int data)
+{
+    if (sve_access_check(s)) {
+        unsigned vsz = vec_full_reg_size(s);
+        if (vsz < 32) {
+            unallocated_encoding(s);
+        } else {
+            tcg_gen_gvec_3_ool(vec_full_reg_offset(s, a->rd),
+                               vec_full_reg_offset(s, a->rn),
+                               vec_full_reg_offset(s, a->rm),
+                               vsz, vsz, data, fn);
+        }
+    }
+    return true;
+}
+
 static gen_helper_gvec_3 * const zip_fns[4] = {
     gen_helper_sve_zip_b, gen_helper_sve_zip_h,
     gen_helper_sve_zip_s, gen_helper_sve_zip_d,
@@ -2361,11 +2378,11 @@ TRANS_FEAT(ZIP1_z, aa64_sve, gen_gvec_ool_arg_zzz,
 TRANS_FEAT(ZIP2_z, aa64_sve, gen_gvec_ool_arg_zzz,
            zip_fns[a->esz], a, vec_full_reg_size(s) / 2)
 
-TRANS_FEAT(ZIP1_q, aa64_sve_f64mm, gen_gvec_ool_arg_zzz,
-           gen_helper_sve2_zip_q, a, 0)
-TRANS_FEAT(ZIP2_q, aa64_sve_f64mm, gen_gvec_ool_arg_zzz,
-           gen_helper_sve2_zip_q, a,
-           QEMU_ALIGN_DOWN(vec_full_reg_size(s), 32) / 2)
+TRANS_FEAT_NONSTREAMING(ZIP1_q, aa64_sve_f64mm, do_interleave_q,
+                        gen_helper_sve2_zip_q, a, 0)
+TRANS_FEAT_NONSTREAMING(ZIP2_q, aa64_sve_f64mm, do_interleave_q,
+                        gen_helper_sve2_zip_q, a,
+                        QEMU_ALIGN_DOWN(vec_full_reg_size(s), 32) / 2)
 
 static gen_helper_gvec_3 * const uzp_fns[4] = {
     gen_helper_sve_uzp_b, gen_helper_sve_uzp_h,
@@ -2377,10 +2394,10 @@ TRANS_FEAT(UZP1_z, aa64_sve, gen_gvec_ool_arg_zzz,
 TRANS_FEAT(UZP2_z, aa64_sve, gen_gvec_ool_arg_zzz,
            uzp_fns[a->esz], a, 1 << a->esz)
 
-TRANS_FEAT(UZP1_q, aa64_sve_f64mm, gen_gvec_ool_arg_zzz,
-           gen_helper_sve2_uzp_q, a, 0)
-TRANS_FEAT(UZP2_q, aa64_sve_f64mm, gen_gvec_ool_arg_zzz,
-           gen_helper_sve2_uzp_q, a, 16)
+TRANS_FEAT_NONSTREAMING(UZP1_q, aa64_sve_f64mm, do_interleave_q,
+                        gen_helper_sve2_uzp_q, a, 0)
+TRANS_FEAT_NONSTREAMING(UZP2_q, aa64_sve_f64mm, do_interleave_q,
+                        gen_helper_sve2_uzp_q, a, 16)
 
 static gen_helper_gvec_3 * const trn_fns[4] = {
     gen_helper_sve_trn_b, gen_helper_sve_trn_h,
@@ -2392,10 +2409,10 @@ TRANS_FEAT(TRN1_z, aa64_sve, gen_gvec_ool_arg_zzz,
 TRANS_FEAT(TRN2_z, aa64_sve, gen_gvec_ool_arg_zzz,
            trn_fns[a->esz], a, 1 << a->esz)
 
-TRANS_FEAT(TRN1_q, aa64_sve_f64mm, gen_gvec_ool_arg_zzz,
-           gen_helper_sve2_trn_q, a, 0)
-TRANS_FEAT(TRN2_q, aa64_sve_f64mm, gen_gvec_ool_arg_zzz,
-           gen_helper_sve2_trn_q, a, 16)
+TRANS_FEAT_NONSTREAMING(TRN1_q, aa64_sve_f64mm, do_interleave_q,
+                        gen_helper_sve2_trn_q, a, 0)
+TRANS_FEAT_NONSTREAMING(TRN2_q, aa64_sve_f64mm, do_interleave_q,
+                        gen_helper_sve2_trn_q, a, 16)
 
 /*
  *** SVE Permute Vector - Predicated Group
-- 
2.43.0

Re: [PATCH v2 3/3] target/arm: Fix 128-bit element ZIP, UZP, TRN

[Peter Maydell]

On Sun, 22 Jun 2025 at 22:36, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> We missed the instructions UDEF when the vector size is too small.

I think this part also applies to FMMLA_d and trans_LD1RO_zprr
and trans_LD1RO_zprr ?

We should also probably put something in so that if the
user asks for a CPU with a max SVE VL of 128 then we disable
FEAT_F64MM.

> We missed marking the instructions non-streaming with SME.
>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/tcg/translate-sve.c | 43 ++++++++++++++++++++++++----------
>  1 file changed, 30 insertions(+), 13 deletions(-)

These code changes look correct to me.

thanks
-- PMM

Re: [PATCH v2 3/3] target/arm: Fix 128-bit element ZIP, UZP, TRN

[Richard Henderson]

On 6/24/25 07:59, Peter Maydell wrote:
> On Sun, 22 Jun 2025 at 22:36, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> We missed the instructions UDEF when the vector size is too small.
> 
> I think this part also applies to FMMLA_d and trans_LD1RO_zprr
> and trans_LD1RO_zprr ?

LD1RO is handled in do_ldro; the address calculation is done beforehand, but that's easily 
discarded with the exception.

Fixed FMMLA.

> We should also probably put something in so that if the
> user asks for a CPU with a max SVE VL of 128 then we disable
> FEAT_F64MM.

Done.


r~