| Message ID | 20180920230145.7565-5-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | MdeModulePkg: add support for dispatching foreign arch PE/COFF images | expand |
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 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Thu, 27 Sep 2018 at 01:34, Kinney, Michael D <michael.d.kinney@intel.com> wrote: > > 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. > In this case, yes, I think we can drop the machine type check. > > -----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 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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/UefiBootManagerLib.inf b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf index 72c5ca1cd59e..09e2134acf8e 100644 --- a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf +++ b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.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
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/UefiBootManagerLib.inf | 1 + 3 files changed, 52 insertions(+), 1 deletion(-) -- 2.17.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel