[RFC PATCH 1/3] target/arm: Make smcr_write() handle SME-without-SVE

Peter Maydell posted 3 patches 1 week, 6 days ago
Maintainers: Peter Maydell <peter.maydell@linaro.org>
There is a newer version of this series
[RFC PATCH 1/3] target/arm: Make smcr_write() handle SME-without-SVE
Posted by Peter Maydell 1 week, 6 days ago
Currently smcr_write() implicitly assumes that SME implies SVE,
because it will call sve_vqm1_sve_for_el() when SMCR.SM is 0, and
sve_vqm1_sve_for_el() will assert in that situation.

This is the only place where we call that function without
it being guarded by a check on whether SVE is implemented.
Adjust smcr_write() so that it also avoids asking for the
SVE vector length when SVE is not implemented.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
I did think about making sve_vqm1_sve_for_el() return some
value rather than asserting, but (a) what would be the right
value? and (b) this was the only place that needed fixing.
---
 target/arm/helper.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index dce648b482..a3dd84a2d6 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -4936,12 +4936,12 @@ static void smcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
                        uint64_t value)
 {
     int cur_el = arm_current_el(env);
-    int old_len = sve_vqm1_for_el(env, cur_el);
+    ARMCPU *cpu = env_archcpu(env);
+    int old_len = cpu_isar_feature(aa64_sve, cpu) ? sve_vqm1_for_el(env, cur_el) : 0;
     uint64_t valid_mask = R_SMCR_LEN_MASK | R_SMCR_FA64_MASK;
-    int new_len;
 
     QEMU_BUILD_BUG_ON(ARM_MAX_VQ > R_SMCR_LEN_MASK + 1);
-    if (cpu_isar_feature(aa64_sme2, env_archcpu(env))) {
+    if (cpu_isar_feature(aa64_sme2, cpu)) {
         valid_mask |= R_SMCR_EZT0_MASK;
     }
     value &= valid_mask;
@@ -4953,10 +4953,17 @@ static void smcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
      * current values for simplicity.  But for QEMU internals, we must still
      * apply the narrower SVL to the Zregs and Pregs -- see the comment
      * above aarch64_sve_narrow_vq.
+     *
+     * If the CPU has only SME and not SVE, then turning streaming mode
+     * on and off can't ever change the SVL; we must avoid calling
+     * sve_vqm1_for_el() to ask for the SVE vector length when SM is 0
+     * because it will assert.
      */
-    new_len = sve_vqm1_for_el(env, cur_el);
-    if (new_len < old_len) {
-        aarch64_sve_narrow_vq(env, new_len + 1);
+    if (cpu_isar_feature(aa64_sve, cpu)) {
+        int new_len = sve_vqm1_for_el(env, cur_el);
+        if (new_len < old_len) {
+            aarch64_sve_narrow_vq(env, new_len + 1);
+        }
     }
 }
 
-- 
2.43.0
Re: [RFC PATCH 1/3] target/arm: Make smcr_write() handle SME-without-SVE
Posted by Peter Maydell 1 week, 5 days ago
On Tue, 27 Jan 2026 at 15:48, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> Currently smcr_write() implicitly assumes that SME implies SVE,
> because it will call sve_vqm1_sve_for_el() when SMCR.SM is 0, and
> sve_vqm1_sve_for_el() will assert in that situation.
>
> This is the only place where we call that function without
> it being guarded by a check on whether SVE is implemented.
> Adjust smcr_write() so that it also avoids asking for the
> SVE vector length when SVE is not implemented.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> I did think about making sve_vqm1_sve_for_el() return some
> value rather than asserting, but (a) what would be the right
> value? and (b) this was the only place that needed fixing.
> ---
>  target/arm/helper.c | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index dce648b482..a3dd84a2d6 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -4936,12 +4936,12 @@ static void smcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>                         uint64_t value)
>  {
>      int cur_el = arm_current_el(env);
> -    int old_len = sve_vqm1_for_el(env, cur_el);
> +    ARMCPU *cpu = env_archcpu(env);
> +    int old_len = cpu_isar_feature(aa64_sve, cpu) ? sve_vqm1_for_el(env, cur_el) : 0;
>      uint64_t valid_mask = R_SMCR_LEN_MASK | R_SMCR_FA64_MASK;
> -    int new_len;
>
>      QEMU_BUILD_BUG_ON(ARM_MAX_VQ > R_SMCR_LEN_MASK + 1);
> -    if (cpu_isar_feature(aa64_sme2, env_archcpu(env))) {
> +    if (cpu_isar_feature(aa64_sme2, cpu)) {
>          valid_mask |= R_SMCR_EZT0_MASK;
>      }
>      value &= valid_mask;
> @@ -4953,10 +4953,17 @@ static void smcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>       * current values for simplicity.  But for QEMU internals, we must still
>       * apply the narrower SVL to the Zregs and Pregs -- see the comment
>       * above aarch64_sve_narrow_vq.
> +     *
> +     * If the CPU has only SME and not SVE, then turning streaming mode
> +     * on and off can't ever change the SVL; we must avoid calling
> +     * sve_vqm1_for_el() to ask for the SVE vector length when SM is 0
> +     * because it will assert.
>       */
> -    new_len = sve_vqm1_for_el(env, cur_el);
> -    if (new_len < old_len) {
> -        aarch64_sve_narrow_vq(env, new_len + 1);
> +    if (cpu_isar_feature(aa64_sve, cpu)) {
> +        int new_len = sve_vqm1_for_el(env, cur_el);
> +        if (new_len < old_len) {
> +            aarch64_sve_narrow_vq(env, new_len + 1);
> +        }
>      }

This isn't right -- I forgot that SMCR also has the SME vector
length, so old_len / new_len can be different here and we need
to call narrow_vq for that case.

Also aarch64_sve_narrow_vq() does this:

    assert(vq <= env_archcpu(env)->sve_max_vq);

and I think that should be asserting that the new vq is <=
MAX(sve_max_vq, sme_max_vq).

-- PMM