Re: [Patch V4 15/15] UefiCpuPkg/PiSmmCpuDxeSmm: Remove unnecessary function

["Ni, Ray" <ray.ni@intel.com>]

Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: Tan, Dun <dun.tan@intel.com>
> Sent: Tuesday, May 16, 2023 6:00 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul
> R <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>
> Subject: [Patch V4 15/15] UefiCpuPkg/PiSmmCpuDxeSmm: Remove unnecessary
> function
> 
> Remove unnecessary function SetNotPresentPage(). We can directly
> use ConvertMemoryPageAttributes to set a range to non-present.
> 
> Signed-off-by: Dun Tan <dun.tan@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c         |  8 ++++++--
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h         | 16 ----------------
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 22 ------------
> ----------
>  3 files changed, 6 insertions(+), 40 deletions(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> index d69e976269..7fa1867b63 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> @@ -1074,10 +1074,14 @@ PiCpuSmmEntry (
>          mSmmShadowStackSize
>          );
>        if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
> -        SetNotPresentPage (
> +        ConvertMemoryPageAttributes (
>            Cr3,
> +          mPagingMode,
>            (EFI_PHYSICAL_ADDRESS)(UINTN)Stacks + mSmmStackSize +
> EFI_PAGES_TO_SIZE (1) + (mSmmStackSize + mSmmShadowStackSize) * Index,
> -          EFI_PAGES_TO_SIZE (1)
> +          EFI_PAGES_TO_SIZE (1),
> +          EFI_MEMORY_RP,
> +          TRUE,
> +          NULL
>            );
>        }
>      }
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> index 12ad86028e..0dc4d758cc 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> @@ -1247,22 +1247,6 @@ SetShadowStack (
>    IN  UINT64                Length
>    );
> 
> -/**
> -  Set not present memory.
> -
> -  @param[in]  Cr3              The page table base address.
> -  @param[in]  BaseAddress      The physical address that is the start address of a
> memory region.
> -  @param[in]  Length           The size in bytes of the memory region.
> -
> -  @retval EFI_SUCCESS           The not present memory is set.
> -**/
> -EFI_STATUS
> -SetNotPresentPage (
> -  IN  UINTN                 Cr3,
> -  IN  EFI_PHYSICAL_ADDRESS  BaseAddress,
> -  IN  UINT64                Length
> -  );
> -
>  /**
>    Initialize the shadow stack related data structure.
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> index 138ff43c9d..95de472ebf 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> @@ -752,28 +752,6 @@ SetShadowStack (
>    return Status;
>  }
> 
> -/**
> -  Set not present memory.
> -
> -  @param[in]  Cr3              The page table base address.
> -  @param[in]  BaseAddress      The physical address that is the start address of a
> memory region.
> -  @param[in]  Length           The size in bytes of the memory region.
> -
> -  @retval EFI_SUCCESS           The not present memory is set.
> -**/
> -EFI_STATUS
> -SetNotPresentPage (
> -  IN  UINTN                 Cr3,
> -  IN  EFI_PHYSICAL_ADDRESS  BaseAddress,
> -  IN  UINT64                Length
> -  )
> -{
> -  EFI_STATUS  Status;
> -
> -  Status = SmmSetMemoryAttributesEx (Cr3, mPagingMode, BaseAddress,
> Length, EFI_MEMORY_RP);
> -  return Status;
> -}
> -
>  /**
>    Retrieves a pointer to the system configuration table from the SMM System
> Table
>    based on a specified GUID.
> --
> 2.31.1.windows.1