From 48574d91bf1289074f2c88b1f83aa3cd37d524d9 Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Mon, 15 Apr 2019 12:31:21 +0100 Subject: [PATCH] AArch64 SVE: Support changing vector lengths for ptrace When writing registers to the kernel, check if regcache VG has been changed. If so then update the thread's vector length, then write back the registers. When reading registers from the kernel, ensure regcache VG register is updated. The regcache registers should already be of the correct length. Remove all the checks that error if the vector length has changed. gdb/ChangeLog: * aarch64-linux-nat.c (store_sveregs_to_thread): Set vector length. * nat/aarch64-sve-linux-ptrace.c (aarch64_sve_set_vq): New function. (aarch64_sve_regs_copy_to_reg_buf): Remove VG checks. (aarch64_sve_regs_copy_from_reg_buf): Likewise. * nat/aarch64-sve-linux-ptrace.h (aarch64_sve_set_vq): New declaration. --- gdb/ChangeLog | 10 ++++ gdb/aarch64-linux-nat.c | 5 ++ gdb/nat/aarch64-sve-linux-ptrace.c | 92 ++++++++++++++++-------------- gdb/nat/aarch64-sve-linux-ptrace.h | 12 +++- 4 files changed, 73 insertions(+), 46 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index b3155ad7443..4e73bf7e279 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,13 @@ +2019-04-15 Alan Hayward + + * aarch64-linux-nat.c (store_sveregs_to_thread): Set vector length. + * nat/aarch64-sve-linux-ptrace.c (aarch64_sve_set_vq): New + function. + (aarch64_sve_regs_copy_to_reg_buf): Remove VG checks. + (aarch64_sve_regs_copy_from_reg_buf): Likewise. + * nat/aarch64-sve-linux-ptrace.h (aarch64_sve_set_vq): New + declaration. + 2019-04-15 Alan Hayward * aarch64-linux-nat.c diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c index c5070c8590d..8ca96143013 100644 --- a/gdb/aarch64-linux-nat.c +++ b/gdb/aarch64-linux-nat.c @@ -412,6 +412,11 @@ store_sveregs_to_thread (struct regcache *regcache) struct iovec iovec; int tid = regcache->ptid ().lwp (); + /* First store vector length to the thread. This is done first to ensure the + ptrace buffers read from the kernel are the correct size. */ + if (!aarch64_sve_set_vq (tid, regcache)) + perror_with_name (_("Unable to set VG register.")); + /* Obtain a dump of SVE registers from ptrace. */ std::unique_ptr base = aarch64_sve_get_sveregs (tid); diff --git a/gdb/nat/aarch64-sve-linux-ptrace.c b/gdb/nat/aarch64-sve-linux-ptrace.c index 30faab22bb1..635b4c9d68a 100644 --- a/gdb/nat/aarch64-sve-linux-ptrace.c +++ b/gdb/nat/aarch64-sve-linux-ptrace.c @@ -27,8 +27,6 @@ #include "common/common-regcache.h" #include "common/byte-vector.h" -static bool vq_change_warned = false; - /* See nat/aarch64-sve-linux-ptrace.h. */ uint64_t @@ -63,6 +61,48 @@ aarch64_sve_get_vq (int tid) /* See nat/aarch64-sve-linux-ptrace.h. */ +bool +aarch64_sve_set_vq (int tid, uint64_t vq) +{ + struct iovec iovec; + struct user_sve_header header; + + iovec.iov_len = sizeof (header); + iovec.iov_base = &header; + + if (ptrace (PTRACE_GETREGSET, tid, NT_ARM_SVE, &iovec) < 0) + { + /* SVE is not supported. */ + return false; + } + + header.vl = sve_vl_from_vq (vq); + + if (ptrace (PTRACE_SETREGSET, tid, NT_ARM_SVE, &iovec) < 0) + { + /* Vector length change failed. */ + return false; + } + + return true; +} + +/* See nat/aarch64-sve-linux-ptrace.h. */ + +bool +aarch64_sve_set_vq (int tid, struct reg_buffer_common *reg_buf) +{ + if (reg_buf->get_register_status (AARCH64_SVE_VG_REGNUM) != REG_VALID) + return false; + + uint64_t reg_vg = 0; + reg_buf->raw_collect (AARCH64_SVE_VG_REGNUM, ®_vg); + + return aarch64_sve_set_vq (tid, sve_vq_from_vg (reg_vg)); +} + +/* See nat/aarch64-sve-linux-ptrace.h. */ + std::unique_ptr aarch64_sve_get_sveregs (int tid) { @@ -95,37 +135,18 @@ aarch64_sve_regs_copy_to_reg_buf (struct reg_buffer_common *reg_buf, { char *base = (char *) buf; struct user_sve_header *header = (struct user_sve_header *) buf; - uint64_t vq, vg_reg_buf = 0; - vq = sve_vq_from_vl (header->vl); + uint64_t vq = sve_vq_from_vl (header->vl); + uint64_t vg = sve_vg_from_vl (header->vl); /* Sanity check the data in the header. */ if (!sve_vl_valid (header->vl) || SVE_PT_SIZE (vq, header->flags) != header->size) error (_("Invalid SVE header from kernel.")); - if (REG_VALID == reg_buf->get_register_status (AARCH64_SVE_VG_REGNUM)) - reg_buf->raw_collect (AARCH64_SVE_VG_REGNUM, &vg_reg_buf); - - if (vg_reg_buf == 0) - { - /* VG has not been set. */ - vg_reg_buf = sve_vg_from_vl (header->vl); - reg_buf->raw_supply (AARCH64_SVE_VG_REGNUM, &vg_reg_buf); - } - else if (vg_reg_buf != sve_vg_from_vl (header->vl) && !vq_change_warned) - { - /* Vector length on the running process has changed. GDB currently does - not support this and will result in GDB showing incorrect partially - incorrect data for the vector registers. Warn once and continue. We - do not expect many programs to exhibit this behaviour. To fix this - we need to spot the change earlier and generate a new target - descriptor. */ - warning (_("SVE Vector length has changed (%ld to %d). " - "Vector registers may show incorrect data."), - vg_reg_buf, sve_vg_from_vl (header->vl)); - vq_change_warned = true; - } + /* Update VG. Note, the registers in the regcache will already be of the + correct length. */ + reg_buf->raw_supply (AARCH64_SVE_VG_REGNUM, &vg); if (HAS_SVE_STATE (*header)) { @@ -187,30 +208,13 @@ aarch64_sve_regs_copy_from_reg_buf (const struct reg_buffer_common *reg_buf, { struct user_sve_header *header = (struct user_sve_header *) buf; char *base = (char *) buf; - uint64_t vq, vg_reg_buf = 0; - - vq = sve_vq_from_vl (header->vl); + uint64_t vq = sve_vq_from_vl (header->vl); /* Sanity check the data in the header. */ if (!sve_vl_valid (header->vl) || SVE_PT_SIZE (vq, header->flags) != header->size) error (_("Invalid SVE header from kernel.")); - if (REG_VALID == reg_buf->get_register_status (AARCH64_SVE_VG_REGNUM)) - reg_buf->raw_collect (AARCH64_SVE_VG_REGNUM, &vg_reg_buf); - - if (vg_reg_buf != 0 && vg_reg_buf != sve_vg_from_vl (header->vl)) - { - /* Vector length on the running process has changed. GDB currently does - not support this and will result in GDB writing invalid data back to - the vector registers. Error and exit. We do not expect many programs - to exhibit this behaviour. To fix this we need to spot the change - earlier and generate a new target descriptor. */ - error (_("SVE Vector length has changed (%ld to %d). " - "Cannot write back registers."), - vg_reg_buf, sve_vg_from_vl (header->vl)); - } - if (!HAS_SVE_STATE (*header)) { /* There is no SVE state yet - the register dump contains a fpsimd diff --git a/gdb/nat/aarch64-sve-linux-ptrace.h b/gdb/nat/aarch64-sve-linux-ptrace.h index 167fc8ef3cf..ee994f26c6f 100644 --- a/gdb/nat/aarch64-sve-linux-ptrace.h +++ b/gdb/nat/aarch64-sve-linux-ptrace.h @@ -39,17 +39,25 @@ uint64_t aarch64_sve_get_vq (int tid); +/* Set VQ in the kernel for the given tid, using either the value VQ or + reading from the register VG in the register buffer. */ + +bool aarch64_sve_set_vq (int tid, uint64_t vq); +bool aarch64_sve_set_vq (int tid, struct reg_buffer_common *reg_buf); + /* Read the current SVE register set using ptrace, allocating space as required. */ extern std::unique_ptr aarch64_sve_get_sveregs (int tid); -/* Put the registers from linux structure buf into register buffer. */ +/* Put the registers from linux structure buf into register buffer. Assumes the + vector lengths in the register buffer match the size in the kernel. */ extern void aarch64_sve_regs_copy_to_reg_buf (struct reg_buffer_common *reg_buf, const void *buf); -/* Put the registers from register buffer into linux structure buf. */ +/* Put the registers from register buffer into linux structure buf. Assumes the + vector lengths in the register buffer match the size in the kernel. */ extern void aarch64_sve_regs_copy_from_reg_buf (const struct reg_buffer_common *reg_buf, -- 2.30.2