[PATCH V3 0/4] Add New Memory Attributes

[PATCH V3 0/4] Add New Memory Attributes

[Oleksiy Yakovlev]

This series of patches add usage of new memory
attributes EFI_MEMORY_SP and EFI_MEMORY_CPU_CRYPTO,
introduced in UEFI2.8 (mantis 1919 and 1872).
First patch fix typos in description and introduce two 
bitmasks for all memory type attributes.
Second and fourth patches get rid of multiple memory attributes
bitmasks definitions trough multiple files and headers,
and replace them with new common definitions from MdePkg.
Third patch includes WP attribute into cache type mask in
CpuDexe.h to make next change in this file more clear.

Oleksiy Yakovlev (4):
  MdePkg: Add New Memory Attributes
  MdeModulePkg: Add New Memory Attributes
  UefiCpuPkg: Update EFI_MEMORY_CACHETYPE_MASK definition
  UefiCpuPkg: Add New Memory Attributes

 MdeModulePkg/Core/Dxe/Gcd/Gcd.c                    | 11 ++---------
 MdeModulePkg/Core/Dxe/Mem/Page.c                   |  9 +++------
 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c      |  7 ++-----
 MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c             | 10 ++--------
 MdePkg/Include/Uefi/UefiSpec.h                     | 10 ++++++++--
 UefiCpuPkg/CpuDxe/CpuDxe.c                         | 11 ++++-------
 UefiCpuPkg/CpuDxe/CpuDxe.h                         | 12 ------------
 UefiCpuPkg/CpuDxe/CpuPageTable.c                   |  6 +++---
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c |  2 +-
 9 files changed, 25 insertions(+), 53 deletions(-)

-- 
2.9.0.windows.1


Please consider the environment before printing this email.

The information contained in this message may be confidential and proprietary to American Megatrends (AMI).  This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited.  Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.

[PATCH V3 1/4] MdePkg: Add New Memory Attributes

[Oleksiy Yakovlev]

Add usage of EFI_MEMORY_SP and EFI_MEMORY_CPU_CRYPTO
attributes introduced in UEFI 2.8
(UEFI 2.8, mantis 1919 and 1872)
Fix typos in EFI_MEMORY_CPU_CRYPTO description.
Add attributes bitmasks, grouped by type.

Signed-off-by: Oleksiy Yakovlev <oleksiyy@ami.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
---
 MdePkg/Include/Uefi/UefiSpec.h | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Include/Uefi/UefiSpec.h b/MdePkg/Include/Uefi/UefiSpec.h
index 558e1bc..05b82e0 100644
--- a/MdePkg/Include/Uefi/UefiSpec.h
+++ b/MdePkg/Include/Uefi/UefiSpec.h
@@ -96,9 +96,9 @@ typedef enum {
 #define EFI_MEMORY_SP               0x0000000000040000ULL
 //
 // If this flag is set, the memory region is capable of being
-// protected with the CPU?s memory cryptographic
+// protected with the CPU's memory cryptographic
 // capabilities. If this flag is clear, the memory region is not
-// capable of being protected with the CPU?s memory
+// capable of being protected with the CPU's memory
 // cryptographic capabilities or the CPU does not support CPU
 // memory cryptographic capabilities.
 //
@@ -109,6 +109,12 @@ typedef enum {
 //
 #define EFI_MEMORY_RUNTIME          0x8000000000000000ULL
 
+//
+// Attributes bitmasks, grouped by type
+//
+#define EFI_CACHE_ATTRIBUTE_MASK (EFI_MEMORY_UC | EFI_MEMORY_WC | EFI_MEMORY_WT | EFI_MEMORY_WB | EFI_MEMORY_UCE | EFI_MEMORY_WP)
+#define EFI_MEMORY_ATTRIBUTE_MASK (EFI_MEMORY_RP | EFI_MEMORY_XP | EFI_MEMORY_RO | EFI_MEMORY_SP | EFI_MEMORY_CPU_CRYPTO)
+
 ///
 /// Memory descriptor version number.
 ///
-- 
2.9.0.windows.1


Please consider the environment before printing this email.

The information contained in this message may be confidential and proprietary to American Megatrends (AMI).  This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited.  Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.

[PATCH V3 2/4] MdeModulePkg: Add New Memory Attributes

[Oleksiy Yakovlev]

Add usage of EFI_MEMORY_SP and EFI_MEMORY_CPU_CRYPTO
attributes introduced in UEFI 2.8
(UEFI 2.8, mantis 1919 and 1872)
Use attributes bitmasks, defined in MdePkg.

Signed-off-by: Oleksiy Yakovlev <oleksiyy@ami.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 MdeModulePkg/Core/Dxe/Gcd/Gcd.c               | 11 ++---------
 MdeModulePkg/Core/Dxe/Mem/Page.c              |  9 +++------
 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c |  7 ++-----
 MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c        | 10 ++--------
 4 files changed, 9 insertions(+), 28 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
index 74f3b1b..2d8c076 100644
--- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
+++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
@@ -35,13 +35,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 #define PRESENT_MEMORY_ATTRIBUTES     (EFI_RESOURCE_ATTRIBUTE_PRESENT)
 
-#define EXCLUSIVE_MEMORY_ATTRIBUTES   (EFI_MEMORY_UC | EFI_MEMORY_WC | \
-                                       EFI_MEMORY_WT | EFI_MEMORY_WB | \
-                                       EFI_MEMORY_WP | EFI_MEMORY_UCE)
-
-#define NONEXCLUSIVE_MEMORY_ATTRIBUTES (EFI_MEMORY_XP | EFI_MEMORY_RP | \
-                                        EFI_MEMORY_RO)
-
 //
 // Module Variables
 //
