Re: [edk2-devel] [PATCH v1 13/13] UefiCpuPkg/PiSmmCpuDxeSmm: Remove SmBases relocation logic
@@ -348,14 +336,10 @@ SmmInitHandler (
[Ray.1] Can you rename this function to a different name? Originally it was
really a handler to initialize SMM env called from SmmInit.nasm. But today it's
purely to initialize the SMM env.
How about "InitializeSmm"? And "EFIAPI" is not needed as it's not called from
ASEMBLY anymore.
{
UINT32 ApicId;
UINTNIndex;
BOOLEAN IsBsp;
- //
- // Update SMM IDT entries' code segment and load IDT
- //
- AsmWriteIdtr (&gcSmiIdtr);
[Ray.2]
OK.
The IDTR update is needed when it's called from SmmInit.nasm as IDTR is not
updated there.
But it's not needed when it's called from SmmEntry.nasm as IDTR is updated
there.
Other changes look good to me.
From: Wu, Jiaxin
Sent: Wednesday, April 10, 2024 21:57
To: devel@edk2.groups.io
Cc: Ni, Ray ; Zeng, Star ; Gerd Hoffmann
; Kumar, Rahul R
Subject: [PATCH v1 13/13] UefiCpuPkg/PiSmmCpuDxeSmm: Remove SmBases relocation
logic
This patch is to remove legacy SmBase relocation in
PiSmmCpuDxeSmm Driver, and the SmBase relocation
behavior will be in the SmmRelocationInit interface:
1. Relocate smbases for each processor.
2. Create the gSmmBaseHobGuid HOB.
Then, PiSmmCpuDxeSmm driver can be simplified to:
1. Consume the gSmmBaseHobGuid for the smbase.
2. ExecuteFirstSmiInit for early SMM Init.
Cc: Ray Ni
Cc: Zeng Star
Cc: Gerd Hoffmann
Cc: Rahul Kumar
Signed-off-by: Jiaxin Wu
---
UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c| 21 +-
UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c | 42
UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm | 96
UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c| 6 +-
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 322 ++-
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 98
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 4 -
UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c | 69 --
UefiCpuPkg/PiSmmCpuDxeSmm/X64/Semaphore.c| 69 --
UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmInit.nasm | 146
10 files changed, 30 insertions(+), 843 deletions(-)
delete mode 100644 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c
delete mode 100644 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm
delete mode 100644 UefiCpuPkg/PiSmmCpuDxeSmm/X64/Semaphore.c
delete mode 100644 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmInit.nasm
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
index b14c289a27..d67fb49890 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
@@ -639,27 +639,14 @@ InitializeCpuProcedure (
//
InitializeCpuBeforeRebase (IsBsp);
}
if (IsBsp) {
-DEBUG ((DEBUG_INFO, "SmmRestoreCpu: mSmmRelocated is %d\n",
mSmmRelocated));
-
//
-// Check whether Smm Relocation is done or not.
-// If not, will do the SmmBases Relocation here!!!
+// Issue SMI IPI (All Excluding Self SMM IPI + BSP SMM IPI) to execute
first SMI init.
//
-if (!mSmmRelocated) {
- //
- // Restore SMBASE for BSP and all APs
- //
- SmmRelocateBases ();
-} else {
- //
- // Issue SMI IPI (All Excluding Self SMM IPI + BSP SMM IPI) to execute
first SMI init.
- //
- ExecuteFirstSmiInit ();
-}
+ExecuteFirstSmiInit ();
}
//
// Skip initialization if mAcpiCpuData is not valid
//
@@ -978,13 +965,13 @@ InitSmmS3ResumeState (
SmmS3ResumeState->SmmS3StackBase =
(EFI_PHYSICAL_ADDRESS)(UINTN)AllocatePages (EFI_SIZE_TO_PAGES
((UINTN)SmmS3ResumeState->SmmS3StackSize));
if (SmmS3ResumeState->SmmS3StackBase == 0) {
SmmS3ResumeState->SmmS3StackSize = 0;
}
-SmmS3ResumeState->SmmS3Cr0 = mSmmCr0;
+SmmS3ResumeState->SmmS3Cr0 = (UINT32)AsmReadCr0 ();
SmmS3ResumeState->SmmS3Cr3 = Cr3;
-SmmS3ResumeState->SmmS3Cr4 = mSmmCr4;
+SmmS3ResumeState->SmmS3Cr4 = (UINT32)AsmReadCr4 ();
if (sizeof (UINTN) == sizeof (UINT64)) {
SmmS3ResumeState->Signature = SMM_S3_RESUME_SMM_64;
}
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c
b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c
deleted file mode 100644
index a9fcc89dda..00
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c
+++ /dev/null
@@ -1,42 +0,0 @@
-/** @file
-Semaphore mechanism to indicate to the BSP that an AP has exited SMM
-after SMBASE relocation.
-
-Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.
-SPDX-License-Identifier: BSD-2-Clause-Patent
-
-**/
-
-#include "PiSmmCpuDxeSmm.h"
-
-UINTN mSmmRelocationOriginalAddress;
-volatile BOOLEAN *mRebasedFlag;
-
-/**
- Hook return address of SMM Save State so that semaphore code
- can be executed immediately after AP exits SMM to indicate to
- the BSP that an AP has exited SMM after SMBASE relocation.
-
- @param[in] CpuIndex The processor index.
- @param[in] RebasedFlag A pointer to a flag that is set to TRUE
- immediately after AP exits SMM.
-
-**/
-VOID
-SemaphoreHook (
-
[edk2-devel] [PATCH v1 13/13] UefiCpuPkg/PiSmmCpuDxeSmm: Remove SmBases relocation logic
This patch is to remove legacy SmBase relocation in
PiSmmCpuDxeSmm Driver, and the SmBase relocation
behavior will be in the SmmRelocationInit interface:
1. Relocate smbases for each processor.
2. Create the gSmmBaseHobGuid HOB.
Then, PiSmmCpuDxeSmm driver can be simplified to:
1. Consume the gSmmBaseHobGuid for the smbase.
2. ExecuteFirstSmiInit for early SMM Init.
Cc: Ray Ni
Cc: Zeng Star
Cc: Gerd Hoffmann
Cc: Rahul Kumar
Signed-off-by: Jiaxin Wu
---
UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c| 21 +-
UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c | 42
UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm | 96
UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c| 6 +-
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 322 ++-
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 98
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 4 -
UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c | 69 --
UefiCpuPkg/PiSmmCpuDxeSmm/X64/Semaphore.c| 69 --
UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmInit.nasm | 146
10 files changed, 30 insertions(+), 843 deletions(-)
delete mode 100644 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c
delete mode 100644 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm
delete mode 100644 UefiCpuPkg/PiSmmCpuDxeSmm/X64/Semaphore.c
delete mode 100644 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmInit.nasm
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
index b14c289a27..d67fb49890 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
@@ -639,27 +639,14 @@ InitializeCpuProcedure (
//
InitializeCpuBeforeRebase (IsBsp);
}
if (IsBsp) {
-DEBUG ((DEBUG_INFO, "SmmRestoreCpu: mSmmRelocated is %d\n",
mSmmRelocated));
-
//
-// Check whether Smm Relocation is done or not.
-// If not, will do the SmmBases Relocation here!!!
+// Issue SMI IPI (All Excluding Self SMM IPI + BSP SMM IPI) to execute
first SMI init.
//
-if (!mSmmRelocated) {
- //
- // Restore SMBASE for BSP and all APs
- //
- SmmRelocateBases ();
-} else {
- //
- // Issue SMI IPI (All Excluding Self SMM IPI + BSP SMM IPI) to execute
first SMI init.
- //
- ExecuteFirstSmiInit ();
-}
+ExecuteFirstSmiInit ();
}
//
// Skip initialization if mAcpiCpuData is not valid
//
@@ -978,13 +965,13 @@ InitSmmS3ResumeState (
SmmS3ResumeState->SmmS3StackBase =
(EFI_PHYSICAL_ADDRESS)(UINTN)AllocatePages (EFI_SIZE_TO_PAGES
((UINTN)SmmS3ResumeState->SmmS3StackSize));
if (SmmS3ResumeState->SmmS3StackBase == 0) {
SmmS3ResumeState->SmmS3StackSize = 0;
}
-SmmS3ResumeState->SmmS3Cr0 = mSmmCr0;
+SmmS3ResumeState->SmmS3Cr0 = (UINT32)AsmReadCr0 ();
SmmS3ResumeState->SmmS3Cr3 = Cr3;
-SmmS3ResumeState->SmmS3Cr4 = mSmmCr4;
+SmmS3ResumeState->SmmS3Cr4 = (UINT32)AsmReadCr4 ();
if (sizeof (UINTN) == sizeof (UINT64)) {
SmmS3ResumeState->Signature = SMM_S3_RESUME_SMM_64;
}
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c
b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c
deleted file mode 100644
index a9fcc89dda..00
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c
+++ /dev/null
@@ -1,42 +0,0 @@
-/** @file
-Semaphore mechanism to indicate to the BSP that an AP has exited SMM
-after SMBASE relocation.
-
-Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.
-SPDX-License-Identifier: BSD-2-Clause-Patent
-
-**/
-
-#include "PiSmmCpuDxeSmm.h"
-
-UINTN mSmmRelocationOriginalAddress;
-volatile BOOLEAN *mRebasedFlag;
-
-/**
- Hook return address of SMM Save State so that semaphore code
- can be executed immediately after AP exits SMM to indicate to
- the BSP that an AP has exited SMM after SMBASE relocation.
-
- @param[in] CpuIndex The processor index.
- @param[in] RebasedFlag A pointer to a flag that is set to TRUE
- immediately after AP exits SMM.
-
-**/
-VOID
-SemaphoreHook (
- IN UINTN CpuIndex,
- IN volatile BOOLEAN *RebasedFlag
- )
-{
- SMRAM_SAVE_STATE_MAP *CpuState;
-
- mRebasedFlag = RebasedFlag;
-
- CpuState = (SMRAM_SAVE_STATE_MAP
*)(UINTN)(SMM_DEFAULT_SMBASE + SMRAM_SAVE_STATE_MAP_OFFSET);
- mSmmRelocationOriginalAddress = (UINTN)HookReturnFromSmm (
- CpuIndex,
- CpuState,
-
(UINT64)(UINTN)&SmmRelocationSemaphoreComplete,
-
(UINT64)(UINTN)&SmmRelocationSemaphoreComplete
- );
-}
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm
b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm
deleted file mode 100644
index b5e77a1a5b..00
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm
+++ /dev/null
@@ -1,96 +0,0 @@
-;---
