From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.81]) by mx.groups.io with SMTP id smtpd.web12.7220.1581691550594376767 for ; Fri, 14 Feb 2020 06:45:50 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=QCQNf5TZ; spf=pass (domain: redhat.com, ip: 207.211.31.81, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1581691549; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=dRmJmMFnW/dNqDgl70gP9aPbZ96pVVLqAnSFp6qZY+s=; b=QCQNf5TZD9gO75uu0hvNfmvDhRztYMjz/x6x8+FvW/dRy2mq96BIYGBm+Vvg7MtEdXu0zx OwFBoRsH9JlCV8rC9ACTciKy3znLHpBw8YzL2rXRLqw4F8c4VInhXHRZ2CfHisGdJ6CxVb Awbe44V2yKBpVgRG1doNtYAaOLF+f3w= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-123-9H1wR6eDNaa4kemll5U82A-1; Fri, 14 Feb 2020 09:45:38 -0500 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id AD73480271B; Fri, 14 Feb 2020 14:45:36 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-153.ams2.redhat.com [10.36.116.153]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6F1B919925; Fri, 14 Feb 2020 14:45:34 +0000 (UTC) Subject: Re: [PATCH 1/1] OvmfPkg IA32: add support for loading X64 Linux images To: Ard Biesheuvel , devel@edk2.groups.io Cc: leif@nuviainc.com, pjones@redhat.com, mjg59@google.com, agraf@csgraf.de, daniel.kiper@oracle.com, nivedita@alum.mit.edu References: <20200214114155.22859-1-ard.biesheuvel@arm.com> From: "Laszlo Ersek" Message-ID: <8f18a788-91bd-63c7-447a-c28fa3613e6d@redhat.com> Date: Fri, 14 Feb 2020 15:45:33 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20200214114155.22859-1-ard.biesheuvel@arm.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-MC-Unique: 9H1wR6eDNaa4kemll5U82A-1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit On 02/14/20 12:41, Ard Biesheuvel wrote: > This is the UEFI counterpart to my Linux series [0] 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 section 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 UEFI shell. (1) Please mention that this driver / protocol implementation ties in with the larger PE/COFF emulator feature that is based on e.g. your commit 57df17fe26cd ("MdeModulePkg/DxeCore: invoke the emulator protocol for foreign images", 2019-04-14). (2) I think we're now in the soft feature freeze, so I believe this patch will have to wait until the next dev cycle opens up. I hope that's OK with you. > > [0] https://lore.kernel.org/linux-arm-kernel/20200213145928.7047-1-ardb@kernel.org/ > > Cc: lersek@redhat.com > Cc: leif@nuviainc.com > Cc: pjones@redhat.com > Cc: mjg59@google.com > Cc: agraf@csgraf.de > Cc: daniel.kiper@oracle.com > Cc: nivedita@alum.mit.edu > Signed-off-by: Ard Biesheuvel > --- > OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.c | 151 ++++++++++++++++++++ > OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.inf | 36 +++++ > OvmfPkg/OvmfPkgIa32.dsc | 2 + > OvmfPkg/OvmfPkgIa32.fdf | 2 + > 4 files changed, 191 insertions(+) > > diff --git a/OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.c b/OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.c > new file mode 100644 > index 000000000000..a9c960c64be9 > --- /dev/null > +++ b/OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.c > @@ -0,0 +1,151 @@ > +/** @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 > + > +#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 > + ) > +{ > + if (ImageType != EFI_IMAGE_SUBSYSTEM_EFI_APPLICATION && > + ImageType != EFI_IMAGE_SUBSYSTEM_EFI_BOOT_SERVICE_DRIVER) { > + return FALSE; > + } > + return TRUE; > +} > + > +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)ImageBase + PeCoffHeaderOffset + > + sizeof(UINT32) + > + sizeof(EFI_IMAGE_FILE_HEADER) + > + 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); > + } > + } > + Section++; > + } > + return NULL; > +} > + > +STATIC > +EFI_STATUS > +EFIAPI > +Unsupported ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_SYSTEM_TABLE *SystemTable > + ) > +{ > + return EFI_UNSUPPORTED; > +} > + > +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 > + ) > +{ > + *EntryPoint = GetCompatEntryPoint (ImageBase) ?: &Unsupported; (3) Operator "?:" is a GNU-ism, please replace it. > + > + 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 3ffaa9b3388f..66522a4bc555 100644 > --- a/OvmfPkg/OvmfPkgIa32.dsc > +++ b/OvmfPkg/OvmfPkgIa32.dsc > @@ -916,3 +916,5 @@ [Components] > NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf > } > !endif > + > + OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.inf > diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf > index 586bbff08585..3411e4ca676e 100644 > --- a/OvmfPkg/OvmfPkgIa32.fdf > +++ b/OvmfPkg/OvmfPkgIa32.fdf > @@ -350,6 +350,8 @@ [FV.DXEFV] > !endif > !endif > > +INF OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.inf > + > ################################################################################ > > [FV.FVMAIN_COMPACT] > (4) Please introduce a new build flag (such as -D LOAD_X64_ON_IA32_ENABLE) for gating this feature. It's fine if the default value of the flag is TRUE, I'd just like a possibility to exclude this driver from the firmware without out-of-tree patches. (5) Can you please explain how EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL relates to Secure Boot and/or Trusted Boot? (5a) Is the ".compat" section included in the image hashing? (5b) Does the DXE core subject such non-native images to verification / measurement at all? (Sorry I didn't follow your original work on EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL.) Thanks! Laszlo