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.
>
> With this change, POR_EL0 is available through:
> * info registers
> * info registers por_el0
> * p $por_el0
> * p/x $por_el0
> * set $por_el0 = <value>
One question, more about general policy on the AArch64 GDB port:
Should the register name have the _el0 suffix? It's more than half of
the register name (though tab completion helps).
We don't currently have that suffix in the name of any register. And in
the case of Guarded Control Stack, I didn't add it. For native
debugging, it will always be the register at EL0. For remote debugging,
I assumed the target would send the register corresponding to the
current exception level in the inferior.
Can there be a situation where it would be possible to see both the EL0
and EL1 (for example) registers at the same time? Or a situation where
the inferior is at EL1 but one wants to see the register at EL0, or
vice-versa?
> ---
> gdb/Makefile.in | 2 ++
> gdb/aarch64-linux-nat.c | 66 ++++++++++++++++++++++++++++++++++++
> gdb/aarch64-linux-tdep.c | 1 +
> gdb/aarch64-tdep.c | 18 ++++++++++
> gdb/aarch64-tdep.h | 11 +++++-
> gdb/arch/aarch64-poe-linux.h | 29 ++++++++++++++++
> gdb/arch/aarch64.c | 4 +++
> gdb/arch/aarch64.h | 7 +++-
> gdb/features/Makefile | 1 +
> gdb/features/aarch64-poe.c | 14 ++++++++
> gdb/features/aarch64-poe.xml | 12 +++++++
> gdb/nat/aarch64-poe-linux.h | 28 +++++++++++++++
> include/elf/common.h | 2 ++
> 13 files changed, 193 insertions(+), 2 deletions(-)
> create mode 100644 gdb/arch/aarch64-poe-linux.h
> 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
>
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index 57f384170ab..3dc9cd07dcb 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -1292,6 +1292,7 @@ HFILES_NO_SRCDIR = \
> arch/aarch32.h \
> arch/aarch64-fpmr-linux.h \
> arch/aarch64-gcs-linux.h \
> + arch/aarch64-poe-linux.h \
> arch/aarch64.h \
> arch/aarch64-insn.h \
> arch/aarch64-mte.h \
> @@ -1540,6 +1541,7 @@ HFILES_NO_SRCDIR = \
> namespace.h \
> nat/aarch64-fpmr-linux.h \
> nat/aarch64-gcs-linux.h \
> + nat/aarch64-poe-linux.h \
> nat/aarch64-hw-point.h \
> nat/aarch64-linux.h \
> nat/aarch64-linux-hw-point.h \
> diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
> index 52ace4aab41..54265920d69 100644
> --- a/gdb/aarch64-linux-nat.c
> +++ b/gdb/aarch64-linux-nat.c
> @@ -34,6 +34,7 @@
> #include "arch/arm.h"
> #include "nat/aarch64-fpmr-linux.h"
> #include "nat/aarch64-gcs-linux.h"
> +#include "nat/aarch64-poe-linux.h"
> #include "nat/aarch64-linux.h"
> #include "nat/aarch64-linux-hw-point.h"
> #include "nat/aarch64-mte-linux-ptrace.h"
> @@ -569,6 +570,54 @@ fetch_gcsregs_from_thread (regcache *regcache)
> &user_gcs.features_locked);
> }
>
> +/* Fill GDB's register array with the POE register value from the current
> + thread. */
> +
> +static void
> +fetch_poeregs_from_thread (regcache *regcache)
> +{
> + aarch64_gdbarch_tdep *tdep
> + = gdbarch_tdep<aarch64_gdbarch_tdep> (regcache->arch ());
> +
> + gdb_assert (tdep->has_poe ());
> +
> + uint64_t user_poe;
> + iovec iovec;
> +
> + iovec.iov_base = &user_poe;
> + iovec.iov_len = sizeof (user_poe);
> +
> + int tid = get_ptrace_pid (regcache->ptid ());
> + if (ptrace (PTRACE_GETREGSET, tid, NT_ARM_POE, &iovec) != 0)
> + perror_with_name (_("Unable to fetch POE register"));
> +
> + regcache->raw_supply (tdep->poe_regnum, &user_poe);
> +}
> +
> +/* Store the NT_ARM_POE register contents from GDB's REGCACHE to the
> + thread associated with REGCACHE. */
> +
> +static void
> +store_poeregs_to_thread (struct regcache *regcache)
> +{
> + aarch64_gdbarch_tdep *tdep
> + = gdbarch_tdep<aarch64_gdbarch_tdep> (regcache->arch ());
> +
> + gdb_assert (tdep->has_poe ());
> +
> + int tid = regcache->ptid ().lwp ();
> +
> + iovec iovec;
> + uint64_t user_poe;
> + iovec.iov_base = &user_poe;
> + iovec.iov_len = sizeof (user_poe);
> +
> + regcache->raw_collect (tdep->poe_regnum, &user_poe);
> +
> + if (ptrace (PTRACE_SETREGSET, tid, NT_ARM_POE, &iovec) != 0)
> + perror_with_name (_("Unable to store POE register"));
> +}
> +
> /* Store to the current thread the valid GCS register set in the GDB's
> register array. */
These functions are fine, but they were added between
fetch_gcsregs_from_thread and store_gcsregs_to_thread. For better
organization of the file, they should be either before or after the GCS
ones.
> @@ -683,6 +732,9 @@ aarch64_fetch_registers (struct regcache *regcache, int regno)
> if (tdep->has_gcs_linux ())
> fetch_gcsregs_from_thread (regcache);
>
> + if (tdep->has_poe ())
> + fetch_poeregs_from_thread (regcache);
> +
> if (tdep->has_fpmr ())
> fetch_fpmr_from_thread (regcache);
> }
> @@ -722,6 +774,10 @@ aarch64_fetch_registers (struct regcache *regcache, int regno)
> && (regno == tdep->gcs_reg_base || regno == tdep->gcs_linux_reg_base
> || regno == tdep->gcs_linux_reg_base + 1))
> fetch_gcsregs_from_thread (regcache);
> + /* POE register? */
> + else if (tdep->has_poe () && (regno == tdep->poe_regnum))
> + fetch_poeregs_from_thread (regcache);
> +
Please remove the extra blank line here.
> /* FPMR? */
> else if (tdep->has_fpmr () && (regno == tdep->fpmr_regnum))
> fetch_fpmr_from_thread (regcache);
> @@ -802,6 +858,9 @@ aarch64_store_registers (struct regcache *regcache, int regno)
>
> if (tdep->has_fpmr ())
> store_fpmr_to_thread (regcache);
> +
> + if (tdep->has_poe ())
> + store_poeregs_to_thread (regcache);
> }
> /* General purpose register? */
> else if (regno < AARCH64_V0_REGNUM)
> @@ -837,6 +896,10 @@ aarch64_store_registers (struct regcache *regcache, int regno)
> else if (tdep->has_fpmr () && regno == tdep->fpmr_regnum)
> store_fpmr_to_thread (regcache);
>
Please remove the extra blank line here. The else ifs should be joined.
> + /* POE register? */
> + else if (tdep->has_poe () && regno == tdep->poe_regnum)
> + store_poeregs_to_thread (regcache);
> +
> /* PAuth registers are read-only. */
> }
>
> @@ -1024,6 +1087,9 @@ aarch64_linux_nat_target::read_description ()
> /* Check for FPMR. */
> features.fpmr = hwcap2 & HWCAP2_FPMR;
>
> + /* Check for POE support. */
> + features.poe = hwcap2 & HWCAP2_POE;
> +
> return aarch64_read_description (features);
> }
>
> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
> index f11eccc1bc1..4a6bc3158c8 100644
> --- a/gdb/aarch64-linux-tdep.c
> +++ b/gdb/aarch64-linux-tdep.c
> @@ -53,6 +53,7 @@
>
> #include "arch/aarch64-fpmr-linux.h"
> #include "arch/aarch64-gcs-linux.h"
> +#include "arch/aarch64-poe-linux.h"
> #include "arch/aarch64-mte.h"
> #include "arch/aarch64-mte-linux.h"
> #include "arch/aarch64-pauth-linux.h"
This should be moved to the patch that makes use of the new header.
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index 886e7ce1e7b..f1e1a9c7a19 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -164,6 +164,11 @@ static const char *const aarch64_gcs_register_names[] = {
> "gcspr"
> };
>
> +static const char *const aarch64_poe_register_names[] = {
> + /* Permission Overlay Extension Register. */
> + "por_el0"
> +};
> +
> static const char *const aarch64_gcs_linux_register_names[] = {
> /* Field in struct user_gcs. */
> "gcs_features_enabled",
> @@ -4556,6 +4561,19 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> fpmr_regnum, "fpmr");
> }
>
> + int poe_regnum = -1;
> + const struct tdesc_feature *feature_poe
> + = tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.poe");
> + if (feature_poe != nullptr)
> + {
> + poe_regnum = num_regs;
> + for (i = 0; i < ARRAY_SIZE (aarch64_poe_register_names); i++)
> + valid_p &= tdesc_numbered_register (feature_poe, tdesc_data.get (),
> + poe_regnum + i,
> + aarch64_poe_register_names[i]);
> + num_regs++;
> + }
> +
> int first_sme_regnum = -1;
> int first_sme2_regnum = -1;
> int first_sme_pseudo_regnum = -1;
> diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
> index dff05a08f84..8029725b145 100644
> --- a/gdb/aarch64-tdep.h
> +++ b/gdb/aarch64-tdep.h
> @@ -208,6 +208,16 @@ struct aarch64_gdbarch_tdep : gdbarch_tdep_base
> return gcs_linux_reg_base != -1;
> }
>
> + /* POE register. This is -1 if no POE feature is available. */
> + int poe_regnum = -1;
> +
> + /* Returns true if the target supports the Linux POE feature. */
Remove the "Linux" word, since the feature isn't specific to it, and
also because this header file isn't OS-specific.
> + bool
> + has_poe () const
> + {
> + return poe_regnum != -1;
> + }
> +
> /* First FPMR register. This is -1 if FPMR is not supported. */
> int fpmr_regnum = -1;
>
> @@ -224,7 +234,6 @@ aarch64_features_from_target_desc (const struct target_desc *tdesc);
>
> extern int aarch64_process_record (struct gdbarch *gdbarch,
> struct regcache *regcache, CORE_ADDR addr);
> -
> displaced_step_copy_insn_closure_up
> aarch64_displaced_step_copy_insn (struct gdbarch *gdbarch,
> CORE_ADDR from, CORE_ADDR to,
This change is unrelated to this patch.
> diff --git a/gdb/arch/aarch64.c b/gdb/arch/aarch64.c
> index 2401a325b7b..a567938d06d 100644
> --- a/gdb/arch/aarch64.c
> +++ b/gdb/arch/aarch64.c
> @@ -28,6 +28,7 @@
> #include "../features/aarch64-sme2.c"
> #include "../features/aarch64-tls.c"
> #include "../features/aarch64-gcs.c"
> +#include "../features/aarch64-poe.c"
> #include "../features/aarch64-gcs-linux.c"
>
> /* See arch/aarch64.h. */
> @@ -77,6 +78,9 @@ aarch64_create_target_description (const aarch64_features &features)
> if (features.fpmr)
> regnum = create_feature_aarch64_fpmr (tdesc.get (), regnum);
>
> + if (features.poe)
> + regnum = create_feature_aarch64_poe (tdesc.get (), regnum);
> +
> return tdesc;
> }
>
> diff --git a/gdb/arch/aarch64.h b/gdb/arch/aarch64.h
> index cf7318cbe74..4e4b50f729e 100644
> --- a/gdb/arch/aarch64.h
> +++ b/gdb/arch/aarch64.h
> @@ -35,6 +35,7 @@ struct aarch64_features
> bool pauth = false;
> bool mte = false;
> bool fpmr = false;
> + bool poe = false;
You should add a comment for this boolean, as mentioned by Matthieu and
Luis. Luis gave a good suggestion for the comment.
> diff --git a/gdb/features/aarch64-poe.xml b/gdb/features/aarch64-poe.xml
> new file mode 100644
> index 00000000000..43ee09e01d6
> --- /dev/null
> +++ b/gdb/features/aarch64-poe.xml
> @@ -0,0 +1,12 @@
> +<?xml version="1.0"?>
> +<!-- Copyright (C) 2026 Free Software Foundation, Inc.
> +
> + Copying and distribution of this file, with or without modification,
> + are permitted in any medium without royalty provided the copyright
> + notice and this notice are preserved. -->
> +
> +<!DOCTYPE feature SYSTEM "gdb-target.dtd">
> +<feature name="org.gnu.gdb.aarch64.poe">
> +
Please remove the extra blank line here.
> + <reg name="por_el0" bitsize="64" type="uint64" group="system"/>
> +</feature>
> diff --git a/gdb/nat/aarch64-poe-linux.h b/gdb/nat/aarch64-poe-linux.h
> new file mode 100644
> index 00000000000..3d858d3ff92
> --- /dev/null
> +++ b/gdb/nat/aarch64-poe-linux.h
> @@ -0,0 +1,28 @@
> +/* 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
> +
This file should include <asm/hwcap.h> so that the system definition is
used if available.
> +/* Feature check for Permission Overlay Extension. */
> +#ifndef HWCAP2_POE
> +#define HWCAP2_POE (1ULL << 63)
> +#endif /* HWCAP2_POE */
> +
> +#endif /* GDB_NAT_AARCH64_POE_LINUX_H */
> diff --git a/include/elf/common.h b/include/elf/common.h
> index 1ae68221a89..f811b8daeb5 100644
> --- a/include/elf/common.h
> +++ b/include/elf/common.h
> @@ -760,6 +760,8 @@
> /* Note: name must be "LINUX". */
> #define NT_ARM_FPMR 0x40e /* AArch64 FPMR. */
> /* Note: name must be "LINUX". */
> +#define NT_ARM_POE 0x40f /* AArch64 POE register. */
> + /* Note: name must be "LINUX". */
> #define NT_ARM_GCS 0x410 /* AArch64 Guarded Control Stack
> registers. */
> /* Note name must be "LINUX". */
This file is under binutils' "jurisdiction", so you should move this
change to the bfd/readelf patch. And consequently also move that patch
to become the first of the series, since this patch needs the definition
above.
Finally, the bfd/readelf patch should be cc'd to the binutils mailing
list. Or alternatively sent there separately from this series.
--
Thiago
(he/him)