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 15E5C2034715B for ; Fri, 20 Oct 2017 03:29:43 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id DB9CA81E09; Fri, 20 Oct 2017 10:33:21 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com DB9CA81E09 Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=lersek@redhat.com Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-150.rdu2.redhat.com [10.10.120.150]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5DFA160317; Fri, 20 Oct 2017 10:33:20 +0000 (UTC) To: Ard Biesheuvel Cc: "Gao, Liming" , Star Zeng , "Jordan Justen (Intel address)" , edk2-devel-01 References: From: Laszlo Ersek Message-ID: <0a8bf4a6-5a50-9e22-ddc7-002383ea3935@redhat.com> Date: Fri, 20 Oct 2017 12:33:19 +0200 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.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Fri, 20 Oct 2017 10:33:22 +0000 (UTC) Subject: Re: dynamic PCD impact on temporary PEI memory 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: Fri, 20 Oct 2017 10:29:43 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 10/20/17 12:22, Ard Biesheuvel wrote: > On 20 October 2017 at 10:43, Laszlo Ersek wrote: >> On 10/20/17 09:57, Ard Biesheuvel wrote: >>> On 19 October 2017 at 23:08, Laszlo Ersek wrote: >> >>>> * Symptom #1: >>>> >>>> I built OVMF for IA32, IA32X64 and X64, both "before" and "after". Then >>>> I compared the log files, to see the impact of the addition of exactly >>>> one UINT32 dynamic PCD to the PCD HOB, on temporary PEI memory usage. >>>> >>>> - Diff between "before" and "after", for IA32: >>>> >>>>> Temp Stack : BaseAddress=0x814000 Length=0x4000 >>>>> Temp Heap : BaseAddress=0x810000 Length=0x4000 >>>>> Total temporary memory: 32768 bytes. >>>>> temporary memory stack ever used: 16384 bytes. >>> >>> The code that performs this check looks broken to me btw: it looks for >>> INIT_CAR_VALUE on the stack, but it is not clear to me where the stack >>> is initialised with this value, and that fact that we always seem to >>> use exactly the entire stack looks suspicious as well. >>> >>> So perhaps you could reuse some of that space for the heap as well? >>> >>> #define INIT_CAR_VALUE 0x5AA55AA5 >> >> Possibly. Two observations hold me back: >> >> - I did notice that "temporary memory stack ever used" was always the >> full size. It seemed wrong (as you say), but without the actual usage >> being reported, I wouldn't know how much to reassign from stack to heap. >> > > This change > > --- a/ArmPlatformPkg/PrePeiCore/AArch64/PrePeiCoreEntryPoint.S > +++ b/ArmPlatformPkg/PrePeiCore/AArch64/PrePeiCoreEntryPoint.S > @@ -84,4 +84,11 @@ _PrepareArguments: > > _SetupPrimaryCoreStack: > mov sp, x1 > - b _PrepareArguments > + > + MOV64 (x8, FixedPcdGet32(PcdCPUCorePrimaryStackSize)) > + MOV64 (x9, 0x5aa55aa55aa55aa5) > +0:sub x10, sp, x8 > + subs x8, x8, #16 > + stp x9, x9, [x10] > + b.mi _PrepareArguments > + b 0b > > gets me from > > Total temporary memory: 16352 bytes. > temporary memory stack ever used: 8176 bytes. > temporary memory heap used for HobList: 4720 bytes. > > to > > Total temporary memory: 16352 bytes. > temporary memory stack ever used: 4820 bytes. > temporary memory heap used for HobList: 4720 bytes. > > so it should simply be a matter of seeding the stack with the correct > magic value in the startup code. Regardless of whether this is of any > use to you in this particular case, I intend to add this to the ARM > startup code. So thanks for spotting this! Right, we should do the same in OVMF SEC then. I didn't realize this was a platform requirement. Given that the platform starts using the stack immediately, after setting it up, in retrospect it looks quite obvious that only the platform can seed the stack. I guess I should prepend a patch like this to the other one I planned to post. > >> - OVMF SEC currently halves the temp RAM between stack and heap. It >> doesn't look hard to update. But, I'm not attracted to tracking down >> other occurrences (if any) of same assumption in other (non-OvmfPkg) >> parts of edk2. >> > > I would not expect there to be many platform wide assumptions > regarding how SEC deals with this, but of course, I am not coming from > the x86 world :-) Haha :) But, you are making a good point; I realize the PCD we use for temp RAM size (combining stack and heap) is from the OvmfPkg.dec file -- its full name is: gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize I'll look more into this as time allows. Thanks! Laszlo