[PATCH] powerpc/mm/hash: Properly mask the ESID bits when building proto-VSID

[PATCH] powerpc/mm/hash: Properly mask the ESID bits when building proto-VSID

[Aneesh Kumar K.V]

proto-vsid is built using both mmu context id and ESID. We should not have
overlapping bits between those. That will result in us having vsid
collision. With the current code we missed masking the top bits of effective
address. This implies for kernel address we ended up using the top 4 bits
as part of proto-vsid, which is wrong. For the kernel we should have the
below mapping

0xf000000000000000 -> 0x7ffff  (19 bits context + 6 bits ESID ).

Without the patch we endup with
0xf000000000000000 ->  0xf7ffff (0x7ffff | 0xf00000)

We didn't observe any issues till now possibly because we never end up
using a context value which could map to the same  VSID as kernel.

Fixes: c60ac5693c4 ("powerpc: Update kernel VSID range")

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/book3s/64/mmu-hash.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/64/mmu-hash.h b/arch/powerpc/include/asm/book3s/64/mmu-hash.h
index 2e6a823fa502..0735d5a8049f 100644
--- a/arch/powerpc/include/asm/book3s/64/mmu-hash.h
+++ b/arch/powerpc/include/asm/book3s/64/mmu-hash.h
@@ -525,6 +525,8 @@ extern void slb_set_size(u16 size);
 #define ESID_BITS		18
 #define ESID_BITS_1T		6
 
+#define ESID_BITS_MASK		((1 << ESID_BITS) - 1)
+#define ESID_BITS_1T_MASK	((1 << ESID_BITS_1T) - 1)
 /*
  * 256MB segment
  * The proto-VSID space has 2^(CONTEX_BITS + ESID_BITS) - 1 segments
@@ -660,9 +662,9 @@ static inline unsigned long get_vsid(unsigned long context, unsigned long ea,
 
 	if (ssize == MMU_SEGSIZE_256M)
 		return vsid_scramble((context << ESID_BITS)
-				     | (ea >> SID_SHIFT), 256M);
+				     | ((ea >> SID_SHIFT) & ESID_BITS_MASK), 256M);
 	return vsid_scramble((context << ESID_BITS_1T)
-			     | (ea >> SID_SHIFT_1T), 1T);
+			     | ((ea >> SID_SHIFT_1T) & ESID_BITS_1T_MASK), 1T);
 }
 
 /*
-- 
2.10.2

Re: [PATCH] powerpc/mm/hash: Properly mask the ESID bits when building proto-VSID

[Michael Ellerman]

"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> writes:

> proto-vsid is built using both mmu context id and ESID. We should not have
> overlapping bits between those. That will result in us having vsid
> collision. With the current code we missed masking the top bits of effective
> address. This implies for kernel address we ended up using the top 4 bits
> as part of proto-vsid, which is wrong. For the kernel we should have the
> below mapping
>
> 0xf000000000000000 -> 0x7ffff  (19 bits context + 6 bits ESID ).
>
> Without the patch we endup with
> 0xf000000000000000 ->  0xf7ffff (0x7ffff | 0xf00000)

As discussed offline, this is missing the << 6.

Which means we end up with:

  ea         = 0xf000000000000000
  context    = 0x7ffff 
  proto_vsid = (0x7ffff << 6 | 0xf000000000000000 >> 40)
             = (0x1ffffc0 | 0xf00000)
             =  0x1ffffc0

ie. the proto-vsid is not affected by the extra bits. Same is true for
the 0xc/d/e regions.

So there is no bug here, as long as we use the current mapping from
kernel EA to context ID.

I'll merge this into next.

cheers

Re: powerpc/mm/hash: Properly mask the ESID bits when building proto-VSID

[Michael Ellerman]

On Sat, 2017-01-28 at 15:48:40 UTC, "Aneesh Kumar K.V" wrote:
> proto-vsid is built using both mmu context id and ESID. We should not have
> overlapping bits between those. That will result in us having vsid
> collision. With the current code we missed masking the top bits of effective
> address. This implies for kernel address we ended up using the top 4 bits
> as part of proto-vsid, which is wrong. For the kernel we should have the
> below mapping
> 
> 0xf000000000000000 -> 0x7ffff  (19 bits context + 6 bits ESID ).
> 
> Without the patch we endup with
> 0xf000000000000000 ->  0xf7ffff (0x7ffff | 0xf00000)
> 
> We didn't observe any issues till now possibly because we never end up
> using a context value which could map to the same  VSID as kernel.
> 
> Fixes: c60ac5693c4 ("powerpc: Update kernel VSID range")
> 
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/79270e0a3fd124388a0407f9edbd6a

cheers