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

From: Jan Kiszka <jan.kiszka@siemens.com>
To: Marek Vasut <marex@denx.de>, Tom Rini <trini@konsulko.com>
Cc: U-Boot Mailing List <u-boot@lists.denx.de>,
	Hai Pham <hai.pham.ud@renesas.com>,
	Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>,
	Stephen Warren <swarren@nvidia.com>,
	Lokesh Vutla <lokeshvutla@ti.com>
Subject: Re: [PATCH] Revert "arm: bootm: Disable LMB reservation for command line and board info on arm64"
Date: Mon, 2 Aug 2021 13:54:57 +0200	[thread overview]
Message-ID: <a29984d2-1633-37a1-1133-04cacc927830@siemens.com> (raw)
In-Reply-To: <4602641b-34d2-22dc-01da-e8f46fcc1be3@denx.de>

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

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