Re: [PATCH V9 14/47] UefiCpuPkg: Enable Tdx support in MpInitLib


Min Xu
 

On March 23, 2022 3:20 PM, Ni Ray wrote:
Min,
MpInitLib contains following APIs. Let's discuss one by one.

MpInitLibInitialize: Directly return. Looks good to me.
MpInitLibGetNumberOfProcessors : Call TdxMpInitLibGetNumberOfProcessors().
Why call a subfunction here but directly return in above API? Not quite
consistent to me. By the way, can you re-use the parameter check logic?
My consideration is that if the complexity of code change is small, then it will return directly. Otherwise a sub-function will be called.
As to the parameter check logic, it will be reused. Thanks for reminder.

MpInitLibGetProcessorInfo : Call TdxMpInitLibGetProcessorInfo (). Consistency
concern.
Same as above.

MpInitLibStartupAllAPs : Directly return in DXE version. But
StartupAllCPUsWorker() directly returns AGAIN! My guess that's for PEI version.
Can you remove the changes in DXE version?
I double checked the code of MpInitLibStartupAllAPs (both DXE and PEI version). It calls StartupAllCPUsWorker which returns directly in Td guest.

MpInitLibStartupThisAP : Directly return in DXE version. How is the PEI version
handled?
It is missed in PEI version. StartupThisAPWorker will be updated to return directly in Td guest.

MpInitLibSwitchBSP : Directly return in DXE version. How is the PEI version
handled?
Sorry it is missed in PEI version. It will be added in the next version.

MpInitLibEnableDisableAP : Directly return in DXE version. But
EnableDisableApWorker () directly returns AGAIN! My guess that's for PEI
version. Can you remove the changes in DXE version?
Thanks for reminder. The changes in DXE version will be removed.

MpInitLibWhoAmI : Directly return. Looks good to me.
MpInitLibStartupAllCPUs : Not handled. Missed?
It is not missed. MpInitLibStartupAllCPUs call StartupAllCPUsWorker which returns directly in Td guest.

Thanks
Min

Join devel@edk2.groups.io to automatically receive all group messages.