On 02/07/16 12:49, Hans de Goede wrote:
Hi Hans,
On 30-06-16 17:24, Simon Glass wrote:
Hi,
On 30 June 2016 at 03:00, Hans de Goede hdegoede@redhat.com wrote:
Hi Andre,
On 30-06-16 02:25, Andre Przywara wrote:
Probably due to some (ill-founded) fear of a large BSS all sunxi boards forced their SPL BSS section into DRAM. This only works if there is no usage of a .BSS variable before the DRAM is initialised. The recent inclusion of tiny-printf breaks this assumption (it has two variables in .BSS), so any early printf (printing a number) hangs a board. This in particular breaks the (WIP) Pine64 SPL, which at the moment links Allwinner's libdram library, trying to print debug information: DRAM:DRAM driver version: V1.0 DRAM Type = <hangs>
Hmm, although 256 bytes is not a lot I would prefer for BSS to stay in DRAM, esp. since the bss use may grow over time, and the SPL space is quite small.
Moreover, given that tiny-printf is specifically meant for use in SPL / restricted environments and having BSS in DRAM is not unheard of for other boards, it seems to me like this is something which should really be fixed in tinyprintf instead.
Have you tried looking into fixing this at the tinyprintf level ?
Marek's fix is one option. Another is to make use of global_data, which will be available from very early and it set to zero.
I think Marek's fix is fine, we should go and merge that.
Only that it's not sufficient as it (all three variables should be covered). I have a better version which I will send. I found the promising -fno-zero-initialized-in-bss gcc switch, but this apparently only works for *explicitly* initialized variables. So: static char zs = 0; is fine (moved to .data), but without the "= 0" it's still in BSS. Initialising those variables is pointless and confusing, so at least requires a comment. At this point I think we can just use the explicit section attribute. A more involved way would be to avoid those global variables in the first place, but I don't think it's worth the effort.
As for the worries about not using BSS before DRAM is initialized coming back to bite us. Yes that may happen, but we are not the only board / mach / soc code to do this.
I don't think that's a good excuse ;-)
I agree that documenting this somewhere would be good (patches welcome).
I wonder if there is a way to _enforce_ this. Can't we somehow mark code that runs before DRAM init and check that this marked code doesn't have anything in BSS? Or maybe tagging this code (or the files) and putting their BSS variables in SRAM, while letting the rest of the SPL use a DRAM based BSS? Having compile warnings or errors is much better than hard to debug breaks.
As for just putting BSS in the sram, nack. Thanks to various efforts we currently have some free space for the SPL, but in the future we will likely add NAND support, and try to move the SPL to dm (with static device instantiation, no space for dt) so that we can get rid of the duplicate non-dm + dm gpio and uart code we currently have. These things combined will push things to their limit and may very well grow the BSS section too.
I am bit worried that U-Boot is accepting hacks that paper over other hacks. Frankly: we shouldn't save memory beyond the point where it breaks things and makes them close to unmaintainable. Relying on variables to be in certain sections or not being accessed before a certain point in time is fragile (in fact it just broke!), so please note my protest against this approach.
We have more SRAM space available in many SoCs, so I will send another patch that makes use of them if possible (starting with the A64). That should be the route that we take for the future, limiting this "BSS in DRAM" hack to just the SoCs that are in desperate need for it.
Cheers, Andre