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

[Ye Li]

Hi Marek,

On Sun, 2021-08-08 at 17:25 +0200, Marek Vasut wrote:
> Caution: EXT Email
> 
> On 8/8/21 4:54 PM, Tom Rini wrote:
> 
> [...]
> 
> > 
> > > 
> > > > 
> > > > 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 ?
> > Yes, LMB has been around since the PowerPC device tree days I
> > suspect (I
> > didn't dig that far back), but only used outside of the "don't
> > overwrite
> > the running U-Boot while we relocate device tree / initrd before
> > booting
> > OS" since 2018 or so.
> So, are we using LMB for two different things now ?
> 
> > 
> > > 
> > > [...]
> > > 
> > > > 
> > > > > 
> > > > > > 
> > > > > > 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://eur01.safelinks.protection.outlook.com/?url=https%3A%
> > > > > 2F%2Fsource.denx.de%2Fu-boot%2Fcustodians%2Fu-boot-sh%2F-
> > > > > %2Ftree%2Flmb-
> > > > > v1&data=04%7C01%7Cye.li%40nxp.com%7Cb9bda480d0494a9249c70
> > > > > 8d95a80c552%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6376
> > > > > 40331407737098%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLC
> > > > > JQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata
> > > > > =yhIbMHWZMjXy59BVDFVbY2owM7TNdWvk%2B3w2IHg78ok%3D&reserve
> > > > > d=0
> > > > > 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 ?
> > It's a good question, yes.  And some more details about what
> > exactly the
> > NXP folks were doing to hit that would also be nice.
> +CC Ye Li.

On i.MX8QM/QXP, we implement the ft_system_setup to update kernel FDT.
It needs larger stack size to parse the FDT to disable nodes if the
corresponding resources are not owned by A core.
When we enabled the initrd relocation in u-boot, it allocates a space
from LMB for initrd just before the SP reservation. The stack overflow
overwrites the initrd and cause kernel issue.

The size of stack reservation actually depends on the implementation.
There are lots of board or soc level functions in the boot sequence.
You can't predict how much stack is needed. So providing a way that can
adjust the size is useful.


Best regards,
Ye Li