Hi Tom,
On Tue, 13 May 2025 at 00:31, Tom Rini <tr...@konsulko.com> wrote:
>
> On Sat, May 10, 2025 at 01:25:57PM +0200, Simon Glass wrote:
> > Hi Tom,
> >
> > On Tue, 6 May 2025 at 18:32, Tom Rini <tr...@konsulko.com> wrote:
> > >
> > > 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 <tr...@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.
> >
> > The whole effort is addressing problems, although it depends on your
> > definition of 'problem'.
> >
> > I'm quite happy with the approach I have taken here, incremental
> > improvement towards a long-term goal. It doesn't need to be constantly
> > rebased into out-of-tree code. People can only test what is in-tree.
> >
> > >
> > > > > 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.
> >
> > I seldom look at the commits that produced the code, but yes.
> >
> > >
> > > > > 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.
> >
> > That commit you pointed to does not actually pass the lmb_alloced()'d
> > size to image_decomp(), so it isn't perfect.
> >
> > I would much rather use the BOOTM limit everywhere, including as the
> > maximum uncompressed size of images. That is what it is for, as I
> > understand it. Users would be quite surprised if the limit were
> > ignored in certain circumstances.
>
> Yes. I believe you misunderstand the limit, and the reports that we (and
> distributions, etc) get when users have a problem would re-enforce that
> too.
OK, perhaps a topic for a future call.
Regards,
Simon
