Re: [PATCH 14/14] target/arm: Support reading m-profile system registers from gdb

[Luis Machado]

Hi,

On 2/20/23 17:37, Peter Maydell wrote:
> On Mon, 20 Feb 2023 at 17:00, Richard Henderson
> <richard.henderson@linaro.org> wrote:
> > On 2/20/23 06:02, Peter Maydell wrote:
> > > > +    g_string_printf(s, "<?xml version=\"1.0\"?>");
> > > > +    g_string_append_printf(s, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">");
> > > > +    g_string_append_printf(s, "<feature name=\"org.gnu.gdb.arm.m-system\">\n");
> > >
> > > Half of these need to be in org.gnu.gdb.arm.secext.
> > > These aren't our own XML features we're making up (if they
> > > were then they would be in org.qemu.something), so we should
> > > follow the existing precedent about what registers go in them.
> >
> > Now that you point it out (and I should have checked myself), we are kinda 
> > making them up.
> >    The only registers within upstream gdb m-system and secext are MSP, PSP, 
> > MSP_NS, MSP_S,
> > PSP_NS, PSP_S.  All the others are our own addition.
>
> I think OpenOCD's implementation includes more than that:
> https://openocd.org/doc-release/doxygen/armv7m_8c_source.html
>
> > Should all the rest be in a third bit of xml?
>
> Luis, do you have the specs for what the existing implementations
> are doing here ?

Support for the extra stack pointers was contributed by ST (Torbjörn and Yvan 
cc-ed), so I'd say ST-Link was the debugging stub the GDB changes were based on 
(to my knowledge). This support includes the org.gnu.gdb.arm.m-system and 
org.gnu.gdb.arm.secext features.

The org.gnu.gdb.arm.m-system feature contains msp and psp 
(gdb/features/arm/arm-m-system.xml) and the org.gnu.gdb.arm.secext feature 
contains the msp_ns, msp_s, psp_ns and psp_s registers 
(gdb/features/arm/arm-secext.xml).

All of the registers should be 32-bit in size. There could be extra registers 
in those two features, but gdb only cares about the 6 registers listed above. 
It is meant for gdb to detect if the additional stack pointer registers are 
available and, if so, track their values.

Adding the stack pointers to other features should be OK, but gdb needs to know 
about that so it can go looking for them. The most convenient way to do it is 
to follow the original features we modeled things from. In this case, the XML 
layout from ST's contributions.
> Ideally gdb should document for every bit of XML it is the
> official owner of (ie in the org.gnu.gdb namespace) what the
> required and optional register values are, including details
> like the register width (which I think the two existing
> implementations that output m-system disagree on).

I went through the OpenOCD code and it seems it has additional registers in the 
m-system category. That should be fine, as long as gdb sees msp and psp.

Of course, all of this would be much better if properly documented in the gdb 
manual, which unfortunately it isn't (in much or any detail). Apologies for 
that.

I'll set some time aside to get the documentation updated / written for these 
features.

In the mean time, feel free to ping me if something needs to be clarified.

> thanks
> -- PMM

IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.