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

["Wu, Hao A" <hao.a.wu@intel.com>]

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Monday, April 15, 2019 3:52 AM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel; Kinney, Michael D; Andrew Fish; Leif Lindholm; Zeng, Star;
> Dong, Eric; Ni, Ray; Gao, Liming; Carsey, Jaben; Shi, Steven; Wang, Jian J; Wu,
> Hao A
> Subject: [PATCH v6 2/7] MdeModulePkg/DxeCore: invoke the emulator
> protocol for foreign images
> 
> 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.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
> ---
>  MdeModulePkg/Core/Dxe/DxeMain.h     |   3 +
>  MdeModulePkg/Core/Dxe/DxeMain.inf   |   1 +
>  MdeModulePkg/Core/Dxe/Image/Image.c | 171 ++++++++++++++++++--
>  3 files changed, 164 insertions(+), 11 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h
> b/MdeModulePkg/Core/Dxe/DxeMain.h
> index 2dec9da5e35b..48ec30a48aa2 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>
> @@ -228,6 +229,8 @@ typedef struct {
>    UINT16                      Machine;
>    /// EBC Protocol pointer
>    EFI_EBC_PROTOCOL            *Ebc;
> +  /// PE/COFF Image Emulator Protocol pointer
> +  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL  *PeCoffEmu;
>    /// Runtime image list
>    EFI_RUNTIME_IMAGE_ENTRY     *RuntimeData;
>    /// Pointer to Loaded Image Device Path Protocol
> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf
> b/MdeModulePkg/Core/Dxe/DxeMain.inf
> index 10375443c0f4..ce6fc19be5e4 100644
> --- a/MdeModulePkg/Core/Dxe/DxeMain.inf
> +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
> @@ -162,6 +162,7 @@
>    gEfiHiiPackageListProtocolGuid                ## SOMETIMES_PRODUCES
>    gEfiEbcProtocolGuid                           ## SOMETIMES_CONSUMES
>    gEfiSmmBase2ProtocolGuid                      ## SOMETIMES_CONSUMES
> +  gEdkiiPeCoffImageEmulatorProtocolGuid         ## SOMETIMES_CONSUMES
> 
>    # Arch Protocols
>    gEfiBdsArchProtocolGuid                       ## CONSUMES
> diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c
> b/MdeModulePkg/Core/Dxe/Image/Image.c
> index eddca140ee1a..57330636b5f3 100644
> --- a/MdeModulePkg/Core/Dxe/Image/Image.c
> +++ b/MdeModulePkg/Core/Dxe/Image/Image.c
> @@ -29,6 +29,15 @@ LOAD_PE32_IMAGE_PRIVATE_DATA
> mLoadPe32PrivateData = {
>    }
>  };
> 
> +typedef struct {
> +  LIST_ENTRY                            Link;
> +  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL  *Emulator;
> +  UINT16                                MachineType;
> +} EMULATOR_ENTRY;
> +
> +STATIC LIST_ENTRY                       mAvailableEmulators;
> +STATIC EFI_EVENT                        mPeCoffEmuProtocolRegistrationEvent;
> +STATIC VOID                             *mPeCoffEmuProtocolNotifyRegistration;
> 
>  //
>  // This code is needed to build the Image handle for the DXE Core
> @@ -67,6 +76,7 @@ LOADED_IMAGE_PRIVATE_DATA mCorePrivateImage  = {
>    NULL,                       // JumpContext
>    0,                          // Machine
>    NULL,                       // Ebc
> +  NULL,                       // PeCoffEmu
>    NULL,                       // RuntimeData
>    NULL                        // LoadedImageDevicePath
>  };
> @@ -118,6 +128,61 @@ GetMachineTypeName (
>    return L"<Unknown>";
>  }
> 
> +/**
> +  Notification event handler registered by CoreInitializeImageServices () to
> +  keep track of which PE/COFF image emulators are available.
> +
> +  @param  Event          The Event that is being processed, not used.
> +  @param  Context        Event Context, not used.
> +
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +PeCoffEmuProtocolNotify (
> +  IN  EFI_EVENT       Event,
> +  IN  VOID            *Context
> +  )
> +{
> +  EFI_STATUS          Status;
> +  UINTN               BufferSize;
> +  EFI_HANDLE          EmuHandle;
> +  EMULATOR_ENTRY      *Entry;
> +
> +  EmuHandle = NULL;
> +
> +  while (TRUE) {
> +    BufferSize = sizeof (EmuHandle);
> +    Status = CoreLocateHandle (
> +               ByRegisterNotify,
> +               NULL,
> +               mPeCoffEmuProtocolNotifyRegistration,
> +               &BufferSize,
> +               &EmuHandle
> +               );
> +    if (EFI_ERROR (Status)) {
> +      //
> +      // If no more notification events exit
> +      //
> +      return;
> +    }
> +
> +    Entry = AllocateZeroPool (sizeof (*Entry));
> +    ASSERT (Entry != NULL);
> +
> +    Status = CoreHandleProtocol (
> +               EmuHandle,
> +               &gEdkiiPeCoffImageEmulatorProtocolGuid,
> +               (VOID **)&Entry->Emulator
> +               );
> +    ASSERT_EFI_ERROR (Status);
> +
> +    Entry->MachineType = Entry->Emulator->MachineType;
> +
> +    InsertTailList (&mAvailableEmulators, &Entry->Link);
> +  }
> +}
> +
>  /**
>    Add the Image Services to EFI Boot Services Table and install the protocol
>    interfaces for this image.
> @@ -192,6 +257,30 @@ CoreInitializeImageServices (
>    gDxeCoreImageHandle = Image->Handle;
>    gDxeCoreLoadedImage = &Image->Info;
> 
> +  //
> +  // Create the PE/COFF emulator protocol registration event
> +  //
> +  Status = CoreCreateEvent (
> +             EVT_NOTIFY_SIGNAL,
> +             TPL_CALLBACK,
> +             PeCoffEmuProtocolNotify,
> +             NULL,
> +             &mPeCoffEmuProtocolRegistrationEvent
> +             );
> +  ASSERT_EFI_ERROR(Status);
> +
> +  //
> +  // Register for protocol notifications on this event
> +  //
> +  Status = CoreRegisterProtocolNotify (
> +             &gEdkiiPeCoffImageEmulatorProtocolGuid,
> +             mPeCoffEmuProtocolRegistrationEvent,
> +             &mPeCoffEmuProtocolNotifyRegistration
> +             );
> +  ASSERT_EFI_ERROR(Status);
> +
> +  InitializeListHead (&mAvailableEmulators);
> +
>    if (FeaturePcdGet (PcdFrameworkCompatibilitySupport)) {
>      //
>      // Export DXE Core PE Loader functionality for backward compatibility.
> @@ -425,6 +514,49 @@ GetPeCoffImageFixLoadingAssignedAddress(
>     DEBUG ((EFI_D_INFO|EFI_D_LOAD, "LOADING MODULE FIXED INFO: Loading
> module at fixed address 0x%11p. Status = %r \n", (VOID
> *)(UINTN)(ImageContext->ImageAddress), Status));
>     return Status;
>  }
> +
> +/**
> +  Decides whether a PE/COFF image can execute on this system, either
> natively
> +  or via emulation/interpretation. In the latter case, the PeCoffEmu member
> +  of the LOADED_IMAGE_PRIVATE_DATA struct pointer is populated with a
> pointer
> +  to the emulator protocol that supports this image.
> +
> +  @param[in]  Image         LOADED_IMAGE_PRIVATE_DATA struct pointer

Hello Ard,

Sorry for the delayed response.

Could you help to update the above comments to:
'@param[in, out]  Image ...'

here to please the Ecc checker when you push the series?

Best Regards,
Hao Wu

> +
> +  @retval     TRUE          The image is supported
> +  @retval     FALSE         The image is not supported
> +
> +**/
> +STATIC
> +BOOLEAN
> +CoreIsImageTypeSupported (
> +  IN OUT LOADED_IMAGE_PRIVATE_DATA  *Image
> +  )
> +{
> +  LIST_ENTRY                        *Link;
> +  EMULATOR_ENTRY                    *Entry;
> +
> +  for (Link = GetFirstNode (&mAvailableEmulators);
> +       !IsNull (&mAvailableEmulators, Link);
> +       Link = GetNextNode (&mAvailableEmulators, Link)) {
> +
> +    Entry = BASE_CR (Link, EMULATOR_ENTRY, Link);
> +    if (Entry->MachineType != Image->ImageContext.Machine) {
> +      continue;
> +    }
> +
> +    if (Entry->Emulator->IsImageSupported (Entry->Emulator,
> +                           Image->ImageContext.ImageType,
> +                           Image->Info.FilePath)) {
> +      Image->PeCoffEmu = Entry->Emulator;
> +      return TRUE;
> +    }
> +  }
> +
> +  return EFI_IMAGE_MACHINE_TYPE_SUPPORTED (Image-
> >ImageContext.Machine) ||
> +         EFI_IMAGE_MACHINE_CROSS_TYPE_SUPPORTED (Image-
> >ImageContext.Machine);
> +}
> +
>  /**
>    Loads, relocates, and invokes a PE/COFF image
> 
> @@ -473,16 +605,15 @@ CoreLoadPeImage (
>      return Status;
>    }
> 
> -  if (!EFI_IMAGE_MACHINE_TYPE_SUPPORTED (Image-
> >ImageContext.Machine)) {
> -    if (!EFI_IMAGE_MACHINE_CROSS_TYPE_SUPPORTED (Image-
> >ImageContext.Machine)) {
> -      //
> -      // The PE/COFF loader can support loading image types that can be
> executed.
> -      // If we loaded an image type that we can not execute return
> EFI_UNSUPORTED.
> -      //
> -      DEBUG ((EFI_D_ERROR, "Image type %s can't be loaded ",
> GetMachineTypeName(Image->ImageContext.Machine)));
> -      DEBUG ((EFI_D_ERROR, "on %s UEFI system.\n",
> GetMachineTypeName(mDxeCoreImageMachineType)));
> -      return EFI_UNSUPPORTED;
> -    }
> +  if (!CoreIsImageTypeSupported (Image)) {
> +    //
> +    // The PE/COFF loader can support loading image types that can be
> executed.
> +    // If we loaded an image type that we can not execute return
> EFI_UNSUPPORTED.
> +    //
> +    DEBUG ((DEBUG_ERROR, "Image type %s can't be loaded on %s UEFI
> system.\n",
> +      GetMachineTypeName (Image->ImageContext.Machine),
> +      GetMachineTypeName (mDxeCoreImageMachineType)));
> +    return EFI_UNSUPPORTED;
>    }
> 
>    //
> @@ -687,6 +818,16 @@ CoreLoadPeImage (
>      if (EFI_ERROR(Status)) {
>        goto Done;
>      }
> +  } else if (Image->PeCoffEmu != NULL) {
> +    Status = Image->PeCoffEmu->RegisterImage (Image->PeCoffEmu,
> +                                 Image->ImageBasePage,
> +                                 EFI_PAGES_TO_SIZE (Image->NumberOfPages),
> +                                 &Image->EntryPoint);
> +    if (EFI_ERROR (Status)) {
> +      DEBUG ((DEBUG_LOAD | DEBUG_ERROR,
> +        "CoreLoadPeImage: Failed to register foreign image with emulator.\n"));
> +      goto Done;
> +    }
>    }
> 
>    //
> @@ -874,6 +1015,13 @@ CoreUnloadAndCloseImage (
>      Image->Ebc->UnloadImage (Image->Ebc, Image->Handle);
>    }
> 
> +  if (Image->PeCoffEmu != NULL) {
> +    //
> +    // If the PE/COFF Emulator protocol exists we must unregister the image.
> +    //
> +    Image->PeCoffEmu->UnregisterImage (Image->PeCoffEmu, Image-
> >ImageBasePage);
> +  }
> +
>    //
>    // Unload image, free Image->ImageContext->ModHandle
>    //
> @@ -1599,7 +1747,8 @@ CoreStartImage (
>    //
>    // The image to be started must have the machine type supported by
> DxeCore.
>    //
> -  if (!EFI_IMAGE_MACHINE_TYPE_SUPPORTED (Image->Machine)) {
> +  if (!EFI_IMAGE_MACHINE_TYPE_SUPPORTED (Image->Machine) &&
> +      Image->PeCoffEmu == NULL) {
>      //
>      // Do not ASSERT here, because image might be loaded via
> EFI_IMAGE_MACHINE_CROSS_TYPE_SUPPORTED
>      // But it can not be started.
> --
> 2.17.1