powerpc/64s/radix: misc minor TLB flush fixes
Nicholas Piggin
reviewerv2 cover powerpc/64s/radix: misc minor TLB flush fixes
*** BLURB HERE ***
Nicholas Piggin (3):
powerpc/64s/radix: fix preempt imbalance in TLB flush powerpc/64s/radix: tlbie improve preempt handling powerpc/64s/radix: Fix process table entry cache invalidation
arch/powerpc/include/asm/mmu_context.h | 4 +++ arch/powerpc/mm/mmu_context_book3s64.c | 25 ++++++++++++---- arch/powerpc/mm/tlb-radix.c | 53 +++++++++++++++++----------------- 3 files changed, 51 insertions(+), 31 deletions(-)
Signature · 1 line
2.13.3
Nicholas Piggin
reviewerPATCH v2 1/3 powerpc/64s/radix: fix preempt imbalance in TLB flush
--- arch/powerpc/mm/tlb-radix.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
arch/powerpc/mm/tlb-radix.c +2 -1
@@ -358,7 +358,7 @@ void radix__flush_tlb_collapsed_pmd(struct mm_struct *mm, unsigned long addr)unsigned long ap =mmu_get_ap (mmu_virtual_psize);unsigned long pid, end; - +preempt_disable (); pid = mm->context.id;if (unlikely (pid ==MMU_NO_CONTEXT )) goto no_context;
@@ -366,6 +366,7 @@ void radix__flush_tlb_collapsed_pmd(struct mm_struct *mm, unsigned long addr)/* 4k page size, just blow the world */ if (PAGE_SIZE ==0x1000 ) {radix__flush_all_mm (mm); +preempt_enable ();return ; }
Signature · 1 line
2.13.3
Re: [PATCH v2 1/3] powerpc/64s/radix: fix preempt imbalance in TLB flush
Nicholas Piggin <[email protected]> writes:
Quoted 28 lines
Signed-off-by Nicholas Piggin <[email protected]>--- arch/powerpc/mm/tlb-radix.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)arch/powerpc/mm/tlb-radix.c +2
-1@@ -358,7 +358,7 @@ void radix__flush_tlb_collapsed_pmd(struct mm_struct *mm, unsigned long addr)unsigned long ap =mmu_get_ap (mmu_virtual_psize);unsigned long pid, end; - +preempt_disable (); pid = mm->context.id;if (unlikely (pid ==MMU_NO_CONTEXT )) goto no_context;@@ -366,6 +366,7 @@ void radix__flush_tlb_collapsed_pmd(struct mm_struct *mm, unsigned long addr)/* 4k page size, just blow the world */ if (PAGE_SIZE ==0x1000 ) {radix__flush_all_mm (mm); +preempt_enable ();return ; }Signature · 1 line
2.13.3
Michael Ellerman
AppliedRe: [v2,1/3] powerpc/64s/radix: fix preempt imbalance in TLB flush
In reply to Nicholas PigginQuoted 2 lines · Nicholas Piggin
Signed-off-by Nicholas Piggin <[email protected]>Reviewed-by Aneesh Kumar K.V <[email protected]>
Applied to powerpc fixes, thanks.
https://git.kernel.org/powerpc/c/26e53d5ebe2e2a5ff7343e820f0ffd
cheers
Nicholas Piggin
reviewerPATCH v2 2/3 powerpc/64s/radix: tlbie improve preempt handling
In reply to Nicholas PigginPreempt should be consistently disabled for mm_is_thread_local tests, so bring the rest of these under preempt_disable().
Preempt does not need to be disabled for the mm->context.id tests, which allows simplification and removal of gotos.
--- arch/powerpc/mm/tlb-radix.c | 48 +++++++++++++++++++++------------------------ 1 file changed, 22 insertions(+), 26 deletions(-)
arch/powerpc/mm/tlb-radix.c +22 -26
@@ -186,16 +186,15 @@ void radix__flush_tlb_mm(struct mm_struct *mm) {unsigned long pid; -preempt_disable (); pid = mm->context.id;if (unlikely (pid ==MMU_NO_CONTEXT )) - goto no_context; +return ; +preempt_disable ();if (!mm_is_thread_local (mm))_tlbie_pid (pid,RIC_FLUSH_TLB );else _tlbiel_pid (pid,RIC_FLUSH_TLB ); -no_context:preempt_enable (); }EXPORT_SYMBOL (radix__flush_tlb_mm);
@@ -204,16 +203,15 @@ void radix__flush_all_mm(struct mm_struct *mm) {unsigned long pid; -preempt_disable (); pid = mm->context.id;if (unlikely (pid ==MMU_NO_CONTEXT )) - goto no_context; +return ; +preempt_disable ();if (!mm_is_thread_local (mm))_tlbie_pid (pid,RIC_FLUSH_ALL );else _tlbiel_pid (pid,RIC_FLUSH_ALL ); -no_context:preempt_enable (); }EXPORT_SYMBOL (radix__flush_all_mm);
@@ -230,15 +228,14 @@ void radix__flush_tlb_page_psize(struct mm_struct *mm, unsigned long vmaddr,unsigned long pid;unsigned long ap =mmu_get_ap (psize); -preempt_disable (); pid = mm->context.id;if (unlikely (pid ==MMU_NO_CONTEXT )) - goto bail; +return ; +preempt_disable ();if (!mm_is_thread_local (mm))_tlbie_va (vmaddr, pid, ap,RIC_FLUSH_TLB );else _tlbiel_va (vmaddr, pid, ap,RIC_FLUSH_TLB ); -bail:preempt_enable (); }
@@ -322,54 +319,53 @@ void radix__flush_tlb_range_psize(struct mm_struct *mm, unsigned long start, {unsigned long pid;unsigned long addr; -int local =mm_is_thread_local (mm); +bool local;unsigned long ap =mmu_get_ap (psize);unsigned long page_size =1UL << mmu_psize_defs[psize].shift; - -preempt_disable (); pid = mm->context.id;if (unlikely (pid ==MMU_NO_CONTEXT )) - goto err_out; +return ; +preempt_disable (); + local =mm_is_thread_local (mm);if (end ==TLB_FLUSH_ALL || (end - start) > tlb_single_page_flush_ceiling * page_size) {if (local)_tlbiel_pid (pid,RIC_FLUSH_TLB );else _tlbie_pid (pid,RIC_FLUSH_TLB ); - goto err_out; - } -for (addr = start; addr < end; addr += page_size) { + }else { +for (addr = start; addr < end; addr += page_size) { -if (local) -_tlbiel_va (addr, pid, ap,RIC_FLUSH_TLB ); -else -_tlbie_va (addr, pid, ap,RIC_FLUSH_TLB ); +if (local) +_tlbiel_va (addr, pid, ap,RIC_FLUSH_TLB ); +else +_tlbie_va (addr, pid, ap,RIC_FLUSH_TLB ); + } } -err_out:preempt_enable (); }#ifdef CONFIG_TRANSPARENT_HUGEPAGE void radix__flush_tlb_collapsed_pmd (struct mm_struct *mm,unsigned long addr) { -int local =mm_is_thread_local (mm);unsigned long ap =mmu_get_ap (mmu_virtual_psize);unsigned long pid, end; +bool local; -preempt_disable (); pid = mm->context.id;if (unlikely (pid ==MMU_NO_CONTEXT )) - goto no_context; +return ;/* 4k page size, just blow the world */ if (PAGE_SIZE ==0x1000 ) {radix__flush_all_mm (mm); -preempt_enable ();return ; } +preempt_disable (); + local =mm_is_thread_local (mm);/* Otherwise first do the PWC */ if (local)_tlbiel_pid (pid,RIC_FLUSH_PWC );
@@ -384,7 +380,7 @@ void radix__flush_tlb_collapsed_pmd(struct mm_struct *mm, unsigned long addr) else _tlbie_va(addr, pid, ap, RIC_FLUSH_TLB); } -no_context: +preempt_enable (); }#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
Signature · 1 line
2.13.3
Re: [PATCH v2 2/3] powerpc/64s/radix: tlbie improve preempt handling
Quoted 6 lines · Nicholas Piggin
Preempt should be consistently disabled for
mm_is_thread_localtests, so bring the rest of these underpreempt_disable().Preempt does not need to be disabled for the
mm->context.idtests, which allows simplification and removal of gotos.
Quoted 146 lines
Signed-off-by Nicholas Piggin <[email protected]>--- arch/powerpc/mm/tlb-radix.c | 48 +++++++++++++++++++++------------------------ 1 file changed, 22 insertions(+), 26 deletions(-)arch/powerpc/mm/tlb-radix.c +22
-26@@ -186,16 +186,15 @@ void radix__flush_tlb_mm(struct mm_struct *mm) {unsigned long pid; -preempt_disable (); pid = mm->context.id;if (unlikely (pid ==MMU_NO_CONTEXT )) - goto no_context; +return ; +preempt_disable ();if (!mm_is_thread_local (mm))_tlbie_pid (pid,RIC_FLUSH_TLB );else _tlbiel_pid (pid,RIC_FLUSH_TLB ); -no_context:preempt_enable (); }EXPORT_SYMBOL (radix__flush_tlb_mm);@@ -204,16 +203,15 @@ void radix__flush_all_mm(struct mm_struct *mm) {unsigned long pid; -preempt_disable (); pid = mm->context.id;if (unlikely (pid ==MMU_NO_CONTEXT )) - goto no_context; +return ; +preempt_disable ();if (!mm_is_thread_local (mm))_tlbie_pid (pid,RIC_FLUSH_ALL );else _tlbiel_pid (pid,RIC_FLUSH_ALL ); -no_context:preempt_enable (); }EXPORT_SYMBOL (radix__flush_all_mm);@@ -230,15 +228,14 @@ void radix__flush_tlb_page_psize(struct mm_struct *mm, unsigned long vmaddr,unsigned long pid;unsigned long ap =mmu_get_ap (psize); -preempt_disable (); pid = mm->context.id;if (unlikely (pid ==MMU_NO_CONTEXT )) - goto bail; +return ; +preempt_disable ();if (!mm_is_thread_local (mm))_tlbie_va (vmaddr, pid, ap,RIC_FLUSH_TLB );else _tlbiel_va (vmaddr, pid, ap,RIC_FLUSH_TLB ); -bail:preempt_enable (); }@@ -322,54 +319,53 @@ void radix__flush_tlb_range_psize(struct mm_struct *mm, unsigned long start, {unsigned long pid;unsigned long addr; -int local =mm_is_thread_local (mm); +bool local;unsigned long ap =mmu_get_ap (psize);unsigned long page_size =1UL << mmu_psize_defs[psize].shift; - -preempt_disable (); pid = mm->context.id;if (unlikely (pid ==MMU_NO_CONTEXT )) - goto err_out; +return ; +preempt_disable (); + local =mm_is_thread_local (mm);if (end ==TLB_FLUSH_ALL || (end - start) > tlb_single_page_flush_ceiling * page_size) {if (local)_tlbiel_pid (pid,RIC_FLUSH_TLB );else _tlbie_pid (pid,RIC_FLUSH_TLB ); - goto err_out; - } -for (addr = start; addr < end; addr += page_size) { + }else { +for (addr = start; addr < end; addr += page_size) { -if (local) -_tlbiel_va (addr, pid, ap,RIC_FLUSH_TLB ); -else -_tlbie_va (addr, pid, ap,RIC_FLUSH_TLB ); +if (local) +_tlbiel_va (addr, pid, ap,RIC_FLUSH_TLB ); +else +_tlbie_va (addr, pid, ap,RIC_FLUSH_TLB ); + } } -err_out:preempt_enable (); }#ifdef CONFIG_TRANSPARENT_HUGEPAGE void radix__flush_tlb_collapsed_pmd (struct mm_struct *mm,unsigned long addr) { -int local =mm_is_thread_local (mm);unsigned long ap =mmu_get_ap (mmu_virtual_psize);unsigned long pid, end; +bool local; -preempt_disable (); pid = mm->context.id;if (unlikely (pid ==MMU_NO_CONTEXT )) - goto no_context; +return ;/* 4k page size, just blow the world */ if (PAGE_SIZE ==0x1000 ) {radix__flush_all_mm (mm); -preempt_enable ();return ; } +preempt_disable (); + local =mm_is_thread_local (mm);/* Otherwise first do the PWC */ if (local)_tlbiel_pid (pid,RIC_FLUSH_PWC );@@ -384,7 +380,7 @@ void radix__flush_tlb_collapsed_pmd(struct mm_struct *mm, unsigned long addr) else _tlbie_va(addr, pid, ap, RIC_FLUSH_TLB); } -no_context: +preempt_enable (); }#endif /* CONFIG_TRANSPARENT_HUGEPAGE */ Signature · 1 line
2.13.3
Michael Ellerman
AppliedRe: [v2,2/3] powerpc/64s/radix: tlbie improve preempt handling
In reply to Nicholas PigginQuoted 7 lines · Nicholas Piggin
Preempt should be consistently disabled for
mm_is_thread_localtests, so bring the rest of these underpreempt_disable().Preempt does not need to be disabled for the
mm->context.idtests, which allows simplification and removal of gotos.Signed-off-by Nicholas Piggin <[email protected]>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/dffe8449c5dd63ff18b47709de7555
cheers
Nicholas Piggin
reviewerPATCH v2 3/3 powerpc/64s/radix: Fix process table entry cache invalidation
In reply to Nicholas PigginAccording to the architecture, the process table entry cache must be flushed with tlbie RIC=2.
Currently the process table entry is set to invalid right before the PID is returned to the allocator, with no invalidation. This works on existing implementations that are known to not cache the process table entry for any except the current PIDR.
It is architecturally correct and cleaner to invalidate with RIC=2 after clearing the process table entry and before the PID is returned to the allocator. This can be done in arch_exit_mmap that runs before the final flush, and to ensure the final flush (fullmm) is always a RIC=2 variant.
--- arch/powerpc/include/asm/mmu_context.h | 4 ++++ arch/powerpc/mm/mmu_context_book3s64.c | 25 ++++++++++++++++++++----- arch/powerpc/mm/tlb-radix.c | 6 +++++- 3 files changed, 29 insertions(+), 6 deletions(-)
arch/powerpc/include/asm/mmu_context.h +4 -0
@@ -164,9 +164,13 @@ static inline void arch_dup_mmap(struct mm_struct *oldmm, { } +#ifndef CONFIG_PPC_BOOK3S_64 static inline void arch_exit_mmap (struct mm_struct *mm) { } +#else +extern void arch_exit_mmap (struct mm_struct *mm); +#endif static inline void arch_unmap(struct mm_struct *mm, struct vm_area_struct *vma,
arch/powerpc/mm/mmu_context_book3s64.c +20 -5
@@ -216,19 +216,34 @@ void destroy_context(struct mm_struct *mm)#ifdef CONFIG_SPAPR_TCE_IOMMU WARN_ON_ONCE (!list_empty (&mm->context.iommu_group_mem_list));#endif +if (radix_enabled ()) +WARN_ON (process_tb[mm->context.id].prtb0 !=0 ); +else +subpage_prot_free (mm); +destroy_pagetable_page (mm); +__destroy_context (mm->context.id); + mm->context.id =MMU_NO_CONTEXT ; +} + +void arch_exit_mmap (struct mm_struct *mm) +{if (radix_enabled ()) {/* * Radix doesn't have a valid bit in the process table * entries. However we know that at least P9 implementation * will avoid caching an entry with an invalid RTS field, * and 0 is invalid. So this will do. +* +* This runs before the "fullmm" tlb flush in exit_mmap, +* which does a RIC=2 tlbie to clear the process table +* entry. See the "fullmm" comments in tlb-radix.c. +* +* No barrier required here after the store because +* this process will do the invalidate, which starts with +* ptesync. */ process_tb[mm->context.id].prtb0 =0 ; - }else -subpage_prot_free (mm); -destroy_pagetable_page (mm); -__destroy_context (mm->context.id); - mm->context.id =MMU_NO_CONTEXT ; + } }#ifdef CONFIG_PPC_RADIX_MMU
arch/powerpc/mm/tlb-radix.c +5 -1
@@ -297,10 +297,14 @@ void radix__tlb_flush(struct mmu_gather *tlb) psize =radix_get_mmu_psize (page_size);/* * if page size is not something we understand, do a full mm flush +* +* A "fullmm" flush must always do a flush_all_mm (RIC=2) flush +* that flushes the process table entry cache upon process teardown. +* See the comment for radix in arch_exit_mmap(). */ if (psize !=-1 && !tlb->fullmm && !tlb->need_flush_all)radix__flush_tlb_range_psize (mm, tlb->start, tlb->end, psize); -else if (tlb->need_flush_all) { +else if (tlb->fullmm || tlb->need_flush_all) { tlb->need_flush_all =0 ;radix__flush_all_mm (mm); }else
Signature · 1 line
2.13.3
Re: [PATCH v2 3/3] powerpc/64s/radix: Fix process table entry cache invalidation
Quoted 8 lines · Nicholas Piggin
According to the architecture, the process table entry cache must be flushed with tlbie RIC=2.
Currently the process table entry is set to invalid right before the PID is returned to the allocator, with no invalidation. This works on existing implementations that are known to not cache the process table entry for any except the current PIDR.
Quoted 6 lines
It is architecturally correct and cleaner to invalidate with RIC=2 after clearing the process table entry and before the PID is returned to the allocator. This can be done in
arch_exit_mmapthat runs before the final flush, and to ensure the final flush (fullmm) is always a RIC=2 variant.
Quoted 91 lines
Signed-off-by Nicholas Piggin <[email protected]>--- arch/powerpc/include/asm/mmu_context.h | 4 ++++ arch/powerpc/mm/mmu_context_book3s64.c | 25 ++++++++++++++++++++----- arch/powerpc/mm/tlb-radix.c | 6 +++++- 3 files changed, 29 insertions(+), 6 deletions(-)arch/powerpc/include/asm/mmu_context.h +4
-0@@ -164,9 +164,13 @@ static inline void arch_dup_mmap(struct mm_struct *oldmm, { } +#ifndef CONFIG_PPC_BOOK3S_64 static inline void arch_exit_mmap (struct mm_struct *mm) { } +#else +extern void arch_exit_mmap (struct mm_struct *mm); +#endif static inline void arch_unmap(struct mm_struct *mm, struct vm_area_struct *vma,arch/powerpc/mm/mmu_context_book3s64.c +20
-5@@ -216,19 +216,34 @@ void destroy_context(struct mm_struct *mm)#ifdef CONFIG_SPAPR_TCE_IOMMU WARN_ON_ONCE (!list_empty (&mm->context.iommu_group_mem_list));#endif +if (radix_enabled ()) +WARN_ON (process_tb[mm->context.id].prtb0 !=0 ); +else +subpage_prot_free (mm); +destroy_pagetable_page (mm); +__destroy_context (mm->context.id); + mm->context.id =MMU_NO_CONTEXT ; +} + +void arch_exit_mmap (struct mm_struct *mm) +{if (radix_enabled ()) {/* * Radix doesn't have a valid bit in the process table * entries. However we know that at least P9 implementation * will avoid caching an entry with an invalid RTS field, * and 0 is invalid. So this will do. +* +* This runs before the "fullmm" tlb flush in exit_mmap, +* which does a RIC=2 tlbie to clear the process table +* entry. See the "fullmm" comments in tlb-radix.c. +* +* No barrier required here after the store because +* this process will do the invalidate, which starts with +* ptesync. */ process_tb[mm->context.id].prtb0 =0 ; - }else -subpage_prot_free (mm); -destroy_pagetable_page (mm); -__destroy_context (mm->context.id); - mm->context.id =MMU_NO_CONTEXT ; + } }#ifdef CONFIG_PPC_RADIX_MMU arch/powerpc/mm/tlb-radix.c +5
-1@@ -297,10 +297,14 @@ void radix__tlb_flush(struct mmu_gather *tlb) psize =radix_get_mmu_psize (page_size);/* * if page size is not something we understand, do a full mm flush +* +* A "fullmm" flush must always do a flush_all_mm (RIC=2) flush +* that flushes the process table entry cache upon process teardown. +* See the comment for radix in arch_exit_mmap(). */ if (psize !=-1 && !tlb->fullmm && !tlb->need_flush_all)radix__flush_tlb_range_psize (mm, tlb->start, tlb->end, psize); -else if (tlb->need_flush_all) { +else if (tlb->fullmm || tlb->need_flush_all) { tlb->need_flush_all =0 ;radix__flush_all_mm (mm); }else Signature · 1 line
2.13.3
Michael Ellerman
AppliedRe: [v2, 3/3] powerpc/64s/radix: Fix process table entry cache invalidation
In reply to Nicholas PigginQuoted 8 lines · Nicholas Piggin
According to the architecture, the process table entry cache must be flushed with tlbie RIC=2.
Currently the process table entry is set to invalid right before the PID is returned to the allocator, with no invalidation. This works on existing implementations that are known to not cache the process table entry for any except the current PIDR.
Quoted 7 lines
It is architecturally correct and cleaner to invalidate with RIC=2 after clearing the process table entry and before the PID is returned to the allocator. This can be done in
arch_exit_mmapthat runs before the final flush, and to ensure the final flush (fullmm) is always a RIC=2 variant.Signed-off-by Nicholas Piggin <[email protected]>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/30b49ec798f0984b905fd94d1957d6
cheers