Re: [Patch V3 4/5] UefiCpuPkg: ApWakeupFunction directly use CpuMpData.

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

Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: Xie, Yuanhao <yuanhao.xie@intel.com>
> Sent: Monday, June 26, 2023 1:57 PM
> To: devel@edk2.groups.io
> Cc: Gerd Hoffmann <kraxel@redhat.com>; Dong, Eric <eric.dong@intel.com>; Ni,
> Ray <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Tom
> Lendacky <thomas.lendacky@amd.com>; Xie, Yuanhao
> <yuanhao.xie@intel.com>
> Subject: [Patch V3 4/5] UefiCpuPkg: ApWakeupFunction directly use CpuMpData.
> 
> In the original design, once the APs finished executing their assembly
> code and switched to executing C code, they would enter a continuous
> loop within a function. In this function, they would collect CpuMpData
> using the MP_CPU_EXCHANGE_INFO mechanism. However, in the updated
> approach, CpuMpData can now be passed directly to the ApWakeUpFunction,
> bypassing the need for MP_CPU_EXCHANGE_INFO. This modification is made
> in preparation for eliminating the requirement of a second
> INIT-SIPI-SIPI sequence in the DXE phase.
> 
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Yuanhao Xie <yuanhao.xie@intel.com>
> ---
>  UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm |  4 ++--
>  UefiCpuPkg/Library/MpInitLib/MpLib.c           | 12 +++---------
>  UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm  |  3 +--
>  3 files changed, 6 insertions(+), 13 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> index 59db4081d6..d117f09ef5 100644
> --- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> +++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> @@ -197,8 +197,8 @@ CProcedureInvoke:
> 
>      push       ebx               ; Push ApIndex
>      mov        eax, esi
> -    add        eax, MP_CPU_EXCHANGE_INFO_OFFSET
> -    push       eax               ; push address of exchange info data buffer
> +    add        eax, MP_CPU_EXCHANGE_INFO_FIELD (CpuMpData)
> +    push       dword [eax]       ; push address of CpuMpData
> 
>      mov        edi, esi
>      add        edi, MP_CPU_EXCHANGE_INFO_FIELD (CFunction)
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index 1252ee9673..15b24b1805 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -710,17 +710,16 @@ PlaceAPInMwaitLoopOrRunLoop (
>  /**
>    This function will be called from AP reset code if BSP uses WakeUpAP.
> 
> -  @param[in] ExchangeInfo     Pointer to the MP exchange info buffer
> +  @param[in] CpuMpData        Pointer to CPU MP Data
>    @param[in] ApIndex          Number of current executing AP
>  **/
>  VOID
>  EFIAPI
>  ApWakeupFunction (
> -  IN MP_CPU_EXCHANGE_INFO  *ExchangeInfo,
> -  IN UINTN                 ApIndex
> +  IN CPU_MP_DATA  *CpuMpData,
> +  IN UINTN        ApIndex
>    )
>  {
> -  CPU_MP_DATA       *CpuMpData;
>    UINTN             ProcessorNumber;
>    EFI_AP_PROCEDURE  Procedure;
>    VOID              *Parameter;
> @@ -731,11 +730,6 @@ ApWakeupFunction (
>    UINTN             CurrentApicMode;
>    AP_STACK_DATA     *ApStackData;
> 
> -  //
> -  // AP finished assembly code and begin to execute C code
> -  //
> -  CpuMpData = ExchangeInfo->CpuMpData;
> -
>    //
>    // AP's local APIC settings will be lost after received INIT IPI
>    // We need to re-initialize them at here
> diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> index 5bcdf7726b..40e80ffab4 100644
> --- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> +++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> @@ -259,8 +259,7 @@ CProcedureInvoke:
>      add        rsp, 20h
> 
>      mov        edx, ebx          ; edx is ApIndex
> -    mov        ecx, esi
> -    add        ecx, MP_CPU_EXCHANGE_INFO_OFFSET ; rcx is address of exchange
> info data buffer
> +    mov        rcx, qword [esi + MP_CPU_EXCHANGE_INFO_FIELD (CpuMpData)]
> 
>      mov        edi, esi
>      add        edi, MP_CPU_EXCHANGE_INFO_FIELD (CFunction)
> --
> 2.36.1.windows.1