From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.136; helo=mga12.intel.com; envelope-from=jordan.l.justen@intel.com; receiver=edk2-devel@lists.01.org Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) (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 3DE3F21193076 for ; Mon, 19 Nov 2018 11:16:43 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 19 Nov 2018 11:16:42 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,254,1539673200"; d="scan'208";a="109856269" Received: from dtan-mobl.amr.corp.intel.com (HELO localhost) ([10.254.55.211]) by orsmga002.jf.intel.com with ESMTP; 19 Nov 2018 11:16:41 -0800 MIME-Version: 1.0 To: Andrew Fish , Michael D Kinney , Liu Yu Message-ID: <154265500096.11985.13073430907870235751@jljusten-skl> From: Jordan Justen In-Reply-To: <154259000092.7306.2206833534307673295@jljusten-skl> Cc: "edk2-devel@lists.01.org" , Laszlo Ersek , Leif Lindholm References: <154253322290.3729.10762860453718631884@jljusten-skl> <81FDCF97-4145-4A7D-93B9-70A4D8B505FF@apple.com> <154259000092.7306.2206833534307673295@jljusten-skl> User-Agent: alot/0.7 Date: Mon, 19 Nov 2018 11:16:41 -0800 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 19:16:43 -0000 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable 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 =3D (CONST EFI_SEC_PEI_HAND_OFF *)((UINTN)(VOID = > > > *)SecCoreData + StackOffset); > > > 792 Private =3D (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. 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. Can something like this change be integrated into the PEI Core? 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