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

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

On Wed, 26 Sep 2018 at 12:07, Zeng, Star <star.zeng@intel.com> wrote:
>
> 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;
>

Hi Star,

Thanks for taking a look.

I tried to avoid introducing new drivers or library classes because it
will break existing platforms that incorporate EbcDxe. Do you think
this is not an issue?

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