Re: [PATCH v3 07/16] [gdbserver/aarch64] sme: Add support for SME

[Thiago Jung Bauermann <thiago.bauermann@linaro.org>]

Hello,

No real comment on this one, looks good to me.

Luis Machado via Gdb-patches <gdb-patches@sourceware.org> writes:

> Enable SME support in gdbserver by adjusting the usual fields.  There is
> not much to this patch because the code is either in gdb or it is shared
> between gdbserver and gdb.  One exception is the bump to gdbserver's
> PBUFSIZ from 18432 to 131104.
>
> Since the ZA register can be quite big (256 * 256 bytes), the g/G remote
> packet will also become quite big
>
> From gdbserver/tdesc.cc:init_target_desc, I estimated the new size should
> be at least (2 * 256 * 256 + 32), which yields 131104.
>
> It is also unlikely we will find a process starting up with SVL set to 256.
>
> Ideally we'd adjust the packet size dynamically based on what we need, but
> for now this should do.
>
> Please note we have the same limitation for SME that we have for SVE, and
> that is the fact gdbserver cannot communicate vector length changes to gdb
> via the remote protocol.
>
> Thiago is working on this improvement, which hopefully will be able to be
> adapted to SME in an easy way.

Unfortunately this was paused due to other Linaro work, but I'm resuming
it now.

> Co-Authored-By: Ezra Sitorus <ezra.sitorus@arm.com>
> ---
>  gdbserver/linux-aarch64-low.cc   | 74 ++++++++++++++++++++++++++++++++
>  gdbserver/linux-aarch64-tdesc.cc |  7 +++
>  gdbserver/server.h               |  2 +-
>  3 files changed, 82 insertions(+), 1 deletion(-)
>
> diff --git a/gdbserver/linux-aarch64-low.cc b/gdbserver/linux-aarch64-low.cc
> index 7c633c212f3..95688480c3d 100644
> --- a/gdbserver/linux-aarch64-low.cc
> +++ b/gdbserver/linux-aarch64-low.cc
> @@ -41,6 +41,7 @@
>  #include "gdb_proc_service.h"
>  #include "arch/aarch64.h"
>  #include "arch/aarch64-mte-linux.h"
> +#include "arch/aarch64-scalable-linux.h"
>  #include "linux-aarch32-tdesc.h"
>  #include "linux-aarch64-tdesc.h"
>  #include "nat/aarch64-mte-linux-ptrace.h"
> @@ -750,6 +751,66 @@ aarch64_sve_regs_copy_from_regcache (struct regcache *regcache, void *buf)
>    memcpy (buf, sve_state.data (), sve_state.size ());
>  }
>  
> +/* Wrapper for aarch64_za_regs_copy_to_reg_buf, to help copying NT_ARM_ZA
> +   state from the thread (BUF) to the register cache.  */
> +
> +static void
> +aarch64_za_regs_copy_to_regcache (struct regcache *regcache,
> +				  ATTRIBUTE_UNUSED const void *buf)
> +{
> +  /* BUF is unused here since we collect the data straight from a ptrace
> +     request, therefore bypassing gdbserver's own call to ptrace.  */

It's a bit of a pity that that the result of that ptrace call goes
unused. If ZA's regset_info.size was set to
sizeof (struct user_za_header) then it could be used to fetch the header
that aarch64_fetch_za_regset () needs.

But I imagine that there's no need for performance in this code path so
it's just a theoretical concern. Also, SVE already works this way.

-- 
Thiago