[PATCH 1/2] powerpc: Make VSID_BITS* dependency explicit

[PATCH 1/2] powerpc: Make VSID_BITS* dependency explicit

[Aneesh Kumar K.V]

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

VSID_BITS and VSID_BITS_1T depends on the context bits  and user esid
bits. Make the dependency explicit

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

diff --git a/arch/powerpc/include/asm/mmu-hash64.h b/arch/powerpc/include/asm/mmu-hash64.h
index 2fdb47a..5f8c2bd 100644
--- a/arch/powerpc/include/asm/mmu-hash64.h
+++ b/arch/powerpc/include/asm/mmu-hash64.h
@@ -381,21 +381,22 @@ extern void slb_set_size(u16 size);
  * hash collisions.
  */
 
+#define CONTEXT_BITS		19
+#define USER_ESID_BITS		18
+#define USER_ESID_BITS_1T	6
+
 /*
  * This should be computed such that protovosid * vsid_mulitplier
  * doesn't overflow 64 bits. It should also be co-prime to vsid_modulus
  */
 #define VSID_MULTIPLIER_256M	ASM_CONST(12538073)	/* 24-bit prime */
-#define VSID_BITS_256M		38
+#define VSID_BITS_256M		(CONTEXT_BITS + USER_ESID_BITS + 1)
 #define VSID_MODULUS_256M	((1UL<<VSID_BITS_256M)-1)
 
 #define VSID_MULTIPLIER_1T	ASM_CONST(12538073)	/* 24-bit prime */
-#define VSID_BITS_1T		26
+#define VSID_BITS_1T		(CONTEXT_BITS + USER_ESID_BITS_1T + 1)
 #define VSID_MODULUS_1T		((1UL<<VSID_BITS_1T)-1)
 
-#define CONTEXT_BITS		19
-#define USER_ESID_BITS		18
-#define USER_ESID_BITS_1T	6
 
 #define USER_VSID_RANGE	(1UL << (USER_ESID_BITS + SID_SHIFT))
 
-- 
1.7.10

[PATCH 2/2] powerpc: Make context bits depend on virtual addr size.

[Aneesh Kumar K.V]

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

Different platforms supports different virtual addr size(n bits). We need to mak sure
0:77-n bits of the VA generated is forced to zero.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/mmu-hash64.h |   22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/mmu-hash64.h b/arch/powerpc/include/asm/mmu-hash64.h
index 5f8c2bd..839efae 100644
--- a/arch/powerpc/include/asm/mmu-hash64.h
+++ b/arch/powerpc/include/asm/mmu-hash64.h
@@ -381,7 +381,27 @@ extern void slb_set_size(u16 size);
  * hash collisions.
  */
 
-#define CONTEXT_BITS		19
+/*
+ * Be careful with this value. This determines the VSID_MODULUS_*  and that
+ * need to be co-prime with VSID_MULTIPLIER*
+ */
+#if defined(CONFIG_POWER6_CPU) || defined(CONFIG_POWER7_CPU)
+/*
+ * Even if cpu support 68 bits, we limit this to 66 because
+ * we support only 2^19 context.
+ */
+#define MAX_VIRTUAL_ADDR_BITS 66
+#else
+/* power4,power 5 and cell is 65 */
+#define MAX_VIRTUAL_ADDR_BITS 65
+#endif
+
+/*
+ * One bit is taken by the kernel, only the rest of space is available for the
+ * user space.
+ */
+#define CONTEXT_BITS		(MAX_VIRTUAL_ADDR_BITS - \
+				 (USER_ESID_BITS + SID_SHIFT + 1))
 #define USER_ESID_BITS		18
 #define USER_ESID_BITS_1T	6
 
-- 
1.7.10

Re: [PATCH 2/2] powerpc: Make context bits depend on virtual addr size.

[Benjamin Herrenschmidt]

On Wed, 2013-02-13 at 00:01 +0530, Aneesh Kumar K.V wrote:
> + * Be careful with this value. This determines the VSID_MODULUS_*  and that
> + * need to be co-prime with VSID_MULTIPLIER*
> + */
> +#if defined(CONFIG_POWER6_CPU) || defined(CONFIG_POWER7_CPU)
> +/*
> + * Even if cpu support 68 bits, we limit this to 66 because
> + * we support only 2^19 context.
> + */
> +#define MAX_VIRTUAL_ADDR_BITS 66
> +#else
> +/* power4,power 5 and cell is 65 */
> +#define MAX_VIRTUAL_ADDR_BITS 65
> +#endif
> +

A compile option ? Really ? Ugh...

Ben.

Re: [PATCH 2/2] powerpc: Make context bits depend on virtual addr size.

[Aneesh Kumar K.V]

Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:

> On Wed, 2013-02-13 at 00:01 +0530, Aneesh Kumar K.V wrote:
>> + * Be careful with this value. This determines the VSID_MODULUS_*  and that
>> + * need to be co-prime with VSID_MULTIPLIER*
>> + */
>> +#if defined(CONFIG_POWER6_CPU) || defined(CONFIG_POWER7_CPU)
>> +/*
>> + * Even if cpu support 68 bits, we limit this to 66 because
>> + * we support only 2^19 context.
>> + */
>> +#define MAX_VIRTUAL_ADDR_BITS 66
>> +#else
>> +/* power4,power 5 and cell is 65 */
>> +#define MAX_VIRTUAL_ADDR_BITS 65
>> +#endif
>> +
>
> A compile option ? Really ? Ugh...

I actually wanted that to be done in Kconfig.cputype, but haven't found
a nice way to do it. Considering we are switching between only two
values, I was thinking an #ifdef would work.

-aneesh

Re: [PATCH 2/2] powerpc: Make context bits depend on virtual addr size.

[Benjamin Herrenschmidt]

On Wed, 2013-02-13 at 08:54 +0530, Aneesh Kumar K.V wrote:
> > A compile option ? Really ? Ugh...
> 
> I actually wanted that to be done in Kconfig.cputype, but haven't found
> a nice way to do it. Considering we are switching between only two
> values, I was thinking an #ifdef would work.

No, we want to support all those processor types from a single kernel image.

Ben.

Re: [PATCH 2/2] powerpc: Make context bits depend on virtual addr size.

[Aneesh Kumar K.V]

Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:

> On Wed, 2013-02-13 at 08:54 +0530, Aneesh Kumar K.V wrote:
>> > A compile option ? Really ? Ugh...
>> 
>> I actually wanted that to be done in Kconfig.cputype, but haven't found
>> a nice way to do it. Considering we are switching between only two
>> values, I was thinking an #ifdef would work.
>
> No, we want to support all those processor types from a single kernel image.

Ok. How about the below patch. This is based on the suggestion from
Paulus. I still have to take care of few comments in the code.

We now split the proto-vsid range differently.
 User:   0 to [2^(CONTEXT_BITS) - 4  + 2^(USER_ESID_BITS)]
 kernel: [2^(CONTEXT_BITS) - 4 + 2^(USER_ESID_BITS)] to 2^(VSID_BITS) - 1

Phileas and Geoff,

Can we check whether this fix the PS3 boot hang ?

-aneesh

diff --git a/arch/powerpc/include/asm/mmu-hash64.h b/arch/powerpc/include/asm/mmu-hash64.h
index 2fdb47a..1e65a01 100644
--- a/arch/powerpc/include/asm/mmu-hash64.h
+++ b/arch/powerpc/include/asm/mmu-hash64.h
@@ -381,21 +381,33 @@ extern void slb_set_size(u16 size);
  * hash collisions.
  */
 
+#define CONTEXT_BITS		19
+#define USER_ESID_BITS		18
+#define USER_ESID_BITS_1T	6
+
+/*
+ * 256MB segment
+ * The proto-VSID space has 2^(CONTEX_BITS + USER_ESID_BITS) - 1 segments
+ * available for user + kernel mapping. The top 4 contexts are used for
+ * kernel mapping. Each segment contains 2^28 bytes. Each
+ * context maps 2^46 bytes (64TB) so we can support 2^19-1 contexts
+ * (19 == 37 + 28 - 46).
+ */
+#define MAX_CONTEXT	((ASM_CONST(1) << CONTEXT_BITS) - 1)
+
+
 /*
  * This should be computed such that protovosid * vsid_mulitplier
  * doesn't overflow 64 bits. It should also be co-prime to vsid_modulus
  */
 #define VSID_MULTIPLIER_256M	ASM_CONST(12538073)	/* 24-bit prime */
-#define VSID_BITS_256M		38
+#define VSID_BITS_256M		(CONTEXT_BITS + USER_ESID_BITS)
 #define VSID_MODULUS_256M	((1UL<<VSID_BITS_256M)-1)
 
 #define VSID_MULTIPLIER_1T	ASM_CONST(12538073)	/* 24-bit prime */
-#define VSID_BITS_1T		26
+#define VSID_BITS_1T		(CONTEXT_BITS + USER_ESID_BITS_1T)
 #define VSID_MODULUS_1T		((1UL<<VSID_BITS_1T)-1)
 
-#define CONTEXT_BITS		19
-#define USER_ESID_BITS		18
-#define USER_ESID_BITS_1T	6
 
 #define USER_VSID_RANGE	(1UL << (USER_ESID_BITS + SID_SHIFT))
 
@@ -513,34 +525,6 @@ typedef struct {
 	})
 #endif /* 1 */
 
-/*
- * This is only valid for addresses >= PAGE_OFFSET
- * The proto-VSID space is divided into two class
- * User:   0 to 2^(CONTEXT_BITS + USER_ESID_BITS) -1
- * kernel: 2^(CONTEXT_BITS + USER_ESID_BITS) to 2^(VSID_BITS) - 1
- *
- * With KERNEL_START at 0xc000000000000000, the proto vsid for
- * the kernel ends up with 0xc00000000 (36 bits). With 64TB
- * support we need to have kernel proto-VSID in the
- * [2^37 to 2^38 - 1] range due to the increased USER_ESID_BITS.
- */
-static inline unsigned long get_kernel_vsid(unsigned long ea, int ssize)
-{
-	unsigned long proto_vsid;
-	/*
-	 * We need to make sure proto_vsid for the kernel is
-	 * >= 2^(CONTEXT_BITS + USER_ESID_BITS[_1T])
-	 */
-	if (ssize == MMU_SEGSIZE_256M) {
-		proto_vsid = ea >> SID_SHIFT;
-		proto_vsid |= (1UL << (CONTEXT_BITS + USER_ESID_BITS));
-		return vsid_scramble(proto_vsid, 256M);
-	}
-	proto_vsid = ea >> SID_SHIFT_1T;
-	proto_vsid |= (1UL << (CONTEXT_BITS + USER_ESID_BITS_1T));
-	return vsid_scramble(proto_vsid, 1T);
-}
-
 /* Returns the segment size indicator for a user address */
 static inline int user_segment_size(unsigned long addr)
 {
@@ -561,6 +545,26 @@ static inline unsigned long get_vsid(unsigned long context, unsigned long ea,
 			     | (ea >> SID_SHIFT_1T), 1T);
 }
 
+/*
+ * This is only valid for addresses >= PAGE_OFFSET
+ * The proto-VSID space is divided into two class
+ * User:   0 to 2^(CONTEXT_BITS) - 4  + 2^(USER_ESID_BITS)
+ * kernel: 2^(CONTEXT_BITS) - 4 + 2^(USER_ESID_BITS) to 2^(VSID_BITS) - 1
+ *
+ * With KERNEL_START at 0xc000000000000000, the proto vsid for
+ * the kernel ends up with 0xc00000000 (36 bits). We use one context
+ * for 0xc, 0xd, 0xe and 0xf.
+ *
+ */
+static inline unsigned long get_kernel_vsid(unsigned long ea, int ssize)
+{
+	unsigned long context;
+	/*
+	 * kernel take the top 4 context from the available range
+	 */
+	context = (MAX_CONTEXT - 4) +  ((ea >> 60) - 0xc);
+	return get_vsid(context, ea, ssize);
+}
 #endif /* __ASSEMBLY__ */
 
 #endif /* _ASM_POWERPC_MMU_HASH64_H_ */
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 4665e82..78c6d0b 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1269,16 +1269,23 @@ _GLOBAL(do_stab_bolted)
 	stw	r9,PACA_EXSLB+EX_CCR(r13)	/* save CR in exc. frame */
 	std	r11,PACA_EXSLB+EX_SRR0(r13)	/* save SRR0 in exc. frame */
 
+	mfspr	r11,SPRN_DAR
+	srdi	r9,r11,60	/* ea >> 60 */
+	srdi	r11,r11,SID_SHIFT
+
+	/* Calculate VSID:
+	 * This is the kernel vsid, we take the top for context from
+	 * the range. context = (MAX_CONTEXT - 4) + ((ea >> 60) - 0xc)
+	 */
+	subi	r9,r9,(0xc + 4 + 1)
+	lis	r10,8
+	add	r9,r9,r10
+
 	/* Hash to the primary group */
 	ld	r10,PACASTABVIRT(r13)
-	mfspr	r11,SPRN_DAR
-	srdi	r11,r11,28
 	rldimi	r10,r11,7,52	/* r10 = first ste of the group */
+	rldimi  r11,r9,USER_ESID_BITS,0 /* proto vsid */
 
-	/* Calculate VSID */
-	/* This is a kernel address, so protovsid = ESID | 1 << 37 */
-	li	r9,0x1
-	rldimi  r11,r9,(CONTEXT_BITS + USER_ESID_BITS),0
 	ASM_VSID_SCRAMBLE(r11, r9, 256M)
 	rldic	r9,r11,12,16	/* r9 = vsid << 12 */
 
diff --git a/arch/powerpc/mm/mmu_context_hash64.c b/arch/powerpc/mm/mmu_context_hash64.c
index 40bc5b0..9c84b16 100644
--- a/arch/powerpc/mm/mmu_context_hash64.c
+++ b/arch/powerpc/mm/mmu_context_hash64.c
@@ -29,15 +29,6 @@
 static DEFINE_SPINLOCK(mmu_context_lock);
 static DEFINE_IDA(mmu_context_ida);
 
-/*
- * 256MB segment
- * The proto-VSID space has 2^(CONTEX_BITS + USER_ESID_BITS) - 1 segments
- * available for user mappings. Each segment contains 2^28 bytes. Each
- * context maps 2^46 bytes (64TB) so we can support 2^19-1 contexts
- * (19 == 37 + 28 - 46).
- */
-#define MAX_CONTEXT	((1UL << CONTEXT_BITS) - 1)
-
 int __init_new_context(void)
 {
 	int index;
@@ -56,7 +47,7 @@ again:
 	else if (err)
 		return err;
 
-	if (index > MAX_CONTEXT) {
+	if (index > (MAX_CONTEXT - 4)) {
 		spin_lock(&mmu_context_lock);
 		ida_remove(&mmu_context_ida, index);
 		spin_unlock(&mmu_context_lock);
diff --git a/arch/powerpc/mm/slb_low.S b/arch/powerpc/mm/slb_low.S
index 1a16ca2..487f998 100644
--- a/arch/powerpc/mm/slb_low.S
+++ b/arch/powerpc/mm/slb_low.S
@@ -56,12 +56,19 @@ _GLOBAL(slb_allocate_realmode)
 	 */
 _GLOBAL(slb_miss_kernel_load_linear)
 	li	r11,0
-	li	r9,0x1
+	/*
+	 * context = (MAX_CONTEXT - 4) + ((ea >> 60) - 0xc)
+	 */
+	srdi	r9,r3,60
+	subi	r9,r9,(0xc + 4 + 1)
+	lis	r10, 8
+	add	r9,r9,r10
+	srdi	r10,r3,28 /* FIXME!! doing it twice */
 	/*
 	 * for 1T we shift 12 bits more.  slb_finish_load_1T will do
 	 * the necessary adjustment
 	 */
-	rldimi  r10,r9,(CONTEXT_BITS + USER_ESID_BITS),0
+	rldimi  r10,r9,USER_ESID_BITS,0
 BEGIN_FTR_SECTION
 	b	slb_finish_load
 END_MMU_FTR_SECTION_IFCLR(MMU_FTR_1T_SEGMENT)
@@ -91,12 +98,19 @@ _GLOBAL(slb_miss_kernel_load_vmemmap)
 	_GLOBAL(slb_miss_kernel_load_io)
 	li	r11,0
 6:
-	li	r9,0x1
+	/*
+	 * context = (MAX_CONTEXT - 4) + ((ea >> 60) - 0xc)
+	 */
+	srdi	r9,r3,60
+	subi	r9,r9,(0xc + 4 + 1)
+	lis	r10,8
+	add	r9,r9,r10
+	srdi	r10,r3,28 /* FIXME!! doing it twice */
 	/*
 	 * for 1T we shift 12 bits more.  slb_finish_load_1T will do
 	 * the necessary adjustment
 	 */
-	rldimi  r10,r9,(CONTEXT_BITS + USER_ESID_BITS),0
+	rldimi  r10,r9,USER_ESID_BITS,0
 BEGIN_FTR_SECTION
 	b	slb_finish_load
 END_MMU_FTR_SECTION_IFCLR(MMU_FTR_1T_SEGMENT)

RE: [PATCH 2/2] powerpc: Make context bits depend on virtual addr size.

[David Laight]

> +#define CONTEXT_BITS		19
> +#define USER_ESID_BITS		18
> +#define USER_ESID_BITS_1T	6
> +
> +/*
> + * 256MB segment
> + * The proto-VSID space has 2^(CONTEX_BITS + USER_ESID_BITS) - 1 =
segments
> + * available for user + kernel mapping. The top 4 contexts are used =
for
> + * kernel mapping. Each segment contains 2^28 bytes. Each
> + * context maps 2^46 bytes (64TB) so we can support 2^19-1 contexts
> + * (19 =3D=3D 37 + 28 - 46).
> + */

I can't help feeling this would be easier to understand if a full
64? 80? bit address was shown with the various bit ranges identified.

Given the comment, I'd have expected CONTEXT_BITS to be calculated
from three other named constants - rather than being set to 19.

	David

RE: [PATCH 2/2] powerpc: Make context bits depend on virtual addr size.

[Aneesh Kumar K.V]

David Laight <David.Laight@aculab.com> writes:

>> +#define CONTEXT_BITS		19
>> +#define USER_ESID_BITS		18
>> +#define USER_ESID_BITS_1T	6
>> +
>> +/*
>> + * 256MB segment
>> + * The proto-VSID space has 2^(CONTEX_BITS + USER_ESID_BITS) - 1 segments
>> + * available for user + kernel mapping. The top 4 contexts are used for
>> + * kernel mapping. Each segment contains 2^28 bytes. Each
>> + * context maps 2^46 bytes (64TB) so we can support 2^19-1 contexts
>> + * (19 == 37 + 28 - 46).
>> + */
>
> I can't help feeling this would be easier to understand if a full
> 64? 80? bit address was shown with the various bit ranges identified.
>
> Given the comment, I'd have expected CONTEXT_BITS to be calculated
> from three other named constants - rather than being set to 19.
>

May be the comments were misleading. We build proto vsid using a
combination of context and ea bits.

Current code does the below:

for kernel:
proto_vsid = ea >> SID_SHIFT;
proto_vsid |= (1UL << (CONTEXT_BITS + USER_ESID_BITS));
for user:
proto_vsid = ea >> SID_SHIFT
proto_vsid |= context << USER_ESID_BITS 

context range is 0 - (2^19 -1)

With this patch we _don't_ give kernel half the proto vsid range.
Instead, we reduce the proto vsid range and then the kernel is given
top 4 context. ie, kernel proto vsid is now

for kenel:
proto_vsid = ea >> SID_SHIFT;
context = (MAX_CONTEXT - 4) +  ((effective address >> 60) - 0xc);
proto_vsid |= context << USER_ESID_BITS 


-aneesh

RE: [PATCH 2/2] powerpc: Make context bits depend on virtual addr size.

[Aneesh Kumar K.V]

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

> David Laight <David.Laight@aculab.com> writes:
>
>>> +#define CONTEXT_BITS		19
>>> +#define USER_ESID_BITS		18
>>> +#define USER_ESID_BITS_1T	6
>>> +
>>> +/*
>>> + * 256MB segment
>>> + * The proto-VSID space has 2^(CONTEX_BITS + USER_ESID_BITS) - 1 segments
>>> + * available for user + kernel mapping. The top 4 contexts are used for
>>> + * kernel mapping. Each segment contains 2^28 bytes. Each
>>> + * context maps 2^46 bytes (64TB) so we can support 2^19-1 contexts
>>> + * (19 == 37 + 28 - 46).
>>> + */
>>
>> I can't help feeling this would be easier to understand if a full
>> 64? 80? bit address was shown with the various bit ranges identified.
>>
>> Given the comment, I'd have expected CONTEXT_BITS to be calculated
>> from three other named constants - rather than being set to 19.
>>
>
> May be the comments were misleading. We build proto vsid using a
> combination of context and ea bits.
>
> Current code does the below:
>
> for kernel:
> proto_vsid = ea >> SID_SHIFT;
> proto_vsid |= (1UL << (CONTEXT_BITS + USER_ESID_BITS));
> for user:
> proto_vsid = ea >> SID_SHIFT
> proto_vsid |= context << USER_ESID_BITS 
>
> context range is 0 - (2^19 -1)
>
> With this patch we _don't_ give kernel half the proto vsid range.
> Instead, we reduce the proto vsid range and then the kernel is given
> top 4 context. ie, kernel proto vsid is now
>
> for kenel:
> proto_vsid = ea >> SID_SHIFT;
> context = (MAX_CONTEXT - 4) +  ((effective address >> 60) - 0xc);
> proto_vsid |= context << USER_ESID_BITS 
>

Hmm that may be an issue, considering ESID for kernel is 36 bits.
We have overlapping bits between shifted value of context and kernel
ESID.

-aneesh

Re[2]: [PATCH 2/2] powerpc: Make context bits depend on virtual addr size.

[Phileas Fogg]

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


>
>Ok. How about the below patch. This is based on the suggestion from
>Paulus. I still have to take care of few comments in the code.
>
>We now split the proto-vsid range differently.
> User:   0 to [2^(CONTEXT_BITS) - 4  + 2^(USER_ESID_BITS)]
> kernel: [2^(CONTEXT_BITS) - 4 + 2^(USER_ESID_BITS)] to 2^(VSID_BITS) - 1
>
>Phileas and Geoff,
>
>Can we check whether this fix the PS3 boot hang ?
 Tested the new patch with Linux 3.8.0-rc7, it seems to be working well.

regards

[-- Attachment #2: Type: text/html, Size: 890 bytes --]

Re: [PATCH 2/2] powerpc: Make context bits depend on virtual addr size.

[Geoff Levand]

Hi Aneesh,

On Wed, 2013-02-13 at 17:10 +0530, Aneesh Kumar K.V wrote:
> Ok. How about the below patch. This is based on the suggestion from
> Paulus. I still have to take care of few comments in the code.
> 
> Phileas and Geoff,
> 
> Can we check whether this fix the PS3 boot hang ?

I did a quick build and boot test, and this one seems to work OK.

-Geoff