I haven't followed the whole thread but looking at v11, I wanted to
check on the following.
1. The library source files (for AmdMmSaveStateLib and
IntelMmSaveStateLib) include several library headers (particularly in
MmSaveStateLib/MmSaveState.h) but do not have a [LibraryClasses] section
in their INF files. Is there a reason?
2. AmdMmSaveStateLib and IntelMmSaveStateLib depend on
SmmServicesTableLib. Can they depend on MmServicesTableLib instead?
3. It seems resources like the EFER register LMA bit should be defined
somewhere agnostic to the MmSaveState.h file in MmSaveStateLib. It is
referenced in MdePkg/Include/Register/Intel/ArchitecturalMsr.h:
https://github.com/tianocore/edk2/blob/5215cd5baf6609e54050c69909273b7f5161c59e/MdePkg/Include/Register/Intel/ArchitecturalMsr.h#L6342
In any case, can the bit be defined in one location?
4. Your change to the SmmCpuFeaturesLib.h interface is not backward
compatible and a build breaking change. I think that should be called
out in your cover letter.
Thanks,
Michael
On 5/6/2023 12:06 AM, Abdul Lateef Attar via groups.io wrote:
PR: https://github.com/tianocore/edk2/pull/4341
V11: Delta changes
Drop the OVMF implementation of MmSaveStateLib
V10: Delta changes:
Addressed review comments from Abner.
V9: Delta changes:
Addressed review comments.
Rename to MmSaveStateLib.
Also rename SMM_ defines to MM_.
Implemented OVMF MmSaveStateLib.
Removes SmmCpuFeaturesReadSaveStateRegister and
SmmCpuFeaturesWriteSaveStateRegister
function interface.
V8 delta changes:
Addressed review comments from Abner,
Fix the whitespace error.
Seperate the Ovmf changes to another patch
V7 delta changes:
Adds SmmSmramSaveStateLib for Intel processor.
Integrate SmmSmramSaveStateLib library.
V6 delta changes:
Addressed review comments for Ray NI.
removed unnecessary EFIAPI.
V5 delta changes:
rebase to master branch.
updated Reviewed-by
V4 delta changes:
rebase to master branch.
added reviewed-by.
V3 delta changes:
Addressed review comments from Abner chang.
Re-arranged patch order.
Cc: Paul Grimes <paul.gri...@amd.com>
Cc: Abner Chang <abner.ch...@amd.com>
Cc: Eric Dong <eric.d...@intel.com>
Cc: Ray Ni <ray...@intel.com>
Cc: Rahul Kumar <rahul1.ku...@intel.com>
Cc: Gerd Hoffmann <kra...@redhat.com>
Cc: Michael D Kinney <michael.d.kin...@intel.com>
Cc: Liming Gao <gaolim...@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang....@intel.com>
Cc: Ard Biesheuvel <ardb+tianoc...@kernel.org>
Cc: Jiewen Yao <jiewen....@intel.com>
Cc: Jordan Justen <jordan.l.jus...@intel.com>
Cc: Abdul Lateef Attar <abdat...@amd.com>
Abdul Lateef Attar (8):
MdePkg: Adds AMD SMRAM save state map
UefiCpuPkg: Adds MmSaveStateLib library class
UefiCpuPkg: Implements MmSaveStateLib library instance
UefiCpuPkg/SmmCpuFeaturesLib: Restructure arch-dependent code
UefiCpuPkg: Implements SmmCpuFeaturesLib for AMD Family
UefiCpuPkg: Implements MmSaveStateLib for Intel
UefiCpuPkg: Removes SmmCpuFeaturesReadSaveStateRegister
OvmfPkg: Uses MmSaveStateLib library
UefiCpuPkg/UefiCpuPkg.dec | 4 +
OvmfPkg/OvmfPkgIa32.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc | 3 +
OvmfPkg/OvmfPkgX64.dsc | 1 +
UefiCpuPkg/UefiCpuPkg.dsc | 14 +
.../MmSaveStateLib/AmdMmSaveStateLib.inf | 28 +
.../MmSaveStateLib/IntelMmSaveStateLib.inf | 28 +
.../AmdSmmCpuFeaturesLib.inf | 38 +
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 2 +
.../Include/Register/Amd/SmramSaveStateMap.h | 194 +++++
UefiCpuPkg/Include/Library/MmSaveStateLib.h | 70 ++
.../Include/Library/SmmCpuFeaturesLib.h | 52 --
.../Library/MmSaveStateLib/MmSaveState.h | 102 +++
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 56 +-
.../SmmCpuFeaturesLib/SmmCpuFeaturesLib.c | 767 ------------------
.../Library/MmSaveStateLib/AmdMmSaveState.c | 309 +++++++
.../Library/MmSaveStateLib/IntelMmSaveState.c | 413 ++++++++++
.../MmSaveStateLib/MmSaveStateCommon.c | 138 ++++
.../SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.c | 387 +++++++++
.../IntelSmmCpuFeaturesLib.c | 70 ++
.../SmmCpuFeaturesLibCommon.c | 128 ---
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 11 +-
UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c | 500 +-----------
MdePkg/MdePkg.ci.yaml | 4 +-
24 files changed, 1812 insertions(+), 1508 deletions(-)
create mode 100644 UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveStateLib.inf
create mode 100644 UefiCpuPkg/Library/MmSaveStateLib/IntelMmSaveStateLib.inf
create mode 100644
UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
create mode 100644 MdePkg/Include/Register/Amd/SmramSaveStateMap.h
create mode 100644 UefiCpuPkg/Include/Library/MmSaveStateLib.h
create mode 100644 UefiCpuPkg/Library/MmSaveStateLib/MmSaveState.h
create mode 100644 UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c
create mode 100644 UefiCpuPkg/Library/MmSaveStateLib/IntelMmSaveState.c
create mode 100644 UefiCpuPkg/Library/MmSaveStateLib/MmSaveStateCommon.c
create mode 100644 UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.c
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#104427): https://edk2.groups.io/g/devel/message/104427
Mute This Topic: https://groups.io/mt/98720163/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
