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

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 3392 bytes --]

On Thu, Jul 29, 2021 at 06:47:02PM +0200, Marek Vasut wrote:
> On 7/29/21 5:23 PM, Tom Rini wrote:
> > On Thu, Jul 29, 2021 at 05:01:09PM +0200, Marek Vasut wrote:
> > > On 7/29/21 9:22 AM, Jan Kiszka wrote:
> > > > From: Jan Kiszka <jan.kiszka@siemens.com>
> > > > 
> > > > This reverts commit 2359fa7a87848626bcbd3399e92c657595880cd7.
> > > > 
> > > > While the goal is valid and there is surely unused memory in that area,
> > > > we also have a lot of crucial things still located at the top-of-memory
> > > > while running lmb_alloc_base. Such things are the page table (tlb_addr),
> > > > relocated U-Boot and the active stack. Possibly more. So this patch was
> > > > premature, we will need relocations of those things first if we want to
> > > > use the range.
> > > > 
> > > > Fixes booting on the IOT2050, but likely also on other boards. It got
> > > > stuck on relocating the FDT - over the relocated U-Boot code.
> > > > 
> > > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > > > ---
> > > > 
> > > > Practically,
> > > > 
> > > > void arch_lmb_reserve(struct lmb *lmb)
> > > > {
> > > > 	lmb_reserve(lmb, gd->relocaddr, gd->ram_top - gd->relocaddr);
> > > > }
> > > 
> > > So this is wrong and breaks a use case on rcar3, where there is more stuff
> > > which should not be reserved until the ram top.
> > > 
> > > I suspect the real fix here is to protect only the stack and tlb on arm64,
> > > not just everything from current stack bottom to ram top ?
> > 
> > Wait, what?  There's never been a U-Boot release that didn't reserve
> > that area
> 
> That does not mean such a huge reservation is correct.



> I apologize for not being overly clear in my answer, I already told Jan I am
> highly overloaded and simply lack the capacity to analyze this problem
> thoroughly on a short notice.

I understand.

> > 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.  This reservation is not at that spot, and a lot
more than that.  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.

> 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.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]