[PATCH 0/9] MIPS: General EVA fixes & cleanups

[PATCH 0/9] MIPS: General EVA fixes & cleanups

[James Hogan]

These patches fix some general MIPS Enhanced Virtual Addressing (EVA)
issues, with the aim of allowing KVM to be fixed to work on EVA host
kernels.

Patches 1-3 improve the CP0_EBase handling, particularly in relation to
the Write Gate (WG) bit which allows the upper bits (63:30 on MIPS64,
31:30 on MIPS32) to be modified. This allows CP0_EBase to be set
correctly with EVA, even when the boot time allocated exception vector
is not in KSeg0. They will also help with Matt's upcoming rproc patches.

Patch 4 then drops the EVA specific L2 cache flushing from
flush_icache_range(), which appeared to work around the partial
CP0_EBase assignment fixed by patch 3.

Patches 5-9 fix the semantics of flush_icache_range(), which only works
on user pointers with EVA. We add a new __flush_icache_user_range() API
in patch 5, fix users of flush_icache_range() with user pointers in
patches 6-8, and finally separate the implementations so that
flush_icache_range() works with kernel addresses in patch 9.

Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Matt Redfearn <matt.redfearn@imgtec.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Radim Krčmář" <rkrcmar@redhat.com>
Cc: Leonid Yegoshin <leonid.yegoshin@imgtec.com>
Cc: linux-mips@linux-mips.org
Cc: kvm@vger.kernel.org

James Hogan (8):
  MIPS: traps: 64bit kernels should read CP0_EBase 64bit
  MIPS: traps: Convert ebase to KSeg0
  MIPS: c-r4k: Drop bc_wback_inv() from icache flush
  MIPS: c-r4k: Split user/kernel flush_icache_range()
  MIPS: cacheflush: Use __flush_icache_user_range()
  MIPS: uprobes: Flush icache via kernel address
  MIPS: KVM: Use __local_flush_icache_user_range()
  MIPS: c-r4k: Fix flush_icache_range() for EVA

Matt Redfearn (1):
  MIPS: traps: Ensure full EBase is written

 arch/mips/include/asm/cacheflush.h |  5 +++-
 arch/mips/kernel/traps.c           | 49 +++++++++++++++++++++++++++--
 arch/mips/kernel/uprobes.c         | 13 ++------
 arch/mips/kvm/dyntrans.c           |  4 +-
 arch/mips/mm/c-octeon.c            |  2 +-
 arch/mips/mm/c-r3k.c               |  2 +-
 arch/mips/mm/c-r4k.c               | 52 ++++++++++++++++++++-----------
 arch/mips/mm/c-tx39.c              |  3 ++-
 arch/mips/mm/cache.c               |  6 +++-
 9 files changed, 104 insertions(+), 32 deletions(-)

-- 
git-series 0.8.10

[PATCH 1/9] MIPS: traps: 64bit kernels should read CP0_EBase 64bit

[James Hogan]

When reading the CP0_EBase register containing the WG (write gate) bit,
the ebase variable should be set to the full value of the register, i.e.
on a 64-bit kernel the full 64-bit width of the register via
read_cp0_ebase_64(), and on a 32-bit kernel the full 32-bit width
including bits 31:30 which may be writeable.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Signed-off-by: Matt Redfearn <matt.redfearn@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
---
 arch/mips/kernel/traps.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
index 3de85be2486a..686903f62fa3 100644
--- a/arch/mips/kernel/traps.c
+++ b/arch/mips/kernel/traps.c
@@ -2214,8 +2214,17 @@ void __init trap_init(void)
 	} else {
 		ebase = CAC_BASE;
 
-		if (cpu_has_mips_r2_r6)
-			ebase += (read_c0_ebase() & 0x3ffff000);
+		if (cpu_has_mips_r2_r6) {
+			if (cpu_has_ebase_wg) {
+#ifdef CONFIG_64BIT
+				ebase = (read_c0_ebase_64() & ~0xfff);
+#else
+				ebase = (read_c0_ebase() & ~0xfff);
+#endif
+			} else {
+				ebase += (read_c0_ebase() & 0x3ffff000);
+			}
+		}
 	}
 
 	if (cpu_has_mmips) {
-- 
git-series 0.8.10

[PATCH 1/9] MIPS: traps: 64bit kernels should read CP0_EBase 64bit

[James Hogan]

When reading the CP0_EBase register containing the WG (write gate) bit,
the ebase variable should be set to the full value of the register, i.e.
on a 64-bit kernel the full 64-bit width of the register via
read_cp0_ebase_64(), and on a 32-bit kernel the full 32-bit width
including bits 31:30 which may be writeable.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Signed-off-by: Matt Redfearn <matt.redfearn@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
---
 arch/mips/kernel/traps.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
index 3de85be2486a..686903f62fa3 100644
--- a/arch/mips/kernel/traps.c
+++ b/arch/mips/kernel/traps.c
@@ -2214,8 +2214,17 @@ void __init trap_init(void)
 	} else {
 		ebase = CAC_BASE;
 
-		if (cpu_has_mips_r2_r6)
-			ebase += (read_c0_ebase() & 0x3ffff000);
+		if (cpu_has_mips_r2_r6) {
+			if (cpu_has_ebase_wg) {
+#ifdef CONFIG_64BIT
+				ebase = (read_c0_ebase_64() & ~0xfff);
+#else
+				ebase = (read_c0_ebase() & ~0xfff);
+#endif
+			} else {
+				ebase += (read_c0_ebase() & 0x3ffff000);
+			}
+		}
 	}
 
 	if (cpu_has_mmips) {
-- 
git-series 0.8.10

Re: [PATCH 1/9] MIPS: traps: 64bit kernels should read CP0_EBase 64bit

[Ralf Baechle]

On Thu, Sep 01, 2016 at 05:30:07PM +0100, James Hogan wrote:

> When reading the CP0_EBase register containing the WG (write gate) bit,
> the ebase variable should be set to the full value of the register, i.e.
> on a 64-bit kernel the full 64-bit width of the register via
> read_cp0_ebase_64(), and on a 32-bit kernel the full 32-bit width
> including bits 31:30 which may be writeable.

How about changing the definition of read/write_c0_ebase to

#define read_c0_ebase()         __read_ulong_c0_register($15, 1)
#define write_c0_ebase(val)     __write_ulong_c0_register($15, 1, val)

or using a new variant like

#define read_c0_ebase_ulong()         __read_ulong_c0_register($15, 1)
#define write_c0_ebase_ulong(val)     __write_ulong_c0_register($15, 1, val)

to avoid the ifdefery?  This could also make this bit

                ebase = cpu_has_mips64r6 ? read_c0_ebase_64()
                                         : (s32)read_c0_ebase();

in cpu-probe.c prettier.

  Ralf

Re: [PATCH 1/9] MIPS: traps: 64bit kernels should read CP0_EBase 64bit

[Matt Redfearn]

Hi Ralf,


On 21/09/16 14:08, Ralf Baechle wrote:
> On Thu, Sep 01, 2016 at 05:30:07PM +0100, James Hogan wrote:
>
>> When reading the CP0_EBase register containing the WG (write gate) bit,
>> the ebase variable should be set to the full value of the register, i.e.
>> on a 64-bit kernel the full 64-bit width of the register via
>> read_cp0_ebase_64(), and on a 32-bit kernel the full 32-bit width
>> including bits 31:30 which may be writeable.
> How about changing the definition of read/write_c0_ebase to
>
> #define read_c0_ebase()         __read_ulong_c0_register($15, 1)
> #define write_c0_ebase(val)     __write_ulong_c0_register($15, 1, val)

James added the {read,write}_c0_ebase_64 functions in 
37fb60f8e3f011c25c120081a73886ad8dbc42fd, because performing a 64bit 
access to 32bit cp0 registers (like ebase on 32bit cpus) was an 
undefined operation pre-r6, so we can't always access them as longs.

>
> or using a new variant like
>
> #define read_c0_ebase_ulong()         __read_ulong_c0_register($15, 1)
> #define write_c0_ebase_ulong(val)     __write_ulong_c0_register($15, 1, val)
>
> to avoid the ifdefery?  This could also make this bit
>
>                  ebase = cpu_has_mips64r6 ? read_c0_ebase_64()
>                                           : (s32)read_c0_ebase();

This relies on being able to determine a 64bit value for ebase, either 
by reading it in its entirety on a 64bit cpu (including on a 32bit 
kernel) or sign extending it from a 32bit read.

Thanks,
Matt

>
> in cpu-probe.c prettier.
>
>    Ralf

Re: [PATCH 1/9] MIPS: traps: 64bit kernels should read CP0_EBase 64bit

[Matt Redfearn]

Hi Ralf,


On 21/09/16 14:08, Ralf Baechle wrote:
> On Thu, Sep 01, 2016 at 05:30:07PM +0100, James Hogan wrote:
>
>> When reading the CP0_EBase register containing the WG (write gate) bit,
>> the ebase variable should be set to the full value of the register, i.e.
>> on a 64-bit kernel the full 64-bit width of the register via
>> read_cp0_ebase_64(), and on a 32-bit kernel the full 32-bit width
>> including bits 31:30 which may be writeable.
> How about changing the definition of read/write_c0_ebase to
>
> #define read_c0_ebase()         __read_ulong_c0_register($15, 1)
> #define write_c0_ebase(val)     __write_ulong_c0_register($15, 1, val)

James added the {read,write}_c0_ebase_64 functions in 
37fb60f8e3f011c25c120081a73886ad8dbc42fd, because performing a 64bit 
access to 32bit cp0 registers (like ebase on 32bit cpus) was an 
undefined operation pre-r6, so we can't always access them as longs.

>
> or using a new variant like
>
> #define read_c0_ebase_ulong()         __read_ulong_c0_register($15, 1)
> #define write_c0_ebase_ulong(val)     __write_ulong_c0_register($15, 1, val)
>
> to avoid the ifdefery?  This could also make this bit
>
>                  ebase = cpu_has_mips64r6 ? read_c0_ebase_64()
>                                           : (s32)read_c0_ebase();

This relies on being able to determine a 64bit value for ebase, either 
by reading it in its entirety on a 64bit cpu (including on a 32bit 
kernel) or sign extending it from a 32bit read.

Thanks,
Matt

>
> in cpu-probe.c prettier.
>
>    Ralf

Re: [PATCH 1/9] MIPS: traps: 64bit kernels should read CP0_EBase 64bit

[Maciej W. Rozycki]

On Wed, 21 Sep 2016, Matt Redfearn wrote:

> > > When reading the CP0_EBase register containing the WG (write gate) bit,
> > > the ebase variable should be set to the full value of the register, i.e.
> > > on a 64-bit kernel the full 64-bit width of the register via
> > > read_cp0_ebase_64(), and on a 32-bit kernel the full 32-bit width
> > > including bits 31:30 which may be writeable.
> > How about changing the definition of read/write_c0_ebase to
> >
> > #define read_c0_ebase()         __read_ulong_c0_register($15, 1)
> > #define write_c0_ebase(val)     __write_ulong_c0_register($15, 1, val)
> 
> James added the {read,write}_c0_ebase_64 functions in
> 37fb60f8e3f011c25c120081a73886ad8dbc42fd, because performing a 64bit access to
> 32bit cp0 registers (like ebase on 32bit cpus) was an undefined operation
> pre-r6, so we can't always access them as longs.

 Well, `long' is 32-bit with 32-bit processors, however in older (as in: 
before 3.50) architecture revisions EBase was 32-bit even with 64-bit 
processors, so I take it you meant "like ebase on 64bit cpus", right?

> > or using a new variant like
> >
> > #define read_c0_ebase_ulong()         __read_ulong_c0_register($15, 1)
> > #define write_c0_ebase_ulong(val)     __write_ulong_c0_register($15, 1, val)
> >
> > to avoid the ifdefery?  This could also make this bit
> >
> >                  ebase = cpu_has_mips64r6 ? read_c0_ebase_64()
> > : (s32)read_c0_ebase();
> 
> This relies on being able to determine a 64bit value for ebase, either by
> reading it in its entirety on a 64bit cpu (including on a 32bit kernel) or sign
> extending it from a 32bit read.

 This does look wrong to me, as I noted above EBase is 64-bit with MIPS64 
processors as from architecture revision 3.50.  Also I don't think we want 
to have EBase set to outside the 32-bit address space with 32-bit kernels.

 So Ralf's proposal is actually close to being right, except for the 
condition.  I'd also move the condition to the macro definition itself so 
that it doesn't have to be repeated inline, making the whole piece in 
question look like:

#define read_c0_ebase		(cpu_has_ebase_wg ?
				 __read_ulong_c0_register($15, 1) :
				 __read_32bit_c0_register($15, 1))

	if (cpu_has_veic || cpu_has_vint) {
		unsigned long size = 0x200 + VECTORSPACING*64;
		ebase = (unsigned long)
		__alloc_bootmem(size, 1 << fls(size), 0);
        } else if (cpu_has_mips_r2_r6) {
		ebase = read_c0_ebase() & ~0xfff;
	} else {
		ebase = CAC_BASE;
	}

-- short and sweet, isn't it?

  Maciej

Re: [PATCH 1/9] MIPS: traps: 64bit kernels should read CP0_EBase 64bit

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 2138 bytes --]

Hi Maciej,

On Sun, Oct 02, 2016 at 11:30:13AM +0100, Maciej W. Rozycki wrote:
> On Wed, 21 Sep 2016, Matt Redfearn wrote:
> 
> > > > When reading the CP0_EBase register containing the WG (write gate) bit,
> > > > the ebase variable should be set to the full value of the register, i.e.
> > > > on a 64-bit kernel the full 64-bit width of the register via
> > > > read_cp0_ebase_64(), and on a 32-bit kernel the full 32-bit width
> > > > including bits 31:30 which may be writeable.
> > > How about changing the definition of read/write_c0_ebase to
> > >
> > > #define read_c0_ebase()         __read_ulong_c0_register($15, 1)
> > > #define write_c0_ebase(val)     __write_ulong_c0_register($15, 1, val)
> > 
> > James added the {read,write}_c0_ebase_64 functions in
> > 37fb60f8e3f011c25c120081a73886ad8dbc42fd, because performing a 64bit access to
> > 32bit cp0 registers (like ebase on 32bit cpus) was an undefined operation
> > pre-r6, so we can't always access them as longs.
> 
>  Well, `long' is 32-bit with 32-bit processors, however in older (as in: 
> before 3.50) architecture revisions EBase was 32-bit even with 64-bit 
> processors,
> so I take it you meant "like ebase on 64bit cpus", right?
> 
> > > or using a new variant like
> > >
> > > #define read_c0_ebase_ulong()         __read_ulong_c0_register($15, 1)
> > > #define write_c0_ebase_ulong(val)     __write_ulong_c0_register($15, 1, val)
> > >
> > > to avoid the ifdefery?  This could also make this bit
> > >
> > >                  ebase = cpu_has_mips64r6 ? read_c0_ebase_64()
> > > : (s32)read_c0_ebase();
> > 
> > This relies on being able to determine a 64bit value for ebase, either by
> > reading it in its entirety on a 64bit cpu (including on a 32bit kernel) or sign
> > extending it from a 32bit read.
> 
>  This does look wrong to me, as I noted above EBase is 64-bit with MIPS64 
> processors as from architecture revision 3.50.  Also I don't think we want 

MIPS64 PRA (I'm looking at r5 and r6) seems to allow for write-gate not
to be implemented, in which case the register is only 32-bits.

Cheers
James

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/9] MIPS: traps: 64bit kernels should read CP0_EBase 64bit

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 2138 bytes --]

Hi Maciej,

On Sun, Oct 02, 2016 at 11:30:13AM +0100, Maciej W. Rozycki wrote:
> On Wed, 21 Sep 2016, Matt Redfearn wrote:
> 
> > > > When reading the CP0_EBase register containing the WG (write gate) bit,
> > > > the ebase variable should be set to the full value of the register, i.e.
> > > > on a 64-bit kernel the full 64-bit width of the register via
> > > > read_cp0_ebase_64(), and on a 32-bit kernel the full 32-bit width
> > > > including bits 31:30 which may be writeable.
> > > How about changing the definition of read/write_c0_ebase to
> > >
> > > #define read_c0_ebase()         __read_ulong_c0_register($15, 1)
> > > #define write_c0_ebase(val)     __write_ulong_c0_register($15, 1, val)
> > 
> > James added the {read,write}_c0_ebase_64 functions in
> > 37fb60f8e3f011c25c120081a73886ad8dbc42fd, because performing a 64bit access to
> > 32bit cp0 registers (like ebase on 32bit cpus) was an undefined operation
> > pre-r6, so we can't always access them as longs.
> 
>  Well, `long' is 32-bit with 32-bit processors, however in older (as in: 
> before 3.50) architecture revisions EBase was 32-bit even with 64-bit 
> processors,
> so I take it you meant "like ebase on 64bit cpus", right?
> 
> > > or using a new variant like
> > >
> > > #define read_c0_ebase_ulong()         __read_ulong_c0_register($15, 1)
> > > #define write_c0_ebase_ulong(val)     __write_ulong_c0_register($15, 1, val)
> > >
> > > to avoid the ifdefery?  This could also make this bit
> > >
> > >                  ebase = cpu_has_mips64r6 ? read_c0_ebase_64()
> > > : (s32)read_c0_ebase();
> > 
> > This relies on being able to determine a 64bit value for ebase, either by
> > reading it in its entirety on a 64bit cpu (including on a 32bit kernel) or sign
> > extending it from a 32bit read.
> 
>  This does look wrong to me, as I noted above EBase is 64-bit with MIPS64 
> processors as from architecture revision 3.50.  Also I don't think we want 

MIPS64 PRA (I'm looking at r5 and r6) seems to allow for write-gate not
to be implemented, in which case the register is only 32-bits.

Cheers
James

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/9] MIPS: traps: 64bit kernels should read CP0_EBase 64bit

[Maciej W. Rozycki]

Hi James,

> >  This does look wrong to me, as I noted above EBase is 64-bit with MIPS64 
> > processors as from architecture revision 3.50.  Also I don't think we want 
> 
> MIPS64 PRA (I'm looking at r5 and r6) seems to allow for write-gate not
> to be implemented, in which case the register is only 32-bits.

 Indeed, but we need to be prepared to handle the width of 64 bits and 
`cpu_has_mips64r6' does not seem to me to be the right condition.

 ISTR a while ago we had a rather lengthy discussion as to how to detect 
the presence of the upper 32 bits without triggering undefined behaviour 
implied by 64-bit CP0 accesses to 32-bit CP0 registers.  As I believe we 
set EBase ourselves I think we are able to make the necessary checks and 
have an accurate condition here, still remembering however that it may go 
back as far as MIPSr3.

  Maciej

Re: [PATCH 1/9] MIPS: traps: 64bit kernels should read CP0_EBase 64bit

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 1314 bytes --]

On Thu, Oct 06, 2016 at 05:18:26PM +0100, Maciej W. Rozycki wrote:
> Hi James,
> 
> > >  This does look wrong to me, as I noted above EBase is 64-bit with MIPS64 
> > > processors as from architecture revision 3.50.  Also I don't think we want 
> > 
> > MIPS64 PRA (I'm looking at r5 and r6) seems to allow for write-gate not
> > to be implemented, in which case the register is only 32-bits.
> 
>  Indeed, but we need to be prepared to handle the width of 64 bits and 
> `cpu_has_mips64r6' does not seem to me to be the right condition.

The relevance of r6 is the assurance that reading a 32-bit COP0 register
with dmfc0 is no longer UNDEFINED (like r5 and before), but reads the
top 32-bits as reserved, i.e. read zero (may need manual sign
extension) and writes ignored.

> 
>  ISTR a while ago we had a rather lengthy discussion as to how to detect 
> the presence of the upper 32 bits without triggering undefined behaviour 
> implied by 64-bit CP0 accesses to 32-bit CP0 registers.  As I believe we 
> set EBase ourselves I think we are able to make the necessary checks and 
> have an accurate condition here, still remembering however that it may go 
> back as far as MIPSr3.

We only set ebase under certain circumstances, otherwise leaving it as
already set.

Cheers
James

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/9] MIPS: traps: 64bit kernels should read CP0_EBase 64bit

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 1314 bytes --]

On Thu, Oct 06, 2016 at 05:18:26PM +0100, Maciej W. Rozycki wrote:
> Hi James,
> 
> > >  This does look wrong to me, as I noted above EBase is 64-bit with MIPS64 
> > > processors as from architecture revision 3.50.  Also I don't think we want 
> > 
> > MIPS64 PRA (I'm looking at r5 and r6) seems to allow for write-gate not
> > to be implemented, in which case the register is only 32-bits.
> 
>  Indeed, but we need to be prepared to handle the width of 64 bits and 
> `cpu_has_mips64r6' does not seem to me to be the right condition.

The relevance of r6 is the assurance that reading a 32-bit COP0 register
with dmfc0 is no longer UNDEFINED (like r5 and before), but reads the
top 32-bits as reserved, i.e. read zero (may need manual sign
extension) and writes ignored.

> 
>  ISTR a while ago we had a rather lengthy discussion as to how to detect 
> the presence of the upper 32 bits without triggering undefined behaviour 
> implied by 64-bit CP0 accesses to 32-bit CP0 registers.  As I believe we 
> set EBase ourselves I think we are able to make the necessary checks and 
> have an accurate condition here, still remembering however that it may go 
> back as far as MIPSr3.

We only set ebase under certain circumstances, otherwise leaving it as
already set.

Cheers
James

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/9] MIPS: traps: 64bit kernels should read CP0_EBase 64bit

[Maciej W. Rozycki]

On Thu, 6 Oct 2016, James Hogan wrote:

> >  ISTR a while ago we had a rather lengthy discussion as to how to detect 
> > the presence of the upper 32 bits without triggering undefined behaviour 
> > implied by 64-bit CP0 accesses to 32-bit CP0 registers.  As I believe we 
> > set EBase ourselves I think we are able to make the necessary checks and 
> > have an accurate condition here, still remembering however that it may go 
> > back as far as MIPSr3.
> 
> We only set ebase under certain circumstances, otherwise leaving it as
> already set.

 How can we install a handler then when we don't know what the upper 32 
bits of EBase are?

  Maciej

Re: [PATCH 1/9] MIPS: traps: 64bit kernels should read CP0_EBase 64bit

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 1382 bytes --]

On Thu, Oct 06, 2016 at 08:56:50PM +0100, Maciej W. Rozycki wrote:
> On Thu, 6 Oct 2016, James Hogan wrote:
> 
> > >  ISTR a while ago we had a rather lengthy discussion as to how to detect 
> > > the presence of the upper 32 bits without triggering undefined behaviour 
> > > implied by 64-bit CP0 accesses to 32-bit CP0 registers.  As I believe we 
> > > set EBase ourselves I think we are able to make the necessary checks and 
> > > have an accurate condition here, still remembering however that it may go 
> > > back as far as MIPSr3.
> > 
> > We only set ebase under certain circumstances, otherwise leaving it as
> > already set.
> 
>  How can we install a handler then when we don't know what the upper 32 
> bits of EBase are?

Right now its assumed the default upper 32 bits are sign extension of
bit 31 in that case (i.e. thats what upper 32bits are clobbered to). I
think the only case where that might not be true would be where WG is
implemented and the bootloader has changed them to e.g. somewhere in
XKPhys, and then cleared WG. We could catch that most of the time by
detecting changed bits 31:30 (as I think you suggested before), but it
still isn't watertight (e.g. xkphys+0x80000000), so if in doubt we
should probably be sure to allocate our own exception vector instead of
guessing at the boot one. What a mess :-(.

Cheers
James

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/9] MIPS: traps: 64bit kernels should read CP0_EBase 64bit

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 1382 bytes --]

On Thu, Oct 06, 2016 at 08:56:50PM +0100, Maciej W. Rozycki wrote:
> On Thu, 6 Oct 2016, James Hogan wrote:
> 
> > >  ISTR a while ago we had a rather lengthy discussion as to how to detect 
> > > the presence of the upper 32 bits without triggering undefined behaviour 
> > > implied by 64-bit CP0 accesses to 32-bit CP0 registers.  As I believe we 
> > > set EBase ourselves I think we are able to make the necessary checks and 
> > > have an accurate condition here, still remembering however that it may go 
> > > back as far as MIPSr3.
> > 
> > We only set ebase under certain circumstances, otherwise leaving it as
> > already set.
> 
>  How can we install a handler then when we don't know what the upper 32 
> bits of EBase are?

Right now its assumed the default upper 32 bits are sign extension of
bit 31 in that case (i.e. thats what upper 32bits are clobbered to). I
think the only case where that might not be true would be where WG is
implemented and the bootloader has changed them to e.g. somewhere in
XKPhys, and then cleared WG. We could catch that most of the time by
detecting changed bits 31:30 (as I think you suggested before), but it
still isn't watertight (e.g. xkphys+0x80000000), so if in doubt we
should probably be sure to allocate our own exception vector instead of
guessing at the boot one. What a mess :-(.

Cheers
James

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/9] MIPS: traps: 64bit kernels should read CP0_EBase 64bit

[Maciej W. Rozycki]

On Thu, 6 Oct 2016, James Hogan wrote:

> >  How can we install a handler then when we don't know what the upper 32 
> > bits of EBase are?
> 
> Right now its assumed the default upper 32 bits are sign extension of
> bit 31 in that case (i.e. thats what upper 32bits are clobbered to). I
> think the only case where that might not be true would be where WG is
> implemented and the bootloader has changed them to e.g. somewhere in
> XKPhys, and then cleared WG. We could catch that most of the time by
> detecting changed bits 31:30 (as I think you suggested before), but it
> still isn't watertight (e.g. xkphys+0x80000000), so if in doubt we
> should probably be sure to allocate our own exception vector instead of
> guessing at the boot one. What a mess :-(.

 Does it really matter in reality though?

 By keeping EBase unchanged we try to install exception handlers in memory 
assigned by the firmware.  This may not necessarily be safe.  I think we 
actually ought to set EBase ourselves, perhaps on a CPU by CPU basis in an 
MP system, pointing to memory we know we can use at will.  If this is 
going to consume say a page of memory per CPU, then still I don't think 
it's a huge waste, and any firmware memory safe to reclaim after boostrap 
we can use for other purposes.

 Have I missed anything?

  Maciej

Re: [PATCH 1/9] MIPS: traps: 64bit kernels should read CP0_EBase 64bit

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 1994 bytes --]

