[v15] Adds AmdSmmCpuFeaturesLib and MmSaveStateLib

[edk2-devel] [PATCH v15 0/8] Adds AmdSmmCpuFeaturesLib and MmSaveStateLib

Posted by Abdul Lateef Attar via groups.io 10 months ago

Backward-compatibility changes:
  This patch series removes the SmmCpuFeaturesReadSaveStateRegister
  and SmmCpuFeaturesWriteSaveStateRegister interface/function.
  SmmReadSaveState() and SmmWriteSaveState() now directly invokes MmSaveStateLib
  routines to save/restore registers.

PR: https://github.com/tianocore/edk2/pull/4597

V15: Delta changes
  Rebase the branch and fix the merge conflicts.
V14: Delta changes
  Added @note to the MmSaveStateLib.h.
  SaveState(Read/Write) of EFI_SMM_SAVE_STATE_REGISTER_PROCESSOR_ID/EFI_MM_SAVE_STATE_REGISTER_PROCESSOR_ID
  is handled by PiSmmCpuDxeSmm driver.
  Fixed PatchCheck warnings.
V13: Delta changes
  Address review comments from Ray Ni
  Changed the BASE _NAME of AmdSmmCpuFeaturesLib.
  Removed EFIAPI from local function.
  Removed CpuIndex parameter from MmSaveStateGetRegisterLma
  Modifed MmSaveStateGetRegisterIndex () to accept RegOffset
    as second parameter.
  Removed FILE_GUID library instance for intel implemention from UefiCpuPkg.dsc.
V12:
  Addressed review comments from Michael.
  Added LibraryClasses to .inf file.
  removed duplicate MACRO definations.
  Moved related MACRO defination to respective file.
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.grimes@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>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Abdul Lateef Attar <abdattar@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                     |  12 +
 .../MmSaveStateLib/AmdMmSaveStateLib.inf      |  34 +
 .../MmSaveStateLib/IntelMmSaveStateLib.inf    |  34 +
 .../AmdSmmCpuFeaturesLib.inf                  |  38 +
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf  |   2 +
 .../Include/Register/Amd/SmramSaveStateMap.h  | 194 +++++
 UefiCpuPkg/Include/Library/MmSaveStateLib.h   |  74 ++
 .../Include/Library/SmmCpuFeaturesLib.h       |  52 --
 .../Library/MmSaveStateLib/MmSaveState.h      |  94 +++
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h    |  56 +-
 .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.c     | 767 ------------------
 .../Library/MmSaveStateLib/AmdMmSaveState.c   | 309 +++++++
 .../Library/MmSaveStateLib/IntelMmSaveState.c | 410 ++++++++++
 .../MmSaveStateLib/MmSaveStateCommon.c        | 132 +++
 .../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, 1809 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

-- 
2.25.1



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

Re: [edk2-devel] [PATCH v15 0/8] Adds AmdSmmCpuFeaturesLib and MmSaveStateLib

Posted by Ni, Ray 9 months, 4 weeks ago

Thanks for reposting.
I will merge it today through this PR: https://github.com/tianocore/edk2/pull/4599

I tried to use your PR but there is a merge-commit there that failed in CI PatchCheck step.

Thanks,
Ray

> -----Original Message-----
> From: Abdul Lateef Attar <abdattar@amd.com>
> Sent: Sunday, July 2, 2023 12:14 PM
> To: devel@edk2.groups.io
> Cc: Abdul Lateef Attar <abdattar@amd.com>; Paul Grimes
> <paul.grimes@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>; Gerd Hoffmann <kraxel@redhat.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Ard
> Biesheuvel <ardb+tianocore@kernel.org>; Yao, Jiewen <jiewen.yao@intel.com>;
> Justen, Jordan L <jordan.l.justen@intel.com>
> Subject: [PATCH v15 0/8] Adds AmdSmmCpuFeaturesLib and MmSaveStateLib
> 
> Backward-compatibility changes:
>   This patch series removes the SmmCpuFeaturesReadSaveStateRegister
>   and SmmCpuFeaturesWriteSaveStateRegister interface/function.
>   SmmReadSaveState() and SmmWriteSaveState() now directly invokes
> MmSaveStateLib
>   routines to save/restore registers.
> 
> PR: https://github.com/tianocore/edk2/pull/4597
> 
> V15: Delta changes
>   Rebase the branch and fix the merge conflicts.
> V14: Delta changes
>   Added @note to the MmSaveStateLib.h.
>   SaveState(Read/Write) of
> EFI_SMM_SAVE_STATE_REGISTER_PROCESSOR_ID/EFI_MM_SAVE_STATE_REGIS
> TER_PROCESSOR_ID
>   is handled by PiSmmCpuDxeSmm driver.
>   Fixed PatchCheck warnings.
> V13: Delta changes
>   Address review comments from Ray Ni
>   Changed the BASE _NAME of AmdSmmCpuFeaturesLib.
>   Removed EFIAPI from local function.
>   Removed CpuIndex parameter from MmSaveStateGetRegisterLma
>   Modifed MmSaveStateGetRegisterIndex () to accept RegOffset
>     as second parameter.
>   Removed FILE_GUID library instance for intel implemention from
> UefiCpuPkg.dsc.
> V12:
>   Addressed review comments from Michael.
>   Added LibraryClasses to .inf file.
>   removed duplicate MACRO definations.
>   Moved related MACRO defination to respective file.
> 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.grimes@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>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Abdul Lateef Attar <abdattar@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                     |  12 +
>  .../MmSaveStateLib/AmdMmSaveStateLib.inf      |  34 +
>  .../MmSaveStateLib/IntelMmSaveStateLib.inf    |  34 +
>  .../AmdSmmCpuFeaturesLib.inf                  |  38 +
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf  |   2 +
>  .../Include/Register/Amd/SmramSaveStateMap.h  | 194 +++++
>  UefiCpuPkg/Include/Library/MmSaveStateLib.h   |  74 ++
>  .../Include/Library/SmmCpuFeaturesLib.h       |  52 --
>  .../Library/MmSaveStateLib/MmSaveState.h      |  94 +++
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h    |  56 +-
>  .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.c     | 767 ------------------
>  .../Library/MmSaveStateLib/AmdMmSaveState.c   | 309 +++++++
>  .../Library/MmSaveStateLib/IntelMmSaveState.c | 410 ++++++++++
>  .../MmSaveStateLib/MmSaveStateCommon.c        | 132 +++
>  .../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, 1809 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
> 
> --
> 2.25.1



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

