Re: [PATCH v3 16/16] [gdb/docs] sme: Document SME registers and features

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

On 7/1/23 09:58, Eli Zaretskii wrote:
>> Date: Fri, 30 Jun 2023 14:46:16 +0100
>> From: Luis Machado via Gdb-patches <gdb-patches@sourceware.org>
>>
>> Updates since v2:
>>
>> - More adjustments based on reviews.
>> - Fixed incorrect number of tile pseudo-registers.
>> - Fixed naming of tile slice pseudo-registers.
>> - More detail about SME and how gdb implements it.
>> - Attempted to clarify the text a bit more.
>>
>> Updates since v1:
>>
>> - Made SME text more thorough.
>> - Adjusted text based on upstream reviews.
>> - Fixed documentation errors (missing itemization for SME registers).
>>
>> Provide documentation for the SME feature and other information that
>> should be useful for users that need to debug a SME-capable target.
>>
>> ---
>>  gdb/NEWS            |  11 ++
>>  gdb/doc/gdb.texinfo | 249 ++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 260 insertions(+)
> 
> Thanks.
> 
>> diff --git a/gdb/NEWS b/gdb/NEWS
>> index fd42864c692..720008a5d97 100644
>> --- a/gdb/NEWS
>> +++ b/gdb/NEWS
>> @@ -3,6 +3,17 @@
>>  
>>  *** Changes since GDB 13
>>  
>> +* GDB now supports the AArch64 Scalable Matrix Extension (SME), which includes
>> +  a new matrix register named ZA, a new thread register TPIDR2 and a new vector
>> +  length register SVG (streaming vector granule).  GDB also supports tracking
>> +  ZA state across signal frames.
>> +
>> +  Some features are still under development or are dependent on ABI specs that
>> +  are still in alpha stage.  For example, manual function calls with ZA state
>> +  don't have any special handling, and tracking of SVG changes based on
>> +  DWARF information is still not implemented, but there are plans to do so in
>> +  the future.
>> +
>>  * The AArch64 'org.gnu.gdb.aarch64.pauth' Pointer Authentication feature string
>>    has been deprecated in favor of the 'org.gnu.gdb.aarch64.pauth_v2' feature
>>    string.
> 
> This part is OK.
> 
>> +@item
>> +@var{vl}: The vector length, in bytes.  It defines the size of each @code{z}
>> +register.                                                                ^^^
> 
> That should be "Z", in upper case.
> 
>> +(@var{svl} for short).  @xref{vl} @xref{svl}
>                                     ^          ^
> Periods missing there.  Also, please leave two spaces between
> sentences.
> 
>> +The minimum size of the @code{ZA} register is 16 x 16 (256) bytes, and the
>> +maximum size is 256 x 256 (65536) bytes.  In streaming mode, with bit @sc{sm}
>> +set, the size of the @code{ZA} register is the size of all the SVE @code{z}
>> +registers combined.                                                     ^^^
> 
> That should be "Z", upper-case.
> 
>> +There is a total of 31 @code{ZA} tile pseudo-registers.  They are
>> +@code{za0b}, @code{za0h} through @code{za1h}, @code{zas0} through @code{zas3},
>> +@code{zad0} through @code{zad7} and @code{zaq0} through @code{zaq15}.
> 
> These pseudo-registers should also be named in upper-case, right?
> 
>> +The tile slice pseudo-registers have the following naming pattern:
>> +@code{za<@var{tile number}><@var{direction}><@var{qualifier}>
>> +<@var{slice number}>}.
> 
> Likewise here.
> 
>> +One such example is calling functions in the program being debugged by
>> +@value{GDBN}.  Such calls are not @acronym{SME}-aware and thus don't take into
>> +account the @code{SVCR} pseudo-register bits nor the @code{ZA} register
>> +contents.  @xref{Calling}
>                             ^
> Period missing there.
> 
>> +The @url{https://github.com/ARM-software/abi-aa/blob/main/aapcs64/aapcs64.rst#the-za-lazy-saving-scheme,
>> +lazy saving scheme} involving the @code{tpidr2} register is not yet supported
>> +by @value{GDBN}, though the @code{tpidr2} register is known and supported
>> +by @value{GDBN}.
> 
> The "tpidr2" register name should be in upper-case.
> 
>> +Lastly, an important limitation for @code{gdbserver} is its inability to
>> +communicate @var{svl} changes to @value{GDBN}.  This means @code{gdbserver},
>> +even though it is capable of adjusting its internal caches to reflect a change
> 
> Please use @command{gdbserver} instead of @code.
> 

Is this the convention? I see 123 matches for @code{gdbserver}, but only 32 for
@command{gdbserver}.

Otherwise, fixed all of the issues for the next iteration.

>> +The @samp{org.gnu.gdb.aarch64.sme} feature is optional.  If present,
>> +it should contain registers @code{ZA}, @code{SVG} and @code{SVCR}.
>> +@xref{AArch64 SME}
>                      ^
> Period missing there.
> 
>> +@item
>> +@code{ZA} is a register represented by a vector of @var{svl}x@var{svl}
>> +bytes.  @xref{svl}
>                      ^
> And there.
> 
>> +Bit 1 (@sc{ZA}) shows whether the @code{ZA} register state is active (in use) or
>> +not.
>> +@xref{aarch64 sme svcr}
>                           ^                          
> And there.
> 
> Reviewed-By: Eli Zaretskii <eliz@gnu.org>