[RFC v2 5/9] x86/mm: Change cpa_flush() to call flush_kernel_range() directly

Rik van Riel posted 9 patches 6 months, 4 weeks ago
There is a newer version of this series
[RFC v2 5/9] x86/mm: Change cpa_flush() to call flush_kernel_range() directly
Posted by Rik van Riel 6 months, 4 weeks ago
From: Rik van Riel <riel@fb.com>

The function cpa_flush() calls __flush_tlb_one_kernel() and
flush_tlb_all().

Replacing that with a call to flush_tlb_kernel_range() allows
cpa_flush() to make use of INVLPGB or RAR without any additional
changes.

Initialize invlpgb_count_max to 1, since flush_tlb_kernel_range()
can now be called before invlpgb_count_max has been initialized
to the value read from CPUID.

[riel: remove now unused __cpa_flush_tlb]

Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
Signed-off-by: Rik van Riel <riel@surriel.com>
---
 arch/x86/kernel/cpu/amd.c    |  2 +-
 arch/x86/mm/pat/set_memory.c | 20 +++++++-------------
 2 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 93da466dfe2c..b2ad8d13211a 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -31,7 +31,7 @@
 
 #include "cpu.h"
 
-u16 invlpgb_count_max __ro_after_init;
+u16 invlpgb_count_max __ro_after_init = 1;
 
 static inline int rdmsrq_amd_safe(unsigned msr, u64 *p)
 {
diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index 30ab4aced761..2454f5249329 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -399,15 +399,6 @@ static void cpa_flush_all(unsigned long cache)
 	on_each_cpu(__cpa_flush_all, (void *) cache, 1);
 }
 
-static void __cpa_flush_tlb(void *data)
-{
-	struct cpa_data *cpa = data;
-	unsigned int i;
-
-	for (i = 0; i < cpa->numpages; i++)
-		flush_tlb_one_kernel(fix_addr(__cpa_addr(cpa, i)));
-}
-
 static int collapse_large_pages(unsigned long addr, struct list_head *pgtables);
 
 static void cpa_collapse_large_pages(struct cpa_data *cpa)
@@ -444,6 +435,7 @@ static void cpa_collapse_large_pages(struct cpa_data *cpa)
 
 static void cpa_flush(struct cpa_data *cpa, int cache)
 {
+	unsigned long start, end;
 	unsigned int i;
 
 	BUG_ON(irqs_disabled() && !early_boot_irqs_disabled);
@@ -453,10 +445,12 @@ static void cpa_flush(struct cpa_data *cpa, int cache)
 		goto collapse_large_pages;
 	}
 
-	if (cpa->force_flush_all || cpa->numpages > tlb_single_page_flush_ceiling)
-		flush_tlb_all();
-	else
-		on_each_cpu(__cpa_flush_tlb, cpa, 1);
+	start = fix_addr(__cpa_addr(cpa, 0));
+	end = fix_addr(__cpa_addr(cpa, cpa->numpages));
+	if (cpa->force_flush_all)
+		end = TLB_FLUSH_ALL;
+
+	flush_tlb_kernel_range(start, end);
 
 	if (!cache)
 		goto collapse_large_pages;
-- 
2.49.0
Re: [RFC v2 5/9] x86/mm: Change cpa_flush() to call flush_kernel_range() directly
Posted by Dave Hansen 6 months, 4 weeks ago
On 5/19/25 18:02, Rik van Riel wrote:
> The function cpa_flush() calls __flush_tlb_one_kernel() and
> flush_tlb_all().
> 
> Replacing that with a call to flush_tlb_kernel_range() allows
> cpa_flush() to make use of INVLPGB or RAR without any additional
> changes.

Yeah, the pageattr.c flushing code has gone through some twists and
turns over the years but it does indeed look like it has converged to be
awfully close to the other flushing code. It used to do wbinvd() and a
full flush, but the wbinvd() disappeared at some point.

I don't immediately see any downsides to doing this. You could probably
even hoist this up to the top of the series. I think it's a good cleanup
on its own.

Also, I'd make the point in the subject and changelog that this isn't
just changing one function to call another, it's removing some
duplicated functionality and consolidating it to existing common code.

Maybe this for the subject:

	x86/mm: Have cpa_flush() use common TLB flushing infrastructure

One super nit:

> +	start = fix_addr(__cpa_addr(cpa, 0));
> +	end = fix_addr(__cpa_addr(cpa, cpa->numpages));

Please vertically align the fix_addr()'s.
Re: [RFC v2 5/9] x86/mm: Change cpa_flush() to call flush_kernel_range() directly
Posted by Borislav Petkov 6 months, 4 weeks ago
On Mon, May 19, 2025 at 09:02:30PM -0400, Rik van Riel wrote:
> From: Rik van Riel <riel@fb.com>
> 
> The function cpa_flush() calls __flush_tlb_one_kernel() and
> flush_tlb_all().
> 
> Replacing that with a call to flush_tlb_kernel_range() allows
> cpa_flush() to make use of INVLPGB or RAR without any additional
> changes.
> 
> Initialize invlpgb_count_max to 1, since flush_tlb_kernel_range()
> can now be called before invlpgb_count_max has been initialized
> to the value read from CPUID.
> 
> [riel: remove now unused __cpa_flush_tlb]
> 
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> Signed-off-by: Rik van Riel <riel@surriel.com>

Please audit all your SOB chains.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette