1. Patchew
  2. EDK2
  3. View series

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

Agyeman, Prince posted 1 patch 4 years, 4 months ago
Failed in applying to current master (apply log)
.../WhiskeylakeURvp/OpenBoardPkgPcd.dsc               | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
[edk2-devel] [edk2-platforms] [PATCH v2] WhiskeylakeOpenBoardPkg: Update PCDs to enable stack sharing
Posted by Agyeman, Prince 4 years, 4 months ago 
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..cfe42883be 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|0x28000
 
   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 (#52221): https://edk2.groups.io/g/devel/message/52221
Mute This Topic: https://groups.io/mt/68589828/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [edk2-platforms] [PATCH v2] WhiskeylakeOpenBoardPkg: Update PCDs to enable stack sharing
Posted by Chiu, Chasel 4 years, 4 months ago 
Hi Nate, Prince,

I would recommend that we set larger gSiPkgTokenSpaceGuid.PcdPeiTemporaryRamStackSize so we do not have to update this again and again later when required temporary ram increased by enabling some boot loader features.
How about set it to 0x30000?

Thanks,
Chasel

> -----Original Message-----
> From: Agyeman, Prince <prince.agyeman@intel.com>
> Sent: Saturday, December 14, 2019 9:29 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 v2] 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..cfe42883be 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|0x28000
> 
>    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 (#52236): https://edk2.groups.io/g/devel/message/52236
Mute This Topic: https://groups.io/mt/68589828/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [edk2-platforms] [PATCH v2] WhiskeylakeOpenBoardPkg: Update PCDs to enable stack sharing
Posted by Kubacki, Michael A 4 years, 4 months ago 
I agree with Chasel. There should be enough T-RAM available to expand this to 0x30000 without a problem and reduce potential thrash in the future.

Thanks,
Michael

> -----Original Message-----
> From: Chiu, Chasel <chasel.chiu@intel.com>
> Sent: Sunday, December 15, 2019 4:32 PM
> To: Agyeman, Prince <prince.agyeman@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>
> Cc: Kubacki, Michael A <michael.a.kubacki@intel.com>;
> devel@edk2.groups.io
> Subject: RE: [edk2-platforms] [PATCH v2] WhiskeylakeOpenBoardPkg:
> Update PCDs to enable stack sharing
> 
> 
> Hi Nate, Prince,
> 
> I would recommend that we set larger
> gSiPkgTokenSpaceGuid.PcdPeiTemporaryRamStackSize so we do not have to
> update this again and again later when required temporary ram increased by
> enabling some boot loader features.
> How about set it to 0x30000?
> 
> Thanks,
> Chasel
> 
> > -----Original Message-----
> > From: Agyeman, Prince <prince.agyeman@intel.com>
> > Sent: Saturday, December 14, 2019 9:29 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 v2] 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
> P
> > kgPcd.dsc
> >
> b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoard
> P
> > kgPcd.dsc
> > index 906f7b7ade..cfe42883be 100644
> > ---
> >
> a/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoard
> P
> > kgPcd.dsc
> > +++
> >
> b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoard
> P
> > 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|0x28000
> >
> >    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 (#52257): https://edk2.groups.io/g/devel/message/52257
Mute This Topic: https://groups.io/mt/68589828/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [edk2-platforms] [PATCH v2] WhiskeylakeOpenBoardPkg: Update PCDs to enable stack sharing
Posted by Nate DeSimone 4 years, 4 months ago 
It's a careful balancing act. The more you allocate to temp ram the less will be available to cache the SPI flash, which can impact responsiveness depending on the size of your IBB. I agree that 192KB is probably a good compromise.

Thanks,
Nate

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

I agree with Chasel. There should be enough T-RAM available to expand this to 0x30000 without a problem and reduce potential thrash in the future.

Thanks,
Michael

> -----Original Message-----
> From: Chiu, Chasel <chasel.chiu@intel.com>
> Sent: Sunday, December 15, 2019 4:32 PM
> To: Agyeman, Prince <prince.agyeman@intel.com>; Desimone, Nathaniel L 
> <nathaniel.l.desimone@intel.com>
> Cc: Kubacki, Michael A <michael.a.kubacki@intel.com>; 
> devel@edk2.groups.io
> Subject: RE: [edk2-platforms] [PATCH v2] WhiskeylakeOpenBoardPkg:
> Update PCDs to enable stack sharing
> 
> 
> Hi Nate, Prince,
> 
> I would recommend that we set larger
> gSiPkgTokenSpaceGuid.PcdPeiTemporaryRamStackSize so we do not have to 
> update this again and again later when required temporary ram 
> increased by enabling some boot loader features.
> How about set it to 0x30000?
> 
> Thanks,
> Chasel
> 
> > -----Original Message-----
> > From: Agyeman, Prince <prince.agyeman@intel.com>
> > Sent: Saturday, December 14, 2019 9:29 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 v2] 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
> P
> > kgPcd.dsc
> >
> b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoard
> P
> > kgPcd.dsc
> > index 906f7b7ade..cfe42883be 100644
> > ---
> >
> a/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoard
> P
> > kgPcd.dsc
> > +++
> >
> b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoard
> P
> > 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|0x28000
> >
> >    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 (#52259): https://edk2.groups.io/g/devel/message/52259
Mute This Topic: https://groups.io/mt/68589828/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.