On 01/07/2018 08:40 PM, Simon Glass wrote:
Hi Stephen,
On 19 December 2017 at 18:30, Stephen Warren <swar...@wwwdotorg.org> wrote:
From: Stephen Warren <swar...@nvidia.com>
U-Boot typically uses a hard-coded value for the stack pointer before
relocation. Implement option SYS_INIT_SP_BSS_OFFSET to instead calculate
the initial SP at run-time. This is useful to avoid hard-coding addresses
into U-Boot, so that can be loaded and executed at arbitrary addresses and
thus avoid using arbitrary addresses at runtime. This option's value is
the offset added to &_bss_start in order to calculate the stack pointer.
This offset should be large enough so that the early malloc region, global
data (gd), and early stack usage do not overlap any appended DTB.
I don't see why this is an offset from bss_start - shouldn't it be bss_end?
BSS can vary in size based on the set of config options enabled, code
growth/shrinkage, etc. Thus, basing the initial SP address on bss_end
would provide too much variability, and hence would be unsafe, whereas
basing the initial SP address on bss_start always provides the exact
same amount of available stack space (assuming an identical DTB for the
comparison).
Also this seems error-prone since we don't know how large the DTB is.
Can we improve this, e.g. by:
- using binman to provide the stack start value or offset
I'd rather not involve binman in the code generation process. Packaging
is fine, but I think the source code and makefiles should dictate
everything that goes into the actual binary to keep things simple.
I did originally think about having binman patch up the init SP address,
but rejected it due to the complexity added by the extra step, and the
fact that we'd be inventing a new tool (the new part of binman
implemented for this feature) to do what we already have a perfectly
good tool for already; the linker. I'm also looking to backport this
change to an older branch of U-Boot that doesn't have binman, although I
believe I'd have the same thought process either way.
- checking the DTB size and automatically using the address
immediately after it finishes (again I suppose binman could provide
that)
Perhaps we can add an extra step in binman that validates the build;
i.e. that (SYS_INIT_SP_BSS_OFFSET - size_of_dtb) >
some_minimum_stack_size. That would be a useful error check, but prevent
binman having to edit parts of the binary that were already created by
the main build process. Note that for a given build, it should be
completely deterministic whether DTB corruption occurs, and whether that
corruption actually impacts U-Boot's operation, so any such check would
be pretty much equivalent to just running U-Boot and seeing if it works.
Admittedly the check might save some annoying debugging though, so would
be a good idea.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
