Re: [PATCH v8 0/9] Adds AmdSmmCpuFeaturesLib and SmmSmramSaveStateLib

["Michael D Kinney" <michael.d.kinney@intel.com>]

Thanks for the feedback Abdul.  We usually do not see this in the history for a new lib instance.

I know we recommend developers start from a template of a lib instance if it is available
and then fill in the specific implementation, but those 2 steps do not go into commit
history.

Mike

> -----Original Message-----
> From: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>
> Sent: Monday, April 10, 2023 9:17 PM
> To: Chang, Abner <Abner.Chang@amd.com>; Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io
> Cc: Grimes, Paul <Paul.Grimes@amd.com>; Kirkendall, Garrett <Garrett.Kirkendall@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>; 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 v8 0/9] Adds AmdSmmCpuFeaturesLib and SmmSmramSaveStateLib
> 
> [AMD Official Use Only - General]
> 
> Hi Mike, Abner,
>         Patch 5/9 is blank implementation of SmmCpuFeaturesLib library class.
> Whereas patch 6/9 is actual implementation of the library for AMD processor family.
> I had separated in two different patches for easy to review.
> Thanks
> AbduL
> 
> -----Original Message-----
> From: Chang, Abner <Abner.Chang@amd.com>
> Sent: 11 April 2023 06:38
> To: Kinney, Michael D <michael.d.kinney@intel.com>; Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>;
> devel@edk2.groups.io
> Cc: Grimes, Paul <Paul.Grimes@amd.com>; Kirkendall, Garrett <Garrett.Kirkendall@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>; 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 v8 0/9] Adds AmdSmmCpuFeaturesLib and SmmSmramSaveStateLib
> 
> [AMD Official Use Only - General]
> 
> Hi Mike,
> I think 5/9 was added for those functions AMD has no implementation as the initial commit, while 6/9 was updated for the
> functions with AMD implementations.
> @Attar, AbdulLateef (Abdul Lateef)can confirm this.
> 
> Thanks
> Abner
> 
> > -----Original Message-----
> > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > Sent: Tuesday, April 11, 2023 12:29 AM
> > To: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>;
> > devel@edk2.groups.io
> > Cc: Grimes, Paul <Paul.Grimes@amd.com>; Kirkendall, Garrett
> > <Garrett.Kirkendall@amd.com>; Chang, Abner <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>;
> > 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>; Kinney, Michael D
> > <michael.d.kinney@intel.com>
> > Subject: RE: [PATCH v8 0/9] Adds AmdSmmCpuFeaturesLib and
> > SmmSmramSaveStateLib
> >
> > Caution: This message originated from an External Source. Use proper
> > caution when opening attachments, clicking links, or responding.
> >
> >
> > Is there a reason why patches 5 and 6 where not combined?
> >
> > Mike
> >
> > > -----Original Message-----
> > > From: Abdul Lateef Attar <abdattar@amd.com>
> > > Sent: Monday, April 10, 2023 4:10 AM
> > > To: devel@edk2.groups.io
> > > Cc: Abdul Lateef Attar <abdattar@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>; 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 v8 0/9] Adds AmdSmmCpuFeaturesLib and
> > > SmmSmramSaveStateLib
> > >
> > > PR: https://github.com/tianocore/edk2/pull/4258
> > >
> > > 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: 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>
> > > 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 (9):
> > >   MdePkg: Adds AMD SMRAM save state map
> > >   UefiCpuPkg: Adds SmmSmramSaveStateLib library class
> > >   UefiCpuPkg: Implements SmmSmramSaveStateLib library class
> > >   UefiCpuPkg/SmmCpuFeaturesLib: Restructure arch-dependent code
> > >   UefiCpuPkg: Initial implementation of AMD's SmmCpuFeaturesLib
> > >   UefiCpuPkg: Implements SmmCpuFeaturesLib for AMD Family
> > >   UefiCpuPkg: Implements SmmSmramSaveStateLib for Intel
> > >   UefiCpuPkg: Uses SmmSmramSaveStateLib library
> > >   OvmfPkg: Uses SmmSmramSaveStateLib library
> > >
> > >  UefiCpuPkg/UefiCpuPkg.dec                     |   4 +
> > >  OvmfPkg/OvmfPkgIa32X64.dsc                    |   2 +
> > >  UefiCpuPkg/UefiCpuPkg.dsc                     |  15 +
> > >  .../AmdSmmCpuFeaturesLib.inf                  |  38 ++
> > >  .../AmdSmmSmramSaveStateLib.inf               |  28 +
> > >  .../IntelSmmSmramSaveStateLib.inf             |  28 +
> > >  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf  |   2 +
> > >  .../Include/Register/Amd/SmramSaveStateMap.h  | 194 +++++++
> > >  .../Include/Library/SmmSmramSaveStateLib.h    |  70 +++
> > >  .../SmmSmramSaveStateLib/SmramSaveState.h     | 100 ++++
> > >  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h    |   2 +
> > >  .../SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.c  | 445
> > ++++++++++++++++
> > >  .../IntelSmmCpuFeaturesLib.c                  | 128 +++++
> > >  .../SmmCpuFeaturesLibCommon.c                 | 128 -----
> > >  .../SmmSmramSaveStateLib/AmdSmramSaveState.c  | 286 ++++++++++
> > >  .../IntelSmramSaveState.c                     | 359 +++++++++++++
> > >  .../SmramSaveStateCommon.c                    | 232 ++++++++
> > >  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c    |   5 +-
> > >  UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c    | 500 +----------------
> > -
> > >  MdePkg/MdePkg.ci.yaml                         |   4 +-
> > >  20 files changed, 1941 insertions(+), 629 deletions(-)  create mode
> > > 100644
> > UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
> > >  create mode 100644
> > >
> > UefiCpuPkg/Library/SmmSmramSaveStateLib/AmdSmmSmramSaveStateLib.
> > inf
> > >  create mode 100644
> > >
> > UefiCpuPkg/Library/SmmSmramSaveStateLib/IntelSmmSmramSaveStateLib.i
> > nf
> > >  create mode 100644
> > MdePkg/Include/Register/Amd/SmramSaveStateMap.h
> > >  create mode 100644
> > UefiCpuPkg/Include/Library/SmmSmramSaveStateLib.h
> > >  create mode 100644
> > > UefiCpuPkg/Library/SmmSmramSaveStateLib/SmramSaveState.h
> > >  create mode 100644
> > > UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.c
> > >  create mode 100644
> > > UefiCpuPkg/Library/SmmSmramSaveStateLib/AmdSmramSaveState.c
> > >  create mode 100644
> > > UefiCpuPkg/Library/SmmSmramSaveStateLib/IntelSmramSaveState.c
> > >  create mode 100644
> > > UefiCpuPkg/Library/SmmSmramSaveStateLib/SmramSaveStateCommon.c
> > >
> > > --
> > > 2.25.1