| Message ID | 20240119201356.7903-3-ansuelsmth@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM: decompressor: support AUTO_ZRELADDR and appended DTB | expand |
On Fri, Jan 19, 2024 at 9:14 PM Christian Marangi <ansuelsmth@gmail.com> wrote: > Some bootloaders can pass broken MEM ATAGs that provide hardcoded > information about mounted RAM size and physical location. > Example booloader provide RAM of size 1.7Gb but actual mounted RAM > size is 512Mb causing kernel panic. > > Add option CONFIG_ARM_ATAG_DTB_COMPAT_IGNORE_MEM to ignore these ATAG > and not augument appended DTB memory node. > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> So you cannot just ignore all the ATAGs I guess? If it's the command line you need, you can pass an identical one in chosen. But if you really need this, it should be there. Acked-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
On Fri, Jan 19, 2024 at 9:14 PM Christian Marangi <ansuelsmth@gmail.com> wrote: > +#if defined(CONFIG_ARM_ATAG_DTB_COMPAT_IGNORE_MEM) > +#define do_ignore_mem 1 > +#else > +#define do_ignore_mem 0 > +#endif Is there a reason why you can't just use: if (IS_ENABLED(CONFIG_ARM_ATAG_DTB_COMPAT_IGNORE_MEM)) in the code? Yours, Linus Walleij
On Sat, Jan 20, 2024 at 12:51:06PM +0100, Linus Walleij wrote: > On Fri, Jan 19, 2024 at 9:14 PM Christian Marangi <ansuelsmth@gmail.com> wrote: > > > Some bootloaders can pass broken MEM ATAGs that provide hardcoded > > information about mounted RAM size and physical location. > > Example booloader provide RAM of size 1.7Gb but actual mounted RAM > > size is 512Mb causing kernel panic. > > > > Add option CONFIG_ARM_ATAG_DTB_COMPAT_IGNORE_MEM to ignore these ATAG > > and not augument appended DTB memory node. > > > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > > So you cannot just ignore all the ATAGs I guess? > If it's the command line you need, you can pass an identical one in > chosen. Ehhh it's not that trivial... Downstream we have a patch that takes just the bootargs from ATAGs but reality is that only MEM is broken. Also duplicating the bootargs from bootloader in chosen is problematic as have tons of device that use cmdlinepart to declare partitions and we are not aware of the partitions of every device. And also there are some device that supports dual partition and the value is provided by the bootloader bootargs so duplicating it would result in not having a good way to support this. > > But if you really need this, it should be there. > Acked-by: Linus Walleij <linus.walleij@linaro.org> >
On Sat, Jan 20, 2024 at 12:52:33PM +0100, Linus Walleij wrote: > On Fri, Jan 19, 2024 at 9:14 PM Christian Marangi <ansuelsmth@gmail.com> wrote: > > > +#if defined(CONFIG_ARM_ATAG_DTB_COMPAT_IGNORE_MEM) > > +#define do_ignore_mem 1 > > +#else > > +#define do_ignore_mem 0 > > +#endif > > Is there a reason why you can't just use: > > if (IS_ENABLED(CONFIG_ARM_ATAG_DTB_COMPAT_IGNORE_MEM)) > in the code? > Was following the pattern, yes I can totally do this change... Will send a v2 with this changed. Since the first patch has to be regression tested, is it ok to add the Tag in v2 or I should wait that to send v2?
On Sat, Jan 20, 2024 at 6:00 PM Christian Marangi <ansuelsmth@gmail.com> wrote: > On Sat, Jan 20, 2024 at 12:52:33PM +0100, Linus Walleij wrote: > > On Fri, Jan 19, 2024 at 9:14 PM Christian Marangi <ansuelsmth@gmail.com> wrote: > > > > > +#if defined(CONFIG_ARM_ATAG_DTB_COMPAT_IGNORE_MEM) > > > +#define do_ignore_mem 1 > > > +#else > > > +#define do_ignore_mem 0 > > > +#endif > > > > Is there a reason why you can't just use: > > > > if (IS_ENABLED(CONFIG_ARM_ATAG_DTB_COMPAT_IGNORE_MEM)) > > in the code? > > > > Was following the pattern, yes I can totally do this change... Will send > a v2 with this changed. > > Since the first patch has to be regression tested, is it ok to add the > Tag in v2 or I should wait that to send v2? Just add the tag. Thanks, Linus Walleij
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index b2ab8db63c4b..6bb5c6b28106 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -1549,6 +1549,18 @@ config ARM_ATAG_DTB_COMPAT bootloaders, this option allows zImage to extract the information from the ATAG list and store it at run time into the appended DTB. +config ARM_ATAG_DTB_COMPAT_IGNORE_MEM + bool "Ignore MEM ATAG information from bootloader" + depends on ARM_ATAG_DTB_COMPAT + help + Some bootloaders can pass broken MEM ATAGs that provide hardcoded + information about mounted RAM size and physical location. + Example booloader provide RAM of size 1.7Gb but actual mounted RAM + size is 512Mb causing kernel panic. + + Enable this option if MEM ATAGs should be ignored and the memory + node in the appended DTB should NOT be augumented. + choice prompt "Kernel command line type" if ARM_ATAG_DTB_COMPAT default ARM_ATAG_DTB_COMPAT_CMDLINE_FROM_BOOTLOADER diff --git a/arch/arm/boot/compressed/atags_to_fdt.c b/arch/arm/boot/compressed/atags_to_fdt.c index 627752f18661..189db9fc7fea 100644 --- a/arch/arm/boot/compressed/atags_to_fdt.c +++ b/arch/arm/boot/compressed/atags_to_fdt.c @@ -10,6 +10,12 @@ #define do_extend_cmdline 0 #endif +#if defined(CONFIG_ARM_ATAG_DTB_COMPAT_IGNORE_MEM) +#define do_ignore_mem 1 +#else +#define do_ignore_mem 0 +#endif + #define NR_BANKS 16 static int node_offset(void *fdt, const char *node_path) @@ -170,6 +176,10 @@ int atags_to_fdt(void *atag_list, void *fdt, int total_space) setprop_string(fdt, "/chosen", "bootargs", atag->u.cmdline.cmdline); } else if (atag->hdr.tag == ATAG_MEM) { + /* Bootloader MEM ATAG are broken and should be ignored */ + if (do_ignore_mem) + continue; + if (memcount >= sizeof(mem_reg_property)/4) continue; if (!atag->u.mem.size)
Some bootloaders can pass broken MEM ATAGs that provide hardcoded information about mounted RAM size and physical location. Example booloader provide RAM of size 1.7Gb but actual mounted RAM size is 512Mb causing kernel panic. Add option CONFIG_ARM_ATAG_DTB_COMPAT_IGNORE_MEM to ignore these ATAG and not augument appended DTB memory node. Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> --- arch/arm/Kconfig | 12 ++++++++++++ arch/arm/boot/compressed/atags_to_fdt.c | 10 ++++++++++ 2 files changed, 22 insertions(+)