Change in coreboot[master]: soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB
V Sowmya has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36032 )
Change subject: soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB ......................................................................
soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB
The current DCACHE_BSP_STACK_SIZE is set to 128KiB for CML & ICL when FSP uses the same stack provided by coreboot. This patch updates it to 129KiB since default value of DCACHE_BSP_STACK_SIZE must be the sum of FSP stack requirement (128KiB) and CB stack requirement (1KiB).
BUG=b:140268415 TEST=Build and boot CML-Hatch.
Change-Id: Icedff8b42e86dc095fb68deb0b8f80b2667cfeda Signed-off-by: V Sowmya v.sowmya@intel.com --- M src/soc/intel/cannonlake/Kconfig M src/soc/intel/icelake/Kconfig 2 files changed, 6 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/36032/1
diff --git a/src/soc/intel/cannonlake/Kconfig b/src/soc/intel/cannonlake/Kconfig index c1fda95..ace7500 100644 --- a/src/soc/intel/cannonlake/Kconfig +++ b/src/soc/intel/cannonlake/Kconfig @@ -120,11 +120,12 @@
config DCACHE_BSP_STACK_SIZE hex - default 0x20000 if FSP_USES_CB_STACK + default 0x20400 if FSP_USES_CB_STACK default 0x4000 help The amount of anticipated stack usage in CAR by bootblock and - other stages. + other stages.In the case of FSP_USES_CB_STACK default value will be + sum of FSP stack requirement (128KiB) and CB stack requirement (1KiB).
config FSP_TEMP_RAM_SIZE hex diff --git a/src/soc/intel/icelake/Kconfig b/src/soc/intel/icelake/Kconfig index 05fe423..4c5d046 100644 --- a/src/soc/intel/icelake/Kconfig +++ b/src/soc/intel/icelake/Kconfig @@ -70,11 +70,12 @@
config DCACHE_BSP_STACK_SIZE hex - default 0x20000 if FSP_USES_CB_STACK + default 0x204000 if FSP_USES_CB_STACK default 0x4000 help The amount of anticipated stack usage in CAR by bootblock and - other stages. + other stages.In the case of FSP_USES_CB_STACK default value will be + sum of FSP stack requirement (128KiB) and CB stack requirement (1KiB).
config FSP_TEMP_RAM_SIZE hex
Hello Aaron Durbin, Patrick Rudolph, Subrata Banik, Aamir Bohra, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36032
to look at the new patch set (#2).
Change subject: soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB ......................................................................
soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB
The current DCACHE_BSP_STACK_SIZE is set to 128KiB for CML & ICL when FSP uses the same stack provided by coreboot. This patch updates it to 129KiB since default value of DCACHE_BSP_STACK_SIZE must be the sum of FSP stack requirement (128KiB) and CB stack requirement (1KiB).
BUG=b:140268415 TEST=Build and boot CML-Hatch.
Change-Id: Icedff8b42e86dc095fb68deb0b8f80b2667cfeda Signed-off-by: V Sowmya v.sowmya@intel.com --- M src/soc/intel/cannonlake/Kconfig M src/soc/intel/icelake/Kconfig 2 files changed, 6 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/36032/2
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36032 )
Change subject: soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB ......................................................................
Patch Set 2: Code-Review+2
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36032 )
Change subject: soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB ......................................................................
Patch Set 2: Code-Review+2
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36032 )
Change subject: soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB ......................................................................
Patch Set 2: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/36032/2/src/soc/intel/icelake/Kconf... File src/soc/intel/icelake/Kconfig:
https://review.coreboot.org/c/coreboot/+/36032/2/src/soc/intel/icelake/Kconf... PS2, Line 77: In space after '.'
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36032 )
Change subject: soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36032/2/src/soc/intel/cannonlake/Kc... File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/36032/2/src/soc/intel/cannonlake/Kc... PS2, Line 128: sum of FSP stack requirement (128KiB) and CB stack requirement (1KiB). Where did the number 1KiB for coreboot stack come from? I don't remember where, but somewhere stack alignment to 4KiB mattered.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36032 )
Change subject: soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB ......................................................................
Patch Set 2: -Code-Review
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36032 )
Change subject: soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36032/2/src/soc/intel/cannonlake/Kc... File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/36032/2/src/soc/intel/cannonlake/Kc... PS2, Line 128: sum of FSP stack requirement (128KiB) and CB stack requirement (1KiB).
Where did the number 1KiB for coreboot stack come from? I don't remember where, but somewhere stack […]
from coreboot stack usage side we were thinking 1KB must be sufficient. Do you think stack has to be 4KB aligned ? i was thinking that stack alignment will be handled by compiler and it might need to be just 16B aligned
https://review.coreboot.org/c/coreboot/+/36032/2/src/soc/intel/icelake/Kconf... File src/soc/intel/icelake/Kconfig:
https://review.coreboot.org/c/coreboot/+/36032/2/src/soc/intel/icelake/Kconf... PS2, Line 77: In
space after '. […]
good catch
Hello Aaron Durbin, Patrick Rudolph, Subrata Banik, Arthur Heymans, Aamir Bohra, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36032
to look at the new patch set (#3).
Change subject: soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB ......................................................................
soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB
The current DCACHE_BSP_STACK_SIZE is set to 128KiB for CML & ICL when FSP uses the same stack provided by coreboot. This patch updates it to 129KiB since default value of DCACHE_BSP_STACK_SIZE must be the sum of FSP stack requirement (128KiB) and CB stack requirement (1KiB).
BUG=b:140268415 TEST=Build and boot CML-Hatch.
Change-Id: Icedff8b42e86dc095fb68deb0b8f80b2667cfeda Signed-off-by: V Sowmya v.sowmya@intel.com --- M src/soc/intel/cannonlake/Kconfig M src/soc/intel/icelake/Kconfig 2 files changed, 6 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/36032/3
V Sowmya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36032 )
Change subject: soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36032/2/src/soc/intel/cannonlake/Kc... File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/36032/2/src/soc/intel/cannonlake/Kc... PS2, Line 128: sum of FSP stack requirement (128KiB) and CB stack requirement (1KiB).
from coreboot stack usage side we were thinking 1KB must be sufficient. […]
We checked the current coreboot stack usage which is ~400bytes and adding 1KiB to FSP stack requirement must be sufficient. Even i couldn't get any reference to the stack alignment of 4KiB.
https://review.coreboot.org/c/coreboot/+/36032/2/src/soc/intel/icelake/Kconf... File src/soc/intel/icelake/Kconfig:
https://review.coreboot.org/c/coreboot/+/36032/2/src/soc/intel/icelake/Kconf... PS2, Line 77: In
good catch
Done
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36032 )
Change subject: soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36032/2/src/soc/intel/cannonlake/Kc... File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/36032/2/src/soc/intel/cannonlake/Kc... PS2, Line 128: sum of FSP stack requirement (128KiB) and CB stack requirement (1KiB).
We checked the current coreboot stack usage which is ~400bytes and adding 1KiB to FSP stack requirem […]
How? With vboot+measured boot enabled (that's what caused postcar stack increased to 4KiB)?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36032 )
Change subject: soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36032/2/src/soc/intel/cannonlake/Kc... File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/36032/2/src/soc/intel/cannonlake/Kc... PS2, Line 128: sum of FSP stack requirement (128KiB) and CB stack requirement (1KiB).
How? […]
Considering that FSP is the biggest consumer, I believe it should be okay?
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36032 )
Change subject: soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB ......................................................................
Patch Set 3: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/36032/2/src/soc/intel/cannonlake/Kc... File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/36032/2/src/soc/intel/cannonlake/Kc... PS2, Line 128: sum of FSP stack requirement (128KiB) and CB stack requirement (1KiB).
Considering that FSP is the biggest consumer, I believe it should be okay?
The alignment requirement I was thinking was for ramstage and does not apply here.
The comment could be explicit 'CB stack requirement in romstage', vboot/measured boot would not call into FSP.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36032 )
Change subject: soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36032/2/src/soc/intel/cannonlake/Kc... File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/36032/2/src/soc/intel/cannonlake/Kc... PS2, Line 128: sum of FSP stack requirement (128KiB) and CB stack requirement (1KiB).
The alignment requirement I was thinking was for ramstage and does not apply here. […]
@Sowmya, do you think you can address Kyosti's comment ?
V Sowmya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36032 )
Change subject: soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36032/2/src/soc/intel/cannonlake/Kc... File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/36032/2/src/soc/intel/cannonlake/Kc... PS2, Line 128: sum of FSP stack requirement (128KiB) and CB stack requirement (1KiB).
@Sowmya, do you think you can address Kyosti's comment ?
@subrata/Kyosti, I think the description says that this is the stack usage in CAR which is used by bootblock/romsatge and CB stack requirement defined here is part of the CAR region hence is it required to explicitly mention 'CB stack requirement in romstage'?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36032 )
Change subject: soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB ......................................................................
Patch Set 3: Code-Review+2
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36032 )
Change subject: soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB ......................................................................
Patch Set 3: Code-Review-1
(1 comment)
https://review.coreboot.org/c/coreboot/+/36032/2/src/soc/intel/cannonlake/Kc... File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/36032/2/src/soc/intel/cannonlake/Kc... PS2, Line 128: sum of FSP stack requirement (128KiB) and CB stack requirement (1KiB).
@subrata/Kyosti, I think the description says that this is the stack usage in CAR which is used by b […]
@Patric: afaict VBOOT uses a heap defined by vboot_work in car.ld.
@V Sowmya: It should explicitly mention romstage stack usage as kyösti says, as the stack pointer is reinitialized at each stage. measuring bootblock/verstage stack usage has no meaning for romstage where FSP-M is called.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36032 )
Change subject: soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB ......................................................................
Patch Set 3: -Code-Review
(1 comment)
https://review.coreboot.org/c/coreboot/+/36032/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36032/3//COMMIT_MSG@12 PS3, Line 12: CB stack requirement (1KiB CB stack requirement is ~1KiB for romstage alone so far.
@Arthur/Kyösti : this will help right ?
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36032 )
Change subject: soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36032/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36032/3//COMMIT_MSG@10 PS3, Line 10: since default since _the_ default.
https://review.coreboot.org/c/coreboot/+/36032/3//COMMIT_MSG@12 PS3, Line 12: CB stack requirement (1KiB
CB stack requirement is ~1KiB for romstage alone so far.
1KiB -> ~1Kib is good.
@Arthur/Kyösti : this will help right ?
Changing 'FSP' to 'FSP-M' and 'CB stack requirement' to 'CB romstage stack requirement' is good enough for me. Don't forget to do the same in the Kconfig help text.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36032 )
Change subject: soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB ......................................................................
Patch Set 3:
Patch Set 3:
(2 comments)
SGTM.
@Sowmya, can you please help to address those comments ?
Hello Kyösti Mälkki, Aaron Durbin, Patrick Rudolph, Subrata Banik, Arthur Heymans, Aamir Bohra, Tim Wawrzynczak, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36032
to look at the new patch set (#4).
Change subject: soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB ......................................................................
soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB
The current DCACHE_BSP_STACK_SIZE is set to 128KiB for CML & ICL when FSP uses the same stack provided by coreboot. This patch updates it to 129KiB since the default value of DCACHE_BSP_STACK_SIZE must be the sum of FSP-M stack requirement (128KiB) and CB romstage stack requirement (~1KiB).
BUG=b:140268415 TEST=Build and boot CML-Hatch.
Change-Id: Icedff8b42e86dc095fb68deb0b8f80b2667cfeda Signed-off-by: V Sowmya v.sowmya@intel.com --- M src/soc/intel/cannonlake/Kconfig M src/soc/intel/icelake/Kconfig 2 files changed, 6 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/36032/4
V Sowmya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36032 )
Change subject: soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB ......................................................................
Patch Set 4:
(2 comments)
Patch Set 3:
Patch Set 3:
(2 comments)
SGTM.
@Sowmya, can you please help to address those comments?
Yes, I pushed the patchset addressing the comments.
https://review.coreboot.org/c/coreboot/+/36032/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36032/3//COMMIT_MSG@10 PS3, Line 10: since default
since _the_ default.
Done
https://review.coreboot.org/c/coreboot/+/36032/3//COMMIT_MSG@12 PS3, Line 12: CB stack requirement (1KiB
CB stack requirement is ~1KiB for romstage alone so far. […]
Done
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36032 )
Change subject: soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB ......................................................................
Patch Set 4: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/36032/4/src/soc/intel/cannonlake/Kc... File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/36032/4/src/soc/intel/cannonlake/Kc... PS4, Line 128: sum of FSP-M stack requirement (128KiB) and CB romstage stack requirement (~1KiB). please respect the maximum line length
V Sowmya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36032 )
Change subject: soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB ......................................................................
Patch Set 4:
(1 comment)
Patch Set 4: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/36032/4/src/soc/intel/cannonlake/Kc... File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/36032/4/src/soc/intel/cannonlake/Kc... PS4, Line 128: sum of FSP-M stack requirement (128KiB) and CB romstage stack requirement (~1KiB).
please respect the maximum line length
This is within the current limit on the length of lines which is 96 columns. https://doc.coreboot.org/coding_style.html [Breaking long lines and strings].
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36032 )
Change subject: soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB ......................................................................
Patch Set 4: Code-Review+2
(2 comments)
https://review.coreboot.org/c/coreboot/+/36032/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36032/4//COMMIT_MSG@9 PS4, Line 9: The current DCACHE_BSP_STACK_SIZE is set to 128KiB for CML & ICL when FSP uses : the same stack provided by coreboot. This patch updates it to 129KiB since the default : value of DCACHE_BSP_STACK_SIZE must be the sum of FSP-M stack requirement (128KiB) : and CB romstage stack requirement (~1KiB). This is exceeding the 75 char linelength however.
https://review.coreboot.org/c/coreboot/+/36032/4/src/soc/intel/cannonlake/Kc... File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/36032/4/src/soc/intel/cannonlake/Kc... PS4, Line 128: sum of FSP-M stack requirement (128KiB) and CB romstage stack requirement (~1KiB).
This is within the current limit on the length of lines which is 96 columns. […]
For Kconfig option it's nice to have them fit in 80x25 terminals, but searching for this symbol does not show this one.
Maulik V Vaghela has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36032 )
Change subject: soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB ......................................................................
Patch Set 4: Code-Review+2
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36032 )
Change subject: soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB ......................................................................
Patch Set 4: Code-Review+2
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36032 )
Change subject: soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB ......................................................................
Patch Set 4:
@sowmya, kindly help to fix the line alignment issue so we can merge this CL?
Hello Kyösti Mälkki, Aaron Durbin, Patrick Rudolph, Subrata Banik, Arthur Heymans, Aamir Bohra, Maulik V Vaghela, Tim Wawrzynczak, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36032
to look at the new patch set (#5).
Change subject: soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB ......................................................................
soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB
The current DCACHE_BSP_STACK_SIZE is set to 128KiB for CML & ICL when FSP uses the same stack provided by coreboot. This patch updates it to 129KiB since the default value of DCACHE_BSP_STACK_SIZE must be the sum of FSP-M stack requirement (128KiB) and CB romstage stack requirement (~1KiB).
BUG=b:140268415 TEST=Build and boot CML-Hatch.
Change-Id: Icedff8b42e86dc095fb68deb0b8f80b2667cfeda Signed-off-by: V Sowmya v.sowmya@intel.com --- M src/soc/intel/cannonlake/Kconfig M src/soc/intel/icelake/Kconfig 2 files changed, 6 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/36032/5
V Sowmya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36032 )
Change subject: soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36032/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36032/4//COMMIT_MSG@9 PS4, Line 9: The current DCACHE_BSP_STACK_SIZE is set to 128KiB for CML & ICL when FSP uses : the same stack provided by coreboot. This patch updates it to 129KiB since the default : value of DCACHE_BSP_STACK_SIZE must be the sum of FSP-M stack requirement (128KiB) : and CB romstage stack requirement (~1KiB).
This is exceeding the 75 char linelength however.
Done
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36032 )
Change subject: soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB ......................................................................
Patch Set 5: Code-Review+2
Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/36032 )
Change subject: soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB ......................................................................
soc/intel/{cnl, icl}: Update the DCACHE_BSP_STACK_SIZE to 129KiB
The current DCACHE_BSP_STACK_SIZE is set to 128KiB for CML & ICL when FSP uses the same stack provided by coreboot. This patch updates it to 129KiB since the default value of DCACHE_BSP_STACK_SIZE must be the sum of FSP-M stack requirement (128KiB) and CB romstage stack requirement (~1KiB).
BUG=b:140268415 TEST=Build and boot CML-Hatch.
Change-Id: Icedff8b42e86dc095fb68deb0b8f80b2667cfeda Signed-off-by: V Sowmya v.sowmya@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/36032 Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org Reviewed-by: Arthur Heymans arthur@aheymans.xyz Reviewed-by: Maulik V Vaghela maulik.v.vaghela@intel.com Reviewed-by: Subrata Banik subrata.banik@intel.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/cannonlake/Kconfig M src/soc/intel/icelake/Kconfig 2 files changed, 6 insertions(+), 4 deletions(-)
Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved Subrata Banik: Looks good to me, approved Maulik V Vaghela: Looks good to me, approved Tim Wawrzynczak: Looks good to me, approved
diff --git a/src/soc/intel/cannonlake/Kconfig b/src/soc/intel/cannonlake/Kconfig index c1fda95..c1f53b1 100644 --- a/src/soc/intel/cannonlake/Kconfig +++ b/src/soc/intel/cannonlake/Kconfig @@ -120,11 +120,12 @@
config DCACHE_BSP_STACK_SIZE hex - default 0x20000 if FSP_USES_CB_STACK + default 0x20400 if FSP_USES_CB_STACK default 0x4000 help The amount of anticipated stack usage in CAR by bootblock and - other stages. + other stages. In the case of FSP_USES_CB_STACK default value will be + sum of FSP-M stack requirement (128KiB) and CB romstage stack requirement (~1KiB).
config FSP_TEMP_RAM_SIZE hex diff --git a/src/soc/intel/icelake/Kconfig b/src/soc/intel/icelake/Kconfig index 05fe423..4ae043a 100644 --- a/src/soc/intel/icelake/Kconfig +++ b/src/soc/intel/icelake/Kconfig @@ -70,11 +70,12 @@
config DCACHE_BSP_STACK_SIZE hex - default 0x20000 if FSP_USES_CB_STACK + default 0x20400 if FSP_USES_CB_STACK default 0x4000 help The amount of anticipated stack usage in CAR by bootblock and - other stages. + other stages. In the case of FSP_USES_CB_STACK default value will be + sum of FSP-M stack requirement (128KiB) and CB romstage stack requirement (~1KiB).
config FSP_TEMP_RAM_SIZE hex
-
Aaron Durbin (Code Review) -
Arthur Heymans (Code Review)
-
Furquan Shaikh (Code Review)
-
Kyösti Mälkki (Code Review)
-
Maulik V Vaghela (Code Review)
-
Patrick Rudolph (Code Review)
-
Subrata Banik (Code Review)
-
Tim Wawrzynczak (Code Review)
-
V Sowmya (Code Review)