Re: [PATCH v2 02/10] Add Aarch64 SVE Linux headers

[Alan Hayward <Alan dot Hayward at arm dot com>]

(Moved review to correct thread)
Thanks for the reviews.

> On 7 Jun 2018, at 21:18, Simon Marchi <simon.marchi@ericsson.com> wrote:
> 
> Hi Alan,
> 
> Just some quick comments.
> 
> I get this when building on x86-64 with --enable-targets=all:

Hmm.. I had lost that flag from my build script. I Re-added it, and
reproduced the issues.

> 
>  CXX    aarch64-tdep.o
> In file included from /home/emaisin/src/binutils-gdb/gdb/nat/aarch64-sve-linux-ptrace.h:29:0,
>                 from /home/emaisin/src/binutils-gdb/gdb/aarch64-tdep.c:61:
> /home/emaisin/src/binutils-gdb/gdb/nat/aarch64-linux-sigcontext.h:19:22: error: field ‘head’ has incomplete type ‘_aarch64_ctx’
>  struct _aarch64_ctx head;
>                      ^
> /home/emaisin/src/binutils-gdb/gdb/nat/aarch64-linux-sigcontext.h:19:9: note: forward declaration of ‘struct _aarch64_ctx’
>  struct _aarch64_ctx head;
>         ^
> 
> First, we should not include "nat/aarch64-sve-linux-ptrace.h" (a file that only makes
> sense when building on AArch64) in aarch64-tdep.c, a file built on all architecture
> when including the support for AArch64 debugging.  It looks like aarch64-tdep.c
> needs sve_vq_from_vl.  Maybe that definition could be moved to arch/, which can be
> included in aarch64-tdep.c.
> 

I had put it in there because I wanted to try and make it a complete block
copied from Linux. The issue makes sense, so I’ve updated to restore
sve_vq_from_vl/sve_vl_from_vq back to arch/aarch64.h and removed it from
nat/aarch64-linux-sigcontext.h 


> Then, is the _aarch64_ctx structure guaranteed to be defined on older AArch64 kernels
> or should we include it too?


_aarch64_ctx is part of the standard aarch64 signal handling. A quick git blame gives
me 2012 - which is roughly the age of aarch64. So, it should always be defined.


Updated patch below. Checked it builds (with other sve patches) on:
X86 all-targets
Aarch64 Linux 4.10 (pre sve headers) ubuntu 16.04
Aarch64 Linux 4.15 (with sve headers) ubuntu 18.04

Are you ok with the new version?



diff --git a/gdb/nat/aarch64-linux-ptrace.h b/gdb/nat/aarch64-linux-ptrace.h
new file mode 100644
index 0000000000000000000000000000000000000000..1d0bf1b314038457632eef22e1c2d010d1604c93
--- /dev/null
+++ b/gdb/nat/aarch64-linux-ptrace.h
@@ -0,0 +1,150 @@
+/* This file contains Aarch64 Linux ptrace defines. It is required for those
+   compiling with older kernel headers.  Eventually, when the kernel defines are
+   old enough, this file should be removed.
+
+   Contents are directly copied directly from
+   linux/arch/arm64/include/uapi/asm/ptrace.h in Linux 4.17.
+
+   See:
+   https://github.com/torvalds/linux/blob/v4.17/arch/arm64/include/uapi/asm/ptrace.h
+*/
+
+#ifndef AARCH64_LINUX_PTRACE_H
+#define AARCH64_LINUX_PTRACE_H
+
+/* SVE/FP/SIMD state (NT_ARM_SVE) */
+
+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;
+};
+
+/* Definitions for user_sve_header.flags: */
+#define SVE_PT_REGS_MASK		(1 << 0)
+
+#define SVE_PT_REGS_FPSIMD		0
+#define SVE_PT_REGS_SVE			SVE_PT_REGS_MASK
+
+/*
+ * Common SVE_PT_* flags:
+ * These must be kept in sync with prctl interface in <linux/ptrace.h>
+ */
+#define SVE_PT_VL_INHERIT		(PR_SVE_VL_INHERIT >> 16)
+#define SVE_PT_VL_ONEXEC		(PR_SVE_SET_VL_ONEXEC >> 16)
+
+
+/*
+ * The remainder of the SVE state follows struct user_sve_header.  The
+ * total size of the SVE state (including header) depends on the
+ * metadata in the header:  SVE_PT_SIZE(vq, flags) gives the total size
+ * of the state in bytes, including the header.
+ *
+ * Refer to <asm/sigcontext.h> for details of how to pass the correct
+ * "vq" argument to these macros.
+ */
+
+/* Offset from the start of struct user_sve_header to the register data */
+#define SVE_PT_REGS_OFFSET					\
+	((sizeof(struct sve_context) + (SVE_VQ_BYTES - 1))	\
+		/ SVE_VQ_BYTES * SVE_VQ_BYTES)
+
+/*
+ * The register data content and layout depends on the value of the
+ * flags field.
+ */
+
+/*
+ * (flags & SVE_PT_REGS_MASK) == SVE_PT_REGS_FPSIMD case:
+ *
+ * The payload starts at offset SVE_PT_FPSIMD_OFFSET, and is of type
+ * struct user_fpsimd_state.  Additional data might be appended in the
+ * future: use SVE_PT_FPSIMD_SIZE(vq, flags) to compute the total size.
+ * SVE_PT_FPSIMD_SIZE(vq, flags) will never be less than
+ * sizeof(struct user_fpsimd_state).
+ */
+
+#define SVE_PT_FPSIMD_OFFSET		SVE_PT_REGS_OFFSET
+
+#define SVE_PT_FPSIMD_SIZE(vq, flags)	(sizeof(struct user_fpsimd_state))
+
+/*
+ * (flags & SVE_PT_REGS_MASK) == SVE_PT_REGS_SVE case:
+ *
+ * The payload starts at offset SVE_PT_SVE_OFFSET, and is of size
+ * SVE_PT_SVE_SIZE(vq, flags).
+ *
+ * Additional macros describe the contents and layout of the payload.
+ * For each, SVE_PT_SVE_x_OFFSET(args) is the start offset relative to
+ * the start of struct user_sve_header, and SVE_PT_SVE_x_SIZE(args) is
+ * the size in bytes:
+ *
+ *	x	type				description
+ *	-	----				-----------
+ *	ZREGS		\
+ *	ZREG		|
+ *	PREGS		| refer to <asm/sigcontext.h>
+ *	PREG		|
+ *	FFR		/
+ *
+ *	FPSR	uint32_t			FPSR
+ *	FPCR	uint32_t			FPCR
+ *
+ * Additional data might be appended in the future.
+ */
+
+#define SVE_PT_SVE_ZREG_SIZE(vq)	SVE_SIG_ZREG_SIZE(vq)
+#define SVE_PT_SVE_PREG_SIZE(vq)	SVE_SIG_PREG_SIZE(vq)
+#define SVE_PT_SVE_FFR_SIZE(vq)		SVE_SIG_FFR_SIZE(vq)
+#define SVE_PT_SVE_FPSR_SIZE		sizeof(__u32)
+#define SVE_PT_SVE_FPCR_SIZE		sizeof(__u32)
+
+#define __SVE_SIG_TO_PT(offset) \
+	((offset) - SVE_SIG_REGS_OFFSET + SVE_PT_REGS_OFFSET)
+
+#define SVE_PT_SVE_OFFSET		SVE_PT_REGS_OFFSET
+
+#define SVE_PT_SVE_ZREGS_OFFSET \
+	__SVE_SIG_TO_PT(SVE_SIG_ZREGS_OFFSET)
+#define SVE_PT_SVE_ZREG_OFFSET(vq, n) \
+	__SVE_SIG_TO_PT(SVE_SIG_ZREG_OFFSET(vq, n))
+#define SVE_PT_SVE_ZREGS_SIZE(vq) \
+	(SVE_PT_SVE_ZREG_OFFSET(vq, SVE_NUM_ZREGS) - SVE_PT_SVE_ZREGS_OFFSET)
+
+#define SVE_PT_SVE_PREGS_OFFSET(vq) \
+	__SVE_SIG_TO_PT(SVE_SIG_PREGS_OFFSET(vq))
+#define SVE_PT_SVE_PREG_OFFSET(vq, n) \
+	__SVE_SIG_TO_PT(SVE_SIG_PREG_OFFSET(vq, n))
+#define SVE_PT_SVE_PREGS_SIZE(vq) \
+	(SVE_PT_SVE_PREG_OFFSET(vq, SVE_NUM_PREGS) - \
+		SVE_PT_SVE_PREGS_OFFSET(vq))
+
+#define SVE_PT_SVE_FFR_OFFSET(vq) \
+	__SVE_SIG_TO_PT(SVE_SIG_FFR_OFFSET(vq))
+
+#define SVE_PT_SVE_FPSR_OFFSET(vq)				\
+	((SVE_PT_SVE_FFR_OFFSET(vq) + SVE_PT_SVE_FFR_SIZE(vq) +	\
+			(SVE_VQ_BYTES - 1))			\
+		/ SVE_VQ_BYTES * SVE_VQ_BYTES)
+#define SVE_PT_SVE_FPCR_OFFSET(vq) \
+	(SVE_PT_SVE_FPSR_OFFSET(vq) + SVE_PT_SVE_FPSR_SIZE)
+
+/*
+ * Any future extension appended after FPCR must be aligned to the next
+ * 128-bit boundary.
+ */
+
+#define SVE_PT_SVE_SIZE(vq, flags)					\
+	((SVE_PT_SVE_FPCR_OFFSET(vq) + SVE_PT_SVE_FPCR_SIZE		\
+			- SVE_PT_SVE_OFFSET + (SVE_VQ_BYTES - 1))	\
+		/ SVE_VQ_BYTES * SVE_VQ_BYTES)
+
+#define SVE_PT_SIZE(vq, flags)						\
+	 (((flags) & SVE_PT_REGS_MASK) == SVE_PT_REGS_SVE ?		\
+		  SVE_PT_SVE_OFFSET + SVE_PT_SVE_SIZE(vq, flags)	\
+		: SVE_PT_FPSIMD_OFFSET + SVE_PT_FPSIMD_SIZE(vq, flags))
+
+#endif /* AARCH64_LINUX_PTRACE_H */
diff --git a/gdb/nat/aarch64-linux-sigcontext.h b/gdb/nat/aarch64-linux-sigcontext.h
new file mode 100644
index 0000000000000000000000000000000000000000..7253b85cc1f28859a68293c02d87052a48aa567f
--- /dev/null
+++ b/gdb/nat/aarch64-linux-sigcontext.h
@@ -0,0 +1,127 @@
+/* This file contains Aarch64 Linux sigcontext defines. It is required for those
+   compiling with older kernel headers.  Eventually, when the kernel defines are
+   old enough, this file should be removed.
+
+   Contents are directly copied directly from
+   linux/arch/arm64/include/uapi/asm/sigcontext.h in Linux 4.17.
+
+   See:
+   https://github.com/torvalds/linux/blob/v4.17/arch/arm64/include/uapi/asm/sigcontext.h
+*/
+
+#ifndef AARCH64_LINUX_SIGCONTEXT_H
+#define AARCH64_LINUX_SIGCONTEXT_H
+
+
+#define SVE_MAGIC	0x53564501
+
+struct sve_context {
+	struct _aarch64_ctx head;
+	__u16 vl;
+	__u16 __reserved[3];
+};
+
+/*
+ * The SVE architecture leaves space for future expansion of the
+ * vector length beyond its initial architectural limit of 2048 bits
+ * (16 quadwords).
+ *
+ * See linux/Documentation/arm64/sve.txt for a description of the VL/VQ
+ * terminology.
+ */
+#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)
+
+/*
+ * If the SVE registers are currently live for the thread at signal delivery,
+ * sve_context.head.size >=
+ *	SVE_SIG_CONTEXT_SIZE(sve_vq_from_vl(sve_context.vl))
+ * and the register data may be accessed using the SVE_SIG_*() macros.
+ *
+ * If sve_context.head.size <
+ *	SVE_SIG_CONTEXT_SIZE(sve_vq_from_vl(sve_context.vl)),
+ * the SVE registers were not live for the thread and no register data
+ * is included: in this case, the SVE_SIG_*() macros should not be
+ * used except for this check.
+ *
+ * The same convention applies when returning from a signal: a caller
+ * will need to remove or resize the sve_context block if it wants to
+ * make the SVE registers live when they were previously non-live or
+ * vice-versa.  This may require the the caller to allocate fresh
+ * memory and/or move other context blocks in the signal frame.
+ *
+ * Changing the vector length during signal return is not permitted:
+ * sve_context.vl must equal the thread's current vector length when
+ * doing a sigreturn.
+ *
+ *
+ * Note: for all these macros, the "vq" argument denotes the SVE
+ * vector length in quadwords (i.e., units of 128 bits).
+ *
+ * The correct way to obtain vq is to use sve_vq_from_vl(vl).  The
+ * result is valid if and only if sve_vl_valid(vl) is true.  This is
+ * guaranteed for a struct sve_context written by the kernel.
+ *
+ *
+ * Additional macros describe the contents and layout of the payload.
+ * For each, SVE_SIG_x_OFFSET(args) is the start offset relative to
+ * the start of struct sve_context, and SVE_SIG_x_SIZE(args) is the
+ * size in bytes:
+ *
+ *	x	type				description
+ *	-	----				-----------
+ *	REGS					the entire SVE context
+ *
+ *	ZREGS	__uint128_t[SVE_NUM_ZREGS][vq]	all Z-registers
+ *	ZREG	__uint128_t[vq]			individual Z-register Zn
+ *
+ *	PREGS	uint16_t[SVE_NUM_PREGS][vq]	all P-registers
+ *	PREG	uint16_t[vq]			individual P-register Pn
+ *
+ *	FFR	uint16_t[vq]			first-fault status register
+ *
+ * Additional data might be appended in the future.
+ */
+
+#define SVE_SIG_ZREG_SIZE(vq)	((__u32)(vq) * SVE_VQ_BYTES)
+#define SVE_SIG_PREG_SIZE(vq)	((__u32)(vq) * (SVE_VQ_BYTES / 8))
+#define SVE_SIG_FFR_SIZE(vq)	SVE_SIG_PREG_SIZE(vq)
+
+#define SVE_SIG_REGS_OFFSET					\
+	((sizeof(struct sve_context) + (SVE_VQ_BYTES - 1))	\
+		/ SVE_VQ_BYTES * SVE_VQ_BYTES)
+
+#define SVE_SIG_ZREGS_OFFSET	SVE_SIG_REGS_OFFSET
+#define SVE_SIG_ZREG_OFFSET(vq, n) \
+	(SVE_SIG_ZREGS_OFFSET + SVE_SIG_ZREG_SIZE(vq) * (n))
+#define SVE_SIG_ZREGS_SIZE(vq) \
+	(SVE_SIG_ZREG_OFFSET(vq, SVE_NUM_ZREGS) - SVE_SIG_ZREGS_OFFSET)
+
+#define SVE_SIG_PREGS_OFFSET(vq) \
+	(SVE_SIG_ZREGS_OFFSET + SVE_SIG_ZREGS_SIZE(vq))
+#define SVE_SIG_PREG_OFFSET(vq, n) \
+	(SVE_SIG_PREGS_OFFSET(vq) + SVE_SIG_PREG_SIZE(vq) * (n))
+#define SVE_SIG_PREGS_SIZE(vq) \
+	(SVE_SIG_PREG_OFFSET(vq, SVE_NUM_PREGS) - SVE_SIG_PREGS_OFFSET(vq))
+
+#define SVE_SIG_FFR_OFFSET(vq) \
+	(SVE_SIG_PREGS_OFFSET(vq) + SVE_SIG_PREGS_SIZE(vq))
+
+#define SVE_SIG_REGS_SIZE(vq) \
+	(SVE_SIG_FFR_OFFSET(vq) + SVE_SIG_FFR_SIZE(vq) - SVE_SIG_REGS_OFFSET)
+
+#define SVE_SIG_CONTEXT_SIZE(vq) (SVE_SIG_REGS_OFFSET + SVE_SIG_REGS_SIZE(vq))
+
+
+#endif /* AARCH64_LINUX_SIGCONTEXT_H */
diff --git a/gdb/nat/aarch64-sve-linux-ptrace.h b/gdb/nat/aarch64-sve-linux-ptrace.h
index 61f841466c8279c14322894e4cedbe3b6e39db4b..2d6f5714c0fd77cd51142500ba04dd0a70717d2d 100644
--- a/gdb/nat/aarch64-sve-linux-ptrace.h
+++ b/gdb/nat/aarch64-sve-linux-ptrace.h
@@ -20,54 +20,22 @@
 #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 zero).  */
-
-uint64_t 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.  */
+#include "aarch64-linux-sigcontext.h"
+#endif

 #ifndef SVE_PT_SVE_ZREG_SIZE
+#include "aarch64-linux-ptrace.h"
+#endif

-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;
-};
+/* 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 zero).  */

-#endif /* SVE_PT_SVE_ZREG_SIZE.  */
+uint64_t aarch64_sve_get_vq (int tid);

 #endif /* aarch64-sve-linux-ptrace.h */