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

Alan Hayward
2018-06-08T14:13:12Z

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 14439 bytes --]

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

\x16º&Öéj×!zÊÞ¶êç×4ïIb²Ö«r\x18\x1dn–­r\x17¬