From: David Spickett <David.Spickett@arm.com>
To: Luis Machado <Luis.Machado@arm.com>,
John Baldwin <jhb@FreeBSD.org>, Pedro Alves <pedro@palves.net>,
"gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Subject: Re: [PATCH] [Arm] Remove dead FPA code
Date: Wed, 5 Oct 2022 08:36:14 +0000 [thread overview]
Message-ID: <AM7PR08MB5511AD38382E5C19D547C22E9C5D9@AM7PR08MB5511.eurprd08.prod.outlook.com> (raw)
Message-ID: <20221005083614.egav-x6dg2-QeRF_dwzAx-_YLyjernuhR1bewwGU8HM@z> (raw)
In-Reply-To: <3cb2d818-83e8-4e2d-5e1b-9b555d1a2217@arm.com>
[-- Attachment #1: Type: text/plain, Size: 7058 bytes --]
> David, does LLDB actively maintain register descriptions for 32-bit Arm?
LLDB has 4 mechanisms for describing registers:
* XML register info (lldb-server doesn't generate this itself for 32-bit Arm but lldb will read it if given)
* qRegisterInfo packet, which tells you the offset of a given register index in the G packet amongst other things (this is an lldb specific packet iirc)
* A script like the one you linked to
* A fallback set of register defs hardcoded. This only has registers for AArch64, x86 and x86_64 and only includes general purpose registers.
As for FPA support all we have is the DWARF numbers. Pretty safe to say no support.
________________________________
From: Luis Machado <Luis.Machado@arm.com>
Sent: 05 October 2022 09:26
To: John Baldwin <jhb@FreeBSD.org>; Pedro Alves <pedro@palves.net>; gdb-patches@sourceware.org <gdb-patches@sourceware.org>; David Spickett <David.Spickett@arm.com>
Subject: Re: [PATCH] [Arm] Remove dead FPA code
On 10/4/22 22:36, John Baldwin wrote:
> On 10/4/22 10:43 AM, Luis Machado wrote:
>> On 10/4/22 18:08, John Baldwin wrote:
>>> On 10/4/22 1:43 AM, Luis Machado via Gdb-patches wrote:
>>>> On 10/3/22 20:16, Pedro Alves wrote:
>>>>> On 2022-09-20 1:30 p.m., Luis Machado via Gdb-patches wrote:
>>>>>
>>>>>> diff --git a/gdb/arch/arm.h b/gdb/arch/arm.h
>>>>>> index 36757493406..74a6ba93bc7 100644
>>>>>> --- a/gdb/arch/arm.h
>>>>>> +++ b/gdb/arch/arm.h
>>>>>> @@ -44,11 +44,6 @@ enum gdb_regnum {
>>>>>> ARM_SP_REGNUM = 13, /* Contains address of top of stack */
>>>>>> ARM_LR_REGNUM = 14, /* address to return to from a function call */
>>>>>> ARM_PC_REGNUM = 15, /* Contains program counter */
>>>>>> - /* F0..F7 are the fp registers for the (obsolete) FPA architecture. */
>>>>>
>>>>> Shouldn't we leave behind a comment explaining why there's a hole between 15 and 25?
>>>>
>>>> I pondered about this a bit more, and I think we should close the gap and bring CPSR down to
>>>> 16, its "natural" position. It is what linux uses for user_regs as well, in gdb/arch/arm-linux.h:
>>>>
>>>> /* The index to access CSPR in user_regs defined in GLIBC. */
>>>> #define ARM_CPSR_GREGNUM 16
>>>>
>>>>>
>>>>> IIRC the numbers can't be changed since we need to handle the case when the target
>>>>> doesn't send an xml tdesc, so it'd be good to help future readers understand why
>>>>> there's a hole.
>>>>
>>>> That's correct. Though a 32-bit Arm target that doesn't support XML descriptions these days is not very
>>>> common. I haven't seen one in a while.
>>>>
>>>> I'm willing to declare old 32-bit Arm targets that don't send XML target descriptions back as unsupported.
>>>>
>>>> To that effect, I suppose we should add a note to make it more explicit.
>>>>
>>>> More below.
>>>
>>> FWIW, the GDB stub in FreeBSD's kernel does not use XML target descriptions
>>> for any architectures, but it also only tends to do GPRs and not any floating
>>> point. For 32-bit ARM it does not report any register values higher than
>>> number 15 (PC), so it would not be affected by changing this.
>>
>> Does it care about CPSR and/or XPSR? Could you please give it a try to see if the defaults would suit it just fine?
>
> Hmm, I misread and it does care about CPSR for the current thread. The
> relevant code is here (From https://cgit.freebsd.org/src/tree/sys/arm/arm/gdb_machdep.c):
>
> void *
> gdb_cpu_getreg(int regnum, size_t *regsz)
> {
>
> *regsz = gdb_cpu_regsz(regnum);
>
> if (kdb_thread == curthread) {
> if (regnum < 13)
> return (&kdb_frame->tf_r0 + regnum);
> if (regnum == 13)
> return (&kdb_frame->tf_svc_sp);
> if (regnum == 14)
> return (&kdb_frame->tf_svc_lr);
> if (regnum == 15)
> return (&kdb_frame->tf_pc);
> if (regnum == 25)
> return (&kdb_frame->tf_spsr);
> }
>
> switch (regnum) {
> case 4: return (&kdb_thrctx->pcb_regs.sf_r4);
> case 5: return (&kdb_thrctx->pcb_regs.sf_r5);
> case 6: return (&kdb_thrctx->pcb_regs.sf_r6);
> case 7: return (&kdb_thrctx->pcb_regs.sf_r7);
> case 8: return (&kdb_thrctx->pcb_regs.sf_r8);
> case 9: return (&kdb_thrctx->pcb_regs.sf_r9);
> case 10: return (&kdb_thrctx->pcb_regs.sf_r10);
> case 11: return (&kdb_thrctx->pcb_regs.sf_r11);
> case 12: return (&kdb_thrctx->pcb_regs.sf_r12);
> case 13: stacktest = kdb_thrctx->pcb_regs.sf_sp + 5 * 4;
> return (&stacktest);
> case 15:
> /*
> * On context switch, the PC is not put in the PCB, but
> * we can retrieve it from the stack.
> */
> if (kdb_thrctx->pcb_regs.sf_sp > KERNBASE) {
> kdb_thrctx->pcb_regs.sf_pc = *(register_t *)
> (kdb_thrctx->pcb_regs.sf_sp + 4 * 4);
> return (&kdb_thrctx->pcb_regs.sf_pc);
> }
> }
>
> return (NULL);
> }
>
> The 'kdb_thread == curthread' case is when a thread enters the debugger due
> to a crash or breakpoint, etc. We do return CPSR for that thread, but we do
> not return it for other threads. It looks like we do also know the FPA register
> size so that we return enough "xx" bytes in the 'g' reply to mark the FP
> registers as unavailable so that we can return the value of CPSR in the 'g'
> reply.
>
> From https://cgit.freebsd.org/src/tree/sys/arm/include/gdb_machdep.h:
>
> #define GDB_NREGS 26
> #define GDB_REG_SP 13
> #define GDB_REG_LR 14
> #define GDB_REG_PC 15
>
> static __inline size_t
> gdb_cpu_regsz(int regnum)
> {
> /*
> * GDB expects the FPA registers f0-f7, each 96 bits wide, to be placed
> * in between the PC and CSPR in response to a "g" packet.
> */
> return (regnum >= 16 && regnum <= 23 ? 12 : sizeof(int));
> }
>
> NetBSD's kernel seems to have similar knowledge:
>
> http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/arch/arm/include/db_machdep.h?rev=1.28&content-type=text/x-cvsweb-markup&only_with_tag=MAIN
>
> (The kgdb bits near the bottom)
>
> Linux's kernel also seems to maybe hardcode this knowledge as well:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/kernel/kgdb.c#n21
>
Yeah, that's what I was worried about. Register discoveries without XML are not great, and more recently debugging stubs have been
exposing more system registers. Having to consider FPA (which was *removed* 10 years ago from GCC, but fell in disuse before then) is not
acceptable at this point.
If those debugging stubs want to skip XML, I think it would be reasonable for them to at least update the expected 'g' packet to contain just
the basic registers, with CPSR as 16.
That might need some coordination. I can coordinate this from the Linux Kernel's side, but I never dealt with the BSD kernels.
next prev parent reply other threads:[~2022-10-05 8:36 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-20 12:30 Luis Machado
2022-09-20 12:47 ` Eli Zaretskii
2022-10-02 13:39 ` Enze Li
2022-10-03 8:27 ` Luis Machado
2022-10-03 17:33 ` John Baldwin
2022-10-03 19:16 ` Pedro Alves
2022-10-04 8:43 ` Luis Machado
2022-10-04 17:08 ` John Baldwin
2022-10-04 17:43 ` Luis Machado
2022-10-04 21:36 ` John Baldwin
2022-10-05 8:26 ` Luis Machado
2022-10-05 8:36 ` David Spickett [this message]
2022-10-05 8:36 ` David Spickett
2022-10-05 16:48 ` John Baldwin
2022-10-05 16:57 ` Richard Earnshaw
2022-10-06 13:02 ` Luis Machado
2022-10-10 14:58 ` Pedro Alves
2022-10-13 7:23 ` Luis Machado
2022-10-13 8:29 ` Pedro Alves
2022-10-13 9:40 ` Luis Machado
2022-10-25 13:54 ` Luis Machado
2022-11-14 14:30 ` Simon Marchi
2022-10-10 14:56 ` Pedro Alves
2022-10-13 7:18 ` Luis Machado
2022-10-13 8:44 ` Pedro Alves
2022-10-13 9:15 ` Luis Machado
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=AM7PR08MB5511AD38382E5C19D547C22E9C5D9@AM7PR08MB5511.eurprd08.prod.outlook.com \
--to=david.spickett@arm.com \
--cc=Luis.Machado@arm.com \
--cc=gdb-patches@sourceware.org \
--cc=jhb@FreeBSD.org \
--cc=pedro@palves.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).