Re: [edk2-devel] [PATCH v2 1/6] 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: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Abdul
> Lateef Attar via groups.io
> Sent: Wednesday, January 11, 2023 2:16 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: [edk2-devel] [PATCH v2 1/6] UefiCpuPkg/SmmCpuFeaturesLib:
> Restructure arch-dependent code
> 
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
> 
> 
> From: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>
> 
> BZ:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugz
> illa.tianocore.org%2Fshow_bug.cgi%3Fid%3D4182&data=05%7C01%7Cabner.
> chang%40amd.com%7C2a65c8492de34ffa909e08daf39b6685%7C3dd8961fe48
> 84e608e11a82d994e183d%7C0%7C0%7C638090146022518344%7CUnknown%
> 7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haW
> wiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=w4DrN5NOlqrFN2IjJ3o4Qr
> gIxSsuqz54g2HkLhyTE64%3D&reserved=0
> 
> 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                  | 128 ++++++++++++++++++
>  .../SmmCpuFeaturesLibCommon.c                 | 128 ------------------
>  2 files changed, 128 insertions(+), 128 deletions(-)
> 
> diff --git
> a/UefiCpuPkg/Library/SmmCpuFeaturesLib/IntelSmmCpuFeaturesLib.c
> b/UefiCpuPkg/Library/SmmCpuFeaturesLib/IntelSmmCpuFeaturesLib.c
> index d5eaaa7a991e..33b1ddf8cfa9 100644
> --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/IntelSmmCpuFeaturesLib.c
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/IntelSmmCpuFeaturesLib.c
> @@ -400,3 +400,131 @@ 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
> 
> +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;
> 
> +}
> 
> +
> 
> +/**
> 
> +  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;
> 
> +}
> 
> diff --git
> a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
> b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
> index 5498fda38da4..cbf4b495185b 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,91 +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.
> 
> --
> 2.25.1
> 
> 
> 
> 
>