* [edk2-platforms] [PATCH] WhiskeylakeOpenBoardPkg: Update PCDs to enable stack sharing
@ 2019-12-12 1:13 Agyeman, Prince
2019-12-12 11:38 ` Chiu, Chasel
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Agyeman, Prince @ 2019-12-12 1:13 UTC (permalink / raw)
To: devel; +Cc: Chasel Chiu, Nate DeSimone, Michael Kubacki
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@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Michael Kubacki <michael.a.kubacki@intel.com>
Signed-off-by: Prince Agyeman <prince.agyeman@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/OpenBoardPkgPcd.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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [edk2-platforms] [PATCH] WhiskeylakeOpenBoardPkg: Update PCDs to enable stack sharing
2019-12-12 1:13 [edk2-platforms] [PATCH] WhiskeylakeOpenBoardPkg: Update PCDs to enable stack sharing Agyeman, Prince
@ 2019-12-12 11:38 ` Chiu, Chasel
2019-12-12 18:13 ` Kubacki, Michael A
2019-12-12 23:06 ` Nate DeSimone
2 siblings, 0 replies; 8+ messages in thread
From: Chiu, Chasel @ 2019-12-12 11:38 UTC (permalink / raw)
To: Agyeman, Prince, devel@edk2.groups.io
Cc: Desimone, Nathaniel L, Kubacki, Michael A
Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>
> -----Original Message-----
> From: Agyeman, Prince <prince.agyeman@intel.com>
> Sent: Thursday, December 12, 2019 9:13 AM
> To: devel@edk2.groups.io
> Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Kubacki, Michael A
> <michael.a.kubacki@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@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Michael Kubacki <michael.a.kubacki@intel.com>
>
> Signed-off-by: Prince Agyeman <prince.agyeman@intel.com>
> ---
> .../WhiskeylakeURvp/OpenBoardPkgPcd.dsc | 11
> +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git
> a/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardP
> kgPcd.dsc
> b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardP
> kgPcd.dsc
> index 906f7b7ade..b3e1da3970 100644
> ---
> a/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardP
> kgPcd.dsc
> +++
> b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardP
> k
> +++ 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-platforms] [PATCH] WhiskeylakeOpenBoardPkg: Update PCDs to enable stack sharing
2019-12-12 1:13 [edk2-platforms] [PATCH] WhiskeylakeOpenBoardPkg: Update PCDs to enable stack sharing Agyeman, Prince
2019-12-12 11:38 ` Chiu, Chasel
@ 2019-12-12 18:13 ` Kubacki, Michael A
2019-12-12 23:06 ` Nate DeSimone
2 siblings, 0 replies; 8+ messages in thread
From: Kubacki, Michael A @ 2019-12-12 18:13 UTC (permalink / raw)
To: Agyeman, Prince, devel@edk2.groups.io; +Cc: Chiu, Chasel, Desimone, Nathaniel L
Reviewed-by: Michael Kubacki <michael.a.kubacki@intel.com>
> -----Original Message-----
> From: Agyeman, Prince <prince.agyeman@intel.com>
> Sent: Wednesday, December 11, 2019 5:13 PM
> To: devel@edk2.groups.io
> Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Kubacki, Michael A
> <michael.a.kubacki@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@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Michael Kubacki <michael.a.kubacki@intel.com>
>
> Signed-off-by: Prince Agyeman <prince.agyeman@intel.com>
> ---
> .../WhiskeylakeURvp/OpenBoardPkgPcd.dsc | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git
> a/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoard
> PkgPcd.dsc
> b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoard
> PkgPcd.dsc
> index 906f7b7ade..b3e1da3970 100644
> ---
> a/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoard
> PkgPcd.dsc
> +++
> b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoard
> Pk
> +++ 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-platforms] [PATCH] WhiskeylakeOpenBoardPkg: Update PCDs to enable stack sharing
2019-12-12 1:13 [edk2-platforms] [PATCH] WhiskeylakeOpenBoardPkg: Update PCDs to enable stack sharing Agyeman, Prince
2019-12-12 11:38 ` Chiu, Chasel
2019-12-12 18:13 ` Kubacki, Michael A
@ 2019-12-12 23:06 ` Nate DeSimone
2019-12-13 16:25 ` Agyeman, Prince
2 siblings, 1 reply; 8+ messages in thread
From: Nate DeSimone @ 2019-12-12 23:06 UTC (permalink / raw)
To: Agyeman, Prince, devel@edk2.groups.io; +Cc: Chiu, Chasel, Kubacki, Michael A
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@intel.com>
Sent: Wednesday, December 11, 2019 5:13 PM
To: devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Kubacki, Michael A <michael.a.kubacki@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@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Michael Kubacki <michael.a.kubacki@intel.com>
Signed-off-by: Prince Agyeman <prince.agyeman@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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-platforms] [PATCH] WhiskeylakeOpenBoardPkg: Update PCDs to enable stack sharing
2019-12-12 23:06 ` Nate DeSimone
@ 2019-12-13 16:25 ` Agyeman, Prince
2019-12-13 22:13 ` Nate DeSimone
[not found] ` <15E00DF9E44B8812.24131@groups.io>
0 siblings, 2 replies; 8+ messages in thread
From: Agyeman, Prince @ 2019-12-13 16:25 UTC (permalink / raw)
To: Desimone, Nathaniel L, devel@edk2.groups.io
Cc: Chiu, Chasel, Kubacki, Michael A
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@intel.com>
Sent: Thursday, December 12, 2019 3:07 PM
To: Agyeman, Prince <prince.agyeman@intel.com>; devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Kubacki, Michael A <michael.a.kubacki@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@intel.com>
Sent: Wednesday, December 11, 2019 5:13 PM
To: devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Kubacki, Michael A <michael.a.kubacki@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@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Michael Kubacki <michael.a.kubacki@intel.com>
Signed-off-by: Prince Agyeman <prince.agyeman@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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-platforms] [PATCH] WhiskeylakeOpenBoardPkg: Update PCDs to enable stack sharing
2019-12-13 16:25 ` Agyeman, Prince
@ 2019-12-13 22:13 ` Nate DeSimone
[not found] ` <15E00DF9E44B8812.24131@groups.io>
1 sibling, 0 replies; 8+ messages in thread
From: Nate DeSimone @ 2019-12-13 22:13 UTC (permalink / raw)
To: Agyeman, Prince, devel@edk2.groups.io; +Cc: Chiu, Chasel, Kubacki, Michael A
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@intel.com>
Sent: Friday, December 13, 2019 8:25 AM
To: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Kubacki, Michael A <michael.a.kubacki@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@intel.com>
Sent: Thursday, December 12, 2019 3:07 PM
To: Agyeman, Prince <prince.agyeman@intel.com>; devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Kubacki, Michael A <michael.a.kubacki@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@intel.com>
Sent: Wednesday, December 11, 2019 5:13 PM
To: devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Kubacki, Michael A <michael.a.kubacki@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@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Michael Kubacki <michael.a.kubacki@intel.com>
Signed-off-by: Prince Agyeman <prince.agyeman@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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [edk2-platforms] [PATCH] WhiskeylakeOpenBoardPkg: Update PCDs to enable stack sharing
[not found] ` <15E00DF9E44B8812.24131@groups.io>
@ 2019-12-13 22:27 ` Nate DeSimone
2019-12-13 23:39 ` Agyeman, Prince
0 siblings, 1 reply; 8+ messages in thread
From: Nate DeSimone @ 2019-12-13 22:27 UTC (permalink / raw)
To: devel@edk2.groups.io, Desimone, Nathaniel L, Agyeman, Prince
Cc: Chiu, Chasel, Kubacki, Michael A
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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Nate DeSimone
Sent: Friday, December 13, 2019 2:14 PM
To: Agyeman, Prince <prince.agyeman@intel.com>; devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Kubacki, Michael A <michael.a.kubacki@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@intel.com>
Sent: Friday, December 13, 2019 8:25 AM
To: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Kubacki, Michael A <michael.a.kubacki@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@intel.com>
Sent: Thursday, December 12, 2019 3:07 PM
To: Agyeman, Prince <prince.agyeman@intel.com>; devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Kubacki, Michael A <michael.a.kubacki@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@intel.com>
Sent: Wednesday, December 11, 2019 5:13 PM
To: devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Kubacki, Michael A <michael.a.kubacki@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@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Michael Kubacki <michael.a.kubacki@intel.com>
Signed-off-by: Prince Agyeman <prince.agyeman@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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [edk2-platforms] [PATCH] WhiskeylakeOpenBoardPkg: Update PCDs to enable stack sharing
2019-12-13 22:27 ` [edk2-devel] " Nate DeSimone
@ 2019-12-13 23:39 ` Agyeman, Prince
0 siblings, 0 replies; 8+ messages in thread
From: Agyeman, Prince @ 2019-12-13 23:39 UTC (permalink / raw)
To: Desimone, Nathaniel L, devel@edk2.groups.io
Cc: Chiu, Chasel, Kubacki, Michael A
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@intel.com>
Sent: Friday, December 13, 2019 2:27 PM
To: devel@edk2.groups.io; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Agyeman, Prince <prince.agyeman@intel.com>
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Kubacki, Michael A <michael.a.kubacki@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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Nate DeSimone
Sent: Friday, December 13, 2019 2:14 PM
To: Agyeman, Prince <prince.agyeman@intel.com>; devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Kubacki, Michael A <michael.a.kubacki@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@intel.com>
Sent: Friday, December 13, 2019 8:25 AM
To: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Kubacki, Michael A <michael.a.kubacki@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@intel.com>
Sent: Thursday, December 12, 2019 3:07 PM
To: Agyeman, Prince <prince.agyeman@intel.com>; devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Kubacki, Michael A <michael.a.kubacki@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@intel.com>
Sent: Wednesday, December 11, 2019 5:13 PM
To: devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Kubacki, Michael A <michael.a.kubacki@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@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Michael Kubacki <michael.a.kubacki@intel.com>
Signed-off-by: Prince Agyeman <prince.agyeman@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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-12-13 23:39 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-12-12 1:13 [edk2-platforms] [PATCH] WhiskeylakeOpenBoardPkg: Update PCDs to enable stack sharing Agyeman, Prince
2019-12-12 11:38 ` Chiu, Chasel
2019-12-12 18:13 ` Kubacki, Michael A
2019-12-12 23:06 ` Nate DeSimone
2019-12-13 16:25 ` Agyeman, Prince
2019-12-13 22:13 ` Nate DeSimone
[not found] ` <15E00DF9E44B8812.24131@groups.io>
2019-12-13 22:27 ` [edk2-devel] " Nate DeSimone
2019-12-13 23:39 ` Agyeman, Prince
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox