On Mon, Aug 09, 2021 at 07:34:34AM +0000, Ye Li wrote:
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.
Thanks for explaining. It sounds like arch/arm/mach-imx/imx8m/soc.c::the ft_system_setup() needs a comment that it uses a lot of stack, due to how complex it is, and that arch/arm/mach-imx/misc.c::board_lmb_reserve() should get moved, and reworked to just reserve another few kilobytes, for the imx8m case.