Re: [PATCH] Revert "arm: bootm: Disable LMB reservation for command line and board info on arm64"

[Jan Kiszka <jan.kiszka@siemens.com>]

On 02.08.21 02:54, Marek Vasut wrote:
> On 7/29/21 6:58 PM, Tom Rini wrote:
> 
> [...]
> 
>>>> so when did rcar3 introduce something there that shouldn't be
>>>> reserved?  And you had phrased this to me on IRC as about reserving
>>>> spot
>>>> for ATAGS, and that not being needed of course on arm64.  But that's
>>>> not
>>>> what's going on.  Perhaps the answer is that rcar3 needs to introduce a
>>>> board_lmb_reserve to free the normal arch one and provide whatever more
>>>> narrow scope it needs.
>>>
>>> Based on the commit message 2359fa7a878 ("arm: bootm: Disable LMB
>>> reservation for command line and board info on arm64") , this is
>>> about ATAGS
>>> and we really don't need to reserve those on arm64.
>>
>> Commit 2359fa7a878 disables the entire arch_lmb_reserve function on
>> aarch64, yes.  I assumed when we had talked that it was a small area
>> being set aside and perhaps mis-recalled that ATAGS tended to live at
>> DDR_BASE + 0x800 or so.
> 
> That arch_lmb_reserve() is responsible for reserving architecture
> specific memory. On arm32 it is ATAGS, on arm64 it is nothing as far as
> I can tell (and see below regarding the TLB).
> 
>> This reservation is not at that spot, and a lot
>> more than that.
> 
> Can you please elaborate on this "lot more" part ? Because as much as I
> studied the reservation code, the "lot more" was ATAGS on arm32 and
> nothing on arm64.

See my commit log.

> 
>> That commit is also only present in v2021.10-rc1 so
>> it's not a great idea for another project to depend on it and can also
>> revert their changes until someone is able to analyze the entire
>> situation again.
> 
> We are in rc1 so we still have enough time to analyze this problem, yes.
> 
>>> I didn't hit the tlb reservation issue Jan has in my tests on rcar3,
>>> so I
>>> suspect that is the real problem here, i.e. we need some more fine
>>> grained
>>> lmb reservation on arm64 ?
>>
>> Yes, some further analysis is needed here.  But due to a lack of time I
>> suspect the best answer for now is for rcar3 to add board_lmb_reserve()
>> and lmb_free what's not needed there.
> 
> So, no.
> 
> The question I would like answered is whether we even really have to
> reserve the TLB area with LMB at all. Linux will set up the TLB again,
> so I think we do not have to.

See my commit log.

> 
> And I _think_ the problem is isolated to K3 due to the non-standard TLB
> and cache init it does in arch/arm/mach-k3/common.c spl_enable_dcache()
> in SPL. The code sets up TLB close to the end of DRAM and enables dcache
> early in SPL, and the end of DRAM is incidentally also where U-Boot is
> relocated shortly after and at that point U-Boot possibly overwrites or
> corrupts that TLB content. This hypothesis can be easily tested on K3,
> just comment out the content of spl_enable_dcache() and see if that
> fixes things.

This is not only TLB, see my commit log. Specifically the clash with the
relocated U-Boot code causes the crash on the k3. But that location does
not look like it is k3-specfic. Other boards are just lucky if the work.

> 
> It would also be interesting to know what the debug() print in
> spl_enable_dcache() prints on the K3 from Jan.

The information is already available, I shared bdinfo.

Jan

-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux