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

[Jordan Justen <jordan.l.justen@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 2384 bytes --]

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.

> >   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
> > 

[-- Attachment #2: 0001-OvmfPkg-Sec-Fill-most-of-temp-RAM-with-PcdInitValueI.patch --]
[-- Type: text/x-diff, Size: 2952 bytes --]

>From 0acb3277e5f7b6ac195c6dbf4fc3ce544e65d9a7 Mon Sep 17 00:00:00 2001
From: Jordan Justen <jordan.l.justen@intel.com>
Date: Sat, 11 Nov 2017 13:25:35 -0800
Subject: [PATCH] OvmfPkg/Sec: Fill most of temp RAM with
 PcdInitValueInTempStack

During a DEBUG build this allows the PEI dispatcher to report the
amount of stack used by SEC/PEI.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
---
 OvmfPkg/Sec/SecMain.c   | 21 ++++++++++++++++++++-
 OvmfPkg/Sec/SecMain.inf |  3 ++-
 2 files changed, 22 insertions(+), 2 deletions(-)

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.<BR>
+  Copyright (c) 2008 - 2017, Intel Corporation. All rights reserved.<BR>
   (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
 
   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));
+  DEBUG_CODE_END ();
+
   //
   // To ensure SMM can't be compromised on S3 resume, we must force re-init of
   // the BaseExtractGuidedSectionLib. Since this is before library contructors
diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf
index 711b595309..c63215393b 100644
--- a/OvmfPkg/Sec/SecMain.inf
+++ b/OvmfPkg/Sec/SecMain.inf
@@ -1,7 +1,7 @@
 ## @file
 #  SEC Driver
 #
-#  Copyright (c) 2008 - 2015, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2008 - 2017, Intel Corporation. All rights reserved.<BR>
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD License
@@ -71,6 +71,7 @@
   gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress
   gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDecompressionScratchEnd
+  gEfiMdeModulePkgTokenSpaceGuid.PcdInitValueInTempStack
 
 [FeaturePcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
-- 
2.15.0