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

[Tom Rini]

On Sat, Feb 22, 2025 at 04:05:55PM +0530, Anshul Dalal wrote:
> 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 at 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/

Thanks for explaining. You should include a fixes tag and explain a bit
more about the problem being fixed in spl_load, thanks.

-- 
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/20250224/b5144daa/attachment.sig>