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

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

On 13 September 2018 at 12:24, Zeng, Star <star.zeng@intel.com> wrote:
> 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?
>

Yes.