Re: [PATCH v2 7/7] MdeModulePkg/DxeCore: remove explicit EBC handling

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 18 September 2018 at 19:16, Ni, Ruiyu <ruiyu.ni@intel.com> wrote:
> On 9/18/2018 9:47 PM, Ard Biesheuvel wrote:
>>
>> On 18 September 2018 at 02:05, Ni, Ruiyu <ruiyu.ni@intel.com> wrote:
>>>
>>> On 9/15/2018 9:28 PM, Ard Biesheuvel wrote:
>>>>
>>>>
>>>> Now that the EBC machine type is no longer classified as a
>>>> natively supported machine type on the architectures that can
>>>> support it via the EBC interpreter, the EBC specific handling
>>>> in DXE core is no longer used and can be removed.
>>>>
>>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>> ---
>>>>    MdeModulePkg/Core/Dxe/DxeMain.h     |  3 --
>>>>    MdeModulePkg/Core/Dxe/DxeMain.inf   |  1 -
>>>>    MdeModulePkg/Core/Dxe/Image/Image.c | 53 ++------------------
>>>>    3 files changed, 3 insertions(+), 54 deletions(-)
>>>>
>>>> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h
>>>> b/MdeModulePkg/Core/Dxe/DxeMain.h
>>>> index ff2418c5ae5e..c473006813fe 100644
>>>> --- a/MdeModulePkg/Core/Dxe/DxeMain.h
>>>> +++ b/MdeModulePkg/Core/Dxe/DxeMain.h
>>>> @@ -42,7 +42,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
>>>> EITHER EXPRESS OR IMPLIED.
>>>>    #include <Protocol/LoadPe32Image.h>
>>>>    #include <Protocol/Security.h>
>>>>    #include <Protocol/Security2.h>
>>>> -#include <Protocol/Ebc.h>
>>>>    #include <Protocol/Reset.h>
>>>>    #include <Protocol/Cpu.h>
>>>>    #include <Protocol/Metronome.h>
>>>> @@ -228,8 +227,6 @@ typedef struct {
>>>>      BASE_LIBRARY_JUMP_BUFFER    *JumpContext;
>>>>      /// Machine type from PE image
>>>>      UINT16                      Machine;
>>>> -  /// EBC Protocol pointer
>>>> -  EFI_EBC_PROTOCOL            *Ebc;
>>>>      /// PE/COFF Image Emulator Protocol pointer
>>>>      EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL  *PeCoffEmu;
>>>>      /// Runtime image list
>>>> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf
>>>> b/MdeModulePkg/Core/Dxe/DxeMain.inf
>>>> index 63e650ee7c27..a969b869b331 100644
>>>> --- a/MdeModulePkg/Core/Dxe/DxeMain.inf
>>>> +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
>>>> @@ -161,7 +161,6 @@
>>>>      gEfiLoadedImageProtocolGuid                   ## PRODUCES
>>>>      gEfiLoadedImageDevicePathProtocolGuid         ## PRODUCES
>>>>      gEfiHiiPackageListProtocolGuid                ## SOMETIMES_PRODUCES
>>>> -  gEfiEbcProtocolGuid                           ## SOMETIMES_CONSUMES
>>>>      gEfiSmmBase2ProtocolGuid                      ## SOMETIMES_CONSUMES
>>>>      gEfiBlockIoProtocolGuid                       ## SOMETIMES_CONSUMES
>>>>      gEdkiiPeCoffImageEmulatorProtocolGuid         ## SOMETIMES_CONSUMES
>>>> diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c
>>>> b/MdeModulePkg/Core/Dxe/Image/Image.c
>>>> index 0a4bb3644af0..902a44455fdd 100644
>>>> --- a/MdeModulePkg/Core/Dxe/Image/Image.c
>>>> +++ b/MdeModulePkg/Core/Dxe/Image/Image.c
>>>> @@ -66,7 +66,6 @@ LOADED_IMAGE_PRIVATE_DATA mCorePrivateImage  = {
>>>>      NULL,                       // JumpBuffer
>>>>      NULL,                       // JumpContext
>>>>      0,                          // Machine
>>>> -  NULL,                       // Ebc
>>>>      NULL,                       // PeCoffEmu
>>>>      NULL,                       // RuntimeData
>>>>      NULL                        // LoadedImageDevicePath
>>>> @@ -83,9 +82,6 @@ typedef struct {
>>>>      CHAR16  *MachineTypeName;
>>>>    } MACHINE_TYPE_INFO;
>>>>    -//
>>>> -// EBC machine is not listed in this table, because EBC is in the
>>>> default
>>>> supported scopes of other machine type.
>>>> -//
>>>>    GLOBAL_REMOVE_IF_UNREFERENCED MACHINE_TYPE_INFO  mMachineTypeInfo[] =
>>>> {
>>>>      {EFI_IMAGE_MACHINE_IA32,           L"IA32"},
>>>>      {EFI_IMAGE_MACHINE_IA64,           L"IA64"},
>>>> @@ -705,51 +701,15 @@ CoreLoadPeImage (
>>>>      InvalidateInstructionCacheRange ((VOID
>>>> *)(UINTN)Image->ImageContext.ImageAddress,
>>>> (UINTN)Image->ImageContext.ImageSize);
>>>>        //
>>>> -  // Copy the machine type from the context to the image private data.
>>>> This
>>>> -  // is needed during image unload to know if we should call an EBC
>>>> protocol
>>>> -  // to unload the image.
>>>> +  // Copy the machine type from the context to the image private data.
>>>>      //
>>>>      Image->Machine = Image->ImageContext.Machine;
>>>>        //
>>>> -  // Get the image entry point. If it's an EBC image, then call into
>>>> the
>>>> -  // interpreter to create a thunk for the entry point and use the
>>>> returned
>>>> -  // value for the entry point.
>>>> +  // Get the image entry point.
>>>>      //
>>>>      Image->EntryPoint   =
>>>> (EFI_IMAGE_ENTRY_POINT)(UINTN)Image->ImageContext.EntryPoint;
>>>> -  if (Image->ImageContext.Machine == EFI_IMAGE_MACHINE_EBC) {
>>>> -    //
>>>> -    // Locate the EBC interpreter protocol
>>>> -    //
>>>> -    Status = CoreLocateProtocol (&gEfiEbcProtocolGuid, NULL, (VOID
>>>> **)&Image->Ebc);
>>>> -    if (EFI_ERROR(Status) || Image->Ebc == NULL) {
>>>> -      DEBUG ((DEBUG_LOAD | DEBUG_ERROR, "CoreLoadPeImage: There is no
>>>> EBC
>>>> interpreter for an EBC image.\n"));
>>>> -      goto Done;
>>>> -    }
>>>> -
>>>> -    //
>>>> -    // Register a callback for flushing the instruction cache so that
>>>> created
>>>> -    // thunks can be flushed.
>>>> -    //
>>>> -    Status = Image->Ebc->RegisterICacheFlush (Image->Ebc,
>>>> (EBC_ICACHE_FLUSH)InvalidateInstructionCacheRange);
>>>> -    if (EFI_ERROR(Status)) {
>>>> -      goto Done;
>>>> -    }
>>>> -
>>>> -    //
>>>> -    // Create a thunk for the image's entry point. This will be the new
>>>> -    // entry point for the image.
>>>> -    //
>>>> -    Status = Image->Ebc->CreateThunk (
>>>> -                           Image->Ebc,
>>>> -                           Image->Handle,
>>>> -                           (VOID *)(UINTN)
>>>> Image->ImageContext.EntryPoint,
>>>> -                           (VOID **) &Image->EntryPoint
>>>> -                           );
>>>> -    if (EFI_ERROR(Status)) {
>>>> -      goto Done;
>>>> -    }
>>>> -  } else if (Image->PeCoffEmu != NULL) {
>>>> +  if (Image->PeCoffEmu != NULL) {
>>>>        Status = Image->PeCoffEmu->RegisterImage (Image->PeCoffEmu,
>>>>                                     Image->ImageBasePage,
>>>>                                     EFI_PAGES_TO_SIZE
>>>> (Image->NumberOfPages),
>>>> @@ -939,13 +899,6 @@ CoreUnloadAndCloseImage (
>>>>        UnprotectUefiImage (&Image->Info, Image->LoadedImageDevicePath);
>>>>    -  if (Image->Ebc != NULL) {
>>>> -    //
>>>> -    // If EBC protocol exists we must perform cleanups for this image.
>>>> -    //
>>>> -    Image->Ebc->UnloadImage (Image->Ebc, Image->Handle);
>>>> -  }
>>>> -
>>>>      if (Image->PeCoffEmu != NULL) {
>>>>        //
>>>>        // If the PE/COFF Emulator protocol exists we must unregister the
>>>> image.
>>>>
>>>
>>> Ard,
>>> Does this change mean EBC and x86 won't be enabled together?
>>>
>>
>> I am not sure I understand your question, so let me just explain again
>> what the purpose is of this patch.
>>
>> In the preceding patches, the EBC driver is updated so it implements
>> the PE/COFF emulator protocol, which is a more generic way of exposing
>> emulator/interpreter functionality to other modules, and the DXE core
>> and UefiBaseTypes.h are updated so that EBC modules will be handled
>> using this new protocol (PE/COFF images that we not built for the
>> native architecture are handed to each existing instance of the
>> PE/COFF emulator protocol until one is found that supports it).
>>
>> This means the explicit EBC handling is new dead code, so it can be
>> removed.
>
> I can understand till now.
>
>
>>
>> On AARCH64 with the X86 emulator installed, EBC and X86 PE/COFF images
>> can both be dispatched.
>
> The X86 emulator contains code only supporting interpreting X86 machine
> code. Why EBC images can be dispatched with the help from X86 emulator?
>

What I mean is, running a AARCH64 build for QEMU with all these
patches applied and the EBC interpreter and the X86 emulator included,
both EBC and X86 PE/COFF images can be dispatched, both via instances
of the PE/COFF emulator protocol.


>
>
>  On builds with only the EBC driver installed
>>
>> (AARCH64 or otherwise), only EBC images and native images can be
>> executed.
>>
>> I hope this answers your question.
>>
>
>
> --
> Thanks,
> Ray