From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=17.151.62.68; helo=nwk-aaemail-lapp03.apple.com; envelope-from=afish@apple.com; receiver=edk2-devel@lists.01.org Received: from nwk-aaemail-lapp03.apple.com (nwk-aaemail-lapp03.apple.com [17.151.62.68]) (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 6C7FA21160A36 for ; Mon, 19 Nov 2018 13:23:35 -0800 (PST) Received: from pps.filterd (nwk-aaemail-lapp03.apple.com [127.0.0.1]) by nwk-aaemail-lapp03.apple.com (8.16.0.22/8.16.0.22) with SMTP id wAJLLgMS065039; Mon, 19 Nov 2018 13:23:33 -0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=apple.com; h=mime-version : content-type : sender : from : message-id : subject : date : in-reply-to : cc : to : references; s=20180706; bh=yKRinl5sX3MtA2wZq8PSitwCsOYI7lKU8EWAfjDFeBE=; b=c3TRYmu1UNY7NhswCLWJv/8WXCPJc29hTrBgzljjZA0GCEAsDYBo3z9Z7ZWdYebJlxIL cssJ2jI0Blq6khmqXtYotRwfCWkX7sz2vIYXx/XM1cCQg9qiUJzLDBQ11uKaFKnM4e9O JnSLKaGjG0hmWiKgBopCr3tjMVZHvVERQ6eYjFIyq9UWXkkCMxxq6AwAiXwBIa8wb7oM pTzUU6kTWKyK1caDaEZD10ZDVXrIfADeyualSqoxuDnXDAud4TN2xJsCD5/oO1jU4w99 VCfjdBs7HptkTSaJMJ9m5RCcsex9v8RXnS5Vsdvel9IDGuHYINknljhXGQk62PbiCWei mA== Received: from ma1-mtap-s03.corp.apple.com (ma1-mtap-s03.corp.apple.com [17.40.76.7]) by nwk-aaemail-lapp03.apple.com with ESMTP id 2ntgdb7701-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO); Mon, 19 Nov 2018 13:23:33 -0800 MIME-version: 1.0 Received: from ma1-mmpp-sz09.apple.com (ma1-mmpp-sz09.apple.com [17.171.128.183]) by ma1-mtap-s03.corp.apple.com (Oracle Communications Messaging Server 8.0.2.3.20180614 64bit (built Jun 14 2018)) with ESMTPS id <0PIG00IFJM38L370@ma1-mtap-s03.corp.apple.com>; Mon, 19 Nov 2018 13:23:32 -0800 (PST) Received: from process_viserion-daemon.ma1-mmpp-sz09.apple.com by ma1-mmpp-sz09.apple.com (Oracle Communications Messaging Server 8.0.2.3.20180614 64bit (built Jun 14 2018)) id <0PIG00600LVR6S00@ma1-mmpp-sz09.apple.com>; Mon, 19 Nov 2018 13:23:32 -0800 (PST) X-Va-A: X-Va-T-CD: 654a2d763c4b4477cd1e9185a623c2f1 X-Va-E-CD: 63889f024351edad8b341b5de07fb5d9 X-Va-R-CD: 1b31b8f27f74d69e316e535bfbff9601 X-Va-CD: 0 X-Va-ID: a8d14a3f-9f4c-44c4-ac9c-8e06f77c68cc X-V-A: X-V-T-CD: 654a2d763c4b4477cd1e9185a623c2f1 X-V-E-CD: 63889f024351edad8b341b5de07fb5d9 X-V-R-CD: 1b31b8f27f74d69e316e535bfbff9601 X-V-CD: 0 X-V-ID: d0a1e5ea-00d7-4602-a51a-ee0fbc8f2507 Received: from process_milters-daemon.ma1-mmpp-sz09.apple.com by ma1-mmpp-sz09.apple.com (Oracle Communications Messaging Server 8.0.2.3.20180614 64bit (built Jun 14 2018)) id <0PIG00B00KXFA900@ma1-mmpp-sz09.apple.com>; Mon, 19 Nov 2018 13:23:32 -0800 (PST) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-11-19_08:,, signatures=0 Received: from [17.234.252.196] (unknown [17.234.252.196]) by ma1-mmpp-sz09.apple.com (Oracle Communications Messaging Server 8.0.2.3.20180614 64bit (built Jun 14 2018)) with ESMTPSA id <0PIG00GYUM35ZX60@ma1-mmpp-sz09.apple.com>; Mon, 19 Nov 2018 13:23:32 -0800 (PST) Sender: afish@apple.com From: Andrew Fish Message-id: Date: Mon, 19 Nov 2018 13:22:27 -0800 In-reply-to: <154265500096.11985.13073430907870235751@jljusten-skl> Cc: Mike Kinney , Liu Yu , "edk2-devel@lists.01.org" , Laszlo Ersek , Leif Lindholm To: Jordan Justen References: <154253322290.3729.10762860453718631884@jljusten-skl> <81FDCF97-4145-4A7D-93B9-70A4D8B505FF@apple.com> <154259000092.7306.2206833534307673295@jljusten-skl> <154265500096.11985.13073430907870235751@jljusten-skl> X-Mailer: Apple Mail (2.3445.6.18) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:, , definitions=2018-11-19_08:, , signatures=0 X-Content-Filtered-By: Mailman/MimeDel 2.1.29 Subject: Re: EmulatorPkg Unix Host Segmentation fault. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 19 Nov 2018 21:23:35 -0000 Content-Type: text/plain; CHARSET=US-ASCII Content-Transfer-Encoding: 7BIT > On Nov 19, 2018, at 11:16 AM, Jordan Justen wrote: > > On 2018-11-18 17:13:21, Jordan Justen wrote: >> On 2018-11-18 14:37:09, Andrew Fish wrote: >>> >>> >>>> On Nov 18, 2018, at 4:07 AM, Liu Yu wrote: >>>> >>>> sorry your path can't fix this issue. if this path just turn off >>>> optimization option within sec.c not global project. >>>> >>>> I have tested different version GCC such as (GCC4,8, GCC5.x, GCC7.x) >>>> and all of them can duplicate this issue (Ubuntu 16.04, 16.10,18.04 ) >>>> >>>> I have traced this issue on my hand. >>>> >>>> you can see Dispatcher.c (MdeModulePkg/Pei/DIspatcher/) Line 792: >>>> >>>> >>>> 790 if (StackOffsetPositive) { >>>> 791 SecCoreData = (CONST EFI_SEC_PEI_HAND_OFF *)((UINTN)(VOID >>>> *)SecCoreData + StackOffset); >>>> 792 Private = (PEI_CORE_INSTANCE *)((UINTN)(VOID *)Private + >>>> StackOffset); >>>> 793 } else { >>>> 794 .......... >>>> 795 .......... >>>> 796 } >>>> >>>> 790 --792 disassembly code >>>> >>>> 0x10200f2ca : test %r14b,%r14b >>>> 0x10200f2cd : je 0x10200f2df >>>> >>>> 0x10200f2cf : mov 0x38(%rsp),%rax >>>> 0x10200f2d4 : lea 0x0(%rbp,%rax,1),%r14 >>>> 0x10200f2d9 : lea (%rbx,%rax,1),%rbp >>>> >>>> we can see Private value have been stored in %rbp (rbp register be >>>> used as general register ) so when call >>>> TemporaryRamSupportPpi->TemporaryRamMigration() >>>> >>> >>> The calling conventions define RBP as non-volatile must be preserved >>> by callee. Using RBP as the frame pointer is optional. >> >> This is kind of a mess. I think the definition of >> EFI_PEI_TEMPORARY_RAM_SUPPORT_PPI is really to blame. It probably >> should not have been spec'd as 'change stack and return'. Instead, it >> should have been given a new function pointer to switch-stack and call >> into. > > Andrew, Mike, > > I developed a PEI Core fix for this issue, which Liu Yu verified. > > Unfortunately, it involves add assembly code in a key area of the PEI > Core. See the top 2 commits of: > > https://github.com/jljusten/edk2/commits/emulator-temp-ram > > I only wrote assembly for X64 nasm. But, I notice that neither the PEI > or DXE Core modules include any assembly code. So, I want to make sure > I'm heading in the right direction before working on it further. > Mike, I seem to remember we hit an issue like this a long time ago? Do you remember the details? Maybe it was we needed to write the TempRamSupport code in assembly? The issue we are hitting is the gEfiTemporaryRamSupportPpiGuid TemporaryRamMigration() function call does a stack switch, but it returns. This causes an issue with the C calling conventions as RBP is optionally a frame pointer. To quote the MSFT spec: "May be used as a frame pointer; must be preserved by callee" 1) It is used as a frame pointer. It looks like our existing code fixes up the frame pointer to match the new location the stack got moved to. 2) Used as a general purpose register, and the value must be preserved. > As I mentioned below, I think PIWG could consider an new > TemporarayRemSupport interface that might work better, but that also > may not be worth the effort. If the current API is not really portable I don;t think it is a bad idea to update it. > > Can something like this change be integrated into the PEI Core? Jordan, I'm not sure? For the RBP == frame pointer case the frame pointer is no longer valid (it is in temp RAM, not DRAM). It seems like the point of SecTemporaryRamSupport() fixing up the callers RBP is for the benefit of the caller. It looks to me like your fix is just negating that fixup. So that would imply that either we could just fix this in SecTemporaryRamSupport() or we have 2 code gen paths and we need to know how the compiler is using RBP to "do the right thing" Do we have other code that supports X64 PEI? Is see OvmfPkg.... https://github.com/tianocore/edk2/blob/master/OvmfPkg/Sec/SecMain.c#L875 Looks like OvmfPkg uses SetJump()/LongJump() to change the stack. // // Use SetJump()/LongJump() to switch to a new stack. // 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); } SaveAndSetDebugTimerInterrupt (OldStatus); return EFI_SUCCESS; } But given the above code is C code RBP is going to be restored on return. This would seem to imply that the adjusting of the callers RBP is not required? So maybe try changing over Emulator to the OvmfPkg TemporaryRamMigration() algorithm? Thanks, Andrew Fish > > Thanks for your time, > > -Jordan > >> By returning with a switched stack, we don't really know what the >> returning code could do with regard to the stack. For example, it >> could have saved the stack value on the stack and then pop it into a >> register and somehow switch the stack back to the old stack. It seems >> unlikely, but I don't think anything prevents it. >> >> Additionally, there is the issue of rbp/ebp being used as a frame >> pointer. This can lead to some variables being used from the temp ram >> location after we return from TemporaryRamMigration. (Assuming rbp/ebp >> is not adjusted to point to the new stack.) >> >> So, is it safe to adjust rbp? Unknown. It may not be if rbp is not >> used as a frame pointer. Is it safe to *not* adjust rbp and >> potentially allow the old temp ram stack to be used? Unknown. >> >> Now, if TemporaryRamMigration received a new function to call into, we >> can safely assume that the new stack transition would proceed as >> expected without having to worry if a compiler is using rbp/ebp as a >> framepointer or not. >> >> Another advantage could have been that something like the BasePkg >> SwitchStack function could have been used, hopefully preventing people >> from trying to write error prone assembly code for >> TemporaryRamMigration. >> >> -Jordan