]> git.proxmox.com Git - mirror_edk2.git/blobdiff - UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
UefiCpuPkg: Replace BSD License with BSD+Patent License
[mirror_edk2.git] / UefiCpuPkg / PiSmmCpuDxeSmm / SmmCpuMemoryManagement.c
index 2d7dba59bf30370241c06f22bb4e690a9533846d..069be3aaa5a5ed529593cf652b53e5f07092f1a2 100644 (file)
@@ -1,20 +1,18 @@
 /** @file\r
 \r
-Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.<BR>\r
-This program and the accompanying materials\r
-are licensed and made available under the terms and conditions of the BSD License\r
-which accompanies this distribution.  The full text of the license may be found at\r
-http://opensource.org/licenses/bsd-license.php\r
-\r
-THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,\r
-WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.\r
+Copyright (c) 2016 - 2019, Intel Corporation. All rights reserved.<BR>\r
+SPDX-License-Identifier: BSD-2-Clause-Patent\r
 \r
 **/\r
 \r
 #include "PiSmmCpuDxeSmm.h"\r
 \r
-#define NEXT_MEMORY_DESCRIPTOR(MemoryDescriptor, Size) \\r
-  ((EFI_MEMORY_DESCRIPTOR *)((UINT8 *)(MemoryDescriptor) + (Size)))\r
+//\r
+// attributes for reserved memory before it is promoted to system memory\r
+//\r
+#define EFI_MEMORY_PRESENT      0x0100000000000000ULL\r
+#define EFI_MEMORY_INITIALIZED  0x0200000000000000ULL\r
+#define EFI_MEMORY_TESTED       0x0400000000000000ULL\r
 \r
 #define PREVIOUS_MEMORY_DESCRIPTOR(MemoryDescriptor, Size) \\r
   ((EFI_MEMORY_DESCRIPTOR *)((UINT8 *)(MemoryDescriptor) - (Size)))\r
@@ -23,12 +21,34 @@ EFI_MEMORY_DESCRIPTOR *mUefiMemoryMap;
 UINTN                 mUefiMemoryMapSize;\r
 UINTN                 mUefiDescriptorSize;\r
 \r
+EFI_GCD_MEMORY_SPACE_DESCRIPTOR   *mGcdMemSpace       = NULL;\r
+UINTN                             mGcdMemNumberOfDesc = 0;\r
+\r
+EFI_MEMORY_ATTRIBUTES_TABLE  *mUefiMemoryAttributesTable = NULL;\r
+\r
 PAGE_ATTRIBUTE_TABLE mPageAttributeTable[] = {\r
   {Page4K,  SIZE_4KB, PAGING_4K_ADDRESS_MASK_64},\r
   {Page2M,  SIZE_2MB, PAGING_2M_ADDRESS_MASK_64},\r
   {Page1G,  SIZE_1GB, PAGING_1G_ADDRESS_MASK_64},\r
 };\r
 \r
+UINTN  mInternalGr3;\r
+\r
+/**\r
+  Set the internal page table base address.\r
+  If it is non zero, further MemoryAttribute modification will be on this page table.\r
+  If it is zero, further MemoryAttribute modification will be on real page table.\r
+\r
+  @param Cr3 page table base.\r
+**/\r
+VOID\r
+SetPageTableBase (\r
+  IN UINTN   Cr3\r
+  )\r
+{\r
+  mInternalGr3 = Cr3;\r
+}\r
+\r
 /**\r
   Return page table base.\r
 \r
@@ -39,6 +59,9 @@ GetPageTableBase (
   VOID\r
   )\r
 {\r
+  if (mInternalGr3 != 0) {\r
+    return mInternalGr3;\r
+  }\r
   return (AsmReadCr3 () & PAGING_4K_ADDRESS_MASK_64);\r
 }\r
 \r
@@ -211,6 +234,17 @@ ConvertPageEntryAttribute (
   if ((Attributes & EFI_MEMORY_RO) != 0) {\r
     if (IsSet) {\r
       NewPageEntry &= ~(UINT64)IA32_PG_RW;\r
+      if (mInternalGr3 != 0) {\r
+        // Environment setup\r
+        // ReadOnly page need set Dirty bit for shadow stack\r
+        NewPageEntry |= IA32_PG_D;\r
+        // Clear user bit for supervisor shadow stack\r
+        NewPageEntry &= ~(UINT64)IA32_PG_U;\r
+      } else {\r
+        // Runtime update\r
+        // Clear dirty bit for non shadow stack, to protect RO page.\r
+        NewPageEntry &= ~(UINT64)IA32_PG_D;\r
+      }\r
     } else {\r
       NewPageEntry |= IA32_PG_RW;\r
     }\r
@@ -552,12 +586,12 @@ SmmSetMemoryAttributesEx (
                                 BaseAddress and Length cannot be modified.\r
   @retval EFI_INVALID_PARAMETER Length is zero.\r
                                 Attributes specified an illegal combination of attributes that\r
-                                cannot be set together.\r
+                                cannot be cleared together.\r
   @retval EFI_OUT_OF_RESOURCES  There are not enough system resources to modify the attributes of\r
                                 the memory resource range.\r
   @retval EFI_UNSUPPORTED       The processor does not support one or more bytes of the memory\r
                                 resource range specified by BaseAddress and Length.\r
-                                The bit mask of attributes is not support for the memory resource\r
+                                The bit mask of attributes is not supported for the memory resource\r
                                 range specified by BaseAddress and Length.\r
 \r
 **/\r
