Thanks for the comments. Agree that the order adjustment can make the patch set clearer. Will modify the commits in next version patch set.
Thanks, Dun -----Original Message----- From: Wu, Jiaxin <jiaxin...@intel.com> Sent: Monday, May 13, 2024 10:49 AM To: devel@edk2.groups.io; Tan, Dun <dun....@intel.com>; Ni, Ray <ray...@intel.com> Subject: RE: [edk2-devel] [PATCH 00/19] Remove some S3 related code in CpuS3.c of smm cpu driver My comments as below: 1. I stronger recommend re-ordering the patches as below so as to combine all related sub-patches together, then we can more easy understand what we are target to resolving: 1) S3 MTRRs operation 2) AP page table unavailiable issue fix. 3) S3 ApHltLoopCode Operation 3) Register table cleanup 1) S3 MTRRs operation: > MdeModulePkg: Add gEdkiiS3MtrrSettingGuid > OvmfPkg: Save MTRR by lockbox in CpuS3DataDxe > UefiCpuPkg: Add locbox lib instance in DSC > UefiCpuPkg: Save MTRR by lockbox in CpuS3DataDxe > UefiCpuPkg: LoadMtrrData for all cpu in S3Resume > UefiCpuPkg: Remove code to load mtrr setting 2) AP page table unavailiable issue fix: > UefiCpuPkg: Disable PG in IA32 ApLoopCode 3) S3 ApHltLoopCode Operation: > UefiCpuPkg:Abstract some DxeMpLib code to function > UefiCpuPkg:Move some code in DxeMpLib to common place > UefiCpuPkg: Install gEdkiiEndOfS3ResumeGuid in S3Resume > UefiCpuPkg:Relocate AP to new safe buffer in PeiMpLib > UefiCpuPkg:Remove code to handle APIC setting and Interrupt > UefiCpuPkg:Rremove code to wakeup AP and relocate ap > UefiCpuPkg: Remove the duplicated mpservice locate > MdeModulePkg: remove MpService2Ppi field in SMM_S3_RESUME_STATE 4) Register table cleanup: > UefiCpuPkg:Set PcdCpuFeaturesInitOnS3Resume to TRUE > UefiCpuPkg: Remove code to set register table > UefiCpuPkg: Remove GetAcpiCpuData() in CpuS3.c 2. Update all intel copyright year. 3. Add all reviewer in the 00 patch. 4. Add PR in the 00 patch (if have). Thanks, Jiaxin > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of duntan > Sent: Friday, May 10, 2024 6:08 PM > To: devel@edk2.groups.io > Subject: [edk2-devel] [PATCH 00/19] Remove some S3 related code in > CpuS3.c of smm cpu driver > > This patch set is to remove some S3 related code in CpuS3.c of smm cpu > driver. It contain commits to: > 1.Save MTRR setting by lockbox in PEI phase 2.Load MTRR in S3Resume.c > before transferring to CpuS3.c in smm cpu driver. > 3.Add callback of gEdkiiEndOfS3ResumeGuid in PeiMpLib to relocate Ap > to new safe buffer. > 4.Install gEdkiiEndOfS3ResumeGuid in S3Resume.c before booting OS to > relocate APs 4.Change PcdCpuFeaturesInitOnS3Resume to TRUE to set > register table in CpuFeaturesPei 5.Remove code to set register > table/load mtrr/wakeup AP and relocate ap in CpuS3.c. > > With this patch set, CpuS3.c in smm CPU driver can be simplified. > > Dun Tan (18): > MdeModulePkg: Add gEdkiiS3MtrrSettingGuid > OvmfPkg: Save MTRR by lockbox in CpuS3DataDxe > UefiCpuPkg: Add locbox lib instance in DSC > UefiCpuPkg: Save MTRR by lockbox in CpuS3DataDxe > UefiCpuPkg: LoadMtrrData for all cpu in S3Resume > UefiCpuPkg: Remove the duplicated mpservice locate > UefiCpuPkg: Install gEdkiiEndOfS3ResumeGuid in S3Resume > UefiCpuPkg:Abstract some DxeMpLib code to function > UefiCpuPkg:Move some code in DxeMpLib to common place > UefiCpuPkg:Relocate AP to new safe buffer in PeiMpLib > UefiCpuPkg: Disable PG in IA32 ApLoopCode > UefiCpuPkg: Remove code to load mtrr setting > UefiCpuPkg:Set PcdCpuFeaturesInitOnS3Resume to TRUE > UefiCpuPkg: Remove code to set register table > UefiCpuPkg:Remove code to handle APIC setting and Interrupt > UefiCpuPkg:Rremove code to wakeup AP and relocate ap > UefiCpuPkg: Remove GetAcpiCpuData() in CpuS3.c > MdeModulePkg: remove MpService2Ppi field in SMM_S3_RESUME_STATE > > Ray Ni (1): > MdePkg: Add MmUnblockMemoryLib to MdeLibs.dsc > > MdeModulePkg/Include/Guid/AcpiS3Context.h | 1 - > MdeModulePkg/MdeModulePkg.dec | 3 +++ > MdePkg/MdeLibs.dsc.inc | 3 ++- > OvmfPkg/CpuS3DataDxe/CpuS3Data.c | 11 +++++++++++ > OvmfPkg/CpuS3DataDxe/CpuS3DataDxe.inf | 2 ++ > UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c | 11 +++++++++++ > UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf | 2 ++ > UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 181 > +++++++++++++++++++++++++++++++++++++++++++++++------------------- > ---------------------------------------------------------------------- > ------------------------ > ----------- > UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm | 4 ++++ > UefiCpuPkg/Library/MpInitLib/MpLib.c | 142 > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > ++++++++++++++++++ > UefiCpuPkg/Library/MpInitLib/MpLib.h | 54 > ++++++++++++++++++++++++++++++++++++++++++++++++++- > UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 4 ++++ > UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 152 > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > ++++++++++++++++++++++++++++ > UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 991 > ++++++++--------- > ---------------------------------------------------------------------- > ------------------------ > ---------------------------------------------------------------------- > ------------------------ > -- > UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/MpFuncs.nasm | 153 ------------ > ---------------------------------------------------------------------- > ------------------------ > --------------------------------------- > UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c | 27 ------------ > -------------- > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 6 ++---- > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 24 ----------- > ------------ > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 3 --- > UefiCpuPkg/PiSmmCpuDxeSmm/X64/MpFuncs.nasm | 189 ------------ > ---------------------------------------------------------------------- > ------------------------ > ------------------------------------------------------------------------- > UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c | 28 ------------ > --------------- > UefiCpuPkg/UefiCpuPkg.dec | 2 +- > UefiCpuPkg/UefiCpuPkg.dsc | 1 + > UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c | 76 > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- > ------------- > UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf | 2 ++ > 25 files changed, 511 insertions(+), 1561 deletions(-) delete mode > 100644 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/MpFuncs.nasm > delete mode 100644 UefiCpuPkg/PiSmmCpuDxeSmm/X64/MpFuncs.nasm > > -- > 2.31.1.windows.1 > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118852): https://edk2.groups.io/g/devel/message/118852 Mute This Topic: https://groups.io/mt/106018119/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-