On Thu, Oct 06, 2016 at 11:41:40PM +0100, Maciej W. Rozycki wrote:
> On Thu, 6 Oct 2016, James Hogan wrote:
> 
> > >  How can we install a handler then when we don't know what the upper 32 
> > > bits of EBase are?
> > 
> > Right now its assumed the default upper 32 bits are sign extension of
> > bit 31 in that case (i.e. thats what upper 32bits are clobbered to). I
> > think the only case where that might not be true would be where WG is
> > implemented and the bootloader has changed them to e.g. somewhere in
> > XKPhys, and then cleared WG. We could catch that most of the time by
> > detecting changed bits 31:30 (as I think you suggested before), but it
> > still isn't watertight (e.g. xkphys+0x80000000), so if in doubt we
> > should probably be sure to allocate our own exception vector instead of
> > guessing at the boot one. What a mess :-(.
> 
>  Does it really matter in reality though?

Good question. The whole thing is based on paranoia really.

> 
>  By keeping EBase unchanged we try to install exception handlers in memory 
> assigned by the firmware.  This may not necessarily be safe.  I think we 
> actually ought to set EBase ourselves, perhaps on a CPU by CPU basis in an 
> MP system, pointing to memory we know we can use at will.  If this is 
> going to consume say a page of memory per CPU, then still I don't think 
> it's a huge waste, and any firmware memory safe to reclaim after boostrap 
> we can use for other purposes.
> 
>  Have I missed anything?

I don't particularly object to always allocating our own vector when
EBase is present. It'd probably break KVM, but that's KVM's fault for
not emulating EBase properly yet.

I suppose there is also an advantage to keeping the bootloader exception
vector as alive as possible at least until Linux has set up its own one,
as it allows early bugs to be caught by the bootloader, which can dump
registers etc and even return to the bootloader prompt.

Cheers
James

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/9] MIPS: traps: 64bit kernels should read CP0_EBase 64bit

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 1994 bytes --]

On Thu, Oct 06, 2016 at 11:41:40PM +0100, Maciej W. Rozycki wrote:
> On Thu, 6 Oct 2016, James Hogan wrote:
> 
> > >  How can we install a handler then when we don't know what the upper 32 
> > > bits of EBase are?
> > 
> > Right now its assumed the default upper 32 bits are sign extension of
> > bit 31 in that case (i.e. thats what upper 32bits are clobbered to). I
> > think the only case where that might not be true would be where WG is
> > implemented and the bootloader has changed them to e.g. somewhere in
> > XKPhys, and then cleared WG. We could catch that most of the time by
> > detecting changed bits 31:30 (as I think you suggested before), but it
> > still isn't watertight (e.g. xkphys+0x80000000), so if in doubt we
> > should probably be sure to allocate our own exception vector instead of
> > guessing at the boot one. What a mess :-(.
> 
>  Does it really matter in reality though?

Good question. The whole thing is based on paranoia really.

> 
>  By keeping EBase unchanged we try to install exception handlers in memory 
> assigned by the firmware.  This may not necessarily be safe.  I think we 
> actually ought to set EBase ourselves, perhaps on a CPU by CPU basis in an 
> MP system, pointing to memory we know we can use at will.  If this is 
> going to consume say a page of memory per CPU, then still I don't think 
> it's a huge waste, and any firmware memory safe to reclaim after boostrap 
> we can use for other purposes.
> 
>  Have I missed anything?

I don't particularly object to always allocating our own vector when
EBase is present. It'd probably break KVM, but that's KVM's fault for
not emulating EBase properly yet.

I suppose there is also an advantage to keeping the bootloader exception
vector as alive as possible at least until Linux has set up its own one,
as it allows early bugs to be caught by the bootloader, which can dump
registers etc and even return to the bootloader prompt.

Cheers
James

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/9] MIPS: traps: 64bit kernels should read CP0_EBase 64bit

[Maciej W. Rozycki]

On Thu, 6 Oct 2016, James Hogan wrote:

> I don't particularly object to always allocating our own vector when
> EBase is present. It'd probably break KVM, but that's KVM's fault for
> not emulating EBase properly yet.

 In most cases we'll use the default KSEG0 base address anyway, won't we?

> I suppose there is also an advantage to keeping the bootloader exception
> vector as alive as possible at least until Linux has set up its own one,
> as it allows early bugs to be caught by the bootloader, which can dump
> registers etc and even return to the bootloader prompt.

 Right, but I think using our own allocated memory actually helps that in 
that we can install our exception handlers first and only then switch 
EBase, which is atomic (modulo probing for WG, but perhaps we don't 
actually have to do that).  Whereas overwriting firmware memory already 
pointed to by EBase while installing exception handlers is pretty much 
destructive right away, as there'll always be a stage at which a partially 
installed handler is non-functional.

  Maciej

Re: [PATCH 1/9] MIPS: traps: 64bit kernels should read CP0_EBase 64bit

[David Daney]

On 10/06/2016 09:18 AM, Maciej W. Rozycki wrote:
> Hi James,
>
>>>   This does look wrong to me, as I noted above EBase is 64-bit with MIPS64
>>> processors as from architecture revision 3.50.  Also I don't think we want
>>
>> MIPS64 PRA (I'm looking at r5 and r6) seems to allow for write-gate not
>> to be implemented, in which case the register is only 32-bits.
>
>   Indeed, but we need to be prepared to handle the width of 64 bits and
> `cpu_has_mips64r6' does not seem to me to be the right condition.

It is not the proper condition.

The presence of a 64-bit EBase should be probed for.

The proper check is to test of the EBase[WG] (bit 11) can be set to 1. 
It it can, this indicates that EBase supports 64-bit accesses.

David.

Re: [PATCH 1/9] MIPS: traps: 64bit kernels should read CP0_EBase 64bit

[David Daney]

On 10/07/2016 08:35 AM, David Daney wrote:
> On 10/06/2016 09:18 AM, Maciej W. Rozycki wrote:
>> Hi James,
>>
>>>>   This does look wrong to me, as I noted above EBase is 64-bit with
>>>> MIPS64
>>>> processors as from architecture revision 3.50.  Also I don't think
>>>> we want
>>>
>>> MIPS64 PRA (I'm looking at r5 and r6) seems to allow for write-gate not
>>> to be implemented, in which case the register is only 32-bits.
>>
>>   Indeed, but we need to be prepared to handle the width of 64 bits and
>> `cpu_has_mips64r6' does not seem to me to be the right condition.
>
> It is not the proper condition.
>
> The presence of a 64-bit EBase should be probed for.
>
> The proper check is to test of the EBase[WG] (bit 11) can be set to 1.
> It it can, this indicates that EBase supports 64-bit accesses.
>

One more thing...

In r5 systems, the only time 64-bit Ebase is really interesting is for 
virtualization.

You could also gate probing WG on the presence of the VZ capability.



> David.
>
>
>

Re: [PATCH 1/9] MIPS: traps: 64bit kernels should read CP0_EBase 64bit

[Maciej W. Rozycki]

On Fri, 7 Oct 2016, David Daney wrote:

> > >   Indeed, but we need to be prepared to handle the width of 64 bits and
> > > `cpu_has_mips64r6' does not seem to me to be the right condition.
> >
> > It is not the proper condition.
> >
> > The presence of a 64-bit EBase should be probed for.
> >
> > The proper check is to test of the EBase[WG] (bit 11) can be set to 1.
> > It it can, this indicates that EBase supports 64-bit accesses.

 Indeed; the problem however is it is destructive, because:

1. Until you have probed for it you cannot use 64-bit DMFC0 to record the 
   old value of EBase.

2. By using 32-bit MFC0 to record it you miss the upper 32 bits of EBase.

3. And you do need to use 32-bit MTC0 to set WG with this probing, which 
   clobbers the upper 32 bits of EBase.

So from MIPSr3 through to MIPSr5 you cannot really use the setting left 
there in EBase by the firmware unless it has also left the WG bit set.

> In r5 systems, the only time 64-bit Ebase is really interesting is for
> virtualization.
> 
> You could also gate probing WG on the presence of the VZ capability.

 Still this is an approximation only, the architecture permits 64-bit 
EBase without VZ.

  Maciej

[PATCH 2/9] MIPS: traps: Convert ebase to KSeg0

[James Hogan]

When allocating boot memory for the exception vector when vectored
interrupts (vint) or vectored external interrupt controllers (veic) are
enabled, try to ensure that the virtual address resides in KSeg0 (and
WARN should that not be possible).

This will be helpful on MIPS64 cores supporting the CP0_EBase Write Gate
(WG) bit once we start using the WG bit to write the full ebase into
CP0_EBase, as we ideally need to avoid hitting the architecturally
poorly defined exception base for Cache Errors when CP0_EBase is in
XKPhys.

An exception is made for Enhanced Virtual Addressing (EVA) kernels which
allow segments to be rearranged and to become uncached during cache
error handling, making it valid for ebase to be elsewhere.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Matt Redfearn <matt.redfearn@imgtec.com>
Cc: Leonid Yegoshin <leonid.yegoshin@imgtec.com>
Cc: linux-mips@linux-mips.org
---
 arch/mips/kernel/traps.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+), 0 deletions(-)

diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
index 686903f62fa3..cb2419dc4651 100644
--- a/arch/mips/kernel/traps.c
+++ b/arch/mips/kernel/traps.c
@@ -2209,8 +2209,25 @@ void __init trap_init(void)
 
 	if (cpu_has_veic || cpu_has_vint) {
 		unsigned long size = 0x200 + VECTORSPACING*64;
+		phys_addr_t ebase_pa;
+
 		ebase = (unsigned long)
 			__alloc_bootmem(size, 1 << fls(size), 0);
+
+		/*
+		 * Try to ensure ebase resides in KSeg0 if possible.
+		 *
+		 * It shouldn't generally be in XKPhys on MIPS64 to avoid
+		 * hitting a poorly defined exception base for Cache Errors.
+		 * The allocation is likely to be in the low 512MB of physical,
+		 * in which case we should be able to convert to KSeg0.
+		 *
+		 * EVA is special though as it allows segments to be rearranged
+		 * and to become uncached during cache error handling.
+		 */
+		ebase_pa = __pa(ebase);
+		if (!IS_ENABLED(CONFIG_EVA) && !WARN_ON(ebase_pa >= 0x20000000))
+			ebase = CKSEG0ADDR(ebase_pa);
 	} else {
 		ebase = CAC_BASE;
 
-- 
git-series 0.8.10

[PATCH 2/9] MIPS: traps: Convert ebase to KSeg0

[James Hogan]

When allocating boot memory for the exception vector when vectored
interrupts (vint) or vectored external interrupt controllers (veic) are
enabled, try to ensure that the virtual address resides in KSeg0 (and
WARN should that not be possible).

This will be helpful on MIPS64 cores supporting the CP0_EBase Write Gate
(WG) bit once we start using the WG bit to write the full ebase into
CP0_EBase, as we ideally need to avoid hitting the architecturally
poorly defined exception base for Cache Errors when CP0_EBase is in
XKPhys.

An exception is made for Enhanced Virtual Addressing (EVA) kernels which
allow segments to be rearranged and to become uncached during cache
error handling, making it valid for ebase to be elsewhere.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Matt Redfearn <matt.redfearn@imgtec.com>
Cc: Leonid Yegoshin <leonid.yegoshin@imgtec.com>
Cc: linux-mips@linux-mips.org
---
 arch/mips/kernel/traps.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+), 0 deletions(-)

diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
index 686903f62fa3..cb2419dc4651 100644
--- a/arch/mips/kernel/traps.c
+++ b/arch/mips/kernel/traps.c
@@ -2209,8 +2209,25 @@ void __init trap_init(void)
 
 	if (cpu_has_veic || cpu_has_vint) {
 		unsigned long size = 0x200 + VECTORSPACING*64;
+		phys_addr_t ebase_pa;
+
 		ebase = (unsigned long)
 			__alloc_bootmem(size, 1 << fls(size), 0);
+
+		/*
+		 * Try to ensure ebase resides in KSeg0 if possible.
+		 *
+		 * It shouldn't generally be in XKPhys on MIPS64 to avoid
+		 * hitting a poorly defined exception base for Cache Errors.
+		 * The allocation is likely to be in the low 512MB of physical,
+		 * in which case we should be able to convert to KSeg0.
+		 *
+		 * EVA is special though as it allows segments to be rearranged
+		 * and to become uncached during cache error handling.
+		 */
+		ebase_pa = __pa(ebase);
+		if (!IS_ENABLED(CONFIG_EVA) && !WARN_ON(ebase_pa >= 0x20000000))
+			ebase = CKSEG0ADDR(ebase_pa);
 	} else {
 		ebase = CAC_BASE;
 
-- 
git-series 0.8.10

[PATCH 3/9] MIPS: traps: Ensure full EBase is written

[James Hogan]

From: Matt Redfearn <matt.redfearn@imgtec.com>

On CPUs which support the EBase WG (write gate) flag, the most
significant bits of the exception base can be changed. Firmware running
on a VP(E) using MIPS rproc may change EBase to point into the user
segment where the firmware is located such that it can service
interrupts. When control is transferred back to the kernel the EBase
must be switched back into the kernel segment, such that the kernel's
exception vectors are used.

Similarly when vectored interrupts (vint) or vectored external interrupt
controllers (veic) are enabled an exception vector is allocated from
bootmem, and written to the EBase register. Due to the WG flag being
clear, only bits 29:12 will be written. Asside from the rproc case above
this is normally fine (as it will usually be a low allocation within the
KSeg0 range, however when Enhanced Virtual Addressing (EVA) is enabled
the allocation may be outside of the traditional KSeg0/KSeg1 address
range, resulting in the wrong EBase being written.

Correct both cases (configure_exception_vector() for the boot CPU, and
per_cpu_trap_init() for secondary CPUs) to write EBase with the WG flag
first if supported.

On the Malta EVA configuration, KSeg0 is mapped to physical address 0,
and memory is allocated from the KUSeg segment which is mapped to
physical address 0x80000000, which physically aliases the RAM at 0. This
only worked due to the exception base address aliasing the same
underlying RAM that was written to & cache flushed, and due to
flush_icache_range() going beyond the call of duty and flushing from the
L2 cache too (due to the differing physical addresses).

Signed-off-by: Matt Redfearn <matt.redfearn@imgtec.com>
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/kernel/traps.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
index cb2419dc4651..4900e590d86e 100644
--- a/arch/mips/kernel/traps.c
+++ b/arch/mips/kernel/traps.c
@@ -2091,6 +2091,14 @@ static void configure_exception_vector(void)
 {
 	if (cpu_has_veic || cpu_has_vint) {
 		unsigned long sr = set_c0_status(ST0_BEV);
+		/* If available, use WG to set top bits of EBASE */
+		if (cpu_has_ebase_wg) {
+#ifdef CONFIG_64BIT
+			write_c0_ebase_64(ebase | MIPS_EBASE_WG);
+#else
+			write_c0_ebase(ebase | MIPS_EBASE_WG);
+#endif
+		}
 		write_c0_ebase(ebase);
 		write_c0_status(sr);
 		/* Setting vector spacing enables EI/VI mode  */
@@ -2127,8 +2135,17 @@ void per_cpu_trap_init(bool is_boot_cpu)
 		 * We shouldn't trust a secondary core has a sane EBASE register
 		 * so use the one calculated by the boot CPU.
 		 */
-		if (!is_boot_cpu)
+		if (!is_boot_cpu) {
+			/* If available, use WG to set top bits of EBASE */
+			if (cpu_has_ebase_wg) {
+#ifdef CONFIG_64BIT
+				write_c0_ebase_64(ebase | MIPS_EBASE_WG);
+#else
+				write_c0_ebase(ebase | MIPS_EBASE_WG);
+#endif
+			}
 			write_c0_ebase(ebase);
+		}
 
 		cp0_compare_irq_shift = CAUSEB_TI - CAUSEB_IP;
 		cp0_compare_irq = (read_c0_intctl() >> INTCTLB_IPTI) & 7;
-- 
git-series 0.8.10

[PATCH 3/9] MIPS: traps: Ensure full EBase is written

[James Hogan]

From: Matt Redfearn <matt.redfearn@imgtec.com>

On CPUs which support the EBase WG (write gate) flag, the most
significant bits of the exception base can be changed. Firmware running
on a VP(E) using MIPS rproc may change EBase to point into the user
segment where the firmware is located such that it can service
interrupts. When control is transferred back to the kernel the EBase
must be switched back into the kernel segment, such that the kernel's
exception vectors are used.

Similarly when vectored interrupts (vint) or vectored external interrupt
controllers (veic) are enabled an exception vector is allocated from
bootmem, and written to the EBase register. Due to the WG flag being
clear, only bits 29:12 will be written. Asside from the rproc case above
this is normally fine (as it will usually be a low allocation within the
KSeg0 range, however when Enhanced Virtual Addressing (EVA) is enabled
the allocation may be outside of the traditional KSeg0/KSeg1 address
range, resulting in the wrong EBase being written.

Correct both cases (configure_exception_vector() for the boot CPU, and
per_cpu_trap_init() for secondary CPUs) to write EBase with the WG flag
first if supported.

On the Malta EVA configuration, KSeg0 is mapped to physical address 0,
and memory is allocated from the KUSeg segment which is mapped to
physical address 0x80000000, which physically aliases the RAM at 0. This
only worked due to the exception base address aliasing the same
underlying RAM that was written to & cache flushed, and due to
flush_icache_range() going beyond the call of duty and flushing from the
L2 cache too (due to the differing physical addresses).

Signed-off-by: Matt Redfearn <matt.redfearn@imgtec.com>
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/kernel/traps.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
index cb2419dc4651..4900e590d86e 100644
--- a/arch/mips/kernel/traps.c
+++ b/arch/mips/kernel/traps.c
@@ -2091,6 +2091,14 @@ static void configure_exception_vector(void)
 {
 	if (cpu_has_veic || cpu_has_vint) {
 		unsigned long sr = set_c0_status(ST0_BEV);
+		/* If available, use WG to set top bits of EBASE */
+		if (cpu_has_ebase_wg) {
+#ifdef CONFIG_64BIT
+			write_c0_ebase_64(ebase | MIPS_EBASE_WG);
+#else
+			write_c0_ebase(ebase | MIPS_EBASE_WG);
+#endif
+		}
 		write_c0_ebase(ebase);
 		write_c0_status(sr);
 		/* Setting vector spacing enables EI/VI mode  */
@@ -2127,8 +2135,17 @@ void per_cpu_trap_init(bool is_boot_cpu)
 		 * We shouldn't trust a secondary core has a sane EBASE register
 		 * so use the one calculated by the boot CPU.
 		 */
-		if (!is_boot_cpu)
+		if (!is_boot_cpu) {
+			/* If available, use WG to set top bits of EBASE */
+			if (cpu_has_ebase_wg) {
+#ifdef CONFIG_64BIT
+				write_c0_ebase_64(ebase | MIPS_EBASE_WG);
+#else
+				write_c0_ebase(ebase | MIPS_EBASE_WG);
+#endif
+			}
 			write_c0_ebase(ebase);
+		}
 
 		cp0_compare_irq_shift = CAUSEB_TI - CAUSEB_IP;
 		cp0_compare_irq = (read_c0_intctl() >> INTCTLB_IPTI) & 7;
-- 
git-series 0.8.10

Re: [PATCH 3/9] MIPS: traps: Ensure full EBase is written

[Ralf Baechle]

On Thu, Sep 01, 2016 at 05:30:09PM +0100, James Hogan wrote:

> On CPUs which support the EBase WG (write gate) flag, the most
> significant bits of the exception base can be changed. Firmware running
> on a VP(E) using MIPS rproc may change EBase to point into the user
> segment where the firmware is located such that it can service
> interrupts. When control is transferred back to the kernel the EBase
> must be switched back into the kernel segment, such that the kernel's
> exception vectors are used.
> 
> Similarly when vectored interrupts (vint) or vectored external interrupt
> controllers (veic) are enabled an exception vector is allocated from
> bootmem, and written to the EBase register. Due to the WG flag being
> clear, only bits 29:12 will be written. Asside from the rproc case above
> this is normally fine (as it will usually be a low allocation within the
> KSeg0 range, however when Enhanced Virtual Addressing (EVA) is enabled
> the allocation may be outside of the traditional KSeg0/KSeg1 address
> range, resulting in the wrong EBase being written.
> 
> Correct both cases (configure_exception_vector() for the boot CPU, and
> per_cpu_trap_init() for secondary CPUs) to write EBase with the WG flag
> first if supported.
> 
> On the Malta EVA configuration, KSeg0 is mapped to physical address 0,
> and memory is allocated from the KUSeg segment which is mapped to
> physical address 0x80000000, which physically aliases the RAM at 0. This
> only worked due to the exception base address aliasing the same
> underlying RAM that was written to & cache flushed, and due to
> flush_icache_range() going beyond the call of duty and flushing from the
> L2 cache too (due to the differing physical addresses).

See comments on 1/9.

I think I can apply the remaining patches already before we finished
sorting out 1/9 and 3/9, so I will do so.

  Ralf

[PATCH 4/9] MIPS: c-r4k: Drop bc_wback_inv() from icache flush

[James Hogan]

The EVA conditional bc_wback_inv() at the end of flush_icache_range() to
flush the modified code all the way back to RAM was apparently there for
debug purposes and to accommodate the Malta EVA configuration which
makes use of a physical alias, and didn't use the CP0_EBase.WG (Write
Gate) bit to put the exception vector in the same physical alias where
the exception vector code is written and is being flushed.

Now that CP0_EBase.WG is used, lets drop this 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: linux-mips@linux-mips.org
---
 arch/mips/mm/c-r4k.c | 11 -----------
 1 file changed, 0 insertions(+), 11 deletions(-)

diff --git a/arch/mips/mm/c-r4k.c b/arch/mips/mm/c-r4k.c
index cd72805b64a7..0335a4be0635 100644
--- a/arch/mips/mm/c-r4k.c
+++ b/arch/mips/mm/c-r4k.c
@@ -752,17 +752,6 @@ static inline void __local_r4k_flush_icache_range(unsigned long start,
 			break;
 		}
 	}
-#ifdef CONFIG_EVA
-	/*
-	 * Due to all possible segment mappings, there might cache aliases
-	 * caused by the bootloader being in non-EVA mode, and the CPU switching
-	 * to EVA during early kernel init. It's best to flush the scache
-	 * to avoid having secondary cores fetching stale data and lead to
-	 * kernel crashes.
-	 */
-	bc_wback_inv(start, (end - start));
-	__sync();
-#endif
 }
 
 static inline void local_r4k_flush_icache_range(unsigned long start,
-- 
git-series 0.8.10

[PATCH 4/9] MIPS: c-r4k: Drop bc_wback_inv() from icache flush

[James Hogan]

The EVA conditional bc_wback_inv() at the end of flush_icache_range() to
flush the modified code all the way back to RAM was apparently there for
debug purposes and to accommodate the Malta EVA configuration which
makes use of a physical alias, and didn't use the CP0_EBase.WG (Write
Gate) bit to put the exception vector in the same physical alias where
the exception vector code is written and is being flushed.

Now that CP0_EBase.WG is used, lets drop this 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: linux-mips@linux-mips.org
---
 arch/mips/mm/c-r4k.c | 11 -----------
 1 file changed, 0 insertions(+), 11 deletions(-)

diff --git a/arch/mips/mm/c-r4k.c b/arch/mips/mm/c-r4k.c
index cd72805b64a7..0335a4be0635 100644
--- a/arch/mips/mm/c-r4k.c
+++ b/arch/mips/mm/c-r4k.c
@@ -752,17 +752,6 @@ static inline void __local_r4k_flush_icache_range(unsigned long start,
 			break;
 		}
 	}
-#ifdef CONFIG_EVA
-	/*
-	 * Due to all possible segment mappings, there might cache aliases
-	 * caused by the bootloader being in non-EVA mode, and the CPU switching
-	 * to EVA during early kernel init. It's best to flush the scache
-	 * to avoid having secondary cores fetching stale data and lead to
-	 * kernel crashes.
-	 */
-	bc_wback_inv(start, (end - start));
-	__sync();
-#endif
 }
 
 static inline void local_r4k_flush_icache_range(unsigned long start,
-- 
git-series 0.8.10

[PATCH 5/9] MIPS: c-r4k: Split user/kernel flush_icache_range()

[James Hogan]

flush_icache_range() is used for both user addresses (i.e.
cacheflush(2)), and kernel addresses (as the API documentation
describes).

This isn't really suitable however for Enhanced Virtual Addressing (EVA)
where cache operations on usermode addresses must use a different
instruction, and the protected cache ops assume user addresses, making
flush_icache_range() ineffective on kernel addresses.

Split out a new __flush_icache_user_range() and
__local_flush_icache_user_range() for users which actually want to flush
usermode addresses (note that flush_icache_user_range() already exists
on various architectures but with different arguments).

The implementation of flush_icache_range() will be changed in an
upcoming commit to use unprotected normal cache ops so as to always work
on the kernel mode address space.

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/include/asm/cacheflush.h | 5 +++++
 arch/mips/mm/c-octeon.c            | 2 ++
 arch/mips/mm/c-r3k.c               | 2 ++
 arch/mips/mm/c-r4k.c               | 2 ++
 arch/mips/mm/c-tx39.c              | 3 +++
 arch/mips/mm/cache.c               | 4 ++++
 6 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/arch/mips/include/asm/cacheflush.h b/arch/mips/include/asm/cacheflush.h
index 34ed22ec6c33..4812d1fed0c2 100644
--- a/arch/mips/include/asm/cacheflush.h
+++ b/arch/mips/include/asm/cacheflush.h
@@ -28,6 +28,7 @@
  *  - flush_cache_sigtramp() flush signal trampoline
  *  - flush_icache_all() flush the entire instruction cache
  *  - flush_data_cache_page() flushes a page from the data cache
+ *  - __flush_icache_user_range(start, end) flushes range of user instructions
  */
 
  /*
@@ -80,6 +81,10 @@ static inline void flush_icache_page(struct vm_area_struct *vma,
 
 extern void (*flush_icache_range)(unsigned long start, unsigned long end);
 extern void (*local_flush_icache_range)(unsigned long start, unsigned long end);
+extern void (*__flush_icache_user_range)(unsigned long start,
+					 unsigned long end);
+extern void (*__local_flush_icache_user_range)(unsigned long start,
+					       unsigned long end);
 
 extern void (*__flush_cache_vmap)(void);
 
diff --git a/arch/mips/mm/c-octeon.c b/arch/mips/mm/c-octeon.c
index 05b1d7cf9514..0e45b061e514 100644
--- a/arch/mips/mm/c-octeon.c
+++ b/arch/mips/mm/c-octeon.c
@@ -294,6 +294,8 @@ void octeon_cache_init(void)
 	flush_data_cache_page		= octeon_flush_data_cache_page;
 	flush_icache_range		= octeon_flush_icache_range;
 	local_flush_icache_range	= local_octeon_flush_icache_range;
+	__flush_icache_user_range	= octeon_flush_icache_range;
+	__local_flush_icache_user_range	= local_octeon_flush_icache_range;
 
 	__flush_kernel_vmap_range	= octeon_flush_kernel_vmap_range;
 
diff --git a/arch/mips/mm/c-r3k.c b/arch/mips/mm/c-r3k.c
index 135ec313c1f6..21e4e662c1fa 100644
--- a/arch/mips/mm/c-r3k.c
+++ b/arch/mips/mm/c-r3k.c
@@ -325,6 +325,8 @@ void r3k_cache_init(void)
 	flush_cache_page = r3k_flush_cache_page;
 	flush_icache_range = r3k_flush_icache_range;
 	local_flush_icache_range = r3k_flush_icache_range;
+	__flush_icache_user_range = r3k_flush_icache_range;
+	__local_flush_icache_user_range = r3k_flush_icache_range;
 
 	__flush_kernel_vmap_range = r3k_flush_kernel_vmap_range;
 
diff --git a/arch/mips/mm/c-r4k.c b/arch/mips/mm/c-r4k.c
index 0335a4be0635..36f4aa6d768f 100644
--- a/arch/mips/mm/c-r4k.c
+++ b/arch/mips/mm/c-r4k.c
@@ -1904,6 +1904,8 @@ void r4k_cache_init(void)
 	flush_data_cache_page	= r4k_flush_data_cache_page;
 	flush_icache_range	= r4k_flush_icache_range;
 	local_flush_icache_range	= local_r4k_flush_icache_range;
+	__flush_icache_user_range	= r4k_flush_icache_range;
+	__local_flush_icache_user_range	= local_r4k_flush_icache_range;
 
 #if defined(CONFIG_DMA_NONCOHERENT) || defined(CONFIG_DMA_MAYBE_COHERENT)
 	if (coherentio) {
diff --git a/arch/mips/mm/c-tx39.c b/arch/mips/mm/c-tx39.c
index 596e18458e04..5c282583edf1 100644
--- a/arch/mips/mm/c-tx39.c
+++ b/arch/mips/mm/c-tx39.c
@@ -411,6 +411,9 @@ void tx39_cache_init(void)
 		break;
 	}
 
+	__flush_icache_user_range = flush_icache_range;
+	__local_flush_icache_user_range = local_flush_icache_range;
+
 	current_cpu_data.icache.waysize = icache_size / current_cpu_data.icache.ways;
 	current_cpu_data.dcache.waysize = dcache_size / current_cpu_data.dcache.ways;
 
diff --git a/arch/mips/mm/cache.c b/arch/mips/mm/cache.c
index bf04c6c479a4..5a644c3fe155 100644
--- a/arch/mips/mm/cache.c
+++ b/arch/mips/mm/cache.c
@@ -33,6 +33,10 @@ void (*flush_icache_range)(unsigned long start, unsigned long end);
 EXPORT_SYMBOL_GPL(flush_icache_range);
 void (*local_flush_icache_range)(unsigned long start, unsigned long end);
 EXPORT_SYMBOL_GPL(local_flush_icache_range);
+void (*__flush_icache_user_range)(unsigned long start, unsigned long end);
+EXPORT_SYMBOL_GPL(__flush_icache_user_range);
+void (*__local_flush_icache_user_range)(unsigned long start, unsigned long end);
+EXPORT_SYMBOL_GPL(__local_flush_icache_user_range);
 
 void (*__flush_cache_vmap)(void);
 void (*__flush_cache_vunmap)(void);
-- 
git-series 0.8.10

[PATCH 5/9] MIPS: c-r4k: Split user/kernel flush_icache_range()

[James Hogan]

flush_icache_range() is used for both user addresses (i.e.
cacheflush(2)), and kernel addresses (as the API documentation
describes).

This isn't really suitable however for Enhanced Virtual Addressing (EVA)
where cache operations on usermode addresses must use a different
instruction, and the protected cache ops assume user addresses, making
flush_icache_range() ineffective on kernel addresses.

Split out a new __flush_icache_user_range() and
__local_flush_icache_user_range() for users which actually want to flush
usermode addresses (note that flush_icache_user_range() already exists
on various architectures but with different arguments).

The implementation of flush_icache_range() will be changed in an
upcoming commit to use unprotected normal cache ops so as to always work
on the kernel mode address space.

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/include/asm/cacheflush.h | 5 +++++
 arch/mips/mm/c-octeon.c            | 2 ++
 arch/mips/mm/c-r3k.c               | 2 ++
 arch/mips/mm/c-r4k.c               | 2 ++
 arch/mips/mm/c-tx39.c              | 3 +++
 arch/mips/mm/cache.c               | 4 ++++
 6 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/arch/mips/include/asm/cacheflush.h b/arch/mips/include/asm/cacheflush.h
index 34ed22ec6c33..4812d1fed0c2 100644
--- a/arch/mips/include/asm/cacheflush.h
+++ b/arch/mips/include/asm/cacheflush.h
@@ -28,6 +28,7 @@
  *  - flush_cache_sigtramp() flush signal trampoline
  *  - flush_icache_all() flush the entire instruction cache
  *  - flush_data_cache_page() flushes a page from the data cache
+ *  - __flush_icache_user_range(start, end) flushes range of user instructions
  */
 
  /*
@@ -80,6 +81,10 @@ static inline void flush_icache_page(struct vm_area_struct *vma,
 
 extern void (*flush_icache_range)(unsigned long start, unsigned long end);
 extern void (*local_flush_icache_range)(unsigned long start, unsigned long end);
+extern void (*__flush_icache_user_range)(unsigned long start,
+					 unsigned long end);
+extern void (*__local_flush_icache_user_range)(unsigned long start,
+					       unsigned long end);
 
 extern void (*__flush_cache_vmap)(void);
 
diff --git a/arch/mips/mm/c-octeon.c b/arch/mips/mm/c-octeon.c
index 05b1d7cf9514..0e45b061e514 100644
--- a/arch/mips/mm/c-octeon.c
+++ b/arch/mips/mm/c-octeon.c
@@ -294,6 +294,8 @@ void octeon_cache_init(void)
 	flush_data_cache_page		= octeon_flush_data_cache_page;
 	flush_icache_range		= octeon_flush_icache_range;
 	local_flush_icache_range	= local_octeon_flush_icache_range;
+	__flush_icache_user_range	= octeon_flush_icache_range;
+	__local_flush_icache_user_range	= local_octeon_flush_icache_range;
 
 	__flush_kernel_vmap_range	= octeon_flush_kernel_vmap_range;
 
diff --git a/arch/mips/mm/c-r3k.c b/arch/mips/mm/c-r3k.c
index 135ec313c1f6..21e4e662c1fa 100644
--- a/arch/mips/mm/c-r3k.c
+++ b/arch/mips/mm/c-r3k.c
@@ -325,6 +325,8 @@ void r3k_cache_init(void)
 	flush_cache_page = r3k_flush_cache_page;
 	flush_icache_range = r3k_flush_icache_range;
 	local_flush_icache_range = r3k_flush_icache_range;
+	__flush_icache_user_range = r3k_flush_icache_range;
+	__local_flush_icache_user_range = r3k_flush_icache_range;
 
 	__flush_kernel_vmap_range = r3k_flush_kernel_vmap_range;
 
diff --git a/arch/mips/mm/c-r4k.c b/arch/mips/mm/c-r4k.c
index 0335a4be0635..36f4aa6d768f 100644
--- a/arch/mips/mm/c-r4k.c
+++ b/arch/mips/mm/c-r4k.c
@@ -1904,6 +1904,8 @@ void r4k_cache_init(void)
 	flush_data_cache_page	= r4k_flush_data_cache_page;
 	flush_icache_range	= r4k_flush_icache_range;
 	local_flush_icache_range	= local_r4k_flush_icache_range;
+	__flush_icache_user_range	= r4k_flush_icache_range;
+	__local_flush_icache_user_range	= local_r4k_flush_icache_range;
 
 #if defined(CONFIG_DMA_NONCOHERENT) || defined(CONFIG_DMA_MAYBE_COHERENT)
 	if (coherentio) {
diff --git a/arch/mips/mm/c-tx39.c b/arch/mips/mm/c-tx39.c
index 596e18458e04..5c282583edf1 100644
--- a/arch/mips/mm/c-tx39.c
+++ b/arch/mips/mm/c-tx39.c
@@ -411,6 +411,9 @@ void tx39_cache_init(void)
 		break;
 	}
 
+	__flush_icache_user_range = flush_icache_range;
+	__local_flush_icache_user_range = local_flush_icache_range;
+
 	current_cpu_data.icache.waysize = icache_size / current_cpu_data.icache.ways;
 	current_cpu_data.dcache.waysize = dcache_size / current_cpu_data.dcache.ways;
 
diff --git a/arch/mips/mm/cache.c b/arch/mips/mm/cache.c
index bf04c6c479a4..5a644c3fe155 100644
--- a/arch/mips/mm/cache.c
+++ b/arch/mips/mm/cache.c
@@ -33,6 +33,10 @@ void (*flush_icache_range)(unsigned long start, unsigned long end);
 EXPORT_SYMBOL_GPL(flush_icache_range);
 void (*local_flush_icache_range)(unsigned long start, unsigned long end);
 EXPORT_SYMBOL_GPL(local_flush_icache_range);
+void (*__flush_icache_user_range)(unsigned long start, unsigned long end);
+EXPORT_SYMBOL_GPL(__flush_icache_user_range);
+void (*__local_flush_icache_user_range)(unsigned long start, unsigned long end);
+EXPORT_SYMBOL_GPL(__local_flush_icache_user_range);
 
 void (*__flush_cache_vmap)(void);
 void (*__flush_cache_vunmap)(void);
-- 
git-series 0.8.10

[PATCH 6/9] MIPS: cacheflush: Use __flush_icache_user_range()

[James Hogan]

The cacheflush(2) system call uses flush_icache_range() to flush a range
of usermode addresses from the icache, so change it to utilise the new
__flush_icache_user_range() API to allow the more generic
flush_icache_range() to be changed to work on kernel addresses only.

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/cache.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/mm/cache.c b/arch/mips/mm/cache.c
index 5a644c3fe155..7ec0847c725a 100644
--- a/arch/mips/mm/cache.c
+++ b/arch/mips/mm/cache.c
@@ -78,7 +78,7 @@ SYSCALL_DEFINE3(cacheflush, unsigned long, addr, unsigned long, bytes,
 	if (!access_ok(VERIFY_WRITE, (void __user *) addr, bytes))
 		return -EFAULT;
 
-	flush_icache_range(addr, addr + bytes);
+	__flush_icache_user_range(addr, addr + bytes);
 
 	return 0;
 }
-- 
git-series 0.8.10

[PATCH 6/9] MIPS: cacheflush: Use __flush_icache_user_range()

[James Hogan]

The cacheflush(2) system call uses flush_icache_range() to flush a range
of usermode addresses from the icache, so change it to utilise the new
__flush_icache_user_range() API to allow the more generic
flush_icache_range() to be changed to work on kernel addresses only.

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/cache.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/mm/cache.c b/arch/mips/mm/cache.c
index 5a644c3fe155..7ec0847c725a 100644
--- a/arch/mips/mm/cache.c
+++ b/arch/mips/mm/cache.c
@@ -78,7 +78,7 @@ SYSCALL_DEFINE3(cacheflush, unsigned long, addr, unsigned long, bytes,
 	if (!access_ok(VERIFY_WRITE, (void __user *) addr, bytes))
 		return -EFAULT;
 
-	flush_icache_range(addr, addr + bytes);
+	__flush_icache_user_range(addr, addr + bytes);
 
 	return 0;
 }
-- 
git-series 0.8.10

[PATCH 7/9] MIPS: uprobes: Flush icache via kernel address

[James Hogan]

Update arch_uprobe_copy_ixol() to use the kmap_atomic() based kernel
address to flush the icache with flush_icache_range(), rather than the
user mapping. We have the kernel mapping available anyway and this
avoids having to switch to using the new __flush_icache_user_range() for
the sake of Enhanced Virtual Addressing (EVA) where flush_icache_range()
will become ineffective on user addresses.

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/kernel/uprobes.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/arch/mips/kernel/uprobes.c b/arch/mips/kernel/uprobes.c
index 8452d933a645..9a507ab1ea38 100644
--- a/arch/mips/kernel/uprobes.c
+++ b/arch/mips/kernel/uprobes.c
@@ -301,19 +301,14 @@ int set_orig_insn(struct arch_uprobe *auprobe, struct mm_struct *mm,
 void __weak arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
 				  void *src, unsigned long len)
 {
-	void *kaddr;
+	void *kaddr, kstart;
 
 	/* Initialize the slot */
 	kaddr = kmap_atomic(page);
-	memcpy(kaddr + (vaddr & ~PAGE_MASK), src, len);
+	kstart = kaddr + (vaddr & ~PAGE_MASK);
+	memcpy(kstart, src, len);
+	flush_icache_range(kstart, kstart + len);
 	kunmap_atomic(kaddr);
-
-	/*
-	 * The MIPS version of flush_icache_range will operate safely on
-	 * user space addresses and more importantly, it doesn't require a
-	 * VMA argument.
-	 */
-	flush_icache_range(vaddr, vaddr + len);
 }
 
 /**
-- 
git-series 0.8.10

[PATCH 7/9] MIPS: uprobes: Flush icache via kernel address

[James Hogan]

Update arch_uprobe_copy_ixol() to use the kmap_atomic() based kernel
address to flush the icache with flush_icache_range(), rather than the
user mapping. We have the kernel mapping available anyway and this
avoids having to switch to using the new __flush_icache_user_range() for
the sake of Enhanced Virtual Addressing (EVA) where flush_icache_range()
will become ineffective on user addresses.

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/kernel/uprobes.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/arch/mips/kernel/uprobes.c b/arch/mips/kernel/uprobes.c
index 8452d933a645..9a507ab1ea38 100644
--- a/arch/mips/kernel/uprobes.c
+++ b/arch/mips/kernel/uprobes.c
@@ -301,19 +301,14 @@ int set_orig_insn(struct arch_uprobe *auprobe, struct mm_struct *mm,
 void __weak arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
 				  void *src, unsigned long len)
 {
-	void *kaddr;
+	void *kaddr, kstart;
 
 	/* Initialize the slot */
 	kaddr = kmap_atomic(page);
-	memcpy(kaddr + (vaddr & ~PAGE_MASK), src, len);
+	kstart = kaddr + (vaddr & ~PAGE_MASK);
+	memcpy(kstart, src, len);
+	flush_icache_range(kstart, kstart + len);
 	kunmap_atomic(kaddr);
-
-	/*
-	 * The MIPS version of flush_icache_range will operate safely on
-	 * user space addresses and more importantly, it doesn't require a
-	 * VMA argument.
-	 */
-	flush_icache_range(vaddr, vaddr + len);
 }
 
 /**
-- 
git-series 0.8.10

Re: [PATCH 7/9] MIPS: uprobes: Flush icache via kernel address

[Ralf Baechle]

On Thu, Sep 01, 2016 at 05:30:13PM +0100, James Hogan wrote:

> Update arch_uprobe_copy_ixol() to use the kmap_atomic() based kernel
> address to flush the icache with flush_icache_range(), rather than the
> user mapping. We have the kernel mapping available anyway and this
> avoids having to switch to using the new __flush_icache_user_range() for
> the sake of Enhanced Virtual Addressing (EVA) where flush_icache_range()
> will become ineffective on user addresses.
> 
> 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/kernel/uprobes.c | 13 ++++---------
>  1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/mips/kernel/uprobes.c b/arch/mips/kernel/uprobes.c
> index 8452d933a645..9a507ab1ea38 100644
> --- a/arch/mips/kernel/uprobes.c
> +++ b/arch/mips/kernel/uprobes.c
> @@ -301,19 +301,14 @@ int set_orig_insn(struct arch_uprobe *auprobe, struct mm_struct *mm,
>  void __weak arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
>  				  void *src, unsigned long len)
>  {
> -	void *kaddr;
> +	void *kaddr, kstart;
>  
>  	/* Initialize the slot */
>  	kaddr = kmap_atomic(page);
> -	memcpy(kaddr + (vaddr & ~PAGE_MASK), src, len);
> +	kstart = kaddr + (vaddr & ~PAGE_MASK);
> +	memcpy(kstart, src, len);
> +	flush_icache_range(kstart, kstart + len);
>  	kunmap_atomic(kaddr);
> -
> -	/*
> -	 * The MIPS version of flush_icache_range will operate safely on
> -	 * user space addresses and more importantly, it doesn't require a
> -	 * VMA argument.
> -	 */
> -	flush_icache_range(vaddr, vaddr + len);

I can't convince myself this is right wrt. to cache aliases ...

  Ralf

Re: [PATCH 7/9] MIPS: uprobes: Flush icache via kernel address

[Leonid Yegoshin]

On 09/21/2016 06:26 AM, Ralf Baechle wrote:
> On Thu, Sep 01, 2016 at 05:30:13PM +0100, James Hogan wrote:
>
>> Update arch_uprobe_copy_ixol() to use the kmap_atomic() based kernel
>> address to flush the icache with flush_icache_range(), rather than the
>> user mapping. We have the kernel mapping available anyway and this
>> avoids having to switch to using the new __flush_icache_user_range() for
>> the sake of Enhanced Virtual Addressing (EVA) where flush_icache_range()
>> will become ineffective on user addresses.
>>
>> 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/kernel/uprobes.c | 13 ++++---------
>>   1 file changed, 4 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/mips/kernel/uprobes.c b/arch/mips/kernel/uprobes.c
>> index 8452d933a645..9a507ab1ea38 100644
>> --- a/arch/mips/kernel/uprobes.c
>> +++ b/arch/mips/kernel/uprobes.c
>> @@ -301,19 +301,14 @@ int set_orig_insn(struct arch_uprobe *auprobe, struct mm_struct *mm,
>>   void __weak arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
>>   				  void *src, unsigned long len)
>>   {
>> -	void *kaddr;
>> +	void *kaddr, kstart;
>>   
>>   	/* Initialize the slot */
>>   	kaddr = kmap_atomic(page);
>> -	memcpy(kaddr + (vaddr & ~PAGE_MASK), src, len);
>> +	kstart = kaddr + (vaddr & ~PAGE_MASK);
>> +	memcpy(kstart, src, len);
>> +	flush_icache_range(kstart, kstart + len);
>>   	kunmap_atomic(kaddr);
>> -
>> -	/*
>> -	 * The MIPS version of flush_icache_range will operate safely on
>> -	 * user space addresses and more importantly, it doesn't require a
>> -	 * VMA argument.
>> -	 */
>> -	flush_icache_range(vaddr, vaddr + len);
> I can't convince myself this is right wrt. to cache aliases ...
>
>    Ralf
>
It is incorrect if there is I-cache aliasing (very rare) and there is no 
HIGHMEM cache aliasing fix (not fixed). But top tree Linux doesn't work 
with I-cache aliasing either.

- Leonid.

Re: [PATCH 7/9] MIPS: uprobes: Flush icache via kernel address

[Leonid Yegoshin]

On 09/21/2016 06:26 AM, Ralf Baechle wrote:
> On Thu, Sep 01, 2016 at 05:30:13PM +0100, James Hogan wrote:
>
>> Update arch_uprobe_copy_ixol() to use the kmap_atomic() based kernel
>> address to flush the icache with flush_icache_range(), rather than the
>> user mapping. We have the kernel mapping available anyway and this
>> avoids having to switch to using the new __flush_icache_user_range() for
>> the sake of Enhanced Virtual Addressing (EVA) where flush_icache_range()
>> will become ineffective on user addresses.
>>
>> 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/kernel/uprobes.c | 13 ++++---------
>>   1 file changed, 4 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/mips/kernel/uprobes.c b/arch/mips/kernel/uprobes.c
>> index 8452d933a645..9a507ab1ea38 100644
>> --- a/arch/mips/kernel/uprobes.c
>> +++ b/arch/mips/kernel/uprobes.c
>> @@ -301,19 +301,14 @@ int set_orig_insn(struct arch_uprobe *auprobe, struct mm_struct *mm,
>>   void __weak arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
>>   				  void *src, unsigned long len)
>>   {
>> -	void *kaddr;
>> +	void *kaddr, kstart;
>>   
>>   	/* Initialize the slot */
>>   	kaddr = kmap_atomic(page);
>> -	memcpy(kaddr + (vaddr & ~PAGE_MASK), src, len);
>> +	kstart = kaddr + (vaddr & ~PAGE_MASK);
>> +	memcpy(kstart, src, len);
>> +	flush_icache_range(kstart, kstart + len);
>>   	kunmap_atomic(kaddr);
>> -
>> -	/*
>> -	 * The MIPS version of flush_icache_range will operate safely on
>> -	 * user space addresses and more importantly, it doesn't require a
>> -	 * VMA argument.
>> -	 */
>> -	flush_icache_range(vaddr, vaddr + len);
> I can't convince myself this is right wrt. to cache aliases ...
>
>    Ralf
>
It is incorrect if there is I-cache aliasing (very rare) and there is no 
HIGHMEM cache aliasing fix (not fixed). But top tree Linux doesn't work 
with I-cache aliasing either.

- Leonid.

Re: [PATCH 7/9] MIPS: uprobes: Flush icache via kernel address

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 2418 bytes --]

On Wed, Sep 21, 2016 at 11:15:55AM -0700, Leonid Yegoshin wrote:
> On 09/21/2016 06:26 AM, Ralf Baechle wrote:
> > On Thu, Sep 01, 2016 at 05:30:13PM +0100, James Hogan wrote:
> >
> >> Update arch_uprobe_copy_ixol() to use the kmap_atomic() based kernel
> >> address to flush the icache with flush_icache_range(), rather than the
> >> user mapping. We have the kernel mapping available anyway and this
> >> avoids having to switch to using the new __flush_icache_user_range() for
> >> the sake of Enhanced Virtual Addressing (EVA) where flush_icache_range()
> >> will become ineffective on user addresses.
> >>
> >> 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/kernel/uprobes.c | 13 ++++---------
> >>   1 file changed, 4 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/arch/mips/kernel/uprobes.c b/arch/mips/kernel/uprobes.c
> >> index 8452d933a645..9a507ab1ea38 100644
> >> --- a/arch/mips/kernel/uprobes.c
> >> +++ b/arch/mips/kernel/uprobes.c
> >> @@ -301,19 +301,14 @@ int set_orig_insn(struct arch_uprobe *auprobe, struct mm_struct *mm,
> >>   void __weak arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
> >>   				  void *src, unsigned long len)
> >>   {
> >> -	void *kaddr;
> >> +	void *kaddr, kstart;
> >>   
> >>   	/* Initialize the slot */
> >>   	kaddr = kmap_atomic(page);
> >> -	memcpy(kaddr + (vaddr & ~PAGE_MASK), src, len);
> >> +	kstart = kaddr + (vaddr & ~PAGE_MASK);
> >> +	memcpy(kstart, src, len);
> >> +	flush_icache_range(kstart, kstart + len);
> >>   	kunmap_atomic(kaddr);
> >> -
> >> -	/*
> >> -	 * The MIPS version of flush_icache_range will operate safely on
> >> -	 * user space addresses and more importantly, it doesn't require a
> >> -	 * VMA argument.
> >> -	 */
> >> -	flush_icache_range(vaddr, vaddr + len);
> > I can't convince myself this is right wrt. to cache aliases ...
> >
> >    Ralf
> >
> It is incorrect if there is I-cache aliasing (very rare) and there is no 
> HIGHMEM cache aliasing fix (not fixed). But top tree Linux doesn't work 
> with I-cache aliasing either.

Well its pretty trivial to just use the newly introduced
__flush_icache_user_range() on the user address instead of using
flush_icache_range().

Cheers
James

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 7/9] MIPS: uprobes: Flush icache via kernel address

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 2418 bytes --]

On Wed, Sep 21, 2016 at 11:15:55AM -0700, Leonid Yegoshin wrote:
> On 09/21/2016 06:26 AM, Ralf Baechle wrote:
> > On Thu, Sep 01, 2016 at 05:30:13PM +0100, James Hogan wrote:
> >
> >> Update arch_uprobe_copy_ixol() to use the kmap_atomic() based kernel
> >> address to flush the icache with flush_icache_range(), rather than the
> >> user mapping. We have the kernel mapping available anyway and this
> >> avoids having to switch to using the new __flush_icache_user_range() for
> >> the sake of Enhanced Virtual Addressing (EVA) where flush_icache_range()
> >> will become ineffective on user addresses.
> >>
> >> 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/kernel/uprobes.c | 13 ++++---------
> >>   1 file changed, 4 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/arch/mips/kernel/uprobes.c b/arch/mips/kernel/uprobes.c
> >> index 8452d933a645..9a507ab1ea38 100644
> >> --- a/arch/mips/kernel/uprobes.c
> >> +++ b/arch/mips/kernel/uprobes.c
> >> @@ -301,19 +301,14 @@ int set_orig_insn(struct arch_uprobe *auprobe, struct mm_struct *mm,
> >>   void __weak arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
> >>   				  void *src, unsigned long len)
> >>   {
> >> -	void *kaddr;
> >> +	void *kaddr, kstart;
> >>   
> >>   	/* Initialize the slot */
> >>   	kaddr = kmap_atomic(page);
> >> -	memcpy(kaddr + (vaddr & ~PAGE_MASK), src, len);
> >> +	kstart = kaddr + (vaddr & ~PAGE_MASK);
> >> +	memcpy(kstart, src, len);
> >> +	flush_icache_range(kstart, kstart + len);
> >>   	kunmap_atomic(kaddr);
> >> -
> >> -	/*
> >> -	 * The MIPS version of flush_icache_range will operate safely on
> >> -	 * user space addresses and more importantly, it doesn't require a
> >> -	 * VMA argument.
> >> -	 */
> >> -	flush_icache_range(vaddr, vaddr + len);
> > I can't convince myself this is right wrt. to cache aliases ...
> >
> >    Ralf
> >
> It is incorrect if there is I-cache aliasing (very rare) and there is no 
> HIGHMEM cache aliasing fix (not fixed). But top tree Linux doesn't work 
> with I-cache aliasing either.

Well its pretty trivial to just use the newly introduced
__flush_icache_user_range() on the user address instead of using
flush_icache_range().

Cheers
James

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 7/9] MIPS: uprobes: Flush icache via kernel address

[Leonid Yegoshin]

On 09/22/2016 02:15 PM, James Hogan wrote:
> On Wed, Sep 21, 2016 at 11:15:55AM -0700, Leonid Yegoshin wrote:
>> On 09/21/2016 06:26 AM, Ralf Baechle wrote:
>>> On Thu, Sep 01, 2016 at 05:30:13PM +0100, James Hogan wrote:
>>>
>>>> Update arch_uprobe_copy_ixol() to use the kmap_atomic() based kernel
>>>> address to flush the icache with flush_icache_range(), rather than the
>>>> user mapping. We have the kernel mapping available anyway and this
>>>> avoids having to switch to using the new __flush_icache_user_range() for
>>>> the sake of Enhanced Virtual Addressing (EVA) where flush_icache_range()
>>>> will become ineffective on user addresses.
>>>>
>>>> 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/kernel/uprobes.c | 13 ++++---------
>>>>    1 file changed, 4 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/arch/mips/kernel/uprobes.c b/arch/mips/kernel/uprobes.c
>>>> index 8452d933a645..9a507ab1ea38 100644
>>>> --- a/arch/mips/kernel/uprobes.c
>>>> +++ b/arch/mips/kernel/uprobes.c
>>>> @@ -301,19 +301,14 @@ int set_orig_insn(struct arch_uprobe *auprobe, struct mm_struct *mm,
>>>>    void __weak arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
>>>>    				  void *src, unsigned long len)
>>>>    {
>>>> -	void *kaddr;
>>>> +	void *kaddr, kstart;
>>>>    
>>>>    	/* Initialize the slot */
>>>>    	kaddr = kmap_atomic(page);
>>>> -	memcpy(kaddr + (vaddr & ~PAGE_MASK), src, len);
>>>> +	kstart = kaddr + (vaddr & ~PAGE_MASK);
>>>> +	memcpy(kstart, src, len);
>>>> +	flush_icache_range(kstart, kstart + len);
>>>>    	kunmap_atomic(kaddr);
>>>> -
>>>> -	/*
>>>> -	 * The MIPS version of flush_icache_range will operate safely on
>>>> -	 * user space addresses and more importantly, it doesn't require a
>>>> -	 * VMA argument.
>>>> -	 */
>>>> -	flush_icache_range(vaddr, vaddr + len);
>>> I can't convince myself this is right wrt. to cache aliases ...
>>>
>>>     Ralf
>>>
>> It is incorrect if there is I-cache aliasing (very rare) and there is no
>> HIGHMEM cache aliasing fix (not fixed). But top tree Linux doesn't work
>> with I-cache aliasing either.
> Well its pretty trivial to just use the newly introduced
> __flush_icache_user_range() on the user address instead of using
> flush_icache_range().

It may not work - you should flush kernel Dcache but user Icache and 
__flush_icache_user_range() doesn't do it. Some CPU may accept an 
aliasing CACHE instruction and flush both colors and it can work in this 
case.

Besides that, I had no time to research - does uprobe keep data on the 
same page as code? If yes, then we may want to make a special flush 
sequence for cache aliasing systems here. (User-Dcache, Write-to-page, 
Kernel-Dcache, User-Icache).

- Leonid

Re: [PATCH 7/9] MIPS: uprobes: Flush icache via kernel address

[Leonid Yegoshin]

On 09/22/2016 02:15 PM, James Hogan wrote:
> On Wed, Sep 21, 2016 at 11:15:55AM -0700, Leonid Yegoshin wrote:
>> On 09/21/2016 06:26 AM, Ralf Baechle wrote:
>>> On Thu, Sep 01, 2016 at 05:30:13PM +0100, James Hogan wrote:
>>>
>>>> Update arch_uprobe_copy_ixol() to use the kmap_atomic() based kernel
>>>> address to flush the icache with flush_icache_range(), rather than the
>>>> user mapping. We have the kernel mapping available anyway and this
>>>> avoids having to switch to using the new __flush_icache_user_range() for
>>>> the sake of Enhanced Virtual Addressing (EVA) where flush_icache_range()
>>>> will become ineffective on user addresses.
>>>>
>>>> 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/kernel/uprobes.c | 13 ++++---------
>>>>    1 file changed, 4 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/arch/mips/kernel/uprobes.c b/arch/mips/kernel/uprobes.c
>>>> index 8452d933a645..9a507ab1ea38 100644
>>>> --- a/arch/mips/kernel/uprobes.c
>>>> +++ b/arch/mips/kernel/uprobes.c
>>>> @@ -301,19 +301,14 @@ int set_orig_insn(struct arch_uprobe *auprobe, struct mm_struct *mm,
>>>>    void __weak arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
>>>>    				  void *src, unsigned long len)
>>>>    {
>>>> -	void *kaddr;
>>>> +	void *kaddr, kstart;
>>>>    
>>>>    	/* Initialize the slot */
>>>>    	kaddr = kmap_atomic(page);
>>>> -	memcpy(kaddr + (vaddr & ~PAGE_MASK), src, len);
>>>> +	kstart = kaddr + (vaddr & ~PAGE_MASK);
>>>> +	memcpy(kstart, src, len);
>>>> +	flush_icache_range(kstart, kstart + len);
>>>>    	kunmap_atomic(kaddr);
>>>> -
>>>> -	/*
>>>> -	 * The MIPS version of flush_icache_range will operate safely on
>>>> -	 * user space addresses and more importantly, it doesn't require a
>>>> -	 * VMA argument.
>>>> -	 */
>>>> -	flush_icache_range(vaddr, vaddr + len);
>>> I can't convince myself this is right wrt. to cache aliases ...
>>>
>>>     Ralf
>>>
>> It is incorrect if there is I-cache aliasing (very rare) and there is no
>> HIGHMEM cache aliasing fix (not fixed). But top tree Linux doesn't work
>> with I-cache aliasing either.
> Well its pretty trivial to just use the newly introduced
> __flush_icache_user_range() on the user address instead of using
> flush_icache_range().

It may not work - you should flush kernel Dcache but user Icache and 
__flush_icache_user_range() doesn't do it. Some CPU may accept an 
aliasing CACHE instruction and flush both colors and it can work in this 
case.

Besides that, I had no time to research - does uprobe keep data on the 
same page as code? If yes, then we may want to make a special flush 
sequence for cache aliasing systems here. (User-Dcache, Write-to-page, 
Kernel-Dcache, User-Icache).

- Leonid

Re: [PATCH 7/9] MIPS: uprobes: Flush icache via kernel address

[Leonid Yegoshin]

Look into https://patchwork.linux-mips.org/patch/10870/ for definition 
of mips_flush_data_cache_range() for reference.

- Leonid

Re: [PATCH 7/9] MIPS: uprobes: Flush icache via kernel address

[Leonid Yegoshin]

Look into https://patchwork.linux-mips.org/patch/10870/ for definition 
of mips_flush_data_cache_range() for reference.

- Leonid

Re: [PATCH 7/9] MIPS: uprobes: Flush icache via kernel address

[James Hogan]

On 22 September 2016 22:38:33 BST, Leonid Yegoshin <Leonid.Yegoshin@imgtec.com> wrote:
>On 09/22/2016 02:15 PM, James Hogan wrote:
>> On Wed, Sep 21, 2016 at 11:15:55AM -0700, Leonid Yegoshin wrote:
>>> On 09/21/2016 06:26 AM, Ralf Baechle wrote:
>>>> On Thu, Sep 01, 2016 at 05:30:13PM +0100, James Hogan wrote:
>>>>
>>>>> Update arch_uprobe_copy_ixol() to use the kmap_atomic() based
>kernel
>>>>> address to flush the icache with flush_icache_range(), rather than
>the
>>>>> user mapping. We have the kernel mapping available anyway and this
>>>>> avoids having to switch to using the new
>__flush_icache_user_range() for
>>>>> the sake of Enhanced Virtual Addressing (EVA) where
>flush_icache_range()
>>>>> will become ineffective on user addresses.
>>>>>
>>>>> 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/kernel/uprobes.c | 13 ++++---------
>>>>>    1 file changed, 4 insertions(+), 9 deletions(-)
>>>>>
>>>>> diff --git a/arch/mips/kernel/uprobes.c
>b/arch/mips/kernel/uprobes.c
>>>>> index 8452d933a645..9a507ab1ea38 100644
>>>>> --- a/arch/mips/kernel/uprobes.c
>>>>> +++ b/arch/mips/kernel/uprobes.c
>>>>> @@ -301,19 +301,14 @@ int set_orig_insn(struct arch_uprobe
>*auprobe, struct mm_struct *mm,
>>>>>    void __weak arch_uprobe_copy_ixol(struct page *page, unsigned
>long vaddr,
>>>>>    				  void *src, unsigned long len)
>>>>>    {
>>>>> -	void *kaddr;
>>>>> +	void *kaddr, kstart;
>>>>>    
>>>>>    	/* Initialize the slot */
>>>>>    	kaddr = kmap_atomic(page);
>>>>> -	memcpy(kaddr + (vaddr & ~PAGE_MASK), src, len);
>>>>> +	kstart = kaddr + (vaddr & ~PAGE_MASK);
>>>>> +	memcpy(kstart, src, len);
>>>>> +	flush_icache_range(kstart, kstart + len);
>>>>>    	kunmap_atomic(kaddr);
>>>>> -
>>>>> -	/*
>>>>> -	 * The MIPS version of flush_icache_range will operate safely on
>>>>> -	 * user space addresses and more importantly, it doesn't require
>a
>>>>> -	 * VMA argument.
>>>>> -	 */
>>>>> -	flush_icache_range(vaddr, vaddr + len);
>>>> I can't convince myself this is right wrt. to cache aliases ...
>>>>
>>>>     Ralf
>>>>
>>> It is incorrect if there is I-cache aliasing (very rare) and there
>is no
>>> HIGHMEM cache aliasing fix (not fixed). But top tree Linux doesn't
>work
>>> with I-cache aliasing either.
>> Well its pretty trivial to just use the newly introduced
>> __flush_icache_user_range() on the user address instead of using
>> flush_icache_range().
>
>It may not work - you should flush kernel Dcache but user Icache and 
>__flush_icache_user_range() doesn't do it.

well it'll do a protected dcache flush (i.e. using CACHEE with EVA). Would kmap/kunmap or variants (fixed to work with aliasing dcache) be able to take care of colouring / further flushing?

In any case, simply changing to the user_ one is a no-op compared to leaving as is where patch 9 would probably break it on EVA by making it operate only on kernel addresses.

Cheers
James

> Some CPU may accept an 
>aliasing CACHE instruction and flush both colors and it can work in
>this 
>case.
>
>Besides that, I had no time to research - does uprobe keep data on the 
>same page as code? If yes, then we may want to make a special flush 
>sequence for cache aliasing systems here. (User-Dcache, Write-to-page, 
>Kernel-Dcache, User-Icache).
>
>- Leonid


--
James Hogan

Re: [PATCH 7/9] MIPS: uprobes: Flush icache via kernel address

[Leonid Yegoshin]

On 09/22/2016 03:13 PM, James Hogan wrote:
> well it'll do a protected dcache flush (i.e. using CACHEE with EVA). Would kmap/kunmap or variants (fixed to work with aliasing dcache) be able to take care of colouring / further flushing?

We should flush kernel D-cache and user I-cache in any cache aliasing 
system. I was wrong - a fixed HIGHMEM doesn't do any difference 
actually, because page may be located in directly addressed memory (all 
HIGHMEM stuff is irrelevant in this case, kmap returns a lowmem address).

>
> In any case, simply changing to the user_ one is a no-op compared to leaving as is where patch 9 would probably break it on EVA by making it operate only on kernel addresses.

EVA or not has no difference here - kernel address can still be a 
different color to user address.

And keeping kernel I-cache flush does break it really, not EVA.

- Leonid.

Re: [PATCH 7/9] MIPS: uprobes: Flush icache via kernel address

[Leonid Yegoshin]

On 09/22/2016 03:13 PM, James Hogan wrote:
> well it'll do a protected dcache flush (i.e. using CACHEE with EVA). Would kmap/kunmap or variants (fixed to work with aliasing dcache) be able to take care of colouring / further flushing?

We should flush kernel D-cache and user I-cache in any cache aliasing 
system. I was wrong - a fixed HIGHMEM doesn't do any difference 
actually, because page may be located in directly addressed memory (all 
HIGHMEM stuff is irrelevant in this case, kmap returns a lowmem address).

>
> In any case, simply changing to the user_ one is a no-op compared to leaving as is where patch 9 would probably break it on EVA by making it operate only on kernel addresses.

EVA or not has no difference here - kernel address can still be a 
different color to user address.

And keeping kernel I-cache flush does break it really, not EVA.

- Leonid.

Re: [PATCH 7/9] MIPS: uprobes: Flush icache via kernel address

[James Hogan]

On 22 September 2016 23:27:41 BST, Leonid Yegoshin <Leonid.Yegoshin@imgtec.com> wrote:
>On 09/22/2016 03:13 PM, James Hogan wrote:
>> well it'll do a protected dcache flush (i.e. using CACHEE with EVA).
>Would kmap/kunmap or variants (fixed to work with aliasing dcache) be
>able to take care of colouring / further flushing?
>
>We should flush kernel D-cache and user I-cache in any cache aliasing 
>system. I was wrong - a fixed HIGHMEM doesn't do any difference 
>actually, because page may be located in directly addressed memory (all
>
>HIGHMEM stuff is irrelevant in this case, kmap returns a lowmem
>address).

Maybe there'd need to be other flush calls too that do the right thing for aliasing.

>
>>
>> In any case, simply changing to the user_ one is a no-op compared to
>leaving as is where patch 9 would probably break it on EVA by making it
>operate only on kernel addresses.
>
>EVA or not has no difference here - kernel address can still be a 
>different color to user address.

i'm ignoring aliasing here. If the code doesn't already handle it then this patchset doesn't care. The goal is not to fix aliasing but to prevent any new breakage due to change in semantics in patch 9 to accommodate eva.

EVA does make a difference if left as is, as flush_icache_range will operate on kernel addresses only after patch 9, so the cache op could literally not happen on the right address (irrespective of aliasing, and not a problem without eva).

it also means the ops won't be protected, so failed page fault would i guess cause kernel oops instead of being ignored (maybe impossible to hit in this case, and definitely the exceptional rather than common case) 

>
>And keeping kernel I-cache flush does break it really, not EVA.

Right, but mainly because on eva user/kernel icache flushes will start to actually differ in what they do after patch 9 .

--
James Hogan

[PATCH 8/9] MIPS: KVM: Use __local_flush_icache_user_range()

[James Hogan]

Convert KVM dynamic translation of guest instructions to flush icache
for guest mapped addresses using the new
__local_flush_icache_user_range() API to allow the more generic
flush_icache_range() to be changed to work on kernel addresses only.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Radim Krčmář" <rkrcmar@redhat.com>
Cc: linux-mips@linux-mips.org
Cc: kvm@vger.kernel.org
---
 arch/mips/kvm/dyntrans.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/mips/kvm/dyntrans.c b/arch/mips/kvm/dyntrans.c
index d280894915ed..5317ed2d8909 100644
--- a/arch/mips/kvm/dyntrans.c
+++ b/arch/mips/kvm/dyntrans.c
@@ -45,8 +45,8 @@ static int kvm_mips_trans_replace(struct kvm_vcpu *vcpu, u32 *opc,
 	} else if (KVM_GUEST_KSEGX((unsigned long) opc) == KVM_GUEST_KSEG23) {
 		local_irq_save(flags);
 		memcpy((void *)opc, (void *)&replace, sizeof(u32));
-		local_flush_icache_range((unsigned long)opc,
-					 (unsigned long)opc + 32);
+		__local_flush_icache_user_range((unsigned long)opc,
+						(unsigned long)opc + 32);
 		local_irq_restore(flags);
 	} else {
 		kvm_err("%s: Invalid address: %p\n", __func__, opc);
-- 
git-series 0.8.10

[PATCH 9/9] MIPS: c-r4k: Fix flush_icache_range() for EVA

[James Hogan]

flush_icache_range() flushes icache lines in a protected fashion for
kernel addresses, however this isn't correct with EVA where protected
cache ops only operate on user addresses, making flush_icache_range()
ineffective.

Split the implementations of __flush_icache_user_range() from
flush_icache_range(), changing the normal flush_icache_range() to use
unprotected normal cache ops.

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 | 43 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 35 insertions(+), 8 deletions(-)

diff --git a/arch/mips/mm/c-r4k.c b/arch/mips/mm/c-r4k.c
index 36f4aa6d768f..eed8aefe1de4 100644
--- a/arch/mips/mm/c-r4k.c
+++ b/arch/mips/mm/c-r4k.c
@@ -722,11 +722,13 @@ struct flush_icache_range_args {
 	unsigned long start;
 	unsigned long end;
 	unsigned int type;
+	bool user;
 };
 
 static inline void __local_r4k_flush_icache_range(unsigned long start,
 						  unsigned long end,
-						  unsigned int type)
+						  unsigned int type,
+						  bool user)
 {
 	if (!cpu_has_ic_fills_f_dc) {
 		if (type == R4K_INDEX ||
@@ -734,7 +736,10 @@ static inline void __local_r4k_flush_icache_range(unsigned long start,
 			r4k_blast_dcache();
 		} else {
 			R4600_HIT_CACHEOP_WAR_IMPL;
-			protected_blast_dcache_range(start, end);
+			if (user)
+				protected_blast_dcache_range(start, end);
+			else
+				blast_dcache_range(start, end);
 		}
 	}
 
@@ -748,7 +753,10 @@ static inline void __local_r4k_flush_icache_range(unsigned long start,
 			break;
 
 		default:
-			protected_blast_icache_range(start, end);
+			if (user)
+				protected_blast_icache_range(start, end);
+			else
+				blast_icache_range(start, end);
 			break;
 		}
 	}
@@ -757,7 +765,13 @@ static inline void __local_r4k_flush_icache_range(unsigned long start,
 static inline void local_r4k_flush_icache_range(unsigned long start,
 						unsigned long end)
 {
-	__local_r4k_flush_icache_range(start, end, R4K_HIT | R4K_INDEX);
+	__local_r4k_flush_icache_range(start, end, R4K_HIT | R4K_INDEX, false);
+}
+
+static inline void local_r4k_flush_icache_user_range(unsigned long start,
+						     unsigned long end)
+{
+	__local_r4k_flush_icache_range(start, end, R4K_HIT | R4K_INDEX, true);
 }
 
 static inline void local_r4k_flush_icache_range_ipi(void *args)
@@ -766,11 +780,13 @@ static inline void local_r4k_flush_icache_range_ipi(void *args)
 	unsigned long start = fir_args->start;
 	unsigned long end = fir_args->end;
 	unsigned int type = fir_args->type;
+	bool user = fir_args->user;
 
-	__local_r4k_flush_icache_range(start, end, type);
+	__local_r4k_flush_icache_range(start, end, type, user);
 }
 
-static void r4k_flush_icache_range(unsigned long start, unsigned long end)
+static void __r4k_flush_icache_range(unsigned long start, unsigned long end,
+				     bool user)
 {
 	struct flush_icache_range_args args;
 	unsigned long size, cache_size;
@@ -778,6 +794,7 @@ static void r4k_flush_icache_range(unsigned long start, unsigned long end)
 	args.start = start;
 	args.end = end;
 	args.type = R4K_HIT | R4K_INDEX;
+	args.user = user;
 
 	/*
 	 * Indexed cache ops require an SMP call.
@@ -803,6 +820,16 @@ static void r4k_flush_icache_range(unsigned long start, unsigned long end)
 	instruction_hazard();
 }
 
+static void r4k_flush_icache_range(unsigned long start, unsigned long end)
+{
+	return __r4k_flush_icache_range(start, end, false);
+}
+
+static void r4k_flush_icache_user_range(unsigned long start, unsigned long end)
+{
+	return __r4k_flush_icache_range(start, end, true);
+}
+
 #if defined(CONFIG_DMA_NONCOHERENT) || defined(CONFIG_DMA_MAYBE_COHERENT)
 
 static void r4k_dma_cache_wback_inv(unsigned long addr, unsigned long size)
@@ -1904,8 +1931,8 @@ void r4k_cache_init(void)
 	flush_data_cache_page	= r4k_flush_data_cache_page;
 	flush_icache_range	= r4k_flush_icache_range;
 	local_flush_icache_range	= local_r4k_flush_icache_range;
-	__flush_icache_user_range	= r4k_flush_icache_range;
-	__local_flush_icache_user_range	= local_r4k_flush_icache_range;
+	__flush_icache_user_range	= r4k_flush_icache_user_range;
+	__local_flush_icache_user_range	= local_r4k_flush_icache_user_range;
 
 #if defined(CONFIG_DMA_NONCOHERENT) || defined(CONFIG_DMA_MAYBE_COHERENT)
 	if (coherentio) {
-- 
git-series 0.8.10

[PATCH 9/9] MIPS: c-r4k: Fix flush_icache_range() for EVA

[James Hogan]

flush_icache_range() flushes icache lines in a protected fashion for
kernel addresses, however this isn't correct with EVA where protected
cache ops only operate on user addresses, making flush_icache_range()
ineffective.

Split the implementations of __flush_icache_user_range() from
flush_icache_range(), changing the normal flush_icache_range() to use
unprotected normal cache ops.

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 | 43 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 35 insertions(+), 8 deletions(-)

diff --git a/arch/mips/mm/c-r4k.c b/arch/mips/mm/c-r4k.c
index 36f4aa6d768f..eed8aefe1de4 100644
--- a/arch/mips/mm/c-r4k.c
+++ b/arch/mips/mm/c-r4k.c
@@ -722,11 +722,13 @@ struct flush_icache_range_args {
 	unsigned long start;
 	unsigned long end;
 	unsigned int type;
+	bool user;
 };
 
 static inline void __local_r4k_flush_icache_range(unsigned long start,
 						  unsigned long end,
-						  unsigned int type)
+						  unsigned int type,
+						  bool user)
 {
 	if (!cpu_has_ic_fills_f_dc) {
 		if (type == R4K_INDEX ||
@@ -734,7 +736,10 @@ static inline void __local_r4k_flush_icache_range(unsigned long start,
 			r4k_blast_dcache();
 		} else {
 			R4600_HIT_CACHEOP_WAR_IMPL;
-			protected_blast_dcache_range(start, end);
+			if (user)
+				protected_blast_dcache_range(start, end);
+			else
+				blast_dcache_range(start, end);
 		}
 	}
 
@@ -748,7 +753,10 @@ static inline void __local_r4k_flush_icache_range(unsigned long start,
 			break;
 
 		default:
-			protected_blast_icache_range(start, end);
+			if (user)
+				protected_blast_icache_range(start, end);
+			else
+				blast_icache_range(start, end);
 			break;
 		}
 	}
@@ -757,7 +765,13 @@ static inline void __local_r4k_flush_icache_range(unsigned long start,
 static inline void local_r4k_flush_icache_range(unsigned long start,
 						unsigned long end)
 {
-	__local_r4k_flush_icache_range(start, end, R4K_HIT | R4K_INDEX);
+	__local_r4k_flush_icache_range(start, end, R4K_HIT | R4K_INDEX, false);
+}
+
+static inline void local_r4k_flush_icache_user_range(unsigned long start,
+						     unsigned long end)
+{
+	__local_r4k_flush_icache_range(start, end, R4K_HIT | R4K_INDEX, true);
 }
 
 static inline void local_r4k_flush_icache_range_ipi(void *args)
@@ -766,11 +780,13 @@ static inline void local_r4k_flush_icache_range_ipi(void *args)
 	unsigned long start = fir_args->start;
 	unsigned long end = fir_args->end;
 	unsigned int type = fir_args->type;
+	bool user = fir_args->user;
 
-	__local_r4k_flush_icache_range(start, end, type);
+	__local_r4k_flush_icache_range(start, end, type, user);
 }
 
-static void r4k_flush_icache_range(unsigned long start, unsigned long end)
+static void __r4k_flush_icache_range(unsigned long start, unsigned long end,
+				     bool user)
 {
 	struct flush_icache_range_args args;
 	unsigned long size, cache_size;
@@ -778,6 +794,7 @@ static void r4k_flush_icache_range(unsigned long start, unsigned long end)
 	args.start = start;
 	args.end = end;
 	args.type = R4K_HIT | R4K_INDEX;
+	args.user = user;
 
 	/*
 	 * Indexed cache ops require an SMP call.
@@ -803,6 +820,16 @@ static void r4k_flush_icache_range(unsigned long start, unsigned long end)
 	instruction_hazard();
 }
 
+static void r4k_flush_icache_range(unsigned long start, unsigned long end)
+{
+	return __r4k_flush_icache_range(start, end, false);
+}
+
+static void r4k_flush_icache_user_range(unsigned long start, unsigned long end)
+{
+	return __r4k_flush_icache_range(start, end, true);
+}
+
 #if defined(CONFIG_DMA_NONCOHERENT) || defined(CONFIG_DMA_MAYBE_COHERENT)
 
 static void r4k_dma_cache_wback_inv(unsigned long addr, unsigned long size)
@@ -1904,8 +1931,8 @@ void r4k_cache_init(void)
 	flush_data_cache_page	= r4k_flush_data_cache_page;
 	flush_icache_range	= r4k_flush_icache_range;
 	local_flush_icache_range	= local_r4k_flush_icache_range;
-	__flush_icache_user_range	= r4k_flush_icache_range;
-	__local_flush_icache_user_range	= local_r4k_flush_icache_range;
+	__flush_icache_user_range	= r4k_flush_icache_user_range;
+	__local_flush_icache_user_range	= local_r4k_flush_icache_user_range;
 
 #if defined(CONFIG_DMA_NONCOHERENT) || defined(CONFIG_DMA_MAYBE_COHERENT)
 	if (coherentio) {
-- 
git-series 0.8.10