S3 cleanup #5606
Merged
S3 cleanup #5606
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
td36
force-pushed
the
s3_cleanup_upstream
branch
from
749343e
to
63a9371
Compare
niruiyu
reviewed
Apr 30, 2024
|
PR can not be merged due to conflict. Please rebase and resubmit |
jiaxinwu
reviewed
May 9, 2024
|
Below patch should after "[UefiCpuPkg:Move some code in DxeMpLib to common place]" and before "[UefiCpuPkg: Relocate AP to new safe buffer in PeiMpLib]": |
jiaxinwu
reviewed
May 9, 2024
td36
force-pushed
the
s3_cleanup_upstream
branch
2 times, most recently
from
22424d5
to
4287f2b
Compare
I think the current order is ok. Installation of gEdkiiEndOfS3ResumeGuid doesn't impact anything till the callback of this GUID is registered in PeiMplib. |
td36
force-pushed
the
s3_cleanup_upstream
branch
2 times, most recently
from
1f18bf1
to
4ff6f28
Compare
jiaxinwu
approved these changes
Jun 3, 2024
|
hi @niruiyu @lgao4 @ardbiesheuvel, could you please help to review and merge the PR? The patch has been reviewed-by you in in previous edk2 community mail loop. |
ardbiesheuvel
approved these changes
Jun 3, 2024
Add gEdkiiS3MtrrSettingGuid a new GUID for s3 MTRR setting. This GUID will be used to save MTRR_SETTINGS at EndOfDxe by LockBox and restore at S3 boot PEI phase for s3 usage. Signed-off-by: Dun Tan <dun.tan@intel.com> Reviewed-by: Ray Ni <ray.ni@intel.com> Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn> Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
Save MTRR by lockbox in CpuS3DataDxe. In S3 boot, The MTRR setting will be restored in S3Resume.c in following patches. Then S3Resume.c will wakeup all APs to load the MTRR setting. This can avoid waking up APs in CpuS3.c. Signed-off-by: Dun Tan <dun.tan@intel.com> Reviewed-by: Ard Biesheuvel <ardb+tianocore@kernel.org> Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Ray Ni <ray.ni@intel.com> Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
Add locbox lib instance in DSC. The SmmLockBoxDxeLib will be consumed by CpuS3DataDxe driver Signed-off-by: Dun Tan <dun.tan@intel.com> Reviewed-by: Ray Ni <ray.ni@intel.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
Save MTRR by lockbox in CpuS3DataDxe. In S3 boot, The MTRR setting will be restored in S3Resume.c in following patches. Then S3Resume.c will wakeup all APs to load the MTRR setting. This can avoid waking up APs in CpuS3.c. Signed-off-by: Dun Tan <dun.tan@intel.com> Reviewed-by: Ray Ni <ray.ni@intel.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
In this commit, S3Resume.c wakeup all Aps to run LoadMtrrData for all cpu before transfer to CpuS3.c in smm cpu driver. The MtrrSetting table can be restored by gEdkiiS3MtrrSettingGuid which is saved by lockbox in PEI phase. This can avoid waking up APs in CpuS3.c. Signed-off-by: Dun Tan <dun.tan@intel.com> Reviewed-by: Ray Ni <ray.ni@intel.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
Remove code to load mtrr setting in CpuS3.c. In previous commits, before transferring to CpuS3.c, MTRR setting has been loaded in S3RestoreConfig2() for all CPU. Signed-off-by: Dun Tan <dun.tan@intel.com> Reviewed-by: Ray Ni <ray.ni@intel.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
Set PcdCpuFeaturesInitOnS3Resume to TRUE. So that CpuFeaturesPei PEIM will initialize the CPU registers and perform CPU features initialization. Signed-off-by: Dun Tan <dun.tan@intel.com> Reviewed-by: Ray Ni <ray.ni@intel.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
Remove code to set register table in CpuS3.c. In previous commit, PcdCpuFeaturesInitOnS3Resume has been set to TRUE. So that CpuFeaturesPei PEIM will initialize the CPU registers and perform CPU features initialization. Signed-off-by: Dun Tan <dun.tan@intel.com> Reviewed-by: Ray Ni <ray.ni@intel.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
Disable paging in IA32 RelocateApLoop assembly code to fix the issue that the AP page table is unavailiable after boot OS under IA32 execution mode. This issue exist in IA32 PEI + IA32 DXE normal boot (also S3 boot with IA32 PEI after previous three commits are accepted). In current MpLib code, the IA32 execution mode code did not create page table in reserved memory like what X64 code did. If PcdCpuStackGuard is TRUE, the PG is enabled for AP in current RelocateApLoop assembly code. And the page table for AP is unavailiable after boot OS. This might cause potential issue. So disable PG in IA32 RelocateApLoop. Signed-off-by: Dun Tan <dun.tan@intel.com> Reviewed-by: Ray Ni <ray.ni@intel.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
Abstract some DxeMpLib code to function in this commit. Some of these internal functions will be moved to common MpLib.c in following commits. Then PeiMpLib can reuse the code. Signed-off-by: Dun Tan <dun.tan@intel.com> Reviewed-by: Ray Ni <ray.ni@intel.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
Move some code in DxeMpLib.C to common MpLib.c. The related code is to relocate Ap to new safe buffer before booting into OS. In next commits, these code also will be used by PeiMpLib. This commit doesn't change any code functionality. Signed-off-by: Dun Tan <dun.tan@intel.com> Reviewed-by: Ray Ni <ray.ni@intel.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
Install gEdkiiEndOfS3ResumeGuid in S3Resume to trigger callback registered by PeiMpLib. The callback is to relocate Ap to new safe memory before jump to OS waking vector in S3 boot flow. Signed-off-by: Dun Tan <dun.tan@intel.com> Reviewed-by: Ray Ni <ray.ni@intel.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
In this commit, change PeiMpLib to install callback of gEdkiiEndOfS3ResumeGuid to relocate AP to new safe buffer. The gEdkiiEndOfS3ResumeGuid is installed in S3Resume.c before jmping to OS waking vector. Previously, code in CpuS3.c of PiSmmCpuDxe driver will prepare the new safe buffer for AP and place AP in hlt loop state. With this code change, we can remove the Machine Instructions of mApHltLoopCode in PiSmmCpuDxe. Also we can reuse the related code in DxeMpLib for PeiMpLib. Signed-off-by: Dun Tan <dun.tan@intel.com> Reviewed-by: Ray Ni <ray.ni@intel.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
Remove ProgramVirtualWireMode()/DisableLvtInterrupts() since APs won't be waken by INIT-SIPI-SIPI in CpuS3.c any more. The two functions has been executed in MpInitLibInitialize() in PeiMplib. Signed-off-by: Dun Tan <dun.tan@intel.com> Reviewed-by: Ray Ni <ray.ni@intel.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
After the code to load mtrr setting, set register table, handle APIC setting and Interrupt after INIT-SIPI-SIPI is moved, the InitializeCpuProcedure() only contains following code logic: 1.Bsp runs ExecuteFirstSmiInit(). 2.Bsp transfers AP to safe hlt-loop During S3 boot, since APs will be relocated to new safe buffer by the callback of gEdkiiEndOfS3ResumeGuid in PeiMpLib, Bsp doesn't need to transfer AP to safe hlt-loop any more. SmmRestoreCpu() in CpuS3 only needs to runs the ExecuteFirstSmiInit() on BSP. So remove code to wakeup AP by INIT-SIPI-SIPI and remove code to relocate ap to safe hlt-loop. Signed-off-by: Dun Tan <dun.tan@intel.com> Reviewed-by: Ray Ni <ray.ni@intel.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
Remove the unneeded assignment of MpService2Ppi field in SmmS3ResumeState. Previously, when the execution combination of PEI and DXE are the same, the pointer of mpservice ppi will be passed to CpuS3.c in smm cpu driver to wakeup all APs, instead of init-sipi-sipi. Currently, CpuS3.c doesn't need to wakeup Aps anymore. So remove the duplicated mpservice locate and assignment to MpService2Ppi field in SmmS3ResumeState. Signed-off-by: Dun Tan <dun.tan@intel.com> Reviewed-by: Ray Ni <ray.ni@intel.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
This MpService2Ppi field in SMM_S3_RESUME_STATE is used to wakeup AP to do the CPU initialization during smm s3 boot when the execution mode of PEI and DXE are the same. Currently, in CpuS3.c of smm cpu driver, BSP doesn't need to wakeup AP anymore. The initialization for AP will be done in S3Resume.c before transfer to CpuS3.c of smm cpu driver. So we can remove the MpService2Ppi field in SMM_S3_RESUME_STATE. Signed-off-by: Dun Tan <dun.tan@intel.com> Reviewed-by: Ray Ni <ray.ni@intel.com> Cc: Jian J Wang <jian.j.wang@intel.com> Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
Remove GetAcpiCpuData() in CpuS3.c. The mAcpiCpuData is not needed in S3 boot anymore. Signed-off-by: Dun Tan <dun.tan@intel.com> Reviewed-by: Ray Ni <ray.ni@intel.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch set has been reviewed-by responding pkg maintainers(Ni, Ray, Wu jiaxin, Ard Biesheuvel and Gao Liming) in previous edk2 community mail loop.
This patch set is to remove some S3 related code in CpuS3.c of smm cpu driver. It contain commits to: