Re: [4/4] MIPS: Sync icache & dcache in set_pte_at

[Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>]

Paul,

On 03/04/2016 02:37 AM, Paul Burton wrote:
> The problem is that there are other code paths which dirty pages (ie.
> call flush_dcache_page), then get as far as set_pte_at *without* calling
> flush_icache_page.

There is no problem with lazy flush of D-cache and PTE update. D-cache 
works in PHYSICAL ADDRESSes and is independent from that is set in PTE 
and TLB. You can even don't flush D-cache during page-fault, keep stuff 
in D-cache and switch any PTE/TLB in any CPU - anything is coherent... 
if we forget for a moment about cache aliasing, non-coherent I-cache and 
DMA.

Cache aliasing is not compatible with SMP (until some public inter-L1 
protocol is changed to accommodate virtual address).

I-cache incoherency is taken into account in flush_icache_page(), 
including a completion of lazy D-cache flush at that point.

DMA ... well it is a complicated issue now but a simplest rule - 
complete all flush before/after DMA and it is done in 
dma_cache_wback_inv/dma_cache_inv (but I intentionally put out of issue 
the speculative access which complicates an issue).

So, you fight a wrong enemy.


>   Again - I included call traces from 2 paths that hit
> this right in the commit message.
>
> So:
>
>    - flush_icache_page isn't always called before we *need* to writeback
>      the page from the dcache. This is demonstrably the case (again, see
>      the commit message), and causes bugs when using UBIFS on boards
>      using the pistachio SoC at least.

You didn't prove that it is because of your model. You referenced to 
flush_icache_page absence in different places but that places don't 
prepare a code page:

     - forking process actually doesn't copy any page but set it 
temporary non-writable, actual copy is done in page fault
     - load_elf_binary doesn't load any code pages and again leaves that 
process to page fault.

>
>    - flush_icache_page is indicated as something that should go away in
>      Documentation/cachetlb.txt. Why do you feel we should make use of
>      it?

I think it is a wrong mark in doc.

I suspect that your problem with UBIFS is because any DMA-ed code page 
should be flushed from DCache during page fault in flush_icache_page. 
Current master branch code does it only if it is "dirty" but I suspect 
that UBIFS may not do it in some place (doesn't mark it as 
"dirty-by-kernel"). Besides that non-DMA-ed pages can be written by 
device driver which has no access to that bit. For this reason in my 
3.10 branch there is a kernel boot parameter option "mips_non_DMA_FS" 
which enforces D-cache flush regardless it is dirty or not.

I again call you to look into 
https://git.linux-mips.org/cgit/yegoshin/mips.git, branch 
android-linux-mti-3.10 to understand that stuff.

- Leonid.