Re: [edk2-devel] [PATCH v15 0/8] Adds AmdSmmCpuFeaturesLib and MmSaveStateLib

Posted by Chang, Abner via groups.io 9 months, 4 weeks ago

[AMD Official Use Only - General]

Thanks Ray, just saw it got merged. 😊

Abner

> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Monday, July 3, 2023 10:00 AM
> To: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>;
> devel@edk2.groups.io
> Cc: Grimes, Paul <Paul.Grimes@amd.com>; Chang, Abner
> <Abner.Chang@amd.com>; Dong, Eric <eric.dong@intel.com>; Kumar, Rahul R
> <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Ard
> Biesheuvel <ardb+tianocore@kernel.org>; Yao, Jiewen
> <jiewen.yao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>
> Subject: RE: [PATCH v15 0/8] Adds AmdSmmCpuFeaturesLib and
> MmSaveStateLib
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Thanks for reposting.
> I will merge it today through this PR:
> https://github.com/tianocore/edk2/pull/4599
>
> I tried to use your PR but there is a merge-commit there that failed in CI
> PatchCheck step.
>
> Thanks,
> Ray
>
> > -----Original Message-----
> > From: Abdul Lateef Attar <abdattar@amd.com>
> > Sent: Sunday, July 2, 2023 12:14 PM
> > To: devel@edk2.groups.io
> > Cc: Abdul Lateef Attar <abdattar@amd.com>; Paul Grimes
> > <paul.grimes@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>; Gerd Hoffmann <kraxel@redhat.com>; Kinney,
> > Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Ard
> > Biesheuvel <ardb+tianocore@kernel.org>; Yao, Jiewen
> <jiewen.yao@intel.com>;
> > Justen, Jordan L <jordan.l.justen@intel.com>
> > Subject: [PATCH v15 0/8] Adds AmdSmmCpuFeaturesLib and
> MmSaveStateLib
> >
> > Backward-compatibility changes:
> >   This patch series removes the SmmCpuFeaturesReadSaveStateRegister
> >   and SmmCpuFeaturesWriteSaveStateRegister interface/function.
> >   SmmReadSaveState() and SmmWriteSaveState() now directly invokes
> > MmSaveStateLib
> >   routines to save/restore registers.
> >
> > PR: https://github.com/tianocore/edk2/pull/4597
> >
> > V15: Delta changes
> >   Rebase the branch and fix the merge conflicts.
> > V14: Delta changes
> >   Added @note to the MmSaveStateLib.h.
> >   SaveState(Read/Write) of
> >
> EFI_SMM_SAVE_STATE_REGISTER_PROCESSOR_ID/EFI_MM_SAVE_STATE_REG
> IS
> > TER_PROCESSOR_ID
> >   is handled by PiSmmCpuDxeSmm driver.
> >   Fixed PatchCheck warnings.
> > V13: Delta changes
> >   Address review comments from Ray Ni
> >   Changed the BASE _NAME of AmdSmmCpuFeaturesLib.
> >   Removed EFIAPI from local function.
> >   Removed CpuIndex parameter from MmSaveStateGetRegisterLma
> >   Modifed MmSaveStateGetRegisterIndex () to accept RegOffset
> >     as second parameter.
> >   Removed FILE_GUID library instance for intel implemention from
> > UefiCpuPkg.dsc.
> > V12:
> >   Addressed review comments from Michael.
> >   Added LibraryClasses to .inf file.
> >   removed duplicate MACRO definations.
> >   Moved related MACRO defination to respective file.
> > 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.grimes@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>
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > Cc: Abdul Lateef Attar <abdattar@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                     |  12 +
> >  .../MmSaveStateLib/AmdMmSaveStateLib.inf      |  34 +
> >  .../MmSaveStateLib/IntelMmSaveStateLib.inf    |  34 +
> >  .../AmdSmmCpuFeaturesLib.inf                  |  38 +
> >  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf  |   2 +
> >  .../Include/Register/Amd/SmramSaveStateMap.h  | 194 +++++
> >  UefiCpuPkg/Include/Library/MmSaveStateLib.h   |  74 ++
> >  .../Include/Library/SmmCpuFeaturesLib.h       |  52 --
> >  .../Library/MmSaveStateLib/MmSaveState.h      |  94 +++
> >  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h    |  56 +-
> >  .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.c     | 767 ------------------
> >  .../Library/MmSaveStateLib/AmdMmSaveState.c   | 309 +++++++
> >  .../Library/MmSaveStateLib/IntelMmSaveState.c | 410 ++++++++++
> >  .../MmSaveStateLib/MmSaveStateCommon.c        | 132 +++
> >  .../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, 1809 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
> >
> > --
> > 2.25.1



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

[edk2-devel] Use gMmst from MmServiceTableLib in MmSaveStateLib

Posted by duntan 9 months, 3 weeks ago

Hi Abdul,

In the new MmSaveStateLib created in this patch set, gSmst from SmmServiceTableLib is used. This causes that only DXE_SMM_DRIVER type module can consume this lib but MM_STANDALONE type module cannot.

In current edk2, there are different MmServicesTableLib and SmmServicesTableLib:
StadaloneMmServicesTableLib(MmServicesTableLib|MM_STANDALONE): initialize gMmst in standalone SMM env. 
                  MmServicesTableLib(MmServicesTableLib|DXE_SMM_DRIVER): initialize gMmst in legacy SMM env. 
SmmServicesTableLib(SmmServicesTableLib|DXE_SMM_DRIVER): initialize gSmst in legacy SMM env.

If MmSaveStateLib uses gMmst from MmServiceTableLib instead of gSmst, then MmSaveStateLib can be consumed by both DXE_SMM_DRIVER and MM_STANDALONE module.
Could you pls send patch to fix this?

Thanks,
Dun

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Abdul Lateef Attar via groups.io
Sent: Sunday, July 2, 2023 12:14 PM
To: devel@edk2.groups.io
Cc: Abdul Lateef Attar <abdattar@amd.com>; Paul Grimes <paul.grimes@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>; Gerd Hoffmann <kraxel@redhat.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Yao, Jiewen <jiewen.yao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>
Subject: [edk2-devel] [PATCH v15 0/8] Adds AmdSmmCpuFeaturesLib and MmSaveStateLib

Backward-compatibility changes:
  This patch series removes the SmmCpuFeaturesReadSaveStateRegister
  and SmmCpuFeaturesWriteSaveStateRegister interface/function.
  SmmReadSaveState() and SmmWriteSaveState() now directly invokes MmSaveStateLib
  routines to save/restore registers.

PR: https://github.com/tianocore/edk2/pull/4597

V15: Delta changes
  Rebase the branch and fix the merge conflicts.
V14: Delta changes
  Added @note to the MmSaveStateLib.h.
  SaveState(Read/Write) of EFI_SMM_SAVE_STATE_REGISTER_PROCESSOR_ID/EFI_MM_SAVE_STATE_REGISTER_PROCESSOR_ID
  is handled by PiSmmCpuDxeSmm driver.
  Fixed PatchCheck warnings.
V13: Delta changes
  Address review comments from Ray Ni
  Changed the BASE _NAME of AmdSmmCpuFeaturesLib.
  Removed EFIAPI from local function.
  Removed CpuIndex parameter from MmSaveStateGetRegisterLma
  Modifed MmSaveStateGetRegisterIndex () to accept RegOffset
    as second parameter.
  Removed FILE_GUID library instance for intel implemention from UefiCpuPkg.dsc.
V12:
  Addressed review comments from Michael.
  Added LibraryClasses to .inf file.
  removed duplicate MACRO definations.
  Moved related MACRO defination to respective file.
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.grimes@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>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Abdul Lateef Attar <abdattar@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                     |  12 +
 .../MmSaveStateLib/AmdMmSaveStateLib.inf      |  34 +
 .../MmSaveStateLib/IntelMmSaveStateLib.inf    |  34 +
 .../AmdSmmCpuFeaturesLib.inf                  |  38 +
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf  |   2 +
 .../Include/Register/Amd/SmramSaveStateMap.h  | 194 +++++
 UefiCpuPkg/Include/Library/MmSaveStateLib.h   |  74 ++
 .../Include/Library/SmmCpuFeaturesLib.h       |  52 --
 .../Library/MmSaveStateLib/MmSaveState.h      |  94 +++
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h    |  56 +-
 .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.c     | 767 ------------------
 .../Library/MmSaveStateLib/AmdMmSaveState.c   | 309 +++++++
 .../Library/MmSaveStateLib/IntelMmSaveState.c | 410 ++++++++++
 .../MmSaveStateLib/MmSaveStateCommon.c        | 132 +++
 .../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, 1809 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

--
2.25.1








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

Re: [edk2-devel] Use gMmst from MmServiceTableLib in MmSaveStateLib

Posted by Ni, Ray 9 months, 3 weeks ago

Abdul,
Can you please send a patch to fix MmSaveStateLib to use gMmst (instead of gSmst)?

Using gSmst forbids the lib instance be linked by standalone MM modules.

Thanks,
Ray

> -----Original Message-----
> From: Tan, Dun <dun.tan@intel.com>
> Sent: Wednesday, July 5, 2023 4:42 PM
> To: devel@edk2.groups.io; abdattar@amd.com
> Cc: Ni, Ray <ray.ni@intel.com>
> Subject: Use gMmst from MmServiceTableLib in MmSaveStateLib
> 
> Hi Abdul,
> 
> In the new MmSaveStateLib created in this patch set, gSmst from
> SmmServiceTableLib is used. This causes that only DXE_SMM_DRIVER type
> module can consume this lib but MM_STANDALONE type module cannot.
> 
> In current edk2, there are different MmServicesTableLib and
> SmmServicesTableLib:
> StadaloneMmServicesTableLib(MmServicesTableLib|MM_STANDALONE):
> initialize gMmst in standalone SMM env.
>                   MmServicesTableLib(MmServicesTableLib|DXE_SMM_DRIVER):
> initialize gMmst in legacy SMM env.
> SmmServicesTableLib(SmmServicesTableLib|DXE_SMM_DRIVER): initialize gSmst
> in legacy SMM env.
> 
> If MmSaveStateLib uses gMmst from MmServiceTableLib instead of gSmst, then
> MmSaveStateLib can be consumed by both DXE_SMM_DRIVER and
> MM_STANDALONE module.
> Could you pls send patch to fix this?
> 
> Thanks,
> Dun
> 
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Abdul Lateef
> Attar via groups.io
> Sent: Sunday, July 2, 2023 12:14 PM
> To: devel@edk2.groups.io
> Cc: Abdul Lateef Attar <abdattar@amd.com>; Paul Grimes
> <paul.grimes@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>; Gerd Hoffmann <kraxel@redhat.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Ard
> Biesheuvel <ardb+tianocore@kernel.org>; Yao, Jiewen <jiewen.yao@intel.com>;
> Justen, Jordan L <jordan.l.justen@intel.com>
> Subject: [edk2-devel] [PATCH v15 0/8] Adds AmdSmmCpuFeaturesLib and
> MmSaveStateLib
> 
> Backward-compatibility changes:
>   This patch series removes the SmmCpuFeaturesReadSaveStateRegister
>   and SmmCpuFeaturesWriteSaveStateRegister interface/function.
>   SmmReadSaveState() and SmmWriteSaveState() now directly invokes
> MmSaveStateLib
>   routines to save/restore registers.
> 
> PR: https://github.com/tianocore/edk2/pull/4597
> 
> V15: Delta changes
>   Rebase the branch and fix the merge conflicts.
> V14: Delta changes
>   Added @note to the MmSaveStateLib.h.
>   SaveState(Read/Write) of
> EFI_SMM_SAVE_STATE_REGISTER_PROCESSOR_ID/EFI_MM_SAVE_STATE_REGIS
> TER_PROCESSOR_ID
>   is handled by PiSmmCpuDxeSmm driver.
>   Fixed PatchCheck warnings.
> V13: Delta changes
>   Address review comments from Ray Ni
>   Changed the BASE _NAME of AmdSmmCpuFeaturesLib.
>   Removed EFIAPI from local function.
>   Removed CpuIndex parameter from MmSaveStateGetRegisterLma
>   Modifed MmSaveStateGetRegisterIndex () to accept RegOffset
>     as second parameter.
>   Removed FILE_GUID library instance for intel implemention from
> UefiCpuPkg.dsc.
> V12:
>   Addressed review comments from Michael.
>   Added LibraryClasses to .inf file.
>   removed duplicate MACRO definations.
>   Moved related MACRO defination to respective file.
> 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.grimes@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>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Abdul Lateef Attar <abdattar@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                     |  12 +
>  .../MmSaveStateLib/AmdMmSaveStateLib.inf      |  34 +
>  .../MmSaveStateLib/IntelMmSaveStateLib.inf    |  34 +
>  .../AmdSmmCpuFeaturesLib.inf                  |  38 +
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf  |   2 +
>  .../Include/Register/Amd/SmramSaveStateMap.h  | 194 +++++
>  UefiCpuPkg/Include/Library/MmSaveStateLib.h   |  74 ++
>  .../Include/Library/SmmCpuFeaturesLib.h       |  52 --
>  .../Library/MmSaveStateLib/MmSaveState.h      |  94 +++
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h    |  56 +-
>  .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.c     | 767 ------------------
>  .../Library/MmSaveStateLib/AmdMmSaveState.c   | 309 +++++++
>  .../Library/MmSaveStateLib/IntelMmSaveState.c | 410 ++++++++++
>  .../MmSaveStateLib/MmSaveStateCommon.c        | 132 +++
>  .../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, 1809 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
> 
> --
> 2.25.1
> 
> 
> 
> 
> 



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

Re: [edk2-devel] Use gMmst from MmServiceTableLib in MmSaveStateLib

Posted by Attar, AbdulLateef (Abdul Lateef) via groups.io 9 months, 3 weeks ago

[AMD Official Use Only - General]

Hi Ray,
        I think Michael raised the similar concerned during the patch review.
