Re: [PATCH v2 0/3] booti: Remove the SYS_BOOTM_LEN limit for booti

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 2612 bytes --]

On Tue, May 06, 2025 at 03:24:21PM +0200, Simon Glass wrote:
> Hi Tom,
> 
> On Mon, 5 May 2025 at 20:14, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Thu, May 01, 2025 at 07:18:32AM -0600, Simon Glass wrote:
> >
> > > This series restores the original behaviour of extlinux booting linux
> > > 'Image' files, which is to ignore CONFIG_SYS_BOOTM_LEN and instead uses
> > > a limit of 10x the compressed size.
> > >
> > > It also adds RISC-V support, since it uses a similar format to ARM64.
> > >
> > > Future work should integrate the code in 'booti' into main 'bootm'
> > > logic.
> >
> > I don't like "in the future we'll remove duplicated code".
> 
> Small series, fixes a problem, can be made larger but then it isn't a bug-fix.

Which is why yes, this should have been instead of the however-many-part
"PXE" series, and then fixups on top, a series to address this problem.
Then other series to address other problems.

> > I also don't
> > like not seeing that what we really need to do, in all cases (not just
> > booti) handle decompression like we do for FIT images, and ask LMB to
> > give us a space to use.
> 
> See bootm_load_os() which does already do this.

Yes, that's what I was asking you to look at. I assume you're even
specifically pointing to commit 69544c4fd8b1 ("bootm: Support
kernel_noload with compression") which you and I did together.

> > A problem is that CONFIG_SYS_BOOTM_LEN was never
> > intended to be the limit on *decompression* as it's the limit on what
> > we're loading to memory from disk. That's what getting me unhappy with
> > this part of the series.
> 
> From what I can tell, that was introduced 11 years ago by this commit:
> 
> 081cc197472 (HEAD) bootm: Export bootm_decomp_image()
> 
> I suppose the idea is that BOOTM is supposed to be a limit on the
> image being loaded, so it is compressed, then the limit needs to apply
> to the size of the uncompressed image, to maintain parity. Otherwise
> there would be no limit at all, since it is pretty easy to devise an
> 100-byte image which expands to fill all available memory.
> 
> Using 10x the uncompressed size doesn't fill me with the joys of spring, TBH.

Yes, it's a matter of heuristics, and also why we have things like lmb
to check and make sure we don't overwrite ourselves. I do not recall if
the 10x used there, or the 4x we used for kernel_noload FIT
decompression, was based on checking what the compression factor the
available algorithms get on typical kernel images or not. That would be
another improvement area, sure.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]