From 15624edf05f2819e5b34d1e8c4d131f4c85a3d18 Mon Sep 17 00:00:00 2001 From: Patrick Rudolph Date: Tue, 4 Jun 2024 11:45:16 +0200 Subject: [PATCH 1/4] GenFv: Add EFI_FV_FILETYPE_MM_STANDALONE EFI_FV_FILETYPE_MM_STANDALONE contains PE32 images, so add it to the list to make sure the files are also rebased. Cc: Bob Feng Cc: Liming Gao Cc: Rebecca Cran Cc: Yuwei Chen Signed-off-by: Patrick Rudolph --- BaseTools/Source/C/GenFv/GenFvInternalLib.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/BaseTools/Source/C/GenFv/GenFvInternalLib.c b/BaseTools/Source/C/GenFv/GenFvInternalLib.c index 29c3363a504d..0190270dbab8 100644 --- a/BaseTools/Source/C/GenFv/GenFvInternalLib.c +++ b/BaseTools/Source/C/GenFv/GenFvInternalLib.c @@ -3537,6 +3537,7 @@ Routine Description: case EFI_FV_FILETYPE_COMBINED_PEIM_DRIVER: case EFI_FV_FILETYPE_DRIVER: case EFI_FV_FILETYPE_DXE_CORE: + case EFI_FV_FILETYPE_MM_STANDALONE: break; case EFI_FV_FILETYPE_FIRMWARE_VOLUME_IMAGE: // @@ -3698,6 +3699,7 @@ Routine Description: case EFI_FV_FILETYPE_DRIVER: case EFI_FV_FILETYPE_DXE_CORE: + case EFI_FV_FILETYPE_MM_STANDALONE: // // Check if section-alignment and file-alignment match or not // From 1d23ede56bd02e5c13a783d2979b2a446dc6e279 Mon Sep 17 00:00:00 2001 From: Patrick Rudolph Date: Tue, 4 Jun 2024 12:04:22 +0200 Subject: [PATCH 2/4] StandaloneMmPkg: Support XIP PE32 in StandaloneMmPkg can be loaded into memory can be executed in place (XiP). That saves memory as they don't need to be relocated to memory when already residing in memory. Cc: Ard Biesheuvel Cc: Jiaxin Wu Cc: Ray Ni Cc: Sami Mujawar Signed-off-by: Patrick Rudolph --- StandaloneMmPkg/Core/Dispatcher.c | 105 +++++++++++++++++++----------- 1 file changed, 67 insertions(+), 38 deletions(-) diff --git a/StandaloneMmPkg/Core/Dispatcher.c b/StandaloneMmPkg/Core/Dispatcher.c index 01a2b3af0dcb..cb3845a7d6ba 100644 --- a/StandaloneMmPkg/Core/Dispatcher.c +++ b/StandaloneMmPkg/Core/Dispatcher.c @@ -134,49 +134,75 @@ MmLoadImage ( return Status; } - PageCount = (UINTN)EFI_SIZE_TO_PAGES ((UINTN)ImageContext.ImageSize + ImageContext.SectionAlignment); - DstBuffer = (UINTN)(-1); - - Status = MmAllocatePages ( - AllocateMaxAddress, - EfiRuntimeServicesCode, - PageCount, - &DstBuffer - ); - if (EFI_ERROR (Status)) { - return Status; - } - - ImageContext.ImageAddress = (EFI_PHYSICAL_ADDRESS)DstBuffer; + PageCount = EFI_SIZE_TO_PAGES ((UINTN)ImageContext.ImageSize + ImageContext.SectionAlignment); // - // Align buffer on section boundary + // XIP image that ImageAddress is same to Image handle. // - ImageContext.ImageAddress += ImageContext.SectionAlignment - 1; - ImageContext.ImageAddress &= ~((EFI_PHYSICAL_ADDRESS)(ImageContext.SectionAlignment - 1)); + if (ImageContext.ImageAddress == (EFI_PHYSICAL_ADDRESS)(UINTN)DriverEntry->Pe32Data) { + DstBuffer = (UINTN)ImageContext.ImageAddress; - // - // Load the image to our new buffer - // - Status = PeCoffLoaderLoadImage (&ImageContext); - if (EFI_ERROR (Status)) { - MmFreePages (DstBuffer, PageCount); - return Status; - } + // + // Load the image to our new buffer + // + Status = PeCoffLoaderLoadImage (&ImageContext); + if (EFI_ERROR (Status)) { + return Status; + } - // - // Relocate the image in our new buffer - // - Status = PeCoffLoaderRelocateImage (&ImageContext); - if (EFI_ERROR (Status)) { - MmFreePages (DstBuffer, PageCount); - return Status; - } + // + // Relocate the image in our new buffer + // + Status = PeCoffLoaderRelocateImage (&ImageContext); + if (EFI_ERROR (Status)) { + return Status; + } + } else { + DstBuffer = (UINTN)(-1); + + Status = MmAllocatePages ( + AllocateMaxAddress, + EfiRuntimeServicesCode, + PageCount, + &DstBuffer + ); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_INFO, "MmLoadImage failed to allocate memory\n")); - // - // Flush the instruction cache so the image data are written before we execute it - // - InvalidateInstructionCacheRange ((VOID *)(UINTN)ImageContext.ImageAddress, (UINTN)ImageContext.ImageSize); + return Status; + } + + ImageContext.ImageAddress = (EFI_PHYSICAL_ADDRESS)DstBuffer; + + // + // Align buffer on section boundary + // + ImageContext.ImageAddress += ImageContext.SectionAlignment - 1; + ImageContext.ImageAddress &= ~((EFI_PHYSICAL_ADDRESS)(ImageContext.SectionAlignment - 1)); + + // + // Load the image to our new buffer + // + Status = PeCoffLoaderLoadImage (&ImageContext); + if (EFI_ERROR (Status)) { + MmFreePages (DstBuffer, PageCount); + return Status; + } + + // + // Relocate the image in our new buffer + // + Status = PeCoffLoaderRelocateImage (&ImageContext); + if (EFI_ERROR (Status)) { + MmFreePages (DstBuffer, PageCount); + return Status; + } + + // + // Flush the instruction cache so the image data is written before we execute it + // + InvalidateInstructionCacheRange ((VOID *)(UINTN)ImageContext.ImageAddress, (UINTN)ImageContext.ImageSize); + } // // Save Image EntryPoint in DriverEntry @@ -192,7 +218,10 @@ MmLoadImage ( (VOID **)&DriverEntry->LoadedImage ); if (EFI_ERROR (Status)) { - MmFreePages (DstBuffer, PageCount); + if (ImageContext.ImageAddress != (EFI_PHYSICAL_ADDRESS)(UINTN)DriverEntry->Pe32Data) { + MmFreePages (DstBuffer, PageCount); + } + return Status; } From 900b69ea82762a4c422a86f1ade585e9e332daa9 Mon Sep 17 00:00:00 2001 From: Patrick Rudolph Date: Wed, 12 Jun 2024 09:50:31 +0200 Subject: [PATCH 3/4] StandaloneMmPkg/Core: Allocate runtime memory on heap Currently the StandaloneMm loader needs to find a free memory range in the secure world and provide it to StandaloneMM to use it as free pool for runtime memory allocations. The caller might not now how much memory space to allocate. In addition allocating a separate memory region in the trusted MM memory space increases the complexity on the MM loader side. Add a PCD that allows to use a fixed size memory buffer on the heap as free memory pool. Using this option will increase the binary size, but it removes the callers need to provide a memory range that can be used as runtime memory. Cc: Ard Biesheuvel Cc: Jiaxin Wu Cc: Ray Ni Cc: Sami Mujawar Signed-off-by: Patrick Rudolph --- StandaloneMmPkg/Core/Pool.c | 23 ++++++++ StandaloneMmPkg/Core/StandaloneMmCore.inf | 2 + .../Drivers/StandaloneMmCpu/EventHandle.c | 54 ++++++++++++------- .../StandaloneMmCpu/StandaloneMmCpu.inf | 3 ++ StandaloneMmPkg/StandaloneMmPkg.dec | 9 ++++ 5 files changed, 71 insertions(+), 20 deletions(-) diff --git a/StandaloneMmPkg/Core/Pool.c b/StandaloneMmPkg/Core/Pool.c index 282caa9869ea..4b0ed5270975 100644 --- a/StandaloneMmPkg/Core/Pool.c +++ b/StandaloneMmPkg/Core/Pool.c @@ -9,6 +9,20 @@ #include "StandaloneMmCore.h" +#if defined (__GNUC__) +#define ALIGN_EFI_PAGE __attribute__((aligned(EFI_PAGE_SIZE))) +#elif defined (_MSC_VER) +#define ALIGN_EFI_PAGE __declspec(align(EFI_PAGE_SIZE)) +#elif defined (__SUNPRO_C) +#define ALIGN_EFI_PAGE + #pragma align EFI_PAGE_SIZE(one,two80) +#else +/* not fatal, might waste a bit of memory */ +#define ALIGN_EFI_PAGE +#endif + +ALIGN_EFI_PAGE STATIC UINT8 FreePoolOnHeap[FixedPcdGet32 (PcdRuntimeMemoryOnHeapSize)]; + LIST_ENTRY mMmPoolLists[MAX_POOL_INDEX]; // // To cache the MMRAM base since when Loading modules At fixed address feature is enabled, @@ -63,6 +77,15 @@ MmInitializeMemoryServices ( MmramRanges[Index].RegionState ); } + + if (FixedPcdGetBool (PcdRuntimeMemoryOnHeap)) { + MmAddMemoryRegion ( + (UINTN)FreePoolOnHeap, + sizeof (FreePoolOnHeap), + EfiConventionalMemory, + EFI_CACHEABLE + ); + } } /** diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.inf b/StandaloneMmPkg/Core/StandaloneMmCore.inf index 02ecd68f37e2..3d6d72572cfc 100644 --- a/StandaloneMmPkg/Core/StandaloneMmCore.inf +++ b/StandaloneMmPkg/Core/StandaloneMmCore.inf @@ -78,6 +78,8 @@ [Pcd] gStandaloneMmPkgTokenSpaceGuid.PcdFwVolMmMaxEncapsulationDepth ##CONSUMES + gStandaloneMmPkgTokenSpaceGuid.PcdRuntimeMemoryOnHeap ##CONSUMES + gStandaloneMmPkgTokenSpaceGuid.PcdRuntimeMemoryOnHeapSize ##CONSUMES # # This configuration fails for CLANGPDB, which does not support PIE in the GCC diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c b/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c index dc11d4375a02..8cc1477e9e19 100644 --- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c +++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c @@ -147,24 +147,35 @@ PiMmStandaloneMmCpuDriverEntry ( NsCommBufferSize = ((EFI_MM_COMMUNICATE_HEADER *)NsCommBufferAddr)->MessageLength + sizeof (EFI_MM_COMMUNICATE_HEADER); - GuidedEventContext = NULL; - // Now that the secure world can see the normal world buffer, allocate - // memory to copy the communication buffer to the secure world. - Status = mMmst->MmAllocatePool ( - EfiRuntimeServicesData, - NsCommBufferSize, - (VOID **)&GuidedEventContext - ); - - if (Status != EFI_SUCCESS) { - DEBUG ((DEBUG_ERROR, "Mem alloc failed - 0x%x\n", EventId)); - return EFI_OUT_OF_RESOURCES; + if (FixedPcdGetBool (PcdRuntimeMemoryOnHeap)) { + // + // When runtime memory is allocated on the heap the buffer allocated by + // MmAllocatePool is within the MM 'secure world'. The MMI handler will + // assert as it checks again if the passed buffer is outside the 'secure world'. + // Thus drop the separate allocation and directly pass the NS buffer to + // the MMI handler. + // + GuidedEventContext = (VOID *)NsCommBufferAddr; + } else { + GuidedEventContext = NULL; + // Now that the secure world can see the normal world buffer, allocate + // memory to copy the communication buffer to the secure world. + Status = mMmst->MmAllocatePool ( + EfiRuntimeServicesData, + NsCommBufferSize, + (VOID **)&GuidedEventContext + ); + + if (Status != EFI_SUCCESS) { + DEBUG ((DEBUG_ERROR, "Mem alloc failed - 0x%x\n", EventId)); + return EFI_OUT_OF_RESOURCES; + } + + // X1 contains the VA of the normal world memory accessible from + // secure world. + CopyMem (GuidedEventContext, (CONST VOID *)NsCommBufferAddr, NsCommBufferSize); } - // X1 contains the VA of the normal world memory accessible from - // secure world. - CopyMem (GuidedEventContext, (CONST VOID *)NsCommBufferAddr, NsCommBufferSize); - // Stash the pointer to the allocated Event Context for this CPU PerCpuGuidedEventContext[CpuNumber] = GuidedEventContext; @@ -188,11 +199,14 @@ PiMmStandaloneMmCpuDriverEntry ( // Free the memory allocation done earlier and reset the per-cpu context ASSERT (GuidedEventContext); - CopyMem ((VOID *)NsCommBufferAddr, (CONST VOID *)GuidedEventContext, NsCommBufferSize); - Status = mMmst->MmFreePool ((VOID *)GuidedEventContext); - if (Status != EFI_SUCCESS) { - return EFI_OUT_OF_RESOURCES; + if (!FixedPcdGetBool (PcdRuntimeMemoryOnHeap)) { + CopyMem ((VOID *)NsCommBufferAddr, (CONST VOID *)GuidedEventContext, NsCommBufferSize); + + Status = mMmst->MmFreePool ((VOID *)GuidedEventContext); + if (Status != EFI_SUCCESS) { + return EFI_OUT_OF_RESOURCES; + } } PerCpuGuidedEventContext[CpuNumber] = NULL; diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf index 486ccbac1b7c..d47883f75058 100644 --- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf +++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf @@ -37,6 +37,9 @@ gEfiMmConfigurationProtocolGuid # PROTOCOL ALWAYS_PRODUCED gEfiMmCpuProtocolGuid # PROTOCOL ALWAYS_PRODUCED +[Pcd] + gStandaloneMmPkgTokenSpaceGuid.PcdRuntimeMemoryOnHeap ##CONSUMES + [Guids] gEfiHobListGuid gEfiMmPeiMmramMemoryReserveGuid diff --git a/StandaloneMmPkg/StandaloneMmPkg.dec b/StandaloneMmPkg/StandaloneMmPkg.dec index fc91bb4c3e11..259c1c5e093b 100644 --- a/StandaloneMmPkg/StandaloneMmPkg.dec +++ b/StandaloneMmPkg/StandaloneMmPkg.dec @@ -53,3 +53,12 @@ # in the MM phase. Minimum value is 1. Sections nested more deeply are rejected. # @Prompt Maximum permitted FwVol section nesting depth (exclusive) in MM. gStandaloneMmPkgTokenSpaceGuid.PcdFwVolMmMaxEncapsulationDepth|0x10|UINT32|0x00000001 + ## Use a fixed size memory buffer on the heap as free memory pool. + # Using this option will increase the binary size, but removes the callers need + # to provide a memory range in the 'secure world' that can be used for runtime + # memory allocations. + # You need at least PcdVariableStoreSize + MAX(PcdMaxVolatileVariableSize, PcdMaxVariableSize, + # PcdMaxAuthVariableSize, PcdMaxHardwareErrorVariableSize) + NvVariableLength of memory. + + gStandaloneMmPkgTokenSpaceGuid.PcdRuntimeMemoryOnHeap|FALSE|BOOLEAN|0x00000002 + gStandaloneMmPkgTokenSpaceGuid.PcdRuntimeMemoryOnHeapSize|0x100000|UINT32|0x00000003 From 2e359181a064aa5d23d4798a6311d6e8858a1d89 Mon Sep 17 00:00:00 2001 From: Patrick Rudolph Date: Wed, 12 Jun 2024 13:35:35 +0200 Subject: [PATCH 4/4] StandaloneMmPkg: Fix building on GCC Add missing EFIAPI qualifier. Cc: Ard Biesheuvel Cc: Jiaxin Wu Cc: Ray Ni Cc: Sami Mujawar Signed-off-by: Patrick Rudolph --- StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c | 1 + StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c | 1 + StandaloneMmPkg/Include/StandaloneMmCpu.h | 2 ++ 3 files changed, 4 insertions(+) diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c b/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c index 8cc1477e9e19..f482c0cef28e 100644 --- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c +++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c @@ -117,6 +117,7 @@ CheckBufferAddr ( @retval EFI_UNSUPPORTED Operation not supported. **/ EFI_STATUS +EFIAPI PiMmStandaloneMmCpuDriverEntry ( IN UINTN EventId, IN UINTN CpuNumber, diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c index c5ec1a5a80c5..70f2735e15d5 100644 --- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c +++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c @@ -89,6 +89,7 @@ GetGuidedHobData ( driver endpoint descriptor. **/ EFI_STATUS +EFIAPI StandaloneMmCpuInitialize ( IN EFI_HANDLE ImageHandle, // not actual imagehandle IN EFI_MM_SYSTEM_TABLE *SystemTable // not actual systemtable diff --git a/StandaloneMmPkg/Include/StandaloneMmCpu.h b/StandaloneMmPkg/Include/StandaloneMmCpu.h index 1dce7c132ec2..4045ca62a7ab 100644 --- a/StandaloneMmPkg/Include/StandaloneMmCpu.h +++ b/StandaloneMmPkg/Include/StandaloneMmCpu.h @@ -17,6 +17,7 @@ typedef EFI_STATUS +EFIAPI (*PI_MM_CPU_DRIVER_ENTRYPOINT) ( IN UINTN EventId, IN UINTN CpuNumber, @@ -59,6 +60,7 @@ extern EFI_MM_CONFIGURATION_PROTOCOL mMmConfig; @retval EFI_OUT_OF_RESOURCES Out of resources. @retval EFI_UNSUPPORTED Operation not supported. **/ +EFIAPI EFI_STATUS PiMmStandaloneMmCpuDriverEntry ( IN UINTN EventId,