Its intentionally kept it as gSmst because of the below reason.

2. AmdMmSaveStateLib and IntelMmSaveStateLib depend on SmmServicesTableLib. Can they depend on MmServicesTableLib instead?
[Attar, AbdulLateef (Abdul Lateef)] MmSaveStateLib is mainly used by PiSmmCpuDxeSmm driver which still uses Smm convention and preserve the SAVE_STATE pointed by gSmst(Instead of gMmst).
Hence I don't think we can move to MmServicesTableLib.


Thanks
AbduL

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com>
Sent: Tuesday, July 11, 2023 1:40 PM
To: Tan, Dun <dun.tan@intel.com>; devel@edk2.groups.io; Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>; Chang, Abner <Abner.Chang@amd.com>
Subject: RE: Use gMmst from MmServiceTableLib in MmSaveStateLib

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


Abdul,
Can you please send a patch to fix MmSaveStateLib to use gMmst (instead of gSmst)?

Using gSmst forbids the lib instance be linked by standalone MM modules.

Thanks,
Ray

> -----Original Message-----
> From: Tan, Dun <dun.tan@intel.com>
> Sent: Wednesday, July 5, 2023 4:42 PM
> To: devel@edk2.groups.io; abdattar@amd.com
> Cc: Ni, Ray <ray.ni@intel.com>
> Subject: Use gMmst from MmServiceTableLib in MmSaveStateLib
>
> Hi Abdul,
>
> In the new MmSaveStateLib created in this patch set, gSmst from
> SmmServiceTableLib is used. This causes that only DXE_SMM_DRIVER type
> module can consume this lib but MM_STANDALONE type module cannot.
>
> In current edk2, there are different MmServicesTableLib and
> SmmServicesTableLib:
> StadaloneMmServicesTableLib(MmServicesTableLib|MM_STANDALONE):
> initialize gMmst in standalone SMM env.
>                   MmServicesTableLib(MmServicesTableLib|DXE_SMM_DRIVER):
> initialize gMmst in legacy SMM env.
> SmmServicesTableLib(SmmServicesTableLib|DXE_SMM_DRIVER): initialize
> gSmst in legacy SMM env.
>
> If MmSaveStateLib uses gMmst from MmServiceTableLib instead of gSmst,
> then MmSaveStateLib can be consumed by both DXE_SMM_DRIVER and
> MM_STANDALONE module.
> Could you pls send patch to fix this?
>
> Thanks,
> Dun
>
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Abdul
> Lateef Attar via groups.io
> Sent: Sunday, July 2, 2023 12:14 PM
> To: devel@edk2.groups.io
> Cc: Abdul Lateef Attar <abdattar@amd.com>; Paul Grimes
> <paul.grimes@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>; Gerd Hoffmann <kraxel@redhat.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>;
> Ard Biesheuvel <ardb+tianocore@kernel.org>; Yao, Jiewen
> <jiewen.yao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>
> Subject: [edk2-devel] [PATCH v15 0/8] Adds AmdSmmCpuFeaturesLib and
> MmSaveStateLib
>
> Backward-compatibility changes:
>   This patch series removes the SmmCpuFeaturesReadSaveStateRegister
>   and SmmCpuFeaturesWriteSaveStateRegister interface/function.
>   SmmReadSaveState() and SmmWriteSaveState() now directly invokes
> MmSaveStateLib
>   routines to save/restore registers.
>
> PR: https://github.com/tianocore/edk2/pull/4597
>
> V15: Delta changes
>   Rebase the branch and fix the merge conflicts.
> V14: Delta changes
>   Added @note to the MmSaveStateLib.h.
>   SaveState(Read/Write) of
> EFI_SMM_SAVE_STATE_REGISTER_PROCESSOR_ID/EFI_MM_SAVE_STATE_REGIS
> TER_PROCESSOR_ID
>   is handled by PiSmmCpuDxeSmm driver.
>   Fixed PatchCheck warnings.
> V13: Delta changes
>   Address review comments from Ray Ni
>   Changed the BASE _NAME of AmdSmmCpuFeaturesLib.
>   Removed EFIAPI from local function.
>   Removed CpuIndex parameter from MmSaveStateGetRegisterLma
>   Modifed MmSaveStateGetRegisterIndex () to accept RegOffset
>     as second parameter.
>   Removed FILE_GUID library instance for intel implemention from
> UefiCpuPkg.dsc.
> V12:
>   Addressed review comments from Michael.
>   Added LibraryClasses to .inf file.
>   removed duplicate MACRO definations.
>   Moved related MACRO defination to respective file.
> 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.grimes@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>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Abdul Lateef Attar <abdattar@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                     |  12 +
>  .../MmSaveStateLib/AmdMmSaveStateLib.inf      |  34 +
>  .../MmSaveStateLib/IntelMmSaveStateLib.inf    |  34 +
>  .../AmdSmmCpuFeaturesLib.inf                  |  38 +
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf  |   2 +
>  .../Include/Register/Amd/SmramSaveStateMap.h  | 194 +++++
>  UefiCpuPkg/Include/Library/MmSaveStateLib.h   |  74 ++
>  .../Include/Library/SmmCpuFeaturesLib.h       |  52 --
>  .../Library/MmSaveStateLib/MmSaveState.h      |  94 +++
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h    |  56 +-
>  .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.c     | 767 ------------------
>  .../Library/MmSaveStateLib/AmdMmSaveState.c   | 309 +++++++
>  .../Library/MmSaveStateLib/IntelMmSaveState.c | 410 ++++++++++
>  .../MmSaveStateLib/MmSaveStateCommon.c        | 132 +++
>  .../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, 1809 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
>
> --
> 2.25.1
>
>
>
> 
>



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

Re: [edk2-devel] Use gMmst from MmServiceTableLib in MmSaveStateLib

Posted by duntan 9 months, 3 weeks ago

AbduL,

For the part ' preserve the SAVE_STATE pointed by gSmst(Instead of gMmst)', do you mean the following code in RestoreSmmConfigurationInS3()?
gSmst->CpuSaveState          = gSmmCpuPrivate->SmmCoreEntryContext.CpuSaveState;

Acctually when PiSmmCpuDxeSmm uses MmServicesTableLib, the gSmst is the same as gMmst. You can check the lib constructor MmServicesTableLibConstructor() and SmmServicesTableLibConstructor. In the two constructor functions, they both locate the same global pointer by gEfiSmmBase2ProtocolGuid/ gEfiMmBaseProtocolGuid(same GUID)  and assign the value to gSmst or gMmst. In this case the only difference of gMmst and gSmst is the naming. So I think  PiSmmCpuDxeSmm can still consume the MmSaveStateLib even MmSaveStateLib use gMmst.

Thanks,
Dun
-----Original Message-----
From: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com> 
Sent: Tuesday, July 11, 2023 5:06 PM
To: Ni, Ray <ray.ni@intel.com>; Tan, Dun <dun.tan@intel.com>; devel@edk2.groups.io; Chang, Abner <Abner.Chang@amd.com>
Subject: RE: Use gMmst from MmServiceTableLib in MmSaveStateLib

[AMD Official Use Only - General]

Hi Ray,
        I think Michael raised the similar concerned during the patch review.
Its intentionally kept it as gSmst because of the below reason.

2. AmdMmSaveStateLib and IntelMmSaveStateLib depend on SmmServicesTableLib. Can they depend on MmServicesTableLib instead?
[Attar, AbdulLateef (Abdul Lateef)] MmSaveStateLib is mainly used by PiSmmCpuDxeSmm driver which still uses Smm convention and preserve the SAVE_STATE pointed by gSmst(Instead of gMmst).
Hence I don't think we can move to MmServicesTableLib.


Thanks
AbduL

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com>
Sent: Tuesday, July 11, 2023 1:40 PM
To: Tan, Dun <dun.tan@intel.com>; devel@edk2.groups.io; Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>; Chang, Abner <Abner.Chang@amd.com>
Subject: RE: Use gMmst from MmServiceTableLib in MmSaveStateLib

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


Abdul,
Can you please send a patch to fix MmSaveStateLib to use gMmst (instead of gSmst)?

Using gSmst forbids the lib instance be linked by standalone MM modules.

Thanks,
Ray

> -----Original Message-----
> From: Tan, Dun <dun.tan@intel.com>
> Sent: Wednesday, July 5, 2023 4:42 PM
> To: devel@edk2.groups.io; abdattar@amd.com
> Cc: Ni, Ray <ray.ni@intel.com>
> Subject: Use gMmst from MmServiceTableLib in MmSaveStateLib
>
> Hi Abdul,
>
> In the new MmSaveStateLib created in this patch set, gSmst from 
> SmmServiceTableLib is used. This causes that only DXE_SMM_DRIVER type 
> module can consume this lib but MM_STANDALONE type module cannot.
>
> In current edk2, there are different MmServicesTableLib and
> SmmServicesTableLib:
> StadaloneMmServicesTableLib(MmServicesTableLib|MM_STANDALONE):
> initialize gMmst in standalone SMM env.
>                   MmServicesTableLib(MmServicesTableLib|DXE_SMM_DRIVER):
> initialize gMmst in legacy SMM env.
> SmmServicesTableLib(SmmServicesTableLib|DXE_SMM_DRIVER): initialize 
> gSmst in legacy SMM env.
>
> If MmSaveStateLib uses gMmst from MmServiceTableLib instead of gSmst, 
> then MmSaveStateLib can be consumed by both DXE_SMM_DRIVER and 
> MM_STANDALONE module.
> Could you pls send patch to fix this?
>
> Thanks,
> Dun
>
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Abdul 
> Lateef Attar via groups.io
> Sent: Sunday, July 2, 2023 12:14 PM
> To: devel@edk2.groups.io
> Cc: Abdul Lateef Attar <abdattar@amd.com>; Paul Grimes 
> <paul.grimes@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>; Gerd Hoffmann <kraxel@redhat.com>; Kinney, 
> Michael D <michael.d.kinney@intel.com>; Gao, Liming 
> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; 
> Ard Biesheuvel <ardb+tianocore@kernel.org>; Yao, Jiewen 
> <jiewen.yao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>
> Subject: [edk2-devel] [PATCH v15 0/8] Adds AmdSmmCpuFeaturesLib and 
> MmSaveStateLib
>
> Backward-compatibility changes:
>   This patch series removes the SmmCpuFeaturesReadSaveStateRegister
>   and SmmCpuFeaturesWriteSaveStateRegister interface/function.
>   SmmReadSaveState() and SmmWriteSaveState() now directly invokes 
> MmSaveStateLib
>   routines to save/restore registers.
>
> PR: https://github.com/tianocore/edk2/pull/4597
>
> V15: Delta changes
>   Rebase the branch and fix the merge conflicts.
> V14: Delta changes
>   Added @note to the MmSaveStateLib.h.
>   SaveState(Read/Write) of
> EFI_SMM_SAVE_STATE_REGISTER_PROCESSOR_ID/EFI_MM_SAVE_STATE_REGIS
> TER_PROCESSOR_ID
>   is handled by PiSmmCpuDxeSmm driver.
>   Fixed PatchCheck warnings.
> V13: Delta changes
>   Address review comments from Ray Ni
>   Changed the BASE _NAME of AmdSmmCpuFeaturesLib.
>   Removed EFIAPI from local function.
>   Removed CpuIndex parameter from MmSaveStateGetRegisterLma
>   Modifed MmSaveStateGetRegisterIndex () to accept RegOffset
>     as second parameter.
>   Removed FILE_GUID library instance for intel implemention from 
> UefiCpuPkg.dsc.
> V12:
>   Addressed review comments from Michael.
>   Added LibraryClasses to .inf file.
>   removed duplicate MACRO definations.
>   Moved related MACRO defination to respective file.
> 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.grimes@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>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Abdul Lateef Attar <abdattar@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                     |  12 +
>  .../MmSaveStateLib/AmdMmSaveStateLib.inf      |  34 +
>  .../MmSaveStateLib/IntelMmSaveStateLib.inf    |  34 +
>  .../AmdSmmCpuFeaturesLib.inf                  |  38 +
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf  |   2 +
>  .../Include/Register/Amd/SmramSaveStateMap.h  | 194 +++++
>  UefiCpuPkg/Include/Library/MmSaveStateLib.h   |  74 ++
>  .../Include/Library/SmmCpuFeaturesLib.h       |  52 --
>  .../Library/MmSaveStateLib/MmSaveState.h      |  94 +++
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h    |  56 +-
>  .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.c     | 767 ------------------
>  .../Library/MmSaveStateLib/AmdMmSaveState.c   | 309 +++++++
>  .../Library/MmSaveStateLib/IntelMmSaveState.c | 410 ++++++++++
>  .../MmSaveStateLib/MmSaveStateCommon.c        | 132 +++
>  .../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, 1809 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
>
> --
> 2.25.1
>
>
>
> 
>



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

Re: [edk2-devel] Use gMmst from MmServiceTableLib in MmSaveStateLib

Posted by Attar, AbdulLateef (Abdul Lateef) via groups.io 9 months, 3 weeks ago

[AMD Official Use Only - General]

Thanks Dun for clarifying, I'll submit the patch.

Regards,
AbduL

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of duntan via groups.io
Sent: Tuesday, July 11, 2023 3:21 PM
To: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>; Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; Chang, Abner <Abner.Chang@amd.com>
Subject: Re: [edk2-devel] Use gMmst from MmServiceTableLib in MmSaveStateLib

[AMD Official Use Only - General]

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


AbduL,

For the part ' preserve the SAVE_STATE pointed by gSmst(Instead of gMmst)', do you mean the following code in RestoreSmmConfigurationInS3()?
gSmst->CpuSaveState          = gSmmCpuPrivate->SmmCoreEntryContext.CpuSaveState;

Acctually when PiSmmCpuDxeSmm uses MmServicesTableLib, the gSmst is the same as gMmst. You can check the lib constructor MmServicesTableLibConstructor() and SmmServicesTableLibConstructor. In the two constructor functions, they both locate the same global pointer by gEfiSmmBase2ProtocolGuid/ gEfiMmBaseProtocolGuid(same GUID)  and assign the value to gSmst or gMmst. In this case the only difference of gMmst and gSmst is the naming. So I think  PiSmmCpuDxeSmm can still consume the MmSaveStateLib even MmSaveStateLib use gMmst.

Thanks,
Dun
-----Original Message-----
From: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>
Sent: Tuesday, July 11, 2023 5:06 PM
To: Ni, Ray <ray.ni@intel.com>; Tan, Dun <dun.tan@intel.com>; devel@edk2.groups.io; Chang, Abner <Abner.Chang@amd.com>
Subject: RE: Use gMmst from MmServiceTableLib in MmSaveStateLib

[AMD Official Use Only - General]

Hi Ray,
        I think Michael raised the similar concerned during the patch review.
Its intentionally kept it as gSmst because of the below reason.

2. AmdMmSaveStateLib and IntelMmSaveStateLib depend on SmmServicesTableLib. Can they depend on MmServicesTableLib instead?
[Attar, AbdulLateef (Abdul Lateef)] MmSaveStateLib is mainly used by PiSmmCpuDxeSmm driver which still uses Smm convention and preserve the SAVE_STATE pointed by gSmst(Instead of gMmst).
Hence I don't think we can move to MmServicesTableLib.


