Re: [edk2-devel] [PATCH v1 4/5] UefiCpuPkg: Implements SmmCpuFeaturesLib for AMD Family
[AMD Official Use Only - General]
Hi Abner,
Thanks for quick review. I'll create a new library and submit
the patch.
Thanks
AbduL
From: abner.chang via groups.io
Sent: 08 December 2022 10:38
To: Attar, AbdulLateef (Abdul Lateef) ;
devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH v1 4/5] UefiCpuPkg: Implements
SmmCpuFeaturesLib for AMD Family
Caution: This message originated from an External Source. Use proper caution
when opening attachments, clicking links, or responding.
Hi Abdul,
This is a little bit confusing because there is one SmramSaveStae.c under
PismmCpuDxeSmm however another one is under SmmCpuFeaturesLib for AMD.
I would suggest we introduce SmramSaveState library under UefiCpuPkg/Library
which is used by both PismmCpuDxeSmm and SmmCpuFeaturesLib. This makes the
library reference clear without duplication.
We can have AMD SmramSaveStateLib instance and just name it as
SmramSaveStateLib.c. That is Intel's decision if they want to move their
implementation from PismmCpuDxeSmm to SmramSaveState or not.
Thanks
Abner
On Tue, Dec 6, 2022 at 09:23 PM, Abdul Lateef Attar wrote:
From: Abdul Lateef Attar
mailto:abdullateef.at...@amd.com>>
BZ:
https://bugzilla.tianocore.org/show_bug.cgi?id=4182<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D4182&data=05%7C01%7CAbdulLateef.Attar%40amd.com%7C32f4892e39954cbdc31d08dad8da1d95%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638060728564261470%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=np1IkSMu3ZBp2tWmKsIO07FFLATLhpDY6JaWiFjluQU%3D&reserved=0>
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 mailto:paul.gri...@amd.com>>
Cc: Garrett Kirkendall
mailto:garrett.kirkend...@amd.com>>
Cc: Abner Chang mailto:abner.ch...@amd.com>>
Cc: Eric Dong mailto:eric.d...@intel.com>>
Cc: Ray Ni mailto:ray...@intel.com>>
Cc: Rahul Kumar mailto:rahul1.ku...@intel.com>>
Signed-off-by: Abdul Lateef Attar
mailto:abdullateef.at...@amd.com>>
---
.../AmdSmmCpuFeaturesLib.inf | 2 +
.../SmmCpuFeaturesLib/Amd/SmramSaveState.h | 109 +
.../SmmCpuFeaturesLib/Amd/SmmCpuFeaturesLib.c | 97 -
.../SmmCpuFeaturesLib/Amd/SmramSaveState.c | 409 ++
4 files changed, 612 insertions(+), 5 deletions(-)
create mode 100644 UefiCpuPkg/Library/SmmCpuFeaturesLib/Amd/SmramSaveState.h
create mode 100644 UefiCpuPkg/Library/SmmCpuFeaturesLib/Amd/SmramSaveState.c
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
b/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
index 08ac0262022f..95eb31d16ead 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
@@ -21,6 +21,8 @@ [Sources]
SmmCpuFeaturesLib.c
SmmCpuFeaturesLibCommon.c
Amd/SmmCpuFeaturesLib.c
+ Amd/SmramSaveState.c
+ Amd/SmramSaveState.h
[Packages]
MdePkg/MdePkg.dec
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/Amd/SmramSaveState.h
b/UefiCpuPkg/Library/SmmCpuFeaturesLib/Amd/SmramSaveState.h
new file mode 100644
index ..290ebdbc9227
--- /dev/null
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/Amd/SmramSaveState.h
@@ -0,0 +1,109 @@
+/** @file
+SMRAM Save State Map header file.
+
+Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.
+Copyright (C) 2022 Advanced Micro Devices, Inc. All rights reserved.
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef SMRAM_SAVESTATE_H_
+#define SMRAM_SAVESTATE_H_
+
+#include
+#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
+
+// Macro used to simplify the lookup table entries of type
CPU_SMM_SAVE_STATE_LOOKUP_ENTRY
+#define SMM_CPU_OFFSET(Field) OFFSET_OF (AMD_SMRAM_SAVE_STATE_MAP, Field)
+
+// Macro used to simplify the lookup table entries of type
CPU_SMM_SAVE_STATE_REGISTER_RANGE
+#define SMM_REGISTER_RANGE(Start, End) { Start, End, End - Start + 1 }
+
+// Structure used to describe a range of registers
+typedef struct {
+ EFI_SMM_SAVE_STATE_REGISTER Start;
+ EFI_SMM_SAVE_STATE_REGISTER End;
+ UINTN Length;
+} CPU_SMM_SAVE_STATE_REGISTER_RANGE;
+
+// Structure used to build a lookup table to retrieve the widths and offsets
+// associated with each supported EFI_SMM_SAVE_STATE_REGISTER value
+
+#define SMM_SAVE_STATE_REGISTER_SMMREVID_INDEX 1
+#define SMM_SAVE_STATE_REGISTER_MAX_INDEX 2
+
+typedef struct {
+ UINT8 Width32;
+ UINT8 Width64;
+ UINT16 Offset32;
+ UINT16 Offset64Lo;
+ UINT16 Offset64
Re: [edk2-devel] [PATCH v1 4/5] UefiCpuPkg: Implements SmmCpuFeaturesLib for AMD Family
Hi Abdul,
This is a little bit confusing because there is one SmramSaveStae.c under
PismmCpuDxeSmm however another one is under SmmCpuFeaturesLib for AMD.
I would suggest we introduce SmramSaveState library under UefiCpuPkg/Library
which is used by both PismmCpuDxeSmm and SmmCpuFeaturesLib. This makes the
library reference clear without duplication.
We can have AMD SmramSaveStateLib instance and just name it as
SmramSaveStateLib.c. That is Intel's decision if they want to move their
implementation from PismmCpuDxeSmm to SmramSaveState or not.
Thanks
Abner
On Tue, Dec 6, 2022 at 09:23 PM, Abdul Lateef Attar wrote:
>
> 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
> ---
> .../AmdSmmCpuFeaturesLib.inf | 2 +
> .../SmmCpuFeaturesLib/Amd/SmramSaveState.h | 109 +
> .../SmmCpuFeaturesLib/Amd/SmmCpuFeaturesLib.c | 97 -
> .../SmmCpuFeaturesLib/Amd/SmramSaveState.c | 409 ++
> 4 files changed, 612 insertions(+), 5 deletions(-)
> create mode 100644
> UefiCpuPkg/Library/SmmCpuFeaturesLib/Amd/SmramSaveState.h
> create mode 100644
> UefiCpuPkg/Library/SmmCpuFeaturesLib/Amd/SmramSaveState.c
>
> diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
> b/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
> index 08ac0262022f..95eb31d16ead 100644
> --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
> @@ -21,6 +21,8 @@ [Sources]
> SmmCpuFeaturesLib.c
> SmmCpuFeaturesLibCommon.c
> Amd/SmmCpuFeaturesLib.c
> + Amd/SmramSaveState.c
> + Amd/SmramSaveState.h
>
> [Packages]
> MdePkg/MdePkg.dec
> diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/Amd/SmramSaveState.h
> b/UefiCpuPkg/Library/SmmCpuFeaturesLib/Amd/SmramSaveState.h
> new file mode 100644
> index ..290ebdbc9227
> --- /dev/null
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/Amd/SmramSaveState.h
> @@ -0,0 +1,109 @@
> +/** @file
> +SMRAM Save State Map header file.
> +
> +Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.
> +Copyright (C) 2022 Advanced Micro Devices, Inc. All rights reserved.
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef SMRAM_SAVESTATE_H_
> +#define SMRAM_SAVESTATE_H_
> +
> +#include
> +#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
> +
> +// Macro used to simplify the lookup table entries of type
> CPU_SMM_SAVE_STATE_LOOKUP_ENTRY
> +#define SMM_CPU_OFFSET(Field) OFFSET_OF (AMD_SMRAM_SAVE_STATE_MAP, Field)
>
> +
> +// Macro used to simplify the lookup table entries of type
> CPU_SMM_SAVE_STATE_REGISTER_RANGE
> +#define SMM_REGISTER_RANGE(Start, End) { Start, End, End - Start + 1 }
> +
> +// Structure used to describe a range of registers
> +typedef struct {
> + EFI_SMM_SAVE_STATE_REGISTER Start;
> + EFI_SMM_SAVE_STATE_REGISTER End;
> + UINTN Length;
> +} CPU_SMM_SAVE_STATE_REGISTER_RANGE;
> +
> +// Structure used to build a lookup table to retrieve the widths and
> offsets
> +// associated with each supported EFI_SMM_SAVE_STATE_REGISTER value
> +
> +#define SMM_SAVE_STATE_REGISTER_SMMREVID_INDEX 1
> +#define SMM_SAVE_STATE_REGISTER_MAX_INDEX 2
> +
> +typedef struct {
> + UINT8 Width32;
> + UINT8 Width64;
> + UINT16 Offset32;
> + UINT16 Offset64Lo;
> + UINT16 Offset64Hi;
> + BOOLEAN Writeable;
> +} CPU_SMM_SAVE_STATE_LOOKUP_ENTRY;
> +
> +/**
> + 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_PARAMTER Buffer is NULL.
> + @retval EFI_UNSUPPORTED This function does not support reading Register.
>
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +InternalSmmCpuFeaturesReadSaveStateRegister (
> + IN UINTN CpuIndex,
> + IN EFI_SMM_SAVE_STATE_REGISTER Register,
> + IN UINTN Width,
> +
