On Fri, Jan 22, 2016 at 1:19 PM, James Hogan <james.hogan@xxxxxxxxxx> wrote:
> On Fri, Jan 22, 2016 at 01:06:14PM +0100, Manuel Lauss wrote:
>> Hi James,
>>
>> On Fri, Jan 22, 2016 at 11:58 AM, James Hogan <james.hogan@xxxxxxxxxx> wrote:
>> > It is still necessary to handle icache coherency in flush_cache_range()
>> > and copy_to_user_page() when the icache fills from the dcache, even
>> > though the dcache does not need to be written back. However when this
>> > handling was added in commit 2eaa7ec286db ("[MIPS] Handle I-cache
>> > coherency in flush_cache_range()"), it did not do any icache flushing
>> > when it fills from dcache.
>> >
>> > Therefore fix r4k_flush_cache_range() to run
>> > local_r4k_flush_cache_range() without taking into account whether icache
>> > fills from dcache, so that the icache coherency gets handled. Checks are
>> > also added in local_r4k_flush_cache_range() so that the dcache blast
>> > doesn't take place when icache fills from dcache.
>> >
>> > A test to mmap a page PROT_READ|PROT_WRITE, modify code in it, and
>> > mprotect it to VM_READ|VM_EXEC (similar to case described in above
>> > commit) can hit this case quite easily to verify the fix.
>> >
>> > A similar check was added in commit f8829caee311 ("[MIPS] Fix aliasing
>> > bug in copy_to_user_page / copy_from_user_page"), so also fix
>> > copy_to_user_page() similarly, to call flush_cache_page() without taking
>> > into account whether icache fills from dcache, since flush_cache_page()
>> > already takes that into account to avoid performing a dcache flush.
>> >
>> > Signed-off-by: James Hogan <james.hogan@xxxxxxxxxx>
>> > Cc: Ralf Baechle <ralf@xxxxxxxxxxxxxx>
>> > Cc: Leonid Yegoshin <leonid.yegoshin@xxxxxxxxxx>
>> > Cc: Manuel Lauss <manuel.lauss@xxxxxxxxx>
>> > Cc: linux-mips@xxxxxxxxxxxxxx
>> > ---
>> > arch/mips/mm/c-r4k.c | 11 +++++++++--
>> > arch/mips/mm/init.c | 2 +-
>> > 2 files changed, 10 insertions(+), 3 deletions(-)
>>
>>
>> I did some light testing on Alchemy and see no problems so far.
>> If it matters: Tested-by: Manuel Lauss <manuel.lauss@xxxxxxxxx>
>
> Thanks Manuel.
>
> FWIW, attached is the test program I mentioned, which hits the first
> part of this patch (flush_cache_range) via mprotect(2) and checks if
> icache seems to have been flushed (tested on mips64r6, but should be
> portable).
With the patch it takes almost 3 times as long to finish the test, but
it does fix
occasional (5 out of 20 runs) failures. The --loop2 test always fails, with or
without the patch:
db1300 ~ # ./mprotect-test --loop2
Initial mprotect SUCCESS
Looped { mprotect RW, modify, mprotect RX, test } SUCCESS
mprotect.c:214: Looped mprotect(2) PROT_READ|PROT_WRITE|PROT_EXEC
didn't sync icache, ret=ffffffff, expected 0
Manuel
