Re: [PATCH v5 1/5] gdb/aarch64: Add POR_EL0 register support for FEAT_S1POE

[Thiago Jung Bauermann <thiago.bauermann@linaro.org>]

Hello Srinath,

<srinath.parvathaneni@arm.com> writes:

> From: Srinath Parvathaneni <srinath.parvathaneni@arm.com>
>
> Add support for the FEAT_S1POE POR_EL0 register on AArch64.
>
> This patch adds POR_EL0 to the AArch64 register set and reads/writes it
> using the NT_ARM_POE ptrace regset.
>
> It also adds POE pseudo-registers por_p0 through por_p15, with each
> pseudo-register mapping to a 4-bit field in POR_EL0, starting from the
> least-significant nibble.
>
> POR_EL0 and the POE pseudo-registers are added to the "por" register
> group, allowing them to be displayed together using the
> "info registers por" command.
>
> With this change, POR_EL0 is accessible through:
>
> * info registers
> * info registers por_el0
> * info registers por
> * p $por_el0
> * p/x $por_el0
> * set $por_el0 = <value>
>
> The individual POE fields can also be accessed and modified through
> $por_p0 to $por_p15.
>
> Example:
> (gdb) info register por_el0
> por_el0        0x7                 [ P0=rwx ]
> (gdb) set $por_el0=0xffffffff77777777
> (gdb) info registers por_el0
> por_el0        0xffffffff77777777 [ P15=??? P14=??? P13=??? P12=???
>                                     P11=??? P10=??? P9=??? P8=???
>                                     P7=rwx P6=rwx P5=rwx P4=rwx
>                                     P3=rwx P2=rwx P1=rwx P0=rwx ]
> (gdb) info registers por
> por_el0        0xffffffff77777777 [ P15=??? P14=??? P13=??? P12=???
>                                     P11=??? P10=??? P9=??? P8=???
>                                     P7=rwx P6=rwx P5=rwx P4=rwx
>                                     P3=rwx P2=rwx P1=rwx P0=rwx ]
> por_p0         0x7                rwx
> por_p1         0x7                rwx
> por_p2         0x7                rwx
> por_p3         0x7                rwx
> por_p4         0x7                rwx
> por_p5         0x7                rwx
> por_p6         0x7                rwx
> por_p7         0x7                rwx
> por_p8         0xf                ???
> por_p9         0xf                ???
> por_p10        0xf                ???
> por_p11        0xf                ???
> por_p12        0xf                ???
> por_p13        0xf                ???
> por_p14        0xf                ???
> por_p15        0xf                ???
>
> Individual POE pseudo-registers can be written independently, with the
> corresponding 4-bit field in POR_EL0 updated accordingly.  For example:
>
> (gdb) set $por_el0=0x7
> (gdb) info registers por_el0
> por_el0        0x0000000000000007 [ P0=rwx ]
> (gdb) set $por_p1=0x7
> (gdb) info registers por_el0
> por_el0        0x0000000000000077 [ P1=rwx P0=rwx ]
> ---
>  gdb/Makefile.in              |   1 +
>  gdb/aarch64-linux-nat.c      |  64 +++++++++
>  gdb/aarch64-tdep.c           | 268 +++++++++++++++++++++++++++++++++++
>  gdb/aarch64-tdep.h           |  12 ++
>  gdb/arch/aarch64.c           |   4 +
>  gdb/arch/aarch64.h           |   8 +-
>  gdb/doc/gdb.texinfo          |  38 +++++
>  gdb/features/Makefile        |   1 +
>  gdb/features/aarch64-poe.c   |  14 ++
>  gdb/features/aarch64-poe.xml |  11 ++
>  gdb/nat/aarch64-poe-linux.h  |  30 ++++
>  11 files changed, 450 insertions(+), 1 deletion(-)
>  create mode 100644 gdb/features/aarch64-poe.c
>  create mode 100644 gdb/features/aarch64-poe.xml
>  create mode 100644 gdb/nat/aarch64-poe-linux.h

Thank you for addressing the review comments and adding the
documentation section.

There are just a few nits below, but with them fixed:

Approved-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>

Except for the gdb.texinfo part, which needs to be approved by Eli.

> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 9d57fffbab2..caee1d33ce2 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -27306,6 +27306,44 @@ p $fpmr
>  $1 = [ F8S1=E5M2 F8S2=E5M2 F8D=E5M2 OSM=Inf OSC=Inf/NaN LSCALE=0 NSCALE=0 LSCALE2=0 ]
>  @end smallexample
>  
> +@subsubsection AArch64 Permission Overlay Extension
> +@cindex Permission Overlay Extension, AArch64
> +@cindex POE, AArch64
> +
> +When @value{GDBN} is debugging the AArch64 architecture and the Permission
> +Overlay Extension (@acronym{POE}) is available, then @value{GDBN} will make the
> +@code{por_el0} register available.
> +
> +To aid debugging, @value{GDBN} interprets the permissions of each feild in

Typo: s/feild/field/

> +@code{por_el0} when displayed using the @code{info registers} command.
> +
> +@smallexample
> +(gdb) info registers por_el0
> +por_el0 0x0000000000000007 [ P0=rwx ]
> +@end smallexample
> +
> +@value{GDBN} also provides pseudo-registers named @code{por_p0} through
> +@code{por_p15} for accessing the individual permission fields in
> +@code{por_el0}. These pseudo-registers can be read and written independently.
> +Writing a pseudo-register updates the corresponding 4-bit field in
> +@code{por_el0}.
> +
> +@smallexample
> +(gdb) set $por_el0 = 0x7
> +(gdb) set $por_p1 = 0x7
> +(gdb) info registers por_el0
> +por_el0 0x0000000000000077 [ P1=rwx P0=rwx ]
> +(gdb) info registers por_p1
> +por_p1		0x7		rwx
> +@end smallexample
> +
> +The @code{por} register group can be used to display @code{por_el0} and all
> +the POE pseudo-registers.
> +
> +@smallexample
> +(gdb) info registers por
> +@end smallexample
> +
>  For more information about @acronym{FPMR}, please refer to the official
>  @uref{https://developer.arm.com/documentation/ddi0601/latest,ignored,
>  architecture registers documentation}.

This paragraph should be in the FPMR subsubsection, not the POE one.

> diff --git a/gdb/nat/aarch64-poe-linux.h b/gdb/nat/aarch64-poe-linux.h
> new file mode 100644
> index 00000000000..74678c7771d
> --- /dev/null
> +++ b/gdb/nat/aarch64-poe-linux.h
> @@ -0,0 +1,30 @@
> +/* Common native Linux definitions for AArch64 Permission Overlay Extension.
> +
> +   Copyright (C) 2026 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef GDB_NAT_AARCH64_POE_LINUX_H
> +#define GDB_NAT_AARCH64_POE_LINUX_H
> +
> +#include <asm/hwcap.h>
> +
> +/* Feature check for Permission Overlay Extension.  */
> +#ifndef HWCAP2_POE
> +#define HWCAP2_POE (1ULL << 63)
> +#endif /* HWCAP2_POE.  */
> +
> +#endif /* GDB_NAT_AARCH64_POE_LINUX_H.  */

This comment still trips gdb/check-include-guards.py. It should be:

#endif /* GDB_NAT_AARCH64_POE_LINUX_H */

-- 
Thiago
(he/him)