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

[PATCH 0/4] 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 an emulator.

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

Cc: Zimmer Vincent <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: Laszlo Ersek <lersek@redhat.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>

Ard Biesheuvel (4):
  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/Bus/Pci/PciBusDxe/PciBus.h       |  1 +
 MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf  |  1 +
 .../Bus/Pci/PciBusDxe/PciOptionRomSupport.c   | 16 +++++-
 MdeModulePkg/Core/Dxe/DxeMain.h               |  3 ++
 MdeModulePkg/Core/Dxe/DxeMain.inf             |  1 +
 MdeModulePkg/Core/Dxe/Image/Image.c           | 39 +++++++++++---
 .../Include/Protocol/PeCoffImageEmulator.h    | 51 +++++++++++++++++++
 .../Library/UefiBootManagerLib/BmLoadOption.c | 26 +++++++++-
 .../Library/UefiBootManagerLib/InternalBm.h   |  1 +
 .../UefiBootManagerLib/UefiBootManagerLib.inf |  1 +
 MdeModulePkg/MdeModulePkg.dec                 |  4 ++
 11 files changed, 136 insertions(+), 8 deletions(-)
 create mode 100644 MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h

-- 
2.17.1

[PATCH 1/4] 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 | 51 ++++++++++++++++++++
 MdeModulePkg/MdeModulePkg.dec                       |  4 ++
 2 files changed, 55 insertions(+)

diff --git a/MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h b/MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h
new file mode 100644
index 000000000000..3391e68557b9
--- /dev/null
+++ b/MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h
@@ -0,0 +1,51 @@
+/** @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;
+
+typedef
+BOOLEAN
+(EFIAPI *IS_PECOFF_IMAGE_SUPPORTED) (
+  IN  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL    *This,
+  IN  UINT16                                  MachineType,
+  IN  UINT16                                  ImageType
+  );
+
+typedef
+EFI_STATUS
+(EFIAPI *REGISTER_PECOFF_IMAGE) (
+  IN  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL    *This,
+  IN  EFI_PHYSICAL_ADDRESS                    ImageBase,
+  IN  UINT64                                  ImageSize
+  );
+
+typedef
+EFI_STATUS
+(EFIAPI *UNREGISTER_PECOFF_IMAGE) (
+  IN  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL    *This,
+  IN  EFI_PHYSICAL_ADDRESS                    ImageBase
+  );
+
+typedef struct _EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL {
+  IS_PECOFF_IMAGE_SUPPORTED                   IsImageSupported;
+  REGISTER_PECOFF_IMAGE                       RegisterImage;
+  UNREGISTER_PECOFF_IMAGE                     UnregisterImage;
+} EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL;
+
+#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 2/4] 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 | 39 +++++++++++++++++---
 3 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h b/MdeModulePkg/Core/Dxe/DxeMain.h
index 7ec82388a3f9..57b3861d9813 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  *Emu;
   /// 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..d7591aa0da6d 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.inf
+++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
@@ -180,6 +180,7 @@
   gEfiVariableArchProtocolGuid                  ## CONSUMES
   gEfiCapsuleArchProtocolGuid                   ## CONSUMES
   gEfiWatchdogTimerArchProtocolGuid             ## CONSUMES
+  gEdkiiPeCoffImageEmulatorProtocolGuid         ## SOMETIMES_CONSUMES
 
 [FeaturePcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdFrameworkCompatibilitySupport     ## CONSUMES
diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c b/MdeModulePkg/Core/Dxe/Image/Image.c
index eddca140ee1a..e2dd80790657 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,                       // Emu
   NULL,                       // RuntimeData
   NULL                        // LoadedImageDevicePath
 };
@@ -476,12 +477,23 @@ CoreLoadPeImage (
   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.
+      // Locate the emulator protocol to check whether it supports this
+      // image.
       //
-      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;
+      Status = CoreLocateProtocol (&gEdkiiPeCoffImageEmulatorProtocolGuid,
+                 NULL, (VOID **)&Image->Emu);
+      if (EFI_ERROR (Status) ||
+          !Image->Emu->IsImageSupported (Image->Emu,
+                                         Image->ImageContext.Machine,
+                                         Image->ImageContext.ImageType)) {
+        //
+        // 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 +699,14 @@ CoreLoadPeImage (
     if (EFI_ERROR(Status)) {
       goto Done;
     }
+  } else if (Image->Emu != NULL) {
+    Status = Image->Emu->RegisterImage (Image->Emu, Image->ImageBasePage,
+                           EFI_PAGES_TO_SIZE (Image->NumberOfPages));
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_LOAD | DEBUG_ERROR,
+        "CoreLoadPeImage: Failed to load register foreign image with emulator.\n"));
+      goto Done;
+    }
   }
 
   //
@@ -874,6 +894,13 @@ CoreUnloadAndCloseImage (
     Image->Ebc->UnloadImage (Image->Ebc, Image->Handle);
   }
 
+  if (Image->Emu != NULL) {
+    //
+    // If the PE/COFF Emulator protocol exists we must unregister the image.
+    //
+    Image->Emu->UnregisterImage (Image->Emu, Image->ImageBasePage);
+  }
+
   //
   // Unload image, free Image->ImageContext->ModHandle
   //
@@ -1599,7 +1626,7 @@ 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->Emu == 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 3/4] 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 | 16 +++++++++++++++-
 3 files changed, 17 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..3d99ea0c1047 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
@@ -97,6 +97,7 @@
   gEfiLoadFile2ProtocolGuid                       ## SOMETIMES_PRODUCES
   gEdkiiIoMmuProtocolGuid                         ## SOMETIMES_CONSUMES
   gEfiLoadedImageDevicePathProtocolGuid           ## CONSUMES
+  gEdkiiPeCoffImageEmulatorProtocolGuid           ## SOMETIMES_CONSUMES
 
 [FeaturePcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdPciBusHotplugDeviceSupport      ## CONSUMES
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
index c2be85a906af..07236afd327d 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
@@ -678,6 +678,7 @@ ProcessOpRomImage (
   MEDIA_RELATIVE_OFFSET_RANGE_DEVICE_PATH  EfiOpRomImageNode;
   VOID                                     *Buffer;
   UINTN                                    BufferSize;
+  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL     *PeCoffEmulator;
 
   Indicator = 0;
 
@@ -693,6 +694,7 @@ ProcessOpRomImage (
   }
   ASSERT (((EFI_PCI_EXPANSION_ROM_HEADER *) RomBarOffset)->Signature == PCI_EXPANSION_ROM_HEADER_SIGNATURE);
 
+  PeCoffEmulator = NULL;
   do {
     EfiRomHeader = (EFI_PCI_EXPANSION_ROM_HEADER *) RomBarOffset;
     if (EfiRomHeader->Signature != PCI_EXPANSION_ROM_HEADER_SIGNATURE) {
@@ -716,7 +718,19 @@ ProcessOpRomImage (
     // Skip the EFI PCI Option ROM image if its machine type is not supported
     //
     if (!EFI_IMAGE_MACHINE_TYPE_SUPPORTED (EfiRomHeader->EfiMachineType)) {
-      goto NextImage;
+      //
+      // Check whether we have a PE/COFF emulator that supports this image
+      //
+      if (PeCoffEmulator == NULL) {
+        gBS->LocateProtocol (&gEdkiiPeCoffImageEmulatorProtocolGuid, NULL,
+               (VOID **)&PeCoffEmulator);
+      }
+      if (PeCoffEmulator == NULL ||
+          !PeCoffEmulator->IsImageSupported (PeCoffEmulator,
+                             EfiRomHeader->EfiMachineType,
+                             EfiRomHeader->EfiSubsystem)) {
+        goto NextImage;
+      }
     }
 
     //
-- 
2.17.1

[PATCH 4/4] 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         | 26 +++++++++++++++++++-
 MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h           |  1 +
 MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf |  1 +
 3 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
index 7bf96646c690..67e3dfd7e9e1 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
@@ -1185,6 +1185,29 @@ EfiBootManagerFreeLoadOptions (
   return EFI_SUCCESS;
 }
 
+STATIC
+BOOLEAN
+BmIsImageTypeSupported (
+  IN  UINT16    MachineType,
+  IN  UINT16    SubSystem
+  )
+{
+  EFI_STATUS                            Status;
+  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL  *Emu;
+
+  if (EFI_IMAGE_MACHINE_TYPE_SUPPORTED (MachineType)) {
+    return TRUE;
+  }
+
+  Status = gBS->LocateProtocol (&gEdkiiPeCoffImageEmulatorProtocolGuid,
+                  NULL, (VOID **)&Emu);
+  if (EFI_ERROR (Status)) {
+    return FALSE;
+  }
+
+  return Emu->IsImageSupported (Emu, MachineType, SubSystem);
+}
+
 /**
   Return whether the PE header of the load option is valid or not.
 
@@ -1235,7 +1258,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

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

[Gao, Liming]

Ard:
  What's purpose to run the not native image in emulator? And, what should be done in the emulator driver, such as X64Emulator? Is the emulator responsible to translate the instruction between the different arch? I see X64Emualtor bases on qemu code. 

Thanks
Liming
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
> Sent: Wednesday, September 12, 2018 9:22 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>; agraf@suse.de; Richardson, Brian <brian.richardson@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Laszlo Ersek <lersek@redhat.com>; Zeng, Star <star.zeng@intel.com>
> Subject: [edk2] [PATCH 0/4] 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 an emulator.
> 
> One implementation of such an emulator can be found here:
> https://github.com/ardbiesheuvel/X86EmulatorPkg
> 
> Cc: Zimmer Vincent <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: Laszlo Ersek <lersek@redhat.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>
> 
> Ard Biesheuvel (4):
>   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/Bus/Pci/PciBusDxe/PciBus.h       |  1 +
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf  |  1 +
>  .../Bus/Pci/PciBusDxe/PciOptionRomSupport.c   | 16 +++++-
>  MdeModulePkg/Core/Dxe/DxeMain.h               |  3 ++
>  MdeModulePkg/Core/Dxe/DxeMain.inf             |  1 +
>  MdeModulePkg/Core/Dxe/Image/Image.c           | 39 +++++++++++---
>  .../Include/Protocol/PeCoffImageEmulator.h    | 51 +++++++++++++++++++
>  .../Library/UefiBootManagerLib/BmLoadOption.c | 26 +++++++++-
>  .../Library/UefiBootManagerLib/InternalBm.h   |  1 +
>  .../UefiBootManagerLib/UefiBootManagerLib.inf |  1 +
>  MdeModulePkg/MdeModulePkg.dec                 |  4 ++
>  11 files changed, 136 insertions(+), 8 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 0/4] MdeModulePkg: add support for dispatching foreign arch PE/COFF images

[Ard Biesheuvel]

On 12 September 2018 at 16:55, Gao, Liming <liming.gao@intel.com> wrote:
> Ard:
>   What's purpose to run the not native image in emulator? And, what should be done in the emulator driver, such as X64Emulator? Is the emulator responsible to translate the instruction between the different arch? I see X64Emualtor bases on qemu code.
>

The primary use case is option ROMs on PCIe cards.

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

[Carsey, Jaben]

Ard,

What happens when more than one emulators want to co-exist?

-Jaben

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Wednesday, September 12, 2018 7:57 AM
> To: Gao, Liming <liming.gao@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; agraf@suse.de; Andrew Fish <afish@apple.com>;
> Richardson, Brian <brian.richardson@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Laszlo Ersek <lersek@redhat.com>; Zeng,
> Star <star.zeng@intel.com>
> Subject: Re: [edk2] [PATCH 0/4] MdeModulePkg: add support for dispatching
> foreign arch PE/COFF images
> 
> On 12 September 2018 at 16:55, Gao, Liming <liming.gao@intel.com> wrote:
> > Ard:
> >   What's purpose to run the not native image in emulator? And, what
> should be done in the emulator driver, such as X64Emulator? Is the emulator
> responsible to translate the instruction between the different arch? I see
> X64Emualtor bases on qemu code.
> >
> 
> The primary use case is option ROMs on PCIe cards.
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

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

[Kinney, Michael D]

Ard,

I think there may be a lot of assumptions in this
proposal that are not documented.  

I do not see any description of how an image is 
started or how calls between native code and emulated
code are handled.

Also, are multiple emulated CPUs supported?  It looks
like there can only be one instance of this new protocol.

Can you please provide more detailed comments for the
functions in the new protocol and document the assumptions
and known limitation in the header?

>From one perspective, EBC is an emulated instruction set.
Would it make sense to have EBC be one of the emulated 
CPU types that can be plugged in?

Thanks,

Mike

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Wednesday, September 12, 2018 6:22 AM
> To: edk2-devel@lists.01.org
> Cc: agraf@suse.de; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>; Zimmer, Vincent
> <vincent.zimmer@intel.com>; Richardson, Brian
> <brian.richardson@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Andrew Fish
> <afish@apple.com>; Laszlo Ersek <lersek@redhat.com>;
> Leif Lindholm <leif.lindholm@linaro.org>; Zeng, Star
> <star.zeng@intel.com>; Dong, Eric <eric.dong@intel.com>;
> Ni, Ruiyu <ruiyu.ni@intel.com>
> Subject: [PATCH 0/4] 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
> an emulator.
> 
> One implementation of such an emulator can be found
> here:
> https://github.com/ardbiesheuvel/X86EmulatorPkg
> 
> Cc: Zimmer Vincent <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: Laszlo Ersek <lersek@redhat.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>
> 
> Ard Biesheuvel (4):
>   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/Bus/Pci/PciBusDxe/PciBus.h       |  1 +
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf  |  1 +
>  .../Bus/Pci/PciBusDxe/PciOptionRomSupport.c   | 16
> +++++-
>  MdeModulePkg/Core/Dxe/DxeMain.h               |  3 ++
>  MdeModulePkg/Core/Dxe/DxeMain.inf             |  1 +
>  MdeModulePkg/Core/Dxe/Image/Image.c           | 39
> +++++++++++---
>  .../Include/Protocol/PeCoffImageEmulator.h    | 51
> +++++++++++++++++++
>  .../Library/UefiBootManagerLib/BmLoadOption.c | 26
> +++++++++-
>  .../Library/UefiBootManagerLib/InternalBm.h   |  1 +
>  .../UefiBootManagerLib/UefiBootManagerLib.inf |  1 +
>  MdeModulePkg/MdeModulePkg.dec                 |  4 ++
>  11 files changed, 136 insertions(+), 8 deletions(-)
>  create mode 100644
> MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h
> 
> --
> 2.17.1

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

[Ard Biesheuvel]

On 12 September 2018 at 17:07, Carsey, Jaben <jaben.carsey@intel.com> wrote:
> Ard,
>
> What happens when more than one emulators want to co-exist?
>

The protocol does not support that at the moment, and it is doubtful
that it would work in practice: the X86-on-AARCH64 emulator works by
mapping the X86 PE/COFF code regions as non-executable, and trapping
the resulting exception to invoke the emulator. Running multiple
emulators side by side would require a considerable amount of
synchronization, so it only makes sense to consider that if there is a
compelling use case. And if there is, we could still abstract from
this at the top level (i.e., this protocol)

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

[Carsey, Jaben]

Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>

Code looks good to me.


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Wednesday, September 12, 2018 6:22 AM
> 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>; agraf@suse.de; Richardson, Brian
> <brian.richardson@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Laszlo Ersek <lersek@redhat.com>; Zeng,
> Star <star.zeng@intel.com>
> Subject: [edk2] [PATCH 0/4] 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 an emulator.
> 
> One implementation of such an emulator can be found here:
> https://github.com/ardbiesheuvel/X86EmulatorPkg
> 
> Cc: Zimmer Vincent <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: Laszlo Ersek <lersek@redhat.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>
> 
> Ard Biesheuvel (4):
>   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/Bus/Pci/PciBusDxe/PciBus.h       |  1 +
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf  |  1 +
>  .../Bus/Pci/PciBusDxe/PciOptionRomSupport.c   | 16 +++++-
>  MdeModulePkg/Core/Dxe/DxeMain.h               |  3 ++
>  MdeModulePkg/Core/Dxe/DxeMain.inf             |  1 +
>  MdeModulePkg/Core/Dxe/Image/Image.c           | 39 +++++++++++---
>  .../Include/Protocol/PeCoffImageEmulator.h    | 51 +++++++++++++++++++
>  .../Library/UefiBootManagerLib/BmLoadOption.c | 26 +++++++++-
>  .../Library/UefiBootManagerLib/InternalBm.h   |  1 +
>  .../UefiBootManagerLib/UefiBootManagerLib.inf |  1 +
>  MdeModulePkg/MdeModulePkg.dec                 |  4 ++
>  11 files changed, 136 insertions(+), 8 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 0/4] MdeModulePkg: add support for dispatching foreign arch PE/COFF images

[Zimmer, Vincent]

I like this approach too

Vincent

> On Sep 12, 2018, at 5:48 PM, Carsey, Jaben <jaben.carsey@intel.com> wrote:
> 
> Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
> 
> Code looks good to me.
> 
> 
>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
>> Ard Biesheuvel
>> Sent: Wednesday, September 12, 2018 6:22 AM
>> 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>; agraf@suse.de; Richardson, Brian
>> <brian.richardson@intel.com>; Kinney, Michael D
>> <michael.d.kinney@intel.com>; Laszlo Ersek <lersek@redhat.com>; Zeng,
>> Star <star.zeng@intel.com>
>> Subject: [edk2] [PATCH 0/4] 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 an emulator.
>> 
>> One implementation of such an emulator can be found here:
>> https://github.com/ardbiesheuvel/X86EmulatorPkg
>> 
>> Cc: Zimmer Vincent <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: Laszlo Ersek <lersek@redhat.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>
>> 
>> Ard Biesheuvel (4):
>>  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/Bus/Pci/PciBusDxe/PciBus.h       |  1 +
>> MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf  |  1 +
>> .../Bus/Pci/PciBusDxe/PciOptionRomSupport.c   | 16 +++++-
>> MdeModulePkg/Core/Dxe/DxeMain.h               |  3 ++
>> MdeModulePkg/Core/Dxe/DxeMain.inf             |  1 +
>> MdeModulePkg/Core/Dxe/Image/Image.c           | 39 +++++++++++---
>> .../Include/Protocol/PeCoffImageEmulator.h    | 51 +++++++++++++++++++
>> .../Library/UefiBootManagerLib/BmLoadOption.c | 26 +++++++++-
>> .../Library/UefiBootManagerLib/InternalBm.h   |  1 +
>> .../UefiBootManagerLib/UefiBootManagerLib.inf |  1 +
>> MdeModulePkg/MdeModulePkg.dec                 |  4 ++
>> 11 files changed, 136 insertions(+), 8 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 0/4] MdeModulePkg: add support for dispatching foreign arch PE/COFF images

[Shi, Steven]

Hi Ard,
Just my curious, are you supporting the below idea of QEMU in UEFI?

QEMU in UEFI - Alexander Graf
https://www.youtube.com/watch?v=uxvAH1Q4Mx0
http://events17.linuxfoundation.org/sites/events/files/slides/QEMU%20in%20UEFI.pdf



Steven Shi
Intel\SSG\STO\UEFI Firmware

Tel: +86 021-61166522
iNet: 821-6522

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Wednesday, September 12, 2018 9:22 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>; agraf@suse.de; Richardson, Brian
> <brian.richardson@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Laszlo Ersek <lersek@redhat.com>; Zeng,
> Star <star.zeng@intel.com>
> Subject: [edk2] [PATCH 0/4] 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 an emulator.
> 
> One implementation of such an emulator can be found here:
> https://github.com/ardbiesheuvel/X86EmulatorPkg
> 
> Cc: Zimmer Vincent <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: Laszlo Ersek <lersek@redhat.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>
> 
> Ard Biesheuvel (4):
>   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/Bus/Pci/PciBusDxe/PciBus.h       |  1 +
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf  |  1 +
>  .../Bus/Pci/PciBusDxe/PciOptionRomSupport.c   | 16 +++++-
>  MdeModulePkg/Core/Dxe/DxeMain.h               |  3 ++
>  MdeModulePkg/Core/Dxe/DxeMain.inf             |  1 +
>  MdeModulePkg/Core/Dxe/Image/Image.c           | 39 +++++++++++---
>  .../Include/Protocol/PeCoffImageEmulator.h    | 51 +++++++++++++++++++
>  .../Library/UefiBootManagerLib/BmLoadOption.c | 26 +++++++++-
>  .../Library/UefiBootManagerLib/InternalBm.h   |  1 +
>  .../UefiBootManagerLib/UefiBootManagerLib.inf |  1 +
>  MdeModulePkg/MdeModulePkg.dec                 |  4 ++
>  11 files changed, 136 insertions(+), 8 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 0/4] MdeModulePkg: add support for dispatching foreign arch PE/COFF images

[Ard Biesheuvel]

On 13 September 2018 at 02:47, Shi, Steven <steven.shi@intel.com> wrote:
> Hi Ard,
> Just my curious, are you supporting the below idea of QEMU in UEFI?
>
> QEMU in UEFI - Alexander Graf
> https://www.youtube.com/watch?v=uxvAH1Q4Mx0
> http://events17.linuxfoundation.org/sites/events/files/slides/QEMU%20in%20UEFI.pdf
>

Yes. X86EmulatorPkg was developed by Alexander and myself.


>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
>> Ard Biesheuvel
>> Sent: Wednesday, September 12, 2018 9:22 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>; agraf@suse.de; Richardson, Brian
>> <brian.richardson@intel.com>; Kinney, Michael D
>> <michael.d.kinney@intel.com>; Laszlo Ersek <lersek@redhat.com>; Zeng,
>> Star <star.zeng@intel.com>
>> Subject: [edk2] [PATCH 0/4] 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 an emulator.
>>
>> One implementation of such an emulator can be found here:
>> https://github.com/ardbiesheuvel/X86EmulatorPkg
>>
>> Cc: Zimmer Vincent <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: Laszlo Ersek <lersek@redhat.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>
>>
>> Ard Biesheuvel (4):
>>   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/Bus/Pci/PciBusDxe/PciBus.h       |  1 +
>>  MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf  |  1 +
>>  .../Bus/Pci/PciBusDxe/PciOptionRomSupport.c   | 16 +++++-
>>  MdeModulePkg/Core/Dxe/DxeMain.h               |  3 ++
>>  MdeModulePkg/Core/Dxe/DxeMain.inf             |  1 +
>>  MdeModulePkg/Core/Dxe/Image/Image.c           | 39 +++++++++++---
>>  .../Include/Protocol/PeCoffImageEmulator.h    | 51 +++++++++++++++++++
>>  .../Library/UefiBootManagerLib/BmLoadOption.c | 26 +++++++++-
>>  .../Library/UefiBootManagerLib/InternalBm.h   |  1 +
>>  .../UefiBootManagerLib/UefiBootManagerLib.inf |  1 +
>>  MdeModulePkg/MdeModulePkg.dec                 |  4 ++
>>  11 files changed, 136 insertions(+), 8 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 1/4] MdeModulePkg: introduce PE/COFF image emulator protocol

[Zeng, Star]

On 2018/9/12 21:21, Ard Biesheuvel wrote:
> 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 | 51 ++++++++++++++++++++
>   MdeModulePkg/MdeModulePkg.dec                       |  4 ++
>   2 files changed, 55 insertions(+)
> 
> diff --git a/MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h b/MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h
> new file mode 100644
> index 000000000000..3391e68557b9
> --- /dev/null
> +++ b/MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h
> @@ -0,0 +1,51 @@
> +/** @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;
> +
> +typedef
> +BOOLEAN
> +(EFIAPI *IS_PECOFF_IMAGE_SUPPORTED) (
> +  IN  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL    *This,
> +  IN  UINT16                                  MachineType,
> +  IN  UINT16                                  ImageType
> +  );
> +
> +typedef
> +EFI_STATUS
> +(EFIAPI *REGISTER_PECOFF_IMAGE) (
> +  IN  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL    *This,
> +  IN  EFI_PHYSICAL_ADDRESS                    ImageBase,
> +  IN  UINT64                                  ImageSize
> +  );
> +
> +typedef
> +EFI_STATUS
> +(EFIAPI *UNREGISTER_PECOFF_IMAGE) (
> +  IN  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL    *This,
> +  IN  EFI_PHYSICAL_ADDRESS                    ImageBase
> +  );
> +
> +typedef struct _EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL {
> +  IS_PECOFF_IMAGE_SUPPORTED                   IsImageSupported;
> +  REGISTER_PECOFF_IMAGE                       RegisterImage;
> +  UNREGISTER_PECOFF_IMAGE                     UnregisterImage;
> +} EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL;

Hi Ard,

There is no any comment for these protocol typedefs?

How about using 
EDKII_PECOFF_IMAGE_EMULATOR_IS_IMAGE_SUPPORTED/EDKII_PECOFF_IMAGE_EMULATOR_REGISTER_IMAGE/EDKII_PECOFF_IMAGE_EMULATOR_UNREGISTER_IMAGE 
as the function typedef names?

Add below line to align with other protocol header files?
extern EFI_GUID gEdkiiPeCoffImageEmulatorProtocolGuid;

Thanks,
Star

> +
> +#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.
> 

Re: [PATCH 2/4] MdeModulePkg/DxeCore: invoke the emulator protocol for foreign images

[Zeng, Star]

On 2018/9/12 21:21, Ard Biesheuvel wrote:
> 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 | 39 +++++++++++++++++---
>   3 files changed, 37 insertions(+), 6 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h b/MdeModulePkg/Core/Dxe/DxeMain.h
> index 7ec82388a3f9..57b3861d9813 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  *Emu;

Hi Ard,

How about using PeCoffEmu as the name to be more specific?

>     /// 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..d7591aa0da6d 100644
> --- a/MdeModulePkg/Core/Dxe/DxeMain.inf
> +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
> @@ -180,6 +180,7 @@
>     gEfiVariableArchProtocolGuid                  ## CONSUMES
>     gEfiCapsuleArchProtocolGuid                   ## CONSUMES
>     gEfiWatchdogTimerArchProtocolGuid             ## CONSUMES
> +  gEdkiiPeCoffImageEmulatorProtocolGuid         ## SOMETIMES_CONSUMES
>   
>   [FeaturePcd]
>     gEfiMdeModulePkgTokenSpaceGuid.PcdFrameworkCompatibilitySupport     ## CONSUMES
> diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c b/MdeModulePkg/Core/Dxe/Image/Image.c
> index eddca140ee1a..e2dd80790657 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,                       // Emu
>     NULL,                       // RuntimeData
>     NULL                        // LoadedImageDevicePath
>   };
> @@ -476,12 +477,23 @@ CoreLoadPeImage (
>     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.
> +      // Locate the emulator protocol to check whether it supports this
> +      // image.
>         //
> -      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;
> +      Status = CoreLocateProtocol (&gEdkiiPeCoffImageEmulatorProtocolGuid,
> +                 NULL, (VOID **)&Image->Emu);
> +      if (EFI_ERROR (Status) ||
> +          !Image->Emu->IsImageSupported (Image->Emu,
> +                                         Image->ImageContext.Machine,
> +                                         Image->ImageContext.ImageType)) {
> +        //
> +        // 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 +699,14 @@ CoreLoadPeImage (
>       if (EFI_ERROR(Status)) {
>         goto Done;
>       }
> +  } else if (Image->Emu != NULL) {
> +    Status = Image->Emu->RegisterImage (Image->Emu, Image->ImageBasePage,
> +                           EFI_PAGES_TO_SIZE (Image->NumberOfPages));
> +    if (EFI_ERROR (Status)) {
> +      DEBUG ((DEBUG_LOAD | DEBUG_ERROR,
> +        "CoreLoadPeImage: Failed to load register foreign image with emulator.\n"));

'load' should not be in the sentence, right?

Thanks,
Star

> +      goto Done;
> +    }
>     }
>   
>     //
> @@ -874,6 +894,13 @@ CoreUnloadAndCloseImage (
>       Image->Ebc->UnloadImage (Image->Ebc, Image->Handle);
>     }
>   
> +  if (Image->Emu != NULL) {
> +    //
> +    // If the PE/COFF Emulator protocol exists we must unregister the image.
> +    //
> +    Image->Emu->UnregisterImage (Image->Emu, Image->ImageBasePage);
> +  }
> +
>     //
>     // Unload image, free Image->ImageContext->ModHandle
>     //
> @@ -1599,7 +1626,7 @@ 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->Emu == NULL) {
>       //
>       // Do not ASSERT here, because image might be loaded via EFI_IMAGE_MACHINE_CROSS_TYPE_SUPPORTED
>       // But it can not be started.
> 

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

[Zeng, Star]

On 2018/9/12 21:21, Ard Biesheuvel wrote:
> 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 | 16 +++++++++++++++-
>   3 files changed, 17 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..3d99ea0c1047 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
> @@ -97,6 +97,7 @@
>     gEfiLoadFile2ProtocolGuid                       ## SOMETIMES_PRODUCES
>     gEdkiiIoMmuProtocolGuid                         ## SOMETIMES_CONSUMES
>     gEfiLoadedImageDevicePathProtocolGuid           ## CONSUMES
> +  gEdkiiPeCoffImageEmulatorProtocolGuid           ## SOMETIMES_CONSUMES
>   
>   [FeaturePcd]
>     gEfiMdeModulePkgTokenSpaceGuid.PcdPciBusHotplugDeviceSupport      ## CONSUMES
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
> index c2be85a906af..07236afd327d 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
> @@ -678,6 +678,7 @@ ProcessOpRomImage (
>     MEDIA_RELATIVE_OFFSET_RANGE_DEVICE_PATH  EfiOpRomImageNode;
>     VOID                                     *Buffer;
>     UINTN                                    BufferSize;
> +  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL     *PeCoffEmulator;
>   
>     Indicator = 0;
>   
> @@ -693,6 +694,7 @@ ProcessOpRomImage (
>     }
>     ASSERT (((EFI_PCI_EXPANSION_ROM_HEADER *) RomBarOffset)->Signature == PCI_EXPANSION_ROM_HEADER_SIGNATURE);
>   
> +  PeCoffEmulator = NULL;
>     do {
>       EfiRomHeader = (EFI_PCI_EXPANSION_ROM_HEADER *) RomBarOffset;
>       if (EfiRomHeader->Signature != PCI_EXPANSION_ROM_HEADER_SIGNATURE) {
> @@ -716,7 +718,19 @@ ProcessOpRomImage (
>       // Skip the EFI PCI Option ROM image if its machine type is not supported
>       //
>       if (!EFI_IMAGE_MACHINE_TYPE_SUPPORTED (EfiRomHeader->EfiMachineType)) {
> -      goto NextImage;
> +      //
> +      // Check whether we have a PE/COFF emulator that supports this image
> +      //
> +      if (PeCoffEmulator == NULL) {
> +        gBS->LocateProtocol (&gEdkiiPeCoffImageEmulatorProtocolGuid, NULL,
> +               (VOID **)&PeCoffEmulator);
> +      }
> +      if (PeCoffEmulator == NULL ||
> +          !PeCoffEmulator->IsImageSupported (PeCoffEmulator,
> +                             EfiRomHeader->EfiMachineType,
> +                             EfiRomHeader->EfiSubsystem)) {
> +        goto NextImage;
> +      }

Hi Ard,

Could these be abstracted to a separate function like the PATCH 4/4 did?

Thanks,
Star

>       }
>   
>       //
> 

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

[Ard Biesheuvel]

On 12 September 2018 at 17:10, Kinney, Michael D
<michael.d.kinney@intel.com> wrote:
> Ard,
>
> I think there may be a lot of assumptions in this
> proposal that are not documented.
>
> I do not see any description of how an image is
> started or how calls between native code and emulated
> code are handled.
>

It is the job of the implementation of the emulator to use the
information provided to it at registration time to ensure that calls
to its entry point are handled. One way could be to remap the code
region non-exec (which is what X86EmulatorPkg does) and hook the
exceptions that result when calling into the non-native code. Another
approach could be to do binary translation at load time, hook the
PE/COFF entry point in the header to point to a load stub etc etc.

These are all implementation details of the emulator, and so I am not
sure what exactly we need to document at the protocol level. Does the
above paragraph capture more or less what you mean?

> Also, are multiple emulated CPUs supported?  It looks
> like there can only be one instance of this new protocol.
>

Yes. I am not sure what the added value is of having multiple
emulators. Do you have anything in mind?

In any case, the singleton protocol could be implemented by a wrapper
driver that encapsulates this multiple dispatch functionality. I don't
think it makes sense to duplicate logic for that purpose in the
various places that I touch in this series, and so it shouldn't affect
the protocol prototypes.

> Can you please provide more detailed comments for the
> functions in the new protocol and document the assumptions
> and known limitation in the header?
>

Of course. But note that they are fairly simple prototypes that don't
impose any limitations by themselves, other than that you should call
RegisterImage() before attempting to invoke its entry point.

> From one perspective, EBC is an emulated instruction set.
> Would it make sense to have EBC be one of the emulated
> CPU types that can be plugged in?
>

We could definitely abstract away the EBC handling in the DXE core to
be routed via this protocol, but that would make the changes more
intrusive. I could look into making those changes on top if there is
interest.

Re: [PATCH 1/4] MdeModulePkg: introduce PE/COFF image emulator protocol

[Ard Biesheuvel]

On 13 September 2018 at 12:05, Zeng, Star <star.zeng@intel.com> wrote:
> On 2018/9/12 21:21, Ard Biesheuvel wrote:
>>
>> 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 | 51
>> ++++++++++++++++++++
>>   MdeModulePkg/MdeModulePkg.dec                       |  4 ++
>>   2 files changed, 55 insertions(+)
>>
>> diff --git a/MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h
>> b/MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h
>> new file mode 100644
>> index 000000000000..3391e68557b9
>> --- /dev/null
>> +++ b/MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h
>> @@ -0,0 +1,51 @@
>> +/** @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;
>> +
>> +typedef
>> +BOOLEAN
>> +(EFIAPI *IS_PECOFF_IMAGE_SUPPORTED) (
>> +  IN  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL    *This,
>> +  IN  UINT16                                  MachineType,
>> +  IN  UINT16                                  ImageType
>> +  );
>> +
>> +typedef
>> +EFI_STATUS
>> +(EFIAPI *REGISTER_PECOFF_IMAGE) (
>> +  IN  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL    *This,
>> +  IN  EFI_PHYSICAL_ADDRESS                    ImageBase,
>> +  IN  UINT64                                  ImageSize
>> +  );
>> +
>> +typedef
>> +EFI_STATUS
>> +(EFIAPI *UNREGISTER_PECOFF_IMAGE) (
>> +  IN  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL    *This,
>> +  IN  EFI_PHYSICAL_ADDRESS                    ImageBase
>> +  );
>> +
>> +typedef struct _EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL {
>> +  IS_PECOFF_IMAGE_SUPPORTED                   IsImageSupported;
>> +  REGISTER_PECOFF_IMAGE                       RegisterImage;
>> +  UNREGISTER_PECOFF_IMAGE                     UnregisterImage;
>> +} EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL;
>
>
> Hi Ard,
>
> There is no any comment for these protocol typedefs?
>
> How about using
> EDKII_PECOFF_IMAGE_EMULATOR_IS_IMAGE_SUPPORTED/EDKII_PECOFF_IMAGE_EMULATOR_REGISTER_IMAGE/EDKII_PECOFF_IMAGE_EMULATOR_UNREGISTER_IMAGE
> as the function typedef names?
>
> Add below line to align with other protocol header files?
> extern EFI_GUID gEdkiiPeCoffImageEmulatorProtocolGuid;
>

Yes, I can do that.

Thanks,
Ard.

Re: [PATCH 2/4] MdeModulePkg/DxeCore: invoke the emulator protocol for foreign images

[Ard Biesheuvel]

On 13 September 2018 at 12:23, Zeng, Star <star.zeng@intel.com> wrote:
> On 2018/9/12 21:21, Ard Biesheuvel wrote:
>>
>> 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 | 39 +++++++++++++++++---
>>   3 files changed, 37 insertions(+), 6 deletions(-)
>>
>> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h
>> b/MdeModulePkg/Core/Dxe/DxeMain.h
>> index 7ec82388a3f9..57b3861d9813 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  *Emu;
>
>
> Hi Ard,
>
> How about using PeCoffEmu as the name to be more specific?
>

Good idea.

>
>>     /// 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..d7591aa0da6d 100644
>> --- a/MdeModulePkg/Core/Dxe/DxeMain.inf
>> +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
>> @@ -180,6 +180,7 @@
>>     gEfiVariableArchProtocolGuid                  ## CONSUMES
>>     gEfiCapsuleArchProtocolGuid                   ## CONSUMES
>>     gEfiWatchdogTimerArchProtocolGuid             ## CONSUMES
>> +  gEdkiiPeCoffImageEmulatorProtocolGuid         ## SOMETIMES_CONSUMES
>>     [FeaturePcd]
>>     gEfiMdeModulePkgTokenSpaceGuid.PcdFrameworkCompatibilitySupport     ##
>> CONSUMES
>> diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c
>> b/MdeModulePkg/Core/Dxe/Image/Image.c
>> index eddca140ee1a..e2dd80790657 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,                       // Emu
>>     NULL,                       // RuntimeData
>>     NULL                        // LoadedImageDevicePath
>>   };
>> @@ -476,12 +477,23 @@ CoreLoadPeImage (
>>     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.
>> +      // Locate the emulator protocol to check whether it supports this
>> +      // image.
>>         //
>> -      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;
>> +      Status = CoreLocateProtocol
>> (&gEdkiiPeCoffImageEmulatorProtocolGuid,
>> +                 NULL, (VOID **)&Image->Emu);
>> +      if (EFI_ERROR (Status) ||
>> +          !Image->Emu->IsImageSupported (Image->Emu,
>> +                                         Image->ImageContext.Machine,
>> +                                         Image->ImageContext.ImageType))
>> {
>> +        //
>> +        // 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 +699,14 @@ CoreLoadPeImage (
>>       if (EFI_ERROR(Status)) {
>>         goto Done;
>>       }
>> +  } else if (Image->Emu != NULL) {
>> +    Status = Image->Emu->RegisterImage (Image->Emu, Image->ImageBasePage,
>> +                           EFI_PAGES_TO_SIZE (Image->NumberOfPages));
>> +    if (EFI_ERROR (Status)) {
>> +      DEBUG ((DEBUG_LOAD | DEBUG_ERROR,
>> +        "CoreLoadPeImage: Failed to load register foreign image with
>> emulator.\n"));
>
>
> 'load' should not be in the sentence, right?
>

Correct. I will remove it.

>> +      goto Done;
>> +    }
>>     }
>>       //
>> @@ -874,6 +894,13 @@ CoreUnloadAndCloseImage (
>>       Image->Ebc->UnloadImage (Image->Ebc, Image->Handle);
>>     }
>>   +  if (Image->Emu != NULL) {
>> +    //
>> +    // If the PE/COFF Emulator protocol exists we must unregister the
>> image.
>> +    //
>> +    Image->Emu->UnregisterImage (Image->Emu, Image->ImageBasePage);
>> +  }
>> +
>>     //
>>     // Unload image, free Image->ImageContext->ModHandle
>>     //
>> @@ -1599,7 +1626,7 @@ 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->Emu ==
>> NULL) {
>>       //
>>       // Do not ASSERT here, because image might be loaded via
>> EFI_IMAGE_MACHINE_CROSS_TYPE_SUPPORTED
>>       // But it can not be started.
>>
>

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

[Ard Biesheuvel]

On 13 September 2018 at 12:24, Zeng, Star <star.zeng@intel.com> wrote:
> On 2018/9/12 21:21, Ard Biesheuvel wrote:
>>
>> 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 | 16
>> +++++++++++++++-
>>   3 files changed, 17 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..3d99ea0c1047 100644
>> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
>> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
>> @@ -97,6 +97,7 @@
>>     gEfiLoadFile2ProtocolGuid                       ## SOMETIMES_PRODUCES
>>     gEdkiiIoMmuProtocolGuid                         ## SOMETIMES_CONSUMES
>>     gEfiLoadedImageDevicePathProtocolGuid           ## CONSUMES
>> +  gEdkiiPeCoffImageEmulatorProtocolGuid           ## SOMETIMES_CONSUMES
>>     [FeaturePcd]
>>     gEfiMdeModulePkgTokenSpaceGuid.PcdPciBusHotplugDeviceSupport      ##
>> CONSUMES
>> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
>> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
>> index c2be85a906af..07236afd327d 100644
>> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
>> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
>> @@ -678,6 +678,7 @@ ProcessOpRomImage (
>>     MEDIA_RELATIVE_OFFSET_RANGE_DEVICE_PATH  EfiOpRomImageNode;
>>     VOID                                     *Buffer;
>>     UINTN                                    BufferSize;
>> +  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL     *PeCoffEmulator;
>>       Indicator = 0;
>>   @@ -693,6 +694,7 @@ ProcessOpRomImage (
>>     }
>>     ASSERT (((EFI_PCI_EXPANSION_ROM_HEADER *) RomBarOffset)->Signature ==
>> PCI_EXPANSION_ROM_HEADER_SIGNATURE);
>>   +  PeCoffEmulator = NULL;
>>     do {
>>       EfiRomHeader = (EFI_PCI_EXPANSION_ROM_HEADER *) RomBarOffset;
>>       if (EfiRomHeader->Signature != PCI_EXPANSION_ROM_HEADER_SIGNATURE) {
>> @@ -716,7 +718,19 @@ ProcessOpRomImage (
>>       // Skip the EFI PCI Option ROM image if its machine type is not
>> supported
>>       //
>>       if (!EFI_IMAGE_MACHINE_TYPE_SUPPORTED
>> (EfiRomHeader->EfiMachineType)) {
>> -      goto NextImage;
>> +      //
>> +      // Check whether we have a PE/COFF emulator that supports this
>> image
>> +      //
>> +      if (PeCoffEmulator == NULL) {
>> +        gBS->LocateProtocol (&gEdkiiPeCoffImageEmulatorProtocolGuid,
>> NULL,
>> +               (VOID **)&PeCoffEmulator);
>> +      }
>> +      if (PeCoffEmulator == NULL ||
>> +          !PeCoffEmulator->IsImageSupported (PeCoffEmulator,
>> +                             EfiRomHeader->EfiMachineType,
>> +                             EfiRomHeader->EfiSubsystem)) {
>> +        goto NextImage;
>> +      }
>
>
> Hi Ard,
>
> Could these be abstracted to a separate function like the PATCH 4/4 did?
>

Yes.

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

[Kinney, Michael D]

Ard,

I think there is a fundamental assumption that
the sizeof(UINTN) and size of pointers of 
the native CPU are the same as the emulated CPU.
If that is not the case, then I would like to see
more details.  Otherwise that is a significant
restriction that needs to be clearly documented.

Protocols that only allow a single instance need to
clearly document that assumption.

If we decide to treat EBC as an emulated CPU, then
we would want to support multiple instances of the
protocol.  The updates to the DXE Core are a bit 
confusing because it has separate handling of EBC
and emulated CPUs.  I think it would make the DXE
Core logic simpler and easier to understand if they
were combined.

I asked about the startup case because if we do EBC,
then we may need more services in the protocol because
of the thunk code between native and EBC code.  At the
time EBC was originally implemented, we did not have 
paging enabled and the EBC interpreter work without 
depending on a page fault handler.

The way the protocol is currently defined, I believe it
fundamentally assumes paging is enabled.  If paging is
not enabled, then the current protocol services are not
sufficient for any emulated CPU.  Do we want this to work
for paging disabled cases?  If not, another assumption
to clearly document.

Thanks,

Mike

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-
> bounces@lists.01.org] On Behalf Of Ard Biesheuvel
> Sent: Thursday, September 13, 2018 3:37 AM
> To: Kinney, Michael D <michael.d.kinney@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;
> agraf@suse.de; Andrew Fish <afish@apple.com>;
> Richardson, Brian <brian.richardson@intel.com>; Laszlo
> Ersek <lersek@redhat.com>; Zeng, Star
> <star.zeng@intel.com>
> Subject: Re: [edk2] [PATCH 0/4] MdeModulePkg: add
> support for dispatching foreign arch PE/COFF images
> 
> On 12 September 2018 at 17:10, Kinney, Michael D
> <michael.d.kinney@intel.com> wrote:
> > Ard,
> >
> > I think there may be a lot of assumptions in this
> > proposal that are not documented.
> >
> > I do not see any description of how an image is
> > started or how calls between native code and emulated
> > code are handled.
> >
> 
> It is the job of the implementation of the emulator to
> use the
> information provided to it at registration time to
> ensure that calls
> to its entry point are handled. One way could be to
> remap the code
> region non-exec (which is what X86EmulatorPkg does) and
> hook the
> exceptions that result when calling into the non-native
> code. Another
> approach could be to do binary translation at load time,
> hook the
> PE/COFF entry point in the header to point to a load
> stub etc etc.
> 
> These are all implementation details of the emulator,
> and so I am not
> sure what exactly we need to document at the protocol
> level. Does the
> above paragraph capture more or less what you mean?
> 
> > Also, are multiple emulated CPUs supported?  It looks
> > like there can only be one instance of this new
> protocol.
> >
> 
> Yes. I am not sure what the added value is of having
> multiple
> emulators. Do you have anything in mind?
> 
> In any case, the singleton protocol could be implemented
> by a wrapper
> driver that encapsulates this multiple dispatch
> functionality. I don't
> think it makes sense to duplicate logic for that purpose
> in the
> various places that I touch in this series, and so it
> shouldn't affect
> the protocol prototypes.
> 
> > Can you please provide more detailed comments for the
> > functions in the new protocol and document the
> assumptions
> > and known limitation in the header?
> >
> 
> Of course. But note that they are fairly simple
> prototypes that don't
> impose any limitations by themselves, other than that
> you should call
> RegisterImage() before attempting to invoke its entry
> point.
> 
> > From one perspective, EBC is an emulated instruction
> set.
> > Would it make sense to have EBC be one of the emulated
> > CPU types that can be plugged in?
> >
> 
> We could definitely abstract away the EBC handling in
> the DXE core to
> be routed via this protocol, but that would make the
> changes more
> intrusive. I could look into making those changes on top
> if there is
> interest.
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

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

[Ard Biesheuvel]

On 13 September 2018 at 19:20, Kinney, Michael D
<michael.d.kinney@intel.com> wrote:
> Ard,
>
> I think there is a fundamental assumption that
> the sizeof(UINTN) and size of pointers of
> the native CPU are the same as the emulated CPU.
> If that is not the case, then I would like to see
> more details.  Otherwise that is a significant
> restriction that needs to be clearly documented.
>

There is no such assumption. The PE/COFF emulator protocol is an
abstract protocol that leaves it fully up to the implementation to
decide whether it can support images of machine type X and image type
Y.

> Protocols that only allow a single instance need to
> clearly document that assumption.
>

I will remove that restriction.

> If we decide to treat EBC as an emulated CPU, then
> we would want to support multiple instances of the
> protocol.  The updates to the DXE Core are a bit
> confusing because it has separate handling of EBC
> and emulated CPUs.  I think it would make the DXE
> Core logic simpler and easier to understand if they
> were combined.
>

Yes, excellent idea, and it results in a nice cleanup as well

> I asked about the startup case because if we do EBC,
> then we may need more services in the protocol because
> of the thunk code between native and EBC code.  At the
> time EBC was originally implemented, we did not have
> paging enabled and the EBC interpreter work without
> depending on a page fault handler.
>
> The way the protocol is currently defined, I believe it
> fundamentally assumes paging is enabled.  If paging is
> not enabled, then the current protocol services are not
> sufficient for any emulated CPU.  Do we want this to work
> for paging disabled cases?  If not, another assumption
> to clearly document.
>

The paging disabled case is interesting. Does the UEFI spec even
permit running in DXE with paging disabled?

In any case, I will send out a v2 as the basis for further discussion.
We can also sit down and discuss it in Vancouver the coming week.

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

[Kinney, Michael D]

Ard,

I look forward to discussing this in more detail this week.
We can update this thread with that information.

Mike

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org]
> On Behalf Of Ard Biesheuvel
> Sent: Saturday, September 15, 2018 6:28 AM
> To: Kinney, Michael D <michael.d.kinney@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>; agraf@suse.de; Richardson, Brian
> <brian.richardson@intel.com>; Laszlo Ersek
> <lersek@redhat.com>; Zeng, Star <star.zeng@intel.com>
> Subject: Re: [edk2] [PATCH 0/4] MdeModulePkg: add support
> for dispatching foreign arch PE/COFF images
> 
> On 13 September 2018 at 19:20, Kinney, Michael D
> <michael.d.kinney@intel.com> wrote:
> > Ard,
> >
> > I think there is a fundamental assumption that
> > the sizeof(UINTN) and size of pointers of
> > the native CPU are the same as the emulated CPU.
> > If that is not the case, then I would like to see
> > more details.  Otherwise that is a significant
> > restriction that needs to be clearly documented.
> >
> 
> There is no such assumption. The PE/COFF emulator protocol
> is an
> abstract protocol that leaves it fully up to the
> implementation to
> decide whether it can support images of machine type X and
> image type
> Y.
> 
> > Protocols that only allow a single instance need to
> > clearly document that assumption.
> >
> 
> I will remove that restriction.
> 
> > If we decide to treat EBC as an emulated CPU, then
> > we would want to support multiple instances of the
> > protocol.  The updates to the DXE Core are a bit
> > confusing because it has separate handling of EBC
> > and emulated CPUs.  I think it would make the DXE
> > Core logic simpler and easier to understand if they
> > were combined.
> >
> 
> Yes, excellent idea, and it results in a nice cleanup as
> well
> 
> > I asked about the startup case because if we do EBC,
> > then we may need more services in the protocol because
> > of the thunk code between native and EBC code.  At the
> > time EBC was originally implemented, we did not have
> > paging enabled and the EBC interpreter work without
> > depending on a page fault handler.
> >
> > The way the protocol is currently defined, I believe it
> > fundamentally assumes paging is enabled.  If paging is
> > not enabled, then the current protocol services are not
> > sufficient for any emulated CPU.  Do we want this to
> work
> > for paging disabled cases?  If not, another assumption
> > to clearly document.
> >
> 
> The paging disabled case is interesting. Does the UEFI
> spec even
> permit running in DXE with paging disabled?
> 
> In any case, I will send out a v2 as the basis for further
> discussion.
> We can also sit down and discuss it in Vancouver the
> coming week.
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

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

[Andrew Fish]

> On Sep 15, 2018, at 6:28 AM, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> 
> On 13 September 2018 at 19:20, Kinney, Michael D
> <michael.d.kinney@intel.com> wrote:
>> Ard,
>> 
>> I think there is a fundamental assumption that
>> the sizeof(UINTN) and size of pointers of
>> the native CPU are the same as the emulated CPU.
>> If that is not the case, then I would like to see
>> more details.  Otherwise that is a significant
>> restriction that needs to be clearly documented.
>> 
> 
> There is no such assumption. The PE/COFF emulator protocol is an
> abstract protocol that leaves it fully up to the implementation to
> decide whether it can support images of machine type X and image type
> Y.
> 

Ard,

Not knowing the size of UINTN or a pointer was very painful in terms of how EBC worked. The compiler was forced to generate code for sizeof vs. resolving it at compile time. 

>> Protocols that only allow a single instance need to
>> clearly document that assumption.
>> 
> 
> I will remove that restriction.
> 
>> If we decide to treat EBC as an emulated CPU, then
>> we would want to support multiple instances of the
>> protocol.  The updates to the DXE Core are a bit
>> confusing because it has separate handling of EBC
>> and emulated CPUs.  I think it would make the DXE
>> Core logic simpler and easier to understand if they
>> were combined.
>> 
> 
> Yes, excellent idea, and it results in a nice cleanup as well
> 
>> I asked about the startup case because if we do EBC,
>> then we may need more services in the protocol because
>> of the thunk code between native and EBC code.  At the
>> time EBC was originally implemented, we did not have
>> paging enabled and the EBC interpreter work without
>> depending on a page fault handler.
>> 
>> The way the protocol is currently defined, I believe it
>> fundamentally assumes paging is enabled.  If paging is
>> not enabled, then the current protocol services are not
>> sufficient for any emulated CPU.  Do we want this to work
>> for paging disabled cases?  If not, another assumption
>> to clearly document.
>> 
> 
> The paging disabled case is interesting. Does the UEFI spec even
> permit running in DXE with paging disabled?

Paging is a function of the processor binding in the UEFI spec. It is not required for IA32. For X64 you have to have paging enable to enter long mode. On other processors you need to turn on paging to control the cache. So I guess the no paging is likely more of a i386 issue? 

> 
> In any case, I will send out a v2 as the basis for further discussion.
> We can also sit down and discuss it in Vancouver the coming week.

I'd suggest passing an EFI device path to IS_PECOFF_IMAGE_SUPPORTED. At some point it might be useful for the emulator to know what device is being emulated. 

For bonus points we could check IsPecoffImageSupported() prior to the native check. Never say never, some one may want to have emulator run on native images for debugging etc. 

Thanks,

Andrew Fish

> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

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

[Ard Biesheuvel]

On 19 September 2018 at 12:35, Andrew Fish <afish@apple.com> wrote:
>
>
>> On Sep 15, 2018, at 6:28 AM, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>>
>> On 13 September 2018 at 19:20, Kinney, Michael D
>> <michael.d.kinney@intel.com> wrote:
>>> Ard,
>>>
>>> I think there is a fundamental assumption that
>>> the sizeof(UINTN) and size of pointers of
>>> the native CPU are the same as the emulated CPU.
>>> If that is not the case, then I would like to see
>>> more details.  Otherwise that is a significant
>>> restriction that needs to be clearly documented.
>>>
>>
>> There is no such assumption. The PE/COFF emulator protocol is an
>> abstract protocol that leaves it fully up to the implementation to
>> decide whether it can support images of machine type X and image type
>> Y.
>>
>
> Ard,
>
> Not knowing the size of UINTN or a pointer was very painful in terms of how EBC worked. The compiler was forced to generate code for sizeof vs. resolving it at compile time.
>

Oh, I'm sure - but my point is that whether architecture X can be
emulated on architecture Y and how is a limitation that affects some
implementations of the protocol, but at the abstract level that we
deal with in the core code, it is not a concern.

>>> Protocols that only allow a single instance need to
>>> clearly document that assumption.
>>>
>>
>> I will remove that restriction.
>>
>>> If we decide to treat EBC as an emulated CPU, then
>>> we would want to support multiple instances of the
>>> protocol.  The updates to the DXE Core are a bit
>>> confusing because it has separate handling of EBC
>>> and emulated CPUs.  I think it would make the DXE
>>> Core logic simpler and easier to understand if they
>>> were combined.
>>>
>>
>> Yes, excellent idea, and it results in a nice cleanup as well
>>
>>> I asked about the startup case because if we do EBC,
>>> then we may need more services in the protocol because
>>> of the thunk code between native and EBC code.  At the
>>> time EBC was originally implemented, we did not have
>>> paging enabled and the EBC interpreter work without
>>> depending on a page fault handler.
>>>
>>> The way the protocol is currently defined, I believe it
>>> fundamentally assumes paging is enabled.  If paging is
>>> not enabled, then the current protocol services are not
>>> sufficient for any emulated CPU.  Do we want this to work
>>> for paging disabled cases?  If not, another assumption
>>> to clearly document.
>>>
>>
>> The paging disabled case is interesting. Does the UEFI spec even
>> permit running in DXE with paging disabled?
>
> Paging is a function of the processor binding in the UEFI spec. It is not required for IA32. For X64 you have to have paging enable to enter long mode. On other processors you need to turn on paging to control the cache. So I guess the no paging is likely more of a i386 issue?
>

Michael spotted yesterday that RISC-V mandates paging disabled, which
is peculiar in itself. But again, whether a certain implementation of
the protocol relies on paging or not is an implementation detail.

>>
>> In any case, I will send out a v2 as the basis for further discussion.
>> We can also sit down and discuss it in Vancouver the coming week.
>
> I'd suggest passing an EFI device path to IS_PECOFF_IMAGE_SUPPORTED. At some point it might be useful for the emulator to know what device is being emulated.
>

I guess you mean for which device we are loading a driver that relies
on emulation? I guess that makes sense for option ROMs, which is the
primary use case, so yes, I can add that.

> For bonus points we could check IsPecoffImageSupported() prior to the native check. Never say never, some one may want to have emulator run on native images for debugging etc.
>

Peter brought this up as well - in some cases, you may want to sandbox
X86 drivers running on X86 by running them on an emulator. If you
think the overhead of performing this check for each image rather than
only for images that have already been found to be for a foreign
architecture is acceptables then I'm happy to change this. But we can
easily do that later as well.

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

[Andrew Fish]

> On Sep 19, 2018, at 1:43 PM, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> 
> On 19 September 2018 at 12:35, Andrew Fish <afish@apple.com <mailto:afish@apple.com>> wrote:
>> 
>> 
>>> On Sep 15, 2018, at 6:28 AM, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>>> 
>>> On 13 September 2018 at 19:20, Kinney, Michael D
>>> <michael.d.kinney@intel.com> wrote:
>>>> Ard,
>>>> 
>>>> I think there is a fundamental assumption that
>>>> the sizeof(UINTN) and size of pointers of
>>>> the native CPU are the same as the emulated CPU.
>>>> If that is not the case, then I would like to see
>>>> more details.  Otherwise that is a significant
>>>> restriction that needs to be clearly documented.
>>>> 
>>> 
>>> There is no such assumption. The PE/COFF emulator protocol is an
>>> abstract protocol that leaves it fully up to the implementation to
>>> decide whether it can support images of machine type X and image type
>>> Y.
>>> 
>> 
>> Ard,
>> 
>> Not knowing the size of UINTN or a pointer was very painful in terms of how EBC worked. The compiler was forced to generate code for sizeof vs. resolving it at compile time.
>> 
> 
> Oh, I'm sure - but my point is that whether architecture X can be
> emulated on architecture Y and how is a limitation that affects some
> implementations of the protocol, but at the abstract level that we
> deal with in the core code, it is not a concern.
> 
>>>> Protocols that only allow a single instance need to
>>>> clearly document that assumption.
>>>> 
>>> 
>>> I will remove that restriction.
>>> 
>>>> If we decide to treat EBC as an emulated CPU, then
>>>> we would want to support multiple instances of the
>>>> protocol.  The updates to the DXE Core are a bit
>>>> confusing because it has separate handling of EBC
>>>> and emulated CPUs.  I think it would make the DXE
>>>> Core logic simpler and easier to understand if they
>>>> were combined.
>>>> 
>>> 
>>> Yes, excellent idea, and it results in a nice cleanup as well
>>> 
>>>> I asked about the startup case because if we do EBC,
>>>> then we may need more services in the protocol because
>>>> of the thunk code between native and EBC code.  At the
>>>> time EBC was originally implemented, we did not have
>>>> paging enabled and the EBC interpreter work without
>>>> depending on a page fault handler.
>>>> 
>>>> The way the protocol is currently defined, I believe it
>>>> fundamentally assumes paging is enabled.  If paging is
>>>> not enabled, then the current protocol services are not
>>>> sufficient for any emulated CPU.  Do we want this to work
>>>> for paging disabled cases?  If not, another assumption
>>>> to clearly document.
>>>> 
>>> 
>>> The paging disabled case is interesting. Does the UEFI spec even
>>> permit running in DXE with paging disabled?
>> 
>> Paging is a function of the processor binding in the UEFI spec. It is not required for IA32. For X64 you have to have paging enable to enter long mode. On other processors you need to turn on paging to control the cache. So I guess the no paging is likely more of a i386 issue?
>> 
> 
> Michael spotted yesterday that RISC-V mandates paging disabled, which
> is peculiar in itself. But again, whether a certain implementation of
> the protocol relies on paging or not is an implementation detail.
> 
>>> 
>>> In any case, I will send out a v2 as the basis for further discussion.
>>> We can also sit down and discuss it in Vancouver the coming week.
>> 
>> I'd suggest passing an EFI device path to IS_PECOFF_IMAGE_SUPPORTED. At some point it might be useful for the emulator to know what device is being emulated.
>> 
> 
> I guess you mean for which device we are loading a driver that relies
> on emulation? I guess that makes sense for option ROMs, which is the
> primary use case, so yes, I can add that.
> 
>> For bonus points we could check IsPecoffImageSupported() prior to the native check. Never say never, some one may want to have emulator run on native images for debugging etc.
>> 
> 
> Peter brought this up as well - in some cases, you may want to sandbox
> X86 drivers running on X86 by running them on an emulator. If you
> think the overhead of performing this check for each image rather than
> only for images that have already been found to be for a foreign
> architecture is acceptables then I'm happy to change this. But we can
> easily do that later as well.

Ard,

I vote for the flexibility. Given ImageType is passed into IsImageSupported() in the simple case it is just going to be checking a register (argument) against a constant. 

Thanks,

Andrew Fish

'