From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=17.151.62.68; helo=nwk-aaemail-lapp03.apple.com; envelope-from=afish@apple.com; receiver=edk2-devel@lists.01.org Received: from nwk-aaemail-lapp03.apple.com (nwk-aaemail-lapp03.apple.com [17.151.62.68]) (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 45DB9211D6734 for ; Fri, 29 Mar 2019 14:37:52 -0700 (PDT) Received: from pps.filterd (nwk-aaemail-lapp03.apple.com [127.0.0.1]) by nwk-aaemail-lapp03.apple.com (8.16.0.27/8.16.0.27) with SMTP id x2TLaiNM063881; Fri, 29 Mar 2019 14:37:51 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=apple.com; h=mime-version : content-transfer-encoding : content-type : sender : subject : from : in-reply-to : date : cc : message-id : references : to; s=20180706; bh=2fiCJaEnF+BBrCyAkgvZ62LNAtxqsanfBd+277sKrF8=; b=f2zXxpXmVxezpLt9xcs6W+8NeHJiicU1sQzm9EB6Au6WFDXOYfTYOKoW/u8wWdjihHRD Y0LWuftlq6Vj/u2ISx/HnQ4HNodk5RfgIupY3zo3Vu5XaUmGkK9WZONZV3r1ZOQeHiIt qCa32XQzJCvjP5kohbzvm7lSI8HNbyVJBuiOPKeKNEqfGEMKT0ECo/fG/6MWkS146yQM Px5u8VyVNnHSwZC1wuTSOwJ6t9HGYqTVrZ0NIEb/lA9u3xw8iCVS08vRu/aeN2i2mJ4x LSF953XeRzjEZVRDytyFgZzLXhtxKMp346Sgdzt4F7cJrK/afDkvatfdw2D9cQnqlGKh 1Q== Received: from ma1-mtap-s02.corp.apple.com (ma1-mtap-s02.corp.apple.com [17.40.76.6]) by nwk-aaemail-lapp03.apple.com with ESMTP id 2re4tsd972-2 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO); Fri, 29 Mar 2019 14:37:51 -0700 MIME-version: 1.0 Received: from ma1-mmpp-sz11.apple.com (ma1-mmpp-sz11.apple.com [17.171.128.33]) by ma1-mtap-s02.corp.apple.com (Oracle Communications Messaging Server 8.0.2.3.20181024 64bit (built Oct 24 2018)) with ESMTPS id <0PP500M0DDF2IM90@ma1-mtap-s02.corp.apple.com>; Fri, 29 Mar 2019 14:37:50 -0700 (PDT) Received: from process_milters-daemon.ma1-mmpp-sz11.apple.com by ma1-mmpp-sz11.apple.com (Oracle Communications Messaging Server 8.0.2.3.20181024 64bit (built Oct 24 2018)) id <0PP500800D0QI900@ma1-mmpp-sz11.apple.com>; Fri, 29 Mar 2019 14:37:50 -0700 (PDT) X-Va-A: X-Va-T-CD: dd2a240d1a14dad3816ec1f2a5370897 X-Va-E-CD: 0dc8999372c60d65c38f236332904462 X-Va-R-CD: fa35155c677ba95b8fde23f3bb1a90d7 X-Va-CD: 0 X-Va-ID: 336c6d36-ecfc-4d46-bcc4-2a4ec07d5723 X-V-A: X-V-T-CD: 81ca60fce39c2560b6c4a7e5841f9b8f X-V-E-CD: 0dc8999372c60d65c38f236332904462 X-V-R-CD: fa35155c677ba95b8fde23f3bb1a90d7 X-V-CD: 0 X-V-ID: e05d5eb7-6230-4998-b1fa-2252b6637553 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2019-03-29_13:,, signatures=0 Received: from [17.234.226.216] (unknown [17.234.226.216]) by ma1-mmpp-sz11.apple.com (Oracle Communications Messaging Server 8.0.2.3.20181024 64bit (built Oct 24 2018)) with ESMTPSA id <0PP50008GDEZMG70@ma1-mmpp-sz11.apple.com>; Fri, 29 Mar 2019 14:37:50 -0700 (PDT) Sender: afish@apple.com From: Andrew Fish In-reply-to: <02A34F284D1DA44BB705E61F7180EF0AAE9ADBB2@ORSMSX114.amr.corp.intel.com> Date: Fri, 29 Mar 2019 14:37:33 -0700 Cc: "Vanguput, Narendra K" , "edk2-devel@lists.01.org" , Laszlo Ersek , "Yao, Jiewen" , "Dong, Eric" Message-id: <113D10BD-153C-49B2-BBDD-0F5932BB2DCC@apple.com> References: <20190329154456.4304-1-narendra.k.vanguput@intel.com> <02A34F284D1DA44BB705E61F7180EF0AAE9ADBB2@ORSMSX114.amr.corp.intel.com> To: "Desimone, Nathaniel L" X-Mailer: Apple Mail (2.3445.6.18) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:, , definitions=2019-03-29_13:, , signatures=0 Subject: Re: [PATCH v8] UefiCpuPkg\CpuSmm: Save & restore CR2 on-demand paging in SMM X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 Mar 2019 21:37:52 -0000 Content-transfer-encoding: 7BIT Content-type: text/plain; CHARSET=US-ASCII > On Mar 29, 2019, at 2:22 PM, Desimone, Nathaniel L wrote: > > 1. Why would you do this for 64 bit but not 32 bit? Is paging enabled on 32-bit, it is required for Long mode? Also I'm not clear why it is an enhancement given you could take a periodic SMM in the kernels page fault handler and trashing CR2 seems bad. Maybe there is some behavior I'm missing? I'm not sure how big an issue this is but if SMM is modifying CR2 it is leaking information about SMM operations outside of SMM. Thanks, Andrew Fish > 2. Why don't you add the if statement to MpService.c instead of spreading it to PageTbl.c? > 3. What is the reason for this anyway? Adding the conditional is probably more execution time than just reading CR2 always. > > Thanks, > Nate > > -----Original Message----- > From: edk2-devel On Behalf Of nkvangup > Sent: Friday, March 29, 2019 8:45 AM > To: edk2-devel@lists.01.org > Cc: Yao, Jiewen ; Dong, Eric ; Laszlo Ersek > Subject: [edk2] [PATCH v8] UefiCpuPkg\CpuSmm: Save & restore CR2 on-demand paging in SMM > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1593 > > For every SMI occurrence, save and restore CR2 register only when SMM on-demand paging support is enabled in 64 bit operation mode. > This is not a bug but to have better improvement of code. > > Patch5 is updated with separate functions for Save and Restore of CR2 based on review feedback. > > Patch6 - Removed Global Cr2 instead used function parameter > > Patch7 - Removed checking Cr2 with 0 as per feedback > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Vanguput Narendra K > Cc: Eric Dong > Cc: Ray Ni > Cc: Laszlo Ersek > Cc: Yao Jiewen > --- > UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c | 26 ++++++++++++++++++++++++++ > UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 9 ++++++--- > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 22 ++++++++++++++++++++++ > UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 30 ++++++++++++++++++++++++++++++ > 4 files changed, 84 insertions(+), 3 deletions(-) > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c > index b734a1ea8c..d3f62ed806 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c > @@ -316,3 +316,29 @@ SetPageTableAttributes ( > > return ; > } > + > +/** > + This function returns with no action for 32 bit. > + > + @param[out] *Cr2 Pointer to variable to hold CR2 register value **/ > +VOID > +SaveCr2 ( > + UINTN *Cr2 > + ) > +{ > + return ; > +} > + > +/** > + This function returns with no action for 32 bit. > + > + @param[in] Cr2 Value to write into CR2 register **/ VOID > +RestoreCr2 ( > + UINTN Cr2 > + ) > +{ > + return ; > +} > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > index 3b0b3b52ac..ce70f77709 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > @@ -1112,9 +1112,11 @@ SmiRendezvous ( > ASSERT(CpuIndex < mMaxNumberOfCpus); > > // > - // Save Cr2 because Page Fault exception in SMM may override its value > + // Save Cr2 because Page Fault exception in SMM may override its > + value, // when using on-demand paging for above 4G memory. > // > - Cr2 = AsmReadCr2 (); > + Cr2 = 0; > + SaveCr2 (&Cr2); > > // > // Perform CPU specific entry hooks > @@ -1253,10 +1255,11 @@ SmiRendezvous ( > > Exit: > SmmCpuFeaturesRendezvousExit (CpuIndex); > + > // > // Restore Cr2 > // > - AsmWriteCr2 (Cr2); > + RestoreCr2 (Cr2); > } > > /** > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h > index 84efb22981..05e1b54ed2 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h > @@ -1243,4 +1243,26 @@ EFIAPI > PiSmmCpuSmiEntryFixupAddress ( > ); > > +/** > + This function reads CR2 register when on-demand paging is enabled > + for 64 bit and no action for 32 bit. > + > + @param[out] *Cr2 Pointer to variable to hold CR2 register value **/ > +VOID > +SaveCr2 ( > + UINTN *Cr2 > + ); > + > +/** > + This function writes into CR2 register when on-demand paging is > +enabled > + for 64 bit and no action for 32 bit. > + > + @param[in] Cr2 Value to write into CR2 register **/ VOID > +RestoreCr2 ( > + UINTN Cr2 > + ); > + > #endif > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > index 2c77cb47a4..e60628c080 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > @@ -1053,3 +1053,33 @@ SetPageTableAttributes ( > > return ; > } > + > +/** > + This function reads CR2 register when on-demand paging is enabled > + > + @param[out] *Cr2 Pointer to variable to hold CR2 register value **/ > +VOID > +SaveCr2 ( > + UINTN *Cr2 > + ) > +{ > + if (!mCpuSmmStaticPageTable) { > + *Cr2 = AsmReadCr2 (); > + } > +} > + > +/** > + This function restores CR2 register when on-demand paging is enabled > + > + @param[in] Cr2 Value to write into CR2 register **/ VOID > +RestoreCr2 ( > + UINTN Cr2 > + ) > +{ > + if (!mCpuSmmStaticPageTable) { > + AsmWriteCr2 (Cr2); > + } > +} > -- > 2.16.2.windows.1 > > _______________________________________________ > edk2-devel mailing list > mailto: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