* [PATCH v6] UefiCpuPkg\CpuSmm: Save & restore CR2 on-demand paging in SMM @ 2019-03-29 4:58 nkvangup 2019-03-29 5:10 ` Ni, Ray 2019-03-30 14:55 ` Zeng, Star 0 siblings, 2 replies; 5+ messages in thread From: nkvangup @ 2019-03-29 4:58 UTC (permalink / raw) To: edk2-devel Cc: Vanguput Narendra K, Eric Dong, Ray Ni, Laszlo Ersek, Yao Jiewen 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. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Vanguput Narendra K <narendra.k.vanguput@intel.com> Cc: Eric Dong <eric.dong@intel.com> Cc: Ray Ni <ray.ni@intel.com> Cc: Laszlo Ersek <lersek@redhat.com> Cc: Yao Jiewen <jiewen.yao@intel.com> --- 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..af96e42982 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[in] *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..c9d147c8a1 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[in] *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..6cb44fbbe5 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[in] *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) && (Cr2 != 0)) { + AsmWriteCr2 (Cr2); + } +} -- 2.16.2.windows.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v6] UefiCpuPkg\CpuSmm: Save & restore CR2 on-demand paging in SMM 2019-03-29 4:58 [PATCH v6] UefiCpuPkg\CpuSmm: Save & restore CR2 on-demand paging in SMM nkvangup @ 2019-03-29 5:10 ` Ni, Ray 2019-03-29 9:31 ` Vanguput, Narendra K 2019-03-30 14:55 ` Zeng, Star 1 sibling, 1 reply; 5+ messages in thread From: Ni, Ray @ 2019-03-29 5:10 UTC (permalink / raw) To: Vanguput, Narendra K, edk2-devel@lists.01.org Cc: Yao, Jiewen, Dong, Eric, Laszlo Ersek > + if ((!mCpuSmmStaticPageTable) && (Cr2 != 0)) { Can the "Cr2 != 0" be removed? > -----Original Message----- > From: edk2-devel <edk2-devel-bounces@lists.01.org> On Behalf Of > nkvangup > Sent: Friday, March 29, 2019 12:58 PM > To: edk2-devel@lists.01.org > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Dong, Eric <eric.dong@intel.com>; > Laszlo Ersek <lersek@redhat.com> > Subject: [edk2] [PATCH v6] 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. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Vanguput Narendra K <narendra.k.vanguput@intel.com> > Cc: Eric Dong <eric.dong@intel.com> > Cc: Ray Ni <ray.ni@intel.com> > Cc: Laszlo Ersek <lersek@redhat.com> > Cc: Yao Jiewen <jiewen.yao@intel.com> > --- > 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..af96e42982 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[in] *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..c9d147c8a1 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[in] *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..6cb44fbbe5 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[in] *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) && (Cr2 != 0)) { > + AsmWriteCr2 (Cr2); > + } > +} > -- > 2.16.2.windows.1 > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v6] UefiCpuPkg\CpuSmm: Save & restore CR2 on-demand paging in SMM 2019-03-29 5:10 ` Ni, Ray @ 2019-03-29 9:31 ` Vanguput, Narendra K 2019-03-29 9:36 ` Ni, Ray 0 siblings, 1 reply; 5+ messages in thread From: Vanguput, Narendra K @ 2019-03-29 9:31 UTC (permalink / raw) To: Ni, Ray, edk2-devel@lists.01.org; +Cc: Yao, Jiewen, Dong, Eric, Laszlo Ersek Hi Ray, While programming, I thought this cannot be 0 as in SDM, it says page-fault linear address. >> CR2 - Contains the page-fault linear address (the linear address that caused a page fault). So added a check for 0. Now as the function is changed like program into CR2 register based on input parameter and checking for 0 is up to caller of this function. And also we don't need to require for checking 0 means, will remove it. Please confirm. Thanks, Naren -----Original Message----- From: Ni, Ray Sent: Friday, March 29, 2019 10:40 AM To: Vanguput, Narendra K <narendra.k.vanguput@intel.com>; edk2-devel@lists.01.org Cc: Yao, Jiewen <jiewen.yao@intel.com>; Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com> Subject: RE: [edk2] [PATCH v6] UefiCpuPkg\CpuSmm: Save & restore CR2 on-demand paging in SMM > + if ((!mCpuSmmStaticPageTable) && (Cr2 != 0)) { Can the "Cr2 != 0" be removed? > -----Original Message----- > From: edk2-devel <edk2-devel-bounces@lists.01.org> On Behalf Of > nkvangup > Sent: Friday, March 29, 2019 12:58 PM > To: edk2-devel@lists.01.org > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Dong, Eric > <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com> > Subject: [edk2] [PATCH v6] 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. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Vanguput Narendra K <narendra.k.vanguput@intel.com> > Cc: Eric Dong <eric.dong@intel.com> > Cc: Ray Ni <ray.ni@intel.com> > Cc: Laszlo Ersek <lersek@redhat.com> > Cc: Yao Jiewen <jiewen.yao@intel.com> > --- > 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..af96e42982 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[in] *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..c9d147c8a1 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[in] *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..6cb44fbbe5 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[in] *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) && (Cr2 != 0)) { > + AsmWriteCr2 (Cr2); > + } > +} > -- > 2.16.2.windows.1 > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v6] UefiCpuPkg\CpuSmm: Save & restore CR2 on-demand paging in SMM 2019-03-29 9:31 ` Vanguput, Narendra K @ 2019-03-29 9:36 ` Ni, Ray 0 siblings, 0 replies; 5+ messages in thread From: Ni, Ray @ 2019-03-29 9:36 UTC (permalink / raw) To: Vanguput, Narendra K, edk2-devel@lists.01.org Cc: Yao, Jiewen, Dong, Eric, Laszlo Ersek Yes. With below "Cr2 !=0" removed from final commit, Reviewed-by: Ray Ni <ray.ni@intel.com> > > +RestoreCr2 ( > > + UINTN Cr2 > > + ) > > +{ > > + if ((!mCpuSmmStaticPageTable) && (Cr2 != 0)) { > > + AsmWriteCr2 (Cr2); > > + } > > +} > -----Original Message----- > From: Vanguput, Narendra K <narendra.k.vanguput@intel.com> > Sent: Friday, March 29, 2019 5:32 PM > To: Ni, Ray <ray.ni@intel.com>; edk2-devel@lists.01.org > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Dong, Eric <eric.dong@intel.com>; > Laszlo Ersek <lersek@redhat.com> > Subject: RE: [edk2] [PATCH v6] UefiCpuPkg\CpuSmm: Save & restore CR2 on- > demand paging in SMM > > Hi Ray, > > While programming, I thought this cannot be 0 as in SDM, it says page-fault > linear address. > >> CR2 - Contains the page-fault linear address (the linear address that > caused a page fault). > > So added a check for 0. > > Now as the function is changed like program into CR2 register based on input > parameter and checking for 0 is up to caller of this function. And also we > don't need to require for checking 0 means, will remove it. Please confirm. > > Thanks, > Naren > > -----Original Message----- > From: Ni, Ray > Sent: Friday, March 29, 2019 10:40 AM > To: Vanguput, Narendra K <narendra.k.vanguput@intel.com>; edk2- > devel@lists.01.org > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Dong, Eric <eric.dong@intel.com>; > Laszlo Ersek <lersek@redhat.com> > Subject: RE: [edk2] [PATCH v6] UefiCpuPkg\CpuSmm: Save & restore CR2 on- > demand paging in SMM > > > + if ((!mCpuSmmStaticPageTable) && (Cr2 != 0)) { > > Can the "Cr2 != 0" be removed? > > > -----Original Message----- > > From: edk2-devel <edk2-devel-bounces@lists.01.org> On Behalf Of > > nkvangup > > Sent: Friday, March 29, 2019 12:58 PM > > To: edk2-devel@lists.01.org > > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Dong, Eric > > <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com> > > Subject: [edk2] [PATCH v6] 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. > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Vanguput Narendra K <narendra.k.vanguput@intel.com> > > Cc: Eric Dong <eric.dong@intel.com> > > Cc: Ray Ni <ray.ni@intel.com> > > Cc: Laszlo Ersek <lersek@redhat.com> > > Cc: Yao Jiewen <jiewen.yao@intel.com> > > --- > > 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..af96e42982 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[in] *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..c9d147c8a1 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[in] *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..6cb44fbbe5 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[in] *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) && (Cr2 != 0)) { > > + AsmWriteCr2 (Cr2); > > + } > > +} > > -- > > 2.16.2.windows.1 > > > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v6] UefiCpuPkg\CpuSmm: Save & restore CR2 on-demand paging in SMM 2019-03-29 4:58 [PATCH v6] UefiCpuPkg\CpuSmm: Save & restore CR2 on-demand paging in SMM nkvangup 2019-03-29 5:10 ` Ni, Ray @ 2019-03-30 14:55 ` Zeng, Star 1 sibling, 0 replies; 5+ messages in thread From: Zeng, Star @ 2019-03-30 14:55 UTC (permalink / raw) To: Vanguput, Narendra K, edk2-devel@lists.01.org Cc: Yao, Jiewen, Dong, Eric, Laszlo Ersek, Zeng, Star Not comment to functionality. But just some observations about code style. Please check them inline. > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > nkvangup > Sent: Friday, March 29, 2019 12:58 PM > To: edk2-devel@lists.01.org > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Dong, Eric <eric.dong@intel.com>; > Laszlo Ersek <lersek@redhat.com> > Subject: [edk2] [PATCH v6] 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. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Vanguput Narendra K <narendra.k.vanguput@intel.com> > Cc: Eric Dong <eric.dong@intel.com> > Cc: Ray Ni <ray.ni@intel.com> > Cc: Laszlo Ersek <lersek@redhat.com> > Cc: Yao Jiewen <jiewen.yao@intel.com> > --- > 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..af96e42982 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[in] *Cr2 Pointer to variable to hold CR2 register value It will be more accurate with "@param[out]" according to https://edk2-docs.gitbooks.io/edk-ii-c-coding-standards-specification/content/5_source_files/57_c_programming.html#table-9-parameter-modifiers. Same comment is also applied to other implementation or prototype in header file. > +**/ > +VOID > +SaveCr2 ( > + UINTN *Cr2 Align with the comments above, it will be better with "OUT UINTN *Cr2" Same comment is also applied to other implementation or prototype in header file. > + ) > +{ > + return ; > +} > + > +/** > + This function returns with no action for 32 bit. > + > + @param[in] Cr2 Value to write into CR2 register > +**/ > +VOID > +RestoreCr2 ( > + UINTN Cr2 It will be better with "IN UINTN Cr2". Same comment is also applied to other implementation or prototype in header file. Thanks, Star > + ) > +{ > + 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..c9d147c8a1 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[in] *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..6cb44fbbe5 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[in] *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) && (Cr2 != 0)) { > + AsmWriteCr2 (Cr2); > + } > +} > -- > 2.16.2.windows.1 > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-03-30 14:55 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-03-29 4:58 [PATCH v6] UefiCpuPkg\CpuSmm: Save & restore CR2 on-demand paging in SMM nkvangup 2019-03-29 5:10 ` Ni, Ray 2019-03-29 9:31 ` Vanguput, Narendra K 2019-03-29 9:36 ` Ni, Ray 2019-03-30 14:55 ` Zeng, Star
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox