From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 1325321EB88E8 for ; Sat, 2 Sep 2017 21:34:20 -0700 (PDT) Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 02 Sep 2017 21:37:06 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.41,467,1498546800"; d="scan'208";a="144916534" Received: from sshamise-mobl2.gar.corp.intel.com (HELO localhost) ([10.249.79.36]) by orsmga005.jf.intel.com with ESMTP; 02 Sep 2017 21:37:02 -0700 MIME-Version: 1.0 To: Ge Song , edk2-devel@lists.01.org, Laszlo Ersek Message-ID: <150441341827.31523.9937848389144561897@jljusten-skl> From: Jordan Justen In-Reply-To: <20170903021214.11516-2-ge.song@hxt-semitech.com> Cc: Ard Biesheuvel References: <20170903021214.11516-1-ge.song@hxt-semitech.com> <20170903021214.11516-2-ge.song@hxt-semitech.com> User-Agent: alot/0.6 Date: Sun, 03 Sep 2017 00:37:00 -0400 Subject: Re: [PATCH v1 1/1] OvmfPkg/SecMain: Fix stack switching to permanent memory fail X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 03 Sep 2017 04:34:20 -0000 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Reviewed-by: Jordan Justen 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=3D0x817790, > > TemporaryMemoryBase=3D8454144, PermanentMemoryBase=3D469016576, > > CopySize=3D32768) > > 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 > > 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 > > 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=3D0x817790, > > TemporaryMemoryBase=3D8454144, PermanentMemoryBase=3D469016576, > > CopySize=3D32768) > > at /home/bird/src/edk2/OvmfPkg/Sec/SecMain.c:943 > > PeiCheckAndSwitchStack (SecCoreData=3D0x1bf4df78, Private=3D0x1bf4d788)= at > > /home/bird/src/edk2/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c:806 > > 806 PeiCore (SecCoreData, NULL, Private); > > Value returned is $1 =3D 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 > > 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 > > > > =3D> 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=C2=AE 64 and IA-32 Architectures Software Developer=E2=80=99s Manua= l, 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=E2=80=99s stack frame." > = > To solve this, update Ebp/Rbp too when Esp/Rsp is updated > = > Cc: Jordan Justen > Cc: Laszlo Ersek > Cc: Ard Biesheuvel > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ge Song > --- > 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) =3D=3D 0) { > #if defined (MDE_CPU_IA32) > JumpBuffer.Esp =3D JumpBuffer.Esp + DebugAgentContext.StackMigrateOf= fset; > + JumpBuffer.Ebp =3D JumpBuffer.Ebp + DebugAgentContext.StackMigrateOf= fset; > #endif = > #if defined (MDE_CPU_X64) > JumpBuffer.Rsp =3D JumpBuffer.Rsp + DebugAgentContext.StackMigrateOf= fset; > + JumpBuffer.Rbp =3D JumpBuffer.Rbp + DebugAgentContext.StackMigrateOf= fset; > #endif = > LongJump (&JumpBuffer, (UINTN)-1); > } > -- = > 2.11.0 >=20