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

Mark Rutland
2014-06-02T11:34:51Z

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

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.