Re: [PATCH v4 2/2] target/arm: Added support for SME register exposure to GDB

["Alex Bennée" <alex.bennee@linaro.org>]

Vacha Bhavsar <vacha.bhavsar@oss.qualcomm.com> writes:

> Hi,
>
>  
>
> I've built a testcase for this similar to the one you suggested. This test 
>
> checks both reading and writing the za register via $za and via the tiles
>
> and tiles slices that gdb produces (i.e., za0hb0). However, these tiles and 
>
> slices are generated from the gdb side, they're not made available by any
>
> of the changes that I have implemented. But this feature of gdb's kicks in
>
> when using gdb14.1 or newer. Due to this, the testcase works correctly when
>
> used with gdb14.1 and above, and fails on any gdb version older than that
>
> as the tiles/slices are not made available by gdb.
>
>  
>
> I was wondering if there is any way to set a requirement on this testcase
>
> which specifies it needs to be run with minimum of gdb version 14.1
>
> which has the functionality to break down the ZA storage into tiles and 
>
> slices?

We have tests in configure to probe the gdbversion, e.g:

  if test "${gdb_arches#*aarch64}" != "$gdb_arches" && version_ge $gdb_version 15.1; then
      echo "GDB_HAS_MTE=y" >> $config_target_mak
  fi

which can then wrap the test in the Makefile, e.g.:

  ifeq ($(GDB_HAS_MTE),y)
  run-gdbstub-mte: mte-8
          $(call run-test, $@, $(GDB_SCRIPT) \
                  --gdb $(GDB) \
                  --qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
                  --bin $< --test $(AARCH64_SRC)/gdbstub/test-mte.py \
                  -- --mode=user, \
          gdbstub MTE support)

  EXTRA_RUNS += run-gdbstub-mte
  endif


>
>  
>
>  
>
> Thanks,
>
> Vacha
>
> On Mon, Aug 4, 2025 at 11:35 AM Alex Bennée <alex.bennee@linaro.org> wrote:
>
>  Vacha Bhavsar <vacha.bhavsar@oss.qualcomm.com> writes:
>
>  > The QEMU GDB stub does not expose the ZA storage SME register to GDB via
>  > the remote serial protocol, which can be a useful functionality to debug SME
>  > code. To provide this functionality in Aarch64 target, this patch registers the
>  > SME register set with the GDB stub. To do so, this patch implements the
>  > aarch64_gdb_get_sme_reg() and aarch64_gdb_set_sme_reg() functions to
>  > specify how to get and set the SME registers, and the
>  > arm_gen_dynamic_smereg_feature() function to generate the target
>  > description in XML format to indicate the target architecture supports SME.
>  > Finally, this patch includes a dyn_smereg_feature structure to hold this
>  > GDB XML description of the SME registers for each CPU.
>  >
>  > Signed-off-by: Vacha Bhavsar <vacha.bhavsar@oss.qualcomm.com>
>  > ---
>  > Changes since v3:
>  > - added changes to aarch64_gdb_set_sme_reg() to address the concerns 
>  > brought up in review regarding endianness
>  >
>  >  target/arm/cpu.h       |   1 +
>  >  target/arm/gdbstub.c   |   6 ++
>  >  target/arm/gdbstub64.c | 122 +++++++++++++++++++++++++++++++++++++++++
>  >  target/arm/internals.h |   3 +
>  >  4 files changed, 132 insertions(+)
>  >
>  > diff --git a/target/arm/cpu.h b/target/arm/cpu.h
>  > index dc9b6dce4c..8bd66d7049 100644
>  > --- a/target/arm/cpu.h
>  > +++ b/target/arm/cpu.h
>  > @@ -933,6 +933,7 @@ struct ArchCPU {
>  >  
>  >      DynamicGDBFeatureInfo dyn_sysreg_feature;
>  >      DynamicGDBFeatureInfo dyn_svereg_feature;
>  > +    DynamicGDBFeatureInfo dyn_smereg_feature;
>  >      DynamicGDBFeatureInfo dyn_m_systemreg_feature;
>  >      DynamicGDBFeatureInfo dyn_m_secextreg_feature;
>  >  
>  > diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
>  > index ce4497ad7c..9c942c77cc 100644
>  > --- a/target/arm/gdbstub.c
>  > +++ b/target/arm/gdbstub.c
>  > @@ -531,6 +531,12 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
>  >              GDBFeature *feature = arm_gen_dynamic_svereg_feature(cs, cs->gdb_num_regs);
>  >              gdb_register_coprocessor(cs, aarch64_gdb_get_sve_reg,
>  >                                       aarch64_gdb_set_sve_reg, feature, 0);
>  > +            if (isar_feature_aa64_sme(&cpu->isar)) {
>  > +                GDBFeature *sme_feature = arm_gen_dynamic_smereg_feature(cs,
>  > +                                             cs->gdb_num_regs);
>  > +                gdb_register_coprocessor(cs, aarch64_gdb_get_sme_reg,
>  > +                    aarch64_gdb_set_sme_reg, sme_feature, 0);
>  > +            }
>  >          } else {
>  >              gdb_register_coprocessor(cs, aarch64_gdb_get_fpu_reg,
>  >                                       aarch64_gdb_set_fpu_reg,
>  > diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
>  > index 64ee9b3b56..3d86980bc9 100644
>  > --- a/target/arm/gdbstub64.c
>  > +++ b/target/arm/gdbstub64.c
>  > @@ -228,6 +228,91 @@ int aarch64_gdb_set_sve_reg(CPUState *cs, uint8_t *buf, int reg)
>  >      return 0;
>  >  }
>  >  
>  > +int aarch64_gdb_get_sme_reg(CPUState *cs, GByteArray *buf, int reg)
>  > +{
>  > +    ARMCPU *cpu = ARM_CPU(cs);
>  > +    CPUARMState *env = &cpu->env;
>  > +
>  > +    switch (reg) {
>  > +    /* Svg register */
>  > +    case 0:
>  > +    {
>  > +        int vq = 0;
>  > +        if (FIELD_EX64(env->svcr, SVCR, SM)) {
>  > +            vq = sve_vqm1_for_el_sm(env, arm_current_el(env),
>  > +                     FIELD_EX64(env->svcr, SVCR, SM)) + 1;
>  > +        }
>  > +        /* svg = vector granules (2 * vector quardwords) in streaming mode */
>  > +        return gdb_get_reg64(buf, vq * 2);
>  > +    }
>  > +    case 1:
>  > +        return gdb_get_reg64(buf, env->svcr);
>  > +    case 2:
>  > +    {
>  > +        int len = 0;
>  > +        int vq = cpu->sme_max_vq;
>  > +        int svl = vq * 16;
>  > +        for (int i = 0; i < svl; i++) {
>  > +            for (int q = 0; q < vq; q++) {
>  > +                len += gdb_get_reg128(buf,
>  > +                           env->za_state.za⚠️[i].d[q * 2 + 1],
>  > +                           env->za_state.za⚠️[i].d[q * 2]);
>  > +            }
>  > +        }
>  > +        return len;
>  > +    }
>  > +    default:
>  > +        /* gdbstub asked for something out of range */
>  > +        qemu_log_mask(LOG_UNIMP, "%s: out of range register %d", __func__, reg);
>  > +        break;
>  > +    }
>  > +
>  > +    return 0;
>  > +}
>  > +
>  > +int aarch64_gdb_set_sme_reg(CPUState *cs, uint8_t *buf, int reg)
>  > +{
>  > +    ARMCPU *cpu = ARM_CPU(cs);
>  > +    CPUARMState *env = &cpu->env;
>  > +
>  > +    switch (reg) {
>  > +    case 0:
>  > +    {
>  > +        /* cannot set svg via gdbstub */
>  > +        return 8;
>  > +    }
>  > +    case 1:
>  > +        aarch64_set_svcr(env, ldq_le_p(buf),
>  > +            R_SVCR_SM_MASK | R_SVCR_ZA_MASK);
>  > +        return 8;
>  > +    case 2:
>  > +        int len = 0;
>  > +        int vq = cpu->sme_max_vq;
>  > +        int svl = vq * 16;
>  > +        for (int i = 0; i < svl; i++) {
>  > +            for (int q = 0; q < vq; q++) {
>  > +                if (target_big_endian()){
>  > +                    env->za_state.za⚠️[i].d[q * 2 + 1] = ldq_p(buf);
>  > +                    buf += 8;
>  > +                    env->za_state.za⚠️[i].d[q * 2] = ldq_p(buf);
>  > +                } else{
>  > +                    env->za_state.za⚠️[i].d[q * 2] = ldq_p(buf);
>  > +                    buf += 8;
>  > +                    env->za_state.za⚠️[i].d[q * 2 + 1] = ldq_p(buf);
>  > +                }
>  > +                buf += 8;
>  > +                len += 16;
>  > +            }
>  > +        }
>  > +        return len;
>  > +    default:
>  > +        /* gdbstub asked for something out of range */
>  > +        break;
>  > +    }
>  > +
>  > +    return 0;
>  > +}
>  > +
>  >  int aarch64_gdb_get_pauth_reg(CPUState *cs, GByteArray *buf, int reg)
>  >  {
>  >      ARMCPU *cpu = ARM_CPU(cs);
>  > @@ -392,6 +477,43 @@ GDBFeature *arm_gen_dynamic_svereg_feature(CPUState *cs, int base_reg)
>  >      return &cpu->dyn_svereg_feature.desc;
>  >  }
>  >  
>  > +GDBFeature *arm_gen_dynamic_smereg_feature(CPUState *cs, int base_reg)
>  > +{
>  > +    ARMCPU *cpu = ARM_CPU(cs);
>  > +    int vq = cpu->sme_max_vq;
>  > +    int svl = vq * 16;
>  > +    GDBFeatureBuilder builder;
>  > +    int reg = 0;
>  > +
>  > +    gdb_feature_builder_init(&builder, &cpu->dyn_smereg_feature.desc,
>  > +        "org.gnu.gdb.aarch64.sme", "sme-registers.xml", base_reg);
>  > +
>  > +
>  > +    /* Create the sme_bv vector type. */
>  > +    gdb_feature_builder_append_tag(&builder,
>  > +        "<vector id=\"sme_bv\" type=\"uint8\" count=\"%d\"/>",
>  > +        svl);
>  > +
>  > +    /* Create the sme_bvv vector type. */
>  > +    gdb_feature_builder_append_tag(
>  > +        &builder, "<vector id=\"sme_bvv\" type=\"sme_bv\" count=\"%d\"/>",
>  > +        svl);
>  > +
>  > +    /* Define the svg, svcr, and za registers. */
>  > +
>  > +    /* fpscr & status registers */
>  > +    gdb_feature_builder_append_reg(&builder, "svg", 64, reg++,
>  > +        "int", NULL);
>  > +    gdb_feature_builder_append_reg(&builder, "svcr", 64, reg++,
>  > +        "int", NULL);
>  > +    gdb_feature_builder_append_reg(&builder, "za", svl * svl * 8, reg++,
>  > +        "sme_bvv", NULL);
>  > +
>  > +    gdb_feature_builder_end(&builder);
>  > +
>  > +    return &cpu->dyn_smereg_feature.desc;
>  > +}
>  > +
>  >  #ifdef CONFIG_USER_ONLY
>  >  int aarch64_gdb_get_tag_ctl_reg(CPUState *cs, GByteArray *buf, int reg)
>  >  {
>  > diff --git a/target/arm/internals.h b/target/arm/internals.h
>  > index c4765e4489..760e1c6490 100644
>  > --- a/target/arm/internals.h
>  > +++ b/target/arm/internals.h
>  > @@ -1808,8 +1808,11 @@ static inline uint64_t pmu_counter_mask(CPUARMState *env)
>  >  }
>  >  
>  >  GDBFeature *arm_gen_dynamic_svereg_feature(CPUState *cpu, int base_reg);
>  > +GDBFeature *arm_gen_dynamic_smereg_feature(CPUState *cpu, int base_reg);
>  >  int aarch64_gdb_get_sve_reg(CPUState *cs, GByteArray *buf, int reg);
>  >  int aarch64_gdb_set_sve_reg(CPUState *cs, uint8_t *buf, int reg);
>  > +int aarch64_gdb_get_sme_reg(CPUState *cs, GByteArray *buf, int reg);
>  > +int aarch64_gdb_set_sme_reg(CPUState *cs, uint8_t *buf, int reg);
>  >  int aarch64_gdb_get_fpu_reg(CPUState *cs, GByteArray *buf, int reg);
>  >  int aarch64_gdb_set_fpu_reg(CPUState *cs, uint8_t *buf, int reg);
>  >  int aarch64_gdb_get_pauth_reg(CPUState *cs, GByteArray *buf, int reg);
>
>  It would also be nice to add a test for this, see tests/tcg/aarch64/gdbstub/test-sve.py
>
>  -- 
>  Alex Bennée
>  Virtualisation Tech Lead @ Linaro

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro