| Message ID | 20250223194943.3518952-5-riel@surriel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | AMD broadcast TLB invalidation | expand |
On Sun, Feb 23, 2025 at 02:48:54PM -0500, Rik van Riel wrote: > Use broadcast TLB invalidation for kernel addresses when available. > > Remove the need to send IPIs for kernel TLB flushes. > > Signed-off-by: Rik van Riel <riel@surriel.com> > Reviewed-by: Nadav Amit <nadav.amit@gmail.com> > Tested-by: Manali Shukla <Manali.Shukla@amd.com> > Tested-by: Brendan Jackman <jackmanb@google.com> > Tested-by: Michael Kelley <mhklinux@outlook.com> What's the point of keeping those Tested-by tags if you still keep changing the patchset? > --- > arch/x86/mm/tlb.c | 34 ++++++++++++++++++++++++++++++++-- > 1 file changed, 32 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c > index dbcb5c968ff9..59396a3c6e9c 100644 > --- a/arch/x86/mm/tlb.c > +++ b/arch/x86/mm/tlb.c > @@ -1077,6 +1077,20 @@ void flush_tlb_all(void) > on_each_cpu(do_flush_tlb_all, NULL, 1); > } > > +static bool invlpgb_kernel_range_flush(struct flush_tlb_info *info) > +{ > + unsigned long addr; > + unsigned long nr; Oh, c'mon: unsigned long addr, nr; > + > + for (addr = info->start; addr < info->end; addr += nr << PAGE_SHIFT) { > + nr = (info->end - addr) >> PAGE_SHIFT; > + nr = clamp_val(nr, 1, invlpgb_count_max); > + invlpgb_flush_addr_nosync(addr, nr); > + } > + __tlbsync(); > + return true; Unused retval - that function looks like it wants to be void.
> On 24 Feb 2025, at 14:31, Borislav Petkov <bp@alien8.de> wrote: > > On Sun, Feb 23, 2025 at 02:48:54PM -0500, Rik van Riel wrote: >> Use broadcast TLB invalidation for kernel addresses when available. >> >> Remove the need to send IPIs for kernel TLB flushes. >> >> Signed-off-by: Rik van Riel <riel@surriel.com> >> Reviewed-by: Nadav Amit <nadav.amit@gmail.com> Nothing concrete against this patch, but I do not remember reviewing it thoroughly, and I do not see that I sent any “Reviewed-by” tag for it before, so please remove it. I only reviewed thoroughly and gave “reviewed-by” for patch 9. [ I would note at this opportunity that while I managed to convince myself patch 9 is safe, I personally would have considered taking a more defensive approach there. I may reiterate it there. ]
diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c index dbcb5c968ff9..59396a3c6e9c 100644 --- a/arch/x86/mm/tlb.c +++ b/arch/x86/mm/tlb.c @@ -1077,6 +1077,20 @@ void flush_tlb_all(void) on_each_cpu(do_flush_tlb_all, NULL, 1); } +static bool invlpgb_kernel_range_flush(struct flush_tlb_info *info) +{ + unsigned long addr; + unsigned long nr; + + for (addr = info->start; addr < info->end; addr += nr << PAGE_SHIFT) { + nr = (info->end - addr) >> PAGE_SHIFT; + nr = clamp_val(nr, 1, invlpgb_count_max); + invlpgb_flush_addr_nosync(addr, nr); + } + __tlbsync(); + return true; +} + static void do_kernel_range_flush(void *info) { struct flush_tlb_info *f = info; @@ -1087,6 +1101,22 @@ static void do_kernel_range_flush(void *info) flush_tlb_one_kernel(addr); } +static void kernel_tlb_flush_all(struct flush_tlb_info *info) +{ + if (cpu_feature_enabled(X86_FEATURE_INVLPGB)) + invlpgb_flush_all(); + else + on_each_cpu(do_flush_tlb_all, NULL, 1); +} + +static void kernel_tlb_flush_range(struct flush_tlb_info *info) +{ + if (cpu_feature_enabled(X86_FEATURE_INVLPGB)) + invlpgb_kernel_range_flush(info); + else + on_each_cpu(do_kernel_range_flush, info, 1); +} + void flush_tlb_kernel_range(unsigned long start, unsigned long end) { struct flush_tlb_info *info; @@ -1097,9 +1127,9 @@ void flush_tlb_kernel_range(unsigned long start, unsigned long end) TLB_GENERATION_INVALID); if (info->end == TLB_FLUSH_ALL) - on_each_cpu(do_flush_tlb_all, NULL, 1); + kernel_tlb_flush_all(info); else - on_each_cpu(do_kernel_range_flush, info, 1); + kernel_tlb_flush_range(info); put_flush_tlb_info(); }