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 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); > } > > 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 ?