Re: [edk2-devel] [PATCH v4 05/14] UefiCpuPkg/SmmRelocationLib: Remove unnecessary global variable
Reviewed-by: Ray Ni
Thanks,
Ray
From: Wu, Jiaxin
Sent: Friday, April 26, 2024 20:17
To: devel@edk2.groups.io
Cc: Ni, Ray ; Zeng, Star ; Gerd Hoffmann
; Kumar, Rahul R
Subject: [PATCH v4 05/14] UefiCpuPkg/SmmRelocationLib: Remove unnecessary
global variable
This patch aims on mProcessorInfo global variable, which can be
defined as local variable in SmmRelocateBases(). With this patch,
no need to allocate the memory for all CPUs to store the
Processor Info.
Cc: Ray Ni
Cc: Zeng Star
Cc: Gerd Hoffmann
Cc: Rahul Kumar
Signed-off-by: Jiaxin Wu
---
.../Library/SmmRelocationLib/SmmRelocationLib.c| 88 --
1 file changed, 32 insertions(+), 56 deletions(-)
diff --git a/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c
b/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c
index 3694a07cbb..86df66a280 100644
--- a/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c
+++ b/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c
@@ -14,15 +14,10 @@
#include "InternalSmmRelocationLib.h"
UINTN mMaxNumberOfCpus = 1;
UINTN mNumberOfCpus= 1;
-//
-// Record all Processors Info
-//
-EFI_PROCESSOR_INFORMATION *mProcessorInfo = NULL;
-
//
// IDT used during SMM Init
//
IA32_DESCRIPTOR gcSmmInitIdtr;
@@ -34,10 +29,15 @@ UINT64 mSmBase;
//
// SmBase Rebased flag for current CPU
//
volatile BOOLEAN mRebased;
+//
+// CpuIndex for current CPU
+//
+UINTN mCpuIndex;
+
/**
This function will get the SmBase for CpuIndex.
@param[in] CpuIndexThe processor index.
@param[in] SmmRelocationStart The start address of Smm relocated memory
in SMRAM.
@@ -138,37 +138,26 @@ VOID
EFIAPI
SmmInitHandler (
VOID
)
{
- UINT32 ApicId;
- UINTN Index;
-
//
// Update SMM IDT entries' code segment and load IDT
//
AsmWriteIdtr ();
- ApicId = GetApicId ();
- for (Index = 0; Index < mNumberOfCpus; Index++) {
-if (ApicId == (UINT32)mProcessorInfo[Index].ProcessorId) {
- //
- // Configure SmBase.
- //
- ConfigureSmBase (mSmBase);
-
- //
- // Hook return after RSM to set SMM re-based flag
- // SMM re-based flag can't be set before RSM, because SMM save state
context might be override
- // by next AP flow before it take effect.
- //
- SemaphoreHook (Index, );
- return;
-}
- }
+ //
+ // Configure SmBase.
+ //
+ ConfigureSmBase (mSmBase);
- ASSERT (FALSE);
+ //
+ // Hook return after RSM to set SMM re-based flag
+ // SMM re-based flag can't be set before RSM, because SMM save state context
might be override
+ // by next AP flow before it take effect.
+ //
+ SemaphoreHook (mCpuIndex, );
}
/**
Relocate SmmBases for each processor.
Execute on first boot and all S3 resumes
@@ -185,17 +174,19 @@ SmmRelocateBases (
IN EDKII_PEI_MP_SERVICES2_PPI *MpServices2,
IN EFI_PHYSICAL_ADDRESSSmmRelocationStart,
IN UINTN TileSize
)
{
- UINT8 BakBuf[BACK_BUF_SIZE];
- SMRAM_SAVE_STATE_MAP BakBuf2;
- SMRAM_SAVE_STATE_MAP *CpuStatePtr;
- UINT8 *U8Ptr;
- UINTN Index;
- UINTN BspIndex;
- UINT32BspApicId;
+ EFI_STATUS Status;
+ UINT8 BakBuf[BACK_BUF_SIZE];
+ SMRAM_SAVE_STATE_MAP BakBuf2;
+ SMRAM_SAVE_STATE_MAP *CpuStatePtr;
+ UINT8 *U8Ptr;
+ UINTN Index;
+ UINTN BspIndex;
+ UINT32 BspApicId;
+ EFI_PROCESSOR_INFORMATION ProcessorInfo;
//
// Make sure the reserved size is large enough for procedure SmmInitTemplate.
//
ASSERT (sizeof (BakBuf) >= gcSmmInitSize);
@@ -230,14 +221,18 @@ SmmRelocateBases (
// Relocate SM bases for all APs
// This is APs' 1st SMI - rebase will be done here, and APs' default SMI
handler will be overridden by gcSmmInitTemplate
//
BspIndex = (UINTN)-1;
for (Index = 0; Index < mNumberOfCpus; Index++) {
-if (BspApicId != (UINT32)mProcessorInfo[Index].ProcessorId) {
- mRebased = FALSE;
- mSmBase = GetSmBase (Index, SmmRelocationStart, TileSize);
- SendSmiIpi ((UINT32)mProcessorInfo[Index].ProcessorId);
+Status = MpServices2->GetProcessorInfo (MpServices2, Index |
CPU_V2_EXTENDED_TOPOLOGY, );
+ASSERT_EFI_ERROR (Status);
+
+if (BspApicId != (UINT32)ProcessorInfo.ProcessorId) {
+ mRebased = FALSE;
+ mSmBase = GetSmBase (Index, SmmRelocationStart, TileSize);
+ mCpuIndex = Index;
+ SendSmiIpi ((UINT32)ProcessorInfo.ProcessorId);
//
// Wait for this AP to finish its 1st SMI
//
while (!mRebased) {
}
@@ -443,11 +438,10 @@ SmmRelocationInit (
UINTN TileSize;
UINT64SmmRelocationSize;
EFI_PHYSICAL_ADDRESS SmmRelocationStart;
UINTN SmmStackSize;
UINT8 *SmmStacks;
- UINTN
[edk2-devel] [PATCH v4 05/14] UefiCpuPkg/SmmRelocationLib: Remove unnecessary global variable
This patch aims on mProcessorInfo global variable, which can be
defined as local variable in SmmRelocateBases(). With this patch,
no need to allocate the memory for all CPUs to store the
Processor Info.
Cc: Ray Ni
Cc: Zeng Star
Cc: Gerd Hoffmann
Cc: Rahul Kumar
Signed-off-by: Jiaxin Wu
---
.../Library/SmmRelocationLib/SmmRelocationLib.c| 88 --
1 file changed, 32 insertions(+), 56 deletions(-)
diff --git a/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c
b/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c
index 3694a07cbb..86df66a280 100644
--- a/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c
+++ b/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c
@@ -14,15 +14,10 @@
#include "InternalSmmRelocationLib.h"
UINTN mMaxNumberOfCpus = 1;
UINTN mNumberOfCpus= 1;
-//
-// Record all Processors Info
-//
-EFI_PROCESSOR_INFORMATION *mProcessorInfo = NULL;
-
//
// IDT used during SMM Init
//
IA32_DESCRIPTOR gcSmmInitIdtr;
@@ -34,10 +29,15 @@ UINT64 mSmBase;
//
// SmBase Rebased flag for current CPU
//
volatile BOOLEAN mRebased;
+//
+// CpuIndex for current CPU
+//
+UINTN mCpuIndex;
+
/**
This function will get the SmBase for CpuIndex.
@param[in] CpuIndexThe processor index.
@param[in] SmmRelocationStart The start address of Smm relocated memory
in SMRAM.
@@ -138,37 +138,26 @@ VOID
EFIAPI
SmmInitHandler (
VOID
)
{
- UINT32 ApicId;
- UINTN Index;
-
//
// Update SMM IDT entries' code segment and load IDT
//
AsmWriteIdtr ();
- ApicId = GetApicId ();
- for (Index = 0; Index < mNumberOfCpus; Index++) {
-if (ApicId == (UINT32)mProcessorInfo[Index].ProcessorId) {
- //
- // Configure SmBase.
- //
- ConfigureSmBase (mSmBase);
-
- //
- // Hook return after RSM to set SMM re-based flag
- // SMM re-based flag can't be set before RSM, because SMM save state
context might be override
- // by next AP flow before it take effect.
- //
- SemaphoreHook (Index, );
- return;
-}
- }
+ //
+ // Configure SmBase.
+ //
+ ConfigureSmBase (mSmBase);
- ASSERT (FALSE);
+ //
+ // Hook return after RSM to set SMM re-based flag
+ // SMM re-based flag can't be set before RSM, because SMM save state context
might be override
+ // by next AP flow before it take effect.
+ //
+ SemaphoreHook (mCpuIndex, );
}
/**
Relocate SmmBases for each processor.
Execute on first boot and all S3 resumes
@@ -185,17 +174,19 @@ SmmRelocateBases (
IN EDKII_PEI_MP_SERVICES2_PPI *MpServices2,
IN EFI_PHYSICAL_ADDRESSSmmRelocationStart,
IN UINTN TileSize
)
{
- UINT8 BakBuf[BACK_BUF_SIZE];
- SMRAM_SAVE_STATE_MAP BakBuf2;
- SMRAM_SAVE_STATE_MAP *CpuStatePtr;
- UINT8 *U8Ptr;
- UINTN Index;
- UINTN BspIndex;
- UINT32BspApicId;
+ EFI_STATUS Status;
+ UINT8 BakBuf[BACK_BUF_SIZE];
+ SMRAM_SAVE_STATE_MAP BakBuf2;
+ SMRAM_SAVE_STATE_MAP *CpuStatePtr;
+ UINT8 *U8Ptr;
+ UINTN Index;
+ UINTN BspIndex;
+ UINT32 BspApicId;
+ EFI_PROCESSOR_INFORMATION ProcessorInfo;
//
// Make sure the reserved size is large enough for procedure SmmInitTemplate.
//
ASSERT (sizeof (BakBuf) >= gcSmmInitSize);
@@ -230,14 +221,18 @@ SmmRelocateBases (
// Relocate SM bases for all APs
// This is APs' 1st SMI - rebase will be done here, and APs' default SMI
handler will be overridden by gcSmmInitTemplate
//
BspIndex = (UINTN)-1;
for (Index = 0; Index < mNumberOfCpus; Index++) {
-if (BspApicId != (UINT32)mProcessorInfo[Index].ProcessorId) {
- mRebased = FALSE;
- mSmBase = GetSmBase (Index, SmmRelocationStart, TileSize);
- SendSmiIpi ((UINT32)mProcessorInfo[Index].ProcessorId);
+Status = MpServices2->GetProcessorInfo (MpServices2, Index |
CPU_V2_EXTENDED_TOPOLOGY, );
+ASSERT_EFI_ERROR (Status);
+
+if (BspApicId != (UINT32)ProcessorInfo.ProcessorId) {
+ mRebased = FALSE;
+ mSmBase = GetSmBase (Index, SmmRelocationStart, TileSize);
+ mCpuIndex = Index;
+ SendSmiIpi ((UINT32)ProcessorInfo.ProcessorId);
//
// Wait for this AP to finish its 1st SMI
//
while (!mRebased) {
}
@@ -443,11 +438,10 @@ SmmRelocationInit (
UINTN TileSize;
UINT64SmmRelocationSize;
EFI_PHYSICAL_ADDRESS SmmRelocationStart;
UINTN SmmStackSize;
UINT8 *SmmStacks;
- UINTN Index;
SmmRelocationStart = 0;
SmmStacks = NULL;
DEBUG ((DEBUG_INFO, "SmmRelocationInit Start \n"));
@@ -473,28 +467,10 @@ SmmRelocationInit (
mMaxNumberOfCpus = mNumberOfCpus;
}
ASSERT (mNumberOfCpus <= mMaxNumberOfCpus);
- //
- //
