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

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

Luis <luis.machado.foss@gmail.com> writes:

> On 25/07/2026 07:20, Thiago Jung Bauermann wrote:
>> Luis <luis.machado.foss@gmail.com> writes:
>> 
>>> On 14/07/2026 21:15, srinath.parvathaneni@arm.com wrote:
>>>> 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.
>>>
>>> Just a general comment, but por_el0 is unfortunate naming for a userspace register. But
>>> alas, it's been done that way in the Linux kernel as far as I can tell.
>> Do you mean the _el0 suffix? We had a discussion about it here:
>> https://inbox.sourceware.org/gdb-patches/87a4sgikk4.fsf@linaro.org/
>> Srinath responded:
>> 
>>> That said, after discussing this with kernel/KVM developers, there are valid
>>> debugging scenarios (e.g. KGDB or guest debugging via KVM/QEMU) where exposing
>>> `POR_EL0`, `POR_EL1`, and `POR_EL2` simultaneously would be useful. This seems
>>> like a broader GDB register naming issue rather than something specific to
>>> FEAT_S1POE.
>> And Marc Zyngier too:
>> 
>>> It would certainly make our life easier if GDB was in general adopting
>>> the architecture nomenclature.
>>>
>>> It is probably fine to have a "shorthand" such as POR for POR_EL0, but
>>> I'd like to make sure that it is possible to unambiguously target the
>>> correct register for the cases where we have to debug a full guest
>>> (which is something people actively do using the QEMU GDB stubs).
>> So as Marc says perhaps we could have an alias por for por_el0, like we
>> have lr for x30, or (in the other direction) x31 for sp?
>> 
>
> Naming registers after their architectural names is fine as long as they're really exposed
> as their architectural selves. Sometimes we don't get exposed purely architectural
> registers via ptrace, so it would be a bit confusing to do that.

Ah, now I got it. Yes, I agree.

> If we have plans to support QEMU bare metal, using the architectural name also makes
> sense.

Agreed.

