Reviewed-by: Ray Ni <ray...@intel.com>
Thanks,
Ray
> -----Original Message-----
> From: Tan, Dun <dun....@intel.com>
> Sent: Thursday, December 7, 2023 3:33 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.d...@intel.com>; Ni, Ray <ray...@intel.com>; Kumar,
> Rahul R <rahul.r.ku...@intel.com>; Gerd Hoffmann <kra...@redhat.com>
> Subject: [Patch V2 6/6] UefiCpuPkg: Avoid assuming only one smmbasehob
>
> Modify the gSmmBaseHobGuid consumption code to
> remove the asuumption that there is only one
> gSmmBaseHobGuid. If the CPU number is big enough,
> there will be more than one SmmBaseHob in the
> HOB list.
>
> Signed-off-by: Dun Tan <dun....@intel.com>
> Cc: Eric Dong <eric.d...@intel.com>
> Cc: Ray Ni <ray...@intel.com>
> Cc: Rahul Kumar <rahul1.ku...@intel.com>
> Cc: Gerd Hoffmann <kra...@redhat.com>
> ---
> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 179
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +++++++++++++++++++++++--------------------------------
> 1 file changed, 147 insertions(+), 32 deletions(-)
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> index b729f8ee63..fab2fed286 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> @@ -586,6 +586,132 @@ SmmReadyToLockEventNotify (
> return EFI_SUCCESS;
> }
>
> +/**
> + Function to compare 2 SMM_BASE_HOB_DATA pointer based on
> ProcessorIndex.
> +
> + @param[in] Buffer1 pointer to SMM_BASE_HOB_DATA
> poiner to compare
> + @param[in] Buffer2 pointer to second
> SMM_BASE_HOB_DATA pointer to compare
> +
> + @retval 0 Buffer1 equal to Buffer2
> + @retval <0 Buffer1 is less than Buffer2
> + @retval >0 Buffer1 is greater than Buffer2
> +**/
> +INTN
> +EFIAPI
> +SmBaseHobCompare (
> + IN CONST VOID *Buffer1,
> + IN CONST VOID *Buffer2
> + )
> +{
> + if ((*(SMM_BASE_HOB_DATA **)Buffer1)->ProcessorIndex >
> (*(SMM_BASE_HOB_DATA **)Buffer2)->ProcessorIndex) {
> + return 1;
> + } else if ((*(SMM_BASE_HOB_DATA **)Buffer1)->ProcessorIndex <
> (*(SMM_BASE_HOB_DATA **)Buffer2)->ProcessorIndex) {
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +/**
> + Extract SmBase for all CPU from SmmBase HOB.
> +
> + @param[in] MaxNumberOfCpus Max NumberOfCpus.
> +
> + @retval SmBaseBuffer Pointer to SmBase Buffer.
> + @retval NULL gSmmBaseHobGuid was not been
> created.
> +**/
> +UINTN *
> +GetSmBase (
> + IN UINTN MaxNumberOfCpus
> + )
> +{
> + UINTN HobCount;
> + EFI_HOB_GUID_TYPE *GuidHob;
> + SMM_BASE_HOB_DATA *SmmBaseHobData;
> + UINTN NumberOfProcessors;
> + SMM_BASE_HOB_DATA **SmBaseHobs;
> + UINTN *SmBaseBuffer;
> + UINTN HobIndex;
> + UINTN SortBuffer;
> + UINTN ProcessorIndex;
> + UINT64 PrevProcessorIndex;
> + EFI_HOB_GUID_TYPE *FirstSmmBaseGuidHob;
> +
> + SmmBaseHobData = NULL;
> + HobIndex = 0;
> + ProcessorIndex = 0;
> + HobCount = 0;
> + NumberOfProcessors = 0;
> +
> + FirstSmmBaseGuidHob = GetFirstGuidHob (&gSmmBaseHobGuid);
> + if (FirstSmmBaseGuidHob == NULL) {
> + return NULL;
> + }
> +
> + GuidHob = FirstSmmBaseGuidHob;
> + while (GuidHob != NULL) {
> + HobCount++;
> + SmmBaseHobData = GET_GUID_HOB_DATA (GuidHob);
> +
> + if (NumberOfProcessors >= MaxNumberOfCpus) {
> + break;
> + }
> +
> + NumberOfProcessors += SmmBaseHobData->NumberOfProcessors;
> + GuidHob = GetNextGuidHob (&gSmmBaseHobGuid,
> GET_NEXT_HOB (GuidHob));
> + }
> +
> + ASSERT (NumberOfProcessors == MaxNumberOfCpus);
> + if (NumberOfProcessors != MaxNumberOfCpus) {
> + CpuDeadLoop ();
> + }
> +
> + SmBaseHobs = AllocatePool (sizeof (SMM_BASE_HOB_DATA *) *
> HobCount);
> + ASSERT (SmBaseHobs != NULL);
> + if (SmBaseHobs == NULL) {
> + return NULL;
> + }
> +
> + //
> + // Record each SmmBaseHob pointer in the SmBaseHobs.
> + // The FirstSmmBaseGuidHob is to speed up this while-loop
> + // without needing to look for SmBaseHob from beginning.
> + //
> + GuidHob = FirstSmmBaseGuidHob;
> + while (HobIndex < HobCount) {
> + SmBaseHobs[HobIndex++] = GET_GUID_HOB_DATA (GuidHob);
> + GuidHob = GetNextGuidHob (&gSmmBaseHobGuid,
> GET_NEXT_HOB (GuidHob));
> + }
> +
> + SmBaseBuffer = (UINTN *)AllocatePool (sizeof (UINTN) *
> (MaxNumberOfCpus));
> + ASSERT (SmBaseBuffer != NULL);
> + if (SmBaseBuffer == NULL) {
> + FreePool (SmBaseHobs);
> + return NULL;
> + }
> +
> + QuickSort (SmBaseHobs, HobCount, sizeof (SMM_BASE_HOB_DATA *),
> (BASE_SORT_COMPARE)SmBaseHobCompare, &SortBuffer);
> + PrevProcessorIndex = 0;
> + for (HobIndex = 0; HobIndex < HobCount; HobIndex++) {
> + //
> + // Make sure no overlap and no gap in the CPU range covered by each
> HOB
> + //
> + ASSERT (SmBaseHobs[HobIndex]->ProcessorIndex ==
> PrevProcessorIndex);
> +
> + //
> + // Cache each SmBase in order.
> + //
> + for (ProcessorIndex = 0; ProcessorIndex <
> SmBaseHobs[HobIndex]->NumberOfProcessors; ProcessorIndex++) {
> + SmBaseBuffer[PrevProcessorIndex + ProcessorIndex] =
> (UINTN)SmBaseHobs[HobIndex]->SmBase[ProcessorIndex];
> + }
> +
> + PrevProcessorIndex += SmBaseHobs[HobIndex]->NumberOfProcessors;
> + }
> +
> + FreePool (SmBaseHobs);
> + return SmBaseBuffer;
> +}
> +
> /**
> Function to compare 2 MP_INFORMATION2_HOB_DATA pointer based on
> ProcessorIndex.
>
> @@ -744,27 +870,22 @@ PiCpuSmmEntry (
> IN EFI_SYSTEM_TABLE *SystemTable
> )
> {
> - EFI_STATUS Status;
> - UINTN Index;
> - VOID *Buffer;
> - UINTN BufferPages;
> - UINTN TileCodeSize;
> - UINTN TileDataSize;
> - UINTN TileSize;
> - UINT8 *Stacks;
> - VOID *Registration;
> - UINT32 RegEax;
> - UINT32 RegEbx;
> - UINT32 RegEcx;
> - UINT32 RegEdx;
> - UINTN FamilyId;
> - UINTN ModelId;
> - UINT32 Cr3;
> - EFI_HOB_GUID_TYPE *GuidHob;
> - SMM_BASE_HOB_DATA *SmmBaseHobData;
> -
> - GuidHob = NULL;
> - SmmBaseHobData = NULL;
> + EFI_STATUS Status;
> + UINTN Index;
> + VOID *Buffer;
> + UINTN BufferPages;
> + UINTN TileCodeSize;
> + UINTN TileDataSize;
> + UINTN TileSize;
> + UINT8 *Stacks;
> + VOID *Registration;
> + UINT32 RegEax;
> + UINT32 RegEbx;
> + UINT32 RegEcx;
> + UINT32 RegEdx;
> + UINTN FamilyId;
> + UINTN ModelId;
> + UINT32 Cr3;
>
> PERF_FUNCTION_BEGIN ();
>
> @@ -986,8 +1107,8 @@ PiCpuSmmEntry (
> // Retrive the allocated SmmBase from gSmmBaseHobGuid. If found,
> // means the SmBase relocation has been done.
> //
> - GuidHob = GetFirstGuidHob (&gSmmBaseHobGuid);
> - if (GuidHob != NULL) {
> + mCpuHotPlugData.SmBase = GetSmBase (mMaxNumberOfCpus);
> + if (mCpuHotPlugData.SmBase != NULL) {
> //
> // Check whether the Required TileSize is enough.
> //
> @@ -997,12 +1118,6 @@ PiCpuSmmEntry (
> return RETURN_BUFFER_TOO_SMALL;
> }
>
> - SmmBaseHobData = GET_GUID_HOB_DATA (GuidHob);
> -
> - //
> - // Assume single instance of HOB produced, expect the
> HOB.NumberOfProcessors equals to the mMaxNumberOfCpus.
> - //
> - ASSERT (SmmBaseHobData->NumberOfProcessors ==
> (UINT32)mMaxNumberOfCpus && SmmBaseHobData->ProcessorIndex == 0);
> mSmmRelocated = TRUE;
> } else {
> //
> @@ -1048,8 +1163,6 @@ PiCpuSmmEntry (
> //
> mCpuHotPlugData.ApicId = (UINT64 *)AllocatePool (sizeof (UINT64) *
> mMaxNumberOfCpus);
> ASSERT (mCpuHotPlugData.ApicId != NULL);
> - mCpuHotPlugData.SmBase = (UINTN *)AllocatePool (sizeof (UINTN) *
> mMaxNumberOfCpus);
> - ASSERT (mCpuHotPlugData.SmBase != NULL);
> mCpuHotPlugData.ArrayLength = (UINT32)mMaxNumberOfCpus;
>
> //
> @@ -1058,7 +1171,9 @@ PiCpuSmmEntry (
> // size for each CPU in the platform
> //
> for (Index = 0; Index < mMaxNumberOfCpus; Index++) {
> - mCpuHotPlugData.SmBase[Index] = mSmmRelocated ?
> (UINTN)SmmBaseHobData->SmBase[Index] : (UINTN)Buffer + Index *
> TileSize - SMM_HANDLER_OFFSET;
> + if (!mSmmRelocated) {
> + mCpuHotPlugData.SmBase[Index] = (UINTN)Buffer + Index * TileSize
> - SMM_HANDLER_OFFSET;
> + }
>
> gSmmCpuPrivate->CpuSaveStateSize[Index] = sizeof
> (SMRAM_SAVE_STATE_MAP);
> gSmmCpuPrivate->CpuSaveState[Index] = (VOID
> *)(mCpuHotPlugData.SmBase[Index] + SMRAM_SAVE_STATE_MAP_OFFSET);
> --
> 2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112220): https://edk2.groups.io/g/devel/message/112220
Mute This Topic: https://groups.io/mt/103030296/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
