[edk2-devel] [PATCH v4 6/7] OvmfPkg IA32: add support for loading X64 images

Ard Biesheuvel posted 7 patches 5 years, 11 months ago
[edk2-devel] [PATCH v4 6/7] OvmfPkg IA32: add support for loading X64 images
Posted by Ard Biesheuvel 5 years, 11 months ago
This is the UEFI counterpart to my Linux series which generalizes
mixed mode support into a feature that requires very little internal
knowledge about the architecture specifics of booting Linux on the
part of the bootloader or firmware.

Instead, we add a .compat PE/COFF header containing an array of
PE_COMPAT nodes containing <machine type, entrypoint> tuples that
describe alternate entrypoints into the image for different native
machine types, e.g., IA-32 in a 64-bit image so it can be booted
from IA-32 firmware.

This patch implements the PE/COFF emulator protocol to take this new
section into account, so that such images can simply be loaded via
LoadImage/StartImage, e.g., straight from the shell.

This feature is based on the EDK2 specific PE/COFF emulator protocol
that was introduced in commit 57df17fe26cd ("MdeModulePkg/DxeCore:
invoke the emulator protocol for foreign images", 2019-04-14).

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2564
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Acked-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.c   | 143 ++++++++++++++++++++
 OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.inf |  37 +++++
 OvmfPkg/OvmfPkgIa32.dsc                               |   5 +
 OvmfPkg/OvmfPkgIa32.fdf                               |   4 +
 4 files changed, 189 insertions(+)

diff --git a/OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.c b/OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.c
new file mode 100644
index 000000000000..ae47917f1589
--- /dev/null
+++ b/OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.c
@@ -0,0 +1,143 @@
+/** @file
+ *  PE/COFF emulator protocol implementation to start Linux kernel
+ *  images from non-native firmware
+ *
+ *  Copyright (c) 2020, ARM Ltd. All rights reserved.<BR>
+ *
+ *  SPDX-License-Identifier: BSD-2-Clause-Patent
+ *
+ */
+
+#include <PiDxe.h>
+
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/PeCoffLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+
+#include <Protocol/PeCoffImageEmulator.h>
+
+#pragma pack (1)
+typedef struct {
+  UINT8   Type;
+  UINT8   Size;
+  UINT16  MachineType;
+  UINT32  EntryPoint;
+} PE_COMPAT_TYPE1;
+#pragma pack ()
+
+STATIC
+BOOLEAN
+EFIAPI
+IsImageSupported (
+  IN  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL    *This,
+  IN  UINT16                                  ImageType,
+  IN  EFI_DEVICE_PATH_PROTOCOL                *DevicePath   OPTIONAL
+  )
+{
+  return ImageType == EFI_IMAGE_SUBSYSTEM_EFI_APPLICATION;
+}
+
+STATIC
+EFI_IMAGE_ENTRY_POINT
+EFIAPI
+GetCompatEntryPoint (
+  IN  EFI_PHYSICAL_ADDRESS              ImageBase
+  )
+{
+  EFI_IMAGE_DOS_HEADER                  *DosHdr;
+  UINTN                                 PeCoffHeaderOffset;
+  EFI_IMAGE_NT_HEADERS32                *Pe32;
+  EFI_IMAGE_SECTION_HEADER              *Section;
+  UINTN                                 NumberOfSections;
+  PE_COMPAT_TYPE1                       *PeCompat;
+  VOID                                  *PeCompatEnd;
+
+  DosHdr = (EFI_IMAGE_DOS_HEADER *)(UINTN)ImageBase;
+  if (DosHdr->e_magic != EFI_IMAGE_DOS_SIGNATURE) {
+    return NULL;
+  }
+
+  PeCoffHeaderOffset = DosHdr->e_lfanew;
+  Pe32 = (EFI_IMAGE_NT_HEADERS32 *)((UINTN)ImageBase + PeCoffHeaderOffset);
+
+  Section = (EFI_IMAGE_SECTION_HEADER *)((UINTN)&Pe32->OptionalHeader +
+                                         Pe32->FileHeader.SizeOfOptionalHeader);
+  NumberOfSections = (UINTN)Pe32->FileHeader.NumberOfSections;
+
+  while (NumberOfSections--) {
+    if (!CompareMem (Section->Name, ".compat", sizeof (Section->Name))) {
+      //
+      // Dereference the section contents to find the mixed mode entry point
+      //
+      PeCompat = (PE_COMPAT_TYPE1 *)((UINTN)ImageBase + Section->VirtualAddress);
+      PeCompatEnd = (UINT8 *)PeCompat + Section->Misc.VirtualSize;
+
+      while (PeCompat->Type != 0 && (VOID *)PeCompat < PeCompatEnd) {
+        if (PeCompat->Type == 1 &&
+            PeCompat->Size >= sizeof (PE_COMPAT_TYPE1) &&
+            EFI_IMAGE_MACHINE_TYPE_SUPPORTED (PeCompat->MachineType)) {
+
+          return (EFI_IMAGE_ENTRY_POINT)((UINTN)ImageBase + PeCompat->EntryPoint);
+        }
+        PeCompat = (PE_COMPAT_TYPE1 *)((UINTN)PeCompat + PeCompat->Size);
+        ASSERT ((VOID *)PeCompat < PeCompatEnd);
+      }
+    }
+    Section++;
+  }
+  return NULL;
+}
+
+STATIC
+EFI_STATUS
+EFIAPI
+RegisterImage (
+  IN      EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL    *This,
+  IN      EFI_PHYSICAL_ADDRESS                    ImageBase,
+  IN      UINT64                                  ImageSize,
+  IN  OUT EFI_IMAGE_ENTRY_POINT                   *EntryPoint
+  )
+{
+  EFI_IMAGE_ENTRY_POINT                           CompatEntryPoint;
+
+  CompatEntryPoint = GetCompatEntryPoint (ImageBase);
+  if (CompatEntryPoint == NULL) {
+    return EFI_UNSUPPORTED;
+  }
+
+  *EntryPoint = CompatEntryPoint;
+  return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+EFIAPI
+UnregisterImage (
+  IN  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL    *This,
+  IN  EFI_PHYSICAL_ADDRESS                    ImageBase
+  )
+{
+  return EFI_SUCCESS;
+}
+
+STATIC EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL mCompatLoaderPeCoffEmuProtocol = {
+  IsImageSupported,
+  RegisterImage,
+  UnregisterImage,
+  EDKII_PECOFF_IMAGE_EMULATOR_VERSION,
+  EFI_IMAGE_MACHINE_X64
+};
+
+EFI_STATUS
+EFIAPI
+CompatImageLoaderDxeEntryPoint (
+  IN EFI_HANDLE        ImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  return gBS->InstallProtocolInterface (&ImageHandle,
+                &gEdkiiPeCoffImageEmulatorProtocolGuid,
+                EFI_NATIVE_INTERFACE,
+                &mCompatLoaderPeCoffEmuProtocol);
+}
diff --git a/OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.inf b/OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.inf
new file mode 100644
index 000000000000..74f06c64bfbf
--- /dev/null
+++ b/OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.inf
@@ -0,0 +1,37 @@
+## @file
+#  PE/COFF emulator protocol implementation to start Linux kernel
+#  images from non-native firmware
+#
+#  Copyright (c) 2020, ARM Ltd. All rights reserved.<BR>
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 1.27
+  BASE_NAME                      = CompatImageLoaderDxe
+  FILE_GUID                      = 1019f54a-2560-41b2-87b0-6750b98f3eff
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+  ENTRY_POINT                    = CompatImageLoaderDxeEntryPoint
+
+[Sources]
+  CompatImageLoaderDxe.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+
+[LibraryClasses]
+  BaseMemoryLib
+  DebugLib
+  PeCoffLib
+  UefiBootServicesTableLib
+  UefiDriverEntryPoint
+
+[Protocols]
+  gEdkiiPeCoffImageEmulatorProtocolGuid   ## PRODUCES
+
+[Depex]
+  TRUE
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 76e52a3de120..8d91903f8b4e 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -33,6 +33,7 @@ [Defines]
   DEFINE SOURCE_DEBUG_ENABLE     = FALSE
   DEFINE TPM2_ENABLE             = FALSE
   DEFINE TPM2_CONFIG_ENABLE      = FALSE
+  DEFINE LOAD_X64_ON_IA32_ENABLE = FALSE
 
   #
   # Network definition
@@ -932,3 +933,7 @@ [Components]
   SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
 !endif
 !endif
+
+!if $(LOAD_X64_ON_IA32_ENABLE) == TRUE
+  OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.inf
+!endif
diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
index 6c342823d206..f57de4a26f92 100644
--- a/OvmfPkg/OvmfPkgIa32.fdf
+++ b/OvmfPkg/OvmfPkgIa32.fdf
@@ -354,6 +354,10 @@ [FV.DXEFV]
 !endif
 !endif
 
+!if $(LOAD_X64_ON_IA32_ENABLE) == TRUE
+INF  OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.inf
+!endif
+
 ################################################################################
 
 [FV.FVMAIN_COMPACT]
-- 
2.17.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55334): https://edk2.groups.io/g/devel/message/55334
Mute This Topic: https://groups.io/mt/71700588/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v4 6/7] OvmfPkg IA32: add support for loading X64 images
Posted by Laszlo Ersek 5 years, 11 months ago
On 03/03/20 15:01, Ard Biesheuvel wrote:
> This is the UEFI counterpart to my Linux series which generalizes
> mixed mode support into a feature that requires very little internal
> knowledge about the architecture specifics of booting Linux on the
> part of the bootloader or firmware.
> 
> Instead, we add a .compat PE/COFF header containing an array of
> PE_COMPAT nodes containing <machine type, entrypoint> tuples that
> describe alternate entrypoints into the image for different native
> machine types, e.g., IA-32 in a 64-bit image so it can be booted
> from IA-32 firmware.
> 
> This patch implements the PE/COFF emulator protocol to take this new
> section into account, so that such images can simply be loaded via
> LoadImage/StartImage, e.g., straight from the shell.
> 
> This feature is based on the EDK2 specific PE/COFF emulator protocol
> that was introduced in commit 57df17fe26cd ("MdeModulePkg/DxeCore:
> invoke the emulator protocol for foreign images", 2019-04-14).
> 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2564
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Acked-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.c   | 143 ++++++++++++++++++++
>  OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.inf |  37 +++++
>  OvmfPkg/OvmfPkgIa32.dsc                               |   5 +
>  OvmfPkg/OvmfPkgIa32.fdf                               |   4 +
>  4 files changed, 189 insertions(+)
> 
> diff --git a/OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.c b/OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.c
> new file mode 100644
> index 000000000000..ae47917f1589
> --- /dev/null
> +++ b/OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.c
> @@ -0,0 +1,143 @@
> +/** @file
> + *  PE/COFF emulator protocol implementation to start Linux kernel
> + *  images from non-native firmware
> + *
> + *  Copyright (c) 2020, ARM Ltd. All rights reserved.<BR>
> + *
> + *  SPDX-License-Identifier: BSD-2-Clause-Patent
> + *
> + */
> +
> +#include <PiDxe.h>
> +
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/PeCoffLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +
> +#include <Protocol/PeCoffImageEmulator.h>
> +
> +#pragma pack (1)
> +typedef struct {
> +  UINT8   Type;
> +  UINT8   Size;
> +  UINT16  MachineType;
> +  UINT32  EntryPoint;
> +} PE_COMPAT_TYPE1;
> +#pragma pack ()
> +
> +STATIC
> +BOOLEAN
> +EFIAPI
> +IsImageSupported (
> +  IN  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL    *This,
> +  IN  UINT16                                  ImageType,
> +  IN  EFI_DEVICE_PATH_PROTOCOL                *DevicePath   OPTIONAL
> +  )
> +{
> +  return ImageType == EFI_IMAGE_SUBSYSTEM_EFI_APPLICATION;
> +}
> +
> +STATIC
> +EFI_IMAGE_ENTRY_POINT
> +EFIAPI
> +GetCompatEntryPoint (
> +  IN  EFI_PHYSICAL_ADDRESS              ImageBase
> +  )
> +{
> +  EFI_IMAGE_DOS_HEADER                  *DosHdr;
> +  UINTN                                 PeCoffHeaderOffset;
> +  EFI_IMAGE_NT_HEADERS32                *Pe32;
> +  EFI_IMAGE_SECTION_HEADER              *Section;
> +  UINTN                                 NumberOfSections;
> +  PE_COMPAT_TYPE1                       *PeCompat;
> +  VOID                                  *PeCompatEnd;
> +
> +  DosHdr = (EFI_IMAGE_DOS_HEADER *)(UINTN)ImageBase;
> +  if (DosHdr->e_magic != EFI_IMAGE_DOS_SIGNATURE) {
> +    return NULL;
> +  }
> +
> +  PeCoffHeaderOffset = DosHdr->e_lfanew;
> +  Pe32 = (EFI_IMAGE_NT_HEADERS32 *)((UINTN)ImageBase + PeCoffHeaderOffset);
> +
> +  Section = (EFI_IMAGE_SECTION_HEADER *)((UINTN)&Pe32->OptionalHeader +
> +                                         Pe32->FileHeader.SizeOfOptionalHeader);
> +  NumberOfSections = (UINTN)Pe32->FileHeader.NumberOfSections;
> +
> +  while (NumberOfSections--) {
> +    if (!CompareMem (Section->Name, ".compat", sizeof (Section->Name))) {
> +      //
> +      // Dereference the section contents to find the mixed mode entry point
> +      //
> +      PeCompat = (PE_COMPAT_TYPE1 *)((UINTN)ImageBase + Section->VirtualAddress);
> +      PeCompatEnd = (UINT8 *)PeCompat + Section->Misc.VirtualSize;
> +
> +      while (PeCompat->Type != 0 && (VOID *)PeCompat < PeCompatEnd) {
> +        if (PeCompat->Type == 1 &&
> +            PeCompat->Size >= sizeof (PE_COMPAT_TYPE1) &&
> +            EFI_IMAGE_MACHINE_TYPE_SUPPORTED (PeCompat->MachineType)) {
> +
> +          return (EFI_IMAGE_ENTRY_POINT)((UINTN)ImageBase + PeCompat->EntryPoint);
> +        }
> +        PeCompat = (PE_COMPAT_TYPE1 *)((UINTN)PeCompat + PeCompat->Size);
> +        ASSERT ((VOID *)PeCompat < PeCompatEnd);

The new pointer comparisons make me really uncomfortable. They look
doubly undefined:

(a) comparing (VOID*) against a pointer to a complete type. The C99
standard says:

------
6.5.8 Relational operators

Constraints

2 One of the following shall hold:
  - both operands have real type;
  - both operands are pointers to qualified or unqualified versions of
    compatible object types; or
  - both operands are pointers to qualified or unqualified versions of
    compatible incomplete types.
------

"void" is an incomplete type that cannot be completed (it is never an
"object type"), and PE_COMPAT_TYPE1 is a complete type (we know its
size). So none of the permitted cases apply.

(b) I don't want to quote all of paragraph 5, but the point is, the
comparisons invoke undefined behavior *at least* when we'd expect them
to evaluate to FALSE. (Basically: when PeCompat does not point to an
element in the array whose last element PeCompatEnd points one past.)


As one alternative, please introduce "PeCompatEnd" as UINTN, set it with:

  PeCompatEnd = (UINTN)(VOID *)PeCompat + Section->Misc.VirtualSize;

and replace the

  (VOID *)PeCompat < PeCompatEnd

comparisons with

  (UINTN)(VOID *)PeCompat < PeCompatEnd

I'm not requesting a repost just for this, you can keep the A-b.

Thanks
Laszlo



> +      }
> +    }
> +    Section++;
> +  }
> +  return NULL;
> +}
> +
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +RegisterImage (
> +  IN      EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL    *This,
> +  IN      EFI_PHYSICAL_ADDRESS                    ImageBase,
> +  IN      UINT64                                  ImageSize,
> +  IN  OUT EFI_IMAGE_ENTRY_POINT                   *EntryPoint
> +  )
> +{
> +  EFI_IMAGE_ENTRY_POINT                           CompatEntryPoint;
> +
> +  CompatEntryPoint = GetCompatEntryPoint (ImageBase);
> +  if (CompatEntryPoint == NULL) {
> +    return EFI_UNSUPPORTED;
> +  }
> +
> +  *EntryPoint = CompatEntryPoint;
> +  return EFI_SUCCESS;
> +}
> +
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +UnregisterImage (
> +  IN  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL    *This,
> +  IN  EFI_PHYSICAL_ADDRESS                    ImageBase
> +  )
> +{
> +  return EFI_SUCCESS;
> +}
> +
> +STATIC EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL mCompatLoaderPeCoffEmuProtocol = {
> +  IsImageSupported,
> +  RegisterImage,
> +  UnregisterImage,
> +  EDKII_PECOFF_IMAGE_EMULATOR_VERSION,
> +  EFI_IMAGE_MACHINE_X64
> +};
> +
> +EFI_STATUS
> +EFIAPI
> +CompatImageLoaderDxeEntryPoint (
> +  IN EFI_HANDLE        ImageHandle,
> +  IN EFI_SYSTEM_TABLE  *SystemTable
> +  )
> +{
> +  return gBS->InstallProtocolInterface (&ImageHandle,
> +                &gEdkiiPeCoffImageEmulatorProtocolGuid,
> +                EFI_NATIVE_INTERFACE,
> +                &mCompatLoaderPeCoffEmuProtocol);
> +}
> diff --git a/OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.inf b/OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.inf
> new file mode 100644
> index 000000000000..74f06c64bfbf
> --- /dev/null
> +++ b/OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.inf
> @@ -0,0 +1,37 @@
> +## @file
> +#  PE/COFF emulator protocol implementation to start Linux kernel
> +#  images from non-native firmware
> +#
> +#  Copyright (c) 2020, ARM Ltd. All rights reserved.<BR>
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 1.27
> +  BASE_NAME                      = CompatImageLoaderDxe
> +  FILE_GUID                      = 1019f54a-2560-41b2-87b0-6750b98f3eff
> +  MODULE_TYPE                    = DXE_DRIVER
> +  VERSION_STRING                 = 1.0
> +  ENTRY_POINT                    = CompatImageLoaderDxeEntryPoint
> +
> +[Sources]
> +  CompatImageLoaderDxe.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +
> +[LibraryClasses]
> +  BaseMemoryLib
> +  DebugLib
> +  PeCoffLib
> +  UefiBootServicesTableLib
> +  UefiDriverEntryPoint
> +
> +[Protocols]
> +  gEdkiiPeCoffImageEmulatorProtocolGuid   ## PRODUCES
> +
> +[Depex]
> +  TRUE
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 76e52a3de120..8d91903f8b4e 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -33,6 +33,7 @@ [Defines]
>    DEFINE SOURCE_DEBUG_ENABLE     = FALSE
>    DEFINE TPM2_ENABLE             = FALSE
>    DEFINE TPM2_CONFIG_ENABLE      = FALSE
> +  DEFINE LOAD_X64_ON_IA32_ENABLE = FALSE
>  
>    #
>    # Network definition
> @@ -932,3 +933,7 @@ [Components]
>    SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
>  !endif
>  !endif
> +
> +!if $(LOAD_X64_ON_IA32_ENABLE) == TRUE
> +  OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.inf
> +!endif
> diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
> index 6c342823d206..f57de4a26f92 100644
> --- a/OvmfPkg/OvmfPkgIa32.fdf
> +++ b/OvmfPkg/OvmfPkgIa32.fdf
> @@ -354,6 +354,10 @@ [FV.DXEFV]
>  !endif
>  !endif
>  
> +!if $(LOAD_X64_ON_IA32_ENABLE) == TRUE
> +INF  OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.inf
> +!endif
> +
>  ################################################################################
>  
>  [FV.FVMAIN_COMPACT]
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55343): https://edk2.groups.io/g/devel/message/55343
Mute This Topic: https://groups.io/mt/71700588/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v4 6/7] OvmfPkg IA32: add support for loading X64 images
Posted by Ard Biesheuvel 5 years, 11 months ago
On Tue, 3 Mar 2020 at 21:54, Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 03/03/20 15:01, Ard Biesheuvel wrote:
> > This is the UEFI counterpart to my Linux series which generalizes
> > mixed mode support into a feature that requires very little internal
> > knowledge about the architecture specifics of booting Linux on the
> > part of the bootloader or firmware.
> >
> > Instead, we add a .compat PE/COFF header containing an array of
> > PE_COMPAT nodes containing <machine type, entrypoint> tuples that
> > describe alternate entrypoints into the image for different native
> > machine types, e.g., IA-32 in a 64-bit image so it can be booted
> > from IA-32 firmware.
> >
> > This patch implements the PE/COFF emulator protocol to take this new
> > section into account, so that such images can simply be loaded via
> > LoadImage/StartImage, e.g., straight from the shell.
> >
> > This feature is based on the EDK2 specific PE/COFF emulator protocol
> > that was introduced in commit 57df17fe26cd ("MdeModulePkg/DxeCore:
> > invoke the emulator protocol for foreign images", 2019-04-14).
> >
> > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2564
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Acked-by: Laszlo Ersek <lersek@redhat.com>
> > ---
> >  OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.c   | 143 ++++++++++++++++++++
> >  OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.inf |  37 +++++
> >  OvmfPkg/OvmfPkgIa32.dsc                               |   5 +
> >  OvmfPkg/OvmfPkgIa32.fdf                               |   4 +
> >  4 files changed, 189 insertions(+)
> >
> > diff --git a/OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.c b/OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.c
> > new file mode 100644
> > index 000000000000..ae47917f1589
> > --- /dev/null
> > +++ b/OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.c
> > @@ -0,0 +1,143 @@
> > +/** @file
> > + *  PE/COFF emulator protocol implementation to start Linux kernel
> > + *  images from non-native firmware
> > + *
> > + *  Copyright (c) 2020, ARM Ltd. All rights reserved.<BR>
> > + *
> > + *  SPDX-License-Identifier: BSD-2-Clause-Patent
> > + *
> > + */
> > +
> > +#include <PiDxe.h>
> > +
> > +#include <Library/BaseMemoryLib.h>
> > +#include <Library/DebugLib.h>
> > +#include <Library/PeCoffLib.h>
> > +#include <Library/UefiBootServicesTableLib.h>
> > +
> > +#include <Protocol/PeCoffImageEmulator.h>
> > +
> > +#pragma pack (1)
> > +typedef struct {
> > +  UINT8   Type;
> > +  UINT8   Size;
> > +  UINT16  MachineType;
> > +  UINT32  EntryPoint;
> > +} PE_COMPAT_TYPE1;
> > +#pragma pack ()
> > +
> > +STATIC
> > +BOOLEAN
> > +EFIAPI
> > +IsImageSupported (
> > +  IN  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL    *This,
> > +  IN  UINT16                                  ImageType,
> > +  IN  EFI_DEVICE_PATH_PROTOCOL                *DevicePath   OPTIONAL
> > +  )
> > +{
> > +  return ImageType == EFI_IMAGE_SUBSYSTEM_EFI_APPLICATION;
> > +}
> > +
> > +STATIC
> > +EFI_IMAGE_ENTRY_POINT
> > +EFIAPI
> > +GetCompatEntryPoint (
> > +  IN  EFI_PHYSICAL_ADDRESS              ImageBase
> > +  )
> > +{
> > +  EFI_IMAGE_DOS_HEADER                  *DosHdr;
> > +  UINTN                                 PeCoffHeaderOffset;
> > +  EFI_IMAGE_NT_HEADERS32                *Pe32;
> > +  EFI_IMAGE_SECTION_HEADER              *Section;
> > +  UINTN                                 NumberOfSections;
> > +  PE_COMPAT_TYPE1                       *PeCompat;
> > +  VOID                                  *PeCompatEnd;
> > +
> > +  DosHdr = (EFI_IMAGE_DOS_HEADER *)(UINTN)ImageBase;
> > +  if (DosHdr->e_magic != EFI_IMAGE_DOS_SIGNATURE) {
> > +    return NULL;
> > +  }
> > +
> > +  PeCoffHeaderOffset = DosHdr->e_lfanew;
> > +  Pe32 = (EFI_IMAGE_NT_HEADERS32 *)((UINTN)ImageBase + PeCoffHeaderOffset);
> > +
> > +  Section = (EFI_IMAGE_SECTION_HEADER *)((UINTN)&Pe32->OptionalHeader +
> > +                                         Pe32->FileHeader.SizeOfOptionalHeader);
> > +  NumberOfSections = (UINTN)Pe32->FileHeader.NumberOfSections;
> > +
> > +  while (NumberOfSections--) {
> > +    if (!CompareMem (Section->Name, ".compat", sizeof (Section->Name))) {
> > +      //
> > +      // Dereference the section contents to find the mixed mode entry point
> > +      //
> > +      PeCompat = (PE_COMPAT_TYPE1 *)((UINTN)ImageBase + Section->VirtualAddress);
> > +      PeCompatEnd = (UINT8 *)PeCompat + Section->Misc.VirtualSize;
> > +
> > +      while (PeCompat->Type != 0 && (VOID *)PeCompat < PeCompatEnd) {
> > +        if (PeCompat->Type == 1 &&
> > +            PeCompat->Size >= sizeof (PE_COMPAT_TYPE1) &&
> > +            EFI_IMAGE_MACHINE_TYPE_SUPPORTED (PeCompat->MachineType)) {
> > +
> > +          return (EFI_IMAGE_ENTRY_POINT)((UINTN)ImageBase + PeCompat->EntryPoint);
> > +        }
> > +        PeCompat = (PE_COMPAT_TYPE1 *)((UINTN)PeCompat + PeCompat->Size);
> > +        ASSERT ((VOID *)PeCompat < PeCompatEnd);
>
> The new pointer comparisons make me really uncomfortable. They look
> doubly undefined:
>
> (a) comparing (VOID*) against a pointer to a complete type. The C99
> standard says:
>
> ------
> 6.5.8 Relational operators
>
> Constraints
>
> 2 One of the following shall hold:
>   - both operands have real type;
>   - both operands are pointers to qualified or unqualified versions of
>     compatible object types; or
>   - both operands are pointers to qualified or unqualified versions of
>     compatible incomplete types.
> ------
>
> "void" is an incomplete type that cannot be completed (it is never an
> "object type"), and PE_COMPAT_TYPE1 is a complete type (we know its
> size). So none of the permitted cases apply.
>
> (b) I don't want to quote all of paragraph 5, but the point is, the
> comparisons invoke undefined behavior *at least* when we'd expect them
> to evaluate to FALSE. (Basically: when PeCompat does not point to an
> element in the array whose last element PeCompatEnd points one past.)
>
>
> As one alternative, please introduce "PeCompatEnd" as UINTN, set it with:
>
>   PeCompatEnd = (UINTN)(VOID *)PeCompat + Section->Misc.VirtualSize;
>
> and replace the
>
>   (VOID *)PeCompat < PeCompatEnd
>
> comparisons with
>
>   (UINTN)(VOID *)PeCompat < PeCompatEnd
>
> I'm not requesting a repost just for this, you can keep the A-b.
>

OK, thanks.

>
>
> > +      }
> > +    }
> > +    Section++;
> > +  }
> > +  return NULL;
> > +}
> > +
> > +STATIC
> > +EFI_STATUS
> > +EFIAPI
> > +RegisterImage (
> > +  IN      EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL    *This,
> > +  IN      EFI_PHYSICAL_ADDRESS                    ImageBase,
> > +  IN      UINT64                                  ImageSize,
> > +  IN  OUT EFI_IMAGE_ENTRY_POINT                   *EntryPoint
> > +  )
> > +{
> > +  EFI_IMAGE_ENTRY_POINT                           CompatEntryPoint;
> > +
> > +  CompatEntryPoint = GetCompatEntryPoint (ImageBase);
> > +  if (CompatEntryPoint == NULL) {
> > +    return EFI_UNSUPPORTED;
> > +  }
> > +
> > +  *EntryPoint = CompatEntryPoint;
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +STATIC
> > +EFI_STATUS
> > +EFIAPI
> > +UnregisterImage (
> > +  IN  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL    *This,
> > +  IN  EFI_PHYSICAL_ADDRESS                    ImageBase
> > +  )
> > +{
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +STATIC EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL mCompatLoaderPeCoffEmuProtocol = {
> > +  IsImageSupported,
> > +  RegisterImage,
> > +  UnregisterImage,
> > +  EDKII_PECOFF_IMAGE_EMULATOR_VERSION,
> > +  EFI_IMAGE_MACHINE_X64
> > +};
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +CompatImageLoaderDxeEntryPoint (
> > +  IN EFI_HANDLE        ImageHandle,
> > +  IN EFI_SYSTEM_TABLE  *SystemTable
> > +  )
> > +{
> > +  return gBS->InstallProtocolInterface (&ImageHandle,
> > +                &gEdkiiPeCoffImageEmulatorProtocolGuid,
> > +                EFI_NATIVE_INTERFACE,
> > +                &mCompatLoaderPeCoffEmuProtocol);
> > +}
> > diff --git a/OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.inf b/OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.inf
> > new file mode 100644
> > index 000000000000..74f06c64bfbf
> > --- /dev/null
> > +++ b/OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.inf
> > @@ -0,0 +1,37 @@
> > +## @file
> > +#  PE/COFF emulator protocol implementation to start Linux kernel
> > +#  images from non-native firmware
> > +#
> > +#  Copyright (c) 2020, ARM Ltd. All rights reserved.<BR>
> > +#
> > +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> > +#
> > +##
> > +
> > +[Defines]
> > +  INF_VERSION                    = 1.27
> > +  BASE_NAME                      = CompatImageLoaderDxe
> > +  FILE_GUID                      = 1019f54a-2560-41b2-87b0-6750b98f3eff
> > +  MODULE_TYPE                    = DXE_DRIVER
> > +  VERSION_STRING                 = 1.0
> > +  ENTRY_POINT                    = CompatImageLoaderDxeEntryPoint
> > +
> > +[Sources]
> > +  CompatImageLoaderDxe.c
> > +
> > +[Packages]
> > +  MdePkg/MdePkg.dec
> > +  MdeModulePkg/MdeModulePkg.dec
> > +
> > +[LibraryClasses]
> > +  BaseMemoryLib
> > +  DebugLib
> > +  PeCoffLib
> > +  UefiBootServicesTableLib
> > +  UefiDriverEntryPoint
> > +
> > +[Protocols]
> > +  gEdkiiPeCoffImageEmulatorProtocolGuid   ## PRODUCES
> > +
> > +[Depex]
> > +  TRUE
> > diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> > index 76e52a3de120..8d91903f8b4e 100644
> > --- a/OvmfPkg/OvmfPkgIa32.dsc
> > +++ b/OvmfPkg/OvmfPkgIa32.dsc
> > @@ -33,6 +33,7 @@ [Defines]
> >    DEFINE SOURCE_DEBUG_ENABLE     = FALSE
> >    DEFINE TPM2_ENABLE             = FALSE
> >    DEFINE TPM2_CONFIG_ENABLE      = FALSE
> > +  DEFINE LOAD_X64_ON_IA32_ENABLE = FALSE
> >
> >    #
> >    # Network definition
> > @@ -932,3 +933,7 @@ [Components]
> >    SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> >  !endif
> >  !endif
> > +
> > +!if $(LOAD_X64_ON_IA32_ENABLE) == TRUE
> > +  OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.inf
> > +!endif
> > diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
> > index 6c342823d206..f57de4a26f92 100644
> > --- a/OvmfPkg/OvmfPkgIa32.fdf
> > +++ b/OvmfPkg/OvmfPkgIa32.fdf
> > @@ -354,6 +354,10 @@ [FV.DXEFV]
> >  !endif
> >  !endif
> >
> > +!if $(LOAD_X64_ON_IA32_ENABLE) == TRUE
> > +INF  OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.inf
> > +!endif
> > +
> >  ################################################################################
> >
> >  [FV.FVMAIN_COMPACT]
> >
>

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55348): https://edk2.groups.io/g/devel/message/55348
Mute This Topic: https://groups.io/mt/71700588/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-