Re: [edk2-devel] [PATCH 00/19] Remove some S3 related code in CpuS3.c of smm cpu driver

["duntan" <dun.tan@intel.com>]

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.wu@intel.com> 
Sent: Monday, May 13, 2024 10:49 AM
To: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com>; Ni, Ray <ray.ni@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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-