[PATCH] TI: include: env: ti_common: move fdtoverlay_addr_r to fix ramdisk size

[Tom Rini]

On Fri, Oct 11, 2024 at 05:39:53PM +0200, Mattijs Korpershoek wrote:
> Hi Judith,
> 
> Thank you for sending this patch.
> 
> On jeu., oct. 10, 2024 at 19:03, Judith Mendez <jm at ti.com> wrote:
> 
> > From: Mattijs Korpershoek <mkorpershoek at baylibre.com>
> >
> > When booting Android with adtbo_idx set, we observe the
> > following crash:
> 
> In upstream/master, this is no such thing as adtbo_idx.
> 
> This is only present in TI's U-Boot fork, via commit [1]
> 
> [1] https://git.ti.com/cgit/ti-u-boot/ti-u-boot/commit/?h=ti-u-boot-2024.04&id=1c6cf852b8b8f869d2c5e39eb071ec3025bf739a
> 
> >
> >    ** Booting bootflow 'mmc at fa10000.bootdev.whole' with android
> >    ## Booting Android Image at 0x82000000 ...
> >    Kernel load addr 0x92000000 size 20195 KiB
> >    Kernel extra command line: console=ttyS2,115200 cma=768M 8250.
> > 	nr_uarts=10 printk.devkmsg=on init=/init quiet firmware_class.
> > 	path=/vendor/firmware mem_sleep_default=deep bootconfig
> >    RAM disk load addr 0x88080000 size 16901 KiB
> >    "Synchronous Abort" handler, esr 0x96000005, far 0x155b104c8
> >    elr: 0000000080808560 lr : 0000000080808558 (reloc)
> >    elr: 00000000ffebf560 lr : 00000000ffebf558
> >    x0 : 00000000fff99000 x1 : 00000000fff9553c
> >    x2 : 000000000000000a x3 : 0000000002800000
> >    x4 : 0000000002800000 x5 : 0000000000000020
> >
> > This happens because the memory at fdtoverlay_addr_r is bogus.
> > In fact:
> >
> > => printenv fdtoverlay_addr_r
> > fdtoverlay_addr_r=0x89000000
> >
> > And the ramdisk address range is:
> > [0x88080000; 0x88080000 + 16901 KiB]
> >
> > Which is equal to:
> > [0x88080000; 0x89101400]
> >
> > So, if we represent the addresses:
> > fdtaddr             0x88000000
> > ramdisk             0x88080000
> > fdtoverlay_addr_r   0x89000000
> > ramisk (end)        0x89101400
> >
> > We see that fdtoverlay_addr_r in fact has been overridden by
> > the Android ramdisk.
> >
> > The maximum ramdisk size is 0x1080000 (15,5 MiB) and a compressed
> > Android vendor ramdisk is 15MiB:
> > $ file vendor_ramdisk.img
> > vendor_ramdisk.img: LZ4 compressed data (v0.1-v0.9)
> > $ du -sh vendor_ramdisk.img
> > 15M     vendor_ramdisk.img
> >
> > When it gets decompressed, it uses 16.5MiB, exceeding the
> > maximum ramdisk size.
> >
> > Increase the maximum ramdisk size to 20.5MiB by moving
> > fdtoverlay_addr_r higher up in the address space to fix the crash.
> 
> I do think that reserving more room between the fdtoverlay_addr_r and
> the ramdisk address is probably a good idea for Linux as well, but I
> think that the commit message might need a rewrite ?

Sorry for the slow reply here. This is I think a case where someone
needs to step back and think about all of the addresses again, and how
much gap should be between areas, along with just how big some areas can
grow to be. There should be some upper reasonable bound on overlays, and
it will be safer long term I believe to have that above the device tree
and below the ramdisk. Placing things above the ramdisk always results
in pain as there's always some new larger image for some valid use case.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20241014/f633d876/attachment.sig>