> On Sep 24, 2015, at 6:03 PM, Ard Biesheuvel <ard.biesheu...@linaro.org> wrote: > > (adding Leif and Andrew, who merged this code originally) >
Does this pattern exist in the BaseTools version for the PeCoffLoaderRelocateImageEx() path? I think we just copied the pattern from the IPF. Thanks, Andrew Fish > On 23 September 2015 at 21:31, Ard Biesheuvel <ard.biesheu...@linaro.org > <mailto:ard.biesheu...@linaro.org>> wrote: >> The base relocation type EFI_IMAGE_REL_BASED_ARM_MOV32T patches an >> absolute address into the immediate fields of an adjacent movt/movw >> instruction pair. >> >> As the instructions are not writable by the program itself, there is >> no need to keep track of the fixup data, since we can reapply the >> relocation for runtime unconditionally. So remove the fixup handling >> for this relocation type. >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org> >> --- >> MdePkg/Library/BasePeCoffLib/Arm/PeCoffLoaderEx.c | 14 ++------------ >> 1 file changed, 2 insertions(+), 12 deletions(-) >> >> diff --git a/MdePkg/Library/BasePeCoffLib/Arm/PeCoffLoaderEx.c >> b/MdePkg/Library/BasePeCoffLib/Arm/PeCoffLoaderEx.c >> index d6bf42738d2b..03a532c60514 100644 >> --- a/MdePkg/Library/BasePeCoffLib/Arm/PeCoffLoaderEx.c >> +++ b/MdePkg/Library/BasePeCoffLib/Arm/PeCoffLoaderEx.c >> @@ -158,13 +158,6 @@ PeCoffLoaderRelocateImageEx ( >> case EFI_IMAGE_REL_BASED_ARM_MOV32T: >> FixupVal = ThumbMovwMovtImmediateAddress (Fixup16) + (UINT32)Adjust; >> ThumbMovwMovtImmediatePatch (Fixup16, FixupVal); >> - >> - if (*FixupData != NULL) { >> - *FixupData = ALIGN_POINTER(*FixupData, sizeof(UINT64)); >> - // Fixup16 is not aligned so we must copy it. Thumb instructions are >> streams of 16 bytes. >> - CopyMem (*FixupData, Fixup16, sizeof (UINT64)); >> - *FixupData = *FixupData + sizeof(UINT64); >> - } >> break; >> >> case EFI_IMAGE_REL_BASED_ARM_MOV32A: >> @@ -229,11 +222,8 @@ PeHotRelocateImageEx ( >> switch ((*Reloc) >> 12) { >> >> case EFI_IMAGE_REL_BASED_ARM_MOV32T: >> - *FixupData = ALIGN_POINTER (*FixupData, sizeof (UINT64)); >> - if (*(UINT64 *) (*FixupData) == ReadUnaligned64 ((UINT64 *)Fixup16)) { >> - FixupVal = ThumbMovwMovtImmediateAddress (Fixup16) + (UINT32)Adjust; >> - ThumbMovwMovtImmediatePatch (Fixup16, FixupVal); >> - } >> + FixupVal = ThumbMovwMovtImmediateAddress (Fixup16) + (UINT32)Adjust; >> + ThumbMovwMovtImmediatePatch (Fixup16, FixupVal); >> break; >> >> case EFI_IMAGE_REL_BASED_ARM_MOV32A: > > To elaborate a bit, this patch actually fixes a bug on ARM: since > PeHotRelocateImageEx () fails to advance the FixupData pointer, it > will think the targets of these relocations have been updated by the > program since the first relocation occurred, and leave them alone > instead of relocating them to the OS VA mapping. > > If people prefer, I could also fix this by adding the increment of > *FixupData in the correct place, but since the whole check is a bit > pointless, I preferred to just rip it out. > > -- > Ard. > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org <mailto:edk2-devel@lists.01.org> > https://lists.01.org/mailman/listinfo/edk2-devel > <https://lists.01.org/mailman/listinfo/edk2-devel> _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
