[PATCH 0/2] MIPS: I6400: Avoid dcache flushes

[PATCH 0/2] MIPS: I6400: Avoid dcache flushes

[James Hogan]

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

[PATCH 0/2] MIPS: I6400: Avoid dcache flushes

[James Hogan]

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

[PATCH 1/2] MIPS: c-r4k: Sync icache when it fills from dcache

[James Hogan]

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

[PATCH 1/2] MIPS: c-r4k: Sync icache when it fills from dcache

[James Hogan]

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

[PATCH 2/2] MIPS: I6400: Icache fills from dcache

[James Hogan]

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

[PATCH 2/2] MIPS: I6400: Icache fills from dcache

[James Hogan]

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

Re: [PATCH 1/2] MIPS: c-r4k: Sync icache when it fills from dcache

[Manuel Lauss]

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

Re: [PATCH 1/2] MIPS: c-r4k: Sync icache when it fills from dcache

[James Hogan]

[-- 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 --]

Re: [PATCH 1/2] MIPS: c-r4k: Sync icache when it fills from dcache

[Joshua Kinard]

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

Re: [PATCH 1/2] MIPS: c-r4k: Sync icache when it fills from dcache

[James Hogan]

[-- 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 --]

Re: [PATCH 1/2] MIPS: c-r4k: Sync icache when it fills from dcache

[Ralf Baechle]

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

Re: [PATCH 1/2] MIPS: c-r4k: Sync icache when it fills from dcache

[Manuel Lauss]

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

Re: [PATCH 1/2] MIPS: c-r4k: Sync icache when it fills from dcache

[James Hogan]

[-- 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 --]

Re: [PATCH 1/2] MIPS: c-r4k: Sync icache when it fills from dcache

[Manuel Lauss]

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