Reviewed-by: Ray Ni <ray...@intel.com> > -----Original Message----- > From: Yao, Jiewen <jiewen....@intel.com> > Sent: Friday, February 26, 2021 8:22 PM > To: Sheng, W <w.sh...@intel.com>; devel@edk2.groups.io > Cc: Dong, Eric <eric.d...@intel.com>; Ni, Ray <ray...@intel.com>; Laszlo > Ersek <ler...@redhat.com>; Kumar, Rahul1 <rahul1.ku...@intel.com>; > Feng, Roger <roger.f...@intel.com> > Subject: RE: [PATCH v6 3/3] UefiCpuPkg/PiSmmCpuDxeSmm: Fix SMM stack > offset is not correct > > Reviewed-by: Jiewen Yao <jiewen....@intel.com> > > > -----Original Message----- > > From: Sheng, W <w.sh...@intel.com> > > Sent: Friday, February 26, 2021 4:03 PM > > To: devel@edk2.groups.io > > Cc: Dong, Eric <eric.d...@intel.com>; Ni, Ray <ray...@intel.com>; Laszlo > Ersek > > <ler...@redhat.com>; Kumar, Rahul1 <rahul1.ku...@intel.com>; Yao, > Jiewen > > <jiewen....@intel.com>; Feng, Roger <roger.f...@intel.com> > > Subject: [PATCH v6 3/3] UefiCpuPkg/PiSmmCpuDxeSmm: Fix SMM stack > offset is > > not correct > > > > In function InitGdt(), SmiPFHandler() and Gen4GPageTable(), it uses > > CpuIndex * mSmmStackSize to get the SMM stack address offset for > > multi processor. It misses the SMM Shadow Stack Size. Each processor > > will use mSmmStackSize + mSmmShadowStackSize in the memory. > > It should use CpuIndex * (mSmmStackSize + mSmmShadowStackSize) to > get > > this SMM stack address offset. If mSmmShadowStackSize > 0 and multi > > processor enabled, it will get the wrong offset value. > > CET shadow stack feature will set the value of mSmmShadowStackSize. > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3237 > > > > Signed-off-by: Sheng Wei <w.sh...@intel.com> > > Cc: Eric Dong <eric.d...@intel.com> > > Cc: Ray Ni <ray...@intel.com> > > Cc: Laszlo Ersek <ler...@redhat.com> > > Cc: Rahul Kumar <rahul1.ku...@intel.com> > > Cc: Jiewen Yao <jiewen....@intel.com> > > Cc: Roger Feng <roger.f...@intel.com> > > --- > > UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 6 ++++-- > > UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 4 +++- > > UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c | 2 +- > > 3 files changed, 8 insertions(+), 4 deletions(-) > > > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > > b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > > index 4bcd217917..6227b2428a 100644 > > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > > @@ -23,6 +23,8 @@ SPIN_LOCK *mPFLock = > > NULL; > > SMM_CPU_SYNC_MODE mCpuSmmSyncMode; > > BOOLEAN mMachineCheckSupported = FALSE; > > > > +extern UINTN mSmmShadowStackSize; > > + > > /** > > Performs an atomic compare exchange operation to get semaphore. > > The compare exchange operation must be performed using > > @@ -920,7 +922,7 @@ Gen4GPageTable ( > > // Add two more pages for known good stack and stack guard page, > > // then find the lower 2MB aligned address. > > // > > - High2MBoundary = (mSmmStackArrayEnd - mSmmStackSize + > EFI_PAGE_SIZE > > * 2) & ~(SIZE_2MB-1); > > + High2MBoundary = (mSmmStackArrayEnd - mSmmStackSize - > > mSmmShadowStackSize + EFI_PAGE_SIZE * 2) & ~(SIZE_2MB-1); > > PagesNeeded = ((High2MBoundary - Low2MBoundary) / SIZE_2MB) + 1; > > } > > // > > @@ -971,7 +973,7 @@ Gen4GPageTable ( > > // Mark the guard page as non-present > > // > > Pte[Index] = PageAddress | mAddressEncMask; > > - GuardPage += mSmmStackSize; > > + GuardPage += (mSmmStackSize + mSmmShadowStackSize); > > if (GuardPage > mSmmStackArrayEnd) { > > GuardPage = 0; > > } > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > > b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > > index cdc1fcefc5..07e7ea70de 100644 > > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > > @@ -13,6 +13,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > > #define PAGE_TABLE_PAGES 8 > > #define ACC_MAX_BIT BIT3 > > > > +extern UINTN mSmmShadowStackSize; > > + > > LIST_ENTRY mPagePool = > > INITIALIZE_LIST_HEAD_VARIABLE > > (mPagePool); > > BOOLEAN m1GPageTableSupport = FALSE; > > BOOLEAN mCpuSmmRestrictedMemoryAccess; > > @@ -1037,7 +1039,7 @@ SmiPFHandler ( > > (PFAddress < (mCpuHotPlugData.SmrrBase + > mCpuHotPlugData.SmrrSize))) { > > DumpCpuContext (InterruptType, SystemContext); > > CpuIndex = GetCpuIndex (); > > - GuardPageAddress = (mSmmStackArrayBase + EFI_PAGE_SIZE + > CpuIndex * > > mSmmStackSize); > > + GuardPageAddress = (mSmmStackArrayBase + EFI_PAGE_SIZE + > CpuIndex * > > (mSmmStackSize + mSmmShadowStackSize)); > > if ((FeaturePcdGet (PcdCpuSmmStackGuard)) && > > (PFAddress >= GuardPageAddress) && > > (PFAddress < (GuardPageAddress + EFI_PAGE_SIZE))) { > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c > > b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c > > index 7ef3b1d488..661c1ba294 100644 > > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c > > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c > > @@ -93,7 +93,7 @@ InitGdt ( > > // > > // Setup top of known good stack as IST1 for each processor. > > // > > - *(UINTN *)(TssBase + TSS_X64_IST1_OFFSET) = (mSmmStackArrayBase > + > > EFI_PAGE_SIZE + Index * mSmmStackSize); > > + *(UINTN *)(TssBase + TSS_X64_IST1_OFFSET) = > (mSmmStackArrayBase + > > EFI_PAGE_SIZE + Index * (mSmmStackSize + mSmmShadowStackSize)); > > } > > } > > > > -- > > 2.16.2.windows.1
-=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#72256): https://edk2.groups.io/g/devel/message/72256 Mute This Topic: https://groups.io/mt/80922789/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
