On 8/8/21 4:00 PM, Tom Rini wrote:
On Sun, Aug 08, 2021 at 03:45:30PM +0200, Marek Vasut wrote:
On 8/6/21 6:49 PM, Tom Rini wrote:
On Fri, Aug 06, 2021 at 12:22:43AM +0200, Marek Vasut wrote:
On 8/3/21 11:51 PM, Tom Rini wrote:
On Mon, Aug 02, 2021 at 05:27:59PM -0400, Tom Rini wrote:
On Thu, Jul 29, 2021 at 09:22:02AM +0200, Jan Kiszka wrote:

From: Jan Kiszka <[email protected]>

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 <[email protected]>
---

Practically,

void arch_lmb_reserve(struct lmb *lmb)
{
        lmb_reserve(lmb, gd->relocaddr, gd->ram_top - gd->relocaddr);
}

worked as well for me - but it left the stack in danger.

I want to cycle back up to this practically part.  Marek, would changing
the arch_lmb_reserve (or possibly even making this a more global thing /
option) still let the area you want exposed on rcar3 (I assume) be
exposed ?  Or would it be covered again?  Part of the problem,
practically speaking, is that we need to ensure that as part of
(attempting and likely succeeding in) booting the OS we don't overwrite
ourself and hang.

I think large part of the problem is the purpose of LMB is unclear at best.

The arch_lmb_reserve() says this:

   54 void arch_lmb_reserve(struct lmb *lmb)
[...]
   59 /*
   60  * Booting a (Linux) kernel image
   61  *
   62  * Allocate space for command line and board info - the
   63  * address should be as high as possible within the reach of
   64  * the kernel (see CONFIG_SYS_BOOTMAPSZ settings), but in unused
   65  * memory, which means far enough below the current stack
   66  * pointer.
   67  */

So basically reserve chunk of memory in the right place, which can be safely
passed to the kernel.

No.  This isn't the case.  We reserve chunks of memory away from other
usage by U-Boot.

Then I have to wonder, why is this not called in board_init_f or
board_init_r , but only after bootm or similar boot command was called ?

I also wonder a little bit.  That it does not is why we have the LMB
checks under fs/ and net/ and doing this sooner would possibly make
dealing with those CVEs either easier or would also address some other
classes of them that may exist.

So, why not move it into the relocation code then ?

I expect it was not simply because up
until rather recently we didn't have any checks for "don't overwrite
specific areas of memory" other than right before firing off the OS (and
modify whatever memory you want to modify is a feature not a bug).

The LMB has been around since forever though ?

[...]

OK, so then there isn't a problem reverting this commit for rcar?

The revert will break the use case where the other CPUs are using memory
above U-Boot, but have a look at the following branch, it should permit me
to parametrize the arch_lmb_reserve() better and reserve the right memory
areas per architecture/mach/board, and even clean the arch_lmb_reserve up
further:
https://source.denx.de/u-boot/custodians/u-boot-sh/-/tree/lmb-v1
So yes, pick the revert and I'll submit the four patches for likely next
release.

Thanks for explaining, I'll pick up the revert patch then.

For your LMB tree, I like the initial approach but looking at
528915c71762 ("imx: Fix potential lmb memory overwritten by stack") I
think that shows the general "4K is enough for stack we hope" is wrong,
and we should do 16K instead for everyone as the default.  But we can
discuss that more too once you post the whole series which again, I
think is the right direction.

The IMX thing is odd indeed and raises a bigger question -- what is the "right" amount of stack to reserve ?

Reply via email to