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

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

Hi Ard,

I am wondering if we can simplify the PciBusDxe driver and 
remove the image type supported check and instead depend on
LoadImage().  LoadImage() will return EFI_UNSUPPORTED if the
image type is not supported.

Thanks,

Mike

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Thursday, September 20, 2018 4:02 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, 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 3/7] MdeModulePkg/PciBusDxe: invoke
> PE/COFF emulator for foreign option ROMs
> 
> 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.1
> 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 |
> 53 +++++++++++++++++++-
>  3 files changed, 54 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..c8b861093292 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
> @@ -96,6 +96,7 @@
>    gEfiIncompatiblePciDeviceSupportProtocolGuid    ##
> SOMETIMES_CONSUMES
>    gEfiLoadFile2ProtocolGuid                       ##
> SOMETIMES_PRODUCES
>    gEdkiiIoMmuProtocolGuid                         ##
> SOMETIMES_CONSUMES
> +  gEdkiiPeCoffImageEmulatorProtocolGuid           ##
> SOMETIMES_CONSUMES
>    gEfiLoadedImageDevicePathProtocolGuid           ##
> CONSUMES
> 
>  [FeaturePcd]
> diff --git
> a/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
> index c2be85a906af..085bd5d571bd 100644
> ---
> a/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
> +++
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
> @@ -651,6 +651,55 @@ RomDecode (
>    }
>  }
> 
> +STATIC
> +BOOLEAN
> +IsImageTypeSupported (
> +  IN  UINT16                    MachineType,
> +  IN  UINT16                    SubSystem,
> +  IN  EFI_DEVICE_PATH_PROTOCOL  *DevicePath
> +  )
> +{
> +  EFI_STATUS                            Status;
> +  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL  *Emu;
> +  UINTN                                 HandleCount;
> +  EFI_HANDLE                            *HandleBuffer;
> +  BOOLEAN                               ReturnValue;
> +  UINTN                                 Index;
> +
> +  if (EFI_IMAGE_MACHINE_TYPE_SUPPORTED (MachineType))
> {
> +    return TRUE;
> +  }
> +
> +  Status = gBS->LocateHandleBuffer (
> +                  ByProtocol,
> +
> &gEdkiiPeCoffImageEmulatorProtocolGuid,
> +                  NULL,
> +                  &HandleCount,
> +                  &HandleBuffer
> +                  );
> +  if (EFI_ERROR (Status)) {
> +    return FALSE;
> +  }
> +
> +  ReturnValue = FALSE;
> +  for (Index = 0; Index < HandleCount; Index++) {
> +    Status = gBS->HandleProtocol (
> +                    HandleBuffer[Index],
> +
> &gEdkiiPeCoffImageEmulatorProtocolGuid,
> +                    (VOID **)&Emu
> +                    );
> +    ASSERT_EFI_ERROR (Status);
> +
> +    if (Emu->IsImageSupported (Emu, MachineType,
> SubSystem, DevicePath)) {
> +      ReturnValue = TRUE;
> +      break;
> +    }
> +  }
> +
> +  FreePool (HandleBuffer);
> +  return ReturnValue;
> +}
> +
>  /**
>    Load and start the Option Rom image.
> 
> @@ -715,7 +764,9 @@ ProcessOpRomImage (
>      //
>      // Skip the EFI PCI Option ROM image if its
> machine type is not supported
>      //
> -    if (!EFI_IMAGE_MACHINE_TYPE_SUPPORTED
> (EfiRomHeader->EfiMachineType)) {
> +    if (!IsImageTypeSupported(EfiRomHeader-
> >EfiMachineType,
> +                              EfiRomHeader-
> >EfiSubsystem,
> +                              PciDevice->DevicePath))
> {
>        goto NextImage;
>      }
> 
> --
> 2.17.1