* Re: [edk2-devel] [PATCH v3 05/18] StandaloneMmPkg: StandaloneMmMemLib: Extends support for X64 architecture
2021-01-21 1:44 ` [edk2-devel] [PATCH v3 05/18] StandaloneMmPkg: StandaloneMmMemLib: Extends support for X64 architecture Kun Qin
@ 2021-01-26 12:37 ` Yao, Jiewen
0 siblings, 0 replies; 2+ messages in thread
From: Yao, Jiewen @ 2021-01-26 12:37 UTC (permalink / raw)
To: Kun Qin, devel@edk2.groups.io
Cc: Ard Biesheuvel, Sami Mujawar, Supreeth Venkatesh
[-- Attachment #1: Type: text/plain, Size: 12329 bytes --]
Thanks. Looks good to me.
Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
From: Kun Qin <kun.q@outlook.com>
Sent: Thursday, January 21, 2021 9:44 AM
To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>; Sami Mujawar <sami.mujawar@arm.com>; Supreeth Venkatesh <supreeth.venkatesh@arm.com>
Subject: RE: [edk2-devel] [PATCH v3 05/18] StandaloneMmPkg: StandaloneMmMemLib: Extends support for X64 architecture
Hi Jiewen,
Do you have further concerns about this specific patch? I did created a Bugzilla ticket to track the OS memory protection concern here: 3168 - Add non-MMRAM memory protection for Standalone MM environment (tianocore.org)<https://bugzilla.tianocore.org/show_bug.cgi?id=3168>. It introduces a new proposal to allow access of DXE/RT regions from MMRAM, but requires non trivial change. Please let me know if you hold different opinions towards the proposal or this patch.
Thanks,
Kun
From: Kun Qin<mailto:kun.q@outlook.com>
Sent: Thursday, January 14, 2021 14:36
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: Ard Biesheuvel<mailto:ard.biesheuvel@arm.com>; Sami Mujawar<mailto:sami.mujawar@arm.com>; Jiewen Yao<mailto:jiewen.yao@intel.com>; Supreeth Venkatesh<mailto:supreeth.venkatesh@arm.com>
Subject: [edk2-devel] [PATCH v3 05/18] 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<mailto:ard.biesheuvel@arm.com>>
Cc: Sami Mujawar <sami.mujawar@arm.com<mailto:sami.mujawar@arm.com>>
Cc: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com<mailto:supreeth.venkatesh@arm.com>>
Signed-off-by: Kun Qin <kun.q@outlook.com<mailto:kun.q@outlook.com>>
---
Notes:
v3:
- Updated destructor description of varibales to pass CI build.
v2:
- Added routine to fix bug of not initializing MmRanges [Jiewen]
- Extends support to x86 instead of x64 only [Hao]
StandaloneMmPkg/Library/StandaloneMmMemLib/AArch64/StandaloneMmMemLibInternal.c | 27 ++++
StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.c | 52 +++++++
StandaloneMmPkg/Library/StandaloneMmMemLib/X86StandaloneMmMemLibInternal.c | 155 ++++++++++++++++++++
StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.inf | 13 +-
4 files changed, 246 insertions(+), 1 deletion(-)
diff --git a/StandaloneMmPkg/Library/StandaloneMmMemLib/AArch64/StandaloneMmMemLibInternal.c b/StandaloneMmPkg/Library/StandaloneMmMemLib/AArch64/StandaloneMmMemLibInternal.c
index cb7c5e677a6b..4124959e0435 100644
--- a/StandaloneMmPkg/Library/StandaloneMmMemLib/AArch64/StandaloneMmMemLibInternal.c
+++ b/StandaloneMmPkg/Library/StandaloneMmMemLib/AArch64/StandaloneMmMemLibInternal.c
@@ -40,4 +40,31 @@ 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.
+ return EFI_SUCCESS;
+}
+
+/**
+ Deinitialize cached Mmram Ranges.
+
+**/
+VOID
+MmMemLibInternalFreeMmramRanges (
+ VOID
+ )
+{
+ // Not implemented for AARCH64.
+}
diff --git a/StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.c b/StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.c
index b533bd8390cd..2737f95315eb 100644
--- a/StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.c
+++ b/StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.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 Mm Mem library.
+
+ @param ImageHandle The image handle of the process.
+ @param MmSystemTable 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/X86StandaloneMmMemLibInternal.c b/StandaloneMmPkg/Library/StandaloneMmMemLib/X86StandaloneMmMemLibInternal.c
new file mode 100644
index 000000000000..1a978541716a
--- /dev/null
+++ b/StandaloneMmPkg/Library/StandaloneMmMemLib/X86StandaloneMmMemLibInternal.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/StandaloneMmMemLib.inf b/StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.inf
index 49da02e54e6d..062b0d7a1161 100644
--- a/StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.inf
+++ b/StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.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
[-- Attachment #2: Type: text/html, Size: 19932 bytes --]
^ permalink raw reply related [flat|nested] 2+ messages in thread