From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 009E381E1F for ; Tue, 22 Nov 2016 00:10:02 -0800 (PST) Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7165881F07; Tue, 22 Nov 2016 08:10:02 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-82.phx2.redhat.com [10.3.116.82]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id uAM8A0K6016244; Tue, 22 Nov 2016 03:10:00 -0500 To: "Fan, Jeff" , edk2-devel-01 References: <20161118135249.26018-1-lersek@redhat.com> <20161118135249.26018-2-lersek@redhat.com> <542CF652F8836A4AB8DBFAAD40ED192A4A2E475A@shsmsx102.ccr.corp.intel.com> Cc: "Kinney, Michael D" , "Justen, Jordan L" , Paolo Bonzini From: Laszlo Ersek Message-ID: <4d5a747c-f404-1b8f-7a6c-cc8ff4a2b758@redhat.com> Date: Tue, 22 Nov 2016 09:09:59 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Tue, 22 Nov 2016 08:10:02 +0000 (UTC) Subject: Re: [PATCH v2 1/4] UefiCpuPkg/PiSmmCpuDxeSmm: dynamic PcdCpuSmmApSyncTimeout, PcdCpuSmmSyncMode X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 22 Nov 2016 08:10:03 -0000 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit On 11/22/16 09:06, Laszlo Ersek wrote: > On 11/22/16 08:20, Fan, Jeff wrote: >> Reviewed-by: Jeff Fan > > 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:lersek@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 >> Cc: Jordan Justen >> Cc: Michael Kinney >> Cc: Paolo Bonzini >> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=230 >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Laszlo Ersek >> --- >> >> 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.

@@ -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.
>> - # 0x01 - Relaxed CPU synchronization method.
>> - # @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.
>> + # 0x01 - Relaxed CPU synchronization method.
>> + # @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 >> >