Attention is currently required from: Julius Werner, Patrick Rudolph, Philipp Hug, ron minnich.
Maximilian Brune has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77974?usp=email )
Change subject: include/memlayout.h: Add OPENSBI linker macro ......................................................................
Patch Set 5:
(1 comment)
File src/arch/riscv/include/arch/memlayout.h:
https://review.coreboot.org/c/coreboot/+/77974/comment/d682dce2_ad328cef : PS5, Line 27: opensbi
why not `REGION(opensbi, CONFIG_OPENSBI_TEXT_START, size, 4K)` ?
I am actually a little uncertain about that. If I choose to use: `REGION(opensbi, CONFIG_OPENSBI_TEXT_START, size, 4K)` then it would look like this in a memlayout file: ``` SECTIONS { DRAM_START(FU540_DRAM) OPENSBI(128K) RAMSTAGE(FU540_DRAM + 128K, 2M) MEM_STACK(FU540_DRAM + 128K + 2M, 20K) POSTRAM_CBFS_CACHE(FU540_DRAM + 3M, 29M) } ``` And that is very inconsistent, because all macros usually get an address and a size. As a reader I would look at this file and think that the OPENSBI macro is placed exactly at the location `FU540_DRAM`. But this may not be the case since I define it to be always at `CONFIG_OPENSBI_TEXT_START`. Without looking inside the macro itself it is very confusing on where it is placed. Bottom line: I chose not to use `REGION(opensbi, CONFIG_OPENSBI_TEXT_START, size, 4K)` for readability reasons.