Re: [PATCH v6 5/6] UefiCpuPkg/SmmCpuFeaturesLib: Skip SMBASE configuration

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

I remember I already provided the R-B.
Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: Wu, Jiaxin <jiaxin.wu@intel.com>
> Sent: Monday, February 13, 2023 4:44 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng, Star
> <star.zeng@intel.com>; Laszlo Ersek <lersek@redhat.com>; Gerd Hoffmann
> <kraxel@redhat.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>
> Subject: [PATCH v6 5/6] UefiCpuPkg/SmmCpuFeaturesLib: Skip SMBASE
> configuration
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4337
> 
> This patch is to avoid configure SMBASE if SmBase relocation has been
> done. If gSmmBaseHobGuid found, means SmBase info has been relocated
> and recorded in the SmBase array. No need to do the relocation in
> SmmCpuFeaturesInitializeProcessor().
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Zeng Star <star.zeng@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
> ---
>  .../Library/SmmCpuFeaturesLib/CpuFeaturesLib.h     |  2 ++
>  .../SmmCpuFeaturesLib/IntelSmmCpuFeaturesLib.c     | 25
> ++++++++++++++++++----
>  .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf        |  6 +++++-
>  .../SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf     |  3 ++-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c      |  3 +--
>  .../StandaloneMmCpuFeaturesLib.inf                 |  6 +++++-
>  6 files changed, 36 insertions(+), 9 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
> b/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
> index fd3e902547..c2e4fbe96b 100644
> --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
> @@ -7,15 +7,17 @@
>  **/
> 
>  #ifndef CPU_FEATURES_LIB_H_
>  #define CPU_FEATURES_LIB_H_
> 
> +#include <Guid/SmmBaseHob.h>
>  #include <Library/SmmCpuFeaturesLib.h>
>  #include <Library/BaseLib.h>
>  #include <Library/PcdLib.h>
>  #include <Library/MemoryAllocationLib.h>
>  #include <Library/DebugLib.h>
> +#include <Library/HobLib.h>
> 
>  /**
>    Performs library initialization.
> 
>    This initialization function contains common functionality shared betwen all
> diff --git
> a/UefiCpuPkg/Library/SmmCpuFeaturesLib/IntelSmmCpuFeaturesLib.c
> b/UefiCpuPkg/Library/SmmCpuFeaturesLib/IntelSmmCpuFeaturesLib.c
> index d5eaaa7a99..1a2c706fa1 100644
> --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/IntelSmmCpuFeaturesLib.c
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/IntelSmmCpuFeaturesLib.c
> @@ -1,9 +1,9 @@
>  /** @file
>  Implementation shared across all library instances.
> 
> -Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2010 - 2023, Intel Corporation. All rights reserved.<BR>
>  Copyright (c) Microsoft Corporation.<BR>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> 
> @@ -36,10 +36,16 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  // Set default value to assume IA-32 Architectural MSRs are used
>  //
>  UINT32  mSmrrPhysBaseMsr = SMM_FEATURES_LIB_IA32_SMRR_PHYSBASE;
>  UINT32  mSmrrPhysMaskMsr =
> SMM_FEATURES_LIB_IA32_SMRR_PHYSMASK;
> 
> +//
> +// Indicate SmBase for each Processors has been relocated or not. If TRUE,
> +// means no need to do the relocation in
> SmmCpuFeaturesInitializeProcessor().
> +//
> +BOOLEAN  mSmmCpuFeaturesSmmRelocated;
> +
>  //
>  // Set default value to assume MTRRs need to be configured on each SMI
>  //
>  BOOLEAN  mNeedConfigureMtrrs = TRUE;
> 
> @@ -142,10 +148,16 @@ CpuFeaturesLibInitialization (
>    //
>    // Allocate array for state of SMRR enable on all CPUs
>    //
>    mSmrrEnabled = (BOOLEAN *)AllocatePool (sizeof (BOOLEAN) *
> GetCpuMaxLogicalProcessorNumber ());
>    ASSERT (mSmrrEnabled != NULL);
> +
> +  //
> +  // If gSmmBaseHobGuid found, means SmBase info has been relocated
> and recorded
> +  // in the SmBase array.
> +  //
> +  mSmmCpuFeaturesSmmRelocated = (BOOLEAN)(GetFirstGuidHob
> (&gSmmBaseHobGuid) != NULL);
>  }
> 
>  /**
>    Called during the very first SMI into System Management Mode to initialize
>    CPU features, including SMBASE, for the currently executing CPU.  Since
> this
> @@ -185,14 +197,19 @@ SmmCpuFeaturesInitializeProcessor (
>    UINT32                RegEdx;
>    UINTN                 FamilyId;
>    UINTN                 ModelId;
> 
>    //
> -  // Configure SMBASE.
> +  // No need to configure SMBASE if SmBase relocation has been done.
>    //
> -  CpuState             = (SMRAM_SAVE_STATE_MAP
> *)(UINTN)(SMM_DEFAULT_SMBASE + SMRAM_SAVE_STATE_MAP_OFFSET);
> -  CpuState->x86.SMBASE = (UINT32)CpuHotPlugData->SmBase[CpuIndex];
> +  if (!mSmmCpuFeaturesSmmRelocated) {
> +    //
> +    // Configure SMBASE.
> +    //
> +    CpuState             = (SMRAM_SAVE_STATE_MAP
> *)(UINTN)(SMM_DEFAULT_SMBASE + SMRAM_SAVE_STATE_MAP_OFFSET);
> +    CpuState->x86.SMBASE = (UINT32)CpuHotPlugData->SmBase[CpuIndex];
> +  }
> 
>    //
>    // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
>    // Volume 3C, Section 35.2 MSRs in the Intel(R) Core(TM) 2 Processor
> Family
>    //
> diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
> b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
> index 9ac7dde78f..46ae2bf85e 100644
> --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
> @@ -1,9 +1,9 @@
>  ## @file
>  #  The CPU specific programming for PiSmmCpuDxeSmm module.
>  #
> -#  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
>  ##
> 
>  [Defines]
> @@ -31,10 +31,14 @@
>  [LibraryClasses]
>    BaseLib
>    PcdLib
>    MemoryAllocationLib
>    DebugLib
> +  HobLib
> +
> +[Guids]
> +  gSmmBaseHobGuid                ## CONSUMES
> 
>  [Pcd]
>    gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber        ##
> SOMETIMES_CONSUMES
> 
>  [FeaturePcd]
> diff --git
> a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
> b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
> index 86d367e0a0..51322ff189 100644
> --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
> @@ -1,10 +1,10 @@
>  ## @file
>  #  The CPU specific programming for PiSmmCpuDxeSmm module when STM
> support
>  #  is included.
>  #
> -#  Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
>  ##
> 
>  [Defines]
> @@ -62,10 +62,11 @@
> 
>  [Guids]
>    gMsegSmramGuid                           ## SOMETIMES_CONSUMES ## HOB
>    gEfiAcpi20TableGuid                      ## SOMETIMES_CONSUMES ##
> SystemTable
>    gEfiAcpi10TableGuid                      ## SOMETIMES_CONSUMES ##
> SystemTable
> +  gSmmBaseHobGuid                          ## CONSUMES
> 
>  [Pcd]
>    gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber        ##
> SOMETIMES_CONSUMES
>    gUefiCpuPkgTokenSpaceGuid.PcdCpuMsegSize                         ##
> SOMETIMES_CONSUMES
>    gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStmExceptionStackSize         ##
> SOMETIMES_CONSUMES
> diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c
> b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c
> index 3cf162ada0..6cb1c515c0 100644
> --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c
> @@ -1,16 +1,15 @@
>  /** @file
>    SMM STM support functions
> 
> -  Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2015 - 2023, Intel Corporation. All rights reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> 
>  #include <PiMm.h>
>  #include <Library/BaseMemoryLib.h>
> -#include <Library/HobLib.h>
>  #include <Library/UefiBootServicesTableLib.h>
>  #include <Library/SmmServicesTableLib.h>
>  #include <Library/TpmMeasurementLib.h>
>  #include <Register/Intel/Cpuid.h>
>  #include <Register/Intel/ArchitecturalMsr.h>
> diff --git
> a/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.i
> nf
> b/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.i
> nf
> index b1f60a5505..c836939d33 100644
> ---
> a/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.i
> nf
> +++
> b/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.i
> nf
> @@ -1,9 +1,9 @@
>  ## @file
>  #  Standalone MM CPU specific programming.
>  #
> -#  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>
>  #  Copyright (c) Microsoft Corporation.<BR>
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
>  ##
> 
> @@ -32,10 +32,14 @@
>  [LibraryClasses]
>    BaseLib
>    DebugLib
>    MemoryAllocationLib
>    PcdLib
> +  HobLib
> +
> +[Guids]
> +  gSmmBaseHobGuid                ## CONSUMES
> 
>  [FixedPcd]
>    gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber        ##
> SOMETIMES_CONSUMES
> 
>  [FeaturePcd]
> --
> 2.16.2.windows.1