Hi Gerd,
-Now you are adding back the AmdSev version.
-It should be the generic version though.
Duplication is as I want to build up the desired functionality in small steps,
generic version is updated in patch3 and ready in patch 6.
Call AsmRelocateApLoopStartAmdSev brings more confusion.
Thanks
Regards,
Yuanhao
-----Original Message-----
From: Gerd Hoffmann <kra...@redhat.com>
Sent: Friday, February 24, 2023 3:38 PM
To: devel@edk2.groups.io; Xie, Yuanhao <yuanhao....@intel.com>
Cc: Dong, Guo <guo.d...@intel.com>; Ni, Ray <ray...@intel.com>; Rhodes, Sean
<sean@starlabs.systems>; Lu, James <james...@intel.com>; Guo, Gua
<gua....@intel.com>
Subject: Re: [edk2-devel] [Patch V3 2/6] UefiCpuPkg: Duplicate
AsmRelocateApLoopAmd.
On Fri, Feb 24, 2023 at 02:05:31AM +0800, Yuanhao Xie wrote:
> Duplicate AsmRelocateApLoopAmd for non-SEV-ES enabled processors.
>
> Cc: Guo Dong <guo.d...@intel.com>
> Cc: Ray Ni <ray...@intel.com>
> Cc: Sean Rhodes <sean@starlabs.systems>
> Cc: James Lu <james...@intel.com>
> Cc: Gua Guo <gua....@intel.com>
> Signed-off-by: Yuanhao Xie <yuanhao....@intel.com>
> Test-by: Yuanhao Xie <yuanhao....@intel.com>
> ---
> UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 68
> ++++++++++++++++++++++++++++++++++++++++++++------------------------
> UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 22 ++++++++++++----------
> UefiCpuPkg/Library/MpInitLib/MpLib.h | 31
> +++++++++++++++++++++++++++++--
> UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm | 33
> +++++++++++++++++----------------
> UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 171
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +++++++++++++++++++++++++++++++
> 5 files changed, 273 insertions(+), 52 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> index a84e9e33ba..dd935a79d3 100644
> --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> @@ -1,7 +1,7 @@
> /** @file
> MP initialize support functions for DXE phase.
>
> - Copyright (c) 2016 - 2020, Intel Corporation. All rights
> reserved.<BR>
> + Copyright (c) 2016 - 2023, Intel Corporation. All rights
> + reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -378,32 +378,44 @@ RelocateApLoop (
> IN OUT VOID *Buffer
> )
> {
> - CPU_MP_DATA *CpuMpData;
> - BOOLEAN MwaitSupport;
> - ASM_RELOCATE_AP_LOOP AsmRelocateApLoopFunc;
> - UINTN ProcessorNumber;
> - UINTN StackStart;
> + CPU_MP_DATA *CpuMpData;
> + BOOLEAN MwaitSupport;
> + ASM_RELOCATE_AP_LOOP AsmRelocateApLoopFunc;
> + ASM_RELOCATE_AP_LOOP_AMDSEV AsmRelocateApLoopFuncAmdSev;
> + UINTN ProcessorNumber;
> + UINTN StackStart;
>
> MpInitLibWhoAmI (&ProcessorNumber);
> CpuMpData = GetCpuMpData ();
> MwaitSupport = IsMwaitSupport ();
> if (CpuMpData->UseSevEsAPMethod) {
> - StackStart = CpuMpData->SevEsAPResetStackStart;
> + StackStart = CpuMpData->SevEsAPResetStackStart;
> + AsmRelocateApLoopFuncAmdSev =
> + (ASM_RELOCATE_AP_LOOP)(UINTN)mReservedApLoopFunc;
mReservedApLoopFuncAmdSev ?
> diff --git a/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm
> b/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm
> index c1e8a045a4..6b48913306 100644
> --- a/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm
> +++ b/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm
> @@ -347,12 +347,13 @@ PM16Mode:
>
> SwitchToRealProcEnd:
>
> ;---------------------------------------------------------------------
> ---------------- -; AsmRelocateApLoop (MwaitSupport, ApTargetCState,
> PmCodeSegment, TopOfApStack, CountTofinish, Pm16CodeSegment,
> SevEsAPJumpTable, WakeupBuffer);
> +; AsmRelocateApLoopAmdSev (MwaitSupport, ApTargetCState,
> +PmCodeSegment, TopOfApStack, CountTofinish, Pm16CodeSegment,
> +SevEsAPJumpTable, WakeupBuffer);
>
> ;---------------------------------------------------------------------
> ----------------
> -AsmRelocateApLoopStart:
> +
> +AsmRelocateApLoopStartAmdSev:
I'd suggest to do the rename in patch #1 too.
> +;--------------------------------------------------------------------
> +----------------- ; AsmRelocateApLoop (MwaitSupport, ApTargetCState,
> +PmCodeSegment, TopOfApStack, CountTofinish, Pm16CodeSegment,
> +SevEsAPJumpTable, WakeupBuffer);
> +;--------------------------------------------------------------------
> +-----------------
> +AsmRelocateApLoopStart:
> +BITS 64
> + cmp qword [rsp + 56], 0 ; SevEsAPJumpTable
> + je NoSevEs
Now you are adding back the AmdSev version.
It should be the generic version though.
If you want add the generic version later in the in the patch series (when
changing the function prototype to drop sev support and add paging
support) you can temporary call AsmRelocateApLoopStartAmdSev in the generic
code path too.
take care,
Gerd
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#100490): https://edk2.groups.io/g/devel/message/100490
Mute This Topic: https://groups.io/mt/97188907/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
