Re: [edk2-devel] [PATCH v2 0/6] Adds AmdSmmCpuFeaturesLib

["Chang, Abner" <abner.chang@amd.com>]

[AMD Official Use Only - General]



> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Thursday, January 12, 2023 9:05 PM
> To: devel@edk2.groups.io; Attar, AbdulLateef (Abdul Lateef)
> <AbdulLateef.Attar@amd.com>
> Cc: Grimes, Paul <Paul.Grimes@amd.com>; Kirkendall, Garrett
> <Garrett.Kirkendall@amd.com>; Chang, Abner <Abner.Chang@amd.com>;
> Eric Dong <eric.dong@intel.com>; Ray Ni <ray.ni@intel.com>; Rahul Kumar
> <rahul1.kumar@intel.com>; Michael D Kinney
> <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>;
> Zhiguang Liu <zhiguang.liu@intel.com>; Gerd Hoffmann
> <kraxel@redhat.com>
> Subject: Re: [edk2-devel] [PATCH v2 0/6] Adds AmdSmmCpuFeaturesLib
> 
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
> 
> 
> On 1/11/23 07:15, Abdul Lateef Attar via groups.io wrote:
> > V2 series addresses review comments from V1.
> > Adds SmmSmramSaveStateLib library
> > Removes global references across modules.
> > drops S3 support patch.
> >
> > PR: https://github.com/tianocore/edk2/pull/3882
> >
> > 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: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> >
> > Abdul Lateef Attar (6):
> >   UefiCpuPkg/SmmCpuFeaturesLib: Restructure arch-dependent code
> >   MdePkg: Adds AMD SMRAM save state map
> >   UefiCpuPkg: Adds SmmSmramSaveStateLib library class
> >   UefiCpuPkg: Implements SmmSmramSaveStateLib library class
> >   UefiCpuPkg: Initial implementation of AMD's SmmCpuFeaturesLib
> >   UefiCpuPkg: Implements SmmCpuFeaturesLib for AMD Family
> >
> >  UefiCpuPkg/UefiCpuPkg.dec                     |   4 +
> >  UefiCpuPkg/UefiCpuPkg.dsc                     |  11 +
> >  .../AmdSmmCpuFeaturesLib.inf                  |  39 ++
> >  .../AmdSmmSmramSaveStateLib.inf               |  28 ++
> >  .../Include/Register/Amd/SmramSaveStateMap.h  | 194 ++++++++
> >  .../Include/Library/SmmSmramSaveStateLib.h    |  69 +++
> >  .../SmmSmramSaveStateLib/SmramSaveState.h     | 102 ++++
> >  .../SmmCpuFeaturesLib/Amd/SmmCpuFeaturesLib.c | 445
> ++++++++++++++++++
> >  .../IntelSmmCpuFeaturesLib.c                  | 128 +++++
> >  .../SmmCpuFeaturesLibCommon.c                 | 128 -----
> >  .../SmmSmramSaveStateLib/Amd/SmramSaveState.c | 318
> +++++++++++++
> >  .../SmramSaveStateCommon.c                    | 124 +++++
> >  MdePkg/MdePkg.ci.yaml                         |   3 +-
> >  13 files changed, 1464 insertions(+), 129 deletions(-)  create mode
> > 100644
> UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
> >  create mode 100644
> >
> UefiCpuPkg/Library/SmmSmramSaveStateLib/AmdSmmSmramSaveStateLib.
> inf
> >  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/Amd/SmmCpuFeaturesLib.c
> >  create mode 100644
> > UefiCpuPkg/Library/SmmSmramSaveStateLib/Amd/SmramSaveState.c
> >  create mode 100644
> > UefiCpuPkg/Library/SmmSmramSaveStateLib/SmramSaveStateCommon.c
> >
> 
> - Is there a feature BZ for this?
Here is the BZ number.
https://bugzilla.tianocore.org/show_bug.cgi?id=4182
We can put the BZ # on cover letter.
> 
> There should be. Explaining the goal of the patch set belongs there.
> Plus, if there is a feature BZ like that, it can be included in the Proposed
> Features list at <https://github.com/tianocore/tianocore.github.io/wiki/EDK-
> II-Release-Planning>,
> and then associated with the next release notes at
> <https://github.com/tianocore/edk2/releases>.
This is not a feature BZ. We had made the change to abstract Intel centric SmmCpuFeatureLib implementation. This patch set provides AMD implementation, there are some implementation differences between AMD and Intel on this library.
Thanks
Abner
> 
> - OVMF's SmmCpuFeaturesLib (OvmfPkg/Library/SmmCpuFeaturesLib)
> matches QEMU, and QEMU uses the AMD flavor of the save state map.
> 
> I don't think OVMF's library instance can be used for physical AMD processors
> whole-sale, so the library instances should remain separate.
> However, the register maps could be shared by some means, perhaps.
> 
> - Please copy Gerd on UefiCpuPkg patches, per commit 0aca5901e344
> ("Maintainers.txt: designate Gerd Hoffmann as UefiCpuPkg reviewer", 2023-
> 01-06).
> 
> Thanks for considering.
> Laszlo