[v1] spl: nand: allow partial nand page reads during nand_spl_load_image
The nand_spl_load_image function was guaranteed to read an entire block into RAM, regardless of how many bytes were to be read. This is particularly problematic when spl_load_legacy_image is called, as this function attempts to load a struct image_header but gets surprised with a full flash sector.
Allow partial reads to restore functionality to the code path spl_nand_load_element() -> spl_load_legacy_img() -> spl_nand_legacy_read(load, header, sizeof(hdr), header);
Signed-off-by: Colin Foster colin.foster@in-advantage.com --- drivers/mtd/nand/raw/nand_spl_loaders.c | 36 ++++++++++++++++--------- 1 file changed, 24 insertions(+), 12 deletions(-)
diff --git a/drivers/mtd/nand/raw/nand_spl_loaders.c b/drivers/mtd/nand/raw/nand_spl_loaders.c index 4befc75c04..84ac482c06 100644 --- a/drivers/mtd/nand/raw/nand_spl_loaders.c +++ b/drivers/mtd/nand/raw/nand_spl_loaders.c @@ -1,9 +1,18 @@ +/* + * Temporary storage for non NAND page aligned and non NAND page sized + * reads. Note: This does not support runtime detected FLASH yet, but + * that should be reasonably easy to fix by making the buffer large + * enough :) + */ +static u8 scratch_buf[CONFIG_SYS_NAND_PAGE_SIZE]; + int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst) { + unsigned int read_this_time = CONFIG_SYS_NAND_PAGE_SIZE; + unsigned int size_remaining = size; unsigned int block, lastblock; unsigned int page, page_offset;
- /* offs has to be aligned to a page address! */ block = offs / CONFIG_SYS_NAND_BLOCK_SIZE; lastblock = (offs + size - 1) / CONFIG_SYS_NAND_BLOCK_SIZE; page = (offs % CONFIG_SYS_NAND_BLOCK_SIZE) / CONFIG_SYS_NAND_PAGE_SIZE; @@ -12,8 +21,18 @@ int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst) while (block <= lastblock) { if (!nand_is_bad_block(block)) { /* Skip bad blocks */ - while (page < CONFIG_SYS_NAND_PAGE_COUNT) { - nand_read_page(block, page, dst); + while (page < CONFIG_SYS_NAND_PAGE_COUNT && + size_remaining > 0) { + if (size_remaining < CONFIG_SYS_NAND_PAGE_SIZE) + { + nand_read_page(block, page, + scratch_buf); + memcpy(dst, scratch_buf, + size_remaining); + read_this_time = size_remaining; + } else { + nand_read_page(block, page, dst); + } /* * When offs is not aligned to page address the * extra offset is copied to dst as well. Copy @@ -26,7 +45,8 @@ int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst) dst = (void *)((int)dst - page_offset); page_offset = 0; } - dst += CONFIG_SYS_NAND_PAGE_SIZE; + dst += read_this_time; + size_remaining -= read_this_time; page++; }
@@ -70,14 +90,6 @@ u32 nand_spl_adjust_offset(u32 sector, u32 offs) }
#ifdef CONFIG_SPL_UBI -/* - * Temporary storage for non NAND page aligned and non NAND page sized - * reads. Note: This does not support runtime detected FLASH yet, but - * that should be reasonably easy to fix by making the buffer large - * enough :) - */ -static u8 scratch_buf[CONFIG_SYS_NAND_PAGE_SIZE]; - /** * nand_spl_read_block - Read data from physical eraseblock into a buffer * @block: Number of the physical eraseblock
Hi Colin,
On Tue, Nov 15, 2022 at 5:35 PM Colin Foster colin.foster@in-advantage.com wrote:
The nand_spl_load_image function was guaranteed to read an entire block into RAM, regardless of how many bytes were to be read. This is particularly problematic when spl_load_legacy_image is called, as this function attempts to load a struct image_header but gets surprised with a full flash sector.
Allow partial reads to restore functionality to the code path spl_nand_load_element() -> spl_load_legacy_img() -> spl_nand_legacy_read(load, header, sizeof(hdr), header);
Signed-off-by: Colin Foster colin.foster@in-advantage.com
drivers/mtd/nand/raw/nand_spl_loaders.c | 36 ++++++++++++++++--------- 1 file changed, 24 insertions(+), 12 deletions(-)
diff --git a/drivers/mtd/nand/raw/nand_spl_loaders.c b/drivers/mtd/nand/raw/nand_spl_loaders.c index 4befc75c04..84ac482c06 100644 --- a/drivers/mtd/nand/raw/nand_spl_loaders.c +++ b/drivers/mtd/nand/raw/nand_spl_loaders.c @@ -1,9 +1,18 @@ +/*
- Temporary storage for non NAND page aligned and non NAND page sized
- reads. Note: This does not support runtime detected FLASH yet, but
- that should be reasonably easy to fix by making the buffer large
- enough :)
- */
+static u8 scratch_buf[CONFIG_SYS_NAND_PAGE_SIZE];
int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst) {
unsigned int read_this_time = CONFIG_SYS_NAND_PAGE_SIZE;unsigned int size_remaining = size; unsigned int block, lastblock; unsigned int page, page_offset;
/* offs has to be aligned to a page address! */ block = offs / CONFIG_SYS_NAND_BLOCK_SIZE; lastblock = (offs + size - 1) / CONFIG_SYS_NAND_BLOCK_SIZE; page = (offs % CONFIG_SYS_NAND_BLOCK_SIZE) / CONFIG_SYS_NAND_PAGE_SIZE;@@ -12,8 +21,18 @@ int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst) while (block <= lastblock) { if (!nand_is_bad_block(block)) { /* Skip bad blocks */
while (page < CONFIG_SYS_NAND_PAGE_COUNT) {nand_read_page(block, page, dst);
while (page < CONFIG_SYS_NAND_PAGE_COUNT &&size_remaining > 0) {if (size_remaining < CONFIG_SYS_NAND_PAGE_SIZE){nand_read_page(block, page,scratch_buf);memcpy(dst, scratch_buf,size_remaining);read_this_time = size_remaining;} else {nand_read_page(block, page, dst);} /* * When offs is not aligned to page address the * extra offset is copied to dst as well. Copy@@ -26,7 +45,8 @@ int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst) dst = (void *)((int)dst - page_offset); page_offset = 0; }
dst += CONFIG_SYS_NAND_PAGE_SIZE;
dst += read_this_time;size_remaining -= read_this_time; page++; }@@ -70,14 +90,6 @@ u32 nand_spl_adjust_offset(u32 sector, u32 offs) }
#ifdef CONFIG_SPL_UBI -/*
- Temporary storage for non NAND page aligned and non NAND page sized
- reads. Note: This does not support runtime detected FLASH yet, but
- that should be reasonably easy to fix by making the buffer large
- enough :)
- */
-static u8 scratch_buf[CONFIG_SYS_NAND_PAGE_SIZE];
/**
- nand_spl_read_block - Read data from physical eraseblock into a buffer
- @block: Number of the physical eraseblock
-- 2.25.1
Reviewed-by: Dario Binacchi dario.binacchi@amarulasolutions.com
Thanks and regards, Dario
Hi Colinn
On Fri, Nov 18, 2022 at 1:08 PM Dario Binacchi dario.binacchi@amarulasolutions.com wrote:
Hi Colin,
On Tue, Nov 15, 2022 at 5:35 PM Colin Foster colin.foster@in-advantage.com wrote:
The nand_spl_load_image function was guaranteed to read an entire block into RAM, regardless of how many bytes were to be read. This is particularly problematic when spl_load_legacy_image is called, as this function attempts to load a struct image_header but gets surprised with a full flash sector.
Allow partial reads to restore functionality to the code path spl_nand_load_element() -> spl_load_legacy_img() -> spl_nand_legacy_read(load, header, sizeof(hdr), header);
Signed-off-by: Colin Foster colin.foster@in-advantage.com
drivers/mtd/nand/raw/nand_spl_loaders.c | 36 ++++++++++++++++--------- 1 file changed, 24 insertions(+), 12 deletions(-)
diff --git a/drivers/mtd/nand/raw/nand_spl_loaders.c b/drivers/mtd/nand/raw/nand_spl_loaders.c index 4befc75c04..84ac482c06 100644 --- a/drivers/mtd/nand/raw/nand_spl_loaders.c +++ b/drivers/mtd/nand/raw/nand_spl_loaders.c @@ -1,9 +1,18 @@ +/*
- Temporary storage for non NAND page aligned and non NAND page sized
- reads. Note: This does not support runtime detected FLASH yet, but
- that should be reasonably easy to fix by making the buffer large
- enough :)
- */
+static u8 scratch_buf[CONFIG_SYS_NAND_PAGE_SIZE];
int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst) {
unsigned int read_this_time = CONFIG_SYS_NAND_PAGE_SIZE;unsigned int size_remaining = size; unsigned int block, lastblock; unsigned int page, page_offset;
/* offs has to be aligned to a page address! */ block = offs / CONFIG_SYS_NAND_BLOCK_SIZE; lastblock = (offs + size - 1) / CONFIG_SYS_NAND_BLOCK_SIZE; page = (offs % CONFIG_SYS_NAND_BLOCK_SIZE) / CONFIG_SYS_NAND_PAGE_SIZE;@@ -12,8 +21,18 @@ int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst) while (block <= lastblock) { if (!nand_is_bad_block(block)) { /* Skip bad blocks */
while (page < CONFIG_SYS_NAND_PAGE_COUNT) {nand_read_page(block, page, dst);
while (page < CONFIG_SYS_NAND_PAGE_COUNT &&size_remaining > 0) {if (size_remaining < CONFIG_SYS_NAND_PAGE_SIZE){nand_read_page(block, page,scratch_buf);memcpy(dst, scratch_buf,size_remaining);read_this_time = size_remaining;} else {nand_read_page(block, page, dst);} /* * When offs is not aligned to page address the * extra offset is copied to dst as well. Copy@@ -26,7 +45,8 @@ int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst) dst = (void *)((int)dst - page_offset); page_offset = 0; }
dst += CONFIG_SYS_NAND_PAGE_SIZE;
dst += read_this_time;size_remaining -= read_this_time; page++; }@@ -70,14 +90,6 @@ u32 nand_spl_adjust_offset(u32 sector, u32 offs) }
#ifdef CONFIG_SPL_UBI -/*
- Temporary storage for non NAND page aligned and non NAND page sized
- reads. Note: This does not support runtime detected FLASH yet, but
- that should be reasonably easy to fix by making the buffer large
- enough :)
- */
-static u8 scratch_buf[CONFIG_SYS_NAND_PAGE_SIZE];
/**
- nand_spl_read_block - Read data from physical eraseblock into a buffer
- @block: Number of the physical eraseblock
-- 2.25.1
Reviewed-by: Dario Binacchi dario.binacchi@amarulasolutions.com
Thanks and regards, Dario
This is the error reported by the CI/CD pipeline:
arm: + corvus 241+arm-linux-gnueabi-ld.bfd: u-boot-spl section `.bss' will not fit in region `.sdram' 242+arm-linux-gnueabi-ld.bfd: SPL image BSS too big 243+arm-linux-gnueabi-ld.bfd: region `.sdram' overflowed by 1388 bytes 244+make[2]: *** [scripts/Makefile.spl:527: spl/u-boot-spl] Error 1 245+make[1]: *** [Makefile:2074: spl/u-boot-spl] Error 2 246+make: *** [Makefile:177: sub-make] Error 2
I think the problem is:
+static u8 scratch_buf[CONFIG_SYS_NAND_PAGE_SIZE]; .+
So, please try with a malloc() and test the patch again. You can take as example the file drivers/mtd/nand/raw/mt7621_nand_spl.c.
Thanks and regards, Dario
--
Dario Binacchi
Embedded Linux Developer
dario.binacchi@amarulasolutions.com
Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310 info@amarulasolutions.com
www.amarulasolutions.com
On Tue, Nov 29, 2022 at 10:00:19AM +0100, Dario Binacchi wrote:
Hi Colinn
On Fri, Nov 18, 2022 at 1:08 PM Dario Binacchi dario.binacchi@amarulasolutions.com wrote:
Hi Colin,
On Tue, Nov 15, 2022 at 5:35 PM Colin Foster colin.foster@in-advantage.com wrote:
The nand_spl_load_image function was guaranteed to read an entire block into RAM, regardless of how many bytes were to be read. This is particularly problematic when spl_load_legacy_image is called, as this function attempts to load a struct image_header but gets surprised with a full flash sector.
Allow partial reads to restore functionality to the code path spl_nand_load_element() -> spl_load_legacy_img() -> spl_nand_legacy_read(load, header, sizeof(hdr), header);
Signed-off-by: Colin Foster colin.foster@in-advantage.com
drivers/mtd/nand/raw/nand_spl_loaders.c | 36 ++++++++++++++++--------- 1 file changed, 24 insertions(+), 12 deletions(-)
diff --git a/drivers/mtd/nand/raw/nand_spl_loaders.c b/drivers/mtd/nand/raw/nand_spl_loaders.c index 4befc75c04..84ac482c06 100644 --- a/drivers/mtd/nand/raw/nand_spl_loaders.c +++ b/drivers/mtd/nand/raw/nand_spl_loaders.c @@ -1,9 +1,18 @@ +/*
- Temporary storage for non NAND page aligned and non NAND page sized
- reads. Note: This does not support runtime detected FLASH yet, but
- that should be reasonably easy to fix by making the buffer large
- enough :)
- */
+static u8 scratch_buf[CONFIG_SYS_NAND_PAGE_SIZE];
int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst) {
unsigned int read_this_time = CONFIG_SYS_NAND_PAGE_SIZE;unsigned int size_remaining = size; unsigned int block, lastblock; unsigned int page, page_offset;
/* offs has to be aligned to a page address! */ block = offs / CONFIG_SYS_NAND_BLOCK_SIZE; lastblock = (offs + size - 1) / CONFIG_SYS_NAND_BLOCK_SIZE; page = (offs % CONFIG_SYS_NAND_BLOCK_SIZE) / CONFIG_SYS_NAND_PAGE_SIZE;@@ -12,8 +21,18 @@ int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst) while (block <= lastblock) { if (!nand_is_bad_block(block)) { /* Skip bad blocks */
while (page < CONFIG_SYS_NAND_PAGE_COUNT) {nand_read_page(block, page, dst);
while (page < CONFIG_SYS_NAND_PAGE_COUNT &&size_remaining > 0) {if (size_remaining < CONFIG_SYS_NAND_PAGE_SIZE){nand_read_page(block, page,scratch_buf);memcpy(dst, scratch_buf,size_remaining);read_this_time = size_remaining;} else {nand_read_page(block, page, dst);} /* * When offs is not aligned to page address the * extra offset is copied to dst as well. Copy@@ -26,7 +45,8 @@ int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst) dst = (void *)((int)dst - page_offset); page_offset = 0; }
dst += CONFIG_SYS_NAND_PAGE_SIZE;
dst += read_this_time;size_remaining -= read_this_time; page++; }@@ -70,14 +90,6 @@ u32 nand_spl_adjust_offset(u32 sector, u32 offs) }
#ifdef CONFIG_SPL_UBI -/*
- Temporary storage for non NAND page aligned and non NAND page sized
- reads. Note: This does not support runtime detected FLASH yet, but
- that should be reasonably easy to fix by making the buffer large
- enough :)
- */
-static u8 scratch_buf[CONFIG_SYS_NAND_PAGE_SIZE];
/**
- nand_spl_read_block - Read data from physical eraseblock into a buffer
- @block: Number of the physical eraseblock
-- 2.25.1
Reviewed-by: Dario Binacchi dario.binacchi@amarulasolutions.com
Thanks and regards, Dario
This is the error reported by the CI/CD pipeline:
arm: + corvus 241+arm-linux-gnueabi-ld.bfd: u-boot-spl section `.bss' will not fit in region `.sdram' 242+arm-linux-gnueabi-ld.bfd: SPL image BSS too big 243+arm-linux-gnueabi-ld.bfd: region `.sdram' overflowed by 1388 bytes 244+make[2]: *** [scripts/Makefile.spl:527: spl/u-boot-spl] Error 1 245+make[1]: *** [Makefile:2074: spl/u-boot-spl] Error 2 246+make: *** [Makefile:177: sub-make] Error 2
I think the problem is:
+static u8 scratch_buf[CONFIG_SYS_NAND_PAGE_SIZE]; .+
So, please try with a malloc() and test the patch again. You can take as example the file drivers/mtd/nand/raw/mt7621_nand_spl.c.
Hi Dario,
Thanks for all the info. I'll make this change and resubmit once I have time to test it.
Thanks and regards, Dario
--
Dario Binacchi
Embedded Linux Developer
dario.binacchi@amarulasolutions.com
Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310 info@amarulasolutions.com
www.amarulasolutions.com
--
Dario Binacchi
Embedded Linux Developer
dario.binacchi@amarulasolutions.com
Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310 info@amarulasolutions.com
www.amarulasolutions.com
participants (2)
-
Colin Foster -
Dario Binacchi