[U-Boot] [Patch v6 3/5] ARMv8/FSL_LSCH3: Add FSL_LSCH3 SoC

[Mark Rutland <mark.rutland@arm.com>]

Hi York,

Apologies for the late reply on this.

I'm somewhat concerned regarding the issues you're seeing with cacheable
page table walks, but I'll ignore that for the moment so we can get the
ordering of maintenance sorted.

On Thu, Jun 05, 2014 at 08:23:09PM +0100, York Sun wrote:
> Freescale LayerScape with Chassis Generation 3 is a set of SoCs with
> ARMv8 cores and 3rd generation of Chassis. We use different MMU setup
> to support memory map and cache attribute for these SoCs. MMU and cache
> are enabled very early to bootst performance, especially for early
> development on emulators. After u-boot relocates to DDR, a new MMU
> table with QBMan cache access is created in DDR. SMMU pagesize is set
> in SMMU_sACR register. Both DDR3 and DDR4 are supported.
> 
> Signed-off-by: York Sun <yorksun@freescale.com>
> Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
> Signed-off-by: Arnab Basu <arnab.basu@freescale.com>
> ---

[...]

> +       /* point TTBR to the new table */
> +       el = current_el();

Could we not place a dsb() here...

> +       if (el == 1) {
> +               asm volatile("dsb sy;msr ttbr0_el1, %0;isb"
> +                            : : "r" ((u64)level0_table) : "memory");
> +       } else if (el == 2) {
> +               asm volatile("dsb sy;msr ttbr0_el2, %0;isb"
> +                            : : "r" ((u64)level0_table) : "memory");
> +       } else if (el == 3) {
> +               asm volatile("dsb sy;msr ttbr0_el3, %0;isb"
> +                            : : "r" ((u64)level0_table) : "memory");
> +       } else {
> +               hang();
> +       }

...and an isb() here?

It would save duplicating them for each EL.

> +       /*
> +        * MMU is already enabled, just need to invalidate TLB to load the
> +        * new table. The new table is compatible with the current table, if
> +        * MMU somehow walks through the new table before invalidation TLB,
> +        * it still works. So we don't need to turn off MMU here.
> +        */
> +}
> +
> +int arch_cpu_init(void)
> +{
> +       icache_enable();

Just to check: the icache is clean/invalid at this point?

> +       __asm_invalidate_dcache_all();
> +       __asm_invalidate_tlb_all();
> +       early_mmu_setup();
> +       set_sctlr(get_sctlr() | CR_C);
> +       return 0;
> +}

[...]

> +/*
> + * This function is called from lib/board.c.
> + * It recreates MMU table in main memory. MMU and d-cache are enabled earlier.
> + * There is no need to disable d-cache for this operation.
> + */
> +void enable_caches(void)
> +{
> +       final_mmu_setup();
> +       flush_dcache_range(gd->arch.tlb_addr,
> +                          gd->arch.tlb_addr +  gd->arch.tlb_size);
> +       __asm_invalidate_tlb_all();
> +}

This looks wrong, given your previous comments that you couldn't get the
MMU to lookup from the cache.

In final_mmu_setup() you pointed the TTBR0_ELx to the new tables, but at
that point the tables aren't guaranteed to be in memory, because they
haven't been flushed. The MMU can start fetching the garbage from memory
immediately, and things might go wrong before __asm_invalidate_tlb_all()
blows away the garbage the MMU read from memory (for instance, the
mapping covering the enable_caches function might get replaced by
garbage).

If the page table walks are non-cacheable, you need to flush the tables
to memory before programming the relevant TTBR.

Thanks,
Mark.