[2/8] Function for reading the Aarch64 SVE vector length.

  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.

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.

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
  

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 */
>
  

Pushed with changes as below.

> On 31 May 2018, at 12:44, Simon Marchi <simon.marchi@ericsson.com> wrote:

> 

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


(Answering both possible ways I thought the question was asking).

The macros were added to the kernel in the same batch of patches that
added all the Linux sve support. So, the macros should always exist
on SVE enabled kernels.

The macros are only calculating offsets within the variable length
structure. There’s is nothing preventing you using them on (say)
x86 linux 3.0 to parse the contents of a copy of the structure.
It’s possible that they might end up getting used for reading sve
core files.

> 

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


Ok. (I started these patches quite a while ago!)

> 

>> 

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


Ok.

> 

>> 		;;

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


Ok.

> 

>> +

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


Ok.

> 

>> +

>> +  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?


Changed to:

+  if (!sve_vl_valid (header.vl))
+    {
+      warning (_("Invalid SVE state from kernel; SVE disabled."));
+      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.


Ok.

> 

>> +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 */
  

On 05/11/2018 11: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.

Note that since the cover email doesn't make it to the git repo, it's better to
do things the other way around, leave the details in the commit, and a
summary in the cover letter.  Someone looking at git log won't have a reference
to the cover letter.  Or, if it makes sense for readers of the code, as
opposed to just a rationale for a change, put it in comments instead.  (I don't
know whether that's true, just stating a principle.)

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

"well", "converters".

> +struct user_sve_header {
> +	__u32 size; /* total meaningful regset content in bytes */
> +	__u32 max_size; /* maxmium possible size for this thread */

"maximum"

Thanks,
Pedro Alves
  

On Friday, May 11 2018, 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.
>
> 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.

Hi Alan,

I'm trying to build GDB on Fedora Rawhide Aarch64, and I'm seeing the
following breakage:

    CXX    aarch64-linux-nat.o
  In file included from ../../gdb/aarch64-tdep.h:25,
                   from ../../gdb/aarch64-linux-nat.c:30:
  ../../gdb/arch/aarch64.h:79: error: "sve_vq_from_vl" redefined [-Werror]
   #define sve_vq_from_vl(vl) ((vl) / 0x10)

  In file included from /usr/include/bits/sigcontext.h:30,
                   from /usr/include/signal.h:291,
                   from build-gnulib/import/signal.h:52,
                   from ../../gdb/linux-nat.h:23,
                   from ../../gdb/aarch64-linux-nat.c:26:
  /usr/include/asm/sigcontext.h:154: note: this is the location of the previous definition
   #define sve_vq_from_vl(vl) ((vl) / SVE_VQ_BYTES)

  In file included from ../../gdb/aarch64-tdep.h:25,
                   from ../../gdb/aarch64-linux-nat.c:30:
  ../../gdb/arch/aarch64.h:80: error: "sve_vl_from_vq" redefined [-Werror]
   #define sve_vl_from_vq(vq) ((vq) * 0x10)

  In file included from /usr/include/bits/sigcontext.h:30,
                   from /usr/include/signal.h:291,
                   from build-gnulib/import/signal.h:52,
                   from ../../gdb/linux-nat.h:23,
                   from ../../gdb/aarch64-linux-nat.c:26:
  /usr/include/asm/sigcontext.h:155: note: this is the location of the previous definition
   #define sve_vl_from_vq(vq) ((vq) * SVE_VQ_BYTES)

    CXX    aarch32-linux-nat.o
    CXX    aarch64-linux-hw-point.o
    CXX    aarch64-linux.o
    CXX    aarch64-sve-linux-ptrace.o
    CXX    cli/cli-cmds.o
  In file included from ../../gdb/nat/aarch64-sve-linux-ptrace.c:26:
  ../../gdb/arch/aarch64.h:79: error: "sve_vq_from_vl" redefined [-Werror]
   #define sve_vq_from_vl(vl) ((vl) / 0x10)

  In file included from ../../gdb/nat/aarch64-sve-linux-ptrace.h:27,
                   from ../../gdb/nat/aarch64-sve-linux-ptrace.c:25:
  /usr/include/asm/sigcontext.h:154: note: this is the location of the previous definition
   #define sve_vq_from_vl(vl) ((vl) / SVE_VQ_BYTES)

  In file included from ../../gdb/nat/aarch64-sve-linux-ptrace.c:26:
  ../../gdb/arch/aarch64.h:80: error: "sve_vl_from_vq" redefined [-Werror]
   #define sve_vl_from_vq(vq) ((vq) * 0x10)

  In file included from ../../gdb/nat/aarch64-sve-linux-ptrace.h:27,
                   from ../../gdb/nat/aarch64-sve-linux-ptrace.c:25:
  /usr/include/asm/sigcontext.h:155: note: this is the location of the previous definition
   #define sve_vl_from_vq(vq) ((vq) * SVE_VQ_BYTES)


I think it may be a good idea to guard these #define's with #ifndef's.
I'm trying to get ahold of a machine to test this (the build failure
happened on a machine which I don't have direct control over).

Thanks,

> 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"
>  		;;
>  	    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).  */
> +
> +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;
> +
> +  long vq = sve_vq_from_vl (header.vl);
> +  gdb_assert (sve_vl_valid (header.vl));
> +
> +  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).  */
> +
> +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 */
> -- 
> 2.15.1 (Apple Git-101)