* [PATCH 0/2] ArmVirtPkg: fixes for ARM and CLANG38 builds
@ 2018-12-11 12:19 Ard Biesheuvel
2018-12-11 12:19 ` [PATCH 1/2] ArmVirtPkg/ArmVirtQemuKernel ARM: make some PCD settings apply to ARM Ard Biesheuvel
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Ard Biesheuvel @ 2018-12-11 12:19 UTC (permalink / raw)
To: edk2-devel; +Cc: lersek, leif.lindholm, philmd, Ard Biesheuvel
Patch #1 reshuffles the .dsc contents of ArmVirtQemuKernel so that we
can run on ARM targets as well.
Patch #2 fixes the CLANG38 build for ArmVirtQemuKernel and ArmVirtXen,
by tweaking the linker options passed to emit the self-relocating PrePi
SEC module.
Ard Biesheuvel (2):
ArmVirtPkg/ArmVirtQemuKernel ARM: make some PCD settings apply to ARM
ArmVirtPkg/PrePiUniCoreRelocatable CLANG38: work around build issues
ArmVirtPkg/ArmVirtQemuKernel.dsc | 32 +++++++++++--------
ArmVirtPkg/ArmVirtXen.dsc | 12 +++++--
.../PrePi/ArmVirtPrePiUniCoreRelocatable.inf | 2 +-
ArmVirtPkg/Include/Platform/Hidden.h | 23 +++++++++++++
4 files changed, 53 insertions(+), 16 deletions(-)
create mode 100644 ArmVirtPkg/Include/Platform/Hidden.h
--
2.19.2
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 1/2] ArmVirtPkg/ArmVirtQemuKernel ARM: make some PCD settings apply to ARM 2018-12-11 12:19 [PATCH 0/2] ArmVirtPkg: fixes for ARM and CLANG38 builds Ard Biesheuvel @ 2018-12-11 12:19 ` Ard Biesheuvel 2018-12-11 15:59 ` Laszlo Ersek 2018-12-11 12:19 ` [PATCH 2/2] ArmVirtPkg/PrePiUniCoreRelocatable CLANG38: work around build issues Ard Biesheuvel 2018-12-11 16:50 ` [PATCH 0/2] ArmVirtPkg: fixes for ARM and CLANG38 builds Ard Biesheuvel 2 siblings, 1 reply; 8+ messages in thread From: Ard Biesheuvel @ 2018-12-11 12:19 UTC (permalink / raw) To: edk2-devel; +Cc: lersek, leif.lindholm, philmd, Ard Biesheuvel Move some PCD settings outs of the [PcdsFixedAtBuild.AARCH64] block, so that they apply to 32-bit ARM as well. Without this change, the ARM build doesn't work. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- ArmVirtPkg/ArmVirtQemuKernel.dsc | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc index d8fbf14e8f4e..9928919bf5b0 100644 --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc @@ -130,6 +130,15 @@ [PcdsFixedAtBuild.common] gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections|TRUE !endif + gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE + gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 } + + # + # The maximum physical I/O addressability of the processor, set with + # BuildCpuHob(). + # + gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16 + [PcdsPatchableInModule.common] # # This will be overridden in the code @@ -146,17 +155,6 @@ [PcdsPatchableInModule.common] gArmTokenSpaceGuid.PcdFdBaseAddress|0x0 gArmTokenSpaceGuid.PcdFvBaseAddress|0x0 -[PcdsFixedAtBuild.AARCH64] - - gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE - gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 } - - # - # The maximum physical I/O addressability of the processor, set with - # BuildCpuHob(). - # - gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16 - [PcdsDynamicDefault.common] gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3 -- 2.19.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] ArmVirtPkg/ArmVirtQemuKernel ARM: make some PCD settings apply to ARM 2018-12-11 12:19 ` [PATCH 1/2] ArmVirtPkg/ArmVirtQemuKernel ARM: make some PCD settings apply to ARM Ard Biesheuvel @ 2018-12-11 15:59 ` Laszlo Ersek 0 siblings, 0 replies; 8+ messages in thread From: Laszlo Ersek @ 2018-12-11 15:59 UTC (permalink / raw) To: Ard Biesheuvel, edk2-devel On 12/11/18 13:19, Ard Biesheuvel wrote: > Move some PCD settings outs of the [PcdsFixedAtBuild.AARCH64] block, > so that they apply to 32-bit ARM as well. Without this change, the > ARM build doesn't work. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > ArmVirtPkg/ArmVirtQemuKernel.dsc | 20 +++++++++----------- > 1 file changed, 9 insertions(+), 11 deletions(-) > > diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc > index d8fbf14e8f4e..9928919bf5b0 100644 > --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc > +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc > @@ -130,6 +130,15 @@ [PcdsFixedAtBuild.common] > gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections|TRUE > !endif > > + gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE > + gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 } > + > + # > + # The maximum physical I/O addressability of the processor, set with > + # BuildCpuHob(). > + # > + gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16 > + > [PcdsPatchableInModule.common] > # > # This will be overridden in the code > @@ -146,17 +155,6 @@ [PcdsPatchableInModule.common] > gArmTokenSpaceGuid.PcdFdBaseAddress|0x0 > gArmTokenSpaceGuid.PcdFvBaseAddress|0x0 > > -[PcdsFixedAtBuild.AARCH64] > - > - gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE > - gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 } > - > - # > - # The maximum physical I/O addressability of the processor, set with > - # BuildCpuHob(). > - # > - gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16 > - > [PcdsDynamicDefault.common] > gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3 > > Reviewed-by: Laszlo Ersek <lersek@redhat.com> ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] ArmVirtPkg/PrePiUniCoreRelocatable CLANG38: work around build issues 2018-12-11 12:19 [PATCH 0/2] ArmVirtPkg: fixes for ARM and CLANG38 builds Ard Biesheuvel 2018-12-11 12:19 ` [PATCH 1/2] ArmVirtPkg/ArmVirtQemuKernel ARM: make some PCD settings apply to ARM Ard Biesheuvel @ 2018-12-11 12:19 ` Ard Biesheuvel 2018-12-11 12:57 ` Leif Lindholm 2018-12-11 16:50 ` [PATCH 0/2] ArmVirtPkg: fixes for ARM and CLANG38 builds Ard Biesheuvel 2 siblings, 1 reply; 8+ messages in thread From: Ard Biesheuvel @ 2018-12-11 12:19 UTC (permalink / raw) To: edk2-devel; +Cc: lersek, leif.lindholm, philmd, Ard Biesheuvel The self-relocating PrePi module that is used by the ArmVirtQemuKernel and ArmVirtXen targets runs the linker in PIE mode so that it emits dynamic relocations into the final image in a way that permits the module to relocate itself into place before calling into the C code. When building these targets using the CLANG38 toolchain, we switch from the BFD to the GOLD linker, which behaves a bit differently when building PIE executables, and insists on emitting GOT indirected symbol references throughout, which means a) that we end up with absolute addresses (which need to be fixed up at load time) for no good reason, and b) we have to add support for handling GOT entries to GenFw if we want to convert them into PE/COFF. So instead, let's emit a shared library. Since the ELF image only serves as the input to GenFw, this does not lead to any loss of functionality, although it does require the -Bsymbolic linker option to be added to ensure that no symbol based dynamic relocations are emitted (which would, e.g., permit lazy binding for shared libraries). So for all other toolchains, the linker option changes are a no-op. Then, we have to convince CLANG38/GOLD that there is no need to refer to symbols via a GOT entry. This is done by forcing hidden visibility for all symbols in all components that make up the PrePi SEC module: this informs the linker that a symbol is never exported or preempted, making it safe to refer to it directly from anywhere in the code, rather than indirectly via a GOT entry. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- ArmVirtPkg/ArmVirtQemuKernel.dsc | 12 ++++++++-- ArmVirtPkg/ArmVirtXen.dsc | 12 ++++++++-- ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf | 2 +- ArmVirtPkg/Include/Platform/Hidden.h | 23 ++++++++++++++++++++ 4 files changed, 44 insertions(+), 5 deletions(-) diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc index 9928919bf5b0..c4324a9e264b 100644 --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc @@ -68,11 +68,19 @@ [LibraryClasses.common] [LibraryClasses.common.UEFI_DRIVER] UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf -[BuildOptions.ARM.EDKII.SEC, BuildOptions.ARM.EDKII.BASE] +[BuildOptions.common.EDKII.SEC, BuildOptions.common.EDKII.BASE] # Avoid MOVT/MOVW instruction pairs in code that may end up in the PIE # executable we build for the relocatable PrePi. They are not runtime # relocatable in ELF. - *_CLANG35_*_CC_FLAGS = -mno-movt + *_CLANG35_ARM_CC_FLAGS = -mno-movt + + # + # CLANG38 with LTO support enabled uses the GNU GOLD linker, which insists + # on emitting GOT based symbol references when running in shared mode, unless + # we override visibility to 'hidden' in all modules that make up the PrePi + # build. + # + GCC:*_CLANG38_*_CC_FLAGS = -include $(WORKSPACE)/ArmVirtPkg/Include/Platform/Hidden.h ################################################################################ # diff --git a/ArmVirtPkg/ArmVirtXen.dsc b/ArmVirtPkg/ArmVirtXen.dsc index 20fae9e675bb..e083666f54ea 100644 --- a/ArmVirtPkg/ArmVirtXen.dsc +++ b/ArmVirtPkg/ArmVirtXen.dsc @@ -57,11 +57,19 @@ [LibraryClasses] [LibraryClasses.common.UEFI_DRIVER] UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf -[BuildOptions.ARM.EDKII.SEC, BuildOptions.ARM.EDKII.BASE] +[BuildOptions.common.EDKII.SEC, BuildOptions.common.EDKII.BASE] # Avoid MOVT/MOVW instruction pairs in code that may end up in the PIE # executable we build for the relocatable PrePi. They are not runtime # relocatable in ELF. - *_CLANG35_*_CC_FLAGS = -mno-movt + *_CLANG35_ARM_CC_FLAGS = -mno-movt + + # + # CLANG38 with LTO support enabled uses the GNU GOLD linker, which insists + # on emitting GOT based symbol references when running in shared mode, unless + # we override visibility to 'hidden' in all modules that make up the PrePi + # build. + # + GCC:*_CLANG38_*_CC_FLAGS = -include $(WORKSPACE)/ArmVirtPkg/Include/Platform/Hidden.h ################################################################################ # diff --git a/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf b/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf index 034ddb41cb48..5fe6cd8eb481 100755 --- a/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf +++ b/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf @@ -105,4 +105,4 @@ [Pcd] gArmTokenSpaceGuid.PcdFvBaseAddress [BuildOptions] - GCC:*_*_*_DLINK_FLAGS = -pie -Wl,-T,$(MODULE_DIR)/Scripts/PrePi-PIE.lds + GCC:*_*_*_DLINK_FLAGS = -shared -Wl,-Bsymbolic -Wl,-T,$(MODULE_DIR)/Scripts/PrePi-PIE.lds diff --git a/ArmVirtPkg/Include/Platform/Hidden.h b/ArmVirtPkg/Include/Platform/Hidden.h new file mode 100644 index 000000000000..20e9f5d896b2 --- /dev/null +++ b/ArmVirtPkg/Include/Platform/Hidden.h @@ -0,0 +1,23 @@ +/** @file + + Copyright (c) 2018, Linaro Limited. All rights reserved. + + This program and the accompanying materials + are licensed and made available under the terms and conditions of the BSD License + which accompanies this distribution. The full text of the license may be found at + http://opensource.org/licenses/bsd-license.php + + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. + +**/ + +// +// Setting the GCC -fvisibility=hidden command line option is not quite the same +// as setting the pragma below: the former only affects definitions, whereas the +// latter affects extern declarations as well. So if we want to ensure that no +// GOT indirected symbol references are emitted, we need to use the pragma, or +// GOT based cross object references could be emitted, e.g., in libraries, and +// these cannot be relaxed to ordinary symbol references at link time. +// +#pragma GCC visibility push (hidden) -- 2.19.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] ArmVirtPkg/PrePiUniCoreRelocatable CLANG38: work around build issues 2018-12-11 12:19 ` [PATCH 2/2] ArmVirtPkg/PrePiUniCoreRelocatable CLANG38: work around build issues Ard Biesheuvel @ 2018-12-11 12:57 ` Leif Lindholm 2018-12-11 12:58 ` Ard Biesheuvel 0 siblings, 1 reply; 8+ messages in thread From: Leif Lindholm @ 2018-12-11 12:57 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: edk2-devel, lersek, philmd Not my package, but a couple of minor style/language comments below (because this is awkward enough any future readers/users will need all the help they can get). On Tue, Dec 11, 2018 at 01:19:36PM +0100, Ard Biesheuvel wrote: > The self-relocating PrePi module that is used by the ArmVirtQemuKernel > and ArmVirtXen targets runs the linker in PIE mode so that it emits > dynamic relocations into the final image in a way that permits the > module to relocate itself into place before calling into the C code. > > When building these targets using the CLANG38 toolchain, we switch > from the BFD to the GOLD linker, which behaves a bit differently when > building PIE executables, and insists on emitting GOT indirected symbol > references throughout, which means a) that we end up with absolute > addresses (which need to be fixed up at load time) for no good reason, > and b) we have to add support for handling GOT entries to GenFw if we > want to convert them into PE/COFF. > > So instead, let's emit a shared library. Since the ELF image only serves > as the input to GenFw, this does not lead to any loss of functionality, > although it does require the -Bsymbolic linker option to be added to > ensure that no symbol based dynamic relocations are emitted (which > would, e.g., permit lazy binding for shared libraries). So for all > other toolchains, the linker option changes are a no-op. > > Then, we have to convince CLANG38/GOLD that there is no need to refer > to symbols via a GOT entry. This is done by forcing hidden visibility > for all symbols in all components that make up the PrePi SEC module: > this informs the linker that a symbol is never exported or preempted, > making it safe to refer to it directly from anywhere in the code, > rather than indirectly via a GOT entry. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > ArmVirtPkg/ArmVirtQemuKernel.dsc | 12 ++++++++-- > ArmVirtPkg/ArmVirtXen.dsc | 12 ++++++++-- > ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf | 2 +- > ArmVirtPkg/Include/Platform/Hidden.h | 23 ++++++++++++++++++++ > 4 files changed, 44 insertions(+), 5 deletions(-) > > diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc > index 9928919bf5b0..c4324a9e264b 100644 > --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc > +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc > @@ -68,11 +68,19 @@ [LibraryClasses.common] > [LibraryClasses.common.UEFI_DRIVER] > UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf > > -[BuildOptions.ARM.EDKII.SEC, BuildOptions.ARM.EDKII.BASE] > +[BuildOptions.common.EDKII.SEC, BuildOptions.common.EDKII.BASE] > # Avoid MOVT/MOVW instruction pairs in code that may end up in the PIE > # executable we build for the relocatable PrePi. They are not runtime > # relocatable in ELF. > - *_CLANG35_*_CC_FLAGS = -mno-movt > + *_CLANG35_ARM_CC_FLAGS = -mno-movt > + > + # > + # CLANG38 with LTO support enabled uses the GNU GOLD linker, which insists > + # on emitting GOT based symbol references when running in shared mode, unless > + # we override visibility to 'hidden' in all modules that make up the PrePi > + # build. > + # > + GCC:*_CLANG38_*_CC_FLAGS = -include $(WORKSPACE)/ArmVirtPkg/Include/Platform/Hidden.h > > ################################################################################ > # > diff --git a/ArmVirtPkg/ArmVirtXen.dsc b/ArmVirtPkg/ArmVirtXen.dsc > index 20fae9e675bb..e083666f54ea 100644 > --- a/ArmVirtPkg/ArmVirtXen.dsc > +++ b/ArmVirtPkg/ArmVirtXen.dsc > @@ -57,11 +57,19 @@ [LibraryClasses] > [LibraryClasses.common.UEFI_DRIVER] > UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf > > -[BuildOptions.ARM.EDKII.SEC, BuildOptions.ARM.EDKII.BASE] > +[BuildOptions.common.EDKII.SEC, BuildOptions.common.EDKII.BASE] > # Avoid MOVT/MOVW instruction pairs in code that may end up in the PIE > # executable we build for the relocatable PrePi. They are not runtime > # relocatable in ELF. > - *_CLANG35_*_CC_FLAGS = -mno-movt > + *_CLANG35_ARM_CC_FLAGS = -mno-movt > + > + # > + # CLANG38 with LTO support enabled uses the GNU GOLD linker, which insists > + # on emitting GOT based symbol references when running in shared mode, unless > + # we override visibility to 'hidden' in all modules that make up the PrePi > + # build. > + # > + GCC:*_CLANG38_*_CC_FLAGS = -include $(WORKSPACE)/ArmVirtPkg/Include/Platform/Hidden.h > > ################################################################################ > # > diff --git a/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf b/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf > index 034ddb41cb48..5fe6cd8eb481 100755 > --- a/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf > +++ b/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf > @@ -105,4 +105,4 @@ [Pcd] > gArmTokenSpaceGuid.PcdFvBaseAddress > > [BuildOptions] > - GCC:*_*_*_DLINK_FLAGS = -pie -Wl,-T,$(MODULE_DIR)/Scripts/PrePi-PIE.lds > + GCC:*_*_*_DLINK_FLAGS = -shared -Wl,-Bsymbolic -Wl,-T,$(MODULE_DIR)/Scripts/PrePi-PIE.lds > diff --git a/ArmVirtPkg/Include/Platform/Hidden.h b/ArmVirtPkg/Include/Platform/Hidden.h > new file mode 100644 > index 000000000000..20e9f5d896b2 > --- /dev/null > +++ b/ArmVirtPkg/Include/Platform/Hidden.h > @@ -0,0 +1,23 @@ > +/** @file > + > + Copyright (c) 2018, Linaro Limited. All rights reserved. > + > + This program and the accompanying materials > + are licensed and made available under the terms and conditions of the BSD License > + which accompanies this distribution. The full text of the license may be found at > + http://opensource.org/licenses/bsd-license.php > + > + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > + > +**/ > + I know this isn't intended to be explicitly included, but could we still have in include guard? > +// > +// Setting the GCC -fvisibility=hidden command line option is not quite the same > +// as setting the pragma below: the former only affects definitions, whereas the > +// latter affects extern declarations as well. So if we want to ensure that no s/latter/pragma/? (Just saves on recursive natural language parsing when those cirquits may already be in use.) / Leif > +// GOT indirected symbol references are emitted, we need to use the pragma, or > +// GOT based cross object references could be emitted, e.g., in libraries, and > +// these cannot be relaxed to ordinary symbol references at link time. > +// > +#pragma GCC visibility push (hidden) > -- > 2.19.2 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] ArmVirtPkg/PrePiUniCoreRelocatable CLANG38: work around build issues 2018-12-11 12:57 ` Leif Lindholm @ 2018-12-11 12:58 ` Ard Biesheuvel 2018-12-11 16:10 ` Laszlo Ersek 0 siblings, 1 reply; 8+ messages in thread From: Ard Biesheuvel @ 2018-12-11 12:58 UTC (permalink / raw) To: Leif Lindholm Cc: edk2-devel@lists.01.org, Laszlo Ersek, Philippe Mathieu-Daudé On Tue, 11 Dec 2018 at 13:57, Leif Lindholm <leif.lindholm@linaro.org> wrote: > > Not my package, but a couple of minor style/language comments below > (because this is awkward enough any future readers/users will need all > the help they can get). > > On Tue, Dec 11, 2018 at 01:19:36PM +0100, Ard Biesheuvel wrote: > > The self-relocating PrePi module that is used by the ArmVirtQemuKernel > > and ArmVirtXen targets runs the linker in PIE mode so that it emits > > dynamic relocations into the final image in a way that permits the > > module to relocate itself into place before calling into the C code. > > > > When building these targets using the CLANG38 toolchain, we switch > > from the BFD to the GOLD linker, which behaves a bit differently when > > building PIE executables, and insists on emitting GOT indirected symbol > > references throughout, which means a) that we end up with absolute > > addresses (which need to be fixed up at load time) for no good reason, > > and b) we have to add support for handling GOT entries to GenFw if we > > want to convert them into PE/COFF. > > > > So instead, let's emit a shared library. Since the ELF image only serves > > as the input to GenFw, this does not lead to any loss of functionality, > > although it does require the -Bsymbolic linker option to be added to > > ensure that no symbol based dynamic relocations are emitted (which > > would, e.g., permit lazy binding for shared libraries). So for all > > other toolchains, the linker option changes are a no-op. > > > > Then, we have to convince CLANG38/GOLD that there is no need to refer > > to symbols via a GOT entry. This is done by forcing hidden visibility > > for all symbols in all components that make up the PrePi SEC module: > > this informs the linker that a symbol is never exported or preempted, > > making it safe to refer to it directly from anywhere in the code, > > rather than indirectly via a GOT entry. > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > --- > > ArmVirtPkg/ArmVirtQemuKernel.dsc | 12 ++++++++-- > > ArmVirtPkg/ArmVirtXen.dsc | 12 ++++++++-- > > ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf | 2 +- > > ArmVirtPkg/Include/Platform/Hidden.h | 23 ++++++++++++++++++++ > > 4 files changed, 44 insertions(+), 5 deletions(-) > > > > diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc > > index 9928919bf5b0..c4324a9e264b 100644 > > --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc > > +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc > > @@ -68,11 +68,19 @@ [LibraryClasses.common] > > [LibraryClasses.common.UEFI_DRIVER] > > UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf > > > > -[BuildOptions.ARM.EDKII.SEC, BuildOptions.ARM.EDKII.BASE] > > +[BuildOptions.common.EDKII.SEC, BuildOptions.common.EDKII.BASE] > > # Avoid MOVT/MOVW instruction pairs in code that may end up in the PIE > > # executable we build for the relocatable PrePi. They are not runtime > > # relocatable in ELF. > > - *_CLANG35_*_CC_FLAGS = -mno-movt > > + *_CLANG35_ARM_CC_FLAGS = -mno-movt > > + > > + # > > + # CLANG38 with LTO support enabled uses the GNU GOLD linker, which insists > > + # on emitting GOT based symbol references when running in shared mode, unless > > + # we override visibility to 'hidden' in all modules that make up the PrePi > > + # build. > > + # > > + GCC:*_CLANG38_*_CC_FLAGS = -include $(WORKSPACE)/ArmVirtPkg/Include/Platform/Hidden.h > > > > ################################################################################ > > # > > diff --git a/ArmVirtPkg/ArmVirtXen.dsc b/ArmVirtPkg/ArmVirtXen.dsc > > index 20fae9e675bb..e083666f54ea 100644 > > --- a/ArmVirtPkg/ArmVirtXen.dsc > > +++ b/ArmVirtPkg/ArmVirtXen.dsc > > @@ -57,11 +57,19 @@ [LibraryClasses] > > [LibraryClasses.common.UEFI_DRIVER] > > UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf > > > > -[BuildOptions.ARM.EDKII.SEC, BuildOptions.ARM.EDKII.BASE] > > +[BuildOptions.common.EDKII.SEC, BuildOptions.common.EDKII.BASE] > > # Avoid MOVT/MOVW instruction pairs in code that may end up in the PIE > > # executable we build for the relocatable PrePi. They are not runtime > > # relocatable in ELF. > > - *_CLANG35_*_CC_FLAGS = -mno-movt > > + *_CLANG35_ARM_CC_FLAGS = -mno-movt > > + > > + # > > + # CLANG38 with LTO support enabled uses the GNU GOLD linker, which insists > > + # on emitting GOT based symbol references when running in shared mode, unless > > + # we override visibility to 'hidden' in all modules that make up the PrePi > > + # build. > > + # > > + GCC:*_CLANG38_*_CC_FLAGS = -include $(WORKSPACE)/ArmVirtPkg/Include/Platform/Hidden.h > > > > ################################################################################ > > # > > diff --git a/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf b/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf > > index 034ddb41cb48..5fe6cd8eb481 100755 > > --- a/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf > > +++ b/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf > > @@ -105,4 +105,4 @@ [Pcd] > > gArmTokenSpaceGuid.PcdFvBaseAddress > > > > [BuildOptions] > > - GCC:*_*_*_DLINK_FLAGS = -pie -Wl,-T,$(MODULE_DIR)/Scripts/PrePi-PIE.lds > > + GCC:*_*_*_DLINK_FLAGS = -shared -Wl,-Bsymbolic -Wl,-T,$(MODULE_DIR)/Scripts/PrePi-PIE.lds > > diff --git a/ArmVirtPkg/Include/Platform/Hidden.h b/ArmVirtPkg/Include/Platform/Hidden.h > > new file mode 100644 > > index 000000000000..20e9f5d896b2 > > --- /dev/null > > +++ b/ArmVirtPkg/Include/Platform/Hidden.h > > @@ -0,0 +1,23 @@ > > +/** @file > > + > > + Copyright (c) 2018, Linaro Limited. All rights reserved. > > + > > + This program and the accompanying materials > > + are licensed and made available under the terms and conditions of the BSD License > > + which accompanies this distribution. The full text of the license may be found at > > + http://opensource.org/licenses/bsd-license.php > > + > > + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > > + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > > + > > +**/ > > + > > I know this isn't intended to be explicitly included, but could we > still have in include guard? > Sure. > > +// > > +// Setting the GCC -fvisibility=hidden command line option is not quite the same > > +// as setting the pragma below: the former only affects definitions, whereas the > > +// latter affects extern declarations as well. So if we want to ensure that no > > s/latter/pragma/? (Just saves on recursive natural language parsing > Fair enough. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] ArmVirtPkg/PrePiUniCoreRelocatable CLANG38: work around build issues 2018-12-11 12:58 ` Ard Biesheuvel @ 2018-12-11 16:10 ` Laszlo Ersek 0 siblings, 0 replies; 8+ messages in thread From: Laszlo Ersek @ 2018-12-11 16:10 UTC (permalink / raw) To: Ard Biesheuvel, Leif Lindholm Cc: edk2-devel@lists.01.org, Philippe Mathieu-Daudé On 12/11/18 13:58, Ard Biesheuvel wrote: > On Tue, 11 Dec 2018 at 13:57, Leif Lindholm <leif.lindholm@linaro.org> wrote: >> >> Not my package, but a couple of minor style/language comments below >> (because this is awkward enough any future readers/users will need all >> the help they can get). >> >> On Tue, Dec 11, 2018 at 01:19:36PM +0100, Ard Biesheuvel wrote: >>> The self-relocating PrePi module that is used by the ArmVirtQemuKernel >>> and ArmVirtXen targets runs the linker in PIE mode so that it emits >>> dynamic relocations into the final image in a way that permits the >>> module to relocate itself into place before calling into the C code. >>> >>> When building these targets using the CLANG38 toolchain, we switch >>> from the BFD to the GOLD linker, which behaves a bit differently when >>> building PIE executables, and insists on emitting GOT indirected symbol >>> references throughout, which means a) that we end up with absolute >>> addresses (which need to be fixed up at load time) for no good reason, >>> and b) we have to add support for handling GOT entries to GenFw if we >>> want to convert them into PE/COFF. >>> >>> So instead, let's emit a shared library. Since the ELF image only serves >>> as the input to GenFw, this does not lead to any loss of functionality, >>> although it does require the -Bsymbolic linker option to be added to >>> ensure that no symbol based dynamic relocations are emitted (which >>> would, e.g., permit lazy binding for shared libraries). So for all >>> other toolchains, the linker option changes are a no-op. >>> >>> Then, we have to convince CLANG38/GOLD that there is no need to refer >>> to symbols via a GOT entry. This is done by forcing hidden visibility >>> for all symbols in all components that make up the PrePi SEC module: >>> this informs the linker that a symbol is never exported or preempted, >>> making it safe to refer to it directly from anywhere in the code, >>> rather than indirectly via a GOT entry. >>> >>> Contributed-under: TianoCore Contribution Agreement 1.1 >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >>> --- >>> ArmVirtPkg/ArmVirtQemuKernel.dsc | 12 ++++++++-- >>> ArmVirtPkg/ArmVirtXen.dsc | 12 ++++++++-- >>> ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf | 2 +- >>> ArmVirtPkg/Include/Platform/Hidden.h | 23 ++++++++++++++++++++ >>> 4 files changed, 44 insertions(+), 5 deletions(-) >>> >>> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc >>> index 9928919bf5b0..c4324a9e264b 100644 >>> --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc >>> +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc >>> @@ -68,11 +68,19 @@ [LibraryClasses.common] >>> [LibraryClasses.common.UEFI_DRIVER] >>> UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf >>> >>> -[BuildOptions.ARM.EDKII.SEC, BuildOptions.ARM.EDKII.BASE] >>> +[BuildOptions.common.EDKII.SEC, BuildOptions.common.EDKII.BASE] >>> # Avoid MOVT/MOVW instruction pairs in code that may end up in the PIE >>> # executable we build for the relocatable PrePi. They are not runtime >>> # relocatable in ELF. >>> - *_CLANG35_*_CC_FLAGS = -mno-movt >>> + *_CLANG35_ARM_CC_FLAGS = -mno-movt >>> + >>> + # >>> + # CLANG38 with LTO support enabled uses the GNU GOLD linker, which insists >>> + # on emitting GOT based symbol references when running in shared mode, unless >>> + # we override visibility to 'hidden' in all modules that make up the PrePi >>> + # build. >>> + # >>> + GCC:*_CLANG38_*_CC_FLAGS = -include $(WORKSPACE)/ArmVirtPkg/Include/Platform/Hidden.h >>> >>> ################################################################################ >>> # >>> diff --git a/ArmVirtPkg/ArmVirtXen.dsc b/ArmVirtPkg/ArmVirtXen.dsc >>> index 20fae9e675bb..e083666f54ea 100644 >>> --- a/ArmVirtPkg/ArmVirtXen.dsc >>> +++ b/ArmVirtPkg/ArmVirtXen.dsc >>> @@ -57,11 +57,19 @@ [LibraryClasses] >>> [LibraryClasses.common.UEFI_DRIVER] >>> UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf >>> >>> -[BuildOptions.ARM.EDKII.SEC, BuildOptions.ARM.EDKII.BASE] >>> +[BuildOptions.common.EDKII.SEC, BuildOptions.common.EDKII.BASE] >>> # Avoid MOVT/MOVW instruction pairs in code that may end up in the PIE >>> # executable we build for the relocatable PrePi. They are not runtime >>> # relocatable in ELF. >>> - *_CLANG35_*_CC_FLAGS = -mno-movt >>> + *_CLANG35_ARM_CC_FLAGS = -mno-movt >>> + >>> + # >>> + # CLANG38 with LTO support enabled uses the GNU GOLD linker, which insists >>> + # on emitting GOT based symbol references when running in shared mode, unless >>> + # we override visibility to 'hidden' in all modules that make up the PrePi >>> + # build. >>> + # >>> + GCC:*_CLANG38_*_CC_FLAGS = -include $(WORKSPACE)/ArmVirtPkg/Include/Platform/Hidden.h >>> >>> ################################################################################ >>> # >>> diff --git a/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf b/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf >>> index 034ddb41cb48..5fe6cd8eb481 100755 >>> --- a/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf >>> +++ b/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf >>> @@ -105,4 +105,4 @@ [Pcd] >>> gArmTokenSpaceGuid.PcdFvBaseAddress >>> >>> [BuildOptions] >>> - GCC:*_*_*_DLINK_FLAGS = -pie -Wl,-T,$(MODULE_DIR)/Scripts/PrePi-PIE.lds >>> + GCC:*_*_*_DLINK_FLAGS = -shared -Wl,-Bsymbolic -Wl,-T,$(MODULE_DIR)/Scripts/PrePi-PIE.lds >>> diff --git a/ArmVirtPkg/Include/Platform/Hidden.h b/ArmVirtPkg/Include/Platform/Hidden.h >>> new file mode 100644 >>> index 000000000000..20e9f5d896b2 >>> --- /dev/null >>> +++ b/ArmVirtPkg/Include/Platform/Hidden.h >>> @@ -0,0 +1,23 @@ >>> +/** @file >>> + >>> + Copyright (c) 2018, Linaro Limited. All rights reserved. >>> + >>> + This program and the accompanying materials >>> + are licensed and made available under the terms and conditions of the BSD License >>> + which accompanies this distribution. The full text of the license may be found at >>> + http://opensource.org/licenses/bsd-license.php >>> + >>> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, >>> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. >>> + >>> +**/ >>> + >> >> I know this isn't intended to be explicitly included, but could we >> still have in include guard? >> > > Sure. > >>> +// >>> +// Setting the GCC -fvisibility=hidden command line option is not quite the same >>> +// as setting the pragma below: the former only affects definitions, whereas the >>> +// latter affects extern declarations as well. So if we want to ensure that no >> >> s/latter/pragma/? (Just saves on recursive natural language parsing >> > > Fair enough. > Acked-by: Laszlo Ersek <lersek@redhat.com> Thanks Laszlo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] ArmVirtPkg: fixes for ARM and CLANG38 builds 2018-12-11 12:19 [PATCH 0/2] ArmVirtPkg: fixes for ARM and CLANG38 builds Ard Biesheuvel 2018-12-11 12:19 ` [PATCH 1/2] ArmVirtPkg/ArmVirtQemuKernel ARM: make some PCD settings apply to ARM Ard Biesheuvel 2018-12-11 12:19 ` [PATCH 2/2] ArmVirtPkg/PrePiUniCoreRelocatable CLANG38: work around build issues Ard Biesheuvel @ 2018-12-11 16:50 ` Ard Biesheuvel 2 siblings, 0 replies; 8+ messages in thread From: Ard Biesheuvel @ 2018-12-11 16:50 UTC (permalink / raw) To: edk2-devel@lists.01.org Cc: Laszlo Ersek, Leif Lindholm, Philippe Mathieu-Daudé On Tue, 11 Dec 2018 at 13:19, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > Patch #1 reshuffles the .dsc contents of ArmVirtQemuKernel so that we > can run on ARM targets as well. > > Patch #2 fixes the CLANG38 build for ArmVirtQemuKernel and ArmVirtXen, > by tweaking the linker options passed to emit the self-relocating PrePi > SEC module. > Thanks all Pushed as de3c440e8a54..e07092edca84 > Ard Biesheuvel (2): > ArmVirtPkg/ArmVirtQemuKernel ARM: make some PCD settings apply to ARM > ArmVirtPkg/PrePiUniCoreRelocatable CLANG38: work around build issues > > ArmVirtPkg/ArmVirtQemuKernel.dsc | 32 +++++++++++-------- > ArmVirtPkg/ArmVirtXen.dsc | 12 +++++-- > .../PrePi/ArmVirtPrePiUniCoreRelocatable.inf | 2 +- > ArmVirtPkg/Include/Platform/Hidden.h | 23 +++++++++++++ > 4 files changed, 53 insertions(+), 16 deletions(-) > create mode 100644 ArmVirtPkg/Include/Platform/Hidden.h > > -- > 2.19.2 > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-12-11 16:50 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-12-11 12:19 [PATCH 0/2] ArmVirtPkg: fixes for ARM and CLANG38 builds Ard Biesheuvel 2018-12-11 12:19 ` [PATCH 1/2] ArmVirtPkg/ArmVirtQemuKernel ARM: make some PCD settings apply to ARM Ard Biesheuvel 2018-12-11 15:59 ` Laszlo Ersek 2018-12-11 12:19 ` [PATCH 2/2] ArmVirtPkg/PrePiUniCoreRelocatable CLANG38: work around build issues Ard Biesheuvel 2018-12-11 12:57 ` Leif Lindholm 2018-12-11 12:58 ` Ard Biesheuvel 2018-12-11 16:10 ` Laszlo Ersek 2018-12-11 16:50 ` [PATCH 0/2] ArmVirtPkg: fixes for ARM and CLANG38 builds Ard Biesheuvel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox