* [PATCH v5 0/4] target/arm: Added support for SME register exposure to GDB
@ 2025-08-11 19:36 Vacha Bhavsar
2025-08-11 19:36 ` [PATCH v5 1/4] target/arm: Increase MAX_PACKET_LENGTH for SME ZA Vacha Bhavsar
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Vacha Bhavsar @ 2025-08-11 19:36 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, qemu-arm, Philippe Mathieu-Daudé,
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.
Additionally, this patch series increases the value of MAX_PACKET_LENGTH
to allow for remote GDB debugging of the ZA register when the vector
length is maximal. Furthermore, based on suggestions from v4 review, this
patch also changes GDBState's line_buf to a dynamically re-sizeable GString.
This patch now also includes a test case for testing SME register exposure
to GDB, based off of the existing SVE test case for the gdbstub.
Vacha Bhavsar (4):
target/arm: Increase MAX_PACKET_LENGTH for SME ZA remote gdb debugging
target/arm: Change GDBState's line_buf to a GString
target/arm: Added support for SME register exposure to GDB
target/arm: Added test case for SME register exposure to GDB
configure | 6 ++
gdbstub/gdbstub.c | 25 +++---
gdbstub/internals.h | 25 +++++-
target/arm/cpu.h | 1 +
target/arm/gdbstub.c | 6 ++
target/arm/gdbstub64.c | 122 ++++++++++++++++++++++++++
target/arm/internals.h | 3 +
tests/tcg/aarch64/Makefile.target | 23 ++++-
tests/tcg/aarch64/gdbstub/test-sme.py | 119 +++++++++++++++++++++++++
9 files changed, 314 insertions(+), 16 deletions(-)
create mode 100644 tests/tcg/aarch64/gdbstub/test-sme.py
--
2.34.1
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH v5 1/4] target/arm: Increase MAX_PACKET_LENGTH for SME ZA 2025-08-11 19:36 [PATCH v5 0/4] target/arm: Added support for SME register exposure to GDB Vacha Bhavsar @ 2025-08-11 19:36 ` Vacha Bhavsar 2025-08-19 9:14 ` Peter Maydell 2025-08-11 19:36 ` [PATCH v5 2/4] target/arm: Change GDBState's line_buf to a GString Vacha Bhavsar ` (2 subsequent siblings) 3 siblings, 1 reply; 16+ messages in thread From: Vacha Bhavsar @ 2025-08-11 19:36 UTC (permalink / raw) To: qemu-devel Cc: Peter Maydell, qemu-arm, Philippe Mathieu-Daudé, Alex Bennée, Paolo Bonzini, Thomas Huth, Vacha Bhavsar This patch increases the value of the MAX_PACKET_LEGNTH to 131104 from 4096 to allow the GDBState.line_buf to be large enough to accommodate the full contents of the SME ZA storage when the vector length is maximal. This is in preparation for a related patch that allows SME register visibility through remote GDB debugging. Signed-off-by: Vacha Bhavsar <vacha.bhavsar@oss.qualcomm.com> --- Changes since v4: - the value for MAX_PACKET_LENGTH is changed from 131100 to 131104 to align with a similar update made to gdbserver --- gdbstub/internals.h | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/gdbstub/internals.h b/gdbstub/internals.h index bf5a5c6302..87f64b6318 100644 --- a/gdbstub/internals.h +++ b/gdbstub/internals.h @@ -11,7 +11,27 @@ #include "exec/cpu-common.h" -#define MAX_PACKET_LENGTH 4096 +/* +* Most "large" transfers (e.g. memory reads, feature XML +* transfer) have mechanisms in the gdb protocol for splitting +* them. However, register values in particular cannot currently +* be split. This packet size must therefore be at least big enough +* for the worst-case register size. Currently that is Arm SME +* ZA storage with a 256x256 byte value. We also must account +* for the conversion from raw data to hex in gdb_memtohex(), +* which writes 2 * size bytes, and for other protocol overhead +* including command, register number and checksum which add +* another 4 bytes of overhead. However, to be consistent with +* the changes made in gdbserver to address this same requirement, +* we add a total of 32 bytes to account for protocol overhead +* (unclear why specifically 32 bytes), bringing the value of +* MAX_PACKET_LENGTH to 2 * 256 * 256 + 32 = 131104. +* +* The commit making this change for gdbserver can be found here: +* https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h= +* b816042e88583f280ad186ff124ab84d31fb592b +*/ +#define MAX_PACKET_LENGTH 131104 /* * Shared structures and definitions -- 2.34.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v5 1/4] target/arm: Increase MAX_PACKET_LENGTH for SME ZA 2025-08-11 19:36 ` [PATCH v5 1/4] target/arm: Increase MAX_PACKET_LENGTH for SME ZA Vacha Bhavsar @ 2025-08-19 9:14 ` Peter Maydell 0 siblings, 0 replies; 16+ messages in thread From: Peter Maydell @ 2025-08-19 9:14 UTC (permalink / raw) To: Vacha Bhavsar Cc: qemu-devel, qemu-arm, Philippe Mathieu-Daudé, Alex Bennée, Paolo Bonzini, Thomas Huth On Mon, 11 Aug 2025 at 20:37, Vacha Bhavsar <vacha.bhavsar@oss.qualcomm.com> wrote: > > This patch increases the value of the MAX_PACKET_LEGNTH to > 131104 from 4096 to allow the GDBState.line_buf to be large enough > to accommodate the full contents of the SME ZA storage when the > vector length is maximal. This is in preparation for a related > patch that allows SME register visibility through remote GDB > debugging. > > Signed-off-by: Vacha Bhavsar <vacha.bhavsar@oss.qualcomm.com> > --- > Changes since v4: > - the value for MAX_PACKET_LENGTH is changed from 131100 to > 131104 to align with a similar update made to gdbserver > --- > gdbstub/internals.h | 22 +++++++++++++++++++++- > 1 file changed, 21 insertions(+), 1 deletion(-) > > diff --git a/gdbstub/internals.h b/gdbstub/internals.h > index bf5a5c6302..87f64b6318 100644 > --- a/gdbstub/internals.h > +++ b/gdbstub/internals.h > @@ -11,7 +11,27 @@ > > #include "exec/cpu-common.h" > > -#define MAX_PACKET_LENGTH 4096 > +/* > +* Most "large" transfers (e.g. memory reads, feature XML > +* transfer) have mechanisms in the gdb protocol for splitting > +* them. However, register values in particular cannot currently > +* be split. This packet size must therefore be at least big enough > +* for the worst-case register size. Currently that is Arm SME > +* ZA storage with a 256x256 byte value. We also must account > +* for the conversion from raw data to hex in gdb_memtohex(), > +* which writes 2 * size bytes, and for other protocol overhead > +* including command, register number and checksum which add > +* another 4 bytes of overhead. However, to be consistent with > +* the changes made in gdbserver to address this same requirement, > +* we add a total of 32 bytes to account for protocol overhead > +* (unclear why specifically 32 bytes), bringing the value of > +* MAX_PACKET_LENGTH to 2 * 256 * 256 + 32 = 131104. > +* > +* The commit making this change for gdbserver can be found here: > +* https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h= > +* b816042e88583f280ad186ff124ab84d31fb592b > +*/ > +#define MAX_PACKET_LENGTH 131104 Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v5 2/4] target/arm: Change GDBState's line_buf to a GString 2025-08-11 19:36 [PATCH v5 0/4] target/arm: Added support for SME register exposure to GDB Vacha Bhavsar 2025-08-11 19:36 ` [PATCH v5 1/4] target/arm: Increase MAX_PACKET_LENGTH for SME ZA Vacha Bhavsar @ 2025-08-11 19:36 ` Vacha Bhavsar 2025-08-12 11:32 ` Peter Maydell 2025-08-11 19:36 ` [PATCH v5 3/4] target/arm: Added support for SME register exposure to Vacha Bhavsar 2025-08-11 19:36 ` [PATCH v5 4/4] target/arm: Added test case for SME register exposure Vacha Bhavsar 3 siblings, 1 reply; 16+ messages in thread From: Vacha Bhavsar @ 2025-08-11 19:36 UTC (permalink / raw) To: qemu-devel Cc: Peter Maydell, qemu-arm, Philippe Mathieu-Daudé, Alex Bennée, Paolo Bonzini, Thomas Huth, Vacha Bhavsar This patch changes GDBState's line_buf from a character array to a GString. This allows line_buf to be dynamically re-sizeable. Signed-off-by: Vacha Bhavsar <vacha.bhavsar@oss.qualcomm.com> --- Changes since v4: - this patch was not present in v4, it has been added as suggested during review of v4 --- gdbstub/gdbstub.c | 25 +++++++++++++------------ gdbstub/internals.h | 3 +-- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c index dd5fb5667c..0634ff9e91 100644 --- a/gdbstub/gdbstub.c +++ b/gdbstub/gdbstub.c @@ -64,6 +64,7 @@ void gdb_init_gdbserver_state(void) memset(&gdbserver_state, 0, sizeof(GDBState)); gdbserver_state.init = true; gdbserver_state.str_buf = g_string_new(NULL); + gdbserver_state.line_buf = g_string_new(NULL); gdbserver_state.mem_buf = g_byte_array_sized_new(MAX_PACKET_LENGTH); gdbserver_state.last_packet = g_byte_array_sized_new(MAX_PACKET_LENGTH + 4); @@ -2369,7 +2370,7 @@ void gdb_read_byte(uint8_t ch) case RS_IDLE: if (ch == '$') { /* start of command packet */ - gdbserver_state.line_buf_index = 0; + g_string_set_size(gdbserver_state.line_buf, 0); gdbserver_state.line_sum = 0; gdbserver_state.state = RS_GETLINE; } else if (ch == '+') { @@ -2393,12 +2394,12 @@ void gdb_read_byte(uint8_t ch) } else if (ch == '#') { /* end of command, start of checksum*/ gdbserver_state.state = RS_CHKSUM1; - } else if (gdbserver_state.line_buf_index >= sizeof(gdbserver_state.line_buf) - 1) { + } else if (gdbserver_state.line_buf->len >= MAX_PACKET_LENGTH) { trace_gdbstub_err_overrun(); gdbserver_state.state = RS_IDLE; } else { /* unescaped command character */ - gdbserver_state.line_buf[gdbserver_state.line_buf_index++] = ch; + g_string_append_c(gdbserver_state.line_buf, (gchar) ch); gdbserver_state.line_sum += ch; } break; @@ -2406,13 +2407,13 @@ void gdb_read_byte(uint8_t ch) if (ch == '#') { /* unexpected end of command in escape sequence */ gdbserver_state.state = RS_CHKSUM1; - } else if (gdbserver_state.line_buf_index >= sizeof(gdbserver_state.line_buf) - 1) { + } else if (gdbserver_state.line_buf->len >= MAX_PACKET_LENGTH) { /* command buffer overrun */ trace_gdbstub_err_overrun(); gdbserver_state.state = RS_IDLE; } else { /* parse escaped character and leave escape state */ - gdbserver_state.line_buf[gdbserver_state.line_buf_index++] = ch ^ 0x20; + g_string_append_c(gdbserver_state.line_buf, (gchar) ch ^ 0x20); gdbserver_state.line_sum += ch; gdbserver_state.state = RS_GETLINE; } @@ -2429,19 +2430,20 @@ void gdb_read_byte(uint8_t ch) } else { /* decode repeat length */ int repeat = ch - ' ' + 3; - if (gdbserver_state.line_buf_index + repeat >= sizeof(gdbserver_state.line_buf) - 1) { + if (gdbserver_state.line_buf->len + repeat >= MAX_PACKET_LENGTH) { /* that many repeats would overrun the command buffer */ trace_gdbstub_err_overrun(); gdbserver_state.state = RS_IDLE; - } else if (gdbserver_state.line_buf_index < 1) { + } else if (gdbserver_state.line_buf->len < 1) { /* got a repeat but we have nothing to repeat */ trace_gdbstub_err_invalid_rle(); gdbserver_state.state = RS_GETLINE; } else { /* repeat the last character */ - memset(gdbserver_state.line_buf + gdbserver_state.line_buf_index, - gdbserver_state.line_buf[gdbserver_state.line_buf_index - 1], repeat); - gdbserver_state.line_buf_index += repeat; + for (int i = 0; i < repeat; i ++){ + g_string_append_c(gdbserver_state.line_buf, + gdbserver_state.line_buf->str[gdbserver_state.line_buf->len - 1]); + } gdbserver_state.line_sum += ch; gdbserver_state.state = RS_GETLINE; } @@ -2454,7 +2456,6 @@ void gdb_read_byte(uint8_t ch) gdbserver_state.state = RS_GETLINE; break; } - gdbserver_state.line_buf[gdbserver_state.line_buf_index] = '\0'; gdbserver_state.line_csum = fromhex(ch) << 4; gdbserver_state.state = RS_CHKSUM2; break; @@ -2477,7 +2478,7 @@ void gdb_read_byte(uint8_t ch) /* send ACK reply */ reply = '+'; gdb_put_buffer(&reply, 1); - gdbserver_state.state = gdb_handle_packet(gdbserver_state.line_buf); + gdbserver_state.state = gdb_handle_packet((char *) gdbserver_state.line_buf->str); } break; default: diff --git a/gdbstub/internals.h b/gdbstub/internals.h index 87f64b6318..27afbef4f5 100644 --- a/gdbstub/internals.h +++ b/gdbstub/internals.h @@ -72,8 +72,7 @@ typedef struct GDBState { CPUState *g_cpu; /* current CPU for other ops */ CPUState *query_cpu; /* for q{f|s}ThreadInfo */ enum RSState state; /* parsing state */ - char line_buf[MAX_PACKET_LENGTH]; - int line_buf_index; + GString *line_buf; int line_sum; /* running checksum */ int line_csum; /* checksum at the end of the packet */ GByteArray *last_packet; -- 2.34.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v5 2/4] target/arm: Change GDBState's line_buf to a GString 2025-08-11 19:36 ` [PATCH v5 2/4] target/arm: Change GDBState's line_buf to a GString Vacha Bhavsar @ 2025-08-12 11:32 ` Peter Maydell 2025-08-12 19:31 ` Vacha Bhavsar 0 siblings, 1 reply; 16+ messages in thread From: Peter Maydell @ 2025-08-12 11:32 UTC (permalink / raw) To: Vacha Bhavsar Cc: qemu-devel, qemu-arm, Philippe Mathieu-Daudé, Alex Bennée, Paolo Bonzini, Thomas Huth On Mon, 11 Aug 2025 at 20:37, Vacha Bhavsar <vacha.bhavsar@oss.qualcomm.com> wrote: > > This patch changes GDBState's line_buf from a character > array to a GString. This allows line_buf to be dynamically > re-sizeable. > > Signed-off-by: Vacha Bhavsar <vacha.bhavsar@oss.qualcomm.com> > --- > Changes since v4: > - this patch was not present in v4, it has been added as > suggested during review of v4 > --- Note that since the GDBState is a file-local variable, not allocated on the stack, it's not an issue that line_buf[MAX_PACKET_LENGTH] is large. So whether we make this change I think should be based on whether it makes the code easier to read and less likely to contain string buffer manipulation bugs. > - if (gdbserver_state.line_buf_index + repeat >= > sizeof(gdbserver_state.line_buf) - 1) { > + if (gdbserver_state.line_buf->len + repeat >= MAX_PACKET_LENGTH) { > /* that many repeats would overrun the command buffer */ This comment no longer makes sense if we don't have a fixed command buffer. In general, if we're moving away from a fixed-sized buffer we should consider what we want to do in all the various places that are currently doing checks against MAX_PACKET_LENGTH. thanks -- PMM ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5 2/4] target/arm: Change GDBState's line_buf to a GString 2025-08-12 11:32 ` Peter Maydell @ 2025-08-12 19:31 ` Vacha Bhavsar 0 siblings, 0 replies; 16+ messages in thread From: Vacha Bhavsar @ 2025-08-12 19:31 UTC (permalink / raw) To: Peter Maydell Cc: qemu-devel, qemu-arm, Philippe Mathieu-Daudé, Alex Bennée, Paolo Bonzini, Thomas Huth [-- Attachment #1: Type: text/plain, Size: 1772 bytes --] Hi, Since this change does not directly impact the original concern for adding SME register exposure to remote GDB (for which the resizing of MAX_PACKET_LENGTH is sufficient), may we leave this patch open to further discussion while the others go through with review? Thanks, Vacha On Tue, Aug 12, 2025 at 7:33 AM Peter Maydell <peter.maydell@linaro.org> wrote: > On Mon, 11 Aug 2025 at 20:37, Vacha Bhavsar > <vacha.bhavsar@oss.qualcomm.com> wrote: > > > > This patch changes GDBState's line_buf from a character > > array to a GString. This allows line_buf to be dynamically > > re-sizeable. > > > > Signed-off-by: Vacha Bhavsar <vacha.bhavsar@oss.qualcomm.com> > > --- > > Changes since v4: > > - this patch was not present in v4, it has been added as > > suggested during review of v4 > > --- > > Note that since the GDBState is a file-local variable, > not allocated on the stack, it's not an issue that > line_buf[MAX_PACKET_LENGTH] is large. So whether we > make this change I think should be based on whether > it makes the code easier to read and less likely to > contain string buffer manipulation bugs. > > > > - if (gdbserver_state.line_buf_index + repeat >= > > sizeof(gdbserver_state.line_buf) - 1) { > > + if (gdbserver_state.line_buf->len + repeat >= > MAX_PACKET_LENGTH) { > > /* that many repeats would overrun the command > buffer */ > > This comment no longer makes sense if we don't have a > fixed command buffer. In general, if we're moving away > from a fixed-sized buffer we should consider what we > want to do in all the various places that are currently > doing checks against MAX_PACKET_LENGTH. > > thanks > -- PMM > [-- Attachment #2: Type: text/html, Size: 2417 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v5 3/4] target/arm: Added support for SME register exposure to 2025-08-11 19:36 [PATCH v5 0/4] target/arm: Added support for SME register exposure to GDB Vacha Bhavsar 2025-08-11 19:36 ` [PATCH v5 1/4] target/arm: Increase MAX_PACKET_LENGTH for SME ZA Vacha Bhavsar 2025-08-11 19:36 ` [PATCH v5 2/4] target/arm: Change GDBState's line_buf to a GString Vacha Bhavsar @ 2025-08-11 19:36 ` Vacha Bhavsar 2025-08-19 9:04 ` Peter Maydell 2025-08-11 19:36 ` [PATCH v5 4/4] target/arm: Added test case for SME register exposure Vacha Bhavsar 3 siblings, 1 reply; 16+ messages in thread From: Vacha Bhavsar @ 2025-08-11 19:36 UTC (permalink / raw) To: qemu-devel Cc: Peter Maydell, qemu-arm, Philippe Mathieu-Daudé, 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> --- 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 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v5 3/4] target/arm: Added support for SME register exposure to 2025-08-11 19:36 ` [PATCH v5 3/4] target/arm: Added support for SME register exposure to Vacha Bhavsar @ 2025-08-19 9:04 ` Peter Maydell 2025-08-21 21:37 ` Vacha Bhavsar 0 siblings, 1 reply; 16+ messages in thread From: Peter Maydell @ 2025-08-19 9:04 UTC (permalink / raw) To: Vacha Bhavsar Cc: qemu-devel, qemu-arm, Philippe Mathieu-Daudé, Alex Bennée, Paolo Bonzini, Thomas Huth 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5 3/4] target/arm: Added support for SME register exposure to 2025-08-19 9:04 ` Peter Maydell @ 2025-08-21 21:37 ` Vacha Bhavsar 2025-08-22 12:27 ` Peter Maydell 0 siblings, 1 reply; 16+ messages in thread From: Vacha Bhavsar @ 2025-08-21 21:37 UTC (permalink / raw) To: Peter Maydell Cc: qemu-devel, qemu-arm, Philippe Mathieu-Daudé, Alex Bennée, Paolo Bonzini, Thomas Huth [-- Attachment #1: Type: text/plain, Size: 11018 bytes --] 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 > [-- Attachment #2: Type: text/html, Size: 14292 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5 3/4] target/arm: Added support for SME register exposure to 2025-08-21 21:37 ` Vacha Bhavsar @ 2025-08-22 12:27 ` Peter Maydell 2025-08-26 18:52 ` Vacha Bhavsar 0 siblings, 1 reply; 16+ messages in thread From: Peter Maydell @ 2025-08-22 12:27 UTC (permalink / raw) To: Vacha Bhavsar Cc: qemu-devel, qemu-arm, Philippe Mathieu-Daudé, Alex Bennée, Paolo Bonzini, Thomas Huth 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5 3/4] target/arm: Added support for SME register exposure to 2025-08-22 12:27 ` Peter Maydell @ 2025-08-26 18:52 ` Vacha Bhavsar 0 siblings, 0 replies; 16+ messages in thread From: Vacha Bhavsar @ 2025-08-26 18:52 UTC (permalink / raw) To: Peter Maydell Cc: qemu-devel, qemu-arm, Philippe Mathieu-Daudé, Alex Bennée, Paolo Bonzini, Thomas Huth [-- Attachment #1: Type: text/plain, Size: 1950 bytes --] 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 > [-- Attachment #2: Type: text/html, Size: 2724 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v5 4/4] target/arm: Added test case for SME register exposure 2025-08-11 19:36 [PATCH v5 0/4] target/arm: Added support for SME register exposure to GDB Vacha Bhavsar ` (2 preceding siblings ...) 2025-08-11 19:36 ` [PATCH v5 3/4] target/arm: Added support for SME register exposure to Vacha Bhavsar @ 2025-08-11 19:36 ` Vacha Bhavsar 2025-08-19 9:13 ` Peter Maydell 3 siblings, 1 reply; 16+ messages in thread From: Vacha Bhavsar @ 2025-08-11 19:36 UTC (permalink / raw) To: qemu-devel Cc: Peter Maydell, qemu-arm, Philippe Mathieu-Daudé, Alex Bennée, Paolo Bonzini, Thomas Huth, Vacha Bhavsar This patch adds a test case to test SME register exposure to a remote gdb debugging session. This test simply sets and reads SME registers. Signed-off-by: Vacha Bhavsar <vacha.bhavsar@oss.qualcomm.com> --- Changes since v4: - this patch was not present in v4, it was added based on suggestions during the review of v4 --- configure | 6 ++ tests/tcg/aarch64/Makefile.target | 23 ++++- tests/tcg/aarch64/gdbstub/test-sme.py | 119 ++++++++++++++++++++++++++ 3 files changed, 147 insertions(+), 1 deletion(-) create mode 100644 tests/tcg/aarch64/gdbstub/test-sme.py diff --git a/configure b/configure index 825057ebf1..177d3775ac 100755 --- a/configure +++ b/configure @@ -1837,6 +1837,12 @@ for target in $target_list; do echo "GDB=$gdb_bin" >> $config_target_mak fi + if test "${gdb_arches#*$arch}" != "$gdb_arches" && version_ge $gdb_version 14.1; then + echo "GDB_HAS_SME_TILES=y" >> $config_target_mak + else + echo "GDB_HAS_SME_TILES=n" >> $config_target_mak + fi + if test "${gdb_arches#*aarch64}" != "$gdb_arches" && version_ge $gdb_version 15.1; then echo "GDB_HAS_MTE=y" >> $config_target_mak fi diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target index 16ddcf4f88..1a33ef17a0 100644 --- a/tests/tcg/aarch64/Makefile.target +++ b/tests/tcg/aarch64/Makefile.target @@ -132,7 +132,28 @@ run-gdbstub-sve-ioctls: sve-ioctls --bin $< --test $(AARCH64_SRC)/gdbstub/test-sve-ioctl.py, \ basic gdbstub SVE ZLEN support) -EXTRA_RUNS += run-gdbstub-sysregs run-gdbstub-sve-ioctls +ifneq ($(CROSS_AS_HAS_ARMV9_SME),) +# SME gdbstub test +ifeq ($(GDB_HAS_SME_TILES),y) +run-gdbstub-sysregs-sme: sysregs + $(call run-test, $@, $(GDB_SCRIPT) \ + --gdb $(GDB) \ + --qemu $(QEMU) --qargs "$(QEMU_OPTS)" \ + --bin $< --test $(AARCH64_SRC)/gdbstub/test-sme.py \ + -- test_sme --gdb_sme_tile_support, \ + basic gdbstub SME support) +else +run-gdbstub-sysregs-sme: sysregs + $(call run-test, $@, $(GDB_SCRIPT) \ + --gdb $(GDB) \ + --qemu $(QEMU) --qargs "$(QEMU_OPTS)" \ + --bin $< --test $(AARCH64_SRC)/gdbstub/test-sme.py, \ + basic gdbstub SME support) + +endif +endif + +EXTRA_RUNS += run-gdbstub-sysregs run-gdbstub-sve-ioctls run-gdbstub-sysregs-sme ifeq ($(GDB_HAS_MTE),y) run-gdbstub-mte: mte-8 diff --git a/tests/tcg/aarch64/gdbstub/test-sme.py b/tests/tcg/aarch64/gdbstub/test-sme.py new file mode 100644 index 0000000000..c2b9d774ae --- /dev/null +++ b/tests/tcg/aarch64/gdbstub/test-sme.py @@ -0,0 +1,119 @@ +from __future__ import print_function +# +# Test the SME registers are visible and changeable via gdbstub +# +# This is launched via tests/guest-debug/run-test.py +# + +import argparse +import gdb +from test_gdbstub import main, report + +MAGIC = 0x01020304 + +def run_test(): + "Run through the tests one by one" + + frame = gdb.selected_frame() + rname = "za" + za = frame.read_register(rname) + report(True, "Reading %s" % rname) + + for i in range(0, 16): + for j in range(0, 16): + cmd = "set $za[%d][%d] = 0x01" % (i, j) + gdb.execute(cmd) + report(True, "%s" % cmd) + for i in range(0, 16): + for j in range(0, 16): + reg = "$za[%d][%d]" % (i, j) + v = gdb.parse_and_eval(reg) + report(str(v.type) == "uint8_t", + "size of %s" % (reg)) + report(int(v) == 0x1, "%s is 0x%x" % (reg, 0x1)) + +def run_test_slices(): + "Run through the tests one by one" + + frame = gdb.selected_frame() + rname = "za" + za = frame.read_register(rname) + report(True, "Reading %s" % rname) + + for i in range(0, 16): + for j in range(0, 16): + cmd = "set $za[%d][%d] = 0x01" % (i, j) + gdb.execute(cmd) + report(True, "%s" % cmd) + for i in range(0, 16): + for j in range(0, 16): + reg = "$za[%d][%d]" % (i, j) + v = gdb.parse_and_eval(reg) + report(str(v.type) == "uint8_t", + "size of %s" % (reg)) + report(int(v) == 0x1, "%s is 0x%x" % (reg, 0x1)) + + for i in range(0, 4): + for j in range(0, 4): + for k in range(0, 4): + cmd = "set $za%dhq%d[%d] = 0x%x" % (i, j, k, MAGIC) + gdb.execute(cmd) + report(True, "%s" % cmd) + for j in range(0, 4): + for k in range(0, 4): + reg = "$za%dhq%d[%d]" % (i, j, k) + v = gdb.parse_and_eval(reg) + report(str(v.type) == "uint128_t", + "size of %s" % (reg)) + report(int(v) == MAGIC, "%s is 0x%x" % (reg, MAGIC)) + + for j in range(0, 4): + for k in range(0, 4): + cmd = "set $za%dvq%d[%d] = 0x%x" % (i, j, k, MAGIC) + gdb.execute(cmd) + report(True, "%s" % cmd) + for j in range(0, 4): + for k in range(0, 4): + reg = "$za%dvq%d[%d]" % (i, j, k) + v = gdb.parse_and_eval(reg) + report(str(v.type) == "uint128_t", + "size of %s" % (reg)) + report(int(v) == MAGIC, "%s is 0x%x" % (reg, MAGIC)) + + for i in range(0, 4): + for j in range(0, 4): + for k in range(0, 4): + cmd = "set $za%dhd%d[%d] = 0x%x" % (i, j, k, MAGIC) + gdb.execute(cmd) + report(True, "%s" % cmd) + for j in range(0, 4): + for k in range(0, 4): + reg = "$za%dhd%d[%d]" % (i, j, k) + v = gdb.parse_and_eval(reg) + report(str(v.type) == "uint64_t", + "size of %s" % (reg)) + report(int(v) == MAGIC, "%s is 0x%x" % (reg, MAGIC)) + + for j in range(0, 4): + for k in range(0, 4): + cmd = "set $za%dvd%d[%d] = 0x%x" % (i, j, k, MAGIC) + gdb.execute(cmd) + report(True, "%s" % cmd) + for j in range(0, 4): + for k in range(0, 4): + reg = "$za%dvd%d[%d]" % (i, j, k) + v = gdb.parse_and_eval(reg) + report(str(v.type) == "uint64_t", + "size of %s" % (reg)) + report(int(v) == MAGIC, "%s is 0x%x" % (reg, MAGIC)) + + +parser = argparse.ArgumentParser(description="A gdbstub test for SME support") +parser.add_argument("--gdb_sme_tile_support", help="GDB support for SME tiles", \ + action="store_true") +args = parser.parse_args() + +if args.gdb_sme_tile_support: + main(run_test_slices, expected_arch="aarch64") +else: + main(run_test, expected_arch="aarch64") \ No newline at end of file -- 2.34.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v5 4/4] target/arm: Added test case for SME register exposure 2025-08-11 19:36 ` [PATCH v5 4/4] target/arm: Added test case for SME register exposure Vacha Bhavsar @ 2025-08-19 9:13 ` Peter Maydell 2025-08-22 17:32 ` Vacha Bhavsar 0 siblings, 1 reply; 16+ messages in thread From: Peter Maydell @ 2025-08-19 9:13 UTC (permalink / raw) To: Vacha Bhavsar Cc: qemu-devel, qemu-arm, Philippe Mathieu-Daudé, Alex Bennée, Paolo Bonzini, Thomas Huth On Mon, 11 Aug 2025 at 20:37, Vacha Bhavsar <vacha.bhavsar@oss.qualcomm.com> wrote: > > This patch adds a test case to test SME register exposure to > a remote gdb debugging session. This test simply sets and > reads SME registers. > > Signed-off-by: Vacha Bhavsar <vacha.bhavsar@oss.qualcomm.com> > --- > Changes since v4: > - this patch was not present in v4, it was added based on > suggestions during the review of v4 > --- > configure | 6 ++ > tests/tcg/aarch64/Makefile.target | 23 ++++- > tests/tcg/aarch64/gdbstub/test-sme.py | 119 ++++++++++++++++++++++++++ > 3 files changed, 147 insertions(+), 1 deletion(-) > create mode 100644 tests/tcg/aarch64/gdbstub/test-sme.py This test fails for me: timeout -s KILL --foreground 120 /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/tests/guest-debug/run-test.py --gdb /usr/bin/gdb-multiarch --qemu /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-clang/qemu-aarch64 --qargs "" --bin sysregs --test /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/tests/tcg/aarch64/gdbstub/test-sme.py -- test_sme --gdb_sme_tile_support > run-gdbstub-sysregs-sme.out warning: File transfers from remote targets can be slow. Use "set sysroot" to access files locally instead. Python Exception <class 'gdb.error'>: That operation is not available on integers of more than 8 bytes. Error occurred in Python: That operation is not available on integers of more than 8 bytes. qemu-aarch64: QEMU: Terminated via GDBstub The gdb is: GNU gdb (Ubuntu 15.0.50.20240403-0ubuntu1) 15.0.50.20240403-git > diff --git a/tests/tcg/aarch64/gdbstub/test-sme.py b/tests/tcg/aarch64/gdbstub/test-sme.py > new file mode 100644 > index 0000000000..c2b9d774ae > --- /dev/null > +++ b/tests/tcg/aarch64/gdbstub/test-sme.py > @@ -0,0 +1,119 @@ > +from __future__ import print_function Alex, do we still need this line in the gdbstub test cases? We can probably assume that the gdb's python is python 3 by now, I hope... > +# > +# Test the SME registers are visible and changeable via gdbstub > +# > +# This is launched via tests/guest-debug/run-test.py > +# All new files must have an SPDX line saying what the license is. You may also wish to add a Copyright line; that's up to you/your employer. > +if args.gdb_sme_tile_support: > + main(run_test_slices, expected_arch="aarch64") > +else: > + main(run_test, expected_arch="aarch64") > \ No newline at end of file Please add the trailing newline. thanks -- PMM ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5 4/4] target/arm: Added test case for SME register exposure 2025-08-19 9:13 ` Peter Maydell @ 2025-08-22 17:32 ` Vacha Bhavsar 2025-08-23 18:13 ` Peter Maydell 0 siblings, 1 reply; 16+ messages in thread From: Vacha Bhavsar @ 2025-08-22 17:32 UTC (permalink / raw) To: Peter Maydell Cc: qemu-devel, qemu-arm, Philippe Mathieu-Daudé, Alex Bennée, Paolo Bonzini, Thomas Huth [-- Attachment #1: Type: text/plain, Size: 4594 bytes --] Hi, We have tried to replicate this issue on our end and it seems to stem from the int casting of gdb.Value type of a 128bit integer. We have run the test with different host architectures, gdb versions and python versions both with and without the int casting. The results are as follows. gdb gdb target python host int cast status version support version architecture 16.3 --enable-targets=all 3.11.13 x86 yes pass 16.3 --enable-targets=all 3.11.13 x86 no pass 16.3 --enable-targets=all 3.10.18 x86 yes pass 16.3 --enable-targets=all 3.10.18 x86 no pass 16.3 --enable-targets=all 3.8.10 x86 yes pass 16.3 --enable-targets=all 3.8.10 x86 no pass 16.3 aarch64 3.11.0rc1 aarch64 yes pass 16.3 aarch64 3.11.0rc1 aarch64 no pass 16.3 aarch64 3.10.12 aarch64 yes pass 16.3 aarch64 3.10.12 aarch64 no pass 15.0 multiarch 3.10.12 aarch64 yes fail 15.0 multiarch 3.10.12 aarch64 no pass 15.0 multiarch 3.11.0rc1 aarch64 yes fail 15.0 multiarch 3.11.0rc1 aarch64 no pass 15.0 multiarch 3.8.10 x86 yes fail 15.0 multiarch 3.8.10 x86 no pass 15.0 multiarch 3.11.13 x86 yes fail 15.0 multiarch 3.11.13 x86 no pass 15.0 multiarch 3.10.18 x86 yes fail 15.0 multiarch 3.10.18 x86 no pass Could we get some more information about your testing environment? Thanks, Vacha On Tue, Aug 19, 2025 at 5:13 AM Peter Maydell <peter.maydell@linaro.org> wrote: > On Mon, 11 Aug 2025 at 20:37, Vacha Bhavsar > <vacha.bhavsar@oss.qualcomm.com> wrote: > > > > This patch adds a test case to test SME register exposure to > > a remote gdb debugging session. This test simply sets and > > reads SME registers. > > > > Signed-off-by: Vacha Bhavsar <vacha.bhavsar@oss.qualcomm.com> > > --- > > Changes since v4: > > - this patch was not present in v4, it was added based on > > suggestions during the review of v4 > > --- > > configure | 6 ++ > > tests/tcg/aarch64/Makefile.target | 23 ++++- > > tests/tcg/aarch64/gdbstub/test-sme.py | 119 ++++++++++++++++++++++++++ > > 3 files changed, 147 insertions(+), 1 deletion(-) > > create mode 100644 tests/tcg/aarch64/gdbstub/test-sme.py > > This test fails for me: > > timeout -s KILL --foreground 120 > /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/tests/guest-debug/run-test.py > --gdb /usr/bin/gdb-multiarch --qemu > /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-clang/qemu-aarch64 > --qargs "" --bin sysregs --test > > /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/tests/tcg/aarch64/gdbstub/test-sme.py > -- test_sme --gdb_sme_tile_support > run-gdbstub-sysregs-sme.out > warning: File transfers from remote targets can be slow. Use "set > sysroot" to access files locally instead. > Python Exception <class 'gdb.error'>: That operation is not available > on integers of more than 8 bytes. > Error occurred in Python: That operation is not available on integers > of more than 8 bytes. > qemu-aarch64: QEMU: Terminated via GDBstub > > The gdb is: > GNU gdb (Ubuntu 15.0.50.20240403-0ubuntu1) 15.0.50.20240403-git > > > diff --git a/tests/tcg/aarch64/gdbstub/test-sme.py > b/tests/tcg/aarch64/gdbstub/test-sme.py > > new file mode 100644 > > index 0000000000..c2b9d774ae > > --- /dev/null > > +++ b/tests/tcg/aarch64/gdbstub/test-sme.py > > @@ -0,0 +1,119 @@ > > +from __future__ import print_function > > Alex, do we still need this line in the gdbstub test cases? > We can probably assume that the gdb's python is python 3 > by now, I hope... > > > +# > > +# Test the SME registers are visible and changeable via gdbstub > > +# > > +# This is launched via tests/guest-debug/run-test.py > > +# > > All new files must have an SPDX line saying what the > license is. You may also wish to add a Copyright line; > that's up to you/your employer. > > > > +if args.gdb_sme_tile_support: > > + main(run_test_slices, expected_arch="aarch64") > > +else: > > + main(run_test, expected_arch="aarch64") > > \ No newline at end of file > > Please add the trailing newline. > > thanks > -- PMM > [-- Attachment #2: Type: text/html, Size: 5817 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5 4/4] target/arm: Added test case for SME register exposure 2025-08-22 17:32 ` Vacha Bhavsar @ 2025-08-23 18:13 ` Peter Maydell 2025-08-26 18:51 ` Vacha Bhavsar 0 siblings, 1 reply; 16+ messages in thread From: Peter Maydell @ 2025-08-23 18:13 UTC (permalink / raw) To: Vacha Bhavsar Cc: qemu-devel, qemu-arm, Philippe Mathieu-Daudé, Alex Bennée, Paolo Bonzini, Thomas Huth On Fri, 22 Aug 2025 at 18:32, Vacha Bhavsar <vacha.bhavsar@oss.qualcomm.com> wrote: > > Hi, > > We have tried to replicate this issue on our end and it > seems to stem from the int casting of gdb.Value type of > a 128bit integer. We have run the test with different > host architectures, gdb versions and python versions > both with and without the int casting. The results are > as follows. > > gdb gdb target python host int cast status > version support version architecture > 16.3 --enable-targets=all 3.11.13 x86 yes pass > 16.3 --enable-targets=all 3.11.13 x86 no pass > 16.3 --enable-targets=all 3.10.18 x86 yes pass > 16.3 --enable-targets=all 3.10.18 x86 no pass > 16.3 --enable-targets=all 3.8.10 x86 yes pass > 16.3 --enable-targets=all 3.8.10 x86 no pass > > 16.3 aarch64 3.11.0rc1 aarch64 yes pass > 16.3 aarch64 3.11.0rc1 aarch64 no pass > 16.3 aarch64 3.10.12 aarch64 yes pass > 16.3 aarch64 3.10.12 aarch64 no pass > > 15.0 multiarch 3.10.12 aarch64 yes fail > 15.0 multiarch 3.10.12 aarch64 no pass > 15.0 multiarch 3.11.0rc1 aarch64 yes fail > 15.0 multiarch 3.11.0rc1 aarch64 no pass > > 15.0 multiarch 3.8.10 x86 yes fail > 15.0 multiarch 3.8.10 x86 no pass > 15.0 multiarch 3.11.13 x86 yes fail > 15.0 multiarch 3.11.13 x86 no pass > 15.0 multiarch 3.10.18 x86 yes fail > 15.0 multiarch 3.10.18 x86 no pass > > Could we get some more information about your testing environment? It's just stock Ubuntu 24.04.3 LTS on x86-64; gdb is gdb-multiarch GNU gdb (Ubuntu 15.0.50.20240403-0ubuntu1) 15.0.50.20240403-git -- PMM ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5 4/4] target/arm: Added test case for SME register exposure 2025-08-23 18:13 ` Peter Maydell @ 2025-08-26 18:51 ` Vacha Bhavsar 0 siblings, 0 replies; 16+ messages in thread From: Vacha Bhavsar @ 2025-08-26 18:51 UTC (permalink / raw) To: Peter Maydell Cc: qemu-devel, qemu-arm, Philippe Mathieu-Daudé, Alex Bennée, Paolo Bonzini, Thomas Huth [-- Attachment #1: Type: text/plain, Size: 3094 bytes --] Hi, It seems the test fails specifically for that version of gdb (15.0.50.20240403-git) when a gdb.Value object which is an integer of more than 8 bytes is cast to an int. This is the case when we test reading the za quadwords. To address this we have edited the test such that the int cast is not used for the za quadwords when this specific version of gdb is detected. In this case, the test is performed without int casting (which still passes). To declare this to users, we have added a warning message to be printed in this scenario above the test results in the output file run-gdbstub-sysregs-sme.out. I have sent a new version of the patch series with these changes, as well as the added spdx/copyright lines. Please let us know what you think of this approach! Thanks, Vacha On Sat, Aug 23, 2025 at 2:13 PM Peter Maydell <peter.maydell@linaro.org> wrote: > On Fri, 22 Aug 2025 at 18:32, Vacha Bhavsar > <vacha.bhavsar@oss.qualcomm.com> wrote: > > > > Hi, > > > > We have tried to replicate this issue on our end and it > > seems to stem from the int casting of gdb.Value type of > > a 128bit integer. We have run the test with different > > host architectures, gdb versions and python versions > > both with and without the int casting. The results are > > as follows. > > > > gdb gdb target python host int cast status > > version support version architecture > > 16.3 --enable-targets=all 3.11.13 x86 yes pass > > 16.3 --enable-targets=all 3.11.13 x86 no pass > > 16.3 --enable-targets=all 3.10.18 x86 yes pass > > 16.3 --enable-targets=all 3.10.18 x86 no pass > > 16.3 --enable-targets=all 3.8.10 x86 yes pass > > 16.3 --enable-targets=all 3.8.10 x86 no pass > > > > 16.3 aarch64 3.11.0rc1 aarch64 yes pass > > 16.3 aarch64 3.11.0rc1 aarch64 no pass > > 16.3 aarch64 3.10.12 aarch64 yes pass > > 16.3 aarch64 3.10.12 aarch64 no pass > > > > 15.0 multiarch 3.10.12 aarch64 yes fail > > 15.0 multiarch 3.10.12 aarch64 no pass > > 15.0 multiarch 3.11.0rc1 aarch64 yes fail > > 15.0 multiarch 3.11.0rc1 aarch64 no pass > > > > 15.0 multiarch 3.8.10 x86 yes fail > > 15.0 multiarch 3.8.10 x86 no pass > > 15.0 multiarch 3.11.13 x86 yes fail > > 15.0 multiarch 3.11.13 x86 no pass > > 15.0 multiarch 3.10.18 x86 yes fail > > 15.0 multiarch 3.10.18 x86 no pass > > > > Could we get some more information about your testing environment? > > It's just stock Ubuntu 24.04.3 LTS on x86-64; gdb is gdb-multiarch > GNU gdb (Ubuntu 15.0.50.20240403-0ubuntu1) 15.0.50.20240403-git > > -- PMM > [-- Attachment #2: Type: text/html, Size: 4022 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2025-08-26 18:53 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-08-11 19:36 [PATCH v5 0/4] target/arm: Added support for SME register exposure to GDB Vacha Bhavsar 2025-08-11 19:36 ` [PATCH v5 1/4] target/arm: Increase MAX_PACKET_LENGTH for SME ZA Vacha Bhavsar 2025-08-19 9:14 ` Peter Maydell 2025-08-11 19:36 ` [PATCH v5 2/4] target/arm: Change GDBState's line_buf to a GString Vacha Bhavsar 2025-08-12 11:32 ` Peter Maydell 2025-08-12 19:31 ` Vacha Bhavsar 2025-08-11 19:36 ` [PATCH v5 3/4] target/arm: Added support for SME register exposure to Vacha Bhavsar 2025-08-19 9:04 ` Peter Maydell 2025-08-21 21:37 ` Vacha Bhavsar 2025-08-22 12:27 ` Peter Maydell 2025-08-26 18:52 ` Vacha Bhavsar 2025-08-11 19:36 ` [PATCH v5 4/4] target/arm: Added test case for SME register exposure Vacha Bhavsar 2025-08-19 9:13 ` Peter Maydell 2025-08-22 17:32 ` Vacha Bhavsar 2025-08-23 18:13 ` Peter Maydell 2025-08-26 18:51 ` 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).