This series try to address a long lasting problem with legacy device
that require an appended DTB and the use of AUTO_ZRELADDR.
With these device AUTO_ZRELADDR is not possible if for some reason at
the start of the RAM it's needed to reserve some space. (example qcom SoC
that allocate reserved space for SMEM)
In the current implementation with appended DTB and AUTO_ZRELADDR,
the memory start is only derived from the PC register and it can't be
changed by declaring additional info in the DTS.
In a normal setup, we have an intentional undocumented chosen property
to handle this and the memory node to declare the start of the memory.
With this applied and ARM_ATAG_DTB_COMPAT_IGNORE_MEM enabled (more
info in the related patch) ipq806x can boot right away with AUTO_ZRELADDR
enabled and a correct memory node defined in DTS.
It's needed to ignore MEM ATAGs as most of the time the values from the
bootloader are hardcoded and OEM didn't care to actually provide them
resulting in funny situation where a Netgear R7800 with 512Mb of RAM
have Uboot passing 1.7GB of RAM with ATAGS.
I'm open to any suggestion on how this can be improved and I would love
some additional testing on other legacy platform but I assume this will
permit many legacy device to be correctly supported without having to
hardcode address.
Christian Marangi (2):
ARM: decompressor: support memory start validation for appended DTB
ARM: decompressor: add option to ignore MEM ATAGs
arch/arm/Kconfig | 12 ++++++++++++
arch/arm/boot/compressed/atags_to_fdt.c | 10 ++++++++++
arch/arm/boot/compressed/head.S | 22 ++++++++++++++++++++++
3 files changed, 44 insertions(+)
--
2.43.0
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 <[email protected]>
---
arch/arm/Kconfig | 12 ++++++++++++
arch/arm/boot/compressed/atags_to_fdt.c | 10 ++++++++++
2 files changed, 22 insertions(+)
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)
--
2.43.0
There is currently a problem with a very specific sets of kernel config
and AUTO_ZRELADDR.
For the most common case AUTO_ZRELADDR check the PC register and
calculate the start of the physical memory. Then fdt_check_mem_start is
called to make sure the detected value makes sense by comparing it with
what is present in DTB in the memory nodes and if additional fixup are
required with the use of linux,usable-memory-range in the chosen node to
hardcode usable memory range in case some reserved space needs to be
addressed. With the help of this function the right address is
calculated and the kernel correctly decompress and loads.
Things starts to become problematic when in the mix,
CONFIG_ARM_APPENDED_DTB is used. This is a particular kernel config is
used when legacy systems doesn't support passing a DTB directly and a
DTB is appended at the end of the image.
In such case, fdt_check_mem_start is skipped in AUTO_ZRELADDR iteration
as the appended DTB can be augumented later with ATAGS passed from the
bootloader (if CONFIG_ARM_ATAG_DTB_COMPAT is enabled).
The main problem and what this patch address is the fact that
fdt_check_mem_start is never called later when the appended DTB is
augumented, hence any fixup and validation is not done making AUTO_ZRELADDR
detection inconsistent and most of the time wrong.
Add support in head.S for this by checking if AUTO_ZRELADDR is enabled
and calling fdt_check_mem_start with the appended DTB and the augumented
values permitting legacy device to provide info in DTB instead of
disabling AUTO_ZRELADDR and hardcoding the physical address offsets.
Signed-off-by: Christian Marangi <[email protected]>
---
arch/arm/boot/compressed/head.S | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index 9f406e9c0ea6..2ff38a8df1f0 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -443,6 +443,28 @@ restart: adr r0, LC1
add r6, r6, r5
add r10, r10, r5
add sp, sp, r5
+
+#ifdef CONFIG_AUTO_ZRELADDR
+ /*
+ * Validate calculated start of physical memory with appended DTB.
+ * In the first iteration for physical memory start calculation,
+ * we skipped validating it as it could have been augumented by
+ * ATAGS stored at an offset from the same start of physical memory.
+ *
+ * We now have parsed them and augumented the appended DTB if asked
+ * so we can finally validate the start of physical memory.
+ *
+ * This is needed to apply additional fixup with
+ * linux,usable-memory-range or to make sure AUTO_ZRELADDR detected
+ * the correct value.
+ */
+ sub r0, r4, #TEXT_OFFSET @ revert to base address
+ mov r1, r8 @ use appended DTB
+ bl fdt_check_mem_start
+
+ /* Determine final kernel image address. */
+ add r4, r0, #TEXT_OFFSET
+#endif
dtb_check_done:
#endif
--
2.43.0
On Fri, Jan 19, 2024 at 9:14 PM Christian Marangi <ansuelsmth@gmailcom> wrote:
> There is currently a problem with a very specific sets of kernel config
> and AUTO_ZRELADDR.
>
> For the most common case AUTO_ZRELADDR check the PC register and
> calculate the start of the physical memory. Then fdt_check_mem_start is
> called to make sure the detected value makes sense by comparing it with
> what is present in DTB in the memory nodes and if additional fixup are
> required with the use of linux,usable-memory-range in the chosen node to
> hardcode usable memory range in case some reserved space needs to be
> addressed. With the help of this function the right address is
> calculated and the kernel correctly decompress and loads.
>
> Things starts to become problematic when in the mix,
> CONFIG_ARM_APPENDED_DTB is used. This is a particular kernel config is
> used when legacy systems doesn't support passing a DTB directly and a
> DTB is appended at the end of the image.
>
> In such case, fdt_check_mem_start is skipped in AUTO_ZRELADDR iteration
> as the appended DTB can be augumented later with ATAGS passed from the
> bootloader (if CONFIG_ARM_ATAG_DTB_COMPAT is enabled).
>
> The main problem and what this patch address is the fact that
> fdt_check_mem_start is never called later when the appended DTB is
> augumented, hence any fixup and validation is not done making AUTO_ZRELADDR
> detection inconsistent and most of the time wrong.
>
> Add support in head.S for this by checking if AUTO_ZRELADDR is enabled
> and calling fdt_check_mem_start with the appended DTB and the augumented
> values permitting legacy device to provide info in DTB instead of
> disabling AUTO_ZRELADDR and hardcoding the physical address offsets.
>
> Signed-off-by: Christian Marangi <[email protected]>
LGTM, so
Reviewed-by: Geert Uytterhoeven <[email protected]>
I plan to give this a try (for regression testing) next week.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68korg
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Christian,
On Fri, Jan 19, 2024 at 9:14 PM Christian Marangi <ansuelsmth@gmailcom> wrote:
> With this applied and ARM_ATAG_DTB_COMPAT_IGNORE_MEM enabled (more
> info in the related patch) ipq806x can boot right away with AUTO_ZRELADDR
> enabled and a correct memory node defined in DTS.
>
> It's needed to ignore MEM ATAGs as most of the time the values from the
> bootloader are hardcoded and OEM didn't care to actually provide them
> resulting in funny situation where a Netgear R7800 with 512Mb of RAM
> have Uboot passing 1.7GB of RAM with ATAGS.
I guess you still need other values from ATAGS, so building a kernel
without CONFIG_ARM_ATAG_DTB_COMPAT is not an option?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68korg
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On Fri, Jan 19, 2024 at 9:14 PM Christian Marangi <ansuelsmth@gmailcom> wrote:
> There is currently a problem with a very specific sets of kernel config
> and AUTO_ZRELADDR.
>
> For the most common case AUTO_ZRELADDR check the PC register and
> calculate the start of the physical memory. Then fdt_check_mem_start is
> called to make sure the detected value makes sense by comparing it with
> what is present in DTB in the memory nodes and if additional fixup are
> required with the use of linux,usable-memory-range in the chosen node to
> hardcode usable memory range in case some reserved space needs to be
> addressed. With the help of this function the right address is
> calculated and the kernel correctly decompress and loads.
>
> Things starts to become problematic when in the mix,
> CONFIG_ARM_APPENDED_DTB is used. This is a particular kernel config is
> used when legacy systems doesn't support passing a DTB directly and a
> DTB is appended at the end of the image.
>
> In such case, fdt_check_mem_start is skipped in AUTO_ZRELADDR iteration
> as the appended DTB can be augumented later with ATAGS passed from the
> bootloader (if CONFIG_ARM_ATAG_DTB_COMPAT is enabled).
>
> The main problem and what this patch address is the fact that
> fdt_check_mem_start is never called later when the appended DTB is
> augumented, hence any fixup and validation is not done making AUTO_ZRELADDR
> detection inconsistent and most of the time wrong.
>
> Add support in head.S for this by checking if AUTO_ZRELADDR is enabled
> and calling fdt_check_mem_start with the appended DTB and the augumented
> values permitting legacy device to provide info in DTB instead of
> disabling AUTO_ZRELADDR and hardcoding the physical address offsets.
>
> Signed-off-by: Christian Marangi <[email protected]>
Looks like the right solution to a very long-standing problem!
Thanks for your tireless hacking at this.
Reviewed-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
On Fri, Jan 19, 2024 at 9:14 PM Christian Marangi <ansuelsmth@gmailcom> 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 <[email protected]>
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 <[email protected]>
Yours,
Linus Walleij
On Fri, Jan 19, 2024 at 9:14 PM Christian Marangi <ansuelsmth@gmailcom> 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 <[email protected]> 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 <[email protected]>
>
> 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 <[email protected]>
>
--
Ansuel
On Sat, Jan 20, 2024 at 12:52:33PM +0100, Linus Walleij wrote:
> On Fri, Jan 19, 2024 at 9:14 PM Christian Marangi <[email protected]> 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?
--
Ansuel
On Sat, Jan 20, 2024 at 12:19:33PM +0100, Geert Uytterhoeven wrote:
> Hi Christian,
>
> On Fri, Jan 19, 2024 at 9:14 PM Christian Marangi <[email protected]> wrote:
> > With this applied and ARM_ATAG_DTB_COMPAT_IGNORE_MEM enabled (more
> > info in the related patch) ipq806x can boot right away with AUTO_ZRELADDR
> > enabled and a correct memory node defined in DTS.
> >
> > It's needed to ignore MEM ATAGs as most of the time the values from the
> > bootloader are hardcoded and OEM didn't care to actually provide them
> > resulting in funny situation where a Netgear R7800 with 512Mb of RAM
> > have Uboot passing 1.7GB of RAM with ATAGS.
>
> I guess you still need other values from ATAGS, so building a kernel
> without CONFIG_ARM_ATAG_DTB_COMPAT is not an option?
>
As I said in the other patch, yes DTB_COMPAT is still needed for
bootargs passed from the bootloader via ATAGs. Sorry for not making this
detail more clear in the cover letter.
--
Ansuel
On Sat, Jan 20, 2024 at 6:00 PM Christian Marangi <ansuelsmth@gmailcom> 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 <[email protected]> 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