* [PATCH 0/2] MIPS: I6400: Avoid dcache flushes @ 2016-01-22 10:58 ` James Hogan 0 siblings, 0 replies; 14+ messages in thread From: James Hogan @ 2016-01-22 10:58 UTC (permalink / raw) To: Ralf Baechle; +Cc: Manuel Lauss, James Hogan, Leonid Yegoshin, linux-mips These patches allow I6400 core to avoid dcache flushes when making recently modified data available to icache. I6400 effectively can fill icache from dirty dcache contents, which means cpu_has_ic_fills_f_dc can evaluate to true (see patch 2). However there are a couple of bugs in the cache handling when cpu_has_ic_fills_f_dc, which need fixing first (see patch 1). That the CPU fills icache from dcache does not imply that the icache is coherent with dcache. Stale lines still need flushing from the icache, even if lines in the dcache don't need writing back first. James Hogan (2): MIPS: c-r4k: Sync icache when it fills from dcache MIPS: I6400: Icache fills from dcache arch/mips/mm/c-r4k.c | 12 ++++++++++-- arch/mips/mm/init.c | 2 +- 2 files changed, 11 insertions(+), 3 deletions(-) Cc: Ralf Baechle <ralf@linux-mips.org> Cc: Leonid Yegoshin <leonid.yegoshin@imgtec.com> Cc: Manuel Lauss <manuel.lauss@gmail.com> Cc: linux-mips@linux-mips.org -- 2.4.10 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 0/2] MIPS: I6400: Avoid dcache flushes @ 2016-01-22 10:58 ` James Hogan 0 siblings, 0 replies; 14+ messages in thread From: James Hogan @ 2016-01-22 10:58 UTC (permalink / raw) To: Ralf Baechle; +Cc: Manuel Lauss, James Hogan, Leonid Yegoshin, linux-mips These patches allow I6400 core to avoid dcache flushes when making recently modified data available to icache. I6400 effectively can fill icache from dirty dcache contents, which means cpu_has_ic_fills_f_dc can evaluate to true (see patch 2). However there are a couple of bugs in the cache handling when cpu_has_ic_fills_f_dc, which need fixing first (see patch 1). That the CPU fills icache from dcache does not imply that the icache is coherent with dcache. Stale lines still need flushing from the icache, even if lines in the dcache don't need writing back first. James Hogan (2): MIPS: c-r4k: Sync icache when it fills from dcache MIPS: I6400: Icache fills from dcache arch/mips/mm/c-r4k.c | 12 ++++++++++-- arch/mips/mm/init.c | 2 +- 2 files changed, 11 insertions(+), 3 deletions(-) Cc: Ralf Baechle <ralf@linux-mips.org> Cc: Leonid Yegoshin <leonid.yegoshin@imgtec.com> Cc: Manuel Lauss <manuel.lauss@gmail.com> Cc: linux-mips@linux-mips.org -- 2.4.10 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] MIPS: c-r4k: Sync icache when it fills from dcache @ 2016-01-22 10:58 ` James Hogan 0 siblings, 0 replies; 14+ messages in thread From: James Hogan @ 2016-01-22 10:58 UTC (permalink / raw) To: Ralf Baechle; +Cc: Manuel Lauss, James Hogan, Leonid Yegoshin, linux-mips 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@imgtec.com> Cc: Ralf Baechle <ralf@linux-mips.org> Cc: Leonid Yegoshin <leonid.yegoshin@imgtec.com> Cc: Manuel Lauss <manuel.lauss@gmail.com> Cc: linux-mips@linux-mips.org --- arch/mips/mm/c-r4k.c | 11 +++++++++-- arch/mips/mm/init.c | 2 +- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/arch/mips/mm/c-r4k.c b/arch/mips/mm/c-r4k.c index caac3d747a90..fc7289dfaf5a 100644 --- a/arch/mips/mm/c-r4k.c +++ b/arch/mips/mm/c-r4k.c @@ -492,7 +492,14 @@ static inline void local_r4k_flush_cache_range(void * args) if (!(has_valid_asid(vma->vm_mm))) return; - r4k_blast_dcache(); + /* + * If dcache can alias, we must blast it since mapping is changing. + * If executable, we must ensure any dirty lines are written back far + * enough to be visible to icache. + */ + if (cpu_has_dc_aliases || (exec && !cpu_has_ic_fills_f_dc)) + r4k_blast_dcache(); + /* If executable, blast stale lines from icache */ if (exec) r4k_blast_icache(); } @@ -502,7 +509,7 @@ static void r4k_flush_cache_range(struct vm_area_struct *vma, { int exec = vma->vm_flags & VM_EXEC; - if (cpu_has_dc_aliases || (exec && !cpu_has_ic_fills_f_dc)) + if (cpu_has_dc_aliases || exec) r4k_on_each_cpu(local_r4k_flush_cache_range, vma); } diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c index 7e5fa0938c21..6c6a843b0d17 100644 --- a/arch/mips/mm/init.c +++ b/arch/mips/mm/init.c @@ -196,7 +196,7 @@ void copy_to_user_page(struct vm_area_struct *vma, if (cpu_has_dc_aliases) SetPageDcacheDirty(page); } - if ((vma->vm_flags & VM_EXEC) && !cpu_has_ic_fills_f_dc) + if (vma->vm_flags & VM_EXEC) flush_cache_page(vma, vaddr, page_to_pfn(page)); } -- 2.4.10 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 1/2] MIPS: c-r4k: Sync icache when it fills from dcache @ 2016-01-22 10:58 ` James Hogan 0 siblings, 0 replies; 14+ messages in thread From: James Hogan @ 2016-01-22 10:58 UTC (permalink / raw) To: Ralf Baechle; +Cc: Manuel Lauss, James Hogan, Leonid Yegoshin, linux-mips 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@imgtec.com> Cc: Ralf Baechle <ralf@linux-mips.org> Cc: Leonid Yegoshin <leonid.yegoshin@imgtec.com> Cc: Manuel Lauss <manuel.lauss@gmail.com> Cc: linux-mips@linux-mips.org --- arch/mips/mm/c-r4k.c | 11 +++++++++-- arch/mips/mm/init.c | 2 +- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/arch/mips/mm/c-r4k.c b/arch/mips/mm/c-r4k.c index caac3d747a90..fc7289dfaf5a 100644 --- a/arch/mips/mm/c-r4k.c +++ b/arch/mips/mm/c-r4k.c @@ -492,7 +492,14 @@ static inline void local_r4k_flush_cache_range(void * args) if (!(has_valid_asid(vma->vm_mm))) return; - r4k_blast_dcache(); + /* + * If dcache can alias, we must blast it since mapping is changing. + * If executable, we must ensure any dirty lines are written back far + * enough to be visible to icache. + */ + if (cpu_has_dc_aliases || (exec && !cpu_has_ic_fills_f_dc)) + r4k_blast_dcache(); + /* If executable, blast stale lines from icache */ if (exec) r4k_blast_icache(); } @@ -502,7 +509,7 @@ static void r4k_flush_cache_range(struct vm_area_struct *vma, { int exec = vma->vm_flags & VM_EXEC; - if (cpu_has_dc_aliases || (exec && !cpu_has_ic_fills_f_dc)) + if (cpu_has_dc_aliases || exec) r4k_on_each_cpu(local_r4k_flush_cache_range, vma); } diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c index 7e5fa0938c21..6c6a843b0d17 100644 --- a/arch/mips/mm/init.c +++ b/arch/mips/mm/init.c @@ -196,7 +196,7 @@ void copy_to_user_page(struct vm_area_struct *vma, if (cpu_has_dc_aliases) SetPageDcacheDirty(page); } - if ((vma->vm_flags & VM_EXEC) && !cpu_has_ic_fills_f_dc) + if (vma->vm_flags & VM_EXEC) flush_cache_page(vma, vaddr, page_to_pfn(page)); } -- 2.4.10 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] MIPS: c-r4k: Sync icache when it fills from dcache 2016-01-22 10:58 ` James Hogan (?) @ 2016-01-22 12:06 ` Manuel Lauss 2016-01-22 12:19 ` James Hogan -1 siblings, 1 reply; 14+ messages in thread From: Manuel Lauss @ 2016-01-22 12:06 UTC (permalink / raw) To: James Hogan; +Cc: Ralf Baechle, Leonid Yegoshin, Linux-MIPS Hi James, On Fri, Jan 22, 2016 at 11:58 AM, James Hogan <james.hogan@imgtec.com> 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@imgtec.com> > Cc: Ralf Baechle <ralf@linux-mips.org> > Cc: Leonid Yegoshin <leonid.yegoshin@imgtec.com> > Cc: Manuel Lauss <manuel.lauss@gmail.com> > Cc: linux-mips@linux-mips.org > --- > 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@gmail.com> Thanks! Manuel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] MIPS: c-r4k: Sync icache when it fills from dcache 2016-01-22 12:06 ` Manuel Lauss @ 2016-01-22 12:19 ` James Hogan 2016-01-22 13:05 ` Joshua Kinard 2016-01-22 14:30 ` Manuel Lauss 0 siblings, 2 replies; 14+ messages in thread From: James Hogan @ 2016-01-22 12:19 UTC (permalink / raw) To: Manuel Lauss; +Cc: Ralf Baechle, Leonid Yegoshin, Linux-MIPS [-- Attachment #1.1: Type: text/plain, Size: 2280 bytes --] 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@imgtec.com> 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@imgtec.com> > > Cc: Ralf Baechle <ralf@linux-mips.org> > > Cc: Leonid Yegoshin <leonid.yegoshin@imgtec.com> > > Cc: Manuel Lauss <manuel.lauss@gmail.com> > > Cc: linux-mips@linux-mips.org > > --- > > 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@gmail.com> 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). Cheers James [-- Attachment #1.2: mprotect.c --] [-- Type: text/x-c, Size: 6033 bytes --] /* * Copyright (C) 2016 Imagination Technologies Ltd. * Author: James Hogan <james.hogan@imgtec.com> * * Test that mprotect keeps icache in sync. * I.e. * 1) mprotect of non-PROT_EXEC mmap() should sync icache. * 2) later mprotect RW->RX should sync icache. * 3) later mprotect RWX->RWX should sync icache. * * Linux man pages do not state what mprotect(2) does with caches. * * IRIX man pages suggest that its mprotect(2) causes cache flushing to take * place to allow for execution. * * The MIPS behaviour was fixed in Linux kernel commit 2eaa7ec286db ("[MIPS] * Handle I-cache coherency in flush_cache_range()"), to accomodate the MIPS16 * dynamic linker which would perform data relocations in a page also containing * code, allocated with mmap VM_READ|VM_WRITE, and then use mprotect(2) to make * it executable. Without the fix, stale icache lines could be used. */ #ifndef __mips__ #error This test only supports MIPS #endif #include <inttypes.h> #include <stdbool.h> #include <stdio.h> #include <stdlib.h> #include <string.h> #include <sys/cachectl.h> #include <sys/mman.h> #include <unistd.h> #define REG_ZERO 0 #define REG_V0 2 #define REG_RA 31 #define OPC_S 26 #define RS_S 21 #define RT_S 16 #define IMM_S 0 #define FUNC_S 0 #define SPECIAL (000u << OPC_S) #define ADDIU (011u << OPC_S) #define JALR (SPECIAL | (011U << FUNC_S)) #define MOV_V0(SIMM) (ADDIU | (REG_V0 << RT_S) | ((uint16_t)(SIMM) << IMM_S)) void usage(char *arg0, FILE *f) { fprintf(f, "Usage: %s <options>:\n" " -h : show this help text\n" " --[no-]loop1 : run/skip RW/RX mprotect loop (=run)\n" " --[no-]loop2 : run/skip RWX mprotect loop (=skip)\n" " --loops <num> : set number of loop iterations (=%u)\n", arg0, 0x10000); } int main(int argc, char **argv) { const int pagesize = getpagesize(); uint32_t *buf; int (*func)(void); unsigned int i; int ret; bool loop1 = true; bool loop2 = false; unsigned int max = 0xffff; for (i = 1; i < argc; ++i) { if (!strcmp(argv[i], "--loop1")) { loop1 = true; } else if (!strcmp(argv[i], "--no-loop1")) { loop1 = false; } else if (!strcmp(argv[i], "--loop2")) { loop2 = true; } else if (!strcmp(argv[i], "--no-loop2")) { loop2 = false; } else if (!strcmp(argv[i], "--loops")) { ++i; if (i >= argc) { fprintf(stderr, "--loops expects an argument\n\n"); goto bad_usage; } max = atoi(argv[i]) - 1; if (max > 0xffff) { fprintf(stderr, "--loops must be >= 1 and <= %u\n\n", 0x10000); goto bad_usage; } } else if (!strcmp(argv[i], "-h")) { usage(argv[0], stdout); return EXIT_SUCCESS; } else { bad_usage: usage(argv[0], stderr); return EXIT_FAILURE; } } /* Map a page where we can generate some code. */ buf = mmap(NULL, pagesize, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, 0, 0); if (buf == MAP_FAILED) { perror("mmap"); return EXIT_FAILURE; } func = (void *)buf; /* * Write a simple function returning a value: * jr ra * addiu v0, zero, -1 */ buf[0] = JALR | (REG_RA << RS_S); buf[1] = MOV_V0(0xffff); /* * Flush cache immediately, so we can fail more gracefully if mprotect * doesn't work. */ ret = cacheflush(buf, sizeof(buf[0]) * 2, BCACHE); if (ret < 0) { perror("cacheflush"); return EXIT_FAILURE; } /* Check mprotect flushes icache. */ /* Make function return 0. */ buf[1] = MOV_V0(0); /* Make the page executable. */ ret = mprotect(buf, pagesize, PROT_READ|PROT_EXEC); if (ret < 0) { perror("initial mprotect\n"); return EXIT_FAILURE; } /* Check the return value. */ ret = func(); if (ret != 0) { fprintf(stderr, "%s:%u: First mprotect(2) PROT_READ|PROT_EXEC didn't sync icache, ret=%x\n", __FILE__, __LINE__, ret); return EXIT_FAILURE; } printf("Initial mprotect SUCCESS\n"); if (loop1) { /* Lets test mprotect(RW), modify, mprotect(RX) a bit more. */ for (i = 0; i <= max; ++i) { /* Make the page writable, non-executable. */ ret = mprotect(buf, pagesize, PROT_READ|PROT_WRITE); if (ret < 0) { perror("mprotect PROT_READ|PROT_WRITE\n"); return EXIT_FAILURE; } /* Make the function return (int16_t)i. */ buf[1] = MOV_V0(i); /* Make the page executable. */ ret = mprotect(buf, pagesize, PROT_READ|PROT_EXEC); if (ret < 0) { perror("mprotect PROT_READ|PROT_EXEC\n"); return EXIT_FAILURE; } /* Check the return value. */ ret = func(); if (ret != (int16_t)i) { fprintf(stderr, "%s:%u: Looped mprotect(2) PROT_READ|PROT_EXEC didn't sync icache, ret=%x, expected %x\n", __FILE__, __LINE__, ret, (int16_t)i); return EXIT_FAILURE; } } printf("Looped { mprotect RW, modify, mprotect RX, test } SUCCESS\n"); } /* * This loop is disabled by default, because Linux detects that the * flags haven't changed and does nothing. */ if (loop2) { /* Make the page writable AND executable. */ ret = mprotect(buf, pagesize, PROT_READ|PROT_WRITE|PROT_EXEC); if (ret < 0) { perror("initial mprotect PROT_READ|PROT_WRITE|PROT_EXEC\n"); return EXIT_FAILURE; } /* Lets test modify while PROT_EXEC. */ for (i = 0; i <= max; ++i) { /* Make the function return (int16_t)i. */ buf[1] = MOV_V0(i); /* Remind kernel of executability. */ ret = mprotect(buf, pagesize, PROT_READ|PROT_WRITE|PROT_EXEC); if (ret < 0) { perror("mprotect PROT_READ|PROT_WRITE|PROT_EXEC\n"); return EXIT_FAILURE; } /* Check the return value. */ ret = func(); if (ret != (int16_t)i) { /* This is expected under Linux, see above. */ fprintf(stderr, "%s:%u: Looped mprotect(2) PROT_READ|PROT_WRITE|PROT_EXEC didn't sync icache, ret=%x, expected %x\n", __FILE__, __LINE__, ret, (int16_t)i); return EXIT_FAILURE; } } printf("Looped { modify, mprotect RWX, test } SUCCESS\n"); } /* Clean up */ ret = munmap(buf, pagesize); if (ret < 0) { perror("munmap\n"); return EXIT_FAILURE; } return EXIT_SUCCESS; } [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] MIPS: c-r4k: Sync icache when it fills from dcache 2016-01-22 12:19 ` James Hogan @ 2016-01-22 13:05 ` Joshua Kinard 2016-01-22 13:54 ` James Hogan 2016-01-22 14:03 ` Ralf Baechle 2016-01-22 14:30 ` Manuel Lauss 1 sibling, 2 replies; 14+ messages in thread From: Joshua Kinard @ 2016-01-22 13:05 UTC (permalink / raw) To: James Hogan, Manuel Lauss; +Cc: Ralf Baechle, Leonid Yegoshin, Linux-MIPS On 01/22/2016 07:19, James Hogan wrote: > On Fri, Jan 22, 2016 at 01:06:14PM +0100, Manuel Lauss wrote: >> Hi James, [snip] > > 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). Here's the output on my Octane, R14000 CPU (mips4): # ./mprotect Initial mprotect SUCCESS Looped { mprotect RW, modify, mprotect RX, test } SUCCESS This is without your patch applied. That look good? I'm assuming this CPU is too old to be affected. I can test with your patch after the blizzard is over later this weekend. -- Joshua Kinard Gentoo/MIPS kumba@gentoo.org 6144R/F5C6C943 2015-04-27 177C 1972 1FB8 F254 BAD0 3E72 5C63 F4E3 F5C6 C943 "The past tempts us, the present confuses us, the future frightens us. And our lives slip away, moment by moment, lost in that vast, terrible in-between." --Emperor Turhan, Centauri Republic ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] MIPS: c-r4k: Sync icache when it fills from dcache 2016-01-22 13:05 ` Joshua Kinard @ 2016-01-22 13:54 ` James Hogan 2016-01-22 14:03 ` Ralf Baechle 1 sibling, 0 replies; 14+ messages in thread From: James Hogan @ 2016-01-22 13:54 UTC (permalink / raw) To: Joshua Kinard; +Cc: Manuel Lauss, Ralf Baechle, Leonid Yegoshin, Linux-MIPS [-- Attachment #1: Type: text/plain, Size: 1106 bytes --] Hi Joshua, On Fri, Jan 22, 2016 at 08:05:58AM -0500, Joshua Kinard wrote: > On 01/22/2016 07:19, James Hogan wrote: > > On Fri, Jan 22, 2016 at 01:06:14PM +0100, Manuel Lauss wrote: > >> Hi James, > [snip] > > > > 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). > > Here's the output on my Octane, R14000 CPU (mips4): > > # ./mprotect > Initial mprotect SUCCESS > Looped { mprotect RW, modify, mprotect RX, test } SUCCESS > > This is without your patch applied. That look good? Yep, that looks good. > I'm assuming this CPU is too old to be affected. I can test with your > patch after the blizzard is over later this weekend. Unless you set MIPS_CACHE_IC_F_DC for your core (only ALCHEMY does, until patch 2 adds I6400 to the list), then cpu_has_ic_fills_f_dc == 0 and everything should already work, as evidenced by the successful result. Thanks James [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] MIPS: c-r4k: Sync icache when it fills from dcache 2016-01-22 13:05 ` Joshua Kinard 2016-01-22 13:54 ` James Hogan @ 2016-01-22 14:03 ` Ralf Baechle 1 sibling, 0 replies; 14+ messages in thread From: Ralf Baechle @ 2016-01-22 14:03 UTC (permalink / raw) To: Joshua Kinard; +Cc: James Hogan, Manuel Lauss, Leonid Yegoshin, Linux-MIPS On Fri, Jan 22, 2016 at 08:05:58AM -0500, Joshua Kinard wrote: > > 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). > > Here's the output on my Octane, R14000 CPU (mips4): > > # ./mprotect > Initial mprotect SUCCESS > Looped { mprotect RW, modify, mprotect RX, test } SUCCESS > > This is without your patch applied. That look good? I'm assuming this CPU is > too old to be affected. I can test with your patch after the blizzard is over > later this weekend. The R10000 family refills the I-cache from the S-cache so is not affected by the scenario James' patch is for. Ralf ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] MIPS: c-r4k: Sync icache when it fills from dcache 2016-01-22 12:19 ` James Hogan 2016-01-22 13:05 ` Joshua Kinard @ 2016-01-22 14:30 ` Manuel Lauss 2016-01-22 15:02 ` James Hogan 1 sibling, 1 reply; 14+ messages in thread From: Manuel Lauss @ 2016-01-22 14:30 UTC (permalink / raw) To: James Hogan; +Cc: Ralf Baechle, Leonid Yegoshin, Linux-MIPS On Fri, Jan 22, 2016 at 1:19 PM, James Hogan <james.hogan@imgtec.com> 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@imgtec.com> 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@imgtec.com> >> > Cc: Ralf Baechle <ralf@linux-mips.org> >> > Cc: Leonid Yegoshin <leonid.yegoshin@imgtec.com> >> > Cc: Manuel Lauss <manuel.lauss@gmail.com> >> > Cc: linux-mips@linux-mips.org >> > --- >> > 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@gmail.com> > > 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] MIPS: c-r4k: Sync icache when it fills from dcache 2016-01-22 14:30 ` Manuel Lauss @ 2016-01-22 15:02 ` James Hogan 2016-01-22 15:55 ` Manuel Lauss 0 siblings, 1 reply; 14+ messages in thread From: James Hogan @ 2016-01-22 15:02 UTC (permalink / raw) To: Manuel Lauss; +Cc: Ralf Baechle, Leonid Yegoshin, Linux-MIPS [-- Attachment #1: Type: text/plain, Size: 3546 bytes --] On Fri, Jan 22, 2016 at 03:30:06PM +0100, Manuel Lauss wrote: > On Fri, Jan 22, 2016 at 1:19 PM, James Hogan <james.hogan@imgtec.com> 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@imgtec.com> 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@imgtec.com> > >> > Cc: Ralf Baechle <ralf@linux-mips.org> > >> > Cc: Leonid Yegoshin <leonid.yegoshin@imgtec.com> > >> > Cc: Manuel Lauss <manuel.lauss@gmail.com> > >> > Cc: linux-mips@linux-mips.org > >> > --- > >> > 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@gmail.com> > > > > 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 Interesting... I suppose at least it brings alchemy in line with behaviour on other cores. > it does fix > occasional (5 out of 20 runs) failures. How big is your icache? With 64KB icache it fails fairly consistently for me, but wouldn't surprise me if smaller caches would make it much harder to hit, especially as it only tests the first cache line in the page. > The --loop3 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 Yes, thats expected. Generic code detects that flags haven't actually changed and doesn't do anything, so icache never gets flushed. I disabled but didn't remove that loop. Thanks a lot for testing! Cheers James [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] MIPS: c-r4k: Sync icache when it fills from dcache 2016-01-22 15:02 ` James Hogan @ 2016-01-22 15:55 ` Manuel Lauss 0 siblings, 0 replies; 14+ messages in thread From: Manuel Lauss @ 2016-01-22 15:55 UTC (permalink / raw) To: James Hogan; +Cc: Ralf Baechle, Leonid Yegoshin, Linux-MIPS On Fri, Jan 22, 2016 at 4:02 PM, James Hogan <james.hogan@imgtec.com> wrote: > On Fri, Jan 22, 2016 at 03:30:06PM +0100, Manuel Lauss wrote: >> On Fri, Jan 22, 2016 at 1:19 PM, James Hogan <james.hogan@imgtec.com> 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@imgtec.com> 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@imgtec.com> >> >> > Cc: Ralf Baechle <ralf@linux-mips.org> >> >> > Cc: Leonid Yegoshin <leonid.yegoshin@imgtec.com> >> >> > Cc: Manuel Lauss <manuel.lauss@gmail.com> >> >> > Cc: linux-mips@linux-mips.org >> >> > --- >> >> > 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@gmail.com> >> > >> > 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 > > Interesting... I suppose at least it brings alchemy in line with > behaviour on other cores. > >> it does fix >> occasional (5 out of 20 runs) failures. > > How big is your icache? icache has16kB > With 64KB icache it fails fairly consistently for me, but wouldn't > surprise me if smaller caches would make it much harder to hit, > especially as it only tests the first cache line in the page. Thanks! Manuel ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/2] MIPS: I6400: Icache fills from dcache @ 2016-01-22 10:58 ` James Hogan 0 siblings, 0 replies; 14+ messages in thread From: James Hogan @ 2016-01-22 10:58 UTC (permalink / raw) To: Ralf Baechle; +Cc: Manuel Lauss, James Hogan, Leonid Yegoshin, linux-mips Coherence Manager 3 (CM3) as present in I6400 can fill icache lines effectively from dirty dcaches, so there is no need to flush dirty lines from dcaches through to L2 prior to icache invalidation. Set the MIPS_CACHE_IC_F_DC flag such that cpu_has_ic_fills_f_dc evaluates to true, which avoids those dcache flushes. Signed-off-by: James Hogan <james.hogan@imgtec.com> Cc: Ralf Baechle <ralf@linux-mips.org> Cc: Leonid Yegoshin <leonid.yegoshin@imgtec.com> Cc: linux-mips@linux-mips.org --- arch/mips/mm/c-r4k.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/mips/mm/c-r4k.c b/arch/mips/mm/c-r4k.c index fc7289dfaf5a..69e7e5873af3 100644 --- a/arch/mips/mm/c-r4k.c +++ b/arch/mips/mm/c-r4k.c @@ -1311,6 +1311,7 @@ static void probe_pcache(void) break; case CPU_ALCHEMY: + case CPU_I6400: c->icache.flags |= MIPS_CACHE_IC_F_DC; break; -- 2.4.10 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/2] MIPS: I6400: Icache fills from dcache @ 2016-01-22 10:58 ` James Hogan 0 siblings, 0 replies; 14+ messages in thread From: James Hogan @ 2016-01-22 10:58 UTC (permalink / raw) To: Ralf Baechle; +Cc: Manuel Lauss, James Hogan, Leonid Yegoshin, linux-mips Coherence Manager 3 (CM3) as present in I6400 can fill icache lines effectively from dirty dcaches, so there is no need to flush dirty lines from dcaches through to L2 prior to icache invalidation. Set the MIPS_CACHE_IC_F_DC flag such that cpu_has_ic_fills_f_dc evaluates to true, which avoids those dcache flushes. Signed-off-by: James Hogan <james.hogan@imgtec.com> Cc: Ralf Baechle <ralf@linux-mips.org> Cc: Leonid Yegoshin <leonid.yegoshin@imgtec.com> Cc: linux-mips@linux-mips.org --- arch/mips/mm/c-r4k.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/mips/mm/c-r4k.c b/arch/mips/mm/c-r4k.c index fc7289dfaf5a..69e7e5873af3 100644 --- a/arch/mips/mm/c-r4k.c +++ b/arch/mips/mm/c-r4k.c @@ -1311,6 +1311,7 @@ static void probe_pcache(void) break; case CPU_ALCHEMY: + case CPU_I6400: c->icache.flags |= MIPS_CACHE_IC_F_DC; break; -- 2.4.10 ^ permalink raw reply related [flat|nested] 14+ messages in thread
end of thread, other threads:[~2016-01-22 15:56 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-01-22 10:58 [PATCH 0/2] MIPS: I6400: Avoid dcache flushes James Hogan 2016-01-22 10:58 ` James Hogan 2016-01-22 10:58 ` [PATCH 1/2] MIPS: c-r4k: Sync icache when it fills from dcache James Hogan 2016-01-22 10:58 ` James Hogan 2016-01-22 12:06 ` Manuel Lauss 2016-01-22 12:19 ` James Hogan 2016-01-22 13:05 ` Joshua Kinard 2016-01-22 13:54 ` James Hogan 2016-01-22 14:03 ` Ralf Baechle 2016-01-22 14:30 ` Manuel Lauss 2016-01-22 15:02 ` James Hogan 2016-01-22 15:55 ` Manuel Lauss 2016-01-22 10:58 ` [PATCH 2/2] MIPS: I6400: Icache " James Hogan 2016-01-22 10:58 ` James Hogan
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.