Hi Jiewen, Before introducing the MmRange initialization logic, I was hitting a bug caused by `MmIsBufferOutsideMmValid` since it returns TRUE regardless of what I passed in. After the patch, this function start to return FALSE when I verify with regions overlapping with MmRanges. As per OS memory protection, I guess you mean the logic around block everything but runtime, ACPI, and reserved memory after ready to lock? I think we should bring in those logic at some point. But as of now, that routine would need information on DXE memory map, which needs extra efforts (such as a designated handler) to fetch under standalone MM environment. On the other hand, one proposal is for standalone MM, only MmRam and regions requested through a specific protocol/handler are allowed. So that we can implement this MmMemLib from a different approach. We can discuss more about this proposal separately if you would like to. Thanks, Kun From: Yao, Jiewen Sent: Tuesday, January 5, 2021 19:38 To: Kun Qin; devel@edk2.groups.io Cc: Ard Biesheuvel; Sami Mujawar; Supreeth Venkatesh Subject: Re: [edk2-devel] [PATCH v2 04/16] StandaloneMmPkg: StandaloneMmMemLib: Extends support for X64 architecture Thanks Qin. A quick question: May I know what test you have run for this change? Also, I think this patch only protect the MM memory, but not OS memory. Is that expected? Will you consider adding OS memory protection later? > -----Original Message----- > From: Kun Qin > Sent: Wednesday, January 6, 2021 2:59 AM > To: devel@edk2.groups.io > Cc: Ard Biesheuvel ; Sami Mujawar > ; Yao, Jiewen ; Supreeth > Venkatesh > Subject: [PATCH v2 04/16] StandaloneMmPkg: StandaloneMmMemLib: > Extends support for X64 architecture > > This change extends StandaloneMmMemLib library to support X64 > architecture. The implementation is ported from > MdePkg/Library/SmmMemLib. > > Cc: Ard Biesheuvel > Cc: Sami Mujawar > Cc: Jiewen Yao > Cc: Supreeth Venkatesh > > Signed-off-by: Kun Qin > --- > > Notes: > v2: > - Added routine to fix bug of not initializing MmRanges [Jiewen] > - Extends support to x86 instead of x64 only [Hao] > > > StandaloneMmPkg/Library/StandaloneMmMemLib/AArch64/StandaloneM > mMemLibInternal.c | 26 ++++ > > StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib > .c | 52 +++++++ > > StandaloneMmPkg/Library/StandaloneMmMemLib/X86StandaloneMmMe > mLibInternal.c | 155 ++++++++++++++++++++ > > StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib > .inf | 13 +- > 4 files changed, 245 insertions(+), 1 deletion(-) > > diff --git > a/StandaloneMmPkg/Library/StandaloneMmMemLib/AArch64/Standalone > MmMemLibInternal.c > b/StandaloneMmPkg/Library/StandaloneMmMemLib/AArch64/Standalone > MmMemLibInternal.c > index cb7c5e677a6b..46dfce5cac86 100644 > --- > a/StandaloneMmPkg/Library/StandaloneMmMemLib/AArch64/Standalone > MmMemLibInternal.c > +++ > b/StandaloneMmPkg/Library/StandaloneMmMemLib/AArch64/Standalone > MmMemLibInternal.c > @@ -40,4 +40,30 @@ > MmMemLibInternalCalculateMaximumSupportAddress ( > DEBUG ((DEBUG_INFO, "mMmMemLibInternalMaximumSupportAddress > = 0x%lx\n", mMmMemLibInternalMaximumSupportAddress)); > } > > +/** > + Initialize cached Mmram Ranges from HOB. > + > + @retval EFI_UNSUPPORTED The routine is unable to extract MMRAM > information. > + @retval EFI_SUCCESS MmRanges are populated successfully. > + > +**/ > +EFI_STATUS > +MmMemLibInternalPopulateMmramRanges ( > + VOID > + ) > +{ > + // Not implemented for AARCH64. > +} > + > +/** > + Deinitialize cached Mmram Ranges. > + > +**/ > +VOID > +MmMemLibInternalFreeMmramRanges ( > + VOID > + ) > +{ > + // Not implemented for AARCH64. > +} > > diff --git > a/StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMem > Lib.c > b/StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMem > Lib.c > index f82cdb3ba816..f43af2b1cc9b 100644 > --- > a/StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMem > Lib.c > +++ > b/StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMem > Lib.c > @@ -37,6 +37,27 @@ > MmMemLibInternalCalculateMaximumSupportAddress ( > VOID > ); > > +/** > + Initialize cached Mmram Ranges from HOB. > + > + @retval EFI_UNSUPPORTED The routine is unable to extract MMRAM > information. > + @retval EFI_SUCCESS MmRanges are populated successfully. > + > +**/ > +EFI_STATUS > +MmMemLibInternalPopulateMmramRanges ( > + VOID > + ); > + > +/** > + Deinitialize cached Mmram Ranges. > + > +**/ > +VOID > +MmMemLibInternalFreeMmramRanges ( > + VOID > + ); > + > /** > This function check if the buffer is valid per processor architecture and not > overlap with MMRAM. > > @@ -253,11 +274,42 @@ MemLibConstructor ( > IN EFI_MM_SYSTEM_TABLE *MmSystemTable > ) > { > + EFI_STATUS Status; > > // > // Calculate and save maximum support address > // > MmMemLibInternalCalculateMaximumSupportAddress (); > > + // > + // Initialize cached Mmram Ranges from HOB. > + // > + Status = MmMemLibInternalPopulateMmramRanges (); > + > + return Status; > +} > + > +/** > + Destructor for the library. free any resources for Mm Mem library > + > + @param ImageHandle The image handle of the process. > + @param SystemTable The EFI System Table pointer. > + > + @retval EFI_SUCCESS The constructor always returns EFI_SUCCESS. > + > +**/ > +EFI_STATUS > +EFIAPI > +MemLibDestructor ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_MM_SYSTEM_TABLE *MmSystemTable > + ) > +{ > + > + // > + // Deinitialize cached Mmram Ranges. > + // > + MmMemLibInternalFreeMmramRanges (); > + > return EFI_SUCCESS; > } > diff --git > a/StandaloneMmPkg/Library/StandaloneMmMemLib/X86StandaloneMmM > emLibInternal.c > b/StandaloneMmPkg/Library/StandaloneMmMemLib/X86StandaloneMm > MemLibInternal.c > new file mode 100644 > index 000000000000..1a978541716a > --- /dev/null > +++ > b/StandaloneMmPkg/Library/StandaloneMmMemLib/X86StandaloneMm > MemLibInternal.c > @@ -0,0 +1,155 @@ > +/** @file > + Internal ARCH Specific file of MM memory check library. > + > + MM memory check library implementation. This library consumes > MM_ACCESS_PROTOCOL > + to get MMRAM information. In order to use this library instance, the > platform should produce > + all MMRAM range via MM_ACCESS_PROTOCOL, including the range for > firmware (like MM Core > + and MM driver) and/or specific dedicated hardware. > + > + Copyright (c) 2015, Intel Corporation. All rights reserved.
> + Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.
> + Copyright (c) Microsoft Corporation. > + > + SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > + > +// > +// Maximum support address used to check input buffer > +// > +extern EFI_PHYSICAL_ADDRESS > mMmMemLibInternalMaximumSupportAddress; > +extern EFI_MMRAM_DESCRIPTOR *mMmMemLibInternalMmramRanges; > +extern UINTN mMmMemLibInternalMmramCount; > + > +/** > + Calculate and save the maximum support address. > + > +**/ > +VOID > +MmMemLibInternalCalculateMaximumSupportAddress ( > + VOID > + ) > +{ > + VOID *Hob; > + UINT32 RegEax; > + UINT8 PhysicalAddressBits; > + > + // > + // Get physical address bits supported. > + // > + Hob = GetFirstHob (EFI_HOB_TYPE_CPU); > + if (Hob != NULL) { > + PhysicalAddressBits = ((EFI_HOB_CPU *) Hob)->SizeOfMemorySpace; > + } else { > + AsmCpuid (0x80000000, &RegEax, NULL, NULL, NULL); > + if (RegEax >= 0x80000008) { > + AsmCpuid (0x80000008, &RegEax, NULL, NULL, NULL); > + PhysicalAddressBits = (UINT8) RegEax; > + } else { > + PhysicalAddressBits = 36; > + } > + } > + // > + // IA-32e paging translates 48-bit linear addresses to 52-bit physical > addresses. > + // > + ASSERT (PhysicalAddressBits <= 52); > + if (PhysicalAddressBits > 48) { > + PhysicalAddressBits = 48; > + } > + > + // > + // Save the maximum support address in one global variable > + // > + mMmMemLibInternalMaximumSupportAddress = > (EFI_PHYSICAL_ADDRESS)(UINTN)(LShiftU64 (1, PhysicalAddressBits) - 1); > + DEBUG ((DEBUG_INFO, "mMmMemLibInternalMaximumSupportAddress > = 0x%lx\n", mMmMemLibInternalMaximumSupportAddress)); > +} > + > +/** > + Initialize cached Mmram Ranges from HOB. > + > + @retval EFI_UNSUPPORTED The routine is unable to extract MMRAM > information. > + @retval EFI_SUCCESS MmRanges are populated successfully. > + > +**/ > +EFI_STATUS > +MmMemLibInternalPopulateMmramRanges ( > + VOID > + ) > +{ > + VOID *HobStart; > + EFI_HOB_GUID_TYPE *GuidHob; > + MM_CORE_DATA_HOB_DATA *DataInHob; > + MM_CORE_PRIVATE_DATA *MmCorePrivateData; > + EFI_HOB_GUID_TYPE *MmramRangesHob; > + EFI_MMRAM_HOB_DESCRIPTOR_BLOCK *MmramRangesHobData; > + EFI_MMRAM_DESCRIPTOR *MmramDescriptors; > + > + HobStart = GetHobList (); > + DEBUG ((DEBUG_INFO, "%a - 0x%x\n", __FUNCTION__, HobStart)); > + > + // > + // Extract MM Core Private context from the Hob. If absent search for > + // a Hob containing the MMRAM ranges > + // > + GuidHob = GetNextGuidHob (&gMmCoreDataHobGuid, HobStart); > + if (GuidHob == NULL) { > + MmramRangesHob = GetFirstGuidHob > (&gEfiMmPeiMmramMemoryReserveGuid); > + if (MmramRangesHob == NULL) { > + return EFI_UNSUPPORTED; > + } > + > + MmramRangesHobData = GET_GUID_HOB_DATA (MmramRangesHob); > + if (MmramRangesHobData == NULL || MmramRangesHobData- > >Descriptor == NULL) { > + return EFI_UNSUPPORTED; > + } > + > + mMmMemLibInternalMmramCount = MmramRangesHobData- > >NumberOfMmReservedRegions; > + MmramDescriptors = MmramRangesHobData->Descriptor; > + } else { > + DataInHob = GET_GUID_HOB_DATA (GuidHob); > + if (DataInHob == NULL) { > + return EFI_UNSUPPORTED; > + } > + > + MmCorePrivateData = (MM_CORE_PRIVATE_DATA *) (UINTN) > DataInHob->Address; > + if (MmCorePrivateData == NULL || MmCorePrivateData- > >MmramRanges == 0) { > + return EFI_UNSUPPORTED; > + } > + > + mMmMemLibInternalMmramCount = (UINTN) MmCorePrivateData- > >MmramRangeCount; > + MmramDescriptors = (EFI_MMRAM_DESCRIPTOR *) (UINTN) > MmCorePrivateData->MmramRanges; > + } > + > + mMmMemLibInternalMmramRanges = AllocatePool > (mMmMemLibInternalMmramCount * sizeof (EFI_MMRAM_DESCRIPTOR)); > + if (mMmMemLibInternalMmramRanges) { > + CopyMem (mMmMemLibInternalMmramRanges, > + MmramDescriptors, > + mMmMemLibInternalMmramCount * sizeof > (EFI_MMRAM_DESCRIPTOR)); > + } > + > + return EFI_SUCCESS; > +} > + > +/** > + Deinitialize cached Mmram Ranges. > + > +**/ > +VOID > +MmMemLibInternalFreeMmramRanges ( > + VOID > + ) > +{ > + if (mMmMemLibInternalMmramRanges != NULL) { > + FreePool (mMmMemLibInternalMmramRanges); > + } > +} > + > diff --git > a/StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMem > Lib.inf > b/StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMem > Lib.inf > index 49da02e54e6d..062b0d7a1161 100644 > --- > a/StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMem > Lib.inf > +++ > b/StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMem > Lib.inf > @@ -8,6 +8,7 @@ > # > # Copyright (c) 2015, Intel Corporation. All rights reserved.
> # Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.
> +# Copyright (c) Microsoft Corporation. > # > # SPDX-License-Identifier: BSD-2-Clause-Patent > # > @@ -22,16 +23,20 @@ [Defines] > PI_SPECIFICATION_VERSION = 0x00010032 > LIBRARY_CLASS = MemLib|MM_STANDALONE > MM_CORE_STANDALONE > CONSTRUCTOR = MemLibConstructor > + DESTRUCTOR = MemLibDestructor > > # > # The following information is for reference only and not required by the > build tools. > # > -# VALID_ARCHITECTURES = AARCH64 > +# VALID_ARCHITECTURES = IA32 X64 AARCH64 > # > > [Sources.Common] > StandaloneMmMemLib.c > > +[Sources.IA32, Sources.X64] > + X86StandaloneMmMemLibInternal.c > + > [Sources.AARCH64] > AArch64/StandaloneMmMemLibInternal.c > > @@ -42,3 +47,9 @@ [Packages] > [LibraryClasses] > BaseMemoryLib > DebugLib > + HobLib > + MemoryAllocationLib > + > +[Guids] > + gMmCoreDataHobGuid ## SOMETIMES_CONSUMES ## HOB > + gEfiMmPeiMmramMemoryReserveGuid ## SOMETIMES_CONSUMES > ## HOB > -- > 2.30.0.windows.1