[PATCH 0/3] Display new stack base and size

[PATCH 0/3] Display new stack base and size

[Jeff Fan]

Dump new stack base and size could help developer to fix stack crash 
issue. 
Normally, stack is changed by EnablePaging64()/DisablePaging64()/SwitchStack().
But these APIs have no knowledge of stack size. It's better to let caller
display the stack base and size informations. Some modules alreadys displayed
stack information. This serial of patches are to fix those modules missing it.

We also fixed one bug in DxeIplPeim, local BaseOfStack is overwritten wrongly. 

https://bugzilla.tianocore.org/show_bug.cgi?id=186

Jeff Fan (3):
  MdeModulePkg: Display new stack base and size
  UefiCpuPkg: Display new stack base and size
  MdeModulePkg/DxeIplPeim: UINTN used wrongly for EFI_PHYSICAL_ADDRESS

 MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c   | 19 ++++++++++++++++++-
 MdeModulePkg/Universal/CapsulePei/UefiCapsule.c   |  8 ++++++++
 MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c  |  8 ++++++++
 UefiCpuPkg/SecCore/SecMain.c                      | 10 +++++++++-
 UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c |  7 +++++++
 5 files changed, 50 insertions(+), 2 deletions(-)

-- 
2.9.3.windows.2

[PATCH 1/3] MdeModulePkg: Display new stack base and size

[Jeff Fan]

Dump new stack base and size information could help developer to narrow down
stack crash issue.

Cc: Feng Tian <feng.tian@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan <jeff.fan@intel.com>
---
 MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c  | 16 ++++++++++++++++
 MdeModulePkg/Universal/CapsulePei/UefiCapsule.c  |  8 ++++++++
 MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c |  8 ++++++++
 3 files changed, 32 insertions(+)

