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....@intel.com>
> Cc: Eric Dong <eric.d...@intel.com>
> Cc: Laszlo Ersek <ler...@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang <jian.j.w...@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
