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

["Dong, Eric" <eric.dong@intel.com>]

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.
> 
>