[U-Boot] [PATCH v2] nand: Fix cache and memory inconsistent issue

we load the secondary stage u-boot image from NAND to system memory by nand_load, we have not flush data cache to memory, not invalidate instruction cache before we jump to RAM. when the system is cache enable and the TLB/page attribute of system memory is cacheable, it will cause issue.
- 83xx family is using the dcache lock, so all of dcache access is cache-inhibited. so you can't see the issue. - 85xx family is using dcache, icache enable, partial cache lock. you will see the issue.
The patch fix the cache issue.
Signed-off-by: Dave Liu daveliu@freescale.com --- nand_spl/nand_boot_fsl_elbc.c | 35 +++++++++++++++++++++++++++++++++++ 1 files changed, 35 insertions(+), 0 deletions(-)
diff --git a/nand_spl/nand_boot_fsl_elbc.c b/nand_spl/nand_boot_fsl_elbc.c index 4a961ea..b0ab734 100644 --- a/nand_spl/nand_boot_fsl_elbc.c +++ b/nand_spl/nand_boot_fsl_elbc.c @@ -27,6 +27,7 @@ #include <asm/io.h> #include <asm/immap_83xx.h> #include <asm/fsl_lbc.h> +#include <asm/cache.h> #include <linux/mtd/nand.h>
#define WINDOW_SIZE 8192 @@ -125,6 +126,33 @@ static void nand_load(unsigned int offs, int uboot_size, uchar *dst) }
/* + * clean the dcache, invalidate the icache + * for powerpc architecture + */ +static void __flush_cache(ulong start, ulong size) +{ + ulong addr, end; + ulong cache_line = CONFIG_SYS_CACHELINE_SIZE; + + end = start + size; + + /* clean the dcache, make sure all of data to memory */ + for (addr = start; addr < end; addr += cache_line) + asm ("dcbst 0,%0": :"r" (addr)); + + /* wait for all dcbst to complete on bus */ + asm ("sync"); + + /* invalidate icache */ + for (addr = start; addr < end; addr += cache_line) + asm ("icbi 0,%0": :"r" (addr)); + + asm ("sync"); + /* flush prefetch queue in any case */ + asm ("isync"); +} + +/* * The main entry for NAND booting. It's necessary that SDRAM is already * configured and available since this code loads the main U-Boot image * from NAND into SDRAM and starts it from there. @@ -143,6 +171,13 @@ void nand_boot(void) * Jump to U-Boot image */ puts("transfering control\n"); + /* + * We need clean dcache and invalidate + * to sync between icache and dcache + * before jump to RAM. make sure all of + * NAND data write to memory. + */ + __flush_cache(CONFIG_SYS_NAND_U_BOOT_DST, CONFIG_SYS_NAND_U_BOOT_SIZE); uboot = (void *)CONFIG_SYS_NAND_U_BOOT_START; uboot(); }

On Fri, Nov 28, 2008 at 08:16:28PM +0800, Dave Liu wrote:
+static void __flush_cache(ulong start, ulong size)
No gratuitous underscores.
+{
- ulong addr, end;
- ulong cache_line = CONFIG_SYS_CACHELINE_SIZE;
- end = start + size;
- /* clean the dcache, make sure all of data to memory */
- for (addr = start; addr < end; addr += cache_line)
asm ("dcbst 0,%0": :"r" (addr));
If (start % cache_line) > (end % cache_line), then you could miss flushing the last cache line.
Make the asm volatile, with a memory clobber, and preferably with spaces around the colons.
Please factor this out into arch code, and make it shareable with other NAND code (such as nand_boot.c).
- /*
* We need clean dcache and invalidate
* to sync between icache and dcache
* before jump to RAM. make sure all of
* NAND data write to memory.
"Clean d-cache and invalidate i-cache, to make sure that no stale data is executed."
-Scott

On Monday 01 December 2008, Scott Wood wrote:
Please factor this out into arch code, and make it shareable with other NAND code (such as nand_boot.c).
Yes, please.
- /*
* We need clean dcache and invalidate
* to sync between icache and dcache
* before jump to RAM. make sure all of
* NAND data write to memory.
"Clean d-cache and invalidate i-cache, to make sure that no stale data is executed."
And the function name is not perfectly fitting for my taste. Why not extract 2 functions, flush_dcache_range() and invalidate_icache_range(). This reminds me that some architectures/platforms already have those functions defined (PPC4xx at least does). But the 2nd parameter is not size but end ("stolen" from the Linux kernel). Perhaps it makes sense to use these functions here as well.
Best regards, Stefan
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================

From: Stefan Roese [mailto:sr@denx.de] On Monday 01 December 2008, Scott Wood wrote:
Please factor this out into arch code, and make it
shareable with other
NAND code (such as nand_boot.c).
Yes, please.
- /*
* We need clean dcache and invalidate
* to sync between icache and dcache
* before jump to RAM. make sure all of
* NAND data write to memory.
"Clean d-cache and invalidate i-cache, to make sure that no stale data is executed."
And the function name is not perfectly fitting for my taste. Why not extract 2 functions, flush_dcache_range() and invalidate_icache_range(). This reminds me that some architectures/platforms already have those functions defined (PPC4xx at least does). But the 2nd parameter is not size but end ("stolen" from the Linux kernel). Perhaps it makes sense to use these functions here as well.
No architecutres/plaforms, I expect we can put all of cache-specificed functions into lib_ppc/cache.S or lib_ppc/cache.c.
How about this?
participants (4)
-
Dave Liu
-
Liu Dave
-
Scott Wood
-
Stefan Roese