Re: [PATCH 3/5] SimicsOpenBoardPkg: Use SmmAccessLib instead of SmmAccessPei.inf

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

Can you fix some typos in the commit message?
With that, Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: Liu, Zhiguang <zhiguang.liu@intel.com>
> Sent: Tuesday, April 25, 2023 3:03 PM
> To: devel@edk2.groups.io
> Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Ni, Ray <ray.ni@intel.com>
> Subject: [PATCH 3/5] SimicsOpenBoardPkg: Use SmmAccessLib instead of
> SmmAccessPei.inf
> 
> SmmAccessPei.inf is a PEIM we should deleted, here is the reason:
> 1. It programs registers MCH_TOLUD to set the Low Usable DRAM,
> but reading LMCH_TOLUD always return zere in QSP platforms
> 2. It programs/reads MCH_TSEGMB to implemte some Smm Access service
> such as open/close/lock. However, this reading LMCH_TOLUD also always
> return zere in QSP platforms
> 3. It returns the hard-code Smm range information. However, there are
> two improper things about this. One is that we already have the hard
> code value about T-Seg base/size in MemDetect. The other Smm range
> informaton is already saved in gEfiSmmSmramMemoryGuid Hob. No need
> hard-code value.
> 
> So, this patch uses another way, calling PeiInstallSmmAccessPpi from
> SmmAccessLib. The lib instance we choose will use the
> gEfiSmmSmramMemoryGuid Hob information.
> In a word, with the patch, we can avoid additional hard-code, and
> avoid programing unimplemented registers.
> 
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
> ---
>  .../SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc    | 7 +------
>  .../SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.fdf    | 1 -
>  Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c  | 9
> +++++++++
>  .../Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf     | 2 ++
>  4 files changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git
> a/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc
> b/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc
> index 7b98baf764..fcae343146 100644
> ---
> a/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc
> +++
> b/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc
> @@ -142,6 +142,7 @@
>    # Silicon Package
>    #####################################
> 
> ReportCpuHobLib|IntelSiliconPkg/Library/ReportCpuHobLib/ReportCpuHobLi
> b.inf
> +
> SmmAccessLib|IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLi
> b/PeiSmmAccessLib.inf
> 
>    #####################################
>    # Platform Package
> @@ -190,12 +191,6 @@
>    #######################################
>    # Silicon Initialization Package
>    #######################################
> -!if gMinPlatformPkgTokenSpaceGuid.PcdBootToShellOnly == FALSE
> -  $(SKT_PKG)/Smm/Access/SmmAccessPei.inf {
> -    <LibraryClasses>
> -      PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
> -  }
> -!endif
> 
>    #####################################
>    # Platform Package
> diff --git
> a/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.fdf
> b/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.fdf
> index 221706ae03..844f9b6dcf 100644
> --- a/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.fdf
> +++
> b/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.fdf
> @@ -165,7 +165,6 @@ INF
> MinPlatformPkg/PlatformInit/SiliconPolicyPei/SiliconPolicyPeiPostMem.inf
>  !include MinPlatformPkg/Include/Fdf/CoreSecurityPostMemoryInclude.fdf
> 
>  INF  UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
> -INF  $(SKT_PKG)/Smm/Access/SmmAccessPei.inf
>  # S3 SMM PEI driver
>  #INF  UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationPei.inf
> 
> diff --git a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
> b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
> index 13ee415f40..f9a5487365 100644
> --- a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
> +++ b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
> @@ -25,6 +25,7 @@
>  #include <Library/CmosAccessLib.h>
>  #include <SimicsPlatforms.h>
>  #include <Guid/SmramMemoryReserve.h>
> +#include <Library/SmmAccessLib.h>
> 
>  #include <CmosMap.h>
> 
> @@ -472,6 +473,8 @@ InitializeRamRegions (
>    VOID
>    )
>  {
> +  EFI_STATUS     Status;
> +
>    QemuInitializeRam ();
> 
>    if (mS3Supported && mBootMode != BOOT_ON_S3_RESUME) {
> @@ -544,4 +547,10 @@ InitializeRamRegions (
>          );
>      }
>    }
> +
> +  //
> +  // Install EFI_PEI_MM_ACCESS_PPI for S3 resume case
> +  //
> +  Status = PeiInstallSmmAccessPpi ();
> +  ASSERT_EFI_ERROR (Status);
>  }
> diff --git a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf
> b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf
> index 618ad4075f..cdc30ad582 100644
> --- a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf
> +++ b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf
> @@ -36,6 +36,7 @@
>    SimicsX58SktPkg/SktPkg.dec
>    SimicsIch10Pkg/Ich10Pkg.dec
>    BoardModulePkg/BoardModulePkg.dec
> +  IntelSiliconPkg/IntelSiliconPkg.dec
> 
>  [Guids]
>    gEfiMemoryTypeInformationGuid
> @@ -55,6 +56,7 @@
>    MtrrLib
>    PcdLib
>    CmosAccessLib
> +  SmmAccessLib
> 
>  [Pcd]
>    gSimicsOpenBoardPkgTokenSpaceGuid.PcdSimicsPeiMemFvBase
> --
> 2.31.1.windows.1