m68k: disable SRAM at startup

[PATCH] m68k: disable SRAM at startup

Posted by Jean-Michel Hautbois 1 year, 3 months ago

Some of the internal SoC registers have a higher priority over the MMU
virtual mappings. The SRAM bank is one of them. If the bootloader
enables the internal SRAM at address 0x80000000, virtual memory access
at this address will not hit the MMU - so no TLB data misses would
occurr.

Since 0x80000000 is the virtual start address of all applications that
bit of memory is getting stomped over with inconsistent code and data
access.

Fix it by disabling the internal SRAM at startup.

Signed-off-by: Greg Ungerer <gerg@linux-m68k.org>
Tested-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
---
 arch/m68k/coldfire/head.S | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/m68k/coldfire/head.S b/arch/m68k/coldfire/head.S
index c6d7fd28c6023..3901a49c47c89 100644
--- a/arch/m68k/coldfire/head.S
+++ b/arch/m68k/coldfire/head.S
@@ -207,6 +207,10 @@ _start:
 	movec	%d0,%CACR
 	nop
 
+	movel   #0,%d0
+	movec   %d0,%rambar
+	nop
+
 #ifdef CONFIG_MMU
 	/*
 	 *	Identity mapping for the kernel region.

---
base-commit: 431c1646e1f86b949fa3685efc50b660a364c2b6
change-id: 20240904-fix-cf-virt-mem-sram-abadb27fff2f

Best regards,
-- 
Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>

Re: [PATCH] m68k: disable SRAM at startup

Posted by Greg Ungerer 1 year, 3 months ago

Hi JM,

On 5/9/24 00:26, Jean-Michel Hautbois wrote:
> Some of the internal SoC registers have a higher priority over the MMU
> virtual mappings. The SRAM bank is one of them. If the bootloader
> enables the internal SRAM at address 0x80000000, virtual memory access
> at this address will not hit the MMU - so no TLB data misses would
> occurr.
> 
> Since 0x80000000 is the virtual start address of all applications that
> bit of memory is getting stomped over with inconsistent code and data
> access.
> 
> Fix it by disabling the internal SRAM at startup.
> 
> Signed-off-by: Greg Ungerer <gerg@linux-m68k.org>
> Tested-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>

I know this change fixed your specific problem, but it is not going
to work as a general solution.

For one not all ColdFire parts have an SRAM region and thus not all have
a valid %rambar register. Secondly some ColdFire parts (like the 5249)
have 2 SRAM regions, with mapping control registers named %rambar0
and %rambar1.

Some ColdFire uClinux applications use the mapped SRAM, so a blanket
disable is not the best idea in any case.

I am thinking it would be better to have a new Kconfig option that
allows disabling by the startup code for those ColdFire parts that
have SRAM (so appropriate "depends on"). That way it will only be
disable when that is what is needed. Maybe also have that configuration
allowing to configure and set the rambars so that a mapping
can be forced on if wanted.

Regards
Greg

> ---
>   arch/m68k/coldfire/head.S | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/arch/m68k/coldfire/head.S b/arch/m68k/coldfire/head.S
> index c6d7fd28c6023..3901a49c47c89 100644
> --- a/arch/m68k/coldfire/head.S
> +++ b/arch/m68k/coldfire/head.S
> @@ -207,6 +207,10 @@ _start:
>   	movec	%d0,%CACR
>   	nop
>   
> +	movel   #0,%d0
> +	movec   %d0,%rambar
> +	nop
> +
>   #ifdef CONFIG_MMU
>   	/*
>   	 *	Identity mapping for the kernel region.
> 
> ---
> base-commit: 431c1646e1f86b949fa3685efc50b660a364c2b6
> change-id: 20240904-fix-cf-virt-mem-sram-abadb27fff2f
> 
> Best regards,

Re: [PATCH] m68k: disable SRAM at startup

Posted by Jean-Michel Hautbois 1 year, 3 months ago

Hi Greg,

On 05/09/2024 09:52, Greg Ungerer wrote:
> Hi JM,
> 
> On 5/9/24 00:26, Jean-Michel Hautbois wrote:
>> Some of the internal SoC registers have a higher priority over the MMU
>> virtual mappings. The SRAM bank is one of them. If the bootloader
>> enables the internal SRAM at address 0x80000000, virtual memory access
>> at this address will not hit the MMU - so no TLB data misses would
>> occurr.
>>
>> Since 0x80000000 is the virtual start address of all applications that
>> bit of memory is getting stomped over with inconsistent code and data
>> access.
>>
>> Fix it by disabling the internal SRAM at startup.
>>
>> Signed-off-by: Greg Ungerer <gerg@linux-m68k.org>
>> Tested-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
> 
> I know this change fixed your specific problem, but it is not going
> to work as a general solution.
> 
> For one not all ColdFire parts have an SRAM region and thus not all have
> a valid %rambar register. Secondly some ColdFire parts (like the 5249)
> have 2 SRAM regions, with mapping control registers named %rambar0
> and %rambar1.

Thanks for this clarification. TBH, I sent the mail and thought about it 
after :-(.
The patch is in my branch, and solves the issues in [1], so I decided to 
send it.
*But* I should have asked first your approval as you are the one who 
found it and mark it as RFC probably. I'm sorry for that...

[1]: 
https://lore.kernel.org/linux-m68k/mvmed8dlewg.fsf@linux-m68k.org/T/#m0e4c8f0b9df0a03f3d0b5cf2246661d0bb3a370b

> 
> Some ColdFire uClinux applications use the mapped SRAM, so a blanket
> disable is not the best idea in any case.
> 
> I am thinking it would be better to have a new Kconfig option that
> allows disabling by the startup code for those ColdFire parts that
> have SRAM (so appropriate "depends on"). That way it will only be
> disable when that is what is needed. Maybe also have that configuration
> allowing to configure and set the rambars so that a mapping
> can be forced on if wanted.

Interesting.
I will keep this patch for my own case for now, as it does not apply and 
wait for this Kconfig option to make it clean !

JM

Re: [PATCH] m68k: disable SRAM at startup

Posted by Philippe De Muyter 1 year, 3 months ago

Hello Jean-Michel and Greg,

Please add "coldfire' in the title and the explanation of this patch.

Best regards

Philippe

On Wed, Sep 04, 2024 at 04:26:14PM +0200, Jean-Michel Hautbois wrote:
> Some of the internal SoC registers have a higher priority over the MMU
> virtual mappings. The SRAM bank is one of them. If the bootloader
> enables the internal SRAM at address 0x80000000, virtual memory access
> at this address will not hit the MMU - so no TLB data misses would
> occurr.
> 
> Since 0x80000000 is the virtual start address of all applications that
> bit of memory is getting stomped over with inconsistent code and data
> access.
> 
> Fix it by disabling the internal SRAM at startup.
> 
> Signed-off-by: Greg Ungerer <gerg@linux-m68k.org>
> Tested-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
> ---
>  arch/m68k/coldfire/head.S | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/m68k/coldfire/head.S b/arch/m68k/coldfire/head.S
> index c6d7fd28c6023..3901a49c47c89 100644
> --- a/arch/m68k/coldfire/head.S
> +++ b/arch/m68k/coldfire/head.S
> @@ -207,6 +207,10 @@ _start:
>  	movec	%d0,%CACR
>  	nop
>  
> +	movel   #0,%d0
> +	movec   %d0,%rambar
> +	nop
> +
>  #ifdef CONFIG_MMU
>  	/*
>  	 *	Identity mapping for the kernel region.
> 
> ---
> base-commit: 431c1646e1f86b949fa3685efc50b660a364c2b6
> change-id: 20240904-fix-cf-virt-mem-sram-abadb27fff2f
> 
> Best regards,
> -- 
> Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
>