From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c06::22a; helo=mail-io0-x22a.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-io0-x22a.google.com (mail-io0-x22a.google.com [IPv6:2607:f8b0:4001:c06::22a]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 65C8E2034715B for ; Fri, 20 Oct 2017 03:35:35 -0700 (PDT) Received: by mail-io0-x22a.google.com with SMTP id i38so12774356iod.2 for ; Fri, 20 Oct 2017 03:39:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=93efMfs6XsbovcUog0XSWpLQGpVHxjBbeNwLa6+P1YI=; b=bqjqszgn9lQeRr9k35xKE8+f1RW+FQbyzQVkKCZtSbs6NlVArX2/W+vWcusKMq5WnP UYECoHMEZMPxqVn8UlHYopv3Ip5YwdVwC3oFIZkW+U5M/9s4Z7zoYCMzuFDv07y79Rpy dEG+inm+bjk6fZrwRikCxmQSDRSvZJdjaaYz8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=93efMfs6XsbovcUog0XSWpLQGpVHxjBbeNwLa6+P1YI=; b=Lmz5S7WCAZ9iJ+nWlfdrEVF9QHuC657baM0Vi7mcU8WbaVwjxTvUcgid399waf1n2/ /iWRUOs3jrco+PIrEo5EMAyMgs4g5m+xFXEOWWAmeRPHuQVVHnOw4E3aqQReFT6ca29C PxaTnBCMirXV1zMZkOH3SItaP8JQ9aJxJRgWrkrg6ej0OM56HIAlSP6mPiNSDMszpqNu GvVMIfh3rbSEGlgQP2MrrfyVZ4LXhopPShwb7ua5c+Ns9V7rLa1RuXE4XJD+LFz+CKrM 4wMDfjYdvTDxJIDI4NpCyZvI4oICf+VWVw7TZrykaMDntiEHrOc/Uo+6/m8PbRNFDVaZ pBgw== X-Gm-Message-State: AMCzsaX78i++rSbmn3GRyMf3cXXb+Jfp23k8zztFbbT7iZgPS6kmBiKB OdKKXe+rZu+TgMLIu2pVVE7CHoPid6Ia/8aD9AsMynzm X-Google-Smtp-Source: ABhQp+St9Z6mtm6WrMBIHmQDsyLrKSF3c2sbLUip3HbdeQu0+frgibhfsw/NpW5f909iF/RXIDSMwDReLmaZHBx1oyU= X-Received: by 10.107.142.208 with SMTP id q199mr5961356iod.186.1508495953773; Fri, 20 Oct 2017 03:39:13 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.131.167 with HTTP; Fri, 20 Oct 2017 03:39:13 -0700 (PDT) In-Reply-To: <0a8bf4a6-5a50-9e22-ddc7-002383ea3935@redhat.com> References: <0a8bf4a6-5a50-9e22-ddc7-002383ea3935@redhat.com> From: Ard Biesheuvel Date: Fri, 20 Oct 2017 11:39:13 +0100 Message-ID: To: Laszlo Ersek Cc: "Gao, Liming" , Star Zeng , "Jordan Justen (Intel address)" , edk2-devel-01 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:35:35 -0000 Content-Type: text/plain; charset="UTF-8" On 20 October 2017 at 11:33, Laszlo Ersek wrote: > 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. > Yes. I'll gladly defer to the experts in this case, even though I co-own OVMF these days ... > 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. > Yes. But even though PEI core seems to assume it, only Nt32Pkg appears to implement it atm. > 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