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 <york...@freescale.com>
> CC: David Feng <feng...@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.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to