From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f68.google.com (mail-wm1-f68.google.com [209.85.128.68]) by mx.groups.io with SMTP id smtpd.web09.587.1582753223475322212 for ; Wed, 26 Feb 2020 13:40:24 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=AfeW+2ZJ; spf=pass (domain: linaro.org, ip: 209.85.128.68, mailfrom: ard.biesheuvel@linaro.org) Received: by mail-wm1-f68.google.com with SMTP id a9so970799wmj.3 for ; Wed, 26 Feb 2020 13:40:23 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=xp0NYSl//ft5LBZXS3HFwaI9mEpmXFQGKc6Z1z//We4=; b=AfeW+2ZJn/XBteK+nbh0b9MgH0O2ciVMjNY+Z6PhpJWuczwTHPyCWs41nR5Wmra9Is 6Ka9WyPTpOtq7b7qH0ASWUJbC2Y0ZGisJq/L3Rt3h2uTFXU+BcKVMVkBb5E/cl3YMdHb 3abuydydfMPHT38UWnriCdPDFj7YjE8ouMmmFzQtr/en05v3NqG4HTAs5yyfo6zSyTUu M0XWKilKYuB+gA8WoKEGYyH27G/xstlrwmFIy0+1FLbemhInY9k1vvue0HV/Oh9jYR4O DN/QWvDPxVu85eEd9skkTLr2XXWIssOAPiJBS9/J0YvcbNA2ibQvLjqOpcqgTbXTmDg6 38GQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=xp0NYSl//ft5LBZXS3HFwaI9mEpmXFQGKc6Z1z//We4=; b=desik2tgKLEJlwnuRv2Go/0WiAEdgjfryTZlosfspebE1i2f7Nows8YG8V0fhMuz2G 4zat+4AslvXdi8hMa5xGxj928p/0liI0L0aTem1HZVwFDoh4g8KSQttLNLi+97SuTHT1 qkKYRtc/nHG4COUN0bLXX/CKijkwf12KWsFnOeE/xCC8cxViBr6g/CnKO3lNj8z1PJAZ OT1gZrKV8r4ECQwNTNgRf3w7Qt4Oi096j7rVl3FPWcXExDFrN1RLe/up2FVW2iFU3VfJ 8+4ACy7T2JJjAc2oGMEEkUnkkaJrVkB7CQKEix1hlw80g2Qf7IKLEZHoRU0swnPJaYdi GJQg== X-Gm-Message-State: APjAAAUXJ1MWwoRyEbH7KEwB9mcIAnSSAbiIyHQSBR5uh/Bn7C+q0UkG R5iqmy3mW6hUQb2LhJhPRJp7tryDJ9q2e8FYzR9PZg== X-Google-Smtp-Source: APXvYqypd0UENnmZK+y6HISrDCXhWKI9Yrw0HxCwjMeapbpeDmkGJybq4DHogCd6e/vb/dseN//dVUn01u6k0KasZSQ= X-Received: by 2002:a7b:cb93:: with SMTP id m19mr915082wmi.133.1582753221823; Wed, 26 Feb 2020 13:40:21 -0800 (PST) MIME-Version: 1.0 References: <20200226194343.2985-1-ard.biesheuvel@linaro.org> <20200226194343.2985-7-ard.biesheuvel@linaro.org> <4d9d61eb-63ba-ed45-a2f9-ba389e75e56c@hpe.com> In-Reply-To: <4d9d61eb-63ba-ed45-a2f9-ba389e75e56c@hpe.com> From: "Ard Biesheuvel" Date: Wed, 26 Feb 2020 22:40:10 +0100 Message-ID: Subject: Re: [edk2-devel] [PATCH v3 6/6] OvmfPkg IA32: add support for loading X64 images To: "Brian J. Johnson" Cc: edk2-devel-groups-io , Laszlo Ersek , Leif Lindholm , "Kinney, Michael D" , Jian J Wang , "Wu, Hao A" , "Ni, Ray" , Zhichao Gao Content-Type: text/plain; charset="UTF-8" On Wed, 26 Feb 2020 at 21:32, Brian J. Johnson wrote: > > On 2/26/20 1:43 PM, 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 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). > > > > Signed-off-by: Ard Biesheuvel > > Acked-by: Laszlo Ersek > > --- > > OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.c | 139 ++++++++++++++++++++ > > OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.inf | 36 +++++ > > OvmfPkg/OvmfPkgIa32.dsc | 5 + > > OvmfPkg/OvmfPkgIa32.fdf | 4 + > > 4 files changed, 184 insertions(+) > > > > diff --git a/OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.c b/OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.c > > new file mode 100644 > > index 000000000000..6dc07f467752 > > --- /dev/null > > +++ b/OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.c > > @@ -0,0 +1,139 @@ > > +/** @file > > + * PE/COFF emulator protocol implementation to start Linux kernel > > + * images from non-native firmware > > + * > > + * Copyright (c) 2020, ARM Ltd. All rights reserved.
> > + * > > + * SPDX-License-Identifier: BSD-2-Clause-Patent > > + * > > + */ > > + > > +#include > > + > > +#include > > +#include > > +#include > > + > > +#include > > + > > +#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; > > + > > + 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); > > + > > + while (PeCompat->Type != 0) { > > + 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); > > + } > > Ard, > > Cool patch series! > Thanks! > I'm not an official reviewer, but I'd feel better about this patch if > you added a condition to exit the "while (PeCompat->Type != 0)" loop if > PeCompat ever gets pointed outside of the section. Otherwise a > malformed or corrupted .compat section could send you off dereferencing > anything at all. > Good point, I'll look into that. > Similarly, it wouldn't hurt to sanity check the header fields, such as > e_lfanew, OptionalHeader, SizeOfOptionalHeader, and NumberOfSections (or > at least verify that all pointers you calculate from them point within > the overall image. Or has that already been done by the PeCoff loader > by the time this code is called? > At this point, we should be able to rely on the LoadImage() boot service to have sanity checked the headers, since we also have to rely on the section having been copied into memory at the correct address. > > + } > > + 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..82369384fbe6 > > --- /dev/null > > +++ b/OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.inf > > @@ -0,0 +1,36 @@ > > +## @file > > +# PE/COFF emulator protocol implementation to start Linux kernel > > +# images from non-native firmware > > +# > > +# Copyright (c) 2020, ARM Ltd. All rights reserved.
> > +# > > +# 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 > > + 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 b6cd5da4f2b3..ff8d80859fb9 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] > > > > > -- > > Brian > > -------------------------------------------------------------------- > > "I don't believe personal letters sent bulk rate." > -- me