Hi,
Thank you for your comments!
What exactly is the packet/response where MAX_PACKET_LENGTH is
causing problems? The commit message doesn't say.
The issue is that when we run something like set $za[0][0] = 0x01
in the gdb client, the client sends the entire contents on the new
expected za register value, at which point the client side gets stuck
and does not return the gdb prompt. The issue is found to be the following
code (line 2396 of gdbstub/gdbstub.c):
else if (gdbserver_state.line_buf_index >= sizeof(gdbserver_state.line_buf)
- 1) {
trace_gdbstub_err_overrun();
gdbserver_state.state = RS_IDLE;
Since the current value of sizeof(gdbserver_state.line_buf) is 4096 whereas
the
entire contents of the P packet coming in from the gdb client is at least
131072
(twice the number of bytes in the za storage at max svl), the above
statement
eventually evaluates to true, causing the state machine to reset to RS_IDLE
and
treat the rest of the packet as if it's looking for a new command. This is
why
the client side gets stuck until there is a timeout and then debugging
continues
as usual.
For this reason, the MAX_PACKET_LENGTH value was increased in an effort to
increase
the size of gdbserver_state.line_buf and avoid entering the above mentioned
clause.
This sounds like something to query with the gdb devs
about what they expect the handling of the SME ZA storage should be.
Will do!
Thanks,
Vacha
On Mon, Aug 4, 2025 at 2:38 PM Peter Maydell <peter.mayd...@linaro.org>
wrote:
> On Mon, 4 Aug 2025 at 19:32, Alex Bennée <alex.ben...@linaro.org> wrote:
> >
> > Peter Maydell <peter.mayd...@linaro.org> writes:
> >
> > > On Mon, 4 Aug 2025 at 16:34, Alex Bennée <alex.ben...@linaro.org>
> wrote:
> > >>
> > >> Vacha Bhavsar <vacha.bhav...@oss.qualcomm.com> writes:
> > >>
> > >> > This patch increases the value of the MAX_PACKET_LEGNTH to
> > >> > 131100 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.bhav...@oss.qualcomm.com>
> > >> > ---
> > >> > Changes since v3:
> > >> > - this patch was not present in version 3
> > >> >
> > >> > gdbstub/internals.h | 2 +-
> > >> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >> >
> > >> > diff --git a/gdbstub/internals.h b/gdbstub/internals.h
> > >> > index bf5a5c6302..b58a66c201 100644
> > >> > --- a/gdbstub/internals.h
> > >> > +++ b/gdbstub/internals.h
> > >> > @@ -11,7 +11,7 @@
> > >> >
> > >> > #include "exec/cpu-common.h"
> > >> >
> > >> > -#define MAX_PACKET_LENGTH 4096
> > >> > +#define MAX_PACKET_LENGTH 131100
> > >>
> > >> This is a rather large expansion for something that ends up in a
> static at:
> > >>
> > >> char line_buf[MAX_PACKET_LENGTH];
> > >>
> > >> I think maybe its time to get rid of this hardcoded define and make
> line_buf a
> > >> dynamically re-sizeable buffer along the lines of str_buf and mem_buf.
> > >> In fact make it a GString and we can get rid of line_buf_index as
> well.
> > >
> > > What exactly is the packet/response where MAX_PACKET_LENGTH is
> > > causing problems? The commit message doesn't say.
> >
> > I assume it would be the g/G or p/P packets. The docs don't seem to say
> > anything about them splitting them across multiple packets.
>
> Probably because nobody thought about the possibility of enormous
> registers. This sounds like something to query with the gdb devs
> about what they expect the handling of the SME ZA storage should be.
>
> > > In general I thought the gdbstub protocol was supposed to handle a
> > > fixed packet length (e.g. in handle_query_xfer_features() the response
> > > packet indicates truncation via "l" vs "m" so the gdb end knows it
> needs
> > > to send another request to get the rest of the data). So if we run
> > > into something which seems to be fixed by raising MAX_PACKET_LENGTH
> > > I would first want to look at whether the underlying problem is
> > > that we're not indicating to gdb "this data is incomplete, you'll
> > > need to ask for more of it" or something of that nature.
> >
> > The docs reference "bulk transfers":
> >
> > ‘PacketSize=bytes’
> >
> > The remote stub can accept packets up to at least bytes in length.
> > GDB will send packets up to this size for bulk transfers, and will
> > never send larger packets. This is a limit on the data characters
> > in the packet, not including the frame and checksum. There is no
> > trailing NUL byte in a remote protocol packet; if the stub stores
> > packets in a NUL-terminated format, it should allow an extra byte
> > in its buffer for the NUL. If this stub feature is not supported,
> > GDB guesses based on the size of the ‘g’ packet response.
>
> We do advertise this.
>
> -- PMM
>
