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

[Srinath Parvathaneni <Srinath.Parvathaneni@arm.com>]

[-- Attachment #1: Type: text/plain, Size: 4823 bytes --]

Thanks Luis and Thiago,
I have addressed the comments and pushed the patch series to the upstream binutils-gdb repository.
Regards,
Srinath
________________________________
From: Luis <luis.machado.foss@gmail.com>
Sent: 03 October 2026 11:15
To: Srinath Parvathaneni <Srinath.Parvathaneni@arm.com>; Thiago Jung Bauermann <thiago.bauermann@linaro.org>; gdb-patches@sourceware.org <gdb-patches@sourceware.org>; Eli Zaretskii <eliz@gnu.org>
Cc: Ezra Sitorus <Ezra.Sitorus@arm.com>; Matthieu Longo <Matthieu.Longo@arm.com>; peter.maydell@linaro.org <peter.maydell@linaro.org>
Subject: Re: [PATCH v5 1/5] gdb/aarch64: Add POR_EL0 register support for FEAT_S1POE

On 22/09/2026 18:31, Srinath Parvathaneni wrote:
> Ping !!
> ------------------------------------------------------------------------
> *From:* Srinath Parvathaneni <Srinath.Parvathaneni@arm.com>
> *Sent:* 18 September 2026 17:47
> *To:* Thiago Jung Bauermann <thiago.bauermann@linaro.org>; gdb-
> patches@sourceware.org <gdb-patches@sourceware.org>; Eli Zaretskii
> <eliz@gnu.org>
> *Cc:* simark@simark.ca <simark@simark.ca>; luis.machado.foss@gmail.com
> <luis.machado.foss@gmail.com>; guinevere@redhat.com
> <guinevere@redhat.com>; Ezra Sitorus <Ezra.Sitorus@arm.com>; Matthieu
> Longo <Matthieu.Longo@arm.com>; peter.maydell@linaro.org
> <peter.maydell@linaro.org>
> *Subject:* Re: [PATCH v5 1/5] gdb/aarch64: Add POR_EL0 register support
> for FEAT_S1POE
> Hi,
>  >_______________________________________
>  >From: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
>  >Sent: 18 September 2026 05:45
>  >To: Srinath Parvathaneni <Srinath.Parvathaneni@arm.com>
>  >Cc: gdb-patches@sourceware.org <gdb-patches@sourceware.org>;
> simark@simark.ca <simark@simark.ca>; luis.machado.foss@gmail.com
> <luis.machado.foss@gmail.com>; guinevere@redhat.com
> <guinevere@redhat.com>; Ezra Sitorus <Ezra.Sitorus@arm.com>; Matthieu
> Longo <Matthieu.Longo@arm.com>; peter.maydell@linaro.org
> <peter.maydell@linaro.org>
>  >Subject: Re: [PATCH v5 1/5] gdb/aarch64: Add POR_EL0 register support
> for FEAT_S1POE
>  >
>  >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.
> ...
>  >>  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.
>
> Thanks Thiago, for the reviews and approval.
>
> Hi Eli, could you please review the documentation for this patch series,
> thanks.
>
> Regards,
> Srinath

Here's some feedback on the docs:

1. Typo: "feild" should be "field".

2. The patch hunk ends with the context line "For more information about
@acronym{FPMR}...". That sentence belongs to the FPMR section. Your new
@subsubsection sits above it, so the FPMR reference would now land at
the end of the POE section. Check that the new section goes after that
paragraph, not before it.

3. Wording of the first paragraph:
    - "debugging the AArch64 architecture" reads oddly. "debugging an
AArch64 target" is more usual.
    - Drop the "then" in "is available, then @value{GDBN} will".
    - "when displayed using" is a dangling modifier. Try "interprets the
permissions of each field in @code{por_el0} when displaying it with the
@code{info registers} command."

4. Examples:
    - Column alignment is inconsistent. por_el0 0x00...07 [ P0=rwx ]
uses single spaces, while por_p1        0x7        rwx is padded like
real info registers output. Use the same style in both.
    - The last @smallexample shows info registers por with no output.
Either show the output, or fold the command into the prose and drop the
block.

5. Markup:
    - Use @samp{por} or @code{por} when you name the group, so it's
clear it's a literal group name.
    - You could also write @code{set $por_p1 = 0x7} inline in the prose,
but the examples are fine as they are.

With those fixed, this is OK.

Approved-By: Luis Machado <luis.machado.foss@gmail.com>

[-- Attachment #2: Type: text/html, Size: 7537 bytes --]