UefiCpuPkg/PiSmmCpuDxeSmm: dynamic PcdCpuSmmApSyncTimeout, PcdCpuSmmSyncMode

Move the declaration of these PCDs from the

  [PcdsFixedAtBuild, PcdsPatchableInModule]

section of "UefiCpuPkg/UefiCpuPkg.dec" to the

  [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]

section. Their types, default values, and token values remain unchanged.

Only UefiCpuPkg/PiSmmCpuDxeSmm consumes these PCDs, specifically on the
call stack of its entry point function, and it turns them into static or
dynamically allocated data in SMRAM:

  PiCpuSmmEntry()                            [PiSmmCpuDxeSmm.c]
    InitializeSmmTimer()                     [SyncTimer.c]
      PcdCpuSmmApSyncTimeout
      -> mTimeoutTicker
    InitializeMpServiceData()                [MpService.c]
      InitializeMpSyncData()                 [MpService.c]
        PcdCpuSmmSyncMode
        -> mSmmMpSyncData->EffectiveSyncMode

However, there's another call path to fetching "PcdCpuSmmSyncMode", namely

  SmmInitHandler()                           [PiSmmCpuDxeSmm.c]
    InitializeMpSyncData()                   [MpService.c]
      PcdCpuSmmSyncMode
      -> mSmmMpSyncData->EffectiveSyncMode

and this path is exercised during S3 resume (as stated by the comment in
SmmInitHandler() too, "Initialize private data during S3 resume").

While we can call the PCD protocol (via PcdLib) for fetching dynamic PCDs
in the entry point function, we cannot do that at S3 resume. Therefore
pre-fetch PcdCpuSmmSyncMode into a new global variable (which lives in
SMRAM) in InitializeMpServiceData(), just before calling
InitializeMpSyncData(). This way InitializeMpSyncData() can retrieve the
stashed PCD value from SMRAM, regardless of the boot mode.

Cc: Jeff Fan <jeff.fan@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=230
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jeff Fan <jeff.fan@intel.com>

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index 9b8db90ff6edb465787c972b250a0133aafc6dfe..cfbf59e8f2cae5f377fe5c9202c46fc3e790890e 100644 (file)
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -24,6 +24,7 @@ UINTN                                       mSmmMpSyncDataSize;
 SMM_CPU_SEMAPHORES                          mSmmCpuSemaphores;\r
 UINTN                                       mSemaphoreSize;\r
 SPIN_LOCK                                   *mPFLock = NULL;\r
+SMM_CPU_SYNC_MODE                           mCpuSmmSyncMode;\r
 \r
 /**\r
   Performs an atomic compare exchange operation to get semaphore.\r
@@ -1338,7 +1339,7 @@ InitializeMpSyncData (
       //\r
       mSmmMpSyncData->BspIndex = (UINT32)-1;\r
     }\r
-    mSmmMpSyncData->EffectiveSyncMode = (SMM_CPU_SYNC_MODE) PcdGet8 (PcdCpuSmmSyncMode);\r
+    mSmmMpSyncData->EffectiveSyncMode = mCpuSmmSyncMode;\r
 \r
     mSmmMpSyncData->Counter       = mSmmCpuSemaphores.SemaphoreGlobal.Counter;\r
     mSmmMpSyncData->InsideSmm     = mSmmCpuSemaphores.SemaphoreGlobal.InsideSmm;\r
@@ -1392,6 +1393,7 @@ InitializeMpServiceData (
                        (sizeof (SMM_CPU_DATA_BLOCK) + sizeof (BOOLEAN)) * gSmmCpuPrivate->SmmCoreEntryContext.NumberOfCpus;\r
   mSmmMpSyncData = (SMM_DISPATCHER_MP_SYNC_DATA*) AllocatePages (EFI_SIZE_TO_PAGES (mSmmMpSyncDataSize));\r
   ASSERT (mSmmMpSyncData != NULL);\r
+  mCpuSmmSyncMode = (SMM_CPU_SYNC_MODE)PcdGet8 (PcdCpuSmmSyncMode);\r
   InitializeMpSyncData ();\r
 \r
   //\r

diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index 89779c447d50875521e5400b23c4f018209570f7..ca560398bbeff4e66d51b2a916b70cc5c3d18fcc 100644 (file)
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -156,10 +156,6 @@
   # @Prompt Processor stack size in SMM.\r
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackSize|0x2000|UINT32|0x32132105\r
 \r
-  ## Specifies timeout value in microseconds for the BSP in SMM to wait for all APs to come into SMM.\r
-  # @Prompt AP synchronization timeout value in SMM.\r
-  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmApSyncTimeout|1000000|UINT64|0x32132104\r
-\r
   ## Indicates if SMM Code Access Check is enabled.\r
   #  If enabled, the SMM handler cannot execute the code outside SMM regions.\r
   #  This PCD is suggested to TRUE in production image.<BR><BR>\r
@@ -168,12 +164,6 @@
   # @Prompt SMM Code Access Check.\r
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmCodeAccessCheckEnable|TRUE|BOOLEAN|0x60000013\r
 \r
-  ## Indicates the CPU synchronization method used when processing an SMI.\r
-  #   0x00  - Traditional CPU synchronization method.<BR>\r
-  #   0x01  - Relaxed CPU synchronization method.<BR>\r
-  # @Prompt SMM CPU Synchronization Method.\r
-  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode|0x00|UINT8|0x60000014\r
-\r
   ## Specifies the number of variable MTRRs reserved for OS use. The default number of\r
   #  MTRRs reserved for OS use is 2.\r
   # @Prompt Number of reserved variable MTRRs.\r
@@ -214,6 +204,16 @@
   # @Prompt Use static page table for all memory in SMM.\r
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStaticPageTable|TRUE|BOOLEAN|0x3213210D\r
 \r
+  ## Specifies timeout value in microseconds for the BSP in SMM to wait for all APs to come into SMM.\r
+  # @Prompt AP synchronization timeout value in SMM.\r
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmApSyncTimeout|1000000|UINT64|0x32132104\r
+\r
+  ## Indicates the CPU synchronization method used when processing an SMI.\r
+  #   0x00  - Traditional CPU synchronization method.<BR>\r
+  #   0x01  - Relaxed CPU synchronization method.<BR>\r
+  # @Prompt SMM CPU Synchronization Method.\r
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode|0x00|UINT8|0x60000014\r
+\r
 [PcdsDynamic, PcdsDynamicEx]\r
   ## Contains the pointer to a CPU S3 data buffer of structure ACPI_CPU_DATA.\r
   # @Prompt The pointer to a CPU S3 data buffer.\r