@@ -665,7 +658,7 @@ ConverToCpuArchAttributes (
 {
   UINT64      CpuArchAttributes;
 
-  CpuArchAttributes = Attributes & NONEXCLUSIVE_MEMORY_ATTRIBUTES;
+  CpuArchAttributes = Attributes & EFI_MEMORY_ATTRIBUTE_MASK;
 
   if ( (Attributes & EFI_MEMORY_UC) == EFI_MEMORY_UC) {
     CpuArchAttributes |= EFI_MEMORY_UC;
@@ -951,7 +944,7 @@ CoreConvertSpace (
         // Keep original CPU arch attributes when caller just calls
         // SetMemorySpaceAttributes() with none CPU arch attributes (for example, RUNTIME).
         //
-        Attributes |= (Entry->Attributes & (EXCLUSIVE_MEMORY_ATTRIBUTES | NONEXCLUSIVE_MEMORY_ATTRIBUTES));
+        Attributes |= (Entry->Attributes & (EFI_CACHE_ATTRIBUTE_MASK | EFI_MEMORY_ATTRIBUTE_MASK));
       }
       Entry->Attributes = Attributes;
       break;
diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem/Page.c
index 1f0e3d9..2c2c9cd 100644
--- a/MdeModulePkg/Core/Dxe/Mem/Page.c
+++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
@@ -1857,8 +1857,7 @@ CoreGetMemoryMap (
       MemoryMap->VirtualStart  = 0;
       MemoryMap->NumberOfPages = RShiftU64 ((MergeGcdMapEntry.EndAddress - MergeGcdMapEntry.BaseAddress + 1), EFI_PAGE_SHIFT);
       MemoryMap->Attribute     = (MergeGcdMapEntry.Attributes & ~EFI_MEMORY_PORT_IO) |
-                                (MergeGcdMapEntry.Capabilities & (EFI_MEMORY_RP | EFI_MEMORY_WP | EFI_MEMORY_XP | EFI_MEMORY_RO |
-                                EFI_MEMORY_UC | EFI_MEMORY_UCE | EFI_MEMORY_WC | EFI_MEMORY_WT | EFI_MEMORY_WB));
+                                (MergeGcdMapEntry.Capabilities & (EFI_CACHE_ATTRIBUTE_MASK | EFI_MEMORY_ATTRIBUTE_MASK));
 
       if (MergeGcdMapEntry.GcdMemoryType == EfiGcdMemoryTypeReserved) {
         MemoryMap->Type = EfiReservedMemoryType;
@@ -1892,8 +1891,7 @@ CoreGetMemoryMap (
       MemoryMap->VirtualStart  = 0;
       MemoryMap->NumberOfPages = RShiftU64 ((MergeGcdMapEntry.EndAddress - MergeGcdMapEntry.BaseAddress + 1), EFI_PAGE_SHIFT);
       MemoryMap->Attribute     = MergeGcdMapEntry.Attributes | EFI_MEMORY_NV |
-                                (MergeGcdMapEntry.Capabilities & (EFI_MEMORY_RP | EFI_MEMORY_WP | EFI_MEMORY_XP | EFI_MEMORY_RO |
-                                EFI_MEMORY_UC | EFI_MEMORY_UCE | EFI_MEMORY_WC | EFI_MEMORY_WT | EFI_MEMORY_WB));
+                                (MergeGcdMapEntry.Capabilities & (EFI_CACHE_ATTRIBUTE_MASK | EFI_MEMORY_ATTRIBUTE_MASK));
       MemoryMap->Type          = EfiPersistentMemory;
 
       //
@@ -1935,8 +1933,7 @@ CoreGetMemoryMap (
   MemoryMapEnd = MemoryMap;
   MemoryMap = MemoryMapStart;
   while (MemoryMap < MemoryMapEnd) {
-    MemoryMap->Attribute &= ~(UINT64)(EFI_MEMORY_RP | EFI_MEMORY_RO |
-                                      EFI_MEMORY_XP);
+    MemoryMap->Attribute &= ~(UINT64)EFI_MEMORY_ATTRIBUTE_MASK;
     MemoryMap = NEXT_MEMORY_DESCRIPTOR (MemoryMap, Size);
   }
   MergeMemoryMap (MemoryMapStart, &BufferSize, Size);
diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
index 92a442f..7d1daf0 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
@@ -42,9 +42,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include "DxeMain.h"
 #include "Mem/HeapGuard.h"
 
-#define CACHE_ATTRIBUTE_MASK   (EFI_MEMORY_UC | EFI_MEMORY_WC | EFI_MEMORY_WT | EFI_MEMORY_WB | EFI_MEMORY_UCE | EFI_MEMORY_WP)
-#define MEMORY_ATTRIBUTE_MASK  (EFI_MEMORY_RP | EFI_MEMORY_XP | EFI_MEMORY_RO)
-
 //
 // Image type definitions
 //
@@ -221,7 +218,7 @@ SetUefiImageMemoryAttributes (
   Status = CoreGetMemorySpaceDescriptor(BaseAddress, &Descriptor);
   ASSERT_EFI_ERROR(Status);
 
-  FinalAttributes = (Descriptor.Attributes & CACHE_ATTRIBUTE_MASK) | (Attributes & MEMORY_ATTRIBUTE_MASK);
+  FinalAttributes = (Descriptor.Attributes & EFI_CACHE_ATTRIBUTE_MASK) | (Attributes & EFI_MEMORY_ATTRIBUTE_MASK);
 
   DEBUG ((DEBUG_INFO, "SetUefiImageMemoryAttributes - 0x%016lx - 0x%016lx (0x%016lx)\n", BaseAddress, Length, FinalAttributes));
 
@@ -924,7 +921,7 @@ InitializeDxeNxMemoryProtectionPolicy (
             (EFI_MEMORY_PRESENT | EFI_MEMORY_INITIALIZED)) {
 
         Attributes = GetPermissionAttributeForMemoryType (EfiConventionalMemory) |
-                     (Entry->Attributes & CACHE_ATTRIBUTE_MASK);
+                     (Entry->Attributes & EFI_CACHE_ATTRIBUTE_MASK);
 
         DEBUG ((DEBUG_INFO,
           "Untested GCD memory space region: - 0x%016lx - 0x%016lx (0x%016lx)\n",
diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
index 0385f1d..599a0cd 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
@@ -39,12 +39,6 @@
 
 #define SMRAM_CAPABILITIES  (EFI_MEMORY_WB | EFI_MEMORY_UC)
 
-#define MEMORY_CACHE_ATTRIBUTES (EFI_MEMORY_UC | EFI_MEMORY_WC | \
-                                 EFI_MEMORY_WT | EFI_MEMORY_WB | \
-                                 EFI_MEMORY_WP | EFI_MEMORY_UCE)
-
-#define MEMORY_PAGE_ATTRIBUTES  (EFI_MEMORY_XP | EFI_MEMORY_RP | EFI_MEMORY_RO)
-
 //
 // Function prototypes from produced protocols
 //
@@ -1710,7 +1704,7 @@ SmmIplEntry (
     CpuArch = NULL;
     Status = gBS->LocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOID **)&CpuArch);
     if (!EFI_ERROR (Status)) {
-      MemDesc.Attributes &= ~(MEMORY_CACHE_ATTRIBUTES | MEMORY_PAGE_ATTRIBUTES);
+      MemDesc.Attributes &= ~(EFI_CACHE_ATTRIBUTE_MASK | EFI_MEMORY_ATTRIBUTE_MASK);
       MemDesc.Attributes |= EFI_MEMORY_WB;
       Status = gDS->SetMemorySpaceAttributes (
                       mSmramCacheBase,
@@ -1727,7 +1721,7 @@ SmmIplEntry (
                &MemDesc
                );
         DEBUG ((DEBUG_INFO, "SMRAM attributes: %016lx\n", MemDesc.Attributes));
-        ASSERT ((MemDesc.Attributes & MEMORY_PAGE_ATTRIBUTES) == 0);
+        ASSERT ((MemDesc.Attributes & EFI_MEMORY_ATTRIBUTE_MASK) == 0);
       );
     }
     //
-- 
2.9.0.windows.1


Please consider the environment before printing this email.

The information contained in this message may be confidential and proprietary to American Megatrends (AMI).  This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited.  Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.

[PATCH V3 3/4] UefiCpuPkg: Update EFI_MEMORY_CACHETYPE_MASK definition

[Oleksiy Yakovlev]

Add EFI_MEMORY_WP attribute to
EFI_MEMORY_CACHETYPE_MASK definition.

Signed-off-by: Oleksiy Yakovlev <oleksiyy@ami.com>
---
 UefiCpuPkg/CpuDxe/CpuDxe.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.h b/UefiCpuPkg/CpuDxe/CpuDxe.h
index 9299eaa..b30a896 100644
--- a/UefiCpuPkg/CpuDxe/CpuDxe.h
+++ b/UefiCpuPkg/CpuDxe/CpuDxe.h
@@ -43,7 +43,8 @@
                                        EFI_MEMORY_WC  | \
                                        EFI_MEMORY_WT  | \
                                        EFI_MEMORY_WB  | \
-                                       EFI_MEMORY_UCE   \
+                                       EFI_MEMORY_UCE | \
+                                       EFI_MEMORY_WP    \
                                        )
 
 #define EFI_MEMORY_PAGETYPE_MASK      (EFI_MEMORY_RP  | \
-- 
2.9.0.windows.1


Please consider the environment before printing this email.

The information contained in this message may be confidential and proprietary to American Megatrends (AMI).  This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited.  Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.

[PATCH V3 4/4] UefiCpuPkg: Add New Memory Attributes

[Oleksiy Yakovlev]

Add usage of EFI_MEMORY_SP and EFI_MEMORY_CPU_CRYPTO
attributes introduced in UEFI 2.8.
(UEFI 2.8, mantis 1919 and 1872).
Use attributes bitmasks, defined in MdePkg.

Signed-off-by: Oleksiy Yakovlev <oleksiyy@ami.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 UefiCpuPkg/CpuDxe/CpuDxe.c                         | 11 ++++-------
 UefiCpuPkg/CpuDxe/CpuDxe.h                         | 13 -------------
 UefiCpuPkg/CpuDxe/CpuPageTable.c                   |  6 +++---
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c |  2 +-
 4 files changed, 8 insertions(+), 24 deletions(-)

diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.c b/UefiCpuPkg/CpuDxe/CpuDxe.c
index a571fc3..52cc26e 100644
--- a/UefiCpuPkg/CpuDxe/CpuDxe.c
+++ b/UefiCpuPkg/CpuDxe/CpuDxe.c
@@ -10,9 +10,6 @@
 #include "CpuMp.h"
 #include "CpuPageTable.h"
 
-#define CACHE_ATTRIBUTE_MASK   (EFI_MEMORY_UC | EFI_MEMORY_WC | EFI_MEMORY_WT | EFI_MEMORY_WB | EFI_MEMORY_UCE | EFI_MEMORY_WP)
-#define MEMORY_ATTRIBUTE_MASK  (EFI_MEMORY_RP | EFI_MEMORY_XP | EFI_MEMORY_RO)
-
 //
 // Global Variables
 //
@@ -417,8 +414,8 @@ CpuSetMemoryAttributes (
     return EFI_SUCCESS;
   }
 
-  CacheAttributes = Attributes & CACHE_ATTRIBUTE_MASK;
-  MemoryAttributes = Attributes & MEMORY_ATTRIBUTE_MASK;
+  CacheAttributes = Attributes & EFI_CACHE_ATTRIBUTE_MASK;
+  MemoryAttributes = Attributes & EFI_MEMORY_ATTRIBUTE_MASK;
 
   if (Attributes != (CacheAttributes | MemoryAttributes)) {
     return EFI_INVALID_PARAMETER;
@@ -677,7 +674,7 @@ SetGcdMemorySpaceAttributes (
     gDS->SetMemorySpaceAttributes (
            RegionStart,
            RegionLength,
-           (MemorySpaceMap[Index].Attributes & ~EFI_MEMORY_CACHETYPE_MASK) | (MemorySpaceMap[Index].Capabilities & Attributes)
+           (MemorySpaceMap[Index].Attributes & ~EFI_CACHE_ATTRIBUTE_MASK) | (MemorySpaceMap[Index].Capabilities & Attributes)
            );
   }
 
@@ -754,7 +751,7 @@ RefreshMemoryAttributesFromMtrr (
     gDS->SetMemorySpaceAttributes (
            MemorySpaceMap[Index].BaseAddress,
            MemorySpaceMap[Index].Length,
-           (MemorySpaceMap[Index].Attributes & ~EFI_MEMORY_CACHETYPE_MASK) |
+           (MemorySpaceMap[Index].Attributes & ~EFI_CACHE_ATTRIBUTE_MASK) |
            (MemorySpaceMap[Index].Capabilities & DefaultAttributes)
            );
   }
diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.h b/UefiCpuPkg/CpuDxe/CpuDxe.h
index b30a896..9771ec8 100644
--- a/UefiCpuPkg/CpuDxe/CpuDxe.h
+++ b/UefiCpuPkg/CpuDxe/CpuDxe.h
@@ -39,19 +39,6 @@
 #include <Guid/IdleLoopEvent.h>
 #include <Guid/VectorHandoffTable.h>
 
-#define EFI_MEMORY_CACHETYPE_MASK     (EFI_MEMORY_UC  | \
-                                       EFI_MEMORY_WC  | \
-                                       EFI_MEMORY_WT  | \
-                                       EFI_MEMORY_WB  | \
-                                       EFI_MEMORY_UCE | \
-                                       EFI_MEMORY_WP    \
-                                       )
-
-#define EFI_MEMORY_PAGETYPE_MASK      (EFI_MEMORY_RP  | \
-                                       EFI_MEMORY_XP  | \
-                                       EFI_MEMORY_RO    \
-                                       )
-
 #define HEAP_GUARD_NONSTOP_MODE       \
         ((PcdGet8 (PcdHeapGuardPropertyMask) & (BIT6|BIT4|BIT1|BIT0)) > BIT6)
 
diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c b/UefiCpuPkg/CpuDxe/CpuPageTable.c
index 0a02cb3..06ee1b8 100644
--- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
+++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
@@ -717,7 +717,7 @@ ConvertMemoryPageAttributes (
     return RETURN_INVALID_PARAMETER;
   }
 
-  if ((Attributes & ~(EFI_MEMORY_RP | EFI_MEMORY_RO | EFI_MEMORY_XP)) != 0) {
+  if ((Attributes & ~EFI_MEMORY_ATTRIBUTE_MASK) != 0) {
     DEBUG ((DEBUG_ERROR, "Attributes(0x%lx) has unsupported bit\n", Attributes));
     return EFI_UNSUPPORTED;
   }
@@ -1018,9 +1018,9 @@ RefreshGcdMemoryAttributesFromPaging (
 
       Length = MIN (PageLength, MemorySpaceLength);
       if (Attributes != (MemorySpaceMap[Index].Attributes &
-                         EFI_MEMORY_PAGETYPE_MASK)) {
+                         EFI_MEMORY_ATTRIBUTE_MASK)) {
         NewAttributes = (MemorySpaceMap[Index].Attributes &
-                         ~EFI_MEMORY_PAGETYPE_MASK) | Attributes;
+                         ~EFI_MEMORY_ATTRIBUTE_MASK) | Attributes;
         Status = gDS->SetMemorySpaceAttributes (
                         BaseAddress,
                         Length,
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
index 9c5a92a..ebfc46a 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
@@ -435,7 +435,7 @@ ConvertMemoryPageAttributes (
   EFI_PHYSICAL_ADDRESS              MaximumSupportMemAddress;
 
   ASSERT (Attributes != 0);
-  ASSERT ((Attributes & ~(EFI_MEMORY_RP | EFI_MEMORY_RO | EFI_MEMORY_XP)) == 0);
+  ASSERT ((Attributes & ~EFI_MEMORY_ATTRIBUTE_MASK) == 0);
 
   ASSERT ((BaseAddress & (SIZE_4KB - 1)) == 0);
   ASSERT ((Length & (SIZE_4KB - 1)) == 0);
-- 
2.9.0.windows.1


Please consider the environment before printing this email.

The information contained in this message may be confidential and proprietary to American Megatrends (AMI).  This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited.  Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.

Re: [edk2-devel] [PATCH V3 1/4] MdePkg: Add New Memory Attributes

[Laszlo Ersek]

Hi Oleksiy,

On 07/02/20 22:50, Oleksiy Yakovlev wrote:
> Add usage of EFI_MEMORY_SP and EFI_MEMORY_CPU_CRYPTO
> attributes introduced in UEFI 2.8
> (UEFI 2.8, mantis 1919 and 1872)
> Fix typos in EFI_MEMORY_CPU_CRYPTO description.
> Add attributes bitmasks, grouped by type.
> 
> Signed-off-by: Oleksiy Yakovlev <oleksiyy@ami.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> Reviewed-by: Liming Gao <liming.gao@intel.com>
> ---
>  MdePkg/Include/Uefi/UefiSpec.h | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)

I think you forgot to pick up Zhiguang Liu's "Reviewed-by" from
<https://edk2.groups.io/g/devel/message/61627> (i.e., under the v2 thread).

Of course I think that can be remedied by the maintainer that merges
this series in the end. (I can help with that too, but the series isn't
fully reviewed yet -- patches v3 #2 through #4 still need maintainer
approval.)

Thanks,
Laszlo

> 
> diff --git a/MdePkg/Include/Uefi/UefiSpec.h b/MdePkg/Include/Uefi/UefiSpec.h
> index 558e1bc..05b82e0 100644
> --- a/MdePkg/Include/Uefi/UefiSpec.h
> +++ b/MdePkg/Include/Uefi/UefiSpec.h
> @@ -96,9 +96,9 @@ typedef enum {
>  #define EFI_MEMORY_SP               0x0000000000040000ULL
>  //
>  // If this flag is set, the memory region is capable of being
> -// protected with the CPU?s memory cryptographic
> +// protected with the CPU's memory cryptographic
>  // capabilities. If this flag is clear, the memory region is not
> -// capable of being protected with the CPU?s memory
> +// capable of being protected with the CPU's memory
>  // cryptographic capabilities or the CPU does not support CPU
>  // memory cryptographic capabilities.
>  //
> @@ -109,6 +109,12 @@ typedef enum {
>  //
>  #define EFI_MEMORY_RUNTIME          0x8000000000000000ULL
>  
> +//
> +// Attributes bitmasks, grouped by type
> +//
> +#define EFI_CACHE_ATTRIBUTE_MASK (EFI_MEMORY_UC | EFI_MEMORY_WC | EFI_MEMORY_WT | EFI_MEMORY_WB | EFI_MEMORY_UCE | EFI_MEMORY_WP)
> +#define EFI_MEMORY_ATTRIBUTE_MASK (EFI_MEMORY_RP | EFI_MEMORY_XP | EFI_MEMORY_RO | EFI_MEMORY_SP | EFI_MEMORY_CPU_CRYPTO)
> +
>  ///
>  /// Memory descriptor version number.
>  ///
> 

Re: [edk2-devel] [PATCH V3 2/4] MdeModulePkg: Add New Memory Attributes

[Laszlo Ersek]

On 07/02/20 22:50, Oleksiy Yakovlev wrote:
> Add usage of EFI_MEMORY_SP and EFI_MEMORY_CPU_CRYPTO
> attributes introduced in UEFI 2.8
> (UEFI 2.8, mantis 1919 and 1872)
> Use attributes bitmasks, defined in MdePkg.
> 
> Signed-off-by: Oleksiy Yakovlev <oleksiyy@ami.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  MdeModulePkg/Core/Dxe/Gcd/Gcd.c               | 11 ++---------
>  MdeModulePkg/Core/Dxe/Mem/Page.c              |  9 +++------
>  MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c |  7 ++-----
>  MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c        | 10 ++--------
>  4 files changed, 9 insertions(+), 28 deletions(-)

This is missing Dandan's R-b from
<https://edk2.groups.io/g/devel/message/61630>. (To be picked up by the
maintainer that merges the series, if no more versions of the set are
needed.)

Thanks
Laszlo

> 
> diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> index 74f3b1b..2d8c076 100644
> --- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> +++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> @@ -35,13 +35,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  #define PRESENT_MEMORY_ATTRIBUTES     (EFI_RESOURCE_ATTRIBUTE_PRESENT)
>  
> -#define EXCLUSIVE_MEMORY_ATTRIBUTES   (EFI_MEMORY_UC | EFI_MEMORY_WC | \
> -                                       EFI_MEMORY_WT | EFI_MEMORY_WB | \
> -                                       EFI_MEMORY_WP | EFI_MEMORY_UCE)
> -
> -#define NONEXCLUSIVE_MEMORY_ATTRIBUTES (EFI_MEMORY_XP | EFI_MEMORY_RP | \
> -                                        EFI_MEMORY_RO)
> -
>  //
>  // Module Variables
>  //
> @@ -665,7 +658,7 @@ ConverToCpuArchAttributes (
>  {
>    UINT64      CpuArchAttributes;
>  
> -  CpuArchAttributes = Attributes & NONEXCLUSIVE_MEMORY_ATTRIBUTES;
> +  CpuArchAttributes = Attributes & EFI_MEMORY_ATTRIBUTE_MASK;
>  
>    if ( (Attributes & EFI_MEMORY_UC) == EFI_MEMORY_UC) {
>      CpuArchAttributes |= EFI_MEMORY_UC;
> @@ -951,7 +944,7 @@ CoreConvertSpace (
>          // Keep original CPU arch attributes when caller just calls
>          // SetMemorySpaceAttributes() with none CPU arch attributes (for example, RUNTIME).
>          //
> -        Attributes |= (Entry->Attributes & (EXCLUSIVE_MEMORY_ATTRIBUTES | NONEXCLUSIVE_MEMORY_ATTRIBUTES));
> +        Attributes |= (Entry->Attributes & (EFI_CACHE_ATTRIBUTE_MASK | EFI_MEMORY_ATTRIBUTE_MASK));
>        }
>        Entry->Attributes = Attributes;
>        break;
> diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem/Page.c
> index 1f0e3d9..2c2c9cd 100644
> --- a/MdeModulePkg/Core/Dxe/Mem/Page.c
> +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
> @@ -1857,8 +1857,7 @@ CoreGetMemoryMap (
>        MemoryMap->VirtualStart  = 0;
>        MemoryMap->NumberOfPages = RShiftU64 ((MergeGcdMapEntry.EndAddress - MergeGcdMapEntry.BaseAddress + 1), EFI_PAGE_SHIFT);
>        MemoryMap->Attribute     = (MergeGcdMapEntry.Attributes & ~EFI_MEMORY_PORT_IO) |
> -                                (MergeGcdMapEntry.Capabilities & (EFI_MEMORY_RP | EFI_MEMORY_WP | EFI_MEMORY_XP | EFI_MEMORY_RO |
> -                                EFI_MEMORY_UC | EFI_MEMORY_UCE | EFI_MEMORY_WC | EFI_MEMORY_WT | EFI_MEMORY_WB));
> +                                (MergeGcdMapEntry.Capabilities & (EFI_CACHE_ATTRIBUTE_MASK | EFI_MEMORY_ATTRIBUTE_MASK));
>  
>        if (MergeGcdMapEntry.GcdMemoryType == EfiGcdMemoryTypeReserved) {
>          MemoryMap->Type = EfiReservedMemoryType;
> @@ -1892,8 +1891,7 @@ CoreGetMemoryMap (
>        MemoryMap->VirtualStart  = 0;
>        MemoryMap->NumberOfPages = RShiftU64 ((MergeGcdMapEntry.EndAddress - MergeGcdMapEntry.BaseAddress + 1), EFI_PAGE_SHIFT);
>        MemoryMap->Attribute     = MergeGcdMapEntry.Attributes | EFI_MEMORY_NV |
> -                                (MergeGcdMapEntry.Capabilities & (EFI_MEMORY_RP | EFI_MEMORY_WP | EFI_MEMORY_XP | EFI_MEMORY_RO |
> -                                EFI_MEMORY_UC | EFI_MEMORY_UCE | EFI_MEMORY_WC | EFI_MEMORY_WT | EFI_MEMORY_WB));
> +                                (MergeGcdMapEntry.Capabilities & (EFI_CACHE_ATTRIBUTE_MASK | EFI_MEMORY_ATTRIBUTE_MASK));
>        MemoryMap->Type          = EfiPersistentMemory;
>  
>        //
> @@ -1935,8 +1933,7 @@ CoreGetMemoryMap (
>    MemoryMapEnd = MemoryMap;
>    MemoryMap = MemoryMapStart;
>    while (MemoryMap < MemoryMapEnd) {
> -    MemoryMap->Attribute &= ~(UINT64)(EFI_MEMORY_RP | EFI_MEMORY_RO |
> -                                      EFI_MEMORY_XP);
> +    MemoryMap->Attribute &= ~(UINT64)EFI_MEMORY_ATTRIBUTE_MASK;
>      MemoryMap = NEXT_MEMORY_DESCRIPTOR (MemoryMap, Size);
>    }
>    MergeMemoryMap (MemoryMapStart, &BufferSize, Size);
> diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> index 92a442f..7d1daf0 100644
> --- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> @@ -42,9 +42,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include "DxeMain.h"
>  #include "Mem/HeapGuard.h"
>  
> -#define CACHE_ATTRIBUTE_MASK   (EFI_MEMORY_UC | EFI_MEMORY_WC | EFI_MEMORY_WT | EFI_MEMORY_WB | EFI_MEMORY_UCE | EFI_MEMORY_WP)
> -#define MEMORY_ATTRIBUTE_MASK  (EFI_MEMORY_RP | EFI_MEMORY_XP | EFI_MEMORY_RO)
> -
>  //
>  // Image type definitions
>  //
> @@ -221,7 +218,7 @@ SetUefiImageMemoryAttributes (
>    Status = CoreGetMemorySpaceDescriptor(BaseAddress, &Descriptor);
>    ASSERT_EFI_ERROR(Status);
>  
> -  FinalAttributes = (Descriptor.Attributes & CACHE_ATTRIBUTE_MASK) | (Attributes & MEMORY_ATTRIBUTE_MASK);
> +  FinalAttributes = (Descriptor.Attributes & EFI_CACHE_ATTRIBUTE_MASK) | (Attributes & EFI_MEMORY_ATTRIBUTE_MASK);
>  
>    DEBUG ((DEBUG_INFO, "SetUefiImageMemoryAttributes - 0x%016lx - 0x%016lx (0x%016lx)\n", BaseAddress, Length, FinalAttributes));
>  
> @@ -924,7 +921,7 @@ InitializeDxeNxMemoryProtectionPolicy (
>              (EFI_MEMORY_PRESENT | EFI_MEMORY_INITIALIZED)) {
>  
>          Attributes = GetPermissionAttributeForMemoryType (EfiConventionalMemory) |
> -                     (Entry->Attributes & CACHE_ATTRIBUTE_MASK);
> +                     (Entry->Attributes & EFI_CACHE_ATTRIBUTE_MASK);
>  
>          DEBUG ((DEBUG_INFO,
>            "Untested GCD memory space region: - 0x%016lx - 0x%016lx (0x%016lx)\n",
> diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
> index 0385f1d..599a0cd 100644
> --- a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
> +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
> @@ -39,12 +39,6 @@
>  
>  #define SMRAM_CAPABILITIES  (EFI_MEMORY_WB | EFI_MEMORY_UC)
>  
> -#define MEMORY_CACHE_ATTRIBUTES (EFI_MEMORY_UC | EFI_MEMORY_WC | \
> -                                 EFI_MEMORY_WT | EFI_MEMORY_WB | \
> -                                 EFI_MEMORY_WP | EFI_MEMORY_UCE)
> -
> -#define MEMORY_PAGE_ATTRIBUTES  (EFI_MEMORY_XP | EFI_MEMORY_RP | EFI_MEMORY_RO)
> -
>  //
>  // Function prototypes from produced protocols
>  //
> @@ -1710,7 +1704,7 @@ SmmIplEntry (
>      CpuArch = NULL;
>      Status = gBS->LocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOID **)&CpuArch);
>      if (!EFI_ERROR (Status)) {
> -      MemDesc.Attributes &= ~(MEMORY_CACHE_ATTRIBUTES | MEMORY_PAGE_ATTRIBUTES);
> +      MemDesc.Attributes &= ~(EFI_CACHE_ATTRIBUTE_MASK | EFI_MEMORY_ATTRIBUTE_MASK);
>        MemDesc.Attributes |= EFI_MEMORY_WB;
>        Status = gDS->SetMemorySpaceAttributes (
>                        mSmramCacheBase,
> @@ -1727,7 +1721,7 @@ SmmIplEntry (
>                 &MemDesc
>                 );
>          DEBUG ((DEBUG_INFO, "SMRAM attributes: %016lx\n", MemDesc.Attributes));
> -        ASSERT ((MemDesc.Attributes & MEMORY_PAGE_ATTRIBUTES) == 0);
> +        ASSERT ((MemDesc.Attributes & EFI_MEMORY_ATTRIBUTE_MASK) == 0);
>        );
>      }
>      //
> 

Re: [edk2-devel] [PATCH V3 3/4] UefiCpuPkg: Update EFI_MEMORY_CACHETYPE_MASK definition

[Laszlo Ersek]

On 07/02/20 22:50, Oleksiy Yakovlev wrote:
> Add EFI_MEMORY_WP attribute to
> EFI_MEMORY_CACHETYPE_MASK definition.
> 
> Signed-off-by: Oleksiy Yakovlev <oleksiyy@ami.com>
> ---
>  UefiCpuPkg/CpuDxe/CpuDxe.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.h b/UefiCpuPkg/CpuDxe/CpuDxe.h
> index 9299eaa..b30a896 100644
> --- a/UefiCpuPkg/CpuDxe/CpuDxe.h
> +++ b/UefiCpuPkg/CpuDxe/CpuDxe.h
> @@ -43,7 +43,8 @@
>                                         EFI_MEMORY_WC  | \
>                                         EFI_MEMORY_WT  | \
>                                         EFI_MEMORY_WB  | \
> -                                       EFI_MEMORY_UCE   \
> +                                       EFI_MEMORY_UCE | \
> +                                       EFI_MEMORY_WP    \
>                                         )
>  
>  #define EFI_MEMORY_PAGETYPE_MASK      (EFI_MEMORY_RP  | \
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Re: [edk2-devel] [PATCH V3 0/4] Add New Memory Attributes

[Laszlo Ersek]

On 07/02/20 22:50, Oleksiy Yakovlev wrote:
> This series of patches add usage of new memory
> attributes EFI_MEMORY_SP and EFI_MEMORY_CPU_CRYPTO,
> introduced in UEFI2.8 (mantis 1919 and 1872).
> First patch fix typos in description and introduce two 
> bitmasks for all memory type attributes.
> Second and fourth patches get rid of multiple memory attributes
> bitmasks definitions trough multiple files and headers,
> and replace them with new common definitions from MdePkg.
> Third patch includes WP attribute into cache type mask in
> CpuDexe.h to make next change in this file more clear.
> 
> Oleksiy Yakovlev (4):
>   MdePkg: Add New Memory Attributes
>   MdeModulePkg: Add New Memory Attributes
>   UefiCpuPkg: Update EFI_MEMORY_CACHETYPE_MASK definition
>   UefiCpuPkg: Add New Memory Attributes
> 
>  MdeModulePkg/Core/Dxe/Gcd/Gcd.c                    | 11 ++---------
>  MdeModulePkg/Core/Dxe/Mem/Page.c                   |  9 +++------
>  MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c      |  7 ++-----
>  MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c             | 10 ++--------
>  MdePkg/Include/Uefi/UefiSpec.h                     | 10 ++++++++--
>  UefiCpuPkg/CpuDxe/CpuDxe.c                         | 11 ++++-------
>  UefiCpuPkg/CpuDxe/CpuDxe.h                         | 12 ------------
>  UefiCpuPkg/CpuDxe/CpuPageTable.c                   |  6 +++---
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c |  2 +-
>  9 files changed, 25 insertions(+), 53 deletions(-)
> 

series
Tested-by: Laszlo Ersek <lersek@redhat.com>

Meaning my usual OVMF regression tests, and (due to patch#2) an
ArmVirtQemu boot test on AARCH64 KVM too.

Thanks
Laszlo

Re: [edk2-devel] [PATCH V3 2/4] MdeModulePkg: Add New Memory Attributes

[Laszlo Ersek]

Jian, Hao,

do you intend to review this patch, or should we go with Dandan's R-b
from <https://edk2.groups.io/g/devel/message/61630>?

Thanks
Laszlo

On 07/06/20 11:13, Laszlo Ersek wrote:
> On 07/02/20 22:50, Oleksiy Yakovlev wrote:
>> Add usage of EFI_MEMORY_SP and EFI_MEMORY_CPU_CRYPTO
>> attributes introduced in UEFI 2.8
>> (UEFI 2.8, mantis 1919 and 1872)
>> Use attributes bitmasks, defined in MdePkg.
>>
>> Signed-off-by: Oleksiy Yakovlev <oleksiyy@ami.com>
>> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>  MdeModulePkg/Core/Dxe/Gcd/Gcd.c               | 11 ++---------
>>  MdeModulePkg/Core/Dxe/Mem/Page.c              |  9 +++------
>>  MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c |  7 ++-----
>>  MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c        | 10 ++--------
>>  4 files changed, 9 insertions(+), 28 deletions(-)
> 
> This is missing Dandan's R-b from
> <https://edk2.groups.io/g/devel/message/61630>. (To be picked up by the
> maintainer that merges the series, if no more versions of the set are
> needed.)
> 
> Thanks
> Laszlo
> 
>>
>> diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
>> index 74f3b1b..2d8c076 100644
>> --- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
>> +++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
>> @@ -35,13 +35,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>>  
>>  #define PRESENT_MEMORY_ATTRIBUTES     (EFI_RESOURCE_ATTRIBUTE_PRESENT)
>>  
>> -#define EXCLUSIVE_MEMORY_ATTRIBUTES   (EFI_MEMORY_UC | EFI_MEMORY_WC | \
>> -                                       EFI_MEMORY_WT | EFI_MEMORY_WB | \
>> -                                       EFI_MEMORY_WP | EFI_MEMORY_UCE)
>> -
>> -#define NONEXCLUSIVE_MEMORY_ATTRIBUTES (EFI_MEMORY_XP | EFI_MEMORY_RP | \
>> -                                        EFI_MEMORY_RO)
>> -
>>  //
>>  // Module Variables
>>  //
>> @@ -665,7 +658,7 @@ ConverToCpuArchAttributes (
>>  {
>>    UINT64      CpuArchAttributes;
>>  
>> -  CpuArchAttributes = Attributes & NONEXCLUSIVE_MEMORY_ATTRIBUTES;
>> +  CpuArchAttributes = Attributes & EFI_MEMORY_ATTRIBUTE_MASK;
>>  
>>    if ( (Attributes & EFI_MEMORY_UC) == EFI_MEMORY_UC) {
>>      CpuArchAttributes |= EFI_MEMORY_UC;
>> @@ -951,7 +944,7 @@ CoreConvertSpace (
>>          // Keep original CPU arch attributes when caller just calls
>>          // SetMemorySpaceAttributes() with none CPU arch attributes (for example, RUNTIME).
>>          //
>> -        Attributes |= (Entry->Attributes & (EXCLUSIVE_MEMORY_ATTRIBUTES | NONEXCLUSIVE_MEMORY_ATTRIBUTES));
>> +        Attributes |= (Entry->Attributes & (EFI_CACHE_ATTRIBUTE_MASK | EFI_MEMORY_ATTRIBUTE_MASK));
>>        }
>>        Entry->Attributes = Attributes;
>>        break;
>> diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem/Page.c
>> index 1f0e3d9..2c2c9cd 100644
>> --- a/MdeModulePkg/Core/Dxe/Mem/Page.c
>> +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
>> @@ -1857,8 +1857,7 @@ CoreGetMemoryMap (
>>        MemoryMap->VirtualStart  = 0;
>>        MemoryMap->NumberOfPages = RShiftU64 ((MergeGcdMapEntry.EndAddress - MergeGcdMapEntry.BaseAddress + 1), EFI_PAGE_SHIFT);
>>        MemoryMap->Attribute     = (MergeGcdMapEntry.Attributes & ~EFI_MEMORY_PORT_IO) |
>> -                                (MergeGcdMapEntry.Capabilities & (EFI_MEMORY_RP | EFI_MEMORY_WP | EFI_MEMORY_XP | EFI_MEMORY_RO |
>> -                                EFI_MEMORY_UC | EFI_MEMORY_UCE | EFI_MEMORY_WC | EFI_MEMORY_WT | EFI_MEMORY_WB));
>> +                                (MergeGcdMapEntry.Capabilities & (EFI_CACHE_ATTRIBUTE_MASK | EFI_MEMORY_ATTRIBUTE_MASK));
>>  
>>        if (MergeGcdMapEntry.GcdMemoryType == EfiGcdMemoryTypeReserved) {
>>          MemoryMap->Type = EfiReservedMemoryType;
>> @@ -1892,8 +1891,7 @@ CoreGetMemoryMap (
>>        MemoryMap->VirtualStart  = 0;
>>        MemoryMap->NumberOfPages = RShiftU64 ((MergeGcdMapEntry.EndAddress - MergeGcdMapEntry.BaseAddress + 1), EFI_PAGE_SHIFT);
>>        MemoryMap->Attribute     = MergeGcdMapEntry.Attributes | EFI_MEMORY_NV |
>> -                                (MergeGcdMapEntry.Capabilities & (EFI_MEMORY_RP | EFI_MEMORY_WP | EFI_MEMORY_XP | EFI_MEMORY_RO |
>> -                                EFI_MEMORY_UC | EFI_MEMORY_UCE | EFI_MEMORY_WC | EFI_MEMORY_WT | EFI_MEMORY_WB));
>> +                                (MergeGcdMapEntry.Capabilities & (EFI_CACHE_ATTRIBUTE_MASK | EFI_MEMORY_ATTRIBUTE_MASK));
>>        MemoryMap->Type          = EfiPersistentMemory;
>>  
>>        //
>> @@ -1935,8 +1933,7 @@ CoreGetMemoryMap (
>>    MemoryMapEnd = MemoryMap;
>>    MemoryMap = MemoryMapStart;
>>    while (MemoryMap < MemoryMapEnd) {
>> -    MemoryMap->Attribute &= ~(UINT64)(EFI_MEMORY_RP | EFI_MEMORY_RO |
>> -                                      EFI_MEMORY_XP);
>> +    MemoryMap->Attribute &= ~(UINT64)EFI_MEMORY_ATTRIBUTE_MASK;
>>      MemoryMap = NEXT_MEMORY_DESCRIPTOR (MemoryMap, Size);
>>    }
>>    MergeMemoryMap (MemoryMapStart, &BufferSize, Size);
>> diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
>> index 92a442f..7d1daf0 100644
>> --- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
>> +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
>> @@ -42,9 +42,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>>  #include "DxeMain.h"
>>  #include "Mem/HeapGuard.h"
>>  
>> -#define CACHE_ATTRIBUTE_MASK   (EFI_MEMORY_UC | EFI_MEMORY_WC | EFI_MEMORY_WT | EFI_MEMORY_WB | EFI_MEMORY_UCE | EFI_MEMORY_WP)
>> -#define MEMORY_ATTRIBUTE_MASK  (EFI_MEMORY_RP | EFI_MEMORY_XP | EFI_MEMORY_RO)
>> -
>>  //
>>  // Image type definitions
>>  //
>> @@ -221,7 +218,7 @@ SetUefiImageMemoryAttributes (
>>    Status = CoreGetMemorySpaceDescriptor(BaseAddress, &Descriptor);
>>    ASSERT_EFI_ERROR(Status);
>>  
>> -  FinalAttributes = (Descriptor.Attributes & CACHE_ATTRIBUTE_MASK) | (Attributes & MEMORY_ATTRIBUTE_MASK);
>> +  FinalAttributes = (Descriptor.Attributes & EFI_CACHE_ATTRIBUTE_MASK) | (Attributes & EFI_MEMORY_ATTRIBUTE_MASK);
>>  
>>    DEBUG ((DEBUG_INFO, "SetUefiImageMemoryAttributes - 0x%016lx - 0x%016lx (0x%016lx)\n", BaseAddress, Length, FinalAttributes));
>>  
>> @@ -924,7 +921,7 @@ InitializeDxeNxMemoryProtectionPolicy (
>>              (EFI_MEMORY_PRESENT | EFI_MEMORY_INITIALIZED)) {
>>  
>>          Attributes = GetPermissionAttributeForMemoryType (EfiConventionalMemory) |
>> -                     (Entry->Attributes & CACHE_ATTRIBUTE_MASK);
>> +                     (Entry->Attributes & EFI_CACHE_ATTRIBUTE_MASK);
>>  
>>          DEBUG ((DEBUG_INFO,
>>            "Untested GCD memory space region: - 0x%016lx - 0x%016lx (0x%016lx)\n",
>> diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
>> index 0385f1d..599a0cd 100644
>> --- a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
>> +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
>> @@ -39,12 +39,6 @@
>>  
>>  #define SMRAM_CAPABILITIES  (EFI_MEMORY_WB | EFI_MEMORY_UC)
>>  
>> -#define MEMORY_CACHE_ATTRIBUTES (EFI_MEMORY_UC | EFI_MEMORY_WC | \
>> -                                 EFI_MEMORY_WT | EFI_MEMORY_WB | \
>> -                                 EFI_MEMORY_WP | EFI_MEMORY_UCE)
>> -
>> -#define MEMORY_PAGE_ATTRIBUTES  (EFI_MEMORY_XP | EFI_MEMORY_RP | EFI_MEMORY_RO)
>> -
>>  //
>>  // Function prototypes from produced protocols
>>  //
>> @@ -1710,7 +1704,7 @@ SmmIplEntry (
>>      CpuArch = NULL;
>>      Status = gBS->LocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOID **)&CpuArch);
>>      if (!EFI_ERROR (Status)) {
>> -      MemDesc.Attributes &= ~(MEMORY_CACHE_ATTRIBUTES | MEMORY_PAGE_ATTRIBUTES);
>> +      MemDesc.Attributes &= ~(EFI_CACHE_ATTRIBUTE_MASK | EFI_MEMORY_ATTRIBUTE_MASK);
>>        MemDesc.Attributes |= EFI_MEMORY_WB;
>>        Status = gDS->SetMemorySpaceAttributes (
>>                        mSmramCacheBase,
>> @@ -1727,7 +1721,7 @@ SmmIplEntry (
>>                 &MemDesc
>>                 );
>>          DEBUG ((DEBUG_INFO, "SMRAM attributes: %016lx\n", MemDesc.Attributes));
>> -        ASSERT ((MemDesc.Attributes & MEMORY_PAGE_ATTRIBUTES) == 0);
>> +        ASSERT ((MemDesc.Attributes & EFI_MEMORY_ATTRIBUTE_MASK) == 0);
>>        );
>>      }
>>      //
>>
> 
> 
> 
> 

Re: [edk2-devel] [PATCH V3 3/4] UefiCpuPkg: Update EFI_MEMORY_CACHETYPE_MASK definition

[Laszlo Ersek]

Eric, Ray,

can you please ACK this patch?

Thanks
Laszlo

On 07/02/20 22:50, Oleksiy Yakovlev wrote:
> Add EFI_MEMORY_WP attribute to
> EFI_MEMORY_CACHETYPE_MASK definition.
> 
> Signed-off-by: Oleksiy Yakovlev <oleksiyy@ami.com>
> ---
>  UefiCpuPkg/CpuDxe/CpuDxe.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.h b/UefiCpuPkg/CpuDxe/CpuDxe.h
> index 9299eaa..b30a896 100644
> --- a/UefiCpuPkg/CpuDxe/CpuDxe.h
> +++ b/UefiCpuPkg/CpuDxe/CpuDxe.h
> @@ -43,7 +43,8 @@
>                                         EFI_MEMORY_WC  | \
>                                         EFI_MEMORY_WT  | \
>                                         EFI_MEMORY_WB  | \
> -                                       EFI_MEMORY_UCE   \
> +                                       EFI_MEMORY_UCE | \
> +                                       EFI_MEMORY_WP    \
>                                         )
>  
>  #define EFI_MEMORY_PAGETYPE_MASK      (EFI_MEMORY_RP  | \
> 

Re: [edk2-devel] [PATCH V3 4/4] UefiCpuPkg: Add New Memory Attributes

[Laszlo Ersek]

Eric, Ray,

can you please ACK this patch too?

Thanks
Laszlo

On 07/02/20 22:50, Oleksiy Yakovlev wrote:
> Add usage of EFI_MEMORY_SP and EFI_MEMORY_CPU_CRYPTO
> attributes introduced in UEFI 2.8.
> (UEFI 2.8, mantis 1919 and 1872).
> Use attributes bitmasks, defined in MdePkg.
> 
> Signed-off-by: Oleksiy Yakovlev <oleksiyy@ami.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  UefiCpuPkg/CpuDxe/CpuDxe.c                         | 11 ++++-------
>  UefiCpuPkg/CpuDxe/CpuDxe.h                         | 13 -------------
>  UefiCpuPkg/CpuDxe/CpuPageTable.c                   |  6 +++---
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c |  2 +-
>  4 files changed, 8 insertions(+), 24 deletions(-)
> 
> diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.c b/UefiCpuPkg/CpuDxe/CpuDxe.c
> index a571fc3..52cc26e 100644
> --- a/UefiCpuPkg/CpuDxe/CpuDxe.c
> +++ b/UefiCpuPkg/CpuDxe/CpuDxe.c
> @@ -10,9 +10,6 @@
>  #include "CpuMp.h"
>  #include "CpuPageTable.h"
>  
> -#define CACHE_ATTRIBUTE_MASK   (EFI_MEMORY_UC | EFI_MEMORY_WC | EFI_MEMORY_WT | EFI_MEMORY_WB | EFI_MEMORY_UCE | EFI_MEMORY_WP)
> -#define MEMORY_ATTRIBUTE_MASK  (EFI_MEMORY_RP | EFI_MEMORY_XP | EFI_MEMORY_RO)
> -
>  //
>  // Global Variables
>  //
> @@ -417,8 +414,8 @@ CpuSetMemoryAttributes (
>      return EFI_SUCCESS;
>    }
>  
> -  CacheAttributes = Attributes & CACHE_ATTRIBUTE_MASK;
> -  MemoryAttributes = Attributes & MEMORY_ATTRIBUTE_MASK;
> +  CacheAttributes = Attributes & EFI_CACHE_ATTRIBUTE_MASK;
> +  MemoryAttributes = Attributes & EFI_MEMORY_ATTRIBUTE_MASK;
>  
>    if (Attributes != (CacheAttributes | MemoryAttributes)) {
>      return EFI_INVALID_PARAMETER;
> @@ -677,7 +674,7 @@ SetGcdMemorySpaceAttributes (
>      gDS->SetMemorySpaceAttributes (
>             RegionStart,
>             RegionLength,
> -           (MemorySpaceMap[Index].Attributes & ~EFI_MEMORY_CACHETYPE_MASK) | (MemorySpaceMap[Index].Capabilities & Attributes)
> +           (MemorySpaceMap[Index].Attributes & ~EFI_CACHE_ATTRIBUTE_MASK) | (MemorySpaceMap[Index].Capabilities & Attributes)
>             );
>    }
>  
> @@ -754,7 +751,7 @@ RefreshMemoryAttributesFromMtrr (
>      gDS->SetMemorySpaceAttributes (
>             MemorySpaceMap[Index].BaseAddress,
>             MemorySpaceMap[Index].Length,
> -           (MemorySpaceMap[Index].Attributes & ~EFI_MEMORY_CACHETYPE_MASK) |
> +           (MemorySpaceMap[Index].Attributes & ~EFI_CACHE_ATTRIBUTE_MASK) |
>             (MemorySpaceMap[Index].Capabilities & DefaultAttributes)
>             );
>    }
> diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.h b/UefiCpuPkg/CpuDxe/CpuDxe.h
> index b30a896..9771ec8 100644
> --- a/UefiCpuPkg/CpuDxe/CpuDxe.h
> +++ b/UefiCpuPkg/CpuDxe/CpuDxe.h
> @@ -39,19 +39,6 @@
>  #include <Guid/IdleLoopEvent.h>
>  #include <Guid/VectorHandoffTable.h>
>  
> -#define EFI_MEMORY_CACHETYPE_MASK     (EFI_MEMORY_UC  | \
> -                                       EFI_MEMORY_WC  | \
> -                                       EFI_MEMORY_WT  | \
> -                                       EFI_MEMORY_WB  | \
> -                                       EFI_MEMORY_UCE | \
> -                                       EFI_MEMORY_WP    \
> -                                       )
> -
> -#define EFI_MEMORY_PAGETYPE_MASK      (EFI_MEMORY_RP  | \
> -                                       EFI_MEMORY_XP  | \
> -                                       EFI_MEMORY_RO    \
> -                                       )
> -
>  #define HEAP_GUARD_NONSTOP_MODE       \
>          ((PcdGet8 (PcdHeapGuardPropertyMask) & (BIT6|BIT4|BIT1|BIT0)) > BIT6)
>  
> diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> index 0a02cb3..06ee1b8 100644
> --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> @@ -717,7 +717,7 @@ ConvertMemoryPageAttributes (
>      return RETURN_INVALID_PARAMETER;
>    }
>  
> -  if ((Attributes & ~(EFI_MEMORY_RP | EFI_MEMORY_RO | EFI_MEMORY_XP)) != 0) {
> +  if ((Attributes & ~EFI_MEMORY_ATTRIBUTE_MASK) != 0) {
>      DEBUG ((DEBUG_ERROR, "Attributes(0x%lx) has unsupported bit\n", Attributes));
>      return EFI_UNSUPPORTED;
>    }
> @@ -1018,9 +1018,9 @@ RefreshGcdMemoryAttributesFromPaging (
>  
>        Length = MIN (PageLength, MemorySpaceLength);
>        if (Attributes != (MemorySpaceMap[Index].Attributes &
> -                         EFI_MEMORY_PAGETYPE_MASK)) {
> +                         EFI_MEMORY_ATTRIBUTE_MASK)) {
>          NewAttributes = (MemorySpaceMap[Index].Attributes &
> -                         ~EFI_MEMORY_PAGETYPE_MASK) | Attributes;
> +                         ~EFI_MEMORY_ATTRIBUTE_MASK) | Attributes;
>          Status = gDS->SetMemorySpaceAttributes (
>                          BaseAddress,
>                          Length,
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> index 9c5a92a..ebfc46a 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> @@ -435,7 +435,7 @@ ConvertMemoryPageAttributes (
>    EFI_PHYSICAL_ADDRESS              MaximumSupportMemAddress;
>  
>    ASSERT (Attributes != 0);
> -  ASSERT ((Attributes & ~(EFI_MEMORY_RP | EFI_MEMORY_RO | EFI_MEMORY_XP)) == 0);
> +  ASSERT ((Attributes & ~EFI_MEMORY_ATTRIBUTE_MASK) == 0);
>  
>    ASSERT ((BaseAddress & (SIZE_4KB - 1)) == 0);
>    ASSERT ((Length & (SIZE_4KB - 1)) == 0);
> 

Re: [edk2-devel] [PATCH V3 3/4] UefiCpuPkg: Update EFI_MEMORY_CACHETYPE_MASK definition

[Dong, Eric]

Sorry, I missed this patch serial.

Acked-by: Eric Dong <eric.dong@intel.com>

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Oleksiy
> Yakovlev
> Sent: Friday, July 3, 2020 4:51 AM
> To: devel@edk2.groups.io
> Cc: Gao, Liming <liming.gao@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Bi, Dandan <dandan.bi@intel.com>; Ni, Ray
> <ray.ni@intel.com>; lersek@redhat.com; Kumar, Rahul1
> <rahul1.kumar@intel.com>; Felixp@ami.com; oleksiyy@ami.com
> Subject: [edk2-devel] [PATCH V3 3/4] UefiCpuPkg: Update
> EFI_MEMORY_CACHETYPE_MASK definition
> 
> Add EFI_MEMORY_WP attribute to
> EFI_MEMORY_CACHETYPE_MASK definition.
> 
> Signed-off-by: Oleksiy Yakovlev <oleksiyy@ami.com>
> ---
>  UefiCpuPkg/CpuDxe/CpuDxe.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.h b/UefiCpuPkg/CpuDxe/CpuDxe.h
> index 9299eaa..b30a896 100644
> --- a/UefiCpuPkg/CpuDxe/CpuDxe.h
> +++ b/UefiCpuPkg/CpuDxe/CpuDxe.h
> @@ -43,7 +43,8 @@
>                                         EFI_MEMORY_WC  | \
>                                         EFI_MEMORY_WT  | \
>                                         EFI_MEMORY_WB  | \
> -                                       EFI_MEMORY_UCE   \
> +                                       EFI_MEMORY_UCE | \
> +                                       EFI_MEMORY_WP    \
>                                         )
> 
>  #define EFI_MEMORY_PAGETYPE_MASK      (EFI_MEMORY_RP  | \
> --
> 2.9.0.windows.1
> 
> 
> Please consider the environment before printing this email.
> 
> The information contained in this message may be confidential and
> proprietary to American Megatrends (AMI).  This communication is intended
> to be read only by the individual or entity to whom it is addressed or by their
> designee. If the reader of this message is not the intended recipient, you are
> on notice that any distribution of this message, in any form, is strictly
> prohibited.  Please promptly notify the sender by reply e-mail or by
> telephone at 770-246-8600, and then delete or destroy all copies of the
> transmission.
> 
> 

Re: [edk2-devel] [PATCH V3 4/4] UefiCpuPkg: Add New Memory Attributes

[Dong, Eric]

Reviewed-by: Eric Dong <eric.dong@intel.com>

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Oleksiy
> Yakovlev
> Sent: Friday, July 3, 2020 4:51 AM
> To: devel@edk2.groups.io
> Cc: Gao, Liming <liming.gao@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Bi, Dandan <dandan.bi@intel.com>; Ni, Ray
> <ray.ni@intel.com>; lersek@redhat.com; Kumar, Rahul1
> <rahul1.kumar@intel.com>; Felixp@ami.com; oleksiyy@ami.com
> Subject: [edk2-devel] [PATCH V3 4/4] UefiCpuPkg: Add New Memory
> Attributes
> 
> Add usage of EFI_MEMORY_SP and EFI_MEMORY_CPU_CRYPTO attributes
> introduced in UEFI 2.8.
> (UEFI 2.8, mantis 1919 and 1872).
> Use attributes bitmasks, defined in MdePkg.
> 
> Signed-off-by: Oleksiy Yakovlev <oleksiyy@ami.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  UefiCpuPkg/CpuDxe/CpuDxe.c                         | 11 ++++-------
>  UefiCpuPkg/CpuDxe/CpuDxe.h                         | 13 -------------
>  UefiCpuPkg/CpuDxe/CpuPageTable.c                   |  6 +++---
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c |  2 +-
>  4 files changed, 8 insertions(+), 24 deletions(-)
> 
> diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.c b/UefiCpuPkg/CpuDxe/CpuDxe.c
> index a571fc3..52cc26e 100644
> --- a/UefiCpuPkg/CpuDxe/CpuDxe.c
> +++ b/UefiCpuPkg/CpuDxe/CpuDxe.c
> @@ -10,9 +10,6 @@
>  #include "CpuMp.h"
>  #include "CpuPageTable.h"
> 
> -#define CACHE_ATTRIBUTE_MASK   (EFI_MEMORY_UC | EFI_MEMORY_WC
> | EFI_MEMORY_WT | EFI_MEMORY_WB | EFI_MEMORY_UCE |
> EFI_MEMORY_WP)
> -#define MEMORY_ATTRIBUTE_MASK  (EFI_MEMORY_RP |
> EFI_MEMORY_XP | EFI_MEMORY_RO)
> -
>  //
>  // Global Variables
>  //
> @@ -417,8 +414,8 @@ CpuSetMemoryAttributes (
>      return EFI_SUCCESS;
>    }
> 
> -  CacheAttributes = Attributes & CACHE_ATTRIBUTE_MASK;
> -  MemoryAttributes = Attributes & MEMORY_ATTRIBUTE_MASK;
> +  CacheAttributes = Attributes & EFI_CACHE_ATTRIBUTE_MASK;
> + MemoryAttributes = Attributes & EFI_MEMORY_ATTRIBUTE_MASK;
> 
>    if (Attributes != (CacheAttributes | MemoryAttributes)) {
>      return EFI_INVALID_PARAMETER;
> @@ -677,7 +674,7 @@ SetGcdMemorySpaceAttributes (
>      gDS->SetMemorySpaceAttributes (
>             RegionStart,
>             RegionLength,
> -           (MemorySpaceMap[Index].Attributes &
> ~EFI_MEMORY_CACHETYPE_MASK) |
> (MemorySpaceMap[Index].Capabilities & Attributes)
> +           (MemorySpaceMap[Index].Attributes &
> + ~EFI_CACHE_ATTRIBUTE_MASK) | (MemorySpaceMap[Index].Capabilities
> &
> + Attributes)
>             );
>    }
> 
> @@ -754,7 +751,7 @@ RefreshMemoryAttributesFromMtrr (
>      gDS->SetMemorySpaceAttributes (
>             MemorySpaceMap[Index].BaseAddress,
>             MemorySpaceMap[Index].Length,
> -           (MemorySpaceMap[Index].Attributes &
> ~EFI_MEMORY_CACHETYPE_MASK) |
> +           (MemorySpaceMap[Index].Attributes &
> + ~EFI_CACHE_ATTRIBUTE_MASK) |
>             (MemorySpaceMap[Index].Capabilities & DefaultAttributes)
>             );
>    }
> diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.h b/UefiCpuPkg/CpuDxe/CpuDxe.h
> index b30a896..9771ec8 100644
> --- a/UefiCpuPkg/CpuDxe/CpuDxe.h
> +++ b/UefiCpuPkg/CpuDxe/CpuDxe.h
> @@ -39,19 +39,6 @@
>  #include <Guid/IdleLoopEvent.h>
>  #include <Guid/VectorHandoffTable.h>
> 
> -#define EFI_MEMORY_CACHETYPE_MASK     (EFI_MEMORY_UC  | \
> -                                       EFI_MEMORY_WC  | \
> -                                       EFI_MEMORY_WT  | \
> -                                       EFI_MEMORY_WB  | \
> -                                       EFI_MEMORY_UCE | \
> -                                       EFI_MEMORY_WP    \
> -                                       )
> -
> -#define EFI_MEMORY_PAGETYPE_MASK      (EFI_MEMORY_RP  | \
> -                                       EFI_MEMORY_XP  | \
> -                                       EFI_MEMORY_RO    \
> -                                       )
> -
>  #define HEAP_GUARD_NONSTOP_MODE       \
>          ((PcdGet8 (PcdHeapGuardPropertyMask) & (BIT6|BIT4|BIT1|BIT0)) >
> BIT6)
> 
> diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> index 0a02cb3..06ee1b8 100644
> --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> @@ -717,7 +717,7 @@ ConvertMemoryPageAttributes (
>      return RETURN_INVALID_PARAMETER;
>    }
> 
> -  if ((Attributes & ~(EFI_MEMORY_RP | EFI_MEMORY_RO |
> EFI_MEMORY_XP)) != 0) {
> +  if ((Attributes & ~EFI_MEMORY_ATTRIBUTE_MASK) != 0) {
>      DEBUG ((DEBUG_ERROR, "Attributes(0x%lx) has unsupported bit\n",
> Attributes));
>      return EFI_UNSUPPORTED;
>    }
> @@ -1018,9 +1018,9 @@ RefreshGcdMemoryAttributesFromPaging (
> 
>        Length = MIN (PageLength, MemorySpaceLength);
>        if (Attributes != (MemorySpaceMap[Index].Attributes &
> -                         EFI_MEMORY_PAGETYPE_MASK)) {
> +                         EFI_MEMORY_ATTRIBUTE_MASK)) {
>          NewAttributes = (MemorySpaceMap[Index].Attributes &
> -                         ~EFI_MEMORY_PAGETYPE_MASK) | Attributes;
> +                         ~EFI_MEMORY_ATTRIBUTE_MASK) | Attributes;
>          Status = gDS->SetMemorySpaceAttributes (
>                          BaseAddress,
>                          Length,
> diff --git
> a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> index 9c5a92a..ebfc46a 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> @@ -435,7 +435,7 @@ ConvertMemoryPageAttributes (
>    EFI_PHYSICAL_ADDRESS              MaximumSupportMemAddress;
> 
>    ASSERT (Attributes != 0);
> -  ASSERT ((Attributes & ~(EFI_MEMORY_RP | EFI_MEMORY_RO |
> EFI_MEMORY_XP)) == 0);
> +  ASSERT ((Attributes & ~EFI_MEMORY_ATTRIBUTE_MASK) == 0);
> 
>    ASSERT ((BaseAddress & (SIZE_4KB - 1)) == 0);
>    ASSERT ((Length & (SIZE_4KB - 1)) == 0);
> --
> 2.9.0.windows.1
> 
> 
> Please consider the environment before printing this email.
> 
> The information contained in this message may be confidential and
> proprietary to American Megatrends (AMI).  This communication is intended
> to be read only by the individual or entity to whom it is addressed or by their
> designee. If the reader of this message is not the intended recipient, you are
> on notice that any distribution of this message, in any form, is strictly
> prohibited.  Please promptly notify the sender by reply e-mail or by
> telephone at 770-246-8600, and then delete or destroy all copies of the
> transmission.
> 
> 

Re: [edk2-devel] [PATCH V3 0/4] Add New Memory Attributes

[Laszlo Ersek]

On 07/02/20 22:50, Oleksiy Yakovlev wrote:
> This series of patches add usage of new memory
> attributes EFI_MEMORY_SP and EFI_MEMORY_CPU_CRYPTO,
> introduced in UEFI2.8 (mantis 1919 and 1872).
> First patch fix typos in description and introduce two 
> bitmasks for all memory type attributes.
> Second and fourth patches get rid of multiple memory attributes
> bitmasks definitions trough multiple files and headers,
> and replace them with new common definitions from MdePkg.
> Third patch includes WP attribute into cache type mask in
> CpuDexe.h to make next change in this file more clear.
> 
> Oleksiy Yakovlev (4):
>   MdePkg: Add New Memory Attributes
>   MdeModulePkg: Add New Memory Attributes
>   UefiCpuPkg: Update EFI_MEMORY_CACHETYPE_MASK definition
>   UefiCpuPkg: Add New Memory Attributes
> 
>  MdeModulePkg/Core/Dxe/Gcd/Gcd.c                    | 11 ++---------
>  MdeModulePkg/Core/Dxe/Mem/Page.c                   |  9 +++------
>  MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c      |  7 ++-----
>  MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c             | 10 ++--------
>  MdePkg/Include/Uefi/UefiSpec.h                     | 10 ++++++++--
>  UefiCpuPkg/CpuDxe/CpuDxe.c                         | 11 ++++-------
>  UefiCpuPkg/CpuDxe/CpuDxe.h                         | 12 ------------
>  UefiCpuPkg/CpuDxe/CpuPageTable.c                   |  6 +++---
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c |  2 +-
>  9 files changed, 25 insertions(+), 53 deletions(-)
> 

Merged as commit range 5d29e2d020e8..e77966b341b9, via
<https://github.com/tianocore/edk2/pull/796>.

Thanks
Laszlo

Re: [edk2-devel] [PATCH V3 0/4] Add New Memory Attributes

[Dhaval Sharma]

[-- Attachment #1: Type: text/plain, Size: 1474 bytes --]

Hi,
I wanted to revisit this thread and I am maintaining the context as there are a lot of details already mentioned here regarding EFI_MEMORY_SP.
Other than what has been addressed here, we also would like to have an option in edk2 to *avoid* using this type of memory for its own purposes. This seems like one of the motivations for original request and is being honored by OS today but not edk2 as it does not have any specific implementation today which takes this attribute into consideration.
I would like to add PCD based implementation which informs edk2 NOT to use this memory for its own purposes and leave it alone (as still available memory to OS).

Specific-purpose memory (SPM). The memory is earmarked for
specific purposes such as for specific device drivers or applications.
The SPM attribute serves as a hint to the OS to avoid allocating this
memory for core OS data or code that can not be relocated.
Prolonged use of this memory for purposes other than the intended
purpose may result in suboptimal platform performance.

Some more context:
https://lwn.net/Articles/784971/


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111337): https://edk2.groups.io/g/devel/message/111337
Mute This Topic: https://groups.io/mt/75267363/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 3169 bytes --]

Re: [edk2-devel] [PATCH V3 0/4] Add New Memory Attributes

[Laszlo Ersek]

On 11/17/23 09:07, Dhaval Sharma wrote:
> Hi,
> I wanted to revisit this thread and I am maintaining the context as
> there are a lot of details already mentioned here regarding EFI_MEMORY_SP.
> Other than what has been addressed here, we also would like to have an
> option in edk2 to *avoid* using this type of memory for its own
> purposes. This seems like one of the motivations for original request
> and is being honored by OS today but not edk2 as it does not have any
> specific implementation today which takes this attribute into consideration.
> I would like to add PCD based implementation which informs edk2 NOT to
> use this memory for its own purposes and leave it alone (as still
> available memory to OS).
> 
> Specific-purpose memory (SPM). The memory is earmarked for
> specific purposes such as for specific device drivers or applications.
> The SPM attribute serves as a hint to the OS to avoid allocating this
> memory for core OS data or code that can not be relocated.
> Prolonged use of this memory for purposes other than the intended
> purpose may result in suboptimal platform performance.
> 
> Some more context:
> https://lwn.net/Articles/784971/

Why was EFI_MEMORY_SP introduced as a memory attribute, rather than its
own memory type?

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111339): https://edk2.groups.io/g/devel/message/111339
Mute This Topic: https://groups.io/mt/75267363/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V3 0/4] Add New Memory Attributes

[Dhaval Sharma]

[-- Attachment #1: Type: text/plain, Size: 2104 bytes --]

Did you mean to add it as a memory type by itself like
EFI_MEMORY_TYPE_INFORMATION?
My interpretation of Memory Type is that it is more of SW usability
construct while Memory Attr is more of HW behavioural construct. Together
they define how a memory region can be used.

On Fri, Nov 17, 2023 at 1:55 PM Laszlo Ersek <lersek@redhat.com> wrote:

> On 11/17/23 09:07, Dhaval Sharma wrote:
> > Hi,
> > I wanted to revisit this thread and I am maintaining the context as
> > there are a lot of details already mentioned here
> regarding EFI_MEMORY_SP.
> > Other than what has been addressed here, we also would like to have an
> > option in edk2 to *avoid* using this type of memory for its own
> > purposes. This seems like one of the motivations for original request
> > and is being honored by OS today but not edk2 as it does not have any
> > specific implementation today which takes this attribute into
> consideration.
> > I would like to add PCD based implementation which informs edk2 NOT to
> > use this memory for its own purposes and leave it alone (as still
> > available memory to OS).
> >
> > Specific-purpose memory (SPM). The memory is earmarked for
> > specific purposes such as for specific device drivers or applications.
> > The SPM attribute serves as a hint to the OS to avoid allocating this
> > memory for core OS data or code that can not be relocated.
> > Prolonged use of this memory for purposes other than the intended
> > purpose may result in suboptimal platform performance.
> >
> > Some more context:
> > https://lwn.net/Articles/784971/
>
> Why was EFI_MEMORY_SP introduced as a memory attribute, rather than its
> own memory type?
>
> Laszlo
>
>

-- 
Thanks!
=D


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111457): https://edk2.groups.io/g/devel/message/111457
Mute This Topic: https://groups.io/mt/75267363/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 3139 bytes --]

Re: [edk2-devel] [PATCH V3 0/4] Add New Memory Attributes

[Laszlo Ersek]

On 11/20/23 05:59, Dhaval Sharma wrote:
> Did you mean to add it as a memory type by itself like
> EFI_MEMORY_TYPE_INFORMATION?

Yes, a new enum constant for EFI_MEMORY_TYPE.

(It wasn't a "loaded question", just wanted to understand the reasoning.)

> My interpretation of Memory Type is that it is more of SW usability
> construct while Memory Attr is more of HW behavioural construct.
> Together they define how a memory region can be used.

I guess it's hard to tell apart. The SPM definition (visible in the
context below, from your earlier email) does seem like a software
usability construct ("avoid allocating for ...").

But, I truly don't know. I guess I was only trying to gauge if I could
be a useful reviewer for this series; probably not.

Thanks!
Laszlo

> 
> On Fri, Nov 17, 2023 at 1:55 PM Laszlo Ersek <lersek@redhat.com
> <mailto:lersek@redhat.com>> wrote:
> 
>     On 11/17/23 09:07, Dhaval Sharma wrote:
>     > Hi,
>     > I wanted to revisit this thread and I am maintaining the context as
>     > there are a lot of details already mentioned here
>     regarding EFI_MEMORY_SP.
>     > Other than what has been addressed here, we also would like to have an
>     > option in edk2 to *avoid* using this type of memory for its own
>     > purposes. This seems like one of the motivations for original request
>     > and is being honored by OS today but not edk2 as it does not have any
>     > specific implementation today which takes this attribute into
>     consideration.
>     > I would like to add PCD based implementation which informs edk2 NOT to
>     > use this memory for its own purposes and leave it alone (as still
>     > available memory to OS).
>     >
>     > Specific-purpose memory (SPM). The memory is earmarked for
>     > specific purposes such as for specific device drivers or applications.
>     > The SPM attribute serves as a hint to the OS to avoid allocating this
>     > memory for core OS data or code that can not be relocated.
>     > Prolonged use of this memory for purposes other than the intended
>     > purpose may result in suboptimal platform performance.
>     >
>     > Some more context:
>     > https://lwn.net/Articles/784971/ <https://lwn.net/Articles/784971/>
> 
>     Why was EFI_MEMORY_SP introduced as a memory attribute, rather than its
>     own memory type?
> 
>     Laszlo
> 
> 
> 
> -- 
> Thanks!
> =D



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111553): https://edk2.groups.io/g/devel/message/111553
Mute This Topic: https://groups.io/mt/75267363/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-