[PPC] Handle qOffsets packet

[luporl]

Summary

On PowerPC, this is needed in order for the debugger to find out
the memory offset where the kernel image was loaded on the remote
target.

This fixes symbol resolution when remote debugging a PowerPC kernel.

[cem]

It’s kind of ugly, but because it only impacts ppc I don’t think it’s risky for other arch. Mechanically the change looks fine; I didn’t verify the qOffsets response packet syntax but if it works, it works. Thanks!

[jhibbits]

sys/gdb/gdb_main.c

143 ↗

(On Diff #65504)

Don't you really want to align to the base of the load segment? Shouldn't you be subtracting KERNBASE instead? I don't know what's at play here, so I may be completely wrong.

[luporl]

sys/gdb/gdb_main.c

143 ↗

(On Diff #65504)

I guess I've wrongly assumed that powerpc/powerpcspe .text section also started at KERNBASE (0x100100), as powerpc64 does, and just subtracting 0x100 would give the correct segment offset. But SIZEOF_HEADERS is added to KERNBASE before their .text section.

This code must return the offset of the first/text segment, as loaded in memory, after being relocated (see https://sourceware.org/gdb/onlinedocs/gdb/General-Query-Packets.html#General-Query-Packets). GDB then relocates the segment and all its sections.

Now, I'm not sure how to do it for powerpc/powerpcspe. Would truncating __startkernel to 64K (segment alignment) do it?
In any case, powerpc/powerpcspe kernel images use more than one LOAD segment, which would require at least the "DataSeg" part of the response to also be implemented, in order to properly relocate data symbols too.
But this would probably need the introduction of a new symbol, to mark the start of .data.

Another option is to change the #ifdef to powerpc64 for now, and then handle 32-bit properly later.

What do you think?

[luporl]

-Fix qOffsets for 32-bit PowerPC

[luporl]

sys/gdb/gdb_main.c

143 ↗

(On Diff #65504)

As discussed with @jhibbits, the best option here, to handle PPC and PPC64 correctly, seems to be the following:
align __startkernel value to 64KB, to get the segment offset.

Obtaining the data segment offset, used on 32-bit PPC only, needed to inform GDB about the relocation of modifiable data symbols, is left as a TODO for later.

[cem]

There is some precedent for putting MD gdb code paths in, e.g., sys/powerpc/powerpc/gdb_machdep.c. The naming convention appears to be something like, gdb_cpu_do_offsets(). If you want to put the current gdb_do_offses code there, that would be cool. I would be fine leaving #ifdef __powerpc__ around the qOffsets handler and not implementing it everywhere, given it doesn't seem to be needed on other platforms. It is also fine with me to gdb_do_offset here, until/unless a second arch shows up needing to provide another implementation.

[luporl]

Move implementation of qOffsets to gdb_machdep.c

[luporl]

In D22767#498741, @cem wrote:

There is some precedent for putting MD gdb code paths in, e.g., sys/powerpc/powerpc/gdb_machdep.c. The naming convention appears to be something like, gdb_cpu_do_offsets(). If you want to put the current gdb_do_offses code there, that would be cool. I would be fine leaving #ifdef __powerpc__ around the qOffsets handler and not implementing it everywhere, given it doesn't seem to be needed on other platforms. It is also fine with me to gdb_do_offset here, until/unless a second arch shows up needing to provide another implementation.

Right, thanks for the feedback. Moving gdb_do_offsets() to powerpc/gdb_machdep.c really seems a better option.
Now there is only one #ifdef __powerpc__ in gdb_main.c, that can be removed if/when other archs need to implement qOffsets handling.