diff mbox

[RFC,2/2] ARM: nommu: remap exception base address to RAM

Message ID 20170115114750.GA5456@afzalpc (mailing list archive)
State New, archived
Headers show

Commit Message

afzal mohammed Jan. 15, 2017, 11:47 a.m. UTC 
Hi,

On Sat, Jan 07, 2017 at 10:43:39PM +0530, Afzal Mohammed wrote:
> On Tue, Dec 13, 2016 at 10:02:26AM +0000, Russell King - ARM Linux wrote:

> > Also, if the region setup for the vectors was moved as well, it would
> > then be possible to check the ID registers to determine whether this
> > is supported, and make the decision where to locate the vectors base
> > more dynamically.
> 
> This would affect Cortex-R's, which is a bit concerning due to lack of
> those platforms with me, let me try to get it right.

QEMU too doesn't seem to provide a Cortex-R target

> Seems translating __setup_mpu() altogether to C

afaics, a kind of C translation is already present as
mpu_setup_region() in arch/arm/mm/nommu.c that takes care of
MPU_RAM_REGION only. And that seems to be a kind of redundant as it is
also done in asm at __setup_mpu(). Git blames asm & C to consecutive
commits, that makes me a little shaky about the conclusion on it being
redundant.

> & installing at a later, but suitable place might be better.

But looks like enabling MPU can't be moved to C & that would
necessitate keeping at least some portion of__setu_mpu() in asm.

Instead, moving region setup only for vectors to C as Russell
suggested at first would have to be done.

A kind of diff at the end is in my mind, with additional changes to
handle the similar during secondary cpu bringup too.

Thinking of invoking mpu_setup() from secondary_start_kernel() in
arch/arm/kernel/smp.c, with mpu_setup() being slightly modified to
avoid storing region details again when invoked by secondary cpu's.

Vladimir, once changes are done after a revisit, i would need your
help to test on Cortex-R.

As an aside, wasn't aware of the fact that Cortex-R supports SMP
Linux, had thought that, of !MMU one's, only Blackfin & J2 had it.


> Also !MMU Kernel could boot on 3 ARM v7-A platforms - AM335x Beagle
> Bone (A8), AM437x IDK (A9) & Vybrid VF610 (on A5 core, note that it
> has M4 core too)

Talking about Cortex-M, AMx3's too have it, to be specific M3, but
they are not Linux-able unlike the one in VF610.

Regards
afzal

--->8---

Comments

Vladimir Murzin Jan. 16, 2017, 9:53 a.m. UTC | #1 
Hi,

On 15/01/17 11:47, Afzal Mohammed wrote:
> Hi,
> 
> On Sat, Jan 07, 2017 at 10:43:39PM +0530, Afzal Mohammed wrote:
>> On Tue, Dec 13, 2016 at 10:02:26AM +0000, Russell King - ARM Linux wrote:
> 
>>> Also, if the region setup for the vectors was moved as well, it would
>>> then be possible to check the ID registers to determine whether this
>>> is supported, and make the decision where to locate the vectors base
>>> more dynamically.
>>
>> This would affect Cortex-R's, which is a bit concerning due to lack of
>> those platforms with me, let me try to get it right.
> 
> QEMU too doesn't seem to provide a Cortex-R target
> 
>> Seems translating __setup_mpu() altogether to C
> 
> afaics, a kind of C translation is already present as
> mpu_setup_region() in arch/arm/mm/nommu.c that takes care of
> MPU_RAM_REGION only. And that seems to be a kind of redundant as it is
> also done in asm at __setup_mpu(). Git blames asm & C to consecutive
> commits, that makes me a little shaky about the conclusion on it being
> redundant.
> 

It is not redundant. MPU setup is done it two steps. The first step done in
asm to enable caches, there only kernel image is covered; the second step takes
care on the whole RAM given via dt or "mem=" parameter.

I think other regions are kept there to avoid C side dancing in case of SMP.

>> & installing at a later, but suitable place might be better.
> 
> But looks like enabling MPU can't be moved to C & that would
> necessitate keeping at least some portion of__setu_mpu() in asm.
> 
> Instead, moving region setup only for vectors to C as Russell
> suggested at first would have to be done.
> 
> A kind of diff at the end is in my mind, with additional changes to
> handle the similar during secondary cpu bringup too.
> 
> Thinking of invoking mpu_setup() from secondary_start_kernel() in
> arch/arm/kernel/smp.c, with mpu_setup() being slightly modified to
> avoid storing region details again when invoked by secondary cpu's.

I have wip patches on reworking MPU setup code. The idea is to start using
mpu_rgn_info[] actively, so asm part for secondariness would just sync-up
content of that array. Additionally, it seems that we can reuse free MPU slots
to cover memory which is discarded due to MPU alignment restrictions... 

> 
> Vladimir, once changes are done after a revisit, i would need your
> help to test on Cortex-R.

I'm more than happy to help, but currently I have limited bandwidth, so if it
can wait till the next dev cycle I'd try to make MPU rework finished by that
time.

> 
> As an aside, wasn't aware of the fact that Cortex-R supports SMP
> Linux, had thought that, of !MMU one's, only Blackfin & J2 had it.
> 
> 
>> Also !MMU Kernel could boot on 3 ARM v7-A platforms - AM335x Beagle
>> Bone (A8), AM437x IDK (A9) & Vybrid VF610 (on A5 core, note that it
>> has M4 core too)
> 
> Talking about Cortex-M, AMx3's too have it, to be specific M3, but
> they are not Linux-able unlike the one in VF610.
> 

Thanks!

Vladimir

> Regards
> afzal
> 
> --->8---
> 
> diff --git a/arch/arm/kernel/head-nommu.S b/arch/arm/kernel/head-nommu.S
> index e0565d73e49e..f8ac79b6136d 100644
> --- a/arch/arm/kernel/head-nommu.S
> +++ b/arch/arm/kernel/head-nommu.S
> @@ -249,20 +249,6 @@ ENTRY(__setup_mpu)
>  	setup_region r0, r5, r6, MPU_INSTR_SIDE @ 0x0, BG region, enabled
>  2:	isb
>  
> -	/* Vectors region */
> -	set_region_nr r0, #MPU_VECTORS_REGION
> -	isb
> -	/* Shared, inaccessible to PL0, rw PL1 */
> -	mov	r0, #CONFIG_VECTORS_BASE	@ Cover from VECTORS_BASE
> -	ldr	r5,=(MPU_AP_PL1RW_PL0NA | MPU_RGN_NORMAL)
> -	/* Writing N to bits 5:1 (RSR_SZ) --> region size 2^N+1 */
> -	mov	r6, #(((2 * PAGE_SHIFT - 1) << MPU_RSR_SZ) | 1 << MPU_RSR_EN)
> -
> -	setup_region r0, r5, r6, MPU_DATA_SIDE	@ VECTORS_BASE, PL0 NA, enabled
> -	beq	3f				@ Memory-map not unified
> -	setup_region r0, r5, r6, MPU_INSTR_SIDE	@ VECTORS_BASE, PL0 NA, enabled
> -3:	isb
> -
>  	/* Enable the MPU */
>  	mrc	p15, 0, r0, c1, c0, 0		@ Read SCTLR
>  	bic     r0, r0, #CR_BR			@ Disable the 'default mem-map'
> diff --git a/arch/arm/mm/nommu.c b/arch/arm/mm/nommu.c
> index e82056df0635..7fe8906322d5 100644
> --- a/arch/arm/mm/nommu.c
> +++ b/arch/arm/mm/nommu.c
> @@ -269,12 +269,19 @@ void __init mpu_setup(void)
>  					ilog2(memblock.memory.regions[0].size),
>  					MPU_AP_PL1RW_PL0RW | MPU_RGN_NORMAL);
>  	if (region_err) {
> -		panic("MPU region initialization failure! %d", region_err);
> +		panic("MPU RAM region initialization failure! %d", region_err);
>  	} else {
> -		pr_info("Using ARMv7 PMSA Compliant MPU. "
> -			 "Region independence: %s, Max regions: %d\n",
> -			mpu_iside_independent() ? "Yes" : "No",
> -			mpu_max_regions());
> +		region_err = mpu_setup_region(MPU_VECTORS_REGION, vectors_base,
> +					ilog2(memblock.memory.regions[0].size),
> +					MPU_AP_PL1RW_PL0NA | MPU_RGN_NORMAL);
> +		if (region_err) {
> +			panic("MPU VECTOR region initialization failure! %d",
> +			      region_err);
> +		} else {
> +			pr_info("Using ARMv7 PMSA Compliant MPU. "
> +				"Region independence: %s, Max regions: %d\n",
> +				mpu_iside_independent() ? "Yes" : "No",
> +				mpu_max_regions());
>  	}
>  }
>  #else
>
afzal mohammed Jan. 16, 2017, 12:34 p.m. UTC | #2 
Hi,

On Mon, Jan 16, 2017 at 09:53:41AM +0000, Vladimir Murzin wrote:
> On 15/01/17 11:47, Afzal Mohammed wrote:

> > mpu_setup_region() in arch/arm/mm/nommu.c that takes care of
> > MPU_RAM_REGION only. And that seems to be a kind of redundant as it is
> > also done in asm at __setup_mpu(). Git blames asm & C to consecutive
> > commits, that makes me a little shaky about the conclusion on it being
> > redundant.
> 
> It is not redundant. MPU setup is done it two steps. The first step done in
> asm to enable caches, there only kernel image is covered; the second step takes
> care on the whole RAM given via dt or "mem=" parameter.

Okay, thanks for the details.

> > Thinking of invoking mpu_setup() from secondary_start_kernel() in
> > arch/arm/kernel/smp.c, with mpu_setup() being slightly modified to
> > avoid storing region details again when invoked by secondary cpu's.
> 
> I have wip patches on reworking MPU setup code. The idea is to start using
> mpu_rgn_info[] actively, so asm part for secondariness would just sync-up
> content of that array. Additionally, it seems that we can reuse free MPU slots
> to cover memory which is discarded due to MPU alignment restrictions... 
> 
> > Vladimir, once changes are done after a revisit, i would need your
> > help to test on Cortex-R.
> 
> I'm more than happy to help, but currently I have limited bandwidth, so if it
> can wait till the next dev cycle I'd try to make MPU rework finished by that
> time.

Okay, please feel free to do MPU rework the way you were planning, you
know more details & have the platform to achieve it with much higher
efficiency than me.

Regards
afzal
diff mbox

Patch

diff --git a/arch/arm/kernel/head-nommu.S b/arch/arm/kernel/head-nommu.S
index e0565d73e49e..f8ac79b6136d 100644
--- a/arch/arm/kernel/head-nommu.S
+++ b/arch/arm/kernel/head-nommu.S
@@ -249,20 +249,6 @@  ENTRY(__setup_mpu)
 	setup_region r0, r5, r6, MPU_INSTR_SIDE @ 0x0, BG region, enabled
 2:	isb
 
-	/* Vectors region */
-	set_region_nr r0, #MPU_VECTORS_REGION
-	isb
-	/* Shared, inaccessible to PL0, rw PL1 */
-	mov	r0, #CONFIG_VECTORS_BASE	@ Cover from VECTORS_BASE
-	ldr	r5,=(MPU_AP_PL1RW_PL0NA | MPU_RGN_NORMAL)
-	/* Writing N to bits 5:1 (RSR_SZ) --> region size 2^N+1 */
-	mov	r6, #(((2 * PAGE_SHIFT - 1) << MPU_RSR_SZ) | 1 << MPU_RSR_EN)
-
-	setup_region r0, r5, r6, MPU_DATA_SIDE	@ VECTORS_BASE, PL0 NA, enabled
-	beq	3f				@ Memory-map not unified
-	setup_region r0, r5, r6, MPU_INSTR_SIDE	@ VECTORS_BASE, PL0 NA, enabled
-3:	isb
-
 	/* Enable the MPU */
 	mrc	p15, 0, r0, c1, c0, 0		@ Read SCTLR
 	bic     r0, r0, #CR_BR			@ Disable the 'default mem-map'
diff --git a/arch/arm/mm/nommu.c b/arch/arm/mm/nommu.c
index e82056df0635..7fe8906322d5 100644
--- a/arch/arm/mm/nommu.c
+++ b/arch/arm/mm/nommu.c
@@ -269,12 +269,19 @@  void __init mpu_setup(void)
 					ilog2(memblock.memory.regions[0].size),
 					MPU_AP_PL1RW_PL0RW | MPU_RGN_NORMAL);
 	if (region_err) {
-		panic("MPU region initialization failure! %d", region_err);
+		panic("MPU RAM region initialization failure! %d", region_err);
 	} else {
-		pr_info("Using ARMv7 PMSA Compliant MPU. "
-			 "Region independence: %s, Max regions: %d\n",
-			mpu_iside_independent() ? "Yes" : "No",
-			mpu_max_regions());
+		region_err = mpu_setup_region(MPU_VECTORS_REGION, vectors_base,
+					ilog2(memblock.memory.regions[0].size),
+					MPU_AP_PL1RW_PL0NA | MPU_RGN_NORMAL);
+		if (region_err) {
+			panic("MPU VECTOR region initialization failure! %d",
+			      region_err);
+		} else {
+			pr_info("Using ARMv7 PMSA Compliant MPU. "
+				"Region independence: %s, Max regions: %d\n",
+				mpu_iside_independent() ? "Yes" : "No",
+				mpu_max_regions());
 	}
 }
 #else