Re: [PATCH v1 1/5] UefiCpuPkg/SmmCpuFeaturesLib: Restructure arch-dependent code

["Chang, Abner" <abner.chang@amd.com>]

[AMD Official Use Only - General]

Reviewed-by: Abner Chang <abner.chang@amd.com>

> -----Original Message-----
> From: Abdul Lateef Attar <abdattar@amd.com>
> Sent: Tuesday, December 6, 2022 9:23 PM
> To: devel@edk2.groups.io
> Cc: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>; Chang,
> Abner <Abner.Chang@amd.com>; Kirkendall, Garrett
> <Garrett.Kirkendall@amd.com>; Grimes, Paul <Paul.Grimes@amd.com>; Eric
> Dong <eric.dong@intel.com>; Ray Ni <ray.ni@intel.com>; Rahul Kumar
> <rahul1.kumar@intel.com>
> Subject: [PATCH v1 1/5] UefiCpuPkg/SmmCpuFeaturesLib: Restructure arch-
> dependent code
> 
> From: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4182
> 
> moves Intel-specific code to the arch-dependent file.
> Other processor families might have different implementation of these
> functions.
> Hence, moving out of the common file.
> 
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Garrett Kirkendall <garrett.kirkendall@amd.com>
> Cc: Paul Grimes <paul.grimes@amd.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Signed-off-by: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>
> ---
>  .../IntelSmmCpuFeaturesLib.c                  | 140 ++++++++++++++++++
>  .../SmmCpuFeaturesLibCommon.c                 | 140 ------------------
>  2 files changed, 140 insertions(+), 140 deletions(-)
> 
> diff --git
> a/UefiCpuPkg/Library/SmmCpuFeaturesLib/IntelSmmCpuFeaturesLib.c
> b/UefiCpuPkg/Library/SmmCpuFeaturesLib/IntelSmmCpuFeaturesLib.c
> index d5eaaa7a991e..994267f393b3 100644
> --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/IntelSmmCpuFeaturesLib.c
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/IntelSmmCpuFeaturesLib.c
> @@ -400,3 +400,143 @@ SmmCpuFeaturesSetSmmRegister (
>      AsmWriteMsr64 (SMM_FEATURES_LIB_SMM_FEATURE_CONTROL,
> Value);   } }++/**+  This function updates the SMRAM save state on the
> currently executing CPU+  to resume execution at a specific address after an
> RSM instruction.  This+  function must evaluate the SMRAM save state to
> determine the execution mode+  the RSM instruction resumes and update
> the resume execution address with+  either NewInstructionPointer32 or
> NewInstructionPoint.  The auto HALT restart+  flag in the SMRAM save state
> must always be cleared.  This function returns+  the value of the instruction
> pointer from the SMRAM save state that was+  replaced.  If this function
> returns 0, then the SMRAM save state was not+  modified.++  This function is
> called during the very first SMI on each CPU after+
> SmmCpuFeaturesInitializeProcessor() to set a flag in normal execution
> mode+  to signal that the SMBASE of each CPU has been updated before the
> default+  SMBASE address is used for the first SMI to the next CPU.++
> @param[in] CpuIndex                 The index of the CPU to hook.  The value+
> must be between 0 and the NumberOfCpus+                                      field in the
> System Management System Table+                                      (SMST).+  @param[in]
> CpuState                 Pointer to SMRAM Save State Map for the+
> currently executing CPU.+  @param[in] NewInstructionPointer32  Instruction
> pointer to use if resuming to+                                      32-bit execution mode from
> 64-bit SMM.+  @param[in] NewInstructionPointer    Instruction pointer to
> use if resuming to+                                      same execution mode as SMM.++
> @retval 0    This function did modify the SMRAM save state.+  @retval > 0
> The original instruction pointer value from the SMRAM save state+
> before it was
> replaced.+**/+UINT64+EFIAPI+SmmCpuFeaturesHookReturnFromSmm (+
> IN UINTN                 CpuIndex,+  IN SMRAM_SAVE_STATE_MAP  *CpuState,+
> IN UINT64                NewInstructionPointer32,+  IN UINT64
> NewInstructionPointer+  )+{+  return 0;+}++/**+  Read an SMM Save State
> register on the target processor.  If this function+  returns
> EFI_UNSUPPORTED, then the caller is responsible for reading the+  SMM
> Save Sate register.++  @param[in]  CpuIndex  The index of the CPU to read
> the SMM Save State.  The+                        value must be between 0 and the
> NumberOfCpus field in+                        the System Management System Table
> (SMST).+  @param[in]  Register  The SMM Save State register to read.+
> @param[in]  Width     The number of bytes to read from the CPU save state.+
> @param[out] Buffer    Upon return, this holds the CPU register value read+
> from the save state.++  @retval EFI_SUCCESS           The register was read
> from Save State.+  @retval EFI_INVALID_PARAMETER  Buffer is NULL.+
> @retval EFI_UNSUPPORTED       This function does not support reading
> Register.++**/+EFI_STATUS+EFIAPI+SmmCpuFeaturesReadSaveStateRegist
> er (+  IN  UINTN                        CpuIndex,+  IN
> EFI_SMM_SAVE_STATE_REGISTER  Register,+  IN  UINTN                        Width,+
> OUT VOID                         *Buffer+  )+{+  return EFI_UNSUPPORTED;+}++/**+
> Writes an SMM Save State register on the target processor.  If this function+
> returns EFI_UNSUPPORTED, then the caller is responsible for writing the+
> SMM Save Sate register.++  @param[in] CpuIndex  The index of the CPU to
> write the SMM Save State.  The+                       value must be between 0 and
> the NumberOfCpus field in+                       the System Management System
> Table (SMST).+  @param[in] Register  The SMM Save State register to write.+
> @param[in] Width     The number of bytes to write to the CPU save state.+
> @param[in] Buffer    Upon entry, this holds the new CPU register value.++
> @retval EFI_SUCCESS           The register was written to Save State.+  @retval
> EFI_INVALID_PARAMETER  Buffer is NULL.+  @retval EFI_UNSUPPORTED
> This function does not support writing
> Register.+**/+EFI_STATUS+EFIAPI+SmmCpuFeaturesWriteSaveStateRegiste
> r (+  IN UINTN                        CpuIndex,+  IN EFI_SMM_SAVE_STATE_REGISTER
> Register,+  IN UINTN                        Width,+  IN CONST VOID
> *Buffer+  )+{+  return EFI_UNSUPPORTED;+}++/**+  Check to see if an SMM
> register is supported by a specified CPU.++  @param[in] CpuIndex  The index
> of the CPU to check for SMM register support.+                       The value must
> be between 0 and the NumberOfCpus field+                       in the System
> Management System Table (SMST).+  @param[in] RegName   Identifies the
> SMM register to check for support.++  @retval TRUE   The SMM register
> specified by RegName is supported by the CPU+                 specified by
> CpuIndex.+  @retval FALSE  The SMM register specified by RegName is not
> supported by the+                 CPU specified by
> CpuIndex.+**/+BOOLEAN+EFIAPI+SmmCpuFeaturesIsSmmRegisterSupport
> ed (+  IN UINTN         CpuIndex,+  IN SMM_REG_NAME  RegName+  )+{+  if
> (FeaturePcdGet (PcdSmmFeatureControlEnable) && (RegName ==
> SmmRegFeatureControl)) {+    return TRUE;+  }++  return FALSE;+}++/**+
> This function is hook point called after the
> gEfiSmmReadyToLockProtocolGuid+  notification is completely
> processed.+**/+VOID+EFIAPI+SmmCpuFeaturesCompleteSmmReadyToLoc
> k (+  VOID+  )+{+}diff --git
> a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
> b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
> index 7777e52740eb..2f8841bbbf77 100644
> ---
> a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
> +++
> b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
> @@ -17,49 +17,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>   #include "CpuFeaturesLib.h" -/**-  This function updates the SMRAM save
> state on the currently executing CPU-  to resume execution at a specific
> address after an RSM instruction.  This-  function must evaluate the SMRAM
> save state to determine the execution mode-  the RSM instruction resumes
> and update the resume execution address with-  either
> NewInstructionPointer32 or NewInstructionPoint.  The auto HALT restart-
> flag in the SMRAM save state must always be cleared.  This function returns-
> the value of the instruction pointer from the SMRAM save state that was-
> replaced.  If this function returns 0, then the SMRAM save state was not-
> modified.--  This function is called during the very first SMI on each CPU
> after-  SmmCpuFeaturesInitializeProcessor() to set a flag in normal execution
> mode-  to signal that the SMBASE of each CPU has been updated before the
> default-  SMBASE address is used for the first SMI to the next CPU.--
> @param[in] CpuIndex                 The index of the CPU to hook.  The value-
> must be between 0 and the NumberOfCpus-                                      field in the
> System Management System Table-                                      (SMST).-  @param[in]
> CpuState                 Pointer to SMRAM Save State Map for the-
> currently executing CPU.-  @param[in] NewInstructionPointer32  Instruction
> pointer to use if resuming to-                                      32-bit execution mode from
> 64-bit SMM.-  @param[in] NewInstructionPointer    Instruction pointer to use
> if resuming to-                                      same execution mode as SMM.--  @retval 0
> This function did modify the SMRAM save state.-  @retval > 0  The original
> instruction pointer value from the SMRAM save state-               before it was
> replaced.-**/-UINT64-EFIAPI-SmmCpuFeaturesHookReturnFromSmm (-  IN
> UINTN                 CpuIndex,-  IN SMRAM_SAVE_STATE_MAP  *CpuState,-  IN
> UINT64                NewInstructionPointer32,-  IN UINT64
> NewInstructionPointer-  )-{-  return 0;-}- /**   Hook point in normal execution
> mode that allows the one CPU that was elected   as monarch during System
> Management Mode initialization to perform additional@@ -90,103 +47,6 @@
> SmmCpuFeaturesRendezvousExit (
>  { } -/**-  Check to see if an SMM register is supported by a specified CPU.--
> @param[in] CpuIndex  The index of the CPU to check for SMM register
> support.-                       The value must be between 0 and the NumberOfCpus
> field-                       in the System Management System Table (SMST).-
> @param[in] RegName   Identifies the SMM register to check for support.--
> @retval TRUE   The SMM register specified by RegName is supported by the
> CPU-                 specified by CpuIndex.-  @retval FALSE  The SMM register
> specified by RegName is not supported by the-                 CPU specified by
> CpuIndex.-**/-BOOLEAN-EFIAPI-SmmCpuFeaturesIsSmmRegisterSupported
> (-  IN UINTN         CpuIndex,-  IN SMM_REG_NAME  RegName-  )-{-  if
> (FeaturePcdGet (PcdSmmFeatureControlEnable) && (RegName ==
> SmmRegFeatureControl)) {-    return TRUE;-  }--  return FALSE;-}--/**-  Read
> an SMM Save State register on the target processor.  If this function-  returns
> EFI_UNSUPPORTED, then the caller is responsible for reading the-  SMM
> Save Sate register.--  @param[in]  CpuIndex  The index of the CPU to read
> the SMM Save State.  The-                        value must be between 0 and the
> NumberOfCpus field in-                        the System Management System Table
> (SMST).-  @param[in]  Register  The SMM Save State register to read.-
> @param[in]  Width     The number of bytes to read from the CPU save state.-
> @param[out] Buffer    Upon return, this holds the CPU register value read-
> from the save state.--  @retval EFI_SUCCESS           The register was read from
> Save State.-  @retval EFI_INVALID_PARAMETER  Buffer is NULL.-  @retval
> EFI_UNSUPPORTED       This function does not support reading Register.--**/-
> EFI_STATUS-EFIAPI-SmmCpuFeaturesReadSaveStateRegister (-  IN  UINTN
> CpuIndex,-  IN  EFI_SMM_SAVE_STATE_REGISTER  Register,-  IN  UINTN
> Width,-  OUT VOID                         *Buffer-  )-{-  return EFI_UNSUPPORTED;-}--
> /**-  Writes an SMM Save State register on the target processor.  If this
> function-  returns EFI_UNSUPPORTED, then the caller is responsible for
> writing the-  SMM Save Sate register.--  @param[in] CpuIndex  The index of
> the CPU to write the SMM Save State.  The-                       value must be
> between 0 and the NumberOfCpus field in-                       the System
> Management System Table (SMST).-  @param[in] Register  The SMM Save
> State register to write.-  @param[in] Width     The number of bytes to write
> to the CPU save state.-  @param[in] Buffer    Upon entry, this holds the new
> CPU register value.--  @retval EFI_SUCCESS           The register was written to
> Save State.-  @retval EFI_INVALID_PARAMETER  Buffer is NULL.-  @retval
> EFI_UNSUPPORTED       This function does not support writing Register.-**/-
> EFI_STATUS-EFIAPI-SmmCpuFeaturesWriteSaveStateRegister (-  IN UINTN
> CpuIndex,-  IN EFI_SMM_SAVE_STATE_REGISTER  Register,-  IN UINTN
> Width,-  IN CONST VOID                   *Buffer-  )-{-  return EFI_UNSUPPORTED;-}-
> -/**-  This function is hook point called after the
> gEfiSmmReadyToLockProtocolGuid-  notification is completely processed.-
> **/-VOID-EFIAPI-SmmCpuFeaturesCompleteSmmReadyToLock (-  VOID-  )-{-
> }- /**   This API provides a method for a CPU to allocate a specific region for
> storing page tables. --
> 2.25.1