Hi Simon, On Wed, 22 May 2019 at 13:42, Simon Goldschmidt <simon.k.r.goldschm...@gmail.com> wrote: > > > > Simon Glass <s...@chromium.org> schrieb am Mi., 22. Mai 2019, 21:34: >> >> Hi Simon, >> >> On Wed, 22 May 2019 at 02:05, Simon Goldschmidt >> <simon.k.r.goldschm...@gmail.com> wrote: >> > >> > On Wed, May 22, 2019 at 2:53 AM Simon Glass <s...@chromium.org> wrote: >> > > >> > > Hi Andreas, >> > > >> > > On Tue, 21 May 2019 at 15:01, Andreas Dannenberg <dannenb...@ti.com> >> > > wrote: >> > > > >> > > > Hi Simon (Glass), >> > > > >> > > > On Sat, May 18, 2019 at 10:08:19AM -0600, Simon Glass wrote: >> > > > > Hi Andreas, >> > > > > >> > > > > On Mon, 6 May 2019 at 22:49, Andreas Dannenberg <dannenb...@ti.com> >> > > > > wrote: >> > > > > > >> > > > > > Hi Simon, >> > > > > > >> > > > > > On Mon, May 06, 2019 at 09:51:56PM -0600, Simon Glass wrote: >> > > > > > > Hi Andreas, >> > > > > > > >> > > > > > > On Fri, 3 May 2019 at 14:25, Andreas Dannenberg >> > > > > > > <dannenb...@ti.com> wrote: >> > > > > > > > >> > > > > > > > Simon, >> > > > > > > > >> > > > > > > > On Sat, Mar 30, 2019 at 09:18:08PM +0100, Simon Goldschmidt >> > > > > > > > wrote: >> > > > > > > > > Simon Glass <s...@chromium.org> schrieb am Sa., 30. März >> > > > > > > > > 2019, 21:06: >> > > > > > > > > >> > > > > > > > > > Hi Simon, >> > > > > > > > > > >> > > > > > > > > > On Wed, 27 Mar 2019 at 13:40, Simon Goldschmidt >> > > > > > > > > > <simon.k.r.goldschm...@gmail.com> wrote: >> > > > > > > > > > > >> > > > > > > > > > > This introduces a new Kconfig option SPL_CLEAR_BSS_F. If >> > > > > > > > > > > enabled, it >> > > > > > > > > > clears >> > > > > > > > > > > the bss before calling board_init_f() instead of >> > > > > > > > > > > clearing it before >> > > > > > > > > > calling >> > > > > > > > > > > board_init_r(). >> > > > > > > > > > > >> > > > > > > > > > > This also ensures that variables placed in BSS can be >> > > > > > > > > > > shared between >> > > > > > > > > > > board_init_f() and board_init_r() in SPL. >> > > > > > > > > > > >> > > > > > > > > > > Such global variables are used, for example, when >> > > > > > > > > > > loading things from FAT >> > > > > > > > > > > before SDRAM is available: the full heap required for >> > > > > > > > > > > FAT uses global >> > > > > > > > > > > variables and clearing BSS after board_init_f() would >> > > > > > > > > > > reset the heap >> > > > > > > > > > state. >> > > > > > > > > > > An example for such a usage is socfpa_arria10 where an >> > > > > > > > > > > FPGA configuration >> > > > > > > > > > > is required before SDRAM can be used. >> > > > > > > > > > > >> > > > > > > > > > > Make the new option depend on ARM for now until more >> > > > > > > > > > > implementations >> > > > > > > > > > follow. >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > I still have objections to this series and I think we >> > > > > > > > > > should discuss >> > > > > > > > > > other ways of solving this problem. >> > > > > > > > > > >> > > > > > > > > > Does socfgpa have SRAM that could be used before SDRAM is >> > > > > > > > > > available? >> > > > > > > > > > If so, can we not use that for the configuration? What >> > > > > > > > > > various are >> > > > > > > > > > actually in BSS that are needed before board_init_r() is >> > > > > > > > > > called? Can >> > > > > > > > > > they not be in a struct created from malloc()? >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > > The problem is the board needs to load an FPGA configuration >> > > > > > > > > from FAT >> > > > > > > > > before SDRAM is available. Yes, this is loaded into SRAM of >> > > > > > > > > course, but the >> > > > > > > > > whole code until that is done uses so many malloc/free >> > > > > > > > > iterations that The >> > > > > > > > > simple mall of implementation would require too much memory. >> > > > > > > > > >> > > > > > > > > And it's the full malloc state variables only that use BSS, >> > > > > > > > > not the FAT >> > > > > > > > > code. >> > > > > > > > >> > > > > > > > I've actually faced very similar issues working on our TI >> > > > > > > > AM654x "System >> > > > > > > > Firmware Loader" implementation (will post upstream soon), >> > > > > > > > where I need >> > > > > > > > to load this firmware and other files from media such as >> > > > > > > > MMC/FAT in a very >> > > > > > > > memory-constrained SPL pre-relocation environment *before* I >> > > > > > > > can bring up >> > > > > > > > DDR. >> > > > > > > > >> > > > > > > > Initially, I modified the fat.c driver to re-use memory so it >> > > > > > > > is not as >> > > > > > > > wasteful during SYS_MALLOC_SIMPLE. While I'm not proud of this >> > > > > > > > solution [1] >> > > > > > > > this allowed us to get going, allowing to load multiple files >> > > > > > > > without >> > > > > > > > issues in pre-relocation SPL. >> > > > > > > >> > > > > > > That seems to point the way to a useful solution I think. We >> > > > > > > could >> > > > > > > have a struct containing allocated pointers which is private to >> > > > > > > FAT, >> > > > > > > and just allocate them the first time. >> > > > > > >> > > > > > The board_init_f()-based loader solution we use extends beyond >> > > > > > MMC/FAT, >> > > > > > but also for OSPI, X/Y-Modem, and (later) USB, network, etc. >> > > > > > >> > > > > > Background: >> > > > > > On our "TI K3" devices we need to do a whole bunch of stuff before >> > > > > > DDR is up with limited memory, namely loading and installing a >> > > > > > firmware >> > > > > > that controls the entire SoC called "System Firmware". It is only >> > > > > > after >> > > > > > this FW is loaded from boot media and successfully started that I >> > > > > > can >> > > > > > bring up DDR. So all this is done in SPL board_init_f(), which as >> > > > > > the >> > > > > > last step brings up DDR. >> > > > > > >> > > > > > Not having BSS available to carry over certain state to the >> > > > > > board_init_r() world would lead to a bunch of hacky changes across >> > > > > > the board I'm afraid, more below. >> > > > > >> > > > > This is really unfortunate. >> > > > > >> > > > > It seems to me that we have two choises: >> > > > > >> > > > > 1. Hack around with board_init_f() such as to remove the distinction >> > > > > between this and board_init_r(). >> > > > > >> > > > > 2. Enter board_init_r() without DRAM ready, and deal with setting it >> > > > > up there. >> > > > > >> > > > > I feel that the second solution is worth exploring. We could have >> > > > > some >> > > > > board-specific init in board_init_r(). We already have >> > > > > spl_board_init() so perhaps we could have spl_early_board_init() >> > > > > called right near the top? >> > > > > >> > > > > We can refactor a few of the functions in spl/spl.c so they can be >> > > > > called from board-specific code if necessary. We could also add new >> > > > > flags to global_data to control the behaviour of the SPL code, and >> > > > > the >> > > > > board code could set these. >> > > > >> > > > Let me explore this option. I can probably make something work but I >> > > > don't yet see how to do it cleanly, maybe it becomes clearer once I put >> > > > some code down. Currently by definition board_init_r() has DDR >> > > > available, and much of the code is geared towards it (for example the >> > > > calling of spl_relocate_stack_gd() before entering board_init_r() which >> > > > will already switch over to DDR). >> > > > >> > > > Also, and not to discourage that we can't improve upon the status quo, >> > > > but there is already a ton of boards using such an "early BSS" >> > > > scheme... >> > > > >> > > > $ git grep --show-function 'memset.*bss' | grep board_init_f | wc -l >> > > > 35 >> > > >> > > Yes I know :-( >> > > >> > > We should migrate these boards to use the generic SPL framework. >> > >> > socfpga_gen5 is one of the architectures listed here. I'm not even sure >> > whether that's actually needed. However, it's hard to test, isn't it? How >> > do you actually tell BSS isn't used before entering board_init_r? >> >> One way might be to link the SPL code without the call to >> board_init_r() and then check the map to make sure BSS is empty. > > > That would be worth a try.
Yes, I did something like this a few years back and it worked OK. The way I did it was (from my fading memory) was using an if() around the call, or perhaps by removing symbols using a linker option...I can't remember. But in any case garbage collection removed dependent code. > >> >> > >> > To be sure, we'd need to initialize unused memory to some magic >> > constant and check that it has been left untouched later (on boards >> > where BSS is available in board_init_r and remains in place when >> > moving on). >> >> I prefer a build-time check. We might even be able to automate it. > > > Of course a build-time check is better here than a runtime check. I just > couldn't come up with one. > > I think it would be really beneficial to add such a check to all boards so we > know what we're discussing: I'll bet there are many of them just using bss > early because No one noticed... > > And in board-local code, that could even be ok, it's just not ok for code > shared with boards not having access to bss early. Yes this would be a really great thing to have. [..] Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
