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

[Vladimir Murzin <vladimir.murzin@arm.com>]

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
>