[U-Boot] [linux-sunxi] Re: [PATCH v2 5/6] sunxi: add code for recalculating the DRAM size in U-Boot

[Siarhei Siamashka <siarhei.siamashka@gmail.com>]

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.

-- 
Best regards,
Siarhei Siamashka