From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.136; helo=mga12.intel.com; envelope-from=narendra.k.vanguput@intel.com; receiver=edk2-devel@lists.01.org Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) (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 C35AC211EC0DB for ; Thu, 28 Mar 2019 22:03:34 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 28 Mar 2019 22:03:34 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,283,1549958400"; d="scan'208";a="135801836" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by fmsmga008.fm.intel.com with ESMTP; 28 Mar 2019 22:03:34 -0700 Received: from bgsmsx105.gar.corp.intel.com (10.223.43.197) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.408.0; Thu, 28 Mar 2019 22:03:33 -0700 Received: from bgsmsx102.gar.corp.intel.com ([169.254.2.2]) by BGSMSX105.gar.corp.intel.com ([169.254.3.90]) with mapi id 14.03.0415.000; Fri, 29 Mar 2019 10:33:31 +0530 From: "Vanguput, Narendra K" To: "Kinney, Michael D" , "edk2-devel@lists.01.org" CC: "Yao, Jiewen" , "Dong, Eric" , Laszlo Ersek , "Chinnusamy, Rajkumar K" Thread-Topic: [edk2] [PATCH v5] UefiCpuPkg\CpuSmm: Save & restore CR2 on-demand paging in SMM Thread-Index: AQHU4OAeoTo/lYdBOUOMt/wtkM5PrqYXqMCAgApusCA= Date: Fri, 29 Mar 2019 05:03:30 +0000 Message-ID: <020B34E8430BB544AB9E0330B597780A664EA67B@BGSMSX102.gar.corp.intel.com> References: <20190322184956.2928-1-narendra.k.vanguput@intel.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action x-ctpclassification: CTP_NT x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYWQxNDY2NzctNDRkZi00M2IzLTkxMTctNjM4MDQ0MjgwMDI1IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiYWROTW1xcWJXenl5Z3UyaE9taHJGbVlFYUxET2sxWlBmYVg4a3pGVEQ0cmJLc3BcL2s4a2FVakVSY1V1bGlackcifQ== x-originating-ip: [10.223.10.10] MIME-Version: 1.0 Subject: Re: [PATCH v5] 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 05:03:35 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Thanks Mike for the comments! I updated and send as PATCH v6. Please review. Thanks, Narendra > -----Original Message----- > From: Kinney, Michael D > Sent: Saturday, March 23, 2019 12:44 AM > To: Vanguput, Narendra K ; edk2- > devel@lists.01.org; Kinney, Michael D > Cc: Yao, Jiewen ; Dong, Eric ; > Laszlo Ersek > Subject: RE: [edk2] [PATCH v5] UefiCpuPkg\CpuSmm: Save & restore CR2 on- > demand paging in SMM >=20 > Hi Narendra, >=20 > With this implementation, you have moved the save/restore location to a > module global variable. The name should be prefixed with 'm' to make tha= t > clear. >=20 > mCr2 >=20 > I do not think using a module global is MP safe. >=20 > The current implementation uses a local on the stack that is MP safe beca= use > each CPU has its own stack. >=20 > Mike >=20 > > -----Original Message----- > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] > > On Behalf Of nkvangup > > Sent: Friday, March 22, 2019 11:50 AM > > To: edk2-devel@lists.01.org > > Cc: Yao, Jiewen ; Dong, Eric > > ; Laszlo Ersek > > Subject: [edk2] [PATCH v5] UefiCpuPkg\CpuSmm: Save & restore CR2 > > on-demand paging in SMM > > > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1593 > > > > 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. > > > > 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 | 22 > > ++++++++++++++++++++++ > > UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 9 +++++--- > > - > > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 16 > > ++++++++++++++++ > > UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 28 > > ++++++++++++++++++++++++++++ > > 4 files changed, 71 insertions(+), 4 deletions(-) > > > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c > > b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c > > index b734a1ea8c..3750332ca8 100644 > > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c > > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c > > @@ -316,3 +316,25 @@ SetPageTableAttributes ( > > > > return ; > > } > > + > > +/** > > + This function returns with no action for 32 bit. > > +**/ > > +VOID > > +SaveCr2 ( > > + VOID > > + ) > > +{ > > +// Do Nothing > > +} > > + > > +/** > > + This function returns with no action for 32 bit. > > +**/ > > +VOID > > +RestoreCr2 ( > > + VOID > > + ) > > +{ > > +// Do Nothing > > +} > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > > b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > > index 3b0b3b52ac..6a5736a3eb 100644 > > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > > @@ -1107,14 +1107,14 @@ SmiRendezvous ( > > BOOLEAN IsBsp; > > BOOLEAN BspInProgress; > > UINTN Index; > > - UINTN Cr2; > > > > 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 =3D AsmReadCr2 (); > > + SaveCr2 (); > > > > // > > // Perform CPU specific entry hooks @@ -1253,10 +1253,11 @@ > > SmiRendezvous ( > > > > Exit: > > SmmCpuFeaturesRendezvousExit (CpuIndex); > > + > > // > > // Restore Cr2 > > // > > - AsmWriteCr2 (Cr2); > > + RestoreCr2 (); > > } > > > > /** > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h > > b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h > > index 84efb22981..71a8c13960 100644 > > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h > > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h > > @@ -1243,4 +1243,20 @@ EFIAPI > > PiSmmCpuSmiEntryFixupAddress ( > > ); > > > > +/** > > + This function saves CR2 register for 64 bit and no > > action for 32 bit. > > +**/ > > +VOID > > +SaveCr2 ( > > + VOID > > + ); > > + > > +/** > > + This function restores CR2 register for 64 bit and no > > action for 32 bit. > > +**/ > > +VOID > > +RestoreCr2 ( > > + VOID > > + ); > > + > > #endif > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > > b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > > index 2c77cb47a4..76a30de171 100644 > > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > > @@ -22,6 +22,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY > KIND, > > EITHER EXPRESS OR IMPLIED. > > LIST_ENTRY mPagePool =3D > > INITIALIZE_LIST_HEAD_VARIABLE (mPagePool); > > BOOLEAN m1GPageTableSupport > > =3D FALSE; > > BOOLEAN > > mCpuSmmStaticPageTable; > > +UINTN Cr2 =3D 0; > > > > /** > > Disable CET. > > @@ -1053,3 +1054,30 @@ SetPageTableAttributes ( > > > > return ; > > } > > + > > +/** > > + This function saves CR2 register. > > +**/ > > +VOID > > +SaveCr2 ( > > + VOID > > + ) > > +{ > > + if (!mCpuSmmStaticPageTable) { > > + Cr2 =3D AsmReadCr2 (); > > + } > > +} > > + > > +/** > > + This function restores CR2 register. > > +**/ > > +VOID > > +RestoreCr2 ( > > + VOID > > + ) > > +{ > > + if ((!mCpuSmmStaticPageTable) && (Cr2 !=3D 0)) { > > + AsmWriteCr2 (Cr2); > > + Cr2 =3D 0; > > + } > > +} > > -- > > 2.16.2.windows.1 > > > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel