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.43; helo=mga05.intel.com; envelope-from=jordan.l.justen@intel.com; receiver=edk2-devel@lists.01.org Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) (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 D773320349DB2 for ; Mon, 13 Nov 2017 01:04:24 -0800 (PST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 13 Nov 2017 01:08:30 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.44,388,1505804400"; d="scan'208";a="6984638" Received: from sacolema-mobl2.amr.corp.intel.com (HELO localhost) ([10.254.117.1]) by orsmga002.jf.intel.com with ESMTP; 13 Nov 2017 01:08:29 -0800 MIME-Version: 1.0 To: Ard Biesheuvel Message-ID: <151056410867.15809.659701894226687543@jljusten-skl> From: Jordan Justen In-Reply-To: Cc: Laszlo Ersek , edk2-devel-01 , Ruiyu Ni References: <20171110154908.306-1-lersek@redhat.com> <151043270153.17841.16763408160801933614@jljusten-skl> <151043786891.19895.6326436717816766532@jljusten-skl> User-Agent: alot/0.6 Date: Mon, 13 Nov 2017 01:08:28 -0800 Subject: Re: [PATCH 0/4] OvmfPkg: measure temp stack usage, restore temp RAM to 64KB 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: Mon, 13 Nov 2017 09:04:25 -0000 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable On 2017-11-12 02:58:37, Ard Biesheuvel wrote: > On 11 November 2017 at 22:04, Jordan Justen w= rote: > > On 2017-11-11 12:38:21, Jordan Justen wrote: > >> On 2017-11-10 07:49:04, Laszlo Ersek wrote: > >> > The first three patches enable the PEI_CORE to report OVMF's temp > >> > SEC/PEI stack and heap usage. > >> > > >> > - This depends on the new fixed PCD "PcdInitValueInTempStack", > >> > recently added for > >> > > >> > ("INIT_CAR_VALUE should be defined in a central location"). > >> > > >> > - Ard recently implemented the same in ArmPlatformPkg, for > >> > ("measure= temp > >> > SEC/PEI stack usage"). > >> > > >> > The last (fourth) patch adapts OVMF to the larger MtrrLib stack dema= nd > >> > that originates from commit 2bbd7e2fbd4b ("UefiCpuPkg/MtrrLib: Update > >> > algorithm to calculate optimal settings", 2017-09-27). OVMF's temp R= AM > >> > size is restored to the historical / original 64KB. > >> > > >> > This work is tracked in > >> > ("measure temp > >> > SEC/PEI stack usage; grow temp SEC/PEI RAM"), with links to related > >> > mailing list discussions. > >> > > >> > Repo: https://github.com/lersek/edk2.git > >> > Branch: temp_ram_tweaks > >> > > >> > Cc: Ard Biesheuvel > >> > Cc: Jordan Justen > >> > Cc: Ruiyu Ni > >> > > >> > Thanks > >> > Laszlo > >> > > >> > Laszlo Ersek (4): > >> > OvmfPkg/Sec/Ia32: free up EAX for other uses while setting up the > >> > stack > >> > OvmfPkg/Sec/Ia32: seed the temporary RAM with PcdInitValueInTempSt= ack > >> > OvmfPkg/Sec/X64: seed the temporary RAM with PcdInitValueInTempSta= ck > >> > >> I'd like to try a different option for these 3. Can you hold off a bit > >> before pushing this series? > > > > I think we should use a C based approach instead, like in the attached > > patch. > > > = > I'm not sure: having to abuse SetJump () True, that was annoying. It seems like we could have AsmReadEsp and AsmReadRsp in BaseLib since we have AsmReadSp for IPF. > and having to leave an > arbitrary 512 byte window both seem pretty good reasons to stick with > assembly. Also true. I chose 512 because it seemed like more than SetMem32 could reasonably need, but also much below the minimum I would expect PEI to use. (It seemed that around 4k ended up being used.) > Is your concern that the stack gets cleared in RELEASE builds as well? No. I just prefer if we can use C rather than assembly whenever it is reasonable. -Jordan > Can't we put an #ifndef MDEPKG_NDEBUG around the code instead? > = > >> > OvmfPkg: restore temporary SEC/PEI RAM size to 64KB > > > > This patch is Reviewed-by: Jordan Justen > > > >> > > >> > OvmfPkg/OvmfPkgIa32.fdf | 2 +- > >> > OvmfPkg/OvmfPkgIa32X64.fdf | 2 +- > >> > OvmfPkg/OvmfPkgX64.fdf | 2 +- > >> > OvmfPkg/Sec/SecMain.inf | 1 + > >> > OvmfPkg/Sec/Ia32/SecEntry.nasm | 19 ++++++++++++++++--- > >> > OvmfPkg/Sec/X64/SecEntry.nasm | 15 +++++++++++++++ > >> > 6 files changed, 35 insertions(+), 6 deletions(-) > >> > > >> > -- > >> > 2.14.1.3.gb7cf6e02401b > >> >