Patrick Georgi has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33068
Change subject: soc/rockchip/rk3288: Disable bootblock console ......................................................................
soc/rockchip/rk3288: Disable bootblock console
Bootblock space is tight on this SoC and recent changes increased it ever so slightly to make this a problem.
Since the bootblock is well-tested, we can get by without console.
Change-Id: I7496a3e313b2c6ee6fb3c4671eac64376d84e0dc Signed-off-by: Patrick Georgi pgeorgi@google.com --- M src/soc/rockchip/rk3288/Kconfig 1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/33068/1
diff --git a/src/soc/rockchip/rk3288/Kconfig b/src/soc/rockchip/rk3288/Kconfig index 38d8752..f845f07 100644 --- a/src/soc/rockchip/rk3288/Kconfig +++ b/src/soc/rockchip/rk3288/Kconfig @@ -30,6 +30,7 @@ select MAINBOARD_HAS_NATIVE_VGA_INIT select MAINBOARD_FORCE_NATIVE_VGA_INIT select HAVE_LINEAR_FRAMEBUFFER + select NO_BOOTBLOCK_CONSOLE
if SOC_ROCKCHIP_RK3288
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33068 )
Change subject: soc/rockchip/rk3288: Disable bootblock console ......................................................................
Patch Set 1:
The issue only appears when compiling with CONFIG_CHROMEOS, so we could make it contingent on that. Also, enabling LTO in coreboot will probably make enough room for things to work again, but that's a larger change than I'd propose to get master green again.
The original issue came up when various die() calls were replaced by die_with_post_code(). Interestingly I couldn't make it go away by #defining the latter to the former (while dropping the post code part)
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33068 )
Change subject: soc/rockchip/rk3288: Disable bootblock console ......................................................................
Patch Set 1: Code-Review+2
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33068 )
Change subject: soc/rockchip/rk3288: Disable bootblock console ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/#/c/33068/1/src/soc/rockchip/rk3288/Kconfig File src/soc/rockchip/rk3288/Kconfig:
https://review.coreboot.org/#/c/33068/1/src/soc/rockchip/rk3288/Kconfig@33 PS1, Line 33: select NO_BOOTBLOCK_CONSOLE Overriding the default for BOOTBLOCK_CONSOLE would still leave the option to enable it.
Or maybe a combination: `default n if CONFIG_CHROMEOS`?
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33068 )
Change subject: soc/rockchip/rk3288: Disable bootblock console ......................................................................
Patch Set 1:
I'm missing some context here, are you saying that the tree is already broken right now? Shouldn't Jenkins prevent that?
If adding post codes caused this, maybe we should see this as a canary that the added patches were badly optimized? Arm devices shouldn't use POST codes anyway. Does it help if you select CONFIG_NO_POST instead? How many bytes added / over the limit are we talking about?
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33068 )
Change subject: soc/rockchip/rk3288: Disable bootblock console ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/33068/1/src/soc/rockchip/rk3288/Kconfig File src/soc/rockchip/rk3288/Kconfig:
https://review.coreboot.org/#/c/33068/1/src/soc/rockchip/rk3288/Kconfig@33 PS1, Line 33: select NO_BOOTBLOCK_CONSOLE
Overriding the default for BOOTBLOCK_CONSOLE would still […]
This is what we do on some samsung SoC as well which is why I picked this variant.
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33068 )
Change subject: soc/rockchip/rk3288: Disable bootblock console ......................................................................
Patch Set 1:
Patch Set 1: I'm missing some context here, are you saying that the tree is already broken right now?
Depending on what you're looking for it's broken for about a week, or for about an hour: The jenkins build continued to work (until ~1h ago) because it does "timeless" builds which came with a shorter version string: just enough to make the build pass.
CrOS downstreaming or manual builds weren't as lucky and failed about a week ago.
Does it help if you select CONFIG_NO_POST instead? How many bytes added / over the limit are we talking about?
After taking out all the asserts, I get the following lines in bootblock.map: ff708e88 T _etext ff708ec0 D _data ff708ec0 d mdev ff708ee0 d rockchip_spi_slaves ff708eec D gpio_port ff708f10 B _bss ff708f10 B maskrom_param ff708f10 D _edata ff708f18 B exception_stack ff709000 A _ebootblock ff709000 B _preram_cbmem_console ff709018 b console_inited ff70901c b spi_flash_info ff709048 b spi_flash_init_done ff70904c b cbmem_console_p ff709050 B vboot_executed ff709058 B _ebss ff709058 B _eprogram
Of note (the ones that normally break the build) are _ebootblock (end of region) and _eprogram (end of code section that should be <= _ebootblock). NO_POST has no effect on these, interestingly enough.
If adding post codes caused this, maybe we should see this as a canary that the added patches were badly optimized? Arm devices shouldn't use POST codes anyway.
We don't use LTO in coreboot (yet) which would probably solve the issue by eliminating the code. The compiler lacks the visibility to know that it can optimize away the post code and call to post_code() and hence the extra function. #define die_with_post_code(val, msg) die((msg)) _nearly_ helps, but not quite: ff708e68 T _etext ff708e80 D _data ... ff709018 B _eprogram
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/33068 )
Change subject: soc/rockchip/rk3288: Disable bootblock console ......................................................................
soc/rockchip/rk3288: Disable bootblock console
Bootblock space is tight on this SoC and recent changes increased it ever so slightly to make this a problem.
Since the bootblock is well-tested, we can get by without console.
Change-Id: I7496a3e313b2c6ee6fb3c4671eac64376d84e0dc Signed-off-by: Patrick Georgi pgeorgi@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/33068 Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Nico Huber nico.h@gmx.de Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/rockchip/rk3288/Kconfig 1 file changed, 1 insertion(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Angel Pons: Looks good to me, approved
diff --git a/src/soc/rockchip/rk3288/Kconfig b/src/soc/rockchip/rk3288/Kconfig index 38d8752..f845f07 100644 --- a/src/soc/rockchip/rk3288/Kconfig +++ b/src/soc/rockchip/rk3288/Kconfig @@ -30,6 +30,7 @@ select MAINBOARD_HAS_NATIVE_VGA_INIT select MAINBOARD_FORCE_NATIVE_VGA_INIT select HAVE_LINEAR_FRAMEBUFFER + select NO_BOOTBLOCK_CONSOLE
if SOC_ROCKCHIP_RK3288
-
Angel Pons (Code Review) -
Julius Werner (Code Review)
-
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)