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.
Agree. I will do so. Thanks Yuanhao -----Original Message----- From: Gerd Hoffmann <[email protected]> Sent: Tuesday, February 21, 2023 5:23 PM To: Xie, Yuanhao <[email protected]> Cc: [email protected]; Dong, Guo <[email protected]>; Ni, Ray <[email protected]>; Rhodes, Sean <[email protected]>; Lu, James <[email protected]>; Guo, Gua <[email protected]>; Tom Lendacky <[email protected]>; Laszlo Ersek <[email protected]> Subject: Re: [Patch V2 1/5] UefiCpuPkg: Duplicate RelocateApLoop for the processors with SEV-ES. 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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#100454): https://edk2.groups.io/g/devel/message/100454 Mute This Topic: https://groups.io/mt/97081028/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
