Created attachment 219527 [details] Hardcoded length of the FPA registers A little intro: I've an x86-64 computer running 13.0-CURRENT r366999 which plays a role of a debugger system running kgdb, and BeagleBone Black running my custom build (review D5986) of 13.0-CURRENT r367528 as a target. I've tried to connect kgdb via a serial link to the target in order to run an external kernel debugger and got: (kgdb) set serial baud 115200 (kgdb) set debug remote 1 (kgdb) target remote /dev/cuaU1 Remote debugging using /dev/cuaU1 Sending packet: $qSupported:multiprocess+;swbreak+;hwbreak+;qRelocInsn+;fork-events+;vfork-events+;exec-events+;vContSupported+;QThreadEvents+;no-resumed+;xmlRegisters=i386#6a...Ack ... Sending packet: $g#67...Ack Packet received: 000000008805a8c084eb1fd113000060ecec1fd1585987c0000804006c7a31c0ecec1fd1000000000000000058ec1fd10100000048ec1fd1147b31c0307b31c0xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx13000060 Truncated register 19 in remote 'g' packet I wasn't able to proceed any further because kgdb wasn't connected to the target.
After several experiments with patched kgdb I discovered that this is what reported by the kernel as a response to the "g" packet: Packet received: 00000000 (r0) 8805a8c0 (r1) 84eb1fd1 (r2) 13000060 (r3) ecec1fd1 (r4) 585987c0 (r5) 00080400 (r6) 6c7a31c0 (r7) ecec1fd1 (r8) 00000000 (r9) 00000000 (r10) 58ec1fd1 (r11) 01000000 (r12) 48ec1fd1 (r13 svc_sp) 147b31c0 (r14 svc_lr) 307b31c0 (r15 pc) xxxxxxxx (r16, f0) xxxxxxxx (r17, f1) xxxxxxxx (r18, f2) xxxxxxxx (r19, f3) xxxxxxxx (r20, f4) xxxxxxxx (r21, f5) xxxxxxxx (r22, f6) xxxxxxxx (r23, f7) xxxxxxxx (r24) 13000060 (r25 spsr) But kgdb expects: Register 0: size=4, offset=0 Register 1: size=4, offset=4 Register 2: size=4, offset=8 Register 3: size=4, offset=12 Register 4: size=4, offset=16 Register 5: size=4, offset=20 Register 6: size=4, offset=24 Register 7: size=4, offset=28 Register 8: size=4, offset=32 Register 9: size=4, offset=36 Register 10: size=4, offset=40 Register 11: size=4, offset=44 Register 12: size=4, offset=48 Register 13: size=4, offset=52 Register 14: size=4, offset=56 Register 15: size=4, offset=60 Register 16: size=12, offset=64 Register 17: size=12, offset=76 Register 18: size=12, offset=88 Register 19: size=12, offset=100 Register 20: size=12, offset=112 Register 21: size=12, offset=124 Register 22: size=12, offset=136 Register 23: size=12, offset=148 Register 24: size=4, offset=160 Register 25: size=4, offset=164 According to the [1] f0-f7 are FPA registers which were implemented in only few devices [2], but GDB still reserves space for them for compatibility. [1] https://developer.arm.com/documentation/dui0056/d/using-the-procedure-call-standard/floating-point-options/the-fpa-architecture [2] https://retrocomputing.stackexchange.com/questions/13400/history-of-arm-linux-and-fpa
It looks like each register can hold a single-precision, double-precision, or extended-precision value which doesn't answer a question why GDB expects 96-bit registers to me. However, I just want to let kgdb work out-of-the-box in future.
Seems reasonable to me. Register 24 is still not handled in the patch, isn't it a problem? The patch also increases the gdb buffer size. I guess it gives a significant throughput improvement? It is probably not too harmful to bump the buffer size, arm kernel configs do not enable GDB.
(In reply to Mark Johnston from comment #3) Hello Mark, kgdb expects 4 bytes for Register 24, so no problem here with the existing implementation of gdb_cpu_regsz(): static __inline size_t gdb_cpu_regsz(int regnum __unused) { return (sizeof(int)); } I didn't test a throughput tbh, but increased GDB_BUFSZ in order to make enough space for these 96-bit registers as I thought. It looks like I haven't even had to touch it in case of the register values transmission.
(In reply to Dmitry Salychev from comment #4) Ahh, I see. In that case I think the code should be added to gdb_cpu_regsz() instead. I don't quite parse the last statement. The GDB_BUFSZ adjustment is unnecessary?
Created attachment 220011 [details] Hardcoded length of the FPA registers gdb_cpu_regsz() modified to report a length of the FPA registers which is expected by GDB (96-bit).
(In reply to Mark Johnston from comment #5) It's a good idea to modify gdb_cpu_regsz()! Btw, you've parsed my statement correctly. GDB_BUFSZ defines a size of the packets in bytes which can be accepted by the gdb stub and not transmitted. I've attached a new patch with only necessary changes.
A commit references this bug: Author: markj Date: Fri Nov 27 16:35:44 UTC 2020 New revision: 368108 URL: https://svnweb.freebsd.org/changeset/base/368108 Log: arm: Correctly report the size of FPA registers to GDB Modern ARM systems do not have an FPA unit but GDB reserves register indices for FPA registers and expects the stub to know their sizes. PR: 251022 Submitted by: Dmitry Salychev <dsl@mcusim.org> MFC after: 2 weeks Changes: head/sys/arm/include/gdb_machdep.h
Thanks Dmitry, I committed the patch with a few cosmetic tweaks.
(In reply to Mark Johnston from comment #9) Thanks Mark! :) It looks like a first patch of mine found its way to FreeBSD. Btw, I'll create a review for https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=251053 as we discussed and open a new PR with another patch which allows to modify stack pointer, link register and r0-r12 in ARM target from kgdb during this weekend.
(In reply to Dmitry Salychev from comment #10) Sounds good. Please ping me and I can help get them reviewed and committed. I am not an ARM expert so we might need to find a better reviewer for anything complicated.
A commit references this bug: Author: markj Date: Fri Dec 11 00:25:35 UTC 2020 New revision: 368535 URL: https://svnweb.freebsd.org/changeset/base/368535 Log: MFC r368108: arm: Correctly report the size of FPA registers to GDB PR: 251022 Changes: _U stable/12/ stable/12/sys/arm/include/gdb_machdep.h