From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by mx.groups.io with SMTP id smtpd.web11.4831.1685433494986662236 for ; Tue, 30 May 2023 00:58:15 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=YLFDwULG; spf=pass (domain: kernel.org, ip: 139.178.84.217, mailfrom: ardb@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 4B9C562545 for ; Tue, 30 May 2023 07:58:14 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id B524DC433A4 for ; Tue, 30 May 2023 07:58:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1685433493; bh=XvIUjiReoEfspXxAKh7z44/2Sv7BpNln4ucXL/VwxFk=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=YLFDwULGT8S+5Mf8WJOpkkjnG6i3QKYE8H2whjKc1CS9YuHh1Cx4Z3LUJBSw6SO8y bUooxeFeNnep83HfV/zDC326dmnaAEICQFit87PTsYAYB0qMhJYDqF1tHod60bUo/h gXSTTC0RcoT12pWq2jeNnO2H2dA+2l5Hhb7KPQWrfHf94r8LJ35jLY9xw21YmgfabO q489aR0hilEgv+JWrP/TbVDsPizpt9wMccggvo8WIi8TyDbL3yKZc4w2PA6chirij8 QCBFIrG0sQqGcQJkiNDYnuWcE9mVAHvVbJWet/7yJ8Kdyv04tiFWhiAiSH1RoP2Mud OZ2xu1kpnsByQ== Received: by mail-lj1-f176.google.com with SMTP id 38308e7fff4ca-2b03d3e41fcso59625681fa.0 for ; Tue, 30 May 2023 00:58:13 -0700 (PDT) X-Gm-Message-State: AC+VfDyINNcM3IMiEC279WTcBt9ab6aGl+TPwv1PwXUDYKx/WZv/Emg6 ai7NYEkvz9rFEUQZRRet84aqevusaHHmaX48hbE= X-Google-Smtp-Source: ACHHUZ7s9r/KJA4JZ49aFS62qJkQjTjcx7KO5WP0PauoxG9V7unOTOY0awWo+qiex9lrHGKtpDNG4LOU7j7/+CX47NU= X-Received: by 2002:a2e:9212:0:b0:2a8:baea:2554 with SMTP id k18-20020a2e9212000000b002a8baea2554mr464410ljg.3.1685433491721; Tue, 30 May 2023 00:58:11 -0700 (PDT) MIME-Version: 1.0 References: <20230529101705.2476949-1-ardb@kernel.org> <20230529101705.2476949-9-ardb@kernel.org> In-Reply-To: From: "Ard Biesheuvel" Date: Tue, 30 May 2023 09:58:00 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [RFC PATCH 08/11] MdeModulePkg/DxeIpl: Relocate and remap XIP capable DXE drivers To: devel@edk2.groups.io, ray.ni@intel.com Cc: "Yao, Jiewen" , Gerd Hoffmann , Taylor Beebe , Oliver Smith-Denny , "Bi, Dandan" , "Gao, Liming" , "Kinney, Michael D" , Leif Lindholm , Michael Kubacki Content-Type: text/plain; charset="UTF-8" On Tue, 30 May 2023 at 08:45, Ni, Ray wrote: > > 1. is it possible that a PE image sits in the right location but the SectionAlignment is larger than FileAlignment so each section still needs to be copied to the aligned memory location? > That is a good question. Currently, the ELF toolchains rely on GenFw to construct the PE images in a way that permits execution in place, but I have no idea how this works with native PE/COFF toolchains. > 2. PeCoffLoaderRelocateImage() might not be called for XIP > Are you saying it is not permitted? Or that it may not happen? In any case, relocating the image in place is exactly what the BaseTools do for XIP PEIMs, so I think applying the same logic here is reasonable. > > -----Original Message----- > > From: devel@edk2.groups.io On Behalf Of Ard > > Biesheuvel > > Sent: Monday, May 29, 2023 6:17 PM > > To: devel@edk2.groups.io > > Cc: Ard Biesheuvel ; Ni, Ray ; Yao, Jiewen > > ; Gerd Hoffmann ; Taylor Beebe > > ; Oliver Smith-Denny ; Bi, Dandan > > ; Gao, Liming ; Kinney, > > Michael D ; Leif Lindholm > > ; Michael Kubacki > > Subject: [edk2-devel] [RFC PATCH 08/11] MdeModulePkg/DxeIpl: Relocate and > > remap XIP capable DXE drivers > > > > Before handing over to the DXE core, iterate over all known FFS files > > and find the ones that can execute in place. These files are then > > relocated in place and mapped with restricted permissions, allowing the > > DXE core to dispatch them without the need to perform any manipulation > > of the file contents or the page table permissions. > > > > Signed-off-by: Ard Biesheuvel > > --- > > MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 1 + > > MdeModulePkg/Core/DxeIplPeim/DxeLoad.c | 196 ++++++++++++++++++++ > > 2 files changed, 197 insertions(+) > > > > diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf > > b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf > > index f1990eac77607854..60112100df78b396 100644 > > --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf > > +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf > > @@ -65,6 +65,7 @@ [LibraryClasses] > > PeimEntryPoint > > > > DebugLib > > > > DebugAgentLib > > > > + PeCoffLib > > > > PeiServicesTablePointerLib > > > > PerformanceLib > > > > > > > > diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c > > b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c > > index 2c19f1a507baf34a..1f20db1faffbd1d2 100644 > > --- a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c > > +++ b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c > > @@ -10,6 +10,9 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > > > > > > #include "DxeIpl.h" > > > > > > > > +#include > > > > +#include > > > > + > > > > // > > > > // Module Globals used in the DXE to PEI hand off > > > > // These must be module globals, so the stack can be switched > > > > @@ -228,6 +231,197 @@ ValidateMemoryTypeInfoVariable ( > > return TRUE; > > > > } > > > > > > > > +/** > > > > + Support routine for the PE/COFF Loader that reads a buffer from a PE/COFF file. > > > > + The function is used for XIP code to have optimized memory copy. > > > > + > > > > + @param FileHandle The handle to the PE/COFF file > > > > + @param FileOffset The offset, in bytes, into the file to read > > > > + @param ReadSize The number of bytes to read from the file starting at > > FileOffset > > > > + @param Buffer A pointer to the buffer to read the data into. > > > > + > > > > + @return EFI_SUCCESS ReadSize bytes of data were read into Buffer from the > > > > + PE/COFF file starting at FileOffset > > > > + > > > > +**/ > > > > +STATIC > > > > +EFI_STATUS > > > > +EFIAPI > > > > +PeiImageRead ( > > > > + IN VOID *FileHandle, > > > > + IN UINTN FileOffset, > > > > + IN UINTN *ReadSize, > > > > + OUT VOID *Buffer > > > > + ) > > > > +{ > > > > + CHAR8 *Destination8; > > > > + CHAR8 *Source8; > > > > + > > > > + Destination8 = Buffer; > > > > + Source8 = (CHAR8 *)((UINTN)FileHandle + FileOffset); > > > > + if (Destination8 != Source8) { > > > > + CopyMem (Destination8, Source8, *ReadSize); > > > > + } > > > > + > > > > + return EFI_SUCCESS; > > > > +} > > > > + > > > > +STATIC > > > > +VOID > > > > +RemapImage ( > > > > + IN EDKII_MEMORY_ATTRIBUTE_PPI *MemoryPpi, > > > > + IN CONST PE_COFF_LOADER_IMAGE_CONTEXT *ImageContext > > > > + ) > > > > +{ > > > > + EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION Hdr; > > > > + EFI_IMAGE_SECTION_HEADER *Section; > > > > + PHYSICAL_ADDRESS SectionAddress; > > > > + EFI_STATUS Status; > > > > + UINT64 Permissions; > > > > + UINTN Index; > > > > + > > > > + if (MemoryPpi == NULL) { > > > > + return; > > > > + } > > > > + > > > > + Hdr.Union = (EFI_IMAGE_OPTIONAL_HEADER_UNION *)((UINT8 > > *)ImageContext->Handle + > > > > + ImageContext->PeCoffHeaderOffset); > > > > + ASSERT (Hdr.Pe32->Signature == EFI_IMAGE_NT_SIGNATURE); > > > > + > > > > + Section = (EFI_IMAGE_SECTION_HEADER *)((UINT8 *)Hdr.Union + sizeof > > (UINT32) + > > > > + sizeof (EFI_IMAGE_FILE_HEADER) + > > > > + Hdr.Pe32->FileHeader.SizeOfOptionalHeader > > > > + ); > > > > + > > > > + for (Index = 0; Index < Hdr.Pe32->FileHeader.NumberOfSections; Index++) { > > > > + SectionAddress = ImageContext->ImageAddress + > > Section[Index].VirtualAddress; > > > > + Permissions = 0; > > > > + > > > > + if ((Section[Index].Characteristics & EFI_IMAGE_SCN_MEM_WRITE) == 0) { > > > > + Permissions |= EFI_MEMORY_RO; > > > > + } > > > > + > > > > + if ((Section[Index].Characteristics & EFI_IMAGE_SCN_MEM_EXECUTE) == 0) { > > > > + Permissions |= EFI_MEMORY_XP; > > > > + } > > > > + > > > > + Status = MemoryPpi->SetPermissions ( > > > > + MemoryPpi, > > > > + SectionAddress, > > > > + Section[Index].Misc.VirtualSize, > > > > + Permissions, > > > > + Permissions ^ EFI_MEMORY_RO ^ EFI_MEMORY_XP > > > > + ); > > > > + ASSERT_EFI_ERROR (Status); > > > > + } > > > > +} > > > > + > > > > +STATIC > > > > +VOID > > > > +RelocateAndRemapDriversInPlace ( > > > > + VOID > > > > + ) > > > > +{ > > > > + EFI_STATUS Status; > > > > + UINTN Instance; > > > > + EFI_PEI_FV_HANDLE VolumeHandle; > > > > + EFI_PEI_FILE_HANDLE FileHandle; > > > > + PE_COFF_LOADER_IMAGE_CONTEXT ImageContext; > > > > + UINT32 AuthenticationState; > > > > + EDKII_MEMORY_ATTRIBUTE_PPI *MemoryPpi; > > > > + > > > > + MemoryPpi = NULL; > > > > + PeiServicesLocatePpi (&gEdkiiMemoryAttributePpiGuid, 0, NULL, (VOID > > **)&MemoryPpi); > > > > + > > > > + Instance = 0; > > > > + do { > > > > + // > > > > + // Traverse all firmware volume instances > > > > + // > > > > + Status = PeiServicesFfsFindNextVolume (Instance, &VolumeHandle); > > > > + if (Status == EFI_NOT_FOUND) { > > > > + return; > > > > + } > > > > + > > > > + ASSERT_EFI_ERROR (Status); > > > > + > > > > + FileHandle = NULL; > > > > + do { > > > > + Status = PeiServicesFfsFindNextFile ( > > > > + EFI_FV_FILETYPE_DRIVER, > > > > + VolumeHandle, > > > > + &FileHandle); > > > > + if (Status == EFI_NOT_FOUND) { > > > > + break; > > > > + } > > > > + > > > > + ASSERT_EFI_ERROR (Status); > > > > + > > > > + ZeroMem (&ImageContext, sizeof (ImageContext)); > > > > + > > > > + Status = PeiServicesFfsFindSectionData3 ( > > > > + EFI_SECTION_PE32, > > > > + 0, > > > > + FileHandle, > > > > + &ImageContext.Handle, > > > > + &AuthenticationState > > > > + ); > > > > + if (Status == EFI_NOT_FOUND) { > > > > + continue; > > > > + } > > > > + > > > > + ASSERT_EFI_ERROR (Status); > > > > + > > > > + ImageContext.ImageRead = PeiImageRead; > > > > + Status = PeCoffLoaderGetImageInfo (&ImageContext); > > > > + if (EFI_ERROR (Status)) { > > > > + ASSERT_EFI_ERROR (Status); > > > > + continue; > > > > + } > > > > + > > > > + ImageContext.ImageAddress = > > (PHYSICAL_ADDRESS)(UINTN)ImageContext.Handle; > > > > + if ((ImageContext.ImageAddress & (ImageContext.SectionAlignment - 1)) != > > 0) { > > > > + DEBUG ((DEBUG_VERBOSE, "%a: skip PE image at %p\n", __func__, > > ImageContext.Handle)); > > > > + continue; > > > > + } > > > > + > > > > + DEBUG (( > > > > + DEBUG_INFO, > > > > + "%a: relocate PE image at %p for execution in place\n", > > > > + __func__, > > > > + ImageContext.Handle > > > > + )); > > > > + > > > > + // > > > > + // 'Load' the image in-place - this just performs a sanity check on > > > > + // the PE metadata but does not actually move any data > > > > + // > > > > + Status = PeCoffLoaderLoadImage (&ImageContext); > > > > + if (EFI_ERROR (Status)) { > > > > + ASSERT_EFI_ERROR (Status); > > > > + continue; > > > > + } > > > > + > > > > + // > > > > + // Relocate this driver in place > > > > + // > > > > + Status = PeCoffLoaderRelocateImage (&ImageContext); > > > > + if (EFI_ERROR (Status)) { > > > > + ASSERT_EFI_ERROR (Status); > > > > + continue; > > > > + } > > > > + > > > > + // > > > > + // Apply section permissions to the page tables > > > > + // > > > > + RemapImage (MemoryPpi, &ImageContext); > > > > + > > > > + } while (TRUE); > > > > + > > > > + Instance++; > > > > + } while (TRUE); > > > > +} > > > > + > > > > /** > > > > Main entry point to last PEIM. > > > > > > > > @@ -436,6 +630,8 @@ DxeLoadCore ( > > DxeCoreEntryPoint > > > > ); > > > > > > > > + RelocateAndRemapDriversInPlace (); > > > > + > > > > // > > > > // Report Status Code EFI_SW_PEI_PC_HANDOFF_TO_NEXT > > > > // > > > > -- > > 2.39.2 > > > > > > > > -=-=-=-=-=-= > > Groups.io Links: You receive all messages sent to this group. > > View/Reply Online (#105370): https://edk2.groups.io/g/devel/message/105370 > > Mute This Topic: https://groups.io/mt/99197142/1712937 > > Group Owner: devel+owner@edk2.groups.io > > Unsubscribe: https://edk2.groups.io/g/devel/unsub [ray.ni@intel.com] > > -=-=-=-=-=-= > > > > > > > >