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 <kun.q@outlook.com>
> Sent: Wednesday, January 6, 2021 2:59 AM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>; Sami Mujawar
> <sami.mujawar@arm.com>; Yao, Jiewen <jiewen.yao@intel.com>; Supreeth
> Venkatesh <supreeth.venkatesh@arm.com>
> 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 <ard.biesheuvel@arm.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
>
> Signed-off-by: Kun Qin <kun.q@outlook.com>
> ---
>
> 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.<BR>
> + Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.<BR>
> + Copyright (c) Microsoft Corporation.
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +#include <PiMm.h>
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/HobLib.h>
> +
> +#include <Guid/MmCoreData.h>
> +#include <Guid/MmramMemoryReserve.h>
> +
> +//
> +// 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.<BR>
> # Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.<BR>
> +# 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