[edk2-devel] [edk2-platforms] [PATCH] WhiskeylakeOpenBoardPkg: Update PCDs to enable stack sharing

[Agyeman, Prince]

Thanks for clarifying. 

Yes the minimum stack size of 160KB works as well.

I will update this patch

Thanks
Prince

-----Original Message-----
From: Desimone, Nathaniel L <nathaniel.l.desimone at intel.com> 
Sent: Friday, December 13, 2019 2:27 PM
To: devel at edk2.groups.io; Desimone, Nathaniel L <nathaniel.l.desimone at intel.com>; Agyeman, Prince <prince.agyeman at intel.com>
Cc: Chiu, Chasel <chasel.chiu at intel.com>; Kubacki, Michael A <michael.a.kubacki at intel.com>
Subject: RE: [edk2-devel] [edk2-platforms] [PATCH] WhiskeylakeOpenBoardPkg: Update PCDs to enable stack sharing

Actually looking at the CoffeeLake FSP Integration Guide it states the minimum stack size to be 160KB (0x28000) so I guess a slight revision to my previous statement:

gSiPkgTokenSpaceGuid.PcdPeiTemporaryRamStackSize|0x28000

Thanks,
Nate

-----Original Message-----
From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Nate DeSimone
Sent: Friday, December 13, 2019 2:14 PM
To: Agyeman, Prince <prince.agyeman at intel.com>; devel at edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu at intel.com>; Kubacki, Michael A <michael.a.kubacki at intel.com>
Subject: Re: [edk2-devel] [edk2-platforms] [PATCH] WhiskeylakeOpenBoardPkg: Update PCDs to enable stack sharing

Hi Prince,

Looking at your patch I see the following two changes:

1. Reduce FSP Temp Ram size from 152KB to 64KB:

-  gIntelFsp2PkgTokenSpaceGuid.PcdFspTemporaryRamSize|0x26000
+  gIntelFsp2PkgTokenSpaceGuid.PcdFspTemporaryRamSize|0x10000

This makes sense because now that the FSP is using the single stack it does not need as much temp ram.

2. Increase the size of the platform's PEI phase stack from 128KB to 256KB:

-  gSiPkgTokenSpaceGuid.PcdPeiTemporaryRamStackSize|0x20000
+  gSiPkgTokenSpaceGuid.PcdPeiTemporaryRamStackSize|0x40000

This also makes sense because the FSP is now running on the same stack as the rest of the platform code, so more space will be needed to run MRC. What I am questioning is if we really need 256KB? Looking at the numbers from before, it seems like MRC was running OK with only 152KB of stack space, does platform code really use that much stack space _at_the_same_time_ that MRC is running? Or maybe, the newer version of MRC uses more stack space now?

The basic summary is... have you tried booting with gSiPkgTokenSpaceGuid.PcdPeiTemporaryRamStackSize|0x26000?

Thanks,
Nate

-----Original Message-----
From: Agyeman, Prince <prince.agyeman at intel.com> 
Sent: Friday, December 13, 2019 8:25 AM
To: Desimone, Nathaniel L <nathaniel.l.desimone at intel.com>; devel at edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu at intel.com>; Kubacki, Michael A <michael.a.kubacki at intel.com>
Subject: RE: [edk2-platforms] [PATCH] WhiskeylakeOpenBoardPkg: Update PCDs to enable stack sharing


Hi Nate,

Which 256KB are you referring to ?

The temporary ram size was reduced from  152KB (0x26000) to 64KB (0x10000)

With stack sharing enabled in FSP,  temp ram size of 0x26000 causes system to hang during memory training.

-  gIntelFsp2PkgTokenSpaceGuid.PcdFspTemporaryRamSize |0x26000
+  gIntelFsp2PkgTokenSpaceGuid.PcdFspTemporaryRamSize|0x10000

-----Original Message-----
From: Desimone, Nathaniel L <nathaniel.l.desimone at intel.com>
Sent: Thursday, December 12, 2019 3:07 PM
To: Agyeman, Prince <prince.agyeman at intel.com>; devel at edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu at intel.com>; Kubacki, Michael A <michael.a.kubacki at intel.com>
Subject: RE: [edk2-platforms] [PATCH] WhiskeylakeOpenBoardPkg: Update PCDs to enable stack sharing

Hi Prince,

Is 256KB really necessary? Could you try the 152KB (0x26000) that we had previously?

Thanks,
Nate

-----Original Message-----
From: Agyeman, Prince <prince.agyeman at intel.com>
Sent: Wednesday, December 11, 2019 5:13 PM
To: devel at edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu at intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone at intel.com>; Kubacki, Michael A <michael.a.kubacki at intel.com>
Subject: [edk2-platforms] [PATCH] WhiskeylakeOpenBoardPkg: Update PCDs to enable stack sharing

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2409

Updated WhiskeylakeURvp PCDs to enable FSP/BL stack sharing.
This fixes the boot failure seen with the latest Coffee Lake (CFL) FSP binary (v 7.0.68.41)

Cc: Chasel Chiu <chasel.chiu at intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone at intel.com>
Cc: Michael Kubacki <michael.a.kubacki at intel.com>

Signed-off-by: Prince Agyeman <prince.agyeman at intel.com>
---
 .../WhiskeylakeURvp/OpenBoardPkgPcd.dsc               | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardPkgPcd.dsc b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardPkgPcd.dsc
index 906f7b7ade..b3e1da3970 100644
--- a/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardPkgPcd.dsc
+++ b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardPk
+++ gPcd.dsc
@@ -54,15 +54,14 @@
   gSiPkgTokenSpaceGuid.PcdTsegSize|0x1000000
 
   #
-  # FSP API mode does not share stack with the boot loader,
-  # so FSP needs more temporary memory for FSP heap + stack size.
+  # When sharing stack with boot loader, FSP only needs small temp ram 
+ for heap
   #
-  gIntelFsp2PkgTokenSpaceGuid.PcdFspTemporaryRamSize|0x26000
+  gIntelFsp2PkgTokenSpaceGuid.PcdFspTemporaryRamSize|0x10000
+
   #
-  # FSP API mode does not need to enlarge the boot loader stack size
-  # since the stacks are separate.
+  # Boot loader stack size has to be big enough to executing FSP
   #
-  gSiPkgTokenSpaceGuid.PcdPeiTemporaryRamStackSize|0x20000
+  gSiPkgTokenSpaceGuid.PcdPeiTemporaryRamStackSize|0x40000
 
   gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0xE0000000
   gMinPlatformPkgTokenSpaceGuid.PcdPciExpressRegionLength|0x10000000
--
2.19.1.windows.1








-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#52219): https://edk2.groups.io/g/devel/message/52219
Mute This Topic: https://groups.io/mt/68227150/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-