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@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 (#105367): https://edk2.groups.io/g/devel/message/105367
Mute This Topic: https://groups.io/mt/99197138/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-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@kernel.org>
> Sent: Monday, May 29, 2023 6:17 PM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel <ardb@kernel.org>; Ni, Ray <ray.ni@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Taylor Beebe
> <t@taylorbeebe.com>; Oliver Smith-Denny <osd@smith-denny.com>; Bi, Dandan
> <dandan.bi@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Kinney,
> Michael D <michael.d.kinney@intel.com>; Leif Lindholm
> <quic_llindhol@quicinc.com>; Michael Kubacki <mikuback@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@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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On Tue, 30 May 2023 at 08:21, Ni, Ray <ray.ni@intel.com> wrote:
>
> 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?
At this point, we are not executing anything in place. The buffer is
only used by the PE/COFF loader to access the file contents, but it
still creates the sections in memory as before, and copies the data
into them.
This is similar to how gBS->Loadimage() with a buffer/size only uses
the buffer contents to access the file data, it does not execute the
image from the buffer.
> 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.
>
The loader does not know whether the FirmwareVolume2 protocol was
produced by DXE core or by some other component, so we cannot assume
that CR_FROM_THIS() is usable.
> > -----Original Message-----
> > From: Ard Biesheuvel <ardb@kernel.org>
> > Sent: Monday, May 29, 2023 6:17 PM
> > To: devel@edk2.groups.io
> > Cc: Ard Biesheuvel <ardb@kernel.org>; Ni, Ray <ray.ni@intel.com>; Yao, Jiewen
> > <jiewen.yao@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Taylor Beebe
> > <t@taylorbeebe.com>; Oliver Smith-Denny <osd@smith-denny.com>; Bi, Dandan
> > <dandan.bi@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Kinney,
> > Michael D <michael.d.kinney@intel.com>; Leif Lindholm
> > <quic_llindhol@quicinc.com>; Michael Kubacki <mikuback@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@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 (#105427): https://edk2.groups.io/g/devel/message/105427
Mute This Topic: https://groups.io/mt/99197138/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
> -----Original Message-----
> From: Ard Biesheuvel <ardb@kernel.org>
> Sent: Tuesday, May 30, 2023 3:52 PM
> To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Gerd Hoffmann <kraxel@redhat.com>;
> Taylor Beebe <t@taylorbeebe.com>; Oliver Smith-Denny <osd@smith-
> denny.com>; Bi, Dandan <dandan.bi@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Kinney, Michael D <michael.d.kinney@intel.com>;
> Leif Lindholm <quic_llindhol@quicinc.com>; Michael Kubacki
> <mikuback@linux.microsoft.com>
> Subject: Re: [edk2-devel] [RFC PATCH 05/11] MdeModulePkg/DxeCore: Use
> memory mapped FV protocol to avoid image copy
>
> On Tue, 30 May 2023 at 08:21, Ni, Ray <ray.ni@intel.com> wrote:
> >
> > 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?
>
> At this point, we are not executing anything in place. The buffer is
> only used by the PE/COFF loader to access the file contents, but it
> still creates the sections in memory as before, and copies the data
> into them.
>
> This is similar to how gBS->Loadimage() with a buffer/size only uses
> the buffer contents to access the file data, it does not execute the
> image from the buffer.
OK.
>
> > 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.
> >
>
> The loader does not know whether the FirmwareVolume2 protocol was
> produced by DXE core or by some other component, so we cannot assume
> that CR_FROM_THIS() is usable.
I see. How about:
a. Define a GUID in DxeCore module internal
b. Install that GUID in the FV handle (the accordingly instance of that GUID can be simply NULL)
as a signature to tell the FV is produced by DxeCore
c. Implement a local function that return location inside the FV when the FvHandle has the
private GUID installed.
>
>
>
> > > -----Original Message-----
> > > From: Ard Biesheuvel <ardb@kernel.org>
> > > Sent: Monday, May 29, 2023 6:17 PM
> > > To: devel@edk2.groups.io
> > > Cc: Ard Biesheuvel <ardb@kernel.org>; Ni, Ray <ray.ni@intel.com>; Yao,
> Jiewen
> > > <jiewen.yao@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Taylor Beebe
> > > <t@taylorbeebe.com>; Oliver Smith-Denny <osd@smith-denny.com>; Bi,
> Dandan
> > > <dandan.bi@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Kinney,
> > > Michael D <michael.d.kinney@intel.com>; Leif Lindholm
> > > <quic_llindhol@quicinc.com>; Michael Kubacki
> <mikuback@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@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 (#105435): https://edk2.groups.io/g/devel/message/105435
Mute This Topic: https://groups.io/mt/99197138/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On Tue, 30 May 2023 at 10:41, Ni, Ray <ray.ni@intel.com> wrote: > > > > > -----Original Message----- > > From: Ard Biesheuvel <ardb@kernel.org> > > Sent: Tuesday, May 30, 2023 3:52 PM > > To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com> > > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; > > Taylor Beebe <t@taylorbeebe.com>; Oliver Smith-Denny <osd@smith- > > denny.com>; Bi, Dandan <dandan.bi@intel.com>; Gao, Liming > > <gaoliming@byosoft.com.cn>; Kinney, Michael D <michael.d.kinney@intel.com>; > > Leif Lindholm <quic_llindhol@quicinc.com>; Michael Kubacki > > <mikuback@linux.microsoft.com> > > Subject: Re: [edk2-devel] [RFC PATCH 05/11] MdeModulePkg/DxeCore: Use > > memory mapped FV protocol to avoid image copy > > > > On Tue, 30 May 2023 at 08:21, Ni, Ray <ray.ni@intel.com> wrote: > > > > > > 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? > > > > At this point, we are not executing anything in place. The buffer is > > only used by the PE/COFF loader to access the file contents, but it > > still creates the sections in memory as before, and copies the data > > into them. > > > > This is similar to how gBS->Loadimage() with a buffer/size only uses > > the buffer contents to access the file data, it does not execute the > > image from the buffer. > > OK. > > > > > > 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. > > > > > > > The loader does not know whether the FirmwareVolume2 protocol was > > produced by DXE core or by some other component, so we cannot assume > > that CR_FROM_THIS() is usable. > > I see. How about: > a. Define a GUID in DxeCore module internal > b. Install that GUID in the FV handle (the accordingly instance of that GUID can be simply NULL) > as a signature to tell the FV is produced by DxeCore > c. Implement a local function that return location inside the FV when the FvHandle has the > private GUID installed. > How is this any different from implementing a protocol? Installing a GUID on the handle and associating it with some code is exactly what this code does, but in the 'official' way. I'm not sure what we will gain by doing the same thing using local routines, other than making the code more difficult to follow. At least the protocol has a clearly defined interface, whereas the private GUID has no intrinsic meaning associated with it. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#105436): https://edk2.groups.io/g/devel/message/105436 Mute This Topic: https://groups.io/mt/99197138/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2026 Red Hat, Inc.