Re: [PATCH 0/4] OvmfPkg: measure temp stack usage, restore temp RAM to 64KB

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 11 November 2017 at 22:04, Jordan Justen <jordan.l.justen@intel.com> wrote:
> 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
>> >     <https://bugzilla.tianocore.org/show_bug.cgi?id=740>
>> >     ("INIT_CAR_VALUE should be defined in a central location").
>> >
>> >   - Ard recently implemented the same in ArmPlatformPkg, for
>> >     <https://bugzilla.tianocore.org/show_bug.cgi?id=748> ("measure temp
>> >     SEC/PEI stack usage").
>> >
>> > The last (fourth) patch adapts OVMF to the larger MtrrLib stack demand
>> > that originates from commit 2bbd7e2fbd4b ("UefiCpuPkg/MtrrLib: Update
>> > algorithm to calculate optimal settings", 2017-09-27). OVMF's temp RAM
>> > size is restored to the historical / original 64KB.
>> >
>> > This work is tracked in
>> > <https://bugzilla.tianocore.org/show_bug.cgi?id=747> ("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 <ard.biesheuvel@linaro.org>
>> > Cc: Jordan Justen <jordan.l.justen@intel.com>
>> > Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>> >
>> > 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 PcdInitValueInTempStack
>> >   OvmfPkg/Sec/X64: seed the temporary RAM with PcdInitValueInTempStack
>>
>> 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 () and having to leave an
arbitrary 512 byte window both seem pretty good reasons to stick with
assembly.

Is your concern that the stack gets cleared in RELEASE builds as well?
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 <jordan.l.justen@intel.com>
>
>> >
>> >  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
>> >