[edk2-devel] [RFC PATCH 05/11] MdeModulePkg/DxeCore: Use memory mapped FV protocol to avoid image copy
Ni, Ray
ray.ni at intel.com
Tue May 30 06:21:33 UTC 2023
GetFileBufferByFilePath() always returns a copy of file buffer even when the file
is in a memory-mapped device.
So, your patch adds a new implementation (abstracted through the new MM FV protocol) that can directly return the file location in the MMIO device.
Several comments:
1. I am not sure if any negative impact due to this change. For example: old logic reads the MMIO device but doesn't execute in the MMIO device. Does MMIO device always support execution in place?
2. If the MMFV protocol is only produced by DxeCore and consumed by DxeCore, can we just implement a local function instead? The challenge might be how to pass the FV_DEVICE instance to the local function. Can we "handle" the "FirmwareVolume2" protocol from the Handle first and use CR_FROM_THIS() macro to retrieve the FV_DEVICE pointer? This helps to not add the MMFV protocol, letting the change a pure DxeCore internal thing.
Thanks,
Ray
> -----Original Message-----
> From: Ard Biesheuvel <ardb at kernel.org>
> Sent: Monday, May 29, 2023 6:17 PM
> To: devel at edk2.groups.io
> Cc: Ard Biesheuvel <ardb at kernel.org>; Ni, Ray <ray.ni at intel.com>; Yao, Jiewen
> <jiewen.yao at intel.com>; Gerd Hoffmann <kraxel at redhat.com>; Taylor Beebe
> <t at taylorbeebe.com>; Oliver Smith-Denny <osd at smith-denny.com>; Bi, Dandan
> <dandan.bi at intel.com>; Gao, Liming <gaoliming at byosoft.com.cn>; Kinney,
> Michael D <michael.d.kinney at intel.com>; Leif Lindholm
> <quic_llindhol at quicinc.com>; Michael Kubacki <mikuback at linux.microsoft.com>
> Subject: [RFC PATCH 05/11] MdeModulePkg/DxeCore: Use memory mapped FV
> protocol to avoid image copy
>
> Use the memory mapped FV protocol to obtain the existing location in
> memory and the size of an image being loaded from a firmware volume.
> This removes the need to do a memcopy of the file data.
>
> Signed-off-by: Ard Biesheuvel <ardb at kernel.org>
> ---
> MdeModulePkg/Core/Dxe/DxeMain.h | 1 +
> MdeModulePkg/Core/Dxe/DxeMain.inf | 3 +
> MdeModulePkg/Core/Dxe/Image/Image.c | 111 +++++++++++++++++---
> MdeModulePkg/Include/Protocol/MemoryMappedFv.h | 59 +++++++++++
> MdeModulePkg/MdeModulePkg.dec | 3 +
> 5 files changed, 163 insertions(+), 14 deletions(-)
>
> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h
> b/MdeModulePkg/Core/Dxe/DxeMain.h
> index 43daa037be441150..a695b457c79b65bb 100644
> --- a/MdeModulePkg/Core/Dxe/DxeMain.h
> +++ b/MdeModulePkg/Core/Dxe/DxeMain.h
> @@ -45,6 +45,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #include <Protocol/HiiPackageList.h>
>
> #include <Protocol/SmmBase2.h>
>
> #include <Protocol/PeCoffImageEmulator.h>
>
> +#include <Protocol/MemoryMappedFv.h>
>
> #include <Guid/MemoryTypeInformation.h>
>
> #include <Guid/FirmwareFileSystem2.h>
>
> #include <Guid/FirmwareFileSystem3.h>
>
> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf
> b/MdeModulePkg/Core/Dxe/DxeMain.inf
> index 35d5bf0dee6f7f3f..a7175cb364b9b5de 100644
> --- a/MdeModulePkg/Core/Dxe/DxeMain.inf
> +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
> @@ -153,6 +153,9 @@ [Protocols]
> gEfiLoadedImageDevicePathProtocolGuid ## PRODUCES
>
> gEfiHiiPackageListProtocolGuid ## SOMETIMES_PRODUCES
>
> gEfiSmmBase2ProtocolGuid ## SOMETIMES_CONSUMES
>
> + ## PRODUCES
>
> + ## CONSUMES
>
> + gEdkiiMemoryMappedFvProtocolGuid
>
> gEdkiiPeCoffImageEmulatorProtocolGuid ## SOMETIMES_CONSUMES
>
>
>
> # Arch Protocols
>
> diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c
> b/MdeModulePkg/Core/Dxe/Image/Image.c
> index f30e369370a09609..3dfab4829b3ca17f 100644
> --- a/MdeModulePkg/Core/Dxe/Image/Image.c
> +++ b/MdeModulePkg/Core/Dxe/Image/Image.c
> @@ -1043,6 +1043,76 @@ CoreUnloadAndCloseImage (
> CoreFreePool (Image);
>
> }
>
>
>
> +/**
>
> + Get the image file data and size directly from a memory mapped FV
>
> +
>
> + If FilePath is NULL, then NULL is returned.
>
> + If FileSize is NULL, then NULL is returned.
>
> + If AuthenticationStatus is NULL, then NULL is returned.
>
> +
>
> + @param[in] FvHandle The firmware volume handle
>
> + @param[in] FilePath The pointer to the device path of the file
>
> + that is abstracted to the file buffer.
>
> + @param[out] FileSize The pointer to the size of the abstracted
>
> + file buffer.
>
> + @param[out] AuthenticationStatus Pointer to the authentication status.
>
> +
>
> + @retval NULL FilePath is NULL, or FileSize is NULL, or AuthenticationStatus
>
> + is NULL, or the file is not memory mapped
>
> + @retval other The abstracted file buffer.
>
> +**/
>
> +STATIC
>
> +VOID *
>
> +GetFileFromMemoryMappedFv (
>
> + IN EFI_HANDLE FvHandle,
>
> + IN CONST EFI_DEVICE_PATH_PROTOCOL *FilePath,
>
> + OUT UINTN *FileSize,
>
> + OUT UINT32 *AuthenticationStatus
>
> + )
>
> +{
>
> + EDKII_MEMORY_MAPPED_FV_PROTOCOL *MemMappedFv;
>
> + CONST EFI_GUID *NameGuid;
>
> + EFI_PHYSICAL_ADDRESS Address;
>
> + EFI_STATUS Status;
>
> +
>
> + if ((FilePath == NULL) ||
>
> + (FileSize == NULL) ||
>
> + (AuthenticationStatus == NULL))
>
> + {
>
> + return NULL;
>
> + }
>
> +
>
> + NameGuid = EfiGetNameGuidFromFwVolDevicePathNode (
>
> + (CONST MEDIA_FW_VOL_FILEPATH_DEVICE_PATH *)FilePath);
>
> + if (NameGuid == NULL) {
>
> + return NULL;
>
> + }
>
> +
>
> + Status = gBS->HandleProtocol (
>
> + FvHandle,
>
> + &gEdkiiMemoryMappedFvProtocolGuid,
>
> + (VOID **)&MemMappedFv
>
> + );
>
> + if (EFI_ERROR (Status)) {
>
> + ASSERT (Status == EFI_UNSUPPORTED);
>
> + return NULL;
>
> + }
>
> +
>
> + Status = MemMappedFv->GetLocationAndSize (
>
> + MemMappedFv,
>
> + NameGuid,
>
> + EFI_SECTION_PE32,
>
> + &Address,
>
> + FileSize,
>
> + AuthenticationStatus
>
> + );
>
> + if (EFI_ERROR (Status) || (Address > (MAX_ADDRESS - *FileSize))) {
>
> + return NULL;
>
> + }
>
> +
>
> + return (VOID *)(UINTN)Address;
>
> +}
>
> +
>
> /**
>
> Loads an EFI image into memory and returns a handle to the image.
>
>
>
> @@ -1164,6 +1234,16 @@ CoreLoadImageCommon (
> Status = CoreLocateDevicePath (&gEfiFirmwareVolume2ProtocolGuid,
> &HandleFilePath, &DeviceHandle);
>
> if (!EFI_ERROR (Status)) {
>
> ImageIsFromFv = TRUE;
>
> +
>
> + //
>
> + // If possible, use the memory mapped file image directly, rather than
> copying it into a buffer
>
> + //
>
> + FHand.Source = GetFileFromMemoryMappedFv (
>
> + DeviceHandle,
>
> + HandleFilePath,
>
> + &FHand.SourceSize,
>
> + &AuthenticationStatus
>
> + );
>
> } else {
>
> HandleFilePath = FilePath;
>
> Status = CoreLocateDevicePath (&gEfiSimpleFileSystemProtocolGuid,
> &HandleFilePath, &DeviceHandle);
>
> @@ -1187,21 +1267,24 @@ CoreLoadImageCommon (
> //
>
> // Get the source file buffer by its device path.
>
> //
>
> - FHand.Source = GetFileBufferByFilePath (
>
> - BootPolicy,
>
> - FilePath,
>
> - &FHand.SourceSize,
>
> - &AuthenticationStatus
>
> - );
>
> if (FHand.Source == NULL) {
>
> - Status = EFI_NOT_FOUND;
>
> - } else {
>
> - FHand.FreeBuffer = TRUE;
>
> - if (ImageIsFromLoadFile) {
>
> - //
>
> - // LoadFile () may cause the device path of the Handle be updated.
>
> - //
>
> - OriginalFilePath = AppendDevicePath (DevicePathFromHandle
> (DeviceHandle), Node);
>
> + FHand.Source = GetFileBufferByFilePath (
>
> + BootPolicy,
>
> + FilePath,
>
> + &FHand.SourceSize,
>
> + &AuthenticationStatus
>
> + );
>
> +
>
> + if (FHand.Source == NULL) {
>
> + Status = EFI_NOT_FOUND;
>
> + } else {
>
> + FHand.FreeBuffer = TRUE;
>
> + if (ImageIsFromLoadFile) {
>
> + //
>
> + // LoadFile () may cause the device path of the Handle be updated.
>
> + //
>
> + OriginalFilePath = AppendDevicePath (DevicePathFromHandle
> (DeviceHandle), Node);
>
> + }
>
> }
>
> }
>
> }
>
> diff --git a/MdeModulePkg/Include/Protocol/MemoryMappedFv.h
> b/MdeModulePkg/Include/Protocol/MemoryMappedFv.h
> new file mode 100644
> index 0000000000000000..821009122113a658
> --- /dev/null
> +++ b/MdeModulePkg/Include/Protocol/MemoryMappedFv.h
> @@ -0,0 +1,59 @@
> +/** @file
>
> + Protocol to obtain information about files in memory mapped firmware
> volumes
>
> +
>
> + Copyright (c) 2023, Google LLC. All rights reserved.<BR>
>
> +
>
> + SPDX-License-Identifier: BSD-2-Clause-Patent
>
> +
>
> +**/
>
> +
>
> +#ifndef EDKII_MEMORY_MAPPED_FV_H_
>
> +#define EDKII_MEMORY_MAPPED_FV_H_
>
> +
>
> +#define EDKII_MEMORY_MAPPED_FV_PROTOCOL_GUID \
>
> + { 0xb9bfa973, 0x5384, 0x441e, { 0xa4, 0xe7, 0x20, 0xe6, 0x5d, 0xaf, 0x2e,
> 0x0f } }
>
> +
>
> +typedef struct _EDKII_MEMORY_MAPPED_FV_PROTOCOL
> EDKII_MEMORY_MAPPED_FV_PROTOCOL;
>
> +
>
> +//
>
> +// Function Prototypes
>
> +//
>
> +
>
> +/**
>
> + Get the physical address and size of a file's section in a memory mapped FV
>
> +
>
> + @param[in] This The protocol pointer
>
> + @param[in] NameGuid The name GUID of the file
>
> + @param[in] SectionType The file section from which to retrieve address and
> size
>
> + @param[out] FileAddress The physical address of the file
>
> + @param[out] FileSize The size of the file
>
> + @param[out] AuthStatus The authentication status associated with the file
>
> +
>
> + @retval EFI_SUCCESS Information about the file was retrieved
> successfully.
>
> + @retval EFI_INVALID_PARAMETER FileAddress was NULL, FileSize was NULL,
> AuthStatus
>
> + was NULL.
>
> + @retval EFI_NOT_FOUND No section of the specified type could be
> located in
>
> + the specified file.
>
> +
>
> +**/
>
> +typedef
>
> +EFI_STATUS
>
> +(EFIAPI *GET_LOCATION_AND_SIZE)(
>
> + IN EDKII_MEMORY_MAPPED_FV_PROTOCOL *This,
>
> + IN CONST EFI_GUID *NameGuid,
>
> + IN EFI_SECTION_TYPE SectionType,
>
> + OUT EFI_PHYSICAL_ADDRESS *FileAddress,
>
> + OUT UINTN *FileSize,
>
> + OUT UINT32 *AuthStatus
>
> + );
>
> +
>
> +//
>
> +// Protocol interface structure
>
> +//
>
> +struct _EDKII_MEMORY_MAPPED_FV_PROTOCOL {
>
> + GET_LOCATION_AND_SIZE GetLocationAndSize;
>
> +};
>
> +
>
> +extern EFI_GUID gEdkiiMemoryMappedFvProtocolGuid;
>
> +
>
> +#endif
>
> diff --git a/MdeModulePkg/MdeModulePkg.dec
> b/MdeModulePkg/MdeModulePkg.dec
> index d65dae18aa81e569..2d72ac733d82195e 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -679,6 +679,9 @@ [Protocols]
> ## Include/Protocol/PlatformBootManager.h
>
> gEdkiiPlatformBootManagerProtocolGuid = { 0xaa17add4, 0x756c, 0x460d,
> { 0x94, 0xb8, 0x43, 0x88, 0xd7, 0xfb, 0x3e, 0x59 } }
>
>
>
> + ## Include/Protocol/MemoryMappedFv.h
>
> + gEdkiiMemoryMappedFvProtocolGuid = { 0xb9bfa973, 0x5384, 0x441e, { 0xa4,
> 0xe7, 0x20, 0xe6, 0x5d, 0xaf, 0x2e, 0x0f } }
>
> +
>
> #
>
> # [Error.gEfiMdeModulePkgTokenSpaceGuid]
>
> # 0x80000001 | Invalid value provided.
>
> --
> 2.39.2
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105409): https://edk2.groups.io/g/devel/message/105409
Mute This Topic: https://groups.io/mt/99197138/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3943202/1813853/130120423/xyzzy [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list