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

[Mark Rutland]

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?

> Also fix a typo of "TCR_EL3_IPS_BITS" in cache_v8.c.
> 
> Signed-off-by: York Sun <yorksun at freescale.com>
> CC: David Feng <fenghua at 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?

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

> +	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?

> +	} 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?

Mark.