* [PATCH] UefiCpuPkg/PiSmmCpu: Correct exception message. @ 2016-11-22 11:47 Jiewen Yao 2016-11-22 21:17 ` Laszlo Ersek 0 siblings, 1 reply; 3+ messages in thread From: Jiewen Yao @ 2016-11-22 11:47 UTC (permalink / raw) To: edk2-devel; +Cc: Laszlo Ersek, Jeff Fan, Michael D Kinney This patch fixes the first part of https://bugzilla.tianocore.org/show_bug.cgi?id=242 Previously, when SMM exception happens, "stack overflow" is misreported. This patch checked the PF address to see it is stack overflow, or it is caused by SMM page protection. Cc: Laszlo Ersek <lersek@redhat.com> Cc: Jeff Fan <jeff.fan@intel.com> Cc: Michael D Kinney <michael.d.kinney@intel.com> Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Jiewen Yao <jiewen.yao@intel.com> --- UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c | 28 +++++++++++++++++--- UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h | 9 +++++++ UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 27 ++++++++++++++++--- 3 files changed, 57 insertions(+), 7 deletions(-) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c index 5033bc5..feca142 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c @@ -91,6 +91,8 @@ SmiPFHandler ( ) { UINTN PFAddress; + UINTN GuardPageAddress; + UINTN CpuIndex; ASSERT (InterruptType == EXCEPT_IA32_PAGE_FAULT); @@ -98,10 +100,30 @@ SmiPFHandler ( PFAddress = AsmReadCr2 (); - if ((FeaturePcdGet (PcdCpuSmmStackGuard)) && - (PFAddress >= mCpuHotPlugData.SmrrBase) && + // + // If a page fault occurs in SMRAM range, it might be in a SMM stack guard page, + // or SMM page protection violation. + // + if ((PFAddress >= mCpuHotPlugData.SmrrBase) && (PFAddress < (mCpuHotPlugData.SmrrBase + mCpuHotPlugData.SmrrSize))) { - DEBUG ((DEBUG_ERROR, "SMM stack overflow!\n")); + CpuIndex = GetCpuIndex (); + GuardPageAddress = (mSmmStackArrayBase + EFI_PAGE_SIZE + CpuIndex * mSmmStackSize); + if ((FeaturePcdGet (PcdCpuSmmStackGuard)) && + (PFAddress >= GuardPageAddress) && + (PFAddress < (GuardPageAddress + EFI_PAGE_SIZE))) { + DEBUG ((DEBUG_ERROR, "SMM stack overflow!\n")); + } + if ((SystemContext.SystemContextIa32->ExceptionData & IA32_PF_EC_ID) != 0) { + DEBUG ((DEBUG_ERROR, "SMM exception at execution (0x%lx)\n", PFAddress)); + DEBUG_CODE ( + DumpModuleInfoByIp (*(UINTN *)(UINTN)SystemContext.SystemContextIa32->Esp); + ); + } else { + DEBUG ((DEBUG_ERROR, "SMM exception at write (0x%lx)\n", PFAddress)); + DEBUG_CODE ( + DumpModuleInfoByIp ((UINTN)SystemContext.SystemContextIa32->Eip); + ); + } CpuDeadLoop (); } diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h index b6fb5cf..04a3dfb 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h @@ -105,6 +105,15 @@ InitPaging ( VOID ); +/** + Get CPU Index from APIC ID. + +**/ +UINTN +GetCpuIndex ( + VOID + ); + // // The flag indicates if execute-disable is supported by processor. // diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c index 531e188..ec8eab7 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c @@ -804,6 +804,8 @@ SmiPFHandler ( ) { UINTN PFAddress; + UINTN GuardPageAddress; + UINTN CpuIndex; ASSERT (InterruptType == EXCEPT_IA32_PAGE_FAULT); @@ -817,12 +819,29 @@ SmiPFHandler ( } // - // If a page fault occurs in SMRAM range, it should be in a SMM stack guard page. + // If a page fault occurs in SMRAM range, it might be in a SMM stack guard page, + // or SMM page protection violation. // - if ((FeaturePcdGet (PcdCpuSmmStackGuard)) && - (PFAddress >= mCpuHotPlugData.SmrrBase) && + if ((PFAddress >= mCpuHotPlugData.SmrrBase) && (PFAddress < (mCpuHotPlugData.SmrrBase + mCpuHotPlugData.SmrrSize))) { - DEBUG ((DEBUG_ERROR, "SMM stack overflow!\n")); + CpuIndex = GetCpuIndex (); + GuardPageAddress = (mSmmStackArrayBase + EFI_PAGE_SIZE + CpuIndex * mSmmStackSize); + if ((FeaturePcdGet (PcdCpuSmmStackGuard)) && + (PFAddress >= GuardPageAddress) && + (PFAddress < (GuardPageAddress + EFI_PAGE_SIZE))) { + DEBUG ((DEBUG_ERROR, "SMM stack overflow!\n")); + } + if ((SystemContext.SystemContextX64->ExceptionData & IA32_PF_EC_ID) != 0) { + DEBUG ((DEBUG_ERROR, "SMM exception at execution (0x%lx)\n", PFAddress)); + DEBUG_CODE ( + DumpModuleInfoByIp (*(UINTN *)(UINTN)SystemContext.SystemContextX64->Rsp); + ); + } else { + DEBUG ((DEBUG_ERROR, "SMM exception at write (0x%lx)\n", PFAddress)); + DEBUG_CODE ( + DumpModuleInfoByIp ((UINTN)SystemContext.SystemContextX64->Rip); + ); + } CpuDeadLoop (); } -- 2.7.4.windows.1 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] UefiCpuPkg/PiSmmCpu: Correct exception message. 2016-11-22 11:47 [PATCH] UefiCpuPkg/PiSmmCpu: Correct exception message Jiewen Yao @ 2016-11-22 21:17 ` Laszlo Ersek 2016-11-23 0:38 ` Yao, Jiewen 0 siblings, 1 reply; 3+ messages in thread From: Laszlo Ersek @ 2016-11-22 21:17 UTC (permalink / raw) To: Jiewen Yao, edk2-devel; +Cc: Michael D Kinney, Jeff Fan On 11/22/16 12:47, Jiewen Yao wrote: > This patch fixes the first part of > https://bugzilla.tianocore.org/show_bug.cgi?id=242 > > Previously, when SMM exception happens, "stack overflow" is misreported. > This patch checked the PF address to see it is stack overflow, or > it is caused by SMM page protection. > > Cc: Laszlo Ersek <lersek@redhat.com> > Cc: Jeff Fan <jeff.fan@intel.com> > Cc: Michael D Kinney <michael.d.kinney@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Jiewen Yao <jiewen.yao@intel.com> > --- > UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c | 28 +++++++++++++++++--- > UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h | 9 +++++++ > UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 27 ++++++++++++++++--- > 3 files changed, 57 insertions(+), 7 deletions(-) > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c > index 5033bc5..feca142 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c > @@ -91,6 +91,8 @@ SmiPFHandler ( > ) > { > UINTN PFAddress; > + UINTN GuardPageAddress; > + UINTN CpuIndex; > > ASSERT (InterruptType == EXCEPT_IA32_PAGE_FAULT); > > @@ -98,10 +100,30 @@ SmiPFHandler ( > > PFAddress = AsmReadCr2 (); > > - if ((FeaturePcdGet (PcdCpuSmmStackGuard)) && > - (PFAddress >= mCpuHotPlugData.SmrrBase) && > + // > + // If a page fault occurs in SMRAM range, it might be in a SMM stack guard page, > + // or SMM page protection violation. > + // > + if ((PFAddress >= mCpuHotPlugData.SmrrBase) && > (PFAddress < (mCpuHotPlugData.SmrrBase + mCpuHotPlugData.SmrrSize))) { > - DEBUG ((DEBUG_ERROR, "SMM stack overflow!\n")); > + CpuIndex = GetCpuIndex (); > + GuardPageAddress = (mSmmStackArrayBase + EFI_PAGE_SIZE + CpuIndex * mSmmStackSize); > + if ((FeaturePcdGet (PcdCpuSmmStackGuard)) && > + (PFAddress >= GuardPageAddress) && > + (PFAddress < (GuardPageAddress + EFI_PAGE_SIZE))) { > + DEBUG ((DEBUG_ERROR, "SMM stack overflow!\n")); > + } > + if ((SystemContext.SystemContextIa32->ExceptionData & IA32_PF_EC_ID) != 0) { > + DEBUG ((DEBUG_ERROR, "SMM exception at execution (0x%lx)\n", PFAddress)); > + DEBUG_CODE ( > + DumpModuleInfoByIp (*(UINTN *)(UINTN)SystemContext.SystemContextIa32->Esp); > + ); > + } else { > + DEBUG ((DEBUG_ERROR, "SMM exception at write (0x%lx)\n", PFAddress)); > + DEBUG_CODE ( > + DumpModuleInfoByIp ((UINTN)SystemContext.SystemContextIa32->Eip); > + ); > + } > CpuDeadLoop (); > } > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h > index b6fb5cf..04a3dfb 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h > @@ -105,6 +105,15 @@ InitPaging ( > VOID > ); > > +/** > + Get CPU Index from APIC ID. > + > +**/ > +UINTN > +GetCpuIndex ( > + VOID > + ); > + > // > // The flag indicates if execute-disable is supported by processor. > // > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > index 531e188..ec8eab7 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > @@ -804,6 +804,8 @@ SmiPFHandler ( > ) > { > UINTN PFAddress; > + UINTN GuardPageAddress; > + UINTN CpuIndex; > > ASSERT (InterruptType == EXCEPT_IA32_PAGE_FAULT); > > @@ -817,12 +819,29 @@ SmiPFHandler ( > } > > // > - // If a page fault occurs in SMRAM range, it should be in a SMM stack guard page. > + // If a page fault occurs in SMRAM range, it might be in a SMM stack guard page, > + // or SMM page protection violation. > // > - if ((FeaturePcdGet (PcdCpuSmmStackGuard)) && > - (PFAddress >= mCpuHotPlugData.SmrrBase) && > + if ((PFAddress >= mCpuHotPlugData.SmrrBase) && > (PFAddress < (mCpuHotPlugData.SmrrBase + mCpuHotPlugData.SmrrSize))) { > - DEBUG ((DEBUG_ERROR, "SMM stack overflow!\n")); > + CpuIndex = GetCpuIndex (); > + GuardPageAddress = (mSmmStackArrayBase + EFI_PAGE_SIZE + CpuIndex * mSmmStackSize); > + if ((FeaturePcdGet (PcdCpuSmmStackGuard)) && > + (PFAddress >= GuardPageAddress) && > + (PFAddress < (GuardPageAddress + EFI_PAGE_SIZE))) { > + DEBUG ((DEBUG_ERROR, "SMM stack overflow!\n")); > + } > + if ((SystemContext.SystemContextX64->ExceptionData & IA32_PF_EC_ID) != 0) { > + DEBUG ((DEBUG_ERROR, "SMM exception at execution (0x%lx)\n", PFAddress)); > + DEBUG_CODE ( > + DumpModuleInfoByIp (*(UINTN *)(UINTN)SystemContext.SystemContextX64->Rsp); > + ); > + } else { > + DEBUG ((DEBUG_ERROR, "SMM exception at write (0x%lx)\n", PFAddress)); > + DEBUG_CODE ( > + DumpModuleInfoByIp ((UINTN)SystemContext.SystemContextX64->Rip); > + ); > + } > CpuDeadLoop (); > } > > (1) The "PFAddress" variable is UINTN in both variants. Printing UINTN with %lx is incorrect in the Ia32 case, because %lx takes an UINT64. I suggest the following pattern for printing UINTN values portably: DEBUG ((level, "%lx", (UINT64)Value)); That is, always use %lx and add an explicit cast. The cast is a no-op on X64, and does the right conversion on Ia32. The %lx conversion specification matches the result in both cases. (2) I tested the X64 stack overflow branch as follows: I temporarily reverted (on top of your present patch) commit 0d0c245dfb147 ("OvmfPkg: set SMM stack size to 16KB"), and then ran the certificate enrollment application that originally triggered the stack overflow. This is the debug output I got: > SMM stack overflow! > SMM exception at write (0x7FF9CFEC) > It is invoked from the instruction before IP(0x7FFE4CA3) in module (.../Build/Ovmf3264/NOOPT_GCC48/X64/MdeModulePkg/Core/PiSmmCore/PiSmmCore/DEBUG/PiSmmCore.dll) Shouldn't you change the IA32_PF_EC_ID check into an "else if"? Because I think once you determine the stack overflow, we shouldn't look for any other kind of exception. (3) I tested the Ia32 execution / write fault branch as follows: I temporarily reverted (on top of your present patch) commit 750ec4cabd07 ("UefiCpuPkg/PiSmmCpu: Check XdSupport before set NX."). Then, under the circumstances I reported in <https://lists.01.org/pipermail/edk2-devel/2016-November/004929.html>, I get: > ConvertPageEntryAttribute 0x7FEA4067->0x800000007FEA4067 > SMM exception at write (0x7FEA4890) > It is invoked from the instruction before IP(0x7FFB879A) in module (.../Build/OvmfIa32/NOOPT_GCC48/IA32/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm/DEBUG/PiSmmCpuDxeSmm.dll) Here the page fault is explained by the fact that we set the unsupported NX bit in the PTE that maps the page, and then we try to read from the page (not write to it). If I remember correctly at least. Is it possible to distinguish "read" from "write" in the fault symptoms? If it is, then I suggest to customize the error message. If it is not possible, then I suggest to replace the word "write" with "access". Thanks! Laszlo ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] UefiCpuPkg/PiSmmCpu: Correct exception message. 2016-11-22 21:17 ` Laszlo Ersek @ 2016-11-23 0:38 ` Yao, Jiewen 0 siblings, 0 replies; 3+ messages in thread From: Yao, Jiewen @ 2016-11-23 0:38 UTC (permalink / raw) To: Laszlo Ersek, edk2-devel@ml01.01.org; +Cc: Kinney, Michael D, Fan, Jeff Yes. You are right. I agree all. Will send out V2 patch. From: Laszlo Ersek [mailto:lersek@redhat.com] Sent: Wednesday, November 23, 2016 5:17 AM To: Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@ml01.01.org Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Fan, Jeff <jeff.fan@intel.com> Subject: Re: [edk2] [PATCH] UefiCpuPkg/PiSmmCpu: Correct exception message. On 11/22/16 12:47, Jiewen Yao wrote: > This patch fixes the first part of > https://bugzilla.tianocore.org/show_bug.cgi?id=242 > > Previously, when SMM exception happens, "stack overflow" is misreported. > This patch checked the PF address to see it is stack overflow, or > it is caused by SMM page protection. > > Cc: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>> > Cc: Jeff Fan <jeff.fan@intel.com<mailto:jeff.fan@intel.com>> > Cc: Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> > --- > UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c | 28 +++++++++++++++++--- > UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h | 9 +++++++ > UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 27 ++++++++++++++++--- > 3 files changed, 57 insertions(+), 7 deletions(-) > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c > index 5033bc5..feca142 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c > @@ -91,6 +91,8 @@ SmiPFHandler ( > ) > { > UINTN PFAddress; > + UINTN GuardPageAddress; > + UINTN CpuIndex; > > ASSERT (InterruptType == EXCEPT_IA32_PAGE_FAULT); > > @@ -98,10 +100,30 @@ SmiPFHandler ( > > PFAddress = AsmReadCr2 (); > > - if ((FeaturePcdGet (PcdCpuSmmStackGuard)) && > - (PFAddress >= mCpuHotPlugData.SmrrBase) && > + // > + // If a page fault occurs in SMRAM range, it might be in a SMM stack guard page, > + // or SMM page protection violation. > + // > + if ((PFAddress >= mCpuHotPlugData.SmrrBase) && > (PFAddress < (mCpuHotPlugData.SmrrBase + mCpuHotPlugData.SmrrSize))) { > - DEBUG ((DEBUG_ERROR, "SMM stack overflow!\n")); > + CpuIndex = GetCpuIndex (); > + GuardPageAddress = (mSmmStackArrayBase + EFI_PAGE_SIZE + CpuIndex * mSmmStackSize); > + if ((FeaturePcdGet (PcdCpuSmmStackGuard)) && > + (PFAddress >= GuardPageAddress) && > + (PFAddress < (GuardPageAddress + EFI_PAGE_SIZE))) { > + DEBUG ((DEBUG_ERROR, "SMM stack overflow!\n")); > + } > + if ((SystemContext.SystemContextIa32->ExceptionData & IA32_PF_EC_ID) != 0) { > + DEBUG ((DEBUG_ERROR, "SMM exception at execution (0x%lx)\n", PFAddress)); > + DEBUG_CODE ( > + DumpModuleInfoByIp (*(UINTN *)(UINTN)SystemContext.SystemContextIa32->Esp); > + ); > + } else { > + DEBUG ((DEBUG_ERROR, "SMM exception at write (0x%lx)\n", PFAddress)); > + DEBUG_CODE ( > + DumpModuleInfoByIp ((UINTN)SystemContext.SystemContextIa32->Eip); > + ); > + } > CpuDeadLoop (); > } > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h > index b6fb5cf..04a3dfb 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h > @@ -105,6 +105,15 @@ InitPaging ( > VOID > ); > > +/** > + Get CPU Index from APIC ID. > + > +**/ > +UINTN > +GetCpuIndex ( > + VOID > + ); > + > // > // The flag indicates if execute-disable is supported by processor. > // > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > index 531e188..ec8eab7 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > @@ -804,6 +804,8 @@ SmiPFHandler ( > ) > { > UINTN PFAddress; > + UINTN GuardPageAddress; > + UINTN CpuIndex; > > ASSERT (InterruptType == EXCEPT_IA32_PAGE_FAULT); > > @@ -817,12 +819,29 @@ SmiPFHandler ( > } > > // > - // If a page fault occurs in SMRAM range, it should be in a SMM stack guard page. > + // If a page fault occurs in SMRAM range, it might be in a SMM stack guard page, > + // or SMM page protection violation. > // > - if ((FeaturePcdGet (PcdCpuSmmStackGuard)) && > - (PFAddress >= mCpuHotPlugData.SmrrBase) && > + if ((PFAddress >= mCpuHotPlugData.SmrrBase) && > (PFAddress < (mCpuHotPlugData.SmrrBase + mCpuHotPlugData.SmrrSize))) { > - DEBUG ((DEBUG_ERROR, "SMM stack overflow!\n")); > + CpuIndex = GetCpuIndex (); > + GuardPageAddress = (mSmmStackArrayBase + EFI_PAGE_SIZE + CpuIndex * mSmmStackSize); > + if ((FeaturePcdGet (PcdCpuSmmStackGuard)) && > + (PFAddress >= GuardPageAddress) && > + (PFAddress < (GuardPageAddress + EFI_PAGE_SIZE))) { > + DEBUG ((DEBUG_ERROR, "SMM stack overflow!\n")); > + } > + if ((SystemContext.SystemContextX64->ExceptionData & IA32_PF_EC_ID) != 0) { > + DEBUG ((DEBUG_ERROR, "SMM exception at execution (0x%lx)\n", PFAddress)); > + DEBUG_CODE ( > + DumpModuleInfoByIp (*(UINTN *)(UINTN)SystemContext.SystemContextX64->Rsp); > + ); > + } else { > + DEBUG ((DEBUG_ERROR, "SMM exception at write (0x%lx)\n", PFAddress)); > + DEBUG_CODE ( > + DumpModuleInfoByIp ((UINTN)SystemContext.SystemContextX64->Rip); > + ); > + } > CpuDeadLoop (); > } > > (1) The "PFAddress" variable is UINTN in both variants. Printing UINTN with %lx is incorrect in the Ia32 case, because %lx takes an UINT64. I suggest the following pattern for printing UINTN values portably: DEBUG ((level, "%lx", (UINT64)Value)); That is, always use %lx and add an explicit cast. The cast is a no-op on X64, and does the right conversion on Ia32. The %lx conversion specification matches the result in both cases. (2) I tested the X64 stack overflow branch as follows: I temporarily reverted (on top of your present patch) commit 0d0c245dfb147 ("OvmfPkg: set SMM stack size to 16KB"), and then ran the certificate enrollment application that originally triggered the stack overflow. This is the debug output I got: > SMM stack overflow! > SMM exception at write (0x7FF9CFEC) > It is invoked from the instruction before IP(0x7FFE4CA3) in module (.../Build/Ovmf3264/NOOPT_GCC48/X64/MdeModulePkg/Core/PiSmmCore/PiSmmCore/DEBUG/PiSmmCore.dll) Shouldn't you change the IA32_PF_EC_ID check into an "else if"? Because I think once you determine the stack overflow, we shouldn't look for any other kind of exception. (3) I tested the Ia32 execution / write fault branch as follows: I temporarily reverted (on top of your present patch) commit 750ec4cabd07 ("UefiCpuPkg/PiSmmCpu: Check XdSupport before set NX."). Then, under the circumstances I reported in <https://lists.01.org/pipermail/edk2-devel/2016-November/004929.html>, I get: > ConvertPageEntryAttribute 0x7FEA4067->0x800000007FEA4067 > SMM exception at write (0x7FEA4890) > It is invoked from the instruction before IP(0x7FFB879A) in module (.../Build/OvmfIa32/NOOPT_GCC48/IA32/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm/DEBUG/PiSmmCpuDxeSmm.dll) Here the page fault is explained by the fact that we set the unsupported NX bit in the PTE that maps the page, and then we try to read from the page (not write to it). If I remember correctly at least. Is it possible to distinguish "read" from "write" in the fault symptoms? If it is, then I suggest to customize the error message. If it is not possible, then I suggest to replace the word "write" with "access". Thanks! Laszlo ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-11-23 0:38 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-11-22 11:47 [PATCH] UefiCpuPkg/PiSmmCpu: Correct exception message Jiewen Yao 2016-11-22 21:17 ` Laszlo Ersek 2016-11-23 0:38 ` Yao, Jiewen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox