[PATCH v3 1/4] powerpc: Introduce local (non-broadcast) forms of tlb invalidates

[PATCH v3 1/4] powerpc: Introduce local (non-broadcast) forms of tlb invalidates

[Kumar Gala]

Introduced a new set of low level tlb invalidate functions that do not
broadcast invalidates on the bus:

_tlbil_all - invalidate all
_tlbil_pid - invalidate based on process id (or mm context)
_tlbil_va  - invalidate based on virtual address (ea + pid)

On non-SMP configs _tlbil_all should be functionally equivalent to _tlbia and
_tlbil_va should be functionally equivalent to _tlbie.

The intent of this change is to handle SMP based invalidates via IPIs instead
of broadcasts as the mechanism scales better for larger number of cores.

On e500 (fsl-booke mmu) based cores move to using MMUCSR for invalidate alls
and tlbsx/tlbwe for invalidate virtual address.

Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---

Fixed _tlbil_pid not to clober the stack pointer.

- k

 arch/powerpc/include/asm/reg_booke.h |    7 ++++
 arch/powerpc/include/asm/tlbflush.h  |   13 +++++---
 arch/powerpc/kernel/misc_32.S        |   52 ++++++++++++++++++++++++++++++++++
 arch/powerpc/kernel/ppc_ksyms.c      |    3 ++
 4 files changed, 70 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/reg_booke.h b/arch/powerpc/include/asm/reg_booke.h
index be980f4..6745376 100644
--- a/arch/powerpc/include/asm/reg_booke.h
+++ b/arch/powerpc/include/asm/reg_booke.h
@@ -109,6 +109,7 @@
 #define SPRN_EVPR	0x3D6	/* Exception Vector Prefix Register */
 #define SPRN_L1CSR0	0x3F2	/* L1 Cache Control and Status Register 0 */
 #define SPRN_L1CSR1	0x3F3	/* L1 Cache Control and Status Register 1 */
+#define SPRN_MMUCSR0	0x3F4	/* MMU Control and Status Register 0 */
 #define SPRN_PIT	0x3DB	/* Programmable Interval Timer */
 #define SPRN_BUCSR	0x3F5	/* Branch Unit Control and Status */
 #define SPRN_L2CSR0	0x3F9	/* L2 Data Cache Control and Status Register 0 */
@@ -410,6 +411,12 @@
 #define L2CSR0_L2LOA	0x00000080	/* L2 Cache Lock Overflow Allocate */
 #define L2CSR0_L2LO	0x00000020	/* L2 Cache Lock Overflow */
 
+/* Bit definitions for MMUCSR0 */
+#define MMUCSR0_TLB1FI	0x00000002	/* TLB1 Flash invalidate */
+#define MMUCSR0_TLB0FI	0x00000004	/* TLB0 Flash invalidate */
+#define MMUCSR0_TLB2FI	0x00000040	/* TLB2 Flash invalidate */
+#define MMUCSR0_TLB3FI	0x00000020	/* TLB3 Flash invalidate */
+
 /* Bit definitions for SGR. */
 #define SGR_NORMAL	0		/* Speculative fetching allowed. */
 #define SGR_GUARDED	1		/* Speculative fetching disallowed. */
diff --git a/arch/powerpc/include/asm/tlbflush.h b/arch/powerpc/include/asm/tlbflush.h
index 361cd5c..38ec1e7 100644
--- a/arch/powerpc/include/asm/tlbflush.h
+++ b/arch/powerpc/include/asm/tlbflush.h
@@ -29,6 +29,9 @@
 #include <linux/mm.h>
 
 extern void _tlbie(unsigned long address, unsigned int pid);
+extern void _tlbil_all(void);
+extern void _tlbil_pid(unsigned int pid);
+extern void _tlbil_va(unsigned long address, unsigned int pid);
 
 #if defined(CONFIG_40x) || defined(CONFIG_8xx)
 #define _tlbia()	asm volatile ("tlbia; sync" : : : "memory")
@@ -38,31 +41,31 @@ extern void _tlbia(void);
 
 static inline void flush_tlb_mm(struct mm_struct *mm)
 {
-	_tlbia();
+	_tlbil_all();
 }
 
 static inline void flush_tlb_page(struct vm_area_struct *vma,
 				  unsigned long vmaddr)
 {
-	_tlbie(vmaddr, vma ? vma->vm_mm->context.id : 0);
+	_tlbil_va(vmaddr, vma ? vma->vm_mm->context.id : 0);
 }
 
 static inline void flush_tlb_page_nohash(struct vm_area_struct *vma,
 					 unsigned long vmaddr)
 {
-	_tlbie(vmaddr, vma ? vma->vm_mm->context.id : 0);
+	flush_tlb_page(vma, vmaddr);
 }
 
 static inline void flush_tlb_range(struct vm_area_struct *vma,
 				   unsigned long start, unsigned long end)
 {
-	_tlbia();
+	_tlbil_all();
 }
 
 static inline void flush_tlb_kernel_range(unsigned long start,
 					  unsigned long end)
 {
-	_tlbia();
+	_tlbil_all();
 }
 
 #elif defined(CONFIG_PPC32)
diff --git a/arch/powerpc/kernel/misc_32.S b/arch/powerpc/kernel/misc_32.S
index 7a6dfbc..430bbce 100644
--- a/arch/powerpc/kernel/misc_32.S
+++ b/arch/powerpc/kernel/misc_32.S
@@ -274,6 +274,9 @@ _GLOBAL(real_writeb)
 /*
  * Flush MMU TLB
  */
+#ifndef CONFIG_FSL_BOOKE
+_GLOBAL(_tlbil_all)
+#endif
 _GLOBAL(_tlbia)
 #if defined(CONFIG_40x)
 	sync			/* Flush to memory before changing mapping */
@@ -344,6 +347,9 @@ _GLOBAL(_tlbia)
 /*
  * Flush MMU TLB for a particular address
  */
+#ifndef CONFIG_FSL_BOOKE
+_GLOBAL(_tlbil_va)
+#endif
 _GLOBAL(_tlbie)
 #if defined(CONFIG_40x)
 	/* We run the search with interrupts disabled because we have to change
@@ -436,6 +442,52 @@ _GLOBAL(_tlbie)
 #endif /* ! CONFIG_40x */
 	blr
 
+#if defined(CONFIG_FSL_BOOKE)
+/*
+ * Flush MMU TLB, but only on the local processor (no broadcast)
+ */
+_GLOBAL(_tlbil_all)
+#define MMUCSR0_TLBFI	(MMUCSR0_TLB0FI | MMUCSR0_TLB1FI | \
+			 MMUCSR0_TLB2FI | MMUCSR0_TLB3FI)
+	li	r3,(MMUCSR0_TLBFI)@l
+	mtspr	SPRN_MMUCSR0, r3
+1:
+	mfspr	r3,SPRN_MMUCSR0
+	andi.	r3,r3,MMUCSR0_TLBFI@l
+	bne	1b
+	blr
+
+/*
+ * Flush MMU TLB for a particular process id, but only on the local processor
+ * (no broadcast)
+ */
+_GLOBAL(_tlbil_pid)
+	li	r3,(MMUCSR0_TLBFI)@l
+	mtspr	SPRN_MMUCSR0, r3
+1:
+	mfspr	r3,SPRN_MMUCSR0
+	andi.	r3,r3,MMUCSR0_TLBFI@l
+	bne	1b
+	blr
+
+/*
+ * Flush MMU TLB for a particular address, but only on the local processor
+ * (no broadcast)
+ */
+_GLOBAL(_tlbil_va)
+	slwi	r4,r4,16
+	mtspr	SPRN_MAS6,r4		/* assume AS=0 for now */
+	tlbsx	0,r3
+	mfspr	r4,SPRN_MAS1		/* check valid */
+	andis.	r3,r4,MAS1_VALID@h
+	beqlr
+	rlwinm	r4,r4,0,1,31
+	mtspr	SPRN_MAS1,r4
+	tlbwe
+	blr
+#endif /* CONFIG_FSL_BOOKE */
+
+
 /*
  * Flush instruction cache.
  * This is a no-op on the 601.
diff --git a/arch/powerpc/kernel/ppc_ksyms.c b/arch/powerpc/kernel/ppc_ksyms.c
index e1ea4fe..8edc235 100644
--- a/arch/powerpc/kernel/ppc_ksyms.c
+++ b/arch/powerpc/kernel/ppc_ksyms.c
@@ -119,6 +119,9 @@ EXPORT_SYMBOL(flush_instruction_cache);
 EXPORT_SYMBOL(flush_tlb_kernel_range);
 EXPORT_SYMBOL(flush_tlb_page);
 EXPORT_SYMBOL(_tlbie);
+#if defined(CONFIG_4xx) || defined(CONFIG_8xx) || defined(CONFIG_FSL_BOOKE)
+EXPORT_SYMBOL(_tlbil_va);
+#endif
 #endif
 EXPORT_SYMBOL(__flush_icache_range);
 EXPORT_SYMBOL(flush_dcache_range);
-- 
1.5.5.1

[PATCH v3 2/4] powerpc: Fixes for CONFIG_PTE_64BIT for SMP support

[Kumar Gala]

There are some minor issues with support 64-bit PTEs on a 32-bit processor
when dealing with SMP.

* We need to order the stores in set_pte_at to make sure the flag word
  is set second.
* Change pte_clear to use pte_update so only the flag word is cleared

Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---

Fixed pte_clear to not break on 6xx.

- k

 arch/powerpc/include/asm/pgtable-ppc32.h |   15 +++++++++++----
 1 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/include/asm/pgtable-ppc32.h b/arch/powerpc/include/asm/pgtable-ppc32.h
index 6fe39e3..dff1b7e 100644
--- a/arch/powerpc/include/asm/pgtable-ppc32.h
+++ b/arch/powerpc/include/asm/pgtable-ppc32.h
@@ -517,7 +517,8 @@ extern unsigned long bad_call_to_PMD_PAGE_SIZE(void);
 
 #define pte_none(pte)		((pte_val(pte) & ~_PTE_NONE_MASK) == 0)
 #define pte_present(pte)	(pte_val(pte) & _PAGE_PRESENT)
-#define pte_clear(mm,addr,ptep)	do { set_pte_at((mm), (addr), (ptep), __pte(0)); } while (0)
+#define pte_clear(mm, addr, ptep) \
+	do { pte_update(ptep, ~_PAGE_HASHPTE, 0); } while (0)
 
 #define pmd_none(pmd)		(!pmd_val(pmd))
 #define	pmd_bad(pmd)		(pmd_val(pmd) & _PMD_BAD)
@@ -612,9 +613,6 @@ static inline unsigned long pte_update(pte_t *p,
 	return old;
 }
 #else /* CONFIG_PTE_64BIT */
-/* TODO: Change that to only modify the low word and move set_pte_at()
- * out of line
- */
 static inline unsigned long long pte_update(pte_t *p,
 					    unsigned long clr,
 					    unsigned long set)
@@ -658,8 +656,17 @@ static inline void set_pte_at(struct mm_struct *mm, unsigned long addr,
 #if _PAGE_HASHPTE != 0
 	pte_update(ptep, ~_PAGE_HASHPTE, pte_val(pte) & ~_PAGE_HASHPTE);
 #else
+#if defined(CONFIG_PTE_64BIT) && defined(CONFIG_SMP)
+	__asm__ __volatile__("\
+		stw%U0%X0 %2,%0\n\
+		eieio\n\
+		stw%U0%X0 %L2,%1"
+	: "=m" (*ptep), "=m" (*((unsigned char *)ptep+4))
+	: "r" (pte) : "memory");
+#else
 	*ptep = pte;
 #endif
+#endif
 }
 
 /*
-- 
1.5.5.1

[PATCH v3 3/4] powerpc/fsl-booke: Fixup 64-bit PTE reading for SMP support

[Kumar Gala]

We need to create a false data dependency to ensure the loads of
the pte are done in the right order.

Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 arch/powerpc/kernel/head_fsl_booke.S |   24 ++++++++++++++++++++----
 1 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S
index 3cb52fa..fa39cce 100644
--- a/arch/powerpc/kernel/head_fsl_booke.S
+++ b/arch/powerpc/kernel/head_fsl_booke.S
@@ -579,13 +579,19 @@ interrupt_base:
 
 	FIND_PTE
 	andc.	r13,r13,r11		/* Check permission */
-	bne	2f			/* Bail if permission mismach */
 
 #ifdef CONFIG_PTE_64BIT
-	lwz	r13, 0(r12)
+#ifdef CONFIG_SMP
+	subf	r10, r11, r12		/* create false data dep */
+	lwzx	r13, r11, r10		/* Get upper pte bits */
+#else
+	lwz	r13, 0(r12)		/* Get upper pte bits */
 #endif
+#endif
+
+	bne	2f			/* Bail if permission/valid mismach */
 
