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

["Zeng, Star" <star.zeng@intel.com>]

A little late feedback. Just an idea.

Do you think whether it can make the consumer simpler like below or not?

Add a new API RegisterInterfaces in the protocol and add a wrapper driver PeCoffEmulatorDxe.
The emulators can call RegisterInterfaces, and consumer will call other APIs simply, PeCoffEmulatorDxe will be a wrapper as manager.

typedef
EFI_STATUS
(EFIAPI *EDKII_PECOFF_IMAGE_EMULATOR_REGISTER_INTERFACES) (
  IN EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL    *This,
  IN EDKII_PECOFF_IMAGE_EMULATOR_IS_IMAGE_SUPPORTED    IsImageSupported,
  IN EDKII_PECOFF_IMAGE_EMULATOR_REGISTER_IMAGE        RegisterImage,
  IN EDKII_PECOFF_IMAGE_EMULATOR_UNREGISTER_IMAGE      UnregisterImage
  );

typedef struct _EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL {
  EDKII_PECOFF_IMAGE_EMULATOR_REGISTER_INTERFACES    RegisterInterfaces;
  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;


Thanks,
Star
-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] 
Sent: Friday, September 21, 2018 7:02 AM
To: edk2-devel@lists.01.org
Cc: 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>; Leif Lindholm <leif.lindholm@linaro.org>; Zeng, Star <star.zeng@intel.com>; Dong, Eric <eric.dong@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Gao, Liming <liming.gao@intel.com>; Carsey, Jaben <jaben.carsey@intel.com>; Shi, Steven <steven.shi@intel.com>
Subject: [PATCH v3 1/7] MdeModulePkg: introduce PE/COFF image emulator protocol

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.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h | 102 ++++++++++++++++++++
 MdeModulePkg/MdeModulePkg.dec                       |   4 +
 2 files changed, 106 insertions(+)

diff --git a/MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h b/MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h
new file mode 100644
index 000000000000..27bad556209c
--- /dev/null
+++ b/MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h
@@ -0,0 +1,102 @@
+/** @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.
+  @param[in] DevicePath   Path to device where the image originated
+                          (e.g., a PCI option ROM)
+
+  @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,
+  IN  EFI_DEVICE_PATH_PROTOCOL                *DevicePath   OPTIONAL
+  );
+
+/**
+  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