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 <[email protected]> wrote: > On Tue, 26 Aug 2025 at 19:50, Vacha Bhavsar > <[email protected]> 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 <[email protected]> > > --- > > 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 >
