public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel] [Patch V2 00/18] Remove some S3 related code in CpuS3.c of smm cpu driver
@ 2024-05-17  9:45 duntan
  2024-05-17  9:45 ` [edk2-devel] [Patch V2 01/18] MdeModulePkg: Add gEdkiiS3MtrrSettingGuid duntan
                   ` (21 more replies)
  0 siblings, 22 replies; 30+ messages in thread
From: duntan @ 2024-05-17  9:45 UTC (permalink / raw)
  To: devel
  Cc: Liming Gao, Jiaxin Wu, Ray Ni, Ard Biesheuvel, Jiewen Yao,
	Gerd Hoffmann, Rahul Kumar

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 <gaoliming@byosoft.com.cn>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@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 ++++++++++++++++++++++++++++++++++++++++++++++++-----------------------------------------------------------------------------------------------------------------------------
 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/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c            |  30 +----------------------------
 UefiCpuPkg/UefiCpuPkg.dec                               |   4 ++--
 UefiCpuPkg/UefiCpuPkg.dsc                               |   3 ++-
 UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c       |  78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------
 UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf |   4 +++-
 24 files changed, 531 insertions(+), 1582 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 (#118997): https://edk2.groups.io/g/devel/message/118997
Mute This Topic: https://groups.io/mt/106150757/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 30+ messages in thread

* [edk2-devel] [Patch V2 01/18] MdeModulePkg: Add gEdkiiS3MtrrSettingGuid
  2024-05-17  9:45 [edk2-devel] [Patch V2 00/18] Remove some S3 related code in CpuS3.c of smm cpu driver duntan
@ 2024-05-17  9:45 ` duntan
  2024-05-17  9:45 ` [edk2-devel] [Patch V2 02/18] OvmfPkg: Save MTRR by lockbox in CpuS3DataDxe duntan
                   ` (20 subsequent siblings)
  21 siblings, 0 replies; 30+ messages in thread
From: duntan @ 2024-05-17  9:45 UTC (permalink / raw)
  To: devel; +Cc: Ray Ni, Liming Gao, Jiaxin Wu

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>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
---
 MdeModulePkg/MdeModulePkg.dec | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index f7339f0aec..02f330a453 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -4,7 +4,7 @@
 # and libraries instances, which are used for those modules.
 #
 # Copyright (c) 2019, NVIDIA CORPORATION. All rights reserved.
-# Copyright (c) 2007 - 2021, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2007 - 2024, Intel Corporation. All rights reserved.<BR>
 # Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
 # (C) Copyright 2016 - 2019 Hewlett Packard Enterprise Development LP<BR>
 # Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
@@ -465,6 +465,9 @@
   gEdk2JedecSfdpSpiDxeDriverGuid  = { 0xBE71701E, 0xB63C, 0x4574, { 0x9C, 0x5C, 0x36, 0x29, 0xE8, 0xEA, 0xC4, 0x14 }}
   gEdk2JedecSfdpSpiSmmDriverGuid  = { 0x95A1E915, 0x195C, 0x477C, { 0x92, 0x6F, 0x7E, 0x24, 0x67, 0xC1, 0xB3, 0x1F }}
 
+  ## This GUID will be used to save MTRR_SETTINGS at EndOfDxe by LockBox and restore at S3 boot PEI phase for s3 usage.
+  gEdkiiS3MtrrSettingGuid = { 0xd77baa84, 0xb332, 0x4463, { 0x9f, 0x1d, 0xce, 0x81, 0x00, 0xfe, 0x7f, 0x35 }}
+
 [Ppis]
   ## Include/Ppi/FirmwareVolumeShadowPpi.h
   gEdkiiPeiFirmwareVolumeShadowPpiGuid = { 0x7dfe756c, 0xed8d, 0x4d77, {0x9e, 0xc4, 0x39, 0x9a, 0x8a, 0x81, 0x51, 0x16 } }
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118998): https://edk2.groups.io/g/devel/message/118998
Mute This Topic: https://groups.io/mt/106150759/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [edk2-devel] [Patch V2 02/18] OvmfPkg: Save MTRR by lockbox in CpuS3DataDxe
  2024-05-17  9:45 [edk2-devel] [Patch V2 00/18] Remove some S3 related code in CpuS3.c of smm cpu driver duntan
  2024-05-17  9:45 ` [edk2-devel] [Patch V2 01/18] MdeModulePkg: Add gEdkiiS3MtrrSettingGuid duntan
@ 2024-05-17  9:45 ` duntan
  2024-05-17  9:45 ` [edk2-devel] [Patch V2 03/18] UefiCpuPkg: Add locbox lib instance in DSC duntan
                   ` (19 subsequent siblings)
  21 siblings, 0 replies; 30+ messages in thread
From: duntan @ 2024-05-17  9:45 UTC (permalink / raw)
  To: devel; +Cc: Ard Biesheuvel, Jiewen Yao, Gerd Hoffmann, Ray Ni, Jiaxin Wu

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>
Cc: 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>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
---
 OvmfPkg/CpuS3DataDxe/CpuS3Data.c      | 13 ++++++++++++-
 OvmfPkg/CpuS3DataDxe/CpuS3DataDxe.inf |  4 +++-
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/CpuS3DataDxe/CpuS3Data.c b/OvmfPkg/CpuS3DataDxe/CpuS3Data.c
index 289048b75d..eacdfa12c3 100644
--- a/OvmfPkg/CpuS3DataDxe/CpuS3Data.c
+++ b/OvmfPkg/CpuS3DataDxe/CpuS3Data.c
@@ -9,7 +9,7 @@ number of CPUs reported by the MP Services Protocol, so this module does not
 support hot plug CPUs.  This module can be copied into a CPU specific package
 and customized if these additional features are required.
 
-Copyright (c) 2013 - 2021, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2013 - 2024, Intel Corporation. All rights reserved.<BR>
 Copyright (c) 2015 - 2020, Red Hat, Inc.
 
 SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -26,6 +26,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Library/MemoryAllocationLib.h>
 #include <Library/MtrrLib.h>
 #include <Library/UefiBootServicesTableLib.h>
+#include <Library/LockBoxLib.h>
 
 #include <Protocol/MpService.h>
 #include <Guid/EventGroup.h>
@@ -130,6 +131,16 @@ CpuS3DataOnEndOfDxe (
   DEBUG ((DEBUG_VERBOSE, "%a\n", __func__));
   MtrrGetAllMtrrs (&AcpiCpuDataEx->MtrrTable);
 
+  //
+  // Save MTRR in lockbox
+  //
+  Status = SaveLockBox (
+             &gEdkiiS3MtrrSettingGuid,
+             &AcpiCpuDataEx->MtrrTable,
+             sizeof (MTRR_SETTINGS)
+             );
+  ASSERT_EFI_ERROR (Status);
+
   //
   // Close event, so it will not be invoked again.
   //
diff --git a/OvmfPkg/CpuS3DataDxe/CpuS3DataDxe.inf b/OvmfPkg/CpuS3DataDxe/CpuS3DataDxe.inf
index 228d5ae1b2..5369613f5d 100644
--- a/OvmfPkg/CpuS3DataDxe/CpuS3DataDxe.inf
+++ b/OvmfPkg/CpuS3DataDxe/CpuS3DataDxe.inf
@@ -9,7 +9,7 @@
 #  support hot plug CPUs.  This module can be copied into a CPU specific package
 #  and customized if these additional features are required.
 #
-#  Copyright (c) 2013-2016, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2013-2024, Intel Corporation. All rights reserved.<BR>
 #  Copyright (c) 2015-2020, Red Hat, Inc.
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -46,9 +46,11 @@
   MtrrLib
   UefiBootServicesTableLib
   UefiDriverEntryPoint
+  LockBoxLib
 
 [Guids]
   gEfiEndOfDxeEventGroupGuid         ## CONSUMES   ## Event
+  gEdkiiS3MtrrSettingGuid
 
 [Protocols]
   gEfiMpServiceProtocolGuid          ## CONSUMES
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118999): https://edk2.groups.io/g/devel/message/118999
Mute This Topic: https://groups.io/mt/106150760/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [edk2-devel] [Patch V2 03/18] UefiCpuPkg: Add locbox lib instance in DSC
  2024-05-17  9:45 [edk2-devel] [Patch V2 00/18] Remove some S3 related code in CpuS3.c of smm cpu driver duntan
  2024-05-17  9:45 ` [edk2-devel] [Patch V2 01/18] MdeModulePkg: Add gEdkiiS3MtrrSettingGuid duntan
  2024-05-17  9:45 ` [edk2-devel] [Patch V2 02/18] OvmfPkg: Save MTRR by lockbox in CpuS3DataDxe duntan
@ 2024-05-17  9:45 ` duntan
  2024-05-17  9:45 ` [edk2-devel] [Patch V2 04/18] UefiCpuPkg: Save MTRR by lockbox in CpuS3DataDxe duntan
                   ` (18 subsequent siblings)
  21 siblings, 0 replies; 30+ messages in thread
From: duntan @ 2024-05-17  9:45 UTC (permalink / raw)
  To: devel; +Cc: Ray Ni, Rahul Kumar, Gerd Hoffmann, Jiaxin Wu

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>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
---
 UefiCpuPkg/UefiCpuPkg.dsc | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/UefiCpuPkg.dsc b/UefiCpuPkg/UefiCpuPkg.dsc
index 3d49f72588..1b52760e1a 100644
--- a/UefiCpuPkg/UefiCpuPkg.dsc
+++ b/UefiCpuPkg/UefiCpuPkg.dsc
@@ -1,7 +1,7 @@
 ## @file
 #  UefiCpuPkg Package
 #
-#  Copyright (c) 2007 - 2023, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2007 - 2024, Intel Corporation. All rights reserved.<BR>
 #  Copyright (C) 2023 - 2024, Advanced Micro Devices, Inc. All rights reserved.<BR>
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -68,6 +68,7 @@
   UnitTestLib|UnitTestFrameworkPkg/Library/UnitTestLib/UnitTestLib.inf
   UnitTestPersistenceLib|UnitTestFrameworkPkg/Library/UnitTestPersistenceLibNull/UnitTestPersistenceLibNull.inf
   UnitTestResultReportLib|UnitTestFrameworkPkg/Library/UnitTestResultReportLib/UnitTestResultReportLibDebugLib.inf
+  LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.inf
 
 [LibraryClasses.common.SEC]
   PlatformSecLib|UefiCpuPkg/Library/PlatformSecLibNull/PlatformSecLibNull.inf
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119000): https://edk2.groups.io/g/devel/message/119000
Mute This Topic: https://groups.io/mt/106150761/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [edk2-devel] [Patch V2 04/18] UefiCpuPkg: Save MTRR by lockbox in CpuS3DataDxe
  2024-05-17  9:45 [edk2-devel] [Patch V2 00/18] Remove some S3 related code in CpuS3.c of smm cpu driver duntan
                   ` (2 preceding siblings ...)
  2024-05-17  9:45 ` [edk2-devel] [Patch V2 03/18] UefiCpuPkg: Add locbox lib instance in DSC duntan
@ 2024-05-17  9:45 ` duntan
  2024-05-17  9:45 ` [edk2-devel] [Patch V2 05/18] UefiCpuPkg: LoadMtrrData for all cpu in S3Resume duntan
                   ` (17 subsequent siblings)
  21 siblings, 0 replies; 30+ messages in thread
From: duntan @ 2024-05-17  9:45 UTC (permalink / raw)
  To: devel; +Cc: Ray Ni, Rahul Kumar, Gerd Hoffmann, Jiaxin Wu

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>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
---
 UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c      | 13 ++++++++++++-
 UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf |  4 +++-
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c b/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c
index 86ce5303ca..c87f82bc22 100644
--- a/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c
+++ b/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c
@@ -9,7 +9,7 @@ number of CPUs reported by the MP Services Protocol, so this module does not
 support hot plug CPUs.  This module can be copied into a CPU specific package
 and customized if these additional features are required.
 
-Copyright (c) 2013 - 2021, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2013 - 2024, Intel Corporation. All rights reserved.<BR>
 Copyright (c) 2015, Red Hat, Inc.
 
 SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -26,6 +26,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Library/DebugLib.h>
 #include <Library/MtrrLib.h>
 #include <Library/MemoryAllocationLib.h>
+#include <Library/LockBoxLib.h>
 
 #include <Protocol/MpService.h>
 #include <Guid/EventGroup.h>
@@ -130,6 +131,16 @@ CpuS3DataOnEndOfDxe (
   DEBUG ((DEBUG_VERBOSE, "%a\n", __func__));
   MtrrGetAllMtrrs (&AcpiCpuDataEx->MtrrTable);
 
+  //
+  // Save MTRR in lockbox
+  //
+  Status = SaveLockBox (
+             &gEdkiiS3MtrrSettingGuid,
+             &AcpiCpuDataEx->MtrrTable,
+             sizeof (MTRR_SETTINGS)
+             );
+  ASSERT_EFI_ERROR (Status);
+
   //
   // Close event, so it will not be invoked again.
   //
diff --git a/UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf b/UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf
index 510133a614..c690f7df5b 100644
--- a/UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf
+++ b/UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf
@@ -9,7 +9,7 @@
 #  support hot plug CPUs.  This module can be copied into a CPU specific package
 #  and customized if these additional features are required.
 #
-#  Copyright (c) 2013-2016, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2013-2024, Intel Corporation. All rights reserved.<BR>
 #  Copyright (c) 2015, Red Hat, Inc.
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -46,9 +46,11 @@
   BaseLib
   MtrrLib
   MemoryAllocationLib
+  LockBoxLib
 
 [Guids]
   gEfiEndOfDxeEventGroupGuid         ## CONSUMES   ## Event
+  gEdkiiS3MtrrSettingGuid
 
 [Protocols]
   gEfiMpServiceProtocolGuid          ## CONSUMES
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119001): https://edk2.groups.io/g/devel/message/119001
Mute This Topic: https://groups.io/mt/106150762/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [edk2-devel] [Patch V2 05/18] UefiCpuPkg: LoadMtrrData for all cpu in S3Resume
  2024-05-17  9:45 [edk2-devel] [Patch V2 00/18] Remove some S3 related code in CpuS3.c of smm cpu driver duntan
                   ` (3 preceding siblings ...)
  2024-05-17  9:45 ` [edk2-devel] [Patch V2 04/18] UefiCpuPkg: Save MTRR by lockbox in CpuS3DataDxe duntan
@ 2024-05-17  9:45 ` duntan
  2024-05-17  9:45 ` [edk2-devel] [Patch V2 06/18] UefiCpuPkg: Remove code to load mtrr setting duntan
                   ` (16 subsequent siblings)
  21 siblings, 0 replies; 30+ messages in thread
From: duntan @ 2024-05-17  9:45 UTC (permalink / raw)
  To: devel; +Cc: Ray Ni, Rahul Kumar, Gerd Hoffmann, Jiaxin Wu

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>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
---
 UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c       | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf |  4 +++-
 2 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
index 4cf676fb3e..078ae2d72d 100644
--- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
+++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
@@ -4,7 +4,7 @@
   This module will execute the boot script saved during last boot and after that,
   control is passed to OS waking up handler.
 
-  Copyright (c) 2006 - 2023, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2006 - 2024, Intel Corporation. All rights reserved.<BR>
   Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -39,6 +39,7 @@
 #include <Library/DebugAgentLib.h>
 #include <Library/LocalApicLib.h>
 #include <Library/ReportStatusCodeLib.h>
+#include <Library/MtrrLib.h>
 
 #include <Library/HobLib.h>
 #include <Library/LockBoxLib.h>
@@ -938,6 +939,20 @@ S3ResumeExecuteBootScript (
   CpuDeadLoop ();
 }
 
+/**
+  Sync up the MTRR values for all processors.
+
+  @param[in] MtrrTable  Address of MTRR setting.
+**/
+VOID
+EFIAPI
+LoadMtrrData (
+  IN VOID  *MtrrTable
+  )
+{
+  MtrrSetAllMtrrs (MtrrTable);
+}
+
 /**
   Restores the platform to its preboot configuration for an S3 resume and
   jumps to the OS waking vector.
@@ -990,6 +1005,7 @@ S3RestoreConfig2 (
   BOOLEAN                        InterruptStatus;
   IA32_CR0                       Cr0;
   EDKII_PEI_MP_SERVICES2_PPI     *MpService2Ppi;
+  MTRR_SETTINGS                  MtrrTable;
 
   TempAcpiS3Context                 = 0;
   TempEfiBootScriptExecutorVariable = 0;
@@ -1082,6 +1098,39 @@ S3RestoreConfig2 (
       Status = SmmAccess->Open ((EFI_PEI_SERVICES **)GetPeiServicesTablePointer (), SmmAccess, Index);
     }
 
+    //
+    // Get MP Services2 Ppi to pass it to Smm S3.
+    //
+    Status = PeiServicesLocatePpi (
+               &gEdkiiPeiMpServices2PpiGuid,
+               0,
+               NULL,
+               (VOID **)&MpService2Ppi
+               );
+    ASSERT_EFI_ERROR (Status);
+
+    //
+    // Restore MTRR setting
+    //
+    VarSize = sizeof (MTRR_SETTINGS);
+    Status  = RestoreLockBox (
+                &gEdkiiS3MtrrSettingGuid,
+                &MtrrTable,
+                &VarSize
+                );
+    ASSERT_EFI_ERROR (Status);
+
+    //
+    // Sync up the MTRR values for all processors.
+    //
+    Status = MpService2Ppi->StartupAllCPUs (
+                              MpService2Ppi,
+                              (EFI_AP_PROCEDURE)LoadMtrrData,
+                              0,
+                              (VOID *)&MtrrTable
+                              );
+    ASSERT_EFI_ERROR (Status);
+
     SmramDescriptor  = (EFI_SMRAM_DESCRIPTOR *)GET_GUID_HOB_DATA (GuidHob);
     SmmS3ResumeState = (SMM_S3_RESUME_STATE *)(UINTN)SmramDescriptor->CpuStart;
 
diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
index 9c9b6f3db3..54de8bc91f 100644
--- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
+++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
@@ -5,7 +5,7 @@
 # This module will excute the boot script saved during last boot and after that,
 # control is passed to OS waking up handler.
 #
-# Copyright (c) 2010 - 2023, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2010 - 2024, Intel Corporation. All rights reserved.<BR>
 # Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
 #
 # SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -67,6 +67,7 @@
   LocalApicLib
   ReportStatusCodeLib
   LockBoxLib
+  MtrrLib
 
 [Guids]
   gEfiBootScriptExecutorVariableGuid            ## SOMETIMES_CONSUMES ## UNDEFINED # LockBox
@@ -79,6 +80,7 @@
   ## SOMETIMES_PRODUCES ## UNDEFINED # Install PPI
   ## SOMETIMES_CONSUMES ## UNDEFINED # Used to do smm communication
   gEdkiiS3SmmInitDoneGuid
+  gEdkiiS3MtrrSettingGuid
 
 [Ppis]
   gEfiPeiS3Resume2PpiGuid                       ## PRODUCES
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119002): https://edk2.groups.io/g/devel/message/119002
Mute This Topic: https://groups.io/mt/106150763/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [edk2-devel] [Patch V2 06/18] UefiCpuPkg: Remove code to load mtrr setting
  2024-05-17  9:45 [edk2-devel] [Patch V2 00/18] Remove some S3 related code in CpuS3.c of smm cpu driver duntan
                   ` (4 preceding siblings ...)
  2024-05-17  9:45 ` [edk2-devel] [Patch V2 05/18] UefiCpuPkg: LoadMtrrData for all cpu in S3Resume duntan
@ 2024-05-17  9:45 ` duntan
  2024-05-17  9:45 ` [edk2-devel] [Patch V2 07/18] UefiCpuPkg:Set PcdCpuFeaturesInitOnS3Resume to TRUE duntan
                   ` (15 subsequent siblings)
  21 siblings, 0 replies; 30+ messages in thread
From: duntan @ 2024-05-17  9:45 UTC (permalink / raw)
  To: devel; +Cc: Ray Ni, Rahul Kumar, Gerd Hoffmann, Jiaxin Wu

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>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 34 +---------------------------------
 1 file changed, 1 insertion(+), 33 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
index d67fb49890..75b8c0d3de 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
@@ -1,7 +1,7 @@
 /** @file
 Code for Processor S3 restoration
 
-Copyright (c) 2006 - 2023, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2024, Intel Corporation. All rights reserved.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -91,36 +91,6 @@ UINT8  mApHltLoopCodeTemplate[] = {
   0xEB, 0xFC              // jmp $-2
 };
 
-/**
-  Sync up the MTRR values for all processors.
-
-  @param MtrrTable  Table holding fixed/variable MTRR values to be loaded.
-**/
-VOID
-EFIAPI
-LoadMtrrData (
-  EFI_PHYSICAL_ADDRESS  MtrrTable
-  )
-
-/*++
-
-Routine Description:
-
-  Sync up the MTRR values for all processors.
-
-Arguments:
-
-Returns:
-    None
-
---*/
-{
-  MTRR_SETTINGS  *MtrrSettings;
-
-  MtrrSettings = (MTRR_SETTINGS *)(UINTN)MtrrTable;
-  MtrrSetAllMtrrs (MtrrSettings);
-}
-
 /**
   Increment semaphore by 1.
 
@@ -554,8 +524,6 @@ InitializeCpuBeforeRebase (
   IN BOOLEAN  IsBsp
   )
 {
-  LoadMtrrData (mAcpiCpuData.MtrrTable);
-
   SetRegister (TRUE);
 
   ProgramVirtualWireMode ();
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119003): https://edk2.groups.io/g/devel/message/119003
Mute This Topic: https://groups.io/mt/106150764/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [edk2-devel] [Patch V2 07/18] UefiCpuPkg:Set PcdCpuFeaturesInitOnS3Resume to TRUE
  2024-05-17  9:45 [edk2-devel] [Patch V2 00/18] Remove some S3 related code in CpuS3.c of smm cpu driver duntan
                   ` (5 preceding siblings ...)
  2024-05-17  9:45 ` [edk2-devel] [Patch V2 06/18] UefiCpuPkg: Remove code to load mtrr setting duntan
@ 2024-05-17  9:45 ` duntan
  2024-05-17  9:46 ` [edk2-devel] [Patch V2 08/18] UefiCpuPkg: Remove code to set register table duntan
                   ` (14 subsequent siblings)
  21 siblings, 0 replies; 30+ messages in thread
From: duntan @ 2024-05-17  9:45 UTC (permalink / raw)
  To: devel; +Cc: Ray Ni, Rahul Kumar, Gerd Hoffmann, Jiaxin Wu

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>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
---
 UefiCpuPkg/UefiCpuPkg.dec | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index f86a6d2bcb..73bae90ddb 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -1,7 +1,7 @@
 ## @file  UefiCpuPkg.dec
 # This Package provides UEFI compatible CPU modules and libraries.
 #
-# Copyright (c) 2007 - 2023, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2007 - 2024, Intel Corporation. All rights reserved.<BR>
 # Copyright (C) 2023 - 2024, Advanced Micro Devices, Inc. All rights reserved.<BR>
 # Copyright (c) 2024, Loongson Technology Corporation Limited. All rights reserved.<BR>
 #
@@ -310,7 +310,7 @@
 
   ## Specifies if CPU features will be initialized during S3 resume.
   # @Prompt If CPU features will be initialized during S3 resume.
-  gUefiCpuPkgTokenSpaceGuid.PcdCpuFeaturesInitOnS3Resume|FALSE|BOOLEAN|0x0000001D
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuFeaturesInitOnS3Resume|TRUE|BOOLEAN|0x0000001D
 
   ## Specifies CPUID Leaf 0x15 Time Stamp Counter and Nominal Core Crystal Clock Frequency.
   # TSC Frequency = ECX (core crystal clock frequency) * EBX/EAX.
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119004): https://edk2.groups.io/g/devel/message/119004
Mute This Topic: https://groups.io/mt/106150765/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [edk2-devel] [Patch V2 08/18] UefiCpuPkg: Remove code to set register table
  2024-05-17  9:45 [edk2-devel] [Patch V2 00/18] Remove some S3 related code in CpuS3.c of smm cpu driver duntan
                   ` (6 preceding siblings ...)
  2024-05-17  9:45 ` [edk2-devel] [Patch V2 07/18] UefiCpuPkg:Set PcdCpuFeaturesInitOnS3Resume to TRUE duntan
@ 2024-05-17  9:46 ` duntan
  2024-05-17  9:46 ` [edk2-devel] [Patch V2 09/18] UefiCpuPkg: Disable PG in IA32 ApLoopCode duntan
                   ` (13 subsequent siblings)
  21 siblings, 0 replies; 30+ messages in thread
From: duntan @ 2024-05-17  9:46 UTC (permalink / raw)
  To: devel; +Cc: Ray Ni, Rahul Kumar, Gerd Hoffmann, Jiaxin Wu

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>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 423 ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 1 file changed, 423 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
index 75b8c0d3de..0bc4a3572d 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
@@ -91,425 +91,6 @@ UINT8  mApHltLoopCodeTemplate[] = {
   0xEB, 0xFC              // jmp $-2
 };
 
-/**
-  Increment semaphore by 1.
-
-  @param      Sem            IN:  32-bit unsigned integer
-
-**/
-VOID
-S3ReleaseSemaphore (
-  IN OUT  volatile UINT32  *Sem
-  )
-{
-  InterlockedIncrement (Sem);
-}
-
-/**
-  Decrement the semaphore by 1 if it is not zero.
-
-  Performs an atomic decrement operation for semaphore.
-  The compare exchange operation must be performed using
-  MP safe mechanisms.
-
-  @param      Sem            IN:  32-bit unsigned integer
-
-**/
-VOID
-S3WaitForSemaphore (
-  IN OUT  volatile UINT32  *Sem
-  )
-{
-  UINT32  Value;
-
-  do {
-    Value = *Sem;
-  } while (Value == 0 ||
-           InterlockedCompareExchange32 (
-             Sem,
-             Value,
-             Value - 1
-             ) != Value);
-}
-
-/**
-  Read / write CR value.
-
-  @param[in]      CrIndex         The CR index which need to read/write.
-  @param[in]      Read            Read or write. TRUE is read.
-  @param[in,out]  CrValue         CR value.
-
-  @retval    EFI_SUCCESS means read/write success, else return EFI_UNSUPPORTED.
-**/
-UINTN
-ReadWriteCr (
-  IN     UINT32   CrIndex,
-  IN     BOOLEAN  Read,
-  IN OUT UINTN    *CrValue
-  )
-{
-  switch (CrIndex) {
-    case 0:
-      if (Read) {
-        *CrValue = AsmReadCr0 ();
-      } else {
-        AsmWriteCr0 (*CrValue);
-      }
-
-      break;
-    case 2:
-      if (Read) {
-        *CrValue = AsmReadCr2 ();
-      } else {
-        AsmWriteCr2 (*CrValue);
-      }
-
-      break;
-    case 3:
-      if (Read) {
-        *CrValue = AsmReadCr3 ();
-      } else {
-        AsmWriteCr3 (*CrValue);
-      }
-
-      break;
-    case 4:
-      if (Read) {
-        *CrValue = AsmReadCr4 ();
-      } else {
-        AsmWriteCr4 (*CrValue);
-      }
-
-      break;
-    default:
-      return EFI_UNSUPPORTED;
-  }
-
-  return EFI_SUCCESS;
-}
-
-/**
-  Initialize the CPU registers from a register table.
-
-  @param[in]  RegisterTable         The register table for this AP.
-  @param[in]  ApLocation            AP location info for this ap.
-  @param[in]  CpuStatus             CPU status info for this CPU.
-  @param[in]  CpuFlags              Flags data structure used when program the register.
-
-  @note This service could be called by BSP/APs.
-**/
-VOID
-ProgramProcessorRegister (
-  IN CPU_REGISTER_TABLE          *RegisterTable,
-  IN EFI_CPU_PHYSICAL_LOCATION   *ApLocation,
-  IN CPU_STATUS_INFORMATION      *CpuStatus,
-  IN PROGRAM_CPU_REGISTER_FLAGS  *CpuFlags
-  )
-{
-  CPU_REGISTER_TABLE_ENTRY  *RegisterTableEntry;
-  UINTN                     Index;
-  UINTN                     Value;
-  CPU_REGISTER_TABLE_ENTRY  *RegisterTableEntryHead;
-  volatile UINT32           *SemaphorePtr;
-  UINT32                    FirstThread;
-  UINT32                    CurrentThread;
-  UINT32                    CurrentCore;
-  UINTN                     ProcessorIndex;
-  UINT32                    *ThreadCountPerPackage;
-  UINT8                     *ThreadCountPerCore;
-  EFI_STATUS                Status;
-  UINT64                    CurrentValue;
-
-  //
-  // Traverse Register Table of this logical processor
-  //
-  RegisterTableEntryHead = (CPU_REGISTER_TABLE_ENTRY *)(UINTN)RegisterTable->RegisterTableEntry;
-
-  for (Index = 0; Index < RegisterTable->TableLength; Index++) {
-    RegisterTableEntry = &RegisterTableEntryHead[Index];
-
-    //
-    // Check the type of specified register
-    //
-    switch (RegisterTableEntry->RegisterType) {
-      //
-      // The specified register is Control Register
-      //
-      case ControlRegister:
-        Status = ReadWriteCr (RegisterTableEntry->Index, TRUE, &Value);
-        if (EFI_ERROR (Status)) {
-          break;
-        }
-
-        if (RegisterTableEntry->TestThenWrite) {
-          CurrentValue = BitFieldRead64 (
-                           Value,
-                           RegisterTableEntry->ValidBitStart,
-                           RegisterTableEntry->ValidBitStart + RegisterTableEntry->ValidBitLength - 1
-                           );
-          if (CurrentValue == RegisterTableEntry->Value) {
-            break;
-          }
-        }
-
-        Value = (UINTN)BitFieldWrite64 (
-                         Value,
-                         RegisterTableEntry->ValidBitStart,
-                         RegisterTableEntry->ValidBitStart + RegisterTableEntry->ValidBitLength - 1,
-                         RegisterTableEntry->Value
-                         );
-        ReadWriteCr (RegisterTableEntry->Index, FALSE, &Value);
-        break;
-      //
-      // The specified register is Model Specific Register
-      //
-      case Msr:
-        if (RegisterTableEntry->TestThenWrite) {
-          Value = (UINTN)AsmReadMsr64 (RegisterTableEntry->Index);
-          if (RegisterTableEntry->ValidBitLength >= 64) {
-            if (Value == RegisterTableEntry->Value) {
-              break;
-            }
-          } else {
-            CurrentValue = BitFieldRead64 (
-                             Value,
-                             RegisterTableEntry->ValidBitStart,
-                             RegisterTableEntry->ValidBitStart + RegisterTableEntry->ValidBitLength - 1
-                             );
-            if (CurrentValue == RegisterTableEntry->Value) {
-              break;
-            }
-          }
-        }
-
-        //
-        // If this function is called to restore register setting after INIT signal,
-        // there is no need to restore MSRs in register table.
-        //
-        if (RegisterTableEntry->ValidBitLength >= 64) {
-          //
-          // If length is not less than 64 bits, then directly write without reading
-          //
-          AsmWriteMsr64 (
-            RegisterTableEntry->Index,
-            RegisterTableEntry->Value
-            );
-        } else {
-          //
-          // Set the bit section according to bit start and length
-          //
-          AsmMsrBitFieldWrite64 (
-            RegisterTableEntry->Index,
-            RegisterTableEntry->ValidBitStart,
-            RegisterTableEntry->ValidBitStart + RegisterTableEntry->ValidBitLength - 1,
-            RegisterTableEntry->Value
-            );
-        }
-
-        break;
-      //
-      // MemoryMapped operations
-      //
-      case MemoryMapped:
-        AcquireSpinLock (&CpuFlags->MemoryMappedLock);
-        MmioBitFieldWrite32 (
-          (UINTN)(RegisterTableEntry->Index | LShiftU64 (RegisterTableEntry->HighIndex, 32)),
-          RegisterTableEntry->ValidBitStart,
-          RegisterTableEntry->ValidBitStart + RegisterTableEntry->ValidBitLength - 1,
-          (UINT32)RegisterTableEntry->Value
-          );
-        ReleaseSpinLock (&CpuFlags->MemoryMappedLock);
-        break;
-      //
-      // Enable or disable cache
-      //
-      case CacheControl:
-        //
-        // If value of the entry is 0, then disable cache.  Otherwise, enable cache.
-        //
-        if (RegisterTableEntry->Value == 0) {
-          AsmDisableCache ();
-        } else {
-          AsmEnableCache ();
-        }
-
-        break;
-
-      case Semaphore:
-        // Semaphore works logic like below:
-        //
-        //  V(x) = LibReleaseSemaphore (Semaphore[FirstThread + x]);
-        //  P(x) = LibWaitForSemaphore (Semaphore[FirstThread + x]);
-        //
-        //  All threads (T0...Tn) waits in P() line and continues running
-        //  together.
-        //
-        //
-        //  T0             T1            ...           Tn
-        //
-        //  V(0...n)       V(0...n)      ...           V(0...n)
-        //  n * P(0)       n * P(1)      ...           n * P(n)
-        //
-        ASSERT (
-          (ApLocation != NULL) &&
-          (CpuStatus->ThreadCountPerPackage != 0) &&
-          (CpuStatus->ThreadCountPerCore != 0) &&
-          (CpuFlags->CoreSemaphoreCount != NULL) &&
-          (CpuFlags->PackageSemaphoreCount != NULL)
-          );
-        switch (RegisterTableEntry->Value) {
-          case CoreDepType:
-            SemaphorePtr       = CpuFlags->CoreSemaphoreCount;
-            ThreadCountPerCore = (UINT8 *)(UINTN)CpuStatus->ThreadCountPerCore;
-
-            CurrentCore = ApLocation->Package * CpuStatus->MaxCoreCount + ApLocation->Core;
-            //
-            // Get Offset info for the first thread in the core which current thread belongs to.
-            //
-            FirstThread   = CurrentCore * CpuStatus->MaxThreadCount;
-            CurrentThread = FirstThread + ApLocation->Thread;
-
-            //
-            // Different cores may have different valid threads in them. If driver maintail clearly
-            // thread index in different cores, the logic will be much complicated.
-            // Here driver just simply records the max thread number in all cores and use it as expect
-            // thread number for all cores.
-            // In below two steps logic, first current thread will Release semaphore for each thread
-            // in current core. Maybe some threads are not valid in this core, but driver don't
-            // care. Second, driver will let current thread wait semaphore for all valid threads in
-            // current core. Because only the valid threads will do release semaphore for this
-            // thread, driver here only need to wait the valid thread count.
-            //
-
-            //
-            // First Notify ALL THREADs in current Core that this thread is ready.
-            //
-            for (ProcessorIndex = 0; ProcessorIndex < CpuStatus->MaxThreadCount; ProcessorIndex++) {
-              S3ReleaseSemaphore (&SemaphorePtr[FirstThread + ProcessorIndex]);
-            }
-
-            //
-            // Second, check whether all VALID THREADs (not all threads) in current core are ready.
-            //
-            for (ProcessorIndex = 0; ProcessorIndex < ThreadCountPerCore[CurrentCore]; ProcessorIndex++) {
-              S3WaitForSemaphore (&SemaphorePtr[CurrentThread]);
-            }
-
-            break;
-
-          case PackageDepType:
-            SemaphorePtr          = CpuFlags->PackageSemaphoreCount;
-            ThreadCountPerPackage = (UINT32 *)(UINTN)CpuStatus->ThreadCountPerPackage;
-            //
-            // Get Offset info for the first thread in the package which current thread belongs to.
-            //
-            FirstThread = ApLocation->Package * CpuStatus->MaxCoreCount * CpuStatus->MaxThreadCount;
-            //
-            // Get the possible threads count for current package.
-            //
-            CurrentThread = FirstThread + CpuStatus->MaxThreadCount * ApLocation->Core + ApLocation->Thread;
-
-            //
-            // Different packages may have different valid threads in them. If driver maintail clearly
-            // thread index in different packages, the logic will be much complicated.
-            // Here driver just simply records the max thread number in all packages and use it as expect
-            // thread number for all packages.
-            // In below two steps logic, first current thread will Release semaphore for each thread
-            // in current package. Maybe some threads are not valid in this package, but driver don't
-            // care. Second, driver will let current thread wait semaphore for all valid threads in
-            // current package. Because only the valid threads will do release semaphore for this
-            // thread, driver here only need to wait the valid thread count.
-            //
-
-            //
-            // First Notify ALL THREADS in current package that this thread is ready.
-            //
-            for (ProcessorIndex = 0; ProcessorIndex < CpuStatus->MaxThreadCount * CpuStatus->MaxCoreCount; ProcessorIndex++) {
-              S3ReleaseSemaphore (&SemaphorePtr[FirstThread + ProcessorIndex]);
-            }
-
-            //
-            // Second, check whether VALID THREADS (not all threads) in current package are ready.
-            //
-            for (ProcessorIndex = 0; ProcessorIndex < ThreadCountPerPackage[ApLocation->Package]; ProcessorIndex++) {
-              S3WaitForSemaphore (&SemaphorePtr[CurrentThread]);
-            }
-
-            break;
-
-          default:
-            break;
-        }
-
-        break;
-
-      default:
-        break;
-    }
-  }
-}
-
-/**
-
-  Set Processor register for one AP.
-
-  @param     PreSmmRegisterTable     Use pre Smm register table or register table.
-
-**/
-VOID
-SetRegister (
-  IN BOOLEAN  PreSmmRegisterTable
-  )
-{
-  CPU_FEATURE_INIT_DATA  *FeatureInitData;
-  CPU_REGISTER_TABLE     *RegisterTable;
-  CPU_REGISTER_TABLE     *RegisterTables;
-  UINT32                 InitApicId;
-  UINTN                  ProcIndex;
-  UINTN                  Index;
-
-  FeatureInitData = &mAcpiCpuData.CpuFeatureInitData;
-
-  if (PreSmmRegisterTable) {
-    RegisterTables = (CPU_REGISTER_TABLE *)(UINTN)FeatureInitData->PreSmmInitRegisterTable;
-  } else {
-    RegisterTables = (CPU_REGISTER_TABLE *)(UINTN)FeatureInitData->RegisterTable;
-  }
-
-  if (RegisterTables == NULL) {
-    return;
-  }
-
-  InitApicId    = GetInitialApicId ();
-  RegisterTable = NULL;
-  ProcIndex     = (UINTN)-1;
-  for (Index = 0; Index < mAcpiCpuData.NumberOfCpus; Index++) {
-    if (RegisterTables[Index].InitialApicId == InitApicId) {
-      RegisterTable = &RegisterTables[Index];
-      ProcIndex     = Index;
-      break;
-    }
-  }
-
-  ASSERT (RegisterTable != NULL);
-
-  if (FeatureInitData->ApLocation != 0) {
-    ProgramProcessorRegister (
-      RegisterTable,
-      (EFI_CPU_PHYSICAL_LOCATION *)(UINTN)FeatureInitData->ApLocation + ProcIndex,
-      &FeatureInitData->CpuStatus,
-      &mCpuFlags
-      );
-  } else {
-    ProgramProcessorRegister (
-      RegisterTable,
-      NULL,
-      &FeatureInitData->CpuStatus,
-      &mCpuFlags
-      );
-  }
-}
-
 /**
   The function is invoked before SMBASE relocation in S3 path to restores CPU status.
 
@@ -524,8 +105,6 @@ InitializeCpuBeforeRebase (
   IN BOOLEAN  IsBsp
   )
 {
-  SetRegister (TRUE);
-
   ProgramVirtualWireMode ();
   if (!IsBsp) {
     DisableLvtInterrupts ();
@@ -563,8 +142,6 @@ InitializeCpuAfterRebase (
   UINTN  TopOfStack;
   UINT8  Stack[128];
 
-  SetRegister (FALSE);
-
   if (mSmmS3ResumeState->MpService2Ppi == 0) {
     if (IsBsp) {
       while (mNumberToFinish > 0) {
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119005): https://edk2.groups.io/g/devel/message/119005
Mute This Topic: https://groups.io/mt/106150766/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [edk2-devel] [Patch V2 09/18] UefiCpuPkg: Disable PG in IA32 ApLoopCode
  2024-05-17  9:45 [edk2-devel] [Patch V2 00/18] Remove some S3 related code in CpuS3.c of smm cpu driver duntan
                   ` (7 preceding siblings ...)
  2024-05-17  9:46 ` [edk2-devel] [Patch V2 08/18] UefiCpuPkg: Remove code to set register table duntan
@ 2024-05-17  9:46 ` duntan
  2024-05-17  9:46 ` [edk2-devel] [Patch V2 10/18] UefiCpuPkg:Abstract some DxeMpLib code to function duntan
                   ` (12 subsequent siblings)
  21 siblings, 0 replies; 30+ messages in thread
From: duntan @ 2024-05-17  9:46 UTC (permalink / raw)
  To: devel; +Cc: Ray Ni, Rahul Kumar, Gerd Hoffmann, Jiaxin Wu

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>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
---
 UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
index d117f09ef5..c0fe631635 100644
--- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
@@ -1,5 +1,5 @@
 ;------------------------------------------------------------------------------ ;
-; Copyright (c) 2015 - 2023, Intel Corporation. All rights reserved.<BR>
+; Copyright (c) 2015 - 2024, Intel Corporation. All rights reserved.<BR>
 ; SPDX-License-Identifier: BSD-2-Clause-Patent
 ;
 ; Module Name:
@@ -225,6 +225,10 @@ RendezvousFunnelProcEnd:
 ;  specific to SEV-ES support and are not applicable on IA32.
 ;-------------------------------------------------------------------------------------
 AsmRelocateApLoopGenericStart:
+    mov        eax, cr0
+    btr        eax, 31             ; Clear CR0.PG
+    mov        cr0, eax            ; Disable paging since the page table might be unavailiable
+
     mov        eax, esp
     mov        esp, [eax + 12]     ; TopOfApStack
     push       dword [eax]         ; push return address for stack trace
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119006): https://edk2.groups.io/g/devel/message/119006
Mute This Topic: https://groups.io/mt/106150767/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [edk2-devel] [Patch V2 10/18] UefiCpuPkg:Abstract some DxeMpLib code to function
  2024-05-17  9:45 [edk2-devel] [Patch V2 00/18] Remove some S3 related code in CpuS3.c of smm cpu driver duntan
                   ` (8 preceding siblings ...)
  2024-05-17  9:46 ` [edk2-devel] [Patch V2 09/18] UefiCpuPkg: Disable PG in IA32 ApLoopCode duntan
@ 2024-05-17  9:46 ` duntan
  2024-05-17  9:46 ` [edk2-devel] [Patch V2 11/18] UefiCpuPkg:Move some code in DxeMpLib to common place duntan
                   ` (11 subsequent siblings)
  21 siblings, 0 replies; 30+ messages in thread
From: duntan @ 2024-05-17  9:46 UTC (permalink / raw)
  To: devel; +Cc: Ray Ni, Rahul Kumar, Gerd Hoffmann, Jiaxin Wu

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>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
---
 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 222 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------------------------------------------------------------------
 1 file changed, 140 insertions(+), 82 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index 57ddb86600..5f0a87c024 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
@@ -1,7 +1,7 @@
 /** @file
   MP initialize support functions for DXE phase.
 
-  Copyright (c) 2016 - 2023, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2016 - 2024, Intel Corporation. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -422,6 +422,144 @@ RelocateApLoop (
   ASSERT (FALSE);
 }
 
+/**
+  Allocate buffer for ApLoopCode.
+
+  @param[in]      Pages    Number of pages to allocate.
+  @param[in, out] Address  Pointer to the allocated buffer.
+**/
+VOID
+AllocateApLoopCodeBuffer (
+  IN UINTN                     Pages,
+  IN OUT EFI_PHYSICAL_ADDRESS  *Address
+  )
+{
+  EFI_STATUS  Status;
+
+  Status = gBS->AllocatePages (
+                  AllocateMaxAddress,
+                  EfiReservedMemoryType,
+                  Pages,
+                  Address
+                  );
+  ASSERT_EFI_ERROR (Status);
+}
+
+/**
+  Remove Nx protection for the range specific by BaseAddress and Length.
+
+  The PEI implementation uses CpuPageTableLib to change the attribute.
+  The DXE implementation uses gDS to change the attribute.
+
+  @param[in] BaseAddress  BaseAddress of the range.
+  @param[in] Length       Length of the range.
+**/
+VOID
+RemoveNxprotection (
+  IN EFI_PHYSICAL_ADDRESS  BaseAddress,
+  IN UINTN                 Length
+  )
+{
+  EFI_STATUS                       Status;
+  EFI_GCD_MEMORY_SPACE_DESCRIPTOR  MemDesc;
+
+  //
+  // TODO: Check EFI_MEMORY_XP bit set or not once it's available in DXE GCD
+  //       service.
+  //
+  Status = gDS->GetMemorySpaceDescriptor (BaseAddress, &MemDesc);
+  if (!EFI_ERROR (Status)) {
+    gDS->SetMemorySpaceAttributes (
+           BaseAddress,
+           Length,
+           MemDesc.Attributes & (~EFI_MEMORY_XP)
+           );
+  }
+}
+
+/**
+  Prepare ApLoopCode.
+
+  @param[in] CpuMpData  Pointer to CpuMpData.
+**/
+VOID
+PrepareApLoopCode (
+  IN CPU_MP_DATA  *CpuMpData
+  )
+{
+  EFI_PHYSICAL_ADDRESS     Address;
+  MP_ASSEMBLY_ADDRESS_MAP  *AddressMap;
+  UINT8                    *ApLoopFunc;
+  UINTN                    ApLoopFuncSize;
+  UINTN                    StackPages;
+  UINTN                    FuncPages;
+  IA32_CR0                 Cr0;
+
+  AddressMap = &CpuMpData->AddressMap;
+  if (CpuMpData->UseSevEsAPMethod) {
+    //
+    // 64-bit AMD processors with SEV-ES
+    //
+    Address        = BASE_4GB - 1;
+    ApLoopFunc     = AddressMap->RelocateApLoopFuncAddressAmdSev;
+    ApLoopFuncSize = AddressMap->RelocateApLoopFuncSizeAmdSev;
+  } else {
+    //
+    // Intel processors (32-bit or 64-bit), 32-bit AMD processors, or 64-bit AMD processors without SEV-ES
+    //
+    Address        = MAX_ADDRESS;
+    ApLoopFunc     = AddressMap->RelocateApLoopFuncAddressGeneric;
+    ApLoopFuncSize = AddressMap->RelocateApLoopFuncSizeGeneric;
+  }
+
+  //
+  // Avoid APs access invalid buffer data which allocated by BootServices,
+  // so we will allocate reserved data for AP loop code. We also need to
+  // allocate this buffer below 4GB due to APs may be transferred to 32bit
+  // protected mode on long mode DXE.
+  // Allocating it in advance since memory services are not available in
+  // Exit Boot Services callback function.
+  //
+  // +------------+ (TopOfApStack)
+  // |  Stack * N |
+  // +------------+ (stack base, 4k aligned)
+  // |  Padding   |
+  // +------------+
+  // |  Ap Loop   |
+  // +------------+ ((low address, 4k-aligned)
+  //
+
+  StackPages = EFI_SIZE_TO_PAGES (CpuMpData->CpuCount * AP_SAFE_STACK_SIZE);
+  FuncPages  = EFI_SIZE_TO_PAGES (ApLoopFuncSize);
+
+  AllocateApLoopCodeBuffer (StackPages + FuncPages, &Address);
+  ASSERT (Address != 0);
+
+  Cr0.UintN = AsmReadCr0 ();
+  if (Cr0.Bits.PG != 0) {
+    //
+    // Make sure that the buffer memory is executable if NX protection is enabled
+    // for EfiReservedMemoryType.
+    //
+    RemoveNxprotection (Address, EFI_PAGES_TO_SIZE (FuncPages));
+  }
+
+  mReservedTopOfApStack = (UINTN)Address + EFI_PAGES_TO_SIZE (StackPages+FuncPages);
+  ASSERT ((mReservedTopOfApStack & (UINTN)(CPU_STACK_ALIGNMENT - 1)) == 0);
+  mReservedApLoop.Data = (VOID *)(UINTN)Address;
+  ASSERT (mReservedApLoop.Data != NULL);
+  CopyMem (mReservedApLoop.Data, ApLoopFunc, ApLoopFuncSize);
+  if (!CpuMpData->UseSevEsAPMethod) {
+    //
+    // processors without SEV-ES and paging is enabled
+    //
+    mApPageTable = CreatePageTable (
+                     (UINTN)Address,
+                     EFI_PAGES_TO_SIZE (StackPages+FuncPages)
+                     );
+  }
+}
+
 /**
   Callback function for ExitBootServices.
 
@@ -477,16 +615,10 @@ InitMpGlobalData (
   )
 {
   EFI_STATUS                       Status;
-  EFI_PHYSICAL_ADDRESS             Address;
   UINTN                            Index;
   EFI_GCD_MEMORY_SPACE_DESCRIPTOR  MemDesc;
   UINTN                            StackBase;
   CPU_INFO_IN_HOB                  *CpuInfoInHob;
-  MP_ASSEMBLY_ADDRESS_MAP          *AddressMap;
-  UINT8                            *ApLoopFunc;
-  UINTN                            ApLoopFuncSize;
-  UINTN                            StackPages;
-  UINTN                            FuncPages;
 
   SaveCpuMpData (CpuMpData);
 
@@ -541,81 +673,7 @@ InitMpGlobalData (
     }
   }
 
-  AddressMap = &CpuMpData->AddressMap;
-  if (CpuMpData->UseSevEsAPMethod) {
-    //
-    // 64-bit AMD processors with SEV-ES
-    //
-    Address        = BASE_4GB - 1;
-    ApLoopFunc     = AddressMap->RelocateApLoopFuncAddressAmdSev;
-    ApLoopFuncSize = AddressMap->RelocateApLoopFuncSizeAmdSev;
-  } else {
-    //
-    // Intel processors (32-bit or 64-bit), 32-bit AMD processors, or 64-bit AMD processors without SEV-ES
-    //
-    Address        = MAX_ADDRESS;
-    ApLoopFunc     = AddressMap->RelocateApLoopFuncAddressGeneric;
-    ApLoopFuncSize = AddressMap->RelocateApLoopFuncSizeGeneric;
-  }
-
-  //
-  // Avoid APs access invalid buffer data which allocated by BootServices,
-  // so we will allocate reserved data for AP loop code. We also need to
-  // allocate this buffer below 4GB due to APs may be transferred to 32bit
-  // protected mode on long mode DXE.
-  // Allocating it in advance since memory services are not available in
-  // Exit Boot Services callback function.
-  //
-  // +------------+ (TopOfApStack)
-  // |  Stack * N |
-  // +------------+ (stack base, 4k aligned)
-  // |  Padding   |
-  // +------------+
-  // |  Ap Loop   |
-  // +------------+ ((low address, 4k-aligned)
-  //
-
-  StackPages = EFI_SIZE_TO_PAGES (CpuMpData->CpuCount * AP_SAFE_STACK_SIZE);
-  FuncPages  = EFI_SIZE_TO_PAGES (ApLoopFuncSize);
-
-  Status = gBS->AllocatePages (
-                  AllocateMaxAddress,
-                  EfiReservedMemoryType,
-                  StackPages + FuncPages,
-                  &Address
-                  );
-  ASSERT_EFI_ERROR (Status);
-
-  //
-  // Make sure that the buffer memory is executable if NX protection is enabled
-  // for EfiReservedMemoryType.
-  //
-  // TODO: Check EFI_MEMORY_XP bit set or not once it's available in DXE GCD
-  //       service.
-  //
-  Status = gDS->GetMemorySpaceDescriptor (Address, &MemDesc);
-  if (!EFI_ERROR (Status)) {
-    gDS->SetMemorySpaceAttributes (
-           Address,
-           EFI_PAGES_TO_SIZE (FuncPages),
-           MemDesc.Attributes & (~EFI_MEMORY_XP)
-           );
-  }
-
-  mReservedTopOfApStack = (UINTN)Address + EFI_PAGES_TO_SIZE (StackPages+FuncPages);
-  ASSERT ((mReservedTopOfApStack & (UINTN)(CPU_STACK_ALIGNMENT - 1)) == 0);
-  mReservedApLoop.Data = (VOID *)(UINTN)Address;
-  ASSERT (mReservedApLoop.Data != NULL);
-  CopyMem (mReservedApLoop.Data, ApLoopFunc, ApLoopFuncSize);
-  if (!CpuMpData->UseSevEsAPMethod) {
-    //
-    // processors without SEV-ES
-    //
-    mApPageTable = CreatePageTable (
-                     (UINTN)Address,
-                     EFI_PAGES_TO_SIZE (StackPages+FuncPages)
-                     );
-  }
+  PrepareApLoopCode (CpuMpData);
 
   Status = gBS->CreateEvent (
                   EVT_TIMER | EVT_NOTIFY_SIGNAL,
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119007): https://edk2.groups.io/g/devel/message/119007
Mute This Topic: https://groups.io/mt/106150768/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [edk2-devel] [Patch V2 11/18] UefiCpuPkg:Move some code in DxeMpLib to common place
  2024-05-17  9:45 [edk2-devel] [Patch V2 00/18] Remove some S3 related code in CpuS3.c of smm cpu driver duntan
                   ` (9 preceding siblings ...)
  2024-05-17  9:46 ` [edk2-devel] [Patch V2 10/18] UefiCpuPkg:Abstract some DxeMpLib code to function duntan
@ 2024-05-17  9:46 ` duntan
  2024-05-17  9:46 ` [edk2-devel] [Patch V2 12/18] UefiCpuPkg: Install gEdkiiEndOfS3ResumeGuid in S3Resume duntan
                   ` (10 subsequent siblings)
  21 siblings, 0 replies; 30+ messages in thread
From: duntan @ 2024-05-17  9:46 UTC (permalink / raw)
  To: devel; +Cc: Ray Ni, Rahul Kumar, Gerd Hoffmann, Jiaxin Wu

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>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
---
 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 151 +++++--------------------------------------------------------------------------------------------------------------------------------------------------
 UefiCpuPkg/Library/MpInitLib/MpLib.c    | 144 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 UefiCpuPkg/Library/MpInitLib/MpLib.h    |  53 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 199 insertions(+), 149 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index 5f0a87c024..f9c5c92c22 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
@@ -20,15 +20,11 @@
 
 #define  AP_SAFE_STACK_SIZE  128
 
-CPU_MP_DATA             *mCpuMpData                  = NULL;
-EFI_EVENT               mCheckAllApsEvent            = NULL;
-EFI_EVENT               mMpInitExitBootServicesEvent = NULL;
-EFI_EVENT               mLegacyBootEvent             = NULL;
-volatile BOOLEAN        mStopCheckAllApsStatus       = TRUE;
-RELOCATE_AP_LOOP_ENTRY  mReservedApLoop;
-UINTN                   mReservedTopOfApStack;
-volatile UINT32         mNumberToFinish = 0;
-UINTN                   mApPageTable;
+CPU_MP_DATA       *mCpuMpData                  = NULL;
+EFI_EVENT         mCheckAllApsEvent            = NULL;
+EFI_EVENT         mMpInitExitBootServicesEvent = NULL;
+EFI_EVENT         mLegacyBootEvent             = NULL;
+volatile BOOLEAN  mStopCheckAllApsStatus       = TRUE;
 
 //
 // Begin wakeup buffer allocation below 0x88000
@@ -368,60 +364,6 @@ GetProtectedModeCS (
   return Index * 8;
 }
 
-/**
-  Do sync on APs.
-
-  @param[in, out] Buffer  Pointer to private data buffer.
-**/
-VOID
-EFIAPI
-RelocateApLoop (
-  IN OUT VOID  *Buffer
-  )
-{
-  CPU_MP_DATA  *CpuMpData;
-  BOOLEAN      MwaitSupport;
-  UINTN        ProcessorNumber;
-  UINTN        StackStart;
-
-  MpInitLibWhoAmI (&ProcessorNumber);
-  CpuMpData    = GetCpuMpData ();
-  MwaitSupport = IsMwaitSupport ();
-  if (CpuMpData->UseSevEsAPMethod) {
-    //
-    // 64-bit AMD processors with SEV-ES
-    //
-    StackStart = CpuMpData->SevEsAPResetStackStart;
-    mReservedApLoop.AmdSevEntry (
-                      MwaitSupport,
-                      CpuMpData->ApTargetCState,
-                      CpuMpData->PmCodeSegment,
-                      StackStart - ProcessorNumber * AP_SAFE_STACK_SIZE,
-                      (UINTN)&mNumberToFinish,
-                      CpuMpData->Pm16CodeSegment,
-                      CpuMpData->SevEsAPBuffer,
-                      CpuMpData->WakeupBuffer
-                      );
-  } else {
-    //
-    // Intel processors (32-bit or 64-bit), 32-bit AMD processors, or 64-bit AMD processors without SEV-ES
-    //
-    StackStart = mReservedTopOfApStack;
-    mReservedApLoop.GenericEntry (
-                      MwaitSupport,
-                      CpuMpData->ApTargetCState,
-                      StackStart - ProcessorNumber * AP_SAFE_STACK_SIZE,
-                      (UINTN)&mNumberToFinish,
-                      mApPageTable
-                      );
-  }
-
-  //
-  // It should never reach here
-  //
-  ASSERT (FALSE);
-}
-
 /**
   Allocate buffer for ApLoopCode.
 
@@ -477,89 +419,6 @@ RemoveNxprotection (
   }
 }
 
-/**
-  Prepare ApLoopCode.
-
-  @param[in] CpuMpData  Pointer to CpuMpData.
-**/
-VOID
-PrepareApLoopCode (
-  IN CPU_MP_DATA  *CpuMpData
-  )
-{
-  EFI_PHYSICAL_ADDRESS     Address;
-  MP_ASSEMBLY_ADDRESS_MAP  *AddressMap;
-  UINT8                    *ApLoopFunc;
-  UINTN                    ApLoopFuncSize;
-  UINTN                    StackPages;
-  UINTN                    FuncPages;
-  IA32_CR0                 Cr0;
-
-  AddressMap = &CpuMpData->AddressMap;
-  if (CpuMpData->UseSevEsAPMethod) {
-    //
-    // 64-bit AMD processors with SEV-ES
-    //
-    Address        = BASE_4GB - 1;
-    ApLoopFunc     = AddressMap->RelocateApLoopFuncAddressAmdSev;
-    ApLoopFuncSize = AddressMap->RelocateApLoopFuncSizeAmdSev;
-  } else {
-    //
-    // Intel processors (32-bit or 64-bit), 32-bit AMD processors, or 64-bit AMD processors without SEV-ES
-    //
-    Address        = MAX_ADDRESS;
-    ApLoopFunc     = AddressMap->RelocateApLoopFuncAddressGeneric;
-    ApLoopFuncSize = AddressMap->RelocateApLoopFuncSizeGeneric;
-  }
-
-  //
-  // Avoid APs access invalid buffer data which allocated by BootServices,
-  // so we will allocate reserved data for AP loop code. We also need to
-  // allocate this buffer below 4GB due to APs may be transferred to 32bit
-  // protected mode on long mode DXE.
-  // Allocating it in advance since memory services are not available in
-  // Exit Boot Services callback function.
-  //
-  // +------------+ (TopOfApStack)
-  // |  Stack * N |
-  // +------------+ (stack base, 4k aligned)
-  // |  Padding   |
-  // +------------+
-  // |  Ap Loop   |
-  // +------------+ ((low address, 4k-aligned)
-  //
-
-  StackPages = EFI_SIZE_TO_PAGES (CpuMpData->CpuCount * AP_SAFE_STACK_SIZE);
-  FuncPages  = EFI_SIZE_TO_PAGES (ApLoopFuncSize);
-
-  AllocateApLoopCodeBuffer (StackPages + FuncPages, &Address);
-  ASSERT (Address != 0);
-
-  Cr0.UintN = AsmReadCr0 ();
-  if (Cr0.Bits.PG != 0) {
-    //
-    // Make sure that the buffer memory is executable if NX protection is enabled
-    // for EfiReservedMemoryType.
-    //
-    RemoveNxprotection (Address, EFI_PAGES_TO_SIZE (FuncPages));
-  }
-
-  mReservedTopOfApStack = (UINTN)Address + EFI_PAGES_TO_SIZE (StackPages+FuncPages);
-  ASSERT ((mReservedTopOfApStack & (UINTN)(CPU_STACK_ALIGNMENT - 1)) == 0);
-  mReservedApLoop.Data = (VOID *)(UINTN)Address;
-  ASSERT (mReservedApLoop.Data != NULL);
-  CopyMem (mReservedApLoop.Data, ApLoopFunc, ApLoopFuncSize);
-  if (!CpuMpData->UseSevEsAPMethod) {
-    //
-    // processors without SEV-ES and paging is enabled
-    //
-    mApPageTable = CreatePageTable (
-                     (UINTN)Address,
-                     EFI_PAGES_TO_SIZE (StackPages+FuncPages)
-                     );
-  }
-}
-
 /**
   Callback function for ExitBootServices.
 
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index d724456502..cbd43c776a 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -1,7 +1,7 @@
 /** @file
   CPU MP Initialize Library common functions.
 
-  Copyright (c) 2016 - 2022, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2016 - 2024, Intel Corporation. All rights reserved.<BR>
   Copyright (c) 2020 - 2024, AMD Inc. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -17,6 +17,11 @@ EFI_GUID  mCpuInitMpLibHobGuid = CPU_INIT_MP_LIB_HOB_GUID;
 EFI_GUID  mMpHandOffGuid       = MP_HANDOFF_GUID;
 EFI_GUID  mMpHandOffConfigGuid = MP_HANDOFF_CONFIG_GUID;
 
+RELOCATE_AP_LOOP_ENTRY  mReservedApLoop;
+UINTN                   mReservedTopOfApStack;
+volatile UINT32         mNumberToFinish = 0;
+UINTN                   mApPageTable;
+
 /**
   Save the volatile registers required to be restored following INIT IPI.
 
@@ -3227,3 +3232,140 @@ ConfidentialComputingGuestHas (
 
   return (CurrentAttr == Attr);
 }
+
+/**
+  Do sync on APs.
+
+  @param[in, out] Buffer  Pointer to private data buffer.
+**/
+VOID
+EFIAPI
+RelocateApLoop (
+  IN OUT VOID  *Buffer
+  )
+{
+  CPU_MP_DATA  *CpuMpData;
+  BOOLEAN      MwaitSupport;
+  UINTN        ProcessorNumber;
+  UINTN        StackStart;
+
+  MpInitLibWhoAmI (&ProcessorNumber);
+  CpuMpData    = GetCpuMpData ();
+  MwaitSupport = IsMwaitSupport ();
+  if (CpuMpData->UseSevEsAPMethod) {
+    //
+    // 64-bit AMD processors with SEV-ES
+    //
+    StackStart = CpuMpData->SevEsAPResetStackStart;
+    mReservedApLoop.AmdSevEntry (
+                      MwaitSupport,
+                      CpuMpData->ApTargetCState,
+                      CpuMpData->PmCodeSegment,
+                      StackStart - ProcessorNumber * AP_SAFE_STACK_SIZE,
+                      (UINTN)&mNumberToFinish,
+                      CpuMpData->Pm16CodeSegment,
+                      CpuMpData->SevEsAPBuffer,
+                      CpuMpData->WakeupBuffer
+                      );
+  } else {
+    //
+    // Intel processors (32-bit or 64-bit), 32-bit AMD processors, or 64-bit AMD processors without SEV-ES
+    //
+    StackStart = mReservedTopOfApStack;
+    mReservedApLoop.GenericEntry (
+                      MwaitSupport,
+                      CpuMpData->ApTargetCState,
+                      StackStart - ProcessorNumber * AP_SAFE_STACK_SIZE,
+                      (UINTN)&mNumberToFinish,
+                      mApPageTable
+                      );
+  }
+
+  //
+  // It should never reach here
+  //
+  ASSERT (FALSE);
+}
+
+/**
+  Prepare ApLoopCode.
+
+  @param[in] CpuMpData  Pointer to CpuMpData.
+**/
+VOID
+PrepareApLoopCode (
+  IN CPU_MP_DATA  *CpuMpData
+  )
+{
+  EFI_PHYSICAL_ADDRESS     Address;
+  MP_ASSEMBLY_ADDRESS_MAP  *AddressMap;
+  UINT8                    *ApLoopFunc;
+  UINTN                    ApLoopFuncSize;
+  UINTN                    StackPages;
+  UINTN                    FuncPages;
+  IA32_CR0                 Cr0;
+
+  AddressMap = &CpuMpData->AddressMap;
+  if (CpuMpData->UseSevEsAPMethod) {
+    //
+    // 64-bit AMD processors with SEV-ES
+    //
+    Address        = BASE_4GB - 1;
+    ApLoopFunc     = AddressMap->RelocateApLoopFuncAddressAmdSev;
+    ApLoopFuncSize = AddressMap->RelocateApLoopFuncSizeAmdSev;
+  } else {
+    //
+    // Intel processors (32-bit or 64-bit), 32-bit AMD processors, or 64-bit AMD processors without SEV-ES
+    //
+    Address        = MAX_ADDRESS;
+    ApLoopFunc     = AddressMap->RelocateApLoopFuncAddressGeneric;
+    ApLoopFuncSize = AddressMap->RelocateApLoopFuncSizeGeneric;
+  }
+
+  //
+  // Avoid APs access invalid buffer data which allocated by BootServices,
+  // so we will allocate reserved data for AP loop code. We also need to
+  // allocate this buffer below 4GB due to APs may be transferred to 32bit
+  // protected mode on long mode DXE.
+  // Allocating it in advance since memory services are not available in
+  // Exit Boot Services callback function.
+  //
+  // +------------+ (TopOfApStack)
+  // |  Stack * N |
+  // +------------+ (stack base, 4k aligned)
+  // |  Padding   |
+  // +------------+
+  // |  Ap Loop   |
+  // +------------+ ((low address, 4k-aligned)
+  //
+
+  StackPages = EFI_SIZE_TO_PAGES (CpuMpData->CpuCount * AP_SAFE_STACK_SIZE);
+  FuncPages  = EFI_SIZE_TO_PAGES (ApLoopFuncSize);
+
+  AllocateApLoopCodeBuffer (StackPages + FuncPages, &Address);
+  ASSERT (Address != 0);
+
+  Cr0.UintN = AsmReadCr0 ();
+  if (Cr0.Bits.PG != 0) {
+    //
+    // Make sure that the buffer memory is executable if NX protection is enabled
+    // for EfiReservedMemoryType.
+    //
+    RemoveNxprotection (Address, EFI_PAGES_TO_SIZE (FuncPages));
+  }
+
+  mReservedTopOfApStack = (UINTN)Address + EFI_PAGES_TO_SIZE (StackPages+FuncPages);
+  ASSERT ((mReservedTopOfApStack & (UINTN)(CPU_STACK_ALIGNMENT - 1)) == 0);
+  mReservedApLoop.Data = (VOID *)(UINTN)Address;
+  ASSERT (mReservedApLoop.Data != NULL);
+  CopyMem (mReservedApLoop.Data, ApLoopFunc, ApLoopFuncSize);
+  if (!CpuMpData->UseSevEsAPMethod) {
+    //
+    // processors without SEV-ES and paging is enabled
+    //
+    mApPageTable = CreatePageTable (
+                     (UINTN)Address,
+                     EFI_PAGES_TO_SIZE (StackPages+FuncPages)
+                     );
+  }
+}
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 179f8e585b..a4a33bf538 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -1,7 +1,7 @@
 /** @file
   Common header file for MP Initialize Library.
 
-  Copyright (c) 2016 - 2023, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2016 - 2024, Intel Corporation. All rights reserved.<BR>
   Copyright (c) 2020 - 2024, AMD Inc. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -357,7 +357,8 @@ typedef
   IN UINTN    StackStart
   );
 
-extern EFI_GUID  mCpuInitMpLibHobGuid;
+extern EFI_GUID         mCpuInitMpLibHobGuid;
+extern volatile UINT32  mNumberToFinish;
 
 /**
   Assembly code to place AP into safe loop mode.
@@ -933,4 +934,52 @@ AmdSevUpdateCpuMpData (
   IN CPU_MP_DATA  *CpuMpData
   );
 
+/**
+  Prepare ApLoopCode.
+
+  @param[in] CpuMpData  Pointer to CpuMpData.
+**/
+VOID
+PrepareApLoopCode (
+  IN CPU_MP_DATA  *CpuMpData
+  );
+
+/**
+  Do sync on APs.
+
+  @param[in, out] Buffer  Pointer to private data buffer.
+**/
+VOID
+EFIAPI
+RelocateApLoop (
+  IN OUT VOID  *Buffer
+  );
+
+/**
+  Allocate buffer for ApLoopCode.
+
+  @param[in]      Pages    Number of pages to allocate.
+  @param[in, out] Address  Pointer to the allocated buffer.
+**/
+VOID
+AllocateApLoopCodeBuffer (
+  IN UINTN                     Pages,
+  IN OUT EFI_PHYSICAL_ADDRESS  *Address
+  );
+
+/**
+  Remove Nx protection for the range specific by BaseAddress and Length.
+
+  The PEI implementation uses CpuPageTableLib to change the attribute.
+  The DXE implementation uses gDS to change the attribute.
+
+  @param[in] BaseAddress  BaseAddress of the range.
+  @param[in] Length       Length of the range.
+**/
+VOID
+RemoveNxprotection (
+  IN EFI_PHYSICAL_ADDRESS  BaseAddress,
+  IN UINTN                 Length
+  );
+
 #endif
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119008): https://edk2.groups.io/g/devel/message/119008
Mute This Topic: https://groups.io/mt/106150770/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [edk2-devel] [Patch V2 12/18] UefiCpuPkg: Install gEdkiiEndOfS3ResumeGuid in S3Resume
  2024-05-17  9:45 [edk2-devel] [Patch V2 00/18] Remove some S3 related code in CpuS3.c of smm cpu driver duntan
                   ` (10 preceding siblings ...)
  2024-05-17  9:46 ` [edk2-devel] [Patch V2 11/18] UefiCpuPkg:Move some code in DxeMpLib to common place duntan
@ 2024-05-17  9:46 ` duntan
  2024-05-17  9:46 ` [edk2-devel] [Patch V2 13/18] UefiCpuPkg:Relocate AP to new safe buffer in PeiMpLib duntan
                   ` (9 subsequent siblings)
  21 siblings, 0 replies; 30+ messages in thread
From: duntan @ 2024-05-17  9:46 UTC (permalink / raw)
  To: devel; +Cc: Ray Ni, Rahul Kumar, Gerd Hoffmann, Jiaxin Wu

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>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
---
 UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
index 078ae2d72d..e6dfa09c71 100644
--- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
+++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
@@ -260,6 +260,12 @@ EFI_PEI_PPI_DESCRIPTOR  mPpiListS3SmmInitDoneTable = {
   0
 };
 
+EFI_PEI_PPI_DESCRIPTOR  mPpiListEndOfS3ResumeTable = {
+  (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
+  &gEdkiiEndOfS3ResumeGuid,
+  0
+};
+
 //
 // Global Descriptor Table (GDT)
 //
@@ -490,6 +496,13 @@ S3ResumeBootOs (
   PERF_INMODULE_BEGIN ("EndOfS3Resume");
 
   DEBUG ((DEBUG_INFO, "Signal EndOfS3Resume\n"));
+
+  //
+  // Install EndOfS3Resume.
+  //
+  Status = PeiServicesInstallPpi (&mPpiListEndOfS3ResumeTable);
+  ASSERT_EFI_ERROR (Status);
+
   //
   // Signal EndOfS3Resume to SMM.
   //
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119009): https://edk2.groups.io/g/devel/message/119009
Mute This Topic: https://groups.io/mt/106150771/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [edk2-devel] [Patch V2 13/18] UefiCpuPkg:Relocate AP to new safe buffer in PeiMpLib
  2024-05-17  9:45 [edk2-devel] [Patch V2 00/18] Remove some S3 related code in CpuS3.c of smm cpu driver duntan
                   ` (11 preceding siblings ...)
  2024-05-17  9:46 ` [edk2-devel] [Patch V2 12/18] UefiCpuPkg: Install gEdkiiEndOfS3ResumeGuid in S3Resume duntan
@ 2024-05-17  9:46 ` duntan
  2024-05-17  9:46 ` [edk2-devel] [Patch V2 14/18] UefiCpuPkg:Remove code to handle APIC setting and Interrupt duntan
                   ` (8 subsequent siblings)
  21 siblings, 0 replies; 30+ messages in thread
From: duntan @ 2024-05-17  9:46 UTC (permalink / raw)
  To: devel; +Cc: Ray Ni, Rahul Kumar, Gerd Hoffmann, Jiaxin Wu

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>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
---
 UefiCpuPkg/Library/MpInitLib/MpLib.h          |   3 +++
 UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   6 +++++-
 UefiCpuPkg/Library/MpInitLib/PeiMpLib.c       | 154 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 161 insertions(+), 2 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index a4a33bf538..cc850c7dd5 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -33,6 +33,7 @@
 #include <Library/HobLib.h>
 #include <Library/PcdLib.h>
 #include <Library/MicrocodeLib.h>
+#include <Library/CpuPageTableLib.h>
 #include <ConfidentialComputingGuestAttr.h>
 
 #include <Register/Amd/Fam17Msr.h>
@@ -68,6 +69,8 @@
 //
 #define DEFAULT_MAX_MICROCODE_PATCH_NUM  8
 
+#define PAGING_4K_ADDRESS_MASK_64  0x000FFFFFFFFFF000ull
+
 //
 // Data structure for microcode patch information
 //
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
index e31e34b6f9..e4a7485fef 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
@@ -1,7 +1,7 @@
 ## @file
 #  MP Initialize Library instance for PEI driver.
 #
-#  Copyright (c) 2016 - 2021, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2016 - 2024, Intel Corporation. All rights reserved.<BR>
 #  Copyright (c) 2024, Loongson Technology Corporation Limited. All rights reserved.<BR>
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -25,10 +25,12 @@
 [Sources.IA32]
   Ia32/AmdSev.c
   Ia32/MpFuncs.nasm
+  Ia32/CreatePageTable.c
 
 [Sources.X64]
   X64/AmdSev.c
   X64/MpFuncs.nasm
+  X64/CreatePageTable.c
 
 [Sources.IA32, Sources.X64]
   AmdSev.c
@@ -64,6 +66,7 @@
   LocalApicLib
   MicrocodeLib
   MtrrLib
+  CpuPageTableLib
 
 [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbBase                       ## CONSUMES
@@ -87,6 +90,7 @@
   gEdkiiS3SmmInitDoneGuid
   gEdkiiMicrocodePatchHobGuid
   gGhcbApicIdsGuid                       ## SOMETIMES_CONSUMES
+  gEdkiiEndOfS3ResumeGuid
 
 [Guids.LoongArch64]
   gProcessorResourceHobGuid              ## SOMETIMES_CONSUMES  ## HOB
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
index 4d3acb491f..16a858d542 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
@@ -1,7 +1,7 @@
 /** @file
   MP initialize support functions for PEI phase.
 
-  Copyright (c) 2016 - 2020, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2016 - 2024, Intel Corporation. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -9,6 +9,7 @@
 #include "MpLib.h"
 #include <Library/PeiServicesLib.h>
 #include <Guid/S3SmmInitDone.h>
+#include <Guid/EndOfS3Resume.h>
 #include <Ppi/ShadowMicrocode.h>
 
 STATIC UINT64  mSevEsPeiWakeupBuffer = BASE_1MB;
@@ -449,6 +450,47 @@ BuildMicrocodeCacheHob (
   return;
 }
 
+/**
+  S3 SMM Init Done notification function.
+
+  @param  PeiServices      Indirect reference to the PEI Services Table.
+  @param  NotifyDesc       Address of the notification descriptor data structure.
+  @param  InvokePpi        Address of the PPI that was invoked.
+
+  @retval EFI_SUCCESS      The function completes successfully.
+
+**/
+EFI_STATUS
+EFIAPI
+NotifyOnEndOfS3Resume (
+  IN  EFI_PEI_SERVICES           **PeiServices,
+  IN  EFI_PEI_NOTIFY_DESCRIPTOR  *NotifyDesc,
+  IN  VOID                       *InvokePpi
+  )
+{
+  CPU_MP_DATA  *CpuMpData;
+
+  CpuMpData       = GetCpuMpData ();
+  mNumberToFinish = CpuMpData->CpuCount - 1;
+  WakeUpAP (CpuMpData, TRUE, 0, RelocateApLoop, NULL, TRUE);
+  while (mNumberToFinish > 0) {
+    CpuPause ();
+  }
+
+  DEBUG ((DEBUG_INFO, "%a() done!\n", __func__));
+
+  return EFI_SUCCESS;
+}
+
+//
+// Global function
+//
+EFI_PEI_NOTIFY_DESCRIPTOR  mEndOfS3ResumeNotifyDesc = {
+  EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST,
+  &gEdkiiEndOfS3ResumeGuid,
+  NotifyOnEndOfS3Resume
+};
+
 /**
   Initialize global data for MP support.
 
@@ -463,12 +505,16 @@ InitMpGlobalData (
 
   BuildMicrocodeCacheHob (CpuMpData);
   SaveCpuMpData (CpuMpData);
+  PrepareApLoopCode (CpuMpData);
 
   ///
   /// Install Notify
   ///
   Status = PeiServicesNotifyPpi (&mS3SmmInitDoneNotifyDesc);
   ASSERT_EFI_ERROR (Status);
+
+  Status = PeiServicesNotifyPpi (&mEndOfS3ResumeNotifyDesc);
+  ASSERT_EFI_ERROR (Status);
 }
 
 /**
@@ -815,3 +861,109 @@ PlatformShadowMicrocode (
 
   return EFI_SUCCESS;
 }
+
+/**
+  Allocate buffer for ApLoopCode.
+
+  @param[in]      Pages    Number of pages to allocate.
+  @param[in, out] Address  Pointer to the allocated buffer.
+**/
+VOID
+AllocateApLoopCodeBuffer (
+  IN UINTN                     Pages,
+  IN OUT EFI_PHYSICAL_ADDRESS  *Address
+  )
+{
+  EFI_STATUS  Status;
+
+  Status = PeiServicesAllocatePages (EfiACPIMemoryNVS, Pages, Address);
+  if (EFI_ERROR (Status)) {
+    *Address = 0;
+  }
+}
+
+/**
+  Remove Nx protection for the range specific by BaseAddress and Length.
+
+  The PEI implementation uses CpuPageTableLib to change the attribute.
+  The DXE implementation uses gDS to change the attribute.
+
+  @param[in] BaseAddress  BaseAddress of the range.
+  @param[in] Length       Length of the range.
+**/
+VOID
+RemoveNxprotection (
+  IN EFI_PHYSICAL_ADDRESS  BaseAddress,
+  IN UINTN                 Length
+  )
+{
+  EFI_STATUS                  Status;
+  UINTN                       PageTable;
+  EFI_PHYSICAL_ADDRESS        Buffer;
+  UINTN                       BufferSize;
+  IA32_MAP_ATTRIBUTE          MapAttribute;
+  IA32_MAP_ATTRIBUTE          MapMask;
+  PAGING_MODE                 PagingMode;
+  IA32_CR4                    Cr4;
+  BOOLEAN                     Page5LevelSupport;
+  UINT32                      RegEax;
+  BOOLEAN                     Page1GSupport;
+  CPUID_EXTENDED_CPU_SIG_EDX  RegEdx;
+
+  if (sizeof (UINTN) == sizeof (UINT64)) {
+    //
+    // Check Page5Level Support or not.
+    //
+    Cr4.UintN         = AsmReadCr4 ();
+    Page5LevelSupport = (Cr4.Bits.LA57 ? TRUE : FALSE);
+
+    //
+    // Check Page1G Support or not.
+    //
+    Page1GSupport = FALSE;
+    AsmCpuid (CPUID_EXTENDED_FUNCTION, &RegEax, NULL, NULL, NULL);
+    if (RegEax >= CPUID_EXTENDED_CPU_SIG) {
+      AsmCpuid (CPUID_EXTENDED_CPU_SIG, NULL, NULL, NULL, &RegEdx.Uint32);
+      if (RegEdx.Bits.Page1GB != 0) {
+        Page1GSupport = TRUE;
+      }
+    }
+
+    //
+    // Decide Paging Mode according Page5LevelSupport & Page1GSupport.
+    //
+    if (Page5LevelSupport) {
+      PagingMode = Page1GSupport ? Paging5Level1GB : Paging5Level;
+    } else {
+      PagingMode = Page1GSupport ? Paging4Level1GB : Paging4Level;
+    }
+  } else {
+    PagingMode = PagingPae;
+  }
+
+  MapAttribute.Uint64 = 0;
+  MapMask.Uint64      = 0;
+  MapMask.Bits.Nx     = 1;
+  PageTable           = AsmReadCr3 () & PAGING_4K_ADDRESS_MASK_64;
+  BufferSize          = 0;
+
+  //
+  // Get required buffer size for changing the pagetable.
+  //
+  Status = PageTableMap (&PageTable, PagingMode, 0, &BufferSize, BaseAddress, Length, &MapAttribute, &MapMask, NULL);
+  if (Status == EFI_BUFFER_TOO_SMALL) {
+    //
+    // Allocate required Buffer.
+    //
+    Status = PeiServicesAllocatePages (
+               EfiBootServicesData,
+               EFI_SIZE_TO_PAGES (BufferSize),
+               &Buffer
+               );
+    ASSERT_EFI_ERROR (Status);
+    Status = PageTableMap (&PageTable, PagingMode, (VOID *)(UINTN)Buffer, &BufferSize, BaseAddress, Length, &MapAttribute, &MapMask, NULL);
+  }
+
+  ASSERT_EFI_ERROR (Status);
+  AsmWriteCr3 (PageTable);
+}
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119010): https://edk2.groups.io/g/devel/message/119010
Mute This Topic: https://groups.io/mt/106150773/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [edk2-devel] [Patch V2 14/18] UefiCpuPkg:Remove code to handle APIC setting and Interrupt
  2024-05-17  9:45 [edk2-devel] [Patch V2 00/18] Remove some S3 related code in CpuS3.c of smm cpu driver duntan
                   ` (12 preceding siblings ...)
  2024-05-17  9:46 ` [edk2-devel] [Patch V2 13/18] UefiCpuPkg:Relocate AP to new safe buffer in PeiMpLib duntan
@ 2024-05-17  9:46 ` duntan
  2024-05-17  9:46 ` [edk2-devel] [Patch V2 15/18] UefiCpuPkg:Remove code to wakeup AP and relocate ap duntan
                   ` (7 subsequent siblings)
  21 siblings, 0 replies; 30+ messages in thread
From: duntan @ 2024-05-17  9:46 UTC (permalink / raw)
  To: devel; +Cc: Ray Ni, Rahul Kumar, Gerd Hoffmann, Jiaxin Wu

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>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
index 0bc4a3572d..89223e1726 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
@@ -105,11 +105,6 @@ InitializeCpuBeforeRebase (
   IN BOOLEAN  IsBsp
   )
 {
-  ProgramVirtualWireMode ();
-  if (!IsBsp) {
-    DisableLvtInterrupts ();
-  }
-
   //
   // Count down the number with lock mechanism.
   //
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119011): https://edk2.groups.io/g/devel/message/119011
Mute This Topic: https://groups.io/mt/106150774/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [edk2-devel] [Patch V2 15/18] UefiCpuPkg:Remove code to wakeup AP and relocate ap
  2024-05-17  9:45 [edk2-devel] [Patch V2 00/18] Remove some S3 related code in CpuS3.c of smm cpu driver duntan
                   ` (13 preceding siblings ...)
  2024-05-17  9:46 ` [edk2-devel] [Patch V2 14/18] UefiCpuPkg:Remove code to handle APIC setting and Interrupt duntan
@ 2024-05-17  9:46 ` duntan
  2024-05-17  9:46 ` [edk2-devel] [Patch V2 16/18] UefiCpuPkg: Remove unneeded MpService2Ppi assignment duntan
                   ` (6 subsequent siblings)
  21 siblings, 0 replies; 30+ messages in thread
From: duntan @ 2024-05-17  9:46 UTC (permalink / raw)
  To: devel; +Cc: Ray Ni, Rahul Kumar, Gerd Hoffmann, Jiaxin Wu

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>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c             | 292 +++++++++-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/MpFuncs.nasm   | 153 ---------------------------------------------------------------------------------------------------------------------------------------------------------
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c |  29 +----------------------------
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf  |   5 +----
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/MpFuncs.nasm    | 189 ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c  |  30 +-----------------------------
 6 files changed, 12 insertions(+), 686 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
index 89223e1726..4f69326fdf 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
@@ -8,30 +8,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 #include "PiSmmCpuDxeSmm.h"
 #include <PiPei.h>
-#include <Ppi/MpServices2.h>
-
-#pragma pack(1)
-typedef struct {
-  UINTN              Lock;
-  VOID               *StackStart;
-  UINTN              StackSize;
-  VOID               *ApFunction;
-  IA32_DESCRIPTOR    GdtrProfile;
-  IA32_DESCRIPTOR    IdtrProfile;
-  UINT32             BufferStart;
-  UINT32             Cr3;
-  UINTN              InitializeFloatingPointUnitsAddress;
-} MP_CPU_EXCHANGE_INFO;
-#pragma pack()
-
-typedef struct {
-  UINT8    *RendezvousFunnelAddress;
-  UINTN    PModeEntryOffset;
-  UINTN    FlatJumpOffset;
-  UINTN    Size;
-  UINTN    LModeEntryOffset;
-  UINTN    LongJumpOffset;
-} MP_ASSEMBLY_ADDRESS_MAP;
 
 //
 // Flags used when program the register.
@@ -44,31 +20,11 @@ typedef struct {
                                                     // package level semaphore.
 } PROGRAM_CPU_REGISTER_FLAGS;
 
-//
-// Signal that SMM BASE relocation is complete.
-//
-volatile BOOLEAN  mInitApsAfterSmmBaseReloc;
-
-/**
-  Get starting address and size of the rendezvous entry for APs.
-  Information for fixing a jump instruction in the code is also returned.
-
-  @param AddressMap  Output buffer for address map information.
-**/
-VOID *
-EFIAPI
-AsmGetAddressMap (
-  MP_ASSEMBLY_ADDRESS_MAP  *AddressMap
-  );
-
 #define LEGACY_REGION_SIZE  (2 * 0x1000)
 #define LEGACY_REGION_BASE  (0xA0000 - LEGACY_REGION_SIZE)
 
-PROGRAM_CPU_REGISTER_FLAGS  mCpuFlags;
-ACPI_CPU_DATA               mAcpiCpuData;
-volatile UINT32             mNumberToFinish;
-MP_CPU_EXCHANGE_INFO        *mExchangeInfo;
-BOOLEAN                     mRestoreSmmConfigurationInS3 = FALSE;
+ACPI_CPU_DATA  mAcpiCpuData;
+BOOLEAN        mRestoreSmmConfigurationInS3 = FALSE;
 
 //
 // S3 boot flag
@@ -82,191 +38,6 @@ SMM_S3_RESUME_STATE  *mSmmS3ResumeState = NULL;
 
 BOOLEAN  mAcpiS3Enable = TRUE;
 
-UINT8  *mApHltLoopCode          = NULL;
-UINT8  mApHltLoopCodeTemplate[] = {
-  0x8B, 0x44, 0x24, 0x04, // mov  eax, dword ptr [esp+4]
-  0xF0, 0xFF, 0x08,       // lock dec  dword ptr [eax]
-  0xFA,                   // cli
-  0xF4,                   // hlt
-  0xEB, 0xFC              // jmp $-2
-};
-
-/**
-  The function is invoked before SMBASE relocation in S3 path to restores CPU status.
-
-  The function is invoked before SMBASE relocation in S3 path. It does first time microcode load
-  and restores MTRRs for both BSP and APs.
-
-  @param   IsBsp   The CPU this function executes on is BSP or not.
-
-**/
-VOID
-InitializeCpuBeforeRebase (
-  IN BOOLEAN  IsBsp
-  )
-{
-  //
-  // Count down the number with lock mechanism.
-  //
-  InterlockedDecrement (&mNumberToFinish);
-
-  if (IsBsp) {
-    //
-    // Bsp wait here till all AP finish the initialization before rebase
-    //
-    while (mNumberToFinish > 0) {
-      CpuPause ();
-    }
-  }
-}
-
-/**
-  The function is invoked after SMBASE relocation in S3 path to restores CPU status.
-
-  The function is invoked after SMBASE relocation in S3 path. It restores configuration according to
-  data saved by normal boot path for both BSP and APs.
-
-  @param   IsBsp   The CPU this function executes on is BSP or not.
-
-**/
-VOID
-InitializeCpuAfterRebase (
-  IN BOOLEAN  IsBsp
-  )
-{
-  UINTN  TopOfStack;
-  UINT8  Stack[128];
-
-  if (mSmmS3ResumeState->MpService2Ppi == 0) {
-    if (IsBsp) {
-      while (mNumberToFinish > 0) {
-        CpuPause ();
-      }
-    } else {
-      //
-      // Place AP into the safe code, count down the number with lock mechanism in the safe code.
-      //
-      TopOfStack  = (UINTN)Stack + sizeof (Stack);
-      TopOfStack &= ~(UINTN)(CPU_STACK_ALIGNMENT - 1);
-      CopyMem ((VOID *)(UINTN)mApHltLoopCode, mApHltLoopCodeTemplate, sizeof (mApHltLoopCodeTemplate));
-      TransferApToSafeState ((UINTN)mApHltLoopCode, TopOfStack, (UINTN)&mNumberToFinish);
-    }
-  }
-}
-
-/**
-  Cpu initialization procedure.
-
-  @param[in,out] Buffer  The pointer to private data buffer.
-
-**/
-VOID
-EFIAPI
-InitializeCpuProcedure (
-  IN OUT VOID  *Buffer
-  )
-{
-  BOOLEAN  IsBsp;
-
-  IsBsp =  (BOOLEAN)(mBspApicId == GetApicId ());
-
-  //
-  // Skip initialization if mAcpiCpuData is not valid
-  //
-  if (mAcpiCpuData.NumberOfCpus > 0) {
-    //
-    // First time microcode load and restore MTRRs
-    //
-    InitializeCpuBeforeRebase (IsBsp);
-  }
-
-  if (IsBsp) {
-    //
-    // Issue SMI IPI (All Excluding  Self SMM IPI + BSP SMM IPI) to execute first SMI init.
-    //
-    ExecuteFirstSmiInit ();
-  }
-
-  //
-  // Skip initialization if mAcpiCpuData is not valid
-  //
-  if (mAcpiCpuData.NumberOfCpus > 0) {
-    if (IsBsp) {
-      //
-      // mNumberToFinish should be set before AP executes InitializeCpuAfterRebase()
-      //
-      mNumberToFinish = (UINT32)(mNumberOfCpus - 1);
-      //
-      // Signal that SMM base relocation is complete and to continue initialization for all APs.
-      //
-      mInitApsAfterSmmBaseReloc = TRUE;
-    } else {
-      //
-      // AP Wait for BSP to signal SMM Base relocation done.
-      //
-      while (!mInitApsAfterSmmBaseReloc) {
-        CpuPause ();
-      }
-    }
-
-    //
-    // Restore MSRs for BSP and all APs
-    //
-    InitializeCpuAfterRebase (IsBsp);
-  }
-}
-
-/**
-  Prepares startup vector for APs.
-
-  This function prepares startup vector for APs.
-
-  @param  WorkingBuffer  The address of the work buffer.
-**/
-VOID
-PrepareApStartupVector (
-  EFI_PHYSICAL_ADDRESS  WorkingBuffer
-  )
-{
-  EFI_PHYSICAL_ADDRESS     StartupVector;
-  MP_ASSEMBLY_ADDRESS_MAP  AddressMap;
-
-  //
-  // Get the address map of startup code for AP,
-  // including code size, and offset of long jump instructions to redirect.
-  //
-  ZeroMem (&AddressMap, sizeof (AddressMap));
-  AsmGetAddressMap (&AddressMap);
-
-  StartupVector = WorkingBuffer;
-
-  //
-  // Copy AP startup code to startup vector, and then redirect the long jump
-  // instructions for mode switching.
-  //
-  CopyMem ((VOID *)(UINTN)StartupVector, AddressMap.RendezvousFunnelAddress, AddressMap.Size);
-  *(UINT32 *)(UINTN)(StartupVector + AddressMap.FlatJumpOffset + 3) = (UINT32)(StartupVector + AddressMap.PModeEntryOffset);
-  if (AddressMap.LongJumpOffset != 0) {
-    *(UINT32 *)(UINTN)(StartupVector + AddressMap.LongJumpOffset + 2) = (UINT32)(StartupVector + AddressMap.LModeEntryOffset);
-  }
-
-  //
-  // Get the start address of exchange data between BSP and AP.
-  //
-  mExchangeInfo = (MP_CPU_EXCHANGE_INFO *)(UINTN)(StartupVector + AddressMap.Size);
-  ZeroMem ((VOID *)mExchangeInfo, sizeof (MP_CPU_EXCHANGE_INFO));
-
-  CopyMem ((VOID *)(UINTN)&mExchangeInfo->GdtrProfile, (VOID *)(UINTN)mAcpiCpuData.GdtrProfile, sizeof (IA32_DESCRIPTOR));
-  CopyMem ((VOID *)(UINTN)&mExchangeInfo->IdtrProfile, (VOID *)(UINTN)mAcpiCpuData.IdtrProfile, sizeof (IA32_DESCRIPTOR));
-
-  mExchangeInfo->StackStart                          = (VOID *)(UINTN)mAcpiCpuData.StackAddress;
-  mExchangeInfo->StackSize                           = mAcpiCpuData.StackSize;
-  mExchangeInfo->BufferStart                         = (UINT32)StartupVector;
-  mExchangeInfo->Cr3                                 = (UINT32)(AsmReadCr3 ());
-  mExchangeInfo->InitializeFloatingPointUnitsAddress = (UINTN)InitializeFloatingPointUnits;
-  mExchangeInfo->ApFunction                          = (VOID *)(UINTN)InitializeCpuProcedure;
-}
-
 /**
   Restore SMM Configuration in S3 boot path.
 
@@ -315,12 +86,11 @@ SmmRestoreCpu (
   VOID
   )
 {
-  SMM_S3_RESUME_STATE         *SmmS3ResumeState;
-  IA32_DESCRIPTOR             Ia32Idtr;
-  IA32_DESCRIPTOR             X64Idtr;
-  IA32_IDT_GATE_DESCRIPTOR    IdtEntryTable[EXCEPTION_VECTOR_NUMBER];
-  EFI_STATUS                  Status;
-  EDKII_PEI_MP_SERVICES2_PPI  *Mp2ServicePpi;
+  SMM_S3_RESUME_STATE       *SmmS3ResumeState;
+  IA32_DESCRIPTOR           Ia32Idtr;
+  IA32_DESCRIPTOR           X64Idtr;
+  IA32_IDT_GATE_DESCRIPTOR  IdtEntryTable[EXCEPTION_VECTOR_NUMBER];
+  EFI_STATUS                Status;
 
   DEBUG ((DEBUG_INFO, "SmmRestoreCpu()\n"));
 
@@ -369,38 +139,10 @@ SmmRestoreCpu (
     }
   }
 
-  mBspApicId = GetApicId ();
   //
-  // Skip AP initialization if mAcpiCpuData is not valid
+  // Issue SMI IPI (All Excluding  Self SMM IPI + BSP SMM IPI) to execute first SMI init.
   //
-  if (mAcpiCpuData.NumberOfCpus > 0) {
-    if (FeaturePcdGet (PcdCpuHotPlugSupport)) {
-      ASSERT (mNumberOfCpus <= mAcpiCpuData.NumberOfCpus);
-    } else {
-      ASSERT (mNumberOfCpus == mAcpiCpuData.NumberOfCpus);
-    }
-
-    mNumberToFinish = (UINT32)mNumberOfCpus;
-
-    //
-    // Execute code for before SmmBaseReloc. Note: This flag is maintained across S3 boots.
-    //
-    mInitApsAfterSmmBaseReloc = FALSE;
-
-    if (mSmmS3ResumeState->MpService2Ppi != 0) {
-      Mp2ServicePpi = (EDKII_PEI_MP_SERVICES2_PPI *)(UINTN)mSmmS3ResumeState->MpService2Ppi;
-      Mp2ServicePpi->StartupAllCPUs (Mp2ServicePpi, InitializeCpuProcedure, 0, NULL);
-    } else {
-      PrepareApStartupVector (mAcpiCpuData.StartupVector);
-      //
-      // Send INIT IPI - SIPI to all APs
-      //
-      SendInitSipiSipiAllExcludingSelf ((UINT32)mAcpiCpuData.StartupVector);
-      InitializeCpuProcedure (NULL);
-    }
-  } else {
-    InitializeCpuProcedure (NULL);
-  }
+  ExecuteFirstSmiInit ();
 
   //
   // Set a flag to restore SMM configuration in S3 path.
@@ -471,8 +213,6 @@ InitSmmS3ResumeState (
   VOID                  *GuidHob;
   EFI_SMRAM_DESCRIPTOR  *SmramDescriptor;
   SMM_S3_RESUME_STATE   *SmmS3ResumeState;
-  EFI_PHYSICAL_ADDRESS  Address;
-  EFI_STATUS            Status;
 
   if (!mAcpiS3Enable) {
     return;
@@ -524,20 +264,6 @@ InitSmmS3ResumeState (
     //
     InitSmmS3Cr3 ();
   }
-
-  //
-  // Allocate safe memory in ACPI NVS for AP to execute hlt loop in
-  // protected mode on S3 path
-  //
-  Address = BASE_4GB - 1;
-  Status  = gBS->AllocatePages (
-                   AllocateMaxAddress,
-                   EfiACPIMemoryNVS,
-                   EFI_SIZE_TO_PAGES (sizeof (mApHltLoopCodeTemplate)),
-                   &Address
-                   );
-  ASSERT_EFI_ERROR (Status);
-  mApHltLoopCode = (UINT8 *)(UINTN)Address;
 }
 
 /**
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/MpFuncs.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/MpFuncs.nasm
deleted file mode 100644
index dbd1418c0d..0000000000
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/MpFuncs.nasm
+++ /dev/null
@@ -1,153 +0,0 @@
-;------------------------------------------------------------------------------ ;
-; Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
-; SPDX-License-Identifier: BSD-2-Clause-Patent
-;
-; Module Name:
-;
-;   MpFuncs.nasm
-;
-; Abstract:
-;
-;   This is the assembly code for Multi-processor S3 support
-;
-;-------------------------------------------------------------------------------
-
-SECTION .text
-
-extern ASM_PFX(InitializeFloatingPointUnits)
-
-%define VacantFlag 0x0
-%define NotVacantFlag 0xff
-
-%define LockLocation RendezvousFunnelProcEnd - RendezvousFunnelProcStart
-%define StackStart LockLocation + 0x4
-%define StackSize LockLocation + 0x8
-%define RendezvousProc LockLocation + 0xC
-%define GdtrProfile LockLocation + 0x10
-%define IdtrProfile LockLocation + 0x16
-%define BufferStart LockLocation + 0x1C
-
-;-------------------------------------------------------------------------------------
-;RendezvousFunnelProc  procedure follows. All APs execute their procedure. This
-;procedure serializes all the AP processors through an Init sequence. It must be
-;noted that APs arrive here very raw...ie: real mode, no stack.
-;ALSO THIS PROCEDURE IS EXECUTED BY APs ONLY ON 16 BIT MODE. HENCE THIS PROC
-;IS IN MACHINE CODE.
-;-------------------------------------------------------------------------------------
-;RendezvousFunnelProc (&WakeUpBuffer,MemAddress);
-
-BITS 16
-global ASM_PFX(RendezvousFunnelProc)
-ASM_PFX(RendezvousFunnelProc):
-RendezvousFunnelProcStart:
-
-; At this point CS = 0x(vv00) and ip= 0x0.
-
-        mov        ax,  cs
-        mov        ds,  ax
-        mov        es,  ax
-        mov        ss,  ax
-        xor        ax,  ax
-        mov        fs,  ax
-        mov        gs,  ax
-
-flat32Start:
-
-        mov        si, BufferStart
-        mov        edx,dword [si]          ; EDX is keeping the start address of wakeup buffer
-
-        mov        si, GdtrProfile
-o32     lgdt       [cs:si]
-
-        mov        si, IdtrProfile
-o32     lidt       [cs:si]
-
-        xor        ax,  ax
-        mov        ds,  ax
-
-        mov        eax, cr0                    ; Get control register 0
-        or         eax, 0x000000001            ; Set PE bit (bit #0)
-        mov        cr0, eax
-
-FLAT32_JUMP:
-
-a32     jmp   dword 0x20:0x0
-
-BITS 32
-PMODE_ENTRY:                         ; protected mode entry point
-
-        mov         ax,  0x8
-o16     mov         ds,  ax
-o16     mov         es,  ax
-o16     mov         fs,  ax
-o16     mov         gs,  ax
-o16     mov         ss,  ax           ; Flat mode setup.
-
-        mov         esi, edx
-
-        mov         edi, esi
-        add         edi, LockLocation
-        mov         al,  NotVacantFlag
-TestLock:
-        xchg        byte [edi], al
-        cmp         al, NotVacantFlag
-        jz          TestLock
-
-ProgramStack:
-
-        mov         edi, esi
-        add         edi, StackSize
-        mov         eax, dword [edi]
-        mov         edi, esi
-        add         edi, StackStart
-        add         eax, dword [edi]
-        mov         esp, eax
-        mov         dword [edi], eax
-
-Releaselock:
-
-        mov         al,  VacantFlag
-        mov         edi, esi
-        add         edi, LockLocation
-        xchg        byte [edi], al
-
-        ;
-        ; Call assembly function to initialize FPU.
-        ;
-        mov         ebx, ASM_PFX(InitializeFloatingPointUnits)
-        call        ebx
-        ;
-        ; Call C Function
-        ;
-        mov         edi, esi
-        add         edi, RendezvousProc
-        mov         eax, dword [edi]
-
-        test        eax, eax
-        jz          GoToSleep
-        call        eax                           ; Call C function
-
-GoToSleep:
-        cli
-        hlt
-        jmp         $-2
-
-RendezvousFunnelProcEnd:
-;-------------------------------------------------------------------------------------
-;  AsmGetAddressMap (&AddressMap);
-;-------------------------------------------------------------------------------------
-global ASM_PFX(AsmGetAddressMap)
-ASM_PFX(AsmGetAddressMap):
-
-        pushad
-        mov         ebp,esp
-
-        mov         ebx, dword [ebp+0x24]
-        mov         dword [ebx], RendezvousFunnelProcStart
-        mov         dword [ebx+0x4], PMODE_ENTRY - RendezvousFunnelProcStart
-        mov         dword [ebx+0x8], FLAT32_JUMP - RendezvousFunnelProcStart
-        mov         dword [ebx+0xc], RendezvousFunnelProcEnd - RendezvousFunnelProcStart
-
-        popad
-        ret
-
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c
index 636dc8d92f..0c1cc51ada 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c
@@ -1,7 +1,7 @@
 /** @file
   SMM CPU misc functions for Ia32 arch specific.
 
-Copyright (c) 2015 - 2023, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2015 - 2024, Intel Corporation. All rights reserved.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -141,33 +141,6 @@ InitGdt (
   return GdtTssTables;
 }
 
-/**
-  Transfer AP to safe hlt-loop after it finished restore CPU features on S3 patch.
-
-  @param[in] ApHltLoopCode          The address of the safe hlt-loop function.
-  @param[in] TopOfStack             A pointer to the new stack to use for the ApHltLoopCode.
-  @param[in] NumberToFinishAddress  Address of Semaphore of APs finish count.
-
-**/
-VOID
-TransferApToSafeState (
-  IN UINTN  ApHltLoopCode,
-  IN UINTN  TopOfStack,
-  IN UINTN  NumberToFinishAddress
-  )
-{
-  SwitchStack (
-    (SWITCH_STACK_ENTRY_POINT)ApHltLoopCode,
-    (VOID *)NumberToFinishAddress,
-    NULL,
-    (VOID *)TopOfStack
-    );
-  //
-  // It should never reach here
-  //
-  ASSERT (FALSE);
-}
-
 /**
   Initialize the shadow stack related data structure.
 
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
index 3354f94a64..f0598b0364 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
@@ -4,7 +4,7 @@
 # This SMM driver performs SMM initialization, deploy SMM Entry Vector,
 # provides CPU specific services in SMM.
 #
-# Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2009 - 2024, Intel Corporation. All rights reserved.<BR>
 # Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
 # Copyright (C) 2023 - 2024 Advanced Micro Devices, Inc. All rights reserved.<BR>
 #
@@ -53,7 +53,6 @@
   Ia32/SmmProfileArch.h
   Ia32/SmiEntry.nasm
   Ia32/SmiException.nasm
-  Ia32/MpFuncs.nasm
   Ia32/Cet.nasm
 
 [Sources.X64]
@@ -63,7 +62,6 @@
   X64/SmmProfileArch.h
   X64/SmiEntry.nasm
   X64/SmiException.nasm
-  X64/MpFuncs.nasm
   X64/Cet.nasm
 
 [Packages]
@@ -136,7 +134,6 @@
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmProfileSize                   ## SOMETIMES_CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackSize                     ## CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmApSyncTimeout                 ## CONSUMES
-  gUefiCpuPkgTokenSpaceGuid.PcdCpuS3DataAddress                    ## SOMETIMES_CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuHotPlugDataAddress               ## SOMETIMES_PRODUCES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmCodeAccessCheckEnable         ## CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode                      ## CONSUMES
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/MpFuncs.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/MpFuncs.nasm
deleted file mode 100644
index a12538f72b..0000000000
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/MpFuncs.nasm
+++ /dev/null
@@ -1,189 +0,0 @@
-;------------------------------------------------------------------------------ ;
-; Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
-; SPDX-License-Identifier: BSD-2-Clause-Patent
-;
-; Module Name:
-;
-;   MpFuncs.nasm
-;
-; Abstract:
-;
-;   This is the assembly code for Multi-processor S3 support
-;
-;-------------------------------------------------------------------------------
-
-%define VacantFlag 0x0
-%define NotVacantFlag 0xff
-
-%define LockLocation RendezvousFunnelProcEnd - RendezvousFunnelProcStart
-%define StackStartAddressLocation LockLocation + 0x8
-%define StackSizeLocation LockLocation + 0x10
-%define CProcedureLocation LockLocation + 0x18
-%define GdtrLocation LockLocation + 0x20
-%define IdtrLocation LockLocation + 0x2A
-%define BufferStartLocation LockLocation + 0x34
-%define Cr3OffsetLocation LockLocation + 0x38
-%define InitializeFloatingPointUnitsAddress LockLocation + 0x3C
-
-;-------------------------------------------------------------------------------------
-;RendezvousFunnelProc  procedure follows. All APs execute their procedure. This
-;procedure serializes all the AP processors through an Init sequence. It must be
-;noted that APs arrive here very raw...ie: real mode, no stack.
-;ALSO THIS PROCEDURE IS EXECUTED BY APs ONLY ON 16 BIT MODE. HENCE THIS PROC
-;IS IN MACHINE CODE.
-;-------------------------------------------------------------------------------------
-;RendezvousFunnelProc (&WakeUpBuffer,MemAddress);
-
-;text      SEGMENT
-DEFAULT REL
-SECTION .text
-
-BITS 16
-global ASM_PFX(RendezvousFunnelProc)
-ASM_PFX(RendezvousFunnelProc):
-RendezvousFunnelProcStart:
-
-; At this point CS = 0x(vv00) and ip= 0x0.
-
-        mov        ax,  cs
-        mov        ds,  ax
-        mov        es,  ax
-        mov        ss,  ax
-        xor        ax,  ax
-        mov        fs,  ax
-        mov        gs,  ax
-
-flat32Start:
-
-        mov        si, BufferStartLocation
-        mov        edx,dword [si]          ; EDX is keeping the start address of wakeup buffer
-
-        mov        si, Cr3OffsetLocation
-        mov        ecx,dword [si]          ; ECX is keeping the value of CR3
-
-        mov        si, GdtrLocation
-o32     lgdt       [cs:si]
-
-        mov        si, IdtrLocation
-o32     lidt       [cs:si]
-
-        xor        ax,  ax
-        mov        ds,  ax
-
-        mov        eax, cr0                    ; Get control register 0
-        or         eax, 0x000000001            ; Set PE bit (bit #0)
-        mov        cr0, eax
-
-FLAT32_JUMP:
-
-a32     jmp   dword 0x20:0x0
-
-BITS 32
-PMODE_ENTRY:                         ; protected mode entry point
-
-        mov        ax,  0x18
-o16     mov        ds,  ax
-o16     mov        es,  ax
-o16     mov        fs,  ax
-o16     mov        gs,  ax
-o16     mov        ss,  ax                     ; Flat mode setup.
-
-        mov        eax, cr4
-        bts        eax, 5
-        mov        cr4, eax
-
-        mov        cr3, ecx
-
-        mov        esi, edx                    ; Save wakeup buffer address
-
-        mov        ecx, 0xc0000080             ; EFER MSR number.
-        rdmsr                                  ; Read EFER.
-        bts        eax, 8                      ; Set LME=1.
-        wrmsr                                  ; Write EFER.
-
-        mov        eax, cr0                    ; Read CR0.
-        bts        eax, 31                     ; Set PG=1.
-        mov        cr0, eax                    ; Write CR0.
-
-LONG_JUMP:
-
-a16     jmp   dword 0x38:0x0
-
-BITS 64
-LongModeStart:
-
-        mov         ax,  0x30
-o16     mov         ds,  ax
-o16     mov         es,  ax
-o16     mov         ss,  ax
-
-        mov  edi, esi
-        add  edi, LockLocation
-        mov  al,  NotVacantFlag
-TestLock:
-        xchg byte [edi], al
-        cmp  al, NotVacantFlag
-        jz   TestLock
-
-ProgramStack:
-
-        mov  edi, esi
-        add  edi, StackSizeLocation
-        mov  rax, qword [edi]
-        mov  edi, esi
-        add  edi, StackStartAddressLocation
-        add  rax, qword [edi]
-        mov  rsp, rax
-        mov  qword [edi], rax
-
-Releaselock:
-
-        mov  al,  VacantFlag
-        mov  edi, esi
-        add  edi, LockLocation
-        xchg byte [edi], al
-
-        ;
-        ; Call assembly function to initialize FPU.
-        ;
-        mov         rax, qword [esi + InitializeFloatingPointUnitsAddress]
-        sub         rsp, 0x20
-        call        rax
-        add         rsp, 0x20
-
-        ;
-        ; Call C Function
-        ;
-        mov         edi, esi
-        add         edi, CProcedureLocation
-        mov         rax, qword [edi]
-
-        test        rax, rax
-        jz          GoToSleep
-
-        sub         rsp, 0x20
-        call        rax
-        add         rsp, 0x20
-
-GoToSleep:
-        cli
-        hlt
-        jmp         $-2
-
-RendezvousFunnelProcEnd:
-
-;-------------------------------------------------------------------------------------
-;  AsmGetAddressMap (&AddressMap);
-;-------------------------------------------------------------------------------------
-; comments here for definition of address map
-global ASM_PFX(AsmGetAddressMap)
-ASM_PFX(AsmGetAddressMap):
-        lea         rax, [RendezvousFunnelProcStart]
-        mov         qword [rcx], rax
-        mov         qword [rcx+0x8], PMODE_ENTRY - RendezvousFunnelProcStart
-        mov         qword [rcx+0x10], FLAT32_JUMP - RendezvousFunnelProcStart
-        mov         qword [rcx+0x18], RendezvousFunnelProcEnd - RendezvousFunnelProcStart
-        mov         qword [rcx+0x20], LongModeStart - RendezvousFunnelProcStart
-        mov         qword [rcx+0x28], LONG_JUMP - RendezvousFunnelProcStart
-        ret
-
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
index c4f21e2155..ca706ee32c 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
@@ -1,7 +1,7 @@
 /** @file
   SMM CPU misc functions for x64 arch specific.
 
-Copyright (c) 2015 - 2023, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2015 - 2024, Intel Corporation. All rights reserved.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -132,34 +132,6 @@ GetProtectedModeCS (
   return Index * 8;
 }
 
-/**
-  Transfer AP to safe hlt-loop after it finished restore CPU features on S3 patch.
-
-  @param[in] ApHltLoopCode          The address of the safe hlt-loop function.
-  @param[in] TopOfStack             A pointer to the new stack to use for the ApHltLoopCode.
-  @param[in] NumberToFinishAddress  Address of Semaphore of APs finish count.
-
-**/
-VOID
-TransferApToSafeState (
-  IN UINTN  ApHltLoopCode,
-  IN UINTN  TopOfStack,
-  IN UINTN  NumberToFinishAddress
-  )
-{
-  AsmDisablePaging64 (
-    GetProtectedModeCS (),
-    (UINT32)ApHltLoopCode,
-    (UINT32)NumberToFinishAddress,
-    0,
-    (UINT32)TopOfStack
-    );
-  //
-  // It should never reach here
-  //
-  ASSERT (FALSE);
-}
-
 /**
   Initialize the shadow stack related data structure.
 
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119012): https://edk2.groups.io/g/devel/message/119012
Mute This Topic: https://groups.io/mt/106150775/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [edk2-devel] [Patch V2 16/18] UefiCpuPkg: Remove unneeded MpService2Ppi assignment
  2024-05-17  9:45 [edk2-devel] [Patch V2 00/18] Remove some S3 related code in CpuS3.c of smm cpu driver duntan
                   ` (14 preceding siblings ...)
  2024-05-17  9:46 ` [edk2-devel] [Patch V2 15/18] UefiCpuPkg:Remove code to wakeup AP and relocate ap duntan
@ 2024-05-17  9:46 ` duntan
  2024-05-17  9:46 ` [edk2-devel] [Patch V2 17/18] MdeModulePkg:Remove MpService2Ppi field in SMM_S3_RESUME_STATE duntan
                   ` (5 subsequent siblings)
  21 siblings, 0 replies; 30+ messages in thread
From: duntan @ 2024-05-17  9:46 UTC (permalink / raw)
  To: devel; +Cc: Ray Ni, Rahul Kumar, Gerd Hoffmann, Jiaxin Wu

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>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
---
 UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
index e6dfa09c71..3e64a115bf 100644
--- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
+++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
@@ -1152,7 +1152,6 @@ S3RestoreConfig2 (
     SmmS3ResumeState->ReturnContext1     = (EFI_PHYSICAL_ADDRESS)(UINTN)AcpiS3Context;
     SmmS3ResumeState->ReturnContext2     = (EFI_PHYSICAL_ADDRESS)(UINTN)EfiBootScriptExecutorVariable;
     SmmS3ResumeState->ReturnStackPointer = (EFI_PHYSICAL_ADDRESS)STACK_ALIGN_DOWN (&Status);
-    SmmS3ResumeState->MpService2Ppi      = 0;
 
     DEBUG ((DEBUG_INFO, "SMM S3 Signature                = %x\n", SmmS3ResumeState->Signature));
     DEBUG ((DEBUG_INFO, "SMM S3 Stack Base               = %x\n", SmmS3ResumeState->SmmS3StackBase));
@@ -1181,19 +1180,6 @@ S3RestoreConfig2 (
     if (((SmmS3ResumeState->Signature == SMM_S3_RESUME_SMM_32) && (sizeof (UINTN) == sizeof (UINT32))) ||
         ((SmmS3ResumeState->Signature == SMM_S3_RESUME_SMM_64) && (sizeof (UINTN) == sizeof (UINT64))))
     {
-      //
-      // Get MP Services2 Ppi to pass it to Smm S3.
-      //
-      Status = PeiServicesLocatePpi (
-                 &gEdkiiPeiMpServices2PpiGuid,
-                 0,
-                 NULL,
-                 (VOID **)&MpService2Ppi
-                 );
-      ASSERT_EFI_ERROR (Status);
-      SmmS3ResumeState->MpService2Ppi = (EFI_PHYSICAL_ADDRESS)(UINTN)MpService2Ppi;
-      DEBUG ((DEBUG_INFO, "SMM S3 MpService2Ppi Point = %lx\n", SmmS3ResumeState->MpService2Ppi));
-
       SwitchStack (
         (SWITCH_STACK_ENTRY_POINT)(UINTN)SmmS3ResumeState->SmmS3ResumeEntryPoint,
         (VOID *)AcpiS3Context,
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119014): https://edk2.groups.io/g/devel/message/119014
Mute This Topic: https://groups.io/mt/106150779/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [edk2-devel] [Patch V2 17/18] MdeModulePkg:Remove MpService2Ppi field in SMM_S3_RESUME_STATE
  2024-05-17  9:45 [edk2-devel] [Patch V2 00/18] Remove some S3 related code in CpuS3.c of smm cpu driver duntan
                   ` (15 preceding siblings ...)
  2024-05-17  9:46 ` [edk2-devel] [Patch V2 16/18] UefiCpuPkg: Remove unneeded MpService2Ppi assignment duntan
@ 2024-05-17  9:46 ` duntan
  2024-05-17  9:46 ` [edk2-devel] [Patch V2 18/18] UefiCpuPkg: Remove GetAcpiCpuData() in CpuS3.c duntan
                   ` (4 subsequent siblings)
  21 siblings, 0 replies; 30+ messages in thread
From: duntan @ 2024-05-17  9:46 UTC (permalink / raw)
  To: devel; +Cc: Ray Ni, Jian J Wang, Liming Gao

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>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
---
 MdeModulePkg/Include/Guid/AcpiS3Context.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/MdeModulePkg/Include/Guid/AcpiS3Context.h b/MdeModulePkg/Include/Guid/AcpiS3Context.h
index 72d173c4fd..6c7237727e 100644
--- a/MdeModulePkg/Include/Guid/AcpiS3Context.h
+++ b/MdeModulePkg/Include/Guid/AcpiS3Context.h
@@ -1,7 +1,7 @@
 /** @file
   Definitions for data structures used in S3 resume.
 
-Copyright (c) 2011 - 2023, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2011 - 2024, Intel Corporation. All rights reserved.<BR>
 
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -30,7 +30,6 @@ typedef struct {
   EFI_PHYSICAL_ADDRESS    ReturnContext1;
   EFI_PHYSICAL_ADDRESS    ReturnContext2;
   EFI_PHYSICAL_ADDRESS    ReturnStackPointer;
-  EFI_PHYSICAL_ADDRESS    MpService2Ppi;
   EFI_PHYSICAL_ADDRESS    Smst;
 } SMM_S3_RESUME_STATE;
 
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119013): https://edk2.groups.io/g/devel/message/119013
Mute This Topic: https://groups.io/mt/106150778/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [edk2-devel] [Patch V2 18/18] UefiCpuPkg: Remove GetAcpiCpuData() in CpuS3.c
  2024-05-17  9:45 [edk2-devel] [Patch V2 00/18] Remove some S3 related code in CpuS3.c of smm cpu driver duntan
                   ` (16 preceding siblings ...)
  2024-05-17  9:46 ` [edk2-devel] [Patch V2 17/18] MdeModulePkg:Remove MpService2Ppi field in SMM_S3_RESUME_STATE duntan
@ 2024-05-17  9:46 ` duntan
  2024-05-20  7:05 ` [edk2-devel] [Patch V2 00/18] Remove some S3 related code in CpuS3.c of smm cpu driver Ni, Ray
                   ` (3 subsequent siblings)
  21 siblings, 0 replies; 30+ messages in thread
From: duntan @ 2024-05-17  9:46 UTC (permalink / raw)
  To: devel; +Cc: Ray Ni, Rahul Kumar, Gerd Hoffmann, Jiaxin Wu

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>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c          | 243 +--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c |   8 +++-----
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h |  26 +-------------------------
 3 files changed, 5 insertions(+), 272 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
index 4f69326fdf..c37a2d4d1b 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
@@ -9,22 +9,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include "PiSmmCpuDxeSmm.h"
 #include <PiPei.h>
 
-//
-// Flags used when program the register.
-//
-typedef struct {
-  volatile UINTN     MemoryMappedLock;              // Spinlock used to program mmio
-  volatile UINT32    *CoreSemaphoreCount;           // Semaphore container used to program
-                                                    // core level semaphore.
-  volatile UINT32    *PackageSemaphoreCount;        // Semaphore container used to program
-                                                    // package level semaphore.
-} PROGRAM_CPU_REGISTER_FLAGS;
-
-#define LEGACY_REGION_SIZE  (2 * 0x1000)
-#define LEGACY_REGION_BASE  (0xA0000 - LEGACY_REGION_SIZE)
-
-ACPI_CPU_DATA  mAcpiCpuData;
-BOOLEAN        mRestoreSmmConfigurationInS3 = FALSE;
+BOOLEAN  mRestoreSmmConfigurationInS3 = FALSE;
 
 //
 // S3 boot flag
@@ -266,232 +251,6 @@ InitSmmS3ResumeState (
   }
 }
 
-/**
-  Copy register table from non-SMRAM into SMRAM.
-
-  @param[in] DestinationRegisterTableList  Points to destination register table.
-  @param[in] SourceRegisterTableList       Points to source register table.
-  @param[in] NumberOfCpus                  Number of CPUs.
-
-**/
-VOID
-CopyRegisterTable (
-  IN CPU_REGISTER_TABLE  *DestinationRegisterTableList,
-  IN CPU_REGISTER_TABLE  *SourceRegisterTableList,
-  IN UINT32              NumberOfCpus
-  )
-{
-  UINTN                     Index;
-  CPU_REGISTER_TABLE_ENTRY  *RegisterTableEntry;
-
-  CopyMem (DestinationRegisterTableList, SourceRegisterTableList, NumberOfCpus * sizeof (CPU_REGISTER_TABLE));
-  for (Index = 0; Index < NumberOfCpus; Index++) {
-    if (DestinationRegisterTableList[Index].TableLength != 0) {
-      DestinationRegisterTableList[Index].AllocatedSize = DestinationRegisterTableList[Index].TableLength * sizeof (CPU_REGISTER_TABLE_ENTRY);
-      RegisterTableEntry                                = AllocateCopyPool (
-                                                            DestinationRegisterTableList[Index].AllocatedSize,
-                                                            (VOID *)(UINTN)SourceRegisterTableList[Index].RegisterTableEntry
-                                                            );
-      ASSERT (RegisterTableEntry != NULL);
-      DestinationRegisterTableList[Index].RegisterTableEntry = (EFI_PHYSICAL_ADDRESS)(UINTN)RegisterTableEntry;
-    }
-  }
-}
-
-/**
-  Check whether the register table is empty or not.
-
-  @param[in] RegisterTable  Point to the register table.
-  @param[in] NumberOfCpus   Number of CPUs.
-
-  @retval TRUE              The register table is empty.
-  @retval FALSE             The register table is not empty.
-**/
-BOOLEAN
-IsRegisterTableEmpty (
-  IN CPU_REGISTER_TABLE  *RegisterTable,
-  IN UINT32              NumberOfCpus
-  )
-{
-  UINTN  Index;
-
-  if (RegisterTable != NULL) {
-    for (Index = 0; Index < NumberOfCpus; Index++) {
-      if (RegisterTable[Index].TableLength != 0) {
-        return FALSE;
-      }
-    }
-  }
-
-  return TRUE;
-}
-
-/**
-  Copy the data used to initialize processor register into SMRAM.
-
-  @param[in,out]  CpuFeatureInitDataDst   Pointer to the destination CPU_FEATURE_INIT_DATA structure.
-  @param[in]      CpuFeatureInitDataSrc   Pointer to the source CPU_FEATURE_INIT_DATA structure.
-
-**/
-VOID
-CopyCpuFeatureInitDatatoSmram (
-  IN OUT CPU_FEATURE_INIT_DATA  *CpuFeatureInitDataDst,
-  IN     CPU_FEATURE_INIT_DATA  *CpuFeatureInitDataSrc
-  )
-{
-  CPU_STATUS_INFORMATION  *CpuStatus;
-
-  if (!IsRegisterTableEmpty ((CPU_REGISTER_TABLE *)(UINTN)CpuFeatureInitDataSrc->PreSmmInitRegisterTable, mAcpiCpuData.NumberOfCpus)) {
-    CpuFeatureInitDataDst->PreSmmInitRegisterTable = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocatePool (mAcpiCpuData.NumberOfCpus * sizeof (CPU_REGISTER_TABLE));
-    ASSERT (CpuFeatureInitDataDst->PreSmmInitRegisterTable != 0);
-
-    CopyRegisterTable (
-      (CPU_REGISTER_TABLE *)(UINTN)CpuFeatureInitDataDst->PreSmmInitRegisterTable,
-      (CPU_REGISTER_TABLE *)(UINTN)CpuFeatureInitDataSrc->PreSmmInitRegisterTable,
-      mAcpiCpuData.NumberOfCpus
-      );
-  }
-
-  if (!IsRegisterTableEmpty ((CPU_REGISTER_TABLE *)(UINTN)CpuFeatureInitDataSrc->RegisterTable, mAcpiCpuData.NumberOfCpus)) {
-    CpuFeatureInitDataDst->RegisterTable = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocatePool (mAcpiCpuData.NumberOfCpus * sizeof (CPU_REGISTER_TABLE));
-    ASSERT (CpuFeatureInitDataDst->RegisterTable != 0);
-
-    CopyRegisterTable (
-      (CPU_REGISTER_TABLE *)(UINTN)CpuFeatureInitDataDst->RegisterTable,
-      (CPU_REGISTER_TABLE *)(UINTN)CpuFeatureInitDataSrc->RegisterTable,
-      mAcpiCpuData.NumberOfCpus
-      );
-  }
-
-  CpuStatus = &CpuFeatureInitDataDst->CpuStatus;
-  CopyMem (CpuStatus, &CpuFeatureInitDataSrc->CpuStatus, sizeof (CPU_STATUS_INFORMATION));
-
-  if (CpuFeatureInitDataSrc->CpuStatus.ThreadCountPerPackage != 0) {
-    CpuStatus->ThreadCountPerPackage = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocateCopyPool (
-                                                                      sizeof (UINT32) * CpuStatus->PackageCount,
-                                                                      (UINT32 *)(UINTN)CpuFeatureInitDataSrc->CpuStatus.ThreadCountPerPackage
-                                                                      );
-    ASSERT (CpuStatus->ThreadCountPerPackage != 0);
-  }
-
-  if (CpuFeatureInitDataSrc->CpuStatus.ThreadCountPerCore != 0) {
-    CpuStatus->ThreadCountPerCore = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocateCopyPool (
-                                                                   sizeof (UINT8) * (CpuStatus->PackageCount * CpuStatus->MaxCoreCount),
-                                                                   (UINT32 *)(UINTN)CpuFeatureInitDataSrc->CpuStatus.ThreadCountPerCore
-                                                                   );
-    ASSERT (CpuStatus->ThreadCountPerCore != 0);
-  }
-
-  if (CpuFeatureInitDataSrc->ApLocation != 0) {
-    CpuFeatureInitDataDst->ApLocation = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocateCopyPool (
-                                                                       mAcpiCpuData.NumberOfCpus * sizeof (EFI_CPU_PHYSICAL_LOCATION),
-                                                                       (EFI_CPU_PHYSICAL_LOCATION *)(UINTN)CpuFeatureInitDataSrc->ApLocation
-                                                                       );
-    ASSERT (CpuFeatureInitDataDst->ApLocation != 0);
-  }
-}
-
-/**
-  Get ACPI CPU data.
-
-**/
-VOID
-GetAcpiCpuData (
-  VOID
-  )
-{
-  ACPI_CPU_DATA           *AcpiCpuData;
-  IA32_DESCRIPTOR         *Gdtr;
-  IA32_DESCRIPTOR         *Idtr;
-  VOID                    *GdtForAp;
-  VOID                    *IdtForAp;
-  VOID                    *MachineCheckHandlerForAp;
-  CPU_STATUS_INFORMATION  *CpuStatus;
-
-  if (!mAcpiS3Enable) {
-    return;
-  }
-
-  //
-  // Prevent use of mAcpiCpuData by initialize NumberOfCpus to 0
-  //
-  mAcpiCpuData.NumberOfCpus = 0;
-
-  //
-  // If PcdCpuS3DataAddress was never set, then do not copy CPU S3 Data into SMRAM
-  //
-  AcpiCpuData = (ACPI_CPU_DATA *)(UINTN)PcdGet64 (PcdCpuS3DataAddress);
-  if (AcpiCpuData == 0) {
-    return;
-  }
-
-  //
-  // For a native platform, copy the CPU S3 data into SMRAM for use on CPU S3 Resume.
-  //
-  CopyMem (&mAcpiCpuData, AcpiCpuData, sizeof (mAcpiCpuData));
-
-  mAcpiCpuData.MtrrTable = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocatePool (sizeof (MTRR_SETTINGS));
-  ASSERT (mAcpiCpuData.MtrrTable != 0);
-
-  CopyMem ((VOID *)(UINTN)mAcpiCpuData.MtrrTable, (VOID *)(UINTN)AcpiCpuData->MtrrTable, sizeof (MTRR_SETTINGS));
-
-  mAcpiCpuData.GdtrProfile = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocatePool (sizeof (IA32_DESCRIPTOR));
-  ASSERT (mAcpiCpuData.GdtrProfile != 0);
-
-  CopyMem ((VOID *)(UINTN)mAcpiCpuData.GdtrProfile, (VOID *)(UINTN)AcpiCpuData->GdtrProfile, sizeof (IA32_DESCRIPTOR));
-
-  mAcpiCpuData.IdtrProfile = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocatePool (sizeof (IA32_DESCRIPTOR));
-  ASSERT (mAcpiCpuData.IdtrProfile != 0);
-
-  CopyMem ((VOID *)(UINTN)mAcpiCpuData.IdtrProfile, (VOID *)(UINTN)AcpiCpuData->IdtrProfile, sizeof (IA32_DESCRIPTOR));
-
-  //
-  // Copy AP's GDT, IDT and Machine Check handler into SMRAM.
-  //
-  Gdtr = (IA32_DESCRIPTOR *)(UINTN)mAcpiCpuData.GdtrProfile;
-  Idtr = (IA32_DESCRIPTOR *)(UINTN)mAcpiCpuData.IdtrProfile;
-
-  GdtForAp = AllocatePool ((Gdtr->Limit + 1) + (Idtr->Limit + 1) + mAcpiCpuData.ApMachineCheckHandlerSize);
-  ASSERT (GdtForAp != NULL);
-  IdtForAp                 = (VOID *)((UINTN)GdtForAp + (Gdtr->Limit + 1));
-  MachineCheckHandlerForAp = (VOID *)((UINTN)IdtForAp + (Idtr->Limit + 1));
-
-  CopyMem (GdtForAp, (VOID *)Gdtr->Base, Gdtr->Limit + 1);
-  CopyMem (IdtForAp, (VOID *)Idtr->Base, Idtr->Limit + 1);
-  CopyMem (MachineCheckHandlerForAp, (VOID *)(UINTN)mAcpiCpuData.ApMachineCheckHandlerBase, mAcpiCpuData.ApMachineCheckHandlerSize);
-
-  Gdtr->Base                             = (UINTN)GdtForAp;
-  Idtr->Base                             = (UINTN)IdtForAp;
-  mAcpiCpuData.ApMachineCheckHandlerBase = (EFI_PHYSICAL_ADDRESS)(UINTN)MachineCheckHandlerForAp;
-
-  ZeroMem (&mAcpiCpuData.CpuFeatureInitData, sizeof (CPU_FEATURE_INIT_DATA));
-
-  if (!PcdGetBool (PcdCpuFeaturesInitOnS3Resume)) {
-    //
-    // If the CPU features will not be initialized by CpuFeaturesPei module during
-    // next ACPI S3 resume, copy the CPU features initialization data into SMRAM,
-    // which will be consumed in SmmRestoreCpu during next S3 resume.
-    //
-    CopyCpuFeatureInitDatatoSmram (&mAcpiCpuData.CpuFeatureInitData, &AcpiCpuData->CpuFeatureInitData);
-
-    CpuStatus = &mAcpiCpuData.CpuFeatureInitData.CpuStatus;
-
-    mCpuFlags.CoreSemaphoreCount = AllocateZeroPool (
-                                     sizeof (UINT32) * CpuStatus->PackageCount *
-                                     CpuStatus->MaxCoreCount * CpuStatus->MaxThreadCount
-                                     );
-    ASSERT (mCpuFlags.CoreSemaphoreCount != NULL);
-
-    mCpuFlags.PackageSemaphoreCount = AllocateZeroPool (
-                                        sizeof (UINT32) * CpuStatus->PackageCount *
-                                        CpuStatus->MaxCoreCount * CpuStatus->MaxThreadCount
-                                        );
-    ASSERT (mCpuFlags.PackageSemaphoreCount != NULL);
-
-    InitializeSpinLock ((SPIN_LOCK *)&mCpuFlags.MemoryMappedLock);
-  }
-}
-
 /**
   Get ACPI S3 enable flag.
 
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
index e400bee8d5..20a1a9cdbc 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
@@ -1,7 +1,7 @@
 /** @file
 Agent Module to load other modules to deploy SMM Entry Vector for X86 CPU.
 
-Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2009 - 2024, Intel Corporation. All rights reserved.<BR>
 Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
 Copyright (C) 2023 - 2024 Advanced Micro Devices, Inc. All rights reserved.<BR>
 
@@ -435,8 +435,8 @@ ExecuteFirstSmiInit (
 /**
   SMM Ready To Lock event notification handler.
 
-  The CPU S3 data is copied to SMRAM for security and mSmmReadyToLock is set to
-  perform additional lock actions that must be performed from SMM on the next SMI.
+  mSmmReadyToLock is set to perform additional lock actions that must be
+  performed from SMM on the next SMI.
 
   @param[in] Protocol   Points to the protocol's unique identifier.
   @param[in] Interface  Points to the interface instance.
@@ -452,8 +452,6 @@ SmmReadyToLockEventNotify (
   IN EFI_HANDLE      Handle
   )
 {
-  GetAcpiCpuData ();
-
   //
   // Cache a copy of UEFI memory map before we start profiling feature.
   //
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index f42910ddf1..315a33d578 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -1,7 +1,7 @@
 /** @file
 Agent Module to load other modules to deploy SMM Entry Vector for X86 CPU.
 
-Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2009 - 2024, Intel Corporation. All rights reserved.<BR>
 Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
 Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved.<BR>
 
@@ -1048,15 +1048,6 @@ InitSmmS3ResumeState (
   IN UINT32  Cr3
   );
 
-/**
-  Get ACPI CPU data.
-
-**/
-VOID
-GetAcpiCpuData (
-  VOID
-  );
-
 /**
   Restore SMM Configuration in S3 boot path.
 
@@ -1075,21 +1066,6 @@ GetAcpiS3EnableFlag (
   VOID
   );
 
-/**
-  Transfer AP to safe hlt-loop after it finished restore CPU features on S3 patch.
-
-  @param[in] ApHltLoopCode          The address of the safe hlt-loop function.
-  @param[in] TopOfStack             A pointer to the new stack to use for the ApHltLoopCode.
-  @param[in] NumberToFinishAddress  Address of Semaphore of APs finish count.
-
-**/
-VOID
-TransferApToSafeState (
-  IN UINTN  ApHltLoopCode,
-  IN UINTN  TopOfStack,
-  IN UINTN  NumberToFinishAddress
-  );
-
 /**
   Set ShadowStack memory.
 
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119015): https://edk2.groups.io/g/devel/message/119015
Mute This Topic: https://groups.io/mt/106150782/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [edk2-devel] [Patch V2 00/18] Remove some S3 related code in CpuS3.c of smm cpu driver
  2024-05-17  9:45 [edk2-devel] [Patch V2 00/18] Remove some S3 related code in CpuS3.c of smm cpu driver duntan
                   ` (17 preceding siblings ...)
  2024-05-17  9:46 ` [edk2-devel] [Patch V2 18/18] UefiCpuPkg: Remove GetAcpiCpuData() in CpuS3.c duntan
@ 2024-05-20  7:05 ` Ni, Ray
  2024-05-24  3:18   ` duntan
  2024-05-24  3:07 ` Wu, Jiaxin
                   ` (2 subsequent siblings)
  21 siblings, 1 reply; 30+ messages in thread
From: Ni, Ray @ 2024-05-20  7:05 UTC (permalink / raw)
  To: Tan, Dun, devel@edk2.groups.io
  Cc: Liming Gao, Wu, Jiaxin, Ard Biesheuvel, Yao, Jiewen,
	Gerd Hoffmann, Kumar, Rahul R

[-- Attachment #1: Type: text/plain, Size: 7224 bytes --]

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 <dun.tan@intel.com>
Sent: Friday, May 17, 2024 17:45
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Liming Gao <gaoliming@byosoft.com.cn>; Wu, Jiaxin <jiaxin.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Yao, Jiewen <jiewen.yao@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Kumar, Rahul R <rahul.r.kumar@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 <gaoliming@byosoft.com.cn>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@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 ++++++++++++++++++++++++++++++++++++++++++++++++-----------------------------------------------------------------------------------------------------------------------------
 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/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c            |  30 +----------------------------
 UefiCpuPkg/UefiCpuPkg.dec                               |   4 ++--
 UefiCpuPkg/UefiCpuPkg.dsc                               |   3 ++-
 UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c       |  78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------
 UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf |   4 +++-
 24 files changed, 531 insertions(+), 1582 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 (#119073): https://edk2.groups.io/g/devel/message/119073
Mute This Topic: https://groups.io/mt/106150757/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 12332 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [edk2-devel] [Patch V2 00/18] Remove some S3 related code in CpuS3.c of smm cpu driver
  2024-05-17  9:45 [edk2-devel] [Patch V2 00/18] Remove some S3 related code in CpuS3.c of smm cpu driver duntan
                   ` (18 preceding siblings ...)
  2024-05-20  7:05 ` [edk2-devel] [Patch V2 00/18] Remove some S3 related code in CpuS3.c of smm cpu driver Ni, Ray
@ 2024-05-24  3:07 ` Wu, Jiaxin
       [not found] ` <17D03DA5161A67E3.17823@groups.io>
       [not found] ` <17D03D9CBFB2693B.16299@groups.io>
  21 siblings, 0 replies; 30+ messages in thread
From: Wu, Jiaxin @ 2024-05-24  3:07 UTC (permalink / raw)
  To: Tan, Dun, devel@edk2.groups.io
  Cc: Liming Gao, Ni, Ray, Ard Biesheuvel, Yao, Jiewen, Gerd Hoffmann,
	Kumar, Rahul R

Series Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>

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

Thanks,
Jiaxin

> -----Original Message-----
> From: Tan, Dun <dun.tan@intel.com>
> Sent: Friday, May 17, 2024 5:46 PM
> To: devel@edk2.groups.io
> Cc: Liming Gao <gaoliming@byosoft.com.cn>; Wu, Jiaxin
> <jiaxin.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Ard Biesheuvel
> <ardb+tianocore@kernel.org>; Yao, Jiewen <jiewen.yao@intel.com>; Gerd
> Hoffmann <kraxel@redhat.com>; Kumar, Rahul R <rahul.r.kumar@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 <gaoliming@byosoft.com.cn>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Cc: Gerd Hoffmann <kraxel@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
> ++++++++++++++++++++++++++++++++++++++++++++++++-----------------
> ----------------------------------------------------------------------------------------------
> --------------
>  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/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c            |  30 +-----------
> -----------------
>  UefiCpuPkg/UefiCpuPkg.dec                               |   4 ++--
>  UefiCpuPkg/UefiCpuPkg.dsc                               |   3 ++-
>  UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c       |  78
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +---------------
>  UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf |   4 +++-
>  24 files changed, 531 insertions(+), 1582 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 (#119182): https://edk2.groups.io/g/devel/message/119182
Mute This Topic: https://groups.io/mt/106150757/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [edk2-devel] [Patch V2 00/18] Remove some S3 related code in CpuS3.c of smm cpu driver
  2024-05-20  7:05 ` [edk2-devel] [Patch V2 00/18] Remove some S3 related code in CpuS3.c of smm cpu driver Ni, Ray
@ 2024-05-24  3:18   ` duntan
  0 siblings, 0 replies; 30+ messages in thread
From: duntan @ 2024-05-24  3:18 UTC (permalink / raw)
  To: Liming Gao, devel@edk2.groups.io
  Cc: Wu, Jiaxin, Ard Biesheuvel, Yao, Jiewen, Gerd Hoffmann,
	Kumar, Rahul R, Ni, Ray

[-- Attachment #1: Type: text/plain, Size: 8488 bytes --]

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 <ray.ni@intel.com>
Sent: Monday, May 20, 2024 3:05 PM
To: Tan, Dun <dun.tan@intel.com>; devel@edk2.groups.io
Cc: Liming Gao <gaoliming@byosoft.com.cn>; Wu, Jiaxin <jiaxin.wu@intel.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Yao, Jiewen <jiewen.yao@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>
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 <dun.tan@intel.com<mailto:dun.tan@intel.com>>
Sent: Friday, May 17, 2024 17:45
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>
Cc: Liming Gao <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Wu, Jiaxin <jiaxin.wu@intel.com<mailto:jiaxin.wu@intel.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Ard Biesheuvel <ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Gerd Hoffmann <kraxel@redhat.com<mailto:kraxel@redhat.com>>; Kumar, Rahul R <rahul.r.kumar@intel.com<mailto:rahul.r.kumar@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 <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>
Cc: Jiaxin Wu <jiaxin.wu@intel.com<mailto:jiaxin.wu@intel.com>>
Cc: Ray Ni <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>>
Cc: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
Cc: Gerd Hoffmann <kraxel@redhat.com<mailto:kraxel@redhat.com>>
Cc: Rahul Kumar <rahul1.kumar@intel.com<mailto:rahul1.kumar@intel.com>>
Cc: Gerd Hoffmann <kraxel@redhat.com<mailto:kraxel@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 ++++++++++++++++++++++++++++++++++++++++++++++++-----------------------------------------------------------------------------------------------------------------------------
 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/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c            |  30 +----------------------------
 UefiCpuPkg/UefiCpuPkg.dec                               |   4 ++--
 UefiCpuPkg/UefiCpuPkg.dsc                               |   3 ++-
 UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c       |  78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------
 UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf |   4 +++-
 24 files changed, 531 insertions(+), 1582 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 (#119184): https://edk2.groups.io/g/devel/message/119184
Mute This Topic: https://groups.io/mt/106150757/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 16442 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [edk2-devel] [Patch V2 17/18] MdeModulePkg:Remove MpService2Ppi field in SMM_S3_RESUME_STATE
       [not found] ` <17D03DA5161A67E3.17823@groups.io>
@ 2024-05-28  6:35   ` duntan
  2024-05-28  9:16     ` 回复: " gaoliming via groups.io
  0 siblings, 1 reply; 30+ messages in thread
From: duntan @ 2024-05-28  6:35 UTC (permalink / raw)
  To: devel@edk2.groups.io, Tan, Dun, Liming Gao; +Cc: Ni, Ray, Wang, Jian J

Hi Liming,

Could you please help to review this patch?
This patch set is to simplify the X86 CPU SMM driver by removing most of S3 related logics and we want to merge it as soon as possible.

Thanks,
Dun

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of duntan
Sent: Friday, May 17, 2024 5:46 PM
To: devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>
Subject: [edk2-devel] [Patch V2 17/18] MdeModulePkg:Remove MpService2Ppi field in SMM_S3_RESUME_STATE

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>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
---
 MdeModulePkg/Include/Guid/AcpiS3Context.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/MdeModulePkg/Include/Guid/AcpiS3Context.h b/MdeModulePkg/Include/Guid/AcpiS3Context.h
index 72d173c4fd..6c7237727e 100644
--- a/MdeModulePkg/Include/Guid/AcpiS3Context.h
+++ b/MdeModulePkg/Include/Guid/AcpiS3Context.h
@@ -1,7 +1,7 @@
 /** @file
   Definitions for data structures used in S3 resume.
 
-Copyright (c) 2011 - 2023, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2011 - 2024, Intel Corporation. All rights reserved.<BR>
 
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -30,7 +30,6 @@ typedef struct {
   EFI_PHYSICAL_ADDRESS    ReturnContext1;
   EFI_PHYSICAL_ADDRESS    ReturnContext2;
   EFI_PHYSICAL_ADDRESS    ReturnStackPointer;
-  EFI_PHYSICAL_ADDRESS    MpService2Ppi;
   EFI_PHYSICAL_ADDRESS    Smst;
 } SMM_S3_RESUME_STATE;
 
--
2.31.1.windows.1








-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119292): https://edk2.groups.io/g/devel/message/119292
Mute This Topic: https://groups.io/mt/106344380/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [edk2-devel] [Patch V2 01/18] MdeModulePkg: Add gEdkiiS3MtrrSettingGuid
       [not found] ` <17D03D9CBFB2693B.16299@groups.io>
@ 2024-05-28  6:37   ` duntan
  2024-05-28  9:14     ` 回复: " gaoliming via groups.io
  0 siblings, 1 reply; 30+ messages in thread
From: duntan @ 2024-05-28  6:37 UTC (permalink / raw)
  To: devel@edk2.groups.io, Tan, Dun, Liming Gao; +Cc: Ni, Ray, Wu, Jiaxin

Hi Liming,

Could you please help to review this patch?
This patch set is to simplify the X86 CPU SMM driver by removing most of S3 related logics and we want to merge it as soon as possible.

Thanks,
Dun

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of duntan
Sent: Friday, May 17, 2024 5:46 PM
To: devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Wu, Jiaxin <jiaxin.wu@intel.com>
Subject: [edk2-devel] [Patch V2 01/18] MdeModulePkg: Add gEdkiiS3MtrrSettingGuid

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>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
---
 MdeModulePkg/MdeModulePkg.dec | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec index f7339f0aec..02f330a453 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -4,7 +4,7 @@
 # and libraries instances, which are used for those modules.
 #
 # Copyright (c) 2019, NVIDIA CORPORATION. All rights reserved.
-# Copyright (c) 2007 - 2021, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2007 - 2024, Intel Corporation. All rights 
+reserved.<BR>
 # Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>  # (C) Copyright 2016 - 2019 Hewlett Packard Enterprise Development LP<BR>  # Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> @@ -465,6 +465,9 @@
   gEdk2JedecSfdpSpiDxeDriverGuid  = { 0xBE71701E, 0xB63C, 0x4574, { 0x9C, 0x5C, 0x36, 0x29, 0xE8, 0xEA, 0xC4, 0x14 }}
   gEdk2JedecSfdpSpiSmmDriverGuid  = { 0x95A1E915, 0x195C, 0x477C, { 0x92, 0x6F, 0x7E, 0x24, 0x67, 0xC1, 0xB3, 0x1F }}
 
+  ## This GUID will be used to save MTRR_SETTINGS at EndOfDxe by LockBox and restore at S3 boot PEI phase for s3 usage.
+  gEdkiiS3MtrrSettingGuid = { 0xd77baa84, 0xb332, 0x4463, { 0x9f, 0x1d, 
+ 0xce, 0x81, 0x00, 0xfe, 0x7f, 0x35 }}
+
 [Ppis]
   ## Include/Ppi/FirmwareVolumeShadowPpi.h
   gEdkiiPeiFirmwareVolumeShadowPpiGuid = { 0x7dfe756c, 0xed8d, 0x4d77, {0x9e, 0xc4, 0x39, 0x9a, 0x8a, 0x81, 0x51, 0x16 } }
--
2.31.1.windows.1








-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119293): https://edk2.groups.io/g/devel/message/119293
Mute This Topic: https://groups.io/mt/106344405/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 30+ messages in thread

* 回复: [edk2-devel] [Patch V2 01/18] MdeModulePkg: Add gEdkiiS3MtrrSettingGuid
  2024-05-28  6:37   ` [edk2-devel] [Patch V2 01/18] MdeModulePkg: Add gEdkiiS3MtrrSettingGuid duntan
@ 2024-05-28  9:14     ` gaoliming via groups.io
  0 siblings, 0 replies; 30+ messages in thread
From: gaoliming via groups.io @ 2024-05-28  9:14 UTC (permalink / raw)
  To: devel, dun.tan; +Cc: 'Ni, Ray', 'Wu, Jiaxin'

Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 duntan
> 发送时间: 2024年5月28日 14:37
> 收件人: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com>; Liming Gao
> <gaoliming@byosoft.com.cn>
> 抄送: Ni, Ray <ray.ni@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> 主题: Re: [edk2-devel] [Patch V2 01/18] MdeModulePkg: Add
> gEdkiiS3MtrrSettingGuid
> 
> Hi Liming,
> 
> Could you please help to review this patch?
> This patch set is to simplify the X86 CPU SMM driver by removing most of
S3
> related logics and we want to merge it as soon as possible.
> 
> Thanks,
> Dun
> 
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of duntan
> Sent: Friday, May 17, 2024 5:46 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Wu,
> Jiaxin <jiaxin.wu@intel.com>
> Subject: [edk2-devel] [Patch V2 01/18] MdeModulePkg: Add
> gEdkiiS3MtrrSettingGuid
> 
> 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>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> ---
>  MdeModulePkg/MdeModulePkg.dec | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/MdeModulePkg.dec
> b/MdeModulePkg/MdeModulePkg.dec index f7339f0aec..02f330a453 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -4,7 +4,7 @@
>  # and libraries instances, which are used for those modules.
>  #
>  # Copyright (c) 2019, NVIDIA CORPORATION. All rights reserved.
> -# Copyright (c) 2007 - 2021, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2007 - 2024, Intel Corporation. All rights
> +reserved.<BR>
>  # Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>  # (C)
Copyright 2016
> - 2019 Hewlett Packard Enterprise Development LP<BR>  # Copyright (c)
2017,
> AMD Incorporated. All rights reserved.<BR> @@ -465,6 +465,9 @@
>    gEdk2JedecSfdpSpiDxeDriverGuid  = { 0xBE71701E, 0xB63C, 0x4574, { 0x9C,
> 0x5C, 0x36, 0x29, 0xE8, 0xEA, 0xC4, 0x14 }}
>    gEdk2JedecSfdpSpiSmmDriverGuid  = { 0x95A1E915, 0x195C, 0x477C,
> { 0x92, 0x6F, 0x7E, 0x24, 0x67, 0xC1, 0xB3, 0x1F }}
> 
> +  ## This GUID will be used to save MTRR_SETTINGS at EndOfDxe by LockBox
> and restore at S3 boot PEI phase for s3 usage.
> +  gEdkiiS3MtrrSettingGuid = { 0xd77baa84, 0xb332, 0x4463, { 0x9f, 0x1d,
> + 0xce, 0x81, 0x00, 0xfe, 0x7f, 0x35 }}
> +
>  [Ppis]
>    ## Include/Ppi/FirmwareVolumeShadowPpi.h
>    gEdkiiPeiFirmwareVolumeShadowPpiGuid = { 0x7dfe756c, 0xed8d, 0x4d77,
> {0x9e, 0xc4, 0x39, 0x9a, 0x8a, 0x81, 0x51, 0x16 } }
> --
> 2.31.1.windows.1
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119294): https://edk2.groups.io/g/devel/message/119294
Mute This Topic: https://groups.io/mt/106345436/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 30+ messages in thread

* 回复: [edk2-devel] [Patch V2 17/18] MdeModulePkg:Remove MpService2Ppi field in SMM_S3_RESUME_STATE
  2024-05-28  6:35   ` [edk2-devel] [Patch V2 17/18] MdeModulePkg:Remove MpService2Ppi field in SMM_S3_RESUME_STATE duntan
@ 2024-05-28  9:16     ` gaoliming via groups.io
  2024-05-28  9:34       ` duntan
  0 siblings, 1 reply; 30+ messages in thread
From: gaoliming via groups.io @ 2024-05-28  9:16 UTC (permalink / raw)
  To: devel, dun.tan; +Cc: 'Ni, Ray', 'Wang, Jian J'

Dun:
  To be compatible, I suggest to keep the same layout of
SMM_S3_RESUME_STATE. MpService2Ppi field can be kept, but it is not used any
more. 

Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 duntan
> 发送时间: 2024年5月28日 14:35
> 收件人: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com>; Liming Gao
> <gaoliming@byosoft.com.cn>
> 抄送: Ni, Ray <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>
> 主题: Re: [edk2-devel] [Patch V2 17/18] MdeModulePkg:Remove MpService2Ppi
> field in SMM_S3_RESUME_STATE
> 
> Hi Liming,
> 
> Could you please help to review this patch?
> This patch set is to simplify the X86 CPU SMM driver by removing most of
S3
> related logics and we want to merge it as soon as possible.
> 
> Thanks,
> Dun
> 
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of duntan
> Sent: Friday, May 17, 2024 5:46 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
Liming
> Gao <gaoliming@byosoft.com.cn>
> Subject: [edk2-devel] [Patch V2 17/18] MdeModulePkg:Remove MpService2Ppi
> field in SMM_S3_RESUME_STATE
> 
> 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>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> ---
>  MdeModulePkg/Include/Guid/AcpiS3Context.h | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/MdeModulePkg/Include/Guid/AcpiS3Context.h
> b/MdeModulePkg/Include/Guid/AcpiS3Context.h
> index 72d173c4fd..6c7237727e 100644
> --- a/MdeModulePkg/Include/Guid/AcpiS3Context.h
> +++ b/MdeModulePkg/Include/Guid/AcpiS3Context.h
> @@ -1,7 +1,7 @@
>  /** @file
>    Definitions for data structures used in S3 resume.
> 
> -Copyright (c) 2011 - 2023, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2011 - 2024, Intel Corporation. All rights reserved.<BR>
> 
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> @@ -30,7 +30,6 @@ typedef struct {
>    EFI_PHYSICAL_ADDRESS    ReturnContext1;
>    EFI_PHYSICAL_ADDRESS    ReturnContext2;
>    EFI_PHYSICAL_ADDRESS    ReturnStackPointer;
> -  EFI_PHYSICAL_ADDRESS    MpService2Ppi;
>    EFI_PHYSICAL_ADDRESS    Smst;
>  } SMM_S3_RESUME_STATE;
> 
> --
> 2.31.1.windows.1
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119295): https://edk2.groups.io/g/devel/message/119295
Mute This Topic: https://groups.io/mt/106345448/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [edk2-devel] [Patch V2 17/18] MdeModulePkg:Remove MpService2Ppi field in SMM_S3_RESUME_STATE
  2024-05-28  9:16     ` 回复: " gaoliming via groups.io
@ 2024-05-28  9:34       ` duntan
  2024-05-28  9:39         ` 回复: " gaoliming via groups.io
  0 siblings, 1 reply; 30+ messages in thread
From: duntan @ 2024-05-28  9:34 UTC (permalink / raw)
  To: gaoliming, devel@edk2.groups.io; +Cc: Ni, Ray, Wang, Jian J

Hi Liming,

Sorry I didn't get what the code needs to be compatible with. Could you please explain more?
The MpService2Ppi field was added in " MdeModulePkg: add MpService2Ppi field in SMM_S3_RESUME_STATE"( b1955cddb2) last year by me. Previously It was added to simplify the code logic to wakeup AP in S3 boot related code of CPU SMM driver when code execution modes for PEI and DXE are the same. In this patch set, all the related code logic was removed.

Thanks,
Dun

-----Original Message-----
From: gaoliming <gaoliming@byosoft.com.cn> 
Sent: Tuesday, May 28, 2024 5:17 PM
To: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com>
Cc: Ni, Ray <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>
Subject: 回复: [edk2-devel] [Patch V2 17/18] MdeModulePkg:Remove MpService2Ppi field in SMM_S3_RESUME_STATE

Dun:
  To be compatible, I suggest to keep the same layout of SMM_S3_RESUME_STATE. MpService2Ppi field can be kept, but it is not used any more. 

Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 duntan
> 发送时间: 2024年5月28日 14:35
> 收件人: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com>; Liming Gao 
> <gaoliming@byosoft.com.cn>
> 抄送: Ni, Ray <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>
> 主题: Re: [edk2-devel] [Patch V2 17/18] MdeModulePkg:Remove 
> MpService2Ppi field in SMM_S3_RESUME_STATE
> 
> Hi Liming,
> 
> Could you please help to review this patch?
> This patch set is to simplify the X86 CPU SMM driver by removing most 
> of
S3
> related logics and we want to merge it as soon as possible.
> 
> Thanks,
> Dun
> 
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of duntan
> Sent: Friday, May 17, 2024 5:46 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
Liming
> Gao <gaoliming@byosoft.com.cn>
> Subject: [edk2-devel] [Patch V2 17/18] MdeModulePkg:Remove 
> MpService2Ppi field in SMM_S3_RESUME_STATE
> 
> 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>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> ---
>  MdeModulePkg/Include/Guid/AcpiS3Context.h | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/MdeModulePkg/Include/Guid/AcpiS3Context.h
> b/MdeModulePkg/Include/Guid/AcpiS3Context.h
> index 72d173c4fd..6c7237727e 100644
> --- a/MdeModulePkg/Include/Guid/AcpiS3Context.h
> +++ b/MdeModulePkg/Include/Guid/AcpiS3Context.h
> @@ -1,7 +1,7 @@
>  /** @file
>    Definitions for data structures used in S3 resume.
> 
> -Copyright (c) 2011 - 2023, Intel Corporation. All rights 
> reserved.<BR>
> +Copyright (c) 2011 - 2024, Intel Corporation. All rights 
> +reserved.<BR>
> 
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> @@ -30,7 +30,6 @@ typedef struct {
>    EFI_PHYSICAL_ADDRESS    ReturnContext1;
>    EFI_PHYSICAL_ADDRESS    ReturnContext2;
>    EFI_PHYSICAL_ADDRESS    ReturnStackPointer;
> -  EFI_PHYSICAL_ADDRESS    MpService2Ppi;
>    EFI_PHYSICAL_ADDRESS    Smst;
>  } SMM_S3_RESUME_STATE;
> 
> --
> 2.31.1.windows.1
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119296): https://edk2.groups.io/g/devel/message/119296
Mute This Topic: https://groups.io/mt/106345552/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 30+ messages in thread

* 回复: [edk2-devel] [Patch V2 17/18] MdeModulePkg:Remove MpService2Ppi field in SMM_S3_RESUME_STATE
  2024-05-28  9:34       ` duntan
@ 2024-05-28  9:39         ` gaoliming via groups.io
  2024-05-28  9:44           ` duntan
  0 siblings, 1 reply; 30+ messages in thread
From: gaoliming via groups.io @ 2024-05-28  9:39 UTC (permalink / raw)
  To: 'Tan, Dun', devel; +Cc: 'Ni, Ray', 'Wang, Jian J'

Dun:
  If you confirm that there is no impact on the existing module, you can remove it. 

Thanks
Liming
> -----邮件原件-----
> 发件人: Tan, Dun <dun.tan@intel.com>
> 发送时间: 2024年5月28日 17:34
> 收件人: gaoliming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io
> 抄送: Ni, Ray <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>
> 主题: RE: [edk2-devel] [Patch V2 17/18] MdeModulePkg:Remove MpService2Ppi
> field in SMM_S3_RESUME_STATE
> 
> Hi Liming,
> 
> Sorry I didn't get what the code needs to be compatible with. Could you please
> explain more?
> The MpService2Ppi field was added in " MdeModulePkg: add MpService2Ppi
> field in SMM_S3_RESUME_STATE"( b1955cddb2) last year by me. Previously It
> was added to simplify the code logic to wakeup AP in S3 boot related code of CPU
> SMM driver when code execution modes for PEI and DXE are the same. In this
> patch set, all the related code logic was removed.
> 
> Thanks,
> Dun
> 
> -----Original Message-----
> From: gaoliming <gaoliming@byosoft.com.cn>
> Sent: Tuesday, May 28, 2024 5:17 PM
> To: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com>
> Cc: Ni, Ray <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>
> Subject: 回复: [edk2-devel] [Patch V2 17/18] MdeModulePkg:Remove
> MpService2Ppi field in SMM_S3_RESUME_STATE
> 
> Dun:
>   To be compatible, I suggest to keep the same layout of
> SMM_S3_RESUME_STATE. MpService2Ppi field can be kept, but it is not used any
> more.
> 
> Thanks
> Liming
> > -----邮件原件-----
> > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 duntan
> > 发送时间: 2024年5月28日 14:35
> > 收件人: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com>; Liming Gao
> > <gaoliming@byosoft.com.cn>
> > 抄送: Ni, Ray <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>
> > 主题: Re: [edk2-devel] [Patch V2 17/18] MdeModulePkg:Remove
> > MpService2Ppi field in SMM_S3_RESUME_STATE
> >
> > Hi Liming,
> >
> > Could you please help to review this patch?
> > This patch set is to simplify the X86 CPU SMM driver by removing most
> > of
> S3
> > related logics and we want to merge it as soon as possible.
> >
> > Thanks,
> > Dun
> >
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of duntan
> > Sent: Friday, May 17, 2024 5:46 PM
> > To: devel@edk2.groups.io
> > Cc: Ni, Ray <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> Liming
> > Gao <gaoliming@byosoft.com.cn>
> > Subject: [edk2-devel] [Patch V2 17/18] MdeModulePkg:Remove
> > MpService2Ppi field in SMM_S3_RESUME_STATE
> >
> > 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>
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > ---
> >  MdeModulePkg/Include/Guid/AcpiS3Context.h | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/MdeModulePkg/Include/Guid/AcpiS3Context.h
> > b/MdeModulePkg/Include/Guid/AcpiS3Context.h
> > index 72d173c4fd..6c7237727e 100644
> > --- a/MdeModulePkg/Include/Guid/AcpiS3Context.h
> > +++ b/MdeModulePkg/Include/Guid/AcpiS3Context.h
> > @@ -1,7 +1,7 @@
> >  /** @file
> >    Definitions for data structures used in S3 resume.
> >
> > -Copyright (c) 2011 - 2023, Intel Corporation. All rights
> > reserved.<BR>
> > +Copyright (c) 2011 - 2024, Intel Corporation. All rights
> > +reserved.<BR>
> >
> >  SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > @@ -30,7 +30,6 @@ typedef struct {
> >    EFI_PHYSICAL_ADDRESS    ReturnContext1;
> >    EFI_PHYSICAL_ADDRESS    ReturnContext2;
> >    EFI_PHYSICAL_ADDRESS    ReturnStackPointer;
> > -  EFI_PHYSICAL_ADDRESS    MpService2Ppi;
> >    EFI_PHYSICAL_ADDRESS    Smst;
> >  } SMM_S3_RESUME_STATE;
> >
> > --
> > 2.31.1.windows.1
> >
> >
> >
> >
> >
> >
> >
> >
> > 
> >
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119297): https://edk2.groups.io/g/devel/message/119297
Mute This Topic: https://groups.io/mt/106345574/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [edk2-devel] [Patch V2 17/18] MdeModulePkg:Remove MpService2Ppi field in SMM_S3_RESUME_STATE
  2024-05-28  9:39         ` 回复: " gaoliming via groups.io
@ 2024-05-28  9:44           ` duntan
  2024-05-30  5:00             ` 回复: " gaoliming via groups.io
  0 siblings, 1 reply; 30+ messages in thread
From: duntan @ 2024-05-28  9:44 UTC (permalink / raw)
  To: gaoliming, devel@edk2.groups.io; +Cc: Ni, Ray, Wang, Jian J

Liming,

Sure, I think we can remove the field. Could you please add your reviewed-by for this patch?

Thanks,
Dun

-----Original Message-----
From: gaoliming <gaoliming@byosoft.com.cn> 
Sent: Tuesday, May 28, 2024 5:39 PM
To: Tan, Dun <dun.tan@intel.com>; devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>
Subject: 回复: [edk2-devel] [Patch V2 17/18] MdeModulePkg:Remove MpService2Ppi field in SMM_S3_RESUME_STATE

Dun:
  If you confirm that there is no impact on the existing module, you can remove it. 

Thanks
Liming
> -----邮件原件-----
> 发件人: Tan, Dun <dun.tan@intel.com>
> 发送时间: 2024年5月28日 17:34
> 收件人: gaoliming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io
> 抄送: Ni, Ray <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>
> 主题: RE: [edk2-devel] [Patch V2 17/18] MdeModulePkg:Remove 
> MpService2Ppi field in SMM_S3_RESUME_STATE
> 
> Hi Liming,
> 
> Sorry I didn't get what the code needs to be compatible with. Could 
> you please explain more?
> The MpService2Ppi field was added in " MdeModulePkg: add MpService2Ppi 
> field in SMM_S3_RESUME_STATE"( b1955cddb2) last year by me. Previously 
> It was added to simplify the code logic to wakeup AP in S3 boot 
> related code of CPU SMM driver when code execution modes for PEI and 
> DXE are the same. In this patch set, all the related code logic was removed.
> 
> Thanks,
> Dun
> 
> -----Original Message-----
> From: gaoliming <gaoliming@byosoft.com.cn>
> Sent: Tuesday, May 28, 2024 5:17 PM
> To: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com>
> Cc: Ni, Ray <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>
> Subject: 回复: [edk2-devel] [Patch V2 17/18] MdeModulePkg:Remove 
> MpService2Ppi field in SMM_S3_RESUME_STATE
> 
> Dun:
>   To be compatible, I suggest to keep the same layout of 
> SMM_S3_RESUME_STATE. MpService2Ppi field can be kept, but it is not 
> used any more.
> 
> Thanks
> Liming
> > -----邮件原件-----
> > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 duntan
> > 发送时间: 2024年5月28日 14:35
> > 收件人: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com>; Liming Gao 
> > <gaoliming@byosoft.com.cn>
> > 抄送: Ni, Ray <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>
> > 主题: Re: [edk2-devel] [Patch V2 17/18] MdeModulePkg:Remove 
> > MpService2Ppi field in SMM_S3_RESUME_STATE
> >
> > Hi Liming,
> >
> > Could you please help to review this patch?
> > This patch set is to simplify the X86 CPU SMM driver by removing 
> > most of
> S3
> > related logics and we want to merge it as soon as possible.
> >
> > Thanks,
> > Dun
> >
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of 
> > duntan
> > Sent: Friday, May 17, 2024 5:46 PM
> > To: devel@edk2.groups.io
> > Cc: Ni, Ray <ray.ni@intel.com>; Wang, Jian J 
> > <jian.j.wang@intel.com>;
> Liming
> > Gao <gaoliming@byosoft.com.cn>
> > Subject: [edk2-devel] [Patch V2 17/18] MdeModulePkg:Remove 
> > MpService2Ppi field in SMM_S3_RESUME_STATE
> >
> > 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>
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > ---
> >  MdeModulePkg/Include/Guid/AcpiS3Context.h | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/MdeModulePkg/Include/Guid/AcpiS3Context.h
> > b/MdeModulePkg/Include/Guid/AcpiS3Context.h
> > index 72d173c4fd..6c7237727e 100644
> > --- a/MdeModulePkg/Include/Guid/AcpiS3Context.h
> > +++ b/MdeModulePkg/Include/Guid/AcpiS3Context.h
> > @@ -1,7 +1,7 @@
> >  /** @file
> >    Definitions for data structures used in S3 resume.
> >
> > -Copyright (c) 2011 - 2023, Intel Corporation. All rights 
> > reserved.<BR>
> > +Copyright (c) 2011 - 2024, Intel Corporation. All rights 
> > +reserved.<BR>
> >
> >  SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > @@ -30,7 +30,6 @@ typedef struct {
> >    EFI_PHYSICAL_ADDRESS    ReturnContext1;
> >    EFI_PHYSICAL_ADDRESS    ReturnContext2;
> >    EFI_PHYSICAL_ADDRESS    ReturnStackPointer;
> > -  EFI_PHYSICAL_ADDRESS    MpService2Ppi;
> >    EFI_PHYSICAL_ADDRESS    Smst;
> >  } SMM_S3_RESUME_STATE;
> >
> > --
> > 2.31.1.windows.1
> >
> >
> >
> >
> >
> >
> >
> >
> > 
> >
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119299): https://edk2.groups.io/g/devel/message/119299
Mute This Topic: https://groups.io/mt/106345604/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 30+ messages in thread

* 回复: [edk2-devel] [Patch V2 17/18] MdeModulePkg:Remove MpService2Ppi field in SMM_S3_RESUME_STATE
  2024-05-28  9:44           ` duntan
@ 2024-05-30  5:00             ` gaoliming via groups.io
  0 siblings, 0 replies; 30+ messages in thread
From: gaoliming via groups.io @ 2024-05-30  5:00 UTC (permalink / raw)
  To: 'Tan, Dun', devel; +Cc: 'Ni, Ray', 'Wang, Jian J'

Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

> -----邮件原件-----
> 发件人: Tan, Dun <dun.tan@intel.com>
> 发送时间: 2024年5月28日 17:44
> 收件人: gaoliming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io
> 抄送: Ni, Ray <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>
> 主题: RE: [edk2-devel] [Patch V2 17/18] MdeModulePkg:Remove MpService2Ppi
> field in SMM_S3_RESUME_STATE
> 
> Liming,
> 
> Sure, I think we can remove the field. Could you please add your reviewed-by for
> this patch?
> 
> Thanks,
> Dun
> 
> -----Original Message-----
> From: gaoliming <gaoliming@byosoft.com.cn>
> Sent: Tuesday, May 28, 2024 5:39 PM
> To: Tan, Dun <dun.tan@intel.com>; devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>
> Subject: 回复: [edk2-devel] [Patch V2 17/18] MdeModulePkg:Remove
> MpService2Ppi field in SMM_S3_RESUME_STATE
> 
> Dun:
>   If you confirm that there is no impact on the existing module, you can remove
> it.
> 
> Thanks
> Liming
> > -----邮件原件-----
> > 发件人: Tan, Dun <dun.tan@intel.com>
> > 发送时间: 2024年5月28日 17:34
> > 收件人: gaoliming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io
> > 抄送: Ni, Ray <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>
> > 主题: RE: [edk2-devel] [Patch V2 17/18] MdeModulePkg:Remove
> > MpService2Ppi field in SMM_S3_RESUME_STATE
> >
> > Hi Liming,
> >
> > Sorry I didn't get what the code needs to be compatible with. Could
> > you please explain more?
> > The MpService2Ppi field was added in " MdeModulePkg: add MpService2Ppi
> > field in SMM_S3_RESUME_STATE"( b1955cddb2) last year by me. Previously
> > It was added to simplify the code logic to wakeup AP in S3 boot
> > related code of CPU SMM driver when code execution modes for PEI and
> > DXE are the same. In this patch set, all the related code logic was removed.
> >
> > Thanks,
> > Dun
> >
> > -----Original Message-----
> > From: gaoliming <gaoliming@byosoft.com.cn>
> > Sent: Tuesday, May 28, 2024 5:17 PM
> > To: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com>
> > Cc: Ni, Ray <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>
> > Subject: 回复: [edk2-devel] [Patch V2 17/18] MdeModulePkg:Remove
> > MpService2Ppi field in SMM_S3_RESUME_STATE
> >
> > Dun:
> >   To be compatible, I suggest to keep the same layout of
> > SMM_S3_RESUME_STATE. MpService2Ppi field can be kept, but it is not
> > used any more.
> >
> > Thanks
> > Liming
> > > -----邮件原件-----
> > > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 duntan
> > > 发送时间: 2024年5月28日 14:35
> > > 收件人: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com>; Liming Gao
> > > <gaoliming@byosoft.com.cn>
> > > 抄送: Ni, Ray <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>
> > > 主题: Re: [edk2-devel] [Patch V2 17/18] MdeModulePkg:Remove
> > > MpService2Ppi field in SMM_S3_RESUME_STATE
> > >
> > > Hi Liming,
> > >
> > > Could you please help to review this patch?
> > > This patch set is to simplify the X86 CPU SMM driver by removing
> > > most of
> > S3
> > > related logics and we want to merge it as soon as possible.
> > >
> > > Thanks,
> > > Dun
> > >
> > > -----Original Message-----
> > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > > duntan
> > > Sent: Friday, May 17, 2024 5:46 PM
> > > To: devel@edk2.groups.io
> > > Cc: Ni, Ray <ray.ni@intel.com>; Wang, Jian J
> > > <jian.j.wang@intel.com>;
> > Liming
> > > Gao <gaoliming@byosoft.com.cn>
> > > Subject: [edk2-devel] [Patch V2 17/18] MdeModulePkg:Remove
> > > MpService2Ppi field in SMM_S3_RESUME_STATE
> > >
> > > 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>
> > > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > > ---
> > >  MdeModulePkg/Include/Guid/AcpiS3Context.h | 3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > >
> > > diff --git a/MdeModulePkg/Include/Guid/AcpiS3Context.h
> > > b/MdeModulePkg/Include/Guid/AcpiS3Context.h
> > > index 72d173c4fd..6c7237727e 100644
> > > --- a/MdeModulePkg/Include/Guid/AcpiS3Context.h
> > > +++ b/MdeModulePkg/Include/Guid/AcpiS3Context.h
> > > @@ -1,7 +1,7 @@
> > >  /** @file
> > >    Definitions for data structures used in S3 resume.
> > >
> > > -Copyright (c) 2011 - 2023, Intel Corporation. All rights
> > > reserved.<BR>
> > > +Copyright (c) 2011 - 2024, Intel Corporation. All rights
> > > +reserved.<BR>
> > >
> > >  SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > > @@ -30,7 +30,6 @@ typedef struct {
> > >    EFI_PHYSICAL_ADDRESS    ReturnContext1;
> > >    EFI_PHYSICAL_ADDRESS    ReturnContext2;
> > >    EFI_PHYSICAL_ADDRESS    ReturnStackPointer;
> > > -  EFI_PHYSICAL_ADDRESS    MpService2Ppi;
> > >    EFI_PHYSICAL_ADDRESS    Smst;
> > >  } SMM_S3_RESUME_STATE;
> > >
> > > --
> > > 2.31.1.windows.1
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > > 
> > >
> >
> >
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119377): https://edk2.groups.io/g/devel/message/119377
Mute This Topic: https://groups.io/mt/106383229/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2024-05-30  5:00 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-17  9:45 [edk2-devel] [Patch V2 00/18] Remove some S3 related code in CpuS3.c of smm cpu driver duntan
2024-05-17  9:45 ` [edk2-devel] [Patch V2 01/18] MdeModulePkg: Add gEdkiiS3MtrrSettingGuid duntan
2024-05-17  9:45 ` [edk2-devel] [Patch V2 02/18] OvmfPkg: Save MTRR by lockbox in CpuS3DataDxe duntan
2024-05-17  9:45 ` [edk2-devel] [Patch V2 03/18] UefiCpuPkg: Add locbox lib instance in DSC duntan
2024-05-17  9:45 ` [edk2-devel] [Patch V2 04/18] UefiCpuPkg: Save MTRR by lockbox in CpuS3DataDxe duntan
2024-05-17  9:45 ` [edk2-devel] [Patch V2 05/18] UefiCpuPkg: LoadMtrrData for all cpu in S3Resume duntan
2024-05-17  9:45 ` [edk2-devel] [Patch V2 06/18] UefiCpuPkg: Remove code to load mtrr setting duntan
2024-05-17  9:45 ` [edk2-devel] [Patch V2 07/18] UefiCpuPkg:Set PcdCpuFeaturesInitOnS3Resume to TRUE duntan
2024-05-17  9:46 ` [edk2-devel] [Patch V2 08/18] UefiCpuPkg: Remove code to set register table duntan
2024-05-17  9:46 ` [edk2-devel] [Patch V2 09/18] UefiCpuPkg: Disable PG in IA32 ApLoopCode duntan
2024-05-17  9:46 ` [edk2-devel] [Patch V2 10/18] UefiCpuPkg:Abstract some DxeMpLib code to function duntan
2024-05-17  9:46 ` [edk2-devel] [Patch V2 11/18] UefiCpuPkg:Move some code in DxeMpLib to common place duntan
2024-05-17  9:46 ` [edk2-devel] [Patch V2 12/18] UefiCpuPkg: Install gEdkiiEndOfS3ResumeGuid in S3Resume duntan
2024-05-17  9:46 ` [edk2-devel] [Patch V2 13/18] UefiCpuPkg:Relocate AP to new safe buffer in PeiMpLib duntan
2024-05-17  9:46 ` [edk2-devel] [Patch V2 14/18] UefiCpuPkg:Remove code to handle APIC setting and Interrupt duntan
2024-05-17  9:46 ` [edk2-devel] [Patch V2 15/18] UefiCpuPkg:Remove code to wakeup AP and relocate ap duntan
2024-05-17  9:46 ` [edk2-devel] [Patch V2 16/18] UefiCpuPkg: Remove unneeded MpService2Ppi assignment duntan
2024-05-17  9:46 ` [edk2-devel] [Patch V2 17/18] MdeModulePkg:Remove MpService2Ppi field in SMM_S3_RESUME_STATE duntan
2024-05-17  9:46 ` [edk2-devel] [Patch V2 18/18] UefiCpuPkg: Remove GetAcpiCpuData() in CpuS3.c duntan
2024-05-20  7:05 ` [edk2-devel] [Patch V2 00/18] Remove some S3 related code in CpuS3.c of smm cpu driver Ni, Ray
2024-05-24  3:18   ` duntan
2024-05-24  3:07 ` Wu, Jiaxin
     [not found] ` <17D03DA5161A67E3.17823@groups.io>
2024-05-28  6:35   ` [edk2-devel] [Patch V2 17/18] MdeModulePkg:Remove MpService2Ppi field in SMM_S3_RESUME_STATE duntan
2024-05-28  9:16     ` 回复: " gaoliming via groups.io
2024-05-28  9:34       ` duntan
2024-05-28  9:39         ` 回复: " gaoliming via groups.io
2024-05-28  9:44           ` duntan
2024-05-30  5:00             ` 回复: " gaoliming via groups.io
     [not found] ` <17D03D9CBFB2693B.16299@groups.io>
2024-05-28  6:37   ` [edk2-devel] [Patch V2 01/18] MdeModulePkg: Add gEdkiiS3MtrrSettingGuid duntan
2024-05-28  9:14     ` 回复: " gaoliming via groups.io

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox