Re: [EXTERNAL] Re: [PATCH] spl: return kernel image header size in os boot

[Anshul Dalal <anshuld@ti.com>]

On Sat Feb 22, 2025 at 1:34 AM IST, Tom Rini wrote:
> On Fri, Feb 21, 2025 at 08:47:51PM +0530, Anshul Dalal wrote:
>
> > During kernel build process the header size is computed including the
> > BSS whereas it's removed when creating the uncompressed image. Therefore
> > the size of the uncompressed image on filesystem will be smaller than
> > the size specifiedin the header.
> > 
> > Therefore it makes sense to return the header size back instead of the
> > file size in falcon boot.
> > 
> > More info:
> > https://lore.kernel.org/u-boot/20250214111656.2358748-1-anshuld@ti.com/
> > 
> > Signed-off-by: Anshul Dalal <anshuld@ti.com>
> > ---
> >  common/spl/spl_ext.c | 4 ++++
> >  common/spl/spl_fat.c | 3 +++
> >  2 files changed, 7 insertions(+)
> > 
> > diff --git a/common/spl/spl_ext.c b/common/spl/spl_ext.c
> > index c5478820a9b..6d8d6544092 100644
> > --- a/common/spl/spl_ext.c
> > +++ b/common/spl/spl_ext.c
> > @@ -17,6 +17,10 @@ static ulong spl_fit_read(struct spl_load_info *load, ulong file_offset,
> >  	ret = ext4fs_read(buf, file_offset, size, &actlen);
> >  	if (ret)
> >  		return ret;
> > +
> > +	if (IS_ENABLED(CONFIG_SPL_OS_BOOT) && IS_ENABLED(CONFIG_CMD_BOOTI))
> > +		return size;
> > +
> >  	return actlen;
> >  }
> >  
> > diff --git a/common/spl/spl_fat.c b/common/spl/spl_fat.c
> > index fce451b7664..ddf85e2cece 100644
> > --- a/common/spl/spl_fat.c
> > +++ b/common/spl/spl_fat.c
> > @@ -55,6 +55,9 @@ static ulong spl_fit_read(struct spl_load_info *load, ulong file_offset,
> >  	if (ret)
> >  		return ret;
> >  
> > +	if (IS_ENABLED(CONFIG_SPL_OS_BOOT) && IS_ENABLED(CONFIG_CMD_BOOTI))
> > +		return size;
> > +
> >  	return actread;
> >  }
>
> Some image formats include information about the BSS as well, and others
> do not. But this is just having some parts of SPL return the filesize
> and not doing some deeper probing. Given that usually problems like this
> arise from loading contents in to memory too close together and then
> getting wiped out by the kernel BSS, what's the problem situation you've
> run in to, where, and with what loaded where in memory?

The problem is not about the allocation of BSS section but when loading
an uncompressed kernel image with the check in _spl_load at
`include/spl_load.h:95` which fails to -EIO even when the image is loaded
properly because it compares the size form the header to the size of the
file read from the FS:

	return read < spl_image->size ? -EIO : 0;

This doesn't work for uncompressed kernel image because the size from
the header (the parameter ulong size for spl_fit_read) includes BSS
which is not part of the final image file. And there is no way of
knowing the file size based on the header. Hence why we check for
CONFIG_CMD_BOOTI for returning the header size directly.

Although it might be better to have a different function altogether for
reading uncompressed image that can be passed to spl_load_init so we are
not using spl_**fit**_read to read non fit images.

You can find the bug report here:
https://lore.kernel.org/u-boot/20250214111656.2358748-1-anshuld@ti.com/

- Anshul