Loader: Add load offset to powerpc kernel entry point
ClosedPublic
Actions

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

Details

Reviewers

imp
• ian

Commits

rS351920: Loader: Add load offset to powerpc kernel entry point

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

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 26215
Build 24715: arc lint + arc unit

Event Timeline

jhibbits created this revision.Aug 16 2019, 3:40 AM

Harbormaster completed remote builds in B25906: Diff 60867.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.Aug 16 2019, 1:37 PM

imp added inline comments.

stand/common/load_elf.c
488–489	and why not adjust it here?
523	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

jhibbits added inline comments.Aug 16 2019, 2:15 PM

stand/common/load_elf.c
488–489	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.Aug 16 2019, 2:56 PM

stand/common/load_elf.c
523	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.

Harbormaster completed remote builds in B26215: Diff 61515.Sep 1 2019, 12:26 AM

jhibbits added inline comments.Sep 1 2019, 12:45 AM

stand/common/load_elf.c
480	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.Sep 5 2019, 9:46 PM

imp added inline comments.

stand/common/load_elf.c
523	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?