diff --git a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
index 8ce72cb..6ec51ff 100644
--- a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
+++ b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
@@ -317,6 +317,14 @@ HandOffToDxeCore (
 
     AsmWriteIdtr (&gLidtDescriptor);
 
+    DEBUG ((
+      DEBUG_INFO,
+      "%a() Stack Base: 0x%lx, Stack Size: 0x%x\n",
+      __FUNCTION__,
+      BaseOfStack,
+      STACK_SIZE
+      ));
+
     //
     // Go to Long Mode and transfer control to DxeCore.
     // Interrupts will not get turned on until the CPU AP is loaded.
@@ -387,6 +395,14 @@ HandOffToDxeCore (
     //
     UpdateStackHob (BaseOfStack, STACK_SIZE);
 
+    DEBUG ((
+      DEBUG_INFO,
+      "%a() Stack Base: 0x%lx, Stack Size: 0x%x\n",
+      __FUNCTION__,
+      BaseOfStack,
+      STACK_SIZE
+      ));
+
     //
     // Transfer the control to the entry point of DxeCore.
     //
diff --git a/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c b/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c
index e60105b..9ac9d22 100644
--- a/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c
+++ b/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c
@@ -321,6 +321,14 @@ Thunk32To64 (
     //
     AsmWriteCr3 ((UINTN) PageTableAddress);
 
+    DEBUG ((
+      DEBUG_INFO,
+      "%a() Stack Base: 0x%lx, Stack Size: 0x%lx\n",
+      __FUNCTION__,
+      Context->StackBufferBase,
+      Context->StackBufferLength
+      ));
+
     //
     // Disable interrupt of Debug timer, since the IDT table cannot work in long mode
     //
diff --git a/MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c b/MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c
index d1042e3..5ad95d2 100644
--- a/MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c
+++ b/MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c
@@ -265,6 +265,14 @@ _ModuleEntryPoint (
   
   ReturnContext->ReturnStatus = Status;
 
+  DEBUG ((
+    DEBUG_INFO,
+    "%a() Stack Base: 0x%lx, Stack Size: 0x%lx\n",
+    __FUNCTION__,
+    EntrypointContext->StackBufferBase,
+    EntrypointContext->StackBufferLength
+    ));
+
   //
   // Disable interrupt of Debug timer, since the new IDT table cannot work in long mode
   //
-- 
2.9.3.windows.2

[PATCH 2/3] UefiCpuPkg: Display new stack base and size

[Jeff Fan]

Dump new stack base and size information could help developer to narrow down
stack crash issue.

Cc: Feng Tian <feng.tian@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan <jeff.fan@intel.com>
---
 UefiCpuPkg/SecCore/SecMain.c                      | 10 +++++++++-
 UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c |  7 +++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/SecCore/SecMain.c b/UefiCpuPkg/SecCore/SecMain.c
index af1e661..2ebbc22 100644
--- a/UefiCpuPkg/SecCore/SecMain.c
+++ b/UefiCpuPkg/SecCore/SecMain.c
@@ -1,7 +1,7 @@
 /** @file
   C functions in SEC
 
-  Copyright (c) 2008 - 2015, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2008 - 2016, Intel Corporation. All rights reserved.<BR>
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD License
   which accompanies this distribution.  The full text of the license may be found at
@@ -237,6 +237,14 @@ SecStartupPhase2(
     PpiList = &mPeiSecPlatformInformationPpi[0];
   }
 
+  DEBUG ((
+    DEBUG_INFO,
+    "%a() Stack Base: 0x%lx, Stack Size: 0x%lx\n",
+    __FUNCTION__,
+    SecCoreData->StackBase,
+    SecCoreData->StackSize
+    ));
+
   //
   // Report Status Code to indicate transferring to PEI core
   //
diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
index f907b30..d306fba 100644
--- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
+++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
@@ -515,6 +515,13 @@ S3ResumeBootOs (
     // Switch to native waking vector
     //
     TempStackTop = (UINTN)&TempStack + sizeof(TempStack);
+    DEBUG ((
+      DEBUG_INFO,
+      "%a() Stack Base: 0x%x, Stack Size: 0x%x\n",
+      __FUNCTION__,
+      TempStackTop,
+      sizeof (TempStack)
+      ));
     if ((Facs->Version == EFI_ACPI_4_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_VERSION) &&
         ((Facs->Flags & EFI_ACPI_4_0_64BIT_WAKE_SUPPORTED_F) != 0) &&
         ((Facs->Flags & EFI_ACPI_4_0_OSPM_64BIT_WAKE__F) != 0)) {
-- 
2.9.3.windows.2

[PATCH 3/3] MdeModulePkg/DxeIplPeim: UINTN used wrongly for EFI_PHYSICAL_ADDRESS

[Jeff Fan]

PeiServicesAllocatePages () will output sizeof (EFI_PHYSICAL_ADDRESS) value.
IdtTableForX64 is sizeof (UINTN) local variable. It will overwrite other local
variable.

This issue is found when we dump BaseOfStack value.

Cc: Feng Tian <feng.tian@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan <jeff.fan@intel.com>
---
 MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
index 6ec51ff..8f6a97a 100644
--- a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
+++ b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
@@ -280,7 +280,7 @@ HandOffToDxeCore (
     Status = PeiServicesAllocatePages (
                EfiBootServicesData,
                EFI_SIZE_TO_PAGES(sizeof (X64_IDT_TABLE) + SizeOfTemplate * IDT_ENTRY_COUNT),
-               (EFI_PHYSICAL_ADDRESS *) &IdtTableForX64
+               &VectorAddress
                );
     ASSERT_EFI_ERROR (Status);
 
@@ -288,6 +288,7 @@ HandOffToDxeCore (
     // Store EFI_PEI_SERVICES** in the 4 bytes immediately preceding IDT to avoid that
     // it may not be gotten correctly after IDT register is re-written.
     //
+    IdtTableForX64 = (X64_IDT_TABLE *) (UINTN) VectorAddress;
     IdtTableForX64->PeiService = GetPeiServicesTablePointer ();
 
     VectorAddress = (EFI_PHYSICAL_ADDRESS) (UINTN) (IdtTableForX64 + 1);
-- 
2.9.3.windows.2

Re: [PATCH 3/3] MdeModulePkg/DxeIplPeim: UINTN used wrongly for EFI_PHYSICAL_ADDRESS

[Gao, Liming]

Reviewed-by: Liming Gao <liming.gao@intel.com>

> -----Original Message-----
> From: Fan, Jeff
> Sent: Friday, November 04, 2016 10:19 AM
> To: edk2-devel@lists.01.org
> Cc: Tian, Feng <feng.tian@intel.com>; Gao, Liming <liming.gao@intel.com>;
> Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: [PATCH 3/3] MdeModulePkg/DxeIplPeim: UINTN used wrongly for
> EFI_PHYSICAL_ADDRESS
> 
> PeiServicesAllocatePages () will output sizeof (EFI_PHYSICAL_ADDRESS)
> value.
> IdtTableForX64 is sizeof (UINTN) local variable. It will overwrite other local
> variable.
> 
> This issue is found when we dump BaseOfStack value.
> 
> Cc: Feng Tian <feng.tian@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael Kinney <michael.d.kinney@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Fan <jeff.fan@intel.com>
> ---
>  MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
> b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
> index 6ec51ff..8f6a97a 100644
> --- a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
> +++ b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
> @@ -280,7 +280,7 @@ HandOffToDxeCore (
>      Status = PeiServicesAllocatePages (
>                 EfiBootServicesData,
>                 EFI_SIZE_TO_PAGES(sizeof (X64_IDT_TABLE) + SizeOfTemplate *
> IDT_ENTRY_COUNT),
> -               (EFI_PHYSICAL_ADDRESS *) &IdtTableForX64
> +               &VectorAddress
>                 );
>      ASSERT_EFI_ERROR (Status);
> 
> @@ -288,6 +288,7 @@ HandOffToDxeCore (
>      // Store EFI_PEI_SERVICES** in the 4 bytes immediately preceding IDT to
> avoid that
>      // it may not be gotten correctly after IDT register is re-written.
>      //
> +    IdtTableForX64 = (X64_IDT_TABLE *) (UINTN) VectorAddress;
>      IdtTableForX64->PeiService = GetPeiServicesTablePointer ();
> 
>      VectorAddress = (EFI_PHYSICAL_ADDRESS) (UINTN) (IdtTableForX64 + 1);
> --
> 2.9.3.windows.2

Re: [PATCH 3/3] MdeModulePkg/DxeIplPeim: UINTN used wrongly for EFI_PHYSICAL_ADDRESS

[Gao, Liming]

Reviewed-by: Liming Gao <liming.gao@intel.com>

> -----Original Message-----
> From: Fan, Jeff
> Sent: Friday, November 04, 2016 10:19 AM
> To: edk2-devel@lists.01.org
> Cc: Tian, Feng <feng.tian@intel.com>; Gao, Liming <liming.gao@intel.com>;
> Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: [PATCH 3/3] MdeModulePkg/DxeIplPeim: UINTN used wrongly for
> EFI_PHYSICAL_ADDRESS
> 
> PeiServicesAllocatePages () will output sizeof (EFI_PHYSICAL_ADDRESS)
> value.
> IdtTableForX64 is sizeof (UINTN) local variable. It will overwrite other local
> variable.
> 
> This issue is found when we dump BaseOfStack value.
> 
> Cc: Feng Tian <feng.tian@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael Kinney <michael.d.kinney@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Fan <jeff.fan@intel.com>
> ---
>  MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
> b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
> index 6ec51ff..8f6a97a 100644
> --- a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
> +++ b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
> @@ -280,7 +280,7 @@ HandOffToDxeCore (
>      Status = PeiServicesAllocatePages (
>                 EfiBootServicesData,
>                 EFI_SIZE_TO_PAGES(sizeof (X64_IDT_TABLE) + SizeOfTemplate *
> IDT_ENTRY_COUNT),
> -               (EFI_PHYSICAL_ADDRESS *) &IdtTableForX64
> +               &VectorAddress
>                 );
>      ASSERT_EFI_ERROR (Status);
> 
> @@ -288,6 +288,7 @@ HandOffToDxeCore (
>      // Store EFI_PEI_SERVICES** in the 4 bytes immediately preceding IDT to
> avoid that
>      // it may not be gotten correctly after IDT register is re-written.
>      //
> +    IdtTableForX64 = (X64_IDT_TABLE *) (UINTN) VectorAddress;
>      IdtTableForX64->PeiService = GetPeiServicesTablePointer ();
> 
>      VectorAddress = (EFI_PHYSICAL_ADDRESS) (UINTN) (IdtTableForX64 + 1);
> --
> 2.9.3.windows.2

Re: [PATCH 0/3] Display new stack base and size

[Tian, Feng]

Series Reviewed-by: Feng Tian <feng.tian@Intel.com>

Thanks
Feng

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jeff Fan
Sent: Friday, November 4, 2016 10:19 AM
To: edk2-devel@lists.01.org
Subject: [edk2] [PATCH 0/3] Display new stack base and size

Dump new stack base and size could help developer to fix stack crash issue. 
Normally, stack is changed by EnablePaging64()/DisablePaging64()/SwitchStack().
But these APIs have no knowledge of stack size. It's better to let caller display the stack base and size informations. Some modules alreadys displayed stack information. This serial of patches are to fix those modules missing it.

We also fixed one bug in DxeIplPeim, local BaseOfStack is overwritten wrongly. 

https://bugzilla.tianocore.org/show_bug.cgi?id=186

Jeff Fan (3):
  MdeModulePkg: Display new stack base and size
  UefiCpuPkg: Display new stack base and size
  MdeModulePkg/DxeIplPeim: UINTN used wrongly for EFI_PHYSICAL_ADDRESS

 MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c   | 19 ++++++++++++++++++-
 MdeModulePkg/Universal/CapsulePei/UefiCapsule.c   |  8 ++++++++
 MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c  |  8 ++++++++
 UefiCpuPkg/SecCore/SecMain.c                      | 10 +++++++++-
 UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c |  7 +++++++
 5 files changed, 50 insertions(+), 2 deletions(-)

--
2.9.3.windows.2

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel