UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 19 ++++++++++++++++++- UefiCpuPkg/Library/MpInitLib/MpLib.c | 2 +- 2 files changed, 19 insertions(+), 2 deletions(-)
> v2 changes:
> a. Use each AP's ApTopOfStack to get the stack base address instead of
> cpu0's ApTopOfStack which is actually set incorrectly before.
> b. Fix cpu0's ApTopOfStack initialization.
> c. Fix wrong debug print format.
The reason is that DXE part initialization will reuse the stack allocated
at PEI phase, if MP was initialized before. Some code added to check this
situation and use stack base address saved in HOB passed from PEI.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
---
UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 19 ++++++++++++++++++-
UefiCpuPkg/Library/MpInitLib/MpLib.c | 2 +-
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index 40c1bf407a..e832c16eca 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
@@ -295,6 +295,7 @@ InitMpGlobalData (
UINTN Index;
EFI_GCD_MEMORY_SPACE_DESCRIPTOR MemDesc;
UINTN StackBase;
+ CPU_INFO_IN_HOB *CpuInfoInHob;
SaveCpuMpData (CpuMpData);
@@ -314,8 +315,21 @@ InitMpGlobalData (
ASSERT (FALSE);
}
+ //
+ // DXE will reuse stack allocated for APs at PEI phase if it's available.
+ // Let's check it here.
+ //
+ // Note: BSP's stack guard is set at DxeIpl phase. But for the sake of
+ // BSP/AP exchange, stack guard for ApTopOfStack of cpu 0 will still be
+ // set here.
+ //
+ CpuInfoInHob = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob;
for (Index = 0; Index < CpuMpData->CpuCount; ++Index) {
- StackBase = CpuMpData->Buffer + Index * CpuMpData->CpuApStackSize;
+ if (CpuInfoInHob != NULL && CpuInfoInHob[Index].ApTopOfStack != 0) {
+ StackBase = CpuInfoInHob[Index].ApTopOfStack - CpuMpData->CpuApStackSize;
+ } else {
+ StackBase = CpuMpData->Buffer + Index * CpuMpData->CpuApStackSize;
+ }
Status = gDS->GetMemorySpaceDescriptor (StackBase, &MemDesc);
ASSERT_EFI_ERROR (Status);
@@ -326,6 +340,9 @@ InitMpGlobalData (
MemDesc.Attributes | EFI_MEMORY_RP
);
ASSERT_EFI_ERROR (Status);
+
+ DEBUG ((DEBUG_INFO, "Stack Guard set at %lx [cpu%lu]!\n",
+ (UINT64)StackBase, (UINT64)Index));
}
}
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 0c2058a7b0..1bfab8467b 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -1498,7 +1498,7 @@ MpInitLibInitialize (
//
// Set BSP basic information
//
- InitializeApData (CpuMpData, 0, 0, CpuMpData->Buffer);
+ InitializeApData (CpuMpData, 0, 0, CpuMpData->Buffer + ApStackSize);
//
// Save assembly code information
//
--
2.15.1.windows.2
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 01/04/18 04:09, Jian J Wang wrote: >> v2 changes: >> a. Use each AP's ApTopOfStack to get the stack base address instead of >> cpu0's ApTopOfStack which is actually set incorrectly before. >> b. Fix cpu0's ApTopOfStack initialization. >> c. Fix wrong debug print format. The end result of this patch looks fine to me. However, please split update (b), which affects "MpLib.c", to a separate patch. The reason is that update (b) addresses a distinct bug. So in v3, - patch v3 1/2 should fix the top-of-stack initialization for the BSP, with its own commit message (referencing the BSP/AP switch service), - patch v3 2/2 should do everything else from v2. I'm ready to R-b such a v3. Thank you! Laszlo > The reason is that DXE part initialization will reuse the stack allocated > at PEI phase, if MP was initialized before. Some code added to check this > situation and use stack base address saved in HOB passed from PEI. > > Cc: Jiewen Yao <jiewen.yao@intel.com> > Cc: Eric Dong <eric.dong@intel.com> > Cc: Laszlo Ersek <lersek@redhat.com> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Jian J Wang <jian.j.wang@intel.com> > --- > UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 19 ++++++++++++++++++- > UefiCpuPkg/Library/MpInitLib/MpLib.c | 2 +- > 2 files changed, 19 insertions(+), 2 deletions(-) > > diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c > index 40c1bf407a..e832c16eca 100644 > --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c > +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c > @@ -295,6 +295,7 @@ InitMpGlobalData ( > UINTN Index; > EFI_GCD_MEMORY_SPACE_DESCRIPTOR MemDesc; > UINTN StackBase; > + CPU_INFO_IN_HOB *CpuInfoInHob; > > SaveCpuMpData (CpuMpData); > > @@ -314,8 +315,21 @@ InitMpGlobalData ( > ASSERT (FALSE); > } > > + // > + // DXE will reuse stack allocated for APs at PEI phase if it's available. > + // Let's check it here. > + // > + // Note: BSP's stack guard is set at DxeIpl phase. But for the sake of > + // BSP/AP exchange, stack guard for ApTopOfStack of cpu 0 will still be > + // set here. > + // > + CpuInfoInHob = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob; > for (Index = 0; Index < CpuMpData->CpuCount; ++Index) { > - StackBase = CpuMpData->Buffer + Index * CpuMpData->CpuApStackSize; > + if (CpuInfoInHob != NULL && CpuInfoInHob[Index].ApTopOfStack != 0) { > + StackBase = CpuInfoInHob[Index].ApTopOfStack - CpuMpData->CpuApStackSize; > + } else { > + StackBase = CpuMpData->Buffer + Index * CpuMpData->CpuApStackSize; > + } > > Status = gDS->GetMemorySpaceDescriptor (StackBase, &MemDesc); > ASSERT_EFI_ERROR (Status); > @@ -326,6 +340,9 @@ InitMpGlobalData ( > MemDesc.Attributes | EFI_MEMORY_RP > ); > ASSERT_EFI_ERROR (Status); > + > + DEBUG ((DEBUG_INFO, "Stack Guard set at %lx [cpu%lu]!\n", > + (UINT64)StackBase, (UINT64)Index)); > } > } > > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c > index 0c2058a7b0..1bfab8467b 100644 > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c > @@ -1498,7 +1498,7 @@ MpInitLibInitialize ( > // > // Set BSP basic information > // > - InitializeApData (CpuMpData, 0, 0, CpuMpData->Buffer); > + InitializeApData (CpuMpData, 0, 0, CpuMpData->Buffer + ApStackSize); > // > // Save assembly code information > // > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
© 2016 - 2024 Red Hat, Inc.