From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (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 D8B72203555F8 for ; Mon, 13 Nov 2017 05:04:59 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C8C6F4E909; Mon, 13 Nov 2017 13:09:05 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-226.rdu2.redhat.com [10.10.120.226]) by smtp.corp.redhat.com (Postfix) with ESMTP id C2EAA77D69; Mon, 13 Nov 2017 13:09:04 +0000 (UTC) From: Laszlo Ersek To: Ard Biesheuvel , Jordan Justen Cc: Ruiyu Ni , edk2-devel-01 References: <20171110154908.306-1-lersek@redhat.com> <151043270153.17841.16763408160801933614@jljusten-skl> <151043786891.19895.6326436717816766532@jljusten-skl> <151056410867.15809.659701894226687543@jljusten-skl> Message-ID: <2984eedf-0fee-0d4b-ac3c-83d6307446b1@redhat.com> Date: Mon, 13 Nov 2017 14:09:03 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Mon, 13 Nov 2017 13:09:05 +0000 (UTC) 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 13:05:00 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 11/13/17 13:34, Laszlo Ersek wrote: > I guess I could live with BASE_LIBRARY_JUMP_BUFFER. Actually: > More specific comments: > >> diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c >> index f7fec3d8c0..077f7d6563 100644 >> --- a/OvmfPkg/Sec/SecMain.c >> +++ b/OvmfPkg/Sec/SecMain.c >> @@ -1,7 +1,7 @@ >> /** @file >> Main SEC phase code. Transitions to PEI. >> >> - Copyright (c) 2008 - 2015, Intel Corporation. All rights reserved.
>> + Copyright (c) 2008 - 2017, Intel Corporation. All rights reserved.
>> (C) Copyright 2016 Hewlett Packard Enterprise Development LP
>> >> This program and the accompanying materials >> @@ -731,6 +731,25 @@ SecCoreStartupWithStack ( >> UINT32 Index; >> volatile UINT8 *Table; >> >> + // >> + // Fill most of temporary RAM with PcdInitValueInTempStack. We stop >> + // filling at the current stack pointer - 512 bytes. >> + // >> + DEBUG_CODE_BEGIN (); >> + BASE_LIBRARY_JUMP_BUFFER JumpBuffer; >> + UINTN StackUsed; >> + >> + SetJump (&JumpBuffer); >> +#if defined (MDE_CPU_IA32) >> + StackUsed = (UINTN)TopOfCurrentStack - JumpBuffer.Esp; >> +#elif defined (MDE_CPU_X64) >> + StackUsed = (UINTN)TopOfCurrentStack - JumpBuffer.Rsp; >> +#endif >> + SetMem32 ((VOID*)(UINTN)PcdGet32 (PcdOvmfSecPeiTempRamBase), >> + PcdGet32 (PcdOvmfSecPeiTempRamSize) - StackUsed - 512, >> + FixedPcdGet32 (PcdInitValueInTempStack)); > > (1) SetMem32() is likely problematic in itself; please refer to the > following comment -- partly visible in the context of Jordan's patch --, > from commit 320b4f084a25 ("OvmfPkg: Sec: force reinit of > BaseExtractGuidedSectionLib handler table", 2015-11-30): > > // > // To ensure SMM can't be compromised on S3 resume, we must force re-init of > // the BaseExtractGuidedSectionLib. Since this is before library contructors > // are called, we must use a loop rather than SetMem. > // > > Thus, we should use a loop and a pointer-to-volatile. (It would likely > be slower than the REP STOSD / REP STOSQ.) given that I'm opposed to calling any library functions before we reach the ProcessLibraryConstructorList() call lower down in SecCoreStartupWithStack(), I cannot agree to calling SetJump() either. Thanks, Laszlo