Thanks
AbduL

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com>
Sent: Tuesday, July 11, 2023 1:40 PM
To: Tan, Dun <dun.tan@intel.com>; devel@edk2.groups.io; Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>; Chang, Abner <Abner.Chang@amd.com>
Subject: RE: Use gMmst from MmServiceTableLib in MmSaveStateLib

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


Abdul,
Can you please send a patch to fix MmSaveStateLib to use gMmst (instead of gSmst)?

Using gSmst forbids the lib instance be linked by standalone MM modules.

Thanks,
Ray

> -----Original Message-----
> From: Tan, Dun <dun.tan@intel.com>
> Sent: Wednesday, July 5, 2023 4:42 PM
> To: devel@edk2.groups.io; abdattar@amd.com
> Cc: Ni, Ray <ray.ni@intel.com>
> Subject: Use gMmst from MmServiceTableLib in MmSaveStateLib
>
> Hi Abdul,
>
> In the new MmSaveStateLib created in this patch set, gSmst from
> SmmServiceTableLib is used. This causes that only DXE_SMM_DRIVER type
> module can consume this lib but MM_STANDALONE type module cannot.
>
> In current edk2, there are different MmServicesTableLib and
> SmmServicesTableLib:
> StadaloneMmServicesTableLib(MmServicesTableLib|MM_STANDALONE):
> initialize gMmst in standalone SMM env.
>                   MmServicesTableLib(MmServicesTableLib|DXE_SMM_DRIVER):
> initialize gMmst in legacy SMM env.
> SmmServicesTableLib(SmmServicesTableLib|DXE_SMM_DRIVER): initialize
> gSmst in legacy SMM env.
>
> If MmSaveStateLib uses gMmst from MmServiceTableLib instead of gSmst,
> then MmSaveStateLib can be consumed by both DXE_SMM_DRIVER and
> MM_STANDALONE module.
> Could you pls send patch to fix this?
>
> Thanks,
> Dun
>
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Abdul
> Lateef Attar via groups.io
> Sent: Sunday, July 2, 2023 12:14 PM
> To: devel@edk2.groups.io
> Cc: Abdul Lateef Attar <abdattar@amd.com>; Paul Grimes
> <paul.grimes@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>; Gerd Hoffmann <kraxel@redhat.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>;
> Ard Biesheuvel <ardb+tianocore@kernel.org>; Yao, Jiewen
> <jiewen.yao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>
> Subject: [edk2-devel] [PATCH v15 0/8] Adds AmdSmmCpuFeaturesLib and
> MmSaveStateLib
>
> Backward-compatibility changes:
>   This patch series removes the SmmCpuFeaturesReadSaveStateRegister
>   and SmmCpuFeaturesWriteSaveStateRegister interface/function.
>   SmmReadSaveState() and SmmWriteSaveState() now directly invokes
> MmSaveStateLib
>   routines to save/restore registers.
>
> PR: https://github.com/tianocore/edk2/pull/4597
>
> V15: Delta changes
>   Rebase the branch and fix the merge conflicts.
> V14: Delta changes
>   Added @note to the MmSaveStateLib.h.
>   SaveState(Read/Write) of
> EFI_SMM_SAVE_STATE_REGISTER_PROCESSOR_ID/EFI_MM_SAVE_STATE_REGIS
> TER_PROCESSOR_ID
>   is handled by PiSmmCpuDxeSmm driver.
>   Fixed PatchCheck warnings.
> V13: Delta changes
>   Address review comments from Ray Ni
>   Changed the BASE _NAME of AmdSmmCpuFeaturesLib.
>   Removed EFIAPI from local function.
>   Removed CpuIndex parameter from MmSaveStateGetRegisterLma
>   Modifed MmSaveStateGetRegisterIndex () to accept RegOffset
>     as second parameter.
>   Removed FILE_GUID library instance for intel implemention from
> UefiCpuPkg.dsc.
> V12:
>   Addressed review comments from Michael.
>   Added LibraryClasses to .inf file.
>   removed duplicate MACRO definations.
>   Moved related MACRO defination to respective file.
> 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.grimes@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>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Abdul Lateef Attar <abdattar@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                     |  12 +
>  .../MmSaveStateLib/AmdMmSaveStateLib.inf      |  34 +
>  .../MmSaveStateLib/IntelMmSaveStateLib.inf    |  34 +
>  .../AmdSmmCpuFeaturesLib.inf                  |  38 +
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf  |   2 +
>  .../Include/Register/Amd/SmramSaveStateMap.h  | 194 +++++
>  UefiCpuPkg/Include/Library/MmSaveStateLib.h   |  74 ++
>  .../Include/Library/SmmCpuFeaturesLib.h       |  52 --
>  .../Library/MmSaveStateLib/MmSaveState.h      |  94 +++
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h    |  56 +-
>  .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.c     | 767 ------------------
>  .../Library/MmSaveStateLib/AmdMmSaveState.c   | 309 +++++++
>  .../Library/MmSaveStateLib/IntelMmSaveState.c | 410 ++++++++++
>  .../MmSaveStateLib/MmSaveStateCommon.c        | 132 +++
>  .../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, 1809 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
>
> --
> 2.25.1
>
>
>
>
>








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