Page MenuHomeFreeBSD

[PowerPC] More relocation fixes
ClosedPublic

Authored by bdragon on Jun 11 2020, 12:18 AM.

Details

Summary

It turns out relocating the symbol table itself can cause issues, like fbt crashing because it applies the offsets to the kernel twice.
This had been previously brought up in rS333447 when the stoffs hack was added.

Instead, teach ddb about the relocation offset proper.

  • Remove the rest of the stoffs hack.
  • Remove my half-baked displace_symbol_table function.
  • Extend ddb initialization to cope with having a relocation offset on the kernel symbol table.
  • Fix my kernel-as-initrd hack to work with booke64 by using a temporary mapping to access the data.
  • Fix another instance of __powerpc__ that is actually RELOCATABLE_KERNEL.
Test Plan

ksyms cross platform
fbt cross platform
verbose boot of kernel across the board to ensure symbol table lookups do not regress anywhere

Diff Detail

Repository
rS 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/ddb/db_main.c
76 ↗(On Diff #72960)

Staticly allocating this is gross, but a) memory management isn't up when we have to do this, and b) there is only one kernel, so there will never be more than one of these.

189 ↗(On Diff #72960)

This is a behavioral change that I need to verify.

I *believe* it is correct to return the adjusted address here instead of the raw symbol table address, but I would like another opinion on this.

227 ↗(On Diff #72960)

again, gross.

sys/powerpc/powerpc/machdep.c
559 ↗(On Diff #72960)

Book-E's kernel isn't executed from the DMAP, so would this work? And 32-bit Book-E doesn't use a DMAP.

sys/powerpc/powerpc/machdep.c
559 ↗(On Diff #72960)

We're talking about the initrd here, which comes to us as a reserved physical region. Also, on 64 bit, the DMAP isn't up yet so we crash if we try and access it.

sys/powerpc/powerpc/machdep.c
559 ↗(On Diff #72960)

Still doesn't answer the 32-bit question :) 32-bit Book-E doesn't have a DMAP, so this will KASSERT(). It might be fine wrapping this whole codepath in a #ifdef _powerpc64__ if it's not really fixable.

sys/powerpc/powerpc/machdep.c
529 ↗(On Diff #72960)

@jhibbits Did you miss this part here where this code is DMAP only?

559 ↗(On Diff #72960)

I just noticed that you were probably missing the bit where this code only runs when the DMAP is active. See comment above.

One last thing: did you try a debug session between the initrd syms setup and pmap_bootstrap() to make sure it doesn't panic due to being unable to access the DMAP?

sys/powerpc/powerpc/machdep.c
529 ↗(On Diff #72960)

I did indeed.

This revision is now accepted and ready to land.Jun 15 2020, 2:17 AM
This revision was automatically updated to reflect the committed changes.