* [PATCH V3] UefiCpuPkg/PiSmmCpu: Correct exception message.
@ 2016-11-23 14:26 Jiewen Yao
2016-11-23 16:37 ` Laszlo Ersek
0 siblings, 1 reply; 3+ messages in thread
From: Jiewen Yao @ 2016-11-23 14:26 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.
It dumps exception data, PF address and the module trigger the issue.
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 | 38 ++++++++++++++++++--
UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h | 9 +++++
UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 37 ++++++++++++++++---
3 files changed, 77 insertions(+), 7 deletions(-)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
index 5033bc5..39e6c9a 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,40 @@ 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"));
+ } else {
+ DEBUG ((DEBUG_ERROR, "SMM exception data - 0x%x(", SystemContext.SystemContextIa32->ExceptionData));
+ DEBUG ((DEBUG_ERROR, "I:%x, R:%x, U:%x, W:%X, P:%x",
+ (SystemContext.SystemContextIa32->ExceptionData & IA32_PF_EC_ID) != 0,
+ (SystemContext.SystemContextIa32->ExceptionData & IA32_PF_EC_RSVD) != 0,
+ (SystemContext.SystemContextIa32->ExceptionData & IA32_PF_EC_US) != 0,
+ (SystemContext.SystemContextIa32->ExceptionData & IA32_PF_EC_WR) != 0,
+ (SystemContext.SystemContextIa32->ExceptionData & IA32_PF_EC_P) != 0
+ ));
+ DEBUG ((DEBUG_ERROR, ")\n", SystemContext.SystemContextIa32->ExceptionData));
+ if ((SystemContext.SystemContextIa32->ExceptionData & IA32_PF_EC_ID) != 0) {
+ DEBUG ((DEBUG_ERROR, "SMM exception at execution (0x%x)\n", PFAddress));
+ DEBUG_CODE (
+ DumpModuleInfoByIp (*(UINTN *)(UINTN)SystemContext.SystemContextIa32->Esp);
+ );
+ } else {
+ DEBUG ((DEBUG_ERROR, "SMM exception at access (0x%x)\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..94f2e03 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,39 @@ 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"));
+ } else {
+ DEBUG ((DEBUG_ERROR, "SMM exception data - 0x%lx(", SystemContext.SystemContextX64->ExceptionData));
+ DEBUG ((DEBUG_ERROR, "I:%x, R:%x, U:%x, W:%X, P:%x",
+ (SystemContext.SystemContextX64->ExceptionData & IA32_PF_EC_ID) != 0,
+ (SystemContext.SystemContextX64->ExceptionData & IA32_PF_EC_RSVD) != 0,
+ (SystemContext.SystemContextX64->ExceptionData & IA32_PF_EC_US) != 0,
+ (SystemContext.SystemContextX64->ExceptionData & IA32_PF_EC_WR) != 0,
+ (SystemContext.SystemContextX64->ExceptionData & IA32_PF_EC_P) != 0
+ ));
+ DEBUG ((DEBUG_ERROR, ")\n", SystemContext.SystemContextX64->ExceptionData));
+ 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 access (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 V3] UefiCpuPkg/PiSmmCpu: Correct exception message.
2016-11-23 14:26 [PATCH V3] UefiCpuPkg/PiSmmCpu: Correct exception message Jiewen Yao
@ 2016-11-23 16:37 ` Laszlo Ersek
2016-11-24 2:01 ` Fan, Jeff
0 siblings, 1 reply; 3+ messages in thread
From: Laszlo Ersek @ 2016-11-23 16:37 UTC (permalink / raw)
To: Jiewen Yao, edk2-devel; +Cc: Jeff Fan, Michael D Kinney
On 11/23/16 15:26, 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.
>
> It dumps exception data, PF address and the module trigger the issue.
>
> 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 | 38 ++++++++++++++++++--
> UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h | 9 +++++
> UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 37 ++++++++++++++++---
> 3 files changed, 77 insertions(+), 7 deletions(-)
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
> index 5033bc5..39e6c9a 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,40 @@ 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"));
> + } else {
> + DEBUG ((DEBUG_ERROR, "SMM exception data - 0x%x(", SystemContext.SystemContextIa32->ExceptionData));
> + DEBUG ((DEBUG_ERROR, "I:%x, R:%x, U:%x, W:%X, P:%x",
> + (SystemContext.SystemContextIa32->ExceptionData & IA32_PF_EC_ID) != 0,
> + (SystemContext.SystemContextIa32->ExceptionData & IA32_PF_EC_RSVD) != 0,
> + (SystemContext.SystemContextIa32->ExceptionData & IA32_PF_EC_US) != 0,
> + (SystemContext.SystemContextIa32->ExceptionData & IA32_PF_EC_WR) != 0,
> + (SystemContext.SystemContextIa32->ExceptionData & IA32_PF_EC_P) != 0
> + ));
> + DEBUG ((DEBUG_ERROR, ")\n", SystemContext.SystemContextIa32->ExceptionData));
The last argument in this DEBUG call can be removed. It causes no bugs
(it is simply ignored), but it would be nice to remove it.
No need to repost just for this.
> + if ((SystemContext.SystemContextIa32->ExceptionData & IA32_PF_EC_ID) != 0) {
> + DEBUG ((DEBUG_ERROR, "SMM exception at execution (0x%x)\n", PFAddress));
> + DEBUG_CODE (
> + DumpModuleInfoByIp (*(UINTN *)(UINTN)SystemContext.SystemContextIa32->Esp);
> + );
> + } else {
> + DEBUG ((DEBUG_ERROR, "SMM exception at access (0x%x)\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..94f2e03 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,39 @@ 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"));
> + } else {
> + DEBUG ((DEBUG_ERROR, "SMM exception data - 0x%lx(", SystemContext.SystemContextX64->ExceptionData));
> + DEBUG ((DEBUG_ERROR, "I:%x, R:%x, U:%x, W:%X, P:%x",
> + (SystemContext.SystemContextX64->ExceptionData & IA32_PF_EC_ID) != 0,
> + (SystemContext.SystemContextX64->ExceptionData & IA32_PF_EC_RSVD) != 0,
> + (SystemContext.SystemContextX64->ExceptionData & IA32_PF_EC_US) != 0,
> + (SystemContext.SystemContextX64->ExceptionData & IA32_PF_EC_WR) != 0,
> + (SystemContext.SystemContextX64->ExceptionData & IA32_PF_EC_P) != 0
> + ));
> + DEBUG ((DEBUG_ERROR, ")\n", SystemContext.SystemContextX64->ExceptionData));
Same comment as above: the last argument is superfluous.
> + 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 access (0x%lx)\n", PFAddress));
> + DEBUG_CODE (
> + DumpModuleInfoByIp ((UINTN)SystemContext.SystemContextX64->Rip);
> + );
> + }
> + }
> CpuDeadLoop ();
> }
>
>
Another thing I noticed: in the following format string, which is used
in two places:
"I:%x, R:%x, U:%x, W:%X, P:%x"
^
|
the %X that stands after W is upper-case, while the rest is lower-case.
It is entirely correct of course, but you might want to lower-case it
for consistency, before pushing the patch.
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
Very nice patch, thank you for it!
Laszlo
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH V3] UefiCpuPkg/PiSmmCpu: Correct exception message.
2016-11-23 16:37 ` Laszlo Ersek
@ 2016-11-24 2:01 ` Fan, Jeff
0 siblings, 0 replies; 3+ messages in thread
From: Fan, Jeff @ 2016-11-24 2:01 UTC (permalink / raw)
To: Laszlo Ersek, Yao, Jiewen, edk2-devel@ml01.01.org; +Cc: Kinney, Michael D
Reviewed-by: Jeff Fan <jeff.fan@intel.com> with Laszlo's comment.
-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com]
Sent: Thursday, November 24, 2016 12:38 AM
To: Yao, Jiewen; edk2-devel@ml01.01.org
Cc: Fan, Jeff; Kinney, Michael D
Subject: Re: [PATCH V3] UefiCpuPkg/PiSmmCpu: Correct exception message.
On 11/23/16 15:26, 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.
>
> It dumps exception data, PF address and the module trigger the issue.
>
> 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 | 38 ++++++++++++++++++--
> UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h | 9 +++++
> UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 37 ++++++++++++++++---
> 3 files changed, 77 insertions(+), 7 deletions(-)
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
> index 5033bc5..39e6c9a 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,40 @@ 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"));
> + } else {
> + DEBUG ((DEBUG_ERROR, "SMM exception data - 0x%x(", SystemContext.SystemContextIa32->ExceptionData));
> + DEBUG ((DEBUG_ERROR, "I:%x, R:%x, U:%x, W:%X, P:%x",
> + (SystemContext.SystemContextIa32->ExceptionData & IA32_PF_EC_ID) != 0,
> + (SystemContext.SystemContextIa32->ExceptionData & IA32_PF_EC_RSVD) != 0,
> + (SystemContext.SystemContextIa32->ExceptionData & IA32_PF_EC_US) != 0,
> + (SystemContext.SystemContextIa32->ExceptionData & IA32_PF_EC_WR) != 0,
> + (SystemContext.SystemContextIa32->ExceptionData & IA32_PF_EC_P) != 0
> + ));
> + DEBUG ((DEBUG_ERROR, ")\n",
> + SystemContext.SystemContextIa32->ExceptionData));
The last argument in this DEBUG call can be removed. It causes no bugs (it is simply ignored), but it would be nice to remove it.
No need to repost just for this.
> + if ((SystemContext.SystemContextIa32->ExceptionData & IA32_PF_EC_ID) != 0) {
> + DEBUG ((DEBUG_ERROR, "SMM exception at execution (0x%x)\n", PFAddress));
> + DEBUG_CODE (
> + DumpModuleInfoByIp (*(UINTN *)(UINTN)SystemContext.SystemContextIa32->Esp);
> + );
> + } else {
> + DEBUG ((DEBUG_ERROR, "SMM exception at access (0x%x)\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..94f2e03 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,39 @@ 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"));
> + } else {
> + DEBUG ((DEBUG_ERROR, "SMM exception data - 0x%lx(", SystemContext.SystemContextX64->ExceptionData));
> + DEBUG ((DEBUG_ERROR, "I:%x, R:%x, U:%x, W:%X, P:%x",
> + (SystemContext.SystemContextX64->ExceptionData & IA32_PF_EC_ID) != 0,
> + (SystemContext.SystemContextX64->ExceptionData & IA32_PF_EC_RSVD) != 0,
> + (SystemContext.SystemContextX64->ExceptionData & IA32_PF_EC_US) != 0,
> + (SystemContext.SystemContextX64->ExceptionData & IA32_PF_EC_WR) != 0,
> + (SystemContext.SystemContextX64->ExceptionData & IA32_PF_EC_P) != 0
> + ));
> + DEBUG ((DEBUG_ERROR, ")\n",
> + SystemContext.SystemContextX64->ExceptionData));
Same comment as above: the last argument is superfluous.
> + 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 access (0x%lx)\n", PFAddress));
> + DEBUG_CODE (
> + DumpModuleInfoByIp ((UINTN)SystemContext.SystemContextX64->Rip);
> + );
> + }
> + }
> CpuDeadLoop ();
> }
>
>
Another thing I noticed: in the following format string, which is used in two places:
"I:%x, R:%x, U:%x, W:%X, P:%x"
^
|
the %X that stands after W is upper-case, while the rest is lower-case.
It is entirely correct of course, but you might want to lower-case it for consistency, before pushing the patch.
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
Very nice patch, thank you for it!
Laszlo
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-11-24 2:01 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-23 14:26 [PATCH V3] UefiCpuPkg/PiSmmCpu: Correct exception message Jiewen Yao
2016-11-23 16:37 ` Laszlo Ersek
2016-11-24 2:01 ` Fan, Jeff
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox