public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: Daniil Egranov <daniil.egranov@arm.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>,
	edk2-devel-01 <edk2-devel@lists.01.org>,
	 "Tian, Feng" <feng.tian@intel.com>,
	Daniil Egranov <daniil.egranov@linaro.org>,
	"Zeng, Star" <star.zeng@intel.com>
Subject: Re: [PATCH 2/2] MdeModulePkg/EbcDxe: cleanup
Date: Fri, 5 Aug 2016 21:27:38 +0200	[thread overview]
Message-ID: <CAKv+Gu_4927CT4XZuXWfSNJU2bX_8y-Jhs2dQEvRFb6JC-7fVg@mail.gmail.com> (raw)
In-Reply-To: <c9776331-3709-a5c5-ca58-bcecacc8f898@arm.com>

On 5 August 2016 at 21:24, Daniil Egranov <daniil.egranov@arm.com> wrote:
> Hi Ard,
>
>
>
> On 08/01/2016 09:22 AM, Ard Biesheuvel wrote:
>>
>> - indentation
>> - premature optimization of the thunking code, i.e., align function
>> prototypes
>>    so we don't have to move arguments around or duplicate them on the
>> stack,
>>    and perform tail calls where possible
>> - adhere to calling convention (stack frame layout)
>> - replace instruction buffer with a fixed struct, so that we no longer
>> have
>>    to traverse it to find the entry point slots
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>   MdeModulePkg/Universal/EbcDxe/AArch64/EbcLowLevel.S | 188
>> ++++++++++---------
>>   MdeModulePkg/Universal/EbcDxe/AArch64/EbcSupport.c  | 193
>> ++++++--------------
>>   2 files changed, 159 insertions(+), 222 deletions(-)
>>
>> diff --git a/MdeModulePkg/Universal/EbcDxe/AArch64/EbcLowLevel.S
>> b/MdeModulePkg/Universal/EbcDxe/AArch64/EbcLowLevel.S
>> index 3c1a461f5e87..d0a5a4c5a37d 100644
>> --- a/MdeModulePkg/Universal/EbcDxe/AArch64/EbcLowLevel.S
>> +++ b/MdeModulePkg/Universal/EbcDxe/AArch64/EbcLowLevel.S
>> @@ -1,10 +1,12 @@
>>   ///** @file
>>   //
>> -//    This code provides low level routines that support the Virtual
>> Machine
>> -//   for option ROMs.
>> +//  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.<BR>
>> +//  Copyright (c) 2015, The Linux Foundation. All rights reserved.<BR>
>>   //  Copyright (c) 2007 - 2014, Intel Corporation. All rights
>> reserved.<BR>
>> +//
>>   //  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
>> @@ -15,9 +17,10 @@
>>   //
>>   //**/
>>   -ASM_GLOBAL ASM_PFX(CopyMem);
>> -ASM_GLOBAL ASM_PFX(EbcInterpret);
>> -ASM_GLOBAL ASM_PFX(ExecuteEbcImageEntryPoint);
>> +ASM_GLOBAL ASM_PFX(EbcLLEbcInterpret);
>> +ASM_GLOBAL ASM_PFX(EbcLLExecuteEbcImageEntryPoint);
>> +
>> +ASM_GLOBAL ASM_PFX(mEbcInstructionBufferTemplate);
>
> The ASM_GLOBAL ASM_PFX(EbcLLCALLEXNative); is missing here. The linking
> fails without it.
>

Thanks for the report. I had a single patch and split it into two, and
apparently, I was sloppy

>>
>> //****************************************************************************
>>   // EbcLLCALLEX
>> @@ -30,102 +33,121 @@ ASM_GLOBAL ASM_PFX(ExecuteEbcImageEntryPoint);
>>   //
>>
>> //****************************************************************************
>>   // UINTN EbcLLCALLEXNative(UINTN FuncAddr, UINTN NewStackPointer, VOID
>> *FramePtr)
>> -ASM_GLOBAL ASM_PFX(EbcLLCALLEXNative);
>>   ASM_PFX(EbcLLCALLEXNative):
>> -      stp  x19, x20, [sp, #-16]!
>> -      stp  x29, x30, [sp, #-16]!
>> -
>> -      mov  x19, x0
>> -      mov  x20, sp
>> -      sub  x2, x2, x1   // Length = NewStackPointer-FramePtr
>> -      sub  sp, sp, x2
>> -      sub  sp, sp, #64  // Make sure there is room for at least 8 args in
>> the new stack
>> -      mov  x0, sp
>> -
>> -      bl   CopyMem      // Sp, NewStackPointer, Length
>> -
>> -      ldp  x0, x1, [sp], #16
>> -      ldp  x2, x3, [sp], #16
>> -      ldp  x4, x5, [sp], #16
>> -      ldp  x6, x7, [sp], #16
>> -
>> -      blr  x19
>> -
>> -      mov  sp,  x20
>> -      ldp  x29, x30, [sp], #16
>> -      ldp  x19, x20, [sp], #16
>> -
>> -      ret
>> +    mov     x8, x0                 // Preserve x0
>> +    mov     x9, x1                 // Preserve x1
>> +
>> +    //
>> +    // If the EBC stack frame is smaller than or equal to 64 bytes, we
>> know there
>> +    // are no stacked arguments #9 and beyond that we need to copy to the
>> native
>> +    // stack. In this case, we can perform a tail call which is much more
>> +    // efficient, since there is no need to touch the native stack at
>> all.
>> +    //
>> +    sub     x3, x2, x1              // Length = NewStackPointer -
>> FramePtr
>> +    cmp     x3, #64
>> +    b.gt    1f
>> +
>> +    adr     x0, 0f
>> +    sub     x0, x0, x3, lsr #1
>> +    br      x0
>> +
>> +    ldr     x7, [x9, #56]
>> +    ldr     x6, [x9, #48]
>> +    ldr     x5, [x9, #40]
>> +    ldr     x4, [x9, #32]
>> +    ldr     x3, [x9, #24]
>> +    ldr     x2, [x9, #16]
>> +    ldr     x1, [x9, #8]
>> +    ldr     x0, [x9]
>> +
>> +0:  br      x8
>> +
>> +    //
>> +    // More than 64 bytes: we need to build the full native stack frame
>> and copy
>> +    // the part of the VM stack exceeding 64 bytes (which may contain
>> stacked
>> +    // arguments) to the native stack
>> +    //
>> +1:  stp     x29, x30, [sp, #-16]!
>> +    mov     x29, sp
>> +
>> +    //
>> +    // Ensure that the stack pointer remains 16 byte aligned,
>> +    // even if the size of the VM stack frame is not a multiple of 16
>> +    //
>> +    add     x1, x1, #64             // Skip over [potential] reg params
>> +    tbz     x3, #3, 2f              // Multiple of 16?
>> +    ldr     x4, [x2, #-8]!          // No? Then push one word
>> +    str     x4, [sp, #-16]!         // ... but use two slots
>> +    b       3f
>> +
>> +2:  ldp     x4, x5, [x2, #-16]!
>> +    stp     x4, x5, [sp, #-16]!
>> +3:  cmp     x2, x1
>> +    b.gt    2b
>> +
>> +    ldp     x0, x1, [x9]
>> +    ldp     x2, x3, [x9, #16]
>> +    ldp     x4, x5, [x9, #32]
>> +    ldp     x6, x7, [x9, #48]
>> +
>> +    blr     x8
>> +
>> +    mov     sp, x29
>> +    ldp     x29, x30, [sp], #16
>> +    ret
>>
>> //****************************************************************************
>>   // EbcLLEbcInterpret
>>   //
>>   // 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.
>>   //
>>
>> //****************************************************************************
>> -ASM_GLOBAL ASM_PFX(EbcLLEbcInterpret);
>>   ASM_PFX(EbcLLEbcInterpret):
>> -    stp  x29, x30, [sp, #-16]!
>> -
>> -    // copy the current arguments 9-16 from old location and add arg 7 to
>> stack
>> -    // keeping 16 byte stack alignment
>> -    sub sp, sp, #80
>> -    str x7, [sp]
>> -    ldr x11, [sp, #96]
>> -    str x11, [sp, #8]
>> -    ldr x11, [sp, #104]
>> -    str x11, [sp, #16]
>> -    ldr x11, [sp, #112]
>> -    str x11, [sp, #24]
>> -    ldr x11, [sp, #120]
>> -    str x11, [sp, #32]
>> -    ldr x11, [sp, #128]
>> -    str x11, [sp, #40]
>> -    ldr x11, [sp, #136]
>> -    str x11, [sp, #48]
>> -    ldr x11, [sp, #144]
>> -    str x11, [sp, #56]
>> -    ldr x11, [sp, #152]
>> -    str x11, [sp, #64]
>> -
>> -    // Shift arguments and add entry point and as argument 1
>> -    mov x7, x6
>> -    mov x6, x5
>> -    mov x5, x4
>> -    mov x4, x3
>> -    mov x3, x2
>> -    mov x2, x1
>> -    mov x1, x0
>> -    mov x0, x9
>> -
>> -    # call C-code
>> -    bl ASM_PFX(EbcInterpret)
>> -    add sp, sp, #80
>> -
>> -    ldp  x29, x30, [sp], #16
>> +    stp     x29, x30, [sp, #-16]!
>> +    mov     x29, sp
>> +
>> +    // push the entry point and the address of args #9 - #16 onto the
>> stack
>> +    add     x17, sp, #16
>> +    stp     x16, x17, [sp, #-16]!
>>   +    // call C-code
>> +    bl      ASM_PFX(EbcInterpret)
>> +
>> +    ldp     x29, x30, [sp, #16]
>> +    add     sp, sp, #32
>>       ret
>>
>> //****************************************************************************
>>   // 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.
>>   //
>>
>> //****************************************************************************
>> -ASM_GLOBAL ASM_PFX(EbcLLExecuteEbcImageEntryPoint);
>>   ASM_PFX(EbcLLExecuteEbcImageEntryPoint):
>> -    stp  x29, x30, [sp, #-16]!
>> -    # build new paramater calling convention
>> -    mov  x2, x1
>> -    mov  x1, x0
>> -    mov  x0, x9
>> -
>> -    # call C-code
>> -    bl ASM_PFX(ExecuteEbcImageEntryPoint)
>> -    ldp  x29, x30, [sp], #16
>> -    ret
>> +    mov     x2, x16
>> +
>> +    // tail call to C code
>> +    b       ASM_PFX(ExecuteEbcImageEntryPoint)
>> +
>>
>> +//****************************************************************************
>> +// 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.
>> +    //
>> +    .long   0xCA112EBC
>> +
>> +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..d15bbc861f33 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.<BR>
>> +Copyright (c) 2015, The Linux Foundation. All rights reserved.<BR>
>>   Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.<BR>
>> +
>>   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.
>> @@ -109,7 +80,6 @@ PushU64 (
>>     //
>>     VmPtr->Gpr[0] -= sizeof (UINT64);
>>     *(UINT64 *) VmPtr->Gpr[0] = Arg;
>> -  return;
>>   }
>>     @@ -118,7 +88,6 @@ PushU64 (
>>       This is a thunk function.
>>   -  @param  EntryPoint            The entrypoint of EBC code.
>>     @param  Arg1                  The 1st argument.
>>     @param  Arg2                  The 2nd argument.
>>     @param  Arg3                  The 3rd argument.
>> @@ -127,14 +96,8 @@ PushU64 (
>>     @param  Arg6                  The 6th argument.
>>     @param  Arg7                  The 7th argument.
>>     @param  Arg8                  The 8th argument.
>> -  @param  Arg9                  The 9th argument.
>> -  @param  Arg10                 The 10th argument.
>> -  @param  Arg11                 The 11th argument.
>> -  @param  Arg12                 The 12th argument.
>> -  @param  Arg13                 The 13th argument.
>> -  @param  Arg14                 The 14th argument.
>> -  @param  Arg15                 The 15th argument.
>> -  @param  Arg16                 The 16th argument.
>> +  @param  EntryPoint            The entrypoint of EBC code.
>> +  @param  Args9_16[]            Array containing arguments #9 to #16.
>>       @return The value returned by the EBC application we're going to
>> run.
>>   @@ -142,7 +105,6 @@ PushU64 (
>>   UINT64
>>   EFIAPI
>>   EbcInterpret (
>> -  IN UINTN      EntryPoint,
>>     IN UINTN      Arg1,
>>     IN UINTN      Arg2,
>>     IN UINTN      Arg3,
>> @@ -151,14 +113,8 @@ EbcInterpret (
>>     IN UINTN      Arg6,
>>     IN UINTN      Arg7,
>>     IN UINTN      Arg8,
>> -  IN UINTN      Arg9,
>> -  IN UINTN      Arg10,
>> -  IN UINTN      Arg11,
>> -  IN UINTN      Arg12,
>> -  IN UINTN      Arg13,
>> -  IN UINTN      Arg14,
>> -  IN UINTN      Arg15,
>> -  IN UINTN      Arg16
>> +  IN UINTN      EntryPoint,
>> +  IN UINTN      Args9_16[]
>>     )
>>   {
>>     //
>> @@ -222,14 +178,14 @@ EbcInterpret (
>>     // For the worst case, assume there are 4 arguments passed in
>> registers, store
>>     // them to VM's stack.
>>     //
>> -  PushU64 (&VmContext, (UINT64) Arg16);
>> -  PushU64 (&VmContext, (UINT64) Arg15);
>> -  PushU64 (&VmContext, (UINT64) Arg14);
>> -  PushU64 (&VmContext, (UINT64) Arg13);
>> -  PushU64 (&VmContext, (UINT64) Arg12);
>> -  PushU64 (&VmContext, (UINT64) Arg11);
>> -  PushU64 (&VmContext, (UINT64) Arg10);
>> -  PushU64 (&VmContext, (UINT64) Arg9);
>> +  PushU64 (&VmContext, (UINT64) Args9_16[7]);
>> +  PushU64 (&VmContext, (UINT64) Args9_16[6]);
>> +  PushU64 (&VmContext, (UINT64) Args9_16[5]);
>> +  PushU64 (&VmContext, (UINT64) Args9_16[4]);
>> +  PushU64 (&VmContext, (UINT64) Args9_16[3]);
>> +  PushU64 (&VmContext, (UINT64) Args9_16[2]);
>> +  PushU64 (&VmContext, (UINT64) Args9_16[1]);
>> +  PushU64 (&VmContext, (UINT64) Args9_16[0]);
>>     PushU64 (&VmContext, (UINT64) Arg8);
>>     PushU64 (&VmContext, (UINT64) Arg7);
>>     PushU64 (&VmContext, (UINT64) Arg6);
>> @@ -281,10 +237,10 @@ EbcInterpret (
>>   /**
>>     Begin executing an EBC image.
>>   -  @param  EntryPoint       The entrypoint of EBC code.
>>     @param  ImageHandle      image handle for the EBC application we're
>> executing
>>     @param  SystemTable      standard system table passed into an driver's
>> entry
>>                              point
>> +  @param  EntryPoint       The entrypoint of EBC code.
>>       @return The value returned by the EBC application we're going to
>> run.
>>   @@ -292,9 +248,9 @@ EbcInterpret (
>>   UINT64
>>   EFIAPI
>>   ExecuteEbcImageEntryPoint (
>> -  IN UINTN                EntryPoint,
>>     IN EFI_HANDLE           ImageHandle,
>> -  IN EFI_SYSTEM_TABLE     *SystemTable
>> +  IN EFI_SYSTEM_TABLE     *SystemTable,
>> +  IN UINTN                EntryPoint
>>     )
>>   {
>>     //
>> @@ -335,7 +291,7 @@ ExecuteEbcImageEntryPoint (
>>     if (EFI_ERROR(Status)) {
>>       return Status;
>>     }
>> -  VmContext.StackTop = (UINT8*)VmContext.StackPool + (STACK_REMAIN_SIZE);
>> +  VmContext.StackTop = (UINT8 *)VmContext.StackPool +
>> (STACK_REMAIN_SIZE);
>>     VmContext.Gpr[0] = (UINT64) ((UINT8*)VmContext.StackPool +
>> STACK_POOL_SIZE);
>>     VmContext.HighStackBottom = (UINTN) VmContext.Gpr[0];
>>     VmContext.Gpr[0] -= sizeof (UINTN);
>> @@ -372,11 +328,6 @@ ExecuteEbcImageEntryPoint (
>>     VmContext.StackRetAddr  = (UINT64) VmContext.Gpr[0];
>>       //
>> -  // Entry function needn't access high stack context, simply
>> -  // put the stack pointer here.
>> -  //
>> -
>> -  //
>>     // Begin executing the EBC code
>>     //
>>     EbcExecute (&VmContext);
>> @@ -414,10 +365,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 +374,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 +435,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,14 +455,19 @@ 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,
>>       // and get return value.
>>       //
>> -    VmPtr->Gpr[7] = EbcLLCALLEXNative (FuncAddr, NewStackPointer,
>> FramePtr);
>> +    // Note that we will not be able to distinguish which part of the
>> interval
>> +    // [NewStackPointer, FramePtr) consists of stacked function arguments
>> for
>> +    // this call, and which part simply consists of locals in the
>> caller's
>> +    // stack frame. All we know is that there is an 8 byte gap at the top
>> that
>> +    // we can ignore.
>> +    //
>> +    VmPtr->Gpr[7] = EbcLLCALLEXNative (FuncAddr, NewStackPointer,
>> FramePtr - 8);
>>         //
>>       // Advance the IP.
>
>


      reply	other threads:[~2016-08-05 19:27 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-01 14:22 [PATCH 1/2] MdeModulePkg/EbcDxe AARCH64: fix comment style Ard Biesheuvel
2016-08-01 14:22 ` [PATCH 2/2] MdeModulePkg/EbcDxe: cleanup Ard Biesheuvel
2016-08-05 19:24   ` Daniil Egranov
2016-08-05 19:27     ` Ard Biesheuvel [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAKv+Gu_4927CT4XZuXWfSNJU2bX_8y-Jhs2dQEvRFb6JC-7fVg@mail.gmail.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox