I spent some time to verify if the IntelMmSaveStateLib behaves the same as the
default implementation in CpuSmm driver.
The result looks good with following embedded test code in BSPHandler():
for (Cpu = 0; Cpu < mMaxNumberOfCpus; Cpu++) {
DEBUG ((DEBUG_INFO, "Testing SMRAM Read CPU[%d]...", Cpu));
for (Register = EFI_MM_SAVE_STATE_REGISTER_GDTBASE; Register <=
EFI_MM_SAVE_STATE_REGISTER_PROCESSOR_ID; Register++) {
DEBUG ((DEBUG_INFO, " Register(%d)...\n", Register));
switch (Register) {
case EFI_MM_SAVE_STATE_REGISTER_IO:
Width = sizeof (IoInfo);
break;
case EFI_MM_SAVE_STATE_REGISTER_LMA:
Width = 1;
break;
default:
Width = 8;
break;
}
Status = MmSaveStateReadRegister (Cpu, Register, Width, &IoInfo);
Status2 = ReadSaveStateRegister (Cpu, Register, Width, &IoInfo2);
ASSERT (Status == Status2);
if (!EFI_ERROR (Status)) {
ASSERT (CompareMem (&IoInfo, &IoInfo2, Width) == 0);
} else {
if (Status == EFI_INVALID_PARAMETER) {
Status = MmSaveStateReadRegister (Cpu, Register, sizeof (UINT32),
&IoInfo);
Status2 = ReadSaveStateRegister (Cpu, Register, sizeof (UINT32),
&IoInfo2);
ASSERT (Status == Status2);
ASSERT (Status == EFI_SUCCESS);
ASSERT (CompareMem (&IoInfo, &IoInfo2, sizeof (UINT32)) == 0);
}
}
}
}
But, one comment: EFI_SMM_SAVE_STATE_REGISTER_PROCESSOR_ID is not handled by
MmSaveStateLib. It's handled by
CpuSmm driver.
I am ok with that. Can you please update the MmSaveStateLib header file to
explicitly mention that? I am afraid I may forget
this without clear function header comments.
Others look good to me.
Thanks,
Ray
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chang, Abner
> via groups.io
> Sent: Tuesday, May 23, 2023 2:36 PM
> To: Ni, Ray <ray...@intel.com>; Attar, AbdulLateef (Abdul Lateef)
> <abdullateef.at...@amd.com>; devel@edk2.groups.io
> Cc: Attar, AbdulLateef (Abdul Lateef) <abdullateef.at...@amd.com>; Grimes,
> Paul <paul.gri...@amd.com>; Dong, Eric <eric.d...@intel.com>; Kumar, Rahul
> R <rahul.r.ku...@intel.com>; Gerd Hoffmann <kra...@redhat.com>; Kinney,
> Michael D <michael.d.kin...@intel.com>; Gao, Liming
> <gaolim...@byosoft.com.cn>; Liu, Zhiguang <zhiguang....@intel.com>; Ard
> Biesheuvel <ardb+tianoc...@kernel.org>; Yao, Jiewen <jiewen....@intel.com>;
> Justen, Jordan L <jordan.l.jus...@intel.com>
> Subject: Re: [edk2-devel] [PATCH v13 0/8] Adds AmdSmmCpuFeaturesLib and
> MmSaveStateLib
>
> [AMD Official Use Only - General]
>
> Let's talk to Liming to see if we have chance to make this included in 202305
> as
> the review process of this patch set looks to me close to finish.
>
> Thanks
> Abner
>
> > -----Original Message-----
> > From: Ni, Ray <ray...@intel.com>
> > Sent: Tuesday, May 23, 2023 2:30 PM
> > To: Chang, Abner <abner.ch...@amd.com>; Attar, AbdulLateef (Abdul
> > Lateef) <abdullateef.at...@amd.com>; devel@edk2.groups.io
> > Cc: Attar, AbdulLateef (Abdul Lateef) <abdullateef.at...@amd.com>; Grimes,
> > Paul <paul.gri...@amd.com>; Dong, Eric <eric.d...@intel.com>; Kumar,
> > Rahul R <rahul.r.ku...@intel.com>; Gerd Hoffmann <kra...@redhat.com>;
> > Kinney, Michael D <michael.d.kin...@intel.com>; Gao, Liming
> > <gaolim...@byosoft.com.cn>; Liu, Zhiguang <zhiguang....@intel.com>; Ard
> > Biesheuvel <ardb+tianoc...@kernel.org>; Yao, Jiewen
> > <jiewen....@intel.com>; Justen, Jordan L <jordan.l.jus...@intel.com>
> > Subject: RE: [PATCH v13 0/8] Adds AmdSmmCpuFeaturesLib and
> > MmSaveStateLib
> >
> > [AMD Official Use Only - General]
> >
> > Caution: This message originated from an External Source. Use proper caution
> > when opening attachments, clicking links, or responding.
> >
> >
> > Abner,
> > Sure. I will review the new patch set.
> >
> > But I am afraid this patch set cannot be included in 202305 stable release
> > because the edk2 repo
> > has been locked for quite a while for 202305 release and is about to
> > unlocked.
> >
> > Thanks,
> > Ray
> >
> > > -----Original Message-----
> > > From: Chang, Abner <abner.ch...@amd.com>
> > > Sent: Tuesday, May 23, 2023 2:28 PM
> > > To: Attar, AbdulLateef (Abdul Lateef) <abdullateef.at...@amd.com>;
> > > devel@edk2.groups.io; Ni, Ray <ray...@intel.com>
> > > Cc: Attar, AbdulLateef (Abdul Lateef) <abdullateef.at...@amd.com>;
> > Grimes,
> > > Paul <paul.gri...@amd.com>; Dong, Eric <eric.d...@intel.com>; Kumar,
> > Rahul
> > > R <rahul.r.ku...@intel.com>; Gerd Hoffmann <kra...@redhat.com>;
> > Kinney,
> > > Michael D <michael.d.kin...@intel.com>; Gao, Liming
> > > <gaolim...@byosoft.com.cn>; Liu, Zhiguang <zhiguang....@intel.com>; Ard
> > > Biesheuvel <ardb+tianoc...@kernel.org>; Yao, Jiewen
> > <jiewen....@intel.com>;
> > > Justen, Jordan L <jordan.l.jus...@intel.com>
> > > Subject: RE: [PATCH v13 0/8] Adds AmdSmmCpuFeaturesLib and
> > MmSaveStateLib
> > >
> > > [AMD Official Use Only - General]
> > >
> > > Hi @Ray Ni,
> > > We are almost there... We need your help to review this patch set as we
> > need
> > > this library to be part of 202305 stable release.
> > >
> > > Thanks
> > > Abner
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Abdul Lateef Attar <abdat...@amd.com>
> > > > Sent: Friday, May 12, 2023 8:32 PM
> > > > To: devel@edk2.groups.io
> > > > Cc: Attar, AbdulLateef (Abdul Lateef) <abdullateef.at...@amd.com>;
> > Grimes,
> > > > Paul <paul.gri...@amd.com>; Chang, Abner <abner.ch...@amd.com>;
> > Eric
> > > > Dong <eric.d...@intel.com>; Ray Ni <ray...@intel.com>; Rahul Kumar
> > > > <rahul1.ku...@intel.com>; Gerd Hoffmann <kra...@redhat.com>;
> > Michael D
> > > > Kinney <michael.d.kin...@intel.com>; Liming Gao
> > > > <gaolim...@byosoft.com.cn>; Zhiguang Liu <zhiguang....@intel.com>; Ard
> > > > Biesheuvel <ardb+tianoc...@kernel.org>; Jiewen Yao
> > > > <jiewen....@intel.com>; Jordan Justen <jordan.l.jus...@intel.com>
> > > > Subject: [PATCH v13 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/4392
> > > >
> > > > 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.gri...@amd.com>
> > > > Cc: Abner Chang <abner.ch...@amd.com>
> > > > Cc: Eric Dong <eric.d...@intel.com>
> > > > Cc: Ray Ni <ray...@intel.com>
> > > > Cc: Rahul Kumar <rahul1.ku...@intel.com>
> > > > Cc: Gerd Hoffmann <kra...@redhat.com>
> > > > Cc: Michael D Kinney <michael.d.kin...@intel.com>
> > > > Cc: Liming Gao <gaolim...@byosoft.com.cn>
> > > > Cc: Zhiguang Liu <zhiguang....@intel.com>
> > > > Cc: Ard Biesheuvel <ardb+tianoc...@kernel.org>
> > > > Cc: Jiewen Yao <jiewen....@intel.com>
> > > > Cc: Jordan Justen <jordan.l.jus...@intel.com>
> > > > Cc: Abdul Lateef Attar <abdat...@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 | 13 +
> > > > .../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 | 70 ++
> > > > .../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, 1806 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 (#105709): https://edk2.groups.io/g/devel/message/105709
Mute This Topic: https://groups.io/mt/98848071/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
