Reviewed-by: Jordan Justen <[email protected]> But, I would like to get a Tested-by or Acked-by from Laszlo, since I expect he'd want to test S3 resume with this change.
-Jordan On 2017-09-02 22:12:14, 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. > > 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 (&JumpBuffer, (UINTN)-1); > > (gdb) info registers > > rax 0x1bf4d3b0 469029808 > > rbx 0x810248 8454728 > > rcx 0x8173e8 8483816 > > rdx 0x8173b0 8483760 > > rsi 0x1bf4a000 469016576 > > rdi 0x8000 32768 > > rbp 0x817520 0x817520 > > rsp 0x8173b0 0x8173b0 > > r8 0x0 0 > > ... > > rip 0xfffcd409 0xfffcd409 <TemporaryRamMigration+365> > > eflags 0x2 [ ] > > cs 0x18 24 > > ss 0x8 8 > > ... > > After execution of LongJump: > > 943 return EFI_SUCCESS; > > (gdb) info registers > > rax 0x0 0 > > rbx 0x810248 8454728 > > rcx 0x0 0 > > rdx 0xffffffffffffffff -1 > > rsi 0x1bf4a000 469016576 > > rdi 0x8000 32768 > > rbp 0x817520 0x817520 > > rsp 0x1bf4d3b0 0x1bf4d3b0 > > r8 0x0 0 > > ... > > rip 0xfffcd42a 0xfffcd42a <TemporaryRamMigration+398> > > eflags 0x86 [ PF SF ] > > cs 0x18 24 > > ss 0x8 8 > > ... > > 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 > > rax 0x0 0 > > rbx 0x810248 8454728 > > rcx 0x0 0 > > rdx 0xffffffffffffffff -1 > > rsi 0x1bf4a000 469016576 > > rdi 0x8000 32768 > > rbp 0x817630 0x817630 > > rsp 0x817530 0x817530 > > r8 0x0 0 > > ... > > rip 0x828135 0x828135 <PeiCheckAndSwitchStack+1573> > > eflags 0x86 [ PF SF ] > > cs 0x18 24 > > ss 0x8 8 > > ... > > > (gdb) disassemble /r > > Dump of assembler code for function TemporaryRamMigration: > > 0x00000000fffcd29c <+0>: 55 push %rbp > > 0x00000000fffcd29d <+1>: 48 89 e5 mov %rsp,%rbp > > 0x00000000fffcd2a0 <+4>: 48 81 ec 70 01 00 00 sub > > $0x170,%rsp > > ... > > ... > > 0x00000000fffcd425 <+393>: e8 80 10 00 00 callq 0xfffce4aa > > <SaveAndSetDebugTimerInterrupt> > > => 0x00000000fffcd42a <+398>: b8 00 00 00 00 mov $0x0,%eax > > 0x00000000fffcd42f <+403>: c9 leaveq > > 0x00000000fffcd430 <+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 <[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]> > --- > 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 > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
