public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: Laszlo Ersek <lersek@redhat.com>
Cc: "Gao, Liming" <liming.gao@intel.com>,
	Star Zeng <star.zeng@intel.com>,
	 "Jordan Justen (Intel address)" <jordan.l.justen@intel.com>,
	edk2-devel-01 <edk2-devel@lists.01.org>
Subject: Re: dynamic PCD impact on temporary PEI memory
Date: Fri, 20 Oct 2017 11:39:13 +0100	[thread overview]
Message-ID: <CAKv+Gu-=3vjy=_K-0xqwZsC_ACOUZEKJifAS+dmobA5uKR-rFg@mail.gmail.com> (raw)
In-Reply-To: <0a8bf4a6-5a50-9e22-ddc7-002383ea3935@redhat.com>

On 20 October 2017 at 11:33, Laszlo Ersek <lersek@redhat.com> wrote:
> On 10/20/17 12:22, Ard Biesheuvel wrote:
>> On 20 October 2017 at 10:43, Laszlo Ersek <lersek@redhat.com> wrote:
>>> On 10/20/17 09:57, Ard Biesheuvel wrote:
>>>> On 19 October 2017 at 23:08, Laszlo Ersek <lersek@redhat.com> 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


      reply	other threads:[~2017-10-20 10:35 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-19 22:08 dynamic PCD impact on temporary PEI memory Laszlo Ersek
2017-10-20  7:57 ` Ard Biesheuvel
2017-10-20  9:43   ` Laszlo Ersek
2017-10-20 10:22     ` Ard Biesheuvel
2017-10-20 10:33       ` Laszlo Ersek
2017-10-20 10:39         ` Ard Biesheuvel [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAKv+Gu-=3vjy=_K-0xqwZsC_ACOUZEKJifAS+dmobA5uKR-rFg@mail.gmail.com' \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox