Re: [edk2-devel] [PATCH] UefiCpuPkg: Use Top of each AP's stack to save CpuMpData

["Ni, Ray" <ray.ni@intel.com>]

> -  MpBuffer    = AllocatePages (EFI_SIZE_TO_PAGES (BufferSize));
> +  //
> +  // Allocate extra ApStackSize to let stack align on ApStackSize bounday
> +  //
> +  MpBuffer = AllocatePages (EFI_SIZE_TO_PAGES (BufferSize));

1. Above comments are not necessary. Can you please remove them?

>    ASSERT (
> -    (CpuMpData->CpuInfoInHob + sizeof (CPU_INFO_IN_HOB) *
> MaxLogicalProcessorNumber) ==
> -    Buffer + BufferSize
> +    (CpuMpData->CpuInfoInHob + sizeof (CPU_INFO_IN_HOB) *
> MaxLogicalProcessorNumber) <=
> +    (UINTN)MpBuffer + BufferSize
>      );

2. Can you try changing "<=" back to "=="? I agree that "Buffer" should
be changed to "MpBuffer" in above check.


> -  For AP, the structure is just after IDT.
> +  For AP, the structure is stored in the top of each AP's stack.

3. Can you change it to "For AP, the pointer of CPU_MP_DATA is
stored in top of each AP's stack."?

> +    ;
> +    ; Reserve 8 bytes for storing CpuMpData.
> +    ; Using sub rsp instead of push rbp to avoid overwriting existed
> CpuMpData

4. Can you explain a bit more in which case the overwritten may happen?
I guess the overwritten may happen when the PcdCpuApLoopMode is
ApInHltLoop but pointer of CpuMpData is only stored in top of AP stack
in first time INIT-SIPI-SIPI.