On 2/21/24 11:24, Gerd Hoffmann wrote:
> On Wed, Feb 21, 2024 at 03:48:25AM +0000, Ni, Ray wrote:
>>>
>>> + MaxCpusPerHob = (MAX_UINT16 - sizeof (EFI_HOB_GUID_TYPE) - sizeof
>>> (MP_HAND_OFF)) / sizeof (PROCESSOR_HAND_OFF);
>>
>> Above formula assumes the maximum HOB length could be 0xFFFF.
>
> Which is IMHO correct.
>
>> But actually the maximum HOB length could be only 0xFFF8 because
>> PeiCore::PeiCreateHob() contains following logic:
>>
>> if (0x10000 - Length <= 0x7) {
>> return EFI_INVALID_PARAMETER;
>> }
>
> That Length is the *data* size, the HOB header is not included.
>
> The "- sizeof (EFI_HOB_GUID_TYPE)" in the formula above accounts the
> space needed for HOB header and GUID.
>From the patch:
MaxCpusPerHob = (MAX_UINT16 - sizeof (EFI_HOB_GUID_TYPE) - sizeof
(MP_HAND_OFF)) / sizeof (PROCESSOR_HAND_OFF);
...
CpusInHob = MIN (CpuMpData->CpuCount - HobBase, MaxCpusPerHob);
MpHandOffSize = sizeof (MP_HAND_OFF) + sizeof (PROCESSOR_HAND_OFF) *
CpusInHob;
MpHandOff = (MP_HAND_OFF *)BuildGuidHob (&mMpHandOffGuid, MpHandOffSize);
Assuming that the division is exact, and that the MIN selects MaxCpusPerHob for
CpusInHob, we get:
MpHandOffSize = sizeof (MP_HAND_OFF) + sizeof (PROCESSOR_HAND_OFF) *
(MAX_UINT16 - sizeof (EFI_HOB_GUID_TYPE) - sizeof (MP_HAND_OFF)) / sizeof
(PROCESSOR_HAND_OFF);
the multiplication and the division cancel out (again, assuming exact division):
MpHandOffSize = sizeof (MP_HAND_OFF) + (MAX_UINT16 - sizeof
(EFI_HOB_GUID_TYPE) - sizeof (MP_HAND_OFF));
the sizeof (MP_HAND_OFF) addition and subtraction cancel out:
MpHandOffSize = MAX_UINT16 - sizeof (EFI_HOB_GUID_TYPE);
then we get:
MpHandOff = (MP_HAND_OFF *)BuildGuidHob (&mMpHandOffGuid, MAX_UINT16 -
sizeof (EFI_HOB_GUID_TYPE));
Inside BuildGuidHob() [MdePkg/Library/PeiHobLib/HobLib.c], we get
DataLength = MAX_UINT16 - sizeof (EFI_HOB_GUID_TYPE);
and further
ASSERT (DataLength <= (0xFFF8 - sizeof (EFI_HOB_GUID_TYPE)));
Substituting for DataLength,
ASSERT (MAX_UINT16 - sizeof (EFI_HOB_GUID_TYPE) <= (0xFFF8 - sizeof
(EFI_HOB_GUID_TYPE)));
which is
ASSERT (MAX_UINT16 <= 0xFFF8);
which fails.
So, as Ray says, and as I wrote under v1, you need to use 0xFFF8 here, not
MAX_UINT16.
... Under v1, I wrote
CpusPerHob = (0xFFE0 - sizeof (MP_HAND_OFF)) / sizeof (PROCESSOR_HAND_OFF);
If you want to make "sizeof (EFI_HOB_GUID_TYPE)" -- 24 bytes -- explicit in
that subtraction, that is fine, but then we just get back to 0xFFF8:
CpusPerHob = (0xFFF8 - sizeof (EFI_HOB_GUID_TYPE) - sizeof (MP_HAND_OFF)) /
sizeof (PROCESSOR_HAND_OFF);
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115766): https://edk2.groups.io/g/devel/message/115766
Mute This Topic: https://groups.io/mt/104472313/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
