On Tue, 3 Apr 2018 13:43:43 +0100 Andre Przywara andre.przywara@arm.com wrote:
Hi Icenowy,
On 03/04/18 12:51, Icenowy Zheng wrote:
于 2018年4月3日 GMT+08:00 下午7:34:55, Maxime Ripard maxime.ripard@bootlin.com 写到:
On Tue, Apr 03, 2018 at 11:13:17AM +0100, Andre Przywara wrote:
>> For hacking it, see my implementation in v1, which assumes the >> only size supported bigger than 2GiB is 3GiB (which is >> acceptable on sunxi, but might not work on other platforms). >> >> As Andre said, that function has another big problem -- it
detects
>> memory with writing to it. This is risky. > > How is it risky when it's done by the SPL?
Originally that was my confusion as well: It's not the SPL calling
that
function. The DRAM controller init function in there knows very precisely how much DRAM we have, but we don't communicate this to
U-Boot
proper. So U-Boot *proper* goes ahead and probes the DRAM. This
means it
could step into secure memory, for instance. On sunxi64 we have
the ATF
running between SPL and U-Boot, also all kind of secure payloads
could
already have been registered. So I wonder if it would be easier to somehow pass on this *one*
word of
information between SPL and U-Boot proper to avoid calling this
function
altogether?
That would definitely make sense yes.
So since the SPL loads the DT anyway (from the FIT image) and puts it
at
the end of the U-Boot (proper) binary, wouldn't it be the easiest to just patch the actual DRAM size in there? IIRC we don't have any FDT write code in the SPL at the moment, and pulling it in would probably push it over the edge again, but:
That assumes that you are loading a FIT image, which might or might not be the case, and on most arm32 chips, most likely won't.
I guess we'd need to find another way (put some information in an SRAM somewhere?) to try to do that for all variants.
Extend the SPL header again? If we found SPL v3+, use the DRAM size encoded and bypass ram_get_size, otherwise fallback to ram_get_size?
Yes, that would be a possibility as well. Though I believe at the moment we don't access the SPL header from U-Boot proper, do we?
We do. The boot device and also the boot.scr location (in the case of FEL boot) is read from the SPL header by the main U-Boot part.
Not a real show-stopper, but we probably need to document that the SPL header would need to stay around.
Maybe.
But if we have a fallback anyway ...
Which fallback? Do you mean calling things like ram_get_size() from U-Boot? We should never do this because this is very wrong.
The D-Cache may be already enabled, causing all kinds of weird effects. Also modifying data in DRAM (even temporarily) while our code is already running from DRAM is dangerous.
(Although it will lead to some days of mess on sunxi-tools, this is a reasonable choice.)
True, but actually I wonder why we have SPL_MAX_VERSION in there in the first place. Can't we just postulate that every new SPL version stays backwards compatible? So if sunxi-tools can deal with v2, a v3 SPL would still be fine, you would just loose the v3 features (if at all)? We can just put a warning in there, to ask users to upgrade. That would have worked already with the v1/v2 transition, I believe.
Yes, that's more or less how this was supposed to work in sunxi-tools from the very beginning. Except that we unfortunately got a bug in this code, which has been reported back in July 2017 and is still not resolved due to various reasons:
https://github.com/linux-sunxi/sunxi-tools/issues/104
But hopefully it can be fixed sooner or later.
Probably worth a separate discussion with some sunxi-tools stakeholders.
On the U-Boot side we can just increase the version number as long as the new header is a backwards compatible superset of the old one.
In the unlikely case if we suddenly have to introduce a compatibility breaking change to the SPL header format, we can always change the SPL header signature from 'SPL' to something else.