Re: [edk2-devel] [PATCH 10/18] UefiCpuPkg:Relocate AP to new safe buffer in PeiMpLib

2024-05-13 Thread Gerd Hoffmann 
On Tue, May 14, 2024 at 05:17:51AM GMT, Ni, Ray wrote:
> Gerd,
> I agree that the logic might be duplicated in multi places.
> 
> But even CPU supports 1G paging, caller can decide whether to use 1G paging 
> or 2M paging, or 4K paging.
> Using a single API to encapsulate the entire logic may not seem flexible.

Sure, I don't want take away that flexibility.  A caller might also
prepare page tables for a paging mode not matching the current CPU
paging mode (i.e. 32-bit PEI preparing page tables for 64-bit DXE).

> Maybe, a lib API to detect 1G paging capability can be added to CpuLib.

Yep, that is exactly what I think would be useful.  Add a

PAGING_MODE PageTableBestMode(VOID);

function with this code to CpuPageTableLib, so callers have the option
to use

PagingMode = PageTableBestMode();

instead of duplicating the code block.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118881): https://edk2.groups.io/g/devel/message/118881
Mute This Topic: https://groups.io/mt/106018135/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 10/18] UefiCpuPkg:Relocate AP to new safe buffer in PeiMpLib

2024-05-13 Thread Ni, Ray 
Gerd,
I agree that the logic might be duplicated in multi places.

But even CPU supports 1G paging, caller can decide whether to use 1G paging or 
2M paging, or 4K paging.
Using a single API to encapsulate the entire logic may not seem flexible.
Maybe, a lib API to detect 1G paging capability can be added to CpuLib.

Thanks,
Ray

From: Gerd Hoffmann 
Sent: Monday, May 13, 2024 19:07
To: Tan, Dun 
Cc: devel@edk2.groups.io ; Ni, Ray ; 
Laszlo Ersek ; Kumar, Rahul R ; Wu, 
Jiaxin 
Subject: Re: [PATCH 10/18] UefiCpuPkg:Relocate AP to new safe buffer in PeiMpLib

  Hi,

> +  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;
> +  }

I'm wondering whenever CpuPageTableLib should get a function for this?
I suspect there a multiple places in edk2 which will need this
functionality ...

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118880): https://edk2.groups.io/g/devel/message/118880
Mute This Topic: https://groups.io/mt/106018135/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 10/18] UefiCpuPkg:Relocate AP to new safe buffer in PeiMpLib

2024-05-13 Thread Gerd Hoffmann 
  Hi,

> +  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;
> +  }

I'm wondering whenever CpuPageTableLib should get a function for this?
I suspect there a multiple places in edk2 which will need this
functionality ...

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118862): https://edk2.groups.io/g/devel/message/118862
Mute This Topic: https://groups.io/mt/106018135/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 10/18] UefiCpuPkg:Relocate AP to new safe buffer in PeiMpLib

2024-05-12 Thread Ni, Ray 
Reviewed-by: Ray Ni 

Thanks,
Ray

From: Tan, Dun 
Sent: Friday, May 10, 2024 18:08
To: devel@edk2.groups.io 
Cc: Ni, Ray ; Laszlo Ersek ; Kumar, Rahul 
R ; Gerd Hoffmann ; Wu, Jiaxin 

Subject: [PATCH 10/18] UefiCpuPkg:Relocate AP to new safe buffer in PeiMpLib

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 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
Cc: Jiaxin Wu 
---
 UefiCpuPkg/Library/MpInitLib/MpLib.h  |   3 +++
 UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   4 
 UefiCpuPkg/Library/MpInitLib/PeiMpLib.c   | 152 

 3 files changed, 159 insertions(+)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h 
b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 11e0d2661f..3efd913395 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -33,6 +33,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 

 #include 
@@ -68,6 +69,8 @@
 //
 #define DEFAULT_MAX_MICROCODE_PATCH_NUM  8

+#define PAGING_4K_ADDRESS_MASK_64  0x000FF000ull
+
 //
 // Data structure for microcode patch information
 //
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf 
b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
index e31e34b6f9..8736690348 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
@@ -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..deb5fc3aac 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
@@ -9,6 +9,7 @@
 #include "MpLib.h"
 #include 
 #include 
+#include 
 #include 

 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  InvokePpiAddress 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]  PagesNumber 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,

[edk2-devel] [PATCH 10/18] UefiCpuPkg:Relocate AP to new safe buffer in PeiMpLib

2024-05-10 Thread duntan 
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 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
Cc: Jiaxin Wu 
---
 UefiCpuPkg/Library/MpInitLib/MpLib.h  |   3 +++
 UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   4 
 UefiCpuPkg/Library/MpInitLib/PeiMpLib.c   | 152 

 3 files changed, 159 insertions(+)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h 
b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 11e0d2661f..3efd913395 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -33,6 +33,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -68,6 +69,8 @@
 //
 #define DEFAULT_MAX_MICROCODE_PATCH_NUM  8
 
+#define PAGING_4K_ADDRESS_MASK_64  0x000FF000ull
+
 //
 // Data structure for microcode patch information
 //
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf 
b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
index e31e34b6f9..8736690348 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
@@ -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..deb5fc3aac 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
@@ -9,6 +9,7 @@
 #include "MpLib.h"
 #include 
 #include 
+#include 
 #include 
 
 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  InvokePpiAddress 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]  PagesNumber 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 attrib