Re: [edk2-devel] [PATCH v8 6/9] UefiCpuPkg: Implements SmmCpuFeaturesLib for AMD Family

2023-04-11 Thread Ni, Ray
Acked-by: Ray Ni 

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Abdul
> Lateef Attar via groups.io
> Sent: Monday, April 10, 2023 7:10 PM
> To: devel@edk2.groups.io
> Cc: Abdul Lateef Attar ; Paul Grimes
> ; Garrett Kirkendall ;
> Abner Chang ; Dong, Eric ;
> Ni, Ray ; Kumar, Rahul R ;
> Abdul Lateef Attar 
> Subject: [edk2-devel] [PATCH v8 6/9] UefiCpuPkg: Implements
> SmmCpuFeaturesLib for AMD Family
> 
> From: Abdul Lateef Attar 
> 
> 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 
> Cc: Garrett Kirkendall 
> Cc: Abner Chang 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Rahul Kumar 
> Signed-off-by: Abdul Lateef Attar 
> Reviewed-by: Abner Chang 
> ---
>  .../AmdSmmCpuFeaturesLib.inf  |   5 +
>  .../SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.c  | 106
> +-
>  2 files changed, 108 insertions(+), 3 deletions(-)
> 
> diff --git
> a/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
> b/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
> index 4c77efc64462..6d6f879e2a43 100644
> --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
> +++
> b/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
> @@ -31,3 +31,8 @@ [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 
>  #include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +// EFER register LMA bit
> +#define LMA  BIT10
> +
> +// Machine Specific Registers (MSRs)
> +#define SMMADDR_ADDRESS  0xC0010112ul
> +#define SMMMASK_ADDRESS  0xC0010113ul
> +#define EFER_ADDRESS 0XC080ul
> +
> +// 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;
> +  UINT32LMAValue;
> +
> +  //
> +  // 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
> norma

[edk2-devel] [PATCH v8 6/9] UefiCpuPkg: Implements SmmCpuFeaturesLib for AMD Family

2023-04-10 Thread Abdul Lateef Attar via groups.io
From: Abdul Lateef Attar 

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 
Cc: Garrett Kirkendall 
Cc: Abner Chang 
Cc: Eric Dong 
Cc: Ray Ni 
Cc: Rahul Kumar 
Signed-off-by: Abdul Lateef Attar 
Reviewed-by: Abner Chang 
---
 .../AmdSmmCpuFeaturesLib.inf  |   5 +
 .../SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.c  | 106 +-
 2 files changed, 108 insertions(+), 3 deletions(-)

diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf 
b/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
index 4c77efc64462..6d6f879e2a43 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
@@ -31,3 +31,8 @@ [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 
 #include 
+#include 
+#include 
+#include 
+#include 
+
+// EFER register LMA bit
+#define LMA  BIT10
+
+// Machine Specific Registers (MSRs)
+#define SMMADDR_ADDRESS  0xC0010112ul
+#define SMMMASK_ADDRESS  0xC0010113ul
+#define EFER_ADDRESS 0XC080ul
+
+// 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;
+  UINT32LMAValue;
+
+  //
+  // 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