Christian Walter has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34640 )
Change subject: tegra210: Increased size of verstage due to overflow ......................................................................
tegra210: Increased size of verstage due to overflow
When imlpementing chagnes in VBOOT, within the build process, tegra210 overflows into the romstage. Reduced the size of Romstage from 104 to 100 and increase the size from verstage from 66 to 70.
Change-Id: Ie00498838a644a6f92881db85833dd0a94b87f53 Signed-off-by: Christian Walter christian.walter@9elements.com --- M src/soc/nvidia/tegra210/include/soc/memlayout.ld 1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/34640/1
diff --git a/src/soc/nvidia/tegra210/include/soc/memlayout.ld b/src/soc/nvidia/tegra210/include/soc/memlayout.ld index 5d7481b..6091dd8 100644 --- a/src/soc/nvidia/tegra210/include/soc/memlayout.ld +++ b/src/soc/nvidia/tegra210/include/soc/memlayout.ld @@ -39,8 +39,8 @@ #endif TIMESTAMP(0x4000D800, 2K) BOOTBLOCK(0x4000E000, 30K) - VERSTAGE(0x40015800, 66K) - ROMSTAGE(0x40026000, 104K) + VERSTAGE(0x40015800, 70K) + ROMSTAGE(0x40026000, 100K) SRAM_END(0x40040000)
DRAM_START(0x80000000)
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34640 )
Change subject: tegra210: Increased size of verstage due to overflow ......................................................................
Patch Set 2:
(4 comments)
https://review.coreboot.org/c/coreboot/+/34640/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34640/2//COMMIT_MSG@7 PS2, Line 7: Increased Imperative mood: Increase
https://review.coreboot.org/c/coreboot/+/34640/2//COMMIT_MSG@9 PS2, Line 9: chagnes changes
https://review.coreboot.org/c/coreboot/+/34640/2//COMMIT_MSG@10 PS2, Line 10: Reduced Reduce
https://review.coreboot.org/c/coreboot/+/34640/2//COMMIT_MSG@10 PS2, Line 10: Romstage romstage
Christian Walter has removed Paul Menzel from this change. ( https://review.coreboot.org/c/coreboot/+/34640 )
Change subject: tegra210: Increased size of verstage due to overflow ......................................................................
Removed reviewer Paul Menzel.
Christian Walter has removed Julius Werner from this change. ( https://review.coreboot.org/c/coreboot/+/34640 )
Change subject: tegra210: Increased size of verstage due to overflow ......................................................................
Removed reviewer Julius Werner.
Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34640 )
Change subject: tegra210: Increase size of verstage due to overflow ......................................................................
Patch Set 6:
(4 comments)
https://review.coreboot.org/c/coreboot/+/34640/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34640/2//COMMIT_MSG@7 PS2, Line 7: Increased
Imperative mood: Increase
Ack
https://review.coreboot.org/c/coreboot/+/34640/2//COMMIT_MSG@9 PS2, Line 9: chagnes
changes
Ack
https://review.coreboot.org/c/coreboot/+/34640/2//COMMIT_MSG@10 PS2, Line 10: Romstage
romstage
Ack
https://review.coreboot.org/c/coreboot/+/34640/2//COMMIT_MSG@10 PS2, Line 10: Reduced
Reduce
Ack
Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34640 )
Change subject: tegra210: Increase size of verstage due to overflow ......................................................................
Patch Set 6:
Patch Set 4:
Are the PTT changes causing the overflow? They really shouldn't, since tegra210 shouldn't have it enabled. How big exactly is the verstage before and after the PTT change?
To be honest - I got no clue. The size of the .elf file is 105300 in both cases. The .debug file is bigger with the VBOOT Changes from the other patch, but still it should have no effect here. Can I safely decrease the size of the CBFS_CACHE from 32 to 30? That would at least solve the problem. Or how can I check the compressed size of the verstage?
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34640 )
Change subject: tegra210: Increase size of verstage due to overflow ......................................................................
Patch Set 6:
To be honest - I got no clue. The size of the .elf file is 105300 in both cases.
The full size of an ELF doesn't tell you much because there are tons of debugging symbols and the like bloating it up. Please run util/crossgcc/xgcc/bin/aarch64-elf-objdump -p on it. 'memsz' is the size that needs to fit into SRAM. That must somehow be going up with your patch, I just want to know how much (it should really not be more than a couple of bytes because your changes shouldn't really affect this platform.
The .debug file is bigger with the VBOOT Changes from the other patch, but still it should have no effect here. Can I safely decrease the size of the CBFS_CACHE from 32 to 30? That would at least solve the problem. Or how can I check the compressed size of the verstage?
Yes, but this patch (moving romstage space to verstage) should also be fine (assuming romstage also still fits). I don't have anything against the patch directly but I want to confirm that your other patches are not causing a big binary size regression first. (Compressed size is irrelevant for this stuff.)
Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34640 )
Change subject: tegra210: Increase size of verstage due to overflow ......................................................................
Patch Set 6:
Patch Set 6:
To be honest - I got no clue. The size of the .elf file is 105300 in both cases.
The full size of an ELF doesn't tell you much because there are tons of debugging symbols and the like bloating it up. Please run util/crossgcc/xgcc/bin/aarch64-elf-objdump -p on it. 'memsz' is the size that needs to fit into SRAM. That must somehow be going up with your patch, I just want to know how much (it should really not be more than a couple of bytes because your changes shouldn't really affect this platform.
The .debug file is bigger with the VBOOT Changes from the other patch, but still it should have no effect here. Can I safely decrease the size of the CBFS_CACHE from 32 to 30? That would at least solve the problem. Or how can I check the compressed size of the verstage?
Yes, but this patch (moving romstage space to verstage) should also be fine (assuming romstage also still fits). I don't have anything against the patch directly but I want to confirm that your other patches are not causing a big binary size regression first. (Compressed size is irrelevant for this stuff.)
So if I build it without the VBOOT it says:
Program Header: LOAD off 0x00000058 vaddr 0x40015800 paddr 0x40015800 align 2**3 filesz 0x0000f680 memsz 0x0000fe38 flags rwx
and with VBOOT
Program Header: LOAD off 0x00000058 vaddr 0x40015800 paddr 0x40015800 align 2**3 filesz 0x0000f680 memsz 0x0000fe38 flags r
To me it looks identical. Could it have any other reason? Actually I can not strip down the Romstage anymore - so I would go for decreasing the CBFS_Cache and increasing the verstage just so the tests pass again. IF I check the .map files - I see changes, I guess because I moved the vboot_setup into the verstage_main - but still this should not cause any increase in terms of space.
Hello Patrick Rudolph, Julius Werner, Paul Menzel, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34640
to look at the new patch set (#7).
Change subject: tegra210: Increase size of verstage due to overflow ......................................................................
tegra210: Increase size of verstage due to overflow
When imlpementing changes in VBOOT, within the build process, tegra210 overflows into the romstage. Reduce the size of romstage from 104 to 100 and increase the size from verstage from 66 to 70.
Change-Id: Ie00498838a644a6f92881db85833dd0a94b87f53 Signed-off-by: Christian Walter christian.walter@9elements.com --- M src/soc/nvidia/tegra210/include/soc/memlayout.ld 1 file changed, 8 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/34640/7
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34640 )
Change subject: tegra210: Increase size of verstage due to overflow ......................................................................
Patch Set 7:
Well, now you reordered the patches and the vboot one still builds, even though it comes first? So this shouldn't be necessary anymore, right?
Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34640 )
Change subject: tegra210: Increase size of verstage due to overflow ......................................................................
Patch Set 8:
Patch Set 7:
Well, now you reordered the patches and the vboot one still builds, even though it comes first? So this shouldn't be necessary anymore, right?
Looks like we still need this.
Philipp Deppenwiese has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34640 )
Change subject: tegra210: Increase size of verstage due to overflow ......................................................................
Patch Set 8: Code-Review+2
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34640 )
Change subject: tegra210: Increase size of verstage due to overflow ......................................................................
Patch Set 8: Code-Review+2
Philipp Deppenwiese has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34640 )
Change subject: tegra210: Increase size of verstage due to overflow ......................................................................
tegra210: Increase size of verstage due to overflow
When imlpementing changes in VBOOT, within the build process, tegra210 overflows into the romstage. Reduce the size of romstage from 104 to 100 and increase the size from verstage from 66 to 70.
Change-Id: Ie00498838a644a6f92881db85833dd0a94b87f53 Signed-off-by: Christian Walter christian.walter@9elements.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/34640 Reviewed-by: Philipp Deppenwiese zaolin.daisuki@gmail.com Reviewed-by: Julius Werner jwerner@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/nvidia/tegra210/include/soc/memlayout.ld 1 file changed, 8 insertions(+), 8 deletions(-)
Approvals: build bot (Jenkins): Verified Philipp Deppenwiese: Looks good to me, approved Julius Werner: Looks good to me, approved
diff --git a/src/soc/nvidia/tegra210/include/soc/memlayout.ld b/src/soc/nvidia/tegra210/include/soc/memlayout.ld index 5d7481b..6d74ab9 100644 --- a/src/soc/nvidia/tegra210/include/soc/memlayout.ld +++ b/src/soc/nvidia/tegra210/include/soc/memlayout.ld @@ -29,17 +29,17 @@ { SRAM_START(0x40000000) PRERAM_CBMEM_CONSOLE(0x40000000, 2K) - PRERAM_CBFS_CACHE(0x40000800, 32K) - VBOOT2_WORK(0x40008800, 12K) - VBOOT2_TPM_LOG(0x4000B800, 2K) + PRERAM_CBFS_CACHE(0x40000800, 30K) + VBOOT2_WORK(0x40008000, 12K) + VBOOT2_TPM_LOG(0x4000B000, 2K) #if ENV_ARM64 - STACK(0x4000C000, 3K) + STACK(0x4000B800, 3K) #else /* AVP gets a separate stack to avoid any chance of handoff races. */ - STACK(0x4000CC00, 3K) + STACK(0x4000C400, 3K) #endif - TIMESTAMP(0x4000D800, 2K) - BOOTBLOCK(0x4000E000, 30K) - VERSTAGE(0x40015800, 66K) + TIMESTAMP(0x4000D000, 2K) + BOOTBLOCK(0x4000D800, 30K) + VERSTAGE(0x40015000, 68k) ROMSTAGE(0x40026000, 104K) SRAM_END(0x40040000)
-
Christian Walter (Code Review) -
Julius Werner (Code Review)
-
Paul Menzel (Code Review)
-
Philipp Deppenwiese (Code Review)