Page MenuHomeFreeBSD

arm64: remove pcb_pc
ClosedPublic

Authored by mhorne on Dec 22 2020, 2:58 PM.

Details

Summary

The program counter field in the PCB is written in exactly one place,
makectx(), upon entry to the debugger. For threads other than
curthread, its value will be empty, or bogus. Rather than writing to
this field in more places, it can be removed in favor of using the value
in the link register.

To make this clearer, pcb->pcb_x[30] is renamed to pcb->pcb_lr, similar
to what already exists in struct trapframe. Also, prefer lr to x30 in
assembly, as it better conveys intention.

This improves PC_REGS() for kdb_thread != curthread. It is required for
a functional gdb stub, fixing the output of info threads, in
particular.

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

sys/arm64/include/pcb.h
41

Why exactly does the layout need to be preserved?

sys/arm64/include/pcb.h
41

kgdb has knowledge of the pcb layout, and will require an update if we change the layout. I'm unsure how easily this can be done while maintaining support for older targets. @jhb probably has an idea.

This revision is now accepted and ready to land.Dec 27 2020, 3:26 PM

For KGDB I explicitly ignore pcb_pc, but I do assume the layout for getting to the pc_sp below using the following structure:

static const struct regcache_map_entry aarch64_fbsd_pcbmap[] =
  {
    { 30, AARCH64_X0_REGNUM, 8 }, /* x0 ... x29 */
    { 1, AARCH64_PC_REGNUM, 8 },
    { 1, REGCACHE_MAP_SKIP, 8 },
    { 1, AARCH64_SP_REGNUM, 8 },
    { 0 }
  };

I could perhaps depend on osreldate which holds __FreeBSD_version to handle the differing layouts by having two versions of the structure.

(The changes look fine to me in general)

In D27720#622405, @jhb wrote:

For KGDB I explicitly ignore pcb_pc, but I do assume the layout for getting to the pc_sp below using the following structure:

static const struct regcache_map_entry aarch64_fbsd_pcbmap[] =
  {
    { 30, AARCH64_X0_REGNUM, 8 }, /* x0 ... x29 */
    { 1, AARCH64_PC_REGNUM, 8 },
    { 1, REGCACHE_MAP_SKIP, 8 },
    { 1, AARCH64_SP_REGNUM, 8 },
    { 0 }
  };

I could perhaps depend on osreldate which holds __FreeBSD_version to handle the differing layouts by having two versions of the structure.

Seems reasonable, and it's nice to know there's an approach if there is ever a need to make breaking changes to struct pcb. I think it's safe to say that it wouldn't be worth the added effort for this change alone.

This revision was automatically updated to reflect the committed changes.