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>]

Srinath Parvathaneni <Srinath.Parvathaneni@arm.com> writes:

>>> *  p/x $por_el0
>>> *  set $por_el0 = <value>
>>>
>>> Example:
>>> (gdb) info register por_el0
>>> por_el0 0x7 [ P15=--- P14=--- P13=--- P12=--- P11=--- P10=--- P9=--- P8=--- P7=---
> P6=--- P5=--- P4=--- P3=--- P2=--- P1=--- P0=rwx ]
>>> (gdb) set $por_el0=0xffffffff77777777
>>> (gdb) info register 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) p $por_el0
>>> $1 = [ 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) p/x $por_el0
>>> $2 = 0xffffffff77777777
>>> (gdb) set $por_el0=0x57
>>> (gdb) info register por_el0
>>> por_el0 0x57 [ P15=--- P14=--- P13=--- P12=--- P11=--- P10=--- P9=--- P8=--- P7=---
> P6=--- P5=--- P4=--- P3=--- P2=--- P1=rw- P0=rwx ]
>>> (gdb) set $por_el0=0xf7f7f7f7f7f7f7f7
>>> (gdb) info register por_el0
>>> por_el0 0xf7f7f7f7f7f7f7f7 [ P15=??? P14=rwx P13=??? P12=rwx P11=??? P10=rwx P9=???
> P8=rwx P7=??? P6=rwx P5=??? P4=rwx P3=??? P2=rwx P1=??? P0=rwx ]
>>> (gdb)
>>
>>Looking at the output above I think it is a bit hard to read.

I think part of the reason for it being hard to read is that the output
is very wide. 

One way to address this is Srinath's suggestion below to not print
zeroed keys. Another would be to improve the output of the flag type to
add line breaks when the terminal's width is reached, as is done when
printing array values.

Another option would be to use a struct rather than a flags type. Then
there would be one type per field, and it would be possible to display
(and set) only one key.

Not sure which of them I prefer. The struct idea has the advantage of
letting the user easily set protection keys individually. OTOH it's
not printed isn a very compact way.

If easily displaying/setting individual keys isn't that important, I
think I slightly prefer not printing zeroed keys as Srinath suggests,
possibly coupled with adding line breaks when the line is too long, to
address the case of having many keys set in the register.

>>Have you considered keeping the original register as it is and having
>>pseudo-registers that print appropriately?
>>
>>If a user wants to see, say, P15, it doesn´t help we print everything
>>else along with it.
>>
>>The other alternative is having a python pretty printer that goes
>>alongside the feature.
>>
>
> Hi Luis,
>
> Thanks for the suggestions.
>
> The reason I chose to decode the register inline is that the raw 64 bit register
> value by itself is not particularly useful. For example, if we kept the original
> formatting, the user would see something like:
>
> (gdb) set $por_el0 = 0xf7f7f7f7f7f7f7f7
> (gdb) info register por_el0
> por_el0         0xf7f7f7f7f7f7f7f7       17868022691004925943
>
> Neither of hexadecimal nor the decimal value tells the user which protection key
> has which permissions without manually decoding each 4-bit nibble.
>
> Although the architecture defines POR_EL0 as a single 64 bit register, it
> doesn't define separate P0-P15 fields. I introduced the P0-P15 labels
> to identify each 4-bit nibble, since each nibble corresponds to one protection
> key's permission encoding.

I agree that if the raw register value isn't useful, then it makes sense
to use a more elaborate type for it.

> We also considered only displaying nibbles that have at least one permission
> enabled, while displaying "???" when the reserved fourth bit of a nibble is set.
> For example:
>
> (gdb) set $por_el0 = 0x57
> (gdb) info register por_el0
> por_el0        0x57  [ P1=rw- P0=rwx ]
>
> (gdb) set $por_el0 = 0xf7
> (gdb) info register por_el0
> por_el0        0xf7  [ P1=??? P0=rwx ]
>
> However, we thought that approach could be misleading because it hides the
> remaining protection keys rather than explicitly showing that they are all
> "---", please let me know if you feel this approach is better?

I think this is better than the output in the commit message.
Especially in the "info register" case where the raw value is also shown
so it's not hard to see that ommitted keys are zeroed. Even with the
print command, using print/x will show the raw value too.

I have a couple more comments on the patch:

>> diff --git a/gdb/arch/aarch64-poe-linux.h b/gdb/arch/aarch64-poe-linux.h
>> new file mode 100644
>> index 00000000000..8623fa43b54
>> --- /dev/null
>> +++ b/gdb/arch/aarch64-poe-linux.h
>> @@ -0,0 +1,29 @@
>> +/* Common Linux target-dependent definitions for AArch64 POE
>> +
>> +   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_ARCH_AARCH64_POE_LINUX_H
>> +#define GDB_ARCH_AARCH64_POE_LINUX_H
>> +
>> +/* Feature check for Permission Overlay Extension.  */
>> +#define AARCH64_HWCAP2_POE (1ULL << 63)
>> +
>> +/* Data or instruction abort caused by Protection Key Violation.  */
>> +#define AARCH64_SEGV_PKUERR 4
>> +
>> +#endif /* GDB_ARCH_AARCH64_POE_LINUX_H.  */

check-include-guards.py complains about the comment above:

$ gdb/check-include-guards.py gdb/arch/aarch64-poe-linux.h
gdb/arch/aarch64-poe-linux.h:29: wrong endif line
$ gdb/check-include-guards.py --update gdb/arch/aarch64-poe-linux.h
$ git diff
diff --git a/gdb/arch/aarch64-poe-linux.h b/gdb/arch/aarch64-poe-linux.h
index 8623fa43b549..d6e4a001cbf9 100644
--- a/gdb/arch/aarch64-poe-linux.h
+++ b/gdb/arch/aarch64-poe-linux.h
@@ -26,4 +26,4 @@
 /* Data or instruction abort caused by Protection Key Violation.  */
 #define AARCH64_SEGV_PKUERR 4

-#endif /* GDB_ARCH_AARCH64_POE_LINUX_H.  */
+#endif /* GDB_ARCH_AARCH64_POE_LINUX_H */

Also, nothing in this patch uses this file. It should be moved to patch 2.

-- 
Thiago
(he/him)