[PATCH v2 0/7] MdeModulePkg: add support for dispatching foreign arch PE/COFF images

[PATCH v2 0/7] MdeModulePkg: add support for dispatching foreign arch PE/COFF images

[Ard Biesheuvel]

Add the basic plumbing to DXE core, the PCI bus driver and the boot manager
to allow PE/COFF images to be dispatched that target an architecture that is
not native for the platform, but which is supported by one of potentially
several available emulators.

One implementation of such an emulator can be found here:
https://github.com/ardbiesheuvel/X86EmulatorPkg

This also allows us to get rid of the special treatment of EBC images in
core code. Instead, the EbcDxe driver is augmented with an implementation
of the EDK2 PE/COFF image emulator so that internal knowledge of how EBC
is implemented (I-cache flushing, thunks) is removed from the DXE core.

Changes since v2:
- subsume the EBC handling into the EDK2 emulator protocol and abstract
  away from EBC specifics in core code.
- allow multiple emulator implementations to co-exist
- incorporate Star's review feedback

Cc: Vincent Zimmer <vincent.zimmer@intel.com>
Cc: Brian Richardson <brian.richardson@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Andrew Fish <afish@apple.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Jaben Carsey <jaben.carsey@intel.com>
Cc: Steven Shi <steven.shi@intel.com>

Ard Biesheuvel (7):
  MdeModulePkg: introduce PE/COFF image emulator protocol
  MdeModulePkg/DxeCore: invoke the emulator protocol for foreign images
  MdeModulePkg/PciBusDxe: invoke PE/COFF emulator for foreign option
    ROMs
  MdeModulePkg/UefiBootManagerLib: allow foreign Driver#### images
  MdeModulePkg/EbcDxe: implement the PE/COFF emulator protocol
  MdePkg/UefiBaseType.h: treat EBC as a non-native machine type
  MdeModulePkg/DxeCore: remove explicit EBC handling

 MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h       |   1 +
 MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf  |   1 +
 .../Bus/Pci/PciBusDxe/PciOptionRomSupport.c   |  51 ++++++-
 MdeModulePkg/Core/Dxe/DxeMain.h               |   6 +-
 MdeModulePkg/Core/Dxe/DxeMain.inf             |   2 +-
 MdeModulePkg/Core/Dxe/Image/Image.c           | 141 +++++++++++-------
 .../Include/Protocol/PeCoffImageEmulator.h    |  99 ++++++++++++
 .../Library/UefiBootManagerLib/BmLoadOption.c |  51 ++++++-
 .../Library/UefiBootManagerLib/InternalBm.h   |   1 +
 .../UefiBootManagerLib/UefiBootManagerLib.inf |   1 +
 MdeModulePkg/MdeModulePkg.dec                 |   4 +
 MdeModulePkg/Universal/EbcDxe/EbcDxe.inf      |   3 +
 MdeModulePkg/Universal/EbcDxe/EbcInt.c        | 124 +++++++++++++++
 MdeModulePkg/Universal/EbcDxe/EbcInt.h        |   3 +
 MdePkg/Include/Uefi/UefiBaseType.h            |   8 +-
 15 files changed, 432 insertions(+), 64 deletions(-)
 create mode 100644 MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h

-- 
2.17.1

[PATCH v2 1/7] MdeModulePkg: introduce PE/COFF image emulator protocol

[Ard Biesheuvel]

Introduce a protocol that can be invoked by the image loading services
to execute foreign architecture PE/COFF images via an emulator.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h | 99 ++++++++++++++++++++
 MdeModulePkg/MdeModulePkg.dec                       |  4 +
 2 files changed, 103 insertions(+)

diff --git a/MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h b/MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h
new file mode 100644
index 000000000000..a604df322999
--- /dev/null
+++ b/MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h
@@ -0,0 +1,99 @@
+/** @file
+  Copyright (c) 2018, Linaro, Ltd. 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
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#ifndef __PECOFF_IMAGE_EMULATOR_PROTOCOL_GUID_H__
+#define __PECOFF_IMAGE_EMULATOR_PROTOCOL_GUID_H__
+
+#define EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL_GUID \
+  { 0x96F46153, 0x97A7, 0x4793, { 0xAC, 0xC1, 0xFA, 0x19, 0xBF, 0x78, 0xEA, 0x97 } }
+
+typedef struct _EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL;
+
+/**
+  Check whether the emulator supports executing a certain PE/COFF image
+
+  @param[in] This         This pointer for EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL
+                          structure
+  @param[in] MachineType  The machine type for which the image was built
+  @param[in] ImageType    Whether the image is an application, a boot time
+                          driver or a runtime driver.
+
+  @retval TRUE            The image is supported by the emulator
+  @retval FALSE           The image is not supported by the emulator.
+**/
+typedef
+BOOLEAN
+(EFIAPI *EDKII_PECOFF_IMAGE_EMULATOR_IS_IMAGE_SUPPORTED) (
+  IN  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL    *This,
+  IN  UINT16                                  MachineType,
+  IN  UINT16                                  ImageType
+  );
+
+/**
+  Register a supported PE/COFF image with the emulator. After this call
+  completes successfully, the PE/COFF image may be started as usual, and
+  it is the responsibility of the emulator implementation that any branch
+  into the code section of the image (including returns from functions called
+  from the foreign code) is executed as if it were running on the machine
+  type it was built for.
+
+  @param[in]      This          This pointer for
+                                EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL structure
+  @param[in]      ImageBase     The base address in memory of the PE/COFF image
+  @param[in]      ImageSize     The size in memory of the PE/COFF image
+  @param[in,out]  EntryPoint    The entry point of the PE/COFF image. Passed by
+                                reference so that the emulator may modify it.
+
+  @retval EFI_SUCCESS           The image was registered with the emulator and
+                                can be started as usual.
+  @retval other                 The image could not be registered.
+
+  If the PE/COFF machine type or image type are not supported by the emulator,
+  then ASSERT().
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EDKII_PECOFF_IMAGE_EMULATOR_REGISTER_IMAGE) (
+  IN      EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL    *This,
+  IN      EFI_PHYSICAL_ADDRESS                    ImageBase,
+  IN      UINT64                                  ImageSize,
+  IN  OUT EFI_IMAGE_ENTRY_POINT                   *EntryPoint
+  );
+
+/**
+  Unregister a PE/COFF image that has been registered with the emulator.
+  This should be done before the image is unloaded from memory.
+
+  @param[in] This         This pointer for EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL
+                          structure
+  @param[in] ImageBase    The base address in memory of the PE/COFF image
+
+  @retval EFI_SUCCESS     The image was unregistered with the emulator.
+  @retval other           Image could not be unloaded.
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EDKII_PECOFF_IMAGE_EMULATOR_UNREGISTER_IMAGE) (
+  IN  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL    *This,
+  IN  EFI_PHYSICAL_ADDRESS                    ImageBase
+  );
+
+typedef struct _EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL {
+  EDKII_PECOFF_IMAGE_EMULATOR_IS_IMAGE_SUPPORTED    IsImageSupported;
+  EDKII_PECOFF_IMAGE_EMULATOR_REGISTER_IMAGE        RegisterImage;
+  EDKII_PECOFF_IMAGE_EMULATOR_UNREGISTER_IMAGE      UnregisterImage;
+} EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL;
+
+extern EFI_GUID gEdkiiPeCoffImageEmulatorProtocolGuid;
+
+#endif
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 6a6d9660edc2..be307329f901 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -617,6 +617,10 @@
 
   ## Include/Protocol/AtaAtapiPolicy.h
   gEdkiiAtaAtapiPolicyProtocolGuid = { 0xe59cd769, 0x5083, 0x4f26,{ 0x90, 0x94, 0x6c, 0x91, 0x9f, 0x91, 0x6c, 0x4e } }
+
+  ## Include/Protocol/PeCoffImageEmulator.h
+  gEdkiiPeCoffImageEmulatorProtocolGuid = { 0x96f46153, 0x97a7, 0x4793, { 0xac, 0xc1, 0xfa, 0x19, 0xbf, 0x78, 0xea, 0x97 } }
+
 #
 # [Error.gEfiMdeModulePkgTokenSpaceGuid]
 #   0x80000001 | Invalid value provided.
-- 
2.17.1

[PATCH v2 2/7] MdeModulePkg/DxeCore: invoke the emulator protocol for foreign images

[Ard Biesheuvel]

When encountering PE/COFF images that cannot be supported natively,
attempt to locate an instance of the PE/COFF image emulator protocol,
and if it supports the image, proceed with loading it and register it
with the emulator.

Contributed-under: TianoCore Contribution Agreement 1.0
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 | 102 +++++++++++++++++---
 3 files changed, 95 insertions(+), 11 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h b/MdeModulePkg/Core/Dxe/DxeMain.h
index 7ec82388a3f9..ff2418c5ae5e 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.h
+++ b/MdeModulePkg/Core/Dxe/DxeMain.h
@@ -53,6 +53,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #include <Protocol/TcgService.h>
 #include <Protocol/HiiPackageList.h>
 #include <Protocol/SmmBase2.h>
+#include <Protocol/PeCoffImageEmulator.h>
 #include <Guid/MemoryTypeInformation.h>
 #include <Guid/FirmwareFileSystem2.h>
 #include <Guid/FirmwareFileSystem3.h>
@@ -229,6 +230,8 @@ typedef struct {
   UINT16                      Machine;
   /// EBC Protocol pointer
   EFI_EBC_PROTOCOL            *Ebc;
+  /// PE/COFF Image Emulator Protocol pointer
+  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL  *PeCoffEmu;
   /// Runtime image list
   EFI_RUNTIME_IMAGE_ENTRY     *RuntimeData;
   /// Pointer to Loaded Image Device Path Protocol
diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf b/MdeModulePkg/Core/Dxe/DxeMain.inf
index 68fa0a01d9bd..63e650ee7c27 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.inf
+++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
@@ -164,6 +164,7 @@
   gEfiEbcProtocolGuid                           ## SOMETIMES_CONSUMES
   gEfiSmmBase2ProtocolGuid                      ## SOMETIMES_CONSUMES
   gEfiBlockIoProtocolGuid                       ## SOMETIMES_CONSUMES
+  gEdkiiPeCoffImageEmulatorProtocolGuid         ## SOMETIMES_CONSUMES
 
   # Arch Protocols
   gEfiBdsArchProtocolGuid                       ## CONSUMES
diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c b/MdeModulePkg/Core/Dxe/Image/Image.c
index eddca140ee1a..0a4bb3644af0 100644
--- a/MdeModulePkg/Core/Dxe/Image/Image.c
+++ b/MdeModulePkg/Core/Dxe/Image/Image.c
@@ -67,6 +67,7 @@ LOADED_IMAGE_PRIVATE_DATA mCorePrivateImage  = {
   NULL,                       // JumpContext
   0,                          // Machine
   NULL,                       // Ebc
+  NULL,                       // PeCoffEmu
   NULL,                       // RuntimeData
   NULL                        // LoadedImageDevicePath
 };
@@ -425,6 +426,69 @@ GetPeCoffImageFixLoadingAssignedAddress(
    DEBUG ((EFI_D_INFO|EFI_D_LOAD, "LOADING MODULE FIXED INFO: Loading module at fixed address 0x%11p. Status = %r \n", (VOID *)(UINTN)(ImageContext->ImageAddress), Status));
    return Status;
 }
+
+/**
+  Decides whether a PE/COFF image can execute on this system, either natively
+  or via emulation/interpretation. In that latter case, the PeCoffEmu member
+  of the LOADED_IMAGE_PRIVATE_DATA struct pointer is populated with a pointer
+  to the emulator protocol that supports this image.
+
+  @param[in]  Image         LOADED_IMAGE_PRIVATE_DATA struct pointer
+**/
+STATIC
+BOOLEAN
+CoreIsImageTypeSupported (
+  IN OUT LOADED_IMAGE_PRIVATE_DATA   *Image
+  )
+{
+  EFI_STATUS                            Status;
+  UINTN                                 HandleCount;
+  EFI_HANDLE                            *HandleBuffer;
+  BOOLEAN                               ReturnValue;
+  UINTN                                 Index;
+  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL  *Emu;
+
+  if (EFI_IMAGE_MACHINE_TYPE_SUPPORTED (Image->ImageContext.Machine) ||
+      EFI_IMAGE_MACHINE_CROSS_TYPE_SUPPORTED (Image->ImageContext.Machine)) {
+    return TRUE;
+  }
+
+  //
+  // Locate the emulator protocol to check whether it supports this
+  // image.
+  //
+  Status = CoreLocateHandleBuffer (
+             ByProtocol,
+             &gEdkiiPeCoffImageEmulatorProtocolGuid,
+             NULL,
+             &HandleCount,
+             &HandleBuffer
+             );
+  if (EFI_ERROR (Status)) {
+    return FALSE;
+  }
+
+  ReturnValue = FALSE;
+  for (Index = 0; Index < HandleCount; Index++) {
+    Status = CoreHandleProtocol (
+                HandleBuffer[Index],
+                &gEdkiiPeCoffImageEmulatorProtocolGuid,
+                (VOID **)&Emu
+                );
+    ASSERT_EFI_ERROR (Status);
+
+    if (Emu->IsImageSupported (Emu, Image->ImageContext.Machine,
+               Image->ImageContext.ImageType)) {
+      ReturnValue = TRUE;
+      Image->PeCoffEmu = Emu;
+      break;
+    }
+  }
+
+  FreePool (HandleBuffer);
+  return ReturnValue;
+}
+
 /**
   Loads, relocates, and invokes a PE/COFF image
 
@@ -473,16 +537,14 @@ CoreLoadPeImage (
     return Status;
   }
 
-  if (!EFI_IMAGE_MACHINE_TYPE_SUPPORTED (Image->ImageContext.Machine)) {
-    if (!EFI_IMAGE_MACHINE_CROSS_TYPE_SUPPORTED (Image->ImageContext.Machine)) {
-      //
-      // The PE/COFF loader can support loading image types that can be executed.
-      // If we loaded an image type that we can not execute return EFI_UNSUPORTED.
-      //
-      DEBUG ((EFI_D_ERROR, "Image type %s can't be loaded ", GetMachineTypeName(Image->ImageContext.Machine)));
-      DEBUG ((EFI_D_ERROR, "on %s UEFI system.\n", GetMachineTypeName(mDxeCoreImageMachineType)));
-      return EFI_UNSUPPORTED;
-    }
+  if (!CoreIsImageTypeSupported (Image)) {
+    //
+    // The PE/COFF loader can support loading image types that can be executed.
+    // If we loaded an image type that we can not execute return EFI_UNSUPORTED.
+    //
+    DEBUG ((EFI_D_ERROR, "Image type %s can't be loaded ", GetMachineTypeName(Image->ImageContext.Machine)));
+    DEBUG ((EFI_D_ERROR, "on %s UEFI system.\n", GetMachineTypeName(mDxeCoreImageMachineType)));
+    return EFI_UNSUPPORTED;
   }
 
   //
@@ -687,6 +749,16 @@ CoreLoadPeImage (
     if (EFI_ERROR(Status)) {
       goto Done;
     }
+  } else if (Image->PeCoffEmu != NULL) {
+    Status = Image->PeCoffEmu->RegisterImage (Image->PeCoffEmu,
+                                 Image->ImageBasePage,
+                                 EFI_PAGES_TO_SIZE (Image->NumberOfPages),
+                                 &Image->EntryPoint);
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_LOAD | DEBUG_ERROR,
+        "CoreLoadPeImage: Failed to register foreign image with emulator.\n"));
+      goto Done;
+    }
   }
 
   //
@@ -874,6 +946,13 @@ CoreUnloadAndCloseImage (
     Image->Ebc->UnloadImage (Image->Ebc, Image->Handle);
   }
 
+  if (Image->PeCoffEmu != NULL) {
+    //
+    // If the PE/COFF Emulator protocol exists we must unregister the image.
+    //
+    Image->PeCoffEmu->UnregisterImage (Image->PeCoffEmu, Image->ImageBasePage);
+  }
+
   //
   // Unload image, free Image->ImageContext->ModHandle
   //
@@ -1599,7 +1678,8 @@ CoreStartImage (
   //
   // The image to be started must have the machine type supported by DxeCore.
   //
-  if (!EFI_IMAGE_MACHINE_TYPE_SUPPORTED (Image->Machine)) {
+  if (!EFI_IMAGE_MACHINE_TYPE_SUPPORTED (Image->Machine) &&
+      Image->PeCoffEmu == NULL) {
     //
     // Do not ASSERT here, because image might be loaded via EFI_IMAGE_MACHINE_CROSS_TYPE_SUPPORTED
     // But it can not be started.
-- 
2.17.1

[PATCH v2 3/7] MdeModulePkg/PciBusDxe: invoke PE/COFF emulator for foreign option ROMs

[Ard Biesheuvel]

When enumerating option ROM images, take into account whether an emulator
exists that would allow dispatch of PE/COFF images built for foreign
architectures.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h              |  1 +
 MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf         |  1 +
 MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c | 51 +++++++++++++++++++-
 3 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
index 55eb3a5a8070..dc57d4876c0f 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
@@ -33,6 +33,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #include <Protocol/PciOverride.h>
 #include <Protocol/PciEnumerationComplete.h>
 #include <Protocol/IoMmu.h>
+#include <Protocol/PeCoffImageEmulator.h>
 
 #include <Library/DebugLib.h>
 #include <Library/UefiDriverEntryPoint.h>
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
index a21dd2b5edf4..c8b861093292 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
@@ -96,6 +96,7 @@
   gEfiIncompatiblePciDeviceSupportProtocolGuid    ## SOMETIMES_CONSUMES
   gEfiLoadFile2ProtocolGuid                       ## SOMETIMES_PRODUCES
   gEdkiiIoMmuProtocolGuid                         ## SOMETIMES_CONSUMES
+  gEdkiiPeCoffImageEmulatorProtocolGuid           ## SOMETIMES_CONSUMES
   gEfiLoadedImageDevicePathProtocolGuid           ## CONSUMES
 
 [FeaturePcd]
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
index c2be85a906af..ac3a9fcaf1d0 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
@@ -651,6 +651,54 @@ RomDecode (
   }
 }
 
+STATIC
+BOOLEAN
+IsImageTypeSupported (
+  IN  UINT16    MachineType,
+  IN  UINT16    SubSystem
+  )
+{
+  EFI_STATUS                            Status;
+  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL  *Emu;
+  UINTN                                 HandleCount;
+  EFI_HANDLE                            *HandleBuffer;
+  BOOLEAN                               ReturnValue;
+  UINTN                                 Index;
+
+  if (EFI_IMAGE_MACHINE_TYPE_SUPPORTED (MachineType)) {
+    return TRUE;
+  }
+
+  Status = gBS->LocateHandleBuffer (
+                  ByProtocol,
+                  &gEdkiiPeCoffImageEmulatorProtocolGuid,
+                  NULL,
+                  &HandleCount,
+                  &HandleBuffer
+                  );
+  if (EFI_ERROR (Status)) {
+    return FALSE;
+  }
+
+  ReturnValue = FALSE;
+  for (Index = 0; Index < HandleCount; Index++) {
+    Status = gBS->HandleProtocol (
+                    HandleBuffer[Index],
+                    &gEdkiiPeCoffImageEmulatorProtocolGuid,
+                    (VOID **)&Emu
+                    );
+    ASSERT_EFI_ERROR (Status);
+
+    if (Emu->IsImageSupported (Emu, MachineType, SubSystem)) {
+      ReturnValue = TRUE;
+      break;
+    }
+  }
+
+  FreePool (HandleBuffer);
+  return ReturnValue;
+}
+
 /**
   Load and start the Option Rom image.
 
@@ -715,7 +763,8 @@ ProcessOpRomImage (
     //
     // Skip the EFI PCI Option ROM image if its machine type is not supported
     //
-    if (!EFI_IMAGE_MACHINE_TYPE_SUPPORTED (EfiRomHeader->EfiMachineType)) {
+    if (!IsImageTypeSupported(EfiRomHeader->EfiMachineType,
+                              EfiRomHeader->EfiSubsystem)) {
       goto NextImage;
     }
 
-- 
2.17.1

[PATCH v2 4/7] MdeModulePkg/UefiBootManagerLib: allow foreign Driver#### images

[Ard Biesheuvel]

Allow PE/COFF images that must execute under emulation for Driver####
options, by relaxing the machine type check to include support for
machine types that is provided by the emulator.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c         | 51 +++++++++++++++++++-
 MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h           |  1 +
 MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf |  1 +
 3 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
index 7bf96646c690..9bd8bbcf0bf1 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
@@ -1185,6 +1185,54 @@ EfiBootManagerFreeLoadOptions (
   return EFI_SUCCESS;
 }
 
+STATIC
+BOOLEAN
+BmIsImageTypeSupported (
+  IN  UINT16    MachineType,
+  IN  UINT16    SubSystem
+  )
+{
+  EFI_STATUS                            Status;
+  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL  *Emu;
+  UINTN                                 HandleCount;
+  EFI_HANDLE                            *HandleBuffer;
+  BOOLEAN                               ReturnValue;
+  UINTN                                 Index;
+
+  if (EFI_IMAGE_MACHINE_TYPE_SUPPORTED (MachineType)) {
+    return TRUE;
+  }
+
+  Status = gBS->LocateHandleBuffer (
+                  ByProtocol,
+                  &gEdkiiPeCoffImageEmulatorProtocolGuid,
+                  NULL,
+                  &HandleCount,
+                  &HandleBuffer
+                  );
+  if (EFI_ERROR (Status)) {
+    return FALSE;
+  }
+
+  ReturnValue = FALSE;
+  for (Index = 0; Index < HandleCount; Index++) {
+    Status = gBS->HandleProtocol (
+                    HandleBuffer[Index],
+                    &gEdkiiPeCoffImageEmulatorProtocolGuid,
+                    (VOID **)&Emu
+                    );
+    ASSERT_EFI_ERROR (Status);
+
+    if (Emu->IsImageSupported (Emu, MachineType, SubSystem)) {
+      ReturnValue = TRUE;
+      break;
+    }
+  }
+
+  FreePool (HandleBuffer);
+  return ReturnValue;
+}
+
 /**
   Return whether the PE header of the load option is valid or not.
 
@@ -1235,7 +1283,8 @@ BmIsLoadOptionPeHeaderValid (
       OptionalHeader = (EFI_IMAGE_OPTIONAL_HEADER32 *) &PeHeader->Pe32.OptionalHeader;
       if ((OptionalHeader->Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC ||
            OptionalHeader->Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) &&
-          EFI_IMAGE_MACHINE_TYPE_SUPPORTED (PeHeader->Pe32.FileHeader.Machine)
+          BmIsImageTypeSupported (PeHeader->Pe32.FileHeader.Machine,
+                                  OptionalHeader->Subsystem)
           ) {
         //
         // Check the Subsystem:
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
index 978fbff966f6..5f64ef304b87 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
+++ b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
@@ -47,6 +47,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #include <Protocol/VariableLock.h>
 #include <Protocol/RamDisk.h>
 #include <Protocol/DeferredImageLoad.h>
+#include <Protocol/PeCoffImageEmulator.h>
 
 #include <Guid/MemoryTypeInformation.h>
 #include <Guid/FileInfo.h>
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
index 72c5ca1cd59e..09e2134acf8e 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
+++ b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
@@ -104,6 +104,7 @@
   gEfiDevicePathProtocolGuid                    ## SOMETIMES_CONSUMES
   gEfiBootLogoProtocolGuid                      ## SOMETIMES_CONSUMES
   gEfiSimpleTextInputExProtocolGuid             ## SOMETIMES_CONSUMES
+  gEdkiiPeCoffImageEmulatorProtocolGuid         ## SOMETIMES_CONSUMES
   gEdkiiVariableLockProtocolGuid                ## SOMETIMES_CONSUMES
   gEfiGraphicsOutputProtocolGuid                ## SOMETIMES_CONSUMES
   gEfiUsbIoProtocolGuid                         ## SOMETIMES_CONSUMES
-- 
2.17.1

[PATCH v2 5/7] MdeModulePkg/EbcDxe: implement the PE/COFF emulator protocol

[Ard Biesheuvel]

Implement the new EDK2 PE/COFF image emulator protocol so that we can
remove the EBC specific handling in the DXE core and other places in
the core code.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 MdeModulePkg/Universal/EbcDxe/EbcDxe.inf |   3 +
 MdeModulePkg/Universal/EbcDxe/EbcInt.c   | 124 ++++++++++++++++++++
 MdeModulePkg/Universal/EbcDxe/EbcInt.h   |   3 +
 3 files changed, 130 insertions(+)

diff --git a/MdeModulePkg/Universal/EbcDxe/EbcDxe.inf b/MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
index 8f128b121d0b..9cba1d0a917b 100644
--- a/MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
+++ b/MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
@@ -62,7 +62,9 @@
   MdeModulePkg/MdeModulePkg.dec
 
 [LibraryClasses]
+  CacheMaintenanceLib
   MemoryAllocationLib
+  PeCoffLib
   UefiBootServicesTableLib
   BaseMemoryLib
   UefiDriverEntryPoint
@@ -73,6 +75,7 @@
 [Protocols]
   gEfiDebugSupportProtocolGuid                  ## PRODUCES
   gEfiEbcProtocolGuid                           ## PRODUCES
+  gEdkiiPeCoffImageEmulatorProtocolGuid         ## PRODUCES
   gEfiEbcVmTestProtocolGuid                     ## SOMETIMES_PRODUCES
   gEfiEbcSimpleDebuggerProtocolGuid             ## SOMETIMES_CONSUMES
 
diff --git a/MdeModulePkg/Universal/EbcDxe/EbcInt.c b/MdeModulePkg/Universal/EbcDxe/EbcInt.c
index 727ba8bcae44..fbd9003565be 100644
--- a/MdeModulePkg/Universal/EbcDxe/EbcInt.c
+++ b/MdeModulePkg/Universal/EbcDxe/EbcInt.c
@@ -349,6 +349,120 @@ UINTN                  mStackNum = 0;
 EFI_EVENT              mEbcPeriodicEvent;
 VM_CONTEXT             *mVmPtr = NULL;
 
+/**
+  Check whether the emulator supports executing a certain PE/COFF image
+
+  @param[in] This         This pointer for EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL
+                          structure
+  @param[in] MachineType  The machine type for which the image was built
+  @param[in] ImageType    Whether the image is an application, a boot time
+                          driver or a runtime driver.
+
+  @retval TRUE            The image is supported by the emulator
+  @retval FALSE           The image is not supported by the emulator.
+**/
+BOOLEAN
+EFIAPI
+EbcIsImageSupported (
+  IN  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL    *This,
+  IN  UINT16                                  MachineType,
+  IN  UINT16                                  ImageType
+  )
+{
+  if (MachineType != EFI_IMAGE_MACHINE_EBC) {
+    return FALSE;
+  }
+
+  if (ImageType != EFI_IMAGE_SUBSYSTEM_EFI_APPLICATION &&
+      ImageType != EFI_IMAGE_SUBSYSTEM_EFI_BOOT_SERVICE_DRIVER) {
+    return FALSE;
+  }
+  return TRUE;
+}
+
+/**
+  Register a supported PE/COFF image with the emulator. After this call
+  completes successfully, the PE/COFF image may be started as usual, and
+  it is the responsibility of the emulator implementation that any branch
+  into the code section of the image (including returns from functions called
+  from the foreign code) is executed as if it were running on the machine
+  type it was built for.
+
+  @param[in]      This          This pointer for
+                                EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL structure
+  @param[in]      ImageBase     The base address in memory of the PE/COFF image
+  @param[in]      ImageSize     The size in memory of the PE/COFF image
+  @param[in,out]  EntryPoint    The entry point of the PE/COFF image. Passed by
+                                reference so that the emulator may modify it.
+
+  @retval EFI_SUCCESS           The image was registered with the emulator and
+                                can be started as usual.
+  @retval other                 The image could not be registered.
+
+  If the PE/COFF machine type or image type are not supported by the emulator,
+  then ASSERT().
+**/
+EFI_STATUS
+EFIAPI
+EbcRegisterImage (
+  IN      EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL    *This,
+  IN      EFI_PHYSICAL_ADDRESS                    ImageBase,
+  IN      UINT64                                  ImageSize,
+  IN  OUT EFI_IMAGE_ENTRY_POINT                   *EntryPoint
+  )
+{
+  DEBUG_CODE_BEGIN ();
+    PE_COFF_LOADER_IMAGE_CONTEXT  ImageContext;
+    EFI_STATUS                    Status;
+
+    ZeroMem (&ImageContext, sizeof (ImageContext));
+
+    ImageContext.Handle    = (VOID *)(UINTN)ImageBase;
+    ImageContext.ImageRead = PeCoffLoaderImageReadFromMemory;
+
+    Status = PeCoffLoaderGetImageInfo (&ImageContext);
+    if (EFI_ERROR (Status)) {
+      return Status;
+    }
+
+    ASSERT (ImageContext.Machine == EFI_IMAGE_MACHINE_EBC);
+    ASSERT (ImageContext.ImageType == EFI_IMAGE_SUBSYSTEM_EFI_APPLICATION ||
+            ImageContext.ImageType == EFI_IMAGE_SUBSYSTEM_EFI_BOOT_SERVICE_DRIVER);
+  DEBUG_CODE_END ();
+
+  EbcRegisterICacheFlush (NULL,
+    (EBC_ICACHE_FLUSH)InvalidateInstructionCacheRange);
+
+  return EbcCreateThunk (NULL, (VOID *)(UINTN)ImageBase, *EntryPoint,
+           (VOID **)EntryPoint);
+}
+
+/**
+  Unregister a PE/COFF image that has been registered with the emulator.
+  This should be done before the image is unloaded from memory.
+
+  @param[in] This         This pointer for EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL
+                          structure
+  @param[in] ImageBase    The base address in memory of the PE/COFF image
+
+  @retval EFI_SUCCESS     The image was unregistered with the emulator.
+  @retval other           Image could not be unloaded.
+**/
+EFI_STATUS
+EFIAPI
+EbcUnregisterImage (
+  IN  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL    *This,
+  IN  EFI_PHYSICAL_ADDRESS                    ImageBase
+  )
+{
+  return EbcUnloadImage (NULL, (VOID *)(UINTN)ImageBase);
+}
+
+EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL mPeCoffEmuProtocol = {
+  EbcIsImageSupported,
+  EbcRegisterImage,
+  EbcUnregisterImage
+};
 
 /**
   Initializes the VM EFI interface.  Allocates memory for the VM interface
@@ -449,6 +563,16 @@ InitializeEbcDriver (
     }
   }
 
+  Status = gBS->InstallProtocolInterface (
+                  &ImageHandle,
+                  &gEdkiiPeCoffImageEmulatorProtocolGuid,
+                  EFI_NATIVE_INTERFACE,
+                  &mPeCoffEmuProtocol
+                  );
+  if (EFI_ERROR(Status)) {
+    goto ErrorExit;
+  }
+
   Status = InitEBCStack();
   if (EFI_ERROR(Status)) {
     goto ErrorExit;
diff --git a/MdeModulePkg/Universal/EbcDxe/EbcInt.h b/MdeModulePkg/Universal/EbcDxe/EbcInt.h
index 8aa7a4abbd63..9b25e91f951c 100644
--- a/MdeModulePkg/Universal/EbcDxe/EbcInt.h
+++ b/MdeModulePkg/Universal/EbcDxe/EbcInt.h
@@ -23,9 +23,12 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #include <Protocol/Ebc.h>
 #include <Protocol/EbcVmTest.h>
 #include <Protocol/EbcSimpleDebugger.h>
+#include <Protocol/PeCoffImageEmulator.h>
 
 #include <Library/BaseLib.h>
+#include <Library/CacheMaintenanceLib.h>
 #include <Library/DebugLib.h>
+#include <Library/PeCoffLib.h>
 #include <Library/UefiDriverEntryPoint.h>
 #include <Library/BaseMemoryLib.h>
 #include <Library/UefiBootServicesTableLib.h>
-- 
2.17.1

[PATCH v2 6/7] MdePkg/UefiBaseType.h: treat EBC as a non-native machine type

[Ard Biesheuvel]

Instead of classifying EBC as a supported machine type and have special
handling in DXE core for loading EBC images, make it a foreign type and
rely on the EDK2 PE/COFF image emulator protocol to claim the image when
the DXE core finds that it cannot be supported natively.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 MdePkg/Include/Uefi/UefiBaseType.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/MdePkg/Include/Uefi/UefiBaseType.h b/MdePkg/Include/Uefi/UefiBaseType.h
index 401db7f620b3..e52121809deb 100644
--- a/MdePkg/Include/Uefi/UefiBaseType.h
+++ b/MdePkg/Include/Uefi/UefiBaseType.h
@@ -250,21 +250,21 @@ typedef union {
 #if   defined (MDE_CPU_IA32)
 
 #define EFI_IMAGE_MACHINE_TYPE_SUPPORTED(Machine) \
-  (((Machine) == EFI_IMAGE_MACHINE_IA32) || ((Machine) == EFI_IMAGE_MACHINE_EBC))
+  ((Machine) == EFI_IMAGE_MACHINE_IA32)
 
 #define EFI_IMAGE_MACHINE_CROSS_TYPE_SUPPORTED(Machine) ((Machine) == EFI_IMAGE_MACHINE_X64)
 
 #elif defined (MDE_CPU_IPF)
 
 #define EFI_IMAGE_MACHINE_TYPE_SUPPORTED(Machine) \
-  (((Machine) == EFI_IMAGE_MACHINE_IA64) || ((Machine) == EFI_IMAGE_MACHINE_EBC))
+  ((Machine) == EFI_IMAGE_MACHINE_IA64)
 
 #define EFI_IMAGE_MACHINE_CROSS_TYPE_SUPPORTED(Machine) (FALSE)
 
 #elif defined (MDE_CPU_X64)
 
 #define EFI_IMAGE_MACHINE_TYPE_SUPPORTED(Machine) \
-  (((Machine) == EFI_IMAGE_MACHINE_X64) || ((Machine) == EFI_IMAGE_MACHINE_EBC))
+  ((Machine) == EFI_IMAGE_MACHINE_X64)
 
 #define EFI_IMAGE_MACHINE_CROSS_TYPE_SUPPORTED(Machine) ((Machine) == EFI_IMAGE_MACHINE_IA32)
 
@@ -277,7 +277,7 @@ typedef union {
 #elif defined (MDE_CPU_AARCH64)
 
 #define EFI_IMAGE_MACHINE_TYPE_SUPPORTED(Machine) \
-  (((Machine) == EFI_IMAGE_MACHINE_AARCH64) || ((Machine) == EFI_IMAGE_MACHINE_EBC))
+  ((Machine) == EFI_IMAGE_MACHINE_AARCH64)
 
 #define EFI_IMAGE_MACHINE_CROSS_TYPE_SUPPORTED(Machine) (FALSE)
 
-- 
2.17.1

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

[Ard Biesheuvel]

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.
-- 
2.17.1

Re: [PATCH v2 0/7] MdeModulePkg: add support for dispatching foreign arch PE/COFF images

[Yao, Jiewen]

HI Ard
This is a great feature.

May I know what test has been done for this patch series?

Would you please share that information? No matter your unit test, or system level test.

Thank you
Yao Jiewen


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Saturday, September 15, 2018 9:29 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Zimmer, Vincent
> <vincent.zimmer@intel.com>; Dong, Eric <eric.dong@intel.com>; Andrew
> Fish <afish@apple.com>; Carsey, Jaben <jaben.carsey@intel.com>;
> Richardson, Brian <brian.richardson@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
> Zeng, Star <star.zeng@intel.com>
> Subject: [edk2] [PATCH v2 0/7] MdeModulePkg: add support for dispatching
> foreign arch PE/COFF images
> 
> Add the basic plumbing to DXE core, the PCI bus driver and the boot manager
> to allow PE/COFF images to be dispatched that target an architecture that is
> not native for the platform, but which is supported by one of potentially
> several available emulators.
> 
> One implementation of such an emulator can be found here:
> https://github.com/ardbiesheuvel/X86EmulatorPkg
> 
> This also allows us to get rid of the special treatment of EBC images in
> core code. Instead, the EbcDxe driver is augmented with an implementation
> of the EDK2 PE/COFF image emulator so that internal knowledge of how EBC
> is implemented (I-cache flushing, thunks) is removed from the DXE core.
> 
> Changes since v2:
> - subsume the EBC handling into the EDK2 emulator protocol and abstract
>   away from EBC specifics in core code.
> - allow multiple emulator implementations to co-exist
> - incorporate Star's review feedback
> 
> Cc: Vincent Zimmer <vincent.zimmer@intel.com>
> Cc: Brian Richardson <brian.richardson@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Andrew Fish <afish@apple.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Jaben Carsey <jaben.carsey@intel.com>
> Cc: Steven Shi <steven.shi@intel.com>
> 
> Ard Biesheuvel (7):
>   MdeModulePkg: introduce PE/COFF image emulator protocol
>   MdeModulePkg/DxeCore: invoke the emulator protocol for foreign
> images
>   MdeModulePkg/PciBusDxe: invoke PE/COFF emulator for foreign option
>     ROMs
>   MdeModulePkg/UefiBootManagerLib: allow foreign Driver#### images
>   MdeModulePkg/EbcDxe: implement the PE/COFF emulator protocol
>   MdePkg/UefiBaseType.h: treat EBC as a non-native machine type
>   MdeModulePkg/DxeCore: remove explicit EBC handling
> 
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h       |   1 +
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf  |   1 +
>  .../Bus/Pci/PciBusDxe/PciOptionRomSupport.c   |  51 ++++++-
>  MdeModulePkg/Core/Dxe/DxeMain.h               |   6 +-
>  MdeModulePkg/Core/Dxe/DxeMain.inf             |   2 +-
>  MdeModulePkg/Core/Dxe/Image/Image.c           | 141
> +++++++++++-------
>  .../Include/Protocol/PeCoffImageEmulator.h    |  99 ++++++++++++
>  .../Library/UefiBootManagerLib/BmLoadOption.c |  51 ++++++-
>  .../Library/UefiBootManagerLib/InternalBm.h   |   1 +
>  .../UefiBootManagerLib/UefiBootManagerLib.inf |   1 +
>  MdeModulePkg/MdeModulePkg.dec                 |   4 +
>  MdeModulePkg/Universal/EbcDxe/EbcDxe.inf      |   3 +
>  MdeModulePkg/Universal/EbcDxe/EbcInt.c        | 124
> +++++++++++++++
>  MdeModulePkg/Universal/EbcDxe/EbcInt.h        |   3 +
>  MdePkg/Include/Uefi/UefiBaseType.h            |   8 +-
>  15 files changed, 432 insertions(+), 64 deletions(-)
>  create mode 100644
> MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h
> 
> --
> 2.17.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

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

[Ni, Ruiyu]

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?

-- 
Thanks,
Ray

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

[Ard Biesheuvel]

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.

On AARCH64 with the X86 emulator installed, EBC and X86 PE/COFF images
can both be dispatched. 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.

Re: [PATCH v2 0/7] MdeModulePkg: add support for dispatching foreign arch PE/COFF images

[Ard Biesheuvel]

On 18 September 2018 at 00:32, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> HI Ard
> This is a great feature.
>

Thanks!

> May I know what test has been done for this patch series?
>
> Would you please share that information? No matter your unit test, or system level test.
>

I have used ArmVirtPkg/ArmVirtQemu.dsc built for AARCH64 in two configurations:
- one replacing the native FAT driver built from source with the EBC
binary version
- another one replacing the FAT driver and including the X86 emulator [0]

The tests involved accessing a FAT partition (both builds) and running
the X86 version of iPXE (the latter build only)



[0] https://github.com/ardbiesheuvel/X86EmulatorPkg


>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
>> Ard Biesheuvel
>> Sent: Saturday, September 15, 2018 9:29 PM
>> To: edk2-devel@lists.01.org
>> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Zimmer, Vincent
>> <vincent.zimmer@intel.com>; Dong, Eric <eric.dong@intel.com>; Andrew
>> Fish <afish@apple.com>; Carsey, Jaben <jaben.carsey@intel.com>;
>> Richardson, Brian <brian.richardson@intel.com>; Gao, Liming
>> <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
>> Zeng, Star <star.zeng@intel.com>
>> Subject: [edk2] [PATCH v2 0/7] MdeModulePkg: add support for dispatching
>> foreign arch PE/COFF images
>>
>> Add the basic plumbing to DXE core, the PCI bus driver and the boot manager
>> to allow PE/COFF images to be dispatched that target an architecture that is
>> not native for the platform, but which is supported by one of potentially
>> several available emulators.
>>
>> One implementation of such an emulator can be found here:
>> https://github.com/ardbiesheuvel/X86EmulatorPkg
>>
>> This also allows us to get rid of the special treatment of EBC images in
>> core code. Instead, the EbcDxe driver is augmented with an implementation
>> of the EDK2 PE/COFF image emulator so that internal knowledge of how EBC
>> is implemented (I-cache flushing, thunks) is removed from the DXE core.
>>
>> Changes since v2:
>> - subsume the EBC handling into the EDK2 emulator protocol and abstract
>>   away from EBC specifics in core code.
>> - allow multiple emulator implementations to co-exist
>> - incorporate Star's review feedback
>>
>> Cc: Vincent Zimmer <vincent.zimmer@intel.com>
>> Cc: Brian Richardson <brian.richardson@intel.com>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Cc: Andrew Fish <afish@apple.com>
>> Cc: Leif Lindholm <leif.lindholm@linaro.org>
>> Cc: Star Zeng <star.zeng@intel.com>
>> Cc: Eric Dong <eric.dong@intel.com>
>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>> Cc: Liming Gao <liming.gao@intel.com>
>> Cc: Jaben Carsey <jaben.carsey@intel.com>
>> Cc: Steven Shi <steven.shi@intel.com>
>>
>> Ard Biesheuvel (7):
>>   MdeModulePkg: introduce PE/COFF image emulator protocol
>>   MdeModulePkg/DxeCore: invoke the emulator protocol for foreign
>> images
>>   MdeModulePkg/PciBusDxe: invoke PE/COFF emulator for foreign option
>>     ROMs
>>   MdeModulePkg/UefiBootManagerLib: allow foreign Driver#### images
>>   MdeModulePkg/EbcDxe: implement the PE/COFF emulator protocol
>>   MdePkg/UefiBaseType.h: treat EBC as a non-native machine type
>>   MdeModulePkg/DxeCore: remove explicit EBC handling
>>
>>  MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h       |   1 +
>>  MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf  |   1 +
>>  .../Bus/Pci/PciBusDxe/PciOptionRomSupport.c   |  51 ++++++-
>>  MdeModulePkg/Core/Dxe/DxeMain.h               |   6 +-
>>  MdeModulePkg/Core/Dxe/DxeMain.inf             |   2 +-
>>  MdeModulePkg/Core/Dxe/Image/Image.c           | 141
>> +++++++++++-------
>>  .../Include/Protocol/PeCoffImageEmulator.h    |  99 ++++++++++++
>>  .../Library/UefiBootManagerLib/BmLoadOption.c |  51 ++++++-
>>  .../Library/UefiBootManagerLib/InternalBm.h   |   1 +
>>  .../UefiBootManagerLib/UefiBootManagerLib.inf |   1 +
>>  MdeModulePkg/MdeModulePkg.dec                 |   4 +
>>  MdeModulePkg/Universal/EbcDxe/EbcDxe.inf      |   3 +
>>  MdeModulePkg/Universal/EbcDxe/EbcInt.c        | 124
>> +++++++++++++++
>>  MdeModulePkg/Universal/EbcDxe/EbcInt.h        |   3 +
>>  MdePkg/Include/Uefi/UefiBaseType.h            |   8 +-
>>  15 files changed, 432 insertions(+), 64 deletions(-)
>>  create mode 100644
>> MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h
>>
>> --
>> 2.17.1
>>
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel

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

[Ni, Ruiyu]

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?


  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

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

[Ard Biesheuvel]

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

Re: [PATCH v2 0/7] MdeModulePkg: add support for dispatching foreign arch PE/COFF images

[Yao, Jiewen]

Thank you Ard. Good to know. 

Did you also try some security test, such as input a bad image to see if the code can return failure gracefully? 

Or enable secure boot to see if the image verification process still works well ?

thank you!
Yao, Jiewen


> 在 2018年9月18日，下午9:53，Ard Biesheuvel <ard.biesheuvel@linaro.org> 写道：
> 
>> On 18 September 2018 at 00:32, Yao, Jiewen <jiewen.yao@intel.com> wrote:
>> HI Ard
>> This is a great feature.
>> 
> 
> Thanks!
> 
>> May I know what test has been done for this patch series?
>> 
>> Would you please share that information? No matter your unit test, or system level test.
>> 
> 
> I have used ArmVirtPkg/ArmVirtQemu.dsc built for AARCH64 in two configurations:
> - one replacing the native FAT driver built from source with the EBC
> binary version
> - another one replacing the FAT driver and including the X86 emulator [0]
> 
> The tests involved accessing a FAT partition (both builds) and running
> the X86 version of iPXE (the latter build only)
> 
> 
> 
> [0] https://github.com/ardbiesheuvel/X86EmulatorPkg
> 
> 
>>> -----Original Message-----
>>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
>>> Ard Biesheuvel
>>> Sent: Saturday, September 15, 2018 9:29 PM
>>> To: edk2-devel@lists.01.org
>>> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Zimmer, Vincent
>>> <vincent.zimmer@intel.com>; Dong, Eric <eric.dong@intel.com>; Andrew
>>> Fish <afish@apple.com>; Carsey, Jaben <jaben.carsey@intel.com>;
>>> Richardson, Brian <brian.richardson@intel.com>; Gao, Liming
>>> <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
>>> Zeng, Star <star.zeng@intel.com>
>>> Subject: [edk2] [PATCH v2 0/7] MdeModulePkg: add support for dispatching
>>> foreign arch PE/COFF images
>>> 
>>> Add the basic plumbing to DXE core, the PCI bus driver and the boot manager
>>> to allow PE/COFF images to be dispatched that target an architecture that is
>>> not native for the platform, but which is supported by one of potentially
>>> several available emulators.
>>> 
>>> One implementation of such an emulator can be found here:
>>> https://github.com/ardbiesheuvel/X86EmulatorPkg
>>> 
>>> This also allows us to get rid of the special treatment of EBC images in
>>> core code. Instead, the EbcDxe driver is augmented with an implementation
>>> of the EDK2 PE/COFF image emulator so that internal knowledge of how EBC
>>> is implemented (I-cache flushing, thunks) is removed from the DXE core.
>>> 
>>> Changes since v2:
>>> - subsume the EBC handling into the EDK2 emulator protocol and abstract
>>>  away from EBC specifics in core code.
>>> - allow multiple emulator implementations to co-exist
>>> - incorporate Star's review feedback
>>> 
>>> Cc: Vincent Zimmer <vincent.zimmer@intel.com>
>>> Cc: Brian Richardson <brian.richardson@intel.com>
>>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>>> Cc: Andrew Fish <afish@apple.com>
>>> Cc: Leif Lindholm <leif.lindholm@linaro.org>
>>> Cc: Star Zeng <star.zeng@intel.com>
>>> Cc: Eric Dong <eric.dong@intel.com>
>>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>>> Cc: Liming Gao <liming.gao@intel.com>
>>> Cc: Jaben Carsey <jaben.carsey@intel.com>
>>> Cc: Steven Shi <steven.shi@intel.com>
>>> 
>>> Ard Biesheuvel (7):
>>>  MdeModulePkg: introduce PE/COFF image emulator protocol
>>>  MdeModulePkg/DxeCore: invoke the emulator protocol for foreign
>>> images
>>>  MdeModulePkg/PciBusDxe: invoke PE/COFF emulator for foreign option
>>>    ROMs
>>>  MdeModulePkg/UefiBootManagerLib: allow foreign Driver#### images
>>>  MdeModulePkg/EbcDxe: implement the PE/COFF emulator protocol
>>>  MdePkg/UefiBaseType.h: treat EBC as a non-native machine type
>>>  MdeModulePkg/DxeCore: remove explicit EBC handling
>>> 
>>> MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h       |   1 +
>>> MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf  |   1 +
>>> .../Bus/Pci/PciBusDxe/PciOptionRomSupport.c   |  51 ++++++-
>>> MdeModulePkg/Core/Dxe/DxeMain.h               |   6 +-
>>> MdeModulePkg/Core/Dxe/DxeMain.inf             |   2 +-
>>> MdeModulePkg/Core/Dxe/Image/Image.c           | 141
>>> +++++++++++-------
>>> .../Include/Protocol/PeCoffImageEmulator.h    |  99 ++++++++++++
>>> .../Library/UefiBootManagerLib/BmLoadOption.c |  51 ++++++-
>>> .../Library/UefiBootManagerLib/InternalBm.h   |   1 +
>>> .../UefiBootManagerLib/UefiBootManagerLib.inf |   1 +
>>> MdeModulePkg/MdeModulePkg.dec                 |   4 +
>>> MdeModulePkg/Universal/EbcDxe/EbcDxe.inf      |   3 +
>>> MdeModulePkg/Universal/EbcDxe/EbcInt.c        | 124
>>> +++++++++++++++
>>> MdeModulePkg/Universal/EbcDxe/EbcInt.h        |   3 +
>>> MdePkg/Include/Uefi/UefiBaseType.h            |   8 +-
>>> 15 files changed, 432 insertions(+), 64 deletions(-)
>>> create mode 100644
>>> MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h
>>> 
>>> --
>>> 2.17.1
>>> 
>>> _______________________________________________
>>> edk2-devel mailing list
>>> edk2-devel@lists.01.org
>>> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH v2 0/7] MdeModulePkg: add support for dispatching foreign arch PE/COFF images

[Yao, Jiewen]

One more, did you enable tpm to see if tpm measurement still works well ?

Also did defer image solution still takes effect with this change?

Sorry to ask many questions, I want to make sure the current security design still work with this new capability. 

thank you!
Yao, Jiewen


> 在 2018年9月19日，下午5:30，Yao, Jiewen <jiewen.yao@intel.com> 写道：
> 
> Thank you Ard. Good to know. 
> 
> Did you also try some security test, such as input a bad image to see if the code can return failure gracefully? 
> 
> Or enable secure boot to see if the image verification process still works well ?
> 
> thank you!
> Yao, Jiewen
> 
> 
>>> 在 2018年9月18日，下午9:53，Ard Biesheuvel <ard.biesheuvel@linaro.org> 写道：
>>> 
>>> On 18 September 2018 at 00:32, Yao, Jiewen <jiewen.yao@intel.com> wrote:
>>> HI Ard
>>> This is a great feature.
>>> 
>> 
>> Thanks!
>> 
>>> May I know what test has been done for this patch series?
>>> 
>>> Would you please share that information? No matter your unit test, or system level test.
>>> 
>> 
>> I have used ArmVirtPkg/ArmVirtQemu.dsc built for AARCH64 in two configurations:
>> - one replacing the native FAT driver built from source with the EBC
>> binary version
>> - another one replacing the FAT driver and including the X86 emulator [0]
>> 
>> The tests involved accessing a FAT partition (both builds) and running
>> the X86 version of iPXE (the latter build only)
>> 
>> 
>> 
>> [0] https://github.com/ardbiesheuvel/X86EmulatorPkg
>> 
>> 
>>>> -----Original Message-----
>>>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
>>>> Ard Biesheuvel
>>>> Sent: Saturday, September 15, 2018 9:29 PM
>>>> To: edk2-devel@lists.01.org
>>>> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Zimmer, Vincent
>>>> <vincent.zimmer@intel.com>; Dong, Eric <eric.dong@intel.com>; Andrew
>>>> Fish <afish@apple.com>; Carsey, Jaben <jaben.carsey@intel.com>;
>>>> Richardson, Brian <brian.richardson@intel.com>; Gao, Liming
>>>> <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
>>>> Zeng, Star <star.zeng@intel.com>
>>>> Subject: [edk2] [PATCH v2 0/7] MdeModulePkg: add support for dispatching
>>>> foreign arch PE/COFF images
>>>> 
>>>> Add the basic plumbing to DXE core, the PCI bus driver and the boot manager
>>>> to allow PE/COFF images to be dispatched that target an architecture that is
>>>> not native for the platform, but which is supported by one of potentially
>>>> several available emulators.
>>>> 
>>>> One implementation of such an emulator can be found here:
>>>> https://github.com/ardbiesheuvel/X86EmulatorPkg
>>>> 
>>>> This also allows us to get rid of the special treatment of EBC images in
>>>> core code. Instead, the EbcDxe driver is augmented with an implementation
>>>> of the EDK2 PE/COFF image emulator so that internal knowledge of how EBC
>>>> is implemented (I-cache flushing, thunks) is removed from the DXE core.
>>>> 
>>>> Changes since v2:
>>>> - subsume the EBC handling into the EDK2 emulator protocol and abstract
>>>> away from EBC specifics in core code.
>>>> - allow multiple emulator implementations to co-exist
>>>> - incorporate Star's review feedback
>>>> 
>>>> Cc: Vincent Zimmer <vincent.zimmer@intel.com>
>>>> Cc: Brian Richardson <brian.richardson@intel.com>
>>>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>>>> Cc: Andrew Fish <afish@apple.com>
>>>> Cc: Leif Lindholm <leif.lindholm@linaro.org>
>>>> Cc: Star Zeng <star.zeng@intel.com>
>>>> Cc: Eric Dong <eric.dong@intel.com>
>>>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>>>> Cc: Liming Gao <liming.gao@intel.com>
>>>> Cc: Jaben Carsey <jaben.carsey@intel.com>
>>>> Cc: Steven Shi <steven.shi@intel.com>
>>>> 
>>>> Ard Biesheuvel (7):
>>>> MdeModulePkg: introduce PE/COFF image emulator protocol
>>>> MdeModulePkg/DxeCore: invoke the emulator protocol for foreign
>>>> images
>>>> MdeModulePkg/PciBusDxe: invoke PE/COFF emulator for foreign option
>>>>   ROMs
>>>> MdeModulePkg/UefiBootManagerLib: allow foreign Driver#### images
>>>> MdeModulePkg/EbcDxe: implement the PE/COFF emulator protocol
>>>> MdePkg/UefiBaseType.h: treat EBC as a non-native machine type
>>>> MdeModulePkg/DxeCore: remove explicit EBC handling
>>>> 
>>>> MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h       |   1 +
>>>> MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf  |   1 +
>>>> .../Bus/Pci/PciBusDxe/PciOptionRomSupport.c   |  51 ++++++-
>>>> MdeModulePkg/Core/Dxe/DxeMain.h               |   6 +-
>>>> MdeModulePkg/Core/Dxe/DxeMain.inf             |   2 +-
>>>> MdeModulePkg/Core/Dxe/Image/Image.c           | 141
>>>> +++++++++++-------
>>>> .../Include/Protocol/PeCoffImageEmulator.h    |  99 ++++++++++++
>>>> .../Library/UefiBootManagerLib/BmLoadOption.c |  51 ++++++-
>>>> .../Library/UefiBootManagerLib/InternalBm.h   |   1 +
>>>> .../UefiBootManagerLib/UefiBootManagerLib.inf |   1 +
>>>> MdeModulePkg/MdeModulePkg.dec                 |   4 +
>>>> MdeModulePkg/Universal/EbcDxe/EbcDxe.inf      |   3 +
>>>> MdeModulePkg/Universal/EbcDxe/EbcInt.c        | 124
>>>> +++++++++++++++
>>>> MdeModulePkg/Universal/EbcDxe/EbcInt.h        |   3 +
>>>> MdePkg/Include/Uefi/UefiBaseType.h            |   8 +-
>>>> 15 files changed, 432 insertions(+), 64 deletions(-)
>>>> create mode 100644
>>>> MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h
>>>> 
>>>> --
>>>> 2.17.1
>>>> 
>>>> _______________________________________________
>>>> edk2-devel mailing list
>>>> edk2-devel@lists.01.org
>>>> https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH v2 0/7] MdeModulePkg: add support for dispatching foreign arch PE/COFF images

[Ard Biesheuvel]

On 19 September 2018 at 02:29, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> Thank you Ard. Good to know.
>
> Did you also try some security test, such as input a bad image to see if the code can return failure gracefully?
>
> Or enable secure boot to see if the image verification process still works well ?
>
> One more, did you enable tpm to see if tpm measurement still works well ?
>
> Also did defer image solution still takes effect with this change?
>
> Sorry to ask many questions, I want to make sure the current security design still work with this new capability.
>

Hello Jiewen,

As far as I can tell, all the security checks are done *before*
CoreLoadPeImage() is called, and the code flow has not changed at all
before that point.

Re: [PATCH v2 0/7] MdeModulePkg: add support for dispatching foreign arch PE/COFF images

[Yao, Jiewen]

Ok. Cool!


> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Wednesday, September 19, 2018 9:55 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Zimmer, Vincent
> <vincent.zimmer@intel.com>; Dong, Eric <eric.dong@intel.com>;
> edk2-devel@lists.01.org; Andrew Fish <afish@apple.com>; Gao, Liming
> <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
> Richardson, Brian <brian.richardson@intel.com>; Carsey, Jaben
> <jaben.carsey@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: Re: [edk2] [PATCH v2 0/7] MdeModulePkg: add support for
> dispatching foreign arch PE/COFF images
> 
> On 19 September 2018 at 02:29, Yao, Jiewen <jiewen.yao@intel.com>
> wrote:
> > Thank you Ard. Good to know.
> >
> > Did you also try some security test, such as input a bad image to see if the
> code can return failure gracefully?
> >
> > Or enable secure boot to see if the image verification process still works
> well ?
> >
> > One more, did you enable tpm to see if tpm measurement still works well ?
> >
> > Also did defer image solution still takes effect with this change?
> >
> > Sorry to ask many questions, I want to make sure the current security
> design still work with this new capability.
> >
> 
> Hello Jiewen,
> 
> As far as I can tell, all the security checks are done *before*
> CoreLoadPeImage() is called, and the code flow has not changed at all
> before that point.