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

2024-05-23 Thread duntan 
Hi Liming,



Could you please help to review the following 2 patches in the patch series?

  [Patch V2 01/18] MdeModulePkg: Add gEdkiiS3MtrrSettingGuid

  [Patch V2 17/18] MdeModulePkg:Remove MpService2Ppi field in 
SMM_S3_RESUME_STATE



Thanks,

Dun

From: Ni, Ray 
Sent: Monday, May 20, 2024 3:05 PM
To: Tan, Dun ; devel@edk2.groups.io
Cc: Liming Gao ; Wu, Jiaxin ; 
Ard Biesheuvel ; Yao, Jiewen ; 
Gerd Hoffmann ; Kumar, Rahul R 
Subject: Re: [Patch V2 00/18] Remove some S3 related code in CpuS3.c of smm cpu 
driver

Following 3 patches still require the Reviewed-by from package maintainers of 
MdeModulePkg and OvmfPkg.
  MdeModulePkg: Add gEdkiiS3MtrrSettingGuid
  OvmfPkg: Save MTRR by lockbox in CpuS3DataDxe
  MdeModulePkg:Remove MpService2Ppi field in SMM_S3_RESUME_STATE

The patch set is a good move to simplify the X86 CPU SMM driver by removing 
most of S3 related logics.
I hope it can be merged next week when the stable tag freeze ends in the end of 
this week.

Thanks,
Ray

From: Tan, Dun mailto:dun@intel.com>>
Sent: Friday, May 17, 2024 17:45
To: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>>
Cc: Liming Gao mailto:gaolim...@byosoft.com.cn>>; Wu, 
Jiaxin mailto:jiaxin...@intel.com>>; Ni, Ray 
mailto:ray...@intel.com>>; Ard Biesheuvel 
mailto:ardb+tianoc...@kernel.org>>; Yao, Jiewen 
mailto:jiewen@intel.com>>; Gerd Hoffmann 
mailto:kra...@redhat.com>>; Kumar, Rahul R 
mailto:rahul.r.ku...@intel.com>>
Subject: [Patch V2 00/18] Remove some S3 related code in CpuS3.c of smm cpu 
driver

Comparing to V1 patchs set, the V2 patch set only adjusts the commits ordering 
and modifies copy right year in some files.
This patch set is to remove some S3 related code in CpuS3.c of smm cpu driver. 
It contain commits to:
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) Register table cleanup:
>   UefiCpuPkg:Set PcdCpuFeaturesInitOnS3Resume to TRUE
>   UefiCpuPkg: Remove code to set register table

4)  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

5)  Remove code to get AcpiCpuData:
>   UefiCpuPkg: Remove GetAcpiCpuData() in CpuS3.c

With this patch set, CpuS3.c in smm CPU driver can be simplified.
The whole patch set has been reviewed-by Ray.
PR for review: https://github.com/tianocore/edk2/pull/5606

Cc: Liming Gao mailto:gaolim...@byosoft.com.cn>>
Cc: Jiaxin Wu mailto:jiaxin...@intel.com>>
Cc: Ray Ni mailto:ray...@intel.com>>
Cc: Ard Biesheuvel mailto:ardb+tianoc...@kernel.org>>
Cc: Jiewen Yao mailto:jiewen@intel.com>>
Cc: Gerd Hoffmann mailto:kra...@redhat.com>>
Cc: Rahul Kumar mailto:rahul1.ku...@intel.com>>
Cc: Gerd Hoffmann mailto:kra...@redhat.com>>

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 code to load mtrr setting
  UefiCpuPkg:Set PcdCpuFeaturesInitOnS3Resume to TRUE
  UefiCpuPkg: Remove code to set register table
  UefiCpuPkg: Disable PG in IA32 ApLoopCode
  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:Remove code to wakeup AP and relocate ap
  UefiCpuPkg: Remove unneeded MpService2Ppi assignment
  MdeModulePkg:Remove MpService2Ppi field in SMM_S3_RESUME_STATE
  UefiCpuPkg: Remove GetAcpiCpuData() in CpuS3.c

 MdeModulePkg/Include/Guid/AcpiS3Context.h   |   3 +--
 MdeModulePkg/MdeModulePkg.dec   |   5 -
 OvmfPkg/CpuS3DataDxe/CpuS3Data.c|  13 -
 OvmfPkg/CpuS3DataDxe/CpuS3DataDxe.inf   |   4 +++-
 UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c |  13 -
 UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf|   4 +++-
 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 183 
++

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

2024-05-23 Thread Wu, Jiaxin 
Series Reviewed-by: Jiaxin Wu 

The series patches are good to me now. Thanks Dun.

Thanks,
Jiaxin

> -Original Message-
> From: Tan, Dun 
> Sent: Friday, May 17, 2024 5:46 PM
> To: devel@edk2.groups.io
> Cc: Liming Gao ; Wu, Jiaxin
> ; Ni, Ray ; Ard Biesheuvel
> ; Yao, Jiewen ; Gerd
> Hoffmann ; Kumar, Rahul R 
> Subject: [Patch V2 00/18] Remove some S3 related code in CpuS3.c of smm
> cpu driver
> 
> Comparing to V1 patchs set, the V2 patch set only adjusts the commits
> ordering and modifies copy right year in some files.
> This patch set is to remove some S3 related code in CpuS3.c of smm cpu
> driver. It contain commits to:
> 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) Register table cleanup:
> >   UefiCpuPkg:Set PcdCpuFeaturesInitOnS3Resume to TRUE
> >   UefiCpuPkg: Remove code to set register table
> 
> 4)  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
> 
> 5)  Remove code to get AcpiCpuData:
> >   UefiCpuPkg: Remove GetAcpiCpuData() in CpuS3.c
> 
> With this patch set, CpuS3.c in smm CPU driver can be simplified.
> The whole patch set has been reviewed-by Ray.
> PR for review: https://github.com/tianocore/edk2/pull/5606
> 
> Cc: Liming Gao 
> Cc: Jiaxin Wu 
> Cc: Ray Ni 
> Cc: Ard Biesheuvel 
> Cc: Jiewen Yao 
> Cc: Gerd Hoffmann 
> Cc: Rahul Kumar 
> Cc: Gerd Hoffmann 
> 
> 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 code to load mtrr setting
>   UefiCpuPkg:Set PcdCpuFeaturesInitOnS3Resume to TRUE
>   UefiCpuPkg: Remove code to set register table
>   UefiCpuPkg: Disable PG in IA32 ApLoopCode
>   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:Remove code to wakeup AP and relocate ap
>   UefiCpuPkg: Remove unneeded MpService2Ppi assignment
>   MdeModulePkg:Remove MpService2Ppi field in SMM_S3_RESUME_STATE
>   UefiCpuPkg: Remove GetAcpiCpuData() in CpuS3.c
> 
>  MdeModulePkg/Include/Guid/AcpiS3Context.h   |   3 +--
>  MdeModulePkg/MdeModulePkg.dec   |   5 -
>  OvmfPkg/CpuS3DataDxe/CpuS3Data.c|  13 -
>  OvmfPkg/CpuS3DataDxe/CpuS3DataDxe.inf   |   4 +++-
>  UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c |  13 -
>  UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf|   4 +++-
>  UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 183
> -
> --
> --
>  UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm  |   6 +-
>  UefiCpuPkg/Library/MpInitLib/MpLib.c| 144
> ++
> ++
> +++-
>  UefiCpuPkg/Library/MpInitLib/MpLib.h|  56
> +++--
>  UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf   |   6 +-
>  UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 154
> ++
> ++
> +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c   | 993 
> +---
> --
> -
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/MpFuncs.nasm | 153 
> --
> ---

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

2024-05-20 Thread Ni, Ray 
Following 3 patches still require the Reviewed-by from package maintainers of 
MdeModulePkg and OvmfPkg.
  MdeModulePkg: Add gEdkiiS3MtrrSettingGuid
  OvmfPkg: Save MTRR by lockbox in CpuS3DataDxe
  MdeModulePkg:Remove MpService2Ppi field in SMM_S3_RESUME_STATE

The patch set is a good move to simplify the X86 CPU SMM driver by removing 
most of S3 related logics.
I hope it can be merged next week when the stable tag freeze ends in the end of 
this week.

Thanks,
Ray

From: Tan, Dun 
Sent: Friday, May 17, 2024 17:45
To: devel@edk2.groups.io 
Cc: Liming Gao ; Wu, Jiaxin ; 
Ni, Ray ; Ard Biesheuvel ; Yao, 
Jiewen ; Gerd Hoffmann ; Kumar, Rahul 
R 
Subject: [Patch V2 00/18] Remove some S3 related code in CpuS3.c of smm cpu 
driver

Comparing to V1 patchs set, the V2 patch set only adjusts the commits ordering 
and modifies copy right year in some files.
This patch set is to remove some S3 related code in CpuS3.c of smm cpu driver. 
It contain commits to:
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) Register table cleanup:
>   UefiCpuPkg:Set PcdCpuFeaturesInitOnS3Resume to TRUE
>   UefiCpuPkg: Remove code to set register table

4)  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

5)  Remove code to get AcpiCpuData:
>   UefiCpuPkg: Remove GetAcpiCpuData() in CpuS3.c

With this patch set, CpuS3.c in smm CPU driver can be simplified.
The whole patch set has been reviewed-by Ray.
PR for review: https://github.com/tianocore/edk2/pull/5606

Cc: Liming Gao 
Cc: Jiaxin Wu 
Cc: Ray Ni 
Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Gerd Hoffmann 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 

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 code to load mtrr setting
  UefiCpuPkg:Set PcdCpuFeaturesInitOnS3Resume to TRUE
  UefiCpuPkg: Remove code to set register table
  UefiCpuPkg: Disable PG in IA32 ApLoopCode
  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:Remove code to wakeup AP and relocate ap
  UefiCpuPkg: Remove unneeded MpService2Ppi assignment
  MdeModulePkg:Remove MpService2Ppi field in SMM_S3_RESUME_STATE
  UefiCpuPkg: Remove GetAcpiCpuData() in CpuS3.c

 MdeModulePkg/Include/Guid/AcpiS3Context.h   |   3 +--
 MdeModulePkg/MdeModulePkg.dec   |   5 -
 OvmfPkg/CpuS3DataDxe/CpuS3Data.c|  13 -
 OvmfPkg/CpuS3DataDxe/CpuS3DataDxe.inf   |   4 +++-
 UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c |  13 -
 UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf|   4 +++-
 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 183 
-
 UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm  |   6 +-
 UefiCpuPkg/Library/MpInitLib/MpLib.c| 144 
+++-
 UefiCpuPkg/Library/MpInitLib/MpLib.h|  56 
+++--
 UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf   |   6 +-
 UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 154 
+-
 UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c   | 993 
+--
 UefiCpuPkg/PiSmmCpuDxeSm

[edk2-devel] [Patch V2 00/18] Remove some S3 related code in CpuS3.c of smm cpu driver

2024-05-17 Thread duntan 
Comparing to V1 patchs set, the V2 patch set only adjusts the commits ordering 
and modifies copy right year in some files.
This patch set is to remove some S3 related code in CpuS3.c of smm cpu driver. 
It contain commits to:
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) Register table cleanup:
>   UefiCpuPkg:Set PcdCpuFeaturesInitOnS3Resume to TRUE
>   UefiCpuPkg: Remove code to set register table

4)  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

5)  Remove code to get AcpiCpuData:
>   UefiCpuPkg: Remove GetAcpiCpuData() in CpuS3.c

With this patch set, CpuS3.c in smm CPU driver can be simplified.
The whole patch set has been reviewed-by Ray.
PR for review: https://github.com/tianocore/edk2/pull/5606

Cc: Liming Gao 
Cc: Jiaxin Wu 
Cc: Ray Ni 
Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Gerd Hoffmann 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 

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 code to load mtrr setting
  UefiCpuPkg:Set PcdCpuFeaturesInitOnS3Resume to TRUE
  UefiCpuPkg: Remove code to set register table
  UefiCpuPkg: Disable PG in IA32 ApLoopCode
  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:Remove code to wakeup AP and relocate ap
  UefiCpuPkg: Remove unneeded MpService2Ppi assignment
  MdeModulePkg:Remove MpService2Ppi field in SMM_S3_RESUME_STATE
  UefiCpuPkg: Remove GetAcpiCpuData() in CpuS3.c

 MdeModulePkg/Include/Guid/AcpiS3Context.h   |   3 +--
 MdeModulePkg/MdeModulePkg.dec   |   5 -
 OvmfPkg/CpuS3DataDxe/CpuS3Data.c|  13 -
 OvmfPkg/CpuS3DataDxe/CpuS3DataDxe.inf   |   4 +++-
 UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c |  13 -
 UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf|   4 +++-
 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 183 
-
 UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm  |   6 +-
 UefiCpuPkg/Library/MpInitLib/MpLib.c| 144 
+++-
 UefiCpuPkg/Library/MpInitLib/MpLib.h|  56 
+++--
 UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf   |   6 +-
 UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 154 
+-
 UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c   | 993 
+--
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/MpFuncs.nasm | 153 
-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c   |  29 
+---
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c  |   8 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h  |  26 
+
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf|   5 +
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/MpFuncs.nasm  | 189 
--
 UefiCpuPkg/PiSmmCp