From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.31; helo=mga06.intel.com; envelope-from=jordan.l.justen@intel.com; receiver=edk2-devel@lists.01.org Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) (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 DDC3D202E53AC for ; Mon, 18 Feb 2019 01:08:27 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 18 Feb 2019 01:08:25 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.58,384,1544515200"; d="scan'208";a="127307594" Received: from mmdandap-mobl1.amr.corp.intel.com (HELO localhost) ([10.254.8.66]) by orsmga003.jf.intel.com with ESMTP; 18 Feb 2019 01:08:25 -0800 MIME-Version: 1.0 In-Reply-To: References: <20190218041141.21363-1-jordan.l.justen@intel.com> <20190218041141.21363-7-jordan.l.justen@intel.com> Cc: "edk2-devel@lists.01.org" , Laszlo Ersek , Anthony Perard , Julien Grall To: Ard Biesheuvel From: Jordan Justen Message-ID: <155048090465.22654.1079629797155553207@jljusten-skl> User-Agent: alot/0.8 Date: Mon, 18 Feb 2019 01:08:24 -0800 Subject: Re: [PATCH 06/10] OvmfPkg/Sec: Disable optimizations for TemporaryRamMigration 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, 18 Feb 2019 09:08:28 -0000 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable On 2019-02-17 23:53:01, Ard Biesheuvel wrote: > On Mon, 18 Feb 2019 at 05:12, Jordan Justen w= rote: > > >=20 > This needs an explanation why optimization needs to be disabled. I'm not sure this is required. The reason I added these patches is to hopefully prevent the compiler from removing the frame pointer. We adjust the frame pointer in the code, and that is a little sketchy if the frame pointer isn't being used. Unfortunately, it can reasonably be argued that the TemporaryRamSupport PPI definition ultimately makes it unsafe to write the migration code in C. I tried reverting both the EmulatorPkg and OvmfPkg patches for disabling the optimizations, and with my setup there was no impact. I think there is a good change that we'd be pretty safe to just drop these two patches to wait and see if someone encounters a situation that requires it. Ok, so based on this explanation, do you think I should add info to the commit message and keep the patches, or just drop them? Thanks, -Jordan >=20 > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Jordan Justen > > Cc: Laszlo Ersek > > Cc: Ard Biesheuvel > > Cc: Anthony Perard > > Cc: Julien Grall > > --- > > OvmfPkg/Sec/SecMain.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c > > index 46ac739862..86c22a2ac9 100644 > > --- a/OvmfPkg/Sec/SecMain.c > > +++ b/OvmfPkg/Sec/SecMain.c > > @@ -873,6 +873,13 @@ SecStartupPhase2( > > CpuDeadLoop (); > > } > > > > +#ifdef __GNUC__ > > +#pragma GCC push_options > > +#pragma GCC optimize ("O0") > > +#else > > +#pragma optimize ("", off) > > +#endif > > + > > EFI_STATUS > > EFIAPI > > TemporaryRamMigration ( > > @@ -946,3 +953,8 @@ TemporaryRamMigration ( > > return EFI_SUCCESS; > > } > > > > +#ifdef __GNUC__ > > +#pragma GCC pop_options > > +#else > > +#pragma optimize ("", on) > > +#endif >=20 > I can't tell from the context if this is the end of the file, but if > it is not, aren't you turning on optimization here for non-GCC even if > it was not enabled on the command line to begin with?