[PATCH] Avoid SDRAM access crash

[PATCH] Avoid SDRAM access crash

[steve]

From: Steve Schefter <steve@scheftech.com>

When remapping the SDRAM with the MMU enabled, we need to ensure
that the translation tables are not still in cache before
invalidating the TLB.  Failure to do so will result in the following
crash approximately 50% of the time:

booting kernel from /dev/nand0.kernel.bb
   Image Name:   Linux-3.3.0PD12.0.0
   OS:           Linux
   Architecture: ARM
   Type:         Kernel Image
   Compression:  uncompressed
   Data Size:    3384824 Bytes =  3.2 MB
   Load Address: 80008000
   Entry Point:  80008000
unable to handle paging request at address 0x80028000
pc : [<8f01f280>]    lr : [<8f005330>]
sp : 8cfff9b0  ip : 0000003f  fp : 00000000
r10: 00001000  r9 : 00000000  r8 : 8d2a8f70
r7 : 8f043818  r6 : 0033a5f8  r5 : 8f04381c  r4 : 00001000
r3 : 80028000  r2 : 00000fff  r1 : 8d2a8f71  r0 : 80028000
Flags: nzCv  IRQs off  FIQs off  Mode SVC_32
[<8f01f280>] (memcpy+0x18/0x20) from [<8f005330>] (uimage_sdram_flush+0x90/0xb8)
[<8f005330>] (uimage_sdram_flush+0x90/0xb8) from [<8f0053a4>] (uncompress_copy+0x4c/0x74)

Signed-off-by: Steve Schefter <steve@scheftech.com>
---
 arch/arm/cpu/mmu.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index c19f931..4ff0430 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -165,6 +165,12 @@ static int arm_mmu_remap_sdram(struct memory_bank *bank)
 		pte += 256;
 	}
 
+	asm volatile (
+		"bl __mmu_cache_flush;"
+		:
+		:
+		: "r0", "r1", "r2", "r3", "r6", "r10", "r12", "lr", "cc", "memory"
+	);
 	tlb_invalidate();
 
 	return 0;
-- 
1.7.4.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Avoid SDRAM access crash

[Sascha Hauer]

Hi Steve,

On Mon, Jul 02, 2012 at 01:22:33PM -0400, steve@scheftech.com wrote:
> From: Steve Schefter <steve@scheftech.com>
> 
> When remapping the SDRAM with the MMU enabled, we need to ensure
> that the translation tables are not still in cache before
> invalidating the TLB.  Failure to do so will result in the following
> crash approximately 50% of the time:
> 
> booting kernel from /dev/nand0.kernel.bb
>    Image Name:   Linux-3.3.0PD12.0.0
>    OS:           Linux
>    Architecture: ARM
>    Type:         Kernel Image
>    Compression:  uncompressed
>    Data Size:    3384824 Bytes =  3.2 MB
>    Load Address: 80008000
>    Entry Point:  80008000
> unable to handle paging request at address 0x80028000
> pc : [<8f01f280>]    lr : [<8f005330>]
> sp : 8cfff9b0  ip : 0000003f  fp : 00000000
> r10: 00001000  r9 : 00000000  r8 : 8d2a8f70
> r7 : 8f043818  r6 : 0033a5f8  r5 : 8f04381c  r4 : 00001000
> r3 : 80028000  r2 : 00000fff  r1 : 8d2a8f71  r0 : 80028000
> Flags: nzCv  IRQs off  FIQs off  Mode SVC_32
> [<8f01f280>] (memcpy+0x18/0x20) from [<8f005330>] (uimage_sdram_flush+0x90/0xb8)
> [<8f005330>] (uimage_sdram_flush+0x90/0xb8) from [<8f0053a4>] (uncompress_copy+0x4c/0x74)

I wonder why this has never hit me. On what hardware did you see this?

Does the following patch solve your problem aswell?

8<----------------------------------------------------

ARM mmu: flush page tables in arm_mmu_remap_sdram()

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/cpu/mmu.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index 55b07a4..607f357 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -147,7 +147,7 @@ static int arm_mmu_remap_sdram(struct memory_bank *bank)
 	if ((phys & (SZ_1M - 1)) || (bank->size & (SZ_1M - 1)))
 		return -EINVAL;
 
-	ptes = memalign(0x400, num_ptes * sizeof(u32));
+	ptes = memalign(PAGE_SIZE, num_ptes * sizeof(u32));
 
 	debug("ptes: 0x%p ttb_start: 0x%08lx ttb_end: 0x%08lx\n",
 			ptes, ttb_start, ttb_end);
@@ -165,6 +165,9 @@ static int arm_mmu_remap_sdram(struct memory_bank *bank)
 		pte += 256;
 	}
 
+	dma_flush_range((unsigned long)ttb, (unsigned long)ttb + 0x4000);
+	dma_flush_range((unsigned long)ptes, num_ptes * sizeof(u32));
+
 	tlb_invalidate();
 
 	return 0;
-- 
1.7.10


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Avoid SDRAM access crash

[Steve Schefter]

Hi Sascha.

> I wonder why this has never hit me. On what hardware did you see this?

Me too.  All I can say is that the timing is tight.  I see crashes at 
various DRAM addresses, all depending on how much was left in cache when 
the tlb invalidate was done.

I'm using the Phytec phyCORE-OMAP44xx card.

> Does the following patch solve your problem aswell?

It does.  I see that routine contains a cache flush as well.

Regards,
	Steve

>
> 8<----------------------------------------------------
>
> ARM mmu: flush page tables in arm_mmu_remap_sdram()
>
> Signed-off-by: Sascha Hauer<s.hauer@pengutronix.de>
> ---
>   arch/arm/cpu/mmu.c |    5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
> index 55b07a4..607f357 100644
> --- a/arch/arm/cpu/mmu.c
> +++ b/arch/arm/cpu/mmu.c
> @@ -147,7 +147,7 @@ static int arm_mmu_remap_sdram(struct memory_bank *bank)
>   	if ((phys&  (SZ_1M - 1)) || (bank->size&  (SZ_1M - 1)))
>   		return -EINVAL;
>
> -	ptes = memalign(0x400, num_ptes * sizeof(u32));
> +	ptes = memalign(PAGE_SIZE, num_ptes * sizeof(u32));
>
>   	debug("ptes: 0x%p ttb_start: 0x%08lx ttb_end: 0x%08lx\n",
>   			ptes, ttb_start, ttb_end);
> @@ -165,6 +165,9 @@ static int arm_mmu_remap_sdram(struct memory_bank *bank)
>   		pte += 256;
>   	}
>
> +	dma_flush_range((unsigned long)ttb, (unsigned long)ttb + 0x4000);
> +	dma_flush_range((unsigned long)ptes, num_ptes * sizeof(u32));
> +
>   	tlb_invalidate();
>
>   	return 0;

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Avoid SDRAM access crash

[Sascha Hauer]

On Tue, Jul 03, 2012 at 07:22:57AM -0400, Steve Schefter wrote:
> Hi Sascha.
> 
> >I wonder why this has never hit me. On what hardware did you see this?
> 
> Me too.  All I can say is that the timing is tight.  I see crashes
> at various DRAM addresses, all depending on how much was left in
> cache when the tlb invalidate was done.
> 
> I'm using the Phytec phyCORE-OMAP44xx card.
> 
> >Does the following patch solve your problem aswell?
> 
> It does.  I see that routine contains a cache flush as well.

Ok, thanks for testing. I applied my version because it flushes only
the relevant parts of the cache and looks more like C ;)

Sascha

> 
> Regards,
> 	Steve
> 
> >
> >8<----------------------------------------------------
> >
> >ARM mmu: flush page tables in arm_mmu_remap_sdram()
> >
> >Signed-off-by: Sascha Hauer<s.hauer@pengutronix.de>
> >---
> >  arch/arm/cpu/mmu.c |    5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> >diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
> >index 55b07a4..607f357 100644
> >--- a/arch/arm/cpu/mmu.c
> >+++ b/arch/arm/cpu/mmu.c
> >@@ -147,7 +147,7 @@ static int arm_mmu_remap_sdram(struct memory_bank *bank)
> >  	if ((phys&  (SZ_1M - 1)) || (bank->size&  (SZ_1M - 1)))
> >  		return -EINVAL;
> >
> >-	ptes = memalign(0x400, num_ptes * sizeof(u32));
> >+	ptes = memalign(PAGE_SIZE, num_ptes * sizeof(u32));
> >
> >  	debug("ptes: 0x%p ttb_start: 0x%08lx ttb_end: 0x%08lx\n",
> >  			ptes, ttb_start, ttb_end);
> >@@ -165,6 +165,9 @@ static int arm_mmu_remap_sdram(struct memory_bank *bank)
> >  		pte += 256;
> >  	}
> >
> >+	dma_flush_range((unsigned long)ttb, (unsigned long)ttb + 0x4000);
> >+	dma_flush_range((unsigned long)ptes, num_ptes * sizeof(u32));
> >+
> >  	tlb_invalidate();
> >
> >  	return 0;
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Avoid SDRAM access crash

[Steve Schefter]

On 7/4/2012 2:51 AM, Sascha Hauer wrote:
>  I applied my version because it flushes only
> the relevant parts of the cache and looks more like C ;)

That's fine by me.  Thanks Sascha.

Regards,
	Steve

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox