Reviewed-by: Ray Ni <ray...@intel.com> > -----Original Message----- > From: Wu, Jiaxin <jiaxin...@intel.com> > Sent: Thursday, February 16, 2023 10:47 AM > To: devel@edk2.groups.io > Cc: Dong, Eric <eric.d...@intel.com>; Ni, Ray <ray...@intel.com>; Zeng, Star > <star.z...@intel.com>; Laszlo Ersek <ler...@redhat.com>; Gerd Hoffmann > <kra...@redhat.com>; Kumar, Rahul R <rahul.r.ku...@intel.com> > Subject: [PATCH v8 4/6] UefiCpuPkg/PiSmmCpuDxeSmm: Consume SMM > Base Hob for SmBase info > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4337 > > Existing SMBASE Relocation is in the PiSmmCpuDxeSmm driver, which > will relocate the SMBASE of each processor by setting the SMBASE > field in the saved state map (at offset 7EF8h) to a new value. > The RSM instruction reloads the internal SMBASE register with the > value in SMBASE field when each time it exits SMM. All subsequent > SMI requests will use the new SMBASE to find the starting address > for the SMI handler (at SMBASE + 8000h). > > Due to the default SMBASE for all x86 processors is 0x30000, the > APs' 1st SMI for rebase has to be executed one by one to avoid > the CPUs over-writing each other's SMM Save State Area (see > existing SmmRelocateBases() function), which means the next AP has > to wait for the previous AP to finish its 1st SMI, then it can call > into its 1st SMI for rebase via Smi Ipi command, thus leading the > existing SMBASE Relocation has to be running in series. Besides, it > needs very complex code to handle the AP exit semaphore > (mRebased[Index]), which will hook return address of SMM Save State > so that semaphore code can be executed immediately after AP exits > SMM for SMBASE relocation (see existing SemaphoreHook() function). > > With SMM Base Hob support, PiSmmCpuDxeSmm does not need the RSM > instruction to do the SMBASE Relocation. SMBASE Register for each > processors have already been programmed and all SMBASE address have > recorded in SMM Base Hob. So the same default SMBASE Address > (0x30000) will not be used, thus the CPUs over-writing each other's > SMM Save State Area will not happen in PiSmmCpuDxeSmm driver. This > way makes the first SMI init can be executed in parallel and save > boot time on multi-core system. Besides, Semaphore Hook code logic > is also not required, which will greatly simplify the SMBASE > Relocation flow. > > Mainly changes as below: > * Assume the biggest possibility of tile size is 8k. > * Combine 2 SMIs (gcSmmInitTemplate & gcSmiHandlerTemplate) into one > (gcSmiHandlerTemplate), the new SMI handler needs to run to 2 paths: > one to SmmCpuFeaturesInitializeProcessor(), the other to SMM Core > Entry Point. > * Issue SMI IPI (All Excluding Self SMM IPI + BSP SMM IPI) for first > SMI init before normal SMI sources happen. > * Call SmmCpuFeaturesInitializeProcessor() in parallel. > > Cc: Eric Dong <eric.d...@intel.com> > Cc: Ray Ni <ray...@intel.com> > Cc: Zeng Star <star.z...@intel.com> > Cc: Laszlo Ersek <ler...@redhat.com> > Cc: Gerd Hoffmann <kra...@redhat.com> > Cc: Rahul Kumar <rahul1.ku...@intel.com> > Signed-off-by: Jiaxin Wu <jiaxin...@intel.com> > --- > UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 31 ++++- > UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 25 +++- > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 166 > ++++++++++++++++++++++----- > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 26 ++++- > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 3 +- > 5 files changed, 214 insertions(+), 37 deletions(-) > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c > b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c > index fb4a44eab6..d408b3f9f7 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c > @@ -1,9 +1,9 @@ > /** @file > Code for Processor S3 restoration > > -Copyright (c) 2006 - 2022, Intel Corporation. All rights reserved.<BR> > +Copyright (c) 2006 - 2023, Intel Corporation. All rights reserved.<BR> > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > > #include "PiSmmCpuDxeSmm.h" > @@ -822,13 +822,38 @@ SmmRestoreCpu ( > // > InitializeCpuBeforeRebase (); > } > > // > - // Restore SMBASE for BSP and all APs > + // Make sure the gSmmBaseHobGuid existence status is the same > between normal and S3 boot. > // > - SmmRelocateBases (); > + ASSERT (mSmmRelocated == (BOOLEAN)(GetFirstGuidHob > (&gSmmBaseHobGuid) != NULL)); > + if (mSmmRelocated != (BOOLEAN)(GetFirstGuidHob > (&gSmmBaseHobGuid) != NULL)) { > + DEBUG (( > + DEBUG_ERROR, > + "gSmmBaseHobGuid %a produced in normal boot but %a in S3 boot!", > + mSmmRelocated ? "is" : "is not", > + mSmmRelocated ? "is not" : "is" > + )); > + CpuDeadLoop (); > + } > + > + // > + // Check whether Smm Relocation is done or not. > + // If not, will do the SmmBases Relocation here!!! > + // > + if (!mSmmRelocated) { > + // > + // Restore SMBASE for BSP and all APs > + // > + SmmRelocateBases (); > + } else { > + // > + // Issue SMI IPI (All Excluding Self SMM IPI + BSP SMM IPI) to execute > first SMI init. > + // > + ExecuteFirstSmiInit (); > + } > > // > // Skip initialization if mAcpiCpuData is not valid > // > if (mAcpiCpuData.NumberOfCpus > 0) { > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > index a0967eb69c..baf827cf9d 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > @@ -1,9 +1,9 @@ > /** @file > SMM MP service implementation > > -Copyright (c) 2009 - 2022, Intel Corporation. All rights reserved.<BR> > +Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR> > Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > @@ -1721,17 +1721,40 @@ SmiRendezvous ( > UINTN Index; > UINTN Cr2; > > ASSERT (CpuIndex < mMaxNumberOfCpus); > > + if (mSmmRelocated) { > + ASSERT (mSmmInitialized != NULL); > + } > + > // > // Save Cr2 because Page Fault exception in SMM may override its value, > // when using on-demand paging for above 4G memory. > // > Cr2 = 0; > SaveCr2 (&Cr2); > > + if (mSmmRelocated && !mSmmInitialized[CpuIndex]) { > + // > + // Perform SmmInitHandler for CpuIndex > + // > + SmmInitHandler (); > + > + // > + // Restore Cr2 > + // > + RestoreCr2 (Cr2); > + > + // > + // Mark the first SMI init for CpuIndex has been done so as to avoid the > reentry. > + // > + mSmmInitialized[CpuIndex] = TRUE; > + > + return; > + } > + > // > // Call the user register Startup function first. > // > if (mSmmMpSyncData->StartupProcedure != NULL) { > mSmmMpSyncData->StartupProcedure (mSmmMpSyncData- > >StartupProcArgs); > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c > b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c > index 6e795d1756..d7a51d7e80 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c > @@ -82,10 +82,12 @@ EDKII_SMM_MEMORY_ATTRIBUTE_PROTOCOL > mSmmMemoryAttribute = { > EdkiiSmmClearMemoryAttributes > }; > > EFI_CPU_INTERRUPT_HANDLER > mExternalVectorTable[EXCEPTION_VECTOR_NUMBER]; > > +BOOLEAN mSmmRelocated = FALSE; > +volatile BOOLEAN *mSmmInitialized = NULL; > UINT32 mBspApicId = 0; > > // > // SMM stack information > // > @@ -381,22 +383,69 @@ SmmInitHandler ( > // Initialize private data during S3 resume > // > InitializeMpSyncData (); > } > > - // > - // Hook return after RSM to set SMM re-based flag > - // > - SemaphoreHook (Index, &mRebased[Index]); > + if (!mSmmRelocated) { > + // > + // Hook return after RSM to set SMM re-based flag > + // > + SemaphoreHook (Index, &mRebased[Index]); > + } > > return; > } > } > > ASSERT (FALSE); > } > > +/** > + Issue SMI IPI (All Excluding Self SMM IPI + BSP SMM IPI) to execute first > SMI init. > + > +**/ > +VOID > +ExecuteFirstSmiInit ( > + VOID > + ) > +{ > + UINTN Index; > + > + if (mSmmInitialized == NULL) { > + mSmmInitialized = (BOOLEAN *)AllocatePool (sizeof (BOOLEAN) * > mMaxNumberOfCpus); > + } > + > + ASSERT (mSmmInitialized != NULL); > + if (mSmmInitialized == NULL) { > + return; > + } > + > + // > + // Reset the mSmmInitialized to false. > + // > + ZeroMem ((VOID *)mSmmInitialized, sizeof (BOOLEAN) * > mMaxNumberOfCpus); > + > + // > + // Get the BSP ApicId. > + // > + mBspApicId = GetApicId (); > + > + // > + // Issue SMI IPI (All Excluding Self SMM IPI + BSP SMM IPI) for SMM init > + // > + SendSmiIpi (mBspApicId); > + SendSmiIpiAllExcludingSelf (); > + > + // > + // Wait for all processors to finish its 1st SMI > + // > + for (Index = 0; Index < mNumberOfCpus; Index++) { > + while (!(BOOLEAN)mSmmInitialized[Index]) { > + } > + } > +} > + > /** > Relocate SmmBases for each processor. > > Execute on first boot and all S3 resumes > > @@ -560,10 +609,15 @@ PiCpuSmmEntry ( > UINT32 RegEcx; > UINT32 RegEdx; > UINTN FamilyId; > UINTN ModelId; > UINT32 Cr3; > + EFI_HOB_GUID_TYPE *GuidHob; > + SMM_BASE_HOB_DATA *SmmBaseHobData; > + > + GuidHob = NULL; > + SmmBaseHobData = NULL; > > // > // Initialize address fixup > // > PiSmmCpuSmmInitFixupAddress (); > @@ -788,30 +842,58 @@ PiCpuSmmEntry ( > // context must be reduced. > // > ASSERT (TileSize <= (SMRAM_SAVE_STATE_MAP_OFFSET + sizeof > (SMRAM_SAVE_STATE_MAP) - SMM_HANDLER_OFFSET)); > > // > - // Allocate buffer for all of the tiles. > - // > - // Intel(R) 64 and IA-32 Architectures Software Developer's Manual > - // Volume 3C, Section 34.11 SMBASE Relocation > - // For Pentium and Intel486 processors, the SMBASE values must be > - // aligned on a 32-KByte boundary or the processor will enter shutdown > - // state during the execution of a RSM instruction. > - // > - // Intel486 processors: FamilyId is 4 > - // Pentium processors : FamilyId is 5 > + // Retrive the allocated SmmBase from gSmmBaseHobGuid. If found, > + // means the SmBase relocation has been done. > // > - BufferPages = EFI_SIZE_TO_PAGES (SIZE_32KB + TileSize * > (mMaxNumberOfCpus - 1)); > - if ((FamilyId == 4) || (FamilyId == 5)) { > - Buffer = AllocateAlignedCodePages (BufferPages, SIZE_32KB); > + GuidHob = GetFirstGuidHob (&gSmmBaseHobGuid); > + if (GuidHob != NULL) { > + // > + // Check whether the Required TileSize is enough. > + // > + if (TileSize > SIZE_8KB) { > + DEBUG ((DEBUG_ERROR, "The Range of Smbase in SMRAM is not > enough -- Required TileSize = 0x%08x, Actual TileSize = 0x%08x\n", TileSize, > SIZE_8KB)); > + CpuDeadLoop (); > + 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->CpuIndex == 0); > + mSmmRelocated = TRUE; > } else { > - Buffer = AllocateAlignedCodePages (BufferPages, SIZE_4KB); > - } > + // > + // When the HOB doesn't exist, allocate new SMBASE itself. > + // > + DEBUG ((DEBUG_INFO, "PiCpuSmmEntry: gSmmBaseHobGuid not > found!\n")); > + // > + // Allocate buffer for all of the tiles. > + // > + // Intel(R) 64 and IA-32 Architectures Software Developer's Manual > + // Volume 3C, Section 34.11 SMBASE Relocation > + // For Pentium and Intel486 processors, the SMBASE values must be > + // aligned on a 32-KByte boundary or the processor will enter shutdown > + // state during the execution of a RSM instruction. > + // > + // Intel486 processors: FamilyId is 4 > + // Pentium processors : FamilyId is 5 > + // > + BufferPages = EFI_SIZE_TO_PAGES (SIZE_32KB + TileSize * > (mMaxNumberOfCpus - 1)); > + if ((FamilyId == 4) || (FamilyId == 5)) { > + Buffer = AllocateAlignedCodePages (BufferPages, SIZE_32KB); > + } else { > + Buffer = AllocateAlignedCodePages (BufferPages, SIZE_4KB); > + } > > - ASSERT (Buffer != NULL); > - DEBUG ((DEBUG_INFO, "SMRAM SaveState Buffer (0x%08x, 0x%08x)\n", > Buffer, EFI_PAGES_TO_SIZE (BufferPages))); > + ASSERT (Buffer != NULL); > + DEBUG ((DEBUG_INFO, "New Allcoated SMRAM SaveState Buffer > (0x%08x, 0x%08x)\n", Buffer, EFI_PAGES_TO_SIZE (BufferPages))); > + } > > // > // Allocate buffer for pointers to array in SMM_CPU_PRIVATE_DATA. > // > gSmmCpuPrivate->ProcessorInfo = (EFI_PROCESSOR_INFORMATION > *)AllocatePool (sizeof (EFI_PROCESSOR_INFORMATION) * > mMaxNumberOfCpus); > @@ -842,11 +924,12 @@ PiCpuSmmEntry ( > // Retrieve APIC ID of each enabled processor from the MP Services > protocol. > // Also compute the SMBASE address, CPU Save State address, and CPU > Save state > // size for each CPU in the platform > // > for (Index = 0; Index < mMaxNumberOfCpus; Index++) { > - mCpuHotPlugData.SmBase[Index] = (UINTN)Buffer + Index * > TileSize > - SMM_HANDLER_OFFSET; > + mCpuHotPlugData.SmBase[Index] = mSmmRelocated ? > (UINTN)SmmBaseHobData->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); > gSmmCpuPrivate->Operation[Index] = SmmCpuNone; > > if (Index < mNumberOfCpus) { > @@ -955,21 +1038,27 @@ PiCpuSmmEntry ( > // Initialize IDT > // > InitializeSmmIdt (); > > // > - // Relocate SMM Base addresses to the ones allocated from SMRAM > + // Check whether Smm Relocation is done or not. > + // If not, will do the SmmBases Relocation here!!! > // > - mRebased = (BOOLEAN *)AllocateZeroPool (sizeof (BOOLEAN) * > mMaxNumberOfCpus); > - ASSERT (mRebased != NULL); > - SmmRelocateBases (); > + if (!mSmmRelocated) { > + // > + // Relocate SMM Base addresses to the ones allocated from SMRAM > + // > + mRebased = (BOOLEAN *)AllocateZeroPool (sizeof (BOOLEAN) * > mMaxNumberOfCpus); > + ASSERT (mRebased != NULL); > + SmmRelocateBases (); > > - // > - // Call hook for BSP to perform extra actions in normal mode after all > - // SMM base addresses have been relocated on all CPUs > - // > - SmmCpuFeaturesSmmRelocationComplete (); > + // > + // Call hook for BSP to perform extra actions in normal mode after all > + // SMM base addresses have been relocated on all CPUs > + // > + SmmCpuFeaturesSmmRelocationComplete (); > + } > > DEBUG ((DEBUG_INFO, "mXdSupported - 0x%x\n", mXdSupported)); > > // > // SMM Time initialization > @@ -996,10 +1085,25 @@ PiCpuSmmEntry ( > ); > } > } > } > > + // > + // For relocated SMBASE, some MSRs & CSRs are still required to be > configured in SMM Mode for SMM Initialization. > + // Those MSRs & CSRs must be configured before normal SMI sources > happen. > + // So, here is to issue SMI IPI (All Excluding Self SMM IPI + BSP SMM > IPI) to > execute first SMI init. > + // > + if (mSmmRelocated) { > + ExecuteFirstSmiInit (); > + > + // > + // Call hook for BSP to perform extra actions in normal mode after all > + // SMM base addresses have been relocated on all CPUs > + // > + SmmCpuFeaturesSmmRelocationComplete (); > + } > + > // > // Fill in SMM Reserved Regions > // > gSmmCpuPrivate->SmmReservedSmramRegion[0].SmramReservedStart = > 0; > gSmmCpuPrivate->SmmReservedSmramRegion[0].SmramReservedSize = > 0; > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h > b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h > index 5f0a38e400..c3731f174b 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h > @@ -1,9 +1,9 @@ > /** @file > Agent Module to load other modules to deploy SMM Entry Vector for X86 > CPU. > > -Copyright (c) 2009 - 2022, Intel Corporation. All rights reserved.<BR> > +Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR> > Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > @@ -23,10 +23,11 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > #include <Protocol/MmMp.h> > > #include <Guid/AcpiS3Context.h> > #include <Guid/MemoryAttributesTable.h> > #include <Guid/PiSmmMemoryAttributesTable.h> > +#include <Guid/SmmBaseHob.h> > > #include <Library/BaseLib.h> > #include <Library/IoLib.h> > #include <Library/TimerLib.h> > #include <Library/SynchronizationLib.h> > @@ -346,10 +347,29 @@ SmmWriteSaveState ( > IN EFI_SMM_SAVE_STATE_REGISTER Register, > IN UINTN CpuIndex, > IN CONST VOID *Buffer > ); > > +/** > + C function for SMI handler. To change all processor's SMMBase Register. > + > +**/ > +VOID > +EFIAPI > +SmmInitHandler ( > + VOID > + ); > + > +/** > + Issue SMI IPI (All Excluding Self SMM IPI + BSP SMM IPI) to execute first > SMI init. > + > +**/ > +VOID > +ExecuteFirstSmiInit ( > + VOID > + ); > + > /** > Read a CPU Save State register on the target processor. > > This function abstracts the differences that whether the CPU Save State > register is in the > IA32 CPU Save State Map or X64 CPU Save State Map. > @@ -400,10 +420,14 @@ WriteSaveStateRegister ( > IN EFI_SMM_SAVE_STATE_REGISTER Register, > IN UINTN Width, > IN CONST VOID *Buffer > ); > > +extern BOOLEAN mSmmRelocated; > +extern volatile BOOLEAN *mSmmInitialized; > +extern UINT32 mBspApicId; > + > extern CONST UINT8 gcSmmInitTemplate[]; > extern CONST UINT16 gcSmmInitSize; > X86_ASSEMBLY_PATCH_LABEL gPatchSmmCr0; > extern UINT32 mSmmCr0; > X86_ASSEMBLY_PATCH_LABEL gPatchSmmCr3; > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf > b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf > index b4b327f60c..9bfa8c1a76 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf > @@ -2,11 +2,11 @@ > # CPU SMM driver. > # > # This SMM driver performs SMM initialization, deploy SMM Entry Vector, > # provides CPU specific services in SMM. > # > -# Copyright (c) 2009 - 2022, Intel Corporation. All rights reserved.<BR> > +# Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR> > # Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > # > # SPDX-License-Identifier: BSD-2-Clause-Patent > # > ## > @@ -112,10 +112,11 @@ > > [Guids] > gEfiAcpiVariableGuid ## SOMETIMES_CONSUMES ## HOB # it > is > used for S3 boot. > gEdkiiPiSmmMemoryAttributesTableGuid ## CONSUMES ## SystemTable > gEfiMemoryAttributesTableGuid ## CONSUMES ## SystemTable > + gSmmBaseHobGuid ## CONSUMES > > [FeaturePcd] > gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmDebug ## > CONSUMES > gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmBlockStartupThisAp ## > CONSUMES > gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection ## > CONSUMES > -- > 2.16.2.windows.1
-=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#100270): https://edk2.groups.io/g/devel/message/100270 Mute This Topic: https://groups.io/mt/96998934/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
