From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ed1-f48.google.com (mail-ed1-f48.google.com [209.85.208.48]) by mx.groups.io with SMTP id smtpd.web11.15952.1621238444575113454 for ; Mon, 17 May 2021 01:00:45 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=pCYYH2NW; spf=pass (domain: linaro.org, ip: 209.85.208.48, mailfrom: etienne.carriere@linaro.org) Received: by mail-ed1-f48.google.com with SMTP id h16so5724682edr.6 for ; Mon, 17 May 2021 01:00:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=ztVyIqDDBlS8Ry8+QC/35BbZ8xS/u03xeugkTSCYWls=; b=pCYYH2NW8VchU5QZ2+5PgEJ4k/erOfIeaOPba6WoFXKJnvbcwYFkM7YNa9nxggLOQp Tdv3zhr9kH7dlWvY9dwO5PI85Nobf74vw+Ng5VPj1mAzqDqkjjPwOA2ZO92vO2ToE5fs t0nv/UrVZj2OiP8vhsD6nw+0oBARHbr6GGCNoQxLDQlKGTyl/XbdZwiIXizhqbogXpoQ eC4qOM5QngF7CvzRYXjXVaUqsswDXmuF7AaSN8mXUY4AQU75DAOsbREX7grVam3yA2fg 7bDd+b1gydcuHMELdpcgeDmGOaN4CTr/RL6X5rp7Mv7whoOCuUfFd48iaztnDU+6QE/H zr7g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=ztVyIqDDBlS8Ry8+QC/35BbZ8xS/u03xeugkTSCYWls=; b=NetSJNsPvV9RSk25j8YOc0eIAw8q5TBcXTNl/O2gWzcYh8KtlQiPU97Z03Qu03IXax C4cijjzADVkWWylqyxIpfD1wu617K2dKzOrviun3WcZtJmLR/aafNw/FEMBfYnl/VKG8 YXqOPnIOlFrldhUPUixDHxJSIVBK4P86dL0Keq8LGxnEA7e8RtOcj524LVxayVHfPuX1 tnJmfgdlvjicEZQCtP+JmLZXhgj01Xt7XwWC6vknw6+whX/C5cx9ENA8FYgve3hBkb69 aH/Tbyab2sp2sdeLxmwcdLn1EqYPLrYfeFHYmrWLYicbXRCuVa8zGCtMNe4yQdX00BVS W5Lg== X-Gm-Message-State: AOAM531fmu4MjBF+1trLaVwYXddub5qIdBnzxb231rfgfA4qwTX5EyMd 9iNPQMIS11xS7f8A8wwJiuvF92I1NN1pvOkpZnT+aw== X-Google-Smtp-Source: ABdhPJyJU8U9t/op+dYoXIakMIFDBGfE2wNe99m82swENfIeYDrHZ45xb/9599dh+zJopAMDLHjORHZ4yFpK5lD+HkY= X-Received: by 2002:aa7:d803:: with SMTP id v3mr7331146edq.150.1621238443051; Mon, 17 May 2021 01:00:43 -0700 (PDT) MIME-Version: 1.0 References: <20210517054911.30665-1-etienne.carriere@linaro.org> <20210517054911.30665-3-etienne.carriere@linaro.org> <006301d74aed$9c7a14f0$d56e3ed0$@byosoft.com.cn> <006901d74af0$f2fa5ee0$d8ef1ca0$@byosoft.com.cn> In-Reply-To: <006901d74af0$f2fa5ee0$d8ef1ca0$@byosoft.com.cn> From: "Etienne Carriere" Date: Mon, 17 May 2021 10:00:32 +0200 Message-ID: Subject: Re: [edk2-devel] [PATCH v2 3/5] GenFv: Arm: support images entered in Thumb mode To: gaoliming Cc: devel@edk2.groups.io, Achin Gupta , Ard Biesheuvel , Jiewen Yao , Leif Lindholm , Sami Mujawar , Sughosh Ganu , Bob Feng Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, 17 May 2021 at 09:48, gaoliming wrote: > > Etienne: > Thanks for your reminder. I try VS compiler and meet with the compiler= error on this line. > > Here, does if ((SecCoreEntryAddress & 1) !=3D 0) mean the lowest bit o= f this address is 1? Yes it does. > > Thanks > Liming > > -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6----- > > =E5=8F=91=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io =E4=BB=A3=E8=A1=A8 Etienne > > Carriere > > =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2021=E5=B9=B45=E6=9C=8817=E6=97= =A5 15:35 > > =E6=94=B6=E4=BB=B6=E4=BA=BA: gaoliming > > =E6=8A=84=E9=80=81: devel@edk2.groups.io; Achin Gupta ; Ard > > Biesheuvel ; Jiewen Yao > > ; Leif Lindholm ; Sami Mujawa= r > > ; Sughosh Ganu ; Bob > > Feng > > =E4=B8=BB=E9=A2=98: Re: [edk2-devel] [PATCH v2 3/5] GenFv: Arm: suppor= t images entered in > > Thumb mode > > > > On Mon, 17 May 2021 at 09:24, gaoliming > > wrote: > > > > > > Acked-by: Liming Gao > > > > > > > -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6----- > > > > =E5=8F=91=E4=BB=B6=E4=BA=BA: Etienne Carriere > > > > =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2021=E5=B9=B45=E6=9C=8817=E6= = =97=A5 13:49 > > > > =E6=94=B6=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io > > > > =E6=8A=84=E9=80=81: Achin Gupta ; Ard Biesheu= vel > > > > ; Jiewen Yao ; Le= if > > > > Lindholm ; Sami Mujawar > > ; > > > > Sughosh Ganu ; Etienne Carriere > > > > ; Bob Feng ; Li= ming > > > > Gao > > > > =E4=B8=BB=E9=A2=98: [PATCH v2 3/5] GenFv: Arm: support images ente= red in Thumb > > mode > > > > > > > > Change GenFv for Arm architecture to generate a specific jump > > > > instruction as image entry instruction, when the target entry labe= l > > > > 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 > > > > Cc: Liming Gao > > > > Cc: Achin Gupta > > > > Cc: Ard Biesheuvel > > > > Cc: Leif Lindholm > > > > Cc: Sughosh Ganu > > > > Signed-off-by: Etienne Carriere > > > > --- > > > > 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 Thum= b mode. > > > > + * From ARM Arch Ref Manual versions b/c/d, section A8.8.25 BL, B= LX > > > > (immediate) > > > > + * BLX (encoding A2) branches to offset in Thumb instruction set = mode. > > > > + * BL (encoding A1) branches to offset in Arm instruction set mod= e. > > > > + */ > > > > +#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 =3D FALSE; > > > > BOOLEAN mRiscV =3D FALSE; > > > > STATIC UINT32 MaxFfsAlignment =3D 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 ju= mping > > > from > > > > the base address, but from base address + 8 > > > > - ResetVector[0] =3D (INT32)(SecCoreEntryAddress - > > > > FvInfo->BaseAddress - 8) >> 2; > > > > + EntryOffset =3D (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] |=3D > > ARMT_UNCONDITIONAL_JUMP_INSTRUCTION; > > > > + if (SecCoreEntryAddress & 1 !=3D 0) { > > > > Sorry, I missed this one. > > This needs extra parantheses. > > > > I'll sent a v3. My apologies... > > > > etienne > > > > > > + ResetVector[0] =3D ARM_JUMP_TO_THUMB(EntryOffset); > > > > + } else { > > > > + ResetVector[0] =3D ARM_JUMP_TO_ARM(EntryOffset); > > > > + } > > > > > > > > // SWI handler movs pc,lr. Just in case a debugger uses S= WI > > > > - ResetVector[2] =3D 0xE1B0F07E; > > > > + ResetVector[2] =3D ARM_RETURN_FROM_EXCEPTION; > > > > > > > > // Place holder to support a common interrupt handler from > > ROM. > > > > // Currently not supported. For this to be used the reset v= ector > > > would > > > > not be in this FV > > > > -- > > > > 2.17.1 > > > > > > > > > > > > > > >=20 > > > > >