* [PATCH] ArmVirtPkg/ArmVirtPrePiUniCoreRelocatable: revert to PIE linking
@ 2019-09-04 23:04 Ard Biesheuvel
2019-09-05 14:19 ` Leif Lindholm
2019-09-05 19:33 ` [edk2-devel] " Laszlo Ersek
0 siblings, 2 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2019-09-04 23:04 UTC (permalink / raw)
To: devel; +Cc: leif.lindholm, lersek, Ard Biesheuvel
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.inf
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
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] ArmVirtPkg/ArmVirtPrePiUniCoreRelocatable: revert to PIE linking
2019-09-04 23:04 [PATCH] ArmVirtPkg/ArmVirtPrePiUniCoreRelocatable: revert to PIE linking Ard Biesheuvel
@ 2019-09-05 14:19 ` Leif Lindholm
2019-09-05 14:25 ` Ard Biesheuvel
2019-09-05 14:27 ` Ard Biesheuvel
2019-09-05 19:33 ` [edk2-devel] " Laszlo Ersek
1 sibling, 2 replies; 10+ messages in thread
From: Leif Lindholm @ 2019-09-05 14:19 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: devel, lersek
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?
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
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] ArmVirtPkg/ArmVirtPrePiUniCoreRelocatable: revert to PIE linking
2019-09-05 14:19 ` Leif Lindholm
@ 2019-09-05 14:25 ` Ard Biesheuvel
2019-09-05 15:55 ` Leif Lindholm
2019-09-05 14:27 ` Ard Biesheuvel
1 sibling, 1 reply; 10+ messages in thread
From: Ard Biesheuvel @ 2019-09-05 14:25 UTC (permalink / raw)
To: Leif Lindholm; +Cc: edk2-devel-groups-io, Laszlo Ersek
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?
>
> 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)
>
Does it work with -M virt,highmem=off ?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ArmVirtPkg/ArmVirtPrePiUniCoreRelocatable: revert to PIE linking
2019-09-05 14:25 ` Ard Biesheuvel
@ 2019-09-05 15:55 ` Leif Lindholm
2019-09-05 17:06 ` Ard Biesheuvel
0 siblings, 1 reply; 10+ messages in thread
From: Leif Lindholm @ 2019-09-05 15:55 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel-groups-io, Laszlo Ersek
On Thu, Sep 05, 2019 at 07:25:39AM -0700, Ard Biesheuvel wrote:
> > > [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?
> >
> > 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)
> >
>
> Does it work with -M virt,highmem=off ?
Ah, yes - that works fine then, also with the patch.
Well, with that, and your explanation in the other thread:
Acked-by: Leif Lindholm <leif.lindholm@linaro.org>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ArmVirtPkg/ArmVirtPrePiUniCoreRelocatable: revert to PIE linking
2019-09-05 15:55 ` Leif Lindholm
@ 2019-09-05 17:06 ` Ard Biesheuvel
2019-09-05 17:16 ` Leif Lindholm
0 siblings, 1 reply; 10+ messages in thread
From: Ard Biesheuvel @ 2019-09-05 17:06 UTC (permalink / raw)
To: Leif Lindholm; +Cc: edk2-devel-groups-io, Laszlo Ersek
On Thu, 5 Sep 2019 at 08:55, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> On Thu, Sep 05, 2019 at 07:25:39AM -0700, Ard Biesheuvel wrote:
> > > > [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?
> > >
> > > 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)
> > >
> >
> > Does it work with -M virt,highmem=off ?
>
> Ah, yes - that works fine then, also with the patch.
>
> Well, with that, and your explanation in the other thread:
> Acked-by: Leif Lindholm <leif.lindholm@linaro.org>
Thanks. I am going to replace the last paragraph of the commit log with
Note that in this particular case, we are interested in PIE linking
only (i.e., producing a .rela section containing dynamic relocations
that the startup code can process directly), and not in position
independent code generation, and by passing the -pie option to the
linker directly using -Wl,-pie (and dropping -shared), we can coerce
the GOLD linker into doing only the former rather than both when it
performs its LTO code generation.
and push (unless you have any objections)
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] ArmVirtPkg/ArmVirtPrePiUniCoreRelocatable: revert to PIE linking
2019-09-05 17:06 ` Ard Biesheuvel
@ 2019-09-05 17:16 ` Leif Lindholm
2019-09-05 17:19 ` Ard Biesheuvel
0 siblings, 1 reply; 10+ messages in thread
From: Leif Lindholm @ 2019-09-05 17:16 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel-groups-io, Laszlo Ersek
On Thu, Sep 05, 2019 at 10:06:56AM -0700, Ard Biesheuvel wrote:
> On Thu, 5 Sep 2019 at 08:55, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> >
> > On Thu, Sep 05, 2019 at 07:25:39AM -0700, Ard Biesheuvel wrote:
> > > > > [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?
> > > >
> > > > 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)
> > > >
> > >
> > > Does it work with -M virt,highmem=off ?
> >
> > Ah, yes - that works fine then, also with the patch.
> >
> > Well, with that, and your explanation in the other thread:
> > Acked-by: Leif Lindholm <leif.lindholm@linaro.org>
>
> Thanks. I am going to replace the last paragraph of the commit log with
>
> Note that in this particular case, we are interested in PIE linking
> only (i.e., producing a .rela section containing dynamic relocations
> that the startup code can process directly), and not in position
> independent code generation, and by passing the -pie option to the
> linker directly using -Wl,-pie (and dropping -shared), we can coerce
> the GOLD linker into doing only the former rather than both when it
> performs its LTO code generation.
>
> and push (unless you have any objections)
No objections.
/
Leif
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] ArmVirtPkg/ArmVirtPrePiUniCoreRelocatable: revert to PIE linking
2019-09-05 17:16 ` Leif Lindholm
@ 2019-09-05 17:19 ` Ard Biesheuvel
0 siblings, 0 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2019-09-05 17:19 UTC (permalink / raw)
To: Leif Lindholm; +Cc: edk2-devel-groups-io, Laszlo Ersek
On Thu, 5 Sep 2019 at 10:16, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> On Thu, Sep 05, 2019 at 10:06:56AM -0700, Ard Biesheuvel wrote:
> > On Thu, 5 Sep 2019 at 08:55, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > >
> > > On Thu, Sep 05, 2019 at 07:25:39AM -0700, Ard Biesheuvel wrote:
> > > > > > [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?
> > > > >
> > > > > 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)
> > > > >
> > > >
> > > > Does it work with -M virt,highmem=off ?
> > >
> > > Ah, yes - that works fine then, also with the patch.
> > >
> > > Well, with that, and your explanation in the other thread:
> > > Acked-by: Leif Lindholm <leif.lindholm@linaro.org>
> >
> > Thanks. I am going to replace the last paragraph of the commit log with
> >
> > Note that in this particular case, we are interested in PIE linking
> > only (i.e., producing a .rela section containing dynamic relocations
> > that the startup code can process directly), and not in position
> > independent code generation, and by passing the -pie option to the
> > linker directly using -Wl,-pie (and dropping -shared), we can coerce
> > the GOLD linker into doing only the former rather than both when it
> > performs its LTO code generation.
> >
> > and push (unless you have any objections)
>
> No objections.
>
Thanks
Pushed as 8a1305a11f3b..04d9d89b7dd4
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ArmVirtPkg/ArmVirtPrePiUniCoreRelocatable: revert to PIE linking
2019-09-05 14:19 ` Leif Lindholm
2019-09-05 14:25 ` Ard Biesheuvel
@ 2019-09-05 14:27 ` Ard Biesheuvel
1 sibling, 0 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2019-09-05 14:27 UTC (permalink / raw)
To: Leif Lindholm; +Cc: edk2-devel-groups-io, Laszlo Ersek
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
> >
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [PATCH] ArmVirtPkg/ArmVirtPrePiUniCoreRelocatable: revert to PIE linking
2019-09-04 23:04 [PATCH] ArmVirtPkg/ArmVirtPrePiUniCoreRelocatable: revert to PIE linking Ard Biesheuvel
2019-09-05 14:19 ` Leif Lindholm
@ 2019-09-05 19:33 ` Laszlo Ersek
2019-09-06 22:47 ` Ard Biesheuvel
1 sibling, 1 reply; 10+ messages in thread
From: Laszlo Ersek @ 2019-09-05 19:33 UTC (permalink / raw)
To: devel, ard.biesheuvel; +Cc: leif.lindholm
On 09/05/19 01:04, 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.inf
> 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
>
Sorry for being late to this patch, I can see [*] it's upstream alrady.
Nonetheless:
Acked-by: Laszlo Ersek <lersek@redhat.com>
[*] Not from my inbox though -- I've got again too much on my plate and
am fetching email in batches. But, between my rebasing of
"ArmVirtPkg/PlatformBootManagerLib: unload image on
EFI_SECURITY_VIOLATION", building it, and attempting to push it, you had
pushed this one -- and I did notice that. :)
Thanks,
Laszlo
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [PATCH] ArmVirtPkg/ArmVirtPrePiUniCoreRelocatable: revert to PIE linking
2019-09-05 19:33 ` [edk2-devel] " Laszlo Ersek
@ 2019-09-06 22:47 ` Ard Biesheuvel
0 siblings, 0 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2019-09-06 22:47 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: edk2-devel-groups-io, Leif Lindholm
On Thu, 5 Sep 2019 at 12:33, Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 09/05/19 01:04, 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.inf
> > 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
> >
>
> Sorry for being late to this patch, I can see [*] it's upstream alrady.
> Nonetheless:
>
> Acked-by: Laszlo Ersek <lersek@redhat.com>
>
Thanks. Since the patch is already merged, I'll find another patch to
attach it to :-)
> [*] Not from my inbox though -- I've got again too much on my plate and
> am fetching email in batches. But, between my rebasing of
> "ArmVirtPkg/PlatformBootManagerLib: unload image on
> EFI_SECURITY_VIOLATION", building it, and attempting to push it, you had
> pushed this one -- and I did notice that. :)
>
> Thanks,
> Laszlo
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-09-06 22:47 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-09-04 23:04 [PATCH] ArmVirtPkg/ArmVirtPrePiUniCoreRelocatable: revert to PIE linking Ard Biesheuvel
2019-09-05 14:19 ` Leif Lindholm
2019-09-05 14:25 ` Ard Biesheuvel
2019-09-05 15:55 ` Leif Lindholm
2019-09-05 17:06 ` Ard Biesheuvel
2019-09-05 17:16 ` Leif Lindholm
2019-09-05 17:19 ` Ard Biesheuvel
2019-09-05 14:27 ` Ard Biesheuvel
2019-09-05 19:33 ` [edk2-devel] " Laszlo Ersek
2019-09-06 22:47 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox