Re: [PATCH v2 4/6] [gdb/aarch64] sme2: Core file support for ZT register set

[Luis Machado <luis.machado@arm.com>]

On 8/27/23 02:21, Thiago Jung Bauermann wrote:
> 
> Luis Machado <luis.machado@arm.com> writes:
> 
>> This patch adds support for ZT register dumps/reads for core files.  The
>> ZT register is available when the SME2 feature is advertised as available
>> by the Linux Kernel.
>>
>> Unlike the enablement for SME1 and the ZA register, support for SME2 is rather
>> simple given the fixed size of the ZT0 register.
>>
>> Validated on the Fast Models.
>> ---
>>  gdb/aarch64-linux-tdep.c | 81 ++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 81 insertions(+)
>>
>> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
>> index 8b0b4d32971..a4d1fb87cbf 100644
>> --- a/gdb/aarch64-linux-tdep.c
>> +++ b/gdb/aarch64-linux-tdep.c
>> @@ -1374,6 +1374,50 @@ aarch64_linux_collect_za_regset (const struct regset *regset,
>>  			    size - SVE_HEADER_SIZE);
>>  }
>>  
>> +/* 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_zt_regset (const struct regset *regset,
>> +				struct regcache *regcache,
>> +				int regnum, const void *buf, size_t size)
>> +{
>> +  /* Read the ZT register note from a core file into the register buffer.  */
>> +
>> +  /* Handle an empty buffer.  */
>> +  if (buf == nullptr)
>> +    return regcache->supply_regset (regset, regnum, nullptr, size);
>> +
>> +  aarch64_gdbarch_tdep *tdep
>> +    = gdbarch_tdep<aarch64_gdbarch_tdep> (regcache->arch ());
> 
> Is it worth adding here a "gdb_assert (size >= AARCH64_SME2_ZT0_SIZE)"?
> 

Yes. That makes sense.

>> +
>> +  /* Supply the ZT0 register contents.  */
>> +  regcache->raw_supply (tdep->sme2_zt0_regnum, buf);
>> +}
>> +
>> +/* 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_zt_regset (const struct regset *regset,
>> +				 const struct regcache *regcache,
>> +				 int regnum, void *buf, size_t size)
>> +{
>> +  /* Read the ZT register contents from the register buffer into the core
>> +     file section.  */
>> +
>> +  gdb_assert (buf != nullptr);
> 
> Also here, is it worth adding a
> "gdb_assert (size >= AARCH64_SME2_ZT0_SIZE)"?
> 

Fixed as well.

>> +
>> +  aarch64_gdbarch_tdep *tdep
>> +    = gdbarch_tdep<aarch64_gdbarch_tdep> (regcache->arch ());
>> +
>> +  /* Dump the register cache contents for the ZT register to the buffer.  */
>> +  regcache->collect_regset (regset, tdep->sme2_zt0_regnum, buf,
>> +			    AARCH64_SME2_ZT0_SIZE);
>> +}
>> +
> 
> Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
>