@@ -604,7 +638,7 @@ SmmClearMemoryAttributesEx (
                                 the memory resource range.\r
   @retval EFI_UNSUPPORTED       The processor does not support one or more bytes of the memory\r
                                 resource range specified by BaseAddress and Length.\r
-                                The bit mask of attributes is not support for the memory resource\r
+                                The bit mask of attributes is not supported for the memory resource\r
                                 range specified by BaseAddress and Length.\r
 \r
 **/\r
@@ -632,12 +666,12 @@ SmmSetMemoryAttributes (
                                 BaseAddress and Length cannot be modified.\r
   @retval EFI_INVALID_PARAMETER Length is zero.\r
                                 Attributes specified an illegal combination of attributes that\r
-                                cannot be set together.\r
+                                cannot be cleared together.\r
   @retval EFI_OUT_OF_RESOURCES  There are not enough system resources to modify the attributes of\r
                                 the memory resource range.\r
   @retval EFI_UNSUPPORTED       The processor does not support one or more bytes of the memory\r
                                 resource range specified by BaseAddress and Length.\r
-                                The bit mask of attributes is not support for the memory resource\r
+                                The bit mask of attributes is not supported for the memory resource\r
                                 range specified by BaseAddress and Length.\r
 \r
 **/\r
@@ -652,7 +686,59 @@ SmmClearMemoryAttributes (
   return SmmClearMemoryAttributesEx (BaseAddress, Length, Attributes, NULL);\r
 }\r
 \r
+/**\r
+  Set ShadowStack memory.\r
+\r
+  @param[in]  Cr3              The page table base address.\r
+  @param[in]  BaseAddress      The physical address that is the start address of a memory region.\r
+  @param[in]  Length           The size in bytes of the memory region.\r
+\r
+  @retval EFI_SUCCESS           The shadow stack memory is set.\r
+**/\r
+EFI_STATUS\r
+SetShadowStack (\r
+  IN  UINTN                                      Cr3,\r
+  IN  EFI_PHYSICAL_ADDRESS                       BaseAddress,\r
+  IN  UINT64                                     Length\r
+  )\r
+{\r
+  EFI_STATUS  Status;\r
+\r
+  SetPageTableBase (Cr3);\r
+\r
+  Status = SmmSetMemoryAttributes (BaseAddress, Length, EFI_MEMORY_RO);\r
+\r
+  SetPageTableBase (0);\r
+\r
+  return Status;\r
+}\r
+\r
+/**\r
+  Set not present memory.\r
 \r
+  @param[in]  Cr3              The page table base address.\r
+  @param[in]  BaseAddress      The physical address that is the start address of a memory region.\r
+  @param[in]  Length           The size in bytes of the memory region.\r
+\r
+  @retval EFI_SUCCESS           The not present memory is set.\r
+**/\r
+EFI_STATUS\r
+SetNotPresentPage (\r
+  IN  UINTN                                      Cr3,\r
+  IN  EFI_PHYSICAL_ADDRESS                       BaseAddress,\r
+  IN  UINT64                                     Length\r
+  )\r
+{\r
+  EFI_STATUS  Status;\r
+\r
+  SetPageTableBase (Cr3);\r
+\r
+  Status = SmmSetMemoryAttributes (BaseAddress, Length, EFI_MEMORY_RP);\r
+\r
+  SetPageTableBase (0);\r
+\r
+  return Status;\r
+}\r
 \r
 /**\r
   Retrieves a pointer to the system configuration table from the SMM System Table\r
@@ -768,6 +854,52 @@ PatchSmmSaveStateMap (
     );\r
 }\r
 \r
+/**\r
+  This function sets GDT/IDT buffer to be RO and XP.\r
+**/\r
+VOID\r
+PatchGdtIdtMap (\r
+  VOID\r
+  )\r
+{\r
+  EFI_PHYSICAL_ADDRESS       BaseAddress;\r
+  UINTN                      Size;\r
+\r
+  //\r
+  // GDT\r
+  //\r
+  DEBUG ((DEBUG_INFO, "PatchGdtIdtMap - GDT:\n"));\r
+\r
+  BaseAddress = mGdtBuffer;\r
+  Size = ALIGN_VALUE(mGdtBufferSize, SIZE_4KB);\r
+  //\r
+  // The range should have been set to RO\r
+  // if it is allocated with EfiRuntimeServicesCode.\r
+  //\r
+  SmmSetMemoryAttributes (\r
+    BaseAddress,\r
+    Size,\r
+    EFI_MEMORY_XP\r
+    );\r
+\r
+  //\r
+  // IDT\r
+  //\r
+  DEBUG ((DEBUG_INFO, "PatchGdtIdtMap - IDT:\n"));\r
+\r
+  BaseAddress = gcSmiIdtr.Base;\r
+  Size = ALIGN_VALUE(gcSmiIdtr.Limit + 1, SIZE_4KB);\r
+  //\r
+  // The range should have been set to RO\r
+  // if it is allocated with EfiRuntimeServicesCode.\r
+  //\r
+  SmmSetMemoryAttributes (\r
+    BaseAddress,\r
+    Size,\r
+    EFI_MEMORY_XP\r
+    );\r
+}\r
+\r
 /**\r
   This function sets memory attribute according to MemoryAttributesTable.\r
 **/\r
@@ -976,6 +1108,80 @@ MergeMemoryMapForNotPresentEntry (
   return ;\r
 }\r
 \r
+/**\r
+  This function caches the GCD memory map information.\r
+**/\r
+VOID\r
+GetGcdMemoryMap (\r
+  VOID\r
+  )\r
+{\r
+  UINTN                            NumberOfDescriptors;\r
+  EFI_GCD_MEMORY_SPACE_DESCRIPTOR  *MemSpaceMap;\r
+  EFI_STATUS                       Status;\r
+  UINTN                            Index;\r
+\r
+  Status = gDS->GetMemorySpaceMap (&NumberOfDescriptors, &MemSpaceMap);\r
+  if (EFI_ERROR (Status)) {\r
+    return ;\r
+  }\r
+\r
+  mGcdMemNumberOfDesc = 0;\r
+  for (Index = 0; Index < NumberOfDescriptors; Index++) {\r
+    if (MemSpaceMap[Index].GcdMemoryType == EfiGcdMemoryTypeReserved &&\r
+        (MemSpaceMap[Index].Capabilities & (EFI_MEMORY_PRESENT | EFI_MEMORY_INITIALIZED | EFI_MEMORY_TESTED)) ==\r
+          (EFI_MEMORY_PRESENT | EFI_MEMORY_INITIALIZED)\r
+          ) {\r
+      mGcdMemNumberOfDesc++;\r
+    }\r
+  }\r
+\r
+  mGcdMemSpace = AllocateZeroPool (mGcdMemNumberOfDesc * sizeof (EFI_GCD_MEMORY_SPACE_DESCRIPTOR));\r
+  ASSERT (mGcdMemSpace != NULL);\r
+  if (mGcdMemSpace == NULL) {\r
+    mGcdMemNumberOfDesc = 0;\r
+    gBS->FreePool (MemSpaceMap);\r
+    return ;\r
+  }\r
+\r
+  mGcdMemNumberOfDesc = 0;\r
+  for (Index = 0; Index < NumberOfDescriptors; Index++) {\r
+    if (MemSpaceMap[Index].GcdMemoryType == EfiGcdMemoryTypeReserved &&\r
+        (MemSpaceMap[Index].Capabilities & (EFI_MEMORY_PRESENT | EFI_MEMORY_INITIALIZED | EFI_MEMORY_TESTED)) ==\r
+          (EFI_MEMORY_PRESENT | EFI_MEMORY_INITIALIZED)\r
+          ) {\r
+      CopyMem (\r
+        &mGcdMemSpace[mGcdMemNumberOfDesc],\r
+        &MemSpaceMap[Index],\r
+        sizeof(EFI_GCD_MEMORY_SPACE_DESCRIPTOR)\r
+        );\r
+      mGcdMemNumberOfDesc++;\r
+    }\r
+  }\r
+\r
+  gBS->FreePool (MemSpaceMap);\r
+}\r
+\r
+/**\r
+  Get UEFI MemoryAttributesTable.\r
+**/\r
+VOID\r
+GetUefiMemoryAttributesTable (\r
+  VOID\r
+  )\r
+{\r
+  EFI_STATUS                   Status;\r
+  EFI_MEMORY_ATTRIBUTES_TABLE  *MemoryAttributesTable;\r
+  UINTN                        MemoryAttributesTableSize;\r
+\r
+  Status = EfiGetSystemConfigurationTable (&gEfiMemoryAttributesTableGuid, (VOID **)&MemoryAttributesTable);\r
+  if (!EFI_ERROR (Status) && (MemoryAttributesTable != NULL)) {\r
+    MemoryAttributesTableSize = sizeof(EFI_MEMORY_ATTRIBUTES_TABLE) + MemoryAttributesTable->DescriptorSize * MemoryAttributesTable->NumberOfEntries;\r
+    mUefiMemoryAttributesTable = AllocateCopyPool (MemoryAttributesTableSize, MemoryAttributesTable);\r
+    ASSERT (mUefiMemoryAttributesTable != NULL);\r
+  }\r
+}\r
+\r
 /**\r
   This function caches the UEFI memory map information.\r
 **/\r
@@ -1035,6 +1241,16 @@ GetUefiMemoryMap (
   ASSERT (mUefiMemoryMap != NULL);\r
 \r
   gBS->FreePool (MemoryMap);\r
+\r
+  //\r
+  // Get additional information from GCD memory map.\r
+  //\r
+  GetGcdMemoryMap ();\r
+\r
+  //\r
+  // Get UEFI memory attributes table.\r
+  //\r
+  GetUefiMemoryAttributesTable ();\r
 }\r
 \r
 /**\r
@@ -1053,36 +1269,85 @@ SetUefiMemMapAttributes (
   EFI_MEMORY_DESCRIPTOR *MemoryMap;\r
   UINTN                 MemoryMapEntryCount;\r
   UINTN                 Index;\r
+  EFI_MEMORY_DESCRIPTOR *Entry;\r
 \r
   DEBUG ((DEBUG_INFO, "SetUefiMemMapAttributes\n"));\r
 \r
-  if (mUefiMemoryMap == NULL) {\r
-    DEBUG ((DEBUG_INFO, "UefiMemoryMap - NULL\n"));\r
-    return ;\r
+  if (mUefiMemoryMap != NULL) {\r
+    MemoryMapEntryCount = mUefiMemoryMapSize/mUefiDescriptorSize;\r
+    MemoryMap = mUefiMemoryMap;\r
+    for (Index = 0; Index < MemoryMapEntryCount; Index++) {\r
+      if (IsUefiPageNotPresent(MemoryMap)) {\r
+        Status = SmmSetMemoryAttributes (\r
+                   MemoryMap->PhysicalStart,\r
+                   EFI_PAGES_TO_SIZE((UINTN)MemoryMap->NumberOfPages),\r
+                   EFI_MEMORY_RP\r
+                   );\r
+        DEBUG ((\r
+          DEBUG_INFO,\r
+          "UefiMemory protection: 0x%lx - 0x%lx %r\n",\r
+          MemoryMap->PhysicalStart,\r
+          MemoryMap->PhysicalStart + (UINT64)EFI_PAGES_TO_SIZE((UINTN)MemoryMap->NumberOfPages),\r
+          Status\r
+          ));\r
+      }\r
+      MemoryMap = NEXT_MEMORY_DESCRIPTOR(MemoryMap, mUefiDescriptorSize);\r
+    }\r
   }\r
+  //\r
+  // Do not free mUefiMemoryMap, it will be checked in IsSmmCommBufferForbiddenAddress().\r
+  //\r
 \r
-  MemoryMapEntryCount = mUefiMemoryMapSize/mUefiDescriptorSize;\r
-  MemoryMap = mUefiMemoryMap;\r
-  for (Index = 0; Index < MemoryMapEntryCount; Index++) {\r
-    if (IsUefiPageNotPresent(MemoryMap)) {\r
+  //\r
+  // Set untested memory as not present.\r
+  //\r
+  if (mGcdMemSpace != NULL) {\r
+    for (Index = 0; Index < mGcdMemNumberOfDesc; Index++) {\r
       Status = SmmSetMemoryAttributes (\r
-                 MemoryMap->PhysicalStart,\r
-                 EFI_PAGES_TO_SIZE((UINTN)MemoryMap->NumberOfPages),\r
+                 mGcdMemSpace[Index].BaseAddress,\r
+                 mGcdMemSpace[Index].Length,\r
                  EFI_MEMORY_RP\r
                  );\r
       DEBUG ((\r
         DEBUG_INFO,\r
-        "UefiMemory protection: 0x%lx - 0x%lx %r\n",\r
-        MemoryMap->PhysicalStart,\r
-        MemoryMap->PhysicalStart + (UINT64)EFI_PAGES_TO_SIZE((UINTN)MemoryMap->NumberOfPages),\r
+        "GcdMemory protection: 0x%lx - 0x%lx %r\n",\r
+        mGcdMemSpace[Index].BaseAddress,\r
+        mGcdMemSpace[Index].BaseAddress + mGcdMemSpace[Index].Length,\r
         Status\r
         ));\r
     }\r
-    MemoryMap = NEXT_MEMORY_DESCRIPTOR(MemoryMap, mUefiDescriptorSize);\r
   }\r
+  //\r
+  // Do not free mGcdMemSpace, it will be checked in IsSmmCommBufferForbiddenAddress().\r
+  //\r
 \r
   //\r
-  // Do free mUefiMemoryMap, it will be checked in IsSmmCommBufferForbiddenAddress().\r
+  // Set UEFI runtime memory with EFI_MEMORY_RO as not present.\r
+  //\r
+  if (mUefiMemoryAttributesTable != NULL) {\r
+    Entry = (EFI_MEMORY_DESCRIPTOR *)(mUefiMemoryAttributesTable + 1);\r
+    for (Index = 0; Index < mUefiMemoryAttributesTable->NumberOfEntries; Index++) {\r
+      if (Entry->Type == EfiRuntimeServicesCode || Entry->Type == EfiRuntimeServicesData) {\r
+        if ((Entry->Attribute & EFI_MEMORY_RO) != 0) {\r
+          Status = SmmSetMemoryAttributes (\r
+                     Entry->PhysicalStart,\r
+                     EFI_PAGES_TO_SIZE((UINTN)Entry->NumberOfPages),\r
+                     EFI_MEMORY_RP\r
+                     );\r
+          DEBUG ((\r
+            DEBUG_INFO,\r
+            "UefiMemoryAttribute protection: 0x%lx - 0x%lx %r\n",\r
+            Entry->PhysicalStart,\r
+            Entry->PhysicalStart + (UINT64)EFI_PAGES_TO_SIZE((UINTN)Entry->NumberOfPages),\r
+            Status\r
+            ));\r
+        }\r
+      }\r
+      Entry = NEXT_MEMORY_DESCRIPTOR (Entry, mUefiMemoryAttributesTable->DescriptorSize);\r
+    }\r
+  }\r
+  //\r
+  // Do not free mUefiMemoryAttributesTable, it will be checked in IsSmmCommBufferForbiddenAddress().\r
   //\r
 }\r
 \r
@@ -1102,21 +1367,44 @@ IsSmmCommBufferForbiddenAddress (
   EFI_MEMORY_DESCRIPTOR *MemoryMap;\r
   UINTN                 MemoryMapEntryCount;\r
   UINTN                 Index;\r
-\r
-  if (mUefiMemoryMap == NULL) {\r
-    return FALSE;\r
+  EFI_MEMORY_DESCRIPTOR *Entry;\r
+\r
+  if (mUefiMemoryMap != NULL) {\r
+    MemoryMap = mUefiMemoryMap;\r
+    MemoryMapEntryCount = mUefiMemoryMapSize/mUefiDescriptorSize;\r
+    for (Index = 0; Index < MemoryMapEntryCount; Index++) {\r
+      if (IsUefiPageNotPresent (MemoryMap)) {\r
+        if ((Address >= MemoryMap->PhysicalStart) &&\r
+            (Address < MemoryMap->PhysicalStart + EFI_PAGES_TO_SIZE((UINTN)MemoryMap->NumberOfPages)) ) {\r
+          return TRUE;\r
+        }\r
+      }\r
+      MemoryMap = NEXT_MEMORY_DESCRIPTOR(MemoryMap, mUefiDescriptorSize);\r
+    }\r
   }\r
 \r
-  MemoryMap = mUefiMemoryMap;\r
-  MemoryMapEntryCount = mUefiMemoryMapSize/mUefiDescriptorSize;\r
-  for (Index = 0; Index < MemoryMapEntryCount; Index++) {\r
-    if (IsUefiPageNotPresent (MemoryMap)) {\r
-      if ((Address >= MemoryMap->PhysicalStart) &&\r
-          (Address < MemoryMap->PhysicalStart + EFI_PAGES_TO_SIZE((UINTN)MemoryMap->NumberOfPages)) ) {\r
+  if (mGcdMemSpace != NULL) {\r
+    for (Index = 0; Index < mGcdMemNumberOfDesc; Index++) {\r
+      if ((Address >= mGcdMemSpace[Index].BaseAddress) &&\r
+          (Address < mGcdMemSpace[Index].BaseAddress + mGcdMemSpace[Index].Length) ) {\r
         return TRUE;\r
       }\r
     }\r
-    MemoryMap = NEXT_MEMORY_DESCRIPTOR(MemoryMap, mUefiDescriptorSize);\r
+  }\r
+\r
+  if (mUefiMemoryAttributesTable != NULL) {\r
+    Entry = (EFI_MEMORY_DESCRIPTOR *)(mUefiMemoryAttributesTable + 1);\r
+    for (Index = 0; Index < mUefiMemoryAttributesTable->NumberOfEntries; Index++) {\r
+      if (Entry->Type == EfiRuntimeServicesCode || Entry->Type == EfiRuntimeServicesData) {\r
+        if ((Entry->Attribute & EFI_MEMORY_RO) != 0) {\r
+          if ((Address >= Entry->PhysicalStart) &&\r
+              (Address < Entry->PhysicalStart + LShiftU64 (Entry->NumberOfPages, EFI_PAGE_SHIFT))) {\r
+            return TRUE;\r
+          }\r
+          Entry = NEXT_MEMORY_DESCRIPTOR (Entry, mUefiMemoryAttributesTable->DescriptorSize);\r
+        }\r
+      }\r
+    }\r
   }\r
   return FALSE;\r
 }\r
@@ -1139,7 +1427,7 @@ IsSmmCommBufferForbiddenAddress (
   @retval EFI_UNSUPPORTED       The processor does not support one or more\r
                                 bytes of the memory resource range specified\r
                                 by BaseAddress and Length.\r
-                                The bit mask of attributes is not support for\r
+                                The bit mask of attributes is not supported for\r
                                 the memory resource range specified by\r
                                 BaseAddress and Length.\r
 \r
@@ -1164,17 +1452,17 @@ EdkiiSmmSetMemoryAttributes (
   @param  BaseAddress       The physical address that is the start address of\r
                             a memory region.\r
   @param  Length            The size in bytes of the memory region.\r
-  @param  Attributes        The bit mask of attributes to set for the memory\r
+  @param  Attributes        The bit mask of attributes to clear for the memory\r
                             region.\r
 \r
-  @retval EFI_SUCCESS           The attributes were set for the memory region.\r
+  @retval EFI_SUCCESS           The attributes were cleared for the memory region.\r
   @retval EFI_INVALID_PARAMETER Length is zero.\r
                                 Attributes specified an illegal combination of\r
-                                attributes that cannot be set together.\r
+                                attributes that cannot be cleared together.\r
   @retval EFI_UNSUPPORTED       The processor does not support one or more\r
                                 bytes of the memory resource range specified\r
                                 by BaseAddress and Length.\r
-                                The bit mask of attributes is not support for\r
+                                The bit mask of attributes is not supported for\r
                                 the memory resource range specified by\r
                                 BaseAddress and Length.\r
 \r
@@ -1192,7 +1480,7 @@ EdkiiSmmClearMemoryAttributes (
 }\r
 \r
 /**\r
-  This function retrieve the attributes of the memory region specified by\r
+  This function retrieves the attributes of the memory region specified by\r
   BaseAddress and Length. If different attributes are got from different part\r
   of the memory region, EFI_NO_MAPPING will be returned.\r
 \r
@@ -1210,9 +1498,6 @@ EdkiiSmmClearMemoryAttributes (
   @retval EFI_UNSUPPORTED       The processor does not support one or more\r
                                 bytes of the memory resource range specified\r
                                 by BaseAddress and Length.\r
-                                The bit mask of attributes is not support for\r
-                                the memory resource range specified by\r
-                                BaseAddress and Length.\r
 \r
 **/\r
 EFI_STATUS\r