On 03/21/2014 03:07 AM, Ralf Baechle wrote:
On Thu, Nov 07, 2013 at 06:35:54PM +0000, Maciej W. Rozycki wrote:
We currently support no MIPS processor that has its I-cache coherent with
the D-cache, no such processor may even exist. We apparently have two
configurations that have fully coherent D-caches and therefore set
cpu_has_ic_fills_f_dc, and these are the Alchemy and NetLogic processor
families. I have checked relevant CPU documentation I was able to track
down and in both cases the respective documents[1][2] clearly state that
the I-cache provides no hardware coherency and whenever instructions in
memory are modified then the I-cache has to be synchronized by software
even though the D-caches are fully coherent.
No, cpu_has_ic_fills_f_dc doesn't mean the D-cache is coherent but rather
that the I-cache is refilled from the D-cache if there was a hit. This
means there is no need to write back the D-cache to S-cache or even memory
which is saving some time.
Therefore we cannot ever avoid the call to flush_cache_page in
copy_to_user_page and here is a change that reflects this observation.
The implementation of flush_cache_page may then choose freely whether it
needs to perform a full cache synchronization with D-cache writeback and
invalidation requests or whether a lone I-cache invalidation will suffice.
The c-r4k.c implementation already respects the setting of
cpu_has_ic_fills_f_dc and avoids touching the D-cache unless necessary.
The lack of I-cache synchronization is typically seen in debugging
sessions e.g. with GDB where software breakpoints are used. When such a
breakpoint is hit and subsequently replaced using a ptrace(2) call with
the original instruction, the BREAK instruction previously executed
sometimes remains in the I-cache and causes the breakpoint just removed to
hit again regardless, resulting in a spurious SIGTRAP signal delivery that
debuggers typically complain about (e.g. "Program received signal SIGTRAP,
Trace/breakpoint trap" in the case of GDB). Of course the I-cache line
containing the BREAK instruction may have since been randomly replaced, in
which case no problem occurs.
[1] "AMD Alchemy Au1200 Processor Data Book", AMD Alchemy, January, 2005,
Publication ID: 32798A
[2] "XLP Processor Family Programming Reference Manual", NetLogic
Microsystems, Revision Level 1.10, February, 2011, Document Number
10724V110PM-CR (regrettably not publicly available)
Signed-off-by: Maciej W. Rozycki <macro@xxxxxxxxxxxxxxxx>
---
Ralf,
Please apply. I've seen these SIGTRAPs in some NetLogic GDB testing and
the removal of this cpu_has_ic_fills_f_dc condition from copy_to_user_page
is really necessary; also the Au1200 document is very explicit about the
requirement of I-cache invalidation in software (see Section 2.3.7.3
"Instruction Cache Coherency").
You found a bug and yes, the fix you sent improves things a bit. But
there is also the cache on a cache coherent system where a page might
be marked for a delayed cache flush with SetPageDcacheDirty(), then
flushed by flush_cache_page() before eventually the delayed cacheflush
flushes it once more for a good meassure.
What do you think about below patch to also deal with the duplicate flushing?
The problem only happens when modifying target executable code through
the ptrace() system call.
For all cases where a program is modifying its own executable memory, we
require that it make the special mips cacheflush system call.
I don't object to modifying this file, but I wonder if the call to the
flushing function should be pushed up into the ptrace() code.
David Daney
Ralf
arch/mips/mm/init.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c
index 6b59617..80072ef 100644
--- a/arch/mips/mm/init.c
+++ b/arch/mips/mm/init.c
@@ -227,13 +227,13 @@ void copy_to_user_page(struct vm_area_struct *vma,
void *vto = kmap_coherent(page, vaddr) + (vaddr & ~PAGE_MASK);
memcpy(vto, src, len);
kunmap_coherent();
+ if (vma->vm_flags & VM_EXEC)
+ flush_cache_page(vma, vaddr, page_to_pfn(page));
} else {
memcpy(dst, src, len);
if (cpu_has_dc_aliases)
SetPageDcacheDirty(page);
}
- if ((vma->vm_flags & VM_EXEC) && !cpu_has_ic_fills_f_dc)
- flush_cache_page(vma, vaddr, page_to_pfn(page));
}
void copy_from_user_page(struct vm_area_struct *vma,
