1. Patchew
  2. QEMU
  3. [PATCH v4 0/2] target/arm: Added support for SME register exposure to GDB
  4. View patch

[PATCH v4 1/2] target/arm: Increase MAX_PACKET_LENGTH for SME ZA remote gdb debugging

Vacha Bhavsar posted 2 patches 4 months, 4 weeks ago
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Peter Maydell <peter.maydell@linaro.org>
There is a newer version of this series
[PATCH v4 1/2] target/arm: Increase MAX_PACKET_LENGTH for SME ZA remote gdb debugging
Posted by Vacha Bhavsar 4 months, 4 weeks ago 
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.bhavsar@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
 
 /*
  * Shared structures and definitions
-- 
2.34.1
Re: [PATCH v4 1/2] target/arm: Increase MAX_PACKET_LENGTH for SME ZA remote gdb debugging
Posted by Alex Bennée 4 months, 2 weeks ago 
Vacha Bhavsar <vacha.bhavsar@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.bhavsar@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.

>  
>  /*
>   * Shared structures and definitions

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [PATCH v4 1/2] target/arm: Increase MAX_PACKET_LENGTH for SME ZA remote gdb debugging
Posted by Peter Maydell 4 months, 2 weeks ago 
On Mon, 4 Aug 2025 at 16:34, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Vacha Bhavsar <vacha.bhavsar@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.bhavsar@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.

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.

thanks
-- PMM
Re: [PATCH v4 1/2] target/arm: Increase MAX_PACKET_LENGTH for SME ZA remote gdb debugging
Posted by Alex Bennée 4 months, 2 weeks ago 
Peter Maydell <peter.maydell@linaro.org> writes:

> On Mon, 4 Aug 2025 at 16:34, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> Vacha Bhavsar <vacha.bhavsar@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.bhavsar@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.

> 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.


>
> thanks
> -- PMM

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [PATCH v4 1/2] target/arm: Increase MAX_PACKET_LENGTH for SME ZA remote gdb debugging
Posted by Peter Maydell 4 months, 2 weeks ago 
On Mon, 4 Aug 2025 at 19:32, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
> > On Mon, 4 Aug 2025 at 16:34, Alex Bennée <alex.bennee@linaro.org> wrote:
> >>
> >> Vacha Bhavsar <vacha.bhavsar@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.bhavsar@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
Re: [PATCH v4 1/2] target/arm: Increase MAX_PACKET_LENGTH for SME ZA remote gdb debugging
Posted by Vacha Bhavsar 4 months, 2 weeks ago 
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.maydell@linaro.org>
wrote:

> On Mon, 4 Aug 2025 at 19:32, Alex Bennée <alex.bennee@linaro.org> wrote:
> >
> > Peter Maydell <peter.maydell@linaro.org> writes:
> >
> > > On Mon, 4 Aug 2025 at 16:34, Alex Bennée <alex.bennee@linaro.org>
> wrote:
> > >>
> > >> Vacha Bhavsar <vacha.bhavsar@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.bhavsar@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
>
Re: [PATCH v4 1/2] target/arm: Increase MAX_PACKET_LENGTH for SME ZA remote gdb debugging
Posted by Vacha Bhavsar 4 months, 2 weeks ago 
Hi,

I reached out to the gdb dev team regarding these questions
on how to correctly handle the SME za storage. This discussion
can be found here:

https://sourceware.org/pipermail/gdb/2025-August/051858.html
https://sourceware.org/pipermail/gdb/2025-August/051860.html

It seems this approach of increasing the buffer size to above
131100 has also been applied to gdbserver as there currently
isn't support for partial data transfers.



Thanks,

Vacha

On Tue, Aug 5, 2025 at 5:21 PM Vacha Bhavsar <vacha.bhavsar@oss.qualcomm.com>
wrote:

> 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.maydell@linaro.org>
> wrote:
>
>> On Mon, 4 Aug 2025 at 19:32, Alex Bennée <alex.bennee@linaro.org> wrote:
>> >
>> > Peter Maydell <peter.maydell@linaro.org> writes:
>> >
>> > > On Mon, 4 Aug 2025 at 16:34, Alex Bennée <alex.bennee@linaro.org>
>> wrote:
>> > >>
>> > >> Vacha Bhavsar <vacha.bhavsar@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.bhavsar@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
>>
>
Re: [PATCH v4 1/2] target/arm: Increase MAX_PACKET_LENGTH for SME ZA remote gdb debugging
Posted by Peter Maydell 4 months, 1 week ago 
On Wed, 6 Aug 2025 at 22:18, Vacha Bhavsar
<vacha.bhavsar@oss.qualcomm.com> wrote:
>
> Hi,
>
> I reached out to the gdb dev team regarding these questions
> on how to correctly handle the SME za storage. This discussion
> can be found here:
>
> https://sourceware.org/pipermail/gdb/2025-August/051858.html
> https://sourceware.org/pipermail/gdb/2025-August/051860.html
>
> It seems this approach of increasing the buffer size to above
> 131100 has also been applied to gdbserver as there currently
> isn't support for partial data transfers.

Thanks for chasing that up.

(1) I note that gdb bumped this to 131104, and we only
have 131100. We should check what the worst-case is and
make sure our number is big enough (maybe we count different
things in our buffer size than gdbstub does?)

(2) We should add a comment to MAX_PACKET_LENGTH, something
like:

/*
 * 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 + 1 bytes, and for other overhead like
 * the command itself or the checksum.
 */

(but ideally check the figures and be a bit less vague about
the "other overhead" :-))

-- PMM
Re: [PATCH v4 1/2] target/arm: Increase MAX_PACKET_LENGTH for SME ZA remote gdb debugging
Posted by Vacha Bhavsar 4 months, 1 week ago 
Hi,

From testing with a max vector length, the minimum size the buffer needs to
be is 131077 to survive the worst case of a P packet with the ZA register.
This is enough to fit the whole ZA register plus the overhead for the P
packet (command + register number, the checksum is not stored in line_buf)
and the null terminator. I had overshot and rounded it safely to 131100,
and it seems gdbserver has done the same thing but they specifically
counted 32 bytes for overhead, it's not specified why. So the 131100 is
large enough, however I have changed it to 131104 just to stay consistent
with gdbserver and have added this note to the comment you've suggested to
be placed above MAX_PACKET_LENGTH. I've also changed line_buf to be a
GString as Alex suggested.

I have sent a new version with these changes. Looking forward to your
feedback!

Thanks,
Vacha

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.