From: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4182

Implements interfaces to read and write save state
registers of AMD's processor family.
Initializes processor SMMADDR and MASK depends
on PcdSmrrEnable flag.
Program or corrects the IP once control returns from SMM.

Cc: Paul Grimes <paul.grimes@amd.com>
Cc: Garrett Kirkendall <garrett.kirkendall@amd.com>
Cc: Abner Chang <abner.chang@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 <abdattar@amd.com>
Reviewed-by: Abner Chang <abner.chang@amd.com>
---
 .../AmdSmmCpuFeaturesLib.inf                  |   6 +
 .../SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.c  | 106 +++++++++++++++++-
 2 files changed, 109 insertions(+), 3 deletions(-)

diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf b/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
index 4c77efc64462..9d5b8c2e972d 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
@@ -31,3 +31,9 @@ [LibraryClasses]
   PcdLib
   MemoryAllocationLib
   DebugLib
+  SmmSmramSaveStateLib
+
+[FeaturePcd]
+  gUefiCpuPkgTokenSpaceGuid.PcdSmrrEnable               ## CONSUMES
+  gUefiCpuPkgTokenSpaceGuid.PcdSmmFeatureControlEnable  ## CONSUMES
+
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.c
index c74e1a0c0c5b..af45be3e265a 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.c
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.c
@@ -11,6 +11,21 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 #include <Library/SmmCpuFeaturesLib.h>
 #include <Uefi/UefiBaseType.h>
+#include <Register/Amd/SmramSaveStateMap.h>
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/SmmSmramSaveStateLib.h>
+
+// EFER register LMA bit
+#define LMA  BIT10
+
+// Machine Specific Registers (MSRs)
+#define SMMADDR_ADDRESS  0xC0010112ul
+#define SMMMASK_ADDRESS  0xC0010113ul
+#define EFER_ADDRESS     0XC0000080ul
+
+// The mode of the CPU at the time an SMI occurs
+STATIC UINT8  mSmmSaveStateRegisterLma;
 
 /**
   Read an SMM Save State register on the target processor.  If this function
@@ -39,7 +54,7 @@ SmmCpuFeaturesReadSaveStateRegister (
   OUT VOID                         *Buffer
   )
 {
-  return EFI_SUCCESS;
+  return SmramSaveStateReadRegister (CpuIndex, Register, Width, Buffer);
 }
 
 /**
@@ -67,7 +82,7 @@ SmmCpuFeaturesWriteSaveStateRegister (
   IN CONST VOID                   *Buffer
   )
 {
-  return EFI_SUCCESS;
+  return SmramSaveStateWriteRegister (CpuIndex, Register, Width, Buffer);
 }
 
 /**
@@ -82,6 +97,13 @@ CpuFeaturesLibInitialization (
   VOID
   )
 {
+  UINT32  LMAValue;
+
+  LMAValue                 = (UINT32)AsmReadMsr64 (EFER_ADDRESS) & LMA;
+  mSmmSaveStateRegisterLma = EFI_SMM_SAVE_STATE_REGISTER_LMA_32BIT;
+  if (LMAValue) {
+    mSmmSaveStateRegisterLma = EFI_SMM_SAVE_STATE_REGISTER_LMA_64BIT;
+  }
 }
 
 /**
@@ -117,6 +139,52 @@ SmmCpuFeaturesInitializeProcessor (
   IN CPU_HOT_PLUG_DATA          *CpuHotPlugData
   )
 {
+  AMD_SMRAM_SAVE_STATE_MAP  *CpuState;
+  UINT32                    LMAValue;
+
+  //
+  // Configure SMBASE.
+  //
+  CpuState             = (AMD_SMRAM_SAVE_STATE_MAP *)(UINTN)(SMM_DEFAULT_SMBASE + SMRAM_SAVE_STATE_MAP_OFFSET);
+  CpuState->x64.SMBASE = (UINT32)CpuHotPlugData->SmBase[CpuIndex];
+
+  // Re-initialize the value of mSmmSaveStateRegisterLma flag which might have been changed in PiCpuSmmDxeSmm Driver
+  // Entry point, to make sure correct value on AMD platform is assigned to be used by SmmCpuFeaturesLib.
+  LMAValue                 = (UINT32)AsmReadMsr64 (EFER_ADDRESS) & LMA;
+  mSmmSaveStateRegisterLma = EFI_SMM_SAVE_STATE_REGISTER_LMA_32BIT;
+  if (LMAValue) {
+    mSmmSaveStateRegisterLma = EFI_SMM_SAVE_STATE_REGISTER_LMA_64BIT;
+  }
+
+  //
+  // If SMRR is supported, then program SMRR base/mask MSRs.
+  // The EFI_MSR_SMRR_PHYS_MASK_VALID bit is not set until the first normal SMI.
+  // The code that initializes SMM environment is running in normal mode
+  // from SMRAM region.  If SMRR is enabled here, then the SMRAM region
+  // is protected and the normal mode code execution will fail.
+  //
+  if (FeaturePcdGet (PcdSmrrEnable)) {
+    //
+    // SMRR size cannot be less than 4-KBytes
+    // SMRR size must be of length 2^n
+    // SMRR base alignment cannot be less than SMRR length
+    //
+    if ((CpuHotPlugData->SmrrSize < SIZE_4KB) ||
+        (CpuHotPlugData->SmrrSize != GetPowerOfTwo32 (CpuHotPlugData->SmrrSize)) ||
+        ((CpuHotPlugData->SmrrBase & ~(CpuHotPlugData->SmrrSize - 1)) != CpuHotPlugData->SmrrBase))
+    {
+      //
+      // Print message and halt if CPU is Monarch
+      //
+      if (IsMonarch) {
+        DEBUG ((DEBUG_ERROR, "SMM Base/Size does not meet alignment/size requirement!\n"));
+        CpuDeadLoop ();
+      }
+    } else {
+      AsmWriteMsr64 (SMMADDR_ADDRESS, CpuHotPlugData->SmrrBase);
+      AsmWriteMsr64 (SMMMASK_ADDRESS, ((~(UINT64)(CpuHotPlugData->SmrrSize - 1)) | 0x6600));
+    }
+  }
 }
 
 /**
@@ -159,7 +227,39 @@ SmmCpuFeaturesHookReturnFromSmm (
   IN UINT64                NewInstructionPointer
   )
 {
-  return 0;
+  UINT64                    OriginalInstructionPointer;
+  AMD_SMRAM_SAVE_STATE_MAP  *AmdCpuState;
+
+  AmdCpuState = (AMD_SMRAM_SAVE_STATE_MAP *)CpuState;
+
+  if (mSmmSaveStateRegisterLma == EFI_SMM_SAVE_STATE_REGISTER_LMA_32BIT) {
+    OriginalInstructionPointer = (UINT64)AmdCpuState->x86._EIP;
+    AmdCpuState->x86._EIP      = (UINT32)NewInstructionPointer;
+    //
+    // Clear the auto HALT restart flag so the RSM instruction returns
+    // program control to the instruction following the HLT instruction.
+    //
+    if ((AmdCpuState->x86.AutoHALTRestart & BIT0) != 0) {
+      AmdCpuState->x86.AutoHALTRestart &= ~BIT0;
+    }
+  } else {
+    OriginalInstructionPointer = AmdCpuState->x64._RIP;
+    if ((AmdCpuState->x64.EFER & LMA) == 0) {
+      AmdCpuState->x64._RIP = (UINT32)NewInstructionPointer32;
+    } else {
+      AmdCpuState->x64._RIP = (UINT32)NewInstructionPointer;
+    }
+
+    //
+    // Clear the auto HALT restart flag so the RSM instruction returns
+    // program control to the instruction following the HLT instruction.
+    //
+    if ((AmdCpuState->x64.AutoHALTRestart & BIT0) != 0) {
+      AmdCpuState->x64.AutoHALTRestart &= ~BIT0;
+    }
+  }
+
+  return OriginalInstructionPointer;
 }
 
 /**
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101867): https://edk2.groups.io/g/devel/message/101867
Mute This Topic: https://groups.io/mt/97839119/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

It's a bit wired that AMD version of SmmCpuFeaturesLib calls AMD version of SmmSmramSaveStateLib.
The question naturally becomes: why not integrate the two lib implementations together to avoid
creating the SmmSmramSaveStateLib library class? Because I cannot imagine AmdSmmCpuFeaturesLib calls
to IntelSmmSmramSaveStateLib.

But I like the idea of separating the SMM save state access logic out of the SmmCpuFeaturesLib.
Do you think CpuSmm driver calling the SmmSmramSaveStateLib is better?


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Abdul
> Lateef Attar via groups.io
> Sent: Saturday, March 25, 2023 1:39 PM
> To: devel@edk2.groups.io
> Cc: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>; Paul Grimes
> <paul.grimes@amd.com>; Garrett Kirkendall <garrett.kirkendall@amd.com>;
> Abner Chang <abner.chang@amd.com>; Dong, Eric <eric.dong@intel.com>;
> Ni, Ray <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>;
> Abdul Lateef Attar <abdattar@amd.com>
> Subject: [edk2-devel] [PATCH v6 6/6] UefiCpuPkg: Implements
> SmmCpuFeaturesLib for AMD Family
> 
> From: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4182
> 
> Implements interfaces to read and write save state
> registers of AMD's processor family.
> Initializes processor SMMADDR and MASK depends
> on PcdSmrrEnable flag.
> Program or corrects the IP once control returns from SMM.
> 
> Cc: Paul Grimes <paul.grimes@amd.com>
> Cc: Garrett Kirkendall <garrett.kirkendall@amd.com>
> Cc: Abner Chang <abner.chang@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 <abdattar@amd.com>
> Reviewed-by: Abner Chang <abner.chang@amd.com>
> ---
>  .../AmdSmmCpuFeaturesLib.inf                  |   6 +
>  .../SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.c  | 106
> +++++++++++++++++-
>  2 files changed, 109 insertions(+), 3 deletions(-)
> 
> diff --git
> a/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
> b/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
> index 4c77efc64462..9d5b8c2e972d 100644
> --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
> +++
> b/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
> @@ -31,3 +31,9 @@ [LibraryClasses]
>    PcdLib
>    MemoryAllocationLib
>    DebugLib
> +  SmmSmramSaveStateLib
> +
> +[FeaturePcd]
> +  gUefiCpuPkgTokenSpaceGuid.PcdSmrrEnable               ## CONSUMES
> +  gUefiCpuPkgTokenSpaceGuid.PcdSmmFeatureControlEnable  ##
> CONSUMES
> +
> diff --git
> a/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.c
> b/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.c
> index c74e1a0c0c5b..af45be3e265a 100644
> --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.c
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.c
> @@ -11,6 +11,21 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  #include <Library/SmmCpuFeaturesLib.h>
>  #include <Uefi/UefiBaseType.h>
> +#include <Register/Amd/SmramSaveStateMap.h>
> +#include <Library/BaseLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/SmmSmramSaveStateLib.h>
> +
> +// EFER register LMA bit
> +#define LMA  BIT10
> +
> +// Machine Specific Registers (MSRs)
> +#define SMMADDR_ADDRESS  0xC0010112ul
> +#define SMMMASK_ADDRESS  0xC0010113ul
> +#define EFER_ADDRESS     0XC0000080ul
> +
> +// The mode of the CPU at the time an SMI occurs
> +STATIC UINT8  mSmmSaveStateRegisterLma;
> 
>  /**
>    Read an SMM Save State register on the target processor.  If this function
> @@ -39,7 +54,7 @@ SmmCpuFeaturesReadSaveStateRegister (
>    OUT VOID                         *Buffer
>    )
>  {
> -  return EFI_SUCCESS;
> +  return SmramSaveStateReadRegister (CpuIndex, Register, Width, Buffer);
>  }
> 
>  /**
> @@ -67,7 +82,7 @@ SmmCpuFeaturesWriteSaveStateRegister (
>    IN CONST VOID                   *Buffer
>    )
>  {
> -  return EFI_SUCCESS;
> +  return SmramSaveStateWriteRegister (CpuIndex, Register, Width, Buffer);
>  }
> 
>  /**
> @@ -82,6 +97,13 @@ CpuFeaturesLibInitialization (
>    VOID
>    )
>  {
> +  UINT32  LMAValue;
> +
> +  LMAValue                 = (UINT32)AsmReadMsr64 (EFER_ADDRESS) & LMA;
> +  mSmmSaveStateRegisterLma =
> EFI_SMM_SAVE_STATE_REGISTER_LMA_32BIT;
> +  if (LMAValue) {
> +    mSmmSaveStateRegisterLma =
> EFI_SMM_SAVE_STATE_REGISTER_LMA_64BIT;
> +  }
>  }
> 
>  /**
> @@ -117,6 +139,52 @@ SmmCpuFeaturesInitializeProcessor (
>    IN CPU_HOT_PLUG_DATA          *CpuHotPlugData
>    )
>  {
> +  AMD_SMRAM_SAVE_STATE_MAP  *CpuState;
> +  UINT32                    LMAValue;
> +
> +  //
> +  // Configure SMBASE.
> +  //
> +  CpuState             = (AMD_SMRAM_SAVE_STATE_MAP
> *)(UINTN)(SMM_DEFAULT_SMBASE + SMRAM_SAVE_STATE_MAP_OFFSET);
> +  CpuState->x64.SMBASE = (UINT32)CpuHotPlugData->SmBase[CpuIndex];
> +
> +  // Re-initialize the value of mSmmSaveStateRegisterLma flag which might
> have been changed in PiCpuSmmDxeSmm Driver
> +  // Entry point, to make sure correct value on AMD platform is assigned to
> be used by SmmCpuFeaturesLib.
> +  LMAValue                 = (UINT32)AsmReadMsr64 (EFER_ADDRESS) & LMA;
> +  mSmmSaveStateRegisterLma =
> EFI_SMM_SAVE_STATE_REGISTER_LMA_32BIT;
> +  if (LMAValue) {
> +    mSmmSaveStateRegisterLma =
> EFI_SMM_SAVE_STATE_REGISTER_LMA_64BIT;
> +  }
> +
> +  //
> +  // If SMRR is supported, then program SMRR base/mask MSRs.
> +  // The EFI_MSR_SMRR_PHYS_MASK_VALID bit is not set until the first
> normal SMI.
> +  // The code that initializes SMM environment is running in normal mode
> +  // from SMRAM region.  If SMRR is enabled here, then the SMRAM region
> +  // is protected and the normal mode code execution will fail.
> +  //
> +  if (FeaturePcdGet (PcdSmrrEnable)) {
> +    //
> +    // SMRR size cannot be less than 4-KBytes
> +    // SMRR size must be of length 2^n
> +    // SMRR base alignment cannot be less than SMRR length
> +    //
> +    if ((CpuHotPlugData->SmrrSize < SIZE_4KB) ||
> +        (CpuHotPlugData->SmrrSize != GetPowerOfTwo32 (CpuHotPlugData-
> >SmrrSize)) ||
> +        ((CpuHotPlugData->SmrrBase & ~(CpuHotPlugData->SmrrSize - 1)) !=
> CpuHotPlugData->SmrrBase))
> +    {
> +      //
> +      // Print message and halt if CPU is Monarch
> +      //
> +      if (IsMonarch) {
> +        DEBUG ((DEBUG_ERROR, "SMM Base/Size does not meet
> alignment/size requirement!\n"));
> +        CpuDeadLoop ();
> +      }
> +    } else {
> +      AsmWriteMsr64 (SMMADDR_ADDRESS, CpuHotPlugData->SmrrBase);
> +      AsmWriteMsr64 (SMMMASK_ADDRESS, ((~(UINT64)(CpuHotPlugData-
> >SmrrSize - 1)) | 0x6600));
> +    }
> +  }
>  }
> 
>  /**
> @@ -159,7 +227,39 @@ SmmCpuFeaturesHookReturnFromSmm (
>    IN UINT64                NewInstructionPointer
>    )
>  {
> -  return 0;
> +  UINT64                    OriginalInstructionPointer;
> +  AMD_SMRAM_SAVE_STATE_MAP  *AmdCpuState;
> +
> +  AmdCpuState = (AMD_SMRAM_SAVE_STATE_MAP *)CpuState;
> +
> +  if (mSmmSaveStateRegisterLma ==
> EFI_SMM_SAVE_STATE_REGISTER_LMA_32BIT) {
> +    OriginalInstructionPointer = (UINT64)AmdCpuState->x86._EIP;
> +    AmdCpuState->x86._EIP      = (UINT32)NewInstructionPointer;
> +    //
> +    // Clear the auto HALT restart flag so the RSM instruction returns
> +    // program control to the instruction following the HLT instruction.
> +    //
> +    if ((AmdCpuState->x86.AutoHALTRestart & BIT0) != 0) {
> +      AmdCpuState->x86.AutoHALTRestart &= ~BIT0;
> +    }
> +  } else {
> +    OriginalInstructionPointer = AmdCpuState->x64._RIP;
> +    if ((AmdCpuState->x64.EFER & LMA) == 0) {
> +      AmdCpuState->x64._RIP = (UINT32)NewInstructionPointer32;
> +    } else {
> +      AmdCpuState->x64._RIP = (UINT32)NewInstructionPointer;
> +    }
> +
> +    //
> +    // Clear the auto HALT restart flag so the RSM instruction returns
> +    // program control to the instruction following the HLT instruction.
> +    //
> +    if ((AmdCpuState->x64.AutoHALTRestart & BIT0) != 0) {
> +      AmdCpuState->x64.AutoHALTRestart &= ~BIT0;
> +    }
> +  }
> +
> +  return OriginalInstructionPointer;
>  }
> 
>  /**
> --
> 2.25.1
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#102246): https://edk2.groups.io/g/devel/message/102246
Mute This Topic: https://groups.io/mt/97839119/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

[AMD Official Use Only - General]

Hi Ray,
        Please see inline.
Thanks
AbduL
-----Original Message-----
From: Ni, Ray <ray.ni@intel.com>
Sent: 31 March 2023 13:29
To: devel@edk2.groups.io; Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>
Cc: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>; Grimes, Paul <Paul.Grimes@amd.com>; Kirkendall, Garrett <Garrett.Kirkendall@amd.com>; Chang, Abner <Abner.Chang@amd.com>; Dong, Eric <eric.dong@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>
Subject: RE: [edk2-devel] [PATCH v6 6/6] UefiCpuPkg: Implements SmmCpuFeaturesLib for AMD Family

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


It's a bit wired that AMD version of SmmCpuFeaturesLib calls AMD version of SmmSmramSaveStateLib.
The question naturally becomes: why not integrate the two lib implementations together to avoid creating the SmmSmramSaveStateLib library class? Because I cannot imagine AmdSmmCpuFeaturesLib calls to IntelSmmSmramSaveStateLib.
[Abdul] Initial version of this patch series has combined implementation of SmmSmramSaveStateLib and AmdSmmCpuFeaturesLib, but reviewers (Abner) suggested to separate the save state library; So that we can implement generic library for both AMD and Intel.

But I like the idea of separating the SMM save state access logic out of the SmmCpuFeaturesLib.
Do you think CpuSmm driver calling the SmmSmramSaveStateLib is better?
[Abdul] I'll make the changes and submit the patch for the same.

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Abdul
> Lateef Attar via groups.io
> Sent: Saturday, March 25, 2023 1:39 PM
> To: devel@edk2.groups.io
> Cc: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>; Paul Grimes
> <paul.grimes@amd.com>; Garrett Kirkendall
> <garrett.kirkendall@amd.com>; Abner Chang <abner.chang@amd.com>; Dong,
> Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul R
> <rahul.r.kumar@intel.com>; Abdul Lateef Attar <abdattar@amd.com>
> Subject: [edk2-devel] [PATCH v6 6/6] UefiCpuPkg: Implements
> SmmCpuFeaturesLib for AMD Family
>
> From: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4182
>
> Implements interfaces to read and write save state registers of AMD's
> processor family.
> Initializes processor SMMADDR and MASK depends on PcdSmrrEnable flag.
> Program or corrects the IP once control returns from SMM.
>
> Cc: Paul Grimes <paul.grimes@amd.com>
> Cc: Garrett Kirkendall <garrett.kirkendall@amd.com>
> Cc: Abner Chang <abner.chang@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 <abdattar@amd.com>
> Reviewed-by: Abner Chang <abner.chang@amd.com>
> ---
>  .../AmdSmmCpuFeaturesLib.inf                  |   6 +
>  .../SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.c  | 106
> +++++++++++++++++-
>  2 files changed, 109 insertions(+), 3 deletions(-)
>
> diff --git
> a/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
> b/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
> index 4c77efc64462..9d5b8c2e972d 100644
> --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
> +++
> b/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
> @@ -31,3 +31,9 @@ [LibraryClasses]
>    PcdLib
>    MemoryAllocationLib
>    DebugLib
> +  SmmSmramSaveStateLib
> +
> +[FeaturePcd]
> +  gUefiCpuPkgTokenSpaceGuid.PcdSmrrEnable               ## CONSUMES
> +  gUefiCpuPkgTokenSpaceGuid.PcdSmmFeatureControlEnable  ##
> CONSUMES
> +
> diff --git
> a/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.c
> b/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.c
> index c74e1a0c0c5b..af45be3e265a 100644
> --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.c
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.c
> @@ -11,6 +11,21 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>
>  #include <Library/SmmCpuFeaturesLib.h>  #include
> <Uefi/UefiBaseType.h>
> +#include <Register/Amd/SmramSaveStateMap.h>
> +#include <Library/BaseLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/SmmSmramSaveStateLib.h>
> +
> +// EFER register LMA bit
> +#define LMA  BIT10
> +
> +// Machine Specific Registers (MSRs)
> +#define SMMADDR_ADDRESS  0xC0010112ul #define SMMMASK_ADDRESS
> +0xC0010113ul
> +#define EFER_ADDRESS     0XC0000080ul
> +
> +// The mode of the CPU at the time an SMI occurs STATIC UINT8
> +mSmmSaveStateRegisterLma;
>
>  /**
>    Read an SMM Save State register on the target processor.  If this
> function @@ -39,7 +54,7 @@ SmmCpuFeaturesReadSaveStateRegister (
>    OUT VOID                         *Buffer
>    )
>  {
> -  return EFI_SUCCESS;
> +  return SmramSaveStateReadRegister (CpuIndex, Register, Width,
> + Buffer);
>  }
>
>  /**
> @@ -67,7 +82,7 @@ SmmCpuFeaturesWriteSaveStateRegister (
>    IN CONST VOID                   *Buffer
>    )
>  {
> -  return EFI_SUCCESS;
> +  return SmramSaveStateWriteRegister (CpuIndex, Register, Width,
> + Buffer);
>  }
>
>  /**
> @@ -82,6 +97,13 @@ CpuFeaturesLibInitialization (
>    VOID
>    )
>  {
> +  UINT32  LMAValue;
> +
> +  LMAValue                 = (UINT32)AsmReadMsr64 (EFER_ADDRESS) & LMA;
> +  mSmmSaveStateRegisterLma =
> EFI_SMM_SAVE_STATE_REGISTER_LMA_32BIT;
> +  if (LMAValue) {
> +    mSmmSaveStateRegisterLma =
> EFI_SMM_SAVE_STATE_REGISTER_LMA_64BIT;
> +  }
>  }
>
>  /**
> @@ -117,6 +139,52 @@ SmmCpuFeaturesInitializeProcessor (
>    IN CPU_HOT_PLUG_DATA          *CpuHotPlugData
>    )
>  {
> +  AMD_SMRAM_SAVE_STATE_MAP  *CpuState;
> +  UINT32                    LMAValue;
> +
> +  //
> +  // Configure SMBASE.
> +  //
> +  CpuState             = (AMD_SMRAM_SAVE_STATE_MAP
> *)(UINTN)(SMM_DEFAULT_SMBASE + SMRAM_SAVE_STATE_MAP_OFFSET);
> +  CpuState->x64.SMBASE = (UINT32)CpuHotPlugData->SmBase[CpuIndex];
> +
> +  // Re-initialize the value of mSmmSaveStateRegisterLma flag which
> + might
> have been changed in PiCpuSmmDxeSmm Driver
> +  // Entry point, to make sure correct value on AMD platform is
> + assigned to
> be used by SmmCpuFeaturesLib.
> +  LMAValue                 = (UINT32)AsmReadMsr64 (EFER_ADDRESS) & LMA;
> +  mSmmSaveStateRegisterLma =
> EFI_SMM_SAVE_STATE_REGISTER_LMA_32BIT;
> +  if (LMAValue) {
> +    mSmmSaveStateRegisterLma =
> EFI_SMM_SAVE_STATE_REGISTER_LMA_64BIT;
> +  }
> +
> +  //
> +  // If SMRR is supported, then program SMRR base/mask MSRs.
> +  // The EFI_MSR_SMRR_PHYS_MASK_VALID bit is not set until the first
> normal SMI.
> +  // The code that initializes SMM environment is running in normal
> + mode  // from SMRAM region.  If SMRR is enabled here, then the SMRAM
> + region  // is protected and the normal mode code execution will fail.
> +  //
> +  if (FeaturePcdGet (PcdSmrrEnable)) {
> +    //
> +    // SMRR size cannot be less than 4-KBytes
> +    // SMRR size must be of length 2^n
> +    // SMRR base alignment cannot be less than SMRR length
> +    //
> +    if ((CpuHotPlugData->SmrrSize < SIZE_4KB) ||
> +        (CpuHotPlugData->SmrrSize != GetPowerOfTwo32 (CpuHotPlugData-
> >SmrrSize)) ||
> +        ((CpuHotPlugData->SmrrBase & ~(CpuHotPlugData->SmrrSize - 1))
> + !=
> CpuHotPlugData->SmrrBase))
> +    {
> +      //
> +      // Print message and halt if CPU is Monarch
> +      //
> +      if (IsMonarch) {
> +        DEBUG ((DEBUG_ERROR, "SMM Base/Size does not meet
> alignment/size requirement!\n"));
> +        CpuDeadLoop ();
> +      }
> +    } else {
> +      AsmWriteMsr64 (SMMADDR_ADDRESS, CpuHotPlugData->SmrrBase);
> +      AsmWriteMsr64 (SMMMASK_ADDRESS, ((~(UINT64)(CpuHotPlugData-
> >SmrrSize - 1)) | 0x6600));
> +    }
> +  }
>  }
>
>  /**
> @@ -159,7 +227,39 @@ SmmCpuFeaturesHookReturnFromSmm (
>    IN UINT64                NewInstructionPointer
>    )
>  {
> -  return 0;
> +  UINT64                    OriginalInstructionPointer;
> +  AMD_SMRAM_SAVE_STATE_MAP  *AmdCpuState;
> +
> +  AmdCpuState = (AMD_SMRAM_SAVE_STATE_MAP *)CpuState;
> +
> +  if (mSmmSaveStateRegisterLma ==
> EFI_SMM_SAVE_STATE_REGISTER_LMA_32BIT) {
> +    OriginalInstructionPointer = (UINT64)AmdCpuState->x86._EIP;
> +    AmdCpuState->x86._EIP      = (UINT32)NewInstructionPointer;
> +    //
> +    // Clear the auto HALT restart flag so the RSM instruction returns
> +    // program control to the instruction following the HLT instruction.
> +    //
> +    if ((AmdCpuState->x86.AutoHALTRestart & BIT0) != 0) {
> +      AmdCpuState->x86.AutoHALTRestart &= ~BIT0;
> +    }
> +  } else {
> +    OriginalInstructionPointer = AmdCpuState->x64._RIP;
> +    if ((AmdCpuState->x64.EFER & LMA) == 0) {
> +      AmdCpuState->x64._RIP = (UINT32)NewInstructionPointer32;
> +    } else {
> +      AmdCpuState->x64._RIP = (UINT32)NewInstructionPointer;
> +    }
> +
> +    //
> +    // Clear the auto HALT restart flag so the RSM instruction returns
> +    // program control to the instruction following the HLT instruction.
> +    //
> +    if ((AmdCpuState->x64.AutoHALTRestart & BIT0) != 0) {
> +      AmdCpuState->x64.AutoHALTRestart &= ~BIT0;
> +    }
> +  }
> +
> +  return OriginalInstructionPointer;
>  }
>
>  /**
> --
> 2.25.1
>
>
>
> 
>



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#102711): https://edk2.groups.io/g/devel/message/102711
Mute This Topic: https://groups.io/mt/97839119/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-