Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34006
Change subject: mb/google/hatch: Fix PRE-CBMEM console overflow issue ......................................................................
mb/google/hatch: Fix PRE-CBMEM console overflow issue
This patch increases PRERAM_CBMEM_CONSOLE_SIZE to fix *** Pre-CBMEM romstage console overflowed, log truncated! *** issue.
Change-Id: I2de4ca2f2001b304850c27df1b3c3b2c827fe25a Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/mainboard/google/hatch/Kconfig 1 file changed, 4 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/34006/1
diff --git a/src/mainboard/google/hatch/Kconfig b/src/mainboard/google/hatch/Kconfig index 8045129..26601d8 100644 --- a/src/mainboard/google/hatch/Kconfig +++ b/src/mainboard/google/hatch/Kconfig @@ -108,6 +108,10 @@ default "kindred" if BOARD_GOOGLE_KINDRED default "kohaku" if BOARD_GOOGLE_KOHAKU
+config PRERAM_CBMEM_CONSOLE_SIZE + hex + default 0xe00 + config VBOOT select HAS_RECOVERY_MRC_CACHE select MRC_CLEAR_NORMAL_CACHE_ON_RECOVERY_RETRAIN
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34006 )
Change subject: mb/google/hatch: Fix PRE-CBMEM console overflow issue ......................................................................
Patch Set 1:
(1 comment)
Hmmm, why make it board specific? If we can safely increase its size (I don't know how to check?), isn't it also possible for all other boards of the same platform?
https://review.coreboot.org/#/c/34006/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/34006/1//COMMIT_MSG@11 PS1, Line 11: issue. You cannot really "fix" it, as it depends on the log level. It's more a mitigation, isn't it?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34006 )
Change subject: mb/google/hatch: Fix PRE-CBMEM console overflow issue ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/34006/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/34006/1//COMMIT_MSG@11 PS1, Line 11: issue.
You cannot really "fix" it, as it depends on the log level. It's […]
:) agree, i had one internal bug hence i have mentioned fixing issue. but i will wait for our team to test and provide feedback. We can go ahead and increase default value itself, it its really fix
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34006 )
Change subject: mb/google/hatch: Fix PRE-CBMEM console overflow issue ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34006/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34006/1//COMMIT_MSG@11 PS1, Line 11: issue.
:) agree, i had one internal bug hence i have mentioned fixing issue. […]
@Nico, This fixes our issue, so we would like to submit this CL but as you said to increase default value itself. I have few opens
1. below are the mb which override default value
src/mainboard/asrock/h110m/Kconfig:40:config PRERAM_CBMEM_CONSOLE_SIZE = 0xd00 src/mainboard/google/reef/Kconfig:108:config PRERAM_CBMEM_CONSOLE_SIZE = 0xe00 src/mainboard/intel/kblrvp/Kconfig:81:config PRERAM_CBMEM_CONSOLE_SIZE = 0xd00 src/soc/amd/stoneyridge/Kconfig:106:config PRERAM_CBMEM_CONSOLE_SIZE =0x1600 src/soc/amd/picasso/Kconfig:99:config PRERAM_CBMEM_CONSOLE_SIZE = 0x1600
2. I don;t think its good idea to make default as 0x1600 which is selected max today from mb and remove this override from mb side
3. I would recommend to increase default from 0xc00 -> 0xe00 and remove mb overrides which falls in that category. I'm not very sure why on AMD platform its set to very high value.
need ur feedback. Other our CL holds good for review
V Sowmya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34006 )
Change subject: mb/google/hatch: Fix PRE-CBMEM console overflow issue ......................................................................
Patch Set 1: Code-Review+2
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34006 )
Change subject: mb/google/hatch: Fix PRE-CBMEM console overflow issue ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34006/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34006/1//COMMIT_MSG@11 PS1, Line 11: issue.
@Nico, This fixes our issue, so we would like to submit this CL […]
I would rather only increase the default per platform. It doesn't have to be at the mainboard level (though, a mainboard can still override a platform default).
e.g. if this 0xe00 works for all Hatch variants, it should work for all mainboards with WHL/CML too.
Before C_ENVIRONMENT_BOOTBLOCK, we had the problem that the console could conflict with the stack. I think that changed, however, so we could actually make the console as big as pos- sible on platforms with C_ENVIRONMENT_BOOTBLOCK (i.e. the linker should tell us if it's too big).
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34006 )
Change subject: mb/google/hatch: Fix PRE-CBMEM console overflow issue ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34006/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34006/1//COMMIT_MSG@11 PS1, Line 11: issue.
I would rather only increase the default per platform. It doesn't have to be at the mainboard level (though, a mainboard can still override a platform default).
e.g. if this 0xe00 works for all Hatch variants, it should work for all mainboards with WHL/CML too.
make sense to me.
Before C_ENVIRONMENT_BOOTBLOCK, we had the problem that the console could conflict with the stack. I think that changed, however, so we could actually make the console as big as pos- sible on platforms with C_ENVIRONMENT_BOOTBLOCK (i.e. the linker should tell us if it's too big).
Hello Spoorthi K, V Sowmya, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34006
to look at the new patch set (#2).
Change subject: soc/intel/cannonlake: Override PRERAM_CBMEM_CONSOLE_SIZE default value ......................................................................
soc/intel/cannonlake: Override PRERAM_CBMEM_CONSOLE_SIZE default value
This patch increases PRERAM_CBMEM_CONSOLE_SIZE to fix *** Pre-CBMEM romstage console overflowed, log truncated! *** issue.
TEST=Verified on Hatch CML platform.
Change-Id: I2de4ca2f2001b304850c27df1b3c3b2c827fe25a Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/soc/intel/cannonlake/Kconfig 1 file changed, 4 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/34006/2
V Sowmya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34006 )
Change subject: soc/intel/cannonlake: Override PRERAM_CBMEM_CONSOLE_SIZE default value ......................................................................
Patch Set 2: Code-Review+2
Spoorthi K has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34006 )
Change subject: soc/intel/cannonlake: Override PRERAM_CBMEM_CONSOLE_SIZE default value ......................................................................
Patch Set 2: Code-Review+1
Verified. The patch fixes the issue. Thanks Subrata.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34006 )
Change subject: soc/intel/cannonlake: Override PRERAM_CBMEM_CONSOLE_SIZE default value ......................................................................
Patch Set 2: Code-Review+1
Subrata Banik has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34006 )
Change subject: soc/intel/cannonlake: Override PRERAM_CBMEM_CONSOLE_SIZE default value ......................................................................
soc/intel/cannonlake: Override PRERAM_CBMEM_CONSOLE_SIZE default value
This patch increases PRERAM_CBMEM_CONSOLE_SIZE to fix *** Pre-CBMEM romstage console overflowed, log truncated! *** issue.
TEST=Verified on Hatch CML platform.
Change-Id: I2de4ca2f2001b304850c27df1b3c3b2c827fe25a Signed-off-by: Subrata Banik subrata.banik@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/34006 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: V Sowmya v.sowmya@intel.com Reviewed-by: Spoorthi K Reviewed-by: Nico Huber nico.h@gmx.de --- M src/soc/intel/cannonlake/Kconfig 1 file changed, 4 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, but someone else must approve V Sowmya: Looks good to me, approved Spoorthi K: Looks good to me, but someone else must approve
diff --git a/src/soc/intel/cannonlake/Kconfig b/src/soc/intel/cannonlake/Kconfig index d697620..5e405db 100644 --- a/src/soc/intel/cannonlake/Kconfig +++ b/src/soc/intel/cannonlake/Kconfig @@ -326,4 +326,8 @@ This sets the Enable8254ClockGating UPD, which according to the FSP Integration guide needs to be disabled in order to boot SeaBIOS, but should otherwise be enabled.
+config PRERAM_CBMEM_CONSOLE_SIZE + hex + default 0xe00 + endif
-
Nico Huber (Code Review) -
Spoorthi K (Code Review)
-
Subrata Banik (Code Review)
-
V Sowmya (Code Review)