Re: [PATCH v2 3/5] GenFv: Arm: support images entered in Thumb mode


Etienne Carriere
 

On Mon, 17 May 2021 at 09:24, gaoliming <gaoliming@byosoft.com.cn> wrote:

Acked-by: Liming Gao <gaoliming@byosoft.com.cn>

-----邮件原件-----
发件人: Etienne Carriere <etienne.carriere@linaro.org>
发送时间: 2021年5月17日 13:49
收件人: devel@edk2.groups.io
抄送: Achin Gupta <achin.gupta@arm.com>; Ard Biesheuvel
<ardb+tianocore@kernel.org>; Jiewen Yao <jiewen.yao@intel.com>; Leif
Lindholm <leif@nuviainc.com>; Sami Mujawar <sami.mujawar@arm.com>;
Sughosh Ganu <sughosh.ganu@linaro.org>; Etienne Carriere
<etienne.carriere@linaro.org>; Bob Feng <bob.c.feng@intel.com>; Liming
Gao <gaoliming@byosoft.com.cn>
主题: [PATCH v2 3/5] GenFv: 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 has its LSBit set when
the entry instruction executes in Thumb mode.

Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Achin Gupta <achin.gupta@arm.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Sughosh Ganu <sughosh.ganu@linaro.org>
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
Changes since v1:
- Fix typos in commit log and inline comments
- Change if() test operand to be an explicit boolean
---
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..5f3fd4f808 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
0xEB000000
#define ARM64_UNCONDITIONAL_JUMP_INSTRUCTION
0x14000000

+/*
+ * Arm instruction to jump to Fv entry 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 0xffffff
+#define ARM_JUMP_TO_ARM(Offset) (0xeb000000 | ((Offset - 8) >> 2))
+
+#define _ARM_JUMP_TO_THUMB(Imm32) (0xfa000000 | \
+ (((Imm32) & (1 << 1)) << (24 - 1))
| \
+ (((Imm32) >> 2) & 0x7fffff))
+#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) {
+ UINT32 EntryOffset;

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] > 0x00FFFFFF) {
- 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 != 0) {
Sorry, I missed this one.
This needs extra parantheses.

I'll sent a v3. My apologies...

etienne

+ 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

Join devel@edk2.groups.io to automatically receive all group messages.