Re: [PATCH] ArmVirtPkg/ArmVirtPrePiUniCoreRelocatable: revert to PIE linking

["Ard Biesheuvel" <ard.biesheuvel@linaro.org>]

On Thu, 5 Sep 2019 at 07:19, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> On Wed, Sep 04, 2019 at 04:04:23PM -0700, Ard Biesheuvel wrote:
> > In some cases, the CLANG38 toolchain profile in LTO mode emits GOT
> > based relocations in spite of our attempts to avoid this, by using
> > hidden visibility, -Bsymbolic etc.
> >
> > On AARCH64, we managed to work around this by processing the GOT
> > based relocations in GenFw. As it turns out, the same issue exists
> > on 32-bit ARM, but unfortunately, we cannot use a similar trick to
> > get rid of the GOT entry, and the relocation metadata is insufficient
> > to locate the GOT entry in the binary.
> >
> > A bit of trial and error reveals that switching the linker options from
> > -pie to -shared in commit e07092edca8442db4a941dbeea0cd196c7bf8ec9 was
> > not the best approach, and instead, we can pass -pie to the linker
> > directly (using -Wl,xxx) rather than to the compiler directly, which
> > turns out to massage the linker in the right way, and prevents if from
> > emitting GOT based relocations. Your mileage may vary ...
> >
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > ---
> > Let's test this on a couple of different Clang versions. If it still
> > produces problems, the only other way I see is to disable the builds
> > of platforms that incorporate this module for ARM/CLANG38 [which is
> > not the end of the world]
> >
> >  ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf b/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.in
> > index 683397b7afd1..9e58e56fce09 100755
> > --- a/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf
> > +++ b/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf
> > @@ -97,4 +97,4 @@
> >    gArmTokenSpaceGuid.PcdFvBaseAddress
> >
> >  [BuildOptions]
> > -  GCC:*_*_*_DLINK_FLAGS = -shared -Wl,-Bsymbolic -Wl,-T,$(MODULE_DIR)/Scripts/PrePi-PIE.lds
> > +  GCC:*_*_*_DLINK_FLAGS = -Wl,-Bsymbolic,-pie,-T,$(MODULE_DIR)/Scripts/PrePi-PIE.lds
>
> We already merged a fix for AARCH64 though - could/should this be
> active on ARM only?
>

I'd like to keep the GenFw change, since we've confirmed that it
correctly handles GOT based relocations should they appear. But if
this fix works for all Clang versions, I'd rather have it active for
AArch64 as well, since the best solution is still not to have any such
relocations in the first place.

> A problem I have with this patch is that ArmVirtQemuKernel curently
> doesn't boot on my qemu (with/without kvm, built with GCC5 or CLANG38,
> with or without this patch):
> ProcessPciHost: Config[0x4010000000+0x10000000) Bus[0x0..0xFF]
> Io[0x0+0x10000)@0x3EFF0000 Mem32[0x10000000+0x2EFF0000)@0x0
> Mem64[0x8000000000+0x8000000000)@0x0
> MapGcdMmioSpace: failed to set memory space attributes for region
> [0x4010000000+0x10000000)
>
> ASSERT_EFI_ERROR (Status = Unsupported)
> ASSERT [PciHostBridgeDxe]
> /work/git/edk2/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c(293):
> !EFI_ERROR (Status)
> qemu-system-arm: terminating on signal 15 from pid 4680 (killall)
>
> /
>     Leif
>
> > --
> > 2.17.1
> >