[U-Boot] [Patch v4 2/5] ARMv8: Adjust MMU setup

[York Sun <yorksun@freescale.com>]

On 06/02/2014 04:34 AM, Mark Rutland wrote:
> On Thu, May 29, 2014 at 09:49:05PM +0100, York Sun wrote:
>> Make MMU functions reusable. Platform code can setup its own MMU tables.
> 
> What exactly does platform code need to setup its own tables for?

The general ARMv8 MMU table is not detail enough to control memory attribute
like cache for all addresses. We have devices mapping to addresses with
different requirement for cache control.

> 
>> Also fix a typo of "TCR_EL3_IPS_BITS" in cache_v8.c.
>>
>> Signed-off-by: York Sun <yorksun@freescale.com>
>> CC: David Feng <fenghua@phytium.com.cn>
>> ---
>> Change log:
>>  v4: new patch, splitted from v3 2/4
>>      Revise set_pgtable_section() to be reused by platform MMU code
>>      Add inline function set_ttbr_tcr_mair() to be used by this and platform mmu code
>>
>>  arch/arm/cpu/armv8/cache_v8.c    |   49 ++++++++++++++++----------------------
>>  arch/arm/include/asm/armv8/mmu.h |   23 ++++++++++++++++++
>>  2 files changed, 43 insertions(+), 29 deletions(-)
>>
>> diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c
>> index a96ecda..67dbd46 100644
>> --- a/arch/arm/cpu/armv8/cache_v8.c
>> +++ b/arch/arm/cpu/armv8/cache_v8.c
>> @@ -12,15 +12,14 @@
>>  DECLARE_GLOBAL_DATA_PTR;
>>  
>>  #ifndef CONFIG_SYS_DCACHE_OFF
>> -
>> -static void set_pgtable_section(u64 section, u64 memory_type)
>> +void set_pgtable_section(u64 *page_table, u64 index, u64 section,
>> +			 u64 memory_type)
>>  {
>> -	u64 *page_table = (u64 *)gd->arch.tlb_addr;
>>  	u64 value;
>>  
>> -	value = (section << SECTION_SHIFT) | PMD_TYPE_SECT | PMD_SECT_AF;
>> +	value = section | PMD_TYPE_SECT | PMD_SECT_AF;
>>  	value |= PMD_ATTRINDX(memory_type);
>> -	page_table[section] = value;
>> +	page_table[index] = value;
>>  }
>>  
>>  /* to activate the MMU we need to set up virtual memory */
>> @@ -28,10 +27,13 @@ static void mmu_setup(void)
>>  {
>>  	int i, j, el;
>>  	bd_t *bd = gd->bd;
>> +	u64 *page_table = (u64 *)gd->arch.tlb_addr;
>>  
>>  	/* Setup an identity-mapping for all spaces */
>> -	for (i = 0; i < (PGTABLE_SIZE >> 3); i++)
>> -		set_pgtable_section(i, MT_DEVICE_NGNRNE);
>> +	for (i = 0; i < (PGTABLE_SIZE >> 3); i++) {
>> +		set_pgtable_section(page_table, i, i << SECTION_SHIFT,
>> +				    MT_DEVICE_NGNRNE);
>> +	}
>>  
>>  	/* Setup an identity-mapping for all RAM space */
>>  	for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
>> @@ -39,36 +41,25 @@ static void mmu_setup(void)
>>  		ulong end = bd->bi_dram[i].start + bd->bi_dram[i].size;
>>  		for (j = start >> SECTION_SHIFT;
>>  		     j < end >> SECTION_SHIFT; j++) {
>> -			set_pgtable_section(j, MT_NORMAL);
>> +			set_pgtable_section(page_table, j, j << SECTION_SHIFT,
>> +					    MT_NORMAL);
>>  		}
>>  	}
>>  
>>  	/* load TTBR0 */
>>  	el = current_el();
>>  	if (el == 1) {
>> -		asm volatile("msr ttbr0_el1, %0"
>> -			     : : "r" (gd->arch.tlb_addr) : "memory");
>> -		asm volatile("msr tcr_el1, %0"
>> -			     : : "r" (TCR_FLAGS | TCR_EL1_IPS_BITS)
>> -			     : "memory");
>> -		asm volatile("msr mair_el1, %0"
>> -			     : : "r" (MEMORY_ATTRIBUTES) : "memory");
>> +		set_ttbr_tcr_mair(el, gd->arch.tlb_addr,
>> +				  TCR_FLAGS | TCR_EL1_IPS_BITS,
>> +				  MEMORY_ATTRIBUTES);
>>  	} else if (el == 2) {
>> -		asm volatile("msr ttbr0_el2, %0"
>> -			     : : "r" (gd->arch.tlb_addr) : "memory");
>> -		asm volatile("msr tcr_el2, %0"
>> -			     : : "r" (TCR_FLAGS | TCR_EL2_IPS_BITS)
>> -			     : "memory");
>> -		asm volatile("msr mair_el2, %0"
>> -			     : : "r" (MEMORY_ATTRIBUTES) : "memory");
>> +		set_ttbr_tcr_mair(el, gd->arch.tlb_addr,
>> +				  TCR_FLAGS | TCR_EL2_IPS_BITS,
>> +				  MEMORY_ATTRIBUTES);
>>  	} else {
>> -		asm volatile("msr ttbr0_el3, %0"
>> -			     : : "r" (gd->arch.tlb_addr) : "memory");
>> -		asm volatile("msr tcr_el3, %0"
>> -			     : : "r" (TCR_FLAGS | TCR_EL2_IPS_BITS)
>> -			     : "memory");
>> -		asm volatile("msr mair_el3, %0"
>> -			     : : "r" (MEMORY_ATTRIBUTES) : "memory");
>> +		set_ttbr_tcr_mair(el, gd->arch.tlb_addr,
>> +				  TCR_FLAGS | TCR_EL3_IPS_BITS,
>> +				  MEMORY_ATTRIBUTES);
>>  	}
>>  
>>  	/* enable the mmu */
>> diff --git a/arch/arm/include/asm/armv8/mmu.h b/arch/arm/include/asm/armv8/mmu.h
>> index 1193e76..7de4ff9 100644
>> --- a/arch/arm/include/asm/armv8/mmu.h
>> +++ b/arch/arm/include/asm/armv8/mmu.h
>> @@ -108,4 +108,27 @@
>>  				TCR_IRGN_WBWA |		\
>>  				TCR_T0SZ(VA_BITS))
>>  
>> +#ifndef __ASSEMBLY__
>> +void set_pgtable_section(u64 *page_table, u64 index,
>> +			 u64 section, u64 memory_type);
>> +static inline void set_ttbr_tcr_mair(int el, u64 table, u64 tcr, u64 attr)
>> +{
>> +	asm volatile("dsb sy;isb");
> 
> Huh? This wasn't anywhere before. Is the isb necessary?

Probably not, but to make sure all previous instructions finish.

> 
> When is this function expected to be called? Is the MMU expected to be
> on?

The general ARMv8 code calls this when MMU is off. For the SoC I am enabling, it
is called when MMU is off for the first time, but MMU on for the 2nd time.

> 
>> +	if (el == 1) {
>> +		asm volatile("msr ttbr0_el1, %0" : : "r" (table) : "memory");
>> +		asm volatile("msr tcr_el1, %0" : : "r" (tcr) : "memory");
>> +		asm volatile("msr mair_el1, %0" : : "r" (attr) : "memory");
> 
> If the MMU is on, this looks really scary -- what are you expecting to
> change in a single invocation?

It is not scary for general ARMv8 code. MMU is off then this is called. For FSL
SoC, I have these called for the 2nd time, when MMU is on. The 2nd time call
points to a new MMU table.

> 
>> +	} else if (el == 2) {
>> +		asm volatile("msr ttbr0_el2, %0" : : "r" (table) : "memory");
>> +		asm volatile("msr tcr_el2, %0" : : "r" (tcr) : "memory");
>> +		asm volatile("msr mair_el2, %0" : : "r" (attr) : "memory");
>> +	} else if (el == 3) {
>> +		asm volatile("msr ttbr0_el3, %0" : : "r" (table) : "memory");
>> +		asm volatile("msr tcr_el3, %0" : : "r" (tcr) : "memory");
>> +		asm volatile("msr mair_el3, %0" : : "r" (attr) : "memory");
>> +	} else {
>> +		hang();
>> +	}
> 
> And no synchronisation to ensure that these writes are complete or even
> ordered w.r.t. each other?
> 

That's why I added asm volatile("dsb sy;isb") before them. The order of these
write doesn't matter. See the code before my change
http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/cpu/armv8/cache_v8.c;h=a96ecda7e3146996d20a2e46b975dced2769255c;hb=HEAD

For the 2nd write used by FSL SoC, I need to flush the cache to make sure the
table is in main DDR and perform TLB invalidation. That's when the loading of
new MMU table happens.

York