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] -=-=-=-=-=-=-=-=-=-=-=-