On Sun, Feb 10, 2019 at 02:07:28PM +0100, Simon Goldschmidt wrote: > On Sun, Feb 10, 2019 at 1:43 AM Tom Rini <tr...@konsulko.com> wrote: > > > > On Sat, Feb 09, 2019 at 10:56:40PM +0100, Simon Goldschmidt wrote: > > > On Fri, Feb 8, 2019 at 10:20 PM Tom Rini <tr...@konsulko.com> wrote: > > > > > > > > On Fri, Feb 08, 2019 at 10:05:41PM +0100, Simon Goldschmidt wrote: > > > > > On Fri, Feb 8, 2019 at 8:46 PM Tom Rini <tr...@konsulko.com> wrote: > > > > > > > > > > > > On Wed, Nov 28, 2018 at 09:52:45PM +0100, Simon Goldschmidt wrote: > > > > > > > > > > > > > SPL currently does not check uImage CRCs when loading U-Boot. > > > > > > > > > > > > > > This patch adds checking the uImage CRC when SPL loads U-Boot. It > > > > > > > does > > > > > > > this by reusing the existing config option SPL_CRC32_SUPPORT to > > > > > > > allow > > > > > > > leaving out the CRC check on boards where the additional code > > > > > > > size or > > > > > > > boot time is a problem (adding the CRC check currently adds ~1.4 > > > > > > > kByte > > > > > > > to flash). > > > > > > > > > > > > > > The SPL_CRC32_SUPPORT config option now gets enabled by default > > > > > > > if SPL > > > > > > > support for legacy images is enabled to check the CRC on all > > > > > > > boards > > > > > > > that don't actively take countermeasures. > > > > > > > > > > > > > > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschm...@gmail.com> > > > > > > > Reviewed-by: Simon Glass <s...@chromium.org> > > > > > > > > > > > > Really sorry for the delay on this, especially as I've found one or > > > > > > two > > > > > > problems. The first problem is that with this vyasa-rk3288 and a > > > > > > few > > > > > > others fail to build due to a number of errors such as: > > > > > > ../common/spl/spl.c:269:12: error: 'struct spl_image_info' has no > > > > > > member > > > > > > named 'dcrc_data' > > > > > > > > > > Hmm, let me check what's wrong there. > > > > > > OK, so the vyasa-rk3288 uses TPL, that's what I got wrong. > > > > > > > > > > > > > > > Second, I believe this is causing a number of platforms with very > > > > > > tight > > > > > > SPL constraints, namely all of 64bit sunxi, to fail to link as they > > > > > > overflow SRAM now. This can be fixed at least by making the new > > > > > > behavior opt-in, but I would fix the vyasa-rk3288 problem first. > > > > > > Thanks > > > > > > in advance! > > > > > > > > > > Well, I thought it would be better to have it default to 'y' since I > > > > > think it > > > > > is what is expected. Shouldn't we explicitly work on those platforms? > > > > > I must say I was pretty shocked to see that SPL did not detect an > > > > > invalid > > > > > CRC... > > > > > > > > > > Can you point me to a config that fails? > > > > > > > > In general, pine64_plus_defconfig is one that might (as I re-run my > > > > build without this patch, I still see some failures but they've scrolled > > > > off screen, but 64bit sunxi _is_ very sensitive to SPL growth). > > > > > > And I also did compile that one but I don't see any error message. How > > > am I supposed to detect the SRAM overflowing? > > > > > > I assume it's .text or .rodata which makes it grow too big? If so, we > > > could > > > still check the CRC by using a smaller algorithm that does not rely on a > > > precalculated table... > > > > That could have been the other patch then. Does all of sunxi build for > > you? If so, we're good then. > > Well, I really can't tell: especially with other patches in the queue that > might > get puhed before mine. text and rodata do grow by ~1.6 KiB summed up, so > that might of course trigger some platforms. > > On the other hand, I think this is a critical feature for reliable boot unless > using FIT. If you don't think so (and in fact, noone seems to have noticed > this > for some years), then I'm happy to re-post this without default 'y'.
Yeah, I think we should leave default y off. Thanks! -- Tom
signature.asc
Description: PGP signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
