On 11/22/16 09:06, Laszlo Ersek wrote: > On 11/22/16 08:20, Fan, Jeff wrote: >> Reviewed-by: Jeff Fan <jeff....@intel.com> > > Thank Jeff!
This was supposed to be "Thank you, Jeff". Not making much progress on the "getting enough sleep" front. :/ Laszlo > I committed this patch in isolation (b43dd22981b7), because I think it > has value without the OvmfPkg changes too. Plus, I think the OvmfPkg > changes might take longer to get into the tree, and I wouldn't like > PiSmmCpuDxeSmm to diverge in the meantime. > > Thanks! > Laszlo > >> -----Original Message----- >> From: Laszlo Ersek [mailto:ler...@redhat.com] >> Sent: Friday, November 18, 2016 9:53 PM >> To: edk2-devel-01 >> Cc: Fan, Jeff; Justen, Jordan L; Kinney, Michael D; Paolo Bonzini >> Subject: [PATCH v2 1/4] 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....@intel.com> >> Cc: Jordan Justen <jordan.l.jus...@intel.com> >> Cc: Michael Kinney <michael.d.kin...@intel.com> >> Cc: Paolo Bonzini <pbonz...@redhat.com> >> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=230 >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Laszlo Ersek <ler...@redhat.com> >> --- >> >> Notes: >> v2: >> - new in v2 >> >> UefiCpuPkg/UefiCpuPkg.dec | 20 ++++++++++---------- >> UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 4 +++- >> 2 files changed, 13 insertions(+), 11 deletions(-) >> >> diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec index >> 89779c447d50..ca560398bbef 100644 >> --- a/UefiCpuPkg/UefiCpuPkg.dec >> +++ b/UefiCpuPkg/UefiCpuPkg.dec >> @@ -156,10 +156,6 @@ [PcdsFixedAtBuild, PcdsPatchableInModule] >> # @Prompt Processor stack size in SMM. >> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackSize|0x2000|UINT32|0x32132105 >> >> - ## Specifies timeout value in microseconds for the BSP in SMM to wait for >> all APs to come into SMM. >> - # @Prompt AP synchronization timeout value in SMM. >> - gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmApSyncTimeout|1000000|UINT64|0x32132104 >> - >> ## Indicates if SMM Code Access Check is enabled. >> # If enabled, the SMM handler cannot execute the code outside SMM >> regions. >> # This PCD is suggested to TRUE in production image.<BR><BR> @@ -168,12 >> +164,6 @@ [PcdsFixedAtBuild, PcdsPatchableInModule] >> # @Prompt SMM Code Access Check. >> >> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmCodeAccessCheckEnable|TRUE|BOOLEAN|0x60000013 >> >> - ## Indicates the CPU synchronization method used when processing an SMI. >> - # 0x00 - Traditional CPU synchronization method.<BR> >> - # 0x01 - Relaxed CPU synchronization method.<BR> >> - # @Prompt SMM CPU Synchronization Method. >> - gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode|0x00|UINT8|0x60000014 >> - >> ## Specifies the number of variable MTRRs reserved for OS use. The >> default number of >> # MTRRs reserved for OS use is 2. >> # @Prompt Number of reserved variable MTRRs. >> @@ -214,6 +204,16 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, >> PcdsDynamicEx] >> # @Prompt Use static page table for all memory in SMM. >> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStaticPageTable|TRUE|BOOLEAN|0x3213210D >> >> + ## Specifies timeout value in microseconds for the BSP in SMM to wait for >> all APs to come into SMM. >> + # @Prompt AP synchronization timeout value in SMM. >> + >> + gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmApSyncTimeout|1000000|UINT64|0x3213 >> + 2104 >> + >> + ## Indicates the CPU synchronization method used when processing an SMI. >> + # 0x00 - Traditional CPU synchronization method.<BR> >> + # 0x01 - Relaxed CPU synchronization method.<BR> >> + # @Prompt SMM CPU Synchronization Method. >> + gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode|0x00|UINT8|0x60000014 >> + >> [PcdsDynamic, PcdsDynamicEx] >> ## Contains the pointer to a CPU S3 data buffer of structure >> ACPI_CPU_DATA. >> # @Prompt The pointer to a CPU S3 data buffer. >> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c >> b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c >> index 9b8db90ff6ed..cfbf59e8f2ca 100644 >> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c >> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c >> @@ -24,6 +24,7 @@ UINTN >> mSmmMpSyncDataSize; >> SMM_CPU_SEMAPHORES mSmmCpuSemaphores; >> UINTN mSemaphoreSize; >> SPIN_LOCK *mPFLock = NULL; >> +SMM_CPU_SYNC_MODE mCpuSmmSyncMode; >> >> /** >> Performs an atomic compare exchange operation to get semaphore. >> @@ -1338,7 +1339,7 @@ InitializeMpSyncData ( >> // >> mSmmMpSyncData->BspIndex = (UINT32)-1; >> } >> - mSmmMpSyncData->EffectiveSyncMode = (SMM_CPU_SYNC_MODE) PcdGet8 >> (PcdCpuSmmSyncMode); >> + mSmmMpSyncData->EffectiveSyncMode = mCpuSmmSyncMode; >> >> mSmmMpSyncData->Counter = >> mSmmCpuSemaphores.SemaphoreGlobal.Counter; >> mSmmMpSyncData->InsideSmm = >> mSmmCpuSemaphores.SemaphoreGlobal.InsideSmm; >> @@ -1392,6 +1393,7 @@ InitializeMpServiceData ( >> (sizeof (SMM_CPU_DATA_BLOCK) + sizeof (BOOLEAN)) * >> gSmmCpuPrivate->SmmCoreEntryContext.NumberOfCpus; >> mSmmMpSyncData = (SMM_DISPATCHER_MP_SYNC_DATA*) AllocatePages >> (EFI_SIZE_TO_PAGES (mSmmMpSyncDataSize)); >> ASSERT (mSmmMpSyncData != NULL); >> + mCpuSmmSyncMode = (SMM_CPU_SYNC_MODE)PcdGet8 (PcdCpuSmmSyncMode); >> InitializeMpSyncData (); >> >> // >> -- >> 2.9.2 >> >> >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.01.org >> https://lists.01.org/mailman/listinfo/edk2-devel >> > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
