Reviewed-by: Ray Ni <[email protected]> > -----Original Message----- > From: Lou, Yun <[email protected]> > Sent: Thursday, September 16, 2021 5:27 PM > To: [email protected] > Cc: Lou, Yun <[email protected]>; Ni, Ray <[email protected]>; Dong, Eric > <[email protected]>; Laszlo Ersek > <[email protected]>; Kumar, Rahul1 <[email protected]> > Subject: [PATCH v2 2/2] UefiCpuPkg: Prevent from re-initializing CPU features > during S3 resume > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3621 > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3631 > > Current CPU feature initialization design: > During normal boot, CpuFeaturesPei module (inside FSP) initializes the > CPU features. During S3 boot, CpuFeaturesPei module does nothing, and > CpuSmm driver (in SMRAM) initializes CPU features instead. > > This code change prevents CpuSmm driver from re-initializing CPU > features during S3 resume if CpuFeaturesPei module has done the same > initialization. > > In addition, EDK2 contains DxeIpl PEIM that calls S3RestoreConfig2 PPI > during S3 boot and this PPI eventually calls CpuSmm driver (in SMRAM) to > initialize the CPU features, so "EDK2 + FSP" does not have the CPU > feature initialization issue during S3 boot. But "coreboot" does not > contain DxeIpl PEIM and the issue appears, unless > "PcdCpuFeaturesInitOnS3Resume" is set to TRUE. > > Signed-off-by: Jason Lou <[email protected]> > Cc: Ray Ni <[email protected]> > Cc: Eric Dong <[email protected]> > Cc: Laszlo Ersek <[email protected]> > Cc: Rahul Kumar <[email protected]> > --- > UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 34 ++++++++++++-------- > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 3 +- > 2 files changed, 23 insertions(+), 14 deletions(-) > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c > b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c > index 2873cba083..2496abb392 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c > @@ -1152,23 +1152,31 @@ GetAcpiCpuData ( > mAcpiCpuData.ApMachineCheckHandlerBase = > (EFI_PHYSICAL_ADDRESS)(UINTN)MachineCheckHandlerForAp; > > > > ZeroMem (&mAcpiCpuData.CpuFeatureInitData, sizeof (CPU_FEATURE_INIT_DATA)); > > - CopyCpuFeatureInitDatatoSmram (&mAcpiCpuData.CpuFeatureInitData, > &AcpiCpuData->CpuFeatureInitData); > > > > - CpuStatus = &mAcpiCpuData.CpuFeatureInitData.CpuStatus; > > + if (!PcdGetBool (PcdCpuFeaturesInitOnS3Resume)) { > > + // > > + // If the CPU features will not be initialized by CpuFeaturesPei module > during > > + // next ACPI S3 resume, copy the CPU features initialization data into > SMRAM, > > + // which will be consumed in SmmRestoreCpu during next S3 resume. > > + // > > + CopyCpuFeatureInitDatatoSmram (&mAcpiCpuData.CpuFeatureInitData, > &AcpiCpuData->CpuFeatureInitData); > > > > - mCpuFlags.CoreSemaphoreCount = AllocateZeroPool ( > > - sizeof (UINT32) * CpuStatus->PackageCount > * > > - CpuStatus->MaxCoreCount * > CpuStatus->MaxThreadCount > > - ); > > - ASSERT (mCpuFlags.CoreSemaphoreCount != NULL); > > + CpuStatus = &mAcpiCpuData.CpuFeatureInitData.CpuStatus; > > > > - mCpuFlags.PackageSemaphoreCount = AllocateZeroPool ( > > - sizeof (UINT32) * > CpuStatus->PackageCount * > > - CpuStatus->MaxCoreCount * > CpuStatus->MaxThreadCount > > - ); > > - ASSERT (mCpuFlags.PackageSemaphoreCount != NULL); > > + mCpuFlags.CoreSemaphoreCount = AllocateZeroPool ( > > + sizeof (UINT32) * > CpuStatus->PackageCount * > > + CpuStatus->MaxCoreCount * > CpuStatus->MaxThreadCount > > + ); > > + ASSERT (mCpuFlags.CoreSemaphoreCount != NULL); > > > > - InitializeSpinLock((SPIN_LOCK*) &mCpuFlags.MemoryMappedLock); > > + mCpuFlags.PackageSemaphoreCount = AllocateZeroPool ( > > + sizeof (UINT32) * > CpuStatus->PackageCount * > > + CpuStatus->MaxCoreCount * > CpuStatus->MaxThreadCount > > + ); > > + ASSERT (mCpuFlags.PackageSemaphoreCount != NULL); > > + > > + InitializeSpinLock((SPIN_LOCK*) &mCpuFlags.MemoryMappedLock); > > + } > > } > > > > /** > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf > b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf > index 76b1462996..0e88071c70 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf > @@ -4,7 +4,7 @@ > # This SMM driver performs SMM initialization, deploy SMM Entry Vector, > > # provides CPU specific services in SMM. > > # > > -# Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.<BR> > > +# Copyright (c) 2009 - 2021, Intel Corporation. All rights reserved.<BR> > > # Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > # > > # SPDX-License-Identifier: BSD-2-Clause-Patent > > @@ -134,6 +134,7 @@ > gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmCodeAccessCheckEnable ## > CONSUMES > > gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode ## > CONSUMES > > gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmShadowStackSize ## > SOMETIMES_CONSUMES > > + gUefiCpuPkgTokenSpaceGuid.PcdCpuFeaturesInitOnS3Resume ## > CONSUMES > > gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable ## > CONSUMES > > gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask ## > CONSUMES > > gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask ## > CONSUMES > > -- > 2.28.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#80806): https://edk2.groups.io/g/devel/message/80806 Mute This Topic: https://groups.io/mt/85647864/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
