(Moved review to correct thread) Thanks for the reviews. > On 7 Jun 2018, at 21:18, Simon Marchi 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 + */ +#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 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 + * 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 #include #include #include -/* 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 */ &j!z޶׏4Ib֫rnr