* [PATCH] spl: return kernel image header size in os boot @ 2025-02-21 15:17 Anshul Dalal 2025-02-21 20:04 ` Tom Rini 2025-02-25 3:06 ` Sean Anderson 0 siblings, 2 replies; 5+ messages in thread From: Anshul Dalal @ 2025-02-21 15:17 UTC (permalink / raw) To: u-boot; +Cc: Anshul Dalal, vigneshr, seanga2 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; } -- 2.43.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] spl: return kernel image header size in os boot 2025-02-21 15:17 [PATCH] spl: return kernel image header size in os boot Anshul Dalal @ 2025-02-21 20:04 ` Tom Rini 2025-02-22 10:35 ` [EXTERNAL] " Anshul Dalal 2025-02-25 3:06 ` Sean Anderson 1 sibling, 1 reply; 5+ messages in thread From: Tom Rini @ 2025-02-21 20:04 UTC (permalink / raw) To: Anshul Dalal; +Cc: u-boot, vigneshr, seanga2 [-- Attachment #1: Type: text/plain, Size: 2018 bytes --] 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? -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [EXTERNAL] Re: [PATCH] spl: return kernel image header size in os boot 2025-02-21 20:04 ` Tom Rini @ 2025-02-22 10:35 ` Anshul Dalal 2025-02-24 15:34 ` Tom Rini 0 siblings, 1 reply; 5+ messages in thread From: Anshul Dalal @ 2025-02-22 10:35 UTC (permalink / raw) To: Tom Rini; +Cc: u-boot, vigneshr, seanga2 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [EXTERNAL] Re: [PATCH] spl: return kernel image header size in os boot 2025-02-22 10:35 ` [EXTERNAL] " Anshul Dalal @ 2025-02-24 15:34 ` Tom Rini 0 siblings, 0 replies; 5+ messages in thread From: Tom Rini @ 2025-02-24 15:34 UTC (permalink / raw) To: Anshul Dalal; +Cc: u-boot, vigneshr, seanga2 [-- Attachment #1: Type: text/plain, Size: 3524 bytes --] 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@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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] spl: return kernel image header size in os boot 2025-02-21 15:17 [PATCH] spl: return kernel image header size in os boot Anshul Dalal 2025-02-21 20:04 ` Tom Rini @ 2025-02-25 3:06 ` Sean Anderson 1 sibling, 0 replies; 5+ messages in thread From: Sean Anderson @ 2025-02-25 3:06 UTC (permalink / raw) To: Anshul Dalal, u-boot; +Cc: vigneshr On 2/21/25 10:17, 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; > } > Reviewed-by: Sean Anderson <seanga2@gmail.com> ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-02-25 3:06 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-02-21 15:17 [PATCH] spl: return kernel image header size in os boot Anshul Dalal 2025-02-21 20:04 ` Tom Rini 2025-02-22 10:35 ` [EXTERNAL] " Anshul Dalal 2025-02-24 15:34 ` Tom Rini 2025-02-25 3:06 ` Sean Anderson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox