[PATCH] (urgent) ppc32: Fix CPUs with soft loaded TLB

[PATCH] (urgent) ppc32: Fix CPUs with soft loaded TLB

[Benjamin Herrenschmidt]

Hi !

The recent introduction of ptep_set_access_flags() with the optimisation
of not flushing the TLB unfortunately broke ppc32 CPUs with no hash table.

The data access exception code path in assembly for these doesn't properly
deal with the case where the TLB entry is present with the wrong PAGE_RW and
will just call do_page_fault again instead of just replacing the TLB entry.

Fixing the asm code for all the different CPU types affected (yah, embedded
PPCs all have different MMUs =P) is painful and need testing I can't do at
the moment, so here's a fix that will just flush the TLB page when changing
the access flags on non-hash based machines. Please apply.

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

===== include/asm-ppc/pgtable.h 1.33 vs edited =====
--- 1.33/include/asm-ppc/pgtable.h	2004-05-26 09:56:17 -05:00
+++ edited/include/asm-ppc/pgtable.h	2004-06-06 16:02:27 -05:00
@@ -555,8 +555,12 @@
 		(_PAGE_DIRTY | _PAGE_ACCESSED | _PAGE_RW);
 	pte_update(ptep, 0, bits);
 }
+
 #define  ptep_set_access_flags(__vma, __address, __ptep, __entry, __dirty) \
-        __ptep_set_access_flags(__ptep, __entry, __dirty)
+	do {								   \
+		__ptep_set_access_flags(__ptep, __entry, __dirty);	   \
+		flush_tlb_page_nohash(__vma, __address);	       	   \
+	} while(0)
 
 /*
  * Macro to mark a page protection value as "uncacheable".
===== arch/ppc/mm/tlb.c 1.10 vs edited =====
--- 1.10/arch/ppc/mm/tlb.c	2004-02-04 23:00:14 -06:00
+++ edited/arch/ppc/mm/tlb.c	2004-06-06 16:01:05 -05:00
@@ -67,6 +67,17 @@
 }
 
 /*
+ * Called by ptep_set_access_flags, must flush on CPUs for which the
+ * DSI handler can't just "fixup" the TLB on a write fault
+ */
+void flush_tlb_page_nohash(struct vm_area_struct *vma, unsigned long addr)
+{
+	if (Hash != 0)
+		return;
+	_tlbie(addr);
+}
+
+/*
  * Called at the end of a mmu_gather operation to make sure the
  * TLB flush is completely done.
  */

Re: [PATCH] (urgent) ppc32: Fix CPUs with soft loaded TLB

[Linus Torvalds]

On Sun, 6 Jun 2004, Benjamin Herrenschmidt wrote:
> 
> The recent introduction of ptep_set_access_flags() with the optimisation
> of not flushing the TLB unfortunately broke ppc32 CPUs with no hash table.

Makes sense, applied.

However, wouldn't it make sense to have this on the ppc64 branch too?

Admittedly on ppc64, the flush_tlb_page_nohash() function would be a
no-op, since it always has the hash tables, but I'm a blue-eyed optimists,
and I'm still hoping that some day IBM will see the error of their ways, 
and get rid of the hash tables entirely. At which point ppc64 too will 
need to flush the TLB entry.

		Linus

Re: [PATCH] (urgent) ppc32: Fix CPUs with soft loaded TLB

[Benjamin Herrenschmidt]

On Sun, 2004-06-06 at 16:20, Linus Torvalds wrote:
> On Sun, 6 Jun 2004, Benjamin Herrenschmidt wrote:
> > 
> > The recent introduction of ptep_set_access_flags() with the optimisation
> > of not flushing the TLB unfortunately broke ppc32 CPUs with no hash table.
> 
> Makes sense, applied.

ARGH. Missed one file. Here is an additional patch (missed tlbflush.h patch)

Sorry.

This adds the definiction of flush_tlb_page_nohash() that was missing
from the previous patch fixing SW-TLB loaded PPCs

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
 
===== include/asm-ppc/tlbflush.h 1.9 vs edited =====
--- 1.9/include/asm-ppc/tlbflush.h	2003-09-15 15:59:05 -05:00
+++ edited/include/asm-ppc/tlbflush.h	2004-06-06 16:01:50 -05:00
@@ -29,6 +29,9 @@
 static inline void flush_tlb_page(struct vm_area_struct *vma,
 				unsigned long vmaddr)
 	{ _tlbie(vmaddr); }
