Re: [edk2-devel] [PATCH 00/19] Remove some S3 related code in CpuS3.c of smm cpu driver
Good suggestions, Jiaxin! Thanks, Ray From: Tan, Dun Sent: Monday, May 13, 2024 11:37 To: Wu, Jiaxin ; devel@edk2.groups.io ; Ni, Ray Subject: RE: [edk2-devel] [PATCH 00/19] Remove some S3 related code in CpuS3.c of smm cpu driver 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 Sent: Monday, May 13, 2024 10:49 AM To: devel@edk2.groups.io; Tan, Dun ; Ni, Ray 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 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 > +++---
Re: [edk2-devel] [PATCH 00/19] Remove some S3 related code in CpuS3.c of smm cpu driver
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 Sent: Monday, May 13, 2024 10:49 AM To: devel@edk2.groups.io; Tan, Dun ; Ni, Ray 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 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
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 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 > Ue
[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 (#118792): https://edk2.groups.io/g/devel/message/118792 Mute This Topic: https://groups.io/mt/106018119/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/
