[v2,3/3] Parse SVE registers in aarch64 core file reading/writing
Commit Message
sve_regmap cannot be global static as the size is dependant on the current
vector length.
2018-07-30 Alan Hayward <alan.hayward@arm.com>
* aarch64-linux-tdep.c (aarch64_linux_supply_sve_regset): New function.
(aarch64_linux_collect_sve_regset): Likewise.
(aarch64_linux_iterate_over_regset_sections): Check for SVE.
* regcache.h (regcache_map_entry_size): New function.
---
gdb/aarch64-linux-tdep.c | 112 ++++++++++++++++++++++++++++++++++++++++++++++-
gdb/regcache.h | 8 ++++
2 files changed, 118 insertions(+), 2 deletions(-)
Comments
On 2018-07-30 05:25 AM, Alan Hayward wrote:
> sve_regmap cannot be global static as the size is dependant on the current
> vector length.
>
> 2018-07-30 Alan Hayward <alan.hayward@arm.com>
>
> * aarch64-linux-tdep.c (aarch64_linux_supply_sve_regset): New function.
> (aarch64_linux_collect_sve_regset): Likewise.
> (aarch64_linux_iterate_over_regset_sections): Check for SVE.
> * regcache.h (regcache_map_entry_size): New function.
> ---
> gdb/aarch64-linux-tdep.c | 112 ++++++++++++++++++++++++++++++++++++++++++++++-
> gdb/regcache.h | 8 ++++
> 2 files changed, 118 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
> index f9a95950da..bd61a2d722 100644
> --- a/gdb/aarch64-linux-tdep.c
> +++ b/gdb/aarch64-linux-tdep.c
> @@ -288,6 +288,85 @@ aarch64_linux_core_read_vq (struct gdbarch *gdbarch, bfd *abfd)
> return vq;
> }
>
> +/* Supply register REGNUM from BUF to REGCACHE, using the register map
> + in REGSET. If REGNUM is -1, do this for all registers in REGSET.
> + If BUF is NULL, set the registers to "unavailable" status. */
> +
> +static void
> +aarch64_linux_supply_sve_regset (const struct regset *regset,
> + struct regcache *regcache,
> + int regnum, const void *buf, size_t size)
> +{
> + struct gdbarch *gdbarch = regcache->arch ();
> + enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> +
> + if (buf == nullptr)
> + return regcache->supply_regset (regset, regnum, nullptr, size);
> + gdb_assert (size > SVE_HEADER_SIZE);
> +
> + /* BUF contains an SVE header followed by a register dump of either the
> + passed in SVE regset or a NEON fpregset. */
> +
> + /* Extract required fields from the header. */
> + uint64_t vg = sve_vg_from_vl (SVE_HEADER_READ (buf, 2, byte_order));
> + uint16_t flags = SVE_HEADER_READ (buf, 4, byte_order);
> +
> + if (regnum == -1 || regnum == AARCH64_SVE_VG_REGNUM)
> + regcache->raw_supply (AARCH64_SVE_VG_REGNUM, &vg);
I think this raw_supply is wrong. The vg local variable is in host byte order,
but the data in the buffer passed to raw_supply should be in target. So you
may need to do a store_unsigned_integer in a buffer and supply that. If this
is a pattern that happens often enough, maybe it would be worth having a
raw_supply that does this conversion from integer to register buffer, a bit
like "regcache::raw_write (int regnum, T val)" does.
> +
> + if (flags & 1)
> + {
> + /* Register dump is a SVE structure. */
> + regcache->supply_regset (regset, regnum,
> + (gdb_byte *) buf + SVE_HEADER_SIZE,
> + size - SVE_HEADER_SIZE);
> + }
> + else
> + {
> + /* Register dump is a fpsimd structure. First clear the SVE
> + registers. */
> + for (int i = 0; i < AARCH64_SVE_Z_REGS_NUM; i++)
> + regcache->raw_supply_zeroed (AARCH64_SVE_Z0_REGNUM + i);
> + for (int i = 0; i < AARCH64_SVE_P_REGS_NUM; i++)
> + regcache->raw_supply_zeroed (AARCH64_SVE_P0_REGNUM + i);
> + regcache->raw_supply_zeroed (AARCH64_SVE_FFR_REGNUM);
Just wondering, should they be made unavailable instead of cleared?
Simon
> On 6 Aug 2018, at 19:29, Simon Marchi <simon.marchi@ericsson.com> wrote:
>
> On 2018-07-30 05:25 AM, Alan Hayward wrote:
>> sve_regmap cannot be global static as the size is dependant on the current
>> vector length.
>>
>> 2018-07-30 Alan Hayward <alan.hayward@arm.com>
>>
>> * aarch64-linux-tdep.c (aarch64_linux_supply_sve_regset): New function.
>> (aarch64_linux_collect_sve_regset): Likewise.
>> (aarch64_linux_iterate_over_regset_sections): Check for SVE.
>> * regcache.h (regcache_map_entry_size): New function.
>> ---
>> gdb/aarch64-linux-tdep.c | 112 ++++++++++++++++++++++++++++++++++++++++++++++-
>> gdb/regcache.h | 8 ++++
>> 2 files changed, 118 insertions(+), 2 deletions(-)
>>
>> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
>> index f9a95950da..bd61a2d722 100644
>> --- a/gdb/aarch64-linux-tdep.c
>> +++ b/gdb/aarch64-linux-tdep.c
>> @@ -288,6 +288,85 @@ aarch64_linux_core_read_vq (struct gdbarch *gdbarch, bfd *abfd)
>> return vq;
>> }
>>
>> +/* Supply register REGNUM from BUF to REGCACHE, using the register map
>> + in REGSET. If REGNUM is -1, do this for all registers in REGSET.
>> + If BUF is NULL, set the registers to "unavailable" status. */
>> +
>> +static void
>> +aarch64_linux_supply_sve_regset (const struct regset *regset,
>> + struct regcache *regcache,
>> + int regnum, const void *buf, size_t size)
>> +{
>> + struct gdbarch *gdbarch = regcache->arch ();
>> + enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>> +
>> + if (buf == nullptr)
>> + return regcache->supply_regset (regset, regnum, nullptr, size);
>> + gdb_assert (size > SVE_HEADER_SIZE);
>> +
>> + /* BUF contains an SVE header followed by a register dump of either the
>> + passed in SVE regset or a NEON fpregset. */
>> +
>> + /* Extract required fields from the header. */
>> + uint64_t vg = sve_vg_from_vl (SVE_HEADER_READ (buf, 2, byte_order));
>> + uint16_t flags = SVE_HEADER_READ (buf, 4, byte_order);
>> +
>> + if (regnum == -1 || regnum == AARCH64_SVE_VG_REGNUM)
>> + regcache->raw_supply (AARCH64_SVE_VG_REGNUM, &vg);
>
> I think this raw_supply is wrong. The vg local variable is in host byte order,
> but the data in the buffer passed to raw_supply should be in target. So you
> may need to do a store_unsigned_integer in a buffer and supply that. If this
> is a pattern that happens often enough, maybe it would be worth having a
> raw_supply that does this conversion from integer to register buffer, a bit
> like "regcache::raw_write (int regnum, T val)" does.
>
Yes! I’ll consider adding the function too.
>> +
>> + if (flags & 1)
>> + {
>> + /* Register dump is a SVE structure. */
>> + regcache->supply_regset (regset, regnum,
>> + (gdb_byte *) buf + SVE_HEADER_SIZE,
>> + size - SVE_HEADER_SIZE);
>> + }
>> + else
>> + {
>> + /* Register dump is a fpsimd structure. First clear the SVE
>> + registers. */
>> + for (int i = 0; i < AARCH64_SVE_Z_REGS_NUM; i++)
>> + regcache->raw_supply_zeroed (AARCH64_SVE_Z0_REGNUM + i);
>> + for (int i = 0; i < AARCH64_SVE_P_REGS_NUM; i++)
>> + regcache->raw_supply_zeroed (AARCH64_SVE_P0_REGNUM + i);
>> + regcache->raw_supply_zeroed (AARCH64_SVE_FFR_REGNUM);
>
> Just wondering, should they be made unavailable instead of cleared?
>
No. The supply_regset directly afterwards will write to the V registers.
With SVE, the V registers are pseudo registers built off the Z registers.
I do the clear to ensure the non overlapping parts of the Z registers are cleared.
@@ -288,6 +288,85 @@ aarch64_linux_core_read_vq (struct gdbarch *gdbarch, bfd *abfd)
return vq;
}
+/* Supply register REGNUM from BUF to REGCACHE, using the register map
+ in REGSET. If REGNUM is -1, do this for all registers in REGSET.
+ If BUF is NULL, set the registers to "unavailable" status. */
+
+static void
+aarch64_linux_supply_sve_regset (const struct regset *regset,
+ struct regcache *regcache,
+ int regnum, const void *buf, size_t size)
+{
+ struct gdbarch *gdbarch = regcache->arch ();
+ enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+
+ if (buf == nullptr)
+ return regcache->supply_regset (regset, regnum, nullptr, size);
+ gdb_assert (size > SVE_HEADER_SIZE);
+
+ /* BUF contains an SVE header followed by a register dump of either the
+ passed in SVE regset or a NEON fpregset. */
+
+ /* Extract required fields from the header. */
+ uint64_t vg = sve_vg_from_vl (SVE_HEADER_READ (buf, 2, byte_order));
+ uint16_t flags = SVE_HEADER_READ (buf, 4, byte_order);
+
+ if (regnum == -1 || regnum == AARCH64_SVE_VG_REGNUM)
+ regcache->raw_supply (AARCH64_SVE_VG_REGNUM, &vg);
+
+ if (flags & 1)
+ {
+ /* Register dump is a SVE structure. */
+ regcache->supply_regset (regset, regnum,
+ (gdb_byte *) buf + SVE_HEADER_SIZE,
+ size - SVE_HEADER_SIZE);
+ }
+ else
+ {
+ /* Register dump is a fpsimd structure. First clear the SVE
+ registers. */
+ for (int i = 0; i < AARCH64_SVE_Z_REGS_NUM; i++)
+ regcache->raw_supply_zeroed (AARCH64_SVE_Z0_REGNUM + i);
+ for (int i = 0; i < AARCH64_SVE_P_REGS_NUM; i++)
+ regcache->raw_supply_zeroed (AARCH64_SVE_P0_REGNUM + i);
+ regcache->raw_supply_zeroed (AARCH64_SVE_FFR_REGNUM);
+
+ /* Then supply the fpsimd registers. */
+ regcache->supply_regset (&aarch64_linux_fpregset, regnum,
+ (gdb_byte *) buf + SVE_HEADER_SIZE,
+ size - SVE_HEADER_SIZE);
+ }
+}
+
+/* Collect register REGNUM from REGCACHE to BUF, using the register
+ map in REGSET. If REGNUM is -1, do this for all registers in
+ REGSET. */
+
+static void
+aarch64_linux_collect_sve_regset (const struct regset *regset,
+ const struct regcache *regcache,
+ int regnum, void *buf, size_t size)
+{
+ struct gdbarch *gdbarch = regcache->arch ();
+ enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+ uint64_t vq = gdbarch_tdep (gdbarch)->vq;
+
+ gdb_assert (buf != NULL);
+ gdb_assert (size > SVE_HEADER_SIZE);
+
+ /* BUF starts with a SVE header prior to the register dump. */
+ SVE_HEADER_WRITE (buf, 0, byte_order, size);
+ SVE_HEADER_WRITE (buf, 1, byte_order, size);
+ SVE_HEADER_WRITE (buf, 2, byte_order, sve_vl_from_vq (vq));
+ SVE_HEADER_WRITE (buf, 3, byte_order, sve_vl_from_vq (vq));
+ SVE_HEADER_WRITE (buf, 4, byte_order, 1);
+ SVE_HEADER_WRITE (buf, 5, byte_order, 0);
+
+ /* The SVE register dump follows. */
+ regcache->collect_regset (regset, regnum, (gdb_byte *) buf + SVE_HEADER_SIZE,
+ size - SVE_HEADER_SIZE);
+}
+
/* Implement the "regset_from_core_section" gdbarch method. */
static void
@@ -296,10 +375,39 @@ aarch64_linux_iterate_over_regset_sections (struct gdbarch *gdbarch,
void *cb_data,
const struct regcache *regcache)
{
+ struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
cb (".reg", AARCH64_LINUX_SIZEOF_GREGSET, AARCH64_LINUX_SIZEOF_GREGSET,
&aarch64_linux_gregset, NULL, cb_data);
- cb (".reg2", AARCH64_LINUX_SIZEOF_FPREGSET, AARCH64_LINUX_SIZEOF_FPREGSET,
- &aarch64_linux_fpregset, NULL, cb_data);
+
+ if (tdep->has_sve ())
+ {
+ /* Create this on the fly in order to handle vector register sizes. */
+ const struct regcache_map_entry sve_regmap[] =
+ {
+ { 32, AARCH64_SVE_Z0_REGNUM, tdep->vq * 16 },
+ { 16, AARCH64_SVE_P0_REGNUM, tdep->vq * 16 / 8 },
+ { 1, AARCH64_SVE_FFR_REGNUM, 4 },
+ { 1, AARCH64_FPSR_REGNUM, 4 },
+ { 1, AARCH64_FPCR_REGNUM, 4 },
+ { 0 }
+ };
+
+ const struct regset aarch64_linux_sve_regset =
+ {
+ sve_regmap,
+ aarch64_linux_supply_sve_regset, aarch64_linux_collect_sve_regset,
+ REGSET_VARIABLE_SIZE
+ };
+
+ cb (".reg-aarch-sve",
+ SVE_HEADER_SIZE + regcache_map_entry_size (aarch64_linux_fpregmap),
+ SVE_HEADER_SIZE + regcache_map_entry_size (sve_regmap),
+ &aarch64_linux_sve_regset, "SVE registers", cb_data);
+ }
+ else
+ cb (".reg2", AARCH64_LINUX_SIZEOF_FPREGSET, AARCH64_LINUX_SIZEOF_FPREGSET,
+ &aarch64_linux_fpregset, NULL, cb_data);
}
/* Implement the "core_read_description" gdbarch method. */
@@ -92,6 +92,14 @@ struct regcache_map_entry
int size;
};
+static inline int regcache_map_entry_size (const struct regcache_map_entry *map)
+{
+ int size = 0;
+ for (int i = 0; map[i].count != 0; i++)
+ size += (map[i].count * map[i].size);
+ return size;
+}
+
/* Special value for the 'regno' field in the struct above. */
enum