This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 2/8] Function for reading the Aarch64 SVE vector length.
- From: Simon Marchi <simon dot marchi at ericsson dot com>
- To: Alan Hayward <alan dot hayward at arm dot com>, <gdb-patches at sourceware dot org>
- Cc: <nd at arm dot com>
- Date: Thu, 31 May 2018 07:44:35 -0400
- Subject: Re: [PATCH 2/8] Function for reading the Aarch64 SVE vector length.
- References: <20180511105256.27388-1-alan.hayward@arm.com> <20180511105256.27388-3-alan.hayward@arm.com>
- Spamdiagnosticmetadata: NSPM
- Spamdiagnosticoutput: 1:99
Hi Alan,
LGTM, with some nits.
On 2018-05-11 06:52 AM, Alan Hayward wrote:
> Add a method for reading the SVE vector length using ptrace. This returns
> 0 for systems without SVE support.
>
> Note the defines taken from Linux kernel headers in aarch64-sve-linux-ptrace.h.
> See the covering email for details about this.
Using ptrace to read the SVE registers (and the SVE register length) will work even
for kernels that didn't have these macros?
> There are multiple ways of expressing the vector length. Thankfully these are
> all wll defined. I've added convertors for going from one to the other.
> Hopefully this will help to prevent future confusion.
Copyright 2017 -> 2018 in the new files.
>
> 2018-05-11 Alan Hayward <alan.hayward@arm.com>
>
> gdb/
> * Makefile.in: Add new header.
> * gdb/arch/aarch64.h (sve_vg_from_vl): New macro.
> (sve_vl_from_vg): Likewise.
> (sve_vq_from_vl): Likewise.
> (sve_vl_from_vq): Likewise.
> (sve_vq_from_vg): Likewise.
> (sve_vg_from_vq): Likewise.
> * configure.nat: Add new c file.
> * nat/aarch64-sve-linux-ptrace.c: New file.
> * nat/aarch64-sve-linux-ptrace.h: New file.
>
> gdbserver/
> * configure.srv: Add new c/h file.
> ---
> gdb/Makefile.in | 1 +
> gdb/arch/aarch64.h | 17 +++++++++
> gdb/configure.nat | 2 +-
> gdb/gdbserver/configure.srv | 1 +
> gdb/nat/aarch64-sve-linux-ptrace.c | 52 +++++++++++++++++++++++++++
> gdb/nat/aarch64-sve-linux-ptrace.h | 73 ++++++++++++++++++++++++++++++++++++++
> 6 files changed, 145 insertions(+), 1 deletion(-)
> create mode 100644 gdb/nat/aarch64-sve-linux-ptrace.c
> create mode 100644 gdb/nat/aarch64-sve-linux-ptrace.h
>
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index 87d74a7703..64042d1bd1 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -1478,6 +1478,7 @@ HFILES_NO_SRCDIR = \
> mi/mi-parse.h \
> nat/aarch64-linux.h \
> nat/aarch64-linux-hw-point.h \
> + nat/aarch64-sve-linux-ptrace.h \
> nat/amd64-linux-siginfo.h \
> nat/gdb_ptrace.h \
> nat/gdb_thread_db.h \
> diff --git a/gdb/arch/aarch64.h b/gdb/arch/aarch64.h
> index 1846e04163..af0b157c51 100644
> --- a/gdb/arch/aarch64.h
> +++ b/gdb/arch/aarch64.h
> @@ -48,6 +48,23 @@ enum aarch64_regnum
> #define AARCH64_V_REGS_NUM 32
> #define AARCH64_NUM_REGS AARCH64_FPCR_REGNUM + 1
>
> +/* There are a number of ways of expressing the current SVE vector size:
> +
> + VL : Vector Length.
> + The number of bytes in an SVE Z register.
> + VQ : Vector Quotient.
> + The number of 128bit chunks in an SVE Z register.
> + VG : Vector Gradient.
> + The number of 64bit chunks in an SVE Z register. */
> +
> +#define sve_vg_from_vl(vl) ((vl) / 8)
> +#define sve_vl_from_vg(vg) ((vg) * 8)
> +#define sve_vq_from_vl(vl) ((vl) / 0x10)
> +#define sve_vl_from_vq(vq) ((vq) * 0x10)
> +#define sve_vq_from_vg(vg) (sve_vq_from_vl (sve_vl_from_vg (vg)))
> +#define sve_vg_from_vq(vq) (sve_vg_from_vl (sve_vl_from_vq (vq)))
> +
> +
> /* Maximum supported VQ value. Increase if required. */
> #define AARCH64_MAX_SVE_VQ 16
>
> diff --git a/gdb/configure.nat b/gdb/configure.nat
> index 6b0f44fede..d7d79adaca 100644
> --- a/gdb/configure.nat
> +++ b/gdb/configure.nat
> @@ -228,7 +228,7 @@ case ${gdb_host} in
> aarch64)
> # Host: AArch64 based machine running GNU/Linux
> NATDEPFILES="${NATDEPFILES} aarch64-linux-nat.o \
> - aarch32-linux-nat.o aarch64-linux-hw-point.o aarch64-linux.o"
> + aarch32-linux-nat.o aarch64-linux-hw-point.o aarch64-linux.o aarch64-sve-linux-ptrace.o"
Please wrap to 80 columns.
> ;;
> arm)
> # Host: ARM based machine running GNU/Linux
> diff --git a/gdb/gdbserver/configure.srv b/gdb/gdbserver/configure.srv
> index ffeefb9b92..8bf0dcc650 100644
> --- a/gdb/gdbserver/configure.srv
> +++ b/gdb/gdbserver/configure.srv
> @@ -54,6 +54,7 @@ case "${target}" in
> srv_tgtobj="$srv_tgtobj arch/aarch64-insn.o"
> srv_tgtobj="$srv_tgtobj arch/aarch64.o"
> srv_tgtobj="$srv_tgtobj linux-aarch64-tdesc.o"
> + srv_tgtobj="$srv_tgtobj aarch64-sve-linux-ptrace.o"
> srv_tgtobj="${srv_tgtobj} $srv_linux_obj"
> srv_linux_regsets=yes
> srv_linux_thread_db=yes
> diff --git a/gdb/nat/aarch64-sve-linux-ptrace.c b/gdb/nat/aarch64-sve-linux-ptrace.c
> new file mode 100644
> index 0000000000..9381786fda
> --- /dev/null
> +++ b/gdb/nat/aarch64-sve-linux-ptrace.c
> @@ -0,0 +1,52 @@
> +/* Common target dependent for AArch64 systems.
> +
> + Copyright (C) 2017 Free Software Foundation, Inc.
> +
> + This file is part of GDB.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> +
> +#include <sys/utsname.h>
> +#include <sys/uio.h>
> +#include "common-defs.h"
> +#include "elf/external.h"
> +#include "elf/common.h"
> +#include "aarch64-sve-linux-ptrace.h"
> +#include "arch/aarch64.h"
> +
> +/* Read VQ for the given tid using ptrace. If SVE is not supported then zero
> + is returned (on a system that supports SVE, then VQ cannot be zeo). */
Here, put a reference to the .h as usual.
> +
> +unsigned long
> +aarch64_sve_get_vq (int tid)
> +{
> + struct iovec iovec;
> + struct user_sve_header header;
> +
> + iovec.iov_len = sizeof (header);
> + iovec.iov_base = &header;
> +
> + /* Ptrace gives the vector length in bytes. Convert it to VQ, the number of
> + 128bit chunks in a Z register. We use VQ because 128bits is the minimum
> + a Z register can increase in size. */
> +
> + if (ptrace (PTRACE_GETREGSET, tid, NT_ARM_SVE, &iovec) < 0)
> + /* SVE is not supported. */
> + return 0;
Add braces here.
> +
> + long vq = sve_vq_from_vl (header.vl);
> + gdb_assert (sve_vl_valid (header.vl));
We should avoid gdb_assert for bad input values (including what we receive from the
kernel). Could we display a warning and return 0?
> +
> + return vq;
> +}
> diff --git a/gdb/nat/aarch64-sve-linux-ptrace.h b/gdb/nat/aarch64-sve-linux-ptrace.h
> new file mode 100644
> index 0000000000..b318150ac1
> --- /dev/null
> +++ b/gdb/nat/aarch64-sve-linux-ptrace.h
> @@ -0,0 +1,73 @@
> +/* Common target dependent for AArch64 systems.
> +
> + Copyright (C) 2017 Free Software Foundation, Inc.
> +
> + This file is part of GDB.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> +
> +#ifndef AARCH64_SVE_LINUX_PTRACE_H
> +#define AARCH64_SVE_LINUX_PTRACE_H
> +
> +/* Where indicated, this file contains defines and macros lifted directly from
> + the Linux kernel headers, with no modification.
> + Refer to Linux kernel documentation for details. */
> +
> +#include <asm/sigcontext.h>
> +#include <sys/utsname.h>
> +#include <sys/ptrace.h>
> +#include <asm/ptrace.h>
> +
> +/* Read VQ for the given tid using ptrace. If SVE is not supported then zero
> + is returned (on a system that supports SVE, then VQ cannot be zeo). */
zeo -> zero.
> +extern unsigned long aarch64_sve_get_vq (int tid);
> +
> +/* Structures and defines taken from sigcontext.h. */
> +
> +#ifndef SVE_SIG_ZREGS_SIZE
> +
> +#define SVE_VQ_BYTES 16 /* number of bytes per quadword */
> +
> +#define SVE_VQ_MIN 1
> +#define SVE_VQ_MAX 512
> +
> +#define SVE_VL_MIN (SVE_VQ_MIN * SVE_VQ_BYTES)
> +#define SVE_VL_MAX (SVE_VQ_MAX * SVE_VQ_BYTES)
> +
> +#define SVE_NUM_ZREGS 32
> +#define SVE_NUM_PREGS 16
> +
> +#define sve_vl_valid(vl) \
> + ((vl) % SVE_VQ_BYTES == 0 && (vl) >= SVE_VL_MIN && (vl) <= SVE_VL_MAX)
> +
> +#endif /* SVE_SIG_ZREGS_SIZE. */
> +
> +
> +/* Structures and defines taken from ptrace.h. */
> +
> +#ifndef SVE_PT_SVE_ZREG_SIZE
> +
> +struct user_sve_header {
> + __u32 size; /* total meaningful regset content in bytes */
> + __u32 max_size; /* maxmium possible size for this thread */
> + __u16 vl; /* current vector length */
> + __u16 max_vl; /* maximum possible vector length */
> + __u16 flags;
> + __u16 __reserved;
> +};
> +
> +#endif /* SVE_PT_SVE_ZREG_SIZE. */
> +
> +#endif /* aarch64-sve-linux-ptrace.h */
>