public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Jason Lou" <yun.lou@intel.com>
To: devel@edk2.groups.io
Cc: Jason <yun.lou@intel.com>, Ray Ni <ray.ni@intel.com>,
	Eric Dong <eric.dong@intel.com>, Laszlo Ersek <lersek@redhat.com>,
	Rahul Kumar <rahul1.kumar@intel.com>
Subject: [PATCH v1 2/2] UefiCpuPkg: Prevent from re-initializing CPU features during S3 resume
Date: Thu, 16 Sep 2021 10:01:32 +0800	[thread overview]
Message-ID: <20210916020132.3639-2-yun.lou@intel.com> (raw)
In-Reply-To: <20210916020132.3639-1-yun.lou@intel.com>

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 <yun.lou@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
---
 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 = (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


  reply	other threads:[~2021-09-16  2:01 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-16  2:01 [PATCH v1 1/2] UefiCpuPkg: Refactor initialization of CPU features during S3 resume Jason Lou
2021-09-16  2:01 ` Jason Lou [this message]
2021-09-16  6:14 ` Ni, Ray
2021-09-16  6:24   ` [edk2-devel] " Jason Lou

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210916020132.3639-2-yun.lou@intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox