Hi Stephen, On 8 January 2018 at 11:34, Stephen Warren <swar...@wwwdotorg.org> wrote: > > 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).
OK. I suppose we are no worse off anyway, since the DTB can vary in size. > >> 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. > There is actually a feature for this, or something similar: http://git.denx.de/?p=u-boot.git;a=blob;f=tools/binman/README;h=08c3e56bdef8d3ee47ac2ec86411465a78dbf567;hb=HEAD#l476 >> - 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. Sounds good. It could even be done in Makefile - we have a few checks that that at present. But if binman is a better place, that is fine. Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
