All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] powerpc/64s/radix: misc minor TLB flush fixes
@ 2017-10-24 13:06 Nicholas Piggin
  2017-10-24 13:06 ` [PATCH v2 1/3] powerpc/64s/radix: fix preempt imbalance in TLB flush Nicholas Piggin
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Nicholas Piggin @ 2017-10-24 13:06 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin, Aneesh Kumar K.V, Benjamin Herrenschmidt

*** 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(-)

-- 
2.13.3

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/3] powerpc/64s/radix: fix preempt imbalance in TLB flush
  2017-10-24 13:06 [PATCH v2 0/3] powerpc/64s/radix: misc minor TLB flush fixes Nicholas Piggin
@ 2017-10-24 13:06 ` Nicholas Piggin
  2017-10-26  6:52   ` Aneesh Kumar K.V
  2017-11-01  5:17   ` [v2,1/3] " Michael Ellerman
  2017-10-24 13:06 ` [PATCH v2 2/3] powerpc/64s/radix: tlbie improve preempt handling Nicholas Piggin
  2017-10-24 13:06 ` [PATCH v2 3/3] powerpc/64s/radix: Fix process table entry cache invalidation Nicholas Piggin
  2 siblings, 2 replies; 10+ messages in thread
From: Nicholas Piggin @ 2017-10-24 13:06 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin, Aneesh Kumar K.V, Benjamin Herrenschmidt

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/mm/tlb-radix.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/mm/tlb-radix.c b/arch/powerpc/mm/tlb-radix.c
index 3a07d7a5e2fe..892544ebc2d2 100644
--- a/arch/powerpc/mm/tlb-radix.c
+++ b/arch/powerpc/mm/tlb-radix.c
@@ -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;
 	}
 
-- 
2.13.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/3] powerpc/64s/radix: tlbie improve preempt handling
  2017-10-24 13:06 [PATCH v2 0/3] powerpc/64s/radix: misc minor TLB flush fixes Nicholas Piggin
  2017-10-24 13:06 ` [PATCH v2 1/3] powerpc/64s/radix: fix preempt imbalance in TLB flush Nicholas Piggin
@ 2017-10-24 13:06 ` Nicholas Piggin
  2017-10-26  6:53   ` Aneesh Kumar K.V
  2017-11-07 23:30   ` [v2,2/3] " Michael Ellerman
  2017-10-24 13:06 ` [PATCH v2 3/3] powerpc/64s/radix: Fix process table entry cache invalidation Nicholas Piggin
  2 siblings, 2 replies; 10+ messages in thread
From: Nicholas Piggin @ 2017-10-24 13:06 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin, Aneesh Kumar K.V, Benjamin Herrenschmidt

Preempt 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.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/mm/tlb-radix.c | 48 +++++++++++++++++++++------------------------
 1 file changed, 22 insertions(+), 26 deletions(-)

diff --git a/arch/powerpc/mm/tlb-radix.c b/arch/powerpc/mm/tlb-radix.c
index 892544ebc2d2..18170dc264aa 100644
--- a/arch/powerpc/mm/tlb-radix.c
+++ b/arch/powerpc/mm/tlb-radix.c
@@ -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 */
-- 
2.13.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 3/3] powerpc/64s/radix: Fix process table entry cache invalidation
  2017-10-24 13:06 [PATCH v2 0/3] powerpc/64s/radix: misc minor TLB flush fixes Nicholas Piggin
  2017-10-24 13:06 ` [PATCH v2 1/3] powerpc/64s/radix: fix preempt imbalance in TLB flush Nicholas Piggin
  2017-10-24 13:06 ` [PATCH v2 2/3] powerpc/64s/radix: tlbie improve preempt handling Nicholas Piggin
@ 2017-10-24 13:06 ` Nicholas Piggin
  2017-10-26  6:53   ` Aneesh Kumar K.V
  2017-11-07 23:30   ` [v2, " Michael Ellerman
  2 siblings, 2 replies; 10+ messages in thread
From: Nicholas Piggin @ 2017-10-24 13:06 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin, Aneesh Kumar K.V, Benjamin Herrenschmidt

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.

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.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 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(-)

diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
index a0d7145d6cd2..20eae6f76247 100644
--- a/arch/powerpc/include/asm/mmu_context.h
+++ b/arch/powerpc/include/asm/mmu_context.h
@@ -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,
diff --git a/arch/powerpc/mm/mmu_context_book3s64.c b/arch/powerpc/mm/mmu_context_book3s64.c
index 05e15386d4cb..6d724dab27c2 100644
--- a/arch/powerpc/mm/mmu_context_book3s64.c
+++ b/arch/powerpc/mm/mmu_context_book3s64.c
@@ -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
diff --git a/arch/powerpc/mm/tlb-radix.c b/arch/powerpc/mm/tlb-radix.c
index 18170dc264aa..0c8464653aa3 100644
--- a/arch/powerpc/mm/tlb-radix.c
+++ b/arch/powerpc/mm/tlb-radix.c
@@ -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
-- 
2.13.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] powerpc/64s/radix: fix preempt imbalance in TLB flush
  2017-10-24 13:06 ` [PATCH v2 1/3] powerpc/64s/radix: fix preempt imbalance in TLB flush Nicholas Piggin
@ 2017-10-26  6:52   ` Aneesh Kumar K.V
  2017-11-01  5:17   ` [v2,1/3] " Michael Ellerman
  1 sibling, 0 replies; 10+ messages in thread
From: Aneesh Kumar K.V @ 2017-10-26  6:52 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: Nicholas Piggin, Benjamin Herrenschmidt

Nicholas Piggin <npiggin@gmail.com> writes:


Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  arch/powerpc/mm/tlb-radix.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/mm/tlb-radix.c b/arch/powerpc/mm/tlb-radix.c
> index 3a07d7a5e2fe..892544ebc2d2 100644
> --- a/arch/powerpc/mm/tlb-radix.c
> +++ b/arch/powerpc/mm/tlb-radix.c
> @@ -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;
>  	}
>
> -- 
> 2.13.3

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/3] powerpc/64s/radix: tlbie improve preempt handling
  2017-10-24 13:06 ` [PATCH v2 2/3] powerpc/64s/radix: tlbie improve preempt handling Nicholas Piggin
@ 2017-10-26  6:53   ` Aneesh Kumar K.V
  2017-11-07 23:30   ` [v2,2/3] " Michael Ellerman
  1 sibling, 0 replies; 10+ messages in thread
From: Aneesh Kumar K.V @ 2017-10-26  6:53 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: Nicholas Piggin, Benjamin Herrenschmidt

Nicholas Piggin <npiggin@gmail.com> writes:

> Preempt 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.
>

Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  arch/powerpc/mm/tlb-radix.c | 48 +++++++++++++++++++++------------------------
>  1 file changed, 22 insertions(+), 26 deletions(-)
>
> diff --git a/arch/powerpc/mm/tlb-radix.c b/arch/powerpc/mm/tlb-radix.c
> index 892544ebc2d2..18170dc264aa 100644
> --- a/arch/powerpc/mm/tlb-radix.c
> +++ b/arch/powerpc/mm/tlb-radix.c
> @@ -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 */
> -- 
> 2.13.3

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 3/3] powerpc/64s/radix: Fix process table entry cache invalidation
  2017-10-24 13:06 ` [PATCH v2 3/3] powerpc/64s/radix: Fix process table entry cache invalidation Nicholas Piggin
@ 2017-10-26  6:53   ` Aneesh Kumar K.V
  2017-11-07 23:30   ` [v2, " Michael Ellerman
  1 sibling, 0 replies; 10+ messages in thread
From: Aneesh Kumar K.V @ 2017-10-26  6:53 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: Nicholas Piggin, Benjamin Herrenschmidt

Nicholas Piggin <npiggin@gmail.com> writes:

> 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.
>
> 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.
>

Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  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(-)
>
> diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
> index a0d7145d6cd2..20eae6f76247 100644
> --- a/arch/powerpc/include/asm/mmu_context.h
> +++ b/arch/powerpc/include/asm/mmu_context.h
> @@ -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,
> diff --git a/arch/powerpc/mm/mmu_context_book3s64.c b/arch/powerpc/mm/mmu_context_book3s64.c
> index 05e15386d4cb..6d724dab27c2 100644
> --- a/arch/powerpc/mm/mmu_context_book3s64.c
> +++ b/arch/powerpc/mm/mmu_context_book3s64.c
> @@ -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
> diff --git a/arch/powerpc/mm/tlb-radix.c b/arch/powerpc/mm/tlb-radix.c
> index 18170dc264aa..0c8464653aa3 100644
> --- a/arch/powerpc/mm/tlb-radix.c
> +++ b/arch/powerpc/mm/tlb-radix.c
> @@ -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
> -- 
> 2.13.3

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [v2,1/3] powerpc/64s/radix: fix preempt imbalance in TLB flush
  2017-10-24 13:06 ` [PATCH v2 1/3] powerpc/64s/radix: fix preempt imbalance in TLB flush Nicholas Piggin
  2017-10-26  6:52   ` Aneesh Kumar K.V
@ 2017-11-01  5:17   ` Michael Ellerman
  1 sibling, 0 replies; 10+ messages in thread
From: Michael Ellerman @ 2017-11-01  5:17 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: Aneesh Kumar K.V, Nicholas Piggin

On Tue, 2017-10-24 at 13:06:52 UTC, Nicholas Piggin wrote:
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

Applied to powerpc fixes, thanks.

https://git.kernel.org/powerpc/c/26e53d5ebe2e2a5ff7343e820f0ffd

cheers

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [v2,2/3] powerpc/64s/radix: tlbie improve preempt handling
  2017-10-24 13:06 ` [PATCH v2 2/3] powerpc/64s/radix: tlbie improve preempt handling Nicholas Piggin
  2017-10-26  6:53   ` Aneesh Kumar K.V
@ 2017-11-07 23:30   ` Michael Ellerman
  1 sibling, 0 replies; 10+ messages in thread
From: Michael Ellerman @ 2017-11-07 23:30 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: Aneesh Kumar K.V, Nicholas Piggin

On Tue, 2017-10-24 at 13:06:53 UTC, Nicholas Piggin wrote:
> Preempt 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.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/dffe8449c5dd63ff18b47709de7555

cheers

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [v2, 3/3] powerpc/64s/radix: Fix process table entry cache invalidation
  2017-10-24 13:06 ` [PATCH v2 3/3] powerpc/64s/radix: Fix process table entry cache invalidation Nicholas Piggin
  2017-10-26  6:53   ` Aneesh Kumar K.V
@ 2017-11-07 23:30   ` Michael Ellerman
  1 sibling, 0 replies; 10+ messages in thread
From: Michael Ellerman @ 2017-11-07 23:30 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: Aneesh Kumar K.V, Nicholas Piggin

On Tue, 2017-10-24 at 13:06:54 UTC, Nicholas Piggin wrote:
> 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.
> 
> 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.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/30b49ec798f0984b905fd94d1957d6

cheers

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-11-07 23:30 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-24 13:06 [PATCH v2 0/3] powerpc/64s/radix: misc minor TLB flush fixes Nicholas Piggin
2017-10-24 13:06 ` [PATCH v2 1/3] powerpc/64s/radix: fix preempt imbalance in TLB flush Nicholas Piggin
2017-10-26  6:52   ` Aneesh Kumar K.V
2017-11-01  5:17   ` [v2,1/3] " Michael Ellerman
2017-10-24 13:06 ` [PATCH v2 2/3] powerpc/64s/radix: tlbie improve preempt handling Nicholas Piggin
2017-10-26  6:53   ` Aneesh Kumar K.V
2017-11-07 23:30   ` [v2,2/3] " Michael Ellerman
2017-10-24 13:06 ` [PATCH v2 3/3] powerpc/64s/radix: Fix process table entry cache invalidation Nicholas Piggin
2017-10-26  6:53   ` Aneesh Kumar K.V
2017-11-07 23:30   ` [v2, " Michael Ellerman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.