On 2017-09-05 20:13:19, Song, Ge wrote: > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ge Song <[email protected]> >
Pushed as 89796c69d9fdaa9bd13d37b6b1687df5e0104ed5. Thanks for noticing that bug and fixing it! I also hope your IT might be to help you configure git send-email to make future contributions from your hxt-semitech.com email a bit easier. Thanks again! -Jordan > > -----Original Message----- > From: [email protected] [mailto:[email protected]] > Sent: 2017年9月6日 11:12 > To: [email protected] > Cc: Jordan Justen <[email protected]>; Laszlo Ersek > <[email protected]>; Ard Biesheuvel <[email protected]>; Song, Ge > <[email protected]> > Subject: [PATCH v3 1/1] OvmfPkg/SecMain: Fix stack switching to permanent > memory > > From: Ge Song <[email protected]> > > 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: > > 0x00000000fffcd29c <+0>:55push %rbp > > 0x00000000fffcd29d <+1>:48 89 e5mov %rsp,%rbp > > 0x00000000fffcd2a0 <+4>:48 81 ec 70 01 00 00sub > > $0x170,%rsp > > ... > > ... > > 0x00000000fffcd425 <+393>:e8 80 10 00 00callq 0xfffce4aa > > <SaveAndSetDebugTimerInterrupt> > > => 0x00000000fffcd42a <+398>:b8 00 00 00 00mov $0x0,%eax > > 0x00000000fffcd42f <+403>:c9leaveq > > 0x00000000fffcd430 <+404>:c3retq > > 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 <[email protected]> > Cc: Laszlo Ersek <[email protected]> > Cc: Ard Biesheuvel <[email protected]> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ge Song <[email protected]> > Tested-by: Laszlo Ersek <[email protected]> > Reviewed-by: Laszlo Ersek <[email protected]> > --- > 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 (&JumpBuffer) == 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 (&JumpBuffer, (UINTN)-1); > } > -- > 2.11.0 > > > > > This email is intended only for the named addressee. It may contain > information that is confidential/private, legally privileged, or > copyright-protected, and you should handle it accordingly. If you are not the > intended recipient, you do not have legal rights to retain, copy, or > distribute this email or its contents, and should promptly delete the email > and all electronic copies in your system; do not retain copies in any media. > If you have received this email in error, please notify the sender promptly. > Thank you. > > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
