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>
---
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..4371a367a0 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 08e2858539..047b1f8133 100644
--- a/target/arm/gdbstub64.c
+++ b/target/arm/gdbstub64.c
@@ -249,6 +249,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);
@@ -413,6 +498,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 1b3d0244fd..41e05066b9 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1802,8 +1802,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);
--
2.34.1
On Mon, 11 Aug 2025 at 20:37, Vacha Bhavsar
<vacha.bhavsar@oss.qualcomm.com> wrote:
>
> 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>
> ---
> 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..4371a367a0 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);
> + }
It's possible to have SME without SVE, so this should not be
inside the "if we have SVE" check.
> } 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 08e2858539..047b1f8133 100644
> --- a/target/arm/gdbstub64.c
> +++ b/target/arm/gdbstub64.c
> @@ -249,6 +249,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:
We should comment what all the cases here are; the usual ploce to
put such a comment is either on the same line as the case, or else
on the line immediately after the case if the comment would be
too long to put on the same line.
The capitalization here is odd, too: the register is either "SVG" or "svg".
> + {
> + 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;
> + }
You don't need braces here, because there are no local variables
in this arm of the switch statement.
> + 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;
But you do need braces here, because there are local variables.
This is (a) because our coding style requires that declarations
go at the top of a block and (b) because clang will complain:
../../target/arm/gdbstub64.c:310:9: error: label followed by a
declaration is a C23 extension [-Werror,-Wc23-extensions]
310 | int len = 0;
| ^
> + for (int i = 0; i < svl; i++) {
> + for (int q = 0; q < vq; q++) {
> + if (target_big_endian()){
Missing space before the {
> + 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);
> @@ -413,6 +498,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 1b3d0244fd..41e05066b9 100644
> --- a/target/arm/internals.h
> +++ b/target/arm/internals.h
> @@ -1802,8 +1802,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);
> --
thanks
-- PMM
Hi,
Regarding having the SME check inside the "if we have SVE" check, we were
looking the the
Arm ARM, specifically the following excerpt from section A1.4:
The architecture provides the following:
- A general-purpose register file.
- A SIMD&FP register file.
- If FEAT_SVE or FEAT_SME is implemented, an SVE scalable vector register
file and an
SVE scalable predicate register file.
- if FEAT_SME is implemented, the scalable ZA storage.
Based on this, we were considering the following update to the change in
gdbstub64.c and
we wanted to get your input.
if (isar_feature_aa64_sve(&cpu->isar) || isar_feature_aa64_sme(&cpu->isar))
{
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);
} else {
gdb_register_coprocessor(cs, aarch64_gdb_get_fpu_reg,
aarch64_gdb_set_fpu_reg,
gdb_find_static_feature("aarch64-fpu.xml"),
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);
}
Thanks,
Vacha
On Tue, Aug 19, 2025 at 5:05 AM Peter Maydell <peter.maydell@linaro.org>
wrote:
> On Mon, 11 Aug 2025 at 20:37, Vacha Bhavsar
> <vacha.bhavsar@oss.qualcomm.com> wrote:
> >
> > 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>
> > ---
> > 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..4371a367a0 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);
> > + }
>
> It's possible to have SME without SVE, so this should not be
> inside the "if we have SVE" check.
>
> > } 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 08e2858539..047b1f8133 100644
> > --- a/target/arm/gdbstub64.c
> > +++ b/target/arm/gdbstub64.c
> > @@ -249,6 +249,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:
>
> We should comment what all the cases here are; the usual ploce to
> put such a comment is either on the same line as the case, or else
> on the line immediately after the case if the comment would be
> too long to put on the same line.
>
> The capitalization here is odd, too: the register is either "SVG" or "svg".
>
> > + {
> > + 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;
> > + }
>
> You don't need braces here, because there are no local variables
> in this arm of the switch statement.
>
> > + 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;
>
> But you do need braces here, because there are local variables.
> This is (a) because our coding style requires that declarations
> go at the top of a block and (b) because clang will complain:
>
> ../../target/arm/gdbstub64.c:310:9: error: label followed by a
> declaration is a C23 extension [-Werror,-Wc23-extensions]
> 310 | int len = 0;
> | ^
>
> > + for (int i = 0; i < svl; i++) {
> > + for (int q = 0; q < vq; q++) {
> > + if (target_big_endian()){
>
> Missing space before the {
>
> > + 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);
> > @@ -413,6 +498,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 1b3d0244fd..41e05066b9 100644
> > --- a/target/arm/internals.h
> > +++ b/target/arm/internals.h
> > @@ -1802,8 +1802,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);
> > --
>
> thanks
> -- PMM
>
On Thu, 21 Aug 2025 at 22:37, Vacha Bhavsar
<vacha.bhavsar@oss.qualcomm.com> wrote:
>
> Hi,
>
> Regarding having the SME check inside the "if we have SVE" check, we were looking the the
> Arm ARM, specifically the following excerpt from section A1.4:
>
> The architecture provides the following:
>
> - A general-purpose register file.
> - A SIMD&FP register file.
> - If FEAT_SVE or FEAT_SME is implemented, an SVE scalable vector register file and an
> SVE scalable predicate register file.
> - if FEAT_SME is implemented, the scalable ZA storage.
>
> Based on this, we were considering the following update to the change in gdbstub64.c and
> we wanted to get your input.
>
> if (isar_feature_aa64_sve(&cpu->isar) || isar_feature_aa64_sme(&cpu->isar)) {
> 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);
> } else {
> gdb_register_coprocessor(cs, aarch64_gdb_get_fpu_reg,
> aarch64_gdb_set_fpu_reg,
> gdb_find_static_feature("aarch64-fpu.xml"),
> 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);
> }
Yes, I think that will be right.
thanks
-- PMM
Hi,
I have sent a new version of this patch series addressing
these comments.
Looking forward to your feedback.
Thanks,
Vacha
On Fri, Aug 22, 2025 at 8:27 AM Peter Maydell <peter.maydell@linaro.org>
wrote:
> On Thu, 21 Aug 2025 at 22:37, Vacha Bhavsar
> <vacha.bhavsar@oss.qualcomm.com> wrote:
> >
> > Hi,
> >
> > Regarding having the SME check inside the "if we have SVE" check, we
> were looking the the
> > Arm ARM, specifically the following excerpt from section A1.4:
> >
> > The architecture provides the following:
> >
> > - A general-purpose register file.
> > - A SIMD&FP register file.
> > - If FEAT_SVE or FEAT_SME is implemented, an SVE scalable vector
> register file and an
> > SVE scalable predicate register file.
> > - if FEAT_SME is implemented, the scalable ZA storage.
> >
> > Based on this, we were considering the following update to the change in
> gdbstub64.c and
> > we wanted to get your input.
> >
> > if (isar_feature_aa64_sve(&cpu->isar) ||
> isar_feature_aa64_sme(&cpu->isar)) {
> > 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);
> > } else {
> > gdb_register_coprocessor(cs, aarch64_gdb_get_fpu_reg,
> > aarch64_gdb_set_fpu_reg,
> >
> gdb_find_static_feature("aarch64-fpu.xml"),
> > 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);
> > }
>
> Yes, I think that will be right.
>
> thanks
> -- PMM
>
© 2016 - 2026 Red Hat, Inc.