Re: [edk2-devel] [PATCH 3/5] GenGv: Arm: support images entered in Thumb mode
Hi Etienne, Thank you for this patch. Please find my comments below marked [SAMI]. Regards, Sami Mujawar On 04/05/2021 04:20 PM, Etienne Carriere wrote: Change GenFv for Arm architecture to generate a specific jump instruction as image entry instruction, when the target entry label is assembled with Thumb instruction set. This is possible since SecCoreEntryAddress value fetched from the PE32 as its LSBit set when the entry instruction executes in Thumb mode. Cc: Bob Feng Cc: Liming Gao Cc: Achin Gupta Cc: Ard Biesheuvel Cc: Leif Lindholm Cc: Sughosh Ganu Signed-off-by: Etienne Carriere --- BaseTools/Source/C/GenFv/GenFvInternalLib.c | 38 +++- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/BaseTools/Source/C/GenFv/GenFvInternalLib.c b/BaseTools/Source/C/GenFv/GenFvInternalLib.c index 6e296b8ad6..3af65146f6 100644 --- a/BaseTools/Source/C/GenFv/GenFvInternalLib.c +++ b/BaseTools/Source/C/GenFv/GenFvInternalLib.c @@ -34,9 +34,27 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include "FvLib.h" #include "PeCoffLib.h" -#define ARMT_UNCONDITIONAL_JUMP_INSTRUCTION 0xEB00 #define ARM64_UNCONDITIONAL_JUMP_INSTRUCTION 0x1400 +/* + * Arm instruction to jump to Fv enry instruction in Arm or Thumb mode. + * From ARM Arch Ref Manual versions b/c/d, section A8.8.25 BL, BLX (immediate) + * BLX (encoding A2) branches to offset in Thumb instruction set mode. + * BL (encoding A1) branches to offset in Arm instruction set mode. + */ +#define ARM_JUMP_OFFSET_MAX0xff +#define ARM_JUMP_TO_ARM(Offset)(0xeb00 | ((Offset - 8) >> 2)) + +#define _ARM_JUMP_TO_THUMB(Imm32) (0xfa00 | \ +(((Imm32) & (1 << 1)) << (24 - 1)) | \ +(((Imm32) >> 2) & 0x7f)) +#define ARM_JUMP_TO_THUMB(Offset) _ARM_JUMP_TO_THUMB((Offset) - 8) + +/* + * Arm instruction to retrun from exception (MOVS PC, LR) + */ +#define ARM_RETURN_FROM_EXCEPTION 0xE1B0F07E + BOOLEAN mArm = FALSE; BOOLEAN mRiscV = FALSE; STATIC UINT32 MaxFfsAlignment = 0; @@ -2203,23 +2221,25 @@ Returns: // if we found an SEC core entry point then generate a branch instruction // to it and populate a debugger SWI entry as well if (UpdateVectorSec) { + UINT32EntryOffset; VerboseMsg("UpdateArmResetVectorIfNeeded updating ARM SEC vector"); - // B SecEntryPoint - signed_immed_24 part +/-32MB offset - // on ARM, the PC is always 8 ahead, so we're not really jumping from the base address, but from base address + 8 - ResetVector[0] = (INT32)(SecCoreEntryAddress - FvInfo->BaseAddress - 8) >> 2; + EntryOffset = (INT32)(SecCoreEntryAddress - FvInfo->BaseAddress); - if (ResetVector[0] > 0x00FF) { -Error(NULL, 0, 3000, "Invalid", "SEC Entry point must be within 32MB of the start of the FV"); + if (EntryOffset > ARM_JUMP_OFFSET_MAX) { + Error(NULL, 0, 3000, "Invalid", "SEC Entry point offset above 1MB of the start of the FV"); return EFI_ABORTED; } - // Add opcode for an unconditional branch with no link. i.e.: " B SecEntryPoint" - ResetVector[0] |= ARMT_UNCONDITIONAL_JUMP_INSTRUCTION; + if (SecCoreEntryAddress & 1) { [SAMI] if condition should evaluate a Boolean value. See https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/5_source_files/57_c_programming#5-7-2-1-boolean-values-variable-type-boolean-do-not-require-explicit-comparisons-to-true-or-false With that changed. [/SAMI] +ResetVector[0] = ARM_JUMP_TO_THUMB(EntryOffset); + } else { +ResetVector[0] = ARM_JUMP_TO_ARM(EntryOffset); + } // SWI handler movs pc,lr. Just in case a debugger uses SWI - ResetVector[2] = 0xE1B0F07E; + ResetVector[2] = ARM_RETURN_FROM_EXCEPTION; // Place holder to support a common interrupt handler from ROM. // Currently not supported. For this to be used the reset vector would not be in this FV -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#75030): https://edk2.groups.io/g/devel/message/75030 Mute This Topic: https://groups.io/mt/82580247/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 3/5] GenGv: Arm: support images entered in Thumb mode
On Tue, 4 May 2021 at 17:20, Etienne Carriere wrote: > > Change GenFv for Arm architecture to generate a specific jump > instruction as image entry instruction, when the target entry label > is assembled with Thumb instruction set. This is possible since > SecCoreEntryAddress value fetched from the PE32 as its LSBit set when > the entry instruction executes in Thumb mode. > > Cc: Bob Feng > Cc: Liming Gao > Cc: Achin Gupta > Cc: Ard Biesheuvel > Cc: Leif Lindholm > Cc: Sughosh Ganu > Signed-off-by: Etienne Carriere This looks fine to me (modulo a couple of typos: GenGv, enry) but this needs an ack from the BaseTools maintainers. Bob, Liming? > --- > BaseTools/Source/C/GenFv/GenFvInternalLib.c | 38 +++- > 1 file changed, 29 insertions(+), 9 deletions(-) > > diff --git a/BaseTools/Source/C/GenFv/GenFvInternalLib.c > b/BaseTools/Source/C/GenFv/GenFvInternalLib.c > index 6e296b8ad6..3af65146f6 100644 > --- a/BaseTools/Source/C/GenFv/GenFvInternalLib.c > +++ b/BaseTools/Source/C/GenFv/GenFvInternalLib.c > @@ -34,9 +34,27 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > #include "FvLib.h" > #include "PeCoffLib.h" > > -#define ARMT_UNCONDITIONAL_JUMP_INSTRUCTION 0xEB00 > #define ARM64_UNCONDITIONAL_JUMP_INSTRUCTION 0x1400 > > +/* > + * Arm instruction to jump to Fv enry instruction in Arm or Thumb mode. > + * From ARM Arch Ref Manual versions b/c/d, section A8.8.25 BL, BLX > (immediate) > + * BLX (encoding A2) branches to offset in Thumb instruction set mode. > + * BL (encoding A1) branches to offset in Arm instruction set mode. > + */ > +#define ARM_JUMP_OFFSET_MAX0xff > +#define ARM_JUMP_TO_ARM(Offset)(0xeb00 | ((Offset - 8) >> 2)) > + > +#define _ARM_JUMP_TO_THUMB(Imm32) (0xfa00 | \ > +(((Imm32) & (1 << 1)) << (24 - 1)) | \ > +(((Imm32) >> 2) & 0x7f)) > +#define ARM_JUMP_TO_THUMB(Offset) _ARM_JUMP_TO_THUMB((Offset) - 8) > + > +/* > + * Arm instruction to retrun from exception (MOVS PC, LR) > + */ > +#define ARM_RETURN_FROM_EXCEPTION 0xE1B0F07E > + > BOOLEAN mArm = FALSE; > BOOLEAN mRiscV = FALSE; > STATIC UINT32 MaxFfsAlignment = 0; > @@ -2203,23 +2221,25 @@ Returns: > // if we found an SEC core entry point then generate a branch instruction > // to it and populate a debugger SWI entry as well > if (UpdateVectorSec) { > + UINT32EntryOffset; > >VerboseMsg("UpdateArmResetVectorIfNeeded updating ARM SEC vector"); > > - // B SecEntryPoint - signed_immed_24 part +/-32MB offset > - // on ARM, the PC is always 8 ahead, so we're not really jumping from > the base address, but from base address + 8 > - ResetVector[0] = (INT32)(SecCoreEntryAddress - FvInfo->BaseAddress - > 8) >> 2; > + EntryOffset = (INT32)(SecCoreEntryAddress - FvInfo->BaseAddress); > > - if (ResetVector[0] > 0x00FF) { > -Error(NULL, 0, 3000, "Invalid", "SEC Entry point must be within 32MB > of the start of the FV"); > + if (EntryOffset > ARM_JUMP_OFFSET_MAX) { > + Error(NULL, 0, 3000, "Invalid", "SEC Entry point offset above 1MB > of the start of the FV"); > return EFI_ABORTED; >} > > - // Add opcode for an unconditional branch with no link. i.e.: " B > SecEntryPoint" > - ResetVector[0] |= ARMT_UNCONDITIONAL_JUMP_INSTRUCTION; > + if (SecCoreEntryAddress & 1) { > +ResetVector[0] = ARM_JUMP_TO_THUMB(EntryOffset); > + } else { > +ResetVector[0] = ARM_JUMP_TO_ARM(EntryOffset); > + } > >// SWI handler movs pc,lr. Just in case a debugger uses SWI > - ResetVector[2] = 0xE1B0F07E; > + ResetVector[2] = ARM_RETURN_FROM_EXCEPTION; > >// Place holder to support a common interrupt handler from ROM. >// Currently not supported. For this to be used the reset vector would > not be in this FV > -- > 2.17.1 > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#74882): https://edk2.groups.io/g/devel/message/74882 Mute This Topic: https://groups.io/mt/82580247/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 3/5] GenGv: Arm: support images entered in Thumb mode
Change GenFv for Arm architecture to generate a specific jump instruction as image entry instruction, when the target entry label is assembled with Thumb instruction set. This is possible since SecCoreEntryAddress value fetched from the PE32 as its LSBit set when the entry instruction executes in Thumb mode. Cc: Bob Feng Cc: Liming Gao Cc: Achin Gupta Cc: Ard Biesheuvel Cc: Leif Lindholm Cc: Sughosh Ganu Signed-off-by: Etienne Carriere --- BaseTools/Source/C/GenFv/GenFvInternalLib.c | 38 +++- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/BaseTools/Source/C/GenFv/GenFvInternalLib.c b/BaseTools/Source/C/GenFv/GenFvInternalLib.c index 6e296b8ad6..3af65146f6 100644 --- a/BaseTools/Source/C/GenFv/GenFvInternalLib.c +++ b/BaseTools/Source/C/GenFv/GenFvInternalLib.c @@ -34,9 +34,27 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include "FvLib.h" #include "PeCoffLib.h" -#define ARMT_UNCONDITIONAL_JUMP_INSTRUCTION 0xEB00 #define ARM64_UNCONDITIONAL_JUMP_INSTRUCTION 0x1400 +/* + * Arm instruction to jump to Fv enry instruction in Arm or Thumb mode. + * From ARM Arch Ref Manual versions b/c/d, section A8.8.25 BL, BLX (immediate) + * BLX (encoding A2) branches to offset in Thumb instruction set mode. + * BL (encoding A1) branches to offset in Arm instruction set mode. + */ +#define ARM_JUMP_OFFSET_MAX0xff +#define ARM_JUMP_TO_ARM(Offset)(0xeb00 | ((Offset - 8) >> 2)) + +#define _ARM_JUMP_TO_THUMB(Imm32) (0xfa00 | \ +(((Imm32) & (1 << 1)) << (24 - 1)) | \ +(((Imm32) >> 2) & 0x7f)) +#define ARM_JUMP_TO_THUMB(Offset) _ARM_JUMP_TO_THUMB((Offset) - 8) + +/* + * Arm instruction to retrun from exception (MOVS PC, LR) + */ +#define ARM_RETURN_FROM_EXCEPTION 0xE1B0F07E + BOOLEAN mArm = FALSE; BOOLEAN mRiscV = FALSE; STATIC UINT32 MaxFfsAlignment = 0; @@ -2203,23 +2221,25 @@ Returns: // if we found an SEC core entry point then generate a branch instruction // to it and populate a debugger SWI entry as well if (UpdateVectorSec) { + UINT32EntryOffset; VerboseMsg("UpdateArmResetVectorIfNeeded updating ARM SEC vector"); - // B SecEntryPoint - signed_immed_24 part +/-32MB offset - // on ARM, the PC is always 8 ahead, so we're not really jumping from the base address, but from base address + 8 - ResetVector[0] = (INT32)(SecCoreEntryAddress - FvInfo->BaseAddress - 8) >> 2; + EntryOffset = (INT32)(SecCoreEntryAddress - FvInfo->BaseAddress); - if (ResetVector[0] > 0x00FF) { -Error(NULL, 0, 3000, "Invalid", "SEC Entry point must be within 32MB of the start of the FV"); + if (EntryOffset > ARM_JUMP_OFFSET_MAX) { + Error(NULL, 0, 3000, "Invalid", "SEC Entry point offset above 1MB of the start of the FV"); return EFI_ABORTED; } - // Add opcode for an unconditional branch with no link. i.e.: " B SecEntryPoint" - ResetVector[0] |= ARMT_UNCONDITIONAL_JUMP_INSTRUCTION; + if (SecCoreEntryAddress & 1) { +ResetVector[0] = ARM_JUMP_TO_THUMB(EntryOffset); + } else { +ResetVector[0] = ARM_JUMP_TO_ARM(EntryOffset); + } // SWI handler movs pc,lr. Just in case a debugger uses SWI - ResetVector[2] = 0xE1B0F07E; + ResetVector[2] = ARM_RETURN_FROM_EXCEPTION; // Place holder to support a common interrupt handler from ROM. // Currently not supported. For this to be used the reset vector would not be in this FV -- 2.17.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#74737): https://edk2.groups.io/g/devel/message/74737 Mute This Topic: https://groups.io/mt/82580247/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 3/5] GenGv: Arm: support images entered in Thumb mode
Change GenFv for Arm architecture to generate a specific jump instruction as image entry instruction when the target entry label is assembled with Thumb instruction set. This is possible since SecCoreEntryAddress value fetched from the PE32 as its LSBit set when the entry instruction executes in Thumb mode. Signed-off-by: Etienne Carriere --- BaseTools/Source/C/GenFv/GenFvInternalLib.c | 39 - 1 file changed, 30 insertions(+), 9 deletions(-) diff --git a/BaseTools/Source/C/GenFv/GenFvInternalLib.c b/BaseTools/Source/C/GenFv/GenFvInternalLib.c index 6e296b8ad6..1b12cb9165 100644 --- a/BaseTools/Source/C/GenFv/GenFvInternalLib.c +++ b/BaseTools/Source/C/GenFv/GenFvInternalLib.c @@ -34,9 +34,28 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include "FvLib.h" #include "PeCoffLib.h" -#define ARMT_UNCONDITIONAL_JUMP_INSTRUCTION 0xEB00 #define ARM64_UNCONDITIONAL_JUMP_INSTRUCTION 0x1400 +// +// Arm instruction to jump to Fv enry instruction in Arm or Thumb mode. +// From ARM Arch Ref Manual versions b/c/d, section A8.8.25 BL, BLX (immediate) +// BLX (encoding A2) branches to offset in Thumb instruction set mode. +// BL (encoding A1) branches to offset in Arm instruction set mode. +// +#define ARM_JUMP_OFFSET_MAX 0xff +#define ARM_JUMP_TO_ARM(Offset) (0xeb00 | ((Offset - 8) >> 2)) + +#define _ARM_JUMP_TO_THUMB(Imm32) (0xfa00 | \ + (((Imm32) & (1 << 1)) << (24 - 1)) | \ + (((Imm32) >> 2) & 0x7f)) +#define ARM_JUMP_TO_THUMB(Offset) _ARM_JUMP_TO_THUMB((Offset) - 8) + +// +// Arm instruction to retrun from exception (MOVS PC, LR) +// +#define ARM_RETURN_FROM_EXCEPTION 0xE1B0F07E + + BOOLEAN mArm = FALSE; BOOLEAN mRiscV = FALSE; STATIC UINT32 MaxFfsAlignment = 0; @@ -2203,23 +,25 @@ Returns: // if we found an SEC core entry point then generate a branch instruction // to it and populate a debugger SWI entry as well if (UpdateVectorSec) { + UINT32EntryOffset; VerboseMsg("UpdateArmResetVectorIfNeeded updating ARM SEC vector"); - // B SecEntryPoint - signed_immed_24 part +/-32MB offset - // on ARM, the PC is always 8 ahead, so we're not really jumping from the base address, but from base address + 8 - ResetVector[0] = (INT32)(SecCoreEntryAddress - FvInfo->BaseAddress - 8) >> 2; + EntryOffset = (INT32)(SecCoreEntryAddress - FvInfo->BaseAddress); - if (ResetVector[0] > 0x00FF) { -Error(NULL, 0, 3000, "Invalid", "SEC Entry point must be within 32MB of the start of the FV"); + if (EntryOffset > ARM_JUMP_OFFSET_MAX) { + Error(NULL, 0, 3000, "Invalid", "SEC Entry point offset above 1MB of the start of the FV"); return EFI_ABORTED; } - // Add opcode for an unconditional branch with no link. i.e.: " B SecEntryPoint" - ResetVector[0] |= ARMT_UNCONDITIONAL_JUMP_INSTRUCTION; + if (SecCoreEntryAddress & 1) { +ResetVector[0] = ARM_JUMP_TO_THUMB(EntryOffset); + } else { +ResetVector[0] = ARM_JUMP_TO_ARM(EntryOffset); + } // SWI handler movs pc,lr. Just in case a debugger uses SWI - ResetVector[2] = 0xE1B0F07E; + ResetVector[2] = ARM_RETURN_FROM_EXCEPTION; // Place holder to support a common interrupt handler from ROM. // Currently not supported. For this to be used the reset vector would not be in this FV -- 2.17.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#72836): https://edk2.groups.io/g/devel/message/72836 Mute This Topic: https://groups.io/mt/81361293/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-