* [PATCH v6 2/3] target/arm: Added support for SME register exposure to GDB
2025-08-26 18:45 [PATCH v6 0/3] " Vacha Bhavsar
@ 2025-08-26 18:45 ` Vacha Bhavsar
0 siblings, 0 replies; 4+ messages in thread
From: Vacha Bhavsar @ 2025-08-26 18:45 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-arm, Philippe Mathieu-Daudé, Alex Bennée,
Paolo Bonzini, Peter Maydell, Thomas Huth, Vacha Bhavsar
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 v5:
- added code to handle the case when we have SME without SVE
- added comments to indicate th cases in aarch64_gdb_get/set_sme_reg
- added/removed braces where necessary
- corrected capitalization in comments
---
target/arm/cpu.h | 1 +
target/arm/gdbstub.c | 9 ++-
target/arm/gdbstub64.c | 121 +++++++++++++++++++++++++++++++++++++++++
target/arm/internals.h | 3 +
4 files changed, 133 insertions(+), 1 deletion(-)
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..110258ec18 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -527,7 +527,7 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
* registers so we don't need to include both.
*/
#ifdef TARGET_AARCH64
- if (isar_feature_aa64_sve(&cpu->isar)) {
+ 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);
@@ -537,6 +537,13 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
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);
+ }
/*
* Note that we report pauth information via the feature name
* org.gnu.gdb.aarch64.pauth_v2, not org.gnu.gdb.aarch64.pauth.
diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
index 08e2858539..d3fd94b93d 100644
--- a/target/arm/gdbstub64.c
+++ b/target/arm/gdbstub64.c
@@ -249,6 +249,90 @@ 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) {
+ case 0: /* svg register */
+ {
+ 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: /* svcr register */
+ return gdb_get_reg64(buf, env->svcr);
+ case 2: /* za register */
+ {
+ 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: /* svg register */
+ /* cannot set svg via gdbstub */
+ return 8;
+ case 1: /* svcr register */
+ aarch64_set_svcr(env, ldq_le_p(buf),
+ R_SVCR_SM_MASK | R_SVCR_ZA_MASK);
+ return 8;
+ case 2: /* za register */
+ {
+ 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 +497,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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v6 2/3] target/arm: Added support for SME register exposure to GDB
@ 2025-08-26 18:50 Vacha Bhavsar
2025-09-02 10:45 ` Peter Maydell
0 siblings, 1 reply; 4+ messages in thread
From: Vacha Bhavsar @ 2025-08-26 18:50 UTC (permalink / raw)
To: qemu-devel
Cc: Philippe Mathieu-Daudé, Peter Maydell, qemu-arm,
Alex Bennée, Paolo Bonzini, Thomas Huth, Vacha Bhavsar
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 v5:
- added code to handle the case when we have SME without SVE
- added comments to indicate th cases in aarch64_gdb_get/set_sme_reg
- added/removed braces where necessary
- corrected capitalization in comments
---
target/arm/cpu.h | 1 +
target/arm/gdbstub.c | 9 ++-
target/arm/gdbstub64.c | 121 +++++++++++++++++++++++++++++++++++++++++
target/arm/internals.h | 3 +
4 files changed, 133 insertions(+), 1 deletion(-)
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..110258ec18 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -527,7 +527,7 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
* registers so we don't need to include both.
*/
#ifdef TARGET_AARCH64
- if (isar_feature_aa64_sve(&cpu->isar)) {
+ 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);
@@ -537,6 +537,13 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
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);
+ }
/*
* Note that we report pauth information via the feature name
* org.gnu.gdb.aarch64.pauth_v2, not org.gnu.gdb.aarch64.pauth.
diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
index 08e2858539..d3fd94b93d 100644
--- a/target/arm/gdbstub64.c
+++ b/target/arm/gdbstub64.c
@@ -249,6 +249,90 @@ 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) {
+ case 0: /* svg register */
+ {
+ 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: /* svcr register */
+ return gdb_get_reg64(buf, env->svcr);
+ case 2: /* za register */
+ {
+ 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: /* svg register */
+ /* cannot set svg via gdbstub */
+ return 8;
+ case 1: /* svcr register */
+ aarch64_set_svcr(env, ldq_le_p(buf),
+ R_SVCR_SM_MASK | R_SVCR_ZA_MASK);
+ return 8;
+ case 2: /* za register */
+ {
+ 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 +497,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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v6 2/3] target/arm: Added support for SME register exposure to GDB
2025-08-26 18:50 [PATCH v6 2/3] target/arm: Added support for SME register exposure to GDB Vacha Bhavsar
@ 2025-09-02 10:45 ` Peter Maydell
2025-09-02 17:52 ` Vacha Bhavsar
0 siblings, 1 reply; 4+ messages in thread
From: Peter Maydell @ 2025-09-02 10:45 UTC (permalink / raw)
To: Vacha Bhavsar
Cc: qemu-devel, Philippe Mathieu-Daudé, qemu-arm,
Alex Bennée, Paolo Bonzini, Thomas Huth
On Tue, 26 Aug 2025 at 19:50, 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>
> ---
> Changes since v5:
> - added code to handle the case when we have SME without SVE
> - added comments to indicate th cases in aarch64_gdb_get/set_sme_reg
> - added/removed braces where necessary
> - corrected capitalization in comments
> ---
> target/arm/cpu.h | 1 +
> target/arm/gdbstub.c | 9 ++-
> target/arm/gdbstub64.c | 121 +++++++++++++++++++++++++++++++++++++++++
> target/arm/internals.h | 3 +
> 4 files changed, 133 insertions(+), 1 deletion(-)
>
> 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..110258ec18 100644
> --- a/target/arm/gdbstub.c
> +++ b/target/arm/gdbstub.c
> @@ -527,7 +527,7 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
> * registers so we don't need to include both.
> */
> #ifdef TARGET_AARCH64
> - if (isar_feature_aa64_sve(&cpu->isar)) {
> + 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);
> @@ -537,6 +537,13 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
> 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);
Your indent here and below for function calls on multiple
lines is wrong -- follow the way the existing code does it,
where the second line lines up with the first argument
after the '('. (We sometimes make an exception where the wrapping
would look terrible, but this is the usual approach.)
> + gdb_register_coprocessor(cs, aarch64_gdb_get_sme_reg,
> + aarch64_gdb_set_sme_reg, sme_feature, 0);
> + }
> /*
> * Note that we report pauth information via the feature name
> * org.gnu.gdb.aarch64.pauth_v2, not org.gnu.gdb.aarch64.pauth.
> diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
> index 08e2858539..d3fd94b93d 100644
> --- a/target/arm/gdbstub64.c
> +++ b/target/arm/gdbstub64.c
> @@ -249,6 +249,90 @@ 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) {
> + case 0: /* svg register */
> + {
> + 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: /* svcr register */
> + return gdb_get_reg64(buf, env->svcr);
> + case 2: /* za register */
> + {
> + 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: /* svg register */
> + /* cannot set svg via gdbstub */
> + return 8;
> + case 1: /* svcr register */
> + aarch64_set_svcr(env, ldq_le_p(buf),
> + R_SVCR_SM_MASK | R_SVCR_ZA_MASK);
> + return 8;
> + case 2: /* za register */
> + {
> + 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 +497,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);
https://sourceware.org/gdb/current/onlinedocs/gdb.html/AArch64-Features.html#AArch64-Features
says ZA should be a vector of bytes, not a vector of a vector of bytes.
Is it wrong ?
> +
> + /* Define the svg, svcr, and za registers. */
> +
> + /* fpscr & status registers */
This comment seems to be wrong and can be deleted.
> + 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);
We will also want to have support for the org.gnu.gdb.aarch64.sme2
feature (which has the ZT0 register), but we can add that as
a separate patch later.
> +
> + 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)
> {
thanks
-- PMM
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v6 2/3] target/arm: Added support for SME register exposure to GDB
2025-09-02 10:45 ` Peter Maydell
@ 2025-09-02 17:52 ` Vacha Bhavsar
0 siblings, 0 replies; 4+ messages in thread
From: Vacha Bhavsar @ 2025-09-02 17:52 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, Philippe Mathieu-Daudé, qemu-arm,
Alex Bennée, Paolo Bonzini, Thomas Huth
[-- Attachment #1: Type: text/plain, Size: 9549 bytes --]
Hi,
Regarding the definition of ZA as a 'vector of bytes' in the gdb
documentation, the choice that we have made in representing it
as a vector of vectors of bytes is based on the xml retrieved by
the native gdb client when run on a host with SME capabilities.
Is it sufficient to document this discrepancy in the commit message?
Thanks,
Vacha
On Tue, Sep 2, 2025 at 6:45 AM Peter Maydell <peter.maydell@linaro.org>
wrote:
> On Tue, 26 Aug 2025 at 19:50, 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>
> > ---
> > Changes since v5:
> > - added code to handle the case when we have SME without SVE
> > - added comments to indicate th cases in aarch64_gdb_get/set_sme_reg
> > - added/removed braces where necessary
> > - corrected capitalization in comments
> > ---
> > target/arm/cpu.h | 1 +
> > target/arm/gdbstub.c | 9 ++-
> > target/arm/gdbstub64.c | 121 +++++++++++++++++++++++++++++++++++++++++
> > target/arm/internals.h | 3 +
> > 4 files changed, 133 insertions(+), 1 deletion(-)
> >
> > 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..110258ec18 100644
> > --- a/target/arm/gdbstub.c
> > +++ b/target/arm/gdbstub.c
> > @@ -527,7 +527,7 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU
> *cpu)
> > * registers so we don't need to include both.
> > */
> > #ifdef TARGET_AARCH64
> > - if (isar_feature_aa64_sve(&cpu->isar)) {
> > + 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);
> > @@ -537,6 +537,13 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU
> *cpu)
> >
> 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);
>
> Your indent here and below for function calls on multiple
> lines is wrong -- follow the way the existing code does it,
> where the second line lines up with the first argument
> after the '('. (We sometimes make an exception where the wrapping
> would look terrible, but this is the usual approach.)
>
> > + gdb_register_coprocessor(cs, aarch64_gdb_get_sme_reg,
> > + aarch64_gdb_set_sme_reg, sme_feature, 0);
> > + }
> > /*
> > * Note that we report pauth information via the feature name
> > * org.gnu.gdb.aarch64.pauth_v2, not org.gnu.gdb.aarch64.pauth.
> > diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
> > index 08e2858539..d3fd94b93d 100644
> > --- a/target/arm/gdbstub64.c
> > +++ b/target/arm/gdbstub64.c
> > @@ -249,6 +249,90 @@ 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) {
> > + case 0: /* svg register */
> > + {
> > + 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: /* svcr register */
> > + return gdb_get_reg64(buf, env->svcr);
> > + case 2: /* za register */
> > + {
> > + 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: /* svg register */
> > + /* cannot set svg via gdbstub */
> > + return 8;
> > + case 1: /* svcr register */
> > + aarch64_set_svcr(env, ldq_le_p(buf),
> > + R_SVCR_SM_MASK | R_SVCR_ZA_MASK);
> > + return 8;
> > + case 2: /* za register */
> > + {
> > + 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 +497,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);
>
>
> https://sourceware.org/gdb/current/onlinedocs/gdb.html/AArch64-Features.html#AArch64-Features
>
> says ZA should be a vector of bytes, not a vector of a vector of bytes.
> Is it wrong ?
>
> > +
> > + /* Define the svg, svcr, and za registers. */
> > +
> > + /* fpscr & status registers */
>
> This comment seems to be wrong and can be deleted.
>
> > + 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);
>
> We will also want to have support for the org.gnu.gdb.aarch64.sme2
> feature (which has the ZT0 register), but we can add that as
> a separate patch later.
>
> > +
> > + 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)
> > {
>
> thanks
> -- PMM
>
[-- Attachment #2: Type: text/html, Size: 12703 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-09-02 17:53 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-26 18:50 [PATCH v6 2/3] target/arm: Added support for SME register exposure to GDB Vacha Bhavsar
2025-09-02 10:45 ` Peter Maydell
2025-09-02 17:52 ` Vacha Bhavsar
-- strict thread matches above, loose matches on Subject: below --
2025-08-26 18:45 [PATCH v6 0/3] " Vacha Bhavsar
2025-08-26 18:45 ` [PATCH v6 2/3] " Vacha Bhavsar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).