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

["Michael D Kinney" <michael.d.kinney@intel.com>]

Ard,

Resend to devel@edk2.groups.io

One comment below on a logic issue that would be exposed 
if multiple emulators are present.

Mike

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Sunday, March 31, 2019 8:27 PM
> 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, Ray <ray.ni@intel.com>; Gao,
> Liming <liming.gao@intel.com>; Carsey, Jaben
> <jaben.carsey@intel.com>; Shi, Steven
> <steven.shi@intel.com>
> Subject: [PATCH v4 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.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel
> <ard.biesheuvel@linaro.org>
> ---
>  MdeModulePkg/Core/Dxe/DxeMain.h     |   3 +
>  MdeModulePkg/Core/Dxe/DxeMain.inf   |   1 +
>  MdeModulePkg/Core/Dxe/Image/Image.c | 144
> ++++++++++++++++++--
>  3 files changed, 137 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..cda447d23ec5 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,39 @@ 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;
> +  EMULATOR_ENTRY      *Entry;
> +
> +  Entry = AllocateZeroPool (sizeof *Entry);
> +  ASSERT (Entry != NULL);
> +
> +  Status = CoreLocateProtocol
> (&gEdkiiPeCoffImageEmulatorProtocolGuid,
> +
> mPeCoffEmuProtocolNotifyRegistration,
> +                               (VOID **)&Entry-
> >Emulator
> +                               );

A protocol notification function that only expects a single
instance of the protocol can use LocateProtocol().  However,
this use case needs to support one or more Emu Protocol 
instances and add each of them to the linked list.  So the
function LocateHandle() should be used with ByRegisterNotify
to make sure the handle with the installation of the specific
protocol instance is located.  Then HandleProtocol() can be
used on that handle to get the protocol instance.

There is another example of this in the DXE Core in the file
Dispatcher.c, function CoreFwVolEventProtocolNotify().  It call
CoreLocateHandle() to get the handle.

    Status = CoreLocateHandle (
               ByRegisterNotify,
               NULL,
               mFwVolEventRegistration,
               &BufferSize,
               &FvHandle
               );

Then calls CoreHandleProtocol() to get the protocol instance.

    Status = CoreHandleProtocol (FvHandle, &gEfiFirmwareVolume2ProtocolGuid, (VOID **)&Fv);


> +  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 +235,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 +492,45 @@
> GetPeCoffImageFixLoadingAssignedAddress(
>     DEBUG ((EFI_D_INFO|EFI_D_LOAD, "LOADING MODULE
> FIXED INFO: Loading module at fixed address 0x%11p.
> Status = %r \n", (VOID *)(UINTN)(ImageContext-
> >ImageAddress), Status));
>     return Status;
>  }
> +
> +/**
> +  Decides whether a PE/COFF image can execute on this
> system, either natively
> +  or via emulation/interpretation. In that latter
> case, the PeCoffEmu member
> +  of the LOADED_IMAGE_PRIVATE_DATA struct pointer is
> populated with a pointer
> +  to the emulator protocol that supports this image.
> +
> +  @param[in]  Image         LOADED_IMAGE_PRIVATE_DATA
> struct pointer
> +**/
> +STATIC
> +BOOLEAN
> +CoreIsImageTypeSupported (
> +  IN OUT LOADED_IMAGE_PRIVATE_DATA  *Image
> +  )
> +{
> +  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 +579,14 @@ CoreLoadPeImage (
>      return Status;
>    }
> 
> -  if (!EFI_IMAGE_MACHINE_TYPE_SUPPORTED (Image-
> >ImageContext.Machine)) {
> -    if (!EFI_IMAGE_MACHINE_CROSS_TYPE_SUPPORTED
> (Image->ImageContext.Machine)) {
> -      //
> -      // The PE/COFF loader can support loading image
> types that can be executed.
> -      // If we loaded an image type that we can not
> execute return EFI_UNSUPORTED.
> -      //
> -      DEBUG ((EFI_D_ERROR, "Image type %s can't be
> loaded ", GetMachineTypeName(Image-
> >ImageContext.Machine)));
> -      DEBUG ((EFI_D_ERROR, "on %s UEFI system.\n",
> GetMachineTypeName(mDxeCoreImageMachineType)));
> -      return EFI_UNSUPPORTED;
> -    }
> +  if (!CoreIsImageTypeSupported (Image)) {
> +    //
> +    // The PE/COFF loader can support loading image
> types that can be executed.
> +    // If we loaded an image type that we can not
> execute return EFI_UNSUPORTED.
> +    //
> +    DEBUG ((EFI_D_ERROR, "Image type %s can't be
> loaded ", GetMachineTypeName(Image-
> >ImageContext.Machine)));
> +    DEBUG ((EFI_D_ERROR, "on %s UEFI system.\n",
> GetMachineTypeName(mDxeCoreImageMachineType)));
> +    return EFI_UNSUPPORTED;
>    }
> 
>    //
> @@ -687,6 +791,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 +988,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 +1720,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