[edk2] [Patch v2] EmbeddedPkg: Correct the way of handling sections with a large size

2018-03-07 Thread Ge Song
Correct the way of handling EFI_SECTION_GUID_DEFINED type sections
with a large size

Cc: Leif Lindholm <leif.lindh...@linaro.org>
Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ge Song <ge.s...@hxt-semitech.com>
Reviewed-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
---
 
EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.c 
| 18 --
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git 
a/EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.c
 
b/EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.c
index 7b08de8ab9fe..e94f5424ef1d 100644
--- 
a/EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.c
+++ 
b/EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.c
@@ -123,6 +123,7 @@ ExtractGuidedSectionGetInfo (
 {
   PRE_PI_EXTRACT_GUIDED_SECTION_DATA  *SavedData;
   UINT32  Index;
+  EFI_GUID*SectionDefinitionGuid;
 
   if (InputSection == NULL) {
 return RETURN_INVALID_PARAMETER;
@@ -134,11 +135,17 @@ ExtractGuidedSectionGetInfo (
 
   SavedData = GetSavedData();
 
+  if (IS_SECTION2 (InputSection)) {
+SectionDefinitionGuid = &(((EFI_GUID_DEFINED_SECTION2 *) 
InputSection)->SectionDefinitionGuid);
+  } else {
+SectionDefinitionGuid = &(((EFI_GUID_DEFINED_SECTION *) 
InputSection)->SectionDefinitionGuid);
+  }
+
   //
   // Search the match registered GetInfo handler for the input guided section.
   //
   for (Index = 0; Index < SavedData->NumberOfExtractHandler; Index ++) {
-if (CompareGuid (>ExtractHandlerGuidTable[Index], 
&(((EFI_GUID_DEFINED_SECTION *) InputSection)->SectionDefinitionGuid))) {
+if (CompareGuid (>ExtractHandlerGuidTable[Index], 
SectionDefinitionGuid)) {
   break;
 }
   }
@@ -172,6 +179,7 @@ ExtractGuidedSectionDecode (
 {
   PRE_PI_EXTRACT_GUIDED_SECTION_DATA  *SavedData;
   UINT32  Index;
+  EFI_GUID*SectionDefinitionGuid;
 
   if (InputSection == NULL) {
 return RETURN_INVALID_PARAMETER;
@@ -182,11 +190,17 @@ ExtractGuidedSectionDecode (
 
   SavedData = GetSavedData();
 
+  if (IS_SECTION2 (InputSection)) {
+SectionDefinitionGuid = &(((EFI_GUID_DEFINED_SECTION2 *) 
InputSection)->SectionDefinitionGuid);
+  } else {
+SectionDefinitionGuid = &(((EFI_GUID_DEFINED_SECTION *) 
InputSection)->SectionDefinitionGuid);
+  }
+
   //
   // Search the match registered GetInfo handler for the input guided section.
   //
   for (Index = 0; Index < SavedData->NumberOfExtractHandler; Index ++) {
-if (CompareGuid (>ExtractHandlerGuidTable[Index], 
&(((EFI_GUID_DEFINED_SECTION *) InputSection)->SectionDefinitionGuid))) {
+if (CompareGuid (>ExtractHandlerGuidTable[Index], 
SectionDefinitionGuid)) {
   break;
 }
   }
-- 
2.11.0


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


[edk2] [Patch] EmbeddedPkg: Correct the way of handling sections with a large size

2018-03-05 Thread Ge Song
Correct the way of handling EFI_SECTION_GUID_DEFINED type sections
with a large size

Cc: Leif Lindholm <leif.lindh...@linaro.org>
Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ge Song <ge.s...@hxt-semitech.com>
---
 
EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.c 
| 18 --
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git 
a/EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.c
 
b/EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.c
index 7b08de8ab9fe..8e7abe202836 100644
--- 
a/EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.c
+++ 
b/EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.c
@@ -123,6 +123,7 @@ ExtractGuidedSectionGetInfo (
 {
   PRE_PI_EXTRACT_GUIDED_SECTION_DATA  *SavedData;
   UINT32  Index;
+  EFI_GUID *SectionDefinitionGuid;
 
   if (InputSection == NULL) {
 return RETURN_INVALID_PARAMETER;
@@ -134,11 +135,17 @@ ExtractGuidedSectionGetInfo (
 
   SavedData = GetSavedData();
 
+  if (IS_SECTION2 (InputSection)) {
+SectionDefinitionGuid = &(((EFI_GUID_DEFINED_SECTION2 *) 
InputSection)->SectionDefinitionGuid);
+  } else {
+SectionDefinitionGuid = &(((EFI_GUID_DEFINED_SECTION *) 
InputSection)->SectionDefinitionGuid);
+  }
+
   //
   // Search the match registered GetInfo handler for the input guided section.
   //
   for (Index = 0; Index < SavedData->NumberOfExtractHandler; Index ++) {
-if (CompareGuid (>ExtractHandlerGuidTable[Index], 
&(((EFI_GUID_DEFINED_SECTION *) InputSection)->SectionDefinitionGuid))) {
+if (CompareGuid (>ExtractHandlerGuidTable[Index], 
SectionDefinitionGuid)) {
   break;
 }
   }
@@ -172,6 +179,7 @@ ExtractGuidedSectionDecode (
 {
   PRE_PI_EXTRACT_GUIDED_SECTION_DATA  *SavedData;
   UINT32  Index;
+  EFI_GUID *SectionDefinitionGuid;
 
   if (InputSection == NULL) {
 return RETURN_INVALID_PARAMETER;
@@ -182,11 +190,17 @@ ExtractGuidedSectionDecode (
 
   SavedData = GetSavedData();
 
+  if (IS_SECTION2 (InputSection)) {
+SectionDefinitionGuid = &(((EFI_GUID_DEFINED_SECTION2 *) 
InputSection)->SectionDefinitionGuid);
+  } else {
+SectionDefinitionGuid = &(((EFI_GUID_DEFINED_SECTION *) 
InputSection)->SectionDefinitionGuid);
+  }
+
   //
   // Search the match registered GetInfo handler for the input guided section.
   //
   for (Index = 0; Index < SavedData->NumberOfExtractHandler; Index ++) {
-if (CompareGuid (>ExtractHandlerGuidTable[Index], 
&(((EFI_GUID_DEFINED_SECTION *) InputSection)->SectionDefinitionGuid))) {
+if (CompareGuid (>ExtractHandlerGuidTable[Index], 
SectionDefinitionGuid)) {
   break;
 }
   }
-- 
2.11.0


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


[edk2] [PATCH v2 1/1] OvmfPkg/SecMain: Fix stack switching to permanent memory

2017-09-05 Thread Ge Song
In earlier PEI stage, temporary memory at PcdOvmfSecPeiTempRamBase is
employed as stack and heap. We move them to the new room and do some
relocation fixup when permanent memory becomes available.
TemporaryRamMigration() is responsible for switching the stack.

Before entering TemporaryRamMigration(), Ebp/Rbp is populated with the
content of Esp/Rsp and used as frame pointer.

After the execution of SetJump/LongJump, stack migrates to new position
while the context keeps unchanged.

But when TemporaryRamMigration() exits, Esp/Rsp is filled with
the content of Ebp/Rbp to destroy this stack frame.

The result is, stack switches back to previous temporary memory.

When permanent memory becomes available, modules that have registered
themselves for shadowing will be scheduled to execute. Some of them
need to consume more memory(heap/stack). Contrast to temporary stack,
permanent stack possesses larger space.

The potential risk is overflowing the stack if stack staying in
temporary memory. When it happens, system may crash during S3 resume.

More detailed information:
> (gdb) disassemble /r
> Dump of assembler code for function TemporaryRamMigration:
>   0xfffcd29c <+0>:55  push   %rbp
>   0xfffcd29d <+1>:48 89 e5mov%rsp,%rbp
>   0xfffcd2a0 <+4>:48 81 ec 70 01 00 00sub
> $0x170,%rsp
>...
>...
>0xfffcd425 <+393>: e8 80 10 00 00  callq  0xfffce4aa
> 
> => 0xfffcd42a <+398>: b8 00 00 00 00  mov$0x0,%eax
>0xfffcd42f <+403>: c9  leaveq
>0xfffcd430 <+404>: c3  retq
> End of assembler dump.

See the description of leave(opcode: c9), from
Intel 64 and IA-32 Architectures Software Developer's Manual, Volume 2A

"Releases the stack frame set up by an earlier ENTER instruction. The
LEAVE instruction copies the frame pointer (in the EBP register) into
the stack pointer register (ESP), which releases the stack space
allocated to the stack frame. The old frame pointer (the frame pointer
for the calling procedure that was saved by the ENTER instruction) is
then popped from the stack into the EBP register, restoring the calling
procedure’s stack frame."

To solve this, update Ebp/Rbp too when Esp/Rsp is updated

Cc: Jordan Justen <jordan.l.jus...@intel.com>
Cc: Laszlo Ersek <ler...@redhat.com>
Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ge Song <songgeb...@gmail.com>
Tested-by: Laszlo Ersek <ler...@redhat.com>
Reviewed-by: Laszlo Ersek <ler...@redhat.com>
---
 OvmfPkg/Sec/SecMain.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
index e1993ec347b5..f7fec3d8c03b 100644
--- a/OvmfPkg/Sec/SecMain.c
+++ b/OvmfPkg/Sec/SecMain.c
@@ -931,9 +931,11 @@ TemporaryRamMigration (
   if (SetJump () == 0) {
 #if defined (MDE_CPU_IA32)
 JumpBuffer.Esp = JumpBuffer.Esp + DebugAgentContext.StackMigrateOffset;
+JumpBuffer.Ebp = JumpBuffer.Ebp + DebugAgentContext.StackMigrateOffset;
 #endif
 #if defined (MDE_CPU_X64)
 JumpBuffer.Rsp = JumpBuffer.Rsp + DebugAgentContext.StackMigrateOffset;
+JumpBuffer.Rbp = JumpBuffer.Rbp + DebugAgentContext.StackMigrateOffset;
 #endif
 LongJump (, (UINTN)-1);
   }
-- 
2.11.0

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


[edk2] [PATCH v2 0/1] Fix stack switching to permanent memory

2017-09-05 Thread Ge Song
Here are changes from v1 patch:
* Revise improper descriptions in subject and commit message
* Remove some debug information to compact the message
* Add a description of practical consequences of the bug

Repo:   https://github.com/sgbird/edk2.git
Branch: stack_switch

Best Regards,
Ge Song

Cc: Jordan Justen <jordan.l.jus...@intel.com>
Cc: Laszlo Ersek <ler...@redhat.com>
Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>

Ge Song (1):
  OvmfPkg/SecMain: Fix stack switching to permanent memory

 OvmfPkg/Sec/SecMain.c | 2 ++
 1 file changed, 2 insertions(+)

-- 
2.11.0

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


Re: [edk2] [PATCH v1 1/1] OvmfPkg/SecMain: Fix stack switching to permanent memory fail

2017-09-04 Thread Ge Song

On 09/04/2017 09:34 PM, Jordan Justen wrote:


On 2017-09-03 16:55:36, Laszlo Ersek wrote:

On 09/03/17 04:12, Ge Song wrote:

In earlier PEI stage, temporary memory(Cache As Ram) is employed as stack
and heap. We move them to the new room and do some relocation fixup when
permanent memory becomes available. TemporaryRamMigration()
is responsible for switching the stack.

[...]


Cc: Jordan Justen <jordan.l.jus...@intel.com>
Cc: Laszlo Ersek <ler...@redhat.com>
Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ge Song <ge.s...@hxt-semitech.com>
---
  OvmfPkg/Sec/SecMain.c | 2 ++
  1 file changed, 2 insertions(+)

(No more separate emails, I promise.)

Before you send v2, can you please amend the patch like this:

   git commit --amend --author='Ge Song <ge.s...@hxt-semitech.com>'

It's OK if you mail it out from another email address, but IMO the git
authorship should match the first Signed-off-by.

I disagree with this. Unless the individual is well known to the EDK
II community as the owner of both email addresses, I think the author
needs to send it from their author email.

I suppose they can send it from another email, and then reply from
their other email saying they contributed it under the Tianocore
Contribution Agreement, but that sounds like a stretch...

-Jordan


OK, I'll try to send it from the same email address next time, the reason
that using two different mail addresses is because the author email is an
exchange account and it's hard to find the mail server preferences for
git-send-email...




The commit message body currently does not start with "From: Ge Song
<ge.s...@hxt-semitech.com>", which means that the git authorship and the
first S-o-b disagree at the moment.

... If I'm wrong about this, I can be convinced, of course.

Thanks
Laszlo


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


Re: [edk2] [PATCH v1 1/1] OvmfPkg/SecMain: Fix stack switching to permanent memory fail

2017-09-04 Thread Ge Song

On 09/04/2017 04:49 AM, Laszlo Ersek wrote:

On 09/03/17 04:12, Ge Song wrote:

In earlier PEI stage, temporary memory(Cache As Ram) is employed as stack

Please remove the "Cache As Ram" reference; it doesn't apply to OVMF.

I suggest "temporary memory at PcdOvmfSecPeiTempRamBase"


and heap. We move them to the new room and do some relocation fixup when
permanent memory becomes available. TemporaryRamMigration()
is responsible for switching the stack.

In the begining of the TemporaryRamMigration(),

I suggest rewording this as "before entering TemporaryRamMigration()" --
when I read the above, I went looking for Ebp/Rbp manipulation within
TemporaryRamMigration().


Ebp/Rbp is populated with content of Esp/Rsp and used as frame pointer,
after the execution of SetJump/LongJump, stack migrates to new position

Please start a new sentence with "after the execution of ...".


while the context keeps unchanged. But when TemporaryRamMigration() exits,
Esp/Rsp is filled with the content of Ebp/Rbp to destroy this stack frame.
The result is, stack switches back to previous temporary momery.

s/momery/memory/

Thanks for your detailed guidance, I'll revise the things you mentioned in
V2 patch.

And I must express my thanks for your article
"Laszlo's unkempt git guide for edk2 contributors and maintainers".
It helps me a lot!

But, more importantly, what are the practical consequences of this bug?

Does it mean that the temporary SEC/PEI stack is used until the end of
PEI, even after permanent RAM is installed, during both normal boot and
S3 resume?

Yes, I think that's exactly true.

If that's the case, I wonder how it was caught.

Once I was tracing a variable and found the abnormal value in SP.

If it was caught during S3 resume, then the only symptom could have been
stack overflow. Because during normal boot, both temporary and permanent
PEI RAM are reserved, so even if we stay on the temporary stack during
S3, if we don't overflow it, we cannot corrupt data:


/**
   Publish system RAM and reserve memory regions

**/
VOID
InitializeRamRegions (
   VOID
   )
{
   if (!mXen) {
 QemuInitializeRam ();
   } else {
 XenPublishRamRegions ();
   }

   if (mS3Supported && mBootMode != BOOT_ON_S3_RESUME) {
 //
 // This is the memory range that will be used for PEI on S3 resume
 //
 BuildMemoryAllocationHob (
   mS3AcpiReservedMemoryBase,
   mS3AcpiReservedMemorySize,
   EfiACPIMemoryNVS
   );

 //
 // Cover the initial RAM area used as stack and temporary PEI heap.
 //
 // This is reserved as ACPI NVS so it can be used on S3 resume.
 //
 BuildMemoryAllocationHob (
   PcdGet32 (PcdOvmfSecPeiTempRamBase),
   PcdGet32 (PcdOvmfSecPeiTempRamSize),
   EfiACPIMemoryNVS
   );

With stack overflow during S3 resume, we may have corrupted OS data
residing very low.

Yes, I think this is the real problem.

In current implementation, stack located in temporary memory will not be
exhausted during the whole PEI phase so there won't be an system crash
during S3 resume caused by this.

The size of stack in temporary memory is 32K, when in permanent memory it
is 128K. This approximately simulate the scene in real machine. 
Logically the

stack should be switched to permanent memory when the latter becomes
available.

Since hardware protection mechanism for stack is disabled, currently using
larger stack can reduced the possibility of stack overflow, thus, the 
possibility

of crash during S3 resume

Check the GDT set up by reset vector module:
GDT_BASE:
; null descriptor
NULL_SELequ $-GDT_BASE
DW  0; limit 15:0
DW  0; base 15:0
DB  0; base 23:16
DB  0; sys flag, dpl, type
DB  0; limit 19:16, flags
DB  0; base 31:24

; linear data segment descriptor
LINEAR_SEL  equ $-GDT_BASE
DW  0x   ; limit 15:0
DW  0; base 15:0
DB  0; base 23:16
DB PRESENT_FLAG(1)|DPL(0)|SYSTEM_FLAG(1)|DESC_TYPE(DATA32_TYPE)
DB 
GRANULARITY_FLAG(1)|DEFAULT_SIZE32(1)|CODE64_FLAG(0)|UPPER_LIMIT(0xf)

DB  0; base 31:24

; linear code segment descriptor
LINEAR_CODE_SEL equ $-GDT_BASE
DW  0x   ; limit 15:0
DW  0; base 15:0
DB  0; base 23:16
DB PRESENT_FLAG(1)|DPL(0)|SYSTEM_FLAG(1)|DESC_TYPE(CODE32_TYPE)
DB 
GRANULARITY_FLAG(1)|DEFAULT_SIZE32(1)|CODE64_FLAG(0)|UPPER_LIMIT(0xf)

DB  0; base 31:24

%ifdef ARCH_X64
; linear code (64-bit) segment descriptor
LINEAR_CODE64_SEL   equ $-GDT_BASE
DW  0x   ; limit 15:0
DW  0; base 15:0
DB  0; base 23:16
DB PRESENT_FLAG(1)|DPL(0)|SYSTEM_FLAG(1)|DESC_TYPE(CODE64_TYPE)
DB 
GRANULARITY_FLAG(1)|DEFAULT_SIZE32(0)|CODE64_FLAG(1)|UPPE

[edk2] [PATCH v1 0/1] Fix stack switching to permanent memory fail

2017-09-02 Thread Ge Song
Repo:   https://github.com/sgbird/edk2.git
Branch: stack_switch

This patch is to fix that it failed to switch the stack from temporary
memory to permanent memory when the latter becomes available

Cc: Jordan Justen <jordan.l.jus...@intel.com>
Cc: Laszlo Ersek <ler...@redhat.com>
Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>

Ge Song (1):
  OvmfPkg/SecMain: Fix stack switching to permanent memory fail

 OvmfPkg/Sec/SecMain.c | 2 ++
 1 file changed, 2 insertions(+)

-- 
2.11.0

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


[edk2] [PATCH v1 1/1] OvmfPkg/SecMain: Fix stack switching to permanent memory fail

2017-09-02 Thread Ge Song
In earlier PEI stage, temporary memory(Cache As Ram) is employed as stack
and heap. We move them to the new room and do some relocation fixup when
permanent memory becomes available. TemporaryRamMigration()
is responsible for switching the stack.

In the begining of the TemporaryRamMigration(),
Ebp/Rbp is populated with content of Esp/Rsp and used as frame pointer,
after the execution of SetJump/LongJump, stack migrates to new position
while the context keeps unchanged. But when TemporaryRamMigration() exits,
Esp/Rsp is filled with the content of Ebp/Rbp to destroy this stack frame.
The result is, stack switches back to previous temporary momery.

More detailed information:
> TemporaryRamMigration (PeiServices=0x817790,
> TemporaryMemoryBase=8454144, PermanentMemoryBase=469016576,
> CopySize=32768)
> at /home/bird/src/edk2/OvmfPkg/Sec/SecMain.c:938
> 938   LongJump (, (UINTN)-1);
> (gdb) info registers
> rax0x1bf4d3b0 469029808
> rbx0x810248   8454728
> rcx0x8173e8   8483816
> rdx0x8173b0   8483760
> rsi0x1bf4a000 469016576
> rdi0x8000 32768
> rbp0x817520   0x817520
> rsp0x8173b0   0x8173b0
> r8 0x00
> ...
> rip0xfffcd409 0xfffcd409 <TemporaryRamMigration+365>
> eflags 0x2[ ]
> cs 0x18   24
> ss 0x88
> ...

After execution of LongJump:
> 943 return EFI_SUCCESS;
> (gdb) info registers
> rax0x00
> rbx0x810248   8454728
> rcx0x00
> rdx0x -1
> rsi0x1bf4a000 469016576
> rdi0x8000 32768
> rbp0x817520   0x817520
> rsp0x1bf4d3b0 0x1bf4d3b0
> r8 0x00
> ...
> rip0xfffcd42a 0xfffcd42a <TemporaryRamMigration+398>
> eflags 0x86   [ PF SF ]
> cs 0x18   24
> ss 0x88
> ...

We can find rsp has changed to new permanent memory

When leaving TemporaryRamMigration(), the stack swithes back to previous
temporary memory:
> (gdb) finish
> Run till exit from #0  TemporaryRamMigration (PeiServices=0x817790,
> TemporaryMemoryBase=8454144, PermanentMemoryBase=469016576,
> CopySize=32768)
> at /home/bird/src/edk2/OvmfPkg/Sec/SecMain.c:943
> PeiCheckAndSwitchStack (SecCoreData=0x1bf4df78, Private=0x1bf4d788) at
> /home/bird/src/edk2/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c:806
> 806 PeiCore (SecCoreData, NULL, Private);
> Value returned is $1 = 0
> (gdb) info registers
> rax0x00
> rbx0x810248   8454728
> rcx0x00
> rdx0x -1
> rsi0x1bf4a000 469016576
> rdi0x8000 32768
> rbp0x817630   0x817630
> rsp0x817530   0x817530
> r8 0x00
> ...
> rip0x828135   0x828135 <PeiCheckAndSwitchStack+1573>
> eflags 0x86   [ PF SF ]
> cs 0x18   24
> ss 0x88
> ...

> (gdb) disassemble /r
> Dump of assembler code for function TemporaryRamMigration:
>   0xfffcd29c <+0>:55  push   %rbp
>   0xfffcd29d <+1>:48 89 e5mov%rsp,%rbp
>   0xfffcd2a0 <+4>:48 81 ec 70 01 00 00sub
> $0x170,%rsp
>...
>...
>0xfffcd425 <+393>: e8 80 10 00 00  callq  0xfffce4aa
> 
> => 0xfffcd42a <+398>: b8 00 00 00 00  mov$0x0,%eax
>0xfffcd42f <+403>: c9  leaveq
>0xfffcd430 <+404>: c3  retq
> End of assembler dump.

See the description of leave(opcode: c9), from
Intel® 64 and IA-32 Architectures Software Developer’s Manual, Volume 2A

"Releases the stack frame set up by an earlier ENTER instruction. The
LEAVE instruction copies the frame pointer (in the EBP register) into
the stack pointer register (ESP), which releases the stack space
allocated to the stack frame. The old frame pointer (the frame pointer
for the calling procedure that was saved by the ENTER instruction) is
then popped from the stack into the EBP register, restoring the calling
procedure’s stack frame."

To solve this, update Ebp/Rbp too when Esp/Rsp is updated

Cc: Jordan Justen <jordan.l.jus...@intel.com>
Cc: Laszlo Ersek <ler...@redhat.com>
Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ge Song <ge.s...@hxt-semitech.com>
---
 OvmfPkg/Sec/SecMain.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
index e1993ec347b5..f7fec3d8