From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.120]) by mx.groups.io with SMTP id smtpd.web11.803.1591734912376179463 for ; Tue, 09 Jun 2020 13:35:12 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=g4IsIrz/; spf=pass (domain: redhat.com, ip: 205.139.110.120, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1591734911; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=iYijoFucBCvRndG/bxi8Qw7aZ4QERuwWtRTlOZBfx3s=; b=g4IsIrz/QoyVleCjFZxb8p04I/XwB+eCXMustzgCVwcIpwk+plUQrsfGlNQ/2vDhwXSh06 hgcrYTxL5UK2zvlHf3QIrM1xy7d+gDPzBwT4oZqiDte49Om7o0yJCN8IaSa006XoaptLs6 5sPLEw1skGVGbzwPLptzBMbeBi8+aNk= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-246-mc8_ks3cMWiQCLgfXJOUCg-1; Tue, 09 Jun 2020 16:35:09 -0400 X-MC-Unique: mc8_ks3cMWiQCLgfXJOUCg-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id B19FB18FF660; Tue, 9 Jun 2020 20:35:07 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-112-194.ams2.redhat.com [10.36.112.194]) by smtp.corp.redhat.com (Postfix) with ESMTP id D225F7BFE2; Tue, 9 Jun 2020 20:35:05 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH 3/4] ArmVirtPkg/PrePi: use standard PeCoff routines for self-relocation To: devel@edk2.groups.io, ard.biesheuvel@arm.com Cc: Bob Feng , Liming Gao , Leif Lindholm , Ilias Apalodimas , Julien Grall , Jiewen Yao References: <20200608173413.1100679-1-ard.biesheuvel@arm.com> <20200608173413.1100679-4-ard.biesheuvel@arm.com> From: "Laszlo Ersek" Message-ID: <70e115e9-42d0-0bd2-7fdb-1be19c7af1db@redhat.com> Date: Tue, 9 Jun 2020 22:35:05 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20200608173413.1100679-4-ard.biesheuvel@arm.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit 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 > --- > 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 > #include > > +#include > #include > #include > #include > @@ -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 > > 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 > > 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