On Sun, 2009-10-25 at 13:39 +0000, Russell King - ARM Linux wrote:

On Tue, Oct 20, 2009 at 12:39:08PM +0100, Catalin Marinas wrote:

quoted

BTW, the cache flushing code in Linux can be optimised a bit more on
VIPT caches:

      * __cpuc_flush_dcache_page() could cope with just D-cache clean
        rather than clean+invalidate

No it can not - that breaks shared mappings.  The problem is that
flush_dcache_page() is used in two circumstances.  These are described
in more detail in cachetlb.txt, but briefly:

1. After the kernel writes to its mapping for a page cache page, and needs
   to ensure that those writes are visible to shared mmap()s in userspace.

2. Before the kernel reads from its mapping for a page cache page, and
   needs to ensure that it reads up to date data written by userspace
   into those mappings.

So just cleaning the D-cache means that (2) will return stale data.

I was referring to VIPT (non-aliasing) case only. Point (2) above is
still valid even if only cleaning the D-cache lines. Shared mappings use
the same cache lines according to the physical address.

quoted

      * whole I-cache invalidation was needed for some ARM1136 erratum.
        We can conditionally revert it to invalidating a range

That's not what the commit (826cbda) says which implemented it.  Also,
since we have broken I-cache flushes even with the erratum enabled,
let's fix the work-around and re-evaluate the situation before changing
anything.

The commit doesn't say but there was an earlier commit (141fa40cff9088)
which was changing from I-cache inv by MVA to full I-cache inv because
of some ARM1136 erratum (371025). When I sent the second, I just
followed the same idea. We could make this conditional but need to run
some benchmarks to check whether there is a real benefit.

quoted

Flush the D-cache during copy_user_highpage()

From: Catalin Marinas <catalin.marinas@arm.com>

The I and D caches for copy-on-write pages on processors with
write-allocate caches become incoherent causing problems on application
relying on CoW for text pages (dynamic linker relocating symbols in a
text page). This patch flushes the D-cache for such pages (possibly
lazily via update_mmu_cache which also takes care of the I-cache).

Actually, I think this is caused by a missing I-cache flush in
flush_cache_range().  Adding that flush should resolve this problem
in hand (and make VIPT aliasing and VIPT non-aliasing behave in the
same manner.)  That's something which my patch previously posted in
this thread did.

I think the I-cache flush would later be done via update_mmu_cache(), if
I recall the call trace correctly.

Anyway, flushing both D and I caches in flush_cache_range() would have
the same impact.

Note also that with ASID tagged VIVT I-cache, we are missing out
on cache flushing.  As you've identified, it's entirely possible
for text page translations to be changed, and according to B3.4.1
bullet 2, a flush is required.

We can do this conditionally at run-time. I don't have such CPU to test
it.

-- 
Catalin