On 06/08/20 19:34, Ard Biesheuvel wrote: > Instead of having a GCC specific routine to perform self-relocation > based on ELF metadata, use the PE/COFF metadata and the existing > PeCoff library routines. This reduces the amount of bespoke assembler > code that is a burden to maintain, and is not portable across the set > of toolchains we support. > > This does require some special care, as we have no control over how > the C code references global symbols, so we need to emit these > references from the calling assembler code. Otherwise, they may be > emitted as absolute references, in which case they need to be fixed > up themselves, leading to a circular dependency. > > Signed-off-by: Ard Biesheuvel <[email protected]> > --- > ArmVirtPkg/ArmVirtQemuKernel.dsc | 10 ++-- > ArmVirtPkg/ArmVirtXen.dsc | 10 ++-- > ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf | 4 +- > ArmVirtPkg/PrePi/PrePi.c | 21 +++++++++ > ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S | 49 +++++--------------- > ArmVirtPkg/PrePi/Arm/ModuleEntryPoint.S | 47 +++++-------------- > 6 files changed, 54 insertions(+), 87 deletions(-) > > diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc > b/ArmVirtPkg/ArmVirtQemuKernel.dsc > index 2a6fd6bc06be..9449a01d6e40 100644 > --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc > +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc > @@ -83,14 +83,12 @@ [LibraryClasses.common.DXE_DRIVER] > [LibraryClasses.common.UEFI_DRIVER] > UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf > > -[BuildOptions.common.EDKII.SEC, BuildOptions.common.EDKII.BASE] > +[BuildOptions] > # > - # 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. > + # We need to avoid jump tables in SEC modules, so that the PE/COFF > + # self-relocation code itself is guaranteed to be position independent. > # > - GCC:*_CLANG38_*_CC_FLAGS = -include > $(WORKSPACE)/ArmVirtPkg/Include/Platform/Hidden.h > + GCC:*_*_*_CC_FLAGS = -fno-jump-tables > > > ################################################################################ > # > diff --git a/ArmVirtPkg/ArmVirtXen.dsc b/ArmVirtPkg/ArmVirtXen.dsc > index 8a489b253684..278f5d3828ce 100644 > --- a/ArmVirtPkg/ArmVirtXen.dsc > +++ b/ArmVirtPkg/ArmVirtXen.dsc > @@ -52,14 +52,12 @@ [LibraryClasses] > [LibraryClasses.common.UEFI_DRIVER] > UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf > > -[BuildOptions.common.EDKII.SEC, BuildOptions.common.EDKII.BASE] > +[BuildOptions] > # > - # 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. > + # We need to avoid jump tables in SEC modules, so that the PE/COFF > + # self-relocation code itself is guaranteed to be position independent. > # > - GCC:*_CLANG38_*_CC_FLAGS = -include > $(WORKSPACE)/ArmVirtPkg/Include/Platform/Hidden.h > + GCC:*_*_*_CC_FLAGS = -fno-jump-tables > > > ################################################################################ > # > diff --git a/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf > b/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf > index 9e58e56fce09..7edf5018089d 100755 > --- a/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf > +++ b/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf > @@ -46,6 +46,7 @@ [LibraryClasses] > SerialPortLib > ExtractGuidedSectionLib > LzmaDecompressLib > + PeCoffLib > PrePiLib > MemoryAllocationLib > HobLib > @@ -95,6 +96,3 @@ [Pcd] > gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress > gArmTokenSpaceGuid.PcdFdBaseAddress > gArmTokenSpaceGuid.PcdFvBaseAddress > - > -[BuildOptions] > - GCC:*_*_*_DLINK_FLAGS = > -Wl,-Bsymbolic,-pie,-T,$(MODULE_DIR)/Scripts/PrePi-PIE.lds > diff --git a/ArmVirtPkg/PrePi/PrePi.c b/ArmVirtPkg/PrePi/PrePi.c > index 72e35028c5bb..0cb064419759 100755 > --- a/ArmVirtPkg/PrePi/PrePi.c > +++ b/ArmVirtPkg/PrePi/PrePi.c > @@ -9,6 +9,7 @@ > #include <PiPei.h> > #include <Pi/PiBootMode.h> > > +#include <Library/PeCoffLib.h> > #include <Library/PrePiLib.h> > #include <Library/PrintLib.h> > #include <Library/PrePiHobListPointerLib.h> > @@ -128,3 +129,23 @@ CEntryPoint ( > // DXE Core should always load and never return > ASSERT (FALSE); > } > + > +VOID > +RelocatePeCoffImage ( > + IN UINTN ImageBase, > + IN PE_COFF_LOADER_READ_FILE ImageRead > + ) > +{ > + PE_COFF_LOADER_IMAGE_CONTEXT ImageContext; > + > + ZeroMem (&ImageContext, sizeof ImageContext); > + > + ImageContext.Handle = (EFI_HANDLE)ImageBase; > + ImageContext.ImageRead = ImageRead; > + PeCoffLoaderGetImageInfo (&ImageContext); > + > + if (ImageContext.ImageAddress != ImageBase) { > + ImageContext.ImageAddress = ImageBase; > + PeCoffLoaderRelocateImage (&ImageContext); > + } > +} > diff --git a/ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S > b/ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S > index 34d6abecb0ac..7c5592b11a46 100644 > --- a/ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S > +++ b/ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S > @@ -9,40 +9,6 @@ > #include <AsmMacroIoLibV8.h> > > ASM_FUNC(_ModuleEntryPoint) > - // > - // We are built as a ET_DYN PIE executable, so we need to process all > - // relative relocations regardless of whether or not we are executing from > - // the same offset we were linked at. This is only possible if we are > - // running from RAM. > - // > - adr x8, __reloc_base > - adr x9, __reloc_start > - adr x10, __reloc_end > - > -.Lreloc_loop: > - cmp x9, x10 > - bhs .Lreloc_done > - > - // > - // AArch64 uses the ELF64 RELA format, which means each entry in the > - // relocation table consists of > - // > - // UINT64 offset : the relative offset of the value that needs > to > - // be relocated > - // UINT64 info : relocation type and symbol index (the latter > is > - // not used for R_AARCH64_RELATIVE relocations) > - // UINT64 addend : value to be added to the value being > relocated > - // > - ldp x11, x12, [x9], #24 // read offset into x11 and info into x12 > - cmp x12, #0x403 // check info == R_AARCH64_RELATIVE? > - bne .Lreloc_loop // not a relative relocation? then skip > - > - ldr x12, [x9, #-8] // read addend into x12 > - add x12, x12, x8 // add reloc base to addend to get relocated > value > - str x12, [x11, x8] // write relocated value at offset > - b .Lreloc_loop > -.Lreloc_done: > - > bl ASM_PFX(DiscoverDramFromDt) > > // Get ID of this CPU in Multicore system > @@ -170,15 +136,24 @@ ASM_PFX(DiscoverDramFromDt): > str x1, [x8] > str x7, [x9] > > + // > + // The runtime address may be different from the link time address so fix > + // up the PE/COFF relocations. Since we are calling a C function, use the > + // window at the beginning of the FD image as a temp stack. > + // > + adr x0, ElfImageBase > + adr x1, PeCoffLoaderImageReadFromMemory > + mov sp, x7 > + bl RelocatePeCoffImage > + > // > // Discover the memory size and offset from the DTB, and record in the > // respective PCDs. This will also return false if a corrupt DTB is > - // encountered. Since we are calling a C function, use the window at the > - // beginning of the FD image as a temp stack. > + // encountered. > // > + mov x0, x28 > adr x1, PcdGet64 (PcdSystemMemoryBase) > adr x2, PcdGet64 (PcdSystemMemorySize) > - mov sp, x7 > bl FindMemnode > cbz x0, .Lout > > diff --git a/ArmVirtPkg/PrePi/Arm/ModuleEntryPoint.S > b/ArmVirtPkg/PrePi/Arm/ModuleEntryPoint.S > index 72d756fdb571..bf39de86e7d0 100644 > --- a/ArmVirtPkg/PrePi/Arm/ModuleEntryPoint.S > +++ b/ArmVirtPkg/PrePi/Arm/ModuleEntryPoint.S > @@ -9,38 +9,6 @@ > #include <AsmMacroIoLib.h> > > ASM_FUNC(_ModuleEntryPoint) > - // > - // We are built as a ET_DYN PIE executable, so we need to process all > - // relative relocations if we are executing from a different offset than we > - // were linked at. This is only possible if we are running from RAM. > - // > - ADRL (r4, __reloc_base) > - ADRL (r5, __reloc_start) > - ADRL (r6, __reloc_end) > - > -.Lreloc_loop: > - cmp r5, r6 > - bhs .Lreloc_done > - > - // > - // AArch32 uses the ELF32 REL format, which means each entry in the > - // relocation table consists of > - // > - // UINT32 offset : the relative offset of the value that needs > to > - // be relocated > - // UINT32 info : relocation type and symbol index (the latter > is > - // not used for R_ARM_RELATIVE relocations) > - // > - ldrd r8, r9, [r5], #8 // read offset into r8 and info into r9 > - cmp r9, #23 // check info == R_ARM_RELATIVE? > - bne .Lreloc_loop // not a relative relocation? then skip > - > - ldr r9, [r8, r4] // read addend into r9 > - add r9, r9, r1 // add image base to addend to get relocated > value > - str r9, [r8, r4] // write relocated value at offset > - b .Lreloc_loop > -.Lreloc_done: > - > // Do early platform specific actions > bl ASM_PFX(ArmPlatformPeiBootAction) > > @@ -172,15 +140,24 @@ ASM_PFX(ArmPlatformPeiBootAction): > str r1, [r8] > str r5, [r7] > > + // > + // The runtime address may be different from the link time address so fix > + // up the PE/COFF relocations. Since we are calling a C function, use the > + // window at the beginning of the FD image as a temp stack. > + // > + ADRL (r0, ElfImageBase) > + ADRL (r1, PeCoffLoaderImageReadFromMemory) > + mov sp, r5 > + bl RelocatePeCoffImage > + > // > // Discover the memory size and offset from the DTB, and record in the > // respective PCDs. This will also return false if a corrupt DTB is > - // encountered. Since we are calling a C function, use the window at the > - // beginning of the FD image as a temp stack. > + // encountered. > // > + mov r0, r10 > ADRL (r1, PcdGet64 (PcdSystemMemoryBase)) > ADRL (r2, PcdGet64 (PcdSystemMemorySize)) > - mov sp, r5 > bl FindMemnode > teq r0, #0 > beq .Lout >
Acked-by: Laszlo Ersek <[email protected]> -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#60986): https://edk2.groups.io/g/devel/message/60986 Mute This Topic: https://groups.io/mt/74757213/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
