Re: [Patch V2 1/5] UefiCpuPkg: Duplicate RelocateApLoop for the processors with SEV-ES.

["Gerd Hoffmann" <kraxel@redhat.com>]

  Hi,

>    if (CpuMpData->UseSevEsAPMethod) {
> -    StackStart = CpuMpData->SevEsAPResetStackStart;
> +    StackStart                  = CpuMpData->SevEsAPResetStackStart;
> +    AsmRelocateApLoopFuncAmdSev = (ASM_RELOCATE_AP_LOOP)(UINTN)mReservedApLoopFunc;
> +    AsmRelocateApLoopFuncAmdSev (
> +      MwaitSupport,
> +      CpuMpData->ApTargetCState,
> +      CpuMpData->PmCodeSegment,
> +      StackStart - ProcessorNumber * AP_SAFE_STACK_SIZE,
> +      (UINTN)&mNumberToFinish,
> +      CpuMpData->Pm16CodeSegment,
> +      CpuMpData->SevEsAPBuffer,
> +      CpuMpData->WakeupBuffer
> +      );

Good.  Thanks for updating it.

> diff --git a/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm b/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm
> index 7c2469f9c5..6b48913306 100644
> --- a/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm
> +++ b/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm
> @@ -346,3 +346,172 @@ PM16Mode:
>      iret
>  
>  SwitchToRealProcEnd:
> +;-------------------------------------------------------------------------------------
> +;  AsmRelocateApLoopAmdSev (MwaitSupport, ApTargetCState, PmCodeSegment, TopOfApStack, CountTofinish, Pm16CodeSegment, SevEsAPJumpTable, WakeupBuffer);
> +;-------------------------------------------------------------------------------------
> +
> +AsmRelocateApLoopStartAmdSev:
> +BITS 64

Hmm, so here you are adding a renamed copy of the AP loop.
Then, in patch #5, you are rewriting the code at the old location.

I'd suggest to move the code instead of copying it, i.e. have one patch
moving (and renaming) the AmdSev AP loop.  Have another patch adding the
new code for the generic AP loop.  That should result in patches which
are easier to read, especially the new generic AP loop code is not mixed
with AmdSev code removal.

take care,
  Gerd