From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-x22f.google.com (mail-wm0-x22f.google.com [IPv6:2a00:1450:400c:c09::22f]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 690601A1E00 for ; Fri, 26 Aug 2016 05:28:49 -0700 (PDT) Received: by mail-wm0-x22f.google.com with SMTP id o80so116945971wme.1 for ; Fri, 26 Aug 2016 05:28:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=qO9Y8viE2be7ft8rIQ3gFpYycypiLsgWbg+zxUdgggM=; b=K6iCc8HEVYEu1xI7XThJx0o4rKSQonpJ9UfVCjvsReshBHdNjNJ/tDQbUtPQBlcvPS YdGeg46HElqw5rzVGesF8wyqmb3PMbrZ62VgumtEPwxF7uVzEqBB0jpReicJCWtf65xs rRHKRqENs9Khgx4CL5lQeQ/SmzciG2rw4oVdg= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=qO9Y8viE2be7ft8rIQ3gFpYycypiLsgWbg+zxUdgggM=; b=H1qc247RIUcYdrm9qCKPSFDPUWj24TmL45lDD963JawwUSkwJ9X/csme6Y2HkI9Tq2 G+C2KutppI1BkvoWVbV229xLh/S6yZ+glLMy4NjEDF3CHPShWjK5vahMOOFdBmYr1Qf0 61Jm2xyMwpTEtSzZcJr3vmauKKJerIotvF4Z3q0Gx+Vzl1LnQ5/6OH7UQmMmZFsqr/Or ZthyJSWI3StqWwltnPN4QEFC/VcjE7IwbtpRlSiR/+fM3SmldYipzX972EAHpsU8lAf7 sjrF85ImCj6bjnrXCrWA23Qh4ZFsTNNfyS2xL4oc/Y8vdTzzp5d1Hw80ge4dml72n3iK ZPuw== X-Gm-Message-State: AE9vXwNpmM6kIbGEOAtxZjVrRUxWZf3cimebVUVyHTt2Vud1ue8sNBs8FpeYUlgUQ7nr5ZEc X-Received: by 10.28.174.76 with SMTP id x73mr3674690wme.60.1472214527765; Fri, 26 Aug 2016 05:28:47 -0700 (PDT) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id o142sm43565648wme.20.2016.08.26.05.28.46 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 26 Aug 2016 05:28:46 -0700 (PDT) Date: Fri, 26 Aug 2016 13:28:45 +0100 From: Leif Lindholm To: Ard Biesheuvel Cc: edk2-devel@lists.01.org, jbrasen@codeaurora.org, feng.tian@intel.com, star.zeng@intel.com, daniil.egranov@arm.com Message-ID: <20160826122845.GE4715@bivouac.eciton.net> References: <1471445945-19239-1-git-send-email-ard.biesheuvel@linaro.org> <1471445945-19239-3-git-send-email-ard.biesheuvel@linaro.org> MIME-Version: 1.0 In-Reply-To: <1471445945-19239-3-git-send-email-ard.biesheuvel@linaro.org> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [PATCH v3 2/4] MdeModulePkg/EbcDxe AARCH64: use a fixed size thunk structure X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 26 Aug 2016 12:28:50 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Wed, Aug 17, 2016 at 04:59:03PM +0200, Ard Biesheuvel wrote: > The thunk generation is needlessly complex, given that it attempts to > deal with variable length instructions, which don't exist on AArch64. > > So replace it with a simple template coded in assembler, with a matching > struct definition in C. That way, we can create and manipulate the thunks > easily without looping over the instructions looking for 'magic' numbers. > > Also, use x16 rather than x9, since it is the architectural register to > use for thunks/veneers. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel Reviewed-by: Leif Lindholm > --- > MdeModulePkg/Universal/EbcDxe/AArch64/EbcLowLevel.S | 32 ++++- > MdeModulePkg/Universal/EbcDxe/AArch64/EbcSupport.c | 133 +++++--------------- > 2 files changed, 58 insertions(+), 107 deletions(-) > > diff --git a/MdeModulePkg/Universal/EbcDxe/AArch64/EbcLowLevel.S b/MdeModulePkg/Universal/EbcDxe/AArch64/EbcLowLevel.S > index a9678432d549..cb7a70b5a4f8 100644 > --- a/MdeModulePkg/Universal/EbcDxe/AArch64/EbcLowLevel.S > +++ b/MdeModulePkg/Universal/EbcDxe/AArch64/EbcLowLevel.S > @@ -3,8 +3,10 @@ > // This code provides low level routines that support the Virtual Machine > // for option ROMs. > // > -// Copyright (c) 2015, The Linux Foundation. All rights reserved. > +// Copyright (c) 2016, Linaro, Ltd. All rights reserved.
> +// Copyright (c) 2015, The Linux Foundation. All rights reserved.
> // Copyright (c) 2007 - 2014, Intel Corporation. All rights reserved.
> +// > // This program and the accompanying materials > // are licensed and made available under the terms and conditions of the BSD License > // which accompanies this distribution. The full text of the license may be found at > @@ -19,6 +21,8 @@ ASM_GLOBAL ASM_PFX(EbcLLCALLEXNative) > ASM_GLOBAL ASM_PFX(EbcLLEbcInterpret) > ASM_GLOBAL ASM_PFX(EbcLLExecuteEbcImageEntryPoint) > > +ASM_GLOBAL ASM_PFX(mEbcInstructionBufferTemplate) > + > //**************************************************************************** > // EbcLLCALLEX > // > @@ -61,7 +65,7 @@ ASM_PFX(EbcLLCALLEXNative): > // > // This function is called by the thunk code to handle an Native to EBC call > // This can handle up to 16 arguments (1-8 on in x0-x7, 9-16 are on the stack) > -// x9 contains the Entry point that will be the first argument when > +// x16 contains the Entry point that will be the first argument when > // EBCInterpret is called. > // > //**************************************************************************** > @@ -97,7 +101,7 @@ ASM_PFX(EbcLLEbcInterpret): > mov x3, x2 > mov x2, x1 > mov x1, x0 > - mov x0, x9 > + mov x0, x16 > > // call C-code > bl ASM_PFX(EbcInterpret) > @@ -111,7 +115,7 @@ ASM_PFX(EbcLLEbcInterpret): > // EbcLLExecuteEbcImageEntryPoint > // > // This function is called by the thunk code to handle the image entry point > -// x9 contains the Entry point that will be the first argument when > +// x16 contains the Entry point that will be the third argument when > // ExecuteEbcImageEntryPoint is called. > // > //**************************************************************************** > @@ -120,9 +124,27 @@ ASM_PFX(EbcLLExecuteEbcImageEntryPoint): > // build new paramater calling convention > mov x2, x1 > mov x1, x0 > - mov x0, x9 > + mov x0, x16 > > // call C-code > bl ASM_PFX(ExecuteEbcImageEntryPoint) > ldp x29, x30, [sp], #16 > ret > + > +//**************************************************************************** > +// mEbcInstructionBufferTemplate > +//**************************************************************************** > + .section ".rodata", "a" > + .align 3 > +ASM_PFX(mEbcInstructionBufferTemplate): > + adr x17, 0f > + ldp x16, x17, [x17] > + br x17 > + > + // > + // Add a magic code here to help the VM recognize the thunk. > + // > + hlt #0xEBC > + > +0: .quad 0 // EBC_ENTRYPOINT_SIGNATURE > + .quad 0 // EBC_LL_EBC_ENTRYPOINT_SIGNATURE > diff --git a/MdeModulePkg/Universal/EbcDxe/AArch64/EbcSupport.c b/MdeModulePkg/Universal/EbcDxe/AArch64/EbcSupport.c > index 23261a070143..a5f21f400274 100644 > --- a/MdeModulePkg/Universal/EbcDxe/AArch64/EbcSupport.c > +++ b/MdeModulePkg/Universal/EbcDxe/AArch64/EbcSupport.c > @@ -2,8 +2,10 @@ > This module contains EBC support routines that are customized based on > the target AArch64 processor. > > -Copyright (c) 2015, The Linux Foundation. All rights reserved. > +Copyright (c) 2016, Linaro, Ltd. All rights reserved.
> +Copyright (c) 2015, The Linux Foundation. All rights reserved.
> Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.
> + > This program and the accompanying materials > are licensed and made available under the terms and conditions of the BSD License > which accompanies this distribution. The full text of the license may be found at > @@ -22,47 +24,16 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > // > #define STACK_REMAIN_SIZE (1024 * 4) > > -// > -// This is instruction buffer used to create EBC thunk > -// > -#define EBC_MAGIC_SIGNATURE 0xCA112EBCCA112EBCull > -#define EBC_ENTRYPOINT_SIGNATURE 0xAFAFAFAFAFAFAFAFull > -#define EBC_LL_EBC_ENTRYPOINT_SIGNATURE 0xFAFAFAFAFAFAFAFAull > -UINT8 mInstructionBufferTemplate[] = { > - 0x03, 0x00, 0x00, 0x14, //b pc+16 > - // > - // Add a magic code here to help the VM recognize the thunk.. > - // > - (UINT8)(EBC_MAGIC_SIGNATURE & 0xFF), > - (UINT8)((EBC_MAGIC_SIGNATURE >> 8) & 0xFF), > - (UINT8)((EBC_MAGIC_SIGNATURE >> 16) & 0xFF), > - (UINT8)((EBC_MAGIC_SIGNATURE >> 24) & 0xFF), > - (UINT8)((EBC_MAGIC_SIGNATURE >> 32) & 0xFF), > - (UINT8)((EBC_MAGIC_SIGNATURE >> 40) & 0xFF), > - (UINT8)((EBC_MAGIC_SIGNATURE >> 48) & 0xFF), > - (UINT8)((EBC_MAGIC_SIGNATURE >> 56) & 0xFF), > - 0x69, 0x00, 0x00, 0x58, //ldr x9, #32 > - 0x8A, 0x00, 0x00, 0x58, //ldr x10, #40 > - 0x05, 0x00, 0x00, 0x14, //b pc+32 > - (UINT8)(EBC_ENTRYPOINT_SIGNATURE & 0xFF), > - (UINT8)((EBC_ENTRYPOINT_SIGNATURE >> 8) & 0xFF), > - (UINT8)((EBC_ENTRYPOINT_SIGNATURE >> 16) & 0xFF), > - (UINT8)((EBC_ENTRYPOINT_SIGNATURE >> 24) & 0xFF), > - (UINT8)((EBC_ENTRYPOINT_SIGNATURE >> 32) & 0xFF), > - (UINT8)((EBC_ENTRYPOINT_SIGNATURE >> 40) & 0xFF), > - (UINT8)((EBC_ENTRYPOINT_SIGNATURE >> 48) & 0xFF), > - (UINT8)((EBC_ENTRYPOINT_SIGNATURE >> 56) & 0xFF), > - (UINT8)(EBC_LL_EBC_ENTRYPOINT_SIGNATURE & 0xFF), > - (UINT8)((EBC_LL_EBC_ENTRYPOINT_SIGNATURE >> 8) & 0xFF), > - (UINT8)((EBC_LL_EBC_ENTRYPOINT_SIGNATURE >> 16) & 0xFF), > - (UINT8)((EBC_LL_EBC_ENTRYPOINT_SIGNATURE >> 24) & 0xFF), > - (UINT8)((EBC_LL_EBC_ENTRYPOINT_SIGNATURE >> 32) & 0xFF), > - (UINT8)((EBC_LL_EBC_ENTRYPOINT_SIGNATURE >> 40) & 0xFF), > - (UINT8)((EBC_LL_EBC_ENTRYPOINT_SIGNATURE >> 48) & 0xFF), > - (UINT8)((EBC_LL_EBC_ENTRYPOINT_SIGNATURE >> 56) & 0xFF), > - 0x40, 0x01, 0x1F, 0xD6 //br x10 > - > -}; > +#pragma pack(1) > +typedef struct { > + UINT32 Instr[3]; > + UINT32 Magic; > + UINT64 EbcEntryPoint; > + UINT64 EbcLlEntryPoint; > +} EBC_INSTRUCTION_BUFFER; > +#pragma pack() > + > +extern CONST EBC_INSTRUCTION_BUFFER mEbcInstructionBufferTemplate; > > /** > Begin executing an EBC image. > @@ -414,10 +385,7 @@ EbcCreateThunks ( > IN UINT32 Flags > ) > { > - UINT8 *Ptr; > - UINT8 *ThunkBase; > - UINT32 Index; > - INT32 ThunkSize; > + EBC_INSTRUCTION_BUFFER *InstructionBuffer; > > // > // Check alignment of pointer to EBC code > @@ -426,51 +394,38 @@ EbcCreateThunks ( > return EFI_INVALID_PARAMETER; > } > > - ThunkSize = sizeof(mInstructionBufferTemplate); > - > - Ptr = AllocatePool (sizeof(mInstructionBufferTemplate)); > - > - if (Ptr == NULL) { > + InstructionBuffer = AllocatePool (sizeof (EBC_INSTRUCTION_BUFFER)); > + if (InstructionBuffer == NULL) { > return EFI_OUT_OF_RESOURCES; > } > - // > - // Print(L"Allocate TH: 0x%X\n", (UINT32)Ptr); > - // > - // Save the start address so we can add a pointer to it to a list later. > - // > - ThunkBase = Ptr; > > // > // Give them the address of our buffer we're going to fix up > // > - *Thunk = (VOID *) Ptr; > + *Thunk = InstructionBuffer; > > // > // Copy whole thunk instruction buffer template > // > - CopyMem (Ptr, mInstructionBufferTemplate, sizeof(mInstructionBufferTemplate)); > + CopyMem (InstructionBuffer, &mEbcInstructionBufferTemplate, > + sizeof (EBC_INSTRUCTION_BUFFER)); > > // > // Patch EbcEntryPoint and EbcLLEbcInterpret > // > - for (Index = 0; Index < sizeof(mInstructionBufferTemplate) - sizeof(UINTN); Index++) { > - if (*(UINTN *)&Ptr[Index] == EBC_ENTRYPOINT_SIGNATURE) { > - *(UINTN *)&Ptr[Index] = (UINTN)EbcEntryPoint; > - } > - if (*(UINTN *)&Ptr[Index] == EBC_LL_EBC_ENTRYPOINT_SIGNATURE) { > - if ((Flags & FLAG_THUNK_ENTRY_POINT) != 0) { > - *(UINTN *)&Ptr[Index] = (UINTN)EbcLLExecuteEbcImageEntryPoint; > - } else { > - *(UINTN *)&Ptr[Index] = (UINTN)EbcLLEbcInterpret; > - } > - } > + InstructionBuffer->EbcEntryPoint = (UINT64)EbcEntryPoint; > + if ((Flags & FLAG_THUNK_ENTRY_POINT) != 0) { > + InstructionBuffer->EbcLlEntryPoint = (UINT64)EbcLLExecuteEbcImageEntryPoint; > + } else { > + InstructionBuffer->EbcLlEntryPoint = (UINT64)EbcLLEbcInterpret; > } > > // > // Add the thunk to the list for this image. Do this last since the add > // function flushes the cache for us. > // > - EbcAddImageThunk (ImageHandle, (VOID *) ThunkBase, ThunkSize); > + EbcAddImageThunk (ImageHandle, InstructionBuffer, > + sizeof (EBC_INSTRUCTION_BUFFER)); > > return EFI_SUCCESS; > } > @@ -500,40 +455,15 @@ EbcLLCALLEX ( > IN UINT8 Size > ) > { > - UINTN IsThunk; > - UINTN TargetEbcAddr; > - UINT8 InstructionBuffer[sizeof(mInstructionBufferTemplate)]; > - UINTN Index; > - UINTN IndexOfEbcEntrypoint; > - > - IsThunk = 1; > - TargetEbcAddr = 0; > - IndexOfEbcEntrypoint = 0; > + CONST EBC_INSTRUCTION_BUFFER *InstructionBuffer; > > // > // Processor specific code to check whether the callee is a thunk to EBC. > // > - CopyMem (InstructionBuffer, (VOID *)FuncAddr, sizeof(InstructionBuffer)); > - // > - // Fill the signature according to mInstructionBufferTemplate > - // > - for (Index = 0; Index < sizeof(mInstructionBufferTemplate) - sizeof(UINTN); Index++) { > - if (*(UINTN *)&mInstructionBufferTemplate[Index] == EBC_ENTRYPOINT_SIGNATURE) { > - *(UINTN *)&InstructionBuffer[Index] = EBC_ENTRYPOINT_SIGNATURE; > - IndexOfEbcEntrypoint = Index; > - } > - if (*(UINTN *)&mInstructionBufferTemplate[Index] == EBC_LL_EBC_ENTRYPOINT_SIGNATURE) { > - *(UINTN *)&InstructionBuffer[Index] = EBC_LL_EBC_ENTRYPOINT_SIGNATURE; > - } > - } > - // > - // Check if we need thunk to native > - // > - if (CompareMem (InstructionBuffer, mInstructionBufferTemplate, sizeof(mInstructionBufferTemplate)) != 0) { > - IsThunk = 0; > - } > + InstructionBuffer = (EBC_INSTRUCTION_BUFFER *)FuncAddr; > > - if (IsThunk == 1){ > + if (CompareMem (InstructionBuffer, &mEbcInstructionBufferTemplate, > + sizeof(EBC_INSTRUCTION_BUFFER) - 2 * sizeof (UINT64)) == 0) { > // > // The callee is a thunk to EBC, adjust the stack pointer down 16 bytes and > // put our return address and frame pointer on the VM stack. > @@ -545,8 +475,7 @@ EbcLLCALLEX ( > VmPtr->Gpr[0] -= 8; > VmWriteMem64 (VmPtr, (UINTN) VmPtr->Gpr[0], (UINT64) (UINTN) (VmPtr->Ip + Size)); > > - CopyMem (&TargetEbcAddr, (UINT8 *)FuncAddr + IndexOfEbcEntrypoint, sizeof(UINTN)); > - VmPtr->Ip = (VMIP) (UINTN) TargetEbcAddr; > + VmPtr->Ip = (VMIP) InstructionBuffer->EbcEntryPoint; > } else { > // > // The callee is not a thunk to EBC, call native code, > -- > 2.7.4 >