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 3A13581E1F for ; Tue, 22 Nov 2016 00:06:52 -0800 (PST) Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (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 BC4034E040; Tue, 22 Nov 2016 08:06:51 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-82.phx2.redhat.com [10.3.116.82]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id uAM86nPp012366; Tue, 22 Nov 2016 03:06:50 -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: Date: Tue, 22 Nov 2016 09:06:49 +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: <542CF652F8836A4AB8DBFAAD40ED192A4A2E475A@shsmsx102.ccr.corp.intel.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Tue, 22 Nov 2016 08:06:51 +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:06:52 -0000 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit On 11/22/16 08:20, Fan, Jeff wrote: > Reviewed-by: Jeff Fan Thank Jeff! 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 >