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

[Jan Kiszka]

On 02.08.21 15:04, Tom Rini wrote:
> On Mon, Aug 02, 2021 at 01:54:57PM +0200, Jan Kiszka wrote:
>> On 02.08.21 13:38, Marek Vasut wrote:
>>> On 8/2/21 1:36 PM, Jan Kiszka wrote:
>>>> On 02.08.21 12:48, Marek Vasut wrote:
>>>>> On 8/2/21 11:37 AM, Jan Kiszka wrote:
>>>>>> 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.
>>>>>
>>>>> This is not particularly useful answer, considering the commit log says:
>>>>> "lot of crucial things", "Possibly more", "likely also on other boards"
>>>>> and other opaque statements. But really, the problem so far happens on
>>>>> one K3 board.
>>>>
>>>> "Such things are the page table (tlb_addr),
>>>> relocated U-Boot and the active stack."
>>>
>>> Please read the rest of my answer, I don't believe the TLB should be
>>> reserved at all. DTTO for the stack. If you think otherwise, please
>>> explain why.
>>
>> Marek, I've provided you with three generic examples of active memory
>> blocks that are relevant while U-Boot is allocating from and also
>> filling that LMB. Please follow those cases and explain to us why they
>> aren't active - or at least prove why they are specific the k3 (for
>> which I found no traces).
>>
>> And stop following the TLB topic for now. That was only my first guess.
>> The actual crash I'm seeing on my board come from plain code
>> overwriting. It could have been TLB as well. It could also have been the
>> stack. All those become unprotected via your reservation removal.
> 
> Jan, one thing I didn't see before is, are you also using
> include/configs/ti_armv7_common.h in the end, like the K3 reference
> platforms, and if not are you setting bootm_size in your environment?  I
> have one more idea on why this fails on your board but not Marek's.
> Thanks.
> 

We are including that header but we didn't use DEFAULT_LINUX_BOOT_ENV,
in fact. That left bootm_size undefined. Can you explain the impact?

Jan

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