Will change the commit based on the comments.
Thanks,
Dun
-----Original Message-----
From: Ni, Ray <ray...@intel.com>
Sent: Wednesday, December 6, 2023 5:24 PM
To: Tan, Dun <dun....@intel.com>; devel@edk2.groups.io
Cc: Dong, Eric <eric.d...@intel.com>; Kumar, Rahul R <rahul.r.ku...@intel.com>;
Gerd Hoffmann <kra...@redhat.com>
Subject: RE: [PATCH 2/6] UefiCpuPkg: Build MpInfo2HOB in CpuMpPei
4 minor comments:
> + DEBUG ((DEBUG_INFO, "BuildMpInformationHob\n"));
1. DEBUG ("Creating MpInformation2 HOB...\n")
> +
> + for (Index = 0; Index < NumberOfProcessorsInHob; Index++) {
> + MpInformation2Entry = GET_MP_INFORMATION_ENTRY
> (MpInformation2HobData, Index);
2. Since EntrySize equals to sizeof (MP_INFORMATION2_ENTRY), is it ok to just
use MpInformation2HobData->Entry[Index]?
3. Do you think "Entry[0]" is more proper than "MpInformation[0]"?
> + DEBUG ((
> + DEBUG_INFO,
> + "ProcessorIndex = %x, ProcessorId = %lx, StatusFlag = %x\n",
> + Index + ProcessorIndex,
> + MpInformation2Entry->ProcessorInfo.ProcessorId,
> + MpInformation2Entry->ProcessorInfo.StatusFlag
> + ));
4. How about the debug messages are as follows:
<space><space>Processor[0000]: ProcessorId = 0x00000000, StatusFlag =
0x00000001\n <space><space><space><space>Location = Package:0 Core:0 Thread:0\n
<space><space><space><space>Location2 = Package:0 Die:0 Tile:0 Module:0 Core:0
Thread:0\n If a number has "0x" prefix, it uses hex format, otherwise it uses
dec format. The debug message should clearly tell what format the number
follows.
Extra 2 spaces in Location/Location2 are to tell that these are extra info for
the processor #0.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112144): https://edk2.groups.io/g/devel/message/112144
Mute This Topic: https://groups.io/mt/102987138/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