>>>> diff --git a/gdb/features/aarch64-poe.c b/gdb/features/aarch64-poe.c
>>>> new file mode 100644
>>>> index 00000000000..4bd795e9fe6
>>>> --- /dev/null
>>>> +++ b/gdb/features/aarch64-poe.c
>>>> @@ -0,0 +1,68 @@
>>>> +/* THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi:set ro:
>>>> +  Original: aarch64-poe.xml */
>>>> +
>>>> +#include "gdbsupport/tdesc.h"
>>>> +
>>>> +static int
>>>> +create_feature_aarch64_poe (struct target_desc *result, long regnum)
>>>> +{
>>>> +  struct tdesc_feature *feature;
>>>> +
>>>> +  feature = tdesc_create_feature (result, "org.gnu.gdb.aarch64.poe");
>>>> +  tdesc_type_with_fields *type_with_fields;
>>>> +  type_with_fields = tdesc_create_enum (feature, "por_el0_fmt", 4);
>>>> +  tdesc_add_enum_value (type_with_fields, 0, "---");
>>>> +  tdesc_add_enum_value (type_with_fields, 1, "r--");
>>>> +  tdesc_add_enum_value (type_with_fields, 2, "--x");
>>>> +  tdesc_add_enum_value (type_with_fields, 3, "r-x");
>>>> +  tdesc_add_enum_value (type_with_fields, 4, "-w-");
>>>> +  tdesc_add_enum_value (type_with_fields, 5, "rw-");
>>>> +  tdesc_add_enum_value (type_with_fields, 6, "-wx");
>>>> +  tdesc_add_enum_value (type_with_fields, 7, "rwx");
>>>> +  tdesc_add_enum_value (type_with_fields, 8, "???");
>>>> +  tdesc_add_enum_value (type_with_fields, 9, "???");
>>>> +  tdesc_add_enum_value (type_with_fields, 10, "???");
>>>> +  tdesc_add_enum_value (type_with_fields, 11, "???");
>>>> +  tdesc_add_enum_value (type_with_fields, 12, "???");
>>>> +  tdesc_add_enum_value (type_with_fields, 13, "???");
>>>> +  tdesc_add_enum_value (type_with_fields, 14, "???");
>>>> +  tdesc_add_enum_value (type_with_fields, 15, "???");
>>>
>>> I'm not a fan of this. Have we considered alternatives like pseudo-registers that map
>>> to/from the raw POR value?
>>>
>>> It feels like this is working around a gdb deficiency of not having a proper type, and
>>> the
>>> right way to solve this would be extending gdb in some way.
>>>
>>> With a pseudo-register all of this would be interiorized in gdb, and we would be left
>>> with
>>> only the raw POR value of 64 bits.
>>>
>>>> +
>>>> +  type_with_fields = tdesc_create_flags (feature, "por_el0_flags", 8);
>>>> +  tdesc_type *field_type;
>>>> +  field_type = tdesc_named_type (feature, "por_el0_fmt");
>>>> +  tdesc_add_typed_bitfield (type_with_fields, "P15", 60, 63, field_type);
>>>> +  field_type = tdesc_named_type (feature, "por_el0_fmt");
>>>> +  tdesc_add_typed_bitfield (type_with_fields, "P14", 56, 59, field_type);
>>>> +  field_type = tdesc_named_type (feature, "por_el0_fmt");
>>>> +  tdesc_add_typed_bitfield (type_with_fields, "P13", 52, 55, field_type);
>>>> +  field_type = tdesc_named_type (feature, "por_el0_fmt");
>>>> +  tdesc_add_typed_bitfield (type_with_fields, "P12", 48, 51, field_type);
>>>> +  field_type = tdesc_named_type (feature, "por_el0_fmt");
>>>> +  tdesc_add_typed_bitfield (type_with_fields, "P11", 44, 47, field_type);
>>>> +  field_type = tdesc_named_type (feature, "por_el0_fmt");
>>>> +  tdesc_add_typed_bitfield (type_with_fields, "P10", 40, 43, field_type);
>>>> +  field_type = tdesc_named_type (feature, "por_el0_fmt");
>>>> +  tdesc_add_typed_bitfield (type_with_fields, "P9", 36, 39, field_type);
>>>> +  field_type = tdesc_named_type (feature, "por_el0_fmt");
>>>> +  tdesc_add_typed_bitfield (type_with_fields, "P8", 32, 35, field_type);
>>>> +  field_type = tdesc_named_type (feature, "por_el0_fmt");
>>>> +  tdesc_add_typed_bitfield (type_with_fields, "P7", 28, 31, field_type);
>>>> +  field_type = tdesc_named_type (feature, "por_el0_fmt");
>>>> +  tdesc_add_typed_bitfield (type_with_fields, "P6", 24, 27, field_type);
>>>> +  field_type = tdesc_named_type (feature, "por_el0_fmt");
>>>> +  tdesc_add_typed_bitfield (type_with_fields, "P5", 20, 23, field_type);
>>>> +  field_type = tdesc_named_type (feature, "por_el0_fmt");
>>>> +  tdesc_add_typed_bitfield (type_with_fields, "P4", 16, 19, field_type);
>>>> +  field_type = tdesc_named_type (feature, "por_el0_fmt");
>>>> +  tdesc_add_typed_bitfield (type_with_fields, "P3", 12, 15, field_type);
>>>> +  field_type = tdesc_named_type (feature, "por_el0_fmt");
>>>> +  tdesc_add_typed_bitfield (type_with_fields, "P2", 8, 11, field_type);
>>>> +  field_type = tdesc_named_type (feature, "por_el0_fmt");
>>>> +  tdesc_add_typed_bitfield (type_with_fields, "P1", 4, 7, field_type);
>>>> +  field_type = tdesc_named_type (feature, "por_el0_fmt");
>>>> +  tdesc_add_typed_bitfield (type_with_fields, "P0", 0, 3, field_type);
>>>
>>> Likewise for the above. This is hardcoding the interpretation of the individual bitfields
>>> into the XML.
>>>
>>> Maybe Thiago has a different opinion here.
>> The interpretation is fixed by the architecture, so hardcoding it makes
>> sense IMHO. But I don't feel strongly about this.
>> 
>
> Sorry, I wasn´t clear in my comment. The interpretation of the bits is dictated by the
> architecture, but does it also require us to expose the raw register as a segmented view
> with withP<0-15> entries?
>
> The last part seems more like a visualization aid to me, and that could be handled
> internally by gdb instead of via XML.

Ah, understood. Now (and having read your other email) I see what you
mean with using pseudo-registers. Indeed having pseudo-registers to
access individual protection keys sounds like a good idea if we expcet
users to want to do that.
-- 
Thiago
(he/him)