Page MenuHomeFreeBSD

Fix PPC64 kernel build with clang8 + lld8
ClosedPublic

Authored by bdragon on Feb 25 2019, 5:53 PM.
Tags
None
Referenced Files
F104055404: D19352.id55864.diff
Mon, Dec 2, 10:34 PM
Unknown Object (File)
Sun, Nov 24, 10:00 AM
Unknown Object (File)
Sun, Nov 24, 6:23 AM
Unknown Object (File)
Sat, Nov 23, 5:59 AM
Unknown Object (File)
Fri, Nov 22, 4:10 AM
Unknown Object (File)
Thu, Nov 21, 11:23 AM
Unknown Object (File)
Thu, Nov 21, 6:36 AM
Unknown Object (File)
Wed, Nov 20, 7:48 AM

Details

Summary

This patch fixes the following lld link errors:

  • unsupported dynamic relocations on read-only sections
  • out-of-range TOC references

This patch is derived from git_bdragon.rtk0.net work, that did most of
the changes in fact.

Test Plan

Built and booted FreeBSD kernel with clang8 + lld8 on QEMU.
On bare metal, the kernel starts booting, but hangs when initializing the APs, possibly due to some other issue introduced by compiling the kernel with clang8.

Also checked that the kernel still compiles and works with base gcc + ld.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

luporl edited the test plan for this revision. (Show Details)
luporl added reviewers: bdragon, jhibbits.

Regarding the sys/conf/Makefile.powerpc, sys/conf/kern.pre.mk and
sys/powerpc/include/vparam.h changes that you left out, leaving them out is correct -- they were POWER9BSD and/or local hacks that are unrelated.

Regarding the linker script changes, I have been running with them for a very long time, but could always use more input on them.

The main idea behind the linker script changes is to force a simplified header so that petitboot can successfully load it, but it also has some bugfixes in it.

It may be possible to run without the linker script changes, but I haven't tried to.

Caveat about elfv2 kernel:
On the Talos II, I have been using my kexec-lite patch https://github.com/antonblanchard/kexec-lite/pull/9 (I keep a precompiled version at https://drop.rtk0.net/kexec for testing) because the kexec-lite in all shipped firmware versions has a calculation bug when executing elfv2 kernels.

  • this is not a problem when using the base compiler since that will build an elfv1 kernel, but when using base/gcc or a cross toolchain, the kernel will be built elfv2 and trigger the kexec-lite entry point bug.

This may be fixed for good in the future by providing a version of loader that runs inside petitboot as a petitboot plugin, but for now I've just been using my modified kexec to load.

I've confirmed that a kernel with this change compiles fine with base gcc + ld, and works fine on both pseries (qemu+kvm) and powernv (power8 machine).

jhibbits added inline comments.
sys/powerpc/aim/aim_machdep.c
399–407 ↗(On Diff #54363)

I really do not like this. This needlessly burns more reset vector space. There has to be a better way. What comes to mind, really is (since they're asm-file-local symbols) is:

GET_TOCBASE(rFoo)
ld %rBar, SYM@toc(rFoo)
mtctr %rBar
bctrl

Or some permutation thereof. The TRAP_GENTRAP and TRAP_TOCBASE addresses are fixed addresses chosen by necessity, since some vectors are restricted to 32 bytes, or 8 instructions so space is at a premium, and with those 2 special addresses it should be possible to address anything within trap_subr64.S, no need to burn extra vector space.

sys/powerpc/aim/trap_subr64.S
336 ↗(On Diff #54363)

There's no need to use the DMAP base here, we're already guaranteed to have IR||DR as 00, as this is the reset vector. This would save 2 instructions here, if we're counting.

This revision now requires changes to proceed.Mar 15 2019, 8:20 PM

Some parts of this will land separately. The ldscript chanages are split off to D19574 as more cases have been discovered where kernels were built with multiple PT_LOAD sections, even on ELFv1, and it should probably go in first.

sys/powerpc/aim/trap_subr64.S
358 ↗(On Diff #54363)

The ld needs to be s/%r2/%r1/ -- my mistake earlier.

394 ↗(On Diff #54363)

The ld needs to be s/%r2/%r3/ -- Same problem as above, my mistake. Otherwise the addis doesn't actually do anything.

The TOC_REF bits have landed separately as rS345676.

Patch updated, merged with parts already commited

luporl edited the test plan for this revision. (Show Details)
  • Do not ommit .toc section when linking

@alfredo.junior_eldorado.org.br and I found out that a kernel linked with lld won't boot if the .toc section is merged into .got.
Also note that the ldscript change was part of the original patch by @git_bdragon.rtk0.net, but was removed when merging with @jhibbits changes (D19574).

This needs further investigation to understand the real causes.
Maybe it is related to this bug: https://bugs.llvm.org/show_bug.cgi?id=31574

Well, it's probably from the alignment being off. Ensuring alignment there makes sense to me imo. Checking what the actual requirements are...

Yep, there is indeed an 8-byte alignment restriction on .toc.
Also the section being subsumed into .got might throw off the linker as well.

bdragon edited reviewers, added: luporl; removed: bdragon.

Commandeering this to post updated trap code.

sys/powerpc/aim/trap_subr64.S
336 ↗(On Diff #54363)

can save ONE instruction, because we still have to zero the register to use as the index base.

Rewrote trap code to not waste valuable trap space for addresses that are easily calculatable.
Tighten up the reset vector by one instruction.

marking done on addressed comments.

@luporl Feel free to commandeer this back again. Please do a test of the updated trap code with clang8/lld8. I have sucessfully booted this with my base/gcc 8.2.0.

Going to work on some issues I noticed with the power_save_sequence.

sys/powerpc/powerpc/cpu_subr64.S
69 ↗(On Diff #55910)

OK, agree w/ jhibbits now that we don't actually need to go through real mode from this, as we haven't overwritten the MSR at this point. Will update again.

This one is not tested properly yet but I wanted to bounce the idea off jhibbits.

  • Compute relative offset to power_save_sequence instead of jumping through a bunch of hoops, since we won't be leaving the current unit anyway.

@git_bdragon.rtk0.net, I haven't tested your last revision, but Alfredo and I have been using your previous revision for more than a month, both in ELFv1 environments as in ELFv2, with no issues so far, and it is essential for the ELFv2 transition on PPC.

Wouldn't it be better to commit the previous version and put this last change in a new review, until it is tested? @jhibbits?

sys/powerpc/aim/trap_subr64.S
506–507 ↗(On Diff #57426)

Since this runs in real-mode, this should be able to turn into:

ld %r1, TRAP_GENTRAP(0)

Same with others.

Remove DMAP_ZERO bits -- since we're in real mode while in the trap area, we can reach the magic trap address as TRAP_GENTRAP(0).

LGTM. Tested on ppcdevref.

This revision is now accepted and ready to land.May 22 2019, 2:09 PM
This revision was automatically updated to reflect the committed changes.