[U-Boot] mach-stm32: Fix mpu region's attribute for STM32H7

Message ID 1510590378-6510-1-git-send-email-patrice.chotard@st.com
State Rejected
Delegated to: Tom Rini
Headers show
Series
  • [U-Boot] mach-stm32: Fix mpu region's attribute for STM32H7
Related show

Commit Message

Patrice CHOTARD Nov. 13, 2017, 4:26 p.m.
From: Patrice Chotard <patrice.chotard@st.com>

The SDRAM region was setup with the wrong attributes.
It must be set to :
  _ XN_EN (Execution of an instruction fetched from this region permitted)
  _ O_I_WB_RD_WR_ALLOC (Outer and inner write-back, write and read allocate)

This fixes hard fault when trying to load and execute kernel linux
in this area.

Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
---
 arch/arm/mach-stm32/stm32h7/soc.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Vikas Manocha Nov. 14, 2017, 2:16 a.m. | #1
Hi Patrice,

> -----Original Message-----
> From: Patrice CHOTARD
> Sent: Monday, November 13, 2017 8:26 AM
> To: u-boot@lists.denx.de; albert.u.boot@aribaud.net; sjg@chromium.org; Vikas MANOCHA <vikas.manocha@st.com>
> Cc: Patrice CHOTARD <patrice.chotard@st.com>; Patrick DELAUNAY <patrick.delaunay@st.com>; Christophe KERELLO
> <christophe.kerello@st.com>
> Subject: [PATCH] mach-stm32: Fix mpu region's attribute for STM32H7
> 
> From: Patrice Chotard <patrice.chotard@st.com>
> 
> The SDRAM region was setup with the wrong attributes.
> It must be set to :
>   _ XN_EN (Execution of an instruction fetched from this region permitted)
>   _ O_I_WB_RD_WR_ALLOC (Outer and inner write-back, write and read allocate)
> 

H7 mpu configuration seems same as F7, can we have one config for F7 & H7.

> This fixes hard fault when trying to load and execute kernel linux in this area.
> 
> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>

In any case,
Reviewed-by: Vikas Manocha <vikas.manocha@st.com>

Cheers,
Vikas

> ---
>  arch/arm/mach-stm32/stm32h7/soc.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mach-stm32/stm32h7/soc.c b/arch/arm/mach-stm32/stm32h7/soc.c
> index 692dbcc..e0d3f11 100644
> --- a/arch/arm/mach-stm32/stm32h7/soc.c
> +++ b/arch/arm/mach-stm32/stm32h7/soc.c
> @@ -30,9 +30,12 @@ int arch_cpu_init(void)
>  		{ 0x00000000, REGION_0, XN_DIS, PRIV_RW_USR_RW,
>  		O_I_WB_RD_WR_ALLOC, REGION_4GB },
> 
> -		/* Code area, executable & strongly ordered */
> -		{ 0xD0000000, REGION_1, XN_EN, PRIV_RW_USR_RW,
> -		STRONG_ORDER, REGION_8MB },
> +		/*
> +		 * Code area, executable, Outer and inner write-back,
> +		 * no write allocate
> +		 */
> +		{ 0xD0000000, REGION_1, XN_DIS, PRIV_RW_USR_RW,
> +		O_I_WB_RD_WR_ALLOC, REGION_32MB },
> 
>  		/* Device area in all H7 : Not executable */
>  		{ 0x40000000, REGION_2, XN_EN, PRIV_RW_USR_RW,
> --
> 1.9.1
Patrice CHOTARD Nov. 14, 2017, 8:41 a.m. | #2
Hi Vikas

On 11/14/2017 03:16 AM, Vikas MANOCHA wrote:
> Hi Patrice,

> 

>> -----Original Message-----

>> From: Patrice CHOTARD

>> Sent: Monday, November 13, 2017 8:26 AM

>> To: u-boot@lists.denx.de; albert.u.boot@aribaud.net; sjg@chromium.org; Vikas MANOCHA <vikas.manocha@st.com>

>> Cc: Patrice CHOTARD <patrice.chotard@st.com>; Patrick DELAUNAY <patrick.delaunay@st.com>; Christophe KERELLO

>> <christophe.kerello@st.com>

>> Subject: [PATCH] mach-stm32: Fix mpu region's attribute for STM32H7

>>

>> From: Patrice Chotard <patrice.chotard@st.com>

>>

>> The SDRAM region was setup with the wrong attributes.

>> It must be set to :

>>    _ XN_EN (Execution of an instruction fetched from this region permitted)

>>    _ O_I_WB_RD_WR_ALLOC (Outer and inner write-back, write and read allocate)

>>

> 

> H7 mpu configuration seems same as F7, can we have one config for F7 & H7.


Between F7 and H7, there is one difference regarding the F7's region 3 
which is not needed on H7 because FMC/QSPI registers are located inside 
H7's region 2.

Just one question about F7's region 1, why is it only 512MB and not 1GB 
long, to include ITCM and DTCM area ? (see Embedded SRAM chapter of 
RM0385 Reference manual)

Nevertheless, i just notice that for H7, i can remove region 1 which 
overlaps region 0 with exactly the same attribute.

Thanks

> 

>> This fixes hard fault when trying to load and execute kernel linux in this area.

>>

>> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>

> 

> In any case,

> Reviewed-by: Vikas Manocha <vikas.manocha@st.com>

> 

> Cheers,

> Vikas

> 

>> ---

>>   arch/arm/mach-stm32/stm32h7/soc.c | 9 ++++++---

>>   1 file changed, 6 insertions(+), 3 deletions(-)

>>

>> diff --git a/arch/arm/mach-stm32/stm32h7/soc.c b/arch/arm/mach-stm32/stm32h7/soc.c

>> index 692dbcc..e0d3f11 100644

>> --- a/arch/arm/mach-stm32/stm32h7/soc.c

>> +++ b/arch/arm/mach-stm32/stm32h7/soc.c

>> @@ -30,9 +30,12 @@ int arch_cpu_init(void)

>>   		{ 0x00000000, REGION_0, XN_DIS, PRIV_RW_USR_RW,

>>   		O_I_WB_RD_WR_ALLOC, REGION_4GB },

>>

>> -		/* Code area, executable & strongly ordered */

>> -		{ 0xD0000000, REGION_1, XN_EN, PRIV_RW_USR_RW,

>> -		STRONG_ORDER, REGION_8MB },

>> +		/*

>> +		 * Code area, executable, Outer and inner write-back,

>> +		 * no write allocate

>> +		 */

>> +		{ 0xD0000000, REGION_1, XN_DIS, PRIV_RW_USR_RW,

>> +		O_I_WB_RD_WR_ALLOC, REGION_32MB },

>>

>>   		/* Device area in all H7 : Not executable */

>>   		{ 0x40000000, REGION_2, XN_EN, PRIV_RW_USR_RW,

>> --

>> 1.9.1

>
Vikas Manocha Nov. 14, 2017, 10:16 p.m. | #3
Hi Patrice,

Cheers,
Vikas

> -----Original Message-----

> From: Patrice CHOTARD

> Sent: Tuesday, November 14, 2017 12:41 AM

> To: Vikas MANOCHA <vikas.manocha@st.com>; u-boot@lists.denx.de; albert.u.boot@aribaud.net; sjg@chromium.org

> Cc: Patrick DELAUNAY <patrick.delaunay@st.com>; Christophe KERELLO <christophe.kerello@st.com>

> Subject: Re: [PATCH] mach-stm32: Fix mpu region's attribute for STM32H7

> 

> Hi Vikas

> 

> On 11/14/2017 03:16 AM, Vikas MANOCHA wrote:

> > Hi Patrice,

> >

> >> -----Original Message-----

> >> From: Patrice CHOTARD

> >> Sent: Monday, November 13, 2017 8:26 AM

> >> To: u-boot@lists.denx.de; albert.u.boot@aribaud.net;

> >> sjg@chromium.org; Vikas MANOCHA <vikas.manocha@st.com>

> >> Cc: Patrice CHOTARD <patrice.chotard@st.com>; Patrick DELAUNAY

> >> <patrick.delaunay@st.com>; Christophe KERELLO

> >> <christophe.kerello@st.com>

> >> Subject: [PATCH] mach-stm32: Fix mpu region's attribute for STM32H7

> >>

> >> From: Patrice Chotard <patrice.chotard@st.com>

> >>

> >> The SDRAM region was setup with the wrong attributes.

> >> It must be set to :

> >>    _ XN_EN (Execution of an instruction fetched from this region permitted)

> >>    _ O_I_WB_RD_WR_ALLOC (Outer and inner write-back, write and read

> >> allocate)

> >>

> >

> > H7 mpu configuration seems same as F7, can we have one config for F7 & H7.

> 

> Between F7 and H7, there is one difference regarding the F7's region 3 which is not needed on H7 because FMC/QSPI registers are

> located inside H7's region 2.


Yes, that’s correct. Any way we can handle it at one place.
In any case, for easy maintenance let's keep the same configuration/code for two with this above exception.

> 

> Just one question about F7's region 1, why is it only 512MB and not 1GB long, to include ITCM and DTCM area ? (see Embedded SRAM

> chapter of

> RM0385 Reference manual)


because 512MB (0x2000_0000) is size of armv7m code area.

Cheers,
Vikas

> 

> Nevertheless, i just notice that for H7, i can remove region 1 which overlaps region 0 with exactly the same attribute.

> 

> Thanks

> 

> >

> >> This fixes hard fault when trying to load and execute kernel linux in this area.

> >>

> >> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>

> >

> > In any case,

> > Reviewed-by: Vikas Manocha <vikas.manocha@st.com>

> >

> > Cheers,

> > Vikas

> >

> >> ---

> >>   arch/arm/mach-stm32/stm32h7/soc.c | 9 ++++++---

> >>   1 file changed, 6 insertions(+), 3 deletions(-)

> >>

> >> diff --git a/arch/arm/mach-stm32/stm32h7/soc.c

> >> b/arch/arm/mach-stm32/stm32h7/soc.c

> >> index 692dbcc..e0d3f11 100644

> >> --- a/arch/arm/mach-stm32/stm32h7/soc.c

> >> +++ b/arch/arm/mach-stm32/stm32h7/soc.c

> >> @@ -30,9 +30,12 @@ int arch_cpu_init(void)

> >>   		{ 0x00000000, REGION_0, XN_DIS, PRIV_RW_USR_RW,

> >>   		O_I_WB_RD_WR_ALLOC, REGION_4GB },

> >>

> >> -		/* Code area, executable & strongly ordered */

> >> -		{ 0xD0000000, REGION_1, XN_EN, PRIV_RW_USR_RW,

> >> -		STRONG_ORDER, REGION_8MB },

> >> +		/*

> >> +		 * Code area, executable, Outer and inner write-back,

> >> +		 * no write allocate

> >> +		 */

> >> +		{ 0xD0000000, REGION_1, XN_DIS, PRIV_RW_USR_RW,

> >> +		O_I_WB_RD_WR_ALLOC, REGION_32MB },

> >>

> >>   		/* Device area in all H7 : Not executable */

> >>   		{ 0x40000000, REGION_2, XN_EN, PRIV_RW_USR_RW,

> >> --

> >> 1.9.1

> >
Patrice CHOTARD Nov. 15, 2017, 10:10 a.m. | #4
Hi Vikas

On 11/14/2017 11:16 PM, Vikas MANOCHA wrote:
> Hi Patrice,

> 

> Cheers,

> Vikas

> 

>> -----Original Message-----

>> From: Patrice CHOTARD

>> Sent: Tuesday, November 14, 2017 12:41 AM

>> To: Vikas MANOCHA <vikas.manocha@st.com>; u-boot@lists.denx.de; albert.u.boot@aribaud.net; sjg@chromium.org

>> Cc: Patrick DELAUNAY <patrick.delaunay@st.com>; Christophe KERELLO <christophe.kerello@st.com>

>> Subject: Re: [PATCH] mach-stm32: Fix mpu region's attribute for STM32H7

>>

>> Hi Vikas

>>

>> On 11/14/2017 03:16 AM, Vikas MANOCHA wrote:

>>> Hi Patrice,

>>>

>>>> -----Original Message-----

>>>> From: Patrice CHOTARD

>>>> Sent: Monday, November 13, 2017 8:26 AM

>>>> To: u-boot@lists.denx.de; albert.u.boot@aribaud.net;

>>>> sjg@chromium.org; Vikas MANOCHA <vikas.manocha@st.com>

>>>> Cc: Patrice CHOTARD <patrice.chotard@st.com>; Patrick DELAUNAY

>>>> <patrick.delaunay@st.com>; Christophe KERELLO

>>>> <christophe.kerello@st.com>

>>>> Subject: [PATCH] mach-stm32: Fix mpu region's attribute for STM32H7

>>>>

>>>> From: Patrice Chotard <patrice.chotard@st.com>

>>>>

>>>> The SDRAM region was setup with the wrong attributes.

>>>> It must be set to :

>>>>     _ XN_EN (Execution of an instruction fetched from this region permitted)

>>>>     _ O_I_WB_RD_WR_ALLOC (Outer and inner write-back, write and read

>>>> allocate)

>>>>

>>>

>>> H7 mpu configuration seems same as F7, can we have one config for F7 & H7.

>>

>> Between F7 and H7, there is one difference regarding the F7's region 3 which is not needed on H7 because FMC/QSPI registers are

>> located inside H7's region 2.

> 

> Yes, that’s correct. Any way we can handle it at one place.

> In any case, for easy maintenance let's keep the same configuration/code for two with this above exception.


Agree, i will abandon this patch and will propose a new one which will 
factorize code between STM32F4/F7 and H7 SoCs family.

> 

>>

>> Just one question about F7's region 1, why is it only 512MB and not 1GB long, to include ITCM and DTCM area ? (see Embedded SRAM

>> chapter of

>> RM0385 Reference manual)

> 

> because 512MB (0x2000_0000) is size of armv7m code area.


Ok

Thanks

Patrice

> 

> Cheers,

> Vikas

> 

>>

>> Nevertheless, i just notice that for H7, i can remove region 1 which overlaps region 0 with exactly the same attribute.

>>

>> Thanks

>>

>>>

>>>> This fixes hard fault when trying to load and execute kernel linux in this area.

>>>>

>>>> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>

>>>

>>> In any case,

>>> Reviewed-by: Vikas Manocha <vikas.manocha@st.com>

>>>

>>> Cheers,

>>> Vikas

>>>

>>>> ---

>>>>    arch/arm/mach-stm32/stm32h7/soc.c | 9 ++++++---

>>>>    1 file changed, 6 insertions(+), 3 deletions(-)

>>>>

>>>> diff --git a/arch/arm/mach-stm32/stm32h7/soc.c

>>>> b/arch/arm/mach-stm32/stm32h7/soc.c

>>>> index 692dbcc..e0d3f11 100644

>>>> --- a/arch/arm/mach-stm32/stm32h7/soc.c

>>>> +++ b/arch/arm/mach-stm32/stm32h7/soc.c

>>>> @@ -30,9 +30,12 @@ int arch_cpu_init(void)

>>>>    		{ 0x00000000, REGION_0, XN_DIS, PRIV_RW_USR_RW,

>>>>    		O_I_WB_RD_WR_ALLOC, REGION_4GB },

>>>>

>>>> -		/* Code area, executable & strongly ordered */

>>>> -		{ 0xD0000000, REGION_1, XN_EN, PRIV_RW_USR_RW,

>>>> -		STRONG_ORDER, REGION_8MB },

>>>> +		/*

>>>> +		 * Code area, executable, Outer and inner write-back,

>>>> +		 * no write allocate

>>>> +		 */

>>>> +		{ 0xD0000000, REGION_1, XN_DIS, PRIV_RW_USR_RW,

>>>> +		O_I_WB_RD_WR_ALLOC, REGION_32MB },

>>>>

>>>>    		/* Device area in all H7 : Not executable */

>>>>    		{ 0x40000000, REGION_2, XN_EN, PRIV_RW_USR_RW,

>>>> --

>>>> 1.9.1

>>>

Patch

diff --git a/arch/arm/mach-stm32/stm32h7/soc.c b/arch/arm/mach-stm32/stm32h7/soc.c
index 692dbcc..e0d3f11 100644
--- a/arch/arm/mach-stm32/stm32h7/soc.c
+++ b/arch/arm/mach-stm32/stm32h7/soc.c
@@ -30,9 +30,12 @@  int arch_cpu_init(void)
 		{ 0x00000000, REGION_0, XN_DIS, PRIV_RW_USR_RW,
 		O_I_WB_RD_WR_ALLOC, REGION_4GB },
 
-		/* Code area, executable & strongly ordered */
-		{ 0xD0000000, REGION_1, XN_EN, PRIV_RW_USR_RW,
-		STRONG_ORDER, REGION_8MB },
+		/*
+		 * Code area, executable, Outer and inner write-back,
+		 * no write allocate
+		 */
+		{ 0xD0000000, REGION_1, XN_DIS, PRIV_RW_USR_RW,
+		O_I_WB_RD_WR_ALLOC, REGION_32MB },
 
 		/* Device area in all H7 : Not executable */
 		{ 0x40000000, REGION_2, XN_EN, PRIV_RW_USR_RW,