Page MenuHomeFreeBSD

Loader: Add load offset to powerpc kernel entry point
ClosedPublic

Authored by jhibbits on Aug 16 2019, 3:40 AM.

Details

Summary

There is logic in ELF loadimage() to relocate kernels, but currently
only type ET_EXEC. PowerPC kernels are ET_DYN, and can be relocated anywhere.
Add the load offset to kernel entry points on this platform.

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

jhibbits created this revision.Aug 16 2019, 3:40 AM
ian accepted this revision.Aug 16 2019, 3:58 AM
This revision is now accepted and ready to land.Aug 16 2019, 3:58 AM
imp requested changes to this revision.Fri, Aug 16, 1:37 PM
imp added inline comments.
stand/common/load_elf.c
484 ↗(On Diff #60867)

and why not adjust it here?

520 ↗(On Diff #60867)

This should move up to where I tagged it above (though maybe not in that if) since we already have a powerpc ifdef there.

This revision now requires changes to proceed.Fri, Aug 16, 1:37 PM
jhibbits added inline comments.Fri, Aug 16, 2:15 PM
stand/common/load_elf.c
484 ↗(On Diff #60867)

Look above and you see this block is for type ET_EXEC. PowerPC kernels are ET_DYN now. This block looks really weird, and I would love to remove it. Not sure if we can, though, because I don't know if any "supported" older release kernels are being booted with a modern loader. Otherwise, yes, I could muck with the condition above and muck with this as well. Or completely remove this block for PowerPC, and do the work below where I added, only.

ian added inline comments.Fri, Aug 16, 2:56 PM
stand/common/load_elf.c
520 ↗(On Diff #60867)

Okay, that's two of us who thought this needed to move up. We were wrong, but clearly the current code structure is going to fool people.

Maybe it would be better to move this block up so that the overall structure is:

  if (ehdr->e_type == ET_DYN) {
#ifdef powerpc
    // new stuff
#endif
  } else if (ehdr->e_type == ET_EXEC) {
    // existing stuff
  }
jhibbits updated this revision to Diff 61515.Sun, Sep 1, 12:26 AM

Address feedback. I think this is better overall, simplifies things.

jhibbits added inline comments.Sun, Sep 1, 12:45 AM
stand/common/load_elf.c
480 ↗(On Diff #61515)

I don't think this block should really be here, but I'm not sure enough to remove it. Since the current idea is that we typically load in the first 256MB, I don't see a problem enough to fix it.

imp accepted this revision.Thu, Sep 5, 9:46 PM
imp added inline comments.
stand/common/load_elf.c
520 ↗(On Diff #60867)

or just make it an else if here for the new stuff.

but why then is this powerpc specific if other platforms aren't currently doing it? Will there be harm to them?

This revision is now accepted and ready to land.Thu, Sep 5, 9:46 PM
ian accepted this revision.Thu, Sep 5, 9:48 PM

Yep, this is much cleaner than the old code.

This revision was automatically updated to reflect the committed changes.