On 06/18/2014 02:43 AM, Mark Rutland wrote:
> 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 <york...@freescale.com>
>> Signed-off-by: Varun Sethi <varun.se...@freescale.com>
>> Signed-off-by: Arnab Basu <arnab.b...@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.
Sure. I can move them out.
>
>> + /*
>> + * 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?
It is not, but it is done first thing inside icache_enable().
>
>> + __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.
>
Agreed here. I will move the flushing before setting TTBR.
Thanks for review. v7 patch is coming after I verify all the changes.
York
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