+static inline void flush_tlb_page_nohash(struct vm_area_struct *vma,
+					 unsigned long vmaddr)
+	{ _tlbie(vmaddr); }
 static inline void flush_tlb_range(struct vm_area_struct *vma,
 				unsigned long start, unsigned long end)
 	{ __tlbia(); }
@@ -44,6 +47,9 @@
 static inline void flush_tlb_page(struct vm_area_struct *vma,
 				unsigned long vmaddr)
 	{ _tlbie(vmaddr); }
+static inline void flush_tlb_page_nohash(struct vm_area_struct *vma,
+					 unsigned long vmaddr)
+	{ _tlbie(vmaddr); }
 static inline void flush_tlb_range(struct mm_struct *mm,
 				unsigned long start, unsigned long end)
 	{ __tlbia(); }
@@ -56,6 +62,7 @@
 struct vm_area_struct;
 extern void flush_tlb_mm(struct mm_struct *mm);
 extern void flush_tlb_page(struct vm_area_struct *vma, unsigned long vmaddr);
+extern void flush_tlb_page_nohash(struct vm_area_struct *vma, unsigned long addr);
 extern void flush_tlb_range(struct vm_area_struct *vma, unsigned long start,
 			    unsigned long end);
 extern void flush_tlb_kernel_range(unsigned long start, unsigned long end);

Re: [PATCH] (urgent) ppc32: Fix CPUs with soft loaded TLB

[Benjamin Herrenschmidt]

> Admittedly on ppc64, the flush_tlb_page_nohash() function would be a
> no-op, since it always has the hash tables, but I'm a blue-eyed optimists,
> and I'm still hoping that some day IBM will see the error of their ways, 
> and get rid of the hash tables entirely. At which point ppc64 too will 
> need to flush the TLB entry.

Hrm... we could. In case you really want it, here's the patch:

Adds a dummy flush_tlb_page_nohash() called by ptep_set_access_bits(),
to be used if we ever have a ppc64 CPU with software loaded TLB.

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

===== include/asm-ppc64/pgtable.h 1.35 vs edited =====
--- 1.35/include/asm-ppc64/pgtable.h	2004-05-30 23:33:05 -05:00
+++ edited/include/asm-ppc64/pgtable.h	2004-06-06 16:39:49 -05:00
@@ -428,7 +428,10 @@
 	:"cc");
 }
 #define  ptep_set_access_flags(__vma, __address, __ptep, __entry, __dirty) \
-        __ptep_set_access_flags(__ptep, __entry, __dirty)
+	do {								   \
+		__ptep_set_access_flags(__ptep, __entry, __dirty);	   \
+		flush_tlb_page_nohash(__vma, __address);	       	   \
+	} while(0)
 
 /*
  * Macro to mark a page protection value as "uncacheable".
===== include/asm-ppc64/tlbflush.h 1.5 vs edited =====
--- 1.5/include/asm-ppc64/tlbflush.h	2004-02-27 06:16:07 -06:00
+++ edited/include/asm-ppc64/tlbflush.h	2004-06-06 16:39:29 -05:00
@@ -6,6 +6,7 @@
  *
  *  - flush_tlb_mm(mm) flushes the specified mm context TLB's
  *  - flush_tlb_page(vma, vmaddr) flushes one page
+ *  - flush_tlb_page_nohash(vma, vmaddr) flushes one page if SW loaded TLB
  *  - flush_tlb_range(vma, start, end) flushes a range of pages
  *  - flush_tlb_kernel_range(start, end) flushes a range of kernel pages
  *  - flush_tlb_pgtables(mm, start, end) flushes a range of page tables
@@ -39,6 +40,7 @@
 
 #define flush_tlb_mm(mm)			flush_tlb_pending()
 #define flush_tlb_page(vma, addr)		flush_tlb_pending()
+#define flush_tlb_page_nohash(vma, addr)       	do { } while (0)
 #define flush_tlb_range(vma, start, end) \
 		do { (void)(start); flush_tlb_pending(); } while (0)
 #define flush_tlb_kernel_range(start, end)	flush_tlb_pending()

Re: [PATCH] (urgent) ppc32: Fix CPUs with soft loaded TLB

[Pantelis Antoniou]

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

Benjamin Herrenschmidt wrote:

>On Sun, 2004-06-06 at 16:20, Linus Torvalds wrote:
>
>>On Sun, 6 Jun 2004, Benjamin Herrenschmidt wrote:
>>
>>>The recent introduction of ptep_set_access_flags() with the optimisation
>>>of not flushing the TLB unfortunately broke ppc32 CPUs with no hash table.
>>>
>>Makes sense, applied.
>>
>
>ARGH. Missed one file. Here is an additional patch (missed tlbflush.h patch)
>
>Sorry.
>
>This adds the definiction of flush_tlb_page_nohash() that was missing
>from the previous patch fixing SW-TLB loaded PPCs
>
>Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
>===== include/asm-ppc/tlbflush.h 1.9 vs edited =====
>--- 1.9/include/asm-ppc/tlbflush.h	2003-09-15 15:59:05 -05:00
>+++ edited/include/asm-ppc/tlbflush.h	2004-06-06 16:01:50 -05:00
>@@ -29,6 +29,9 @@
> static inline void flush_tlb_page(struct vm_area_struct *vma,
> 				unsigned long vmaddr)
> 	{ _tlbie(vmaddr); }
>+static inline void flush_tlb_page_nohash(struct vm_area_struct *vma,
>+					 unsigned long vmaddr)
>+	{ _tlbie(vmaddr); }
> static inline void flush_tlb_range(struct vm_area_struct *vma,
> 				unsigned long start, unsigned long end)
> 	{ __tlbia(); }
>@@ -44,6 +47,9 @@
> static inline void flush_tlb_page(struct vm_area_struct *vma,
> 				unsigned long vmaddr)
> 	{ _tlbie(vmaddr); }
>+static inline void flush_tlb_page_nohash(struct vm_area_struct *vma,
>+					 unsigned long vmaddr)
>+	{ _tlbie(vmaddr); }
> static inline void flush_tlb_range(struct mm_struct *mm,
> 				unsigned long start, unsigned long end)
> 	{ __tlbia(); }
>@@ -56,6 +62,7 @@
> struct vm_area_struct;
> extern void flush_tlb_mm(struct mm_struct *mm);
> extern void flush_tlb_page(struct vm_area_struct *vma, unsigned long vmaddr);
>+extern void flush_tlb_page_nohash(struct vm_area_struct *vma, unsigned long addr);
> extern void flush_tlb_range(struct vm_area_struct *vma, unsigned long start,
> 			    unsigned long end);
> extern void flush_tlb_kernel_range(unsigned long start, unsigned long end);
>
>
>
>
>
>
Hi

Unfortunately this is not enough for me on 8xx.

When starting init it bombs with

Freeing unused kernel memory: 60k init
Oops: kernel access of bad area, sig: 11 [#1]
NIP: C00070F0 LR: C000C580 SP: C0FD7E00 REGS: c0fd7d50 TRAP: 0300    Not 
tainted
MSR: 00009032 EE: 1 PR: 0 FP: 0 ME: 1 IR/DR: 11
DAR: 300009D4, DSISR: C2000000
TASK = c0fda870[1] 'init' THREAD: c0fd6000Last syscall: 11
GPR00: C01C4D60 C0FD7E00 C0FDA870 30000000 00000100 00000F3B 30000000 
00000040
GPR08: 00000000 C01C4D60 C0F3E4C0 0001E760 00016FCB 1001F0AC 00FD0100 
007FFE30
GPR16: FFFFFFFF 00F841E0 00000001 007FFEC0 007FFF00 00000000 00000000 
C01C4D60
GPR24: 00000000 C01C3300 00000000 C0F3FF14 300009D4 C0F3E4C0 00F3B889 
C0200760
NIP [c00070f0] __flush_dcache_icache+0x10/0x3c
LR [c000c580] update_mmu_cache+0x98/0x9c
Call trace:
 [c0044270] do_no_page+0x19c/0x35c
 [c00445ec] handle_mm_fault+0xf8/0x178
 [c000bbac] do_page_fault+0x1f0/0x3e8
 [c0004868] handle_page_fault+0xc/0x80


In order to fix this I now have to remove update_mmu_cache  by defining 
it empty.

Please see the following patch.

Regards

Pantelis


[-- Attachment #2: 8xx-tlb.patch --]
[-- Type: text/x-patch, Size: 1398 bytes --]

===== include/asm-ppc/tlbflush.h 1.10 vs edited =====
--- 1.10/include/asm-ppc/tlbflush.h     Mon Jun  7 00:01:50 2004
+++ edited/include/asm-ppc/tlbflush.h   Mon Jun  7 10:20:49 2004
@@ -56,6 +56,9 @@
 static inline void flush_tlb_kernel_range(unsigned long start,
                                unsigned long end)
        { __tlbia(); }
+static inline void update_mmu_cache(struct vm_area_struct *vma,
+                               unsigned long addr, pte_t pte)
+       { /* nothing */ }
  
 #else  /* 6xx, 7xx, 7xxx cpus */
 struct mm_struct;
@@ -78,6 +81,7 @@
 {
 }
  
+#if !defined(CONFIG_8xx)
 /*
  * This gets called at the end of handling a page fault, when
  * the kernel has put a new PTE into the page table for the process.
@@ -88,6 +92,7 @@
  * waiting for the inevitable extra hash-table miss exception.
  */
 extern void update_mmu_cache(struct vm_area_struct *, unsigned long, pte_t);
+#endif
  
 #endif /* _PPC_TLBFLUSH_H */
 #endif /*__KERNEL__ */
===== arch/ppc/mm/init.c 1.36 vs edited =====
--- 1.36/arch/ppc/mm/init.c     Sat May 15 05:00:22 2004
+++ edited/arch/ppc/mm/init.c   Mon Jun  7 10:19:36 2004
@@ -605,6 +605,7 @@
        kunmap(page);
 }
  
+#if !defined(CONFIG_8xx)
 /*
  * This is called at the end of handling a user page fault, when the
  * fault has been handled by updating a PTE in the linux page tables.
@@ -642,3 +643,4 @@
        }
 #endif
 }
+#endif

Re: [PATCH] (urgent) ppc32: Fix CPUs with soft loaded TLB

[Benjamin Herrenschmidt]

> Hi
> 
> Unfortunately this is not enough for me on 8xx.
> 
> When starting init it bombs with
>
> .../...
>
> In order to fix this I now have to remove update_mmu_cache  by defining 
> it empty.

Looks bogus. Please, instead of random band-aids, try to analyse the
problem a bit deeper, that would help. SW TLB load works on 603 now,
so there must be something wrong in the low level 8xx related code.

do_no_page should have inserted/fixed a PTE, so the cache flush in
update_mmu_cache should work. If not, then something went wrong,
but then, please at least have a look at what PTE was inserted and
why it would be faulting that way, stop randomly playing with
update_mmu_cache.

Ben.

Re: [PATCH] (urgent) ppc32: Fix CPUs with soft loaded TLB

[Tom Rini]

On Mon, Jun 07, 2004 at 10:25:32AM +0300, Pantelis Antoniou wrote:

> Benjamin Herrenschmidt wrote:
> 
> >On Sun, 2004-06-06 at 16:20, Linus Torvalds wrote:
> >
> >>On Sun, 6 Jun 2004, Benjamin Herrenschmidt wrote:
> >>
> >>>The recent introduction of ptep_set_access_flags() with the optimisation
> >>>of not flushing the TLB unfortunately broke ppc32 CPUs with no hash 
> >>>table.
> >>>
> >>Makes sense, applied.
> >>
> >
> >ARGH. Missed one file. Here is an additional patch (missed tlbflush.h 
> >patch)
> >
> >Sorry.
> >
> >This adds the definiction of flush_tlb_page_nohash() that was missing
> >from the previous patch fixing SW-TLB loaded PPCs
> >
> >Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
[snip]
> Hi
> 
> Unfortunately this is not enough for me on 8xx.
[snip]
> In order to fix this I now have to remove update_mmu_cache  by defining 
> it empty.
> 
> Please see the following patch.

But this now matches the way things are on 2.4, so is it really a
problem?

-- 
Tom Rini
http://gate.crashing.org/~trini/