From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by mx.groups.io with SMTP id smtpd.web10.5613.1631757707564338450 for ; Wed, 15 Sep 2021 19:01:47 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.115, mailfrom: yun.lou@intel.com) X-IronPort-AV: E=McAfee;i="6200,9189,10108"; a="222124245" X-IronPort-AV: E=Sophos;i="5.85,297,1624345200"; d="scan'208";a="222124245" Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Sep 2021 19:01:46 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.85,297,1624345200"; d="scan'208";a="553627639" Received: from shwdeopenlab102.ccr.corp.intel.com ([10.239.183.74]) by fmsmga002.fm.intel.com with ESMTP; 15 Sep 2021 19:01:44 -0700 From: "Jason Lou" To: devel@edk2.groups.io Cc: Jason , Ray Ni , Eric Dong , Laszlo Ersek , Rahul Kumar Subject: [PATCH v1 2/2] UefiCpuPkg: Prevent from re-initializing CPU features during S3 resume Date: Thu, 16 Sep 2021 10:01:32 +0800 Message-Id: <20210916020132.3639-2-yun.lou@intel.com> X-Mailer: git-send-email 2.28.0.windows.1 In-Reply-To: <20210916020132.3639-1-yun.lou@intel.com> References: <20210916020132.3639-1-yun.lou@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D3621 REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D3631 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 Cc: Ray Ni Cc: Eric Dong Cc: Laszlo Ersek Cc: Rahul Kumar --- 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 1270992f38..fece75266d 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c @@ -1155,23 +1155,31 @@ GetAcpiCpuData ( mAcpiCpuData.ApMachineCheckHandlerBase =3D (EFI_PHYSICAL_ADDRESS)(UINTN)= MachineCheckHandlerForAp;=0D =0D ZeroMem (&mAcpiCpuData.CpuFeatureInitData, sizeof (CPU_FEATURE_INIT_DATA= ));=0D - CopyCpuFeatureInitDatatoSmram (&mAcpiCpuData.CpuFeatureInitData, &AcpiCp= uData->CpuFeatureInitData);=0D =0D - CpuStatus =3D &mAcpiCpuData.CpuFeatureInitData.CpuStatus;=0D + if (!PcdGetBool (PcdCpuFeaturesInitOnS3Resume)) {=0D + //=0D + // If the CPU features will not be initialized by CpuFeaturesPei modul= e during=0D + // next ACPI S3 resume, copy the CPU features initialization data into= SMRAM,=0D + // which will be consumed in SmmRestoreCpu during next S3 resume.=0D + //=0D + CopyCpuFeatureInitDatatoSmram (&mAcpiCpuData.CpuFeatureInitData, &Acpi= CpuData->CpuFeatureInitData);=0D =0D - mCpuFlags.CoreSemaphoreCount =3D AllocateZeroPool (=0D - sizeof (UINT32) * CpuStatus->PackageCou= nt *=0D - CpuStatus->MaxCoreCount * CpuStatus->Ma= xThreadCount=0D - );=0D - ASSERT (mCpuFlags.CoreSemaphoreCount !=3D NULL);=0D + CpuStatus =3D &mAcpiCpuData.CpuFeatureInitData.CpuStatus;=0D =0D - mCpuFlags.PackageSemaphoreCount =3D AllocateZeroPool (=0D - sizeof (UINT32) * CpuStatus->Package= Count *=0D - CpuStatus->MaxCoreCount * CpuStatus-= >MaxThreadCount=0D - );=0D - ASSERT (mCpuFlags.PackageSemaphoreCount !=3D NULL);=0D + mCpuFlags.CoreSemaphoreCount =3D AllocateZeroPool (=0D + sizeof (UINT32) * CpuStatus->PackageC= ount *=0D + CpuStatus->MaxCoreCount * CpuStatus->= MaxThreadCount=0D + );=0D + ASSERT (mCpuFlags.CoreSemaphoreCount !=3D NULL);=0D =0D - InitializeSpinLock((SPIN_LOCK*) &mCpuFlags.MemoryMappedLock);=0D + mCpuFlags.PackageSemaphoreCount =3D AllocateZeroPool (=0D + sizeof (UINT32) * CpuStatus->Packa= geCount *=0D + CpuStatus->MaxCoreCount * CpuStatu= s->MaxThreadCount=0D + );=0D + ASSERT (mCpuFlags.PackageSemaphoreCount !=3D NULL);=0D +=0D + InitializeSpinLock((SPIN_LOCK*) &mCpuFlags.MemoryMappedLock);=0D + }=0D }=0D =0D /**=0D diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf b/UefiCpuPkg/PiSm= mCpuDxeSmm/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,=0D # provides CPU specific services in SMM.=0D #=0D -# Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.
=0D +# Copyright (c) 2009 - 2021, Intel Corporation. All rights reserved.
=0D # Copyright (c) 2017, AMD Incorporated. All rights reserved.
=0D #=0D # SPDX-License-Identifier: BSD-2-Clause-Patent=0D @@ -134,6 +134,7 @@ gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmCodeAccessCheckEnable ## CONS= UMES=0D gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode ## CONS= UMES=0D gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmShadowStackSize ## SOME= TIMES_CONSUMES=0D + gUefiCpuPkgTokenSpaceGuid.PcdCpuFeaturesInitOnS3Resume ## CONS= UMES=0D gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable ## CONS= UMES=0D gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask ##= CONSUMES=0D gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask ##= CONSUMES=0D --=20 2.28.0.windows.1