Re: [PATCH v3 4/7] MdeModulePkg/UefiBootManagerLib: allow foreign Driver#### images

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

Hi Ard,

Similar to my PciBusDxe feedback, it would be better if
the image supported information was determined by calling
LoadImage() and checking for an EFI_UNSUPPORTED return
value.

This also has the advantage of reducing the number of 
components that need to be aware of any new protocols
associated with emulation with the best case being the
DXE Core and the modules that provide the emulators.

Thanks,

Mike

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-
> bounces@lists.01.org] On Behalf Of Ard Biesheuvel
> Sent: Thursday, September 20, 2018 4:02 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Zimmer, Vincent
> <vincent.zimmer@intel.com>; Dong, Eric
> <eric.dong@intel.com>; Andrew Fish <afish@apple.com>;
> Carsey, Jaben <jaben.carsey@intel.com>; Richardson,
> Brian <brian.richardson@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Zeng, Star
> <star.zeng@intel.com>
> Subject: [edk2] [PATCH v3 4/7]
> MdeModulePkg/UefiBootManagerLib: allow foreign
> Driver#### images
> 
> Allow PE/COFF images that must execute under emulation
> for Driver####
> options, by relaxing the machine type check to include
> support for
> machine types that is provided by an emulator.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel
> <ard.biesheuvel@linaro.org>
> ---
>  MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
> | 51 +++++++++++++++++++-
>  MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
> |  1 +
> 
> MdeModulePkg/Library/UefiBootManagerLib/UefiBootManager
> Lib.inf |  1 +
>  3 files changed, 52 insertions(+), 1 deletion(-)
> 
> diff --git
> a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.
> c
> b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.
> c
> index 7bf96646c690..f6fda8f2c3f7 100644
> ---
> a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.
> c
> +++
> b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.
> c
> @@ -1185,6 +1185,54 @@ EfiBootManagerFreeLoadOptions (
>    return EFI_SUCCESS;
>  }
> 
> +STATIC
> +BOOLEAN
> +BmIsImageTypeSupported (
> +  IN  UINT16    MachineType,
> +  IN  UINT16    SubSystem
> +  )
> +{
> +  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, NULL)) {
> +      ReturnValue = TRUE;
> +      break;
> +    }
> +  }
> +
> +  FreePool (HandleBuffer);
> +  return ReturnValue;
> +}
> +
>  /**
>    Return whether the PE header of the load option is
> valid or not.
> 
> @@ -1235,7 +1283,8 @@ BmIsLoadOptionPeHeaderValid (
>        OptionalHeader = (EFI_IMAGE_OPTIONAL_HEADER32 *)
> &PeHeader->Pe32.OptionalHeader;
>        if ((OptionalHeader->Magic ==
> EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC ||
>             OptionalHeader->Magic ==
> EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) &&
> -          EFI_IMAGE_MACHINE_TYPE_SUPPORTED (PeHeader-
> >Pe32.FileHeader.Machine)
> +          BmIsImageTypeSupported (PeHeader-
> >Pe32.FileHeader.Machine,
> +                                  OptionalHeader-
> >Subsystem)
>            ) {
>          //
>          // Check the Subsystem:
> diff --git
> a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
> b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
> index 978fbff966f6..5f64ef304b87 100644
> ---
> a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
> +++
> b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
> @@ -47,6 +47,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS
> OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>  #include <Protocol/VariableLock.h>
>  #include <Protocol/RamDisk.h>
>  #include <Protocol/DeferredImageLoad.h>
> +#include <Protocol/PeCoffImageEmulator.h>
> 
>  #include <Guid/MemoryTypeInformation.h>
>  #include <Guid/FileInfo.h>
> diff --git
> a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManag
> erLib.inf
> b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManag
> erLib.inf
> index 72c5ca1cd59e..09e2134acf8e 100644
> ---
> a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManag
> erLib.inf
> +++
> b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManag
> erLib.inf
> @@ -104,6 +104,7 @@
>    gEfiDevicePathProtocolGuid                    ##
> SOMETIMES_CONSUMES
>    gEfiBootLogoProtocolGuid                      ##
> SOMETIMES_CONSUMES
>    gEfiSimpleTextInputExProtocolGuid             ##
> SOMETIMES_CONSUMES
> +  gEdkiiPeCoffImageEmulatorProtocolGuid         ##
> SOMETIMES_CONSUMES
>    gEdkiiVariableLockProtocolGuid                ##
> SOMETIMES_CONSUMES
>    gEfiGraphicsOutputProtocolGuid                ##
> SOMETIMES_CONSUMES
>    gEfiUsbIoProtocolGuid                         ##
> SOMETIMES_CONSUMES
> --
> 2.17.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel