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

["Attar, AbdulLateef (Abdul Lateef)" <AbdulLateef.Attar@amd.com>]

[AMD Official Use Only - General]

Got it, will merge these two patches(5/9 and 6/9) into single patch.

-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@intel.com>
Sent: 12 April 2023 00:19
To: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>; Chang, Abner <Abner.Chang@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>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: RE: [PATCH v8 0/9] Adds AmdSmmCpuFeaturesLib and SmmSmramSaveStateLib

[AMD Official Use Only - General]

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


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