Page MenuHomeFreeBSD

Loader: Add load offset to powerpc kernel entry point
ClosedPublic

Authored by jhibbits on Aug 16 2019, 3:40 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Nov 18, 6:39 PM
Unknown Object (File)
Tue, Nov 5, 11:24 AM
Unknown Object (File)
Oct 21 2024, 12:53 PM
Unknown Object (File)
Oct 17 2024, 6:01 PM
Unknown Object (File)
Oct 17 2024, 6:01 PM
Unknown Object (File)
Oct 17 2024, 6:00 PM
Unknown Object (File)
Oct 17 2024, 6:00 PM
Unknown Object (File)
Oct 17 2024, 5:31 PM
Subscribers
None

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 - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

This revision is now accepted and ready to land.Aug 16 2019, 3:58 AM
imp requested changes to this revision.Aug 16 2019, 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.Aug 16 2019, 1:37 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.

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
  }

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

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 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.Sep 5 2019, 9:46 PM

Yep, this is much cleaner than the old code.