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

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

[AMD Official Use Only - General]

Hi Ray,
        I just rebase the PR and it passed all CI tests.
Hope, this is sufficient, or do I need to re-submit the patch ?
Thanks
AbduL


-----Original Message-----
From: Ni, Ray <ray.ni@intel.com>
Sent: 13 January 2023 14:28
To: Laszlo Ersek <lersek@redhat.com>; 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>; Dong, Eric <eric.dong@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang <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.


It seems the patch set is not based on the edk2 master branch.
Can you confirm?

> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Thursday, January 12, 2023 9:05 PM
> To: devel@edk2.groups.io; abdattar@amd.com
> Cc: 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>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>;
> Liu, Zhiguang <zhiguang.liu@intel.com>; Gerd Hoffmann
> <kraxel@redhat.com>
> Subject: Re: [edk2-devel] [PATCH v2 0/6] Adds AmdSmmCpuFeaturesLib
>
> 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?
>
> 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>.
>
> - 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