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

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

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

>       }
>   
>       //
>