[edk2,v3,4/7] MdeModulePkg/UefiBootManagerLib: allow foreign Driver#### images

Message ID 20180920230145.7565-5-ard.biesheuvel@linaro.org
State New
Headers show
Series
  • MdeModulePkg: add support for dispatching foreign arch PE/COFF images
Related show

Commit Message

Ard Biesheuvel Sept. 20, 2018, 11:01 p.m.
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

Comments

Kinney, Michael D Sept. 26, 2018, 11:34 p.m. | #1
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
Ard Biesheuvel Dec. 27, 2018, 10:16 a.m. | #2
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

Patch

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