Attention is currently required from: Rex-BC Chen, Yu-Ping Wu. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63924 )
Change subject: soc/mediatek/mt8186: Enlarge CBFS_MCACHE to 16K ......................................................................
Patch Set 4: Code-Review+1
(2 comments)
Patchset:
PS1:
(oh, was that because romstages should never return - only jump directly to ramstage?)
Yes, basically you can only overlap regions that are never needed at the same time (with the consideration that a region is also "needed" by the stage that loads code into it). The bootblock does load the romstage in a !VBOOT scenario, but the romstage doesn't return to the bootblock, so once you're in romstage you can overwrite the bootblock with DRAM init code.
- Use `OVERLAP_VERSTAGE_ROMSTAGE` instead of `OVERLAP_DECOMPRESSOR_*`
Well, there's no difference between those two macros when COMPRESS_BOOTBLOCK is not enabled. On the other hand you should always try enabling COMPRESS_BOOTBLOCK unless you've already confirmed that it doesn't provide a benefit for your platform (when measuring that, keep in mind that boot ROM loading usually do not show up in timestamps... you'll need to check with a SPI analyzer). But that's not related to this patch.
File src/soc/mediatek/mt8186/include/soc/memlayout.ld:
https://review.coreboot.org/c/coreboot/+/63924/comment/e174957f_8b2152bb PS2, Line 25: TTB(0x00100000, 28K) : DMA_COHERENT(0x00107000, 4K) : TPM_TCPA_LOG(0x00108000, 2K) : FMAP_CACHE(0x00108800, 2K) : WATCHDOG_TOMBSTONE(0x00109000, 4) : CBFS_MCACHE(0x00109004, 16K - 4) : /* EMPTY(0x0010d000, 4K) */ : STACK(0x0010E000, 7K) : TIMESTAMP(0x0010FC00, 1K) : /* MT8186 has 64KB SRAM. */
I think you are right. […]
Yes, that's a good point, it would be good if you could add an extra assertion to the STACK() macro. Both base and size should be divisible by 16 (AArch64 ABI requires SP to be 16-byte aligned between function calls).