1. The function name can be "GetMpInformation()" without mentioning
"FromMpInfo2Hob".
> + EFI_HOB_GUID_TYPE *GuidHob;
> + EFI_HOB_GUID_TYPE *FirstMpInfor2Hob;
2. "FirstMpInfo2Hob". Please remove "r".
>+ FirstMpInfor2Hob = GetFirstGuidHob (&gMpInformationHobGuid2);
3. Please update comments to explain "FirstMpInfo2Hob" is to speed up the 2nd
while-loop without needing to look for MpInfo2Hob from beginning.
> +
> + ASSERT (CpuCount <= PcdGet32 (PcdCpuMaxLogicalProcessorNumber));
> + *NumberOfCpus = CpuCount;
4. There is no "return" before "*NumberOfCpus" assignment. So, why not remove
"CpuCount" and directly udpates
"*NumberOfCpus" in the while-loop?
> +
> + MpInfomation2Buffer = AllocatePool (sizeof
> (MP_INFORMATION2_HOB_DATA *) * HobCount);
5. MpInfomation2Buffer -> MpInfo2Hobs
6. Can you move "PrevProcessorIndex" assignment just above the "for" loop?
> + for (Index = 0; Index < HobCount; Index++) {
7. Index -> HobIndex
> + CopyMem (
> + ProcessorInfo + PrevProcessorIndex + ProcessorIndex,
8. &ProcessorInfo[PrevProcessorIndex + ProcessorIndex]
> +
> + *ProcessorInfoPointer = ProcessorInfo;
9. If you let the function just return ProcessorInfo and NULL when failure
happens, will it simplify the code?
>
> +[Depex]
> + TRUE
> -[Depex]
> - gEfiMpServiceProtocolGuid
10. The depex change means that CpuSmm driver could run before CpuMp driver
runs. Have you verified if CpuSmm can start well even removing CpuMp DXE driver?
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112106): https://edk2.groups.io/g/devel/message/112106
Mute This Topic: https://groups.io/mt/102987139/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
