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.115; helo=mga14.intel.com; envelope-from=michael.d.kinney@intel.com; receiver=edk2-devel@lists.01.org Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) (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 C6554211E7454 for ; Fri, 22 Mar 2019 12:13:39 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 22 Mar 2019 12:13:39 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,256,1549958400"; d="scan'208";a="154899544" Received: from orsmsx110.amr.corp.intel.com ([10.22.240.8]) by fmsmga004.fm.intel.com with ESMTP; 22 Mar 2019 12:13:39 -0700 Received: from orsmsx157.amr.corp.intel.com (10.22.240.23) by ORSMSX110.amr.corp.intel.com (10.22.240.8) with Microsoft SMTP Server (TLS) id 14.3.408.0; Fri, 22 Mar 2019 12:13:38 -0700 Received: from orsmsx112.amr.corp.intel.com ([169.254.3.233]) by ORSMSX157.amr.corp.intel.com ([169.254.9.126]) with mapi id 14.03.0415.000; Fri, 22 Mar 2019 12:13:38 -0700 From: "Kinney, Michael D" To: "Vanguput, Narendra K" , "edk2-devel@lists.01.org" , "Kinney, Michael D" CC: "Yao, Jiewen" , "Dong, Eric" , Laszlo Ersek Thread-Topic: [edk2] [PATCH v5] UefiCpuPkg\CpuSmm: Save & restore CR2 on-demand paging in SMM Thread-Index: AQHU4OAkciRdh90M00q0FUxwkMauBaYYA0iA Date: Fri, 22 Mar 2019 19:13:37 +0000 Message-ID: References: <20190322184956.2928-1-narendra.k.vanguput@intel.com> In-Reply-To: <20190322184956.2928-1-narendra.k.vanguput@intel.com> 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-originating-ip: [10.22.254.139] 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, 22 Mar 2019 19:13:40 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Narendra, With this implementation, you have moved the save/restore location to a module global variable. The name should be prefixed with 'm' to make that clear. =20 mCr2 I do not think using a module global is MP safe. The current implementation uses a local on the stack that is MP safe because each CPU has its own stack. Mike > -----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 >=20 > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1593 >=20 > 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. >=20 > Patch5 is updated with separate functions for Save and > Restore of CR2 > based on review feedback. >=20 > 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(-) >=20 > 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 ( >=20 > 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; >=20 > ASSERT(CpuIndex < mMaxNumberOfCpus); >=20 > // > - // 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 (); >=20 > // > // Perform CPU specific entry hooks > @@ -1253,10 +1253,11 @@ SmiRendezvous ( >=20 > Exit: > SmmCpuFeaturesRendezvousExit (CpuIndex); > + > // > // Restore Cr2 > // > - AsmWriteCr2 (Cr2); > + RestoreCr2 (); > } >=20 > /** > 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 ( > ); >=20 > +/** > + 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; >=20 > /** > Disable CET. > @@ -1053,3 +1054,30 @@ SetPageTableAttributes ( >=20 > 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 >=20 > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel