[PATCH 0/8] x86/mm/cpa: Further optimizations

[PATCH 0/8] x86/mm/cpa: Further optimizations

[Peter Zijlstra]

Thomas' CPA patches made me look at the tlb invalidate crud around there. These
patches are the result of cleaning some of that up a bit. They are based on top
of Thomas' patches.

Dave, can you have a wee look at the Atom errata thing; this does grow the
amount of code between the page-table update and the TLB flush a bit, but all
that PTI crud we did a while back already made it a non-trivial amount of code.

---
 pageattr.c |  128 ++++++++++++++++++++++---------------------------------------
 1 file changed, 48 insertions(+), 80 deletions(-)

[PATCH 1/8] x86/mm/cpa: Use flush_tlb_all()

[Peter Zijlstra]

Instead of open-coding it..

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/mm/pageattr.c |   12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -285,16 +285,6 @@ static void cpa_flush_all(unsigned long
 	on_each_cpu(__cpa_flush_all, (void *) cache, 1);
 }
 
-static void __cpa_flush_range(void *arg)
-{
-	/*
-	 * We could optimize that further and do individual per page
-	 * tlb invalidates for a low number of pages. Caveat: we must
-	 * flush the high aliases on 64bit as well.
-	 */
-	__flush_tlb_all();
-}
-
 static void cpa_flush_range(unsigned long start, int numpages, int cache)
 {
 	unsigned int i, level;
@@ -303,7 +293,7 @@ static void cpa_flush_range(unsigned lon
 	BUG_ON(irqs_disabled() && !early_boot_irqs_disabled);
 	WARN_ON(PAGE_ALIGN(start) != start);
 
-	on_each_cpu(__cpa_flush_range, NULL, 1);
+	flush_tlb_all();
 
 	if (!cache)
 		return;

Re: [PATCH 1/8] x86/mm/cpa: Use flush_tlb_all()

[Peter Zijlstra]

On Wed, Sep 19, 2018 at 10:50:17AM +0200, Peter Zijlstra wrote:
> Instead of open-coding it..
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  arch/x86/mm/pageattr.c |   12 +-----------
>  1 file changed, 1 insertion(+), 11 deletions(-)
> 
> --- a/arch/x86/mm/pageattr.c
> +++ b/arch/x86/mm/pageattr.c
> @@ -285,16 +285,6 @@ static void cpa_flush_all(unsigned long
>  	on_each_cpu(__cpa_flush_all, (void *) cache, 1);
>  }
>  
> -static void __cpa_flush_range(void *arg)
> -{
> -	/*
> -	 * We could optimize that further and do individual per page
> -	 * tlb invalidates for a low number of pages. Caveat: we must
> -	 * flush the high aliases on 64bit as well.
> -	 */
> -	__flush_tlb_all();
> -}

Hmm,.. so in patch #4 I do switch to flush_tlb_kernel_range(). What are
those high aliases that comment talks about?

Re: [PATCH 1/8] x86/mm/cpa: Use flush_tlb_all()

[Thomas Gleixner]

On Wed, 19 Sep 2018, Peter Zijlstra wrote:
> On Wed, Sep 19, 2018 at 10:50:17AM +0200, Peter Zijlstra wrote:
> > Instead of open-coding it..
> > 
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > ---
> >  arch/x86/mm/pageattr.c |   12 +-----------
> >  1 file changed, 1 insertion(+), 11 deletions(-)
> > 
> > --- a/arch/x86/mm/pageattr.c
> > +++ b/arch/x86/mm/pageattr.c
> > @@ -285,16 +285,6 @@ static void cpa_flush_all(unsigned long
> >  	on_each_cpu(__cpa_flush_all, (void *) cache, 1);
> >  }
> >  
> > -static void __cpa_flush_range(void *arg)
> > -{
> > -	/*
> > -	 * We could optimize that further and do individual per page
> > -	 * tlb invalidates for a low number of pages. Caveat: we must
> > -	 * flush the high aliases on 64bit as well.
> > -	 */
> > -	__flush_tlb_all();
> > -}
> 
> Hmm,.. so in patch #4 I do switch to flush_tlb_kernel_range(). What are
> those high aliases that comment talks about?

We have two mappings for the kernel. The 'real one' and the direct mapping
alias and for most of the operations, we have to make sure that the table
entries are identical in both maps.

The comments in that code probably need some care.

Thanks,

	tglx

[tip:x86/mm] x86/mm/cpa: Use flush_tlb_all()

[tip-bot for Peter Zijlstra]

Commit-ID:  c6185b1f21a47af94617fde3af7e803817b522a9
Gitweb:     https://git.kernel.org/tip/c6185b1f21a47af94617fde3af7e803817b522a9
Author:     Peter Zijlstra <peterz@infradead.org>
AuthorDate: Wed, 19 Sep 2018 10:50:17 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 27 Sep 2018 20:39:40 +0200

x86/mm/cpa: Use flush_tlb_all()

Instead of open-coding it..

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Dave Hansen <dave.hansen@intel.com>
Cc: Bin Yang <bin.yang@intel.com>
Cc: Mark Gross <mark.gross@intel.com>
Link: https://lkml.kernel.org/r/20180919085947.831102058@infradead.org

---
 arch/x86/mm/pageattr.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index 4e55ded01be5..a22f6b71a308 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -285,16 +285,6 @@ static void cpa_flush_all(unsigned long cache)
 	on_each_cpu(__cpa_flush_all, (void *) cache, 1);
 }
 
-static void __cpa_flush_range(void *arg)
-{
-	/*
-	 * We could optimize that further and do individual per page
-	 * tlb invalidates for a low number of pages. Caveat: we must
-	 * flush the high aliases on 64bit as well.
-	 */
-	__flush_tlb_all();
-}
-
 static void cpa_flush_range(unsigned long start, int numpages, int cache)
 {
 	unsigned int i, level;
@@ -303,7 +293,7 @@ static void cpa_flush_range(unsigned long start, int numpages, int cache)
 	BUG_ON(irqs_disabled() && !early_boot_irqs_disabled);
 	WARN_ON(PAGE_ALIGN(start) != start);
 
-	on_each_cpu(__cpa_flush_range, NULL, 1);
+	flush_tlb_all();
 
 	if (!cache)
 		return;

[PATCH 2/8] x86/mm/cpa: Move flush_tlb_all()

[Peter Zijlstra]

There is an atom errata, where we do a local TLB invalidate right
before we return and then do a global TLB invalidate.

Move the global invalidate up a little bit and avoid the local
invalidate entirely.

This does put the global invalidate under pgd_lock, but that shouldn't
matter.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/mm/pageattr.c |   44 +++++++++++++++++---------------------------
 1 file changed, 17 insertions(+), 27 deletions(-)

--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -996,14 +996,24 @@ __split_large_page(struct cpa_data *cpa,
 	__set_pmd_pte(kpte, address, mk_pte(base, __pgprot(_KERNPG_TABLE)));
 
 	/*
-	 * Intel Atom errata AAH41 workaround.
+	 * Do a global flush tlb after splitting the large page
+	 * and before we do the actual change page attribute in the PTE.
 	 *
-	 * The real fix should be in hw or in a microcode update, but
-	 * we also probabilistically try to reduce the window of having
-	 * a large TLB mixed with 4K TLBs while instruction fetches are
-	 * going on.
+	 * Without this, we violate the TLB application note, that says:
+	 * "The TLBs may contain both ordinary and large-page
+	 *  translations for a 4-KByte range of linear addresses. This
+	 *  may occur if software modifies the paging structures so that
+	 *  the page size used for the address range changes. If the two
+	 *  translations differ with respect to page frame or attributes
+	 *  (e.g., permissions), processor behavior is undefined and may
+	 *  be implementation-specific."
+	 *
+	 * We do this global tlb flush inside the cpa_lock, so that we
+	 * don't allow any other cpu, with stale tlb entries change the
+	 * page attribute in parallel, that also falls into the
+	 * just split large page entry.
 	 */
-	__flush_tlb_all();
+	flush_tlb_all();
 	spin_unlock(&pgd_lock);
 
 	return 0;
@@ -1528,28 +1538,8 @@ static int __change_page_attr(struct cpa
 	 * We have to split the large page:
 	 */
 	err = split_large_page(cpa, kpte, address);
-	if (!err) {
-		/*
-		 * Do a global flush tlb after splitting the large page
-		 * and before we do the actual change page attribute in the PTE.
-		 *
-		 * With out this, we violate the TLB application note, that says
-		 * "The TLBs may contain both ordinary and large-page
-		 *  translations for a 4-KByte range of linear addresses. This
-		 *  may occur if software modifies the paging structures so that
-		 *  the page size used for the address range changes. If the two
-		 *  translations differ with respect to page frame or attributes
-		 *  (e.g., permissions), processor behavior is undefined and may
-		 *  be implementation-specific."
-		 *
-		 * We do this global tlb flush inside the cpa_lock, so that we
-		 * don't allow any other cpu, with stale tlb entries change the
-		 * page attribute in parallel, that also falls into the
-		 * just split large page entry.
-		 */
-		flush_tlb_all();
+	if (!err)
 		goto repeat;
-	}
 
 	return err;
 }

Re: [PATCH 2/8] x86/mm/cpa: Move flush_tlb_all()

[Dave Hansen]

On 09/19/2018 01:50 AM, Peter Zijlstra wrote:
> @@ -1528,28 +1538,8 @@ static int __change_page_attr(struct cpa
>  	 * We have to split the large page:
>  	 */
>  	err = split_large_page(cpa, kpte, address);
> -	if (!err) {
...
> -		flush_tlb_all();
> +	if (!err)
>  		goto repeat;
> -	}

I _think_ this is the only use of split_large_page()'s error code.  We
can just make it return void now, right?

BTW, I also really like doing the "final" TLB flush as close as possible
to the actual PTE manipulation.  Makes it much less likely that we break it.

Re: [PATCH 2/8] x86/mm/cpa: Move flush_tlb_all()

[Peter Zijlstra]

On Fri, Sep 21, 2018 at 02:34:32PM -0700, Dave Hansen wrote:
> On 09/19/2018 01:50 AM, Peter Zijlstra wrote:
> > @@ -1528,28 +1538,8 @@ static int __change_page_attr(struct cpa
> >  	 * We have to split the large page:
> >  	 */
> >  	err = split_large_page(cpa, kpte, address);
> > -	if (!err) {
> ...
> > -		flush_tlb_all();
> > +	if (!err)
> >  		goto repeat;
> > -	}
> 
> I _think_ this is the only use of split_large_page()'s error code.  We
> can just make it return void now, right?

Then how does it decide to goto repeat or error out?

> BTW, I also really like doing the "final" TLB flush as close as possible
> to the actual PTE manipulation.  Makes it much less likely that we break it.

Yes.

Re: [PATCH 2/8] x86/mm/cpa: Move flush_tlb_all()

[Dave Hansen]

On 09/22/2018 03:37 AM, Peter Zijlstra wrote:
> On Fri, Sep 21, 2018 at 02:34:32PM -0700, Dave Hansen wrote:
>> On 09/19/2018 01:50 AM, Peter Zijlstra wrote:
>>> @@ -1528,28 +1538,8 @@ static int __change_page_attr(struct cpa
>>>  	 * We have to split the large page:
>>>  	 */
>>>  	err = split_large_page(cpa, kpte, address);
>>> -	if (!err) {
>> ...
>>> -		flush_tlb_all();
>>> +	if (!err)
>>>  		goto repeat;
>>> -	}
>> I _think_ this is the only use of split_large_page()'s error code.  We
>> can just make it return void now, right?
> Then how does it decide to goto repeat or error out?

D'oh.  Good point.  I was misreading the diff.

[tip:x86/mm] x86/mm/cpa: Move flush_tlb_all()

[tip-bot for Peter Zijlstra]

Commit-ID:  c0a759abf5a686a37b9204c13b7e281fe516c8f0
Gitweb:     https://git.kernel.org/tip/c0a759abf5a686a37b9204c13b7e281fe516c8f0
Author:     Peter Zijlstra <peterz@infradead.org>
AuthorDate: Wed, 19 Sep 2018 10:50:18 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 27 Sep 2018 20:39:40 +0200

x86/mm/cpa: Move flush_tlb_all()

There is an atom errata, where we do a local TLB invalidate right
before we return and then do a global TLB invalidate.

Move the global invalidate up a little bit and avoid the local
invalidate entirely.

This does put the global invalidate under pgd_lock, but that shouldn't
matter.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Dave Hansen <dave.hansen@intel.com>
Cc: Bin Yang <bin.yang@intel.com>
Cc: Mark Gross <mark.gross@intel.com>
Link: https://lkml.kernel.org/r/20180919085947.882287392@infradead.org

---
 arch/x86/mm/pageattr.c | 44 +++++++++++++++++---------------------------
 1 file changed, 17 insertions(+), 27 deletions(-)

diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index a22f6b71a308..b6a4c638f086 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -999,14 +999,24 @@ __split_large_page(struct cpa_data *cpa, pte_t *kpte, unsigned long address,
 	__set_pmd_pte(kpte, address, mk_pte(base, __pgprot(_KERNPG_TABLE)));
 
 	/*
-	 * Intel Atom errata AAH41 workaround.
+	 * Do a global flush tlb after splitting the large page
+	 * and before we do the actual change page attribute in the PTE.
 	 *
-	 * The real fix should be in hw or in a microcode update, but
-	 * we also probabilistically try to reduce the window of having
-	 * a large TLB mixed with 4K TLBs while instruction fetches are
-	 * going on.
+	 * Without this, we violate the TLB application note, that says:
+	 * "The TLBs may contain both ordinary and large-page
+	 *  translations for a 4-KByte range of linear addresses. This
+	 *  may occur if software modifies the paging structures so that
+	 *  the page size used for the address range changes. If the two
+	 *  translations differ with respect to page frame or attributes
+	 *  (e.g., permissions), processor behavior is undefined and may
+	 *  be implementation-specific."
+	 *
+	 * We do this global tlb flush inside the cpa_lock, so that we
+	 * don't allow any other cpu, with stale tlb entries change the
+	 * page attribute in parallel, that also falls into the
+	 * just split large page entry.
 	 */
-	__flush_tlb_all();
+	flush_tlb_all();
 	spin_unlock(&pgd_lock);
 
 	return 0;
@@ -1531,28 +1541,8 @@ repeat:
 	 * We have to split the large page:
 	 */
 	err = split_large_page(cpa, kpte, address);
-	if (!err) {
-		/*
-		 * Do a global flush tlb after splitting the large page
-		 * and before we do the actual change page attribute in the PTE.
-		 *
-		 * With out this, we violate the TLB application note, that says
-		 * "The TLBs may contain both ordinary and large-page
-		 *  translations for a 4-KByte range of linear addresses. This
-		 *  may occur if software modifies the paging structures so that
-		 *  the page size used for the address range changes. If the two
-		 *  translations differ with respect to page frame or attributes
-		 *  (e.g., permissions), processor behavior is undefined and may
-		 *  be implementation-specific."
-		 *
-		 * We do this global tlb flush inside the cpa_lock, so that we
-		 * don't allow any other cpu, with stale tlb entries change the
-		 * page attribute in parallel, that also falls into the
-		 * just split large page entry.
-		 */
-		flush_tlb_all();
+	if (!err)
 		goto repeat;
-	}
 
 	return err;
 }

[PATCH 3/8] x86/mm/cpa: Unconditionally avoid WBINDV when we can

[Peter Zijlstra]

CAT has happened, WBINDV is bad (even before CAT blowing away the
entire cache on a multi-core platform wasn't nice), try not to use it
ever.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/mm/pageattr.c |   18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)

--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -319,26 +319,12 @@ static void cpa_flush_array(unsigned lon
 			    int in_flags, struct page **pages)
 {
 	unsigned int i, level;
-#ifdef CONFIG_PREEMPT
-	/*
-	 * Avoid wbinvd() because it causes latencies on all CPUs,
-	 * regardless of any CPU isolation that may be in effect.
-	 *
-	 * This should be extended for CAT enabled systems independent of
-	 * PREEMPT because wbinvd() does not respect the CAT partitions and
-	 * this is exposed to unpriviledged users through the graphics
-	 * subsystem.
-	 */
-	unsigned long do_wbinvd = 0;
-#else
-	unsigned long do_wbinvd = cache && numpages >= 1024; /* 4M threshold */
-#endif
 
 	BUG_ON(irqs_disabled() && !early_boot_irqs_disabled);
 
-	on_each_cpu(__cpa_flush_all, (void *) do_wbinvd, 1);
+	flush_tlb_all();
 
-	if (!cache || do_wbinvd)
+	if (!cache)
 		return;
 
 	/*

[tip:x86/mm] x86/mm/cpa: Unconditionally avoid WBINDV when we can

[tip-bot for Peter Zijlstra]

Commit-ID:  ddd07b750382adc2b78fdfbec47af8a6e0d8ef37
Gitweb:     https://git.kernel.org/tip/ddd07b750382adc2b78fdfbec47af8a6e0d8ef37
Author:     Peter Zijlstra <peterz@infradead.org>
AuthorDate: Wed, 19 Sep 2018 10:50:19 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 27 Sep 2018 20:39:41 +0200

x86/mm/cpa: Unconditionally avoid WBINDV when we can

CAT has happened, WBINDV is bad (even before CAT blowing away the
entire cache on a multi-core platform wasn't nice), try not to use it
ever.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Dave Hansen <dave.hansen@intel.com>
Cc: Bin Yang <bin.yang@intel.com>
Cc: Mark Gross <mark.gross@intel.com>
Link: https://lkml.kernel.org/r/20180919085947.933674526@infradead.org

---
 arch/x86/mm/pageattr.c | 18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index b6a4c638f086..bd9b0ac07352 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -319,26 +319,12 @@ static void cpa_flush_array(unsigned long *start, int numpages, int cache,
 			    int in_flags, struct page **pages)
 {
 	unsigned int i, level;
-#ifdef CONFIG_PREEMPT
-	/*
-	 * Avoid wbinvd() because it causes latencies on all CPUs,
-	 * regardless of any CPU isolation that may be in effect.
-	 *
-	 * This should be extended for CAT enabled systems independent of
-	 * PREEMPT because wbinvd() does not respect the CAT partitions and
-	 * this is exposed to unpriviledged users through the graphics
-	 * subsystem.
-	 */
-	unsigned long do_wbinvd = 0;
-#else
-	unsigned long do_wbinvd = cache && numpages >= 1024; /* 4M threshold */
-#endif
 
 	BUG_ON(irqs_disabled() && !early_boot_irqs_disabled);
 
-	on_each_cpu(__cpa_flush_all, (void *) do_wbinvd, 1);
+	flush_tlb_all();
 
-	if (!cache || do_wbinvd)
+	if (!cache)
 		return;
 
 	/*

[PATCH 4/8] x86/mm/cpa: Use flush_tlb_kernel_range()

[Peter Zijlstra]

Both cpa_flush_range() and cpa_flush_array() have a well specified
range, use that to do a range based TLB invalidate.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/mm/pageattr.c |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -293,7 +293,7 @@ static void cpa_flush_range(unsigned lon
 	BUG_ON(irqs_disabled() && !early_boot_irqs_disabled);
 	WARN_ON(PAGE_ALIGN(start) != start);
 
-	flush_tlb_all();
+	flush_tlb_kernel_range(start, start + PAGE_SIZE * numpages);
 
 	if (!cache)
 		return;
@@ -315,14 +315,15 @@ static void cpa_flush_range(unsigned lon
 	}
 }
 
-static void cpa_flush_array(unsigned long *start, int numpages, int cache,
+static void cpa_flush_array(unsigned long baddr, unsigned long *start,
+			    int numpages, int cache,
 			    int in_flags, struct page **pages)
 {
 	unsigned int i, level;
 
 	BUG_ON(irqs_disabled() && !early_boot_irqs_disabled);
 
-	flush_tlb_all();
+	flush_tlb_kernel_range(baddr, baddr + PAGE_SIZE * numpages);
 
 	if (!cache)
 		return;
@@ -1754,7 +1755,7 @@ static int change_page_attr_set_clr(unsi
 	 */
 	if (!ret && boot_cpu_has(X86_FEATURE_CLFLUSH)) {
 		if (cpa.flags & (CPA_PAGES_ARRAY | CPA_ARRAY)) {
-			cpa_flush_array(addr, numpages, cache,
+			cpa_flush_array(baddr, addr, numpages, cache,
 					cpa.flags, pages);
 		} else
 			cpa_flush_range(baddr, numpages, cache);

[tip:x86/mm] x86/mm/cpa: Use flush_tlb_kernel_range()

[tip-bot for Peter Zijlstra]

Commit-ID:  a7295fd53c39ce781a9792c9dd2c8747bf274160
Gitweb:     https://git.kernel.org/tip/a7295fd53c39ce781a9792c9dd2c8747bf274160
Author:     Peter Zijlstra <peterz@infradead.org>
AuthorDate: Wed, 19 Sep 2018 10:50:20 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 27 Sep 2018 20:39:41 +0200

x86/mm/cpa: Use flush_tlb_kernel_range()

Both cpa_flush_range() and cpa_flush_array() have a well specified
range, use that to do a range based TLB invalidate.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Dave Hansen <dave.hansen@intel.com>
Cc: Bin Yang <bin.yang@intel.com>
Cc: Mark Gross <mark.gross@intel.com>
Link: https://lkml.kernel.org/r/20180919085947.985193217@infradead.org

---
 arch/x86/mm/pageattr.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index bd9b0ac07352..02eb18403594 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -293,7 +293,7 @@ static void cpa_flush_range(unsigned long start, int numpages, int cache)
 	BUG_ON(irqs_disabled() && !early_boot_irqs_disabled);
 	WARN_ON(PAGE_ALIGN(start) != start);
 
-	flush_tlb_all();
+	flush_tlb_kernel_range(start, start + PAGE_SIZE * numpages);
 
 	if (!cache)
 		return;
@@ -315,14 +315,15 @@ static void cpa_flush_range(unsigned long start, int numpages, int cache)
 	}
 }
 
-static void cpa_flush_array(unsigned long *start, int numpages, int cache,
+static void cpa_flush_array(unsigned long baddr, unsigned long *start,
+			    int numpages, int cache,
 			    int in_flags, struct page **pages)
 {
 	unsigned int i, level;
 
 	BUG_ON(irqs_disabled() && !early_boot_irqs_disabled);
 
-	flush_tlb_all();
+	flush_tlb_kernel_range(baddr, baddr + PAGE_SIZE * numpages);
 
 	if (!cache)
 		return;
@@ -1757,7 +1758,7 @@ static int change_page_attr_set_clr(unsigned long *addr, int numpages,
 	 */
 	if (!ret && boot_cpu_has(X86_FEATURE_CLFLUSH)) {
 		if (cpa.flags & (CPA_PAGES_ARRAY | CPA_ARRAY)) {
-			cpa_flush_array(addr, numpages, cache,
+			cpa_flush_array(baddr, addr, numpages, cache,
 					cpa.flags, pages);
 		} else
 			cpa_flush_range(baddr, numpages, cache);

[PATCH 5/8] x86/mm/cpa: Move CLFLUSH test into cpa_flush_range()

[Peter Zijlstra]

Rather than guarding all cpa_flush_range() uses with a CLFLUSH test,
put it inside.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/mm/pageattr.c |   15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -293,6 +293,11 @@ static void cpa_flush_range(unsigned lon
 	BUG_ON(irqs_disabled() && !early_boot_irqs_disabled);
 	WARN_ON(PAGE_ALIGN(start) != start);
 
+	if (!static_cpu_has(X86_FEATURE_CLFLUSH)) {
+		cpa_flush_all(cache);
+		return;
+	}
+
 	flush_tlb_kernel_range(start, start + PAGE_SIZE * numpages);
 
 	if (!cache)
@@ -2075,10 +2080,7 @@ static int __set_memory_enc_dec(unsigned
 	/*
 	 * Before changing the encryption attribute, we need to flush caches.
 	 */
-	if (static_cpu_has(X86_FEATURE_CLFLUSH))
-		cpa_flush_range(start, numpages, 1);
-	else
-		cpa_flush_all(1);
+	cpa_flush_range(start, numpages, 1);
 
 	ret = __change_page_attr_set_clr(&cpa, 1);
 
@@ -2089,10 +2091,7 @@ static int __set_memory_enc_dec(unsigned
 	 * in case TLB flushing gets optimized in the cpa_flush_range()
 	 * path use the same logic as above.
 	 */
-	if (static_cpu_has(X86_FEATURE_CLFLUSH))
-		cpa_flush_range(start, numpages, 0);
-	else
-		cpa_flush_all(0);
+	cpa_flush_range(start, numpages, 0);
 
 	return ret;
 }

[tip:x86/mm] x86/mm/cpa: Move CLFLUSH test into cpa_flush_range()

[tip-bot for Peter Zijlstra]

Commit-ID:  5f464b33b17219a233af1267c621632225bc7acc
Gitweb:     https://git.kernel.org/tip/5f464b33b17219a233af1267c621632225bc7acc
Author:     Peter Zijlstra <peterz@infradead.org>
AuthorDate: Wed, 19 Sep 2018 10:50:21 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 27 Sep 2018 20:39:41 +0200

x86/mm/cpa: Move CLFLUSH test into cpa_flush_range()

Rather than guarding all cpa_flush_range() uses with a CLFLUSH test,
put it inside.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Dave Hansen <dave.hansen@intel.com>
Cc: Bin Yang <bin.yang@intel.com>
Cc: Mark Gross <mark.gross@intel.com>
Link: https://lkml.kernel.org/r/20180919085948.036195503@infradead.org

---
 arch/x86/mm/pageattr.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index 02eb18403594..3cc4a2ae4dbb 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -293,6 +293,11 @@ static void cpa_flush_range(unsigned long start, int numpages, int cache)
 	BUG_ON(irqs_disabled() && !early_boot_irqs_disabled);
 	WARN_ON(PAGE_ALIGN(start) != start);
 
+	if (!static_cpu_has(X86_FEATURE_CLFLUSH)) {
+		cpa_flush_all(cache);
+		return;
+	}
+
 	flush_tlb_kernel_range(start, start + PAGE_SIZE * numpages);
 
 	if (!cache)
@@ -2078,10 +2083,7 @@ static int __set_memory_enc_dec(unsigned long addr, int numpages, bool enc)
 	/*
 	 * Before changing the encryption attribute, we need to flush caches.
 	 */
-	if (static_cpu_has(X86_FEATURE_CLFLUSH))
-		cpa_flush_range(start, numpages, 1);
-	else
-		cpa_flush_all(1);
+	cpa_flush_range(start, numpages, 1);
 
 	ret = __change_page_attr_set_clr(&cpa, 1);
 
@@ -2092,10 +2094,7 @@ static int __set_memory_enc_dec(unsigned long addr, int numpages, bool enc)
 	 * in case TLB flushing gets optimized in the cpa_flush_range()
 	 * path use the same logic as above.
 	 */
-	if (static_cpu_has(X86_FEATURE_CLFLUSH))
-		cpa_flush_range(start, numpages, 0);
-	else
-		cpa_flush_all(0);
+	cpa_flush_range(start, numpages, 0);
 
 	return ret;
 }

[PATCH 6/8] x86/mm/cpa: Move CLFLUSH test into cpa_flush_array()

[Peter Zijlstra]

Rather than guarding cpa_flush_array() users with a CLFLUSH test, put
it inside.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/mm/pageattr.c |   29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -328,6 +328,11 @@ static void cpa_flush_array(unsigned lon
 
 	BUG_ON(irqs_disabled() && !early_boot_irqs_disabled);
 
+	if (!static_cpu_has(X86_FEATURE_CLFLUSH)) {
+		cpa_flush_all(cache);
+		return;
+	}
+
 	flush_tlb_kernel_range(baddr, baddr + PAGE_SIZE * numpages);
 
 	if (!cache)
@@ -1753,19 +1758,19 @@ static int change_page_attr_set_clr(unsi
 	cache = !!pgprot2cachemode(mask_set);
 
 	/*
-	 * On success we use CLFLUSH, when the CPU supports it to
-	 * avoid the WBINVD. If the CPU does not support it and in the
-	 * error case we fall back to cpa_flush_all (which uses
-	 * WBINVD):
-	 */
-	if (!ret && boot_cpu_has(X86_FEATURE_CLFLUSH)) {
-		if (cpa.flags & (CPA_PAGES_ARRAY | CPA_ARRAY)) {
-			cpa_flush_array(baddr, addr, numpages, cache,
-					cpa.flags, pages);
-		} else
-			cpa_flush_range(baddr, numpages, cache);
-	} else
+	 * On error; flush everything to be sure.
+	 */
+	if (ret) {
 		cpa_flush_all(cache);
+		goto out;
+	}
+
+	if (cpa.flags & (CPA_PAGES_ARRAY | CPA_ARRAY)) {
+		cpa_flush_array(baddr, addr, numpages, cache,
+				cpa.flags, pages);
+	} else {
+		cpa_flush_range(baddr, numpages, cache);
+	}
 
 out:
 	return ret;

[tip:x86/mm] x86/mm/cpa: Move CLFLUSH test into cpa_flush_array()

[tip-bot for Peter Zijlstra]

Commit-ID:  fce2ce9544e9f098ba828442221ce99c2a5ecb0f
Gitweb:     https://git.kernel.org/tip/fce2ce9544e9f098ba828442221ce99c2a5ecb0f
Author:     Peter Zijlstra <peterz@infradead.org>
AuthorDate: Wed, 19 Sep 2018 10:50:22 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 27 Sep 2018 20:39:42 +0200

x86/mm/cpa: Move CLFLUSH test into cpa_flush_array()

Rather than guarding cpa_flush_array() users with a CLFLUSH test, put
it inside.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Dave Hansen <dave.hansen@intel.com>
Cc: Bin Yang <bin.yang@intel.com>
Cc: Mark Gross <mark.gross@intel.com>
Link: https://lkml.kernel.org/r/20180919085948.087848187@infradead.org

---
 arch/x86/mm/pageattr.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index 3cc4a2ae4dbb..33d89d505f93 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -328,6 +328,11 @@ static void cpa_flush_array(unsigned long baddr, unsigned long *start,
 
 	BUG_ON(irqs_disabled() && !early_boot_irqs_disabled);
 
+	if (!static_cpu_has(X86_FEATURE_CLFLUSH)) {
+		cpa_flush_all(cache);
+		return;
+	}
+
 	flush_tlb_kernel_range(baddr, baddr + PAGE_SIZE * numpages);
 
 	if (!cache)
@@ -1756,19 +1761,19 @@ static int change_page_attr_set_clr(unsigned long *addr, int numpages,
 	cache = !!pgprot2cachemode(mask_set);
 
 	/*
-	 * On success we use CLFLUSH, when the CPU supports it to
-	 * avoid the WBINVD. If the CPU does not support it and in the
-	 * error case we fall back to cpa_flush_all (which uses
-	 * WBINVD):
+	 * On error; flush everything to be sure.
 	 */
-	if (!ret && boot_cpu_has(X86_FEATURE_CLFLUSH)) {
-		if (cpa.flags & (CPA_PAGES_ARRAY | CPA_ARRAY)) {
-			cpa_flush_array(baddr, addr, numpages, cache,
-					cpa.flags, pages);
-		} else
-			cpa_flush_range(baddr, numpages, cache);
-	} else
+	if (ret) {
 		cpa_flush_all(cache);
+		goto out;
+	}
+
+	if (cpa.flags & (CPA_PAGES_ARRAY | CPA_ARRAY)) {
+		cpa_flush_array(baddr, addr, numpages, cache,
+				cpa.flags, pages);
+	} else {
+		cpa_flush_range(baddr, numpages, cache);
+	}
 
 out:
 	return ret;

[PATCH 7/8] x86/mm/cpa: Factor common code between cpa_flush_*()

[Peter Zijlstra]

The start of cpa_flush_range() and cpa_flush_array() is the same, use
a common function.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/mm/pageattr.c |   29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -285,22 +285,28 @@ static void cpa_flush_all(unsigned long
 	on_each_cpu(__cpa_flush_all, (void *) cache, 1);
 }
 
-static void cpa_flush_range(unsigned long start, int numpages, int cache)
+static bool __cpa_flush_range(unsigned long start, int numpages, int cache)
 {
-	unsigned int i, level;
-	unsigned long addr;
-
 	BUG_ON(irqs_disabled() && !early_boot_irqs_disabled);
+
 	WARN_ON(PAGE_ALIGN(start) != start);
 
 	if (!static_cpu_has(X86_FEATURE_CLFLUSH)) {
 		cpa_flush_all(cache);
-		return;
+		return true;
 	}
 
 	flush_tlb_kernel_range(start, start + PAGE_SIZE * numpages);
 
-	if (!cache)
+	return !cache;
+}
+
+static void cpa_flush_range(unsigned long start, int numpages, int cache)
+{
+	unsigned int i, level;
+	unsigned long addr;
+
+	if (__cpa_flush_range(start, numpages, cache))
 		return;
 
 	/*
@@ -326,16 +332,7 @@ static void cpa_flush_array(unsigned lon
 {
 	unsigned int i, level;
 
-	BUG_ON(irqs_disabled() && !early_boot_irqs_disabled);
-
-	if (!static_cpu_has(X86_FEATURE_CLFLUSH)) {
-		cpa_flush_all(cache);
-		return;
-	}
-
-	flush_tlb_kernel_range(baddr, baddr + PAGE_SIZE * numpages);
-
-	if (!cache)
+	if (__cpa_flush_range(baddr, numpages, cache))
 		return;
 
 	/*

[tip:x86/mm] x86/mm/cpa: Factor common code between cpa_flush_*()

[tip-bot for Peter Zijlstra]

Commit-ID:  47e262ac5b84015c4a101ff51767c464fb7497a6
Gitweb:     https://git.kernel.org/tip/47e262ac5b84015c4a101ff51767c464fb7497a6
Author:     Peter Zijlstra <peterz@infradead.org>
AuthorDate: Wed, 19 Sep 2018 10:50:23 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 27 Sep 2018 20:39:42 +0200

x86/mm/cpa: Factor common code between cpa_flush_*()

The start of cpa_flush_range() and cpa_flush_array() is the same, use
a common function.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Dave Hansen <dave.hansen@intel.com>
Cc: Bin Yang <bin.yang@intel.com>
Cc: Mark Gross <mark.gross@intel.com>
Link: https://lkml.kernel.org/r/20180919085948.138859183@infradead.org

---
 arch/x86/mm/pageattr.c | 29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index 33d89d505f93..dc552824e86a 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -285,22 +285,28 @@ static void cpa_flush_all(unsigned long cache)
 	on_each_cpu(__cpa_flush_all, (void *) cache, 1);
 }
 
-static void cpa_flush_range(unsigned long start, int numpages, int cache)
+static bool __cpa_flush_range(unsigned long start, int numpages, int cache)
 {
-	unsigned int i, level;
-	unsigned long addr;
-
 	BUG_ON(irqs_disabled() && !early_boot_irqs_disabled);
+
 	WARN_ON(PAGE_ALIGN(start) != start);
 
 	if (!static_cpu_has(X86_FEATURE_CLFLUSH)) {
 		cpa_flush_all(cache);
-		return;
+		return true;
 	}
 
 	flush_tlb_kernel_range(start, start + PAGE_SIZE * numpages);
 
-	if (!cache)
+	return !cache;
+}
+
+static void cpa_flush_range(unsigned long start, int numpages, int cache)
+{
+	unsigned int i, level;
+	unsigned long addr;
+
+	if (__cpa_flush_range(start, numpages, cache))
 		return;
 
 	/*
@@ -326,16 +332,7 @@ static void cpa_flush_array(unsigned long baddr, unsigned long *start,
 {
 	unsigned int i, level;
 
-	BUG_ON(irqs_disabled() && !early_boot_irqs_disabled);
-
-	if (!static_cpu_has(X86_FEATURE_CLFLUSH)) {
-		cpa_flush_all(cache);
-		return;
-	}
-
-	flush_tlb_kernel_range(baddr, baddr + PAGE_SIZE * numpages);
-
-	if (!cache)
+	if (__cpa_flush_range(baddr, numpages, cache))
 		return;
 
 	/*

[PATCH 8/8] x86/mm/cpa: Optimize __cpa_flush_range()

[Peter Zijlstra]

If we IPI for WBINDV, then we might as well kill the entire TLB too.
But if we don't have to invalidate cache, there is no reason not to
use a range TLB flush.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/mm/pageattr.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -291,7 +291,7 @@ static bool __cpa_flush_range(unsigned l
 
 	WARN_ON(PAGE_ALIGN(start) != start);
 
-	if (!static_cpu_has(X86_FEATURE_CLFLUSH)) {
+	if (cache && !static_cpu_has(X86_FEATURE_CLFLUSH)) {
 		cpa_flush_all(cache);
 		return true;
 	}

[tip:x86/mm] x86/mm/cpa: Optimize __cpa_flush_range()

[tip-bot for Peter Zijlstra]

Commit-ID:  7904ba8a66f400182a204893c92098994e22a88d
Gitweb:     https://git.kernel.org/tip/7904ba8a66f400182a204893c92098994e22a88d
Author:     Peter Zijlstra <peterz@infradead.org>
AuthorDate: Wed, 19 Sep 2018 10:50:24 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 27 Sep 2018 20:39:42 +0200

x86/mm/cpa: Optimize __cpa_flush_range()

If we IPI for WBINDV, then we might as well kill the entire TLB too.
But if we don't have to invalidate cache, there is no reason not to
use a range TLB flush.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Dave Hansen <dave.hansen@intel.com>
Cc: Bin Yang <bin.yang@intel.com>
Cc: Mark Gross <mark.gross@intel.com>
Link: https://lkml.kernel.org/r/20180919085948.195633798@infradead.org

---
 arch/x86/mm/pageattr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index dc552824e86a..62bb30b4bd2a 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -291,7 +291,7 @@ static bool __cpa_flush_range(unsigned long start, int numpages, int cache)
 
 	WARN_ON(PAGE_ALIGN(start) != start);
 
-	if (!static_cpu_has(X86_FEATURE_CLFLUSH)) {
+	if (cache && !static_cpu_has(X86_FEATURE_CLFLUSH)) {
 		cpa_flush_all(cache);
 		return true;
 	}

[PATCH 0/8] x86/mm/cpa: Further optimizations

[Dave Hansen]

On 09/19/2018 01:50 AM, Peter Zijlstra wrote:
> Dave, can you have a wee look at the Atom errata thing; this does grow the
> amount of code between the page-table update and the TLB flush a bit, but all
> that PTI crud we did a while back already made it a non-trivial amount of code.

Oh, were you worried that the act of sending the IPIs widens the window
vs. just doing the local flush?  We definitely do the
smp_call_function() dance *before* we do the local TLB invalidation in
in on_each_cpu().

But, the existing flush was not a "true" fix anyway.  It just tended to
work around things.

If the issue pops back up, I'd prefer that we just do:

	__flush_tlb_all();
	flush_tlb_all();

Preferably with an actual model/family check for the "AAH41" workaround.
 The real value of what you did was moving the flush_tlb_all() closer,
and that remains either way.

Re: [PATCH 0/8] x86/mm/cpa: Further optimizations

[Dave Hansen]

BTW, on all of these:

Reviewed-by: Dave Hansen <dave.hansen@intel.com>