-	 /* Jump to common tlb load */
+	/* Jump to common tlb load */
 	b	finish_tlb_load
 2:
 	/* The bailout.  Restore registers to pre-exception conditions
@@ -640,6 +646,16 @@ interrupt_base:
 
 	FIND_PTE
 	andc.	r13,r13,r11		/* Check permission */
+
+#ifdef CONFIG_PTE_64BIT
+#ifdef CONFIG_SMP
+	subf	r10, r11, r12		/* create false data dep */
+	lwzx	r13, r11, r10		/* Get upper pte bits */
+#else
+	lwz	r13, 0(r12)		/* Get upper pte bits */
+#endif
+#endif
+
 	bne	2f			/* Bail if permission mismach */
 
 #ifdef CONFIG_PTE_64BIT
@@ -702,7 +718,7 @@ interrupt_base:
 /*
  * Both the instruction and data TLB miss get to this
  * point to load the TLB.
- *	r10 - EA of fault
+ *	r10 - available to use
  *	r11 - TLB (info from Linux PTE)
  *	r12 - available to use
  *	r13 - upper bits of PTE (if PTE_64BIT) or available to use
-- 
1.5.5.1

[PATCH v3 4/4] powerpc/mm: Implement _PAGE_SPECIAL & pte_special() for 32-bit

[Kumar Gala]

Implement _PAGE_SPECIAL and pte_special() for 32-bit powerpc. This bit will
be used by the fast get_user_pages() to differenciate PTEs that correspond
to a valid struct page from special mappings that don't such as IO mappings
obtained via io_remap_pfn_ranges().

We currently only implement this on sub-arch that support SMP or will so
in the future (6xx, 44x, FSL-BookE) and not (8xx, 40x).

Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 arch/powerpc/include/asm/pgtable-ppc32.h |   15 +++++++++++++--
 1 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/pgtable-ppc32.h b/arch/powerpc/include/asm/pgtable-ppc32.h
index dff1b7e..9fc6250 100644
--- a/arch/powerpc/include/asm/pgtable-ppc32.h
+++ b/arch/powerpc/include/asm/pgtable-ppc32.h
@@ -261,6 +261,7 @@ extern int icache_44x_need_flush;
 #define _PAGE_HWEXEC	0x00000004		/* H: Execute permission */
 #define _PAGE_ACCESSED	0x00000008		/* S: Page referenced */
 #define _PAGE_DIRTY	0x00000010		/* S: Page dirty */
+#define _PAGE_SPECIAL	0x00000020		/* S: Special page */
 #define _PAGE_USER	0x00000040		/* S: User page */
 #define _PAGE_ENDIAN	0x00000080		/* H: E bit */
 #define _PAGE_GUARDED	0x00000100		/* H: G bit */
@@ -276,6 +277,7 @@ extern int icache_44x_need_flush;
 /* ERPN in a PTE never gets cleared, ignore it */
 #define _PTE_NONE_MASK	0xffffffff00000000ULL
 
+#define __HAVE_ARCH_PTE_SPECIAL
 
 #elif defined(CONFIG_FSL_BOOKE)
 /*
@@ -305,6 +307,7 @@ extern int icache_44x_need_flush;
 #define _PAGE_COHERENT	0x00100	/* H: M bit */
 #define _PAGE_NO_CACHE	0x00200	/* H: I bit */
 #define _PAGE_WRITETHRU	0x00400	/* H: W bit */
+#define _PAGE_SPECIAL	0x00800 /* S: Special page */
 
 #ifdef CONFIG_PTE_64BIT
 /* ERPN in a PTE never gets cleared, ignore it */
@@ -315,6 +318,8 @@ extern int icache_44x_need_flush;
 #define _PMD_PRESENT_MASK (PAGE_MASK)
 #define _PMD_BAD	(~PAGE_MASK)
 
+#define __HAVE_ARCH_PTE_SPECIAL
+
 #elif defined(CONFIG_8xx)
 /* Definitions for 8xx embedded chips. */
 #define _PAGE_PRESENT	0x0001	/* Page is valid */
@@ -362,6 +367,7 @@ extern int icache_44x_need_flush;
 #define _PAGE_ACCESSED	0x100	/* R: page referenced */
 #define _PAGE_EXEC	0x200	/* software: i-cache coherency required */
 #define _PAGE_RW	0x400	/* software: user write access allowed */
+#define _PAGE_SPECIAL	0x800	/* software: Special page */
 
 #define _PTE_NONE_MASK	_PAGE_HASHPTE
 
@@ -372,6 +378,8 @@ extern int icache_44x_need_flush;
 /* Hash table based platforms need atomic updates of the linux PTE */
 #define PTE_ATOMIC_UPDATES	1
 
+#define __HAVE_ARCH_PTE_SPECIAL
+
 #endif
 
 /*
@@ -404,6 +412,9 @@ extern int icache_44x_need_flush;
 #ifndef _PAGE_WRITETHRU
 #define _PAGE_WRITETHRU	0
 #endif
+#ifndef _PAGE_SPECIAL
+#define _PAGE_SPECIAL	0
+#endif
 #ifndef _PMD_PRESENT_MASK
 #define _PMD_PRESENT_MASK	_PMD_PRESENT
 #endif
@@ -534,7 +545,7 @@ static inline int pte_write(pte_t pte)		{ return pte_val(pte) & _PAGE_RW; }
 static inline int pte_dirty(pte_t pte)		{ return pte_val(pte) & _PAGE_DIRTY; }
 static inline int pte_young(pte_t pte)		{ return pte_val(pte) & _PAGE_ACCESSED; }
 static inline int pte_file(pte_t pte)		{ return pte_val(pte) & _PAGE_FILE; }
-static inline int pte_special(pte_t pte)	{ return 0; }
+static inline int pte_special(pte_t pte)	{ return pte_val(pte) & _PAGE_SPECIAL; }
 
 static inline void pte_uncache(pte_t pte)       { pte_val(pte) |= _PAGE_NO_CACHE; }
 static inline void pte_cache(pte_t pte)         { pte_val(pte) &= ~_PAGE_NO_CACHE; }
@@ -553,7 +564,7 @@ static inline pte_t pte_mkdirty(pte_t pte) {
 static inline pte_t pte_mkyoung(pte_t pte) {
 	pte_val(pte) |= _PAGE_ACCESSED; return pte; }
 static inline pte_t pte_mkspecial(pte_t pte) {
-	return pte; }
+	pte_val(pte) |= _PAGE_SPECIAL; return pte; }
 static inline unsigned long pte_pgprot(pte_t pte)
 {
 	return __pgprot(pte_val(pte)) & PAGE_PROT_BITS;
-- 
1.5.5.1

Re: [PATCH v3 4/4] powerpc/mm: Implement _PAGE_SPECIAL & pte_special() for 32-bit

[Benjamin Herrenschmidt]

On Wed, 2008-09-03 at 13:09 -0500, Kumar Gala wrote:
> Implement _PAGE_SPECIAL and pte_special() for 32-bit powerpc. This bit will
> be used by the fast get_user_pages() to differenciate PTEs that correspond
> to a valid struct page from special mappings that don't such as IO mappings
> obtained via io_remap_pfn_ranges().
> 
> We currently only implement this on sub-arch that support SMP or will so
> in the future (6xx, 44x, FSL-BookE) and not (8xx, 40x).
> 
> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

> ---
>  arch/powerpc/include/asm/pgtable-ppc32.h |   15 +++++++++++++--
>  1 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/pgtable-ppc32.h b/arch/powerpc/include/asm/pgtable-ppc32.h
> index dff1b7e..9fc6250 100644
> --- a/arch/powerpc/include/asm/pgtable-ppc32.h
> +++ b/arch/powerpc/include/asm/pgtable-ppc32.h
> @@ -261,6 +261,7 @@ extern int icache_44x_need_flush;
>  #define _PAGE_HWEXEC	0x00000004		/* H: Execute permission */
>  #define _PAGE_ACCESSED	0x00000008		/* S: Page referenced */
>  #define _PAGE_DIRTY	0x00000010		/* S: Page dirty */
> +#define _PAGE_SPECIAL	0x00000020		/* S: Special page */
>  #define _PAGE_USER	0x00000040		/* S: User page */
>  #define _PAGE_ENDIAN	0x00000080		/* H: E bit */
>  #define _PAGE_GUARDED	0x00000100		/* H: G bit */
> @@ -276,6 +277,7 @@ extern int icache_44x_need_flush;
>  /* ERPN in a PTE never gets cleared, ignore it */
>  #define _PTE_NONE_MASK	0xffffffff00000000ULL
>  
> +#define __HAVE_ARCH_PTE_SPECIAL
>  
>  #elif defined(CONFIG_FSL_BOOKE)
>  /*
> @@ -305,6 +307,7 @@ extern int icache_44x_need_flush;
>  #define _PAGE_COHERENT	0x00100	/* H: M bit */
>  #define _PAGE_NO_CACHE	0x00200	/* H: I bit */
>  #define _PAGE_WRITETHRU	0x00400	/* H: W bit */
> +#define _PAGE_SPECIAL	0x00800 /* S: Special page */
>  
>  #ifdef CONFIG_PTE_64BIT
>  /* ERPN in a PTE never gets cleared, ignore it */
> @@ -315,6 +318,8 @@ extern int icache_44x_need_flush;
>  #define _PMD_PRESENT_MASK (PAGE_MASK)
>  #define _PMD_BAD	(~PAGE_MASK)
>  
> +#define __HAVE_ARCH_PTE_SPECIAL
> +
>  #elif defined(CONFIG_8xx)
>  /* Definitions for 8xx embedded chips. */
>  #define _PAGE_PRESENT	0x0001	/* Page is valid */
> @@ -362,6 +367,7 @@ extern int icache_44x_need_flush;
>  #define _PAGE_ACCESSED	0x100	/* R: page referenced */
>  #define _PAGE_EXEC	0x200	/* software: i-cache coherency required */
>  #define _PAGE_RW	0x400	/* software: user write access allowed */
> +#define _PAGE_SPECIAL	0x800	/* software: Special page */
>  
>  #define _PTE_NONE_MASK	_PAGE_HASHPTE
>  
> @@ -372,6 +378,8 @@ extern int icache_44x_need_flush;
>  /* Hash table based platforms need atomic updates of the linux PTE */
>  #define PTE_ATOMIC_UPDATES	1
>  
> +#define __HAVE_ARCH_PTE_SPECIAL
> +
>  #endif
>  
>  /*
> @@ -404,6 +412,9 @@ extern int icache_44x_need_flush;
>  #ifndef _PAGE_WRITETHRU
>  #define _PAGE_WRITETHRU	0
>  #endif
> +#ifndef _PAGE_SPECIAL
> +#define _PAGE_SPECIAL	0
> +#endif
>  #ifndef _PMD_PRESENT_MASK
>  #define _PMD_PRESENT_MASK	_PMD_PRESENT
>  #endif
> @@ -534,7 +545,7 @@ static inline int pte_write(pte_t pte)		{ return pte_val(pte) & _PAGE_RW; }
>  static inline int pte_dirty(pte_t pte)		{ return pte_val(pte) & _PAGE_DIRTY; }
>  static inline int pte_young(pte_t pte)		{ return pte_val(pte) & _PAGE_ACCESSED; }
>  static inline int pte_file(pte_t pte)		{ return pte_val(pte) & _PAGE_FILE; }
> -static inline int pte_special(pte_t pte)	{ return 0; }
> +static inline int pte_special(pte_t pte)	{ return pte_val(pte) & _PAGE_SPECIAL; }
>  
>  static inline void pte_uncache(pte_t pte)       { pte_val(pte) |= _PAGE_NO_CACHE; }
>  static inline void pte_cache(pte_t pte)         { pte_val(pte) &= ~_PAGE_NO_CACHE; }
> @@ -553,7 +564,7 @@ static inline pte_t pte_mkdirty(pte_t pte) {
>  static inline pte_t pte_mkyoung(pte_t pte) {
>  	pte_val(pte) |= _PAGE_ACCESSED; return pte; }
>  static inline pte_t pte_mkspecial(pte_t pte) {
> -	return pte; }
> +	pte_val(pte) |= _PAGE_SPECIAL; return pte; }
>  static inline unsigned long pte_pgprot(pte_t pte)
>  {
>  	return __pgprot(pte_val(pte)) & PAGE_PROT_BITS;

Re: [PATCH v3 3/4] powerpc/fsl-booke: Fixup 64-bit PTE reading for SMP support

[Benjamin Herrenschmidt]

>  	FIND_PTE
>  	andc.	r13,r13,r11		/* Check permission */
> -	bne	2f			/* Bail if permission mismach */
>  
>  #ifdef CONFIG_PTE_64BIT
> -	lwz	r13, 0(r12)
> +#ifdef CONFIG_SMP
> +	subf	r10, r11, r12		/* create false data dep */
> +	lwzx	r13, r11, r10		/* Get upper pte bits */
> +#else
> +	lwz	r13, 0(r12)		/* Get upper pte bits */
>  #endif
> +#endif

Minor nit, but we don't usually use spaces after "," in asm.

Cheers,
Ben.

Re: [PATCH v3 2/4] powerpc: Fixes for CONFIG_PTE_64BIT for SMP support

[Benjamin Herrenschmidt]

On Wed, 2008-09-03 at 13:09 -0500, Kumar Gala wrote:
> There are some minor issues with support 64-bit PTEs on a 32-bit processor
> when dealing with SMP.
> 
> * We need to order the stores in set_pte_at to make sure the flag word
>   is set second.
> * Change pte_clear to use pte_update so only the flag word is cleared
> 
> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
> ---
> 
> Fixed pte_clear to not break on 6xx.

Thanks :-)

>  static inline unsigned long long pte_update(pte_t *p,
>  					    unsigned long clr,
>  					    unsigned long set)
> @@ -658,8 +656,17 @@ static inline void set_pte_at(struct mm_struct *mm, unsigned long addr,
>  #if _PAGE_HASHPTE != 0
>  	pte_update(ptep, ~_PAGE_HASHPTE, pte_val(pte) & ~_PAGE_HASHPTE);
>  #else
> +#if defined(CONFIG_PTE_64BIT) && defined(CONFIG_SMP)

Minor nit, but there's #elif :-)

> +	__asm__ __volatile__("\
> +		stw%U0%X0 %2,%0\n\
> +		eieio\n\
> +		stw%U0%X0 %L2,%1"
> +	: "=m" (*ptep), "=m" (*((unsigned char *)ptep+4))
> +	: "r" (pte) : "memory");

Any reason why it has to be in assembly ? Can gcc re-order stores around
eieio() if written in C ? I would hope not but heh ... it's gcc :-)

I also wonder if you should first ensure that the PTE is invalid and
if not, clear it and flush the TLB page ... Or at least add a
WARN_ON(pte_valid()) in case we get that wrong ...

> +#else
>  	*ptep = pte;
>  #endif
> +#endif
>  }
>  
>  /*

Re: [PATCH v3 2/4] powerpc: Fixes for CONFIG_PTE_64BIT for SMP support

[Kumar Gala]

On Sep 3, 2008, at 10:12 PM, Benjamin Herrenschmidt wrote:

> On Wed, 2008-09-03 at 13:09 -0500, Kumar Gala wrote:
>> There are some minor issues with support 64-bit PTEs on a 32-bit  
>> processor
>> when dealing with SMP.
>>
>> * We need to order the stores in set_pte_at to make sure the flag  
>> word
>>  is set second.
>> * Change pte_clear to use pte_update so only the flag word is cleared
>>
>> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
>> ---
>>
>> Fixed pte_clear to not break on 6xx.
>
> Thanks :-)
>
>> static inline unsigned long long pte_update(pte_t *p,
>> 					    unsigned long clr,
>> 					    unsigned long set)
>> @@ -658,8 +656,17 @@ static inline void set_pte_at(struct mm_struct  
>> *mm, unsigned long addr,
>> #if _PAGE_HASHPTE != 0
>> 	pte_update(ptep, ~_PAGE_HASHPTE, pte_val(pte) & ~_PAGE_HASHPTE);
>> #else
>> +#if defined(CONFIG_PTE_64BIT) && defined(CONFIG_SMP)
>
> Minor nit, but there's #elif :-)
>
>> +	__asm__ __volatile__("\
>> +		stw%U0%X0 %2,%0\n\
>> +		eieio\n\
>> +		stw%U0%X0 %L2,%1"
>> +	: "=m" (*ptep), "=m" (*((unsigned char *)ptep+4))
>> +	: "r" (pte) : "memory");
>
> Any reason why it has to be in assembly ? Can gcc re-order stores  
> around
> eieio() if written in C ? I would hope not but heh ... it's gcc :-)

I'm leaving it asm.  Its more explicit and easier to read in my  
opinion and I don't give gcc a chance to screw us over.
>
> I also wonder if you should first ensure that the PTE is invalid and
> if not, clear it and flush the TLB page ... Or at least add a
> WARN_ON(pte_valid()) in case we get that wrong ...

I believe that's an issue since kmap_atomic() will call set_pte_at and  
have the valid bit set.

- k

Re: [PATCH v3 2/4] powerpc: Fixes for CONFIG_PTE_64BIT for SMP support

[Benjamin Herrenschmidt]

On Fri, 2008-09-05 at 14:44 -0500, Kumar Gala wrote:
> > I also wonder if you should first ensure that the PTE is invalid and
> > if not, clear it and flush the TLB page ... Or at least add a
> > WARN_ON(pte_valid()) in case we get that wrong ...
> 
> I believe that's an issue since kmap_atomic() will call set_pte_at and  
> have the valid bit set.

Hrm... on the other hand, it's safe because kmap_atomic() is per-CPU and
thus won't race with anything...

On the other hand, if others (mprotect, mremap, whoever...) does it,
it's not safe.

I'm keen on letting set_pte_at() to the job and maybe if we can remove
the explicit flush kmap_atomic does... Though it's not totally trivial
to "know" it's a flush that doesn't need global invalidations...

May be worth moving your current stuff into a __set_pte_at() that you
use from kmap atomic, and have set_pte_at() wrap that along with a
present warn. That would do for the time being.

Ben.

Re: [PATCH v3 2/4] powerpc: Fixes for CONFIG_PTE_64BIT for SMP support

[Kumar Gala]

On Sep 5, 2008, at 5:38 PM, Benjamin Herrenschmidt wrote:

> On Fri, 2008-09-05 at 14:44 -0500, Kumar Gala wrote:
>>> I also wonder if you should first ensure that the PTE is invalid and
>>> if not, clear it and flush the TLB page ... Or at least add a
>>> WARN_ON(pte_valid()) in case we get that wrong ...
>>
>> I believe that's an issue since kmap_atomic() will call set_pte_at  
>> and
>> have the valid bit set.
>
> Hrm... on the other hand, it's safe because kmap_atomic() is per-CPU  
> and
> thus won't race with anything...
>
> On the other hand, if others (mprotect, mremap, whoever...) does it,
> it's not safe.
>
> I'm keen on letting set_pte_at() to the job and maybe if we can remove
> the explicit flush kmap_atomic does... Though it's not totally trivial
> to "know" it's a flush that doesn't need global invalidations...
>
> May be worth moving your current stuff into a __set_pte_at() that you
> use from kmap atomic, and have set_pte_at() wrap that along with a
> present warn. That would do for the time being.

That works for me.

- k

Re: [PATCH v3 1/4] powerpc: Introduce local (non-broadcast) forms of tlb invalidates

[Rafal Jaworowski]

Kumar Gala wrote:
[...]
> The intent of this change is to handle SMP based invalidates via IPIs instead
> of broadcasts as the mechanism scales better for larger number of cores.

Hi Kumar,

How is the inter-IPI deadlock avoidance designed in this new approach? I don't
know the close details of low-level Book-E VM design in Linux, but am thinking
about a scenario when we have two TLB misses hitting almost immediately on two
different cores and they both want to send a TLB invalidate IPI to each other.
How do you manage this?

The reason I ask is we had similar considerations (and problems) when bringing
SMP to the dual core e500 on FreeBSD and ended up not using IPIs, at least for
now, because of such concerns (and actual problems of this kind).

Rafal

Re: [PATCH v3 1/4] powerpc: Introduce local (non-broadcast) forms of tlb invalidates

[Benjamin Herrenschmidt]

On Thu, 2008-09-04 at 21:17 +0200, Rafal Jaworowski wrote:
> Kumar Gala wrote:
> [...]
> > The intent of this change is to handle SMP based invalidates via IPIs instead
> > of broadcasts as the mechanism scales better for larger number of cores.
> 
> Hi Kumar,
> 
> How is the inter-IPI deadlock avoidance designed in this new approach? I don't
> know the close details of low-level Book-E VM design in Linux, but am thinking
> about a scenario when we have two TLB misses hitting almost immediately on two
> different cores and they both want to send a TLB invalidate IPI to each other.
> How do you manage this?

Simple.. Just look how it's done on x86 :-) The flush_tlb_* operations
happen with interrupt enabled and no major lock held. They shouldn't
deadlock.

> The reason I ask is we had similar considerations (and problems) when bringing
> SMP to the dual core e500 on FreeBSD and ended up not using IPIs, at least for
> now, because of such concerns (and actual problems of this kind).

Well, it depends how your VM is designed. The linux one has beeing doing
IPIs for invalidations forever on x86 so it's not hard to adapt.

Ben.